Skip to content

fix(gateway): hard-exit after graceful shutdown so a wedged worker can't strand the gateway - #140

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/gateway-shutdown-force-exit
Jul 1, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
fix/gateway-shutdown-force-exit

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Symptom

Restarting the gateway occasionally left it "down but not exited" for ~2.5 minutes (measured: a 149s blackout on one restart), during which launchd/systemd could not relaunch it — the old process was still alive, so the supervisor wouldn't spawn a new one. Most restarts are sub-second; this is intermittent.

Root cause (measured, not theorized)

From gateway-exit-diag.log for the slow restart:

  • asyncio.run.returned at T+0 — async teardown done in ~6s (adapters disconnected, SessionDB closed, sessions persisted)
  • atexit.hook at T+149s — the interpreter hung for 2.5 min after the loop returned

The gateway runs each agent turn on a ThreadPoolExecutor (gateway/run.py). In CPython, those workers are daemon=False, and concurrent.futures registers an atexit hook that joins every worker thread before the interpreter can exit. If an agent turn was still executing in a worker at SIGTERM, the process blocked at interpreter finalization until that in-flight turn completed. Verified empirically on 3.11.15: ThreadPoolExecutor worker daemon=False, concurrent.futures.thread._python_exit atexit-join present.

Across 185 recorded shutdowns the hang fired once (149s); every other exit was sub-second — it needs a worker mid-blocking-call at the SIGTERM instant.

gateway/run.py::main() already force-exits for exactly this reason (_exit_after_graceful_shutdown), but production runs via hermes_cli/gateway.py::run_gateway(), which lacked the backstop.

Fix

Apply the same graceful-teardown-then-hard-exit pattern to the production path, plus diagnostics.

  • hermes_cli/gateway.py: run_gateway() force-exits via a new _force_process_exit() at both the clean (0) and failure (1) tails, after start_gateway()'s graceful teardown. Nothing of value is lost — the graceful work already ran; we hard-exit the husk. The helper flushes stdio and logs any lingering non-daemon threads (greppable force-exit: lingering non-daemon ...) so the next stall is diagnosable at a glance. Wrapped in a helper so tests neutralize os._exit.
  • gateway/run.py: _shutdown_executor() now does a bounded drain (_executor_drain_timeout(), default 8s, HERMES_GATEWAY_EXECUTOR_DRAIN_TIMEOUT override) and logs in-flight worker count + drain time (Executor drain: N worker(s) in-flight ... drained in Xs / clean in Xs). A wedged worker no longer blocks here; the CLI backstop finalizes it.

Tests (RED→GREEN verified)

  • test_run_gateway_force_exits_zero_after_clean_shutdown / ..._exits_nonzero_when_start_gateway_reports_failure — force-exit fires with the right code (fail on old fall-through/sys.exit).
  • test_force_process_exit_calls_os_exit_and_flushes — flushes stdio, calls os._exit(code).
  • test_shutdown_executor_bounded_by_drain_timeout — _shutdown_executor returns within budget even when a worker blocks 5s (RED-proven: unbounded variant blocks 4.97s and fails).

Existing run_gateway() end-to-end tests updated to neutralize the new force-exit (shared _install_fake_gateway_run helper stubs _force_process_exit), mirroring the existing tests/gateway/test_gateway_process_exit.py pattern for main().

…n't strand the gateway

Restarting the gateway occasionally left it "down but not exited" for ~2.5
minutes (observed: a 149s restart blackout), during which launchd/systemd could
not relaunch it. Root cause: the gateway runs each agent turn on a
ThreadPoolExecutor whose workers are non-daemon, and concurrent.futures registers
an atexit hook that JOINS all worker threads before the interpreter can exit. If
an agent turn was still executing in a worker at SIGTERM, the async teardown
completed quickly (adapters disconnected, SessionDB closed, sessions persisted)
but the process then blocked at interpreter finalization until that in-flight
turn finished — stranding the gateway between "stopped" and "exited".

The `gateway/run.py::main()` entrypoint already force-exits for exactly this
reason, but production runs via `hermes_cli/gateway.py::run_gateway()`, which
lacked the backstop. This applies the same pattern there.

