Skip to content

fix(coding-agent): give background processes an owner, a kind, and a real cleanup path - #349

Merged
liuedcson merged 3 commits into
mainfrom
fix/background-process-lifecycle
Nov 7, 2026
Merged

liuedcson merged 3 commits into
mainfrom
fix/background-process-lifecycle

Conversation

@liuedcson

Copy link
Copy Markdown
Collaborator

Background

run_in_background documented that "the process is terminated when the session exits", but the implementation only called trackDetachedChildPid + unref(). No session_shutdown handler 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. No owner. A session could not reclaim its own trees without risking another session's, so cleanup only ever ran at whole-process exit.
  2. No kind. "The session is still working" (a subagent) and "a process is still running" (a dev server the user is inspecting) were indistinguishable. A session whose reply was finished but whose dev server was still up read as unfinished.
  3. No default host. Five hand-written calls in the CLI modes were the only cleanup path. The Web/Desktop host never called it, so it leaked by construction, not by oversight.

实现

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 excludes bash: a background command is a side effect the user inspects, not work the session is still performing. subagent and workflow count.

修复前: 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 after PROCESS_TREE_TERM_GRACE_MS. Abort and timeout paths use it, so a shell's trap ... TERM cleanup 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)

Layer Mechanism Covers
1 releaseOwnerGracefully(ownerId) from session_shutdown session deleted, replaced, resumed away
2 releaseAllProcessesSync() on process.exit, registered inside the registry any host, including one that wrote no shutdown path
3 prependListener("exit"), same registration crash, uncaught exception
4 Detached IPC supervisor (utils/parent-death-guard.ts) SIGKILL, which no in-process handler survives

The 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_list and bash_kill let 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

Event Background command
Switching which session is viewed keeps running
Forking a session keeps running; the new session starts with none
Reloading settings or extensions keeps running
Session deleted, replaced, or resumed away stopped
Host exits stopped

docs/orchestration-lifecycle.md records this and the layer table.

验证

New tests:

  • test/process-registry.test.ts (7) — kind separation, per-owner release, cross-session isolation, SIGTERM handler 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 carries kind: "bash", a command dies with its session while another session's keeps running, bash_list/bash_kill are session-scoped, cross-session kill is refused, the log file records output.

Regression evidence:

  • test/subagent-child-env.test.ts used to leave fake-child.mjs processes behind (6 orphans per run, still alive after 15 s). It now terminates them: 0 orphans after a run, verified with ps and a 6 s wait.
  • pnpm run check passes, including all 16 repository guards and check:public-boundary.

Not verified: real Windows hosts. The supervisor's taskkill path is exercised by inference only.

边界

  • Log files are retained after their command stops, matching the choice to keep a stopped dev server inspectable. The registry carries logPath; nothing deletes it.
  • This does not add a session-scoped process tree API beyond bash; workflow kind is registered but has no producer yet.

…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.
…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.
@liuedcson
liuedcson merged commit a4141af into main Nov 7, 2026
2 checks passed
@liuedcson
liuedcson deleted the fix/background-process-lifecycle branch November 7, 2026 08:32
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