From 19f2d292f9147096176b6800a53e02b341fdd5a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Sun, 12 Apr 2026 10:39:56 +0800 Subject: [PATCH] fix(core): fall back to CLI confirmation when IDE diff open fails (#3031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: centralize IDE diff interaction in CoreToolScheduler - Move openDiff/confirmation handling from edit.ts and write-file.ts into CoreToolScheduler.openIdeDiffIfEnabled(), called after permission hooks - Use structuredClone in buildInvocation to prevent params mutation leaking to LLM history (fixes #2709 token waste) - Use confirmationDetails as single data source for IDE diff content, only rely on ModifyContext.createUpdatedParams() for parameter transform - Skip inline modify when IDE content unchanged, preserving original tool params for multi-edit-on-same-file scenarios (mitigates #2702) - Remove ideConfirmation field from ToolEditConfirmationDetails - Remove dead resolveIdeDiffForOutcome from ACP Session.ts - Fix memory tool scope fallback in createUpdatedParams Closes #2709 Closes #2673 * fix(core): fall back to CLI confirmation when IDE diff open fails * fix(core): narrow IDE diff error handling scope --------- Co-authored-by: 胡玮文 Co-authored-by: tanzhenxin --- .../core/src/core/coreToolScheduler.test.ts | 110 ++++++++++++++++++ packages/core/src/core/coreToolScheduler.ts | 27 +++-- 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index ee53116c4..4a1ce835c 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -3485,6 +3485,116 @@ describe('CoreToolScheduler IDE interaction', () => { expect(completedCalls[0].status).toBe('cancelled'); }); + it('should fall back to CLI confirmation when opening the IDE diff fails', async () => { + const { mockConfig } = createIdeMockConfig({ + ideMode: true, + }); + + mockIdeClient.openDiff.mockRejectedValue(new Error('IDE disconnected')); + + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + + const scheduler = new CoreToolScheduler({ + config: mockConfig, + onAllToolCallsComplete, + onToolCallsUpdate, + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const request = { + callId: 'ide-open-fail-1', + name: 'mockModifiableTool', + args: { param: 'value' }, + isClientInitiated: false, + prompt_id: 'prompt-ide-open-fail-1', + }; + + const abortController = new AbortController(); + await scheduler.schedule([request], abortController.signal); + + const awaitingCall = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + + expect(awaitingCall.status).toBe('awaiting_approval'); + expect(mockIdeClient.openDiff).toHaveBeenCalled(); + expect(onAllToolCallsComplete).not.toHaveBeenCalled(); + }); + + it('should not swallow confirmation handling errors after IDE diff opens', async () => { + const { mockConfig } = createIdeMockConfig({ + ideMode: true, + }); + + mockIdeClient.openDiff.mockResolvedValue({ + status: 'rejected', + }); + + const scheduler = new CoreToolScheduler({ + config: mockConfig, + onAllToolCallsComplete: vi.fn(), + onToolCallsUpdate: vi.fn(), + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + + const request = { + callId: 'ide-confirmation-error-1', + name: 'mockModifiableTool', + args: { param: 'value' }, + isClientInitiated: false, + prompt_id: 'prompt-ide-confirmation-error-1', + }; + const confirmationDetails = { + type: 'edit', + title: 'Confirm Mock Tool', + fileName: 'test.txt', + filePath: 'test.txt', + fileDiff: 'diff', + originalContent: 'originalContent', + newContent: 'newContent', + onConfirm: vi.fn(), + } satisfies ToolCallConfirmationDetails; + const confirmationError = new Error('confirmation handling failed'); + + ( + scheduler as unknown as { + toolCalls: WaitingToolCall[]; + } + ).toolCalls = [ + { + status: 'awaiting_approval', + request, + tool: {} as never, + invocation: {} as never, + confirmationDetails, + }, + ]; + + vi.spyOn(scheduler, 'handleConfirmationResponse').mockRejectedValue( + confirmationError, + ); + + await expect( + ( + scheduler as unknown as { + openIdeDiffIfEnabled: ( + confirmationDetails: ToolCallConfirmationDetails, + callId: string, + signal: AbortSignal, + ) => Promise; + } + ).openIdeDiffIfEnabled( + confirmationDetails, + request.callId, + new AbortController().signal, + ), + ).rejects.toThrow('confirmation handling failed'); + }); + it('should not call openDiff when IDE mode is disabled', async () => { const { mockConfig } = createIdeMockConfig({ ideMode: false, diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 4c82c3095..c8f0c3cb2 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -1219,13 +1219,24 @@ export class CoreToolScheduler { if (confirmationDetails.type !== 'edit' || !this.config.getIdeMode()) { return; } - const ideClient = await IdeClient.getInstance(); - if (!ideClient.isDiffingEnabled()) return; - const resolution = await ideClient.openDiff( - confirmationDetails.filePath, - confirmationDetails.newContent, - ); + let resolution: Awaited>; + try { + const ideClient = await IdeClient.getInstance(); + if (!ideClient.isDiffingEnabled()) return; + + resolution = await ideClient.openDiff( + confirmationDetails.filePath, + confirmationDetails.newContent, + ); + } catch (error) { + if (!signal.aborted) { + debugLogger.warn( + `IDE diff open failed for ${callId}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + return; + } // Guard: skip if the tool was already handled (e.g. by CLI // confirmation). Without this check, resolveDiffFromCli @@ -1244,7 +1255,7 @@ export class CoreToolScheduler { const userEdited = resolution.content != null && resolution.content !== confirmationDetails.newContent; - this.handleConfirmationResponse( + await this.handleConfirmationResponse( callId, confirmationDetails.onConfirm, ToolConfirmationOutcome.ProceedOnce, @@ -1252,7 +1263,7 @@ export class CoreToolScheduler { userEdited ? { newContent: resolution.content } : undefined, ); } else { - this.handleConfirmationResponse( + await this.handleConfirmationResponse( callId, confirmationDetails.onConfirm, ToolConfirmationOutcome.Cancel,