scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-10-07 02:58:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: prevent duplicate tool_result blocks in native protocol mode for read_file (#9272)

Browse source 
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.
This commit is contained in:
Daniel 2025-11-14 17:47:39 -05:00 • committed by GitHub
parent 4a9ff8f4ef
commit 1b196461f3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 9 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/core/tools/ReadFileTool.ts
View file

@ -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)

10
src/core/tools/__tests__/readFileTool.spec.ts
View file

@ -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"))
})
})