Skip to content

Commit 128923b

Browse files
committed
fix: apply EXPLORATION_TOOLS 5x limit to success path
The previous fix (fa4808b) only exempted exploration tools from coarse fingerprint tracking in the ERROR path. However, most read/grep calls succeed, and the success path had no exploration exemption. Root cause: Cross-turn accumulation. Reading the same file across many conversation turns (legitimate re-verification) accumulated counts, triggering the guard at 9 reads when limit was 2. Fix: Add EXPLORATION_LIMIT_MULTIPLIER (5x) for exploration tools in the success path. Now read/grep/glob/ls/stat/semsearch get a limit of maxRepeat * 5 = 10 by default, allowing normal exploratory patterns. Includes reproduction test confirming the fix works (9 historical reads of same file no longer triggers guard). Closes #33
1 parent f0edb89 commit 128923b

2 files changed

Lines changed: 81 additions & 5 deletions

File tree

src/provider/tool-loop-guard.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export function parseToolLoopMaxRepeat(
7575
// to allow legitimate exploration across different files/targets while still
7676
// catching spray-and-pray patterns.
7777
const COARSE_LIMIT_MULTIPLIER = 3;
78+
const EXPLORATION_LIMIT_MULTIPLIER = 5;
7879

7980
export function createToolLoopGuard(
8081
messages: Array<unknown>,
@@ -113,6 +114,16 @@ export function createToolLoopGuard(
113114
const repeatCount = (counts.get(successFingerprint) ?? 0) + 1;
114115
counts.set(successFingerprint, repeatCount);
115116

117+
// Exploration tools (read, grep, glob, etc.) get a higher limit because
118+
// re-reading the same file across turns is legitimate behavior (verifying
119+
// edits, checking state, etc.). Use 5x multiplier for these tools.
120+
const isExplorationTool = EXPLORATION_TOOLS.has(
121+
toolCall.function.name.toLowerCase(),
122+
);
123+
const effectiveMaxRepeat = isExplorationTool
124+
? maxRepeat * EXPLORATION_LIMIT_MULTIPLIER
125+
: maxRepeat;
126+
116127
// Some tools (notably edit/write) can get stuck in "successful" loops where
117128
// the model keeps re-issuing the same operation with slightly different
118129
// content (e.g. trailing newline differences). Track a coarse signature for
@@ -129,14 +140,14 @@ export function createToolLoopGuard(
129140
coarseCounts.set(coarseSuccessFingerprint, coarseRepeatCount);
130141
}
131142
const coarseTriggered = coarseSuccessFingerprint
132-
? coarseRepeatCount > maxRepeat
143+
? coarseRepeatCount > effectiveMaxRepeat
133144
: false;
134145
return {
135146
fingerprint: coarseTriggered ? coarseSuccessFingerprint! : successFingerprint,
136147
repeatCount: coarseTriggered ? coarseRepeatCount : repeatCount,
137-
maxRepeat,
148+
maxRepeat: effectiveMaxRepeat,
138149
errorClass,
139-
triggered: repeatCount > maxRepeat || coarseTriggered,
150+
triggered: repeatCount > effectiveMaxRepeat || coarseTriggered,
140151
tracked: true,
141152
};
142153
}

tests/unit/provider-tool-loop-guard.test.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,13 @@ describe("tool loop guard", () => {
9090
2,
9191
);
9292

93+
// Use 'edit' instead of 'read' - exploration tools have 5x limit multiplier
9394
const call = {
9495
id: "c1",
9596
type: "function",
9697
function: {
97-
name: "read",
98-
arguments: JSON.stringify({ path: "foo.txt" }),
98+
name: "edit",
99+
arguments: JSON.stringify({ path: "foo.txt", content: "bar" }),
99100
},
100101
} as const;
101102

@@ -576,3 +577,67 @@ describe("tool loop guard", () => {
576577
expect(readDecision.triggered).toBe(true);
577578
});
578579
});
580+
581+
// Reproduction test for issue #33: cross-turn accumulation
582+
it("ISSUE_33: should not trigger on exploration tool reads across turns", () => {
583+
// Simulate: user asks agent to read file A in turn 1, turn 3, turn 5, turn 7, turn 9
584+
// This is legitimate behavior - re-reading a file to verify changes is normal
585+
const history = [];
586+
587+
// Build 8 historical turns where agent read the same file (with success)
588+
for (let turn = 1; turn <= 8; turn++) {
589+
history.push({
590+
role: "assistant",
591+
content: null,
592+
tool_calls: [
593+
{
594+
id: `read-turn-${turn}`,
595+
type: "function",
596+
function: {
597+
name: "read",
598+
arguments: JSON.stringify({ path: "src/important-file.ts" }),
599+
},
600+
},
601+
],
602+
});
603+
history.push({
604+
role: "tool",
605+
tool_call_id: `read-turn-${turn}`,
606+
content: "export function foo() { return 42; }",
607+
});
608+
// User message between turns (simulating conversation flow)
609+
if (turn < 8) {
610+
history.push({
611+
role: "user",
612+
content: `Turn ${turn + 1}: Please check the file again`,
613+
});
614+
}
615+
}
616+
617+
const guard = createToolLoopGuard(history, 2);
618+
619+
// Now agent reads the same file again in current turn (turn 9)
620+
const decision = guard.evaluate({
621+
id: "read-turn-9",
622+
type: "function",
623+
function: {
624+
name: "read",
625+
arguments: JSON.stringify({ path: "src/important-file.ts" }),
626+
},
627+
});
628+
629+
// CURRENT BEHAVIOR (BUG): This triggers because count = 9 > limit 2
630+
// EXPECTED BEHAVIOR: Should NOT trigger - reading same file across turns is legitimate
631+
console.log("Issue #33 reproduction:", {
632+
triggered: decision.triggered,
633+
repeatCount: decision.repeatCount,
634+
maxRepeat: decision.maxRepeat,
635+
fingerprint: decision.fingerprint,
636+
});
637+
638+
// This test documents current (buggy) behavior
639+
// When fixed, change expect to: expect(decision.triggered).toBe(false);
640+
expect(decision.triggered).toBe(false); // FIXED: exploration tools get 5x limit
641+
expect(decision.repeatCount).toBe(9); // 8 historical + 1 current
642+
expect(decision.maxRepeat).toBe(10); // 2 * 5 (EXPLORATION_LIMIT_MULTIPLIER)
643+
});

0 commit comments

Comments
 (0)