From 9b419a2d3c2657c192008e732149d61170b32c01 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:30:49 -0700 Subject: [PATCH] fix(state): holder scan resolves symlinked homes; doctor's ro URI escapes reserved chars Review findings on #110914 (@ehz0ah): the psutil leg compared watched abspath against the kernel-resolved path psutil reports, so a symlinked HERMES_HOME on macOS returned no holders and let the fallback probe mint replacement sidecars under a live writer. Both sides now realpath. doctor's `file:{path}?mode=ro` truncated at '?'/'#' in a home name; build the URI with as_uri(). --- hermes_cli/doctor_state.py | 3 ++- hermes_state_holders.py | 6 +++-- .../test_doctor_wal_checkpoint_guard.py | 17 ++++++++++++ ...est_state_db_second_process_maintenance.py | 27 +++++++++++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/hermes_cli/doctor_state.py b/hermes_cli/doctor_state.py index abf9408221..1a3397b3a2 100644 --- a/hermes_cli/doctor_state.py +++ b/hermes_cli/doctor_state.py @@ -151,7 +151,8 @@ def _check_directory_structure(should_fix: bool, f: Finding) -> None: def _session_count(state_db_path: Path): import sqlite3 # mode=ro: doctor is a reader; a writable open of a gateway-held WAL DB is the second-writer class (#103339). - conn = sqlite3.connect(f"file:{state_db_path}?mode=ro", uri=True) + # as_uri() percent-encodes '?' / '#' in the home path; a raw f-string URI truncates there. + conn = sqlite3.connect(Path(state_db_path).resolve().as_uri() + "?mode=ro", uri=True) try: return conn.execute("SELECT COUNT(*) FROM sessions").fetchone()[0] finally: diff --git a/hermes_state_holders.py b/hermes_state_holders.py index 1a69807f2b..ab7b84bc20 100644 --- a/hermes_state_holders.py +++ b/hermes_state_holders.py @@ -188,7 +188,9 @@ def foreign_state_db_holders(db_path: Path) -> List[Tuple[int, str]]: if _IS_WINDOWS: return [] - db_path_str = os.path.abspath(os.fspath(db_path)) + # realpath, not abspath: psutil/libproc report the kernel-resolved pathname, so a symlinked + # HERMES_HOME would otherwise make every holder invisible and let maintenance proceed. + db_path_str = os.path.realpath(os.fspath(db_path)) watched = { canonical_sqlite_path(db_path_str), canonical_sqlite_path(db_path_str + "-wal"), @@ -303,7 +305,7 @@ def foreign_state_db_holders(db_path: Path) -> List[Tuple[int, str]]: continue for opened in info.get("open_files") or (): path = getattr(opened, "path", "") - if path and canonical_sqlite_path(path) in watched: + if path and canonical_sqlite_path(os.path.realpath(path)) in watched: holders.append((pid, path)) except Exception as exc: logger.warning( diff --git a/tests/hermes_cli/test_doctor_wal_checkpoint_guard.py b/tests/hermes_cli/test_doctor_wal_checkpoint_guard.py index 297b92719c..4d9192d534 100644 --- a/tests/hermes_cli/test_doctor_wal_checkpoint_guard.py +++ b/tests/hermes_cli/test_doctor_wal_checkpoint_guard.py @@ -47,3 +47,20 @@ def test_doctor_checkpoint_runs_only_on_the_exclusive_repair_guard(tmp_path, mon assert finding.fixed == 1 and not finding.issues # Every writable open went through the repair connector (probe + exclusive guard); none was a bare connect. assert bare_connects and len(bare_connects) == len(guard_connects) >= 2 + + +def test_session_count_reads_a_home_with_uri_reserved_characters(tmp_path): + """`file:` URIs treat '?' and '#' as delimiters; a home named `profile?blue` must still count.""" + import sqlite3 + + from hermes_cli.doctor_state import _session_count + + home = tmp_path / "profile?blue#x" + home.mkdir() + db = home / "state.db" + conn = sqlite3.connect(db) + conn.execute("CREATE TABLE sessions(id TEXT)") + conn.execute("INSERT INTO sessions VALUES ('a'), ('b')") + conn.commit() + conn.close() + assert _session_count(db) == 2 diff --git a/tests/hermes_state/test_state_db_second_process_maintenance.py b/tests/hermes_state/test_state_db_second_process_maintenance.py index cfc704e091..947eb8a147 100644 --- a/tests/hermes_state/test_state_db_second_process_maintenance.py +++ b/tests/hermes_state/test_state_db_second_process_maintenance.py @@ -56,3 +56,30 @@ def test_repair_refuses_delete_mode_db_held_open_by_another_process(delete_mode_ holder.wait(timeout=10) assert report["repaired"] is False assert "stop the gateway" in (report["error"] or "").lower() + + +def test_holder_scan_sees_through_a_symlinked_home(tmp_path): + """psutil/libproc report the resolved pathname; a holder opened via the real path must be found + when the scan is asked about the alias, or maintenance proceeds under a live writer. The Linux + /proc leg already compares inodes; the textual psutil leg (macOS) is the one this pins.""" + import sqlite3 + + from hermes_state_holders import foreign_state_db_holders + + real = tmp_path / "real-home" + real.mkdir() + alias = tmp_path / "alias-home" + alias.symlink_to(real, target_is_directory=True) + db = real / "state.db" + sqlite3.connect(db).execute("CREATE TABLE t(x)").connection.close() + holder = subprocess.Popen( + [sys.executable, "-c", + f"import sqlite3, sys, time; c = sqlite3.connect({str(db)!r}); c.execute('BEGIN IMMEDIATE'); " + "print('held', flush=True); time.sleep(30)"], + stdout=subprocess.PIPE, text=True) + try: + assert holder.stdout.readline().strip() == "held" + assert any(pid == holder.pid for pid, _ in foreign_state_db_holders(alias / "state.db")) + finally: + holder.kill() + holder.wait()