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 0000000..a25b69c Binary files /dev/null and b/packages/core/src/guard/repeat-tool.ts differ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6916e34..b281f1e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -230,6 +230,14 @@ export { type SpillSource, type SpillStore, } from './spill/index.js'; +// Loop-hygiene guards +export { + RepeatToolGuard, + DEFAULT_REPEAT_EXCLUDE, + type RepeatGuardOptions, + type RepeatReminder, + type RepeatReminderKind, +} from './guard/index.js'; // Agent loop's approval callback type (M3b) export type { ApprovalCallback, ApprovalDecision } from './agent.js';