fix(agent): scope subagent reply dedupe to origin message
Made-with: Cursor
This commit is contained in:
@@ -13,6 +13,8 @@ from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.base import BaseChannel
|
||||
from nanobot.channels.manager import ChannelManager
|
||||
from nanobot.config.schema import ChannelsConfig
|
||||
from nanobot.providers.transcription import GroqTranscriptionProvider as _GroqProvider
|
||||
from nanobot.providers.transcription import OpenAITranscriptionProvider as _OpenAIProvider
|
||||
from nanobot.utils.restart import RestartNotice
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -338,9 +340,6 @@ async def test_base_channel_passes_language_to_groq_transcription_provider():
|
||||
# Transcription provider HTTP tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
from nanobot.providers.transcription import GroqTranscriptionProvider as _GroqProvider
|
||||
from nanobot.providers.transcription import OpenAITranscriptionProvider as _OpenAIProvider
|
||||
|
||||
|
||||
class _StubResponse:
|
||||
def raise_for_status(self):
|
||||
@@ -791,6 +790,50 @@ async def test_send_with_retry_skips_send_when_streamed():
|
||||
assert send_delta_called is False
|
||||
|
||||
|
||||
def test_outbound_duplicate_suppression_is_scoped_to_origin_message() -> None:
|
||||
fake_config = SimpleNamespace(
|
||||
channels=ChannelsConfig(send_max_retries=3),
|
||||
providers=SimpleNamespace(groq=SimpleNamespace(api_key="")),
|
||||
)
|
||||
|
||||
mgr = ChannelManager.__new__(ChannelManager)
|
||||
mgr.config = fake_config
|
||||
mgr.bus = MessageBus()
|
||||
mgr.channels = {}
|
||||
mgr._dispatch_task = None
|
||||
mgr._origin_reply_fingerprints = {}
|
||||
|
||||
first = OutboundMessage(
|
||||
channel="feishu",
|
||||
chat_id="chat123",
|
||||
content="Done",
|
||||
metadata={"message_id": "msg-1"},
|
||||
)
|
||||
duplicate = OutboundMessage(
|
||||
channel="feishu",
|
||||
chat_id="chat123",
|
||||
content=" Done ",
|
||||
metadata={"origin_message_id": "msg-1"},
|
||||
)
|
||||
separate_turn = OutboundMessage(
|
||||
channel="feishu",
|
||||
chat_id="chat123",
|
||||
content="Done",
|
||||
metadata={"message_id": "msg-2"},
|
||||
)
|
||||
new_origin_content = OutboundMessage(
|
||||
channel="feishu",
|
||||
chat_id="chat123",
|
||||
content="Done with extra details",
|
||||
metadata={"origin_message_id": "msg-1"},
|
||||
)
|
||||
|
||||
assert mgr._should_suppress_outbound(first) is False
|
||||
assert mgr._should_suppress_outbound(duplicate) is True
|
||||
assert mgr._should_suppress_outbound(separate_turn) is False
|
||||
assert mgr._should_suppress_outbound(new_origin_content) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_with_retry_propagates_cancelled_error():
|
||||
"""_send_with_retry should re-raise CancelledError for graceful shutdown."""
|
||||
|
||||
@@ -2,7 +2,6 @@ import asyncio
|
||||
import zipfile
|
||||
from io import BytesIO
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
@@ -17,19 +16,27 @@ except ImportError:
|
||||
if not DINGTALK_AVAILABLE:
|
||||
pytest.skip("DingTalk dependencies not installed (dingtalk-stream)", allow_module_level=True)
|
||||
|
||||
from nanobot.bus.queue import MessageBus
|
||||
import nanobot.channels.dingtalk as dingtalk_module
|
||||
from nanobot.channels.dingtalk import DingTalkChannel, NanobotDingTalkHandler
|
||||
from nanobot.channels.dingtalk import DingTalkConfig
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.dingtalk import DingTalkChannel, DingTalkConfig, NanobotDingTalkHandler
|
||||
|
||||
|
||||
class _FakeResponse:
|
||||
def __init__(self, status_code: int = 200, json_body: dict | None = None) -> None:
|
||||
def __init__(
|
||||
self,
|
||||
status_code: int = 200,
|
||||
json_body: dict | None = None,
|
||||
*,
|
||||
content: bytes = b"",
|
||||
headers: dict[str, str] | None = None,
|
||||
url: str = "https://example.com/file",
|
||||
) -> None:
|
||||
self.status_code = status_code
|
||||
self._json_body = json_body or {}
|
||||
self.text = "{}"
|
||||
self.content = b""
|
||||
self.headers = {"content-type": "application/json"}
|
||||
self.text = content.decode("utf-8", errors="replace") if content else "{}"
|
||||
self.content = content
|
||||
self.headers = headers or {"content-type": "application/json"}
|
||||
self.url = httpx.URL(url)
|
||||
|
||||
def json(self) -> dict:
|
||||
return self._json_body
|
||||
@@ -46,11 +53,13 @@ class _FakeHttp:
|
||||
return _FakeResponse()
|
||||
|
||||
async def post(self, url: str, json=None, headers=None, **kwargs):
|
||||
self.calls.append({"method": "POST", "url": url, "json": json, "headers": headers})
|
||||
self.calls.append(
|
||||
{"method": "POST", "url": url, "json": json, "headers": headers, "kwargs": kwargs}
|
||||
)
|
||||
return self._next_response()
|
||||
|
||||
async def get(self, url: str, **kwargs):
|
||||
self.calls.append({"method": "GET", "url": url})
|
||||
self.calls.append({"method": "GET", "url": url, "kwargs": kwargs})
|
||||
return self._next_response()
|
||||
|
||||
|
||||
@@ -242,6 +251,245 @@ async def test_download_dingtalk_file(tmp_path, monkeypatch) -> None:
|
||||
assert channel._http.calls[1]["method"] == "GET"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_rejects_private_http_target_before_fetch() -> None:
|
||||
"""Remote media fetches must not reach loopback/private addresses."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"internal secret",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="http://127.0.0.1/admin.txt",
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("http://127.0.0.1/admin.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
assert channel._http.calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_rejects_private_redirect_result() -> None:
|
||||
"""A public-looking media URL must not be accepted after redirecting private."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"metadata bytes",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="http://127.0.0.1/metadata",
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/safe.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
assert len(channel._http.calls) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_rejects_oversized_remote_response(monkeypatch) -> None:
|
||||
"""DingTalk media downloads should enforce a byte cap before upload."""
|
||||
monkeypatch.setattr(dingtalk_module, "DINGTALK_MAX_REMOTE_MEDIA_BYTES", 8, raising=False)
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"123456789",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="https://example.com/large.txt",
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/large.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_does_not_follow_remote_redirects_by_default() -> None:
|
||||
"""Redirects are refused by default instead of followed into internal networks."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
302,
|
||||
headers={"location": "http://127.0.0.1/metadata"},
|
||||
url="https://example.com/redirect.txt",
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
assert channel._http.calls[0]["kwargs"]["follow_redirects"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_follows_safe_redirect_when_explicitly_enabled() -> None:
|
||||
"""Operators can opt in to public redirects without enabling private redirects."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(
|
||||
client_id="app",
|
||||
client_secret="secret",
|
||||
allow_from=["*"],
|
||||
allow_remote_media_redirects=True,
|
||||
),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
302,
|
||||
headers={"location": "https://example.com/final.txt"},
|
||||
url="https://example.com/redirect.txt",
|
||||
),
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"redirected media",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="https://example.com/final.txt",
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt")
|
||||
|
||||
assert (data, filename, content_type) == (b"redirected media", "redirect.txt", "text/plain")
|
||||
assert [call["url"] for call in channel._http.calls] == [
|
||||
"https://example.com/redirect.txt",
|
||||
"https://example.com/final.txt",
|
||||
]
|
||||
assert all(call["kwargs"]["follow_redirects"] is False for call in channel._http.calls)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_blocks_cross_host_redirect_without_allowlist() -> None:
|
||||
"""Redirect opt-in should not allow arbitrary cross-host redirects by default."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(
|
||||
client_id="app",
|
||||
client_secret="secret",
|
||||
allow_from=["*"],
|
||||
allow_remote_media_redirects=True,
|
||||
),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
302,
|
||||
headers={"location": "https://example.org/final.txt"},
|
||||
url="https://example.com/redirect.txt",
|
||||
),
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"cross-host media",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="https://example.org/final.txt",
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
assert [call["url"] for call in channel._http.calls] == ["https://example.com/redirect.txt"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_allows_cross_host_redirect_when_allowlisted() -> None:
|
||||
"""Operators can explicitly allow a known CDN/download host for redirects."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(
|
||||
client_id="app",
|
||||
client_secret="secret",
|
||||
allow_from=["*"],
|
||||
allow_remote_media_redirects=True,
|
||||
remote_media_redirect_allowed_hosts=["example.org"],
|
||||
),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
302,
|
||||
headers={"location": "https://example.org/final.txt"},
|
||||
url="https://example.com/redirect.txt",
|
||||
),
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"cross-host media",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="https://example.org/final.txt",
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt")
|
||||
|
||||
assert (data, filename, content_type) == (b"cross-host media", "redirect.txt", "text/plain")
|
||||
assert [call["url"] for call in channel._http.calls] == [
|
||||
"https://example.com/redirect.txt",
|
||||
"https://example.org/final.txt",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_read_media_bytes_blocks_private_redirect_even_when_redirects_enabled() -> None:
|
||||
"""Redirect opt-in must still validate each hop before fetching it."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(
|
||||
client_id="app",
|
||||
client_secret="secret",
|
||||
allow_from=["*"],
|
||||
allow_remote_media_redirects=True,
|
||||
),
|
||||
MessageBus(),
|
||||
)
|
||||
channel._http = _FakeHttp(
|
||||
responses=[
|
||||
_FakeResponse(
|
||||
302,
|
||||
headers={"location": "http://127.0.0.1/metadata"},
|
||||
url="https://example.com/redirect.txt",
|
||||
),
|
||||
_FakeResponse(
|
||||
200,
|
||||
content=b"internal secret",
|
||||
headers={"content-type": "text/plain"},
|
||||
url="http://127.0.0.1/metadata",
|
||||
),
|
||||
]
|
||||
)
|
||||
|
||||
data, filename, content_type = await channel._read_media_bytes("https://example.com/redirect.txt")
|
||||
|
||||
assert (data, filename, content_type) == (None, None, None)
|
||||
assert [call["url"] for call in channel._http.calls] == ["https://example.com/redirect.txt"]
|
||||
|
||||
|
||||
def test_normalize_upload_payload_zips_html_attachment() -> None:
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
|
||||
@@ -380,6 +380,62 @@ async def test_on_message_skips_typing_for_self_message() -> None:
|
||||
assert client.typing_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_message_skips_pre_startup_event() -> None:
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
client = _FakeAsyncClient("", "", "", None)
|
||||
channel.client = client
|
||||
channel._started_at_ms = 1_000_000
|
||||
|
||||
handled: list[str] = []
|
||||
|
||||
async def _fake_handle_message(**kwargs) -> None:
|
||||
handled.append(kwargs["sender_id"])
|
||||
|
||||
channel._handle_message = _fake_handle_message # type: ignore[method-assign]
|
||||
|
||||
room = SimpleNamespace(room_id="!room:matrix.org", display_name="Test room")
|
||||
old_event = SimpleNamespace(
|
||||
sender="@alice:matrix.org", body="old", source={}, server_timestamp=999_999
|
||||
)
|
||||
fresh_event = SimpleNamespace(
|
||||
sender="@alice:matrix.org", body="fresh", source={}, server_timestamp=1_000_001
|
||||
)
|
||||
|
||||
await channel._on_message(room, old_event)
|
||||
await channel._on_message(room, fresh_event)
|
||||
|
||||
assert handled == ["@alice:matrix.org"]
|
||||
assert client.typing_calls == [
|
||||
("!room:matrix.org", True, TYPING_NOTICE_TIMEOUT_MS),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_media_message_skips_pre_startup_event() -> None:
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
client = _FakeAsyncClient("", "", "", None)
|
||||
channel.client = client
|
||||
channel._started_at_ms = 1_000_000
|
||||
|
||||
handled: list[str] = []
|
||||
|
||||
async def _fake_handle_message(**kwargs) -> None:
|
||||
handled.append(kwargs["sender_id"])
|
||||
|
||||
channel._handle_message = _fake_handle_message # type: ignore[method-assign]
|
||||
|
||||
room = SimpleNamespace(room_id="!room:matrix.org", display_name="Test room")
|
||||
old_event = SimpleNamespace(
|
||||
sender="@alice:matrix.org", body="old", source={}, server_timestamp=999_999
|
||||
)
|
||||
|
||||
await channel._on_media_message(room, old_event)
|
||||
|
||||
assert handled == []
|
||||
assert client.typing_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_message_skips_typing_for_denied_sender() -> None:
|
||||
channel = MatrixChannel(_make_config(allow_from=["@bob:matrix.org"]), MessageBus())
|
||||
@@ -1190,6 +1246,44 @@ async def test_send_progress_keeps_typing_keepalive_running() -> None:
|
||||
await channel.stop()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_empty_content_does_not_call_room_send() -> None:
|
||||
"""Progress messages with empty content must not produce an empty body: '' event."""
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
client = _FakeAsyncClient("", "", "", None)
|
||||
channel.client = client
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="matrix",
|
||||
chat_id="!room:matrix.org",
|
||||
content="",
|
||||
metadata={"_progress": True},
|
||||
)
|
||||
)
|
||||
|
||||
assert client.room_send_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_whitespace_only_content_does_not_call_room_send() -> None:
|
||||
"""Progress messages with whitespace-only content must not produce an empty message."""
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
client = _FakeAsyncClient("", "", "", None)
|
||||
channel.client = client
|
||||
|
||||
await channel.send(
|
||||
OutboundMessage(
|
||||
channel="matrix",
|
||||
chat_id="!room:matrix.org",
|
||||
content=" \n\n ",
|
||||
metadata={"_progress": True},
|
||||
)
|
||||
)
|
||||
|
||||
assert client.room_send_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_clears_typing_when_send_fails() -> None:
|
||||
channel = MatrixChannel(_make_config(), MessageBus())
|
||||
|
||||
Reference in New Issue
Block a user