fix(mcp): stop a timed-out worker's loop instead of abandoning it

[kjellouli]

Problem

When an MCP operation exceeds its deadline, MCPClientBase._run_isolated_operation abandons the worker:

finally:
    if timed_out:
        worker.kill(terminate_thread=False)   # abandoned, not stopped

A worker whose event loop is still cycling then spins at 100% CPU holding the GIL for the lifetime of the process. That starves every other Python thread, which makes subsequent update_tools polls likelier to blow their own deadline in turn.

Evidence

Observed on agent0ai/agent-zero:latest:

• one leaked MCPClient-<server>-update_tools-<uuid> thread at 11h55m of CPU since container start
• measured at 100.8% of a core over a 10s /proc/<pid>/task/<tid>/stat sample
• py-spy dump showed it active+gil:

  select (selectors.py:464)
  _run_once (nest_asyncio.py:115)
  run_forever (nest_asyncio.py:81)
  _run_event_loop (defer.py:41)
  

• across ~12h there were 60 abandonments over four servers (prelab 21, twenty 15, bureau_docs 12, hound 6) leaving exactly one thread alive — so most abandoned workers do exit, but a wedged one never does
• docker restart reclaimed the core: 100.17% → 0.51%

Why not simply terminate_thread=True

That was my first instinct and it is wrong on current ready. DeferredTask.kill(terminate_thread=True) waits on cleanup_future.result() and thread.join(), both unbounded, and the drain runs on the very loop that is not progressing. Verified: against pristine ready, kill(terminate_thread=True) on a wedged worker never returns (killed at a 20s cap). Abandoning was a deliberate trade — "so Agent Zero can continue" — not an oversight.

So this PR does not touch that path.

Fix

Stop the loop, and wait on nothing:

loop = getattr(worker.event_loop_thread, "loop", None)
if loop is not None and loop.is_running():
    loop.call_soon_threadsafe(loop.stop)

loop.stop() is what ends the spin. No drain, no join, so the caller cannot block — the property that made abandoning necessary is preserved.

Relationship to #1794

#1794 bounds the defer.py teardown (DRAIN_TIMEOUT, cleanup_future.result(timeout=...), thread.join(timeout=...)). It does not touch mcp_handler.py, so the MCP timeout path still passes terminate_thread=False and this leak survives it.

Deliberately kept self-contained so there is no overlap or merge-order dependency: this is safe to merge before or after #1794, in either order. Once #1794 lands, _stop_worker_loop can be folded into a plain kill(terminate_thread=True) and I'm happy to send that follow-up.

Tradeoff

Stopping without draining emits Task was destroyed but it is pending! for the abandoned operation. That is strictly better than a thread spinning at 100% CPU forever, and #1794's bounded drain would remove the warning.

Validation

• Added tests/test_mcp_worker_termination.py, driving a genuinely spinning worker (while True: await asyncio.sleep(0) — the state a wedged worker is actually left in, as opposed to a blocked loop, which no callback can reach).
• Ran the equivalent harness standalone against this branch: caller returned in 0.00s, worker thread exited. Against the unpatched path the thread stays alive indefinitely.
• I could not run pytest tests/ in full here — helpers.mcp_handler imports the whole model stack (litellm) and I did not have those dependencies installed. The new test should run in CI where they are present; please flag if it does not.

Based on ready, matching the branch recent merged PRs use.

🤖 Generated with Claude Code