refactor(mcp): extract oauthCredsChanged helper, harden popup error handling

- Dedupe OAuth credential diff logic shared by PATCH /servers/[id] and
  POST /servers; both routes now call a single helper that handles
  decrypt failure as a change.
- Normalize oauthClientId ('' → null) in the useUpdateMcpServer optimistic
  cache write so it matches the post-invalidation server shape.
- Clear all in-flight connecting state when the OAuth callback posts an
  early failure without a serverId.

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

Author: waleedlatif1

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

@@ -6,7 +6,7 @@ import { toError } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { updateMcpServerBodySchema } from '@/lib/api/contracts/mcp'
-import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
+import { encryptSecret } from '@/lib/core/security/encryption'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
   McpDnsResolutionError,
@@ -16,7 +16,7 @@ import {
   validateMcpServerSsrf,
 } from '@/lib/mcp/domain-check'
 import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
-import { revokeMcpOauthTokens } from '@/lib/mcp/oauth'
+import { oauthCredsChanged, revokeMcpOauthTokens } from '@/lib/mcp/oauth'
 import { mcpService } from '@/lib/mcp/service'
 import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
 
@@ -125,27 +125,15 @@ export const PATCH = withRouteHandler(
         }
 
         const urlChanged = body.url !== undefined && currentServer?.url !== body.url
