Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 132 additions & 20 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5929,11 +5929,10 @@ def _find_stale_dashboard_pids(
disk is updated, causing a silent frontend/backend mismatch (e.g. new
auth headers the old backend doesn't recognise → every API call 401s).

The dashboard has no service manager (systemd / launchd), no PID file,
and we can't know the original launch args — so the only sane action
after an update is to kill the stale process and let the user restart
it. This helper is just the detection step; see
``_kill_stale_dashboard_processes`` for the kill.
The dashboard may be manually started or managed by the optional
``hermes-dashboard.service`` systemd unit. Managed units are restarted
through their owning systemd scope; only manually-started processes use
the kill path because we can't know their original launch args.

*exclude_pids* is an optional set of PIDs that must never be returned.
This is used by the Hermes Desktop Electron app to protect its own
Expand Down Expand Up @@ -6163,8 +6162,121 @@ def _format_time_ago(iso_ts: str) -> str:
return "recently"


_DASHBOARD_SYSTEMD_UNIT = "hermes-dashboard.service"


def _restart_managed_dashboard_service(
reason: str,
unit: str = _DASHBOARD_SYSTEMD_UNIT,
) -> bool:
"""Restart a systemd-managed dashboard instead of raw-killing its PID.

Returns True when a dashboard unit was found and handled (successfully or
with a printed actionable failure). Returning True deliberately prevents
the caller from falling back to ``os.kill``: systemd treats a direct
SIGTERM of the service's main PID as a clean stop, so ``Restart=on-failure``
will not bring the dashboard back.
"""
if sys.platform == "win32":
return False

def _systemctl(*args: str, timeout: int = 10) -> subprocess.CompletedProcess:
return subprocess.run(
["systemctl", *args],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reported reproduction is a systemctl --user unit, but this helper always selects the system manager. It will not discover or restart ~/.config/systemd/user/hermes-dashboard.service, then returns False and the caller still SIGTERMs the dashboard PID. Probe/restart the user scope (and system scope if intended) explicitly.

capture_output=True,
text=True,
timeout=timeout,
)

# Probe the user manager first: Hermes installs Linux services in the
# user's systemd scope by default. Only fall back to the system manager
# when the unit is not present there, preserving root/system deployments.
# Crucially, keep the selected scope for *all* probes and the restart — a
# user unit must never be restarted through the system manager (or raw-killed).
scope: tuple[str, ...] | None = None
listed: subprocess.CompletedProcess | None = None
for candidate in (("--user",), ()):
try:
result = _systemctl(
*candidate, "list-unit-files", unit, "--no-legend", "--no-pager"
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
continue
if result.returncode != 0:
continue
unit_rows = (result.stdout or "").splitlines()
if any(row.split()[0:1] == [unit] for row in unit_rows if row.split()):
scope = candidate
listed = result
break

if scope is None or listed is None:
return False

try:
active = _systemctl(*scope, "is-active", unit)
enabled = _systemctl(*scope, "is-enabled", unit)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
return False

active_state = (active.stdout or "").strip()
enabled_state = (enabled.stdout or "").strip()
if active_state != "active" and enabled_state not in {
"enabled",
"enabled-runtime",
"linked",
"linked-runtime",
"static",
"generated",
}:
return False

print()
print(f"⟲ Restarting managed dashboard service ({reason})")

scope_label = "systemctl --user" if scope else "sudo systemctl"
restart = ("systemctl", *scope, "restart", unit)
commands = [restart]
if not scope:
# System units may require privilege escalation; user units must use
# the user manager directly and never prompt for sudo.
commands.append(("sudo", "-n", "systemctl", "restart", unit))

errors: list[str] = []
for command in commands:
try:
result = subprocess.run(
list(command),
capture_output=True,
text=True,
timeout=60,
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as e:
errors.append(f"{' '.join(command)}: {e}")
continue
if result.returncode == 0:
print(f" ✓ restarted {unit}")
return True
errors.append(
f"{' '.join(command)}: {(result.stderr or result.stdout or '').strip()}"
)

print(f" ✗ failed to restart {unit}")
for err in errors:
if err.strip():
print(f" {err}")
print(
" Dashboard is managed by systemd; not raw-killing its PID because "
"systemd would treat that as a clean stop."
)
print(f" Restart manually: {scope_label} restart {unit}")
return True


def _kill_stale_dashboard_processes(
reason: str = "the running backend no longer matches the updated frontend",
*,
restart_managed: bool = False,
) -> None:
"""Kill running ``hermes dashboard`` processes.

Expand All @@ -6180,10 +6292,15 @@ def _kill_stale_dashboard_processes(
Windows: ``taskkill /PID <pid> /F`` since there's no clean SIGTERM
equivalent for background console apps.

The dashboard isn't auto-restarted because we don't know the original
launch args (--host, --port, --insecure, --tui, --no-open). The user
restarts it manually; a hint is printed.
Manually-started dashboards are not auto-restarted because we don't know
the original launch args (--host, --port, --insecure, --tui, --no-open).
When ``restart_managed`` is true (the ``hermes update`` path), a detected
``hermes-dashboard.service`` is restarted through systemd instead of
raw-killing its main PID.
"""
if restart_managed and _restart_managed_dashboard_service(reason):
return

# When the Hermes Desktop Electron app spawns this dashboard as a
# backend child, it sets HERMES_DESKTOP_CHILD_PID so that the update
# path can skip killing the desktop-managed process. (#37532)
Expand Down Expand Up @@ -6535,7 +6652,7 @@ def _update_via_zip(args):
print(" ℹ Leaving running dashboard process(es) untouched because the")
print(" Node.js dependency refresh did not complete.")
else:
_kill_stale_dashboard_processes()
_kill_stale_dashboard_processes(restart_managed=True)


def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[str]:
Expand Down Expand Up @@ -11395,22 +11512,17 @@ def _resolve_manage_cmd(scope_: str, scope_cmd_: list, svc_name_: str):
except Exception as e:
logger.debug("Legacy unit check during update failed: %s", e)

# Kill stale dashboard processes — the dashboard has no service
# manager, so leaving it alive after a code update produces a
# silent frontend/backend mismatch. We can't auto-restart it
# (no saved launch args) but we can stop it, and a hint is
# printed for the user to re-launch.
#
# Exception: if the Node dependency refresh failed, the rebuilt
# frontend the new backend expects may not exist, so stopping a
# working dashboard would leave the user with nothing running
# rather than a usable (if mixed) state (#30271). Leave it alone.
# Restart a managed dashboard through systemd, or stop stale manual
# dashboard processes. Raw-killing a systemd-owned dashboard PID makes
# systemd treat it as a clean stop, leaving the Cloudflare origin dead.
# Preserve the safety rule above: a failed Node refresh leaves the
# currently running dashboard untouched.
if node_failures:
print()
print(" ℹ Leaving running dashboard process(es) untouched because the")
print(" Node.js dependency refresh did not complete.")
else:
_kill_stale_dashboard_processes()
_kill_stale_dashboard_processes(restart_managed=True)

print()
print("Tip: You can now select a provider and model:")
Expand Down
128 changes: 128 additions & 0 deletions tests/hermes_cli/test_update_stale_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from hermes_cli.main import (
_find_stale_dashboard_pids,
_kill_stale_dashboard_processes,
_restart_managed_dashboard_service,
_warn_stale_dashboard_processes, # back-compat alias
)

Expand All @@ -47,6 +48,7 @@ def _refresh_bindings_against_live_module():
"""
global _find_stale_dashboard_pids
global _kill_stale_dashboard_processes
global _restart_managed_dashboard_service
global _warn_stale_dashboard_processes

live = sys.modules.get("hermes_cli.main")
Expand All @@ -55,6 +57,7 @@ def _refresh_bindings_against_live_module():

_find_stale_dashboard_pids = live._find_stale_dashboard_pids
_kill_stale_dashboard_processes = live._kill_stale_dashboard_processes
_restart_managed_dashboard_service = live._restart_managed_dashboard_service
_warn_stale_dashboard_processes = live._warn_stale_dashboard_processes
yield

Expand Down Expand Up @@ -332,6 +335,131 @@ def fake_kill(pid, sig):
assert "✓ stopped PID 12345" in out
assert "failed to stop" not in out

def test_update_path_restarts_managed_dashboard_instead_of_killing(self, capsys):
"""A systemd-managed dashboard must be restarted through systemd.

Raw-killing the unit's main PID makes systemd record a clean stop, so
Restart=on-failure does not recover the Cloudflare origin.
"""
calls: list[list[str]] = []

def fake_run(args, *a, **kw):
calls.append(list(args))
if args == ["systemctl", "--user", "list-unit-files", "hermes-dashboard.service", "--no-legend", "--no-pager"]:
return MagicMock(returncode=0, stdout="", stderr="")
if args[:2] == ["systemctl", "list-unit-files"]:
return MagicMock(returncode=0, stdout="hermes-dashboard.service enabled enabled\n", stderr="")
if args[:2] == ["systemctl", "is-active"]:
return MagicMock(returncode=0, stdout="active\n", stderr="")
if args[:2] == ["systemctl", "is-enabled"]:
return MagicMock(returncode=0, stdout="enabled\n", stderr="")
if args == ["systemctl", "restart", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="", stderr="")
raise AssertionError(f"unexpected subprocess.run call: {args}")

with patch("subprocess.run", side_effect=fake_run), \
patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[12345]) as find_pids, \
patch("os.kill") as kill:
_kill_stale_dashboard_processes(restart_managed=True)

assert ["systemctl", "restart", "hermes-dashboard.service"] in calls
find_pids.assert_not_called()
kill.assert_not_called()

out = capsys.readouterr().out
assert "Restarting managed dashboard service" in out
assert "✓ restarted hermes-dashboard.service" in out

def test_user_scope_restart_never_falls_back_to_system_or_sudo(self, capsys):
"""A user unit is discovered and restarted through ``systemctl --user``."""
calls: list[list[str]] = []

def fake_run(args, *a, **kw):
calls.append(list(args))
if args == ["systemctl", "--user", "list-unit-files", "hermes-dashboard.service", "--no-legend", "--no-pager"]:
return MagicMock(returncode=0, stdout="hermes-dashboard.service enabled enabled\n", stderr="")
if args == ["systemctl", "--user", "is-active", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="active\n", stderr="")
if args == ["systemctl", "--user", "is-enabled", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="enabled\n", stderr="")
if args == ["systemctl", "--user", "restart", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="", stderr="")
raise AssertionError(f"unexpected subprocess.run call: {args}")

with patch("subprocess.run", side_effect=fake_run), \
patch("hermes_cli.main._find_stale_dashboard_pids", return_value=[12345]) as find_pids, \
patch("os.kill") as kill:
_kill_stale_dashboard_processes(restart_managed=True)

assert calls == [
["systemctl", "--user", "list-unit-files", "hermes-dashboard.service", "--no-legend", "--no-pager"],
["systemctl", "--user", "is-active", "hermes-dashboard.service"],
["systemctl", "--user", "is-enabled", "hermes-dashboard.service"],
["systemctl", "--user", "restart", "hermes-dashboard.service"],
]
assert all(call[:1] != ["sudo"] and call[:2] != ["systemctl"] for call in calls)
find_pids.assert_not_called()
kill.assert_not_called()
assert "✓ restarted hermes-dashboard.service" in capsys.readouterr().out

def test_user_scope_restart_failure_does_not_try_system_or_sudo(self):
"""A failed user-manager restart remains fail-closed and never raw-kills."""
calls: list[list[str]] = []

def fake_run(args, *a, **kw):
calls.append(list(args))
if args == ["systemctl", "--user", "list-unit-files", "hermes-dashboard.service", "--no-legend", "--no-pager"]:
return MagicMock(returncode=0, stdout="hermes-dashboard.service enabled enabled\n", stderr="")
if args[-2:] == ["is-active", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="active\n", stderr="")
if args[-2:] == ["is-enabled", "hermes-dashboard.service"]:
return MagicMock(returncode=0, stdout="enabled\n", stderr="")
if args[-2:] == ["restart", "hermes-dashboard.service"]:
return MagicMock(returncode=1, stdout="", stderr="user manager unavailable")
raise AssertionError(f"unexpected subprocess.run call: {args}")

with patch("subprocess.run", side_effect=fake_run), \
patch("hermes_cli.main._find_stale_dashboard_pids") as find_pids, \
patch("os.kill") as kill:
_kill_stale_dashboard_processes(restart_managed=True)

assert calls[-1] == ["systemctl", "--user", "restart", "hermes-dashboard.service"]
assert not any(call[:1] == ["sudo"] or call == ["systemctl", "restart", "hermes-dashboard.service"] for call in calls)
find_pids.assert_not_called()
kill.assert_not_called()

def test_managed_dashboard_restart_failure_does_not_raw_kill(self, capsys):
"""If systemd restart cannot run, print the fix and do not kill the PID."""
def fake_run(args, *a, **kw):
if args == ["systemctl", "--user", "list-unit-files", "hermes-dashboard.service", "--no-legend", "--no-pager"]:
return MagicMock(returncode=0, stdout="", stderr="")
if args[:2] == ["systemctl", "list-unit-files"]:
return MagicMock(returncode=0, stdout="hermes-dashboard.service enabled enabled\n", stderr="")
if args[:2] == ["systemctl", "is-active"]:
return MagicMock(returncode=0, stdout="active\n", stderr="")
if args[:2] == ["systemctl", "is-enabled"]:
return MagicMock(returncode=0, stdout="enabled\n", stderr="")
if args == ["systemctl", "restart", "hermes-dashboard.service"]:
return MagicMock(returncode=1, stdout="", stderr="Interactive authentication required.\n")
if args == ["sudo", "-n", "systemctl", "restart", "hermes-dashboard.service"]:
return MagicMock(returncode=1, stdout="", stderr="a password is required\n")
raise AssertionError(f"unexpected subprocess.run call: {args}")

with patch("subprocess.run", side_effect=fake_run), \
patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[12345]) as find_pids, \
patch("os.kill") as kill:
_kill_stale_dashboard_processes(restart_managed=True)

find_pids.assert_not_called()
kill.assert_not_called()

out = capsys.readouterr().out
assert "failed to restart hermes-dashboard.service" in out
assert "not raw-killing its PID" in out
assert "sudo systemctl restart hermes-dashboard.service" in out


class TestKillStaleDashboardWindows:
"""Kill path on Windows: taskkill /F."""
Expand Down
Loading