Skip to content
Closed
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
142 changes: 140 additions & 2 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -7862,6 +7862,135 @@ def _print_curator_recent_run_notice() -> None:
pass


_DASHBOARD_SYSTEMD_UNIT_NAMES: tuple[str, ...] = (
"hermes-dashboard.service",
"hermes-webui.service",
"hermes-web-ui.service",
)


def _systemd_scope_args(*, system: bool) -> list[str]:
return ["systemctl"] if system else ["systemctl", "--user"]


def _dashboard_systemd_unit_candidates(*, system: bool) -> list[str]:
"""Return installed systemd units that look like Hermes dashboard services.

Hermes does not ship a first-class dashboard service manager yet, but some
deployments keep the web UI alive with a local systemd unit. ``hermes
update`` deliberately stops stale ``hermes dashboard`` backends after
rebuilding the frontend; if one of those backends belongs to systemd, the
update path should hand control back to systemd instead of leaving the web
UI offline.

We keep the detection conservative: known unit names are accepted only if
present, and wildcard matches must have an ``ExecStart`` containing a
Hermes dashboard command.
"""
if sys.platform.startswith("win") or sys.platform == "darwin":
return []
if not shutil.which("systemctl"):
return []

scope = _systemd_scope_args(system=system)
candidates: list[str] = []

