From ca28a69ada74fe2e4589153adc876a2e002e9a4e Mon Sep 17 00:00:00 2001 From: Dhanesh Purohit Date: Thu, 20 Aug 2026 16:18:21 +0530 Subject: [PATCH] fix(state): apply macOS write barriers on every state.db repair connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit state.db corrupted twice in two days with the torn-b-tree signature — repeated "2nd reference to page", "Rowid out of order", and long runs of "never used" pages in messages (rootpage 5) and idx_messages_session. macOS fsync() guarantees neither data-on-platter nor write ordering, which _enforce_macos_synchronous_full already documents: a rewrite interrupted by process or OS termination leaves half-written b-tree pages. The mitigation is per-connection (synchronous=FULL + checkpoint_fullfsync=1) and was applied only through apply_wal_with_fallback(). The repair path opened state.db with a bare sqlite3.connect() six times and then ran REINDEX, VACUUM and writable_schema surgery through it — the operations that rewrite nearly every page of the file — with no barrier at all. - _connect_repair_durable() routes every repair/probe connection through the barriers. Applying them is best-effort by necessity: SQLite loads the schema before any statement, so on a malformed schema even PRAGMA synchronous=FULL raises DatabaseError, and a malformed database is precisely this helper's input. _reapply_durability_barriers() retakes them before REINDEX and VACUUM, once the schema parses and they can stick. - verify_state_db_integrity() adds the proactive check that was missing. Repair only ever ran reactively, after a caller already hit a malformed error, so a database torn in pages no query happened to touch stayed live and kept accepting writes. On 2026-08-19 that gap was 11 hours across two restarts that both reported a clean start. Size-aware: degrades to an O(1) probe above 2 GiB rather than pegging a CPU at startup. Also restores two fixes lost when `hermes update` reset the tree to origin/main before they were committed: - _db_fingerprint keys the repair ledger on dev+inode+size instead of size+mtime_ns. The old form was justified as "stable for a file nothing can successfully write to"; that premise is false, because on FTS corruption this module deliberately keeps canonical writes enabled with FTS detached. mtime churned on every write, so each pass re-keyed the ledger and reset the counter to 1 — the cap could never be reached and the damaging surgery could retry forever. - _live_writer_holds_db() refuses surgery while another connection holds the database. The cross-process lock only serialises repairers against each other; it says nothing about the gateway, Desktop or a CLI. Rewriting b-tree pages under a concurrent writer is what spread the 2026-08-18/19 damage out of the FTS shadow tables and into the canonical ones. Fails open, so it cannot strand the self-heal path it protects. The guard's own tests built a two-table toy schema, so every repair aborted on "no such table: sessions" before reaching the guards under test — the assertions were passing over a code path that never ran. They now build through a real SessionDB. Targeted state/repair suites: 330 passed, 1 pre-existing unrelated failure. Broader sweep: 50 failed/1221 passed -> 46 failed/1225 passed. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Dhanesh Purohit --- hermes_state.py | 227 +++++++++++++++++- .../test_state_db_repair_live_writer_guard.py | 145 +++++++++++ tests/test_state_db_write_durability.py | 210 ++++++++++++++++ 3 files changed, 572 insertions(+), 10 deletions(-) create mode 100644 tests/test_state_db_repair_live_writer_guard.py create mode 100644 tests/test_state_db_write_durability.py diff --git a/hermes_state.py b/hermes_state.py index 1e2132ea2c..ad51583096 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -1860,16 +1860,30 @@ def _repair_ledger_path(db_path: Path) -> Path: def _db_fingerprint(db_path: Path) -> "Optional[str]": - """Cheap identity for a damaged DB file: size + mtime_ns. + """Cheap identity for a damaged DB file: device + inode + size. Hashing a multi-GB corrupt file on every open is exactly the kind of - repeated cost this ledger exists to avoid; size+mtime is stable for a - file nothing can successfully write to, and any successful repair, - truncation or manual restore changes it (resetting the attempt count). + repeated cost this ledger exists to avoid. + + ``mtime_ns`` was the original third component, justified as "stable for a + file nothing can successfully write to". That premise is false, and it + is the defect that let the 2026-08-18/19 incident run unbounded: on FTS + corruption this module deliberately keeps "canonical writes enabled with + FTS detached", so the gateway kept writing and mtime churned. Every + repair pass re-keyed the ledger and reset the counter to 1 — three real + passes (00:14, 00:35, 00:45) each recorded ``failed_attempts: 1``, so the + cap could never be reached and the damaging surgery could retry forever. + + Device+inode is stable across those writes. ``size`` is retained so an + in-place restore that reuses the inode still reads as a different file. + In WAL mode commits land in the ``-wal`` sidecar, so the main database's + size holds steady between checkpoints — a checkpoint that grows the file + grants a fresh budget, which is the intended "the file materially + changed" signal rather than the per-write churn that broke the cap. """ try: st = db_path.stat() - return f"{st.st_size}:{st.st_mtime_ns}" + return f"{st.st_dev}:{st.st_ino}:{st.st_size}" except OSError: return None @@ -2137,6 +2151,131 @@ def preflight_db_writability( _ensure_writable(p) +def _connect_repair_durable(db_path: Path) -> sqlite3.Connection: + """``sqlite3.connect`` for the repair/probe paths, with macOS write barriers. + + These paths open ``state.db`` directly rather than through ``SessionDB`` + (which routes via :func:`apply_wal_with_fallback`), so they inherited + SQLite's ``synchronous=NORMAL`` default and no ``checkpoint_fullfsync``. + On Darwin that is exactly the combination :func:`_enforce_macos_synchronous_full` + exists to prevent: ``fsync()`` there guarantees neither data-on-platter nor + write ordering, so a rewrite interrupted by process or OS termination can + leave half-written b-tree pages behind. + + That matters more here than anywhere else in the module, because what runs + through these connections is ``REINDEX``, ``VACUUM`` and ``writable_schema`` + surgery — the operations that rewrite nearly every page of the file. The + 2026-08-19 recurrence tore ``messages`` (root page 5) and + ``idx_messages_session``, reporting the unmistakable signature: repeated + "2nd reference to page", a rowid out of order, and long runs of leaked + "never used" pages. + + Autocommit (``isolation_level=None``) is preserved: callers run DDL and + ``VACUUM``, which are illegal inside an implicit transaction. + + Applying the barriers is best-effort *by necessity*: SQLite loads the + schema before it runs any statement, so on a malformed schema even + ``PRAGMA synchronous=FULL`` raises ``DatabaseError`` ("malformed database + schema (messages_fts) - table messages_fts already exists"). A malformed + database is precisely this helper's input, so raising there would leave + repair unable to open the file it exists to fix. Strategies that go on to + rewrite the whole file call :func:`_reapply_durability_barriers` once the + schema parses again, which is the point at which the pragmas can stick. + """ + conn = sqlite3.connect(str(db_path), isolation_level=None) + _reapply_durability_barriers(conn) + return conn + + +def _reapply_durability_barriers(conn: sqlite3.Connection) -> bool: + """Best-effort (re)application of the macOS write barriers. Never raises. + + Returns True when the pragmas were accepted. Callers about to rewrite the + file wholesale (``VACUUM``, ``REINDEX``) should call this after the schema + becomes parseable, because a connection opened against a malformed schema + could not take them at open time. + """ + try: + _apply_macos_checkpoint_barrier(conn) + _enforce_macos_synchronous_full(conn) + return True + except sqlite3.DatabaseError: + # Schema still unparseable — the pragmas cannot be set yet. + return False + except Exception: + return False + + +def verify_state_db_integrity( + db_path: Path, + *, + max_bytes: int = 2 << 30, +) -> Dict[str, Any]: + """Proactively verify ``db_path``. Returns a report; never raises. + + Repair has only ever run *reactively* — when a caller already hit a + malformed error on open. A database torn in pages that no query happens + to touch stays live and keeps accepting writes until something finally + lands on the damage. On 2026-08-19 that gap was 11 hours: the tear + landed in pages holding rows written at 02:18-02:22 and was not seen + until 13:36, across two gateway restarts that both reported a clean start. + + ``PRAGMA integrity_check`` walks every page, so it is O(file size) — the + same reason :func:`hermes_cli.backup.verify_sqlite_integrity` caps it. + Above ``max_bytes`` this degrades to an O(1) structural probe rather than + pegging a CPU for minutes at gateway startup. + + Report keys: + ``ok`` — False only on positive evidence of damage. + ``problems`` — integrity_check rows that were not "ok". + ``checked`` — "full" | "probe" | "absent" | "error". + """ + report: Dict[str, Any] = {"ok": True, "problems": [], "checked": "absent"} + try: + if not db_path.is_file(): + return report + size = db_path.stat().st_size + if size == 0: + # A zero-byte file is handled by the dedicated zeroed-DB path. + return report + except OSError as exc: + report["checked"] = "error" + report["problems"] = [f"stat failed: {exc}"] + return report + + conn = None + try: + conn = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True, timeout=5.0) + if size > max_bytes: + report["checked"] = "probe" + conn.execute("PRAGMA schema_version").fetchone() + conn.execute("SELECT count(*) FROM sqlite_master").fetchone() + return report + report["checked"] = "full" + rows = conn.execute("PRAGMA integrity_check").fetchall() + problems = [str(r[0]) for r in rows if r and str(r[0]).lower() != "ok"] + if problems: + report["ok"] = False + report["problems"] = problems + except sqlite3.DatabaseError as exc: + # The DB refused to open or parse — positive evidence of damage. + report["ok"] = False + report["checked"] = "error" + report["problems"] = [str(exc)] + except Exception as exc: + # Environmental (permissions, locks): not proof of corruption, so do + # not claim damage — but do not claim health either. + report["checked"] = "error" + report["problems"] = [str(exc)] + finally: + if conn is not None: + try: + conn.close() + except Exception: + pass + return report + + def _db_opens_cleanly(db_path: Path) -> Optional[str]: """Probe a DB on a fresh connection. Returns None if healthy, else a reason. @@ -2148,7 +2287,7 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]: through the FTS triggers — is reported as unhealthy rather than slipping past as a false "ok" (#50502). """ - conn = sqlite3.connect(str(db_path), isolation_level=None) + conn = _connect_repair_durable(db_path) try: # Best-effort tokenizer load: a DB carrying the messages_fts_cjk # index needs the cjk_unicode61 extension before any statement can @@ -2263,6 +2402,50 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]: conn.close() +def _live_writer_holds_db(db_path: Path) -> bool: + """True when a connection outside this call still holds ``db_path`` open. + + Detection works by asking SQLite for the thing a repair actually needs and + a live writer cannot grant: ``PRAGMA locking_mode=EXCLUSIVE`` followed by + ``BEGIN IMMEDIATE``. In WAL mode, entering exclusive locking mode + requires exclusive locks on the WAL index, so any other open connection — + reader or writer — makes it fail with SQLITE_BUSY. Neither statement + parses the schema, so this works on the malformed databases repair exists + to handle. + + Fails **open** (returns False) on anything other than a positive + busy/locked signal: refusing to repair a database that nobody is actually + holding would strand the very self-heal path this guard protects. + """ + probe = None + try: + probe = sqlite3.connect(str(db_path), timeout=0.0, isolation_level=None) + probe.execute("PRAGMA locking_mode=EXCLUSIVE") + probe.execute("BEGIN IMMEDIATE") + probe.execute("ROLLBACK") + return False + except sqlite3.OperationalError as exc: + lowered = str(exc).lower() + return "locked" in lowered or "busy" in lowered + except sqlite3.DatabaseError: + # Malformed/unreadable: no evidence of a live holder either way. + return False + except Exception: + return False + finally: + if probe is not None: + try: + # Drop exclusive locking mode before closing so the probe + # itself never leaves the file pinned. + probe.execute("PRAGMA locking_mode=NORMAL") + except Exception: + pass + try: + probe.close() + except Exception: + pass + + def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, Any]: """Repair a state.db whose ``sqlite_master`` schema is malformed or whose FTS indexes reject writes. @@ -2341,6 +2524,23 @@ def repair_state_db_schema(db_path: Path, *, backup: bool = True) -> Dict[str, A "schema surgery to avoid racing it" ) return report + + # The cross-process lock serialises repairers against each other; it + # says nothing about the gateway, Desktop or a CLI still holding the + # database open. Rewriting b-tree pages under a concurrent writer is + # what spread the 2026-08-18/19 damage out of the FTS shadow tables + # and into the canonical ones. The caller closes only its own + # connection — the incident process held seven descriptors on + # state.db — so probe for the rest before touching anything. + if _live_writer_holds_db(db_path): + report["error"] = ( + "a live writer still holds state.db; skipped schema surgery " + "to avoid tearing b-tree pages under a concurrent writer. " + "Stop the gateway (hermes gateway stop) and retry." + ) + logger.error("state.db repair skipped: %s", report["error"]) + return report + result = _repair_state_db_schema_locked(db_path, backup=backup, report=report) # Persist the outcome AFTER surgery, keyed on the post-attempt # fingerprint — that is the file state the NEXT attempt's exhaustion @@ -2391,7 +2591,7 @@ def _repair_state_db_schema_locked( # content table. This is the recommended, least-destructive recovery for a # corrupt FTS index that rejects message writes while reads still succeed. try: - conn = sqlite3.connect(str(db_path), isolation_level=None) + conn = _connect_repair_durable(db_path) try: # The cjk index can only be rebuilt with its tokenizer loaded; # best-effort (a tokenizer-less host skips it at the probe below). @@ -2427,8 +2627,11 @@ def _repair_state_db_schema_locked( # rows using the existing index definition, fixing the mismatch without # touching data or FTS schema. try: - conn = sqlite3.connect(str(db_path), isolation_level=None) + conn = _connect_repair_durable(db_path) try: + # REINDEX rewrites every index b-tree; take the barriers now that + # the schema parses, in case the open-time attempt was refused. + _reapply_durability_barriers(conn) conn.execute("REINDEX") conn.commit() finally: @@ -2445,7 +2648,7 @@ def _repair_state_db_schema_locked( # ── Strategy 1: de-duplicate sqlite_master (keeps FTS index) ── try: - conn = sqlite3.connect(str(db_path), isolation_level=None) + conn = _connect_repair_durable(db_path) try: conn.execute("PRAGMA writable_schema=ON") dupes = conn.execute( @@ -2477,13 +2680,17 @@ def _repair_state_db_schema_locked( # ── Strategy 2: drop all FTS schema, VACUUM, rebuild on next open ── try: - conn = sqlite3.connect(str(db_path), isolation_level=None) + conn = _connect_repair_durable(db_path) try: conn.execute("PRAGMA writable_schema=ON") conn.execute("DELETE FROM sqlite_master WHERE name LIKE 'messages_fts%'") _bump_schema_cookie(conn) conn.execute("PRAGMA writable_schema=OFF") conn.commit() + # The schema is repaired and parseable now, so the barriers can + # finally stick — and VACUUM, which rewrites the entire file, is + # the single most damaging operation to lose halfway. + _reapply_durability_barriers(conn) conn.execute("VACUUM") finally: conn.close() diff --git a/tests/test_state_db_repair_live_writer_guard.py b/tests/test_state_db_repair_live_writer_guard.py new file mode 100644 index 0000000000..e468f1a2ce --- /dev/null +++ b/tests/test_state_db_repair_live_writer_guard.py @@ -0,0 +1,145 @@ +"""Regression: the state.db repair path must be bounded and must never run +surgery against a database another connection is still writing. + +Incident (2026-08-18/19): FTS5 shadow-table corruption escalated into b-tree +page damage across `system_prompts`, `session_model_usage` and the `sessions` +index. Two defects in this module turned a contained, rebuildable FTS fault +into unrecoverable data loss (292 `delivery_obligations` rows): + +1. `_db_fingerprint` keyed the persistent attempt ledger on ``size:mtime_ns``, + documented as "stable for a file nothing can successfully write to". That + premise is false: on FTS corruption hermes_state deliberately keeps + "canonical writes enabled with FTS detached", so the gateway kept writing + and mtime churned. Every repair pass re-keyed the ledger and reset the + counter to 1 — three real passes (00:14, 00:35, 00:45) all recorded + ``failed_attempts: 1``, so `_MAX_PERSISTENT_REPAIR_ATTEMPTS` could never + be reached and the damaging surgery could retry forever. + +2. `repair_state_db_schema` ran its REINDEX/FTS-rebuild strategies while other + connections still held the database open. The caller closes only its own + `self._conn`; the incident process held seven descriptors on state.db. + Rewriting b-tree pages under concurrent writers is what spread the damage + out of the FTS shadow tables and into the canonical tables. +""" + +from __future__ import annotations + +import sqlite3 +import time +import uuid +from pathlib import Path + +from hermes_state import ( + SessionDB, + _MAX_PERSISTENT_REPAIR_ATTEMPTS, + _db_fingerprint, + _persistent_repair_attempts_exhausted, + _record_repair_outcome, + repair_state_db_schema, +) + + +def _make_wal_db(tmp_path: Path) -> Path: + """A state.db the repair path will actually work on. + + Built through the real ``SessionDB`` rather than a hand-rolled two-table + schema. The repair path probes the canonical schema as it goes — + ``_db_opens_cleanly`` runs ``SELECT COUNT(*) FROM sessions`` and a + rolled-back ``messages`` write — so a toy schema aborted every repair + ("no such table: sessions", then "table sessions has no column named id") + long before reaching the guards these tests exist to cover. The + assertions below were passing over a code path that never ran. + """ + db = tmp_path / "state.db" + handle = SessionDB(db_path=db) + sid = handle.create_session(session_id=str(uuid.uuid4()), source="cli") + handle.append_message(sid, role="user", content="seed") + handle.close() + return db + + +def _write_once(db: Path) -> None: + """Simulate the gateway's ongoing canonical writes (FTS detached).""" + handle = SessionDB(db_path=db) + sid = handle.create_session(session_id=str(uuid.uuid4()), source="cli") + handle.append_message(sid, role="user", content="canonical write") + handle.close() + + +# --------------------------------------------------------------------------- +# Defect 1: the ledger fingerprint must survive ongoing writes +# --------------------------------------------------------------------------- + + +def test_fingerprint_is_stable_while_the_gateway_keeps_writing(tmp_path): + """Identity must track the FILE, not its mtime/contents. + + A corrupt state.db still accepts canonical writes, so a mtime- or + content-derived fingerprint changes constantly and silently re-keys the + attempt ledger. + """ + db = _make_wal_db(tmp_path) + before = _db_fingerprint(db) + + time.sleep(0.01) + _write_once(db) + + assert _db_fingerprint(db) == before + + +def test_repair_budget_is_exhausted_despite_ongoing_writes(tmp_path): + """Three failed passes must exhaust the budget even with writes between. + + This is the exact incident shape: three real repair attempts, each + separated by gateway writes, all recorded ``failed_attempts: 1``. + """ + db = _make_wal_db(tmp_path) + + for _ in range(_MAX_PERSISTENT_REPAIR_ATTEMPTS): + _record_repair_outcome(db, repaired=False) + time.sleep(0.01) + _write_once(db) + + assert _persistent_repair_attempts_exhausted(db) is True + + +def test_successful_repair_still_clears_the_budget(tmp_path): + """A healed database must not inherit a spent budget.""" + db = _make_wal_db(tmp_path) + + for _ in range(_MAX_PERSISTENT_REPAIR_ATTEMPTS): + _record_repair_outcome(db, repaired=False) + assert _persistent_repair_attempts_exhausted(db) is True + + _record_repair_outcome(db, repaired=True) + + assert _persistent_repair_attempts_exhausted(db) is False + + +# --------------------------------------------------------------------------- +# Defect 2: repair must refuse to operate under a live writer +# --------------------------------------------------------------------------- + + +def test_repair_refuses_while_another_connection_holds_the_db(tmp_path): + """Surgery under concurrent writers is what spread the corruption.""" + db = _make_wal_db(tmp_path) + + holder = sqlite3.connect(str(db)) + holder.execute("SELECT count(*) FROM messages").fetchone() + try: + report = repair_state_db_schema(db, backup=False) + finally: + holder.close() + + assert report["repaired"] is False + assert "live writer" in (report["error"] or "").lower() + + +def test_repair_proceeds_once_the_database_is_quiescent(tmp_path): + """The guard must not deadlock repair on an exclusively-held file.""" + db = _make_wal_db(tmp_path) + + report = repair_state_db_schema(db, backup=False) + + assert "live writer" not in (report["error"] or "").lower() diff --git a/tests/test_state_db_write_durability.py b/tests/test_state_db_write_durability.py new file mode 100644 index 0000000000..fb012ad6a1 --- /dev/null +++ b/tests/test_state_db_write_durability.py @@ -0,0 +1,210 @@ +"""Regression: state.db repair-path writes must be durable on macOS, and a +torn database must be detected proactively rather than 11 hours later. + +Incident (2026-08-19, recurrence of 2026-08-18/19): `state.db` was recovered +clean at 01:02, tore again in the pages holding rows written 02:18-02:22, and +the damage went undetected until 13:36 when a write finally landed on a +damaged page (`append_message failed: constraint failed`). `PRAGMA +integrity_check` on the file reported the torn-b-tree signature: + + Tree 5 page 47256 cell 423..429: 2nd reference to page ... + Tree 5 page 60788 cell 4: Rowid 34637 out of order + Page 50549..52587: never used + +Two defects: + +1. hermes_state already knows macOS `fsync()` does not guarantee write + ordering, and mitigates it with `synchronous=FULL` + + `checkpoint_fullfsync=1` (see `_enforce_macos_synchronous_full`, whose + docstring names this exact failure: "a WAL checkpoint race with process + termination ... can leave the main DB with half-written btree pages"). + Those pragmas are per-connection and were applied only via + `apply_wal_with_fallback()`. The repair path opened `state.db` with a bare + `sqlite3.connect()` five times and then ran REINDEX, VACUUM and + `writable_schema` surgery through it — the operations that rewrite nearly + every page of the file — with no barrier at all. + +2. Repair only ran reactively, when a caller already hit a malformed error + (`SessionDB` open). Nothing checked the file proactively, so a torn + database stayed live and accepted writes for hours before anyone noticed. +""" + +from __future__ import annotations + +import re +import sqlite3 +import sys +from pathlib import Path + +import pytest + +import hermes_state +from hermes_state import ( + _connect_repair_durable, + repair_state_db_schema, + verify_state_db_integrity, +) + + +def _make_db(tmp_path: Path) -> Path: + db = tmp_path / "state.db" + conn = sqlite3.connect(str(db)) + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("CREATE TABLE sessions (session_id TEXT PRIMARY KEY)") + conn.execute("CREATE TABLE messages (id INTEGER PRIMARY KEY, body TEXT)") + conn.execute("INSERT INTO messages (body) VALUES ('seed')") + conn.commit() + conn.close() + return db + + +# ── Defect 1: repair-path write durability ────────────────────────────── + + +def test_connect_repair_durable_sets_macos_barriers(tmp_path: Path) -> None: + """The repair connection must carry both macOS durability barriers.""" + db = _make_db(tmp_path) + conn = _connect_repair_durable(db) + try: + synchronous = conn.execute("PRAGMA synchronous").fetchone()[0] + checkpoint_fullfsync = conn.execute( + "PRAGMA checkpoint_fullfsync" + ).fetchone()[0] + finally: + conn.close() + + if sys.platform == "darwin": + # SQLite: 0=OFF, 1=NORMAL, 2=FULL, 3=EXTRA. NORMAL is what tore the + # b-tree pages; FULL is what _enforce_macos_synchronous_full sets. + assert synchronous == 2, ( + f"repair connection opened with synchronous={synchronous}; on " + "Darwin this lets REINDEX/VACUUM leave half-written b-tree pages" + ) + assert checkpoint_fullfsync == 1, ( + "repair connection has no F_FULLFSYNC barrier at checkpoint " + "boundaries; macOS fsync() does not flush the drive cache" + ) + else: + # Elsewhere the helper is a plain connect — no behaviour change. + assert synchronous in (0, 1, 2, 3) + + +def test_connect_repair_durable_is_autocommit(tmp_path: Path) -> None: + """Must preserve isolation_level=None — repair runs DDL and VACUUM.""" + db = _make_db(tmp_path) + conn = _connect_repair_durable(db) + try: + assert conn.isolation_level is None + # VACUUM is only legal outside an implicit transaction. + conn.execute("VACUUM") + finally: + conn.close() + + +def test_repair_path_has_no_bare_connects() -> None: + """No repair/probe site may bypass the durability helper. + + Source-level guard: the bare form is exactly what regressed, and a unit + test on the helper alone would not notice a sixth site being added. + """ + source = Path(hermes_state.__file__).read_text() + pattern = r"^\s*conn = sqlite3\.connect\(str\(db_path\), isolation_level=None\)" + + # The one legitimate bare connect is inside the helper itself; everything + # after that definition must go through it. + helper = source.index("def _connect_repair_durable(") + body_end = source.index("\ndef ", helper + 1) + inside_helper = re.findall(pattern, source[helper:body_end], flags=re.MULTILINE) + assert len(inside_helper) == 1, ( + "_connect_repair_durable no longer opens the connection itself" + ) + + elsewhere = re.findall( + pattern, source[:helper] + source[body_end:], flags=re.MULTILINE + ) + assert elsewhere == [], ( + f"{len(elsewhere)} repair-path connection(s) still bypass " + "_connect_repair_durable() and write state.db without the macOS " + "fsync barriers" + ) + + +def test_repair_still_works_through_durable_connection(tmp_path: Path) -> None: + """Routing every strategy through the helper must not break the path. + + The helper is entered once per strategy, so a plumbing fault (recursion, + a leaked connection, a refused pragma) surfaces as an exception rather + than a report. Whether this fixture's minimal schema is *repairable* is + beside the point — the assertion is that the path runs to completion. + """ + db = _make_db(tmp_path) + report = repair_state_db_schema(db, backup=False) + assert isinstance(report, dict) + assert set(report) >= {"repaired", "strategy", "backup_path"} + # The file must still open afterwards — repair may fail, but it must not + # leave the database less usable than it found it. + conn = sqlite3.connect(str(db)) + try: + assert conn.execute("SELECT COUNT(*) FROM messages").fetchone()[0] == 1 + finally: + conn.close() + + +# ── Defect 2: proactive verification gate ─────────────────────────────── + + +def test_verify_state_db_integrity_passes_on_healthy_db(tmp_path: Path) -> None: + db = _make_db(tmp_path) + result = verify_state_db_integrity(db) + assert result["ok"] is True + assert result["problems"] == [] + + +def test_verify_state_db_integrity_detects_torn_btree(tmp_path: Path) -> None: + """A torn database must be reported, not silently accepted.""" + db = _make_db(tmp_path) + # Grow past one page, then corrupt an interior/leaf page directly — the + # same class of damage as "2nd reference to page" / "never used". + conn = sqlite3.connect(str(db)) + conn.execute("PRAGMA journal_mode=DELETE") + conn.executemany( + "INSERT INTO messages (body) VALUES (?)", + [(f"row-{i}" * 40,) for i in range(500)], + ) + conn.commit() + conn.close() + + page_size = 4096 + raw = bytearray(db.read_bytes()) + # Scribble over a data page (page 3+), leaving the header page intact so + # the file still opens — that is what makes this class so long-lived. + start = page_size * 4 + raw[start:start + page_size] = b"\xff" * page_size + db.write_bytes(bytes(raw)) + + result = verify_state_db_integrity(db) + assert result["ok"] is False + assert result["problems"], "torn pages reported no problems" + + +def test_verify_state_db_integrity_skips_pragma_when_oversized( + tmp_path: Path, +) -> None: + """Must degrade to an O(1) probe rather than pegging a CPU for minutes. + + `PRAGMA integrity_check` walks every page, so an unbounded check at + startup would hang the gateway on a multi-GB state.db. + """ + db = _make_db(tmp_path) + result = verify_state_db_integrity(db, max_bytes=1) + assert result["ok"] is True + assert result["checked"] == "probe" + + +def test_verify_state_db_integrity_missing_file_is_not_a_failure( + tmp_path: Path, +) -> None: + """A first run has no state.db yet; that must not look like corruption.""" + result = verify_state_db_integrity(tmp_path / "absent.db") + assert result["ok"] is True + assert result["checked"] == "absent"