Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
28 changes: 12 additions & 16 deletions src/core/assistant-message/NativeToolCallParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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.
*
Expand All @@ -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,
Expand Down
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
29 changes: 29 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,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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
178 changes: 178 additions & 0 deletions src/utils/__tests__/mcp-name.spec.ts
Original file line number Diff line number Diff line change
@@ -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",
})
})
})
})
Loading
Loading