diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 768d448d..25b5734b 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -1309,92 +1309,62 @@ fn failed_node_cancels_downstream_but_afterany_reconcile_runs() { ); } -/// The swap-failure recovery: `Swap` fails → the `AfterOk` `PostSwap` is -/// cancel-cascaded → its terminal state still satisfies `Reconcile`'s -/// `AfterAny(PostSwap)` edge, so recovery-start runs and brings a wanted-up -/// agent back on its old config. -#[test] -fn swap_failure_still_runs_reconcile() { - let q = JobQueue::new(1); - let id = submit(&q, rebuild("agent-a", "r")); - // meta_sync + prebuild + stop_for_update - for _ in 0..3 { - let c = claim_one(&q); - q.complete_node(c.node_id, Ok(())); - } - let swap = claim_one(&q); - assert_eq!(swap.kind.as_str(), "swap"); - q.complete_node(swap.node_id, Err("update failed".to_owned())); - // PostSwap (AfterOk on the failed Swap) is cancel-cascaded; Reconcile is - // next-claimable via its AfterAny(PostSwap) edge. - let reconcile = claim_one(&q); - assert_eq!(reconcile.kind.as_str(), "reconcile"); - q.complete_node(reconcile.node_id, Ok(())); - let all_dags = q.snapshot(); - let dag = all_dags.iter().find(|d| d.id == id).expect("dag"); - assert_eq!( - dag.nodes - .iter() - .find(|n| n.kind == "post_swap") - .expect("post_swap node") - .state, - State::Skipped, - "PostSwap is ruled out by the failed Swap, and says so on the wire" - ); - assert_eq!( - dag.nodes - .iter() - .find(|n| n.kind == "swap") - .expect("swap node") - .state, - State::Failed, - "and the failure that ruled it out is still on the wire" - ); - assert_eq!(state_of(&q, id), State::Failed); -} - /// The swap-success path: `Swap` ok → the `AfterOk` `PostSwap` (bookkeeping /// tail) runs, and only then does `Reconcile` fire — serialized behind /// `PostSwap` (not racing it) because `Reconcile` deps `AfterAny(PostSwap)`. #[test] -fn swap_ok_runs_post_swap_before_reconcile() { +fn rebuild_reconcile_waits_for_the_whole_build_subtree() { + // Replaces `swap_ok_runs_post_swap_before_reconcile` and + // `swap_failure_still_runs_reconcile`, which walked the same DAG with the + // swap succeeding in one and failing in the other. + // + // The interesting claim was "Reconcile must wait for PostSwap, not race + // it" — and it does *not* come from an edge between them. `reconcile` deps + // `AfterAny(prebuild)`, while `post_swap` sits inside prebuild's subtree + // (post_swap → stop_for_update → prebuild). A parent is not terminal until + // its subtree is, so prebuild cannot satisfy that edge while post_swap is + // outstanding. **The ordering is the parent chain, not a dependency.** + // + // Both facts are asserted in `rebuild_chain_is_declared_serial`; this test + // states the derived property explicitly because the indirection is the + // easy thing to break — someone flattening the chain would keep every edge + // and still lose the guarantee. let q = JobQueue::new(1); let id = submit(&q, rebuild("agent-a", "r")); - // meta_sync + prebuild + stop_for_update - for _ in 0..3 { - let c = claim_one(&q); - q.complete_node(c.node_id, Ok(())); - } - let swap = claim_one(&q); - assert_eq!(swap.kind.as_str(), "swap"); - q.complete_node(swap.node_id, Ok(())); - // PostSwap runs next, and nothing else is claimable while it does — the - // tail serializes ahead of Reconcile. - let post_swap = claim_one(&q); - assert_eq!(post_swap.kind.as_str(), "post_swap"); - assert!( - q.claim_ready().is_empty(), - "Reconcile must wait for PostSwap, not race it" + let shape = declared_shape(&q, id); + let parent_of = |kind: &str| { + shape + .iter() + .find(|d| d.kind == kind) + .unwrap_or_else(|| panic!("{kind} node")) + .parent + }; + assert_eq!(parent_of("post_swap"), Some("stop_for_update")); + assert_eq!(parent_of("stop_for_update"), Some("prebuild")); + assert_eq!( + shape + .iter() + .find(|d| d.kind == "reconcile") + .expect("reconcile node") + .after, + vec![("prebuild", "done|failed|skipped".to_owned())], + "reconcile gates on prebuild's roll-up, which covers the whole build \ + subtree — including post_swap — and runs on failure too" ); - q.complete_node(post_swap.node_id, Ok(())); - let reconcile = claim_one(&q); - assert_eq!(reconcile.kind.as_str(), "reconcile"); - q.complete_node(reconcile.node_id, Ok(())); - settle_rebuild_tail(&q, "agent-a", true); - assert_eq!(state_of(&q, id), State::Done); } -#[test] -fn failed_reconcile_marks_dag_failed() { - let q = JobQueue::new(1); - let id = submit( - &q, - templates::reconcile_only("agent-a", Source::Manual, "start".to_owned()), - ); - let c = claim_one(&q); - q.complete_node(c.node_id, Err("start failed".to_owned())); - assert_eq!(state_of(&q, id), State::Failed); -} +// `swap_failure_still_runs_reconcile` lived here. +// +// It asserted that a failed swap leaves `post_swap` `Skipped` and `swap` +// `Failed`, and that reconcile still runs. All three are hive_jobq's cascade +// (`failed_after_ok_dep_cancels_dependents_but_after_any_still_runs`), and the +// "says so on the wire" half turned out to be nothing: `snapshot` fills +// `NodeView { state: node.state, .. }`, a straight copy of the same `State` +// type, so there is no c0re-side mapping to get wrong. +// +// `failed_reconcile_marks_dag_failed` lived here too — a one-node DAG whose +// node fails, asserting the DAG reads `Failed`. That is `failed_child_rolls_ +// parent_up_to_failed` in hive_jobq, restated through a c0re template. // ---- cancel ----