From 61d96ad69f0cba3dd43dd3ac0302dc48617c292f Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 17 Dec 2025 18:54:03 -0800 Subject: [PATCH 1/2] fix(graph): prevent cyclic dependencies in graph following ReactFlow examples --- .../[workspaceId]/w/[workflowId]/workflow.tsx | 27 ++++++++++ apps/sim/stores/workflows/workflow/store.ts | 15 +++++- apps/sim/stores/workflows/workflow/utils.ts | 49 +++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx index 906e4a245d..461b64fa8a 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx @@ -56,6 +56,7 @@ import { useWorkflowDiffStore } from '@/stores/workflow-diff/store' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { getUniqueBlockName } from '@/stores/workflows/utils' import { useWorkflowStore } from '@/stores/workflows/workflow/store' +import { wouldCreateCycle } from '@/stores/workflows/workflow/utils' /** Lazy-loaded components for non-critical UI that can load after initial render */ const LazyChat = lazy(() => @@ -1638,6 +1639,31 @@ const WorkflowContent = React.memo(() => { setIsErrorConnectionDrag(false) }, []) + /** + * Validates if a connection is allowed before it's created. + * Prevents cycles and other invalid connections. + */ + const isValidConnection = useCallback( + (connection: { source: string | null; target: string | null }) => { + if (!connection.source || !connection.target) { + return false + } + + if (connection.source === connection.target) { + return false + } + + const currentEdges = useWorkflowStore.getState().edges + + if (wouldCreateCycle(currentEdges, connection.source, connection.target)) { + return false + } + + return true + }, + [] + ) + /** Handles new edge connections with container boundary validation. */ const onConnect = useCallback( (connection: any) => { @@ -2249,6 +2275,7 @@ const WorkflowContent = React.memo(() => { onConnect={effectivePermissions.canEdit ? onConnect : undefined} onConnectStart={effectivePermissions.canEdit ? onConnectStart : undefined} onConnectEnd={effectivePermissions.canEdit ? onConnectEnd : undefined} + isValidConnection={effectivePermissions.canEdit ? isValidConnection : undefined} nodeTypes={nodeTypes} edgeTypes={edgeTypes} onDrop={effectivePermissions.canEdit ? onDrop : undefined} diff --git a/apps/sim/stores/workflows/workflow/store.ts b/apps/sim/stores/workflows/workflow/store.ts index 5adcd6dd73..681fcd33cf 100644 --- a/apps/sim/stores/workflows/workflow/store.ts +++ b/apps/sim/stores/workflows/workflow/store.ts @@ -20,7 +20,11 @@ import type { WorkflowState, WorkflowStore, } from '@/stores/workflows/workflow/types' -import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' +import { + generateLoopBlocks, + generateParallelBlocks, + wouldCreateCycle, +} from '@/stores/workflows/workflow/utils' const logger = createLogger('WorkflowStore') @@ -428,6 +432,15 @@ export const useWorkflowStore = create()( return } + // Prevent self-connections and cycles + if (wouldCreateCycle(get().edges, edge.source, edge.target)) { + logger.warn('Prevented edge that would create a cycle', { + source: edge.source, + target: edge.target, + }) + return + } + // Check for duplicate connections const isDuplicate = get().edges.some( (existingEdge) => diff --git a/apps/sim/stores/workflows/workflow/utils.ts b/apps/sim/stores/workflows/workflow/utils.ts index a4b4a30193..b7a962392a 100644 --- a/apps/sim/stores/workflows/workflow/utils.ts +++ b/apps/sim/stores/workflows/workflow/utils.ts @@ -1,7 +1,56 @@ +import type { Edge } from 'reactflow' import type { BlockState, Loop, Parallel } from '@/stores/workflows/workflow/types' const DEFAULT_LOOP_ITERATIONS = 5 +/** + * Check if adding an edge would create a cycle in the graph. + * Uses depth-first search to detect if the source node is reachable from the target node. + * + * @param edges - Current edges in the graph + * @param sourceId - Source node ID of the proposed edge + * @param targetId - Target node ID of the proposed edge + * @returns true if adding this edge would create a cycle + */ +export function wouldCreateCycle(edges: Edge[], sourceId: string, targetId: string): boolean { + if (sourceId === targetId) { + return true + } + + const adjacencyList = new Map() + for (const edge of edges) { + if (!adjacencyList.has(edge.source)) { + adjacencyList.set(edge.source, []) + } + adjacencyList.get(edge.source)!.push(edge.target) + } + + const visited = new Set() + + function canReachSource(currentNode: string): boolean { + if (currentNode === sourceId) { + return true + } + + if (visited.has(currentNode)) { + return false + } + + visited.add(currentNode) + + const neighbors = adjacencyList.get(currentNode) || [] + for (const neighbor of neighbors) { + if (canReachSource(neighbor)) { + return true + } + } + + return false + } + + return canReachSource(targetId) +} + /** * Convert UI loop block to executor Loop format * From 846018122ef395cdbc558c31facbfb74af5f1825 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 17 Dec 2025 19:01:28 -0800 Subject: [PATCH 2/2] ack PR comment --- .../workflow-block/workflow-block.tsx | 32 ++++++++++++++++--- .../[workspaceId]/w/[workflowId]/workflow.tsx | 32 ------------------- 2 files changed, 27 insertions(+), 37 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx index 786d577c91..a24d3ff9e4 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx @@ -40,6 +40,8 @@ import { useSelectorDisplayName } from '@/hooks/use-selector-display-name' import { useVariablesStore } from '@/stores/panel/variables/store' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { useSubBlockStore } from '@/stores/workflows/subblock/store' +import { useWorkflowStore } from '@/stores/workflows/workflow/store' +import { wouldCreateCycle } from '@/stores/workflows/workflow/utils' const logger = createLogger('WorkflowBlock') @@ -844,7 +846,11 @@ export const WorkflowBlock = memo(function WorkflowBlock({ data-handleid='target' isConnectableStart={false} isConnectableEnd={true} - isValidConnection={(connection) => connection.source !== id} + isValidConnection={(connection) => { + if (connection.source === id) return false + const edges = useWorkflowStore.getState().edges + return !wouldCreateCycle(edges, connection.source!, connection.target!) + }} /> )} @@ -1045,7 +1051,11 @@ export const WorkflowBlock = memo(function WorkflowBlock({ data-handleid={`condition-${cond.id}`} isConnectableStart={true} isConnectableEnd={false} - isValidConnection={(connection) => connection.target !== id} + isValidConnection={(connection) => { + if (connection.target === id) return false + const edges = useWorkflowStore.getState().edges + return !wouldCreateCycle(edges, connection.source!, connection.target!) + }} /> ) })} @@ -1064,7 +1074,11 @@ export const WorkflowBlock = memo(function WorkflowBlock({ data-handleid='error' isConnectableStart={true} isConnectableEnd={false} - isValidConnection={(connection) => connection.target !== id} + isValidConnection={(connection) => { + if (connection.target === id) return false + const edges = useWorkflowStore.getState().edges + return !wouldCreateCycle(edges, connection.source!, connection.target!) + }} /> )} @@ -1081,7 +1095,11 @@ export const WorkflowBlock = memo(function WorkflowBlock({ data-handleid='source' isConnectableStart={true} isConnectableEnd={false} - isValidConnection={(connection) => connection.target !== id} + isValidConnection={(connection) => { + if (connection.target === id) return false + const edges = useWorkflowStore.getState().edges + return !wouldCreateCycle(edges, connection.source!, connection.target!) + }} /> {shouldShowDefaultHandles && ( @@ -1100,7 +1118,11 @@ export const WorkflowBlock = memo(function WorkflowBlock({ data-handleid='error' isConnectableStart={true} isConnectableEnd={false} - isValidConnection={(connection) => connection.target !== id} + isValidConnection={(connection) => { + if (connection.target === id) return false + const edges = useWorkflowStore.getState().edges + return !wouldCreateCycle(edges, connection.source!, connection.target!) + }} /> )} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx index 461b64fa8a..cb6ac6448d 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx @@ -56,7 +56,6 @@ import { useWorkflowDiffStore } from '@/stores/workflow-diff/store' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { getUniqueBlockName } from '@/stores/workflows/utils' import { useWorkflowStore } from '@/stores/workflows/workflow/store' -import { wouldCreateCycle } from '@/stores/workflows/workflow/utils' /** Lazy-loaded components for non-critical UI that can load after initial render */ const LazyChat = lazy(() => @@ -1639,40 +1638,10 @@ const WorkflowContent = React.memo(() => { setIsErrorConnectionDrag(false) }, []) - /** - * Validates if a connection is allowed before it's created. - * Prevents cycles and other invalid connections. - */ - const isValidConnection = useCallback( - (connection: { source: string | null; target: string | null }) => { - if (!connection.source || !connection.target) { - return false - } - - if (connection.source === connection.target) { - return false - } - - const currentEdges = useWorkflowStore.getState().edges - - if (wouldCreateCycle(currentEdges, connection.source, connection.target)) { - return false - } - - return true - }, - [] - ) - /** Handles new edge connections with container boundary validation. */ const onConnect = useCallback( (connection: any) => { if (connection.source && connection.target) { - // Prevent self-connections - if (connection.source === connection.target) { - return - } - // Check if connecting nodes across container boundaries const sourceNode = getNodes().find((n) => n.id === connection.source) const targetNode = getNodes().find((n) => n.id === connection.target) @@ -2275,7 +2244,6 @@ const WorkflowContent = React.memo(() => { onConnect={effectivePermissions.canEdit ? onConnect : undefined} onConnectStart={effectivePermissions.canEdit ? onConnectStart : undefined} onConnectEnd={effectivePermissions.canEdit ? onConnectEnd : undefined} - isValidConnection={effectivePermissions.canEdit ? isValidConnection : undefined} nodeTypes={nodeTypes} edgeTypes={edgeTypes} onDrop={effectivePermissions.canEdit ? onDrop : undefined}