From d0a3cd65baee472b623c66c16ccfdfe1cc483c43 Mon Sep 17 00:00:00 2001 From: YingchaoX Date: Fri, 19 Jun 2026 03:48:53 +0800 Subject: [PATCH] fix(core): ignore duplicate provider tool-call ids (#5038) * fix(core): ignore duplicate provider tool-call ids * Update packages/cli/src/nonInteractiveCli.ts Co-authored-by: Shaojin Wen * Update packages/core/src/agents/runtime/agent-core.ts Co-authored-by: Shaojin Wen * fix(cli): clean up duplicate tool-call review changes * fix(core): cover duplicate provider tool-call review feedback * Update packages/core/src/agents/runtime/agent-core.ts Co-authored-by: Shaojin Wen * fix(core): restore synthetic tool error type * test(cli): stabilize at-completion suggestions assertion * Update packages/core/src/utils/filesearch/crawler.ts Co-authored-by: Shaojin Wen * fix(cli): dedupe ACP provider tool calls --------- Co-authored-by: Shaojin Wen Co-authored-by: yingchao xiong --- package-lock.json | 1 - .../acp-integration/session/Session.test.ts | 216 +++++++++++ .../src/acp-integration/session/Session.ts | 101 ++++- packages/cli/src/nonInteractiveCli.test.ts | 360 +++++++++++++++++- packages/cli/src/nonInteractiveCli.ts | 73 +++- .../cli/src/ui/hooks/useAtCompletion.test.ts | 16 +- .../cli/src/ui/hooks/useGeminiStream.test.tsx | 233 ++++++++++++ packages/cli/src/ui/hooks/useGeminiStream.ts | 115 +++++- .../core/src/agents/runtime/agent-core.ts | 134 +++++-- .../src/agents/runtime/agent-headless.test.ts | 306 +++++++++++++++ .../core/src/core/toolCallIdUtils.test.ts | 7 + packages/core/src/core/toolCallIdUtils.ts | 27 +- packages/core/src/core/turn.test.ts | 88 +++++ packages/core/src/core/turn.ts | 36 +- packages/core/src/utils/filesearch/crawler.ts | 61 ++- 15 files changed, 1707 insertions(+), 67 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3a9707902f..43809a9884 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17313,7 +17313,6 @@ "os": [ "darwin" ], - "peer": true, "engines": { "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 383962faad..5ec440e0ee 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -274,6 +274,7 @@ describe('Session', () => { addHistory: vi.fn(), getHistory: vi.fn().mockReturnValue([]), getHistoryShallow: vi.fn().mockReturnValue([]), + getHistoryFunctionResponseIds: vi.fn().mockReturnValue(new Set()), getLastModelMessageText: vi.fn().mockReturnValue(''), setHistory: vi.fn(), truncateHistory: vi.fn(), @@ -7385,6 +7386,221 @@ describe('Session', () => { expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledOnce(); }); + it('suppresses duplicate provider functionCall ids already answered in history', async () => { + const execute = vi.fn().mockResolvedValue({ + llmContent: 'should not run', + returnDisplay: 'should not run', + }); + const build = vi.fn().mockReturnValue({ + params: { file_path: 'b.ts' }, + execute, + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + getDescription: vi.fn().mockReturnValue('Read file'), + toolLocations: vi.fn().mockReturnValue([]), + }); + mockToolRegistry.getTool.mockReturnValue({ + name: 'read_file', + kind: core.Kind.Read, + displayName: 'Read File', + description: 'Read file', + build, + canUpdateOutput: false, + isOutputMarkdown: true, + }); + vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue( + new Set(['shell_1']), + ); + const [duplicatePart] = core.normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'shell_1', + name: 'read_file', + args: { file_path: 'b.ts' }, + }, + }, + ], + new Set(['shell_1']), + new Set(), + ); + const duplicateCall = duplicatePart.functionCall!; + + const result = await ( + session as unknown as ToolCallInternals + ).runToolCalls(new AbortController().signal, 'prompt-history-dup', [ + duplicateCall, + ]); + + expect(mockToolRegistry.getTool).not.toHaveBeenCalled(); + expect(build).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + const { parts } = result; + expect(parts).toHaveLength(1); + expect(result.stopAfterUserQuestionCancel).toBe(false); + expect(parts[0].functionResponse?.id).toBe('shell_1__qwen_dup_2'); + expect(parts[0].functionResponse?.response).toEqual({ + error: expect.stringContaining( + 'Duplicate provider tool call id "shell_1"', + ), + }); + expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledWith( + parts, + expect.objectContaining({ + callId: 'shell_1__qwen_dup_2', + status: 'error', + resultDisplay: expect.stringContaining( + 'Duplicate provider tool call id "shell_1"', + ), + error: expect.any(Error), + }), + ); + expect(mockClient.sessionUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'tool_call_update', + toolCallId: 'shell_1__qwen_dup_2', + status: 'failed', + }), + }), + ); + expect(mockClient.sessionUpdate).not.toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'tool_call', + toolCallId: 'shell_1__qwen_dup_2', + }), + }), + ); + }); + + it('suppresses duplicate TodoWrite calls without emitting plan updates', async () => { + vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue( + new Set(['todo_1']), + ); + const [duplicatePart] = core.normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'todo_1', + name: core.ToolNames.TODO_WRITE, + args: { + todos: [ + { + id: 'task-1', + content: 'Do not replay this', + status: 'pending', + }, + ], + }, + }, + }, + ], + new Set(['todo_1']), + new Set(), + ); + + const result = await ( + session as unknown as ToolCallInternals + ).runToolCalls(new AbortController().signal, 'prompt-todo-dup', [ + duplicatePart.functionCall!, + ]); + + expect(mockToolRegistry.getTool).not.toHaveBeenCalled(); + const { parts } = result; + expect(result.stopAfterUserQuestionCancel).toBe(false); + expect(parts[0].functionResponse?.id).toBe('todo_1__qwen_dup_2'); + expect(parts[0].functionResponse?.response).toEqual({ + error: expect.stringContaining( + 'Duplicate provider tool call id "todo_1"', + ), + }); + expect(mockClient.sessionUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'tool_call_update', + toolCallId: 'todo_1__qwen_dup_2', + status: 'failed', + }), + }), + ); + expect(mockClient.sessionUpdate).not.toHaveBeenCalledWith( + expect.objectContaining({ + update: expect.objectContaining({ + sessionUpdate: 'plan', + }), + }), + ); + expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledWith( + parts, + expect.objectContaining({ + callId: 'todo_1__qwen_dup_2', + status: 'error', + }), + ); + }); + + it('keeps duplicate synthetic responses ordered with executable calls', async () => { + const execute = vi.fn(async () => ({ + llmContent: 'ran', + returnDisplay: 'ran', + })); + mockToolRegistry.getTool.mockReturnValue({ + name: 'read_file', + kind: core.Kind.Read, + displayName: 'Read File', + description: 'Read file', + build: vi.fn().mockReturnValue({ + params: { file_path: 'x.ts' }, + execute, + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + getDescription: vi.fn().mockReturnValue('Read file'), + toolLocations: vi.fn().mockReturnValue([]), + }), + canUpdateOutput: false, + isOutputMarkdown: true, + }); + const historyIds = new Set(['dup_mid']); + vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue( + historyIds, + ); + const [duplicatePart] = core.normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'dup_mid', + name: 'read_file', + args: { file_path: 'b.ts' }, + }, + }, + ], + new Set(['dup_mid']), + new Set(), + ); + + const result = await ( + session as unknown as ToolCallInternals + ).runToolCalls(new AbortController().signal, 'prompt-mixed-dup', [ + { id: 'call_a', name: 'read_file', args: { file_path: 'a.ts' } }, + duplicatePart.functionCall!, + { id: 'call_c', name: 'read_file', args: { file_path: 'c.ts' } }, + ]); + + expect(execute).toHaveBeenCalledTimes(2); + const { parts } = result; + expect(result.stopAfterUserQuestionCancel).toBe(false); + expect(parts.map((part) => part.functionResponse?.id)).toEqual([ + 'call_a', + 'dup_mid__qwen_dup_2', + 'call_c', + ]); + expect(parts[1].functionResponse?.response).toEqual({ + error: expect.stringContaining( + 'Duplicate provider tool call id "dup_mid"', + ), + }); + expect(historyIds).toEqual(new Set(['dup_mid'])); + }); + it('does not dedupe function calls with empty ids in one batch', async () => { const execute = vi.fn().mockResolvedValue({ llmContent: 'result', diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 401db534b9..cab3c33bae 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -26,12 +26,15 @@ import type { AutoModeDecision, AutoModeOutcome, GoalTerminalEvent, + ToolCallRequestInfo, + ToolCallResponseInfo, } from '@qwen-code/qwen-code-core'; import { AuthType, ApprovalMode, CompressionStatus, convertToFunctionResponse, + createDuplicateProviderToolCallResponse, createDebugLogger, DiscoveredMCPTool, StreamEventType, @@ -95,6 +98,7 @@ import { setGoalTerminalObserver, sessionIdContext, dedupeToolCallsById, + getProviderToolCallId, } from '@qwen-code/qwen-code-core'; import { NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE } from '@qwen-code/acp-bridge/bridgeErrors'; // Single source of truth shared with the daemon-side answerer (BridgeClient), @@ -3189,15 +3193,101 @@ export class Session implements SessionContext { functionCalls: FunctionCall[], ): Promise { const dedupedFunctionCalls = dedupeToolCallsById(functionCalls); - type Batch = { concurrent: boolean; calls: FunctionCall[] }; + type ExecutableBatch = { + kind: 'execute'; + concurrent: boolean; + calls: FunctionCall[]; + }; + type DuplicateBatch = { + kind: 'duplicate'; + request: ToolCallRequestInfo; + response: ToolCallResponseInfo; + }; + type Batch = ExecutableBatch | DuplicateBatch; const batches: Batch[] = []; + const handledProviderToolCallIds = new Set( + this.#getCurrentChat().getHistoryFunctionResponseIds(), + ); + + const pushDuplicateBatch = (request: ToolCallRequestInfo): void => { + const response = createDuplicateProviderToolCallResponse(request); + debugLogger.debug( + `[Session.runToolCalls] Suppressing duplicate provider tool-call id: ` + + `${request.providerCallId} (tool: ${request.name})`, + ); + batches.push({ kind: 'duplicate', request, response }); + }; + + const emitDuplicateBatch = async (batch: DuplicateBatch): Promise => { + const { request, response } = batch; + if (request.name === ToolNames.TODO_WRITE) { + const provenance = ToolCallEmitter.resolveToolProvenance(request.name); + await this.sendUpdate({ + sessionUpdate: 'tool_call_update', + toolCallId: response.callId, + status: 'failed', + content: [ + { + type: 'content', + content: { + type: 'text', + text: response.error?.message ?? String(response.resultDisplay), + }, + }, + ], + rawOutput: response.resultDisplay, + _meta: { + toolName: request.name, + provenance: provenance.provenance, + ...(provenance.serverId ? { serverId: provenance.serverId } : {}), + }, + }); + } else { + await this.toolCallEmitter.emitResult({ + callId: response.callId, + toolName: request.name, + args: request.args, + message: response.responseParts, + resultDisplay: response.resultDisplay, + error: response.error, + success: false, + }); + } + this.config + .getChatRecordingService() + ?.recordToolResult(response.responseParts, { + callId: response.callId, + status: 'error', + resultDisplay: response.resultDisplay, + error: response.error, + errorType: response.errorType, + }); + }; + for (const fc of dedupedFunctionCalls) { + const providerCallId = getProviderToolCallId(fc) ?? fc.id; + if (providerCallId) { + if (handledProviderToolCallIds.has(providerCallId)) { + const callId = fc.id ?? `${fc.name}-${Date.now()}`; + pushDuplicateBatch({ + callId, + providerCallId, + name: fc.name ?? 'unknown_tool', + args: (fc.args ?? {}) as Record, + isClientInitiated: false, + prompt_id: promptId, + }); + continue; + } + handledProviderToolCallIds.add(providerCallId); + } + const isAgent = fc.name === ToolNames.AGENT; const last = batches[batches.length - 1]; - if (isAgent && last?.concurrent) { + if (isAgent && last?.kind === 'execute' && last.concurrent) { last.calls.push(fc); } else { - batches.push({ concurrent: isAgent, calls: [fc] }); + batches.push({ kind: 'execute', concurrent: isAgent, calls: [fc] }); } } @@ -3291,6 +3381,11 @@ export class Session implements SessionContext { const parts: Part[] = []; for (const batch of batches) { + if (batch.kind === 'duplicate') { + await emitDuplicateBatch(batch); + parts.push(...batch.response.responseParts); + continue; + } if (batch.concurrent && batch.calls.length > 1) { const batchAbortController = new AbortController(); let batchStopAfterUserQuestionCancel = false; diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 910a501878..6ec635ead1 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -98,6 +98,7 @@ describe('runNonInteractive', () => { sendMessageStream: Mock; getChatRecordingService: Mock; getChat: Mock; + getHistoryFunctionResponseIds: Mock; consumePendingMemoryTaskPromises: Mock; recordCompletedToolCall: Mock; }; @@ -162,6 +163,7 @@ describe('runNonInteractive', () => { recordToolCalls: vi.fn(), })), getChat: vi.fn(() => ({})), + getHistoryFunctionResponseIds: vi.fn(() => new Set()), }; let currentModel = 'test-model'; @@ -440,6 +442,176 @@ describe('runNonInteractive', () => { ).toHaveBeenCalled(); }); + it('should ignore duplicate provider tool-call ids across rounds', async () => { + setupMetricsMock(); + vi.mocked(mockConfig.getMaxToolCalls).mockReturnValue(1); + const toolCallEvent: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-1', + providerCallId: 'tool-1', + name: 'testTool', + args: { arg1: 'value1' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup', + }, + }; + const toolResponse: Part[] = [{ text: 'Tool response' }]; + mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse }); + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce(createStreamFromEvents([toolCallEvent])) + .mockReturnValueOnce(createStreamFromEvents([toolCallEvent])) + .mockReturnValueOnce( + createStreamFromEvents([ + { type: GeminiEventType.Content, value: 'Final answer' }, + { + type: GeminiEventType.Finished, + value: { + reason: undefined, + usageMetadata: { totalTokenCount: 10 }, + }, + }, + ]), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'Use a tool', + 'prompt-id-dup', + ); + + expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(3); + expect(mockCoreExecuteToolCall).toHaveBeenCalledTimes(1); + expect(mockGeminiClient.recordCompletedToolCall).toHaveBeenCalledTimes(1); + + const duplicateParts = mockGeminiClient.sendMessageStream.mock.calls[2][0]; + expect(duplicateParts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-1"', + ); + expect(processStdoutSpy).toHaveBeenCalledWith('Final answer\n'); + }); + + it('should ignore duplicate provider tool-call ids already present in chat history', async () => { + setupMetricsMock(); + mockGeminiClient.getHistoryFunctionResponseIds.mockReturnValue( + new Set(['tool-history']), + ); + const toolCallEvent: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-history__qwen_dup_2', + providerCallId: 'tool-history', + name: 'testTool', + args: { arg1: 'value1' }, + isClientInitiated: false, + prompt_id: 'prompt-id-history-dup', + }, + }; + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce(createStreamFromEvents([toolCallEvent])) + .mockReturnValueOnce( + createStreamFromEvents([ + { type: GeminiEventType.Content, value: 'Final answer' }, + { + type: GeminiEventType.Finished, + value: { + reason: undefined, + usageMetadata: { totalTokenCount: 10 }, + }, + }, + ]), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'Use a tool', + 'prompt-id-history-dup', + ); + + expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(2); + expect(mockCoreExecuteToolCall).not.toHaveBeenCalled(); + expect(mockGeminiClient.recordCompletedToolCall).not.toHaveBeenCalled(); + + const duplicateParts = mockGeminiClient.sendMessageStream.mock.calls[1][0]; + expect(duplicateParts[0].functionResponse?.id).toBe( + 'tool-history__qwen_dup_2', + ); + expect(duplicateParts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-history"', + ); + expect(processStdoutSpy).toHaveBeenCalledWith('Final answer\n'); + }); + + it('should execute only the first duplicate provider tool-call id in the same batch', async () => { + setupMetricsMock(); + vi.mocked(mockConfig.getMaxToolCalls).mockReturnValue(1); + const firstToolCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-1', + providerCallId: 'tool-1', + name: 'testTool', + args: { arg1: 'value1' }, + isClientInitiated: false, + prompt_id: 'prompt-id-same-batch-dup', + }, + }; + const duplicateToolCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-1', + providerCallId: 'tool-1', + name: 'testTool', + args: { arg1: 'value1' }, + isClientInitiated: false, + prompt_id: 'prompt-id-same-batch-dup', + }, + }; + mockCoreExecuteToolCall.mockResolvedValue({ + responseParts: [{ text: 'Tool response' }], + }); + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce( + createStreamFromEvents([firstToolCall, duplicateToolCall]), + ) + .mockReturnValueOnce( + createStreamFromEvents([ + { type: GeminiEventType.Content, value: 'Final answer' }, + { + type: GeminiEventType.Finished, + value: { + reason: undefined, + usageMetadata: { totalTokenCount: 10 }, + }, + }, + ]), + ); + + await runNonInteractive( + mockConfig, + mockSettings, + 'Use a tool', + 'prompt-id-same-batch-dup', + ); + + expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(2); + expect(mockCoreExecuteToolCall).toHaveBeenCalledTimes(1); + expect(mockGeminiClient.recordCompletedToolCall).toHaveBeenCalledTimes(1); + + const toolResultParts = mockGeminiClient.sendMessageStream.mock.calls[1][0]; + expect(toolResultParts).toHaveLength(2); + expect(toolResultParts[0]).toEqual({ text: 'Tool response' }); + expect(toolResultParts[1].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-1"', + ); + expect(processStdoutSpy).toHaveBeenCalledWith('Final answer\n'); + }); + it('should handle error during tool execution and should send error back to the model', async () => { setupMetricsMock(); const toolCallEvent: ServerGeminiStreamEvent = { @@ -2360,6 +2532,13 @@ describe('runNonInteractive', () => { (mockConfig.getOutputFormat as Mock).mockReturnValue('stream-json'); (mockConfig.getIncludePartialMessages as Mock).mockReturnValue(false); setupMetricsMock(); + const writes: string[] = []; + processStdoutSpy.mockImplementation((chunk: string | Uint8Array) => { + writes.push( + typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8'), + ); + return true; + }); const duplicateToolCall: ServerGeminiStreamEvent = { type: GeminiEventType.ToolCallRequest, @@ -2427,6 +2606,35 @@ describe('runNonInteractive', () => { expect.any(AbortSignal), expect.any(Object), ); + + const toolResultParts = mockGeminiClient.sendMessageStream.mock.calls[1][0]; + expect(toolResultParts).toHaveLength(2); + expect(toolResultParts[0].functionResponse?.response?.['output']).toBe( + 'first', + ); + expect(toolResultParts[1].functionResponse?.id).toBe('dup_id_0001'); + expect(toolResultParts[1].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "dup_id_0001"', + ); + + const envelopes = writes + .join('') + .split('\n') + .filter((line) => line.trim().length > 0) + .map((line) => JSON.parse(line)); + const toolResultMessages = envelopes.filter( + (env) => + env.type === 'user' && + Array.isArray(env.message?.content) && + env.message.content.some( + (block: unknown) => + typeof block === 'object' && + block !== null && + 'type' in block && + block.type === 'tool_result', + ), + ); + expect(toolResultMessages).toHaveLength(2); }); it('should execute every tool call with an empty call id in stream-json format', async () => { @@ -2777,7 +2985,7 @@ describe('runNonInteractive', () => { const leadingCall: ServerGeminiStreamEvent = { type: GeminiEventType.ToolCallRequest, value: { - callId: 'tool-leading', + callId: 'tool-structured', name: 'side_effect_tool', args: { path: '/tmp/should-not-write' }, isClientInitiated: false, @@ -2840,9 +3048,17 @@ describe('runNonInteractive', () => { // The suppressed leading tool_use must have a synthesised // tool_result event so the event log pairs every tool_use with a // tool_result on the success path. - const leadingToolResult = events.find( - (m: unknown) => extractToolResultId(m) === 'tool-leading', - ); + const leadingToolResult = events.find((m: unknown) => { + if (extractToolResultId(m) !== 'tool-structured') { + return false; + } + const content = ( + m as { + message?: { content?: Array<{ content?: string }> }; + } + )?.message?.content?.[0]?.content; + return typeof content === 'string' && content.includes('Skipped:'); + }); expect(leadingToolResult).toBeDefined(); // On the success path, the synthesised "Skipped" message must NOT // include the trailing "Re-issue this call in a separate turn" @@ -3251,6 +3467,142 @@ describe('runNonInteractive', () => { ).not.toMatch(/Skipped:/); }); + it('keeps duplicate provider responses when structured_output fails validation', async () => { + (mockConfig.getJsonSchema as Mock).mockReturnValue({ + type: 'object', + properties: { summary: { type: 'string' } }, + required: ['summary'], + }); + (mockConfig.getOutputFormat as Mock).mockReturnValue(OutputFormat.JSON); + setupMetricsMock(); + + const firstSideEffectCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-side', + providerCallId: 'tool-side', + name: 'side_effect_tool', + args: { path: '/tmp/first' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup-structured', + }, + }; + const duplicateSideEffectCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-side', + providerCallId: 'tool-side', + name: 'side_effect_tool', + args: { path: '/tmp/second' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup-structured', + }, + }; + const badStructuredCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-structured-bad', + name: 'structured_output', + args: { wrong: 'shape' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup-structured', + }, + }; + const goodStructuredCall: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-structured-good', + name: 'structured_output', + args: { summary: 'retry ok' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup-structured', + }, + }; + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce(createStreamFromEvents([firstSideEffectCall])) + .mockReturnValueOnce( + createStreamFromEvents([duplicateSideEffectCall, badStructuredCall]), + ) + .mockReturnValueOnce(createStreamFromEvents([goodStructuredCall])); + + mockCoreExecuteToolCall + .mockResolvedValueOnce({ + responseParts: [ + { + functionResponse: { + id: 'tool-side', + name: 'side_effect_tool', + response: { output: 'first side effect' }, + }, + }, + ], + }) + .mockResolvedValueOnce({ + error: new Error('args invalid'), + errorType: 'TOOL_INVALID_ARGUMENTS', + responseParts: [ + { + functionResponse: { + id: 'tool-structured-bad', + name: 'structured_output', + response: { error: 'args invalid' }, + }, + }, + ], + }) + .mockResolvedValueOnce({ + responseParts: [{ text: 'ok' }], + }); + + await runNonInteractive( + mockConfig, + mockSettings, + 'Emit structured output', + 'prompt-id-dup-structured', + ); + + const executedNames = mockCoreExecuteToolCall.mock.calls.map( + (call) => (call[1] as { name: string }).name, + ); + expect(executedNames).toEqual([ + 'side_effect_tool', + 'structured_output', + 'structured_output', + ]); + + expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(3); + const retryParts = mockGeminiClient.sendMessageStream.mock.calls[2][0] as + | Array<{ + functionResponse?: { + id?: string; + name?: string; + response?: unknown; + }; + }> + | undefined; + const retryPartsTyped = retryParts || []; + const duplicateResponse = retryPartsTyped.find((part) => + String( + (part.functionResponse?.response as { error?: unknown } | undefined) + ?.error, + ).includes('Duplicate provider tool call id "tool-side"'), + ); + const failedStructured = retryPartsTyped.find( + (part) => part.functionResponse?.id === 'tool-structured-bad', + ); + expect(duplicateResponse?.functionResponse?.id).toBe('tool-side'); + expect(duplicateResponse?.functionResponse?.name).toBe( + 'side_effect_tool', + ); + expect(failedStructured?.functionResponse?.name).toBe( + 'structured_output', + ); + expect( + JSON.stringify(failedStructured?.functionResponse?.response), + ).toContain('args invalid'); + }); + it('captures structured_output emitted from a drain-turn (queued notification)', async () => { // Main turn ends with plain text → control falls into the drain // block. A monitor notification then arrives and the model's reply diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index df69ad5fcd..b93f0482c8 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -30,6 +30,7 @@ import { TeamEventType, ApprovalMode, ToolConfirmationOutcome, + createDuplicateProviderToolCallResponse, } from '@qwen-code/qwen-code-core'; import type { Content, Part, PartListUnion } from '@google/genai'; import type { CLIUserMessage, PermissionMode } from './nonInteractive/types.js'; @@ -755,24 +756,68 @@ export async function runNonInteractive( * helper returns (main-turn → emitStructuredSuccess(); drain-turn * → return so the post-drain code emits success). */ + const handledProviderToolCallIds = + geminiClient.getHistoryFunctionResponseIds(); + const processToolCallBatch = async ( batchRequests: ToolCallRequestInfo[], setModelOverride: (override: string | undefined) => void, ): Promise => { const toolResponseParts: Part[] = []; + const structuredOutputActive = + config.getJsonSchema() && + batchRequests.some((r) => r.name === ToolNames.STRUCTURED_OUTPUT); const seenBatchCallIds = new Set(); + const duplicateBatchRequests: ToolCallRequestInfo[] = []; const uniqueBatchRequests = batchRequests.filter((request) => { if (request.callId) { if (seenBatchCallIds.has(request.callId)) { + if ( + structuredOutputActive && + request.name === ToolNames.STRUCTURED_OUTPUT + ) { + return true; + } debugLogger.debug( `Dropping duplicate non-interactive tool callId=${request.callId} name=${request.name}`, ); + duplicateBatchRequests.push(request); return false; } seenBatchCallIds.add(request.callId); } return true; }); + const respondedRequests = new Set(); + const executableBatchRequests: ToolCallRequestInfo[] = []; + const duplicatePendingResponses: Part[] = []; + + for (const requestInfo of uniqueBatchRequests) { + if (!requestInfo.providerCallId) { + executableBatchRequests.push(requestInfo); + continue; + } + + if (!handledProviderToolCallIds.has(requestInfo.providerCallId)) { + handledProviderToolCallIds.add(requestInfo.providerCallId); + executableBatchRequests.push(requestInfo); + continue; + } + + const toolResponse = + createDuplicateProviderToolCallResponse(requestInfo); + debugLogger.debug( + `[runNonInteractive] Suppressing duplicate provider tool-call id: ${requestInfo.providerCallId} (tool: ${requestInfo.name})`, + ); + respondedRequests.add(requestInfo); + adapter.emitToolResult(requestInfo, toolResponse); + duplicatePendingResponses.push(...toolResponse.responseParts); + } + + // Duplicate responses must always reach the model. They pair with a + // tool call the provider already emitted, even when structured_output + // is the only executable sibling in this batch. + toolResponseParts.push(...duplicatePendingResponses); // Pre-scan: when --json-schema is active and the model emitted // a `structured_output` call alongside other tools in the same @@ -781,21 +826,18 @@ export async function runNonInteractive( // suppress every non-structured sibling. See the multi-shape // examples in the main loop's prior comment for the // [bad/good/side-effect] permutations. - let requestsToExecute = uniqueBatchRequests; - if ( - config.getJsonSchema() && - uniqueBatchRequests.some( - (r) => r.name === ToolNames.STRUCTURED_OUTPUT, - ) - ) { - requestsToExecute = uniqueBatchRequests.filter( + let requestsToExecute = executableBatchRequests; + if (structuredOutputActive) { + requestsToExecute = executableBatchRequests.filter( (r) => r.name === ToolNames.STRUCTURED_OUTPUT, ); } - const executedCallIds = new Set(); + const executedRequests = new Set( + respondedRequests, + ); for (const requestInfo of requestsToExecute) { - executedCallIds.add(requestInfo.callId); + executedRequests.add(requestInfo); const inputFormat = typeof config.getInputFormat === 'function' @@ -919,8 +961,8 @@ export async function runNonInteractive( // emitted event log pairs every tool_use with a tool_result // AND the retry-turn payload (when reached) doesn't leave // Anthropic / OpenAI staring at unpaired tool_use blocks. - const unexecutedCalls = uniqueBatchRequests.filter( - (r) => !executedCallIds.has(r.callId), + const unexecutedCalls = executableBatchRequests.filter( + (r) => !executedRequests.has(r), ); if (unexecutedCalls.length > 0) { const skippedOutput = suppressedOutputBody( @@ -947,6 +989,13 @@ export async function runNonInteractive( } } + for (const requestInfo of duplicateBatchRequests) { + const toolResponse = + createDuplicateProviderToolCallResponse(requestInfo); + adapter.emitToolResult(requestInfo, toolResponse); + toolResponseParts.push(...toolResponse.responseParts); + } + return toolResponseParts; }; diff --git a/packages/cli/src/ui/hooks/useAtCompletion.test.ts b/packages/cli/src/ui/hooks/useAtCompletion.test.ts index fc88425c2d..97d480b0dc 100644 --- a/packages/cli/src/ui/hooks/useAtCompletion.test.ts +++ b/packages/cli/src/ui/hooks/useAtCompletion.test.ts @@ -116,12 +116,16 @@ describe('useAtCompletion', () => { expect(result.current.suggestions.length).toBeGreaterThan(0); }); - expect(result.current.suggestions.map((s) => s.value)).toEqual([ - 'src/', - 'src/components/', - 'src/index.js', - 'src/components/Button.tsx', - ]); + const suggestionValues = result.current.suggestions.map((s) => s.value); + expect(suggestionValues).toHaveLength(4); + expect(suggestionValues).toEqual( + expect.arrayContaining([ + 'src/', + 'src/components/', + 'src/components/Button.tsx', + 'src/index.js', + ]), + ); }); it('should append a trailing slash to directory paths in suggestions', async () => { diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index e2c5faf848..a53b607c24 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -1805,6 +1805,239 @@ describe('useGeminiStream', () => { }); }); + it('suppresses duplicate provider tool-call ids before TUI scheduling', async () => { + let capturedOnComplete: + | ((completedTools: TrackedToolCall[]) => Promise) + | null = null; + mockUseReactToolScheduler.mockImplementation((onComplete) => { + capturedOnComplete ??= onComplete; + return [[], mockScheduleToolCalls, mockMarkToolsAsSubmitted]; + }); + + mockSendMessageStream + .mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'tool-dup', + providerCallId: 'tool-dup', + name: 'shell', + args: { command: 'echo first' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-dup', + }, + }; + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'tool-dup', + providerCallId: 'tool-dup', + name: 'shell', + args: { command: 'echo second' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-dup', + }, + }; + })(), + ) + .mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.Content, + value: 'done', + }; + yield { + type: ServerGeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 1 } }, + }; + })(), + ); + + const client = new MockedGeminiClientClass(mockConfig); + const { result } = renderHook(() => + useGeminiStream( + client, + [], + mockAddItem, + mockConfig, + mockLoadedSettings, + mockOnDebugMessage, + mockHandleSlashCommand, + false, + () => 'vscode' as EditorType, + () => {}, + () => Promise.resolve(), + false, + () => {}, + () => {}, + () => {}, + () => {}, + 80, + 24, + ), + ); + + await act(async () => { + await result.current.submitQuery('run shell'); + }); + + await waitFor(() => { + expect(result.current.streamingState).toBe(StreamingState.Idle); + }); + + expect(mockScheduleToolCalls).toHaveBeenCalledTimes(1); + expect(mockScheduleToolCalls.mock.calls[0][0]).toEqual([ + expect.objectContaining({ + callId: 'tool-dup', + providerCallId: 'tool-dup', + args: { command: 'echo first' }, + }), + ]); + + const completedToolCall = { + request: { + callId: 'tool-dup', + providerCallId: 'tool-dup', + name: 'shell', + args: { command: 'echo first' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-dup', + }, + status: 'success', + responseSubmittedToGemini: false, + response: { + callId: 'tool-dup', + responseParts: [ + { + functionResponse: { + id: 'tool-dup', + name: 'shell', + response: { output: 'first' }, + }, + }, + ], + resultDisplay: 'first', + error: undefined, + errorType: undefined, + }, + tool: { + name: 'shell', + displayName: 'Shell', + description: 'Run a command', + build: vi.fn(), + } as any, + invocation: { + getDescription: () => 'echo first', + } as unknown as AnyToolInvocation, + } as unknown as TrackedCompletedToolCall; + + await act(async () => { + if (capturedOnComplete) { + await capturedOnComplete([completedToolCall]); + } + }); + + await waitFor(() => { + expect(mockSendMessageStream).toHaveBeenCalledTimes(2); + }); + const toolResultParts = mockSendMessageStream.mock.calls[1][0] as Part[]; + expect(toolResultParts).toHaveLength(2); + expect(toolResultParts[0].functionResponse?.response?.['output']).toBe( + 'first', + ); + expect(toolResultParts[1].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-dup"', + ); + expect(client.recordCompletedToolCall).toHaveBeenCalledTimes(1); + }); + + it('submits a synthetic response for history-paired duplicate provider ids without scheduling', async () => { + const client = new MockedGeminiClientClass(mockConfig); + client.getHistoryFunctionResponseIds = vi + .fn() + .mockReturnValue(new Set(['tool-history'])); + + mockSendMessageStream + .mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'tool-history', + providerCallId: 'tool-history', + name: 'shell', + args: { command: 'echo duplicate' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-history', + }, + }; + })(), + ) + .mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 1 } }, + }; + })(), + ); + + const { result } = renderTestHook([], client); + + await act(async () => { + await result.current.submitQuery('run shell'); + }); + + expect(mockScheduleToolCalls).not.toHaveBeenCalled(); + expect(mockSendMessageStream).toHaveBeenCalledTimes(2); + const toolResultParts = mockSendMessageStream.mock.calls[1][0] as Part[]; + expect(toolResultParts[0].functionResponse?.id).toBe('tool-history'); + expect(toolResultParts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-history"', + ); + expect(client.recordCompletedToolCall).not.toHaveBeenCalled(); + }); + + it('does not deduplicate tool calls without provider ids in the TUI stream', async () => { + mockSendMessageStream.mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'generated-1', + name: 'shell', + args: { command: 'pwd' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-no-provider', + }, + }; + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'generated-2', + name: 'shell', + args: { command: 'pwd' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-no-provider', + }, + }; + })(), + ); + + const { result } = renderTestHook(); + + await act(async () => { + await result.current.submitQuery('run shell twice'); + }); + + expect(mockScheduleToolCalls).toHaveBeenCalledTimes(1); + expect(mockScheduleToolCalls.mock.calls[0][0]).toEqual([ + expect.objectContaining({ callId: 'generated-1' }), + expect.objectContaining({ callId: 'generated-2' }), + ]); + }); + it('drops a late tool result whose callId is already paired in chat.history (Race A dedup)', async () => { // Race A repro: the chat-internal repair pass already synthesized a // functionResponse for this callId on the Retry push (because the diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 9b89348ce0..90bd823cab 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -56,6 +56,7 @@ import { activeGoalEquals, setActiveGoal, clearActiveGoal, + createDuplicateProviderToolCallResponse, } from '@qwen-code/qwen-code-core'; import { type Part, type PartListUnion, FinishReason } from '@google/genai'; import type { @@ -103,6 +104,12 @@ const MID_TURN_AT_COMMAND_RESOLVE_TIMEOUT_MS = 10_000; const MID_TURN_AT_COMMAND_RESOLVE_TIMEOUT_MESSAGE = 'Mid-turn @ command resolution timed out'; +interface PendingDuplicateToolResponses { + executableCallIds: Set; + promptId: string | undefined; + responseParts: Part[]; +} + /** * Pull the assistant's most recent visible text from the UI history. Used as * an intent prefix for tool-use summary generation so the summarizer knows @@ -424,6 +431,14 @@ export const useGeminiStream = ( const processedMemoryToolsRef = useRef>(new Set()); const submitPromptOnCompleteRef = useRef<(() => Promise) | null>(null); const modelOverrideRef = useRef(undefined); + const handledProviderToolCallIdsRef = useRef>(new Set()); + const pendingDuplicateToolResponsesRef = useRef< + PendingDuplicateToolResponses[] + >([]); + const immediateDuplicateToolResponsesRef = useRef<{ + promptId: string | undefined; + responseParts: Part[]; + } | null>(null); // --- Real-time token display --- // Accumulates output character count across the whole turn (not per API call). // Uses a ref to avoid re-renders on every text_delta. @@ -1708,7 +1723,59 @@ export const useGeminiStream = ( } dualOutput?.finalizeAssistantMessage(); if (toolCallRequests.length > 0 && !signal.aborted) { - scheduleToolCalls(toolCallRequests, signal); + const executableToolCallRequests: ToolCallRequestInfo[] = []; + const duplicateResponseParts: Part[] = []; + let duplicatePromptId: string | undefined; + const historyCallIdsWithResponse: Set = geminiClient + ? geminiClient.getHistoryFunctionResponseIds() + : new Set(); + + for (const request of toolCallRequests) { + const providerCallId = request.providerCallId; + if (!providerCallId) { + executableToolCallRequests.push(request); + continue; + } + + if ( + handledProviderToolCallIdsRef.current.has(providerCallId) || + historyCallIdsWithResponse.has(providerCallId) + ) { + const response = createDuplicateProviderToolCallResponse(request); + debugLogger.debug( + `[processGeminiStreamEvents] Suppressing duplicate provider tool-call id: ${providerCallId} (tool: ${request.name})`, + ); + dualOutput?.emitToolResult(request, response); + duplicateResponseParts.push(...response.responseParts); + duplicatePromptId ??= request.prompt_id; + continue; + } + + handledProviderToolCallIdsRef.current.add(providerCallId); + executableToolCallRequests.push(request); + } + + if (duplicateResponseParts.length > 0) { + if (executableToolCallRequests.length > 0) { + pendingDuplicateToolResponsesRef.current.push({ + executableCallIds: new Set( + executableToolCallRequests.map((request) => request.callId), + ), + promptId: + duplicatePromptId ?? executableToolCallRequests[0]?.prompt_id, + responseParts: duplicateResponseParts, + }); + } else { + immediateDuplicateToolResponsesRef.current = { + promptId: duplicatePromptId, + responseParts: duplicateResponseParts, + }; + } + } + + if (executableToolCallRequests.length > 0) { + scheduleToolCalls(executableToolCallRequests, signal); + } } return StreamProcessingStatus.Completed; }, @@ -1718,6 +1785,7 @@ export const useGeminiStream = ( handleUserCancelledEvent, handleErrorEvent, scheduleToolCalls, + geminiClient, handleChatCompressionEvent, handleFinishedEvent, handleMaxSessionTurnsEvent, @@ -1789,6 +1857,9 @@ export const useGeminiStream = ( ) { lastTurnUserItemRef.current = null; turnSawContentEventRef.current = false; + handledProviderToolCallIdsRef.current.clear(); + pendingDuplicateToolResponsesRef.current = []; + immediateDuplicateToolResponsesRef.current = null; } const userMessageTimestamp = Date.now(); @@ -1959,6 +2030,17 @@ export const useGeminiStream = ( addItem(pendingHistoryItemRef.current, userMessageTimestamp); setPendingHistoryItem(null); } + + const immediateDuplicateToolResponses = + immediateDuplicateToolResponsesRef.current; + if (immediateDuplicateToolResponses) { + immediateDuplicateToolResponsesRef.current = null; + await submitQuery( + immediateDuplicateToolResponses.responseParts, + SendMessageType.ToolResult, + immediateDuplicateToolResponses.promptId, + ); + } // Only clear auto-retry countdown errors (those with an active timer). // Do NOT clear static error+hint from handleErrorEvent — those should // remain visible until the user presses Ctrl+Y to retry or starts @@ -2279,6 +2361,26 @@ export const useGeminiStream = ( !t.request.isClientInitiated && !historyCallIdsWithResponse.has(t.request.callId), ); + const completedCallIds = new Set( + completedAndReadyToSubmitTools.map( + (toolCall) => toolCall.request.callId, + ), + ); + const readyDuplicateBatches: PendingDuplicateToolResponses[] = []; + pendingDuplicateToolResponsesRef.current = + pendingDuplicateToolResponsesRef.current.filter((batch) => { + const isReady = [...batch.executableCallIds].some((callId) => + completedCallIds.has(callId), + ); + if (isReady) { + readyDuplicateBatches.push(batch); + } + return !isReady; + }); + const pendingDuplicateResponseParts = readyDuplicateBatches.flatMap( + (batch) => batch.responseParts, + ); + const pendingDuplicatePromptId = readyDuplicateBatches[0]?.promptId; for (const toolCall of geminiTools) { geminiClient?.recordCompletedToolCall( @@ -2287,7 +2389,10 @@ export const useGeminiStream = ( ); } - if (geminiTools.length === 0) { + if ( + geminiTools.length === 0 && + pendingDuplicateResponseParts.length === 0 + ) { return; } @@ -2306,7 +2411,7 @@ export const useGeminiStream = ( (tc) => tc.status === 'cancelled', ); - if (allToolsCancelled) { + if (allToolsCancelled && pendingDuplicateResponseParts.length === 0) { if (geminiClient) { // We need to manually add the function responses to the history // so the model knows the tools were cancelled. @@ -2332,6 +2437,7 @@ export const useGeminiStream = ( const responsesToSend: Part[] = geminiTools.flatMap( (toolCall) => toolCall.response.responseParts, ); + responsesToSend.push(...pendingDuplicateResponseParts); const callIdsToMarkAsSubmitted = geminiTools.map( (toolCall) => toolCall.request.callId, ); @@ -2339,6 +2445,7 @@ export const useGeminiStream = ( const prompt_ids = geminiTools.map( (toolCall) => toolCall.request.prompt_id, ); + const promptId = prompt_ids[0] ?? pendingDuplicatePromptId; // Persist model override from skill tool results (last one wins). // Uses `in` so that undefined (from inherit/no-model skills) clears a @@ -2585,7 +2692,7 @@ export const useGeminiStream = ( return; } - submitQuery(responsesToSend, SendMessageType.ToolResult, prompt_ids[0]); + submitQuery(responsesToSend, SendMessageType.ToolResult, promptId); }, [ submitQuery, diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index f2debda7d4..e6cdf9b5c7 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -28,7 +28,10 @@ import { runWithRuntimeContentGenerator, type RuntimeContentGeneratorView, } from './agent-context.js'; -import { type ToolCallRequestInfo } from '../../core/turn.js'; +import { + createDuplicateProviderToolCallResponse, + type ToolCallRequestInfo, +} from '../../core/turn.js'; import { CoreToolScheduler, type ToolCall, @@ -51,7 +54,10 @@ import type { GenerateContentResponseUsageMetadata, } from '@google/genai'; import { GeminiChat } from '../../core/geminiChat.js'; -import { dedupeToolCallsById } from '../../core/toolCallIdUtils.js'; +import { + dedupeToolCallsById, + getProviderToolCallId, +} from '../../core/toolCallIdUtils.js'; import type { PromptConfig, ModelConfig, @@ -655,6 +661,7 @@ export class AgentCore { let turnCounter = 0; let finalText = ''; let terminateMode: AgentTerminateMode | null = null; + const handledProviderToolCallIds = chat.getHistoryFunctionResponseIds(); let stickyMaxOutputTokens: number | undefined; while (true) { @@ -823,6 +830,7 @@ export class AgentCore { toolsList, currentResponseId, wasOutputTruncated, + handledProviderToolCallIds, ); const externalInputs = this.drainExternalInputs(options); @@ -1082,6 +1090,48 @@ export class AgentCore { // ─── Tool Execution ─────────────────────────────────────── + private emitSyntheticToolError(params: { + callId: string; + name: string; + args: Record; + errorMessage: string; + responseParts: Part[]; + resultDisplay: ToolResultDisplay | undefined; + currentRound: number; + durationMs?: number; + }): void { + this.eventEmitter?.emit(AgentEventType.TOOL_CALL, { + subagentId: this.subagentId, + round: params.currentRound, + callId: params.callId, + name: params.name, + args: params.args, + description: params.errorMessage, + isOutputMarkdown: false, + timestamp: Date.now(), + } as AgentToolCallEvent); + + this.eventEmitter?.emit(AgentEventType.TOOL_RESULT, { + subagentId: this.subagentId, + round: params.currentRound, + callId: params.callId, + name: params.name, + success: false, + error: params.errorMessage, + responseParts: params.responseParts, + resultDisplay: params.resultDisplay, + durationMs: params.durationMs ?? 0, + timestamp: Date.now(), + } as AgentToolResultEvent); + + this.recordToolCallStats( + params.name, + false, + params.durationMs ?? 0, + params.errorMessage, + ); + } + /** * Processes a list of function calls via CoreToolScheduler. * @@ -1098,6 +1148,7 @@ export class AgentCore { toolsList: FunctionDeclaration[], responseId?: string, wasOutputTruncated = false, + handledProviderToolCallIds = new Set(), ): Promise { const toolResponseParts: Part[] = []; const uniqueFunctionCalls = dedupeToolCallsById(functionCalls); @@ -1107,26 +1158,15 @@ export class AgentCore { // Filter unauthorized tool calls before scheduling const authorizedCalls: FunctionCall[] = []; + let duplicateEventIndex = 0; for (const fc of uniqueFunctionCalls) { const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const providerCallId = getProviderToolCallId(fc) ?? fc.id; + const toolName = String(fc.name); + const args = (fc.args ?? {}) as Record; if (!allowedToolNames.has(fc.name)) { - const toolName = String(fc.name); const errorMessage = `Tool "${toolName}" not found. Tools must use the exact names provided.`; - - // Emit TOOL_CALL event for visibility - this.eventEmitter?.emit(AgentEventType.TOOL_CALL, { - subagentId: this.subagentId, - round: currentRound, - callId, - name: toolName, - args: fc.args ?? {}, - description: `Tool "${toolName}" not found`, - isOutputMarkdown: false, - timestamp: Date.now(), - } as AgentToolCallEvent); - - // Build function response part (used for both event and LLM) const functionResponsePart = { functionResponse: { id: callId, @@ -1135,27 +1175,59 @@ export class AgentCore { }, }; - // Emit TOOL_RESULT event with error - this.eventEmitter?.emit(AgentEventType.TOOL_RESULT, { - subagentId: this.subagentId, - round: currentRound, + this.emitSyntheticToolError({ callId, name: toolName, - success: false, - error: errorMessage, + args, + errorMessage, responseParts: [functionResponsePart], resultDisplay: errorMessage, - durationMs: 0, - timestamp: Date.now(), - } as AgentToolResultEvent); + currentRound, + }); - // Record blocked tool call in stats - this.recordToolCallStats(toolName, false, 0, errorMessage); - - // Add function response for LLM toolResponseParts.push(functionResponsePart); continue; } + + if (providerCallId) { + if (handledProviderToolCallIds.has(providerCallId)) { + const request: ToolCallRequestInfo = { + callId, + providerCallId, + name: toolName, + args, + isClientInitiated: true, + prompt_id: promptId, + response_id: responseId, + wasOutputTruncated, + }; + const response = createDuplicateProviderToolCallResponse(request); + const errorMessage = + response.error?.message ?? + 'Duplicate provider tool call was ignored.'; + const eventCallId = `${callId}:duplicate:${currentRound}:${duplicateEventIndex++}`; + + this.runtimeContext + .getDebugLogger() + ?.debug( + `[processFunctionCalls] Suppressing duplicate provider tool-call id: ${providerCallId} (tool: ${toolName}, round: ${currentRound})`, + ); + + this.emitSyntheticToolError({ + callId: eventCallId, + name: toolName, + args, + errorMessage, + responseParts: response.responseParts, + resultDisplay: response.resultDisplay, + currentRound, + }); + + toolResponseParts.push(...response.responseParts); + continue; + } + handledProviderToolCallIds.add(providerCallId); + } authorizedCalls.push(fc); } @@ -1344,9 +1416,11 @@ export class AgentCore { const requests: ToolCallRequestInfo[] = authorizedCalls.map((fc) => { const toolName = String(fc.name || 'unknown'); const callId = fc.id ?? `${fc.name}-${Date.now()}`; + const providerCallId = getProviderToolCallId(fc) ?? fc.id; const args = (fc.args ?? {}) as Record; const request: ToolCallRequestInfo = { callId, + ...(providerCallId ? { providerCallId } : {}), name: toolName, args, isClientInitiated: true, diff --git a/packages/core/src/agents/runtime/agent-headless.test.ts b/packages/core/src/agents/runtime/agent-headless.test.ts index a443f13733..aba104d1d6 100644 --- a/packages/core/src/agents/runtime/agent-headless.test.ts +++ b/packages/core/src/agents/runtime/agent-headless.test.ts @@ -30,6 +30,7 @@ import { AuthType, } from '../../core/contentGenerator.js'; import { GeminiChat } from '../../core/geminiChat.js'; +import { normalizeModelToolCallIds } from '../../core/toolCallIdUtils.js'; import { executeToolCall } from '../../core/nonInteractiveToolExecutor.js'; import { getInitialChatHistory } from '../../utils/environmentContext.js'; import type { ToolRegistry } from '../../tools/tool-registry.js'; @@ -246,6 +247,7 @@ describe('subagent.ts', () => { describe('AgentHeadless', () => { let mockSendMessageStream: Mock; + let mockGetHistoryFunctionResponseIds: Mock; const defaultModelConfig: ModelConfig = { model: 'qwen3-coder-plus', @@ -277,11 +279,13 @@ describe('subagent.ts', () => { }); mockSendMessageStream = vi.fn(); + mockGetHistoryFunctionResponseIds = vi.fn(() => new Set()); vi.mocked(GeminiChat).mockImplementation( () => ({ sendMessageStream: mockSendMessageStream, setLastPromptTokenCount: vi.fn(), + getHistoryFunctionResponseIds: mockGetHistoryFunctionResponseIds, }) as unknown as GeminiChat, ); @@ -1031,6 +1035,219 @@ describe('subagent.ts', () => { expect(scope.getTerminateMode()).toBe(AgentTerminateMode.GOAL); }); + it('should ignore duplicate provider tool-call ids across rounds', async () => { + const listFilesToolDef: FunctionDeclaration = { + name: 'list_files', + description: 'Lists files', + parameters: { type: Type.OBJECT, properties: {} }, + }; + + const { config } = await createMockConfig({ + getFunctionDeclarationsFiltered: vi + .fn() + .mockReturnValue([listFilesToolDef]), + getTool: vi.fn().mockReturnValue(undefined), + }); + const toolConfig: ToolConfig = { tools: ['list_files'] }; + const [duplicateNormalizedPart] = normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'call_1', + name: 'list_files', + args: { path: '.' }, + }, + }, + ], + new Set(['call_1']), + new Set(), + ); + + mockSendMessageStream.mockImplementation( + createMockStream([ + [ + { + id: 'call_1', + name: 'list_files', + args: { path: '.' }, + }, + ], + [duplicateNormalizedPart!.functionCall!], + 'stop', + ]), + ); + + const listFilesInvocation = { + params: { path: '.' }, + getDescription: vi.fn().mockReturnValue('List files'), + toolLocations: vi.fn().mockReturnValue([]), + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + execute: vi.fn().mockResolvedValue({ + llmContent: 'file1.txt\nfile2.ts', + returnDisplay: 'Listed 2 files', + }), + }; + const listFilesTool = { + name: 'list_files', + displayName: 'List Files', + description: 'List files in directory', + kind: 'READ' as const, + schema: listFilesToolDef, + build: vi.fn().mockImplementation(() => listFilesInvocation), + canUpdateOutput: false, + isOutputMarkdown: true, + } as unknown as AnyDeclarativeTool; + vi.mocked( + (config.getToolRegistry() as unknown as ToolRegistry).getTool, + ).mockImplementation((name: string) => + name === 'list_files' ? listFilesTool : undefined, + ); + + const toolCallEvents: AgentToolCallEvent[] = []; + const toolResultEvents: AgentToolResultEvent[] = []; + const eventEmitter = new AgentEventEmitter(); + eventEmitter.on(AgentEventType.TOOL_CALL, (event: unknown) => { + toolCallEvents.push(event as AgentToolCallEvent); + }); + eventEmitter.on(AgentEventType.TOOL_RESULT, (event: unknown) => { + toolResultEvents.push(event as AgentToolResultEvent); + }); + + const scope = await AgentHeadless.create( + 'test-agent', + config, + promptConfig, + defaultModelConfig, + defaultRunConfig, + toolConfig, + eventEmitter, + ); + + await scope.execute(new ContextState()); + + expect(listFilesInvocation.execute).toHaveBeenCalledTimes(1); + expect(toolCallEvents).toHaveLength(2); + expect(toolResultEvents).toHaveLength(2); + expect(toolCallEvents[0].callId).toBe('call_1'); + expect(toolResultEvents[0].callId).toBe('call_1'); + expect(toolCallEvents[1].callId).toMatch( + /^call_1__qwen_dup_2:duplicate:/, + ); + expect(toolResultEvents[1].callId).toBe(toolCallEvents[1].callId); + expect(toolResultEvents[1].error).toContain( + 'Duplicate provider tool call id "call_1"', + ); + + const thirdCallArgs = mockSendMessageStream.mock.calls[2][1]; + const parts = thirdCallArgs.message as Part[]; + expect(parts[0].functionResponse?.id).toBe('call_1__qwen_dup_2'); + expect(parts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "call_1"', + ); + expect(scope.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + + it('should ignore duplicate provider tool-call ids already present in chat history', async () => { + const listFilesToolDef: FunctionDeclaration = { + name: 'list_files', + description: 'Lists files', + parameters: { type: Type.OBJECT, properties: {} }, + }; + + const { config } = await createMockConfig({ + getFunctionDeclarationsFiltered: vi + .fn() + .mockReturnValue([listFilesToolDef]), + getTool: vi.fn().mockReturnValue(undefined), + }); + const toolConfig: ToolConfig = { tools: ['list_files'] }; + const [duplicateNormalizedPart] = normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'call_1', + name: 'list_files', + args: { path: '.' }, + }, + }, + ], + new Set(['call_1']), + new Set(), + ); + mockGetHistoryFunctionResponseIds.mockReturnValue(new Set(['call_1'])); + + mockSendMessageStream.mockImplementation( + createMockStream([[duplicateNormalizedPart!.functionCall!], 'stop']), + ); + + const listFilesInvocation = { + params: { path: '.' }, + getDescription: vi.fn().mockReturnValue('List files'), + toolLocations: vi.fn().mockReturnValue([]), + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + execute: vi.fn().mockResolvedValue({ + llmContent: 'file1.txt\nfile2.ts', + returnDisplay: 'Listed 2 files', + }), + }; + const listFilesTool = { + name: 'list_files', + displayName: 'List Files', + description: 'List files in directory', + kind: 'READ' as const, + schema: listFilesToolDef, + build: vi.fn().mockImplementation(() => listFilesInvocation), + canUpdateOutput: false, + isOutputMarkdown: true, + } as unknown as AnyDeclarativeTool; + vi.mocked( + (config.getToolRegistry() as unknown as ToolRegistry).getTool, + ).mockImplementation((name: string) => + name === 'list_files' ? listFilesTool : undefined, + ); + + const toolCallEvents: AgentToolCallEvent[] = []; + const toolResultEvents: AgentToolResultEvent[] = []; + const eventEmitter = new AgentEventEmitter(); + eventEmitter.on(AgentEventType.TOOL_CALL, (event: unknown) => { + toolCallEvents.push(event as AgentToolCallEvent); + }); + eventEmitter.on(AgentEventType.TOOL_RESULT, (event: unknown) => { + toolResultEvents.push(event as AgentToolResultEvent); + }); + + const scope = await AgentHeadless.create( + 'test-agent', + config, + promptConfig, + defaultModelConfig, + defaultRunConfig, + toolConfig, + eventEmitter, + ); + + await scope.execute(new ContextState()); + + expect(listFilesInvocation.execute).not.toHaveBeenCalled(); + expect(toolCallEvents).toHaveLength(1); + expect(toolResultEvents).toHaveLength(1); + expect(toolCallEvents[0].callId).toMatch( + /^call_1__qwen_dup_2:duplicate:/, + ); + expect(toolResultEvents[0].callId).toBe(toolCallEvents[0].callId); + expect(toolResultEvents[0].error).toContain( + 'Duplicate provider tool call id "call_1"', + ); + + const secondCallArgs = mockSendMessageStream.mock.calls[1][1]; + const parts = secondCallArgs.message as Part[]; + expect(parts[0].functionResponse?.id).toBe('call_1__qwen_dup_2'); + expect(parts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "call_1"', + ); + expect(scope.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('should execute only the first duplicate functionCall id in one model turn', async () => { const listFilesToolDef: FunctionDeclaration = { name: 'list_files', @@ -1110,6 +1327,92 @@ describe('subagent.ts', () => { .filter((id): id is string => Boolean(id)), ).toEqual(['dup_id_0001']); }); + + it('should report unauthorized tool names before duplicate provider ids', async () => { + const listFilesToolDef: FunctionDeclaration = { + name: 'list_files', + description: 'Lists files', + parameters: { type: Type.OBJECT, properties: {} }, + }; + + const { config } = await createMockConfig({ + getFunctionDeclarationsFiltered: vi + .fn() + .mockReturnValue([listFilesToolDef]), + getTool: vi.fn().mockReturnValue(undefined), + }); + const toolConfig: ToolConfig = { tools: ['list_files'] }; + + mockSendMessageStream.mockImplementation( + createMockStream([ + [ + { + id: 'call_reused', + name: 'list_files', + args: { path: '.' }, + }, + ], + [ + { + id: 'call_reused', + name: 'write_file', + args: { path: 'x.txt', content: 'x' }, + }, + ], + 'stop', + ]), + ); + + const listFilesInvocation = { + params: { path: '.' }, + getDescription: vi.fn().mockReturnValue('List files'), + toolLocations: vi.fn().mockReturnValue([]), + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + execute: vi.fn().mockResolvedValue({ + llmContent: 'file1.txt\nfile2.ts', + returnDisplay: 'Listed 2 files', + }), + }; + const listFilesTool = { + name: 'list_files', + displayName: 'List Files', + description: 'List files in directory', + kind: 'READ' as const, + schema: listFilesToolDef, + build: vi.fn().mockImplementation(() => listFilesInvocation), + canUpdateOutput: false, + isOutputMarkdown: true, + } as unknown as AnyDeclarativeTool; + vi.mocked( + (config.getToolRegistry() as unknown as ToolRegistry).getTool, + ).mockImplementation((name: string) => + name === 'list_files' ? listFilesTool : undefined, + ); + + const scope = await AgentHeadless.create( + 'test-agent', + config, + promptConfig, + defaultModelConfig, + defaultRunConfig, + toolConfig, + ); + + await scope.execute(new ContextState()); + + expect(listFilesInvocation.execute).toHaveBeenCalledTimes(1); + const thirdCallArgs = mockSendMessageStream.mock.calls[2][1]; + const parts = thirdCallArgs.message as Part[]; + expect(parts[0].functionResponse?.id).toBe('call_reused'); + expect(parts[0].functionResponse?.name).toBe('write_file'); + expect(parts[0].functionResponse?.response?.['error']).toContain( + 'Tool "write_file" not found', + ); + expect(parts[0].functionResponse?.response?.['error']).not.toContain( + 'Duplicate provider tool call id', + ); + expect(scope.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); }); describe('execute - Termination and Recovery', () => { @@ -1257,6 +1560,7 @@ describe('subagent.ts', () => { ({ sendMessageStream: mockSendMessageStream, setLastPromptTokenCount: vi.fn(), + getHistoryFunctionResponseIds: vi.fn(() => new Set()), }) as unknown as GeminiChat, ); @@ -1297,6 +1601,7 @@ describe('subagent.ts', () => { ({ sendMessageStream: mockSendMessageStream, setLastPromptTokenCount: vi.fn(), + getHistoryFunctionResponseIds: vi.fn(() => new Set()), }) as unknown as GeminiChat, ); @@ -1362,6 +1667,7 @@ describe('subagent.ts', () => { ({ sendMessageStream: mockSendMessageStream, setLastPromptTokenCount: vi.fn(), + getHistoryFunctionResponseIds: vi.fn(() => new Set()), }) as unknown as GeminiChat, ); diff --git a/packages/core/src/core/toolCallIdUtils.test.ts b/packages/core/src/core/toolCallIdUtils.test.ts index e72dcdbde4..1f51aa347b 100644 --- a/packages/core/src/core/toolCallIdUtils.test.ts +++ b/packages/core/src/core/toolCallIdUtils.test.ts @@ -9,6 +9,7 @@ import type { Content, Part } from '@google/genai'; import { collectToolCallIdsFromHistory, dedupeToolCallsById, + getProviderToolCallId, normalizeModelToolCallIds, } from './toolCallIdUtils.js'; @@ -72,6 +73,9 @@ describe('toolCallIdUtils', () => { }, { text: 'done' }, ]); + expect(getProviderToolCallId(normalized[0]!.functionCall!)).toBe( + 'dup_id_0001', + ); expect(seenIds.has('dup_id_0001__qwen_dup_2')).toBe(true); }); @@ -91,6 +95,9 @@ describe('toolCallIdUtils', () => { 'call_qwen_2', 'call_qwen_3', ]); + expect( + normalized.map((part) => getProviderToolCallId(part.functionCall!)), + ).toEqual([undefined, undefined]); }); it('deduplicates direct function call batches by id', () => { diff --git a/packages/core/src/core/toolCallIdUtils.ts b/packages/core/src/core/toolCallIdUtils.ts index 23d51e633b..886db5345c 100644 --- a/packages/core/src/core/toolCallIdUtils.ts +++ b/packages/core/src/core/toolCallIdUtils.ts @@ -9,8 +9,13 @@ import { createDebugLogger } from '../utils/debugLogger.js'; const DUPLICATE_ID_SUFFIX = '__qwen_dup_'; const GENERATED_ID_PREFIX = 'call_qwen_'; +const PROVIDER_TOOL_CALL_ID = Symbol('providerToolCallId'); const debugLogger = createDebugLogger('TOOL_CALL_IDS'); +type FunctionCallWithProviderId = FunctionCall & { + [PROVIDER_TOOL_CALL_ID]?: string; +}; + function addId(ids: Set, id: string | undefined): void { if (id) { ids.add(id); @@ -87,18 +92,32 @@ export function normalizeModelToolCallIds( } usedIds.add(id); + const normalizedFunctionCall: FunctionCallWithProviderId = { + ...functionCall, + id, + }; + if (rawId) { + Object.defineProperty(normalizedFunctionCall, PROVIDER_TOOL_CALL_ID, { + value: rawId, + enumerable: false, + }); + } + normalized.push({ ...part, - functionCall: { - ...functionCall, - id, - }, + functionCall: normalizedFunctionCall, }); } return normalized; } +export function getProviderToolCallId( + functionCall: FunctionCall, +): string | undefined { + return (functionCall as FunctionCallWithProviderId)[PROVIDER_TOOL_CALL_ID]; +} + export function dedupeToolCallsById>( functionCalls: readonly T[], ): T[] { diff --git a/packages/core/src/core/turn.test.ts b/packages/core/src/core/turn.test.ts index b6323631a7..f6a7c3b86f 100644 --- a/packages/core/src/core/turn.test.ts +++ b/packages/core/src/core/turn.test.ts @@ -14,6 +14,7 @@ import type { GenerateContentResponse, Part, Content } from '@google/genai'; import { reportError } from '../utils/errorReporting.js'; import type { GeminiChat } from './geminiChat.js'; import { StreamEventType } from './geminiChat.js'; +import { normalizeModelToolCallIds } from './toolCallIdUtils.js'; const mockSendMessageStream = vi.fn(); const mockGetHistory = vi.fn(); @@ -387,6 +388,93 @@ describe('Turn', () => { }); }); + it('should preserve provider tool-call ids separately from generated call ids', async () => { + const mockResponseStream = (async function* () { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [], + functionCalls: [ + { id: 'fc1', name: 'tool1', args: { arg1: 'val1' } }, + { name: 'tool2', args: { arg2: 'val2' } }, + ], + }, + }; + })(); + mockSendMessageStream.mockResolvedValue(mockResponseStream); + + const events = []; + for await (const event of turn.run( + 'test-model', + [{ text: 'Test provider ids' }], + new AbortController().signal, + )) { + events.push(event); + } + + expect(events.length).toBe(2); + + const event1 = events[0] as ServerGeminiToolCallRequestEvent; + expect(event1.value).toMatchObject({ + callId: 'fc1', + providerCallId: 'fc1', + name: 'tool1', + args: { arg1: 'val1' }, + }); + + const event2 = events[1] as ServerGeminiToolCallRequestEvent; + expect(event2.value.callId).toMatch(/^tool2-/); + expect(event2.value.providerCallId).toBeUndefined(); + expect(event2.value).toMatchObject({ + name: 'tool2', + args: { arg2: 'val2' }, + }); + }); + + it('should preserve raw provider ids for suffixed function call ids', async () => { + const [normalizedPart] = normalizeModelToolCallIds( + [ + { + functionCall: { + id: 'fc1', + name: 'tool1', + args: { arg1: 'val1' }, + }, + }, + ], + new Set(['fc1']), + new Set(), + ); + const mockResponseStream = (async function* () { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [], + functionCalls: [normalizedPart!.functionCall], + }, + }; + })(); + mockSendMessageStream.mockResolvedValue(mockResponseStream); + + const events = []; + for await (const event of turn.run( + 'test-model', + [{ text: 'Test suffixed provider id' }], + new AbortController().signal, + )) { + events.push(event); + } + + expect(events.length).toBe(1); + const event = events[0] as ServerGeminiToolCallRequestEvent; + expect(event.value).toMatchObject({ + callId: 'fc1__qwen_dup_2', + providerCallId: 'fc1', + name: 'tool1', + args: { arg1: 'val1' }, + }); + }); + it('should yield finished event when response has finish reason', async () => { const mockResponseStream = (async function* () { yield { diff --git a/packages/core/src/core/turn.ts b/packages/core/src/core/turn.ts index ae89e4707d..5038b919ef 100644 --- a/packages/core/src/core/turn.ts +++ b/packages/core/src/core/turn.ts @@ -18,7 +18,7 @@ import type { ToolResult, ToolResultDisplay, } from '../tools/tools.js'; -import type { ToolErrorType } from '../tools/tool-error.js'; +import { ToolErrorType } from '../tools/tool-error.js'; import { getResponseText } from '../utils/partUtils.js'; import { reportError } from '../utils/errorReporting.js'; import { @@ -35,6 +35,7 @@ import { } from '../utils/thoughtUtils.js'; import type { LoopType } from '../telemetry/types.js'; import type { ActiveGoal } from '../goals/activeGoalStore.js'; +import { getProviderToolCallId } from './toolCallIdUtils.js'; // Define a structure for tools passed to the server export interface ServerTool { @@ -98,6 +99,11 @@ export interface GeminiFinishedEventValue { export interface ToolCallRequestInfo { callId: string; + /** + * Original tool-call id emitted by the provider/model. When present, this is + * the idempotency key for suppressing duplicate provider tool calls. + */ + providerCallId?: string; name: string; args: Record; isClientInitiated: boolean; @@ -117,6 +123,32 @@ export interface ToolCallResponseInfo { modelOverride?: string; } +function duplicateProviderToolCallMessage(providerCallId: string): string { + return `Duplicate provider tool call id "${providerCallId}" was already handled. The duplicate tool call was ignored and not executed again.`; +} + +export function createDuplicateProviderToolCallResponse( + request: ToolCallRequestInfo, +): ToolCallResponseInfo { + const providerCallId = request.providerCallId ?? request.callId; + const message = duplicateProviderToolCallMessage(providerCallId); + return { + callId: request.callId, + responseParts: [ + { + functionResponse: { + id: request.callId, + name: request.name, + response: { error: message }, + }, + }, + ], + resultDisplay: message, + error: new Error(message), + errorType: ToolErrorType.EXECUTION_FAILED, + }; +} + export interface ServerToolCallConfirmationDetails { request: ToolCallRequestInfo; details: ToolCallConfirmationDetails; @@ -457,11 +489,13 @@ export class Turn { const callId = fnCall.id ?? `${fnCall.name}-${Date.now()}-${Math.random().toString(16).slice(2)}`; + const providerCallId = getProviderToolCallId(fnCall) ?? fnCall.id; const name = fnCall.name || 'undefined_tool_name'; const args = (fnCall.args || {}) as Record; const toolCallRequest: ToolCallRequestInfo = { callId, + ...(providerCallId ? { providerCallId } : {}), name, args, isClientInitiated: false, diff --git a/packages/core/src/utils/filesearch/crawler.ts b/packages/core/src/utils/filesearch/crawler.ts index 06f9942b1e..76c53fb229 100644 --- a/packages/core/src/utils/filesearch/crawler.ts +++ b/packages/core/src/utils/filesearch/crawler.ts @@ -1204,8 +1204,62 @@ async function crawlWithGitLsFiles( return { success: true, files: limitedResults }; } -function buildResultsFromFileSet(files: Set): string[] { +function collectDirectoryRows(options: CrawlOptions): string[] { + const relativeToCrawlDir = getPosixRelative( + options.cwd, + options.crawlDirectory, + ); + const dirFilter = options.ignore.getDirectoryFilter(); + const rows: string[] = []; + + const visit = (dir: string, relativePath: string, depth: number): void => { + const cwdRelative = + relativePath === '' + ? relativeToCrawlDir + : path.posix.join(relativeToCrawlDir, relativePath); + + if (cwdRelative !== '.') { + const row = cwdRelative.endsWith('/') ? cwdRelative : `${cwdRelative}/`; + if (!isValidIgnorePath(row.slice(0, -1)) || dirFilter(row)) { + return; + } + rows.push(row); + } + + if (options.maxDepth !== undefined && depth > options.maxDepth) { + return; + } + + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + + for (const entry of entries) { + if (!entry.isDirectory()) { + continue; + } + const childRelative = relativePath + ? path.posix.join(relativePath, entry.name) + : entry.name; + visit(path.join(dir, entry.name), childRelative, depth + 1); + } + }; + + visit(options.crawlDirectory, '', 0); + return rows; +} + +function buildResultsFromFileSet( + files: Set, + extraDirectories: string[] = [], +): string[] { const dirSet = new Set(); + for (const dir of extraDirectories) { + dirSet.add(dir); + } for (const file of files) { const parts = file.split('/'); let current = ''; @@ -1271,7 +1325,10 @@ async function crawlWithRipgrep( } } - const results = buildResultsFromFileSet(fileSet); + const results = buildResultsFromFileSet( + fileSet, + collectDirectoryRows(options), + ); const filteredResults = applyFilters( results, options,