fix(acp-snapshot): accept Windows termination exit races
Treat a fallback kill refusal as successful termination when the child already carries an OS exit marker. Windows maps Node's supported signal names to forced termination, so the requested signal can end the process between the launcher error race and its fallback SIGKILL. Drain inherited stdio, the ACP parser, and in-flight callbacks before propagating the original child error in either exit-race path. Preserve AggregateError reporting only for a refused fallback while the process is still live, and add a deterministic cross-platform regression for that ordering.
This commit is contained in:
@@ -231,6 +231,15 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const propagateFailureAfterDrain = async (): Promise<never> => {
|
||||||
|
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
|
// An `error` after spawn is not an exit edge: in particular, a failed
|
||||||
// signal can leave the subprocess live. Force termination, await the
|
// signal can leave the subprocess live. Force termination, await the
|
||||||
// already-observed exit edge, and only then propagate the child error so
|
// 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)
|
child.once('error', observeFallbackError)
|
||||||
if (!child.kill('SIGKILL')) {
|
if (!child.kill('SIGKILL')) {
|
||||||
child.off('error', observeFallbackError)
|
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()
|
closeUpdateStream()
|
||||||
throw new AggregateError(
|
throw new AggregateError(
|
||||||
[failure, new Error('Fallback SIGKILL was not accepted by the child process')],
|
[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',
|
'ACP test agent failed and fallback termination was refused',
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
await drained
|
return propagateFailureAfterDrain()
|
||||||
closeUpdateStream()
|
|
||||||
throw failure
|
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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 () => {
|
it('rejects promptly when fallback termination emits an error', async () => {
|
||||||
const { dir } = await scenario({})
|
const { dir } = await scenario({})
|
||||||
const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir })
|
const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir })
|
||||||
|
|||||||
Reference in New Issue
Block a user