fix: address argus+mara review on #1243 — stale docs, MANAGER_DEFAULT refs, ruth migration

CLAUDE.md

@@ -234,7 +234,7 @@ hive-ag3nt/        in-container harness crate; produces ONE `hive`
                          model selection (persisted at /harness/hyperhive-model)
   src/turn.rs           claude --print + stream-json pump; --compact retry;
                          proactive compaction + auto session-reset
-  src/mcp.rs            embedded MCP server (rmcp): unified HiveServer {socket, flavor} (AgentServer + ManagerServer are type aliases)
+  src/mcp.rs            embedded MCP server (rmcp): `AgentServer { socket }` — single role, tool groups gate access
   src/forge_notify.rs   Forgejo webhook subscriber: formats new-issue /
                          new-PR / comment / review notifications as
                          broker messages; embeds a body excerpt

docs/conventions.md

@@ -308,8 +308,8 @@ the new `HIVE_TOOL_GROUPS` env var. Agents with no entry get no var.
 **Runtime resolution** — at session start the harness reads `HIVE_TOOL_GROUPS`
 (a comma-separated list of snake_case group names injected by the meta renderer
 from `tool-groups.json`). Unrecognised tokens are logged and skipped. Falls back
-to `ToolGroup::AGENT_DEFAULT` (`messaging`, `meta`, `inbox`, `execution`) or
-`ToolGroup::MANAGER_DEFAULT` (all groups) when the var is absent or empty.
+to `ToolGroup::AGENT_DEFAULT` (`messaging`, `meta`, `inbox`, `execution`) when
+the var is absent or empty.
 
 **Updating the surface** — when a new `#[tool]` fn is added to `HiveServer`
 in `hive-ag3nt/src/mcp.rs`, add its name to the matching `ToolGroup::tools()`

hive-ag3nt/src/mcp.rs

