diff --git a/hive-c0re/src/agent_config/topology.rs b/hive-c0re/src/agent_config/topology.rs index 08acd088..19c6c278 100644 --- a/hive-c0re/src/agent_config/topology.rs +++ b/hive-c0re/src/agent_config/topology.rs @@ -12,22 +12,8 @@ //! enforcement semantics: `docs/agent-hierarchy.md::Current state`. //! `` sentinel resolution (delivered by [`resolve_recipient`]): //! `docs/conventions.md::Recipient sentinels`. -//! -//! ## Graph representation -//! -//! The on-disk format stays as a flat JSON map `name → parent | null` -//! (small, git-diffable). In-memory, heavy algorithms (descendant checks, -//! cycle detection) use a [`petgraph`] directed graph where each edge runs -//! **parent → child**. This replaces the ad-hoc bounded walks that existed -//! before: petgraph's `has_path_connecting` / `is_cyclic_directed` -//! are correct for graphs of any depth (no 32-hop ceiling) and well-tested. -//! The graph is built on demand from the flat map; it is not cached across -//! calls (the map is small and disk I/O dominates anyway). use std::collections::BTreeMap; - -use petgraph::algo::{has_path_connecting, is_cyclic_directed}; -use petgraph::graph::{DiGraph, NodeIndex}; use std::path::PathBuf; const TOPOLOGY_FILE: &str = "topology.json"; @@ -150,75 +136,29 @@ pub fn resolve_recipient_in( } /// True when `candidate` is `ancestor` or any descendant of -/// `ancestor` per the current on-disk topology. -/// -/// Delegates to [`is_descendant_of_in`] on the result of [`read`] so -/// the algorithm is the same petgraph BFS used everywhere else. No -/// depth limit — the 32-hop bounded walk this replaced was correct for -/// any plausible hive but carried a latent ceiling; this has none. -/// -/// Used by the cancel-authorization checks in `socket_server` to enforce +/// `ancestor` per the current topology. Walks parents from +/// `candidate` upward; the walk terminates at root or on a cycle +/// (cycle defence: bounded to 32 hops, more than any plausible +/// hive depth). Used by the cancel-authorization check in +/// `socket_server::handle_cancel_schedule` to enforce /// "managers can cancel anything their subtree owns." #[must_use] pub fn is_descendant_of(candidate: &str, ancestor: &str) -> bool { - is_descendant_of_in(&read(), candidate, ancestor) -} - -/// Build an in-memory petgraph directed graph from the topology map. -/// -/// Edges run **parent → child** so that: -/// - `children_of(name)` = outgoing neighbours of `name`'s node -/// - `is_descendant_of(candidate, ancestor)` = path exists from `ancestor` -/// to `candidate` via `has_path_connecting` -/// - cycle detection = `is_cyclic_directed` after a speculative edge insert -/// -/// Returns the graph and a `BTreeMap` for O(log n) -/// name-to-node lookups. Both are local to each call site — the graph is -/// not cached. Hive topologies are small (< ~100 nodes); building on demand -/// is dominated by the surrounding disk read. -#[must_use] -fn build_graph( - topo: &BTreeMap>, -) -> (DiGraph, BTreeMap) { - let mut graph: DiGraph = DiGraph::new(); - let mut idx: BTreeMap = BTreeMap::new(); - - // Add one node per agent. - for name in topo.keys() { - let ni = graph.add_node(name.clone()); - idx.insert(name.clone(), ni); - } - // Add parent→child edges. - for (name, parent_opt) in topo { - if let Some(parent) = parent_opt - && let (Some(&p_idx), Some(&c_idx)) = (idx.get(parent), idx.get(name.as_str())) - { - graph.add_edge(p_idx, c_idx, ()); - } - } - (graph, idx) -} - -/// Return true when `candidate` is a descendant of `ancestor` in the -/// given topology map. Uses petgraph BFS/DFS (`has_path_connecting`) -/// — no depth limit and no manually bounded walk. Pure; no disk I/O. -/// -/// Same semantics as the disk-reading [`is_descendant_of`]: a node is -/// considered a descendant of itself (`candidate == ancestor` → true). -#[must_use] -pub fn is_descendant_of_in( - topo: &BTreeMap>, - candidate: &str, - ancestor: &str, -) -> bool { if candidate == ancestor { return true; } - let (graph, idx) = build_graph(topo); - let (Some(&anc_ni), Some(&cand_ni)) = (idx.get(ancestor), idx.get(candidate)) else { - return false; - }; - has_path_connecting(&graph, anc_ni, cand_ni, None) + let topo = read(); + let mut cur = candidate.to_owned(); + for _ in 0..32 { + let Some(parent) = topo.get(&cur).cloned().flatten() else { + return false; + }; + if parent == ancestor { + return true; + } + cur = parent; + } + false } /// Persist the topology map. Sorted JSON output (`BTreeMap` is sorted by @@ -286,19 +226,20 @@ pub fn apply_set_parent( if p == child { return Err("an agent cannot be its own parent".to_owned()); } - // Cycle check via petgraph: build the current graph, speculatively - // insert the proposed parent→child edge, then test for cycles with - // `is_cyclic_directed`. This replaces the earlier ad-hoc 32-hop - // ancestor walk — petgraph is correct for any tree depth and the - // algorithm is well-tested. - let (mut graph, idx) = build_graph(topo); - if let (Some(&p_ni), Some(&c_ni)) = (idx.get(p), idx.get(child)) { - graph.add_edge(p_ni, c_ni, ()); - if is_cyclic_directed(&graph) { + // Cycle check: walk `p`'s ancestors in the EXISTING map. If + // we hit `child`, then making `child`'s parent = `p` would + // close the loop (child → … → p → child). + let mut cur = p.to_owned(); + for _ in 0..32 { + if cur == child { return Err(format!( "cycle: {p} is in {child}'s subtree (would create a loop)" )); } + let Some(next) = topo.get(&cur).cloned().flatten() else { + break; + }; + cur = next; } } let mut next = topo.clone(); @@ -877,75 +818,6 @@ mod tests { assert!(top_level_agents_in(&topo).is_empty()); } - // ----------------------------------------------------------------------- - // is_descendant_of_in tests (petgraph-backed; pure / no disk I/O) - // ----------------------------------------------------------------------- - - #[test] - fn is_descendant_of_in_self_is_true() { - let topo = topo_three_level(); - assert!(is_descendant_of_in(&topo, "alice", "alice")); - assert!(is_descendant_of_in( - &topo, - crate::lifecycle::MANAGER_NAME, - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_direct_child() { - let topo = topo_three_level(); - // alice is a direct child of manager. - assert!(is_descendant_of_in( - &topo, - "alice", - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_grandchild() { - let topo = topo_three_level(); - // bob is manager → alice → bob; should be reachable from manager. - assert!(is_descendant_of_in( - &topo, - "bob", - crate::lifecycle::MANAGER_NAME - )); - assert!(is_descendant_of_in( - &topo, - "carol", - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_parent_not_descendant_of_child() { - let topo = topo_three_level(); - // alice is NOT a descendant of bob (alice is bob's grandparent). - assert!(!is_descendant_of_in(&topo, "alice", "bob")); - assert!(!is_descendant_of_in( - &topo, - crate::lifecycle::MANAGER_NAME, - "alice" - )); - } - - #[test] - fn is_descendant_of_in_sibling_is_not_descendant() { - let topo = topo_three_level(); - // bob and carol are siblings under alice; neither descends from the other. - assert!(!is_descendant_of_in(&topo, "bob", "carol")); - assert!(!is_descendant_of_in(&topo, "carol", "bob")); - } - - #[test] - fn is_descendant_of_in_unknown_is_false() { - let topo = topo_three_level(); - assert!(!is_descendant_of_in(&topo, "nobody", "alice")); - assert!(!is_descendant_of_in(&topo, "alice", "nobody")); - } - // ----------------------------------------------------------------------- // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) // -----------------------------------------------------------------------