From 3429a8c5a61b2340ffb43f1683aa5f0fb1955974 Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 25 Jul 2026 23:24:48 +0200 Subject: [PATCH] job_queue: grow the rebuild subgraph from DeployApply (#2664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The config-PR deploy's apply node still did the whole container rebuild inline, through the last surviving `lifecycle::rebuild_no_meta` call. It now merges, opens the two-phase meta deploy, and returns the ordinary rebuild chain as a subgraph the scheduler grafts into the live DAG under it. A new `FinalizeDeploy` node, gated on that graft, plants the deploy tag and commits the staged lock. Net effect: "did the agent come back up?" is answered by `Reconcile` succeeding, the same way it is for every other rebuild, instead of by a fused inline start — and each deploy phase is its own queue node, so the dashboard shows which one is running. The grafted nodes root on the apply node, so they land inside `DeployWindow`'s subtree and re-enter the meta window and build slot it already holds rather than deadlocking against them. The new happy-path test runs on a one-slot queue specifically to pin that down. `FinalizeDeploy`'s two git writes are fatal, deliberately: they are what tells `DeployTail` a deploy confirmed good, so a node that merely warned on them could report success while leaving the tail looking at the git state of a failure — and the tail would then roll a good deploy back. The trailing `meta::finalize_deploy` stays warn-only, since by then the container already runs the new config. The `failed/` annotated tag moves into the tail, which is now the only place holding a failed deploy. It reads the reason off the DAG via a new `JobQueue::first_error`, and is gated on `main` having actually moved — the rollback ref is parked *before* the merge, so its existence alone does not mean a merge happened, and a pre-merge rejection must not tag the previous, innocent head. Removing the last inline rebuild orphaned a chain of now-dead code: `rebuild_no_meta`, `container_exists`, `Coordinator::set_queue_build_log` and `JobQueue::set_build_log_id_running`, all deleted here. --- docs/approvals.md | 18 +-- docs/coordinator.md | 20 ++- hive-c0re/src/actions.rs | 189 +++++++++++++++------------ hive-c0re/src/coordinator.rs | 10 -- hive-c0re/src/job_queue/exec.rs | 26 +++- hive-c0re/src/job_queue/mod.rs | 27 ++-- hive-c0re/src/job_queue/model.rs | 46 +++++-- hive-c0re/src/job_queue/templates.rs | 48 ++++++- hive-c0re/src/job_queue/tests.rs | 137 ++++++++++++++++++- hive-c0re/src/lifecycle/mod.rs | 122 +---------------- 10 files changed, 387 insertions(+), 256 deletions(-) diff --git a/docs/approvals.md b/docs/approvals.md index d54b590e..6ae2be1c 100644 --- a/docs/approvals.md +++ b/docs/approvals.md @@ -247,20 +247,20 @@ a plain `nix flake lock` leaves an existing applied override in place (it only re-locks when the declared url itself changes), so the forge-declared / applied-deployed split is stable. -Per-deploy lock flow (two-phase, owned by -`actions::run_deploy_apply` → `deploy_applied_target` → -`meta::{prepare,finalize,abort}_deploy`, with the abort half moved out -into `actions::run_deploy_tail`): +Per-deploy lock flow (two-phase), spread across the deploy subtree's +nodes — each phase is its own node, so the queue can show which one is +running and a restart resumes at node granularity: -1. `meta::prepare_deploy(name)` runs +1. `DeployApply` → `meta::prepare_deploy(name)` runs `nix flake lock --update-input agent-` without committing. Working tree of meta now points the input at `applied//main` (which the deploy already fast-forwarded to the reviewed PR head). -2. `lifecycle::rebuild_no_meta` runs - `nixos-container update --flake meta#`. Nix - evaluates against the staged lock. -3. On success — `meta::finalize_deploy(name, sha, "deployed/ +2. The rebuild subgraph `DeployApply` grows into the DAG builds and + swaps the container (`Prebuild → StopForUpdate → Swap → PostSwap`, + plus `Reconcile`). Nix evaluates against the staged lock. +3. On success — `FinalizeDeploy` drops the rollback ref, plants + `deployed/`, then `meta::finalize_deploy(name, sha, "deployed/ ")` stages `flake.lock` and commits with `deploy deployed/ `. Meta's git log gains one entry per successful deploy. diff --git a/docs/coordinator.md b/docs/coordinator.md index 1ed1fbf1..9e3ac7c8 100644 --- a/docs/coordinator.md +++ b/docs/coordinator.md @@ -355,11 +355,17 @@ Sequence for a rebuild DAG (each step is its own queue node): slot, so the next DAG's `Prebuild` overlaps the container boot — the old "deferred start" split, now structural. -The approval apply-commit pipeline still drives `lifecycle::rebuild_no_meta` -(the fused stop/update/start path with an inline start) inside its -`DeployApply` node, because it verifies the agent comes back up before -finalizing the deploy tag. Breaking that fused path apart into the -`Prebuild → Swap → Reconcile` chain above is increment 2b of #2664, not 2a. +The approval deploy uses this same chain rather than a rebuild path of its +own. Its `DeployApply` node does not build: it merges, opens the two-phase +meta deploy, and returns the chain above as a subgraph the scheduler grafts +into the live DAG under that node. A `FinalizeDeploy` node gated on the +graft's completion then plants the deploy tag — so "did the agent come back +up?" is answered by `Reconcile` succeeding, the same way it is for every +other rebuild, instead of by a fused inline start. + +The grafted nodes land *inside* `DeployWindow`'s subtree, so they re-enter +the meta window and build slot it already holds rather than deadlocking +against it. ### Cold-start fallback @@ -371,8 +377,8 @@ half-started at that point. Fallback: `stop` (graceful SIGTERM drain) → `kill` (SIGKILL any lingering processes) → `start` (clean cold-start, no generation transition, new activation runs cleanly). Both errors are preserved and surfaced if the cold-start also fails. The fallback -lives in `lifecycle::start_with_fallback`, shared by the apply-commit deploy's -inline start and every `Reconcile` node's start action. +lives in `lifecycle::start_with_fallback`, used by every `Reconcile` node's +start action. ### Spawn path (new container) diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index 3db5de87..e9a1df93 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -19,7 +19,8 @@ use crate::lifecycle; /// /// Dispatch: /// - `MergeConfigPr` → a `DeployWindow` DAG (`MergeVerify → DeployApply → -/// DeployTail` under a resource-holding root; ~30-90s) +/// FinalizeDeploy`, plus an `AfterAny` `DeployTail`, under a +/// resource-holding root; ~30-90s) /// - `UpdateMetaInputs` → a `MetaUpdate` DAG (fan-out on completion) /// - `Spawn` → a `Spawn` DAG (`Create → WriteDropin → Reconcile`) /// - `InitConfig` → inline (<1s; queue card would be noise) @@ -105,8 +106,9 @@ pub async fn approve(coord: Arc, id: i64) -> Result<()> { // The work ends in a container rebuild, so route it through the // rebuild queue. The queue worker dispatches the deploy DAG's // nodes to `run_deploy_merge_verify` (drift gate + eval), - // `run_deploy_apply` (ff-merge + rebuild) and `run_deploy_tail` - // (compensation + forge mirror). + // `run_deploy_apply` (ff-merge, then grows the rebuild subgraph), + // `run_finalize_deploy` (deploy tag + lock commit) and + // `run_deploy_tail` (compensation + forge mirror). enqueue_approval_rebuild( &coord, approval.agent.as_str(), @@ -148,8 +150,8 @@ fn enqueue_approval_rebuild( /// handed between nodes: hive-c0re can restart between the apply and the tail, /// and the whole point of splitting the deploy is that the tail still knows /// what to undo when it does. The ref's existence IS the "a merge landed but -/// hasn't been confirmed good yet" flag — [`run_deploy_apply`] drops it the -/// moment the rebuild succeeds. +/// hasn't been confirmed good yet" flag — [`run_finalize_deploy`] drops it the +/// moment the rebuild has come up clean. fn rollback_ref(approval_id: i64) -> String { format!("refs/hyperhive/rollback/{approval_id}") } @@ -164,7 +166,6 @@ struct DeployCtx { pr: u64, /// The PR head sha the operator reviewed (`approval.fetched_sha`). reviewed: String, - agent_dir: std::path::PathBuf, applied_dir: std::path::PathBuf, /// The agent's forge config repo (`/`). repo: String, @@ -184,7 +185,6 @@ fn deploy_ctx(coord: &Coordinator, approval_id: i64) -> Result { Ok(DeployCtx { pr, reviewed, - agent_dir: crate::paths::agent_runtime_dir(approval.agent.as_str()), applied_dir: crate::paths::applied_dir(approval.agent.as_str()), repo: crate::forge::config_repo(approval.agent.as_str()), approval, @@ -246,20 +246,24 @@ pub async fn run_deploy_merge_verify( } /// `DeployApply` node body — the irreversible half. Parks the rollback ref, -/// fast-forward-merges the PR (THE merge), then runs the deploy proper. +/// fast-forward-merges the PR (THE merge), then opens the deploy. /// /// The ref is parked *before* the merge, so a hive-c0re crash anywhere from /// here on still leaves [`run_deploy_tail`] enough to undo. If the merge itself /// fails, `main` never moved and the tail's compensation is a no-op against the /// same sha — harmless, and cheaper than trying to be clever about it. /// +/// Returning `Ok` is the signal for the caller to grow the rebuild subgraph into +/// this DAG under this node; the build itself does not happen here. +/// /// # Errors /// /// Returns an error if the approval can't be loaded, if reading or parking the /// pre-merge `main` sha fails, if the forge refuses the fast-forward merge -/// (including a head that drifted between verify and merge), or if the deploy -/// of the merged target fails. From the merge onward a failure is *not* -/// retryable on its own — [`run_deploy_tail`] runs `AfterAny` to compensate. +/// (including a head that drifted between verify and merge), or if +/// fast-forwarding `applied/main` / `meta::prepare_deploy` fails. From the merge +/// onward a failure is *not* retryable on its own — [`run_deploy_tail`] runs +/// `AfterAny` to compensate. pub async fn run_deploy_apply( coord: &Arc, queue_entry_id: Option, @@ -290,13 +294,11 @@ pub async fn run_deploy_apply( Err(e) => bail!("ff-merge PR #{pr}: {e}"), } - deploy_applied_target( + prepare_applied_target( coord, agent, - &ctx.agent_dir, &ctx.applied_dir, &ctx.reviewed, - approval_id, queue_entry_id, ) .await @@ -307,9 +309,11 @@ pub async fn run_deploy_apply( /// construction: it is the recovery step, so it has nothing to hand a failure /// to. Every fallible call inside warns and continues. /// -/// 1. If the rollback ref survived, the deploy did not confirm good: roll -/// `applied/main` back to the parked sha, resync the working tree, and drop -/// the staged meta lock so the deploy log only ever shows successes. +/// 1. If the rollback ref survived, the deploy did not confirm good: tag the +/// merged commit `failed/` (annotated with the DAG's first error, while +/// that sha is still reachable), then roll `applied/main` back to the parked +/// sha, resync the working tree, and drop the staged meta lock so the deploy +/// log only ever shows successes. /// 2. Mirror the agent's config repo to the forge. `main` is already ff'd by /// the merge, so only the `deployed/` / `failed/` tag refspec /// actually lands — that's what gives the merged commit a forge-visible @@ -339,6 +343,34 @@ pub async fn run_deploy_tail( "deploy tail: rollback ref outlived a successful deploy; dropping it without compensating" ); } else { + // Mark the commit that failed to deploy, before undoing the merge + // that put it on `main`. After the rollback below, that sha is only + // reachable through this tag. + // + // Gated on `main` having actually moved: the rollback ref is parked + // *before* the merge, so its existence alone doesn't mean a merge + // happened. A pre-merge rejection (drift gate, eval failure, or the + // ff-merge itself failing) has no deployed commit to blame, and + // tagging the previous — innocent — head would point the operator at + // a commit that never got near a container. + // + // The annotation is read off the DAG rather than passed down from + // the node that failed: this node runs `AfterAny` its subject, so by + // now that node has settled `Failed` with its error recorded. + if let Ok(merged) = lifecycle::git_rev_parse(&applied_dir, "refs/heads/main").await + && merged != prev_main + { + let tag = format!("failed/{approval_id}"); + let body = queue_entry_id + .and_then(|dag_id| coord.job_queue.first_error(dag_id)) + .unwrap_or_else(|| "deploy failed".to_owned()); + if let Err(e) = + lifecycle::git_tag_annotated(&applied_dir, &tag, &merged, &body).await + { + tracing::warn!(%agent, approval_id, error = ?e, "deploy tail: annotate failed tag failed"); + } + } + coord.set_queue_step(queue_entry_id, "roll back applied/main"); if let Err(e) = lifecycle::git_update_ref(&applied_dir, "refs/heads/main", &prev_main).await @@ -740,9 +772,11 @@ fn finish_approval( result } -/// Post-merge deploy for the config-PR flow. Fast-forwards `applied/main` to -/// `target`, syncs the working tree, runs the meta two-phase deploy + container -/// rebuild, and plants the `deployed/` / `failed/` bookkeeping tag. +/// Open the deploy for the config-PR flow: fast-forward `applied/main` to +/// `target`, sync the working tree, and run phase 1 of the meta two-phase +/// deploy. The container rebuild that used to run inline here is now the +/// subgraph [`run_deploy_apply`]'s node grows into the DAG, and the closing half +/// is [`run_finalize_deploy`]. /// /// **Undo is not this function's job.** Every early return here leaves the /// applied repo dirty on purpose — [`run_deploy_tail`] owns compensation, and @@ -751,18 +785,14 @@ fn finish_approval( /// pre-merge sha in a git ref instead of a local variable. /// /// Caller-specific bits stay OUT of here: fetching the PR head, the -/// `verify_commit` gate, and the ff-merge. `target` is both what `applied/main` -/// fast-forwards to and the sha `meta::finalize_deploy` records — for a merge -/// they are always the same reviewed head. The agent always already exists here +/// `verify_commit` gate, and the ff-merge. The agent always already exists here /// (a merge is never a first spawn), so there's no `sync_agents` step — the /// operator `Spawn` flow owns first-time meta registration. -async fn deploy_applied_target( +async fn prepare_applied_target( coord: &Arc, agent: &str, - agent_dir: &std::path::Path, applied_dir: &std::path::Path, target: &str, - id: i64, queue_entry_id: Option, ) -> Result<()> { coord.set_queue_step(queue_entry_id, "fast-forward applied/main"); @@ -777,70 +807,59 @@ async fn deploy_applied_target( .map_err(|e| anyhow::anyhow!("read-tree to main: {e:#}"))?; coord.set_queue_step(queue_entry_id, "meta prepare_deploy"); - // Phase 1 of the meta two-phase deploy: relock without committing. + // Phase 1 of the meta two-phase deploy: relock without committing. The + // staged lock then stays uncommitted across the whole appended rebuild — + // which is why the `MetaWindow` is held by the deploy root, not by a node. crate::meta::prepare_deploy(agent) .await - .map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}"))?; + .map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}")) +} - // Container-level rebuild (or first-time create) against meta#. - // Step labels are emitted inside rebuild_no_meta via the callback so - // the dashboard reflects actual phase progress rather than a static - // "nixos-container update" label for the whole multi-minute window. - let hive = coord.hive_env(); - let paths = Coordinator::agent_paths(agent, agent_dir.to_path_buf()); - let build_result = lifecycle::rebuild_no_meta( - agent, - &hive, - &paths, - // Inline start: the apply-commit flow verifies the agent comes - // back up before finalizing the deploy tag, so the start stays - // part of this entry rather than a deferred fast-lane follow-up. - false, - &|step| coord.set_queue_step(queue_entry_id, step), - &|log_id| coord.set_queue_build_log(queue_entry_id, log_id), - ) - .await; +/// `FinalizeDeploy` node body — phase 2 of the meta two-phase deploy, run once +/// the appended rebuild subgraph has built, swapped, and brought the container +/// back up. +/// +/// Drops the rollback ref *first*: from here the deploy is good and +/// [`run_deploy_tail`] must not roll `main` back. Ordering that ahead of the tag +/// plant is what makes the tail's `deployed/` cross-check a second line of +/// defence rather than the only one. No agent kick — the rebuild's own +/// `PostSwap` already did it. +/// +/// # Errors +/// +/// Returns an error if the approval can't be loaded, if dropping the rollback +/// ref fails, or if planting the `deployed/` tag fails. Those two git writes +/// *are* the deploy's "confirmed good" signal, so warning past them would let +/// this node report success while leaving the tail looking at the git state of a +/// failure — and the tail would then compensate a good deploy. A failing +/// `meta::finalize_deploy` is deliberately *not* an error: the container is +/// already running the new config by then, and the staged `flake.lock` it +/// couldn't commit is something the operator can land by hand. +pub async fn run_finalize_deploy( + coord: &Arc, + queue_entry_id: Option, + approval_id: i64, +) -> Result<()> { + let ctx = deploy_ctx(coord, approval_id)?; + let agent = ctx.approval.agent.as_str(); + let target = ctx.reviewed.as_str(); - match build_result { - Ok(_) => { - coord.set_queue_step(queue_entry_id, "finalize deploy"); - // Drop the compensation ref FIRST: from here the deploy is good and - // the tail must not roll `main` back. Ordering it ahead of the tag - // plant is what makes the tail's `deployed/` cross-check a - // second line of defence rather than the only one. - if let Err(e) = lifecycle::git_delete_ref(applied_dir, &rollback_ref(id)).await { - tracing::warn!(%agent, %id, error = ?e, "drop rollback ref after successful deploy failed"); - } - let tag = format!("deployed/{id}"); - if let Err(e) = lifecycle::git_tag(applied_dir, &tag, target).await { - tracing::warn!(%agent, %id, error = ?e, "plant deployed tag failed"); - } - if let Err(e) = crate::meta::finalize_deploy(agent, target, &tag).await { - // The build itself succeeded — meta lock landed but - // couldn't be committed. Surface as a soft warn so the - // operator can git-commit by hand if they care. - tracing::warn!(%agent, %id, error = ?e, "meta finalize_deploy failed"); - } - // Wake the agent on its next turn so claude sees the - // config change took effect. Same hint pattern as - // auto_update::rebuild_agent — manager approved a - // proposal, agent picks up where it left off with the - // new env / packages. - coord.kick_agent(agent, "config update applied"); - Ok(()) - } - Err(e) => { - // Plant the failure marker here rather than in the tail: this is - // the only place that holds the build error to annotate it with. - // The repo-state rollback is the tail's, via the parked ref. - let tag = format!("failed/{id}"); - let body = format!("{e:#}"); - if let Err(te) = lifecycle::git_tag_annotated(applied_dir, &tag, target, &body).await { - tracing::warn!(%agent, %id, error = ?te, "annotate failed tag failed"); - } - Err(e) - } + coord.set_queue_step(queue_entry_id, "drop rollback ref"); + lifecycle::git_delete_ref(&ctx.applied_dir, &rollback_ref(approval_id)) + .await + .map_err(|e| anyhow::anyhow!("drop rollback ref for approval {approval_id}: {e:#}"))?; + + coord.set_queue_step(queue_entry_id, "plant deployed tag"); + let tag = format!("deployed/{approval_id}"); + lifecycle::git_tag(&ctx.applied_dir, &tag, target) + .await + .map_err(|e| anyhow::anyhow!("plant {tag}: {e:#}"))?; + + coord.set_queue_step(queue_entry_id, "meta finalize_deploy"); + if let Err(e) = crate::meta::finalize_deploy(agent, target, &tag).await { + tracing::warn!(%agent, approval_id, error = ?e, "meta finalize_deploy failed"); } + Ok(()) } /// Tear down a sub-agent container. By default this is non-destructive to diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 0e78a4d7..ea27baea 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -683,16 +683,6 @@ impl Coordinator { } } - /// Link a `build_logs` row to the currently-running node of DAG - /// `id` and re-emit the snapshot. Same DAG-id-only compatibility - /// surface as [`Self::set_queue_step`]. - pub fn set_queue_build_log(self: &Arc, id: Option, log_id: i64) { - let Some(id) = id else { return }; - if self.job_queue.set_build_log_id_running(id, log_id) { - self.emit_rebuild_queue_snapshot(); - } - } - /// Subscribe to the shutdown watch channel. Background tasks call /// this at spawn time and break their loop when the receiver /// transitions to `true` (via `Coordinator::request_shutdown`). diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 6fc5eb36..1aa2f9f8 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -31,7 +31,7 @@ pub struct NodeOutput { /// runtime — the single in-DAG-growth channel. Each inner /// `Vec` is one independent subgraph whose `deps` are local /// (0-based within that subgraph); the scheduler appends each via - /// [`JobQueue::append_subgraph`], which rebases the deps onto the DAG's + /// [`super::JobQueue::append_subgraph`], which rebases the deps onto the DAG's /// node-id space and roots the subgraph on the emitting node. Used both /// for the multi-node case (`MetaLock` growing one rebuild subgraph per /// agent — the startup sweep's stale agents, the meta-update cascade's @@ -102,6 +102,7 @@ pub(super) async fn run_node(coord: &Arc, claim: &Claim) -> Result< NodeKind::DeployWindow { .. } => run_deploy_window(claim), NodeKind::MergeVerify { .. } => run_merge_verify(coord, claim).await, NodeKind::DeployApply { .. } => run_deploy_apply(coord, claim).await, + NodeKind::FinalizeDeploy { .. } => run_finalize_deploy(coord, claim).await, NodeKind::DeployTail { .. } => run_deploy_tail(coord, claim).await, NodeKind::SetWanted { up, .. } => run_set_wanted(coord, claim, *up), // Pure grouping container — no work; completing it lets it reach @@ -615,10 +616,27 @@ async fn run_merge_verify(coord: &Arc, claim: &Claim) -> Result, claim: &Claim) -> Result { - crate::actions::run_deploy_apply(coord, Some(claim.dag_id), deploy_approval_id(claim)?) + crate::actions::run_deploy_apply(coord, Some(claim.dag_id), deploy_approval_id(claim)?).await?; + Ok(NodeOutput { + append_subgraph: vec![super::templates::deploy_rebuild_nodes(claim.kind.agent())], + }) +} + +/// Deploy phase 3 — close the staged-lock window once the appended rebuild has +/// come up clean: drop the rollback ref, plant the `deployed/` tag, commit +/// the staged lock. +async fn run_finalize_deploy(coord: &Arc, claim: &Claim) -> Result { + crate::actions::run_finalize_deploy(coord, Some(claim.dag_id), deploy_approval_id(claim)?) .await .map(|()| NodeOutput::default()) } diff --git a/hive-c0re/src/job_queue/mod.rs b/hive-c0re/src/job_queue/mod.rs index 09d90c24..abd57d8c 100644 --- a/hive-c0re/src/job_queue/mod.rs +++ b/hive-c0re/src/job_queue/mod.rs @@ -500,17 +500,6 @@ impl JobQueue { true } - /// Link a `build_logs` row to the DAG's currently-running node — DAG-id-only - /// compatibility surface (approval pipeline callbacks). - pub fn set_build_log_id_running(&self, dag_id: u64, log_id: i64) -> bool { - let mut inner = self.lock(); - let Some(node_id) = inner.running_node_of(dag_id) else { - return false; - }; - inner.node_rt.entry(node_id).or_default().build_log_id = Some(log_id); - true - } - /// The `build_logs` row id linked to the wire node id `node_id`, if any — /// the lookup behind the `GET /api/build-log/` query endpoint (the /// client fetches a node's captured build output on demand rather than @@ -526,6 +515,22 @@ impl JobQueue { .and_then(|(_, rt)| rt.build_log_id) } + /// The first failed node's error in `dag_id`, if any has failed yet. + /// + /// Unlike the roll-up summary this is readable *mid-flight*, which is the + /// point: a compensation node runs `AfterAny` its subject, so when it asks, + /// the DAG is still `Finishing` (the compensation node itself is running) + /// while the node it is compensating for has already settled `Failed`. That + /// lets the compensation annotate its bookkeeping with the reason the deploy + /// failed, instead of having the error handed down from the node that hit + /// it. `None` when nothing has failed — the ordinary success path. + #[must_use] + pub fn first_error(&self, dag_id: u64) -> Option { + let inner = self.lock(); + let container = inner.container(dag_id)?; + inner.dag_first_error(container) + } + /// A DAG's terminal roll-up summary, computed on demand from its container. /// `None` if the DAG id is unknown. Test-only — production reads the summary /// `complete_node` returns when the container rolls up terminal. diff --git a/hive-c0re/src/job_queue/model.rs b/hive-c0re/src/job_queue/model.rs index e69c2bd5..4323ab35 100644 --- a/hive-c0re/src/job_queue/model.rs +++ b/hive-c0re/src/job_queue/model.rs @@ -209,16 +209,36 @@ pub enum NodeKind { /// untouched, so it is safely retryable and cancel-safe: nothing downstream /// has happened yet. MergeVerify { agent: String }, - /// Deploy phase 2 — everything from the irreversible fast-forward onward: - /// ff-merge the reviewed head to `main` via the forge API, two-phase meta - /// `prepare_deploy`, the container rebuild, then on success the - /// `deployed/` tag + `finalize_deploy`. + /// Deploy phase 2 — the irreversible fast-forward plus the *opening* half of + /// the two-phase meta deploy: park the rollback ref, ff-merge the reviewed + /// head to `main` via the forge API, ff `applied/main`, and + /// `meta::prepare_deploy` (which stages `flake.lock` uncommitted). /// - /// Still one node in this increment: splitting the tail into - /// `FfMain`/`PrepareDeploy`/rebuild/`FinalizeDeploy` children is the next - /// one. What *is* already split out is the compensation path — see + /// It does **not** run the container rebuild itself. It grows the ordinary + /// rebuild subgraph into this DAG as its own children + /// ([`super::templates::deploy_rebuild_nodes`], `relock = false` — the lock + /// is already staged), so the multi-minute build renders as the same real + /// nodes every other rebuild does instead of one opaque box. Closing the + /// staged-lock window is likewise its own node + /// ([`NodeKind::FinalizeDeploy`]), and the compensation path is /// [`NodeKind::DeployTail`]. DeployApply { agent: String }, + /// Deploy phase 3 — close the two-phase meta deploy once the rebuild + /// subgraph under [`NodeKind::DeployApply`] has come up clean: drop the + /// rollback ref, plant the `deployed/` tag, commit the staged + /// `flake.lock` (`meta::finalize_deploy`). + /// + /// Its two git steps are **fatal**, deliberately. They are the writes that + /// tell [`NodeKind::DeployTail`] a deploy confirmed good, so a node that + /// merely warned on them could report success while leaving the tail + /// looking at the git state of a failure — and the tail would then roll a + /// *good* deploy back. Failing loudly keeps the node's outcome and the + /// repo's state saying the same thing. + /// + /// The trailing `meta::finalize_deploy` stays warn-only: by then the + /// container already runs the new config, and an uncommitted staged lock is + /// something the operator can commit by hand. + FinalizeDeploy { agent: String }, /// Deploy compensation **and bookkeeping** tail — `AfterAny` /// [`NodeKind::DeployApply`], so it runs on success, failure, and cancel /// alike, in the same spirit as the rebuild template's tail `Reconcile` @@ -238,7 +258,7 @@ pub enum NodeKind { /// parked in the applied repo rather than passed between nodes: /// `DeployApply` writes the pre-merge `main` sha to /// `refs/hyperhive/rollback/` before the fast-forward and - /// deletes it once the deploy has been finalized. So the ref existing *is* + /// [`NodeKind::FinalizeDeploy`] deletes it. So the ref existing *is* /// the "a merge landed and was not finalized" signal, and its absence makes /// this node a no-op. Parking it in git rather than in a node payload also /// means it survives a `hive-c0re` restart mid-deploy, which an in-memory @@ -299,6 +319,7 @@ impl NodeKind { NodeKind::DeployWindow { .. } => "deploy_window", NodeKind::MergeVerify { .. } => "merge_verify", NodeKind::DeployApply { .. } => "deploy_apply", + NodeKind::FinalizeDeploy { .. } => "finalize_deploy", NodeKind::DeployTail { .. } => "deploy_tail", NodeKind::SetWanted { .. } => "set_wanted", NodeKind::Dag { .. } => "dag", @@ -328,6 +349,7 @@ impl NodeKind { | NodeKind::DeployWindow { agent } | NodeKind::MergeVerify { agent } | NodeKind::DeployApply { agent } + | NodeKind::FinalizeDeploy { agent } | NodeKind::DeployTail { agent } | NodeKind::SetWanted { agent, .. } => agent, NodeKind::MetaLock { .. } | NodeKind::Dag { .. } => "", @@ -389,6 +411,13 @@ impl NodeKind { /// That is why the meta preamble is its own [`NodeKind::MetaSync`] node, /// and why that node is a sibling rather than `Prebuild`'s parent (a /// resource held by a parent covers its whole subtree). + /// + /// Two of these kinds run *inside* a [`NodeKind::DeployWindow`]'s subtree + /// (the appended rebuild's `MetaSync`, and [`NodeKind::FinalizeDeploy`]). + /// They still declare the window: a descendant re-enters an ancestor's hold + /// through the crate's recursive lock, exactly as `Start` / `Stop` re-enter + /// a `Reconcile`'s agent lease. Declaring it is what keeps the requirement + /// true of the *node* rather than of one particular DAG shape. pub fn needs_meta_window(&self) -> bool { matches!( self, @@ -397,6 +426,7 @@ impl NodeKind { | NodeKind::MetaLock { .. } | NodeKind::WritePermFile { .. } | NodeKind::DeployWindow { .. } + | NodeKind::FinalizeDeploy { .. } ) } } diff --git a/hive-c0re/src/job_queue/templates.rs b/hive-c0re/src/job_queue/templates.rs index 0fd03da8..12d4496e 100644 --- a/hive-c0re/src/job_queue/templates.rs +++ b/hive-c0re/src/job_queue/templates.rs @@ -123,6 +123,50 @@ pub(crate) fn rebuild_nodes(agent: &str, relock: bool, base: u64) -> Vec Vec { + let mut nodes = rebuild_nodes(agent, false, 0); + nodes.push(node( + NodeKind::FinalizeDeploy { + agent: agent.to_owned(), + }, + vec![ + Dep { + on: 1, + when: DepWhen::AfterOk, + }, + Dep { + on: 5, + when: DepWhen::AfterOk, + }, + ], + )); + nodes +} + /// One uniform rebuild shape — no `was_running` branch. `StopForUpdate` /// noops when already down; the tail `Reconcile` auto-noops the start /// when `wanted = Offline` (a rebuild of a deliberately-stopped agent @@ -149,7 +193,9 @@ pub fn rebuild(agent: &str, source: Source, reason: String, relock: bool) -> Dag /// nothing, so a failure here cancel-cascades its siblings with the forge and /// the applied repo exactly as they were. /// - `DeployApply` (2, child, `AfterOk` `MergeVerify`): the irreversible half — -/// ff-merge, `prepare_deploy`, rebuild, `finalize_deploy`. +/// ff-merge + `prepare_deploy`. It doesn't rebuild inline; it grows +/// [`deploy_rebuild_nodes`] into this DAG as its own children, so the build +/// and the closing `FinalizeDeploy` are real nodes under the same window. /// - `DeployTail` (3, child, `AfterAny` `DeployApply`): the compensation + /// bookkeeping tail — rollback when a merge landed unfinalized, forge tag /// mirror, PR failure comment (see [`NodeKind::DeployTail`]). diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 53d2ee55..32b2fdec 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -976,6 +976,132 @@ fn deploy_dag_runs_phases_in_order_and_tails_a_failed_apply() { assert_eq!(summary.approval_id, Some(7)); } +/// The deploy's happy path: `DeployApply` does not build. It grows the ordinary +/// rebuild chain into the live DAG under itself, and `FinalizeDeploy` — gated on +/// that graft finishing — plants the deploy tag last. +/// +/// The queue is built with **one** build slot on purpose. `DeployWindow` already +/// holds that slot (and the meta window) for the whole subtree, so the grafted +/// `Prebuild` can only ever claim by *re-entering* its ancestor's hold. If the +/// graft were rooted anywhere outside `DeployWindow`'s subtree it would block on +/// a resource its own DAG owns and deadlock — this test is what pins that down. +#[test] +fn deploy_apply_grows_rebuild_subgraph_and_finalizes_after_it() { + let q = JobQueue::new(1); + let id = submit( + &q, + templates::approval_deploy("agent-a", 11, "approval 11".to_owned()), + ); + + let root = claim_one(&q); + assert!(matches!(root.kind, NodeKind::DeployWindow { .. })); + q.complete_node(id, root.node_id, Ok(())); + let verify = claim_one(&q); + q.complete_node(id, verify.node_id, Ok(())); + + let apply = claim_one(&q); + assert!(matches!(apply.kind, NodeKind::DeployApply { .. })); + // Mirrors the scheduler: the executor's `NodeOutput` subgraphs are grafted + // BEFORE the emitting node is completed. Completing first would settle the + // apply node `Done` with nothing under it, opening the tail's `AfterAny` + // gate immediately and letting the deploy "finish" before it had built. + let grown = q.append_subgraph( + id, + &templates::deploy_rebuild_nodes("agent-a"), + apply.node_id, + ); + assert!(!grown.is_empty(), "subgraph grafted onto the apply node"); + q.complete_node(id, apply.node_id, Ok(())); + + // The grafted chain runs in rebuild order. `claim_one` asserts exactly one + // claimable node at each step, which also proves the `AfterAny` tail stays + // shut: `DeployApply` is `Finishing` (not terminal) while its new children + // run, and `Finishing` satisfies neither dep kind. + for expected in [ + "meta_sync", + "prebuild", + "stop_for_update", + "swap", + "post_swap", + "reconcile", + ] { + let c = claim_one(&q); + assert_eq!(c.kind.as_str(), expected, "grafted phase order"); + q.complete_node(id, c.node_id, Ok(())); + } + + let finalize = claim_one(&q); + assert!( + matches!(finalize.kind, NodeKind::FinalizeDeploy { .. }), + "the deploy tag is planted only after the rebuild came up clean" + ); + q.complete_node(id, finalize.node_id, Ok(())); + + let tail = claim_one(&q); + assert!(matches!(tail.kind, NodeKind::DeployTail { .. })); + q.complete_node(id, tail.node_id, Ok(())); + + let summary = q.terminal_summary(id).expect("dag terminal"); + assert_eq!(summary.state, State::Done); + assert_eq!(summary.approval_id, Some(11)); +} + +/// A failure *inside* the grafted rebuild is the failure mode the subgraph +/// growth introduces: the deploy is already merged and the container half-swapped. +/// `FinalizeDeploy` must be cancel-cascaded (its `AfterOk` gate never opens) so +/// no `deployed/` tag is planted, while the tail still runs to compensate. +/// `Reconcile` is deliberately still reached — it boots the container back up. +#[test] +fn deploy_dag_skips_finalize_but_still_tails_a_failed_graft() { + let q = JobQueue::new(1); + let id = submit( + &q, + templates::approval_deploy("agent-a", 13, "approval 13".to_owned()), + ); + + let root = claim_one(&q); + q.complete_node(id, root.node_id, Ok(())); + let verify = claim_one(&q); + q.complete_node(id, verify.node_id, Ok(())); + let apply = claim_one(&q); + q.append_subgraph( + id, + &templates::deploy_rebuild_nodes("agent-a"), + apply.node_id, + ); + q.complete_node(id, apply.node_id, Ok(())); + + for expected in ["meta_sync", "prebuild", "stop_for_update"] { + let c = claim_one(&q); + assert_eq!(c.kind.as_str(), expected); + q.complete_node(id, c.node_id, Ok(())); + } + let swap = claim_one(&q); + assert_eq!(swap.kind.as_str(), "swap"); + q.complete_node(id, swap.node_id, Err("profile swap failed".into())); + + // `Reconcile` hangs off `Prebuild` with `AfterAny`, so a failed swap still + // reaches it — bringing the container back up is exactly what it's for. + let reconcile = claim_one(&q); + assert_eq!(reconcile.kind.as_str(), "reconcile"); + q.complete_node(id, reconcile.node_id, Ok(())); + + let tail = claim_one(&q); + assert!( + matches!(tail.kind, NodeKind::DeployTail { .. }), + "finalize is cancel-cascaded, so the tail is the next claimable node" + ); + q.complete_node(id, tail.node_id, Ok(())); + + let summary = q.terminal_summary(id).expect("dag terminal"); + assert_eq!(summary.state, State::Failed); + assert_eq!( + q.first_error(id).as_deref(), + Some("profile swap failed"), + "the tail annotates failed/ with this" + ); +} + /// A pre-merge rejection (drift gate, eval failure) cancel-cascades the /// irreversible half via its `AfterOk` edge, but the tail is still reached — /// it owns the forge mirror, not just compensation. @@ -1036,19 +1162,18 @@ fn set_step_only_on_running_and_signals_change() { fn set_build_log_id_links_running_node() { let q = JobQueue::new(1); let id = submit(&q, rebuild("agent-a", "r")); - assert!( - !q.set_build_log_id_running(id, 41), - "no running node yet → refused" - ); let c = claim_one(&q); assert!(q.set_build_log_id(id, c.node_id, 42)); - assert!(q.set_build_log_id_running(id, 43)); q.complete_node(id, c.node_id, Ok(())); + assert!( + !q.set_build_log_id(id, c.node_id, 99), + "node no longer running → refused" + ); // The log id is fetched by node id (the `GET /api/build-log/` lookup), // not carried on the wire — it survives completion in the node runtime. assert_eq!( q.build_log_id_of(c.node_id.get()), - Some(43), + Some(42), "log id survives completion" ); } diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index a1a2347f..f5818f23 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -246,9 +246,9 @@ pub async fn create_container(name: &str, hive: &HiveEnv, paths: &AgentPaths) -> create_only(name).await } -/// Rebuild-path preamble shared by the job queue's `Prebuild` node and -/// `rebuild_no_meta`: fail fast on a port collision, then make sure -/// the applied repo + state dirs exist. Container untouched. +/// Rebuild-path preamble, run by the job queue's `Prebuild` node: fail fast on +/// a port collision, then make sure the applied repo + state dirs exist. +/// Container untouched. pub async fn prepare_rebuild_dirs(name: &str, paths: &AgentPaths) -> Result<()> { validate(name)?; if let Some(other) = port_collision(name).await { @@ -331,19 +331,6 @@ pub async fn agents_for_meta_listing() -> Result> { agents_for_meta(None).await } -/// True when the named container already exists (appears in -/// `nixos-container list`). Used by the apply-commit path to decide -/// between first-spawn (`nixos-container create`) and normal rebuild -/// (`nixos-container update`). -pub async fn container_exists(name: &str) -> bool { - let container = container_name(name); - list() - .await - .unwrap_or_default() - .iter() - .any(|c| c == &container) -} - pub async fn kill(name: &str) -> Result<()> { validate(name)?; priv_run("stop", name).await @@ -453,9 +440,9 @@ async fn wait_until_running(name: &str, timeout: std::time::Duration) -> bool { } } -/// Internal implementation of the cold-start fallback. Used by -/// [`start_with_fallback`] (public, token-gated) and by -/// [`rebuild_no_meta`] where the preamble is already enforced structurally. +/// Internal implementation of the cold-start fallback, behind +/// [`start_with_fallback`] (public, token-gated) — the inner form exists for +/// callers that have already run the drop-in preamble themselves. /// /// [`start`] already treats unit-active (not the exit code) as success and /// waits out a slow boot, so this only layers the activation-error recovery on @@ -539,101 +526,6 @@ pub async fn destroy(name: &str) -> Result<()> { Ok(()) } -/// Container-level rebuild without touching the meta repo. The one -/// remaining fused stop/update/start pipeline: the approval deploy -/// (`actions::deploy_applied_target`) drives meta through the -/// two-phase prepare/finalize/abort flow itself and needs the inline -/// start to verify the agent comes back up before finalizing. Every -/// other rebuild is a job-queue DAG (`Prebuild → StopForUpdate → Swap -/// → Reconcile`) whose `Prebuild` executor owns the meta sync + -/// relock this path's deleted `rebuild` wrapper used to do. -/// -/// `on_step` is called at each phase boundary with a short human-readable -/// label so callers can surface progress (e.g. update the rebuild-queue -/// step shown in the dashboard). Pass `&|_| ()` when progress reporting -/// is not needed. -/// -/// `on_build_log_id` is called with the build-log row id immediately after -/// the `nixos-container update` log row opens, before the actual update -/// command starts. Callers can use this to link the queue entry to the log -/// for live streaming. Pass `&|_| ()` when not needed. -/// -/// `defer_start` skips the start-after-update for a previously-running -/// container and returns `true` instead, so a queue-side caller can hand -/// the (potentially slow) container boot to the fast lane rather than -/// holding the serialized build lane through it. With `defer_start = -/// false` the start (with cold-start fallback) runs inline as before and -/// the return value is always `false`. The spawn path always starts -/// inline — a freshly-created container boots as part of provisioning. -pub async fn rebuild_no_meta( - name: &str, - hive: &HiveEnv, - paths: &AgentPaths, - defer_start: bool, - on_step: &(dyn Fn(&str) + Send + Sync), - on_build_log_id: &(dyn Fn(i64) + Send + Sync), -) -> Result { - prepare_rebuild_dirs(name, paths).await?; - let flake_ref = format!("{}#{name}", crate::paths::meta_root().display()); - if container_exists(name).await { - // Rebuild strategy: stop-before-update + pre-build. - // See `docs/coordinator.md::Container lifecycle`. - let was_running = is_running(name).await; - write_dropins(name, hive, paths).await?; - if was_running { - on_step("nix build"); - prebuild_toplevel(name, &flake_ref, &|_| ()).await?; - on_step("nixos-container stop"); - priv_run("stop", name).await?; - } - on_step("nixos-container update"); - let update_result = priv_run_inner("update", name, Some(on_build_log_id)).await; - if let Err(ref update_err) = update_result { - // The update failed (e.g. nix build error). If the agent was - // running before we stopped it, try to bring it back up on the - // previous successful configuration so it doesn't stay dead. - // The start failure is logged but not promoted to an error — - // we always propagate the original update error (below). - if was_running { - tracing::warn!( - %name, - error = %update_err, - "nixos-container update failed; attempting restart on old config" - ); - on_step("nixos-container start (recovery)"); - if let Err(e) = priv_run("start", name).await { - tracing::warn!(%name, error = %e, "recovery start after failed update also failed"); - } - } - } - update_result?; - if was_running { - if defer_start { - // The caller re-queues the start on the fast lane so the - // build lane is freed for the next entry instead of - // waiting out the container boot here. - return Ok(true); - } - on_step("nixos-container start"); - // write_dropins was called above; use the inner fn directly - // since the preamble is enforced structurally in this path. - start_with_fallback_inner(name).await?; - } - Ok(false) - } else { - // Spawn path: create is atomic, no prebuild needed. - // See `docs/coordinator.md::Spawn path`. - on_step("nixos-container create"); - priv_run("create", name).await?; - // Runtime dir must exist before nixos-container start. - ensure_agent_runtime_dir(name)?; - write_dropins(name, hive, paths).await?; - on_step("nixos-container start"); - priv_run("start", name).await?; - Ok(false) - } -} - /// Pre-build `system.build.toplevel` against `meta#` so the /// subsequent `nixos-container update` finds the result cached and /// skips straight to the profile-swap. Store-warming only — container @@ -652,7 +544,7 @@ pub async fn prebuild_toplevel( use tokio::io::{AsyncBufReadExt, BufReader}; // Split `#` so we can re-emit with the explicit // `nixosConfigurations.` segment. The flake_ref shape is - // constructed by `rebuild_no_meta` and always contains exactly one + // constructed by the caller and always contains exactly one // `#`; `split_once` returning None here would be a programmer // error we'd want to surface loudly rather than paper over. let (flake_root, fragment) = flake_ref