diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index 5a4df2670d..33fb7fccad 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -89,6 +89,37 @@ def _format_db_size(db_path: Path) -> str: return "size unknown" +def _report_database_holders(name: str, db_path: Path) -> None: + """Name the processes holding ``db_path`` (or a WAL sidecar) so the operator knows what to stop before the + offline journal-mode conversion; a partial or unavailable scan is reported as "cannot prove quiet", never as + an all-clear (the scan is the same fail-closed authority repair/VACUUM/checkpoint admission uses).""" + from hermes_state_holders import _read_proc_argv, foreign_state_db_holders, psutil + if sys.platform == "win32": + check_warn(f"{name}: cannot prove the database is quiet", "(holder scan is unavailable on Windows)") + return + unknown: list[str] = [] + by_pid: dict[int, set[str]] = {} + for pid, target in foreign_state_db_holders(db_path): + if pid <= 0 or target.startswith("uninspectable"): + unknown.append(target) + else: + by_pid.setdefault(pid, set()).add(Path(target.removesuffix(" (deleted)")).name) + for pid in sorted(by_pid): + argv = _read_proc_argv(pid) # /proc only; macOS holders come from psutil + if argv is None and psutil is not None: + try: + argv = psutil.Process(pid).cmdline() or None + except Exception: + argv = None + who = " ".join([Path(argv[0]).name, *argv[1:]])[:80] if argv else "command line unavailable" + check_info(f"{name} is held by PID {pid} ({who}): {', '.join(sorted(by_pid[pid]))}") + if unknown: + check_warn(f"{name}: cannot prove the database is quiet", + f"(holder scan incomplete: {unknown[0][:120]}" + (f"; +{len(unknown) - 1} more" if len(unknown) > 1 else "") + ")") + elif not by_pid: + check_info(f"{name}: no other process holds it right now — the offline conversion can run") + + def _report_database_journal_modes(hermes_home: Path | None = None, version_info: tuple[int, ...] | None = None) -> None: """List each database's journal mode; warn on WAL under a vulnerable SQLite, and on a configured ``database.journal_mode: delete`` that never took effect.""" @@ -122,6 +153,7 @@ def _report_database_journal_modes(hermes_home: Path | None = None, version_info + ("; 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)") + _report_database_holders(name, path) elif error is not None: if vulnerable: check_warn(f"{name}: journal mode could not be read", f"({error}; cannot rule out WAL exposure)") diff --git a/tests/hermes_cli/test_doctor_journal_modes.py b/tests/hermes_cli/test_doctor_journal_modes.py index d0e19f3a90..b208a3a2ef 100644 --- a/tests/hermes_cli/test_doctor_journal_modes.py +++ b/tests/hermes_cli/test_doctor_journal_modes.py @@ -10,6 +10,8 @@ read-only engine open creates -wal/-shm sidecar files next to a WAL database. import os import re import sqlite3 +import subprocess +import sys import pytest @@ -461,6 +463,43 @@ class TestConfiguredDeleteNeverApplied: assert "state.db: WAL journal mode" not in out assert ("To clear the exposure:" in out) is exposed + @pytest.mark.skipif(sys.platform == "win32", reason="holder scan has no Windows backend") + def test_wal_db_under_configured_delete_names_its_holders(self, tmp_path, capsys, monkeypatch): + # The offline conversion needs the file quiet, so doctor must say WHICH process to stop — a + # subprocess holding a real connection is named by PID; the doctor process itself is not a holder. + db = tmp_path / "state.db" + _make_db(db, journal_mode="WAL") + monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "delete") + holder = subprocess.Popen( + [sys.executable, "-c", + "import sqlite3, sys; c = sqlite3.connect(sys.argv[1]); c.execute('SELECT count(*) FROM t'); " + "print('ready', flush=True); sys.stdin.readline()", str(db)], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True, + ) + try: + assert holder.stdout.readline().strip() == "ready" + doctor_platform._report_database_journal_modes(tmp_path, (3, 51, 3)) + finally: + holder.stdin.write("\n") + holder.stdin.flush() + holder.wait(timeout=30) + + out = capsys.readouterr().out + assert f"state.db is held by PID {holder.pid}" in out and "state.db" in out.split("held by PID")[1] + assert "no other process holds it" not in out and "cannot prove" not in out + + def test_partial_holder_scan_is_never_an_all_clear(self, tmp_path, capsys, monkeypatch): + _make_db(tmp_path / "state.db", journal_mode="WAL") + monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "delete") + monkeypatch.setattr("hermes_state_holders.foreign_state_db_holders", + lambda path: [(-1, "open-file scan unavailable")]) + + doctor_platform._report_database_journal_modes(tmp_path, (3, 51, 3)) + + out = capsys.readouterr().out + assert "cannot prove the database is quiet" in out and "open-file scan unavailable" in out + assert "no other process holds it" not in out and "held by PID" not in out + def test_configured_wal_keeps_the_informational_line(self, tmp_path, capsys, monkeypatch): _make_db(tmp_path / "state.db", journal_mode="WAL") monkeypatch.setattr("hermes_state_wal.resolve_journal_mode", lambda: "wal") diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 5b0e5bdda0..4ad47a51ce 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -127,7 +127,10 @@ 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. +`PRAGMA journal_mode=DELETE` on the file. 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. ## Environment Variable Substitution