Skip to content

fix: enforce workflow validation before deploy and run #3473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MaxwellCalkin wants to merge 2 commits into
base: main
Choose a base branch
from
+33 −8
Open

fix: enforce workflow validation before deploy and run #3473

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a072602
fix: enforce workflow validation before deploy and run
MaxwellCalkin Mar 9, 2026
2766a39
fix: address review feedback on validation logic
MaxwellCalkin Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions apps/sim/app/api/workflows/[id]/deploy/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
createSchedulesForDeploy,
validateWorkflowSchedules,
} from '@/lib/workflows/schedules'
import { validateWorkflowState } from '@/lib/workflows/sanitization/validation'
import { validateWorkflowPermissions } from '@/lib/workflows/utils'
import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils'
import type { WorkflowState } from '@/stores/workflows/workflow/types'
Expand Down Expand Up @@ -134,6 +135,21 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
return createErrorResponse('Failed to load workflow state', 500)
}

// Validate workflow state (block types, edges, tool references)
const workflowValidation = validateWorkflowState({
blocks: normalizedData.blocks,
edges: normalizedData.edges,
loops: normalizedData.loops,
parallels: normalizedData.parallels,
} as WorkflowState)
Comment on lines +139 to +145
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type assertion papers over a missing variables field

normalizedData may not include a variables property, so the cast as WorkflowState silently satisfies TypeScript while potentially producing undefined at runtime for that field. validateWorkflowState only uses blocks and edges, but if the function signature ever evolves, this cast could hide a real gap.

Consider spreading a safe default instead:

Suggested change
const workflowValidation = validateWorkflowState({
blocks: normalizedData.blocks,
edges: normalizedData.edges,
loops: normalizedData.loops,
parallels: normalizedData.parallels,
} as WorkflowState)
const workflowValidation = validateWorkflowState({
blocks: normalizedData.blocks,
edges: normalizedData.edges,
loops: normalizedData.loops,
parallels: normalizedData.parallels,
variables: {},
} as WorkflowState)

if (!workflowValidation.valid) {
const errorSummary = workflowValidation.errors.join('; ')
logger.warn(
`[${requestId}] Workflow validation failed for ${id}: ${errorSummary}`
)
return createErrorResponse(`Workflow validation failed: ${errorSummary}`, 400)
}

const scheduleValidation = validateWorkflowSchedules(normalizedData.blocks)
if (!scheduleValidation.isValid) {
logger.warn(
Expand Down
13 changes: 7 additions & 6 deletions ...e/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,13 @@ export function useDeployment({

/**
* Handle deploy button click
* First deploy: calls API to deploy, then opens modal on success
* Already deployed: opens modal directly (validation happens on Update in modal)
* First deploy: runs pre-deploy checks, calls API to deploy, then opens modal on success
* Already deployed: runs pre-deploy checks, then opens modal (redeployment happens in modal)
*/
const handleDeployClick = useCallback(async () => {
if (!workflowId) return { success: false, shouldOpenModal: false }

if (isDeployed) {
return { success: true, shouldOpenModal: true }
}

// Always run pre-deploy checks, even for redeployments
const { blocks, edges, loops, parallels } = useWorkflowStore.getState()
const liveBlocks = mergeSubblockState(blocks, workflowId)
const checkResult = runPreDeployChecks({
Expand All @@ -56,6 +53,10 @@ export function useDeployment({
return { success: false, shouldOpenModal: false }
}

if (isDeployed) {
return { success: true, shouldOpenModal: true }
}

setIsDeploying(true)
try {
const response = await fetch(`/api/workflows/${workflowId}/deploy`, {
Expand Down
6 changes: 5 additions & 1 deletion apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,11 @@ export const Panel = memo(function Panel() {
// Compute run button state
const canRun = userPermissions.canRead // Running only requires read permissions
const isLoadingPermissions = userPermissions.isLoading
const hasValidationErrors = false // TODO: Add validation logic if needed

// Validate workflow has connected blocks (at least one edge means blocks are wired together)
const hasEdges = useWorkflowStore((state) => state.edges.length > 0)
const hasValidationErrors = hasBlocks && !hasEdges
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-block workflows incorrectly blocked by edge check

hasValidationErrors = hasBlocks && !hasEdges disables the Run button for any workflow that has blocks but zero edges. A workflow consisting of exactly one standalone block (e.g. a single agent block) has no edges by definition and would always show the button as disabled — even though it may be a perfectly valid, runnable workflow.

The intent (catching disconnected/dangling blocks) is good, but using edges.length === 0 as the sole proxy is too coarse. Consider checking that the starter block has at least one outgoing edge, or that the graph is connected, rather than requiring any edge to exist across the whole workflow.


Comment thread
cursor[bot] marked this conversation as resolved.
const isWorkflowBlocked = isExecuting || hasValidationErrors
const isButtonDisabled = !isExecuting && (isWorkflowBlocked || (!canRun && !isLoadingPermissions))

Expand Down