diff --git a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx index 68b662df7b..4edd41d66c 100644 --- a/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx +++ b/packages/cli/src/ui/components/AlternateBufferQuittingDisplay.test.tsx @@ -182,7 +182,6 @@ describe('AlternateBufferQuittingDisplay', () => { type: 'info', title: 'Confirm Tool', prompt: 'Confirm this action?', - onConfirm: async () => {}, }, }, ], diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx index b232ff948a..089140d1ff 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.test.tsx @@ -210,7 +210,6 @@ describe('', () => { command: 'echo "\u001b[31mhello\u001b[0m"', rootCommand: 'echo', rootCommands: ['echo'], - onConfirm: async () => {}, }, }, ], diff --git a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx index 6df45442c1..8ddbcbce4d 100644 --- a/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx +++ b/packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { Box } from 'ink'; import { ToolConfirmationQueue } from './ToolConfirmationQueue.js'; import { ToolCallStatus, StreamingState } from '../types.js'; @@ -70,7 +70,6 @@ describe('ToolConfirmationQueue', () => { command: 'ls', rootCommand: 'ls', rootCommands: ['ls'], - onConfirm: vi.fn(), }, }, index: 1, @@ -144,7 +143,6 @@ describe('ToolConfirmationQueue', () => { fileDiff: longDiff, originalContent: 'old', newContent: 'new', - onConfirm: vi.fn(), }, }, index: 1, @@ -192,7 +190,6 @@ describe('ToolConfirmationQueue', () => { fileDiff: longDiff, originalContent: 'old', newContent: 'new', - onConfirm: vi.fn(), }, }, index: 1, @@ -242,7 +239,6 @@ describe('ToolConfirmationQueue', () => { fileDiff: longDiff, originalContent: 'old', newContent: 'new', - onConfirm: vi.fn(), }, }, index: 1, diff --git a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx index a1159d4658..807a173778 100644 --- a/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx +++ b/packages/cli/src/ui/components/messages/RedirectionConfirmation.test.tsx @@ -4,10 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { describe, it, expect, beforeAll } from 'vitest'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import type { - ToolCallConfirmationDetails, + SerializableConfirmationDetails, Config, } from '@google/gemini-cli-core'; import { initializeShellParsers } from '@google/gemini-cli-core'; @@ -24,13 +24,12 @@ describe('ToolConfirmationMessage Redirection', () => { } as unknown as Config; it('should display redirection warning and tip for redirected commands', () => { - const confirmationDetails: ToolCallConfirmationDetails = { + const confirmationDetails: SerializableConfirmationDetails = { type: 'exec', title: 'Confirm Shell Command', command: 'echo "hello" > test.txt', rootCommand: 'echo, redirection (>)', rootCommands: ['echo'], - onConfirm: vi.fn(), }; const { lastFrame } = renderWithProviders( diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 283a24843f..3d0afc4ec8 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -7,7 +7,7 @@ import { describe, it, expect, vi } from 'vitest'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import type { - ToolCallConfirmationDetails, + SerializableConfirmationDetails, Config, } from '@google/gemini-cli-core'; import { renderWithProviders } from '../../../test-utils/render.js'; @@ -39,12 +39,11 @@ describe('ToolConfirmationMessage', () => { } as unknown as Config; it('should not display urls if prompt and url are the same', () => { - const confirmationDetails: ToolCallConfirmationDetails = { + const confirmationDetails: SerializableConfirmationDetails = { type: 'info', title: 'Confirm Web Fetch', prompt: 'https://example.com', urls: ['https://example.com'], - onConfirm: vi.fn(), }; const { lastFrame } = renderWithProviders( @@ -61,7 +60,7 @@ describe('ToolConfirmationMessage', () => { }); it('should display urls if prompt and url are different', () => { - const confirmationDetails: ToolCallConfirmationDetails = { + const confirmationDetails: SerializableConfirmationDetails = { type: 'info', title: 'Confirm Web Fetch', prompt: @@ -69,7 +68,6 @@ describe('ToolConfirmationMessage', () => { urls: [ 'https://raw.githubusercontent.com/google/gemini-react/main/README.md', ], - onConfirm: vi.fn(), }; const { lastFrame } = renderWithProviders( @@ -86,14 +84,13 @@ describe('ToolConfirmationMessage', () => { }); it('should display multiple commands for exec type when provided', () => { - const confirmationDetails: ToolCallConfirmationDetails = { + const confirmationDetails: SerializableConfirmationDetails = { type: 'exec', title: 'Confirm Multiple Commands', command: 'echo "hello"', // Primary command rootCommand: 'echo', rootCommands: ['echo'], commands: ['echo "hello"', 'ls -la', 'whoami'], // Multi-command list - onConfirm: vi.fn(), }; const { lastFrame } = renderWithProviders( @@ -114,7 +111,7 @@ describe('ToolConfirmationMessage', () => { }); describe('with folder trust', () => { - const editConfirmationDetails: ToolCallConfirmationDetails = { + const editConfirmationDetails: SerializableConfirmationDetails = { type: 'edit', title: 'Confirm Edit', fileName: 'test.txt', @@ -122,33 +119,29 @@ describe('ToolConfirmationMessage', () => { fileDiff: '...diff...', originalContent: 'a', newContent: 'b', - onConfirm: vi.fn(), }; - const execConfirmationDetails: ToolCallConfirmationDetails = { + const execConfirmationDetails: SerializableConfirmationDetails = { type: 'exec', title: 'Confirm Execution', command: 'echo "hello"', rootCommand: 'echo', rootCommands: ['echo'], - onConfirm: vi.fn(), }; - const infoConfirmationDetails: ToolCallConfirmationDetails = { + const infoConfirmationDetails: SerializableConfirmationDetails = { type: 'info', title: 'Confirm Web Fetch', prompt: 'https://example.com', urls: ['https://example.com'], - onConfirm: vi.fn(), }; - const mcpConfirmationDetails: ToolCallConfirmationDetails = { + const mcpConfirmationDetails: SerializableConfirmationDetails = { type: 'mcp', title: 'Confirm MCP Tool', serverName: 'test-server', toolName: 'test-tool', toolDisplayName: 'Test Tool', - onConfirm: vi.fn(), }; describe.each([ @@ -214,7 +207,7 @@ describe('ToolConfirmationMessage', () => { }); describe('enablePermanentToolApproval setting', () => { - const editConfirmationDetails: ToolCallConfirmationDetails = { + const editConfirmationDetails: SerializableConfirmationDetails = { type: 'edit', title: 'Confirm Edit', fileName: 'test.txt', @@ -222,7 +215,6 @@ describe('ToolConfirmationMessage', () => { fileDiff: '...diff...', originalContent: 'a', newContent: 'b', - onConfirm: vi.fn(), }; it('should NOT show "Allow for all future sessions" when setting is false (default)', () => { @@ -275,7 +267,7 @@ describe('ToolConfirmationMessage', () => { }); describe('Modify with external editor option', () => { - const editConfirmationDetails: ToolCallConfirmationDetails = { + const editConfirmationDetails: SerializableConfirmationDetails = { type: 'edit', title: 'Confirm Edit', fileName: 'test.txt', @@ -283,7 +275,6 @@ describe('ToolConfirmationMessage', () => { fileDiff: '...diff...', originalContent: 'a', newContent: 'b', - onConfirm: vi.fn(), }; it('should show "Modify with external editor" when NOT in IDE mode', () => { diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index a527c13314..13feb1682f 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -11,7 +11,6 @@ import { DiffRenderer } from './DiffRenderer.js'; import { RenderInline } from '../../utils/InlineMarkdownRenderer.js'; import { type SerializableConfirmationDetails, - type ToolCallConfirmationDetails, type Config, type ToolConfirmationPayload, ToolConfirmationOutcome, @@ -38,9 +37,7 @@ import { ExitPlanModeDialog } from '../ExitPlanModeDialog.js'; export interface ToolConfirmationMessageProps { callId: string; - confirmationDetails: - | ToolCallConfirmationDetails - | SerializableConfirmationDetails; + confirmationDetails: SerializableConfirmationDetails; config: Config; isFocused?: boolean; availableTerminalHeight?: number; diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index d2ada4d659..3ce4fc54eb 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -70,7 +70,6 @@ describe('', () => { type: 'info', title: 'Confirm tool', prompt: 'Do you want to proceed?', - onConfirm: vi.fn(), }, }), ]; diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx index 3260ff3f0f..5a0cb5143e 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.test.tsx @@ -14,7 +14,6 @@ import { ToolConfirmationOutcome, MessageBusType, IdeClient, - type ToolCallConfirmationDetails, } from '@google/gemini-cli-core'; import { ToolCallStatus, type IndividualToolCallDisplay } from '../types.js'; @@ -50,21 +49,9 @@ describe('ToolActionsContext', () => { resultDisplay: undefined, confirmationDetails: { type: 'info', title: 'title', prompt: 'prompt' }, }, - { - callId: 'legacy-call', - name: 'legacy-tool', - description: 'desc', - status: ToolCallStatus.Confirming, - resultDisplay: undefined, - confirmationDetails: { - type: 'info', - title: 'legacy', - prompt: 'prompt', - onConfirm: vi.fn(), - } as ToolCallConfirmationDetails, - }, { callId: 'edit-call', + correlationId: 'corr-edit', name: 'edit-tool', description: 'desc', status: ToolCallStatus.Confirming, @@ -77,8 +64,7 @@ describe('ToolActionsContext', () => { fileDiff: 'diff', originalContent: 'old', newContent: 'new', - onConfirm: vi.fn(), - } as ToolCallConfirmationDetails, + }, }, ]; @@ -92,7 +78,7 @@ describe('ToolActionsContext', () => { ); - it('publishes to MessageBus for tools with correlationId (Modern Path)', async () => { + it('publishes to MessageBus for tools with correlationId', async () => { const { result } = renderHook(() => useToolActions(), { wrapper }); await result.current.confirm( @@ -110,27 +96,6 @@ describe('ToolActionsContext', () => { }); }); - it('calls onConfirm for legacy tools (Legacy Path)', async () => { - const { result } = renderHook(() => useToolActions(), { wrapper }); - const legacyDetails = mockToolCalls[1] - .confirmationDetails as ToolCallConfirmationDetails; - - await result.current.confirm( - 'legacy-call', - ToolConfirmationOutcome.ProceedOnce, - ); - - if (legacyDetails && 'onConfirm' in legacyDetails) { - expect(legacyDetails.onConfirm).toHaveBeenCalledWith( - ToolConfirmationOutcome.ProceedOnce, - undefined, - ); - } else { - throw new Error('Expected onConfirm to be present'); - } - expect(mockMessageBus.publish).not.toHaveBeenCalled(); - }); - it('handles cancel by calling confirm with Cancel outcome', async () => { const { result } = renderHook(() => useToolActions(), { wrapper }); @@ -170,13 +135,11 @@ describe('ToolActionsContext', () => { '/f.txt', 'accepted', ); - const editDetails = mockToolCalls[2] - .confirmationDetails as ToolCallConfirmationDetails; - if (editDetails && 'onConfirm' in editDetails) { - expect(editDetails.onConfirm).toHaveBeenCalled(); - } else { - throw new Error('Expected onConfirm to be present'); - } + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + correlationId: 'corr-edit', + }), + ); }); it('updates isDiffingEnabled when IdeClient status changes', async () => { diff --git a/packages/cli/src/ui/contexts/ToolActionsContext.tsx b/packages/cli/src/ui/contexts/ToolActionsContext.tsx index b0b67ebf38..d2d4f4322a 100644 --- a/packages/cli/src/ui/contexts/ToolActionsContext.tsx +++ b/packages/cli/src/ui/contexts/ToolActionsContext.tsx @@ -18,7 +18,6 @@ import { MessageBusType, type Config, type ToolConfirmationPayload, - type ToolCallConfirmationDetails, debugLogger, } from '@google/gemini-cli-core'; import type { IndividualToolCallDisplay } from '../types.js'; @@ -113,8 +112,7 @@ export const ToolActionsProvider: React.FC = ( await ideClient?.resolveDiffFromCli(details.filePath, cliOutcome); } - // 2. Dispatch - // PATH A: Event Bus (Modern) + // 2. Dispatch via Event Bus if (tool.correlationId) { await config.getMessageBus().publish({ type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, @@ -127,20 +125,7 @@ export const ToolActionsProvider: React.FC = ( return; } - // PATH B: Legacy Callback (Adapter or Old Scheduler) - if ( - details && - 'onConfirm' in details && - typeof details.onConfirm === 'function' - ) { - await (details as ToolCallConfirmationDetails).onConfirm( - outcome, - payload, - ); - return; - } - - debugLogger.warn(`ToolActions: No confirmation mechanism for ${callId}`); + debugLogger.warn(`ToolActions: No correlationId for ${callId}`); }, [config, ideClient, toolCalls, isDiffingEnabled], ); diff --git a/packages/cli/src/ui/hooks/toolMapping.ts b/packages/cli/src/ui/hooks/toolMapping.ts index e83fb583bf..00072b3d14 100644 --- a/packages/cli/src/ui/hooks/toolMapping.ts +++ b/packages/cli/src/ui/hooks/toolMapping.ts @@ -7,7 +7,6 @@ import { type ToolCall, type Status as CoreStatus, - type ToolCallConfirmationDetails, type SerializableConfirmationDetails, type ToolResultDisplay, debugLogger, @@ -76,10 +75,8 @@ export function mapToDisplay( }; let resultDisplay: ToolResultDisplay | undefined = undefined; - let confirmationDetails: - | ToolCallConfirmationDetails - | SerializableConfirmationDetails - | undefined = undefined; + let confirmationDetails: SerializableConfirmationDetails | undefined = + undefined; let outputFile: string | undefined = undefined; let ptyId: number | undefined = undefined; let correlationId: string | undefined = undefined; diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index ed7168667a..3130ee6365 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -32,6 +32,7 @@ import { GeminiEventType as ServerGeminiEventType, ToolErrorType, ToolConfirmationOutcome, + MessageBusType, tokenLimit, debugLogger, coreEvents, @@ -49,6 +50,11 @@ const mockSendMessageStream = vi .fn() .mockReturnValue((async function* () {})()); const mockStartChat = vi.fn(); +const mockMessageBus = { + publish: vi.fn(), + subscribe: vi.fn(), + unsubscribe: vi.fn(), +}; const MockedGeminiClientClass = vi.hoisted(() => vi.fn().mockImplementation(function (this: any, _config: any) { @@ -250,6 +256,7 @@ describe('useGeminiStream', () => { isJitContextEnabled: vi.fn(() => false), getGlobalMemory: vi.fn(() => ''), getUserMemory: vi.fn(() => ''), + getMessageBus: vi.fn(() => mockMessageBus), getIdeMode: vi.fn(() => false), getEnableHooks: vi.fn(() => false), } as unknown as Config; @@ -399,7 +406,6 @@ describe('useGeminiStream', () => { toolName: string, callId: string, confirmationType: 'edit' | 'info', - mockOnConfirm: Mock, status: TrackedToolCall['status'] = 'awaiting_approval', ): TrackedWaitingToolCall => ({ request: { @@ -416,7 +422,6 @@ describe('useGeminiStream', () => { ? { type: 'edit', title: 'Confirm Edit', - onConfirm: mockOnConfirm, fileName: 'file.txt', filePath: '/test/file.txt', fileDiff: 'fake diff', @@ -426,7 +431,6 @@ describe('useGeminiStream', () => { : { type: 'info', title: `${toolName} confirmation`, - onConfirm: mockOnConfirm, prompt: `Execute ${toolName}?`, }, tool: { @@ -438,6 +442,7 @@ describe('useGeminiStream', () => { invocation: { getDescription: () => 'Mock description', } as unknown as AnyToolInvocation, + correlationId: `corr-${callId}`, }); // Helper to render hook with default parameters - reduces boilerplate @@ -1763,10 +1768,9 @@ describe('useGeminiStream', () => { describe('handleApprovalModeChange', () => { it('should auto-approve all pending tool calls when switching to YOLO mode', async () => { - const mockOnConfirm = vi.fn().mockResolvedValue(undefined); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - createMockToolCall('replace', 'call1', 'edit', mockOnConfirm), - createMockToolCall('read_file', 'call2', 'info', mockOnConfirm), + createMockToolCall('replace', 'call1', 'edit'), + createMockToolCall('read_file', 'call2', 'info'), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1776,21 +1780,27 @@ describe('useGeminiStream', () => { }); // Both tool calls should be auto-approved - expect(mockOnConfirm).toHaveBeenCalledTimes(2); - expect(mockOnConfirm).toHaveBeenCalledWith( - ToolConfirmationOutcome.ProceedOnce, + expect(mockMessageBus.publish).toHaveBeenCalledTimes(2); + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: 'corr-call1', + outcome: ToolConfirmationOutcome.ProceedOnce, + }), + ); + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ + correlationId: 'corr-call2', + outcome: ToolConfirmationOutcome.ProceedOnce, + }), ); }); it('should only auto-approve edit tools when switching to AUTO_EDIT mode', async () => { - const mockOnConfirmReplace = vi.fn().mockResolvedValue(undefined); - const mockOnConfirmWrite = vi.fn().mockResolvedValue(undefined); - const mockOnConfirmRead = vi.fn().mockResolvedValue(undefined); - const awaitingApprovalToolCalls: TrackedToolCall[] = [ - createMockToolCall('replace', 'call1', 'edit', mockOnConfirmReplace), - createMockToolCall('write_file', 'call2', 'edit', mockOnConfirmWrite), - createMockToolCall('read_file', 'call3', 'info', mockOnConfirmRead), + createMockToolCall('replace', 'call1', 'edit'), + createMockToolCall('write_file', 'call2', 'edit'), + createMockToolCall('read_file', 'call3', 'info'), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1800,21 +1810,21 @@ describe('useGeminiStream', () => { }); // Only replace and write_file should be auto-approved - expect(mockOnConfirmReplace).toHaveBeenCalledWith( - ToolConfirmationOutcome.ProceedOnce, + expect(mockMessageBus.publish).toHaveBeenCalledTimes(2); + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ correlationId: 'corr-call1' }), ); - expect(mockOnConfirmWrite).toHaveBeenCalledWith( - ToolConfirmationOutcome.ProceedOnce, + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ correlationId: 'corr-call2' }), + ); + expect(mockMessageBus.publish).not.toHaveBeenCalledWith( + expect.objectContaining({ correlationId: 'corr-call3' }), ); - - // read_file should not be auto-approved - expect(mockOnConfirmRead).not.toHaveBeenCalled(); }); it('should not auto-approve any tools when switching to REQUIRE_CONFIRMATION mode', async () => { - const mockOnConfirm = vi.fn().mockResolvedValue(undefined); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - createMockToolCall('replace', 'call1', 'edit', mockOnConfirm), + createMockToolCall('replace', 'call1', 'edit'), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1824,21 +1834,19 @@ describe('useGeminiStream', () => { }); // No tools should be auto-approved - expect(mockOnConfirm).not.toHaveBeenCalled(); + expect(mockMessageBus.publish).not.toHaveBeenCalled(); }); it('should handle errors gracefully when auto-approving tool calls', async () => { const debuggerSpy = vi .spyOn(debugLogger, 'warn') .mockImplementation(() => {}); - const mockOnConfirmSuccess = vi.fn().mockResolvedValue(undefined); - const mockOnConfirmError = vi - .fn() - .mockRejectedValue(new Error('Approval failed')); + + mockMessageBus.publish.mockRejectedValueOnce(new Error('Bus error')); const awaitingApprovalToolCalls: TrackedToolCall[] = [ - createMockToolCall('replace', 'call1', 'edit', mockOnConfirmSuccess), - createMockToolCall('write_file', 'call2', 'edit', mockOnConfirmError), + createMockToolCall('replace', 'call1', 'edit'), + createMockToolCall('write_file', 'call2', 'edit'), ]; const { result } = renderTestHook(awaitingApprovalToolCalls); @@ -1847,13 +1855,10 @@ describe('useGeminiStream', () => { await result.current.handleApprovalModeChange(ApprovalMode.YOLO); }); - // Both confirmation methods should be called - expect(mockOnConfirmSuccess).toHaveBeenCalled(); - expect(mockOnConfirmError).toHaveBeenCalled(); - - // Error should be logged + // Both should be attempted despite first error + expect(mockMessageBus.publish).toHaveBeenCalledTimes(2); expect(debuggerSpy).toHaveBeenCalledWith( - 'Failed to auto-approve tool call call2:', + 'Failed to auto-approve tool call call1:', expect.any(Error), ); @@ -1882,6 +1887,7 @@ describe('useGeminiStream', () => { invocation: { getDescription: () => 'Mock description', } as unknown as AnyToolInvocation, + correlationId: 'corr-1', } as unknown as TrackedWaitingToolCall, ]; @@ -1893,83 +1899,9 @@ describe('useGeminiStream', () => { }); }); - it('should skip tool calls without onConfirm method in confirmationDetails', async () => { - const awaitingApprovalToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - // No onConfirm method - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - } as any, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, - ]; - - const { result } = renderTestHook(awaitingApprovalToolCalls); - - // Should not throw an error - await act(async () => { - await result.current.handleApprovalModeChange(ApprovalMode.YOLO); - }); - }); - it('should only process tool calls with awaiting_approval status', async () => { - const mockOnConfirmAwaiting = vi.fn().mockResolvedValue(undefined); - const mockOnConfirmExecuting = vi.fn().mockResolvedValue(undefined); - const mixedStatusToolCalls: TrackedToolCall[] = [ - { - request: { - callId: 'call1', - name: 'replace', - args: { old_string: 'old', new_string: 'new' }, - isClientInitiated: false, - prompt_id: 'prompt-id-1', - }, - status: 'awaiting_approval', - responseSubmittedToGemini: false, - confirmationDetails: { - type: 'edit', - title: 'Confirm Edit', - onConfirm: mockOnConfirmAwaiting, - fileName: 'file.txt', - filePath: '/test/file.txt', - fileDiff: 'fake diff', - originalContent: 'old', - newContent: 'new', - }, - tool: { - name: 'replace', - displayName: 'replace', - description: 'Replace text', - build: vi.fn(), - } as any, - invocation: { - getDescription: () => 'Mock description', - } as unknown as AnyToolInvocation, - } as TrackedWaitingToolCall, + createMockToolCall('replace', 'call1', 'edit'), { request: { callId: 'call2', @@ -1991,6 +1923,7 @@ describe('useGeminiStream', () => { } as unknown as AnyToolInvocation, startTime: Date.now(), liveOutput: 'Writing...', + correlationId: 'corr-call2', } as TrackedExecutingToolCall, ]; @@ -2000,9 +1933,14 @@ describe('useGeminiStream', () => { await result.current.handleApprovalModeChange(ApprovalMode.YOLO); }); - // Only the awaiting_approval tool should be processed - expect(mockOnConfirmAwaiting).toHaveBeenCalledTimes(1); - expect(mockOnConfirmExecuting).not.toHaveBeenCalled(); + // Only the awaiting_approval tool should be processed. + expect(mockMessageBus.publish).toHaveBeenCalledTimes(1); + expect(mockMessageBus.publish).toHaveBeenCalledWith( + expect.objectContaining({ correlationId: 'corr-call1' }), + ); + expect(mockMessageBus.publish).not.toHaveBeenCalledWith( + expect.objectContaining({ correlationId: 'corr-call2' }), + ); }); }); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index bba6977ffa..13f6d8cf70 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -20,6 +20,7 @@ import { ApprovalMode, parseAndFormatApiError, ToolConfirmationOutcome, + MessageBusType, promptIdContext, tokenLimit, debugLogger, @@ -1408,10 +1409,15 @@ export const useGeminiStream = ( // Process pending tool calls sequentially to reduce UI chaos for (const call of awaitingApprovalCalls) { - const details = call.confirmationDetails; - if (details && 'onConfirm' in details) { + if (call.correlationId) { try { - await details.onConfirm(ToolConfirmationOutcome.ProceedOnce); + await config.getMessageBus().publish({ + type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, + correlationId: call.correlationId, + confirmed: true, + requiresUserConfirmation: false, + outcome: ToolConfirmationOutcome.ProceedOnce, + }); } catch (error) { debugLogger.warn( `Failed to auto-approve tool call ${call.request.callId}:`, @@ -1422,7 +1428,7 @@ export const useGeminiStream = ( } } }, - [toolCalls], + [config, toolCalls], ); const handleCompletedTools = useCallback( diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index 4a04d6225c..8ebf439630 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -10,12 +10,10 @@ import { renderHook } from '../../test-utils/render.js'; import { useToolScheduler } from './useToolScheduler.js'; import { MessageBusType, - ToolConfirmationOutcome, Scheduler, type Config, type MessageBus, type CompletedToolCall, - type ToolCallConfirmationDetails, type ToolCallsUpdateMessage, type AnyDeclarativeTool, type AnyToolInvocation, @@ -132,122 +130,6 @@ describe('useToolScheduler', () => { }); }); - it('injects onConfirm callback for awaiting_approval tools (Adapter Pattern)', async () => { - const { result } = renderHook(() => - useToolScheduler( - vi.fn().mockResolvedValue(undefined), - mockConfig, - () => undefined, - ), - ); - - const mockToolCall = { - status: 'awaiting_approval' as const, - request: { - callId: 'call-1', - name: 'test_tool', - args: {}, - isClientInitiated: false, - prompt_id: 'p1', - }, - tool: createMockTool(), - invocation: createMockInvocation({ - getDescription: () => 'Confirming test tool', - }), - confirmationDetails: { type: 'info', title: 'Confirm', prompt: 'Sure?' }, - correlationId: 'corr-123', - }; - - act(() => { - void mockMessageBus.publish({ - type: MessageBusType.TOOL_CALLS_UPDATE, - toolCalls: [mockToolCall], - schedulerId: ROOT_SCHEDULER_ID, - } as ToolCallsUpdateMessage); - }); - - const [toolCalls] = result.current; - const call = toolCalls[0]; - if (call.status !== 'awaiting_approval') { - throw new Error('Expected status to be awaiting_approval'); - } - const confirmationDetails = - call.confirmationDetails as ToolCallConfirmationDetails; - - expect(confirmationDetails).toBeDefined(); - expect(typeof confirmationDetails.onConfirm).toBe('function'); - - // Test that onConfirm publishes to MessageBus - const publishSpy = vi.spyOn(mockMessageBus, 'publish'); - await confirmationDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce); - - expect(publishSpy).toHaveBeenCalledWith({ - type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, - correlationId: 'corr-123', - confirmed: true, - requiresUserConfirmation: false, - outcome: ToolConfirmationOutcome.ProceedOnce, - payload: undefined, - }); - }); - - it('injects onConfirm with payload (Inline Edit support)', async () => { - const { result } = renderHook(() => - useToolScheduler( - vi.fn().mockResolvedValue(undefined), - mockConfig, - () => undefined, - ), - ); - - const mockToolCall = { - status: 'awaiting_approval' as const, - request: { - callId: 'call-1', - name: 'test_tool', - args: {}, - isClientInitiated: false, - prompt_id: 'p1', - }, - tool: createMockTool(), - invocation: createMockInvocation(), - confirmationDetails: { type: 'edit', title: 'Edit', filePath: 'test.ts' }, - correlationId: 'corr-edit', - }; - - act(() => { - void mockMessageBus.publish({ - type: MessageBusType.TOOL_CALLS_UPDATE, - toolCalls: [mockToolCall], - schedulerId: ROOT_SCHEDULER_ID, - } as ToolCallsUpdateMessage); - }); - - const [toolCalls] = result.current; - const call = toolCalls[0]; - if (call.status !== 'awaiting_approval') { - throw new Error('Expected awaiting_approval'); - } - const confirmationDetails = - call.confirmationDetails as ToolCallConfirmationDetails; - - const publishSpy = vi.spyOn(mockMessageBus, 'publish'); - const mockPayload = { newContent: 'updated code' }; - await confirmationDetails.onConfirm( - ToolConfirmationOutcome.ProceedOnce, - mockPayload, - ); - - expect(publishSpy).toHaveBeenCalledWith({ - type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, - correlationId: 'corr-edit', - confirmed: true, - requiresUserConfirmation: false, - outcome: ToolConfirmationOutcome.ProceedOnce, - payload: mockPayload, - }); - }); - it('preserves responseSubmittedToGemini flag across updates', () => { const { result } = renderHook(() => useToolScheduler( diff --git a/packages/cli/src/ui/hooks/useToolScheduler.ts b/packages/cli/src/ui/hooks/useToolScheduler.ts index b50ed1b717..89bee14342 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.ts @@ -6,21 +6,18 @@ import { type Config, - type MessageBus, type ToolCallRequestInfo, type ToolCall, type CompletedToolCall, - type ToolConfirmationPayload, MessageBusType, - ToolConfirmationOutcome, + ROOT_SCHEDULER_ID, Scheduler, type EditorType, type ToolCallsUpdateMessage, - ROOT_SCHEDULER_ID, } from '@google/gemini-cli-core'; import { useCallback, useState, useMemo, useEffect, useRef } from 'react'; -// Re-exporting types compatible with legacy hook expectations +// Re-exporting types compatible with hook expectations export type ScheduleFn = ( request: ToolCallRequestInfo | ToolCallRequestInfo[], signal: AbortSignal, @@ -109,8 +106,8 @@ export function useToolScheduler( const internalAdaptToolCalls = useCallback( (coreCalls: ToolCall[], prevTracked: TrackedToolCall[]) => - adaptToolCalls(coreCalls, prevTracked, messageBus), - [messageBus], + adaptToolCalls(coreCalls, prevTracked), + [], ); useEffect(() => { @@ -227,12 +224,11 @@ export function useToolScheduler( } /** - * ADAPTER: Merges UI metadata (submitted flag) and injects legacy callbacks. + * ADAPTER: Merges UI metadata (submitted flag). */ function adaptToolCalls( coreCalls: ToolCall[], prevTracked: TrackedToolCall[], - messageBus: MessageBus, ): TrackedToolCall[] { const prevMap = new Map(prevTracked.map((t) => [t.request.callId, t])); @@ -240,34 +236,6 @@ function adaptToolCalls( const prev = prevMap.get(coreCall.request.callId); const responseSubmittedToGemini = prev?.responseSubmittedToGemini ?? false; - // Inject onConfirm adapter for tools awaiting approval. - // The Core provides data-only (serializable) confirmationDetails. We must - // inject the legacy callback function that proxies responses back to the - // MessageBus. - if (coreCall.status === 'awaiting_approval' && coreCall.correlationId) { - const correlationId = coreCall.correlationId; - return { - ...coreCall, - confirmationDetails: { - ...coreCall.confirmationDetails, - onConfirm: async ( - outcome: ToolConfirmationOutcome, - payload?: ToolConfirmationPayload, - ) => { - await messageBus.publish({ - type: MessageBusType.TOOL_CONFIRMATION_RESPONSE, - correlationId, - confirmed: outcome !== ToolConfirmationOutcome.Cancel, - requiresUserConfirmation: false, - outcome, - payload, - }); - }, - }, - responseSubmittedToGemini, - }; - } - return { ...coreCall, responseSubmittedToGemini, diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index c48b81bf9c..ca9a992f80 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -9,7 +9,6 @@ import type { GeminiCLIExtension, MCPServerConfig, ThoughtSummary, - ToolCallConfirmationDetails, SerializableConfirmationDetails, ToolResultDisplay, RetrieveUserQuotaResponse, @@ -64,10 +63,7 @@ export interface ToolCallEvent { name: string; args: Record; resultDisplay: ToolResultDisplay | undefined; - confirmationDetails: - | ToolCallConfirmationDetails - | SerializableConfirmationDetails - | undefined; + confirmationDetails: SerializableConfirmationDetails | undefined; correlationId?: string; } @@ -77,10 +73,7 @@ export interface IndividualToolCallDisplay { description: string; resultDisplay: ToolResultDisplay | undefined; status: ToolCallStatus; - confirmationDetails: - | ToolCallConfirmationDetails - | SerializableConfirmationDetails - | undefined; + confirmationDetails: SerializableConfirmationDetails | undefined; renderOutputAsMarkdown?: boolean; ptyId?: number; outputFile?: string; diff --git a/packages/cli/src/ui/utils/textUtils.test.ts b/packages/cli/src/ui/utils/textUtils.test.ts index 0f9b2fcd39..be7f69d9f6 100644 --- a/packages/cli/src/ui/utils/textUtils.test.ts +++ b/packages/cli/src/ui/utils/textUtils.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect } from 'vitest'; import type { - ToolCallConfirmationDetails, + SerializableConfirmationDetails, ToolEditConfirmationDetails, } from '@google/gemini-cli-core'; import { @@ -366,13 +366,12 @@ describe('textUtils', () => { describe('toolConfirmationDetails case study', () => { it('should sanitize command and rootCommand for exec type', () => { - const details: ToolCallConfirmationDetails = { + const details: SerializableConfirmationDetails = { title: '\u001b[34mfake-title\u001b[0m', type: 'exec', command: '\u001b[31mmls -l\u001b[0m', rootCommand: '\u001b[32msudo apt-get update\u001b[0m', rootCommands: ['sudo'], - onConfirm: async () => {}, }; const sanitized = escapeAnsiCtrlCodes(details); @@ -387,14 +386,13 @@ describe('textUtils', () => { }); it('should sanitize properties for edit type', () => { - const details: ToolCallConfirmationDetails = { + const details: SerializableConfirmationDetails = { type: 'edit', title: '\u001b[34mEdit File\u001b[0m', fileName: '\u001b[31mfile.txt\u001b[0m', filePath: '/path/to/\u001b[32mfile.txt\u001b[0m', fileDiff: 'diff --git a/file.txt b/file.txt\n--- a/\u001b[33mfile.txt\u001b[0m\n+++ b/file.txt', - onConfirm: async () => {}, } as unknown as ToolEditConfirmationDetails; const sanitized = escapeAnsiCtrlCodes(details); @@ -412,13 +410,12 @@ describe('textUtils', () => { }); it('should sanitize properties for mcp type', () => { - const details: ToolCallConfirmationDetails = { + const details: SerializableConfirmationDetails = { type: 'mcp', title: '\u001b[34mCloud Run\u001b[0m', serverName: '\u001b[31mmy-server\u001b[0m', toolName: '\u001b[32mdeploy\u001b[0m', toolDisplayName: '\u001b[33mDeploy Service\u001b[0m', - onConfirm: async () => {}, }; const sanitized = escapeAnsiCtrlCodes(details); @@ -434,12 +431,11 @@ describe('textUtils', () => { }); it('should sanitize properties for info type', () => { - const details: ToolCallConfirmationDetails = { + const details: SerializableConfirmationDetails = { type: 'info', title: '\u001b[34mWeb Search\u001b[0m', prompt: '\u001b[31mSearch for cats\u001b[0m', urls: ['https://\u001b[32mgoogle.com\u001b[0m'], - onConfirm: async () => {}, }; const sanitized = escapeAnsiCtrlCodes(details); @@ -457,12 +453,11 @@ describe('textUtils', () => { }); it('should not change the object if no sanitization is needed', () => { - const details: ToolCallConfirmationDetails = { + const details: SerializableConfirmationDetails = { type: 'info', title: 'Web Search', prompt: 'Search for cats', urls: ['https://google.com'], - onConfirm: async () => {}, }; const sanitized = escapeAnsiCtrlCodes(details);