fix(cli): Group cancelled tool call responses to prevent API errors (#3333)
This commit is contained in:
parent
8adc586973
commit
4be32d1f73
|
@ -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<void>)
|
||||||
|
| 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 () => {
|
it('should not flicker streaming state to Idle between tool completion and submission', async () => {
|
||||||
const toolCallResponseParts: PartListUnion = [
|
const toolCallResponseParts: PartListUnion = [
|
||||||
{ text: 'tool 1 final response' },
|
{ text: 'tool 1 final response' },
|
||||||
|
|
|
@ -636,21 +636,21 @@ export const useGeminiStream = (
|
||||||
const responsesToAdd = geminiTools.flatMap(
|
const responsesToAdd = geminiTools.flatMap(
|
||||||
(toolCall) => toolCall.response.responseParts,
|
(toolCall) => toolCall.response.responseParts,
|
||||||
);
|
);
|
||||||
|
const combinedParts: Part[] = [];
|
||||||
for (const response of responsesToAdd) {
|
for (const response of responsesToAdd) {
|
||||||
let parts: Part[];
|
|
||||||
if (Array.isArray(response)) {
|
if (Array.isArray(response)) {
|
||||||
parts = response;
|
combinedParts.push(...response);
|
||||||
} else if (typeof response === 'string') {
|
} else if (typeof response === 'string') {
|
||||||
parts = [{ text: response }];
|
combinedParts.push({ text: response });
|
||||||
} else {
|
} else {
|
||||||
parts = [response];
|
combinedParts.push(response);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
geminiClient.addHistory({
|
geminiClient.addHistory({
|
||||||
role: 'user',
|
role: 'user',
|
||||||
parts,
|
parts: combinedParts,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
const callIdsToMarkAsSubmitted = geminiTools.map(
|
const callIdsToMarkAsSubmitted = geminiTools.map(
|
||||||
(toolCall) => toolCall.request.callId,
|
(toolCall) => toolCall.request.callId,
|
||||||
|
|
Loading…
Reference in New Issue