From 4121e11d878756aba23d24c3f4299cd1c8418cce Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 17 Sep 2026 19:27:15 +0200 Subject: [PATCH] hive-c0re: drop the subtree check from the scheduling verbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The topology predicate `is_descendant_of` gated the four schedule- managing verbs: a caller could only name a schedule owned by an agent at or below itself in `topology.json`. Those gates now permit any requester, so the predicate, its pure `_in` form and the `schedule_authorized` wrapper built on it are gone rather than left returning a constant. The other two wrappers went earlier with the verbs they served — `require_descendant` with the lifecycle MCP verbs in 87970a8c, and `resolve_agent_state_target` with `get_loose_ends`'s agent parameter. `require_group(agent, "scheduling", ...)` is untouched and still fires at dispatch for every one of the five scheduling verbs, so holding the tool group remains the gate; what goes is the ownership restriction layered on top of it. The three schedule-mutating verbs keep their row lookup as a plain existence check, so a caller naming a schedule that does not exist still gets `not found` rather than a message from deeper in the cancel path. `list_schedules` stops filtering per row: it would only have hidden rows the requester may act on anyway. Error messages, tool descriptions and docs that described the subtree relation are reworded — a refusal message naming a topology that no longer decides anything is worse than none. The six `is_descendant_of_in` unit tests go with the function they test; the permit behaviour they leave unasserted is picked up by the next commit. Refs #4472 --- docs/tools/scheduling.md | 19 ++- hive-agent-mcp/README.md | 3 +- hive-agent-mcp/src/mcp/mod.rs | 15 +- hive-agent/prompts/system.md | 4 +- hive-c0re/src/agent_config/topology.rs | 118 +--------------- hive-c0re/src/socket_server/mod.rs | 15 +- hive-c0re/src/socket_server/schedules.rs | 166 +++++------------------ hive-core-agent-sock/src/lib.rs | 4 +- 8 files changed, 67 insertions(+), 277 deletions(-) diff --git a/docs/tools/scheduling.md b/docs/tools/scheduling.md index 30641cb5..15291fb9 100644 --- a/docs/tools/scheduling.md +++ b/docs/tools/scheduling.md @@ -10,10 +10,9 @@ targets only yourself — use `remind` for an unapproved self-wake. The other four verbs need no approval: holding the `scheduling` tool group is the whole gate. -Authorization is one rule for all four verbs: you reach schedules you own -and any owned by an agent in your topology subtree. `list_schedules` -applies it too, so the snapshot only ever shows schedules you could also -cancel. +Authorization is one rule for all four verbs: you reach every schedule in +the hive, whoever owns it. `list_schedules` shows the same set, so the +snapshot only ever shows schedules you could also cancel. ### `request_schedule_prompt(targets, body, first_fire_at_unix, interval_seconds?, description?)` @@ -61,13 +60,11 @@ consumes one-shot schedules and cancels them afterwards. ### `list_schedules()` -Snapshot the schedules you're authorized to see (active, and cancelled -but not yet reaped) — same read scope as the rest of this group: your -own, plus any owned by a sub-agent in your subtree (everything, for -the operator). Returns id, owner, body, per-target `last_fired_at` -and `last_result`, `next_fire_at_unix`, `interval_seconds`. Use to -look up an id before cancelling, or to audit upcoming wake-ups in -your subtree. +Snapshot every schedule in the hive (active, and cancelled but not yet +reaped) — same read scope as the rest of this group. Returns id, owner, +body, per-target `last_fired_at` and `last_result`, +`next_fire_at_unix`, `interval_seconds`. Use to look up an id before +cancelling, or to audit upcoming wake-ups across the hive. ## `read_host_journal` capability diff --git a/hive-agent-mcp/README.md b/hive-agent-mcp/README.md index e154e58d..3f6f086a 100644 --- a/hive-agent-mcp/README.md +++ b/hive-agent-mcp/README.md @@ -10,8 +10,7 @@ would hit. HTTP is the sole transport; there is no stdio mode here. This is where the core hyperhive tool surface lives: `send`, `recv`, `remind`, `get_loose_ends`, `set_status`, -`get_agent_meta`, lifecycle (`kill`/`start`/`restart`/`update` on the -agent's own subtree), scheduling, and the approval-request tools. Reach +`get_agent_meta`, scheduling, and the approval-request tools. Reach for this crate when you're adding or changing a built-in tool rather than an `extraMcpServers` add-on — those are separate stdio bridges (see `hive-bash-mcp`, `hive-matrix-mcp`) that dial the harness socket diff --git a/hive-agent-mcp/src/mcp/mod.rs b/hive-agent-mcp/src/mcp/mod.rs index bdc99a4c..455df407 100644 --- a/hive-agent-mcp/src/mcp/mod.rs +++ b/hive-agent-mcp/src/mcp/mod.rs @@ -639,8 +639,7 @@ impl AgentServer { keep their next_fire_at unchanged (the manual fire is additive). One-shot \ schedules are CONSUMED by the manual fire (cancelled afterwards): the operator's \ intent on a one-shot is 'send this now, the scheduled time was wrong'. \n\n\ - Authorization mirrors `cancel_schedule`: you can fire your own schedules + any \ - owned by a sub-agent in your subtree per topology.json." + Authorization mirrors `cancel_schedule`: you can fire any schedule, whoever owns it." )] async fn fire_schedule_now(&self, Parameters(args): Parameters) -> String { let log = format!("{args:?}"); @@ -662,8 +661,7 @@ impl AgentServer { whole schedule (all recipients flipped). With a non-empty `targets` list, cancels \ just those recipients; the schedule keeps firing for any remaining active targets \ and auto-cancels its parent row when every target is cancelled. \n\n\ - Authorization: the manager can cancel its own schedules + any schedule whose \ - owner is one of its sub-agents per topology.json. Other owners are refused." + Authorization: you can cancel any schedule, whoever owns it." )] async fn cancel_schedule(&self, Parameters(args): Parameters) -> String { let log = format!("{args:?}"); @@ -693,8 +691,8 @@ impl AgentServer { swap a target atomically. Re-adding a previously-removed target starts a fresh \ per-target history (drops the tombstone). Draining all targets auto-cancels the \ parent schedule. \n\n\ - Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit your \ - own schedules + any owned by a sub-agent in your subtree per topology.json. \ + Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit any \ + schedule, whoever owns it. \ Refuses cancelled schedules (the row's terminal — submit a fresh one)." )] async fn edit_schedule(&self, Parameters(args): Parameters) -> String { @@ -721,12 +719,11 @@ impl AgentServer { } #[tool( - description = "List the scheduled prompts you're authorized to see — your own, plus \ - any owned by a sub-agent in your subtree (everything, for the operator) — active and \ + description = "List every scheduled prompt in the hive, whoever owns it — active and \ cancelled-but-not-yet-reaped. Returns schedule id, owner, body, target set with \ per-target last_fired_at + last_result, next fire time, recurring interval. \ Use this to look up an id before calling `cancel_schedule`, or to audit what \ - your subtree is going to be woken up about next." + the hive is going to be woken up about next." )] async fn list_schedules(&self) -> String { run_tool_envelope("list_schedules", String::new(), async move { diff --git a/hive-agent/prompts/system.md b/hive-agent/prompts/system.md index 53d7d18b..13221bcc 100644 --- a/hive-agent/prompts/system.md +++ b/hive-agent/prompts/system.md @@ -4,14 +4,14 @@ Tools (hyperhive surface). Full signature + behavior for each comes from the too - **Inbox / messaging** (always available): `mcp__hyperhive__recv`, `mcp__hyperhive__ack_until`, `mcp__hyperhive__send`, `mcp__hyperhive__get_loose_ends`, `mcp__hyperhive__cancel_loose_end`, `mcp__hyperhive__mark_todos_done`, `mcp__hyperhive__remind`, `mcp__hyperhive__set_status`, `mcp__hyperhive__get_agent_meta`. One habit worth internalizing beyond the tool descriptions themselves: prefer ending the turn over repeatedly polling `recv` when idle (only turn-boundaries observe in-container todo wakes — bash-task completions, matrix unread, forge activity — and ending the turn is also your checkpoint). For a large todo backlog (`get_loose_ends` caps at 40 rows), clear reviewed ids in bulk with `mark_todos_done` rather than cancelling one at a time — there's no blind range-clear, only ids you've actually looked at. - **Extra MCP tools** (some agents only): `mcp____` — agent-specific (matrix client, scraper, db connector, etc.) declared in your `agent.nix` under `hyperhive.extraMcpServers`. First-class tools, already operator-approved at deploy time. -- **Scheduling** (_requires `scheduling` tool group_): `request_schedule_prompt` (queues an approval), `cancel_schedule`, `fire_schedule_now`, `edit_schedule`, `list_schedules` (these four don't need approval — you can manage schedules you own or that a sub-agent in your subtree owns). +- **Scheduling** (_requires `scheduling` tool group_): `request_schedule_prompt` (queues an approval), `cancel_schedule`, `fire_schedule_now`, `edit_schedule`, `list_schedules` (these four don't need approval — you can manage any schedule, whoever owns it). - **Diagnostics**: `get_host_journal` (_requires `read_host_journal` capability_). Need new packages, env vars, or other NixOS config for yourself? You can't edit your own config directly — open a PR on your `agent-configs/` repo, or contact the operator directly. Config repos live at `/agents/{label}/config/` (read-only inside your container). All changes flow through operator-approved commits. Your config repo is mounted **read-only** at `/agents/{label}/config/` — `agent.nix` plus whatever extra files define you (declared packages, env vars, MCP servers). Read it to see exactly what defines you before asking for a change, so you can point at the precise file and line. -Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when an agent in your subtree needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which queues a `MergeConfigPr` approval on open/update. The operator only signs off on changes; you run the day-to-day. +Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when any agent needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which queues a `MergeConfigPr` approval on open/update. The operator only signs off on changes; you run the day-to-day. Messages from sender `system` are hyperhive helper events (JSON body, `event` field discriminates): `approval_resolved`, `container_crash`, `needs_update`. Use these to react to lifecycle changes: diff --git a/hive-c0re/src/agent_config/topology.rs b/hive-c0re/src/agent_config/topology.rs index cc74f4cc..8dbff20c 100644 --- a/hive-c0re/src/agent_config/topology.rs +++ b/hive-c0re/src/agent_config/topology.rs @@ -16,17 +16,17 @@ //! ## Graph representation //! //! The on-disk format stays as a flat JSON map `name → parent | null` -//! (small, git-diffable). In-memory, heavy algorithms (descendant checks, -//! cycle detection) use a [`petgraph`] directed graph where each edge runs +//! (small, git-diffable). In-memory, cycle detection uses a [`petgraph`] +//! directed graph where each edge runs //! **parent → child**. This replaces the ad-hoc bounded walks that existed -//! before: petgraph's `has_path_connecting` / `is_cyclic_directed` -//! are correct for graphs of any depth (no 32-hop ceiling) and well-tested. +//! before: petgraph's `is_cyclic_directed` +//! is correct for graphs of any depth (no 32-hop ceiling) and well-tested. //! The graph is built on demand from the flat map; it is not cached across //! calls (the map is small and disk I/O dominates anyway). use std::collections::BTreeMap; -use petgraph::algo::{has_path_connecting, is_cyclic_directed}; +use petgraph::algo::is_cyclic_directed; use petgraph::graph::{DiGraph, NodeIndex}; use std::path::PathBuf; @@ -141,27 +141,10 @@ pub fn resolve_recipient_in( } } -/// True when `candidate` is `ancestor` or any descendant of -/// `ancestor` per the current on-disk topology. -/// -/// Delegates to [`is_descendant_of_in`] on the result of [`read`] so -/// the algorithm is the same petgraph BFS used everywhere else. No -/// depth limit — the 32-hop bounded walk this replaced was correct for -/// any plausible hive but carried a latent ceiling; this has none. -/// -/// Used by the cancel-authorization checks in `socket_server` to enforce -/// "managers can cancel anything their subtree owns." -#[must_use] -pub fn is_descendant_of(candidate: &str, ancestor: &str) -> bool { - is_descendant_of_in(&read(), candidate, ancestor) -} - /// Build an in-memory petgraph directed graph from the topology map. /// /// Edges run **parent → child** so that: /// - `children_of(name)` = outgoing neighbours of `name`'s node -/// - `is_descendant_of(candidate, ancestor)` = path exists from `ancestor` -/// to `candidate` via `has_path_connecting` /// - cycle detection = `is_cyclic_directed` after a speculative edge insert /// /// Returns the graph and a `BTreeMap` for O(log n) @@ -191,28 +174,6 @@ fn build_graph( (graph, idx) } -/// Return true when `candidate` is a descendant of `ancestor` in the -/// given topology map. Uses petgraph BFS/DFS (`has_path_connecting`) -/// — no depth limit and no manually bounded walk. Pure; no disk I/O. -/// -/// Same semantics as the disk-reading [`is_descendant_of`]: a node is -/// considered a descendant of itself (`candidate == ancestor` → true). -#[must_use] -pub fn is_descendant_of_in( - topo: &BTreeMap>, - candidate: &str, - ancestor: &str, -) -> bool { - if candidate == ancestor { - return true; - } - let (graph, idx) = build_graph(topo); - let (Some(&anc_ni), Some(&cand_ni)) = (idx.get(ancestor), idx.get(candidate)) else { - return false; - }; - has_path_connecting(&graph, anc_ni, cand_ni, None) -} - /// Persist the topology map. Sorted JSON output (`BTreeMap` is sorted by /// key) keeps git diffs minimal across re-writes. Best-effort — /// returns an `io::Error` so callers can decide whether a failure @@ -742,75 +703,6 @@ mod tests { assert!(top_level_agents_in(&topo).is_empty()); } - // ----------------------------------------------------------------------- - // is_descendant_of_in tests (petgraph-backed; pure / no disk I/O) - // ----------------------------------------------------------------------- - - #[test] - fn is_descendant_of_in_self_is_true() { - let topo = topo_three_level(); - assert!(is_descendant_of_in(&topo, "alice", "alice")); - assert!(is_descendant_of_in( - &topo, - crate::lifecycle::MANAGER_NAME, - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_direct_child() { - let topo = topo_three_level(); - // alice is a direct child of manager. - assert!(is_descendant_of_in( - &topo, - "alice", - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_grandchild() { - let topo = topo_three_level(); - // bob is manager → alice → bob; should be reachable from manager. - assert!(is_descendant_of_in( - &topo, - "bob", - crate::lifecycle::MANAGER_NAME - )); - assert!(is_descendant_of_in( - &topo, - "carol", - crate::lifecycle::MANAGER_NAME - )); - } - - #[test] - fn is_descendant_of_in_parent_not_descendant_of_child() { - let topo = topo_three_level(); - // alice is NOT a descendant of bob (alice is bob's grandparent). - assert!(!is_descendant_of_in(&topo, "alice", "bob")); - assert!(!is_descendant_of_in( - &topo, - crate::lifecycle::MANAGER_NAME, - "alice" - )); - } - - #[test] - fn is_descendant_of_in_sibling_is_not_descendant() { - let topo = topo_three_level(); - // bob and carol are siblings under alice; neither descends from the other. - assert!(!is_descendant_of_in(&topo, "bob", "carol")); - assert!(!is_descendant_of_in(&topo, "carol", "bob")); - } - - #[test] - fn is_descendant_of_in_unknown_is_false() { - let topo = topo_three_level(); - assert!(!is_descendant_of_in(&topo, "nobody", "alice")); - assert!(!is_descendant_of_in(&topo, "alice", "nobody")); - } - // ----------------------------------------------------------------------- // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) // ----------------------------------------------------------------------- diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 2b808f8c..6f564921 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -4,10 +4,10 @@ //! you are `foo`; the manager socket simply serves as `ruth`. There is no //! privilege flag — both transports run the same [`serve`] / [`dispatch`] //! code, and authority derives uniformly from the caller's identity: -//! topology (`is_descendant_of`) for subtree-relational verbs, capabilities -//! for hive-wide queries, and tool-group membership for the orchestration -//! verbs. `ruth` reaches every agent only as a consequence of being the -//! topology root, not via any hardcoded name match. +//! capabilities for the hive-wide queries and tool-group membership for the +//! orchestration verbs. An agent-targeting verb may name any agent, so `ruth` +//! reaches every agent exactly the way every other agent does, not via any +//! hardcoded name match. use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -87,10 +87,9 @@ pub fn start(agent: &str, socket_path: &Path, coord: Arc) -> Result /// Bind + serve the manager socket. This is now **pure transport**: it grants /// no authority of its own — it just serves requests as `agent = MANAGER_AGENT` -/// ("ruth"), and ruth's reach comes entirely from being the topology root -/// (`is_descendant_of` covers every agent) plus the capabilities / tool-groups -/// it holds, identical to connecting on a per-agent socket — ruth uses the -/// standard per-agent runtime dir + socket, with no dedicated helpers. +/// ("ruth"), and ruth's reach comes entirely from the capabilities / +/// tool-groups it holds, identical to connecting on a per-agent socket — ruth +/// uses the standard per-agent runtime dir + socket, with no dedicated helpers. pub fn start_manager(coord: Arc) -> Result<()> { use std::os::unix::fs::PermissionsExt as _; let dir = crate::paths::agent_runtime_dir(crate::lifecycle::MANAGER_NAME); diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index c2b07254..15e52ff8 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -1,6 +1,6 @@ //! Scheduled-prompt request handlers (`ListSchedules` / //! `RequestSchedulePrompt` / `CancelSchedule` / `EditSchedule` / -//! `FireScheduleNow`), their shared ownership check, and the +//! `FireScheduleNow`) and the //! schedule-to-wire mapping reused by the dashboard //! (`schedule_to_wire_public` / `filter_ghost_schedule_targets`). @@ -10,20 +10,15 @@ use hive_core_agent_sock::Response; use crate::coordinator::Coordinator; -/// `ListSchedules` — snapshot the scheduled prompts `requester` is -/// authorized to see: its own schedules, ones owned by an agent in its -/// subtree, or (for the operator) everything. Uses the same -/// `schedule_authorized` rule `CancelSchedule`/`EditSchedule`/ -/// `FireScheduleNow` already enforce, applied here as a per-row filter -/// instead of a hard reject. +/// `ListSchedules` — snapshot every scheduled prompt. Ownership no longer +/// narrows the view: `CancelSchedule`/`EditSchedule`/`FireScheduleNow` accept +/// any requester, so filtering the listing would only hide rows the requester +/// can act on anyway. pub(super) fn handle_list_schedules(coord: &Arc, requester: &str) -> Response { + tracing::debug!(%requester, "list schedules"); match coord.scheduled_prompts.list() { Ok(schedules) => Response::Schedules { - schedules: schedules - .into_iter() - .filter(|s| schedule_authorized(requester, &s.owner)) - .map(schedule_to_wire) - .collect(), + schedules: schedules.into_iter().map(schedule_to_wire).collect(), }, Err(e) => Response::Err { message: format!("list scheduled prompts: {e:#}"), @@ -108,39 +103,20 @@ pub(super) fn handle_request_schedule_prompt( Response::Ok } -/// Cancel a schedule (whole or per-target). Manager-surface -/// authorization: a manager can cancel its own schedules + any -/// schedule whose owner is one of its sub-agents (topology-walked). -/// The operator surface bypasses this and can cancel anything; -/// agents reaching this path through the manager get the -/// topology-scoped check. +/// Cancel a schedule (whole or per-target). Any requester may cancel any +/// schedule, whoever owns it. pub(super) fn handle_cancel_schedule( coord: &Arc, requester: &str, schedule_id: i64, targets: Option<&[String]>, ) -> Response { - let schedule = match coord.scheduled_prompts.get(schedule_id) { - Ok(Some(s)) => s, - Ok(None) => { - return Response::Err { - message: format!("schedule {schedule_id} not found"), - }; - } - Err(e) => { - return Response::Err { - message: format!("read schedule {schedule_id}: {e:#}"), - }; - } - }; - if !schedule_authorized(requester, &schedule.owner) { - return Response::Err { - message: format!( - "not authorized: {requester} cannot cancel schedule owned by {owner}", - owner = schedule.owner - ), - }; + // Existence check only — it buys the caller a clean "not found" instead + // of whatever the cancel itself would say about an absent row. + if let Some(err) = require_schedule(coord, schedule_id) { + return err; } + tracing::info!(%requester, %schedule_id, "cancel schedule"); let result = match targets { Some(list) if !list.is_empty() => coord .scheduled_prompts @@ -160,37 +136,18 @@ pub(super) fn handle_cancel_schedule( } } -/// Authorize + dispatch a `FireScheduleNow` request from the -/// manager surface. Same ownership rules as `CancelSchedule`: -/// requester can fire its own schedules + any owned by an agent -/// in its subtree. The actual fan-out lives in +/// Dispatch a `FireScheduleNow` request. Same rule as `CancelSchedule`: any +/// requester, any schedule. The actual fan-out lives in /// `scheduled_prompts_worker::fire_now`. pub(super) async fn handle_fire_schedule_now( coord: &Arc, requester: &str, schedule_id: i64, ) -> Response { - let schedule = match coord.scheduled_prompts.get(schedule_id) { - Ok(Some(s)) => s, - Ok(None) => { - return Response::Err { - message: format!("schedule {schedule_id} not found"), - }; - } - Err(e) => { - return Response::Err { - message: format!("read schedule {schedule_id}: {e:#}"), - }; - } - }; - if !schedule_authorized(requester, &schedule.owner) { - return Response::Err { - message: format!( - "not authorized: {requester} cannot fire schedule owned by {owner}", - owner = schedule.owner - ), - }; + if let Some(err) = require_schedule(coord, schedule_id) { + return err; } + tracing::info!(%requester, %schedule_id, "fire schedule now"); // MCP fire_schedule_now stays no-reset (cadence intact); the // reset-timer option is a dashboard-dialog affordance. match crate::scheduled_prompts_worker::fire_now(coord, schedule_id, false).await { @@ -222,13 +179,11 @@ pub(super) struct EditSchedulePatch { pub(super) targets_remove: Option>, } -/// Authorize + dispatch a `EditSchedule` patch. Same ownership -/// rules as `CancelSchedule` — the manager can edit -/// schedules it owns + any owned by an agent in its subtree. -/// Forwards the partial payload to +/// Dispatch an `EditSchedule` patch. Same rule as `CancelSchedule`: any +/// requester, any schedule. Forwards the partial payload to /// `ScheduledPrompts::update` which enforces the cancelled-row / /// zero-interval validation. Returns `Ok` on a clean update; -/// `Err` with the underlying message on any auth / validation +/// `Err` with the underlying message on any validation /// failure so the dashboard can surface it verbatim. pub(super) fn handle_edit_schedule( coord: &Arc, @@ -244,27 +199,10 @@ pub(super) fn handle_edit_schedule( targets_add, targets_remove, } = patch; - let schedule = match coord.scheduled_prompts.get(schedule_id) { - Ok(Some(s)) => s, - Ok(None) => { - return Response::Err { - message: format!("schedule {schedule_id} not found"), - }; - } - Err(e) => { - return Response::Err { - message: format!("read schedule {schedule_id}: {e:#}"), - }; - } - }; - if !schedule_authorized(requester, &schedule.owner) { - return Response::Err { - message: format!( - "not authorized: {requester} cannot edit schedule owned by {owner}", - owner = schedule.owner - ), - }; + if let Some(err) = require_schedule(coord, schedule_id) { + return err; } + tracing::info!(%requester, %schedule_id, "edit schedule"); let patch = crate::scheduled_prompts::UpdateSchedule { body, description, @@ -284,23 +222,19 @@ pub(super) fn handle_edit_schedule( } } -/// Permission check shared by the whole scheduling surface: `requester` -/// can act on a schedule it owns, on one owned by an agent in its -/// subtree (delegated to topology — see `crate::topology::is_descendant_of`), -/// or, as the operator, on anything. `CancelSchedule`/`EditSchedule`/ -/// `FireScheduleNow` apply it as a hard reject; `ListSchedules` applies -/// it as a per-row filter. -fn schedule_authorized(requester: &str, owner: &str) -> bool { - if requester == owner { - return true; +/// Shared existence guard for the three schedule-mutating verbs: `None` when +/// the row is there, `Some(Err)` to short-circuit when it isn't or the read +/// failed. Not a permission check — any requester may act on any schedule. +fn require_schedule(coord: &Arc, schedule_id: i64) -> Option { + match coord.scheduled_prompts.get(schedule_id) { + Ok(Some(_)) => None, + Ok(None) => Some(Response::Err { + message: format!("schedule {schedule_id} not found"), + }), + Err(e) => Some(Response::Err { + message: format!("read schedule {schedule_id}: {e:#}"), + }), } - if requester == hive_sh4re::manager::OPERATOR_RECIPIENT { - return true; - } - // Manager can act on anything owned by an agent in its subtree. For - // the current single-manager topology that covers everything, but - // the check stays correct as the tree grows. - crate::topology::is_descendant_of(owner, requester) } /// Map a `scheduled_prompts::Schedule` to its public wire shape. @@ -396,32 +330,6 @@ mod tests { } } - #[test] - fn schedule_authorized_self_is_true() { - // Disk-free branch — never reaches `is_descendant_of`. - assert!(schedule_authorized("iris", "iris")); - } - - #[test] - fn schedule_authorized_operator_is_true() { - // Disk-free branch — never reaches `is_descendant_of`. - assert!(schedule_authorized( - hive_sh4re::manager::OPERATOR_RECIPIENT, - "iris" - )); - } - - #[test] - fn schedule_authorized_unrelated_requester_is_false() { - // Control: neither self nor operator, and no on-disk topology in - // a test sandbox connects these two names — pins that the - // predicate isn't accidentally stuck at `true`. - assert!(!schedule_authorized( - "definitely-not-the-owner", - "also-not-the-requester" - )); - } - #[test] fn ghost_filter_drops_dead_agents_keeps_live() { let live: std::collections::HashSet = ["iris".to_owned(), "damocles".to_owned()] diff --git a/hive-core-agent-sock/src/lib.rs b/hive-core-agent-sock/src/lib.rs index f7c37212..8b8c7a47 100644 --- a/hive-core-agent-sock/src/lib.rs +++ b/hive-core-agent-sock/src/lib.rs @@ -157,9 +157,7 @@ pub enum Request { #[serde(default, skip_serializing_if = "Option::is_none")] targets: Option>, }, - /// List the schedules the requester is authorized to see: its own, - /// plus any owned by an agent in its subtree (everything, for the - /// operator). + /// List every schedule in the hive, whoever owns it. ListSchedules, /// *(privileged)* Fire a scheduled prompt out of band immediately. FireScheduleNow { id: i64 },