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

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

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/agent-core-v2": 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-v2/docs/state-manifest.d.ts

@@ -1010,7 +1010,7 @@ export interface AgentStateSnapshot {
   'llmRequester.lastConfigLogSignature': string | undefined;
   'llmRequester.mediaDegradedTurns': Set<number>;
   'llmRequester.mediaStrippedTurns': Map<number, /* MediaStripSnapshot — packages/agent-core-v2/src/agent/contextProjector/contextProjector.ts */ {
-    readonly "__@mediaStripSnapshotBrand@2667": undefined;
+    readonly "__@mediaStripSnapshotBrand@2671": undefined;
   }>;
   'llmRequester.turnConfigs': Map<number, /* TurnRequestConfig — packages/agent-core-v2/src/agent/llmRequester/llmRequesterService.ts */ {
     readonly resolved: /* ProfileModelContext — packages/agent-core-v2/src/agent/profile/profile.ts */ {

packages/agent-core-v2/src/agent/toolDedupe/toolDedupeService.ts

@@ -24,6 +24,7 @@ import { canonicalTelemetryArgs } from '#/_base/utils/canonical-args';
 import type { ToolCallDedupDetectedEvent, ToolCallRepeatEvent } from '#/app/telemetry/events';
 import { ITelemetryService } from '#/app/telemetry/telemetry';
 import type { LLMRequestTrace } from '#/kosong/contract/requestTrace';
+import { parseToolCallArguments } from '#/tool/tool-args-parse';
 import { IAgentLoopService } from '#/agent/loop/loop';
 import { IAgentStateService } from '#/agent/state/agentState';
 import { IAgentToolExecutorService, type ToolCallDupType } from '#/agent/toolExecutor/toolExecutor';
@@ -179,6 +180,13 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
       }
     });
     toolExecutor.hooks.onDidExecuteTool.register('toolDedupe', async (ctx, next) => {
+      this.registerSkipped(
+        ctx.toolCall.id,
+        ctx.toolCall.name,
+        ctx.args,
+        ctx.toolCall.arguments,
+        ctx.trace,
+      );
       ctx.result = await this.finalizeResult(
         ctx.toolCall.id,
         ctx.toolCall.name,
@@ -305,6 +313,23 @@ export class AgentToolDedupeService extends Disposable implements IAgentToolDedu
     return { syntheticResult: null };
   }
 
+  private registerSkipped(
+    toolCallId: string,
+    toolName: string,
+    args: unknown,
+    rawArguments: unknown,
+    trace: LLMRequestTrace | undefined,
+  ): void {
+    if (this.callKeyByCallId.has(toolCallId)) return;
+    const keyArgs =
+      rawArguments !== undefined &&
+      rawArguments !== null &&
+      parseToolCallArguments(rawArguments).parseFailed
+        ? rawArguments
+        : args;
+    this.checkToolCall(toolCallId, toolName, keyArgs, trace);
+  }
+
   private recordDupType(
     toolCallId: string,
     toolName: string,

packages/agent-core-v2/src/agent/toolExecutor/toolExecutorService.ts

@@ -27,6 +27,7 @@ import {
   type JsonType,
   type ToolArgsValidator,
 } from '#/tool/args-validator';
+import { parseToolCallArguments } from '#/tool/tool-args-parse';
 import { PathSecurityError } from '#/tool/path-access';
 import { isAbortError, isUserCancellation } from '#/_base/utils/abort';
 import { IEventBus } from '#/app/event/eventBus';
@@ -595,17 +596,13 @@ export class AgentToolExecutorService implements IAgentToolExecutorService {
     result: ToolResult,
     options: ToolExecutorExecuteOptions,
   ): Promise<ToolResult> {
-    if (call.kind === 'rejected') {
-      return result;
-    }
-
     const didCtx: ToolDidExecuteContext = {
       turnId: options.turnId,
       signal: options.signal,
       trace: options.trace,
       toolCall: call.toolCall,
       toolCalls: [call.toolCall],
-      tool: call.tool,
+      tool: call.kind === 'runnable' ? call.tool : undefined,
       args: call.args,
       result: result as ExecutableToolResult,
     };
@@ -756,24 +753,6 @@ function preflightToolCall(
   return { kind: 'runnable', toolCall, toolName, tool, args: parsedArgs.data };
 }
 
-export function parseToolCallArguments(raw: unknown): {
-  readonly data: unknown;
-  readonly parseFailed: boolean;
-  readonly error?: string;
-} {
-  if (raw === null || raw === undefined || (typeof raw === 'string' && raw.length === 0)) {
-    return { data: {}, parseFailed: false };
-  }
-  if (typeof raw !== 'string') {
-    return { data: raw, parseFailed: false };
-  }
-  try {
-    return { data: JSON.parse(raw) as unknown, parseFailed: false };
-  } catch (error) {
-    return { data: {}, parseFailed: true, error: errorMessage(error) };
-  }
-}
-
 function validateExecutableToolArgs(tool: ExecutableTool, args: unknown): string | null {
   let validator = validators.get(tool);
   if (validator === undefined) {

packages/agent-core-v2/src/agent/toolExecutor/toolHooks.ts

@@ -18,7 +18,9 @@
  *   `waitUntil(promise)`; the executor awaits all of them before dispatching
  *   an allowed call (e.g. MCP initial load).
  * - `hooks.onDidExecuteTool` (ordered hook slot, `ToolDidExecuteContext`):
- *   post-execution result finalization, kept as an `OrderedHookSlot`.
+ *   post-execution result finalization, kept as an `OrderedHookSlot`. Every
+ *   call reaches it — including preflight-rejected ones (missing/unavailable
+ *   tool, guard denial, invalid args), which arrive without `tool` set.
  *
  * Participants such as `permissionGate`, `toolDedupe`, `externalHooks`,
  * `goal`, `plan`, `swarm`, `btw`, and `mcp` register through these surfaces.

packages/agent-core-v2/src/tool/tool-args-parse.ts

@@ -0,0 +1,30 @@
+/**
+ * `tool` domain (L3) — tool-call arguments parsing.
+ *
+ * Decodes the provider's raw `arguments` payload into a plain value. A
+ * payload that fails JSON parsing is normalized to `{}` and flagged with
+ * `parseFailed`, so callers can tell "the model sent an empty object" apart
+ * from "the model sent malformed text". Pure helper; no scoped service.
+ */
+
+export function parseToolCallArguments(raw: unknown): {
+  readonly data: unknown;
+  readonly parseFailed: boolean;
+  readonly error?: string;
+} {
+  if (raw === null || raw === undefined || (typeof raw === 'string' && raw.length === 0)) {
+    return { data: {}, parseFailed: false };
+  }
+  if (typeof raw !== 'string') {
+    return { data: raw, parseFailed: false };
+  }
+  try {
+    return { data: JSON.parse(raw) as unknown, parseFailed: false };
+  } catch (error) {
+    return {
+      data: {},
+      parseFailed: true,
+      error: error instanceof Error ? error.message : String(error),
+    };
+  }
+}

packages/agent-core-v2/test/agent/toolDedupe/toolDedupe.test.ts

@@ -1,4 +1,4 @@
-import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { DisposableStore } from '#/_base/di/lifecycle';
 import { createServices, type TestInstantiationService } from '#/_base/di/test';
@@ -8,8 +8,10 @@ import { type ToolCall } from '#/kosong/contract/message';
 import { emptyUsage } from '#/kosong/contract/usage';
 import { ITelemetryService } from '#/app/telemetry/telemetry';
 import { ISessionContext } from '#/session/sessionContext/sessionContext';
+import type { ISessionProcessRunner } from '#/session/process/processRunner';
 import { IAgentScopeContext } from '#/agent/scopeContext/scopeContext';
 import { IAgentLoopService } from '#/agent/loop/loop';
+import { IAgentProfileService } from '#/agent/profile/profile';
 import { IAgentStateService } from '#/agent/state/agentState';
 import { AgentStateService } from '#/agent/state/agentStateService';
 import type { ExecutableTool, ExecutableToolContext, ExecutableToolResult, ToolExecution, ToolResult } from '#/tool/toolContract';
@@ -26,6 +28,8 @@ import { stubLoopWithHooks } from '../loop/stubs';
 import { stubToolExecutorEvents } from '../toolExecutor/stubs';
 import { registerToolResultTruncationServices } from '../toolResultTruncation/stubs';
 import { registerTestAgentWireServices } from '../../wire/stubs';
+import { createTestAgent, execEnvServices, telemetryServices } from '../../harness';
+import { createFakeProcessRunner } from '../../tools/fixtures/fake-exec';
 
 const { REMINDER_TEXT_1, REMINDER_TEXT_3, makeReminderText2 } = toolDedupeTesting;
 const ZERO_USAGE = emptyUsage();
@@ -832,4 +836,166 @@ describe('AgentToolDedupeService', () => {
       expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
     });
   });
+
+  describe('preflight-rejected calls (bypass onBeforeExecuteTool)', () => {
+    // Calls rejected by args validation in preflight never fire
+    // onBeforeExecuteTool; the dedupe hook registers them late at
+    // onDidExecuteTool time so the repeat breaker still counts them.
+    class StrictTool implements ExecutableTool<Record<string, unknown>> {
+      readonly name = 'Strict';
+      readonly description = 'Requires a command string.';
+      readonly parameters = {
+        type: 'object',
+        properties: { command: { type: 'string' } },
+        required: ['command'],
+        additionalProperties: true,
+      };
+      readonly calls: Array<Record<string, unknown>> = [];
+
+      resolveExecution(args: Record<string, unknown>): ToolExecution {
+        return {
+          approvalRule: this.name,
+          execute: async () => {
+            this.calls.push(args);
+            return { output: 'ran' };
+          },
+        };
+      }
+    }
+
+    function invalidCall(id: string): ToolCall {
+      // Missing the required "command".
+      return { type: 'function', id, name: 'Strict', arguments: JSON.stringify({ timeout: 60 }) };
+    }
+
+    function malformedCall(id: string, rawArguments: string): ToolCall {
+      return { type: 'function', id, name: 'Strict', arguments: rawArguments };
+    }
+
+    it('counts rejected calls toward the streak and force-stops at 12, keeping the error flag', async () => {
+      const h = createHarness();
+      const tool = new StrictTool();
+      h.registry.register(tool);
+      let last: ToolResult | undefined;
+      for (let i = 0; i < 12; i += 1) {
+        const [result] = await runStep(h, 1, i + 1, [invalidCall(`c${String(i)}`)]);
+        last = result!.result;
+      }
+      expect(tool.calls).toHaveLength(0);
+      expect(last!.isError).toBe(true);
+      expect(last!.stopTurn).toBe(true);
+      expect(last!.output as string).toContain(REMINDER_TEXT_3.trim());
+      const actions = telemetryEvents
+        .filter((e) => e.event === 'tool_call_repeat')
+        .map((e) => e.properties?.['action']);
+      expect(actions).toEqual(['none', 'r1', 'r1', 'r2', 'r2', 'r2', 'r3', 'r3', 'r3', 'r3', 'stop']);
+    });
+
+    it('does not double-register a call that already went through onBeforeExecuteTool', async () => {
+      const h = createHarness(undefined, { executorEvents: true });
+      for (let i = 0; i < 2; i += 1) {
+        await beforeStep(h, 1, i + 1);
+        const callId = `c${String(i)}`;
+        expect(await h.fireBefore(willCtx(callId, 'Read', { p: 1 }))).toBeUndefined();
+        const d = didCtx(callId, 'Read', { p: 1 }, okResult('R'));
+        await h.executor.hooks.onDidExecuteTool.run(d);
+        await afterStep(h, 1, i + 1);
+      }
+      // Exactly one repeat at count 2 — a double registration would inflate
+      // the streak and fire the reminder one occurrence early.
+      const repeats = telemetryEvents.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 h = createHarness();
+      h.registry.register(new StrictTool());
+      for (let i = 0; i < 2; i += 1) {
+        await runStep(h, 1, i + 1, [malformedCall(`c${String(i)}`, '{"command":')]);
+      }
+      const repeats = telemetryEvents.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 h = createHarness();
+      h.registry.register(new StrictTool());
+      const raws = ['{"command":', '{"comand":', '{"command": "ls"'];
+      for (let i = 0; i < 3; i += 1) {
+        await runStep(h, 1, i + 1, [malformedCall(`c${String(i)}`, raws[i]!)]);
+      }
+      // All three normalize to {} on parse failure, but the raw texts
+      // differ, so no repeat streak may form.
+      expect(telemetryEvents.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
+    });
+  });
+
+  describe('turn-level repeat breaker for rejected calls', () => {
+    function invalidBashCallWithId(id: string): ToolCall {
+      // Missing the required "command".
+      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 rejectedBashAgent(records: TelemetryRecord[]): {
+      readonly ctx: ReturnType<typeof createTestAgent>;
+      readonly exec: ReturnType<typeof vi.fn>;
+    } {
+      const exec = vi.fn<ISessionProcessRunner['exec']>().mockRejectedValue(new Error('Bash should not execute'));
+      const ctx = createTestAgent(
+        telemetryServices(recordingTelemetry(records)),
+        execEnvServices({ processRunner: createFakeProcessRunner({ exec: exec as unknown as ISessionProcessRunner['exec'] }) }),
+      );
+      ctx.get(IAgentProfileService).update({ activeToolNames: ['Bash'] });
+      records.length = 0;
+      return { ctx, exec };
+    }
+
+    it('force-stops a turn that keeps re-issuing the same validation-rejected call', async () => {
+      const records: TelemetryRecord[] = [];
+      const { ctx, exec } = rejectedBashAgent(records);
+
+      // 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(exec).not.toHaveBeenCalled();
+      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, exec } = rejectedBashAgent(records);
+
+      // 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(exec).not.toHaveBeenCalled();
+      expect(ctx.llmCalls).toHaveLength(13);
+      expect(records.filter((entry) => entry.event === 'tool_call_repeat')).toHaveLength(0);
+    });
+  });
 });

packages/agent-core-v2/test/agent/toolExecutor/toolExecutor.test.ts

@@ -17,7 +17,8 @@ import {
 } from '#/tool/toolContract';
 import { IAgentToolExecutorService } from '#/agent/toolExecutor/toolExecutor';
 import type { BeforeToolExecuteEvent } from '#/agent/toolExecutor/toolHooks';
-import { AgentToolExecutorService, parseToolCallArguments } from '#/agent/toolExecutor/toolExecutorService';
+import { AgentToolExecutorService } from '#/agent/toolExecutor/toolExecutorService';
+import { parseToolCallArguments } from '#/tool/tool-args-parse';
 import { IAgentToolResultTruncationService } from '#/agent/toolResultTruncation/toolResultTruncation';
 import { makeAgentScopeContext, IAgentScopeContext } from '#/agent/scopeContext/scopeContext';
 import { IAgentToolRegistryService } from '#/agent/toolRegistry/toolRegistry';

packages/agent-core-v2/test/agent/toolSelect/toolSelectService.test.ts

@@ -857,6 +857,7 @@ describe('AgentToolSelectService executor interception', () => {
         `Tool "${MCP_ALPHA}" is available but not loaded. ` +
         `Call select_tools with ["${MCP_ALPHA}"] first, then call the tool.`,
       isError: true,
+      stopTurn: false,
     });
     expect(alpha.calls).toBe(0);
   });
@@ -887,6 +888,7 @@ describe('AgentToolSelectService executor interception', () => {
       output:
         `Tool "${MCP_ALPHA}" was loaded but is no longer active. Ask the user to enable it before calling it again.`,
       isError: true,
+      stopTurn: false,
     });
     expect(alpha.calls).toBe(0);
   });