Skip to content

fix: catch BaseException in MCP shutdown guards to suppress double-Ctrl+C traceback - #39479

Merged
teknium1 merged 3 commits into
mainfrom
hermes/hermes-ba721783
Jun 5, 2026
Merged

fix: catch BaseException in MCP shutdown guards to suppress double-Ctrl+C traceback#39479
teknium1 merged 3 commits into
mainfrom
hermes/hermes-ba721783

Conversation

@teknium1

@teknium1 teknium1 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

A second Ctrl+C during MCP shutdown now exits cleanly instead of printing a KeyboardInterrupt traceback.

The CLI's _signal_handler_q raises KeyboardInterrupt (a BaseException, not Exception). The two best-effort teardown guards on the shutdown path caught only Exception, so a second interrupt during the up-to-15s future.result(timeout=15) wait escaped cleanup and surfaced as an unhandled traceback.

Salvage of #39453 by @HeLLGURD onto current main (the original branch was stale — its diff also showed a _branched_from block already present on main; that phantom chunk drops out on cherry-pick). Closes #39367.

Changes

  • cli.py _run_cleanup(): except Exceptionexcept BaseException
  • tools/mcp_tool.py shutdown_mcp_servers(): except Exceptionexcept BaseException

Both guards are pure best-effort teardown (pass / log-and-continue), so the wider catch only lets cleanup finish and the process exit without a traceback.

Validation

E2E (real source from worktree):

except Exception (old) except BaseException (new)
2nd Ctrl+C → KeyboardInterrupt escapes guard → traceback swallowed → clean exit

Both files compile clean.

Credit: @HeLLGURD (commits cherry-picked with authorship preserved).

Infographic

mcp-shutdown-guard-fix

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: hermes/hermes-ba721783 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: 9850 on HEAD, 9850 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5107 pre-existing issues carried over.

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

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/mcp MCP client and OAuth labels Jun 5, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Small but effective shutdown robustness fix. Replacing Exception with BaseException is the right move here: cleanup paths should not let KeyboardInterrupt/SystemExit break shutdown.

✅ Looks Good

  • Change is surgical and fully aligned with intended behavior.
  • Scope is limited to MCP shutdown + CLI cleanup.
  • No observable side effects on normal execution paths.

💡 Suggestions

  • Consider documenting this non-obvious choice inline (one comment) so future readers don’t revert it back to Exception during cleanup refactors.

Reviewed by Hermes Agent

@teknium1
teknium1 merged commit 2069e78 into main Jun 5, 2026
23 checks passed
@teknium1
teknium1 deleted the hermes/hermes-ba721783 branch June 5, 2026 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI: 2nd Ctrl+C during MCP shutdown escapes cleanup → KeyboardInterrupt traceback on exit

4 participants