mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Add node-level on_failure override
A node can now set its own on_failure attribute to override the
graph-level failed-node routing policy in either direction: a
best-effort node can keep route inside an exit graph, and a critical
node can exit while the rest of the graph keeps the default. An absent
node attribute inherits the graph policy.
- Node::on_failure returns Option<OnFailure> so absence means inherit
- Graph::resolve_on_failure(node_id) is the single resolution point,
returning ResolvedOnFailure { policy, scope } so the executor's
end-of-run message names the scope that stopped routing
- The core Graph trait method becomes resolve_on_failure(node_id); the
graph-scope failure message is unchanged
- The failed-human-gate fallthrough block stays independent of a
node-level route override
- Validation now accepts and value-checks node-level on_failure (it
previously warned that node placement had no effect) and keeps the
edge-placement warning with updated wording
- Document precedence in transitions, failures, and the DOT reference,
and extend today's changelog entry
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chraa21RK7i2KqHdZSJLb8
This commit is contained in:
parent
9223349101
commit
491babe5da
15 changed files with 469 additions and 69 deletions
|
|
@ -10,6 +10,11 @@ target exists.
|
|||
|
||||
The default `on_failure="route"` preserves existing workflow behavior.
|
||||
|
||||
A node can also set its own `on_failure` to override the graph policy in
|
||||
either direction: a best-effort node can use `on_failure="route"` inside an
|
||||
`exit` graph, or a single critical node can use `on_failure="exit"` while the
|
||||
rest of the graph keeps the default.
|
||||
|
||||
```dot
|
||||
digraph Build {
|
||||
graph [on_failure="exit"]
|
||||
|
|
|
|||
|
|
@ -62,6 +62,8 @@ digraph Build {
|
|||
|
||||
Fabro still uses an explicit recovery edge, such as `condition="outcome=failed"`, before it applies this policy. Matching preferred labels and suggested next node IDs also remain explicit routes. If no explicit edge matches, `exit` skips the unconditional edge and checks retry targets. The run ends as failed only when no retry target exists.
|
||||
|
||||
Set `on_failure` on a node to control that node alone. The node-level attribute overrides the graph level, in both directions: a node can opt out of a graph-level `exit` with `on_failure="route"`, or stop the run on its own failure with `on_failure="exit"` while the rest of the graph keeps the default. A node without the attribute inherits the graph policy. See [Failed-node routing policy](/workflows/transitions#failed-node-routing-policy).
|
||||
|
||||
The policy applies only to `failed`. Other outcomes keep their normal routing behavior. For parallel nodes, the policy uses the completed parallel node's final outcome. It does not stop or cancel individual branches early.
|
||||
|
||||
## Retry layers
|
||||
|
|
@ -330,7 +332,7 @@ A node failure does **not** automatically terminate the run. Fabro follows this
|
|||
4. **Node retries** — re-execute the entire handler (per the retry policy)
|
||||
5. **Direct jump** — use `jump_to_node` when the outcome supplies one
|
||||
6. **Explicit edge routing** — look for a matching condition, preferred label, or suggested next node
|
||||
7. **Failure policy** — with `on_failure="exit"`, skip the unconditional edge; with `route` or no attribute, keep normal fallback routing
|
||||
7. **Failure policy** — with an effective `on_failure="exit"` (node-level `on_failure` first, then graph-level), skip the unconditional edge; with `route` or no attribute, keep normal fallback routing
|
||||
8. **Unconditional edge** — in `route` mode, use an edge without a condition as the fallback
|
||||
9. **Retry target** — if no edge was selected, check `retry_target` and `fallback_retry_target` on the node, then on the graph
|
||||
10. **Run failure** — if none of the above produces a path forward, the run terminates
|
||||
|
|
|
|||
|
|
@ -199,6 +199,7 @@ Other node types still need their shape, because their attributes don't identify
|
|||
| `class` | String | Classes for [stylesheet](/workflows/stylesheets) targeting. Separate multiple classes with spaces. Commas are also accepted for compatibility. |
|
||||
| `timeout` | Duration | Execution timeout (e.g. `900s`). An agent's wait for human input does not consume this budget. On a human node, this is the response deadline. |
|
||||
| `max_visits` | Integer | Max times this node can execute in a run. Overrides the graph-level `max_node_visits` for this node. |
|
||||
| `on_failure` | String | Failed-node routing policy for this node: `route` or `exit`. Overrides the graph-level `on_failure`. |
|
||||
| `max_retries` | Integer | Override default retry count |
|
||||
| `retry_policy` | String | Named preset: `none`, `standard`, `aggressive`, `linear`, `patient` |
|
||||
| `retry_target` | String | Node ID to jump to on retry |
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ When a node completes, it produces an **outcome** with a [stage outcome](/execut
|
|||
2. **Condition match** — Edges with a `condition` attribute are evaluated first. If one or more conditions match, the edge with the highest `weight` wins (lexical tiebreak on target node ID).
|
||||
3. **Preferred label** — If the node's outcome includes a preferred label (for example, from a human gate selection), the edge whose `label` matches is chosen.
|
||||
4. **Suggested next** — If the node suggests a specific next node ID, the edge pointing to that node is chosen.
|
||||
5. **Failure policy** — For a failed outcome and graph-level `on_failure="exit"`, Fabro skips the unconditional fallback.
|
||||
5. **Failure policy** — For a failed outcome with an effective `on_failure="exit"` policy (node-level `on_failure` first, then graph-level), Fabro skips the unconditional fallback.
|
||||
6. **Unconditional fallback** — Edges without conditions are considered last, again using `weight` then lexical tiebreak.
|
||||
7. **Retry target** — For a failed outcome with no selected edge, Fabro checks node-level and graph-level `retry_target` and `fallback_retry_target` values.
|
||||
|
||||
|
|
@ -21,11 +21,13 @@ If no edge or retry target supplies a next node, the workflow ends. A failed nod
|
|||
|
||||
## Failed-node routing policy
|
||||
|
||||
The graph-level `on_failure` attribute controls whether a failed node can take an unconditional edge:
|
||||
The `on_failure` attribute controls whether a failed node can take an unconditional edge:
|
||||
|
||||
- `route` keeps the existing routing behavior. It is the default.
|
||||
- `exit` stops normal fallback routing after explicit routes fail to match.
|
||||
|
||||
Set it at the graph level to apply the policy to every node, or on a node to control that node alone. A node-level `on_failure` overrides the graph level. A node without the attribute inherits the graph policy.
|
||||
|
||||
This lets a linear workflow stop at the first failed work node:
|
||||
|
||||
```dot title="stop-on-failure.fabro"
|
||||
|
|
@ -42,12 +44,30 @@ digraph Build {
|
|||
}
|
||||
```
|
||||
|
||||
Node-level overrides work in both directions. A strict graph can mark one best-effort node as `route` so its failure continues down the unconditional edge, and a default graph can mark one critical node as `exit`:
|
||||
|
||||
```dot title="mixed-policies.fabro"
|
||||
digraph Build {
|
||||
graph [on_failure="exit"]
|
||||
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
implement [prompt="Implement the change"]
|
||||
lint [prompt="Run optional lint cleanup" on_failure="route"]
|
||||
verify [prompt="Verify the implementation"]
|
||||
|
||||
start -> implement -> lint -> verify -> exit
|
||||
}
|
||||
```
|
||||
|
||||
The `exit` policy only applies to the `failed` outcome. It does not change routing for `succeeded`, `partially_succeeded`, or `skipped` outcomes.
|
||||
|
||||
Conditioned edges, matching preferred labels, and matching suggested node IDs are explicit recovery routes. They still take priority under `exit`. An unmatched preferred label or suggested node ID does not make an unconditional edge explicit.
|
||||
|
||||
Retry targets also remain available. Fabro checks them after it skips the unconditional fallback. Use graph-level `max_node_visits` or node-level `max_visits` to bound workflows whose retry targets return to a failing path.
|
||||
|
||||
A failed human gate never falls through to an unconditional edge, regardless of policy. Node-level `on_failure="route"` does not change that; route an interrupted gate explicitly with `condition="outcome=failed"`.
|
||||
|
||||
When `exit` stops routing, Fabro checkpoints the failed node without a next node and ends the run as failed. It does not execute the graph's exit node or emit an edge selection for an edge it did not take. An explicit recovery route can still reach the exit node normally.
|
||||
|
||||
For a parallel node, `exit` sees the final outcome returned by the parallel handler. It can stop routing for a failed parallel outcome. It does not add branch-level fail-fast behavior, and a `partially_succeeded` parallel outcome continues normally.
|
||||
|
|
|
|||
|
|
@ -359,6 +359,24 @@ fn invalid() {
|
|||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_node_on_failure_is_a_validation_failure() {
|
||||
let context = test_context!();
|
||||
let mut cmd = context.validate();
|
||||
cmd.arg(fixture("on_failure_node_invalid.fabro"));
|
||||
fabro_snapshot!(context.filters(), cmd, @"
|
||||
success: false
|
||||
exit_code: 1
|
||||
----- stdout -----
|
||||
----- stderr -----
|
||||
Workflow: InvalidNodeOnFailure (3 nodes, 2 edges)
|
||||
Graph: [FIXTURES]/on_failure_node_invalid.fabro
|
||||
error [node: work]: Node 'work' has invalid on_failure value 'stop' (on_failure_valid)
|
||||
fix: Use one of: route, exit
|
||||
× Validation failed
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_on_failure_is_a_validation_failure() {
|
||||
let context = test_context!();
|
||||
|
|
|
|||
|
|
@ -17,48 +17,53 @@ impl LintRule for Rule {
|
|||
fn apply(&self, graph: &Graph) -> Vec<Diagnostic> {
|
||||
let mut diagnostics = Vec::new();
|
||||
|
||||
let invalid_value_message = match graph.attrs.get("on_failure") {
|
||||
None => None,
|
||||
Some(AttrValue::String(value)) if value.parse::<OnFailure>().is_ok() => None,
|
||||
Some(AttrValue::String(value)) => {
|
||||
Some(format!("Graph has invalid on_failure value '{value}'"))
|
||||
}
|
||||
Some(_) => Some("Graph attribute 'on_failure' must be a string".to_string()),
|
||||
};
|
||||
if let Some(message) = invalid_value_message {
|
||||
diagnostics.push(Diagnostic {
|
||||
let invalid_value = |subject: &str, value: &AttrValue| -> Option<Diagnostic> {
|
||||
let message = match value {
|
||||
AttrValue::String(value) if value.parse::<OnFailure>().is_ok() => return None,
|
||||
AttrValue::String(value) => {
|
||||
format!("{subject} has invalid on_failure value '{value}'")
|
||||
}
|
||||
_ => format!("{subject} attribute 'on_failure' must be a string"),
|
||||
};
|
||||
Some(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Error,
|
||||
message,
|
||||
fix: Some(format!("Use one of: {}", OnFailure::expected_values())),
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
|
||||
let misplaced = |subject: String| Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
"{subject} sets 'on_failure', which has no effect outside graph scope"
|
||||
),
|
||||
fix: Some("Move 'on_failure' to the graph attributes".to_string()),
|
||||
..Diagnostic::default()
|
||||
})
|
||||
};
|
||||
|
||||
for node in graph.nodes.values() {
|
||||
if node.attrs.contains_key("on_failure") {
|
||||
diagnostics.push(Diagnostic {
|
||||
node_id: Some(node.id.clone()),
|
||||
..misplaced(format!("Node '{}'", node.id))
|
||||
});
|
||||
if let Some(value) = graph.attrs.get("on_failure") {
|
||||
diagnostics.extend(invalid_value("Graph", value));
|
||||
}
|
||||
|
||||
let mut node_ids: Vec<&String> = graph.nodes.keys().collect();
|
||||
node_ids.sort();
|
||||
for node_id in node_ids {
|
||||
let node = &graph.nodes[node_id];
|
||||
if let Some(value) = node.attrs.get("on_failure") {
|
||||
if let Some(diagnostic) = invalid_value(&format!("Node '{node_id}'"), value) {
|
||||
diagnostics.push(Diagnostic {
|
||||
node_id: Some(node_id.clone()),
|
||||
..diagnostic
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for edge in &graph.edges {
|
||||
if edge.attrs.contains_key("on_failure") {
|
||||
diagnostics.push(Diagnostic {
|
||||
rule: self.name().to_string(),
|
||||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
"Edge '{} -> {}' sets 'on_failure', which has no effect on edges",
|
||||
edge.from, edge.to
|
||||
),
|
||||
fix: Some("Set 'on_failure' on the graph or the source node".to_string()),
|
||||
edge: Some((edge.from.clone(), edge.to.clone())),
|
||||
..misplaced(format!("Edge '{} -> {}'", edge.from, edge.to))
|
||||
..Diagnostic::default()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -89,6 +94,18 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_supported_node_values() {
|
||||
let mut graph = minimal_graph();
|
||||
for value in ["route", "exit"] {
|
||||
graph.nodes.insert(
|
||||
"work".to_string(),
|
||||
node_with_attrs("work", &[("on_failure", value)]),
|
||||
);
|
||||
assert!(Rule.apply(&graph).is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_unsupported_graph_value() {
|
||||
let mut graph = minimal_graph();
|
||||
|
|
@ -129,13 +146,49 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn warns_for_node_and_edge_placement() {
|
||||
fn rejects_unsupported_node_value() {
|
||||
let mut graph = minimal_graph();
|
||||
graph.nodes.insert(
|
||||
"work".to_string(),
|
||||
node_with_attrs("work", &[("on_failure", "exit")]),
|
||||
node_with_attrs("work", &[("on_failure", "stop")]),
|
||||
);
|
||||
|
||||
let diagnostics = Rule.apply(&graph);
|
||||
|
||||
assert_eq!(diagnostics.len(), 1);
|
||||
assert_eq!(diagnostics[0].severity, Severity::Error);
|
||||
assert_eq!(
|
||||
diagnostics[0].message,
|
||||
"Node 'work' has invalid on_failure value 'stop'"
|
||||
);
|
||||
assert_eq!(diagnostics[0].node_id.as_deref(), Some("work"));
|
||||
assert_eq!(
|
||||
diagnostics[0].fix.as_deref(),
|
||||
Some("Use one of: route, exit")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_non_string_node_value() {
|
||||
let mut graph = minimal_graph();
|
||||
let mut node = node_with_attrs("work", &[]);
|
||||
node.attrs
|
||||
.insert("on_failure".to_string(), AttrValue::Boolean(true));
|
||||
graph.nodes.insert("work".to_string(), node);
|
||||
|
||||
let diagnostics = Rule.apply(&graph);
|
||||
|
||||
assert_eq!(diagnostics.len(), 1);
|
||||
assert_eq!(diagnostics[0].severity, Severity::Error);
|
||||
assert_eq!(
|
||||
diagnostics[0].message,
|
||||
"Node 'work' attribute 'on_failure' must be a string"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn warns_for_edge_placement() {
|
||||
let mut graph = minimal_graph();
|
||||
let mut edge = Edge::new("start", "work");
|
||||
edge.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
|
|
@ -145,24 +198,19 @@ mod tests {
|
|||
|
||||
let diagnostics = Rule.apply(&graph);
|
||||
|
||||
assert_eq!(diagnostics.len(), 2);
|
||||
assert!(
|
||||
diagnostics
|
||||
.iter()
|
||||
.all(|diagnostic| diagnostic.severity == Severity::Warning)
|
||||
assert_eq!(diagnostics.len(), 1);
|
||||
assert_eq!(diagnostics[0].severity, Severity::Warning);
|
||||
assert_eq!(
|
||||
diagnostics[0].message,
|
||||
"Edge 'start -> work' sets 'on_failure', which has no effect on edges"
|
||||
);
|
||||
assert!(
|
||||
diagnostics
|
||||
.iter()
|
||||
.all(|diagnostic| diagnostic.message.contains("graph scope"))
|
||||
assert_eq!(
|
||||
diagnostics[0].fix.as_deref(),
|
||||
Some("Set 'on_failure' on the graph or the source node")
|
||||
);
|
||||
assert!(
|
||||
diagnostics
|
||||
.iter()
|
||||
.any(|diagnostic| diagnostic.node_id.as_deref() == Some("work"))
|
||||
assert_eq!(
|
||||
diagnostics[0].edge,
|
||||
Some(("start".to_string(), "work".to_string()))
|
||||
);
|
||||
assert!(diagnostics.iter().any(|diagnostic| {
|
||||
diagnostic.edge == Some(("start".to_string(), "work".to_string()))
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use std::sync::Arc;
|
|||
use fabro_core::error::{Error as CoreError, Result as CoreResult};
|
||||
use fabro_core::graph::{EdgeSelection as CoreEdgeSelection, EdgeSpec, Graph, NodeSpec};
|
||||
use fabro_graphviz::graph::types::{Edge as GvEdge, Graph as GvGraph, Node as GvNode};
|
||||
use fabro_types::OnFailure;
|
||||
use fabro_types::ResolvedOnFailure;
|
||||
|
||||
use crate::context::Context;
|
||||
use crate::outcome::{BilledModelUsage, Outcome};
|
||||
|
|
@ -131,7 +131,7 @@ impl Graph for WorkflowGraph {
|
|||
routing::get_retry_target(failed_node_id, self.inner())
|
||||
}
|
||||
|
||||
fn on_failure(&self) -> OnFailure {
|
||||
self.inner().on_failure()
|
||||
fn resolve_on_failure(&self, node_id: &str) -> ResolvedOnFailure {
|
||||
self.inner().resolve_on_failure(node_id)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -75,7 +75,7 @@ pub(crate) fn select_edge<'a>(
|
|||
}
|
||||
}
|
||||
|
||||
if outcome.status.is_failure() && graph.on_failure() == OnFailure::Exit {
|
||||
if outcome.status.is_failure() && graph.resolve_on_failure(&node.id).policy == OnFailure::Exit {
|
||||
return None;
|
||||
}
|
||||
|
||||
|
|
@ -264,6 +264,13 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
fn set_node_on_failure(graph: &mut Graph, node_id: &str, value: &str) {
|
||||
graph.nodes.get_mut(node_id).unwrap().attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String(value.to_string()),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn normalize_label_lowercase_and_trim() {
|
||||
assert_eq!(normalize_label(" Yes "), "yes");
|
||||
|
|
@ -524,6 +531,84 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_exit_overrides_graph_route_and_blocks_unconditional_edge() {
|
||||
for graph_policy in [None, Some(OnFailure::Route)] {
|
||||
let mut graph = make_graph_with_edges(vec![Edge::new("a", "b")]);
|
||||
if let Some(policy) = graph_policy {
|
||||
set_on_failure(&mut graph, policy);
|
||||
}
|
||||
set_node_on_failure(&mut graph, "a", "exit");
|
||||
let node = graph.nodes.get("a").unwrap();
|
||||
let outcome = Outcome::fail_classify("boom");
|
||||
|
||||
assert!(
|
||||
select_edge(node, &outcome, &Context::new(), &graph, "deterministic").is_none()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_route_overrides_graph_exit_and_takes_unconditional_edge() {
|
||||
let mut graph = make_graph_with_edges(vec![Edge::new("a", "b")]);
|
||||
set_on_failure(&mut graph, OnFailure::Exit);
|
||||
set_node_on_failure(&mut graph, "a", "route");
|
||||
let node = graph.nodes.get("a").unwrap();
|
||||
let outcome = Outcome::fail_classify("boom");
|
||||
|
||||
let selected =
|
||||
select_edge(node, &outcome, &Context::new(), &graph, "deterministic").unwrap();
|
||||
|
||||
assert_eq!(selected.edge.to, "b");
|
||||
assert_eq!(selected.reason, "unconditional");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_policy_does_not_affect_routing_from_other_nodes() {
|
||||
let mut graph = make_graph_with_edges(vec![Edge::new("a", "b")]);
|
||||
set_node_on_failure(&mut graph, "b", "exit");
|
||||
let node = graph.nodes.get("a").unwrap();
|
||||
let outcome = Outcome::fail_classify("boom");
|
||||
|
||||
let selected =
|
||||
select_edge(node, &outcome, &Context::new(), &graph, "deterministic").unwrap();
|
||||
|
||||
assert_eq!(selected.edge.to, "b");
|
||||
assert_eq!(selected.reason, "unconditional");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_node_on_failure_value_inherits_graph_policy() {
|
||||
let mut graph = make_graph_with_edges(vec![Edge::new("a", "b")]);
|
||||
set_on_failure(&mut graph, OnFailure::Exit);
|
||||
set_node_on_failure(&mut graph, "a", "bogus");
|
||||
let node = graph.nodes.get("a").unwrap();
|
||||
let outcome = Outcome::fail_classify("boom");
|
||||
|
||||
assert!(select_edge(node, &outcome, &Context::new(), &graph, "deterministic").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failed_human_gate_ignores_node_route_override() {
|
||||
let mut graph = make_graph_with_edges(vec![
|
||||
Edge::new("gate", "approve"),
|
||||
Edge::new("gate", "skip"),
|
||||
]);
|
||||
set_on_failure(&mut graph, OnFailure::Exit);
|
||||
set_node_on_failure(&mut graph, "gate", "route");
|
||||
let gate = graph.nodes.get_mut("gate").unwrap();
|
||||
gate.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("hexagon".to_string()),
|
||||
);
|
||||
let node = graph.nodes.get("gate").unwrap().clone();
|
||||
let outcome = Outcome::fail_deterministic(
|
||||
"human interaction interrupted before an answer was provided",
|
||||
);
|
||||
|
||||
assert!(select_edge(&node, &outcome, &Context::new(), &graph, "deterministic").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_edge_condition_match() {
|
||||
let mut e1 = Edge::new("a", "fail_path");
|
||||
|
|
|
|||
|
|
@ -1292,6 +1292,33 @@ async fn on_failure_exit_uses_retry_target_instead_of_unconditional_edge() {
|
|||
assert_eq!(*run.visits.lock().unwrap(), vec!["work", "recovery"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn node_on_failure_exit_overrides_graph_route_policy() {
|
||||
let graph = on_failure_graph(r#"work [on_failure="exit"]"#);
|
||||
let run = run_on_failure(&graph, Emitter::default()).await;
|
||||
|
||||
assert_eq!(run.outcome.status, StageOutcome::Failed {
|
||||
retry_requested: false,
|
||||
});
|
||||
assert_eq!(
|
||||
run.outcome.failure_reason(),
|
||||
Some("stage work failed and node on_failure=exit stopped routing")
|
||||
);
|
||||
assert_eq!(*run.visits.lock().unwrap(), vec!["work"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn node_on_failure_route_overrides_graph_exit_policy() {
|
||||
let graph = on_failure_graph(
|
||||
r#"graph [on_failure="exit"]
|
||||
work [on_failure="route"]"#,
|
||||
);
|
||||
let run = run_on_failure(&graph, Emitter::default()).await;
|
||||
|
||||
assert_eq!(run.outcome.status, StageOutcome::Succeeded);
|
||||
assert_eq!(*run.visits.lock().unwrap(), vec!["work", "downstream"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn goal_gate_routes_to_retry_target_on_failure() {
|
||||
// Pipeline:
|
||||
|
|
|
|||
|
|
@ -283,13 +283,15 @@ impl<G: Graph + 'static> Executor<G> {
|
|||
NextStep::End => {
|
||||
let mut outcome = last_outcome.clone();
|
||||
if outcome.status.is_failure() {
|
||||
let message = match graph.on_failure() {
|
||||
let resolved = graph.resolve_on_failure(node.id());
|
||||
let message = match resolved.policy {
|
||||
OnFailure::Route => {
|
||||
format!("stage {} failed with no outgoing fail edge", node.id())
|
||||
}
|
||||
OnFailure::Exit => format!(
|
||||
"stage {} failed and graph on_failure=exit stopped routing",
|
||||
node.id()
|
||||
"stage {} failed and {} on_failure=exit stopped routing",
|
||||
node.id(),
|
||||
resolved.scope
|
||||
),
|
||||
};
|
||||
outcome = Outcome::fail(&message);
|
||||
|
|
@ -2260,6 +2262,70 @@ mod tests {
|
|||
assert_eq!(log.run_end.as_ref(), Some(&outcome));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_node_exit_policy_overrides_graph_route_and_names_node_scope() {
|
||||
let graph = TestGraph::new(
|
||||
vec![
|
||||
TestNode::new("work"),
|
||||
TestNode::new("downstream"),
|
||||
TestNode::terminal("end"),
|
||||
],
|
||||
vec![
|
||||
TestEdge::new("work", "downstream"),
|
||||
TestEdge::new("downstream", "end"),
|
||||
],
|
||||
"work",
|
||||
)
|
||||
.with_node_on_failure("work", OnFailure::Exit);
|
||||
let state = ExecutionState::new(&graph).unwrap();
|
||||
let executor = ExecutorBuilder::new(
|
||||
Arc::new(AlwaysFailHandler::new("boom")) as Arc<dyn NodeHandler<TestGraph>>
|
||||
)
|
||||
.build();
|
||||
|
||||
let (outcome, state) = executor.run(&graph, state).await.unwrap();
|
||||
|
||||
assert_eq!(outcome.status, StageOutcome::Failed {
|
||||
retry_requested: false,
|
||||
});
|
||||
assert_eq!(
|
||||
outcome
|
||||
.failure
|
||||
.as_ref()
|
||||
.map(|failure| failure.message.as_str()),
|
||||
Some("stage work failed and node on_failure=exit stopped routing")
|
||||
);
|
||||
assert!(!state.node_outcomes.contains_key("downstream"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_node_route_policy_overrides_graph_exit() {
|
||||
let graph = TestGraph::new(
|
||||
vec![
|
||||
TestNode::new("work"),
|
||||
TestNode::new("downstream"),
|
||||
TestNode::terminal("end"),
|
||||
],
|
||||
vec![
|
||||
TestEdge::new("work", "downstream"),
|
||||
TestEdge::new("downstream", "end"),
|
||||
],
|
||||
"work",
|
||||
)
|
||||
.with_on_failure(OnFailure::Exit)
|
||||
.with_node_on_failure("work", OnFailure::Route);
|
||||
let state = ExecutionState::new(&graph).unwrap();
|
||||
let handler = DispatchHandler::new(Arc::new(AlwaysSucceedHandler))
|
||||
.with_handler("work", Arc::new(AlwaysFailHandler::new("boom")));
|
||||
let executor =
|
||||
ExecutorBuilder::new(Arc::new(handler) as Arc<dyn NodeHandler<TestGraph>>).build();
|
||||
|
||||
let (outcome, state) = executor.run(&graph, state).await.unwrap();
|
||||
|
||||
assert_eq!(outcome.status, StageOutcome::Succeeded);
|
||||
assert!(state.node_outcomes.contains_key("downstream"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_goal_gate_retry_target_to_terminal_fails_without_looping() {
|
||||
let terminal_visits = Arc::new(AtomicU32::new(0));
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
use std::collections::HashMap;
|
||||
|
||||
use fabro_types::OnFailure;
|
||||
use fabro_types::ResolvedOnFailure;
|
||||
|
||||
use crate::context::Context;
|
||||
use crate::error::Result;
|
||||
|
|
@ -42,5 +42,7 @@ pub trait Graph: Send + Sync {
|
|||
outcomes: &HashMap<String, Outcome<Self::Meta>>,
|
||||
) -> std::result::Result<(), String>;
|
||||
fn get_retry_target(&self, failed_node_id: &str) -> Option<String>;
|
||||
fn on_failure(&self) -> OnFailure;
|
||||
/// Effective failure routing policy for a node: node-level `on_failure`
|
||||
/// overrides the graph level, and an absent node attribute inherits it.
|
||||
fn resolve_on_failure(&self, node_id: &str) -> ResolvedOnFailure;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ use std::sync::Arc;
|
|||
use std::sync::atomic::{AtomicU32, Ordering};
|
||||
|
||||
use async_trait::async_trait;
|
||||
use fabro_types::OnFailure;
|
||||
use fabro_types::{OnFailure, OnFailureScope, ResolvedOnFailure};
|
||||
|
||||
use crate::context::Context;
|
||||
use crate::error::{Error, HandlerErrorDetail, Result};
|
||||
|
|
@ -119,11 +119,12 @@ impl EdgeSpec for TestEdge {
|
|||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct TestGraph {
|
||||
pub nodes: Vec<TestNode>,
|
||||
pub edges: Vec<TestEdge>,
|
||||
pub start_node_id: String,
|
||||
pub retry_targets: HashMap<String, String>,
|
||||
pub on_failure: OnFailure,
|
||||
pub nodes: Vec<TestNode>,
|
||||
pub edges: Vec<TestEdge>,
|
||||
pub start_node_id: String,
|
||||
pub retry_targets: HashMap<String, String>,
|
||||
pub on_failure: OnFailure,
|
||||
pub node_on_failure: HashMap<String, OnFailure>,
|
||||
}
|
||||
|
||||
impl TestGraph {
|
||||
|
|
@ -134,6 +135,7 @@ impl TestGraph {
|
|||
start_node_id: start.to_string(),
|
||||
retry_targets: HashMap::new(),
|
||||
on_failure: OnFailure::Route,
|
||||
node_on_failure: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -148,6 +150,12 @@ impl TestGraph {
|
|||
self.on_failure = on_failure;
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_node_on_failure(mut self, node_id: &str, on_failure: OnFailure) -> Self {
|
||||
self.node_on_failure.insert(node_id.to_string(), on_failure);
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
impl Graph for TestGraph {
|
||||
|
|
@ -217,7 +225,9 @@ impl Graph for TestGraph {
|
|||
}
|
||||
}
|
||||
|
||||
if outcome.status.is_failure() && self.on_failure == OnFailure::Exit {
|
||||
if outcome.status.is_failure()
|
||||
&& self.resolve_on_failure(node.id()).policy == OnFailure::Exit
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
|
|
@ -257,8 +267,17 @@ impl Graph for TestGraph {
|
|||
self.retry_targets.get(failed_node_id).cloned()
|
||||
}
|
||||
|
||||
fn on_failure(&self) -> OnFailure {
|
||||
self.on_failure
|
||||
fn resolve_on_failure(&self, node_id: &str) -> ResolvedOnFailure {
|
||||
match self.node_on_failure.get(node_id) {
|
||||
Some(policy) => ResolvedOnFailure {
|
||||
policy: *policy,
|
||||
scope: OnFailureScope::Node,
|
||||
},
|
||||
None => ResolvedOnFailure {
|
||||
policy: self.on_failure,
|
||||
scope: OnFailureScope::Graph,
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -36,6 +36,22 @@ impl OnFailure {
|
|||
}
|
||||
}
|
||||
|
||||
/// The scope whose `on_failure` attribute supplied a resolved policy.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display, strum::IntoStaticStr)]
|
||||
#[strum(serialize_all = "snake_case")]
|
||||
pub enum OnFailureScope {
|
||||
Node,
|
||||
Graph,
|
||||
}
|
||||
|
||||
/// A failure routing policy together with the scope that supplied it, so
|
||||
/// failure messages can name the attribute that stopped routing.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct ResolvedOnFailure {
|
||||
pub policy: OnFailure,
|
||||
pub scope: OnFailureScope,
|
||||
}
|
||||
|
||||
/// Typed attribute values for nodes, edges, and graph-level attributes.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub enum AttrValue {
|
||||
|
|
@ -313,6 +329,15 @@ impl Node {
|
|||
self.str_attr("fallback_retry_target")
|
||||
}
|
||||
|
||||
/// Node-level failure routing policy override. `None` means the node
|
||||
/// inherits the graph-level policy. Invalid values are rejected during
|
||||
/// workflow validation, so runtime resolution treats them as absent.
|
||||
#[must_use]
|
||||
pub fn on_failure(&self) -> Option<OnFailure> {
|
||||
self.str_attr("on_failure")
|
||||
.and_then(|value| value.parse().ok())
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn fidelity(&self) -> Option<&str> {
|
||||
self.str_attr("fidelity")
|
||||
|
|
@ -581,6 +606,24 @@ impl Graph {
|
|||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Effective failure routing policy for a node. A node-level `on_failure`
|
||||
/// attribute overrides the graph level; an absent (or invalid, hence
|
||||
/// validation-rejected) node attribute inherits the graph policy.
|
||||
#[must_use]
|
||||
pub fn resolve_on_failure(&self, node_id: &str) -> ResolvedOnFailure {
|
||||
let node_policy = self.nodes.get(node_id).and_then(Node::on_failure);
|
||||
match node_policy {
|
||||
Some(policy) => ResolvedOnFailure {
|
||||
policy,
|
||||
scope: OnFailureScope::Node,
|
||||
},
|
||||
None => ResolvedOnFailure {
|
||||
policy: self.on_failure(),
|
||||
scope: OnFailureScope::Graph,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
/// Graph-level `default_fidelity`.
|
||||
pub fn default_fidelity(&self) -> Option<&str> {
|
||||
self.attrs
|
||||
|
|
@ -736,6 +779,63 @@ mod tests {
|
|||
assert_eq!(graph.on_failure(), OnFailure::Exit);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn node_on_failure_parses_valid_values_and_ignores_invalid_ones() {
|
||||
let mut node = Node::new("work");
|
||||
assert_eq!(node.on_failure(), None);
|
||||
|
||||
node.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String("exit".to_string()),
|
||||
);
|
||||
assert_eq!(node.on_failure(), Some(OnFailure::Exit));
|
||||
|
||||
node.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String("stop".to_string()),
|
||||
);
|
||||
assert_eq!(node.on_failure(), None);
|
||||
|
||||
node.attrs
|
||||
.insert("on_failure".to_string(), AttrValue::Boolean(true));
|
||||
assert_eq!(node.on_failure(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_on_failure_prefers_node_policy_over_graph_policy() {
|
||||
let mut graph = Graph::new("test");
|
||||
graph.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String("exit".to_string()),
|
||||
);
|
||||
graph.nodes.insert("bare".to_string(), Node::new("bare"));
|
||||
let mut invalid = Node::new("invalid");
|
||||
invalid.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String("bogus".to_string()),
|
||||
);
|
||||
graph.nodes.insert("invalid".to_string(), invalid);
|
||||
let mut route = Node::new("route");
|
||||
route.attrs.insert(
|
||||
"on_failure".to_string(),
|
||||
AttrValue::String("route".to_string()),
|
||||
);
|
||||
graph.nodes.insert("route".to_string(), route);
|
||||
|
||||
// Node attribute wins over the graph policy.
|
||||
assert_eq!(graph.resolve_on_failure("route"), ResolvedOnFailure {
|
||||
policy: OnFailure::Route,
|
||||
scope: OnFailureScope::Node,
|
||||
});
|
||||
// Absent, invalid, and unknown nodes inherit the graph policy.
|
||||
for node_id in ["bare", "invalid", "missing"] {
|
||||
assert_eq!(graph.resolve_on_failure(node_id), ResolvedOnFailure {
|
||||
policy: OnFailure::Exit,
|
||||
scope: OnFailureScope::Graph,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn attr_value_as_str() {
|
||||
let val = AttrValue::String("hello".to_string());
|
||||
|
|
|
|||
|
|
@ -78,8 +78,8 @@ pub use event_envelope::EventEnvelope;
|
|||
pub use fabro_model::ReasoningEffort;
|
||||
pub use failure_signature::FailureSignature;
|
||||
pub use graph::{
|
||||
AttrValue, ContextKeyAttr, Edge, Graph, KNOWN_HANDLER_TYPES, Node, OnFailure,
|
||||
is_known_handler_type, is_llm_handler_type, shape_to_handler_type,
|
||||
AttrValue, ContextKeyAttr, Edge, Graph, KNOWN_HANDLER_TYPES, Node, OnFailure, OnFailureScope,
|
||||
ResolvedOnFailure, is_known_handler_type, is_llm_handler_type, shape_to_handler_type,
|
||||
};
|
||||
pub use input_scalar::{JsonScalarToTomlError, json_scalar_to_toml_value};
|
||||
pub use interview::{
|
||||
|
|
|
|||
7
test/on_failure_node_invalid.fabro
Normal file
7
test/on_failure_node_invalid.fabro
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
digraph InvalidNodeOnFailure {
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
work [prompt="Do the work" on_failure="stop"]
|
||||
|
||||
start -> work -> exit
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue