fix: enable workflow validation before deployment (was hardcoded to false)#3579
fix: enable workflow validation before deployment (was hardcoded to false)#3579guoyangzhen wants to merge 4 commits intosimstudioai:mainfrom
Conversation
PR SummaryMedium Risk Overview The workflow panel now computes Written by Cursor Bugbot for commit d646ee1. This will update automatically on new commits. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR enables workflow validation before deployment by replacing a hardcoded Key findings:
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Panel (panel.tsx)
participant WorkflowStore
participant validateWorkflowState
participant DeployAPI (route.ts)
User->>Panel (panel.tsx): Edits workflow / opens panel
Panel (panel.tsx)->>WorkflowStore: useWorkflowStore selector
WorkflowStore-->>Panel (panel.tsx): { blocks, edges } (loops/parallels missing ⚠️)
Panel (panel.tsx)->>validateWorkflowState: validateWorkflowState({ blocks, edges })
validateWorkflowState-->>Panel (panel.tsx): { valid, errors }
Note over Panel (panel.tsx): hasValidationErrors controls Run button disabled state
User->>Panel (panel.tsx): Clicks Deploy
Panel (panel.tsx)->>DeployAPI (route.ts): POST /api/workflows/[id]/deploy
DeployAPI (route.ts)->>DeployAPI (route.ts): loadWorkflowFromNormalizedTables(id)
DeployAPI (route.ts)->>validateWorkflowState: validateWorkflowState({ blocks, edges }) (loops/parallels missing ⚠️)
validateWorkflowState-->>DeployAPI (route.ts): { valid, errors }
alt Validation fails (incl. false positives for loops/parallels)
DeployAPI (route.ts)-->>Panel (panel.tsx): 400 Workflow has validation errors
else Validation passes
DeployAPI (route.ts)->>DeployAPI (route.ts): validateWorkflowSchedules(...)
DeployAPI (route.ts)->>DeployAPI (route.ts): deployWorkflow(...)
DeployAPI (route.ts)-->>Panel (panel.tsx): 200 Deployed
end
Last reviewed commit: 70dd04d |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const hasValidationErrors = useWorkflowStore((state) => { | ||
| if (Object.keys(state.blocks).length === 0) return false | ||
| const result = validateWorkflowState({ | ||
| blocks: state.blocks, | ||
| edges: state.edges, | ||
| }) | ||
| return !result.valid | ||
| }) |
There was a problem hiding this comment.
Missing loops and parallels causes false-positive validation errors
validateWorkflowState checks edges against loop and parallel container IDs (lines 270–292 of validation.ts). Because state.loops and state.parallels are not passed here, any workflow that uses a loop or parallel block will have all edges connected to those containers reported as referencing "non-existent" source/target blocks — causing hasValidationErrors to be true for valid workflows and permanently disabling the Run button.
| const hasValidationErrors = useWorkflowStore((state) => { | |
| if (Object.keys(state.blocks).length === 0) return false | |
| const result = validateWorkflowState({ | |
| blocks: state.blocks, | |
| edges: state.edges, | |
| }) | |
| return !result.valid | |
| }) | |
| const hasValidationErrors = useWorkflowStore((state) => { | |
| if (Object.keys(state.blocks).length === 0) return false | |
| const result = validateWorkflowState({ | |
| blocks: state.blocks, | |
| edges: state.edges, | |
| loops: state.loops, | |
| parallels: state.parallels, | |
| }) | |
| return !result.valid | |
| }) |
| const { validateWorkflowState } = await import('@/lib/workflows/sanitization/validation') | ||
| const stateValidation = validateWorkflowState({ | ||
| blocks: normalizedData.blocks, | ||
| edges: normalizedData.edges, | ||
| }) | ||
| if (!stateValidation.valid) { | ||
| logger.warn( | ||
| `[${requestId}] Workflow validation failed for ${id}: ${stateValidation.errors.join('; ')}` | ||
| ) | ||
| return createErrorResponse( | ||
| `Workflow has validation errors: ${stateValidation.errors.join('; ')}`, | ||
| 400 | ||
| ) | ||
| } |
There was a problem hiding this comment.
Missing loops and parallels blocks valid workflow deployments
normalizedData already contains loops and parallels (used in the GET handler on lines 83–86 of this same file), but they are omitted from the validateWorkflowState call here. The validation function uses workflowState.loops || {} and workflowState.parallels || {}, so passing undefined means every edge whose source or target is a loop/parallel container will be flagged as referencing a non-existent block — causing the deployment to be rejected with a 400 even for a perfectly valid workflow.
| const { validateWorkflowState } = await import('@/lib/workflows/sanitization/validation') | |
| const stateValidation = validateWorkflowState({ | |
| blocks: normalizedData.blocks, | |
| edges: normalizedData.edges, | |
| }) | |
| if (!stateValidation.valid) { | |
| logger.warn( | |
| `[${requestId}] Workflow validation failed for ${id}: ${stateValidation.errors.join('; ')}` | |
| ) | |
| return createErrorResponse( | |
| `Workflow has validation errors: ${stateValidation.errors.join('; ')}`, | |
| 400 | |
| ) | |
| } | |
| const stateValidation = validateWorkflowState({ | |
| blocks: normalizedData.blocks, | |
| edges: normalizedData.edges, | |
| loops: normalizedData.loops, | |
| parallels: normalizedData.parallels, | |
| }) |
| const hasValidationErrors = useWorkflowStore((state) => { | ||
| if (Object.keys(state.blocks).length === 0) return false | ||
| const result = validateWorkflowState({ | ||
| blocks: state.blocks, | ||
| edges: state.edges, | ||
| }) | ||
| return !result.valid | ||
| }) |
There was a problem hiding this comment.
Expensive validation runs on every Zustand store update
validateWorkflowState iterates all blocks, calls getBlock() and getTool() per block, and checks all edges — all synchronously inside a Zustand selector. Selectors run on every state change, including every single keystroke in any block's input field. For larger workflows this will cause perceptible UI lag.
Consider memoising outside the selector or debouncing:
// Coarse selector — only re-validates when blocks/edges identity changes
const { blocks, edges, loops, parallels } = useWorkflowStore(
useShallow((state) => ({
blocks: state.blocks,
edges: state.edges,
loops: state.loops,
parallels: state.parallels,
}))
)
const hasValidationErrors = useMemo(() => {
if (Object.keys(blocks).length === 0) return false
return !validateWorkflowState({ blocks, edges, loops, parallels }).valid
}, [blocks, edges, loops, parallels])This keeps the computation lazy but avoids re-running it on unrelated store updates (e.g. execution state, cursor position).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| edges: state.edges, | ||
| }) | ||
| return !result.valid | ||
| }) |
There was a problem hiding this comment.
Validation in selector mutates Zustand store state
Medium Severity
Calling validateWorkflowState inside a Zustand selector causes direct mutation of the store state. The validation function internally calls sanitizeAgentToolsInBlocks, which mutates block.subBlocks.tools.value on the original store objects (line 160 in validation.ts assigns toolsSubBlock.value = cleaned). Since the selector receives references to live store state, this silently rewrites agent tool arrays on every store update — outside of Zustand's set() — violating its immutability contract and potentially causing stale references or subtle inconsistencies in other components.
| edges: state.edges, | ||
| }) | ||
| return !result.valid | ||
| }) |
There was a problem hiding this comment.
Expensive validation runs on every store update
Medium Severity
validateWorkflowState is called inside a Zustand selector, meaning it re-executes on every store state change — not just when blocks or edges change. This includes high-frequency updates like block dragging (position changes at ~60fps) and text input in sub-blocks. The function iterates all blocks, performs registry lookups, runs sanitizeAgentToolsInBlocks (which creates filtered/mapped arrays), builds Set objects from edge data, and allocates multiple temporary objects per call, creating unnecessary GC pressure and main-thread work.
1 participant
Problem
The
hasValidationErrorsflag inpanel.tsxwas hardcoded tofalsewith a TODO comment, allowing completely broken workflows to be deployed. The backend deploy route also only validated schedules, not workflow state.Users could deploy workflows with:
Fix
Frontend (
panel.tsx):const hasValidationErrors = falsewith actualvalidateWorkflowState()callBackend (
deploy/route.ts):validateWorkflowState()check before allowing deploymentDefense in Depth
Fix applied at both layers:
Fixes #3444