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.
This commit is contained in:
parent
5202e5c5ba
commit
7bb68fe819
3 changed files with 75 additions and 2 deletions
|
|
@ -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('⚠'));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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:#}"),
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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<String> },
|
||||
/// Either `Send` failed or `Recv` errored.
|
||||
Err { message: String },
|
||||
/// `Recv` result: zero or more messages, FIFO-ordered, never
|
||||
|
|
|
|||
Loading…
Reference in a new issue