hive-c0re: remove the approvals subsystem
Nothing submits an approval since scheduled prompts left hive-c0re, so the
queue only acted on rows written before that.
- hive-c0re: the approvals store and its migrations, approve/deny/
resolve actions, the ResolveApproval tail node (and meta_update's
approval_id), POST /api/approve and /api/deny, the approval views on
/api/state, the approval_resolved dashboard event, the orphan GC,
destroy/purge failing pending rows, and the approval rows in
get_loose_ends. DestroyBookkeeping loses the purge flag, which only
worded the approval-failure note.
- The CancelLooseEnd request goes: its only hive-c0re arm was the
approval one. cancel_loose_end handles reminder and todo in the harness.
- Wire types: hive_sh4re::approvals (ReminderStats moves to inbox),
LooseEnd::Approval, CancelLooseEndKind, HelperEvent::ApprovalResolved,
ToolGroup::Approvals, HostRequest::{Pending,Approve,Deny} and
HostResponse.approvals. HOST_SOCK_VERSION goes to 2 for the removed
field.
- hivectl loses the approvals verbs; docs/tools/hivectl-cli.md is
regenerated.
- The system prompt drops approval_resolved.
- The approvals table stays in broker.sqlite, unread, the same way the
operator_questions table was left when its store was removed.
- An `approvals` entry still in tool-groups.json is skipped with a warning
by ToolGroup::parse_list, like any unknown group.
- docs/agent-lifecycle/approvals.md is renamed to config-changes.md so the
code's doc pointers resolve; its content follows in the docs commit.
This commit is contained in:
parent
99bd912ae0
commit
58ef299c3c
60 changed files with 153 additions and 1828 deletions
|
|
@ -3,10 +3,9 @@
|
|||
//! authenticates the caller: connecting to `<.../agents/foo/mcp.sock>` means
|
||||
//! 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
|
||||
//! (tool-group membership for cancelling an approval). 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.
|
||||
//! code, and authority derives uniformly from the caller's identity. 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;
|
||||
|
|
@ -292,12 +291,6 @@ async fn dispatch(
|
|||
hive_core_agent_sock::Request::GetAgentMeta { name } => {
|
||||
handle_get_agent_meta(coord, agent, name.as_ref()).await
|
||||
}
|
||||
hive_core_agent_sock::Request::CancelLooseEnd { kind, id } => {
|
||||
handle_cancel_loose_end(coord, agent, *kind, *id).map_or_else(
|
||||
|message| hive_core_agent_sock::Response::Err { message },
|
||||
|()| hive_core_agent_sock::Response::Ok,
|
||||
)
|
||||
}
|
||||
hive_core_agent_sock::Request::AckTurn => handle_ack_turn(coord, agent),
|
||||
hive_core_agent_sock::Request::AckUntil { up_to } => handle_ack_until(coord, agent, *up_to),
|
||||
hive_core_agent_sock::Request::RequeueInflight => handle_requeue_inflight(coord, agent),
|
||||
|
|
@ -565,93 +558,6 @@ fn handle_get_loose_ends(coord: &Arc<Coordinator>, agent: &str) -> Response {
|
|||
}
|
||||
}
|
||||
|
||||
/// Handle `CancelLooseEnd` from a per-agent socket. Dispatches by kind, each
|
||||
/// with its own auth check: reminder cancels are handled entirely in-container
|
||||
/// (this arm should be unreachable in practice, kept only so the match stays
|
||||
/// exhaustive) and approval cancels require the `approvals` tool-group (the
|
||||
/// grantable capability) AND ownership — the canceller must be the approval's
|
||||
/// submitter — so no positional / hardcoded privilege and no cross-agent
|
||||
/// cancellation. (The operator's cancel-anything path is a separate handler.)
|
||||
fn handle_cancel_loose_end(
|
||||
coord: &Arc<Coordinator>,
|
||||
canceller: &str,
|
||||
kind: hive_sh4re::inbox::CancelLooseEndKind,
|
||||
id: i64,
|
||||
) -> Result<(), String> {
|
||||
match kind {
|
||||
hive_sh4re::inbox::CancelLooseEndKind::Reminder => {
|
||||
// Reminders are now agent-local (in-container store) — the
|
||||
// agent-mcp `cancel_loose_end` tool branches on this kind and
|
||||
// dials the agent's own socket directly, never forwarding to
|
||||
// hive-c0re. This arm should be unreachable in practice; kept
|
||||
// only so the match stays exhaustive.
|
||||
Err(format!(
|
||||
"reminder {id}: reminders are handled locally by the agent, \
|
||||
not by hive-c0re"
|
||||
))
|
||||
}
|
||||
hive_sh4re::inbox::CancelLooseEndKind::Approval => {
|
||||
// Withdrawing an approval needs the grantable `approvals`
|
||||
// tool-group (held by any approval-submitting orchestrator)
|
||||
// AND ownership: only the agent that submitted the approval
|
||||
// may withdraw it. Without the ownership check, any
|
||||
// approvals-group agent could cancel any other's approval by
|
||||
// id. A NULL submitter (legacy row predating the column) is
|
||||
// treated as operator-initiated (no agent tracking predates
|
||||
// the column).
|
||||
check_can_cancel_approval(canceller)?;
|
||||
let submitter = coord
|
||||
.approvals
|
||||
.submitter_of(id)
|
||||
.map_err(|e| format!("{e:#}"))?
|
||||
.unwrap_or_else(|| "operator".to_owned());
|
||||
if submitter != canceller {
|
||||
return Err(format!(
|
||||
"cancel_loose_end: approval {id} was submitted by {submitter}, \
|
||||
not {canceller}; only the submitting agent can withdraw it"
|
||||
));
|
||||
}
|
||||
let approval = coord
|
||||
.approvals
|
||||
.mark_cancelled(id, canceller)
|
||||
.map_err(|e| format!("{e:#}"))?;
|
||||
tracing::info!(%id, %canceller, agent = %approval.agent, "approval cancelled");
|
||||
coord.emit_approval_resolved(crate::coordinator::ApprovalResolved {
|
||||
id: approval.id,
|
||||
agent: approval.agent.as_str(),
|
||||
approval_kind: <&str>::from(approval.kind),
|
||||
status: "cancelled",
|
||||
note: approval.note,
|
||||
description: approval.description,
|
||||
});
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Capability guard on the `Approval` cancel arm: the caller must hold the
|
||||
/// `approvals` tool-group (the grantable capability for approval-submitting
|
||||
/// orchestrators), checked server-side via `tool_groups::groups_for`. Pulled
|
||||
/// out so the auth check has its own focused unit test — exercising the full
|
||||
/// `handle_cancel_loose_end` flow would need a `Coordinator` fixture (broker +
|
||||
/// sqlite). Keys on a grantable capability, not a positional / hardcoded
|
||||
/// privilege.
|
||||
fn check_can_cancel_approval(canceller: &str) -> Result<(), String> {
|
||||
const APPROVALS_GROUP: &str = "approvals";
|
||||
if crate::tool_groups::groups_for(canceller)
|
||||
.map_err(|e| format!("cancel_loose_end: {e}"))?
|
||||
.iter()
|
||||
.any(|g| g == APPROVALS_GROUP)
|
||||
{
|
||||
Ok(())
|
||||
} else {
|
||||
Err(
|
||||
"cancel_loose_end: cancelling approval rows requires the `approvals` tool group"
|
||||
.to_owned(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// `Send` handler. Applies the
|
||||
/// 4 KiB body cap, then routes broadcast (`to == "*"`) / unicast through
|
||||
/// their respective broker calls.
|
||||
|
|
@ -736,17 +642,6 @@ mod tests {
|
|||
(dir, Arc::new(coord))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_cancel_rejects_callers_without_the_approvals_group() {
|
||||
// A caller that doesn't hold the `approvals` tool-group must not be
|
||||
// able to cancel approval rows even if it invents an id. The guard is
|
||||
// server-side so client cooperation is irrelevant — and it keys on a
|
||||
// grantable capability (the tool-group), not on any agent name.
|
||||
// `groups_for` of a name with no tool_groups.json entry is empty.
|
||||
let err = check_can_cancel_approval("nobody-with-no-groups").unwrap_err();
|
||||
assert!(err.contains("approvals` tool group"), "{err}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn over_long_line_is_rejected_after_reading_only_the_bound() {
|
||||
let total = 2 * REQUEST_LINE_MAX_BYTES as u64;
|
||||
|
|
|
|||
Loading…
Reference in a new issue