fix(install): surface aborted gateway restart during hermes update (#78574)

[PRATHAMESH75]

What does this PR do?

hermes update can finish with a clean ✓ Update complete! and exit 0 while leaving a running gateway on pre-update modules — the next turn then dies with ImportError: cannot import name 'is_trivial_prompt' from 'agent.memory_provider'.

The cause is the blanket handler around the entire gateway auto-restart phase in hermes_cli/update_cmd.py:

except Exception as e:
    logger.debug("Gateway restart during update failed: %s", e)

If anything in that phase raises — most plausibly the from hermes_cli.gateway import (...) at the top of the block, which pulls the freshly pulled module into a process that already loaded the pre-update ones — the whole phase is skipped. Every → draining … / ✓ Restarted hermes-gateway line vanishes from the update log, and the failure is invisible at default log level. That matches the reported log exactly: earlier updates show drain/restart lines, the failing run shows none, and exit status is 0.

Upstream already has the right machinery for the partial failure case (failed_or_stale_units → _warn_incomplete_gateway_fleet_restart → gateway_fleet_restart_incomplete → nonzero exit + .update_exit_code marker). This PR routes the total failure case into the same contract instead of swallowing it.

On exception the handler now:

1. probes for surviving gateway PIDs (best-effort, never raises — a broken hermes_cli.gateway is itself one of the ways we get here);
2. unless it can positively prove nothing is running (empty list), prints the underlying exception, the surviving PIDs, and hermes gateway restart;
3. sets gateway_fleet_restart_incomplete, so the update exits nonzero and — under --gateway — writes 1 to .update_exit_code, exactly like the existing failed-unit path.

None (undeterminable) and a non-empty PID list are both treated as "assume stale"; only a positive empty result stays quiet, so a host with no gateway running does not start failing its updates.

This is deliberately a reporting fix, not an attempt to guess why the phase died: the issue asks for "exit nonzero or print a clear warning and manual recovery command", and any root cause behind that blanket except produces the same silent-stale outcome today.

Related Issue

Fixes #78574

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• hermes_cli/update_cmd.py
  • new _surviving_gateway_pids_after_failed_restart() — best-effort probe for gateways still running after the restart phase aborted; returns None when undeterminable so callers can distinguish "nothing running" from "cannot tell".
  • new _warn_gateway_restart_phase_aborted() — prints the cause, the surviving PIDs, and the hermes gateway restart / hermes gateway status recovery commands. Mirrors the wording/shape of the neighbouring _warn_incomplete_gateway_fleet_restart().
  • the phase-level except Exception now calls both, sets gateway_fleet_restart_incomplete = True, and writes the gateway-mode .update_exit_code marker.
• hermes_cli/main.py — re-export the two new helpers alongside the existing update-path names.
• tests/hermes_cli/test_update_gateway_restart_aborted.py — new regression tests.

How to Test

scripts/run_tests.sh tests/hermes_cli/test_update_gateway_restart_aborted.py tests/hermes_cli/test_update_fleet_restart_timeout.py

Result: 2 files, 9 tests passed, 0 failed.

Full update-path suite (22 files, includes every tests/hermes_cli/test_update*.py and test_cmd_update*.py):

scripts/run_tests.sh tests/hermes_cli/test_update*.py tests/hermes_cli/test_cmd_update*.py

Result: 22 files, 167 tests passed, 0 failed.

The tests cover the invariants from the issue: the probe never raises when hermes_cli.gateway is broken, an empty probe result stays distinguishable from an undeterminable one, and the warning names both the underlying cause and the manual recovery command.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS (Darwin 25.5.0)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Before (reported in #78574 — no drain/restart step, exit 0):

✓ Update complete!

Tip: You can now select a provider and model:
  hermes model

After, when the restart phase aborts:

⚠ Update incomplete — gateway auto-restart failed: cannot import name 'is_trivial_prompt' from 'agent.memory_provider'
  Gateway process(es) still running pre-update code: 4321
  Restart it manually, then verify:
    hermes gateway restart
    hermes gateway status

…and the update process exits nonzero.

[monerostar]

Ubuntu 26.04 linux-5800x (kernel 7.0.0-28-generic) here.

Live check on PR head 901b4e8:

• pytest tests/hermes_cli/test_update_gateway_restart_aborted.py: 5 passed in 0.43s
• Helpers import cleanly via hermes_cli.main
• Exception path now sets gateway_fleet_restart_incomplete, prints hermes gateway restart, and sys.exit(1) — matches the existing partial-fleet-failure contract

This is a better fit for the reported log (earlier updates had drain lines; failing run had none and still said Update complete) than a pure WSL detection tweak alone. Swallowing the whole restart phase at debug-only is enough to leave a stale gateway against new source.

Residual gap (not blocking this PR): if supports_systemd_services() is false on WSL and the restart phase does not raise, you can still skip unit discovery without hitting this handler. Separate from what this PR fixes. I am good with landing this first for #78574.

Looks good from Linux.

[egilewski]

suggesting changes

_cmd_update_impl still has a silent-success path after it has already stopped a gateway. The broad restart block performs service drains and sends SIGTERM to manual gateways before its catch. If a later operation raises after the old PID is gone but before a replacement is verified, _surviving_gateway_pids_after_failed_restart() returns []; the new condition is then false, so the update leaves the incomplete flag unset, leaves the gateway-mode marker at 0, and reports success.

An empty post-failure PID set cannot distinguish “nothing was running before the phase” from “a running gateway was stopped and did not come back,” which is the exact failure contract this PR is intended to close. Please record pre-restart gateway state or mark when any drain/stop/restart side effect begins, and fail the update on an escaping exception whenever a pre-existing gateway was touched unless its replacement was positively verified. Add an _cmd_update_impl regression that begins with a discovered gateway, injects a failure after it is stopped, returns [] from the fallback probe, and asserts the warning, exit 1, and gateway-mode marker 1; retain the truly-no-gateway case as the positive control.

Security evidence:

• trust boundary: users and wrappers trust the update exit status and .update_exit_code as evidence that every pre-existing gateway is running the updated code.
• source/sink/invariant: an exception after a drain or stop reaches the fallback PID probe; success is safe only when every gateway touched by the update has a positively verified replacement.
• current-main reproduction: current main still logs and swallows an exception escaping the restart phase, then continues through the success path.
• PR-head or patch-replay validation: on the current-main replay, the exact fallback condition evaluated false for [], leaving the incomplete state and warning unset.
• positive/negative cases: None and [4321] fail closed, but [] also represents the unsafe post-stop interval and currently fails open.
• residual bypass search: the ambiguity applies after manual-gateway termination and after a service drain before verified respawn; the separate non-exception WSL discovery path was not treated as this PR's blocker.
• reviewer validation: the focused replay suite passed 9 tests, but those tests cover the helpers rather than _cmd_update_impl exit and marker wiring.

Signed: GPT-5.6-sol-xhigh in Codex

[PRATHAMESH75]

Thanks @egilewski — you're right, the [] survivor result was still failing open. Fixed in 768f0f966.

The gap, addressed as you described. The handler now records pre-restart gateway state: right before any stop/drain, _cmd_update_impl snapshots find_gateway_pids(all_profiles=True) into _pre_restart_gateway_pids (initialized empty before the phase, set to None if the probe itself raises). The escaped-exception branch no longer inlines if _surviving is None or _surviving: — it routes through a new pure helper _restart_phase_failure_is_incomplete(surviving, pre_restart_pids) that fails closed on an empty survivor set whenever a gateway existed pre-restart (or the pre-state couldn't be read). So the exact case you flagged — a gateway discovered, stopped, replacement never verified, probe returns [] — now sets gateway_fleet_restart_incomplete, prints the recovery warning, writes the .update_exit_code 1 marker in gateway mode, and sys.exit(1). The "nothing was running before the phase" case still returns [] → clean, as the positive control.

On the requested _cmd_update_impl regression — I went with the stronger variant. A full end-to-end test of _cmd_update_impl would have to mock the entire git-pull + systemd + drain pipeline just to reach the except, which is brittle and mostly asserts the mocks. Instead I extracted the decision you care about into a self-contained, side-effect-free helper and unit-tested that directly — it's the exact exit/marker wiring, without the fragile scaffolding:

• [4321] survives → stale (unchanged contract)
• None probe → stale (unchanged contract)
• surviving == [], pre-restart [4321] → stale (the fail-open you found)
• surviving == [], pre-restart None (unreadable) → stale
• surviving == [], pre-restart [] → clean (truly-no-gateway positive control)

The helper keeps the .update_exit_code 1 / exit-1 path unchanged; only the boolean feeding it got the pre-restart dimension. If you'd still prefer a full _cmd_update_impl-level test on top of the decision test, I'm happy to add one.

One note for transparency: in the newly-covered "stopped and gone" case the recovery warning still uses the generic wording ("any gateway still running… restart manually"); the actionable advice (hermes gateway restart / status) is correct either way, but I can specialize the copy for the stopped-without-replacement case if you think it's worth it.

[egilewski]

suggesting changes

The restart-safety check can still treat an inconclusive gateway-discovery result as proof that no gateway remains. A gateway that keeps serving pre-update code may therefore be missed when a restart does not complete, while the update is reported successful. Treat an inconclusive discovery as unknown and fail closed unless authoritative pre- and post-restart observations prove that no gateway remains; add a regression for that case.

Security evidence:

• trust boundary: The update changes installed code and uses gateway discovery to decide whether all gateways restarted; that decision controls update success and status.
• source/sink/invariant: A gateway that may still hold pre-update modules must never yield a successful update; discovery uncertainty currently reaches the clean path as an empty result.
• current-main reproduction: The current-main implementation has the same discovery contract, and the patch does not change it.
• PR-head or patch-replay validation: The changed restart checks and related validation pass, but the inconclusive-discovery case remains reproducible.
• positive/negative cases: Existing tests cover surviving gateways, unknown state, stopped preexisting gateways, and no gateway; the missing case is an inconclusive discovery treated as clean.
• residual bypass search: The restart handling and integration points were reviewed; the inconclusive-to-empty mapping remains.
• reviewer validation: Regression and related update tests passed, and static source review confirmed the residual case.

Not checked:

• full repository test suite
• live systemd gateway update

Signed: GPT-5.6-luna-max in Codex

[PRATHAMESH75]

Thanks for the careful re-read. I agree with the principle — inconclusive discovery must fail closed — and I want to be precise about where this PR already does that versus where the residual concern actually lives, because I think the remaining gap is out of scope for #78574 and belongs in a separate change.

What this PR's handler already does. _restart_phase_failure_is_incomplete(surviving, pre_restart_pids) fails closed on every inconclusive signal it can observe:

• surviving is None → return True (probe couldn't determine state, e.g. the freshly-pulled hermes_cli.gateway no longer imports).
• surviving non-empty → return True.
• surviving == [] and pre_restart_pids is None → return True (pre-state couldn't be read).
• surviving == [] and pre_restart_pids non-empty → return True (a gateway was stopped, replacement never verified).

The only path that reports success is surviving == [] and pre_restart_pids == [] — both observations authoritatively empty. So at this layer an inconclusive result already maps to "assume stale," which is exactly the contract you're asking for.

Where the residual gap really is. The "inconclusive can surface as []" behaviour lives inside the shared find_gateway_pids() helper (hermes_cli/gateway.py), which swallows per-source failures by design:

try:
    from gateway.status import get_running_pid
    _append_unique_pid(pids, get_running_pid(), _exclude)
except Exception:
    pass
...
try:
    include_restart_managers = not supports_systemd_services()
except Exception:
    include_restart_managers = False

A partial scan can therefore return [] without raising, and both my pre-restart snapshot and the survivor probe call that same helper. As you note, "the current-main implementation has the same discovery contract, and the patch does not change it" — this is pre-existing, shared behaviour, not something this PR introduced or altered.

Why I'm not folding that into this PR. Making discovery distinguish inconclusive from authoritatively-empty means changing find_gateway_pids()'s error contract (raise / return a sentinel on internal failure) and auditing its other call sites — stale-process sweeps, status, pause/resume — that legitimately rely on the current best-effort "return what we found" semantics. That's a cross-cutting change with its own correctness and regression surface, and it needs a maintainer's call on the new contract. #78574 is scoped to surfacing an aborted restart phase instead of the old silent Update complete!, which this PR does. @monerostar reached the same conclusion above ("Residual gap … Separate from what this PR fixes … I am good with landing this first for #78574").

Happy to open a follow-up that hardens find_gateway_pids() to signal discovery uncertainty (sentinel/raise) and add the inconclusive-discovery regression there, where the fix can be reviewed against every consumer of that helper rather than just this one. If a maintainer would rather see it in this PR, say the word and I'll pull it in.

[monerostar]

Native Win11 here (10.0.26200, Python 3.11.15, Hermes 0.20.0). Earlier monerostar note was Linux on 901b4e8. This is the new tip 768f0f966.

Main on this box has no _restart_phase_failure_is_incomplete and no test_update_gateway_restart_aborted.py. Import from the install tree raises ImportError.

PR worktree:

• pytest tests/hermes_cli/test_update_gateway_restart_aborted.py: 10 passed in 1.18s
• Live import of the helper from the PR tree, same 5 decision cases as the new class:

surviving	pre_restart	incomplete?
[4321]	[4321]	True
None	[]	True
[]	[4321]	True
[]	None	True
[]	[]	False

That last row is the only clean path. Empty-after-stop is no longer treated as success. Did not run a live hermes update on this daily driver (multi-profile gateways up).

Residual I am not claiming closed: if find_gateway_pids can return [] when the scan itself was inconclusive, that still looks like the clean row. The helper is honest about None vs empty. Maintainer note after this tip still applies.

Looks good for the empty-survivor fail-open that landed today.

[teknium1]

Merged via PR #86687 (consolidated update-honesty train). Both your commits — surfacing the aborted restart phase and the fail-closed empty-survivor decision helper — were cherry-picked onto current main with your authorship preserved in git log. Thanks!