fix(gateway): resolve the session DB inside the active profile scope

Fixes #88532. A multiplexed gateway serves every profile from one process, but SessionStore bound a single SessionDB during __init__: self._db = SessionDB() SessionDB(db_path=None) resolves _default_db_path() at call time and does follow the context-local HERMES_HOME override, so the path machinery was already correct. The problem was when it ran: at construction, on the process's own root home, long before any inbound event enters _profile_runtime_scope. Every profile's rows therefore landed in the root state.db, even though the scope had redirected get_hermes_home() correctly for the turn (that helper's own docstring lists "sessions" among what it scopes). The rows still carry the right profile_name, stamped from source.profile by the same handler, so nothing in the data looks wrong. The only visible symptom is the desktop listing a profile's session under the default bot: _open_session_db_for_profile opens profiles/<name>/state.db, which never received the write. Look the handle up through a property instead, resolving the active scope per access and caching one handle per resolved path so a hot inbound path opens SQLite once per profile rather than once per message. Construction stays under the cache lock so a concurrent first message on a profile cannot open and then leak a second handle. Assignment is preserved as an explicit pin, which is what the existing suites rely on when they install a fake handle or disable the DB with store._db = None, and a pin keeps winning across scope changes. Behavior is unchanged when no profile scope is active, so single-profile gateways resolve exactly the path they did before. This does not migrate rows that already landed in the root store; those stay where they are.
2026-08-17 14:53:01 -05:00
parent 10134bd846
commit 17ba992108
2 changed files with 264 additions and 15 deletions
@@ -1235,6 +1235,13 @@ class AsyncSessionStore:
        return _offloaded


