Skip to content

Commit 210bf41

Browse files
author
aadamgough
committed
added validation and greptile comments
1 parent b7a3a4a commit 210bf41

File tree

4 files changed

+205
-76
lines changed

4 files changed

+205
-76
lines changed

‎apps/sim/app/api/mcp/workflow-servers/validate/route.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { createLogger } from '@sim/logger'
2+
import { NextResponse } from 'next/server'
3+
import { getSession } from '@/lib/auth'
4+
import { loadWorkflowFromNormalizedTables } from '@/lib/workflows/persistence/utils'
5+
import { hasValidStartBlockInState } from '@/lib/workflows/triggers/trigger-utils'
6+
7+
const logger = createLogger('ValidateMcpWorkflowsAPI')
8+
9+
/**
10+
* POST /api/mcp/workflow-servers/validate
11+
* Validates if workflows have valid start blocks for MCP usage
12+
*/
13+
export async function POST(request: Request) {
14+
try {
15+
const session = await getSession()
16+
if (!session?.user?.id) {
17+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
18+
}
19+
20+
const body = await request.json()
21+
const { workflowIds } = body
22+
23+
if (!Array.isArray(workflowIds) || workflowIds.length === 0) {
24+
return NextResponse.json({ error: 'workflowIds must be a non-empty array' }, { status: 400 })
25+
}
26+
27+
const results: Record<string, boolean> = {}
28+
29+
for (const workflowId of workflowIds) {
30+
try {
31+
const state = await loadWorkflowFromNormalizedTables(workflowId)
32+
results[workflowId] = hasValidStartBlockInState(state)
33+
} catch (error) {
34+
logger.warn(`Failed to validate workflow ${workflowId}:`, error)
35+
results[workflowId] = false
36+
}
37+
}
38+
39+
return NextResponse.json({ data: results })
40+
} catch (error) {
41+
logger.error('Failed to validate workflows for MCP:', error)
42+
return NextResponse.json({ error: 'Failed to validate workflows' }, { status: 500 })
43+
}
44+
}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/mcp/mcp.tsx‎

Lines changed: 73 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ export function McpDeploy({
147147
})
148148
const [parameterDescriptions, setParameterDescriptions] = useState<Record<string, string>>({})
149149
const [pendingServerChanges, setPendingServerChanges] = useState<Set<string>>(new Set())
150+
const [saveError, setSaveError] = useState<string | null>(null)
150151

151152
const parameterSchema = useMemo(
152153
() => generateParameterSchema(inputFormat, parameterDescriptions),
@@ -302,25 +303,60 @@ export function McpDeploy({
302303
if (selectedServerIds.length === 0) return
303304

304305
onSubmittingChange?.(true)
305-
try {
306-
const actualSet = new Set(actualServerIds)
307-
const toAdd = selectedServerIds.filter((id) => !actualSet.has(id))
308-
const toRemove = actualServerIds.filter((id) => !selectedServerIds.includes(id))
309-
const toUpdate = selectedServerIds.filter((id) => actualSet.has(id))
306+
setSaveError(null)
307+
308+
const actualSet = new Set(actualServerIds)
309+
const toAdd = selectedServerIds.filter((id) => !actualSet.has(id))
310+
const toRemove = actualServerIds.filter((id) => !selectedServerIds.includes(id))
311+
const toUpdate = selectedServerIds.filter((id) => actualSet.has(id))
312+
313+
const errors: string[] = []
314+
315+
for (const serverId of toAdd) {
316+
setPendingServerChanges((prev) => new Set(prev).add(serverId))
317+
try {
318+
await addToolMutation.mutateAsync({
319+
workspaceId,
320+
serverId,
321+
workflowId,
322+
toolName: toolName.trim(),
323+
toolDescription: toolDescription.trim() || undefined,
324+
parameterSchema,
325+
})
326+
onAddedToServer?.()
327+
logger.info(`Added workflow ${workflowId} as tool to server ${serverId}`)
328+
} catch (error) {
329+
const serverName = servers.find((s) => s.id === serverId)?.name || serverId
330+
errors.push(`Failed to add to "${serverName}"`)
331+
logger.error(`Failed to add tool to server ${serverId}:`, error)
332+
} finally {
333+
setPendingServerChanges((prev) => {
334+
const next = new Set(prev)
335+
next.delete(serverId)
336+
return next
337+
})
338+
}
339+
}
310340

311-
for (const serverId of toAdd) {
341+
for (const serverId of toRemove) {
342+
const toolInfo = serverToolsMap[serverId]
343+
if (toolInfo?.tool) {
312344
setPendingServerChanges((prev) => new Set(prev).add(serverId))
313345
try {
314-
await addToolMutation.mutateAsync({
346+
await deleteToolMutation.mutateAsync({
315347
workspaceId,
316348
serverId,
317-
workflowId,
318-
toolName: toolName.trim(),
319-
toolDescription: toolDescription.trim() || undefined,
320-
parameterSchema,
349+
toolId: toolInfo.tool.id,
350+
})
351+
setServerToolsMap((prev) => {
352+
const next = { ...prev }
353+
delete next[serverId]
354+
return next
321355
})
322-
onAddedToServer?.()
323-
logger.info(`Added workflow ${workflowId} as tool to server ${serverId}`)
356+
} catch (error) {
357+
const serverName = servers.find((s) => s.id === serverId)?.name || serverId
358+
errors.push(`Failed to remove from "${serverName}"`)
359+
logger.error(`Failed to remove tool from server ${serverId}:`, error)
324360
} finally {
325361
setPendingServerChanges((prev) => {
326362
const next = new Set(prev)
@@ -329,35 +365,13 @@ export function McpDeploy({
329365
})
330366
}
331367
}
368+
}
332369

333-
for (const serverId of toRemove) {
334-
const toolInfo = serverToolsMap[serverId]
335-
if (toolInfo?.tool) {
336-
setPendingServerChanges((prev) => new Set(prev).add(serverId))
337-
try {
338-
await deleteToolMutation.mutateAsync({
339-
workspaceId,
340-
serverId,
341-
toolId: toolInfo.tool.id,
342-
})
343-
setServerToolsMap((prev) => {
344-
const next = { ...prev }
345-
delete next[serverId]
346-
return next
347-
})
348-
} finally {
349-
setPendingServerChanges((prev) => {
350-
const next = new Set(prev)
351-
next.delete(serverId)
352-
return next
353-
})
354-
}
355-
}
356-
}
357-
358-
for (const serverId of toUpdate) {
359-
const toolInfo = serverToolsMap[serverId]
360-
if (toolInfo?.tool) {
370+
for (const serverId of toUpdate) {
371+
const toolInfo = serverToolsMap[serverId]
372+
if (toolInfo?.tool) {
373+
setPendingServerChanges((prev) => new Set(prev).add(serverId))
374+
try {
361375
await updateToolMutation.mutateAsync({
362376
workspaceId,
363377
serverId,
@@ -366,23 +380,35 @@ export function McpDeploy({
366380
toolDescription: toolDescription.trim() || undefined,
367381
parameterSchema,
368382
})
383+
} catch (error) {
384+
const serverName = servers.find((s) => s.id === serverId)?.name || serverId
385+
errors.push(`Failed to update "${serverName}"`)
386+
logger.error(`Failed to update tool on server ${serverId}:`, error)
387+
} finally {
388+
setPendingServerChanges((prev) => {
389+
const next = new Set(prev)
390+
next.delete(serverId)
391+
return next
392+
})
369393
}
370394
}
395+
}
371396

372-
refetchServers()
397+
refetchServers()
373398

399+
if (errors.length > 0) {
400+
setSaveError(errors.join('. '))
401+
} else {
374402
setPendingSelectedServerIds(null)
375403
setSavedValues({
376404
toolName,
377405
toolDescription,
378406
parameterDescriptions: { ...parameterDescriptions },
379407
})
380408
onCanSaveChange?.(false)
381-
onSubmittingChange?.(false)
382-
} catch (error) {
383-
logger.error('Failed to save tool configuration:', error)
384-
onSubmittingChange?.(false)
385409
}
410+
411+
onSubmittingChange?.(false)
386412
}, [
387413
toolName,
388414
toolDescription,
@@ -582,11 +608,7 @@ export function McpDeploy({
582608
)}
583609
</div>
584610

585-
{addToolMutation.isError && (
586-
<p className='mt-[6.5px] text-[12px] text-[var(--text-error)]'>
587-
{addToolMutation.error?.message || 'Failed to add tool'}
588-
</p>
589-
)}
611+
{saveError && <p className='mt-[6.5px] text-[12px] text-[var(--text-error)]'>{saveError}</p>}
590612
</form>
591613
)
592614
}

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

Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
204204
return availableWorkflows.find((w) => w.id === selectedWorkflowId)
205205
}, [availableWorkflows, selectedWorkflowId])
206206

207+
const selectedWorkflowInvalid = selectedWorkflow && selectedWorkflow.hasStartBlock === false
208+
207209
if (isLoading) {
208210
return (
209211
<div className='flex h-full flex-col gap-[16px]'>
@@ -502,7 +504,12 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
502504
) : undefined
503505
}
504506
/>
505-
{addToolMutation.isError && (
507+
{selectedWorkflowInvalid && (
508+
<p className='text-[11px] text-[var(--text-error)] leading-tight'>
509+
Workflow must have a Start block to be used as an MCP tool
510+
</p>
511+
)}
512+
{addToolMutation.isError && !selectedWorkflowInvalid && (
506513
<p className='text-[11px] text-[var(--text-error)] leading-tight'>
507514
{addToolMutation.error?.message || 'Failed to add workflow'}
508515
</p>
@@ -523,7 +530,7 @@ function ServerDetailView({ workspaceId, serverId, onBack }: ServerDetailViewPro
523530
<Button
524531
variant='tertiary'
525532
onClick={handleAddWorkflow}
526-
disabled={!selectedWorkflowId || addToolMutation.isPending}
533+
disabled={!selectedWorkflowId || selectedWorkflowInvalid || addToolMutation.isPending}
527534
>
528535
{addToolMutation.isPending ? 'Adding...' : 'Add Workflow'}
529536
</Button>
@@ -560,6 +567,7 @@ export function WorkflowMcpServers({ resetKey }: WorkflowMcpServersProps) {
560567
const [selectedServerId, setSelectedServerId] = useState<string | null>(null)
561568
const [serverToDelete, setServerToDelete] = useState<WorkflowMcpServer | null>(null)
562569
const [deletingServers, setDeletingServers] = useState<Set<string>>(new Set())
570+
const [createError, setCreateError] = useState<string | null>(null)
563571

564572
useEffect(() => {
565573
if (resetKey !== undefined) {
@@ -573,37 +581,63 @@ export function WorkflowMcpServers({ resetKey }: WorkflowMcpServersProps) {
573581
return servers.filter((server) => server.name.toLowerCase().includes(search))
574582
}, [servers, searchTerm])
575583

584+
const invalidWorkflows = useMemo(() => {
585+
return selectedWorkflowIds
586+
.map((id) => deployedWorkflows.find((w) => w.id === id))
587+
.filter((w) => w && w.hasStartBlock === false)
588+
.map((w) => w!.name)
589+
}, [selectedWorkflowIds, deployedWorkflows])
590+
591+
const hasInvalidWorkflows = invalidWorkflows.length > 0
592+
576593
const resetForm = useCallback(() => {
577594
setFormData({ name: '' })
578595
setSelectedWorkflowIds([])
579596
setShowAddForm(false)
597+
setCreateError(null)
580598
}, [])
581599

582600
const handleCreateServer = async () => {
583601
if (!formData.name.trim()) return
584602

603+
setCreateError(null)
604+
605+
let server: WorkflowMcpServer | undefined
585606
try {
586-
const server = await createServerMutation.mutateAsync({
607+
server = await createServerMutation.mutateAsync({
587608
workspaceId,
588609
name: formData.name.trim(),
589610
})
611+
} catch (err) {
612+
logger.error('Failed to create server:', err)
613+
setCreateError(err instanceof Error ? err.message : 'Failed to create server')
614+
return
615+
}
590616

591-
if (selectedWorkflowIds.length > 0 && server?.id) {
592-
await Promise.all(
593-
selectedWorkflowIds.map((workflowId) =>
594-
addToolMutation.mutateAsync({
595-
workspaceId,
596-
serverId: server.id,
597-
workflowId,
598-
})
599-
)
600-
)
617+
if (selectedWorkflowIds.length > 0 && server?.id) {
618+
const workflowErrors: string[] = []
619+
620+
for (const workflowId of selectedWorkflowIds) {
621+
try {
622+
await addToolMutation.mutateAsync({
623+
workspaceId,
624+
serverId: server.id,
625+
workflowId,
626+
})
627+
} catch (err) {
628+
const workflowName =
629+
deployedWorkflows.find((w) => w.id === workflowId)?.name || workflowId
630+
workflowErrors.push(workflowName)
631+
logger.error(`Failed to add workflow ${workflowId} to server:`, err)
632+
}
601633
}
602634

603-
resetForm()
604-
} catch (err) {
605-
logger.error('Failed to create server:', err)
635+
if (workflowErrors.length > 0) {
636+
setCreateError(`Server created but failed to add workflows: ${workflowErrors.join(', ')}`)
637+
}
606638
}
639+
640+
resetForm()
607641
}
608642

609643
const handleDeleteServer = async () => {
@@ -694,6 +728,13 @@ export function WorkflowMcpServers({ resetKey }: WorkflowMcpServersProps) {
694728
disabled={deployedWorkflows.length === 0}
695729
/>
696730
</FormField>
731+
{hasInvalidWorkflows && (
732+
<p className='ml-[112px] text-[11px] text-[var(--text-error)] leading-tight'>
733+
Workflow must have a Start block to be used as an MCP tool
734+
</p>
735+
)}
736+
737+
{createError && <p className='text-[12px] text-[var(--text-error)]'>{createError}</p>}
697738

698739
<div className='flex items-center justify-end gap-[8px] pt-[4px]'>
699740
<Button variant='ghost' onClick={resetForm}>
@@ -702,7 +743,10 @@ export function WorkflowMcpServers({ resetKey }: WorkflowMcpServersProps) {
702743
<Button
703744
onClick={handleCreateServer}
704745
disabled={
705-
!isFormValid || createServerMutation.isPending || addToolMutation.isPending
746+
!isFormValid ||
747+
hasInvalidWorkflows ||
748+
createServerMutation.isPending ||
749+
addToolMutation.isPending
706750
}
707751
variant='tertiary'
708752
>

0 commit comments

Comments
 (0)