Fix

apps/sim/lib/copilot/tools/server/workflow/workflow-operations/builders.test.ts

@@ -2,7 +2,11 @@
  * @vitest-environment node
  */
 import { describe, expect, it, vi } from 'vitest'
-import { createBlockFromParams } from './builders'
+import {
+  createBlockFromParams,
+  pruneInvalidSubflowBoundaryEdgesForBlock,
+  validateSubflowBoundaryEdge,
+} from './builders'
 
 const agentBlockConfig = {
   type: 'agent',
@@ -42,3 +46,86 @@ describe('createBlockFromParams', () => {
     expect(block.outputs.answer.type).toBe('string')
   })
 })
+
+describe('validateSubflowBoundaryEdge', () => {
+  it('rejects child-to-root crossing edges', () => {
+    const state = {
+      blocks: {
+        child: { id: 'child', type: 'function', data: { parentId: 'loop1' } },
+        root: { id: 'root', type: 'function', data: {} },
+      },
+    }
+
+    const result = validateSubflowBoundaryEdge(state, 'child', 'root', 'source')
+    expect(result.valid).toBe(false)
+  })
+
+  it('accepts same-parent child edges', () => {
+    const state = {
+      blocks: {
+        childA: { id: 'childA', type: 'function', data: { parentId: 'loop1' } },
+        childB: { id: 'childB', type: 'function', data: { parentId: 'loop1' } },
+      },
+    }
+
+    const result = validateSubflowBoundaryEdge(state, 'childA', 'childB', 'source')
+    expect(result.valid).toBe(true)
+  })
+
+  it('enforces loop start and end handle boundaries', () => {
+    const state = {
+      blocks: {
+        loop1: { id: 'loop1', type: 'loop', data: {} },
+        child: { id: 'child', type: 'function', data: { parentId: 'loop1' } },
+        outside: { id: 'outside', type: 'function', data: {} },
+      },
+    }
+
+    expect(validateSubflowBoundaryEdge(state, 'loop1', 'outside', 'loop-start-source').valid).toBe(
+      false
+    )
+    expect(validateSubflowBoundaryEdge(state, 'loop1', 'child', 'loop-start-source').valid).toBe(
+      true
+    )
+    expect(validateSubflowBoundaryEdge(state, 'loop1', 'child', 'loop-end-source').valid).toBe(
+      false
+    )
+    expect(validateSubflowBoundaryEdge(state, 'loop1', 'outside', 'loop-end-source').valid).toBe(
+      true
+    )
+  })
+})
+
+describe('pruneInvalidSubflowBoundaryEdgesForBlock', () => {
+  it('removes stale edges that become invalid after extraction', () => {
+    const state: any = {
+      blocks: {
+        loop1: { id: 'loop1', type: 'loop', data: {} },
+        child: { id: 'child', type: 'function', data: {} },
+      },
+      edges: [
+        {
+          id: 'edge-loop-start-to-child',
+          source: 'loop1',
+          sourceHandle: 'loop-start-source',
+          target: 'child',
+          targetHandle: 'target',
+        },
+      ],
+    }
+    const skipped: any[] = []
+    const logger = { info: vi.fn(), warn: vi.fn() } as any
+
+    pruneInvalidSubflowBoundaryEdgesForBlock(
+      state,
+      'child',
+      'extract_from_subflow',
+      logger,
+      skipped
+    )
+
+    expect(state.edges).toHaveLength(0)
+    expect(skipped).toHaveLength(1)
+    expect(skipped[0].type).toBe('invalid_subflow_boundary_edge')
+  })
+})

apps/sim/lib/copilot/tools/server/workflow/workflow-operations/builders.ts

@@ -411,6 +411,35 @@ export function createValidatedEdge(
   // Use normalized handle if available (e.g., 'if' -> 'condition-{uuid}')
   const finalSourceHandle = sourceValidation.normalizedHandle || sourceHandle
 
+  const boundaryValidation = validateSubflowBoundaryEdge(
+    modifiedState,
+    sourceBlockId,
+    targetBlockId,
+    finalSourceHandle
+  )
+  if (!boundaryValidation.valid) {
+    logger.warn('Invalid subflow boundary edge. Edge skipped.', {
+      sourceBlockId,
+      targetBlockId,
+      sourceHandle: finalSourceHandle,
+      reason: boundaryValidation.reason,
+    })
+    skippedItems?.push({
+      type: 'invalid_subflow_boundary_edge',
+      operationType,
+      blockId: sourceBlockId,
+      reason:
+        boundaryValidation.reason ||
+        `Edge from "${sourceBlockId}" to "${targetBlockId}" crosses an invalid subflow boundary`,
+      details: {
+        sourceHandle: finalSourceHandle,
+        targetHandle,
+        targetId: targetBlockId,
+      },
+    })
+    return false
+  }
+
   modifiedState.edges.push({
     id: crypto.randomUUID(),
     source: sourceBlockId,
@@ -422,6 +451,191 @@ export function createValidatedEdge(
   return true
 }
 
+function getParentId(block: any): string | null {
+  const parentId = block?.data?.parentId
+  if (typeof parentId !== 'string' || !parentId.trim()) {
+    return null
+  }
+  return parentId
+}
+
+function isLoopStartHandle(sourceHandle: string): boolean {
+  return sourceHandle === 'loop-start-source'
+}
+
+function isParallelStartHandle(sourceHandle: string): boolean {
+  return sourceHandle === 'parallel-start-source'
+}
+
+function isLoopEndHandle(sourceHandle: string): boolean {
+  return sourceHandle === 'loop-end-source'
+}
+
+function isParallelEndHandle(sourceHandle: string): boolean {
+  return sourceHandle === 'parallel-end-source'
+}
+
+/**
+ * Validates whether an edge violates subflow boundary rules.
+ * Rules:
+ * 1. Blocks inside a subflow may only connect to blocks inside the same subflow.
+ * 2. loop/parallel start handles must target a direct child in the same container.
+ * 3. loop/parallel end handles must target outside the same container.
+ */
+export function validateSubflowBoundaryEdge(
+  modifiedState: any,
+  sourceBlockId: string,
+  targetBlockId: string,
+  sourceHandle: string
+): { valid: boolean; reason?: string } {
+  const sourceBlock = modifiedState?.blocks?.[sourceBlockId]
+  const targetBlock = modifiedState?.blocks?.[targetBlockId]
+  if (!sourceBlock || !targetBlock) {
+    // Source/target existence is validated earlier.
+    return { valid: true }
+  }
+
+  const sourceParentId = getParentId(sourceBlock)
+  const targetParentId = getParentId(targetBlock)
+
+  if (isLoopStartHandle(sourceHandle)) {
+    if (sourceBlock.type !== 'loop') {
+      return {
+        valid: false,
+        reason: `Handle "${sourceHandle}" is only valid for loop blocks`,
+      }
+    }
+    if (targetParentId !== sourceBlockId) {
+      return {
+        valid: false,
+        reason: `Loop start edges must target a block inside loop "${sourceBlockId}"`,
+      }
+    }
+    return { valid: true }
+  }
+
+  if (isParallelStartHandle(sourceHandle)) {
+    if (sourceBlock.type !== 'parallel') {
+      return {
+        valid: false,
+        reason: `Handle "${sourceHandle}" is only valid for parallel blocks`,
+      }
+    }
+    if (targetParentId !== sourceBlockId) {
+      return {
+        valid: false,
+        reason: `Parallel start edges must target a block inside parallel "${sourceBlockId}"`,
+      }
+    }
+    return { valid: true }
+  }
+
+  if (isLoopEndHandle(sourceHandle)) {
+    if (sourceBlock.type !== 'loop') {
+      return {
+        valid: false,
+        reason: `Handle "${sourceHandle}" is only valid for loop blocks`,
+      }
+    }
+    if (targetParentId === sourceBlockId) {
+      return {
+        valid: false,
+        reason: `Loop end edges cannot target a block inside loop "${sourceBlockId}"`,
+      }
+    }
+    return { valid: true }
+  }
+
+  if (isParallelEndHandle(sourceHandle)) {
+    if (sourceBlock.type !== 'parallel') {
+      return {
+        valid: false,
+        reason: `Handle "${sourceHandle}" is only valid for parallel blocks`,
+      }
+    }
+    if (targetParentId === sourceBlockId) {
+      return {
+        valid: false,
+        reason: `Parallel end edges cannot target a block inside parallel "${sourceBlockId}"`,
+      }
+    }
+    return { valid: true }
+  }
+
+  if (sourceParentId !== targetParentId) {
+    return {
+      valid: false,
+      reason: `Edge crosses subflow boundary: source parent "${sourceParentId ?? 'root'}", target parent "${targetParentId ?? 'root'}"`,
+    }
+  }
+
+  return { valid: true }
+}
+
+/**
+ * Remove existing edges touching the given block that violate subflow boundary rules.
+ * This is used after structural moves (insert/extract) to prevent stale cross-boundary edges.
+ */
+export function pruneInvalidSubflowBoundaryEdgesForBlock(
+  modifiedState: any,
+  blockId: string,
+  operationType: string,
+  logger: ReturnType<typeof createLogger>,
+  skippedItems?: SkippedItem[]
+): void {
+  if (!modifiedState?.edges || !Array.isArray(modifiedState.edges)) {
+    return
+  }
+
+  const prunedEdges: any[] = []
+
+  modifiedState.edges = modifiedState.edges.filter((edge: any) => {
+    const touchesBlock = edge?.source === blockId || edge?.target === blockId
+    if (!touchesBlock) {
+      return true
+    }
+
+    const sourceHandle = typeof edge?.sourceHandle === 'string' ? edge.sourceHandle : 'source'
+    const boundaryValidation = validateSubflowBoundaryEdge(
+      modifiedState,
+      edge.source,
+      edge.target,
+      sourceHandle
+    )
+
+    if (boundaryValidation.valid) {
+      return true
+    }
+
+    prunedEdges.push(edge)
+    skippedItems?.push({
+      type: 'invalid_subflow_boundary_edge',
+      operationType,
+      blockId,
+      reason:
+        boundaryValidation.reason ||
+        `Removed edge "${edge.id || 'unknown'}" due to invalid subflow boundary`,
+      details: {
+        edgeId: edge.id,
+        source: edge.source,
+        sourceHandle: edge.sourceHandle,
+        target: edge.target,
+        targetHandle: edge.targetHandle,
+      },
+    })
+    return false
+  })
+
+  if (prunedEdges.length > 0) {
+    logger.info('Pruned invalid subflow boundary edges after structural move', {
+      blockId,
+      operationType,
+      prunedCount: prunedEdges.length,
+      edgeIds: prunedEdges.map((edge) => edge.id).filter(Boolean),
+    })
+  }
+}
+
 /**
  * Adds connections as edges for a block.
  * Supports multiple target formats:

apps/sim/lib/copilot/tools/server/workflow/workflow-operations/operations.ts

@@ -13,6 +13,7 @@ import {
   normalizeArrayWithIds,
   normalizeResponseFormat,
   normalizeTools,
+  pruneInvalidSubflowBoundaryEdgesForBlock,
   shouldNormalizeArrayIds,
   updateCanonicalModesForInputs,
 } from './builders'
@@ -26,6 +27,55 @@ import {
 
 const logger = createLogger('EditWorkflowServerTool')
 
+function applyContainerConfigFromInputs(
+  block: { type?: string; data?: Record<string, any> },
+  inputs?: Record<string, any>
+): void {
+  if (!inputs || !block?.type) return
+
+  if (block.type === 'loop') {
+    block.data = block.data || {}
+    const validLoopTypes = ['for', 'forEach', 'while', 'doWhile']
+    if (inputs.loopType !== undefined && validLoopTypes.includes(inputs.loopType)) {
+      block.data.loopType = inputs.loopType
+    }
+
+    const effectiveLoopType = inputs.loopType ?? block.data.loopType ?? 'for'
+    if (inputs.iterations !== undefined && effectiveLoopType === 'for') {
+      block.data.count = inputs.iterations
+    }
+    if (inputs.collection !== undefined && effectiveLoopType === 'forEach') {
+      block.data.collection = inputs.collection
+    }
+    if (
+      inputs.condition !== undefined &&
+      (effectiveLoopType === 'while' || effectiveLoopType === 'doWhile')
+    ) {
+      if (effectiveLoopType === 'doWhile') {
+        block.data.doWhileCondition = inputs.condition
+      } else {
+        block.data.whileCondition = inputs.condition
+      }
+    }
+    return
+  }
+
+  if (block.type === 'parallel') {
+    block.data = block.data || {}
+    const validParallelTypes = ['count', 'collection']
+    if (inputs.parallelType !== undefined && validParallelTypes.includes(inputs.parallelType)) {
+      block.data.parallelType = inputs.parallelType
+    }
+    const effectiveParallelType = inputs.parallelType ?? block.data.parallelType ?? 'count'
+    if (inputs.count !== undefined && effectiveParallelType === 'count') {
+      block.data.count = inputs.count
+    }
+    if (inputs.collection !== undefined && effectiveParallelType === 'collection') {
+      block.data.collection = inputs.collection
+    }
+  }
+}
+
 export function handleDeleteOperation(op: EditWorkflowOperation, ctx: OperationContext): void {
   const { modifiedState, skippedItems } = ctx
   const { block_id } = op
@@ -190,55 +240,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon
       applyTriggerConfigToBlockSubblocks(block, block.subBlocks.triggerConfig.value)
     }
 
-    // Update loop/parallel configuration in block.data (strict validation)
-    if (block.type === 'loop') {
-      block.data = block.data || {}
-      // loopType is always valid
-      if (params.inputs.loopType !== undefined) {
-        const validLoopTypes = ['for', 'forEach', 'while', 'doWhile']
-        if (validLoopTypes.includes(params.inputs.loopType)) {
-          block.data.loopType = params.inputs.loopType
-        }
-      }
-      const effectiveLoopType = params.inputs.loopType ?? block.data.loopType ?? 'for'
-      // iterations only valid for 'for' loopType
-      if (params.inputs.iterations !== undefined && effectiveLoopType === 'for') {
-        block.data.count = params.inputs.iterations
-      }
-      // collection only valid for 'forEach' loopType
-      if (params.inputs.collection !== undefined && effectiveLoopType === 'forEach') {
-        block.data.collection = params.inputs.collection
-      }
-      // condition only valid for 'while' or 'doWhile' loopType
-      if (
-        params.inputs.condition !== undefined &&
-        (effectiveLoopType === 'while' || effectiveLoopType === 'doWhile')
-      ) {
-        if (effectiveLoopType === 'doWhile') {
-          block.data.doWhileCondition = params.inputs.condition
-        } else {
-          block.data.whileCondition = params.inputs.condition
-        }
-      }
-    } else if (block.type === 'parallel') {
-      block.data = block.data || {}
-      // parallelType is always valid
-      if (params.inputs.parallelType !== undefined) {
-        const validParallelTypes = ['count', 'collection']
-        if (validParallelTypes.includes(params.inputs.parallelType)) {
-          block.data.parallelType = params.inputs.parallelType
-        }
-      }
-      const effectiveParallelType = params.inputs.parallelType ?? block.data.parallelType ?? 'count'
-      // count only valid for 'count' parallelType
-      if (params.inputs.count !== undefined && effectiveParallelType === 'count') {
-        block.data.count = params.inputs.count
-      }
-      // collection only valid for 'collection' parallelType
-      if (params.inputs.collection !== undefined && effectiveParallelType === 'collection') {
-        block.data.collection = params.inputs.collection
-      }
-    }
+    applyContainerConfigFromInputs(block, params.inputs)
 
     const editBlockConfig = getBlock(block.type)
     if (editBlockConfig) {
@@ -394,56 +396,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon
       }
     })
 
-    // Update loop/parallel configuration based on type (strict validation)
-    if (block.type === 'loop') {
-      block.data = block.data || {}
-      // loopType is always valid
-      if (params.inputs?.loopType) {
-        const validLoopTypes = ['for', 'forEach', 'while', 'doWhile']
-        if (validLoopTypes.includes(params.inputs.loopType)) {
-          block.data.loopType = params.inputs.loopType
-        }
-      }
-      const effectiveLoopType = params.inputs?.loopType ?? block.data.loopType ?? 'for'
-      // iterations only valid for 'for' loopType
-      if (params.inputs?.iterations && effectiveLoopType === 'for') {
-        block.data.count = params.inputs.iterations
-      }
-      // collection only valid for 'forEach' loopType
-      if (params.inputs?.collection && effectiveLoopType === 'forEach') {
-        block.data.collection = params.inputs.collection
-      }
-      // condition only valid for 'while' or 'doWhile' loopType
-      if (
-        params.inputs?.condition &&
-        (effectiveLoopType === 'while' || effectiveLoopType === 'doWhile')
-      ) {
-        if (effectiveLoopType === 'doWhile') {
-          block.data.doWhileCondition = params.inputs.condition
-        } else {
-          block.data.whileCondition = params.inputs.condition
-        }
-      }
-    } else if (block.type === 'parallel') {
-      block.data = block.data || {}
-      // parallelType is always valid
-      if (params.inputs?.parallelType) {
-        const validParallelTypes = ['count', 'collection']
-        if (validParallelTypes.includes(params.inputs.parallelType)) {
-          block.data.parallelType = params.inputs.parallelType
-        }
-      }
-      const effectiveParallelType =
-        params.inputs?.parallelType ?? block.data.parallelType ?? 'count'
-      // count only valid for 'count' parallelType
-      if (params.inputs?.count && effectiveParallelType === 'count') {
-        block.data.count = params.inputs.count
-      }
-      // collection only valid for 'collection' parallelType
-      if (params.inputs?.collection && effectiveParallelType === 'collection') {
-        block.data.collection = params.inputs.collection
-      }
-    }
+    applyContainerConfigFromInputs(block, params.inputs)
   }
 
   // Handle connections update (convert to edges)
@@ -620,42 +573,9 @@ export function handleAddOperation(op: EditWorkflowOperation, ctx: OperationCont
     skippedItems
   )
 
-  // Set loop/parallel data on parent block BEFORE adding to blocks (strict validation)
-  if (params.nestedNodes) {
-    if (params.type === 'loop') {
-      const validLoopTypes = ['for', 'forEach', 'while', 'doWhile']
-      const loopType =
-        params.inputs?.loopType && validLoopTypes.includes(params.inputs.loopType)
-          ? params.inputs.loopType
-          : 'for'
-      newBlock.data = {
-        ...newBlock.data,
-        loopType,
-        // Only include type-appropriate fields
-        ...(loopType === 'forEach' &&
-          params.inputs?.collection && { collection: params.inputs.collection }),
-        ...(loopType === 'for' && params.inputs?.iterations && { count: params.inputs.iterations }),
-        ...(loopType === 'while' &&
-          params.inputs?.condition && { whileCondition: params.inputs.condition }),
-        ...(loopType === 'doWhile' &&
-          params.inputs?.condition && { doWhileCondition: params.inputs.condition }),
-      }
-    } else if (params.type === 'parallel') {
-      const validParallelTypes = ['count', 'collection']
-      const parallelType =
-        params.inputs?.parallelType && validParallelTypes.includes(params.inputs.parallelType)
-          ? params.inputs.parallelType
-          : 'count'
-      newBlock.data = {
-        ...newBlock.data,
-        parallelType,
-        // Only include type-appropriate fields
-        ...(parallelType === 'collection' &&
-          params.inputs?.collection && { collection: params.inputs.collection }),
-        ...(parallelType === 'count' && params.inputs?.count && { count: params.inputs.count }),
-      }
-    }
-  }
+  // Set loop/parallel data on parent block BEFORE adding to blocks.
+  // This must happen for both empty containers and containers with nested nodes.
+  applyContainerConfigFromInputs(newBlock, params.inputs)
 
   // Add parent block FIRST before adding children
   // This ensures children can reference valid parentId
@@ -834,6 +754,15 @@ export function handleInsertIntoSubflowOperation(
       extent: 'parent' as const,
     }
 
+    // A moved block cannot keep stale edges that now cross a subflow boundary.
+    pruneInvalidSubflowBoundaryEdgesForBlock(
+      modifiedState,
+      block_id,
+      'insert_into_subflow',
+      logger,
+      skippedItems
+    )
+
     // Update inputs if provided (with validation)
     if (params.inputs) {
       // Validate inputs against block configuration
@@ -1012,6 +941,12 @@ export function handleExtractFromSubflowOperation(
     block.data.extent = undefined
   }
 
-  // Note: We keep the block and its edges, just remove parent relationship
-  // The block becomes a root-level block
+  // Remove edges that became invalid after crossing a subflow boundary.
+  pruneInvalidSubflowBoundaryEdgesForBlock(
+    modifiedState,
+    block_id,
+    'extract_from_subflow',
+    logger,
+    skippedItems
+  )
 }

apps/sim/lib/copilot/tools/server/workflow/workflow-operations/types.ts

@@ -44,6 +44,7 @@ export type SkippedItemType =
   | 'invalid_edge_source'
   | 'invalid_source_handle'
   | 'invalid_target_handle'
+  | 'invalid_subflow_boundary_edge'
   | 'invalid_subblock_field'
   | 'missing_required_params'
   | 'invalid_subflow_parent'