Merge branch 'codex/canonical-tool-output' into codex/code-mode-typed-results

packages/subagent/subagent-acp/README.md

@@ -12,7 +12,7 @@ The returned run id is minted in the parent namespace. The child server's sessio
 
 After publication, the provider sends the prompt and collects streamed `agent_message_chunk` text into `SubagentResult.output`. A prompt/transport failure resolves with `stopReason: 'error'`, or `aborted` when the required request signal or disposal requested cancellation.
 
-`dispose()` is idempotent. It removes the signal listener, requests ACP cancellation when possible, closes stdin, waits `disposeEofGraceMs`, escalates to SIGTERM, waits `disposeGraceMs`, and finally uses SIGKILL if necessary. Every run uses a fresh process; process pooling is not implemented.
+`dispose()` is idempotent. It removes the signal listener, requests ACP cancellation when possible, closes stdin, and waits `disposeEofGraceMs`. POSIX then escalates through SIGTERM and `disposeGraceMs` before SIGKILL; Windows force-terminates directly because Node maps both signals to `TerminateProcess`. After forced termination, every platform waits at most `disposeGraceMs` for exit and rejects on a signal error or missing exit. Every run uses a fresh process; process pooling is not implemented.
 
 ## Capabilities and context
 
@@ -28,8 +28,8 @@ ACP advertises no start-time capabilities because this process cannot enforce th
 | `cwd` | parent session cwd | Working-directory override for the child process and its ACP session; must be non-empty, a relative value resolves against the harness launch directory at load, and the result must name a directory the harness can enter. |
 | `permission` | `reject` | Auto-answer permission requests by rejecting or choosing the first allow-shaped option. |
 | `env` | `{}` | Explicit child environment layered over a credential-scrubbed parent environment. |
