From 6d2d0ed847cc9c2fe02889d89c65fea78132d06f Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 18:20:53 +0200 Subject: [PATCH 1/6] feat(#962): topology-driven child bind mounts + can_manage_top_level_agents role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the MANAGER_NAME special-case from set_nspawn_flags in favour of two general mechanisms: 1. Topology-driven child mounts: every agent now gets its direct children's state, harness, and config dirs bind-mounted (RW). Root's children are the top-level agents, so root gets the same access it did before via the old /agents blob bind — but derived from topology, not a hardcoded name check. 2. can_manage_top_level_agents role: agents holding this role additionally get every top-level agent treated as a virtual child (same RW mounts) plus /applied and /meta as RO. Designed for recovery: a role holder can update a top-level agent's config even when that agent is down. Root receives this role by default on first reconcile_roles call. Operator can revoke it with set_role. Every agent (including root) now gets its own state/harness/config dirs via the standard path. Roles are stored in meta/roles.json (same dir as topology.json); reconcile_roles is called from reconcile so both files stay in sync. --- hive-c0re/src/lifecycle.rs | 102 ++++++++++++++++----------------- hive-c0re/src/topology.rs | 112 +++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 50 deletions(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index 4e28c1a5..65595d78 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1064,6 +1064,23 @@ const HOST_META_ROOT: &str = "/var/lib/hyperhive/meta"; /// Shared directory accessible to all agents. All agents bind-mount this RW. const HOST_SHARED_ROOT: &str = "/var/lib/hyperhive/shared"; +/// Append bind flags for `child`'s state, harness, and config dirs into +/// `binds`. All three are RW so the parent can read/write state and +/// submit config-change requests. Creates missing host-side directories +/// so nspawn doesn't refuse to start; missing dirs are non-fatal. +fn bind_child_agent_dirs(child: &str, binds: &mut String) { + use std::fmt::Write as _; + let state_dir = format!("{HOST_AGENTS_ROOT}/{child}/state"); + let harness_dir = format!("{HOST_AGENTS_ROOT}/{child}/harness"); + let config_dir = format!("{HOST_AGENTS_ROOT}/{child}/config"); + for dir in [&state_dir, &harness_dir, &config_dir] { + let _ = std::fs::create_dir_all(dir); + } + let _ = write!(binds, " --bind={state_dir}:/agents/{child}/state"); + let _ = write!(binds, " --bind={harness_dir}:/agents/{child}/harness"); + let _ = write!(binds, " --bind={config_dir}:/agents/{child}/config"); +} + fn set_nspawn_flags( container: &str, runtime_dir: &Path, @@ -1097,21 +1114,17 @@ fn set_nspawn_flags( shared = HOST_SHARED_ROOT, ); - // Per-agent state + harness dirs. Skipped for the manager — - // the `/agents` bind below already exposes both (along with - // every sub-agent's). For regular agents the harness dir is - // the sibling of notes_dir (same parent, "harness" subdir). - if container != MANAGER_CONTAINER { + // Own state, harness, and config dirs — same for every agent including + // root. Config is RO: an agent must not edit its own config; changes + // only ever flow through the approval queue. + { let _ = write!( binds, " --bind={notes}:/agents/{agent_name}/state", notes = notes_dir.display(), ); - // Harness dir: sibling of notes_dir under the agent state root. - // systemd-nspawn refuses to start when the bind source is missing; - // ensure_state_dir already creates it, but be defensive here. - if let Some(parent) = notes_dir.parent() { - let harness_dir = parent.join("harness"); + if let Some(state_parent) = notes_dir.parent() { + let harness_dir = state_parent.join("harness"); if !harness_dir.exists() { let _ = std::fs::create_dir_all(&harness_dir); } @@ -1121,34 +1134,41 @@ fn set_nspawn_flags( harness = harness_dir.display(), ); } + let own_config = format!("{HOST_AGENTS_ROOT}/{agent_name}/config"); + std::fs::create_dir_all(&own_config) + .with_context(|| format!("create {own_config}"))?; + let _ = write!(binds, " --bind-ro={own_config}:/agents/{agent_name}/config"); } - if container == MANAGER_CONTAINER { + + // Topology-driven child mounts: every direct child of this agent gets + // its state, harness, and config dirs bind-mounted RW so the parent + // can read state and manage config. + let direct_children = crate::topology::children_of(agent_name); + for child in &direct_children { + bind_child_agent_dirs(child, &mut binds); + } + + // `can_manage_top_level_agents` role: additionally mount every + // top-level agent (direct child of root) as a virtual child. Enables + // recovery — a role holder can update a top-level agent's config even + // when that agent is down. Also grants RO access to /applied and /meta. + if crate::topology::has_role( + agent_name, + crate::topology::ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS, + ) { + let top_level = crate::topology::children_of(MANAGER_NAME); + for tl in &top_level { + if !direct_children.contains(tl) { + bind_child_agent_dirs(tl, &mut binds); + } + } // systemd-nspawn refuses to start a container whose bind // source doesn't exist. The meta repo is created by the // startup migration, but make sure the directory is there - // before the manager comes up in case set_nspawn_flags fires - // first (e.g. cold start with no agents). + // before the role holder comes up in case set_nspawn_flags + // fires first (e.g. cold start with no agents). std::fs::create_dir_all(HOST_META_ROOT) .with_context(|| format!("create {HOST_META_ROOT}"))?; - // Manager edits sub-agent proposed/ repos and its own. RW so it can - // git-commit. Sub-agents see only their own /run/hive socket and - // /root/.claude (no /agents or /applied). - // - // /applied is a separate RO mount of the hive-c0re-only applied - // repos so the manager can `git fetch /applied//.git - // refs/tags/*:refs/tags/applied/*` to mirror deployed/failed/ - // denied tags into its proposed clones and diff against - // what's actually deployed. RO bind makes destructive git - // plumbing inside the container unable to corrupt applied. - // - // /meta is a third RO mount exposing the system-wide deploy - // flake (`git log /meta --oneline` shows every deploy across - // every agent; `cat /meta/flake.lock` resolves which sha each - // agent is pinned at right now). - let _ = write!( - binds, - " --bind={HOST_AGENTS_ROOT}:{CONTAINER_MANAGER_AGENTS_MOUNT}", - ); let _ = write!( binds, " --bind-ro={HOST_APPLIED_ROOT}:{CONTAINER_MANAGER_APPLIED_MOUNT}", @@ -1158,24 +1178,6 @@ fn set_nspawn_flags( " --bind-ro={HOST_META_ROOT}:{mount}", mount = crate::meta::CONTAINER_MANAGER_META_MOUNT, ); - } else { - // Sub-agents get a READ-ONLY view of their own proposed - // config repo at /agents//config — agent.nix plus - // whatever extra files the manager split the config into. - // Lets an agent inspect exactly what defines it and request - // precise changes from the manager. RO is load-bearing: the - // agent must NOT edit its own config — changes only ever flow - // through the manager's RW proposed repo + the approval - // queue. The manager already has this dir RW via the /agents - // tree bind above. - let config_dir = format!("{HOST_AGENTS_ROOT}/{agent_name}/config"); - // nspawn refuses to start when a bind source is missing. - // `setup_proposed` seeds this dir before spawn reaches here, - // but create defensively so a missing repo degrades to an - // empty RO dir instead of a container that won't boot. - std::fs::create_dir_all(&config_dir).with_context(|| format!("create {config_dir}"))?; - let _ = write!(binds, " --bind-ro={config_dir}:/agents/{agent_name}/config"); - } // Web-socket subdir: bind-mount `/run/hive-agent//` into the // container so the harness can bind `web.sock` there and the host-side diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index 898bb433..0a120ee3 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -4,6 +4,10 @@ //! alongside the meta `flake.nix`, so topology changes thread through //! the same git commit log as deploys. //! +//! Agent roles are stored alongside in `roles.json` as a flat map of +//! `name → [role, ...]`. Roles gate additional bind-mount grants; see +//! `lifecycle::set_nspawn_flags` for the consumer. +//! //! Format, rationale, read/reconcile/inject/surface flow, and target //! enforcement semantics: `docs/agent-hierarchy.md::Current state`. //! `` sentinel resolution (delivered by [`resolve_recipient`]): @@ -278,6 +282,114 @@ pub fn reconcile(agent_names: &[String]) -> std::io::Result { if changed { write(¤t)?; } + // Keep roles in sync with the agent set. + reconcile_roles(agent_names)?; + Ok(changed) +} + +// --------------------------------------------------------------------------- +// Roles +// --------------------------------------------------------------------------- + +/// Agents with this role have the top-level agents (direct children of the +/// root/manager agent) added as virtual children for bind-mount and +/// config-change purposes. Enables recovery: if a top-level agent is down, +/// a role holder can still read its state and update its config. +/// +/// The root agent receives this role by default on first `reconcile_roles` +/// call; operators can revoke it with `set_role`. +pub const ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS: &str = "can_manage_top_level_agents"; + +const ROLES_FILE: &str = "roles.json"; + +#[must_use] +pub fn roles_path() -> std::path::PathBuf { + crate::meta::meta_dir().join(ROLES_FILE) +} + +/// Read the roles map from disk. Returns an empty map when absent or +/// unparsable — same safe-degradation pattern as `topology::read`. +#[must_use] +pub fn read_roles() -> BTreeMap> { + let Ok(raw) = std::fs::read_to_string(roles_path()) else { + return BTreeMap::new(); + }; + serde_json::from_str(&raw).unwrap_or_default() +} + +/// Persist the roles map. Sorted output keeps git diffs minimal. +pub fn write_roles(roles: &BTreeMap>) -> std::io::Result<()> { + let path = roles_path(); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let text = serde_json::to_string_pretty(roles) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + std::fs::write(&path, format!("{text}\n")) +} + +/// Return true when `name` holds `role`. +#[must_use] +pub fn has_role(name: &str, role: &str) -> bool { + has_role_in(&read_roles(), name, role) +} + +/// Pure form of [`has_role`] for unit tests. +#[must_use] +pub fn has_role_in(roles: &BTreeMap>, name: &str, role: &str) -> bool { + roles + .get(name) + .is_some_and(|rs| rs.iter().any(|r| r == role)) +} + +/// Grant or revoke a role for `name`. Idempotent — no disk write when the +/// state is already correct. +pub fn set_role(name: &str, role: &str, enabled: bool) -> Result<(), String> { + let mut roles = read_roles(); + let list = roles.entry(name.to_owned()).or_default(); + let held = list.iter().any(|r| r == role); + match (enabled, held) { + (true, false) => list.push(role.to_owned()), + (false, true) => list.retain(|r| r != role), + _ => return Ok(()), + } + if roles.get(name).is_some_and(|l| l.is_empty()) { + roles.remove(name); + } + write_roles(&roles).map_err(|e| format!("write roles.json: {e}")) +} + +/// Reconcile `roles.json` against the current agent set: +/// - Seeds root's default `can_manage_top_level_agents` role on first +/// appearance (operator can revoke with `set_role`). +/// - Drops entries for agents that no longer exist. +/// +/// Returns true when the file changed. +pub fn reconcile_roles(agent_names: &[String]) -> std::io::Result { + let mut roles = read_roles(); + let mut changed = false; + + let root = crate::lifecycle::MANAGER_NAME; + if agent_names.iter().any(|n| n == root) && !roles.contains_key(root) { + roles.insert( + root.to_owned(), + vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], + ); + changed = true; + } + + let known: std::collections::HashSet<_> = agent_names.iter().collect(); + roles.retain(|name, _| { + let keep = known.contains(name); + if !keep { + changed = true; + } + keep + }); + + if changed { + write_roles(&roles)?; + } Ok(changed) } From d79df752d64b5de388bde3a8925430c5a31f867b Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 18:31:09 +0200 Subject: [PATCH 2/6] fix(#962): keep empty role entries as tombstone; add role unit tests set_role previously removed empty entries after revoke, causing reconcile_roles to re-seed the role on the next tick (absent key = never seen = seed). Fix: never remove empty entries; an empty list is a tombstone meaning "explicitly revoked". Also adds unit tests for has_role_in, set_role revoke semantics, and the reconcile_roles seed/no-seed distinction (pure in-memory, no disk). --- hive-c0re/src/topology.rs | 86 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 3 deletions(-) diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index 0a120ee3..662c7586 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -344,6 +344,11 @@ pub fn has_role_in(roles: &BTreeMap>, name: &str, role: &str /// Grant or revoke a role for `name`. Idempotent — no disk write when the /// state is already correct. +/// +/// Empty role lists are kept in the map (never removed). An absent key means +/// "never seen" (seed on next `reconcile_roles`); an empty list means +/// "explicitly revoked" (do not re-seed). Callers that want to remove an +/// agent from the map entirely should use `reconcile_roles` (agent departure). pub fn set_role(name: &str, role: &str, enabled: bool) -> Result<(), String> { let mut roles = read_roles(); let list = roles.entry(name.to_owned()).or_default(); @@ -353,9 +358,8 @@ pub fn set_role(name: &str, role: &str, enabled: bool) -> Result<(), String> { (false, true) => list.retain(|r| r != role), _ => return Ok(()), } - if roles.get(name).is_some_and(|l| l.is_empty()) { - roles.remove(name); - } + // Intentionally do NOT remove empty entries — an empty list signals an + // explicit revoke and prevents reconcile_roles from re-seeding the role. write_roles(&roles).map_err(|e| format!("write roles.json: {e}")) } @@ -618,4 +622,80 @@ mod tests { let topo = topo_three_level(); assert!(children_of_in(&topo, "nobody").is_empty()); } + + // ----------------------------------------------------------------------- + // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) + // ----------------------------------------------------------------------- + + #[test] + fn has_role_in_returns_true_when_role_held() { + let mut roles = BTreeMap::new(); + roles.insert( + "alice".to_owned(), + vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], + ); + assert!(has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS)); + } + + #[test] + fn has_role_in_returns_false_for_absent_agent() { + let roles: BTreeMap> = BTreeMap::new(); + assert!(!has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS)); + } + + #[test] + fn has_role_in_returns_false_for_empty_list() { + let mut roles = BTreeMap::new(); + roles.insert("alice".to_owned(), vec![]); + assert!(!has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS)); + } + + /// Revoking a role must leave the key present with an empty list so + /// `reconcile_roles` does not re-seed it. + #[test] + fn set_role_revoke_keeps_empty_entry_as_tombstone() { + // Build an in-memory roles map as set_role would see it after granting. + let mut roles: BTreeMap> = BTreeMap::new(); + roles.insert( + "root".to_owned(), + vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], + ); + + // Simulate the revoke path of set_role (in-memory, no disk). + let list = roles.entry("root".to_owned()).or_default(); + let held = list.iter().any(|r| r == ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); + assert!(held); + list.retain(|r| r != ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); + + // Key must still be present (tombstone), just with an empty list. + assert!(roles.contains_key("root"), "empty entry must not be removed"); + assert!(roles["root"].is_empty()); + } + + /// `reconcile_roles` must not re-seed root when its entry exists but is + /// empty (operator explicitly revoked the role). + #[test] + fn reconcile_roles_in_does_not_reseed_after_explicit_revoke() { + let agent_names = vec!["root".to_owned(), "alice".to_owned()]; + let mut roles: BTreeMap> = BTreeMap::new(); + // Tombstone: root was seen before but all roles were revoked. + roles.insert("root".to_owned(), vec![]); + + let root = crate::lifecycle::MANAGER_NAME; + let root_present = agent_names.iter().any(|n| n == root); + let should_seed = root_present && !roles.contains_key(root); + // should_seed must be false because "root" key is present (tombstone). + assert!(!should_seed, "reconcile_roles must not re-seed an explicit revoke"); + } + + /// `reconcile_roles` seeds root on first appearance (no prior entry). + #[test] + fn reconcile_roles_in_seeds_root_when_absent() { + let agent_names = vec!["root".to_owned(), "alice".to_owned()]; + let roles: BTreeMap> = BTreeMap::new(); // empty + + let root = crate::lifecycle::MANAGER_NAME; + let should_seed = agent_names.iter().any(|n| n == root) && !roles.contains_key(root); + assert!(should_seed, "reconcile_roles must seed root when absent"); + } } From 828be8e2c8d54735661acf5646c3e286187a4044 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 18:54:37 +0200 Subject: [PATCH 3/6] refactor(#962): replace children_of(MANAGER_NAME) with top_level_agents() Add topology::top_level_agents_in(topo) and top_level_agents() which find the topology root by structure (parent=None) rather than by name, then return its children. Lifecycle.rs role logic now uses this instead of children_of(MANAGER_NAME), removing the hardcoded manager-name reference from the bind-mount logic. Also adds two unit tests for top_level_agents_in. --- hive-c0re/src/lifecycle.rs | 2 +- hive-c0re/src/topology.rs | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index 65595d78..0a30a9df 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1156,7 +1156,7 @@ fn set_nspawn_flags( agent_name, crate::topology::ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS, ) { - let top_level = crate::topology::children_of(MANAGER_NAME); + let top_level = crate::topology::top_level_agents(); for tl in &top_level { if !direct_children.contains(tl) { bind_child_agent_dirs(tl, &mut binds); diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index 662c7586..01378307 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -74,6 +74,34 @@ pub fn children_of_in( .collect() } +/// Return the agents that are direct children of the topology root +/// (the unique agent whose own parent is `None`). These are the +/// "top-level" agents — one layer below the auto-managed manager. +/// +/// Callers should use this instead of `children_of(MANAGER_NAME)` so +/// the manager's logical name is not hardcoded outside lifecycle.rs. +/// If the topology has no root agent (e.g. empty file on cold boot), +/// returns an empty vec. +#[must_use] +pub fn top_level_agents() -> Vec { + top_level_agents_in(&read()) +} + +/// Pure form of [`top_level_agents`] for unit tests. +#[must_use] +pub fn top_level_agents_in(topo: &BTreeMap>) -> Vec { + // Find the unique root (parent = None). If none or multiple exist, + // fall back to empty — the topology is malformed or uninitialised. + let roots: Vec<&str> = topo + .iter() + .filter_map(|(name, parent)| if parent.is_none() { Some(name.as_str()) } else { None }) + .collect(); + match roots.as_slice() { + [root] => children_of_in(topo, root), + _ => vec![], + } +} + /// Resolve a magic recipient sentinel (currently just /// [`hive_sh4re::PARENT_RECIPIENT`]) to a real broker recipient at /// send time. Returns an owned `String` so callers can plug it @@ -623,6 +651,20 @@ mod tests { assert!(children_of_in(&topo, "nobody").is_empty()); } + #[test] + fn top_level_agents_in_returns_children_of_root() { + let topo = topo_three_level(); + // root's children = [alice]; bob+carol are under alice. + let top = top_level_agents_in(&topo); + assert_eq!(top, vec!["alice"]); + } + + #[test] + fn top_level_agents_in_empty_topo_returns_empty() { + let topo = BTreeMap::new(); + assert!(top_level_agents_in(&topo).is_empty()); + } + // ----------------------------------------------------------------------- // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) // ----------------------------------------------------------------------- From a4e0628ba183782b7c62f64fa15b829593827378 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 19:00:19 +0200 Subject: [PATCH 4/6] fix(#962): top_level_agents_in delegates to children_of(MANAGER_NAME) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous parent=None heuristic was wrong — the manager is not required to be the structural topology root (it can have a parent). Delegate to children_of_in(MANAGER_NAME) directly; topology.rs is the right place for this knowledge. Update comment in lifecycle.rs to say "direct child of the manager" instead of "direct child of root". --- hive-c0re/src/lifecycle.rs | 7 ++++--- hive-c0re/src/topology.rs | 24 +++++++----------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index 0a30a9df..b23db43b 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1149,9 +1149,10 @@ fn set_nspawn_flags( } // `can_manage_top_level_agents` role: additionally mount every - // top-level agent (direct child of root) as a virtual child. Enables - // recovery — a role holder can update a top-level agent's config even - // when that agent is down. Also grants RO access to /applied and /meta. + // top-level agent (direct child of the manager) as a virtual child. + // Enables recovery — a role holder can update a top-level agent's + // config even when that agent is down. Also grants RO access to + // /applied and /meta. if crate::topology::has_role( agent_name, crate::topology::ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS, diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index 01378307..fc2e9cd2 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -74,14 +74,13 @@ pub fn children_of_in( .collect() } -/// Return the agents that are direct children of the topology root -/// (the unique agent whose own parent is `None`). These are the -/// "top-level" agents — one layer below the auto-managed manager. +/// Return the direct children of the manager agent — the "top-level" +/// agents that role holders with `can_manage_top_level_agents` are +/// granted access to. /// -/// Callers should use this instead of `children_of(MANAGER_NAME)` so -/// the manager's logical name is not hardcoded outside lifecycle.rs. -/// If the topology has no root agent (e.g. empty file on cold boot), -/// returns an empty vec. +/// Callers outside `topology` should use this instead of +/// `children_of(MANAGER_NAME)` so the manager's logical name stays +/// confined to the topology module. #[must_use] pub fn top_level_agents() -> Vec { top_level_agents_in(&read()) @@ -90,16 +89,7 @@ pub fn top_level_agents() -> Vec { /// Pure form of [`top_level_agents`] for unit tests. #[must_use] pub fn top_level_agents_in(topo: &BTreeMap>) -> Vec { - // Find the unique root (parent = None). If none or multiple exist, - // fall back to empty — the topology is malformed or uninitialised. - let roots: Vec<&str> = topo - .iter() - .filter_map(|(name, parent)| if parent.is_none() { Some(name.as_str()) } else { None }) - .collect(); - match roots.as_slice() { - [root] => children_of_in(topo, root), - _ => vec![], - } + children_of_in(topo, crate::lifecycle::MANAGER_NAME) } /// Resolve a magic recipient sentinel (currently just From aae4be19bd8cc3acdabe60fffd262c9f8d134946 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 19:01:49 +0200 Subject: [PATCH 5/6] fix(#962): remove hardcoded "root" string from docstrings and tests Replace literal "root" with MANAGER_NAME constant in role tests; update ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS docstring to say "manager" not "root/manager agent". --- hive-c0re/src/topology.rs | 48 +++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index fc2e9cd2..2010fa5c 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -310,11 +310,11 @@ pub fn reconcile(agent_names: &[String]) -> std::io::Result { // --------------------------------------------------------------------------- /// Agents with this role have the top-level agents (direct children of the -/// root/manager agent) added as virtual children for bind-mount and -/// config-change purposes. Enables recovery: if a top-level agent is down, -/// a role holder can still read its state and update its config. +/// manager) added as virtual children for bind-mount and config-change +/// purposes. Enables recovery: if a top-level agent is down, a role holder +/// can still read its state and update its config. /// -/// The root agent receives this role by default on first `reconcile_roles` +/// The manager receives this role by default on first `reconcile_roles` /// call; operators can revoke it with `set_role`. pub const ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS: &str = "can_manage_top_level_agents"; @@ -686,48 +686,46 @@ mod tests { /// `reconcile_roles` does not re-seed it. #[test] fn set_role_revoke_keeps_empty_entry_as_tombstone() { + let mgr = crate::lifecycle::MANAGER_NAME; // Build an in-memory roles map as set_role would see it after granting. let mut roles: BTreeMap> = BTreeMap::new(); - roles.insert( - "root".to_owned(), - vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()], - ); + roles.insert(mgr.to_owned(), vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()]); // Simulate the revoke path of set_role (in-memory, no disk). - let list = roles.entry("root".to_owned()).or_default(); + let list = roles.entry(mgr.to_owned()).or_default(); let held = list.iter().any(|r| r == ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); assert!(held); list.retain(|r| r != ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS); // Key must still be present (tombstone), just with an empty list. - assert!(roles.contains_key("root"), "empty entry must not be removed"); - assert!(roles["root"].is_empty()); + assert!(roles.contains_key(mgr), "empty entry must not be removed"); + assert!(roles[mgr].is_empty()); } - /// `reconcile_roles` must not re-seed root when its entry exists but is - /// empty (operator explicitly revoked the role). + /// `reconcile_roles` must not re-seed the manager when its entry exists + /// but is empty (operator explicitly revoked the role). #[test] fn reconcile_roles_in_does_not_reseed_after_explicit_revoke() { - let agent_names = vec!["root".to_owned(), "alice".to_owned()]; + let mgr = crate::lifecycle::MANAGER_NAME; + let agent_names = vec![mgr.to_owned(), "alice".to_owned()]; let mut roles: BTreeMap> = BTreeMap::new(); - // Tombstone: root was seen before but all roles were revoked. - roles.insert("root".to_owned(), vec![]); + // Tombstone: manager was seen before but all roles were revoked. + roles.insert(mgr.to_owned(), vec![]); - let root = crate::lifecycle::MANAGER_NAME; - let root_present = agent_names.iter().any(|n| n == root); - let should_seed = root_present && !roles.contains_key(root); - // should_seed must be false because "root" key is present (tombstone). + let mgr_present = agent_names.iter().any(|n| n == mgr); + let should_seed = mgr_present && !roles.contains_key(mgr); + // should_seed must be false because manager key is present (tombstone). assert!(!should_seed, "reconcile_roles must not re-seed an explicit revoke"); } - /// `reconcile_roles` seeds root on first appearance (no prior entry). + /// `reconcile_roles` seeds the manager on first appearance (no prior entry). #[test] fn reconcile_roles_in_seeds_root_when_absent() { - let agent_names = vec!["root".to_owned(), "alice".to_owned()]; + let mgr = crate::lifecycle::MANAGER_NAME; + let agent_names = vec![mgr.to_owned(), "alice".to_owned()]; let roles: BTreeMap> = BTreeMap::new(); // empty - let root = crate::lifecycle::MANAGER_NAME; - let should_seed = agent_names.iter().any(|n| n == root) && !roles.contains_key(root); - assert!(should_seed, "reconcile_roles must seed root when absent"); + let should_seed = agent_names.iter().any(|n| n == mgr) && !roles.contains_key(mgr); + assert!(should_seed, "reconcile_roles must seed manager when absent"); } } From fd87cf9924e274f29ab5bb29d1086370c5ff0eae Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 1 Jun 2026 19:05:24 +0200 Subject: [PATCH 6/6] fix(#962): top_level_agents = parentless agents, not children of manager Per mara's design: the role grants access to every agent with no parent in the topology (parent=None), derived purely from structure. No agent name is hardcoded. In normal operation this is just the manager; any additional parentless agents the operator creates are also covered. Update ROLE docstring, lifecycle.rs comment, and unit tests accordingly. Add a multi-root test to document the behaviour with multiple parentless agents. --- hive-c0re/src/lifecycle.rs | 7 +++---- hive-c0re/src/topology.rs | 40 +++++++++++++++++++++++++------------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index b23db43b..a90e7cfa 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -1149,10 +1149,9 @@ fn set_nspawn_flags( } // `can_manage_top_level_agents` role: additionally mount every - // top-level agent (direct child of the manager) as a virtual child. - // Enables recovery — a role holder can update a top-level agent's - // config even when that agent is down. Also grants RO access to - // /applied and /meta. + // parentless agent in the topology as a virtual child. Enables + // recovery — a role holder can update those agents' configs even + // when they are down. Also grants RO access to /applied and /meta. if crate::topology::has_role( agent_name, crate::topology::ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS, diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index 2010fa5c..d02d30bb 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -74,13 +74,13 @@ pub fn children_of_in( .collect() } -/// Return the direct children of the manager agent — the "top-level" -/// agents that role holders with `can_manage_top_level_agents` are -/// granted access to. +/// Return every agent that has no parent in the topology. These are the +/// "top-level" agents a `can_manage_top_level_agents` role holder is +/// granted access to. No agent name is hardcoded — the set is derived +/// purely from topology structure. /// -/// Callers outside `topology` should use this instead of -/// `children_of(MANAGER_NAME)` so the manager's logical name stays -/// confined to the topology module. +/// In normal operation this is just the manager, but any agent the +/// operator explicitly places outside the hierarchy is also included. #[must_use] pub fn top_level_agents() -> Vec { top_level_agents_in(&read()) @@ -89,7 +89,9 @@ pub fn top_level_agents() -> Vec { /// Pure form of [`top_level_agents`] for unit tests. #[must_use] pub fn top_level_agents_in(topo: &BTreeMap>) -> Vec { - children_of_in(topo, crate::lifecycle::MANAGER_NAME) + topo.iter() + .filter_map(|(name, parent)| if parent.is_none() { Some(name.clone()) } else { None }) + .collect() } /// Resolve a magic recipient sentinel (currently just @@ -309,10 +311,10 @@ pub fn reconcile(agent_names: &[String]) -> std::io::Result { // Roles // --------------------------------------------------------------------------- -/// Agents with this role have the top-level agents (direct children of the -/// manager) added as virtual children for bind-mount and config-change -/// purposes. Enables recovery: if a top-level agent is down, a role holder -/// can still read its state and update its config. +/// Agents with this role have every parentless agent in the topology +/// (see `top_level_agents`) added as virtual children for bind-mount +/// and config-change purposes. Enables recovery: if a top-level agent +/// is down, a role holder can still read its state and update its config. /// /// The manager receives this role by default on first `reconcile_roles` /// call; operators can revoke it with `set_role`. @@ -642,11 +644,21 @@ mod tests { } #[test] - fn top_level_agents_in_returns_children_of_root() { + fn top_level_agents_in_returns_parentless_agents() { let topo = topo_three_level(); - // root's children = [alice]; bob+carol are under alice. + // Only the manager has no parent (alice/bob/carol all have parents). let top = top_level_agents_in(&topo); - assert_eq!(top, vec!["alice"]); + assert_eq!(top, vec![crate::lifecycle::MANAGER_NAME]); + } + + #[test] + fn top_level_agents_in_multi_root_returns_all_parentless() { + let mut topo = topo_three_level(); + // Simulate a second parentless agent alongside the manager. + topo.insert("orphan".to_owned(), None); + let mut top = top_level_agents_in(&topo); + top.sort(); + assert_eq!(top, vec![crate::lifecycle::MANAGER_NAME, "orphan"]); } #[test]