mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(gateway): revive gateway on /restart under Restart=on-failure units
The in-chat /restart command was leaving the gateway dead on systemd
deployments using Restart=on-failure (the default for many
operator-managed and tutorial-style unit files). The gateway drained,
exited cleanly (code 0), and was never revived — the only recovery was
a host reboot.
Root cause was a multi-layer assumption mismatch:
1. gateway/run.py:_stop_impl assumed all systemd units use
Restart=always, so the Linux/systemd branch returned exit code 0
and relied on a `systemd-run` transient helper to restart the unit
immediately. Units with Restart=on-failure never see a clean exit
as a trigger, so nothing revived the process.
2. gateway/run.py:_launch_systemd_restart_shortcut hardcoded
`--user` scope, so it could not even locate the unit PID on
system-level deployments (the common case for
/etc/systemd/system/hermes-gateway.service). It silently returned
without launching the helper.
3. Even after the scope detection was fixed, the helper could not
actually start: non-root gateway units (User=ubunutu) hit a Polkit
denial on `systemd-run --system` ("Interactive authentication
required"), and `--user` requires a D-Bus user session that is
typically absent on headless servers.
The fix is two-fold:
* `_stop_impl` now always exits with GATEWAY_SERVICE_RESTART_EXIT_CODE
(75 / EX_TEMPFAIL) on service-managed restarts, regardless of
platform. Combined with RestartForceExitStatus=75 in the unit file,
systemd treats the planned restart as a controlled failure and
revives the gateway via Restart=on-failure, with RestartSec as the
only delay. The planned-restart helper is still attempted (for
RestartSec=0 setups that want sub-second restarts) but is no longer
load-bearing.
* `_launch_systemd_restart_shortcut` now probes both system and user
scopes via MainPID equality and uses whichever scope actually owns
the gateway process. It bails out safely if neither matches.
StartLimitBurst in the unit file still bounds accidental restart
loops, and the macOS launchd path is unchanged.
Verified end-to-end on Ubuntu 24.04 with hermes-gateway as a
/etc/systemd/system/... service running under User=ubunutu. The
unit uses Restart=on-failure, RestartSec=30, RestartForceExitStatus=75,
StartLimitIntervalSec=600, StartLimitBurst=5. /restart from Feishu now
drains cleanly, exits 75, and the gateway is back online ~30s later
without manual intervention.
Tests: tests/gateway/test_gateway_shutdown.py renamed the affected
case to test_gateway_stop_systemd_service_restart_uses_tempfail and
now asserts exit_code == GATEWAY_SERVICE_RESTART_EXIT_CODE.
14/14 tests in this module pass.
This commit is contained in:
parent
0a75616514
commit
40dbfa0e3c
2 changed files with 59 additions and 32 deletions
|
|
@ -5859,34 +5859,51 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
service_name = "hermes-gateway"
|
||||
|
||||
current_pid = os.getpid()
|
||||
show = subprocess.run(
|
||||
[
|
||||
systemctl,
|
||||
"--user",
|
||||
"show",
|
||||
service_name,
|
||||
"--property=MainPID",
|
||||
"--value",
|
||||
],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=2,
|
||||
)
|
||||
if (show.stdout or "").strip() != str(current_pid):
|
||||
|
||||
# Detect whether the gateway unit is registered as a system or
|
||||
# user service. Daemon-style deployments are typically system
|
||||
# units (e.g. /etc/systemd/system/hermes-gateway.service), while
|
||||
# `hermes setup` under a non-root account may register a user
|
||||
# unit. Hard-coding ``--user`` broke system-unit deployments:
|
||||
# systemctl returned an empty MainPID, the PID-equality check
|
||||
# below failed, and the planned-restart helper was never
|
||||
# launched — leaving the gateway dead until a manual reboot.
|
||||
def _query_pid(scope_flags):
|
||||
try:
|
||||
out = subprocess.run(
|
||||
[systemctl, *scope_flags, "show", service_name,
|
||||
"--property=MainPID", "--value"],
|
||||
capture_output=True, text=True, timeout=2,
|
||||
)
|
||||
return (out.stdout or "").strip()
|
||||
except Exception:
|
||||
return ""
|
||||
|
||||
system_pid = _query_pid([])
|
||||
user_pid = _query_pid(["--user"])
|
||||
if str(current_pid) == system_pid:
|
||||
scope_flags = []
|
||||
systemctl_scope = "systemctl"
|
||||
elif str(current_pid) == user_pid:
|
||||
scope_flags = ["--user"]
|
||||
systemctl_scope = "systemctl --user"
|
||||
else:
|
||||
# MainPID does not match in either scope — likely invoked
|
||||
# outside of systemd or the unit was renamed. Bail out
|
||||
# rather than restart the wrong unit.
|
||||
return
|
||||
|
||||
systemctl_user = "systemctl --user"
|
||||
service_arg = shlex.quote(service_name)
|
||||
shell_cmd = (
|
||||
f"while kill -0 {current_pid} 2>/dev/null; do sleep 0.2; done; "
|
||||
f"{systemctl_user} reset-failed {service_arg}; "
|
||||
f"{systemctl_user} restart {service_arg}"
|
||||
f"{systemctl_scope} reset-failed {service_arg}; "
|
||||
f"{systemctl_scope} restart {service_arg}"
|
||||
)
|
||||
unit_name = f"{service_name}-planned-restart-{current_pid}".replace(".", "-")
|
||||
subprocess.Popen(
|
||||
[
|
||||
systemd_run,
|
||||
"--user",
|
||||
*scope_flags,
|
||||
"--collect",
|
||||
"--unit",
|
||||
unit_name,
|
||||
|
|
@ -5899,9 +5916,10 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
start_new_session=True,
|
||||
)
|
||||
logger.info(
|
||||
"Launched systemd planned-restart helper for %s (pid=%s)",
|
||||
"Launched systemd planned-restart helper for %s (pid=%s, scope=%s)",
|
||||
service_name,
|
||||
current_pid,
|
||||
"user" if scope_flags else "system",
|
||||
)
|
||||
except Exception as e:
|
||||
logger.debug("Failed to launch systemd planned-restart helper: %s", e)
|
||||
|
|
@ -7871,17 +7889,21 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
|
||||
if self._restart_requested and self._restart_via_service:
|
||||
self._launch_systemd_restart_shortcut()
|
||||
# systemd units use Restart=always, so a planned restart should
|
||||
# exit cleanly and still be relaunched. Using TEMPFAIL here
|
||||
# makes systemd treat the operator-requested restart as a
|
||||
# failure and can trip stepped restart backoff. launchd's
|
||||
# KeepAlive.SuccessfulExit=false needs a non-zero exit to
|
||||
# relaunch, so keep the old code on macOS.
|
||||
self._exit_code = (
|
||||
GATEWAY_SERVICE_RESTART_EXIT_CODE
|
||||
if sys.platform == "darwin" or not os.environ.get("INVOCATION_ID")
|
||||
else 0
|
||||
)
|
||||
# Always exit with TEMPFAIL (75) on service-managed
|
||||
# restarts. The shortcut helper above is best-effort and
|
||||
# commonly fails on real deployments: non-root gateway
|
||||
# units hit Polkit denials when invoking ``systemd-run
|
||||
# --system``, headless boxes have no user bus for
|
||||
# ``--user``, and operator-managed unit files may use
|
||||
# ``Restart=on-failure`` rather than ``Restart=always``.
|
||||
# Exit 75 paired with ``RestartForceExitStatus=75`` makes
|
||||
# systemd treat the planned restart as a controlled
|
||||
# failure and revive the unit via ``Restart=on-failure``,
|
||||
# regardless of whether the helper survived. Without
|
||||
# this, a clean exit (0) on Linux left the gateway dead
|
||||
# until someone rebooted the host. ``StartLimitBurst``
|
||||
# in the unit file still bounds accidental loops.
|
||||
self._exit_code = GATEWAY_SERVICE_RESTART_EXIT_CODE
|
||||
self._exit_reason = self._exit_reason or "Gateway restart requested"
|
||||
|
||||
self._draining = False
|
||||
|
|
|
|||
|
|
@ -138,7 +138,7 @@ async def test_gateway_stop_interrupts_after_drain_timeout():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gateway_stop_systemd_service_restart_exits_cleanly(tmp_path, monkeypatch):
|
||||
async def test_gateway_stop_systemd_service_restart_uses_tempfail(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path)
|
||||
runner, adapter = make_restart_runner()
|
||||
adapter.disconnect = AsyncMock()
|
||||
|
|
@ -149,7 +149,12 @@ async def test_gateway_stop_systemd_service_restart_exits_cleanly(tmp_path, monk
|
|||
await runner.stop(restart=True, service_restart=True)
|
||||
|
||||
runner._launch_systemd_restart_shortcut.assert_called_once_with()
|
||||
assert runner._exit_code == 0
|
||||
# Exit 75 (EX_TEMPFAIL) so RestartForceExitStatus=75 in the unit
|
||||
# file revives the gateway via Restart=on-failure, even when the
|
||||
# planned-restart helper fails (Polkit denial, missing user bus,
|
||||
# headless box, or operator-managed unit using on-failure instead
|
||||
# of always). StartLimitBurst still bounds accidental loops.
|
||||
assert runner._exit_code == GATEWAY_SERVICE_RESTART_EXIT_CODE
|
||||
assert (tmp_path / ".restart_pending.json").exists()
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue