mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: add detection of non-continuous tool repeated calls (#5496)
- Enhanced ToolRepetitionDetector to detect non-continuous repetitive patterns (e.g., ABABAB) - Added pattern detection algorithm that identifies repeating sequences of tool calls - Maintains backward compatibility with existing consecutive repetition detection - Added comprehensive tests for pattern detection scenarios - Addresses issue where AI could get stuck in alternating tool call loops Fixes #5496
This commit is contained in:
parent
7fe1c0f47a
commit
041233723b
2 changed files with 342 additions and 7 deletions
|
|
@ -2,14 +2,21 @@ import { ToolUse } from "../../shared/tools"
|
|||
import { t } from "../../i18n"
|
||||
|
||||
/**
|
||||
* Class for detecting consecutive identical tool calls
|
||||
* to prevent the AI from getting stuck in a loop.
|
||||
* Class for detecting both consecutive and non-consecutive repetitive tool call patterns
|
||||
* to prevent the AI from getting stuck in loops.
|
||||
*/
|
||||
export class ToolRepetitionDetector {
|
||||
private previousToolCallJson: string | null = null
|
||||
private consecutiveIdenticalToolCallCount: number = 0
|
||||
private readonly consecutiveIdenticalToolCallLimit: number
|
||||
|
||||
// Enhanced pattern detection for non-continuous repetitions
|
||||
private toolCallHistory: string[] = []
|
||||
private readonly maxHistorySize: number = 20 // Keep last 20 tool calls for pattern detection
|
||||
private readonly minPatternLength: number = 2 // Minimum pattern length (e.g., AB)
|
||||
private readonly maxPatternLength: number = 6 // Maximum pattern length (e.g., ABCDEF)
|
||||
private readonly minPatternRepetitions: number = 2 // Minimum repetitions to consider it a loop
|
||||
|
||||
/**
|
||||
* Creates a new ToolRepetitionDetector
|
||||
* @param limit The maximum number of identical consecutive tool calls allowed
|
||||
|
|
@ -19,7 +26,7 @@ export class ToolRepetitionDetector {
|
|||
}
|
||||
|
||||
/**
|
||||
* Checks if the current tool call is identical to the previous one
|
||||
* Checks if the current tool call is identical to the previous one or forms a repetitive pattern
|
||||
* and determines if execution should be allowed
|
||||
*
|
||||
* @param currentToolCallBlock ToolUse object representing the current tool call
|
||||
|
|
@ -35,7 +42,10 @@ export class ToolRepetitionDetector {
|
|||
// Serialize the block to a canonical JSON string for comparison
|
||||
const currentToolCallJson = this.serializeToolUse(currentToolCallBlock)
|
||||
|
||||
// Compare with previous tool call
|
||||
// Add to history for pattern detection
|
||||
this.addToHistory(currentToolCallJson)
|
||||
|
||||
// Check for consecutive repetitions (existing behavior)
|
||||
if (this.previousToolCallJson === currentToolCallJson) {
|
||||
this.consecutiveIdenticalToolCallCount++
|
||||
} else {
|
||||
|
|
@ -43,11 +53,10 @@ export class ToolRepetitionDetector {
|
|||
this.previousToolCallJson = currentToolCallJson
|
||||
}
|
||||
|
||||
// Check if limit is reached
|
||||
// Check if consecutive limit is reached
|
||||
if (this.consecutiveIdenticalToolCallCount >= this.consecutiveIdenticalToolCallLimit) {
|
||||
// Reset counters to allow recovery if user guides the AI past this point
|
||||
this.consecutiveIdenticalToolCallCount = 0
|
||||
this.previousToolCallJson = null
|
||||
this.resetState()
|
||||
|
||||
// Return result indicating execution should not be allowed
|
||||
return {
|
||||
|
|
@ -59,6 +68,21 @@ export class ToolRepetitionDetector {
|
|||
}
|
||||
}
|
||||
|
||||
// Check for non-consecutive repetitive patterns
|
||||
const patternDetectionResult = this.detectRepetitivePattern()
|
||||
if (patternDetectionResult.isRepetitive) {
|
||||
// Reset state to allow recovery
|
||||
this.resetState()
|
||||
|
||||
return {
|
||||
allowExecution: false,
|
||||
askUser: {
|
||||
messageKey: "mistake_limit_reached",
|
||||
messageDetail: t("tools:toolRepetitionLimitReached", { toolName: currentToolCallBlock.name }),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// Execution is allowed
|
||||
return { allowExecution: true }
|
||||
}
|
||||
|
|
@ -92,4 +116,106 @@ export class ToolRepetitionDetector {
|
|||
// Convert to a canonical JSON string
|
||||
return JSON.stringify(toolObject)
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds a tool call to the history for pattern detection
|
||||
* @param toolCallJson Serialized tool call JSON
|
||||
*/
|
||||
private addToHistory(toolCallJson: string): void {
|
||||
this.toolCallHistory.push(toolCallJson)
|
||||
|
||||
// Keep history size manageable
|
||||
if (this.toolCallHistory.length > this.maxHistorySize) {
|
||||
this.toolCallHistory.shift() // Remove oldest entry
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resets the internal state of the detector
|
||||
*/
|
||||
private resetState(): void {
|
||||
this.consecutiveIdenticalToolCallCount = 0
|
||||
this.previousToolCallJson = null
|
||||
// Clear history to prevent false positives after reset
|
||||
this.toolCallHistory = []
|
||||
}
|
||||
|
||||
/**
|
||||
* Detects repetitive patterns in the tool call history
|
||||
* @returns Object indicating if a repetitive pattern was detected
|
||||
*/
|
||||
private detectRepetitivePattern(): { isRepetitive: boolean; pattern?: string[] } {
|
||||
// Need at least enough history to detect a pattern
|
||||
if (this.toolCallHistory.length < this.minPatternLength * this.minPatternRepetitions) {
|
||||
return { isRepetitive: false }
|
||||
}
|
||||
|
||||
// Try different pattern lengths, starting from the smallest
|
||||
for (let patternLength = this.minPatternLength; patternLength <= this.maxPatternLength; patternLength++) {
|
||||
// Don't check patterns longer than what we can fit with minimum repetitions
|
||||
if (patternLength * this.minPatternRepetitions > this.toolCallHistory.length) {
|
||||
break
|
||||
}
|
||||
|
||||
// Check if the last N elements form a repeating pattern
|
||||
if (this.isRepeatingPattern(patternLength)) {
|
||||
const pattern = this.toolCallHistory.slice(-patternLength)
|
||||
return { isRepetitive: true, pattern }
|
||||
}
|
||||
}
|
||||
|
||||
return { isRepetitive: false }
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if the end of the history forms a repeating pattern of the given length
|
||||
* @param patternLength Length of the pattern to check
|
||||
* @returns True if a repeating pattern is found
|
||||
*/
|
||||
private isRepeatingPattern(patternLength: number): boolean {
|
||||
const totalLength = patternLength * this.minPatternRepetitions
|
||||
if (this.toolCallHistory.length < totalLength) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Get the pattern from the end
|
||||
const pattern = this.toolCallHistory.slice(-patternLength)
|
||||
|
||||
// Check if this pattern repeats the required number of times
|
||||
for (let i = 1; i < this.minPatternRepetitions; i++) {
|
||||
const startIndex = this.toolCallHistory.length - (i + 1) * patternLength
|
||||
const endIndex = this.toolCallHistory.length - i * patternLength
|
||||
|
||||
if (startIndex < 0) {
|
||||
return false
|
||||
}
|
||||
|
||||
const segment = this.toolCallHistory.slice(startIndex, endIndex)
|
||||
if (!this.arraysEqual(segment, pattern)) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Utility method to check if two arrays are equal
|
||||
* @param arr1 First array
|
||||
* @param arr2 Second array
|
||||
* @returns True if arrays are equal
|
||||
*/
|
||||
private arraysEqual(arr1: string[], arr2: string[]): boolean {
|
||||
if (arr1.length !== arr2.length) {
|
||||
return false
|
||||
}
|
||||
|
||||
for (let i = 0; i < arr1.length; i++) {
|
||||
if (arr1[i] !== arr2[i]) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -302,4 +302,213 @@ describe("ToolRepetitionDetector", () => {
|
|||
expect(result3.askUser).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
// ===== Non-Continuous Pattern Detection tests =====
|
||||
describe("non-continuous pattern detection", () => {
|
||||
it("should detect ABAB pattern repetition", () => {
|
||||
const detector = new ToolRepetitionDetector(5) // Set high limit to focus on pattern detection
|
||||
|
||||
// Create ABAB pattern
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const result = detector.check(createToolUse("toolB", "toolB"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
expect(result.askUser?.messageKey).toBe("mistake_limit_reached")
|
||||
})
|
||||
|
||||
it("should detect ABCABC pattern repetition", () => {
|
||||
const detector = new ToolRepetitionDetector(10) // Set high limit to focus on pattern detection
|
||||
|
||||
// Create ABCABC pattern
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolC", "toolC"))
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
const result = detector.check(createToolUse("toolC", "toolC"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should detect ABAB pattern (2 repetitions)", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create ABAB pattern (2 repetitions of AB)
|
||||
// First AB
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
// Second AB - should trigger after completing the second repetition
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const result = detector.check(createToolUse("toolB", "toolB"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should not detect pattern with insufficient repetitions", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create AB pattern only once (not enough repetitions)
|
||||
const result1 = detector.check(createToolUse("toolA", "toolA"))
|
||||
const result2 = detector.check(createToolUse("toolB", "toolB"))
|
||||
|
||||
expect(result1.allowExecution).toBe(true)
|
||||
expect(result2.allowExecution).toBe(true)
|
||||
})
|
||||
|
||||
it("should allow different tools without pattern", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create sequence without repetitive pattern
|
||||
const result1 = detector.check(createToolUse("toolA", "toolA"))
|
||||
const result2 = detector.check(createToolUse("toolB", "toolB"))
|
||||
const result3 = detector.check(createToolUse("toolC", "toolC"))
|
||||
const result4 = detector.check(createToolUse("toolD", "toolD"))
|
||||
|
||||
expect(result1.allowExecution).toBe(true)
|
||||
expect(result2.allowExecution).toBe(true)
|
||||
expect(result3.allowExecution).toBe(true)
|
||||
expect(result4.allowExecution).toBe(true)
|
||||
})
|
||||
|
||||
it("should detect pattern with different parameters", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create ABAB pattern with different parameters
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
detector.check(createToolUse("write_to_file", "write_to_file", { path: "file2.ts", content: "test" }))
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
const result = detector.check(
|
||||
createToolUse("write_to_file", "write_to_file", { path: "file2.ts", content: "test" }),
|
||||
)
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should reset pattern detection after limit reached", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create ABAB pattern to trigger detection
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const limitResult = detector.check(createToolUse("toolB", "toolB"))
|
||||
expect(limitResult.allowExecution).toBe(false)
|
||||
|
||||
// After reset, should allow new tools
|
||||
const result = detector.check(createToolUse("toolC", "toolC"))
|
||||
expect(result.allowExecution).toBe(true)
|
||||
})
|
||||
|
||||
it("should handle complex patterns like ABCDEFABCDEF", () => {
|
||||
const detector = new ToolRepetitionDetector(15)
|
||||
|
||||
// Create ABCDEFABCDEF pattern
|
||||
const tools = ["toolA", "toolB", "toolC", "toolD", "toolE", "toolF"]
|
||||
|
||||
// First iteration
|
||||
for (const tool of tools) {
|
||||
detector.check(createToolUse(tool, tool))
|
||||
}
|
||||
|
||||
// Second iteration - should trigger on the last tool
|
||||
for (let i = 0; i < tools.length - 1; i++) {
|
||||
detector.check(createToolUse(tools[i], tools[i]))
|
||||
}
|
||||
|
||||
const result = detector.check(createToolUse(tools[tools.length - 1], tools[tools.length - 1]))
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should not detect pattern when tools are similar but not identical", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create similar but not identical pattern - different tools entirely
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
detector.check(createToolUse("write_to_file", "write_to_file", { path: "file2.ts" })) // Different tool
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
const result = detector.check(createToolUse("list_files", "list_files", { path: "." })) // Different tool
|
||||
|
||||
expect(result.allowExecution).toBe(true) // Should allow since tools are different
|
||||
})
|
||||
|
||||
it("should detect pattern when same tool has different parameters in repetitive sequence", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Create pattern with same tool but different parameters - this IS a repetitive pattern
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file2.ts" })) // Different parameter
|
||||
detector.check(createToolUse("read_file", "read_file", { path: "file1.ts" }))
|
||||
const result = detector.check(createToolUse("read_file", "read_file", { path: "file2.ts" }))
|
||||
|
||||
expect(result.allowExecution).toBe(false) // Should detect pattern even with different parameters
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should handle edge case with minimum pattern length", () => {
|
||||
const detector = new ToolRepetitionDetector(10)
|
||||
|
||||
// Test minimum pattern length (2)
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const result = detector.check(createToolUse("toolB", "toolB"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
})
|
||||
|
||||
// ===== Integration tests for both consecutive and pattern detection =====
|
||||
describe("integration: consecutive and pattern detection", () => {
|
||||
it("should prioritize consecutive detection over pattern detection", () => {
|
||||
const detector = new ToolRepetitionDetector(2)
|
||||
|
||||
// This should trigger consecutive detection before pattern detection
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const result = detector.check(createToolUse("toolA", "toolA"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should detect patterns when consecutive limit is higher", () => {
|
||||
const detector = new ToolRepetitionDetector(10) // High consecutive limit
|
||||
|
||||
// Create ABAB pattern - should be caught by pattern detection
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const result = detector.check(createToolUse("toolB", "toolB"))
|
||||
|
||||
expect(result.allowExecution).toBe(false)
|
||||
expect(result.askUser).toBeDefined()
|
||||
})
|
||||
|
||||
it("should work correctly after multiple resets", () => {
|
||||
const detector = new ToolRepetitionDetector(2)
|
||||
|
||||
// First reset via consecutive detection
|
||||
detector.check(createToolUse("toolA", "toolA"))
|
||||
const firstLimit = detector.check(createToolUse("toolA", "toolA"))
|
||||
expect(firstLimit.allowExecution).toBe(false)
|
||||
|
||||
// Second reset via pattern detection
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
detector.check(createToolUse("toolC", "toolC"))
|
||||
detector.check(createToolUse("toolB", "toolB"))
|
||||
const secondLimit = detector.check(createToolUse("toolC", "toolC"))
|
||||
expect(secondLimit.allowExecution).toBe(false)
|
||||
|
||||
// Should work normally after resets
|
||||
const result = detector.check(createToolUse("toolD", "toolD"))
|
||||
expect(result.allowExecution).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue