feat: sanitize MCP server/tool names for API compatibility (#10054)

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<TName> | 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,

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,
 				},

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,
 				},

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<string, NodeJS.Timeout> = new Map()
 	private isProgrammaticUpdate: boolean = false
 	private flagResetTimer?: NodeJS.Timeout
+	private sanitizedNameRegistry: Map<string, string> = 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<McpTool[]> {
 		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(

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",
+			})
+		})
+	})
+})

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 }
+}