From bc7caf572fd7456e4941c343616a90fab53f3f07 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 1 Jun 2026 23:16:56 +0200 Subject: [PATCH] fix(#1038): include parent_id in Rebuild dedup key to prevent cascade swallowing --- hive-c0re/src/rebuild_queue.rs | 63 ++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/hive-c0re/src/rebuild_queue.rs b/hive-c0re/src/rebuild_queue.rs index 37dc02d9..fd162e9e 100644 --- a/hive-c0re/src/rebuild_queue.rs +++ b/hive-c0re/src/rebuild_queue.rs @@ -223,7 +223,13 @@ impl RebuildQueue { /// /// Dedup rule: /// - `Rebuild` / `Spawn` / `Destroy`: a `Queued` entry with the same - /// `(kind, agent)` swallows the new request. + /// `(kind, agent, parent_id)` swallows the new request. `parent_id` + /// is part of the key so that a `MetaUpdate` cascade rebuild (with a + /// specific `parent_id`) never collapses into a standalone rebuild or + /// a cascade from a different `MetaUpdate`. Without this guard a + /// cascade rebuild pre-enqueued before the lock bump would be swallowed + /// by an existing `Queued` startup-sweep rebuild, causing the agent to + /// never rebuild against the post-bump meta. /// - `MetaUpdate`: dedup ALSO requires the `inputs` field to match — /// two meta-updates with different input lists are distinct work /// and must queue separately, otherwise the second meta-update @@ -287,13 +293,17 @@ impl RebuildQueue { // and, for MetaUpdate, the same `inputs` list (see method // docstring for why). Approval-driven entries also require the // approval_id to match so two distinct approvals for the same - // agent never collapse into one queue slot. + // agent never collapse into one queue slot. Rebuild (and Spawn / + // Destroy) entries also require parent_id to match so a + // MetaUpdate cascade rebuild is never swallowed by an unrelated + // queued rebuild (e.g. from the startup sweep). for entry in &mut inner.entries { if entry.state == QueueState::Queued && entry.kind == kind && entry.agent == agent && (kind != QueueKind::MetaUpdate || entry.inputs == inputs) && entry.approval_id == approval_id + && entry.parent_id == parent_id { if !entry.reason.contains(&reason) { use std::fmt::Write as _; @@ -1147,6 +1157,55 @@ mod tests { assert!(!q.set_step(999, "anything")); } + /// A MetaUpdate cascade Rebuild (with parent_id = Some(meta_id)) must + /// NOT dedup into a pre-existing Queued Rebuild with a different parent_id + /// (e.g. from a startup sweep). Without the parent_id dedup guard the + /// cascade rebuild would be swallowed and the agent would never rebuild + /// against the post-lock-bump meta. + #[test] + fn meta_update_cascade_does_not_dedup_into_startup_sweep_rebuild() { + let q = RebuildQueue::new(); + // Startup sweep enqueues a Rebuild for alice with its own parent_id. + let sweep = q.enqueue( + QueueKind::StartupSweep, + "hyperhive".to_owned(), + QueueSource::AutoUpdate, + "boot sweep".to_owned(), + None, + ); + let sweep_rebuild = q.enqueue( + QueueKind::Rebuild, + "alice".to_owned(), + QueueSource::StartupSweep, + "startup sweep".to_owned(), + Some(sweep), + ); + // MetaUpdate cascade pre-enqueues another Rebuild for alice. + let meta = q.enqueue( + QueueKind::MetaUpdate, + "hyperhive".to_owned(), + QueueSource::Manual, + "bump nixpkgs".to_owned(), + None, + ); + let cascade_rebuild = q.enqueue( + QueueKind::Rebuild, + "alice".to_owned(), + QueueSource::MetaUpdate, + "meta-update cascade".to_owned(), + Some(meta), + ); + // The two Rebuilds have different parent_ids — must NOT dedup. + assert_ne!(sweep_rebuild, cascade_rebuild, + "cascade rebuild must be distinct from startup-sweep rebuild"); + let snap = q.snapshot(); + let rebuilds: Vec<_> = snap.iter() + .filter(|e| e.kind == QueueKind::Rebuild && e.agent == "alice") + .collect(); + assert_eq!(rebuilds.len(), 2, + "both rebuilds must be present in the queue"); + } + #[test] fn finish_clears_step() { let q = RebuildQueue::new();