fix(state): second-process maintenance on state.db refuses ANY foreign holder

`hermes doctor --fix`'s WAL checkpoint and `repair_state_db_schema`'s
preflight documented themselves as fail-OPEN: `live_writer_holds_db` only
refused on unknown/deleted/uninspectable holders and then trusted a
`BEGIN IMMEDIATE` probe, which is blind to a `journal_mode=DELETE` reader
(SHARED only) and cannot run on a malformed file — exactly the states repair
and checkpoint get invoked in. A repair in a second process then REINDEXed /
VACUUMed a file the gateway still held (#103339 item 2).

- `hermes_state_holders.live_writer_holds_db`: any foreign holder of the DB or
  a sidecar is a live holder; the probe is only an additional positive signal.
- doctor `--fix`: the checkpoint runs on `_exclusive_repair_db_guard`'s
  connection instead of a bare writable `sqlite3.connect`, so an opener
  arriving after the scan is refused, not joined; `_session_count` is a
  `mode=ro` reader.
- Normal SessionDB writers are untouched: gateway + dashboard in two processes
  both keep writing (a process-wide flock on the write path — PR #109270's
  shape — would break that).

Tests: the two-process repair race test releases the test process's own
header-probe fd (it is a genuine holder now); the mid-repair writer fixture
opens its connection after staging starts (a pre-existing holder is refused up
front, which is the point).

Refs #103339 #100896

hermes_cli/doctor_state.py

@@ -150,7 +150,8 @@ def _check_directory_structure(should_fix: bool, f: Finding) -> None:
 
 def _session_count(state_db_path: Path):
     import sqlite3
-    conn = sqlite3.connect(str(state_db_path))
+    # mode=ro: doctor is a reader; a writable open of a gateway-held WAL DB is the second-writer class (#103339).
+    conn = sqlite3.connect(f"file:{state_db_path}?mode=ro", uri=True)
     try:
         return conn.execute("SELECT COUNT(*) FROM sessions").fetchone()[0]
     finally:
@@ -256,24 +257,27 @@ def _state_db_wal(f: Finding, should_fix: bool, state_db_path: Path) -> None:
             check_warn(f"WAL file is large ({size // (1024*1024)} MB)", "(may indicate missed checkpoints)")
             if not should_fix:
                 return f.issues.append("Large WAL file — run 'hermes doctor --fix' to checkpoint")
-            # Checkpoint-lock premise (#40177): a bare connect runs WAL recovery and the checkpoint joins the
-            # live WAL — under a running gateway that second-writer handling corrupts state.db. Skip instead.
+            # Checkpoint-lock premise (#40177, #103339): a bare connect runs WAL recovery and the checkpoint
+            # joins the live WAL — under a running gateway that second-writer handling corrupts state.db.
+            # Holder scan first (any other process holding the DB, or an unknown, fails closed), then run the
+            # checkpoint on the exclusive repair guard so an opener arriving in between is refused, not joined.
             from hermes_state_holders import live_writer_holds_db
-            from hermes_state_repair import _connect_repair_durable
+            from hermes_state_repair import _connect_repair_durable, _exclusive_repair_db_guard
+            _SKIP = ("Large WAL file — cannot prove state.db is quiet (stop the profile's gateway first, then "
+                     "re-run 'hermes doctor --fix' to checkpoint)")
             if live_writer_holds_db(state_db_path, connect_repair_durable=_connect_repair_durable):
-                # Honest disjunction (gate C1): a True here means "held OR
-                # unprovable" — the DatabaseError lane fires when SQLite
-                # cannot open the file at all, with nobody holding it. Never
-                # assert a live writer as fact.
+                # Honest disjunction (gate C1): a True here means "held OR unprovable" — never assert a live
+                # writer as fact.
                 check_warn("WAL checkpoint skipped: cannot prove state.db is quiet",
-                           "(a live writer holds it, or it is unreadable — stop the profile's gateway "
+                           "(another process holds it, or it is unreadable — stop the profile's gateway "
                            "and re-run 'hermes doctor --fix')")
-                return f.issues.append("Large WAL file — cannot prove state.db is quiet (stop the profile's "
-                                       "gateway first, then re-run 'hermes doctor --fix' to checkpoint)")
-            import contextlib
-            import sqlite3
-            with contextlib.closing(sqlite3.connect(str(state_db_path))) as conn:
-                conn.execute("PRAGMA wal_checkpoint(PASSIVE)")
+                return f.issues.append(_SKIP)
+            with _exclusive_repair_db_guard(state_db_path) as (guard, guard_error):
+                if guard is None:
+                    check_warn("WAL checkpoint skipped: could not take exclusive ownership of state.db",
+                               f"({guard_error}; stop the profile's gateway and re-run 'hermes doctor --fix')")
+                    return f.issues.append(_SKIP)
+                guard.execute("PRAGMA wal_checkpoint(PASSIVE)")
             check_ok(f"WAL checkpoint performed ({size // 1024}K → {wal_size() // 1024}K)")
             f.fixed += 1
         elif size > 10 * 1024 * 1024:  # 10 MB

hermes_state_holders.py

@@ -320,15 +320,14 @@ def live_writer_holds_db(
     *,
     connect_repair_durable: Callable[..., sqlite3.Connection],
 ) -> bool:
-    """Return whether repair lacks proven exclusive ownership of ``db_path``."""
-    foreign_holders = foreign_state_db_holders(db_path)
-    if any(
-        pid < 0
-        or path.startswith("uninspectable holder:")
-        or path.startswith("uninspectable descriptor:")
-        or path.endswith(" (deleted)")
-        for pid, path in foreign_holders
-    ):
+    """Return whether repair lacks proven exclusive ownership of ``db_path``.
+
+    ANY foreign process holding the DB or a sidecar is a live holder (#103339): the lock probe below
+    cannot see a DELETE-mode reader (SHARED only) and cannot run at all on a malformed file, and those
+    are exactly the states repair/VACUUM/checkpoint get invoked in. The holder scan is the authority and
+    fails closed on its own failures (unknown/uninspectable sentinels); the probe only adds a positive
+    lock signal on top."""
+    if foreign_state_db_holders(db_path):
         return True
 
     probe = None
@@ -342,8 +341,7 @@ def live_writer_holds_db(
         lowered = str(exc).lower()
         return "locked" in lowered or "busy" in lowered
     except sqlite3.DatabaseError:
-        return False
-    except Exception:
+        # Malformed/unreadable with no holder on the scan: nobody else has it open, so repair may run.
         return False
     finally:
         if probe is not None:

hermes_state_repair.py

@@ -824,15 +824,12 @@ def _db_opens_cleanly(db_path: Path) -> Optional[str]:
 
 
 def _live_writer_holds_db(db_path: Path) -> bool:
-    """True when a connection outside this call still holds ``db_path`` open.
+    """True when another process (or a connection outside this call) still holds ``db_path``.
 
-    Asks SQLite for what a repair needs and a live holder cannot grant: ``locking_mode=EXCLUSIVE`` then
-    ``BEGIN IMMEDIATE`` — in WAL mode that needs exclusive WAL-index locks, so any other open connection fails
-    it with SQLITE_BUSY; neither statement parses the schema, so it works on malformed DBs. Fails **open**
-    (False) on anything but a positive busy/locked signal: refusing to repair a DB nobody holds would strand
-    the self-heal path. In ``journal_mode=DELETE`` a held reader takes only SHARED and this returns False;
-    repair is then serialised only by the cross-process repairer lock. Before probing, the foreign-holder scan
-    (``hermes_state_holders``) fails closed on deleted-WAL-generation, uninspectable, or unknown holders."""
+    The foreign-holder scan (``hermes_state_holders``) is the authority: any other process with the DB or a
+    WAL sidecar open, a deleted WAL generation, or an unknown/uninspectable holder fails CLOSED. The SQLite
+    probe (``locking_mode=EXCLUSIVE`` + ``BEGIN IMMEDIATE``) is only an additional positive signal — it cannot
+    see a ``journal_mode=DELETE`` reader and cannot run on a malformed file, which is why the scan comes first."""
     import hermes_state_holders as _state_holders
     return _state_holders.live_writer_holds_db(db_path, connect_repair_durable=_connect_repair_durable)
 

tests/hermes_cli/test_doctor_wal_checkpoint_guard.py

@@ -0,0 +1,49 @@
+"""#103339 item 2: ``hermes doctor --fix`` never checkpoints state.db through a bare writable ``sqlite3.connect``
+— the checkpoint runs on the exclusive repair guard, so an opener arriving after the holder scan is refused."""
+
+from __future__ import annotations
+
+import sqlite3
+from pathlib import Path
+
+import hermes_state_repair
+from hermes_cli.doctor_report import Finding
+from hermes_cli.doctor_state import _state_db_wal
+
+
+def test_doctor_checkpoint_runs_only_on_the_exclusive_repair_guard(tmp_path, monkeypatch):
+    db = tmp_path / "state.db"
+    setup = sqlite3.connect(str(db))
+    setup.execute("CREATE TABLE t(x)")
+    setup.execute("PRAGMA journal_mode=WAL")
+    setup.execute("INSERT INTO t VALUES (1)")
+    setup.commit()
+    setup.close()
+    wal = Path(f"{db}-wal")
+    with open(wal, "ab") as handle:
+        handle.truncate(51 * 1024 * 1024)
+
+    bare_connects: list[str] = []
+    real_connect = sqlite3.connect
+
+    def _spy(database, *args, **kwargs):
+        if not str(database).startswith("file:") or "mode=ro" not in str(database):
+            bare_connects.append(str(database))
+        return real_connect(database, *args, **kwargs)
+
+    monkeypatch.setattr(sqlite3, "connect", _spy)
+    guard_connects: list[Path] = []
+    real_durable = hermes_state_repair._connect_repair_durable
+
+    def _durable(path, **kwargs):
+        guard_connects.append(Path(path))
+        return real_durable(path, **kwargs)
+
+    monkeypatch.setattr(hermes_state_repair, "_connect_repair_durable", _durable)
+
+    finding = Finding()
+    _state_db_wal(finding, True, db)
+
+    assert finding.fixed == 1 and not finding.issues
+    # Every writable open went through the repair connector (probe + exclusive guard); none was a bare connect.
+    assert bare_connects and len(bare_connects) == len(guard_connects) >= 2

tests/hermes_state/test_state_db_malformed_repair.py

@@ -506,6 +506,17 @@ print(json.dumps(repair_state_db_schema({db!r})), flush=True)
 """
 
 
+def _release_header_probe_fds() -> None:
+    """Close this process's cached header-probe fds (no SQLite connection is live, so no lock is at risk)."""
+    import os
+
+    import hermes_state_dbfile
+    with hermes_state_dbfile._HEADER_PROBE_LOCK:
+        for fd, _dev, _ino in hermes_state_dbfile._HEADER_PROBE_FDS.values():
+            os.close(fd)
+        hermes_state_dbfile._HEADER_PROBE_FDS.clear()
+
+
 @pytest.mark.skipif(sys.platform == "win32", reason="POSIX flock test")
 def test_two_processes_repairing_at_once_perform_surgery_once(tmp_path):
     """Concurrent repairers serialise; the loser sees a healed DB and stops.
@@ -518,6 +529,9 @@ def test_two_processes_repairing_at_once_perform_surgery_once(tmp_path):
     db_path = tmp_path / "state.db"
     _build_healthy_db(db_path)
     _corrupt_duplicate_fts(db_path)
+    # The test process itself must not count as a holder: _build_healthy_db's SessionDB left the
+    # process-lifetime header-probe fd open, and a second process refuses ANY foreign holder (#103339).
+    _release_header_probe_fds()
 
     script = _REPAIR_SCRIPT.format(
         root=str(Path(hermes_state.__file__).parent), db=str(db_path)

tests/hermes_state/test_state_db_repair_non_destructive.py

@@ -81,16 +81,17 @@ def _writer_after_stage(
     """Try one real cross-process write after staging has begun.
 
     The repair process owns the SQLite exclusion for the complete
-    stage/strategy/promotion interval.  A writer is allowed to fail or to
-    wait until that interval ends and commit afterwards; what is forbidden is
-    a successful commit that promotion silently overwrites.
+    stage/strategy/promotion interval.  The writer arrives mid-repair (a
+    pre-existing foreign holder is refused up front, #103339) and is allowed to
+    fail or to wait until that interval ends and commit afterwards; what is
+    forbidden is a successful commit that promotion silently overwrites.
     """
+    ready.set()
+    if not start.wait(20):
+        result.put(("not-started", "repair did not reach staging"))
+        return
     conn = sqlite3.connect(db_path, timeout=0.75, isolation_level=None)
     try:
-        ready.set()
-        if not start.wait(20):
-            result.put(("not-started", "repair did not reach staging"))
-            return
         try:
             conn.execute(
                 "INSERT INTO messages (body) VALUES ('committed-after-stage')"

tests/hermes_state/test_state_db_second_process_maintenance.py

@@ -0,0 +1,58 @@
+"""#103339 item 2: structural maintenance from a second PROCESS must refuse while ANY other process holds
+state.db, even where SQLite's own lock probe is blind (``journal_mode=DELETE`` reader; malformed file)."""
+
+from __future__ import annotations
+
+import select
+import subprocess
+import sys
+import uuid
+from pathlib import Path
+
+import pytest
+
+from hermes_state import SessionDB
+from hermes_state_repair import repair_state_db_schema
+
+_HOLDER = """
+import sqlite3, sys
+conn = sqlite3.connect(sys.argv[1])
+conn.execute("SELECT count(*) FROM sessions").fetchall()
+print("ready", flush=True)
+sys.stdin.read(1)
+conn.close()
+"""
+
+
+@pytest.fixture
+def delete_mode_db(tmp_path, monkeypatch) -> Path:
+    import hermes_state_wal
+    monkeypatch.setattr(hermes_state_wal, "resolve_journal_mode", lambda: "delete")
+    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()
+    assert db.read_bytes()[18] == 1, "fixture must be a rollback-journal DB"
+    return db
+
+
+@pytest.mark.linux_only
+def test_repair_refuses_delete_mode_db_held_open_by_another_process(delete_mode_db):
+    """A held DELETE-mode reader takes only SHARED, so ``BEGIN IMMEDIATE`` succeeds and the lock probe sees
+    nothing; the holder scan must still refuse — REINDEX/VACUUM from a second process is the #103339 class."""
+    db = delete_mode_db
+    holder = subprocess.Popen([sys.executable, "-c", _HOLDER, str(db)], stdin=subprocess.PIPE,
+                              stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
+    try:
+        assert select.select([holder.stdout], [], [], 10)[0] and holder.stdout.readline().strip() == "ready"
+        with open(db, "r+b") as fh:  # schema page garbage under the holder: the shape repair is invoked on
+            fh.seek(100)
+            fh.write(b"\xff" * (4096 - 100))
+        report = repair_state_db_schema(db, backup=False)
+    finally:
+        holder.stdin.write("x")
+        holder.stdin.close()
+        holder.wait(timeout=10)
+    assert report["repaired"] is False
+    assert "stop the gateway" in (report["error"] or "").lower()