fix(agent-core): count validation-rejected tool calls toward the repeat breaker

.changeset/dedup-register-rejected-calls.md

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/agent-core": patch
+---
+
+Count validation-rejected tool calls toward the repeat breaker so reminders fire at 3/5/8 and the turn force-stops at 12.

packages/agent-core/src/agent/turn/index.ts

@@ -978,6 +978,16 @@ export class TurnFlow {
               return this.agent.permission.beforeToolCall(ctx);
             },
             finalizeToolResult: async (ctx) => {
+              // Calls rejected in preflight (e.g. invalid args) never reach
+              // prepareToolExecution, so register them here — otherwise the
+              // repeat breaker cannot count them and the model can re-issue
+              // the same invalid call indefinitely.
+              deduper.registerSkipped(
+                ctx.toolCall.id,
+                ctx.toolCall.name,
+                ctx.args,
+                ctx.toolCall.arguments,
+              );
               // Resolve dedup BEFORE firing the PostToolUse hook so same-step
               // dups (whose ctx.result is the dedup placeholder) report the
               // original's real outcome, not an empty success.

packages/agent-core/src/agent/turn/tool-dedup.ts

@@ -2,6 +2,7 @@ import type { ContentPart } from '@moonshot-ai/kosong';
 
 import type { TelemetryClient } from '../../telemetry';
 import type { LLMRequestTrace } from '../../loop/llm';
+import { parseToolCallArguments } from '../../loop/tool-args-parse';
 import type { ExecutableToolResult } from '../../loop/types';
 
 import { canonicalTelemetryArgs } from './canonical-args';
@@ -190,6 +191,35 @@ export class ToolCallDeduplicator {
     return null;
   }
 
+  /**
+   * Register a call that bypassed `prepareToolExecution` — e.g. args
+   * validation rejected it in preflight, so the prepare hook never ran. Must
+   * be called before `finalizeResult` for such calls, otherwise the repeat
+   * circuit breaker never counts rejected calls and the model can re-issue
+   * the same invalid call without ever tripping the streak. No-op when the
+   * call was already registered through the normal prepare path.
+   *
+   * `rawArguments` is the provider's raw arguments string. Args that failed
+   * JSON parsing were normalized to `{}` by the loop, which would key every
+   * malformed-but-different attempt identically; those are keyed on the raw
+   * text so only true re-issues count as repeats.
+   */
+  registerSkipped(
+    toolCallId: string,
+    toolName: string,
+    args: unknown,
+    rawArguments?: string | null,
+  ): void {
+    if (this.callKeyByCallId.has(toolCallId)) return;
+    const keyArgs =
+      rawArguments !== undefined &&
+      rawArguments !== null &&
+      parseToolCallArguments(rawArguments).parseFailed
+        ? rawArguments
+        : args;
+    this.checkSameStep(toolCallId, toolName, keyArgs);
+  }
+
   /**
    * Called from `finalizeToolResult`, in provider order. For first-occurrence
    * calls, projects the consecutive streak ending at this call and, if the

packages/agent-core/test/agent/turn.test.ts

@@ -793,6 +793,61 @@ describe('Agent turn flow', () => {
     });
   });
 
+  it('force-stops a turn that keeps re-issuing the same validation-rejected call', async () => {
+    const records: TelemetryRecord[] = [];
+    const ctx = testAgent({
+      kaos: createCommandKaos('bad'),
+      telemetry: recordingTelemetry(records),
+    });
+    ctx.configure({ tools: ['Bash'] });
+    await ctx.rpc.setPermission({ mode: 'yolo' });
+    records.length = 0;
+
+    // 12 identical calls missing the required "command": each is rejected in
+    // preflight. If the breaker did not count them, the turn would keep going
+    // and consume the 13th scripted response.
+    for (let i = 0; i < 12; i += 1) {
+      ctx.mockNextResponse(invalidBashCallWithId(`call_bad_${String(i)}`));
+    }
+    ctx.mockNextResponse({ type: 'text', text: 'must never be generated' });
+
+    await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Repeat the bad call' }] });
+    await ctx.untilTurnEnd();
+
+    expect(ctx.llmCalls).toHaveLength(12);
+    const actions = records
+      .filter((entry) => entry.event === 'tool_call_repeat')
+      .map((entry) => entry.properties?.['action']);
+    expect(actions).toEqual([
+      'none', 'r1', 'r1', 'r2', 'r2', 'r2', 'r3', 'r3', 'r3', 'r3', 'stop',
+    ]);
+  });
+
+  it('does not force-stop when the malformed argument text keeps changing', async () => {
+    const records: TelemetryRecord[] = [];
+    const ctx = testAgent({
+      kaos: createCommandKaos('bad'),
+      telemetry: recordingTelemetry(records),
+    });
+    ctx.configure({ tools: ['Bash'] });
+    await ctx.rpc.setPermission({ mode: 'yolo' });
+    records.length = 0;
+
+    // 12 rejected calls, each with DIFFERENT malformed raw JSON: all normalize
+    // to {} on parse failure, but they are not repeats of the same call, so
+    // the turn must not be force-stopped.
+    for (let i = 0; i < 12; i += 1) {
+      ctx.mockNextResponse(malformedBashCallWithId(`call_mal_${String(i)}`, i));
+    }
+    ctx.mockNextResponse({ type: 'text', text: 'recovered' });
+
+    await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Repeat the bad call' }] });
+    await ctx.untilTurnEnd();
+
+    expect(ctx.llmCalls).toHaveLength(13);
+    expect(records.filter((entry) => entry.event === 'tool_call_repeat')).toHaveLength(0);
+  });
+
   it('fires PostToolUse for same-step dups with the original real output, not the dedup placeholder', async () => {
     // Hook command asserts the dup's PostToolUse payload carries the real
     // stdout ('dup'), not the placeholder ('').
@@ -2799,6 +2854,25 @@ function bashCallWithId(id: string, command: string): ToolCall {
   };
 }
 
+function invalidBashCallWithId(id: string): ToolCall {
+  return {
+    type: 'function',
+    id,
+    name: 'Bash',
+    arguments: JSON.stringify({ timeout: 60 }),
+  };
+}
+
+function malformedBashCallWithId(id: string, variant: number): ToolCall {
+  // Invalid JSON (unquoted key), unique per variant.
+  return {
+    type: 'function',
+    id,
+    name: 'Bash',
+    arguments: `{"command_${String(variant)}: "ls"`,
+  };
+}
+
 function agentSwarmCall(): ToolCall {
   return {
     type: 'function',

packages/agent-core/test/agent/turn/tool-dedup.test.ts

@@ -573,4 +573,78 @@ describe('ToolCallDeduplicator', () => {
       expect(true).toBe(true);
     });
   });
+
+  describe('skipped registration (calls rejected before prepareToolExecution)', () => {
+    // Calls rejected in preflight (e.g. invalid args) never reach
+    // prepareToolExecution/checkSameStep; the loop registers them late via
+    // registerSkipped at finalize time so the breaker still counts them.
+    async function runRejected(
+      dedup: ToolCallDeduplicator,
+      callId: string,
+      result: ExecutableToolResult,
+    ): Promise<ExecutableToolResult> {
+      dedup.registerSkipped(callId, 'Bash', { timeout: 60 });
+      return dedup.finalizeResult(callId, 'Bash', { timeout: 60 }, result);
+    }
+
+    it('counts skipped calls toward the streak and force-stops at 12, keeping the error flag', async () => {
+      const dedup = new ToolCallDeduplicator();
+      let last: ExecutableToolResult | undefined;
+      for (let i = 0; i < 12; i += 1) {
+        dedup.beginStep();
+        last = await runRejected(dedup, `c${String(i)}`, errResult('Invalid args'));
+        dedup.endStep();
+      }
+      expect(last!.isError).toBe(true);
+      expect(last!.stopTurn).toBe(true);
+      expect(last!.output as string).toContain(REMINDER_TEXT_3.trim());
+    });
+
+    it('does not double-register a call that already went through checkSameStep', async () => {
+      const { client, events } = makeRecordingTelemetry();
+      const dedup = new ToolCallDeduplicator({ telemetry: client });
+      for (let i = 0; i < 2; i += 1) {
+        dedup.beginStep();
+        const callId = `c${String(i)}`;
+        expect(dedup.checkSameStep(callId, 'Read', { p: 1 })).toBeNull();
+        dedup.registerSkipped(callId, 'Read', { p: 1 });
+        await dedup.finalizeResult(callId, 'Read', { p: 1 }, okResult('R'));
+        dedup.endStep();
+      }
+      // Exactly one repeat at count 2 — a double registration would inflate
+      // the streak and fire the reminder one occurrence early.
+      const repeats = events.filter((e) => e.event === 'tool_call_repeat');
+      expect(repeats.map((e) => e.properties?.['repeat_count'])).toEqual([2]);
+    });
+
+    it('counts identical malformed argument texts as repeats', async () => {
+      const { client, events } = makeRecordingTelemetry();
+      const dedup = new ToolCallDeduplicator({ telemetry: client });
+      for (let i = 0; i < 2; i += 1) {
+        dedup.beginStep();
+        const callId = `c${String(i)}`;
+        dedup.registerSkipped(callId, 'Bash', {}, '{"command":');
+        await dedup.finalizeResult(callId, 'Bash', {}, errResult('Invalid args'));
+        dedup.endStep();
+      }
+      const repeats = events.filter((e) => e.event === 'tool_call_repeat');
+      expect(repeats.map((e) => e.properties?.['repeat_count'])).toEqual([2]);
+    });
+
+    it('does not treat different malformed argument texts as the same call', async () => {
+      const { client, events } = makeRecordingTelemetry();
+      const dedup = new ToolCallDeduplicator({ telemetry: client });
+      const raws = ['{"command":', '{"comand":', '{"command": "ls"'];
+      for (let i = 0; i < 3; i += 1) {
+        dedup.beginStep();
+        const callId = `c${String(i)}`;
+        dedup.registerSkipped(callId, 'Bash', {}, raws[i]);
+        await dedup.finalizeResult(callId, 'Bash', {}, errResult('Invalid args'));
+        dedup.endStep();
+      }
+      // All three normalize to {} on parse failure, but the raw texts differ,
+      // so no repeat streak may form.
+      expect(events.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
+    });
+  });
 });