diff --git a/hermes_state.py b/hermes_state.py index c795815164..6d6a4ddb62 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -3049,6 +3049,22 @@ def count_db_holders(db_path: Path) -> Optional[int]: return None +def _read_proc_cmdline(pid: int) -> Optional[str]: + """Read /proc//cmdline, world-readable even when fd table is not. + + Returns the cmdline as a space-joined string, or None when unreadable + (process exited, or hidepid mount). + """ + try: + with open(f"/proc/{pid}/cmdline", "rb") as f: + raw = f.read() + if not raw: + return None + return raw.replace(b"\x00", b" ").decode("utf-8", "replace").strip() + except OSError: + return None + + # Lifecycle statuses surfaced by session pickers. Classification looks ONLY at # a session's final message row — role, whether it carries tool_calls, and its # finish_reason — so it stays O(1) per session (see @@ -4217,9 +4233,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) Automatic FTS repair is structural maintenance, not an ordinary WAL write. It must not run while another process remains attached: a sidecar reset under that holder can leave the two processes writing - through different WAL inodes. ``psutil`` reads the kernel's open-file - table, including Linux ``(deleted)`` descriptors, so this also catches - a split brain already in progress. + through different WAL inodes. A scan failure is represented as an unknown holder. Skipping optional automatic maintenance is safer than assuming quiescence; canonical @@ -4245,20 +4259,71 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin) _canonical(db_path + "-shm"), } holders: List[Tuple[int, str]] = [] + + # On Linux, read /proc//fd symlinks directly. psutil's + # open_files() filters through isfile_strict(), which stats the + # literal path — for an unlinked WAL sidecar the kernel returns + # "/path/state.db-wal (deleted)" and stat fails, so the entry is + # silently dropped and the split-brain holder is never seen. + # /proc readlinks preserve the "(deleted)" suffix so _canonical can + # strip it and match. + if sys.platform.startswith("linux"): + try: + own_pid = os.getpid() + for pid_str in os.listdir("/proc"): + if not pid_str.isdigit(): + continue + pid = int(pid_str) + if pid == own_pid: + continue + fd_dir = f"/proc/{pid}/fd" + try: + fds = os.listdir(fd_dir) + except OSError: + # Cannot read this process's fd table (different + # user, e.g. root gateway vs user desktop). + # /proc//cmdline is world-readable by default, + # so check whether this is a Hermes process. + cmdline = _read_proc_cmdline(pid) + if cmdline is not None: + holders.append((pid, f"uninspectable holder: {cmdline[:80]}")) + continue + for fd in fds: + try: + target = os.readlink(f"{fd_dir}/{fd}") + except OSError: + continue + if _canonical(target) in watched: + holders.append((pid, target)) + except Exception as exc: + logger.warning( + "Could not prove state.db has no foreign holders; " + "deferring automatic FTS maintenance: %s", + exc, + ) + return holders or [(-1, f"open-file scan failed: {exc}")] + return holders + + # macOS / BSD: use psutil.open_files(). macOS does not use the + # "(deleted)" suffix convention, so psutil's filtering is safe here. try: for process in psutil.process_iter(["pid", "open_files"]): info = process.info pid = int(info["pid"]) if pid == os.getpid(): continue + # psutil's as_dict() converts AccessDenied to None, which + # or-() turns into an empty iteration. On macOS this is + # acceptable: the gateway/desktop topology from the issue is + # Linux-specific (systemd units running as root). for opened in info.get("open_files") or (): path = getattr(opened, "path", "") if path and _canonical(path) in watched: holders.append((pid, path)) except Exception as exc: logger.warning( - "Could not prove state.db has no foreign holders; deferring " - "automatic FTS maintenance: %s", + "Could not prove state.db has no foreign holders; " + "deferring automatic FTS maintenance: %s", exc, ) return holders or [(-1, f"open-file scan failed: {exc}")] diff --git a/tests/state/test_fts_runtime_rebuild.py b/tests/state/test_fts_runtime_rebuild.py index d8bc8fb887..fee735ebb4 100644 --- a/tests/state/test_fts_runtime_rebuild.py +++ b/tests/state/test_fts_runtime_rebuild.py @@ -13,6 +13,7 @@ sync triggers, and retries the canonical write. Search degrades to ``LIKE`` until a later open atomically rebuilds the index and restores the triggers. """ +import os import sqlite3 from types import SimpleNamespace @@ -122,11 +123,105 @@ class TestRuntimeFtsRebuild: monkeypatch.setattr(hermes_state, "psutil", FakePsutil) monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False) monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111) + # Force the macOS/psutil path even on Linux test runners + monkeypatch.setattr(hermes_state.sys, "platform", "darwin") assert db._foreign_state_db_holders() == [ (222, f"{db_path}-wal (deleted)") ] + def test_foreign_holder_detection_proc_readlink_deleted_wal( + self, db, tmp_path, monkeypatch + ): + """Linux /proc//fd readlinks preserve '(deleted)' suffix. + + psutil.open_files() drops these entries (isfile_strict stats the + literal path and fails). The /proc path catches the split-brain + holder that psutil silently misses. + """ + db_path = tmp_path / "state.db" + db_path_wal = str(db_path) + "-wal" + + # Build a fake /proc with two PIDs: self (111) and foreign (222). + proc_root = tmp_path / "proc" + for pid in (111, 222, 333): + fd_dir = proc_root / str(pid) / "fd" + fd_dir.mkdir(parents=True) + # PID 222 holds the deleted WAL sidecar + os.symlink(db_path_wal + " (deleted)", str(proc_root / "222" / "fd" / "3")) + # PID 111 (self) holds the db — should be excluded + os.symlink(str(db_path), str(proc_root / "111" / "fd" / "3")) + # PID 333 holds an unrelated file + other = tmp_path / "other.db" + other.touch() + os.symlink(str(other), str(proc_root / "333" / "fd" / "3")) + + monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False) + monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111) + monkeypatch.setattr(hermes_state.sys, "platform", "linux") + real_listdir = os.listdir + def _listdir(path): + if isinstance(path, str): + path = path.replace("/proc", str(proc_root)) + return real_listdir(path) + monkeypatch.setattr(hermes_state.os, "listdir", _listdir) + real_readlink = os.readlink + def _readlink(path): + path = path.replace("/proc", str(proc_root)) + return real_readlink(path) + monkeypatch.setattr(hermes_state.os, "readlink", _readlink) + + holders = db._foreign_state_db_holders() + assert holders == [(222, db_path_wal + " (deleted)")] + + def test_foreign_holder_uninspectable_process_cmdline_fallback( + self, db, tmp_path, monkeypatch + ): + """A process whose fd table is unreadable (different user) is still + flagged when /proc//cmdline identifies it as a Hermes process.""" + db_path = tmp_path / "state.db" + + proc_root = tmp_path / "proc" + for pid in (111, 222): + (proc_root / str(pid) / "fd").mkdir(parents=True) + # PID 222's fd dir is unreadable (PermissionError) + os.chmod(proc_root / "222" / "fd", 0o000) + # PID 222's cmdline is world-readable and looks like Hermes + cmdline_path = proc_root / "222" / "cmdline" + cmdline_path.write_bytes(b"python3\x00hermes_cli.main\x00chat\x00") + + monkeypatch.setattr(hermes_state, "_IS_WINDOWS", False) + monkeypatch.setattr(hermes_state.os, "getpid", lambda: 111) + monkeypatch.setattr(hermes_state.sys, "platform", "linux") + real_listdir = os.listdir + def _listdir(path): + if isinstance(path, str): + path = path.replace("/proc", str(proc_root)) + return real_listdir(path) + monkeypatch.setattr(hermes_state.os, "listdir", _listdir) + # _read_proc_cmdline opens /proc//cmdline directly; redirect + # it to our fake proc tree. + def _fake_cmdline(pid): + fake_path = str(proc_root / str(pid) / "cmdline") + try: + with open(fake_path, "rb") as f: + raw = f.read() + if not raw: + return None + return raw.replace(b"\x00", b" ").decode("utf-8", "replace").strip() + except OSError: + return None + monkeypatch.setattr(hermes_state, "_read_proc_cmdline", _fake_cmdline) + + holders = db._foreign_state_db_holders() + # Should include PID 222 with the cmdline info + assert len(holders) == 1 + assert holders[0][0] == 222 + assert "hermes_cli.main" in holders[0][1] + + # Cleanup + os.chmod(proc_root / "222" / "fd", 0o755) + def test_corruption_error_classification_covers_both_sqlite_messages(self): """SQLite's message for a corrupt FTS index varies by version: older builds raise the generic malformed-image error, newer builds raise an