feat(plan): refactor ToolConfirmationPayload to union type (#17980)

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?: ToolConfirmationPayload) => {
-      outcome: ToolConfirmationOutcome,
-      payload?: { answers?: { [questionIndex: string]: string } },
-    ) => {
       void confirm(callId, outcome, payload).catch((error: unknown) => {
         debugLogger.error(
           `Failed to handle tool confirmation for ${callId}:`,

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<string, unknown>,
+        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: [] }),

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,

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;
     }

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,

packages/core/src/scheduler/tool-modifier.ts

@@ -70,7 +70,7 @@ export class ToolModificationHandler {
   ): Promise<ModificationResult | undefined> {
     if (
       toolCall.confirmationDetails.type !== 'edit' ||
-      !payload.newContent ||
+      !('newContent' in payload) ||
       !isModifiableDeclarativeTool(toolCall.tool)
     ) {
       return undefined;

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;
         }
       },

packages/core/src/tools/tools.ts

@@ -693,14 +693,18 @@ export interface ToolEditConfirmationDetails {
   ideConfirmation?: Promise<DiffUpdateResult>;
 }
 
-export interface ToolConfirmationPayload {
+export interface ToolEditConfirmationPayload {
-  // used to override `modifiedProposedContent` for modifiable tools in the
+  newContent: string;
-  // inline modify flow
-  newContent?: string;
-  // used for askuser tool to return user's answers
-  answers?: { [questionIndex: string]: string };
 }
 
+export interface ToolAskUserConfirmationPayload {
+  answers: { [questionIndex: string]: string };
+}
+
+export type ToolConfirmationPayload =
+  | ToolEditConfirmationPayload
+  | ToolAskUserConfirmationPayload;
+
 export interface ToolExecuteConfirmationDetails {
   type: 'exec';
   title: string;