fix(mcp): reset connection on any auth-type flip, scope h2 to live transport, keep tools/list diagnostics

Author: waleedlatif1

apps/sim/lib/mcp/client.ts

@@ -10,6 +10,7 @@ import {
 } from '@modelcontextprotocol/sdk/types.js'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
+import { truncate } from '@sim/utils/string'
 import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
 import { getMcpSafeErrorDiagnostics } from '@/lib/mcp/error-diagnostics'
 import { McpOauthRedirectRequired } from '@/lib/mcp/oauth'
@@ -278,6 +279,10 @@ export class McpClient {
         maxTotalTimeoutMs,
         sessionIdPresent: Boolean(this.transport.sessionId),
         error: getMcpSafeErrorDiagnostics(error),
+        // tools/list runs after auth succeeded, so its error text (schema/protocol/
+        // upstream) is the actionable diagnostic and is very unlikely to carry a
+        // credential — include a bounded message the structural fields can't convey.
+        message: truncate(getErrorMessage(error, ''), 300),
       })
       throw error
     }

apps/sim/lib/mcp/oauth/probe.test.ts

@@ -15,8 +15,10 @@ const { mockCreatePinnedFetch, mockCreateSsrfGuardedMcpFetch, mockPinnedFetch, m
     }
   })
 
+vi.mock('@/lib/core/security/input-validation.server', () => ({
+  createPinnedFetch: mockCreatePinnedFetch,
+}))
 vi.mock('@/lib/mcp/pinned-fetch', () => ({
-  createPinnedMcpFetch: mockCreatePinnedFetch,
   createSsrfGuardedMcpFetch: mockCreateSsrfGuardedMcpFetch,
 }))
 

apps/sim/lib/mcp/oauth/probe.ts

@@ -1,8 +1,9 @@
 import { extractWWWAuthenticateParams } from '@modelcontextprotocol/sdk/client/auth.js'
 import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
 import { createLogger } from '@sim/logger'
+import { createPinnedFetch } from '@/lib/core/security/input-validation.server'
 import { isLoopbackHostname } from '@/lib/core/utils/urls'
-import { createPinnedMcpFetch, createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
+import { createSsrfGuardedMcpFetch } from '@/lib/mcp/pinned-fetch'
 import type { McpAuthType } from '@/lib/mcp/types'
 
 const logger = createLogger('McpOauthProbe')
@@ -33,7 +34,7 @@ export async function detectMcpAuthType(
   }
 
   const probeFetch: FetchLike = resolvedIP
-    ? createPinnedMcpFetch(resolvedIP)
+    ? createPinnedFetch(resolvedIP)
     : createSsrfGuardedMcpFetch()
 
   const controller = new AbortController()

apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts

@@ -91,8 +91,52 @@ describe('MCP server lifecycle orchestration', () => {
     // Flipping to OAuth must reset the server to disconnected — it hasn't
     // completed an auth flow, so it can't remain 'connected'.
     expect(dbChainMockFns.set).toHaveBeenCalledWith(
-      expect.objectContaining({ connectionStatus: 'disconnected', lastConnected: null })
+      expect.objectContaining({
+        connectionStatus: 'disconnected',
+        lastConnected: null,
+        lastError: null,
+      })
     )
     expect(mockClearCache).toHaveBeenCalledWith('workspace-1')
   })
+
+  it('resets an OAuth server to disconnected when its auth type flips to headers', async () => {
+    dbChainMockFns.limit.mockResolvedValueOnce([
+      {
+        url: 'https://example.com/mcp',
+        authType: 'oauth',
+        oauthClientId: 'client-1',
+        oauthClientSecret: 'secret-1',
+      },
+    ])
+    dbChainMockFns.returning.mockResolvedValueOnce([
+      {
+        id: 'server-1',
+        workspaceId: 'workspace-1',
+        name: 'Example',
+        transport: 'streamable-http',
+        url: 'https://example.com/mcp',
+        authType: 'headers',
+      },
+    ])
+
+    const result = await performUpdateMcpServer({
+      workspaceId: 'workspace-1',
+      userId: 'user-1',
+      serverId: 'server-1',
+      authType: 'headers',
+    })
+
+    expect(result.success).toBe(true)
+    // Flipping away from OAuth invalidates any prior connection — it must not
+    // keep reading as 'connected' with a stale lastError until re-discovery.
+    expect(dbChainMockFns.set).toHaveBeenCalledWith(
+      expect.objectContaining({
+        authType: 'headers',
+        connectionStatus: 'disconnected',
+        lastConnected: null,
+        lastError: null,
+      })
+    )
+  })
 })

