hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(db): honour require_wal on the probe-unknown path; touch nothing at all

Browse Source 
Follow-up to the probe-unknown guard:

- `require_wal=True` now raises WalUnsupportedError when the on-disk mode
  cannot be read instead of reporting an unverified "wal" (the function's
  contract is "the mode actually set").
- Drop `_apply_wal_companions` from the branch: journal_size_limit /
  synchronous pragmas were being applied to a file whose mode is unknown,
  contradicting the "touch nothing" rule the branch exists for.
- Cut the retracted #104596 mechanism narrative from comments, log text and
  docstrings; the guard is hardening (same rule the DELETE branch already
  applied), not a root-cause fix.
- Tests: one binding test per file (no pragma of any kind reaches the
  connection; require_wal raises); drop the log-dedupe test (`_log_once`
  is already covered). test_captures_cause_on_failed_init's double now
  fails only set-pragmas, matching the read-only-mount it simulates.
This commit is contained in:
kshitijk4poor
2026-09-09 17:33:52 +05:30
committed by kshitij
parent d0653ab090
commit a7f2a593d1
4 changed files with 40 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
hermes_state_wal.py
View File

@@ -33,8 +33,7 @@ _WAL_SIZE_LIMIT_BYTES = 64 * 1024 * 1024 # 64 MiB
_wal_fallback_warned_paths: set[str] = set()
_wal_fallback_warned_lock = threading.Lock()
# Dedup WARNING for the #104596 probe-unknown guard (WAL path touching nothing
# while ownership is not provably exclusive).
# Dedup for the probe-unknown WARNING (on-disk journal mode unreadable, nothing touched).
_wal_probe_unknown_paths: set[str] = set()
_wal_probe_unknown_lock = threading.Lock()
_wal_reset_bug_warned_paths: set[str] = set()
@@ -165,7 +164,8 @@ def resolve_journal_mode() -> str:
class WalUnsupportedError(sqlite3.OperationalError):
"""Raised by :func:`apply_wal_with_fallback` under ``require_wal=True`` when
the filesystem cannot provide WAL (SQLITE_PROTOCOL raised, or macOS-NFS silent
refusal). Subclasses ``OperationalError`` so DB-init handlers still catch it."""
refusal) or the on-disk mode cannot be verified (probe blocked by a concurrent
opener). Subclasses ``OperationalError`` so DB-init handlers still catch it."""
def _verify_configured_delete(actual: str) -> str:
@@ -178,8 +178,9 @@ def _verify_configured_delete(actual: str) -> str:
def apply_wal_with_fallback(conn: sqlite3.Connection, *, db_label: str = "state.db", require_wal: bool = False) -> str:
"""Set ``journal_mode=WAL`` on ``conn``, falling back to DELETE on failure.
Returns the mode actually set. Shared by :class:`SessionDB` and ``hermes_cli.kanban_db_connect.connect``.
WAL-incompatible filesystems either raise ``OperationalError`` ("locking protocol" / "disk I/O error") or —
Returns the mode actually set — or, when the read-only mode probe is blocked by a concurrent opener, ``"wal"``
as the assumed mode with nothing touched (``require_wal=True`` raises instead). Shared by :class:`SessionDB`
and ``hermes_cli.kanban_db_connect.connect``. WAL-incompatible filesystems either raise ``OperationalError`` ("locking protocol" / "disk I/O error") or —
macOS NFS / SMB / AgentFS — silently refuse and stay in DELETE; either way log ERROR once per process per
``db_label`` and fall back. ``require_wal=True`` raises :class:`WalUnsupportedError` instead. WAL-reset-bug
builds (https://sqlite.org/wal.html#walresetbug) never enable WAL on non-WAL files; an already-WAL DB keeps WAL
@@ -220,17 +221,13 @@ def apply_wal_with_fallback(conn: sqlite3.Connection, *, db_label: str = "state.
raise sqlite3.OperationalError(_CANNOT_VERIFY_DELETE_MSG)
return _verify_configured_delete(_set_journal_mode_no_wait(conn, "DELETE"))
if current_mode is None:
# #104596: the probe failed (locked/busy under load) and the on-disk mode is unknown. A fresh 0-page
# DB probes "delete" cleanly, so None here means the probe could not read a real file — most often
# because a sibling connection (same process or another) holds it, in WAL, with live -wal/-shm sidecars.
# Emitting the set-pragma inside _enable_wal would re-run WAL-init and UNLINK those sidecars under the
# holder: two -shm generations that cannot see each other's locks -> split-brain corruption
# (btreeInitPage error 11). Ownership is not provably exclusive, so touch nothing: an already-WAL DB
# keeps working (this connection inherits the mode from the on-disk header, exactly like the early
# return above), the rare true-DELETE file degrades to DELETE with the warning below, and a new DB
# reaches _enable_wal on its next open. Mirrors the vulnerable-gate path's indeterminate handling.
# Probe failed (locked/busy): ownership not provably exclusive, same as the DELETE branch above. A fresh
# 0-page DB probes "delete" cleanly, so this is a real file some other connection holds — running WAL-init
# would unlink its -wal/-shm sidecars. Touch nothing: the connection inherits whatever mode the header has.
if require_wal:
raise WalUnsupportedError("could not verify the on-disk journal mode (database is locked — possible "
"concurrent openers); cannot guarantee WAL")
_log_once("wal_probe_unknown", db_label)
_apply_wal_companions(conn)
return "wal"
return _enable_wal(conn, db_label, require_wal, current_mode)
@@ -408,16 +405,11 @@ _ONCE_LOGS = {
"this DB and run a one-time offline 'PRAGMA journal_mode=DELETE' on the file. This message fires once per "
"process per database."),
"wal_probe_unknown": (_wal_probe_unknown_lock, "_wal_probe_unknown_paths", logging.WARNING,
# #104596: the read-only probe failed (locked/busy under load). A fresh 0-page DB probes "delete" cleanly,
# so None means the probe could not read a real file — most often because a sibling connection holds it, in
# WAL, with live -wal/-shm sidecars. Emitting the set-pragma here re-runs WAL-init and unlinks those
# sidecars under the holder: two -shm generations that cannot see each other's locks -> split-brain
# corruption. Keep the WARNING (not ERROR): the connection still inherits WAL from the on-disk header, so
# the common case is harmless — only the rare true-DELETE file degrades (silently, to DELETE).
"%s: could not verify the on-disk journal mode (database is locked / busy under load); refusing to issue a "
"journal-mode set-pragma while ownership is not provably exclusive (it could unlink the -wal/-shm sidecars "
"a sibling connection still holds open — split-brain corruption, #104596). Assuming the configured "
"journal_mode=wal and leaving the file untouched. This message fires once per process per database."),
# WARNING, not ERROR: the connection inherits the header's mode, so an already-WAL file (the common case)
# keeps working; only a true-DELETE file stays DELETE for this connection.
"%s: could not verify the on-disk journal mode (database is locked / busy); not issuing a journal-mode "
"set-pragma while another connection may hold the file (it could unlink the -wal/-shm sidecars that "
"connection still uses). Leaving the file untouched; this connection inherits the on-disk mode. This message fires once per process per database."),
}

