fix: use -- separator for MCP tool names to avoid underscore parsing ambiguity

The previous underscore-based format was incompatible with NativeToolCallParser.parseDynamicMcpTool() which uses underscore splitting. For a server named 'my server' with tool 'get_tool', the built name 'mcp_my_server_get_tool' got parsed incorrectly.

Now using '--' (double hyphen) as the separator:
- Format: mcp--serverName--toolName
- Server names with underscores (from space sanitization) are correctly preserved
- Double hyphens in names are collapsed to single to avoid separator conflicts

Author: daniel-lxs

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/utils/__tests__/mcp-name.spec.ts

@@ -1,6 +1,13 @@
-import { sanitizeMcpName, buildMcpToolName, parseMcpToolName } from "../mcp-name"
+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("_")
@@ -36,6 +43,12 @@ describe("mcp-name utilities", () => {
 			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")
@@ -49,65 +62,77 @@ describe("mcp-name utilities", () => {
 	})
 
 	describe("buildMcpToolName", () => {
-		it("should build tool name with mcp_ prefix", () => {
-			expect(buildMcpToolName("server", "tool")).toBe("mcp_server_tool")
+		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")
+			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")
+			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)
+			expect(result.startsWith("mcp--")).toBe(true)
 		})
 
 		it("should handle names starting with numbers", () => {
-			expect(buildMcpToolName("123server", "456tool")).toBe("mcp__123server__456tool")
+			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({
+			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("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({
+			expect(parseMcpToolName("mcp--server--tool_name")).toEqual({
 				serverName: "server",
 				toolName: "tool_name",
 			})
 		})
 
-		it("should handle server names with underscores (edge case)", () => {
-			// Note: parseMcpToolName uses simple split, so it can't distinguish
-			// server_name_tool from server + name_tool
-			// The first underscore after 'mcp_' is treated as the server/tool separator
-			const result = parseMcpToolName("mcp_my_server_tool")
-			expect(result).toEqual({
-				serverName: "my",
-				toolName: "server_tool",
+		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()
+			expect(parseMcpToolName("mcp--")).toBeNull()
+			expect(parseMcpToolName("mcp--server")).toBeNull()
 		})
 	})
 
@@ -121,14 +146,32 @@ describe("mcp-name utilities", () => {
 			})
 		})
 
-		it("should preserve sanitized names through roundtrip", () => {
-			// Names with spaces become underscores, but roundtrip still works
-			// for the sanitized version
+		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",
-				toolName: "server_my_tool",
+				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

@@ -8,8 +8,26 @@
  * - 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
@@ -25,6 +43,9 @@ export function sanitizeMcpName(name: string): string {
 	// 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
@@ -40,21 +61,20 @@ export function sanitizeMcpName(name: string): string {
 
 /**
  * Build a full MCP tool function name from server and tool names.
- * The format is: mcp_{sanitized_server_name}_{sanitized_tool_name}
+ * 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
+ * @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}
-	// "mcp_" = 4 chars, "_" separator = 1 char, max total = 64
-	const fullName = `mcp_${sanitizedServer}_${sanitizedTool}`
+	// 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) {
@@ -66,30 +86,31 @@ export function buildMcpToolName(serverName: string, toolName: string): string {
 
 /**
  * Parse an MCP tool function name back into server and tool names.
- * This handles sanitized names by splitting on the expected format.
+ * 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")
+ * @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 {
-	if (!mcpToolName.startsWith("mcp_")) {
+	const prefix = MCP_TOOL_PREFIX + MCP_TOOL_SEPARATOR
+	if (!mcpToolName.startsWith(prefix)) {
 		return null
 	}
 
-	// Remove the "mcp_" prefix
-	const remainder = mcpToolName.slice(4)
+	// Remove the "mcp--" prefix
+	const remainder = mcpToolName.slice(prefix.length)
 
-	// Find the first underscore to split server from tool
-	const underscoreIndex = remainder.indexOf("_")
-	if (underscoreIndex === -1) {
+	// 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, underscoreIndex)
-	const toolName = remainder.slice(underscoreIndex + 1)
+	const serverName = remainder.slice(0, separatorIndex)
+	const toolName = remainder.slice(separatorIndex + MCP_TOOL_SEPARATOR.length)
 
 	if (!serverName || !toolName) {
 		return null