fix(mcp): keep OAuth spinner until popup closes; remove dead comments

Move popup-opening into the mutation result so the caller can track its
lifecycle. The 'Connecting…' spinner now stays until the user dismisses
or completes the OAuth popup, preventing accidental double-clicks that
would re-navigate the in-flight popup and invalidate state. Auto-OAuth
after server creation now uses the same shared helper for consistent
visual feedback.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: waleedlatif1

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

@@ -53,7 +53,6 @@ export const PATCH = withRouteHandler(
           }
         )
 
-        // Remove workspaceId from body to prevent it from being updated
         const { workspaceId: _, oauthClientSecret, ...updateData } = body
         const finalUpdateData: Record<string, unknown> = { ...updateData }
         if (oauthClientSecret !== undefined) {
@@ -88,7 +87,6 @@ export const PATCH = withRouteHandler(
           }
         }
 
-        // Get the current server to check if URL or OAuth credentials are changing
         const [currentServer] = await db
           .select({
             url: mcpServers.url,

apps/sim/app/api/mcp/servers/route.ts

@@ -243,9 +243,7 @@ export const POST = withRouteHandler(
             transport: body.transport,
             workspaceId,
           })
-        } catch (_e) {
-          // Silently fail
-        }
+        } catch (_e) {}
 
         const sourceParam = body.source as string | undefined
         const source =

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

@@ -215,6 +215,38 @@ export function MCP({ initialServerId }: MCPProps) {
   >({})
   const [expandedTools, setExpandedTools] = useState<Set<string>>(() => new Set())
 
+  const startOauthForServer = useCallback(
+    async (serverId: string) => {
+      setConnectingOauthServers((prev) => new Set(prev).add(serverId))
+      const clear = () =>
+        setConnectingOauthServers((prev) => {
+          const next = new Set(prev)
+          next.delete(serverId)
+          return next
+        })
+      try {
+        const result = await startOauthMutation.mutateAsync({ serverId, workspaceId })
+        if (result.status === 'already_authorized') {
+          clear()
+          return
+        }
+        const { popup } = result
+        const interval = window.setInterval(() => {
+          if (popup.closed) {
+            window.clearInterval(interval)
+            clear()
+          }
+        }, 500)
+      } catch (e) {
+        clear()
+        logger.error('Failed to start MCP OAuth', e)
+        toast.error(e instanceof Error ? e.message : 'Failed to start authorization')
+      }
+    },
+    // eslint-disable-next-line react-hooks/exhaustive-deps -- mutateAsync is stable
+    [workspaceId]
+  )
+
   const handleRemoveServer = useCallback((serverId: string, serverName: string) => {
     setServerToDelete({ id: serverId, name: serverName })
     setShowDeleteDialog(true)
@@ -465,24 +497,7 @@ export function MCP({ initialServerId }: MCPProps) {
                     size='sm'
                     disabled={connectingOauthServers.has(server.id)}
                     onClick={async () => {
-                      setConnectingOauthServers((prev) => new Set(prev).add(server.id))
-                      try {
-                        await startOauthMutation.mutateAsync({
-                          serverId: server.id,
-                          workspaceId,
-                        })
-                      } catch (e) {
-                        logger.error('Failed to start MCP OAuth', e)
-                        toast.error(
-                          e instanceof Error ? e.message : 'Failed to start authorization'
-                        )
-                      } finally {
-                        setConnectingOauthServers((prev) => {
-                          const next = new Set(prev)
-                          next.delete(server.id)
-                          return next
-                        })
-                      }
+                      await startOauthForServer(server.id)
                     }}
                   >
                     {connectingOauthServers.has(server.id) ? 'Connecting…' : 'Connect with OAuth'}
@@ -745,15 +760,7 @@ export function MCP({ initialServerId }: MCPProps) {
             config: { ...config, enabled: true },
           })
           if (result.authType === 'oauth') {
-            try {
-              await startOauthMutation.mutateAsync({
-                serverId: result.serverId,
-                workspaceId,
-              })
-            } catch (e) {
-              logger.error('Failed to start MCP OAuth', e)
-              toast.error(e instanceof Error ? e.message : 'Failed to start authorization')
-            }
+            await startOauthForServer(result.serverId)
           }
         }}
         workspaceId={workspaceId}

apps/sim/hooks/queries/mcp.ts

@@ -15,7 +15,6 @@ import {
   type McpServerTestResult,
   type RefreshMcpServerResult,
   refreshMcpServerContract,
-  type StartMcpOauthResult,
   startMcpOauthContract,
   testMcpServerConnectionContract,
   updateMcpServerContract,
@@ -170,21 +169,23 @@ export function useCreateMcpServer() {
 }
 
 /**
- * Mutation that begins the OAuth flow for an MCP server. Calls
- * `/api/mcp/oauth/start`, opens the upstream authorization URL in a popup
- * when the server requires user consent, and resolves to the typed result.
- *
- * `data.status === 'redirect'` means a popup was opened; the caller should
- * wait for the `mcp-oauth` postMessage from the callback page.
- * `data.status === 'already_authorized'` means valid tokens already exist.
+ * Result of `useStartMcpOauth`. When `popup` is set, the caller should wait
+ * for it to close (or for the `mcp-oauth` postMessage) before clearing any
+ * "connecting" UI state.
  */
+export type StartMcpOauthMutationResult =
+  | { status: 'redirect'; popup: Window }
+  | { status: 'already_authorized' }
+
 export function useStartMcpOauth() {
-  return useMutation<StartMcpOauthResult, Error, { serverId: string; workspaceId: string }>({
-    mutationFn: async ({ serverId, workspaceId }) => {
-      const result = await requestJson(startMcpOauthContract, {
-        query: { serverId, workspaceId },
-      })
-      if (result.status === 'redirect') {
+  return useMutation<StartMcpOauthMutationResult, Error, { serverId: string; workspaceId: string }>(
+    {
+      mutationFn: async ({ serverId, workspaceId }) => {
+        const result = await requestJson(startMcpOauthContract, {
+          query: { serverId, workspaceId },
+        })
+        if (result.status === 'already_authorized') return { status: 'already_authorized' }
+
         const parsedUrl = new URL(result.authorizationUrl)
         if (parsedUrl.protocol !== 'https:') {
           throw new Error('Authorization URL must use HTTPS')
@@ -197,10 +198,10 @@ export function useStartMcpOauth() {
         if (!popup) {
           throw new Error('Popup blocked. Please allow popups for this site and retry.')
         }
-      }
-      return result
-    },
-  })
+        return { status: 'redirect', popup }
+      },
+    }
+  )
 }
 
 interface DeleteMcpServerParams {