From 032b3c33e15e2623e4683fff9ff608d0bcd8a327 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 25 Mar 2026 00:10:40 -0400 Subject: [PATCH] Co-locate unit tests with extracted helper modules Move unit tests from engine.rs to their respective modules: - 72 tests to graph_ops.rs (retry policy, edge selection, fidelity, thread_id, etc.) - 5 tests to run_dir.rs (node_dir, visit_from_context) - 1 test to sandbox_git.rs (git_checkpoint_includes_builtin_excludes) Fix clippy needless_borrow in pipeline/finalize.rs. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-workflows/src/engine.rs | 1048 ----------------- lib/crates/fabro-workflows/src/graph_ops.rs | 945 +++++++++++++++ .../fabro-workflows/src/pipeline/finalize.rs | 2 +- lib/crates/fabro-workflows/src/run_dir.rs | 48 + lib/crates/fabro-workflows/src/sandbox_git.rs | 71 ++ 5 files changed, 1065 insertions(+), 1049 deletions(-) diff --git a/lib/crates/fabro-workflows/src/engine.rs b/lib/crates/fabro-workflows/src/engine.rs index a61362327..0e2f54f1f 100644 --- a/lib/crates/fabro-workflows/src/engine.rs +++ b/lib/crates/fabro-workflows/src/engine.rs @@ -16,13 +16,9 @@ use tokio_util::sync::CancellationToken; use crate::checkpoint::Checkpoint; use crate::context; use crate::context::Context; -#[cfg(test)] -use crate::error::FailureCategory; use crate::error::{FabroError, Result}; use crate::event::{EventEmitter, WorkflowRunEvent}; use crate::handler::{EngineServices, HandlerRegistry}; -#[cfg(test)] -use crate::outcome::OutcomeExt; use crate::outcome::{Outcome, StageStatus}; #[cfg(test)] use fabro_config::config::FabroConfig; @@ -32,8 +28,6 @@ use fabro_graphviz::graph::{Edge, Node}; use fabro_hooks::{HookContext, HookDecision, HookEvent, HookRunner}; use fabro_interview::Interviewer; -#[cfg(test)] -use crate::graph_ops::{best_by_weight_then_lexical, normalize_label, weighted_random}; pub(crate) use crate::graph_ops::{ build_retry_policy, check_goal_gates, classify_outcome, get_retry_target, is_terminal, node_script, set_hook_node, @@ -668,681 +662,6 @@ mod tests { } } - // --- RetryPolicy preset tests --- - - #[test] - fn retry_policy_none() { - let policy = RetryPolicy::none(); - assert_eq!(policy.max_attempts, 1); - } - - #[test] - fn retry_policy_standard() { - let policy = RetryPolicy::standard(); - assert_eq!(policy.max_attempts, 5); - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(5_000)); - } - - #[test] - fn retry_policy_aggressive() { - let policy = RetryPolicy::aggressive(); - assert_eq!(policy.max_attempts, 5); - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(500)); - } - - #[test] - fn retry_policy_linear() { - let policy = RetryPolicy::linear(); - assert_eq!(policy.max_attempts, 3); - assert_eq!(policy.backoff.factor, 1.0); - } - - #[test] - fn visit_from_context_defaults_to_first_visit() { - let ctx = Context::new(); - assert_eq!(visit_from_context(&ctx), 1); - } - - #[test] - fn visit_from_context_preserves_stored_visit() { - let ctx = Context::new(); - ctx.set( - crate::context::keys::INTERNAL_NODE_VISIT_COUNT, - serde_json::json!(3), - ); - assert_eq!(visit_from_context(&ctx), 3); - } - - #[test] - fn retry_policy_patient() { - let policy = RetryPolicy::patient(); - assert_eq!(policy.max_attempts, 3); - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(2000)); - } - - // --- build_retry_policy tests --- - - #[test] - fn build_retry_policy_from_node() { - let mut node = Node::new("n"); - node.attrs - .insert("max_retries".to_string(), AttrValue::Integer(3)); - let graph = Graph::new("test"); - let policy = build_retry_policy(&node, &graph); - assert_eq!(policy.max_attempts, 4); // 3 retries + 1 initial - } - - #[test] - fn build_retry_policy_from_graph_default() { - let node = Node::new("n"); - let mut graph = Graph::new("test"); - graph - .attrs - .insert("default_max_retries".to_string(), AttrValue::Integer(2)); - let policy = build_retry_policy(&node, &graph); - assert_eq!(policy.max_attempts, 3); // 2 retries + 1 initial - } - - #[test] - fn build_retry_policy_no_attrs_uses_graph_default_0() { - let node = Node::new("n"); - let graph = Graph::new("test"); - let policy = build_retry_policy(&node, &graph); - assert_eq!(policy.max_attempts, 1); // default_max_retries=0 + 1 - } - - #[test] - fn build_retry_policy_from_retry_policy_attr() { - let mut node = Node::new("n"); - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String("aggressive".to_string()), - ); - let graph = Graph::new("test"); - let policy = build_retry_policy(&node, &graph); - assert_eq!(policy.max_attempts, 5); - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(500)); - } - - #[test] - fn build_retry_policy_fallback_when_no_retry_policy_attr() { - let mut node = Node::new("n"); - node.attrs - .insert("max_retries".to_string(), AttrValue::Integer(3)); - let graph = Graph::new("test"); - let policy = build_retry_policy(&node, &graph); - assert_eq!(policy.max_attempts, 4); // 3 retries + 1 initial - // Should use default backoff, not a preset's backoff - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(5_000)); - } - - #[test] - fn build_retry_policy_all_presets() { - let presets = [ - ("none", 1u32), - ("standard", 5), - ("aggressive", 5), - ("linear", 3), - ("patient", 3), - ]; - let graph = Graph::new("test"); - let (name, expected) = presets[0]; - let mut node = Node::new("n"); - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String(name.to_string()), - ); - assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); - - let (name, expected) = presets[1]; - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String(name.to_string()), - ); - assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); - - let (name, expected) = presets[2]; - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String(name.to_string()), - ); - assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); - - let (name, expected) = presets[3]; - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String(name.to_string()), - ); - assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); - - let (name, expected) = presets[4]; - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String(name.to_string()), - ); - assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); - } - - #[test] - fn build_retry_policy_unknown_preset_falls_back() { - let mut node = Node::new("n"); - node.attrs.insert( - "retry_policy".to_string(), - AttrValue::String("unknown_preset".to_string()), - ); - let graph = Graph::new("test"); - let policy = build_retry_policy(&node, &graph); - // Unknown preset should fall back to graph default_max_retries=0 - assert_eq!(policy.max_attempts, 1); - } - - // --- normalize_label tests --- - - #[test] - fn normalize_label_lowercase_and_trim() { - assert_eq!(normalize_label(" Yes "), "yes"); - } - - #[test] - fn normalize_label_strip_bracket_prefix() { - assert_eq!(normalize_label("[A] Approve"), "approve"); - assert_eq!(normalize_label("[F] Fix"), "fix"); - } - - #[test] - fn normalize_label_strip_paren_prefix() { - assert_eq!(normalize_label("Y) Yes"), "yes"); - } - - #[test] - fn normalize_label_strip_dash_prefix() { - assert_eq!(normalize_label("Y - Yes"), "yes"); - } - - #[test] - fn normalize_label_plain() { - assert_eq!(normalize_label("next"), "next"); - } - - // --- best_by_weight_then_lexical tests --- - - #[test] - fn best_by_weight_highest_wins() { - let e1 = Edge::new("a", "x"); - let mut e2 = Edge::new("a", "y"); - e2.attrs.insert("weight".to_string(), AttrValue::Integer(5)); - let result = best_by_weight_then_lexical(&[&e1, &e2]).unwrap(); - assert_eq!(result.to, "y"); - } - - #[test] - fn best_by_weight_lexical_tiebreak() { - let e1 = Edge::new("a", "beta"); - let e2 = Edge::new("a", "alpha"); - let result = best_by_weight_then_lexical(&[&e1, &e2]).unwrap(); - assert_eq!(result.to, "alpha"); - } - - #[test] - fn best_by_weight_empty_returns_none() { - let result = best_by_weight_then_lexical(&[]); - assert!(result.is_none()); - } - - // --- weighted_random tests --- - - #[test] - fn weighted_random_empty_returns_none() { - assert!(weighted_random(&[]).is_none()); - } - - #[test] - fn weighted_random_single_edge() { - let e = Edge::new("a", "b"); - let result = weighted_random(&[&e]).unwrap(); - assert_eq!(result.to, "b"); - } - - #[test] - fn weighted_random_zero_weight_all_selected() { - let e1 = Edge::new("a", "b"); - let e2 = Edge::new("a", "c"); - let edges = vec![&e1, &e2]; - let mut seen_b = false; - let mut seen_c = false; - for _ in 0..200 { - let pick = weighted_random(&edges).unwrap(); - if pick.to == "b" { - seen_b = true; - } - if pick.to == "c" { - seen_c = true; - } - } - assert!(seen_b, "expected target 'b' to be selected at least once"); - assert!(seen_c, "expected target 'c' to be selected at least once"); - } - - #[test] - fn weighted_random_high_weight_dominates() { - let mut heavy = Edge::new("a", "heavy"); - heavy - .attrs - .insert("weight".to_string(), AttrValue::Integer(100)); - let mut light = Edge::new("a", "light"); - light - .attrs - .insert("weight".to_string(), AttrValue::Integer(1)); - let edges = vec![&heavy, &light]; - let mut heavy_count = 0; - for _ in 0..500 { - let pick = weighted_random(&edges).unwrap(); - if pick.to == "heavy" { - heavy_count += 1; - } - } - let ratio = heavy_count as f64 / 500.0; - assert!( - ratio > 0.90, - "expected heavy edge to win >90% of the time, got {ratio:.2}" - ); - } - - // --- select_edge tests --- - - fn make_graph_with_edges(edges: Vec) -> Graph { - let mut g = Graph::new("test"); - for edge in &edges { - if !g.nodes.contains_key(&edge.from) { - g.nodes.insert(edge.from.clone(), Node::new(&edge.from)); - } - if !g.nodes.contains_key(&edge.to) { - g.nodes.insert(edge.to.clone(), Node::new(&edge.to)); - } - } - g.edges = edges; - g - } - - #[test] - fn select_edge_no_edges() { - let g = Graph::new("test"); - let node = Node::new("a"); - let outcome = Outcome::success(); - let context = Context::new(); - assert!(select_edge(&node, &outcome, &context, &g, "deterministic").is_none()); - } - - #[test] - fn select_edge_single_unconditional() { - let g = make_graph_with_edges(vec![Edge::new("a", "b")]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "b"); - assert_eq!(sel.reason, "unconditional"); - } - - #[test] - fn select_edge_condition_match() { - let mut e1 = Edge::new("a", "fail_path"); - e1.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=fail".to_string()), - ); - let mut e2 = Edge::new("a", "success_path"); - e2.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=success".to_string()), - ); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "success_path"); - assert_eq!(sel.reason, "condition"); - } - - #[test] - fn select_edge_preferred_label() { - let mut e1 = Edge::new("a", "approve"); - e1.attrs.insert( - "label".to_string(), - AttrValue::String("[A] Approve".to_string()), - ); - let mut e2 = Edge::new("a", "fix"); - e2.attrs.insert( - "label".to_string(), - AttrValue::String("[F] Fix".to_string()), - ); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let mut outcome = Outcome::success(); - outcome.preferred_label = Some("Fix".to_string()); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "fix"); - assert_eq!(sel.reason, "preferred_label"); - } - - #[test] - fn select_edge_suggested_next_ids() { - let e1 = Edge::new("a", "path1"); - let e2 = Edge::new("a", "path2"); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let mut outcome = Outcome::success(); - outcome.suggested_next_ids = vec!["path2".to_string()]; - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "path2"); - assert_eq!(sel.reason, "suggested_next"); - } - - #[test] - fn select_edge_weight_tiebreak() { - let mut e1 = Edge::new("a", "low"); - e1.attrs.insert("weight".to_string(), AttrValue::Integer(1)); - let mut e2 = Edge::new("a", "high"); - e2.attrs - .insert("weight".to_string(), AttrValue::Integer(10)); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "high"); - assert_eq!(sel.reason, "unconditional"); - } - - #[test] - fn select_edge_lexical_tiebreak() { - let e1 = Edge::new("a", "charlie"); - let e2 = Edge::new("a", "alpha"); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "alpha"); - assert_eq!(sel.reason, "unconditional"); - } - - #[test] - fn select_edge_condition_beats_unconditional() { - let mut e_cond = Edge::new("a", "cond_path"); - e_cond.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=success".to_string()), - ); - let e_uncond = Edge::new("a", "uncond_path"); - let g = make_graph_with_edges(vec![e_cond, e_uncond]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "cond_path"); - assert_eq!(sel.reason, "condition"); - } - - #[test] - fn select_edge_random_returns_some_edge() { - let e1 = Edge::new("a", "b"); - let e2 = Edge::new("a", "c"); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "random").unwrap(); - assert!(sel.edge.to == "b" || sel.edge.to == "c"); - assert_eq!(sel.reason, "unconditional"); - } - - #[test] - fn select_edge_random_preferred_label_still_wins() { - let mut e1 = Edge::new("a", "approve"); - e1.attrs.insert( - "label".to_string(), - AttrValue::String("Approve".to_string()), - ); - let e2 = Edge::new("a", "other"); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let mut outcome = Outcome::success(); - outcome.preferred_label = Some("Approve".to_string()); - let context = Context::new(); - let sel = select_edge(node, &outcome, &context, &g, "random").unwrap(); - assert_eq!(sel.edge.to, "approve"); - assert_eq!(sel.reason, "preferred_label"); - } - - #[test] - fn select_edge_failed_human_gate_does_not_fall_through_to_unconditional() { - let g = make_graph_with_edges(vec![ - Edge::new("gate", "approve"), - Edge::new("gate", "skip"), - ]); - let mut node = g.nodes.get("gate").unwrap().clone(); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("hexagon".to_string()), - ); - let outcome = - Outcome::fail_deterministic("human interaction aborted before an answer was provided"); - let context = Context::new(); - - assert!(select_edge(&node, &outcome, &context, &g, "deterministic").is_none()); - } - - #[test] - fn select_edge_failed_human_gate_routes_via_fail_condition() { - let mut fail = Edge::new("gate", "retry"); - fail.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=fail".to_string()), - ); - let approve = Edge::new("gate", "approve"); - let g = make_graph_with_edges(vec![fail, approve]); - let mut node = g.nodes.get("gate").unwrap().clone(); - node.attrs.insert( - "shape".to_string(), - AttrValue::String("hexagon".to_string()), - ); - let outcome = - Outcome::fail_deterministic("human interaction aborted before an answer was provided"); - let context = Context::new(); - - let sel = select_edge(&node, &outcome, &context, &g, "deterministic").unwrap(); - assert_eq!(sel.edge.to, "retry"); - assert_eq!(sel.reason, "condition"); - } - - #[test] - fn select_edge_deterministic_no_fallback_when_no_condition_matches() { - let mut e1 = Edge::new("a", "path1"); - e1.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=fail".to_string()), - ); - let mut e2 = Edge::new("a", "path2"); - e2.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=error".to_string()), - ); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - assert!(select_edge(node, &outcome, &context, &g, "deterministic").is_none()); - } - - #[test] - fn select_edge_random_no_fallback_when_no_condition_matches() { - let mut e1 = Edge::new("a", "path1"); - e1.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=fail".to_string()), - ); - let mut e2 = Edge::new("a", "path2"); - e2.attrs.insert( - "condition".to_string(), - AttrValue::String("outcome=error".to_string()), - ); - let g = make_graph_with_edges(vec![e1, e2]); - let node = g.nodes.get("a").unwrap(); - let outcome = Outcome::success(); - let context = Context::new(); - assert!(select_edge(node, &outcome, &context, &g, "random").is_none()); - } - - // --- check_goal_gates tests --- - - #[test] - fn goal_gates_all_satisfied() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs - .insert("goal_gate".to_string(), AttrValue::Boolean(true)); - g.nodes.insert("work".to_string(), n); - - let mut outcomes = HashMap::new(); - outcomes.insert("work".to_string(), Outcome::success()); - - assert!(check_goal_gates(&g, &outcomes).is_ok()); - } - - #[test] - fn goal_gates_partial_success_counts() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs - .insert("goal_gate".to_string(), AttrValue::Boolean(true)); - g.nodes.insert("work".to_string(), n); - - let mut outcomes = HashMap::new(); - let mut o = Outcome::success(); - o.status = StageStatus::PartialSuccess; - outcomes.insert("work".to_string(), o); - - assert!(check_goal_gates(&g, &outcomes).is_ok()); - } - - #[test] - fn goal_gates_failed_returns_node_id() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs - .insert("goal_gate".to_string(), AttrValue::Boolean(true)); - g.nodes.insert("work".to_string(), n); - - let mut outcomes = HashMap::new(); - outcomes.insert("work".to_string(), Outcome::fail_classify("test")); - - assert_eq!(check_goal_gates(&g, &outcomes), Err("work".to_string())); - } - - #[test] - fn goal_gates_non_gate_nodes_ignored() { - let mut g = Graph::new("test"); - g.nodes.insert("work".to_string(), Node::new("work")); - - let mut outcomes = HashMap::new(); - outcomes.insert("work".to_string(), Outcome::fail_classify("test")); - - assert!(check_goal_gates(&g, &outcomes).is_ok()); - } - - // --- get_retry_target tests --- - - #[test] - fn retry_target_from_node() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs.insert( - "retry_target".to_string(), - AttrValue::String("plan".to_string()), - ); - g.nodes.insert("work".to_string(), n); - g.nodes.insert("plan".to_string(), Node::new("plan")); - - assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); - } - - #[test] - fn retry_target_from_fallback() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs.insert( - "fallback_retry_target".to_string(), - AttrValue::String("plan".to_string()), - ); - g.nodes.insert("work".to_string(), n); - g.nodes.insert("plan".to_string(), Node::new("plan")); - - assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); - } - - #[test] - fn retry_target_from_graph() { - let mut g = Graph::new("test"); - g.nodes.insert("work".to_string(), Node::new("work")); - g.nodes.insert("plan".to_string(), Node::new("plan")); - g.attrs.insert( - "retry_target".to_string(), - AttrValue::String("plan".to_string()), - ); - - assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); - } - - #[test] - fn retry_target_none_when_missing() { - let mut g = Graph::new("test"); - g.nodes.insert("work".to_string(), Node::new("work")); - assert!(get_retry_target("work", &g).is_none()); - } - - #[test] - fn retry_target_skips_nonexistent_node() { - let mut g = Graph::new("test"); - let mut n = Node::new("work"); - n.attrs.insert( - "retry_target".to_string(), - AttrValue::String("nonexistent".to_string()), - ); - g.nodes.insert("work".to_string(), n); - // No "nonexistent" node -- should fall through to graph-level - assert!(get_retry_target("work", &g).is_none()); - } - - // --- is_terminal tests --- - - #[test] - fn terminal_by_shape() { - let mut n = Node::new("exit"); - n.attrs.insert( - "shape".to_string(), - AttrValue::String("Msquare".to_string()), - ); - assert!(is_terminal(&n)); - } - - #[test] - fn terminal_by_type() { - let mut n = Node::new("end"); - n.attrs - .insert("type".to_string(), AttrValue::String("exit".to_string())); - assert!(is_terminal(&n)); - } - - #[test] - fn non_terminal_node() { - let n = Node::new("work"); - assert!(!is_terminal(&n)); - } - // --- WorkflowRunEngine integration tests --- fn simple_graph() -> Graph { @@ -1635,64 +954,6 @@ mod tests { assert!(!cp.completed_nodes.contains(&"path_a".to_string())); } - // --- resolve_fidelity tests --- - - #[test] - fn fidelity_defaults_to_compact() { - use crate::context::keys::Fidelity; - let node = Node::new("work"); - let graph = Graph::new("test"); - assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Compact); - } - - #[test] - fn fidelity_from_graph_default() { - use crate::context::keys::Fidelity; - let node = Node::new("work"); - let mut graph = Graph::new("test"); - graph.attrs.insert( - "default_fidelity".to_string(), - AttrValue::String("truncate".to_string()), - ); - assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Truncate); - } - - #[test] - fn fidelity_from_node_overrides_graph() { - use crate::context::keys::Fidelity; - let mut node = Node::new("work"); - node.attrs.insert( - "fidelity".to_string(), - AttrValue::String("full".to_string()), - ); - let mut graph = Graph::new("test"); - graph.attrs.insert( - "default_fidelity".to_string(), - AttrValue::String("truncate".to_string()), - ); - assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Full); - } - - #[test] - fn fidelity_from_edge_overrides_node() { - use crate::context::keys::Fidelity; - let mut node = Node::new("work"); - node.attrs.insert( - "fidelity".to_string(), - AttrValue::String("full".to_string()), - ); - let mut edge = Edge::new("a", "work"); - edge.attrs.insert( - "fidelity".to_string(), - AttrValue::String("summary:high".to_string()), - ); - let graph = Graph::new("test"); - assert_eq!( - resolve_fidelity(Some(&edge), &node, &graph), - Fidelity::SummaryHigh - ); - } - // --- start.json and node status tests --- #[tokio::test] @@ -1848,151 +1109,6 @@ mod tests { ); } - // --- resolve_thread_id tests --- - - #[test] - fn thread_id_from_node_attribute() { - let mut node = Node::new("work"); - node.attrs.insert( - "thread_id".to_string(), - AttrValue::String("main-thread".to_string()), - ); - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(None, &node, &graph, Some("prev")), - Some("main-thread".to_string()) - ); - } - - #[test] - fn thread_id_from_edge_attribute() { - let node = Node::new("work"); - let mut edge = Edge::new("prev", "work"); - edge.attrs.insert( - "thread_id".to_string(), - AttrValue::String("edge-thread".to_string()), - ); - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), - Some("edge-thread".to_string()) - ); - } - - #[test] - fn thread_id_node_used_when_no_edge_thread() { - // When the edge has no thread_id, the node's thread_id is used. - let mut node = Node::new("work"); - node.attrs.insert( - "thread_id".to_string(), - AttrValue::String("node-thread".to_string()), - ); - let edge = Edge::new("prev", "work"); - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), - Some("node-thread".to_string()) - ); - } - - #[test] - fn thread_id_edge_overrides_node() { - // Edge thread_id should take precedence over node thread_id, - // matching the fidelity precedence where edge > node. - let mut node = Node::new("work"); - node.attrs.insert( - "thread_id".to_string(), - AttrValue::String("node-thread".to_string()), - ); - let mut edge = Edge::new("prev", "work"); - edge.attrs.insert( - "thread_id".to_string(), - AttrValue::String("edge-thread".to_string()), - ); - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), - Some("edge-thread".to_string()), - "edge thread_id should override node thread_id" - ); - } - - #[test] - fn thread_id_from_graph_default_thread() { - let node = Node::new("work"); - let mut graph = Graph::new("test"); - graph.attrs.insert( - "default_thread".to_string(), - AttrValue::String("shared-thread".to_string()), - ); - assert_eq!( - resolve_thread_id(None, &node, &graph, Some("prev")), - Some("shared-thread".to_string()) - ); - } - - #[test] - fn thread_id_edge_overrides_graph_default() { - let node = Node::new("work"); - let mut edge = Edge::new("prev", "work"); - edge.attrs.insert( - "thread_id".to_string(), - AttrValue::String("edge-thread".to_string()), - ); - let mut graph = Graph::new("test"); - graph.attrs.insert( - "default_thread".to_string(), - AttrValue::String("shared-thread".to_string()), - ); - assert_eq!( - resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), - Some("edge-thread".to_string()) - ); - } - - #[test] - fn thread_id_graph_default_overrides_class() { - let mut node = Node::new("work"); - node.classes = vec!["planning".to_string()]; - let mut graph = Graph::new("test"); - graph.attrs.insert( - "default_thread".to_string(), - AttrValue::String("shared-thread".to_string()), - ); - assert_eq!( - resolve_thread_id(None, &node, &graph, Some("prev")), - Some("shared-thread".to_string()) - ); - } - - #[test] - fn thread_id_from_node_class() { - let mut node = Node::new("work"); - node.classes = vec!["planning".to_string(), "review".to_string()]; - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(None, &node, &graph, Some("prev")), - Some("planning".to_string()) - ); - } - - #[test] - fn thread_id_fallback_to_previous_node() { - let node = Node::new("work"); - let graph = Graph::new("test"); - assert_eq!( - resolve_thread_id(None, &node, &graph, Some("prev_node")), - Some("prev_node".to_string()) - ); - } - - #[test] - fn thread_id_none_when_no_sources() { - let node = Node::new("start"); - let graph = Graph::new("test"); - assert_eq!(resolve_thread_id(None, &node, &graph, None), None); - } - // --- Gap #15: StartRecord run_id field test --- #[tokio::test] @@ -2746,32 +1862,6 @@ mod tests { ); } - // --- node_dir visit-count tests --- - - #[test] - fn node_dir_first_visit() { - let root = Path::new("/tmp/logs"); - assert_eq!(node_dir(root, "work", 1), root.join("nodes").join("work")); - } - - #[test] - fn node_dir_second_visit() { - let root = Path::new("/tmp/logs"); - assert_eq!( - node_dir(root, "work", 2), - root.join("nodes").join("work-visit_2") - ); - } - - #[test] - fn node_dir_fifth_visit() { - let root = Path::new("/tmp/logs"); - assert_eq!( - node_dir(root, "work", 5), - root.join("nodes").join("work-visit_5") - ); - } - // --- panic.txt tests --- /// Handler that always panics. @@ -2844,78 +1934,6 @@ mod tests { ); } - // --- classify_outcome tests --- - - #[test] - fn classify_outcome_returns_none_for_success() { - assert!(classify_outcome(&Outcome::success()).is_none()); - } - - #[test] - fn classify_outcome_returns_none_for_skipped() { - assert!(classify_outcome(&Outcome::skipped("")).is_none()); - } - - #[test] - fn classify_outcome_returns_none_for_partial_success() { - let outcome = Outcome { - status: StageStatus::PartialSuccess, - ..Outcome::success() - }; - assert!(classify_outcome(&outcome).is_none()); - } - - #[test] - fn classify_outcome_reads_failure_detail() { - let mut outcome = Outcome::fail_classify("some error"); - // Override the FailureDetail's class directly - outcome.failure.as_mut().unwrap().category = FailureCategory::BudgetExhausted; - assert_eq!( - classify_outcome(&outcome), - Some(FailureCategory::BudgetExhausted) - ); - } - - #[test] - fn classify_outcome_uses_failure_reason_heuristics() { - let outcome = Outcome::fail_classify("rate limited by provider"); - assert_eq!( - classify_outcome(&outcome), - Some(FailureCategory::TransientInfra) - ); - } - - #[test] - fn classify_outcome_defaults_to_deterministic() { - let outcome = Outcome::fail_classify("something went wrong"); - assert_eq!( - classify_outcome(&outcome), - Some(FailureCategory::Deterministic) - ); - } - - #[test] - fn classify_outcome_fail_no_reason_is_deterministic() { - let outcome = Outcome { - status: StageStatus::Fail, - failure: None, - ..Outcome::success() - }; - assert_eq!( - classify_outcome(&outcome), - Some(FailureCategory::Deterministic) - ); - } - - #[test] - fn classify_outcome_retry_status_uses_heuristics() { - let outcome = Outcome::retry_classify("connection refused"); - assert_eq!( - classify_outcome(&outcome), - Some(FailureCategory::TransientInfra) - ); - } - // --- Circuit breaker tests --- /// Build a graph where `work` always fails deterministically, @@ -3493,72 +2511,6 @@ mod tests { ); } - #[tokio::test] - async fn git_checkpoint_includes_builtin_excludes() { - // Set up a real git repo - let repo_dir = tempfile::tempdir().unwrap(); - let repo = repo_dir.path(); - std::process::Command::new("git") - .args(["init"]) - .current_dir(repo) - .output() - .unwrap(); - std::process::Command::new("git") - .args([ - "-c", - "user.name=Test", - "-c", - "user.email=test@test.com", - "commit", - "--allow-empty", - "-m", - "initial", - ]) - .current_dir(repo) - .output() - .unwrap(); - - // Create files in both tracked and excluded directories - std::fs::write(repo.join("hello.txt"), "hello").unwrap(); - std::fs::create_dir_all(repo.join("node_modules/pkg")).unwrap(); - std::fs::write(repo.join("node_modules/pkg/index.js"), "module").unwrap(); - std::fs::create_dir_all(repo.join(".venv/lib")).unwrap(); - std::fs::write(repo.join(".venv/lib/site.py"), "venv").unwrap(); - - let sandbox = fabro_agent::LocalSandbox::new(repo.to_path_buf()); - let author = crate::git::GitAuthor::default(); - - // Call git_checkpoint with empty user excludes — built-in excludes should still apply - let result = - git_checkpoint(&sandbox, "run1", "work", "success", 1, None, &[], &author).await; - assert!(result.is_ok(), "git_checkpoint failed: {:?}", result.err()); - - // Verify that excluded directories were NOT staged - let status = sandbox - .exec_command( - "git diff --cached --name-only HEAD~1", - 10_000, - None, - None, - None, - ) - .await - .unwrap(); - let staged_files: Vec<&str> = status.stdout.lines().collect(); - assert!( - staged_files.contains(&"hello.txt"), - "expected hello.txt to be staged, got: {staged_files:?}" - ); - assert!( - !staged_files.iter().any(|f| f.contains("node_modules")), - "node_modules should be excluded from checkpoint, got: {staged_files:?}" - ); - assert!( - !staged_files.iter().any(|f| f.contains(".venv")), - ".venv should be excluded from checkpoint, got: {staged_files:?}" - ); - } - #[tokio::test] async fn git_checkpoint_skipped_for_start_node() { // Set up a real git repo for checkpoint testing diff --git a/lib/crates/fabro-workflows/src/graph_ops.rs b/lib/crates/fabro-workflows/src/graph_ops.rs index 16c8b81b5..fb323887a 100644 --- a/lib/crates/fabro-workflows/src/graph_ops.rs +++ b/lib/crates/fabro-workflows/src/graph_ops.rs @@ -422,3 +422,948 @@ pub(crate) fn node_script(node: &Node) -> Option { .and_then(|v| v.as_str()) .map(String::from) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::Context; + use crate::error::FailureCategory; + use crate::outcome::{Outcome, OutcomeExt, StageStatus}; + use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; + use std::collections::HashMap; + use std::time::Duration; + + // --- RetryPolicy preset tests --- + + #[test] + fn retry_policy_none() { + let policy = RetryPolicy::none(); + assert_eq!(policy.max_attempts, 1); + } + + #[test] + fn retry_policy_standard() { + let policy = RetryPolicy::standard(); + assert_eq!(policy.max_attempts, 5); + assert_eq!(policy.backoff.initial_delay, Duration::from_millis(5_000)); + } + + #[test] + fn retry_policy_aggressive() { + let policy = RetryPolicy::aggressive(); + assert_eq!(policy.max_attempts, 5); + assert_eq!(policy.backoff.initial_delay, Duration::from_millis(500)); + } + + #[test] + fn retry_policy_linear() { + let policy = RetryPolicy::linear(); + assert_eq!(policy.max_attempts, 3); + assert_eq!(policy.backoff.factor, 1.0); + } + + #[test] + fn retry_policy_patient() { + let policy = RetryPolicy::patient(); + assert_eq!(policy.max_attempts, 3); + assert_eq!(policy.backoff.initial_delay, Duration::from_millis(2000)); + } + + // --- build_retry_policy tests --- + + #[test] + fn build_retry_policy_from_node() { + let mut node = Node::new("n"); + node.attrs + .insert("max_retries".to_string(), AttrValue::Integer(3)); + let graph = Graph::new("test"); + let policy = build_retry_policy(&node, &graph); + assert_eq!(policy.max_attempts, 4); // 3 retries + 1 initial + } + + #[test] + fn build_retry_policy_from_graph_default() { + let node = Node::new("n"); + let mut graph = Graph::new("test"); + graph + .attrs + .insert("default_max_retries".to_string(), AttrValue::Integer(2)); + let policy = build_retry_policy(&node, &graph); + assert_eq!(policy.max_attempts, 3); // 2 retries + 1 initial + } + + #[test] + fn build_retry_policy_no_attrs_uses_graph_default_0() { + let node = Node::new("n"); + let graph = Graph::new("test"); + let policy = build_retry_policy(&node, &graph); + assert_eq!(policy.max_attempts, 1); // default_max_retries=0 + 1 + } + + #[test] + fn build_retry_policy_from_retry_policy_attr() { + let mut node = Node::new("n"); + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String("aggressive".to_string()), + ); + let graph = Graph::new("test"); + let policy = build_retry_policy(&node, &graph); + assert_eq!(policy.max_attempts, 5); + assert_eq!(policy.backoff.initial_delay, Duration::from_millis(500)); + } + + #[test] + fn build_retry_policy_fallback_when_no_retry_policy_attr() { + let mut node = Node::new("n"); + node.attrs + .insert("max_retries".to_string(), AttrValue::Integer(3)); + let graph = Graph::new("test"); + let policy = build_retry_policy(&node, &graph); + assert_eq!(policy.max_attempts, 4); // 3 retries + 1 initial + // Should use default backoff, not a preset's backoff + assert_eq!(policy.backoff.initial_delay, Duration::from_millis(5_000)); + } + + #[test] + fn build_retry_policy_all_presets() { + let presets = [ + ("none", 1u32), + ("standard", 5), + ("aggressive", 5), + ("linear", 3), + ("patient", 3), + ]; + let graph = Graph::new("test"); + let (name, expected) = presets[0]; + let mut node = Node::new("n"); + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String(name.to_string()), + ); + assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); + + let (name, expected) = presets[1]; + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String(name.to_string()), + ); + assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); + + let (name, expected) = presets[2]; + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String(name.to_string()), + ); + assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); + + let (name, expected) = presets[3]; + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String(name.to_string()), + ); + assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); + + let (name, expected) = presets[4]; + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String(name.to_string()), + ); + assert_eq!(build_retry_policy(&node, &graph).max_attempts, expected); + } + + #[test] + fn build_retry_policy_unknown_preset_falls_back() { + let mut node = Node::new("n"); + node.attrs.insert( + "retry_policy".to_string(), + AttrValue::String("unknown_preset".to_string()), + ); + let graph = Graph::new("test"); + let policy = build_retry_policy(&node, &graph); + // Unknown preset should fall back to graph default_max_retries=0 + assert_eq!(policy.max_attempts, 1); + } + + // --- normalize_label tests --- + + #[test] + fn normalize_label_lowercase_and_trim() { + assert_eq!(normalize_label(" Yes "), "yes"); + } + + #[test] + fn normalize_label_strip_bracket_prefix() { + assert_eq!(normalize_label("[A] Approve"), "approve"); + assert_eq!(normalize_label("[F] Fix"), "fix"); + } + + #[test] + fn normalize_label_strip_paren_prefix() { + assert_eq!(normalize_label("Y) Yes"), "yes"); + } + + #[test] + fn normalize_label_strip_dash_prefix() { + assert_eq!(normalize_label("Y - Yes"), "yes"); + } + + #[test] + fn normalize_label_plain() { + assert_eq!(normalize_label("next"), "next"); + } + + // --- best_by_weight_then_lexical tests --- + + #[test] + fn best_by_weight_highest_wins() { + let e1 = Edge::new("a", "x"); + let mut e2 = Edge::new("a", "y"); + e2.attrs.insert("weight".to_string(), AttrValue::Integer(5)); + let result = best_by_weight_then_lexical(&[&e1, &e2]).unwrap(); + assert_eq!(result.to, "y"); + } + + #[test] + fn best_by_weight_lexical_tiebreak() { + let e1 = Edge::new("a", "beta"); + let e2 = Edge::new("a", "alpha"); + let result = best_by_weight_then_lexical(&[&e1, &e2]).unwrap(); + assert_eq!(result.to, "alpha"); + } + + #[test] + fn best_by_weight_empty_returns_none() { + let result = best_by_weight_then_lexical(&[]); + assert!(result.is_none()); + } + + // --- weighted_random tests --- + + #[test] + fn weighted_random_empty_returns_none() { + assert!(weighted_random(&[]).is_none()); + } + + #[test] + fn weighted_random_single_edge() { + let e = Edge::new("a", "b"); + let result = weighted_random(&[&e]).unwrap(); + assert_eq!(result.to, "b"); + } + + #[test] + fn weighted_random_zero_weight_all_selected() { + let e1 = Edge::new("a", "b"); + let e2 = Edge::new("a", "c"); + let edges = vec![&e1, &e2]; + let mut seen_b = false; + let mut seen_c = false; + for _ in 0..200 { + let pick = weighted_random(&edges).unwrap(); + if pick.to == "b" { + seen_b = true; + } + if pick.to == "c" { + seen_c = true; + } + } + assert!(seen_b, "expected target 'b' to be selected at least once"); + assert!(seen_c, "expected target 'c' to be selected at least once"); + } + + #[test] + fn weighted_random_high_weight_dominates() { + let mut heavy = Edge::new("a", "heavy"); + heavy + .attrs + .insert("weight".to_string(), AttrValue::Integer(100)); + let mut light = Edge::new("a", "light"); + light + .attrs + .insert("weight".to_string(), AttrValue::Integer(1)); + let edges = vec![&heavy, &light]; + let mut heavy_count = 0; + for _ in 0..500 { + let pick = weighted_random(&edges).unwrap(); + if pick.to == "heavy" { + heavy_count += 1; + } + } + let ratio = heavy_count as f64 / 500.0; + assert!( + ratio > 0.90, + "expected heavy edge to win >90% of the time, got {ratio:.2}" + ); + } + + // --- select_edge tests --- + + fn make_graph_with_edges(edges: Vec) -> Graph { + let mut g = Graph::new("test"); + for edge in &edges { + if !g.nodes.contains_key(&edge.from) { + g.nodes.insert(edge.from.clone(), Node::new(&edge.from)); + } + if !g.nodes.contains_key(&edge.to) { + g.nodes.insert(edge.to.clone(), Node::new(&edge.to)); + } + } + g.edges = edges; + g + } + + #[test] + fn select_edge_no_edges() { + let g = Graph::new("test"); + let node = Node::new("a"); + let outcome = Outcome::success(); + let context = Context::new(); + assert!(select_edge(&node, &outcome, &context, &g, "deterministic").is_none()); + } + + #[test] + fn select_edge_single_unconditional() { + let g = make_graph_with_edges(vec![Edge::new("a", "b")]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "b"); + assert_eq!(sel.reason, "unconditional"); + } + + #[test] + fn select_edge_condition_match() { + let mut e1 = Edge::new("a", "fail_path"); + e1.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=fail".to_string()), + ); + let mut e2 = Edge::new("a", "success_path"); + e2.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=success".to_string()), + ); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "success_path"); + assert_eq!(sel.reason, "condition"); + } + + #[test] + fn select_edge_preferred_label() { + let mut e1 = Edge::new("a", "approve"); + e1.attrs.insert( + "label".to_string(), + AttrValue::String("[A] Approve".to_string()), + ); + let mut e2 = Edge::new("a", "fix"); + e2.attrs.insert( + "label".to_string(), + AttrValue::String("[F] Fix".to_string()), + ); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let mut outcome = Outcome::success(); + outcome.preferred_label = Some("Fix".to_string()); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "fix"); + assert_eq!(sel.reason, "preferred_label"); + } + + #[test] + fn select_edge_suggested_next_ids() { + let e1 = Edge::new("a", "path1"); + let e2 = Edge::new("a", "path2"); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let mut outcome = Outcome::success(); + outcome.suggested_next_ids = vec!["path2".to_string()]; + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "path2"); + assert_eq!(sel.reason, "suggested_next"); + } + + #[test] + fn select_edge_weight_tiebreak() { + let mut e1 = Edge::new("a", "low"); + e1.attrs.insert("weight".to_string(), AttrValue::Integer(1)); + let mut e2 = Edge::new("a", "high"); + e2.attrs + .insert("weight".to_string(), AttrValue::Integer(10)); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "high"); + assert_eq!(sel.reason, "unconditional"); + } + + #[test] + fn select_edge_lexical_tiebreak() { + let e1 = Edge::new("a", "charlie"); + let e2 = Edge::new("a", "alpha"); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "alpha"); + assert_eq!(sel.reason, "unconditional"); + } + + #[test] + fn select_edge_condition_beats_unconditional() { + let mut e_cond = Edge::new("a", "cond_path"); + e_cond.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=success".to_string()), + ); + let e_uncond = Edge::new("a", "uncond_path"); + let g = make_graph_with_edges(vec![e_cond, e_uncond]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "cond_path"); + assert_eq!(sel.reason, "condition"); + } + + #[test] + fn select_edge_random_returns_some_edge() { + let e1 = Edge::new("a", "b"); + let e2 = Edge::new("a", "c"); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "random").unwrap(); + assert!(sel.edge.to == "b" || sel.edge.to == "c"); + assert_eq!(sel.reason, "unconditional"); + } + + #[test] + fn select_edge_random_preferred_label_still_wins() { + let mut e1 = Edge::new("a", "approve"); + e1.attrs.insert( + "label".to_string(), + AttrValue::String("Approve".to_string()), + ); + let e2 = Edge::new("a", "other"); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let mut outcome = Outcome::success(); + outcome.preferred_label = Some("Approve".to_string()); + let context = Context::new(); + let sel = select_edge(node, &outcome, &context, &g, "random").unwrap(); + assert_eq!(sel.edge.to, "approve"); + assert_eq!(sel.reason, "preferred_label"); + } + + #[test] + fn select_edge_failed_human_gate_does_not_fall_through_to_unconditional() { + let g = make_graph_with_edges(vec![ + Edge::new("gate", "approve"), + Edge::new("gate", "skip"), + ]); + let mut node = g.nodes.get("gate").unwrap().clone(); + node.attrs.insert( + "shape".to_string(), + AttrValue::String("hexagon".to_string()), + ); + let outcome = + Outcome::fail_deterministic("human interaction aborted before an answer was provided"); + let context = Context::new(); + + assert!(select_edge(&node, &outcome, &context, &g, "deterministic").is_none()); + } + + #[test] + fn select_edge_failed_human_gate_routes_via_fail_condition() { + let mut fail = Edge::new("gate", "retry"); + fail.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=fail".to_string()), + ); + let approve = Edge::new("gate", "approve"); + let g = make_graph_with_edges(vec![fail, approve]); + let mut node = g.nodes.get("gate").unwrap().clone(); + node.attrs.insert( + "shape".to_string(), + AttrValue::String("hexagon".to_string()), + ); + let outcome = + Outcome::fail_deterministic("human interaction aborted before an answer was provided"); + let context = Context::new(); + + let sel = select_edge(&node, &outcome, &context, &g, "deterministic").unwrap(); + assert_eq!(sel.edge.to, "retry"); + assert_eq!(sel.reason, "condition"); + } + + #[test] + fn select_edge_deterministic_no_fallback_when_no_condition_matches() { + let mut e1 = Edge::new("a", "path1"); + e1.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=fail".to_string()), + ); + let mut e2 = Edge::new("a", "path2"); + e2.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=error".to_string()), + ); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + assert!(select_edge(node, &outcome, &context, &g, "deterministic").is_none()); + } + + #[test] + fn select_edge_random_no_fallback_when_no_condition_matches() { + let mut e1 = Edge::new("a", "path1"); + e1.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=fail".to_string()), + ); + let mut e2 = Edge::new("a", "path2"); + e2.attrs.insert( + "condition".to_string(), + AttrValue::String("outcome=error".to_string()), + ); + let g = make_graph_with_edges(vec![e1, e2]); + let node = g.nodes.get("a").unwrap(); + let outcome = Outcome::success(); + let context = Context::new(); + assert!(select_edge(node, &outcome, &context, &g, "random").is_none()); + } + + // --- check_goal_gates tests --- + + #[test] + fn goal_gates_all_satisfied() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs + .insert("goal_gate".to_string(), AttrValue::Boolean(true)); + g.nodes.insert("work".to_string(), n); + + let mut outcomes = HashMap::new(); + outcomes.insert("work".to_string(), Outcome::success()); + + assert!(check_goal_gates(&g, &outcomes).is_ok()); + } + + #[test] + fn goal_gates_partial_success_counts() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs + .insert("goal_gate".to_string(), AttrValue::Boolean(true)); + g.nodes.insert("work".to_string(), n); + + let mut outcomes = HashMap::new(); + let mut o = Outcome::success(); + o.status = StageStatus::PartialSuccess; + outcomes.insert("work".to_string(), o); + + assert!(check_goal_gates(&g, &outcomes).is_ok()); + } + + #[test] + fn goal_gates_failed_returns_node_id() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs + .insert("goal_gate".to_string(), AttrValue::Boolean(true)); + g.nodes.insert("work".to_string(), n); + + let mut outcomes = HashMap::new(); + outcomes.insert("work".to_string(), Outcome::fail_classify("test")); + + assert_eq!(check_goal_gates(&g, &outcomes), Err("work".to_string())); + } + + #[test] + fn goal_gates_non_gate_nodes_ignored() { + let mut g = Graph::new("test"); + g.nodes.insert("work".to_string(), Node::new("work")); + + let mut outcomes = HashMap::new(); + outcomes.insert("work".to_string(), Outcome::fail_classify("test")); + + assert!(check_goal_gates(&g, &outcomes).is_ok()); + } + + // --- get_retry_target tests --- + + #[test] + fn retry_target_from_node() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs.insert( + "retry_target".to_string(), + AttrValue::String("plan".to_string()), + ); + g.nodes.insert("work".to_string(), n); + g.nodes.insert("plan".to_string(), Node::new("plan")); + + assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); + } + + #[test] + fn retry_target_from_fallback() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs.insert( + "fallback_retry_target".to_string(), + AttrValue::String("plan".to_string()), + ); + g.nodes.insert("work".to_string(), n); + g.nodes.insert("plan".to_string(), Node::new("plan")); + + assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); + } + + #[test] + fn retry_target_from_graph() { + let mut g = Graph::new("test"); + g.nodes.insert("work".to_string(), Node::new("work")); + g.nodes.insert("plan".to_string(), Node::new("plan")); + g.attrs.insert( + "retry_target".to_string(), + AttrValue::String("plan".to_string()), + ); + + assert_eq!(get_retry_target("work", &g), Some("plan".to_string())); + } + + #[test] + fn retry_target_none_when_missing() { + let mut g = Graph::new("test"); + g.nodes.insert("work".to_string(), Node::new("work")); + assert!(get_retry_target("work", &g).is_none()); + } + + #[test] + fn retry_target_skips_nonexistent_node() { + let mut g = Graph::new("test"); + let mut n = Node::new("work"); + n.attrs.insert( + "retry_target".to_string(), + AttrValue::String("nonexistent".to_string()), + ); + g.nodes.insert("work".to_string(), n); + // No "nonexistent" node -- should fall through to graph-level + assert!(get_retry_target("work", &g).is_none()); + } + + // --- is_terminal tests --- + + #[test] + fn terminal_by_shape() { + let mut n = Node::new("exit"); + n.attrs.insert( + "shape".to_string(), + AttrValue::String("Msquare".to_string()), + ); + assert!(is_terminal(&n)); + } + + #[test] + fn terminal_by_type() { + let mut n = Node::new("end"); + n.attrs + .insert("type".to_string(), AttrValue::String("exit".to_string())); + assert!(is_terminal(&n)); + } + + #[test] + fn non_terminal_node() { + let n = Node::new("work"); + assert!(!is_terminal(&n)); + } + + // --- resolve_fidelity tests --- + + #[test] + fn fidelity_defaults_to_compact() { + use crate::context::keys::Fidelity; + let node = Node::new("work"); + let graph = Graph::new("test"); + assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Compact); + } + + #[test] + fn fidelity_from_graph_default() { + use crate::context::keys::Fidelity; + let node = Node::new("work"); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_fidelity".to_string(), + AttrValue::String("truncate".to_string()), + ); + assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Truncate); + } + + #[test] + fn fidelity_from_node_overrides_graph() { + use crate::context::keys::Fidelity; + let mut node = Node::new("work"); + node.attrs.insert( + "fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_fidelity".to_string(), + AttrValue::String("truncate".to_string()), + ); + assert_eq!(resolve_fidelity(None, &node, &graph), Fidelity::Full); + } + + #[test] + fn fidelity_from_edge_overrides_node() { + use crate::context::keys::Fidelity; + let mut node = Node::new("work"); + node.attrs.insert( + "fidelity".to_string(), + AttrValue::String("full".to_string()), + ); + let mut edge = Edge::new("a", "work"); + edge.attrs.insert( + "fidelity".to_string(), + AttrValue::String("summary:high".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_fidelity(Some(&edge), &node, &graph), + Fidelity::SummaryHigh + ); + } + + // --- resolve_thread_id tests --- + + #[test] + fn thread_id_from_node_attribute() { + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("main-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("main-thread".to_string()) + ); + } + + #[test] + fn thread_id_from_edge_attribute() { + let node = Node::new("work"); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("edge-thread".to_string()) + ); + } + + #[test] + fn thread_id_node_used_when_no_edge_thread() { + // When the edge has no thread_id, the node's thread_id is used. + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("node-thread".to_string()), + ); + let edge = Edge::new("prev", "work"); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("node-thread".to_string()) + ); + } + + #[test] + fn thread_id_edge_overrides_node() { + // Edge thread_id should take precedence over node thread_id, + // matching the fidelity precedence where edge > node. + let mut node = Node::new("work"); + node.attrs.insert( + "thread_id".to_string(), + AttrValue::String("node-thread".to_string()), + ); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("edge-thread".to_string()), + "edge thread_id should override node thread_id" + ); + } + + #[test] + fn thread_id_from_graph_default_thread() { + let node = Node::new("work"); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("shared-thread".to_string()) + ); + } + + #[test] + fn thread_id_edge_overrides_graph_default() { + let node = Node::new("work"); + let mut edge = Edge::new("prev", "work"); + edge.attrs.insert( + "thread_id".to_string(), + AttrValue::String("edge-thread".to_string()), + ); + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(Some(&edge), &node, &graph, Some("prev")), + Some("edge-thread".to_string()) + ); + } + + #[test] + fn thread_id_graph_default_overrides_class() { + let mut node = Node::new("work"); + node.classes = vec!["planning".to_string()]; + let mut graph = Graph::new("test"); + graph.attrs.insert( + "default_thread".to_string(), + AttrValue::String("shared-thread".to_string()), + ); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("shared-thread".to_string()) + ); + } + + #[test] + fn thread_id_from_node_class() { + let mut node = Node::new("work"); + node.classes = vec!["planning".to_string(), "review".to_string()]; + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev")), + Some("planning".to_string()) + ); + } + + #[test] + fn thread_id_fallback_to_previous_node() { + let node = Node::new("work"); + let graph = Graph::new("test"); + assert_eq!( + resolve_thread_id(None, &node, &graph, Some("prev_node")), + Some("prev_node".to_string()) + ); + } + + #[test] + fn thread_id_none_when_no_sources() { + let node = Node::new("start"); + let graph = Graph::new("test"); + assert_eq!(resolve_thread_id(None, &node, &graph, None), None); + } + + // --- classify_outcome tests --- + + #[test] + fn classify_outcome_returns_none_for_success() { + assert!(classify_outcome(&Outcome::success()).is_none()); + } + + #[test] + fn classify_outcome_returns_none_for_skipped() { + assert!(classify_outcome(&Outcome::skipped("")).is_none()); + } + + #[test] + fn classify_outcome_returns_none_for_partial_success() { + let outcome = Outcome { + status: StageStatus::PartialSuccess, + ..Outcome::success() + }; + assert!(classify_outcome(&outcome).is_none()); + } + + #[test] + fn classify_outcome_reads_failure_detail() { + let mut outcome = Outcome::fail_classify("some error"); + // Override the FailureDetail's class directly + outcome.failure.as_mut().unwrap().category = FailureCategory::BudgetExhausted; + assert_eq!( + classify_outcome(&outcome), + Some(FailureCategory::BudgetExhausted) + ); + } + + #[test] + fn classify_outcome_uses_failure_reason_heuristics() { + let outcome = Outcome::fail_classify("rate limited by provider"); + assert_eq!( + classify_outcome(&outcome), + Some(FailureCategory::TransientInfra) + ); + } + + #[test] + fn classify_outcome_defaults_to_deterministic() { + let outcome = Outcome::fail_classify("something went wrong"); + assert_eq!( + classify_outcome(&outcome), + Some(FailureCategory::Deterministic) + ); + } + + #[test] + fn classify_outcome_fail_no_reason_is_deterministic() { + let outcome = Outcome { + status: StageStatus::Fail, + failure: None, + ..Outcome::success() + }; + assert_eq!( + classify_outcome(&outcome), + Some(FailureCategory::Deterministic) + ); + } + + #[test] + fn classify_outcome_retry_status_uses_heuristics() { + let outcome = Outcome::retry_classify("connection refused"); + assert_eq!( + classify_outcome(&outcome), + Some(FailureCategory::TransientInfra) + ); + } +} diff --git a/lib/crates/fabro-workflows/src/pipeline/finalize.rs b/lib/crates/fabro-workflows/src/pipeline/finalize.rs index 953130488..4211e2047 100644 --- a/lib/crates/fabro-workflows/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/finalize.rs @@ -254,7 +254,7 @@ pub async fn finalize( .unwrap_or_default(); if let ( Some(ref base_branch), - Some(ref run_branch), + Some(run_branch), Some(ref creds), Some(ref origin), ) = ( diff --git a/lib/crates/fabro-workflows/src/run_dir.rs b/lib/crates/fabro-workflows/src/run_dir.rs index 75d712ff6..91d56ca37 100644 --- a/lib/crates/fabro-workflows/src/run_dir.rs +++ b/lib/crates/fabro-workflows/src/run_dir.rs @@ -59,3 +59,51 @@ pub(crate) fn write_node_status(run_dir: &Path, node_id: &str, visit: usize, out let _ = std::fs::write(node_dir.join("status.json"), json); } } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + use crate::context::Context; + + #[test] + fn visit_from_context_defaults_to_first_visit() { + let ctx = Context::new(); + assert_eq!(visit_from_context(&ctx), 1); + } + + #[test] + fn visit_from_context_preserves_stored_visit() { + let ctx = Context::new(); + ctx.set( + crate::context::keys::INTERNAL_NODE_VISIT_COUNT, + serde_json::json!(3), + ); + assert_eq!(visit_from_context(&ctx), 3); + } + + #[test] + fn node_dir_first_visit() { + let root = Path::new("/tmp/logs"); + assert_eq!(node_dir(root, "work", 1), root.join("nodes").join("work")); + } + + #[test] + fn node_dir_second_visit() { + let root = Path::new("/tmp/logs"); + assert_eq!( + node_dir(root, "work", 2), + root.join("nodes").join("work-visit_2") + ); + } + + #[test] + fn node_dir_fifth_visit() { + let root = Path::new("/tmp/logs"); + assert_eq!( + node_dir(root, "work", 5), + root.join("nodes").join("work-visit_5") + ); + } +} diff --git a/lib/crates/fabro-workflows/src/sandbox_git.rs b/lib/crates/fabro-workflows/src/sandbox_git.rs index 617016e55..39de405a4 100644 --- a/lib/crates/fabro-workflows/src/sandbox_git.rs +++ b/lib/crates/fabro-workflows/src/sandbox_git.rs @@ -228,3 +228,74 @@ pub async fn git_replace_worktree(sandbox: &dyn Sandbox, path: &str, branch: &st let _ = git_remove_worktree(sandbox, path).await; git_add_worktree(sandbox, path, branch).await } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn git_checkpoint_includes_builtin_excludes() { + // Set up a real git repo + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(repo) + .output() + .unwrap(); + std::process::Command::new("git") + .args([ + "-c", + "user.name=Test", + "-c", + "user.email=test@test.com", + "commit", + "--allow-empty", + "-m", + "initial", + ]) + .current_dir(repo) + .output() + .unwrap(); + + // Create files in both tracked and excluded directories + std::fs::write(repo.join("hello.txt"), "hello").unwrap(); + std::fs::create_dir_all(repo.join("node_modules/pkg")).unwrap(); + std::fs::write(repo.join("node_modules/pkg/index.js"), "module").unwrap(); + std::fs::create_dir_all(repo.join(".venv/lib")).unwrap(); + std::fs::write(repo.join(".venv/lib/site.py"), "venv").unwrap(); + + let sandbox = fabro_agent::LocalSandbox::new(repo.to_path_buf()); + let author = crate::git::GitAuthor::default(); + + // Call git_checkpoint with empty user excludes — built-in excludes should still apply + let result = + git_checkpoint(&sandbox, "run1", "work", "success", 1, None, &[], &author).await; + assert!(result.is_ok(), "git_checkpoint failed: {:?}", result.err()); + + // Verify that excluded directories were NOT staged + let status = sandbox + .exec_command( + "git diff --cached --name-only HEAD~1", + 10_000, + None, + None, + None, + ) + .await + .unwrap(); + let staged_files: Vec<&str> = status.stdout.lines().collect(); + assert!( + staged_files.contains(&"hello.txt"), + "expected hello.txt to be staged, got: {staged_files:?}" + ); + assert!( + !staged_files.iter().any(|f| f.contains("node_modules")), + "node_modules should be excluded from checkpoint, got: {staged_files:?}" + ); + assert!( + !staged_files.iter().any(|f| f.contains(".venv")), + ".venv should be excluded from checkpoint, got: {staged_files:?}" + ); + } +}