fix(mcp): stabilize authenticated tool discovery

apps/sim/app/api/mcp/oauth/start/route.test.ts

@@ -162,6 +162,23 @@ describe('MCP OAuth start route', () => {
     expect(body.error).not.toContain('ECONNREFUSED')
     expect(body.error).not.toContain('internal-db-host')
   })
+
+  it('returns an actionable 4xx when the server does not support dynamic client registration', async () => {
+    mcpOauthMockFns.mockMcpAuthGuarded.mockRejectedValueOnce(
+      new Error('Incompatible auth server: does not support dynamic client registration')
+    )
+    const request = new NextRequest(
+      'http://localhost:3000/api/mcp/oauth/start?workspaceId=workspace-1&serverId=server-1'
+    )
+
+    const response = await GET(request)
+    const body = await response.json()
+
+    expect(response.status).toBe(422)
+    expect(body.error).toBe(
+      "This server doesn't support OAuth client registration. Configure a token instead."
+    )
+  })
 })
 
 describe('surfaceOauthError', () => {

apps/sim/app/api/mcp/oauth/start/route.ts

@@ -2,7 +2,7 @@ import { OAuthError, ServerError } from '@modelcontextprotocol/sdk/server/auth/e
 import { db } from '@sim/db'
 import { mcpServers } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { toError } from '@sim/utils/errors'
+import { getErrorMessage, toError } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
@@ -25,6 +25,14 @@ import { createMcpErrorResponse } from '@/lib/mcp/utils'
 const logger = createLogger('McpOauthStartAPI')
 const OAUTH_START_TTL_MS = 10 * 60 * 1000
 const MAX_SURFACED_ERROR_LENGTH = 250
+const DCR_UNSUPPORTED_MESSAGE =
+  "This server doesn't support OAuth client registration. Configure a token instead."
+
+function isDynamicClientRegistrationUnsupported(error: unknown): boolean {
+  return getErrorMessage(error, '')
+    .toLowerCase()
+    .includes('does not support dynamic client registration')
+}
 
 export function surfaceOauthError(error: unknown): string {
   // Spec-compliant OAuth servers throw typed subclasses with clean RFC 6749 fields.
@@ -148,6 +156,9 @@ export const GET = withRouteHandler(
             authorizationUrl: e.authorizationUrl,
           })
         }
+        if (isDynamicClientRegistrationUnsupported(e)) {
+          return createMcpErrorResponse(toError(e), DCR_UNSUPPORTED_MESSAGE, 422)
+        }
         throw e
       }
     } catch (error) {

apps/sim/app/api/mcp/servers/[id]/refresh/route.test.ts

@@ -4,17 +4,19 @@
 import type { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockClearCache, mockDiscoverServerTools, mockSelect, mockUpdateSet } = vi.hoisted(() => ({
-  mockClearCache: vi.fn(),
-  mockDiscoverServerTools: vi.fn(),
-  mockSelect: vi.fn(),
-  mockUpdateSet: vi.fn(),
-}))
+const { mockClearCache, mockDiscoverServerTools, mockSelect, mockUpdate, mockUpdateSet } =
+  vi.hoisted(() => ({
+    mockClearCache: vi.fn(),
+    mockDiscoverServerTools: vi.fn(),
+    mockSelect: vi.fn(),
+    mockUpdate: vi.fn(),
+    mockUpdateSet: vi.fn(),
+  }))
 
 vi.mock('@sim/db', () => ({
   db: {
     select: mockSelect,
-    update: vi.fn().mockReturnValue({ set: mockUpdateSet }),
+    update: mockUpdate.mockReturnValue({ set: mockUpdateSet }),
   },
 }))
 
@@ -43,7 +45,7 @@ vi.mock('@/lib/mcp/middleware', () => ({
 vi.mock('@/lib/mcp/service', () => ({
   mcpService: {
     clearCache: mockClearCache,
-    discoverServerTools: mockDiscoverServerTools,
+    discoverServerToolsWithMetadata: mockDiscoverServerTools,
   },
 }))
 
@@ -57,6 +59,7 @@ const initialServer = {
   connectionStatus: 'connected',
   lastError: null,
   lastConnected: new Date('2026-01-01T00:00:00.000Z'),
+  lastToolsRefresh: new Date('2026-01-01T00:00:00.000Z'),
   toolCount: 4,
   statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
 }
@@ -81,6 +84,8 @@ function selectRows(rows: unknown[]) {
 describe('MCP server refresh route', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    mockSelect.mockReset()
+    mockSelect.mockReturnValue(selectRows([persistedServer]))
     mockSelect.mockReturnValueOnce(selectRows([initialServer]))
     mockUpdateSet.mockReturnValue({
       where: vi.fn().mockReturnValue({ returning: vi.fn().mockResolvedValue([persistedServer]) }),
@@ -112,11 +117,7 @@ describe('MCP server refresh route', () => {
     mockDiscoverServerTools.mockRejectedValueOnce(
       new Error(`Upstream reflected ${reflectedSecret}`)
     )
-    mockUpdateSet.mockReturnValueOnce({
-      where: vi.fn().mockReturnValue({
-        returning: vi.fn().mockResolvedValue([initialServer]),
-      }),
-    })
+    mockSelect.mockReturnValueOnce(selectRows([initialServer]))
 
     const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
       method: 'POST',
@@ -128,6 +129,7 @@ describe('MCP server refresh route', () => {
       expect.objectContaining({
         status: 'disconnected',
         error: 'Internal server error',
+        toolCount: 0,
         workflowsUpdated: 0,
       })
     )
@@ -140,13 +142,10 @@ describe('MCP server refresh route', () => {
     const newerSuccessfulServer = {
       ...initialServer,
       lastConnected: new Date(Date.now() + 60_000),
+      lastToolsRefresh: new Date(Date.now() + 60_000),
       toolCount: 7,
     }
-    mockUpdateSet.mockReturnValueOnce({
-      where: vi.fn().mockReturnValue({
-        returning: vi.fn().mockResolvedValue([newerSuccessfulServer]),
-      }),
-    })
+    mockSelect.mockReturnValueOnce(selectRows([newerSuccessfulServer]))
 
     const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
       method: 'POST',
@@ -162,25 +161,26 @@ describe('MCP server refresh route', () => {
         workflowsUpdated: 0,
       })
     )
-    expect(mockClearCache).toHaveBeenCalledWith('workspace-1')
+    expect(mockClearCache).not.toHaveBeenCalled()
   })
 
   it('does not 500 when workflow sync fails after a successful discovery', async () => {
-    mockDiscoverServerTools.mockResolvedValueOnce([
-      {
-        name: 'search',
-        description: 'Search tool',
-        inputSchema: {},
-        serverId: 'server-1',
-        serverName: 'OAuth Server',
-      },
-    ])
+    mockDiscoverServerTools.mockResolvedValueOnce({
+      tools: [
+        {
+          name: 'search',
+          description: 'Search tool',
+          inputSchema: {},
+          serverId: 'server-1',
+          serverName: 'OAuth Server',
+        },
+      ],
+      state: 'published',
+    })
     // The route's server lookup consumes the first select (beforeEach). The sync's
     // workflow select is left unmocked, so it throws — exercising the guard that
     // keeps a secondary sync failure from turning a successful refresh into a 500.
-    mockUpdateSet.mockReturnValueOnce({
-      where: vi.fn().mockReturnValue({ returning: vi.fn().mockResolvedValue([initialServer]) }),
-    })
+    mockSelect.mockReturnValueOnce(selectRows([initialServer]))
 
     const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
       method: 'POST',
@@ -197,4 +197,60 @@ describe('MCP server refresh route', () => {
       })
     )
   })
+
+  it('reports live tools when cache degradation prevents status publication', async () => {
+    mockDiscoverServerTools.mockResolvedValueOnce({
+      tools: [
+        {
+          name: 'search',
+          description: 'Search tool',
+          inputSchema: {},
+          serverId: 'server-1',
+          serverName: 'OAuth Server',
+        },
+      ],
+      state: 'unavailable',
+    })
+    mockSelect.mockReturnValueOnce(selectRows([persistedServer]))
+
+    const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
+      method: 'POST',
+    }) as NextRequest
+    const response = await POST(request, { params: Promise.resolve({ id: 'server-1' }) })
+    const body = await response.json()
+
+    expect(body.data).toEqual(
+      expect.objectContaining({
+        status: 'connected',
+        error: null,
+        toolCount: 1,
+      })
+    )
+  })
+
+  it('fails closed without syncing workflows when discovery is superseded', async () => {
+    mockDiscoverServerTools.mockResolvedValueOnce({
+      tools: [],
+      state: 'superseded',
+    })
+    mockSelect.mockReturnValueOnce(selectRows([initialServer]))
+
+    const request = new Request('http://localhost/api/mcp/servers/server-1/refresh', {
+      method: 'POST',
+    }) as NextRequest
+    const response = await POST(request, { params: Promise.resolve({ id: 'server-1' }) })
+    const body = await response.json()
+
+    expect(body.data).toEqual(
+      expect.objectContaining({
+        status: 'disconnected',
+        error: 'Tool discovery was superseded by a newer refresh. Please retry.',
+        toolCount: 0,
+        workflowsUpdated: 0,
+        updatedWorkflowIds: [],
+      })
+    )
+    expect(mockSelect).toHaveBeenCalledTimes(2)
+    expect(mockUpdate).not.toHaveBeenCalled()
+  })
 })

apps/sim/app/api/mcp/servers/[id]/refresh/route.ts

@@ -9,7 +9,7 @@ import { mcpServerIdParamsSchema } from '@/lib/api/contracts/mcp'
 import { validationErrorResponse } from '@/lib/api/server'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { withMcpAuth } from '@/lib/mcp/middleware'
-import { mcpService } from '@/lib/mcp/service'
+import { type McpServerDiscoveryState, mcpService } from '@/lib/mcp/service'
 import type { McpTool, McpToolSchema } from '@/lib/mcp/types'
 import {
   categorizeError,
@@ -188,16 +188,18 @@ export const POST = withRouteHandler(
 
         let syncResult: SyncResult = { updatedCount: 0, updatedWorkflowIds: [] }
         let discoveredTools: McpTool[] = []
+        let discoveryState: McpServerDiscoveryState | null = null
         let discoveryError: string | null = null
-        const discoveryStartedAt = new Date()
 
         try {
-          discoveredTools = await mcpService.discoverServerTools(
+          const discovery = await mcpService.discoverServerToolsWithMetadata(
             userId,
             serverId,
             workspaceId,
             true
           )
+          discoveredTools = discovery.tools
+          discoveryState = discovery.state
           logger.info(
             `[${requestId}] Discovered ${discoveredTools.length} tools from server ${serverId}`
           )
@@ -208,14 +210,37 @@ export const POST = withRouteHandler(
           })
         }
 
-        if (discoveryError === null) {
+        const [refreshedServer] = await db
+          .select({
+            name: mcpServers.name,
+            url: mcpServers.url,
+            connectionStatus: mcpServers.connectionStatus,
+            lastConnected: mcpServers.lastConnected,
+            lastToolsRefresh: mcpServers.lastToolsRefresh,
+            lastError: mcpServers.lastError,
+            toolCount: mcpServers.toolCount,
+          })
+          .from(mcpServers)
+          .where(
+            and(
+              eq(mcpServers.id, serverId),
+              eq(mcpServers.workspaceId, workspaceId),
+              isNull(mcpServers.deletedAt)
+            )
+          )
+          .limit(1)
+
+        if (discoveryError === null && discoveryState !== 'superseded') {
           try {
             syncResult = await syncToolSchemasToWorkflows(
               workspaceId,
               serverId,
               discoveredTools,
               requestId,
-              { url: server.url ?? undefined, name: server.name ?? undefined }
+              {
+                url: refreshedServer?.url ?? undefined,
+                name: refreshedServer?.name ?? undefined,
+              }
             )
           } catch (error) {
             // Discovery already persisted status and cached tools; a workflow-sync
@@ -227,47 +252,36 @@ export const POST = withRouteHandler(
           }
         }
 
-        const now = new Date()
-
-        const [refreshedServer] = await db
-          .update(mcpServers)
-          .set({
-            lastToolsRefresh: now,
-            updatedAt: now,
-          })
-          .where(
-            and(
-              eq(mcpServers.id, serverId),
-              eq(mcpServers.workspaceId, workspaceId),
-              isNull(mcpServers.deletedAt)
-            )
-          )
-          .returning({
-            connectionStatus: mcpServers.connectionStatus,
-            lastConnected: mcpServers.lastConnected,
-            lastError: mcpServers.lastError,
-            toolCount: mcpServers.toolCount,
-          })
-
         let connectionStatus = refreshedServer?.connectionStatus ?? 'error'
         let lastError = refreshedServer ? refreshedServer.lastError : discoveryError
-        const toolCount = refreshedServer?.toolCount ?? discoveredTools.length
+        let toolCount = refreshedServer?.toolCount ?? discoveredTools.length
+
+        if (discoveryState === 'superseded') {
+          connectionStatus = 'disconnected'
+          lastError = 'Tool discovery was superseded by a newer refresh. Please retry.'
+          toolCount = 0
+        } else if (
+          discoveryError === null &&
+          (discoveryState === 'unavailable' || discoveryState === 'winner-cache')
+        ) {
+          connectionStatus = 'connected'
+          lastError = null
+          toolCount = discoveredTools.length
+        }
 
         if (discoveryError !== null && connectionStatus === 'connected') {
           const newerSuccessWonRace =
-            refreshedServer?.lastConnected != null &&
-            refreshedServer.lastConnected > discoveryStartedAt
+            refreshedServer?.lastToolsRefresh != null &&
+            (server.lastToolsRefresh == null ||
+              refreshedServer.lastToolsRefresh > server.lastToolsRefresh)
 
           if (!newerSuccessWonRace) {
             connectionStatus = 'disconnected'
             lastError = discoveryError
+            toolCount = 0
           }
         }
 
-        if (connectionStatus === 'connected') {
-          await mcpService.clearCache(workspaceId)
-        }
-
         return createMcpSuccessResponse({
           status: connectionStatus,
           toolCount,