fix(vscode): shut the ACP CLI down gracefully instead of killing it

packages/cli/src/acp-integration/acpAgent.test.ts

@@ -1665,6 +1665,97 @@ describe('runAcpAgent shutdown cleanup', () => {
     await agentPromise;
   });
 
+  it('SIGTERM during ide_close drain runs shutdownMcpPool once', async () => {
+    // The VS Code extension escalates to SIGTERM once its shutdown grace
+    // expires; when that lands mid-ide_close, the signal path must join the
+    // in-flight MCP pool drain instead of running shutdownMcpPool again.
+    const { agent, agentPromise } = await startPreloadTestAgent();
+    expect(agent).toBeDefined();
+
+    let resolveDrain!: () => void;
+    const shutdownMcpPool = vi.fn(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveDrain = resolve;
+        }),
+    );
+    const disposeSessions = vi.fn().mockResolvedValue(undefined);
+    Object.assign(agent!, { shutdownMcpPool, disposeSessions });
+
+    await vi.waitFor(() => {
+      expect(sigTermListeners.length).toBeGreaterThan(0);
+    });
+
+    // connection.closed resolving drives the ide_close path, which parks on
+    // the drain held open here.
+    mockConnectionState.resolve();
+    await vi.waitFor(() => {
+      expect(shutdownMcpPool).toHaveBeenCalledTimes(1);
+    });
+
+    sigTermListeners[0]('SIGTERM');
+    // The signal path passes disposeSessions and then joins the parked drain;
+    // its runExitCleanup can only run once the shared drain settles.
+    await vi.waitFor(() => {
+      expect(disposeSessions).toHaveBeenCalledTimes(1);
+    });
+    expect(shutdownMcpPool).toHaveBeenCalledTimes(1);
+    expect(mockRunExitCleanup).not.toHaveBeenCalled();
+
+    resolveDrain();
+    await agentPromise;
+    await vi.waitFor(() => {
+      expect(processExitSpy).toHaveBeenCalledWith(0);
+    });
+
+    expect(shutdownMcpPool).toHaveBeenCalledTimes(1);
+    expect(disposeSessions).toHaveBeenCalledTimes(1);
+    expect(mockRunExitCleanup).toHaveBeenCalledTimes(1);
+  });
+
+  it('SIGTERM during ide_close disposes sessions once', async () => {
+    // Same overlap, one step later: the signal path must join the in-flight
+    // disposeSessions instead of snapshotting the same sessions and running
+    // closeStoredSession (beginClose/abort) for each a second time.
+    const { agent, agentPromise } = await startPreloadTestAgent();
+    expect(agent).toBeDefined();
+
+    const shutdownMcpPool = vi.fn().mockResolvedValue(undefined);
+    let resolveDispose!: () => void;
+    const disposeSessions = vi.fn(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveDispose = resolve;
+        }),
+    );
+    Object.assign(agent!, { shutdownMcpPool, disposeSessions });
+
+    await vi.waitFor(() => {
+      expect(sigTermListeners.length).toBeGreaterThan(0);
+    });
+
+    // ide_close drains (the mock resolves immediately) and then parks on the
+    // dispose held open here.
+    mockConnectionState.resolve();
+    await vi.waitFor(() => {
+      expect(disposeSessions).toHaveBeenCalledTimes(1);
+    });
+
+    sigTermListeners[0]('SIGTERM');
+    await flushImmediate();
+    expect(disposeSessions).toHaveBeenCalledTimes(1);
+    expect(mockRunExitCleanup).not.toHaveBeenCalled();
+
+    resolveDispose();
+    await agentPromise;
+    await vi.waitFor(() => {
+      expect(processExitSpy).toHaveBeenCalledWith(0);
+    });
+
+    expect(disposeSessions).toHaveBeenCalledTimes(1);
+    expect(shutdownMcpPool).toHaveBeenCalledTimes(1);
+  });
+
   it('still exits even if runExitCleanup throws', async () => {
     mockRunExitCleanup.mockRejectedValueOnce(new Error('cleanup failed'));
 

packages/cli/src/acp-integration/acpAgent.ts

@@ -2910,18 +2910,42 @@ export async function runAcpAgent(
 
   // Both the SIGTERM handler and the IDE-initiated close path need
   // to drain the MCP pool before runExitCleanup. Single helper
-  // closure keeps the timeout + log labels consistent.
+  // closure keeps the timeout + log labels consistent. The memoized
+  // promise makes it idempotent: a SIGTERM landing mid-ide_close (the
+  // VS Code extension escalates to SIGTERM once its shutdown grace
+  // expires) joins the in-flight drain instead of running
+  // shutdownMcpPool a second time. First call wins; later calls —
+  // including a stricter one — join it.
+  let drainPoolPromise: Promise<void> | undefined;
   const drainPoolBeforeExit = async (
     label: string,
     strict = false,
   ): Promise<void> => {
     if (!agentInstance) return;
-    try {
-      await agentInstance.shutdownMcpPool(8_000);
-    } catch (err) {
-      debugLogger.error(`[ACP] MCP pool drain (${label}) error:`, err);
-      if (strict) throw err;
-    }
+    if (drainPoolPromise) return drainPoolPromise;
+    drainPoolPromise = (async () => {
+      try {
+        await agentInstance?.shutdownMcpPool(8_000);
+      } catch (err) {
+        debugLogger.error(`[ACP] MCP pool drain (${label}) error:`, err);
+        if (strict) throw err;
+      }
+    })();
+    return drainPoolPromise;
+  };
+
+  // disposeSessions() is idempotent per call but not under concurrency: two
+  // overlapping calls snapshot the same session entries and each runs
+  // closeStoredSession for them (double beginClose, double abort). SIGTERM
+  // landing mid-ide_close is exactly that overlap, so both shutdown paths
+  // share one in-flight dispose.
+  let disposeSessionsPromise: Promise<void> | undefined;
+  const disposeSessionsOnce = (): Promise<void> => {
+    if (!agentInstance) return Promise.resolve();
+    if (!disposeSessionsPromise) {
+      disposeSessionsPromise = agentInstance.disposeSessions();
+    }
+    return disposeSessionsPromise;
   };
 
   // Handle SIGTERM/SIGINT for graceful shutdown.
@@ -3079,7 +3103,7 @@ export async function runAcpAgent(
     try {
       // Fire SessionEnd hook for all active sessions (aligned with core path)
       await fireSessionEndOnce(SessionEndReason.Other);
-      await agentInstance?.disposeSessions();
+      await disposeSessionsOnce();
 
       try {
         process.stdin.destroy();
@@ -3131,7 +3155,7 @@ export async function runAcpAgent(
       // Mirror the SIGTERM handler's pool drain on the IDE-initiated
       // normal close path to avoid leaking shared MCP entries.
       await drainPoolBeforeExit('ide_close');
-      await agentInstance?.disposeSessions();
+      await disposeSessionsOnce();
     }
   } finally {
     process.off('SIGTERM', shutdownHandler);