diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index bbf84c7591..5646f78708 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -144,8 +144,8 @@ def _report_database_journal_modes(hermes_home: Path | None = None, version_info check_warn(f"{name} is in WAL mode ({size}) despite database.journal_mode=delete", "(the setting never applied: an existing WAL database is never live-downgraded" + ("; also exposed to the WAL-reset bug" if vulnerable else "") - + ". Stop every Hermes process for this profile, then run a one-time offline " - "'PRAGMA journal_mode=DELETE' on the file)") + + ". Stop every Hermes process for this profile, then run " + f"`hermes sessions set-journal-mode delete{'' if name == 'state.db' else f' --db {path}'}`)") _report_database_holders(name, path) elif error is not None: if vulnerable: @@ -159,9 +159,9 @@ def _report_database_journal_modes(hermes_home: Path | None = None, version_info if vulnerable: exposed.append(name) check_warn(f"{name} is in WAL mode on a cross-VM filesystem (virtiofs/9p, {size})", - "(WAL can silently corrupt across the VM boundary; stop every Hermes process and run a one-time " - "offline 'PRAGMA journal_mode=DELETE' on the file, then set `database.journal_mode: delete` — " - "or move the database onto a native/named volume)") + "(WAL can silently corrupt across the VM boundary; stop every Hermes process and run " + f"`hermes sessions set-journal-mode delete{'' if name == 'state.db' else f' --db {path}'}`, then " + "set `database.journal_mode: delete` — or move the database onto a native/named volume)") elif mode == "wal" and vulnerable: exposed.append(name) check_warn(f"{name} is in WAL mode ({size})", "(exposed to the WAL-reset bug until SQLite is upgraded)") diff --git a/hermes_cli/sessions_cmd.py b/hermes_cli/sessions_cmd.py index c1d02e416e..958d42dc91 100644 --- a/hermes_cli/sessions_cmd.py +++ b/hermes_cli/sessions_cmd.py @@ -971,9 +971,15 @@ def _cmd_repair_profiles(args): return cmd_repair_profiles(args) +def _cmd_set_journal_mode(args): + from hermes_cli.sessions_cmd_journal_mode import cmd_set_journal_mode + return cmd_set_journal_mode(args) + + _PRE_DB_HANDLERS = { "repair": _cmd_repair, "recover": _cmd_recover, "import": _cmd_import, "repair-profiles": _cmd_repair_profiles, # opens every profile's store itself + "set-journal-mode": _cmd_set_journal_mode, # offline: must not open the store it converts } _OBSERVATIONAL_DB_ACTIONS = frozenset({"list", "stats", "pinned"}) _DB_HANDLERS = { diff --git a/hermes_cli/sessions_cmd_journal_mode.py b/hermes_cli/sessions_cmd_journal_mode.py new file mode 100644 index 0000000000..4c9585d983 --- /dev/null +++ b/hermes_cli/sessions_cmd_journal_mode.py @@ -0,0 +1,77 @@ +"""`hermes sessions set-journal-mode delete|wal` — the offline self-service path for #100896. + +``database.journal_mode: delete`` can never self-apply on a store that is already WAL: +``apply_wal_with_fallback`` deliberately never live-downgrades (#68545 — other gateway/cron/worker connections +may hold uncheckpointed WAL commits). Before this command the only escape hatch was an undocumented hand-run +``PRAGMA journal_mode=DELETE``. This runs it under the same admission the maintenance paths use: refuse while +ANY foreign process holds the file or a sidecar (``foreign_state_db_holders``), flip without waiting out openers +(``_set_journal_mode_no_wait``), then verify the header bytes 18/19 SQLite writes for the mode. +""" +from __future__ import annotations + +import os +import sqlite3 +from pathlib import Path + +# SQLite file header: bytes 18 (write version) / 19 (read version) are 1 for rollback-journal, 2 for WAL. +_HEADER_VERSION_BYTES = {"delete": (1, 1), "wal": (2, 2)} + + +def _header_mode(db_path: Path) -> str: + """Journal mode as the on-disk header reports it (no SQLite connection is open in this process here).""" + fd = os.open(db_path, os.O_RDONLY) + try: + head = os.pread(fd, 20, 0) + finally: + os.close(fd) + if len(head) < 20 or head[:16] != b"SQLite format 3\x00": + return "not-a-database" + return {(1, 1): "delete", (2, 2): "wal"}.get((head[18], head[19]), f"unknown({head[18]}/{head[19]})") + + +def cmd_set_journal_mode(args) -> int: + from hermes_state import DEFAULT_DB_PATH + from hermes_state_holders import describe_holder_pid, foreign_state_db_holders + from hermes_state_wal import _set_journal_mode_no_wait, is_sqlite_wal_reset_vulnerable, resolve_journal_mode + + target = args.mode + db_path = Path(getattr(args, "db", None) or DEFAULT_DB_PATH) + if not db_path.exists(): + print(f"No database at {db_path} (nothing to convert).") + return 1 + current = _header_mode(db_path) + if current == target: + print(f"✓ {db_path} is already journal_mode={target}.") + return 0 + if target == "wal" and is_sqlite_wal_reset_vulnerable(): + print(f"✗ Refusing to enable WAL: the linked SQLite {sqlite3.sqlite_version} has the WAL-reset bug " + "(https://sqlite.org/wal.html#walresetbug). Upgrade to 3.51.3+ first.") + return 1 + holders = foreign_state_db_holders(db_path) + if holders: + print(f"✗ Refusing to change the journal mode of {db_path}: other processes hold it open " + "(a live switch would destroy their uncheckpointed commits). Stop them and re-run:") + for pid, target_path in holders: + print(f" pid {pid} — {describe_holder_pid(pid) if pid > 0 else 'scan'}: {target_path}") + return 1 + # timeout=0: any opener that appeared between the scan and the flip makes the pragma fail with + # 'database is locked' instead of sneaking the switch between a writer's transactions. + conn = sqlite3.connect(str(db_path), timeout=0.0, isolation_level=None) + try: + try: + reported = _set_journal_mode_no_wait(conn, target.upper()) + except sqlite3.OperationalError as exc: + print(f"✗ journal_mode={target.upper()} refused: {exc} (another opener appeared; nothing was changed)") + return 1 + finally: + conn.close() + after = _header_mode(db_path) + if after != target: + print(f"✗ SQLite reported journal_mode={reported or '?'} but the file header reads {after}; not converted.") + return 1 + print(f"✓ {db_path}: journal_mode {current} → {after} (header verified).") + configured = resolve_journal_mode() + if configured != target: + print(f" Note: config.yaml has database.journal_mode: {configured} — the next open would switch the file " + f"back. Set `database.journal_mode: {target}` in config.yaml to keep it.") + return 0 diff --git a/hermes_cli/subcommands/sessions.py b/hermes_cli/subcommands/sessions.py index 9e8fc885c1..b83b2a54b8 100644 --- a/hermes_cli/subcommands/sessions.py +++ b/hermes_cli/subcommands/sessions.py @@ -167,6 +167,18 @@ def build_sessions_parser(subparsers, *, cmd_sessions: Callable) -> None: help="Only report whether the database opens cleanly; do not modify it") _flag(sessions_repair, "--no-backup", help="Skip the timestamped backup copy (not recommended)") + sessions_set_journal_mode = sessions_subparsers.add_parser( + "set-journal-mode", help="Convert state.db between journal_mode=WAL and DELETE offline (every holder stopped)", + description="Switch the on-disk journal mode of the session store. Hermes never " + "live-downgrades a WAL database at startup (other processes may hold " + "uncheckpointed commits), so `database.journal_mode: delete` cannot " + "self-apply to an existing WAL store. Run this with the gateway, " + "dashboard and every CLI stopped: it refuses while any process holds " + "the file, switches the mode, and verifies the file header.") + sessions_set_journal_mode.add_argument("mode", choices=("delete", "wal"), help="Target journal mode") + sessions_set_journal_mode.add_argument("--db", default=None, metavar="PATH", + help="Convert another Hermes SQLite store (e.g. kanban.db) instead of the profile's state.db") + sessions_repair_routing = sessions_subparsers.add_parser( "repair-routing", help="Re-stamp gateway sessions that lost their routing identity", description="Find gateway conversations stranded in session rows whose " diff --git a/hermes_state_wal.py b/hermes_state_wal.py index 50ecc793e6..114853c313 100644 --- a/hermes_state_wal.py +++ b/hermes_state_wal.py @@ -518,9 +518,9 @@ _ONCE_LOGS = { "delete_overridden": (_delete_overridden_warned_lock, "_delete_overridden_warned_paths", logging.ERROR, # Never-live-downgrade keeps WAL; without this the operator never learns their delete had no effect. "%s: database.journal_mode=delete is configured but the on-disk database is already WAL; keeping WAL (a live " - "downgrade under open connections can corrupt the DB). To apply journal_mode=DELETE, stop all connections to " - "this DB and run a one-time offline 'PRAGMA journal_mode=DELETE' on the file. This message fires once per " - "process per database."), + "downgrade under open connections can corrupt the DB). To apply journal_mode=DELETE, stop every Hermes " + "process using this database and run `hermes sessions set-journal-mode delete` (add `--db PATH` for a " + "store other than state.db). This message fires once per process per database."), "wal_probe_unknown": (_wal_probe_unknown_lock, "_wal_probe_unknown_paths", logging.WARNING, # 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. @@ -540,8 +540,8 @@ _ONCE_LOGS = { "%s: existing WAL-mode database is on a cross-VM filesystem (virtiofs/9p — typical for Docker Desktop / " "OrbStack / Podman host bind mounts). SQLite WAL shared-memory is not coherent across the VM boundary and " "concurrent writers can silently corrupt the database. Hermes does not live-downgrade an on-disk WAL database. " - "Fix one of two ways: stop every Hermes process using this database and run a one-time offline " - "'PRAGMA journal_mode=DELETE' on the file (set `database.journal_mode: delete` in config.yaml to keep it), " + "Fix one of two ways: stop every Hermes process using this database and run `hermes sessions " + "set-journal-mode delete` (set `database.journal_mode: delete` in config.yaml to keep it), " "or move the database onto a native volume (e.g. a named Docker volume). This message fires once per process " "per database."), } diff --git a/tests/hermes_cli/test_doctor_journal_modes.py b/tests/hermes_cli/test_doctor_journal_modes.py index b208a3a2ef..0a6ba46faf 100644 --- a/tests/hermes_cli/test_doctor_journal_modes.py +++ b/tests/hermes_cli/test_doctor_journal_modes.py @@ -333,7 +333,7 @@ class TestReportDatabaseJournalModes: out = capsys.readouterr().out assert "state.db is in WAL mode on a cross-VM filesystem" in out - assert "PRAGMA journal_mode=DELETE" in out + assert "hermes sessions set-journal-mode delete" in out def test_vulnerable_runtime_wal_db_is_exposed(self, tmp_path, capsys): _make_db(tmp_path / "state.db", journal_mode="WAL") @@ -459,7 +459,7 @@ class TestConfiguredDeleteNeverApplied: out = capsys.readouterr().out assert "state.db is in WAL mode" in out and "despite database.journal_mode=delete" in out - assert "never live-downgraded" in out and "PRAGMA journal_mode=DELETE" in out + assert "never live-downgraded" in out and "hermes sessions set-journal-mode delete" in out assert "state.db: WAL journal mode" not in out assert ("To clear the exposure:" in out) is exposed diff --git a/tests/hermes_cli/test_sessions_set_journal_mode.py b/tests/hermes_cli/test_sessions_set_journal_mode.py new file mode 100644 index 0000000000..8fbbf27db5 --- /dev/null +++ b/tests/hermes_cli/test_sessions_set_journal_mode.py @@ -0,0 +1,69 @@ +"""`hermes sessions set-journal-mode` converts an existing WAL store offline and refuses under a foreign holder. + +#100896 (@ruangraung): `database.journal_mode: delete` never self-applies to a store that is already WAL because +open never live-downgrades; this command is the sanctioned offline path and must fail closed while any other +process holds the file. +""" +import argparse +import sqlite3 +import subprocess +import sys +import time + +import pytest + +from hermes_cli.sessions_cmd import cmd_sessions + + +def _wal_store(path): + conn = sqlite3.connect(str(path), isolation_level=None) + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("CREATE TABLE t(x)") + conn.execute("INSERT INTO t VALUES (1), (2), (3)") + conn.close() + assert path.read_bytes()[18:20] == b"\x02\x02" + + +def _args(mode): + return argparse.Namespace(sessions_action="set-journal-mode", mode=mode, db=None) + + +@pytest.mark.skipif(sys.platform == "win32", reason="holder scan and header probe are POSIX-only") +def test_set_journal_mode_converts_wal_store_offline(tmp_path, monkeypatch, capsys): + db = tmp_path / "state.db" + _wal_store(db) + monkeypatch.setattr("hermes_state.DEFAULT_DB_PATH", db) + + assert cmd_sessions(_args("delete")) == 0 + + assert db.read_bytes()[18:20] == b"\x01\x01", "header must report rollback-journal mode" + assert not (tmp_path / "state.db-wal").exists() + conn = sqlite3.connect(str(db)) + assert conn.execute("PRAGMA journal_mode").fetchone()[0].lower() == "delete" + assert conn.execute("SELECT count(*) FROM t").fetchone()[0] == 3 + conn.close() + assert "wal → delete" in capsys.readouterr().out + + +@pytest.mark.skipif(sys.platform == "win32", reason="holder scan is POSIX-only") +def test_set_journal_mode_refuses_while_another_process_holds_the_store(tmp_path, monkeypatch, capsys): + db = tmp_path / "state.db" + _wal_store(db) + monkeypatch.setattr("hermes_state.DEFAULT_DB_PATH", db) + holder = subprocess.Popen( + [sys.executable, "-c", + f"import sqlite3, time; c = sqlite3.connect({str(db)!r}); c.execute('SELECT 1'); time.sleep(60)"], + stdin=subprocess.DEVNULL, + ) + try: + deadline = time.monotonic() + 10 + while time.monotonic() < deadline and not (tmp_path / "state.db-shm").exists(): + time.sleep(0.05) + assert cmd_sessions(_args("delete")) == 1 + finally: + holder.kill() + holder.wait() + + out = capsys.readouterr().out + assert f"pid {holder.pid}" in out + assert db.read_bytes()[18:20] == b"\x02\x02", "a refused switch must leave the file untouched" diff --git a/tests/hermes_state/test_cross_vm_fs_wal_refusal.py b/tests/hermes_state/test_cross_vm_fs_wal_refusal.py index 995f95eb21..3969cefa3d 100644 --- a/tests/hermes_state/test_cross_vm_fs_wal_refusal.py +++ b/tests/hermes_state/test_cross_vm_fs_wal_refusal.py @@ -120,5 +120,5 @@ class TestWalRefusalOnCrossVmFs: conn.close() errors = [r for r in caplog.records if r.levelno == logging.ERROR and "cross-VM" in r.getMessage()] assert len(errors) == 1 - assert "PRAGMA journal_mode=DELETE" in errors[0].getMessage() + assert "hermes sessions set-journal-mode delete" in errors[0].getMessage() assert "native volume" in errors[0].getMessage() diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index ae268412e8..8cf9694a66 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -102,8 +102,8 @@ database: # live-downgraded — Hermes keeps WAL and logs an error telling you the # configured delete did not apply (or that the WAL database sits on a # cross-VM mount). To convert an existing database, stop - # every process using it and run a one-time offline - # `PRAGMA journal_mode=DELETE` on the file. + # every process using it and run + # `hermes sessions set-journal-mode delete` (see Sessions). journal_mode: wal # Durability level for every state.db connection: OFF, NORMAL, FULL, @@ -126,8 +126,9 @@ The reverse never happens automatically: a database that is already in WAL mode is not live-downgraded when you set `journal_mode: delete` (a downgrade under open connections can corrupt it). `hermes doctor` warns ` is in WAL mode despite database.journal_mode=delete` until you stop -every Hermes process for the profile and run a one-time offline -`PRAGMA journal_mode=DELETE` on the file. Under that warning it names the +every Hermes process for the profile and run +`hermes sessions set-journal-mode delete` (it refuses while anything still +holds the file and verifies the converted header). Under that warning it names the processes currently holding the database (` is held by PID ()`) so you know what to stop; when the holder scan is partial or unavailable it says `cannot prove the database is quiet` instead of giving an all-clear. diff --git a/website/docs/user-guide/sessions.md b/website/docs/user-guide/sessions.md index af3b2b9cd8..e02cc54bcd 100644 --- a/website/docs/user-guide/sessions.md +++ b/website/docs/user-guide/sessions.md @@ -700,6 +700,30 @@ index in memory and would write it back), and is safe to re-run: a second run finds nothing. +### Convert the Store Between WAL and DELETE Journal Mode + +`database.journal_mode: delete` only applies to databases Hermes creates. An +existing `state.db` that is already in WAL mode is **never** live-downgraded at +open — other gateway, dashboard or cron processes may hold uncheckpointed WAL +commits, and a downgrade underneath them destroys those commits — so Hermes +keeps WAL and logs one `ERROR` per process telling you the configured `delete` +did not apply. The self-service conversion is: + +```bash +# stop every process using the profile's store first (gateway, dashboard, CLIs, cron) +hermes sessions set-journal-mode delete # WAL -> rollback journal +hermes sessions set-journal-mode wal # back to WAL +hermes sessions set-journal-mode delete --db ~/.hermes/kanban.db # another Hermes store +``` + +The command refuses — naming each PID and command — while any process still +holds the file or its `-wal`/`-shm` sidecars, switches the mode without +waiting out openers (a holder that appears mid-way makes SQLite refuse instead +of racing it), and verifies the file header reports the new mode. It reminds +you to set `database.journal_mode` to the same value when the config disagrees, +because the next open re-applies the configured mode. + + ## Importing Sessions from Claude Code and Codex CLI Started a conversation in another agent CLI? You can pull it into Hermes and