Sanitize MCP tool names for model providers

Author: jsourcebot

packages/web/src/ee/features/chat/mcp/mcpToolSets.test.ts

@@ -101,7 +101,7 @@ function createMockClient(overrides: Partial<McpToolSet> & { serverName: string
 // --- Tests ---
 
 // Import after mocks are set up
-const { getMcpTools } = await import('./mcpToolSets');
+const { getMcpTools, sanitizeMcpToolNameForModel } = await import('./mcpToolSets');
 
 beforeEach(() => {
     vi.clearAllMocks();
@@ -114,6 +114,17 @@ beforeEach(() => {
     mockRedisSet.mockResolvedValue('OK');
 });
 
+describe('sanitizeMcpToolNameForModel', () => {
+    test('replaces provider-invalid characters with underscores', () => {
+        expect(sanitizeMcpToolNameForModel('mcp_backstage__catalog.query-catalog-entities'))
+            .toBe('mcp_backstage__catalog_query_catalog_entities');
+    });
+
+    test('returns an underscore for an empty name', () => {
+        expect(sanitizeMcpToolNameForModel('')).toBe('_');
+    });
+});
+
 describe('getMcpTools', () => {
     test('single server with single tool produces correctly namespaced key', async () => {
         const mockClient = createMockMcpClient([
@@ -129,6 +140,58 @@ describe('getMcpTools', () => {
         expect(result.failedServers).toEqual([]);
     });
 
+    test('sanitizes model-facing keys for MCP tools with punctuation', async () => {
+        const mockClient = createMockMcpClient([
+            { name: 'catalog.query-catalog-entities', description: 'Query catalog entities' },
+        ]);
+        mockCreateMCPClient.mockResolvedValue(mockClient);
+
+        const result = await getMcpTools([
+            createMockClient({ serverId: 'server-backstage', serverName: 'Backstage' }),
+        ]);
+
+        expect(Object.keys(result.tools)).toEqual([
+            'mcp_backstage__catalog_query_catalog_entities',
+        ]);
+
+        const tool = result.tools['mcp_backstage__catalog_query_catalog_entities'];
+        await expect(
+            tool.execute({ filter: 'kind=component' }, { messages: [], toolCallId: 'test' })
+        ).resolves.toEqual({ content: [{ type: 'text', text: 'result' }] });
+
+        expect(mockServerToolUpsert).toHaveBeenCalledWith(expect.objectContaining({
+            where: {
+                mcpServerId_toolName: {
+                    mcpServerId: 'server-backstage',
+                    toolName: 'catalog.query-catalog-entities',
+                },
+            },
+        }));
+        expect(mockCaptureEvent).toHaveBeenCalledWith('ask_mcp_tool_call_completed', expect.objectContaining({
+            serverId: 'server-backstage',
+            toolName: 'catalog.query-catalog-entities',
+            success: true,
+        }));
+    });
+
+    test('adds stable suffixes when sanitized tool names collide', async () => {
+        const mockClient = createMockMcpClient([
+            { name: 'catalog.query', description: 'Query catalog' },
+            { name: 'catalog_query', description: 'Query catalog with underscore' },
+        ]);
+        mockCreateMCPClient.mockResolvedValue(mockClient);
+
+        const result = await getMcpTools([
+            createMockClient({ serverName: 'Backstage' }),
+        ]);
+
+        const toolNames = Object.keys(result.tools);
+        expect(toolNames).toHaveLength(2);
+        expect(new Set(toolNames).size).toBe(2);
+        expect(toolNames).not.toContain('mcp_backstage__catalog_query');
+        expect(toolNames.every((toolName) => /^mcp_backstage__catalog_query_[0-9a-f]{8}$/.test(toolName))).toBe(true);
+    });
+
     test('multiple servers produce tools with distinct prefixes', async () => {
         const linearClient = createMockMcpClient([
             { name: 'list_issues', description: 'List issues' },

packages/web/src/ee/features/chat/mcp/mcpToolSets.ts

@@ -113,6 +113,77 @@ function getOAuthScopeHash(oauthScopes: string[]): string {
         .slice(0, 16);
 }
 
+/**
+ * Provider APIs such as OpenAI Responses reject dots and other punctuation in
+ * tool names. MCP tool names are server-controlled, so normalize the fully
+ * qualified name before exposing it to the model.
+ */
+export function sanitizeMcpToolNameForModel(name: string): string {
+    let sanitized = '';
+
+    for (const character of name) {
+        if (
+            (character >= 'a' && character <= 'z') ||
+            (character >= 'A' && character <= 'Z') ||
+            (character >= '0' && character <= '9') ||
+            character === '_'
+        ) {
+            sanitized += character;
+        } else {
+            sanitized += '_';
+        }
+    }
+
+    return sanitized || '_';
+}
+
+function getMcpToolNameCollisionSuffix(qualifiedName: string): string {
+    return createHash('sha256')
+        .update(qualifiedName)
+        .digest('hex')
+        .slice(0, 8);
+}
+
+function buildModelToolNameMap(prefix: string, toolNames: string[]): Map<string, string> {
+    const entries = toolNames.map((toolName) => {
+        const qualifiedName = `${prefix}__${toolName}`;
+        return {
+            toolName,
+            qualifiedName,
+            sanitizedName: sanitizeMcpToolNameForModel(qualifiedName),
+        };
+    });
+
+    const sanitizedNameCounts = new Map<string, number>();
+    for (const entry of entries) {
+        sanitizedNameCounts.set(
+            entry.sanitizedName,
+            (sanitizedNameCounts.get(entry.sanitizedName) ?? 0) + 1,
+        );
+    }
+
+    const modelToolNames = new Map<string, string>();
+    const usedModelToolNames = new Set<string>();
+    for (const entry of [...entries].sort((a, b) => a.qualifiedName.localeCompare(b.qualifiedName))) {
+        let modelToolName = entry.sanitizedName;
+
+        if ((sanitizedNameCounts.get(entry.sanitizedName) ?? 0) > 1) {
+            modelToolName = `${entry.sanitizedName}_${getMcpToolNameCollisionSuffix(entry.qualifiedName)}`;
+        }
+
+        let collisionIndex = 0;
+        while (usedModelToolNames.has(modelToolName)) {
+            collisionIndex += 1;
+            modelToolName = `${entry.sanitizedName}_${getMcpToolNameCollisionSuffix(`${entry.qualifiedName}\0${collisionIndex}`)}`;
+        }
+
+        usedModelToolNames.add(modelToolName);
+        modelToolNames.set(entry.toolName, modelToolName);
+    }
+
+    return modelToolNames;
+}
+
 function getMcpListToolsCacheKey(client: McpToolSet): string {
     return [
         'mcp:list-tools:v1',
@@ -200,6 +271,7 @@ export async function getMcpTools(clients: McpToolSet[], analyticsContext?: McpT
             const toolDefinitions = await getListToolsResult(mcpClient, client, connectionTimeoutMs);
             const tools = mcpClient.toolsFromDefinitions(toolDefinitions);
             const prefix = `mcp_${sanitizedName}`;
+            const modelToolNames = buildModelToolNameMap(prefix, Object.keys(tools));
             await createMissingMcpServerToolRows({
                 serverId,
                 tools: toolDefinitions.tools.map((tool) => {
@@ -258,7 +330,8 @@ export async function getMcpTools(clients: McpToolSet[], analyticsContext?: McpT
                 });
 
                 const originalExecute = tool.execute;
-                const qualifiedName = `${prefix}__${toolName}`;
+                const rawQualifiedName = `${prefix}__${toolName}`;
+                const qualifiedName = modelToolNames.get(toolName) ?? sanitizeMcpToolNameForModel(rawQualifiedName);
                 const timeoutMs = env.SOURCEBOT_MCP_TOOL_CALL_TIMEOUT_MS;
 
                 const executeWithTimeout = (async (input: unknown, options: ToolExecutionOptions) => {