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
2026-04-06 03:00:16 -04:00 · 2025-12-02 12:37:59 -08:00
parent 0ae7eb197a
commit 3b4f227e43
5 changed files with 180 additions and 59 deletions
--- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx
+++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx
@@ -40,6 +40,7 @@ function McpInputWithTags({
  const [cursorPosition, setCursorPosition] = useState(0)
  const [activeSourceBlockId, setActiveSourceBlockId] = useState<string | null>(null)
  const inputRef = useRef<HTMLInputElement>(null)
+  const inputNameRef = useRef(`mcp_input_${Math.random()}`)

  const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
    const newValue = e.target.value
@@ -104,11 +105,19 @@ function McpInputWithTags({
          onDragOver={handleDragOver}
          placeholder={placeholder}
          disabled={disabled}
+          name={inputNameRef.current}
          autoComplete='off'
+          autoCapitalize='off'
+          spellCheck='false'
+          data-form-type='other'
+          data-lpignore='true'
+          data-1p-ignore
+          readOnly
+          onFocus={(e) => e.currentTarget.removeAttribute('readOnly')}
          className={cn(!isPassword && 'text-transparent caret-foreground')}
        />
        {!isPassword && (
-          <div className='pointer-events-none absolute inset-0 flex items-center overflow-hidden bg-transparent px-3 text-sm'>
+          <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'>
            <div className='whitespace-pre'>
              {formatDisplayText(value?.toString() || '', {
                accessiblePrefixes,
@@ -157,6 +166,7 @@ function McpTextareaWithTags({
  const [cursorPosition, setCursorPosition] = useState(0)
  const [activeSourceBlockId, setActiveSourceBlockId] = useState<string | null>(null)
  const textareaRef = useRef<HTMLTextAreaElement>(null)
+  const textareaNameRef = useRef(`mcp_textarea_${Math.random()}`)

  const handleChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
    const newValue = e.target.value
@@ -220,9 +230,16 @@ function McpTextareaWithTags({
        placeholder={placeholder}
        disabled={disabled}
        rows={rows}
+        name={textareaNameRef.current}
+        autoComplete='off'
+        autoCapitalize='off'
+        spellCheck='false'
+        data-form-type='other'
+        data-lpignore='true'
+        data-1p-ignore
        className={cn('min-h-[80px] resize-none text-transparent caret-foreground')}
      />
-      <div className='pointer-events-none absolute inset-0 overflow-auto whitespace-pre-wrap break-words p-3 text-sm'>
+      <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'>
        {formatDisplayText(value || '', {
          accessiblePrefixes,
          highlightAll: !accessiblePrefixes,
@@ -298,6 +315,17 @@ export function McpDynamicArgs({
      if (disabled) return

      const current = currentArgs()
+
+      if (value === '' && (current[paramName] === undefined || current[paramName] === null)) {
+        return
+      }
+
+      if (value === '') {
+        const { [paramName]: _, ...rest } = current
+        setToolArgs(Object.keys(rest).length > 0 ? rest : {})
+        return
+      }
+
      const updated = { ...current, [paramName]: value }
      setToolArgs(updated)
    },
@@ -509,7 +537,32 @@ export function McpDynamicArgs({
  }

  return (
-    <div className='space-y-4'>
+    <div className='relative space-y-4'>
+      {/* Hidden dummy inputs to prevent browser password manager autofill */}
+      <input
+        type='text'
+        name='fakeusernameremembered'
+        autoComplete='username'
+        style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
+        tabIndex={-1}
+        readOnly
+      />
+      <input
+        type='password'
+        name='fakepasswordremembered'
+        autoComplete='current-password'
+        style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
+        tabIndex={-1}
+        readOnly
+      />
+      <input
+        type='email'
+        name='fakeemailremembered'
+        autoComplete='email'
+        style={{ position: 'absolute', left: '-9999px', opacity: 0, pointerEvents: 'none' }}
+        tabIndex={-1}
+        readOnly
+      />
      {toolSchema.properties &&
        Object.entries(toolSchema.properties).map(([paramName, paramSchema]) => {
          const inputType = getInputType(paramSchema as any)
--- 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
@@ -2107,7 +2107,10 @@ export function ToolInput({
                                <ShortInput
                                  blockId={blockId}
                                  subBlockId={`${subBlockId}-tool-${toolIndex}-${param.id}`}
-                                  placeholder={param.description}
+                                  placeholder={
+                                    param.description ||
+                                    `Enter ${formatParameterLabel(param.id).toLowerCase()}`
+                                  }
                                  password={isPasswordParameter(param.id)}
                                  config={{
                                    id: `${subBlockId}-tool-${toolIndex}-${param.id}`,
--- a/apps/sim/hooks/queries/mcp.ts
+++ b/apps/sim/hooks/queries/mcp.ts
@@ -152,12 +152,14 @@ export function useCreateMcpServer() {
      }

      logger.info(`Created MCP server: ${config.name} in workspace: ${workspaceId}`)
-      return { ...serverData, connectionStatus: 'disconnected' as const }
+      return {
+        ...serverData,
+        connectionStatus: 'disconnected' as const,
+        serverId: data.data?.serverId,
+      }
    },
    onSuccess: (_data, variables) => {
-      // Invalidate servers list to refetch
      queryClient.invalidateQueries({ queryKey: mcpKeys.servers(variables.workspaceId) })
-      // Invalidate tools as new server may provide new tools
      queryClient.invalidateQueries({ queryKey: mcpKeys.tools(variables.workspaceId) })
    },
  })
--- a/apps/sim/lib/mcp/client.ts
+++ b/apps/sim/lib/mcp/client.ts
@@ -42,7 +42,13 @@ export class McpClient {
    '2024-11-05', // Initial stable release
  ]

-  constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy) {
+  /**
+   * Creates a new MCP client
+   * @param config - Server configuration
+   * @param securityPolicy - Optional security policy
+   * @param sessionId - Optional session ID for session restoration (from previous connection)
+   */
+  constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy, sessionId?: string) {
    this.config = config
    this.connectionStatus = { connected: false }
    this.securityPolicy = securityPolicy ?? {
@@ -59,6 +65,7 @@ export class McpClient {
      requestInit: {
        headers: this.config.headers,
      },
+      sessionId,
    })

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

+  getSessionId(): string | undefined {
+    return this.transport.sessionId
+  }
+
  /**
   * Request user consent for tool execution
   */
--- a/apps/sim/lib/mcp/service.ts
+++ b/apps/sim/lib/mcp/service.ts
@@ -49,10 +49,35 @@ class McpService {
  private cacheMisses = 0
  private entriesEvicted = 0

+  private sessionCache = new Map<string, string>()
+
  constructor() {
    this.startPeriodicCleanup()
  }

+  /**
+   * Get cached session ID for a server
+   */
+  private getCachedSessionId(serverId: string): string | undefined {
+    return this.sessionCache.get(serverId)
+  }
+
+  /**
+   * Cache session ID for a server
+   */
+  private cacheSessionId(serverId: string, sessionId: string): void {
+    this.sessionCache.set(serverId, sessionId)
+    logger.debug(`Cached session ID for server ${serverId}`)
+  }
+
+  /**
+   * Clear cached session ID for a server
+   */
+  private clearCachedSessionId(serverId: string): void {
+    this.sessionCache.delete(serverId)
+    logger.debug(`Cleared cached session ID for server ${serverId}`)
+  }
+
  /**
   * Start periodic cleanup of expired cache entries
   */
@@ -306,7 +331,7 @@ class McpService {
  }

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

-    const client = new McpClient(config, securityPolicy)
-    await client.connect()
-    return client
+    const cachedSessionId = this.getCachedSessionId(config.id)
+
+    const client = new McpClient(config, securityPolicy, cachedSessionId)
+
+    try {
+      await client.connect()
+
+      const newSessionId = client.getSessionId()
+      if (newSessionId) {
+        this.cacheSessionId(config.id, newSessionId)
+      }
+
+      return client
+    } catch (error) {
+      if (cachedSessionId && this.isSessionError(error)) {
+        logger.debug(`Session restoration failed for server ${config.id}, retrying fresh`)
+        this.clearCachedSessionId(config.id)
+
+        const freshClient = new McpClient(config, securityPolicy)
+        await freshClient.connect()
+
+        const freshSessionId = freshClient.getSessionId()
+        if (freshSessionId) {
+          this.cacheSessionId(config.id, freshSessionId)
+        }
+
+        return freshClient
+      }
+
+      throw error
+    }
+  }
+
+  private isSessionError(error: unknown): boolean {
+    if (error instanceof Error) {
+      const message = error.message.toLowerCase()
+      return (
+        message.includes('no valid session') ||
+        message.includes('invalid session') ||
+        message.includes('session expired')
+      )
+    }
+    return false
  }

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

+    logger.info(
+      `[${requestId}] Executing MCP tool ${toolCall.name} on server ${serverId} for user ${userId}`
+    )
+
+    const config = await this.getServerConfig(serverId, workspaceId)
+    if (!config) {
+      throw new Error(`Server ${serverId} not found or not accessible`)
+    }
+
+    const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
+
+    const client = await this.createClient(resolvedConfig)
+
    try {
-      logger.info(
-        `[${requestId}] Executing MCP tool ${toolCall.name} on server ${serverId} for user ${userId}`
-      )
-
-      const config = await this.getServerConfig(serverId, workspaceId)
-      if (!config) {
-        throw new Error(`Server ${serverId} not found or not accessible`)
-      }
-
-      const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
-
-      const client = await this.createClient(resolvedConfig)
-
-      try {
-        const result = await client.callTool(toolCall)
-        logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
-        return result
-      } finally {
-        await client.disconnect()
-      }
-    } catch (error) {
-      logger.error(
-        `[${requestId}] Failed to execute tool ${toolCall.name} on server ${serverId}:`,
-        error
-      )
-      throw error
+      const result = await client.callTool(toolCall)
+      logger.info(`[${requestId}] Successfully executed tool ${toolCall.name}`)
+      return result
+    } finally {
+      await client.disconnect()
    }
  }

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

+    logger.info(`[${requestId}] Discovering tools from server ${serverId} for user ${userId}`)
+
+    const config = await this.getServerConfig(serverId, workspaceId)
+    if (!config) {
+      throw new Error(`Server ${serverId} not found or not accessible`)
+    }
+
+    const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
+
+    const client = await this.createClient(resolvedConfig)
+
    try {
-      logger.info(`[${requestId}] Discovering tools from server ${serverId} for user ${userId}`)
-
-      const config = await this.getServerConfig(serverId, workspaceId)
-      if (!config) {
-        throw new Error(`Server ${serverId} not found or not accessible`)
-      }
-
-      const resolvedConfig = await this.resolveConfigEnvVars(config, userId, workspaceId)
-
-      const client = await this.createClient(resolvedConfig)
-
-      try {
-        const tools = await client.listTools()
-        logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
-        return tools
-      } finally {
-        await client.disconnect()
-      }
-    } catch (error) {
-      logger.error(`[${requestId}] Failed to discover tools from server ${serverId}:`, error)
-      throw error
+      const tools = await client.listTools()
+      logger.info(`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`)
+      return tools
+    } finally {
+      await client.disconnect()
    }
  }