improvement(mcp): align tool discovery timeouts and retries with MCP protocol standard

- Raise tools/list idle timeout 10s -> 30s (derived from per-server config,
  clamped to max execution timeout) with a 60s hard cap via maxTotalTimeout,
  matching the SDK's DEFAULT_REQUEST_TIMEOUT_MSEC. Enable resetTimeoutOnProgress
  with an onprogress handler so slow-but-alive servers are not spuriously failed.
- Retry read-only tools/list on transient transport errors (timeout,
  connection-closed, 429/5xx, session 404/400, network) with jittered backoff.
  tools/call stays conservative (session-error retry only) since it is not
  idempotent. The MCP SDK does not retry POST requests, so the app owns this.
- Wire client.onerror so out-of-band transport failures are observed, not
  silently dropped.
- Map timeouts to a user-facing message and surface refresh/remove failures via
  the standard emcn toast instead of swallowing them.

Author: waleedlatif1

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx

@@ -1,7 +1,7 @@
 'use client'
 
 import { useEffect, useRef, useState } from 'react'
-import { Badge, Button, Chip, ChipConfirmModal, cn, Tooltip } from '@sim/emcn'
+import { Badge, Button, Chip, ChipConfirmModal, cn, Tooltip, toast } from '@sim/emcn'
 import { ArrowLeft } from '@sim/emcn/icons'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
