Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions packages/core/src/agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<system-reminder>` 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');
});
});
});
42 changes: 42 additions & 0 deletions packages/core/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<ToolContext['askUser']>;
Expand Down Expand Up @@ -531,6 +534,11 @@ export async function runAgent(opts: RunAgentOptions): Promise<RunAgentResult> {
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<void> => {
Expand Down Expand Up @@ -640,6 +648,7 @@ export async function runAgent(opts: RunAgentOptions): Promise<RunAgentResult> {
// 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<string, ToolResultBlock>();
const guardReminders: string[] = [];
type Ready = { toolUse: ToolUseBlock; handler: NonNullable<ReturnType<typeof opts.tools.get>> };
const ready: Ready[] = [];

Expand Down Expand Up @@ -673,6 +682,33 @@ export async function runAgent(opts: RunAgentOptions): Promise<RunAgentResult> {
}
};

/**
* 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<void> => {
if (guardReminders.length === 0) return;
const msg: StoredMessage = {
role: 'user',
content: [
{
type: 'text',
text: `<system-reminder>\n${guardReminders.join('\n\n')}\n</system-reminder>`,
},
],
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,
Expand All @@ -698,6 +734,11 @@ export async function runAgent(opts: RunAgentOptions): Promise<RunAgentResult> {

// 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)
Expand Down Expand Up @@ -891,6 +932,7 @@ export async function runAgent(opts: RunAgentOptions): Promise<RunAgentResult> {
for (const r of serial) await execOne(r);

await flushToolResults();
await flushGuardReminders();

// M3c: auto-compact if the *current* context crossed the threshold.
//
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/guard/index.ts
Original file line number Diff line number Diff line change
@@ -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';
115 changes: 115 additions & 0 deletions packages/core/src/guard/repeat-tool.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>, 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();
});
});
Binary file added packages/core/src/guard/repeat-tool.ts
Binary file not shown.
8 changes: 8 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading