From 041233723b999204ec4857cfde6df41be22abb98 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 9 Jul 2025 04:15:02 +0000 Subject: [PATCH] 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 --- src/core/tools/ToolRepetitionDetector.ts | 140 +++++++++++- .../__tests__/ToolRepetitionDetector.spec.ts | 209 ++++++++++++++++++ 2 files changed, 342 insertions(+), 7 deletions(-) diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index a82574ba0e..be910db2b6 100644 --- a/src/core/tools/ToolRepetitionDetector.ts +++ b/src/core/tools/ToolRepetitionDetector.ts @@ -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 + } } diff --git a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts index 972d401141..56dfa70340 100644 --- a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts +++ b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts @@ -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) + }) + }) })