Make the SDK own the whole CLI process tree

[dandriscoll]

Summary

• Give the Rust SDK a small cross-platform process-tree containment
  primitive (rust/src/process_tree.rs): a dedicated process group on
  Unix, a Job Object carrying JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE on
  Windows.
• Route Client::stop, Client::force_stop, and Drop for ClientInner
  through it, so all three now terminate the CLI's whole process tree
  (the CLI and any descendant it spawned: an MCP server, a shell tool,
  a subagent process), not only the CLI itself.
• Both the stdio and TCP spawn paths attach containment through the same
  Client::build_command sequence, so they get identical ownership.
• Attaching containment failing degrades to the previous root-only
  teardown rather than turning a containment failure into a hard start
  failure (relevant on Windows, where Job Object setup can fail; Unix
  attachment is established at fork and cannot practically fail once
  spawn() succeeds). An actual OS termination call failing is still
  surfaced: collected into StopErrors for stop, logged for
  force_stop/Drop.
• The tracked root child is reaped exclusively through Tokio's
  Child::{wait,try_wait,kill,start_kill}; nothing added here calls a
  raw waitpid on that pid, so there is no risk of the SDK and Tokio
  racing to reap (and neither ends up with ECHILD) the same process.

Design notes (relative to an earlier attempt at this)

An earlier revision of #2292 explored the same shape and was reset to a
narrower fix before merge, for two kinds of reasons worth answering here
directly:

• Scope: the bug that PR actually fixed (orphans with zero
  descendants) was a reference-cycle in the lifecycle dispatcher, already
  fixed independently and unrelated to descendant containment. This PR is
  about descendant containment specifically, a different, forward-looking
  requirement (prove a descendant, and a resource it holds, actually gets
  released), not a resubmission of that bug fix.
• Implementation concerns raised in review, addressed here:
  • Windows attachment does not spawn the process suspended or scan
    threads via CreateToolhelp32Snapshot; it assigns the Job Object
    immediately after a normal spawn. This accepts a small window where a
    child that spawns its own descendant extremely fast could escape
    containment (documented in the module), in exchange for not adding
    CREATE_SUSPENDED/ResumeThread/snapshot-scan complexity or turning a
    transient Job Object failure into a hard CLI start failure.
  • Nothing here reaps descendants with a raw waitpid. The earlier
    version's waitpid(-pgid, ...) loop competed with Tokio's own
    ownership of the root child's pid. Descendant liveness here is checked
    with a signal-0 existence probe, and the group/job is only ever
    signalled (never waited on) directly.
  • Termination reads the child/tree out of the same Mutex<Option<_>>>
    slots the existing code already used for the root child, taken once;
    nothing here re-signals a process group id after the slot has been
    emptied, so a since-recycled pgid is never addressed.
  • This PR is scoped to the Rust SDK only, at the requester's explicit
    ask. It does not attempt cross-SDK parity with .NET's
    Kill(entireProcessTree: true) or the direct-child-only Node/Python/
    Go/Java SDKs. See Unknowns.

Validation

• cd rust && cargo test --no-default-features --features test-support --lib
  : 220 passed, 0 failed (215 pre-existing + 5 new: 3 in
  process_tree::tests, 2 Client-level in lib.rs's existing test
  module).
• cargo +nightly-2026-04-14 fmt --all -- --config-path .rustfmt.nightly.toml --check
  : clean.
• cargo clippy --all-targets --features test-support -- --no-deps -D warnings -D clippy::unwrap_used -D clippy::disallowed_macros -D clippy::await_holding_invalid_type: clean. (The bundled-in-process
  feature variant of this exact CI command could not be exercised locally due
  to an unrelated transient network failure downloading that feature's CLI
  archive; this change does not touch that feature.)
• cargo doc --no-deps --features test-support with RUSTDOCFLAGS=-D warnings
  : clean.
