fix(mcp): keep last-known-good tools instead of flashing red on a transient failure (#5839)

* fix(mcp): keep last-known-good tools instead of flashing red on a transient failure

A connected server with cached tools was turning red 'Failed to discover' on a
single transient discovery probe blip, even though the stored status stayed
connected with tools — because useMcpToolsQuery dropped React Query's retained
data on error, and the row's showDiscoveryError fired over a populated server.

Now the aggregate keeps last-known-good tools across a failed refetch, and the
row only hard-reds when there are genuinely no tools to show. A persistent
failure still surfaces via the stored connectionStatus (gated by
MAX_CONSECUTIVE_FAILURES), matching how Claude Code / LibreChat / OpenCode /
VS Code handle a transient tools/list failure on a live connection.

Validated against those clients' source + the MCP spec before changing; this
supersedes the #5829 over-correction that showed the error even when tools
existed.

* fix(mcp): drop stale tools once a server is persistently failed, keep on transient

Sharpens the last-known-good behavior with the transient-vs-persistent distinction
the reference clients make: keep cached tools through a transient discovery failure
(stored status still healthy) so a populated server doesn't blank, but drop them
once the stored connectionStatus crosses its failure threshold to error/disconnected
— so the shared aggregate the workflow editor consumes stops offering a dead
server's stale tool schemas.

Author: waleedlatif1

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

@@ -97,12 +97,12 @@ function ServerListItem({
     server.lastError,
     server.authType
   )
-  // A live discovery failure whose stored status hasn't caught up yet would otherwise read as
-  // "0 tools"; surface it directly so a failed row reads as failed, not empty.
-  // Shown even when cached tools exist: a present discoveryError means the LATEST
-  // discovery failed, and silently showing the stale tool count would hide that.
+  // Only hard-red when there are no last-known tools to show. A populated, connected server
+  // stays on its tool count through a transient probe failure; a persistent failure flips
+  // `connectionStatus` to error/disconnected and reads as failed through that path instead.
   const showDiscoveryError =
     Boolean(discoveryError) &&
+    tools.length === 0 &&
     server.connectionStatus !== 'error' &&
     server.connectionStatus !== 'disconnected'
   const hasConnectionIssue =

apps/sim/hooks/queries/mcp.test.tsx

@@ -3,7 +3,7 @@
  */
 import { act, type ReactNode } from 'react'
 import { sleep } from '@sim/utils/helpers'
-import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
+import { QueryClient, QueryClientProvider, useQueryClient } from '@tanstack/react-query'
 import { createRoot, type Root } from 'react-dom/client'
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -20,7 +20,12 @@ import {
   listMcpServersContract,
   type McpServer,
 } from '@/lib/api/contracts/mcp'
-import { useForceRefreshMcpTools, useMcpServers, useMcpToolsQuery } from '@/hooks/queries/mcp'
+import {
+  mcpKeys,
+  useForceRefreshMcpTools,
+  useMcpServers,
+  useMcpToolsQuery,
+} from '@/hooks/queries/mcp'
 
 const WORKSPACE_ID = 'workspace-1'
 
@@ -181,6 +186,89 @@ describe('useMcpToolsQuery', () => {
     unmount()
   })
 
+  it('keeps last-known-good tools when a later discovery refetch fails', async () => {
+    let discoverCalls = 0
+    mockRequestJson.mockImplementation(async (contract) => {
+      if (contract === listMcpServersContract) {
+        return {
+          success: true,
+          data: { servers: [server('s1', { authType: 'headers', connectionStatus: 'connected' })] },
+        }
+      }
+      if (contract === discoverMcpToolsContract) {
+        discoverCalls++
+        if (discoverCalls === 1) {
+          return { success: true, data: { tools: [{ name: 'tool-a', serverId: 's1' }] } }
+        }
+        throw new Error('transient stall')
+      }
+      throw new Error('Unexpected MCP request')
+    })
+
+    const { getResult, unmount } = renderHookWithClient(() => ({
+      tools: useMcpToolsQuery(WORKSPACE_ID),
+      queryClient: useQueryClient(),
+    }))
+    await flush()
+    expect(getResult().tools.data).toHaveLength(1)
+
+    // Force a refetch that fails; the last successful tools must survive.
+    await act(async () => {
+      await getResult().queryClient.invalidateQueries({
+        queryKey: mcpKeys.serverToolsList(WORKSPACE_ID, 's1'),
+      })
+    })
+    await flush()
+
+    expect(getResult().tools.data).toHaveLength(1)
+    expect(getResult().tools.toolsStateByServer.get('s1')?.error).toBeInstanceOf(Error)
+
+    unmount()
+  })
+
+  it('drops stale tools once a non-OAuth server is persistently failed', async () => {
+    let discoverCalls = 0
+    let listCalls = 0
+    mockRequestJson.mockImplementation(async (contract) => {
+      if (contract === listMcpServersContract) {
+        listCalls++
+        // Healthy on first read, error state after the failed refetch invalidates the list.
+        const connectionStatus = listCalls === 1 ? 'connected' : 'error'
+        return {
+          success: true,
+          data: { servers: [server('s1', { authType: 'headers', connectionStatus })] },
+        }
+      }
+      if (contract === discoverMcpToolsContract) {
+        discoverCalls++
+        if (discoverCalls === 1) {
+          return { success: true, data: { tools: [{ name: 'tool-a', serverId: 's1' }] } }
+        }
+        throw new Error('persistent failure')
+      }
+      throw new Error('Unexpected MCP request')
+    })
+
+    const { getResult, unmount } = renderHookWithClient(() => ({
+      tools: useMcpToolsQuery(WORKSPACE_ID),
+      queryClient: useQueryClient(),
+    }))
+    await flush()
+    expect(getResult().tools.data).toHaveLength(1)
+
+    await act(async () => {
+      await getResult().queryClient.invalidateQueries({
+        queryKey: mcpKeys.serverToolsList(WORKSPACE_ID, 's1'),
+      })
+    })
+    await flush()
+
+    // Stored status is now 'error' → the dead server's stale tools are dropped from the aggregate.
+    expect(getResult().tools.data).toHaveLength(0)
+
+    unmount()
+  })
+
   it('does not force-refresh disconnected OAuth servers', async () => {
     mockServers([
       server('oauth-disconnected', { authType: 'oauth', connectionStatus: 'disconnected' }),

apps/sim/hooks/queries/mcp.ts

@@ -182,21 +182,27 @@ export function useMcpToolsQuery(workspaceId: string) {
     let hasData = false
     let anyServerLoading = false
     let firstError: Error | null = null
+    const statusById = new Map(servers?.map((s) => [s.id, s.connectionStatus]))
     const toolsStateByServer = new Map<
       string,
       { isLoading: boolean; isFetching: boolean; error: Error | null }
     >()
     for (let index = 0; index < results.length; index++) {
       const result = results[index]
-      // Drop stale data from servers whose latest refetch errored.
-      if (result.data && !result.isError) {
+      const serverId = serverIds[index]
+      const status = serverId ? statusById.get(serverId) : undefined
+      const persistentlyFailed = status === 'error' || status === 'disconnected'
+      // Keep last-known-good tools through a transient failure (React Query retains `data`, the
+      // stored status is still healthy) so a populated server doesn't blank — but drop them once
+      // the stored status crosses its failure threshold, so the workflow editor stops offering a
+      // dead server's stale tools. Matches how reference MCP clients treat transient vs. closed.
+      if (result.data && (!result.isError || !persistentlyFailed)) {
         tools.push(...result.data)
         hasData = true
       }
       if (result.isLoading) anyServerLoading = true
       if (!firstError && result.error instanceof Error) firstError = result.error
 
-      const serverId = serverIds[index]
       if (serverId) {
         toolsStateByServer.set(serverId, {
           isLoading: result.isLoading,
@@ -213,7 +219,7 @@ export function useMcpToolsQuery(workspaceId: string) {
       error: hasData ? null : firstError,
       toolsStateByServer,
     }
-  }, [results, serversLoading, serverIds])
+  }, [results, serversLoading, serverIds, servers])
 }
 
 export function useForceRefreshMcpTools() {