Skip to content

Commit 3b4f227

Browse files
waleedlatif1waleedlatif1
authored
fix(mcp): reuse sessionID for consecutive MCP tool calls, fix dynamic args clearing, fix refreshing tools on save (#2158)
* fix(mcp): reuse sessionID for consecutive MCP tool calls, fix dynamic args clearing, fix refreshing tools on save * prevent defaults * fix subblock text area * added placeholders in tool-inp for mcp dynamic args * ack PR comments
1 parent 0ae7eb1 commit 3b4f227

File tree

5 files changed

+174
-53
lines changed

5 files changed

+174
-53
lines changed

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx‎

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ function McpInputWithTags({
4040
const [cursorPosition, setCursorPosition] = useState(0)
4141
const [activeSourceBlockId, setActiveSourceBlockId] = useState<string | null>(null)
4242
const inputRef = useRef<HTMLInputElement>(null)
43+
const inputNameRef = useRef(`mcp_input_${Math.random()}`)
4344

4445
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
4546
const newValue = e.target.value
@@ -104,11 +105,19 @@ function McpInputWithTags({
104105
onDragOver={handleDragOver}
105106
placeholder={placeholder}
106107
disabled={disabled}
108+
name={inputNameRef.current}
107109
autoComplete='off'
110+
autoCapitalize='off'
111+
spellCheck='false'
112+
data-form-type='other'
113+
data-lpignore='true'
114+
data-1p-ignore
115+
readOnly
116+
onFocus={(e) => e.currentTarget.removeAttribute('readOnly')}
108117
className={cn(!isPassword && 'text-transparent caret-foreground')}
109118
/>
110119
{!isPassword && (
111-
<div className='pointer-events-none absolute inset-0 flex items-center overflow-hidden bg-transparent px-3 text-sm'>
120+
<div className='pointer-events-none absolute inset-0 flex items-center overflow-hidden bg-transparent px-[8px] py-[6px] font-medium font-sans text-sm'>
112121
<div className='whitespace-pre'>
113122
{formatDisplayText(value?.toString() || '', {
114123
accessiblePrefixes,
@@ -157,6 +166,7 @@ function McpTextareaWithTags({
157166
const [cursorPosition, setCursorPosition] = useState(0)
158167
const [activeSourceBlockId, setActiveSourceBlockId] = useState<string | null>(null)
159168
const textareaRef = useRef<HTMLTextAreaElement>(null)
169+
const textareaNameRef = useRef(`mcp_textarea_${Math.random()}`)
160170

161171
const handleChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
162172
const newValue = e.target.value
@@ -220,9 +230,16 @@ function McpTextareaWithTags({
220230
placeholder={placeholder}
221231
disabled={disabled}
222232
rows={rows}
233+
name={textareaNameRef.current}
234+
autoComplete='off'
235+
autoCapitalize='off'
236+
spellCheck='false'
237+
data-form-type='other'
238+
data-lpignore='true'
239+
data-1p-ignore
223240
className={cn('min-h-[80px] resize-none text-transparent caret-foreground')}
224241
/>
225-
<div className='pointer-events-none absolute inset-0 overflow-auto whitespace-pre-wrap break-words p-3 text-sm'>
242+
<div className='pointer-events-none absolute inset-0 overflow-auto whitespace-pre-wrap break-words px-[8px] py-[8px] font-medium font-sans text-sm'>
226243
{formatDisplayText(value || '', {
227244
accessiblePrefixes,
228245
highlightAll: !accessiblePrefixes,
@@ -298,6 +315,17 @@ export function McpDynamicArgs({
298315
if (disabled) return
299316

300317
const current = currentArgs()
318+
319+
if (value === '' && (current[paramName] === undefined || current[paramName] === null)) {
320+
return
321+
}
322+
323+
if (value === '') {
324+
const { [paramName]: _, ...rest } = current
325+
setToolArgs(Object.keys(rest).length > 0 ? rest : {})
326+
return
327+
}
328+
301329
const updated = { ...current, [paramName]: value }
302330
setToolArgs(updated)
303331
},
@@ -509,7 +537,32 @@ export function McpDynamicArgs({
509537
}
510538

511539
return (
512-
<div className='space-y-4'>
540+
<div className='relative space-y-4'>
541+
{/* Hidden dummy inputs to prevent browser password manager autofill */}
542+
<input
543+
type='text'
544+
name='fakeusernameremembered'
545+
autoComplete='username'
546+
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
547+
tabIndex={-1}
548+
readOnly
549+
/>
550+
<input
551+
type='password'
552+
name='fakepasswordremembered'
553+
autoComplete='current-password'
554+
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
555+
tabIndex={-1}
556+
readOnly
557+
/>
558+
<input
559+
type='email'
560+
name='fakeemailremembered'
561+
autoComplete='email'
562+
style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
563+
tabIndex={-1}
564+
readOnly
565+
/>
513566
{toolSchema.properties &&
514567
Object.entries(toolSchema.properties).map(([paramName, paramSchema]) => {
515568
const inputType = getInputType(paramSchema as any)

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2107,7 +2107,10 @@ export function ToolInput({
21072107
<ShortInput
21082108
blockId={blockId}
21092109
subBlockId={`${subBlockId}-tool-${toolIndex}-${param.id}`}
2110-
placeholder={param.description}
2110+
placeholder={
2111+
param.description ||
2112+
`Enter ${formatParameterLabel(param.id).toLowerCase()}`
2113+
}
21112114
password={isPasswordParameter(param.id)}
21122115
config={{
21132116
id: `${subBlockId}-tool-${toolIndex}-${param.id}`,

‎apps/sim/hooks/queries/mcp.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,14 @@ export function useCreateMcpServer() {
152152
}
153153

154154
logger.info(`Created MCP server: ${config.name} in workspace: ${workspaceId}`)
155-
return { ...serverData, connectionStatus: 'disconnected' as const }
155+
return {
156+
...serverData,
157+
connectionStatus: 'disconnected' as const,
158+
serverId: data.data?.serverId,
159+
}
156160
},
157161
onSuccess: (_data, variables) => {
158-
// Invalidate servers list to refetch
159162
queryClient.invalidateQueries({ queryKey: mcpKeys.servers(variables.workspaceId) })
160-
// Invalidate tools as new server may provide new tools
161163
queryClient.invalidateQueries({ queryKey: mcpKeys.tools(variables.workspaceId) })
162164
},
163165
})

‎apps/sim/lib/mcp/client.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,13 @@ export class McpClient {
4242
'2024-11-05', // Initial stable release
4343
]
4444

45-
constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy) {
45+
/**
46+
* Creates a new MCP client
47+
* @param config - Server configuration
48+
* @param securityPolicy - Optional security policy
49+
* @param sessionId - Optional session ID for session restoration (from previous connection)
50+
*/
51+
constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy, sessionId?: string) {
4652
this.config = config
4753
this.connectionStatus = { connected: false }
4854
this.securityPolicy = securityPolicy ?? {
@@ -59,6 +65,7 @@ export class McpClient {
5965
requestInit: {
6066
headers: this.config.headers,
6167
},
68+
sessionId,
6269
})
6370

6471
this.client = new Client(
@@ -255,6 +262,10 @@ export class McpClient {
255262
return typeof serverVersion === 'string' ? serverVersion : undefined
256263
}
257264

265+
getSessionId(): string | undefined {
266+
return this.transport.sessionId
267+
}
268+
258269
/**
259270
* Request user consent for tool execution
260271
*/

‎apps/sim/lib/mcp/service.ts‎

Lines changed: 97 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,35 @@ class McpService {
4949
private cacheMisses = 0
5050
private entriesEvicted = 0
5151

52+
private sessionCache = new Map<string, string>()
53+
5254
constructor() {
5355
this.startPeriodicCleanup()
5456
}
5557

58+
/**
59+
* Get cached session ID for a server
60+
*/
61+
private getCachedSessionId(serverId: string): string | undefined {
62+
return this.sessionCache.get(serverId)
63+
}
64+
65+
/**
66+
* Cache session ID for a server
67+
*/
68+
private cacheSessionId(serverId: string, sessionId: string): void {
69+
this.sessionCache.set(serverId, sessionId)
70+
logger.debug(`Cached session ID for server ${serverId}`)
71+
}
72+
73+
/**
74+
* Clear cached session ID for a server
75+
*/
76+
private clearCachedSessionId(serverId: string): void {
77+
this.sessionCache.delete(serverId)
78+
logger.debug(`Cleared cached session ID for server ${serverId}`)
79+
}
80+
5681
/**
5782
* Start periodic cleanup of expired cache entries
5883
*/
@@ -306,7 +331,7 @@ class McpService {
306331
}
307332

308333
/**
309-
* Create and connect to an MCP client with security policy
334+
* Create and connect to an MCP client
310335
*/
311336
private async createClient(config: McpServerConfig): Promise<McpClient> {
312337
const securityPolicy = {
@@ -316,9 +341,49 @@ class McpService {
316341
allowedOrigins: config.url ? [new URL(config.url).origin] : undefined,
317342
}
318343

319-
const client = new McpClient(config, securityPolicy)
320-
await client.connect()
321-
return client
344+
const cachedSessionId = this.getCachedSessionId(config.id)
345+
346+
const client = new McpClient(config, securityPolicy, cachedSessionId)
347+
348+
try {
349+
await client.connect()
350+
351+
const newSessionId = client.getSessionId()
352+
if (newSessionId) {
353+
this.cacheSessionId(config.id, newSessionId)
354+
}
355+
356+
return client
357+
} catch (error) {
358+
if (cachedSessionId && this.isSessionError(error)) {
359+
logger.debug(`Session restoration failed for server ${config.id}, retrying fresh`)
360+
this.clearCachedSessionId(config.id)
361+
362+
const freshClient = new McpClient(config, securityPolicy)
363+
await freshClient.connect()
364+
365+
const freshSessionId = freshClient.getSessionId()
366+
if (freshSessionId) {
367+
this.cacheSessionId(config.id, freshSessionId)
368+
}
369+
370+
return freshClient
371+
}
372+
373+
throw error
374+
}
375+
}
376+
377+
private isSessionError(error: unknown): boolean {
378+
if (error instanceof Error) {
379+
const message = error.message.toLowerCase()
380+
return (
381+
message.includes('no valid session') ||
382+
message.includes('invalid session') ||
383+
message.includes('session expired')
384+
)
385+
}
386+
return false
322387
}
323388

324389
/**
@@ -332,33 +397,25 @@ class McpService {
332397
): Promise<McpToolResult> {
333398
const requestId = generateRequestId()
334399

335-
try {
336-
logger.info(
337-
`[${requestId}] Executing MCP tool ${toolCall.name} on server ${serverId} for user ${userId}`
338-
)
400+
logger.info(
401+
`[${requestId}] Executing MCP tool ${toolCall.name} on server ${serverId} for user ${userId}`
402+
)
339403

340-
const config = await this.getServerConfig(serverId, workspaceId)
341-
if (!config) {
342-
throw new Error(`Server ${serverId} not found or not accessible`)
343-
}
404+
const config = await this.getServerConfig(serverId, workspaceId)
405+
if (!config) {
406+
throw new Error(`Server ${serverId} not found or not accessible`)
407+
}
344408

345-
const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
409+
const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
346410

347-
const client = await this.createClient(resolvedConfig)
411+
const client = await this.createClient(resolvedConfig)
348412

349-
try {
350-
const result = await client.callTool(toolCall)
351-
logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
352-
return result
353-
} finally {
354-
await client.disconnect()
355-
}
356-
} catch (error) {
357-
logger.error(
358-
`[${requestId}] Failed to execute tool ${toolCall.name} on server ${serverId}:`,
359-
error
360-
)
361-
throw error
413+
try {
414+
const result = await client.callTool(toolCall)
415+
logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
416+
return result
417+
} finally {
418+
await client.disconnect()
362419
}
363420
}
364421

@@ -442,28 +499,23 @@ class McpService {
442499
): Promise<McpTool[]> {
443500
const requestId = generateRequestId()
444501

445-
try {
446-
logger.info(`[${requestId}] Discovering tools from server ${serverId} for user ${userId}`)
502+
logger.info(`[${requestId}] Discovering tools from server ${serverId} for user ${userId}`)
447503

448-
const config = await this.getServerConfig(serverId, workspaceId)
449-
if (!config) {
450-
throw new Error(`Server ${serverId} not found or not accessible`)
451-
}
504+
const config = await this.getServerConfig(serverId, workspaceId)
505+
if (!config) {
506+
throw new Error(`Server ${serverId} not found or not accessible`)
507+
}
452508

453-
const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
509+
const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
454510

455-
const client = await this.createClient(resolvedConfig)
511+
const client = await this.createClient(resolvedConfig)
456512

457-
try {
458-
const tools = await client.listTools()
459-
logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
460-
return tools
461-
} finally {
462-
await client.disconnect()
463-
}
464-
} catch (error) {
465-
logger.error(`[${requestId}] Failed to discover tools from server ${serverId}:`, error)
466-
throw error
513+
try {
514+
const tools = await client.listTools()
515+
logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
516+
return tools
517+
} finally {
518+
await client.disconnect()
467519
}
468520
}
469521

0 commit comments

Comments
 (0)