Merge branch 'codex/simp-shared-acp-test-launcher' into codex/simp-trim-hook-snapshot-noise

packages/support/acp-snapshot/src/harness.ts

@@ -244,9 +244,9 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise
   )
 
   // Failure-safe teardown: wait for a still-running child, then attempt BOTH
-  // directory removals even when an earlier cleanup rejects. The main outcome
-  // wins over teardown noise so a step/harvest failure is never replaced; on a
-  // successful run, the first cleanup failure remains visible to the caller.
+  // directory removals even when an earlier cleanup rejects. Report every
+  // teardown failure alongside a scenario failure so neither orthogonal
+  // outcome hides the other.
   const cleanupResults: PromiseSettledResult<unknown>[] = []
   const cleanup = async (action: () => Promise<unknown>): Promise<void> => {
     cleanupResults.push(...await Promise.allSettled([action()]))
@@ -256,10 +256,18 @@ export async function runScenario(input: InputScript, opts: RunOptions): Promise
   await cleanup(() => rm(cwd, { recursive: true, force: true }))
   await cleanup(() => rm(sessionsRoot, { recursive: true, force: true }))
 
+  const cleanupFailures = cleanupResults
+    .filter((result): result is PromiseRejectedResult => result.status === 'rejected')
+    .map(result => result.reason as unknown)
+  if (cleanupFailures.length > 0) {
+    throw new AggregateError(
+      outcome.status === 'rejected' ? [outcome.error, ...cleanupFailures] : cleanupFailures,
+      outcome.status === 'rejected'
+        ? 'snapshot scenario and cleanup failed'
+        : 'snapshot cleanup failed',
+    )
+  }
   if (outcome.status === 'rejected') throw outcome.error
-  const cleanupFailure = cleanupResults.find((result): result is PromiseRejectedResult => result.status === 'rejected')
-  /* v8 ignore next 1 -- defensive OS cleanup failure after an otherwise successful real subprocess run */
-  if (cleanupFailure !== undefined) throw cleanupFailure.reason
   return outcome.value
 }
 

packages/support/acp-snapshot/tests/harness.spec.ts

@@ -3,11 +3,29 @@ import { once } from 'node:events'
 import { tmpdir } from 'node:os'
 import { delimiter, join } from 'node:path'
 import { fileURLToPath } from 'node:url'
-import { afterAll, describe, expect, it } from 'vitest'
+import { afterAll, describe, expect, it, vi } from 'vitest'
 import { PROTOCOL_VERSION } from '@agentclientprotocol/sdk'
 import { runScenario, type AgentUnderTest, type InputStep } from '../src/harness.ts'
 import { launchAcpTestAgent } from '../src/launcher.ts'
 
+const fsControl = vi.hoisted(() => ({ cleanupFailure: undefined as Error | undefined }))
+
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs/promises')>()
+  return {
+    ...actual,
+    async rm(...args: Parameters<typeof actual.rm>): Promise<void> {
+      if (String(args[0]).includes('acp-snap-cwd-') && fsControl.cleanupFailure !== undefined) {
+        const failure = fsControl.cleanupFailure
+        fsControl.cleanupFailure = undefined
+        await actual.rm(...args)
+        throw failure
+      }
+      await actual.rm(...args)
+    },
+  }
+})
+
 /**
  * Unit tests for the subprocess harness, driven through the REAL spawn path
  * (tsx loader, temp cwd, env plumbing) against the scripted fake ACP bin in
@@ -238,6 +256,24 @@ describe('runScenario', () => {
     )).rejects.toThrow(/expected the prompt to fail/)
   })
 
+  it('reports scenario and cleanup failures together', { timeout: 20_000 }, async () => {
+    const { fixtureFile } = await scenario({ prompt: 'respond' })
+    const cleanupFailure = new Error('cleanup failed')
+    fsControl.cleanupFailure = cleanupFailure
+
+    const failure = await runScenario(
+      { steps: [...boot, { op: 'promptExpectError', text: 'fine' }] },
+      { agent: AGENT, mode: 'replay', fixtureFile },
+    ).catch((error: unknown): unknown => error)
+
+    expect(failure).toBeInstanceOf(AggregateError)
+    const failures = (failure as AggregateError).errors as unknown[]
+    expect(failures).toHaveLength(2)
+    expect(failures[0]).toBeInstanceOf(Error)
+    expect((failures[0] as Error).message).toMatch(/expected the prompt to fail/)
+    expect(failures[1]).toBe(cleanupFailure)
+  })
+
   it('newSessionExpectError swallows the rejection, with and without extra dirs', { timeout: 20_000 }, async () => {
     const { fixtureFile } = await scenario({ rejectExtraDirs: true })
     const result = await runScenario(