diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 693327a022..9656218750 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 e7ed744c78..250778c907 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 f893a3d332..f4625919f7 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 c20e40a4fe..9b6eac6d0c 100644 --- a/src/utils/__tests__/mcp-name.spec.ts +++ b/src/utils/__tests__/mcp-name.spec.ts @@ -10,15 +10,16 @@ import { HYPHEN_ENCODING, MAX_TOOL_NAME_LENGTH, HASH_SUFFIX_LENGTH, - mcpToolNameRegistry, - clearMcpToolNameRegistry, + clearEncodedNameCache, computeHashSuffix, + findToolByEncodedMcpName, + hasHashSuffix, } from "../mcp-name" describe("mcp-name utilities", () => { - // Clear the registry before each test to ensure isolation + // Clear the cache before each test to ensure isolation beforeEach(() => { - clearMcpToolNameRegistry() + clearEncodedNameCache() }) describe("constants", () => { @@ -175,27 +176,27 @@ describe("mcp-name utilities", () => { expect(result).toMatch(/_[a-f0-9]{8}$/) }) - it("should use hash suffix for long names and register them", () => { + it("should use hash suffix for long names and cache them", () => { const longServer = "a".repeat(80) const longTool = "b".repeat(80) const result = buildMcpToolName(longServer, longTool) - // The shortened name should be registered - expect(mcpToolNameRegistry.has(result)).toBe(true) - const registered = mcpToolNameRegistry.get(result) - expect(registered).toEqual({ - serverName: longServer, - toolName: longTool, - }) + // The shortened name should be deterministic + expect(result.length).toBe(128) + 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(80) const longTool = "b".repeat(80) - // Build the same name twice - clearMcpToolNameRegistry() + // Build the same name twice with cache cleared between + clearEncodedNameCache() const result1 = buildMcpToolName(longServer, longTool) - clearMcpToolNameRegistry() + clearEncodedNameCache() const result2 = buildMcpToolName(longServer, longTool) // Should produce identical results expect(result1).toBe(result2) @@ -427,46 +428,6 @@ describe("mcp-name utilities", () => { }) describe("hash suffix roundtrip - fixes issue #10766", () => { - it("should preserve original names through roundtrip with long hyphenated tool names", () => { - // This is the exact scenario from issue #10766 - // Tool name with many hyphens that exceeds 128 chars when encoded - 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 tool name - const builtName = buildMcpToolName(serverName, toolName) - - // Should be truncated to 128 chars with hash suffix - expect(builtName.length).toBe(128) - expect(builtName).toMatch(/_[a-f0-9]{8}$/) - - // The critical fix: parsing should return the ORIGINAL names - const parsed = parseMcpToolName(builtName) - expect(parsed).toEqual({ - serverName: serverName, // Original with hyphens! - toolName: toolName, // Original with hyphens! - }) - }) - - it("should not corrupt hyphen encoding when truncation is needed", () => { - // Long server and tool names that would cause truncation mid-encoding - const serverName = "very-long-server-name-with-many-hyphens-and-extra-content-to-exceed" - const toolName = "another-long-tool-name-with-hyphens-and-extra-content-to-exceed" - - // Build the tool name - const builtName = buildMcpToolName(serverName, toolName) - - // Should be truncated to 128 chars - expect(builtName.length).toBe(128) - - // Parse should return original names (via registry lookup) - const parsed = parseMcpToolName(builtName) - expect(parsed).toEqual({ - serverName: serverName, - toolName: toolName, - }) - }) - it("should work correctly when names do not need truncation", () => { // Short names that don't need truncation const serverName = "server" @@ -486,38 +447,113 @@ describe("mcp-name utilities", () => { }) }) - it("should handle the registry lookup for shortened names", () => { - const serverName = "a".repeat(80) - const toolName = "b".repeat(80) + "-hyphen" + 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 registers the shortened name - const builtName = buildMcpToolName(serverName, toolName) + // Build the encoded tool name + const encodedName = buildMcpToolName(serverName, toolName) - // Verify it's in the registry - expect(mcpToolNameRegistry.has(builtName)).toBe(true) + // Should be truncated to 128 chars with hash suffix + expect(encodedName.length).toBe(128) + expect(encodedName).toMatch(/_[a-f0-9]{8}$/) - // Parse uses the registry to get original names - const parsed = parseMcpToolName(builtName) - expect(parsed).toEqual({ - serverName: serverName, - toolName: toolName, - }) + // 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("clearMcpToolNameRegistry", () => { - it("should clear all registered tool names", () => { - // Register some tool names via buildMcpToolName + 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(80) + const toolName = "b".repeat(80) + "-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(80) const longTool = "b".repeat(80) buildMcpToolName(longServer, longTool) + buildMcpToolName("server", "tool") - expect(mcpToolNameRegistry.size).toBeGreaterThan(0) + // Clear the cache + clearEncodedNameCache() - // Clear the registry - clearMcpToolNameRegistry() - - expect(mcpToolNameRegistry.size).toBe(0) + // 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(128) + expect(result).toMatch(/_[a-f0-9]{8}$/) }) }) }) diff --git a/src/utils/mcp-name.ts b/src/utils/mcp-name.ts index 25e90f6896..57ff975677 100644 --- a/src/utils/mcp-name.ts +++ b/src/utils/mcp-name.ts @@ -46,21 +46,48 @@ export const MAX_TOOL_NAME_LENGTH = 128 export const HASH_SUFFIX_LENGTH = 8 /** - * Registry mapping shortened tool names (with hash suffix) to their original - * server and tool names. This is used to look up the original names when - * the model returns a shortened tool name. - * - * Key: shortened MCP tool name (e.g., "mcp--server--tool_a1b2c3d4") - * Value: { serverName, toolName } with original (decoded) names + * LRU cache for encoded tool names. + * Maps "serverName:toolName" to the encoded MCP tool name. + * This avoids recomputing hash suffixes for frequently used tools. */ -export const mcpToolNameRegistry = new Map() +const ENCODED_NAME_CACHE_SIZE = 100 +const encodedNameCache = new Map() /** - * Clear the MCP tool name registry. - * Should be called when MCP servers are refreshed or disconnected. + * Get an encoded name from the LRU cache, updating recency. */ -export function clearMcpToolNameRegistry(): void { - mcpToolNameRegistry.clear() +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() } /** @@ -166,22 +193,30 @@ export function sanitizeMcpName(name: string): string { * The format is: mcp--{sanitized_server_name}--{sanitized_tool_name} * * The total length is capped at 128 characters per MCP spec. - * When truncation is needed, a hash suffix is appended to preserve uniqueness - * and the mapping is stored in the registry for later lookup. + * 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}` - // If within limit, return as-is + // If within limit, cache and return if (fullName.length <= MAX_TOOL_NAME_LENGTH) { + setCachedEncodedName(cacheKey, fullName) return fullName } @@ -195,9 +230,8 @@ export function buildMcpToolName(serverName: string, toolName: string): string { const truncatedBase = fullName.slice(0, maxTruncatedLength) const shortenedName = `${truncatedBase}${suffixWithSeparator}` - // Register the mapping from shortened name to original names - // Store the original (decoded) names so parseMcpToolName can return them directly - mcpToolNameRegistry.set(shortenedName, { serverName, toolName }) + // Cache the result + setCachedEncodedName(cacheKey, shortenedName) return shortenedName } @@ -218,8 +252,9 @@ export function decodeMcpName(sanitizedName: string): string { * This handles sanitized names by splitting on the "--" separator * and decoding triple underscores back to hyphens. * - * For shortened names (those with hash suffixes), this first checks - * the registry for the original names. + * 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 @@ -230,13 +265,6 @@ export function parseMcpToolName(mcpToolName: string): { serverName: string; too return null } - // First, check if this is a shortened name in the registry - // This handles names that were truncated with hash suffixes - const registeredName = mcpToolNameRegistry.get(mcpToolName) - if (registeredName) { - return registeredName - } - // Remove the "mcp--" prefix const remainder = mcpToolName.slice(prefix.length) @@ -259,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) +}