ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(gateway): use SIGUSR1 graceful restart on launchd, not bare SIGTERM

Browse Source 
`hermes gateway restart` on macOS never took the graceful path, so every
restart — including deliberate ones — was reported to chat as an unplanned
shutdown.

`launchd_restart()` diverged from `systemd_restart()` in two ways, each
sufficient to break it on its own:

1. Wrong helper. It called `_request_gateway_self_restart()`, which is gated
   on `_is_pid_ancestor_of_current_process()`. That holds only when the CLI
   was spawned *by* the gateway (in-chat `/restart`). Invoked from a shell the
   gateway is a sibling, so the guard returns False and SIGUSR1 is never sent.
   `_graceful_restart_via_sigusr1()` — same job, no ancestry gate, already
   used by `systemd_restart()` and the updater — had no launchd call site.

2. Wrong budget. It waited `_get_restart_drain_timeout()`, which defaults to
   0, so `_wait_for_gateway_exit(timeout=0.0)` could never succeed. The
   systemd branch uses `_get_restart_exit_wait_budget()`
   (drain + after_turn + 15s headroom); `resolve_restart_exit_wait_budget()`
   documents that callers falling back to a hard kill must cover both phases
   or they reintroduce #77184.

The result was a bare SIGTERM followed immediately by `kickstart -k`. Since
SIGTERM leaves `restart_requested` False, the gateway exited 1 instead of 75
and announced "⚠️ Gateway shutting down — Your current task will be
interrupted." instead of "restarting", dropping the resume_pending handoff
that lets a session resume after the bounce.

Observed on macOS 27.0 / Hermes 0.20.4:

    → Stopping gateway (PID 49787) — draining in-flight runs (up to 0s)...
    ⚠ Gateway PID 49787 still running after 0.0s — restart may fail
    ⚠ Gateway drain timed out after 0s — forcing launchd restart

Send SIGUSR1 with the exit-wait budget and return on success, leaving
launchd's unconditional KeepAlive to revive the process. `kickstart -k` stays
as the fallback for a genuine drain timeout, but must not run after a
successful graceful exit or it would kill the replacement instance.

