fix: Include nativeArgs in tool repetition detection (#9377)

* 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

* Try to use safe-stable-stringify in the tool repetition detector

---------

Co-authored-by: Matt Rubens <[email protected]>

Author: daniel-lxs

pnpm-lock.yaml

@@ -1,3 +1,4 @@
+import stringify from "safe-stable-stringify"
 import { ToolUse } from "../../shared/tools"
 import { t } from "../../i18n"
 
@@ -95,26 +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<string, unknown> = {}
-
-		// 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]
-			}
+		const toolObject: Record<string, any> = {
+			name: toolUse.name,
+			params: toolUse.params,
 		}
 
-		// Create the object with the tool name and sorted parameters
-		const toolObject = {
-			name: toolUse.name,
-			parameters: sortedParams,
+		// 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)
 	}
 }

src/core/tools/ToolRepetitionDetector.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()
+		})
+	})
 })

src/core/tools/__tests__/ToolRepetitionDetector.spec.ts

@@ -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",