From bb2c961e9a6ddbab3696e198c56c72ac52ab7edf Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:01:10 +0530 Subject: [PATCH] fix(state): VACUUM is gated by the same quarantine rule as the checkpoints Follow-up to #106315. vacuum() ran PRAGMA wal_checkpoint + VACUUM + wal_checkpoint(TRUNCATE) on self._conn with no quarantine check; the only guard it inherited (optimize_fts raising DeletedWalGenerationError) was swallowed by its own try/except and the rewrite proceeded on the split-brain handle. Mutation on main: vacuum() returned 2 and rewrote pages after the write stop. --- hermes_state_maintenance.py | 9 ++++++++- tests/hermes_state/test_deleted_wal_checkpoint_guard.py | 6 ++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/hermes_state_maintenance.py b/hermes_state_maintenance.py index 90735feaf4..de2dd1fa27 100644 --- a/hermes_state_maintenance.py +++ b/hermes_state_maintenance.py @@ -336,7 +336,14 @@ class SessionMaintenanceMixin: """VACUUM to reclaim space after large deletes (SQLite never shrinks on its own). Takes an exclusive lock — callers must ensure no other writers are active. FTS5 segments are merged first (:meth:`optimize_fts`) so their pages are reclaimed too; - returns the number of FTS indexes optimized (0 on merge failure / no FTS).""" + returns the number of FTS indexes optimized (0 on merge failure / no FTS). A quarantined + handle (corrupt image, replaced file, lost WAL generation) never checkpoints or rewrites + pages: the halt raised by ``optimize_fts`` would otherwise be swallowed below and the + VACUUM would proceed on the split-brain handle (#105670).""" + quarantine_reason = self._quarantine_reason() + if quarantine_reason is not None: + logger.warning("Skipping VACUUM for %s: this handle observed %s.", self.db_path, quarantine_reason) + return 0 optimized = 0 try: optimized = self.optimize_fts() # manages its own lock diff --git a/tests/hermes_state/test_deleted_wal_checkpoint_guard.py b/tests/hermes_state/test_deleted_wal_checkpoint_guard.py index e9d510390f..be93c2b835 100644 --- a/tests/hermes_state/test_deleted_wal_checkpoint_guard.py +++ b/tests/hermes_state/test_deleted_wal_checkpoint_guard.py @@ -62,7 +62,7 @@ def _unlink_sidecars(db_path: Path) -> None: @pytest.mark.linux_only # deleted-WAL write halt uses Linux unlink semantics def test_close_after_halt_runs_no_checkpoint(tmp_path, force_wal): - """A writer halted by DeletedWalGenerationError must not checkpoint (periodic or close) nor run FTS repair.""" + """A writer halted by DeletedWalGenerationError must not checkpoint (periodic, VACUUM or close) nor run FTS repair.""" path = tmp_path / "state.db" db = _make_db(path, "s", "before") _require_wal(db) @@ -72,11 +72,13 @@ def test_close_after_halt_runs_no_checkpoint(tmp_path, force_wal): db.append_message("s", role="user", content="after-unlink") assert db._db_wal_generation_lost is True - # Neither the periodic checkpoint, the stale-FTS retry, nor close() may touch the file now. + # Neither the periodic checkpoint, the stale-FTS retry, VACUUM, nor close() may touch the file now. with patch.object(db._conn, "execute", wraps=db._conn.execute) as mock_execute: db._try_wal_checkpoint() db._fts_stale = True assert db.retry_deferred_fts_recovery() is False + assert db.vacuum() == 0 + assert not [c for c in mock_execute.call_args_list if "vacuum" in str(c).lower()], "VACUUM ran on a quarantined handle" db.close() # No checkpoint call should have been made. checkpoint_calls = [