From 5f7c4c2ee307ecbbe1109e4933b840f0bf07a59d Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 24 Nov 2025 23:10:07 -0500 Subject: [PATCH 1/3] fix: preserve dynamic MCP tool names in native mode API history Fixes issue where dynamic MCP tools (mcp_serverName_toolName) were being converted to 'use_mcp_tool' in API conversation history, causing the model to mistakenly think 'use_mcp_tool' is a valid tool in native mode. Changes: - Added McpToolUse type to preserve original tool names in native mode - Updated NativeToolCallParser to return McpToolUse for dynamic MCP tools - Modified presentAssistantMessage to handle mcp_tool_use type - Updated Task.ts to store McpToolUse with original names in API history - Added McpToolUse to AssistantMessageContent type union The XML mode behavior remains unchanged - it continues using use_mcp_tool. --- .../assistant-message/NativeToolCallParser.ts | 56 +++--- .../parseAssistantMessage.ts | 4 +- .../presentAssistantMessage.ts | 174 +++++++++++++++++- src/core/task/Task.ts | 57 ++++-- src/shared/tools.ts | 21 +++ 5 files changed, 257 insertions(+), 55 deletions(-) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 8febac87105..c0442b2d4e0 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -1,5 +1,11 @@ import { type ToolName, toolNames, type FileEntry } from "@roo-code/types" -import { type ToolUse, type ToolParamName, toolParamNames, type NativeToolArgs } from "../../shared/tools" +import { + type ToolUse, + type McpToolUse, + type ToolParamName, + toolParamNames, + type NativeToolArgs, +} from "../../shared/tools" import { parseJSON } from "partial-json" import type { ApiStreamToolCallStartChunk, @@ -490,10 +496,10 @@ export class NativeToolCallParser { id: string name: TName arguments: string - }): ToolUse | null { + }): ToolUse | McpToolUse | null { // Check if this is a dynamic MCP tool (mcp_serverName_toolName) if (typeof toolCall.name === "string" && toolCall.name.startsWith("mcp_")) { - return this.parseDynamicMcpTool(toolCall) as ToolUse | null + return this.parseDynamicMcpTool(toolCall) } // Validate tool name @@ -719,14 +725,14 @@ export class NativeToolCallParser { /** * Parse dynamic MCP tools (named mcp_serverName_toolName). - * These are generated dynamically by getMcpServerTools() and need to be - * converted back to use_mcp_tool format. + * These are generated dynamically by getMcpServerTools() and are returned + * as McpToolUse objects that preserve the original tool name. + * + * In native mode, MCP tools are NOT converted to use_mcp_tool - they keep + * their original name so it appears correctly in API conversation history. + * The use_mcp_tool wrapper is only used in XML mode. */ - private static parseDynamicMcpTool(toolCall: { - id: string - name: string - arguments: string - }): ToolUse<"use_mcp_tool"> | null { + public static parseDynamicMcpTool(toolCall: { id: string; name: string; arguments: string }): McpToolUse | null { try { const args = JSON.parse(toolCall.arguments) @@ -734,36 +740,22 @@ export class NativeToolCallParser { // The dynamic tool schema includes these as const properties const serverName = args.server_name const toolName = args.tool_name - const toolInputProps = args.toolInputProps + const toolInputProps = args.toolInputProps || {} if (!serverName || !toolName) { console.error(`Missing server_name or tool_name in dynamic MCP tool`) return null } - // Build params for backward compatibility with XML protocol - const params: Partial> = { - server_name: serverName, - tool_name: toolName, - } - - if (toolInputProps) { - params.arguments = JSON.stringify(toolInputProps) - } - - // Build nativeArgs with properly typed structure - const nativeArgs: NativeToolArgs["use_mcp_tool"] = { - server_name: serverName, - tool_name: toolName, + const result: McpToolUse = { + type: "mcp_tool_use" as const, + id: toolCall.id, + // Keep the original tool name (e.g., "mcp_serverName_toolName") for API history + name: toolCall.name, + serverName, + toolName, arguments: toolInputProps, - } - - const result: ToolUse<"use_mcp_tool"> = { - type: "tool_use" as const, - name: "use_mcp_tool", - params, partial: false, - nativeArgs, } return result diff --git a/src/core/assistant-message/parseAssistantMessage.ts b/src/core/assistant-message/parseAssistantMessage.ts index ebb8674c8fa..e07b8cc3db8 100644 --- a/src/core/assistant-message/parseAssistantMessage.ts +++ b/src/core/assistant-message/parseAssistantMessage.ts @@ -1,8 +1,8 @@ import { type ToolName, toolNames } from "@roo-code/types" -import { TextContent, ToolUse, ToolParamName, toolParamNames } from "../../shared/tools" +import { TextContent, ToolUse, McpToolUse, ToolParamName, toolParamNames } from "../../shared/tools" -export type AssistantMessageContent = TextContent | ToolUse +export type AssistantMessageContent = TextContent | ToolUse | McpToolUse export function parseAssistantMessage(assistantMessage: string): AssistantMessageContent[] { let contentBlocks: AssistantMessageContent[] = [] diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 3282eb90d18..048fc77aa0a 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -6,7 +6,7 @@ import type { ToolName, ClineAsk, ToolProgressStatus } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { defaultModeSlug, getModeBySlug } from "../../shared/modes" -import type { ToolParamName, ToolResponse, ToolUse } from "../../shared/tools" +import type { ToolParamName, ToolResponse, ToolUse, McpToolUse } from "../../shared/tools" import { Package } from "../../shared/package" import { fetchInstructionsTool } from "../tools/FetchInstructionsTool" @@ -100,6 +100,171 @@ export async function presentAssistantMessage(cline: Task) { } switch (block.type) { + case "mcp_tool_use": { + // Handle native MCP tool calls (from mcp_serverName_toolName dynamic tools) + // These are converted to the same execution path as use_mcp_tool but preserve + // their original name in API history + const mcpBlock = block as McpToolUse + + if (cline.didRejectTool) { + // For native protocol, we must send a tool_result for every tool_use to avoid API errors + const toolCallId = mcpBlock.id + const errorMessage = !mcpBlock.partial + ? `Skipping MCP tool ${mcpBlock.name} due to user rejecting a previous tool.` + : `MCP tool ${mcpBlock.name} was interrupted and not executed due to user rejecting a previous tool.` + + if (toolCallId) { + cline.userMessageContent.push({ + type: "tool_result", + tool_use_id: toolCallId, + content: errorMessage, + is_error: true, + } as Anthropic.ToolResultBlockParam) + } + break + } + + if (cline.didAlreadyUseTool) { + const toolCallId = mcpBlock.id + const errorMessage = `MCP tool [${mcpBlock.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message.` + + if (toolCallId) { + cline.userMessageContent.push({ + type: "tool_result", + tool_use_id: toolCallId, + content: errorMessage, + is_error: true, + } as Anthropic.ToolResultBlockParam) + } + break + } + + // Track if we've already pushed a tool result + let hasToolResult = false + const toolCallId = mcpBlock.id + const toolProtocol = TOOL_PROTOCOL.NATIVE // MCP tools in native mode always use native protocol + + const pushToolResult = (content: ToolResponse) => { + if (hasToolResult) { + console.warn( + `[presentAssistantMessage] Skipping duplicate tool_result for mcp_tool_use: ${toolCallId}`, + ) + return + } + + let resultContent: string + let imageBlocks: Anthropic.ImageBlockParam[] = [] + + if (typeof content === "string") { + resultContent = content || "(tool did not return anything)" + } else { + const textBlocks = content.filter((item) => item.type === "text") + imageBlocks = content.filter((item) => item.type === "image") as Anthropic.ImageBlockParam[] + resultContent = + textBlocks.map((item) => (item as Anthropic.TextBlockParam).text).join("\n") || + "(tool did not return anything)" + } + + if (toolCallId) { + cline.userMessageContent.push({ + type: "tool_result", + tool_use_id: toolCallId, + content: resultContent, + } as Anthropic.ToolResultBlockParam) + + if (imageBlocks.length > 0) { + cline.userMessageContent.push(...imageBlocks) + } + } + + hasToolResult = true + cline.didAlreadyUseTool = true + } + + const toolDescription = () => `[mcp_tool: ${mcpBlock.serverName}/${mcpBlock.toolName}]` + + const askApproval = async ( + type: ClineAsk, + partialMessage?: string, + progressStatus?: ToolProgressStatus, + isProtected?: boolean, + ) => { + const { response, text, images } = await cline.ask( + type, + partialMessage, + false, + progressStatus, + isProtected || false, + ) + + if (response !== "yesButtonClicked") { + if (text) { + await cline.say("user_feedback", text, images) + pushToolResult( + formatResponse.toolResult( + formatResponse.toolDeniedWithFeedback(text, toolProtocol), + images, + ), + ) + } else { + pushToolResult(formatResponse.toolDenied(toolProtocol)) + } + cline.didRejectTool = true + return false + } + + if (text) { + await cline.say("user_feedback", text, images) + pushToolResult( + formatResponse.toolResult(formatResponse.toolApprovedWithFeedback(text, toolProtocol), images), + ) + } + + return true + } + + const handleError = async (action: string, error: Error) => { + const errorString = `Error ${action}: ${JSON.stringify(serializeError(error))}` + await cline.say( + "error", + `Error ${action}:\n${error.message ?? JSON.stringify(serializeError(error), null, 2)}`, + ) + pushToolResult(formatResponse.toolError(errorString, toolProtocol)) + } + + if (!mcpBlock.partial) { + cline.recordToolUsage("use_mcp_tool") // Record as use_mcp_tool for analytics + TelemetryService.instance.captureToolUsage(cline.taskId, "use_mcp_tool", toolProtocol) + } + + // Execute the MCP tool using the same handler as use_mcp_tool + // Create a synthetic ToolUse block that the useMcpToolTool can handle + const syntheticToolUse: ToolUse<"use_mcp_tool"> = { + type: "tool_use", + id: mcpBlock.id, + name: "use_mcp_tool", + params: { + server_name: mcpBlock.serverName, + tool_name: mcpBlock.toolName, + arguments: JSON.stringify(mcpBlock.arguments), + }, + partial: mcpBlock.partial, + nativeArgs: { + server_name: mcpBlock.serverName, + tool_name: mcpBlock.toolName, + arguments: mcpBlock.arguments, + }, + } + + await useMcpToolTool.handle(cline, syntheticToolUse, { + askApproval, + handleError, + pushToolResult, + removeClosingTag: (tag, text) => text || "", + toolProtocol, + }) + break + } case "text": { if (cline.didRejectTool || cline.didAlreadyUseTool) { break @@ -721,14 +886,13 @@ export async function presentAssistantMessage(cline: Task) { }) break case "access_mcp_resource": - await accessMcpResourceTool( - cline, - block, + await accessMcpResourceTool.handle(cline, block as ToolUse<"access_mcp_resource">, { askApproval, handleError, pushToolResult, removeClosingTag, - ) + toolProtocol, + }) break case "ask_followup_question": await askFollowupQuestionTool.handle(cline, block as ToolUse<"ask_followup_question">, { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index f7b0adc28ab..9b60c312abf 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2845,7 +2845,9 @@ export class Task extends EventEmitter implements TaskLike { // Check if we have any content to process (text or tool uses) const hasTextContent = assistantMessage.length > 0 - const hasToolUses = this.assistantMessageContent.some((block) => block.type === "tool_use") + const hasToolUses = this.assistantMessageContent.some( + (block) => block.type === "tool_use" || block.type === "mcp_tool_use", + ) if (hasTextContent || hasToolUses) { // Display grounding sources to the user if they exist @@ -2870,20 +2872,41 @@ export class Task extends EventEmitter implements TaskLike { } // Add tool_use blocks with their IDs for native protocol - const toolUseBlocks = this.assistantMessageContent.filter((block) => block.type === "tool_use") - for (const toolUse of toolUseBlocks) { - // Get the tool call ID that was stored during parsing - const toolCallId = (toolUse as any).id - if (toolCallId) { - // nativeArgs is already in the correct API format for all tools - const input = toolUse.nativeArgs || toolUse.params - - assistantContent.push({ - type: "tool_use" as const, - id: toolCallId, - name: toolUse.name, - input, - }) + // This handles both regular ToolUse and McpToolUse types + const toolUseBlocks = this.assistantMessageContent.filter( + (block) => block.type === "tool_use" || block.type === "mcp_tool_use", + ) + for (const block of toolUseBlocks) { + if (block.type === "mcp_tool_use") { + // McpToolUse already has the original tool name (e.g., "mcp_serverName_toolName") + const mcpBlock = block as import("../../shared/tools").McpToolUse + if (mcpBlock.id) { + assistantContent.push({ + type: "tool_use" as const, + id: mcpBlock.id, + name: mcpBlock.name, // Original dynamic name + input: { + server_name: mcpBlock.serverName, + tool_name: mcpBlock.toolName, + toolInputProps: mcpBlock.arguments, + }, + }) + } + } else { + // Regular ToolUse + const toolUse = block as import("../../shared/tools").ToolUse + const toolCallId = toolUse.id + if (toolCallId) { + // nativeArgs is already in the correct API format for all tools + const input = toolUse.nativeArgs || toolUse.params + + assistantContent.push({ + type: "tool_use" as const, + id: toolCallId, + name: toolUse.name, + input, + }) + } } } @@ -2917,7 +2940,9 @@ export class Task extends EventEmitter implements TaskLike { // If the model did not tool use, then we need to tell it to // either use a tool or attempt_completion. - const didToolUse = this.assistantMessageContent.some((block) => block.type === "tool_use") + const didToolUse = this.assistantMessageContent.some( + (block) => block.type === "tool_use" || block.type === "mcp_tool_use", + ) if (!didToolUse) { const modelInfo = this.api.getModel().info diff --git a/src/shared/tools.ts b/src/shared/tools.ts index f280dcd4119..6aea5fd3114 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -82,6 +82,7 @@ export type ToolProtocol = "xml" | "native" * Tools not listed here will fall back to `any` for backward compatibility. */ export type NativeToolArgs = { + access_mcp_resource: { server_name: string; uri: string } read_file: { files: FileEntry[] } attempt_completion: { result: string } execute_command: { command: string; cwd?: string } @@ -121,6 +122,26 @@ export interface ToolUse { nativeArgs?: TName extends keyof NativeToolArgs ? NativeToolArgs[TName] : never } +/** + * Represents a native MCP tool call from the model. + * In native mode, MCP tools are called directly with their prefixed name (e.g., "mcp_serverName_toolName") + * rather than through the use_mcp_tool wrapper. This type preserves the original tool name + * so it appears correctly in API conversation history. + */ +export interface McpToolUse { + type: "mcp_tool_use" + id?: string // Tool call ID from the API + /** The original tool name from the API (e.g., "mcp_serverName_toolName") */ + name: string + /** Extracted server name from the tool name */ + serverName: string + /** Extracted tool name from the tool name */ + toolName: string + /** Arguments passed to the MCP tool */ + arguments: Record + partial: boolean +} + export interface ExecuteCommandToolUse extends ToolUse<"execute_command"> { name: "execute_command" // Pick, "command"> makes "command" required, but Partial<> makes it optional From 7945d3b59e6ed1c2af1bf53815e73662de92bd4a Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 24 Nov 2025 23:11:15 -0500 Subject: [PATCH 2/3] feat: add access_mcp_resource as native tool and update MCP server instructions - Refactored accessMcpResourceTool to extend BaseTool class - Added native tool definition for access_mcp_resource - Updated MCP servers section to provide different instructions based on protocol - XML mode: mentions use_mcp_tool wrapper - Native mode: explains mcp_{server}_{tool} naming pattern - Registered access_mcp_resource in native tools list --- .../assistant-message/NativeToolCallParser.ts | 4 +- src/core/prompts/sections/mcp-servers.ts | 7 +- .../tools/native-tools/access_mcp_resource.ts | 41 +++++++++ src/core/prompts/tools/native-tools/index.ts | 2 + src/core/tools/accessMcpResourceTool.ts | 86 +++++++++++-------- 5 files changed, 100 insertions(+), 40 deletions(-) create mode 100644 src/core/prompts/tools/native-tools/access_mcp_resource.ts diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index c0442b2d4e0..08f842e8894 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -256,9 +256,9 @@ export class NativeToolCallParser { /** * Finalize a streaming tool call. - * Parses the complete JSON and returns the final ToolUse. + * Parses the complete JSON and returns the final ToolUse or McpToolUse. */ - public static finalizeStreamingToolCall(id: string): ToolUse | null { + public static finalizeStreamingToolCall(id: string): ToolUse | McpToolUse | null { const toolCall = this.streamingToolCalls.get(id) if (!toolCall) { console.warn(`[NativeToolCallParser] Attempting to finalize unknown tool call: ${id}`) diff --git a/src/core/prompts/sections/mcp-servers.ts b/src/core/prompts/sections/mcp-servers.ts index 678099922f0..3eb1569c5ad 100644 --- a/src/core/prompts/sections/mcp-servers.ts +++ b/src/core/prompts/sections/mcp-servers.ts @@ -53,6 +53,11 @@ export async function getMcpServersSection( .join("\n\n")}` : "(No MCP servers currently connected)" + // Different instructions based on protocol + const toolAccessInstructions = includeToolDescriptions + ? `When a server is connected, you can use the server's tools via the \`use_mcp_tool\` tool, and access the server's resources via the \`access_mcp_resource\` tool.` + : `When a server is connected, each server's tools are available as native tools with the naming pattern \`mcp_{server_name}_{tool_name}\`. For example, a tool named 'get_forecast' from a server named 'weather' would be available as \`mcp_weather_get_forecast\`. You can also access server resources using the \`access_mcp_resource\` tool.` + const baseSection = `MCP SERVERS The Model Context Protocol (MCP) enables communication between the system and MCP servers that provide additional tools and resources to extend your capabilities. MCP servers can be one of two types: @@ -62,7 +67,7 @@ The Model Context Protocol (MCP) enables communication between the system and MC # Connected MCP Servers -When a server is connected, you can use the server's tools via the \`use_mcp_tool\` tool, and access the server's resources via the \`access_mcp_resource\` tool. +${toolAccessInstructions} ${connectedServers}` diff --git a/src/core/prompts/tools/native-tools/access_mcp_resource.ts b/src/core/prompts/tools/native-tools/access_mcp_resource.ts new file mode 100644 index 00000000000..2876fcf5604 --- /dev/null +++ b/src/core/prompts/tools/native-tools/access_mcp_resource.ts @@ -0,0 +1,41 @@ +import type OpenAI from "openai" + +const ACCESS_MCP_RESOURCE_DESCRIPTION = `Request to access a resource provided by a connected MCP server. Resources represent data sources that can be used as context, such as files, API responses, or system information. + +Parameters: +- server_name: (required) The name of the MCP server providing the resource +- uri: (required) The URI identifying the specific resource to access + +Example: Accessing a weather resource +{ "server_name": "weather-server", "uri": "weather://san-francisco/current" } + +Example: Accessing a file resource from an MCP server +{ "server_name": "filesystem-server", "uri": "file:///path/to/data.json" }` + +const SERVER_NAME_PARAMETER_DESCRIPTION = `The name of the MCP server providing the resource` + +const URI_PARAMETER_DESCRIPTION = `The URI identifying the specific resource to access` + +export default { + type: "function", + function: { + name: "access_mcp_resource", + description: ACCESS_MCP_RESOURCE_DESCRIPTION, + strict: true, + parameters: { + type: "object", + properties: { + server_name: { + type: "string", + description: SERVER_NAME_PARAMETER_DESCRIPTION, + }, + uri: { + type: "string", + description: URI_PARAMETER_DESCRIPTION, + }, + }, + required: ["server_name", "uri"], + additionalProperties: false, + }, + }, +} satisfies OpenAI.Chat.ChatCompletionTool diff --git a/src/core/prompts/tools/native-tools/index.ts b/src/core/prompts/tools/native-tools/index.ts index 941fd3fddb0..bb0d50da88d 100644 --- a/src/core/prompts/tools/native-tools/index.ts +++ b/src/core/prompts/tools/native-tools/index.ts @@ -1,4 +1,5 @@ import type OpenAI from "openai" +import accessMcpResource from "./access_mcp_resource" import askFollowupQuestion from "./ask_followup_question" import attemptCompletion from "./attempt_completion" import browserAction from "./browser_action" @@ -29,6 +30,7 @@ export { convertOpenAIToolToAnthropic, convertOpenAIToolsToAnthropic } from "./c */ export function getNativeTools(partialReadsEnabled: boolean = true): OpenAI.Chat.ChatCompletionTool[] { return [ + accessMcpResource, apply_diff_single_file, askFollowupQuestion, attemptCompletion, diff --git a/src/core/tools/accessMcpResourceTool.ts b/src/core/tools/accessMcpResourceTool.ts index c8a40f9236d..ff6705ef6cf 100644 --- a/src/core/tools/accessMcpResourceTool.ts +++ b/src/core/tools/accessMcpResourceTool.ts @@ -1,45 +1,44 @@ import { ClineAskUseMcpServer } from "../../shared/ExtensionMessage" -import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools" +import type { ToolUse } from "../../shared/tools" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" +import { BaseTool, ToolCallbacks } from "./BaseTool" -export async function accessMcpResourceTool( - cline: Task, - block: ToolUse, - askApproval: AskApproval, - handleError: HandleError, - pushToolResult: PushToolResult, - removeClosingTag: RemoveClosingTag, -) { - const server_name: string | undefined = block.params.server_name - const uri: string | undefined = block.params.uri - - try { - if (block.partial) { - const partialMessage = JSON.stringify({ - type: "access_mcp_resource", - serverName: removeClosingTag("server_name", server_name), - uri: removeClosingTag("uri", uri), - } satisfies ClineAskUseMcpServer) +interface AccessMcpResourceParams { + server_name: string + uri: string +} + +export class AccessMcpResourceTool extends BaseTool<"access_mcp_resource"> { + readonly name = "access_mcp_resource" as const + + parseLegacy(params: Partial>): AccessMcpResourceParams { + return { + server_name: params.server_name || "", + uri: params.uri || "", + } + } - await cline.ask("use_mcp_server", partialMessage, block.partial).catch(() => {}) - return - } else { + async execute(params: AccessMcpResourceParams, task: Task, callbacks: ToolCallbacks): Promise { + const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks + const { server_name, uri } = params + + try { if (!server_name) { - cline.consecutiveMistakeCount++ - cline.recordToolError("access_mcp_resource") - pushToolResult(await cline.sayAndCreateMissingParamError("access_mcp_resource", "server_name")) + task.consecutiveMistakeCount++ + task.recordToolError("access_mcp_resource") + pushToolResult(await task.sayAndCreateMissingParamError("access_mcp_resource", "server_name")) return } if (!uri) { - cline.consecutiveMistakeCount++ - cline.recordToolError("access_mcp_resource") - pushToolResult(await cline.sayAndCreateMissingParamError("access_mcp_resource", "uri")) + task.consecutiveMistakeCount++ + task.recordToolError("access_mcp_resource") + pushToolResult(await task.sayAndCreateMissingParamError("access_mcp_resource", "uri")) return } - cline.consecutiveMistakeCount = 0 + task.consecutiveMistakeCount = 0 const completeMessage = JSON.stringify({ type: "access_mcp_resource", @@ -50,12 +49,13 @@ export async function accessMcpResourceTool( const didApprove = await askApproval("use_mcp_server", completeMessage) if (!didApprove) { + pushToolResult(formatResponse.toolDenied(toolProtocol)) return } // Now execute the tool - await cline.say("mcp_server_request_started") - const resourceResult = await cline.providerRef.deref()?.getMcpHub()?.readResource(server_name, uri) + await task.say("mcp_server_request_started") + const resourceResult = await task.providerRef.deref()?.getMcpHub()?.readResource(server_name, uri) const resourceResultPretty = resourceResult?.contents @@ -81,13 +81,25 @@ export async function accessMcpResourceTool( } }) - await cline.say("mcp_server_response", resourceResultPretty, images) + await task.say("mcp_server_response", resourceResultPretty, images) pushToolResult(formatResponse.toolResult(resourceResultPretty, images)) - - return + } catch (error) { + await handleError("accessing MCP resource", error instanceof Error ? error : new Error(String(error))) } - } catch (error) { - await handleError("accessing MCP resource", error) - return + } + + override async handlePartial(task: Task, block: ToolUse<"access_mcp_resource">): Promise { + const server_name = this.removeClosingTag("server_name", block.params.server_name, true) + const uri = this.removeClosingTag("uri", block.params.uri, true) + + const partialMessage = JSON.stringify({ + type: "access_mcp_resource", + serverName: server_name, + uri: uri, + } satisfies ClineAskUseMcpServer) + + await task.ask("use_mcp_server", partialMessage, block.partial).catch(() => {}) } } + +export const accessMcpResourceTool = new AccessMcpResourceTool() From 626791e39eb919c25c02acccc49bd843824da563 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 25 Nov 2025 11:17:00 -0500 Subject: [PATCH 3/3] fix: properly finalize MCP tool calls in native protocol mode - Simplify MCP tool schema to pass arguments directly (no toolInputProps wrapper) - Extract server_name/tool_name from function name (mcp_serverName_toolName) - Add finalizeRawChunks() call after stream ends to properly convert MCP tools - Add dynamic MCP tool validation against mcp group in mode permissions - Fix NativeToolCallParser to support string names for dynamic MCP tools --- .../assistant-message/NativeToolCallParser.ts | 48 ++++++++++++++----- .../prompts/tools/native-tools/mcp_server.ts | 28 +++-------- src/core/task/Task.ts | 38 +++++++++++++-- .../__tests__/askFollowupQuestionTool.spec.ts | 18 ++++--- .../tools/__tests__/validateToolUse.spec.ts | 36 ++++++++++++++ src/shared/modes.ts | 10 ++++ 6 files changed, 132 insertions(+), 46 deletions(-) diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 08f842e8894..e02764c879a 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -47,11 +47,12 @@ export type ToolCallStreamEvent = ApiStreamToolCallStartChunk | ApiStreamToolCal */ export class NativeToolCallParser { // Streaming state management for argument accumulation (keyed by tool call id) + // Note: name is string to accommodate dynamic MCP tools (mcp_serverName_toolName) private static streamingToolCalls = new Map< string, { id: string - name: ToolName + name: string argumentsAccumulator: string } >() @@ -194,8 +195,9 @@ export class NativeToolCallParser { /** * Start streaming a new tool call. * Initializes tracking for incremental argument parsing. + * Accepts string to support both ToolName and dynamic MCP tools (mcp_serverName_toolName). */ - public static startStreamingToolCall(id: string, name: ToolName): void { + public static startStreamingToolCall(id: string, name: string): void { this.streamingToolCalls.set(id, { id, name, @@ -235,6 +237,11 @@ export class NativeToolCallParser { // Accumulate the JSON string toolCall.argumentsAccumulator += chunk + // For dynamic MCP tools, we don't return partial updates - wait for final + if (toolCall.name.startsWith("mcp_")) { + return null + } + // Parse whatever we can from the incomplete JSON! // partial-json-parser extracts partial values (strings, arrays, objects) immediately try { @@ -243,7 +250,7 @@ export class NativeToolCallParser { // Create partial ToolUse with extracted values return this.createPartialToolUse( toolCall.id, - toolCall.name, + toolCall.name as ToolName, partialArgs || {}, true, // partial ) @@ -266,9 +273,10 @@ export class NativeToolCallParser { } // Parse the complete accumulated JSON + // Cast to any for the name since parseToolCall handles both ToolName and dynamic MCP tools const finalToolUse = this.parseToolCall({ id: toolCall.id, - name: toolCall.name, + name: toolCall.name as ToolName, arguments: toolCall.argumentsAccumulator, }) @@ -703,6 +711,15 @@ export class NativeToolCallParser { } break + case "access_mcp_resource": + if (args.server_name !== undefined && args.uri !== undefined) { + nativeArgs = { + server_name: args.server_name, + uri: args.uri, + } as NativeArgsFor + } + break + default: break } @@ -734,16 +751,23 @@ export class NativeToolCallParser { */ public static parseDynamicMcpTool(toolCall: { id: string; name: string; arguments: string }): McpToolUse | null { try { - const args = JSON.parse(toolCall.arguments) + // Parse the arguments - these are the actual tool arguments passed directly + const args = JSON.parse(toolCall.arguments || "{}") + + // Extract server_name and tool_name from the tool name itself + // Format: mcp_serverName_toolName + const nameParts = toolCall.name.split("_") + if (nameParts.length < 3 || nameParts[0] !== "mcp") { + console.error(`Invalid dynamic MCP tool name format: ${toolCall.name}`) + return null + } - // Extract server_name and tool_name from the arguments - // The dynamic tool schema includes these as const properties - const serverName = args.server_name - const toolName = args.tool_name - const toolInputProps = args.toolInputProps || {} + // Server name is the second part, tool name is everything after + const serverName = nameParts[1] + const toolName = nameParts.slice(2).join("_") if (!serverName || !toolName) { - console.error(`Missing server_name or tool_name in dynamic MCP tool`) + console.error(`Could not extract server_name or tool_name from: ${toolCall.name}`) return null } @@ -754,7 +778,7 @@ export class NativeToolCallParser { name: toolCall.name, serverName, toolName, - arguments: toolInputProps, + arguments: args, partial: false, } diff --git a/src/core/prompts/tools/native-tools/mcp_server.ts b/src/core/prompts/tools/native-tools/mcp_server.ts index e174e0f0779..36efdf83fb4 100644 --- a/src/core/prompts/tools/native-tools/mcp_server.ts +++ b/src/core/prompts/tools/native-tools/mcp_server.ts @@ -29,8 +29,10 @@ export function getMcpServerTools(mcpHub?: McpHub): OpenAI.Chat.ChatCompletionTo const toolInputProps = originalSchema?.properties ?? {} const toolInputRequired = (originalSchema?.required ?? []) as string[] - // Create a proper JSON Schema object for toolInputProps - const toolInputPropsSchema: Record = { + // Build parameters directly from the tool's input schema. + // The server_name and tool_name are encoded in the function name itself + // (e.g., mcp_serverName_toolName), so they don't need to be in the arguments. + const parameters: OpenAI.FunctionParameters = { type: "object", properties: toolInputProps, additionalProperties: false, @@ -38,28 +40,10 @@ export function getMcpServerTools(mcpHub?: McpHub): OpenAI.Chat.ChatCompletionTo // Only add required if there are required fields if (toolInputRequired.length > 0) { - toolInputPropsSchema.required = toolInputRequired + parameters.required = toolInputRequired } - // Build parameters with all properties defined before adding required array - const parameters = { - type: "object", - properties: { - toolInputProps: toolInputPropsSchema, - server_name: { - type: "string", - const: server.name, - }, - tool_name: { - type: "string", - const: tool.name, - }, - }, - required: ["server_name", "tool_name", "toolInputProps"], - additionalProperties: false, - } as OpenAI.FunctionParameters - - // Use triple underscores as separator to allow underscores in tool and server names + // Use mcp_ prefix to identify dynamic MCP tools const toolDefinition: OpenAI.Chat.ChatCompletionTool = { type: "function", function: { diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9b60c312abf..cdb084a8475 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -2538,6 +2538,37 @@ export class Task extends EventEmitter implements TaskLike { } } + // Finalize any remaining streaming tool calls that weren't explicitly ended + // This is critical for MCP tools which need tool_call_end events to be properly + // converted from ToolUse to McpToolUse via finalizeStreamingToolCall() + const finalizeEvents = NativeToolCallParser.finalizeRawChunks() + for (const event of finalizeEvents) { + if (event.type === "tool_call_end") { + // Finalize the streaming tool call + const finalToolUse = NativeToolCallParser.finalizeStreamingToolCall(event.id) + + if (finalToolUse) { + // Store the tool call ID + ;(finalToolUse as any).id = event.id + + // Get the index and replace partial with final + const toolUseIndex = this.streamingToolCallIndices.get(event.id) + if (toolUseIndex !== undefined) { + this.assistantMessageContent[toolUseIndex] = finalToolUse + } + + // Clean up tracking + this.streamingToolCallIndices.delete(event.id) + + // Mark that we have new content to process + this.userMessageContentReady = false + + // Present the finalized tool call + presentAssistantMessage(this) + } + } + } + // Create a copy of current token values to avoid race conditions const currentTokens = { input: inputTokens, @@ -2879,17 +2910,14 @@ export class Task extends EventEmitter implements TaskLike { for (const block of toolUseBlocks) { if (block.type === "mcp_tool_use") { // McpToolUse already has the original tool name (e.g., "mcp_serverName_toolName") + // The arguments are the raw tool arguments (matching the simplified schema) const mcpBlock = block as import("../../shared/tools").McpToolUse if (mcpBlock.id) { assistantContent.push({ type: "tool_use" as const, id: mcpBlock.id, name: mcpBlock.name, // Original dynamic name - input: { - server_name: mcpBlock.serverName, - tool_name: mcpBlock.toolName, - toolInputProps: mcpBlock.arguments, - }, + input: mcpBlock.arguments, // Direct tool arguments }) } } else { diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 9f0b25274bf..074617130c9 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -191,15 +191,19 @@ describe("askFollowupQuestionTool", () => { const result = NativeToolCallParser.finalizeStreamingToolCall("call_456") expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") expect(result?.name).toBe("ask_followup_question") expect(result?.partial).toBe(false) - expect(result?.nativeArgs).toEqual({ - question: "Choose an option", - follow_up: [ - { text: "Yes", mode: "code" }, - { text: "No", mode: null }, - ], - }) + // Type guard: regular tools have type 'tool_use', MCP tools have type 'mcp_tool_use' + if (result?.type === "tool_use") { + expect(result.nativeArgs).toEqual({ + question: "Choose an option", + follow_up: [ + { text: "Yes", mode: "code" }, + { text: "No", mode: null }, + ], + }) + } }) }) }) diff --git a/src/core/tools/__tests__/validateToolUse.spec.ts b/src/core/tools/__tests__/validateToolUse.spec.ts index 7802674605b..ed41d286a54 100644 --- a/src/core/tools/__tests__/validateToolUse.spec.ts +++ b/src/core/tools/__tests__/validateToolUse.spec.ts @@ -103,6 +103,42 @@ describe("mode-validator", () => { }) }) + describe("dynamic MCP tools", () => { + it("allows dynamic MCP tools when mcp group is in mode groups", () => { + // Code mode has mcp group, so dynamic MCP tools should be allowed + expect(isToolAllowedForMode("mcp_context7_resolve-library-id", codeMode, [])).toBe(true) + expect(isToolAllowedForMode("mcp_serverName_toolName", codeMode, [])).toBe(true) + }) + + it("disallows dynamic MCP tools when mcp group is not in mode groups", () => { + const customModes: ModeConfig[] = [ + { + slug: "no-mcp-mode", + name: "No MCP Mode", + roleDefinition: "Custom role", + groups: ["read", "edit"] as const, + }, + ] + // Custom mode without mcp group should not allow dynamic MCP tools + expect(isToolAllowedForMode("mcp_context7_resolve-library-id", "no-mcp-mode", customModes)).toBe(false) + expect(isToolAllowedForMode("mcp_serverName_toolName", "no-mcp-mode", customModes)).toBe(false) + }) + + it("allows dynamic MCP tools in custom mode with mcp group", () => { + const customModes: ModeConfig[] = [ + { + slug: "custom-mcp-mode", + name: "Custom MCP Mode", + roleDefinition: "Custom role", + groups: ["read", "mcp"] as const, + }, + ] + expect(isToolAllowedForMode("mcp_context7_resolve-library-id", "custom-mcp-mode", customModes)).toBe( + true, + ) + }) + }) + describe("tool requirements", () => { it("respects tool requirements when provided", () => { const requirements = { apply_diff: false } diff --git a/src/shared/modes.ts b/src/shared/modes.ts index f68d25c6829..484e79fd445 100644 --- a/src/shared/modes.ts +++ b/src/shared/modes.ts @@ -176,6 +176,10 @@ export function isToolAllowedForMode( if (ALWAYS_AVAILABLE_TOOLS.includes(tool as any)) { return true } + + // Check if this is a dynamic MCP tool (mcp_serverName_toolName) + // These should be allowed if the mcp group is allowed for the mode + const isDynamicMcpTool = tool.startsWith("mcp_") if (experiments && Object.values(EXPERIMENT_IDS).includes(tool as ExperimentId)) { if (!experiments[tool]) { return false @@ -204,6 +208,12 @@ export function isToolAllowedForMode( const groupConfig = TOOL_GROUPS[groupName] + // Check if this is a dynamic MCP tool and the mcp group is allowed + if (isDynamicMcpTool && groupName === "mcp") { + // Dynamic MCP tools are allowed if the mcp group is in the mode's groups + return true + } + // If the tool isn't in this group's tools, continue to next group if (!groupConfig.tools.includes(tool)) { continue