diff --git a/packages/support/acp-snapshot/src/harness.ts b/packages/support/acp-snapshot/src/harness.ts index 3db1355b0f..22b22deacd 100644 --- a/packages/support/acp-snapshot/src/harness.ts +++ b/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[] = [] const cleanup = async (action: () => Promise): Promise => { 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 } diff --git a/packages/support/acp-snapshot/tests/harness.spec.ts b/packages/support/acp-snapshot/tests/harness.spec.ts index 902f48d2cf..de764cfd8a 100644 --- a/packages/support/acp-snapshot/tests/harness.spec.ts +++ b/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() + return { + ...actual, + async rm(...args: Parameters): Promise { + 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(