diff --git a/tests/test_issue1897_profile_switch_agent_cache.py b/tests/test_issue1897_profile_switch_agent_cache.py index 9f7a676d..0b82a3a6 100644 --- a/tests/test_issue1897_profile_switch_agent_cache.py +++ b/tests/test_issue1897_profile_switch_agent_cache.py @@ -1,21 +1,11 @@ -"""Regression checks for #1897 — same-session profile switch identity bleed. +"""Regression coverage for #1897 — same-session profile switch identity bleed.""" -When a user switches profiles mid-session in the WebUI, `session_id` stays -stable but the active profile's HERMES_HOME changes. The streaming layer -caches `AIAgent` instances by `session_id` plus a signature blob in -`api/streaming.py`. Before the fix, that signature did NOT include -`_profile_home`, so the second turn after a profile switch reused the agent -built under the previous profile — including its `_cached_system_prompt` -loaded from the OLD profile's SOUL.md. The new persona's identity files -never reached the LLM. - -These tests exercise the source-string contract — a unit-level functional -test would require constructing a full streaming worker. Source-string -tests are sufficient because the signature blob is a single literal list -that drives the cache-hit comparison; if `_profile_home` is in that list, -profile switches force a cache miss and a fresh agent build. -""" +from __future__ import annotations +import os +import queue +import sys +import types from pathlib import Path @@ -24,45 +14,232 @@ STREAMING_PY = (REPO / "api" / "streaming.py").read_text(encoding="utf-8") def _signature_block() -> str: - """Return the literal text of the cache-signature blob.""" sig_start = STREAMING_PY.index("_sig_blob = _json.dumps") sig_end = STREAMING_PY.index("], sort_keys=True)", sig_start) return STREAMING_PY[sig_start:sig_end] +def test_same_session_profile_switch_rebuilds_agent_under_new_soul_home(tmp_path, monkeypatch): + """Switching profiles in one WebUI session must not reuse old SOUL.md. + + The fake AIAgent mirrors the real failure mode: it reads SOUL.md from + HERMES_HOME at construction time and keeps that value in a cached system + prompt. Two consecutive turns on the same profile should reuse the agent; + changing only ``session.profile`` should create a fresh agent whose cached + prompt comes from the new synthetic profile home. + """ + sys.path.insert(0, str(REPO)) + from api import config as cfg + from api import oauth + from api import profiles + from api import streaming + + default_home = tmp_path / "hermes-home" + profile_a_home = default_home / "profiles" / "alpha" + profile_b_home = default_home / "profiles" / "beta" + profile_a_home.mkdir(parents=True) + profile_b_home.mkdir(parents=True) + (profile_a_home / "SOUL.md").write_text( + "PROFILE_ALPHA_SYNTHETIC_SOUL", + encoding="utf-8", + ) + (profile_b_home / "SOUL.md").write_text( + "PROFILE_BETA_SYNTHETIC_SOUL", + encoding="utf-8", + ) + + class FakeSession: + def __init__(self): + self.session_id = "issue1897-same-session" + self.title = "Pinned test title" + self.workspace = str(tmp_path) + self.model = "test-model" + self.model_provider = None + self.profile = "alpha" + self.personality = None + self.messages = [] + self.context_messages = [] + self.tool_calls = [] + self.input_tokens = 0 + self.output_tokens = 0 + self.estimated_cost = None + self.context_length = 0 + self.threshold_tokens = 0 + self.last_prompt_tokens = 0 + self.active_stream_id = None + self.pending_user_message = None + self.pending_attachments = [] + self.pending_started_at = None + self.llm_title_generated = True + + def save(self, *args, **kwargs): + return None + + def compact(self): + return { + "session_id": self.session_id, + "title": self.title, + "workspace": self.workspace, + "model": self.model, + "created_at": 0, + "updated_at": 0, + "pinned": False, + "archived": False, + "project_id": None, + "profile": self.profile, + "input_tokens": self.input_tokens, + "output_tokens": self.output_tokens, + "estimated_cost": self.estimated_cost, + "personality": self.personality, + } + + constructed_agents = [] + prompts_used_for_runs = [] + homes_seen_during_runs = [] + + class SoulCachingAgent: + def __init__(self, **kwargs): + self.session_id = kwargs.get("session_id") + self.model = kwargs.get("model") + self.provider = kwargs.get("provider") + self.base_url = kwargs.get("base_url") + self.context_compressor = None + self.session_prompt_tokens = 0 + self.session_completion_tokens = 0 + self.session_estimated_cost_usd = None + self.ephemeral_system_prompt = None + self._last_error = None + self.stream_delta_callback = kwargs.get("stream_delta_callback") + self.tool_progress_callback = kwargs.get("tool_progress_callback") + self.reasoning_callback = kwargs.get("reasoning_callback") + self.clarify_callback = kwargs.get("clarify_callback") + home = Path(os.environ["HERMES_HOME"]) + self.constructed_home = str(home) + self._cached_system_prompt = (home / "SOUL.md").read_text(encoding="utf-8") + constructed_agents.append(self) + + def run_conversation(self, **kwargs): + prompts_used_for_runs.append(self._cached_system_prompt) + homes_seen_during_runs.append(os.environ.get("HERMES_HOME")) + history = list(kwargs.get("conversation_history") or []) + return { + "messages": history + + [ + {"role": "user", "content": kwargs.get("persist_user_message", "")}, + { + "role": "assistant", + "content": f"reply from {self._cached_system_prompt}", + }, + ] + } + + def interrupt(self, _message): + return None + + fake_session = FakeSession() + fake_runtime_module = types.ModuleType("hermes_cli.runtime_provider") + fake_runtime_module.resolve_runtime_provider = lambda requested=None: { + "provider": requested or "test-provider", + "api_key": "synthetic-key", + "base_url": None, + } + fake_hermes_cli = types.ModuleType("hermes_cli") + fake_hermes_cli.runtime_provider = fake_runtime_module + fake_hermes_state = types.ModuleType("hermes_state") + fake_hermes_state.SessionDB = lambda: None + + def home_for_profile(profile_name): + return {"alpha": profile_a_home, "beta": profile_b_home}[profile_name] + + monkeypatch.setattr(streaming, "get_session", lambda _sid: fake_session) + monkeypatch.setattr(streaming, "_get_ai_agent", lambda: SoulCachingAgent) + monkeypatch.setattr( + streaming, + "resolve_model_provider", + lambda _model: ("test-model", "test-provider", None), + ) + monkeypatch.setattr(streaming, "_maybe_schedule_title_refresh", lambda *args, **kwargs: None) + monkeypatch.setattr(profiles, "get_hermes_home_for_profile", home_for_profile) + monkeypatch.setattr(profiles, "get_profile_runtime_env", lambda _home: {}) + monkeypatch.setattr( + oauth, + "resolve_runtime_provider_with_anthropic_env_lock", + lambda _resolver, requested=None: { + "provider": requested or "test-provider", + "api_key": "synthetic-key", + "base_url": None, + }, + ) + monkeypatch.setattr("api.config.get_config", lambda: {}) + monkeypatch.setattr("api.config._resolve_cli_toolsets", lambda _cfg: []) + monkeypatch.setattr("api.config.load_settings", lambda: {}) + monkeypatch.setitem(sys.modules, "hermes_cli", fake_hermes_cli) + monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_module) + monkeypatch.setitem(sys.modules, "hermes_state", fake_hermes_state) + + with cfg.SESSION_AGENT_CACHE_LOCK: + cfg.SESSION_AGENT_CACHE.clear() + streaming.STREAMS.clear() + streaming.CANCEL_FLAGS.clear() + streaming.AGENT_INSTANCES.clear() + streaming.STREAM_PARTIAL_TEXT.clear() + streaming.STREAM_REASONING_TEXT.clear() + streaming.STREAM_LIVE_TOOL_CALLS.clear() + + def run_turn(profile_name: str, stream_id: str, text: str): + fake_session.profile = profile_name + streaming.STREAMS[stream_id] = queue.Queue() + streaming._run_agent_streaming( + session_id=fake_session.session_id, + msg_text=text, + model="test-model", + model_provider="test-provider", + workspace=str(tmp_path), + stream_id=stream_id, + ) + + run_turn("alpha", "issue1897-stream-1", "first turn") + run_turn("alpha", "issue1897-stream-2", "same profile second turn") + assert len(constructed_agents) == 1, "same-profile turns should reuse the cached agent" + + run_turn("beta", "issue1897-stream-3", "profile switched turn") + + assert prompts_used_for_runs == [ + "PROFILE_ALPHA_SYNTHETIC_SOUL", + "PROFILE_ALPHA_SYNTHETIC_SOUL", + "PROFILE_BETA_SYNTHETIC_SOUL", + ] + assert [agent.constructed_home for agent in constructed_agents] == [ + str(profile_a_home), + str(profile_b_home), + ] + assert homes_seen_during_runs == [ + str(profile_a_home), + str(profile_a_home), + str(profile_b_home), + ] + with cfg.SESSION_AGENT_CACHE_LOCK: + assert cfg.SESSION_AGENT_CACHE[fake_session.session_id][0] is constructed_agents[-1] + + def test_cache_signature_includes_profile_home(): - """The cache signature must include `_profile_home` so that switching - profiles mid-session forces a cache miss instead of reusing the - previous profile's agent (and its baked-in SOUL.md / system prompt).""" block = _signature_block() assert "_profile_home" in block, ( - "SESSION_AGENT_CACHE signature is missing `_profile_home`. " - "Without this, same-session profile switches reuse the cached " - "agent built under the previous profile's HERMES_HOME, leaking " - "the old persona's SOUL.md into the new profile's turns. " - "See #1897." + "SESSION_AGENT_CACHE signature is missing `_profile_home`. Without this, " + "same-session profile switches reuse the cached agent built under the " + "previous profile's HERMES_HOME, leaking the old SOUL.md into new turns." ) def test_profile_home_resolved_before_cache_signature(): - """The `_profile_home` variable must be assigned before the cache - signature is built, otherwise the reference would NameError.""" profile_home_assignment = STREAMING_PY.index("_profile_home = str(_profile_home_path)") sig_start = STREAMING_PY.index("_sig_blob = _json.dumps") - assert profile_home_assignment < sig_start, ( - "`_profile_home` must be resolved before the SESSION_AGENT_CACHE " - "signature is built. If this ordering changed, #1897 would " - "regress with a NameError instead of an identity bleed." - ) + assert profile_home_assignment < sig_start def test_signature_uses_profile_home_with_fallback(): - """The signature must use `_profile_home or ''` — the fallback to an - empty string preserves cache stability when HERMES_HOME is unset - (older deployments / single-profile installs).""" block = _signature_block() assert "_profile_home or ''" in block, ( - "Signature should use `_profile_home or ''` so that single-profile " - "deployments (where _profile_home may be empty) get a stable " - "cache key rather than churning on each turn." + "Signature should use `_profile_home or ''` so empty-home deployments get " + "a stable cache key rather than unnecessary cache churn." )