diff --git a/hermes_cli/dashboard_procs.py b/hermes_cli/dashboard_procs.py index 35a047f6f1..84bd48573e 100644 --- a/hermes_cli/dashboard_procs.py +++ b/hermes_cli/dashboard_procs.py @@ -144,6 +144,153 @@ def _scan_dashboard_processes( ] return dashboard_processes + +def _hermes_home_for_pid(pid: int) -> str | None: + """Best-effort ``HERMES_HOME`` from *pid*'s environment.""" + try: + import psutil + + home = psutil.Process(pid).environ().get("HERMES_HOME") + if home: + return home + except Exception: + pass + try: + raw = Path(f"/proc/{pid}/environ").read_bytes() + except (OSError, PermissionError): + return None + for part in raw.split(b"\x00"): + if part.startswith(b"HERMES_HOME="): + return part.split(b"=", 1)[1].decode("utf-8", errors="replace") or None + return None + + +def _is_ephemeral_port_zero_backend(argv: list[str]) -> bool: + """True for Desktop-style ``serve|dashboard --port 0`` backends (#78821). + + Ephemeral-port backends are owned by Hermes Desktop (or become PPID-1 + orphans after a prior update respawn). Replaying them after + ``hermes update`` multiplies listening backends because ``--port 0`` + always binds a fresh free port. Covers both ``serve`` and the legacy + ``dashboard --no-open`` fallback older Desktop runtimes use. + """ + if _dashboard_subcommand_index(argv) is None: + return False + for i, tok in enumerate(argv): + if tok == "--port" and i + 1 < len(argv) and str(argv[i + 1]) == "0": + return True + if tok.startswith("--port=") and tok.split("=", 1)[1].strip() == "0": + return True + return False + + +def _dashboard_subcommand_index(argv: list[str]) -> int | None: + for i, tok in enumerate(argv): + if tok in ("serve", "dashboard"): + return i + return None + + +def _normalize_dashboard_cmdline(argv: list[str]) -> tuple[str, ...]: + """Collapse argv to profile flags + serve/dashboard tail for dedupe.""" + idx = _dashboard_subcommand_index(argv) + if idx is None: + return tuple(argv) + prefix: list[str] = [] + i = 0 + while i < idx: + tok = argv[i] + if tok in ("--profile", "-p") and i + 1 < idx: + prefix.extend([tok, argv[i + 1]]) + i += 2 + continue + if tok.startswith("--profile="): + prefix.append(tok) + i += 1 + return tuple(prefix + list(argv[idx:])) + + +def _profile_key_for_respawn( + argv: list[str], hermes_home: str | None = None +) -> str: + """Stable owner key: ``HERMES_HOME`` when known, else ``--profile`` / ``-p``. + + ``HERMES_HOME`` ending in ``profiles/`` is normalized to + ``profile:`` so it shares a cap with an explicit ``--profile`` + flag for the same profile (#78821). Non-profile homes (including + distinct ``…/.hermes`` roots) keep a resolved ``home:`` key so + unrelated installs do not collapse together. + """ + profile_name: str | None = None + for i, tok in enumerate(argv): + if tok in ("--profile", "-p") and i + 1 < len(argv): + profile_name = argv[i + 1] + break + if tok.startswith("--profile="): + profile_name = tok.split("=", 1)[1] + break + + if hermes_home: + try: + home_path = Path(hermes_home).resolve() + except (OSError, RuntimeError, ValueError): + home_path = Path(hermes_home) + parts = home_path.parts + if len(parts) >= 2 and parts[-2] == "profiles" and parts[-1]: + return f"profile:{parts[-1]}" + try: + return f"home:{os.path.normcase(str(home_path))}" + except (OSError, RuntimeError, ValueError): + return f"home:{os.path.normcase(hermes_home)}" + + if profile_name: + return f"profile:{profile_name}" + return "profile:default" + + +def _filter_dashboard_respawn_candidates( + candidates: list[tuple[int, list[str], str | None]], +) -> list[list[str]]: + """Select which killed manual backends to respawn after ``hermes update``. + + Each candidate is ``(pid, argv, hermes_home)``. + + Rules (#78821): + 1. Never resurrect Desktop ephemeral ``serve|dashboard --port 0`` + backends — Desktop (``HERMES_DESKTOP_CHILD_PID``) owns their + lifecycle. These are also the PPID-1 orphans that previously + multiplied across updates because ``--port 0`` always binds a + fresh free port. + 2. Dedupe by normalized cmdline (identical argv → one respawn). + 3. Cap at most one managed backend per profile / ``HERMES_HOME``. + + Intentionally does **not** blanket-skip every PPID-1 process: a prior + ``hermes update`` respawn detaches with ``start_new_session=True``, so + fixed-port manual backends are reparented to init and must still be + eligible for the next update's #40449 restart. + """ + selected: list[list[str]] = [] + seen_cmdlines: set[tuple[str, ...]] = set() + seen_profiles: set[str] = set() + + for _pid, argv, hermes_home in candidates: + if not argv: + continue + if _is_ephemeral_port_zero_backend(argv): + continue + norm = _normalize_dashboard_cmdline(argv) + if norm in seen_cmdlines: + continue + profile_key = _profile_key_for_respawn(argv, hermes_home) + if profile_key in seen_profiles: + continue + seen_cmdlines.add(norm) + seen_profiles.add(profile_key) + selected.append(list(argv)) + + return selected + + def _kill_stale_dashboard_processes( reason: str = "the running backend no longer matches the updated frontend", *, @@ -210,6 +357,7 @@ def _kill_stale_dashboard_processes( pid_cgroup: dict[int, str | None] = {} pid_service: dict[int, str | None] = {} pid_cmdline: dict[int, list[str]] = {} + pid_home: dict[int, str | None] = {} if restart_managed and sys.platform != "win32": for pid in pids: cg_path = _m()._get_pid_cgroup_path(pid) @@ -218,9 +366,12 @@ def _kill_stale_dashboard_processes( if not pid_service[pid]: # Manually-started process: preserve its exact argv so we # can respawn it after the update (#40449, #68934). + # Snapshot HERMES_HOME before the kill so per-profile caps + # still work after the process is gone (#78821). cmdline = _m()._dashboard_cmdline_for_pid(pid) if cmdline: pid_cmdline[pid] = cmdline + pid_home[pid] = _hermes_home_for_pid(pid) killed: list[int] = [] failed: list[tuple[int, str]] = [] @@ -294,12 +445,14 @@ def _kill_stale_dashboard_processes( # back after our clean SIGTERM, and the Desktop can't reconnect (#68934). # - manually-started PIDs: respawn the argv captured before the kill # (#40449) — detached, headless, logged to logs/dashboard-restart.log. + # Filtered so Desktop ``serve|dashboard --port 0`` backends are not + # resurrected and duplicates collapse to one per profile (#78821). restarted_services: list[str] = [] unrecovered: list[int] = [] if killed and restart_managed: failed_restarts: list[tuple[str, str]] = [] seen_services: set[str] = set() - respawn_cmds: list[list[str]] = [] + respawn_candidates: list[tuple[int, list[str], str | None]] = [] for pid in killed: svc_name = pid_service.get(pid) if svc_name: @@ -312,7 +465,9 @@ def _kill_stale_dashboard_processes( failed_restarts.append((svc_name, "systemctl restart returned non-zero")) unrecovered.append(pid) elif pid in pid_cmdline: - respawn_cmds.append(pid_cmdline[pid]) + respawn_candidates.append( + (pid, pid_cmdline[pid], pid_home.get(pid)) + ) else: unrecovered.append(pid) @@ -321,6 +476,7 @@ def _kill_stale_dashboard_processes( for svc, err in failed_restarts: print(f" ⚠ {svc}: {err}") + respawn_cmds = _filter_dashboard_respawn_candidates(respawn_candidates) if respawn_cmds: failed_cmds = _m()._respawn_dashboard_processes(respawn_cmds) if failed_cmds: diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 58eac00527..22ea6e5972 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7561,6 +7561,7 @@ def cmd_gui(args: argparse.Namespace): # monkeypatches on hermes_cli.main. keep resolving unchanged. from hermes_cli.dashboard_procs import ( # noqa: F401 _detect_concurrent_hermes_instances, + _filter_dashboard_respawn_candidates, _kill_stale_dashboard_processes, _scan_dashboard_processes, ) @@ -7889,6 +7890,10 @@ def _respawn_dashboard_processes(commands: list[list[str]]) -> list[list[str]]: Spawns each recovered argv detached (new session, output to the profile's ``logs/dashboard-restart.log``). Returns the commands that failed to spawn; the caller prints the manual hint for those. + + Callers must pre-filter via ``_filter_dashboard_respawn_candidates`` so + Desktop ``serve|dashboard --port 0`` backends are not replayed and + duplicates are capped per profile (#78821). """ from hermes_constants import get_hermes_home diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index 547b4f887a..c1caf89815 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -91,6 +91,7 @@ class TestFindStaleDashboardPids: + @pytest.mark.skipif(sys.platform == "win32", reason="ps-based scan path") def test_self_pid_excluded(self): with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock( @@ -289,6 +290,7 @@ class TestWindowsWmicEncoding: ) +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX kill + systemd restart") class TestSupervisedBackendRestart: """After the kill, systemd-supervised PIDs get their owning unit restarted (#68934) — SIGTERM reads as a clean stop to systemd, so @@ -333,6 +335,7 @@ class TestManualBackendRespawn: return sys.modules["hermes_cli.main"] + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX cmdline capture + respawn") def test_argv_capture_failure_falls_back_to_hint(self, capsys): live = self._live() @@ -354,6 +357,85 @@ class TestManualBackendRespawn: out = capsys.readouterr().out assert "Restart anything not auto-restarted" in out + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX cmdline capture + respawn") + def test_non_orphan_fixed_port_still_respawns(self, capsys): + """A supervised-by-shell dashboard with a fixed port is still restarted.""" + live = self._live() + argv = ["hermes", "dashboard", "--port", "8300"] + + def fake_kill(pid, sig): + if sig == 0: + raise ProcessLookupError + + with patch.object(live, "_restart_managed_dashboard_service", return_value=False), \ + patch.object(live, "_find_stale_dashboard_pids", return_value=[6001]), \ + patch.object(live, "_get_pid_cgroup_path", return_value=None), \ + patch.object(live, "_get_systemd_service_for_pid", return_value=None), \ + patch.object(live, "_dashboard_cmdline_for_pid", return_value=argv), \ + patch("hermes_cli.dashboard_procs._hermes_home_for_pid", return_value=None), \ + patch.object(live, "_respawn_dashboard_processes", return_value=[]) as respawn, \ + patch("os.kill", side_effect=fake_kill), \ + patch("time.sleep"): + _kill_stale_dashboard_processes(restart_managed=True) + + respawn.assert_called_once_with([argv]) + assert "when you're ready" not in capsys.readouterr().out + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX cmdline capture + respawn") + def test_port_zero_serves_killed_without_respawn(self, capsys): + """``serve --port 0`` backends are stopped but not resurrected (#78821).""" + live = self._live() + argv = [ + "python", "-m", "hermes_cli.main", + "serve", "--host", "127.0.0.1", "--port", "0", + ] + + def fake_kill(pid, sig): + if sig == 0: + raise ProcessLookupError + + with patch.object(live, "_restart_managed_dashboard_service", return_value=False), \ + patch.object(live, "_find_stale_dashboard_pids", + return_value=[7001, 7002, 7003]), \ + patch.object(live, "_get_pid_cgroup_path", return_value=None), \ + patch.object(live, "_get_systemd_service_for_pid", return_value=None), \ + patch.object(live, "_dashboard_cmdline_for_pid", return_value=argv), \ + patch("hermes_cli.dashboard_procs._hermes_home_for_pid", return_value=None), \ + patch.object(live, "_respawn_dashboard_processes") as respawn, \ + patch("os.kill", side_effect=fake_kill), \ + patch("time.sleep"): + result = _kill_stale_dashboard_processes(restart_managed=True) + + respawn.assert_not_called() + assert sorted(result["killed"]) == [7001, 7002, 7003] + # Intentional skips are not "unrecovered" — no noisy manual hint. + assert result["unrecovered"] == [] + assert "when you're ready" not in capsys.readouterr().out + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX cmdline capture + respawn") + def test_detached_fixed_port_still_respawns_after_prior_update(self, capsys): + """PPID-1 fixed-port backends (prior start_new_session respawn) stay eligible.""" + live = self._live() + argv = ["hermes", "dashboard", "--port", "8300"] + + def fake_kill(pid, sig): + if sig == 0: + raise ProcessLookupError + + with patch.object(live, "_restart_managed_dashboard_service", return_value=False), \ + patch.object(live, "_find_stale_dashboard_pids", return_value=[8001]), \ + patch.object(live, "_get_pid_cgroup_path", return_value=None), \ + patch.object(live, "_get_systemd_service_for_pid", return_value=None), \ + patch.object(live, "_dashboard_cmdline_for_pid", return_value=argv), \ + patch("hermes_cli.dashboard_procs._hermes_home_for_pid", return_value=None), \ + patch.object(live, "_respawn_dashboard_processes", return_value=[]) as respawn, \ + patch("os.kill", side_effect=fake_kill), \ + patch("time.sleep"): + _kill_stale_dashboard_processes(restart_managed=True) + + respawn.assert_called_once_with([argv]) + assert "when you're ready" not in capsys.readouterr().out + def test_respawn_adds_no_open_to_dashboard_commands(self, tmp_path, monkeypatch): """Respawned `dashboard` argv gains --no-open; `serve` argv untouched.""" live = self._live() @@ -386,12 +468,140 @@ class TestManualBackendRespawn: assert "✗ failed to restart" in out +class TestFilterDashboardRespawnCandidates: + """Unit tests for respawn filtering / dedupe / orphan skip (#78821).""" + + def test_skips_serve_port_zero(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = [ + "python", "-m", "hermes_cli.main", + "--profile", "mini-cat", + "serve", "--host", "127.0.0.1", "--port", "0", + ] + assert _filter_dashboard_respawn_candidates([ + (42, argv, "/home/u/.hermes/profiles/mini-cat"), + ]) == [] + + def test_skips_legacy_dashboard_port_zero(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = [ + "hermes", "--profile", "coder", + "dashboard", "--no-open", "--host", "127.0.0.1", "--port", "0", + ] + assert _filter_dashboard_respawn_candidates([(7, argv, None)]) == [] + + def test_skips_serve_port_equals_zero(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = ["hermes", "serve", "--port=0"] + assert _filter_dashboard_respawn_candidates([(1, argv, None)]) == [] + + def test_keeps_ppid1_fixed_port_for_repeat_update(self): + """Detached prior-update respawns (PPID 1) must remain restartable (#40449).""" + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = ["hermes", "dashboard", "--port", "9119"] + assert _filter_dashboard_respawn_candidates([(10, argv, None)]) == [argv] + + def test_dedupes_identical_normalized_cmdlines(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + a = ["/usr/bin/python3", "-m", "hermes_cli.main", "dashboard", "--port", "8300"] + b = ["/other/python", "-m", "hermes_cli.main", "dashboard", "--port", "8300"] + out = _filter_dashboard_respawn_candidates([ + (1, a, None), + (2, b, None), + ]) + assert out == [a] + + def test_caps_one_per_profile(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + a = ["hermes", "--profile", "coder", "dashboard", "--port", "8300"] + b = ["hermes", "--profile", "coder", "dashboard", "--port", "8301"] + c = ["hermes", "--profile", "writer", "dashboard", "--port", "8302"] + out = _filter_dashboard_respawn_candidates([ + (1, a, None), + (2, b, None), + (3, c, None), + ]) + assert out == [a, c] + + def test_caps_one_per_hermes_home(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + home = "/tmp/hermes-home-a" + a = ["hermes", "dashboard", "--port", "8300"] + b = ["hermes", "dashboard", "--port", "8301"] + out = _filter_dashboard_respawn_candidates([ + (1, a, home), + (2, b, home), + ]) + assert out == [a] + + def test_profile_flag_and_profiles_home_share_cap(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + a = ["hermes", "--profile", "coder", "dashboard", "--port", "8300"] + b = ["hermes", "dashboard", "--port", "8301"] + out = _filter_dashboard_respawn_candidates([ + (1, a, None), + (2, b, "/home/u/.hermes/profiles/coder"), + ]) + assert out == [a] + + def test_default_profile_same_root_home_caps(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + a = ["hermes", "--profile", "default", "dashboard", "--port", "8300"] + b = ["hermes", "dashboard", "--port", "8301"] + home = "/home/u/.hermes" + out = _filter_dashboard_respawn_candidates([ + (1, a, home), + (2, b, home), + ]) + assert out == [a] + + def test_distinct_dot_hermes_homes_do_not_share_cap(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + a = ["hermes", "dashboard", "--port", "8300"] + b = ["hermes", "dashboard", "--port", "8301"] + out = _filter_dashboard_respawn_candidates([ + (1, a, "/home/u/.hermes"), + (2, b, "/work/project/.hermes"), + ]) + assert out == [a, b] + + def test_keeps_fixed_port_serve(self): + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = ["hermes", "serve", "--host", "0.0.0.0", "--port", "9119"] + assert _filter_dashboard_respawn_candidates([ + (9, argv, None), + ]) == [argv] + + def test_seventeen_port_zero_orphans_collapse_to_zero(self): + """The reported accumulation case: many identical serve --port 0 → none.""" + from hermes_cli.dashboard_procs import _filter_dashboard_respawn_candidates + + argv = [ + "python", "-m", "hermes_cli.main", + "serve", "--host", "127.0.0.1", "--port", "0", + ] + candidates = [(i, argv, None) for i in range(17)] + assert _filter_dashboard_respawn_candidates(candidates) == [] + + class TestCmdlineCapture: """_dashboard_cmdline_for_pid reads /proc on Linux, ps on macOS.""" def _live(self): return sys.modules["hermes_cli.main"] + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX /proc cmdline path") def test_reads_proc_cmdline_when_available(self, tmp_path, monkeypatch): live = self._live() proc_file = tmp_path / "cmdline" @@ -417,6 +627,7 @@ class TestCmdlineCapture: assert argv == ["/usr/bin/python3", "-m", "hermes_cli.main", "serve"] + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX ps cmdline fallback") def test_falls_back_to_ps_without_proc(self, monkeypatch): live = self._live()