Skip to content

fix: kill entire CLI process tree on stop/forceStop (Windows) - #2073

Open
rinceyuan wants to merge 2 commits into
github:mainfrom
rinceyuan:fix/windows-process-tree-kill
Open

fix: kill entire CLI process tree on stop/forceStop (Windows)#2073
rinceyuan wants to merge 2 commits into
github:mainfrom
rinceyuan:fix/windows-process-tree-kill

Conversation

@rinceyuan

Copy link
Copy Markdown
Contributor

Summary

Fix process tree leak on Windows when stop()/forceStop() terminates the CLI.

On Windows, ChildProcess.kill() (Node.js) and Popen.terminate()/kill() (Python) only terminate the immediate process via TerminateProcess(), leaving grandchildren orphaned. Each create_session()/stop() cycle leaks a full copilot process tree (~3-5 processes) that survives until idle timeout or OOM.

Changes

SDK Fix
Node.js (client.ts) New killProcessTree() helper: uses taskkill /T /F /PID on Windows, falls back to standard .kill(signal) on Unix
Python (client.py) New _kill_process_tree() helper: uses taskkill /T on Windows, os.killpg() on Unix, with fallback to proc.kill()

Validation

  • TypeScript compiles clean (tsc --noEmit)
  • Python syntax validated (ast.parse)
  • Manual Windows test: spawned a 2-level node process tree, confirmed taskkill /T kills both parent and grandchild (see test output below)

parent=47632 child=27936 pre:parent alive pre:child alive taskkill OK post:parent DEAD(OK) post:child DEAD(OK)

Notes

  • Go, .NET, Rust, and Java SDKs may have similar issues but use different process management; those are left for follow-up.
  • The Unix path uses os.killpg() in Python (requires the child to be a process group leader); falls back gracefully if not applicable.

Closes #1804

On Windows, ChildProcess.kill() / Popen.terminate() only terminates the
immediate process via TerminateProcess(), leaving grandchildren (copilot
workers, MCP servers, etc.) orphaned. Each session cycle leaks a full
process tree that accumulates until OOM.

Fix: use \	askkill /T /F /PID\ on Windows to terminate the entire tree.
On Unix, use os.killpg() in Python for process group cleanup.

Node.js (client.ts):
- Add killProcessTree() helper wrapping taskkill /T on Windows
- Replace child.kill() in stop() and forceStop() with killProcessTree()

Python (client.py):
- Add _kill_process_tree() helper with taskkill /T (Windows) and
  os.killpg() (Unix) strategies
- Replace self._cli_process.terminate()/kill() in stop() and
  force_stop() with _kill_process_tree()

Tested: spawned a 2-level process tree on Windows, confirmed both parent
and child are terminated after taskkill /T.

Closes github#1804
@rinceyuan
rinceyuan requested a review from a team as a code owner July 24, 2026 03:33
@rinceyuan

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree company=Microsoft

@rinceyuan

Copy link
Copy Markdown
Contributor Author

@stephentoub This fixes the Windows process tree leak reported in #1804. Affects both Node.js and Python SDKs — each stop()/forceStop() cycle was orphaning the CLI's child processes. The fix uses \ askkill /T\ on Windows. Manually verified on Windows 11. Happy to add the Go/.NET fixes in a follow-up if desired.

Add CopilotClientOptions.processGroup (default: false). When enabled:

- The CLI child is spawned with detached:true on Unix, making it a
  process group leader (PGID == PID).
- killProcessTree() uses process.kill(-pid, signal) to signal the
  entire group atomically, reaping all descendant processes (tool
  subprocesses, MCP servers, shells) instead of only the immediate child.
- On Windows this is a no-op — taskkill /T already handles tree kill.
- The detached child is unref'd to prevent keeping the event loop alive.

This enables embedders running long-lived daemon hosts to guarantee the
agent's full process tree is confined and reaped on stop/crash, without
relying on the OS to reparent orphans.

Partially addresses github#1935
@SteveSandersonMS

Copy link
Copy Markdown
Contributor

The way this is implemented in the PR currently won’t work because:

  • Python calls killpg() without starting the CLI in a separate process group, which can kill the host and sibling processes.
  • Node’s process-group behavior is opt-in and defaults to false, so normal POSIX clients retain the leak.
  • Python does not check whether taskkill succeeded.
  • The change covers only Node and Python, despite the same lifecycle requirement applying across SDKs.
  • There are no real process-tree tests, and the existing Node lifecycle test fails.

There is a small, coherent cross-language change we could accept: add one private “terminate owned runtime tree” operation per SDK, called from the existing owned-process termination point.

Its behavior should be:

Windows: taskkill /T /F /PID <root>
POSIX:   signal the runtime’s private process group

POSIX also needs one small spawn-time change to place the runtime in its own process group/session. Otherwise group termination could kill the host. No public API is needed.

Per language, this is approximately:

SDK Spawn-time isolation Teardown
Node detached: true process.kill(-pid, signal)
Python start_new_session=True os.killpg(pid, signal)
Go SysProcAttr.Setpgid = true syscall.Kill(-pid, signal)
Rust process_group(0) signal negative PID
Java Use ProcessHandle.descendants() snapshot descendants, kill them, then root
.NET None needed existing Kill(entireProcessTree: true)

Each SDK should call that helper from the process-termination section used by stop() and forceStop(). External-server and in-proc paths must not call it.

This is about the smallest useful implementation across all languages and OSes:

  • one private helper per SDK;
  • one POSIX spawn flag per applicable SDK;
  • no public options;
  • no Job Objects;
  • no crash-cleanup guarantee;
  • no redesign of graceful shutdown.

The tests can also be narrow: start a helper process that starts one long-lived child, then verify both disappear after stop() and forceStop() on Windows and POSIX. Also verify external and in-proc modes do not enter tree termination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CopilotClient.stop() leaks the CLI server's child process tree on Windows (orphaned node/copilot.exe per session)

2 participants