-| `disposeEofGraceMs` | `6000` | Grace after stdin EOF before SIGTERM. |
-| `disposeGraceMs` | `3000` | Grace after SIGTERM before SIGKILL. |
+| `disposeEofGraceMs` | `6000` | Grace after stdin EOF before platform termination. |
+| `disposeGraceMs` | `3000` | Exit-confirmation grace after termination; POSIX also waits this long after SIGTERM before SIGKILL. |
 
 ```yaml
 - id: subagent-acp

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

@@ -52,7 +52,7 @@ export interface Config {
    * before the parent escalates to a signal.
    */
   disposeEofGraceMs?: number
-  /** Grace period (ms) between `SIGTERM` and the `SIGKILL` escalation on dispose. */
+  /** Termination confirmation window (ms), including forced exit on every platform. */
   disposeGraceMs?: number
 }
 

packages/subagent/subagent-acp/src/run.ts

@@ -60,9 +60,9 @@ export interface AcpRunSpec {
    */
   disposeEofGraceMs: number
   /**
-   * Grace period (ms) between `SIGTERM` and the `SIGKILL` escalation in
-   * {@link SubagentRun.dispose}. The plugin fills this from its
-   * `disposeGraceMs` config.
+   * Termination confirmation window (ms) in {@link SubagentRun.dispose}; POSIX applies it after
+   * `SIGTERM` and `SIGKILL`, while Windows applies it after direct forced termination. The plugin
+   * fills this from its `disposeGraceMs` config.
    */
   disposeGraceMs: number
   /**
@@ -79,7 +79,7 @@ export interface AcpRunSpec {
 /** EOF grace for child flush and nested-process teardown; wider than the signal grace below. */
 export const DEFAULT_DISPOSE_EOF_GRACE_MS = 6_000
 
-/** Default grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config; mirrors the bash executor). */
+/** Default POSIX grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config). */
 export const DEFAULT_DISPOSE_GRACE_MS = 3_000
 
 /**
@@ -304,9 +304,9 @@ export async function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpe
       if (disposal !== undefined) return disposal
       request.signal.removeEventListener('abort', onAbort)
       requestCancel()
-      // The shared EOF → TERM → KILL ladder awaits exit. ACP normally quiesces
-      // from stdin EOF, including the final flush, so this backend uses a wider
-      // EOF grace before signals escalate.
+      // The shared platform-aware ladder awaits exit. ACP normally quiesces from
+      // stdin EOF, including the final flush, so this backend uses a wider EOF
+      // grace before process termination escalates.
       disposal = disposeProcess()
       return disposal
     },

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

@@ -215,7 +215,8 @@ describe('cwd resolution', () => {
     await ctx.fiber.dispose()
   })
 
-  it('rejects a config cwd directory without search permission at load', async () => {
+  // Windows ACLs do not expose the POSIX directory search-bit state this fixture creates.
+  it.skipIf(process.platform === 'win32')('rejects a config cwd directory without search permission at load', async () => {
     // statSync().isDirectory() is true for a mode-600 directory, but a
     // subprocess cwd needs SEARCH permission — spawn would fail EACCES.
     const tmp = mkdtempSync(join(tmpdir(), 'acp-noexec-'))
@@ -472,13 +473,9 @@ describe('dsh-subagent-acp', () => {
     }
   })
 
-  it('escalates to SIGTERM for a child that ignores EOF but is not SIGTERM-trapping', async () => {
-    // A child that keeps its loop alive past stdin EOF (so the graceful window
-    // times out) but exits cooperatively on SIGTERM must die on the SIGTERM tier
-    // — dispose returns there, never reaching the SIGKILL tier. The child touches
-    // a SIGTERM marker from its signal handler: SIGKILL is uncatchable, so if
-    // dispose had skipped the middle rung (EOF→SIGKILL) the handler would never
-    // run and the marker would be absent — making this a GENUINE middle-tier guard.
+  it('terminates a child that ignores EOF using the host platform semantics', async () => {
+    // POSIX uses the catchable SIGTERM tier and records the marker. Windows has
+    // no distinct graceful signal, so disposal skips directly to forced exit.
     const tmp = mkdtempSync(join(tmpdir(), 'acp-ignore-eof-'))
     const ready = join(tmp, 'ready')
     const sigterm = join(tmp, 'sigterm')
@@ -492,7 +489,7 @@ describe('dsh-subagent-acp', () => {
           MOCK_HANG: '1', MOCK_IGNORE_EOF: '1', MOCK_TEXT: 'x',
           MOCK_READY_FILE: ready, MOCK_SIGTERM_FILE: sigterm,
         },
-        // Tiny EOF grace so the ignored-EOF window elapses fast, then SIGTERM.
+        // Tiny EOF grace so the ignored-EOF window elapses quickly.
         disposeEofGraceMs: 150,
         disposeGraceMs: 2000,
       }
@@ -503,9 +500,7 @@ describe('dsh-subagent-acp', () => {
         run.dispose(),
         new Promise((_r, reject) => { setTimeout(() => { reject(new Error('dispose did not return')) }, 5000) }),
       ])).resolves.toBeUndefined()
-      // The child caught SIGTERM and exited — proof the middle rung fired (not a
-      // jump straight to the uncatchable SIGKILL).
-      expect(existsSync(sigterm)).toBe(true)
+      expect(existsSync(sigterm)).toBe(process.platform !== 'win32')
     } finally {
       rmSync(tmp, { recursive: true, force: true })
     }

packages/subagent/subagent-subprocess/README.md

@@ -16,13 +16,13 @@ Spawn-failure capture: a promise that resolves (never rejects) with the child's
 
 ### `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)):
+The platform-aware dispose ladder resolves only once the child has ACTUALLY exited — quiescence reached, not merely requested (see [defensive patterns](../../../docs/defensive-patterns.md)):
 
 1. stdin EOF (when stdin is piped), then wait `graces.disposeEofGraceMs` — a cooperative child quiesces on its own, its flushes and nested-subprocess teardown intact;
-2. `SIGTERM`, then wait `graces.disposeGraceMs`;
-3. `SIGKILL`, then await the now-certain exit — a child that ignores EOF and traps `SIGTERM` cannot wedge dispose forever.
+2. on POSIX, `SIGTERM`, then wait `graces.disposeGraceMs`;
+3. force termination — `SIGKILL` on POSIX and Node's `TerminateProcess` mapping on Windows — then wait at most `graces.disposeGraceMs` for exit; a signal error or missing exit rejects disposal.
 
-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 two graces (`DisposeLadderGraces`) come from the consuming plugin's `disposeEofGraceMs`/`disposeGraceMs` Config fields. POSIX uses `disposeGraceMs` after both the graceful and forced signals; Windows skips the redundant graceful signal but uses it to bound forced-exit confirmation. The EOF window is deliberately separate and usually wider, since cooperative teardown may 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.
 
@@ -35,7 +35,7 @@ A per-run isolated config directory for an external CLI child (the target of `CL
 
 ## Testing
 
-`tests/subagent-subprocess.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (the rm-failure path injects its rejection at the fs boundary — a real recursive-rm failure is not portably provokable, and root ignores permission bits); the exit waits and the dispose ladder run against a scriptable fake child, driving each escalation tier deterministically. The [ACP backend suite](../subagent-acp/README.md) exercises the same ladder against real subprocesses (EOF-cooperative, EOF-ignoring, and SIGTERM-trapping children) end to end.
+`tests/subagent-subprocess.spec.ts`: the env scrub and config-dir helpers run against the real process env and real filesystem (the rm-failure path injects its rejection at the fs boundary — a real recursive-rm failure is not portably provokable, and root ignores permission bits); the exit waits and platform termination paths run against a scriptable fake child. The [ACP backend suite](../subagent-acp/README.md) exercises them against real subprocesses end to end.
 
 ## Model Experience
 

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

@@ -51,16 +51,6 @@ export function spawnFailure(child: ChildProcess): Promise<Error> {
   })
 }
 
-/**
- * Resolve once the child process exits (any code/signal); immediate if it is
- * already gone.
- * @param child - the child process to await.
- */
-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() }))
-}
-
 /**
  * Race the child's exit against a timer. Neither outcome leaves anything
  * behind on the child: the exit listener is removed on timeout and the timer
@@ -97,36 +87,85 @@ export interface DisposeLadderGraces {
   /**
    * Tier-1 window (ms): after stdin EOF, how long the child gets to quiesce
    * ON ITS OWN — flush durable state, tear down its own nested subprocesses —
-   * before the parent escalates to `SIGTERM`. A separate (usually WIDER)
+   * before the parent escalates to platform termination. A separate (usually WIDER)
    * grace than {@link DisposeLadderGraces.disposeGraceMs}: a cooperative
    * child's EOF-driven teardown may itself be waiting on a signal-trapping
    * grandchild plus a final flush, needing more than one signal-grace of
    * headroom.
    */
   disposeEofGraceMs: number
-  /** Tier-2 window (ms): between `SIGTERM` and the `SIGKILL` escalation. */
+  /**
+   * Termination confirmation window (ms): POSIX applies it after `SIGTERM` and again after
+   * `SIGKILL`; Windows applies it after the direct forced termination.
+   */
   disposeGraceMs: number
 }
 
+/** Force-terminate a child and reject if no exit edge arrives within the configured grace. */
+function forceTerminateWithin(child: ChildProcess, ms: number): Promise<void> {
+  if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve()
+  return new Promise<void>((resolve, reject) => {
+    let accepted = false
+    let settled = false
+    const cleanup = (): void => {
+      clearTimeout(timer)
+      child.off('exit', onExit)
+      child.off('error', onError)
+    }
+    const settle = (complete: () => void): void => {
+      if (settled) return
+      settled = true
+      cleanup()
+      complete()
+    }
+    const onExit = (): void => { settle(resolve) }
+    const onError = (error: Error): void => { settle(() => { reject(error) }) }
+    child.once('exit', onExit)
+    child.once('error', onError)
+    const timer = setTimeout(() => {
+      const disposition = accepted ? 'accepted' : 'refused'
+      settle(() => {
+        reject(new Error(`child process did not exit within ${ms}ms after SIGKILL was ${disposition}`))
+      })
+    }, ms).unref()
+    try {
+      accepted = child.kill('SIGKILL')
+      if (child.exitCode !== null || child.signalCode !== null) settle(resolve)
+    } catch (error: unknown) {
+      settle(() => { reject(new Error('SIGKILL failed', { cause: error })) })
+    }
+  })
+}
+
 /**
  * Tear a child process down to quiescence, resolving only after exit: close stdin and allow
- * cooperative flush, then send `SIGTERM`, then `SIGKILL` and await the forced exit.
+ * cooperative flush, then use the host's graceful and forced termination semantics. POSIX
+ * sends `SIGTERM` before `SIGKILL`; Windows skips directly to forced termination because Node
+ * maps both signals to `TerminateProcess`.
  *
  * @param child - the child process to tear down.
  * @param graces - the two grace periods, from the consuming plugin's Config.
+ * @param platform - the host platform, injectable for unit coverage.
+ * @throws When forced termination errors or the child does not report exit within
+ * `disposeGraceMs`.
  */
