From 7bb68fe819987fff5975823be803d797313d14d6 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 27 Aug 2026 11:26:26 +0200 Subject: [PATCH] c0re: guard the agent-creation path every hive actually uses The reserved-name check landed only in `swarm-controller::create_agent`. That daemon is opt-in and off on most hives, so the ordinary per-hive flow -- `request_init_config` -> `handle_request_init_config` -> `require_new_child` -> `submit_init_config` -- stayed exactly as unguarded as before: an agent named `operator`, `forge` or `system` was still creatable through the path every hive uses, with no warning. Caught in review by argus. The issue named `create_agent` as the existing shape to copy, so the shape got copied and the question of which OTHER sites create an agent never got asked -- an issue naming one call site is describing an exemplar, not an inventory. Same treatment as the other path: warn, do not refuse. The warning needs somewhere to go. `Response` had `Ok` (carries nothing) and `Err` (refuses), so a check that warns had no way to reach its caller. Adds `Response::OkWarn { warnings }` -- additive, every existing `Ok` site is untouched -- rendered by `format_ack` *after* the success line rather than instead of it: the approval really was queued, and a warning shown as a failure invites a retry that queues a second one. Mutation-verified: dropping the warnings and unconditionally appending a marker each turn a different test red. --- hive-agent-mcp/src/mcp/render.rs | 43 ++++++++++++++++++- .../src/socket_server/config_approvals.rs | 21 ++++++++- hive-core-agent-sock/src/lib.rs | 13 ++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/hive-agent-mcp/src/mcp/render.rs b/hive-agent-mcp/src/mcp/render.rs index 435ef834..66a09d61 100644 --- a/hive-agent-mcp/src/mcp/render.rs +++ b/hive-agent-mcp/src/mcp/render.rs @@ -33,6 +33,18 @@ pub fn format_ack( ) -> String { match resp { Ok(hive_core_agent_sock::Response::Ok) => ok_msg, + // Succeeded, with something the caller needs to read. Rendered + // after the success line rather than instead of it: the operation + // DID happen, and a warning shown as though it were a failure + // invites a retry that would only queue a second one. + Ok(hive_core_agent_sock::Response::OkWarn { warnings }) => { + let mut out = ok_msg; + for w in &warnings { + out.push_str("\n⚠️ "); + out.push_str(w); + } + out + } other => reply_err(other, tool), } } @@ -598,7 +610,7 @@ pub fn annotate_retries(mut s: String, retries: u32) -> String { #[cfg(test)] mod tests { - use super::format_recv; + use super::{format_ack, format_recv}; fn msg(id: i64, from: &str, body: &str) -> hive_sh4re::inbox::DeliveredMessage { hive_sh4re::inbox::DeliveredMessage { @@ -651,4 +663,33 @@ mod tests { let batch = 9u64.min(u64::from(hive_sh4re::inbox::RECV_BATCH_MAX)); assert!(out.contains(&format!("max: {batch}"))); } + + #[test] + fn ok_warn_keeps_the_success_line_and_appends_each_warning() { + let out = format_ack( + Ok(hive_core_agent_sock::Response::OkWarn { + warnings: vec!["name is reserved".to_owned(), "second thing".to_owned()], + }), + "request_init_config", + "init_config approval queued for forge".to_owned(), + ); + // The operation HAPPENED — dropping the success line would read as a + // failure and invite a retry that queues a second approval. + assert!(out.starts_with("init_config approval queued for forge")); + assert!(out.contains("⚠️ name is reserved")); + assert!(out.contains("⚠️ second thing")); + } + + #[test] + fn plain_ok_is_untouched_by_the_warning_path() { + // Absence arm: without it, a renderer that always appended a + // warning marker would pass the test above. + let out = format_ack( + Ok(hive_core_agent_sock::Response::Ok), + "request_init_config", + "queued".to_owned(), + ); + assert_eq!(out, "queued"); + assert!(!out.contains('⚠')); + } } diff --git a/hive-c0re/src/socket_server/config_approvals.rs b/hive-c0re/src/socket_server/config_approvals.rs index 83a2856b..01ef9dff 100644 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ b/hive-c0re/src/socket_server/config_approvals.rs @@ -29,8 +29,27 @@ pub(super) fn handle_request_init_config( return err; } tracing::info!(%agent, %name, "request_init_config"); + // Warn, do not refuse: an agent already created under a colliding + // name must stay re-initialisable, so the refusal comes later, once + // the warning has had time to be seen. + // + // Checked HERE and not only in `swarm-controller::create_agent`: + // that daemon is opt-in and off on most hives, while this is the + // path the `request_init_config` tool takes on every hive. Guarding + // only the rarer one would have left the common flow exactly as + // unguarded as before. + let warnings = if hive_types::is_reserved_name(name) { + tracing::warn!(%agent, %name, "request_init_config: reserved name"); + vec![format!( + "agent name {name:?} is a reserved protocol name — messages from this agent will be \ + indistinguishable from hyperhive's own; this will become an error" + )] + } else { + Vec::new() + }; match submit_init_config(coord, name, Some(agent), description) { - Ok(_id) => Response::Ok, + Ok(_id) if warnings.is_empty() => Response::Ok, + Ok(_id) => Response::OkWarn { warnings }, Err(e) => Response::Err { message: format!("{e:#}"), }, diff --git a/hive-core-agent-sock/src/lib.rs b/hive-core-agent-sock/src/lib.rs index 14c59c92..0d983dd8 100644 --- a/hive-core-agent-sock/src/lib.rs +++ b/hive-core-agent-sock/src/lib.rs @@ -245,6 +245,19 @@ pub enum Request { pub enum Response { /// `Send` succeeded. Ok, + /// The operation succeeded **and** raised advisories the caller + /// should see. + /// + /// Distinct from `Err` on purpose: a check that WARNS rather than + /// refuses has no other way to reach the caller. Folding it into + /// `Err` would refuse; folding it into `Ok` would drop it, leaving + /// the daemon's log as the only record — and the person who can act + /// on the advisory is the one holding this response, not the one + /// reading the journal. + /// + /// Additive: every existing site returning `Ok` keeps returning it, + /// so a caller that never matches this variant is unaffected. + OkWarn { warnings: Vec }, /// Either `Send` failed or `Recv` errored. Err { message: String }, /// `Recv` result: zero or more messages, FIFO-ordered, never