From da6ba1584a9e90c716120c28f2f5d2c36633e9e8 Mon Sep 17 00:00:00 2001
From: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Date: Mon, 15 Jun 2026 14:24:46 +0800
Subject: [PATCH] fix(core): honor skipLoopDetection for the deterministic
 tool-call loop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#5036 carved the deterministic identical-tool-call check out of the
`model.skipLoopDetection` gate, turning it into a hard-stop that fires
even when loop detection is disabled. Because `skipLoopDetection`
defaults to true (settingsSchema: "to avoid false-positive
interruptions"), this silently re-enabled loop halts for the default
configuration and broke the documented escape hatch — the
non-interactive guidance in nonInteractiveCli.ts told users to set
`model.skipLoopDetection: true`, which no longer disabled the halt and
is unreachable in non-interactive mode (no disable dialog).

Gate both the deterministic and heuristic detector paths behind the
single flag again. The deterministic split, retry-reset, and pending
tool-call splice introduced by #5036 still apply once detection is
explicitly enabled (skipLoopDetection: false), so the runaway guard
remains available as opt-in without overriding the default-off contract.
---
 packages/core/src/core/client.test.ts | 50 ++++++++++++++++++++++++---
 packages/core/src/core/client.ts      | 11 +++++-
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts
index 4cecabd4403..60157fc06f6 100644
--- a/packages/core/src/core/client.test.ts
+++ b/packages/core/src/core/client.test.ts
@@ -6070,7 +6070,7 @@ Other open files:
       expect(mockCheckNextSpeaker).not.toHaveBeenCalled();
     });
 
-    it('keeps deterministic tool-call checks when skipLoopDetection is true', async () => {
+    it('does not run loop checks when skipLoopDetection is true', async () => {
       // Arrange
       // Ensure config returns true for skipLoopDetection
       vi.spyOn(client['config'], 'getSkipLoopDetection').mockReturnValue(true);
@@ -6106,13 +6106,14 @@ Other open files:
         // consume stream
       }
 
-      expect(ldMock.addAndCheckDeterministicToolCallLoop).toHaveBeenCalledTimes(
-        2,
-      );
+      // Assert - neither detector path runs when skipLoopDetection is true
+      expect(
+        ldMock.addAndCheckDeterministicToolCallLoop,
+      ).not.toHaveBeenCalled();
       expect(ldMock.addAndCheckHeuristicLoops).not.toHaveBeenCalled();
     });
 
-    it('hard-stops identical tool calls even when skipLoopDetection is true', async () => {
+    it('does not hard-stop identical tool calls when skipLoopDetection is true', async () => {
       vi.spyOn(client['config'], 'getSkipLoopDetection').mockReturnValue(true);
 
       mockTurnRunFn.mockReturnValue(
@@ -6144,6 +6145,45 @@ Other open files:
         ),
       );
 
+      // skipLoopDetection defaults to true, so even repeated identical calls
+      // must not be halted — the documented escape hatch stays effective.
+      expect(events.some((e) => e.type === GeminiEventType.LoopDetected)).toBe(
+        false,
+      );
+    });
+
+    it('hard-stops identical tool calls when loop detection is enabled', async () => {
+      vi.spyOn(client['config'], 'getSkipLoopDetection').mockReturnValue(false);
+
+      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-loop-identical',
+        ),
+      );
+
       expect(events.at(-1)).toEqual({
         type: GeminiEventType.LoopDetected,
         value: { loopType: LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS },
diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts
index 45a132ec122..7348fe55247 100644
--- a/packages/core/src/core/client.ts
+++ b/packages/core/src/core/client.ts
@@ -2097,11 +2097,20 @@ export class GeminiClient {
           didUpdateIdeContextState = true;
         }
 
+        // Loop detection is opt-in: `model.skipLoopDetection` defaults to true
+        // (see settingsSchema) to avoid false-positive interruptions. Keep BOTH
+        // the deterministic identical-tool-call check and the heuristic checks
+        // behind this single flag so the documented `model.skipLoopDetection`
+        // escape hatch stays honest (including the non-interactive hint in
+        // nonInteractiveCli.ts). The deterministic split, retry-reset, and
+        // pending-call splice below still apply once detection is enabled.
+        const skipLoopDetection = this.config.getSkipLoopDetection();
         const deterministicToolCallLoop =
+          !skipLoopDetection &&
           this.loopDetector.addAndCheckDeterministicToolCallLoop(event);
         const heuristicLoop =
           !deterministicToolCallLoop &&
-          !this.config.getSkipLoopDetection() &&
+          !skipLoopDetection &&
           this.loopDetector.addAndCheckHeuristicLoops(event);
         if (deterministicToolCallLoop || heuristicLoop) {
           const loopType = this.loopDetector.getLastLoopType();