From 84383568b225166039dbd7e7ce08c533338c4918 Mon Sep 17 00:00:00 2001 From: damocles Date: Wed, 3 Jun 2026 23:04:34 +0200 Subject: [PATCH] fix: true atomic bulk topology -- apply_set_parent + single write, add # Errors docs --- hive-c0re/src/coordinator.rs | 7 ++++++- hive-c0re/src/meta.rs | 29 ++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 21040ddd..ee0b8440 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -719,7 +719,12 @@ impl Coordinator { /// Batch version of [`reparent_with_notify`]: applies all moves under a /// single `META_LOCK` acquisition (one git commit) then sends per-agent /// notifications for each move that actually changed topology. - /// First validation failure aborts the whole batch. + /// First validation failure aborts the whole batch with no disk writes. + /// + /// # Errors + /// + /// Propagates any error returned by [`crate::meta::bulk_commit_topology`] + /// (validation failure or topology-file write error). pub async fn reparent_bulk_with_notify( self: &Arc, moves: &[(&str, Option<&str>)], diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index ae020fde..f4653462 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -400,13 +400,26 @@ pub async fn commit_topology( /// Batch variant of [`commit_topology`]: applies every `(child, new_parent)` /// move under a single `META_LOCK` acquisition and creates **one** git commit /// for all of them. Moves are applied in the order given; the first -/// validation error short-circuits the whole batch (no partial writes because -/// `topology::set_parent` rewrites the in-memory map atomically and we only -/// flush to disk after all moves have passed validation). +/// validation error short-circuits the whole batch. True atomic write: all +/// moves are pre-validated against a cumulative in-memory state with +/// [`crate::topology::apply_set_parent`] before anything touches disk, then +/// [`crate::topology::write`] is called exactly once. If any move fails +/// validation the topology file is never modified. +/// +/// The multi-move commit message uses `moves[0].1` as the destination label. +/// This is intentional: the dashboard bulk-move UI always sends a single +/// destination for all selected agents, so the message is always accurate in +/// practice. /// /// Returns a `Vec` of `(child, old_parent)` pairs for every move that /// actually changed the topology (idempotent same-parent moves are skipped), /// so the caller can send targeted notifications. +/// +/// # Errors +/// +/// Returns a `String` error if any move fails validation (cycle, unknown +/// agent, etc.) or if the topology file cannot be written. Git-commit failure +/// is logged as a warning and does not propagate — `sync_agents` will recover. pub async fn bulk_commit_topology( moves: &[(&str, Option<&str>)], ) -> std::result::Result)>, String> { @@ -416,9 +429,15 @@ pub async fn bulk_commit_topology( let _guard = META_LOCK.lock().await; // Snapshot parents before any writes so we can compute the diff. let topo_before = crate::topology::read(); - // Validate + apply all moves in sequence; first error aborts. + // Validate all moves against a cumulative in-memory state -- no disk + // writes yet; first error aborts with the topology file untouched. + let mut next = topo_before.clone(); for (child, new_parent) in moves { - crate::topology::set_parent(child, *new_parent)?; + next = crate::topology::apply_set_parent(&next, child, *new_parent)?; + } + // Only flush to disk if something actually changed. + if next != topo_before { + crate::topology::write(&next).map_err(|e| format!("{e:#}"))?; } // Commit the whole batch as one git operation. let dir = meta_dir();