From 75b94b63eb4abee530826f8e66770f76a098008a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90=E7=9B=AE?= Date: Thu, 19 Mar 2026 10:37:52 +0800 Subject: [PATCH 1/5] fix: clean up ACP connection state when child process exits (#1780) When the ACP child process exits unexpectedly (e.g., user cancels execution), the connection state (sdkConnection, sessionId, child) was not being cleaned up. This caused subsequent message sends to fail with cryptic errors instead of a clear "Not connected" message. Changes: - Clear sdkConnection/sessionId/child in the exit handler - Check child.exitCode in isConnected to detect spontaneous exits - Add isConnected check to ensureConnection as defense-in-depth - Add unit tests for isConnected, ensureConnection, and disconnect cleanup Made-with: Cursor --- .../src/services/acpConnection.test.ts | 108 +++++++++++++++++- .../src/services/acpConnection.ts | 9 +- 2 files changed, 109 insertions(+), 8 deletions(-) diff --git a/packages/vscode-ide-companion/src/services/acpConnection.test.ts b/packages/vscode-ide-companion/src/services/acpConnection.test.ts index 32977171a..03a41b3ed 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.test.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.test.ts @@ -14,11 +14,25 @@ vi.mock('vscode', () => ({})); import { AcpConnection } from './acpConnection.js'; import { ACP_ERROR_CODES } from '../constants/acpSchema.js'; +type AcpConnectionInternal = { + child: { killed: boolean; exitCode: number | null } | null; + sdkConnection: unknown; + sessionId: string | null; + mapReadTextFileError: (error: unknown, filePath: string) => unknown; + ensureConnection: () => unknown; +}; + +function createConnection(overrides?: Partial) { + const conn = new AcpConnection() as unknown as AcpConnectionInternal; + if (overrides) { + Object.assign(conn, overrides); + } + return conn; +} + describe('AcpConnection readTextFile error mapping', () => { it('maps ENOENT to RESOURCE_NOT_FOUND RequestError', () => { - const conn = new AcpConnection() as unknown as { - mapReadTextFileError: (error: unknown, filePath: string) => unknown; - }; + const conn = createConnection(); const enoent = Object.assign(new Error('missing file'), { code: 'ENOENT' }); expect(() => @@ -31,9 +45,7 @@ describe('AcpConnection readTextFile error mapping', () => { }); it('keeps non-ENOENT RequestError unchanged', () => { - const conn = new AcpConnection() as unknown as { - mapReadTextFileError: (error: unknown, filePath: string) => unknown; - }; + const conn = createConnection(); const requestError = new RequestError( ACP_ERROR_CODES.INTERNAL_ERROR, 'Internal error', @@ -44,3 +56,87 @@ describe('AcpConnection readTextFile error mapping', () => { ); }); }); + +describe('AcpConnection.isConnected', () => { + it('returns true when child is alive', () => { + const conn = createConnection({ + child: { killed: false, exitCode: null }, + }); + expect((conn as unknown as AcpConnection).isConnected).toBe(true); + }); + + it('returns false when child is null', () => { + const conn = createConnection({ child: null }); + expect((conn as unknown as AcpConnection).isConnected).toBe(false); + }); + + it('returns false when child was killed', () => { + const conn = createConnection({ + child: { killed: true, exitCode: null }, + }); + expect((conn as unknown as AcpConnection).isConnected).toBe(false); + }); + + it('returns false when child exited on its own (exitCode set)', () => { + const conn = createConnection({ + child: { killed: false, exitCode: 143 }, + }); + expect((conn as unknown as AcpConnection).isConnected).toBe(false); + }); +}); + +describe('AcpConnection.ensureConnection', () => { + it('throws when sdkConnection is null', () => { + const conn = createConnection({ + sdkConnection: null, + child: { killed: false, exitCode: null }, + }); + expect(() => conn.ensureConnection()).toThrow('Not connected to ACP agent'); + }); + + it('throws when process has exited (exitCode set)', () => { + const conn = createConnection({ + sdkConnection: {}, + child: { killed: false, exitCode: 1 }, + }); + expect(() => conn.ensureConnection()).toThrow('Not connected to ACP agent'); + }); + + it('throws when child is null (process exited and cleaned up)', () => { + const conn = createConnection({ + sdkConnection: {}, + child: null, + }); + expect(() => conn.ensureConnection()).toThrow('Not connected to ACP agent'); + }); + + it('returns sdkConnection when process is alive', () => { + const fakeSdk = { send: vi.fn() }; + const conn = createConnection({ + sdkConnection: fakeSdk, + child: { killed: false, exitCode: null }, + }); + expect(conn.ensureConnection()).toBe(fakeSdk); + }); +}); + +describe('AcpConnection child exit cleanup', () => { + it('disconnect clears child, sdkConnection, and sessionId', () => { + const conn = createConnection({ + child: { + killed: false, + exitCode: null, + kill: vi.fn(), + } as unknown as AcpConnectionInternal['child'], + sdkConnection: {}, + sessionId: 'test-session', + }); + + const acpConn = conn as unknown as AcpConnection; + acpConn.disconnect(); + + expect(acpConn.isConnected).toBe(false); + expect(acpConn.hasActiveSession).toBe(false); + expect(acpConn.currentSessionId).toBeNull(); + }); +}); diff --git a/packages/vscode-ide-companion/src/services/acpConnection.ts b/packages/vscode-ide-companion/src/services/acpConnection.ts index 5d263b618..cea93eb8b 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.ts @@ -145,6 +145,9 @@ export class AcpConnection { console.error( `[ACP qwen] Process exited with code: ${code}, signal: ${signal}`, ); + this.sdkConnection = null; + this.sessionId = null; + this.child = null; }); await new Promise((resolve) => setTimeout(resolve, 1000)); @@ -334,7 +337,7 @@ export class AcpConnection { } private ensureConnection(): ClientSideConnection { - if (!this.sdkConnection) { + if (!this.sdkConnection || !this.isConnected) { throw new Error('Not connected to ACP agent'); } return this.sdkConnection; @@ -541,7 +544,9 @@ export class AcpConnection { } get isConnected(): boolean { - return this.child !== null && !this.child.killed; + return ( + this.child !== null && !this.child.killed && this.child.exitCode === null + ); } get hasActiveSession(): boolean { From f231830b9f4e342eae13572799869f4842306cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90=E7=9B=AE?= Date: Thu, 19 Mar 2026 10:41:35 +0800 Subject: [PATCH 2/5] fix: improve ACP exit diagnostics and add onDisconnected callback - Record lastExitCode/lastExitSignal for better error messages on startup failure - Reset exit info on reconnect to avoid stale data - Add onDisconnected callback so upper layers can react to process exit - Add clarifying comment on ensureConnection defense-in-depth checks - Expand tests: onDisconnected contract, disconnect kill verification, exit info init Made-with: Cursor --- .../src/services/acpConnection.test.ts | 56 +++++++++++++++++-- .../src/services/acpConnection.ts | 15 ++++- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/packages/vscode-ide-companion/src/services/acpConnection.test.ts b/packages/vscode-ide-companion/src/services/acpConnection.test.ts index 03a41b3ed..e3661a584 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.test.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.test.ts @@ -15,9 +15,11 @@ import { AcpConnection } from './acpConnection.js'; import { ACP_ERROR_CODES } from '../constants/acpSchema.js'; type AcpConnectionInternal = { - child: { killed: boolean; exitCode: number | null } | null; + child: { killed: boolean; exitCode: number | null; kill?: () => void } | null; sdkConnection: unknown; sessionId: string | null; + lastExitCode: number | null; + lastExitSignal: string | null; mapReadTextFileError: (error: unknown, filePath: string) => unknown; ensureConnection: () => unknown; }; @@ -30,6 +32,15 @@ function createConnection(overrides?: Partial) { return conn; } +function createMockChild(overrides?: Record) { + return { + killed: false, + exitCode: null, + kill: vi.fn(), + ...overrides, + } as unknown as AcpConnectionInternal['child']; +} + describe('AcpConnection readTextFile error mapping', () => { it('maps ENOENT to RESOURCE_NOT_FOUND RequestError', () => { const conn = createConnection(); @@ -123,11 +134,7 @@ describe('AcpConnection.ensureConnection', () => { describe('AcpConnection child exit cleanup', () => { it('disconnect clears child, sdkConnection, and sessionId', () => { const conn = createConnection({ - child: { - killed: false, - exitCode: null, - kill: vi.fn(), - } as unknown as AcpConnectionInternal['child'], + child: createMockChild(), sdkConnection: {}, sessionId: 'test-session', }); @@ -139,4 +146,41 @@ describe('AcpConnection child exit cleanup', () => { expect(acpConn.hasActiveSession).toBe(false); expect(acpConn.currentSessionId).toBeNull(); }); + + it('disconnect calls kill on the child process', () => { + const mockKill = vi.fn(); + const conn = createConnection({ + child: createMockChild({ kill: mockKill }), + sdkConnection: {}, + sessionId: 'test-session', + }); + + (conn as unknown as AcpConnection).disconnect(); + expect(mockKill).toHaveBeenCalledOnce(); + }); +}); + +describe('AcpConnection onDisconnected callback', () => { + it('has a default no-op onDisconnected handler', () => { + const acpConn = new AcpConnection(); + expect(acpConn.onDisconnected).toBeTypeOf('function'); + expect(() => acpConn.onDisconnected(143, 'SIGTERM')).not.toThrow(); + }); + + it('allows setting a custom onDisconnected handler', () => { + const acpConn = new AcpConnection(); + const spy = vi.fn(); + acpConn.onDisconnected = spy; + + acpConn.onDisconnected(1, null); + expect(spy).toHaveBeenCalledWith(1, null); + }); +}); + +describe('AcpConnection lastExitCode/lastExitSignal', () => { + it('initializes exit info as null', () => { + const conn = createConnection(); + expect(conn.lastExitCode).toBeNull(); + expect(conn.lastExitSignal).toBeNull(); + }); }); diff --git a/packages/vscode-ide-companion/src/services/acpConnection.ts b/packages/vscode-ide-companion/src/services/acpConnection.ts index cea93eb8b..59579b2b1 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.ts @@ -52,6 +52,8 @@ export class AcpConnection { private sessionId: string | null = null; private workingDir: string = process.cwd(); private fileHandler = new AcpFileHandler(); + private lastExitCode: number | null = null; + private lastExitSignal: string | null = null; onSessionUpdate: (data: SessionNotification) => void = () => {}; onPermissionRequest: (data: RequestPermissionRequest) => Promise<{ @@ -63,6 +65,8 @@ export class AcpConnection { onAuthenticateUpdate: (data: AuthenticateUpdateNotification) => void = () => {}; onEndTurn: (reason?: string) => void = () => {}; + onDisconnected: (code: number | null, signal: string | null) => void = + () => {}; onAskUserQuestion: (data: AskUserQuestionRequest) => Promise<{ optionId: string; answers?: Record; @@ -78,6 +82,8 @@ export class AcpConnection { this.disconnect(); } + this.lastExitCode = null; + this.lastExitSignal = null; this.workingDir = workingDir; const env = { ...process.env }; @@ -145,9 +151,12 @@ export class AcpConnection { console.error( `[ACP qwen] Process exited with code: ${code}, signal: ${signal}`, ); + this.lastExitCode = code; + this.lastExitSignal = signal; this.sdkConnection = null; this.sessionId = null; this.child = null; + this.onDisconnected(code, signal); }); await new Promise((resolve) => setTimeout(resolve, 1000)); @@ -157,7 +166,9 @@ export class AcpConnection { } if (!this.child || this.child.killed) { - throw new Error(`Qwen ACP process failed to start`); + throw new Error( + `Qwen ACP process failed to start (exit code: ${this.lastExitCode}, signal: ${this.lastExitSignal})`, + ); } // Convert Node.js child process streams to Web Streams for SDK @@ -337,6 +348,8 @@ export class AcpConnection { } private ensureConnection(): ClientSideConnection { + // sdkConnection is cleared asynchronously by the exit handler; + // isConnected (via exitCode) catches the race window before the exit event fires. if (!this.sdkConnection || !this.isConnected) { throw new Error('Not connected to ACP agent'); } From 692e063cd75f31eddf4fc50e695b897c2844b0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90=E7=9B=AE?= Date: Thu, 19 Mar 2026 11:07:29 +0800 Subject: [PATCH 3/5] fix: address PR review feedback - Add exitCode fallback in startup failure message to handle edge case where exit event hasn't fired yet - Add guard check in exit handler to avoid redundant cleanup if disconnect() already ran - Add JSDoc for onDisconnected callback - Add comment explaining exit code 143 (SIGTERM) in test Made-with: Cursor --- .../src/services/acpConnection.test.ts | 1 + .../src/services/acpConnection.ts | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/vscode-ide-companion/src/services/acpConnection.test.ts b/packages/vscode-ide-companion/src/services/acpConnection.test.ts index e3661a584..881d40d01 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.test.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.test.ts @@ -89,6 +89,7 @@ describe('AcpConnection.isConnected', () => { }); it('returns false when child exited on its own (exitCode set)', () => { + // 143 = 128 + 15 (SIGTERM) const conn = createConnection({ child: { killed: false, exitCode: 143 }, }); diff --git a/packages/vscode-ide-companion/src/services/acpConnection.ts b/packages/vscode-ide-companion/src/services/acpConnection.ts index 59579b2b1..736c8a73a 100644 --- a/packages/vscode-ide-companion/src/services/acpConnection.ts +++ b/packages/vscode-ide-companion/src/services/acpConnection.ts @@ -65,6 +65,7 @@ export class AcpConnection { onAuthenticateUpdate: (data: AuthenticateUpdateNotification) => void = () => {}; onEndTurn: (reason?: string) => void = () => {}; + /** Invoked when the child process exits (expected or unexpected). */ onDisconnected: (code: number | null, signal: string | null) => void = () => {}; onAskUserQuestion: (data: AskUserQuestionRequest) => Promise<{ @@ -153,10 +154,12 @@ export class AcpConnection { ); this.lastExitCode = code; this.lastExitSignal = signal; - this.sdkConnection = null; - this.sessionId = null; - this.child = null; - this.onDisconnected(code, signal); + if (this.child) { + this.sdkConnection = null; + this.sessionId = null; + this.child = null; + this.onDisconnected(code, signal); + } }); await new Promise((resolve) => setTimeout(resolve, 1000)); @@ -166,8 +169,10 @@ export class AcpConnection { } if (!this.child || this.child.killed) { + const code = this.lastExitCode ?? this.child?.exitCode ?? null; + const signal = this.lastExitSignal; throw new Error( - `Qwen ACP process failed to start (exit code: ${this.lastExitCode}, signal: ${this.lastExitSignal})`, + `Qwen ACP process failed to start (exit code: ${code}, signal: ${signal})`, ); } From 2e6ab6562bad5090a47818da4a8ebdd1bd48518f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90=E7=9B=AE?= Date: Thu, 19 Mar 2026 11:46:57 +0800 Subject: [PATCH 4/5] fix: handle SIGTERM/SIGINT in ACP mode for graceful shutdown (#1884) The ACP process was ignoring SIGTERM because signal handlers registered elsewhere in the CLI startup path (e.g., stdin raw mode restoration) override Node.js's default exit behavior. This caused the process to remain alive after the IDE sends SIGTERM, leading to zombie connections and exit code 143 errors in JetBrains IntelliJ IDEA. Add explicit SIGTERM/SIGINT handlers in runAcpAgent() that destroy stdin, which closes the ndjson stream and resolves connection.closed, allowing the process to exit gracefully. Made-with: Cursor --- packages/cli/src/acp-integration/acpAgent.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 246d80019..777171cca 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -87,7 +87,25 @@ export async function runAcpAgent( stream, ); + // Handle SIGTERM/SIGINT for graceful shutdown. + // Without this, signal handlers registered elsewhere in the CLI + // (e.g., stdin raw mode restoration) override the default exit behavior, + // causing the ACP process to ignore termination signals. + const shutdownHandler = () => { + debugLogger.debug('[ACP] Shutdown signal received, closing streams'); + try { + process.stdin.destroy(); + } catch { + // stdin may already be closed + } + }; + process.on('SIGTERM', shutdownHandler); + process.on('SIGINT', shutdownHandler); + await connection.closed; + + process.off('SIGTERM', shutdownHandler); + process.off('SIGINT', shutdownHandler); } function toStdioServer(server: McpServer): McpServerStdio | undefined { From a6e8aec7f4a5dfe6d75c72b2068db978e6254b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B2=90=E7=9B=AE?= Date: Thu, 19 Mar 2026 11:53:13 +0800 Subject: [PATCH 5/5] fix: destroy both stdin and stdout on shutdown, guard against duplicate signals - Close stdout in addition to stdin to prevent hanging writes during shutdown - Add shuttingDown guard to ensure cleanup runs only once Made-with: Cursor --- packages/cli/src/acp-integration/acpAgent.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 777171cca..0cbace59e 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -91,13 +91,21 @@ export async function runAcpAgent( // Without this, signal handlers registered elsewhere in the CLI // (e.g., stdin raw mode restoration) override the default exit behavior, // causing the ACP process to ignore termination signals. + let shuttingDown = false; const shutdownHandler = () => { + if (shuttingDown) return; + shuttingDown = true; debugLogger.debug('[ACP] Shutdown signal received, closing streams'); try { process.stdin.destroy(); } catch { // stdin may already be closed } + try { + process.stdout.destroy(); + } catch { + // stdout may already be closed + } }; process.on('SIGTERM', shutdownHandler); process.on('SIGINT', shutdownHandler);