From a006117a1c0e0d8c129e44633d403af21367e09c Mon Sep 17 00:00:00 2001 From: Josh Tsai <128559392+bounce12340@users.noreply.github.com> Date: Wed, 22 Jul 2026 07:38:25 +0800 Subject: [PATCH] fix(serve): restart-friendly exit after update stops a supervised backend - hermes update (ZIP and git-pull paths) writes a restart marker before terminating serve/dashboard pids; `hermes serve --stop` does not. - serve/dashboard consumes its own fresh marker on graceful shutdown and exits 75 (EX_TEMPFAIL, same semantics as the gateway subsystem) so systemd Restart=on-failure brings it back; without a marker, or without a supervisor, behavior is unchanged. - stale-marker hygiene: startup clears leftovers (Windows taskkill /F skips graceful exit), entries expire after 10 minutes, corrupt files are discarded. - docs: recommend Restart=on-failure/always in the desktop systemd section. Fixes #68934 Co-Authored-By: Claude Fable 5 --- hermes_cli/main.py | 21 +++- hermes_cli/serve_restart_marker.py | 83 ++++++++++++++++ hermes_cli/web_server.py | 10 ++ tests/hermes_cli/test_serve_command.py | 99 +++++++++++++++++++ .../hermes_cli/test_update_stale_dashboard.py | 54 ++++++++++ website/docs/user-guide/desktop.md | 4 + 6 files changed, 267 insertions(+), 4 deletions(-) create mode 100644 hermes_cli/serve_restart_marker.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 1db8d66546e46..a1d922e5cca9e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -468,6 +468,7 @@ def _try_termux_ultrafast_version() -> bool: from hermes_cli.subcommands.plugins import build_plugins_parser from hermes_cli.subcommands.mcp import build_mcp_parser from hermes_cli.subcommands.claw import build_claw_parser +from hermes_cli.serve_restart_marker import write_restart_markers def _require_tty(command_name: str) -> None: @@ -7047,11 +7048,14 @@ def _kill_stale_dashboard_processes( reason: str = "the running backend no longer matches the updated frontend", *, restart_managed: bool = False, + restart_hint: bool = False, ) -> None: """Kill running ``hermes dashboard`` processes. - Called at the end of ``hermes update`` (default ``reason``) and also - from ``hermes dashboard --stop`` (which overrides ``reason``). The + Called at the end of ``hermes update`` (default ``reason`` with + ``restart_managed=True`` and ``restart_hint=True``) and also from + ``hermes dashboard --stop`` (which overrides ``reason`` and requests + neither a managed restart nor a supervisor restart). The dashboard has no service manager, so after a code update the running process is guaranteed to be serving stale Python against a freshly-updated JS bundle. Leaving it alive produces silent @@ -7067,6 +7071,12 @@ def _kill_stale_dashboard_processes( 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. + + ``restart_hint`` covers the supervisors that unit lookup cannot reach: a + systemd unit under a different name, s6/runit, or a container restart + policy. It leaves a marker so the terminated process exits non-zero and + its supervisor brings it back; otherwise the printed manual hint remains + the recovery path. """ if restart_managed and _restart_managed_dashboard_service(reason): return @@ -7095,6 +7105,9 @@ def _kill_stale_dashboard_processes( if not pids: return + if restart_hint: + write_restart_markers(pids) + print() print(f"⟲ Stopping {len(pids)} dashboard process(es) ({reason})") @@ -7487,7 +7500,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(restart_managed=True) + _kill_stale_dashboard_processes(restart_managed=True, restart_hint=True) def _stash_local_changes_if_needed(git_cmd: list[str], cwd: Path) -> Optional[str]: @@ -13027,7 +13040,7 @@ def _on_unit_timeout(svc_name: str, exc: subprocess.TimeoutExpired) -> None: print(" ℹ Leaving running dashboard process(es) untouched because the") print(" Node.js dependency refresh did not complete.") else: - _kill_stale_dashboard_processes(restart_managed=True) + _kill_stale_dashboard_processes(restart_managed=True, restart_hint=True) print() print("Tip: You can now select a provider and model:") diff --git a/hermes_cli/serve_restart_marker.py b/hermes_cli/serve_restart_marker.py new file mode 100644 index 0000000000000..04cfba1726c16 --- /dev/null +++ b/hermes_cli/serve_restart_marker.py @@ -0,0 +1,83 @@ +"""Restart markers for dashboard/serve processes stopped by an update.""" + +from __future__ import annotations + +import json +import logging +import time +from pathlib import Path + +from hermes_constants import get_hermes_home +from utils import atomic_json_write + +logger = logging.getLogger(__name__) + +# EX_TEMPFAIL from sysexits.h, matching the gateway restart contract: a +# non-zero exit asks Restart=on-failure supervisors to bring the service back. +RESTART_EXIT_CODE = 75 + +_MARKER_FILENAME = "serve_restart.json" + + +def _marker_path() -> Path: + return get_hermes_home() / "runtime" / _MARKER_FILENAME + + +def _remove_marker(path: Path) -> None: + try: + path.unlink(missing_ok=True) + except OSError as exc: + logger.debug("Failed to remove serve restart marker %s: %s", path, exc) + + +def write_restart_markers(pids: list[int]) -> None: + """Record processes that should exit non-zero after update shutdown.""" + normalized = sorted({pid for pid in pids if pid > 0}) + if not normalized: + return + try: + atomic_json_write( + _marker_path(), + {"pids": normalized, "written_at": time.time()}, + sort_keys=True, + ) + except (OSError, TypeError, ValueError) as exc: + logger.warning("Failed to write serve restart marker: %s", exc) + + +def consume_restart_marker(pid: int, max_age_seconds: float = 600) -> bool: + """Consume a fresh update marker for *pid*, cleaning stale residue.""" + path = _marker_path() + try: + data = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return False + except (OSError, json.JSONDecodeError): + _remove_marker(path) + return False + + try: + written_at = float(data["written_at"]) + marked_pids = [int(value) for value in data["pids"]] + age = time.time() - written_at + except (KeyError, TypeError, ValueError): + _remove_marker(path) + return False + + if age < 0 or age > max_age_seconds or pid not in marked_pids: + _remove_marker(path) + return False + + remaining = [marked_pid for marked_pid in marked_pids if marked_pid != pid] + if not remaining: + _remove_marker(path) + else: + try: + atomic_json_write( + path, + {"pids": remaining, "written_at": written_at}, + sort_keys=True, + ) + except (OSError, TypeError, ValueError) as exc: + logger.debug("Failed to update serve restart marker %s: %s", path, exc) + return True diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 7b73e13eaffde..e9e99de7d0619 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -57,6 +57,7 @@ sys.path.insert(0, str(PROJECT_ROOT)) from hermes_cli import __version__, __release_date__ +from hermes_cli.serve_restart_marker import RESTART_EXIT_CODE, consume_restart_marker from hermes_cli.config import ( cfg_get, DEFAULT_CONFIG, @@ -19929,6 +19930,10 @@ def start_server( ``ssh_session_token`` and ``ssh_owner_nonce`` are process-local Desktop SSH bootstrap state. Neither is persisted or exported to child processes. """ + # A restarted process gets a new PID and must not inherit marker residue + # from the serve instance that the updater stopped. + consume_restart_marker(os.getpid()) + _apply_ssh_session_token(ssh_session_token or "") _apply_ssh_owner_nonce(ssh_owner_nonce) @@ -20189,6 +20194,8 @@ def _loop_heartbeat(expected: float) -> None: # uvicorn's own machinery and run on the loop factory it picks. if sys.platform != "win32": asyncio.run(_serve()) + if consume_restart_marker(os.getpid()): + sys.exit(RESTART_EXIT_CODE) return # Windows-only path. Resolve the runner + loop factory FIRST (and fall back @@ -20214,3 +20221,6 @@ def _loop_heartbeat(expected: float) -> None: _runner(_serve(), loop_factory=_loop_factory) else: asyncio.run(_serve()) + + if consume_restart_marker(os.getpid()): + sys.exit(RESTART_EXIT_CODE) diff --git a/tests/hermes_cli/test_serve_command.py b/tests/hermes_cli/test_serve_command.py index 911b0db958348..5c3c9fb93d5b3 100644 --- a/tests/hermes_cli/test_serve_command.py +++ b/tests/hermes_cli/test_serve_command.py @@ -12,7 +12,12 @@ from __future__ import annotations import argparse +import json +import pytest +import uvicorn + +from hermes_cli import serve_restart_marker, web_server from hermes_cli.subcommands.dashboard import build_dashboard_parser @@ -68,3 +73,97 @@ def test_serve_is_a_headless_backend_but_dashboard_is_not(): # build; only `serve` carries it. assert getattr(_parser().parse_args(["serve"]), "headless_backend", False) is True assert getattr(_parser().parse_args(["dashboard"]), "headless_backend", False) is False + + +@pytest.fixture +def restart_marker_path(monkeypatch, tmp_path): + monkeypatch.setattr( + serve_restart_marker, + "get_hermes_home", + lambda: tmp_path, + ) + return tmp_path / "runtime" / "serve_restart.json" + + +class TestServeRestartMarker: + def test_matching_fresh_marker_is_consumed(self, restart_marker_path): + serve_restart_marker.write_restart_markers([12345]) + + assert serve_restart_marker.consume_restart_marker(12345) is True + assert not restart_marker_path.exists() + + def test_matching_pid_is_removed_without_dropping_others( + self, restart_marker_path + ): + serve_restart_marker.write_restart_markers([12345, 54321]) + + assert serve_restart_marker.consume_restart_marker(12345) is True + marker = json.loads(restart_marker_path.read_text(encoding="utf-8")) + assert marker["pids"] == [54321] + assert isinstance(marker["written_at"], float) + + def test_pid_mismatch_cleans_marker(self, restart_marker_path): + serve_restart_marker.write_restart_markers([12345]) + + assert serve_restart_marker.consume_restart_marker(54321) is False + assert not restart_marker_path.exists() + + def test_expired_marker_is_cleaned(self, monkeypatch, restart_marker_path): + restart_marker_path.parent.mkdir(parents=True) + restart_marker_path.write_text( + json.dumps({"pids": [12345], "written_at": 100.0}), + encoding="utf-8", + ) + monkeypatch.setattr(serve_restart_marker.time, "time", lambda: 701.0) + + assert serve_restart_marker.consume_restart_marker(12345) is False + assert not restart_marker_path.exists() + + def test_missing_marker_returns_false(self, restart_marker_path): + assert serve_restart_marker.consume_restart_marker(12345) is False + + +def _stub_start_server(monkeypatch): + class _FakeConfig: + loaded = True + + def __init__(self, *args, **kwargs): + pass + + class _FakeServer: + def __init__(self, _config): + pass + + monkeypatch.setattr(uvicorn, "Config", _FakeConfig) + monkeypatch.setattr(uvicorn, "Server", _FakeServer) + monkeypatch.setattr(web_server.sys, "platform", "linux") + monkeypatch.setattr( + web_server.asyncio, + "run", + lambda coro: coro.close(), + ) + + +class TestServeRestartExit: + def test_marker_exits_with_restart_code(self, monkeypatch): + _stub_start_server(monkeypatch) + calls = iter([False, True]) + monkeypatch.setattr( + web_server, + "consume_restart_marker", + lambda _pid: next(calls), + ) + with pytest.raises(SystemExit) as exc_info: + web_server.start_server(open_browser=False) + + assert exc_info.value.code == serve_restart_marker.RESTART_EXIT_CODE + + def test_no_marker_returns_normally(self, monkeypatch): + _stub_start_server(monkeypatch) + monkeypatch.setattr( + web_server, + "consume_restart_marker", + lambda _pid: False, + ) + + assert web_server.start_server(open_browser=False) is None diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index fd26590786ecf..97bb37b0b1821 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -21,6 +21,7 @@ import pytest from hermes_cli.main import ( + cmd_dashboard, _find_stale_dashboard_pids, _kill_stale_dashboard_processes, _restart_managed_dashboard_service, @@ -50,6 +51,7 @@ def _refresh_bindings_against_live_module(): global _kill_stale_dashboard_processes global _restart_managed_dashboard_service global _warn_stale_dashboard_processes + global cmd_dashboard live = sys.modules.get("hermes_cli.main") if live is None: @@ -59,6 +61,7 @@ def _refresh_bindings_against_live_module(): _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 + cmd_dashboard = live.cmd_dashboard yield @@ -86,6 +89,10 @@ def _side_effect(args, *a, **kw): class TestFindStaleDashboardPids: """Unit tests for the ps/wmic-based detection step.""" + @pytest.fixture(autouse=True) + def _use_posix_ps_parser(self, monkeypatch): + monkeypatch.setattr(sys, "platform", "linux") + def test_no_matches_returns_empty(self): with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock( @@ -231,6 +238,53 @@ def test_exclude_all_pids_returns_empty(self): assert pids == [] +class TestKillRestartMarker: + def test_restart_hint_writes_marker_before_sigterm(self, monkeypatch): + import signal as _signal + import gateway.status as gateway_status + + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setattr(gateway_status, "_pid_exists", lambda _pid: False) + events: list[tuple[str, object]] = [] + + def fake_write(pids): + events.append(("marker", list(pids))) + + def fake_kill(pid, sig): + events.append(("kill", (pid, sig))) + + with patch("hermes_cli.main._find_stale_dashboard_pids", + return_value=[12345]), \ + patch("hermes_cli.main.write_restart_markers", + side_effect=fake_write), \ + patch("os.kill", side_effect=fake_kill), \ + patch("time.sleep"): + _kill_stale_dashboard_processes(restart_hint=True) + + assert events[0] == ("marker", [12345]) + assert events[1] == ("kill", (12345, _signal.SIGTERM)) + + def test_dashboard_stop_does_not_write_restart_marker(self, monkeypatch): + import types + + monkeypatch.setattr(sys, "platform", "win32") + args = types.SimpleNamespace( + ssh_session_token_file=None, + status=False, + stop=True, + ) + with patch("hermes_cli.main._find_stale_dashboard_pids", + side_effect=[[12345], [12345], []]), \ + patch("hermes_cli.main.write_restart_markers") as marker_write, \ + patch("subprocess.run", + return_value=MagicMock(returncode=0, stdout="", stderr="")), \ + pytest.raises(SystemExit) as exc_info: + cmd_dashboard(args) + + assert exc_info.value.code == 0 + marker_write.assert_not_called() + + @pytest.mark.skipif(sys.platform == "win32", reason="POSIX kill semantics") class TestKillStaleDashboardPosix: """Kill path on Linux / macOS: SIGTERM then SIGKILL any survivors.""" diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index 34ccf2a810bd2..78815387cdd80 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -209,6 +209,10 @@ Separately, make sure the **gateway is running** on the remote host if you rely Prefer not to keep a plaintext password at rest? Set `HERMES_DASHBOARD_BASIC_AUTH_PASSWORD_HASH` to a scrypt hash instead — compute it with `python -c "from plugins.dashboard_auth.basic import hash_password; print(hash_password('PW'))"`. Full configuration surface (config.yaml keys, every env var, the rate limiter): [Web Dashboard → Username/password provider](./features/web-dashboard.md#usernamepassword-provider-no-oauth-idp). Running the backend as a systemd service? Give the unit `EnvironmentFile=%h/.hermes/.env` so the credentials are in the environment at boot. +Set `Restart=on-failure` (or `Restart=always`) to recover automatically after +remote updates. When an update stops `hermes serve`, it exits with status 75 so +the supervisor restarts it; `hermes serve --stop` remains a clean stop and does +not request a restart. :::warning The backend reads and writes your `.env` (API keys, secrets) and can run agent commands. The **username/password** setup shown above is for a trusted network — never expose a password-protected backend directly to the open internet; put it behind a VPN. [Tailscale](https://tailscale.com/) is the clean option: bind to the machine's tailscale IP (`--host `) and use `http://:9119` as the Remote URL so only your tailnet can reach it. To reach a backend over the public internet, use the **OAuth (Nous Portal)** provider instead.