@@ -224,6 +224,7 @@ export function MCP() {
       logger.info(`Removed MCP server: ${serverId}`)
     } catch (error) {
       logger.error('Failed to remove MCP server:', error)
+      toast.error('Failed to remove MCP server', { description: getErrorMessage(error) })
     } finally {
       setDeletingServers((prev) => {
         const newSet = new Set(prev)
@@ -301,6 +302,7 @@ export function MCP() {
       }
     } catch (error) {
       logger.error('Failed to refresh MCP server:', error)
+      toast.error('Failed to refresh MCP server', { description: getErrorMessage(error) })
     }
   }
 

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

@@ -3,14 +3,15 @@
  */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockLogger, mockSdkConnect } = vi.hoisted(() => ({
+const { mockLogger, mockSdkConnect, mockSdkListTools } = vi.hoisted(() => ({
   mockLogger: {
     debug: vi.fn(),
     error: vi.fn(),
     info: vi.fn(),
     warn: vi.fn(),
   },
   mockSdkConnect: vi.fn().mockResolvedValue(undefined),
+  mockSdkListTools: vi.fn().mockResolvedValue({ tools: [] }),
 }))
 
 vi.mock('@sim/logger', () => ({
@@ -37,7 +38,7 @@ vi.mock('@modelcontextprotocol/sdk/client/index.js', () => ({
             .mockImplementation((_schema: unknown, handler: () => Promise<void>) => {
               capturedNotificationHandler = handler
             }),
-          listTools: vi.fn().mockResolvedValue({ tools: [] }),
+          listTools: mockSdkListTools,
         })
       }
     }
@@ -80,6 +81,7 @@ describe('McpClient notification handler', () => {
     capturedNotificationHandler = null
     vi.clearAllMocks()
     mockSdkConnect.mockResolvedValue(undefined)
+    mockSdkListTools.mockResolvedValue({ tools: [] })
   })
 
   it('fires onToolsChanged when a notification arrives while connected', async () => {
@@ -152,6 +154,26 @@ describe('McpClient notification handler', () => {
     expect(mockSdkConnect).toHaveBeenCalledWith(expect.anything(), { timeout: 30_000 })
   })
 
+  it('bounds tools/list with an idle timeout, hard cap, and progress reset', async () => {
+    const client = new McpClient({
+      config: createConfig(),
+      securityPolicy: { requireConsent: false, auditLevel: 'basic' },
+    })
+
+    await client.connect()
+    await client.listTools()
+
+    expect(mockSdkListTools).toHaveBeenCalledWith(
+      undefined,
+      expect.objectContaining({
+        timeout: 30_000,
+        maxTotalTimeout: 60_000,
+        resetTimeoutOnProgress: true,
+        onprogress: expect.any(Function),
+      })
+    )
+  })
+
   it('logs connection diagnostics without header values', async () => {
     const client = new McpClient({
       config: {

apps/sim/lib/mcp/client.ts

@@ -104,6 +104,17 @@ export class McpClient {
         capabilities: {},
       }
     )
+
+    // The SDK invokes onerror for out-of-band transport failures (SSE
+    // disconnects, exhausted reconnects) that are not tied to an in-flight
+    // request. Without a handler these are silently dropped, so wire one for
+    // observability. See @modelcontextprotocol/sdk Protocol.onerror.
+    this.client.onerror = (error) => {
+      logger.warn(`MCP transport error for ${this.config.name}`, {
+        serverId: this.config.id,
+        error: sanitizeForLogging(getErrorMessage(error, 'Unknown transport error'), 200),
+      })
+    }
   }
 
   /**
@@ -217,9 +228,34 @@ export class McpClient {
       throw new McpConnectionError('Not connected to server', this.config.name)
     }
 
+    const configuredTimeout = this.config.timeout
+    const idleTimeoutMs = Math.min(
+      configuredTimeout !== undefined && Number.isFinite(configuredTimeout) && configuredTimeout > 0
+        ? Math.floor(configuredTimeout)
+        : MCP_CLIENT_CONSTANTS.LIST_TOOLS_TIMEOUT_MS,
+      getMaxExecutionTimeout()
+    )
+    const maxTotalTimeoutMs = Math.max(
+      idleTimeoutMs,
+      MCP_CLIENT_CONSTANTS.LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS
+    )
+
     try {
       const result: ListToolsResult = await this.client.listTools(undefined, {
-        timeout: MCP_CLIENT_CONSTANTS.LIST_TOOLS_TIMEOUT_MS,
+        // `timeout` bounds silence between progress notifications; `maxTotalTimeout`
+        // is the hard ceiling regardless of progress. `resetTimeoutOnProgress` only
+        // takes effect when an `onprogress` callback is supplied (the SDK sets the
+        // request progressToken only then), so both are passed together.
+        timeout: idleTimeoutMs,
+        maxTotalTimeout: maxTotalTimeoutMs,
+        resetTimeoutOnProgress: true,
+        onprogress: (progress) => {
+          logger.debug(`Tool discovery progress from ${this.config.name}`, {
+            serverId: this.config.id,
+            progress: progress.progress,
+            total: progress.total,
+          })
+        },
       })
 
       if (!result.tools || !Array.isArray(result.tools)) {

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

@@ -400,7 +400,11 @@ describe('McpService.discoverTools per-server caching', () => {
 
   it('successful discoverServerTools clears the negative cache', async () => {
     mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
-    mockListTools.mockRejectedValueOnce(new Error('Request timed out'))
+    // A timeout is transient/retryable, so it must fail every attempt to reach
+    // the persisted-failure path.
+    mockListTools
+      .mockRejectedValueOnce(new Error('Request timed out'))
+      .mockRejectedValueOnce(new Error('Request timed out'))
 
     await expect(mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)).rejects.toThrow(
       'Request timed out'
@@ -450,7 +454,9 @@ describe('McpService.discoverTools per-server caching', () => {
         statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
       }),
     ])
-    mockListTools.mockRejectedValueOnce(new Error('Request timed out'))
+    mockListTools
+      .mockRejectedValueOnce(new Error('Request timed out'))
+      .mockRejectedValueOnce(new Error('Request timed out'))
 
     await expect(mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)).rejects.toThrow(
       'Request timed out'
@@ -459,12 +465,25 @@ describe('McpService.discoverTools per-server caching', () => {
     expect(mockUpdateSet).toHaveBeenCalledWith(
       expect.objectContaining({
         connectionStatus: 'disconnected',
-        lastError: 'Request timed out',
+        // Raw SDK timeout text is mapped to a user-facing message before persisting.
+        lastError: 'The MCP server took too long to respond and timed out',
         statusConfig: { consecutiveFailures: 1, lastSuccessfulDiscovery: null },
       })
     )
   })
 
+  it('retries a transient tools/list timeout and succeeds on the second attempt', async () => {
+    mockGetWorkspaceServersRows.mockResolvedValue([dbRow('mcp-a', 'A')])
+    mockListTools
+      .mockRejectedValueOnce(new Error('Request timed out'))
+      .mockResolvedValueOnce([tool('a1', 'mcp-a')])
+
+    const tools = await mcpService.discoverServerTools(USER_ID, 'mcp-a', WORKSPACE_ID)
+
+    expect(tools.map((t) => t.name)).toEqual(['a1'])
+    expect(mockListTools).toHaveBeenCalledTimes(2)
+  })
+
   it('persists and negative-caches per-server UnauthorizedError for headers auth', async () => {
     const reflectedCredential = 'Bearer static-secret-for-server-discovery'
     mockGetWorkspaceServersRows.mockResolvedValue([

apps/sim/lib/mcp/service.ts

@@ -1,10 +1,12 @@
 import { UnauthorizedError } from '@modelcontextprotocol/sdk/client/auth.js'
 import { StreamableHTTPError } from '@modelcontextprotocol/sdk/client/streamableHttp.js'
+import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'
 import { db } from '@sim/db'
 import { mcpServers } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { sleep } from '@sim/utils/helpers'
+import { backoffWithJitter } from '@sim/utils/retry'
 import { and, eq, isNull, lte, or } from 'drizzle-orm'
 import { isTest } from '@/lib/core/config/env-flags'
 import { generateRequestId } from '@/lib/core/utils/request'
@@ -85,9 +87,54 @@ function getDiscoveryFailureMessage(
   if (authType !== 'oauth' && error instanceof UnauthorizedError) {
     return 'Authentication failed'
   }
+  if (isTimeoutError(error)) {
+    return 'The MCP server took too long to respond and timed out'
+  }
   return getErrorMessage(error, fallback)
 }
 
+function isTimeoutError(error: unknown): boolean {
+  if (error instanceof McpError && error.code === ErrorCode.RequestTimeout) {
+    return true
+  }
+  return getErrorMessage(error, '').toLowerCase().includes('timed out')
+}
+
+/**
+ * Transient transport failures that a read-only `tools/list` may safely retry.
+ * `tools/list` is idempotent, so re-issuing it after a timeout, dropped
+ * connection, or 5xx/429 cannot double-apply side effects — unlike `tools/call`,
+ * which is never retried on these. OAuth-authorization and terminal 4xx errors
+ * are intentionally excluded: they need re-auth or a config fix, not a retry.
+ * The MCP TypeScript SDK does not retry POST requests itself (only SSE streams),
+ * so the app owns this.
+ */
+function isRetryableDiscoveryError(error: unknown): boolean {
+  if (isTimeoutError(error)) return true
+  if (error instanceof McpError) {
+    return error.code === ErrorCode.ConnectionClosed
+  }
+  if (error instanceof StreamableHTTPError) {
+    // 404/400 = stale/malformed session id: retrying rebuilds the client with no
+    // session id, which re-initializes per MCP spec. 429/5xx = transient upstream.
+    const code = error.code
+    return (
+      code === 404 ||
+      code === 400 ||
+      code === 429 ||
+      (typeof code === 'number' && code >= 500 && code <= 599)
+    )
+  }
+  const message = getErrorMessage(error, '').toLowerCase()
+  return (
+    message.includes('econnreset') ||
+    message.includes('socket hang up') ||
+    message.includes('etimedout') ||
+    message.includes('fetch failed') ||
+    message.includes('network')
+  )
+}
+
 class McpService {
   private cacheAdapter: McpCacheStorageAdapter
   private readonly cacheTimeout = MCP_CONSTANTS.CACHE_TIMEOUT
@@ -772,12 +819,12 @@ class McpService {
           await client.disconnect()
         }
       } catch (error) {
-        if (this.isSessionError(error) && attempt < maxRetries - 1) {
+        if (isRetryableDiscoveryError(error) && attempt < maxRetries - 1) {
           logger.warn(
-            `[${requestId}] Session error discovering tools from server ${serverId}, retrying (attempt ${attempt + 1}):`,
+            `[${requestId}] Transient error discovering tools from server ${serverId}, retrying (attempt ${attempt + 1}):`,
             error
           )
-          await sleep(100)
+          await sleep(backoffWithJitter(attempt + 1, null, { baseMs: 250, maxMs: 2000 }))
           continue
         }
         // Drop positive cache so a follow-up doesn't return stale tools.

apps/sim/lib/mcp/utils.ts

@@ -51,7 +51,20 @@ export function sanitizeHeaders(
 export const MCP_CLIENT_CONSTANTS = {
   CLIENT_TIMEOUT: DEFAULT_EXECUTION_TIMEOUT_MS,
   AUTO_REFRESH_INTERVAL: 5 * 60 * 1000,
-  LIST_TOOLS_TIMEOUT_MS: 10_000,
+  /**
+   * Idle timeout for a `tools/list` request. Raised from the former aggressive
+   * 10s toward the MCP TypeScript SDK's 60s default (`DEFAULT_REQUEST_TIMEOUT_MSEC`)
+   * so a legitimately slow-to-enumerate server is not spuriously failed with
+   * `McpError -32001`. Combined with `resetTimeoutOnProgress`, this is the gap
+   * allowed between progress notifications, not the absolute ceiling.
+   */
+  LIST_TOOLS_TIMEOUT_MS: 30_000,
+  /**
+   * Absolute ceiling for a `tools/list` request regardless of progress, mirroring
+   * the SDK's `maxTotalTimeout` safeguard. Matches the SDK's 60s default so a
+   * server emitting continuous progress still cannot hang discovery indefinitely.
+   */
+  LIST_TOOLS_MAX_TOTAL_TIMEOUT_MS: 60_000,
   FAILURE_CACHE_TTL_MS: 120_000,
 } as const