5
tests/test_hermes_state_wal_fallback.py
View File

@@ -428,7 +428,8 @@ class TestGetLastInitError:
"""When SessionDB() raises, the cause is preserved for slash commands.
Simulates a filesystem where BOTH WAL and DELETE journal modes fail —
e.g. a read-only mount where no ``PRAGMA journal_mode=X`` works. The
e.g. a read-only mount where no ``PRAGMA journal_mode=X`` works (the
read-only mode probe still succeeds, as on a real mount). The
fallback tries DELETE and also gets rejected; the exception bubbles
out of ``SessionDB.__init__`` and the cause is captured.
"""
@@ -437,7 +438,7 @@ class TestGetLastInitError:
class _BothPragmasFailConnection(sqlite3.Connection):
def execute(self, sql, *args, **kwargs): # type: ignore[override]
if "journal_mode" in sql.lower():
if "journal_mode=" in sql.lower().replace(" ", ""):
raise sqlite3.OperationalError(
"locking protocol: read-only filesystem"
)

86
tests/test_journal_mode_config.py
View File

@@ -43,19 +43,13 @@ def _reset_configured_delete_override_warned_paths():
def test_wal_probe_unknown_never_emits_set_pragma(monkeypatch, tmp_path, caplog):
"""#104596: probe failure (None) must not reach the WAL set-pragma.
The DELETE branch refuses to flip modes when the on-disk mode cannot be
verified (ownership not provably exclusive). The configured-WAL branch
used to fall through to ``_enable_wal`` and emit ``PRAGMA
journal_mode=WAL`` anyway; when a sibling connection holds the DB in WAL
with live -wal/-shm sidecars, WAL-init unlinks those sidecars under the
holder -> two -shm generations -> split-brain corruption (btreeInitPage
error 11). The guard must touch nothing and assume the configured mode.
"""
"""Probe failure (None) on the configured-WAL path must not reach any journal-mode
pragma: the file may be held by a sibling whose -wal/-shm sidecars WAL-init would
unlink. The DELETE branch already refused; this binds the WAL branch to the same rule,
and ``require_wal=True`` must raise instead of reporting an unverified "wal"."""
import logging
from hermes_state_wal import apply_wal_with_fallback
from hermes_state_wal import WalUnsupportedError, apply_wal_with_fallback
_configure_mode(monkeypatch, tmp_path, "wal")
_disable_vulnerable_gate(monkeypatch)
@@ -64,80 +58,34 @@ def test_wal_probe_unknown_never_emits_set_pragma(monkeypatch, tmp_path, caplog)
class _SpyConnection(sqlite3.Connection):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.emitted: list[str] = []
self.pragmas: list[str] = []
def execute(self, sql, *args, **kwargs): # type: ignore[override]
if "journal_mode" in sql.lower() and "=" in sql.lower():
self.emitted.append(str(sql))
if str(sql).lstrip().lower().startswith("pragma"):
self.pragmas.append(str(sql))
return super().execute(sql, *args, **kwargs)
db_path = tmp_path / "probe-unknown.db"
sibling = sqlite3.connect(str(db_path))
try:
# A DB that is genuinely WAL on disk; the sibling holds the sidecars.
assert (
sibling.execute("PRAGMA journal_mode=WAL").fetchone()[0].lower()
== "wal"
)
# The probe fails under load (locked/busy) -> on-disk mode unknown.
monkeypatch.setattr(
"hermes_state_wal._on_disk_journal_mode", lambda _conn: None
)
assert sibling.execute("PRAGMA journal_mode=WAL").fetchone()[0].lower() == "wal"
monkeypatch.setattr("hermes_state_wal._on_disk_journal_mode", lambda _conn: None)
conn = sqlite3.connect(str(db_path), factory=_SpyConnection)
try:
with caplog.at_level(logging.WARNING, logger="hermes_state_wal"):
result = apply_wal_with_fallback(
conn, db_label="probe-unknown.db"
)
assert result == "wal" # assumed configured mode
assert conn.emitted == [] # no set-pragma while ownership unproven
# The sibling's on-disk WAL state is untouched.
assert (
sibling.execute("PRAGMA journal_mode").fetchone()[0].lower()
== "wal"
)
assert any(
"could not verify the on-disk journal mode" in r.getMessage()
for r in caplog.records
)
assert apply_wal_with_fallback(conn, db_label="probe-unknown.db") == "wal"
assert conn.pragmas == [] # nothing touched while ownership is unproven
assert sibling.execute("PRAGMA journal_mode").fetchone()[0].lower() == "wal"
assert any("could not verify the on-disk journal mode" in r.getMessage() for r in caplog.records)
with pytest.raises(WalUnsupportedError, match="could not verify the on-disk journal mode"):
apply_wal_with_fallback(conn, db_label="probe-unknown.db", require_wal=True)
assert conn.pragmas == []
finally:
conn.close()
finally:
sibling.close()
def test_wal_probe_unknown_warns_once_per_database(monkeypatch, tmp_path, caplog):
"""Dedupe contract: one WARNING per (process, db_label)."""
import logging
from hermes_state_wal import apply_wal_with_fallback
_configure_mode(monkeypatch, tmp_path, "wal")
_disable_vulnerable_gate(monkeypatch)
hermes_state_wal._wal_probe_unknown_paths.clear()
monkeypatch.setattr(
"hermes_state_wal._on_disk_journal_mode", lambda _conn: None
)
db_path = tmp_path / "warn.db"
conn = sqlite3.connect(str(db_path))
try:
with caplog.at_level(logging.WARNING, logger="hermes_state_wal"):
assert apply_wal_with_fallback(conn, db_label="warn.db") == "wal"
assert apply_wal_with_fallback(conn, db_label="warn.db") == "wal"
assert (
apply_wal_with_fallback(conn, db_label="other.db") == "wal"
)
hits = [
r
for r in caplog.records
if "could not verify the on-disk journal mode" in r.getMessage()
]
assert len(hits) == 2 # warn.db once, other.db once
finally:
conn.close()
def test_database_journal_mode_has_a_canonical_default():
from hermes_cli.config import DEFAULT_CONFIG

17
tests/test_sqlite_wal_reset_gate.py
View File

@@ -351,20 +351,9 @@ class TestNoDowngradeUnderConcurrentOpeners:
check.close()
def test_probe_unreadable_touches_nothing(self, tmp_path, monkeypatch, caplog):
"""#104596: when the on-disk mode cannot be verified (possible
concurrent openers — same-process siblings holding live -wal/-shm
sidecars), the WAL path must not emit the set-pragma at all.
Previously it fell through to ``PRAGMA journal_mode=WAL``; on an
incompatible filesystem that raised, but on a WAL DB whose sidecars
a sibling connection still holds, WAL-init silently unlinked them ->
two -shm generations -> split-brain corruption (btreeInitPage error
11). The guard now refuses to touch anything while ownership is
unproven: it assumes the configured WAL mode, logs one warning per
database, and returns without issuing the set-pragma — strictly more
conservative than the old raise (which still required emitting the
dangerous pragma first).
"""
"""Non-vulnerable runtime, probe blocked ("database is locked"): the WAL path
must not emit the set-pragma (here it would raise "locking protocol"); it
warns once and reports the inherited mode instead."""
import logging
monkeypatch.setattr(