diff --git a/src/error.rs b/src/error.rs index ba2a3c2..70ce1db 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,6 +74,31 @@ pub enum ValidationError { /// The highest schema version this crate understands. supported: u32, }, + + /// A `condition` node has an outgoing edge whose `from_port` is not one of + /// its two declared branch ports (`"true"` / `"false"`). + /// + /// Routing is keyed EXCLUSIVELY on `from_port` (see `engine::outgoing_by_port` + /// / `handler_routing`) — `to_port` is never consulted to decide which + /// successor fires. A condition node authored with the branch label on + /// `to_port` instead (e.g. `{from_port:"main", to_port:"true"}` and + /// `{from_port:"main", to_port:"false"}`) puts both edges in the SAME + /// `from_port` group, which `handler_routing` classifies as a parallel + /// `FanOut` — silently driving BOTH branches unconditionally instead of + /// gating on the condition's actual result. This is a HARD authoring + /// mistake, not a runtime data issue, so it is rejected here rather than + /// left as a silent no-op condition. + #[error( + "condition node {node} has an outgoing edge with from_port {from_port:?} — condition \ + edges must emit on from_port \"true\" or \"false\" (the branch label belongs on \ + from_port, not to_port; routing is keyed exclusively on from_port)" + )] + InvalidConditionRouting { + /// The offending condition node's id. + node: String, + /// The edge's actual (invalid) `from_port` value. + from_port: String, + }, } /// Errors produced while compiling or running a workflow. @@ -160,6 +185,16 @@ mod tests { "schema_version 5 is newer than this crate supports (max 1); \ upgrade tinyflows to load this graph" ); + assert_eq!( + ValidationError::InvalidConditionRouting { + node: "gate".to_string(), + from_port: "main".to_string(), + } + .to_string(), + "condition node gate has an outgoing edge with from_port \"main\" — condition \ + edges must emit on from_port \"true\" or \"false\" (the branch label belongs on \ + from_port, not to_port; routing is keyed exclusively on from_port)" + ); } #[test] diff --git a/src/validate.rs b/src/validate.rs index 346a034..daea24e 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -113,6 +113,32 @@ pub fn validate(graph: &WorkflowGraph) -> Result<(), ValidationError> { } } + // A `condition` node's outgoing edges must emit on `from_port` "true" or + // "false" — routing is keyed EXCLUSIVELY on `from_port` (see + // `engine::outgoing_by_port` / `handler_routing`), so any other value + // (most commonly the default `"main"`, from an authoring mistake that put + // the branch label on `to_port` instead) is a hard authoring bug: it + // silently degrades to a parallel `FanOut` that drives BOTH branches + // unconditionally, with no runtime error or warning to point at the + // mistake. Caught here, at the door, instead. + let condition_node_ids: HashSet<&str> = graph + .nodes + .iter() + .filter(|n| n.kind == NodeKind::Condition) + .map(|n| n.id.as_str()) + .collect(); + for edge in &graph.edges { + if condition_node_ids.contains(edge.from_node.as_str()) + && edge.from_port != "true" + && edge.from_port != "false" + { + return Err(ValidationError::InvalidConditionRouting { + node: edge.from_node.clone(), + from_port: edge.from_port.clone(), + }); + } + } + Ok(()) } @@ -457,6 +483,153 @@ mod tests { assert_eq!(validate(&graph), Ok(())); } + fn condition_node(id: &str) -> Node { + node(id, NodeKind::Condition) + } + + #[test] + fn accepts_condition_with_branch_label_on_from_port() { + // The CORRECT shape (B23/B24): the branch label lives on `from_port`, + // `to_port` stays `"main"`. + let graph = WorkflowGraph { + nodes: vec![ + node("t", NodeKind::Trigger), + condition_node("gate"), + node("yes", NodeKind::Agent), + node("no", NodeKind::Agent), + ], + edges: vec![ + Edge { + from_node: "t".to_string(), + from_port: "main".to_string(), + to_node: "gate".to_string(), + to_port: "main".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "true".to_string(), + to_node: "yes".to_string(), + to_port: "main".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "false".to_string(), + to_node: "no".to_string(), + to_port: "main".to_string(), + }, + ], + ..Default::default() + }; + assert_eq!(validate(&graph), Ok(())); + } + + #[test] + fn accepts_condition_with_only_one_branch_wired() { + // Wiring only the `true` (or only the `false`) branch is legal — the + // other simply dead-ends. + let graph = WorkflowGraph { + nodes: vec![ + node("t", NodeKind::Trigger), + condition_node("gate"), + node("yes", NodeKind::Agent), + ], + edges: vec![ + Edge { + from_node: "t".to_string(), + from_port: "main".to_string(), + to_node: "gate".to_string(), + to_port: "main".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "true".to_string(), + to_node: "yes".to_string(), + to_port: "main".to_string(), + }, + ], + ..Default::default() + }; + assert_eq!(validate(&graph), Ok(())); + } + + #[test] + fn rejects_condition_with_branch_label_on_to_port_instead_of_from_port() { + // The BAD shape (B23/B24 — the exact bug the workflow_builder agent + // produced live): both edges share `from_port: "main"` with the branch + // label on `to_port` instead. Without this check, `handler_routing` + // would see one `from_port` group with two targets and classify it as + // a parallel `FanOut`, silently driving BOTH branches unconditionally. + let graph = WorkflowGraph { + nodes: vec![ + node("t", NodeKind::Trigger), + condition_node("gate"), + node("yes", NodeKind::Agent), + node("no", NodeKind::Agent), + ], + edges: vec![ + Edge { + from_node: "t".to_string(), + from_port: "main".to_string(), + to_node: "gate".to_string(), + to_port: "main".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "main".to_string(), + to_node: "yes".to_string(), + to_port: "true".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "main".to_string(), + to_node: "no".to_string(), + to_port: "false".to_string(), + }, + ], + ..Default::default() + }; + assert_eq!( + validate(&graph), + Err(ValidationError::InvalidConditionRouting { + node: "gate".to_string(), + from_port: "main".to_string(), + }) + ); + } + + #[test] + fn rejects_condition_with_unrecognized_from_port() { + let graph = WorkflowGraph { + nodes: vec![ + node("t", NodeKind::Trigger), + condition_node("gate"), + node("other", NodeKind::Agent), + ], + edges: vec![ + Edge { + from_node: "t".to_string(), + from_port: "main".to_string(), + to_node: "gate".to_string(), + to_port: "main".to_string(), + }, + Edge { + from_node: "gate".to_string(), + from_port: "maybe".to_string(), + to_node: "other".to_string(), + to_port: "main".to_string(), + }, + ], + ..Default::default() + }; + assert_eq!( + validate(&graph), + Err(ValidationError::InvalidConditionRouting { + node: "gate".to_string(), + from_port: "maybe".to_string(), + }) + ); + } + #[test] fn duplicate_id_is_reported_before_trigger_checks() { // Two agents sharing an id and no trigger: the duplicate-id check runs