diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 6fd81cfa21..5b022f3875 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -49,6 +49,7 @@ import { Turn, type ServerGeminiStreamEvent, } from './turn.js'; +import { LoopType } from '../telemetry/types.js'; vi.mock('../utils/retry.js', () => ({ retryWithBackoff: vi.fn(async (fn) => await fn()), @@ -271,6 +272,8 @@ vi.mock('../telemetry/loggers.js', () => ({ logChatCompression: vi.fn(), logNextSpeakerCheck: vi.fn(), logApiRequest: vi.fn(), + logLoopDetected: vi.fn(), + logLoopDetectionDisabled: vi.fn(), })); const { mockClientDebugLogger } = vi.hoisted(() => ({ @@ -4110,7 +4113,7 @@ hello // Force LoopDetector to trip on the first event. const loopDetector = client['loopDetector']; - vi.spyOn(loopDetector, 'addAndCheck').mockReturnValue(true); + vi.spyOn(loopDetector, 'addAndCheckHeuristicLoops').mockReturnValue(true); vi.spyOn(loopDetector, 'getLastLoopType').mockReturnValue(null); mockTurnRunFn.mockReturnValue( @@ -6026,14 +6029,15 @@ Other open files: expect(mockCheckNextSpeaker).not.toHaveBeenCalled(); }); - it('does not run loop checks when skipLoopDetection is true', async () => { + it('keeps deterministic tool-call checks when skipLoopDetection is true', async () => { // Arrange // Ensure config returns true for skipLoopDetection vi.spyOn(client['config'], 'getSkipLoopDetection').mockReturnValue(true); // Replace loop detector with spies const ldMock = { - addAndCheck: vi.fn().mockReturnValue(false), + addAndCheckDeterministicToolCallLoop: vi.fn().mockReturnValue(false), + addAndCheckHeuristicLoops: vi.fn().mockReturnValue(false), reset: vi.fn(), }; // @ts-expect-error override private for testing @@ -6061,8 +6065,49 @@ Other open files: // consume stream } - // Assert - loop detection methods should not be called when skipLoopDetection is true - expect(ldMock.addAndCheck).not.toHaveBeenCalled(); + expect(ldMock.addAndCheckDeterministicToolCallLoop).toHaveBeenCalledTimes( + 2, + ); + expect(ldMock.addAndCheckHeuristicLoops).not.toHaveBeenCalled(); + }); + + it('hard-stops identical tool calls even when skipLoopDetection is true', async () => { + vi.spyOn(client['config'], 'getSkipLoopDetection').mockReturnValue(true); + + mockTurnRunFn.mockReturnValue( + (async function* () { + for (let i = 0; i < 5; i++) { + yield { + type: GeminiEventType.ToolCallRequest, + value: { + callId: `repeat-${i}`, + name: 'run_shell_command', + args: { command: 'echo repeated' }, + }, + }; + } + })(), + ); + + const mockChat: Partial = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + }; + client['chat'] = mockChat as GeminiChat; + + const events = await fromAsync( + client.sendMessageStream( + [{ text: 'repeat a tool' }], + new AbortController().signal, + 'prompt-id-skip-loop-identical', + ), + ); + + expect(events.at(-1)).toEqual({ + type: GeminiEventType.LoopDetected, + value: { loopType: LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS }, + }); + expect(events).toHaveLength(5); }); describe('retry sendMessageType', () => { diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 9ed3f50d10..fbe5615353 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -111,7 +111,7 @@ import { promptIdContext } from '../utils/promptIdContext.js'; import { retryWithBackoff, isUnattendedMode } from '../utils/retry.js'; import { subagentNameContext } from '../utils/subagentNameContext.js'; import { escapeSystemReminderTags } from '../utils/xml.js'; -import { ApiRetryEvent } from '../telemetry/types.js'; +import { ApiRetryEvent, LoopType } from '../telemetry/types.js'; import { logApiRetry } from '../telemetry/loggers.js'; // Hook types and utilities @@ -321,10 +321,7 @@ export class GeminiClient { this.initializedSessionId = sessionId; // Clean up stale tool result files from previous sessions (fire-and-forget) - void cleanupOldToolResults( - Storage.getGlobalTempDir(), - 24 * 60 * 60 * 1000, - ); + void cleanupOldToolResults(Storage.getGlobalTempDir(), 24 * 60 * 60 * 1000); } /** @@ -658,10 +655,7 @@ export class GeminiClient { debugLogger.debug('[FILE_READ_CACHE] clear after resetChat'); this.config.getFileReadCache().clear(); // Clean up old tool result overflow files on /clear - void cleanupOldToolResults( - Storage.getGlobalTempDir(), - 24 * 60 * 60 * 1000, - ); + void cleanupOldToolResults(Storage.getGlobalTempDir(), 24 * 60 * 60 * 1000); this.config.getBaseLlmClient().clearPerModelGeneratorCache(); // Abort any in-flight auto-memory recall so the stale controller // does not leak into the next session. @@ -2095,24 +2089,40 @@ export class GeminiClient { didUpdateIdeContextState = true; } - if (!this.config.getSkipLoopDetection()) { - if (this.loopDetector.addAndCheck(event)) { - const loopType = this.loopDetector.getLastLoopType(); - yield { - type: GeminiEventType.LoopDetected, - ...(loopType && { value: { loopType } }), - }; - if (arenaAgentClient) { - await arenaAgentClient.reportError('Loop detected'); - } - this.lastApiCompletionTimestamp = Date.now(); - if (isTopLevelInteraction) - endInteractionSpan('error', { errorMessage: 'loop detected' }); - // finally cleanup catches this, but cancel explicitly to match - // the cleanup pattern at other early-return sites. - this.cancelPendingMemoryPrefetch(); - return turn; + const deterministicToolCallLoop = + this.loopDetector.addAndCheckDeterministicToolCallLoop(event); + const heuristicLoop = + !deterministicToolCallLoop && + !this.config.getSkipLoopDetection() && + this.loopDetector.addAndCheckHeuristicLoops(event); + if (deterministicToolCallLoop || heuristicLoop) { + const loopType = this.loopDetector.getLastLoopType(); + if ( + event.type === GeminiEventType.ToolCallRequest && + loopType === LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS + ) { + const repeatedCount = + this.loopDetector.getConsecutiveToolCallCount(); + const repeatedStartIndex = Math.max( + 0, + turn.pendingToolCalls.length - repeatedCount, + ); + turn.pendingToolCalls.splice(repeatedStartIndex); } + yield { + type: GeminiEventType.LoopDetected, + ...(loopType && { value: { loopType } }), + }; + if (arenaAgentClient) { + await arenaAgentClient.reportError('Loop detected'); + } + this.lastApiCompletionTimestamp = Date.now(); + if (isTopLevelInteraction) + endInteractionSpan('error', { errorMessage: 'loop detected' }); + // finally cleanup catches this, but cancel explicitly to match + // the cleanup pattern at other early-return sites. + this.cancelPendingMemoryPrefetch(); + return turn; } // Update arena status on Finished events — stats are derived // automatically from uiTelemetryService by the reporter. diff --git a/packages/core/src/services/loopDetectionService.test.ts b/packages/core/src/services/loopDetectionService.test.ts index 2d0ba5e3d6..d4ea53ee04 100644 --- a/packages/core/src/services/loopDetectionService.test.ts +++ b/packages/core/src/services/loopDetectionService.test.ts @@ -142,6 +142,39 @@ describe('LoopDetectionService', () => { expect(loggers.logLoopDetected).toHaveBeenCalledTimes(1); }); + it('should reset the deterministic tool-call counter on retry', () => { + const event = createToolCallRequestEvent('testTool', { param: 'value' }); + for (let i = 0; i < TOOL_CALL_LOOP_THRESHOLD - 1; i++) { + expect(service.addAndCheckDeterministicToolCallLoop(event)).toBe(false); + } + + expect( + service.addAndCheckDeterministicToolCallLoop({ + type: GeminiEventType.Retry, + } as ServerGeminiStreamEvent), + ).toBe(false); + + for (let i = 0; i < TOOL_CALL_LOOP_THRESHOLD - 1; i++) { + expect(service.addAndCheckDeterministicToolCallLoop(event)).toBe(false); + } + expect(loggers.logLoopDetected).not.toHaveBeenCalled(); + }); + + it('should expose the current consecutive tool-call count', () => { + const event = createToolCallRequestEvent('testTool', { param: 'value' }); + for (let i = 0; i < TOOL_CALL_LOOP_THRESHOLD - 1; i++) { + service.addAndCheckDeterministicToolCallLoop(event); + } + + expect(service.getConsecutiveToolCallCount()).toBe( + TOOL_CALL_LOOP_THRESHOLD - 1, + ); + expect(service.addAndCheckDeterministicToolCallLoop(event)).toBe(true); + expect(service.getConsecutiveToolCallCount()).toBe( + TOOL_CALL_LOOP_THRESHOLD, + ); + }); + it('should not detect a loop when disabled for session', () => { service.disableForSession(); expect(loggers.logLoopDetectionDisabled).toHaveBeenCalledTimes(1); diff --git a/packages/core/src/services/loopDetectionService.ts b/packages/core/src/services/loopDetectionService.ts index ed0006733b..f6e4d90933 100644 --- a/packages/core/src/services/loopDetectionService.ts +++ b/packages/core/src/services/loopDetectionService.ts @@ -102,6 +102,10 @@ export class LoopDetectionService { return this.lastLoopType; } + getConsecutiveToolCallCount(): number { + return this.toolCallRepetitionCount; + } + /** * Disables loop detection for the current session. */ @@ -125,6 +129,14 @@ export class LoopDetectionService { * @returns true if a loop is detected, false otherwise */ addAndCheck(event: ServerGeminiStreamEvent): boolean { + if (this.addAndCheckDeterministicToolCallLoop(event)) { + return true; + } + + return this.addAndCheckHeuristicLoops(event); + } + + addAndCheckHeuristicLoops(event: ServerGeminiStreamEvent): boolean { if (this.loopDetected || this.disabledForSession) { return this.loopDetected; } @@ -139,12 +151,11 @@ export class LoopDetectionService { // observable progress — any prior thoughts should not carry over. this.thoughtHistory = []; - const toolCallLoop = this.checkToolCallLoop(event.value); this.trackToolCall(event.value); const readFileLoop = this.checkReadFileLoop(); const actionStagnation = this.checkActionStagnation(); - this.loopDetected = toolCallLoop || readFileLoop || actionStagnation; + this.loopDetected = readFileLoop || actionStagnation; break; } case GeminiEventType.Content: { @@ -162,6 +173,31 @@ export class LoopDetectionService { return this.loopDetected; } + addAndCheckDeterministicToolCallLoop( + event: ServerGeminiStreamEvent, + ): boolean { + if (this.loopDetected) { + return true; + } + + if (event.type === GeminiEventType.Retry) { + this.resetToolCallCount(); + return false; + } + + if ( + this.disabledForSession || + event.type !== GeminiEventType.ToolCallRequest + ) { + return false; + } + + if (this.checkToolCallLoop(event.value)) { + this.loopDetected = true; + } + return this.loopDetected; + } + private checkToolCallLoop(toolCall: { name: string; args: object }): boolean { const key = this.getToolCallKey(toolCall); if (this.lastToolCallKey === key) {