Skip to content

Commit 4fd9cf3

Browse files
waleedlatif1claude
waleedlatif1
and
claude
committed
refactor(mcp): cleanup pass — emcn buttons, derived state, OAuth header bug
- Prevent stored Authorization header from overwriting OAuth Bearer in McpClient - Per-user connection cache keying in connection-manager (token-leak prevention) - Tighten types in use-mcp-tools and tools/execute (drop `any`) - Replace raw <button> with emcn Button in mcp settings + form modal - Modal Cancel: variant='ghost' → 'default' to match design system - Derive editInitialData and showDeleteDialog from existing state - Replace refreshingServers Record + chained timer with mutation state - Trigger MCP OAuth start on create when authType==='oauth' from tool-input - Invalidate servers/storedTools alongside tools on force-refresh Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 0de4158 commit 4fd9cf3

8 files changed

Lines changed: 149 additions & 132 deletions

File tree

‎apps/sim/app/api/mcp/tools/execute/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ export const POST = withRouteHandler(
111111

112112
if (tool.inputSchema?.properties) {
113113
for (const [paramName, paramSchema] of Object.entries(tool.inputSchema.properties)) {
114-
const schema = paramSchema as any
114+
const schema = hasType(paramSchema) ? paramSchema : null
115+
if (!schema) continue
115116
const value = args[paramName]
116117

117118
if (value === undefined || value === null) {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -700,18 +700,19 @@ export function McpServerFormModal({
700700
</div>
701701
</div>
702702

703-
<button
703+
<Button
704704
type='button'
705+
variant='ghost'
705706
onClick={() => setShowAdvanced((v) => !v)}
706-
className='mt-1 flex items-center gap-1 self-start text-[var(--text-secondary)] text-small hover-hover:text-[var(--text-primary)]'
707+
className='mt-1 gap-1 self-start px-0 py-0 text-small'
707708
>
708709
{showAdvanced ? (
709710
<ChevronDown className='h-[14px] w-[14px]' />
710711
) : (
711712
<ChevronRight className='h-[14px] w-[14px]' />
712713
)}
713714
Advanced settings
714-
</button>
715+
</Button>
715716
{showAdvanced && (
716717
<div className='flex flex-col gap-2'>
717718
<FormField label='OAuth Client ID (optional)'>
@@ -779,7 +780,7 @@ export function McpServerFormModal({
779780
)}
780781
</div>
781782
<div className='flex items-center gap-2'>
782-
<Button variant='ghost' onClick={() => onOpenChange(false)}>
783+
<Button variant='default' onClick={() => onOpenChange(false)}>
783784
Cancel
784785
</Button>
785786
{formMode === 'json' ? (

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

Lines changed: 62 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,25 @@ function ServerListItem({
129129
)
130130
}
131131

132+
function buildEditInitialData(server: McpServer) {
133+
const entries: { key: string; value: string }[] = server.headers
134+
? Object.entries(server.headers).map(([key, value]) => ({ key, value }))
135+
: []
136+
if (entries.length === 0) entries.push({ key: '', value: '' })
137+
const last = entries[entries.length - 1]
138+
if (last.key !== '' || last.value !== '') entries.push({ key: '', value: '' })
139+
140+
return {
141+
name: server.name || '',
142+
transport: (server.transport as McpTransport) || 'streamable-http',
143+
url: server.url || '',
144+
timeout: 30000,
145+
headers: entries,
146+
oauthClientId: server.oauthClientId || undefined,
147+
hasOauthClientSecret: server.hasOauthClientSecret === true,
148+
}
149+
}
150+
132151
interface MCPProps {
133152
initialServerId?: string | null
134153
}
@@ -165,19 +184,7 @@ export function MCP({ initialServerId }: MCPProps) {
165184
const { data: allowedMcpDomains = null } = useAllowedMcpDomains()
166185

167186
const [showAddModal, setShowAddModal] = useState(false)
168-
const [showEditModal, setShowEditModal] = useState(false)
169-
const [editInitialData, setEditInitialData] = useState<
170-
| {
171-
name: string
172-
transport: McpTransport
173-
url?: string
174-
timeout?: number
175-
headers?: { key: string; value: string }[]
176-
oauthClientId?: string
177-
hasOauthClientSecret?: boolean
178-
}
179-
| undefined
180-
>(undefined)
187+
const [editingServerId, setEditingServerId] = useState<string | null>(null)
181188

182189
const [searchTerm, setSearchTerm] = useState('')
183190
const [deletingServers, setDeletingServers] = useState<Set<string>>(() => new Set())
@@ -192,8 +199,8 @@ export function MCP({ initialServerId }: MCPProps) {
192199
}
193200
}, [])
194201

195-
const [showDeleteDialog, setShowDeleteDialog] = useState(false)
196202
const [serverToDeleteId, setServerToDeleteId] = useState<string | null>(null)
203+
const showDeleteDialog = serverToDeleteId !== null
197204

198205
const [selectedServerId, setSelectedServerId] = useState<string | null>(initialServerId ?? null)
199206

@@ -238,9 +245,6 @@ export function MCP({ initialServerId }: MCPProps) {
238245
return () => window.removeEventListener('message', onMessage)
239246
}, [queryClient, workspaceId])
240247

241-
const [refreshingServers, setRefreshingServers] = useState<
242-
Record<string, { status: 'refreshing' | 'refreshed'; workflowsUpdated?: number }>
243-
>({})
244248
const [expandedTools, setExpandedTools] = useState<Set<string>>(() => new Set())
245249

246250
const startOauthForServer = async (serverId: string) => {
@@ -277,13 +281,11 @@ export function MCP({ initialServerId }: MCPProps) {
277281

278282
const handleRemoveServer = (serverId: string) => {
279283
setServerToDeleteId(serverId)
280-
setShowDeleteDialog(true)
281284
}
282285

283286
const confirmDeleteServer = async () => {
284287
if (!serverToDeleteId) return
285288

286-
setShowDeleteDialog(false)
287289
const serverId = serverToDeleteId
288290
setServerToDeleteId(null)
289291

@@ -344,7 +346,6 @@ export function MCP({ initialServerId }: MCPProps) {
344346

345347
const handleRefreshServer = async (serverId: string) => {
346348
try {
347-
setRefreshingServers((prev) => ({ ...prev, [serverId]: { status: 'refreshing' } }))
348349
const result = await refreshServerMutation.mutateAsync({ workspaceId, serverId })
349350
logger.info(
350351
`Refreshed MCP server: ${serverId}, workflows updated: ${result.workflowsUpdated}`
@@ -369,50 +370,30 @@ export function MCP({ initialServerId }: MCPProps) {
369370
logger.warn('Failed to reload workflow subblock values:', reloadError)
370371
}
371372
}
372-
373-
setRefreshingServers((prev) => ({
374-
...prev,
375-
[serverId]: { status: 'refreshed', workflowsUpdated: result.workflowsUpdated },
376-
}))
377-
setTimeout(() => {
378-
setRefreshingServers((prev) => {
379-
const newState = { ...prev }
380-
delete newState[serverId]
381-
return newState
382-
})
383-
}, 3000)
384373
} catch (error) {
385374
logger.error('Failed to refresh MCP server:', error)
386-
setRefreshingServers((prev) => {
387-
const newState = { ...prev }
388-
delete newState[serverId]
389-
return newState
390-
})
391375
}
392376
}
393377

394-
const handleOpenEditModal = (server: McpServer) => {
395-
const headers: { key: string; value: string }[] = server.headers
396-
? Object.entries(server.headers).map(([key, value]) => ({ key, value }))
397-
: [{ key: '', value: '' }]
398-
if (headers.length === 0) headers.push({ key: '', value: '' })
399-
400-
const lastHeader = headers[headers.length - 1]
401-
if (lastHeader.key !== '' || lastHeader.value !== '') {
402-
headers.push({ key: '', value: '' })
403-
}
404-
405-
setEditInitialData({
406-
name: server.name || '',
407-
transport: (server.transport as McpTransport) || 'streamable-http',
408-
url: server.url || '',
409-
timeout: 30000,
410-
headers,
411-
oauthClientId: server.oauthClientId || undefined,
412-
hasOauthClientSecret: server.hasOauthClientSecret === true,
413-
})
414-
setShowEditModal(true)
415-
}
378+
useEffect(() => {
379+
if (!refreshServerMutation.isSuccess) return
380+
const timeout = window.setTimeout(() => refreshServerMutation.reset(), 3000)
381+
return () => window.clearTimeout(timeout)
382+
// eslint-disable-next-line react-hooks/exhaustive-deps -- mutation object is unstable; isSuccess flag is the trigger
383+
}, [refreshServerMutation.isSuccess])
384+
385+
const refreshingServerId = refreshServerMutation.isPending
386+
? refreshServerMutation.variables?.serverId
387+
: null
388+
const refreshedServerId = refreshServerMutation.isSuccess
389+
? refreshServerMutation.variables?.serverId
390+
: null
391+
const refreshedWorkflowsUpdated = refreshServerMutation.data?.workflowsUpdated
392+
393+
const editingServer = editingServerId
394+
? (servers.find((s) => s.id === editingServerId) as McpServer | undefined)
395+
: undefined
396+
const editInitialData = editingServer ? buildEditInitialData(editingServer) : undefined
416397

417398
const selectedServer = (() => {
418399
if (!selectedServerId) return null
@@ -547,11 +528,12 @@ export function MCP({ initialServerId }: MCPProps) {
547528
key={tool.name}
548529
className='overflow-hidden rounded-md border bg-[var(--surface-3)]'
549530
>
550-
<button
531+
<Button
551532
type='button'
533+
variant='ghost'
552534
onClick={() => hasParams && toggleToolExpanded(tool.name)}
553535
className={cn(
554-
'flex w-full items-start justify-between px-2.5 py-2 text-left',
536+
'flex h-auto w-full items-start justify-between rounded-none px-2.5 py-2 text-left text-sm',
555537
hasParams && 'cursor-pointer hover-hover:bg-[var(--surface-4)]'
556538
)}
557539
disabled={!hasParams}
@@ -593,7 +575,7 @@ export function MCP({ initialServerId }: MCPProps) {
593575
)}
594576
/>
595577
)}
596-
</button>
578+
</Button>
597579

598580
{isExpanded && hasParams && (
599581
<div className='border-[var(--border-1)] border-t bg-[var(--surface-2)] px-2.5 py-2'>
@@ -660,25 +642,27 @@ export function MCP({ initialServerId }: MCPProps) {
660642
<Button
661643
onClick={() => handleRefreshServer(server.id)}
662644
variant='default'
663-
disabled={!!refreshingServers[server.id]}
645+
disabled={refreshingServerId === server.id || refreshedServerId === server.id}
664646
>
665-
{refreshingServers[server.id]?.status === 'refreshing'
647+
{refreshingServerId === server.id
666648
? 'Refreshing...'
667-
: refreshingServers[server.id]?.status === 'refreshed'
668-
? refreshingServers[server.id].workflowsUpdated
669-
? `Synced (${refreshingServers[server.id].workflowsUpdated} workflow${refreshingServers[server.id].workflowsUpdated === 1 ? '' : 's'})`
649+
: refreshedServerId === server.id
650+
? refreshedWorkflowsUpdated
651+
? `Synced (${refreshedWorkflowsUpdated} workflow${refreshedWorkflowsUpdated === 1 ? '' : 's'})`
670652
: 'Refreshed'
671653
: 'Refresh Tools'}
672654
</Button>
673-
<Button onClick={() => handleOpenEditModal(server)} variant='default'>
655+
<Button onClick={() => setEditingServerId(server.id)} variant='default'>
674656
Edit
675657
</Button>
676658
</div>
677659
</div>
678660

679661
<McpServerFormModal
680-
open={showEditModal}
681-
onOpenChange={setShowEditModal}
662+
open={editingServerId !== null}
663+
onOpenChange={(open) => {
664+
if (!open) setEditingServerId(null)
665+
}}
682666
mode='edit'
683667
initialData={editInitialData}
684668
onSubmit={async (config) => {
@@ -753,7 +737,7 @@ export function MCP({ initialServerId }: MCPProps) {
753737
tools={tools}
754738
isDeleting={deletingServers.has(server.id)}
755739
isLoadingTools={isLoadingTools}
756-
isRefreshing={refreshingServers[server.id]?.status === 'refreshing'}
740+
isRefreshing={refreshingServerId === server.id}
757741
onRemove={() => handleRemoveServer(server.id)}
758742
onViewDetails={() => handleViewDetails(server.id)}
759743
/>
@@ -787,7 +771,12 @@ export function MCP({ initialServerId }: MCPProps) {
787771
allowedMcpDomains={allowedMcpDomains}
788772
/>
789773

790-
<Modal open={showDeleteDialog} onOpenChange={setShowDeleteDialog}>
774+
<Modal
775+
open={showDeleteDialog}
776+
onOpenChange={(open) => {
777+
if (!open) setServerToDeleteId(null)
778+
}}
779+
>
791780
<ModalContent size='sm'>
792781
<ModalHeader>Delete MCP Server</ModalHeader>
793782
<ModalBody>
@@ -800,7 +789,7 @@ export function MCP({ initialServerId }: MCPProps) {
800789
</p>
801790
</ModalBody>
802791
<ModalFooter>
803-
<Button variant='default' onClick={() => setShowDeleteDialog(false)}>
792+
<Button variant='default' onClick={() => setServerToDeleteId(null)}>
804793
Cancel
805794
</Button>
806795
<Button variant='destructive' onClick={confirmDeleteServer}>

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
useForceRefreshMcpTools,
6565
useMcpServers,
6666
useMcpToolsEvents,
67+
useStartMcpOauth,
6768
useStoredMcpTools,
6869
} from '@/hooks/queries/mcp'
6970
import { useWorkflowState, useWorkflows } from '@/hooks/queries/workflows'
@@ -536,6 +537,7 @@ export const ToolInput = memo(function ToolInput({
536537
useMcpToolsEvents(workspaceId)
537538
const { navigateToSettings } = useSettingsNavigation()
538539
const createMcpServer = useCreateMcpServer()
540+
const startMcpOauth = useStartMcpOauth()
539541
const { data: allowedMcpDomains = null } = useAllowedMcpDomains()
540542
const availableEnvVars = useAvailableEnvVarKeys(workspaceId)
541543
const mcpDataLoading = mcpLoading || mcpServersLoading
@@ -2135,7 +2137,13 @@ export const ToolInput = memo(function ToolInput({
21352137
onOpenChange={setMcpModalOpen}
21362138
mode='add'
21372139
onSubmit={async (config) => {
2138-
await createMcpServer.mutateAsync({ workspaceId, config: { ...config, enabled: true } })
2140+
const result = await createMcpServer.mutateAsync({
2141+
workspaceId,
2142+
config: { ...config, enabled: true },
2143+
})
2144+
if (result.authType === 'oauth') {
2145+
await startMcpOauth.mutateAsync({ serverId: result.id, workspaceId })
2146+
}
21392147
}}
21402148
workspaceId={workspaceId}
21412149
availableEnvVars={availableEnvVars}

‎apps/sim/hooks/mcp/use-mcp-tools.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
* using TanStack Query for optimal caching and performance
66
*/
77

8-
import type React from 'react'
8+
import type { ComponentType, SVGProps } from 'react'
99
import { useCallback, useMemo } from 'react'
1010
import { createLogger } from '@sim/logger'
1111
import { useQueryClient } from '@tanstack/react-query'
1212
import { McpIcon } from '@/components/icons'
1313
import { createMcpToolId } from '@/lib/mcp/shared'
14+
import type { McpToolSchema } from '@/lib/mcp/types'
1415
import { mcpKeys, useMcpToolsQuery } from '@/hooks/queries/mcp'
1516

1617
const logger = createLogger('useMcpTools')
@@ -22,9 +23,9 @@ export interface McpToolForUI {
2223
serverId: string
2324
serverName: string
2425
type: 'mcp'
25-
inputSchema: any
26+
inputSchema: McpToolSchema
2627
bgColor: string
27-
icon: React.ComponentType<any>
28+
icon: ComponentType<SVGProps<SVGSVGElement>>
2829
}
2930

3031
export interface UseMcpToolsResult {

‎apps/sim/hooks/queries/mcp.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ export function useForceRefreshMcpTools() {
118118
mutationFn: (workspaceId: string) => fetchMcpTools(workspaceId, true),
119119
onSettled: (_data, _error, workspaceId) => {
120120
queryClient.invalidateQueries({ queryKey: mcpKeys.toolsList(workspaceId) })
121+
queryClient.invalidateQueries({ queryKey: mcpKeys.serversList(workspaceId) })
122+
queryClient.invalidateQueries({ queryKey: mcpKeys.storedToolsList(workspaceId) })
121123
},
122124
})
123125
}

‎apps/sim/lib/mcp/client.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ export class McpClient {
9393
const useOauth = this.config.authType === 'oauth' && this.authProvider != null
9494
this.transport = new StreamableHTTPClientTransport(new URL(this.config.url), {
9595
authProvider: useOauth ? this.authProvider : undefined,
96-
requestInit: {
97-
headers: this.config.headers,
98-
},
96+
requestInit: useOauth ? undefined : { headers: this.config.headers },
9997
})
10098

10199
this.client = new Client(

0 commit comments

Comments
 (0)