diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 28a0f668..507c9dea 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -112,7 +112,6 @@ pub(crate) async fn run_terminal_hook(coord: &Arc, terminal: &super crate::actions::resolve_approval_dag(coord, terminal).await; } Some(super::HookKind::EmitRebuilt) => emit_rebuilt(coord, terminal), - Some(super::HookKind::RevertIntent) => revert_intent(coord, terminal).await, None => {} } } @@ -141,30 +140,6 @@ fn emit_rebuilt(coord: &Arc, terminal: &super::TerminalDag) { } } -/// Power-op hook: on a *cancelled* DAG, revert each targeted agent's `wanted` -/// intent to its observed state — the operator's cancel means "don't do it", so -/// the intent snaps back instead of the flip executing as a surprise side effect -/// of some later reconcile. Noop on any non-cancelled outcome. -/// -/// Only valid for templates carrying a `SetWanted` head (start / stop): the -/// revert writes *observed* state, so dispatching it for a template that never -/// wrote an intent doesn't restore anything — it invents one. See -/// [`super::terminal_hook`]. -async fn revert_intent(coord: &Arc, terminal: &super::TerminalDag) { - if terminal.state != State::Cancelled { - return; - } - for agent in &terminal.agents { - let running = crate::lifecycle::is_running(agent).await; - if let Err(e) = coord - .power - .set(agent, crate::power::Wanted::from_running(running)) - { - tracing::warn!(%agent, error = ?e, "agent_power: cancel revert failed"); - } - } -} - /// Write the agent's durable power intent — the DAG-node form of the old /// pre-submit `set_wanted` side effect. Store-only (no container touch), so /// build-slot-exempt; but it takes the agent's lifecycle lease (see diff --git a/hive-c0re/src/job_queue/mod.rs b/hive-c0re/src/job_queue/mod.rs index 17b8c7c0..6ebcc9f1 100644 --- a/hive-c0re/src/job_queue/mod.rs +++ b/hive-c0re/src/job_queue/mod.rs @@ -169,22 +169,18 @@ pub enum HookKind { ResolveApproval, /// Rebuild / perm-change: emit one `Rebuilt` manager event per agent. EmitRebuilt, - /// Intent-writing power-op: on a *cancelled* DAG, revert each agent's - /// `wanted` intent. Only for templates that actually carry a `SetWanted` - /// head — reverting an intent a DAG never wrote invents one. - RevertIntent, } /// The terminal hook a DAG needs, from its template + approval id — or `None` -/// for a DAG with no terminal side effect (meta-update, boot, bare reconcile). +/// for a DAG with no terminal side effect (power-op, meta-update, boot, bare +/// reconcile). /// -/// Restart is deliberately **not** a `RevertIntent` template. `restart_chain` -/// writes no `SetWanted` (it bounces the container and lets the tail -/// `Reconcile` converge to the agent's existing intent), so there is nothing -/// for a cancel to revert — and `revert_intent` writes the *observed* state, -/// which for a down-but-`wanted = Up` agent (crashed, or mid-bounce) would -/// flip it to `Offline` and keep it down. Cancelling a restart must leave the -/// intent exactly as it was found. +/// A cancelled DAG deliberately gets **no** compensating hook. [`JobQueue::cancel`] +/// refuses unless every work node is still `Pending`, and a cancel *cascade* +/// rolls up `Failed` (see `dag_rollup`), never `Cancelled` — so on a +/// `Cancelled` DAG no node ever executed and there is nothing to undo. A power +/// op's `SetWanted` head provably never ran, so its intent is still whatever +/// the operator last set it to. #[must_use] pub fn terminal_hook(template: Template, approval_id: Option) -> Option { if approval_id.is_some() { @@ -192,7 +188,6 @@ pub fn terminal_hook(template: Template, approval_id: Option) -> Option Some(HookKind::EmitRebuilt), - Template::Start | Template::Stop | Template::GracefulStop => Some(HookKind::RevertIntent), _ => None, } } diff --git a/hive-c0re/src/job_queue/scheduler.rs b/hive-c0re/src/job_queue/scheduler.rs index 1c2022f9..850d6bb2 100644 --- a/hive-c0re/src/job_queue/scheduler.rs +++ b/hive-c0re/src/job_queue/scheduler.rs @@ -10,10 +10,10 @@ //! held, so it appears when the agent's owner node starts and disappears when //! its subgraph settles — one pill per agent a DAG touches. //! -//! Per-DAG terminal work (approval resolution, `Rebuilt`, cancelled-power-op -//! intent revert) is not drained here: it runs as the DAG's focused terminal -//! node (`ResolveApproval` / `EmitRebuilt` / `RevertIntent`), dispatched through -//! `exec::run_node` like any other node once the DAG settles. +//! Per-DAG terminal work (approval resolution, `Rebuilt`) is not drained here: +//! it runs as the DAG's focused terminal node (`ResolveApproval` / +//! `EmitRebuilt`), dispatched through `exec::run_node` like any other node once +//! the DAG settles. //! //! In-DAG growth (a `MetaLock` growing rebuild subgraphs, a `Reconcile` fanning //! its `Start`/`Stop`) flows through `NodeOutput.append_subgraph`, applied diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index f0de064e..094af83a 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -863,49 +863,64 @@ fn cancel_refuses_running_dag() { assert_eq!(state_of(&q, id), State::Running); } -/// A cancelled restart must fire **no** intent revert. `restart_chain` writes -/// no `SetWanted`, so a cancel has nothing to roll back — and the revert hook -/// writes the agent's *observed* state, which for a down-but-`wanted = Up` -/// agent (crashed, or caught mid-bounce) would flip the intent to `Offline` -/// and leave it deliberately-stopped as far as reconcile and crash-watch are -/// concerned. Invisible for a running agent (observed == recorded), which is -/// why it went unnoticed; the window is exactly "observed ≠ intent", which is -/// when a restart is most likely to be issued and then cancelled. +/// A cancelled power op must fire **no** compensating hook — not even one that +/// carries a `SetWanted` head. +/// +/// `cancel` refuses unless every work node is still `Pending` +/// (`cancel_refuses_running_dag`) and a cancel *cascade* rolls up `Failed` +/// rather than `Cancelled`, so a `Cancelled` DAG provably never executed a +/// node: its `SetWanted` never ran and the agent's intent still reads whatever +/// the operator last set. A "revert" instead writes the agent's *observed* +/// state, which for a down-but-`wanted = Up` agent (crashed, or caught +/// mid-bounce) flips the intent to `Offline` and leaves it +/// deliberately-stopped as far as reconcile and crash-watch are concerned. #[test] -fn cancelled_restart_reverts_no_intent() { +fn cancelled_power_op_fires_no_hook() { for graceful in [false, true] { for running in [false, true] { - let q = JobQueue::new(1); let targets = vec![("agent-a".to_owned(), running)]; - let spec = - submit::restart_spec(&targets, graceful, Source::Manual, "bounce".to_owned()); - assert!( - !spec - .nodes - .iter() - .any(|n| matches!(n.kind, NodeKind::SetWanted { .. })), - "restart writes no intent (graceful={graceful}, running={running})" - ); - let id = submit(&q, spec); - let summary = q.cancel(id).expect("cancelled while queued"); - assert_eq!(summary.state, State::Cancelled); - assert_eq!( - terminal_hook(summary.template, summary.approval_id), - None, - "cancelled restart (graceful={graceful}, running={running}) must \ - not revert an intent it never wrote" - ); + let cases = [ + ( + "restart", + false, + submit::restart_spec(&targets, graceful, Source::Manual, "bounce".to_owned()), + ), + ( + "stop", + true, + submit::stop_spec(&targets, graceful, Source::Manual, "stop".to_owned()), + ), + ( + "start", + true, + submit::start_spec( + &[("agent-a".to_owned(), running, false)], + Source::Manual, + "start".to_owned(), + ), + ), + ]; + for (name, writes_intent, spec) in cases { + assert_eq!( + spec.nodes + .iter() + .any(|n| matches!(n.kind, NodeKind::SetWanted { .. })), + writes_intent, + "{name} intent head (graceful={graceful}, running={running})" + ); + let q = JobQueue::new(1); + let id = submit(&q, spec); + let summary = q.cancel(id).expect("cancelled while queued"); + assert_eq!(summary.state, State::Cancelled); + assert_eq!( + terminal_hook(summary.template, summary.approval_id), + None, + "cancelled {name} (graceful={graceful}, running={running}) must \ + fire no hook — no node of it ever ran" + ); + } } } - // Contrast: stop *does* carry a `SetWanted` head, so its cancel still has a - // real intent flip to undo. - let q = JobQueue::new(1); - let id = submit(&q, stop_online(&["agent-a"], false, "stop")); - let summary = q.cancel(id).expect("cancelled while queued"); - assert_eq!( - terminal_hook(summary.template, summary.approval_id), - Some(HookKind::RevertIntent) - ); } // ---- terminal reporting + lease release ----