refactor: hide subagent implementation helpers

packages/subagent/subagent-subprocess/README.md

@@ -6,7 +6,7 @@ Every tunable is a **parameter**: the dispose ladder takes its grace periods per
 
 ## What it exports
 
-### `SENSITIVE_ENV_PATTERN` / `buildChildEnv(extra)`
+### `buildChildEnv(extra)`
 
 The credential env scrub (same pattern as the [bash executor](../../bash/bash-local/README.md)): the child env is the ambient env minus credential-shaped vars (`/KEY|SECRET|TOKEN/i`), with `extra` layered on top AFTER the scrub. `PATH`, `HOME`, `TMPDIR`, locale, and proxy vars survive, so the child CLI runs normally; the parent's own secrets never leak implicitly, while an explicitly supplied credential (the child's OWN key in a backend's `env` config) still reaches the child.
 
@@ -14,10 +14,6 @@ The credential env scrub (same pattern as the [bash executor](../../bash/bash-lo
 
 Spawn-failure capture: a promise that resolves (never rejects) with the child's first `error` event. A spawn failure such as `ENOENT` is an event, not a thrown exception — without a listener Node crashes the parent process — so call this in the same tick as `spawn()` and race it in the run's result path; a bad command then settles as an ordinary child-level failure. For a child that spawns cleanly the promise never settles.
 
-### `waitForExit(child)` / `exitsWithin(child, ms)`
-
-Exit waits over a `ChildProcess`: resolve once the child exits by any code or signal (immediately if it is already gone), or race that against a timer (`true` = exited in time). The race cleans up after itself on both outcomes — the pending timer is `unref()`ed and cleared on exit, the exit listener removed on timeout — so repeated calls (the dispose ladder's tiers, a poll loop) never accumulate listeners on the child.
-
 ### `disposeChildProcess(child, graces)`
 
 The three-tier dispose ladder. Resolves only once the child has ACTUALLY exited — quiescence reached, not merely requested (see [defensive patterns](../../../docs/defensive-patterns.md)):
@@ -28,6 +24,8 @@ The three-tier dispose ladder. Resolves only once the child has ACTUALLY exited
 
 The two graces (`DisposeLadderGraces`) come from the consuming plugin's `disposeEofGraceMs`/`disposeGraceMs` Config fields; the EOF window is deliberately a separate — usually wider — grace than the signal tier, since a cooperative child's EOF teardown may itself await a signal-trapping grandchild plus a final flush.
 
+The exit waits are internal to this ladder. They clean up their timer and listener on either outcome, so escalation never accumulates listeners on the child.
+
 ### `createIsolatedConfigDir(prefix, pinnedPath?)`
 
 A per-run isolated config directory for an external CLI child (the target of `CLAUDE_CONFIG_DIR` / `CODEX_HOME`-style redirection), so child behavior is a function of deployment config alone — never of whatever `~/.claude` / `~/.codex`-style state exists on the host. Returns an `IsolatedConfigDir` handle: `path` goes into the child env, `remove()` runs on dispose.

packages/subagent/subagent-subprocess/src/index.ts

@@ -32,7 +32,7 @@ import { join } from 'node:path'
  * the scrub, so an intended `DEEPSEEK_API_KEY` survives while an incidental
  * `AWS_SECRET_ACCESS_KEY` does not.
  */
-export const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i
+const SENSITIVE_ENV_PATTERN = /KEY|SECRET|TOKEN/i
 
 /**
  * The ambient env minus credential-shaped vars, plus the caller's explicit
@@ -72,7 +72,7 @@ export function spawnFailure(child: ChildProcess): Promise<Error> {
  * already gone.
  * @param child - the child process to await.
  */
-export function waitForExit(child: ChildProcess): Promise<void> {
+function waitForExit(child: ChildProcess): Promise<void> {
   if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve()
   return new Promise<void>(resolve => child.once('exit', () => { resolve() }))
 }
@@ -87,7 +87,7 @@ export function waitForExit(child: ChildProcess): Promise<void> {
  * @returns `true` if the child exits within `ms` (immediately if it is
  * already gone), `false` on timeout.
  */
-export function exitsWithin(child: ChildProcess, ms: number): Promise<boolean> {
+function exitsWithin(child: ChildProcess, ms: number): Promise<boolean> {
   if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true)
   return new Promise<boolean>((resolve) => {
     const onExit = (): void => {

packages/subagent/subagent-subprocess/tests/subagent-subprocess.spec.ts

@@ -9,10 +9,7 @@ import {
   buildChildEnv,
   createIsolatedConfigDir,
   disposeChildProcess,
-  exitsWithin,
-  SENSITIVE_ENV_PATTERN,
   spawnFailure,
-  waitForExit,
 } from '../src/index.ts'
 
 // `rm` is wrapped (real-passthrough by default) so ONE test can inject a
@@ -47,6 +44,8 @@ interface FakeChildScript {
   diesOn?: LethalTrigger
   /** Delay (ms) between the lethal trigger and the exit event. */
   delayMs?: number
+  /** Complete the scripted exit inside the triggering call. */
+  synchronousExit?: boolean
   /** `false` models a child spawned without a stdin pipe. */
   stdin?: boolean
 }
@@ -80,11 +79,13 @@ class FakeChild extends EventEmitter {
     // SIGKILL is uncatchable — it always fells the child; any other trigger
     // only when the scenario scripts it as the lethal one.
     if (trigger !== 'SIGKILL' && this.script.diesOn !== trigger) return
-    setTimeout(() => {
+    const exit = (): void => {
       if (trigger === 'eof') this.exitCode = 0
       else this.signalCode = trigger
       this.emit('exit', this.exitCode, this.signalCode)
-    }, this.script.delayMs ?? 0)
+    }
+    if (this.script.synchronousExit === true) exit()
+    else setTimeout(exit, this.script.delayMs ?? 0)
   }
 }
 
@@ -93,7 +94,7 @@ function asChild(fake: FakeChild): ChildProcess {
   return fake as unknown as ChildProcess
 }
 
-describe('buildChildEnv / SENSITIVE_ENV_PATTERN', () => {
+describe('buildChildEnv', () => {
   it('drops credential-shaped ambient vars (KEY/SECRET/TOKEN, case-insensitive)', () => {
     process.env.DSH_PROC_TEST_API_KEY = 'leak'
     process.env.dsh_proc_test_secret = 'leak'
@@ -111,7 +112,6 @@ describe('buildChildEnv / SENSITIVE_ENV_PATTERN', () => {
   })
 
   it('forwards normal ambient vars', () => {
-    expect(SENSITIVE_ENV_PATTERN.test('PATH')).toBe(false)
     expect(buildChildEnv({}).PATH).toBe(process.env.PATH)
   })
 
@@ -149,7 +149,7 @@ describe('spawnFailure', () => {
     const fake = new FakeChild({ diesOn: 'SIGTERM' })
     const failure = spawnFailure(asChild(fake))
     fake.kill('SIGTERM')
-    await waitForExit(asChild(fake))
+    await new Promise<void>(resolve => fake.once('exit', () => { resolve() }))
     // A clean lifecycle emits `exit`, never `error` — the capture stays
     // pending forever, so a race against it is decided by the other arms.
     const settled = await Promise.race([
@@ -160,51 +160,6 @@ describe('spawnFailure', () => {
   })
 })
 
-describe('waitForExit / exitsWithin', () => {
-  it('resolves immediately for a child that already exited by code', async () => {
-    const fake = new FakeChild()
-    fake.exitCode = 0
-    await expect(waitForExit(asChild(fake))).resolves.toBeUndefined()
-  })
-
-  it('resolves immediately for a child that already died by signal', async () => {
-    const fake = new FakeChild()
-    fake.signalCode = 'SIGTERM'
-    await expect(waitForExit(asChild(fake))).resolves.toBeUndefined()
-  })
-
-  it('resolves on the exit event of a live child', async () => {
-    const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 })
-    const exited = waitForExit(asChild(fake))
-    fake.kill('SIGTERM')
-    await expect(exited).resolves.toBeUndefined()
-    expect(fake.signalCode).toBe('SIGTERM')
-  })
-
-  it('exitsWithin resolves true immediately for an already-exited child (no listener attached)', async () => {
-    const fake = new FakeChild()
-    fake.exitCode = 0
-    await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true)
-    expect(fake.listenerCount('exit')).toBe(0)
-  })
-
-  it('exitsWithin resolves true when the child exits inside the window', async () => {
-    const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 })
-    fake.kill('SIGTERM')
-    await expect(exitsWithin(asChild(fake), 1000)).resolves.toBe(true)
-    // The once-listener fired and the grace timer was cleared — nothing lingers.
-    expect(fake.listenerCount('exit')).toBe(0)
-  })
-
-  it('exitsWithin resolves false on timeout for a child that never exits', async () => {
-    const fake = new FakeChild() // nothing short of SIGKILL fells it; no signal sent
-    await expect(exitsWithin(asChild(fake), 20)).resolves.toBe(false)
-    // The timeout arm removed its exit listener: repeated waits (a poll loop,
-    // the ladder's tiers) never accumulate listeners on the same child.
-    expect(fake.listenerCount('exit')).toBe(0)
-  })
-})
-
 describe('disposeChildProcess', () => {
   it('returns immediately for an already-exited child (no EOF, no signals)', async () => {
     const fake = new FakeChild()
@@ -230,12 +185,28 @@ describe('disposeChildProcess', () => {
     expect(fake.exitCode).toBe(0)
   })
 
+  it('recognizes a child that exits synchronously on stdin EOF', async () => {
+    const fake = new FakeChild({ diesOn: 'eof', synchronousExit: true })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1000, disposeGraceMs: 1000 })
+    expect(fake.exitCode).toBe(0)
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
+
   it('tier 2: a child that ignores EOF but honors SIGTERM dies on the middle rung', async () => {
     const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 })
     await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 })
     expect(fake.stdinEnded).toBe(true)
     expect(fake.kills).toEqual(['SIGTERM'])
     expect(fake.signalCode).toBe('SIGTERM')
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
+
+  it('recognizes a child that exits synchronously on SIGTERM', async () => {
+    const fake = new FakeChild({ diesOn: 'SIGTERM', synchronousExit: true })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 })
+    expect(fake.kills).toEqual(['SIGTERM'])
+    expect(fake.signalCode).toBe('SIGTERM')
+    expect(fake.listenerCount('exit')).toBe(0)
   })
 
   it('tier 3: a SIGTERM-trapping child is SIGKILLed, and dispose resolves only after the exit', async () => {
@@ -247,6 +218,13 @@ describe('disposeChildProcess', () => {
     expect(fake.signalCode).toBe('SIGKILL')
   })
 
+  it('recognizes a child already gone when the final exit wait begins', async () => {
+    const fake = new FakeChild({ synchronousExit: true })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 })
+    expect(fake.kills).toEqual(['SIGTERM', 'SIGKILL'])
+    expect(fake.signalCode).toBe('SIGKILL')
+  })
+
   it('walks the ladder for a child spawned without a stdin pipe', async () => {
     const fake = new FakeChild({ stdin: false, diesOn: 'SIGTERM', delayMs: 5 })
     await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 })