fix(tools): kill the whole process tree on Windows terminal timeouts

[lEWFkRAD]

What does this PR do?

Fixes a Windows-only process leak in LocalEnvironment._kill_process (tools/environments/local.py).

On POSIX, _kill_process kills the entire process group — _spawn_process calls os.setsid, and the kill path does killpg(SIGTERM) → wait → killpg(SIGKILL), with a comment explicitly warning that returning early "leaves orphaned process-group members behind."

On Windows, the branch was a single proc.terminate(). That reaches only the direct child — the shell wrapper — and every grandchild survives as an orphan whenever a terminal command hits its timeout or is interrupted.

Production impact (Windows 11): during a gateway restart window in which the agent's terminal tool logged repeated 5s timeouts ([Command timed out after 5s], rc 124), four orphaned hermes.exe → python.exe pairs were left behind, burning 0.5–1.0 CPU cores each for 7–9 hours (~22 CPU-hours total) until killed manually. The orphaning mechanism itself reproduces deterministically (step 1 below), independent of that incident.

Why this approach: Windows has no process group for this spawn path, so explicit tree enumeration is the only way to reach grandchildren. The fix mirrors two existing in-repo patterns: the psutil children(recursive=True) sweep in gateway/platforms/whatsapp.py, and the exact replacement scripts/check-windows-footguns.py prescribes (line ~215). psutil is already a hard dependency. The old proc.terminate() is kept as the fallback when the sweep cannot complete, so the new path never does less than the old one. Adjacent prior art: #40110 fixed the same bug class for shell-wrapper MCP servers.

Related Issue

No existing issue — found via production incident forensics on a Windows 11 deployment. Happy to open one first if preferred.

Type of Change

• ðŸ�› Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/environments/local.py — new module-level _kill_process_tree(pid, *, timeout) helper: psutil sweep of [root] + children(recursive=True), wait_procs verification, one re-enumeration pass for children spawned mid-kill; returns True only when everything is verified gone. The _IS_WINDOWS branch of _kill_process now calls it and falls back to the previous proc.terminate() if the sweep can't finish, then reaps the wrapper handle.
• tests/tools/test_local_env_tree_kill.py — three new tests (details below). The helper is platform-neutral, so they run on all platforms with no skips, including the test that exercises the real Windows branch via monkeypatched _IS_WINDOWS.

How to Test

1. Repro the bug on the unpatched tree (Windows): spawn python -c "<parent that spawns a sleeping grandchild>", call LocalEnvironment._kill_process(parent), observe the grandchild survives:

   parent pid=38548  grandchild pid=37592
   after LIVE _kill_process: parent=False grandchild=True
   => BUG REPRODUCED: grandchild ORPHANED by proc.terminate()-only kill
   

2. Run the new tests on this branch:

   tests/tools/test_local_env_tree_kill.py::test_kill_process_tree_kills_grandchildren PASSED
   tests/tools/test_local_env_tree_kill.py::test_kill_process_tree_on_missing_pid_returns_true PASSED
   tests/tools/test_local_env_tree_kill.py::test_windows_branch_uses_tree_kill PASSED
   ============================== 3 passed in 0.74s ==============================
   

   (Windows 11, Python 3.12.10, pytest 9.0.2, --timeout-method=thread per the Windows runner caveat.)
