cherry-pick: fix(mcp): move t.cancel() inside try block (#60213)

[AIalliAI]

Cherry-pick of PR NousResearch#60213 from NousResearch/hermes-agent.

Original PR

• Upstream: fix(mcp): move t.cancel() inside try block to prevent RuntimeError on exit (#60197) NousResearch/hermes-agent#60213
• Author: webtecnica
• Issue: Fixes [Bug] RuntimeError: Event loop is closed during /exit (MCPServerTask.shutdown) NousResearch/hermes-agent#60197 - RuntimeError when /exit is called with MCP tasks running

Changes

Moves t.cancel() inside try block at both cleanup locations in tools/mcp_tool.py:

1. _wait_for_lifecycle_event method
2. _wait_for_reconnect_or_shutdown method

This prevents RuntimeError('Event loop is closed') during shutdown when the event loop closes before MCP tasks finish cleanup.

Verification

• ✅ Author preserved: webtecnica
• ✅ Minimal change (8 insertions, 4 deletions)
• ✅ Commit message follows format
• ✅ Python syntax valid

[AIalliAI]

Review: APPROVE (submitted as comment - unable to approve own PR)

Verification

• ✅ Fix verified: t.cancel() moved inside try block at both locations
  • _wait_for_lifecycle_event (lines 1800-1808)
  • _wait_for_reconnect_or_shutdown (lines 1767-1769)
• ✅ Author preserved: webtecnica <75556242+webtecnica@users.noreply.github.com>
• ✅ Cherry-pick metadata correct with co-author attribution
• ✅ Minimal change (8 insertions, 4 deletions)
• ✅ Commit message follows format: fix(mcp): description (#60197)
• ✅ No AI attribution in code or commits
• ✅ Python syntax valid (py_compile passed)

Analysis

The fix correctly addresses RuntimeError('Event loop is closed') that occurs when /exit is called during MCP task execution. By moving t.cancel() inside the try block and catching RuntimeError alongside asyncio.CancelledError, the cleanup code no longer raises during shutdown.

Applied at both cleanup locations in the MCP tool lifecycle management, ensuring consistent behavior.

Recommendation: MERGE

[AIalliAI]

Hermes Agent Code Review

Verdict: APPROVED (note: cannot self-approve — submitted as comment)

Correctness

• Moving t.cancel() inside the try block is the right fix: when the event loop is already closed (shutdown via /exit), t.cancel() raises RuntimeError("Event loop is closed"). Catching it prevents the crash reported in NousResearch#60197.
• Applied consistently at both cleanup locations (_wait_for_lifecycle_event and _wait_for_reconnect_or_shutdown).

Edge Cases

• Task not cancelled when loop is closed: If t.cancel() raises RuntimeError, we skip await t — but the loop is already closed, so there is nothing to clean up. Acceptable.
• await t also raising RuntimeError: Now explicitly caught (was previously caught only by the broad Exception handler), which is an improvement.
• Other exceptions from t.cancel(): Caught by the except Exception fallback, identical to the original behavior.

Code Quality

• Splitting except (CancelledError, RuntimeError) from except Exception improves readability and documents intent.
• The pass pattern in both handlers is appropriate for a finally cleanup block — these are best-effort cleanup operations during teardown.
• Minimal diff (8 insertions, 4 deletions) — no unnecessary changes.

Cherry-Pick Hygiene

• Author preserved: webtecnica
• Commit message follows conventional format
• Cleanly cherry-picked from upstream NousResearch#60213

Minor Nit (non-blocking)

• The except (asyncio.CancelledError, RuntimeError): pass is technically redundant — the except Exception: pass below would catch both anyway. But the explicit listing serves as documentation that these are the expected exceptions during shutdown.

---

Reviewed by Hermes Agent