From ff337a9a47d0f92b1506910f5d8fd7003b8cfc21 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 23 Oct 2026 20:29:23 +0000 Subject: [PATCH 1/4] Fix agent chat session reply: channel membership and slug collision Fix 1 (route only, tinyagentos/routes/agent_chat_session.py, before `record_reply`): work out the target channel the same way `_handle_reply` does (body `channel_id`; else the channel of the message `trace_id` names, via `request.app.state.chat_messages.get_message`). If there is a target channel, load it with `request.app.state.chat_channels.get_channel` and answer 403 `{"error": "not a member of this channel"}` unless the agent's slug is in `channel["members"]`. No target channel = unchanged (falls back to the agent's own DM). Fix 2 (`_resolve_slug`): two registry agents whose canonical ids strip to the same slug must not share a session. After the local-agent collision check, refuse with 403 `{"error": "handle collides with another registry agent"}` when any OTHER active registry entry strips to the same slug (use the registry store on app.state; read tinyagentos/agent_registry_store.py for the listing method, do not invent one). Tests added: reply with foreign channel_id -> 403; reply with trace_id of message in foreign channel -> 403; reply into own DM -> 202 (positive control); second registry agent with colliding slug -> 403. RED-PROOF: ``` .......F.FF... [100%] =================================== FAILURES =================================== _ TestChatSessionSlugCollision.test_second_registry_agent_with_colliding_slug_gets_403 _ self = chat_app = async def test_second_registry_agent_with_colliding_slug_gets_403(self, chat_app): """Two registry agents whose canonical_ids strip to the same slug -> 403.""" _cid1, token1 = await _agent(chat_app, "@agent-one", display_name="Shared Name") _cid2, token2 = await _agent(chat_app, "@agent-two", display_name="Shared Name") # First agent should work mock = AsyncMock() chat_app.state.bridge_sessions.record_reply = mock async with _bare(chat_app) as c: r1 = await c.post(REPLY, headers=_hdr(token1), json={"kind": "final", "content": "hi"}) assert r1.status_code == 202 # Second agent with same slug should be rejected on reply async with _bare(chat_app) as c: r2 = await c.post(REPLY, headers=_hdr(token2), json={"kind": "final", "content": "hi"}) > assert r2.status_code == 403 E assert 202 == 403 E + where 202 = .status_code tests/test_agent_chat_session_routes.py:363: AssertionError ____ TestChatSessionReplyMembership.test_reply_foreign_channel_id_gets_403 _____ self = chat_app_with_stores = async def test_reply_foreign_channel_id_gets_403(self, chat_app_with_stores): """Reply with a channel_id the agent is not a member of -> 403.""" _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") _cid_b, token_b = await _agent(chat_app_with_stores, "@guest-b") # Create a channel that only guest-b is a member of channels = chat_app_with_stores.state.chat_channels foreign_channel = await channels.create_channel( name="private", type="group", created_by="user", members=["user", "guest-b"], ) foreign_channel_id = foreign_channel["id"] mock = AsyncMock() chat_app_with_stores.state.bridge_sessions.record_reply = mock async with _bare(chat_app_with_stores) as c: r = await c.post( REPLY, headers=_hdr(token_a), json={"kind": "final", "content": "hi", "channel_id": foreign_channel_id}, ) > assert r.status_code == 403 E assert 202 == 403 E + where 202 = .status_code tests/test_agent_chat_session_routes.py::TestChatSessionReplyMembership::test_reply_foreign_channel_id_gets_403 _ TestChatSessionReplyMembership.test_reply_trace_id_in_foreign_channel_gets_403 _ self = chat_app_with_stores = async def test_reply_trace_id_in_foreign_channel_gets_403(self, chat_app_with_stores): """Reply with trace_id of a message in a channel the agent is not a member of -> 403.""" _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") _cid_b, token_b = await _agent(chat_app_with_stores, "@guest-b") # Create a channel that only guest-b is a member of channels = chat_app_with_stores.state.chat_channels messages = chat_app_with_stores.state.chat_messages foreign_channel = await channels.create_channel( name="private", type="group", created_by="user", members=["user", "guest-b"], ) foreign_channel_id = foreign_channel["id"] # Create a message in that channel from guest-b msg = await messages.send_message( channel_id=foreign_channel_id, author_id="guest-b", author_type="agent", content="hello from guest-b", state="complete", ) trace_id = msg["id"] mock = AsyncMock() chat_app_with_stores.state.bridge_sessions.record_reply = mock async with _bare(chat_app_with_stores) as c: r = await c.post( REPLY, headers=_hdr(token_a), json={"kind": "final", "content": "hi", "trace_id": trace_id}, ) > assert r.status_code == 403 E assert 202 == 403 E + where 202 = .status_code tests/test_agent_chat_session_routes.py:314: AssertionError =========================== short test summary info ============================ FAILED tests/test_agent_chat_session_routes.py::TestChatSessionSlugCollision::test_second_registry_agent_with_colliding_slug_gets_403 FAILED tests/test_agent_chat_session_routes.py::TestChatSessionReplyMembership::test_reply_foreign_channel_id_gets_403 FAILED tests/test_agent_chat_session_routes.py::TestChatSessionReplyMembership::test_reply_trace_id_in_foreign_channel_gets_403 3 failed, 11 passed in 10.98s ``` GREEN: ``` .............. [100%] 14 passed in 10.44s ``` Docs-Reviewed: Updated docs/agent-coordination.md with new security checks --- .../tsk-wfkhb2-agent-chat-session-fix.md | 4 + docs/agent-coordination.md | 8 + tests/test_agent_chat_session_routes.py | 155 ++++++++++++++++++ tinyagentos/routes/agent_chat_session.py | 61 +++++++ 4 files changed, 228 insertions(+) create mode 100644 changelog.d/tsk-wfkhb2-agent-chat-session-fix.md diff --git a/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md b/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md new file mode 100644 index 000000000..587102d08 --- /dev/null +++ b/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md @@ -0,0 +1,4 @@ +### Fixed + +- Agent chat session reply endpoint now enforces channel membership: a registry agent can only reply in channels where its slug is listed as a member. Requests targeting a foreign channel (via `channel_id` or `trace_id`) return 403 `{"error": "not a member of this channel"}`. +- Registry agent slug collision prevention: when two active registry agents have canonical IDs that strip to the same slug, only the first-registered agent may use the session; subsequent agents receive 403 `{"error": "handle collides with another registry agent"}`. \ No newline at end of file diff --git a/docs/agent-coordination.md b/docs/agent-coordination.md index 54f27b5d1..2a2100665 100644 --- a/docs/agent-coordination.md +++ b/docs/agent-coordination.md @@ -2096,6 +2096,14 @@ never drain or answer a local agent's session. A request with no Bearer is refus middleware before the route runs (401); a JWT without the grant gets 403; an unavailable bridge registry gets 503. +A reply targeting a channel (via explicit `channel_id` or via `trace_id` +resolving to a message's channel) is accepted only when the agent's slug is +a member of that channel; otherwise 403 `not a member of this channel` is +returned and the reply is not recorded. Two registry agents whose canonical +IDs strip to the same slug cannot share a session: only the first-registered +agent may use it; subsequent agents receive 403 `handle collides with another +registry agent`. + ## In-process LLM gateway (`/api/llm/v1`, scoped gateway keys, session or host local token) `tinyagentos/llm_gateway/` is the in-controller replacement for the LiteLLM diff --git a/tests/test_agent_chat_session_routes.py b/tests/test_agent_chat_session_routes.py index 1a115e575..3ed2c4305 100644 --- a/tests/test_agent_chat_session_routes.py +++ b/tests/test_agent_chat_session_routes.py @@ -14,6 +14,8 @@ from tinyagentos.agent_registry_store import mint_registry_token from tinyagentos.bridge_session import BridgeSessionRegistry +from tinyagentos.chat.channel_store import ChatChannelStore +from tinyagentos.chat.message_store import ChatMessageStore from taos_test_csrf import csrf_event_hooks EVENTS = "/api/agents/self/chat/events" @@ -41,6 +43,39 @@ async def chat_app(app, tmp_data_dir): await store.close() +@pytest_asyncio.fixture +async def chat_app_with_stores(app, tmp_data_dir): + """Chat app with real chat_channels and chat_messages stores for membership tests.""" + for attr in ("agent_registry", "agent_grants", "metrics"): + store = getattr(app.state, attr) + if store._db is None: + await store.init() + # Initialise real chat stores + chat_messages = app.state.chat_messages + chat_channels = app.state.chat_channels + if chat_messages._db is None: + await chat_messages.init() + if chat_channels._db is None: + await chat_channels.init() + app.state.bridge_sessions = BridgeSessionRegistry( + trace_registry=None, + chat_messages=chat_messages, + chat_channels=chat_channels, + chat_hub=None, + archive=None, + ) + app.state._startup_complete = True + yield app + for attr in ("agent_registry", "agent_grants", "metrics"): + store = getattr(app.state, attr) + if store._db is not None: + await store.close() + if chat_messages._db is not None: + await chat_messages.close() + if chat_channels._db is not None: + await chat_channels.close() + + async def _agent(app, handle, *, scopes=("chat_session",), display_name=None): registry = app.state.agent_registry priv, _pub = app.state.agent_registry_keypair @@ -209,6 +244,126 @@ async def test_body_identity_is_ignored(self, chat_app): assert "guest-b" not in slugs +@pytest.mark.asyncio +class TestChatSessionReplyMembership: + """Tests for channel membership enforcement on reply (Fix 1).""" + + async def test_reply_foreign_channel_id_gets_403(self, chat_app_with_stores): + """Reply with a channel_id the agent is not a member of -> 403.""" + _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") + _cid_b, token_b = await _agent(chat_app_with_stores, "@guest-b") + + # Create a channel that only guest-b is a member of + channels = chat_app_with_stores.state.chat_channels + foreign_channel = await channels.create_channel( + name="private", + type="group", + created_by="user", + members=["user", "guest-b"], + ) + foreign_channel_id = foreign_channel["id"] + + mock = AsyncMock() + chat_app_with_stores.state.bridge_sessions.record_reply = mock + + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token_a), + json={"kind": "final", "content": "hi", "channel_id": foreign_channel_id}, + ) + assert r.status_code == 403 + assert r.json() == {"error": "not a member of this channel"} + mock.assert_not_called() + + async def test_reply_trace_id_in_foreign_channel_gets_403(self, chat_app_with_stores): + """Reply with trace_id of a message in a channel the agent is not a member of -> 403.""" + _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") + _cid_b, token_b = await _agent(chat_app_with_stores, "@guest-b") + + # Create a channel that only guest-b is a member of + channels = chat_app_with_stores.state.chat_channels + messages = chat_app_with_stores.state.chat_messages + foreign_channel = await channels.create_channel( + name="private", + type="group", + created_by="user", + members=["user", "guest-b"], + ) + foreign_channel_id = foreign_channel["id"] + + # Create a message in that channel from guest-b + msg = await messages.send_message( + channel_id=foreign_channel_id, + author_id="guest-b", + author_type="agent", + content="hello from guest-b", + state="complete", + ) + trace_id = msg["id"] + + mock = AsyncMock() + chat_app_with_stores.state.bridge_sessions.record_reply = mock + + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token_a), + json={"kind": "final", "content": "hi", "trace_id": trace_id}, + ) + assert r.status_code == 403 + assert r.json() == {"error": "not a member of this channel"} + mock.assert_not_called() + + async def test_reply_into_own_dm_is_202(self, chat_app_with_stores): + """Reply into the agent's own DM channel -> 202 (positive control).""" + _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") + + # Ensure the agent has a DM channel (create one if needed) + channels = chat_app_with_stores.state.chat_channels + await channels.create_channel( + name="DM with guest-a", + type="dm", + created_by="user", + members=["user", "guest-a"], + ) + + mock = AsyncMock() + chat_app_with_stores.state.bridge_sessions.record_reply = mock + + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token_a), + json={"kind": "final", "content": "hi"}, + ) + assert r.status_code == 202 + mock.assert_awaited_once() + + +@pytest.mark.asyncio +class TestChatSessionSlugCollision: + """Tests for registry agent slug collision enforcement (Fix 2).""" + + async def test_second_registry_agent_with_colliding_slug_gets_403(self, chat_app): + """Two registry agents whose canonical_ids strip to the same slug -> 403.""" + _cid1, token1 = await _agent(chat_app, "@agent-one", display_name="Shared Name") + _cid2, token2 = await _agent(chat_app, "@agent-two", display_name="Shared Name") + + # First agent should work + mock = AsyncMock() + chat_app.state.bridge_sessions.record_reply = mock + async with _bare(chat_app) as c: + r1 = await c.post(REPLY, headers=_hdr(token1), json={"kind": "final", "content": "hi"}) + assert r1.status_code == 202 + + # Second agent with same slug should be rejected on reply + async with _bare(chat_app) as c: + r2 = await c.post(REPLY, headers=_hdr(token2), json={"kind": "final", "content": "hi"}) + assert r2.status_code == 403 + assert r2.json() == {"error": "handle collides with another registry agent"} + + def test_chat_session_scope_registered_in_both_sets(): from tinyagentos.routes.agent_auth_requests import VALID_SCOPES from tinyagentos.routes.agent_registry import _ALLOWED_SCOPES diff --git a/tinyagentos/routes/agent_chat_session.py b/tinyagentos/routes/agent_chat_session.py index 4821645ae..844e78194 100644 --- a/tinyagentos/routes/agent_chat_session.py +++ b/tinyagentos/routes/agent_chat_session.py @@ -39,6 +39,29 @@ async def _resolve_slug(request: Request) -> str | JSONResponse: return JSONResponse( {"error": "handle collides with a local agent"}, status_code=403 ) + # Two registry agents whose canonical_ids strip to the same slug must not + # share a session. Check for any OTHER active registry entry with the same + # derived slug. Allow the oldest (first registered) agent to proceed; reject + # any newer agents that collide. + registry = getattr(request.app.state, "agent_registry", None) + if registry is not None: + try: + active_agents = await registry.list_all(status="active") + except Exception: + active_agents = [] + # list_all returns agents ordered by id (oldest first). Find the first + # agent with this slug; if it's not the current agent, reject. + for other in active_agents: + other_cid = other.get("canonical_id") + if other_cid: + other_slug = _CANONICAL_AGENT_ID_RE.sub("", other_cid) + if other_slug == slug: + if other_cid != cid: + return JSONResponse( + {"error": "handle collides with another registry agent"}, status_code=403 + ) + # This is the first agent with this slug; allow it. + break return slug @@ -68,6 +91,30 @@ async def event_generator(): ) +async def _resolve_target_channel(request: Request, body: dict, slug: str) -> str | None: + """Determine the target channel for a reply, mirroring _handle_reply logic. + + Returns the channel_id if one can be determined, else None (falls back to + the agent's own DM in the bridge). + """ + # Explicit channel_id in body takes precedence. + channel_id = body.get("channel_id") + if channel_id: + return channel_id + # Fall back to the channel of the message named by trace_id. + trace_id = body.get("trace_id") + if trace_id: + chat_messages = getattr(request.app.state, "chat_messages", None) + if chat_messages is not None: + try: + orig = await chat_messages.get_message(trace_id) + except Exception: + orig = None + if orig and orig.get("channel_id"): + return orig["channel_id"] + return None + + @router.post("/api/agents/self/chat/reply") async def self_chat_reply(request: Request): """Record a reply from the calling agent under its own slug. @@ -86,5 +133,19 @@ async def self_chat_reply(request: Request): return JSONResponse({"error": "invalid JSON"}, status_code=400) if not isinstance(body, dict): return JSONResponse({"error": "invalid JSON"}, status_code=400) + + # Fix 1: enforce channel membership before accepting the reply. + target_channel_id = await _resolve_target_channel(request, body, slug) + if target_channel_id: + chat_channels = getattr(request.app.state, "chat_channels", None) + if chat_channels is not None: + channel = await chat_channels.get_channel(target_channel_id) + if channel: + members = channel.get("members") or [] + if slug not in members: + return JSONResponse( + {"error": "not a member of this channel"}, status_code=403 + ) + await bridge.record_reply(slug, body) return JSONResponse({"accepted": True}, status_code=202) From 17a3a0cd18164356eb07177e32765d3ea014206d Mon Sep 17 00:00:00 2001 From: jaylfc Date: Fri, 23 Oct 2026 20:41:23 +0000 Subject: [PATCH 2/4] address in-house review findings for tsk-wfkhb2 --- changelog.d/tsk-wfkhb2-agent-chat-session-fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md b/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md index 587102d08..219de8d4f 100644 --- a/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md +++ b/changelog.d/tsk-wfkhb2-agent-chat-session-fix.md @@ -1,4 +1,4 @@ ### Fixed - Agent chat session reply endpoint now enforces channel membership: a registry agent can only reply in channels where its slug is listed as a member. Requests targeting a foreign channel (via `channel_id` or `trace_id`) return 403 `{"error": "not a member of this channel"}`. -- Registry agent slug collision prevention: when two active registry agents have canonical IDs that strip to the same slug, only the first-registered agent may use the session; subsequent agents receive 403 `{"error": "handle collides with another registry agent"}`. \ No newline at end of file +- Registry agent slug collision prevention: when two active registry agents have canonical IDs that strip to the same slug, only the first-registered agent may use the session; subsequent agents receive 403 `{"error": "handle collides with another registry agent"}`. From c77da1d4deb5c0c789b6f53ae90e60318dd6f160 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 24 Oct 2026 01:00:35 +0000 Subject: [PATCH 3/4] tsk-bnohyn [OPEN] fix-forward #3940 (tsk-wfkhb2): fail closed on registry errors and unknown channels in the agent reply route ``` FAILED tests/test_agent_chat_session_routes.py::TestChatSessionSlugRegistryError::test_registry_list_all_raises_returns_503_and_no_reply FAILED tests/test_agent_chat_session_routes.py::TestChatSessionUnknownChannel::test_reply_unknown_channel_id_gets_403 2 failed, 15 passed in 23.53s ``` ``` ................. [100%] 17 passed in 23.19s ``` Docs-Reviewed: fail-closed error paths and membership checks match existing route behavior --- ...tsk-bnohyn-agent-chat-reply-fail-closed.md | 2 ++ tests/test_agent_chat_session_routes.py | 31 +++++++++++++++++++ tinyagentos/routes/agent_chat_session.py | 14 ++++++--- 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 changelog.d/tsk-bnohyn-agent-chat-reply-fail-closed.md diff --git a/changelog.d/tsk-bnohyn-agent-chat-reply-fail-closed.md b/changelog.d/tsk-bnohyn-agent-chat-reply-fail-closed.md new file mode 100644 index 000000000..e68199669 --- /dev/null +++ b/changelog.d/tsk-bnohyn-agent-chat-reply-fail-closed.md @@ -0,0 +1,2 @@ +### Fixed +- fail closed on registry errors and unknown channels in the agent reply route diff --git a/tests/test_agent_chat_session_routes.py b/tests/test_agent_chat_session_routes.py index b22b8036f..de0c3953e 100644 --- a/tests/test_agent_chat_session_routes.py +++ b/tests/test_agent_chat_session_routes.py @@ -475,3 +475,34 @@ async def empty_subscribe(slug): assert called_reader_id == "guest-replay" finally: await outbox.close() + + +@pytest.mark.asyncio +class TestChatSessionSlugRegistryError: + async def test_registry_list_all_raises_returns_503_and_no_reply(self, chat_app): + _cid, token = await _agent(chat_app, "@guest-reg-err") + registry = chat_app.state.agent_registry + registry.list_all = AsyncMock(side_effect=RuntimeError("db")) + chat_app.state.bridge_sessions.record_reply = AsyncMock() + async with _bare(chat_app) as c: + r = await c.post(REPLY, headers=_hdr(token), json={"kind": "final", "content": "hi"}) + assert r.status_code == 503 + assert r.json() == {"error": "agent registry unavailable"} + chat_app.state.bridge_sessions.record_reply.assert_not_called() + + +@pytest.mark.asyncio +class TestChatSessionUnknownChannel: + async def test_reply_unknown_channel_id_gets_403(self, chat_app_with_stores): + _cid_a, token_a = await _agent(chat_app_with_stores, "@guest-a") + mock = AsyncMock() + chat_app_with_stores.state.bridge_sessions.record_reply = mock + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token_a), + json={"kind": "final", "content": "hi", "channel_id": "chan-does-not-exist"}, + ) + assert r.status_code == 403 + assert r.json() == {"error": "not a member of this channel"} + mock.assert_not_called() diff --git a/tinyagentos/routes/agent_chat_session.py b/tinyagentos/routes/agent_chat_session.py index 9aae36678..66d595eee 100644 --- a/tinyagentos/routes/agent_chat_session.py +++ b/tinyagentos/routes/agent_chat_session.py @@ -49,8 +49,10 @@ async def _resolve_slug(request: Request) -> str | JSONResponse: try: active_agents = await registry.list_all(status="active") except Exception: - active_agents = [] - # list_all returns agents ordered by id (oldest first). Find the first + logger.warning("agent registry list_all failed while resolving slug", exc_info=True) + return JSONResponse({"error": "agent registry unavailable"}, status_code=503) + # list_all returns agents ordered by id (oldest first) via + # SELECT * FROM agent_registry WHERE status = ? ORDER BY id. Find the first # agent with this slug; if it's not the current agent, reject. for other in active_agents: other_cid = other.get("canonical_id") @@ -149,7 +151,7 @@ async def event_generator(): ) -async def _resolve_target_channel(request: Request, body: dict, slug: str) -> str | None: +async def _resolve_target_channel(request: Request, body: dict) -> str | None: """Determine the target channel for a reply, mirroring _handle_reply logic. Returns the channel_id if one can be determined, else None (falls back to @@ -193,7 +195,7 @@ async def self_chat_reply(request: Request): return JSONResponse({"error": "invalid JSON"}, status_code=400) # Fix 1: enforce channel membership before accepting the reply. - target_channel_id = await _resolve_target_channel(request, body, slug) + target_channel_id = await _resolve_target_channel(request, body) if target_channel_id: chat_channels = getattr(request.app.state, "chat_channels", None) if chat_channels is not None: @@ -204,6 +206,10 @@ async def self_chat_reply(request: Request): return JSONResponse( {"error": "not a member of this channel"}, status_code=403 ) + else: + return JSONResponse( + {"error": "not a member of this channel"}, status_code=403 + ) await bridge.record_reply(slug, body) return JSONResponse({"accepted": True}, status_code=202) From 03b6724597dfd135366e7b9335148cea258d7e1f Mon Sep 17 00:00:00 2001 From: jaylfc Date: Sat, 24 Oct 2026 01:15:26 +0000 Subject: [PATCH 4/4] fix: return 503 when trace_id lookup fails on self chat reply (tsk-4wadcj) ```python tests/test_agent_chat_session_routes.py::TestChatSessionReplyTraceLookupError::test_trace_id_lookup_raises_returns_503_and_no_reply FAILED tests/test_agent_chat_session_routes.py::TestChatSessionReplyTraceLookupError::test_trace_id_not_found_still_returns_202 PASSED 1 failed, 18 passed ``` ```python tests/test_agent_chat_session_routes.py - 19 passed ``` Docs-Reviewed: API behaviour hardened; no agent-coordination doc change required for this specific fail-open fix --- .../3942-agent-chat-reply-trace-lookup-503.md | 2 ++ tests/test_agent_chat_session_routes.py | 32 +++++++++++++++++++ tinyagentos/routes/agent_chat_session.py | 9 ++++-- 3 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 changelog.d/3942-agent-chat-reply-trace-lookup-503.md diff --git a/changelog.d/3942-agent-chat-reply-trace-lookup-503.md b/changelog.d/3942-agent-chat-reply-trace-lookup-503.md new file mode 100644 index 000000000..6e008a057 --- /dev/null +++ b/changelog.d/3942-agent-chat-reply-trace-lookup-503.md @@ -0,0 +1,2 @@ +### Fixed +- Agent chat reply trace_id lookup failures now return 503 instead of falling back to self-DM (tsk-4wadcj) diff --git a/tests/test_agent_chat_session_routes.py b/tests/test_agent_chat_session_routes.py index de0c3953e..0c9b02ea8 100644 --- a/tests/test_agent_chat_session_routes.py +++ b/tests/test_agent_chat_session_routes.py @@ -491,6 +491,38 @@ async def test_registry_list_all_raises_returns_503_and_no_reply(self, chat_app) chat_app.state.bridge_sessions.record_reply.assert_not_called() +@pytest.mark.asyncio +class TestChatSessionReplyTraceLookupError: + async def test_trace_id_lookup_raises_returns_503_and_no_reply(self, chat_app_with_stores): + _cid, token = await _agent(chat_app_with_stores, "@guest-trace-err") + messages = chat_app_with_stores.state.chat_messages + messages.get_message = AsyncMock(side_effect=RuntimeError("db")) + chat_app_with_stores.state.bridge_sessions.record_reply = AsyncMock() + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token), + json={"kind": "final", "content": "hi", "trace_id": "msg-x"}, + ) + assert r.status_code == 503 + assert r.json() == {"error": "chat store unavailable"} + chat_app_with_stores.state.bridge_sessions.record_reply.assert_not_called() + + async def test_trace_id_not_found_still_returns_202(self, chat_app_with_stores): + _cid, token = await _agent(chat_app_with_stores, "@guest-trace-miss") + messages = chat_app_with_stores.state.chat_messages + messages.get_message = AsyncMock(return_value=None) + chat_app_with_stores.state.bridge_sessions.record_reply = AsyncMock() + async with _bare(chat_app_with_stores) as c: + r = await c.post( + REPLY, + headers=_hdr(token), + json={"kind": "final", "content": "hi", "trace_id": "msg-missing"}, + ) + assert r.status_code == 202 + chat_app_with_stores.state.bridge_sessions.record_reply.assert_awaited_once() + + @pytest.mark.asyncio class TestChatSessionUnknownChannel: async def test_reply_unknown_channel_id_gets_403(self, chat_app_with_stores): diff --git a/tinyagentos/routes/agent_chat_session.py b/tinyagentos/routes/agent_chat_session.py index 66d595eee..5838fe675 100644 --- a/tinyagentos/routes/agent_chat_session.py +++ b/tinyagentos/routes/agent_chat_session.py @@ -151,11 +151,11 @@ async def event_generator(): ) -async def _resolve_target_channel(request: Request, body: dict) -> str | None: +async def _resolve_target_channel(request: Request, body: dict) -> str | None | JSONResponse: """Determine the target channel for a reply, mirroring _handle_reply logic. Returns the channel_id if one can be determined, else None (falls back to - the agent's own DM in the bridge). + the agent's own DM in the bridge). Returns a JSONResponse on store errors. """ # Explicit channel_id in body takes precedence. channel_id = body.get("channel_id") @@ -169,7 +169,8 @@ async def _resolve_target_channel(request: Request, body: dict) -> str | None: try: orig = await chat_messages.get_message(trace_id) except Exception: - orig = None + logger.warning("chat message lookup failed while resolving reply target", exc_info=True) + return JSONResponse({"error": "chat store unavailable"}, status_code=503) if orig and orig.get("channel_id"): return orig["channel_id"] return None @@ -196,6 +197,8 @@ async def self_chat_reply(request: Request): # Fix 1: enforce channel membership before accepting the reply. target_channel_id = await _resolve_target_channel(request, body) + if isinstance(target_channel_id, JSONResponse): + return target_channel_id if target_channel_id: chat_channels = getattr(request.app.state, "chat_channels", None) if chat_channels is not None: