diff --git a/tests/test_profile_isolation_runtime.py b/tests/test_profile_isolation_runtime.py index ec80265e43d0..23251f5cf64e 100644 --- a/tests/test_profile_isolation_runtime.py +++ b/tests/test_profile_isolation_runtime.py @@ -205,3 +205,170 @@ async def driver(): seen = _under_override(prof_b, lambda: asyncio.run(driver())) assert seen == str(prof_b) + + +# --------------------------------------------------------------------------- +# M3 — RPC handlers that resolve HERMES_HOME before the per-turn binding +# --------------------------------------------------------------------------- + +@pytest.fixture +def gateway_two_profiles(tmp_path, monkeypatch): + """A real profile root: launch profile "launcher" + foreign profile "worker". + + Their ``config.yaml`` files name DIFFERENT default models, so any value that + leaks from the launch profile is visible in the assertion rather than + coincidentally equal (the reason the field-reported rows were ambiguous — + both real profiles happened to name the same default). + + Yields ``(server, launcher_home, worker_home)`` with the gateway module + posed as a backend launched under "launcher": ``HERMES_HOME`` is what the + profile-name resolver reads, ``server._hermes_home`` the import-frozen path + ``_load_cfg`` falls back to when no override is bound. + """ + root = tmp_path / "hermes-root" + launcher = root / "profiles" / "launcher" + worker = root / "profiles" / "worker" + for home, model in ((launcher, "launcher/model-A"), (worker, "worker/model-B")): + home.mkdir(parents=True) + (home / "config.yaml").write_text( + f'model:\n default: "{model}"\n', encoding="utf-8" + ) + + monkeypatch.setenv("HERMES_HOME", str(launcher)) + # _resolve_model checks these first; the host's environment must not decide + # the answer for a test about which config.yaml gets read. + monkeypatch.delenv("HERMES_MODEL", raising=False) + monkeypatch.delenv("HERMES_INFERENCE_MODEL", raising=False) + + from tui_gateway import server + + monkeypatch.setattr(server, "_hermes_home", launcher) + # _load_cfg caches on the resolved path; start from cold and restore after. + monkeypatch.setattr(server, "_cfg_cache", None) + monkeypatch.setattr(server, "_cfg_path", None) + monkeypatch.setattr(server, "_cfg_mtime", None) + return server, launcher, worker + + +def _row(home: Path, session_key: str) -> dict | None: + from hermes_state import SessionDB + + db = SessionDB(db_path=home / "state.db") + try: + return db.get_session(session_key) + finally: + db.close() + + +class TestSessionRowIdentityUsesOwnProfile: + """``_ensure_session_db_row`` runs on the RPC thread, before the turn thread + binds HERMES_HOME — so it must bind the session's home itself. + + This is a permanent corruption, not a transient one: ``_insert_session_row`` + upserts under ``model = COALESCE(sessions.model, excluded.model)``, so the + agent's own later (correct) lazy-create cannot repair a wrong first write. + """ + + def test_row_model_falls_back_to_the_sessions_own_profile_default( + self, gateway_two_profiles + ): + server, _launcher, worker = gateway_two_profiles + + server._ensure_session_db_row( + { + "session_key": "s-worker", + "profile_home": str(worker), + "model_override": None, + } + ) + + row = _row(worker, "s-worker") + assert row is not None, "row must land in the worker profile's state.db" + assert row["model"] == "worker/model-B" + assert row["model"] != "launcher/model-A", "launch profile's default leaked" + + def test_row_is_attributed_to_its_profile(self, gateway_two_profiles): + """A session whose first turn never runs keeps whatever this write left, + so the row has to name its profile up front rather than rely on the + agent's backfill.""" + server, _launcher, worker = gateway_two_profiles + + server._ensure_session_db_row( + {"session_key": "s-worker", "profile_home": str(worker)} + ) + + assert _row(worker, "s-worker")["profile_name"] == "worker" + + def test_launch_profile_session_keeps_its_own_default(self, gateway_two_profiles): + """Control: binding the session's home must not mean "always foreign".""" + server, launcher, _worker = gateway_two_profiles + + server._ensure_session_db_row( + {"session_key": "s-launch", "profile_home": str(launcher)} + ) + + row = _row(launcher, "s-launch") + assert row["model"] == "launcher/model-A" + assert row["profile_name"] == "launcher" + + def test_explicit_composer_pick_still_wins(self, gateway_two_profiles): + """The anti-race intent is unchanged: an explicit pick is never + overwritten by any profile's default.""" + server, _launcher, worker = gateway_two_profiles + + server._ensure_session_db_row( + { + "session_key": "s-picked", + "profile_home": str(worker), + "model_override": {"model": "picked/model-C", "provider": "openrouter"}, + } + ) + + row = _row(worker, "s-picked") + assert row["model"] == "picked/model-C" + + +class TestSessionProfileNameDoesNotFlip: + """``session.create``/lazy-resume answered with the LAUNCH profile's name + while the deferred build — the one caller that DOES bind the home — answered + with the session's, so the client watched ``profile_name`` change under it. + """ + + def test_session_create_reports_the_requested_profile( + self, gateway_two_profiles, monkeypatch + ): + server, _launcher, _worker = gateway_two_profiles + # Building the agent is a separate (network-touching) concern; what is + # under test is the response the client paints from, sent before it. + monkeypatch.setattr(server, "_schedule_agent_build", lambda _sid: None) + monkeypatch.setattr( + server, "_schedule_session_cap_enforcement", lambda *a, **k: None + ) + + resp = server.handle_request( + { + "id": "1", + "method": "session.create", + "params": {"cols": 80, "profile": "worker"}, + } + ) + sid = resp["result"]["session_id"] + try: + info = resp["result"]["info"] + assert info["profile_name"] == "worker" + assert info["model"] == "worker/model-B" + # ...and the deferred build's session.info agrees, so nothing flips. + assert ( + server._session_info(None, server._sessions[sid])["profile_name"] + == "worker" + ) + finally: + server._sessions.pop(sid, None) + + def test_lazy_resume_info_reports_the_sessions_profile(self, gateway_two_profiles): + server, _launcher, worker = gateway_two_profiles + + info = server._lazy_resume_info(str(worker), profile_home=str(worker)) + + assert info["profile_name"] == "worker" + assert info["model"] == "worker/model-B" diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 8fe8e1f938b0..259b3a524cc2 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -3575,7 +3575,7 @@ def test_ensure_session_db_row_persists_explicit_cwd(monkeypatch, tmp_path): created = [] class _FakeDB: - def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None): + def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None, profile_name=None): created.append( {"key": key, "source": source, "model": model, "model_config": model_config, "cwd": cwd} ) @@ -3594,7 +3594,7 @@ def test_ensure_session_db_row_persists_session_source(monkeypatch): created = [] class _FakeDB: - def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None): + def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None, profile_name=None): created.append( {"key": key, "source": source, "model": model, "model_config": model_config, "cwd": cwd} ) @@ -3615,7 +3615,7 @@ def test_ensure_session_db_row_defaults_to_no_workspace(monkeypatch, tmp_path): created = [] class _FakeDB: - def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None): + def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None, profile_name=None): created.append( {"key": key, "source": source, "model": model, "model_config": model_config, "cwd": cwd} ) @@ -3642,7 +3642,7 @@ def test_ensure_session_db_row_persists_session_model_override(monkeypatch): created = [] class _FakeDB: - def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None): + def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None, profile_name=None): created.append( {"key": key, "model": model, "model_config": model_config, "cwd": cwd} ) @@ -3674,7 +3674,7 @@ def test_ensure_session_db_row_no_override_uses_global(monkeypatch): created = [] class _FakeDB: - def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None): + def create_session(self, key, source=None, model=None, model_config=None, parent_session_id=None, cwd=None, profile_name=None): created.append({"model": model, "model_config": model_config}) monkeypatch.setattr(server, "_get_db", lambda: _FakeDB()) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 9eddf0b10071..a94b47954574 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1094,6 +1094,26 @@ def _profile_home(profile: str | None) -> Path | None: return home if (home / "state.db").exists() or home.exists() else None +@contextlib.contextmanager +def _profile_home_bound(profile_home: Path | str | None): + """Bind a session's own HERMES_HOME for the block (no-op without one). + + In app-global remote mode one backend serves every profile, so anything a + handler resolves *from* HERMES_HOME — ``config.yaml`` (hence the default + model) and the profile's own name — silently answers for the LAUNCH profile + unless the session's home is bound first. The per-turn binding happens on + the turn thread; RPC handlers run before it, so they must bind it + themselves. Accepts a ``Path`` or the ``str`` sessions carry in + ``profile_home``; falsy means "launch profile", which is already ambient. + """ + token = set_hermes_home_override(profile_home) if profile_home else None + try: + yield + finally: + if token is not None: + reset_hermes_home_override(token) + + def _profile_scoped(handler): """Bind ``params['profile']``'s HERMES_HOME around a pet RPC handler. @@ -1995,7 +2015,24 @@ def _ensure_session_db_row(session: dict) -> None: # so resume restores effort + fast too, not just the model name. override = session.get("model_override") override = override if isinstance(override, dict) else {} - row_model = str(override.get("model") or "").strip() or _resolve_model() + row_model = str(override.get("model") or "").strip() + # Everything below reads HERMES_HOME, and this helper runs on the RPC thread + # — before the per-turn binding in the turn thread. Unbound, `_resolve_model` + # reads the LAUNCH profile's config.yaml and stamps ITS default onto a + # foreign profile's row, which the same first-writer-wins COALESCE described + # above then makes permanent: the agent's own correct lazy-create can no + # longer repair it. Bind the session's own home so the fallback names the + # model that profile is actually configured for. + with _profile_home_bound(profile_home): + row_model = row_model or _resolve_model() + # Attribute the row to its profile from the start. The agent backfills + # this on its first turn (run_agent.py), so today an unattributed row is + # only briefly wrong — but a session whose turn never runs (a spawn that + # dies before its first prompt) keeps a NULL profile_name forever. + # "default" → NULL mirrors the agent's own normalization, so the two + # writers can't disagree under COALESCE. + resolved_profile = _current_profile_name() + row_profile_name = None if resolved_profile == "default" else resolved_profile model_config: dict = {} for src_key, cfg_key in ( ("model", "model"), @@ -2049,6 +2086,7 @@ def _ensure_session_db_row(session: dict) -> None: model_config=model_config or None, parent_session_id=parent_session_id, cwd=_session_cwd(session) if session.get("explicit_cwd") else None, + profile_name=row_profile_name, ) except Exception: logger.debug("failed to persist desktop session row", exc_info=True) @@ -3897,6 +3935,14 @@ def _session_info(agent, session: dict | None = None) -> dict: ) cfg_personality = ((_load_cfg().get("display") or {}).get("personality") or "") personality = (session or {}).get("personality", cfg_personality) + # Name the session's OWN profile. Only the deferred build calls this with the + # profile home already bound; every other caller is on a thread where the + # ambient HERMES_HOME is the launch profile's, so resolving it here (rather + # than at 28 call sites) is what stops the client seeing profile_name flip + # from the launcher's name to the session's between session.create and the + # build's session.info. + with _profile_home_bound((session or {}).get("profile_home")): + profile_name = _current_profile_name() reasoning_config = getattr(agent, "reasoning_config", None) reasoning_effort = "" if isinstance(reasoning_config, dict): @@ -3951,7 +3997,7 @@ def _session_info(agent, session: dict | None = None) -> dict: "update_behind": None, "update_command": "", "usage": _session_usage_snapshot(session), - "profile_name": _current_profile_name(), + "profile_name": profile_name, } try: from hermes_cli.config import ( @@ -6671,6 +6717,15 @@ def _(rid, params: dict) -> dict: _schedule_agent_build(sid) _schedule_session_cap_enforcement() # trim detached idle sessions over the cap + # The two HERMES_HOME-derived fields of the response below must answer for + # the profile this chat was created *under*. This handler runs on the RPC + # thread with nothing bound, so unbound they report the launch profile and + # the client sees both flip once the deferred build (which does bind the + # home) emits its session.info. + with _profile_home_bound(profile_home): + create_default_model = _resolve_model() if not session_model_override else "" + create_profile_name = _current_profile_name() + return _ok( rid, { @@ -6686,7 +6741,7 @@ def _(rid, params: dict) -> dict: "model": ( session_model_override.get("model") if session_model_override - else _resolve_model() + else create_default_model ), **( {"provider": session_model_override["provider"]} @@ -6700,7 +6755,7 @@ def _(rid, params: dict) -> dict: "project": _project_info_for_cwd(_sessions[sid]["cwd"]), "lazy": True, "desktop_contract": DESKTOP_BACKEND_CONTRACT, - "profile_name": _current_profile_name(), + "profile_name": create_profile_name, }, }, ) @@ -6890,19 +6945,29 @@ def _lazy_resume_info( model: str = "", provider: str = "", stored_session: dict | None = None, + profile_home: Path | str | None = None, ) -> dict: """session.info for a not-yet-built session (the shape session.create - returns). tools/skills land later when the deferred build emits session.info.""" + returns). tools/skills land later when the deferred build emits session.info. + + ``profile_home`` is the resumed session's own home: without it the two + HERMES_HOME-derived fields (the model fallback and the profile name) answer + for the launch profile, so the client paints the wrong ones until the + deferred build's session.info corrects them. + """ + with _profile_home_bound(profile_home): + resolved_model = model or _resolve_model() + profile_name = _current_profile_name() info = { "cwd": cwd, "branch": _git_branch_for_cwd(cwd), "project": _project_info_for_cwd(cwd), - "model": model or _resolve_model(), + "model": resolved_model, "tools": {}, "skills": {}, "lazy": True, "desktop_contract": DESKTOP_BACKEND_CONTRACT, - "profile_name": _current_profile_name(), + "profile_name": profile_name, "usage": _stored_session_usage(stored_session), } if provider: @@ -7143,7 +7208,9 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: "resumed": target, "message_count": len(messages), "messages": messages, - "info": _lazy_resume_info(cwd, stored_session=found), + "info": _lazy_resume_info( + cwd, stored_session=found, profile_home=profile_home + ), "inflight": None, "running": child_running, "session_key": target, @@ -7234,6 +7301,7 @@ def _reuse_live_payload(sid: str, session: dict) -> dict: model=model_override.get("model") or "", provider=overrides.get("provider_override") or "", stored_session=found, + profile_home=profile_home, ), "inflight": None, "running": False,