From 4be32d1f73bc2d9e4d22a71eee7b142e9b0aa577 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Sat, 5 Jul 2025 13:56:39 -0700 Subject: [PATCH] fix(cli): Group cancelled tool call responses to prevent API errors (#3333) --- .../cli/src/ui/hooks/useGeminiStream.test.tsx | 106 ++++++++++++++++++ packages/cli/src/ui/hooks/useGeminiStream.ts | 16 +-- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index 1ccf3123..6a41234b 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -604,6 +604,112 @@ describe('useGeminiStream', () => { }); }); + it('should group multiple cancelled tool call responses into a single history entry', async () => { + const cancelledToolCall1: TrackedCancelledToolCall = { + request: { + callId: 'cancel-1', + name: 'toolA', + args: {}, + isClientInitiated: false, + }, + tool: { + name: 'toolA', + description: 'descA', + getDescription: vi.fn(), + } as any, + status: 'cancelled', + response: { + callId: 'cancel-1', + responseParts: [ + { functionResponse: { name: 'toolA', id: 'cancel-1' } }, + ], + resultDisplay: undefined, + error: undefined, + }, + responseSubmittedToGemini: false, + }; + const cancelledToolCall2: TrackedCancelledToolCall = { + request: { + callId: 'cancel-2', + name: 'toolB', + args: {}, + isClientInitiated: false, + }, + tool: { + name: 'toolB', + description: 'descB', + getDescription: vi.fn(), + } as any, + status: 'cancelled', + response: { + callId: 'cancel-2', + responseParts: [ + { functionResponse: { name: 'toolB', id: 'cancel-2' } }, + ], + resultDisplay: undefined, + error: undefined, + }, + responseSubmittedToGemini: false, + }; + const allCancelledTools = [cancelledToolCall1, cancelledToolCall2]; + const client = new MockedGeminiClientClass(mockConfig); + + let capturedOnComplete: + | ((completedTools: TrackedToolCall[]) => Promise) + | null = null; + + mockUseReactToolScheduler.mockImplementation((onComplete) => { + capturedOnComplete = onComplete; + return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted]; + }); + + renderHook(() => + useGeminiStream( + client, + [], + mockAddItem, + mockSetShowHelp, + mockConfig, + mockOnDebugMessage, + mockHandleSlashCommand, + false, + () => 'vscode' as EditorType, + () => {}, + () => Promise.resolve(), + ), + ); + + // Trigger the onComplete callback with multiple cancelled tools + await act(async () => { + if (capturedOnComplete) { + await capturedOnComplete(allCancelledTools); + } + }); + + await waitFor(() => { + // The tools should be marked as submitted locally + expect(mockMarkToolsAsSubmitted).toHaveBeenCalledWith([ + 'cancel-1', + 'cancel-2', + ]); + + // Crucially, addHistory should be called only ONCE + expect(client.addHistory).toHaveBeenCalledTimes(1); + + // And that single call should contain BOTH function responses + expect(client.addHistory).toHaveBeenCalledWith({ + role: 'user', + parts: [ + ...(cancelledToolCall1.response.responseParts as Part[]), + ...(cancelledToolCall2.response.responseParts as Part[]), + ], + }); + + // No message should be sent back to the API for a turn with only cancellations + expect(mockSendMessageStream).not.toHaveBeenCalled(); + }); + }); + it('should not flicker streaming state to Idle between tool completion and submission', async () => { const toolCallResponseParts: PartListUnion = [ { text: 'tool 1 final response' }, diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index e2226761..bba01bc9 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -636,20 +636,20 @@ export const useGeminiStream = ( const responsesToAdd = geminiTools.flatMap( (toolCall) => toolCall.response.responseParts, ); + const combinedParts: Part[] = []; for (const response of responsesToAdd) { - let parts: Part[]; if (Array.isArray(response)) { - parts = response; + combinedParts.push(...response); } else if (typeof response === 'string') { - parts = [{ text: response }]; + combinedParts.push({ text: response }); } else { - parts = [response]; + combinedParts.push(response); } - geminiClient.addHistory({ - role: 'user', - parts, - }); } + geminiClient.addHistory({ + role: 'user', + parts: combinedParts, + }); } const callIdsToMarkAsSubmitted = geminiTools.map(