scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-08-28 05:27:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: correct tool repetition detector to not block first tool call when limit is 1 (#6836)

Browse source 
Co-authored-by: Roo Code <roomote@roocode.com>
This commit is contained in:
roomote[bot] 2025-08-11 17:36:30 -04:00 • committed by GitHub
parent b1300e5550
commit bd39fe6fd4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 98 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/core/tools/ToolRepetitionDetector.ts
View file

@ -39,7 +39,7 @@ export class ToolRepetitionDetector {
if (this.previousToolCallJson === currentToolCallJson) {
this.consecutiveIdenticalToolCallCount++
} else {
this.consecutiveIdenticalToolCallCount = 1 // Start with 1 for the first occurrence
this.consecutiveIdenticalToolCallCount = 0 // Reset to 0 for a new tool
this.previousToolCallJson = currentToolCallJson
}

150
src/core/tools/__tests__/ToolRepetitionDetector.spec.ts
View file

@ -32,30 +32,38 @@ describe("ToolRepetitionDetector", () => {
const detector = new ToolRepetitionDetector()
// We'll verify this through behavior in subsequent tests
// First call (counter = 1)
// First call (counter = 0)
const result1 = detector.check(createToolUse("test", "test-tool"))
expect(result1.allowExecution).toBe(true)
// Second identical call (counter = 2)
// Second identical call (counter = 1)
const result2 = detector.check(createToolUse("test", "test-tool"))
expect(result2.allowExecution).toBe(true)
// Third identical call (counter = 3) reaches the default limit
// Third identical call (counter = 2)
const result3 = detector.check(createToolUse("test", "test-tool"))
expect(result3.allowExecution).toBe(false)
expect(result3.allowExecution).toBe(true)
// Fourth identical call (counter = 3) reaches the default limit
const result4 = detector.check(createToolUse("test", "test-tool"))
expect(result4.allowExecution).toBe(false)
})
it("should use the custom limit when provided", () => {
const customLimit = 2
const detector = new ToolRepetitionDetector(customLimit)
// First call (counter = 1)
// First call (counter = 0)
const result1 = detector.check(createToolUse("test", "test-tool"))
expect(result1.allowExecution).toBe(true)
// Second identical call (counter = 2) reaches the custom limit
// Second identical call (counter = 1)
const result2 = detector.check(createToolUse("test", "test-tool"))
expect(result2.allowExecution).toBe(false)
expect(result2.allowExecution).toBe(true)
// Third identical call (counter = 2) reaches the custom limit
const result3 = detector.check(createToolUse("test", "test-tool"))
expect(result3.allowExecution).toBe(false)
})
})
@ -97,17 +105,21 @@ describe("ToolRepetitionDetector", () => {
it("should allow execution when repetition is below limit and block when limit reached", () => {
const detector = new ToolRepetitionDetector(3)
// First call (counter = 1)
// First call (counter = 0)
const result1 = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result1.allowExecution).toBe(true)
// Second identical call (counter = 2)
// Second identical call (counter = 1)
const result2 = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result2.allowExecution).toBe(true)
// Third identical call (counter = 3) reaches limit
// Third identical call (counter = 2)
const result3 = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result3.allowExecution).toBe(false)
expect(result3.allowExecution).toBe(true)
// Fourth identical call (counter = 3) reaches limit
const result4 = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result4.allowExecution).toBe(false)
})
})
@ -116,13 +128,16 @@ describe("ToolRepetitionDetector", () => {
it("should block execution when repetition reaches the limit", () => {
const detector = new ToolRepetitionDetector(3)
// First call (counter = 1)
// First call (counter = 0)
detector.check(createToolUse("repeat", "repeat-tool"))
// Second identical call (counter = 2)
// Second identical call (counter = 1)
detector.check(createToolUse("repeat", "repeat-tool"))
// Third identical call (counter = 3) - should reach limit
// Third identical call (counter = 2)
detector.check(createToolUse("repeat", "repeat-tool"))
// Fourth identical call (counter = 3) - should reach limit
const result = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result.allowExecution).toBe(false)
@ -136,6 +151,7 @@ describe("ToolRepetitionDetector", () => {
// Reach the limit
detector.check(createToolUse("repeat", "repeat-tool"))
detector.check(createToolUse("repeat", "repeat-tool"))
const limitResult = detector.check(createToolUse("repeat", "repeat-tool")) // This reaches limit
expect(limitResult.allowExecution).toBe(false)
@ -152,6 +168,7 @@ describe("ToolRepetitionDetector", () => {
// Reach the limit with a specific tool
detector.check(createToolUse("problem", "problem-tool"))
detector.check(createToolUse("problem", "problem-tool"))
const limitResult = detector.check(createToolUse("problem", "problem-tool")) // This reaches limit
expect(limitResult.allowExecution).toBe(false)
@ -165,13 +182,17 @@ describe("ToolRepetitionDetector", () => {
// Reach the limit
detector.check(createToolUse("repeat", "repeat-tool"))
detector.check(createToolUse("repeat", "repeat-tool"))
const limitResult = detector.check(createToolUse("repeat", "repeat-tool")) // This reaches limit
expect(limitResult.allowExecution).toBe(false)
// First call after reset
detector.check(createToolUse("repeat", "repeat-tool"))
// Second identical call (counter = 2) should reach limit again
// Second call after reset
detector.check(createToolUse("repeat", "repeat-tool"))
// Third identical call (counter = 2) should reach limit again
const result = detector.check(createToolUse("repeat", "repeat-tool"))
expect(result.allowExecution).toBe(false)
expect(result.askUser).toBeDefined()
@ -186,6 +207,7 @@ describe("ToolRepetitionDetector", () => {
// Reach the limit
detector.check(createToolUse("test", toolName))
detector.check(createToolUse("test", toolName))
const result = detector.check(createToolUse("test", toolName))
expect(result.allowExecution).toBe(false)
@ -201,7 +223,8 @@ describe("ToolRepetitionDetector", () => {
// Create an empty tool call - a tool with no parameters
// Use the empty tool directly in the check calls
detector.check(createToolUse("empty-tool", "empty-tool"))
const result = detector.check(createToolUse("empty-tool"))
detector.check(createToolUse("empty-tool", "empty-tool"))
const result = detector.check(createToolUse("empty-tool", "empty-tool"))
expect(result.allowExecution).toBe(false)
expect(result.askUser).toBeDefined()
@ -210,7 +233,7 @@ describe("ToolRepetitionDetector", () => {
it("should handle different tool names with identical serialized JSON", () => {
const detector = new ToolRepetitionDetector(2)
// First, call with tool-name-1 twice to set up the counter
// First, call with tool-name-1 to set up the counter
const toolUse1 = createToolUse("tool-name-1", "tool-name-1", { param: "value" })
detector.check(toolUse1)
@ -223,21 +246,25 @@ describe("ToolRepetitionDetector", () => {
;(detector as any).serializeToolUse = (tool: ToolUse) => {
// Use string comparison for the name since it's technically an enum
if (String(tool.name) === "tool-name-2") {
return (detector as any).serializeToolUse(toolUse1) // Return the same JSON as toolUse1
return originalSerialize.call(detector, toolUse1) // Return the same JSON as toolUse1
}
return originalSerialize(tool)
return originalSerialize.call(detector, tool)
}
// This should detect as a repetition now
const result = detector.check(toolUse2)
// Second call - this should be considered identical due to our mock
const result2 = detector.check(toolUse2)
expect(result2.allowExecution).toBe(true) // Still allowed (counter = 1)
// Third call - should be blocked (limit is 2)
const result3 = detector.check(toolUse2)
// Restore the original method
;(detector as any).serializeToolUse = originalSerialize
// Since we're directly manipulating the internal state for testing,
// we still expect it to consider this a repetition
expect(result.allowExecution).toBe(false)
expect(result.askUser).toBeDefined()
// we expect it to consider this a repetition
expect(result3.allowExecution).toBe(false)
expect(result3.askUser).toBeDefined()
})
it("should treat tools with same parameters in different order as identical", () => {
@ -247,11 +274,13 @@ describe("ToolRepetitionDetector", () => {
const toolUse1 = createToolUse("same-tool", "same-tool", { a: "1", b: "2", c: "3" })
detector.check(toolUse1)
// Create tool with same parameters but in different order
// Second call with same parameters but in different order
const toolUse2 = createToolUse("same-tool", "same-tool", { c: "3", a: "1", b: "2" })
detector.check(toolUse2)
// This should still detect as a repetition due to canonical JSON with sorted keys
const result = detector.check(toolUse2)
// Third call - should be blocked (limit is 2)
const toolUse3 = createToolUse("same-tool", "same-tool", { b: "2", c: "3", a: "1" })
const result = detector.check(toolUse3)
// Since parameters are sorted alphabetically in the serialized JSON,
// these should be considered identical
@ -262,46 +291,58 @@ describe("ToolRepetitionDetector", () => {
// ===== Explicit Nth Call Blocking tests =====
describe("explicit Nth call blocking behavior", () => {
it("should block on the 1st call for limit 1", () => {
it("should allow the 1st call but block on the 2nd call for limit 1", () => {
const detector = new ToolRepetitionDetector(1)
// First call (counter = 1) should be blocked
const result = detector.check(createToolUse("tool", "tool-name"))
expect(result.allowExecution).toBe(false)
expect(result.askUser).toBeDefined()
})
it("should block on the 2nd call for limit 2", () => {
const detector = new ToolRepetitionDetector(2)
// First call (counter = 1)
// First call (counter = 0) should be allowed
const result1 = detector.check(createToolUse("tool", "tool-name"))
expect(result1.allowExecution).toBe(true)
expect(result1.askUser).toBeUndefined()
// Second call (counter = 2) should be blocked
// Second identical call (counter = 1) should be blocked
const result2 = detector.check(createToolUse("tool", "tool-name"))
expect(result2.allowExecution).toBe(false)
expect(result2.askUser).toBeDefined()
})
it("should block on the 3rd call for limit 3 (default)", () => {
const detector = new ToolRepetitionDetector(3)
it("should allow first 2 calls but block on the 3rd call for limit 2", () => {
const detector = new ToolRepetitionDetector(2)
// First call (counter = 1)
// First call (counter = 0)
const result1 = detector.check(createToolUse("tool", "tool-name"))
expect(result1.allowExecution).toBe(true)
// Second call (counter = 2)
// Second identical call (counter = 1)
const result2 = detector.check(createToolUse("tool", "tool-name"))
expect(result2.allowExecution).toBe(true)
// Third call (counter = 3) should be blocked
// Third identical call (counter = 2) should be blocked
const result3 = detector.check(createToolUse("tool", "tool-name"))
expect(result3.allowExecution).toBe(false)
expect(result3.askUser).toBeDefined()
})
it("should allow first 3 calls but block on the 4th call for limit 3 (default)", () => {
const detector = new ToolRepetitionDetector(3)
// First call (counter = 0)
const result1 = detector.check(createToolUse("tool", "tool-name"))
expect(result1.allowExecution).toBe(true)
// Second identical call (counter = 1)
const result2 = detector.check(createToolUse("tool", "tool-name"))
expect(result2.allowExecution).toBe(true)
// Third identical call (counter = 2)
const result3 = detector.check(createToolUse("tool", "tool-name"))
expect(result3.allowExecution).toBe(true)
// Fourth identical call (counter = 3) should be blocked
const result4 = detector.check(createToolUse("tool", "tool-name"))
expect(result4.allowExecution).toBe(false)
expect(result4.askUser).toBeDefined()
})
it("should never block when limit is 0 (unlimited)", () => {
const detector = new ToolRepetitionDetector(0)
@ -318,18 +359,18 @@ describe("ToolRepetitionDetector", () => {
const detector5 = new ToolRepetitionDetector(5)
const tool = createToolUse("tool", "tool-name")
// First 4 calls should be allowed
for (let i = 0; i < 4; i++) {
// First 5 calls should be allowed
for (let i = 0; i < 5; i++) {
const result = detector5.check(tool)
expect(result.allowExecution).toBe(true)
expect(result.askUser).toBeUndefined()
}
// 5th call should be blocked
const result5 = detector5.check(tool)
expect(result5.allowExecution).toBe(false)
expect(result5.askUser).toBeDefined()
expect(result5.askUser?.messageKey).toBe("mistake_limit_reached")
// 6th call should be blocked
const result6 = detector5.check(tool)
expect(result6.allowExecution).toBe(false)
expect(result6.askUser).toBeDefined()
expect(result6.askUser?.messageKey).toBe("mistake_limit_reached")
})
it("should reset counter after blocking and allow new attempts", () => {
@ -339,7 +380,10 @@ describe("ToolRepetitionDetector", () => {
// First call allowed
expect(detector.check(tool).allowExecution).toBe(true)
// Second call should block (limit is 2)
// Second call allowed
expect(detector.check(tool).allowExecution).toBe(true)
// Third call should block (limit is 2)
const blocked = detector.check(tool)
expect(blocked.allowExecution).toBe(false)