From a639a1ab43f67fca0871b24bb1621cb8079cc5f9 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 2 Sep 2026 13:10:49 +0200 Subject: [PATCH] hive-c0re: read_agent_status is not called from outside its module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Its doc justified the pub with "so socket_server and socket_server can populate AgentMeta" — the same module named twice, and both of those call read_agent_status_live instead. The bare function has exactly one caller, the wrapper eleven lines below it. container_view is not a pub module and this is a binary crate, so the pub only ever granted sibling access that nothing took. Also splits the parsing half of read_meta_locked_revs into parse_locked_revs, which needed a flake.lock on disk to exercise, and tests it: alias-to-rev mapping, a follows input (an array of path segments, which is why that arm is a continue), a node with no rev, and the malformed shapes that must yield an empty map rather than panic. --- hive-c0re/src/container_view.rs | 102 +++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 8 deletions(-) diff --git a/hive-c0re/src/container_view.rs b/hive-c0re/src/container_view.rs index e5aebd02..f6a84b2c 100644 --- a/hive-c0re/src/container_view.rs +++ b/hive-c0re/src/container_view.rs @@ -228,12 +228,13 @@ fn read_harness_flags(name: &hive_types::Ident) -> (bool, bool, bool) { /// Read the agent's free-text status and the Unix timestamp when it was last set /// (derived from the file's mtime). Returns `(None, None)` when the file is absent -/// or empty. `pub` so `socket_server` and `socket_server` can populate `AgentMeta`. +/// or empty. /// -/// NB: callers building `AgentMeta` for a *stopped* container should -/// clear the result — the on-disk status is a stale snapshot from -/// before the stop. Use `read_agent_status_live` for that. -pub fn read_agent_status(name: &hive_types::Ident) -> (Option, Option) { +/// NB: a caller building `AgentMeta` for a *stopped* container must clear +/// the result — the on-disk status is a stale snapshot from before the +/// stop. [`read_agent_status_live`] applies that rule, and is what the +/// socket server and the swarm status reader actually call. +fn read_agent_status(name: &hive_types::Ident) -> (Option, Option) { let path = Coordinator::agent_notes_dir(name).join("hyperhive-status"); let meta = std::fs::metadata(&path).ok(); // Read at most STATUS_MAX_CHARS * 4 + 2 bytes: 4 is the max UTF-8 byte @@ -322,11 +323,20 @@ pub fn hive_swarm_names() -> (Option, Option) { /// Map of `agent-` → locked sha from meta's flake.lock. Used to /// render the `deployed:` chip per container row. fn read_meta_locked_revs() -> HashMap { - let mut out = HashMap::new(); let Ok(raw) = std::fs::read_to_string(crate::paths::meta_flake_lock()) else { - return out; + return HashMap::new(); }; - let Ok(json) = serde_json::from_str::(&raw) else { + parse_locked_revs(&raw) +} + +/// The parsing half of [`read_meta_locked_revs`], split out so it can be +/// exercised without a `flake.lock` on disk. +/// +/// Every failure is the same empty map: the chip is decoration, and a +/// malformed lock must not take the dashboard down. +fn parse_locked_revs(raw: &str) -> HashMap { + let mut out = HashMap::new(); + let Ok(json) = serde_json::from_str::(raw) else { return out; }; let Some(nodes) = json.get("nodes").and_then(|v| v.as_object()) else { @@ -358,3 +368,79 @@ fn read_meta_locked_revs() -> HashMap { } out } + +#[cfg(test)] +mod tests { + use super::parse_locked_revs; + + /// Shape of a real `meta/flake.lock`: `root` names a node whose + /// `inputs` map alias → node name, and each node carries `locked.rev`. + fn lock(inputs: &str, nodes: &str) -> String { + format!(r#"{{"root":"root","nodes":{{"root":{{"inputs":{{{inputs}}}}},{nodes}}}}}"#) + } + + /// Keyed by the **alias** (what the chip renders), not by the node + /// name it points at. Those differ whenever nix dedupes a node — + /// `agent-bob` → `agent-bob_2` — so the fixture makes them differ, + /// otherwise the test cannot tell the two keyings apart. + #[test] + fn maps_each_alias_to_its_locked_rev() { + let raw = lock( + r#""agent-alice":"agent-alice","agent-bob":"agent-bob_2""#, + r#""agent-alice":{"locked":{"rev":"aaa111"}},"agent-bob_2":{"locked":{"rev":"bbb222"}}"#, + ); + let got = parse_locked_revs(&raw); + assert_eq!(got.get("agent-alice").map(String::as_str), Some("aaa111")); + assert_eq!( + got.get("agent-bob").map(String::as_str), + Some("bbb222"), + "keyed by alias, not by the deduped node name" + ); + assert!(!got.contains_key("agent-bob_2")); + assert_eq!(got.len(), 2); + } + + /// A `follows` input is stored as an array of path segments, not a + /// node name. Skipping it is why the match arm is a `continue`. + #[test] + fn a_follows_input_is_skipped_without_losing_its_siblings() { + let raw = lock( + r#""agent-alice":"agent-alice","nixpkgs":["agent-alice","nixpkgs"]"#, + r#""agent-alice":{"locked":{"rev":"aaa111"}}"#, + ); + let got = parse_locked_revs(&raw); + assert_eq!(got.len(), 1, "the sibling still resolves"); + assert!(!got.contains_key("nixpkgs")); + } + + #[test] + fn an_input_whose_node_has_no_rev_is_skipped() { + let raw = lock( + r#""agent-alice":"agent-alice","agent-bob":"agent-bob""#, + r#""agent-alice":{"locked":{"rev":"aaa111"}},"agent-bob":{"locked":{}}"#, + ); + let got = parse_locked_revs(&raw); + assert_eq!(got.len(), 1); + assert!(got.contains_key("agent-alice")); + } + + /// Every malformed shape yields an empty map rather than a panic — + /// the chip is decoration and must not take the dashboard down. + #[test] + fn malformed_input_yields_an_empty_map() { + for raw in [ + "not json at all", + "{}", + r#"{"root":"root"}"#, + r#"{"nodes":{"root":{"inputs":{}}}}"#, + r#"{"root":"missing","nodes":{"root":{"inputs":{"a":"a"}}}}"#, + ] { + assert!(parse_locked_revs(raw).is_empty(), "for input: {raw}"); + } + // Control: the well-formed shape these are degraded from does + // resolve, so the emptiness above is the guard and not the parser + // being inert. + let ok = lock(r#""a":"a""#, r#""a":{"locked":{"rev":"abc"}}"#); + assert_eq!(parse_locked_revs(&ok).len(), 1); + } +}