fix(mcp): kill Windows stdio MCP orphan trees (#61059)

Windows has no POSIX parent-death supervisor/killpg safety net, so an
ungraceful exit of the hermes process left every stdio MCP child tree
(npx.cmd -> node.exe) running as orphans with ParentId=null, piling up
across session restarts.

- _run_stdio now attaches the process to a KILL_ON_JOB_CLOSE job object
  before spawning stdio children (self-guarded no-op off Windows), so the
  whole child tree dies with the parent at the kernel level.
- Windows reaps kill the process tree (direct child + descendants) in the
  lifecycle orphan sweep and the spawn-ledger startup sweep, where there
  is no pgid to group-kill.

Fixes #61059

hermes_cli/process_identity.py

@@ -399,11 +399,17 @@ def reap_orphaned_mcp_helpers(*, project_root: Optional[Path] = None, kill_fn=No
                 proc = psutil.Process(pid)
                 if not _same_incarnation(proc, entry.get("create_time")):
                     continue  # PID reused since registration
-                proc.terminate()
-                try:
-                    proc.wait(timeout=2.0)
-                except psutil.TimeoutExpired:
-                    proc.kill()
+                # Windows: descendants (npx.cmd → node.exe) have no pgid to group-kill and
+                # reparent with ParentId=null when the direct child exits first (#61059), so
+                # reap the whole tree. On POSIX the killpg-based sweep already reaches them.
+                if _IS_WINDOWS:
+                    _kill_process_tree_windows(proc)
+                else:
+                    proc.terminate()
+                    try:
+                        proc.wait(timeout=2.0)
+                    except psutil.TimeoutExpired:
+                        proc.kill()
             reaped.append(pid)
         except Exception:
             logger.debug("mcp-helper orphan reap failed for %s", entry, exc_info=True)
@@ -415,6 +421,36 @@ def reap_orphaned_mcp_helpers(*, project_root: Optional[Path] = None, kill_fn=No
 # Layer 3 — Windows job-object self-attach
 
 
+def _kill_process_tree_windows(proc) -> None:
+    """Terminate *proc* and every still-alive descendant (npx.cmd → node.exe), Windows-only
+    (#61059): without a pgid there is no group-kill, and grandchildren reparent to nothing
+    (ParentId=null) once the direct child exits, so they must be reached through the tree."""
+    import psutil
+
+    try:
+        descendants = proc.children(recursive=True)
+    except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
+        descendants = []
+    for child in descendants:
+        try:
+            child.terminate()
+        except Exception:  # noqa: BLE001 - raced away or refused; keep going
+            pass
+    try:
+        proc.terminate()
+    except Exception:  # noqa: BLE001
+        pass
+    try:
+        _, alive = psutil.wait_procs(descendants + [proc], timeout=2.0)
+    except Exception:  # noqa: BLE001 - broken fake/raced process; nothing more to force-kill
+        return
+    for survivor in alive:
+        try:
+            survivor.kill()
+        except Exception:  # noqa: BLE001
+            pass
+
+
 def attach_self_to_kill_on_close_job() -> bool:
     """Place this process in a job that dies (whole tree) when we die. Windows-only, idempotent.
 

tests/tools/test_mcp_windows_orphan_fix.py

@@ -0,0 +1,143 @@
+"""Tests for the Windows MCP orphan fix (#61059).
+
+Windows has no POSIX parent-death supervisor / killpg safety net, so orphaned
+npx → node.exe trees accumulated across session restarts. The fix has two rungs:
+
+1. ``_run_stdio`` attaches this process to a KILL_ON_JOB_CLOSE job object before
+   spawning stdio children, so any child/grandchild created after the attach dies
+   with the parent at the kernel level — even on an ungraceful exit.
+2. Windows reaps kill the whole process tree (direct child + descendants), since
+   there is no pgid to group-kill and grandchildren reparent with ParentId=null.
+
+The job-attach call itself is ctypes/Win32 and untestable off Windows; here we
+verify it is invoked on the Windows spawn path and not on POSIX, plus the
+best-effort contract of the tree-kill helpers.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from tools.mcp_tool import MCPServerTask, _MCP_AVAILABLE
+
+pytestmark = pytest.mark.skipif(not _MCP_AVAILABLE, reason="MCP SDK not installed")
+
+
+def _run_stdio_with_mocks(os_name: str, attach_mock) -> None:
+    """Drive _run_stdio's pre-spawn path with the transport mocked out."""
+    import tools.mcp_tool_transport as transport_mod
+
+    mock_session = MagicMock()
+    mock_session.initialize = AsyncMock()
+
+    async def _serve(session, timeout, **kwargs):
+        return "ok"
+
+    task = MCPServerTask("test-win-orphan")
+    task._serve_session = _serve
+    task._session_kwargs = lambda: {}
+
+    async def fake_preflight(name, command, args):
+        return command, args
+
+    with (
+        patch("hermes_cli.process_identity.attach_self_to_kill_on_close_job", attach_mock),
+        patch("tools.mcp_tool.StdioServerParameters"),
+        patch("tools.mcp_tool.stdio_client", return_value=(mock_stdio_cm := MagicMock())),
+        patch("tools.mcp_tool.ClientSession", return_value=(mock_session_cm := MagicMock())),
+        patch("tools.mcp_tool._preflight_stdio_command", fake_preflight),
+        patch("tools.mcp_tool_lifecycle._snapshot_child_pids", return_value=set()),
+        patch("tools.mcp_tool_lifecycle._kill_orphaned_mcp_children"),
+        patch("tools.mcp_tool_config._write_stderr_log_header"),
+    ):
+        mock_stdio_cm.__aenter__ = AsyncMock(return_value=(object(), object()))
+        mock_stdio_cm.__aexit__ = AsyncMock(return_value=False)
+        mock_session_cm.__aenter__ = AsyncMock(return_value=mock_session)
+        mock_session_cm.__aexit__ = AsyncMock(return_value=False)
+        import asyncio
+
+        # invoke the mixin method directly: start() would enter the reconnect loop because the
+        # mocked session never completes a real handshake.
+        asyncio.run(task._run_stdio({"command": "echo", "args": ["hello"]}))
+
+
+def test_windows_spawn_attaches_kill_on_close_job():
+    """On Windows the stdio spawn path must self-attach to a kill-on-close job (#61059)."""
+    attach = MagicMock(return_value=True)
+    _run_stdio_with_mocks("nt", attach)
+    attach.assert_called_once_with()
+
+
+def test_spawn_always_attempts_attach():
+    """The attach call is unconditional and self-guarded (no-op off Windows); the spawn
+    path must invoke it so Windows sessions get job coverage without any os.name checks
+    that would be hard to mock at the spawn site."""
+    attach = MagicMock(return_value=False)  # POSIX no-op return
+    _run_stdio_with_mocks("posix", attach)
+    attach.assert_called_once_with()
+
+
+def test_windows_attach_failure_never_blocks_spawn():
+    """A failed job attach (e.g. nested-job refusal) must not fail the MCP connection."""
+    attach = MagicMock(side_effect=OSError("access denied"))
+    _run_stdio_with_mocks("nt", attach)  # raises → would fail the test
+    attach.assert_called_once_with()
+
+
+class TestWindowsTreeKillHelpers:
+    """Best-effort contract of the tree-kill helpers (Win32 paths can't run here)."""
+
+    def test_tree_kill_swallows_missing_process(self):
+        import tools.mcp_tool_lifecycle as lifecycle
+
+        # Must not raise for a PID that raced away. (_kill_windows_process_tree carries no
+        # os.name gate itself — the gate lives at the _signal_mcp_process call site — so this
+        # exercises the helper directly without patching os.name, which breaks pathlib.)
+        lifecycle._kill_windows_process_tree(999999999, 15)
+
+    def test_tree_kill_terminates_descendants(self, monkeypatch):
+        import tools.mcp_tool_lifecycle as lifecycle
+
+        terminated = []
+        killed = []
+
+        class FakeProc:
+            def __init__(self, pid):
+                self.pid = pid
+
+            def children(self, recursive=True):
+                return [FakeProc(2), FakeProc(3)]
+
+            def terminate(self):
+                terminated.append(self.pid)
+
+            def kill(self):
+                killed.append(self.pid)
+
+        fake_psutil = MagicMock()
+        fake_psutil.Process = FakeProc
+        fake_psutil.wait_procs = lambda procs, timeout: ([], [])  # all exited gracefully
+        monkeypatch.setitem(__import__("sys").modules, "psutil", fake_psutil)
+
+        lifecycle._kill_windows_process_tree(1, 15)
+        # SIGTERM pass: descendants only — the direct child was already signalled by the caller.
+        assert sorted(terminated) == [2, 3]
+        assert killed == []
+
+        # Force pass: survivors of the wait are killed. wait_procs reports everyone alive.
+        fake_psutil.wait_procs = lambda procs, timeout: ([], list(procs))
+        lifecycle._kill_windows_process_tree(1, 9)
+        assert sorted(killed) == [2, 3]
+
+    def test_ledger_tree_kill_helper_swallows_errors(self):
+        """_kill_process_tree_windows never raises, even for a vanished process."""
+        from hermes_cli.process_identity import _kill_process_tree_windows
+
+        class Boom:
+            def children(self, recursive=True):
+                raise OSError("gone")
+
+            def terminate(self):
+                raise OSError("gone")
+
+        _kill_process_tree_windows(Boom())  # must not raise

tools/mcp_tool_lifecycle.py

@@ -266,6 +266,36 @@ def _signal_mcp_process(pid: int, sig: int, server_name: str, pgid: Optional[int
         os.kill(pid, sig)
     except (ProcessLookupError, PermissionError, OSError):
         pass
+    if os.name == "nt":  # Windows has no pgid reaching reparented grandchildren — kill the tree
+        _kill_windows_process_tree(pid, sig)
+
+
+def _kill_windows_process_tree(pid: int, sig: int) -> None:
+    """Windows counterpart of the POSIX killpg path (#61059): after the direct child is signalled,
+    terminate every still-alive descendant (npx.cmd → node.exe) so graceful teardown cannot leave
+    orphans reparented with ParentId=null. Best-effort, per-descendant; never raises."""
+    import signal as _signal
+    try:
+        import psutil
+    except ImportError:
+        return
+    try:
+        parent = psutil.Process(pid)
+        descendants = parent.children(recursive=True)
+    except (psutil.NoSuchProcess, psutil.AccessDenied, OSError):
+        return
+    for child in descendants:
+        try:
+            child.terminate()
+        except Exception:  # noqa: BLE001 - raced away or refused; sweep continues
+            pass
+    if sig == getattr(_signal, "SIGKILL", _signal.SIGTERM):  # force pass: don't wait for graceful exit
+        _, alive = psutil.wait_procs(descendants, timeout=0)
+        for child in alive:
+            try:
+                child.kill()
+            except Exception:  # noqa: BLE001
+                pass
 
 
 def _kill_orphaned_mcp_children(include_active: bool = False, server_name: Optional[str] = None) -> None:

tools/mcp_tool_transport.py

@@ -363,6 +363,18 @@ class MCPServerTransportMixin:
             command=command, args=args, env=safe_env or None, cwd=stdio_cwd,
             # Windows pipes can split non-UTF-8 bytes at chunk boundaries; substitute, don't raise.
             encoding_error_handler="replace")
+        # Windows has no POSIX parent-death supervisor / killpg safety net (#61059): when this
+        # process dies ungracefully (crash, force-quit), the stdio child trees — npx.cmd →
+        # node.exe — survive as orphans with ParentId=null and pile up across restarts. Attaching
+        # THIS process to a KILL_ON_JOB_CLOSE job before the spawn makes every child (and
+        # grandchild) created after it die with the parent at the kernel level, so no orphan can
+        # outlive us. Idempotent; BREAKAWAY_OK keeps deliberate breakaway children escaping.
+        # Self-guards: a cheap no-op returning False on non-Windows.
+        try:
+            from hermes_cli.process_identity import attach_self_to_kill_on_close_job
+            attach_self_to_kill_on_close_job()
+        except Exception:
+            logger.debug("job-object self-attach failed before stdio spawn", exc_info=True)
         # Reap orphans of prior attempts first (else retries pile up zombie pairs); unscoped on purpose;
         # off-loop because the reaper blocks up to 2s.
         await asyncio.to_thread(_lifecycle._kill_orphaned_mcp_children)