diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 4963ea16..13340fca 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -67,6 +67,21 @@ pub struct Coordinator { /// Read by the dashboard to render a spinner; cleared when the action /// resolves (success or failure). transient: 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 + /// kill / restart sets `Stopping` → runs `nixos-container stop` → + /// drop clears the transient → poll fires next tick and sees the + /// container missing-from-running with no active transient → + /// spurious "container stopped without an operator action" + /// message (closes #425). + /// + /// `clear_transient` stamps the cleared kind here with an + /// `Instant`; `recent_transient_within(grace)` returns the set of + /// agents whose tombstone is still inside the grace window. Crash + /// watcher consults both this and the active map before declaring + /// a stop deliberate. + recent_transient: Mutex>, /// Unified wire-facing event channel feeding the dashboard SSE /// stream. Carries broker messages (mirrored from `broker.subscribe` /// by the forwarder task in `main.rs`) and dashboard-only mutation @@ -211,6 +226,7 @@ impl Coordinator { context_window_tokens, agents: Mutex::new(HashMap::new()), transient: Mutex::new(HashMap::new()), + recent_transient: Mutex::new(HashMap::new()), dashboard_events, event_seq: AtomicU64::new(0), meta_updates_active: AtomicU64::new(0), @@ -563,8 +579,18 @@ impl Coordinator { } pub fn clear_transient(&self, name: &str) { - let removed = self.transient.lock().unwrap().remove(name).is_some(); - if removed { + let removed = self.transient.lock().unwrap().remove(name); + if let Some(state) = removed { + // Stamp the tombstone so the crash watcher can still see + // "operator kicked this off recently" on its next 10s poll + // — without this, the clear-then-poll race produced a + // spurious ContainerCrash on every operator stop/restart + // (#425). Old entries get reaped lazily on read so the map + // doesn't grow unbounded. + self.recent_transient + .lock() + .unwrap() + .insert(name.to_owned(), (state.kind, std::time::Instant::now())); self.emit_dashboard_event(DashboardEvent::TransientCleared { seq: self.next_seq(), name: name.to_owned(), @@ -572,6 +598,18 @@ impl Coordinator { } } + /// Set of agents whose transient was cleared within the last + /// `grace` seconds — i.e. agents the operator just acted on, + /// whose stop the crash watcher should NOT classify as a crash. + /// Lazily reaps entries older than `grace` so the map stays + /// bounded by the active agent count. + pub fn recent_transient_within(&self, grace: std::time::Duration) -> HashMap { + let now = std::time::Instant::now(); + let mut map = self.recent_transient.lock().unwrap(); + map.retain(|_, (_, ts)| now.duration_since(*ts) <= grace); + map.iter().map(|(k, (kind, _))| (k.clone(), *kind)).collect() + } + /// Set a transient state and return a guard that clears it on drop. /// Use this from any path where the surrounding future could be /// cancelled or panic between set and clear (HTTP handlers, spawned diff --git a/hive-c0re/src/crash_watch.rs b/hive-c0re/src/crash_watch.rs index e00a420c..fb7fd755 100644 --- a/hive-c0re/src/crash_watch.rs +++ b/hive-c0re/src/crash_watch.rs @@ -25,6 +25,13 @@ use crate::lifecycle::{self, AGENT_PREFIX, MANAGER_NAME}; const POLL_INTERVAL: Duration = Duration::from_secs(10); +/// How long an operator-initiated transient stays "recently cleared" +/// for the purpose of suppressing crash events. Three full +/// `POLL_INTERVAL`s gives the post-lifecycle path comfortable +/// breathing room — the watcher will have polled at least twice +/// inside the window even with worst-case timer skew (#425). +const RECENT_TRANSIENT_GRACE: Duration = Duration::from_secs(30); + pub fn spawn(coord: Arc) { let mut shutdown = coord.shutdown_rx(); tokio::spawn(async move { @@ -92,17 +99,14 @@ pub fn spawn(coord: Arc) { fn emit_crash_transitions(coord: &Coordinator, prev: &HashSet, current: &HashSet) { let transients = coord.transient_snapshot(); + // Operator actions whose RAII guard already cleared but only just; + // suppresses the race where `lifecycle::kill` returns + drops the + // guard between two crash-watch polls (closes #425). + let recent = coord.recent_transient_within(RECENT_TRANSIENT_GRACE); for stopped in prev.difference(current) { - let deliberate = transients.get(stopped).is_some_and(|st| { - matches!( - st.kind, - TransientKind::Stopping - | TransientKind::Restarting - | TransientKind::Destroying - | TransientKind::Rebuilding - ) - }); - if deliberate { + let active = transients.get(stopped).map(|st| st.kind); + let recently_cleared = recent.get(stopped).copied(); + if is_deliberate_stop(active, recently_cleared) { continue; } tracing::warn!(agent = %stopped, "container crash detected"); @@ -113,6 +117,76 @@ fn emit_crash_transitions(coord: &Coordinator, prev: &HashSet, current: } } +/// Pure classifier: did the operator stop / restart / destroy / +/// rebuild this container, or did it crash? Splits the matcher out so +/// it has a focused unit test (#425) without needing a Coordinator +/// fixture. `active` is the currently-set transient (if any), +/// `recently_cleared` is one whose RAII guard dropped within the +/// grace window. +fn is_deliberate_stop( + active: Option, + recently_cleared: Option, +) -> bool { + let is_op_kind = |kind: TransientKind| { + matches!( + kind, + TransientKind::Stopping + | TransientKind::Restarting + | TransientKind::Destroying + | TransientKind::Rebuilding + ) + }; + active.is_some_and(is_op_kind) || recently_cleared.is_some_and(is_op_kind) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn deliberate_when_active_transient_is_operator_kind() { + for kind in [ + TransientKind::Stopping, + TransientKind::Restarting, + TransientKind::Destroying, + TransientKind::Rebuilding, + ] { + assert!(is_deliberate_stop(Some(kind), None), "{kind:?}"); + } + } + + #[test] + fn deliberate_when_recent_transient_is_operator_kind() { + // Race the #425 bug repros: lifecycle action completes + drops + // the guard between two polls. recent_transient catches it. + for kind in [ + TransientKind::Stopping, + TransientKind::Restarting, + TransientKind::Destroying, + TransientKind::Rebuilding, + ] { + assert!(is_deliberate_stop(None, Some(kind)), "{kind:?}"); + } + } + + #[test] + fn not_deliberate_with_no_transient_at_all() { + // The real-crash case — fires the ContainerCrash event. + assert!(!is_deliberate_stop(None, None)); + } + + #[test] + fn not_deliberate_when_only_spawning_starting() { + // Spawning/Starting are never paired with a "stopped" transition + // — they're starts. If we see one alongside a stop, it's + // unrelated (e.g. just-started container died), still a crash. + for kind in [TransientKind::Spawning, TransientKind::Starting] { + assert!(!is_deliberate_stop(Some(kind), None), "{kind:?} active"); + assert!(!is_deliberate_stop(None, Some(kind)), "{kind:?} recent"); + } + } +} + fn emit_login_transitions( coord: &Coordinator, prev: &HashSet,