Skip to content

Commit 7f1c651

Browse files
waleedlatif1claude
andcommitted
fix(mcp): clear OAuth tokens on server revival; clear popup intervals on unmount
- POST upsert: when reviving a soft-deleted server, drop any prior mcpServerOauth rows so stale tokens never silently carry over. - mcp.tsx: track the popup-closed setInterval per server in a ref and clear it on component unmount to avoid leaked timers. - client.ts: don't log OAuth-redirect/Unauthorized as connect errors; these are expected control flow during the auth bootstrap. - Use toError() for error message extraction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 20fb108 commit 7f1c651

4 files changed

Lines changed: 37 additions & 16 deletions

File tree

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,16 @@ export const POST = withRouteHandler(
165165
}
166166
const oauthCredsChanged = clientIdChanged || clientSecretChanged
167167

168-
if (urlChanged || oauthCredsChanged) {
168+
const isRevival = existingServer.deletedAt !== null
169+
if (urlChanged || oauthCredsChanged || isRevival) {
169170
await db.delete(mcpServerOauth).where(eq(mcpServerOauth.mcpServerId, serverId))
171+
const reason = isRevival
172+
? 'server revival'
173+
: urlChanged
174+
? 'URL change'
175+
: 'OAuth credential change'
170176
logger.info(
171-
`[${requestId}] Cleared OAuth credentials for server ${serverId} due to ${urlChanged ? 'URL' : 'OAuth credential'} change`
177+
`[${requestId}] Cleared OAuth credentials for server ${serverId} due to ${reason}`
172178
)
173179
}
174180

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
5+
import { toError } from '@sim/utils/errors'
56
import { ChevronDown, ChevronRight } from 'lucide-react'
67
import {
78
Button,
@@ -556,8 +557,7 @@ export function McpServerFormModal({
556557

557558
onOpenChange(false)
558559
} catch (error) {
559-
const message = error instanceof Error ? error.message : 'Failed to save server'
560-
setSubmitError(message)
560+
setSubmitError(toError(error).message || 'Failed to save server')
561561
logger.error('Failed to save MCP server:', error)
562562
} finally {
563563
setIsSubmitting(false)
@@ -618,8 +618,7 @@ export function McpServerFormModal({
618618

619619
onOpenChange(false)
620620
} catch (error) {
621-
const message = error instanceof Error ? error.message : 'Failed to save server'
622-
setSubmitError(message)
621+
setSubmitError(toError(error).message || 'Failed to save server')
623622
logger.error('Failed to save MCP server from JSON:', error)
624623
} finally {
625624
setIsSubmitting(false)

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use client'
22

3-
import { useCallback, useEffect, useMemo, useState } from 'react'
3+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
5+
import { toError } from '@sim/utils/errors'
56
import { useQueryClient } from '@tanstack/react-query'
67
import { ChevronDown, Plus, Search } from 'lucide-react'
78
import { useParams } from 'next/navigation'
@@ -177,6 +178,15 @@ export function MCP({ initialServerId }: MCPProps) {
177178
const [searchTerm, setSearchTerm] = useState('')
178179
const [deletingServers, setDeletingServers] = useState<Set<string>>(() => new Set())
179180
const [connectingOauthServers, setConnectingOauthServers] = useState<Set<string>>(() => new Set())
181+
const oauthPopupIntervalsRef = useRef<Map<string, number>>(new Map())
182+
183+
useEffect(() => {
184+
const intervals = oauthPopupIntervalsRef.current
185+
return () => {
186+
for (const id of intervals.values()) window.clearInterval(id)
187+
intervals.clear()
188+
}
189+
}, [])
180190

181191
const [showDeleteDialog, setShowDeleteDialog] = useState(false)
182192
const [serverToDelete, setServerToDelete] = useState<{ id: string; name: string } | null>(null)
@@ -218,12 +228,18 @@ export function MCP({ initialServerId }: MCPProps) {
218228
const startOauthForServer = useCallback(
219229
async (serverId: string) => {
220230
setConnectingOauthServers((prev) => new Set(prev).add(serverId))
221-
const clear = () =>
231+
const clear = () => {
232+
const existing = oauthPopupIntervalsRef.current.get(serverId)
233+
if (existing !== undefined) {
234+
window.clearInterval(existing)
235+
oauthPopupIntervalsRef.current.delete(serverId)
236+
}
222237
setConnectingOauthServers((prev) => {
223238
const next = new Set(prev)
224239
next.delete(serverId)
225240
return next
226241
})
242+
}
227243
try {
228244
const result = await startOauthMutation.mutateAsync({ serverId, workspaceId })
229245
if (result.status === 'already_authorized') {
@@ -232,15 +248,13 @@ export function MCP({ initialServerId }: MCPProps) {
232248
}
233249
const { popup } = result
234250
const interval = window.setInterval(() => {
235-
if (popup.closed) {
236-
window.clearInterval(interval)
237-
clear()
238-
}
251+
if (popup.closed) clear()
239252
}, 500)
253+
oauthPopupIntervalsRef.current.set(serverId, interval)
240254
} catch (e) {
241255
clear()
242256
logger.error('Failed to start MCP OAuth', e)
243-
toast.error(e instanceof Error ? e.message : 'Failed to start authorization')
257+
toast.error(toError(e).message || 'Failed to start authorization')
244258
}
245259
},
246260
// eslint-disable-next-line react-hooks/exhaustive-deps -- mutateAsync is stable

apps/sim/lib/mcp/client.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
ToolListChangedNotificationSchema,
1818
} from '@modelcontextprotocol/sdk/types.js'
1919
import { createLogger } from '@sim/logger'
20+
import { toError } from '@sim/utils/errors'
2021
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
2122
import { McpOauthRedirectRequired } from '@/lib/mcp/oauth'
2223
import {
@@ -139,13 +140,14 @@ export class McpClient {
139140
protocolVersion: serverVersion,
140141
})
141142
} catch (error) {
142-
const errorMessage = error instanceof Error ? error.message : 'Unknown error'
143-
this.connectionStatus.lastError = errorMessage
144143
this.isConnected = false
145-
logger.error(`Failed to connect to MCP server ${this.config.name}:`, error)
146144
if (error instanceof McpOauthRedirectRequired || error instanceof UnauthorizedError) {
145+
this.connectionStatus.lastError = undefined
147146
throw error
148147
}
148+
const errorMessage = toError(error).message
149+
this.connectionStatus.lastError = errorMessage
150+
logger.error(`Failed to connect to MCP server ${this.config.name}:`, error)
149151
throw new McpConnectionError(errorMessage, this.config.name)
150152
}
151153
}

0 commit comments

Comments
 (0)