diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index a7fe80938a..c0b0cb14f1 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -1938,6 +1938,170 @@ describe('Session', () => { expect(finishedSpy).toHaveBeenCalled(); }); + + it('stops an ACP prompt after a repeated duplicate provider id without sending an empty follow-up', async () => { + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO); + vi.mocked(mockChat.getHistoryFunctionResponseIds) + .mockReturnValueOnce(new Set()) + .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 execute = vi.fn().mockResolvedValue({ + llmContent: 'first result', + returnDisplay: 'first result', + }); + mockToolRegistry.getTool.mockReturnValue({ + name: 'read_file', + kind: core.Kind.Read, + displayName: 'Read File', + description: 'Read file', + build: vi.fn().mockReturnValue({ + params: { file_path: 'a.ts' }, + execute, + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + getDescription: vi.fn().mockReturnValue('Read file'), + toolLocations: vi.fn().mockReturnValue([]), + }), + canUpdateOutput: false, + isOutputMarkdown: true, + }); + + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { + functionCalls: [ + { + id: 'shell_1', + name: 'read_file', + args: { file_path: 'a.ts' }, + }, + ], + }, + }, + ]), + ) + .mockResolvedValueOnce( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { functionCalls: [duplicateCall] }, + }, + ]), + ) + .mockResolvedValueOnce( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { + functionCalls: [ + duplicateCall, + { + id: 'fresh_shell', + name: 'read_file', + args: { file_path: 'c.ts' }, + }, + ], + }, + }, + ]), + ); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'read the file' }], + }); + + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(3); + expect(execute).toHaveBeenCalledTimes(1); + const duplicateFollowUp = vi.mocked(mockChat.sendMessageStream).mock + .calls[2][1] as { message: Part[] }; + expect(duplicateFollowUp.message).toHaveLength(1); + expect( + duplicateFollowUp.message[0].functionResponse?.response?.['error'], + ).toContain('Duplicate provider tool call id "shell_1"'); + }); + + it('clears duplicate provider id tracking between ACP prompts', async () => { + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO); + 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!; + + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { functionCalls: [duplicateCall] }, + }, + ]), + ) + .mockResolvedValueOnce(createEmptyStream()) + .mockResolvedValueOnce( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { functionCalls: [duplicateCall] }, + }, + ]), + ) + .mockResolvedValueOnce(createEmptyStream()); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'first prompt' }], + }); + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'second prompt' }], + }); + + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + const firstFollowUp = vi.mocked(mockChat.sendMessageStream).mock + .calls[1][1] as { message: Part[] }; + const secondFollowUp = vi.mocked(mockChat.sendMessageStream).mock + .calls[3][1] as { message: Part[] }; + + expect(firstFollowUp.message).toHaveLength(1); + expect(secondFollowUp.message).toHaveLength(1); + expect( + firstFollowUp.message[0].functionResponse?.response?.['error'], + ).toContain('Duplicate provider tool call id "shell_1"'); + expect( + secondFollowUp.message[0].functionResponse?.response?.['error'], + ).toContain('Duplicate provider tool call id "shell_1"'); + }); }); describe('tool outcome telemetry (#4602 review)', () => { @@ -6589,6 +6753,7 @@ describe('Session', () => { ) => Promise<{ parts: Part[]; stopAfterPermissionCancel: boolean; + repeatedDuplicateProviderToolCall?: boolean; }>; }; @@ -8124,6 +8289,77 @@ describe('Session', () => { ); }); + it('drops repeated duplicate provider functionCall ids after the first synthetic response', 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 firstResult = await ( + session as unknown as ToolCallInternals + ).runToolCalls(new AbortController().signal, 'prompt-history-dup', [ + duplicateCall, + ]); + const secondResult = await ( + session as unknown as ToolCallInternals + ).runToolCalls(new AbortController().signal, 'prompt-history-dup', [ + duplicateCall, + { id: 'fresh_shell', name: 'read_file', args: { file_path: 'c.ts' } }, + ]); + + expect(mockToolRegistry.getTool).not.toHaveBeenCalled(); + expect(build).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + expect(firstResult.parts).toHaveLength(1); + expect(firstResult.parts[0].functionResponse?.id).toBe( + 'shell_1__qwen_dup_2', + ); + expect(firstResult.parts[0].functionResponse?.response).toEqual({ + error: expect.stringContaining( + 'Duplicate provider tool call id "shell_1"', + ), + }); + expect(secondResult.parts).toHaveLength(0); + expect(secondResult.repeatedDuplicateProviderToolCall).toBe(true); + expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledTimes( + 1, + ); + expect(mockClient.sessionUpdate).toHaveBeenCalledTimes(1); + }); + it('suppresses duplicate TodoWrite calls without emitting plan updates', async () => { vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue( new Set(['todo_1']), diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 5c0f0292dd..74bfc0a366 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -35,6 +35,8 @@ import { CompressionStatus, convertToFunctionResponse, createDuplicateProviderToolCallResponse, + findRepeatedDuplicateProviderToolCall, + markDuplicateProviderToolCallResponseSent, createDebugLogger, DiscoveredMCPTool, StreamEventType, @@ -184,6 +186,7 @@ type AutoCompressionSendResult = type RunToolResult = { parts: Part[]; stopAfterPermissionCancel: boolean; + repeatedDuplicateProviderToolCall?: boolean; }; const PERMISSION_CANCEL_SKIP_MESSAGE = @@ -670,6 +673,10 @@ export class Session implements SessionContext { private lastPromptTokenCountChat: GeminiChat | null = null; private midTurnDrainUnavailable = false; private midTurnDrainTimeoutStrikes = 0; + // ACP can continue one logical conversation through prompt, cron, and + // background loops, so keep this with the session instead of a single + // runToolCalls invocation. + private readonly duplicateProviderToolCallResponseIds = new Set(); // Messages from a drain that the daemon answered but we timed out waiting for // (the daemon already spliced + SSE-published them). Re-injected on the next // batch so a transient stall can't silently lose them. See @@ -1115,6 +1122,8 @@ export class Session implements SessionContext { return { stopReason: 'cancelled' }; } + this.duplicateProviderToolCallResponseIds.clear(); + // Track this prompt's completion for the next prompt to await let resolveCompletion!: () => void; this.pendingPromptCompletion = new Promise((resolve) => { @@ -1591,15 +1600,10 @@ export class Session implements SessionContext { ); return { stopReason: 'end_turn' }; } - nextMessage = { - role: 'user', - parts: [ - ...toolRun.parts, - ...(await this.#drainMidTurnUserMessages( - pendingSend.signal, - )), - ], - }; + nextMessage = await this.#buildNextMessageAfterToolRun( + toolRun, + pendingSend.signal, + ); } } @@ -1865,13 +1869,10 @@ export class Session implements SessionContext { ); return { stopReason: 'end_turn' }; } - nextMessage = { - role: 'user', - parts: [ - ...toolRun.parts, - ...(await this.#drainMidTurnUserMessages(pendingSend.signal)), - ], - }; + nextMessage = await this.#buildNextMessageAfterToolRun( + toolRun, + pendingSend.signal, + ); } } @@ -2049,6 +2050,23 @@ export class Session implements SessionContext { await this.messageRewriter?.waitForPendingRewrites(); } + async #buildNextMessageAfterToolRun( + toolRun: RunToolResult, + abortSignal: AbortSignal, + ): Promise { + if (toolRun.repeatedDuplicateProviderToolCall) { + debugLogger.debug( + 'Stopping ACP turn after dropping repeated duplicate provider tool-call response.', + ); + return null; + } + const parts = [ + ...toolRun.parts, + ...(await this.#drainMidTurnUserMessages(abortSignal)), + ]; + return { role: 'user', parts }; + } + #recordCompressionTokenCount(info: ChatCompressionInfo): void { this.#syncPromptTokenCountWithCurrentChat(); const tokenCount = this.#extractCompressionTokenCount(info); @@ -2563,13 +2581,10 @@ export class Session implements SessionContext { ); return; } - nextMessage = { - role: 'user', - parts: [ - ...toolRun.parts, - ...(await this.#drainMidTurnUserMessages(ac.signal)), - ], - }; + nextMessage = await this.#buildNextMessageAfterToolRun( + toolRun, + ac.signal, + ); } } } catch (error) { @@ -2882,13 +2897,10 @@ export class Session implements SessionContext { await this.#emitBackgroundNotificationEndTurn('end_turn'); return; } - nextMessage = { - role: 'user', - parts: [ - ...toolRun.parts, - ...(await this.#drainMidTurnUserMessages(ac.signal)), - ], - }; + nextMessage = await this.#buildNextMessageAfterToolRun( + toolRun, + ac.signal, + ); } } @@ -3246,12 +3258,38 @@ export class Session implements SessionContext { const handledProviderToolCallIds = new Set( this.#getCurrentChat().getHistoryFunctionResponseIds(), ); + const repeatedDuplicateCall = findRepeatedDuplicateProviderToolCall( + dedupedFunctionCalls, + (fc) => getProviderToolCallId(fc) ?? fc.id, + handledProviderToolCallIds, + this.duplicateProviderToolCallResponseIds, + ); + if (repeatedDuplicateCall) { + const providerCallId = + getProviderToolCallId(repeatedDuplicateCall) ?? + repeatedDuplicateCall.id; + debugLogger.debug( + `[Session.runToolCalls] Dropping batch after repeated duplicate provider tool-call id: ` + + `${providerCallId} (tool: ${repeatedDuplicateCall.name ?? 'unknown_tool'})`, + ); + return { + parts: [], + stopAfterPermissionCancel: false, + repeatedDuplicateProviderToolCall: true, + }; + } const pushDuplicateBatch = (request: ToolCallRequestInfo): void => { + const providerCallId = request.providerCallId ?? request.callId; + markDuplicateProviderToolCallResponseSent( + providerCallId, + this.duplicateProviderToolCallResponseIds, + ); + const response = createDuplicateProviderToolCallResponse(request); debugLogger.debug( `[Session.runToolCalls] Suppressing duplicate provider tool-call id: ` + - `${request.providerCallId} (tool: ${request.name})`, + `${providerCallId} (tool: ${request.name})`, ); batches.push({ kind: 'duplicate', request, response }); }; @@ -3457,7 +3495,11 @@ export class Session implements SessionContext { } if (shouldStop) { await appendSkippedAfter(parts, batch.calls[batch.calls.length - 1]); - return { parts, stopAfterPermissionCancel: true }; + return { + parts, + stopAfterPermissionCancel: true, + repeatedDuplicateProviderToolCall: false, + }; } } else { for (const fc of batch.calls) { @@ -3465,12 +3507,20 @@ export class Session implements SessionContext { parts.push(...r.parts); if (r.stopAfterPermissionCancel) { await appendSkippedAfter(parts, fc); - return { parts, stopAfterPermissionCancel: true }; + return { + parts, + stopAfterPermissionCancel: true, + repeatedDuplicateProviderToolCall: false, + }; } } } } - return { parts, stopAfterPermissionCancel: false }; + return { + parts, + stopAfterPermissionCancel: false, + repeatedDuplicateProviderToolCall: false, + }; } /** diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index a341e6dbd4..0f9dab4330 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -477,7 +477,7 @@ describe('runNonInteractive', () => { toolCallEvent, { type: GeminiEventType.LoopDetected, - value: { loopType: LoopType.GLOBAL_TOOL_CALL_DUPLICATE }, + value: { loopType: LoopType.REPETITIVE_THOUGHTS }, }, ]; mockGeminiClient.sendMessageStream.mockReturnValue( @@ -667,6 +667,159 @@ describe('runNonInteractive', () => { expect(processStdoutSpy).toHaveBeenCalledWith('Final answer\n'); }); + it('should stop repeated duplicate provider tool-call responses', 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-loop', + }, + }; + const freshToolCallEvent: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-2', + providerCallId: 'tool-2', + name: 'testTool', + args: { arg1: 'value2' }, + isClientInitiated: false, + prompt_id: 'prompt-id-dup-loop', + }, + }; + mockCoreExecuteToolCall.mockResolvedValue({ + responseParts: [{ text: 'Tool response' }], + }); + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce(createStreamFromEvents([toolCallEvent])) + .mockReturnValueOnce(createStreamFromEvents([toolCallEvent])) + .mockReturnValueOnce( + createStreamFromEvents([toolCallEvent, freshToolCallEvent]), + ); + + const exitCode = await runNonInteractive( + mockConfig, + mockSettings, + 'Use a tool', + 'prompt-id-dup-loop', + ); + + expect(exitCode).toBe(1); + 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(processStderrSpy).toHaveBeenCalledWith( + expect.stringContaining(LoopType.GLOBAL_TOOL_CALL_DUPLICATE), + ); + expect(processStderrSpy).toHaveBeenCalledWith( + expect.stringContaining( + 'always-on guard and cannot be disabled via `model.skipLoopDetection`', + ), + ); + expect(processStderrSpy).not.toHaveBeenCalledWith( + expect.stringContaining( + 'Set the `model.skipLoopDetection` setting to true', + ), + ); + }); + + it('should stop repeated duplicate provider tool-call responses from drain items', async () => { + setupMetricsMock(); + mockGeminiClient.getHistoryFunctionResponseIds.mockReturnValue( + new Set(['tool-drain']), + ); + + const notificationXml = + '\n' + + 'mon_1\n' + + 'monitor\n' + + 'running\n' + + 'Monitor emitted event #1.\n' + + 'ready\n' + + ''; + mockMonitorRegistry.setNotificationCallback.mockImplementation((cb) => { + if (!cb) return; + cb('Monitor "logs" event #1: ready', notificationXml, { + monitorId: 'mon_1', + toolUseId: 'tool_mon_1', + status: 'running', + eventCount: 1, + }); + }); + + const duplicateToolCallEvent: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-drain__qwen_dup_2', + providerCallId: 'tool-drain', + name: 'testTool', + args: { arg1: 'value1' }, + isClientInitiated: false, + prompt_id: 'prompt-id-drain-dup-loop', + }, + }; + const freshToolCallEvent: ServerGeminiStreamEvent = { + type: GeminiEventType.ToolCallRequest, + value: { + callId: 'tool-fresh', + providerCallId: 'tool-fresh', + name: 'testTool', + args: { arg1: 'value2' }, + isClientInitiated: false, + prompt_id: 'prompt-id-drain-dup-loop', + }, + }; + + mockGeminiClient.sendMessageStream + .mockReturnValueOnce( + createStreamFromEvents([ + { type: GeminiEventType.Content, value: 'Monitor launched.' }, + { + type: GeminiEventType.Finished, + value: { + reason: undefined, + usageMetadata: { totalTokenCount: 2 }, + }, + }, + ]), + ) + .mockReturnValueOnce(createStreamFromEvents([duplicateToolCallEvent])) + .mockReturnValueOnce( + createStreamFromEvents([duplicateToolCallEvent, freshToolCallEvent]), + ); + + const exitCode = await runNonInteractive( + mockConfig, + mockSettings, + 'Watch the logs', + 'prompt-id-drain-dup-loop', + ); + + expect(exitCode).toBe(1); + expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(3); + expect(mockCoreExecuteToolCall).not.toHaveBeenCalled(); + + const duplicateParts = mockGeminiClient.sendMessageStream.mock + .calls[2][0] as Part[]; + expect(duplicateParts[0].functionResponse?.response?.['error']).toContain( + 'Duplicate provider tool call id "tool-drain"', + ); + expect(processStderrSpy).toHaveBeenCalledWith( + expect.stringContaining(LoopType.GLOBAL_TOOL_CALL_DUPLICATE), + ); + }); + it('should ignore duplicate provider tool-call ids already present in chat history', async () => { setupMetricsMock(); mockGeminiClient.getHistoryFunctionResponseIds.mockReturnValue( diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 24af0d19d3..3108cc8787 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -31,6 +31,8 @@ import { ApprovalMode, ToolConfirmationOutcome, createDuplicateProviderToolCallResponse, + markDuplicateProviderToolCallResponseSent, + findRepeatedDuplicateProviderToolCall, } from '@qwen-code/qwen-code-core'; import type { Content, Part, PartListUnion } from '@google/genai'; import type { CLIUserMessage, PermissionMode } from './nonInteractive/types.js'; @@ -126,7 +128,8 @@ function formatLoopDetectedMessage(loopType: LoopType | undefined): string { // it for those always-on loop types. const isAlwaysOn = loopType === LoopType.TURN_TOOL_CALL_CAP || - loopType === LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS; + loopType === LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS || + loopType === LoopType.GLOBAL_TOOL_CALL_DUPLICATE; const hint = isAlwaysOn ? ' This is an always-on guard and cannot be disabled via `model.skipLoopDetection`.' : ' Set the `model.skipLoopDetection` setting to true to disable.'; @@ -796,15 +799,29 @@ export async function runNonInteractive( */ const handledProviderToolCallIds = geminiClient.getHistoryFunctionResponseIds(); + // Tracks duplicate-error responses emitted during this headless run. + // Once a provider id reaches this set, seeing it again is terminal for + // the current tool batch so we do not send partial tool responses. + const duplicateProviderToolCallResponseIds = new Set(); + + type ToolCallBatchResult = { + responseParts: Part[]; + repeatedDuplicateProviderToolCall: boolean; + }; const processToolCallBatch = async ( batchRequests: ToolCallRequestInfo[], setModelOverride: (override: string | undefined) => void, - ): Promise => { + ): Promise => { const toolResponseParts: Part[] = []; const structuredOutputActive = config.getJsonSchema() && batchRequests.some((r) => r.name === ToolNames.STRUCTURED_OUTPUT); + const getProviderResponseId = ( + request: ToolCallRequestInfo, + ): string | undefined => + request.providerCallId ?? + (structuredOutputActive ? undefined : request.callId || undefined); const seenBatchCallIds = new Set(); const duplicateBatchRequests: ToolCallRequestInfo[] = []; const uniqueBatchRequests = batchRequests.filter((request) => { @@ -826,26 +843,51 @@ export async function runNonInteractive( } return true; }); + const repeatedDuplicateRequest = findRepeatedDuplicateProviderToolCall( + [...uniqueBatchRequests, ...duplicateBatchRequests], + getProviderResponseId, + handledProviderToolCallIds, + duplicateProviderToolCallResponseIds, + ); + if (repeatedDuplicateRequest) { + const providerCallId = + repeatedDuplicateRequest.providerCallId ?? + repeatedDuplicateRequest.callId; + debugLogger.debug( + `[runNonInteractive] Dropping batch after repeated duplicate provider tool-call id: ${providerCallId} (tool: ${repeatedDuplicateRequest.name})`, + ); + return { + responseParts: [], + repeatedDuplicateProviderToolCall: true, + }; + } + const respondedRequests = new Set(); const executableBatchRequests: ToolCallRequestInfo[] = []; const duplicatePendingResponses: Part[] = []; for (const requestInfo of uniqueBatchRequests) { - if (!requestInfo.providerCallId) { + const providerCallId = getProviderResponseId(requestInfo); + if (!providerCallId) { executableBatchRequests.push(requestInfo); continue; } - if (!handledProviderToolCallIds.has(requestInfo.providerCallId)) { - handledProviderToolCallIds.add(requestInfo.providerCallId); + if (!handledProviderToolCallIds.has(providerCallId)) { + handledProviderToolCallIds.add(providerCallId); executableBatchRequests.push(requestInfo); continue; } + markDuplicateProviderToolCallResponseSent( + providerCallId, + duplicateProviderToolCallResponseIds, + ); + const toolResponse = createDuplicateProviderToolCallResponse(requestInfo); debugLogger.debug( - `[runNonInteractive] Suppressing duplicate provider tool-call id: ${requestInfo.providerCallId} (tool: ${requestInfo.name})`, + `[runNonInteractive] Suppressing duplicate provider tool-call id: ${providerCallId} (tool: ${requestInfo.name})`, ); respondedRequests.add(requestInfo); adapter.emitToolResult(requestInfo, toolResponse); @@ -1028,13 +1070,23 @@ export async function runNonInteractive( } for (const requestInfo of duplicateBatchRequests) { + const providerCallId = getProviderResponseId(requestInfo); + if (!providerCallId) continue; + markDuplicateProviderToolCallResponseSent( + providerCallId, + duplicateProviderToolCallResponseIds, + ); + const toolResponse = createDuplicateProviderToolCallResponse(requestInfo); adapter.emitToolResult(requestInfo, toolResponse); toolResponseParts.push(...toolResponse.responseParts); } - return toolResponseParts; + return { + responseParts: toolResponseParts, + repeatedDuplicateProviderToolCall: false, + }; }; while (true) { @@ -1175,12 +1227,12 @@ export async function runNonInteractive( // `modelOverride` so the next turn's sendMessageStream sees // it; the drain turn updates a per-item `itemModelOverride` // scoped to that drain item. - const toolResponseParts = await processToolCallBatch( - toolCallRequests, - (override) => { - modelOverride = override; - }, - ); + const { + responseParts: toolResponseParts, + repeatedDuplicateProviderToolCall, + } = await processToolCallBatch(toolCallRequests, (override) => { + modelOverride = override; + }); if (structuredSubmission !== undefined) { // Single-shot terminal contract; aborts in-flight background @@ -1190,6 +1242,16 @@ export async function runNonInteractive( // post-loop branch — see emitStructuredSuccess above. return emitStructuredSuccess(); } + if ( + repeatedDuplicateProviderToolCall && + toolResponseParts.length === 0 + ) { + loopDetectedMessage = emitLoopDetectedMessage( + config, + LoopType.GLOBAL_TOOL_CALL_DUPLICATE, + ); + return emitLoopDetectedResult(); + } currentMessages = [{ role: 'user', parts: toolResponseParts }]; hasUnsentToolResponse = true; } else { @@ -1401,7 +1463,10 @@ export async function runNonInteractive( // sendMessageStream picks up the per-item override), // while the main loop binds to the session-scoped // `modelOverride`. - const itemToolResponseParts = await processToolCallBatch( + const { + responseParts: itemToolResponseParts, + repeatedDuplicateProviderToolCall, + } = await processToolCallBatch( itemToolCallRequests, (override) => { itemModelOverride = override; @@ -1413,6 +1478,17 @@ export async function runNonInteractive( // the post-drain code will emit the terminal result. return; } + if ( + repeatedDuplicateProviderToolCall && + itemToolResponseParts.length === 0 + ) { + loopDetectedMessage = emitLoopDetectedMessage( + config, + LoopType.GLOBAL_TOOL_CALL_DUPLICATE, + ); + loopDetected = true; + return; + } itemMessages = [{ role: 'user', parts: itemToolResponseParts }]; } else { break; diff --git a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx index 01bd03087d..af29a31111 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.test.tsx +++ b/packages/cli/src/ui/hooks/useGeminiStream.test.tsx @@ -2592,6 +2592,80 @@ describe('useGeminiStream', () => { expect(client.recordCompletedToolCall).not.toHaveBeenCalled(); }); + it('drops repeated history-paired duplicate provider ids after the first synthetic response', 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-loop', + }, + }; + })(), + ) + .mockReturnValueOnce( + (async function* () { + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'tool-history', + providerCallId: 'tool-history', + name: 'shell', + args: { command: 'echo duplicate again' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-history-loop', + }, + }; + yield { + type: ServerGeminiEventType.ToolCallRequest, + value: { + callId: 'tool-fresh', + providerCallId: 'tool-fresh', + name: 'shell', + args: { command: 'echo fresh' }, + isClientInitiated: false, + prompt_id: 'prompt-tui-history-loop', + }, + }; + yield { + type: ServerGeminiEventType.Finished, + value: { reason: undefined, usageMetadata: { totalTokenCount: 1 } }, + }; + })(), + ); + + const { result } = renderTestHook([], client); + + await act(async () => { + await result.current.submitQuery('run shell'); + }); + + await waitFor(() => { + expect(result.current.streamingState).toBe(StreamingState.Idle); + }); + + expect(mockScheduleToolCalls).not.toHaveBeenCalled(); + expect(mockSendMessageStream).toHaveBeenCalledTimes(2); + const toolResultParts = mockSendMessageStream.mock.calls[1][0] as Part[]; + expect(toolResultParts).toHaveLength(1); + 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* () { diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 348557861b..70fec97c5d 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -60,6 +60,8 @@ import { setActiveGoal, clearActiveGoal, createDuplicateProviderToolCallResponse, + markDuplicateProviderToolCallResponseSent, + findRepeatedDuplicateProviderToolCall, } from '@qwen-code/qwen-code-core'; import { type Part, type PartListUnion, FinishReason } from '@google/genai'; import type { @@ -513,6 +515,11 @@ export const useGeminiStream = ( const submitPromptOnCompleteRef = useRef<(() => Promise) | null>(null); const modelOverrideRef = useRef(undefined); const handledProviderToolCallIdsRef = useRef>(new Set()); + // Scoped to a top-level submit and cleared below before a new user prompt. + // Repeated duplicate provider ids within that submit are terminal/drop-only. + const duplicateProviderToolCallResponseIdsRef = useRef>( + new Set(), + ); const pendingDuplicateToolResponsesRef = useRef< PendingDuplicateToolResponses[] >([]); @@ -1994,6 +2001,23 @@ export const useGeminiStream = ( const historyCallIdsWithResponse: Set = geminiClient ? geminiClient.getHistoryFunctionResponseIds() : new Set(); + const handledProviderIds = new Set([ + ...handledProviderToolCallIdsRef.current, + ...historyCallIdsWithResponse, + ]); + const repeatedDuplicateRequest = findRepeatedDuplicateProviderToolCall( + toolCallRequests, + (request) => request.providerCallId, + handledProviderIds, + duplicateProviderToolCallResponseIdsRef.current, + ); + if (repeatedDuplicateRequest?.providerCallId) { + debugLogger.debug( + `[processGeminiStreamEvents] Dropping batch after repeated duplicate provider tool-call id: ${repeatedDuplicateRequest.providerCallId} (tool: ${repeatedDuplicateRequest.name})`, + ); + loopDetectedRef.current = true; + return StreamProcessingStatus.Completed; + } for (const request of toolCallRequests) { const providerCallId = request.providerCallId; @@ -2006,6 +2030,11 @@ export const useGeminiStream = ( handledProviderToolCallIdsRef.current.has(providerCallId) || historyCallIdsWithResponse.has(providerCallId) ) { + markDuplicateProviderToolCallResponseSent( + providerCallId, + duplicateProviderToolCallResponseIdsRef.current, + ); + const response = createDuplicateProviderToolCallResponse(request); debugLogger.debug( `[processGeminiStreamEvents] Suppressing duplicate provider tool-call id: ${providerCallId} (tool: ${request.name})`, @@ -2133,6 +2162,7 @@ export const useGeminiStream = ( lastTurnUserItemRef.current = null; turnSawContentEventRef.current = false; handledProviderToolCallIdsRef.current.clear(); + duplicateProviderToolCallResponseIdsRef.current.clear(); pendingDuplicateToolResponsesRef.current = []; immediateDuplicateToolResponsesRef.current = null; } diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index e6cdf9b5c7..71dcbb682b 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -30,6 +30,8 @@ import { } from './agent-context.js'; import { createDuplicateProviderToolCallResponse, + findRepeatedDuplicateProviderToolCall, + markDuplicateProviderToolCallResponseSent, type ToolCallRequestInfo, } from '../../core/turn.js'; import { @@ -662,6 +664,9 @@ export class AgentCore { let finalText = ''; let terminateMode: AgentTerminateMode | null = null; const handledProviderToolCallIds = chat.getHistoryFunctionResponseIds(); + // Scoped to this reasoning loop. A second duplicate response for the same + // provider id would keep deterministic providers in a tool-result loop. + const duplicateProviderToolCallResponseIds = new Set(); let stickyMaxOutputTokens: number | undefined; while (true) { @@ -822,7 +827,7 @@ export class AgentCore { } if (functionCalls.length > 0) { - currentMessages = await this.processFunctionCalls( + const toolCallResult = await this.processFunctionCalls( functionCalls, roundAbortController, promptId, @@ -831,7 +836,13 @@ export class AgentCore { currentResponseId, wasOutputTruncated, handledProviderToolCallIds, + duplicateProviderToolCallResponseIds, ); + if (toolCallResult.repeatedDuplicateProviderToolCall) { + terminateMode = AgentTerminateMode.LOOP_DETECTED; + break; + } + currentMessages = toolCallResult.messages; const externalInputs = this.drainExternalInputs(options); if (externalInputs.length > 0) { @@ -848,6 +859,10 @@ export class AgentCore { // is a model-facing detail, not part of the original message. this.emitExternalInputEvents(externalInputs); } + if ((currentMessages[0]?.parts?.length ?? 0) === 0) { + terminateMode = AgentTerminateMode.ERROR; + break; + } } else { const immediateExternalInputs = this.drainExternalInputs(options); if (immediateExternalInputs.length > 0) { @@ -1149,12 +1164,36 @@ export class AgentCore { responseId?: string, wasOutputTruncated = false, handledProviderToolCallIds = new Set(), - ): Promise { + duplicateProviderToolCallResponseIds = new Set(), + ): Promise<{ + messages: Content[]; + repeatedDuplicateProviderToolCall: boolean; + }> { const toolResponseParts: Part[] = []; const uniqueFunctionCalls = dedupeToolCallsById(functionCalls); // Build allowed tool names set for filtering const allowedToolNames = new Set(toolsList.map((t) => t.name)); + const repeatedDuplicateCall = findRepeatedDuplicateProviderToolCall( + uniqueFunctionCalls, + (fc) => getProviderToolCallId(fc) ?? fc.id, + handledProviderToolCallIds, + duplicateProviderToolCallResponseIds, + ); + if (repeatedDuplicateCall) { + const providerCallId = + getProviderToolCallId(repeatedDuplicateCall) ?? + repeatedDuplicateCall.id; + this.runtimeContext + .getDebugLogger() + ?.debug( + `[processFunctionCalls] Dropping batch after repeated duplicate provider tool-call id: ${providerCallId} (tool: ${String(repeatedDuplicateCall.name)}, round: ${currentRound})`, + ); + return { + messages: [{ role: 'user', parts: [] }], + repeatedDuplicateProviderToolCall: true, + }; + } // Filter unauthorized tool calls before scheduling const authorizedCalls: FunctionCall[] = []; @@ -1191,6 +1230,11 @@ export class AgentCore { if (providerCallId) { if (handledProviderToolCallIds.has(providerCallId)) { + markDuplicateProviderToolCallResponseSent( + providerCallId, + duplicateProviderToolCallResponseIds, + ); + const request: ToolCallRequestInfo = { callId, providerCallId, @@ -1523,7 +1567,10 @@ export class AgentCore { }); } - return [{ role: 'user', parts: toolResponseParts }]; + return { + messages: [{ role: 'user', parts: toolResponseParts }], + repeatedDuplicateProviderToolCall: false, + }; } // ─── Observable state accessors ──────────────────────────── diff --git a/packages/core/src/agents/runtime/agent-headless.test.ts b/packages/core/src/agents/runtime/agent-headless.test.ts index aba104d1d6..f69cc9236e 100644 --- a/packages/core/src/agents/runtime/agent-headless.test.ts +++ b/packages/core/src/agents/runtime/agent-headless.test.ts @@ -1147,6 +1147,115 @@ describe('subagent.ts', () => { expect(scope.getTerminateMode()).toBe(AgentTerminateMode.GOAL); }); + it('should stop repeated duplicate provider tool-call responses', 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!], + [ + duplicateNormalizedPart!.functionCall!, + { + id: 'call_2', + name: 'list_files', + args: { path: './fresh' }, + }, + ], + ]), + ); + + 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 toolResultEvents: AgentToolResultEvent[] = []; + const eventEmitter = new AgentEventEmitter(); + 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(mockSendMessageStream).toHaveBeenCalledTimes(3); + expect(toolResultEvents).toHaveLength(2); + 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.LOOP_DETECTED); + }); + it('should ignore duplicate provider tool-call ids already present in chat history', async () => { const listFilesToolDef: FunctionDeclaration = { name: 'list_files', diff --git a/packages/core/src/agents/runtime/agent-interactive.ts b/packages/core/src/agents/runtime/agent-interactive.ts index 00e013341c..54c8d569d2 100644 --- a/packages/core/src/agents/runtime/agent-interactive.ts +++ b/packages/core/src/agents/runtime/agent-interactive.ts @@ -481,6 +481,11 @@ function terminateModeMessage( return { text: 'Agent stopped: time limit reached.', level: 'warning' }; case AgentTerminateMode.ERROR: return { text: 'Agent stopped due to an error.', level: 'error' }; + case AgentTerminateMode.LOOP_DETECTED: + return { + text: 'Agent stopped: duplicate tool-call loop detected.', + level: 'error', + }; case AgentTerminateMode.CANCELLED: case AgentTerminateMode.SHUTDOWN: return null; diff --git a/packages/core/src/agents/runtime/agent-types.ts b/packages/core/src/agents/runtime/agent-types.ts index 21838fc75e..82362e2377 100644 --- a/packages/core/src/agents/runtime/agent-types.ts +++ b/packages/core/src/agents/runtime/agent-types.ts @@ -107,6 +107,8 @@ export enum AgentTerminateMode { GOAL = 'GOAL', /** The agent's execution terminated because it exceeded the maximum number of turns. */ MAX_TURNS = 'MAX_TURNS', + /** The agent's execution terminated after detecting a tool-call loop. */ + LOOP_DETECTED = 'LOOP_DETECTED', /** The agent's execution was cancelled via an abort signal. */ CANCELLED = 'CANCELLED', /** The agent was gracefully shut down (e.g., arena/team session ended). */ diff --git a/packages/core/src/core/turn.test.ts b/packages/core/src/core/turn.test.ts index f6a7c3b86f..a3a3e4d99a 100644 --- a/packages/core/src/core/turn.test.ts +++ b/packages/core/src/core/turn.test.ts @@ -9,7 +9,12 @@ import type { ServerGeminiToolCallRequestEvent, ServerGeminiErrorEvent, } from './turn.js'; -import { CompressionStatus, Turn, GeminiEventType } from './turn.js'; +import { + CompressionStatus, + Turn, + GeminiEventType, + findRepeatedDuplicateProviderToolCall, +} from './turn.js'; import type { GenerateContentResponse, Part, Content } from '@google/genai'; import { reportError } from '../utils/errorReporting.js'; import type { GeminiChat } from './geminiChat.js'; @@ -37,6 +42,54 @@ vi.mock('../utils/errorReporting', () => ({ reportError: vi.fn(), })); +describe('findRepeatedDuplicateProviderToolCall', () => { + const getProviderCallId = (item: { providerCallId?: string }) => + item.providerCallId; + + it('finds a handled provider id that already received a synthetic response', () => { + const items = [{ providerCallId: 'fresh' }, { providerCallId: 'handled' }]; + + expect( + findRepeatedDuplicateProviderToolCall( + items, + getProviderCallId, + new Set(['handled']), + new Set(['handled']), + ), + ).toBe(items[1]); + }); + + it('finds a handled provider id repeated within the same batch', () => { + const items = [ + { providerCallId: 'handled' }, + { providerCallId: 'fresh' }, + { providerCallId: 'handled' }, + ]; + + expect( + findRepeatedDuplicateProviderToolCall( + items, + getProviderCallId, + new Set(['handled']), + new Set(), + ), + ).toBe(items[0]); + }); + + it('ignores unhandled repeated ids', () => { + const items = [{ providerCallId: 'fresh' }, { providerCallId: 'fresh' }]; + + expect( + findRepeatedDuplicateProviderToolCall( + items, + getProviderCallId, + new Set(['handled']), + new Set(), + ), + ).toBeUndefined(); + }); +}); + describe('Turn', () => { let turn: Turn; // Define a type for the mocked Chat instance for clarity diff --git a/packages/core/src/core/turn.ts b/packages/core/src/core/turn.ts index 5038b919ef..47bc9366bb 100644 --- a/packages/core/src/core/turn.ts +++ b/packages/core/src/core/turn.ts @@ -149,6 +149,42 @@ export function createDuplicateProviderToolCallResponse( }; } +export function markDuplicateProviderToolCallResponseSent( + providerCallId: string, + duplicateProviderToolCallResponseIds: Set, +): void { + duplicateProviderToolCallResponseIds.add(providerCallId); +} + +export function findRepeatedDuplicateProviderToolCall( + items: readonly T[], + getProviderCallId: (item: T) => string | undefined, + handledProviderToolCallIds: ReadonlySet, + duplicateProviderToolCallResponseIds: ReadonlySet, +): T | undefined { + const repeatedProviderIds = new Map(); + for (const item of items) { + const providerCallId = getProviderCallId(item); + if (!providerCallId || !handledProviderToolCallIds.has(providerCallId)) { + continue; + } + repeatedProviderIds.set( + providerCallId, + (repeatedProviderIds.get(providerCallId) ?? 0) + 1, + ); + } + + return items.find((item) => { + const providerCallId = getProviderCallId(item); + return ( + providerCallId !== undefined && + handledProviderToolCallIds.has(providerCallId) && + (duplicateProviderToolCallResponseIds.has(providerCallId) || + (repeatedProviderIds.get(providerCallId) ?? 0) > 1) + ); + }); +} + export interface ServerToolCallConfirmationDetails { request: ToolCallRequestInfo; details: ToolCallConfirmationDetails;