From 2bb1d80bd909590d5cec666647ecaa86c0caa9ee Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Sun, 30 Aug 2026 21:17:38 -0300 Subject: [PATCH] test(api): cover the declared-conversation precedence through the real handlers The review asked for "real handler + DB coverage for both precedence paths". The first pass did not deliver that: TestBindFollowsPrecedence restated the gate expression inside the test, so it asserted a copy of the rule rather than the rule, and would have stayed green if the handlers stopped applying it. These drive POST /v1/responses and POST /v1/runs over real routes with a real adapter and a real SessionDB: - the declared key selects and records the conversation, and the row it produces carries the key; - three replies on one declared key land on one session id; - an undeclared request keeps a per-request id and records nothing; - a request carrying conversation A's previous_response_id plus a foreign header key settles on A, records nothing, leaves A's own key intact, and the foreign key still cannot recover A -- the end-to-end shape of the blocker; - /v1/runs, which owns its agent lifecycle rather than routing through _run_agent, settles on the declared conversation, and an explicit body session_id outranks the header key without rebinding it. The _run_agent stand-in creates the session row the way AIAgent._ensure_db_session does and performs the bind the way _run_agent's finally block does, so the assertions land on real rows instead of on a mock's call args. /v1/runs is captured at _create_agent for the same reason. The restated-gate tests are kept as the cheap unit layer beneath these. Found in review by @andrexibiza, whose analysis located each of these defects and specified what a correct fix had to prove. Co-Authored-By: Andrex Ibiza, MBA --- .../test_api_server_declared_conversation.py | 224 +++++++++++++++++- 1 file changed, 223 insertions(+), 1 deletion(-) diff --git a/tests/gateway/test_api_server_declared_conversation.py b/tests/gateway/test_api_server_declared_conversation.py index e901f9ad61..f46489d756 100644 --- a/tests/gateway/test_api_server_declared_conversation.py +++ b/tests/gateway/test_api_server_declared_conversation.py @@ -17,9 +17,15 @@ the durable conversation boundaries already recorded in import types import pytest +from aiohttp import web +from aiohttp.test_utils import TestClient, TestServer from gateway.config import PlatformConfig -from gateway.platforms.api_server import APIServerAdapter +from gateway.platforms.api_server import ( + APIServerAdapter, + cors_middleware, + security_headers_middleware, +) from hermes_state import SessionDB KEY = "agent:main:api_server:room-42:member-7" @@ -364,3 +370,219 @@ class TestBindFollowsPrecedence: assert a._declared_conversation_session(KEY) == "sess-A" assert a._declared_conversation_session(OTHER_KEY) is None + + +API_KEY = "test-api-key" + + +@pytest.fixture +def live(tmp_path): + """A real adapter behind real routes, with a scratch state.db attached.""" + adapter = APIServerAdapter( + PlatformConfig(enabled=True, extra={"host": "127.0.0.1", "port": 0, "key": API_KEY}) + ) + db = SessionDB(tmp_path / "state.db") + adapter._session_db = db + + mws = [mw for mw in (cors_middleware, security_headers_middleware) if mw is not None] + app = web.Application(middlewares=mws) + app["api_server_adapter"] = adapter + app.router.add_post("/v1/responses", adapter._handle_responses) + app.router.add_post("/v1/runs", adapter._handle_runs) + try: + yield adapter, db, app + finally: + db.close() + + +def _spy_run_agent(adapter, seen): + """Stand in for _run_agent, faithful to its bind contract. + + Production records the peer in _run_agent's ``finally`` when the caller + opted in; mirroring that here keeps the assertion on the real handler's + decision AND on the row it produces, instead of on a gate expression + restated inside the test. + """ + + async def _fake(**kwargs): + seen.append(kwargs) + # AIAgent._ensure_db_session() creates the row during the turn. + sid = kwargs.get("session_id") + db = adapter._ensure_session_db() + if sid and db is not None and db.get_session(sid) is None: + db.create_session(session_id=sid, source=SOURCE, model="m") + if kwargs.get("bind_declared_conversation"): + adapter._bind_declared_conversation( + kwargs.get("session_id"), kwargs.get("gateway_session_key") + ) + return ( + {"final_response": "ok", "messages": [], "api_calls": 1}, + {"input_tokens": 0, "output_tokens": 0, "total_tokens": 0}, + ) + + return _fake + + +def _headers(session_key=None): + h = {"Authorization": f"Bearer {API_KEY}"} + if session_key: + h["X-Hermes-Session-Key"] = session_key + return h + + +class TestResponsesHandlerPrecedence: + """POST /v1/responses driven end to end, not a restated gate.""" + + @pytest.mark.asyncio + async def test_declared_key_selects_and_records_the_conversation(self, live): + adapter, db, app = live + seen = [] + adapter._run_agent = _spy_run_agent(adapter, seen) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/v1/responses", + json={"model": "hermes-agent", "input": "hi"}, + headers=_headers(KEY), + ) + assert resp.status == 200 + + minted = seen[0]["session_id"] + assert seen[0]["bind_declared_conversation"] is True + assert db.get_session(minted)["session_key"] == KEY + + @pytest.mark.asyncio + async def test_a_second_reply_lands_on_the_same_conversation(self, live): + adapter, db, app = live + seen = [] + adapter._run_agent = _spy_run_agent(adapter, seen) + + async with TestClient(TestServer(app)) as cli: + for _ in range(3): + resp = await cli.post( + "/v1/responses", + json={"model": "hermes-agent", "input": "hi"}, + headers=_headers(KEY), + ) + assert resp.status == 200 + + assert len({k["session_id"] for k in seen}) == 1 + + @pytest.mark.asyncio + async def test_undeclared_request_keeps_a_per_request_id(self, live): + adapter, db, app = live + seen = [] + adapter._run_agent = _spy_run_agent(adapter, seen) + + async with TestClient(TestServer(app)) as cli: + for _ in range(2): + resp = await cli.post( + "/v1/responses", + json={"model": "hermes-agent", "input": "hi"}, + headers=_headers(), + ) + assert resp.status == 200 + + assert len({k["session_id"] for k in seen}) == 2 + assert all(k["bind_declared_conversation"] is False for k in seen) + + @pytest.mark.asyncio + async def test_the_response_chain_outranks_the_header_and_records_nothing(self, live): + """The blocker: a chained turn carrying a foreign key must not rebind.""" + adapter, db, app = live + seen = [] + adapter._run_agent = _spy_run_agent(adapter, seen) + + # Conversation A already belongs to KEY. + _seed(db, "sess-A", key=KEY) + adapter._response_store.put( + "resp_A", + { + "conversation_history": [], + "session_id": "sess-A", + "instructions": None, + }, + ) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/v1/responses", + json={ + "model": "hermes-agent", + "input": "hi", + "previous_response_id": "resp_A", + }, + headers=_headers(OTHER_KEY), + ) + assert resp.status == 200 + + assert seen[0]["session_id"] == "sess-A" + assert seen[0]["bind_declared_conversation"] is False + # A keeps its own key; the header key cannot recover it. + assert db.get_session("sess-A")["session_key"] == KEY + assert adapter._declared_conversation_session(KEY) == "sess-A" + assert adapter._declared_conversation_session(OTHER_KEY) is None + + +class TestRunsHandlerPrecedence: + """POST /v1/runs driven end to end. + + /v1/runs owns its agent lifecycle rather than routing through _run_agent, + so the session it settled on is captured where it builds the agent. + """ + + @staticmethod + def _capture_agent(adapter, seen): + real = adapter._create_agent + + def _spy(*a, **kw): + seen.append(kw) + agent = types.SimpleNamespace( + session_id=kw.get("session_id"), + session_prompt_tokens=0, + session_completion_tokens=0, + session_total_tokens=0, + run_conversation=lambda **_kw: {"final_response": "ok"}, + interrupt=lambda *_a, **_k: None, + ) + return agent + + adapter._create_agent = _spy + return real + + @pytest.mark.asyncio + async def test_declared_key_selects_the_conversation(self, live): + adapter, db, app = live + seen = [] + self._capture_agent(adapter, seen) + _seed(db, "sess-live", key=KEY) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/v1/runs", + json={"model": "hermes-agent", "input": "hi"}, + headers=_headers(KEY), + ) + assert resp.status in (200, 202) + + assert seen and seen[0]["session_id"] == "sess-live" + + @pytest.mark.asyncio + async def test_explicit_body_session_outranks_the_header_key(self, live): + adapter, db, app = live + seen = [] + self._capture_agent(adapter, seen) + _seed(db, "sess-live", key=KEY) + + async with TestClient(TestServer(app)) as cli: + resp = await cli.post( + "/v1/runs", + json={"model": "hermes-agent", "input": "hi", + "session_id": "explicit-session"}, + headers=_headers(KEY), + ) + assert resp.status in (200, 202) + + assert seen and seen[0]["session_id"] == "explicit-session" + # KEY still resolves to its own conversation, never the explicit one. + assert adapter._declared_conversation_session(KEY) == "sess-live"