Repository navigation
fix(coding-agent): give background processes an owner, a kind, and a real cleanup path - #349
Merged
Merged
Conversation
…real cleanup path
`run_in_background` documented "the process is terminated when the session
exits", but only called `trackDetachedChildPid` + `unref()`. There was no
`session_shutdown` handler, so the promise was never kept: a dev server started
in a test host outlived it for two days.
The detached-child registry was a global `Set<number>`, which caused three
separate problems:
- It knew a pid but not who owned it, so a session could not reclaim its own
trees without risking another session's.
- It knew no kind, so "the session is still working" (a subagent) and "a
process is still running" (a dev server the user is inspecting) were
indistinguishable. A session with a running dev server read as unfinished.
- It was only reclaimed by five hand-written calls in the CLI modes. Every
other host, including the Web/Desktop one, never called it at all.
Replace it with `core/process-registry.ts`: registrations carry `kind`
(`bash` | `subagent` | `workflow`), an owner session id, a label, and the log
path. `isUnfinishedWork()` answers the completion question and deliberately
excludes `bash`.
Reclamation now has four layers instead of one:
1. `releaseOwnerGracefully(ownerId)` sends SIGTERM, then SIGKILL after a grace
window. The session calls it from `session_shutdown`, which fires on delete,
new, resume and quit - not on a session switch or a fork, so a dev server
keeps running while the user looks at another conversation.
2. Abort and timeout use the same graceful path, so a shell's `trap ... TERM`
cleanup runs. They used to send an immediate SIGKILL.
3. `process.prependListener("exit")` is registered inside the registry, so any
host reclaims its children without opting in. This is what the Web/Desktop
host was missing.
4. `utils/parent-death-guard.ts` runs a detached IPC supervisor whose channel
doubles as a liveness lease. `SIGKILL` cannot be caught in-process, so this
is the only layer that covers a host that dies without running handlers.
`bash_list` and `bash_kill` let the model enumerate and stop its own session's
background commands, which it previously had to remember from tool results.
Verified on macOS: the guard test uses a child that ignores SIGTERM, so it only
passes on the SIGKILL escalation. `subagent-child-env` used to leave
`fake-child.mjs` processes behind; it now terminates them and leaves none.
liuedcson
requested review from
MelodyVAR,
Uking-xxx and
ZouR-Ma
as code owners
November 7, 2026 07:43
…mand sooner Two problems in the previous commit, found by review and by CI. `bash_kill` with `all: true` called `releaseOwnerGracefully(ownerId)` with no pid set. That function is owner-wide and kind-agnostic by design, which is right for session teardown but wrong here: a session running a subagent and a dev server would have lost both, while the tool description only promises to stop "background commands". Pass the pids the tool actually means, and record the dimension rule on `releaseOwnerGracefully` so the next narrow caller does not repeat it. The abort and timeout paths used the 3s teardown grace. A foreground command must look stopped the moment the user presses Escape or the deadline elapses: `test/suite/regressions/step-command-runtime-recovery.test.ts` asserts the tree is gone within 1s, and `json-mode-durability` spent its whole 60s budget waiting. Split the constant: 3s stays for session teardown, and an interrupted command gets `INTERRUPTED_COMMAND_GRACE_MS` (500ms). The call still waits for the tree to exit, so a `trap ... TERM` handler keeps its window; it just does not get three seconds to decide whether to honor it. Verified: `test/background-command-lifecycle.test.ts` gains a case where a session owns both a bash command and a subagent tree, and `bash_kill all` stops only the command. Six suites (60 tests) pass locally.
…h split-off descendants A foreground command is spawned detached too, so a host killed with SIGKILL left it running. The guard was armed only for `run_in_background`, which meant the most common command shape had no layer covering it at all — the same orphan, one code path over. Arm it for every command: the supervisor is one process per host, so this costs a lease rather than a process, and the lease is released when the command exits on its own. Separately, `terminateProcessTreeGracefully` only ever signalled `kill(-pid)`. A command that puts a child in its own session or process group (`setsid`, `nohup ... &`) is outside that group, so it survived both the SIGTERM and the SIGKILL. `killProcessTree` already walked Linux descendants for this reason; the graceful path did not. Share the walk, and take one snapshot before the first signal: after the SIGTERM the parent is usually gone, so a second `/proc` read would find nothing and the escalation would miss the very descendant that ignored the first signal. `test/suite/regressions/step-command-runtime-recovery.test.ts` covers exactly this shape on Linux; it was failing before this change. Verified: `test/foreground-command-guard.test.ts` kills a host that is running a real foreground command and asserts the command dies; 17 tests across the four process suites pass.
This was referenced Nov 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
run_in_backgrounddocumented that "the process is terminated when the session exits", but the implementation only calledtrackDetachedChildPid+unref(). Nosession_shutdownhandler existed, so the promise was never kept. A dev server started by a test host outlived it by two days, kept firing a durable cron job every minute, and produced about 6500 billed model requests.The detached-child registry was a global
Set<number>. That single data structure caused three separate defects:实现
1. Kind- and owner-aware registry (
packages/coding-agent/src/core/process-registry.ts)A registration carries
kind(bash|subagent|workflow), the owning session id, a label, the working directory, and the log path.isUnfinishedWork(kind)answers the completion question and deliberately excludesbash: a background command is a side effect the user inspects, not work the session is still performing.subagentandworkflowcount.修复前: a single
Set<number>with no way to ask either question.2. Graceful termination (
packages/coding-agent/src/utils/process-tree.ts)terminateProcessTreeGracefully()sends SIGTERM to the process group, then SIGKILL afterPROCESS_TREE_TERM_GRACE_MS. Abort and timeout paths use it, so a shell'strap ... TERMcleanup runs.修复前:
killProcessTree()(immediate SIGKILL, uncatchable) on both paths, so cleanup handlers were silently skipped.killProcessTree()remains for host exit, where waiting is not an option.3. Four reclamation layers (was one)
releaseOwnerGracefully(ownerId)fromsession_shutdownreleaseAllProcessesSync()onprocess.exit, registered inside the registryprependListener("exit"), same registrationutils/parent-death-guard.ts)SIGKILL, which no in-process handler survivesThe guard's IPC channel doubles as a liveness lease: when the channel closes for any reason, the supervisor signals every armed group. Leases are keyed by a random id, so a stale disarm cannot cancel a live lease and a recycled pid is never signalled.
4. Model-facing controls
bash_listandbash_killlet the model enumerate and stop its own session's background commands. Previously it had to remember pids from earlier tool results, which a resumed or compacted session had lost. Both are scoped to the caller's session, so a subagent or fork cannot kill a dev server another conversation owns.Semantics
docs/orchestration-lifecycle.mdrecords this and the layer table.验证
New tests:
test/process-registry.test.ts(7) — kind separation, per-owner release, cross-session isolation,SIGTERMhandler runs before escalation, synchronous release.test/parent-death-guard.test.ts(2) — the SIGKILL case uses a child that ignores SIGTERM, so the test can only pass on the supervisor's SIGKILL escalation (502 ms ≈ 300 ms grace); a second test proves a disarmed lease does not report a normal exit as a parent death.test/background-command-lifecycle.test.ts(6) — the tool result carrieskind: "bash", a command dies with its session while another session's keeps running,bash_list/bash_killare session-scoped, cross-session kill is refused, the log file records output.Regression evidence:
test/subagent-child-env.test.tsused to leavefake-child.mjsprocesses behind (6 orphans per run, still alive after 15 s). It now terminates them: 0 orphans after a run, verified withpsand a 6 s wait.pnpm run checkpasses, including all 16 repository guards andcheck:public-boundary.Not verified: real Windows hosts. The supervisor's
taskkillpath is exercised by inference only.边界
logPath; nothing deletes it.bash;workflowkind is registered but has no producer yet.