apps/sim/lib/mcp/orchestration/server-lifecycle.ts

@@ -352,12 +352,14 @@ export async function performUpdateMcpServer(
     const shouldClearOauth = urlChanged || credsChanged
     const resolvedAuthType = (updateData.authType ?? currentServer.authType) as McpAuthType
     const authTypeChanged = resolvedAuthType !== currentServer.authType
-    // Switching a server to OAuth (via creds/URL change or a plain authType flip)
-    // must reset it to disconnected: an OAuth server has not completed its auth
-    // flow, so it can't legitimately remain 'connected' (matches the create path).
-    if ((shouldClearOauth || authTypeChanged) && resolvedAuthType === 'oauth') {
+    // Any auth-type change (either direction) or an OAuth creds/URL change
+    // invalidates the current connection: reset to disconnected and clear stale
+    // connection/error state so the server can't falsely read as connected (or
+    // show a stale error) until the next discovery re-evaluates it.
+    if (authTypeChanged || (shouldClearOauth && resolvedAuthType === 'oauth')) {
       updateData.connectionStatus = 'disconnected'
       updateData.lastConnected = null
+      updateData.lastError = null
     }
 
     if (shouldClearOauth) await revokeMcpOauthTokens(params.serverId)

apps/sim/lib/mcp/pinned-fetch.test.ts

@@ -31,7 +31,7 @@ describe('createSsrfGuardedMcpFetch', () => {
     await fetchLike('https://attacker.example/revoke', { method: 'POST' })
 
     expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith('https://attacker.example/revoke')
-    expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10', { allowH2: true })
+    expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10')
     expect(sentinelFetch).toHaveBeenCalledWith('https://attacker.example/revoke', {
       method: 'POST',
     })
@@ -54,7 +54,7 @@ describe('createSsrfGuardedMcpFetch', () => {
     await fetchLike(new URL('https://attacker.example/discover'))
 
     expect(mockValidateMcpServerSsrf).toHaveBeenCalledWith('https://attacker.example/discover')
-    expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10', { allowH2: true })
+    expect(mockCreatePinnedFetch).toHaveBeenCalledWith('203.0.113.10')
   })
 
   it('falls back to global fetch when validation returns no IP', async () => {

apps/sim/lib/mcp/pinned-fetch.ts

@@ -3,12 +3,13 @@ import { createPinnedFetch } from '@/lib/core/security/input-validation.server'
 import { validateMcpServerSsrf } from '@/lib/mcp/domain-check'
 
 /**
- * Pinned fetch for all MCP traffic. MCP servers are commonly deployed behind
- * HTTP/2 fronts (CDNs, cloud LBs), and undici's Agent is h1.1-only unless opted
- * into h2 via ALPN — so every MCP connection enables it. This is the single
- * source of the "MCP implies h2" decision; the base `createPinnedFetch` keeps
- * its h1.1 default for non-MCP consumers. Pinning is unaffected: the pinned
- * lookup forces the socket to `resolvedIP` regardless of negotiated protocol.
+ * Pinned fetch for the long-lived MCP transport, which reuses one Agent across
+ * a connection's requests. MCP servers are commonly behind HTTP/2 fronts (CDNs,
+ * cloud LBs), and undici's Agent is h1.1-only unless opted into h2 via ALPN, so
+ * the transport enables it. h2 is *not* used for one-shot flows (OAuth discovery,
+ * auth-type probe), where a per-request Agent would leave idle h2 sessions with
+ * no reuse benefit. Pinning is unaffected: the pinned lookup forces the socket to
+ * `resolvedIP` regardless of negotiated protocol.
  */
 export function createPinnedMcpFetch(resolvedIP: string): typeof fetch {
   return createPinnedFetch(resolvedIP, { allowH2: true })
@@ -37,7 +38,7 @@ export function createSsrfGuardedMcpFetch(): FetchLike {
   return (async (url, init) => {
     const target = typeof url === 'string' ? url : url.href
     const resolvedIP = await validateMcpServerSsrf(target)
-    const pinnedFetch: FetchLike = resolvedIP ? createPinnedMcpFetch(resolvedIP) : globalThis.fetch
+    const pinnedFetch: FetchLike = resolvedIP ? createPinnedFetch(resolvedIP) : globalThis.fetch
     return pinnedFetch(url, init)
   }) satisfies FetchLike
 }