diff --git a/src/utils/__tests__/mcp-name.spec.ts b/src/utils/__tests__/mcp-name.spec.ts index 0f3e37d575..2764c07752 100644 --- a/src/utils/__tests__/mcp-name.spec.ts +++ b/src/utils/__tests__/mcp-name.spec.ts @@ -8,9 +8,19 @@ import { MCP_TOOL_SEPARATOR, MCP_TOOL_PREFIX, HYPHEN_ENCODING, + MAX_TOOL_NAME_LENGTH, + HASH_SUFFIX_LENGTH, + mcpToolNameRegistry, + clearMcpToolNameRegistry, + computeHashSuffix, } from "../mcp-name" describe("mcp-name utilities", () => { + // Clear the registry before each test to ensure isolation + beforeEach(() => { + clearMcpToolNameRegistry() + }) + describe("constants", () => { it("should have correct separator and prefix", () => { expect(MCP_TOOL_SEPARATOR).toBe("--") @@ -20,6 +30,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 +164,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 register them", () => { + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + 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, + }) + }) + + it("should produce deterministic hash suffixes", () => { + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + // Build the same name twice + clearMcpToolNameRegistry() + const result1 = buildMcpToolName(longServer, longTool) + clearMcpToolNameRegistry() + 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 +405,119 @@ 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 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 64 chars when encoded + const serverName = "abcdefghij-kl-mnop-qrs-tuv" + const toolName = "wxyz-abcd-efghijk-lmno" + + // Build the tool name + const builtName = buildMcpToolName(serverName, toolName) + + // Should be truncated to 64 chars with hash suffix + expect(builtName.length).toBe(64) + 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" + const toolName = "another-long-tool-name-with-hyphens" + + // Build the tool name + const builtName = buildMcpToolName(serverName, toolName) + + // Should be truncated to 64 chars + expect(builtName.length).toBe(64) + + // 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" + 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 handle the registry lookup for shortened names", () => { + const serverName = "a".repeat(30) + const toolName = "b".repeat(30) + "-hyphen" + + // Build registers the shortened name + const builtName = buildMcpToolName(serverName, toolName) + + // Verify it's in the registry + expect(mcpToolNameRegistry.has(builtName)).toBe(true) + + // Parse uses the registry to get original names + const parsed = parseMcpToolName(builtName) + expect(parsed).toEqual({ + serverName: serverName, + toolName: toolName, + }) + }) + }) + + describe("clearMcpToolNameRegistry", () => { + it("should clear all registered tool names", () => { + // Register some tool names via buildMcpToolName + const longServer = "a".repeat(50) + const longTool = "b".repeat(50) + buildMcpToolName(longServer, longTool) + + expect(mcpToolNameRegistry.size).toBeGreaterThan(0) + + // Clear the registry + clearMcpToolNameRegistry() + + expect(mcpToolNameRegistry.size).toBe(0) + }) + }) }) diff --git a/src/utils/mcp-name.ts b/src/utils/mcp-name.ts index 3e6d1ab8a4..47f9fa14e1 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,50 @@ export const MCP_TOOL_PREFIX = "mcp" */ export const HYPHEN_ENCODING = "___" +/** + * Maximum length for tool names (Gemini's limit). + */ +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 + +/** + * 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 + */ +export const mcpToolNameRegistry = new Map() + +/** + * Clear the MCP tool name registry. + * Should be called when MCP servers are refreshed or disconnected. + */ +export function clearMcpToolNameRegistry(): void { + mcpToolNameRegistry.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 @@ -119,6 +165,8 @@ export function sanitizeMcpName(name: string): string { * The format is: mcp--{sanitized_server_name}--{sanitized_tool_name} * * The total length is capped at 64 characters to conform to API limits. + * When truncation is needed, a hash suffix is appended to preserve uniqueness + * and the mapping is stored in the registry for later lookup. * * @param serverName - The MCP server name * @param toolName - The tool name @@ -131,12 +179,26 @@ export function buildMcpToolName(serverName: string, toolName: string): string { // 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, return as-is + if (fullName.length <= MAX_TOOL_NAME_LENGTH) { + 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}` + + // 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 }) + + return shortenedName } /** @@ -155,6 +217,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. + * * @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 */ @@ -164,6 +229,13 @@ 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)