diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index d4389c22e8..c5d10590ca 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/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 } diff --git a/src/core/prompts/responses.ts b/src/core/prompts/responses.ts index ace37b7c68..6535f25f5c 100644 --- a/src/core/prompts/responses.ts +++ b/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\n${feedback}\n`, + 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\n${feedback}\n` + }, - toolApprovedWithFeedback: (feedback?: string) => - `The user approved this operation and provided the following context:\n\n${feedback}\n`, + 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\n${feedback}\n` + }, - toolError: (error?: string) => `The tool execution failed with the following error:\n\n${error}\n`, + 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\n${error}\n` + }, - 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\n${feedback}\n`, + 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\n${feedback}\n` + }, 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}` }, diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index e72f26c51a..9230619ebd 100644 --- a/src/core/prompts/system.ts +++ b/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, diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index ef6623ed93..c5ad24bca3 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/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 { - 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 } diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 2899dc6ee3..27189476dc 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/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 { const { question, follow_up } = params - const { handleError, pushToolResult } = callbacks + const { handleError, pushToolResult, toolProtocol } = callbacks try { if (!question) { diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index b2cac7c4f2..a0b358ea5f 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/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 { const { result } = params - const { handleError, pushToolResult, askFinishSubTaskApproval, toolDescription } = callbacks + const { handleError, pushToolResult, askFinishSubTaskApproval, toolDescription, toolProtocol } = callbacks const preventCompletionWithOpenTodos = vscode.workspace .getConfiguration(Package.name) diff --git a/src/core/tools/BaseTool.ts b/src/core/tools/BaseTool.ts index 818fd1bf9a..e960b2541f 100644 --- a/src/core/tools/BaseTool.ts +++ b/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 } /** diff --git a/src/core/tools/CodebaseSearchTool.ts b/src/core/tools/CodebaseSearchTool.ts index 6daefe77c1..0637ac5241 100644 --- a/src/core/tools/CodebaseSearchTool.ts +++ b/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 { - 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() diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index ddf09cb8f0..aa6bb097d0 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/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 { 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 } diff --git a/src/core/tools/FetchInstructionsTool.ts b/src/core/tools/FetchInstructionsTool.ts index d1610d9ee4..0632fc12b2 100644 --- a/src/core/tools/FetchInstructionsTool.ts +++ b/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 { - const { handleError, pushToolResult, askApproval } = callbacks + const { handleError, pushToolResult, askApproval, toolProtocol } = callbacks const { task: taskParam } = params try { diff --git a/src/core/tools/GenerateImageTool.ts b/src/core/tools/GenerateImageTool.ts index 4b41f840d4..60914a86a7 100644 --- a/src/core/tools/GenerateImageTool.ts +++ b/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 { 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 } diff --git a/src/core/tools/InsertContentTool.ts b/src/core/tools/InsertContentTool.ts index 55b31f05ba..68acc229b9 100644 --- a/src/core/tools/InsertContentTool.ts +++ b/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 { 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 } diff --git a/src/core/tools/MultiApplyDiffTool.ts b/src/core/tools/MultiApplyDiffTool.ts index f8e9ba64e0..7e076d27a9 100644 --- a/src/core/tools/MultiApplyDiffTool.ts +++ b/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 - ? "\nMaking 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." + ? 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.", + }) + : "\nMaking 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." : "" // Push the final result combining all operation results diff --git a/src/core/tools/NewTaskTool.ts b/src/core/tools/NewTaskTool.ts index 2535cef1d2..ce68b20f3e 100644 --- a/src/core/tools/NewTaskTool.ts +++ b/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 { const { mode, message, todos } = params - const { askApproval, handleError, pushToolResult } = callbacks + const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks try { // Validate required parameters. diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 98bccfe273..d6989c103e 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/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 { - 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) diff --git a/src/core/tools/RunSlashCommandTool.ts b/src/core/tools/RunSlashCommandTool.ts index 2196e25725..c82b6cc0a8 100644 --- a/src/core/tools/RunSlashCommandTool.ts +++ b/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 { 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() diff --git a/src/core/tools/SwitchModeTool.ts b/src/core/tools/SwitchModeTool.ts index 17ef11d988..df418cfdfc 100644 --- a/src/core/tools/SwitchModeTool.ts +++ b/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 { const { mode_slug, reason } = params - const { askApproval, handleError, pushToolResult } = callbacks + const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks try { if (!mode_slug) { diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index 94d1b25e13..bf2c2b5301 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/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 { - const { pushToolResult, handleError, askApproval } = callbacks + const { pushToolResult, handleError, askApproval, toolProtocol } = callbacks try { const todosRaw = params.todos diff --git a/src/core/tools/UseMcpToolTool.ts b/src/core/tools/UseMcpToolTool.ts index f24c3b9c02..b276293f6f 100644 --- a/src/core/tools/UseMcpToolTool.ts +++ b/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 { - const { askApproval, handleError, pushToolResult } = callbacks + const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks try { // Validate parameters diff --git a/src/core/tools/WriteToFileTool.ts b/src/core/tools/WriteToFileTool.ts index 4fb6a80a35..4c355beb07 100644 --- a/src/core/tools/WriteToFileTool.ts +++ b/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 { - 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 } diff --git a/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts b/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts index d7ee79649b..4e0044c5ee 100644 --- a/src/core/tools/__tests__/applyDiffTool.experiment.spec.ts +++ b/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", }) }) }) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 68aa49aa50..2aa46b5100 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/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( diff --git a/src/core/tools/__tests__/attemptCompletionTool.spec.ts b/src/core/tools/__tests__/attemptCompletionTool.spec.ts index 72b4560c4a..9ab4d57ebb 100644 --- a/src/core/tools/__tests__/attemptCompletionTool.spec.ts +++ b/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) diff --git a/src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts b/src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts index c4732867e3..f93a29caaf 100644 --- a/src/core/tools/__tests__/executeCommandTimeout.integration.spec.ts +++ b/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() diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index a56adac40c..0406a83d2a 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/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 }) }) diff --git a/src/core/tools/__tests__/generateImageTool.test.ts b/src/core/tools/__tests__/generateImageTool.test.ts index 68b0e36f4b..483533e34d 100644 --- a/src/core/tools/__tests__/generateImageTool.test.ts +++ b/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")) diff --git a/src/core/tools/__tests__/insertContentTool.spec.ts b/src/core/tools/__tests__/insertContentTool.spec.ts index bf7bff670c..aa1a8f2f3a 100644 --- a/src/core/tools/__tests__/insertContentTool.spec.ts +++ b/src/core/tools/__tests__/insertContentTool.spec.ts @@ -161,6 +161,7 @@ describe("insertContentTool", () => { toolResult = result }, removeClosingTag: mockRemoveClosingTag, + toolProtocol: "xml", }) return toolResult diff --git a/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts b/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts index 2f6c1c264a..1e9c61e11c 100644 --- a/src/core/tools/__tests__/listCodeDefinitionNamesTool.spec.ts +++ b/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) diff --git a/src/core/tools/__tests__/newTaskTool.spec.ts b/src/core/tools/__tests__/newTaskTool.spec.ts index d86e5453d8..43ba3ea9d3 100644 --- a/src/core/tools/__tests__/newTaskTool.spec.ts +++ b/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 diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 2401df2bba..11f533e54f 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/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 diff --git a/src/core/tools/__tests__/useMcpToolTool.spec.ts b/src/core/tools/__tests__/useMcpToolTool.spec.ts index 3a4743e92f..130047ae15 100644 --- a/src/core/tools/__tests__/useMcpToolTool.spec.ts +++ b/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 diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts index e96fff6356..714269795e 100644 --- a/src/core/tools/__tests__/writeToFileTool.spec.ts +++ b/src/core/tools/__tests__/writeToFileTool.spec.ts @@ -237,6 +237,7 @@ describe("writeToFileTool", () => { handleError: mockHandleError, pushToolResult: mockPushToolResult, removeClosingTag: mockRemoveClosingTag, + toolProtocol: "xml", }) return toolResult diff --git a/src/core/tools/simpleReadFileTool.ts b/src/core/tools/simpleReadFileTool.ts index ee6656c5c8..1b41e9e9d6 100644 --- a/src/core/tools/simpleReadFileTool.ts +++ b/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 diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index d42eba082c..983307d919 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/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 { 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, "&").replace(//g, ">") - } - return value - }, - attributeValueProcessor: (name, value) => { - if (typeof value === "string") { - // Only escape <, >, and & characters - return value.replace(/&/g, "&").replace(//g, ">") - } - 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, "&").replace(//g, ">") + } + return value + }, + attributeValueProcessor: (name, value) => { + if (typeof value === "string") { + // Only escape <, >, and & characters + return value.replace(/&/g, "&").replace(//g, ">") + } + return value + }, + }) - return builder.build(xmlObj) + return builder.build(xmlObj) + } } async revertChanges(): Promise {