• Before/after proof, twice, on the same tests:
  1. Temporarily forced process_tree::attach to always return None
     (simulating the containment feature not existing at all): both new
     Client-level tests (force_stop_..., stop_...) failed within
     ~15s with "descendant survived Client::force_stop" /
     "...Client::stop" respectively. The process-tree unit test failed
     immediately on its own attach(...).expect(...).
  2. Temporarily made ProcessTree::terminate a no-op while leaving
     attachment itself intact (simulating "group/job termination
     disabled" specifically): the process-tree unit test failed at
     "root did not exit within the timeout after process-tree
     termination" after ~15s.
  3. Restored both, reran the full suite: 220/220 passed again, in ~2
     seconds (no injected delay).

Unknowns

• Cross-SDK parity: this lands in the Rust SDK only, at the requester's
  explicit scope. .NET already does whole-tree termination; Node,
  Python, Go, and Java remain direct-child-only after this change.
• Unix containment is signal-based and requires this process's own
  teardown code to actually run; it provides no protection if this
  process itself is killed or crashes before calling terminate(). This
  is inherent to process groups (there is no Unix equivalent of Windows'
  JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE) and is documented in the module.
• The Windows assign-race window described above (a child spawning its
  own descendant faster than AssignProcessToJobObject can run) is
  accepted, not closed.
• Not exercised against the real Copilot CLI end-to-end (an MCP server or
  subagent process it spawns for real); coverage here uses a
  purpose-built helper process holding a real OS-enforced lock,
  standing in for "a descendant holding a resource."

Generated by Copilot, reviewed and adjusted by the author before commit.

[Copilot]

Copilot review overview

🟡 Changes recommended

Unix startup failures can still orphan descendants before process-tree ownership reaches ClientInner.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 3

New issues introduced by this change (4)

Severity	Finding
	rust/​src/​lib.rs — The process-tree handle is not yet owned by ClientInner, and dropping it does not terminate a…
	rust/​src/​lib.rs — The new whole-tree behavior in Drop has no regression coverage. The existing drop test constructs…
	rust/​src/​errors.rs — This public shutdown contract omits runtime-shutdown errors, although Client::stop collects them…
	rust/​src/​lib.rs — This unconditionally promises whole-tree termination, but startup intentionally permits containment…

What changed in this PR

Adds Rust SDK process-tree containment so CLI descendants are terminated during shutdown.

Changes:

• Adds Unix process groups and Windows Job Objects.
• Integrates containment into spawn and teardown paths.
• Adds process-level tests and updates shutdown documentation.

File	Description
rust/​src/​process_tree.rs	Implements cross-platform containment and tests.
rust/​src/​lib.rs	Integrates containment into client lifecycle.
rust/​src/​errors.rs	Updates shutdown error documentation.
rust/​Cargo.toml	Adds platform-specific dependencies.
rust/​Cargo.lock	Locks new dependencies.

Suppressed comments (2)

rust/src/errors.rs:413

• The documented ordering skips runtime shutdown, whose errors are actually inserted after session destroys and before process-tree termination. This makes the positional contract for errors() inaccurate.

    /// Borrow the collected errors as a slice, in the order they occurred
    /// (per-session destroys first, then process-tree termination, then
    /// the final child reap).

rust/src/lib.rs:2585

• This also overstates force_stop when Job Object attachment failed: the implementation then has no tree handle and kills only the root child. Document that fallback in this public API contract.

    /// process is wedged on I/O. Terminates the CLI's whole process tree
    /// (the CLI itself and any descendant it spawned) and sends a kill
    /// signal to the CLI child, without awaiting reaper completion, and
    /// immediately drops all per-session router state so dependent tasks
    /// observe a closed channel rather than a hang.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

The Windows tests use a nonexistent PROCESS_SYNCHRONIZE constant and will not compile.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	rust/​src/​process_tree.rs — windows-sys does not define PROCESS_SYNCHRONIZE; the standard access right accepted by…

Issues resolved since last review (4)

Severity	Finding
	rust/​src/​lib.rs — This unconditionally promises whole-tree termination, but startup intentionally permits containment… View resolved comment
	rust/​src/​errors.rs — This public shutdown contract omits runtime-shutdown errors, although Client::stop collects them… View resolved comment
	rust/​src/​lib.rs — The new whole-tree behavior in Drop has no regression coverage. The existing drop test constructs… View resolved comment
	rust/​src/​lib.rs — The process-tree handle is not yet owned by ClientInner, and dropping it does not terminate a… View resolved comment

[SteveSandersonMS]

Thanks for this, Dan — the process-tree containment mechanism itself (process_tree.rs) is well written and the idea is sound in principle.

I rebased this branch onto the latest main and resolved the conflicts in rust/src/lib.rs and rust/Cargo.toml (both from the recently merged extension_launch_provider work).

I tried to validate the fix end-to-end against the real CLI, using the three scenarios named in the PR description:

• Shell tool: I added a new E2E test that starts a shell command backgrounding a delayed side effect, then calls force_stop(). It failed — the descendant survived. Digging in, I found the CLI's own shell.exec puts each shell command in its own new process group (by design, so shell.kill can target one command without affecting siblings). That structurally escapes an SDK-level killpg on the root's group, even for an ordinary non-detached child.
• MCP server: I added an E2E test with a real MCP server subprocess and confirmed it does die after force_stop() — but as a negative control, I temporarily disabled this PR's process-tree kill entirely and reran the same test. The MCP server died anyway, in the same way. It turns out MCP servers here already exit on their own once the CLI closes their stdio pipes (EOF on stdin), regardless of this fix.
• Subagent: I couldn't find any evidence subagents are separate OS processes at all (the docs and RPC events describe them as in-process task delegation), so there doesn't seem to be anything for this fix to protect there either.

So right now I can't confirm this fix changes real-world behavior for any of the three cases named in the PR description, even though the underlying mechanism is correct and would matter for some other kind of descendant (e.g., a plain child process that isn't reparented and doesn't rely on stdio EOF for its own lifecycle).

Do you have a specific scenario in mind where you're confident this fix makes a real difference? If so, could you add an E2E test for that case (against the real CLI, not the synthetic self-re-exec test binary) so we can verify it and merge on that basis? Alternatively, if it's easier, we could close this for now until there's a more complete plan for which descendants this should actually cover.

For tracking, I've moved this to draft — please mark as ready to review when appropriate.

[SteveSandersonMS]

Closing this in favor of #2458, which has already merged and covers the path forward.