From 0aa056cb93341cfb991799e499b09bb28cfd3fdf Mon Sep 17 00:00:00 2001
From: daniel-lxs <ricciodaniel98@gmail.com>
Date: Fri, 14 Nov 2025 16:18:29 -0500
Subject: [PATCH] fix: prevent duplicate tool_result blocks in native protocol
 mode for read_file

When read_file encountered errors (e.g., file not found), it would call
handleError() which internally calls pushToolResult(), then continue to
call pushToolResult() again with the final XML. In native protocol mode,
this created two tool_result blocks with the same tool_call_id, causing
400 errors on subsequent API calls.

This fix replaces handleError() with task.say() for error notifications.
The agent still receives error details through the XML in the single
final pushToolResult() call.

This change works for both protocols:
- Native: Only one tool_result per tool_call_id (fixes duplicate issue)
- XML: Only one text block with complete XML (cleaner than before)

Agent visibility preserved: Errors are included in the XML response
sent to the agent via pushToolResult().

Tests: All 44 tests passing. Updated test to verify say() is called.
---
 src/core/tools/ReadFileTool.ts                | 13 +++++--------
 src/core/tools/__tests__/readFileTool.spec.ts | 10 ++++------
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts
index 11c1e077997..363e6065073 100644
--- a/src/core/tools/ReadFileTool.ts
+++ b/src/core/tools/ReadFileTool.ts
@@ -147,7 +147,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								error: errorMsg,
 								xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
 							})
-							await handleError(`reading file ${relPath}`, new Error(errorMsg))
+							await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 							hasRangeError = true
 							break
 						}
@@ -158,7 +158,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								error: errorMsg,
 								xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
 							})
-							await handleError(`reading file ${relPath}`, new Error(errorMsg))
+							await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 							hasRangeError = true
 							break
 						}
@@ -363,10 +363,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 									error: `Error reading image file: ${errorMsg}`,
 									xmlContent: `<file><path>${relPath}</path><error>Error reading image file: ${errorMsg}</error></file>`,
 								})
-								await handleError(
-									`reading image file ${relPath}`,
-									error instanceof Error ? error : new Error(errorMsg),
-								)
+								await task.say("error", `Error reading image file ${relPath}: ${errorMsg}`)
 								continue
 							}
 						}
@@ -498,7 +495,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 						error: `Error reading file: ${errorMsg}`,
 						xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
 					})
-					await handleError(`reading file ${relPath}`, error instanceof Error ? error : new Error(errorMsg))
+					await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 				}
 			}
 
@@ -570,7 +567,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 				})
 			}
 
-			await handleError(`reading file ${relPath}`, error instanceof Error ? error : new Error(errorMsg))
+			await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 
 			const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
 
diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts
index bff1f6c58f4..a81906718b9 100644
--- a/src/core/tools/__tests__/readFileTool.spec.ts
+++ b/src/core/tools/__tests__/readFileTool.spec.ts
@@ -1602,10 +1602,7 @@ describe("read_file tool with image support", () => {
 			// Setup - simulate read error
 			mockedFsReadFile.mockRejectedValue(new Error("Failed to read image"))
 
-			// Create a spy for handleError
-			const handleErrorSpy = vi.fn()
-
-			// Execute with the spy
+			// Execute
 			const argsContent = `<file><path>${testImagePath}</path></file>`
 			const toolUse: ReadFileToolUse = {
 				type: "tool_use",
@@ -1616,7 +1613,7 @@ describe("read_file tool with image support", () => {
 
 			await readFileTool.handle(localMockCline, toolUse, {
 				askApproval: localMockCline.ask,
-				handleError: handleErrorSpy, // Use our spy here
+				handleError: vi.fn(),
 				pushToolResult: (result: ToolResponse) => {
 					toolResult = result
 				},
@@ -1625,7 +1622,8 @@ describe("read_file tool with image support", () => {
 
 			// Verify error handling
 			expect(toolResult).toContain("<error>Error reading image file: Failed to read image</error>")
-			expect(handleErrorSpy).toHaveBeenCalled()
+			// Verify that say was called to show error to user
+			expect(localMockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Failed to read image"))
 		})
 	})