Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -5142,6 +5142,7 @@ def _clear_bytecode_cache(root: Path) -> int:
_resume_windows_gateways_after_update,
_run_logged_subprocess,
_run_pre_update_backup,
_service_unit_supports_graceful_sigusr1_restart,
_should_skip_upstream_prompt,
_stash_apply_failed_only_on_existing_untracked,
_stash_local_changes_if_needed,
Expand Down
71 changes: 46 additions & 25 deletions hermes_cli/update_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -3452,7 +3452,8 @@ def _for_each_systemd_gateway_unit(
process_unit,
on_unit_timeout,
) -> None:
"""Process each ``hermes-gateway*.service`` from ``systemctl list-units``.
"""Process each ``hermes-gateway*.service``/``hermes-serve*.service`` unit
from ``systemctl list-units``.

``subprocess.TimeoutExpired`` raised by ``process_unit`` is isolated to
that unit via ``on_unit_timeout`` so one wedged systemctl call cannot
Expand All @@ -3466,15 +3467,27 @@ def _for_each_systemd_gateway_unit(
if not unit.endswith(".service"):
continue
# list-units is already pattern-filtered, but keep the name gate so a
# stray non-gateway line cannot enter the restart path.
if not unit.startswith("hermes-gateway"):
# stray non-gateway/serve line cannot enter the restart path.
if not (unit.startswith("hermes-gateway") or unit.startswith("hermes-serve")):

@unsupportedpastels unsupportedpastels Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: this prefix also accepts hermes-server.service, and the hermes-serve* systemctl pattern selects it. Consider constraining the match to the exact base unit or the hyphenated profile family (hermes-serve / hermes-serve-*) and adding a near-prefix rejection test.

continue
svc_name = unit.removesuffix(".service")
try:
process_unit(svc_name)
except subprocess.TimeoutExpired as exc:
on_unit_timeout(svc_name, exc)

def _service_unit_supports_graceful_sigusr1_restart(svc_name: str) -> bool:
"""Whether *svc_name* wires SIGUSR1 to a graceful drain-then-restart.

Only ``hermes-gateway*`` units run ``gateway/run.py``, which installs the
SIGUSR1 handler. ``hermes-serve*`` units (#83438) don't, so sending them
SIGUSR1 would just invoke the default terminate action and burn the full
drain budget waiting for an exit that was never graceful — go straight to
the blunt ``systemctl restart`` path for those instead.
"""
return svc_name.startswith("hermes-gateway")


def _warn_incomplete_gateway_fleet_restart(failed_units: list) -> None:
"""Print an explicit incomplete-update warning for unrestarted units."""
if not failed_units:
Expand All @@ -3488,7 +3501,7 @@ def _warn_incomplete_gateway_fleet_restart(failed_units: list) -> None:
seen.add(name)
ordered.append(name)
print()
print("⚠ Update incomplete — some gateway units were not restarted:")
print("⚠ Update incomplete — some units were not restarted:")
for name in ordered:
print(f" - {name}")
print(" Skipped units may still be running pre-update code (mixed")
Expand Down Expand Up @@ -5210,7 +5223,8 @@ def _resolve_manage_cmd(scope_: str, scope_cmd_: list, svc_name_: str):
externally_supervised_profiles = []

# --- Systemd services (Linux) ---
# Discover all hermes-gateway* units (default + profiles)
# Discover all hermes-gateway* units (default + profiles) plus
# hermes-serve* units (the Desktop app's backend, #83438).
if supports_systemd_services():
try:
_ensure_user_systemd_env()
Expand All @@ -5227,6 +5241,7 @@ def _resolve_manage_cmd(scope_: str, scope_cmd_: list, svc_name_: str):
+ [
"list-units",
"hermes-gateway*",
"hermes-serve*",

@unsupportedpastels unsupportedpastels Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: this adds a first restart for hermes-serve, while the updater still calls _finish_dashboard_update_cleanup() later. Its existing restart_managed=True path scans hermes serve PIDs and restarts the owning custom systemd unit. On a Serve-only install, the newly restarted process may therefore be found and restarted a second time. It may be worth deduplicating the lifecycle paths and adding an orchestration test for Serve-only and Dashboard+Serve cases.

"--plain",
"--no-legend",
"--no-pager",
Expand Down Expand Up @@ -5273,27 +5288,33 @@ def _restart_one_systemd_gateway_unit(svc_name: str) -> None:
# The gateway's SIGUSR1 handler calls
# request_restart(via_service=True) → drain →
# exit; systemd's Restart=always respawns the unit.
# hermes-serve has no such handler (it isn't
# gateway/run.py), so skip straight to the blunt
# restart below rather than sending it an unhandled
# signal and waiting out the drain budget for
# nothing.
_main_pid = 0
try:
_show = subprocess.run(
scope_cmd
+ [
"show",
svc_name,
"--property=MainPID",
"--value",
],
capture_output=True,
text=True, encoding="utf-8", errors="replace",
timeout=5,
)
_main_pid = int((_show.stdout or "").strip() or 0)
except (
ValueError,
subprocess.TimeoutExpired,
FileNotFoundError,
):
_main_pid = 0
if _service_unit_supports_graceful_sigusr1_restart(svc_name):
try:
_show = subprocess.run(
scope_cmd
+ [
"show",
svc_name,
"--property=MainPID",
"--value",
],
capture_output=True,
text=True, encoding="utf-8", errors="replace",
timeout=5,
)
_main_pid = int((_show.stdout or "").strip() or 0)
except (
ValueError,
subprocess.TimeoutExpired,
FileNotFoundError,
):
_main_pid = 0

_graceful_ok = False
if _main_pid > 0:
Expand Down
39 changes: 39 additions & 0 deletions tests/hermes_cli/test_update_fleet_restart_timeout.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from hermes_cli.main import (
_for_each_systemd_gateway_unit,
_service_unit_supports_graceful_sigusr1_restart,
_warn_incomplete_gateway_fleet_restart,
)

Expand Down Expand Up @@ -90,6 +91,44 @@ def test_non_gateway_units_in_list_output_are_ignored(self):

assert seen == ["hermes-gateway-coder"]

def test_hermes_serve_units_are_included(self):
# #83438 — hermes update restarted hermes-gateway* units but left
# hermes-serve* (the Desktop app's backend) on stale pre-update code.
seen: list[str] = []

_for_each_systemd_gateway_unit(
"\n".join(
[
"ssh.service loaded active running",
"hermes-serve.service loaded active running",
"hermes-serve-work.service loaded active running",
"hermes-gateway.service loaded active running",
"",
]
),
process_unit=seen.append,
on_unit_timeout=lambda *_: pytest.fail("unexpected timeout"),
)

assert seen == ["hermes-serve", "hermes-serve-work", "hermes-gateway"]


class TestGracefulSigusr1Eligibility:
def test_gateway_units_are_eligible(self):
assert _service_unit_supports_graceful_sigusr1_restart("hermes-gateway")
assert _service_unit_supports_graceful_sigusr1_restart(
"hermes-gateway-work"
)

def test_serve_units_are_not_eligible(self):
# hermes-serve doesn't run gateway/run.py, so it never installs the
# SIGUSR1 handler — sending it the signal would just terminate the
# process (the default action) instead of draining gracefully.
assert not _service_unit_supports_graceful_sigusr1_restart("hermes-serve")
assert not _service_unit_supports_graceful_sigusr1_restart(
"hermes-serve-work"
)

def test_process_errors_other_than_timeout_still_propagate(self):
def process_unit(_svc_name: str) -> None:
raise RuntimeError("not a timeout")
Expand Down
Loading