diff --git a/docs/session-lifecycle.md b/docs/session-lifecycle.md index 77ec2a4f48..c3bff2b59b 100644 --- a/docs/session-lifecycle.md +++ b/docs/session-lifecycle.md @@ -666,6 +666,14 @@ When a session expires: | `agent.agent_cache.max_evictions_per_pass` | `int` | `16` | Cap on sessions shed per pressure pass | | `agent.agent_cache.protect_recent` | `int` | `8` | MRU sessions the pressure pass never touches | +## State database and FTS recovery + +The canonical transcript lives in the `sessions` and `messages` tables. FTS5 +tables and their sync triggers are derived indexes that can be detached and +rebuilt without deleting canonical messages. See +[`docs/state-db-recovery.md`](state-db-recovery.md) for the bounded live failure +mode and the explicit repair procedure. + ### Reset Policy (per-platform/type, in config.yaml) ```yaml diff --git a/docs/state-db-recovery.md b/docs/state-db-recovery.md new file mode 100644 index 0000000000..c56b5b5f7a --- /dev/null +++ b/docs/state-db-recovery.md @@ -0,0 +1,61 @@ +# State database and FTS recovery + +`state.db` stores two different data classes: + +- `sessions` and `messages` are the canonical transcript. +- `messages_fts*` tables and their sync triggers are derived search indexes. + +The derived indexes may be detached temporarily. They must not turn a live +message write or search into an unbounded full-transcript rebuild. + +## Live behavior when FTS is corrupt + +If an FTS write or search reports the corruption error class, `SessionDB`: + +1. records the durable `fts_stale` marker; +2. removes the FTS sync triggers in the same transaction; +3. retries canonical writes without the derived-index sinks; and +4. serves searches from canonical rows through the `LIKE` fallback. + +The failing live operation never runs `FTS5('rebuild')`. Existing recovery +ownership remains unchanged: a later `SessionDB` open may rebuild under the +cross-process admission lock and foreign-holder guard. If that guarded rebuild +cannot run, FTS remains detached, canonical writes stay available, and +`hermes doctor` reports the explicit repair command. + +## Explicit repair + +Stop every process that can open the profile database before repairing it. +Keep them stopped for the complete repair and verification window. + +```bash +hermes gateway stop +HERMES_HOME="$HOME/.hermes" hermes sessions repair --check-only +HERMES_HOME="$HOME/.hermes" hermes sessions repair +``` + +`sessions repair` creates a SQLite backup by default and performs structural +work through the repository's guarded snapshot-and-promotion path. Do not copy +`state.db`, `state.db-wal`, and `state.db-shm` independently with `cp`; those +files are one live SQLite image. + +After repair, verify the health probe, stale marker, trigger set, and canonical +row counts before restarting the gateway: + +```bash +HERMES_HOME="$HOME/.hermes" hermes sessions repair --check-only +sqlite3 "$HOME/.hermes/state.db" \ + "SELECT key, value FROM state_meta WHERE key = 'fts_stale';" +sqlite3 "$HOME/.hermes/state.db" \ + "SELECT type, name FROM sqlite_master WHERE name IN + ('messages_fts_insert','messages_fts_update','messages_fts_delete') + ORDER BY name;" +sqlite3 "$HOME/.hermes/state.db" \ + "SELECT 'sessions', COUNT(*) FROM sessions + UNION ALL SELECT 'messages', COUNT(*) FROM messages;" +``` + +The marker query should return no row, the expected FTS triggers should be +present, and canonical row counts must not decrease. If repair fails, preserve +both the live database and the reported backup; never delete canonical rows to +make a derived-index error disappear. diff --git a/hermes_state.py b/hermes_state.py index 9ca8fee3b8..8040c0f6b4 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -4875,12 +4875,6 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) self._read_open_failed_at = 0.0 self._wal_active = False self._write_count = 0 - # One-shot guard for the runtime FTS rebuild recovery on the write - # path. A corrupt FTS shadow table makes EVERY message write raise - # the malformed/corrupt error class via the sync triggers; we repair - # in place at most once per SessionDB instance so a genuinely - # unrecoverable database can't put writers into a rebuild loop. - self._fts_runtime_rebuild_attempted = False # One-shot guard for the usermerge-floor config write on the # incremental FTS merge cadence (see _merge_fts_incrementally). self._fts_usermerge_floor_applied = False @@ -5792,13 +5786,12 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) continue # Corrupt FTS shadow tables make every write raise the # malformed/corrupt error class through the FTS sync triggers - # while the canonical messages table is intact. Recover here, - # at the shared persistence boundary, so every caller gets the - # same guarantee. First try the cheap in-place repair. If that - # one-shot path is unavailable or corruption recurs, detach the - # derived indexes and retry against the canonical tables. - if self._try_runtime_fts_rebuild(exc): - continue + # while the canonical messages table is intact. Never run a + # full-message FTS5 rebuild from this live persistence path: + # on a multi-gigabyte state.db that can hold the writer lock + # for minutes. Atomically detach the derived indexes instead, + # then retry the canonical write. The existing stale-open and + # explicit repair paths retain rebuild ownership. if self._enter_fts_fail_open(exc): continue raise @@ -6023,70 +6016,6 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) pass return signalled - def _try_runtime_fts_rebuild(self, exc: sqlite3.DatabaseError) -> bool: - """One-shot in-place FTS rebuild after a corrupt-index write failure. - - Returns True when a rebuild was performed and the failed write should - be retried; False when the error isn't the FTS-corruption class, FTS - is disabled, or a rebuild was already attempted for this instance. - - Delegates to :meth:`rebuild_fts` (the FTS5 ``'rebuild'`` command — - index rewritten from the canonical messages table, zero message-row - mutation). Safe to call from ``_execute_write``'s except path: the - failed transaction was rolled back and ``self._lock`` released before - the exception propagated, and ``rebuild_fts`` re-acquires it. - E2E-verified: a corrupted ``messages_fts_data`` shadow table rejects - every append; after the in-place rebuild the same append succeeds and - search works again. - """ - if self._fts_runtime_rebuild_attempted: - return False - if not self._fts_enabled: - return False - if not self._is_fts_write_corruption_error(exc): - return False - # Set the one-shot flag before the foreign-holder check: even when - # the rebuild is skipped, the fail-open path that follows persists - # the FTS_STALE_KEY marker so the next process startup will retry - # via _recover_stale_fts (which has its own holder guard). Setting - # the flag here also avoids re-running the expensive psutil scan on - # every subsequent corrupted write through this instance. - self._fts_runtime_rebuild_attempted = True - foreign_holders = self._foreign_state_db_holders() - if foreign_holders: - logger.warning( - "Skipping automatic state.db FTS rebuild while foreign " - "processes hold the database or WAL sidecars (%s); detaching " - "FTS sync so canonical writes can continue.", - foreign_holders, - ) - return False - logger.warning( - "state.db write failed with an FTS-corruption error (%s) — " - "attempting one-shot in-place FTS rebuild; canonical message " - "rows are preserved.", exc, - ) - try: - rebuilt = self.rebuild_fts() - except Exception as rebuild_exc: - logger.error( - "In-place FTS rebuild failed (%s); the database needs the " - "full offline repair path (repair_state_db_schema).", - rebuild_exc, - ) - return False - if not rebuilt: - logger.error( - "In-place FTS rebuild made no progress; the database needs " - "the full offline repair path (repair_state_db_schema)." - ) - return False - logger.warning( - "state.db FTS indexes rebuilt in place (%d); retrying the failed write.", - rebuilt, - ) - return True - def _enter_fts_fail_open(self, exc: sqlite3.DatabaseError) -> bool: """Detach corrupt FTS indexes so canonical writes can continue. diff --git a/hermes_state_search.py b/hermes_state_search.py index 221e9747f8..40fddb70fd 100644 --- a/hermes_state_search.py +++ b/hermes_state_search.py @@ -1945,30 +1945,17 @@ class SessionSearchMixin: "trigram/LIKE", exc_info=True, ) except sqlite3.DatabaseError as exc: - # Same corruption class as the other FTS reads: rebuild - # in place once and retry; on refusal/failure fall back. - if self._try_runtime_fts_rebuild(exc): - try: - with self._read_ctx() as conn: - cjk_cursor = conn.execute( - cjk_sql, cjk_params - ) - matches = [ - dict(row) for row in cjk_cursor.fetchall() - ] - _trigram_succeeded = True - except sqlite3.DatabaseError: - logger.warning( - "CJK-bigram FTS search still failing after " - "in-place rebuild; falling back to " - "trigram/LIKE." - ) - else: - logger.warning( - "CJK-bigram FTS search hit a corruption error " - "(%s) and no in-place rebuild was possible; " - "falling back to trigram/LIKE.", exc, - ) + # A full-message rebuild is unbounded and holds the writer + # lock, so a live search never performs one. Detach the + # derived indexes and answer from canonical rows instead. + # Non-FTS corruption is not safe to reinterpret here. + if not self._enter_fts_fail_open(exc): + raise + logger.warning( + "CJK-bigram FTS search hit a corruption error (%s); " + "detached FTS and falling back to canonical LIKE.", + exc, + ) if ( not _trigram_succeeded @@ -2029,38 +2016,17 @@ class SessionSearchMixin: # Trigram query failed at runtime — fall through to LIKE. pass except sqlite3.DatabaseError as exc: - # Same corruption class the main FTS5 MATCH branch - # self-heals above: a corrupt trigram shadow table raises - # malformed / "fts5: corrupt structure record", which is a - # DatabaseError (parent of the OperationalError syntax arm - # caught first). Rebuild once outside the lock — the lock - # is released here so rebuild_fts() can re-acquire it — - # and retry the trigram query. If the rebuild is refused - # (already attempted / FTS disabled / different error - # class) or the retry fails again, fall through to the - # LIKE substring path, which reads only the canonical - # messages table, so CJK search stays available. - if self._try_runtime_fts_rebuild(exc): - try: - with self._read_ctx() as conn: - tri_cursor = conn.execute( - tri_sql, tri_params - ) - matches = [ - dict(row) for row in tri_cursor.fetchall() - ] - _trigram_succeeded = True - except sqlite3.DatabaseError: - logger.warning( - "Trigram FTS search still failing after " - "in-place rebuild; falling back to LIKE." - ) - else: - logger.warning( - "Trigram FTS search hit a corruption error (%s) " - "and no in-place rebuild was possible; falling " - "back to LIKE.", exc, - ) + # Preserve the same bounded recovery contract as the CJK + # and main FTS paths: detach derived indexes, then fall + # through to the canonical LIKE query. A non-FTS storage + # error remains fatal rather than being hidden as a miss. + if not self._enter_fts_fail_open(exc): + raise + logger.warning( + "Trigram FTS search hit a corruption error (%s); " + "detached FTS and falling back to canonical LIKE.", + exc, + ) if not _trigram_succeeded: # Short / mixed CJK query, trigram unavailable, or trigram # <3 CJK chars. Fall back to LIKE substring search. @@ -2124,17 +2090,23 @@ class SessionSearchMixin: except sqlite3.DatabaseError as exc: # A corrupt FTS index raises the malformed / "fts5: corrupt # structure record" class on the MATCH read, the same class the - # write path self-heals (#66296). OperationalError (query - # syntax) is a subclass caught above; this arm is the corruption - # parent. Rebuild the index in place once — the read context - # holds no writer lock, so rebuild_fts() can acquire it — and - # retry, so search self-heals for read-only sessions (cron/CLI - # history search) that never trigger a write to repair it first. - if not self._try_runtime_fts_rebuild(exc): + # write path handles (#66296). OperationalError (query syntax) + # is a subclass caught above; this arm is the corruption + # parent. Live search must remain bounded, so detach the + # derived indexes and answer from canonical message rows. The + # existing stale-open/repair paths retain rebuild ownership. + if not self._enter_fts_fail_open(exc): raise - with self._read_ctx() as conn: - cursor = conn.execute(sql, params) - matches = [dict(row) for row in cursor.fetchall()] + matches = self._search_messages_like_fallback( + query, + source_filter=source_filter, + exclude_sources=exclude_sources, + role_filter=role_filter, + limit=limit, + offset=offset, + sort=sort, + include_inactive=include_inactive, + ) # Deferred-rebuild supplement (schema v23): while the background # backfill is pending, the FTS indexes only cover rows outside the diff --git a/tests/hermes_cli/test_session_handoff.py b/tests/hermes_cli/test_session_handoff.py index 7d0b03bce8..fdbcee7ca2 100644 --- a/tests/hermes_cli/test_session_handoff.py +++ b/tests/hermes_cli/test_session_handoff.py @@ -13,6 +13,7 @@ flip pending → running, and finishes with ``complete_handoff`` or from __future__ import annotations import time +from contextlib import contextmanager import pytest @@ -107,6 +108,26 @@ class TestHandoffStateDB: assert db.get_handoff_state(sid)["state"] == "completed" assert db.list_pending_handoffs() == [] + def test_handoff_reads_use_read_context_during_reconnect(self, db, monkeypatch): + """Handoff polling must not borrow the reconnectable writer handle.""" + sid = "sess-read-path" + self._make_session(db, sid) + db.request_handoff(sid, "discord") + + entered = [] + original = db._read_ctx + + @contextmanager + def traced_read_ctx(): + entered.append(True) + with original() as conn: + yield conn + + monkeypatch.setattr(db, "_read_ctx", traced_read_ctx) + assert db.get_handoff_state(sid)["state"] == "pending" + assert [row["id"] for row in db.list_pending_handoffs()] == [sid] + assert len(entered) == 2 + class TestHandoffCommandRegistration: """Slash-command surface checks.""" diff --git a/tests/state/test_fts_runtime_rebuild.py b/tests/state/test_fts_runtime_rebuild.py index ed71afbc5e..8779186e72 100644 --- a/tests/state/test_fts_runtime_rebuild.py +++ b/tests/state/test_fts_runtime_rebuild.py @@ -1,4 +1,4 @@ -"""Runtime FTS-corruption self-heal on the SessionDB write path (#65637 class). +"""Bounded FTS-corruption recovery on live SessionDB paths. A corrupted FTS5 shadow table (``messages_fts_data``) makes every message write raise ``sqlite3.DatabaseError: database disk image is malformed`` @@ -7,10 +7,10 @@ intact. Before this fix the gateway swallowed the failure at debug level and the in-memory session advanced while disk silently fell behind — surfacing later as "Persisted transcript lagged live cached history" amnesia. -The fix: ``_execute_write`` first attempts a one-shot in-place FTS rebuild. -If corruption persists, it records a durable stale marker, detaches the FTS -sync triggers, and retries the canonical write. Search degrades to ``LIKE`` -until a later open atomically rebuilds the index and restores the triggers. +The fix records a durable stale marker, detaches the FTS sync triggers, and +retries the canonical write immediately. Live search degrades to canonical +``LIKE`` queries. The existing guarded stale-open or explicit repair path may +rebuild later, outside the failed live write/search operation. """ import json @@ -367,116 +367,124 @@ class TestRuntimeFtsRebuild: assert _meta_value(tmp_path / "state.db", FTS_STALE_KEY) is None assert _base_fts_triggers(tmp_path / "state.db") == set(_FTS_TRIGGERS) - def test_append_self_heals_after_fts_corruption(self, db, tmp_path): + def test_append_defers_rebuild_after_fts_corruption( + self, db, tmp_path, monkeypatch + ): if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") + db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "hello world") - _corrupt_fts(tmp_path / "state.db") + _corrupt_fts(db_path) + monkeypatch.setattr( + db, + "rebuild_fts", + lambda: pytest.fail("live write must not rebuild the full FTS index"), + ) - # Before the fix this raised DatabaseError and the row was lost. + # The canonical write survives without waiting for a full index scan. msg_id = db.append_message("s1", "user", "healed append") assert msg_id is not None - assert _message_contents(tmp_path / "state.db") == [ + assert _message_contents(db_path) == [ "hello world", "healed append", ] + assert db._fts_stale is True + assert _meta_value(db_path, FTS_STALE_KEY) == "1" + assert _base_fts_triggers(db_path) == set() - def test_search_works_after_self_heal(self, db, tmp_path): + def test_search_works_from_canonical_rows_after_fail_open(self, db, tmp_path): if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") + db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "before corruption") - _corrupt_fts(tmp_path / "state.db") + _corrupt_fts(db_path) db.append_message("s1", "user", "searchable needle text") - raw = sqlite3.connect(str(tmp_path / "state.db")) - hits = raw.execute( - "SELECT rowid FROM messages_fts WHERE messages_fts MATCH 'needle'" - ).fetchall() - raw.close() - assert len(hits) == 1 + results = db.search_messages("needle") + assert results + assert any("needle" in (row.get("snippet") or "") for row in results) + assert db._fts_stale is True - def test_search_messages_self_heals_after_fts_corruption(self, db, tmp_path): + def test_search_messages_defers_rebuild_after_fts_corruption( + self, db, tmp_path, monkeypatch + ): """A read-only session that only SEARCHES (no write after corruption) - must self-heal too. The MATCH read raises the corruption class - (DatabaseError / 'fts5: corrupt structure record'), NOT the - OperationalError that search_messages caught — so before this fix the - search crashed until a write or restart rebuilt the index. + must stay available without starting an unbounded index scan. """ if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") + db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "a searchable needle here") - _corrupt_fts(tmp_path / "state.db") - # Injected via a raw connection, so no write on THIS instance has - # consumed the one-shot rebuild yet. - assert db._fts_runtime_rebuild_attempted is False + _corrupt_fts(db_path) + monkeypatch.setattr( + db, + "rebuild_fts", + lambda: pytest.fail("live search must not rebuild the full FTS index"), + ) results = db.search_messages("needle") - assert db._fts_runtime_rebuild_attempted is True # the search rebuilt it - assert results # non-empty: the rebuilt index matched the query + assert db._fts_stale is True + assert _meta_value(db_path, FTS_STALE_KEY) == "1" + assert _base_fts_triggers(db_path) == set() + assert results assert any("needle" in (r.get("snippet") or "") for r in results) - def test_trigram_search_self_heals_after_fts_corruption(self, db, tmp_path): + def test_trigram_search_defers_rebuild_after_fts_corruption( + self, db, tmp_path, monkeypatch + ): """The CJK/trigram MATCH branch has the same read-corruption exposure - as the main FTS5 branch: it caught only OperationalError (query - syntax), so a corrupt trigram shadow table raised DatabaseError - straight out of search_messages. It must self-heal via the shared - one-shot rebuild and answer from the rebuilt trigram index. + as the main FTS5 branch and must fall back to canonical rows. """ if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") if not db._trigram_available: pytest.skip("trigram tokenizer unavailable in this build") + db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "关于大别山项目的进展报告") - _corrupt_trigram_fts(tmp_path / "state.db") - assert db._fts_runtime_rebuild_attempted is False + _corrupt_trigram_fts(db_path) + monkeypatch.setattr( + db, + "rebuild_fts", + lambda: pytest.fail("live search must not rebuild the full FTS index"), + ) # >=3 CJK chars per token → routed to the trigram branch. results = db.search_messages("大别山项目") - assert db._fts_runtime_rebuild_attempted is True # search rebuilt it + assert db._fts_stale is True + assert _meta_value(db_path, FTS_STALE_KEY) == "1" + assert _base_fts_triggers(db_path) == set() assert results - # The rebuilt trigram index answered (trigram snippets use >>> <<<), - # i.e. we did not silently degrade to the LIKE fallback. - assert any(">>>" in (r.get("snippet") or "") for r in results) + assert any("大别山项目" in (r.get("snippet") or "") for r in results) - - def test_second_corruption_fails_open_and_rebuilds_on_reopen( - self, db, tmp_path - ): + def test_corruption_fails_open_and_rebuilds_on_reopen(self, db, tmp_path): if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "seed") _corrupt_fts(db_path) - db.append_message("s1", "user", "first heal") # consumes the one shot - assert db._fts_runtime_rebuild_attempted is True - - # A second corruption must not strand the canonical transcript. The - # derived indexes are detached and marked stale instead of looping. - _corrupt_fts(db_path) - db.append_message("s1", "user", "second corruption") + db.append_message("s1", "user", "corruption survives") assert _message_contents(db_path) == [ "seed", - "first heal", - "second corruption", + "corruption survives", ] assert db._fts_stale is True assert _meta_value(db_path, FTS_STALE_KEY) == "1" assert _base_fts_triggers(db_path) == set() # Search remains available from canonical rows while FTS is stale. - results = db.search_messages("second corruption") + results = db.search_messages("corruption survives") assert results - assert any("second corruption" in row["snippet"] for row in results) + assert any("corruption survives" in row["snippet"] for row in results) # A later open atomically rebuilds all canonical rows before triggers # return, then clears the durable breadcrumb. @@ -486,30 +494,29 @@ class TestRuntimeFtsRebuild: assert reopened._fts_stale is False assert _meta_value(db_path, FTS_STALE_KEY) is None assert _base_fts_triggers(db_path) == set(_FTS_TRIGGERS) - results = reopened.search_messages("second corruption") + results = reopened.search_messages("corruption survives") assert results finally: reopened.close() - def test_failed_in_place_rebuild_fails_open(self, db, tmp_path, monkeypatch): + def test_non_fts_write_error_after_fail_open_raises_not_hangs( + self, db, tmp_path + ): if not db._fts_enabled: pytest.skip("FTS5 unavailable in this build") db_path = tmp_path / "state.db" db.create_session("s1", source="test") db.append_message("s1", "user", "seed") _corrupt_fts(db_path) - - def _failed_rebuild(): - raise sqlite3.DatabaseError("rebuild could not read corrupt FTS") - - monkeypatch.setattr(db, "rebuild_fts", _failed_rebuild) db.append_message("s1", "user", "canonical survives") - assert _message_contents(db_path)[-1] == "canonical survives" - assert _meta_value(db_path, FTS_STALE_KEY) == "1" - assert _base_fts_triggers(db_path) == set() + def _persistent_non_fts_error(conn): + raise sqlite3.DatabaseError("routine integrity check failed") - def test_foreign_holder_skips_runtime_rebuild_and_fails_open( + with pytest.raises(sqlite3.DatabaseError, match="routine integrity"): + db._execute_write(_persistent_non_fts_error) + + def test_live_write_does_not_scan_foreign_holders( self, db, tmp_path, monkeypatch ): if not db._fts_enabled: @@ -522,7 +529,7 @@ class TestRuntimeFtsRebuild: monkeypatch.setattr( db, "_foreign_state_db_holders", - lambda: [(4242, str(db_path) + "-wal")], + lambda: pytest.fail("live fail-open must not enter rebuild admission"), raising=False, )