fix(acp): stop repeated invalid tool parameter loops (#6076)

packages/cli/src/acp-integration/session/Session.test.ts

@@ -2347,6 +2347,87 @@ describe('Session', () => {
         ).toContain('Duplicate provider tool call id "shell_1"');
       });
 
+      it('stops an ACP prompt after repeated invalid tool parameters with fresh ids', async () => {
+        mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
+        const build = vi.fn().mockImplementation(() => {
+          throw new Error('Parameter "questions" must be an array.');
+        });
+        mockToolRegistry.getTool.mockReturnValue({
+          name: 'ask_user_question',
+          kind: core.Kind.Other,
+          displayName: 'Ask User Question',
+          description: 'Ask user question',
+          build,
+          canUpdateOutput: false,
+          isOutputMarkdown: true,
+        });
+
+        mockChat.sendMessageStream = vi
+          .fn()
+          .mockResolvedValueOnce(
+            createStreamWithChunks([
+              {
+                type: core.StreamEventType.CHUNK,
+                value: {
+                  functionCalls: [
+                    {
+                      id: 'ask_1',
+                      name: 'ask_user_question',
+                      args: { questions: '[{"question":"Continue?"}]' },
+                    },
+                  ],
+                },
+              },
+            ]),
+          )
+          .mockResolvedValueOnce(
+            createStreamWithChunks([
+              {
+                type: core.StreamEventType.CHUNK,
+                value: {
+                  functionCalls: [
+                    {
+                      id: 'ask_2',
+                      name: 'ask_user_question',
+                      args: { questions: '[{"question":"Continue?"}]' },
+                    },
+                  ],
+                },
+              },
+            ]),
+          )
+          .mockResolvedValueOnce(
+            createStreamWithChunks([
+              {
+                type: core.StreamEventType.CHUNK,
+                value: {
+                  functionCalls: [
+                    {
+                      id: 'ask_3',
+                      name: 'ask_user_question',
+                      args: { questions: '[{"question":"Continue?"}]' },
+                    },
+                  ],
+                },
+              },
+            ]),
+          )
+          .mockResolvedValueOnce(createEmptyStream());
+
+        await session.prompt({
+          sessionId: 'test-session-id',
+          prompt: [{ type: 'text', text: 'ask me before continuing' }],
+        });
+
+        expect(build).toHaveBeenCalledTimes(3);
+        expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(3);
+        expect(debugLoggerWarnSpy).toHaveBeenCalledWith(
+          expect.stringContaining(
+            'Stopping ACP turn after repeated tool parameter errors',
+          ),
+        );
+      });
+
       it('clears duplicate provider id tracking between ACP prompts', async () => {
         mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
         vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue(

packages/cli/src/acp-integration/session/Session.ts

@@ -106,6 +106,9 @@ import {
   endToolExecutionSpan,
   logConversationFinishedEvent,
   ConversationFinishedEvent,
+  logLoopDetected,
+  LoopDetectedEvent,
+  LoopType,
   acquireSleepInhibitor,
   clearGoalTerminalObserver,
   setGoalTerminalObserver,
@@ -209,11 +212,85 @@ type RunToolResult = {
   parts: Part[];
   stopAfterPermissionCancel: boolean;
   repeatedDuplicateProviderToolCall?: boolean;
+  loopDetected?: boolean;
 };
 
+type DaemonToolLoopState = {
+  totalToolCalls: number;
+  invalidToolParamErrors: Map<string, number>;
+  loopDetected: boolean;
+};
+
+const DAEMON_TURN_TOOL_CALL_CAP = 100;
+const DAEMON_INVALID_TOOL_PARAMS_THRESHOLD = 3;
+
 const PERMISSION_CANCEL_SKIP_MESSAGE =
   'Skipped because a permission request was cancelled before the user answered; user input is required before continuing.';
 
+function createDaemonToolLoopState(): DaemonToolLoopState {
+  return {
+    totalToolCalls: 0,
+    invalidToolParamErrors: new Map(),
+    loopDetected: false,
+  };
+}
+
+function recordDaemonLoopDetected(
+  config: Config,
+  promptId: string,
+  loopType: LoopType,
+  message: string,
+  loopState: DaemonToolLoopState,
+): true {
+  if (!loopState.loopDetected) {
+    loopState.loopDetected = true;
+    debugLogger.warn(message);
+    logLoopDetected(config, new LoopDetectedEvent(loopType, promptId));
+  }
+  return true;
+}
+
+function recordDaemonToolCalls(
+  config: Config,
+  promptId: string,
+  loopState: DaemonToolLoopState | undefined,
+  count: number,
+): boolean {
+  if (!loopState || loopState.loopDetected)
+    return loopState?.loopDetected ?? false;
+  loopState.totalToolCalls += count;
+  if (loopState.totalToolCalls <= DAEMON_TURN_TOOL_CALL_CAP) return false;
+  return recordDaemonLoopDetected(
+    config,
+    promptId,
+    LoopType.TURN_TOOL_CALL_CAP,
+    `Stopping ACP turn after ${loopState.totalToolCalls} tool calls in one turn.`,
+    loopState,
+  );
+}
+
+function recordDaemonInvalidToolParams(
+  config: Config,
+  promptId: string,
+  loopState: DaemonToolLoopState | undefined,
+  toolName: string,
+  error: Error,
+): boolean {
+  if (!loopState || loopState.loopDetected)
+    return loopState?.loopDetected ?? false;
+  const key = `${toolName}\0${error.message}`;
+  const count = (loopState.invalidToolParamErrors.get(key) ?? 0) + 1;
+  loopState.invalidToolParamErrors.set(key, count);
+  if (count < DAEMON_INVALID_TOOL_PARAMS_THRESHOLD) return false;
+  return recordDaemonLoopDetected(
+    config,
+    promptId,
+    LoopType.INVALID_TOOL_PARAMS_STAGNATION,
+    `Stopping ACP turn after repeated tool parameter errors from ${toolName}: ${error.message}`,
+    loopState,
+  );
+}
+
 // The drain is served from an in-memory queue, so a conforming client answers
 // near-instantly (or rejects with -32601). No response within this window
 // means the client silently drops unknown methods; without a deadline the
@@ -1653,6 +1730,7 @@ export class Session implements SessionContext {
 
             let nextMessage: Content | null = { role: 'user', parts };
             let turnCount = 0;
+            const toolLoopState = createDaemonToolLoopState();
 
             // conversation_finished must fire on every terminal path of the
             // turn — the loop below has cancel/abort/no-stream early-returns
@@ -1820,6 +1898,7 @@ export class Session implements SessionContext {
                     pendingSend.signal,
                     promptId,
                     functionCalls,
+                    toolLoopState,
                   );
                   if (toolRun.stopAfterPermissionCancel) {
                     await this.#preserveCancelledPermissionToolRun(
@@ -1981,6 +2060,7 @@ export class Session implements SessionContext {
           role: 'user',
           parts: continueParts,
         };
+        const toolLoopState = createDaemonToolLoopState();
 
         // Process the follow-up message and any tool calls that result
         while (nextMessage !== null) {
@@ -2096,6 +2176,7 @@ export class Session implements SessionContext {
               pendingSend.signal,
               promptId,
               functionCalls,
+              toolLoopState,
             );
             if (toolRun.stopAfterPermissionCancel) {
               await this.#preserveCancelledPermissionToolRun(
@@ -2293,6 +2374,10 @@ export class Session implements SessionContext {
     toolRun: RunToolResult,
     abortSignal: AbortSignal,
   ): Promise<Content | null> {
+    if (toolRun.loopDetected) {
+      debugLogger.debug('Stopping ACP turn after daemon loop detection.');
+      return null;
+    }
     if (toolRun.repeatedDuplicateProviderToolCall) {
       debugLogger.debug(
         'Stopping ACP turn after dropping repeated duplicate provider tool-call response.',
@@ -2894,6 +2979,7 @@ export class Session implements SessionContext {
                 role: 'user',
                 parts: [...cronReminders, { text: modelText }],
               };
+              const toolLoopState = createDaemonToolLoopState();
 
               while (nextMessage !== null) {
                 turnCount++;
@@ -2982,6 +3068,7 @@ export class Session implements SessionContext {
                     ac.signal,
                     promptId,
                     functionCalls,
+                    toolLoopState,
                   );
                   if (toolRun.stopAfterPermissionCancel) {
                     await this.#preserveCancelledPermissionToolRun(
@@ -3197,6 +3284,7 @@ export class Session implements SessionContext {
             role: 'user',
             parts: [...notificationReminders, ...notificationParts],
           };
+          const toolLoopState = createDaemonToolLoopState();
 
           while (nextMessage !== null) {
             if (ac.signal.aborted) {
@@ -3297,6 +3385,7 @@ export class Session implements SessionContext {
                 ac.signal,
                 promptId,
                 functionCalls,
+                toolLoopState,
               );
               if (toolRun.stopAfterPermissionCancel) {
                 await this.#preserveCancelledPermissionToolRun(
@@ -3650,7 +3739,23 @@ export class Session implements SessionContext {
     abortSignal: AbortSignal,
     promptId: string,
     functionCalls: FunctionCall[],
+    toolLoopState?: DaemonToolLoopState,
   ): Promise<RunToolResult> {
+    if (
+      recordDaemonToolCalls(
+        this.config,
+        promptId,
+        toolLoopState,
+        functionCalls.length,
+      )
+    ) {
+      return {
+        parts: [],
+        stopAfterPermissionCancel: false,
+        loopDetected: true,
+      };
+    }
+
     const dedupedFunctionCalls = dedupeToolCallsById(functionCalls);
     type ExecutableBatch = {
       kind: 'execute';
@@ -3846,6 +3951,7 @@ export class Session implements SessionContext {
           promptId,
           calls[idx],
           onStopAfterPermissionCancel,
+          toolLoopState,
         )
           .then((r) => {
             results[idx] = r;
@@ -3898,9 +4004,18 @@ export class Session implements SessionContext {
           abortSignal.removeEventListener('abort', propagateAbort);
         }
         let shouldStop = false;
+        let shouldStopForLoop = false;
         for (const r of results) {
           parts.push(...r.parts);
           shouldStop ||= r.stopAfterPermissionCancel;
+          shouldStopForLoop ||= r.loopDetected === true;
+        }
+        if (shouldStopForLoop) {
+          return {
+            parts,
+            stopAfterPermissionCancel: false,
+            loopDetected: true,
+          };
         }
         if (shouldStop) {
           await appendSkippedAfter(parts, batch.calls[batch.calls.length - 1]);
@@ -3912,8 +4027,21 @@ export class Session implements SessionContext {
         }
       } else {
         for (const fc of batch.calls) {
-          const r = await this.runTool(abortSignal, promptId, fc);
+          const r = await this.runTool(
+            abortSignal,
+            promptId,
+            fc,
+            undefined,
+            toolLoopState,
+          );
           parts.push(...r.parts);
+          if (r.loopDetected) {
+            return {
+              parts,
+              stopAfterPermissionCancel: false,
+              loopDetected: true,
+            };
+          }
           if (r.stopAfterPermissionCancel) {
             await appendSkippedAfter(parts, fc);
             return {
@@ -3972,6 +4100,7 @@ export class Session implements SessionContext {
     promptId: string,
     fc: FunctionCall,
     onStopAfterPermissionCancel?: () => void,
+    toolLoopState?: DaemonToolLoopState,
   ): Promise<RunToolResult> {
     const callId = fc.id ?? `${fc.name}-${Date.now()}`;
     let args = (fc.args ?? {}) as Record<string, unknown>;
@@ -4110,8 +4239,10 @@ export class Session implements SessionContext {
         // Get approval mode for hook context (defined outside try for catch block access)
         const approvalMode = this.config.getApprovalMode();
 
+        let toolBuildSucceeded = false;
         try {
           const invocation = tool.build(args);
+          toolBuildSucceeded = true;
 
           // Production AgentTool always initializes `eventEmitter` on its
           // invocation (`agent.ts:392`). Be defensive about the `undefined`
@@ -4886,9 +5017,21 @@ export class Session implements SessionContext {
             errorType: undefined,
           });
 
+          const loopDetected =
+            !activeToolAbortSignal.aborted &&
+            !toolBuildSucceeded &&
+            recordDaemonInvalidToolParams(
+              this.config,
+              promptId,
+              toolLoopState,
+              toolName,
+              error,
+            );
+
           return {
             parts: errorResponse(error),
             stopAfterPermissionCancel: nestedPermissionCancelled,
+            loopDetected,
           };
         }
       }); // end runInToolSpanContext

packages/cli/src/nonInteractiveCli.ts

@@ -138,6 +138,8 @@ const LOOP_TYPE_LABELS: Record<LoopType, string> = {
     'the model alternated between the same two tool calls in a repeating pattern',
   [LoopType.TURN_TOOL_CALL_CAP]:
     'the model exceeded the maximum number of tool calls allowed in a single turn',
+  [LoopType.INVALID_TOOL_PARAMS_STAGNATION]:
+    'the model repeatedly sent invalid tool parameters without correcting them',
 };
 
 function formatLoopDetectedMessage(loopType: LoopType | undefined): string {
@@ -150,7 +152,8 @@ function formatLoopDetectedMessage(loopType: LoopType | undefined): string {
     loopType === LoopType.TURN_TOOL_CALL_CAP ||
     loopType === LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS ||
     loopType === LoopType.SHELL_COMMAND_STAGNATION ||
-    loopType === LoopType.GLOBAL_TOOL_CALL_DUPLICATE;
+    loopType === LoopType.GLOBAL_TOOL_CALL_DUPLICATE ||
+    loopType === LoopType.INVALID_TOOL_PARAMS_STAGNATION;
   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.';

packages/core/src/index.ts

@@ -346,6 +346,7 @@ export {
   logExtensionDisable,
   logExtensionEnable,
   logIdeConnection,
+  logLoopDetected,
   logModelSlashCommand,
   logPromptSuggestion,
   logSpeculation,
@@ -360,6 +361,7 @@ export {
   ExtensionUninstallEvent,
   IdeConnectionEvent,
   IdeConnectionType,
+  LoopDetectedEvent,
   LoopType,
   ModelSlashCommandEvent,
   PromptSuggestionEvent,

packages/core/src/telemetry/types.ts

@@ -443,6 +443,8 @@ export enum LoopType {
   ALTERNATING_TOOL_CALL_PATTERN = 'alternating_tool_call_pattern',
   /** Total tool calls in a single turn exceeded the always-on hard cap, regardless of pattern. */
   TURN_TOOL_CALL_CAP = 'turn_tool_call_cap',
+  /** The same tool repeatedly failed schema validation with fresh tool-call ids. */
+  INVALID_TOOL_PARAMS_STAGNATION = 'invalid_tool_params_stagnation',
 }
 
 export class LoopDetectedEvent implements BaseTelemetryEvent {