diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index e58d0ca600..2272c1a4dd 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -13,6 +13,7 @@ import { type SerializableConfirmationDetails, type ToolCallConfirmationDetails, type Config, + type ToolConfirmationPayload, ToolConfirmationOutcome, hasRedirection, debugLogger, @@ -65,10 +66,7 @@ export const ToolConfirmationMessage: React.FC< const isTrustedFolder = config.isTrustedFolder(); const handleConfirm = useCallback( - ( - outcome: ToolConfirmationOutcome, - payload?: { answers?: { [questionIndex: string]: string } }, - ) => { + (outcome: ToolConfirmationOutcome, payload?: ToolConfirmationPayload) => { void confirm(callId, outcome, payload).catch((error: unknown) => { debugLogger.error( `Failed to handle tool confirmation for ${callId}:`, diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 154b975638..0df9fd58eb 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -1847,6 +1847,83 @@ describe('CoreToolScheduler Sequential Execution', () => { modifyWithEditorSpy.mockRestore(); }); + it('should handle inline modify with empty new content', async () => { + // Mock the modifiable check to return true for this test + const isModifiableSpy = vi + .spyOn(modifiableToolModule, 'isModifiableDeclarativeTool') + .mockReturnValue(true); + + const mockTool = new MockModifiableTool(); + const mockToolRegistry = { + getTool: () => mockTool, + getAllToolNames: () => [], + } as unknown as ToolRegistry; + + const mockConfig = createMockConfig({ + getToolRegistry: () => mockToolRegistry, + isInteractive: () => true, + }); + mockConfig.getHookSystem = vi.fn().mockReturnValue(undefined); + + const scheduler = new CoreToolScheduler({ + config: mockConfig, + getPreferredEditor: () => 'vscode', + }); + + // Manually inject a waiting tool call + const callId = 'call-1'; + const toolCall: WaitingToolCall = { + status: 'awaiting_approval', + request: { + callId, + name: 'mockModifiableTool', + args: {}, + isClientInitiated: false, + prompt_id: 'p1', + }, + tool: mockTool, + invocation: {} as unknown as ToolInvocation< + Record, + ToolResult + >, + confirmationDetails: { + type: 'edit', + title: 'Confirm', + fileName: 'test.txt', + filePath: 'test.txt', + fileDiff: 'diff', + originalContent: 'old', + newContent: 'new', + onConfirm: async () => {}, + }, + startTime: Date.now(), + }; + + const schedulerInternals = scheduler as unknown as { + toolCalls: ToolCall[]; + toolModifier: { applyInlineModify: Mock }; + }; + schedulerInternals.toolCalls = [toolCall]; + + const applyInlineModifySpy = vi + .spyOn(schedulerInternals.toolModifier, 'applyInlineModify') + .mockResolvedValue({ + updatedParams: { content: '' }, + updatedDiff: 'diff-empty', + }); + + await scheduler.handleConfirmationResponse( + callId, + async () => {}, + ToolConfirmationOutcome.ProceedOnce, + new AbortController().signal, + { newContent: '' } as ToolConfirmationPayload, + ); + + expect(applyInlineModifySpy).toHaveBeenCalled(); + isModifiableSpy.mockRestore(); + }); + it('should pass serverName to policy engine for DiscoveredMCPTool', async () => { const mockMcpTool = { tool: async () => ({ functionDeclarations: [] }), diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 124cef32b9..30093f289e 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -789,7 +789,7 @@ export class CoreToolScheduler { } else { // If the client provided new content, apply it and wait for // re-confirmation. - if (payload?.newContent && toolCall) { + if (payload && 'newContent' in payload && toolCall) { const result = await this.toolModifier.applyInlineModify( toolCall as WaitingToolCall, payload, diff --git a/packages/core/src/scheduler/confirmation.ts b/packages/core/src/scheduler/confirmation.ts index 76ab6fac97..e5e94d5501 100644 --- a/packages/core/src/scheduler/confirmation.ts +++ b/packages/core/src/scheduler/confirmation.ts @@ -156,7 +156,7 @@ export async function resolveConfirmation( if (outcome === ToolConfirmationOutcome.ModifyWithEditor) { await handleExternalModification(deps, toolCall, signal); - } else if (response.payload?.newContent) { + } else if (response.payload && 'newContent' in response.payload) { await handleInlineModification(deps, toolCall, response.payload, signal); outcome = ToolConfirmationOutcome.ProceedOnce; } diff --git a/packages/core/src/scheduler/tool-modifier.test.ts b/packages/core/src/scheduler/tool-modifier.test.ts index 8107e4c901..0dc1a55a49 100644 --- a/packages/core/src/scheduler/tool-modifier.test.ts +++ b/packages/core/src/scheduler/tool-modifier.test.ts @@ -193,13 +193,46 @@ describe('ToolModificationHandler', () => { const result = await handler.applyInlineModify( mockWaitingToolCall, - { newContent: undefined } as unknown as ToolConfirmationPayload, + {} as ToolConfirmationPayload, // no newContent property new AbortController().signal, ); expect(result).toBeUndefined(); }); + it('should process empty string as valid new content', async () => { + vi.mocked( + modifiableToolModule.isModifiableDeclarativeTool, + ).mockReturnValue(true); + (Diff.createPatch as unknown as Mock).mockReturnValue('mock-diff-empty'); + + mockModifyContext.getCurrentContent.mockResolvedValue('old content'); + mockModifyContext.getFilePath.mockReturnValue('test.txt'); + mockModifyContext.createUpdatedParams.mockReturnValue({ + content: '', + }); + + const mockWaitingToolCall = createMockWaitingToolCall({ + tool: mockModifiableTool, + }); + + const result = await handler.applyInlineModify( + mockWaitingToolCall, + { newContent: '' }, + new AbortController().signal, + ); + + expect(mockModifyContext.createUpdatedParams).toHaveBeenCalledWith( + expect.any(String), + '', + expect.any(Object), + ); + expect(result).toEqual({ + updatedParams: { content: '' }, + updatedDiff: 'mock-diff-empty', + }); + }); + it('should calculate diff and return updated params', async () => { vi.mocked( modifiableToolModule.isModifiableDeclarativeTool, diff --git a/packages/core/src/scheduler/tool-modifier.ts b/packages/core/src/scheduler/tool-modifier.ts index c7d9c93c67..d964372bde 100644 --- a/packages/core/src/scheduler/tool-modifier.ts +++ b/packages/core/src/scheduler/tool-modifier.ts @@ -70,7 +70,7 @@ export class ToolModificationHandler { ): Promise { if ( toolCall.confirmationDetails.type !== 'edit' || - !payload.newContent || + !('newContent' in payload) || !isModifiableDeclarativeTool(toolCall.tool) ) { return undefined; diff --git a/packages/core/src/tools/ask-user.ts b/packages/core/src/tools/ask-user.ts index 0e1989967b..c155dec4e9 100644 --- a/packages/core/src/tools/ask-user.ts +++ b/packages/core/src/tools/ask-user.ts @@ -180,7 +180,7 @@ export class AskUserInvocation extends BaseToolInvocation< payload?: ToolConfirmationPayload, ) => { this.confirmationOutcome = outcome; - if (payload?.answers) { + if (payload && 'answers' in payload) { this.userAnswers = payload.answers; } }, diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index d95e5246bf..9c308ecba6 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -693,14 +693,18 @@ export interface ToolEditConfirmationDetails { ideConfirmation?: Promise; } -export interface ToolConfirmationPayload { - // used to override `modifiedProposedContent` for modifiable tools in the - // inline modify flow - newContent?: string; - // used for askuser tool to return user's answers - answers?: { [questionIndex: string]: string }; +export interface ToolEditConfirmationPayload { + newContent: string; } +export interface ToolAskUserConfirmationPayload { + answers: { [questionIndex: string]: string }; +} + +export type ToolConfirmationPayload = + | ToolEditConfirmationPayload + | ToolAskUserConfirmationPayload; + export interface ToolExecuteConfirmationDetails { type: 'exec'; title: string;