hive-c0re: drop the subtree check from the scheduling verbs
The topology predicate `is_descendant_of` gated the four schedule-
managing verbs: a caller could only name a schedule owned by an agent at
or below itself in `topology.json`. Those gates now permit any requester,
so the predicate, its pure `_in` form and the `schedule_authorized`
wrapper built on it are gone rather than left returning a constant. The
other two wrappers went earlier with the verbs they served —
`require_descendant` with the lifecycle MCP verbs in 87970a8c, and
`resolve_agent_state_target` with `get_loose_ends`'s agent parameter.
`require_group(agent, "scheduling", ...)` is untouched and still fires at
dispatch for every one of the five scheduling verbs, so holding the tool
group remains the gate; what goes is the ownership restriction layered on
top of it.
The three schedule-mutating verbs keep their row lookup as a plain
existence check, so a caller naming a schedule that does not exist still
gets `not found` rather than a message from deeper in the cancel path.
`list_schedules` stops filtering per row: it would only have hidden rows
the requester may act on anyway.
Error messages, tool descriptions and docs that described the subtree
relation are reworded — a refusal message naming a topology that no
longer decides anything is worse than none.
The six `is_descendant_of_in` unit tests go with the function they test;
the permit behaviour they leave unasserted is picked up by the next
commit.
Refs #4472
This commit is contained in:
parent
bb0afcd256
commit
4121e11d87
8 changed files with 67 additions and 277 deletions
|
|
@ -16,17 +16,17 @@
|
|||
//! ## 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
|
||||
//! (small, git-diffable). In-memory, cycle detection uses 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.
|
||||
//! before: petgraph's `is_cyclic_directed`
|
||||
//! is 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::algo::is_cyclic_directed;
|
||||
use petgraph::graph::{DiGraph, NodeIndex};
|
||||
use std::path::PathBuf;
|
||||
|
||||
|
|
@ -141,27 +141,10 @@ 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
|
||||
/// "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<name → NodeIndex>` for O(log n)
|
||||
|
|
@ -191,28 +174,6 @@ fn build_graph(
|
|||
(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<String, Option<String>>,
|
||||
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)
|
||||
}
|
||||
|
||||
/// Persist the topology map. Sorted JSON output (`BTreeMap` is sorted by
|
||||
/// key) keeps git diffs minimal across re-writes. Best-effort —
|
||||
/// returns an `io::Error` so callers can decide whether a failure
|
||||
|
|
@ -742,75 +703,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)
|
||||
// -----------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue