From f110c5e08377f4074a0ec5927226f72c821a742b Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Sun, 19 Jul 2026 12:41:46 +0800
Subject: [PATCH] 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.
---
 packages/support/acp-snapshot/src/launcher.ts | 17 ++++++++++---
 .../acp-snapshot/tests/harness.spec.ts        | 24 +++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

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<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
       // 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 })