Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 46 additions & 7 deletions hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,9 +646,12 @@ def remove_board(slug: str, *, archive: bool = True) -> dict:
clear_current_board()

# A concurrent connect(board=normed) after the rename/delete recreates
# an empty sqlite file via mkdir(exist_ok=True); the cache entry must be
# dropped first so the schema init pass re-runs on that fresh file.
_INITIALIZED_PATHS.discard(str((d / "kanban.db").resolve()))
# an empty sqlite file via mkdir(exist_ok=True); the cache entries must be
# dropped first so the schema init pass re-runs on that fresh file and the
# fresh (DELETE-mode) file is put back into WAL via _ensure_wal_once.
_gone = str((d / "kanban.db").resolve())
_INITIALIZED_PATHS.discard(_gone)
_WAL_DONE.discard(_gone)

if archive:
archive_root = boards_root() / "_archived"
Expand Down Expand Up @@ -1087,6 +1090,12 @@ class Event:

_INITIALIZED_PATHS: set[str] = set()
_INIT_LOCK = threading.RLock()
# journal_mode=WAL is a persistent, file-level property — no need to re-apply
# it on every connection. Tracks paths already confirmed in WAL so
# _ensure_wal_once() skips apply_wal_with_fallback on steady-state connects.
# See _ensure_wal_once (incident 2026-05-27: per-connection re-toggling
# desynced -wal/-shm coordination and corrupted the kanban DB).
_WAL_DONE: set[str] = set()
_SQLITE_HEADER = b"SQLite format 3\x00"
DEFAULT_BUSY_TIMEOUT_MS = 120_000

Expand Down Expand Up @@ -1355,6 +1364,33 @@ def _guard_existing_db_is_healthy(path: Path) -> None:
raise KanbanDbCorruptError(resolved, backup, reason)


def _ensure_wal_once(
conn: sqlite3.Connection, resolved: str, *, db_label: str
) -> None:
"""Put the DB in WAL mode at most once per process per path.

WAL is persistent at the file level, so once a DB is in WAL every later
connection inherits it. Re-issuing ``PRAGMA journal_mode=WAL`` on every
connection is dangerous: each call takes an exclusive lock, and the DELETE
fallback in :func:`hermes_state.apply_wal_with_fallback` can flip the
persisted mode out from under other live connections. Combined with a
connection leak, that re-toggling is what desynced the ``-wal``/``-shm``
coordination and corrupted the kanban DB on 2026-05-27.

We read the current mode first and only invoke the fallback helper when
the DB is not already WAL; ``_WAL_DONE`` then skips even that read on
subsequent connects. Must be called inside ``_INIT_LOCK``.
"""
if resolved in _WAL_DONE:
return
row = conn.execute("PRAGMA journal_mode").fetchone()
current = (row[0] if row else "").lower()
if current != "wal":
from hermes_state import apply_wal_with_fallback
apply_wal_with_fallback(conn, db_label=db_label)
_WAL_DONE.add(resolved)


