Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/core/assistant-message/presentAssistantMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,20 +241,32 @@ 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"> = {
type: "tool_use",
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,
},
Expand Down
8 changes: 6 additions & 2 deletions src/core/prompts/tools/native-tools/mcp_server.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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,
},
Expand Down
33 changes: 33 additions & 0 deletions src/services/mcp/McpHub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -154,6 +155,8 @@ export class McpHub {
private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map()
private isProgrammaticUpdate: boolean = false
private flagResetTimer?: NodeJS.Timeout
// Registry for O(1) lookup of original server names from sanitized names
private sanitizedNameRegistry: Map<string, string> = new Map()

constructor(provider: ClineProvider) {
this.providerRef = new WeakRef(provider)
Expand Down Expand Up @@ -627,6 +630,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) {
Expand Down Expand Up @@ -910,6 +917,25 @@ 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.
* Uses O(1) registry lookup instead of scanning all connections.
* @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 {
// First try exact match (in case the original name was already valid)
const exactMatch = this.connections.find((conn) => conn.server.name === sanitizedServerName)
if (exactMatch) {
return exactMatch.server.name
}

// O(1) lookup using the sanitized name registry
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
Expand Down Expand Up @@ -1027,6 +1053,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(
Expand Down
135 changes: 135 additions & 0 deletions src/utils/__tests__/mcp-name.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import { sanitizeMcpName, buildMcpToolName, parseMcpToolName } from "../mcp-name"

describe("mcp-name utilities", () => {
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 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", () => {
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")
})
})

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 handle tool names with underscores", () => {
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 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", () => {
// Names with spaces become underscores, but roundtrip still works
// for the sanitized version
const toolName = buildMcpToolName("my_server", "my_tool")
const parsed = parseMcpToolName(toolName)
expect(parsed).toEqual({
serverName: "my",
toolName: "server_my_tool",
})
})
})
})
99 changes: 99 additions & 0 deletions src/utils/mcp-name.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/**
* 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
*/

/**
* Sanitize a name to be safe for use in API function names.
*
* @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, "")

// 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}
// "mcp_" = 4 chars, "_" separator = 1 char, max total = 64
const fullName = `mcp_${sanitizedServer}_${sanitizedTool}`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This underscore-based format is incompatible with the parsing in NativeToolCallParser.parseDynamicMcpTool(), which uses naive underscore splitting. For a server named "my server" with tool "get_tool", the built name mcp_my_server_get_tool gets parsed as serverName="my" and toolName="server_get_tool". The sanitizedNameRegistry maps "my_server" to "my server", but the lookup receives "my", so resolution fails.

Similarly, server names starting with numbers (e.g., "123server") produce mcp__123server_tool (double underscore), which parses to an empty serverName, causing parseDynamicMcpTool() to return null.

Consider using a different separator character that's allowed by Gemini's function name requirements (dots, colons, or dashes) to avoid ambiguity with underscores from sanitization.

Fix it with Roo Code or mention @roomote and request a fix.


// 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 expected format.
*
* 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 {
if (!mcpToolName.startsWith("mcp_")) {
return null
}

// Remove the "mcp_" prefix
const remainder = mcpToolName.slice(4)

// Find the first underscore to split server from tool
const underscoreIndex = remainder.indexOf("_")
if (underscoreIndex === -1) {
return null
}

const serverName = remainder.slice(0, underscoreIndex)
const toolName = remainder.slice(underscoreIndex + 1)

if (!serverName || !toolName) {
return null
}

return { serverName, toolName }
}
Loading