fix(core): hard-stop repeated identical tool calls (#5036)

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<GeminiChat> = {
+        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', () => {

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.

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);

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) {