From b9f86e415d3af3f9401066f926c8a4973c986e0b Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 20:21:57 +0200 Subject: [PATCH] job_queue: the reconcile gate is the parent chain, not an edge swap_ok_runs_post_swap_before_reconcile drove a rebuild DAG to observe that Reconcile waits for PostSwap. That ordering is not a dependency between them: Reconcile deps AfterAny(Prebuild), and PostSwap sits inside Prebuild's subtree, so Prebuild cannot satisfy the edge while PostSwap is outstanding. Renamed to say what it checks, and it asserts the parent chain plus that edge instead of running anything. Kept as its own test rather than folded into the chain table because the indirection is the easy thing to break -- flattening the chain preserves every edge and still loses the guarantee. swap_failure_still_runs_reconcile is deleted. Its cascade claims are hive-jobq's, and the "says so on the wire" half was nothing: snapshot fills NodeView { state: node.state, .. }, a straight copy of the same State type, so there is no host-side mapping that could disagree. failed_reconcile_marks_dag_failed is deleted too: a one-node DAG whose node fails, asserting the DAG reads Failed, is failed_child_rolls_parent_up_to_failed restated through a c0re template. --- hive-c0re/src/job_queue/tests.rs | 124 ++++++++++++------------------- 1 file changed, 47 insertions(+), 77 deletions(-) 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 ----