From 37479466bc615057ee1865c395d00a3b5e0569a2 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 18 Nov 2025 20:39:33 -0500 Subject: [PATCH 1/2] fix: Include nativeArgs in tool repetition detection Fixes false positive 'stuck in a loop' error for native protocol tools like read_file that store parameters in nativeArgs instead of params. Previously, the ToolRepetitionDetector only compared the params object, which was empty for native protocol tools. This caused all read_file calls to appear identical, triggering false loop detection even when reading different files. Changes: - Updated serializeToolUse() to include nativeArgs in comparison - Added comprehensive tests for native protocol scenarios - Maintains backward compatibility with XML protocol tools Closes: Issue reported in Discord about read_file loop detection --- src/core/tools/ToolRepetitionDetector.ts | 17 ++- .../__tests__/ToolRepetitionDetector.spec.ts | 135 ++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index 1d88c66ad7e..a95fc74077c 100644 --- a/src/core/tools/ToolRepetitionDetector.ts +++ b/src/core/tools/ToolRepetitionDetector.ts @@ -108,10 +108,25 @@ export class ToolRepetitionDetector { } } - // Create the object with the tool name and sorted parameters + // For native protocol tools, also include nativeArgs if present + // This ensures tools with array parameters (like read_file with files array) + // are properly differentiated instead of all appearing identical + const sortedNativeArgs: Record = {} + if (toolUse.nativeArgs && typeof toolUse.nativeArgs === "object") { + const nativeKeys = Object.keys(toolUse.nativeArgs).sort() + for (const key of nativeKeys) { + if (Object.prototype.hasOwnProperty.call(toolUse.nativeArgs, key)) { + sortedNativeArgs[key] = (toolUse.nativeArgs as Record)[key] + } + } + } + + // Create the object with the tool name, sorted parameters, and sorted native args const toolObject = { name: toolUse.name, parameters: sortedParams, + // Only include nativeArgs if it has content + ...(Object.keys(sortedNativeArgs).length > 0 ? { nativeArgs: sortedNativeArgs } : {}), } // Convert to a canonical JSON string diff --git a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts index 8313d83960f..3e156dd7c49 100644 --- a/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts +++ b/src/core/tools/__tests__/ToolRepetitionDetector.spec.ts @@ -562,4 +562,139 @@ describe("ToolRepetitionDetector", () => { expect(result.askUser).toBeDefined() }) }) + + // ===== Native Protocol (nativeArgs) tests ===== + describe("native protocol with nativeArgs", () => { + it("should differentiate read_file calls with different files in nativeArgs", () => { + const detector = new ToolRepetitionDetector(2) + + // Create read_file tool use with nativeArgs (like native protocol does) + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, // Empty for native protocol + partial: false, + nativeArgs: { + files: [{ path: "file1.ts" }], + }, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, // Empty for native protocol + partial: false, + nativeArgs: { + files: [{ path: "file2.ts" }], + }, + } + + // First call with file1 + expect(detector.check(readFile1).allowExecution).toBe(true) + + // Second call with file2 - should be treated as different + expect(detector.check(readFile2).allowExecution).toBe(true) + + // Third call with file1 again - should reset counter + expect(detector.check(readFile1).allowExecution).toBe(true) + }) + + it("should detect repetition when same files are read multiple times with nativeArgs", () => { + const detector = new ToolRepetitionDetector(2) + + // Create identical read_file tool uses + const readFile: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, // Empty for native protocol + partial: false, + nativeArgs: { + files: [{ path: "same-file.ts" }], + }, + } + + // First call allowed + expect(detector.check(readFile).allowExecution).toBe(true) + + // Second call allowed + expect(detector.check(readFile).allowExecution).toBe(true) + + // Third identical call should be blocked (limit is 2) + const result = detector.check(readFile) + expect(result.allowExecution).toBe(false) + expect(result.askUser).toBeDefined() + }) + + it("should differentiate read_file calls with multiple files in different orders", () => { + const detector = new ToolRepetitionDetector(2) + + const readFile1: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, + partial: false, + nativeArgs: { + files: [{ path: "a.ts" }, { path: "b.ts" }], + }, + } + + const readFile2: ToolUse = { + type: "tool_use", + name: "read_file" as ToolName, + params: {}, + partial: false, + nativeArgs: { + files: [{ path: "b.ts" }, { path: "a.ts" }], + }, + } + + // Different order should be treated as different calls + expect(detector.check(readFile1).allowExecution).toBe(true) + expect(detector.check(readFile2).allowExecution).toBe(true) + }) + + it("should handle tools with both params and nativeArgs", () => { + const detector = new ToolRepetitionDetector(2) + + const tool1: ToolUse = { + type: "tool_use", + name: "execute_command" as ToolName, + params: { command: "ls" }, + partial: false, + nativeArgs: { + command: "ls", + cwd: "/home/user", + }, + } + + const tool2: ToolUse = { + type: "tool_use", + name: "execute_command" as ToolName, + params: { command: "ls" }, + partial: false, + nativeArgs: { + command: "ls", + cwd: "/home/admin", + }, + } + + // Different cwd in nativeArgs should make these different + expect(detector.check(tool1).allowExecution).toBe(true) + expect(detector.check(tool2).allowExecution).toBe(true) + }) + + it("should handle tools with only params (no nativeArgs)", () => { + const detector = new ToolRepetitionDetector(2) + + const legacyTool = createToolUse("read_file", "read_file", { path: "test.txt" }) + + // Should work the same as before + expect(detector.check(legacyTool).allowExecution).toBe(true) + expect(detector.check(legacyTool).allowExecution).toBe(true) + + const result = detector.check(legacyTool) + expect(result.allowExecution).toBe(false) + expect(result.askUser).toBeDefined() + }) + }) }) From fb69d0d240ea8e12d9c63ca707bbc8d2173edb17 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 18 Nov 2025 21:05:31 -0500 Subject: [PATCH 2/2] Try to use safe-stable-stringify in the tool repetition detector --- pnpm-lock.yaml | 11 ++++++- src/core/tools/ToolRepetitionDetector.ts | 40 +++++------------------- src/package.json | 1 + 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5f82147ecb8..cb94763a83b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -773,6 +773,9 @@ importers: reconnecting-eventsource: specifier: ^1.6.4 version: 1.6.4 + safe-stable-stringify: + specifier: ^2.5.0 + version: 2.5.0 sanitize-filename: specifier: ^1.6.3 version: 1.6.3 @@ -8831,6 +8834,10 @@ packages: resolution: {integrity: sha512-x/+Cz4YrimQxQccJf5mKEbIa1NzeCRNI5Ecl/ekmlYaampdNLPalVyIcCZNNH3MvmqBugV5TMYZXv0ljslUlaw==} engines: {node: '>= 0.4'} + safe-stable-stringify@2.5.0: + resolution: {integrity: sha512-b3rppTKm9T+PsVCBEOUR46GWI7fdOs00VKZ1+9c1EWDaDMvjQc6tUwuFyIprgGgTcWoVHSKrU8H31ZHA2e0RHA==} + engines: {node: '>=10'} + safer-buffer@2.1.2: resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} @@ -14066,7 +14073,7 @@ snapshots: sirv: 3.0.1 tinyglobby: 0.2.14 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.2.1)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.50)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0) '@vitest/utils@3.2.4': dependencies: @@ -19361,6 +19368,8 @@ snapshots: es-errors: 1.3.0 is-regex: 1.2.1 + safe-stable-stringify@2.5.0: {} + safer-buffer@2.1.2: {} sanitize-filename@1.6.3: diff --git a/src/core/tools/ToolRepetitionDetector.ts b/src/core/tools/ToolRepetitionDetector.ts index a95fc74077c..9e70bb41a00 100644 --- a/src/core/tools/ToolRepetitionDetector.ts +++ b/src/core/tools/ToolRepetitionDetector.ts @@ -1,3 +1,4 @@ +import stringify from "safe-stable-stringify" import { ToolUse } from "../../shared/tools" import { t } from "../../i18n" @@ -95,41 +96,16 @@ export class ToolRepetitionDetector { * @returns JSON string representation of the tool use with sorted parameter keys */ private serializeToolUse(toolUse: ToolUse): string { - // Create a new parameters object with alphabetically sorted keys - const sortedParams: Record = {} - - // Get parameter keys and sort them alphabetically - const sortedKeys = Object.keys(toolUse.params).sort() - - // Populate the sorted parameters object in a type-safe way - for (const key of sortedKeys) { - if (Object.prototype.hasOwnProperty.call(toolUse.params, key)) { - sortedParams[key] = toolUse.params[key as keyof typeof toolUse.params] - } - } - - // For native protocol tools, also include nativeArgs if present - // This ensures tools with array parameters (like read_file with files array) - // are properly differentiated instead of all appearing identical - const sortedNativeArgs: Record = {} - if (toolUse.nativeArgs && typeof toolUse.nativeArgs === "object") { - const nativeKeys = Object.keys(toolUse.nativeArgs).sort() - for (const key of nativeKeys) { - if (Object.prototype.hasOwnProperty.call(toolUse.nativeArgs, key)) { - sortedNativeArgs[key] = (toolUse.nativeArgs as Record)[key] - } - } + const toolObject: Record = { + name: toolUse.name, + params: toolUse.params, } - // Create the object with the tool name, sorted parameters, and sorted native args - const toolObject = { - name: toolUse.name, - parameters: sortedParams, - // Only include nativeArgs if it has content - ...(Object.keys(sortedNativeArgs).length > 0 ? { nativeArgs: sortedNativeArgs } : {}), + // Only include nativeArgs if it has content + if (toolUse.nativeArgs && Object.keys(toolUse.nativeArgs).length > 0) { + toolObject.nativeArgs = toolUse.nativeArgs } - // Convert to a canonical JSON string - return JSON.stringify(toolObject) + return stringify(toolObject) } } diff --git a/src/package.json b/src/package.json index fa678beaeb4..dde6913b638 100644 --- a/src/package.json +++ b/src/package.json @@ -509,6 +509,7 @@ "puppeteer-chromium-resolver": "^24.0.0", "puppeteer-core": "^23.4.0", "reconnecting-eventsource": "^1.6.4", + "safe-stable-stringify": "^2.5.0", "sanitize-filename": "^1.6.3", "say": "^0.16.0", "serialize-error": "^12.0.0",