From 1770b5184598722969a0a610ab93d983dd893637 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 17 May 2026 11:43:14 +0200 Subject: [PATCH] manager mcp: expose 'remind' tool sharing storage helper with agent surface --- TODO.md | 2 +- hive-ag3nt/src/mcp.rs | 40 +++++++++++++++++++++++++ hive-c0re/src/agent_server.rs | 46 +++++++++++++++-------------- hive-c0re/src/manager_server.rs | 14 +++++++++ hive-c0re/src/reminder_scheduler.rs | 31 ++++++++++++++++++- hive-sh4re/src/lib.rs | 11 +++++++ 6 files changed, 120 insertions(+), 24 deletions(-) diff --git a/TODO.md b/TODO.md index a2acde8..257282a 100644 --- a/TODO.md +++ b/TODO.md @@ -16,7 +16,7 @@ - ~~Handle text overflow → suggest file_path option for long messages~~ ✓ fixed — Remind dispatch rejects `message.len() > 4096` (when no `file_path` was supplied) with an error pointing at the `file_path` escape hatch. - Per-agent reminder limits (burst capacity, rate limiting) - ~~**Expose `remind` MCP tool**~~ ✓ fixed — `mcp__hyperhive__remind` now on `AgentServer`; takes `message`, exactly one of `delay_seconds` / `at_unix_timestamp`, optional `file_path`. Manager surface still missing (no `ManagerRequest::Remind` variant) — separate item below. -- **Manager-side `remind`**: mirror of the agent tool but on `ManagerServer`. Needs `ManagerRequest::Remind` variant in hive-sh4re, dispatch in manager_server.rs, MCP tool wiring. +- ~~**Manager-side `remind`**~~ ✓ fixed — `ManagerRequest::Remind` variant added, dispatch reuses `agent_server::store_remind` helper (shared across both surfaces), `mcp__hyperhive__remind` now on `ManagerServer` (auto-file lands at `/state/reminders/auto-.md` — manager's legacy state mount). - ~~**File path delivery**~~ ✓ fixed — scheduler now writes the reminder body to the requested `file_path` (mapped from container `/agents//state/...` to host `/var/lib/hyperhive/agents//state/...`) and delivers a short pointer message in its place. Path-traversal + foreign-agent-state writes are rejected; on rejection or write failure the body falls back to inline delivery with a noted warning. New module `hive-c0re/src/reminder_scheduler.rs` (extracted from main.rs). - ~~**Orphan reminders**~~ ✓ fixed — `Broker::deliver_reminder` wraps the inbox INSERT + reminders UPDATE in one sqlite transaction; partial failure can no longer cause duplicate delivery on the next tick. - ~~**Unbounded batches**~~ ✓ fixed — scheduler now calls `get_due_reminders(REMINDER_BATCH_LIMIT)` (cap = 100/tick); overflow stays due and gets picked up next cycle. diff --git a/hive-ag3nt/src/mcp.rs b/hive-ag3nt/src/mcp.rs index 3a36ac0..ce98231 100644 --- a/hive-ag3nt/src/mcp.rs +++ b/hive-ag3nt/src/mcp.rs @@ -665,6 +665,45 @@ impl ManagerServer { .await } + #[tool( + description = "Schedule a reminder that lands in the manager's own inbox at a future \ + time (sender will appear as `reminder`). Use for self-paced manager follow-ups: \ + 'recheck pending approval in 10m', 'nudge alice if she hasn't replied by 14:00 \ + UTC'. Set EXACTLY ONE of `delay_seconds` (fire N seconds from now) or \ + `at_unix_timestamp` (fire at absolute epoch second). Body soft-caps at 1 KiB \ + inline — anything larger gets auto-persisted to a file under `/state/reminders/` \ + (the manager's own state mount) and the inbox message becomes a short pointer. \ + Pass `file_path` if you want to control the destination yourself." + )] + async fn remind(&self, Parameters(args): Parameters) -> String { + let log = format!("{args:?}"); + run_tool_envelope("remind", log, async move { + let timing = match (args.delay_seconds, args.at_unix_timestamp) { + (Some(_), Some(_)) => { + return "remind failed: pass exactly one of `delay_seconds` or \ + `at_unix_timestamp`, not both" + .to_string(); + } + (None, None) => { + return "remind failed: pass exactly one of `delay_seconds` or \ + `at_unix_timestamp`" + .to_string(); + } + (Some(s), None) => hive_sh4re::ReminderTiming::InSeconds { seconds: s }, + (None, Some(t)) => hive_sh4re::ReminderTiming::At { unix_timestamp: t }, + }; + let (resp, retries) = self + .dispatch(hive_sh4re::ManagerRequest::Remind { + message: args.message, + timing, + file_path: args.file_path, + }) + .await; + annotate_retries(format_ack(resp, "remind", "reminder scheduled".to_string()), retries) + }) + .await + } + #[tool( description = "Fetch recent journal log lines for a sub-agent container. Useful \ for diagnosing MCP server registration failures, startup crashes, plugin install \ @@ -753,6 +792,7 @@ pub fn allowed_mcp_tools(flavor: Flavor) -> Vec { "request_apply_commit", "ask_operator", "get_logs", + "remind", ], }; let mut out: Vec = names diff --git a/hive-c0re/src/agent_server.rs b/hive-c0re/src/agent_server.rs index 35b6193..39c3ed8 100644 --- a/hive-c0re/src/agent_server.rs +++ b/hive-c0re/src/agent_server.rs @@ -227,30 +227,32 @@ fn handle_remind( timing: &hive_sh4re::ReminderTiming, file_path: Option<&str>, ) -> AgentResponse { - let due_at = match resolve_due_at(timing) { - Ok(t) => t, - Err(e) => { - return AgentResponse::Err { - message: format!("invalid reminder timing: {e:#}"), - }; - } - }; - let (stored_message, stored_path) = match prepare_remind_storage(agent, message, file_path) { - Ok(pair) => pair, - Err(e) => return AgentResponse::Err { message: e }, - }; - match coord + match store_remind(coord, agent, message, timing, file_path) { + Ok(()) => AgentResponse::Ok, + Err(message) => AgentResponse::Err { message }, + } +} + +/// Shared remind-storage path used by both the agent and the manager +/// dispatchers. Validates timing, applies the auto-file overflow +/// dance (see [`prepare_remind_storage`]), and writes the reminder +/// row. Returns `Ok(())` on success, or a caller-ready error string +/// the dispatcher wraps in `*Response::Err`. +pub(crate) fn store_remind( + coord: &Arc, + agent: &str, + message: &str, + timing: &hive_sh4re::ReminderTiming, + file_path: Option<&str>, +) -> Result<(), String> { + let due_at = resolve_due_at(timing).map_err(|e| format!("invalid reminder timing: {e:#}"))?; + let (stored_message, stored_path) = prepare_remind_storage(agent, message, file_path)?; + let id = coord .broker .store_reminder(agent, &stored_message, stored_path.as_deref(), due_at) - { - Ok(id) => { - tracing::info!(%id, %agent, %due_at, "reminder scheduled"); - AgentResponse::Ok - } - Err(e) => AgentResponse::Err { - message: format!("failed to store reminder: {e:#}"), - }, - } + .map_err(|e| format!("failed to store reminder: {e:#}"))?; + tracing::info!(%id, %agent, %due_at, "reminder scheduled"); + Ok(()) } /// Decide what we actually store in the reminders row, applying the diff --git a/hive-c0re/src/manager_server.rs b/hive-c0re/src/manager_server.rs index 0c5145a..e7b9307 100644 --- a/hive-c0re/src/manager_server.rs +++ b/hive-c0re/src/manager_server.rs @@ -307,6 +307,20 @@ async fn dispatch(req: &ManagerRequest, coord: &Arc) -> ManagerResp }, } } + ManagerRequest::Remind { + message, + timing, + file_path, + } => match crate::agent_server::store_remind( + coord, + MANAGER_AGENT, + message, + timing, + file_path.as_deref(), + ) { + Ok(()) => ManagerResponse::Ok, + Err(message) => ManagerResponse::Err { message }, + }, ManagerRequest::RequestApplyCommit { agent, commit_ref, diff --git a/hive-c0re/src/reminder_scheduler.rs b/hive-c0re/src/reminder_scheduler.rs index 99699d2..c7d83f0 100644 --- a/hive-c0re/src/reminder_scheduler.rs +++ b/hive-c0re/src/reminder_scheduler.rs @@ -165,6 +165,20 @@ pub fn write_payload(agent: &str, host_path: &Path, message: &str) -> Result<(), Ok(()) } +/// Container-visible state prefix the caller's `file_path` must live +/// under. Sub-agents see their state at `/agents//state/`; +/// the manager keeps the legacy `/state/` mount (see +/// `lifecycle::set_nspawn_flags`). Auto-file paths use the same +/// prefix so the round-trip is symmetric. +#[must_use] +pub fn container_state_prefix(agent: &str) -> String { + if agent == hive_sh4re::MANAGER_AGENT { + "/state/".to_owned() + } else { + format!("/agents/{agent}/state/") + } +} + /// Map an agent-visible container path to the matching host path, /// validating that it lives under the agent's own state subtree, has /// a non-empty relative tail, and doesn't try to traverse out via @@ -172,7 +186,7 @@ pub fn write_payload(agent: &str, host_path: &Path, message: &str) -> Result<(), /// reason string on rejection. `pub` so `agent_server::handle_remind` /// can reuse it for the at-remind-time auto-file path. pub fn resolve_host_path(agent: &str, req_path: &str) -> Result { - let prefix = format!("/agents/{agent}/state/"); + let prefix = container_state_prefix(agent); let Some(rel) = req_path.strip_prefix(&prefix) else { return Err(format!( "must be absolute and under `{prefix}` (got `{req_path}`)" @@ -230,6 +244,21 @@ mod tests { ); } + #[test] + fn manager_uses_legacy_state_prefix() { + // The manager container mounts its state at `/state/` (legacy), + // not `/agents/manager/state/`. Same host path; different + // container-visible path. resolve_host_path needs to know. + assert_eq!(container_state_prefix("manager"), "/state/"); + let p = resolve_host_path("manager", "/state/reminders/x.md").unwrap(); + assert_eq!( + p, + PathBuf::from("/var/lib/hyperhive/agents/manager/state/reminders/x.md") + ); + // And the sub-agent prefix must NOT be accepted for the manager. + assert!(resolve_host_path("manager", "/agents/manager/state/x.md").is_err()); + } + #[test] fn prepare_body_passthrough_when_no_file_path() { let s = prepare_body("foo", "hello world", None); diff --git a/hive-sh4re/src/lib.rs b/hive-sh4re/src/lib.rs index 314aba6..6de7228 100644 --- a/hive-sh4re/src/lib.rs +++ b/hive-sh4re/src/lib.rs @@ -486,6 +486,17 @@ pub enum ManagerRequest { #[serde(default)] lines: Option, }, + /// Mirror of `AgentRequest::Remind` on the manager surface — schedule + /// a reminder addressed to the manager itself. Same semantics: body + /// soft-caps at 1 KiB, oversize bodies auto-persist to + /// `/state/reminders/auto-.md` (the manager container's own state + /// mount) and the inbox sees a pointer. + Remind { + message: String, + timing: ReminderTiming, + #[serde(default)] + file_path: Option, + }, } #[derive(Debug, Clone, Serialize, Deserialize)]