for unit in _DASHBOARD_SYSTEMD_UNIT_NAMES:
try:
result = subprocess.run(
scope + ["show", unit, "--property=LoadState", "--value"],
capture_output=True,
text=True,
timeout=5,
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
continue
if result.returncode == 0 and (result.stdout or "").strip() == "loaded":
candidates.append(unit)

try:
result = subprocess.run(
scope
+ [
"list-unit-files",
"hermes*dashboard*.service",
"hermes*webui*.service",
"hermes*web-ui*.service",
"--no-legend",
"--no-pager",
],
capture_output=True,
text=True,
timeout=5,
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
result = None

if result and result.returncode == 0:
for line in (result.stdout or "").splitlines():
parts = line.split()
if not parts or not parts[0].endswith(".service"):
continue
unit = parts[0]
if unit in candidates:
continue
try:
show = subprocess.run(
scope + ["show", unit, "--property=ExecStart", "--value"],
capture_output=True,
text=True,
timeout=5,
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError):
continue
exec_start = show.stdout or ""
if show.returncode == 0 and (
"hermes dashboard" in exec_start
or "hermes_cli.main dashboard" in exec_start
or "hermes_cli/main.py dashboard" in exec_start
):
candidates.append(unit)

return candidates


def _restart_dashboard_systemd_services() -> list[tuple[str, bool, str]]:
"""Restart installed Hermes dashboard systemd units, returning outcomes."""
outcomes: list[tuple[str, bool, str]] = []
for system in (False, True):
units = _dashboard_systemd_unit_candidates(system=system)
if not units:
continue
scope = _systemd_scope_args(system=system)
scope_label = "system" if system else "user"
for unit in units:
try:
subprocess.run(
scope + ["daemon-reload"],
capture_output=True,
text=True,
timeout=10,
)
subprocess.run(
scope + ["enable", unit],

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.

enable changes persistence, not just restart state. Because known-name candidates are accepted when merely loaded (:7898-7909), this can permanently enable a unit the operator intentionally disabled. Preserve that policy; do not enable it during update.

capture_output=True,
text=True,
timeout=10,
)
result = subprocess.run(
scope + ["restart", unit],
capture_output=True,
text=True,
timeout=20,
)
except (FileNotFoundError, subprocess.TimeoutExpired, OSError) as e:
outcomes.append((f"{scope_label}:{unit}", False, str(e)))
continue
msg = (result.stderr or result.stdout or "").strip()
outcomes.append((f"{scope_label}:{unit}", result.returncode == 0, msg))
return outcomes


def _format_time_ago(iso_ts: str) -> str:
"""Render an ISO timestamp as `Xh ago` / `Xd ago` / `Xm ago`. Best effort."""
try:
Expand Down Expand Up @@ -7997,8 +8126,17 @@ def _kill_stale_dashboard_processes(
print(f" ✗ failed to stop PID {pid}: {err_msg}")

if killed:
print(" Restart the dashboard when you're ready:")
print(" hermes dashboard --port <port>")
restart_outcomes = _restart_dashboard_systemd_services()

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.

This restart is triggered after any stale dashboard PID is killed, but candidate discovery only checks unit names/ExecStart and never verifies its MainPID was one of those PIDs. A dormant matching unit can be started for an unrelated manual dashboard. Match service MainPID to the pre-kill PID set before restarting.

if restart_outcomes:
print(" Restarting dashboard systemd service(s):")
for unit, ok, msg in restart_outcomes:
if ok:
print(f" ✓ restarted {unit}")
else:
print(f" ✗ failed to restart {unit}: {msg or 'unknown error'}")
else:
print(" Restart the dashboard when you're ready:")
print(" hermes dashboard --port <port>")


# Back-compat alias: some tests and any external callers may import the old
Expand Down
100 changes: 100 additions & 0 deletions tests/hermes_cli/test_update_stale_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import pytest

from hermes_cli.main import (
_dashboard_systemd_unit_candidates,
_find_stale_dashboard_pids,
_kill_stale_dashboard_processes,
_restart_dashboard_systemd_services,
_warn_stale_dashboard_processes, # back-compat alias
)

Expand All @@ -45,16 +47,20 @@ def _refresh_bindings_against_live_module():
ordering within the worker. The fix lives in the test module because
the two pollutants above are load-bearing for their own tests.
"""
global _dashboard_systemd_unit_candidates
global _find_stale_dashboard_pids
global _kill_stale_dashboard_processes
global _restart_dashboard_systemd_services
global _warn_stale_dashboard_processes

live = sys.modules.get("hermes_cli.main")
if live is None:
live = importlib.import_module("hermes_cli.main")

_dashboard_systemd_unit_candidates = live._dashboard_systemd_unit_candidates
_find_stale_dashboard_pids = live._find_stale_dashboard_pids
_kill_stale_dashboard_processes = live._kill_stale_dashboard_processes
_restart_dashboard_systemd_services = live._restart_dashboard_systemd_services
_warn_stale_dashboard_processes = live._warn_stale_dashboard_processes
yield

Expand Down Expand Up @@ -253,6 +259,7 @@ def fake_kill(pid, sig):

with patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[12345, 12346]), \
patch("hermes_cli.main._restart_dashboard_systemd_services", return_value=[]), \
patch("os.kill", side_effect=fake_kill), \
patch("time.sleep"):
_kill_stale_dashboard_processes()
Expand Down Expand Up @@ -285,6 +292,7 @@ def fake_kill(pid, sig):

with patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[99999]), \
patch("hermes_cli.main._restart_dashboard_systemd_services", return_value=[]), \
patch("os.kill", side_effect=fake_kill), \
patch("time.sleep"), \
patch("time.monotonic", side_effect=[0.0] + [10.0] * 20):
Expand All @@ -308,6 +316,7 @@ def fake_kill(pid, sig):

with patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[12345]), \
patch("hermes_cli.main._restart_dashboard_systemd_services", return_value=[]), \
patch("os.kill", side_effect=fake_kill), \
patch("time.sleep"):
_kill_stale_dashboard_processes() # must not raise
Expand All @@ -324,6 +333,7 @@ def fake_kill(pid, sig):

with patch("hermes_cli.main._find_stale_dashboard_pids",
return_value=[12345]), \
patch("hermes_cli.main._restart_dashboard_systemd_services", return_value=[]), \
patch("os.kill", side_effect=fake_kill), \
patch("time.sleep"):
_kill_stale_dashboard_processes()
Expand Down Expand Up @@ -378,6 +388,96 @@ def fake_run(args, *a, **kw):
assert "Access is denied" in out


class TestDashboardSystemdRestart:
"""Dashboard services are restarted after update stops stale backends."""

def test_detects_loaded_known_user_unit(self, monkeypatch):
monkeypatch.setattr(sys, "platform", "linux")
calls: list[list[str]] = []

def fake_run(args, *a, **kw):
calls.append(list(args))
joined = " ".join(args)
if "show hermes-dashboard.service" in joined and "LoadState" in joined:
return MagicMock(returncode=0, stdout="loaded\n", stderr="")
return MagicMock(returncode=0, stdout="", stderr="")

with patch("shutil.which", return_value="/bin/systemctl"), \
patch("subprocess.run", side_effect=fake_run):
assert _dashboard_systemd_unit_candidates(system=False) == [
"hermes-dashboard.service"
]

assert calls[0][:2] == ["systemctl", "--user"]

def test_detects_wildcard_unit_only_when_execstart_is_dashboard(self, monkeypatch):
monkeypatch.setattr(sys, "platform", "linux")

def fake_run(args, *a, **kw):
joined = " ".join(args)
if "--property=LoadState" in joined:
return MagicMock(returncode=1, stdout="not-found\n", stderr="")
if "list-unit-files" in joined:
return MagicMock(
returncode=0,
stdout="my-hermes-dashboard.service enabled\nother-dashboard.service enabled\n",
stderr="",
)
if "show my-hermes-dashboard.service" in joined and "ExecStart" in joined:
return MagicMock(
returncode=0,
stdout="{ path=/usr/bin/hermes ; argv[]=hermes dashboard --port 9119 }\n",
stderr="",
)
if "show other-dashboard.service" in joined and "ExecStart" in joined:
return MagicMock(returncode=0, stdout="/usr/bin/grafana-server\n", stderr="")
return MagicMock(returncode=0, stdout="", stderr="")

with patch("shutil.which", return_value="/bin/systemctl"), \
patch("subprocess.run", side_effect=fake_run):
assert _dashboard_systemd_unit_candidates(system=False) == [
"my-hermes-dashboard.service"
]

def test_restart_enables_and_restarts_detected_unit(self, monkeypatch):
monkeypatch.setattr(sys, "platform", "linux")
commands: list[list[str]] = []

def fake_candidates(*, system: bool):
return ["hermes-dashboard.service"] if not system else []

def fake_run(args, *a, **kw):
commands.append(list(args))
return MagicMock(returncode=0, stdout="", stderr="")

with patch("hermes_cli.main._dashboard_systemd_unit_candidates", side_effect=fake_candidates), \
patch("subprocess.run", side_effect=fake_run):
assert _restart_dashboard_systemd_services() == [
("user:hermes-dashboard.service", True, "")
]

assert ["systemctl", "--user", "daemon-reload"] in commands
assert ["systemctl", "--user", "enable", "hermes-dashboard.service"] in commands
assert ["systemctl", "--user", "restart", "hermes-dashboard.service"] in commands

def test_kill_restarts_systemd_service_instead_of_manual_hint(self, capsys):
def fake_kill(pid, sig):
if sig == 0:
raise ProcessLookupError

with patch("hermes_cli.main._find_stale_dashboard_pids", return_value=[12345]), \
patch("hermes_cli.main._restart_dashboard_systemd_services", return_value=[
("user:hermes-dashboard.service", True, "")
]), \
patch("os.kill", side_effect=fake_kill), \
patch("time.sleep"):
_kill_stale_dashboard_processes()

out = capsys.readouterr().out
assert "✓ restarted user:hermes-dashboard.service" in out
assert "Restart the dashboard when you're ready" not in out


class TestBackCompatAlias:
"""``_warn_stale_dashboard_processes`` is kept as an alias for the
new kill function so old imports don't break."""
Expand Down