Skip to content

fix(gateway): revive gateway on /restart under Restart=on-failure units - #50204

Closed
randomuser2026x wants to merge 1 commit into
NousResearch:mainfrom
randomuser2026x:fix/gateway-systemd-restart-on-failure
Closed

randomuser2026x wants to merge 1 commit into
NousResearch:mainfrom
randomuser2026x:fix/gateway-systemd-restart-on-failure

Conversation

@randomuser2026x

Copy link
Copy Markdown
Contributor

Problem

The in-chat /restart command leaves the gateway dead on systemd deployments whose unit file uses Restart=on-failure (the most common setup recommended in deployment guides and used by hermes setup on Ubuntu). The gateway drains, exits cleanly, and never comes back — the only recovery is a host reboot.

Confirmed reproduction: operator-managed unit file at `/etc/systemd/system/hermes-gateway.service` with `User=ubunutu`, `Restart=on-failure`, `RestartForceExitStatus=75`. After `/restart` from Feishu the gateway stays down indefinitely until someone runs `sudo systemctl restart hermes-gateway` or reboots the host.

Root cause

Three independent bugs in series, any one of which breaks `/restart` on affected deployments:

  1. Wrong exit-code assumption (`gateway/run.py` `_stop_impl`): the Linux/systemd branch returned exit code `0` under the assumption that all systemd units use `Restart=always` and would relaunch on a clean exit. Units with `Restart=on-failure` never see `0` as a trigger, so systemd does nothing.

  2. Hardcoded `--user` scope (`gateway/run.py` `_launch_systemd_restart_shortcut`): the planned-restart helper assumed the unit was registered as a user service. On system-level deployments (the common case) `systemctl --user show hermes-gateway` returns an empty `MainPID`, the PID-equality check fails, and the helper silently returns without launching anything.

  3. Polkit denial on `systemd-run --system` (deployment reality): non-root gateway units (e.g. `User=ubunutu`) invoke `systemd-run --system` to create the transient restart helper, but Polkit rejects it with `Interactive authentication required`. The `--user` fallback requires a D-Bus user session that is absent on headless servers. So even after the scope bug above is fixed, the helper still cannot start in real deployments.

Fix

Always exit `75` (EX_TEMPFAIL) on service-managed restarts. Combined with the existing `RestartForceExitStatus=75` directive 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.

```diff

  • self._exit_code = (
  • GATEWAY_SERVICE_RESTART_EXIT_CODE
    
  • if sys.platform == "darwin" or not os.environ.get("INVOCATION_ID")
    
  • else 0
    
  • )
  • self._exit_code = GATEWAY_SERVICE_RESTART_EXIT_CODE
    ```

The planned-restart helper is still attempted (for setups that want sub-`RestartSec` restarts), but it is no longer load-bearing. If `systemd-run` is denied by Polkit or the user bus is missing, the helper silently fails and systemd's own `Restart=` machinery takes over.

A second commit also fixes the scope detection in `_launch_systemd_restart_shortcut` so the helper (when it does work) targets the correct scope:

```python
system_pid = _query_pid([]) # systemctl show ...
user_pid = _query_pid(["--user"])

if current_pid == system_pid: scope = [] # /etc/systemd/system
elif current_pid == user_pid: scope = ["--user"] # systemctl --user
else: return # not systemd
```

Behavior matrix after fix

Unit file Helper outcome Gateway revived by
`Restart=always`, root gateway ✓ sub-second helper or systemd
`Restart=on-failure`, root gateway ✓ sub-second helper or systemd (exit 75)
`Restart=on-failure`, non-root gateway ✗ Polkit denied systemd (exit 75)
Headless without D-Bus user session ✗ no user bus systemd (exit 75)
Not under systemd n/a existing non-systemd paths

Safety

  • `StartLimitBurst` / `StartLimitIntervalSec` in the unit file still bound accidental restart loops.
  • macOS launchd path is unchanged (`KeepAlive.SuccessfulExit=false`).
  • No impact on crash recovery: crashes still produce the same exit codes they always did.

