feat(core): enhanced loop detection with stagnation + validation-retry checks

packages/cli/src/nonInteractiveCli.ts

@@ -20,6 +20,7 @@ import {
   promptIdContext,
   OutputFormat,
   InputFormat,
+  LoopType,
   uiTelemetryService,
   parseAndFormatApiError,
   createDebugLogger,
@@ -51,6 +52,38 @@ import {
   computeUsageFromMetrics,
 } from './utils/nonInteractiveHelpers.js';
 
+// Human-readable labels for the detectors that can fire mid-stream.
+// Surfaced to stderr in TEXT mode so a headless run that halts on a loop
+// doesn't exit with empty stdout and no explanation — see PR #3236 review.
+const LOOP_TYPE_LABELS: Record<LoopType, string> = {
+  [LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS]:
+    'the model repeated the same tool call with identical arguments',
+  [LoopType.CHANTING_IDENTICAL_SENTENCES]:
+    'the model repeated the same sentence in its output',
+  [LoopType.REPETITIVE_THOUGHTS]:
+    'the model repeated the same reasoning thought',
+  [LoopType.READ_FILE_LOOP]:
+    'the model spent too many consecutive calls reading files without making progress',
+  [LoopType.ACTION_STAGNATION]:
+    'the model kept calling the same tool without making progress',
+};
+
+function emitLoopDetectedMessage(
+  config: Config,
+  loopType: LoopType | undefined,
+): void {
+  // In TEXT mode the adapter swallows LoopDetected, so we print here. In
+  // JSON modes the adapter emits a structured result, which is enough.
+  if (config.getOutputFormat() !== OutputFormat.TEXT) {
+    return;
+  }
+  const reason = loopType ? LOOP_TYPE_LABELS[loopType] : undefined;
+  const detail = reason ? ` (${loopType}: ${reason})` : '';
+  process.stderr.write(
+    `Loop detection halted the run${detail}. Set the \`model.skipLoopDetection\` setting to true to disable.\n`,
+  );
+}
+
 /**
  * Emits a final message for slash command results.
  * Note: systemMessage should already be emitted before calling this function.
@@ -340,6 +373,9 @@ export async function runNonInteractive(
           if (event.type === GeminiEventType.ToolCallRequest) {
             toolCallRequests.push(event.value);
           }
+          if (event.type === GeminiEventType.LoopDetected) {
+            emitLoopDetectedMessage(config, event.value?.loopType);
+          }
           if (
             outputFormat === OutputFormat.TEXT &&
             event.type === GeminiEventType.Error
@@ -506,6 +542,9 @@ export async function runNonInteractive(
                 if (event.type === GeminiEventType.ToolCallRequest) {
                   itemToolCallRequests.push(event.value);
                 }
+                if (event.type === GeminiEventType.LoopDetected) {
+                  emitLoopDetectedMessage(config, event.value?.loopType);
+                }
                 if (
                   outputFormat === OutputFormat.TEXT &&
                   event.type === GeminiEventType.Error

packages/core/src/core/client.ts

@@ -883,7 +883,11 @@ export class GeminiClient {
     for await (const event of resultStream) {
       if (!this.config.getSkipLoopDetection()) {
         if (this.loopDetector.addAndCheck(event)) {
-          yield { type: GeminiEventType.LoopDetected };
+          const loopType = this.loopDetector.getLastLoopType();
+          yield {
+            type: GeminiEventType.LoopDetected,
+            ...(loopType && { value: { loopType } }),
+          };
           if (arenaAgentClient) {
             await arenaAgentClient.reportError('Loop detected');
           }

packages/core/src/core/coreToolScheduler.test.ts

@@ -4110,4 +4110,150 @@ describe('CoreToolScheduler validation retry loop detection', () => {
     expect(msg).toBeDefined();
     expect(msg).not.toContain(RETRY_LOOP_STOP_DIRECTIVE);
   });
+
+  it('should isolate retry counters per-tool across batches', async () => {
+    // Regression: the batch-level continues-loop check used to keep *all*
+    // retry state whenever any current request matched a previously failing
+    // tool. That let stale counts for an unrelated tool survive long enough
+    // to fire RETRY LOOP DETECTED prematurely the next time that tool was
+    // called. The correct behaviour prunes counters per-tool: keep only
+    // counters whose tool name actually appears in the current batch.
+    class StrictToolAlt extends BaseDeclarativeTool<
+      { other: string },
+      ToolResult
+    > {
+      static readonly Name = 'strictStringToolAlt';
+      constructor() {
+        super(
+          StrictToolAlt.Name,
+          'StrictStringToolAlt',
+          'Alt tool requiring string other param.',
+          Kind.Other,
+          {
+            type: 'object',
+            properties: { other: { type: 'string' } },
+            required: ['other'],
+          },
+        );
+      }
+      protected createInvocation(params: {
+        other: string;
+      }): ToolInvocation<{ other: string }, ToolResult> {
+        return new (class extends BaseToolInvocation<
+          { other: string },
+          ToolResult
+        > {
+          constructor(p: { other: string }) {
+            super(p);
+          }
+          getDescription() {
+            return 'strictStringToolAlt invocation';
+          }
+          async execute(): Promise<ToolResult> {
+            return { llmContent: 'ok', returnDisplay: 'ok' };
+          }
+        })(params);
+      }
+    }
+
+    const toolA = new StrictStringTool();
+    const toolB = new StrictToolAlt();
+    const mockToolRegistry = {
+      ensureTool: async (name: string) =>
+        name === StrictStringTool.Name
+          ? toolA
+          : name === StrictToolAlt.Name
+            ? toolB
+            : undefined,
+      getTool: (name: string) =>
+        name === StrictStringTool.Name
+          ? toolA
+          : name === StrictToolAlt.Name
+            ? toolB
+            : undefined,
+      getFunctionDeclarations: () => [],
+      tools: new Map(),
+      discovery: {},
+      registerTool: () => {},
+      getToolByName: (name: string) =>
+        name === StrictStringTool.Name
+          ? toolA
+          : name === StrictToolAlt.Name
+            ? toolB
+            : undefined,
+      getToolByDisplayName: () => undefined,
+      getTools: () => [],
+      discoverTools: async () => {},
+      getAllTools: () => [],
+      getAllToolNames: () => [StrictStringTool.Name, StrictToolAlt.Name],
+      getToolsByServer: () => [],
+    } as unknown as ToolRegistry;
+
+    const mockConfig = {
+      getSessionId: () => 'test-session-id',
+      getUsageStatisticsEnabled: () => true,
+      getDebugMode: () => false,
+      getApprovalMode: () => ApprovalMode.YOLO,
+      getPermissionsAllow: () => [],
+      getContentGeneratorConfig: () => ({
+        model: 'test-model',
+        authType: 'gemini',
+      }),
+      getShellExecutionConfig: () => ({
+        terminalWidth: 90,
+        terminalHeight: 30,
+      }),
+      storage: { getProjectTempDir: () => '/tmp' },
+      getTruncateToolOutputThreshold: () => 100,
+      getTruncateToolOutputLines: () => 10,
+      getToolRegistry: () => mockToolRegistry,
+      getUseModelRouter: () => false,
+      getGeminiClient: () => null,
+      isInteractive: () => true,
+      getIdeMode: () => false,
+      getExperimentalZedIntegration: () => false,
+      getChatRecordingService: () => undefined,
+      getMessageBus: vi.fn().mockReturnValue(undefined),
+      getDisableAllHooks: vi.fn().mockReturnValue(true),
+      setApprovalMode: vi.fn(),
+    } as unknown as Config;
+
+    const onToolCallsUpdate = vi.fn();
+    const scheduler = new CoreToolScheduler({
+      config: mockConfig,
+      onAllToolCallsComplete: vi.fn(),
+      onToolCallsUpdate,
+      getPreferredEditor: () => 'vscode',
+      onEditorClose: vi.fn(),
+    });
+
+    // Tool A fails twice, accumulating a retry count of 2.
+    await scheduler.schedule(
+      [makeRequest('a1', StrictStringTool.Name, { value: 123 })],
+      new AbortController().signal,
+    );
+    await scheduler.schedule(
+      [makeRequest('a2', StrictStringTool.Name, { value: 123 })],
+      new AbortController().signal,
+    );
+
+    // Now a batch for tool B only — tool A's counter must be pruned because
+    // A is not present in this batch.
+    await scheduler.schedule(
+      [makeRequest('b1', StrictToolAlt.Name, { other: 456 })],
+      new AbortController().signal,
+    );
+
+    // Tool A fails once more. Under the old wholesale-keep behaviour this
+    // would be the third consecutive A failure and would trip the directive.
+    // Under per-tool pruning the counter starts fresh at 1 and no directive
+    // should be emitted.
+    await scheduler.schedule(
+      [makeRequest('a3', StrictStringTool.Name, { value: 123 })],
+      new AbortController().signal,
+    );
+    const msg = getLastErrorMessage(onToolCallsUpdate);
+    expect(msg).toBeDefined();
+    expect(msg).not.toContain(RETRY_LOOP_STOP_DIRECTIVE);
+  });
 });

packages/core/src/core/coreToolScheduler.ts

@@ -794,20 +794,20 @@ export class CoreToolScheduler {
       }
       const requestsToProcess = Array.isArray(request) ? request : [request];
 
-      // Check if this batch continues a validation retry loop.
-      // Keys are "<toolName>:<errorMessage>"; if no request reuses a tool name
-      // that previously failed validation, reset the tracker.
+      // Prune validation retry state per-tool, not wholesale. Keys are
+      // "<toolName>:<errorMessage>"; retain counters only for tools actually
+      // present in the current batch. Keeping every tracked tool's counters
+      // whenever any current request matched caused stale counts for
+      // unrelated tools to survive and fire RETRY LOOP DETECTED prematurely
+      // the next time those tools were used.
       if (this.validationRetryCounts.size > 0) {
-        const prevTools = new Set<string>();
-        for (const key of this.validationRetryCounts.keys()) {
+        const currentToolNames = new Set(requestsToProcess.map((r) => r.name));
+        for (const key of [...this.validationRetryCounts.keys()]) {
           const sep = key.indexOf(':');
-          prevTools.add(sep === -1 ? key : key.slice(0, sep));
-        }
-        const hasPrevFailingTool = requestsToProcess.some((r) =>
-          prevTools.has(r.name),
-        );
-        if (!hasPrevFailingTool) {
-          this.validationRetryCounts.clear();
+          const toolName = sep === -1 ? key : key.slice(0, sep);
+          if (!currentToolNames.has(toolName)) {
+            this.validationRetryCounts.delete(key);
+          }
         }
       }
 

packages/core/src/core/turn.ts

@@ -33,6 +33,7 @@ import {
   parseThought,
   type ThoughtSummary,
 } from '../utils/thoughtUtils.js';
+import type { LoopType } from '../telemetry/types.js';
 
 // Define a structure for tools passed to the server
 export interface ServerTool {
@@ -194,6 +195,12 @@ export type ServerGeminiFinishedEvent = {
 
 export type ServerGeminiLoopDetectedEvent = {
   type: GeminiEventType.LoopDetected;
+  // The loop type is optional so historical call sites that don't produce one
+  // (tests, fixtures) stay valid. Real emissions in client.ts always populate
+  // it so downstream consumers can surface a concrete reason to the user.
+  value?: {
+    loopType: LoopType;
+  };
 };
 
 export type ServerGeminiCitationEvent = {

packages/core/src/index.ts

@@ -227,6 +227,7 @@ export {
   ExtensionUninstallEvent,
   IdeConnectionEvent,
   IdeConnectionType,
+  LoopType,
   ModelSlashCommandEvent,
   PromptSuggestionEvent,
   SpeculationEvent,