Skip to content

Commit 6a3e350

Browse files
committed
fix(mcp): deduplicate json validation, remove redundant state resets, clean up timer
1 parent 9b72b5c commit 6a3e350

File tree

2 files changed

+36
-30
lines changed

2 files changed

+36
-30
lines changed

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/mcp/mcp.tsx

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -719,24 +719,39 @@ export function MCP({ initialServerId }: MCPProps) {
719719
)
720720

721721
/**
722-
* Adds an MCP server from parsed JSON config.
722+
* Validates parsed JSON config for name and domain requirements.
723+
* Returns the config if valid, null otherwise (sets jsonError on failure).
723724
*/
724-
const handleAddServerFromJson = useCallback(async () => {
725+
const validateJsonConfig = useCallback((): {
726+
name: string
727+
url: string
728+
headers: Record<string, string>
729+
} | null => {
725730
const config = parseJsonConfig(jsonInput)
726-
if (!config) return
731+
if (!config) return null
727732

728733
if (!config.name) {
729734
setJsonError(
730735
'Server name is required. Use the mcpServers format: { "mcpServers": { "name": { ... } } }'
731736
)
732-
return
737+
return null
733738
}
734739

735740
if (!isDomainAllowed(config.url, allowedMcpDomains)) {
736741
setJsonError('Domain not permitted by server policy')
737-
return
742+
return null
738743
}
739744

745+
return config
746+
}, [jsonInput, parseJsonConfig, allowedMcpDomains])
747+
748+
/**
749+
* Adds an MCP server from parsed JSON config.
750+
*/
751+
const handleAddServerFromJson = useCallback(async () => {
752+
const config = validateJsonConfig()
753+
if (!config) return
754+
740755
setIsAddingServer(true)
741756
try {
742757
const serverConfig = {
@@ -768,24 +783,13 @@ export function MCP({ initialServerId }: MCPProps) {
768783
})
769784

770785
logger.info(`Added MCP server from JSON: ${config.name}`)
771-
setJsonInput('')
772-
setJsonError(null)
773-
setAddFormMode('form')
774786
resetForm()
775787
} catch (error) {
776788
logger.error('Failed to add MCP server from JSON:', error)
777789
} finally {
778790
setIsAddingServer(false)
779791
}
780-
}, [
781-
jsonInput,
782-
parseJsonConfig,
783-
testConnection,
784-
createServerMutation,
785-
workspaceId,
786-
resetForm,
787-
allowedMcpDomains,
788-
])
792+
}, [validateJsonConfig, testConnection, createServerMutation, workspaceId, resetForm])
789793

790794
/**
791795
* Opens the delete confirmation dialog for an MCP server.
@@ -1639,18 +1643,8 @@ export function MCP({ initialServerId }: MCPProps) {
16391643
<Button
16401644
variant='default'
16411645
onClick={() => {
1642-
const config = parseJsonConfig(jsonInput)
1646+
const config = validateJsonConfig()
16431647
if (!config) return
1644-
if (!config.name) {
1645-
setJsonError(
1646-
'Server name is required. Use the mcpServers format: { "mcpServers": { "name": { ... } } }'
1647-
)
1648-
return
1649-
}
1650-
if (!isDomainAllowed(config.url, allowedMcpDomains)) {
1651-
setJsonError('Domain not permitted by server policy')
1652-
return
1653-
}
16541648
testConnection({
16551649
name: config.name,
16561650
transport: 'streamable-http',

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/workflow-mcp-servers/workflow-mcp-servers.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
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'
55
import { Check, Clipboard, Plus, Search, Server } from 'lucide-react'
66
import { useParams } from 'next/navigation'
@@ -85,6 +85,15 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
8585

8686
const addToWorkspaceMutation = useCreateMcpServer()
8787
const [addedToWorkspace, setAddedToWorkspace] = useState(false)
88+
const addedToWorkspaceTimerRef = useRef<ReturnType<typeof setTimeout>>(null)
89+
90+
useEffect(() => {
91+
return () => {
92+
if (addedToWorkspaceTimerRef.current) {
93+
clearTimeout(addedToWorkspaceTimerRef.current)
94+
}
95+
}
96+
}, [])
8897

8998
const [copiedConfig, setCopiedConfig] = useState(false)
9099
const [activeConfigTab, setActiveConfigTab] = useState<McpClientType>('cursor')
@@ -493,7 +502,10 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
493502
},
494503
})
495504
setAddedToWorkspace(true)
496-
setTimeout(() => setAddedToWorkspace(false), 3000)
505+
addedToWorkspaceTimerRef.current = setTimeout(
506+
() => setAddedToWorkspace(false),
507+
3000
508+
)
497509
} catch (err) {
498510
logger.error('Failed to add server to workspace:', err)
499511
}

0 commit comments

Comments
 (0)