fix: enforce workflow validation before deploy and run #3505
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import type { Edge } from 'reactflow' | ||
| import { getBlock } from '@/blocks/registry' | ||
| import type { SubBlockConfig } from '@/blocks/types' | ||
| import type { BlockState } from '@/stores/workflows/workflow/types' | ||
|
|
||
| export interface WorkflowValidationError { | ||
| blockId: string | ||
| blockName: string | ||
| message: string | ||
| } | ||
|
|
||
| /** | ||
| * Check whether a sub-block's `required` condition is satisfied given current block state. | ||
| */ | ||
| function isSubBlockRequired( | ||
| subBlockConfig: SubBlockConfig, | ||
| blockState: BlockState | ||
| ): boolean { | ||
| const req = subBlockConfig.required | ||
| if (req === undefined || req === false) return false | ||
| if (req === true) return true | ||
|
Comment on lines
+19
to
+21
Contributor
There was a problem hiding this comment. Function variant of The
This produces false-positive validation errors for any sub-block that conditionally declares The function should be called with the current block's sub-block values: if (typeof req === 'function') {
const values = Object.fromEntries(
Object.entries(blockState.subBlocks).map(([k, v]) => [k, v?.value])
)
const resolved = req(values)
// treat resolved like the object variant
...
} |
||
|
|
||
| // Conditional requirement: check field value | ||
| const fieldValue = blockState.subBlocks[req.field]?.value | ||
| const matches = Array.isArray(req.value) | ||
| ? req.value.includes(fieldValue as string | number | boolean) | ||
| : fieldValue === req.value | ||
| const fieldSatisfied = req.not ? !matches : matches | ||
|
|
||
| if (req.and) { | ||
| const andValue = blockState.subBlocks[req.and.field]?.value | ||
| const andMatches = Array.isArray(req.and.value) | ||
| ? req.and.value.includes(andValue as string | number | boolean) | ||
| : andValue === req.and.value | ||
| const andSatisfied = req.and.not ? !andMatches : andMatches | ||
| return fieldSatisfied && andSatisfied | ||
| } | ||
|
|
||
| return fieldSatisfied | ||
| } | ||
|
|
||
| /** | ||
| * Validate that all required sub-block fields in a workflow are filled | ||
| * and that non-trigger blocks have at least one incoming connection. | ||
| */ | ||
| export function validateWorkflowBlocks( | ||
| blocks: Record<string, BlockState>, | ||
| edges: Edge[] | ||
| ): WorkflowValidationError[] { | ||
| const errors: WorkflowValidationError[] = [] | ||
|
|
||
| // Build set of block IDs that have incoming edges | ||
| const blocksWithIncoming = new Set<string>() | ||
| for (const edge of edges) { | ||
| blocksWithIncoming.add(edge.target) | ||
| } | ||
|
|
||
| for (const [blockId, blockState] of Object.entries(blocks)) { | ||
| if (!blockState.enabled) continue | ||
|
|
||
| const blockConfig = getBlock(blockState.type) | ||
| if (!blockConfig) continue | ||
|
|
||
| // Skip container-type blocks (loops, parallels) | ||
| if (blockState.data?.type === 'loop' || blockState.data?.type === 'parallel') continue | ||
|
|
||
| // Check required sub-block fields | ||
| for (const subBlockConfig of blockConfig.subBlocks) { | ||
| if (!isSubBlockRequired(subBlockConfig, blockState)) continue | ||
|
|
||
| const subBlockState = blockState.subBlocks[subBlockConfig.id] | ||
| const value = subBlockState?.value | ||
| const isEmpty = | ||
| value === null || | ||
| value === undefined || | ||
| value === '' || | ||
| (Array.isArray(value) && value.length === 0) | ||
|
|
||
| if (isEmpty) { | ||
| errors.push({ | ||
| blockId, | ||
| blockName: blockState.name, | ||
| message: `Missing required field: ${subBlockConfig.title || subBlockConfig.id}`, | ||
| }) | ||
| } | ||
| } | ||
|
Comment on lines
+67
to
+86
Contributor
There was a problem hiding this comment. Multiple errors per block for the same missing field The loop over Consider deduplicating to at most one error per block (the first failing field), or grouping errors by This is a style/UX suggestion; the current behavior is not broken. Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| // Non-trigger blocks should have at least one incoming connection | ||
| if (blockConfig.category !== 'triggers' && !blocksWithIncoming.has(blockId)) { | ||
| errors.push({ | ||
| blockId, | ||
| blockName: blockState.name, | ||
| message: 'Block is not connected to any input', | ||
| }) | ||
| } | ||
|
There was a problem hiding this comment. Blocks inside containers always fail connection validationHigh Severity The "incoming connection" check doesn't account for blocks that are children of loop/parallel containers. Blocks inside a container have Additional Locations (1) |
||
| } | ||
|
|
||
| return errors | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import ordering violates project alias conventions
The import of
validateWorkflowBlockson line 58 appears after the store imports (@/stores/…), breaking the established grouping order where@/lib/…utilities come before@/stores/…. Per the project's import-path rules, library utilities should be in a distinct group above store imports.(Move the
@/lib/workflows/validationimport up into the@/lib/…import group, before the@/stores/…group.)Rule Used: Import patterns for the Sim application (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!