From 3cb03c510aca01478ea21dbe4f20a1834a711e39 Mon Sep 17 00:00:00 2001 From: t Date: Fri, 14 Aug 2026 12:35:21 +0800 Subject: [PATCH] feat(core): interrupt the loop where the model is repeating itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The most expensive failure an agent has is not an error — it is the loop that neither errors nor terminates. The model calls the same tool with the same arguments, reads the same answer, and calls it again, quietly spending the budget until a turn cap or a human notices. The guard watches each run's chain of tool calls, counts consecutive calls with identical canonicalized arguments, and at 3, 5, and 8 injects an escalating reminder: first a brief nudge, then one naming the tool, the run length, and the arguments it keeps sending. It has no veto. It is not in the tool list, cannot block a call, and cannot rewrite arguments. That is what makes false positives affordable — polling a file for a change is a legitimate identical repeat, and it costs one paragraph. Two behaviours are deliberate rather than incidental: Excluded tools are transparent to the chain, not resets. `Grep X → TodoWrite → Grep X` is still two consecutive `Grep X`, so bookkeeping interleaved into a loop cannot launder it. Denied calls count. Detection sits before the gate, because a model hammering a call the gate keeps refusing is precisely the loop worth interrupting. The reminder is appended as its own user message after the tool results, not as a text block beside them: the provider maps a user message's text and its tool_result blocks to separate wire messages, and a `user` turn between an assistant's tool_calls and their `tool` replies is a sequence the API rejects. Co-Authored-By: Claude Opus 5 --- packages/core/src/agent.test.ts | 119 ++++++++++++++++++++ packages/core/src/agent.ts | 42 +++++++ packages/core/src/guard/index.ts | 12 ++ packages/core/src/guard/repeat-tool.test.ts | 115 +++++++++++++++++++ packages/core/src/guard/repeat-tool.ts | Bin 0 -> 6204 bytes packages/core/src/index.ts | 8 ++ 6 files changed, 296 insertions(+) create mode 100644 packages/core/src/guard/index.ts create mode 100644 packages/core/src/guard/repeat-tool.test.ts create mode 100644 packages/core/src/guard/repeat-tool.ts diff --git a/packages/core/src/agent.test.ts b/packages/core/src/agent.test.ts index a53b8ee..14fcc0e 100644 --- a/packages/core/src/agent.test.ts +++ b/packages/core/src/agent.test.ts @@ -1286,4 +1286,123 @@ describe('runAgent', () => { expect(resultText(result.history)).toContain('was not saved'); }); }); + + describe('repeat-call guard', () => { + const spinTool: ToolHandler = { + name: 'Spin', + definition: { + name: 'Spin', + description: 'always says the same thing', + inputSchema: { type: 'object', properties: {} }, + }, + execute: () => Promise.resolve({ content: 'nothing changed' }), + }; + const spin = (i: number): ProviderResult => + toolUse('again', { type: 'tool_use', id: `call_${i}`, name: 'Spin', input: { q: 1 } }); + + /** + * Guard reminders in order. Matched on the guard's own phrasing rather than + * the `` wrapper, which the loop also uses for the date and + * cwd reminders it prepends to every user message. + */ + function reminders(history: StoredMessage[]): string[] { + const out: string[] = []; + for (const msg of history) { + if (msg.role !== 'user') continue; + for (const block of msg.content) { + if ( + typeof block !== 'string' && + block.type === 'text' && + block.text.includes('in a row') + ) { + out.push(block.text); + } + } + } + return out; + } + + it('nudges the model once it starts repeating itself', async () => { + const result = await runAgent({ + provider: new MockProvider([spin(1), spin(2), spin(3), endTurn('ok')]), + tools: new ToolRegistry([spinTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + }); + + const fired = reminders(result.history); + expect(fired).toHaveLength(1); + expect(fired[0]).toContain('called Spin 3 times in a row'); + }); + + it('stays silent when the calls differ', async () => { + const result = await runAgent({ + provider: new MockProvider([ + toolUse('a', { type: 'tool_use', id: 'c1', name: 'Spin', input: { q: 1 } }), + toolUse('b', { type: 'tool_use', id: 'c2', name: 'Spin', input: { q: 2 } }), + toolUse('c', { type: 'tool_use', id: 'c3', name: 'Spin', input: { q: 3 } }), + endTurn('ok'), + ]), + tools: new ToolRegistry([spinTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + }); + expect(reminders(result.history)).toHaveLength(0); + }); + + it('can be turned off', async () => { + const result = await runAgent({ + provider: new MockProvider([spin(1), spin(2), spin(3), endTurn('ok')]), + tools: new ToolRegistry([spinTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + repeatGuard: false, + }); + expect(reminders(result.history)).toHaveLength(0); + }); + + it('keeps the reminder out of the tool-result message', async () => { + // The provider maps a user message's text and its tool_result blocks to + // separate wire messages, and a `user` turn between an assistant's + // tool_calls and their `tool` replies is a sequence the API rejects. The + // reminder therefore has to be its own message, after the results. + const result = await runAgent({ + provider: new MockProvider([spin(1), spin(2), spin(3), endTurn('ok')]), + tools: new ToolRegistry([spinTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + }); + + for (const msg of result.history) { + const kinds = new Set(msg.content.map((b) => (typeof b === 'string' ? 'string' : b.type))); + expect(kinds.has('tool_result') && kinds.has('text')).toBe(false); + } + }); + + it('counts calls the gate refused', async () => { + // A model hammering a denied call is exactly the loop worth interrupting. + const result = await runAgent({ + provider: new MockProvider([spin(1), spin(2), spin(3), endTurn('ok')]), + tools: new ToolRegistry([spinTool]), + systemPrompt: '', + userMessage: 'go', + model: 'deepseek-chat', + cwd, + mode: 'default', + permissions: { deny: ['Spin'] }, + }); + + const fired = reminders(result.history); + expect(fired).toHaveLength(1); + expect(fired[0]).toContain('called Spin 3 times in a row'); + }); + }); }); diff --git a/packages/core/src/agent.ts b/packages/core/src/agent.ts index e1712a6..8cfb80a 100644 --- a/packages/core/src/agent.ts +++ b/packages/core/src/agent.ts @@ -7,6 +7,7 @@ import type { FileContract } from './config/file-contract.js'; import type { UnattendedApprovalPolicy } from './cron/index.js'; import type { LedgerSink } from './ledger/index.js'; import { buildToolCallRecord, ledgerKindForTool, readPathFor } from './ledger/record-tool-call.js'; +import { RepeatToolGuard, type RepeatGuardOptions } from './guard/index.js'; import { dispatchToolCall, type DispatchVerdict } from './harness/tool-dispatcher.js'; import { TaskManager, type TaskRunner } from './tasks/manager.js'; import type { HookDispatcher } from './hooks/index.js'; @@ -118,6 +119,8 @@ export interface RunAgentOptions { * and it is the seam tests use. */ spillStore?: SpillStore; + /** Nudge the model when it repeats an identical tool call. `false` disables it. */ + repeatGuard?: false | RepeatGuardOptions; /** Host callback for AskUserQuestion tool. Optional — when absent the tool * errors. */ askUser?: NonNullable; @@ -531,6 +534,11 @@ export async function runAgent(opts: RunAgentOptions): Promise { const totalUsage = { inputTokens: 0, outputTokens: 0, reasoningTokens: 0, cacheReadTokens: 0 }; let turnsUsed = 0; + // One chain per run. A run begins with a user message, which is exactly when + // a fresh line of work starts, so there is nothing to carry over. + const repeatGuard = + opts.repeatGuard === false ? undefined : new RepeatToolGuard(opts.repeatGuard ?? {}); + // Stop hook — fires when the TOP-LEVEL agent finishes a run (a sub-agent's // completion is signalled by SubagentStop instead). Observation only. const fireStop = async (reason: string): Promise => { @@ -640,6 +648,7 @@ export async function runAgent(opts: RunAgentOptions): Promise { // re-assembled in the model's original order regardless of finish order. const toolBlocks = result.content.filter((b): b is ToolUseBlock => b.type === 'tool_use'); const resultsById = new Map(); + const guardReminders: string[] = []; type Ready = { toolUse: ToolUseBlock; handler: NonNullable> }; const ready: Ready[] = []; @@ -673,6 +682,33 @@ export async function runAgent(opts: RunAgentOptions): Promise { } }; + /** + * Append any guard reminders as their own user message. + * + * This has to be a separate message appended AFTER the tool results, not a + * text block alongside them: the provider maps a user message's text and + * its tool_result blocks to separate wire messages, and a `user` turn + * between the assistant's tool_calls and their `tool` replies is a sequence + * the API rejects. + */ + const flushGuardReminders = async (): Promise => { + if (guardReminders.length === 0) return; + const msg: StoredMessage = { + role: 'user', + content: [ + { + type: 'text', + text: `\n${guardReminders.join('\n\n')}\n`, + }, + ], + timestamp: new Date().toISOString(), + }; + history.push(msg); + if (opts.session && opts.persistSessionMessages !== false) { + await opts.session.manager.append(opts.session.id, msg); + } + }; + /** Record a gate refusal as both a tool result (the model sees it) and an event (the host does). */ const recordBlocked = ( toolUse: ToolUseBlock, @@ -698,6 +734,11 @@ export async function runAgent(opts: RunAgentOptions): Promise { // Phase 1 — sequential gate + approval. for (const toolUse of toolBlocks) { + // Counted before the gate, so a model hammering a denied call is caught + // by the same chain as one hammering an allowed one. + const repeat = repeatGuard?.observe(toolUse.name, toolUse.input); + if (repeat) guardReminders.push(repeat.text); + const handler = !allowedToolNames || allowedToolNames.has(toolUse.name) ? opts.tools.get(toolUse.name) @@ -891,6 +932,7 @@ export async function runAgent(opts: RunAgentOptions): Promise { for (const r of serial) await execOne(r); await flushToolResults(); + await flushGuardReminders(); // M3c: auto-compact if the *current* context crossed the threshold. // diff --git a/packages/core/src/guard/index.ts b/packages/core/src/guard/index.ts new file mode 100644 index 0000000..fb278d3 --- /dev/null +++ b/packages/core/src/guard/index.ts @@ -0,0 +1,12 @@ +// Loop-hygiene guards — advisory plugins that watch the agent loop for +// unproductive patterns. A guard never vetoes; it only tells the model what it +// is doing. +// Spec: docs/DSH_ADOPTION_PLAN.md §1.2 + +export { + RepeatToolGuard, + DEFAULT_REPEAT_EXCLUDE, + type RepeatGuardOptions, + type RepeatReminder, + type RepeatReminderKind, +} from './repeat-tool.js'; diff --git a/packages/core/src/guard/repeat-tool.test.ts b/packages/core/src/guard/repeat-tool.test.ts new file mode 100644 index 0000000..a7b3c6c --- /dev/null +++ b/packages/core/src/guard/repeat-tool.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect } from 'vitest'; +import { RepeatToolGuard } from './repeat-tool.js'; + +/** Call the guard n times with the same arguments; return every reminder fired. */ +function run(guard: RepeatToolGuard, tool: string, input: Record, times: number) { + const fired = []; + for (let i = 0; i < times; i++) { + const r = guard.observe(tool, input); + if (r) fired.push(r); + } + return fired; +} + +describe('RepeatToolGuard', () => { + it('stays quiet below the first threshold', () => { + const guard = new RepeatToolGuard(); + expect(run(guard, 'Grep', { pattern: 'x' }, 2)).toHaveLength(0); + }); + + it('fires at each configured threshold and nowhere else', () => { + const guard = new RepeatToolGuard(); + const fired = run(guard, 'Grep', { pattern: 'x' }, 10); + expect(fired.map((r) => r.runLength)).toEqual([3, 5, 8]); + }); + + it('escalates from a brief nudge to a detailed one', () => { + const guard = new RepeatToolGuard(); + const fired = run(guard, 'Grep', { pattern: 'x' }, 6); + expect(fired.map((r) => r.kind)).toEqual(['brief', 'detailed']); + expect(fired[0].text).not.toContain('"pattern"'); + expect(fired[1].text).toContain('"pattern":"x"'); + }); + + it('treats argument order as irrelevant', () => { + const guard = new RepeatToolGuard(); + guard.observe('Grep', { a: 1, b: 2 }); + guard.observe('Grep', { b: 2, a: 1 }); + const r = guard.observe('Grep', { a: 1, b: 2 }); + expect(r?.runLength).toBe(3); + }); + + it('resets when the arguments change', () => { + const guard = new RepeatToolGuard(); + run(guard, 'Grep', { pattern: 'x' }, 2); + expect(guard.observe('Grep', { pattern: 'y' })).toBeNull(); + expect(guard.observe('Grep', { pattern: 'y' })).toBeNull(); + expect(guard.observe('Grep', { pattern: 'y' })?.runLength).toBe(3); + }); + + it('resets when a different tool is called', () => { + const guard = new RepeatToolGuard(); + run(guard, 'Grep', { pattern: 'x' }, 2); + guard.observe('Read', { file_path: 'a' }); + expect(guard.observe('Grep', { pattern: 'x' })).toBeNull(); + }); + + it('does not let an excluded tool launder a loop', () => { + // The point of exclusion: bookkeeping interleaved into a loop is still a loop. + const guard = new RepeatToolGuard(); + guard.observe('Grep', { pattern: 'x' }); + guard.observe('TodoWrite', { todos: [] }); + guard.observe('Grep', { pattern: 'x' }); + guard.observe('TodoWrite', { todos: [] }); + expect(guard.observe('Grep', { pattern: 'x' })?.runLength).toBe(3); + }); + + it('never reports on an excluded tool itself', () => { + const guard = new RepeatToolGuard(); + expect(run(guard, 'TodoWrite', { todos: [] }, 20)).toHaveLength(0); + }); + + it('supports wildcard exclusions', () => { + const guard = new RepeatToolGuard({ exclude: ['mcp__*'] }); + expect(run(guard, 'mcp__db__query', { q: 1 }, 10)).toHaveLength(0); + expect(run(guard, 'Grep', { pattern: 'x' }, 3)).toHaveLength(1); + }); + + it('caps the arguments it quotes back', () => { + // Only the detailed form quotes arguments, so this needs the second threshold. + const guard = new RepeatToolGuard({ thresholds: [2, 3], argumentsPreviewChars: 50 }); + const fired = run(guard, 'Write', { content: 'z'.repeat(10_000) }, 3); + const detailed = fired[1]; + expect(detailed.kind).toBe('detailed'); + expect(detailed.text).toContain('more characters'); + expect(detailed.text.length).toBeLessThan(600); + }); + + it('detects on the full arguments even when the preview is capped', () => { + // The cap bounds the reminder, never the comparison — two payloads that + // differ only past the cap must not count as the same call. + const guard = new RepeatToolGuard({ thresholds: [2], argumentsPreviewChars: 10 }); + guard.observe('Write', { content: `${'z'.repeat(100)}A` }); + expect(guard.observe('Write', { content: `${'z'.repeat(100)}B` })).toBeNull(); + }); + + it('rejects an unusable threshold list instead of falling back', () => { + expect(() => new RepeatToolGuard({ thresholds: [] })).toThrow(/at least one/); + expect(() => new RepeatToolGuard({ thresholds: [1] })).toThrow(/>= 2/); + expect(() => new RepeatToolGuard({ thresholds: [2.5] })).toThrow(/integers/); + }); + + it('normalizes thresholds given out of order or duplicated', () => { + const guard = new RepeatToolGuard({ thresholds: [5, 3, 3] }); + const fired = run(guard, 'Grep', { pattern: 'x' }, 6); + expect(fired.map((r) => r.runLength)).toEqual([3, 5]); + expect(fired.map((r) => r.kind)).toEqual(['brief', 'detailed']); + }); + + it('reset() clears the chain', () => { + const guard = new RepeatToolGuard(); + run(guard, 'Grep', { pattern: 'x' }, 2); + guard.reset(); + expect(guard.observe('Grep', { pattern: 'x' })).toBeNull(); + }); +}); diff --git a/packages/core/src/guard/repeat-tool.ts b/packages/core/src/guard/repeat-tool.ts new file mode 100644 index 0000000000000000000000000000000000000000..a25b69c3b198dc91327497657fcb3fee0ddc521e GIT binary patch literal 6204 zcma)A;d0!z5$yipbJg}&i^3tibCT~nho5HX7)!yMD$y;TY&01I9k-5>9RF%t* ziTrYE>&TfDqj|z*t+OM_P38`tT|R$z^6dQ8^u_thcdyP)UZ(4u{`;TL(=Xbw(-nNc ze=5HzoUvQ62X9yGlP;Z3>4m3C%c3>5W-&Z$K^!d$=O^JiFFh|!b`5K?t@b@xy|IPI z?z>F{w}aIi?G1mcwz6nSCv2$H%fc6H<#oA*0gkE8#VUJ?*r_d^=Ua{}`0M-uvtv3Q#)r`blIA*)r6m{*$gt+$F@ ztCdwttLjyZ_Z0bDsG@8vCVLD-OdKtY?M=b9zO8lT*a5btDpE`2lUB+JLKKz8)NJ4L z1}Y5%Ako@dc!phC^4hM83UP1~`o1Z&M`Rpq<`F*gCSPiw?(H4!?*oSzoU;W7h$&x~ zYC_q{H~rv>>@|YRSwz4! zoJ5*r&$0dlwa%N`(cGBpYprWeE3*TjWX&3`H=Iv?8?bUDED_AirY1MvaFxvfw5)7$ zolj`?H7bX`r62$CPwEE9Q}{6BC^%n$s{F4EU~0H*5g3J<4bkrMC*Y!PhI?i8O`$hWSKyc~{EznT1D=03hzc_gihxNF!IpFe z)?|3Yeyu%99p0=p@&thq zE>8dQ;`Ez$PoJM$@aQieK19uM~zsonFJ$Km8l9Ks`$H3ZdBQT`kLyxGMj4L{gR3-7L=S=tF~$3 z09wWBCu@%Yebgk-SXmre#!gJ*_eRO<`8UnUH{Zz=qEW$iG#)pxaf2T7)8q0zN zzc~@kW(Y-?4qm*_8FuwmfZ8NF`2{NdjtF9QNjo>zAzCeL$0wu{BvJzWT{=H%C!|Fs z?BJ5o{-{rAbcIvHmUvGgT=o`$sPqOwUY_38$?R_r?xHPMmxUKYf(7n0wZ=GoJ5Ikd zMU{+FEOP%~CNXQ{V{cQmFrV-;c#rb#5IQiBxWQ%u0Aw$%5tpLMOEi__@G9N^>;#9s?Sj%Y1RWq-@dR3j z`(fL)#VY`>se*~vv!dZCb1*hoH|RJOw-NNKb{P?%tpVoh819JNJv8L0RjlSjudrAd zL9R-hgN>n^MUIcmECJ@yw9qrq*7xbsqvdj+fdgy3juI1`B|RQeB-gH3R!1lZYzrbc zC#Rib`gUGfC@BTcAye3zEYIDZ4f2h0?(f$EMM6rCs+@C@Ym?cH$(nAP`B@!c*nK%%5~g z0+5GFf(2z5(e~09_|QuG&-I`kMiZRaPe4v_4kQJLFc!?TAdY#BkAtM{imKt54PfU$;XuMOg>qRxgcF2V<2=R_ylYr$hqlUGfp{}TYg#h# zS-ohS&X0mY)y`U}^*Ty2Qq0ow!yIpXI(KCEVi{B zTMKc2i4@=M9tXLd(M81_VTu)aRj_&&^*AoHSw|;by9D~>>VjF4!_uA*xjIa>D3&<< zqLjK5B-jOqZqa|iCLgcN9P0T-C(+wTV;nws^ov3cdSGLq*sL@pa(;IFXAY z?u#!2SjvSJI=DLsLVP%1pzfI#V4&R)d)iKduLqxZ;{St?{gURvYQ2Gn^rX8eING@= zU>{pq7x%~(C*nLd)~crX3=gB)>0FNAM98%Bh}cRvY=jGn?gPOWdDS5i*yy%D7=~N4 z_SE`wBSEytY@9k|vvF`p4j+-40AW7)b6xj$1o-JgObGRJ(iToj46G~}= z5U!*0W0MKW5Z~g!xB4x)PVp$wS=xsd{|>;}8F)s|R<dmpaKvR|l^z zadJbu;y~j@22BZa`0cD+1F8tCGlbg8?J>aJ0igar^N%Q_O-R%0yU@|z9uAp7fFh_b z+v{jG$7x&g(gHzJ2k$stI4*F;Dn9b`pkilIdR1iC+?R37fyKfd3gZcT2$$-(Tfhh0 zQOWfRe`3}_cHFgCm9k_F&}CmCF6#ovlnRGU0S8)jIAgRT0|Kmr$pWViTmrJ-<1;4^ MDi`G~JOg(A2YD