topology: operator-driven move-agent via set_parent (#486)

This commit is contained in:
damocles 2026-05-26 19:15:00 +02:00 committed by Mara
commit 5456377622
5 changed files with 245 additions and 0 deletions

View file

@ -72,6 +72,7 @@ pub async fn serve(port: u16, coord: Arc<Coordinator>) -> Result<()> {
.route("/cancel-reminder/{id}", post(post_cancel_reminder))
.route("/retry-reminder/{id}", post(post_retry_reminder))
.route("/request-spawn", post(post_request_spawn))
.route("/api/topology/set-parent", post(post_set_parent))
.route("/op-send", post(post_op_send))
.route("/meta-update", post(post_meta_update))
.route("/api/schedules", get(api_schedules).post(post_schedule_new))
@ -849,6 +850,17 @@ struct RequestSpawnForm {
name: String,
}
/// `POST /api/topology/set-parent` body. `child` is required.
/// `new_parent` may be:
/// - omitted entirely (form field absent) → no-op error,
/// - empty string → promote to root,
/// - non-empty → new parent's logical name.
#[derive(Deserialize)]
struct SetParentForm {
child: String,
new_parent: Option<String>,
}
#[derive(Deserialize)]
struct AnswerForm {
answer: String,
@ -1846,6 +1858,45 @@ async fn post_request_spawn(
}
}
/// `POST /api/topology/set-parent` — operator-driven parent move
/// (#486). Form fields: `child` (required, agent name), `new_parent`
/// (optional — empty / absent string ⇒ promote to root). Refuses
/// cycles, unknown agents, and reparenting the manager. On success
/// re-emits container snapshots so the dashboard tree repaints
/// without a refresh.
async fn post_set_parent(
State(state): State<AppState>,
Form(form): Form<SetParentForm>,
) -> Response {
let child = form.child.trim().to_owned();
if child.is_empty() {
return error_response("set-parent: `child` required");
}
// 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.
let new_parent = form
.new_parent
.as_deref()
.map(str::trim)
.filter(|s| !s.is_empty())
.map(str::to_owned);
match crate::topology::set_parent(&child, new_parent.as_deref()) {
Ok(()) => {
tracing::info!(
child = %child,
new_parent = ?new_parent,
"operator: set-parent via dashboard"
);
// Topology drives ContainerView.parent; refresh the
// snapshot so connected viewers see the new tree.
state.coord.rescan_containers_and_emit().await;
(StatusCode::OK, "ok").into_response()
}
Err(e) => error_response(&format!("set-parent {child} failed: {e}")),
}
}
async fn post_rebuild(State(state): State<AppState>, AxumPath(name): AxumPath<String>) -> Response {
let logical = strip_container_prefix(&name);
state.coord.rebuild_queue.enqueue(

View file

@ -102,6 +102,19 @@ enum Cmd {
Approve { id: i64 },
/// Deny a pending request by id.
Deny { id: i64 },
/// Move an agent in the topology tree (#486). Set `--parent` to
/// a new parent agent name; pass `--root` to promote the agent
/// to root (no parent). Refuses cycles, unknown agents, and
/// any attempt to reparent the manager.
SetParent {
child: String,
/// New parent agent name. Mutually exclusive with `--root`.
#[arg(long, conflicts_with = "root")]
parent: Option<String>,
/// Promote `child` to root (no parent).
#[arg(long)]
root: bool,
},
}
#[tokio::main]
@ -143,6 +156,20 @@ async fn main() -> Result<()> {
render(client::request(&cli.socket, HostRequest::Approve { id }).await?)
}
Cmd::Deny { id } => render(client::request(&cli.socket, HostRequest::Deny { id }).await?),
Cmd::SetParent {
child,
parent,
root,
} => {
let new_parent = if root { None } else { parent };
render(
client::request(
&cli.socket,
HostRequest::SetParent { child, new_parent },
)
.await?,
)
}
}
}

View file

@ -185,6 +185,17 @@ async fn dispatch(req: &HostRequest, coord: Arc<Coordinator>) -> HostResponse {
actions::deny(&coord, *id, None).await?;
HostResponse::success()
}
HostRequest::SetParent { child, new_parent } => {
tracing::info!(%child, ?new_parent, "set_parent");
crate::topology::set_parent(child, new_parent.as_deref())
.map_err(anyhow::Error::msg)?;
// ContainerView.parent is read from topology.json — a
// change here means every container row potentially
// moves in the dashboard tree. Rescan + diff-emit so
// open viewers repaint without polling.
coord.rescan_containers_and_emit().await;
HostResponse::success()
}
})
}
.await;

View file

@ -123,6 +123,72 @@ pub fn default_seed(agent_names: &[String]) -> BTreeMap<String, Option<String>>
out
}
/// Pure validation + apply for [`set_parent`]. Splits off so tests
/// can exercise the rules (cycle / unknown / manager-protect) on
/// an in-memory `BTreeMap` without touching the on-disk
/// `topology.json`. Returns either the post-move map (caller
/// writes it back) or a user-readable error string.
pub fn apply_set_parent(
topo: &BTreeMap<String, Option<String>>,
child: &str,
new_parent: Option<&str>,
) -> Result<BTreeMap<String, Option<String>>, String> {
if child == crate::lifecycle::MANAGER_NAME {
return Err("cannot reparent the manager — it is structurally root".to_owned());
}
if !topo.contains_key(child) {
return Err(format!("unknown agent: {child}"));
}
if let Some(p) = new_parent {
if !topo.contains_key(p) {
return Err(format!("unknown parent: {p}"));
}
if p == child {
return Err("an agent cannot be its own parent".to_owned());
}
// 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();
next.insert(child.to_owned(), new_parent.map(str::to_owned));
Ok(next)
}
/// Operator-driven parent move (#486 / #487). 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 (see #486 comment 5042). Once
/// sub-manager bind mounts land alongside #361, 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(&current, child, new_parent)?;
if next == current {
return Ok(());
}
write(&next).map_err(|e| format!("write topology.json: {e}"))
}
/// 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
@ -191,4 +257,85 @@ mod tests {
let seed = default_seed(&[]);
assert!(seed.is_empty());
}
fn topo_three_level() -> BTreeMap<String, Option<String>> {
let mut m = BTreeMap::new();
m.insert(crate::lifecycle::MANAGER_NAME.to_owned(), None);
m.insert(
"alice".to_owned(),
Some(crate::lifecycle::MANAGER_NAME.to_owned()),
);
m.insert("bob".to_owned(), Some("alice".to_owned()));
m.insert("carol".to_owned(), Some("alice".to_owned()));
m
}
#[test]
fn apply_set_parent_promotes_to_root() {
let next = apply_set_parent(&topo_three_level(), "alice", None).unwrap();
assert_eq!(next.get("alice"), Some(&None));
}
#[test]
fn apply_set_parent_reparents_under_sibling_subtree() {
// bob and carol both under alice; move carol under bob.
let next = apply_set_parent(&topo_three_level(), "carol", Some("bob")).unwrap();
assert_eq!(next.get("carol"), Some(&Some("bob".to_owned())));
}
#[test]
fn apply_set_parent_refuses_manager_move() {
let err =
apply_set_parent(&topo_three_level(), crate::lifecycle::MANAGER_NAME, None).unwrap_err();
assert!(err.contains("manager"), "err = {err}");
}
#[test]
fn apply_set_parent_refuses_unknown_child() {
let err = apply_set_parent(&topo_three_level(), "nobody", Some("alice")).unwrap_err();
assert!(err.contains("unknown agent"), "err = {err}");
}
#[test]
fn apply_set_parent_refuses_unknown_parent() {
let err = apply_set_parent(&topo_three_level(), "bob", Some("nobody")).unwrap_err();
assert!(err.contains("unknown parent"), "err = {err}");
}
#[test]
fn apply_set_parent_refuses_self() {
let err = apply_set_parent(&topo_three_level(), "alice", Some("alice")).unwrap_err();
assert!(err.contains("own parent"), "err = {err}");
}
#[test]
fn apply_set_parent_refuses_cycle() {
// bob's parent is alice; trying to make alice's parent =
// bob would close the loop alice → bob → alice.
let err = apply_set_parent(&topo_three_level(), "alice", Some("bob")).unwrap_err();
assert!(err.contains("cycle"), "err = {err}");
}
#[test]
fn apply_set_parent_refuses_deep_cycle() {
// Three-deep chain: manager → alice → bob → carol. Moving
// alice under carol would create the loop alice → carol → bob → alice.
let mut topo = BTreeMap::new();
topo.insert(crate::lifecycle::MANAGER_NAME.to_owned(), None);
topo.insert(
"alice".to_owned(),
Some(crate::lifecycle::MANAGER_NAME.to_owned()),
);
topo.insert("bob".to_owned(), Some("alice".to_owned()));
topo.insert("carol".to_owned(), Some("bob".to_owned()));
let err = apply_set_parent(&topo, "alice", Some("carol")).unwrap_err();
assert!(err.contains("cycle"), "err = {err}");
}
#[test]
fn apply_set_parent_is_idempotent_noop() {
// bob is already under alice — same value returned.
let next = apply_set_parent(&topo_three_level(), "bob", Some("alice")).unwrap();
assert_eq!(next, topo_three_level());
}
}

View file

@ -46,6 +46,15 @@ pub enum HostRequest {
Approve { id: i64 },
/// Deny a pending request by id.
Deny { id: i64 },
/// Move an agent in the topology tree (#486). Pass `new_parent =
/// None` to promote the agent to root, or `Some(name)` to set a
/// new parent. Refuses cycles, unknown agents, and any attempt
/// to reparent the manager (which is structurally root).
/// Pure topology-json edit today; bind-mount work follows in #361.
SetParent {
child: String,
new_parent: Option<String>,
},
}
#[derive(Debug, Clone, Serialize, Deserialize)]