From 25e56908448582f003af4e8a73b64b79b6b4a84d Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:56:15 +0530 Subject: [PATCH] refactor(doctor): share the holder pid renderer and mirror the guard's remedy doctor_platform already rendered "PID N (cmd)" inline for the journal-mode holder report; the retired-WAL warning needs the same text, so the loop moves to hermes_state_holders.describe_holder_pid and both sites call it. The issue line now names the same writers the DeletedWalGenerationError message names (gateway, dashboard, cron) and repeats "do not delete the WAL yourself", so doctor and the guard tell one story. The test also spies _state_db_stats, the probe that printed the literal "0 process(es) holding the DB open" line the issue is about. --- hermes_cli/doctor_platform.py | 11 ++--------- hermes_cli/doctor_state.py | 9 +++++---- hermes_state_holders.py | 12 ++++++++++++ tests/hermes_cli/test_doctor_wal_holder_guard.py | 1 + 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/hermes_cli/doctor_platform.py b/hermes_cli/doctor_platform.py index 33fb7fccad..bbf84c7591 100644 --- a/hermes_cli/doctor_platform.py +++ b/hermes_cli/doctor_platform.py @@ -93,7 +93,7 @@ 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 + from hermes_state_holders import describe_holder_pid, foreign_state_db_holders if sys.platform == "win32": check_warn(f"{name}: cannot prove the database is quiet", "(holder scan is unavailable on Windows)") return @@ -105,14 +105,7 @@ def _report_database_holders(name: str, db_path: Path) -> None: 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]))}") + check_info(f"{name} is held by {describe_holder_pid(pid)}: {', '.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 "") + ")") diff --git a/hermes_cli/doctor_state.py b/hermes_cli/doctor_state.py index 0a038c64de..0df11bb5fe 100644 --- a/hermes_cli/doctor_state.py +++ b/hermes_cli/doctor_state.py @@ -350,15 +350,16 @@ def _retired_wal_holders(f: Finding, state_db_path: Path, _DHH: str) -> bool: "0 holding the DB open" beside a green state.db line — the opposite of the truth.""" from hermes_constants import profile_cli_selector from hermes_state_dbfile import iter_deleted_sqlite_sidecar_holders + from hermes_state_holders import describe_holder_pid pids = list(dict.fromkeys(pid for pid, _ in iter_deleted_sqlite_sidecar_holders(state_db_path))) if not pids: return False - rendered = ", ".join(f"pid {pid}" for pid in pids) + rendered = ", ".join(describe_holder_pid(pid) for pid in pids) check_warn(f"{_DHH}/state.db: {len(pids)} process(es) still hold a retired WAL generation ({rendered})", "(every new session refuses to open until they exit; health/stats probes skipped)") - f.issues.append(f"state.db retired WAL generation held by {rendered} — stop them: " - f"'hermes {profile_cli_selector()}gateway stop', quit the Desktop app / dashboard, " - "then rerun 'hermes doctor'") + f.issues.append(f"state.db retired WAL generation held by {rendered} — stop the gateway, dashboard and " + f"cron writers among them ('hermes {profile_cli_selector()}gateway stop', quit the Desktop " + "app), do not delete the WAL yourself, then rerun 'hermes doctor'") return True diff --git a/hermes_state_holders.py b/hermes_state_holders.py index 794dffa301..1f1fe4c246 100644 --- a/hermes_state_holders.py +++ b/hermes_state_holders.py @@ -54,6 +54,18 @@ def _read_proc_argv(pid: int) -> Optional[List[str]]: return None +def describe_holder_pid(pid: int) -> str: + """``PID 123 (hermes gateway run)`` for operator-facing holder lists; /proc argv first, psutil elsewhere.""" + argv = _read_proc_argv(pid) + if argv is None and psutil is not None: + try: + argv = psutil.Process(pid).cmdline() or None + except Exception: + argv = None + who = " ".join(" ".join([os.path.basename(argv[0]), *argv[1:]]).split())[:80] if argv else "command line unavailable" + return f"PID {pid} ({who})" + + def _looks_like_python_executable(program: str) -> bool: name = os.path.basename(program).lower().removesuffix(".exe") for prefix in ("python", "pypy"): diff --git a/tests/hermes_cli/test_doctor_wal_holder_guard.py b/tests/hermes_cli/test_doctor_wal_holder_guard.py index a66f0b0d38..6007c142ff 100644 --- a/tests/hermes_cli/test_doctor_wal_holder_guard.py +++ b/tests/hermes_cli/test_doctor_wal_holder_guard.py @@ -66,6 +66,7 @@ def test_doctor_names_retired_wal_holders_instead_of_healthy_state_db(tmp_path, lambda path: [(4242, f"{path}-wal"), (4242, f"{path}-shm")]) probed = [] monkeypatch.setattr(doctor_state, "_state_db_health", lambda *a, **k: probed.append(a)) + monkeypatch.setattr(doctor_state, "_state_db_stats", lambda *a, **k: probed.append(a)) monkeypatch.setattr(doctor_state, "_state_db_wal", lambda *a, **k: probed.append(a)) finding = doctor_state._check_state_db(True)