@@ -43,8 +43,7 @@ pub enum SocketReply {
     Recent(Vec<hive_sh4re::InboxRow>),
     Logs(String),
     HostJournal(String),
-    /// `list_schedules` result — used by the manager surface only;
-    /// `AgentResponse` has no equivalent variant.
+    /// `list_schedules` result — returned by `list_schedules` (scheduling tool group).
     Schedules(Vec<hive_sh4re::WireSchedule>),
     /// `list_containers` result — descendant containers with running status.
     Containers(Vec<hive_sh4re::ContainerInfo>),
@@ -738,7 +737,8 @@ impl AgentServer {
         description = "List loose ends pending against this agent: unanswered questions \
         where you are the asker (waiting on someone) or the target (someone's waiting on \
         you), pending reminders you scheduled, plus — for the manager only — pending \
-        approvals you submitted that the operator hasn't acted on yet. Also lists active \
+        approvals you submitted that the operator hasn't acted on yet (agents with the \
+        `approvals` tool group also see their own pending approvals). Also lists active \
         local tasks published by external MCP daemons (e.g. running bash tasks). Cheap sweep, no args. Useful \
         at turn start to remember what you owe / what's owed to you without scrolling \
         inbox history. Output is a short bulleted list with ids, ages in seconds, and \
@@ -1448,7 +1448,7 @@ pub async fn serve_agent_stdio(socket: PathBuf) -> Result<()> {
 }
 
 // -----------------------------------------------------------------------------
-// Manager tool surface
+// Privileged tool arg types (lifecycle, approvals, scheduling, diagnostics)
 // -----------------------------------------------------------------------------
 
 #[derive(Debug, serde::Deserialize, schemars::JsonSchema)]
@@ -1559,12 +1559,11 @@ pub struct CancelLooseEndArgs {
 
 #[derive(Debug, serde::Deserialize, schemars::JsonSchema)]
 pub struct GetLooseEndsArgs {
-    /// Whose loose ends to list. Omit (or `null`) for your own — the
-    /// manager's: approvals you submitted + questions where you are
-    /// asker/target + your own pending reminders. Pass `"*"` for a
-    /// hive-wide view of EVERY pending approval, unanswered question,
-    /// and reminder across the swarm. Pass a specific agent name to
-    /// inspect just that agent's threads.
+    /// Whose loose ends to list. Omit (or `null`) for your own: approvals
+    /// you submitted + questions where you are asker/target + your own
+    /// pending reminders. Pass `"*"` for a hive-wide view of EVERY pending
+    /// approval, unanswered question, and reminder across the swarm. Pass a
+    /// specific agent name to inspect just that agent's threads.
     #[serde(default)]
     pub agent: Option<String>,
 }
@@ -1576,7 +1575,7 @@ pub struct AgentGetLooseEndsArgs {
     /// Pass any other agent name to inspect their threads — requires the
     /// `query_agent_state` capability; without it the request is rejected
     /// with an error. The `"*"` hive-wide value is not available on the
-    /// agent socket; use the manager socket for swarm-wide scans.
+    /// agent socket.
     #[serde(default)]
     pub agent: Option<String>,
 }
@@ -1744,13 +1743,6 @@ pub const SERVER_NAME: &str = "hyperhive";
 /// should plan in /state notes instead.
 pub const ALLOWED_BUILTIN_TOOLS: &[&str] = &["Edit", "Glob", "Grep", "Read", "Write"];
 
-/// Kept for API stability — was two-variant (Agent/Manager) but the manager
-/// role no longer exists. Only `Agent` remains; everything is perms + caps.
-#[derive(Debug, Clone, Copy)]
-pub enum Flavor {
-    Agent,
-}
-
 /// Env var written by the meta renderer with a comma-separated list of
 /// `hive_sh4re::ToolGroup` snake_case names (e.g. `"messaging,inbox,meta"`).
 /// When present, the harness expands the groups into per-tool allow entries

hive-c0re/src/meta.rs

@@ -755,7 +755,7 @@ where
             .map_or_else(|| "null".to_owned(), |p| format!("\"{p}\""));
         // Emit `toolGroups = "group1,group2"` when the operator has
         // explicitly configured groups for this agent. Absent entry = null
-        // = harness falls back to its role default (no env var emitted,
+        // = harness falls back to AGENT_DEFAULT (no env var emitted,
         // no rebuild cascade for agents whose groups haven't changed).
         let groups = tool_groups_map.get(&spec.name).cloned().unwrap_or_default();
         let tool_groups_attr = if groups.is_empty() {

hive-c0re/src/migrate.rs

@@ -1,5 +1,6 @@
-//! Startup auto-migration. Five idempotent phases: applied repo,
-//! proposed repo, meta repo, container repoint, root→h-root rename.
+//! Startup auto-migration. Six idempotent phases: applied repo,
+//! proposed repo, meta repo, container repoint, root→h-root rename,
+//! and manager tool-groups backfill.
 //! Kill-switch: `HIVE_SKIP_META_MIGRATION=1`. Full migration sequence
 //! and phase details: `docs/approvals.md::Migration from the pre-tag`.
 
@@ -12,6 +13,7 @@ use tokio::process::Command;
 use crate::coordinator::Coordinator;
 use crate::lifecycle::{self, AGENT_PREFIX, MANAGER_CONTAINER, MANAGER_NAME};
 use crate::meta;
+use crate::tool_groups;
 
 const KILL_SWITCH: &str = "HIVE_SKIP_META_MIGRATION";
 
@@ -112,6 +114,11 @@ pub async fn run(coord: &Arc<Coordinator>) -> Result<()> {
     // fresh installs (conf file absent) and after first successful run.
     rename_manager_container(coord).await;
 
+    // Phase 6: ensure ruth has explicit tool groups so removing the
+    // role-based fallback (Role::Manager → MANAGER_DEFAULT) doesn't
+    // silently strip her privileged tools on next rebuild.
+    backfill_manager_tool_groups(&names);
+
     Ok(())
 }
 
@@ -322,6 +329,37 @@ async fn repoint_container(name: &str) -> Result<()> {
     Ok(())
 }
 
+/// Phase 6: if ruth is a deployed agent and has no explicit entry in
+/// `tool-groups.json`, set her groups to `MANAGER_DEFAULT` (all groups).
+/// Idempotent — skips when entry already present. Prevents a silent tool
+/// downgrade when upgrading from a build that relied on the manager-flavor
+/// fallback in `effective_tool_groups()`.
+fn backfill_manager_tool_groups(names: &[String]) {
+    if !names.iter().any(|n| n == MANAGER_NAME) {
+        return; // ruth not deployed — nothing to backfill
+    }
+    let existing = tool_groups::groups_for(MANAGER_NAME);
+    if !existing.is_empty() {
+        tracing::debug!(
+            "migration: ruth already has explicit tool groups — skipping backfill"
+        );
+        return;
+    }
+    let all_groups: Vec<String> = hive_sh4re::ToolGroup::MANAGER_DEFAULT
+        .iter()
+        .map(|g| g.as_str().to_owned())
+        .collect();
+    match tool_groups::set_groups(MANAGER_NAME, &all_groups) {
+        Ok(()) => tracing::info!(
+            "migration: backfilled ruth's tool groups to MANAGER_DEFAULT (all groups)"
+        ),
+        Err(e) => tracing::warn!(
+            error = ?e,
+            "migration: failed to backfill ruth's tool groups — she may lose privileged tools on next rebuild"
+        ),
+    }
+}
+
 async fn raw_git(dir: &Path, args: &[&str]) -> Result<()> {
     let out = lifecycle::git_command()
         .current_dir(dir)

hive-c0re/src/tool_groups.rs

@@ -12,16 +12,14 @@
 //! }
 //! ```
 //!
-//! An absent entry (or an absent file) means "use the harness role
-//! default" — agents get `messaging + meta + inbox`, the manager gets
-//! all groups. `render_flake` in `meta.rs` reads this file and
-//! injects `HIVE_TOOL_GROUPS` into each agent's systemd service env;
-//! agents with no entry get no env var and the harness falls back.
+//! An absent entry (or an absent file) means "use the harness default"
+//! (`AGENT_DEFAULT`: `messaging + meta + inbox + execution`). `render_flake`
+//! in `meta.rs` reads this file and injects `HIVE_TOOL_GROUPS` into each
+//! agent's systemd service env; agents with no entry get no env var and the
+//! harness falls back to `AGENT_DEFAULT`.
 //!
 //! Write path: `set_groups` is called from the dashboard action handler
 //! that the operator uses to grant/revoke tool groups per agent.
-//! The manager may also request group changes via an approval; hive-c0re
-//! applies the change on approval, commits the file, and cascades a rebuild.
 
 use std::collections::BTreeMap;
 use std::path::PathBuf;

hive-sh4re/src/lib.rs

@@ -802,8 +802,8 @@ pub struct SchedulePromptPayload {
 /// `HIVE_TOOL_GROUPS` from the environment (a comma-separated list of
 /// `snake_case` group names written by the meta renderer from per-agent
 /// config) and expands it to the matching tool names for `--allowedTools`.
-/// When the env var is absent the harness falls back to the flavor default
-/// (`AGENT_DEFAULT` or `MANAGER_DEFAULT`). See `docs/conventions.md::Tool groups`.
+/// When the env var is absent the harness falls back to `AGENT_DEFAULT`.
+/// See `docs/conventions.md::Tool groups`.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum ToolGroup {
@@ -878,13 +878,12 @@ impl ToolGroup {
         }
     }
 
-    /// Default tool groups for a plain agent harness — equivalent to the
-    /// old `Flavor::Agent` allow-list. Used when `HIVE_TOOL_GROUPS` is unset.
+    /// Default tool groups for an agent harness. Used when `HIVE_TOOL_GROUPS` is unset.
     pub const AGENT_DEFAULT: &'static [Self] =
         &[Self::Messaging, Self::Meta, Self::Inbox, Self::Execution];
 
-    /// Default tool groups for the manager harness — equivalent to the
-    /// old `Flavor::Manager` allow-list. Used when `HIVE_TOOL_GROUPS` is unset.
+    /// Convenience preset for a fully-privileged agent (all groups).
+    /// Use this as a starting point in `tool-groups.json` for root/manager agents.
     pub const MANAGER_DEFAULT: &'static [Self] = &[
         Self::Messaging,
         Self::Meta,