diff --git a/hive-c0re/src/agent_config/capabilities.rs b/hive-c0re/src/agent_config/capabilities.rs index 9e077798..90e9f57b 100644 --- a/hive-c0re/src/agent_config/capabilities.rs +++ b/hive-c0re/src/agent_config/capabilities.rs @@ -22,7 +22,7 @@ use hive_sh4re::permissions::Capability; use std::collections::BTreeMap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; const CAPABILITIES_FILE: &str = "capabilities.json"; @@ -61,50 +61,35 @@ fn prune_unknown(map: &mut BTreeMap>) -> bool { dropped_any } -/// Read the per-agent capability map. Returns an empty map when the -/// file is absent or unparsable — callers treat a missing entry as -/// "no extra capabilities". Any entry that isn't a recognised -/// [`Capability`] is dropped (with a `warn!`) from what's returned — -/// a stale or typo'd name is never honoured — but `read` itself never -/// writes; the on-disk file only gets repaired the next time something -/// calls `write`/`set_caps` anyway. -#[must_use] -pub fn read() -> BTreeMap> { - let path = capabilities_path(); - let Ok(raw) = std::fs::read_to_string(&path) else { - return BTreeMap::new(); - }; - let mut map: BTreeMap> = serde_json::from_str(&raw).unwrap_or_default(); +/// Read the per-agent capability map. An absent file is the empty map — +/// callers treat a missing entry as "no extra capabilities". A file +/// that exists but can't be read or parsed is an error. Any entry that +/// isn't a recognised [`Capability`] is dropped (with a `warn!`) from +/// what's returned — a stale or typo'd name is never honoured — but +/// `read` itself never writes; the on-disk file only gets repaired the +/// next time something calls `set_caps`/`remove_agent` anyway. +pub fn read() -> std::io::Result>> { + read_from(&capabilities_path()) +} + +fn read_from(path: &Path) -> std::io::Result>> { + let mut map = super::read_map(path)?; prune_unknown(&mut map); - map + Ok(map) } /// Look up the configured capabilities for one agent. Returns an empty -/// vec when the agent has no entry. -#[must_use] -pub fn caps_for(name: &str) -> Vec { - read().get(name).cloned().unwrap_or_default() +/// vec when the agent has no entry. Errors as [`read`] does. +pub fn caps_for(name: &str) -> std::io::Result> { + Ok(read()?.get(name).cloned().unwrap_or_default()) } -/// Check whether an agent holds a specific capability. -#[must_use] -pub fn has_cap(name: &str, cap: hive_sh4re::permissions::Capability) -> bool { - caps_for(name) +/// Check whether an agent holds a specific capability. Errors as +/// [`read`] does. +pub fn has_cap(name: &str, cap: hive_sh4re::permissions::Capability) -> std::io::Result { + Ok(caps_for(name)? .iter() - .any(|s| s.eq_ignore_ascii_case(<&str>::from(cap))) -} - -/// Persist the full capability map. Sorted JSON output keeps diffs -/// minimal. Best-effort — returns `io::Error` so callers decide -/// whether to abort or log. -pub fn write(map: &BTreeMap>) -> std::io::Result<()> { - let path = capabilities_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - let text = serde_json::to_string_pretty(map) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - std::fs::write(&path, format!("{text}\n")) + .any(|s| s.eq_ignore_ascii_case(<&str>::from(cap)))) } /// Filter `caps` down to recognised [`Capability`] names (warning per @@ -137,20 +122,30 @@ fn apply_known_caps(current: &mut BTreeMap>, name: &str, cap /// rather than written — an unknown grant should never look like it /// took effect. An empty `caps` vec, or one that becomes empty after /// dropping unknown names, removes the entry (agent has no -/// capabilities). +/// capabilities). Fails without writing if the existing file can't be +/// read or parsed. pub fn set_caps(name: &str, caps: &[String]) -> std::io::Result<()> { - let mut current = read(); + set_caps_at(&capabilities_path(), name, caps) +} + +fn set_caps_at(path: &Path, name: &str, caps: &[String]) -> std::io::Result<()> { + let mut current = read_from(path)?; apply_known_caps(&mut current, name, caps); - write(¤t) + super::write_map(path, ¤t) } /// Remove an agent from the capability map entirely. Called by /// `meta::sync_agents` when an agent is deprovisioned so stale entries -/// don't accumulate. No-op if the agent has no entry. +/// don't accumulate. No-op if the agent has no entry. Fails without +/// writing if the existing file can't be read or parsed. pub fn remove_agent(name: &str) -> std::io::Result<()> { - let mut current = read(); + remove_agent_at(&capabilities_path(), name) +} + +fn remove_agent_at(path: &Path, name: &str) -> std::io::Result<()> { + let mut current = read_from(path)?; if current.remove(name).is_some() { - write(¤t)?; + super::write_map(path, ¤t)?; } Ok(()) } @@ -159,11 +154,51 @@ pub fn remove_agent(name: &str) -> std::io::Result<()> { mod tests { use super::*; - // `read`/`write`/`set_caps` shell out to `crate::paths::meta_root`, + // `read`/`set_caps`/`remove_agent` shell out to `crate::paths::meta_root`, // which is hardcoded to `/var/lib/hyperhive` (no test override) — so // these tests pin the pure decision logic (`prune_unknown`, - // `apply_known_caps`) directly against in-memory maps rather than - // round-tripping through the real capabilities file. + // `apply_known_caps`) against in-memory maps, and the file handling + // through the path-taking `*_at` / `read_from` variants. + + const TRUNCATED: &str = "{\n \"atlas\": [\"manage_root_ag"; + + fn corrupt_file() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join(CAPABILITIES_FILE); + std::fs::write(&path, TRUNCATED).expect("seed"); + (dir, path) + } + + #[test] + fn read_of_a_corrupt_file_is_an_error() { + let (_dir, path) = corrupt_file(); + read_from(&path).expect_err("truncated JSON must not read as a map"); + } + + #[test] + fn set_caps_leaves_a_corrupt_file_untouched() { + let (_dir, path) = corrupt_file(); + set_caps_at(&path, "ruth", &["manage_root_agent".to_owned()]) + .expect_err("a corrupt file must not be overwritten"); + assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes()); + } + + #[test] + fn remove_agent_leaves_a_corrupt_file_untouched() { + let (_dir, path) = corrupt_file(); + remove_agent_at(&path, "atlas").expect_err("a corrupt file must not be overwritten"); + assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes()); + } + + #[test] + fn set_caps_round_trips_through_a_missing_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join(CAPABILITIES_FILE); + assert!(read_from(&path).expect("missing file is empty").is_empty()); + set_caps_at(&path, "ruth", &["manage_root_agent".to_owned()]).expect("set"); + let map = read_from(&path).expect("read"); + assert_eq!(map["ruth"], vec!["manage_root_agent".to_owned()]); + } #[test] fn prune_unknown_keeps_known_name() { diff --git a/hive-c0re/src/agent_config/mod.rs b/hive-c0re/src/agent_config/mod.rs index 8fc2807b..249077bd 100644 --- a/hive-c0re/src/agent_config/mod.rs +++ b/hive-c0re/src/agent_config/mod.rs @@ -13,3 +13,91 @@ pub mod limits; pub mod resource_limits; pub mod tool_groups; pub mod topology; + +use std::collections::BTreeMap; +use std::io::{self, Write as _}; +use std::path::Path; + +/// Read a per-agent `name → [string]` JSON map. A missing file is the +/// empty map; any other read failure, or content that doesn't parse, is +/// an error, so a read-modify-write can't write back a map that has lost +/// every other agent's entry. +fn read_map(path: &Path) -> io::Result>> { + let raw = match std::fs::read_to_string(path) { + Ok(raw) => raw, + Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(BTreeMap::new()), + Err(e) => { + return Err(io::Error::new( + e.kind(), + format!("read {}: {e}", path.display()), + )); + } + }; + serde_json::from_str(&raw).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("parse {}: {e}", path.display()), + ) + }) +} + +/// Serialise `map` as pretty JSON and replace `path` with it atomically: +/// a temp file in the same directory (`rename(2)` is only atomic within a +/// filesystem), fsynced, renamed over `path`, then the directory fsynced +/// so the rename itself survives a crash. A reader sees the old file or +/// the new one, never a truncated one. +fn write_map(path: &Path, map: &BTreeMap>) -> io::Result<()> { + let text = serde_json::to_string_pretty(map) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; + let (Some(dir), Some(name)) = (path.parent(), path.file_name()) else { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} has no parent directory or file name", path.display()), + )); + }; + std::fs::create_dir_all(dir)?; + let tmp = dir.join(format!(".{}.tmp", name.to_string_lossy())); + let mut file = std::fs::File::create(&tmp)?; + file.write_all(format!("{text}\n").as_bytes())?; + file.sync_all()?; + drop(file); + std::fs::rename(&tmp, path)?; + std::fs::File::open(dir)?.sync_all() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn write_map_round_trips_and_leaves_no_temp_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("map.json"); + let map = BTreeMap::from([("alice".to_owned(), vec!["messaging".to_owned()])]); + write_map(&path, &map).expect("first write"); + let replaced = BTreeMap::from([("bob".to_owned(), vec!["inbox".to_owned()])]); + write_map(&path, &replaced).expect("replacing write"); + assert_eq!(read_map(&path).expect("read"), replaced); + let entries: Vec<_> = std::fs::read_dir(dir.path()) + .expect("read_dir") + .map(|e| e.expect("entry").file_name()) + .collect(); + assert_eq!(entries, vec![std::ffi::OsString::from("map.json")]); + } + + #[test] + fn read_map_of_a_missing_file_is_empty() { + let dir = tempfile::tempdir().expect("tempdir"); + let map = read_map(&dir.path().join("absent.json")).expect("missing file is not an error"); + assert!(map.is_empty()); + } + + #[test] + fn read_map_of_a_truncated_file_is_an_error() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("map.json"); + std::fs::write(&path, "{\n \"alice\": [\"messag").expect("seed"); + let err = read_map(&path).expect_err("truncated JSON must not read as a map"); + assert_eq!(err.kind(), io::ErrorKind::InvalidData); + } +} diff --git a/hive-c0re/src/agent_config/tool_groups.rs b/hive-c0re/src/agent_config/tool_groups.rs index 8c93c64b..04d5b9ec 100644 --- a/hive-c0re/src/agent_config/tool_groups.rs +++ b/hive-c0re/src/agent_config/tool_groups.rs @@ -22,7 +22,7 @@ //! that the operator uses to grant/revoke tool groups per agent. use std::collections::BTreeMap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::Context as _; @@ -33,37 +33,18 @@ pub fn tool_groups_path() -> PathBuf { crate::paths::meta_root().join(TOOL_GROUPS_FILE) } -/// Read the per-agent tool-group map. Returns an empty map when the -/// file is absent or unparsable — callers treat a missing entry as -/// "use role default". -#[must_use] -pub fn read() -> BTreeMap> { - let path = tool_groups_path(); - let Ok(raw) = std::fs::read_to_string(&path) else { - return BTreeMap::new(); - }; - serde_json::from_str(&raw).unwrap_or_default() +/// Read the per-agent tool-group map. An absent file is the empty map — +/// callers treat a missing entry as "use role default". A file that +/// exists but can't be read or parsed is an error. +pub fn read() -> std::io::Result>> { + super::read_map(&tool_groups_path()) } /// Look up the configured tool groups for one agent. Returns an empty /// vec when the agent has no entry — callers should treat this as -/// "use the harness role default." -#[must_use] -pub fn groups_for(name: &str) -> Vec { - read().get(name).cloned().unwrap_or_default() -} - -/// Persist the full tool-groups map. Sorted JSON output keeps diffs -/// minimal. Use `set_groups` (or `remove_agent`) from outside this -/// module — they go through the validated write path. -fn write(map: &BTreeMap>) -> std::io::Result<()> { - let path = tool_groups_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - let text = serde_json::to_string_pretty(map) - .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; - std::fs::write(&path, format!("{text}\n")) +/// "use the harness role default." Errors as [`read`] does. +pub fn groups_for(name: &str) -> std::io::Result> { + Ok(read()?.get(name).cloned().unwrap_or_default()) } /// Validate a slice of group name strings against `ToolGroup::ALL`. @@ -96,25 +77,76 @@ pub fn validate_groups(groups: &[String]) -> anyhow::Result<()> { /// Set the tool groups for one agent and persist the map. An empty /// `groups` vec removes the entry (agent reverts to role default). -/// Returns an error if any name is not in `ToolGroup::ALL`. +/// Returns an error if any name is not in `ToolGroup::ALL`, or if the +/// existing file can't be read or parsed — the file is then left +/// untouched. pub fn set_groups(name: &str, groups: &[String]) -> anyhow::Result<()> { + set_groups_at(&tool_groups_path(), name, groups) +} + +fn set_groups_at(path: &Path, name: &str, groups: &[String]) -> anyhow::Result<()> { if !groups.is_empty() { validate_groups(groups)?; } - let mut current = read(); + let mut current = super::read_map(path)?; if groups.is_empty() { current.remove(name); } else { current.insert(name.to_owned(), groups.to_vec()); } - write(¤t).with_context(|| format!("write tool-groups for {name}")) + super::write_map(path, ¤t).with_context(|| format!("write tool-groups for {name}")) } /// Drop the entry for an agent that is being destroyed. Idempotent. +/// Fails without writing if the existing file can't be read or parsed. pub fn remove_agent(name: &str) -> std::io::Result<()> { - let mut current = read(); + remove_agent_at(&tool_groups_path(), name) +} + +fn remove_agent_at(path: &Path, name: &str) -> std::io::Result<()> { + let mut current = super::read_map(path)?; if current.remove(name).is_some() { - write(¤t)?; + super::write_map(path, ¤t)?; } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + const TRUNCATED: &str = "{\n \"alice\": [\"messaging\", \"meta\"],\n \"bob\": [\"mess"; + + fn corrupt_file() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join(TOOL_GROUPS_FILE); + std::fs::write(&path, TRUNCATED).expect("seed"); + (dir, path) + } + + #[test] + fn set_groups_leaves_a_corrupt_file_untouched() { + let (_dir, path) = corrupt_file(); + set_groups_at(&path, "ruth", &["messaging".to_owned()]) + .expect_err("a corrupt file must not be overwritten"); + assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes()); + } + + #[test] + fn remove_agent_leaves_a_corrupt_file_untouched() { + let (_dir, path) = corrupt_file(); + remove_agent_at(&path, "alice").expect_err("a corrupt file must not be overwritten"); + assert_eq!(std::fs::read(&path).expect("read"), TRUNCATED.as_bytes()); + } + + #[test] + fn set_groups_keeps_other_agents_entries() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join(TOOL_GROUPS_FILE); + set_groups_at(&path, "alice", &["inbox".to_owned()]).expect("set on missing file"); + set_groups_at(&path, "ruth", &["messaging".to_owned()]).expect("set"); + let map = crate::agent_config::read_map(&path).expect("read"); + assert_eq!(map["alice"], vec!["inbox".to_owned()]); + assert_eq!(map["ruth"], vec!["messaging".to_owned()]); + } +} diff --git a/hive-c0re/src/coordinator.rs b/hive-c0re/src/coordinator.rs index 35da2088..06c48abe 100644 --- a/hive-c0re/src/coordinator.rs +++ b/hive-c0re/src/coordinator.rs @@ -643,7 +643,15 @@ impl Coordinator { .iter() .map(|c| (<&str>::from(*c), c.description())) .collect(); - let assignments = crate::capabilities::read(); + // An empty snapshot would show every agent as having lost its + // capabilities; emit nothing and let the HTTP refetch report it. + let assignments = match crate::capabilities::read() { + Ok(map) => map, + Err(e) => { + tracing::warn!(error = ?e, "capabilities snapshot skipped"); + return; + } + }; // Best-effort roster (sync path); on a contended cache miss we // emit explicit keys only — the HTTP refetch fills the rest in. let roster = self.live_container_names_blocking().unwrap_or_default(); @@ -670,7 +678,13 @@ impl Coordinator { .iter() .map(|g| (<&str>::from(*g), g.description())) .collect(); - let assignments = crate::tool_groups::read(); + let assignments = match crate::tool_groups::read() { + Ok(map) => map, + Err(e) => { + tracing::warn!(error = ?e, "tool-groups snapshot skipped"); + return; + } + }; let roster = self.live_container_names_blocking().unwrap_or_default(); let (agents, effective) = crate::dashboard::permissions::roster_and_effective( roster, diff --git a/hive-c0re/src/dashboard/permissions.rs b/hive-c0re/src/dashboard/permissions.rs index 121324c8..0ec25b05 100644 --- a/hive-c0re/src/dashboard/permissions.rs +++ b/hive-c0re/src/dashboard/permissions.rs @@ -42,12 +42,15 @@ pub(super) struct ToolGroupsSnapshot { #[utoipa::path( get, path = "/api/tool-groups", - responses((status = 200, description = "tool-group catalogue + assignments", body = ToolGroupsSnapshot)), + responses( + (status = 200, description = "tool-group catalogue + assignments", body = ToolGroupsSnapshot), + (status = 500, description = "tool-groups file unreadable"), + ), tag = "permissions" )] pub(super) async fn get_tool_groups( State(state): State, -) -> axum::Json { +) -> Result, ProblemDetails> { let groups = hive_sh4re::permissions::ToolGroup::ALL .iter() .map(|g| <&str>::from(*g)) @@ -56,7 +59,7 @@ pub(super) async fn get_tool_groups( .iter() .map(|g| (<&str>::from(*g), g.description())) .collect(); - let assignments = crate::tool_groups::read(); + let assignments = crate::tool_groups::read().map_err(|e| unreadable(&e))?; let roster = state .coord .containers_snapshot() @@ -65,13 +68,20 @@ pub(super) async fn get_tool_groups( .map(|c| c.name); let (agents, effective) = roster_and_effective(roster, &assignments, &tool_group_default_names()); - axum::Json(ToolGroupsSnapshot { + Ok(axum::Json(ToolGroupsSnapshot { groups, descriptions, assignments, agents, effective, - }) + })) +} + +/// A permission file that exists but can't be read or parsed. Surfaced +/// as a 500 rather than an empty table, which would read as "nobody has +/// any grants". +fn unreadable(e: &std::io::Error) -> ProblemDetails { + ProblemDetails::from_status_code(StatusCode::INTERNAL_SERVER_ERROR).with_detail(e.to_string()) } /// The role-default tool-group names the harness falls back to for an @@ -198,19 +208,22 @@ pub(super) struct CapabilitiesSnapshot { #[utoipa::path( get, path = "/api/capabilities", - responses((status = 200, description = "capability catalogue + assignments", body = CapabilitiesSnapshot)), + responses( + (status = 200, description = "capability catalogue + assignments", body = CapabilitiesSnapshot), + (status = 500, description = "capabilities file unreadable"), + ), tag = "permissions" )] pub(super) async fn get_capabilities( State(state): State, -) -> axum::Json { +) -> Result, ProblemDetails> { use hive_sh4re::permissions::Capability; let caps = Capability::ALL.iter().map(|c| <&str>::from(*c)).collect(); let descriptions = Capability::ALL .iter() .map(|c| (<&str>::from(*c), c.description())) .collect(); - let assignments = crate::capabilities::read(); + let assignments = crate::capabilities::read().map_err(|e| unreadable(&e))?; let roster = state .coord .containers_snapshot() @@ -219,13 +232,13 @@ pub(super) async fn get_capabilities( .map(|c| c.name); // Capability default is "no extra caps" — empty default slice. let (agents, effective) = roster_and_effective(roster, &assignments, &[]); - axum::Json(CapabilitiesSnapshot { + Ok(axum::Json(CapabilitiesSnapshot { caps, descriptions, assignments, agents, effective, - }) + })) } #[derive(Deserialize, ToSchema)] @@ -406,12 +419,15 @@ pub(super) struct StalePermsResponse { #[utoipa::path( get, path = "/api/permissions/stale", - responses((status = 200, description = "ghost agent names with stale permission entries", body = StalePermsResponse)), + responses( + (status = 200, description = "ghost agent names with stale permission entries", body = StalePermsResponse), + (status = 500, description = "tool-groups/capabilities file unreadable"), + ), tag = "permissions" )] pub(super) async fn get_stale_permissions( State(state): State, -) -> axum::Json { +) -> Result, ProblemDetails> { // Live container names — includes stopped-but-configured containers. let live: std::collections::HashSet = state .coord @@ -432,8 +448,8 @@ pub(super) async fn get_stale_permissions( // Known = live roster ∪ kept-state names. let known: std::collections::HashSet<&String> = live.iter().chain(kept.iter()).collect(); // Explicit entries in either JSON file. - let caps = crate::capabilities::read(); - let tgs = crate::tool_groups::read(); + let caps = crate::capabilities::read().map_err(|e| unreadable(&e))?; + let tgs = crate::tool_groups::read().map_err(|e| unreadable(&e))?; let mut ghost_names: Vec = caps .keys() .chain(tgs.keys()) @@ -443,7 +459,7 @@ pub(super) async fn get_stale_permissions( .into_iter() .collect(); ghost_names.sort(); - axum::Json(StalePermsResponse { stale: ghost_names }) + Ok(axum::Json(StalePermsResponse { stale: ghost_names })) } /// Clear all explicit permission entries for a named agent without diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index 4bf1da77..e1bb5bc4 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -339,7 +339,7 @@ async fn set_nspawn_flags( // parent field took with it the unconditional grant every // agent used to get over its own direct children — so an agent with // no capability now sees its own dirs and nothing else. - if crate::capabilities::has_cap(agent_name, Capability::ManageRootAgent) { + if crate::capabilities::has_cap(agent_name, Capability::ManageRootAgent)? { // Skipping self is a no-op, not a narrowing: `agent_notes_dir` is // `agent_state_dir/state` and `config_bind_source` is shared, so // binding the holder as its own virtual child reproduced the two diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index e510ab40..42fce888 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -102,7 +102,7 @@ pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { &hive.context_window_tokens, &hive.agent_memory_max, agents, - ); + )?; let flake_path = dir.join("flake.nix"); let on_disk = std::fs::read_to_string(&flake_path).unwrap_or_default(); let initial = !dir.join(".git").exists(); @@ -585,7 +585,7 @@ fn render_flake( context_window_tokens: &std::collections::HashMap, hive_memory_max: &str, agents: &[AgentSpec], -) -> String { +) -> Result { render_flake_with_lookup( hyperhive_flake, docs_flake, @@ -1000,7 +1000,7 @@ fn render_flake_with_lookup( hive_memory_max: &str, agents: &[AgentSpec], lookup: F, -) -> String +) -> Result where F: Fn(&str) -> Vec<&'static str>, { @@ -1316,8 +1316,8 @@ where nixosConfigurations = { "#, ); - let tool_groups_map = crate::tool_groups::read(); - let capabilities_map = crate::capabilities::read(); + let tool_groups_map = crate::tool_groups::read()?; + let capabilities_map = crate::capabilities::read()?; let resource_limits_map = crate::resource_limits::read(); for spec in agents { // Emit `toolGroups = "group1,group2"` when the operator has @@ -1370,7 +1370,7 @@ where ); } out.push_str(" };\n };\n}\n"); - out + Ok(out) } /// Return the list of file names that are currently staged (index differs @@ -1671,7 +1671,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); // nixpkgs is a top-level input with an explicit URL; hyperhive // follows it. assert!( @@ -1718,7 +1719,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); assert!( !out.contains("hyperhive-docs"), "no docs input/source when docs_flake is empty:\n{out}" @@ -1737,7 +1739,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); assert!( out.contains(r#"networking.hostName = "h-${name}";"#), "an agent container with no hostname of its own is called `nixos`, \ @@ -1778,7 +1781,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); assert!( out.contains( "services.hyperhive.agent.claudeCodePath = \"/nix/store/cccc-claude-code-2.1.220\";" @@ -1807,7 +1811,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); assert!( !out.contains("claudeCodePath"), "no claude assignment when unpinned:\n{out}" @@ -1828,7 +1833,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); assert!( out.contains("nixpkgs.follows = \"hyperhive/nixpkgs\""), "expected fallback follows:\n{out}" @@ -1864,7 +1870,8 @@ mod tests { sample_spec("dmatrix", false, 9003), ], lookup, - ); + ) + .expect("render flake"); // bitburner declares nixpkgs → follows emitted. assert!( out.contains("agent-bitburner.inputs.nixpkgs.follows = \"nixpkgs\""), @@ -1894,7 +1901,8 @@ mod tests { "4G", &[sample_spec("alice", false, 9001)], |_| Vec::new(), - ); + ) + .expect("render flake"); // No agent-side follows when the lookup reports nothing // declared — protects agents whose flake.lock can't be read // (missing / unparsable) from being broken by a follows on a @@ -1932,7 +1940,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); unsafe { std::env::remove_var("HIVE_FORGE_URL"); } @@ -2110,7 +2119,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); unsafe { std::env::remove_var("HIVE_FORGE_URL"); std::env::remove_var("HIVE_MATRIX_URL"); @@ -2147,7 +2157,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); let want = format!( "agent-alice.url = \"git+file://{}\"", crate::paths::applied_dir("alice").display() @@ -2193,6 +2204,7 @@ mod tests { "4G", &[sample_spec("alice", false, 9001)], ) + .expect("render flake") }; // A leftover peer-CA file + the env var that used to name it. Both @@ -2286,6 +2298,7 @@ mod tests { "4G", &[sample_spec("alice", false, 9001)], ) + .expect("render flake") }; unsafe { std::env::remove_var("HYPERHIVE_OTEL_EXTRA_RESOURCE_ATTRIBUTES"); @@ -2361,6 +2374,7 @@ mod tests { "4G", &[sample_spec("alice", false, 9001)], ) + .expect("render flake") }; unsafe { std::env::remove_var("HYPERHIVE_GITHUB_DISABLED"); @@ -2403,7 +2417,8 @@ mod tests { &std::collections::HashMap::new(), "4G", &[sample_spec("alice", false, 9001)], - ); + ) + .expect("render flake"); let want_bytes = 4u64 * 1024 * 1024 * 1024; assert!( out.contains(&format!("memoryMaxBytes = {want_bytes};")), diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 9b6c84b4..3777cc3e 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -598,15 +598,14 @@ async fn dispatch_orchestration(req: &Request, agent: &str, coord: &Arc Option { - if crate::tool_groups::groups_for(agent) - .iter() - .any(|g| g == group) - { - None - } else { - Some(Response::Err { + match crate::tool_groups::groups_for(agent) { + Ok(groups) if groups.iter().any(|g| g == group) => None, + Ok(_) => Some(Response::Err { message: format!("agent `{agent}` cannot {action}: requires the `{group}` tool group"), - }) + }), + Err(e) => Some(Response::Err { + message: format!("agent `{agent}` cannot {action}: {e}"), + }), } } @@ -699,6 +698,7 @@ fn handle_cancel_loose_end( fn check_can_cancel_approval(canceller: &str) -> Result<(), String> { const APPROVALS_GROUP: &str = "approvals"; if crate::tool_groups::groups_for(canceller) + .map_err(|e| format!("cancel_loose_end: {e}"))? .iter() .any(|g| g == APPROVALS_GROUP) { diff --git a/hive-c0re/src/workers/auto_update.rs b/hive-c0re/src/workers/auto_update.rs index ee5aa1dd..d2899ebc 100644 --- a/hive-c0re/src/workers/auto_update.rs +++ b/hive-c0re/src/workers/auto_update.rs @@ -212,11 +212,23 @@ pub async fn ensure_root_agent(coord: &Arc) -> Result<()> { /// /// Skips a name that already has an entry: a destroy+recreate under the /// same name must not silently reset an operator's chosen group set back -/// to the default. +/// to the default. An unreadable file is logged and left alone: ruth has +/// already been spawned, and seeding stays best-effort like the write +/// below. fn seed_manager_tool_groups() { - if !tool_groups::groups_for(MANAGER_NAME).is_empty() { - tracing::debug!("manager tool groups already set — leaving as-is"); - return; + match tool_groups::groups_for(MANAGER_NAME) { + Ok(groups) if groups.is_empty() => {} + Ok(_) => { + tracing::debug!("manager tool groups already set — leaving as-is"); + return; + } + Err(e) => { + tracing::warn!( + error = ?e, + "tool-groups file unreadable — not seeding ruth's tool groups" + ); + return; + } } let all_groups: Vec = hive_sh4re::permissions::ToolGroup::MANAGER_DEFAULT .iter() @@ -263,7 +275,7 @@ fn should_seed_manager_caps(store_written: bool) -> bool { /// entry that has been emptied rather than tombstoning it, so "the manager /// has no entry" cannot tell a fresh hive apart from a deliberate revoke. /// File existence can: every grant and revoke goes through -/// `meta::commit_capabilities` → `capabilities::set_caps` → `write`, which +/// `meta::commit_capabilities` → `capabilities::set_caps`, which /// writes the file even when the result is an empty `{}`. So while the file /// is absent nobody has ever had a say, and once it exists this is inert /// forever — including on the destroy+recreate path, matching