diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index 085d2bf7..cdf84e88 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -899,99 +899,20 @@ pub async fn run_finalize_deploy(coord: &Arc, approval_id: i64) -> /// imperative infra that `auto_update::ensure_root_agent` recreates on the /// next hive-c0re startup if absent, so destroying it is transient rather /// than something to refuse at the API. -pub async fn destroy(coord: &Arc, name: &str, purge: bool) -> Result<()> { +/// +/// Submits the teardown DAG and returns — it does not wait for the container to +/// go away. Same contract as every other lifecycle op (`rebuild`, `kill`, +/// `restart`, `start`): the queue owns the work, the caller gets an +/// acknowledgement. Progress is visible as real nodes on the dashboard. +pub fn destroy(coord: &Arc, name: &str, purge: bool) { tracing::info!(%name, purge, "destroy"); - // Guard auto-clears on the success path's final scope exit and on - // every early-return / cancellation along the way. - // Destroy has no queue node behind it, so nothing in the graph says this - // container is going away on purpose — without this the crash watcher - // reports every destroy as a crash and the manager tries to recover it. - let guard = coord.suppress_crash_watch(name); - lifecycle::destroy(name).await?; - coord.unregister_agent(name); - let runtime = crate::paths::agent_runtime_dir(name); - if runtime.exists() { - let _ = std::fs::remove_dir_all(&runtime); + if let Err(e) = coord.job_queue.insert_job(|b| { + crate::job_queue::templates::destroy(b, name, purge); + Vec::new() + }) { + tracing::error!(agent = %name, error = ?e, "destroy: insert failed"); } - if purge { - // The state root may be a btrfs subvolume: a subvolume root - // can't be removed with rmdir/`remove_dir_all`, so delete it via - // hive-priv (root) first. No-op for plain-dir agents — the loop below - // then handles the plain-dir state root plus the applied dir. - if let Err(e) = crate::priv_client::delete_agent_subvolume(name).await { - tracing::warn!(error = ?e, %name, "purge: delete state subvolume failed"); - } - // A malformed name can't have a persistent state tree (the state dir - // is only ever created under a validated Ident), so its removal is a - // no-op — skip the state-dir sweep and just clear the applied dir. - let state_dir = hive_types::Ident::parse(name) - .ok() - .map(|id| crate::paths::agent_state_dir(&id)); - for dir in state_dir - .into_iter() - .chain([crate::paths::applied_dir(name)]) - { - if dir.exists() - && let Err(e) = std::fs::remove_dir_all(&dir) - { - tracing::warn!(error = ?e, dir = %dir.display(), "purge: remove failed"); - } - } - } - // Meta flake: drop the agent's input + nixosConfiguration so a - // future spawn under the same name re-seeds cleanly, and so the - // meta lock doesn't reference a vanished applied repo. Log + keep - // going on failure — destroy already succeeded at the - // nixos-container level, the meta repo is just bookkeeping. - if let Err(e) = sync_meta_after_lifecycle(coord).await { - tracing::warn!(error = ?e, %name, "meta sync after destroy failed"); - } - let _ = coord.approvals.fail_pending_for_agent( - name, - if purge { - "agent purged" - } else { - "agent destroyed" - }, - ); - // Drop the durable power intent — a future agent of the same name - // seeds fresh from its observed state. - if let Err(e) = coord.power.remove(name) { - tracing::warn!(%name, error = ?e, "agent_power: remove on destroy failed"); - } - drop(guard); - let _ = coord - .push_todo( - hive_sh4re::manager::MANAGER_AGENT, - "core", - Some(format!("destroyed:{name}")), - format!("agent '{name}' destroyed"), - None, - false, - ) - .await; - // Container row disappeared — rescan so the dashboard fires - // `ContainerRemoved` for the gone row, then emit the - // tombstones snapshot (gained one on destroy, lost one on - // purge — recompute either way). - coord.rescan_containers_and_emit().await; - crate::dashboard::emit_tombstones_snapshot(coord).await; - // Re-emit the schedules snapshot: the rescan above refreshed the live - // roster, so any schedule that still targets the just-destroyed agent - // now drops that ghost column live (no page reload needed). - coord.emit_schedules_snapshot(); - // Update tmpfiles.d to remove the destroyed agent's dirs from the - // boot-time pre-creation list. Best-effort: failure is logged only. - tokio::spawn(lifecycle::sync_tmpfiles()); - Ok(()) -} - -/// Rerender the meta flake from whatever containers still exist on -/// disk. Called after lifecycle ops that change the agent set (today: -/// destroy). Idempotent — a no-op when nothing changed. -async fn sync_meta_after_lifecycle(coord: &Coordinator) -> Result<()> { - let agents = lifecycle::agents_for_meta_listing().await?; - crate::meta::sync_agents(&coord.hive_env(), &agents).await + coord.emit_rebuild_queue_snapshot(); } pub fn deny(coord: &Coordinator, id: i64, note: Option<&str>) -> Result<()> { diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 8c227a16..7be394d5 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -116,13 +116,6 @@ pub struct Coordinator { /// is never injected into containers. pub model_prices: crate::hive_stats::PriceTable, agents: Mutex>, - /// Agents whose lifecycle action (currently just spawn) is in flight. - /// Read by the dashboard to render a spinner; cleared when the action - /// resolves (success or failure). - /// Agents whose container is being taken down by work with **no queue node - /// behind it** (destroy, migration), so the crash watcher must not report - /// the disappearance as a crash. Not a pill — see [`CrashWatchSuppression`]. - crash_suppressed: Mutex>, /// Tombstone for transients that have JUST been cleared. The /// crash watcher polls every 10s and would race the /// drop-clears-immediately path of `TransientGuard`: an operator @@ -400,57 +393,6 @@ fn fold_tombstones_by_agent<'a>( out } -/// Tombstone label for work with **no queue node behind it** — the -/// out-of-band operations (destroy, migration) that hold a -/// [`CrashWatchGuard`] instead of appearing in the derived transient set. -/// -/// [`Coordinator::recent_transient`] is keyed by `(agent, label)` so concurrent -/// pills can't overwrite each other's `deliberate_stop`; a guard has no node and -/// therefore no node label, so it needs one of its own. Angle-bracketed to keep -/// it out of the `NodeKind::as_str` namespace — no node can ever render this. -const NO_NODE_LABEL: &str = ""; - -/// RAII handle returned by [`Coordinator::suppress_crash_watch`]. While held, -/// the crash watcher treats this container disappearing as **expected**. -/// -/// This is *not* a dashboard pill. Transients are derived from running queue -/// nodes and nothing stores them. But destroy and migration take a container -/// down without a node behind them, so nothing in the graph says the -/// disappearance was intended — and without that, `crash_watch` fires a -/// `ContainerCrash` for every destroy and every migrated agent, and the manager -/// tries to "recover" containers that were removed on purpose. -/// -/// It is held rather than stamped once because -/// [`crate::workers::crash_watch`]'s grace window is finite and these -/// operations are not: a long destroy would outlive a single tombstone. The -/// tombstone is stamped on drop, covering the poll that lands just after. -/// -/// Goes away entirely once destroy + migration are real queue nodes. -#[must_use = "suppression lasts as long as the guard; bind it for the operation's duration \ - (`let _guard = coord.suppress_crash_watch(...)`). An unbound call drops it \ - immediately and the very next poll can report a deliberate stop as a crash."] -pub struct CrashWatchSuppression { - coord: Arc, - name: String, -} - -impl Drop for CrashWatchSuppression { - fn drop(&mut self) { - self.coord - .crash_suppressed - .lock() - .unwrap() - .remove(&self.name); - // Tombstone the release so the next poll — which may land in the - // window between the container going away and this guard dropping — - // still reads the stop as deliberate. - self.coord.recent_transient.lock().unwrap().insert( - (self.name.clone(), NO_NODE_LABEL.to_owned()), - (true, std::time::Instant::now()), - ); - } -} - /// RAII guard for the `meta-update` in-progress flag, held for the /// duration of a `run_meta_update` background task. Created by /// `Coordinator::meta_update_guard`. Drop decrements the active-run @@ -586,7 +528,6 @@ impl Coordinator { agent_io_weight, model_prices, agents: Mutex::new(HashMap::new()), - crash_suppressed: Mutex::new(HashSet::new()), recent_transient: Mutex::new(HashMap::new()), recent_crashes: Mutex::new(HashMap::new()), graceful_stop_pending: Mutex::new(HashSet::new()), @@ -1300,35 +1241,6 @@ impl Coordinator { map.iter().map(|(k, v)| (k.clone(), v.len())).collect() } - /// Tell the crash watcher that `name`'s container is going down **on - /// purpose**, for the lifetime of the returned guard. See - /// [`CrashWatchSuppression`] for why this exists at all. - /// - /// Only for the operations with no queue node behind them. Anything the - /// job queue runs answers this from the node itself - /// ([`crate::job_queue::NodeKind::takes_container_down`]) and must not come - /// through here. - /// - /// The guard's `Drop` runs even on task cancellation, so an aborted HTTP - /// request or a panic mid-destroy can't leave a container permanently - /// exempt from crash reporting. - pub fn suppress_crash_watch(self: &Arc, name: &str) -> CrashWatchSuppression { - self.crash_suppressed - .lock() - .unwrap() - .insert(name.to_owned()); - CrashWatchSuppression { - coord: self.clone(), - name: name.to_owned(), - } - } - - /// Whether a no-node operation is currently taking this container down. - #[must_use] - pub fn crash_watch_suppressed(&self, name: &str) -> bool { - self.crash_suppressed.lock().unwrap().contains(name) - } - /// Every live transient, keyed by agent. /// /// **Derived on read, stored nowhere.** Straight off the running graph, so diff --git a/hive-c0re/src/dashboard/lifecycle_ops.rs b/hive-c0re/src/dashboard/lifecycle_ops.rs index e12723fa..855c2830 100644 --- a/hive-c0re/src/dashboard/lifecycle_ops.rs +++ b/hive-c0re/src/dashboard/lifecycle_ops.rs @@ -436,10 +436,9 @@ pub(super) struct DestroyForm { params(("name" = String, Path, description = "agent name")), request_body(content = DestroyForm, content_type = "application/x-www-form-urlencoded"), responses( - (status = 200, description = "destroyed", body = String), + (status = 200, description = "destroy queued", body = String), (status = 400, description = "bad agent name"), (status = 404, description = "no such agent"), - (status = 500, description = "destroy failed"), ), tag = "lifecycle_ops" )] @@ -453,11 +452,10 @@ pub(super) async fn post_destroy( } // Checkbox semantics: any non-empty value (axum sends "on") = purge. let purge = form.purge.as_deref().is_some_and(|v| !v.is_empty()); - // `actions::destroy` rescans the container list on success, so the - // `ContainerRemoved` event lands before we return 200. The matching - // form carries `data-no-refresh`. - match actions::destroy(&state.coord, &name, purge).await { - Ok(()) => (StatusCode::OK, "ok").into_response(), - Err(e) => error_response(&format!("destroy {name} failed: {e:#}")), - } + // Submit-and-return, like every other lifecycle endpoint here. The + // container rescan now runs in the DAG's bookkeeping tail, so + // `ContainerRemoved` arrives *after* this 200 rather than before it — the + // row disappears when the event lands, same as a rebuild's does. + actions::destroy(&state.coord, &name, purge); + (StatusCode::OK, "ok").into_response() } diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index d392bd78..9efcf81c 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -73,6 +73,15 @@ pub(super) async fn run_node( NodeKind::RebuildBookkeeping { .. } => run_rebuild_bookkeeping(coord, agent).await, NodeKind::Provision { .. } => run_provision(coord, agent).await, NodeKind::Create { .. } => run_create(agent).await, + NodeKind::DestroyContainer { .. } => run_destroy_container(coord, agent).await, + NodeKind::PurgeState { .. } => { + run_purge_state(agent).await; + Ok(()) + } + NodeKind::DestroyBookkeeping { purge, .. } => { + run_destroy_bookkeeping(coord, agent, *purge).await; + Ok(()) + } NodeKind::MetaLock { sweep, fanout, @@ -163,6 +172,112 @@ async fn run_resolve_approval( Ok(()) } +/// Rerender the meta flake from whatever containers still exist on disk. +/// Idempotent — a no-op when nothing changed. Lives here because the destroy +/// tail is its only caller; it moved with `destroy` when that became a DAG. +async fn sync_meta_after_lifecycle(coord: &Coordinator) -> Result<()> { + let agents = crate::lifecycle::agents_for_meta_listing().await?; + crate::meta::sync_agents(&coord.hive_env(), &agents).await +} + +/// `nixos-container destroy`, then drop the agent from the roster and clear +/// its ephemeral runtime dir (the mcp socket, which does not survive a restart +/// anyway). +/// +/// The only fallible step is the destroy itself: once the container is gone the +/// un-registration cannot meaningfully fail, and returning early would strand +/// the roster claiming an agent that no longer exists. +async fn run_destroy_container(coord: &Arc, agent: &str) -> Result<()> { + crate::lifecycle::destroy(agent).await?; + coord.unregister_agent(agent); + let runtime = crate::paths::agent_runtime_dir(agent); + if runtime.exists() { + let _ = std::fs::remove_dir_all(&runtime); + } + Ok(()) +} + +/// The `purge = true` half: wipe the agent's persistent trees. +/// +/// Every step is best-effort-with-a-warning rather than fatal, and that is +/// deliberate — the container is already destroyed by the time this runs, so +/// failing the node would leave the operator with a half-purged agent and a red +/// DAG, when what they can actually act on is the log line naming the path. +async fn run_purge_state(agent: &str) { + // The state root may be a btrfs subvolume: a subvolume root can't be + // removed with rmdir/`remove_dir_all`, so delete it via hive-priv (root) + // first. No-op for plain-dir agents — the loop below then handles the + // plain-dir state root plus the applied dir. + if let Err(e) = crate::priv_client::delete_agent_subvolume(agent).await { + tracing::warn!(error = ?e, %agent, "purge: delete state subvolume failed"); + } + // A malformed name can't have a persistent state tree (the state dir is + // only ever created under a validated Ident), so its removal is a no-op — + // skip the state-dir sweep and just clear the applied dir. + let state_dir = hive_types::Ident::parse(agent) + .ok() + .map(|id| crate::paths::agent_state_dir(&id)); + for dir in state_dir + .into_iter() + .chain([crate::paths::applied_dir(agent)]) + { + if dir.exists() + && let Err(e) = std::fs::remove_dir_all(&dir) + { + tracing::warn!(error = ?e, dir = %dir.display(), "purge: remove failed"); + } + } +} + +/// Post-destroy bookkeeping. Infallible by construction: every step is +/// warn-and-continue, because the destroy it follows has already succeeded and +/// none of this is undoable — a failed meta sync or power-store write is a +/// bookkeeping drift to log, not a reason to red a DAG whose container is +/// already gone. +async fn run_destroy_bookkeeping(coord: &Arc, agent: &str, purge: bool) { + // Meta flake: drop the agent's input + nixosConfiguration so a future spawn + // under the same name re-seeds cleanly, and so the meta lock doesn't + // reference a vanished applied repo. + if let Err(e) = sync_meta_after_lifecycle(coord).await { + tracing::warn!(error = ?e, %agent, "meta sync after destroy failed"); + } + let _ = coord.approvals.fail_pending_for_agent( + agent, + if purge { + "agent purged" + } else { + "agent destroyed" + }, + ); + // Drop the durable power intent — a future agent of the same name seeds + // fresh from its observed state. + if let Err(e) = coord.power.remove(agent) { + tracing::warn!(%agent, error = ?e, "agent_power: remove on destroy failed"); + } + let _ = coord + .push_todo( + hive_sh4re::manager::MANAGER_AGENT, + "core", + Some(format!("destroyed:{agent}")), + format!("agent '{agent}' destroyed"), + None, + false, + ) + .await; + // Container row disappeared — rescan so the dashboard fires + // `ContainerRemoved` for the gone row, then emit the tombstones snapshot + // (gained one on destroy, lost one on purge — recompute either way). + coord.rescan_containers_and_emit().await; + crate::dashboard::emit_tombstones_snapshot(coord).await; + // Re-emit the schedules snapshot: the rescan above refreshed the live + // roster, so any schedule that still targets the just-destroyed agent now + // drops that ghost column live (no page reload needed). + coord.emit_schedules_snapshot(); + // Update tmpfiles.d to remove the destroyed agent's dirs from the boot-time + // pre-creation list. Best-effort: failure is logged only. + tokio::spawn(crate::lifecycle::sync_tmpfiles()); +} + /// Emit this agent's rebuild-complete todo. `ok` is not computed — it is which /// of the tail pair the graph let run. The failure note comes from the DAG's /// first failing node, since the branch knows *that* it failed but not *why*. diff --git a/hive-c0re/src/job_queue/model.rs b/hive-c0re/src/job_queue/model.rs index 2f5e4573..be77e688 100644 --- a/hive-c0re/src/job_queue/model.rs +++ b/hive-c0re/src/job_queue/model.rs @@ -78,6 +78,36 @@ pub enum NodeKind { /// First-spawn `nixos-container create` proper. Assumes the /// upstream `Provision` node already registered the agent in meta. Create { agent: String }, + /// `nixos-container destroy` plus the un-registration that follows it: + /// drop the agent from the coordinator's roster and clear its ephemeral + /// runtime dir. + /// + /// **Deliberately not in [`NodeKind::takes_container_down`]**, and that + /// is the design rather than an oversight. This node runs *downstream of + /// a `Stop`*, which already carries the flag honestly, so by the time it + /// claims there is nothing left to take down. A container still live here + /// is a real bug and must page someone — a `true` would absorb exactly + /// that signal, and the flag's whole asymmetry (see that method) is that + /// a wrong `true` silently swallows a crash. + DestroyContainer { agent: String }, + /// The `purge = true` half of a destroy: delete the agent's state + /// subvolume (via hive-priv, since a subvolume root defeats + /// `remove_dir_all`) plus its state and applied dirs. Its own node + /// because it is conditional — a plain destroy never inserts it — and + /// because it is the irreversible step, so it earns a distinct row in + /// the graph rather than hiding inside a bookkeeping tail. + PurgeState { agent: String }, + /// The post-destroy bookkeeping tail: meta sync, fail the agent's pending + /// approvals, drop the durable power intent, notify the manager, rescan + /// containers, re-emit the tombstone + schedule snapshots, resync + /// tmpfiles. Split from [`NodeKind::DestroyContainer`] for the same + /// reason [`NodeKind::RebuildBookkeeping`] is split from `Swap`: + /// dashboard visibility and retry granularity for work that is pure + /// store/meta bookkeeping and touches no container. + /// + /// `purge` only selects the wording of the approval-failure reason and + /// the manager notification; the destructive work is `PurgeState`'s. + DestroyBookkeeping { agent: String, purge: bool }, /// Meta flake lock bump. `sweep = false`: `meta::lock_update` /// (commit fused, under `META_LOCK`) with this node's own `inputs`; /// `sweep = true`: `meta::lock_update_hyperhive`, *non-fatal* (a @@ -341,6 +371,9 @@ impl NodeKind { NodeKind::RebuildBookkeeping { .. } => "rebuild_bookkeeping", NodeKind::Provision { .. } => "provision", NodeKind::Create { .. } => "create", + NodeKind::DestroyContainer { .. } => "destroy_container", + NodeKind::PurgeState { .. } => "purge_state", + NodeKind::DestroyBookkeeping { .. } => "destroy_bookkeeping", NodeKind::MetaLock { .. } => "meta_lock", NodeKind::Reconcile { .. } => "reconcile", NodeKind::Start { .. } => "start", @@ -378,6 +411,9 @@ impl NodeKind { | NodeKind::RebuildBookkeeping { agent } | NodeKind::Provision { agent } | NodeKind::Create { agent } + | NodeKind::DestroyContainer { agent } + | NodeKind::PurgeState { agent } + | NodeKind::DestroyBookkeeping { agent, .. } | NodeKind::Reconcile { agent } | NodeKind::Start { agent } | NodeKind::Stop { agent } @@ -435,6 +471,12 @@ impl NodeKind { // - `Create` / `Start` / `SetWanted{up}` bring a container UP. A // container disappearing *while starting* is a genuine crash and has // to keep reporting as one. + // - `DestroyContainer` looks like the most obvious `true` on this list + // and is the one that must stay `false`. It is edged downstream of a + // `Stop`, so the container is already down when it claims; the stop + // that the operator asked for is accounted for by the node that + // performs it. A container found alive at destroy time is a genuine + // bug, and a `true` here would suppress the alert that says so. // - `Reconcile` is a planner; it fans out `Start` / `Stop`, which carry // their own answer. // - `DeployWindow` brackets a deploy without itself stopping anything. diff --git a/hive-c0re/src/job_queue/templates.rs b/hive-c0re/src/job_queue/templates.rs index ee67e388..94d7366a 100644 --- a/hive-c0re/src/job_queue/templates.rs +++ b/hive-c0re/src/job_queue/templates.rs @@ -474,6 +474,59 @@ pub fn spawn(builder: &JobBuilder, agent: &str, approval_id: i64) { resolve_approval_tails(builder, approval_id, provision); } +/// Teardown: `Stop` → `DestroyContainer` → (`PurgeState`) → `DestroyBookkeeping`. +/// +/// The chain is the point, not a decomposition for its own sake. Destroy used +/// to be a straight-line async fn with no queue node behind it, so nothing in +/// the graph could answer "is this container going down on purpose?" — which is +/// why an imperative crash-watch suppression guard existed at all. Reusing the +/// existing [`NodeKind::Stop`] answers it structurally: `Stop` already declares +/// `takes_container_down`, so the suppression is derived from the graph like +/// every other lifecycle op's. +/// +/// That also makes the precondition an edge rather than an assertion. +/// `DestroyContainer` runs only after `Stop` succeeded, so it operates on an +/// already-stopped container and carries `takes_container_down = false` +/// permanently — a container still alive at that point is a real bug and stays +/// loud instead of being absorbed by a flag. +/// +/// `Stop` is idempotent against an already-down container, so the common +/// "destroy something that isn't running" path costs nothing extra. +/// +/// `Stop` is the group root and holds the agent lease for the whole teardown; +/// the rest are `part_of` children that borrow it, so no other op can interleave +/// with a half-destroyed agent. `PurgeState` is inserted only when asked for — +/// the graph shows the irreversible step as its own row when it happens, and +/// omits it entirely when it doesn't. +pub fn destroy(builder: &JobBuilder, agent: &str, purge: bool) { + let a = || agent.to_owned(); + let stop = builder + .node(NodeKind::Stop { agent: a() }) + .needs(Resource::Agent(a())); + // `part_of` IS the ordering: a child runs once its parent reaches + // `Finishing`, and a node may not also declare a dep on its own parent + // (dep-scope validation rejects it — it would deadlock). So the + // "container is already stopped" precondition is the group edge itself, + // with no explicit `after_ok(stop)` to add. + let destroy = builder + .node(NodeKind::DestroyContainer { agent: a() }) + .part_of(stop); + // The bookkeeping tail hangs off the purge when there is one, so the + // irreversible delete lands before the meta sync that stops referencing it. + let last = if purge { + builder + .node(NodeKind::PurgeState { agent: a() }) + .part_of(stop) + .after_ok(destroy) + } else { + destroy + }; + let _tail = builder + .node(NodeKind::DestroyBookkeeping { agent: a(), purge }) + .part_of(stop) + .after_ok(last); +} + /// Perm change: commit the JSON file(s), then the rebuild subgraph so /// the updated `HIVE_TOOL_GROUPS` / `HIVE_CAPABILITIES` env var takes /// effect in the container. Group-roots are `WritePermFile` plus the rebuild diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 4ac0a3f8..4e27357a 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -1714,6 +1714,97 @@ fn spawn_shape_provision_create_dropin_reconcile() { ); } +/// The destroy chain, and the reason it is a chain: `Stop` is reused so the +/// crash-watch answer comes from the node that actually stops the container. +/// +/// Asserting the *edges* is the point. `destroy_container` runs `after_ok` a +/// `stop`, which is what makes "the container is already down here" a +/// structural fact rather than a convention — see the companion test below for +/// why that matters. +#[test] +fn destroy_shape_stop_then_destroy_then_bookkeeping() { + let q = JobQueue::new(1); + insert(&q, |builder| { + templates::destroy(builder, "doomed", false); + }); + assert_eq!( + declared_shape(&q), + vec![ + row("stop", None, &[]), + // No explicit edge to `stop`: `part_of` already gates the child on + // its parent reaching `Finishing`, and declaring a dep on your own + // parent is rejected outright (it would deadlock). The precondition + // is the group membership. + row("destroy_container", Some("stop"), &[]), + row( + "destroy_bookkeeping", + Some("stop"), + &[("destroy_container", "done")] + ), + ] + ); +} + +/// `purge` inserts the irreversible delete as its own node, between the destroy +/// and the bookkeeping tail — so the meta sync that stops referencing the agent +/// runs *after* its trees are actually gone, and a purge is visibly distinct +/// from a plain destroy on the graph instead of being a hidden boolean. +#[test] +fn destroy_shape_purge_inserts_purge_state_before_the_tail() { + let q = JobQueue::new(1); + insert(&q, |builder| { + templates::destroy(builder, "doomed", true); + }); + assert_eq!( + declared_shape(&q), + vec![ + row("stop", None, &[]), + row("destroy_container", Some("stop"), &[]), + row( + "purge_state", + Some("stop"), + &[("destroy_container", "done")] + ), + row( + "destroy_bookkeeping", + Some("stop"), + &[("purge_state", "done")] + ), + ] + ); +} + +/// The counter-case to `rebuild_chain_nodes_suppress_crash_watch`, and the one +/// assertion in this file that exists to stop a *plausible* edit rather than a +/// wrong one. +/// +/// `destroy_container` is the most obvious candidate for `takes_container_down` +/// on the whole list and must stay `false`. It is edged downstream of a `Stop` +/// that already carries the flag, so the intentional stop is already accounted +/// for; a container still alive when this node claims is a genuine bug. Since a +/// wrong `true` **silently swallows a real crash** while a wrong `false` only +/// costs a spurious event, this is the asymmetry that has to be pinned. +#[test] +fn destroy_container_must_not_suppress_crash_watch() { + assert!( + !NodeKind::DestroyContainer { + agent: "a".to_owned() + } + .takes_container_down(), + "destroy_container runs after a Stop that already declared the \ + container is going down; claiming it again would suppress the alert \ + for a container found unexpectedly alive" + ); + // The upstream node is where the `true` lives — assert it here too, so the + // pair reads as one property and moving the flag breaks this test. + assert!( + NodeKind::Stop { + agent: "a".to_owned() + } + .takes_container_down() + ); +} + #[test] fn perm_change_shape_prefixes_rebuild_chain() { let q = JobQueue::new(1); diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index eedb54d7..753e67c3 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -144,7 +144,7 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { handle_start(&coord, &agents, &infra).await? } HostRequest::Destroy { name, purge } => { - actions::destroy(&coord, name.as_str(), *purge).await?; + actions::destroy(&coord, name.as_str(), *purge); HostResponse::success() } HostRequest::Rebuild { name } => { diff --git a/hive-c0re/src/workers/crash_watch.rs b/hive-c0re/src/workers/crash_watch.rs index 6ee46982..cc15e6f3 100644 --- a/hive-c0re/src/workers/crash_watch.rs +++ b/hive-c0re/src/workers/crash_watch.rs @@ -88,17 +88,17 @@ fn emit_crash_transitions(coord: &Coordinator, prev: &HashSet, current: // guard between two crash-watch polls. let recent = coord.recent_transient_within(RECENT_TRANSIENT_GRACE); for stopped in prev.difference(current) { - // Two sources, because a container can go down on purpose either way: - // a running queue node that declared it takes the container down, or a - // no-node operation (destroy, migration) holding a suppression guard. + // One source: a running queue node that declared it takes the container + // down. There used to be a second — an imperative suppression guard for + // operations with no node behind them — and destroy becoming a DAG + // removed the last caller, so intent now has exactly one home. // `any`, not "the" pill: an agent can have several running nodes at // once (a lease-exempt build alongside a lease-holding stop), and it // only takes one of them expecting the container down for this to be // a deliberate stop rather than a crash. let active = transients .get(stopped) - .map(|sts| sts.iter().any(|st| st.takes_container_down)) - .or_else(|| coord.crash_watch_suppressed(stopped).then_some(true)); + .map(|sts| sts.iter().any(|st| st.takes_container_down)); let recently_cleared = recent.get(stopped).copied(); if is_deliberate_stop(active, recently_cleared) { continue;