The wedged-loop escalation (#81642) still short-circuits ahead of this, so a
provably dead event loop is not handed a signal it cannot process.

Tests: adds a launchd counterpart to the existing systemd graceful-restart
test, asserting SIGUSR1 with the exit-wait budget and no bare SIGTERM or
kickstart on success. Updates the three wedged-gateway tests, which asserted
the old SIGTERM-plus-drain shape; they also now stub
`_graceful_restart_via_sigusr1` so no real signal escapes to the fake PID.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Joby Ellington
2026-08-21 01:16:50 -05:00
committed by kshitij
parent 1348e65e26
commit 7a76046a86
3 changed files with 93 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/gateway.py
+34 -18
View File
@@ -5549,7 +5549,6 @@ def _wait_for_launchd_service_pid(
def launchd_restart():
label = get_launchd_label()
target = f"{_launchd_domain()}/{label}"
drain_timeout = _get_restart_drain_timeout()
from gateway.status import get_running_pid
try:
@@ -5573,26 +5572,43 @@ def launchd_restart():
_escalate_wedged_gateway(pid)
pid = None
if pid is not None:
# Announce the drain BEFORE waiting on it. This wait can run for
# the full drain budget (180s by default) while the old gateway
# finishes in-flight agent runs, and it streams into surfaces with
# no other feedback — the desktop updater's live output most of
# all, where a silent stop here reads as "update stuck" (#44515).
# Mirrors the systemd branch's "draining (up to Ns)..." line.
# Graceful in-band restart, mirroring the systemd branch.
#
# Previously this sent a bare SIGTERM and waited
# ``_get_restart_drain_timeout()`` — which defaults to 0, so the
# wait could never succeed and every restart fell through to
# ``kickstart -k``. A bare SIGTERM also leaves
# ``restart_requested`` False, so the gateway exits 1 instead of
# 75 and reports itself to chat as "shutting down" rather than
# "restarting", losing the resume_pending handoff.
#
# SIGUSR1 is the drain-aware path: refuse new turns, wait for
# in-flight work (``agent.restart_after_turn_timeout``), then
# stop() within ``agent.restart_drain_timeout``. The wait budget
# must cover BOTH phases plus headroom (#77184) — the raw drain
# timeout covers only the second.
#
# Announce the wait BEFORE it runs: it can last the full budget
# while the old gateway finishes in-flight agent runs, and it
# streams into surfaces with no other feedback — the desktop
# updater's live output most of all, where a silent stop here
# reads as "update stuck" (#44515).
wait_budget = _get_restart_exit_wait_budget()
print(
f"→ Stopping gateway (PID {pid}) — draining in-flight runs "
f"(up to {drain_timeout:.0f}s)..."
f"(up to {wait_budget:.0f}s)..."
)
if _graceful_restart_via_sigusr1(pid, wait_budget):
# The gateway exited with the planned-restart code and
# launchd's unconditional KeepAlive revives it. Do NOT
# kickstart here: the replacement may already be up, and
# ``-k`` would kill it and restart a second time.
print("✓ Service restart requested")
_clear_launchd_unsupported_marker()
return
print(
f"⚠ Gateway drain timed out after {wait_budget:.0f}s — forcing launchd restart"
)
try:
terminate_pid(pid, force=False)
except (ProcessLookupError, PermissionError, OSError):
pid = None
if pid is not None:
exited = _wait_for_gateway_exit(timeout=drain_timeout, force_after=None)
if not exited:
print(
f"⚠ Gateway drain timed out after {drain_timeout:.0f}s — forcing launchd restart"
)
subprocess.run(["launchctl", "kickstart", "-k", target], check=True, timeout=90)
print("✓ Service restarted")
_clear_launchd_unsupported_marker()
tests/hermes_cli/test_gateway_service.py
+52
View File
@@ -920,8 +920,60 @@ class TestGatewaySystemServiceRouting:
assert result is False
assert replacement_observed == [True]
def test_launchd_restart_uses_sigusr1_and_exit_wait_budget(self, monkeypatch, capsys):
"""launchd_restart must take the same graceful path as systemd_restart.
Regression: it previously sent a bare SIGTERM and waited
``_get_restart_drain_timeout()`` (default 0), so the wait could never
succeed and every restart fell through to ``kickstart -k``. A bare
SIGTERM leaves ``restart_requested`` False, so the gateway exits 1
instead of 75 and announces itself as "shutting down" rather than
"restarting", dropping the resume_pending handoff.
"""
calls = []
monkeypatch.setattr(gateway_cli, "get_launchd_label", lambda: "ai.hermes.gateway")
monkeypatch.setattr(gateway_cli, "_launchd_domain", lambda: "gui/501")
monkeypatch.setattr("gateway.status.get_running_pid", lambda *a, **k: 654)
monkeypatch.setattr(gateway_cli, "_request_gateway_self_restart", lambda pid: False)
monkeypatch.setattr(
gateway_cli,
"probe_gateway_loop_liveness",
lambda pid, **kw: gateway_cli.GATEWAY_LOOP_ALIVE,
)
# Wait budget covers after-turn deferral + drain + headroom (#77184);
# the raw drain timeout (0 by default) must not be used here.
monkeypatch.setattr(gateway_cli, "_get_restart_drain_timeout", lambda: 0.0)
monkeypatch.setattr(gateway_cli, "_get_restart_exit_wait_budget", lambda: 27.0)
monkeypatch.setattr(
gateway_cli,
"_graceful_restart_via_sigusr1",
lambda pid, timeout: calls.append(("graceful", pid, timeout)) or True,
)
monkeypatch.setattr(
gateway_cli,
"terminate_pid",
lambda pid, force=False: calls.append(("sigterm", pid)),
)
monkeypatch.setattr(
gateway_cli.subprocess,
"run",
lambda *a, **k: calls.append(("kickstart", a[0])) or SimpleNamespace(
returncode=0, stdout="", stderr=""
),
)
monkeypatch.setattr(gateway_cli, "_clear_launchd_unsupported_marker", lambda: None)
gateway_cli.launchd_restart()
assert ("graceful", 654, 27.0) in calls
# A bare SIGTERM would strand the gateway on the unplanned-shutdown path.
assert not any(call[0] == "sigterm" for call in calls)
# ``-k`` after a successful graceful exit would kill the replacement.
assert not any(call[0] == "kickstart" for call in calls)
out = capsys.readouterr().out
assert "27" in out
assert "up to 0s" not in out
tests/hermes_cli/test_update_wedged_gateway.py
+7 -4
View File
@@ -193,6 +193,8 @@ class TestLaunchdRestartWedgedIntegration:
monkeypatch.setattr(gateway_cli, "get_launchd_label", lambda: "ai.hermes.gateway")
monkeypatch.setattr(gateway_cli, "_launchd_domain", lambda: "gui/501")
monkeypatch.setattr(gateway_cli, "_get_restart_drain_timeout", lambda: 180.0)
# Wait budget covers after-turn deferral + drain + headroom (#77184).
monkeypatch.setattr(gateway_cli, "_get_restart_exit_wait_budget", lambda: 195.0)
monkeypatch.setattr("gateway.status.get_running_pid", lambda *a, **k: 4242)
monkeypatch.setattr(
gateway_cli, "_request_gateway_self_restart", lambda pid: False
@@ -212,10 +214,11 @@ class TestLaunchdRestartWedgedIntegration:
"terminate_pid",
lambda pid, force=False: events.append("sigterm"),
)
# Never let a real SIGUSR1 escape to PID 4242 during tests.
monkeypatch.setattr(
gateway_cli,
"_wait_for_gateway_exit",
lambda timeout, force_after=None: events.append(("drain", timeout)) or True,
"_graceful_restart_via_sigusr1",
lambda pid, timeout: events.append(("drain", pid, timeout)) or True,
)
monkeypatch.setattr(
gateway_cli.subprocess,
@@ -241,11 +244,11 @@ class TestLaunchdRestartWedgedIntegration:
events = self._setup(monkeypatch, gateway_cli.GATEWAY_LOOP_ALIVE)
gateway_cli.launchd_restart()
assert "escalate" not in events
assert ("drain", 180.0) in events
assert ("drain", 4242, 195.0) in events
def test_unknown_liveness_keeps_full_drain_budget(self, monkeypatch):
"""Ambiguity (no heartbeat) must never trigger escalation."""
events = self._setup(monkeypatch, gateway_cli.GATEWAY_LOOP_UNKNOWN)
gateway_cli.launchd_restart()
assert "escalate" not in events
assert ("drain", 180.0) in events
assert ("drain", 4242, 195.0) in events