fix(subagent): preserve Codex process exit facts

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

@@ -164,13 +164,7 @@ export async function disposeCodexChild(
   wire: CodexAppServerWire,
   child: SubprocessHandle,
 ): Promise<void> {
-  const failures: Error[] = []
-  let outcome: SubprocessOutcome | undefined
-  try {
-    wire.close()
-  } catch (error: unknown) {
-    failures.push(thrown(error))
-  }
+  wire.close()
 
   if (child.pid > 0) {
     try {
@@ -182,31 +176,17 @@ export async function disposeCodexChild(
     try {
       await child.waitForExit()
     } catch (error: unknown) {
-      failures.push(thrown(error))
-    }
-    try {
-      outcome = await child.done
-    } catch (error: unknown) {
-      failures.push(thrown(error))
+      const outcome = await child.done
+      throw new CodexRunFailure({
+        stage: 'teardown',
+        category: 'unknown',
+        outcome,
+      }, thrown(error))
     }
+    await child.done
   } else {
     await child.done.catch(() => {})
   }
-
-  const firstFailure = failures[0]
-  if (firstFailure === undefined) return
-  const facts = {
-    stage: 'teardown',
-    category: 'unknown',
-    outcome,
-  } as const
-  if (failures.length === 1) {
-    throw new CodexRunFailure(facts, firstFailure)
-  }
-  throw new AggregateError(
-    failures.map(failure => new CodexRunFailure(facts, failure)),
-    `subagent-codex: ${failureDiagnostic(facts)}`,
-  )
 }
 
 /**
@@ -271,16 +251,23 @@ export async function startCodexRun(
     }
   }
 
-  const processFailure: Promise<never> = child.done.then(
-    outcome => Promise.reject(new CodexRunFailure({
-      stage: 'process',
-      category: 'process-exit',
-      outcome,
-    })),
-    (error: unknown) => Promise.reject(new CodexRunFailure({
-      stage: 'process',
-      category: 'unknown',
-    }, thrown(error))),
+  let processFailureFacts: CodexFailureFacts | undefined
+  const processFailure: Promise<never> = child.done.then<never>(
+    (outcome) => {
+      processFailureFacts = {
+        stage: 'process',
+        category: 'process-exit',
+        outcome,
+      }
+      throw new CodexRunFailure(processFailureFacts)
+    },
+    (error: unknown) => {
+      processFailureFacts = {
+        stage: 'process',
+        category: 'unknown',
+      }
+      throw new CodexRunFailure(processFailureFacts, thrown(error))
+    },
   )
   // A normal post-result dispose also closes the process. Keep that expected
   // late rejection observed after the result race has already settled.
@@ -349,19 +336,32 @@ export async function startCodexRun(
           processFailure,
         ])
         if (terminal.stopReason === 'completed') return terminal
-        const facts = wire.collectFailure() ?? {
-          stage: 'turn',
-          category: 'unknown',
-        }
+        const facts = wire.collectFailure()
         return { ...terminal, diagnostic: recordFailureDiagnostic(facts) }
       } catch (error: unknown) {
         // Give stderr data already queued in Node one turn to reach the wire
-        // before settlement snapshots the diagnostic; later OS data is best-effort.
+        // before settlement snapshots the diagnostic.
         await new Promise<void>((resolve) => { setImmediate(resolve) })
-        const wireFacts = wire.collectFailure()
+        const endedBeforeTerminal = wire.endedBeforeTerminal()
+        if (
+          endedBeforeTerminal
+          && processFailureFacts === undefined
+          && !runAbort.signal.aborted
+        ) {
+          try {
+            const exited = await child.waitForExit(
+              AbortSignal.timeout(spec.disposeGraceMs),
+            )
+            if (exited) await child.done
+          } catch {
+            // The wire failure remains authoritative when exit observation fails.
+          }
+        }
         const facts = error instanceof CodexRunFailure
           ? error.facts
-          : wireFacts ?? { stage: 'turn', category: 'unknown' }
+          : endedBeforeTerminal && processFailureFacts !== undefined
+            ? processFailureFacts
+            : wire.collectFailure()
         recordFailureDiagnostic(facts)
         throw error instanceof CodexRunFailure
           ? error

packages/subagent/subagent-codex/src/wire.ts

@@ -237,6 +237,8 @@ export class CodexAppServerWire {
     readonly reason: string
   } | undefined
   private stderrTail = ''
+  private inputEnded = false
+  private terminalObserved = false
   private closed = false
 
   constructor(
@@ -270,6 +272,14 @@ export class CodexAppServerWire {
     this.transport.start()
   }
 
+  /**
+   * Whether protocol output ended before a terminal turn notification.
+   * @returns `true` only for an early protocol close without a terminal turn.
+   */
+  endedBeforeTerminal(): boolean {
+    return this.inputEnded && !this.terminalObserved
+  }
+
   /**
    * Perform the required app-server initialize/initialized handshake.
    * @param signal - unpublished-start cancellation.
@@ -415,10 +425,11 @@ export class CodexAppServerWire {
 
   /**
    * The structured failure fact observed for this published turn.
-   * @returns a fixed stage/category pair and optional HTTP status.
+   * Call only after a non-completed return or rejection from {@link runTurn}.
+   * @returns the fixed stage/category pair and optional HTTP status.
    */
-  collectFailure(): CodexWireFailureFacts | undefined {
-    return this.failure
+  collectFailure(): CodexWireFailureFacts {
+    return this.failure as CodexWireFailureFacts
   }
 
   /**
@@ -469,6 +480,7 @@ export class CodexAppServerWire {
   }
 
   private readonly onInputEnd = (): void => {
+    this.inputEnded = true
     this.fail(new Error('subagent-codex: app-server protocol stream closed'))
   }
 
@@ -720,6 +732,7 @@ export class CodexAppServerWire {
       return
     }
     if (id !== this.turnId) return
+    this.terminalObserved = true
     if (!['completed', 'interrupted', 'failed'].includes(String(turn.status))) {
       throw new Error(`subagent-codex: app-server returned invalid terminal turn status ${String(turn.status)}`)
     }

packages/subagent/subagent-codex/tests/real-product.spec.ts

@@ -15,7 +15,10 @@ import { Context } from '@deepseek-ai/cordis'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import type { Agent } from '@deepseek-ai/dsh-agent'
 import SubagentRuntime from '@deepseek-ai/dsh-subagent'
-import type { SubprocessHandle } from '@deepseek-ai/dsh-subprocess'
+import type {
+  SubprocessHandle,
+  SubprocessOutcome,
+} from '@deepseek-ai/dsh-subprocess'
 import LocalSubprocessRuntime from '@deepseek-ai/dsh-subprocess-local'
 import * as codex from '../src/index.ts'
 import type { CodexPermissionMode } from '../src/run.ts'
@@ -132,6 +135,26 @@ async function expectQuiescent(handles: readonly SubprocessHandle[]): Promise<vo
   }
 }
 
+function expectedProcessExitDiagnostic(outcome: SubprocessOutcome): string {
+  const fields = [
+    'product: Codex',
+    'stage: process',
+    'category: process-exit',
+  ]
+  if (outcome.exitCode !== null) fields.push(`exit code: ${outcome.exitCode}`)
+  if (outcome.signal !== null) fields.push(`signal: ${outcome.signal}`)
+  return `Product subagent failure (${fields.join('; ')})`
+}
+
+interface JsonSchemaNode {
+  readonly enum?: string[]
+  readonly format?: string
+  readonly minimum?: number
+  readonly properties?: Record<string, JsonSchemaNode>
+  readonly required?: string[]
+  readonly type?: string | string[]
+}
+
 function responseInputTexts(body: Record<string, unknown>): string[] {
   if (!Array.isArray(body.input)) return []
   return body.input.flatMap((item): string[] => {
@@ -175,10 +198,7 @@ describe('real @openai/codex 0.147.0 product', () => {
     )) as {
       definitions: {
         CodexErrorInfo: {
-          oneOf: Array<{
-            enum?: string[]
-            properties?: Record<string, unknown>
-          }>
+          oneOf: JsonSchemaNode[]
         }
       }
     }
@@ -203,6 +223,16 @@ describe('real @openai/codex 0.147.0 product', () => {
       'responseTooManyFailedAttempts',
       'activeTurnNotSteerable',
     ])
+    for (const variant of schema.definitions.CodexErrorInfo.oneOf.slice(1, 5)) {
+      const category = Object.keys(variant.properties ?? {})[0]!
+      const detail = variant.properties?.[category]
+      expect(detail?.required).toBeUndefined()
+      expect(detail?.properties?.httpStatusCode).toEqual({
+        format: 'uint16',
+        minimum: 0,
+        type: ['integer', 'null'],
+      })
+    }
 
     const run = await harness.ctx.subagents.start('codex', {
       prompt: [{ type: 'text', text: task }],
@@ -323,10 +353,10 @@ describe('real @openai/codex 0.147.0 product', () => {
       await fixture.requestStarted
       expect(harness.handles).toHaveLength(1)
       harness.handles[0]!.terminate()
-      await harness.handles[0]!.done
+      const outcome = await harness.handles[0]!.done
       await expect(run.result).resolves.toEqual({
         output: [],
-        diagnostic: 'Product subagent failure (product: Codex; stage: turn; category: unknown)',
+        diagnostic: expectedProcessExitDiagnostic(outcome),
         stopReason: 'error',
       })
       await run.dispose()

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

@@ -1479,50 +1479,6 @@ describe('run lifecycle and quiescence', () => {
     }
   })
 
-  it('uses safe unknown fallbacks when the wire supplies no failure fact', async () => {
-    {
-      const collectFailure = vi.spyOn(
-        CodexAppServerWire.prototype,
-        'collectFailure',
-      ).mockReturnValue(undefined)
-      const { child, run, turnStart } = await publishRun()
-      child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
-      child.peer.send(turnCompleted('failed', 'turn-1', 'thread-1', {
-        codexErrorInfo: 'contextWindowExceeded',
-      }))
-      await expect(run.result).resolves.toEqual({
-        output: [],
-        diagnostic: expectedFailureDiagnostic('turn', 'unknown'),
-        stopReason: 'max-tokens',
-      })
-      collectFailure.mockRestore()
-      await run.dispose()
-    }
-    {
-      const runTurn = vi.spyOn(CodexAppServerWire.prototype, 'runTurn')
-        .mockRejectedValueOnce(new Error('SECRET_TOKEN wire failure'))
-      const child = fakeChild()
-      const starting = startCodexRun(request(), runSpec(child))
-      const initialize = await child.peer.nextMethod('initialize')
-      child.peer.respond(initialize, { userAgent: 'codex-cli 0.147.0' })
-      await child.peer.nextMethod('initialized')
-      const threadStart = await child.peer.nextMethod('thread/start')
-      child.peer.respond(threadStart, {
-        thread: { id: 'thread-1', ephemeral: true },
-      })
-      const run = await starting
-      const result = await run.result
-      expect(result).toEqual({
-        output: [],
-        diagnostic: expectedFailureDiagnostic('turn', 'unknown'),
-        stopReason: 'error',
-      })
-      expect(result.diagnostic).not.toContain('SECRET_TOKEN')
-      runTurn.mockRestore()
-      await run.dispose()
-    }
-  })
-
   it('flattens child exit and protocol failures after publication', async () => {
     const errors: string[] = []
     const outcomes: SubprocessOutcome[] = [
@@ -1549,9 +1505,45 @@ describe('run lifecycle and quiescence', () => {
       )
       await run.dispose().catch(() => {})
     }
+    {
+      const outcome = { exitCode: 17, signal: 'SIGABRT' } as const
+      const child = fakeChild({ exitOnTerminate: false })
+      const { run, turnStart } = await publishRun(child, undefined, {
+        disposeGraceMs: 100,
+      })
+      child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
+      child.fromChild.emit('end')
+      setTimeout(() => { child.settle(outcome) }, 5)
+      await expect(run.result).resolves.toEqual({
+        output: [],
+        diagnostic: expectedFailureDiagnostic('process', 'process-exit', {
+          outcome,
+        }),
+        stopReason: 'error',
+      })
+      await run.dispose().catch(() => {})
+    }
+    {
+      const child = fakeChild({ exitOnTerminate: false })
+      const { run, turnStart } = await publishRun(child)
+      child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
+      child.peer.send(turnCompleted('failed', 'turn-1', 'thread-1', {
+        codexErrorInfo: 'other',
+      }))
+      await nextTask()
+      child.fromChild.emit('end')
+      child.settle({ exitCode: 17, signal: 'SIGABRT' })
+      await expect(run.result).resolves.toEqual({
+        output: [],
+        diagnostic: expectedFailureDiagnostic('turn', 'other'),
+        stopReason: 'error',
+      })
+      await run.dispose().catch(() => {})
+    }
     {
       const child = fakeChild()
       const { run, turnStart } = await publishRun(child, undefined, {
+        disposeGraceMs: 10,
         onError: () => { throw new Error('diagnostic sink') },
       })
       child.peer.respond(turnStart, { turn: { id: 'turn-1' } })
@@ -1700,6 +1692,19 @@ describe('run lifecycle and quiescence', () => {
       .rejects.toThrow(expectedFailureDiagnostic('initialize', 'unknown'))
     await expect(spawnFailure).rejects.not.toThrow('SECRET_TOKEN')
 
+    const asyncSpawnFailureChild = fakeChild({
+      pid: -1,
+      doneError: new Error('SECRET_TOKEN async spawn failure'),
+    })
+    const asyncSpawnFailure = startCodexRun(
+      request(),
+      runSpec(asyncSpawnFailureChild),
+    )
+    await expect(asyncSpawnFailure)
+      .rejects.toThrow(expectedFailureDiagnostic('initialize', 'unknown'))
+    await expect(asyncSpawnFailure).rejects.not.toThrow('SECRET_TOKEN')
+    expect(asyncSpawnFailureChild.terminate).not.toHaveBeenCalled()
+
     const child = fakeChild()
     const starting = startCodexRun(request(), runSpec(child))
     const initialize = await child.peer.nextMethod('initialize')
@@ -1709,6 +1714,31 @@ describe('run lifecycle and quiescence', () => {
     await expect(starting).rejects.not.toThrow('invalid initialize response')
     expect(child.terminate).toHaveBeenCalledTimes(1)
 
+    const cleanupFailureChild = fakeChild({
+      waitForExitError: new Error('SECRET_TOKEN wait failure'),
+    })
+    const cleanupFailure = startCodexRun(
+      request(),
+      runSpec(cleanupFailureChild),
+    )
+    const cleanupFailureInitialize = await cleanupFailureChild.peer
+      .nextMethod('initialize')
+    cleanupFailureChild.peer.respond(cleanupFailureInitialize, null)
+    const cleanupError: unknown = await cleanupFailure.then(
+      () => undefined,
+      (error: unknown) => error,
+    )
+    expect(cleanupError).toBeInstanceOf(AggregateError)
+    expect(String(cleanupError)).toContain(
+      expectedFailureDiagnostic('initialize', 'unknown'),
+    )
+    expect(String(cleanupError)).toContain(expectedFailureDiagnostic(
+      'teardown',
+      'unknown',
+      { outcome: { exitCode: 0, signal: null } },
+    ))
+    expect(String(cleanupError)).not.toContain('SECRET_TOKEN')
+
     const cleanupRaceAbort = new AbortController()
     const cleanupRaceChild = fakeChild({ exitOnTerminate: false })
     const cleanupRace = startCodexRun(
@@ -1796,27 +1826,6 @@ describe('run lifecycle and quiescence', () => {
     expect(child.terminate).toHaveBeenCalledTimes(1)
   })
 
-  it('rolls back a subprocess done rejection during startup', async () => {
-    const child = fakeChild({ doneError: new Error('spawn observer failed') })
-    const error: unknown = await startCodexRun(request(), runSpec(child)).then(
-      () => undefined,
-      (failure: unknown) => failure,
-    )
-    expect(error).toBeInstanceOf(AggregateError)
-    if (!(error instanceof AggregateError)) {
-      throw new Error('expected startup and rollback failures')
-    }
-    expect(error.errors).toEqual([
-      expect.objectContaining({
-        message: `subagent-codex: ${expectedFailureDiagnostic('initialize', 'unknown')}`,
-      }),
-      expect.objectContaining({
-        message: `subagent-codex: ${expectedFailureDiagnostic('teardown', 'unknown')}`,
-      }),
-    ])
-    expect(child.terminate).toHaveBeenCalledTimes(1)
-  })
-
   it('keeps overlapping runs isolated', async () => {
     const initialStderrListeners = {
       error: process.stderr.listenerCount('error'),
@@ -1957,7 +1966,6 @@ describe('run lifecycle and quiescence', () => {
       stopReason: 'error',
     })
     expect(spawn).toHaveBeenCalledWith(expect.objectContaining({
-      argv: codexAppServerArgv(),
       env: { OPENAI_API_KEY: 'fake' },
       graceMs: 25,
       cwd: process.cwd(),
@@ -2020,39 +2028,24 @@ describe('disposeCodexChild', () => {
     expect(child.waitForExit).not.toHaveBeenCalled()
   })
 
-  it('reports direct-child observer failure and accepts absent stdin', async () => {
-    {
-      const child = fakeChild({
-        doneError: new Error('close observer failed'),
-      })
-      const wire = defaultWire(child)
-      await expect(disposeCodexChild(wire, child.handle))
-        .rejects.toThrow(expectedFailureDiagnostic('teardown', 'unknown'))
-    }
-    {
-      const child = fakeChild()
-      const handle = { ...child.handle, stdin: undefined }
-      const wire = defaultWire(child)
-      await expect(disposeCodexChild(wire, handle)).resolves.toBeUndefined()
-    }
+  it('accepts absent stdin', async () => {
+    const child = fakeChild()
+    const handle = { ...child.handle, stdin: undefined }
+    const wire = defaultWire(child)
+    await expect(disposeCodexChild(wire, handle)).resolves.toBeUndefined()
   })
 
-  it('aggregates wire-close and tree-wait failures with safe teardown facts', async () => {
+  it('reports tree-wait failure with safe teardown facts', async () => {
     const child = fakeChild({
       waitForExitError: new Error('SECRET_TOKEN wait failure'),
     })
     const wire = defaultWire(child)
-    vi.spyOn(wire, 'close').mockImplementation(() => {
-      throw new Error('/private/secret.txt close failure')
-    })
     const disposal = disposeCodexChild(wire, child.handle)
-    await expect(disposal).rejects.toBeInstanceOf(AggregateError)
     await expect(disposal).rejects.toThrow(expectedFailureDiagnostic(
       'teardown',
       'unknown',
       { outcome: { exitCode: 0, signal: null } },
     ))
     await expect(disposal).rejects.not.toThrow('SECRET_TOKEN')
-    await expect(disposal).rejects.not.toThrow('/private/secret.txt')
   })
 })