mirror of
https://github.com/CherryHQ/cherry-studio.git
synced 2025-12-30 15:59:09 +08:00
refactor: simplify buildFunctionCallToolName to use mcp__{server}__{tool} format (#12186)
This commit is contained in:
LiuVaayne
GitHub
parent
efbe64e5da
commit
528d6d37f2
@ -785,7 +785,7 @@ class McpService {
|
||||
...tool,
|
||||
inputSchema: z.parse(MCPToolInputSchema, tool.inputSchema),
|
||||
outputSchema: tool.outputSchema ? z.parse(MCPToolOutputSchema, tool.outputSchema) : undefined,
|
||||
id: buildFunctionCallToolName(server.name, tool.name, server.id),
|
||||
id: buildFunctionCallToolName(server.name, tool.name),
|
||||
serverId: server.id,
|
||||
serverName: server.name,
|
||||
type: 'mcp'
|
||||
|
||||
@ -3,194 +3,223 @@ import { describe, expect, it } from 'vitest'
|
||||
import { buildFunctionCallToolName } from '../mcp'
|
||||
|
||||
describe('buildFunctionCallToolName', () => {
|
||||
describe('basic functionality', () => {
|
||||
it('should combine server name and tool name', () => {
|
||||
describe('basic format', () => {
|
||||
it('should return format mcp__{server}__{tool}', () => {
|
||||
const result = buildFunctionCallToolName('github', 'search_issues')
|
||||
expect(result).toContain('github')
|
||||
expect(result).toContain('search')
|
||||
expect(result).toBe('mcp__github__search_issues')
|
||||
})
|
||||
|
||||
it('should sanitize names by replacing dashes with underscores', () => {
|
||||
const result = buildFunctionCallToolName('my-server', 'my-tool')
|
||||
// Input dashes are replaced, but the separator between server and tool is a dash
|
||||
expect(result).toBe('my_serv-my_tool')
|
||||
expect(result).toContain('_')
|
||||
})
|
||||
|
||||
it('should handle empty server names gracefully', () => {
|
||||
const result = buildFunctionCallToolName('', 'tool')
|
||||
expect(result).toBeTruthy()
|
||||
it('should handle simple server and tool names', () => {
|
||||
expect(buildFunctionCallToolName('fetch', 'get_page')).toBe('mcp__fetch__get_page')
|
||||
expect(buildFunctionCallToolName('database', 'query')).toBe('mcp__database__query')
|
||||
expect(buildFunctionCallToolName('cherry_studio', 'search')).toBe('mcp__cherry_studio__search')
|
||||
})
|
||||
})
|
||||
|
||||
describe('uniqueness with serverId', () => {
|
||||
it('should generate different IDs for same server name but different serverIds', () => {
|
||||
const serverId1 = 'server-id-123456'
|
||||
const serverId2 = 'server-id-789012'
|
||||
const serverName = 'github'
|
||||
const toolName = 'search_repos'
|
||||
|
||||
const result1 = buildFunctionCallToolName(serverName, toolName, serverId1)
|
||||
const result2 = buildFunctionCallToolName(serverName, toolName, serverId2)
|
||||
|
||||
expect(result1).not.toBe(result2)
|
||||
expect(result1).toContain('123456')
|
||||
expect(result2).toContain('789012')
|
||||
describe('valid JavaScript identifier', () => {
|
||||
it('should always start with mcp__ prefix (valid JS identifier start)', () => {
|
||||
const result = buildFunctionCallToolName('123server', '456tool')
|
||||
expect(result).toMatch(/^mcp__/)
|
||||
expect(result).toBe('mcp__123server__456tool')
|
||||
})
|
||||
|
||||
it('should generate same ID when serverId is not provided', () => {
|
||||
it('should only contain alphanumeric chars and underscores', () => {
|
||||
const result = buildFunctionCallToolName('my-server', 'my-tool')
|
||||
expect(result).toBe('mcp__my_server__my_tool')
|
||||
expect(result).toMatch(/^[a-zA-Z][a-zA-Z0-9_]*$/)
|
||||
})
|
||||
|
||||
it('should be a valid JavaScript identifier', () => {
|
||||
const testCases = [
|
||||
['github', 'create_issue'],
|
||||
['my-server', 'fetch-data'],
|
||||
['test@server', 'tool#name'],
|
||||
['server.name', 'tool.action'],
|
||||
['123abc', 'def456']
|
||||
]
|
||||
|
||||
for (const [server, tool] of testCases) {
|
||||
const result = buildFunctionCallToolName(server, tool)
|
||||
// Valid JS identifiers match this pattern
|
||||
expect(result).toMatch(/^[a-zA-Z_][a-zA-Z0-9_]*$/)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('character sanitization', () => {
|
||||
it('should replace dashes with underscores', () => {
|
||||
const result = buildFunctionCallToolName('my-server', 'my-tool-name')
|
||||
expect(result).toBe('mcp__my_server__my_tool_name')
|
||||
})
|
||||
|
||||
it('should replace special characters with underscores', () => {
|
||||
const result = buildFunctionCallToolName('test@server!', 'tool#name$')
|
||||
expect(result).toBe('mcp__test_server__tool_name')
|
||||
})
|
||||
|
||||
it('should replace dots with underscores', () => {
|
||||
const result = buildFunctionCallToolName('server.name', 'tool.action')
|
||||
expect(result).toBe('mcp__server_name__tool_action')
|
||||
})
|
||||
|
||||
it('should replace spaces with underscores', () => {
|
||||
const result = buildFunctionCallToolName('my server', 'my tool')
|
||||
expect(result).toBe('mcp__my_server__my_tool')
|
||||
})
|
||||
|
||||
it('should collapse consecutive underscores', () => {
|
||||
const result = buildFunctionCallToolName('my--server', 'my___tool')
|
||||
expect(result).toBe('mcp__my_server__my_tool')
|
||||
expect(result).not.toMatch(/_{3,}/)
|
||||
})
|
||||
|
||||
it('should trim leading and trailing underscores from parts', () => {
|
||||
const result = buildFunctionCallToolName('_server_', '_tool_')
|
||||
expect(result).toBe('mcp__server__tool')
|
||||
})
|
||||
|
||||
it('should handle names with only special characters', () => {
|
||||
const result = buildFunctionCallToolName('---', '###')
|
||||
expect(result).toBe('mcp____')
|
||||
})
|
||||
})
|
||||
|
||||
describe('length constraints', () => {
|
||||
it('should not exceed 63 characters', () => {
|
||||
const longServerName = 'a'.repeat(50)
|
||||
const longToolName = 'b'.repeat(50)
|
||||
const result = buildFunctionCallToolName(longServerName, longToolName)
|
||||
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
})
|
||||
|
||||
it('should truncate server name to max 20 chars', () => {
|
||||
const longServerName = 'abcdefghijklmnopqrstuvwxyz' // 26 chars
|
||||
const result = buildFunctionCallToolName(longServerName, 'tool')
|
||||
|
||||
expect(result).toBe('mcp__abcdefghijklmnopqrst__tool')
|
||||
expect(result).toContain('abcdefghijklmnopqrst') // First 20 chars
|
||||
expect(result).not.toContain('uvwxyz') // Truncated
|
||||
})
|
||||
|
||||
it('should truncate tool name to max 35 chars', () => {
|
||||
const longToolName = 'a'.repeat(40)
|
||||
const result = buildFunctionCallToolName('server', longToolName)
|
||||
|
||||
const expectedTool = 'a'.repeat(35)
|
||||
expect(result).toBe(`mcp__server__${expectedTool}`)
|
||||
})
|
||||
|
||||
it('should not end with underscores after truncation', () => {
|
||||
// Create a name that would end with underscores after truncation
|
||||
const longServerName = 'a'.repeat(20)
|
||||
const longToolName = 'b'.repeat(35) + '___extra'
|
||||
const result = buildFunctionCallToolName(longServerName, longToolName)
|
||||
|
||||
expect(result).not.toMatch(/_+$/)
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
})
|
||||
|
||||
it('should handle max length edge case exactly', () => {
|
||||
// mcp__ (5) + server (20) + __ (2) + tool (35) = 62 chars
|
||||
const server = 'a'.repeat(20)
|
||||
const tool = 'b'.repeat(35)
|
||||
const result = buildFunctionCallToolName(server, tool)
|
||||
|
||||
expect(result.length).toBe(62)
|
||||
expect(result).toBe(`mcp__${'a'.repeat(20)}__${'b'.repeat(35)}`)
|
||||
})
|
||||
})
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should handle empty server name', () => {
|
||||
const result = buildFunctionCallToolName('', 'tool')
|
||||
expect(result).toBe('mcp____tool')
|
||||
})
|
||||
|
||||
it('should handle empty tool name', () => {
|
||||
const result = buildFunctionCallToolName('server', '')
|
||||
expect(result).toBe('mcp__server__')
|
||||
})
|
||||
|
||||
it('should handle both empty names', () => {
|
||||
const result = buildFunctionCallToolName('', '')
|
||||
expect(result).toBe('mcp____')
|
||||
})
|
||||
|
||||
it('should handle whitespace-only names', () => {
|
||||
const result = buildFunctionCallToolName(' ', ' ')
|
||||
expect(result).toBe('mcp____')
|
||||
})
|
||||
|
||||
it('should trim whitespace from names', () => {
|
||||
const result = buildFunctionCallToolName(' server ', ' tool ')
|
||||
expect(result).toBe('mcp__server__tool')
|
||||
})
|
||||
|
||||
it('should handle unicode characters', () => {
|
||||
const result = buildFunctionCallToolName('服务器', '工具')
|
||||
// Unicode chars are replaced with underscores, then collapsed
|
||||
expect(result).toMatch(/^mcp__/)
|
||||
})
|
||||
|
||||
it('should handle mixed case', () => {
|
||||
const result = buildFunctionCallToolName('MyServer', 'MyTool')
|
||||
expect(result).toBe('mcp__MyServer__MyTool')
|
||||
})
|
||||
})
|
||||
|
||||
describe('deterministic output', () => {
|
||||
it('should produce consistent results for same input', () => {
|
||||
const serverName = 'github'
|
||||
const toolName = 'search_repos'
|
||||
|
||||
const result1 = buildFunctionCallToolName(serverName, toolName)
|
||||
const result2 = buildFunctionCallToolName(serverName, toolName)
|
||||
const result3 = buildFunctionCallToolName(serverName, toolName)
|
||||
|
||||
expect(result1).toBe(result2)
|
||||
expect(result2).toBe(result3)
|
||||
})
|
||||
|
||||
it('should include serverId suffix when provided', () => {
|
||||
const serverId = 'abc123def456'
|
||||
const result = buildFunctionCallToolName('server', 'tool', serverId)
|
||||
it('should produce different results for different inputs', () => {
|
||||
const result1 = buildFunctionCallToolName('server1', 'tool')
|
||||
const result2 = buildFunctionCallToolName('server2', 'tool')
|
||||
const result3 = buildFunctionCallToolName('server', 'tool1')
|
||||
const result4 = buildFunctionCallToolName('server', 'tool2')
|
||||
|
||||
// Should include last 6 chars of serverId
|
||||
expect(result).toContain('ef456')
|
||||
})
|
||||
})
|
||||
|
||||
describe('character sanitization', () => {
|
||||
it('should replace invalid characters with underscores', () => {
|
||||
const result = buildFunctionCallToolName('test@server', 'tool#name')
|
||||
expect(result).not.toMatch(/[@#]/)
|
||||
expect(result).toMatch(/^[a-zA-Z0-9_-]+$/)
|
||||
})
|
||||
|
||||
it('should ensure name starts with a letter', () => {
|
||||
const result = buildFunctionCallToolName('123server', '456tool')
|
||||
expect(result).toMatch(/^[a-zA-Z]/)
|
||||
})
|
||||
|
||||
it('should handle consecutive underscores/dashes', () => {
|
||||
const result = buildFunctionCallToolName('my--server', 'my__tool')
|
||||
expect(result).not.toMatch(/[_-]{2,}/)
|
||||
})
|
||||
})
|
||||
|
||||
describe('length constraints', () => {
|
||||
it('should truncate names longer than 63 characters', () => {
|
||||
const longServerName = 'a'.repeat(50)
|
||||
const longToolName = 'b'.repeat(50)
|
||||
const result = buildFunctionCallToolName(longServerName, longToolName, 'id123456')
|
||||
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
})
|
||||
|
||||
it('should not end with underscore or dash after truncation', () => {
|
||||
const longServerName = 'a'.repeat(50)
|
||||
const longToolName = 'b'.repeat(50)
|
||||
const result = buildFunctionCallToolName(longServerName, longToolName, 'id123456')
|
||||
|
||||
expect(result).not.toMatch(/[_-]$/)
|
||||
})
|
||||
|
||||
it('should preserve serverId suffix even with long server/tool names', () => {
|
||||
const longServerName = 'a'.repeat(50)
|
||||
const longToolName = 'b'.repeat(50)
|
||||
const serverId = 'server-id-xyz789'
|
||||
|
||||
const result = buildFunctionCallToolName(longServerName, longToolName, serverId)
|
||||
|
||||
// The suffix should be preserved and not truncated
|
||||
expect(result).toContain('xyz789')
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
})
|
||||
|
||||
it('should ensure two long-named servers with different IDs produce different results', () => {
|
||||
const longServerName = 'a'.repeat(50)
|
||||
const longToolName = 'b'.repeat(50)
|
||||
const serverId1 = 'server-id-abc123'
|
||||
const serverId2 = 'server-id-def456'
|
||||
|
||||
const result1 = buildFunctionCallToolName(longServerName, longToolName, serverId1)
|
||||
const result2 = buildFunctionCallToolName(longServerName, longToolName, serverId2)
|
||||
|
||||
// Both should be within limit
|
||||
expect(result1.length).toBeLessThanOrEqual(63)
|
||||
expect(result2.length).toBeLessThanOrEqual(63)
|
||||
|
||||
// They should be different due to preserved suffix
|
||||
expect(result1).not.toBe(result2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('edge cases with serverId', () => {
|
||||
it('should handle serverId with only non-alphanumeric characters', () => {
|
||||
const serverId = '------' // All dashes
|
||||
const result = buildFunctionCallToolName('server', 'tool', serverId)
|
||||
|
||||
// Should still produce a valid unique suffix via fallback hash
|
||||
expect(result).toBeTruthy()
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
expect(result).toMatch(/^[a-zA-Z][a-zA-Z0-9_-]*$/)
|
||||
// Should have a suffix (underscore followed by something)
|
||||
expect(result).toMatch(/_[a-z0-9]+$/)
|
||||
})
|
||||
|
||||
it('should produce different results for different non-alphanumeric serverIds', () => {
|
||||
const serverId1 = '------'
|
||||
const serverId2 = '!!!!!!'
|
||||
|
||||
const result1 = buildFunctionCallToolName('server', 'tool', serverId1)
|
||||
const result2 = buildFunctionCallToolName('server', 'tool', serverId2)
|
||||
|
||||
// Should be different because the hash fallback produces different values
|
||||
expect(result1).not.toBe(result2)
|
||||
})
|
||||
|
||||
it('should handle empty string serverId differently from undefined', () => {
|
||||
const resultWithEmpty = buildFunctionCallToolName('server', 'tool', '')
|
||||
const resultWithUndefined = buildFunctionCallToolName('server', 'tool', undefined)
|
||||
|
||||
// Empty string is falsy, so both should behave the same (no suffix)
|
||||
expect(resultWithEmpty).toBe(resultWithUndefined)
|
||||
})
|
||||
|
||||
it('should handle serverId with mixed alphanumeric and special chars', () => {
|
||||
const serverId = 'ab@#cd' // Mixed chars, last 6 chars contain some alphanumeric
|
||||
const result = buildFunctionCallToolName('server', 'tool', serverId)
|
||||
|
||||
// Should extract alphanumeric chars: 'abcd' from 'ab@#cd'
|
||||
expect(result).toContain('abcd')
|
||||
expect(result3).not.toBe(result4)
|
||||
})
|
||||
})
|
||||
|
||||
describe('real-world scenarios', () => {
|
||||
it('should handle GitHub MCP server instances correctly', () => {
|
||||
const serverName = 'github'
|
||||
const toolName = 'search_repositories'
|
||||
|
||||
const githubComId = 'server-github-com-abc123'
|
||||
const gheId = 'server-ghe-internal-xyz789'
|
||||
|
||||
const tool1 = buildFunctionCallToolName(serverName, toolName, githubComId)
|
||||
const tool2 = buildFunctionCallToolName(serverName, toolName, gheId)
|
||||
|
||||
// Should be different
|
||||
expect(tool1).not.toBe(tool2)
|
||||
|
||||
// Both should be valid identifiers
|
||||
expect(tool1).toMatch(/^[a-zA-Z][a-zA-Z0-9_-]*$/)
|
||||
expect(tool2).toMatch(/^[a-zA-Z][a-zA-Z0-9_-]*$/)
|
||||
|
||||
// Both should be <= 63 chars
|
||||
expect(tool1.length).toBeLessThanOrEqual(63)
|
||||
expect(tool2.length).toBeLessThanOrEqual(63)
|
||||
it('should handle GitHub MCP server', () => {
|
||||
expect(buildFunctionCallToolName('github', 'create_issue')).toBe('mcp__github__create_issue')
|
||||
expect(buildFunctionCallToolName('github', 'search_repositories')).toBe('mcp__github__search_repositories')
|
||||
expect(buildFunctionCallToolName('github', 'get_pull_request')).toBe('mcp__github__get_pull_request')
|
||||
})
|
||||
|
||||
it('should handle tool names that already include server name prefix', () => {
|
||||
const result = buildFunctionCallToolName('github', 'github_search_repos')
|
||||
expect(result).toBeTruthy()
|
||||
// Should not double the server name
|
||||
expect(result.split('github').length - 1).toBeLessThanOrEqual(2)
|
||||
it('should handle filesystem MCP server', () => {
|
||||
expect(buildFunctionCallToolName('filesystem', 'read_file')).toBe('mcp__filesystem__read_file')
|
||||
expect(buildFunctionCallToolName('filesystem', 'write_file')).toBe('mcp__filesystem__write_file')
|
||||
expect(buildFunctionCallToolName('filesystem', 'list_directory')).toBe('mcp__filesystem__list_directory')
|
||||
})
|
||||
|
||||
it('should handle hyphenated server names (common in npm packages)', () => {
|
||||
expect(buildFunctionCallToolName('cherry-fetch', 'get_page')).toBe('mcp__cherry_fetch__get_page')
|
||||
expect(buildFunctionCallToolName('mcp-server-github', 'search')).toBe('mcp__mcp_server_github__search')
|
||||
})
|
||||
|
||||
it('should handle scoped npm package style names', () => {
|
||||
const result = buildFunctionCallToolName('@anthropic/mcp-server', 'chat')
|
||||
expect(result).toBe('mcp__anthropic_mcp_server__chat')
|
||||
})
|
||||
|
||||
it('should handle tools with long descriptive names', () => {
|
||||
const result = buildFunctionCallToolName('github', 'search_repositories_by_language_and_stars')
|
||||
expect(result.length).toBeLessThanOrEqual(63)
|
||||
expect(result).toMatch(/^mcp__github__search_repositories_by_lan/)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -1,56 +1,28 @@
|
||||
export function buildFunctionCallToolName(serverName: string, toolName: string, serverId?: string) {
|
||||
const sanitizedServer = serverName.trim().replace(/-/g, '_')
|
||||
const sanitizedTool = toolName.trim().replace(/-/g, '_')
|
||||
/**
|
||||
* Builds a valid JavaScript function name for MCP tool calls.
|
||||
* Format: mcp__{server_name}__{tool_name}
|
||||
*
|
||||
* @param serverName - The MCP server name
|
||||
* @param toolName - The tool name from the server
|
||||
* @returns A valid JS identifier in format mcp__{server}__{tool}, max 63 chars
|
||||
*/
|
||||
export function buildFunctionCallToolName(serverName: string, toolName: string): string {
|
||||
// Sanitize to valid JS identifier chars (alphanumeric + underscore only)
|
||||
const sanitize = (str: string): string =>
|
||||
str
|
||||
.trim()
|
||||
.replace(/[^a-zA-Z0-9]/g, '_') // Replace all non-alphanumeric with underscore
|
||||
.replace(/_{2,}/g, '_') // Collapse multiple underscores
|
||||
.replace(/^_+|_+$/g, '') // Trim leading/trailing underscores
|
||||
|
||||
// Calculate suffix first to reserve space for it
|
||||
// Suffix format: "_" + 6 alphanumeric chars = 7 chars total
|
||||
let serverIdSuffix = ''
|
||||
if (serverId) {
|
||||
// Take the last 6 characters of the serverId for brevity
|
||||
serverIdSuffix = serverId.slice(-6).replace(/[^a-zA-Z0-9]/g, '')
|
||||
const server = sanitize(serverName).slice(0, 20) // Keep server name short
|
||||
const tool = sanitize(toolName).slice(0, 35) // More room for tool name
|
||||
|
||||
// Fallback: if suffix becomes empty (all non-alphanumeric chars), use a simple hash
|
||||
if (!serverIdSuffix) {
|
||||
const hash = serverId.split('').reduce((acc, char) => acc + char.charCodeAt(0), 0)
|
||||
serverIdSuffix = hash.toString(36).slice(-6) || 'x'
|
||||
}
|
||||
}
|
||||
let name = `mcp__${server}__${tool}`
|
||||
|
||||
// Reserve space for suffix when calculating max base name length
|
||||
const SUFFIX_LENGTH = serverIdSuffix ? serverIdSuffix.length + 1 : 0 // +1 for underscore
|
||||
const MAX_BASE_LENGTH = 63 - SUFFIX_LENGTH
|
||||
|
||||
// Combine server name and tool name
|
||||
let name = sanitizedTool
|
||||
if (!sanitizedTool.includes(sanitizedServer.slice(0, 7))) {
|
||||
name = `${sanitizedServer.slice(0, 7) || ''}-${sanitizedTool || ''}`
|
||||
}
|
||||
|
||||
// Replace invalid characters with underscores or dashes
|
||||
// Keep a-z, A-Z, 0-9, underscores and dashes
|
||||
name = name.replace(/[^a-zA-Z0-9_-]/g, '_')
|
||||
|
||||
// Ensure name starts with a letter or underscore (for valid JavaScript identifier)
|
||||
if (!/^[a-zA-Z]/.test(name)) {
|
||||
name = `tool-${name}`
|
||||
}
|
||||
|
||||
// Remove consecutive underscores/dashes (optional improvement)
|
||||
name = name.replace(/[_-]{2,}/g, '_')
|
||||
|
||||
// Truncate base name BEFORE adding suffix to ensure suffix is never cut off
|
||||
if (name.length > MAX_BASE_LENGTH) {
|
||||
name = name.slice(0, MAX_BASE_LENGTH)
|
||||
}
|
||||
|
||||
// Handle edge case: ensure we still have a valid name if truncation left invalid chars at edges
|
||||
if (name.endsWith('_') || name.endsWith('-')) {
|
||||
name = name.slice(0, -1)
|
||||
}
|
||||
|
||||
// Now append the suffix - it will always fit within 63 chars
|
||||
if (serverIdSuffix) {
|
||||
name = `${name}_${serverIdSuffix}`
|
||||
// Ensure max 63 chars and clean trailing underscores
|
||||
if (name.length > 63) {
|
||||
name = name.slice(0, 63).replace(/_+$/, '')
|
||||
}
|
||||
|
||||
return name
|
||||
|
||||
Loading…
Reference in New Issue
Block a user