diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 58d07239628..250afdc3890 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -13,6 +13,7 @@ import type { ApiStreamToolCallDeltaChunk, ApiStreamToolCallEndChunk, } from "../../api/transform/stream" +import { MCP_TOOL_PREFIX, MCP_TOOL_SEPARATOR, parseMcpToolName } from "../../utils/mcp-name" /** * Helper type to extract properly typed native arguments for a given tool. @@ -238,7 +239,8 @@ export class NativeToolCallParser { toolCall.argumentsAccumulator += chunk // For dynamic MCP tools, we don't return partial updates - wait for final - if (toolCall.name.startsWith("mcp_")) { + const mcpPrefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR + if (toolCall.name.startsWith(mcpPrefix)) { return null } @@ -554,8 +556,9 @@ export class NativeToolCallParser { name: TName arguments: string }): ToolUse | McpToolUse | null { - // Check if this is a dynamic MCP tool (mcp_serverName_toolName) - if (typeof toolCall.name === "string" && toolCall.name.startsWith("mcp_")) { + // Check if this is a dynamic MCP tool (mcp--serverName--toolName) + const mcpPrefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR + if (typeof toolCall.name === "string" && toolCall.name.startsWith(mcpPrefix)) { return this.parseDynamicMcpTool(toolCall) } @@ -811,7 +814,7 @@ export class NativeToolCallParser { } /** - * Parse dynamic MCP tools (named mcp_serverName_toolName). + * Parse dynamic MCP tools (named mcp--serverName--toolName). * These are generated dynamically by getMcpServerTools() and are returned * as McpToolUse objects that preserve the original tool name. * @@ -825,26 +828,19 @@ export class NativeToolCallParser { 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") { + // Format: mcp--serverName--toolName (using -- separator) + const parsed = parseMcpToolName(toolCall.name) + if (!parsed) { console.error(`Invalid dynamic MCP tool name format: ${toolCall.name}`) return null } - // 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(`Could not extract server_name or tool_name from: ${toolCall.name}`) - return null - } + const { serverName, toolName } = parsed 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 + // Keep the original tool name (e.g., "mcp--serverName--toolName") for API history name: toolCall.name, serverName, toolName, diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index dd1337b6d52..78d2afc166f 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -241,6 +241,18 @@ export async function presentAssistantMessage(cline: Task) { TelemetryService.instance.captureToolUsage(cline.taskId, "use_mcp_tool", toolProtocol) } + // Resolve sanitized server name back to original server name + // The serverName from parsing is sanitized (e.g., "my_server" from "my server") + // We need the original name to find the actual MCP connection + const mcpHub = cline.providerRef.deref()?.getMcpHub() + let resolvedServerName = mcpBlock.serverName + if (mcpHub) { + const originalName = mcpHub.findServerNameBySanitizedName(mcpBlock.serverName) + if (originalName) { + resolvedServerName = originalName + } + } + // 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"> = { @@ -248,13 +260,13 @@ export async function presentAssistantMessage(cline: Task) { id: mcpBlock.id, name: "use_mcp_tool", params: { - server_name: mcpBlock.serverName, + server_name: resolvedServerName, tool_name: mcpBlock.toolName, arguments: JSON.stringify(mcpBlock.arguments), }, partial: mcpBlock.partial, nativeArgs: { - server_name: mcpBlock.serverName, + server_name: resolvedServerName, tool_name: mcpBlock.toolName, arguments: mcpBlock.arguments, }, diff --git a/src/core/prompts/tools/native-tools/mcp_server.ts b/src/core/prompts/tools/native-tools/mcp_server.ts index 36efdf83fb4..f40da7cf500 100644 --- a/src/core/prompts/tools/native-tools/mcp_server.ts +++ b/src/core/prompts/tools/native-tools/mcp_server.ts @@ -1,5 +1,6 @@ import type OpenAI from "openai" import { McpHub } from "../../../../services/mcp/McpHub" +import { buildMcpToolName } from "../../../../utils/mcp-name" /** * Dynamically generates native tool definitions for all enabled tools across connected MCP servers. @@ -43,11 +44,14 @@ export function getMcpServerTools(mcpHub?: McpHub): OpenAI.Chat.ChatCompletionTo parameters.required = toolInputRequired } - // Use mcp_ prefix to identify dynamic MCP tools + // Build sanitized tool name for API compliance + // The name is sanitized to conform to API requirements (e.g., Gemini's function name restrictions) + const toolName = buildMcpToolName(server.name, tool.name) + const toolDefinition: OpenAI.Chat.ChatCompletionTool = { type: "function", function: { - name: `mcp_${server.name}_${tool.name}`, + name: toolName, description: tool.description, parameters: parameters, }, diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 2db26332fae..3d54cb670e2 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -33,6 +33,7 @@ import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual, getWorkspacePath } from "../../utils/path" import { injectVariables } from "../../utils/config" import { safeWriteJson } from "../../utils/safeWriteJson" +import { sanitizeMcpName } from "../../utils/mcp-name" // Discriminated union for connection states export type ConnectedMcpConnection = { @@ -154,6 +155,7 @@ export class McpHub { private configChangeDebounceTimers: Map = new Map() private isProgrammaticUpdate: boolean = false private flagResetTimer?: NodeJS.Timeout + private sanitizedNameRegistry: Map = new Map() constructor(provider: ClineProvider) { this.providerRef = new WeakRef(provider) @@ -627,6 +629,10 @@ export class McpHub { // Remove existing connection if it exists with the same source await this.deleteConnection(name, source) + // Register the sanitized name for O(1) lookup + const sanitizedName = sanitizeMcpName(name) + this.sanitizedNameRegistry.set(sanitizedName, name) + // Check if MCP is globally enabled const mcpEnabled = await this.isMcpEnabled() if (!mcpEnabled) { @@ -910,6 +916,22 @@ export class McpHub { ) } + /** + * Find a connection by sanitized server name. + * This is used when parsing MCP tool responses where the server name has been + * sanitized (e.g., hyphens replaced with underscores) for API compliance. + * @param sanitizedServerName The sanitized server name from the API tool call + * @returns The original server name if found, or null if no match + */ + public findServerNameBySanitizedName(sanitizedServerName: string): string | null { + const exactMatch = this.connections.find((conn) => conn.server.name === sanitizedServerName) + if (exactMatch) { + return exactMatch.server.name + } + + return this.sanitizedNameRegistry.get(sanitizedServerName) ?? null + } + private async fetchToolsList(serverName: string, source?: "global" | "project"): Promise { try { // Use the helper method to find the connection @@ -1027,6 +1049,13 @@ export class McpHub { if (source && conn.server.source !== source) return true return false }) + + // Remove from sanitized name registry if no more connections with this name exist + const remainingConnections = this.connections.filter((conn) => conn.server.name === name) + if (remainingConnections.length === 0) { + const sanitizedName = sanitizeMcpName(name) + this.sanitizedNameRegistry.delete(sanitizedName) + } } async updateServerConnections( diff --git a/src/utils/__tests__/mcp-name.spec.ts b/src/utils/__tests__/mcp-name.spec.ts new file mode 100644 index 00000000000..76c069ee8d0 --- /dev/null +++ b/src/utils/__tests__/mcp-name.spec.ts @@ -0,0 +1,178 @@ +import { sanitizeMcpName, buildMcpToolName, parseMcpToolName, MCP_TOOL_SEPARATOR, MCP_TOOL_PREFIX } from "../mcp-name" + +describe("mcp-name utilities", () => { + describe("constants", () => { + it("should have correct separator and prefix", () => { + expect(MCP_TOOL_SEPARATOR).toBe("--") + expect(MCP_TOOL_PREFIX).toBe("mcp") + }) + }) + + describe("sanitizeMcpName", () => { + it("should return underscore placeholder for empty input", () => { + expect(sanitizeMcpName("")).toBe("_") + }) + + it("should replace spaces with underscores", () => { + expect(sanitizeMcpName("my server")).toBe("my_server") + expect(sanitizeMcpName("server name here")).toBe("server_name_here") + }) + + it("should remove invalid characters", () => { + expect(sanitizeMcpName("server@name!")).toBe("servername") + expect(sanitizeMcpName("test#$%^&*()")).toBe("test") + }) + + it("should keep valid characters (alphanumeric, underscore, dot, colon, dash)", () => { + expect(sanitizeMcpName("server_name")).toBe("server_name") + expect(sanitizeMcpName("server.name")).toBe("server.name") + expect(sanitizeMcpName("server:name")).toBe("server:name") + expect(sanitizeMcpName("server-name")).toBe("server-name") + expect(sanitizeMcpName("Server123")).toBe("Server123") + }) + + it("should prepend underscore if name starts with non-letter/underscore", () => { + expect(sanitizeMcpName("123server")).toBe("_123server") + expect(sanitizeMcpName("-server")).toBe("_-server") + expect(sanitizeMcpName(".server")).toBe("_.server") + }) + + it("should not modify names that start with letter or underscore", () => { + expect(sanitizeMcpName("server")).toBe("server") + expect(sanitizeMcpName("_server")).toBe("_server") + expect(sanitizeMcpName("Server")).toBe("Server") + }) + + it("should replace double-hyphen sequences with single hyphen to avoid separator conflicts", () => { + expect(sanitizeMcpName("server--name")).toBe("server-name") + expect(sanitizeMcpName("test---server")).toBe("test-server") + expect(sanitizeMcpName("my----tool")).toBe("my-tool") + }) + + it("should handle complex names with multiple issues", () => { + expect(sanitizeMcpName("My Server @ Home!")).toBe("My_Server__Home") + expect(sanitizeMcpName("123-test server")).toBe("_123-test_server") + }) + + it("should return placeholder for names that become empty after sanitization", () => { + expect(sanitizeMcpName("@#$%")).toBe("_unnamed") + // Spaces become underscores, which is a valid character, so it returns "_" + expect(sanitizeMcpName(" ")).toBe("_") + }) + }) + + describe("buildMcpToolName", () => { + it("should build tool name with mcp-- prefix and -- separators", () => { + expect(buildMcpToolName("server", "tool")).toBe("mcp--server--tool") + }) + + it("should sanitize both server and tool names", () => { + expect(buildMcpToolName("my server", "my tool")).toBe("mcp--my_server--my_tool") + }) + + it("should handle names with special characters", () => { + expect(buildMcpToolName("server@name", "tool!name")).toBe("mcp--servername--toolname") + }) + + it("should truncate long names to 64 characters", () => { + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + const result = buildMcpToolName(longServer, longTool) + expect(result.length).toBeLessThanOrEqual(64) + expect(result.startsWith("mcp--")).toBe(true) + }) + + it("should handle names starting with numbers", () => { + expect(buildMcpToolName("123server", "456tool")).toBe("mcp--_123server--_456tool") + }) + + it("should preserve underscores in server and tool names", () => { + expect(buildMcpToolName("my_server", "my_tool")).toBe("mcp--my_server--my_tool") + }) + }) + + describe("parseMcpToolName", () => { + it("should parse valid mcp tool names", () => { + expect(parseMcpToolName("mcp--server--tool")).toEqual({ + serverName: "server", + toolName: "tool", + }) + }) + + it("should return null for non-mcp tool names", () => { + expect(parseMcpToolName("server--tool")).toBeNull() + expect(parseMcpToolName("tool")).toBeNull() + }) + + it("should return null for old underscore format", () => { + expect(parseMcpToolName("mcp_server_tool")).toBeNull() + }) + + it("should handle tool names with underscores", () => { + expect(parseMcpToolName("mcp--server--tool_name")).toEqual({ + serverName: "server", + toolName: "tool_name", + }) + }) + + it("should correctly handle server names with underscores (fixed from old behavior)", () => { + // With the new -- separator, server names with underscores work correctly + expect(parseMcpToolName("mcp--my_server--tool")).toEqual({ + serverName: "my_server", + toolName: "tool", + }) + }) + + it("should handle both server and tool names with underscores", () => { + expect(parseMcpToolName("mcp--my_server--get_forecast")).toEqual({ + serverName: "my_server", + toolName: "get_forecast", + }) + }) + + it("should return null for malformed names", () => { + expect(parseMcpToolName("mcp--")).toBeNull() + expect(parseMcpToolName("mcp--server")).toBeNull() + }) + }) + + describe("roundtrip behavior", () => { + it("should be able to parse names that were built", () => { + const toolName = buildMcpToolName("server", "tool") + const parsed = parseMcpToolName(toolName) + expect(parsed).toEqual({ + serverName: "server", + toolName: "tool", + }) + }) + + it("should preserve sanitized names through roundtrip with underscores", () => { + // Names with underscores now work correctly through roundtrip + const toolName = buildMcpToolName("my_server", "my_tool") + const parsed = parseMcpToolName(toolName) + expect(parsed).toEqual({ + serverName: "my_server", + toolName: "my_tool", + }) + }) + + it("should handle spaces that get converted to underscores", () => { + // "my server" becomes "my_server" after sanitization + const toolName = buildMcpToolName("my server", "get tool") + const parsed = parseMcpToolName(toolName) + expect(parsed).toEqual({ + serverName: "my_server", + toolName: "get_tool", + }) + }) + + it("should handle complex server and tool names", () => { + const toolName = buildMcpToolName("Weather API", "get_current_forecast") + const parsed = parseMcpToolName(toolName) + expect(parsed).toEqual({ + serverName: "Weather_API", + toolName: "get_current_forecast", + }) + }) + }) +}) diff --git a/src/utils/mcp-name.ts b/src/utils/mcp-name.ts new file mode 100644 index 00000000000..af50f392cfe --- /dev/null +++ b/src/utils/mcp-name.ts @@ -0,0 +1,120 @@ +/** + * Utilities for sanitizing MCP server and tool names to conform to + * API function name requirements (e.g., Gemini's restrictions). + * + * Gemini function name requirements: + * - Must start with a letter or an underscore + * - Must be alphanumeric (a-z, A-Z, 0-9), underscores (_), dots (.), colons (:), or dashes (-) + * - Maximum length of 64 characters + */ + +/** + * Separator used between MCP prefix, server name, and tool name. + * We use "--" (double hyphen) because: + * 1. It's allowed by Gemini (dashes are permitted in function names) + * 2. It won't conflict with underscores in sanitized server/tool names + * 3. It's unique enough to be a reliable delimiter for parsing + */ +export const MCP_TOOL_SEPARATOR = "--" + +/** + * Prefix for all MCP tool function names. + */ +export const MCP_TOOL_PREFIX = "mcp" + +/** + * Sanitize a name to be safe for use in API function names. + * This removes special characters and ensures the name starts correctly. + * + * Note: This does NOT remove dashes from names, but the separator "--" is + * distinct enough (double hyphen) that single hyphens in names won't conflict. + * + * @param name - The original name (e.g., MCP server name or tool name) + * @returns A sanitized name that conforms to API requirements + */ +export function sanitizeMcpName(name: string): string { + if (!name) { + return "_" + } + + // Replace spaces with underscores first + let sanitized = name.replace(/\s+/g, "_") + + // Remove any characters that are not alphanumeric, underscores, dots, colons, or dashes + sanitized = sanitized.replace(/[^a-zA-Z0-9_.\-:]/g, "") + + // Replace any double-hyphen sequences with single hyphen to avoid separator conflicts + sanitized = sanitized.replace(/--+/g, "-") + + // Ensure the name starts with a letter or underscore + if (sanitized.length > 0 && !/^[a-zA-Z_]/.test(sanitized)) { + sanitized = "_" + sanitized + } + + // If empty after sanitization, use a placeholder + if (!sanitized) { + sanitized = "_unnamed" + } + + return sanitized +} + +/** + * Build a full MCP tool function name from server and tool names. + * The format is: mcp--{sanitized_server_name}--{sanitized_tool_name} + * + * The total length is capped at 64 characters to conform to API limits. + * + * @param serverName - The MCP server name + * @param toolName - The tool name + * @returns A sanitized function name in the format mcp--serverName--toolName + */ +export function buildMcpToolName(serverName: string, toolName: string): string { + const sanitizedServer = sanitizeMcpName(serverName) + const sanitizedTool = sanitizeMcpName(toolName) + + // Build the full name: mcp--{server}--{tool} + const fullName = `${MCP_TOOL_PREFIX}${MCP_TOOL_SEPARATOR}${sanitizedServer}${MCP_TOOL_SEPARATOR}${sanitizedTool}` + + // Truncate if necessary (max 64 chars for Gemini) + if (fullName.length > 64) { + return fullName.slice(0, 64) + } + + return fullName +} + +/** + * Parse an MCP tool function name back into server and tool names. + * This handles sanitized names by splitting on the "--" separator. + * + * Note: This returns the sanitized names, not the original names. + * The original names cannot be recovered from the sanitized version. + * + * @param mcpToolName - The full MCP tool name (e.g., "mcp--weather--get_forecast") + * @returns An object with serverName and toolName, or null if parsing fails + */ +export function parseMcpToolName(mcpToolName: string): { serverName: string; toolName: string } | null { + const prefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR + if (!mcpToolName.startsWith(prefix)) { + return null + } + + // Remove the "mcp--" prefix + const remainder = mcpToolName.slice(prefix.length) + + // Split on the separator to get server and tool names + const separatorIndex = remainder.indexOf(MCP_TOOL_SEPARATOR) + if (separatorIndex === -1) { + return null + } + + const serverName = remainder.slice(0, separatorIndex) + const toolName = remainder.slice(separatorIndex + MCP_TOOL_SEPARATOR.length) + + if (!serverName || !toolName) { + return null + } + + return { serverName, toolName } +}