From bb1e1b3d18b0fbefce1e12aa19e9e593e838ef7e Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 23 Oct 2026 05:03:27 +0530 Subject: [PATCH] fix(validate): a loop on a side path no longer bounds a cycle that bypasses it The unbounded-cycle check accepted a cycle as bounded when any loop node lay anywhere on it. A loop hanging off a side path (b -> l -> c) left the direct b -> c -> b cycle with no pass count, and validation accepted it. A cycle is now bounded only when its back-edge touches a loop node, or no path from its head back to its tail avoids every loop node. --- .../loop_tests/loop_tests_part_01_tests.rs | 38 ++++++++++++++++ crates/tinyflows/src/validate/loops.rs | 44 ++++++++++++++++--- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs b/crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs index 8f0593a1..c30e4f4b 100644 --- a/crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs +++ b/crates/tinyflows/src/loop_tests/loop_tests_part_01_tests.rs @@ -239,3 +239,41 @@ fn a_merge_outside_the_cycle_is_accepted() { }; assert_eq!(validate_all(&graph), Vec::new()); } + +/// A `loop` node that hangs off the cycle on a side path does not bound it: the +/// direct `b -> c -> b` cycle never passes through the loop, so it spins with no +/// pass count. Refuse it like any other unbounded cycle. +#[test] +fn loop_on_side_path_does_not_bound_a_bypass_cycle() { + let graph = WorkflowGraph { + nodes: vec![ + node("t", NodeKind::Trigger), + node("b", NodeKind::OutputParser), + node("c", NodeKind::OutputParser), + node_cfg( + "l", + NodeKind::Loop, + serde_json::json!({ "max_iterations": 2 }), + ), + node("work", NodeKind::OutputParser), + ], + edges: vec![ + edge_on("t", "main", "b"), + edge_on("b", "main", "c"), + edge_on("c", "main", "b"), + edge_on("b", "main", "l"), + edge_on("l", "body", "work"), + edge_on("work", "main", "l"), + edge_on("l", "done", "c"), + ], + ..Default::default() + }; + + let errors = validate_all(&graph); + assert!( + errors + .iter() + .any(|error| matches!(error, ValidationError::IllegalCycle(_))), + "the b <-> c cycle bypasses the loop and must be refused: {errors:?}" + ); +} diff --git a/crates/tinyflows/src/validate/loops.rs b/crates/tinyflows/src/validate/loops.rs index 01493cd5..9e1f9d65 100644 --- a/crates/tinyflows/src/validate/loops.rs +++ b/crates/tinyflows/src/validate/loops.rs @@ -181,16 +181,48 @@ pub(super) fn validate_loops(graph: &WorkflowGraph, errors: &mut Vec from` path that never counts a pass. + // The cycle is bounded only when the loop sits on every way round. + let bounded = is_loop(to) || is_loop(from) || !path_avoiding(graph, to, from, is_loop); if !bounded { errors.push(ValidationError::IllegalCycle(to.clone())); } } } } + +/// Whether `target` is reachable from `start` without passing through a node +/// `skip` matches. `start` and `target` themselves are never skipped. +fn path_avoiding( + graph: &WorkflowGraph, + start: &str, + target: &str, + skip: impl Fn(&str) -> bool, +) -> bool { + let mut seen = HashSet::new(); + let mut stack = vec![start]; + while let Some(node) = stack.pop() { + if node == target { + return true; + } + if !seen.insert(node) || (node != start && skip(node)) { + continue; + } + stack.extend( + graph + .edges + .iter() + .filter(|edge| edge.from_node == node) + .map(|edge| edge.to_node.as_str()), + ); + } + false +}