fix(mcp): recover cleanly from OAuth failures

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

@@ -0,0 +1,108 @@
+/**
+ * @vitest-environment node
+ */
+import type { NextRequest } from 'next/server'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockDiscoverServerTools, mockSelect, mockUpdateSet } = vi.hoisted(() => ({
+  mockDiscoverServerTools: vi.fn(),
+  mockSelect: vi.fn(),
+  mockUpdateSet: vi.fn(),
+}))
+
+vi.mock('@sim/db', () => ({
+  db: {
+    select: mockSelect,
+    update: vi.fn().mockReturnValue({ set: mockUpdateSet }),
+  },
+}))
+
+vi.mock('@/lib/core/utils/with-route-handler', () => ({
+  withRouteHandler: (handler: unknown) => handler,
+}))
+
+vi.mock('@/lib/mcp/middleware', () => ({
+  withMcpAuth:
+    () =>
+    (
+      handler: (
+        request: NextRequest,
+        context: { userId: string; workspaceId: string; requestId: string },
+        routeContext: { params: Promise<{ id: string }> }
+      ) => Promise<Response>
+    ) =>
+    (request: NextRequest, routeContext: { params: Promise<{ id: string }> }) =>
+      handler(
+        request,
+        { userId: 'user-1', workspaceId: 'workspace-1', requestId: 'request-1' },
+        routeContext
+      ),
+}))
+
+vi.mock('@/lib/mcp/service', () => ({
+  mcpService: {
+    clearCache: vi.fn(),
+    discoverServerTools: mockDiscoverServerTools,
+  },
+}))
+
+import { POST } from '@/app/api/mcp/servers/[id]/refresh/route'
+
+const initialServer = {
+  id: 'server-1',
+  workspaceId: 'workspace-1',
+  name: 'OAuth Server',
+  url: 'https://example.com/mcp',
+  connectionStatus: 'connected',
+  lastError: null,
+  lastConnected: new Date('2026-01-01T00:00:00.000Z'),
+  toolCount: 4,
+  statusConfig: { consecutiveFailures: 0, lastSuccessfulDiscovery: null },
+}
+
+const persistedServer = {
+  ...initialServer,
+  connectionStatus: 'disconnected',
+  lastError: null,
+  toolCount: 0,
+}
+
+function selectRows(rows: unknown[]) {
+  return {
+    from: vi.fn().mockReturnValue({
+      where: vi.fn().mockReturnValue({
+        limit: vi.fn().mockResolvedValue(rows),
+      }),
+    }),
+  }
+}
+
+describe('MCP server refresh route', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockSelect.mockReturnValueOnce(selectRows([initialServer]))
+    mockUpdateSet.mockReturnValue({
+      where: vi.fn().mockReturnValue({ returning: vi.fn().mockResolvedValue([persistedServer]) }),
+    })
+  })
+
+  it('preserves the service-persisted OAuth pending status', async () => {
+    mockDiscoverServerTools.mockRejectedValueOnce(new Error('OAuth authorization required'))
+
+    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: null,
+      })
+    )
+    expect(mockUpdateSet).not.toHaveBeenCalledWith(
+      expect.objectContaining({ connectionStatus: expect.anything() })
+    )
+  })
+})

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

@@ -1,15 +1,16 @@
 import { db } from '@sim/db'
 import { mcpServers, workflow, workflowBlocks } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { toError } from '@sim/utils/errors'
+import { getErrorMessage, toError } from '@sim/utils/errors'
+import { truncate } from '@sim/utils/string'
 import { and, eq, inArray, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 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 { McpServerStatusConfig, McpTool, McpToolSchema } from '@/lib/mcp/types'
+import type { McpTool, McpToolSchema } from '@/lib/mcp/types'
 import {
   createMcpErrorResponse,
   createMcpSuccessResponse,
@@ -184,17 +185,9 @@ export const POST = withRouteHandler(
           )
         }
 
-        let connectionStatus: 'connected' | 'disconnected' | 'error' = 'error'
-        let toolCount = 0
-        let lastError: string | null = null
         let syncResult: SyncResult = { updatedCount: 0, updatedWorkflowIds: [] }
         let discoveredTools: McpTool[] = []
-
-        const currentStatusConfig: McpServerStatusConfig =
-          (server.statusConfig as McpServerStatusConfig | null) ?? {
-            consecutiveFailures: 0,
-            lastSuccessfulDiscovery: null,
-          }
+        let discoveryError: string | null = null
 
         try {
           discoveredTools = await mcpService.discoverServerTools(
@@ -203,48 +196,53 @@ export const POST = withRouteHandler(
             workspaceId,
             true
           )
-          connectionStatus = 'connected'
-          toolCount = discoveredTools.length
-          logger.info(`[${requestId}] Discovered ${toolCount} tools from server ${serverId}`)
+          logger.info(
+            `[${requestId}] Discovered ${discoveredTools.length} tools from server ${serverId}`
+          )
+        } catch (error) {
+          discoveryError = truncate(
+            getErrorMessage(error, 'Connection failed').split('\n')[0],
+            200,
+            ''
+          )
+          logger.warn(`[${requestId}] Failed to connect to server ${serverId}:`, error)
+        }
 
+        if (discoveryError === null) {
           syncResult = await syncToolSchemasToWorkflows(
             workspaceId,
             serverId,
             discoveredTools,
             requestId,
             { url: server.url ?? undefined, name: server.name ?? undefined }
           )
-        } catch (error) {
-          connectionStatus = 'error'
-          lastError =
-            error instanceof Error
-              ? error.message.split('\n')[0].slice(0, 200)
-              : 'Connection failed'
-          logger.warn(`[${requestId}] Failed to connect to server ${serverId}:`, error)
         }
 
         const now = new Date()
-        const newStatusConfig =
-          connectionStatus === 'connected'
-            ? { consecutiveFailures: 0, lastSuccessfulDiscovery: now.toISOString() }
-            : {
-                consecutiveFailures: currentStatusConfig.consecutiveFailures + 1,
-                lastSuccessfulDiscovery: currentStatusConfig.lastSuccessfulDiscovery,
-              }
 
         const [refreshedServer] = await db
           .update(mcpServers)
           .set({
             lastToolsRefresh: now,
-            connectionStatus,
-            lastError,
-            lastConnected: connectionStatus === 'connected' ? now : server.lastConnected,
-            toolCount,
-            statusConfig: newStatusConfig,
             updatedAt: now,
           })
-          .where(eq(mcpServers.id, serverId))
-          .returning()
+          .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,
+          })
+
+        const connectionStatus = refreshedServer?.connectionStatus ?? 'error'
+        const lastError = refreshedServer ? refreshedServer.lastError : discoveryError
+        const toolCount = refreshedServer?.toolCount ?? discoveredTools.length
 
         if (connectionStatus === 'connected') {
           await mcpService.clearCache(workspaceId)

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

@@ -21,6 +21,8 @@ import {
   mcpServerIdParam,
   mcpServerIdUrlKeys,
 } from '@/app/workspace/[workspaceId]/settings/[section]/search-params'
+import { getRefreshActionState } from '@/app/workspace/[workspaceId]/settings/components/mcp/refresh-action-state'
+import { getServerToolsLabel } from '@/app/workspace/[workspaceId]/settings/components/mcp/server-tools-label'
 import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu'
 import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
 import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel'
@@ -58,16 +60,6 @@ function formatTransportLabel(transport: string): string {
     .join('-')
 }
 
-function formatToolsLabel(tools: McpTool[], connectionStatus?: string): string {
-  if (connectionStatus === 'error') {
-    return 'Unable to connect'
-  }
-  const count = tools.length
-  const plural = count !== 1 ? 's' : ''
-  const names = count > 0 ? `: ${tools.map((t) => t.name).join(', ')}` : ''
-  return `${count} tool${plural}${names}`
-}
-
 interface ServerListItemProps {
   server: McpServer
   tools: McpTool[]
@@ -88,8 +80,9 @@ function ServerListItem({
   onViewDetails,
 }: ServerListItemProps) {
   const transportLabel = formatTransportLabel(server.transport || 'http')
-  const toolsLabel = formatToolsLabel(tools, server.connectionStatus)
-  const isError = server.connectionStatus === 'error'
+  const toolsLabel = getServerToolsLabel(tools, server.connectionStatus, server.lastError)
+  const hasConnectionIssue =
+    server.connectionStatus === 'error' || server.connectionStatus === 'disconnected'
 
   return (
     <div className='flex items-center justify-between gap-3'>
@@ -103,7 +96,7 @@ function ServerListItem({
         <p
           className={cn(
             'truncate text-sm',
-            isError ? 'text-[var(--text-error)]' : 'text-[var(--text-muted)]'
+            hasConnectionIssue ? 'text-[var(--text-error)]' : 'text-[var(--text-muted)]'
           )}
         >
           {isRefreshing
@@ -295,19 +288,11 @@ export function MCP() {
   }
 
   useEffect(() => {
-    if (!refreshServerMutation.isSuccess) return
+    if (!refreshServerMutation.isSuccess && !refreshServerMutation.isError) return
     const timeout = window.setTimeout(() => refreshServerMutation.reset(), 3000)
     return () => window.clearTimeout(timeout)
-    // eslint-disable-next-line react-hooks/exhaustive-deps -- mutation object is unstable; isSuccess flag is the trigger
-  }, [refreshServerMutation.isSuccess])
-
-  const refreshingServerId = refreshServerMutation.isPending
-    ? refreshServerMutation.variables?.serverId
-    : null
-  const refreshedServerId = refreshServerMutation.isSuccess
-    ? refreshServerMutation.variables?.serverId
-    : null
-  const refreshedWorkflowsUpdated = refreshServerMutation.data?.workflowsUpdated
+    // eslint-disable-next-line react-hooks/exhaustive-deps -- mutation object is unstable; status flags are the triggers
+  }, [refreshServerMutation.isSuccess, refreshServerMutation.isError])
 
   const editingServer = editingServerId
     ? (servers.find((s) => s.id === editingServerId) as McpServer | undefined)
@@ -372,25 +357,23 @@ export function MCP() {
   if (selectedServer) {
     const { server, tools } = selectedServer
     const transportLabel = formatTransportLabel(server.transport || 'http')
-
-    const refreshLabel =
-      refreshingServerId === server.id
-        ? 'Refreshing...'
-        : refreshedServerId === server.id
-          ? refreshedWorkflowsUpdated
-            ? `Synced (${refreshedWorkflowsUpdated} workflow${refreshedWorkflowsUpdated === 1 ? '' : 's'})`
-            : 'Refreshed'
-          : 'Refresh tools'
+    const isCurrentRefresh = refreshServerMutation.variables?.serverId === server.id
+    const refreshAction = getRefreshActionState({
+      mutationStatus: isCurrentRefresh ? refreshServerMutation.status : 'idle',
+      connectionStatus: isCurrentRefresh ? refreshServerMutation.data?.status : undefined,
+      workflowsUpdated: isCurrentRefresh ? refreshServerMutation.data?.workflowsUpdated : undefined,
+    })
 
     return (
       <SettingsPanel
         back={{ text: 'MCP tools', icon: ArrowLeft, onSelect: handleBackToList }}
         title={server.name || 'Unnamed Server'}
         actions={[
           {
-            text: refreshLabel,
+            text: refreshAction.text,
+            textTone: refreshAction.textTone,
             onSelect: () => handleRefreshServer(server.id),
-            disabled: refreshingServerId === server.id || refreshedServerId === server.id,
+            disabled: refreshAction.disabled,
           },
           {
             text: 'Edit',
@@ -417,11 +400,11 @@ export function MCP() {
               </div>
             )}
 
-            {server.connectionStatus === 'error' && (
+            {server.connectionStatus !== 'connected' && (
               <div className='flex flex-col gap-2'>
                 <span className='text-[var(--text-muted)] text-caption'>Status</span>
                 <p className='text-[var(--text-error)] text-sm'>
-                  {server.lastError || 'Unable to connect'}
+                  {getServerToolsLabel([], server.connectionStatus, server.lastError)}
                 </p>
               </div>
             )}
@@ -634,7 +617,10 @@ export function MCP() {
                   tools={tools}
                   isDeleting={deletingServers.has(server.id)}
                   isLoadingTools={isLoadingTools}
-                  isRefreshing={refreshingServerId === server.id}
+                  isRefreshing={
+                    refreshServerMutation.isPending &&
+                    refreshServerMutation.variables?.serverId === server.id
+                  }
                   onRemove={() => handleRemoveServer(server.id)}
                   onViewDetails={() => handleViewDetails(server.id)}
                 />