diff --git a/docs/conventions.md b/docs/conventions.md index 909a7646..894b6f48 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -332,13 +332,16 @@ 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`. +**Always-on tools** — `set_status`, `compact`, and `mark_todos_done` are +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`. +`mark_todos_done` is here because todos are pushed to an agent independent of +whether it holds `inbox` — an agent without that group still needs a way to +clear them. **Config storage** — per-agent tool groups live in `/var/lib/hyperhive/meta/tool-groups.json` (hive-c0re-owned, committed to the diff --git a/hive-agent/src/mcp_config.rs b/hive-agent/src/mcp_config.rs index 2f4820c7..a36fce3a 100644 --- a/hive-agent/src/mcp_config.rs +++ b/hive-agent/src/mcp_config.rs @@ -423,6 +423,22 @@ mod tests { ); } + #[test] + fn mark_todos_done_present_with_no_groups() { + // Regression: mark_todos_done was declared as a + // tool but never added to any ToolGroup, so no agent could ever + // get it into --allowedTools regardless of which groups it held — + // including `inbox`, which only gates get_loose_ends/ + // cancel_loose_end/remind. + let tools = allowed_mcp_tools(&[]); + assert!( + tools.contains(&qualified("mark_todos_done")), + "mark_todos_done missing from empty-group allow-list: {tools:?}" + ); + let tools = allowed_mcp_tools(&[ToolGroup::Inbox]); + assert!(tools.contains(&qualified("mark_todos_done"))); + } + #[test] fn set_status_present_without_meta_group() { let tools = allowed_mcp_tools(&[ToolGroup::Messaging, ToolGroup::Inbox]); diff --git a/hive-sh4re/src/lib.rs b/hive-sh4re/src/lib.rs index 6509778f..a9c9cef0 100644 --- a/hive-sh4re/src/lib.rs +++ b/hive-sh4re/src/lib.rs @@ -639,7 +639,19 @@ impl ToolGroup { /// than on tool groups, and every agent should be able to reach for it /// regardless of which optional groups it's been granted — same /// reasoning as `set_status`. - pub const ALWAYS_ON_TOOLS: &'static [&'static str] = &["set_status", "compact"]; + /// + /// `mark_todos_done` too (a critical bug every agent hit): it was + /// declared as a `#[tool]` fn but never added to *any* + /// group's [`tools`](Self::tools), including `Inbox`, so no agent could + /// ever get it into `--allowedTools` and every call prompted for + /// approval it can't get. Todos are pushed to an agent independent of + /// whether it holds `Inbox` (that group only gates + /// `get_loose_ends`/`cancel_loose_end`/`remind`), so an agent without + /// `Inbox` could accumulate todos it can never clear — same + /// "every agent needs this regardless of optional groups" shape as + /// `set_status`/`compact`, not a narrower `Inbox`-only fix. + pub const ALWAYS_ON_TOOLS: &'static [&'static str] = + &["set_status", "compact", "mark_todos_done"]; /// The Claude built-in tool names enabled by this group. Only /// `WebTools` returns a non-empty slice; all other groups return `&[]`