From f50d80e8eb7c8d5e288f0f4d7abe8e3b799ac49a Mon Sep 17 00:00:00 2001 From: Connor Black Date: Mon, 15 Jun 2026 16:35:14 -0400 Subject: [PATCH] =?UTF-8?q?fix(state):=20detect=20silent=20WAL=E2=86=92DEL?= =?UTF-8?q?ETE=20fallback=20on=20macOS=20NFS=20/=20SMB=20(no=20false=20"wa?= =?UTF-8?q?l")?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apply_wal_with_fallback() only detected WAL-incompatible filesystems via a RAISED OperationalError matched against _WAL_INCOMPAT_MARKERS. But macOS NFS, SMB/CIFS, and overlay filesystems (e.g. AgentFS's NFS-backed mount) refuse the WAL switch WITHOUT raising: `PRAGMA journal_mode=WAL` returns the still-effective mode ('delete') and no exception. The code then ran `return "wal"` unconditionally, so it: 1. returned a false "wal" while the DB was actually in DELETE mode, and 2. never called _log_wal_fallback_once, so the operator got ZERO signal that concurrency had silently degraded (reader-blocks-writer). state.db and kanban.db share this path, so a session/kanban board DB on a network or overlay filesystem ran in DELETE with no diagnostic. Fix: read the row `PRAGMA journal_mode=WAL` returns and verify it is actually 'wal' instead of assuming success; on a silent no-op, emit the existing fallback WARNING and return the true mode. The raise-based path is unchanged. Reproduced on a real AgentFS NFS overlay (PRAGMA journal_mode=WAL returned ('delete',) with no OperationalError). Adds a regression test for the silent-no-op shape; the 16 existing WAL-fallback tests are unchanged. --- hermes_state.py | 26 +++++++++++--- tests/test_hermes_state_wal_fallback.py | 48 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index cee5d4fc671..c50c91062f0 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -616,10 +616,28 @@ def apply_wal_with_fallback( return actual try: - conn.execute("PRAGMA journal_mode=WAL") - _apply_macos_checkpoint_barrier(conn) - _enforce_macos_synchronous_full(conn) - return "wal" + # ``PRAGMA journal_mode=WAL`` is a query-that-sets: it RETURNS the + # resulting journal mode. Network filesystems that refuse WAL by + # *raising* SQLITE_PROTOCOL ("locking protocol") are handled in the + # except branch below. But macOS NFS — and SMB/CIFS, and the AgentFS + # NFS overlay — refuse the switch WITHOUT raising: the pragma simply + # returns the still-effective mode (e.g. ``delete``). Trust the + # returned row, not the mere absence of an exception; otherwise we + # report a false ``"wal"`` AND skip the fallback WARNING, leaving the + # DB silently in DELETE (reader-blocks-writer) with no signal. + row = conn.execute("PRAGMA journal_mode=WAL").fetchone() + mode = str(row[0]).strip().lower() if row and row[0] is not None else "" + if mode == "wal": + _apply_macos_checkpoint_barrier(conn) + _enforce_macos_synchronous_full(conn) + return "wal" + _log_wal_fallback_once( + db_label, + sqlite3.OperationalError( + f"journal_mode=WAL refused without raising (still {mode!r})" + ), + ) + return mode or "delete" except sqlite3.OperationalError as exc: msg = str(exc).lower() if not any(marker in msg for marker in _WAL_INCOMPAT_MARKERS): diff --git a/tests/test_hermes_state_wal_fallback.py b/tests/test_hermes_state_wal_fallback.py index 2fee37396d1..b82c6af0b6e 100644 --- a/tests/test_hermes_state_wal_fallback.py +++ b/tests/test_hermes_state_wal_fallback.py @@ -55,6 +55,27 @@ def _open_blocking(path, reason="locking protocol", **kwargs): return sqlite3.connect(str(path), factory=factory, **kwargs), attempts +def _make_silent_noop_factory(returned_mode: str = "delete"): + """Return a ``sqlite3.Connection`` subclass whose ``PRAGMA journal_mode=WAL`` + silently NO-OPs: it returns the still-effective journal mode (e.g. + ``delete``) WITHOUT raising — the way macOS NFS / SMB / the AgentFS NFS + overlay behave. NFS that *raises* SQLITE_PROTOCOL is covered by + :func:`_make_blocking_factory`; this is the other, quieter failure shape. + """ + + class _WalSilentNoOpConnection(sqlite3.Connection): + def execute(self, sql, *args, **kwargs): # type: ignore[override] + if "journal_mode=wal" in sql.lower().replace(" ", ""): + # Refuse the WAL switch but DON'T raise; report the mode that is + # actually still in force, exactly as the NFS client does. + return super().execute( + f"PRAGMA journal_mode={returned_mode}", *args, **kwargs + ) + return super().execute(sql, *args, **kwargs) + + return _WalSilentNoOpConnection + + @pytest.fixture(autouse=True) def _reset_last_init_error(): """Reset the module-global last-error before and after each test.""" @@ -94,6 +115,33 @@ class TestApplyWalWithFallback: + def test_falls_back_when_wal_silently_refused(self, tmp_path, caplog): + """macOS NFS / SMB / AgentFS-NFS can REFUSE the WAL switch WITHOUT + raising: ``PRAGMA journal_mode=WAL`` returns the still-effective mode + ('delete') and no exception. The marker-exception path never fires, so + the function must detect the silent no-op from the PRAGMA's RETURN + value — otherwise it returns a false 'wal' and skips the WARNING. + + Reproduced on a real AgentFS NFS overlay, where + ``PRAGMA journal_mode=WAL`` returned ('delete',) with no + OperationalError. + """ + factory = _make_silent_noop_factory("delete") + conn = sqlite3.connect( + str(tmp_path / "macnfs.db"), factory=factory, isolation_level=None + ) + with caplog.at_level("WARNING", logger="hermes_state"): + mode = apply_wal_with_fallback(conn, db_label="kanban.db") + + assert mode == "delete", "must report the true mode, not a false 'wal'" + assert ( + conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "delete" + ) + warnings = [r for r in caplog.records if r.levelname == "WARNING"] + assert len(warnings) == 1, "silent no-op must still emit exactly one WARNING" + assert "kanban.db" in warnings[0].getMessage() + conn.close() + def test_reraises_on_disk_io_error(self, tmp_path): """Transient EIO from ``PRAGMA journal_mode=WAL`` must NOT silently downgrade to DELETE.