Changes:
- hermes_cli/gateway.py: `run_gateway()` now force-exits via a new
  `_force_process_exit()` helper at BOTH the clean (code 0) and failure (code 1)
  tails, after start_gateway()'s graceful teardown. The helper flushes stdio and
  logs any lingering non-daemon threads (greppable "force-exit: lingering
  non-daemon ...") so the next stall is diagnosable at a glance. Wrapped in a
  helper so tests can neutralize os._exit.
- gateway/run.py: `_shutdown_executor()` now does a BOUNDED drain
  (`_executor_drain_timeout()`, default 8s, HERMES_GATEWAY_EXECUTOR_DRAIN_TIMEOUT
  override) and logs how many workers were in-flight and how long the drain took
  ("Executor drain: N worker(s) in-flight ... drained in Xs" / "clean in Xs").
  A wedged worker no longer blocks here; the CLI hard-exit backstop finalizes it.

Tests (RED→GREEN verified):
- run_gateway force-exits(0) on clean shutdown and (1) on failure.
- _force_process_exit flushes stdio and calls os._exit with the code.
- _shutdown_executor returns within the drain budget even when a worker blocks
  for 5s (the wedged-worker case), and emits the in-flight diagnostic.
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🔎 Lint report: fix/gateway-shutdown-force-exit vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 12271 on HEAD, 12271 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 6430 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@greptile-apps

greptile-apps Bot commented Jul 1, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR adds a hard os._exit backstop to the production gateway entrypoint (hermes_cli/gateway.py::run_gateway) so that a wedged non-daemon ThreadPoolExecutor worker — which concurrent.futures would otherwise join via atexit, blocking interpreter finalization — cannot strand the gateway "down but not exited" after graceful teardown. A matching bounded-drain loop is added to _shutdown_executor so clean exits remain clean, with detailed logging for the wedged case.

  • hermes_cli/gateway.py: Introduces _force_process_exit(code) (flushes stdio, logs lingering non-daemon threads, calls os._exit) and replaces sys.exit(1) on the failure tail while adding _force_process_exit(0) at the clean tail; the KeyboardInterrupt console path is intentionally left as a plain return.
  • gateway/run.py: Replaces the unconditional wait=False shutdown with a bounded polling drain (default 8 s, HERMES_GATEWAY_EXECUTOR_DRAIN_TIMEOUT override) and emits structured log lines for both the clean and stalled cases.
  • Tests: New targeted tests for the force-exit paths and the bounded drain; existing tests updated to neutralize _force_process_exit via a shared _install_fake_gateway_run helper.

Confidence Score: 4/5

Safe to merge; the graceful-teardown logic is unchanged and the hard exit fires only after it completes, so no in-flight work is dropped.

The fix is well-targeted and backed by three new tests covering the exact failure modes. The bounded drain correctly uses Python's while/else idiom, the thread-name filter matches the executor's thread_name_prefix, and the os._exit path is cleanly isolated in a monkeypatchable helper. Two minor observations remain: a missing blank line (PEP 8 E302) after _executor_drain_timeout, and the KeyboardInterrupt console path still returning normally — meaning a wedged worker at Ctrl+C time could produce the same hang in interactive use, though the production daemon path is fully covered.

gateway/run.py — missing blank line after _executor_drain_timeout; hermes_cli/gateway.py — KeyboardInterrupt exit path returns without _force_process_exit.

Important Files Changed

Filename Overview
gateway/run.py Adds _executor_drain_timeout() helper and bounded polling drain to _shutdown_executor; a missing blank line between the new function and the following module-level regex constant is a PEP 8 E302 violation.
hermes_cli/gateway.py Adds _force_process_exit helper and wires it into both exit tails of run_gateway; the KeyboardInterrupt console path deliberately returns normally and is the only remaining path susceptible to a wedged-worker hang.
tests/gateway/test_session_env.py Adds test_shutdown_executor_bounded_by_drain_timeout which proves the drain returns within budget even when a worker blocks 5 s — correctly tests the core correctness of the bounded wait.
tests/hermes_cli/test_gateway.py Updates _install_fake_gateway_run to neutralize _force_process_exit, adds three new targeted tests for the force-exit paths; all existing tests correctly updated to the new contract.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Supervisor as launchd/systemd
    participant Process as gateway process
    participant Executor as ThreadPoolExecutor
    participant Worker as worker thread

    Supervisor->>Process: SIGTERM
    Process->>Executor: _shutdown_executor()
    Executor->>Worker: "shutdown(wait=False, cancel_futures=True)"
    Note over Executor,Worker: bounded poll up to 8 s
    alt worker finishes within budget
        Worker-->>Executor: thread exits cleanly
    else worker still running at deadline
        Executor-->>Process: "timed_out=True logged"
    end
    Process->>Process: _exit_diag(gateway.exit_clean)
    Process->>Process: _force_process_exit(0)
    Note over Process: flush stdio, log lingering threads
    Process->>Process: os._exit(0) skips atexit join
    Process-->>Supervisor: process exited immediately
    Supervisor->>Process: respawn
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Supervisor as launchd/systemd
    participant Process as gateway process
    participant Executor as ThreadPoolExecutor
    participant Worker as worker thread

    Supervisor->>Process: SIGTERM
    Process->>Executor: _shutdown_executor()
    Executor->>Worker: "shutdown(wait=False, cancel_futures=True)"
    Note over Executor,Worker: bounded poll up to 8 s
    alt worker finishes within budget
        Worker-->>Executor: thread exits cleanly
    else worker still running at deadline
        Executor-->>Process: "timed_out=True logged"
    end
    Process->>Process: _exit_diag(gateway.exit_clean)
    Process->>Process: _force_process_exit(0)
    Note over Process: flush stdio, log lingering threads
    Process->>Process: os._exit(0) skips atexit join
    Process-->>Supervisor: process exited immediately
    Supervisor->>Process: respawn
Loading

Comments Outside Diff (1)

  1. hermes_cli/gateway.py, line 4558-4566 (link)

    P2 KeyboardInterrupt path left unguarded against wedged-worker hang

    The KeyboardInterrupt handler returns normally without calling _force_process_exit, so a non-daemon executor worker still running at Ctrl+C time will block interpreter finalization via concurrent.futures's atexit join — the same hang this PR fixes for the daemon path. In an interactive console session the user can force-kill manually, but it is the one remaining path where the 149 s stall could recur (e.g., a developer stopping a local gateway while a slow agent turn is in flight).

Reviews (1): Last reviewed commit: "fix(gateway): hard-exit after graceful s..." | Re-trigger Greptile

Comment thread gateway/run.py
@Kyzcreig
Kyzcreig merged commit fb69fbf into main Jul 1, 2026
33 checks passed
@Kyzcreig
Kyzcreig deleted the fix/gateway-shutdown-force-exit branch July 1, 2026 03:56
Kyzcreig added a commit that referenced this pull request Jul 1, 2026
…bservability + reboot-parity proof (#142)

* fix(gateway): auto-surface a self-initiated restart that was drain-interrupted (restart_consumed_interrupted)

A turn whose last act fires safe-restart.py sets _session_initiated_restart, so
on shutdown the F1/F2 cascade-breaker stamps restart_consumed — deliberately
EXCLUDED from _AUTO_RESUME_REASONS to break the restart->resume->restart cascade.
But if that same turn was ALSO still running when the 180s drain timed out
(genuinely interrupted in-flight work), it fell in a seam: the cascade guard
suppressed the auto-wake AND #136 preserve-and-prompt never got to surface it,
so the session stayed silent until the next inbound message (observed live
2026-06-30 20:30 — Apollo's Discord turn preserved but not surfaced).

Fix (restart-only; reboot is unaffected — proven): a new resume_reason
restart_consumed_interrupted, stamped when a self-initiated restart is marked at
the POST-DRAIN-TIMEOUT site (interrupted=True) rather than the pre-drain
speculative site. It IS in _AUTO_RESUME_REASONS (so startup auto-resume
proactively surfaces the interrupted work, surface-and-wait, no tool replay),
but still records the F2 replay-mark, so a genuine restart->resume->restart loop
is still bounded and suspended at threshold within the 300s window.

- _resume_reason_for_shutdown_mark gains interrupted kwarg; _self_initiated
  helper also treats an entry already carrying restart_consumed[_interrupted] as
  self-initiated.
- Only the post-timeout mark site passes interrupted=True (the pre-drain
  speculative mark, which is cleared on graceful finish, stays interrupted=False).
- _resume_reason_phrase maps the new reason to 'a gateway restart'.

Reboot parity (Task C, proof-not-code): a self-initiated REBOOT never sets
_session_initiated_restart (the detector matches literal safe-restart.py, not
safe-reboot.py; reboot-watcher writes no breadcrumb), so it marks shutdown_timeout
(already auto-resumes). Reboot inherits #136/#137 preservation via the shared
SIGTERM spine unchanged.

Tests: 7 cascade unit tests + 1 integration call-site gate (real stop() through
the drain-timeout branch) + 2 reboot-parity tests. The integration test is
MUTATION-PROVEN: flipping the post-timeout site to interrupted=False turns it RED.
Change-detector allow-list test converted to an additive-invariant assertion.
Spec: plans/2026-07-01_restart-reboot-continuity-observability-SPEC.md (pass-3
APPROVE, all RCs folded).

* feat(gateway): PHASE= observability for the shutdown mark + boot-resume decisions

Task B (spec 2026-07-01 restart-reboot-continuity). The gateway shutdown spine
already logs phase TIMINGS ('Shutdown phase: drain done at +Xs', #140's
'Executor drain:', 'Auto-resume skipped') — that existing 'Auto-resume skipped'
line is what let us diagnose the 20:30 seam. This adds the two per-session
DECISION points that were invisible:

- PHASE=shutdown_mark key=... reason=... interrupted=... replay_marked=... marked=...
  at _mark_resume_pending_for_shutdown — makes the restart_consumed vs
  restart_consumed_interrupted vs shutdown_timeout choice observable per session.
- PHASE=boot_resume_scheduled key=... reason=... platform=...
  when a session is proactively resumed at boot (surface-and-wait).

Both use the module logger at INFO (INV-3/D-8: no fsync, no rollover in the hot
path — logging must never eat the 180s drain budget) and carry session key +
flags ONLY, never transcript content (INV-6).

Tests: PHASE-line-emitted, no-content-leak (INV-6), and a slow/blocking-handler
latency gate (D-8 — proves the mark path doesn't fan out or block). Watcher-side
PHASE= markers (safe-restart watcher.py, safe-reboot reboot-watcher.py) ship in
the skills tree.

* fix(gateway): address Opus code-review — real INV-6/INV-3 gates, no overclaim

Opus code-review BLOCKed the first cut: two invariant tests passed vacuously.
Fixed both to be genuine gates, plus the required-changes:

- INV-6 (no transcript content): the test was a TAUTOLOGY — it declared a SECRET
  marker but never seeded it where the PHASE logger could reach it, so the
  assertion was true by construction. Now seeds the sentinel into the transcript
  (append_to_transcript), _pending_messages, and the running-agent object, then
  asserts it never appears in any PHASE= line. MUTATION-PROVEN: adding msg=%s to
  the log line turns it RED.
- INV-3 (fail-open / non-blocking): the old test over-claimed 'non-blocking' with
  a bound its own docstring admitted didn't prove it. Replaced with an honest
  single-emit + FAIL-OPEN gate, and made the CODE genuinely fail-open (both PHASE
  logs wrapped in try/except so a raising/broken sink can't abort the resume-mark
  — losing the mark = losing the interrupted work). Dropped the overclaiming code
  comment (handler blocking is a gateway-logging-config concern, not this site's).
- RC#3: added a direct reason-UPGRADE test (mark clean restart_consumed → mark
  interrupted → upgraded to restart_consumed_interrupted) so the overwrite can't
  silently regress if mark_resume_pending ever gains idempotency.
- Residual #31 (reason drift): added a test proving a self-restart reason does NOT
  leak into a later unrelated shutdown (clear_resume_pending nulls resume_reason +
  the post-turn gate pops _session_initiated_restart → back to shutdown_timeout).
- Test hygiene: the logger-level test now restores the prior level in finally.
- Maintainability: promoted the 4-site magic literal to a named module constant
  _REASON_RESTART_CONSUMED_INTERRUPTED.

199 passed / 1 skipped.

* fix(gateway): code-review-2 follow-ups — WARNING-level audit breadcrumb + restart_consumed constant

Code-review pass-2 = APPROVE WITH CHANGES (0 blockers; both prior fake-gate
BLOCKERs certified genuinely resolved). Two non-blocking RCs landed:

- (SRE) PHASE breadcrumbs were logger.info → lost when a deployment runs
  gateway.run at WARNING+, exactly while diagnosing a bad restart. Made the mark
  log LEVEL-AWARE: an INTERRUPTED or replay-marked mark (the diagnostic case) and
  the boot_resume_scheduled breadcrumb now log at WARNING (survive a raised
  threshold, matching the sibling drain-timeout warning); a routine clean mark
  stays INFO so normal shutdowns don't spam WARNING.
- (Maintainability) promoted the clean-sibling literal 'restart_consumed' to a
  named constant _REASON_RESTART_CONSUMED alongside its twin
  _REASON_RESTART_CONSUMED_INTERRUPTED, so the cascade-guard pair moves together.

Residual #31 (reason drift) verified against the PRODUCTION path:
_apply_post_turn_resume_gate (called post-turn at the run loop) pops
_session_initiated_restart AND calls clear_resume_pending (nulls resume_reason) —
so the drift the test proves absent cannot ship.

Tests: WARNING-level-survives + clean-stays-INFO + caplog-propagation-pin.
209 passed / 1 skipped.

---------

Co-authored-by: Apollo <apollo@ang.ventures>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant