mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-09 17:29:12 +00:00
feat(agent-core): detect and escalate repeated tool calls to prevent infinite loops (#364)
* feat(agent-core): add dead-end detection for repeated tool calls - inject stronger dead-end reminder at streak 10 and force-stop turn at streak 15 - emit `tool_call_repeat` telemetry events for streak >= 2 - pass telemetry client from TurnFlow to ToolCallDeduplicator - add comprehensive tests for dead-end stop and telemetry behavior * fix(agent-core): tune tool-dedup thresholds and preserve isError state - Lower repeat-reminder thresholds to 3, 5, 8 and force-stop to 12 - Force-stop no longer overrides successful tool results to isError - Move stopTurn field into ExecutableToolSuccessResult for explicit loop-control without requiring isError
This commit is contained in:
parent
33496bd031
commit
9e6972423e
4 changed files with 274 additions and 20 deletions
|
|
@ -541,7 +541,7 @@ export class TurnFlow {
|
|||
|
||||
private async runStepLoop(turnId: number, signal: AbortSignal): Promise<LoopTurnStopReason> {
|
||||
let stopHookContinuationUsed = false;
|
||||
const deduper = new ToolCallDeduplicator();
|
||||
const deduper = new ToolCallDeduplicator({ telemetry: this.agent.telemetry });
|
||||
await this.agent.mcp?.waitForInitialLoad(signal);
|
||||
// Surface the active goal at the start of the turn (append-only; no-op when
|
||||
// goal mode is off). Each goal continuation is its own turn, so this re-injects
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import type { ContentPart } from '@moonshot-ai/kosong';
|
||||
|
||||
import type { TelemetryClient } from '../../telemetry';
|
||||
import type { ExecutableToolResult } from '../../loop/types';
|
||||
|
||||
import { canonicalTelemetryArgs } from './canonical-args';
|
||||
|
|
@ -26,6 +27,19 @@ function makeReminderText2(toolName: string, repeatCount: number, args: unknown)
|
|||
);
|
||||
}
|
||||
|
||||
const REMINDER_TEXT_3 =
|
||||
'\n\n<system-reminder>\n' +
|
||||
'You are stuck in a dead end and have repeatedly made the same function call without progress.\n' +
|
||||
'Stop all function calls immediately. Do not call any tool in your next response.\n' +
|
||||
'In analysis, review the current execution state and identify why progress is blocked.\n' +
|
||||
'Then return a text-only summary to the user that reports the current problem, what has already been tried, and what information or decision is needed next.' +
|
||||
'\n</system-reminder>';
|
||||
|
||||
const REPEAT_REMINDER_1_START = 3;
|
||||
const REPEAT_REMINDER_2_START = 5;
|
||||
const REPEAT_REMINDER_3_START = 8;
|
||||
const REPEAT_FORCE_STOP_STREAK = 12;
|
||||
|
||||
interface Deferred<T> {
|
||||
readonly promise: Promise<T>;
|
||||
resolve(value: T): void;
|
||||
|
|
@ -63,6 +77,14 @@ function appendReminder(result: ExecutableToolResult, reminderText: string): Exe
|
|||
: { ...result, output: newOutput };
|
||||
}
|
||||
|
||||
function forceStopResult(
|
||||
result: ExecutableToolResult,
|
||||
reminderText: string,
|
||||
): ExecutableToolResult {
|
||||
const withReminder = appendReminder(result, reminderText);
|
||||
return { ...withReminder, stopTurn: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Placeholder result returned from `checkSameStep` for a duplicate call. Never
|
||||
* reaches the model — it is replaced in `finalizeResult` by awaiting the
|
||||
|
|
@ -82,8 +104,17 @@ const DEDUP_PLACEHOLDER_RESULT: ExecutableToolResult = { output: '' };
|
|||
* reuses the original call's result instead of executing the tool twice.
|
||||
* - Cross-step dedup: when the exact same call is repeated consecutively
|
||||
* across steps, the result returned to the model is suffixed with a system
|
||||
* reminder at specific streak thresholds (3, 5, and 8) to nudge the model
|
||||
* to try a different approach.
|
||||
* reminder once the streak hits 3. The reminder escalates as the streak
|
||||
* grows: r1 (gentle nudge) from streak 3, r2 (concrete repeat report) from
|
||||
* streak 5, r3 (dead-end stop instruction) from streak 8. From streak 12
|
||||
* onward the turn is force-stopped via `{ stopTurn: true }` so the loop
|
||||
* cannot keep spinning on the same call. Force-stop does not flip a
|
||||
* successful tool result into an error — the underlying tool's `isError`
|
||||
* is preserved.
|
||||
*
|
||||
* Telemetry: every finalized original call with streak >= 2 emits a
|
||||
* `tool_call_repeat` event carrying the current streak count as `repeat_count`
|
||||
* along with the tool name and which action was taken (none/r1/r2/r3/stop).
|
||||
*/
|
||||
export class ToolCallDeduplicator {
|
||||
private stepDeferreds = new Map<string, Deferred<ExecutableToolResult>>();
|
||||
|
|
@ -101,6 +132,11 @@ export class ToolCallDeduplicator {
|
|||
private callKeyByCallId = new Map<string, string>();
|
||||
private consecutiveKey: string | null = null;
|
||||
private consecutiveCount = 0;
|
||||
private readonly telemetry: TelemetryClient | undefined;
|
||||
|
||||
constructor(options?: { readonly telemetry?: TelemetryClient | undefined }) {
|
||||
this.telemetry = options?.telemetry;
|
||||
}
|
||||
|
||||
beginStep(): void {
|
||||
for (const deferred of this.stepDeferreds.values()) {
|
||||
|
|
@ -195,10 +231,27 @@ export class ToolCallDeduplicator {
|
|||
}
|
||||
|
||||
let finalResult = result;
|
||||
if (streak === 3) {
|
||||
finalResult = appendReminder(result, REMINDER_TEXT_1);
|
||||
} else if (streak === 5 || streak === 8) {
|
||||
let action: 'none' | 'r1' | 'r2' | 'r3' | 'stop' = 'none';
|
||||
if (streak >= REPEAT_FORCE_STOP_STREAK) {
|
||||
finalResult = forceStopResult(result, REMINDER_TEXT_3);
|
||||
action = 'stop';
|
||||
} else if (streak >= REPEAT_REMINDER_3_START) {
|
||||
finalResult = appendReminder(result, REMINDER_TEXT_3);
|
||||
action = 'r3';
|
||||
} else if (streak >= REPEAT_REMINDER_2_START) {
|
||||
finalResult = appendReminder(result, makeReminderText2(toolName, streak, args));
|
||||
action = 'r2';
|
||||
} else if (streak >= REPEAT_REMINDER_1_START) {
|
||||
finalResult = appendReminder(result, REMINDER_TEXT_1);
|
||||
action = 'r1';
|
||||
}
|
||||
|
||||
if (streak >= 2) {
|
||||
this.telemetry?.track('tool_call_repeat', {
|
||||
tool_name: toolName,
|
||||
repeat_count: streak,
|
||||
action,
|
||||
});
|
||||
}
|
||||
|
||||
this.stepDeferreds.get(key)?.resolve(finalResult);
|
||||
|
|
@ -208,5 +261,10 @@ export class ToolCallDeduplicator {
|
|||
|
||||
export const __testing = {
|
||||
REMINDER_TEXT_1,
|
||||
REMINDER_TEXT_3,
|
||||
makeReminderText2,
|
||||
REPEAT_REMINDER_1_START,
|
||||
REPEAT_REMINDER_2_START,
|
||||
REPEAT_REMINDER_3_START,
|
||||
REPEAT_FORCE_STOP_STREAK,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -85,11 +85,7 @@ export interface ExecutableToolErrorResult {
|
|||
readonly isError: true;
|
||||
/** See {@link ExecutableToolSuccessResult.message}. */
|
||||
readonly message?: string | undefined;
|
||||
/**
|
||||
* Internal loop-control hint. Tool result events strip this field before
|
||||
* persistence; it only tells the current turn whether another model step is
|
||||
* allowed after this tool batch.
|
||||
*/
|
||||
/** See {@link ExecutableToolSuccessResult.stopTurn}. */
|
||||
readonly stopTurn?: boolean | undefined;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,9 +1,31 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import type { ExecutableToolResult } from '../../../src/loop/types';
|
||||
import type {
|
||||
TelemetryClient,
|
||||
TelemetryProperties,
|
||||
} from '../../../src/telemetry';
|
||||
import { ToolCallDeduplicator, __testing } from '../../../src/agent/turn/tool-dedup';
|
||||
|
||||
const { REMINDER_TEXT_1, makeReminderText2 } = __testing;
|
||||
const { REMINDER_TEXT_1, REMINDER_TEXT_3, makeReminderText2 } = __testing;
|
||||
|
||||
interface RecordedTelemetryEvent {
|
||||
readonly event: string;
|
||||
readonly properties: TelemetryProperties | undefined;
|
||||
}
|
||||
|
||||
function makeRecordingTelemetry(): {
|
||||
readonly client: TelemetryClient;
|
||||
readonly events: RecordedTelemetryEvent[];
|
||||
} {
|
||||
const events: RecordedTelemetryEvent[] = [];
|
||||
const client: TelemetryClient = {
|
||||
track: (event, properties) => {
|
||||
events.push({ event, properties });
|
||||
},
|
||||
};
|
||||
return { client, events };
|
||||
}
|
||||
|
||||
function okResult(text: string): ExecutableToolResult {
|
||||
return { output: text };
|
||||
|
|
@ -98,7 +120,7 @@ describe('ToolCallDeduplicator', () => {
|
|||
expect(last!.output as string).not.toContain('repeated_times');
|
||||
});
|
||||
|
||||
it('does not inject reminder at 4 consecutive', async () => {
|
||||
it('keeps injecting reminder1 at 4 consecutive', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
let last: ExecutableToolResult | undefined;
|
||||
for (let i = 0; i < 4; i += 1) {
|
||||
|
|
@ -106,7 +128,8 @@ describe('ToolCallDeduplicator', () => {
|
|||
last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
expect(last!.output as string).not.toContain('<system-reminder>');
|
||||
expect(last!.output as string).toContain('<system-reminder>');
|
||||
expect(last!.output as string).toContain('repeating the exact same tool call');
|
||||
});
|
||||
|
||||
it('injects reminder2 at exactly 5 consecutive', async () => {
|
||||
|
|
@ -123,18 +146,20 @@ describe('ToolCallDeduplicator', () => {
|
|||
expect(last!.output as string).toContain('arguments:');
|
||||
});
|
||||
|
||||
it('does not inject reminder at 6 or 7 consecutive', async () => {
|
||||
it.each([6, 7])('keeps injecting reminder2 at %i consecutive', async (streak) => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
let last: ExecutableToolResult | undefined;
|
||||
for (let i = 0; i < 7; i += 1) {
|
||||
for (let i = 0; i < streak; i += 1) {
|
||||
dedup.beginStep();
|
||||
last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
expect(last!.output as string).not.toContain('<system-reminder>');
|
||||
expect(last!.output as string).toContain('<system-reminder>');
|
||||
expect(last!.output as string).toContain(`repeated_times: ${String(streak)}`);
|
||||
expect(last!.output as string).toContain('tool: Read');
|
||||
});
|
||||
|
||||
it('injects reminder2 at exactly 8 consecutive', async () => {
|
||||
it('injects the dead-end reminder at exactly 8 consecutive', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
let last: ExecutableToolResult | undefined;
|
||||
for (let i = 0; i < 8; i += 1) {
|
||||
|
|
@ -143,8 +168,7 @@ describe('ToolCallDeduplicator', () => {
|
|||
dedup.endStep();
|
||||
}
|
||||
expect(last!.output as string).toContain('<system-reminder>');
|
||||
expect(last!.output as string).toContain('repeated_times: 8');
|
||||
expect(last!.output as string).toContain('tool: Read');
|
||||
expect(last!.output as string).toContain('stuck in a dead end');
|
||||
});
|
||||
|
||||
it('resets streak when a different call is interleaved', async () => {
|
||||
|
|
@ -361,4 +385,180 @@ describe('ToolCallDeduplicator', () => {
|
|||
expect(finalDup).toEqual(dupCached);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dead-end stop reminder (streak >= 8)', () => {
|
||||
function stopTurnOf(result: ExecutableToolResult): boolean | undefined {
|
||||
return result.stopTurn;
|
||||
}
|
||||
|
||||
async function runStreak(
|
||||
dedup: ToolCallDeduplicator,
|
||||
count: number,
|
||||
): Promise<ExecutableToolResult> {
|
||||
let last: ExecutableToolResult | undefined;
|
||||
for (let i = 0; i < count; i += 1) {
|
||||
dedup.beginStep();
|
||||
last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
return last!;
|
||||
}
|
||||
|
||||
it('injects the dead-end reminder at exactly 8 consecutive without force-stopping', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
const last = await runStreak(dedup, 8);
|
||||
expect(last.output as string).toContain('<system-reminder>');
|
||||
expect(last.output as string).toContain('stuck in a dead end');
|
||||
expect(last.output as string).toContain('Stop all function calls immediately');
|
||||
// 8 is the reminder threshold, not yet force-stop.
|
||||
expect(last.isError).toBeUndefined();
|
||||
expect(stopTurnOf(last)).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([8, 9, 10, 11])(
|
||||
'keeps injecting the dead-end reminder without stopping the turn at streak %i',
|
||||
async (streak) => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
const last = await runStreak(dedup, streak);
|
||||
expect(last.output as string).toContain('stuck in a dead end');
|
||||
expect(last.isError).toBeUndefined();
|
||||
expect(stopTurnOf(last)).toBeUndefined();
|
||||
},
|
||||
);
|
||||
|
||||
it('force-stops the turn at exactly 12 consecutive without marking the tool failed', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
const last = await runStreak(dedup, 12);
|
||||
expect(last.output as string).toContain('stuck in a dead end');
|
||||
// The underlying tool succeeded — force-stop must not flip it to error.
|
||||
expect(last.isError).toBeUndefined();
|
||||
expect(stopTurnOf(last)).toBe(true);
|
||||
});
|
||||
|
||||
it('continues force-stopping past 12 consecutive', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
const last = await runStreak(dedup, 14);
|
||||
expect(last.isError).toBeUndefined();
|
||||
expect(stopTurnOf(last)).toBe(true);
|
||||
});
|
||||
|
||||
it('preserves the dead-end reminder text exactly', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
const last = await runStreak(dedup, 8);
|
||||
expect(last.output as string).toContain(REMINDER_TEXT_3.trim());
|
||||
});
|
||||
|
||||
it('keeps an error result error when force-stopping', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
let last: ExecutableToolResult | undefined;
|
||||
for (let i = 0; i < 12; i += 1) {
|
||||
dedup.beginStep();
|
||||
last = await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, errResult('boom'));
|
||||
dedup.endStep();
|
||||
}
|
||||
// The underlying tool was an error — that must survive force-stop.
|
||||
expect(last!.isError).toBe(true);
|
||||
expect(stopTurnOf(last!)).toBe(true);
|
||||
expect(last!.output as string).toContain('stuck in a dead end');
|
||||
});
|
||||
});
|
||||
|
||||
describe('repeat telemetry', () => {
|
||||
it('emits tool_call_repeat with the streak count starting at the second occurrence', async () => {
|
||||
const { client, events } = makeRecordingTelemetry();
|
||||
const dedup = new ToolCallDeduplicator({ telemetry: client });
|
||||
for (let i = 0; i < 3; i += 1) {
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
const repeats = events.filter((e) => e.event === 'tool_call_repeat');
|
||||
expect(repeats.map((e) => e.properties?.['repeat_count'])).toEqual([2, 3]);
|
||||
expect(repeats.every((e) => e.properties?.['tool_name'] === 'Read')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not emit telemetry on the first call', async () => {
|
||||
const { client, events } = makeRecordingTelemetry();
|
||||
const dedup = new ToolCallDeduplicator({ telemetry: client });
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, 'c0', 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
expect(events.filter((e) => e.event === 'tool_call_repeat')).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('labels the action as r1/r2/r3 according to the reminder tier from streak 3 through 11', async () => {
|
||||
const { client, events } = makeRecordingTelemetry();
|
||||
const dedup = new ToolCallDeduplicator({ telemetry: client });
|
||||
for (let i = 0; i < 11; i += 1) {
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
const byCount = new Map<number, string>();
|
||||
for (const e of events) {
|
||||
if (e.event !== 'tool_call_repeat') continue;
|
||||
byCount.set(e.properties?.['repeat_count'] as number, e.properties?.['action'] as string);
|
||||
}
|
||||
expect(byCount.get(2)).toBe('none');
|
||||
expect(byCount.get(3)).toBe('r1');
|
||||
expect(byCount.get(4)).toBe('r1');
|
||||
expect(byCount.get(5)).toBe('r2');
|
||||
expect(byCount.get(6)).toBe('r2');
|
||||
expect(byCount.get(7)).toBe('r2');
|
||||
expect(byCount.get(8)).toBe('r3');
|
||||
expect(byCount.get(9)).toBe('r3');
|
||||
expect(byCount.get(10)).toBe('r3');
|
||||
expect(byCount.get(11)).toBe('r3');
|
||||
});
|
||||
|
||||
it('labels the action as "stop" at streak 12+', async () => {
|
||||
const { client, events } = makeRecordingTelemetry();
|
||||
const dedup = new ToolCallDeduplicator({ telemetry: client });
|
||||
for (let i = 0; i < 13; i += 1) {
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
const at12 = events.find(
|
||||
(e) => e.event === 'tool_call_repeat' && e.properties?.['repeat_count'] === 12,
|
||||
);
|
||||
const at13 = events.find(
|
||||
(e) => e.event === 'tool_call_repeat' && e.properties?.['repeat_count'] === 13,
|
||||
);
|
||||
expect(at12?.properties?.['action']).toBe('stop');
|
||||
expect(at13?.properties?.['action']).toBe('stop');
|
||||
});
|
||||
|
||||
it('resets the count when a different call interleaves', async () => {
|
||||
const { client, events } = makeRecordingTelemetry();
|
||||
const dedup = new ToolCallDeduplicator({ telemetry: client });
|
||||
for (let i = 0; i < 2; i += 1) {
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, `a${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, 'b1', 'Read', { p: 2 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, 'c1', 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
const counts = events
|
||||
.filter((e) => e.event === 'tool_call_repeat')
|
||||
.map((e) => e.properties?.['repeat_count']);
|
||||
// Only the second Read({p:1}) is a repeat; the streak then breaks.
|
||||
expect(counts).toEqual([2]);
|
||||
});
|
||||
|
||||
it('does not emit telemetry when no client is provided', async () => {
|
||||
const dedup = new ToolCallDeduplicator();
|
||||
for (let i = 0; i < 3; i += 1) {
|
||||
dedup.beginStep();
|
||||
await runOriginal(dedup, `c${String(i)}`, 'Read', { p: 1 }, okResult('R'));
|
||||
dedup.endStep();
|
||||
}
|
||||
// Exercises the optional telemetry path; should complete silently.
|
||||
expect(true).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue