From da8ede3797946ebcb1346df63bcbcd4a5d4fa421 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 21 Jul 2026 23:28:59 -0700 Subject: [PATCH 1/9] improvement(mcp): subscribe settings page to the live push, lean on it over re-probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../[workspaceId]/settings/components/mcp/mcp.tsx | 4 ++++ apps/sim/hooks/queries/mcp.ts | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx index 32cc51cc2f9..ab18f209f85 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx @@ -39,6 +39,7 @@ import { useDeleteMcpServer, useForceRefreshMcpTools, useMcpServers, + useMcpToolsEvents, useMcpToolsQuery, useRefreshMcpServer, useStoredMcpTools, @@ -195,6 +196,9 @@ export function MCP() { } = useMcpServers(workspaceId) const { data: mcpToolsData = [], toolsStateByServer } = useMcpToolsQuery(workspaceId) const { data: storedTools = [], refetch: refetchStoredTools } = useStoredMcpTools(workspaceId) + // Real-time refresh via the shared list_changed → SSE push (same subscription the workflow + // editor uses), so tool changes reflect here without re-probing on every visit. + useMcpToolsEvents(workspaceId) const forceRefreshToolsMutation = useForceRefreshMcpTools() const forceRefreshTools = forceRefreshToolsMutation.mutate const createServerMutation = useCreateMcpServer() diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index 2f0f2002080..b9ab42f25db 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/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 From 289c81da14e75c9b6aae0a37214cf43f7be9fef8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 21 Jul 2026 23:34:13 -0700 Subject: [PATCH 2/9] fix(mcp): re-sync tools on SSE reconnect so a missed list_changed can't strand stale tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/hooks/queries/mcp.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index b9ab42f25db..f9e732c3cce 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -563,6 +563,15 @@ export function useMcpToolsEvents(workspaceId: string) { invalidate(serverId) }) + // EventSource fires `onopen` on the initial connect and on every auto-reconnect. A + // reconnect may have missed a `tools_changed` event during the gap, so re-sync the + // whole workspace on reconnect (never on the first open — the query already fetched). + let opened = false + source.onopen = () => { + if (opened) invalidate() + opened = true + } + source.onerror = () => { logger.warn(`SSE connection error for workspace ${workspaceId}`) } From f8fc2ed65b0c204fc648a74f27c43482ca66be1b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 21 Jul 2026 23:40:28 -0700 Subject: [PATCH 3/9] 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. --- apps/sim/hooks/queries/mcp.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index f9e732c3cce..2ed6f8ba067 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -527,6 +527,12 @@ const sseConnections: Map = ((globalThis as Record)[SSE_KEY] as Map) ?? ((globalThis as Record)[SSE_KEY] = new Map()) +/** 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 = + ((globalThis as Record)[SSE_SUBSCRIBED_KEY] as Set) ?? + ((globalThis as Record)[SSE_SUBSCRIBED_KEY] = new Set()) + /** Subscribes to `tools_changed` SSE events and invalidates the affected query keys. */ export function useMcpToolsEvents(workspaceId: string) { const queryClient = useQueryClient() @@ -563,12 +569,16 @@ export function useMcpToolsEvents(workspaceId: string) { invalidate(serverId) }) - // EventSource fires `onopen` on the initial connect and on every auto-reconnect. A - // reconnect may have missed a `tools_changed` event during the gap, so re-sync the - // whole workspace on reconnect (never on the first open — the query already fetched). + // 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, + // and on the first open of a RE-subscription (leaving the tab tears the connection down, + // so events fired while unsubscribed would otherwise be missed). Skip only the very first + // subscription of the session — the queries fetch fresh on their own initial mount. + const isResubscribe = sseEverSubscribed.has(workspaceId) + sseEverSubscribed.add(workspaceId) let opened = false source.onopen = () => { - if (opened) invalidate() + if (opened || isResubscribe) invalidate() opened = true } From 82d32c4746d4f8aa3dfec0fe87b0a10f4d3b63f8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 21 Jul 2026 23:56:05 -0700 Subject: [PATCH 4/9] fix(mcp): reconcile a recovered server's status on successful discovery + fix stale comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/hooks/queries/mcp.test.tsx | 34 ++++++++++++++++++++++++++++- apps/sim/hooks/queries/mcp.ts | 22 ++++++++++++++----- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/apps/sim/hooks/queries/mcp.test.tsx b/apps/sim/hooks/queries/mcp.test.tsx index ad6c120f516..0f27355de64 100644 --- a/apps/sim/hooks/queries/mcp.test.tsx +++ b/apps/sim/hooks/queries/mcp.test.tsx @@ -139,7 +139,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({ @@ -156,6 +160,34 @@ describe('useMcpToolsQuery', () => { unmount() }) + it('refreshes the server list when a previously-failed server discovers successfully', async () => { + let listCalls = 0 + mockRequestJson.mockImplementation(async (contract) => { + if (contract === listMcpServersContract) { + listCalls++ + return { + success: true, + data: { + servers: [server('s1', { authType: 'headers', connectionStatus: 'disconnected' })], + }, + } + } + if (contract === discoverMcpToolsContract) { + return { success: true, data: { tools: [{ name: 'tool-a', serverId: 's1' }] } } + } + throw new Error('Unexpected MCP request') + }) + + const { unmount } = renderHookWithClient(() => useMcpToolsQuery(WORKSPACE_ID)) + await flush() + + // A successful probe against a server the cache still shows 'disconnected' refreshes the + // list (server-side the status is now 'connected') so the row clears its red state. + expect(listCalls).toBeGreaterThanOrEqual(2) + + unmount() + }) + it('refreshes the server list after a connected OAuth discovery fails', async () => { let serverListRequests = 0 mockRequestJson.mockImplementation(async (contract) => { diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index f0ffd1465c7..ba77b8efbc6 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -167,7 +167,19 @@ export function useMcpToolsQuery(workspaceId: string) { queryKey: mcpKeys.serverToolsList(workspaceId, serverId), queryFn: async ({ signal }: { signal?: AbortSignal }) => { try { - return await fetchMcpTools(workspaceId, false, signal, serverId) + const tools = await fetchMcpTools(workspaceId, false, signal, serverId) + // A successful probe flips the stored status to `connected` server-side; if the + // cached list still shows this server failed, refresh it so the row clears its red + // state (and its tools stop being dropped) instead of waiting out the list stale-time. + const cached = queryClient.getQueryData(mcpKeys.serversList(workspaceId)) + const status = cached?.find((s) => s.id === serverId)?.connectionStatus + if (status && status !== 'connected') { + queryClient.invalidateQueries( + { queryKey: mcpKeys.serversList(workspaceId) }, + { cancelRefetch: false } + ) + } + return tools } catch (error) { await queryClient.invalidateQueries( { queryKey: mcpKeys.serversList(workspaceId) }, @@ -198,10 +210,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 From b3e6b24d4e666d503113ac323d7ebf160d963100 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 22 Jul 2026 00:00:01 -0700 Subject: [PATCH 5/9] fix(mcp): correct stale SSRF pin docstring and bound the remaining OAuth callback steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/app/api/mcp/oauth/callback/route.ts | 25 +++++++++++++------- apps/sim/lib/mcp/domain-check.ts | 19 ++++++++------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/apps/sim/app/api/mcp/oauth/callback/route.ts b/apps/sim/app/api/mcp/oauth/callback/route.ts index 7694f23b010..79b7058ddea 100644 --- a/apps/sim/app/api/mcp/oauth/callback/route.ts +++ b/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) } diff --git a/apps/sim/lib/mcp/domain-check.ts b/apps/sim/lib/mcp/domain-check.ts index e04b6f04cc3..1c4c40e3ac1 100644 --- a/apps/sim/lib/mcp/domain-check.ts +++ b/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 */ From 0b0658c17224c29b92aee7134b7285c823e96e38 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 22 Jul 2026 00:07:16 -0700 Subject: [PATCH 6/9] fix(mcp): resync on a first SSE open that recovered from an earlier connection error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/hooks/queries/mcp.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index ba77b8efbc6..f2f4ba0ff65 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -589,18 +589,21 @@ export function useMcpToolsEvents(workspaceId: string) { // 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, - // and on the first open of a RE-subscription (leaving the tab tears the connection down, - // so events fired while unsubscribed would otherwise be missed). Skip only the very first - // subscription of the session — the queries fetch fresh on their own initial mount. + // 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) invalidate() + if (opened || isResubscribe || erroredBeforeOpen) invalidate() opened = true } source.onerror = () => { + if (!opened) erroredBeforeOpen = true logger.warn(`SSE connection error for workspace ${workspaceId}`) } From ced941c0aaf8b36df4be236b2549e7cbca0d59bb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 22 Jul 2026 00:21:07 -0700 Subject: [PATCH 7/9] fix(mcp): make the SSE push subscription intrinsic to the tools query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../[workspaceId]/settings/components/mcp/mcp.tsx | 4 ---- .../sub-block/components/tool-input/tool-input.tsx | 2 -- apps/sim/hooks/queries/mcp.test.tsx | 12 ++++++++++++ apps/sim/hooks/queries/mcp.ts | 5 +++++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx index 4b2696109d0..5558e23fb35 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx @@ -39,7 +39,6 @@ import { useDeleteMcpServer, useForceRefreshMcpTools, useMcpServers, - useMcpToolsEvents, useMcpToolsQuery, useRefreshMcpServer, useStoredMcpTools, @@ -196,9 +195,6 @@ export function MCP() { } = useMcpServers(workspaceId) const { data: mcpToolsData = [], toolsStateByServer } = useMcpToolsQuery(workspaceId) const { data: storedTools = [], refetch: refetchStoredTools } = useStoredMcpTools(workspaceId) - // Real-time refresh via the shared list_changed → SSE push (same subscription the workflow - // editor uses), so tool changes reflect here without re-probing on every visit. - useMcpToolsEvents(workspaceId) const forceRefreshToolsMutation = useForceRefreshMcpTools() const forceRefreshTools = forceRefreshToolsMutation.mutate const createServerMutation = useCreateMcpServer() diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index a4a937a5e27..3cfcb3fe3e8 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/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 }) diff --git a/apps/sim/hooks/queries/mcp.test.tsx b/apps/sim/hooks/queries/mcp.test.tsx index 0f27355de64..cd06c118c85 100644 --- a/apps/sim/hooks/queries/mcp.test.tsx +++ b/apps/sim/hooks/queries/mcp.test.tsx @@ -103,13 +103,25 @@ 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() + ;(globalThis as unknown as Record).__mcp_sse_connections = undefined + ;(globalThis as unknown as Record).__mcp_sse_subscribed = undefined }) it('does not auto-discover disconnected or errored OAuth servers', async () => { diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index f2f4ba0ff65..5e1ba92a412 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -146,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 From 55ba015beaadf7aae6bfebaf3f519f9ecf0f712c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 22 Jul 2026 00:28:27 -0700 Subject: [PATCH 8/9] test(mcp): clear the shared SSE collections instead of reassigning globalThis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/hooks/queries/mcp.test.tsx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/sim/hooks/queries/mcp.test.tsx b/apps/sim/hooks/queries/mcp.test.tsx index cd06c118c85..f43c8b76dbe 100644 --- a/apps/sim/hooks/queries/mcp.test.tsx +++ b/apps/sim/hooks/queries/mcp.test.tsx @@ -120,8 +120,12 @@ describe('useMcpToolsQuery', () => { afterEach(() => { vi.restoreAllMocks() - ;(globalThis as unknown as Record).__mcp_sse_connections = undefined - ;(globalThis as unknown as Record).__mcp_sse_subscribed = undefined + // 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 } + ).__mcp_sse_connections?.clear() + ;(globalThis as unknown as { __mcp_sse_subscribed?: Set }).__mcp_sse_subscribed?.clear() }) it('does not auto-discover disconnected or errored OAuth servers', async () => { From 1815e2159a09b5abcc6eac3f10efc6457fdc13fc Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 22 Jul 2026 01:02:07 -0700 Subject: [PATCH 9/9] fix(mcp): drop the success-path status reconciliation heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/hooks/queries/mcp.test.tsx | 28 ---------------------------- apps/sim/hooks/queries/mcp.ts | 14 +------------- 2 files changed, 1 insertion(+), 41 deletions(-) diff --git a/apps/sim/hooks/queries/mcp.test.tsx b/apps/sim/hooks/queries/mcp.test.tsx index f43c8b76dbe..18e3f2d5e34 100644 --- a/apps/sim/hooks/queries/mcp.test.tsx +++ b/apps/sim/hooks/queries/mcp.test.tsx @@ -176,34 +176,6 @@ describe('useMcpToolsQuery', () => { unmount() }) - it('refreshes the server list when a previously-failed server discovers successfully', async () => { - let listCalls = 0 - mockRequestJson.mockImplementation(async (contract) => { - if (contract === listMcpServersContract) { - listCalls++ - return { - success: true, - data: { - servers: [server('s1', { authType: 'headers', connectionStatus: 'disconnected' })], - }, - } - } - if (contract === discoverMcpToolsContract) { - return { success: true, data: { tools: [{ name: 'tool-a', serverId: 's1' }] } } - } - throw new Error('Unexpected MCP request') - }) - - const { unmount } = renderHookWithClient(() => useMcpToolsQuery(WORKSPACE_ID)) - await flush() - - // A successful probe against a server the cache still shows 'disconnected' refreshes the - // list (server-side the status is now 'connected') so the row clears its red state. - expect(listCalls).toBeGreaterThanOrEqual(2) - - unmount() - }) - it('refreshes the server list after a connected OAuth discovery fails', async () => { let serverListRequests = 0 mockRequestJson.mockImplementation(async (contract) => { diff --git a/apps/sim/hooks/queries/mcp.ts b/apps/sim/hooks/queries/mcp.ts index 5e1ba92a412..f7fbffe9176 100644 --- a/apps/sim/hooks/queries/mcp.ts +++ b/apps/sim/hooks/queries/mcp.ts @@ -172,19 +172,7 @@ export function useMcpToolsQuery(workspaceId: string) { queryKey: mcpKeys.serverToolsList(workspaceId, serverId), queryFn: async ({ signal }: { signal?: AbortSignal }) => { try { - const tools = await fetchMcpTools(workspaceId, false, signal, serverId) - // A successful probe flips the stored status to `connected` server-side; if the - // cached list still shows this server failed, refresh it so the row clears its red - // state (and its tools stop being dropped) instead of waiting out the list stale-time. - const cached = queryClient.getQueryData(mcpKeys.serversList(workspaceId)) - const status = cached?.find((s) => s.id === serverId)?.connectionStatus - if (status && status !== 'connected') { - queryClient.invalidateQueries( - { queryKey: mcpKeys.serversList(workspaceId) }, - { cancelRefetch: false } - ) - } - return tools + return await fetchMcpTools(workspaceId, false, signal, serverId) } catch (error) { await queryClient.invalidateQueries( { queryKey: mcpKeys.serversList(workspaceId) },