From 86c5febdd1d31200c6fa982c2e98eb27c116e876 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:23:34 -0700 Subject: [PATCH] fix(mcp): watchdog wrap after OSV preflight + forward SIGTERM to child group Two fixes on top of the salvaged parent-death watchdog: - Apply the watchdog wrap AFTER the OSV malware preflight so the check inspects the real npx/uvx package instead of the python wrapper (the wrap previously made the preflight a silent no-op for every stdio server). - The real server runs in its own process group under the watchdog, so the graceful-shutdown killpg no longer reached it; the watchdog now forwards SIGTERM/SIGINT to the child's group, keeping wedged servers killable on clean shutdown. --- tools/mcp_stdio_watchdog.py | 13 +++++++++++++ tools/mcp_tool.py | 25 +++++++++++++++---------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tools/mcp_stdio_watchdog.py b/tools/mcp_stdio_watchdog.py index 46761c16337..17e6fb4c41c 100644 --- a/tools/mcp_stdio_watchdog.py +++ b/tools/mcp_stdio_watchdog.py @@ -138,6 +138,19 @@ def main(argv: list[str] | None = None) -> int: start_new_session=True, ) + # Because the real server lives in its OWN process group (above), the + # parent's graceful-shutdown killpg of *our* group no longer reaches it. + # Forward SIGTERM/SIGINT to the child's group so graceful teardown + # (`_kill_orphaned_mcp_children`, shutdown sweeps) still kills a wedged + # server that ignores stdin EOF — otherwise the watchdog wrap would + # invert the bug it fixes. + def _forward_shutdown(signum, frame): # noqa: ARG001 + _terminate_process_group(proc) + sys.exit(128 + signum) + + signal.signal(signal.SIGTERM, _forward_shutdown) + signal.signal(signal.SIGINT, _forward_shutdown) + watchdog = threading.Thread( target=_watchdog_loop, args=(proc, args.ppid, args.create_time), diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 1d45a079027..5f5726d769b 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -1949,21 +1949,14 @@ class MCPServerTask: safe_env = _build_safe_env(user_env) command, safe_env = _resolve_stdio_command(command, safe_env) - # Wrap the real command in a parent-death watchdog supervisor so an - # ungraceful exit of this Hermes process (kill -9, crash, force-quit) - # can't leave the stdio MCP child (and its own descendants, e.g. - # mcp-remote's spawned `node`) running forever. On a clean exit, - # MCPServerTask.shutdown() / _kill_orphaned_mcp_children() still do - # the reaping as before -- this only covers the case where that code - # never gets to run. POSIX-only (relies on process groups); no-op - # elsewhere, matching existing killpg-based cleanup's platform scope. - command, args = _wrap_command_with_watchdog(command, args) - # Check package against OSV malware database before spawning. # Run off the event loop (the urllib HTTPS call is blocking) and bound # it with a wall-clock timeout so a stalled SSL handshake can't freeze # MCP discovery / gateway startup (#29184). The check is fail-open, so # on timeout we log and proceed rather than blocking indefinitely. + # NOTE: must run against the REAL command/args — the watchdog wrap + # below rewrites argv to `python -m tools.mcp_stdio_watchdog …`, + # which would silently turn the preflight into a no-op. from tools.osv_check import check_package_for_malware try: malware_error = await asyncio.wait_for( @@ -1982,6 +1975,18 @@ class MCPServerTask: f"MCP server '{self.name}': {malware_error}" ) + # Wrap the real command in a parent-death watchdog supervisor so an + # ungraceful exit of this Hermes process (kill -9, crash, force-quit) + # can't leave the stdio MCP child (and its own descendants, e.g. + # mcp-remote's spawned `node`) running forever. On a clean exit, + # MCPServerTask.shutdown() / _kill_orphaned_mcp_children() still do + # the reaping as before -- this only covers the case where that code + # never gets to run. POSIX-only (relies on process groups); no-op + # elsewhere, matching existing killpg-based cleanup's platform scope. + # Applied AFTER the OSV preflight so the check inspects the real + # package, not the watchdog wrapper. + command, args = _wrap_command_with_watchdog(command, args) + server_params = StdioServerParameters( command=command, args=args,