diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index 9e70bb41a0..5ea8dd073e 100644 --- a/src/core/tools/ToolRepetitionDetector.ts +++ b/src/core/tools/ToolRepetitionDetector.ts @@ -5,18 +5,28 @@ import { t } from "../../i18n" /** * Class for detecting consecutive identical tool calls * to prevent the AI from getting stuck in a loop. + * + * Also includes path-based detection for read_file to catch cases where + * the model reads the same file with different parameters (e.g., different line ranges). */ export class ToolRepetitionDetector { private previousToolCallJson: string | null = null private consecutiveIdenticalToolCallCount: number = 0 private readonly consecutiveIdenticalToolCallLimit: number + // Path-based tracking for read_file + private previousReadFilePaths: string | null = null + private consecutiveReadFilePathCount: number = 0 + private readonly readFilePathLimit: number + /** * Creates a new ToolRepetitionDetector * @param limit The maximum number of identical consecutive tool calls allowed + * @param readFilePathLimit The maximum number of consecutive read_file calls for the same file path (default: same as limit) */ - constructor(limit: number = 3) { + constructor(limit: number = 3, readFilePathLimit?: number) { this.consecutiveIdenticalToolCallLimit = limit + this.readFilePathLimit = readFilePathLimit ?? limit } /** @@ -40,6 +50,12 @@ export class ToolRepetitionDetector { return { allowExecution: true } } + // Check path-based repetition for read_file + const pathRepetitionResult = this.checkReadFilePathRepetition(currentToolCallBlock) + if (!pathRepetitionResult.allowExecution) { + return pathRepetitionResult + } + // Serialize the block to a canonical JSON string for comparison const currentToolCallJson = this.serializeToolUse(currentToolCallBlock) @@ -74,6 +90,111 @@ export class ToolRepetitionDetector { return { allowExecution: true } } + /** + * Checks for path-based repetition specifically for read_file tool. + * This catches cases where the model reads the same file with different parameters + * (e.g., different line ranges), which would not be caught by identical call detection. + * + * @param toolUse The ToolUse object to check + * @returns Object indicating if execution is allowed and a message to show if not + */ + private checkReadFilePathRepetition(toolUse: ToolUse): { + allowExecution: boolean + askUser?: { + messageKey: string + messageDetail: string + } + } { + // Only apply to read_file tool + if (toolUse.name !== "read_file") { + // Reset path tracking when switching to a different tool + this.previousReadFilePaths = null + this.consecutiveReadFilePathCount = 0 + return { allowExecution: true } + } + + // Extract file paths from the tool use + const currentPaths = this.extractReadFilePaths(toolUse) + + // Compare with previous paths + if (this.previousReadFilePaths === currentPaths) { + this.consecutiveReadFilePathCount++ + } else { + this.consecutiveReadFilePathCount = 0 + this.previousReadFilePaths = currentPaths + } + + // Check if limit is reached (0 means unlimited) + if (this.readFilePathLimit > 0 && this.consecutiveReadFilePathCount >= this.readFilePathLimit) { + // Reset counters to allow recovery if user guides the AI past this point + this.consecutiveReadFilePathCount = 0 + this.previousReadFilePaths = null + + // Return result indicating execution should not be allowed + return { + allowExecution: false, + askUser: { + messageKey: "mistake_limit_reached", + messageDetail: t("tools:readFilePathRepetitionLimitReached", { toolName: toolUse.name }), + }, + } + } + + return { allowExecution: true } + } + + /** + * Extracts file paths from a read_file tool use. + * Handles both params-based and nativeArgs-based formats. + * + * @param toolUse The read_file ToolUse object + * @returns A canonical string representation of the file paths + */ + private extractReadFilePaths(toolUse: ToolUse): string { + const paths: string[] = [] + + // Check nativeArgs first (native protocol format) + if (toolUse.nativeArgs && typeof toolUse.nativeArgs === "object") { + const nativeArgs = toolUse.nativeArgs as { files?: Array<{ path?: string }> } + if (nativeArgs.files && Array.isArray(nativeArgs.files)) { + for (const file of nativeArgs.files) { + if (file.path) { + paths.push(file.path) + } + } + } + } + + // Check params (legacy format or if nativeArgs didn't have files) + if (paths.length === 0 && toolUse.params) { + // Single file path + if (toolUse.params.path) { + paths.push(toolUse.params.path as string) + } + // Multiple files format (params.files is a JSON array string or array) + if (toolUse.params.files) { + const files = toolUse.params.files + if (typeof files === "string") { + try { + const parsed = JSON.parse(files) + if (Array.isArray(parsed)) { + for (const file of parsed) { + if (file.path) { + paths.push(file.path) + } + } + } + } catch { + // Ignore parse errors + } + } + } + } + + // Sort paths for consistent comparison + return paths.sort().join("|") + } + /** * Checks if a tool use is a browser scroll action * diff --git a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts index 3e156dd7c4..b06d97ce54 100644 --- a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts +++ b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts @@ -12,6 +12,10 @@ vitest.mock("../../../i18n", () => ({ if (key === "tools:toolRepetitionLimitReached" && options?.toolName) { return `Roo appears to be stuck in a loop, attempting the same action (${options.toolName}) repeatedly. This might indicate a problem with its current strategy.` } + // For readFilePathRepetitionLimitReached key, return a specific message. + if (key === "tools:readFilePathRepetitionLimitReached") { + return `Roo appears to be stuck reading the same file repeatedly with different parameters. This might indicate a problem with its current strategy.` + } return key }), })) @@ -697,4 +701,257 @@ describe("ToolRepetitionDetector", () => { expect(result.askUser).toBeDefined() }) }) + + // ===== Path-based repetition detection for read_file ===== + describe("path-based read_file repetition detection", () => { + it("should detect repetition when same file is read with different line ranges", () => { + const detector = new ToolRepetitionDetector(3) + + // First read of file with lines 1-50 + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "1", end_line: "50" }, + partial: false, + } + + // Second read of same file with lines 51-100 + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "51", end_line: "100" }, + partial: false, + } + + // Third read of same file with lines 101-150 + const readFile3: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "101", end_line: "150" }, + partial: false, + } + + // Fourth read of same file - should trigger path-based detection + const readFile4: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "151", end_line: "200" }, + partial: false, + } + + // First three calls allowed + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + expect(detector.check(readFile3).allowExecution).toBe(true) + + // Fourth call should be blocked by path-based detection + const result = detector.check(readFile4) + expect(result.allowExecution).toBe(false) + expect(result.askUser).toBeDefined() + expect(result.askUser?.messageKey).toBe("mistake_limit_reached") + }) + + it("should detect repetition with nativeArgs format and different line ranges", () => { + const detector = new ToolRepetitionDetector(2) + + // Read same file with different nativeArgs line ranges + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, + partial: false, + nativeArgs: { + files: [{ path: "same-file.ts" }], + }, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, + partial: false, + nativeArgs: { + files: [{ path: "same-file.ts" }], + }, + } + + const readFile3: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, + partial: false, + nativeArgs: { + files: [{ path: "same-file.ts" }], + }, + } + + // First two allowed + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + + // Third should be blocked + const result = detector.check(readFile3) + expect(result.allowExecution).toBe(false) + expect(result.askUser).toBeDefined() + }) + + it("should allow reading different files consecutively", () => { + const detector = new ToolRepetitionDetector(2) + + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "file1.ts" }, + partial: false, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "file2.ts" }, + partial: false, + } + + const readFile3: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "file3.ts" }, + partial: false, + } + + // All should be allowed since they're different files + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + expect(detector.check(readFile3).allowExecution).toBe(true) + }) + + it("should reset path counter when a different tool is used", () => { + const detector = new ToolRepetitionDetector(2) + + const readFileTool: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts" }, + partial: false, + } + + const otherTool = createToolUse("execute_command", "execute_command", { command: "ls" }) + + // Read same file twice + expect(detector.check(readFileTool).allowExecution).toBe(true) + expect(detector.check(readFileTool).allowExecution).toBe(true) + + // Use a different tool - should reset path counter + expect(detector.check(otherTool).allowExecution).toBe(true) + + // Should be able to read the same file again + expect(detector.check(readFileTool).allowExecution).toBe(true) + }) + + it("should use custom readFilePathLimit when provided", () => { + // Set identical call limit to 10, but path limit to 2 + const detector = new ToolRepetitionDetector(10, 2) + + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "1" }, + partial: false, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "50" }, + partial: false, + } + + const readFile3: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "100" }, + partial: false, + } + + // First two allowed + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + + // Third should be blocked by path limit + const result = detector.check(readFile3) + expect(result.allowExecution).toBe(false) + expect(result.askUser).toBeDefined() + }) + + it("should recover after path-based limit is reached", () => { + const detector = new ToolRepetitionDetector(10, 2) // High identical limit, low path limit + + // Use different params each time to avoid identical call detection + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "1" }, + partial: false, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "50" }, + partial: false, + } + + const readFile3: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "100" }, + partial: false, + } + + const readFile4: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "150" }, + partial: false, + } + + // Hit the path-based limit (limit is 2) + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + expect(detector.check(readFile3).allowExecution).toBe(false) // Blocked by path limit + + // Should be able to continue after limit reset + expect(detector.check(readFile4).allowExecution).toBe(true) + }) + + it("should not affect other tools when tracking read_file paths", () => { + const detector = new ToolRepetitionDetector(5) + + // Mix read_file and other tools + const readFileTool: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "1" }, + partial: false, + } + + const executeCommand = createToolUse("execute_command", "execute_command", { command: "ls" }) + + // Read file + expect(detector.check(readFileTool).allowExecution).toBe(true) + + // Execute command multiple times - should not be affected by read_file path tracking + expect(detector.check(executeCommand).allowExecution).toBe(true) + expect(detector.check(executeCommand).allowExecution).toBe(true) + + // Read the same file (after reset from different tool) + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: { path: "test.ts", start_line: "50" }, + partial: false, + } + expect(detector.check(readFile2).allowExecution).toBe(true) + }) + }) }) diff --git a/src/i18n/locales/en/tools.json b/src/i18n/locales/en/tools.json index 94e1820249..db8dd9b612 100644 --- a/src/i18n/locales/en/tools.json +++ b/src/i18n/locales/en/tools.json @@ -7,6 +7,7 @@ "imageWithSize": "Image file ({{size}} KB)" }, "toolRepetitionLimitReached": "Roo appears to be stuck in a loop, attempting the same action ({{toolName}}) repeatedly. This might indicate a problem with its current strategy. Consider rephrasing the task, providing more specific instructions, or guiding it towards a different approach.", + "readFilePathRepetitionLimitReached": "Roo appears to be stuck reading the same file repeatedly with different parameters. This might indicate a problem with its current strategy. Consider rephrasing the task, providing more specific instructions, or guiding it towards a different approach.", "unknownToolError": "Roo tried to use an unknown tool: \"{{toolName}}\". Retrying...", "codebaseSearch": { "approval": "Searching for '{{query}}' in codebase..."