diff --git a/packages/support/acp-snapshot/src/launcher.ts b/packages/support/acp-snapshot/src/launcher.ts index de5082eca4..e39c6552f0 100644 --- a/packages/support/acp-snapshot/src/launcher.ts +++ b/packages/support/acp-snapshot/src/launcher.ts @@ -231,6 +231,15 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe return } + const propagateFailureAfterDrain = async (): Promise => { + await drained + closeUpdateStream() + throw failure + } + // Windows implements the supported signal names as forced termination. The exit markers + // may therefore arrive after the error wins the race above but before fallback begins. + if (!isRunning(child)) return propagateFailureAfterDrain() + // An `error` after spawn is not an exit edge: in particular, a failed // signal can leave the subprocess live. Force termination, await the // already-observed exit edge, and only then propagate the child error so @@ -240,6 +249,10 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe child.once('error', observeFallbackError) if (!child.kill('SIGKILL')) { child.off('error', observeFallbackError) + // A successful earlier signal may win between the live check and this fallback call. + // In that case `kill()` correctly reports no process to signal; the original child error + // remains the shutdown result once inherited stdio and callbacks have drained. + if (!isRunning(child)) return propagateFailureAfterDrain() closeUpdateStream() throw new AggregateError( [failure, new Error('Fallback SIGKILL was not accepted by the child process')], @@ -258,9 +271,7 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe 'ACP test agent failed and fallback termination was refused', ) } - await drained - closeUpdateStream() - throw failure + return propagateFailureAfterDrain() }, } } diff --git a/packages/support/acp-snapshot/tests/harness.spec.ts b/packages/support/acp-snapshot/tests/harness.spec.ts index 5589858fe9..2c4478e158 100644 --- a/packages/support/acp-snapshot/tests/harness.spec.ts +++ b/packages/support/acp-snapshot/tests/harness.spec.ts @@ -176,6 +176,30 @@ describe('runScenario', () => { } }) + it('preserves the child error when fallback refusal races with an exit marker', async () => { + const { dir } = await scenario({}) + const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir }) + await launched.spawned + + const childFailure = Object.assign(new Error('signal failed while the child exited'), { code: 'EPERM' }) + const originalKill = launched.child.kill.bind(launched.child) + const kill = vi.spyOn(launched.child, 'kill').mockImplementation((signal) => { + if (signal === 'SIGTERM') return true + originalKill('SIGKILL') + Object.defineProperty(launched.child, 'signalCode', { configurable: true, enumerable: true, writable: true, value: 'SIGKILL' }) + return false + }) + try { + launched.child.emit('error', childFailure) + await expect(launched.close('SIGTERM')).rejects.toBe(childFailure) + expect(kill).toHaveBeenNthCalledWith(1, 'SIGTERM') + expect(kill).toHaveBeenNthCalledWith(2, 'SIGKILL') + } finally { + kill.mockRestore() + if (launched.child.exitCode === null && launched.child.signalCode === null) originalKill('SIGKILL') + } + }) + it('rejects promptly when fallback termination emits an error', async () => { const { dir } = await scenario({}) const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir })