feat: implement workflow validation to reject duplicate edges and remove redundant edge filtering logic

Author: hanorik

workflow/validation.go

@@ -23,6 +23,9 @@ import (
 // distinct Node instances that share the same Name.
 var ErrDuplicateNodeName = errors.New("duplicate node name")
 
+// ErrDuplicateEdge is returned when an edge set contains two identical edges.
+var ErrDuplicateEdge = errors.New("duplicate edge")
+
 // validateUniqueNames checks that all nodes in the edge set have unique names.
 // If duplicate node names are found, it returns an error. The equality between
 // nodes is checked by comparing the nodes directly.
@@ -48,3 +51,25 @@ func validateUniqueNames(edges []Edge) error {
 	}
 	return nil
 }
+
+// validateWorkflow executes a set of workflow validation checks.
+func validateWorkflow(workflow *Workflow) error {
+	if err := validateUniqueEdges(workflow); err != nil {
+		return err
+	}
+	return nil
+}
+
+// validateUniqueEdges checks that there are no duplicate edges in the workflow.
+func validateUniqueEdges(workflow *Workflow) error {
+	for node, edges := range workflow.edges {
+		uniqueEdges := make(map[Node]struct{})
+		for _, edge := range edges {
+			if _, ok := uniqueEdges[edge.To]; ok {
+				return fmt.Errorf("%w: from %s to %s", ErrDuplicateEdge, node.Name(), edge.To.Name())
+			}
+			uniqueEdges[edge.To] = struct{}{}
+		}
+	}
+	return nil
+}

workflow/validation_test.go

@@ -64,3 +64,15 @@ func TestUniqueNames(t *testing.T) {
 		})
 	}
 }
+
+func TestDuplicateEdges(t *testing.T) {
+	nodeA := &dummyNode{name: "A"}
+	nodeB := &dummyNode{name: "B"}
+	edges := []Edge{
+		{From: nodeA, To: nodeB},
+		{From: nodeA, To: nodeB},
+	}
+	if err := validateUniqueEdges(&Workflow{edges: map[Node][]Edge{nodeA: edges}}); err == nil {
+		t.Errorf("expected an error, got %v", err)
+	}
+}

workflow/workflow.go

@@ -185,7 +185,11 @@ func New(edges []Edge) (*Workflow, error) {
 	for _, edge := range edges {
 		adj[edge.From] = append(adj[edge.From], edge)
 	}
-	return &Workflow{edges: adj}, nil
+	wf := &Workflow{edges: adj}
+	if err := validateWorkflow(wf); err != nil {
+		return nil, fmt.Errorf("workflow validation failed: %w", err)
+	}
+	return wf, nil
 }
 
 type nodeInput struct {
@@ -208,15 +212,10 @@ func (w *Workflow) findNextNodes(currentNode Node, input any, event *session.Eve
 	}
 	matched := false
 	queue := []nodeInput{}
-	added := make(map[Node]struct{})
 	var defaultRouteNode Node
 	for _, edge := range w.edges[currentNode] {
-		if _, ok := added[edge.To]; ok {
-			continue
-		}
 		if edge.Route == nil {
 			queue = append(queue, nodeInput{node: edge.To, input: input})
-			added[edge.To] = struct{}{}
 			continue
 		}
 		if edge.Route == Default {
@@ -225,7 +224,6 @@ func (w *Workflow) findNextNodes(currentNode Node, input any, event *session.Eve
 		}
 		if edge.Route.Matches(event) {
 			queue = append(queue, nodeInput{node: edge.To, input: input})
-			added[edge.To] = struct{}{}
 			matched = true
 		}
 	}

workflow/workflow_test.go

@@ -232,10 +232,10 @@ func TestWorkflowRouting(t *testing.T) {
 	}
 
 	type testCase struct {
-		name           string
-		startRoutes    []string
-		edges          func(nodeStart *CustomRouteNode, nodeA, nodeB *FunctionNode, nodeC *CustomRouteNode, nodeD *FunctionNode) []Edge
-		expectedExec   []string
+		name         string
+		startRoutes  []string
+		edges        func(nodeStart *CustomRouteNode, nodeA, nodeB *FunctionNode, nodeC *CustomRouteNode, nodeD *FunctionNode) []Edge
+		expectedExec []string
 	}
 
 	createNodes := func() (*CustomRouteNode, *FunctionNode, *FunctionNode, *CustomRouteNode, *FunctionNode, *testTracker) {
@@ -383,21 +383,6 @@ func TestWorkflowRouting(t *testing.T) {
 			},
 			expectedExec: nil,
 		},
-		{
-			name:        "duplicate edges to same node",
-			startRoutes: []string{"branchA"},
-			edges: func(x *CustomRouteNode, a, b *FunctionNode, c *CustomRouteNode, d *FunctionNode) []Edge {
-				return []Edge{
-					{From: Start, To: x},
-					{From: x, To: a},
-					{From: x, To: a, Route: StringRoute("branchA")},
-					{From: x, To: b},
-					{From: x, To: c},
-					{From: c, To: d},
-				}
-			},
-			expectedExec: []string{"A", "B", "C", "D"},
-		},
 	}
 
 	for _, tc := range tests {