diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 693327a022e..96562187501 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -276,6 +276,7 @@ export async function presentAssistantMessage(cline: Task) { // Execute the MCP tool using the same handler as use_mcp_tool // Create a synthetic ToolUse block that the useMcpToolTool can handle + // Include the original encoded MCP name for lookup via encoded name comparison const syntheticToolUse: ToolUse<"use_mcp_tool"> = { type: "tool_use", id: mcpBlock.id, @@ -290,6 +291,7 @@ export async function presentAssistantMessage(cline: Task) { server_name: resolvedServerName, tool_name: mcpBlock.toolName, arguments: mcpBlock.arguments, + _encodedMcpName: mcpBlock.name, // Original encoded name for lookup via comparison }, } diff --git a/src/core/tools/UseMcpToolTool.ts b/src/core/tools/UseMcpToolTool.ts index e7ed744c78c..250778c9076 100644 --- a/src/core/tools/UseMcpToolTool.ts +++ b/src/core/tools/UseMcpToolTool.ts @@ -4,6 +4,7 @@ import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { t } from "../../i18n" import type { ToolUse } from "../../shared/tools" +import { findToolByEncodedMcpName, hasHashSuffix } from "../../utils/mcp-name" import { BaseTool, ToolCallbacks } from "./BaseTool" @@ -35,7 +36,11 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { } } - async execute(params: UseMcpToolParams, task: Task, callbacks: ToolCallbacks): Promise { + async execute( + params: UseMcpToolParams & { _encodedMcpName?: string }, + task: Task, + callbacks: ToolCallbacks, + ): Promise { const { askApproval, handleError, pushToolResult, toolProtocol } = callbacks try { @@ -48,11 +53,22 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { const { serverName, toolName, parsedArguments } = validation // Validate that the tool exists on the server - const toolValidation = await this.validateToolExists(task, serverName, toolName, pushToolResult) + // Pass the original encoded MCP name for lookup via comparison when direct lookup fails + const encodedMcpName = params._encodedMcpName + const toolValidation = await this.validateToolExists( + task, + serverName, + toolName, + pushToolResult, + encodedMcpName, + ) if (!toolValidation.isValid) { return } + // Use the resolved tool name (may differ from parsed name for shortened names) + const resolvedToolName = toolValidation.resolvedToolName || toolName + // Reset mistake count on successful validation task.consecutiveMistakeCount = 0 @@ -60,7 +76,7 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { const completeMessage = JSON.stringify({ type: "use_mcp_tool", serverName, - toolName, + toolName: resolvedToolName, arguments: params.arguments ? JSON.stringify(params.arguments) : undefined, } satisfies ClineAskUseMcpServer) @@ -75,7 +91,7 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { await this.executeToolAndProcessResult( task, serverName, - toolName, + resolvedToolName, parsedArguments, executionId, pushToolResult, @@ -156,7 +172,8 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { serverName: string, toolName: string, pushToolResult: (content: string) => void, - ): Promise<{ isValid: boolean; availableTools?: string[] }> { + encodedMcpName?: string, + ): Promise<{ isValid: boolean; availableTools?: string[]; resolvedToolName?: string }> { try { // Get the MCP hub to access server information const provider = task.providerRef.deref() @@ -205,8 +222,20 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { return { isValid: false, availableTools: [] } } - // Check if the requested tool exists - const tool = server.tools.find((tool) => tool.name === toolName) + // Check if the requested tool exists by direct name match + let tool = server.tools.find((tool) => tool.name === toolName) + let resolvedToolName = toolName + + // If direct lookup fails and we have an encoded MCP name with hash suffix, + // try to find the tool by comparing encoded names + if (!tool && encodedMcpName && hasHashSuffix(encodedMcpName)) { + const availableToolNames = server.tools.map((t) => t.name) + const matchedToolName = findToolByEncodedMcpName(serverName, encodedMcpName, availableToolNames) + if (matchedToolName) { + tool = server.tools.find((t) => t.name === matchedToolName) + resolvedToolName = matchedToolName + } + } if (!tool) { // Tool not found - provide list of available tools @@ -252,7 +281,7 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> { } // Tool exists and is enabled - return { isValid: true, availableTools: server.tools.map((tool) => tool.name) } + return { isValid: true, availableTools: server.tools.map((tool) => tool.name), resolvedToolName } } catch (error) { // If there's an error during validation, log it but don't block the tool execution // The actual tool call might still fail with a proper error diff --git a/src/shared/tools.ts b/src/shared/tools.ts index f893a3d332e..f4625919f73 100644 --- a/src/shared/tools.ts +++ b/src/shared/tools.ts @@ -108,7 +108,12 @@ export type NativeToolArgs = { search_files: { path: string; regex: string; file_pattern?: string | null } switch_mode: { mode_slug: string; reason: string } update_todo_list: { todos: string } - use_mcp_tool: { server_name: string; tool_name: string; arguments?: Record } + use_mcp_tool: { + server_name: string + tool_name: string + arguments?: Record + _encodedMcpName?: string + } write_to_file: { path: string; content: string } // Add more tools as they are migrated to native protocol } diff --git a/src/utils/__tests__/mcp-name.spec.ts b/src/utils/__tests__/mcp-name.spec.ts index 0f3e37d5750..cb25e33e344 100644 --- a/src/utils/__tests__/mcp-name.spec.ts +++ b/src/utils/__tests__/mcp-name.spec.ts @@ -8,9 +8,20 @@ import { MCP_TOOL_SEPARATOR, MCP_TOOL_PREFIX, HYPHEN_ENCODING, + MAX_TOOL_NAME_LENGTH, + HASH_SUFFIX_LENGTH, + clearEncodedNameCache, + computeHashSuffix, + findToolByEncodedMcpName, + hasHashSuffix, } from "../mcp-name" describe("mcp-name utilities", () => { + // Clear the cache before each test to ensure isolation + beforeEach(() => { + clearEncodedNameCache() + }) + describe("constants", () => { it("should have correct separator and prefix", () => { expect(MCP_TOOL_SEPARATOR).toBe("--") @@ -20,6 +31,14 @@ describe("mcp-name utilities", () => { it("should have correct hyphen encoding", () => { expect(HYPHEN_ENCODING).toBe("___") }) + + it("should have correct max tool name length", () => { + expect(MAX_TOOL_NAME_LENGTH).toBe(64) + }) + + it("should have correct hash suffix length", () => { + expect(HASH_SUFFIX_LENGTH).toBe(8) + }) }) describe("isMcpTool", () => { @@ -146,12 +165,52 @@ describe("mcp-name utilities", () => { expect(buildMcpToolName("server@name", "tool!name")).toBe("mcp--servername--toolname") }) - it("should truncate long names to 64 characters", () => { + it("should truncate long names to 64 characters with hash suffix", () => { const longServer = "a".repeat(50) const longTool = "b".repeat(50) const result = buildMcpToolName(longServer, longTool) expect(result.length).toBeLessThanOrEqual(64) + expect(result.length).toBe(64) expect(result.startsWith("mcp--")).toBe(true) + // Should end with underscore + 8 char hash suffix + expect(result).toMatch(/_[a-f0-9]{8}$/) + }) + + it("should use hash suffix for long names and cache them", () => { + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + const result = buildMcpToolName(longServer, longTool) + + // The shortened name should be deterministic + expect(result.length).toBe(64) + expect(result).toMatch(/_[a-f0-9]{8}$/) + + // Building again should return the same result (from cache) + const result2 = buildMcpToolName(longServer, longTool) + expect(result2).toBe(result) + }) + + it("should produce deterministic hash suffixes", () => { + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + // Build the same name twice with cache cleared between + clearEncodedNameCache() + const result1 = buildMcpToolName(longServer, longTool) + clearEncodedNameCache() + const result2 = buildMcpToolName(longServer, longTool) + // Should produce identical results + expect(result1).toBe(result2) + }) + + it("should produce unique hash suffixes for different tools", () => { + const longServer = "a".repeat(50) + const result1 = buildMcpToolName(longServer, "tool1_" + "x".repeat(40)) + const result2 = buildMcpToolName(longServer, "tool2_" + "y".repeat(40)) + // Both should be truncated + expect(result1.length).toBe(64) + expect(result2.length).toBe(64) + // Should have different hash suffixes + expect(result1).not.toBe(result2) }) it("should handle names starting with numbers", () => { @@ -347,4 +406,154 @@ describe("mcp-name utilities", () => { }) }) }) + + describe("computeHashSuffix", () => { + it("should compute deterministic hash for the same inputs", () => { + const hash1 = computeHashSuffix("server", "tool") + const hash2 = computeHashSuffix("server", "tool") + expect(hash1).toBe(hash2) + }) + + it("should return 8-character hex string", () => { + const hash = computeHashSuffix("server", "tool") + expect(hash).toHaveLength(8) + expect(hash).toMatch(/^[a-f0-9]{8}$/) + }) + + it("should produce different hashes for different inputs", () => { + const hash1 = computeHashSuffix("server1", "tool") + const hash2 = computeHashSuffix("server2", "tool") + expect(hash1).not.toBe(hash2) + }) + }) + + describe("hash suffix roundtrip - fixes issue #10766", () => { + it("should work correctly when names do not need truncation", () => { + // Short names that don't need truncation + const serverName = "server" + const toolName = "get-data" + + const builtName = buildMcpToolName(serverName, toolName) + + // Should NOT have hash suffix + expect(builtName).toBe("mcp--server--get___data") + expect(builtName.length).toBeLessThan(64) + + // Normal decode path should work + const parsed = parseMcpToolName(builtName) + expect(parsed).toEqual({ + serverName: "server", + toolName: "get-data", // Hyphen decoded from ___ + }) + }) + + it("should find tool by encoded name comparison for shortened names", () => { + // This is the new approach: instead of registry, compare encoded names + const serverName = "abcdefghij-kl-mnop-qrs-tuv-with-extra-long-suffix-to-exceed-limit" + const toolName = "wxyz-abcd-efghijk-lmno-plus-additional-long-suffix-here" + + // Build the encoded tool name + const encodedName = buildMcpToolName(serverName, toolName) + + // Should be truncated to 128 chars with hash suffix + expect(encodedName.length).toBe(64) + expect(encodedName).toMatch(/_[a-f0-9]{8}$/) + + // The new approach: use findToolByEncodedMcpName to find the matching tool + const availableTools = [toolName, "other-tool", "another-tool"] + const foundTool = findToolByEncodedMcpName(serverName, encodedName, availableTools) + expect(foundTool).toBe(toolName) + }) + + it("should not find tool when none matches the encoded name", () => { + const serverName = "server" + const encodedName = "mcp--server--nonexistent_tool_a1b2c3d4" + + const availableTools = ["tool1", "tool2", "tool3"] + const foundTool = findToolByEncodedMcpName(serverName, encodedName, availableTools) + expect(foundTool).toBeNull() + }) + }) + + describe("findToolByEncodedMcpName", () => { + it("should find tool by exact encoded name match", () => { + const serverName = "myserver" + const toolName = "get-data" + const encodedName = buildMcpToolName(serverName, toolName) + + const availableTools = ["other-tool", "get-data", "another-tool"] + const foundTool = findToolByEncodedMcpName(serverName, encodedName, availableTools) + expect(foundTool).toBe("get-data") + }) + + it("should find tool for shortened names with hash suffix", () => { + const serverName = "a".repeat(50) + const toolName = "b".repeat(50) + "-hyphen" + const encodedName = buildMcpToolName(serverName, toolName) + + // The encoded name should have a hash suffix + expect(hasHashSuffix(encodedName)).toBe(true) + + const availableTools = ["other-tool", toolName, "another-tool"] + const foundTool = findToolByEncodedMcpName(serverName, encodedName, availableTools) + expect(foundTool).toBe(toolName) + }) + + it("should return null when no tool matches", () => { + const serverName = "server" + const encodedName = buildMcpToolName(serverName, "nonexistent") + + const availableTools = ["tool1", "tool2"] + const foundTool = findToolByEncodedMcpName(serverName, encodedName, availableTools) + expect(foundTool).toBeNull() + }) + + it("should handle empty available tools list", () => { + const serverName = "server" + const encodedName = buildMcpToolName(serverName, "tool") + + const foundTool = findToolByEncodedMcpName(serverName, encodedName, []) + expect(foundTool).toBeNull() + }) + }) + + describe("hasHashSuffix", () => { + it("should return true for names with hash suffix", () => { + expect(hasHashSuffix("mcp--server--tool_a1b2c3d4")).toBe(true) + expect(hasHashSuffix("mcp--server--tool_12345678")).toBe(true) + expect(hasHashSuffix("mcp--server--tool_abcdef00")).toBe(true) + }) + + it("should return false for names without hash suffix", () => { + expect(hasHashSuffix("mcp--server--tool")).toBe(false) + expect(hasHashSuffix("mcp--server--tool_name")).toBe(false) + expect(hasHashSuffix("mcp--server--tool___name")).toBe(false) + }) + + it("should return false for names with partial hash patterns", () => { + // Not enough hex chars + expect(hasHashSuffix("mcp--server--tool_abc")).toBe(false) + // No underscore before hash + expect(hasHashSuffix("mcp--server--toola1b2c3d4")).toBe(false) + }) + }) + + describe("clearEncodedNameCache", () => { + it("should clear the encoded name cache", () => { + // Build some names to populate cache + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + buildMcpToolName(longServer, longTool) + buildMcpToolName("server", "tool") + + // Clear the cache + clearEncodedNameCache() + + // Verify cache is cleared by checking that rebuilding takes the same path + // (we can't directly access the cache, but the function should work) + const result = buildMcpToolName(longServer, longTool) + expect(result.length).toBe(64) + expect(result).toMatch(/_[a-f0-9]{8}$/) + }) + }) }) diff --git a/src/utils/mcp-name.ts b/src/utils/mcp-name.ts index 3e6d1ab8a47..cd1d7823134 100644 --- a/src/utils/mcp-name.ts +++ b/src/utils/mcp-name.ts @@ -3,6 +3,8 @@ * API function name requirements across all providers. */ +import * as crypto from "crypto" + /** * Separator used between MCP prefix, server name, and tool name. * We use "--" (double hyphen) because: @@ -30,6 +32,78 @@ export const MCP_TOOL_PREFIX = "mcp" */ export const HYPHEN_ENCODING = "___" +/** + * Maximum length for tool names (Gemini's function name limit). + * * The MCP spec recommends 128, but Gemini enforces 64 characters. + */ +export const MAX_TOOL_NAME_LENGTH = 64 + +/** + * Length of hash suffix used when truncation is needed. + * Using 8 characters from base36 gives us ~2.8 trillion combinations, + * which is more than enough to avoid collisions. + */ +export const HASH_SUFFIX_LENGTH = 8 + +/** + * LRU cache for encoded tool names. + * Maps "serverName:toolName" to the encoded MCP tool name. + * This avoids recomputing hash suffixes for frequently used tools. + */ +const ENCODED_NAME_CACHE_SIZE = 100 +const encodedNameCache = new Map() + +/** + * Get an encoded name from the LRU cache, updating recency. + */ +function getCachedEncodedName(key: string): string | undefined { + const value = encodedNameCache.get(key) + if (value !== undefined) { + // Move to end (most recently used) by deleting and re-adding + encodedNameCache.delete(key) + encodedNameCache.set(key, value) + } + return value +} + +/** + * Set an encoded name in the LRU cache, evicting oldest if needed. + */ +function setCachedEncodedName(key: string, value: string): void { + if (encodedNameCache.has(key)) { + encodedNameCache.delete(key) + } else if (encodedNameCache.size >= ENCODED_NAME_CACHE_SIZE) { + // Evict the oldest entry (first in iteration order) + const oldestKey = encodedNameCache.keys().next().value + if (oldestKey) { + encodedNameCache.delete(oldestKey) + } + } + encodedNameCache.set(key, value) +} + +/** + * Clear the encoded name cache. + * Exported for testing purposes. + */ +export function clearEncodedNameCache(): void { + encodedNameCache.clear() +} + +/** + * Compute a deterministic hash suffix for a given server and tool name combination. + * Uses the original (not encoded) names to ensure consistency. + * + * @param serverName - The original server name (before sanitization) + * @param toolName - The original tool name (before sanitization) + * @returns An 8-character alphanumeric hash suffix + */ +export function computeHashSuffix(serverName: string, toolName: string): string { + const hash = crypto.createHash("sha256").update(`${serverName}:${toolName}`).digest("hex") + // Use first 8 hex characters for the suffix + return hash.slice(0, HASH_SUFFIX_LENGTH) +} + /** * Normalize an MCP tool name by converting underscore separators back to hyphens. * This handles the case where models (especially Claude) convert hyphens to underscores @@ -118,25 +192,48 @@ 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 total length is capped at 64 characters to conform to API limits. + * The total length is capped at 64 characters for Gemini compatibility. + * When truncation is needed, a hash suffix is appended to preserve uniqueness. + * The result is cached for efficient repeated lookups. * * @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 { + // Check cache first + const cacheKey = `${serverName}:${toolName}` + const cached = getCachedEncodedName(cacheKey) + if (cached !== undefined) { + return cached + } + 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) + // If within limit, cache and return + if (fullName.length <= MAX_TOOL_NAME_LENGTH) { + setCachedEncodedName(cacheKey, fullName) + return fullName } - return fullName + // Need to truncate: use hash suffix to preserve uniqueness + // Format: truncated_name_HASHSUFFIX (underscore + 8 hex chars = 9 chars for suffix) + const hashSuffix = computeHashSuffix(serverName, toolName) + const suffixWithSeparator = `_${hashSuffix}` // "_" + 8 chars = 9 chars + const maxTruncatedLength = MAX_TOOL_NAME_LENGTH - suffixWithSeparator.length // 64 - 9 = 55 + + // Truncate the full name and append hash suffix + const truncatedBase = fullName.slice(0, maxTruncatedLength) + const shortenedName = `${truncatedBase}${suffixWithSeparator}` + + // Cache the result + setCachedEncodedName(cacheKey, shortenedName) + + return shortenedName } /** @@ -155,6 +252,10 @@ export function decodeMcpName(sanitizedName: string): string { * This handles sanitized names by splitting on the "--" separator * and decoding triple underscores back to hyphens. * + * Note: For shortened names (those with hash suffixes), this function + * returns the parsed names which may be truncated. Use findToolByEncodedMcpName() + * to find the correct original tool name by comparing encoded names. + * * @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 */ @@ -186,3 +287,40 @@ export function parseMcpToolName(mcpToolName: string): { serverName: string; too toolName: decodeMcpName(toolName), } } + +/** + * Find a tool by comparing encoded MCP names. + * This is used when the model returns a shortened name with a hash suffix. + * Instead of using a registry, we compare by encoding each available tool name + * and checking if it matches the encoded name from the model. + * + * @param serverName - The original server name (decoded) + * @param encodedMcpName - The full encoded MCP tool name returned by the model + * @param availableToolNames - List of available tool names on the server + * @returns The matching original tool name, or null if not found + */ +export function findToolByEncodedMcpName( + serverName: string, + encodedMcpName: string, + availableToolNames: string[], +): string | null { + for (const toolName of availableToolNames) { + const encoded = buildMcpToolName(serverName, toolName) + if (encoded === encodedMcpName) { + return toolName + } + } + return null +} + +/** + * Check if an MCP tool name appears to be a shortened name with a hash suffix. + * Shortened names have the pattern: ...._XXXXXXXX where X is a hex character. + * + * @param mcpToolName - The MCP tool name to check + * @returns true if the name appears to have a hash suffix + */ +export function hasHashSuffix(mcpToolName: string): boolean { + // Check for pattern: ends with underscore + 8 hex characters + return /_.{8}$/.test(mcpToolName) && /[a-f0-9]{8}$/.test(mcpToolName) +}