fix: format tool responses for native protocol (#9270)

* fix: format tool responses for native protocol

- Add toolResultFormatting utilities for protocol detection
- ReadFileTool now builds both XML and native formats
- Native format returns clean, readable text without XML tags
- Legacy conversation history conversion is protocol-aware
- All tests passing (55 total)

* refactor: use isNativeProtocol from @roo-code/types

Remove duplicate implementation and import from types package instead

src/core/task/Task.ts

@@ -10,6 +10,8 @@ import delay from "delay"
 import pWaitFor from "p-wait-for"
 import { serializeError } from "serialize-error"
 import { Package } from "../../shared/package"
+import { isNativeProtocol } from "@roo-code/types"
+import { getCurrentToolProtocol, formatToolInvocation } from "../tools/helpers/toolResultFormatting"
 
 import {
 	type TaskLike,
@@ -1399,19 +1401,21 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		let existingApiConversationHistory: ApiMessage[] = await this.getSavedApiConversationHistory()
 
 		// v2.0 xml tags refactor caveat: since we don't use tools anymore, we need to replace all tool use blocks with a text block since the API disallows conversations with tool uses and no tool schema
+		// Now also protocol-aware: format according to current protocol setting
+		const protocol = getCurrentToolProtocol()
+		const useNative = isNativeProtocol(protocol)
+
 		const conversationWithoutToolBlocks = existingApiConversationHistory.map((message) => {
 			if (Array.isArray(message.content)) {
 				const newContent = message.content.map((block) => {
 					if (block.type === "tool_use") {
-						// It's important we convert to the new tool schema
-						// format so the model doesn't get confused about how to
-						// invoke tools.
-						const inputAsXml = Object.entries(block.input as Record<string, string>)
-							.map(([key, value]) => `<${key}>\n${value}\n</${key}>`)
-							.join("\n")
+						// Format tool invocation based on protocol
+						const params = block.input as Record<string, any>
+						const formattedText = formatToolInvocation(block.name, params, protocol)
+
 						return {
 							type: "text",
-							text: `<${block.name}>\n${inputAsXml}\n</${block.name}>`,
+							text: formattedText,
 						} as Anthropic.Messages.TextBlockParam
 					} else if (block.type === "tool_result") {
 						// Convert block.content to text block array, removing images

src/core/tools/ReadFileTool.ts

@@ -1,6 +1,7 @@
 import path from "path"
 import { isBinaryFile } from "isbinaryfile"
 import type { FileEntry, LineRange } from "@roo-code/types"
+import { isNativeProtocol } from "@roo-code/types"
 
 import { Task } from "../task/Task"
 import { ClineSayTool } from "../../shared/ExtensionMessage"
@@ -14,6 +15,7 @@ import { readLines } from "../../integrations/misc/read-lines"
 import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text"
 import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
 import { parseXml } from "../../utils/xml"
+import { getCurrentToolProtocol } from "./helpers/toolResultFormatting"
 import {
 	DEFAULT_MAX_IMAGE_FILE_SIZE_MB,
 	DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB,
@@ -35,6 +37,7 @@ interface FileResult {
 	notice?: string
 	lineRanges?: LineRange[]
 	xmlContent?: string
+	nativeContent?: string
 	imageDataUrl?: string
 	feedbackText?: string
 	feedbackImages?: any[]
@@ -105,12 +108,15 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 	async execute(params: { files: FileEntry[] }, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { handleError, pushToolResult } = callbacks
 		const fileEntries = params.files
+		const protocol = getCurrentToolProtocol()
+		const useNative = isNativeProtocol(protocol)
 
 		if (!fileEntries || fileEntries.length === 0) {
 			task.consecutiveMistakeCount++
 			task.recordToolError("read_file")
 			const errorMsg = await task.sayAndCreateMissingParamError("read_file", "args (containing valid file paths)")
-			pushToolResult(`<files><error>${errorMsg}</error></files>`)
+			const errorResult = useNative ? `Error: ${errorMsg}` : `<files><error>${errorMsg}</error></files>`
+			pushToolResult(errorResult)
 			return
 		}
 
@@ -146,6 +152,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								status: "blocked",
 								error: errorMsg,
 								xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
+								nativeContent: `File: ${relPath}\nError: Error reading file: ${errorMsg}`,
 							})
 							await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 							hasRangeError = true
@@ -157,6 +164,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								status: "blocked",
 								error: errorMsg,
 								xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
+								nativeContent: `File: ${relPath}\nError: Error reading file: ${errorMsg}`,
 							})
 							await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 							hasRangeError = true
@@ -175,6 +183,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 							status: "blocked",
 							error: errorMsg,
 							xmlContent: `<file><path>${relPath}</path><error>${errorMsg}</error></file>`,
+							nativeContent: `File: ${relPath}\nError: ${errorMsg}`,
 						})
 						continue
 					}
@@ -228,6 +237,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 						updateFileResult(fileResult.path, {
 							status: "denied",
 							xmlContent: `<file><path>${fileResult.path}</path><status>Denied by user</status></file>`,
+							nativeContent: `File: ${fileResult.path}\nStatus: Denied by user`,
 							feedbackText: text,
 							feedbackImages: images,
 						})
@@ -248,6 +258,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								updateFileResult(fileResult.path, {
 									status: "denied",
 									xmlContent: `<file><path>${fileResult.path}</path><status>Denied by user</status></file>`,
+									nativeContent: `File: ${fileResult.path}\nStatus: Denied by user`,
 								})
 							}
 						})
@@ -260,6 +271,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 							updateFileResult(fileResult.path, {
 								status: "denied",
 								xmlContent: `<file><path>${fileResult.path}</path><status>Denied by user</status></file>`,
+								nativeContent: `File: ${fileResult.path}\nStatus: Denied by user`,
 							})
 						})
 					}
@@ -299,6 +311,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 					updateFileResult(relPath, {
 						status: "denied",
 						xmlContent: `<file><path>${relPath}</path><status>Denied by user</status></file>`,
+						nativeContent: `File: ${relPath}\nStatus: Denied by user`,
 						feedbackText: text,
 						feedbackImages: images,
 					})
@@ -343,6 +356,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 									await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource)
 									updateFileResult(relPath, {
 										xmlContent: `<file><path>${relPath}</path>\n<notice>${validationResult.notice}</notice>\n</file>`,
+										nativeContent: `File: ${relPath}\nNote: ${validationResult.notice}`,
 									})
 									continue
 								}
@@ -353,6 +367,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 
 								updateFileResult(relPath, {
 									xmlContent: `<file><path>${relPath}</path>\n<notice>${imageResult.notice}</notice>\n</file>`,
+									nativeContent: `File: ${relPath}\nNote: ${imageResult.notice}`,
 									imageDataUrl: imageResult.dataUrl,
 								})
 								continue
@@ -362,6 +377,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 									status: "error",
 									error: `Error reading image file: ${errorMsg}`,
 									xmlContent: `<file><path>${relPath}</path><error>Error reading image file: ${errorMsg}</error></file>`,
+									nativeContent: `File: ${relPath}\nError: Error reading image file: ${errorMsg}`,
 								})
 								await task.say("error", `Error reading image file ${relPath}: ${errorMsg}`)
 								continue
@@ -375,6 +391,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 							updateFileResult(relPath, {
 								notice: `Binary file format: ${fileFormat}`,
 								xmlContent: `<file><path>${relPath}</path>\n<binary_file format="${fileFormat}">Binary file - content not displayed</binary_file>\n</file>`,
+								nativeContent: `File: ${relPath}\nBinary file (${fileFormat}) - content not displayed`,
 							})
 							continue
 						}
@@ -382,6 +399,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 
 					if (fileResult.lineRanges && fileResult.lineRanges.length > 0) {
 						const rangeResults: string[] = []
+						const nativeRangeResults: string[] = []
+
 						for (const range of fileResult.lineRanges) {
 							const content = addLineNumbers(
 								await readLines(fullPath, range.end - 1, range.start - 1),
@@ -389,9 +408,12 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 							)
 							const lineRangeAttr = ` lines="${range.start}-${range.end}"`
 							rangeResults.push(`<content${lineRangeAttr}>\n${content}</content>`)
+							nativeRangeResults.push(`Lines ${range.start}-${range.end}:\n${content}`)
 						}
+
 						updateFileResult(relPath, {
 							xmlContent: `<file><path>${relPath}</path>\n${rangeResults.join("\n")}\n</file>`,
+							nativeContent: `File: ${relPath}\n${nativeRangeResults.join("\n\n")}`,
 						})
 						continue
 					}
@@ -403,9 +425,10 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 								task.rooIgnoreController,
 							)
 							if (defResult) {
-								let xmlInfo = `<notice>Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines</notice>\n`
+								const notice = `Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines`
 								updateFileResult(relPath, {
-									xmlContent: `<file><path>${relPath}</path>\n<list_code_definition_names>${defResult}</list_code_definition_names>\n${xmlInfo}</file>`,
+									xmlContent: `<file><path>${relPath}</path>\n<list_code_definition_names>${defResult}</list_code_definition_names>\n<notice>${notice}</notice>\n</file>`,
+									nativeContent: `File: ${relPath}\nCode Definitions:\n${defResult}\n\nNote: ${notice}`,
 								})
 							}
 						} catch (error) {
@@ -424,6 +447,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 						const content = addLineNumbers(await readLines(fullPath, maxReadFileLine - 1, 0))
 						const lineRangeAttr = ` lines="1-${maxReadFileLine}"`
 						let xmlInfo = `<content${lineRangeAttr}>\n${content}</content>\n`
+						let nativeInfo = `Lines 1-${maxReadFileLine}:\n${content}\n`
 
 						try {
 							const defResult = await parseSourceCodeDefinitionsForFile(
@@ -433,10 +457,16 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 							if (defResult) {
 								const truncatedDefs = truncateDefinitionsToLineLimit(defResult, maxReadFileLine)
 								xmlInfo += `<list_code_definition_names>${truncatedDefs}</list_code_definition_names>\n`
+								nativeInfo += `\nCode Definitions:\n${truncatedDefs}\n`
 							}
-							xmlInfo += `<notice>Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines</notice>\n`
+
+							const notice = `Showing only ${maxReadFileLine} of ${totalLines} total lines. Use line_range if you need to read more lines`
+							xmlInfo += `<notice>${notice}</notice>\n`
+							nativeInfo += `\nNote: ${notice}`
+
 							updateFileResult(relPath, {
 								xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>`,
+								nativeContent: `File: ${relPath}\n${nativeInfo}`,
 							})
 						} catch (error) {
 							if (error instanceof Error && error.message.startsWith("Unsupported language:")) {
@@ -459,6 +489,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 					let content = await extractTextFromFile(fullPath)
 					let xmlInfo = ""
 
+					let nativeInfo = ""
+
 					if (budgetResult.shouldTruncate && budgetResult.maxChars !== undefined) {
 						const truncateResult = truncateFileContent(
 							content,
@@ -476,31 +508,52 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 						xmlInfo =
 							content.length > 0 ? `<content${lineRangeAttr}>\n${content}</content>\n` : `<content/>`
 						xmlInfo += `<notice>${truncateResult.notice}</notice>\n`
+
+						nativeInfo =
+							content.length > 0
+								? `Lines 1-${displayedLines}:\n${content}\n\nNote: ${truncateResult.notice}`
+								: `Note: ${truncateResult.notice}`
 					} else {
 						const lineRangeAttr = ` lines="1-${totalLines}"`
 						xmlInfo = totalLines > 0 ? `<content${lineRangeAttr}>\n${content}</content>\n` : `<content/>`
 
 						if (totalLines === 0) {
 							xmlInfo += `<notice>File is empty</notice>\n`
+							nativeInfo = "Note: File is empty"
+						} else {
+							nativeInfo = `Lines 1-${totalLines}:\n${content}`
 						}
 					}
 
 					await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource)
 
-					updateFileResult(relPath, { xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>` })
+					updateFileResult(relPath, {
+						xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>`,
+						nativeContent: `File: ${relPath}\n${nativeInfo}`,
+					})
 				} catch (error) {
 					const errorMsg = error instanceof Error ? error.message : String(error)
 					updateFileResult(relPath, {
 						status: "error",
 						error: `Error reading file: ${errorMsg}`,
 						xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
+						nativeContent: `File: ${relPath}\nError: Error reading file: ${errorMsg}`,
 					})
 					await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 				}
 			}
 
-			const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
-			const filesXml = `<files>\n${xmlResults.join("\n")}\n</files>`
+			// Build final result based on protocol
+			let finalResult: string
+			if (useNative) {
+				const nativeResults = fileResults
+					.filter((result) => result.nativeContent)
+					.map((result) => result.nativeContent)
+				finalResult = nativeResults.join("\n\n---\n\n")
+			} else {
+				const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
+				finalResult = `<files>\n${xmlResults.join("\n")}\n</files>`
+			}
 
 			const fileImageUrls = fileResults
 				.filter((result) => result.imageDataUrl)
@@ -534,26 +587,26 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 
 			if (statusMessage || imagesToInclude.length > 0) {
 				const result = formatResponse.toolResult(
-					statusMessage || filesXml,
+					statusMessage || finalResult,
 					imagesToInclude.length > 0 ? imagesToInclude : undefined,
 				)
 
 				if (typeof result === "string") {
 					if (statusMessage) {
-						pushToolResult(`${result}\n${filesXml}`)
+						pushToolResult(`${result}\n${finalResult}`)
 					} else {
 						pushToolResult(result)
 					}
 				} else {
 					if (statusMessage) {
-						const textBlock = { type: "text" as const, text: filesXml }
+						const textBlock = { type: "text" as const, text: finalResult }
 						pushToolResult([...result, textBlock])
 					} else {
 						pushToolResult(result)
 					}
 				}
 			} else {
-				pushToolResult(filesXml)
+				pushToolResult(finalResult)
 			}
 		} catch (error) {
 			const relPath = fileEntries[0]?.path || "unknown"
@@ -564,14 +617,25 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 					status: "error",
 					error: `Error reading file: ${errorMsg}`,
 					xmlContent: `<file><path>${relPath}</path><error>Error reading file: ${errorMsg}</error></file>`,
+					nativeContent: `File: ${relPath}\nError: Error reading file: ${errorMsg}`,
 				})
 			}
 
 			await task.say("error", `Error reading file ${relPath}: ${errorMsg}`)
 
-			const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
+			// Build final error result based on protocol
+			let errorResult: string
+			if (useNative) {
+				const nativeResults = fileResults
+					.filter((result) => result.nativeContent)
+					.map((result) => result.nativeContent)
+				errorResult = nativeResults.join("\n\n---\n\n")
+			} else {
+				const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
+				errorResult = `<files>\n${xmlResults.join("\n")}\n</files>`
+			}
 
-			pushToolResult(`<files>\n${xmlResults.join("\n")}\n</files>`)
+			pushToolResult(errorResult)
 		}
 	}
 

src/core/tools/helpers/__tests__/toolResultFormatting.spec.ts

@@ -0,0 +1,97 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"
+import * as vscode from "vscode"
+import { TOOL_PROTOCOL, isNativeProtocol } from "@roo-code/types"
+import { formatToolInvocation, getCurrentToolProtocol } from "../toolResultFormatting"
+
+vi.mock("vscode", () => ({
+	workspace: {
+		getConfiguration: vi.fn(),
+	},
+}))
+
+describe("toolResultFormatting", () => {
+	let mockGetConfiguration: ReturnType<typeof vi.fn>
+
+	beforeEach(() => {
+		mockGetConfiguration = vi.fn()
+		;(vscode.workspace.getConfiguration as any).mockReturnValue({
+			get: mockGetConfiguration,
+		})
+	})
+
+	afterEach(() => {
+		vi.clearAllMocks()
+	})
+
+	describe("getCurrentToolProtocol", () => {
+		it("should return configured protocol", () => {
+			mockGetConfiguration.mockReturnValue(TOOL_PROTOCOL.NATIVE)
+			expect(getCurrentToolProtocol()).toBe(TOOL_PROTOCOL.NATIVE)
+		})
+
+		it("should default to xml when config is not set", () => {
+			mockGetConfiguration.mockReturnValue("xml")
+			expect(getCurrentToolProtocol()).toBe("xml")
+		})
+	})
+
+	describe("isNativeProtocol", () => {
+		it("should return true for native protocol", () => {
+			expect(isNativeProtocol(TOOL_PROTOCOL.NATIVE)).toBe(true)
+		})
+
+		it("should return false for XML protocol", () => {
+			expect(isNativeProtocol("xml")).toBe(false)
+		})
+	})
+
+	describe("formatToolInvocation", () => {
+		it("should format for XML protocol", () => {
+			const result = formatToolInvocation("read_file", { path: "test.ts" }, "xml")
+
+			expect(result).toContain("<read_file>")
+			expect(result).toContain("<path>")
+			expect(result).toContain("test.ts")
+			expect(result).toContain("</path>")
+			expect(result).toContain("</read_file>")
+		})
+
+		it("should format for native protocol", () => {
+			const result = formatToolInvocation("read_file", { path: "test.ts" }, TOOL_PROTOCOL.NATIVE)
+
+			expect(result).toBe("Called read_file with path: test.ts")
+			expect(result).not.toContain("<")
+		})
+
+		it("should handle multiple parameters for XML", () => {
+			const result = formatToolInvocation(
+				"read_file",
+				{ path: "test.ts", start_line: "1", end_line: "10" },
+				"xml",
+			)
+
+			expect(result).toContain("<path>\ntest.ts\n</path>")
+			expect(result).toContain("<start_line>\n1\n</start_line>")
+			expect(result).toContain("<end_line>\n10\n</end_line>")
+		})
+
+		it("should handle multiple parameters for native", () => {
+			const result = formatToolInvocation("read_file", { path: "test.ts", start_line: "1" }, TOOL_PROTOCOL.NATIVE)
+
+			expect(result).toContain("Called read_file with")
+			expect(result).toContain("path: test.ts")
+			expect(result).toContain("start_line: 1")
+		})
+
+		it("should handle empty parameters", () => {
+			const result = formatToolInvocation("list_files", {}, TOOL_PROTOCOL.NATIVE)
+			expect(result).toBe("Called list_files")
+		})
+
+		it("should use config when protocol not specified", () => {
+			mockGetConfiguration.mockReturnValue(TOOL_PROTOCOL.NATIVE)
+			const result = formatToolInvocation("read_file", { path: "test.ts" })
+			expect(result).toBe("Called read_file with path: test.ts")
+		})
+	})
+})

src/core/tools/helpers/toolResultFormatting.ts

@@ -0,0 +1,31 @@
+import * as vscode from "vscode"
+import { Package } from "../../../shared/package"
+import { TOOL_PROTOCOL, ToolProtocol, isNativeProtocol } from "@roo-code/types"
+
+/**
+ * Gets the current tool protocol from workspace configuration.
+ */
+export function getCurrentToolProtocol(): ToolProtocol {
+	return vscode.workspace.getConfiguration(Package.name).get<ToolProtocol>("toolProtocol", "xml")
+}
+
+/**
+ * Formats tool invocation parameters for display based on protocol.
+ * Used for legacy conversation history conversion.
+ */
+export function formatToolInvocation(toolName: string, params: Record<string, any>, protocol?: ToolProtocol): string {
+	const effectiveProtocol = protocol ?? getCurrentToolProtocol()
+	if (isNativeProtocol(effectiveProtocol)) {
+		// Native protocol: readable format
+		const paramsList = Object.entries(params)
+			.map(([key, value]) => `${key}: ${typeof value === "string" ? value : JSON.stringify(value)}`)
+			.join(", ")
+		return `Called ${toolName}${paramsList ? ` with ${paramsList}` : ""}`
+	} else {
+		// XML protocol: preserve XML format
+		const paramsXml = Object.entries(params)
+			.map(([key, value]) => `<${key}>\n${value}\n</${key}>`)
+			.join("\n")
+		return `<${toolName}>\n${paramsXml}\n</${toolName}>`
+	}
+}