Skip to content

Commit 22abf98

Browse files
waleedlatif1waleedlatif1
authored
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
1 parent aa1d896 commit 22abf98

File tree

7 files changed

+814
-186
lines changed

7 files changed

+814
-186
lines changed

‎apps/sim/app/api/mcp/tools/execute/route.ts‎

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ const logger = createLogger('McpToolExecutionAPI')
1515

1616
export const dynamic = 'force-dynamic'
1717

18-
// Type definitions for improved type safety
1918
interface SchemaProperty {
2019
type: 'string' | 'number' | 'boolean' | 'object' | 'array'
2120
description?: string
@@ -31,9 +30,6 @@ interface ToolExecutionResult {
3130
error?: string
3231
}
3332

34-
/**
35-
* Type guard to safely check if a schema property has a type field
36-
*/
3733
function hasType(prop: unknown): prop is SchemaProperty {
3834
return typeof prop === 'object' && prop !== null && 'type' in prop
3935
}
@@ -57,7 +53,8 @@ export const POST = withMcpAuth('read')(
5753
userId: userId,
5854
})
5955

60-
const { serverId, toolName, arguments: args } = body
56+
const { serverId, toolName, arguments: rawArgs } = body
57+
const args = rawArgs || {}
6158

6259
const serverIdValidation = validateStringParam(serverId, 'serverId')
6360
if (!serverIdValidation.isValid) {
@@ -75,22 +72,31 @@ export const POST = withMcpAuth('read')(
7572
`[${requestId}] Executing tool ${toolName} on server ${serverId} for user ${userId} in workspace ${workspaceId}`
7673
)
7774

78-
let tool = null
75+
let tool: McpTool | null = null
7976
try {
80-
const tools = await mcpService.discoverServerTools(userId, serverId, workspaceId)
81-
tool = tools.find((t) => t.name === toolName)
82-
83-
if (!tool) {
84-
return createMcpErrorResponse(
85-
new Error(
86-
`Tool ${toolName} not found on server ${serverId}. Available tools: ${tools.map((t) => t.name).join(', ')}`
87-
),
88-
'Tool not found',
89-
404
90-
)
77+
if (body.toolSchema) {
78+
tool = {
79+
name: toolName,
80+
inputSchema: body.toolSchema,
81+
serverId: serverId,
82+
serverName: 'provided-schema',
83+
} as McpTool
84+
logger.debug(`[${requestId}] Using provided schema for ${toolName}, skipping discovery`)
85+
} else {
86+
const tools = await mcpService.discoverServerTools(userId, serverId, workspaceId)
87+
tool = tools.find((t) => t.name === toolName) ?? null
88+
89+
if (!tool) {
90+
return createMcpErrorResponse(
91+
new Error(
92+
`Tool ${toolName} not found on server ${serverId}. Available tools: ${tools.map((t) => t.name).join(', ')}`
93+
),
94+
'Tool not found',
95+
404
96+
)
97+
}
9198
}
9299

93-
// Cast arguments to their expected types based on tool schema
94100
if (tool.inputSchema?.properties) {
95101
for (const [paramName, paramSchema] of Object.entries(tool.inputSchema.properties)) {
96102
const schema = paramSchema as any
@@ -100,7 +106,6 @@ export const POST = withMcpAuth('read')(
100106
continue
101107
}
102108

103-
// Cast numbers
104109
if (
105110
(schema.type === 'number' || schema.type === 'integer') &&
106111
typeof value === 'string'
@@ -110,42 +115,33 @@ export const POST = withMcpAuth('read')(
110115
if (!Number.isNaN(numValue)) {
111116
args[paramName] = numValue
112117
}
113-
}
114-
// Cast booleans
115-
else if (schema.type === 'boolean' && typeof value === 'string') {
118+
} else if (schema.type === 'boolean' && typeof value === 'string') {
116119
if (value.toLowerCase() === 'true') {
117120
args[paramName] = true
118121
} else if (value.toLowerCase() === 'false') {
119122
args[paramName] = false
120123
}
121-
}
122-
// Cast arrays
123-
else if (schema.type === 'array' && typeof value === 'string') {
124+
} else if (schema.type === 'array' && typeof value === 'string') {
124125
const stringValue = value.trim()
125126
if (stringValue) {
126127
try {
127-
// Try to parse as JSON first (handles ["item1", "item2"])
128128
const parsed = JSON.parse(stringValue)
129129
if (Array.isArray(parsed)) {
130130
args[paramName] = parsed
131131
} else {
132-
// JSON parsed but not an array, wrap in array
133132
args[paramName] = [parsed]
134133
}
135-
} catch (error) {
136-
// JSON parsing failed - treat as comma-separated if contains commas, otherwise single item
134+
} catch {
137135
if (stringValue.includes(',')) {
138136
args[paramName] = stringValue
139137
.split(',')
140138
.map((item) => item.trim())
141139
.filter((item) => item)
142140
} else {
143-
// Single item - wrap in array since schema expects array
144141
args[paramName] = [stringValue]
145142
}
146143
}
147144
} else {
148-
// Empty string becomes empty array
149145
args[paramName] = []
150146
}
151147
}
@@ -172,7 +168,7 @@ export const POST = withMcpAuth('read')(
172168

173169
const toolCall: McpToolCall = {
174170
name: toolName,
175-
arguments: args || {},
171+
arguments: args,
176172
}
177173

178174
const result = await Promise.race([
@@ -197,7 +193,6 @@ export const POST = withMcpAuth('read')(
197193
}
198194
logger.info(`[${requestId}] Successfully executed tool ${toolName} on server ${serverId}`)
199195

200-
// Track MCP tool execution
201196
try {
202197
const { trackPlatformEvent } = await import('@/lib/core/telemetry')
203198
trackPlatformEvent('platform.mcp.tool_executed', {
@@ -206,8 +201,8 @@ export const POST = withMcpAuth('read')(
206201
'mcp.execution_status': 'success',
207202
'workspace.id': workspaceId,
208203
})
209-
} catch (_e) {
210-
// Silently fail
204+
} catch {
205+
// Telemetry failure is non-critical
211206
}
212207

213208
return createMcpSuccessResponse(transformedResult)
@@ -220,12 +215,9 @@ export const POST = withMcpAuth('read')(
220215
}
221216
)
222217

223-
/**
224-
* Validate tool arguments against schema
225-
*/
226218
function validateToolArguments(tool: McpTool, args: Record<string, unknown>): string | null {
227219
if (!tool.inputSchema) {
228-
return null // No schema to validate against
220+
return null
229221
}
230222

231223
const schema = tool.inputSchema
@@ -270,9 +262,6 @@ function validateToolArguments(tool: McpTool, args: Record<string, unknown>): st
270262
return null
271263
}
272264

273-
/**
274-
* Transform MCP tool result to platform format
275-
*/
276265
function transformToolResult(result: McpToolResult): ToolExecutionResult {
277266
if (result.isError) {
278267
return {

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type React from 'react'
2-
import { useCallback, useEffect, useMemo, useState } from 'react'
2+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
33
import { useQuery } from '@tanstack/react-query'
44
import { Loader2, PlusIcon, WrenchIcon, XIcon } from 'lucide-react'
55
import { useParams } from 'next/navigation'
@@ -845,6 +845,52 @@ export function ToolInput({
845845
? (value as unknown as StoredTool[])
846846
: []
847847

848+
const hasBackfilledRef = useRef(false)
849+
useEffect(() => {
850+
if (
851+
isPreview ||
852+
mcpLoading ||
853+
mcpTools.length === 0 ||
854+
selectedTools.length === 0 ||
855+
hasBackfilledRef.current
856+
) {
857+
return
858+
}
859+
860+
const mcpToolsNeedingSchema = selectedTools.filter(
861+
(tool) => tool.type === 'mcp' && !tool.schema && tool.params?.toolName
862+
)
863+
864+
if (mcpToolsNeedingSchema.length === 0) {
865+
return
866+
}
867+
868+
const updatedTools = selectedTools.map((tool) => {
869+
if (tool.type !== 'mcp' || tool.schema || !tool.params?.toolName) {
870+
return tool
871+
}
872+
873+
const mcpTool = mcpTools.find(
874+
(mt) => mt.name === tool.params?.toolName && mt.serverId === tool.params?.serverId
875+
)
876+
877+
if (mcpTool?.inputSchema) {
878+
logger.info(`Backfilling schema for MCP tool: ${tool.params.toolName}`)
879+
return { ...tool, schema: mcpTool.inputSchema }
880+
}
881+
882+
return tool
883+
})
884+
885+
const hasChanges = updatedTools.some((tool, i) => tool.schema && !selectedTools[i].schema)
886+
887+
if (hasChanges) {
888+
hasBackfilledRef.current = true
889+
logger.info(`Backfilled schemas for ${mcpToolsNeedingSchema.length} MCP tool(s)`)
890+
setStoreValue(updatedTools)
891+
}
892+
}, [mcpTools, mcpLoading, selectedTools, isPreview, setStoreValue])
893+
848894
/**
849895
* Checks if a tool is already selected in the current workflow
850896
* @param toolId - The tool identifier to check
@@ -2314,7 +2360,7 @@ export function ToolInput({
23142360
mcpTools={mcpTools}
23152361
searchQuery={searchQuery || ''}
23162362
customFilter={customFilter}
2317-
onToolSelect={(tool) => handleMcpToolSelect(tool, false)}
2363+
onToolSelect={handleMcpToolSelect}
23182364
disabled={false}
23192365
/>
23202366

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/components/server-list-item/server-list-item.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ interface ServerListItemProps {
2828
server: any
2929
tools: any[]
3030
isDeleting: boolean
31+
isLoadingTools?: boolean
3132
onRemove: () => void
3233
onViewDetails: () => void
3334
}
@@ -39,6 +40,7 @@ export function ServerListItem({
3940
server,
4041
tools,
4142
isDeleting,
43+
isLoadingTools = false,
4244
onRemove,
4345
onViewDetails,
4446
}: ServerListItemProps) {
@@ -54,7 +56,9 @@ export function ServerListItem({
5456
</span>
5557
<span className='text-[13px] text-[var(--text-secondary)]'>({transportLabel})</span>
5658
</div>
57-
<p className='truncate text-[13px] text-[var(--text-muted)]'>{toolsLabel}</p>
59+
<p className='truncate text-[13px] text-[var(--text-muted)]'>
60+
{isLoadingTools && tools.length === 0 ? 'Loading...' : toolsLabel}
61+
</p>
5862
</div>
5963
<div className='flex flex-shrink-0 items-center gap-[4px]'>
6064
<Button

‎apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components-new/settings-modal/components/mcp/mcp.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,12 @@ export function MCP() {
8080
isLoading: serversLoading,
8181
error: serversError,
8282
} = useMcpServers(workspaceId)
83-
const { data: mcpToolsData = [], error: toolsError } = useMcpToolsQuery(workspaceId)
83+
const {
84+
data: mcpToolsData = [],
85+
error: toolsError,
86+
isLoading: toolsLoading,
87+
isFetching: toolsFetching,
88+
} = useMcpToolsQuery(workspaceId)
8489
const createServerMutation = useCreateMcpServer()
8590
const deleteServerMutation = useDeleteMcpServer()
8691
const { testResult, isTestingConnection, testConnection, clearTestResult } = useMcpServerTest()
@@ -632,13 +637,15 @@ export function MCP() {
632637
{filteredServers.map((server) => {
633638
if (!server?.id) return null
634639
const tools = toolsByServer[server.id] || []
640+
const isLoadingTools = toolsLoading || toolsFetching
635641

636642
return (
637643
<ServerListItem
638644
key={server.id}
639645
server={server}
640646
tools={tools}
641647
isDeleting={deletingServers.has(server.id)}
648+
isLoadingTools={isLoadingTools}
642649
onRemove={() => handleRemoveServer(server.id, server.name || 'this server')}
643650
onViewDetails={() => handleViewDetails(server.id)}
644651
/>

0 commit comments

Comments
 (0)