fix(mcp): keep a legacy server row from blanking the whole server list (#5762)

Response-side Zod .catch() tolerance on the three strict-enum-over-free-text MCP columns (transport/authType/connectionStatus) so one legacy row can't fail the whole server-list validation; fork copy normalizes transport; create/upsert path resets connection status on any auth-type flip (mirrors update path); bulk discovery drops the positive tool cache on OAuth-pending. Request bodies stay strict.

Author: waleedlatif1

apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts

@@ -395,6 +395,9 @@ export async function copyForkResourceContainers(
         createdBy: userId,
         url: typeof row.url === 'string' ? rewriteEnv(row.url) : row.url,
         headers,
+        // Normalize legacy `http`/`sse` transports to the only supported value so a
+        // forked row never carries a transport the API contract would reject.
+        transport: 'streamable-http',
         connectionStatus: 'disconnected',
         lastConnected: null,
         lastError: null,

apps/sim/lib/api/contracts/mcp.ts

@@ -26,7 +26,12 @@ const optionalNumberFromNullableSchema = z.preprocess(
 
 const optionalConnectionStatusFromNullableSchema = z.preprocess(
   (value) => (value === null ? undefined : value),
-  z.enum(['connected', 'disconnected', 'error']).optional()
+  // `connection_status` is a free-text column; tolerate an off-enum value as undefined
+  // rather than failing the whole list's validation.
+  z
+    .enum(['connected', 'disconnected', 'error'])
+    .optional()
+    .catch(undefined)
 )
 
 const optionalHeadersFromNullableSchema = z.preprocess(
@@ -36,6 +41,16 @@ const optionalHeadersFromNullableSchema = z.preprocess(
 
 export const mcpTransportSchema = z.enum(['streamable-http'])
 
+/**
+ * Transport as read back from storage. The `transport` column is free text, and
+ * rows predating the Streamable HTTP consolidation (or copied verbatim by an
+ * older fork) may still hold legacy `http`/`sse` values. Every server is operated
+ * over Streamable HTTP regardless, so any non-canonical value normalizes to the
+ * supported transport — this stops a single legacy row from failing the entire
+ * server list's response validation.
+ */
+const mcpTransportResponseSchema = mcpTransportSchema.catch('streamable-http')
+
 export const mcpAuthTypeSchema = z.enum(['none', 'headers', 'oauth'])
 
 const consecutiveFailuresSchema = z.preprocess(
@@ -98,8 +113,10 @@ export const mcpServerSchema = z
     workspaceId: z.string(),
     name: z.string(),
     description: optionalStringFromNullableSchema,
-    transport: mcpTransportSchema,
-    authType: mcpAuthTypeSchema.optional(),
+    transport: mcpTransportResponseSchema,
+    // Response-side tolerance: `auth_type` is a free-text column, so a value outside
+    // the enum normalizes to undefined rather than failing the whole list's validation.
+    authType: mcpAuthTypeSchema.optional().catch(undefined),
     url: optionalStringFromNullableSchema,
     timeout: optionalNumberFromNullableSchema,
     retries: optionalNumberFromNullableSchema,

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

@@ -14,13 +14,19 @@ import {
 } from '@sim/testing'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockClearCache, mockOauthCredsChanged, mockRevokeOauthTokens, mockEvictServerConnections } =
-  vi.hoisted(() => ({
-    mockClearCache: vi.fn(),
-    mockOauthCredsChanged: vi.fn(),
-    mockRevokeOauthTokens: vi.fn(),
-    mockEvictServerConnections: vi.fn(),
-  }))
+const {
+  mockClearCache,
+  mockOauthCredsChanged,
+  mockRevokeOauthTokens,
+  mockEvictServerConnections,
+  mockGenerateMcpServerId,
+} = vi.hoisted(() => ({
+  mockClearCache: vi.fn(),
+  mockOauthCredsChanged: vi.fn(),
+  mockRevokeOauthTokens: vi.fn(),
+  mockEvictServerConnections: vi.fn(),
+  mockGenerateMcpServerId: vi.fn(),
+}))
 
 vi.mock('@sim/audit', () => auditMock)
 vi.mock('@sim/db', () => ({
@@ -52,10 +58,11 @@ vi.mock('@/lib/mcp/service', () => ({
     evictServerConnections: mockEvictServerConnections,
   },
 }))
-vi.mock('@/lib/mcp/utils', () => ({ generateMcpServerId: vi.fn() }))
+vi.mock('@/lib/mcp/utils', () => ({ generateMcpServerId: mockGenerateMcpServerId }))
 vi.mock('@/lib/posthog/server', () => posthogServerMock)
 
 import {
+  performCreateMcpServer,
   performDeleteMcpServer,
   performUpdateMcpServer,
 } from '@/lib/mcp/orchestration/server-lifecycle'
@@ -149,6 +156,42 @@ describe('MCP server lifecycle orchestration', () => {
     expect(mockRevokeOauthTokens).toHaveBeenCalledWith('server-1')
   })
 
+  it('resets to disconnected when a create/upsert flips an existing OAuth server to headers', async () => {
+    mockGenerateMcpServerId.mockReturnValue('server-1')
+    dbChainMockFns.limit.mockResolvedValueOnce([
+      {
+        id: 'server-1',
+        deletedAt: null,
+        url: 'https://example.com/mcp',
+        authType: 'oauth',
+        oauthClientId: 'client-1',
+        oauthClientSecret: 'secret-1',
+      },
+    ])
+
+    const result = await performCreateMcpServer({
+      workspaceId: 'workspace-1',
+      userId: 'user-1',
+      name: 'Example',
+      url: 'https://example.com/mcp',
+      authType: 'headers',
+    })
+
+    expect(result.success).toBe(true)
+    // Upsert must mirror the update path: an auth-type flip resets to disconnected and clears the
+    // stale error instead of optimistically marking the server connected.
+    expect(dbChainMockFns.set).toHaveBeenCalledWith(
+      expect.objectContaining({
+        authType: 'headers',
+        connectionStatus: 'disconnected',
+        lastConnected: null,
+        lastError: null,
+      })
+    )
+    // ...and revoke the now-orphaned OAuth tokens.
+    expect(mockRevokeOauthTokens).toHaveBeenCalledWith('server-1')
+  })
+
   it('evicts the deleted server from the connection pool (row is already gone from clearCache)', async () => {
     dbChainMockFns.returning.mockResolvedValueOnce([
       { id: 'server-1', workspaceId: 'workspace-1', name: 'Example', transport: 'streamable-http' },

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

@@ -169,7 +169,10 @@ export async function performCreateMcpServer(
         currentEncryptedClientSecret: existingServer.oauthClientSecret,
       })
       const isRevival = existingServer.deletedAt !== null
-      const shouldClearOauth = urlChanged || credsChanged || isRevival
+      const authTypeChanged = existingServer.authType !== resolvedAuthType
+      // Turning OAuth off orphans its tokens; revoke and delete them, mirroring the update path.
+      const oauthDisabled = existingServer.authType === 'oauth' && resolvedAuthType !== 'oauth'
+      const shouldClearOauth = urlChanged || credsChanged || isRevival || oauthDisabled
 
       if (shouldClearOauth) await revokeMcpOauthTokens(serverId)
 
@@ -190,12 +193,16 @@ export async function performCreateMcpServer(
           updatedAt: new Date(),
           deletedAt: null,
         }
-        if (resolvedAuthType === 'oauth') {
-          if (shouldClearOauth) {
-            updateValues.connectionStatus = 'disconnected'
-            updateValues.lastConnected = null
-          }
-        } else {
+        if (authTypeChanged || (shouldClearOauth && resolvedAuthType === 'oauth')) {
+          // An auth-type flip, or an OAuth URL/creds change, invalidates any prior connection:
+          // reset to disconnected and clear the stale error so the UI never shows
+          // connected-with-error until re-discovery. Mirrors performUpdateMcpServer.
+          updateValues.connectionStatus = 'disconnected'
+          updateValues.lastConnected = null
+          updateValues.lastError = null
+        } else if (resolvedAuthType !== 'oauth') {
+          // A non-OAuth (re-)registration with unchanged auth optimistically marks the server
+          // reachable; discovery corrects it if the endpoint is unhealthy.
           updateValues.connectionStatus = 'connected'
           updateValues.lastConnected = new Date()
         }

apps/sim/lib/mcp/service.ts

@@ -760,11 +760,20 @@ class McpService {
           return
         }
         if (outcome.kind === 'oauth-pending') {
-          // Mark disconnected so the UI surfaces the re-auth button.
+          // Mark disconnected so the UI surfaces the re-auth button, and drop the positive
+          // tool cache so a follow-up force-refresh can't serve tools for a server that now
+          // needs re-auth (mirrors the single-server discovery path).
           logger.info(`[${requestId}] Skipping server ${server.name}: OAuth authorization pending`)
           deferredSideEffects.push(
             this.markServerOauthPending(server.id, workspaceId, discoveryStartedAt).then(
-              () => undefined
+              async (statusApplied) => {
+                if (!statusApplied) return
+                await this.cacheAdapter
+                  .delete(serverCacheKey(workspaceId, server.id))
+                  .catch((err) =>
+                    logger.warn(`[${requestId}] Cache delete failed for ${server.name}:`, err)
+                  )
+              }
             )
           )
           return