From 6ce71556e274aa703ab37273dfe8f236073339c2 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 3 Aug 2026 17:47:07 +0200 Subject: [PATCH] fix(#2971): thread the queue node id into nix_logged `nix_logged` wrote its `build_logs` row with `node_id = None`, so a deploy's eval/relock log was reachable from the agent+kind+time listing but not from the queue node that ran it. The comment justifying the `None` said both callers are "reached from outside the queue as well as from inside it". That is no longer true: `verify_commit` and `prepare_deploy` have exactly one caller each, and both sit under the `MergeVerify` / `DeployApply` arms of `exec.rs`'s node dispatch, where the `NodeId` is already in scope. Threads `Option` from the dispatch down, mirroring `prebuild_toplevel`'s existing `Some(id.get())` at exec.rs:241. Kept as `Option` rather than a bare `u64` because `meta::prepare_deploy` / `meta::verify_commit` are public API and a future non-queue caller has no node to name; the comment now says that instead of the stale claim. No behaviour change beyond the log row gaining its node link. --- hive-c0re/src/actions.rs | 28 +++++++++++++++++++++------- hive-c0re/src/job_queue/exec.rs | 14 ++++++++------ hive-c0re/src/meta.rs | 31 ++++++++++++++++++++++--------- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index 61ee3618..83f5bd07 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -213,7 +213,11 @@ fn deploy_ctx(coord: &Coordinator, approval_id: i64) -> Result { /// fails, if the reviewed head does not descend from `applied/main`, or if the /// eval-verify of the reviewed commit fails. Every one of these leaves the forge /// and `main` untouched, so the node is safely retryable. -pub async fn run_deploy_merge_verify(coord: &Arc, approval_id: i64) -> Result<()> { +pub async fn run_deploy_merge_verify( + coord: &Arc, + approval_id: i64, + node_id: Option, +) -> Result<()> { let ctx = deploy_ctx(coord, approval_id)?; let pr = ctx.pr; let reviewed = ctx.reviewed.as_str(); @@ -250,9 +254,14 @@ pub async fn run_deploy_merge_verify(coord: &Arc, approval_id: i64) } // 4. Eval-verify BEFORE the irreversible merge (bad nix fails fast here). - crate::meta::verify_commit(ctx.approval.agent.as_str(), &ctx.applied_dir, reviewed) - .await - .map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?; + crate::meta::verify_commit( + ctx.approval.agent.as_str(), + &ctx.applied_dir, + reviewed, + node_id, + ) + .await + .map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?; Ok(()) } @@ -275,7 +284,11 @@ pub async fn run_deploy_merge_verify(coord: &Arc, approval_id: i64) /// 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, approval_id: i64) -> Result<()> { +pub async fn run_deploy_apply( + coord: &Arc, + approval_id: i64, + node_id: Option, +) -> Result<()> { let ctx = deploy_ctx(coord, approval_id)?; let agent = ctx.approval.agent.as_str(); let pr = ctx.pr; @@ -300,7 +313,7 @@ pub async fn run_deploy_apply(coord: &Arc, approval_id: i64) -> Res Err(e) => bail!("ff-merge PR #{pr}: {e}"), } - prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed, &prev_main).await + prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed, &prev_main, node_id).await } /// `DeployTail` node body — compensation + bookkeeping, `AfterAny` the apply @@ -817,6 +830,7 @@ async fn prepare_applied_target( applied_dir: &std::path::Path, target: &str, expected_main: &str, + node_id: Option, ) -> Result<()> { // Fast-forward applied/main to target + sync the working tree. Meta input // pins `?ref=main`, so this is what makes nix re-lock to the target commit @@ -840,7 +854,7 @@ async fn prepare_applied_target( // 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) + crate::meta::prepare_deploy(agent, node_id) .await .map_err(|e| anyhow::anyhow!("meta prepare_deploy: {e:#}")) } diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 1fa2b382..a7f30146 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -103,9 +103,11 @@ pub(super) async fn run_node( // that could never fire. NodeKind::WritePermFile { payload, .. } => run_write_perm_file(coord, agent, payload).await, NodeKind::Reparent { moves } => run_reparent(coord, moves).await, - NodeKind::MergeVerify { approval_id, .. } => run_merge_verify(coord, *approval_id).await, + NodeKind::MergeVerify { approval_id, .. } => { + run_merge_verify(coord, *approval_id, id).await + } NodeKind::DeployApply { approval_id, .. } => { - run_deploy_apply(coord, *approval_id).await.map(|()| { + run_deploy_apply(coord, *approval_id, id).await.map(|()| { super::templates::deploy_rebuild_nodes(&builder, agent, *approval_id); }) } @@ -574,8 +576,8 @@ async fn run_reparent( /// Deploy phase 1 — drift gate, fetch, eval-verify. Mutates nothing, so a /// failure here cancel-cascades the rest of the subtree with the forge and the /// applied repo exactly as they were. -async fn run_merge_verify(coord: &Arc, approval_id: i64) -> Result<()> { - crate::actions::run_deploy_merge_verify(coord, approval_id).await +async fn run_merge_verify(coord: &Arc, approval_id: i64, id: NodeId) -> Result<()> { + crate::actions::run_deploy_merge_verify(coord, approval_id, Some(id.get())).await } /// Deploy phase 2 — the irreversible half: ff-merge, then phase 1 of the @@ -587,8 +589,8 @@ async fn run_merge_verify(coord: &Arc, approval_id: i64) -> Result< /// their `MetaSync` declares is re-entered rather than deadlocked against the /// ancestor already holding it. On failure nothing is appended and the tail /// compensates, exactly as before. -async fn run_deploy_apply(coord: &Arc, approval_id: i64) -> Result<()> { - crate::actions::run_deploy_apply(coord, approval_id).await +async fn run_deploy_apply(coord: &Arc, approval_id: i64, id: NodeId) -> Result<()> { + crate::actions::run_deploy_apply(coord, approval_id, Some(id.get())).await } /// Deploy phase 3 — close the staged-lock window once the appended rebuild has diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 6da608f3..cff3b067 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -243,7 +243,7 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { /// the index. Doesn't commit — `finalize_deploy` commits on build /// success, `abort_deploy` drops the staged change on failure so /// meta history only carries successful deploys. -pub async fn prepare_deploy(name: &str) -> Result<()> { +pub async fn prepare_deploy(name: &str, node_id: Option) -> Result<()> { let _guard = META_LOCK.lock().await; let dir = crate::paths::meta_root(); let input = format!("agent-{name}"); @@ -268,6 +268,7 @@ pub async fn prepare_deploy(name: &str) -> Result<()> { ], name, "prepare-deploy", + node_id, ) .await?; // Stage the new lock — git+file://'s dirty-tree fetcher reads @@ -375,7 +376,12 @@ fn applied_override_url(applied_dir: &Path) -> String { /// resolution at the same cost profile as the legacy pre-merge check; /// the real container build still runs (and can still roll back) on the /// actual apply, so this is the right "would this apply" gate. -pub async fn verify_commit(name: &str, applied_dir: &Path, sha: &str) -> Result<()> { +pub async fn verify_commit( + name: &str, + applied_dir: &Path, + sha: &str, + node_id: Option, +) -> Result<()> { let _guard = META_LOCK.lock().await; let dir = crate::paths::meta_root(); let input = format!("agent-{name}"); @@ -393,6 +399,7 @@ pub async fn verify_commit(name: &str, applied_dir: &Path, sha: &str) -> Result< ], name, "verify", + node_id, ) .await } @@ -1586,16 +1593,22 @@ async fn nix(dir: &Path, args: &[&str]) -> Result<()> { /// zero build logs to look at. Best-effort logging: a missing global /// handle or a failed `start()` just skips the row — the command still /// runs and its exit status is still enforced. -async fn nix_logged(dir: &Path, args: &[&str], agent: &str, kind: &str) -> Result<()> { +async fn nix_logged( + dir: &Path, + args: &[&str], + agent: &str, + kind: &str, + node_id: Option, +) -> Result<()> { let cmdline = format!("nix {}", nix_argv(args).join(" ")); let logs = crate::build_logs::global(); let log_id = logs.as_ref().and_then(|h| { - // No node id: `nix_logged`'s two callers are meta-flake operations - // reached from outside the queue as well as from inside it, and the - // agent+kind+time listing is how they're surfaced today. Linking them - // to a node would mean threading the id through `meta`'s public API - // for no current reader — worth doing when something wants it. - h.start(agent, kind, &cmdline, None) + // `node_id` links the row to the queue node that ran it, so the + // dashboard can reach this log from the node instead of only from the + // agent+kind+time listing. Both call paths are queue-only today and + // pass `Some`; it stays an `Option` because these are `meta`'s public + // API and a future non-queue caller has no node to name. + h.start(agent, kind, &cmdline, node_id) .map_err(|e| { tracing::warn!(error = ?e, %kind, "build_logs: start failed (meta log dropped)"); })