-        const clientIdChanged =
-          body.oauthClientId !== undefined &&
-          (body.oauthClientId || null) !== (currentServer?.oauthClientId ?? null)
-        let clientSecretChanged = false
-        if (oauthClientSecret !== undefined) {
-          if (!oauthClientSecret) {
-            clientSecretChanged = currentServer?.oauthClientSecret != null
-          } else if (!currentServer?.oauthClientSecret) {
-            clientSecretChanged = true
-          } else {
-            try {
-              const currentPlaintext = (await decryptSecret(currentServer.oauthClientSecret))
-                .decrypted
-              clientSecretChanged = currentPlaintext !== oauthClientSecret
-            } catch {
-              clientSecretChanged = true
-            }
-          }
-        }
-        const oauthCredsChanged = clientIdChanged || clientSecretChanged
-        const shouldClearOauth = urlChanged || oauthCredsChanged
+        const credsChanged = await oauthCredsChanged({
+          incomingClientId: body.oauthClientId,
+          incomingClientIdProvided: body.oauthClientId !== undefined,
+          incomingClientSecret: oauthClientSecret,
+          incomingClientSecretProvided: oauthClientSecret !== undefined,
+          currentClientId: currentServer?.oauthClientId,
+          currentEncryptedClientSecret: currentServer?.oauthClientSecret,
+        })
+        const shouldClearOauth = urlChanged || credsChanged
 
         const resolvedAuthType = finalUpdateData.authType ?? currentServer?.authType
         if (shouldClearOauth && resolvedAuthType === 'oauth') {

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

@@ -8,7 +8,7 @@ import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { createMcpServerBodySchema, deleteMcpServerByQuerySchema } from '@/lib/api/contracts/mcp'
 import { validationErrorResponse } from '@/lib/api/server'
-import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
+import { encryptSecret } from '@/lib/core/security/encryption'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import {
   McpDnsResolutionError,
@@ -18,7 +18,7 @@ import {
   validateMcpServerSsrf,
 } from '@/lib/mcp/domain-check'
 import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
-import { detectMcpAuthType, revokeMcpOauthTokens } from '@/lib/mcp/oauth'
+import { detectMcpAuthType, oauthCredsChanged, revokeMcpOauthTokens } from '@/lib/mcp/oauth'
 import { mcpService } from '@/lib/mcp/service'
 import {
   createMcpErrorResponse,
@@ -163,29 +163,17 @@ export const POST = withRouteHandler(
             `[${requestId}] Server with ID ${serverId} already exists, updating instead of creating`
           )
 
-          const clientIdChanged =
-            oauthClientIdProvided &&
-            (oauthClientId || null) !== (existingServer.oauthClientId ?? null)
-          let clientSecretChanged = false
-          if (oauthClientSecretProvided) {
-            if (!body.oauthClientSecret) {
-              clientSecretChanged = existingServer.oauthClientSecret != null
-            } else if (!existingServer.oauthClientSecret) {
-              clientSecretChanged = true
-            } else {
-              try {
-                const currentPlaintext = (await decryptSecret(existingServer.oauthClientSecret))
-                  .decrypted
-                clientSecretChanged = currentPlaintext !== body.oauthClientSecret
-              } catch {
-                clientSecretChanged = true
-              }
-            }
-          }
-          const oauthCredsChanged = clientIdChanged || clientSecretChanged
+          const credsChanged = await oauthCredsChanged({
+            incomingClientId: oauthClientId,
+            incomingClientIdProvided: oauthClientIdProvided,
+            incomingClientSecret: body.oauthClientSecret,
+            incomingClientSecretProvided: oauthClientSecretProvided,
+            currentClientId: existingServer.oauthClientId,
+            currentEncryptedClientSecret: existingServer.oauthClientSecret,
+          })
 
           const isRevival = existingServer.deletedAt !== null
-          const shouldClearOauth = urlChanged || oauthCredsChanged || isRevival
+          const shouldClearOauth = urlChanged || credsChanged || isRevival
 
           if (shouldClearOauth) {
             await revokeMcpOauthTokens(serverId)

apps/sim/hooks/mcp/use-mcp-oauth-popup.ts

@@ -70,6 +70,13 @@ export function useMcpOauthPopup({ workspaceId }: UseMcpOauthPopupProps) {
           next.delete(serverId)
           return next
         })
+      } else if (!data.ok) {
+        // Early callback failures (missing params, invalid state) post back
+        // without a serverId, so we can't target a specific row — clear all
+        // in-flight popups instead of leaving the UI stuck on "Connecting…".
+        for (const id of popupIntervalsRef.current.values()) window.clearInterval(id)
+        popupIntervalsRef.current.clear()
+        setConnectingServers((prev) => (prev.size === 0 ? prev : new Set()))
       }
       if (data.ok) {
         queryClient.invalidateQueries({ queryKey: mcpKeys.serversList(workspaceId) })

apps/sim/hooks/queries/mcp.ts

@@ -270,7 +270,11 @@ export function useUpdateMcpServer() {
       )
 
       if (previousServers) {
-        const { oauthClientSecret: _omitSecret, ...safeUpdates } = updates
+        const { oauthClientSecret: _omitSecret, oauthClientId, ...rest } = updates
+        const safeUpdates: Partial<McpServer> = { ...rest }
+        if (oauthClientId !== undefined) {
+          safeUpdates.oauthClientId = oauthClientId || null
+        }
         queryClient.setQueryData<McpServer[]>(
           mcpKeys.serversList(workspaceId),
           previousServers.map((server) =>

apps/sim/lib/mcp/oauth/creds-diff.ts

@@ -0,0 +1,39 @@
+import { decryptSecret } from '@/lib/core/security/encryption'
+
+interface OauthCredsDiffParams {
+  incomingClientId: string | null | undefined
+  incomingClientIdProvided: boolean
+  incomingClientSecret: string | null | undefined
+  incomingClientSecretProvided: boolean
+  currentClientId: string | null | undefined
+  currentEncryptedClientSecret: string | null | undefined
+}
+
+/**
+ * Detect whether OAuth client credentials on an MCP server row have changed.
+ * Decrypt failure (corrupted ciphertext, rotated key) is treated as a change so
+ * admins can overwrite an unusable stored secret instead of getting a 500.
+ */
+export async function oauthCredsChanged(params: OauthCredsDiffParams): Promise<boolean> {
+  const clientIdChanged =
+    params.incomingClientIdProvided &&
+    (params.incomingClientId || null) !== (params.currentClientId ?? null)
+
+  let clientSecretChanged = false
+  if (params.incomingClientSecretProvided) {
+    if (!params.incomingClientSecret) {
+      clientSecretChanged = params.currentEncryptedClientSecret != null
+    } else if (!params.currentEncryptedClientSecret) {
+      clientSecretChanged = true
+    } else {
+      try {
+        const { decrypted } = await decryptSecret(params.currentEncryptedClientSecret)
+        clientSecretChanged = decrypted !== params.incomingClientSecret
+      } catch {
+        clientSecretChanged = true
+      }
+    }
+  }
+
+  return clientIdChanged || clientSecretChanged
+}

apps/sim/lib/mcp/oauth/index.ts

@@ -2,6 +2,7 @@ export type {
   McpOauthCallbackMessage,
   McpOauthCallbackReason,
 } from './callback-reasons'
+export { oauthCredsChanged } from './creds-diff'
 export { detectMcpAuthType } from './probe'
 export {
   loadPreregisteredClient,