From 538ecae74c86ba610cbd05fd4e4b9db0f674c1f2 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Thu, 29 Jan 2026 13:59:43 +0000 Subject: [PATCH] fix: add path-based repetition detection for read_file tool This enhances the ToolRepetitionDetector to detect when the same file is being read repeatedly with different parameters (e.g., different line ranges). Previously, the detector only caught byte-for-byte identical tool calls. This change adds secondary detection specifically for read_file calls that target the same file path, regardless of other parameters. This helps prevent models like GLM4.5 from getting stuck in loops where they read the same file over and over with slightly different line ranges. Fixes #11071 --- src/core/tools/ToolRepetitionDetector.ts | 123 ++++++++- .../__tests__/ToolRepetitionDetector.spec.ts | 257 ++++++++++++++++++ src/i18n/locales/en/tools.json | 1 + 3 files changed, 380 insertions(+), 1 deletion(-) diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index 9e70bb41a00..5ea8dd073e0 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 3e156dd7c49..b06d97ce541 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 94e1820249b..db8dd9b6127 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..."