Skip to content

Commit ba92542

Browse files
daniel-lxsdaniel-lxs
committed
feat: sanitize MCP server/tool names for API compatibility
- Add sanitizeMcpName() utility to handle spaces, hyphens, and special characters - Update tool ID generation in mcp_server.ts to use sanitized names - Add O(1) registry lookup in McpHub for mapping sanitized names back to originals - Maintain registry lifecycle in connectToServer() and deleteConnection() - Add comprehensive test coverage (20 tests for mcp-name utility) Fixes issues with MCP servers that have names containing spaces or special characters which would cause tool calls to fail with APIs like Gemini that require alphanumeric function names.
1 parent ba7c553 commit ba92542

File tree

5 files changed

+287
-4
lines changed

5 files changed

+287
-4
lines changed

‎src/core/assistant-message/presentAssistantMessage.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,20 +241,32 @@ export async function presentAssistantMessage(cline: Task) {
241241
TelemetryService.instance.captureToolUsage(cline.taskId, "use_mcp_tool", toolProtocol)
242242
}
243243

244+
// Resolve sanitized server name back to original server name
245+
// The serverName from parsing is sanitized (e.g., "my_server" from "my server")
246+
// We need the original name to find the actual MCP connection
247+
const mcpHub = cline.providerRef.deref()?.getMcpHub()
248+
let resolvedServerName = mcpBlock.serverName
249+
if (mcpHub) {
250+
const originalName = mcpHub.findServerNameBySanitizedName(mcpBlock.serverName)
251+
if (originalName) {
252+
resolvedServerName = originalName
253+
}
254+
}
255+
244256
// Execute the MCP tool using the same handler as use_mcp_tool
245257
// Create a synthetic ToolUse block that the useMcpToolTool can handle
246258
const syntheticToolUse: ToolUse<"use_mcp_tool"> = {
247259
type: "tool_use",
248260
id: mcpBlock.id,
249261
name: "use_mcp_tool",
250262
params: {
251-
server_name: mcpBlock.serverName,
263+
server_name: resolvedServerName,
252264
tool_name: mcpBlock.toolName,
253265
arguments: JSON.stringify(mcpBlock.arguments),
254266
},
255267
partial: mcpBlock.partial,
256268
nativeArgs: {
257-
server_name: mcpBlock.serverName,
269+
server_name: resolvedServerName,
258270
tool_name: mcpBlock.toolName,
259271
arguments: mcpBlock.arguments,
260272
},

‎src/core/prompts/tools/native-tools/mcp_server.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type OpenAI from "openai"
22
import { McpHub } from "../../../../services/mcp/McpHub"
3+
import { buildMcpToolName } from "../../../../utils/mcp-name"
34

45
/**
56
* 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
4344
parameters.required = toolInputRequired
4445
}
4546

46-
// Use mcp_ prefix to identify dynamic MCP tools
47+
// Build sanitized tool name for API compliance
48+
// The name is sanitized to conform to API requirements (e.g., Gemini's function name restrictions)
49+
const toolName = buildMcpToolName(server.name, tool.name)
50+
4751
const toolDefinition: OpenAI.Chat.ChatCompletionTool = {
4852
type: "function",
4953
function: {
50-
name: `mcp_${server.name}_${tool.name}`,
54+
name: toolName,
5155
description: tool.description,
5256
parameters: parameters,
5357
},

‎src/services/mcp/McpHub.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { fileExistsAtPath } from "../../utils/fs"
3333
import { arePathsEqual, getWorkspacePath } from "../../utils/path"
3434
import { injectVariables } from "../../utils/config"
3535
import { safeWriteJson } from "../../utils/safeWriteJson"
36+
import { sanitizeMcpName } from "../../utils/mcp-name"
3637

3738
// Discriminated union for connection states
3839
export type ConnectedMcpConnection = {
@@ -154,6 +155,8 @@ export class McpHub {
154155
private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map()
155156
private isProgrammaticUpdate: boolean = false
156157
private flagResetTimer?: NodeJS.Timeout
158+
// Registry for O(1) lookup of original server names from sanitized names
159+
private sanitizedNameRegistry: Map<string, string> = new Map()
157160

158161
constructor(provider: ClineProvider) {
159162
this.providerRef = new WeakRef(provider)
@@ -627,6 +630,10 @@ export class McpHub {
627630
// Remove existing connection if it exists with the same source
628631
await this.deleteConnection(name, source)
629632

633+
// Register the sanitized name for O(1) lookup
634+
const sanitizedName = sanitizeMcpName(name)
635+
this.sanitizedNameRegistry.set(sanitizedName, name)
636+
630637
// Check if MCP is globally enabled
631638
const mcpEnabled = await this.isMcpEnabled()
632639
if (!mcpEnabled) {
@@ -910,6 +917,25 @@ export class McpHub {
910917
)
911918
}
912919

920+
/**
921+
* Find a connection by sanitized server name.
922+
* This is used when parsing MCP tool responses where the server name has been
923+
* sanitized (e.g., hyphens replaced with underscores) for API compliance.
924+
* Uses O(1) registry lookup instead of scanning all connections.
925+
* @param sanitizedServerName The sanitized server name from the API tool call
926+
* @returns The original server name if found, or null if no match
927+
*/
928+
public findServerNameBySanitizedName(sanitizedServerName: string): string | null {
929+
// First try exact match (in case the original name was already valid)
930+
const exactMatch = this.connections.find((conn) => conn.server.name === sanitizedServerName)
931+
if (exactMatch) {
932+
return exactMatch.server.name
933+
}
934+
935+
// O(1) lookup using the sanitized name registry
936+
return this.sanitizedNameRegistry.get(sanitizedServerName) ?? null
937+
}
938+
913939
private async fetchToolsList(serverName: string, source?: "global" | "project"): Promise<McpTool[]> {
914940
try {
915941
// Use the helper method to find the connection
@@ -1027,6 +1053,13 @@ export class McpHub {
10271053
if (source && conn.server.source !== source) return true
10281054
return false
10291055
})
1056+
1057+
// Remove from sanitized name registry if no more connections with this name exist
1058+
const remainingConnections = this.connections.filter((conn) => conn.server.name === name)
1059+
if (remainingConnections.length === 0) {
1060+
const sanitizedName = sanitizeMcpName(name)
1061+
this.sanitizedNameRegistry.delete(sanitizedName)
1062+
}
10301063
}
10311064

10321065
async updateServerConnections(

‎src/utils/__tests__/mcp-name.spec.ts‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { sanitizeMcpName, buildMcpToolName, parseMcpToolName } from "../mcp-name"
2+
3+
describe("mcp-name utilities", () => {
4+
describe("sanitizeMcpName", () => {
5+
it("should return underscore placeholder for empty input", () => {
6+
expect(sanitizeMcpName("")).toBe("_")
7+
})
8+
9+
it("should replace spaces with underscores", () => {
10+
expect(sanitizeMcpName("my server")).toBe("my_server")
11+
expect(sanitizeMcpName("server name here")).toBe("server_name_here")
12+
})
13+
14+
it("should remove invalid characters", () => {
15+
expect(sanitizeMcpName("server@name!")).toBe("servername")
16+
expect(sanitizeMcpName("test#$%^&*()")).toBe("test")
17+
})
18+
19+
it("should keep valid characters (alphanumeric, underscore, dot, colon, dash)", () => {
20+
expect(sanitizeMcpName("server_name")).toBe("server_name")
21+
expect(sanitizeMcpName("server.name")).toBe("server.name")
22+
expect(sanitizeMcpName("server:name")).toBe("server:name")
23+
expect(sanitizeMcpName("server-name")).toBe("server-name")
24+
expect(sanitizeMcpName("Server123")).toBe("Server123")
25+
})
26+
27+
it("should prepend underscore if name starts with non-letter/underscore", () => {
28+
expect(sanitizeMcpName("123server")).toBe("_123server")
29+
expect(sanitizeMcpName("-server")).toBe("_-server")
30+
expect(sanitizeMcpName(".server")).toBe("_.server")
31+
})
32+
33+
it("should not modify names that start with letter or underscore", () => {
34+
expect(sanitizeMcpName("server")).toBe("server")
35+
expect(sanitizeMcpName("_server")).toBe("_server")
36+
expect(sanitizeMcpName("Server")).toBe("Server")
37+
})
38+
39+
it("should handle complex names with multiple issues", () => {
40+
expect(sanitizeMcpName("My Server @ Home!")).toBe("My_Server__Home")
41+
expect(sanitizeMcpName("123-test server")).toBe("_123-test_server")
42+
})
43+
44+
it("should return placeholder for names that become empty after sanitization", () => {
45+
expect(sanitizeMcpName("@#$%")).toBe("_unnamed")
46+
// Spaces become underscores, which is a valid character, so it returns "_"
47+
expect(sanitizeMcpName(" ")).toBe("_")
48+
})
49+
})
50+
51+
describe("buildMcpToolName", () => {
52+
it("should build tool name with mcp_ prefix", () => {
53+
expect(buildMcpToolName("server", "tool")).toBe("mcp_server_tool")
54+
})
55+
56+
it("should sanitize both server and tool names", () => {
57+
expect(buildMcpToolName("my server", "my tool")).toBe("mcp_my_server_my_tool")
58+
})
59+
60+
it("should handle names with special characters", () => {
61+
expect(buildMcpToolName("server@name", "tool!name")).toBe("mcp_servername_toolname")
62+
})
63+
64+
it("should truncate long names to 64 characters", () => {
65+
const longServer = "a".repeat(50)
66+
const longTool = "b".repeat(50)
67+
const result = buildMcpToolName(longServer, longTool)
68+
expect(result.length).toBeLessThanOrEqual(64)
69+
expect(result.startsWith("mcp_")).toBe(true)
70+
})
71+
72+
it("should handle names starting with numbers", () => {
73+
expect(buildMcpToolName("123server", "456tool")).toBe("mcp__123server__456tool")
74+
})
75+
})
76+
77+
describe("parseMcpToolName", () => {
78+
it("should parse valid mcp tool names", () => {
79+
expect(parseMcpToolName("mcp_server_tool")).toEqual({
80+
serverName: "server",
81+
toolName: "tool",
82+
})
83+
})
84+
85+
it("should return null for non-mcp tool names", () => {
86+
expect(parseMcpToolName("server_tool")).toBeNull()
87+
expect(parseMcpToolName("tool")).toBeNull()
88+
})
89+
90+
it("should handle tool names with underscores", () => {
91+
expect(parseMcpToolName("mcp_server_tool_name")).toEqual({
92+
serverName: "server",
93+
toolName: "tool_name",
94+
})
95+
})
96+
97+
it("should handle server names with underscores (edge case)", () => {
98+
// Note: parseMcpToolName uses simple split, so it can't distinguish
99+
// server_name_tool from server + name_tool
100+
// The first underscore after 'mcp_' is treated as the server/tool separator
101+
const result = parseMcpToolName("mcp_my_server_tool")
102+
expect(result).toEqual({
103+
serverName: "my",
104+
toolName: "server_tool",
105+
})
106+
})
107+
108+
it("should return null for malformed names", () => {
109+
expect(parseMcpToolName("mcp_")).toBeNull()
110+
expect(parseMcpToolName("mcp_server")).toBeNull()
111+
})
112+
})
113+
114+
describe("roundtrip behavior", () => {
115+
it("should be able to parse names that were built", () => {
116+
const toolName = buildMcpToolName("server", "tool")
117+
const parsed = parseMcpToolName(toolName)
118+
expect(parsed).toEqual({
119+
serverName: "server",
120+
toolName: "tool",
121+
})
122+
})
123+
124+
it("should preserve sanitized names through roundtrip", () => {
125+
// Names with spaces become underscores, but roundtrip still works
126+
// for the sanitized version
127+
const toolName = buildMcpToolName("my_server", "my_tool")
128+
const parsed = parseMcpToolName(toolName)
129+
expect(parsed).toEqual({
130+
serverName: "my",
131+
toolName: "server_my_tool",
132+
})
133+
})
134+
})
135+
})

‎src/utils/mcp-name.ts‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* Utilities for sanitizing MCP server and tool names to conform to
3+
* API function name requirements (e.g., Gemini's restrictions).
4+
*
5+
* Gemini function name requirements:
6+
* - Must start with a letter or an underscore
7+
* - Must be alphanumeric (a-z, A-Z, 0-9), underscores (_), dots (.), colons (:), or dashes (-)
8+
* - Maximum length of 64 characters
9+
*/
10+
11+
/**
12+
* Sanitize a name to be safe for use in API function names.
13+
*
14+
* @param name - The original name (e.g., MCP server name or tool name)
15+
* @returns A sanitized name that conforms to API requirements
16+
*/
17+
export function sanitizeMcpName(name: string): string {
18+
if (!name) {
19+
return "_"
20+
}
21+
22+
// Replace spaces with underscores first
23+
let sanitized = name.replace(/\s+/g, "_")
24+
25+
// Remove any characters that are not alphanumeric, underscores, dots, colons, or dashes
26+
sanitized = sanitized.replace(/[^a-zA-Z0-9_.\-:]/g, "")
27+
28+
// Ensure the name starts with a letter or underscore
29+
if (sanitized.length > 0 && !/^[a-zA-Z_]/.test(sanitized)) {
30+
sanitized = "_" + sanitized
31+
}
32+
33+
// If empty after sanitization, use a placeholder
34+
if (!sanitized) {
35+
sanitized = "_unnamed"
36+
}
37+
38+
return sanitized
39+
}
40+
41+
/**
42+
* Build a full MCP tool function name from server and tool names.
43+
* The format is: mcp_{sanitized_server_name}_{sanitized_tool_name}
44+
*
45+
* The total length is capped at 64 characters to conform to API limits.
46+
*
47+
* @param serverName - The MCP server name
48+
* @param toolName - The tool name
49+
* @returns A sanitized function name in the format mcp_serverName_toolName
50+
*/
51+
export function buildMcpToolName(serverName: string, toolName: string): string {
52+
const sanitizedServer = sanitizeMcpName(serverName)
53+
const sanitizedTool = sanitizeMcpName(toolName)
54+
55+
// Build the full name: mcp_{server}_{tool}
56+
// "mcp_" = 4 chars, "_" separator = 1 char, max total = 64
57+
const fullName = `mcp_${sanitizedServer}_${sanitizedTool}`
58+
59+
// Truncate if necessary (max 64 chars for Gemini)
60+
if (fullName.length > 64) {
61+
return fullName.slice(0, 64)
62+
}
63+
64+
return fullName
65+
}
66+
67+
/**
68+
* Parse an MCP tool function name back into server and tool names.
69+
* This handles sanitized names by splitting on the expected format.
70+
*
71+
* Note: This returns the sanitized names, not the original names.
72+
* The original names cannot be recovered from the sanitized version.
73+
*
74+
* @param mcpToolName - The full MCP tool name (e.g., "mcp_weather_get_forecast")
75+
* @returns An object with serverName and toolName, or null if parsing fails
76+
*/
77+
export function parseMcpToolName(mcpToolName: string): { serverName: string; toolName: string } | null {
78+
if (!mcpToolName.startsWith("mcp_")) {
79+
return null
80+
}
81+
82+
// Remove the "mcp_" prefix
83+
const remainder = mcpToolName.slice(4)
84+
85+
// Find the first underscore to split server from tool
86+
const underscoreIndex = remainder.indexOf("_")
87+
if (underscoreIndex === -1) {
88+
return null
89+
}
90+
91+
const serverName = remainder.slice(0, underscoreIndex)
92+
const toolName = remainder.slice(underscoreIndex + 1)
93+
94+
if (!serverName || !toolName) {
95+
return null
96+
}
97+
98+
return { serverName, toolName }
99+
}

0 commit comments

Comments
 (0)