-export async function disposeChildProcess(child: ChildProcess, graces: DisposeLadderGraces): Promise<void> {
+export async function disposeChildProcess(
+  child: ChildProcess,
+  graces: DisposeLadderGraces,
+  platform: NodeJS.Platform = process.platform,
+): Promise<void> {
   // Already gone: nothing to reap.
   if (child.exitCode !== null || child.signalCode !== null) return
   // 1. Close stdin and allow cooperative teardown and durable-state flush.
   child.stdin?.end()
   if (await exitsWithin(child, graces.disposeEofGraceMs)) return
-  // 2. SIGTERM, escalating if the child still does not exit within the grace.
-  child.kill('SIGTERM')
-  if (await exitsWithin(child, graces.disposeGraceMs)) return
-  // 3. Force-kill and await the (now-certain) exit.
-  child.kill('SIGKILL')
-  await waitForExit(child)
+  // 2. POSIX gets a catchable graceful signal; Windows signals all force-terminate.
+  if (platform !== 'win32') {
+    child.kill('SIGTERM')
+    if (await exitsWithin(child, graces.disposeGraceMs)) return
+  }
+  // 3. Force-kill and await a bounded exit edge.
+  await forceTerminateWithin(child, graces.disposeGraceMs)
 }
 
 /**

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

@@ -191,7 +191,7 @@ describe('disposeChildProcess', () => {
 
   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 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }, 'linux')
     expect(fake.stdinEnded).toBe(true)
     expect(fake.kills).toEqual(['SIGTERM'])
     expect(fake.signalCode).toBe('SIGTERM')
@@ -200,7 +200,7 @@ describe('disposeChildProcess', () => {
 
   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 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }, 'linux')
     expect(fake.kills).toEqual(['SIGTERM'])
     expect(fake.signalCode).toBe('SIGTERM')
     expect(fake.listenerCount('exit')).toBe(0)
@@ -208,7 +208,7 @@ describe('disposeChildProcess', () => {
 
   it('tier 3: a SIGTERM-trapping child is SIGKILLed, and dispose resolves only after the exit', async () => {
     const fake = new FakeChild({ delayMs: 5 }) // only SIGKILL fells it
-    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 }, 'linux')
     expect(fake.kills).toEqual(['SIGTERM', 'SIGKILL'])
     // Quiescence, not a request: at resolution the child has ACTUALLY exited
     // (the exit event landed, despite the scripted post-SIGKILL delay).
@@ -217,16 +217,103 @@ describe('disposeChildProcess', () => {
 
   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 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 20 }, 'linux')
     expect(fake.kills).toEqual(['SIGTERM', 'SIGKILL'])
     expect(fake.signalCode).toBe('SIGKILL')
   })
 
+  it.each(['exitCode', 'signalCode'] as const)('accepts a late OS %s marker before the final forced wait', async (marker) => {
+    const fake = new FakeChild()
+    vi.spyOn(fake, 'kill').mockImplementation((signal) => {
+      fake.kills.push(signal)
+      queueMicrotask(() => {
+        if (marker === 'exitCode') fake.exitCode = 0
+        else fake.signalCode = 'SIGTERM'
+      })
+      return true
+    })
+
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 1, disposeGraceMs: 10 }, 'linux')
+    expect(fake.kills).toEqual(['SIGTERM'])
+  })
+
   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 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }, 'linux')
     expect(fake.kills).toEqual(['SIGTERM'])
   })
+
+  it('skips the redundant SIGTERM tier on Windows and awaits forced exit', async () => {
+    const fake = new FakeChild({ diesOn: 'SIGTERM', delayMs: 5 })
+    await disposeChildProcess(asChild(fake), { disposeEofGraceMs: 20, disposeGraceMs: 1000 }, 'win32')
+    expect(fake.kills).toEqual(['SIGKILL'])
+    expect(fake.signalCode).toBe('SIGKILL')
+  })
+
+  it('propagates a forced-termination error without waiting for the grace', async () => {
+    const fake = new FakeChild()
+    const failure = Object.assign(new Error('kill EPERM'), { code: 'EPERM' })
+    vi.spyOn(fake, 'kill').mockImplementation((signal) => {
+      fake.kills.push(signal)
+      fake.emit('error', failure)
+      return false
+    })
+
+    await expect(disposeChildProcess(
+      asChild(fake),
+      { disposeEofGraceMs: 1, disposeGraceMs: 1000 },
+      'win32',
+    )).rejects.toBe(failure)
+    expect(fake.kills).toEqual(['SIGKILL'])
+    expect(fake.listenerCount('error')).toBe(0)
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
+
+  it('wraps a synchronous forced-termination exception and removes its listeners', async () => {
+    const fake = new FakeChild()
+    const failure = new Error('invalid signal state')
+    vi.spyOn(fake, 'kill').mockImplementation(() => { throw failure })
+
+    await expect(disposeChildProcess(
+      asChild(fake),
+      { disposeEofGraceMs: 1, disposeGraceMs: 1000 },
+      'win32',
+    )).rejects.toMatchObject({ message: 'SIGKILL failed', cause: failure })
+    expect(fake.listenerCount('error')).toBe(0)
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
+
+  it('bounds a refused forced termination that produces no error or exit', async () => {
+    const fake = new FakeChild()
+    vi.spyOn(fake, 'kill').mockImplementation((signal) => {
+      fake.kills.push(signal)
+      return false
+    })
+
+    await expect(disposeChildProcess(
+      asChild(fake),
+      { disposeEofGraceMs: 1, disposeGraceMs: 10 },
+      'win32',
+    )).rejects.toThrow('child process did not exit within 10ms after SIGKILL was refused')
+    expect(fake.listenerCount('error')).toBe(0)
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
+
+  it('bounds an accepted forced termination that never reports exit', async () => {
+    const fake = new FakeChild()
+    vi.spyOn(fake, 'kill').mockImplementation((signal) => {
+      fake.kills.push(signal)
+      return true
+    })
+
+    await expect(disposeChildProcess(
+      asChild(fake),
+      { disposeEofGraceMs: 1, disposeGraceMs: 10 },
+      'win32',
+    )).rejects.toThrow('child process did not exit within 10ms after SIGKILL was accepted')
+    expect(fake.listenerCount('error')).toBe(0)
+    expect(fake.listenerCount('exit')).toBe(0)
+  })
 })
 
 describe('createIsolatedConfigDir', () => {
@@ -236,8 +323,9 @@ describe('createIsolatedConfigDir', () => {
       expect(dir.path.startsWith(join(tmpdir(), 'dsh-subagent-subprocess-test-'))).toBe(true)
       const st = await stat(dir.path)
       expect(st.isDirectory()).toBe(true)
-      // Private (0700) per the defensive-patterns temp-dir rule.
-      expect(st.mode & 0o777).toBe(0o700)
+      // Windows reports synthetic POSIX mode bits; privacy comes from the
+      // inherited directory ACL rather than chmod-compatible mode bits.
+      if (process.platform !== 'win32') expect(st.mode & 0o777).toBe(0o700)
     } finally {
       await dir.remove()
     }