manager mcp: expose 'remind' tool sharing storage helper with agent surface
This commit is contained in:
parent
0e6bac8388
commit
1770b51845
6 changed files with 120 additions and 24 deletions
2
TODO.md
2
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.
|
- ~~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)
|
- 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.
|
- ~~**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-<ts>.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/<agent>/state/...` to host `/var/lib/hyperhive/agents/<agent>/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).
|
- ~~**File path delivery**~~ ✓ fixed — scheduler now writes the reminder body to the requested `file_path` (mapped from container `/agents/<agent>/state/...` to host `/var/lib/hyperhive/agents/<agent>/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.
|
- ~~**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.
|
- ~~**Unbounded batches**~~ ✓ fixed — scheduler now calls `get_due_reminders(REMINDER_BATCH_LIMIT)` (cap = 100/tick); overflow stays due and gets picked up next cycle.
|
||||||
|
|
|
||||||
|
|
@ -665,6 +665,45 @@ impl ManagerServer {
|
||||||
.await
|
.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<RemindArgs>) -> 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(
|
#[tool(
|
||||||
description = "Fetch recent journal log lines for a sub-agent container. Useful \
|
description = "Fetch recent journal log lines for a sub-agent container. Useful \
|
||||||
for diagnosing MCP server registration failures, startup crashes, plugin install \
|
for diagnosing MCP server registration failures, startup crashes, plugin install \
|
||||||
|
|
@ -753,6 +792,7 @@ pub fn allowed_mcp_tools(flavor: Flavor) -> Vec<String> {
|
||||||
"request_apply_commit",
|
"request_apply_commit",
|
||||||
"ask_operator",
|
"ask_operator",
|
||||||
"get_logs",
|
"get_logs",
|
||||||
|
"remind",
|
||||||
],
|
],
|
||||||
};
|
};
|
||||||
let mut out: Vec<String> = names
|
let mut out: Vec<String> = names
|
||||||
|
|
|
||||||
|
|
@ -227,30 +227,32 @@ fn handle_remind(
|
||||||
timing: &hive_sh4re::ReminderTiming,
|
timing: &hive_sh4re::ReminderTiming,
|
||||||
file_path: Option<&str>,
|
file_path: Option<&str>,
|
||||||
) -> AgentResponse {
|
) -> AgentResponse {
|
||||||
let due_at = match resolve_due_at(timing) {
|
match store_remind(coord, agent, message, timing, file_path) {
|
||||||
Ok(t) => t,
|
Ok(()) => AgentResponse::Ok,
|
||||||
Err(e) => {
|
Err(message) => AgentResponse::Err { message },
|
||||||
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,
|
/// Shared remind-storage path used by both the agent and the manager
|
||||||
Err(e) => return AgentResponse::Err { message: e },
|
/// dispatchers. Validates timing, applies the auto-file overflow
|
||||||
};
|
/// dance (see [`prepare_remind_storage`]), and writes the reminder
|
||||||
match coord
|
/// row. Returns `Ok(())` on success, or a caller-ready error string
|
||||||
|
/// the dispatcher wraps in `*Response::Err`.
|
||||||
|
pub(crate) fn store_remind(
|
||||||
|
coord: &Arc<Coordinator>,
|
||||||
|
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
|
.broker
|
||||||
.store_reminder(agent, &stored_message, stored_path.as_deref(), due_at)
|
.store_reminder(agent, &stored_message, stored_path.as_deref(), due_at)
|
||||||
{
|
.map_err(|e| format!("failed to store reminder: {e:#}"))?;
|
||||||
Ok(id) => {
|
|
||||||
tracing::info!(%id, %agent, %due_at, "reminder scheduled");
|
tracing::info!(%id, %agent, %due_at, "reminder scheduled");
|
||||||
AgentResponse::Ok
|
Ok(())
|
||||||
}
|
|
||||||
Err(e) => AgentResponse::Err {
|
|
||||||
message: format!("failed to store reminder: {e:#}"),
|
|
||||||
},
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Decide what we actually store in the reminders row, applying the
|
/// Decide what we actually store in the reminders row, applying the
|
||||||
|
|
|
||||||
|
|
@ -307,6 +307,20 @@ async fn dispatch(req: &ManagerRequest, coord: &Arc<Coordinator>) -> 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 {
|
ManagerRequest::RequestApplyCommit {
|
||||||
agent,
|
agent,
|
||||||
commit_ref,
|
commit_ref,
|
||||||
|
|
|
||||||
|
|
@ -165,6 +165,20 @@ pub fn write_payload(agent: &str, host_path: &Path, message: &str) -> Result<(),
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Container-visible state prefix the caller's `file_path` must live
|
||||||
|
/// under. Sub-agents see their state at `/agents/<name>/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,
|
/// Map an agent-visible container path to the matching host path,
|
||||||
/// validating that it lives under the agent's own state subtree, has
|
/// 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
|
/// 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`
|
/// reason string on rejection. `pub` so `agent_server::handle_remind`
|
||||||
/// can reuse it for the at-remind-time auto-file path.
|
/// can reuse it for the at-remind-time auto-file path.
|
||||||
pub fn resolve_host_path(agent: &str, req_path: &str) -> Result<PathBuf, String> {
|
pub fn resolve_host_path(agent: &str, req_path: &str) -> Result<PathBuf, String> {
|
||||||
let prefix = format!("/agents/{agent}/state/");
|
let prefix = container_state_prefix(agent);
|
||||||
let Some(rel) = req_path.strip_prefix(&prefix) else {
|
let Some(rel) = req_path.strip_prefix(&prefix) else {
|
||||||
return Err(format!(
|
return Err(format!(
|
||||||
"must be absolute and under `{prefix}` (got `{req_path}`)"
|
"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]
|
#[test]
|
||||||
fn prepare_body_passthrough_when_no_file_path() {
|
fn prepare_body_passthrough_when_no_file_path() {
|
||||||
let s = prepare_body("foo", "hello world", None);
|
let s = prepare_body("foo", "hello world", None);
|
||||||
|
|
|
||||||
|
|
@ -486,6 +486,17 @@ pub enum ManagerRequest {
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
lines: Option<u32>,
|
lines: Option<u32>,
|
||||||
},
|
},
|
||||||
|
/// 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-<ts>.md` (the manager container's own state
|
||||||
|
/// mount) and the inbox sees a pointer.
|
||||||
|
Remind {
|
||||||
|
message: String,
|
||||||
|
timing: ReminderTiming,
|
||||||
|
#[serde(default)]
|
||||||
|
file_path: Option<String>,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue