Browse Source

fix: Update tools to return structured JSON for native protocol (#9373)

Daniel 1 month ago
parent
commit
271d01bada
34 changed files with 320 additions and 94 deletions
  1. 38 9
      src/core/assistant-message/presentAssistantMessage.ts
  2. 96 14
      src/core/prompts/responses.ts
  3. 1 1
      src/core/prompts/system.ts
  4. 2 2
      src/core/tools/ApplyDiffTool.ts
  5. 1 1
      src/core/tools/AskFollowupQuestionTool.ts
  6. 1 1
      src/core/tools/AttemptCompletionTool.ts
  7. 2 1
      src/core/tools/BaseTool.ts
  8. 1 1
      src/core/tools/CodebaseSearchTool.ts
  9. 2 2
      src/core/tools/ExecuteCommandTool.ts
  10. 1 1
      src/core/tools/FetchInstructionsTool.ts
  11. 3 3
      src/core/tools/GenerateImageTool.ts
  12. 2 2
      src/core/tools/InsertContentTool.ts
  13. 11 2
      src/core/tools/MultiApplyDiffTool.ts
  14. 1 1
      src/core/tools/NewTaskTool.ts
  15. 1 1
      src/core/tools/ReadFileTool.ts
  16. 1 1
      src/core/tools/RunSlashCommandTool.ts
  17. 1 1
      src/core/tools/SwitchModeTool.ts
  18. 1 1
      src/core/tools/UpdateTodoListTool.ts
  19. 1 1
      src/core/tools/UseMcpToolTool.ts
  20. 2 2
      src/core/tools/WriteToFileTool.ts
  21. 3 0
      src/core/tools/__tests__/applyDiffTool.experiment.spec.ts
  22. 3 0
      src/core/tools/__tests__/askFollowupQuestionTool.spec.ts
  23. 9 0
      src/core/tools/__tests__/attemptCompletionTool.spec.ts
  24. 5 0
      src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts
  25. 8 5
      src/core/tools/__tests__/executeCommandTool.spec.ts
  26. 9 0
      src/core/tools/__tests__/generateImageTool.test.ts
  27. 1 0
      src/core/tools/__tests__/insertContentTool.spec.ts
  28. 8 0
      src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts
  29. 15 0
      src/core/tools/__tests__/newTaskTool.spec.ts
  30. 6 0
      src/core/tools/__tests__/readFileTool.spec.ts
  31. 12 0
      src/core/tools/__tests__/useMcpToolTool.spec.ts
  32. 1 0
      src/core/tools/__tests__/writeToFileTool.spec.ts
  33. 2 0
      src/core/tools/simpleReadFileTool.ts
  34. 69 41
      src/integrations/editor/DiffViewProvider.ts

+ 38 - 9
src/core/assistant-message/presentAssistantMessage.ts

@@ -13,7 +13,7 @@ import { fetchInstructionsTool } from "../tools/FetchInstructionsTool"
 import { listFilesTool } from "../tools/ListFilesTool"
 import { readFileTool } from "../tools/ReadFileTool"
 import { getSimpleReadFileToolDescription, simpleReadFileTool } from "../tools/simpleReadFileTool"
-import { shouldUseSingleFileRead } from "@roo-code/types"
+import { shouldUseSingleFileRead, TOOL_PROTOCOL } from "@roo-code/types"
 import { writeToFileTool } from "../tools/WriteToFileTool"
 import { applyDiffTool } from "../tools/MultiApplyDiffTool"
 import { insertContentTool } from "../tools/InsertContentTool"
@@ -284,10 +284,10 @@ export async function presentAssistantMessage(cline: Task) {
 			// Native protocol tool calls ALWAYS have an ID (set when parsed from tool_call chunks).
 			// XML protocol tool calls NEVER have an ID (parsed from XML text).
 			const toolCallId = (block as any).id
-			const isNative = !!toolCallId
+			const toolProtocol = toolCallId ? TOOL_PROTOCOL.NATIVE : TOOL_PROTOCOL.XML
 
 			const pushToolResult = (content: ToolResponse) => {
-				if (isNative && toolCallId) {
+				if (toolProtocol === TOOL_PROTOCOL.NATIVE) {
 					// For native protocol, only allow ONE tool_result per tool call
 					if (hasToolResult) {
 						console.warn(
@@ -360,9 +360,14 @@ export async function presentAssistantMessage(cline: Task) {
 					// Handle both messageResponse and noButtonClicked with text.
 					if (text) {
 						await cline.say("user_feedback", text, images)
-						pushToolResult(formatResponse.toolResult(formatResponse.toolDeniedWithFeedback(text), images))
+						pushToolResult(
+							formatResponse.toolResult(
+								formatResponse.toolDeniedWithFeedback(text, toolProtocol),
+								images,
+							),
+						)
 					} else {
-						pushToolResult(formatResponse.toolDenied())
+						pushToolResult(formatResponse.toolDenied(toolProtocol))
 					}
 					cline.didRejectTool = true
 					return false
@@ -371,7 +376,9 @@ export async function presentAssistantMessage(cline: Task) {
 				// Handle yesButtonClicked with text.
 				if (text) {
 					await cline.say("user_feedback", text, images)
-					pushToolResult(formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text), images))
+					pushToolResult(
+						formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text, toolProtocol), images),
+					)
 				}
 
 				return true
@@ -394,7 +401,7 @@ export async function presentAssistantMessage(cline: Task) {
 					`Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`,
 				)
 
-				pushToolResult(formatResponse.toolError(errorString))
+				pushToolResult(formatResponse.toolError(errorString, toolProtocol))
 			}
 
 			// If block is partial, remove partial closing tag so its not
@@ -446,7 +453,7 @@ export async function presentAssistantMessage(cline: Task) {
 				)
 			} catch (error) {
 				cline.consecutiveMistakeCount++
-				pushToolResult(formatResponse.toolError(error.message))
+				pushToolResult(formatResponse.toolError(error.message, toolProtocol))
 				break
 			}
 
@@ -485,6 +492,7 @@ export async function presentAssistantMessage(cline: Task) {
 					pushToolResult(
 						formatResponse.toolError(
 							`Tool call repetition limit reached for ${block.name}. Please try a different approach.`,
+							toolProtocol,
 						),
 					)
 					break
@@ -499,6 +507,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "update_todo_list":
@@ -507,6 +516,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "apply_diff": {
@@ -514,12 +524,13 @@ export async function presentAssistantMessage(cline: Task) {
 
 					// Check if this tool call came from native protocol by checking for ID
 					// Native calls always have IDs, XML calls never do
-					if (isNative) {
+					if (toolProtocol === TOOL_PROTOCOL.NATIVE) {
 						await applyDiffToolClass.handle(cline, block as ToolUse<"apply_diff">, {
 							askApproval,
 							handleError,
 							pushToolResult,
 							removeClosingTag,
+							toolProtocol,
 						})
 						break
 					}
@@ -544,6 +555,7 @@ export async function presentAssistantMessage(cline: Task) {
 							handleError,
 							pushToolResult,
 							removeClosingTag,
+							toolProtocol,
 						})
 					}
 					break
@@ -555,6 +567,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "read_file":
@@ -568,6 +581,7 @@ export async function presentAssistantMessage(cline: Task) {
 							handleError,
 							pushToolResult,
 							removeClosingTag,
+							toolProtocol,
 						)
 					} else {
 						// Type assertion is safe here because we're in the "read_file" case
@@ -576,6 +590,7 @@ export async function presentAssistantMessage(cline: Task) {
 							handleError,
 							pushToolResult,
 							removeClosingTag,
+							toolProtocol,
 						})
 					}
 					break
@@ -585,6 +600,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "list_files":
@@ -593,6 +609,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "codebase_search":
@@ -601,6 +618,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "list_code_definition_names":
@@ -609,6 +627,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "search_files":
@@ -617,6 +636,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "browser_action":
@@ -625,6 +645,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "execute_command":
@@ -633,6 +654,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "use_mcp_tool":
@@ -641,6 +663,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "access_mcp_resource":
@@ -659,6 +682,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "switch_mode":
@@ -667,6 +691,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "new_task":
@@ -675,6 +700,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "attempt_completion": {
@@ -685,6 +711,7 @@ export async function presentAssistantMessage(cline: Task) {
 						removeClosingTag,
 						askFinishSubTaskApproval,
 						toolDescription,
+						toolProtocol,
 					}
 					await attemptCompletionTool.handle(
 						cline,
@@ -699,6 +726,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 				case "generate_image":
@@ -708,6 +736,7 @@ export async function presentAssistantMessage(cline: Task) {
 						handleError,
 						pushToolResult,
 						removeClosingTag,
+						toolProtocol,
 					})
 					break
 			}

+ 96 - 14
src/core/prompts/responses.ts

@@ -6,18 +6,61 @@ import { RooProtectedController } from "../protect/RooProtectedController"
 import { ToolProtocol, isNativeProtocol, TOOL_PROTOCOL } from "@roo-code/types"
 
 export const formatResponse = {
-	toolDenied: () => `The user denied this operation.`,
+	toolDenied: (protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "denied",
+				message: "The user denied this operation.",
+			})
+		}
+		return `The user denied this operation.`
+	},
 
-	toolDeniedWithFeedback: (feedback?: string) =>
-		`The user denied this operation and provided the following feedback:\n<feedback>\n${feedback}\n</feedback>`,
+	toolDeniedWithFeedback: (feedback?: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "denied",
+				message: "The user denied this operation and provided the following feedback",
+				feedback: feedback,
+			})
+		}
+		return `The user denied this operation and provided the following feedback:\n<feedback>\n${feedback}\n</feedback>`
+	},
 
-	toolApprovedWithFeedback: (feedback?: string) =>
-		`The user approved this operation and provided the following context:\n<feedback>\n${feedback}\n</feedback>`,
+	toolApprovedWithFeedback: (feedback?: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "approved",
+				message: "The user approved this operation and provided the following context",
+				feedback: feedback,
+			})
+		}
+		return `The user approved this operation and provided the following context:\n<feedback>\n${feedback}\n</feedback>`
+	},
 
-	toolError: (error?: string) => `The tool execution failed with the following error:\n<error>\n${error}\n</error>`,
+	toolError: (error?: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "error",
+				message: "The tool execution failed",
+				error: error,
+			})
+		}
+		return `The tool execution failed with the following error:\n<error>\n${error}\n</error>`
+	},
 
-	rooIgnoreError: (path: string) =>
-		`Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`,
+	rooIgnoreError: (path: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "error",
+				type: "access_denied",
+				message: "Access blocked by .rooignore",
+				path: path,
+				suggestion: "Try to continue without this file, or ask the user to update the .rooignore file",
+			})
+		}
+		return `Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`
+	},
 
 	noToolsUsed: (protocol?: ToolProtocol) => {
 		const instructions = getToolInstructionsReminder(protocol)
@@ -34,8 +77,16 @@ Otherwise, if you have not completed the task and do not need additional informa
 (This is an automated message, so do not respond to it conversationally.)`
 	},
 
-	tooManyMistakes: (feedback?: string) =>
-		`You seem to be having trouble proceeding. The user has provided the following feedback to help guide you:\n<feedback>\n${feedback}\n</feedback>`,
+	tooManyMistakes: (feedback?: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "guidance",
+				message: "You seem to be having trouble proceeding",
+				feedback: feedback,
+			})
+		}
+		return `You seem to be having trouble proceeding. The user has provided the following feedback to help guide you:\n<feedback>\n${feedback}\n</feedback>`
+	},
 
 	missingToolParameterError: (paramName: string, protocol?: ToolProtocol) => {
 		const instructions = getToolInstructionsReminder(protocol)
@@ -82,15 +133,46 @@ Otherwise, if you have not completed the task and do not need additional informa
 		return `${isNewFile ? newFileGuidance : existingFileGuidance}\n${instructions}`
 	},
 
-	invalidMcpToolArgumentError: (serverName: string, toolName: string) =>
-		`Invalid JSON argument used with ${serverName} for ${toolName}. Please retry with a properly formatted JSON argument.`,
+	invalidMcpToolArgumentError: (serverName: string, toolName: string, protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "error",
+				type: "invalid_argument",
+				message: "Invalid JSON argument",
+				server: serverName,
+				tool: toolName,
+				suggestion: "Please retry with a properly formatted JSON argument",
+			})
+		}
+		return `Invalid JSON argument used with ${serverName} for ${toolName}. Please retry with a properly formatted JSON argument.`
+	},
 
-	unknownMcpToolError: (serverName: string, toolName: string, availableTools: string[]) => {
+	unknownMcpToolError: (serverName: string, toolName: string, availableTools: string[], protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "error",
+				type: "unknown_tool",
+				message: "Tool does not exist on server",
+				server: serverName,
+				tool: toolName,
+				available_tools: availableTools.length > 0 ? availableTools : [],
+				suggestion: "Please use one of the available tools or check if the server is properly configured",
+			})
+		}
 		const toolsList = availableTools.length > 0 ? availableTools.join(", ") : "No tools available"
 		return `Tool '${toolName}' does not exist on server '${serverName}'.\n\nAvailable tools on this server: ${toolsList}\n\nPlease use one of the available tools or check if the server is properly configured.`
 	},
 
-	unknownMcpServerError: (serverName: string, availableServers: string[]) => {
+	unknownMcpServerError: (serverName: string, availableServers: string[], protocol?: ToolProtocol) => {
+		if (isNativeProtocol(protocol ?? TOOL_PROTOCOL.XML)) {
+			return JSON.stringify({
+				status: "error",
+				type: "unknown_server",
+				message: "Server is not configured",
+				server: serverName,
+				available_servers: availableServers.length > 0 ? availableServers : [],
+			})
+		}
 		const serversList = availableServers.length > 0 ? availableServers.join(", ") : "No servers available"
 		return `Server '${serverName}' is not configured. Available servers: ${serversList}`
 	},

+ 1 - 1
src/core/prompts/system.ts

@@ -86,7 +86,7 @@ async function generatePrompt(
 	const effectiveProtocol = getEffectiveProtocol(settings?.toolProtocol)
 
 	const [modesSection, mcpServersSection] = await Promise.all([
-		getModesSection(context, isNativeProtocol(effectiveProtocol)),
+		getModesSection(context),
 		shouldIncludeMcp
 			? getMcpServersSection(
 					mcpHub,

+ 2 - 2
src/core/tools/ApplyDiffTool.ts

@@ -32,7 +32,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 	}
 
 	async execute(params: ApplyDiffParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 		let { path: relPath, diff: diffContent } = params
 
 		if (diffContent && !task.api.getModel().id.includes("claude")) {
@@ -58,7 +58,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 
 			if (!accessAllowed) {
 				await task.say("rooignore_error", relPath)
-				pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
+				pushToolResult(formatResponse.rooIgnoreError(relPath, toolProtocol))
 				return
 			}
 

+ 1 - 1
src/core/tools/AskFollowupQuestionTool.ts

@@ -65,7 +65,7 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> {
 
 	async execute(params: AskFollowupQuestionParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { question, follow_up } = params
-		const { handleError, pushToolResult } = callbacks
+		const { handleError, pushToolResult, toolProtocol } = callbacks
 
 		try {
 			if (!question) {

+ 1 - 1
src/core/tools/AttemptCompletionTool.ts

@@ -32,7 +32,7 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> {
 
 	async execute(params: AttemptCompletionParams, task: Task, callbacks: AttemptCompletionCallbacks): Promise<void> {
 		const { result } = params
-		const { handleError, pushToolResult, askFinishSubTaskApproval, toolDescription } = callbacks
+		const { handleError, pushToolResult, askFinishSubTaskApproval, toolDescription, toolProtocol } = callbacks
 
 		const preventCompletionWithOpenTodos = vscode.workspace
 			.getConfiguration(Package.name)

+ 2 - 1
src/core/tools/BaseTool.ts

@@ -7,7 +7,7 @@ import type {
 	AskApproval,
 	NativeToolArgs,
 } from "../../shared/tools"
-import type { ToolName } from "@roo-code/types"
+import type { ToolName, ToolProtocol } from "@roo-code/types"
 
 /**
  * Callbacks passed to tool execution
@@ -17,6 +17,7 @@ export interface ToolCallbacks {
 	handleError: HandleError
 	pushToolResult: PushToolResult
 	removeClosingTag: RemoveClosingTag
+	toolProtocol: ToolProtocol
 }
 
 /**

+ 1 - 1
src/core/tools/CodebaseSearchTool.ts

@@ -32,7 +32,7 @@ export class CodebaseSearchTool extends BaseTool<"codebase_search"> {
 	}
 
 	async execute(params: CodebaseSearchParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 		const { query, path: directoryPrefix } = params
 
 		const workspacePath = task.cwd && task.cwd.trim() !== "" ? task.cwd : getWorkspacePath()

+ 2 - 2
src/core/tools/ExecuteCommandTool.ts

@@ -38,7 +38,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 
 	async execute(params: ExecuteCommandParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { command, cwd: customCwd } = params
-		const { handleError, pushToolResult, askApproval, removeClosingTag } = callbacks
+		const { handleError, pushToolResult, askApproval, removeClosingTag, toolProtocol } = callbacks
 
 		try {
 			if (!command) {
@@ -52,7 +52,7 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 
 			if (ignoredFileAttemptedToAccess) {
 				await task.say("rooignore_error", ignoredFileAttemptedToAccess)
-				pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess)))
+				pushToolResult(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess, toolProtocol))
 				return
 			}
 

+ 1 - 1
src/core/tools/FetchInstructionsTool.ts

@@ -19,7 +19,7 @@ export class FetchInstructionsTool extends BaseTool<"fetch_instructions"> {
 	}
 
 	async execute(params: FetchInstructionsParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { handleError, pushToolResult, askApproval } = callbacks
+		const { handleError, pushToolResult, askApproval, toolProtocol } = callbacks
 		const { task: taskParam } = params
 
 		try {

+ 3 - 3
src/core/tools/GenerateImageTool.ts

@@ -27,7 +27,7 @@ export class GenerateImageTool extends BaseTool<"generate_image"> {
 
 	async execute(params: GenerateImageParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { prompt, path: relPath, image: inputImagePath } = params
-		const { handleError, pushToolResult, askApproval, removeClosingTag } = callbacks
+		const { handleError, pushToolResult, askApproval, removeClosingTag, toolProtocol } = callbacks
 
 		const provider = task.providerRef.deref()
 		const state = await provider?.getState()
@@ -62,7 +62,7 @@ export class GenerateImageTool extends BaseTool<"generate_image"> {
 		const accessAllowed = task.rooIgnoreController?.validateAccess(relPath)
 		if (!accessAllowed) {
 			await task.say("rooignore_error", relPath)
-			pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
+			pushToolResult(formatResponse.rooIgnoreError(relPath, toolProtocol))
 			return
 		}
 
@@ -82,7 +82,7 @@ export class GenerateImageTool extends BaseTool<"generate_image"> {
 			const inputImageAccessAllowed = task.rooIgnoreController?.validateAccess(inputImagePath)
 			if (!inputImageAccessAllowed) {
 				await task.say("rooignore_error", inputImagePath)
-				pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(inputImagePath)))
+				pushToolResult(formatResponse.rooIgnoreError(inputImagePath, toolProtocol))
 				return
 			}
 

+ 2 - 2
src/core/tools/InsertContentTool.ts

@@ -39,7 +39,7 @@ export class InsertContentTool extends BaseTool<"insert_content"> {
 
 	async execute(params: InsertContentParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { path: relPath, line: lineNumber, content } = params
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 
 		try {
 			// Validate required parameters
@@ -68,7 +68,7 @@ export class InsertContentTool extends BaseTool<"insert_content"> {
 
 			if (!accessAllowed) {
 				await task.say("rooignore_error", relPath)
-				pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
+				pushToolResult(formatResponse.rooIgnoreError(relPath, toolProtocol))
 				return
 			}
 

+ 11 - 2
src/core/tools/MultiApplyDiffTool.ts

@@ -69,6 +69,7 @@ export async function applyDiffTool(
 			handleError,
 			pushToolResult,
 			removeClosingTag,
+			toolProtocol,
 		})
 	}
 
@@ -88,6 +89,7 @@ export async function applyDiffTool(
 				handleError,
 				pushToolResult,
 				removeClosingTag,
+				toolProtocol,
 			})
 		}
 	}
@@ -266,7 +268,7 @@ Original error: ${errorMessage}`
 				await cline.say("rooignore_error", relPath)
 				updateOperationResult(relPath, {
 					status: "blocked",
-					error: formatResponse.rooIgnoreError(relPath),
+					error: formatResponse.rooIgnoreError(relPath, undefined),
 				})
 				continue
 			}
