From 6f689d0b05439039ace7771f955fb885e48263bc Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Sat, 29 Aug 2026 11:10:39 +0530 Subject: [PATCH] test(cache): pin the prompt-cache scope isolation invariant for per-response session ids Squash of the three commits on PR #96768 (net diff is tests-only: the mid-series production hunk in agent/transports/codex.py was reverted within the PR after review). Pins the cache-scope isolation invariant for hosts that mint one physical session per response, plus the system-prompt write-path lifecycle under a per-response session (#96570). --- tests/agent/test_prompt_cache_scope.py | 185 ++++++++++++++++++++++ tests/agent/test_system_prompt_restore.py | 68 ++++++++ 2 files changed, 253 insertions(+) diff --git a/tests/agent/test_prompt_cache_scope.py b/tests/agent/test_prompt_cache_scope.py index d7c9765ba6..c261a487ad 100644 --- a/tests/agent/test_prompt_cache_scope.py +++ b/tests/agent/test_prompt_cache_scope.py @@ -384,3 +384,188 @@ class TestAuxiliaryRuntimeThreading: assert aux._runtime_main_value("cache_scope") == "" finally: aux.reset_runtime_main(token) + + +class TestPerResponseRunNonceIsolation: + """Issue #96570 — hosts that mint one physical session per RESPONSE. + + Hermes Studio group chat builds ``gc_run____`` + for every reply and destroys it when the reply completes + (``groupRuntimeSessionId``), so every conversation-affinity hint Hermes + derives from that id is re-keyed on every reply. What is demonstrated here + is the routing/affinity mechanism moving per response; no provider cache + telemetry or billing outcome is measured or claimed. + + The normalizer cannot repair that from the id alone: a physical session id + is an identity, and Hermes' public session API lets a client choose one + freely (``POST /v1/sessions`` honors ``body["id"]``/``body["session_id"]``). + These tests pin the isolation invariant that any future scope rule has to + keep — collapsing a trailing token because it *looks* like per-run noise + merges independent conversations, which is the failure class already + recorded on #79017. + """ + + RUN = "gc_run_room42_default_Worker" + RESPONSE_1 = f"{RUN}_5f2c1ab9d4e34f7a8b0c6d1e2f3a4b5c" + RESPONSE_2 = f"{RUN}_9a7e3b1c05d24e6fb83a1c7d9e0f2a4b" + + SYS = [ + {"role": "system", "content": "sys"}, + {"role": "user", "content": "hi"}, + ] + + @staticmethod + def _prompt_cache_key(session_id: str) -> str: + from agent.transports.codex import ResponsesApiTransport + + return ResponsesApiTransport().build_kwargs( + model="gpt-5.5", + messages=[{"role": "system", "content": "same"}], + tools=[], + session_id=session_id, + )["prompt_cache_key"] + + def test_external_uuid_identity_remains_isolated(self): + """Two independent API-supplied ids may differ only in a trailing hex. + + ``POST /v1/sessions`` preserves a client-provided id verbatim, so a + whole trailing 32-hex token is not per-run noise by construction. + """ + first = "customer_chat_11111111111141118111111111111111" + second = "customer_chat_22222222222242228222222222222222" + + assert _cache_scope_from_session_id(first) != _cache_scope_from_session_id( + second + ) + assert self._prompt_cache_key(first) != self._prompt_cache_key(second) + + def test_studio_truncation_does_not_merge_members(self): + """Studio truncates the semantic prefix BEFORE appending the nonce. + + ``groupRuntimeSessionId`` slices its ``gc_run___`` + prefix to 96 characters and only then appends the per-response token, + so two members of one room whose names diverge past that boundary are + distinguished *only* by that token. Any rule that drops it merges two + distinct agents onto one affinity key. + """ + room = "mabc1234qwerty" + profile = "default" + common_name = "A" * 70 + + first = ( + f"gc_run_{room}_{profile}_{common_name}Worker"[:96] + + "_11111111111141118111111111111111" + ) + second = ( + f"gc_run_{room}_{profile}_{common_name}Reviewer"[:96] + + "_22222222222242228222222222222222" + ) + + assert first != second + assert _cache_scope_from_session_id(first) != _cache_scope_from_session_id( + second + ) + + def test_cron_normalization_stays_the_only_carve_out(self): + """The one accepted exception: cron's per-fire timestamp (#51395).""" + assert ( + _cache_scope_from_session_id("cron_backup_20260814_120000") + == "cron_backup" + ) + assert ( + _cache_scope_from_session_id(self.RESPONSE_1) == self.RESPONSE_1 + ) + + def test_parentless_rows_resolve_to_their_own_scope(self, db): + """A row with no lineage is its own scope — permanently. + + The Studio bridge creates the row with ``create_session(id, source, + model)`` — no ``parent_session_id`` — so ``resolve_prompt_cache_scope`` + returns the physical id. This is the invariant, not a defect record: + an owner Hermes was never told about must never be guessed. #96811 + closes the gap by having the host declare the logical conversation (a + stable session id, or an explicit key); rows that still declare + nothing keep resolving exactly like this. + """ + db.create_session(self.RESPONSE_1, source="studio") + db.create_session(self.RESPONSE_2, source="studio") + + assert resolve_prompt_cache_scope(_agent(self.RESPONSE_1, db)) == self.RESPONSE_1 + assert resolve_prompt_cache_scope(_agent(self.RESPONSE_2, db)) == self.RESPONSE_2 + + def test_distinct_ids_keep_distinct_affinity_keys(self): + """Every affinity surface isolates two distinct physical ids. + + This is the isolation invariant restated at the wire layer, and it is + also the reported symptom: Studio hands these two ids to the same + logical conversation, so the routing/affinity key moves per response. + + It stays true after #96811. The logical identity is supplied one layer + up — ``cache_scope_id`` here, the ambient conversation context for the + provider profiles — and these call sites pass neither, exactly as an + undeclared conversation would. + """ + from agent.transports.chat_completions import _add_prompt_cache_key + from agent.transports.codex import ResponsesApiTransport + + transport = ResponsesApiTransport() + base = dict(model="gpt-5.5", messages=self.SYS, tools=[]) + assert ( + transport.build_kwargs(**base, session_id=self.RESPONSE_1)[ + "prompt_cache_key" + ] + != transport.build_kwargs(**base, session_id=self.RESPONSE_2)[ + "prompt_cache_key" + ] + ) + + xai = dict(model="grok-4", messages=self.SYS, tools=[], is_xai_responses=True) + assert ( + transport.build_kwargs(**xai, session_id=self.RESPONSE_1)["extra_headers"][ + "x-grok-conv-id" + ] + != transport.build_kwargs(**xai, session_id=self.RESPONSE_2)[ + "extra_headers" + ]["x-grok-conv-id"] + ) + + def chat_key(session_id): + kwargs: dict = {} + _add_prompt_cache_key( + kwargs, + messages=[{"role": "system", "content": "sys"}], + tools=None, + supports_prompt_cache_key=True, + session_id=session_id, + ) + return kwargs.get("prompt_cache_key") + + assert chat_key(self.RESPONSE_1) != chat_key(self.RESPONSE_2) + + @pytest.mark.parametrize("profile_name", ["openrouter", "nous"]) + def test_distinct_ids_keep_distinct_provider_sticky_keys(self, profile_name): + """OpenRouter/Nous route by this key, and it isolates distinct ids. + + Same invariant as above on the sticky-routing surface: two ids of one + Studio conversation re-key it on every reply, and a declared logical + identity would arrive through the conversation contextvar, not from + re-reading this id. + """ + from agent.portal_tags import ( + reset_conversation_context, + set_conversation_context, + ) + from providers import get_provider_profile + + profile = get_provider_profile(profile_name) + keys = [] + for session_id in (self.RESPONSE_1, self.RESPONSE_2): + token = set_conversation_context(session_id) + try: + keys.append( + profile.build_extra_body(session_id=session_id)["session_id"] + ) + finally: + reset_conversation_context(token) + + assert keys[0] != keys[1] diff --git a/tests/agent/test_system_prompt_restore.py b/tests/agent/test_system_prompt_restore.py index ddbbc73c85..d115c64149 100644 --- a/tests/agent/test_system_prompt_restore.py +++ b/tests/agent/test_system_prompt_restore.py @@ -379,5 +379,73 @@ class TestReconstructStaticPrefixMemoization: assert getattr(agent, "_static_rebuild_failed_for", None) is None +class TestPerResponseSessionWritePath: + """The write path under an embedding host's per-response session (#96570). + + Hermes Studio group chat pre-creates the SQLite row and pre-persists the + user message BEFORE ``run_conversation()``, then runs one turn under a + session id it destroys afterwards. The row therefore starts with a null + system prompt and a non-empty history on its own genuine FIRST turn, which + is what trips the "stored system prompt is null" warning — the warning is + a first-turn artifact of that lifecycle, not evidence of a lost write. + + This pins the write path against that exact lifecycle: the freshly built + prompt must land in the pre-created row within the same run. + """ + + def _agent(self, db, session_id): + agent = _make_agent(session_db=db, prebuilt_prompt="GROUP_PROMPT") + agent.session_id = session_id + return agent + + def test_prepersisted_row_stores_the_freshly_built_prompt(self, tmp_path): + from hermes_state import SessionDB + + session_id = "gc_run_room42_default_Worker_5f2c1ab9d4e34f7a8b0c6d1e2f3a4b5c" + with SessionDB(db_path=tmp_path / "state.db") as db: + # What the bridge does before the turn starts. + db.create_session(session_id, source="studio") + db.append_message(session_id=session_id, role="user", content="hi") + + _restore_or_build_system_prompt( + self._agent(db, session_id), + None, + [{"role": "user", "content": "hi"}], + ) + + assert db.get_session(session_id)["system_prompt"] == "GROUP_PROMPT" + + def test_warning_is_a_first_turn_artifact_not_a_lost_write( + self, tmp_path, caplog + ): + """Second turn of the SAME id restores — so nothing was dropped.""" + from hermes_state import SessionDB + + session_id = "gc_run_room42_default_Worker_9a7e3b1c05d24e6fb83a1c7d9e0f2a4b" + history = [{"role": "user", "content": "hi"}] + with SessionDB(db_path=tmp_path / "state.db") as db: + db.create_session(session_id, source="studio") + db.append_message(session_id=session_id, role="user", content="hi") + + with caplog.at_level( + logging.WARNING, logger="agent.conversation_loop" + ): + _restore_or_build_system_prompt( + self._agent(db, session_id), None, history + ) + assert "is null" in caplog.text + + caplog.clear() + second = self._agent(db, session_id) + with caplog.at_level( + logging.WARNING, logger="agent.conversation_loop" + ): + _restore_or_build_system_prompt(second, None, history) + + assert second._cached_system_prompt == "GROUP_PROMPT" + second._build_system_prompt.assert_not_called() + assert "is null" not in caplog.text + + if __name__ == "__main__": pytest.main([__file__, "-v"])