Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand Down
173 changes: 173 additions & 0 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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
Expand Down