From a8ee894429a71541a96ac55c29bd5918b6249453 Mon Sep 17 00:00:00 2001
From: damocles <damocles@hyperhive.local>
Date: Mon, 1 Jun 2026 21:37:48 +0200
Subject: [PATCH] feat(#1013): validate tool-group names in set_groups against
 ToolGroup::ALL

---
 hive-c0re/src/tool_groups.rs | 39 +++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/hive-c0re/src/tool_groups.rs b/hive-c0re/src/tool_groups.rs
index c9341709..049f2e7b 100644
--- a/hive-c0re/src/tool_groups.rs
+++ b/hive-c0re/src/tool_groups.rs
@@ -26,6 +26,8 @@
 use std::collections::BTreeMap;
 use std::path::PathBuf;
 
+use anyhow::Context as _;
+
 const TOOL_GROUPS_FILE: &str = "tool-groups.json";
 
 #[must_use]
@@ -55,7 +57,8 @@ pub fn groups_for(name: &str) -> Vec<String> {
 
 /// Persist the full tool-groups map. Sorted JSON output keeps diffs
 /// minimal. Best-effort — returns `io::Error` so callers decide
-/// whether to abort or log.
+/// whether to abort or log. Prefer `set_groups` over calling this
+/// directly — `set_groups` validates group names before writing.
 pub fn write(map: &BTreeMap<String, Vec<String>>) -> std::io::Result<()> {
     let path = tool_groups_path();
     if let Some(parent) = path.parent() {
@@ -66,16 +69,46 @@ pub fn write(map: &BTreeMap<String, Vec<String>>) -> std::io::Result<()> {
     std::fs::write(&path, format!("{text}\n"))
 }
 
+/// Validate a slice of group name strings against `ToolGroup::ALL`.
+/// Returns `Ok(())` when all names are known, or `Err` listing the
+/// unrecognised names so callers can surface a useful error message.
+pub fn validate_groups(groups: &[String]) -> anyhow::Result<()> {
+    let valid: std::collections::BTreeSet<&str> =
+        hive_sh4re::ToolGroup::ALL.iter().map(|g| g.as_str()).collect();
+    let unknown: Vec<&str> = groups
+        .iter()
+        .map(String::as_str)
+        .filter(|s| !valid.contains(s))
+        .collect();
+    if unknown.is_empty() {
+        Ok(())
+    } else {
+        anyhow::bail!(
+            "unknown tool group(s): {}; valid names are: {}",
+            unknown.join(", "),
+            hive_sh4re::ToolGroup::ALL
+                .iter()
+                .map(|g| g.as_str())
+                .collect::<Vec<_>>()
+                .join(", ")
+        )
+    }
+}
+
 /// Set the tool groups for one agent and persist the map. An empty
 /// `groups` vec removes the entry (agent reverts to role default).
-pub fn set_groups(name: &str, groups: &[String]) -> std::io::Result<()> {
+/// Returns an error if any name is not in `ToolGroup::ALL`.
+pub fn set_groups(name: &str, groups: &[String]) -> anyhow::Result<()> {
+    if !groups.is_empty() {
+        validate_groups(groups)?;
+    }
     let mut current = read();
     if groups.is_empty() {
         current.remove(name);
     } else {
         current.insert(name.to_owned(), groups.to_vec());
     }
-    write(&current)
+    write(&current).with_context(|| format!("write tool-groups for {name}"))
 }
 
 /// Drop the entry for an agent that is being destroyed. Idempotent.