job_queue: retire the now-off-wire step sub-step label
The `step` label was taken off the wire in #2661, when each deploy phase became a first-class DAG node. Since then it has been written but never read: `NodeRuntime` derives only `Debug, Default, Clone` — no serde — so the field could not reach any client, and the only reads of it were the dedup checks inside its own setters. This deletes the machinery. Removed: - `NodeRuntime.step`, `set_step`, `set_step_running`, and the `rt.step = None` clear in `complete_node`. `NodeRuntime` keeps its remaining `build_log_id` field (deliberately still a struct — collapsing it to a bare `Option<i64>` would churn every call site for no gain). - `Ctx::step` and its ~15 call sites in `job_queue/exec.rs`. `Ctx` itself stays: it is the build-log sink, which `run_prebuild` and `run_swap` still use. - `Coordinator::set_queue_step` and its 11 callers in `actions.rs`. - `JobQueue::running_node_of`, reachable only from `set_queue_step`. - `swap_update`'s `on_step` parameter and its one body call. - The `set_step_only_on_running_and_signals_change` test. Dropping the calls orphaned parameters, which are removed with their call sites: `ctx` on ten executors that used it only as a step sink, and `queue_entry_id` on `run_deploy_merge_verify` / `run_deploy_apply` / `run_finalize_deploy` plus both `coord` and `queue_entry_id` on `prepare_applied_target`. `run_deploy_tail` KEEPS its `queue_entry_id` — that one has a genuine surviving use (the build-log link in the failure comment posted to the PR). One behavioural change, called out so it is not mistaken for a dropped dashboard refresh: `Ctx::step` and `set_queue_step` each emitted a `rebuild_queue_changed` snapshot when the label changed, and those emissions go away with them. This is safe — the snapshot payload has no step field, so those pushes carried nothing a client could observe. Real state transitions still emit from the scheduler's claim and completion paths, from `submit`, and from the three `actions.rs` sites. Net effect is strictly fewer redundant SSE pushes. Docs: `docs/coordinator.md` still listed `step` as a `NodeView` wire field and `docs/web-ui/dashboard.md` documented a cyan `↳ <step>` sub-line under each queue row. Neither has existed since #2661 — both corrected here, plus the `job_queue/model.rs` module doc. Not touched: `frontend/packages/dashboard/src/system-sections.css` has a dead `.rqe-step` rule with no JS referencing it. Left for the frontend owner rather than deleted here. Closes: #2664
This commit is contained in:
parent
ff62bf2235
commit
1db3cc32a1
9 changed files with 51 additions and 210 deletions
|
|
@ -211,17 +211,12 @@ fn deploy_ctx(coord: &Coordinator, approval_id: i64) -> Result<DeployCtx> {
|
|||
/// fails, 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<Coordinator>,
|
||||
queue_entry_id: Option<u64>,
|
||||
approval_id: i64,
|
||||
) -> Result<()> {
|
||||
pub async fn run_deploy_merge_verify(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
||||
let ctx = deploy_ctx(coord, approval_id)?;
|
||||
let pr = ctx.pr;
|
||||
let reviewed = ctx.reviewed.as_str();
|
||||
|
||||
// 1. Drift gate: the live PR head must still equal what was reviewed.
|
||||
coord.set_queue_step(queue_entry_id, "verify PR head");
|
||||
let head = crate::forge::pr_head_sha(&ctx.repo, pr)
|
||||
.await
|
||||
.map_err(|e| anyhow::anyhow!("read PR #{pr} head: {e}"))?;
|
||||
|
|
@ -232,13 +227,11 @@ pub async fn run_deploy_merge_verify(
|
|||
}
|
||||
|
||||
// 2. Fetch the reviewed head into applied so ff/verify/deploy resolve it.
|
||||
coord.set_queue_step(queue_entry_id, "fetch PR head");
|
||||
crate::forge::fetch_pr_head_into_applied(&ctx.repo, pr)
|
||||
.await
|
||||
.map_err(|e| anyhow::anyhow!("fetch PR #{pr} head into applied: {e}"))?;
|
||||
|
||||
// 3. Eval-verify BEFORE the irreversible merge (bad nix fails fast here).
|
||||
coord.set_queue_step(queue_entry_id, "verify proposal (eval)");
|
||||
crate::meta::verify_commit(ctx.approval.agent.as_str(), &ctx.applied_dir, reviewed)
|
||||
.await
|
||||
.map_err(|e| anyhow::anyhow!("verify merge head {reviewed}: {e:#}"))?;
|
||||
|
|
@ -264,11 +257,7 @@ pub async fn run_deploy_merge_verify(
|
|||
/// 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<Coordinator>,
|
||||
queue_entry_id: Option<u64>,
|
||||
approval_id: i64,
|
||||
) -> Result<()> {
|
||||
pub async fn run_deploy_apply(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
||||
let ctx = deploy_ctx(coord, approval_id)?;
|
||||
let agent = ctx.approval.agent.as_str();
|
||||
let pr = ctx.pr;
|
||||
|
|
@ -285,7 +274,6 @@ pub async fn run_deploy_apply(
|
|||
// both advances `main` to the reviewed head and marks the PR merged — no
|
||||
// direct push to the protected branch. A failure here means `main` was NOT
|
||||
// advanced, so it's fatal: we must not deploy a head the forge didn't merge.
|
||||
coord.set_queue_step(queue_entry_id, "fast-forward-merge PR");
|
||||
match crate::forge::merge_config_pr_ff(&ctx.repo, pr, &ctx.reviewed).await {
|
||||
Ok(()) => {}
|
||||
Err(crate::forge::ForgeMergeError::HeadDrift { expected, actual }) => bail!(
|
||||
|
|
@ -294,14 +282,7 @@ pub async fn run_deploy_apply(
|
|||
Err(e) => bail!("ff-merge PR #{pr}: {e}"),
|
||||
}
|
||||
|
||||
prepare_applied_target(
|
||||
coord,
|
||||
agent,
|
||||
&ctx.applied_dir,
|
||||
&ctx.reviewed,
|
||||
queue_entry_id,
|
||||
)
|
||||
.await
|
||||
prepare_applied_target(agent, &ctx.applied_dir, &ctx.reviewed).await
|
||||
}
|
||||
|
||||
/// `DeployTail` node body — compensation + bookkeeping, `AfterAny` the apply
|
||||
|
|
@ -371,7 +352,6 @@ pub async fn run_deploy_tail(
|
|||
}
|
||||
}
|
||||
|
||||
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
|
||||
{
|
||||
|
|
@ -389,7 +369,6 @@ pub async fn run_deploy_tail(
|
|||
}
|
||||
}
|
||||
|
||||
coord.set_queue_step(queue_entry_id, "forge push");
|
||||
if let Err(e) = crate::forge::push_config(agent).await {
|
||||
tracing::warn!(%agent, error = ?e, "forge: push_config after merge failed");
|
||||
}
|
||||
|
|
@ -789,13 +768,10 @@ fn finish_approval(
|
|||
/// (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 prepare_applied_target(
|
||||
coord: &Arc<Coordinator>,
|
||||
agent: &str,
|
||||
applied_dir: &std::path::Path,
|
||||
target: &str,
|
||||
queue_entry_id: Option<u64>,
|
||||
) -> Result<()> {
|
||||
coord.set_queue_step(queue_entry_id, "fast-forward applied/main");
|
||||
// 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
|
||||
// on the prepare_deploy step below.
|
||||
|
|
@ -806,7 +782,6 @@ async fn prepare_applied_target(
|
|||
.await
|
||||
.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. 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.
|
||||
|
|
@ -835,27 +810,20 @@ async fn prepare_applied_target(
|
|||
/// `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<Coordinator>,
|
||||
queue_entry_id: Option<u64>,
|
||||
approval_id: i64,
|
||||
) -> Result<()> {
|
||||
pub async fn run_finalize_deploy(coord: &Arc<Coordinator>, approval_id: i64) -> Result<()> {
|
||||
let ctx = deploy_ctx(coord, approval_id)?;
|
||||
let agent = ctx.approval.agent.as_str();
|
||||
let target = ctx.reviewed.as_str();
|
||||
|
||||
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");
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue