fix(mcp): correct _stdio_children_dead logic and prevent coroutine leak (#96016, #96030)

[loulanyue]

Summary

Fixes #96016, #96019, #96030

• In tools/mcp_tool.py (_stdio_children_dead):
  • Fix logic inversion: previously returned True when psutil.pid_exists(pid) was True (indicating the process is alive), which caused live stdio MCP subprocesses to be prematurely flagged as dead and killed with TimeoutError.
  • Now correctly returns False if any tracked child PID is alive, and True only when all child processes have exited.
• In tools/mcp_tool.py (_make_tool_handler):
  • Fix unawaited coroutine leak in _watch_ok check by avoiding calling _watch_children() directly during liveness inspection.
• In tests/tools/test_mcp_stdio_children_dead.py:
  • Add unit tests verifying _stdio_children_dead behavior on empty, alive, and dead PIDs.

[KeyArgo]

Corroborating #96030 — independent confirmation of the coroutine-leak premise.

I independently root-caused #96030 (reporter ilgo) to the same line: _make_tool_handler's stdio-watch fast-fail guard at tools/mcp_tool.py (~6173) evaluated inspect.isawaitable(_watch_children()), which CALLS _watch_children() and discards the returned coroutine unawaited. On every successful MCP tool call (stdio child-watcher present) that leaks one RuntimeWarning: coroutine 'MCPServerTask._watch_stdio_children' was never awaited per call — confirmed on upstream main.

Your change to probe the function type (inspect.iscoroutinefunction(_watch_children) or callable(...)) instead of invoking it resolves the leak, and the accompanying _stdio_children_dead fix addresses the sibling unreachable-return False in the same race (#96016). Your PR is broader than my local draft (which only touched the guard line), so I'm not pushing a competing fix — yours is the right one.

One suggestion for review: callable() is true for any bound method, so the or callable(_watch_children) arm can never gate anything on a real server (the child-watcher is always a bound coroutine method there). It only matters for MagicMock/stub sessions, where it correctly re-enables the race. That's fine behaviorally, just noting the arm is effectively a mock-compat allowance rather than a liveness check — might read clearer as a comment. Not blocking.

[kshitijk4poor]

Closing — both halves of this PR are now handled, with your coroutine-leak fix landing via salvage:

• Polarity fix: already on main via fix(mcp): un-invert the stdio children liveness check #94339 (@liuhao1024's canonical carrier, merged today, commit ef46ec0) — the many-way duplicate on that hunk.
• Coroutine leak (your unique find): salvaged into fix(mcp): stop leaking an unawaited watcher coroutine per stdio tool call (salvage #96044) #96251 with your authorship preserved (auto-merge armed). Good catch — the _watch_ok probe was creating a never-awaited coroutine on literally every stdio MCP tool call. One adjustment in the follow-up commit: the replacement probe is inspect.iscoroutinefunction(_watch_children) alone, without the or callable(...) arm — callable(MagicMock) is True, which would have flipped stubbed sessions into the fast-fail race that the surrounding comment deliberately routes to the plain-await path. Two regression tests pin both properties.

Thanks for the contribution — the leak half would have been easy to miss under the pile of polarity duplicates.