def connect(
db_path: Optional[Path] = None,
*,
Expand Down Expand Up @@ -1403,8 +1439,9 @@ def connect(
# WAL doesn't work on network filesystems (NFS/SMB/FUSE). Shared helper
# falls back to DELETE with one WARNING so kanban stays usable there.
# See hermes_state._WAL_INCOMPAT_MARKERS for detection logic.
from hermes_state import apply_wal_with_fallback
apply_wal_with_fallback(conn, db_label=f"kanban.db ({path.name})")
# _ensure_wal_once skips re-applying WAL if already confirmed this
# process — avoids per-connection exclusive lock (incident 2026-05-27).
_ensure_wal_once(conn, resolved, db_label=f"kanban.db ({path.name})")
# FULL (was NORMAL): fsync before each checkpoint to narrow the
# crash window that can leave a b-tree page header torn.
conn.execute("PRAGMA synchronous=FULL")
Expand Down Expand Up @@ -1488,10 +1525,12 @@ def init_db(
path = kanban_db_path(board=board)
path.parent.mkdir(parents=True, exist_ok=True)
resolved = str(path.resolve())
# Clear the cache entry so the underlying connect() re-runs the
# schema + migration pass unconditionally.
# Clear the cache entries so the underlying connect() re-runs the schema +
# migration pass unconditionally, and _ensure_wal_once re-applies WAL on
# the rebuilt DB.
with _INIT_LOCK:
_INITIALIZED_PATHS.discard(resolved)
_WAL_DONE.discard(resolved)
with contextlib.closing(connect(path)):
pass
return path
Expand Down
129 changes: 128 additions & 1 deletion tests/hermes_cli/test_kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2380,8 +2380,12 @@ def test_connect_falls_back_to_delete_on_locking_protocol(tmp_path, monkeypatch,
monkeypatch.setenv("HERMES_HOME", str(home))
monkeypatch.setattr(Path, "home", lambda: tmp_path)

# Clear module cache so a fresh connect() is attempted
# Clear module caches so a fresh connect() is attempted. _WAL_DONE must
# also be cleared: the fixture leaves the path absent from _WAL_DONE so
# _ensure_wal_once() still runs the PRAGMA check, which lets the blocking
# connection below trigger the NFS fallback.
kb._INITIALIZED_PATHS.clear()
kb._WAL_DONE.clear()

real_connect = _sqlite3.connect

Expand Down Expand Up @@ -4086,3 +4090,126 @@ def test_bare_connect_does_not_close_on_context_exit(tmp_path):
# Still usable after with-block exit (the leak).
conn.execute("SELECT 1").fetchone()
conn.close() # explicit close to avoid leaking THIS test


# ---------------------------------------------------------------------------
# _ensure_wal_once: WAL PRAGMA issued at most once per process per path
# ---------------------------------------------------------------------------


def test_wal_pragma_not_reissued_on_reconnect(tmp_path, monkeypatch):
"""apply_wal_with_fallback must not be called on every connect().

Re-issuing PRAGMA journal_mode=WAL on every connection takes an exclusive
lock each time and risks the DELETE fallback flipping the persisted mode
out from under other live connections (incident 2026-05-27). After the
first successful connect _WAL_DONE caches the path and _ensure_wal_once()
becomes a no-op for all subsequent connects — apply_wal_with_fallback is
never called again for that path.
"""
from unittest.mock import patch as _patch, call as _call

db_path = tmp_path / "kanban.db"
resolved = str(db_path.resolve())
kb._INITIALIZED_PATHS.discard(resolved)
kb._WAL_DONE.discard(resolved)

import hermes_state as _hs
real_wal = _hs.apply_wal_with_fallback
calls: list[str] = []

def tracking_wal(conn, *, db_label=""):
calls.append(db_label)
return real_wal(conn, db_label=db_label)

with _patch.object(_hs, "apply_wal_with_fallback", side_effect=tracking_wal):
# First connect — apply_wal_with_fallback must be called.
conn1 = kb.connect(db_path=db_path)
calls_after_first = len(calls)
conn1.close()

# Second connect — _WAL_DONE is populated; apply_wal_with_fallback
# must NOT be called again.
conn2 = kb.connect(db_path=db_path)
calls_after_second = len(calls)
conn2.close()

assert calls_after_first >= 1, "apply_wal_with_fallback should run on first connect"
assert calls_after_second == calls_after_first, (
f"apply_wal_with_fallback called again on reconnect "
f"(calls: first={calls_after_first}, second={calls_after_second})"
)


def test_wal_done_cleared_by_init_db(tmp_path):
"""init_db() must evict _WAL_DONE so a fresh DB is re-entered into WAL.

Scenario: DB is connected (WAL mode, _WAL_DONE populated). The file is
then deleted and recreated empty — simulating a DB reset or corruption
recovery. Without clearing _WAL_DONE, the subsequent init_db() would see
the stale cache entry and skip _ensure_wal_once(), leaving the fresh DB in
DELETE mode. With the fix, init_db() discards the stale _WAL_DONE entry
first, so _ensure_wal_once() reads "delete" and calls apply_wal_with_fallback.
"""
import hermes_state as _hs
from unittest.mock import patch as _patch

db_path = tmp_path / "kanban.db"
resolved = str(db_path.resolve())
kb._INITIALIZED_PATHS.discard(resolved)
kb._WAL_DONE.discard(resolved)

real_wal = _hs.apply_wal_with_fallback
calls: list[str] = []

def tracking_wal(conn, *, db_label=""):
calls.append(db_label)
return real_wal(conn, db_label=db_label)

with _patch.object(_hs, "apply_wal_with_fallback", side_effect=tracking_wal):
# First connect — WAL applied, _WAL_DONE populated.
conn = kb.connect(db_path=db_path)
conn.close()
calls_after_first = len(calls)
assert calls_after_first >= 1

# Delete and recreate an empty DB file — simulates a reset/recovery.
# The new file starts in SQLite's default DELETE journal mode.
db_path.unlink()
db_path.touch()

# init_db() must clear _WAL_DONE before reconnecting so
# _ensure_wal_once() re-reads the journal_mode of the fresh file.
kb.init_db(db_path=db_path)
assert len(calls) > calls_after_first, (
"init_db() must trigger apply_wal_with_fallback on the rebuilt DB; "
"stale _WAL_DONE entry would have caused it to skip WAL setup entirely"
)

# Confirm the DB is now actually in WAL mode.
import sqlite3 as _sqlite3
c = _sqlite3.connect(str(db_path))
mode = c.execute("PRAGMA journal_mode").fetchone()[0]
c.close()
assert mode == "wal", f"Expected WAL after init_db rebuild, got: {mode}"


def test_wal_done_cleared_by_remove_board(tmp_path, monkeypatch):
"""remove_board() must evict the path from _WAL_DONE so a fresh board re-enters WAL."""
home = tmp_path / ".hermes"
home.mkdir()
monkeypatch.setenv("HERMES_HOME", str(home))
monkeypatch.setattr("pathlib.Path.home", lambda: tmp_path)

slug = "test-wal-evict"
kb.init_db(board=slug)
board_db = kb.kanban_db_path(board=slug)
resolved = str(board_db.resolve())

# Populate _WAL_DONE.
conn = kb.connect(board=slug)
conn.close()
assert resolved in kb._WAL_DONE

kb.remove_board(slug, archive=False)
assert resolved not in kb._WAL_DONE, "remove_board() must clear _WAL_DONE"