nix/dashboard: walk hyperhive subtree before agents in meta inputs panel (#638)
mara on #638: in the dashboard's inputs section, `nixpkgs` appeared under an `agent-*` path instead of `hyperhive/nixpkgs` where the operator expects it. Root cause (post-#632 follows refactor): - meta's top-level `nixpkgs.follows = "hyperhive/nixpkgs"` is a `follows` chain, rendered in `flake.lock` as an array — the `String` extractor in `walk_meta_inputs` correctly skips it (can't `nix flake update` a follows alias). - That left the root-level recursion to find `nixpkgs` only through some other input's subtree. - Recursion order was the BTreeMap's alphabetical key order, so `agent-z` (or any agent starting with a letter before `h`) got walked first and claimed `nixpkgs` at `agent-z/nixpkgs`. Hyperhive's subsequent walk skipped `nixpkgs` (already visited). Fix: sort `to_recurse` so hyperhive's subtree is descended first, matching the same "hyperhive first, then alpha" priority `read_meta_inputs` already uses for the final output ordering. Now `nixpkgs` is claimed under `hyperhive/nixpkgs` regardless of which agents the operator has spawned. Added regression test covering the exact post-#632 lock shape (`["hyperhive", "nixpkgs"]` follows array at root, agent-z alphabetically before hyperhive). Asserts the emitted path is `hyperhive/nixpkgs` and that `agent-z/nixpkgs` is NOT emitted (the spanning-tree visited set guarantees one claim per node). Closes #638.
This commit is contained in:
parent
459baebbb8
commit
e0070417fb
1 changed files with 82 additions and 8 deletions
|
|
@ -579,6 +579,19 @@ fn walk_meta_inputs(
|
||||||
}
|
}
|
||||||
to_recurse.push((target_name.clone(), path));
|
to_recurse.push((target_name.clone(), path));
|
||||||
}
|
}
|
||||||
|
// Recurse hyperhive's subtree before any agent's — without this,
|
||||||
|
// when meta's top-level `nixpkgs` is a `follows` alias (post-#632
|
||||||
|
// / #526) the `String` check above skips it, and the alphabetical
|
||||||
|
// BTreeMap iteration descends into `agent-*` first. The agent
|
||||||
|
// walk then claims `nixpkgs` at `agent-X/nixpkgs` instead of
|
||||||
|
// `hyperhive/nixpkgs`, which is where the operator expects it
|
||||||
|
// (closes #638). Sort by the same "hyperhive first, then alpha"
|
||||||
|
// priority `read_meta_inputs` uses for the final output.
|
||||||
|
to_recurse.sort_by(|(a, _), (b, _)| match (a.as_str(), b.as_str()) {
|
||||||
|
("hyperhive", _) => std::cmp::Ordering::Less,
|
||||||
|
(_, "hyperhive") => std::cmp::Ordering::Greater,
|
||||||
|
_ => a.cmp(b),
|
||||||
|
});
|
||||||
for (target_name, path) in to_recurse {
|
for (target_name, path) in to_recurse {
|
||||||
walk_meta_inputs(nodes, &target_name, &path, visited, out);
|
walk_meta_inputs(nodes, &target_name, &path, visited, out);
|
||||||
}
|
}
|
||||||
|
|
@ -1285,6 +1298,70 @@ mod tests {
|
||||||
p
|
p
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn walk_meta_inputs_keeps_nixpkgs_under_hyperhive_post_follows_refactor() {
|
||||||
|
// Reproduce the post-#632 (and pre-fix #638) shape: meta has
|
||||||
|
// `nixpkgs.follows = "hyperhive/nixpkgs"` at the top level
|
||||||
|
// (rendered as an array — `["hyperhive" "nixpkgs"]` — which
|
||||||
|
// walk_meta_inputs skips because we can't `nix flake update`
|
||||||
|
// a follows alias). The remaining top-level inputs are
|
||||||
|
// `hyperhive` (string) and `agent-z` (string). Without the
|
||||||
|
// hyperhive-first recursion sort, the BTreeMap alphabetical
|
||||||
|
// order descends into `agent-z` first and claims
|
||||||
|
// `nixpkgs` at `agent-z/nixpkgs`.
|
||||||
|
let raw = r#"{
|
||||||
|
"root": "root",
|
||||||
|
"version": 7,
|
||||||
|
"nodes": {
|
||||||
|
"root": {
|
||||||
|
"inputs": {
|
||||||
|
"hyperhive": "hyperhive",
|
||||||
|
"nixpkgs": ["hyperhive", "nixpkgs"],
|
||||||
|
"agent-z": "agent-z"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"hyperhive": {
|
||||||
|
"inputs": { "nixpkgs": "nixpkgs" },
|
||||||
|
"locked": {"rev": "hhrev", "lastModified": 1},
|
||||||
|
"original": {"url": "git+file:///tmp/hyperhive"}
|
||||||
|
},
|
||||||
|
"agent-z": {
|
||||||
|
"inputs": { "nixpkgs": "nixpkgs" },
|
||||||
|
"locked": {"rev": "azrev", "lastModified": 2},
|
||||||
|
"original": {"url": "git+file:///tmp/agent-z"}
|
||||||
|
},
|
||||||
|
"nixpkgs": {
|
||||||
|
"locked": {"rev": "npkrev", "lastModified": 3},
|
||||||
|
"original": {"url": "github:NixOS/nixpkgs/nixos-26.05"}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}"#;
|
||||||
|
let json: serde_json::Value = serde_json::from_str(raw).unwrap();
|
||||||
|
let nodes = json.get("nodes").unwrap().as_object().unwrap();
|
||||||
|
let root_name = json.get("root").unwrap().as_str().unwrap();
|
||||||
|
let mut visited = std::collections::HashSet::new();
|
||||||
|
visited.insert(root_name.to_owned());
|
||||||
|
let mut out = Vec::new();
|
||||||
|
walk_meta_inputs(nodes, root_name, "", &mut visited, &mut out);
|
||||||
|
|
||||||
|
let nixpkgs = out
|
||||||
|
.iter()
|
||||||
|
.find(|v| v.rev == "npkrev")
|
||||||
|
.expect("nixpkgs node should be emitted exactly once");
|
||||||
|
assert_eq!(
|
||||||
|
nixpkgs.name, "hyperhive/nixpkgs",
|
||||||
|
"nixpkgs should be claimed under hyperhive, not under agent-z (closes #638). \
|
||||||
|
got: {:?}",
|
||||||
|
nixpkgs.name
|
||||||
|
);
|
||||||
|
// And the agent-z path should NOT also carry a nixpkgs entry —
|
||||||
|
// the spanning-tree visited set guarantees it's claimed once.
|
||||||
|
assert!(
|
||||||
|
!out.iter().any(|v| v.name == "agent-z/nixpkgs"),
|
||||||
|
"agent-z/nixpkgs should not be emitted (already claimed under hyperhive)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn validate_agent_name_accepts_canonical_shapes() {
|
fn validate_agent_name_accepts_canonical_shapes() {
|
||||||
assert!(validate_agent_name("damocles").is_none());
|
assert!(validate_agent_name("damocles").is_none());
|
||||||
|
|
@ -1293,7 +1370,10 @@ mod tests {
|
||||||
assert!(validate_agent_name("snake_case").is_none());
|
assert!(validate_agent_name("snake_case").is_none());
|
||||||
assert!(validate_agent_name("mixed_2-3").is_none());
|
assert!(validate_agent_name("mixed_2-3").is_none());
|
||||||
let max = "a".repeat(63);
|
let max = "a".repeat(63);
|
||||||
assert!(validate_agent_name(&max).is_none(), "63-char name should pass");
|
assert!(
|
||||||
|
validate_agent_name(&max).is_none(),
|
||||||
|
"63-char name should pass"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// The two-axis guard (`guard_agent_name`) wires `validate_agent_name`
|
// The two-axis guard (`guard_agent_name`) wires `validate_agent_name`
|
||||||
|
|
@ -1864,13 +1944,7 @@ async fn guard_agent_name(state: &AppState, name: &str) -> Option<Response> {
|
||||||
}
|
}
|
||||||
let snapshot = state.coord.containers_snapshot().await;
|
let snapshot = state.coord.containers_snapshot().await;
|
||||||
if !snapshot.iter().any(|c| c.name == name) {
|
if !snapshot.iter().any(|c| c.name == name) {
|
||||||
return Some(
|
return Some((StatusCode::NOT_FOUND, format!("no such agent: {name}")).into_response());
|
||||||
(
|
|
||||||
StatusCode::NOT_FOUND,
|
|
||||||
format!("no such agent: {name}"),
|
|
||||||
)
|
|
||||||
.into_response(),
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue