diff --git a/hermes_cli/web_models.py b/hermes_cli/web_models.py index 86f766ff20..fa5dd37243 100644 --- a/hermes_cli/web_models.py +++ b/hermes_cli/web_models.py @@ -345,6 +345,17 @@ class SessionRename(BaseModel): profile: Optional[str] = None +class SessionOwnerBackfill(BaseModel): + """Body for POST /api/sessions/owner-backfill (#94724 legacy migration). + + ``profile`` scopes WHICH profile's state.db is stamped (same semantics as + every other session route); the stamped value is always that store's own + serving-profile identity — the caller cannot inject an arbitrary owner. + """ + + profile: Optional[str] = None + + # --- from web_server.py (originally lines 12149-12174) --- class SessionPrune(BaseModel): diff --git a/hermes_cli/web_routers/sessions.py b/hermes_cli/web_routers/sessions.py index 32ab0ee8cb..3289e6d52c 100644 --- a/hermes_cli/web_routers/sessions.py +++ b/hermes_cli/web_routers/sessions.py @@ -26,6 +26,7 @@ from hermes_cli.web_deps import late from hermes_cli.web_models import ( BulkDeleteSessions, SessionImport, + SessionOwnerBackfill, SessionPrune, SessionRename, ) @@ -702,6 +703,51 @@ async def delete_session_endpoint(session_id: str, profile: Optional[str] = None return await asyncio.to_thread(_delete) +@manage_router.post("/api/sessions/owner-backfill") +async def backfill_session_owner_profiles(body: SessionOwnerBackfill): + """Stamp legacy ``profile_name = NULL`` session rows with this store's own + serving-profile identity (#94724 legacy-session migration). + + Pre-#95407 rows never recorded an owning profile. That was fine while one + backend served everything, but a Desktop with registry topology (≥2 + registered connections) fails closed on unowned rows by design — leaving + every pre-campaign session unresumable with no migration path. Each + profile's ``state.db`` belongs to exactly one profile, so stamping that + store's own name is a single-match backfill, never a guess; the value + written is the SAME serving-profile identity the list endpoints already + stamp onto outgoing rows (``row_profile`` in ``get_sessions``). Idempotent + and one-shot-per-row: non-NULL owners are never overwritten and a second + call reports 0. + """ + profile_name: Optional[str] = None + if body.profile: + profile_name, _ = _cron_profile_home(body.profile) + stamp = profile_name or _cron_default_profile() + + def _backfill(): + db = _open_session_db_for_profile(body.profile, read_only=False) + try: + return db.backfill_null_session_profiles(stamp) + finally: + db.close() + + try: + stamped = await asyncio.to_thread(_backfill) + except HTTPException: + raise + except Exception: + _log.exception("POST /api/sessions/owner-backfill failed") + raise HTTPException(status_code=500, detail="Internal server error") + + if stamped: + _log.info( + "owner-backfill: stamped %d legacy NULL-profile session row(s) with profile %r", + stamped, + stamp, + ) + return {"ok": True, "stamped": stamped, "profile": stamp} + + @manage_router.patch("/api/sessions/{session_id}") async def rename_session_endpoint(session_id: str, body: SessionRename): """Update a session: rename, archive, hide, pin, and/or mark read/unread. diff --git a/hermes_state.py b/hermes_state.py index e5e35523fd..c53c23f1b6 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -9385,6 +9385,45 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) return self._execute_write(_do) > 0 + def backfill_null_session_profiles(self, profile_name: str) -> int: + """One-shot owner backfill for legacy pre-ownership session rows. + + Sessions created before the durable-ownership work (#95407 lineage) + carry ``profile_name = NULL``. On single-backend installs that was + harmless, but once a Desktop registers a second connection the + fail-closed owner ladder (which is correct for new sessions) can no + longer route those rows anywhere — every pre-campaign session becomes + unresumable after upgrade (#94724, field report). + + This store belongs to exactly one profile — the profile whose + ``state.db`` this is — so stamping its own name onto rows that never + recorded one is a single-match backfill, not a guess. Rules mirror the + ``create_session`` COALESCE contract: + + * only ``NULL``/empty ``profile_name`` rows are touched — a non-NULL + owner is NEVER overwritten; + * idempotent and one-shot-per-row: a second run matches zero rows. + + Returns the number of rows stamped (0 when nothing was legacy). + """ + stamp = (profile_name or "").strip() + if not stamp: + return 0 + + def _do(conn): + cursor = conn.execute( + """UPDATE sessions + SET profile_name = ? + WHERE profile_name IS NULL OR TRIM(profile_name) = ''""", + (stamp,), + ) + rowcount = cursor.rowcount + if rowcount is None or rowcount < 0: + rowcount = conn.execute("SELECT changes()").fetchone()[0] + return rowcount + + return int(self._execute_write(_do) or 0) + def set_session_archived(self, session_id: str, archived: bool) -> bool: """Archive or unarchive a session. diff --git a/tests/hermes_cli/test_session_owner_backfill.py b/tests/hermes_cli/test_session_owner_backfill.py new file mode 100644 index 0000000000..93f1f99864 --- /dev/null +++ b/tests/hermes_cli/test_session_owner_backfill.py @@ -0,0 +1,141 @@ +"""Legacy NULL-profile session owner backfill (#94724). + +Pre-#95407 session rows carry ``profile_name = NULL``. Once a Desktop has +registry topology (≥2 registered connections) the fail-closed owner ladder +can no longer route those rows, making every pre-campaign session +unresumable. POST /api/sessions/owner-backfill stamps each store's own +serving-profile identity onto its legacy rows — single-match by construction +(a profile's state.db belongs to exactly one profile), idempotent, and never +overwriting a non-NULL owner. +""" + +import sqlite3 + +import pytest + + +@pytest.fixture +def client(monkeypatch, _isolate_hermes_home): + try: + from starlette.testclient import TestClient + except ImportError: + pytest.skip("fastapi/starlette not installed") + + import hermes_state + from hermes_constants import get_hermes_home + from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN + + monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db") + + client = TestClient(app) + client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return client + + +def _seed(db_path, rows): + """Insert bare session rows the way a pre-ownership install left them.""" + from hermes_state import SessionDB + + db = SessionDB(db_path=db_path) + try: + for session_id, profile_name in rows: + db.create_session(session_id, source="cli", profile_name=profile_name) + db.append_message( + session_id, role="user", content=f"hello from {session_id}" + ) + # create_session backfills nothing here, but be explicit: force the + # legacy shape at the SQL level so the fixture cannot silently depend + # on create_session's own COALESCE behavior. + for session_id, profile_name in rows: + if profile_name is None: + db._conn.execute( + "UPDATE sessions SET profile_name = NULL WHERE id = ?", + (session_id,), + ) + db._conn.commit() + finally: + db.close() + + +def _profiles(db_path): + conn = sqlite3.connect(str(db_path)) + try: + return dict(conn.execute("SELECT id, profile_name FROM sessions").fetchall()) + finally: + conn.close() + + +def test_backfill_stamps_only_null_rows_and_is_idempotent(client): + from hermes_constants import get_hermes_home + + db_path = get_hermes_home() / "state.db" + _seed( + db_path, + [ + ("legacy-null-1", None), + ("legacy-null-2", None), + ("owned-other", "researcher"), + ], + ) + + resp = client.post("/api/sessions/owner-backfill", json={}) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["ok"] is True + # Exactly the two legacy rows were stamped; the count is reported so the + # caller can log it. + assert body["stamped"] == 2 + assert body["profile"] == "default" + + stamped = _profiles(db_path) + assert stamped["legacy-null-1"] == "default" + assert stamped["legacy-null-2"] == "default" + # Fail-closed contract: a non-NULL owner is NEVER overwritten, even when + # it names a different profile than the serving store. + assert stamped["owned-other"] == "researcher" + + # One-shot-per-row: a second run finds nothing left to stamp and the rows + # are byte-identical. + resp2 = client.post("/api/sessions/owner-backfill", json={}) + assert resp2.status_code == 200 + assert resp2.json()["stamped"] == 0 + assert _profiles(db_path) == stamped + + +def test_backfilled_rows_circulate_owned_on_the_list_endpoint(client): + """After the backfill, the durable stamp (not just the per-response + serving-profile decoration) owns the rows: the raw DB column is non-NULL, + which is what survives into any other consumer of state.db.""" + from hermes_constants import get_hermes_home + + db_path = get_hermes_home() / "state.db" + _seed(db_path, [("legacy-null-3", None)]) + + assert _profiles(db_path)["legacy-null-3"] is None + + resp = client.post("/api/sessions/owner-backfill", json={}) + assert resp.status_code == 200 + assert resp.json()["stamped"] == 1 + + listed = client.get("/api/sessions?limit=50&offset=0").json()["sessions"] + row = next(s for s in listed if s["id"] == "legacy-null-3") + assert row["profile"] == "default" + assert _profiles(db_path)["legacy-null-3"] == "default" + + +def test_backfill_treats_empty_string_profile_as_legacy(client): + """TRIM('') rows are the same stranded class as NULL — stamp them too.""" + from hermes_constants import get_hermes_home + + db_path = get_hermes_home() / "state.db" + _seed(db_path, [("legacy-empty", None)]) + + conn = sqlite3.connect(str(db_path)) + conn.execute("UPDATE sessions SET profile_name = ' ' WHERE id = 'legacy-empty'") + conn.commit() + conn.close() + + resp = client.post("/api/sessions/owner-backfill", json={}) + assert resp.status_code == 200 + assert resp.json()["stamped"] == 1 + assert _profiles(get_hermes_home() / "state.db")["legacy-empty"] == "default"