fix(workflow-edges): enforce edge/block validation server-side, not just client-side

apps/sim/executor/constants.ts

@@ -1,3 +1,7 @@
+import {
+  normalizeWorkflowBlockName,
+  RESERVED_WORKFLOW_BLOCK_NAMES,
+} from '@sim/workflow-types/workflow'
 import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
 import type { LoopType, ParallelType } from '@/lib/workflows/types'
 
@@ -151,11 +155,12 @@ export const SPECIAL_REFERENCE_PREFIXES = [
   REFERENCE.PREFIX.VARIABLE,
 ] as const
 
-export const RESERVED_BLOCK_NAMES = [
-  REFERENCE.PREFIX.LOOP,
-  REFERENCE.PREFIX.PARALLEL,
-  REFERENCE.PREFIX.VARIABLE,
-] as const
+/**
+ * Delegates to the shared implementation in `@sim/workflow-types` so the
+ * client store and the realtime persistence layer agree on the same reserved
+ * names. Values intentionally mirror REFERENCE.PREFIX.{LOOP,PARALLEL,VARIABLE} above.
+ */
+export const RESERVED_BLOCK_NAMES = RESERVED_WORKFLOW_BLOCK_NAMES
 
 export const LOOP_REFERENCE = {
   ITERATION: 'iteration',
@@ -478,12 +483,10 @@ export function escapeRegExp(value: string): string {
  * spaces and dots. Used for both block names and variable names to ensure
  * consistent matching.
  *
- * Dots are stripped because `.` is the reference path delimiter — a name like
- * "Trigger.dev 1" must normalize to "triggerdev1" so the reference
- * `<triggerdev1.output>` parses unambiguously. Dotted names could never be
- * referenced before (the first path segment cut the name at the dot), so
- * stripping dots cannot break any previously working reference.
+ * Delegates to the shared implementation in `@sim/workflow-types` so the
+ * client store and the realtime persistence layer normalize block names
+ * identically when checking for reserved/duplicate names.
  */
 export function normalizeName(name: string): string {
-  return name.toLowerCase().replace(/\s+/g, '').replace(/\./g, '')
+  return normalizeWorkflowBlockName(name)
 }

apps/sim/hooks/use-collaborative-workflow.ts

@@ -12,6 +12,7 @@ import {
   WORKFLOW_OPERATIONS,
 } from '@sim/realtime-protocol/constants'
 import { generateId } from '@sim/utils/id'
+import { getWorkflowBlockNameConflict } from '@sim/workflow-types/workflow'
 import { useQueryClient } from '@tanstack/react-query'
 import { isEqual } from 'es-toolkit'
 import type { Edge } from 'reactflow'
@@ -27,7 +28,6 @@ import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-
 import { useSocket } from '@/app/workspace/providers/socket-provider'
 import { getBlock } from '@/blocks'
 import { getSubBlocksDependingOnChange } from '@/blocks/utils'
-import { normalizeName, RESERVED_BLOCK_NAMES } from '@/executor/constants'
 import { invalidateDeploymentQueries } from '@/hooks/queries/deployments'
 import { useUndoRedo } from '@/hooks/use-undo-redo'
 import {
@@ -54,7 +54,11 @@ import type {
   Position,
   WorkflowState,
 } from '@/stores/workflows/workflow/types'
-import { findAllDescendantNodes, isBlockProtected } from '@/stores/workflows/workflow/utils'
+import {
+  filterAcyclicEdges,
+  findAllDescendantNodes,
+  isBlockProtected,
+} from '@/stores/workflows/workflow/utils'
 
 const logger = createLogger('CollaborativeWorkflow')
 
@@ -1028,27 +1032,26 @@ export function useCollaborativeWorkflow() {
       }
 
       const trimmedName = name.trim()
-      const normalizedNewName = normalizeName(trimmedName)
+      const currentBlocks = useWorkflowStore.getState().blocks
+      const siblingNamesById = Object.fromEntries(
+        Object.entries(currentBlocks).map(([blockId, b]) => [blockId, b.name])
+      )
+      const conflict = getWorkflowBlockNameConflict(id, trimmedName, siblingNamesById)
 
-      if (!normalizedNewName) {
+      if (conflict?.reason === 'empty') {
         logger.error('Cannot rename block to empty name')
         toast.error('Block name cannot be empty')
         return { success: false, error: 'Block name cannot be empty' }
       }
 
-      if ((RESERVED_BLOCK_NAMES as readonly string[]).includes(normalizedNewName)) {
+      if (conflict?.reason === 'reserved') {
         logger.error(`Cannot rename block to reserved name: "${trimmedName}"`)
         toast.error(`"${trimmedName}" is a reserved name and cannot be used`)
         return { success: false, error: `"${trimmedName}" is a reserved name` }
       }
 
-      const currentBlocks = useWorkflowStore.getState().blocks
-      const conflictingBlock = Object.entries(currentBlocks).find(
-        ([blockId, block]) => blockId !== id && normalizeName(block.name) === normalizedNewName
-      )
-
-      if (conflictingBlock) {
-        const conflictName = conflictingBlock[1].name
+      if (conflict?.reason === 'duplicate') {
+        const conflictName = currentBlocks[conflict.conflictingBlockId as string].name
         logger.error(`Cannot rename block to "${trimmedName}" - conflicts with "${conflictName}"`)
         toast.error(`Block name "${trimmedName}" already exists`)
         return { success: false, error: `Block name "${trimmedName}" already exists` }
@@ -1423,7 +1426,12 @@ export function useCollaborativeWorkflow() {
       const currentEdges = useWorkflowStore.getState().edges
       const validEdges = filterValidEdges(edges, blocks)
       const newEdges = filterNewEdges(validEdges, currentEdges)
-      if (newEdges.length === 0) return false
+      // Reject cyclic edges here, before they are queued for realtime/DB
+      // persistence — the local store also runs this check, but only after
+      // an unfiltered payload would already be enqueued. Filtering once
+      // here keeps the queued payload and the local store in agreement.
+      const acyclicEdges = filterAcyclicEdges(newEdges, currentEdges)
+      if (acyclicEdges.length === 0) return false
 
       const operationId = generateId()
 
@@ -1432,16 +1440,16 @@ export function useCollaborativeWorkflow() {
         operation: {
           operation: EDGES_OPERATIONS.BATCH_ADD_EDGES,
           target: OPERATION_TARGETS.EDGES,
-          payload: { edges: newEdges },
+          payload: { edges: acyclicEdges },
         },
         workflowId: activeWorkflowId || '',
         userId: session?.user?.id || 'unknown',
       })
 
-      useWorkflowStore.getState().batchAddEdges(newEdges, { skipValidation: true })
+      useWorkflowStore.getState().batchAddEdges(acyclicEdges, { skipValidation: true })
 
       if (!options?.skipUndoRedo) {
-        newEdges.forEach((edge) => undoRedo.recordAddEdge(edge.id))
+        acyclicEdges.forEach((edge) => undoRedo.recordAddEdge(edge.id))
       }
 
       return true

apps/sim/stores/workflows/utils.ts

@@ -1,5 +1,6 @@
 import { generateId } from '@sim/utils/id'
 import { mergeSubblockStateWithValues } from '@sim/workflow-persistence/subblocks'
+import { filterUniqueWorkflowEdges } from '@sim/workflow-types/workflow'
 import type { Edge } from 'reactflow'
 import { DEFAULT_DUPLICATE_OFFSET } from '@/lib/workflows/autolayout/constants'
 import { getEffectiveBlockOutputs } from '@/lib/workflows/blocks/block-outputs'
@@ -32,16 +33,7 @@ export function filterValidEdges(edges: Edge[], blocks: Record<string, BlockStat
 }
 
 export function filterNewEdges(edgesToAdd: Edge[], currentEdges: Edge[]): Edge[] {
-  return edgesToAdd.filter((edge) => {
-    if (edge.source === edge.target) return false
-    return !currentEdges.some(
-      (e) =>
-        e.source === edge.source &&
-        e.sourceHandle === edge.sourceHandle &&
-        e.target === edge.target &&
-        e.targetHandle === edge.targetHandle
-    )
-  })
+  return filterUniqueWorkflowEdges(edgesToAdd, currentEdges)
 }
 
 export interface RegeneratedState {

apps/sim/stores/workflows/workflow/edge-validation.test.ts

@@ -0,0 +1,99 @@
+import type { Edge } from 'reactflow'
+import { describe, expect, it } from 'vitest'
+import { validateEdges } from '@/stores/workflows/workflow/edge-validation'
+import type { BlockState } from '@/stores/workflows/workflow/types'
+
+function makeBlock(id: string, type: string, overrides?: Partial<BlockState>): BlockState {
+  return {
+    id,
+    type,
+    name: id,
+    position: { x: 0, y: 0 },
+    subBlocks: {},
+    outputs: {},
+    enabled: true,
+    ...overrides,
+  }
+}
+
+function makeEdge(id: string, source: string, target: string): Edge {
+  return { id, source, target, type: 'default' }
+}
+
+describe('validateEdges', () => {
+  it('accepts an edge between two root-scope blocks', () => {
+    const blocks = {
+      a: makeBlock('a', 'starter'),
+      b: makeBlock('b', 'function'),
+    }
+    const result = validateEdges([makeEdge('e1', 'a', 'b')], blocks)
+    expect(result.valid).toHaveLength(1)
+    expect(result.dropped).toHaveLength(0)
+  })
+
+  it('drops an edge referencing a missing block', () => {
+    const blocks = { a: makeBlock('a', 'starter') }
+    const result = validateEdges([makeEdge('e1', 'a', 'missing')], blocks)
+    expect(result.valid).toHaveLength(0)
+    expect(result.dropped[0].reason).toBe('edge references a missing block')
+  })
+
+  it('drops an edge touching an annotation-only (note) block', () => {
+    const blocks = {
+      a: makeBlock('a', 'note'),
+      b: makeBlock('b', 'function'),
+    }
+    const result = validateEdges([makeEdge('e1', 'a', 'b')], blocks)
+    expect(result.valid).toHaveLength(0)
+    expect(result.dropped[0].reason).toBe('edge references an annotation-only block')
+  })
+
+  it('drops an edge targeting a trigger block', () => {
+    const blocks = {
+      a: makeBlock('a', 'function'),
+      b: makeBlock('b', 'function', { triggerMode: true }),
+    }
+    const result = validateEdges([makeEdge('e1', 'a', 'b')], blocks)
+    expect(result.valid).toHaveLength(0)
+    expect(result.dropped[0].reason).toBe('trigger blocks cannot be edge targets')
+  })
+
+  it('drops an edge crossing loop scope boundaries', () => {
+    const blocks = {
+      loop: makeBlock('loop', 'loop'),
+      inner: makeBlock('inner', 'function', { data: { parentId: 'loop', extent: 'parent' } }),
+      outer: makeBlock('outer', 'function'),
+    }
+    const result = validateEdges([makeEdge('e1', 'inner', 'outer')], blocks)
+    expect(result.valid).toHaveLength(0)
+    expect(result.dropped[0].reason).toContain('different scopes')
+  })
+
+  it('accepts an edge from a loop container into its own child', () => {
+    const blocks = {
+      loop: makeBlock('loop', 'loop'),
+      inner: makeBlock('inner', 'function', { data: { parentId: 'loop', extent: 'parent' } }),
+    }
+    const result = validateEdges([makeEdge('e1', 'loop', 'inner')], blocks)
+    expect(result.valid).toHaveLength(1)
+  })
+
+  it('accepts an edge from a loop child back out to its own container', () => {
+    const blocks = {
+      loop: makeBlock('loop', 'loop'),
+      inner: makeBlock('inner', 'function', { data: { parentId: 'loop', extent: 'parent' } }),
+    }
+    const result = validateEdges([makeEdge('e1', 'inner', 'loop')], blocks)
+    expect(result.valid).toHaveLength(1)
+  })
+
+  it('accepts edges between two siblings inside the same loop', () => {
+    const blocks = {
+      loop: makeBlock('loop', 'loop'),
+      a: makeBlock('a', 'function', { data: { parentId: 'loop', extent: 'parent' } }),
+      b: makeBlock('b', 'function', { data: { parentId: 'loop', extent: 'parent' } }),
+    }
+    const result = validateEdges([makeEdge('e1', 'a', 'b')], blocks)
+    expect(result.valid).toHaveLength(1)
+  })
+})

apps/sim/stores/workflows/workflow/edge-validation.ts

@@ -1,6 +1,9 @@
+import {
+  getWorkflowEdgeScopeDropReason,
+  isWorkflowAnnotationOnlyBlockType,
+} from '@sim/workflow-types/workflow'
 import type { Edge } from 'reactflow'
 import { TriggerUtils } from '@/lib/workflows/triggers/triggers'
-import { isAnnotationOnlyBlock } from '@/executor/constants'
 import type { BlockState } from '@/stores/workflows/workflow/types'
 
 interface DroppedEdge {
@@ -13,40 +16,6 @@ export interface EdgeValidationResult {
   dropped: DroppedEdge[]
 }
 
-function isContainerBlock(block: BlockState | undefined): boolean {
-  return block?.type === 'loop' || block?.type === 'parallel'
-}
-
-function getParentId(block: BlockState | undefined): string | null {
-  return block?.data?.parentId ?? null
-}
-
-function getScopeDropReason(edge: Edge, blocks: Record<string, BlockState>): string | null {
-  const sourceBlock = blocks[edge.source]
-  const targetBlock = blocks[edge.target]
-
-  if (!sourceBlock || !targetBlock) {
-    return 'edge references a missing block'
-  }
-
-  const sourceParent = getParentId(sourceBlock)
-  const targetParent = getParentId(targetBlock)
-
-  if (sourceParent === targetParent) {
-    return null
-  }
-
-  if (targetParent === edge.source && isContainerBlock(sourceBlock)) {
-    return null
-  }
-
-  if (sourceParent === edge.target && isContainerBlock(targetBlock)) {
-    return null
-  }
-
-  return `blocks are in different scopes (${sourceParent ?? 'root'} -> ${targetParent ?? 'root'})`
-}
-
 export function validateEdges(
   edges: Edge[],
   blocks: Record<string, BlockState>
@@ -63,7 +32,10 @@ export function validateEdges(
       continue
     }
 
-    if (isAnnotationOnlyBlock(sourceBlock.type) || isAnnotationOnlyBlock(targetBlock.type)) {
+    if (
+      isWorkflowAnnotationOnlyBlockType(sourceBlock.type) ||
+      isWorkflowAnnotationOnlyBlockType(targetBlock.type)
+    ) {
       dropped.push({ edge, reason: 'edge references an annotation-only block' })
       continue
     }
@@ -73,7 +45,7 @@ export function validateEdges(
       continue
     }
 
-    const scopeDropReason = getScopeDropReason(edge, blocks)
+    const scopeDropReason = getWorkflowEdgeScopeDropReason(edge, blocks)
     if (scopeDropReason) {
       dropped.push({ edge, reason: scopeDropReason })
       continue