From 5dc05faf7990cc25b5697c417efd586ccd73fffb Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 27 Sep 2026 00:53:56 -0700 Subject: [PATCH] fix(update): restart a dashboard's systemd unit only when the unit's MainPID is the dashboard A .service cgroup only says where a process was started. A dashboard started by hand from a shell that runs under some unit (a CI runner agent, cron.service, a tmux or IDE user service, the gateway's terminal tool) sits in that unit's cgroup, and hermes update restarted that unrelated unit instead of respawning the dashboard, leaving it down. _get_systemd_service_for_pid now claims the unit only when its live MainPID is the PID; otherwise the argv respawn path runs, which is what the update plan already predicted (manual-serve / respawn-argv). --- hermes_cli/main_dashboard.py | 26 ++++++++++-- .../hermes_cli/test_update_stale_dashboard.py | 41 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/hermes_cli/main_dashboard.py b/hermes_cli/main_dashboard.py index c3067c721d..150bb117f3 100644 --- a/hermes_cli/main_dashboard.py +++ b/hermes_cli/main_dashboard.py @@ -184,18 +184,38 @@ def _pid_unified_cgroup_entries(pid: int): def _get_systemd_service_for_pid(pid: int) -> str | None: - """The systemd service unit name *pid* belongs to (``hermes-serve.service``), or None. + """The systemd service unit that supervises *pid* (``hermes-serve.service``), or None. - None when the PID isn't part of a service, the file is unreadable, or off Linux. + A ``.service`` cgroup alone only says where the process was started: a dashboard launched by + hand from a shell that itself runs under some unit (a CI runner agent, ``cron.service``, a + tmux or IDE user service, the gateway's own terminal tool) sits in THAT unit's cgroup. The + unit owns the backend only when its live ``MainPID`` is this PID; otherwise restarting it + restarts an unrelated service and leaves the dashboard down. None when the PID isn't part of + a service, ownership can't be proved, the file is unreadable, or off Linux. """ for cg_path in _pid_unified_cgroup_entries(pid): if cg_path.endswith(".service"): svc_name = cg_path.rsplit("/", 1)[-1] - if svc_name: + if svc_name and _unit_main_pid_is(svc_name, cg_path, pid): return svc_name return None +def _unit_main_pid_is(svc_name: str, cgroup_path: str, pid: int) -> bool: + """True when *svc_name*'s live ``MainPID`` is *pid* (read-only ``systemctl show``).""" + scope = _extract_scope_from_cgroup(cgroup_path) + scopes = {"user": [["--user"]], "system": [[]]}.get(scope or "", [[], ["--user"]]) + for scope_args in scopes: + try: + result = _run_probe( + ["systemctl", *scope_args, "show", svc_name, "--property=MainPID", "--value"], timeout=10) + except _SYSTEMCTL_ERRORS: + continue + if result.returncode == 0 and (result.stdout or "").strip() == str(pid): + return True + return False + + def _extract_scope_from_cgroup(cgroup_entry: str) -> str | None: """``user`` / ``system`` from a cgroup path (``/user.slice/…`` vs ``/system.slice/…``), else None.""" if "/system.slice/" in cgroup_entry: diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index 27f137c973..92ec16e3b7 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -359,6 +359,47 @@ class TestSupervisedBackendRestart: restart.assert_not_called() assert result == {"matched": [], "killed": [], "failed": []} + @pytest.mark.parametrize("main_pid, restarted", [("991", False), ("4321", True)], + ids=["foreign-unit-cgroup", "unit-main-process"]) + def test_only_the_unit_whose_main_process_is_the_backend_is_restarted(self, main_pid, restarted): + """A dashboard started by hand from a shell inside some unit (CI runner agent, cron, tmux, + the gateway's terminal tool) sits in that unit's cgroup. Only a unit whose MainPID IS the + backend supervises it; any other unit is not restarted and the backend is respawned from + its argv instead.""" + live = self._live() + argv = ["hermes", "dashboard", "--port", "8300"] + unit_cgroup = "/system.slice/hosted-compute-agent.service" + probes: list[list[str]] = [] + + def fake_probe(cmd, *, timeout): + probes.append(list(cmd)) + out = main_pid if cmd[-2:] == ["--property=MainPID", "--value"] else "" + return MagicMock(returncode=0, stdout=out, stderr="") + + def fake_kill(pid, sig): + if sig == 0: + raise ProcessLookupError + + with patch.object(main_dashboard, "_restart_managed_dashboard_service", return_value=False), \ + patch.object(live, "_find_stale_dashboard_pids", return_value=[4321]), \ + patch.object(main_dashboard, "_pid_unified_cgroup_entries", lambda pid: iter([unit_cgroup])), \ + patch.object(main_dashboard, "_run_probe", side_effect=fake_probe), \ + patch.object(main_dashboard, "_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"): + result = _kill_stale_dashboard_processes(restart_managed=True) + + restarts = [c for c in probes if "restart" in c] + if restarted: + assert restarts == [["systemctl", "restart", "hosted-compute-agent.service"]] + respawn.assert_not_called() + else: + assert restarts == [], f"restarted a unit that does not supervise the dashboard: {restarts}" + respawn.assert_called_once_with([argv]) + assert result["unrecovered"] == [] + class TestManualBackendRespawn: """Manually-started dashboards/serves have their argv captured before the