diff --git a/docs/agent-hierarchy.md b/docs/agent-hierarchy.md index 2e3ce4b7..f6e6be8d 100644 --- a/docs/agent-hierarchy.md +++ b/docs/agent-hierarchy.md @@ -70,7 +70,11 @@ where system-level facts live. alongside its `flake.nix` regeneration. New agents land at their default position (manager as parent, manager itself as root); removed agents drop. Existing entries are preserved as-is so - operator overrides stick across regenerations. + operator overrides stick across regenerations. Pending-init agents + (an operator-approved proposed config repo but no container yet — + `Coordinator::pending_init_names`) are kept too, so the + `child -> parent` edge written when `request_init_config` is approved + survives the gap until the first apply-commit spawns the container. 3. **Inject**: `meta::render_flake` looks up each agent's parent and passes it to `mkAgent`. When non-null, the mkAgent body sets `HIVE_PARENT = parent` in the agent's systemd service environment @@ -121,7 +125,7 @@ can't: | variant | semantic | post-milestone | |---|---|---| -| `RequestInitConfig` | seed an agent's proposed config repo | **topology** — descendants only | +| `RequestInitConfig` | seed an agent's proposed config repo | **topology** — existing direct child (re-init) or a brand-new name (child added under self on approval); a name owned by a different parent is refused | | `RequestApplyCommit` | submit a commit sha for operator approval | **topology** — descendants only | | `Kill` / `Start` / `Restart` / `Update` | container lifecycle on an existing agent | **topology** — descendants only | | `RequestUpdateMetaInputs` | bump meta `flake.lock` | **per-agent cap** (root-only today; a future "let coder bump its own input" might grant it) | diff --git a/hive-c0re/src/actions.rs b/hive-c0re/src/actions.rs index 61c06b7e..16ff465e 100644 --- a/hive-c0re/src/actions.rs +++ b/hive-c0re/src/actions.rs @@ -314,6 +314,16 @@ async fn run_approval_init_config( notes_dir: std::path::PathBuf, ) -> Result<()> { let result: Result<()> = async { + // Place the new child under its requesting parent (carried in + // commit_ref by submit_init_config). An empty commit_ref means + // no explicit parent was named (privileged manager socket, or an + // approval queued before the new-child feature) — write no edge + // and let `topology::reconcile` assign the default position on + // first spawn, so this path never names a specific root agent. + if !approval.commit_ref.is_empty() { + crate::topology::add_child(&approval.agent, &approval.commit_ref) + .map_err(|e| anyhow::anyhow!("topology add_child: {e}"))?; + } lifecycle::setup_proposed(&proposed_dir, &approval.agent).await?; lifecycle::ensure_agent_state_subvolume(&approval.agent).await?; lifecycle::ensure_claude_dir(&claude_dir)?; diff --git a/hive-c0re/src/agent_server.rs b/hive-c0re/src/agent_server.rs index 8db42dfd..4b61bedb 100644 --- a/hive-c0re/src/agent_server.rs +++ b/hive-c0re/src/agent_server.rs @@ -456,6 +456,40 @@ fn require_child(agent: &str, target: &str, action: &str) -> Option Option { + if let Some(reason) = crate::dashboard::validate_agent_name(target) { + return Some(AgentResponse::Err { + message: format!("agent `{agent}` cannot {action} `{target}`: {reason}"), + }); + } + match crate::topology::read().get(target) { + // brand-new name — requester becomes the parent on approval. + None => None, + // already our direct child — re-init / config update path. + Some(Some(p)) if p == agent => None, + // owned by someone else, or a root agent — refuse. + Some(_) => Some(AgentResponse::Err { + message: format!( + "agent `{agent}` cannot {action} `{target}`: it already exists \ + under a different parent in the topology tree" + ), + }), + } +} + /// `GetLooseEnds` — resolve the (optionally cross-agent) target then /// read its loose ends. fn handle_get_loose_ends( @@ -696,11 +730,11 @@ fn handle_request_init_config( name: &str, description: Option, ) -> AgentResponse { - if let Some(err) = require_child(agent, name, "request_init_config for") { + if let Some(err) = require_new_child(agent, name, "request_init_config for") { return err; } tracing::info!(%agent, %name, "agent: request_init_config for child"); - match crate::manager_server::submit_init_config(coord, name, description) { + match crate::manager_server::submit_init_config(coord, name, Some(agent), description) { Ok(_id) => AgentResponse::Ok, Err(e) => AgentResponse::Err { message: format!("{e:#}"), @@ -717,7 +751,7 @@ async fn handle_request_apply_commit( commit_ref: &str, description: Option<&str>, ) -> AgentResponse { - if let Some(err) = require_child(agent, target_agent, "request_apply_commit for") { + if let Some(err) = require_new_child(agent, target_agent, "request_apply_commit for") { return err; } tracing::info!(%agent, %target_agent, %commit_ref, "agent: request_apply_commit for child"); diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 451a94a2..73de3a5a 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -1403,4 +1403,21 @@ impl Coordinator { out.sort(); out } + + /// Agents that have an operator-approved proposed config repo but + /// were never deployed — proposed `.git` exists, applied `.git` does + /// not. Their `topology.json` parent edge (written at `InitConfig` + /// approval) must survive `topology::reconcile` until the first + /// apply-commit spawns the container. Distinct from tombstones, + /// which have an applied repo from a prior deploy. + #[must_use] + pub fn pending_init_names() -> Vec { + Self::kept_state_names() + .into_iter() + .filter(|n| { + Self::agent_proposed_dir(n).join(".git").exists() + && !Self::agent_applied_dir(n).join(".git").exists() + }) + .collect() + } } diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 64089084..d28c83ee 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1370,7 +1370,7 @@ async fn api_audit_log(State(state): State) -> Response { /// on reject — caller wraps the reason in a 400 response. Conservative /// whitelist matching `nixos-container` basename rules and the existing /// agent-name convention across the codebase. -fn validate_agent_name(name: &str) -> Option<&'static str> { +pub(crate) fn validate_agent_name(name: &str) -> Option<&'static str> { if name.is_empty() { return Some("agent name must not be empty"); } diff --git a/hive-c0re/src/manager_server.rs b/hive-c0re/src/manager_server.rs index 644bbad1..0df86f01 100644 --- a/hive-c0re/src/manager_server.rs +++ b/hive-c0re/src/manager_server.rs @@ -151,7 +151,11 @@ fn handle_manager_init_config( description: Option, ) -> ManagerResponse { tracing::info!(%name, "manager: request_init_config"); - match submit_init_config(coord, name, description) { + // No explicit parent edge from the privileged socket — the new + // agent takes `topology::reconcile`'s default position on first + // spawn. The agent socket is the path that records an explicit + // requester-as-parent edge. + match submit_init_config(coord, name, None, description) { Ok(_id) => ManagerResponse::Ok, Err(e) => ManagerResponse::Err { message: format!("{e:#}"), @@ -403,9 +407,20 @@ pub(crate) fn validate_commit_ref(commit_ref: &str) -> Result<()> { /// Queue an `InitConfig` approval for a brand-new agent whose config repo /// does not yet exist. Shared between the manager and agent sockets. +/// +/// `parent`, when `Some`, is the agent that will own the new child once +/// the operator approves: it is stashed in the approval's `commit_ref` +/// field (unused for `InitConfig` otherwise — same pattern +/// `UpdateMetaInputs` uses to carry its inputs JSON) and consumed in +/// `run_approval_init_config` to write the `child -> parent` topology +/// edge. The agent socket passes the requesting agent. `None` (the +/// privileged manager socket) writes no explicit edge — the new agent +/// lands at `topology::reconcile`'s default position when it first +/// spawns, so no caller has to name a specific root agent here. pub(crate) fn submit_init_config( coord: &Arc, name: &str, + parent: Option<&str>, description: Option, ) -> anyhow::Result { let proposed_dir = crate::coordinator::Coordinator::agent_proposed_dir(name); @@ -421,7 +436,7 @@ pub(crate) fn submit_init_config( .submit_kind( name, hive_sh4re::ApprovalKind::InitConfig, - "", + parent.unwrap_or(""), description.as_deref(), ) .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 36dec055..0e7d7ade 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -115,7 +115,8 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { // only fills in missing entries. Idempotent; when nothing changed // the file isn't touched. let agent_names: Vec = agents.iter().map(|a| a.name.clone()).collect(); - crate::topology::reconcile(&agent_names) + let pending = crate::coordinator::Coordinator::pending_init_names(); + crate::topology::reconcile(&agent_names, &pending) .with_context(|| format!("reconcile {}", crate::topology::topology_path().display()))?; // Refresh /var/lib/hyperhive/run/agent-ports.json so the hive-gateway diff --git a/hive-c0re/src/topology.rs b/hive-c0re/src/topology.rs index b14eb249..3656c2a0 100644 --- a/hive-c0re/src/topology.rs +++ b/hive-c0re/src/topology.rs @@ -272,42 +272,108 @@ pub fn set_parent(child: &str, new_parent: Option<&str>) -> Result<(), String> { 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 [`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 +/// [`reconcile`] would otherwise default it to the manager). +/// +/// Idempotent when the edge already exists. Refuses to clobber an entry +/// whose parent differs (one agent can't steal another's child) and +/// validates that `parent` is itself a known agent. +pub fn add_child(child: &str, parent: &str) -> Result<(), String> { + let current = read(); + match apply_add_child(¤t, child, parent)? { + Some(next) => write(&next).map_err(|e| format!("write topology.json: {e}")), + None => Ok(()), + } +} + +/// Pure form of [`add_child`] for unit tests. Returns the post-insert +/// map (caller writes it back), `None` for an idempotent no-op (edge +/// already present), or an error string (unknown parent / name owned by +/// a different parent). +pub fn apply_add_child( + topo: &BTreeMap>, + child: &str, + parent: &str, +) -> Result>>, String> { + if !topo.contains_key(parent) { + return Err(format!("unknown parent: {parent}")); + } + match topo.get(child) { + Some(Some(p)) if p == parent => return Ok(None), + Some(existing) => { + return Err(format!( + "agent {child} already exists in topology (parent: {existing:?}); \ + refusing to reparent via add_child" + )); + } + None => {} + } + let mut next = topo.clone(); + next.insert(child.to_owned(), Some(parent.to_owned())); + Ok(Some(next)) +} + /// Reconcile `topology.json` against the current agent set. Adds an /// entry (default: parent = manager, manager itself = root) for any /// agent missing from the file; removes entries for agents no longer /// present. Existing entries are preserved as-is — operator/manager /// choices stick across regenerations. Returns true when the file /// changed and should be re-committed by the caller. -pub fn reconcile(agent_names: &[String]) -> std::io::Result { - let mut current = read(); +/// +/// `pending` lists agents that have an operator-approved proposed config +/// repo but no container yet (init'd, not yet spawned). They are KEPT +/// (not dropped) so the parent edge written at `InitConfig` approval +/// survives until the first apply-commit, but they are NOT seeded with a +/// default parent here and NOT added to roles — that happens when the +/// container actually spawns and the name moves into `agent_names`. +pub fn reconcile(agent_names: &[String], pending: &[String]) -> std::io::Result { + let (next, changed) = apply_reconcile(&read(), agent_names, pending); + if changed { + write(&next)?; + } + // Keep roles in sync with the live agent set (pending agents get + // roles when they spawn). + reconcile_roles(agent_names)?; + Ok(changed) +} + +/// Pure form of [`reconcile`] for unit tests. Adds missing live agents +/// at their default position, drops entries for agents that are neither +/// live nor pending-init, and reports whether anything changed. +#[must_use] +pub fn apply_reconcile( + current: &BTreeMap>, + agent_names: &[String], + pending: &[String], +) -> (BTreeMap>, bool) { + let mut next = current.clone(); let mut changed = false; - // Add missing agents at their default position. for name in agent_names { - if !current.contains_key(name) { + if !next.contains_key(name) { let parent = if name == crate::lifecycle::MANAGER_NAME { None } else { Some(crate::lifecycle::MANAGER_NAME.to_owned()) }; - current.insert(name.clone(), parent); + next.insert(name.clone(), parent); changed = true; } } - // Drop entries for agents that no longer exist. - let known: std::collections::HashSet<_> = agent_names.iter().collect(); - current.retain(|name, _| { + let known: std::collections::HashSet<&String> = + agent_names.iter().chain(pending.iter()).collect(); + next.retain(|name, _| { let keep = known.contains(name); if !keep { changed = true; } keep }); - if changed { - write(¤t)?; - } - // Keep roles in sync with the agent set. - reconcile_roles(agent_names)?; - Ok(changed) + (next, changed) } // --------------------------------------------------------------------------- @@ -562,6 +628,90 @@ mod tests { assert_eq!(next, topo_three_level()); } + #[test] + fn apply_add_child_inserts_new_edge_under_parent() { + // alice spawns a brand-new child `dora`: the edge lands + // with alice as parent without disturbing the rest of the tree. + let next = apply_add_child(&topo_three_level(), "dora", "alice") + .unwrap() + .expect("brand-new edge should produce a map"); + assert_eq!(next.get("dora"), Some(&Some("alice".to_owned()))); + // existing entries untouched. + assert_eq!(next.get("bob"), Some(&Some("alice".to_owned()))); + } + + #[test] + fn apply_add_child_is_idempotent_when_edge_exists() { + // bob already under alice — re-init returns a no-op (None). + assert!( + apply_add_child(&topo_three_level(), "bob", "alice") + .unwrap() + .is_none() + ); + } + + #[test] + fn apply_add_child_refuses_unknown_parent() { + let err = apply_add_child(&topo_three_level(), "dora", "nobody").unwrap_err(); + assert!(err.contains("unknown parent"), "err = {err}"); + } + + #[test] + fn apply_add_child_refuses_name_owned_by_other_parent() { + // bob lives under alice; the manager can't claim it via add_child. + let err = apply_add_child(&topo_three_level(), "bob", crate::lifecycle::MANAGER_NAME) + .unwrap_err(); + assert!(err.contains("already exists"), "err = {err}"); + } + + #[test] + fn apply_reconcile_adds_missing_live_agent_under_manager() { + let live = vec![ + crate::lifecycle::MANAGER_NAME.to_owned(), + "newbie".to_owned(), + ]; + let (next, changed) = apply_reconcile(&BTreeMap::new(), &live, &[]); + assert!(changed); + assert_eq!( + next.get("newbie"), + Some(&Some(crate::lifecycle::MANAGER_NAME.to_owned())) + ); + } + + #[test] + fn apply_reconcile_drops_vanished_agent() { + let live = vec![ + crate::lifecycle::MANAGER_NAME.to_owned(), + "alice".to_owned(), + ]; + // carol + bob are gone from the live set and not pending. + let (next, changed) = apply_reconcile(&topo_three_level(), &live, &[]); + assert!(changed); + assert!(!next.contains_key("bob")); + assert!(!next.contains_key("carol")); + assert!(next.contains_key("alice")); + } + + #[test] + fn apply_reconcile_keeps_pending_init_agent_edge() { + // `dora` was init'd under alice (edge present) but has no + // container yet, so it's absent from the live set. It must NOT + // be dropped, and its alice-parent edge must be preserved (not + // re-seeded under the manager). + let mut topo = topo_three_level(); + topo.insert("dora".to_owned(), Some("alice".to_owned())); + let live = vec![ + crate::lifecycle::MANAGER_NAME.to_owned(), + "alice".to_owned(), + "bob".to_owned(), + "carol".to_owned(), + ]; + let pending = vec!["dora".to_owned()]; + let (next, changed) = apply_reconcile(&topo, &live, &pending); + assert!(!changed, "no change expected: {next:?}"); + assert_eq!(next.get("dora"), Some(&Some("alice".to_owned()))); + } + #[test] fn resolve_recipient_passes_through_ordinary_names() { let topo = topo_three_level();