fix(gateway): bound final shutdown cleanup

[embwl0x]

Summary

Fixes #58666.

When /restart is requested under a service manager, the gateway sets _shutdown_event after adapter teardown but then runs final tool/environment cleanup inline on the event loop. If that best-effort cleanup wedges, the loop never returns to the main gateway task, so housekeeping never stops and launchd/systemd never observes process exit.

This adds a bounded late-shutdown helper for the post-adapter final-cleanup phase:

• runs synchronous cleanup in a daemon worker so a wedged hook cannot keep the process alive
• reads the deadline from gateway.shutdown_cleanup_timeout in config.yaml (default 5s)
• reports done, timed out, and failed distinctly instead of logging exceptions as successful cleanup
• contains SystemExit/KeyboardInterrupt raised by the cleanup worker while preserving cancellation of the shutdown task itself
• logs and continues shutdown when cleanup exceeds the deadline
• preserves legacy inline behavior when the setting is 0

The regression injects a generic post-disconnect final-cleanup wedge. It proves stop(restart=True, service_restart=True) still completes and preserves the service-restart exit code without claiming an unproven channel_overrides trigger. Additional coverage proves inline exceptions and worker SystemExit are reported as failed cleanup.

This supersedes closed #61011. That version used a new non-secret HERMES_* environment variable; this rescope follows the maintainer review by making the timeout a typed gateway.* configuration value with YAML loading and serialization coverage.

Duplicate / overlap check

• inspected the current issue timeline and all publish-gate matches
• channel_overrides matches concern model/routing behavior, launchd matches concern service management, and MCP shutdown matches bound an earlier subsystem
• none bounds this generic post-adapter final-cleanup phase

Verification

• rebased onto exact current main 8b09a9df8476010a78e86d6c32254b4ac14a8c4f
• 77 focused gateway shutdown and config tests passed
• Ruff, lock validation, git diff --check, public identity/privacy checks, gitleaks, and the existing-PR review-follow-up gate passed

Current head: 5fb82eb188439957f7e04842c45a6424ad5c8046

[teknium1]

Thanks for the focused shutdown hardening. The underlying premise is present on current main: _stop_impl() sets _shutdown_event at gateway/run.py:8296 and then performs synchronous final cleanup at gateway/run.py:8305, so a wedge there can still hold the event loop.

Problems

• The new test does not establish the claimed channel_overrides trigger. It configures that value but explicitly injects the hang by monkeypatching process_registry.kill_all; the final-cleanup segment in current main (gateway/run.py:8298-8309) does not read channel_overrides.

Suggested changes

• Re-scope the test as a generic wedged-final-cleanup regression, or add a reproduction that demonstrates the reported channel_overrides path causing the cleanup wedge.

The config.yaml surface is aligned with AGENTS.md:102-106 and addresses the environment-variable concern that closed #61011. This is an automated hermes-sweeper review.

[embwl0x]

Addressed in 747e56c2c. The regression is now scoped to the invariant it actually proves: a generic wedged final-cleanup step cannot hold service-manager shutdown. Removed the unrelated channel_overrides fixture and renamed the test accordingly.\n\nValidation: tests/gateway/test_gateway_shutdown.py + tests/gateway/test_config.py (123 passed), and ruff passed.

[embwl0x]

Refreshed onto current main (d9f1043c3) in 8ed299312. The resolution composes the bounded final-cleanup helper with the newer Telegram connect timeout and loop-watchdog configuration. Reverified: 184 shutdown/config/cache-cleanup tests passed, Ruff passed, publish gate and gitleaks passed.

[embwl0x]

Refreshed onto current main (a4973c3) in 4fb9f839d. The conflict resolution preserves upstream's DEFAULT_CONFIG extraction and places gateway.shutdown_cleanup_timeout in hermes_cli/config_defaults.py, while retaining the bounded final-cleanup behavior and duck-typed runner compatibility. Local verification: 184 shutdown/config/cache-cleanup tests passed; Ruff, git diff --check, publish gate, and gitleaks passed. GitHub reports MERGEABLE, and all required CI passed, including all eight Python slices and Desktop E2E. The non-required arm64 Docker build passed; the non-required amd64 Docker build was still running at final read-back.

[embwl0x]

Rebased onto current main (a991dfc25) and resolved the gateway shutdown/config conflicts in de24b1f246.

The resolution preserves upstream's extracted hermes_cli/config_defaults.py, newer gateway shutdown/session cleanup paths, stall-notify timeout, and current adjacent tests while retaining the bounded post-adapter final-cleanup behavior and duck-typed runner compatibility.

Local verification: 90 focused and adjacent tests passed across shutdown, config, cache cleanup, restart drain, bounded adapter teardown, channel overrides, and async session storage. Ruff, byte compilation, uv lock --check, diff check, publish gate, and gitleaks passed. GitHub reports MERGEABLE, and all required CI passed, including all eight Python slices. The two non-required Docker builds remain in progress.

[embwl0x]

Rebased onto current main (9829746df) and resolved the gateway shutdown/config conflicts in e91b0b0c5.

The resolution preserves upstream's selective multiplex profile serving, explicit unserved-route rejection, user-boundary completion routing, and current adjacent config tests while retaining the bounded post-adapter final-cleanup behavior and duck-typed runner compatibility. GitHub now reports the PR MERGEABLE; the conflict signal is cleared.

Local verification:

• 89 passed, 2 Windows-only skipped across shutdown, config, cache cleanup, restart drain, bounded adapter teardown, channel overrides, and async session storage
• Ruff, byte compilation, uv lock --check, git diff --check, publish gate, and gitleaks passed

Fresh CI passed 11/12 Python slices, e2e, Ruff/ty, Windows and macOS lanes, OSV, supply-chain, attribution, lockfile, history, and both Docker builds. Slice 5 failed only tests/gateway/test_multiplex_busy_input_mode.py::test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries.

I reproduced that failure identically in an untouched worktree at exact base 9829746df: the test does not mark research as a served profile after main's new route-rejection invariant. The focused test-fixture correction is already open and fully green in #83745, so I did not import unrelated baseline repair into this PR. Once #83745 lands, refreshing this branch will retrigger a clean aggregate.

[embwl0x]

Refreshed onto exact current main (222465d84) now that the shared busy-mode fixture correction is upstream.

• New head: 5079f9c96
• Local shutdown/config/session matrix: 104 passed, 2 Windows-only skipped
• Ruff, diff check, public identity, publish gate, and gitleaks: passed
• Replacement GitHub CI: 33 passed checks, 12 intentional skips, zero failures or pending checks

The bounded post-adapter final-cleanup behavior and timeout=0 compatibility remain intact. GitHub reports the PR mergeable.

[embwl0x]

Rebased onto current main (56a41715d) and resolved the shutdown-test conflict in 71ecc164e.

The resolution preserves upstream’s completion-batch cancellation-before-adapter-disconnect regression and this PR’s wedged post-adapter final-cleanup deadline regression. The production/config patch remains unchanged in substance.

Local verification:

• 91 passed, 2 Windows-only skipped across shutdown, config, cache cleanup, restart drain, bounded adapter teardown, channel overrides, and async session storage
• Focused immediate pre-push rerun: 69 passed
• Ruff, byte compilation, uv lock --check, git diff --check, publish gate, and gitleaks passed

GitHub now reports mergeable: true; replacement CI is running.

[embwl0x]

CI failure resolved on head d4eeee72107f1356d73b42cc8476441cb0241c98. The branch was rebased onto current main 1f8fdc7bd824c8d07e3cefe109bd96425ec3171f, which includes the upstream fix for the failing test_slash_worker_accepts_profile_home Path mock. Local proof: the exact failing node now passes, plus 75 gateway/TUI tests covering config, shutdown, cache cleanup, and async session storage. Replacement CI passed all required checks, all 12 Python slices, and both Docker architectures.

[embwl0x]

Resolved the latest base conflict on current main in 2b3874e08.

The resolution preserves both the bounded final-shutdown cleanup from this PR and current main shutdown/cache teardown behavior, including fake-runner and async-session-store contracts.

Local proof: 95 relevant tests passed with 2 expected Windows skips; Ruff, diff/security gates, and the existing-PR publish gate passed. Replacement GitHub CI is fully green, and GitHub reports the PR mergeable.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): bound final shutdown cleanup

1. _runner in _run_bounded_shutdown_step (gateway/run.py, ~line 7212) catches BaseException and transports it to the future, but the coroutine only catches Exception/TimeoutError. A SystemExit/KeyboardInterrupt raised inside the cleanup thread therefore propagates out of the bounded step mid-shutdown — arguably correct (do not mask Ctrl+C), but inconsistent with the "best-effort, never wedge exit" intent. Either catch BaseException at the await site too, or document why those two escape.
2. The step returns True when func() raised (except Exception: return True), so the caller logs "final-cleanup tool kill done" even when the cleanup threw. The log/telemetry cannot distinguish "completed" from "failed but finished". Returning False (or a tri-state) on exception would make the phase status truthful.
3. After a timeout the daemon thread keeps running while the process proceeds to exit; _kill_tool_subprocesses mutates process_registry shared state. Today final-cleanup is the last phase so there is no concurrent access, but the thread is not cancelled and can outlive the loop — worth keeping the docstring note (already present) or adding a cancel() hook for future phases.
4. Cosmetic: the %.1f format renders "exceeded 0.0s" for a 0.01s timeout (the test asserts exactly that string); %.2f or a ceil would be less confusing. Config plumbing, clamping, and test coverage are otherwise solid.

[embwl0x]

Resolved the renewed current-main conflict and addressed the cleanup-status review at 5fb82eb188439957f7e04842c45a6424ad5c8046.

• Replayed the authored fix onto exact upstream main 8b09a9df8476010a78e86d6c32254b4ac14a8c4f, preserving main's watchdog defaults and planned service-restart regression.
• _run_bounded_shutdown_step() now distinguishes done, timed out, and failed; exceptions no longer produce a misleading successful-cleanup log.
• Worker SystemExit/KeyboardInterrupt are contained as failed best-effort cleanup, while cancellation of the shutdown task still propagates. Timeout logs now retain hundredths for short configured deadlines.
• Added inline-failure and worker-SystemExit regression coverage.
• Local verification: 77 focused shutdown/config tests passed; Ruff, lock/diff checks, review-follow-up gate, and gitleaks passed.
• Replacement CI is running.

[embwl0x]

Replacement CI is fully green on 5fb82eb188439957f7e04842c45a6424ad5c8046: Python and E2E tests, native Windows/macOS lanes, Ruff/ty and Windows-footgun enforcement, security/attribution/history gates, both Docker architectures, Nix, and All required checks pass all succeeded.