From a4d15cea284f4d772a067aa4f21eef116c912c48 Mon Sep 17 00:00:00 2001 From: damocles Date: Tue, 9 Jun 2026 00:00:40 +0200 Subject: [PATCH] fix(#1548): make set_status always-on regardless of tool groups --- docs/conventions.md | 10 ++++++++- hive-ag3nt/src/mcp.rs | 49 +++++++++++++++++++++++++++++++++++++++++-- hive-sh4re/src/lib.rs | 17 ++++++++++++--- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/docs/conventions.md b/docs/conventions.md index 8a73a381..86112af0 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -278,7 +278,7 @@ binary flavor. | Group | Tools | |---|---| | `messaging` | `send`, `recv`, `ask`, `answer` | -| `meta` | `set_status`, `get_agent_meta` | +| `meta` | `get_agent_meta` (`set_status` is always-on, see below) | | `inbox` | `get_loose_ends`, `cancel_loose_end`, `remind`, `request_next_turn` | | `execution` | vestigial — `mcp__bash__run` / `mcp__bash__status` are always available unconditionally via `extraMcpServers`; this group's entries expand to non-existent `mcp__hyperhive__run` / `mcp__hyperhive__status` and have no effect. See `docs/tools/bash.md`. | | `lifecycle` | `kill`, `start`, `restart`, `update` *(privileged)* | @@ -286,6 +286,14 @@ binary flavor. | `scheduling` | `request_schedule_prompt`, `fire_schedule_now`, `cancel_schedule`, `edit_schedule`, `list_schedules` *(privileged)* | | `diagnostics` | `get_logs` *(privileged)* | +**Always-on tools** — `set_status` is exposed to every agent regardless of +which groups it holds (`ToolGroup::ALWAYS_ON_TOOLS`). The operator dashboard +depends on every agent being able to report its status chip, and the +server-side `SetStatus` handler has no tool-group check (only length +validation), so gating it would only desync the `--allowedTools` list from +what the host actually accepts. Revoking `meta` therefore drops +`get_agent_meta` but never `set_status`. + **Config storage** — per-agent tool groups live in `/var/lib/hyperhive/meta/tool-groups.json` (hive-c0re-owned, committed to the meta repo alongside `topology.json`). Format: `{ "alice": ["messaging", "meta", diff --git a/hive-ag3nt/src/mcp.rs b/hive-ag3nt/src/mcp.rs index e613aa92..c05bfaea 100644 --- a/hive-ag3nt/src/mcp.rs +++ b/hive-ag3nt/src/mcp.rs @@ -1847,10 +1847,14 @@ fn effective_tool_groups() -> Vec { #[must_use] pub fn allowed_mcp_tools(groups: &[hive_sh4re::ToolGroup]) -> Vec { // Collect all tool names, deduplicating while preserving order. + // Always-on tools (e.g. `set_status`) come first so they're present + // regardless of which groups the agent is granted — a misconfigured + // agent still has to be able to report its dashboard status. let mut seen = std::collections::HashSet::new(); - let mut out: Vec = groups + let mut out: Vec = hive_sh4re::ToolGroup::ALWAYS_ON_TOOLS .iter() - .flat_map(|g| g.tools()) + .copied() + .chain(groups.iter().flat_map(|g| g.tools().iter().copied())) .filter(|t| seen.insert(*t)) .map(|t| format!("mcp__{SERVER_NAME}__{t}")) .collect(); @@ -2076,3 +2080,44 @@ mod recv_hint_tests { assert_eq!(out, "(empty)"); } } + +#[cfg(test)] +mod allowed_tools_tests { + use super::{SERVER_NAME, allowed_mcp_tools}; + use hive_sh4re::ToolGroup; + + fn qualified(tool: &str) -> String { + format!("mcp__{SERVER_NAME}__{tool}") + } + + #[test] + fn set_status_present_with_no_groups() { + // The whole point of #1548: an agent with zero tool groups + // (or any group set that omits `meta`) must still be able to + // report its dashboard status. + let tools = allowed_mcp_tools(&[]); + assert!( + tools.contains(&qualified("set_status")), + "set_status missing from empty-group allow-list: {tools:?}" + ); + } + + #[test] + fn set_status_present_without_meta_group() { + let tools = allowed_mcp_tools(&[ToolGroup::Messaging, ToolGroup::Inbox]); + assert!(tools.contains(&qualified("set_status"))); + // get_agent_meta stays gated behind `meta` — only set_status is always-on. + assert!(!tools.contains(&qualified("get_agent_meta"))); + } + + #[test] + fn no_duplicate_set_status_when_meta_granted() { + let tools = allowed_mcp_tools(&[ToolGroup::Meta]); + let count = tools + .iter() + .filter(|t| **t == qualified("set_status")) + .count(); + assert_eq!(count, 1, "set_status duplicated: {tools:?}"); + assert!(tools.contains(&qualified("get_agent_meta"))); + } +} diff --git a/hive-sh4re/src/lib.rs b/hive-sh4re/src/lib.rs index 58ef97df..35c25df3 100644 --- a/hive-sh4re/src/lib.rs +++ b/hive-sh4re/src/lib.rs @@ -809,7 +809,7 @@ pub struct SchedulePromptPayload { pub enum ToolGroup { /// `send`, `recv`, `ask`, `answer` Messaging, - /// `set_status`, `get_agent_meta` + /// `get_agent_meta` (`set_status` is always-on — see `ALWAYS_ON_TOOLS`) Meta, /// `get_loose_ends`, `cancel_loose_end`, `remind`, `request_next_turn` Inbox, @@ -841,7 +841,7 @@ impl ToolGroup { pub fn tools(self) -> &'static [&'static str] { match self { Self::Messaging => &["send", "recv", "ask", "answer"], - Self::Meta => &["set_status", "get_agent_meta"], + Self::Meta => &["get_agent_meta"], Self::Inbox => &[ "get_loose_ends", "cancel_loose_end", @@ -867,6 +867,15 @@ impl ToolGroup { } } + /// MCP tools that are always exposed regardless of which tool groups an + /// agent is granted. `set_status` lives here because the operator + /// dashboard depends on every agent being able to report its status + /// chip — gating it behind a group would let a misconfigured agent go + /// dark on the dashboard. The server-side `SetStatus` handler has no + /// tool-group check either (only length validation), so listing it here + /// keeps the `--allowedTools` list honest with that reality. + pub const ALWAYS_ON_TOOLS: &'static [&'static str] = &["set_status"]; + /// The Claude built-in tool names enabled by this group. Only /// `WebTools` returns a non-empty slice; all other groups return `&[]` /// (they control MCP tools via `tools()` instead). @@ -932,7 +941,9 @@ impl ToolGroup { pub fn description(self) -> &'static str { match self { Self::Messaging => "send, recv, ask, answer — core agent communication", - Self::Meta => "set_status, get_agent_meta — identity and status", + Self::Meta => { + "get_agent_meta — identity introspection (set_status is always available)" + } Self::Inbox => { "get_loose_ends, cancel_loose_end, remind, request_next_turn — self-scheduling" }