@@ -734,9 +736,16 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
 			}
 		}
 
+		// Check protocol for notice formatting
+		const toolProtocol = resolveToolProtocol(cline.apiConfiguration, cline.api.getModel().info)
 		const singleBlockNotice =
 			totalSearchBlocks === 1
-				? "\n<notice>Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.</notice>"
+				? isNativeProtocol(toolProtocol)
+					? "\n" +
+						JSON.stringify({
+							notice: "Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.",
+						})
+					: "\n<notice>Making multiple related changes in a single apply_diff is more efficient. If other changes are needed in this file, please include them as additional SEARCH/REPLACE blocks.</notice>"
 				: ""
 
 		// Push the final result combining all operation results

+ 1 - 1
src/core/tools/NewTaskTool.ts

@@ -30,7 +30,7 @@ export class NewTaskTool extends BaseTool<"new_task"> {
 
 	async execute(params: NewTaskParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { mode, message, todos } = params
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 
 		try {
 			// Validate required parameters.

+ 1 - 1
src/core/tools/ReadFileTool.ts

@@ -106,7 +106,7 @@ export class ReadFileTool extends BaseTool<"read_file"> {
 	}
 
 	async execute(params: { files: FileEntry[] }, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { handleError, pushToolResult } = callbacks
+		const { handleError, pushToolResult, toolProtocol } = callbacks
 		const fileEntries = params.files
 		const modelInfo = task.api.getModel().info
 		const protocol = resolveToolProtocol(task.apiConfiguration, modelInfo)

+ 1 - 1
src/core/tools/RunSlashCommandTool.ts

@@ -22,7 +22,7 @@ export class RunSlashCommandTool extends BaseTool<"run_slash_command"> {
 
 	async execute(params: RunSlashCommandParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { command: commandName, args } = params
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 
 		// Check if run slash command experiment is enabled
 		const provider = task.providerRef.deref()

+ 1 - 1
src/core/tools/SwitchModeTool.ts

@@ -23,7 +23,7 @@ export class SwitchModeTool extends BaseTool<"switch_mode"> {
 
 	async execute(params: SwitchModeParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
 		const { mode_slug, reason } = params
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 
 		try {
 			if (!mode_slug) {

+ 1 - 1
src/core/tools/UpdateTodoListTool.ts

@@ -23,7 +23,7 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> {
 	}
 
 	async execute(params: UpdateTodoListParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { pushToolResult, handleError, askApproval } = callbacks
+		const { pushToolResult, handleError, askApproval, toolProtocol } = callbacks
 
 		try {
 			const todosRaw = params.todos

+ 1 - 1
src/core/tools/UseMcpToolTool.ts

@@ -35,7 +35,7 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> {
 	}
 
 	async execute(params: UseMcpToolParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { askApproval, handleError, pushToolResult } = callbacks
+		const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks
 
 		try {
 			// Validate parameters

+ 2 - 2
src/core/tools/WriteToFileTool.ts

@@ -38,7 +38,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
 	}
 
 	async execute(params: WriteToFileParams, task: Task, callbacks: ToolCallbacks): Promise<void> {
-		const { pushToolResult, handleError, askApproval, removeClosingTag } = callbacks
+		const { pushToolResult, handleError, askApproval, removeClosingTag, toolProtocol } = callbacks
 		const relPath = params.path
 		let newContent = params.content
 		const predictedLineCount = params.line_count
@@ -63,7 +63,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
 
 		if (!accessAllowed) {
 			await task.say("rooignore_error", relPath)
-			pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
+			pushToolResult(formatResponse.rooIgnoreError(relPath, toolProtocol))
 			return
 		}
 

+ 3 - 0
src/core/tools/__tests__/applyDiffTool.experiment.spec.ts

@@ -108,6 +108,7 @@ describe("applyDiffTool experiment routing", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 	})
 
@@ -131,6 +132,7 @@ describe("applyDiffTool experiment routing", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 	})
 
@@ -190,6 +192,7 @@ describe("applyDiffTool experiment routing", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "native",
 		})
 	})
 })

+ 3 - 0
src/core/tools/__tests__/askFollowupQuestionTool.spec.ts

@@ -36,6 +36,7 @@ describe("askFollowupQuestionTool", () => {
 			handleError: vi.fn(),
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: vi.fn((tag, content) => content),
+			toolProtocol: "xml",
 		})
 
 		expect(mockCline.ask).toHaveBeenCalledWith(
@@ -61,6 +62,7 @@ describe("askFollowupQuestionTool", () => {
 			handleError: vi.fn(),
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: vi.fn((tag, content) => content),
+			toolProtocol: "xml",
 		})
 
 		expect(mockCline.ask).toHaveBeenCalledWith(
@@ -88,6 +90,7 @@ describe("askFollowupQuestionTool", () => {
 			handleError: vi.fn(),
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: vi.fn((tag, content) => content),
+			toolProtocol: "xml",
 		})
 
 		expect(mockCline.ask).toHaveBeenCalledWith(

+ 9 - 0
src/core/tools/__tests__/attemptCompletionTool.spec.ts

@@ -83,6 +83,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -108,6 +109,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -137,6 +139,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -176,6 +179,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -218,6 +222,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -261,6 +266,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -303,6 +309,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -346,6 +353,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 
@@ -389,6 +397,7 @@ describe("attemptCompletionTool", () => {
 				removeClosingTag: mockRemoveClosingTag,
 				askFinishSubTaskApproval: mockAskFinishSubTaskApproval,
 				toolDescription: mockToolDescription,
+				toolProtocol: "xml",
 			}
 			await attemptCompletionTool.handle(mockTask as Task, block, callbacks)
 

+ 5 - 0
src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts

@@ -278,6 +278,7 @@ describe("Command Execution Timeout Integration", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should complete successfully without timeout because "npm" is in allowlist
@@ -309,6 +310,7 @@ describe("Command Execution Timeout Integration", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should timeout because "sleep" is not in allowlist
@@ -340,6 +342,7 @@ describe("Command Execution Timeout Integration", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should timeout because allowlist is empty
@@ -374,6 +377,7 @@ describe("Command Execution Timeout Integration", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockPushToolResult).toHaveBeenCalled()
@@ -392,6 +396,7 @@ describe("Command Execution Timeout Integration", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockPushToolResult).toHaveBeenCalled()

+ 8 - 5
src/core/tools/__tests__/executeCommandTool.spec.ts

@@ -147,6 +147,7 @@ describe("executeCommandTool", () => {
 				handleError: mockHandleError as unknown as HandleError,
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 				removeClosingTag: mockRemoveClosingTag as unknown as RemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify
@@ -168,6 +169,7 @@ describe("executeCommandTool", () => {
 				handleError: mockHandleError as unknown as HandleError,
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 				removeClosingTag: mockRemoveClosingTag as unknown as RemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify - confirm the command was approved and result was pushed
@@ -190,6 +192,7 @@ describe("executeCommandTool", () => {
 				handleError: mockHandleError as unknown as HandleError,
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 				removeClosingTag: mockRemoveClosingTag as unknown as RemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify
@@ -211,6 +214,7 @@ describe("executeCommandTool", () => {
 				handleError: mockHandleError as unknown as HandleError,
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 				removeClosingTag: mockRemoveClosingTag as unknown as RemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify
@@ -230,7 +234,6 @@ describe("executeCommandTool", () => {
 
 			const mockRooIgnoreError = "RooIgnore error"
 			;(formatResponse.rooIgnoreError as any).mockReturnValue(mockRooIgnoreError)
-			;(formatResponse.toolError as any).mockReturnValue("Tool error")
 
 			// Execute
 			await executeCommandTool.handle(mockCline as unknown as Task, mockToolUse, {
@@ -238,16 +241,16 @@ describe("executeCommandTool", () => {
 				handleError: mockHandleError as unknown as HandleError,
 				pushToolResult: mockPushToolResult as unknown as PushToolResult,
 				removeClosingTag: mockRemoveClosingTag as unknown as RemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify
 			expect(validateCommandMock).toHaveBeenCalledWith("cat .env")
 			expect(mockCline.say).toHaveBeenCalledWith("rooignore_error", ".env")
-			expect(formatResponse.rooIgnoreError).toHaveBeenCalledWith(".env")
-			expect(formatResponse.toolError).toHaveBeenCalledWith(mockRooIgnoreError)
-			expect(mockPushToolResult).toHaveBeenCalled()
+			expect(formatResponse.rooIgnoreError).toHaveBeenCalledWith(".env", "xml")
+			expect(mockPushToolResult).toHaveBeenCalledWith(mockRooIgnoreError)
 			expect(mockAskApproval).not.toHaveBeenCalled()
-			// executeCommandInTerminal should not be called since param was missing
+			// executeCommandInTerminal should not be called since rooignore blocked it
 		})
 	})
 

+ 9 - 0
src/core/tools/__tests__/generateImageTool.test.ts

@@ -87,6 +87,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should not process anything when partial
@@ -112,6 +113,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should not process anything when partial
@@ -150,6 +152,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should process the complete block
@@ -191,6 +194,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Check that cline.say was called with image data containing cache-busting parameter
@@ -227,6 +231,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockCline.consecutiveMistakeCount).toBe(1)
@@ -250,6 +255,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockCline.consecutiveMistakeCount).toBe(1)
@@ -283,6 +289,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockPushToolResult).toHaveBeenCalledWith(
@@ -313,6 +320,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Input image not found"))
@@ -336,6 +344,7 @@ describe("generateImageTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("Unsupported image format"))

+ 1 - 0
src/core/tools/__tests__/insertContentTool.spec.ts

@@ -161,6 +161,7 @@ describe("insertContentTool", () => {
 				toolResult = result
 			},
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		return toolResult

+ 8 - 0
src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts

@@ -85,6 +85,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions)
@@ -121,6 +122,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions)
@@ -157,6 +159,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should only include definitions starting at or before line 25
@@ -196,6 +199,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should include foo (starts at 10) but not bar (starts at 60)
@@ -236,6 +240,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should include foo and bar but not baz
@@ -275,6 +280,7 @@ describe("listCodeDefinitionNamesTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should keep header but exclude all definitions beyond line 50
@@ -299,6 +305,7 @@ describe("listCodeDefinitionNamesTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -328,6 +335,7 @@ describe("listCodeDefinitionNamesTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockPushToolResult).toHaveBeenCalledWith(mockDefinitions)

+ 15 - 0
src/core/tools/__tests__/newTaskTool.spec.ts

@@ -140,6 +140,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		// Verify askApproval was called
@@ -176,6 +177,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockStartSubtask).toHaveBeenCalledWith(
@@ -202,6 +204,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockStartSubtask).toHaveBeenCalledWith(
@@ -228,6 +231,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockStartSubtask).toHaveBeenCalledWith(
@@ -254,6 +258,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		// Should NOT error when todos is missing
@@ -285,6 +290,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		// Should parse and include todos when provided
@@ -317,6 +323,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "mode")
@@ -341,6 +348,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockSayAndCreateMissingParamError).toHaveBeenCalledWith("new_task", "message")
@@ -365,6 +373,7 @@ describe("newTaskTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		expect(mockStartSubtask).toHaveBeenCalledWith(
@@ -402,6 +411,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should NOT error when todos is missing and setting is disabled
@@ -439,6 +449,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should error when todos is missing and setting is enabled
@@ -476,6 +487,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should NOT error when todos is provided and setting is enabled
@@ -519,6 +531,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Should NOT error when todos is empty string and setting is enabled
@@ -554,6 +567,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Verify that VSCode configuration was accessed with Package.name
@@ -588,6 +602,7 @@ describe("newTaskTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Assert: configuration was read using the dynamic nightly namespace

+ 6 - 0
src/core/tools/__tests__/readFileTool.spec.ts

@@ -336,6 +336,7 @@ describe("read_file tool with maxReadFileLine setting", () => {
 				toolResult = result
 			},
 			removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
+			toolProtocol: "xml",
 		})
 
 		return toolResult
@@ -645,6 +646,7 @@ describe("read_file tool XML output structure", () => {
 				toolResult = result
 			},
 			removeClosingTag: (param: ToolParamName, content?: string) => content ?? "",
+			toolProtocol: "xml",
 		})
 
 		return toolResult
@@ -749,6 +751,7 @@ describe("read_file tool XML output structure", () => {
 						localResult = result
 					},
 					removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
+					toolProtocol: "xml",
 				})
 				// In multi-image scenarios, the result is pushed to pushToolResult, not returned directly.
 				// We need to check the mock's calls to get the result.
@@ -1369,6 +1372,7 @@ describe("read_file tool XML output structure", () => {
 					toolResult = result
 				},
 				removeClosingTag: (param: ToolParamName, content?: string) => content ?? "",
+				toolProtocol: "xml",
 			})
 
 			// Verify
@@ -1456,6 +1460,7 @@ describe("read_file tool with image support", () => {
 				toolResult = result
 			},
 			removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
+			toolProtocol: "xml",
 		})
 
 		console.log("Result type:", Array.isArray(toolResult) ? "array" : typeof toolResult)
@@ -1627,6 +1632,7 @@ describe("read_file tool with image support", () => {
 					toolResult = result
 				},
 				removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
+				toolProtocol: "xml",
 			})
 
 			// Verify error handling

+ 12 - 0
src/core/tools/__tests__/useMcpToolTool.spec.ts

@@ -90,6 +90,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -116,6 +117,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -157,6 +159,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -186,6 +189,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.ask).toHaveBeenCalledWith("use_mcp_server", expect.stringContaining("use_mcp_tool"), true)
@@ -224,6 +228,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(0)
@@ -256,6 +261,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.say).not.toHaveBeenCalledWith("mcp_server_request_started")
@@ -296,6 +302,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockHandleError).toHaveBeenCalledWith("executing MCP tool", error)
@@ -339,6 +346,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -384,6 +392,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(1)
@@ -433,6 +442,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			expect(mockTask.consecutiveMistakeCount).toBe(0)
@@ -473,6 +483,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Assert
@@ -514,6 +525,7 @@ describe("useMcpToolTool", () => {
 				handleError: mockHandleError,
 				pushToolResult: mockPushToolResult,
 				removeClosingTag: mockRemoveClosingTag,
+				toolProtocol: "xml",
 			})
 
 			// Assert

+ 1 - 0
src/core/tools/__tests__/writeToFileTool.spec.ts

@@ -237,6 +237,7 @@ describe("writeToFileTool", () => {
 			handleError: mockHandleError,
 			pushToolResult: mockPushToolResult,
 			removeClosingTag: mockRemoveClosingTag,
+			toolProtocol: "xml",
 		})
 
 		return toolResult

+ 2 - 0
src/core/tools/simpleReadFileTool.ts

@@ -13,6 +13,7 @@ import { countFileLines } from "../../integrations/misc/line-counter"
 import { readLines } from "../../integrations/misc/read-lines"
 import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text"
 import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
+import { ToolProtocol, isNativeProtocol } from "@roo-code/types"
 import {
 	DEFAULT_MAX_IMAGE_FILE_SIZE_MB,
 	DEFAULT_MAX_TOTAL_IMAGE_SIZE_MB,
@@ -38,6 +39,7 @@ export async function simpleReadFileTool(
 	handleError: HandleError,
 	pushToolResult: PushToolResult,
 	_removeClosingTag: RemoveClosingTag,
+	toolProtocol?: ToolProtocol,
 ) {
 	const filePath: string | undefined = block.params.path
 

+ 69 - 41
src/integrations/editor/DiffViewProvider.ts

@@ -12,7 +12,8 @@ import { formatResponse } from "../../core/prompts/responses"
 import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
 import { ClineSayTool } from "../../shared/ExtensionMessage"
 import { Task } from "../../core/task/Task"
-import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
+import { DEFAULT_WRITE_DELAY_MS, isNativeProtocol } from "@roo-code/types"
+import { resolveToolProtocol } from "../../utils/resolveToolProtocol"
 
 import { DecorationController } from "./DecorationController"
 
@@ -300,11 +301,12 @@ export class DiffViewProvider {
 	}
 
 	/**
-	 * Formats a standardized XML response for file write operations
+	 * Formats a standardized response for file write operations
 	 *
+	 * @param task Task instance to get protocol info
 	 * @param cwd Current working directory for path resolution
 	 * @param isNewFile Whether this is a new file or an existing file being modified
-	 * @returns Formatted message and say object for UI feedback
+	 * @returns Formatted message (JSON for native protocol, XML for legacy)
 	 */
 	async pushToolWriteResult(task: Task, cwd: string, isNewFile: boolean): Promise<string> {
 		if (!this.relPath) {
@@ -324,49 +326,75 @@ export class DiffViewProvider {
 			await task.say("user_feedback_diff", JSON.stringify(say))
 		}
 
-		// Build XML response
-		const xmlObj = {
-			file_write_result: {
+		// Check which protocol we're using
+		const toolProtocol = resolveToolProtocol(task.apiConfiguration, task.api.getModel().info)
+		const useNative = isNativeProtocol(toolProtocol)
+
+		// Build notices array
+		const notices = [
+			"You do not need to re-read the file, as you have seen all changes",
+			"Proceed with the task using these changes as the new baseline.",
+			...(this.userEdits
+				? [
+						"If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.",
+					]
+				: []),
+		]
+
+		if (useNative) {
+			// Return JSON for native protocol
+			const result: any = {
 				path: this.relPath,
 				operation: isNewFile ? "created" : "modified",
-				user_edits: this.userEdits ? this.userEdits : undefined,
-				problems: this.newProblemsMessage || undefined,
-				notice: {
-					i: [
-						"You do not need to re-read the file, as you have seen all changes",
-						"Proceed with the task using these changes as the new baseline.",
-						...(this.userEdits
-							? [
-									"If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.",
-								]
-							: []),
-					],
+				notice: notices.join(" "),
+			}
+
+			if (this.userEdits) {
+				result.user_edits = this.userEdits
+			}
+
+			if (this.newProblemsMessage) {
+				result.problems = this.newProblemsMessage
+			}
+
+			return JSON.stringify(result)
+		} else {
+			// Build XML response for legacy protocol
+			const xmlObj = {
+				file_write_result: {
+					path: this.relPath,
+					operation: isNewFile ? "created" : "modified",
+					user_edits: this.userEdits ? this.userEdits : undefined,
+					problems: this.newProblemsMessage || undefined,
+					notice: {
+						i: notices,
+					},
 				},
-			},
-		}
+			}
 
-		const builder = new XMLBuilder({
-			format: true,
-			indentBy: "",
-			suppressEmptyNode: true,
-			processEntities: false,
-			tagValueProcessor: (name, value) => {
-				if (typeof value === "string") {
-					// Only escape <, >, and & characters
-					return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
-				}
-				return value
-			},
-			attributeValueProcessor: (name, value) => {
-				if (typeof value === "string") {
-					// Only escape <, >, and & characters
-					return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
-				}
-				return value
-			},
-		})
+			const builder = new XMLBuilder({
+				format: true,
+				indentBy: "",
+				suppressEmptyNode: true,
+				processEntities: false,
+				tagValueProcessor: (name, value) => {
+					if (typeof value === "string") {
+						// Only escape <, >, and & characters
+						return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
+					}
+					return value
+				},
+				attributeValueProcessor: (name, value) => {
+					if (typeof value === "string") {
+						// Only escape <, >, and & characters
+						return value.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")
+					}
+					return value
+				},
+			})
 
-		return builder.build(xmlObj)
+			return builder.build(xmlObj)
+		}
 	}
 
 	async revertChanges(): Promise<void> {