+# Sentinel for "no explicit SessionDB has been pinned on this store", so the
+# ``_db`` property can distinguish "resolve from the active profile scope"
+# from a deliberate ``store._db = None`` (which disables the DB and selects
+# the JSONL fallback).  A plain ``None`` cannot express both.
+_DB_UNPINNED = object()
+
+
 class SessionStore:
    """
    Manages session storage and retrieval.
@@ -1285,21 +1292,91 @@ class SessionStore:
            getattr(config, "write_sessions_json", True)
        )
        
-        # Initialize SQLite session database
-        self._db = None
-        try:
-            from hermes_state import SessionDB
-            self._db = SessionDB()
-        except RuntimeError as e:
-            if "live-system guard" in str(e):
-                # Test-isolation guard fired: a pytest-context process
-                # resolved the developer's production state.db. Never
-                # swallow this into the JSONL fallback — the whole point
-                # is a loud, hard failure.
-                raise
-            print(f"[gateway] Warning: SQLite session store unavailable, falling back to JSONL: {e}")
-        except Exception as e:
-            print(f"[gateway] Warning: SQLite session store unavailable, falling back to JSONL: {e}")
+        # Initialize SQLite session database.
+        #
+        # Handles are cached per resolved path and looked up through the
+        # ``_db`` property instead of being bound to one handle here.  A
+        # multiplexed gateway serves every profile from a SINGLE process, so
+        # a handle bound during __init__ is frozen to the process's own root
+        # home; every profile's rows then land in the root state.db even
+        # though ``_profile_runtime_scope`` has already redirected
+        # ``get_hermes_home()`` for the turn (its docstring lists "sessions"
+        # among what it scopes).  The row still carries the right
+        # ``profile_name``, so the damage is invisible in the data and shows
+        # up only as the desktop listing a profile's session under the
+        # default bot -- ``_open_session_db_for_profile`` reads
+        # ``profiles/<name>/state.db``, which never received the write.
+        # See #88532.
+        #
+        # Priming the handle for the current scope here keeps the startup
+        # diagnostics exactly where they were: the live-DB isolation guard
+        # still raises during construction, and the JSONL-fallback warning
+        # is still printed once at startup rather than on first use.
+        self._db_pinned = _DB_UNPINNED
+        self._db_handles: Dict[Path, Any] = {}
+        self._db_handles_lock = threading.Lock()
+        self._open_session_db_for_active_scope()
+
+    def _open_session_db_for_active_scope(self):
+        """Return the SessionDB for the profile scope active on this task.
+
+        ``SessionDB(db_path=None)`` resolves ``_default_db_path()`` at call
+        time, and that helper follows the context-local HERMES_HOME override
+        installed by ``_profile_runtime_scope``.  Resolving here rather than
+        once in ``__init__`` is the whole fix for #88532: it lets the
+        scoping that the multiplexed inbound path already performs actually
+        reach session storage.
+
+        Handles are cached per resolved path, so a hot inbound path opens
+        SQLite once per profile rather than once per message, and two
+        profiles never share a handle.  Construction is done under the lock
+        so a concurrent first message on the same profile cannot open (and
+        then leak) a second handle for the same path.
+
+        A construction failure is cached as ``None`` for that path, matching
+        the previous behavior where a failed startup left ``_db`` None for
+        the life of the store and callers fell back to JSONL.
+        """
+        from hermes_state import SessionDB, _default_db_path
+
+        path = Path(_default_db_path())
+        with self._db_handles_lock:
+            if path in self._db_handles:
+                return self._db_handles[path]
+            db = None
+            try:
+                db = SessionDB()
+            except RuntimeError as e:
+                if "live-system guard" in str(e):
+                    # Test-isolation guard fired: a pytest-context process
+                    # resolved the developer's production state.db. Never
+                    # swallow this into the JSONL fallback — the whole point
+                    # is a loud, hard failure.  Deliberately not cached: the
+                    # guard must fire again on the next attempt.
+                    raise
+                print(f"[gateway] Warning: SQLite session store unavailable, falling back to JSONL: {e}")
+            except Exception as e:
+                print(f"[gateway] Warning: SQLite session store unavailable, falling back to JSONL: {e}")
+            self._db_handles[path] = db
+            return db
+
+    @property
+    def _db(self):
+        """The SessionDB for the active profile scope, or a pinned override.
+
+        Assigning ``store._db`` pins that value for every subsequent read,
+        which is what tests rely on to install a fake or to disable the DB
+        with ``store._db = None``.  Unpinned (the production path), each read
+        resolves the scope so a multiplexed profile's writes reach its own
+        store.
+        """
+        if self._db_pinned is not _DB_UNPINNED:
+            return self._db_pinned
+        return self._open_session_db_for_active_scope()
+
+    @_db.setter
+    def _db(self, value) -> None:
+        self._db_pinned = value

    def _has_active_processes_safe(self, session_key: str, *, context: str) -> bool:
        """Return whether a session has active work, failing closed on registry errors."""
@@ -0,0 +1,172 @@
+"""Regression coverage for #88532.
+
+A multiplexed gateway serves every profile from one process.  ``SessionStore``
+used to bind a single ``SessionDB`` during ``__init__``, freezing it to the
+process's own root home, so a named profile's sessions were physically written
+to the root ``state.db`` even though ``_profile_runtime_scope`` had already
+redirected ``get_hermes_home()`` for that turn.  The rows carried the correct
+``profile_name``, which is why the only visible symptom was the desktop listing
+a profile's session under the default bot: the desktop reads
+``profiles/<name>/state.db``, which never received the write.
+
+These tests pin the handle to the *active* scope rather than to construction
+time.  ``test_write_under_profile_scope_lands_in_profile_store`` is the one
+that reproduces the report; it fails against the pre-fix code with the session
+row sitting in the root store.
+"""
+
+import sqlite3
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from gateway.config import GatewayConfig
+from gateway.session import SessionStore
+from hermes_constants import reset_hermes_home_override, set_hermes_home_override
+
+
+@pytest.fixture
+def multiplex_homes(tmp_path, monkeypatch):
+    """A root home plus a named profile home, with HERMES_HOME on the root.
+
+    Mirrors the reported layout: one gateway process launched under the root
+    home, serving a ``fitness`` profile whose store lives under
+    ``profiles/fitness``.
+    """
+    import hermes_state
+
+    root = tmp_path / "hermes"
+    profile = root / "profiles" / "fitness"
+    root.mkdir(parents=True)
+    profile.mkdir(parents=True)
+    monkeypatch.setenv("HERMES_HOME", str(root))
+
+    # The suite-wide fixture in conftest re-points ``hermes_state.DEFAULT_DB_PATH``
+    # at a fake home, which trips the deliberate escape hatch in
+    # ``_default_db_path()``: a re-pointed constant wins over everything,
+    # including the context-local override.  That is correct for tests that
+    # want one fixed DB, but it would pin every lookup here to a single path
+    # and make these assertions vacuous.  Restore the import-time snapshot so
+    # the hatch is closed and resolution goes through ``get_hermes_home()``,
+    # which is what production does.  ``HERMES_HOME`` above still keeps that
+    # resolution inside ``tmp_path``, so no real store is ever opened.
+    monkeypatch.setattr(
+        hermes_state, "DEFAULT_DB_PATH", hermes_state._IMPORT_DEFAULT_DB_PATH
+    )
+    return root, profile
+
+
+def _make_store(root: Path) -> SessionStore:
+    with patch("gateway.session.SessionStore._ensure_loaded"):
+        store = SessionStore(sessions_dir=root / "sessions", config=GatewayConfig())
+    store._loaded = True
+    return store
+
+
+def _session_ids(db_path: Path) -> set:
+    """Read session ids straight out of a state.db, or empty if absent."""
+    if not db_path.exists():
+        return set()
+    conn = sqlite3.connect(str(db_path))
+    try:
+        rows = conn.execute("SELECT id FROM sessions").fetchall()
+    except sqlite3.OperationalError:
+        # No sessions table: nothing was ever written here.
+        return set()
+    finally:
+        conn.close()
+    return {r[0] for r in rows}
+
+
+def test_store_uses_root_db_when_no_profile_scope_is_active(multiplex_homes):
+    """Single-profile gateways are unaffected: no scope, same path as before."""
+    root, _profile = multiplex_homes
+    store = _make_store(root)
+
+    assert Path(store._db.db_path) == root / "state.db"
+
+
+def test_db_handle_follows_the_active_profile_scope(multiplex_homes):
+    """The handle is resolved per access, not frozen at construction."""
+    root, profile = multiplex_homes
+    store = _make_store(root)
+
+    # Constructed outside any scope, exactly as the gateway constructs it.
+    assert Path(store._db.db_path) == root / "state.db"
+
+    token = set_hermes_home_override(str(profile))
+    try:
+        assert Path(store._db.db_path) == profile / "state.db"
+    finally:
+        reset_hermes_home_override(token)
+
+    # And the scope is restored once the turn's scope exits.
+    assert Path(store._db.db_path) == root / "state.db"
+
+
+def test_write_under_profile_scope_lands_in_profile_store(multiplex_homes):
+    """The reported bug: the row must be in the profile's own file.
+
+    This is the assertion the issue makes by hand with ``sqlite3``: the
+    session for profile ``fitness`` belongs in ``profiles/fitness/state.db``
+    and must NOT be in the root store.
+    """
+    root, profile = multiplex_homes
+    store = _make_store(root)
+
+    token = set_hermes_home_override(str(profile))
+    try:
+        store._db.create_session("20260817_233028_542fda58", "feishu")
+    finally:
+        reset_hermes_home_override(token)
+
+    assert _session_ids(profile / "state.db") == {"20260817_233028_542fda58"}
+    assert _session_ids(root / "state.db") == set()
+
+
+def test_handles_are_cached_per_path(multiplex_homes):
+    """One handle per profile: no reopen per message, no sharing across profiles."""
+    root, profile = multiplex_homes
+    store = _make_store(root)
+
+    root_first = store._db
+    root_second = store._db
+    assert root_first is root_second
+
+    token = set_hermes_home_override(str(profile))
+    try:
+        profile_first = store._db
+        profile_second = store._db
+    finally:
+        reset_hermes_home_override(token)
+
+    assert profile_first is profile_second
+    assert profile_first is not root_first
+
+
+def test_explicitly_pinned_handle_still_wins(multiplex_homes):
+    """``store._db = ...`` remains authoritative for every subsequent read.
+
+    Guardrail rather than a bug reproduction: a large number of existing
+    tests install a fake handle or disable the DB this way, and the property
+    must not quietly resolve past a deliberate assignment.
+    """
+    root, profile = multiplex_homes
+    store = _make_store(root)
+
+    sentinel = object()
+    store._db = sentinel
+    token = set_hermes_home_override(str(profile))
+    try:
+        assert store._db is sentinel
+    finally:
+        reset_hermes_home_override(token)
+
+    # Disabling the DB (the JSONL-fallback path) must survive scope changes.
+    store._db = None
+    token = set_hermes_home_override(str(profile))
+    try:
+        assert store._db is None
+    finally:
+        reset_hermes_home_override(token)