3. Regression check — full local-environment suite (test_base_environment, test_local_interrupt_cleanup, test_local_background_child_hang, test_local_env_blocklist, test_local_env_cwd_recovery, test_local_env_windows_msys, test_local_shell_init + the new file): 16 failed / 76 passed with this patch vs 16 failed / 73 passed on clean origin/main — the 16 are identical pre-existing POSIX-assumption failures on Windows (Homebrew PATH, / root cwd, bashrc sourcing); this PR adds the 3 new passes and changes nothing else.
4. scripts/check-windows-footguns.py --diff origin/main: ✓ No Windows footguns found (2 file(s) scanned).

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate (closest: fix(mcp): recursive descendant PID tracking for shell-wrapper MCP servers #40110, different subsystem)
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — relevant suite run on Windows 11 with --timeout-method=thread; the only failures are 16 pre-existing POSIX-assumption failures identical on clean origin/main (counts above); the hermetic Linux runner is unaffected by this diff
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11 (10.0.26200), Python 3.12.10

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — docstrings on the new helper and branch comment; no user-facing docs affected
• I've updated cli-config.yaml.example if I added/changed config keys — N/A, no config changes
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — POSIX path untouched; helper is platform-neutral and tested on all platforms
• I've updated tool descriptions/schemas if I changed tool behavior — N/A, no schema changes

Screenshots / Logs

Before (unpatched, Windows 11 — grandchild orphaned):

parent pid=38548  grandchild pid=37592
alive before kill: parent=True grandchild=True
after LIVE _kill_process: parent=False grandchild=True
=> BUG REPRODUCED: grandchild ORPHANED by proc.terminate()-only kill

After (this branch, same machine):

tests/tools/test_local_env_tree_kill.py::test_kill_process_tree_kills_grandchildren PASSED [ 33%]
tests/tools/test_local_env_tree_kill.py::test_kill_process_tree_on_missing_pid_returns_true PASSED [ 66%]
tests/tools/test_local_env_tree_kill.py::test_windows_branch_uses_tree_kill PASSED [100%]
============================== 3 passed in 0.74s ==============================

Incident telemetry that motivated the fix (orphans found burning cores hours after their 5s timeout):

PID 23476  "hermes gateway status"            19,328 CPU-s   (~54% of a core, 9.5h)
PID 24532  "hermes gateway status"            16,428 CPU-s
PID 28852  "hermes gateway status"            17,364 CPU-s
PID 32384  "hermes photon webhook register…"  25,833 CPU-s   (~103% of a core, 7h)

🤖 Generated with Claude Code

[liuhao1024]

Verification: process-tree kill on Windows timeouts

Reviewed the full diff — this is a clean fix for a real Windows orphan-process problem.

• _kill_process_tree correctly uses psutil to enumerate and kill the full descendant tree, with a two-sweep strategy to catch children spawned between enumeration and the first kill
• Graceful fallback: if psutil is not installed or the sweep fails, the caller falls back to the legacy single-process terminate()
• The _IS_WINDOWS monkeypatch test exercises the real Windows code path on all platforms — good cross-platform CI coverage
• Error handling covers NoSuchProcess and AccessDenied at every level

No issues found. The implementation matches the existing whatsapp.py psutil sweep pattern already in the codebase.

[austinpickett]

✅ Approved

The bug: On Windows, _kill_process called bare proc.terminate() which kills only the direct child (the shell wrapper). Grandchildren survived as orphans after a terminal timeout — observed in production as python.exe processes burning CPU for hours.

The fix: Introduces _kill_process_tree(pid) using psutil for a recursive tree kill, replacing the bare terminate() on Windows. Two-sweep strategy (enumerate → kill → re-enumerate survivors → kill again) handles processes spawned between enumeration and the first kill. Falls back to the legacy single-process kill if psutil is unavailable or the sweep fails. The POSIX killpg path is unchanged.

Tests: 3 tests — grandchild survival (the exact production bug), missing-PID no-op, and the LocalEnvironment integration test confirming the code path is exercised.

Reviewed by Hermes Agent

[liuhao1024]

Verification comment — reviewed the diff, found no issues.

The _kill_process_tree implementation correctly uses psutil to enumerate and kill all descendant processes, with a two-sweep strategy to catch children spawned between enumeration and kill. The Windows branch in _kill_process properly falls back to the legacy proc.terminate() if the tree sweep fails. Test coverage includes grandchild kill verification, missing PID handling, and a cross-platform Windows branch exercise via monkeypatched _IS_WINDOWS. Clean fix for a real orphan-process issue on Windows.

[lEWFkRAD]

Approved by austinpickett, verified by liuhao1024 (two independent verifications). Clean fix for Windows process tree kill on timeouts. Ready for merge.

[lEWFkRAD]

Approved by austinpickett + verified by liuhao1024. Ready.

[lEWFkRAD]

Closing: upstream e5253d8 (fix(desktop): tree-kill Windows terminal descendants, merged via #55547) rewrote the same Windows branch of LocalEnvironment._kill_process to route through gateway.status.terminate_pid (taskkill /PID /T /F - a full tree kill with hide flags), with a proc.kill() fallback and regression tests. The intent of this PR (no orphaned grandchildren on Windows terminal timeouts) is fully served on main, so there is nothing left to salvage here.