fix(mcp): address second-round bugbot review

- useMcpToolsQuery serverIds: filter on enabled + workspaceId match.
  Disabled rows no longer trigger discover calls that get negative-cached,
  and keepPreviousData on useMcpServers no longer races a workspace switch
  into cross-workspace discover requests.
- Aggregate skips per-server data when that server's latest refetch errored,
  so a broken server's last-known tools no longer linger in the workspace
  view while its card shows an error.
- discoverServerTools failure path drops the positive cache alongside writing
  the negative-cache marker. A cache-respecting follow-up now fails fast via
  cooldown instead of returning stale tools from a now-broken server.
- useMcpTools.refreshTools drops the dead forceRefresh param — the per-server
  queryFn always sends refresh=false, so the flag was never effective. Callers
  wanting cache-bypass should use useForceRefreshMcpTools.

Author: waleedlatif1

apps/sim/hooks/mcp/use-mcp-tools.ts

@@ -32,7 +32,7 @@ export interface UseMcpToolsResult {
   mcpTools: McpToolForUI[]
   isLoading: boolean
   error: string | null
-  refreshTools: (forceRefresh?: boolean) => Promise<void>
+  refreshTools: () => Promise<void>
   getToolsByServer: (serverId: string) => McpToolForUI[]
 }
 
@@ -55,22 +55,20 @@ export function useMcpTools(workspaceId: string): UseMcpToolsResult {
     }))
   }, [mcpToolsData])
 
-  const refreshTools = useCallback(
-    async (forceRefresh = false) => {
-      if (!workspaceId) {
-        logger.warn('Cannot refresh tools: no workspaceId provided')
-        return
-      }
+  // Soft refresh: invalidate per-server query entries so React Query refetches
+  // them, but let the server-side cache decide whether to go live (the per-server
+  // queryFn always sends refresh=false). For an explicit cache-bypass refresh,
+  // use `useForceRefreshMcpTools` instead.
+  const refreshTools = useCallback(async () => {
+    if (!workspaceId) {
+      logger.warn('Cannot refresh tools: no workspaceId provided')
+      return
+    }
 
-      logger.info('Refreshing MCP tools', { forceRefresh, workspaceId })
-
-      await queryClient.invalidateQueries({
-        queryKey: mcpKeys.serverToolsWorkspace(workspaceId),
-        refetchType: forceRefresh ? 'active' : 'all',
-      })
-    },
-    [workspaceId, queryClient]
-  )
+    await queryClient.invalidateQueries({
+      queryKey: mcpKeys.serverToolsWorkspace(workspaceId),
+    })
+  }, [workspaceId, queryClient])
 
   const getToolsByServer = useCallback(
     (serverId: string): McpToolForUI[] => {

apps/sim/hooks/queries/mcp.ts

@@ -140,7 +140,20 @@ export function useMcpServerTools(workspaceId: string, serverId?: string) {
 export function useMcpToolsQuery(workspaceId: string) {
   const { data: servers, isLoading: serversLoading } = useMcpServers(workspaceId)
 
-  const serverIds = useMemo(() => (servers ? servers.map((s) => s.id).sort() : []), [servers])
+  // Filter on (a) enabled (disabled servers would 404 in discover and pollute the
+  // negative cache), and (b) workspaceId match (useMcpServers keeps the previous
+  // workspace's data via keepPreviousData, so during a workspace switch we'd
+  // otherwise spawn discover calls for the wrong workspace's IDs).
+  const serverIds = useMemo(
+    () =>
+      servers
+        ? servers
+            .filter((s) => s.enabled && s.workspaceId === workspaceId)
+            .map((s) => s.id)
+            .sort()
+        : [],
+    [servers, workspaceId]
+  )
 
   const results = useQueries({
     queries: serverIds.map((serverId) => ({
@@ -160,7 +173,10 @@ export function useMcpToolsQuery(workspaceId: string) {
     let anyServerLoading = false
     let firstError: Error | null = null
     for (const result of results) {
-      if (result.data) {
+      // Drop stale data when the latest refetch failed — otherwise React Query's
+      // stale-while-revalidate behavior would surface broken-server tools in the
+      // aggregate while the per-server card shows an error.
+      if (result.data && !result.isError) {
         tools.push(...result.data)
         hasData = true
       }

apps/sim/lib/mcp/service.ts

@@ -723,7 +723,17 @@ class McpService {
           await sleep(100)
           continue
         }
-        await this.markServerUnhealthy(workspaceId, serverId, error)
+        // Drop any stale positive cache so a follow-up cache-respecting call
+        // doesn't keep returning old tools from a now-broken server. The
+        // negative cache marker then makes that follow-up fail fast.
+        await Promise.allSettled([
+          this.cacheAdapter
+            .delete(serverCacheKey(workspaceId, serverId))
+            .catch((err) =>
+              logger.warn(`[${requestId}] Cache delete failed for ${serverId}:`, err)
+            ),
+          this.markServerUnhealthy(workspaceId, serverId, error),
+        ])
         throw error
       }
     }