From 22abf988357cbe8525d53f123fc092df13a2df2b Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 9 Dec 2025 12:44:53 -0800 Subject: [PATCH] fix(mcp): prevent redundant MCP server discovery calls at runtime, use cached tool schema instead (#2273) * fix(mcp): prevent redundant MCP server discovery calls at runtime, use cached tool schema instead * added backfill, added loading state for tools in settings > mcp * fix tool inp --- apps/sim/app/api/mcp/tools/execute/route.ts | 71 ++- .../components/tool-input/tool-input.tsx | 50 ++- .../server-list-item/server-list-item.tsx | 6 +- .../settings-modal/components/mcp/mcp.tsx | 9 +- .../handlers/agent/agent-handler.test.ts | 334 ++++++++++++++ .../executor/handlers/agent/agent-handler.ts | 418 +++++++++++++----- apps/sim/lib/mcp/service.ts | 122 +++-- 7 files changed, 819 insertions(+), 191 deletions(-) diff --git a/apps/sim/app/api/mcp/tools/execute/route.ts b/apps/sim/app/api/mcp/tools/execute/route.ts index 48205811d..d58d0bea2 100644 --- a/apps/sim/app/api/mcp/tools/execute/route.ts +++ b/apps/sim/app/api/mcp/tools/execute/route.ts @@ -15,7 +15,6 @@ const logger = createLogger('McpToolExecutionAPI') export const dynamic = 'force-dynamic' -// Type definitions for improved type safety interface SchemaProperty { type: 'string' | 'number' | 'boolean' | 'object' | 'array' description?: string @@ -31,9 +30,6 @@ interface ToolExecutionResult { error?: string } -/** - * Type guard to safely check if a schema property has a type field - */ function hasType(prop: unknown): prop is SchemaProperty { return typeof prop === 'object' && prop !== null && 'type' in prop } @@ -57,7 +53,8 @@ export const POST = withMcpAuth('read')( userId: userId, }) - const { serverId, toolName, arguments: args } = body + const { serverId, toolName, arguments: rawArgs } = body + const args = rawArgs || {} const serverIdValidation = validateStringParam(serverId, 'serverId') if (!serverIdValidation.isValid) { @@ -75,22 +72,31 @@ export const POST = withMcpAuth('read')( `[${requestId}] Executing tool ${toolName} on server ${serverId} for user ${userId} in workspace ${workspaceId}` ) - let tool = null + let tool: McpTool | null = null try { - const tools = await mcpService.discoverServerTools(userId, serverId, workspaceId) - tool = tools.find((t) => t.name === toolName) + if (body.toolSchema) { + tool = { + name: toolName, + inputSchema: body.toolSchema, + serverId: serverId, + serverName: 'provided-schema', + } as McpTool + logger.debug(`[${requestId}] Using provided schema for ${toolName}, skipping discovery`) + } else { + const tools = await mcpService.discoverServerTools(userId, serverId, workspaceId) + tool = tools.find((t) => t.name === toolName) ?? null - if (!tool) { - return createMcpErrorResponse( - new Error( - `Tool ${toolName} not found on server ${serverId}. Available tools: ${tools.map((t) => t.name).join(', ')}` - ), - 'Tool not found', - 404 - ) + if (!tool) { + return createMcpErrorResponse( + new Error( + `Tool ${toolName} not found on server ${serverId}. Available tools: ${tools.map((t) => t.name).join(', ')}` + ), + 'Tool not found', + 404 + ) + } } - // Cast arguments to their expected types based on tool schema if (tool.inputSchema?.properties) { for (const [paramName, paramSchema] of Object.entries(tool.inputSchema.properties)) { const schema = paramSchema as any @@ -100,7 +106,6 @@ export const POST = withMcpAuth('read')( continue } - // Cast numbers if ( (schema.type === 'number' || schema.type === 'integer') && typeof value === 'string' @@ -110,42 +115,33 @@ export const POST = withMcpAuth('read')( if (!Number.isNaN(numValue)) { args[paramName] = numValue } - } - // Cast booleans - else if (schema.type === 'boolean' && typeof value === 'string') { + } else if (schema.type === 'boolean' && typeof value === 'string') { if (value.toLowerCase() === 'true') { args[paramName] = true } else if (value.toLowerCase() === 'false') { args[paramName] = false } - } - // Cast arrays - else if (schema.type === 'array' && typeof value === 'string') { + } else if (schema.type === 'array' && typeof value === 'string') { const stringValue = value.trim() if (stringValue) { try { - // Try to parse as JSON first (handles ["item1", "item2"]) const parsed = JSON.parse(stringValue) if (Array.isArray(parsed)) { args[paramName] = parsed } else { - // JSON parsed but not an array, wrap in array args[paramName] = [parsed] } - } catch (error) { - // JSON parsing failed - treat as comma-separated if contains commas, otherwise single item + } catch { if (stringValue.includes(',')) { args[paramName] = stringValue .split(',') .map((item) => item.trim()) .filter((item) => item) } else { - // Single item - wrap in array since schema expects array args[paramName] = [stringValue] } } } else { - // Empty string becomes empty array args[paramName] = [] } } @@ -172,7 +168,7 @@ export const POST = withMcpAuth('read')( const toolCall: McpToolCall = { name: toolName, - arguments: args || {}, + arguments: args, } const result = await Promise.race([ @@ -197,7 +193,6 @@ export const POST = withMcpAuth('read')( } logger.info(`[${requestId}] Successfully executed tool ${toolName} on server ${serverId}`) - // Track MCP tool execution try { const { trackPlatformEvent } = await import('@/lib/core/telemetry') trackPlatformEvent('platform.mcp.tool_executed', { @@ -206,8 +201,8 @@ export const POST = withMcpAuth('read')( 'mcp.execution_status': 'success', 'workspace.id': workspaceId, }) - } catch (_e) { - // Silently fail + } catch { + // Telemetry failure is non-critical } return createMcpSuccessResponse(transformedResult) @@ -220,12 +215,9 @@ export const POST = withMcpAuth('read')( } ) -/** - * Validate tool arguments against schema - */ function validateToolArguments(tool: McpTool, args: Record): string | null { if (!tool.inputSchema) { - return null // No schema to validate against + return null } const schema = tool.inputSchema @@ -270,9 +262,6 @@ function validateToolArguments(tool: McpTool, args: Record): st return null } -/** - * Transform MCP tool result to platform format - */ function transformToolResult(result: McpToolResult): ToolExecutionResult { if (result.isError) { return { diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index f925c04c3..c68e7b13d 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useQuery } from '@tanstack/react-query' import { Loader2, PlusIcon, WrenchIcon, XIcon } from 'lucide-react' import { useParams } from 'next/navigation' @@ -845,6 +845,52 @@ export function ToolInput({ ? (value as unknown as StoredTool[]) : [] + const hasBackfilledRef = useRef(false) + useEffect(() => { + if ( + isPreview || + mcpLoading || + mcpTools.length === 0 || + selectedTools.length === 0 || + hasBackfilledRef.current + ) { + return + } + + const mcpToolsNeedingSchema = selectedTools.filter( + (tool) => tool.type === 'mcp' && !tool.schema && tool.params?.toolName + ) + + if (mcpToolsNeedingSchema.length === 0) { + return + } + + const updatedTools = selectedTools.map((tool) => { + if (tool.type !== 'mcp' || tool.schema || !tool.params?.toolName) { + return tool + } + + const mcpTool = mcpTools.find( + (mt) => mt.name === tool.params?.toolName && mt.serverId === tool.params?.serverId + ) + + if (mcpTool?.inputSchema) { + logger.info(`Backfilling schema for MCP tool: ${tool.params.toolName}`) + return { ...tool, schema: mcpTool.inputSchema } + } + + return tool + }) + + const hasChanges = updatedTools.some((tool, i) => tool.schema && !selectedTools[i].schema) + + if (hasChanges) { + hasBackfilledRef.current = true + logger.info(`Backfilled schemas for ${mcpToolsNeedingSchema.length} MCP tool(s)`) + setStoreValue(updatedTools) + } + }, [mcpTools, mcpLoading, selectedTools, isPreview, setStoreValue]) + /** * Checks if a tool is already selected in the current workflow * @param toolId - The tool identifier to check @@ -2314,7 +2360,7 @@ export function ToolInput({ mcpTools={mcpTools} searchQuery={searchQuery || ''} customFilter={customFilter} - onToolSelect={(tool) => handleMcpToolSelect(tool, false)} + onToolSelect={handleMcpToolSelect} disabled={false} /> diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/components/server-list-item/server-list-item.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/components/server-list-item/server-list-item.tsx index a4de645a8..e35a82f64 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/components/server-list-item/server-list-item.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/components/server-list-item/server-list-item.tsx @@ -28,6 +28,7 @@ interface ServerListItemProps { server: any tools: any[] isDeleting: boolean + isLoadingTools?: boolean onRemove: () => void onViewDetails: () => void } @@ -39,6 +40,7 @@ export function ServerListItem({ server, tools, isDeleting, + isLoadingTools = false, onRemove, onViewDetails, }: ServerListItemProps) { @@ -54,7 +56,9 @@ export function ServerListItem({ ({transportLabel}) -

{toolsLabel}

+

+ {isLoadingTools && tools.length === 0 ? 'Loading...' : toolsLabel} +