improvement(mcp): push-driven settings freshness + lifecycle-audit fixes (#5842)

* improvement(mcp): subscribe settings page to the live push, lean on it over re-probing

The list_changed → SSE push pipeline was already wired end-to-end but only
mounted in the workflow-editor tool picker. Subscribe the settings MCP page to
the same shared, reference-counted EventSource so tool changes reflect in real
time there too — the reference-client model (discover once, refresh on push).

With push now active on the settings page, raise MCP_SERVER_TOOLS_STALE_TIME
from 30s to 5min (matching the server-side cache TTL) so revisiting the page
leans on push + stored state instead of re-probing every connected server.
There is no background poll (no refetchInterval) — this only affects
refetch-on-visit-if-stale; real changes still arrive instantly via push.

* fix(mcp): re-sync tools on SSE reconnect so a missed list_changed can't strand stale tools

A dropped/reconnected EventSource may miss a tools_changed event during the gap,
which — with a longer stale time — could leave the page showing old tools until
a remount or manual refresh. Invalidate the workspace tools on reconnect (never
on the first open), the standard resync-on-reconnect pattern for push clients.

* fix(mcp): resync on re-subscribe so events missed while the tab was closed reconcile

Leaving the settings tab tears down the shared EventSource; remounting created a
new connection whose first open was skipped, so a tools_changed fired while
unsubscribed wasn't reconciled (the 5min stale time also won't refetch on
remount). Track a per-workspace 'ever subscribed' flag: skip resync only on the
session's first subscription (queries fetch fresh then); resync on the first
open of any re-subscription and on every reconnect.

* fix(mcp): reconcile a recovered server's status on successful discovery + fix stale comment

Lifecycle-audit findings:
- The per-server tools queryFn invalidated the server list only on error. A server
  that recovered via the stale re-probe returned fresh tools while the cached status
  still showed failed → 'tools present but row red' until the list independently
  refetched (a window the 5min stale-time widened). Now a successful probe against a
  server the cache still shows non-connected refreshes the list so the row clears.
- Correct the keep/drop comment: tools drop once the stored status leaves 'connected'
  (disconnected/error), not at a 3-failure threshold.

* fix(mcp): correct stale SSRF pin docstring and bound the remaining OAuth callback steps

Lifecycle-audit follow-ups (merged-code):
- validateMcpServerSsrf's return docstring described 'pin subsequent connections',
  which no longer holds for the public path — the returned IP is a policy signal
  selecting the validate-at-connect guarded fetch; redirect/rebind safety comes
  from per-connect validation + followRedirectsGuarded, not pinning (only the
  self-hosted private carve-out still literally pins). Corrected so a future
  reader can't reintroduce a real pin from the misleading text.
- The callback wrapped only the five post-auth steps in timedStep; the earlier
  loadOauthRowByState, getSession, and server SELECT (plus the provider_error
  clearState) were unbounded, contradicting the 'every awaited step bounded'
  invariant. Wrapped them so a wedged DB read surfaces as a labeled timeout.

* fix(mcp): resync on a first SSE open that recovered from an earlier connection error

If the initial EventSource connection errors before opening (and the initial tools
query fails with retry:false), the first successful onopen was skipped, leaving the
query stale. Track erroredBeforeOpen so a first open that followed a connection
error also resyncs — only a clean first subscription still skips.

* fix(mcp): make the SSE push subscription intrinsic to the tools query

The 5min stale time assumed push, but useMcpToolsEvents was only mounted in the
settings page and the tool picker — other consumers of the tools query (dynamic
args, tool selector, canvas block via useMcpToolsQuery/useMcpTools) had no
subscription, so list_changed updates could stay invisible for the full window.

Mount useMcpToolsEvents inside useMcpToolsQuery so every tools consumer gets
real-time push from the shared, reference-counted connection — no consumer is
left re-probing. Removed the now-redundant explicit mounts from the settings
page and tool-input.

* test(mcp): clear the shared SSE collections instead of reassigning globalThis

mcp.ts captures the connections Map and subscribed Set in module consts at import,
so setting the globalThis property to undefined didn't reset what the module uses —
subscription state leaked across tests. Clear the shared instances instead.

* fix(mcp): drop the success-path status reconciliation heuristic

The client-side serversList invalidation on a successful probe assumed the probe
updated the stored status, but a server-side cache hit returns tools without
touching status — so in the failed-cache-delete edge it fired a pointless
refetch. The benefit (instant vs the 60s serversList stale-time for status
recovery) doesn't justify the incorrect assumption; rely on the existing 60s
stale-time + SSE push for status recovery instead. Keeps the corrected keep/drop
comment.

Author: waleedlatif1

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

@@ -125,12 +125,19 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
     serverId?: string
   ) => htmlClose(message, ok, reason, serverId, state)
 
-  const initialRow = state ? await loadOauthRowByState(state).catch(() => null) : null
+  const initialRow = state
+    ? await timedStep('loadOauthRowByState', 15_000, () => loadOauthRowByState(state)).catch(
+        () => null
+      )
+    : null
   const stateRowServerId = initialRow?.mcpServerId
 
   if (errorParam) {
     logger.warn(`MCP OAuth callback received error: ${errorParam}`)
-    if (initialRow) await clearState(initialRow.id, 'callback:provider_error').catch(() => {})
+    if (initialRow)
+      await timedStep('clearState(provider_error)', 10_000, () =>
+        clearState(initialRow.id, 'callback:provider_error')
+      ).catch(() => {})
     return respond(`Authorization failed: ${errorParam}`, false, 'provider_error', stateRowServerId)
   }
   if (!state || !code) {
@@ -144,7 +151,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
   let serverId: string | undefined
   try {
-    const session = await getSession()
+    const session = await timedStep('getSession', 15_000, () => getSession())
     if (!session?.user?.id) {
       return respond(
         'You must be signed in to complete authorization.',
@@ -169,11 +176,13 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       )
     }
 
-    const [server] = await db
-      .select({ id: mcpServers.id, url: mcpServers.url, workspaceId: mcpServers.workspaceId })
-      .from(mcpServers)
-      .where(and(eq(mcpServers.id, row.mcpServerId), isNull(mcpServers.deletedAt)))
-      .limit(1)
+    const [server] = await timedStep('loadServer', 15_000, () =>
+      db
+        .select({ id: mcpServers.id, url: mcpServers.url, workspaceId: mcpServers.workspaceId })
+        .from(mcpServers)
+        .where(and(eq(mcpServers.id, row.mcpServerId), isNull(mcpServers.deletedAt)))
+        .limit(1)
+    )
     if (!server || !server.url) {
       return respond('Server no longer exists.', false, 'server_gone', serverId)
     }

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx

@@ -79,7 +79,6 @@ import {
   useCreateMcpServer,
   useForceRefreshMcpTools,
   useMcpServers,
-  useMcpToolsEvents,
   useStoredMcpTools,
 } from '@/hooks/queries/mcp'
 import { useWorkflowState, useWorkflows } from '@/hooks/queries/workflows'
@@ -570,7 +569,6 @@ export const ToolInput = memo(function ToolInput({
   const { data: mcpServers = [], isLoading: mcpServersLoading } = useMcpServers(workspaceId)
   const { data: storedMcpTools = [] } = useStoredMcpTools(workspaceId)
   const forceRefreshMcpTools = useForceRefreshMcpTools().mutate
-  useMcpToolsEvents(workspaceId)
   const { navigateToSettings } = useSettingsNavigation()
   const createMcpServer = useCreateMcpServer()
   const { startOauthForServer } = useMcpOauthPopup({ workspaceId })

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

@@ -103,13 +103,29 @@ function mockServers(servers: McpServer[]) {
   })
 }
 
+// jsdom has no EventSource; useMcpToolsQuery mounts the shared SSE subscription.
+class FakeEventSource {
+  onopen: (() => void) | null = null
+  onerror: (() => void) | null = null
+  constructor(public url: string) {}
+  addEventListener(): void {}
+  close(): void {}
+}
+
 describe('useMcpToolsQuery', () => {
   beforeEach(() => {
     vi.clearAllMocks()
+    ;(globalThis as unknown as { EventSource: unknown }).EventSource = FakeEventSource
   })
 
   afterEach(() => {
     vi.restoreAllMocks()
+    // mcp.ts captured these Map/Set instances in module consts at import, so reassigning the
+    // globalThis property wouldn't reset what the module uses — clear the shared instances.
+    ;(
+      globalThis as unknown as { __mcp_sse_connections?: Map<string, unknown> }
+    ).__mcp_sse_connections?.clear()
+    ;(globalThis as unknown as { __mcp_sse_subscribed?: Set<string> }).__mcp_sse_subscribed?.clear()
   })
 
   it('does not auto-discover disconnected or errored OAuth servers', async () => {
@@ -139,7 +155,11 @@ describe('useMcpToolsQuery', () => {
     const { unmount } = renderHookWithClient(() => useMcpToolsQuery(WORKSPACE_ID))
     await flush()
 
-    expect(mockRequestJson).toHaveBeenCalledTimes(3)
+    // Both eligible servers get discovered.
+    const discoveryCalls = mockRequestJson.mock.calls.filter(
+      ([contract]) => contract === discoverMcpToolsContract
+    )
+    expect(discoveryCalls).toHaveLength(2)
     expect(mockRequestJson).toHaveBeenCalledWith(
       discoverMcpToolsContract,
       expect.objectContaining({

apps/sim/hooks/queries/mcp.ts

@@ -42,7 +42,13 @@ const logger = createLogger('McpQueries')
 export type { McpServerStatusConfig, McpTool, StoredMcpTool }
 
 export const MCP_SERVER_LIST_STALE_TIME = 60 * 1000
-export const MCP_SERVER_TOOLS_STALE_TIME = 30 * 1000
+/**
+ * Tool discovery is kept fresh by the `list_changed` → SSE push (see `useMcpToolsEvents`),
+ * so the query only needs a re-probe-on-visit fallback for servers without push. Matches the
+ * server-side cache TTL (`MCP_CONSTANTS.CACHE_TIMEOUT`) — no reference MCP client re-probes
+ * more often than its cache; real changes arrive via push regardless of this value.
+ */
+export const MCP_SERVER_TOOLS_STALE_TIME = 5 * 60 * 1000
 export const MCP_STORED_TOOL_LIST_STALE_TIME = 60 * 1000
 export const MCP_ALLOWED_DOMAINS_STALE_TIME = 5 * 60 * 1000
 
@@ -140,6 +146,11 @@ function isServerEligibleForDiscovery(server: McpServer, workspaceId: string): b
 export function useMcpToolsQuery(workspaceId: string) {
   const queryClient = useQueryClient()
   const { data: servers, isLoading: serversLoading } = useMcpServers(workspaceId)
+  // Push is intrinsic to consuming the tools query: every surface that reads tools (settings,
+  // tool picker, dynamic args, tool selector, canvas block) gets real-time `list_changed`
+  // refresh via the shared, reference-counted subscription — so the 5-min stale time is always
+  // push-backed and no consumer is left re-probing.
+  useMcpToolsEvents(workspaceId)
 
   /**
    * Skip disabled rows, rows retained from a previous workspace, and OAuth rows
@@ -192,10 +203,10 @@ export function useMcpToolsQuery(workspaceId: string) {
       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.
+      // Keep last-known-good tools while the stored status is still `connected` (React Query
+      // retains `data` across a failed refetch, so a populated server doesn't blank on a
+      // transient probe error) — but drop them once the stored status leaves `connected`
+      // (disconnected/error), so the workflow editor stops offering a dead server's stale tools.
       if (result.data && (!result.isError || !persistentlyFailed)) {
         tools.push(...result.data)
         hasData = true
@@ -527,6 +538,12 @@ const sseConnections: Map<string, SseEntry> =
   ((globalThis as Record<string, unknown>)[SSE_KEY] as Map<string, SseEntry>) ??
   ((globalThis as Record<string, unknown>)[SSE_KEY] = new Map<string, SseEntry>())
 
+/** Per-workspace flag: has this session ever held a live SSE subscription for it? */
+const SSE_SUBSCRIBED_KEY = '__mcp_sse_subscribed' as const
+const sseEverSubscribed: Set<string> =
+  ((globalThis as Record<string, unknown>)[SSE_SUBSCRIBED_KEY] as Set<string>) ??
+  ((globalThis as Record<string, unknown>)[SSE_SUBSCRIBED_KEY] = new Set<string>())
+
 /** Subscribes to `tools_changed` SSE events and invalidates the affected query keys. */
 export function useMcpToolsEvents(workspaceId: string) {
   const queryClient = useQueryClient()
@@ -563,7 +580,23 @@ export function useMcpToolsEvents(workspaceId: string) {
         invalidate(serverId)
       })
 
+      // EventSource fires `onopen` on the initial connect and on every auto-reconnect. Re-sync
+      // the workspace whenever we could have missed a `tools_changed` event: on any reconnect,
+      // on the first open of a RE-subscription (leaving the tab tears the connection down), and
+      // on a first open that only succeeded after an earlier connection error (the initial tools
+      // query may have failed during that gap and won't retry itself). Skip only a clean first
+      // subscription — the queries fetch fresh on their own initial mount.
+      const isResubscribe = sseEverSubscribed.has(workspaceId)
+      sseEverSubscribed.add(workspaceId)
+      let opened = false
+      let erroredBeforeOpen = false
+      source.onopen = () => {
+        if (opened || isResubscribe || erroredBeforeOpen) invalidate()
+        opened = true
+      }
+
       source.onerror = () => {
+        if (!opened) erroredBeforeOpen = true
         logger.warn(`SSE connection error for workspace ${workspaceId}`)
       }
 

apps/sim/lib/mcp/domain-check.ts

@@ -139,14 +139,17 @@ function isLocalhostHostname(hostname: string): boolean {
  * URLs with env var references in the hostname are skipped — they will be
  * validated after resolution at execution time.
  *
- * Returns the IP address to pin subsequent connections to (the resolved IP for
- * hostnames, or the literal itself for public IP-literal URLs) so the caller can
- * prevent DNS-rebinding TOCTOU attacks and stop redirects from escaping to
- * internal hosts. Pinning matters for IP literals too: without it the transport
- * uses the default fetch, which follows an attacker-controlled 3xx redirect to a
- * private/metadata address. Returns null only when pinning is unnecessary or
- * impossible: no URL, allowlist-only mode, env-var hostnames (validated later),
- * and localhost on self-hosted (no rebinding risk against a fixed loopback).
+ * Returns the resolved IP (or the literal itself for IP-literal URLs) as a
+ * non-null **policy signal**: the SSRF guard is active for this server. A public
+ * resolution selects the validate-at-connect guarded fetch — DNS-rebinding TOCTOU
+ * and redirect escapes are prevented by re-validating every socket connect and
+ * following redirects under per-hop validation (see `createSsrfGuardedMcpFetch` /
+ * `followRedirectsGuarded`), NOT by pinning to this address. The value is literally
+ * pinned only for the self-hosted private/loopback carve-out (a policy-permitted
+ * DNS alias the guarded lookup would otherwise filter). Returns null when the guard
+ * is unnecessary or impossible: no URL, allowlist-only mode, env-var hostnames
+ * (validated later), and localhost on self-hosted (no rebinding risk against a
+ * fixed loopback).
  *
  * @throws McpSsrfError if the URL resolves to a blocked IP address
  */