diff --git a/docs/coordinator.md b/docs/coordinator.md index 15d79a46..316326f4 100644 --- a/docs/coordinator.md +++ b/docs/coordinator.md @@ -54,7 +54,6 @@ Cheap — no build slot: | `Drain` | await the harness clearing the fence, bounded by the 3-min graceful-stop timeout; resolves ok either way | | `WriteDropin` | `set_nspawn_flags` + `set_resource_limits` + daemon-reload | | `WritePermFile` | commit `tool-groups.json` / `capabilities.json` (single git commit under `META_LOCK`) + emit the P3RM1SS10NS snapshots | -| `Reparent` | `set-parent` / `set-parent-bulk`: apply every `(child, new_parent)` move under one `META_LOCK` commit (`meta::bulk_commit_topology`), send the per-agent move notifications, rescan + diff-emit. Agentless like `MetaLock` — a bulk move can span multiple agents, and a reparent touches the meta repo, not any one container. `moves` is `(Ident, Option)` pairs, not raw strings — mara: "use Ident type instead of string" (#2719, issuecomment 42691). Rides `Template::MetaUpdate` rather than a dedicated `Template` variant — that enum is on its way out (see `#2665`, still open/blocked on a scope question) and is already internal-only (not on `DagView`'s wire shape), so the stand-in only affects `terminal_hook` dispatch (resolves to no hook either way) and history-retention bucketing | There is deliberately **no `GitCommit` node**: `meta.rs` fuses each mutation with its commit under its internal `META_LOCK` mutex, so a standalone commit @@ -67,7 +66,7 @@ container build: - **The deploy window** (`Resource::MetaWindow`): a global, capacity-1 queue resource declared by every node kind that mutates the meta repo — `MetaSync`, - `MetaLock`, `WritePermFile`, `Reparent`, `Provision`'s agent registration, and + `MetaLock`, `WritePermFile`, `Provision`'s agent registration, and `DeployWindow` — the deploy subtree's root, which holds it across every phase below it (`NodeKind::needs_meta_window`). Two meta mutations can therefore never interleave, so no commit lands inside another @@ -123,7 +122,6 @@ perm-change(a): WritePermFile(a) → «rebuild subgraph» meta-update(inp): MetaLock(inp) →(in-DAG) «rebuild subgraph» per affected agent boot: (if any rev marker stale) MetaLock(hyperhive) →(in-DAG) «rebuild subgraph» per stale agent; plus Reconcile(a) for every drifted agent (all ONE DAG) -reparent(moves): Reparent(moves) (no rebuild — topology.json is read live) ``` Notable collapses: @@ -192,7 +190,7 @@ resources are free. Resources: write, not a container op, but takes the lease anyway so a power-op DAG's intent write + reconcile is atomic — two racing ops can't clobber intent before either reconciles.) **Lease-exempt**: `MetaSync`, `Prebuild`, - `MetaLock`, `WritePermFile`, `Reparent` — + `MetaLock`, `WritePermFile` — they touch the store / meta, not the running container, which is exactly why a stop can land while another DAG's prebuild is still building. diff --git a/hive-c0re/src/agent_config/topology.rs b/hive-c0re/src/agent_config/topology.rs index ba030b86..1ac11516 100644 --- a/hive-c0re/src/agent_config/topology.rs +++ b/hive-c0re/src/agent_config/topology.rs @@ -252,7 +252,7 @@ pub fn default_seed(agent_names: &[String]) -> BTreeMap> out } -/// Pure validation + apply for [`crate::meta::bulk_commit_topology`]. Splits off so tests +/// Pure validation + apply for [`set_parent`]. Splits off so tests /// can exercise the rules (cycle / unknown) on an in-memory /// `BTreeMap` without touching the on-disk `topology.json`. Returns /// either the post-move map (caller writes it back) or a @@ -298,10 +298,30 @@ pub fn apply_set_parent( Ok(next) } +/// Operator-driven parent move. Set `child`'s parent to `new_parent` +/// (or `None` to promote to root). See [`apply_set_parent`] for the +/// validation rules. The operator-set parent sticks across +/// `reconcile()` calls (which preserves existing entries). +/// +/// No bind-mount / container churn today — the hierarchy is +/// currently logical-only. Once sub-manager bind mounts land, the +/// caller adds an umount-old / mount-new / restart-cascade step on +/// top. +pub fn set_parent(child: &str, new_parent: Option<&str>) -> Result<(), String> { + let current = read(); + // Idempotent no-op fast path: skip the disk write when nothing + // changes. apply_set_parent still runs to surface validation + // errors (e.g. unknown child) so the caller gets a real signal. + let next = apply_set_parent(¤t, child, new_parent)?; + if next == current { + return Ok(()); + } + write(&next).map_err(|e| format!("write topology.json: {e}")) +} + /// Declare a brand-new agent's parent edge before the agent exists in -/// the container set. Unlike [`crate::meta::bulk_commit_topology`] / -/// [`apply_set_parent`] (which reparent an entry that must already be -/// present), this inserts a fresh `child -> parent` +/// the container set. Unlike [`set_parent`] (which reparents an entry +/// that must already be present), this inserts a fresh `child -> parent` /// row. Used by the `InitConfig` approval to place a just-scaffolded /// sub-agent under its requesting parent, so the edge is in place /// before the first apply-commit spawns the container (and before diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index cfe9aa34..d44fe4cc 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -926,11 +926,9 @@ impl Coordinator { } } - /// Apply topology reparent(s) + fan the resulting notifications out to - /// the affected agents. Applies all moves under a single `META_LOCK` - /// acquisition (one git commit — a single-move call is just a - /// one-element slice) then, for each move that actually changed - /// topology, drops a one-line system message into the inbox of: + /// Apply a topology reparent + fan the resulting notifications + /// out to the three affected agents. On success, drops a + /// one-line system message into the inbox of: /// /// 1. The **old parent** (if any) — `"{child} moved out of your /// subtree to {new_parent_or_root}"`. @@ -948,6 +946,76 @@ impl Coordinator { /// `from = hive_sh4re::SYSTEM_SENDER` so the dashboard renders /// them under the existing system-source styling. /// + /// Idempotent: if `topology::set_parent` skipped the disk write + /// (same-parent no-op), no messages fire. The `topology` module's + /// validation (`apply_set_parent`: unknown agent, cycle, etc.) + /// runs *before* any message is sent, so a refused move never + /// notifies anyone. + /// + /// Also drives a `rescan_containers_and_emit` after the write so + /// the dashboard tree re-renders without polling. + pub async fn reparent_with_notify( + self: &Arc, + child: &str, + new_parent: Option<&str>, + ) -> std::result::Result<(), String> { + // Snapshot the old parent BEFORE the write so the + // notifications can describe both sides of the change. + let topo_before = crate::topology::read(); + let old_parent = topo_before.get(child).cloned().flatten(); + + // The disk write + git commit happen here under META_LOCK, so + // the topology.json change is committed atomically and the + // working tree is never left dirty between the write and the + // next meta operation. Topology validation + idempotent + // fast-path inside `set_parent` may short-circuit (same + // parent → no-op). We mirror the same idempotent shape for + // the notifications: if nothing changed, send nothing. + crate::meta::commit_topology(child, new_parent).await?; + + let changed = old_parent.as_deref() != new_parent; + if changed { + let old_label = old_parent.as_deref().unwrap_or(""); + let new_label = new_parent.unwrap_or(""); + // System-source notifications. Best-effort: a failed broker + // send is logged + ignored so a transient sqlite error + // doesn't bubble out and unwind the topology write. + if let Some(op) = old_parent.as_deref() { + let _ = self.broker.send(&hive_sh4re::Message { + from: hive_sh4re::trusted_sender(hive_sh4re::SYSTEM_SENDER), + to: op.to_owned(), + body: format!("{child} moved out of your subtree to {new_label}"), + in_reply_to: None, + }); + } + if let Some(np) = new_parent { + let _ = self.broker.send(&hive_sh4re::Message { + from: hive_sh4re::trusted_sender(hive_sh4re::SYSTEM_SENDER), + to: np.to_owned(), + body: format!( + "{child} just moved into your subtree (was previously under {old_label})" + ), + in_reply_to: None, + }); + } + // Coalesce: if a prior move notification is already pending in + // the broker (agent was offline for multiple moves), update it + // in-place so the agent sees "A to C" not "A to B" then "B to C". + let _ = self + .broker + .send_coalescing_reparent(child, old_label, new_label); + } + + // Rescan + diff-emit regardless of whether messages fired — + // even an idempotent no-op might have happened concurrently + // with another lifecycle event the dashboard cares about. + self.rescan_containers_and_emit().await; + Ok(()) + } + + /// 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 with no disk writes. /// /// # Errors diff --git a/hive-c0re/src/dashboard/topology.rs b/hive-c0re/src/dashboard/topology.rs index 8d180675..2519b6af 100644 --- a/hive-c0re/src/dashboard/topology.rs +++ b/hive-c0re/src/dashboard/topology.rs @@ -2,12 +2,8 @@ //! //! Operator-driven agent reparenting — single (`/api/topology/set-parent`, //! form-encoded) and bulk (`/api/topology/set-parent-bulk`, JSON array → -//! one git commit). Both submit a `NodeKind::Reparent` DAG to the job -//! queue (fire-and-forget, like every other queue-backed op — the -//! dashboard tree repaints off the queue's own snapshot/rescan once the -//! commit lands, same as a rebuild or restart). The executor delegates to -//! `Coordinator::reparent_bulk_with_notify`, which wraps -//! `crate::meta::bulk_commit_topology` with the move-notification messages +//! one git commit). Both go through `Coordinator::reparent*_with_notify`, +//! which wraps `topology::set_parent` with the move-notification messages //! and the `ContainerView` rescan. use axum::{ @@ -19,8 +15,7 @@ use serde::Deserialize; use problem_details::ProblemDetails; -use super::{AppState, error_problem}; -use crate::job_queue::{Source, submit}; +use super::{AppState, error_problem, error_response}; /// `POST /api/topology/set-parent` body. `child` is required. /// `new_parent` may be: @@ -49,13 +44,11 @@ pub(super) struct SetParentBulkEntry { /// `POST /api/topology/set-parent` — operator-driven parent move. /// Form fields: `child` (required, agent name), `new_parent` /// (optional — empty / absent string ⇒ promote to root). Refuses -/// cycles and unknown agents (surfaced async on the job view — this -/// handler only validates the identifiers, not the move itself). The -/// manager is reparentable like any other agent — its privileges come -/// from the privileged MCP socket, not its tree position. Submitting -/// re-emits the queue snapshot immediately so the dashboard shows the -/// queued move without a refresh; the tree itself repaints once the -/// commit lands. +/// cycles and unknown agents. The manager is reparentable like any +/// other agent — its privileges come from the privileged MCP socket, +/// not its tree position. On success +/// re-emits container snapshots so the dashboard tree repaints +/// without a refresh. pub(super) async fn post_set_parent( State(state): State, Form(form): Form, @@ -65,8 +58,6 @@ pub(super) async fn post_set_parent( return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) .with_detail("set-parent: `child` required")); } - let child = hive_types::Ident::parse(&child) - .map_err(|e| error_problem(&format!("set-parent: `child` {e}")))?; // Empty / whitespace-only `new_parent` ⇒ promote to root. Web // forms submit the empty string for a "no value" radio button, // so this is the ergonomic encoding. @@ -75,65 +66,54 @@ pub(super) async fn post_set_parent( .as_deref() .map(str::trim) .filter(|s| !s.is_empty()) - .map(hive_types::Ident::parse) - .transpose() - .map_err(|e| error_problem(&format!("set-parent: `new_parent` {e}")))?; - tracing::info!( - child = %child, - new_parent = ?new_parent, - "operator: set-parent via dashboard" - ); - submit::reparent( - &state.coord, - vec![(child, new_parent)], - Source::Manual, - "manual set-parent via dashboard".to_owned(), - ); - Ok((StatusCode::OK, "ok").into_response()) + .map(str::to_owned); + // `reparent_with_notify` wraps `topology::set_parent` with the + // three notification messages + the ContainerView rescan. + // Idempotent same-parent calls skip both the messages and the + // disk write per the topology fast-path. + match state + .coord + .reparent_with_notify(&child, new_parent.as_deref()) + .await + { + Ok(()) => { + tracing::info!( + child = %child, + new_parent = ?new_parent, + "operator: set-parent via dashboard" + ); + Ok((StatusCode::OK, "ok").into_response()) + } + Err(e) => Err(error_problem(&format!("set-parent {child} failed: {e}"))), + } } /// `POST /api/topology/set-parent-bulk` — move multiple agents in a single /// request, producing **one** git commit. JSON body: `[{"child":"name", /// "new_parent":"target-or-null"}, ...]`. Empty array is a no-op (200 OK). -/// First identifier that fails to parse aborts the whole batch before -/// anything is submitted — a partially-invalid bulk move never reaches -/// the queue. +/// First validation error aborts the whole batch. pub(super) async fn post_set_parent_bulk( State(state): State, axum::Json(body): axum::Json>, -) -> Result { +) -> Response { if body.is_empty() { - return Ok((StatusCode::OK, "ok").into_response()); + return (StatusCode::OK, "ok").into_response(); } - // Collect into `Result<_, String>` first, not `ProblemDetails` directly — - // clippy::result_large_err flags a ~232-byte `Err` variant threaded - // through this closure's `?`. `String` is small enough to satisfy the - // lint; the single `map_err` below promotes it to a `ProblemDetails` - // once, after the fallible collect. - let moves = body + // Collect borrows for the coordinator call. + let moves: Vec<(&str, Option<&str>)> = body .iter() .map(|e| { - let child = hive_types::Ident::parse(e.child.trim()) - .map_err(|err| format!("set-parent-bulk: `{}` {err}", e.child))?; - let new_parent = e - .new_parent - .as_deref() - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(hive_types::Ident::parse) - .transpose() - .map_err(|err| format!("set-parent-bulk: `{}` {err}", e.child))?; - Ok((child, new_parent)) + let child: &str = &e.child; + let parent: Option<&str> = e.new_parent.as_deref().filter(|s| !s.is_empty()); + (child, parent) }) - .collect::, String>>() - .map_err(|e| error_problem(&e))?; - let names: Vec<&str> = body.iter().map(|e| e.child.as_str()).collect(); - tracing::info!(agents = ?names, "operator: set-parent-bulk via dashboard"); - submit::reparent( - &state.coord, - moves, - Source::Manual, - "manual set-parent-bulk via dashboard".to_owned(), - ); - Ok((StatusCode::OK, "ok").into_response()) + .collect(); + match state.coord.reparent_bulk_with_notify(&moves).await { + Ok(()) => { + let names: Vec<&str> = body.iter().map(|e| e.child.as_str()).collect(); + tracing::info!(agents = ?names, "operator: set-parent-bulk via dashboard"); + (StatusCode::OK, "ok").into_response() + } + Err(e) => error_response(&format!("set-parent-bulk failed: {e}")), + } } diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 0adc028b..507c9dea 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -89,7 +89,6 @@ pub(super) async fn run_node(coord: &Arc, claim: &Claim) -> Result< NodeKind::Drain { .. } => run_drain(coord, claim).await, NodeKind::WriteDropin { .. } => run_write_dropin(coord, claim).await, NodeKind::WritePermFile { .. } => run_write_perm_file(coord, claim).await, - NodeKind::Reparent { .. } => run_reparent(coord, claim).await, NodeKind::DeployWindow { .. } => run_deploy_window(claim), NodeKind::MergeVerify { .. } => run_merge_verify(coord, claim).await, NodeKind::DeployApply { .. } => run_deploy_apply(coord, claim).await, @@ -531,33 +530,6 @@ async fn run_write_perm_file(coord: &Arc, claim: &Claim) -> Result< Ok(NodeOutput::default()) } -/// Apply the node's `(child, new_parent)` moves as one `META_LOCK`-fused -/// commit (`Coordinator::reparent_bulk_with_notify`, which already handles -/// both the single- and bulk-move case, sends the per-agent move -/// notifications, and rescans + diff-emits the container tree). Runs under -/// the deploy window (`NodeKind::needs_meta_window`), same reasoning as -/// `run_write_perm_file`: a topology commit landing inside another node's -/// staged deploy window would sweep the staged lock into its commit. -async fn run_reparent(coord: &Arc, claim: &Claim) -> Result { - let NodeKind::Reparent { moves } = &claim.kind else { - anyhow::bail!("run_reparent on a non-Reparent node"); - }; - let refs: Vec<(&str, Option<&str>)> = moves - .iter() - .map(|(child, parent)| { - ( - child.as_str(), - parent.as_ref().map(hive_types::Ident::as_str), - ) - }) - .collect(); - coord - .reparent_bulk_with_notify(&refs) - .await - .map_err(|e| anyhow::anyhow!(e))?; - Ok(NodeOutput::default()) -} - /// The approval id every deploy phase re-reads its approval row by. Fails the /// node when the DAG carries none, which would mean a `MergeConfigPr` DAG was /// built without going through `templates::approval_deploy`. diff --git a/hive-c0re/src/job_queue/model.rs b/hive-c0re/src/job_queue/model.rs index 5d353d31..d393018d 100644 --- a/hive-c0re/src/job_queue/model.rs +++ b/hive-c0re/src/job_queue/model.rs @@ -186,19 +186,6 @@ pub enum NodeKind { /// (commit fused under `META_LOCK`). The payload rides this node — the only /// consumer — rather than the generic DAG container. WritePermFile { agent: String, payload: PermPayload }, - /// Topology move(s) — `set-parent` (len 1) or `set-parent-bulk` (len N) — - /// as a single queue node. Agentless like [`NodeKind::MetaLock`]: a - /// reparent touches the meta repo, not any one container, and a bulk - /// move spans multiple agents anyway. `needs_meta_window() = true`, same - /// precedent as [`NodeKind::WritePermFile`] (also a small - /// git-commit-under-`META_LOCK` op) — a reparent's commit must not land - /// inside another node's staged deploy `prepare_deploy`→`finalize_deploy` - /// window. `(child, new_parent)` pairs, applied in order under one - /// `META_LOCK` acquisition / one git commit (`meta::bulk_commit_topology` - /// handles both the single- and multi-move case uniformly). - Reparent { - moves: Vec<(hive_types::Ident, Option)>, - }, /// Group root of the approval-deploy (`MergeConfigPr`) subtree, and the /// node that **owns the deploy window**. It performs no work of its own — /// it exists so the resources it declares (the global @@ -329,7 +316,6 @@ impl NodeKind { NodeKind::Drain { .. } => "drain", NodeKind::WriteDropin { .. } => "write_dropin", NodeKind::WritePermFile { .. } => "write_perm_file", - NodeKind::Reparent { .. } => "reparent", NodeKind::DeployWindow { .. } => "deploy_window", NodeKind::MergeVerify { .. } => "merge_verify", NodeKind::DeployApply { .. } => "deploy_apply", @@ -341,8 +327,7 @@ impl NodeKind { } /// The agent this node targets, or `""` for agentless kinds - /// ([`NodeKind::MetaLock`] on the `hyperhive` pseudo-agent, - /// [`NodeKind::Reparent`] which can span multiple agents, and the + /// ([`NodeKind::MetaLock`] on the `hyperhive` pseudo-agent, and the /// [`NodeKind::Dag`] container). #[must_use] pub fn agent(&self) -> &str { @@ -367,7 +352,7 @@ impl NodeKind { | NodeKind::FinalizeDeploy { agent } | NodeKind::DeployTail { agent } | NodeKind::SetWanted { agent, .. } => agent, - NodeKind::MetaLock { .. } | NodeKind::Reparent { .. } | NodeKind::Dag { .. } => "", + NodeKind::MetaLock { .. } | NodeKind::Dag { .. } => "", } } @@ -387,7 +372,7 @@ impl NodeKind { /// Container-affecting kinds require the DAG to hold the agent's /// lifecycle lease (acquired at the first such node, held until the /// DAG is terminal). Lease-exempt kinds (`MetaSync`, `Prebuild`, - /// `Provision`, `MetaLock`, `WritePermFile`, `Reparent`) touch the store / meta repo, not the + /// `Provision`, `MetaLock`, `WritePermFile`) touch the store / meta repo, not the /// running container — which is exactly why a `Prebuild` can overlap /// another DAG's work on the same agent. `Provision` precedes the /// container's existence entirely, so the lease is first taken at the @@ -440,7 +425,6 @@ impl NodeKind { | NodeKind::Provision { .. } | NodeKind::MetaLock { .. } | NodeKind::WritePermFile { .. } - | NodeKind::Reparent { .. } | NodeKind::DeployWindow { .. } | NodeKind::FinalizeDeploy { .. } ) diff --git a/hive-c0re/src/job_queue/submit.rs b/hive-c0re/src/job_queue/submit.rs index c9648162..6f010c4b 100644 --- a/hive-c0re/src/job_queue/submit.rs +++ b/hive-c0re/src/job_queue/submit.rs @@ -420,19 +420,3 @@ pub fn meta_update( ) -> u64 { submit_and_emit(coord, templates::meta_update(inputs, source, reason, None)) } - -/// Topology move(s) as a queue DAG. `moves` is `(child, new_parent)` pairs — -/// one entry for `set-parent`, N for `set-parent-bulk`. Fire-and-forget like -/// everything else in this module: submits and returns a DAG id -/// immediately, the caller learns the outcome async (dashboard job view / -/// `hivectl`'s `QueueDag` poll). Wired from `server.rs`'s `HostRequest:: -/// SetParent` (hivectl) and `dashboard/topology.rs`'s `set-parent`/ -/// `set-parent-bulk` handlers. -pub fn reparent( - coord: &Arc, - moves: Vec<(hive_types::Ident, Option)>, - source: Source, - reason: String, -) -> u64 { - submit_and_emit(coord, templates::reparent(moves, source, reason)) -} diff --git a/hive-c0re/src/job_queue/templates.rs b/hive-c0re/src/job_queue/templates.rs index 5aec3c0a..12d4496e 100644 --- a/hive-c0re/src/job_queue/templates.rs +++ b/hive-c0re/src/job_queue/templates.rs @@ -22,7 +22,6 @@ //! spawn(a): Provision(a) → Create(a) → WriteDropin(a) → Reconcile(a) [wanted=Up at approve] //! perm-change(a): WritePermFile(a) → «rebuild subgraph» //! meta-update(inp): MetaLock(inp) →«in-DAG rebuild subgraph per affected a» -//! reparent(moves): Reparent(moves) [no rebuild — topology.json is read live] //! ``` //! //! For the dynamic power-op shapes (`stop` / `start` / `restart`, built from @@ -342,41 +341,6 @@ pub fn meta_update( } } -/// Topology move(s) as a single-node DAG. `moves` is `(child, new_parent)` -/// pairs — len 1 for `set-parent`, len N for `set-parent-bulk`, applied -/// uniformly by the one [`NodeKind::Reparent`] node (which holds the global -/// meta window for its duration, same precedent as [`NodeKind::WritePermFile`]). -/// No rebuild subgraph: `topology.json` is read live by every consumer -/// (dashboard tree, ``/`` sentinel routing, permission -/// checks), so a parent move needs no container rebuild to take effect. -/// No transient pill either — the node is agentless (no lease to hang one -/// off of) and near-instant. -/// -/// Rides `Template::MetaUpdate` rather than a dedicated variant because -/// `Template` is being removed and nothing should dispatch on it — a fresh -/// variant would just be more surface to delete later. `Template` is already -/// internal-only (not on `DagView`'s wire shape — the dashboard derives its -/// label from `nodes`), so the choice of stand-in variant only affects -/// `terminal_hook` dispatch (`MetaUpdate` resolves to `None`, same as a -/// dedicated variant would) and the per-template history-retention bucket — -/// both cosmetic. Swap this to whatever the eventual node-kind-derived -/// dispatch lands with, whenever it lands. -pub fn reparent( - moves: Vec<(hive_types::Ident, Option)>, - source: Source, - reason: String, -) -> DagSpec { - DagSpec { - template: Template::MetaUpdate, - source, - reason, - approval_id: None, - inputs: Vec::new(), - transient: None, - nodes: vec![node(NodeKind::Reparent { moves }, Vec::new())], - } -} - // The boot is assembled inline in `workers/auto_update.rs::submit_boot_tree` // as ONE `Boot` DAG (a sweep `MetaLock` root that grows rebuild subgraphs // in-DAG, plus a `Reconcile` root per drifted agent) — no anchor node and no diff --git a/hive-c0re/src/job_queue/tests.rs b/hive-c0re/src/job_queue/tests.rs index 30a08535..094af83a 100644 --- a/hive-c0re/src/job_queue/tests.rs +++ b/hive-c0re/src/job_queue/tests.rs @@ -13,10 +13,6 @@ fn submit(q: &JobQueue, spec: DagSpec) -> u64 { q.submit(spec).expect("valid spec") } -fn ident(s: &str) -> hive_types::Ident { - hive_types::Ident::parse(s).expect("valid test ident") -} - fn rebuild(agent: &str, reason: &str) -> DagSpec { templates::rebuild(agent, Source::Manual, reason.to_owned(), true) } @@ -1361,52 +1357,3 @@ fn perm_change_shape_prefixes_rebuild_chain() { } assert_eq!(state_of(&q, id), State::Done); } - -#[test] -fn reparent_shape_is_a_lone_agentless_meta_window_node() { - // Single-move `set-parent` shape: one node, no rebuild subgraph (no - // container rebuild needed for a parent move), agentless like - // `MetaLock`, and it must declare the meta window — a topology commit - // must not land inside another node's staged deploy window. - let q = JobQueue::new(1); - let id = submit( - &q, - templates::reparent( - vec![(ident("alice"), Some(ident("bob")))], - Source::Manual, - "set-parent".to_owned(), - ), - ); - let c = claim_one(&q); - assert_eq!(c.kind.as_str(), "reparent"); - assert_eq!(c.agent, "", "Reparent is agentless — no per-agent lease"); - assert!( - c.kind.needs_meta_window(), - "a topology commit must hold the same MetaWindow as WritePermFile" - ); - assert!(!c.kind.needs_lease()); - assert!(!c.kind.needs_build_slot()); - q.complete_node(id, c.node_id, Ok(())); - assert_eq!(state_of(&q, id), State::Done); -} - -#[test] -fn reparent_bulk_shape_carries_every_move_on_one_node() { - // `set-parent-bulk`: still ONE node (one git commit, `moves.len() > 1`), - // not one node per move — bulk atomicity across every move in the - // request is the reason a single node was chosen in the first place. - let moves = vec![(ident("alice"), Some(ident("bob"))), (ident("carol"), None)]; - let q = JobQueue::new(1); - let id = submit( - &q, - templates::reparent(moves.clone(), Source::Manual, "set-parent-bulk".to_owned()), - ); - let c = claim_one(&q); - assert_eq!(c.kind.as_str(), "reparent"); - let NodeKind::Reparent { moves: got } = &c.kind else { - panic!("expected a Reparent node, got {:?}", c.kind); - }; - assert_eq!(got, &moves); - q.complete_node(id, c.node_id, Ok(())); - assert_eq!(state_of(&q, id), State::Done); -} diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index f9896c25..c17a90bd 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -562,10 +562,41 @@ pub async fn commit_perms( Ok(()) } -/// Applies every `(child, new_parent)` move under a single `META_LOCK` -/// acquisition and creates **one** git commit for all of them — a -/// single-move call is just a one-element slice, so there's no separate -/// non-batch entry point. Moves are applied in the order given; the first +/// Write the topology file and commit it atomically under `META_LOCK`. +/// Returns `Err(String)` on validation failure (unknown agent, cycle, +/// etc.) — same shape as `topology::set_parent` — so callers can +/// surface the error as a user-visible message. Git failures are +/// logged as warnings and don't propagate: the topology write already +/// succeeded, and `sync_agents` will pick up any un-committed change +/// on the next run as a safety net. +pub async fn commit_topology( + child: &str, + new_parent: Option<&str>, +) -> std::result::Result<(), String> { + let _guard = META_LOCK.lock().await; + crate::topology::set_parent(child, new_parent)?; + let dir = crate::paths::meta_root(); + let stage = async { + git(&dir, &["add", "topology.json"]).await?; + if paths_dirty(&dir, &["topology.json"]).await? { + git_commit_paths( + &dir, + &format!("topology: {} → {}", child, new_parent.unwrap_or("")), + &["topology.json"], + ) + .await?; + } + Ok::<_, anyhow::Error>(()) + }; + if let Err(e) = stage.await { + tracing::warn!(%child, ?new_parent, error = ?e, "commit_topology: topology written but git commit failed (sync_agents will recover)"); + } + Ok(()) +} + +/// 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. 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 diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index e560bb48..18cddca5 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -180,17 +180,19 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { } HostRequest::SetParent { child, new_parent } => { tracing::info!(%child, ?new_parent, "set_parent"); - // Fire-and-forget, like every other queue-backed op: - // submit returns a DAG id immediately, the caller polls - // `QueueDag` (`hivectl`'s wait/progress loop) for the - // outcome instead of blocking here on the commit. - let id = crate::job_queue::submit::reparent( - &coord, - vec![(child.clone(), new_parent.clone())], - crate::job_queue::Source::Manual, - "manual set-parent via hivectl".to_owned(), - ); - HostResponse::queued(vec![id]) + // `reparent_with_notify` wraps `topology::set_parent` + // with the three notification messages + the + // ContainerView rescan. Idempotent same-parent calls + // skip both the messages and the disk write per the + // topology fast-path. + coord + .reparent_with_notify( + child.as_str(), + new_parent.as_ref().map(hive_types::Ident::as_str), + ) + .await + .map_err(anyhow::Error::msg)?; + HostResponse::success() } HostRequest::SetResourceLimits { name,