From 0a4d3abaf339cba74ea841f9d0455e884da7cfdd Mon Sep 17 00:00:00 2001 From: EndeavorYen Date: Wed, 26 Aug 2026 18:36:54 -0700 Subject: [PATCH] fix(tui): fail closed when a named profile's state.db won't open A deferred agent build for a named-profile session swallowed a failed profile-store open (except Exception -> session_db = None), so _make_agent silently bound the launch _get_db() handle and every turn bled into the wrong profile's state.db exactly when the profile store was briefly unopenable. Opening the named profile then looked blank. Route the open through _open_profile_session_db, which raises a clear 'profile session store unavailable' error instead; the deferred build's existing except path turns that into agent_error + an error event, so the user gets a clear failure and no agent turn against the wrong store. Salvaged from #90219 (hardening half), adapted to main's current deferred-build/_transfer_db_to_agent structure. Related to #87723 and #89789. #88532 covered SessionStore only. --- .../test_named_profile_session_db.py | 166 ++++++++++++++++++ tui_gateway/server.py | 44 +++-- 2 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 tests/hermes_state/test_named_profile_session_db.py diff --git a/tests/hermes_state/test_named_profile_session_db.py b/tests/hermes_state/test_named_profile_session_db.py new file mode 100644 index 0000000000..038c496c23 --- /dev/null +++ b/tests/hermes_state/test_named_profile_session_db.py @@ -0,0 +1,166 @@ +"""Named-profile agents must FAIL CLOSED when their profile state.db won't open. + +Desktop Bot Mode / app-global remote mode talks to a single TUI backend while +stamping ``profile_home`` for a named profile. The deferred agent build opens +that profile's ``state.db`` and hands it to ``_make_agent``. If that open used +to fail, the build silently fell back to the launch handle (``session_db = +None`` → ``_make_agent``'s ``_get_db()`` default), so the session's rows and +messages bled into the wrong profile's store exactly when the profile store +was briefly unopenable — and opening the named profile looked blank. + +These tests pin the fail-closed contract: an unopenable profile store raises a +clear error (no agent turn), and NO path — deferred build or the +``_init_session`` cwd hydration — touches the launch ``state.db`` instead. + +#88532 made SessionStore follow HERMES_HOME; this covers the TUI/agent +SessionDB handle. Related to #87723 and #89789. +""" + +from __future__ import annotations + +import sqlite3 +import threading +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import hermes_state +from hermes_state import SessionDB + + +@pytest.fixture +def homes(tmp_path, monkeypatch): + root = tmp_path / "hermes" + profile = root / "profiles" / "worker" + root.mkdir(parents=True) + profile.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(root)) + monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", hermes_state._IMPORT_DEFAULT_DB_PATH) + return root, profile + + +def _ids(db_path: Path) -> set[str]: + if not db_path.exists(): + return set() + conn = sqlite3.connect(str(db_path)) + try: + try: + return {row[0] for row in conn.execute("SELECT id FROM sessions")} + except sqlite3.OperationalError: + return set() + finally: + conn.close() + + +def _make_store_unopenable(profile: Path) -> Path: + """A directory named state.db — SessionDB cannot open it. Real, no mocks.""" + store = profile / "state.db" + store.mkdir() + return store + + +def test_open_profile_session_db_returns_profile_handle(homes): + root, profile = homes + from tui_gateway import server + + db = server._open_profile_session_db(str(profile)) + try: + db.create_session("20260820_tui_worker", "tui", profile_name="worker") + finally: + db.close() + + assert _ids(profile / "state.db") == {"20260820_tui_worker"} + assert _ids(root / "state.db") == set() + + +def test_open_profile_session_db_raises_when_store_unopenable(homes): + """A genuinely unopenable state.db (directory in its place) must raise.""" + root, profile = homes + from tui_gateway import server + + _make_store_unopenable(profile) + with pytest.raises(RuntimeError, match="profile session store unavailable"): + server._open_profile_session_db(str(profile)) + assert _ids(root / "state.db") == set() + + +def test_open_profile_session_db_does_not_fallback_on_open_failure(homes, monkeypatch): + """SessionDB constructor failure must raise, not reuse/return launch.""" + root, profile = homes + from tui_gateway import server + + class Boom(Exception): + pass + + def boom_db(**_kwargs): + raise Boom("profile store unavailable") + + monkeypatch.setattr("hermes_state.SessionDB", boom_db) + with pytest.raises(RuntimeError, match="profile session store unavailable") as ei: + server._open_profile_session_db(str(profile)) + assert isinstance(ei.value.__cause__, Boom) + assert _ids(root / "state.db") == set() + + +def test_deferred_build_fails_closed_when_profile_store_unopenable(homes, monkeypatch): + """_start_agent_build must error out — never build against the launch DB. + + Before the fix, the deferred build swallowed the open failure + (``except Exception: session_db = None``) and _make_agent bound the + launch ``_get_db()`` handle: the agent ran, and every turn landed in the + wrong profile's state.db. Now the build must record a clear agent_error, + emit an error event, and never reach _make_agent. + """ + root, profile = homes + from tui_gateway import server + + _make_store_unopenable(profile) + launch = SessionDB(db_path=root / "state.db") + sid = "sid-worker-build" + make_agent_calls: list[dict] = [] + events: list[tuple[str, str, dict | None]] = [] + + monkeypatch.setattr(server, "_get_db", lambda: launch) + monkeypatch.setattr(server, "_set_session_context", lambda *a, **kw: []) + monkeypatch.setattr(server, "_clear_session_context", lambda tokens: None) + monkeypatch.setattr( + server, "_emit", lambda event, _sid, payload=None: events.append((event, _sid, payload)) + ) + + def fake_make_agent(_sid, _key, **kw): + make_agent_calls.append(kw) + return SimpleNamespace() + + monkeypatch.setattr(server, "_make_agent", fake_make_agent) + + session = { + "session_key": "key-worker-build", + "agent_ready": threading.Event(), + "profile_home": str(profile), + } + with server._sessions_lock: + server._sessions[sid] = session + try: + server._start_agent_build(sid, session) + thread = session.get("_agent_build_thread") + assert thread is not None + thread.join(timeout=30) + assert not thread.is_alive() + assert session["agent_ready"].is_set() + + # FAIL CLOSED: no agent was built at all — especially not one bound + # to the launch handle. + assert make_agent_calls == [] + assert session.get("agent") is None + assert "profile session store unavailable" in str(session.get("agent_error")) + assert any( + evt == "error" and "agent init failed" in str((payload or {}).get("message")) + for evt, _s, payload in events + ) + # And nothing bled into the launch store. + assert _ids(root / "state.db") == set() + finally: + with server._sessions_lock: + server._sessions.pop(sid, None) + launch.close() diff --git a/tui_gateway/server.py b/tui_gateway/server.py index e6a669af62..8bc77155fd 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1832,6 +1832,29 @@ def _transfer_db_to_agent(agent, db) -> bool: return False +def _open_profile_session_db(profile_home): + """Open a DEDICATED handle on ``profile_home``'s ``state.db`` — FAIL CLOSED. + + A named-profile agent whose profile store cannot be opened must surface a + clear error and never get built against the launch ``state.db``: a silent + fallback bleeds the session's rows and messages into the wrong profile's + store exactly when the profile store is briefly unopenable (locked, + unreadable, mid-restore), and the named profile then looks blank. Callers + let the raised error abort the agent build (deferred builds route it to + the build's ``agent_error`` path) instead of swallowing it back onto the + launch handle. + """ + from hermes_state import SessionDB + + db_path = Path(profile_home) / "state.db" + try: + return SessionDB(db_path=db_path) + except Exception as exc: + raise RuntimeError( + f"profile session store unavailable: {db_path}: {exc}" + ) from exc + + @contextlib.contextmanager def _profile_db(params: dict | None = None): """Yield the SessionDB for ``params['profile']`` (app-global remote mode). @@ -2720,17 +2743,16 @@ def _start_agent_build(sid: str, session: dict) -> None: secret_token = set_secret_scope(build_profile_secret_scope(Path(profile_home))) except Exception: pass - try: - from hermes_state import SessionDB - - # DEDICATED handle — ours until _transfer_db_to_agent hands - # it to the built agent in the finally below. Every path - # that leaves this build without that transfer (the except - # below, and a session reaped mid-build) must close it. - session_db = SessionDB(db_path=Path(profile_home) / "state.db") - owns_db = True - except Exception: - session_db = None + # DEDICATED handle — ours until _transfer_db_to_agent hands + # it to the built agent in the finally below. Every path + # that leaves this build without that transfer (the except + # below, and a session reaped mid-build) must close it. + # FAIL CLOSED on open failure: the raise routes to the + # ``except`` below (clear agent_error, no agent turn) instead + # of silently binding _make_agent's launch-DB default and + # bleeding this session into the wrong profile's state.db. + session_db = _open_profile_session_db(profile_home) + owns_db = True try: from tui_gateway.entry import ensure_mcp_discovery_started