mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-11 22:51:26 +00:00
fix: use hash suffix instead of truncation for long MCP tool names
Addresses Issue #10766 where MCP tool names with hyphens fail when using native tool calling due to truncation cutting in the middle of hyphen encodings (___). Changes: - Add hash suffix (8 hex chars) when tool names exceed 64 characters instead of simple truncation - Register shortened names in a registry to allow lookup of original server/tool names - Parse function now checks registry first for shortened names - Add comprehensive tests for hash suffix behavior and roundtrip This ensures that long tool names with hyphens can be correctly resolved even after shortening, preserving the original tool identity.
This commit is contained in:
Roo Code
parent
8b9f02aa0d
commit
a1cfe4cef3
2 changed files with 250 additions and 5 deletions
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string, { serverName: string; toolName: string }>()
|
||||
|
||||
/**
|
||||
* 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)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue