From 3e2ba3f5574b43dc5af5d03e146769be241f96a3 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Tue, 14 Jul 2026 10:13:05 +0800
Subject: [PATCH] fix: stop ACP teardown when fallback kill fails

---
 packages/support/acp-snapshot/src/launcher.ts | 27 ++++++++-
 .../acp-snapshot/tests/harness.spec.ts        | 59 +++++++++++++++++++
 2 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/packages/support/acp-snapshot/src/launcher.ts b/packages/support/acp-snapshot/src/launcher.ts
index f1271986c0..8d25a6f56c 100644
--- a/packages/support/acp-snapshot/src/launcher.ts
+++ b/packages/support/acp-snapshot/src/launcher.ts
@@ -65,7 +65,7 @@ export interface LaunchedAcpTestAgent {
   stderr(): string
   /** Resolve when a future session update matches the predicate. */
   waitForUpdate(match: (update: SessionNotification['update']) => boolean): Promise<SessionNotification['update']>
-  /** Gracefully close stdin, or send a signal, then wait for process exit, inherited stdio closure, ACP parsing, and client callbacks. */
+  /** Close the process and drain its streams and callbacks; rejects promptly if fallback termination is refused. */
   close(signal?: NodeJS.Signals): Promise<void>
 }
 
@@ -231,8 +231,29 @@ export function launchAcpTestAgent(options: AcpTestLaunchOptions): LaunchedAcpTe
       // signal can leave the subprocess live. Force termination, await the
       // already-observed exit edge, and only then propagate the child error so
       // callers may safely remove cwd/session resources after close rejects.
-      child.kill('SIGKILL')
-      await exited
+      const fallbackError = Promise.withResolvers<Error>()
+      const observeFallbackError = (error: Error): void => { fallbackError.resolve(error) }
+      child.once('error', observeFallbackError)
+      if (!child.kill('SIGKILL')) {
+        child.off('error', observeFallbackError)
+        closeUpdateStream()
+        throw new AggregateError(
+          [failure, new Error('Fallback SIGKILL was not accepted by the child process')],
+          'ACP test agent failed and fallback termination was refused',
+        )
+      }
+      const fallbackFailure = await Promise.race([
+        exited.then((): undefined => undefined),
+        fallbackError.promise,
+      ])
+      child.off('error', observeFallbackError)
+      if (fallbackFailure !== undefined) {
+        closeUpdateStream()
+        throw new AggregateError(
+          [failure, fallbackFailure],
+          'ACP test agent failed and fallback termination was refused',
+        )
+      }
       await drained
       closeUpdateStream()
       throw failure
diff --git a/packages/support/acp-snapshot/tests/harness.spec.ts b/packages/support/acp-snapshot/tests/harness.spec.ts
index 23b4192a4d..ff7c304121 100644
--- a/packages/support/acp-snapshot/tests/harness.spec.ts
+++ b/packages/support/acp-snapshot/tests/harness.spec.ts
@@ -131,6 +131,65 @@ describe('runScenario', () => {
     expect(launched.stderr()).toContain('late inherited stderr')
   })
 
+  it('rejects promptly when fallback termination is refused', async () => {
+    const { dir } = await scenario({})
+    const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir })
+    await launched.spawned
+
+    const childFailure = Object.assign(new Error('signal refused'), { code: 'EPERM' })
+    const originalKill = launched.child.kill.bind(launched.child)
+    const kill = vi.spyOn(launched.child, 'kill').mockReturnValue(false)
+    const closed = new Promise<void>(resolve => launched.child.once('close', () => { resolve() }))
+    try {
+      launched.child.emit('error', childFailure)
+      const rejection = await launched.close('SIGTERM').catch((error: unknown): unknown => error)
+      expect(rejection).toBeInstanceOf(AggregateError)
+      expect(rejection).toMatchObject({
+        message: 'ACP test agent failed and fallback termination was refused',
+        errors: [
+          childFailure,
+          expect.objectContaining({ message: 'Fallback SIGKILL was not accepted by the child process' }),
+        ],
+      })
+      expect(kill).toHaveBeenNthCalledWith(1, 'SIGTERM')
+      expect(kill).toHaveBeenNthCalledWith(2, 'SIGKILL')
+    } finally {
+      kill.mockRestore()
+      originalKill('SIGKILL')
+      await closed
+    }
+  })
+
+  it('rejects promptly when fallback termination emits an error', async () => {
+    const { dir } = await scenario({})
+    const launched = launchAcpTestAgent({ agent: AGENT, cwd: dir })
+    await launched.spawned
+
+    const childFailure = Object.assign(new Error('signal refused'), { code: 'EPERM' })
+    const fallbackFailure = Object.assign(new Error('fallback signal refused'), { code: 'EPERM' })
+    const originalKill = launched.child.kill.bind(launched.child)
+    const kill = vi.spyOn(launched.child, 'kill').mockImplementation((signal) => {
+      if (signal === 'SIGKILL') queueMicrotask(() => launched.child.emit('error', fallbackFailure))
+      return signal === 'SIGKILL'
+    })
+    const closed = new Promise<void>(resolve => launched.child.once('close', () => { resolve() }))
+    try {
+      launched.child.emit('error', childFailure)
+      const rejection = await launched.close('SIGTERM').catch((error: unknown): unknown => error)
+      expect(rejection).toBeInstanceOf(AggregateError)
+      expect(rejection).toMatchObject({
+        message: 'ACP test agent failed and fallback termination was refused',
+        errors: [childFailure, fallbackFailure],
+      })
+      expect(kill).toHaveBeenNthCalledWith(1, 'SIGTERM')
+      expect(kill).toHaveBeenNthCalledWith(2, 'SIGKILL')
+    } finally {
+      kill.mockRestore()
+      originalKill('SIGKILL')
+      await closed
+    }
+  })
+
   it('waits for in-flight client callbacks after the ACP stream closes', { timeout: 20_000 }, async () => {
     const { dir, fixtureFile } = await scenario({ permissionProbe: true })
     let releasePermission: (() => void) | undefined