Verification

End-to-end on Ubuntu 24.04 with hermes-gateway as a `/etc/systemd/system/` service under `User=ubunutu`, using `Restart=on-failure`, `RestartSec=30`, `RestartForceExitStatus=75`, `StartLimitIntervalSec=600`, `StartLimitBurst=5`.

  • `/restart` from Feishu: gateway drains in <1s, exits 75, systemd revives it ~30s later. No manual intervention.
  • Repeated 5× in quick succession: works correctly, then `start-limit-hit` kicks in as expected.
  • After `systemctl reset-failed`: `/restart` works again.

Tests

`tests/gateway/test_gateway_shutdown.py`:

  • Renamed `test_gateway_stop_systemd_service_restart_exits_cleanly` → `test_gateway_stop_systemd_service_restart_uses_tempfail`.
  • Asserts `_exit_code == GATEWAY_SERVICE_RESTART_EXIT_CODE` (75) instead of `0`.

```
$ pytest tests/gateway/test_gateway_shutdown.py
14 passed in 16.58s
```

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.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles labels Jun 21, 2026
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jun 21, 2026
@benschewel

Copy link
Copy Markdown

Independent root-cause analysis on a separate systemd deployment reached the same conclusion, and we verified this fix end-to-end.

Evidence from our incident (system unit with Restart=on-failure):

  • The regression is commit b14e15c48e ("clean service restart notifications"), which switched the planned-restart exit from 75 to 0 on the assumption that units use Restart=always. Our unit is Restart=on-failure, so the clean exit-0 was treated as success and the gateway stayed down (~48 min until a manual systemctl restart).
  • The exit-diag log confirmed the incident was the first-ever Linux SystemExit: 0; every prior restart exited 75 and relaunched fine.
  • _launch_systemd_restart_shortcut() queries the --user scope, which no-ops on a system unit, so it couldn't compensate — exactly what the _query_pid(scope_flags) change here addresses.

We applied the same core change (self._exit_code = GATEWAY_SERVICE_RESTART_EXIT_CODE) as a hotfix and verified on a real box: SIGUSR1 → process exits 75 → Restart=on-failure relaunches within RestartSec → Telegram reconnects, NRestarts=1. Works. +1 to merging.

One note for anyone mirroring the unit change: SuccessExitStatus=75 would be the wrong directive under Restart=on-failure — it reclassifies 75 as success and would skip the relaunch. RestartForceExitStatus=75 as used here is correct.

teknium1 added a commit that referenced this pull request Jul 1, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
teknium1 added a commit that referenced this pull request Jul 1, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
teknium1 added a commit that referenced this pull request Jul 1, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Merged via #56362 (commit 2f167a2) — cherry-picked onto current main with your authorship preserved in git log. Thanks for the fix!

The salvage probes both systemctl show and systemctl --user show to pick the right scope (system-unit deployments were silently no-op'ing on the hardcoded --user), and always exits 75 so RestartForceExitStatus=75 revives the unit under Restart=on-failure too, not just Restart=always. A small follow-up commit corrected a comment (Hermes-generated units set StartLimitIntervalSec=0, so StartLimitBurst doesn't bound loops) and added the AUTHOR_MAP entry.

@teknium1 teknium1 closed this Jul 1, 2026
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
Jasper6439 pushed a commit to Jasper6439/hermes-agent that referenced this pull request Jul 5, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
- Correct the exit-75 comment: Hermes-generated units set
  StartLimitIntervalSec=0 (rate limiting disabled), so StartLimitBurst
  does not bound loops. The real bound is that genuine crashes exit
  non-zero-but-not-75, and RestartForceExitStatus=75 only whitelists
  the planned code.
- Add randomuser2026x AUTHOR_MAP entry (CI blocks unmapped emails).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants