From 5516fa39c32c38f8c1045c0f9cc3ef7cd0d4c854 Mon Sep 17 00:00:00 2001 From: Waleed Date: Wed, 17 Dec 2025 19:13:39 -0800 Subject: [PATCH] fix(graph): prevent cyclic dependencies in graph following ReactFlow examples (#2439) * fix(graph): prevent cyclic dependencies in graph following ReactFlow examples * ack PR comment --- .../workflow-block/workflow-block.tsx | 32 ++++++++++-- .../[workspaceId]/w/[workflowId]/workflow.tsx | 5 -- apps/sim/stores/workflows/workflow/store.ts | 15 +++++- apps/sim/stores/workflows/workflow/utils.ts | 49 +++++++++++++++++++ 4 files changed, 90 insertions(+), 11 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 786d577c9..a24d3ff9e 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 906e4a245..cb6ac6448 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx @@ -1642,11 +1642,6 @@ const WorkflowContent = React.memo(() => { 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) diff --git a/apps/sim/stores/workflows/workflow/store.ts b/apps/sim/stores/workflows/workflow/store.ts index 5adcd6dd7..681fcd33c 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 a4b4a3019..b7a962392 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 *