Skip to content

Commit

Permalink
fix: prevent cyclic connections between nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
srijanpatel committed Jan 27, 2025
1 parent b7583d2 commit 55373e8
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 2 deletions.
5 changes: 4 additions & 1 deletion frontend/src/components/nodes/DynamicNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import NodeOutputDisplay from './NodeOutputDisplay'
import NodeOutputModal from './NodeOutputModal'
import isEqual from 'lodash/isEqual'
import NodeErrorDisplay from './NodeErrorDisplay'
import { isTargetAncestorOfSource } from '@/utils/cyclicEdgeUtils'

const baseNodeStyle = {
width: 'auto',
Expand Down Expand Up @@ -244,7 +245,9 @@ const DynamicNode: React.FC<DynamicNodeProps> = ({
// Check if nodes have the same parent or both have no parent
const fromNodeParentId = connection.fromNode?.parentId
const toNodeParentId = connection.toNode?.parentId
const canConnect = fromNodeParentId === toNodeParentId
const canConnect =
fromNodeParentId === toNodeParentId &&
!isTargetAncestorOfSource(connection.fromNode.id, connection.toNode.id, nodes, edges)

if (
canConnect &&
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/components/nodes/logic/RouterNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '../../../types/api_types/routerSchemas'
import NodeOutputDisplay from '../NodeOutputDisplay'
import { isEqual } from 'lodash'
import { isTargetAncestorOfSource } from '@/utils/cyclicEdgeUtils'

interface RouterNodeData {
title?: string
Expand Down Expand Up @@ -94,6 +95,16 @@ export const RouterNode: React.FC<RouterNodeProps> = ({ id, data, readOnly = fal

// If a new connection is in progress to this node, show that source node as well
if (connection.inProgress && connection.toNode?.id === id && connection.fromNode) {
const fromNodeParentId = connection.fromNode?.parentId
const toNodeParentId = connection.toNode?.parentId
const canConnect =
fromNodeParentId === toNodeParentId &&
!isTargetAncestorOfSource(connection.fromNode.id, connection.toNode.id, nodes, edges)

if (!canConnect) {
// not a valid connection
return
}
const existing = finalPredecessors.find((p) => p?.id === connection.fromNode?.id)
if (!existing && isFlowWorkflowNode(connection.fromNode)) {
finalPredecessors = [...finalPredecessors, connection.fromNode]
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/components/nodes/loops/DynamicGroupNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import useDetachNodes from './useDetachNodes'
import { getRelativeNodesBounds } from './groupNodeUtils'
import { RootState } from '@/store/store'
import { getNodeTitle } from '@/utils/flowUtils'
import { isTargetAncestorOfSource } from '@/utils/cyclicEdgeUtils'
import { updateNodeTitle } from '@/store/flowSlice'
import styles from '../DynamicNode.module.css'
import { TaskStatus } from '@/types/api_types/taskSchemas'
Expand Down Expand Up @@ -156,7 +157,9 @@ const DynamicGroupNode: React.FC<DynamicGroupNodeProps> = ({ id }) => {
// Check if nodes have the same parent or both have no parent
const fromNodeParentId = connection.fromNode?.parentId
const toNodeParentId = connection.toNode?.parentId
const canConnect = fromNodeParentId === toNodeParentId
const canConnect =
fromNodeParentId === toNodeParentId &&
!isTargetAncestorOfSource(connection.fromNode.id, connection.toNode.id, nodes, edges)

if (
canConnect &&
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/store/flowSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { TestInput } from '@/types/api_types/workflowSchemas'
import { WorkflowDefinition, WorkflowNodeCoordinates } from '@/types/api_types/workflowSchemas'
import { RouteConditionGroup } from '@/types/api_types/routerSchemas'
import { isEqual } from 'lodash'
import { isTargetAncestorOfSource } from '@/utils/cyclicEdgeUtils'

export interface NodeTypes {
[key: string]: any
Expand Down Expand Up @@ -275,6 +276,16 @@ const flowSlice = createSlice({
},

connect: (state, action: PayloadAction<{ connection: Connection }>) => {
if (
isTargetAncestorOfSource(
action.payload.connection.source,
action.payload.connection.target,
state.nodes,
state.edges
)
) {
return
}
saveToHistory(state)
const { connection } = action.payload

Expand Down
25 changes: 25 additions & 0 deletions frontend/src/utils/cyclicEdgeUtils.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Node, Edge } from '@xyflow/react'

export const isTargetAncestorOfSource = (sourceId: string, targetId: string, nodes: Node[], edges: Edge[]): boolean => {
if (!sourceId || !targetId) {
return false
}
if (sourceId === targetId) {
return true
}

// Find all outgoing edges from the target node
const outgoingEdges = edges.filter((edge) => edge.source === targetId)
if (outgoingEdges.length === 0) {
return false
}

// Get all child nodes
const childNodes = outgoingEdges.map((edge) => nodes.find((node) => node.id === edge.target)).filter(Boolean)
if (childNodes.length === 0) {
return false
}

// Check if any child node is the source node
return childNodes.some((node) => isTargetAncestorOfSource(sourceId, node.id, nodes, edges))
}

0 comments on commit 55373e8

Please sign in to comment.