Skip to content

fix(util): timed-out shell commands no longer leave child processes running - #62815

Open
only21mil wants to merge 1 commit into
anomalyco:devfrom
only21mil:kill-whole-group
Open

only21mil wants to merge 1 commit into
anomalyco:devfrom
only21mil:kill-whole-group

Conversation

@only21mil

@only21mil only21mil commented Nov 9, 2026 •

Copy link
Copy Markdown

Issue for this PR

Related to #59006, which asks for a way to stop an agent's whole process tree. No issue covers this exact case, so this PR fixes one focused part of it: what a shell timeout or abort leaves behind.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

When a shell command times out or is aborted, stop sends SIGTERM to the command's process group. It escalates to SIGKILL only if the leader (sh) is still running when forceKillAfter ends. The leader usually exits on SIGTERM at once, so the escalation never happens. A child that outlives the SIGTERM (one that handles or ignores it, or one started while the signal went out) keeps running with no parent, and nothing tracks the group any more. A command that keeps starting child processes can then run until the user's process limit is used up.

With forceKillAfter set, stop now waits for the whole group to empty, not just the leader. If the grace period ends with members left, it sends SIGKILL to the group every 10 ms until kill reports ESRCH, for at most one second. A group ID stays reserved while any member is alive, and the loop stops at the first ESRCH, so it only signals the command's own group.

Every stop with forceKillAfter gets this: the shell tool's timeout and abort, and scope release with RELEASE_FORCE_KILL_AFTER, for example a command that exits non-zero or an MCP stdio server. Windows (taskkill /T), detached: false and stops without forceKillAfter are unchanged. A command that exits 0 still leaves its background children running, as before.

This is separate from #61876, which moves shells into the process daemon. It works on the current spawner and doesn't depend on that PR.

How did you verify your code works?

  • New test in packages/core/test/effect/cross-spawn-spawner.test.ts. The leader exits on SIGTERM while its child ignores SIGTERM and starts one short sleep after another (one at a time, for at most 10 s). It fails on dev because the group is still alive after kill, and passes with this change. Checked on Linux x86_64 and macOS 27 arm64.
  • From packages/core, run bun run script/test.ts test/effect/cross-spawn-spawner.test.ts (33 pass, 1 skip, 0 fail) and the seven shell/MCP/process files listed in the evidence (206 pass, 24 skip, 0 fail). Run bun check --no-script and tsc --noEmit in packages/util, and bun check --no-script -b -p tsconfig.tests.json in packages/core; run Prettier and oxlint on the two changed files. The Linux and macOS before/after runs and the timeout transcript also pass.
  • Runtime check: a short script runs that command through the spawner, waits 1 s, calls handle.kill({ forceKillAfter: "3 seconds" }) as the shell tool's timeout does, and lists the group. On dev, kill returns in 3 ms and two members are still there 4 s later, still starting new sleep processes. With this change, kill returns after the 3 s grace period and the group is empty. macOS gives the same result.

Not checked: Windows, whose code path is unchanged.

Screenshots / recordings

No UI change. Below are test and runtime output from Linux, before and after.

The new test on dev: it fails because the process group is still alive after kill.

New test failing on dev

The new test with this change: it passes.

New test passing with this change

A timeout on dev: kill returns at once, and the group keeps two members that are still starting new processes 4 s later.

Process group still running after a timeout on dev

A timeout with this change: kill returns after the 3 s grace period, and the group is empty.

Process group empty after a timeout with this change

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly


Added by the opencode bot (AI-generated).

Related: #59006, #53564

A stop with forceKillAfter sent SIGTERM to the process group and escalated
to SIGKILL only while the leader was still running. When the leader exited
on SIGTERM, any child that outlived the signal kept running, and could keep
starting more processes, with nothing tracking the group.

After the first signal, wait for the whole group to empty. When the grace
period ends, send SIGKILL to the group until it has no members, for at most
one second.

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: the fix works on a normal Linux host, but it adds a delay of about 4 s to every shell timeout or abort when opencode runs as PID 1, which is what the official Docker image does.

How I checked

  • In a PID namespace whose init reaps processes, the new test fails on the base and passes with this change. The whole spawner test file also passes there.
  • In this sandbox, PID 1 doesn't reap orphaned processes, so dead group members stay as zombies. kill(-pgid, 0) keeps succeeding on zombies, so the group never looks empty.
  • In that setup, I timed handle.kill({ forceKillAfter: "3 seconds" }) on sh -c 'sleep 30 & echo $!; wait', where sleep exits on SIGTERM. It takes 2 ms on dev and 4002 ms with this change: the full grace period, then the full 1 s SIGKILL loop. Under a reaping init it takes 2 ms.
  • The new test also fails in that setup.

Who is affected: packages/cli/Dockerfile uses ENTRYPOINT ["opencode"] with no init, so opencode is PID 1 and never reaps orphans (#53564). In that image, any timed-out or aborted command with a background child now costs forceKillAfter + 1 s, as does any scope release that goes through stop. Details and options are in the inline comment.

Public API: unchanged.

Related: #59006, #53564

// Descendants can outlive the leader in its group. A group ID stays reserved until its last member is reaped, so
// signalling the group until ESRCH stays on this group.
const grouped = globalThis.process.platform !== "win32" && (command.options.detached ?? true)
const untilGroupGone = (signal: NodeJS.Signals | 0) =>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kill(-pgid, 0) also succeeds while a group member is a zombie. When no process reaps orphans, the group never reaches ESRCH. That's the case when opencode is PID 1, as in the official Docker image (#53564), or in containers without --init.

In that case, every stop with forceKillAfter waits the full grace period, then runs the SIGKILL loop until its 1 s cap. This happens even when every member already died on SIGTERM. I measured 2 ms on dev and 4002 ms here for sh -c 'sleep 30 & wait' with a 3 s forceKillAfter.

Some possible ways out:

  • Don't count zombies as live members (on Linux, read the state in /proc/<pid>/stat).
  • Once the leader has exited, send one SIGKILL to the group without waiting for ESRCH.
  • Wait for the empty group only on the SIGKILL path, not during the grace period.


yield* handle.kill({ forceKillAfter: 100 })

expect(alive(group)).toBe(false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test assumes orphaned members get reaped. Where PID 1 doesn't reap them (for example a container with no init), they stay zombies, the group stays alive, and this assertion fails after about 1.1 s. It failed that way for me in a sandbox like that; it passes under a reaping init.

This branch has not been deployed

No deployments
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.

1 participant