Skip to content

fix: kill the agent's whole process group when ChildGuard drops#251

Open
SPIKESPIGEL404 wants to merge 1 commit into
agentclientprotocol:mainfrom
SPIKESPIGEL404:fix/child-guard-process-group
Open

fix: kill the agent's whole process group when ChildGuard drops#251
SPIKESPIGEL404 wants to merge 1 commit into
agentclientprotocol:mainfrom
SPIKESPIGEL404:fix/child-guard-process-group

Conversation

@SPIKESPIGEL404

Copy link
Copy Markdown

Fixes #249.

Problem

ChildGuard::drop kills only the immediate child. The dominant agent-distribution pattern (per the official registry) is a wrapper launcher — npx → node, uvx → python — so teardown kills the wrapper and orphans the real agent onto pid 1, where it does not reliably exit on stdin EOF. One leaked agent process per session teardown, for every AcpAgent consumer. Observed in the wild with @zed-industries/claude-code-acp lingering minutes after its client exited.

Fix

  • spawn_process builds the command as std::process::Command and calls process_group(0) (unix) before converting with async_process::Command::from, making the child its own process-group leader. Windows keeps the existing CREATE_NO_WINDOW path untouched.
  • ChildGuard::drop SIGKILLs the process group first (rustix::process::kill_process_group), then falls back to the existing direct-child kill() (a no-op double-tap on unix; the only behavior on non-unix). Group kill also covers the inverse failure — the wrapper dying on its own while its grandchild lives — because the group persists while any member does.

rustix is added as a unix-only dependency; it is already in the tree transitively via async-process, so no new supply chain.

Semantics note

Group-kill on drop means anything the agent's tree left running dies with the session. That is the natural reading of ChildGuard's existing contract ("kills the process when dropped") extended to what the process is in practice — a tree. An agent intentionally daemonizing long-lived helpers would need to double-fork out of the group, which is the conventional unix opt-out.

Not in this PR

  • Windows Job Objects — the equivalent tree-kill on Windows. Left as follow-up; behavior there is unchanged (immediate child only).

Testing

  • New regression test test_child_guard_kills_wrapper_chain_process_group: spawns a bash → bash inner.sh chain (mimicking npx → node; ; : defeats bash's exec optimization so the chain really is two processes), captures the grandchild's pid, drops the guard, and asserts the grandchild is gone.
  • Full crate suite passes; clippy (pedantic set) and rustfmt clean.
  • Battle-tested shape: the same approach (group-leader spawn + group kill) has been running in bitrouter's substrate against real claude-code-acp — zero surviving processes through a 4-level env → npx → npm → node chain (feat(acp): per-session ACP substrate (acp serve|prompt) bitrouter/bitrouter#613).

AcpAgent's ChildGuard only killed the immediate child. Agents are
commonly distributed behind wrapper launchers - the official agent
registry's dominant distribution is npx/uvx - so the spawned tree is
wrapper -> real agent. Killing the wrapper orphans the agent onto
pid 1, where it does not reliably exit on stdin EOF and can outlive
the client indefinitely (one leaked agent per session teardown).

The child is now spawned as its own process-group leader (unix) and
ChildGuard SIGKILLs the group before the direct-child kill fallback.
This also covers the wrapper dying on its own while its grandchild
lives. Windows keeps the existing single-kill behavior (Job Objects
would be the equivalent; left as follow-up).

Regression test spawns a bash->bash wrapper chain and asserts the
grandchild dies when the guard drops.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AcpAgent teardown orphans the real agent when the command is a wrapper launcher (npx/uvx)

1 participant