diff --git a/hive-agent-mcp/src/mcp/render.rs b/hive-agent-mcp/src/mcp/render.rs index 66a09d61..435ef834 100644 --- a/hive-agent-mcp/src/mcp/render.rs +++ b/hive-agent-mcp/src/mcp/render.rs @@ -33,18 +33,6 @@ 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), } } @@ -610,7 +598,7 @@ pub fn annotate_retries(mut s: String, retries: u32) -> String { #[cfg(test)] mod tests { - use super::{format_ack, format_recv}; + use super::format_recv; fn msg(id: i64, from: &str, body: &str) -> hive_sh4re::inbox::DeliveredMessage { hive_sh4re::inbox::DeliveredMessage { @@ -663,33 +651,4 @@ 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 abb28b64..83a2856b 100644 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ b/hive-c0re/src/socket_server/config_approvals.rs @@ -29,46 +29,8 @@ 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. - // - // The blacklist itself comes from nix via `HIVE_RESERVED_NAMES`, so it - // stays a config change rather than a rebuild. An UNSET variable means - // this daemon was never told — which is not the same as "no name is - // reserved", and saying nothing there would be a check that reports - // clean because it could not run. - let raw = hive_types::reserved_names_raw(); - let warnings = match raw.as_deref().map(hive_types::parse_reserved_names) { - None => { - tracing::error!( - var = hive_types::RESERVED_NAMES_ENV, - "request_init_config: reserved-name check could not run — variable not set" - ); - vec![format!( - "the reserved-name check did not run: {} is unset, so {name:?} was accepted \ - without being checked against the protocol literals", - hive_types::RESERVED_NAMES_ENV - )] - } - Some(reserved) if hive_types::is_reserved_name(name, &reserved) => { - 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" - )] - } - Some(_) => Vec::new(), - }; match submit_init_config(coord, name, Some(agent), description) { - Ok(_id) if warnings.is_empty() => Response::Ok, - Ok(_id) => Response::OkWarn { warnings }, + Ok(_id) => Response::Ok, 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 0d983dd8..14c59c92 100644 --- a/hive-core-agent-sock/src/lib.rs +++ b/hive-core-agent-sock/src/lib.rs @@ -245,19 +245,6 @@ 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 diff --git a/hive-sh4re/src/manager.rs b/hive-sh4re/src/manager.rs index a5cdf017..dcfd72e8 100644 --- a/hive-sh4re/src/manager.rs +++ b/hive-sh4re/src/manager.rs @@ -131,105 +131,3 @@ pub struct SchedulePromptPayload { #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, } - -#[cfg(test)] -mod reserved_name_tests { - use super::{ - CHILDREN_RECIPIENT, MANAGER_AGENT, OPERATOR_RECIPIENT, PARENT_RECIPIENT, SYSTEM_SENDER, - }; - use hive_types::{Ident, RESERVED_NAMES_ENV, is_reserved_name}; - - /// The blacklist as nix rendered it for this test run. - /// - /// **Panics when the variable is absent, deliberately.** The list lives - /// in `nix/reserved-names.nix` now, so this test can no longer read it - /// from the Rust tree; `nix/checks.nix` and `nix/devshell.nix` both - /// export it. Skipping instead would turn "nobody wired the variable" - /// into a green run — the same shape as a checker that reports clean - /// because it crashed, and the whole point of a drift test is that it - /// is the thing that notices. - fn reserved() -> Vec { - let raw = hive_types::reserved_names_raw().unwrap_or_else(|| { - panic!( - "{RESERVED_NAMES_ENV} is unset or blank, so the drift test cannot run. \ - nix/checks.nix and nix/devshell.nix are supposed to export it from \ - nix/reserved-names.nix — fix the plumbing rather than this test." - ) - }); - hive_types::parse_reserved_names(&raw) - .into_iter() - .map(str::to_owned) - .collect() - } - - /// The sentinels declared here and the blacklist nix owns are two - /// spellings of one fact, in places that cannot import each other's - /// intent. This pins them together: adding a sentinel without reserving - /// it now fails here rather than years later, when an agent takes the - /// name. - #[test] - fn ident_shaped_sentinels_are_reserved() { - let owned = reserved(); - let reserved: Vec<&str> = owned.iter().map(String::as_str).collect(); - for sentinel in [OPERATOR_RECIPIENT, SYSTEM_SENDER] { - assert!( - is_reserved_name(sentinel, &reserved), - "{sentinel:?} is a sentinel an agent could be named — it must be in \ - nix/reserved-names.nix" - ); - } - } - - /// Every entry nix hands us must be a name an agent could actually have - /// been given. One that `Ident::parse` rejects is dead weight — nothing - /// could ever have been created with it, so listing it implies a guard - /// doing nothing. `graceful-stop` is what makes this worth asserting: it - /// is hyphenated, and a charset tightening would silently retire it. - /// - /// This assertion used to live beside the list in `hive-types`; it moved - /// here because here is where the real list is readable. - #[test] - fn every_reserved_name_is_a_valid_ident() { - let owned = reserved(); - assert!( - !owned.is_empty(), - "the blacklist rendered empty — an empty list makes every assertion below vacuous" - ); - for name in &owned { - assert!( - Ident::parse(name).is_ok(), - "{name:?} is reserved but not a parseable ident — one of the two is wrong" - ); - } - } - - /// The other half, and the reason the test above is not vacuous: these - /// sentinels are *unreachable* as agent names because the charset - /// rejects them, so they are correctly absent from the list. If a - /// charset change ever made one parseable, it would become a real - /// collision and this test is what notices. - #[test] - fn bracketed_recipients_cannot_be_agent_names() { - let owned = reserved(); - let reserved: Vec<&str> = owned.iter().map(String::as_str).collect(); - for sentinel in [PARENT_RECIPIENT, CHILDREN_RECIPIENT] { - assert!( - Ident::parse(sentinel).is_err(), - "{sentinel:?} parses as an ident now — it is reachable as an agent name and must be reserved" - ); - assert!(!is_reserved_name(sentinel, &reserved)); - } - } - - /// `ruth` is a real agent, not a protocol literal, so it is not - /// reserved: a second agent wanting the name is a *taken* name, which - /// the roster check answers. Recorded as a test so the distinction is - /// enforced rather than remembered. - #[test] - fn manager_name_is_taken_not_reserved() { - let owned = reserved(); - let reserved: Vec<&str> = owned.iter().map(String::as_str).collect(); - assert!(Ident::parse(MANAGER_AGENT).is_ok()); - assert!(!is_reserved_name(MANAGER_AGENT, &reserved)); - } -} diff --git a/hive-types/src/lib.rs b/hive-types/src/lib.rs index d1a51508..40d04857 100644 --- a/hive-types/src/lib.rs +++ b/hive-types/src/lib.rs @@ -6,64 +6,6 @@ //! and get serde-validated parsing at the socket boundary for free — with //! no cross-crate coupling and without growing `hive-sh4re`. -/// The environment variable nix uses to hand this process the blacklist of -/// names that already mean something to the message layer. -/// -/// **Nix owns the list**, not this crate: `nix/reserved-names.nix` is the one -/// copy, and it reaches `hive-c0re`, `swarm-controller`, the swarm collector's -/// own assertion and the test suite from that single file. Keeping the -/// blacklist current is therefore a config change, not a rebuild of a binary -/// — and there is no second list to drift. -/// -/// Deliberately **not** enforced inside [`Ident::parse`]. Parsing runs on -/// every read of an already-created name, so rejecting there would make -/// existing agents unreadable rather than un-creatable, and it would be a -/// refusal — which is a stronger action than the warning this list is -/// currently used for. Creation sites call [`is_reserved_name`]; readers do -/// not. -/// -/// ⚠️ An **unset** variable means *this process was not told*, which is not -/// the same as *nothing is reserved*. Callers must say so out loud rather -/// than silently treating every name as available. -pub const RESERVED_NAMES_ENV: &str = "HIVE_RESERVED_NAMES"; - -/// Split the value of [`RESERVED_NAMES_ENV`] into names. -/// -/// Whitespace-separated, not JSON, and that is a deliberate departure from -/// the structured env vars elsewhere in the tree. Every entry is an [`Ident`], -/// whose charset is `[a-z0-9-]` — so whitespace cannot occur *inside* a name -/// and the encoding is provably lossless. Paying for a JSON parser here would -/// mean either a dependency in a crate whose entire purpose is to have none, -/// or a separate copy of the parse in every consumer. -#[must_use] -pub fn parse_reserved_names(raw: &str) -> Vec<&str> { - raw.split_whitespace().collect() -} - -/// The raw value of [`RESERVED_NAMES_ENV`], or `None` when this process was -/// never told what the blacklist is. -/// -/// A **blank** value folds into `None` on purpose. Nix always renders a -/// non-empty list, so a variable that is present but empty is a rendering -/// fault, not an operator declaring that nothing is reserved — and the two -/// must not look the same to a caller whose next move is to warn about it. -#[must_use] -pub fn reserved_names_raw() -> Option { - std::env::var(RESERVED_NAMES_ENV) - .ok() - .filter(|raw| !raw.trim().is_empty()) -} - -/// Whether `name` is one of the protocol literals in `reserved`. -/// -/// Call at **creation** sites only. A caller that is reading or routing an -/// existing name must not consult this: the name is already in use, and the -/// question there is where it goes, not whether it should exist. -#[must_use] -pub fn is_reserved_name(name: &str, reserved: &[&str]) -> bool { - reserved.contains(&name) -} - /// A validated hive identifier: 1-63 chars of `[a-z0-9-]`. /// /// The single ident type for agent names, forge labels, and matrix / github @@ -156,7 +98,7 @@ impl<'de> serde::Deserialize<'de> for Ident { #[cfg(test)] mod ident_tests { - use super::{Ident, is_reserved_name, parse_reserved_names}; + use super::Ident; #[test] fn accepts_canonical_shapes() { @@ -193,53 +135,6 @@ mod ident_tests { } } - #[test] - fn reserved_names_are_flagged_and_ordinary_names_are_not() { - // A sample, not the real list: the real one lives in nix now, and - // what this crate still owns is the *predicate*. The content of the - // blacklist is asserted where the env var is readable — see - // `hive-sh4re`'s drift test. - let reserved = ["operator", "system", "graceful-stop"]; - // Presence arm: every entry must actually be reported. - for name in reserved { - assert!(is_reserved_name(name, &reserved), "{name:?} not reported"); - } - // Absence arm, and the reason this test can fail: without it a - // predicate that always returns `true` passes the loop above. - for ok in ["atlas", "damocles", "iris", "operator-2", "sys", "forged"] { - assert!(!is_reserved_name(ok, &reserved), "{ok:?} must NOT be"); - } - // An empty list reserves nothing — the shape a caller gets when the - // env var is unset, and the reason a caller must not treat that - // case as "nothing is reserved" without saying so. - assert!(!is_reserved_name("operator", &[])); - } - - #[test] - fn parses_the_env_encoding() { - assert_eq!( - parse_reserved_names("operator system graceful-stop"), - ["operator", "system", "graceful-stop"] - ); - // Newlines and runs of spaces are what a nix-rendered list looks - // like when someone reformats the file; both must fold away. - assert_eq!( - parse_reserved_names(" operator\n system \n"), - ["operator", "system"] - ); - // Absence and emptiness collapse to the same empty list, which is - // why the CALLER, not this function, has to distinguish them. - assert!(parse_reserved_names("").is_empty()); - // Every name the encoding can carry must survive a round trip - // through `Ident::parse`: a blacklist entry that is not a legal - // ident is dead weight, since nothing could ever be created with - // it. `graceful-stop` is the one that makes this worth asserting — - // it is hyphenated, and a charset tightening would retire it. - for name in parse_reserved_names("operator system todo graceful-stop") { - assert!(Ident::parse(name).is_ok(), "{name:?} is not an ident"); - } - } - #[test] fn round_trips_and_serde_validates() { let id = Ident::parse("damocles").unwrap(); diff --git a/nix/checks.nix b/nix/checks.nix index 0944d240..e9e01878 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -62,14 +62,6 @@ in version = "0.1.0"; cargoTestExtraArgs = "--workspace"; HIVE_ASSETS_DIR = "${self.packages.${system}.assets}/share/hyperhive"; - # The reserved-name blacklist moved out of the Rust tree and into - # `reserved-names.nix`, so the test that pins the message layer's - # sentinels against it can only run if nix hands it the same list the - # daemons get. Exported here and in `devshell.nix`, and the test PANICS - # rather than skipping when the variable is missing: a drift test that - # quietly does nothing is worse than no drift test, because it still - # shows up green. - HIVE_RESERVED_NAMES = pkgs.lib.concatStringsSep " " (import ./reserved-names.nix); }; # Rustdoc gate. Builds the workspace's docs and turns rustdoc's own diff --git a/nix/devshell.nix b/nix/devshell.nix index 0d3400aa..36c07af8 100644 --- a/nix/devshell.nix +++ b/nix/devshell.nix @@ -5,12 +5,6 @@ { pkgs, rust }: { default = pkgs.mkShell { - # Same list the daemons are handed and the same one `checks.nix` gives - # the test derivation — `nix/reserved-names.nix`. Without it here, a - # plain `cargo test` in the shell would panic on the drift test, so a - # developer's local run and CI would disagree about a test that exists - # precisely to stop two spellings of one fact from drifting. - HIVE_RESERVED_NAMES = pkgs.lib.concatStringsSep " " (import ./reserved-names.nix); packages = rust.nativeBuildInputs ++ (with pkgs; [ diff --git a/nix/host-modules/hive-c0re/environment.nix b/nix/host-modules/hive-c0re/environment.nix index c1df0c6e..92b612e8 100644 --- a/nix/host-modules/hive-c0re/environment.nix +++ b/nix/host-modules/hive-c0re/environment.nix @@ -143,21 +143,6 @@ in # mandatory, so this is unconditional (the whole env block is already # gated on hyperhive being enabled). See `docs/gateway.md::HIVE_FORGE_URL`. HIVE_FORGE_URL = "http://${config.services.hyperhive.swarm.forge.domain}"; - - # The one blacklist of names an agent may not take — see - # `nix/reserved-names.nix`, which is also read by the swarm controller, by - # the swarm collector's owner assertion, and by the test suite. Nix owns it - # so that keeping it current is a config change, not a rebuild of a binary. - # - # Whitespace-separated rather than JSON: every entry is an `Ident` - # (`[a-z0-9-]`), so a space can never occur inside a name and the encoding - # cannot be lossy. Spelling it as JSON would put a parser in the crate whose - # whole point is to have no dependencies. - # - # Unconditional on purpose. The consumer treats an ABSENT variable as "I was - # never told" and says so out loud, which is the correct reading — but it is - # a reading no correctly-built hive should ever have to make. - HIVE_RESERVED_NAMES = lib.concatStringsSep " " (import ../../reserved-names.nix); } // lib.optionalAttrs diff --git a/nix/host-modules/swarm-controller.nix b/nix/host-modules/swarm-controller.nix index 5ba54251..8a94855f 100644 --- a/nix/host-modules/swarm-controller.nix +++ b/nix/host-modules/swarm-controller.nix @@ -648,13 +648,6 @@ in # `swarm.peerHives`, `swarm.hives` minus this hive) rather than # peers-minus-self. Consumed by `GET /api/hives` # (swarm-controller/src/main.rs::load_hives). - # The one blacklist of names an agent may not take, shared verbatim - # with hive-c0re and with the collector's owner assertion — see - # `nix/reserved-names.nix`. Whitespace-separated rather than JSON - # like its neighbour below, because every entry is an `Ident` and so - # cannot contain a space; the neighbour carries objects and has no - # such option. - HIVE_RESERVED_NAMES = lib.concatStringsSep " " (import ../reserved-names.nix); SWARM_CONTROLLER_HIVES = builtins.toJSON ( lib.mapAttrsToList (name: h: { inherit name; diff --git a/nix/host-modules/swarm-otel.nix b/nix/host-modules/swarm-otel.nix index 9d5ebd00..4408ab49 100644 --- a/nix/host-modules/swarm-otel.nix +++ b/nix/host-modules/swarm-otel.nix @@ -55,16 +55,9 @@ let # repeated at each site would let the guard and the config drift apart, which # is the failure this guard exists to prevent. swarmTierName = "swarm"; - # Every `` no hive may take. Read from `nix/reserved-names.nix`, the - # same file the daemons are handed as `HIVE_RESERVED_NAMES`, because agent - # names and hive names are ONE namespace going forward — a locally-owned - # list here would be a second copy to keep in step, which is the failure a - # single blacklist exists to prevent. - # - # The assertion below still checks the string this module emits: the file - # is asserted to CONTAIN `swarmTierName`, so a rename that dropped it from - # the file would be an eval error rather than a silently missing guard. - reservedOwners = import ../reserved-names.nix; + # Every `` this module claims for itself. One entry today; a second + # swarm-tier pipeline would be added here and inherit the check for free. + reservedOwners = [ swarmTierName ]; # A published target is declared as ONE url, because that url is also the # audience its token is minted for — but prometheus wants the same fact in @@ -588,23 +581,6 @@ in List the swarm's hives. ''; } - { - # The blacklist now lives in a shared file, so this module no longer - # controls its contents — and a guard whose subject can be edited - # elsewhere has to assert that its own case is still in there. Without - # this, deleting one line from `reserved-names.nix` would silently - # retire the check below rather than fail anything. - assertion = lib.elem swarmTierName reservedOwners; - message = '' - nix/reserved-names.nix no longer contains '${swarmTierName}', which - the swarm collector needs reserved: it names components - `/` and uses the hive name as the owner, so a hive - called '${swarmTierName}' would replace the swarm tier's own - pipelines and lose its own. - - Put it back, or give this module a different swarmTierName. - ''; - } { # A hive whose name is one this module claims for itself collides in # the collector's component namespace, and `//` resolves it silently: diff --git a/nix/reserved-names.nix b/nix/reserved-names.nix deleted file mode 100644 index f7fbb6b2..00000000 --- a/nix/reserved-names.nix +++ /dev/null @@ -1,67 +0,0 @@ -# The one blacklist: names no agent and no hive may take. -# -# Nix owns this list and hands it to the Rust side as an environment variable -# (`HIVE_RESERVED_NAMES`), so keeping it current is a config change rather than -# a rebuild of a binary. Read by: -# -# - `host-modules/hive-c0re/environment.nix` -> the env var, for the -# `request_init_config` path every hive uses -# - `host-modules/swarm-controller.nix` -> the same var, for the -# swarm-level `create_agent` path -# - `host-modules/swarm-otel.nix` -> its `` assertion, so -# a hive name and an agent name are checked against ONE list -# - `nix/checks.nix` -> exported into `cargo test`, -# which is what keeps the message layer's sentinels from drifting away -# from this file -# -# A plain nix file rather than a module option because two of those readers are -# flake-level (`checks.nix`) and cannot see a NixOS option. -# -# ⚠️ Agent names and hive names are ONE namespace going forward (mara, 2026-08-27: -# "agent and hive names live in the swarm level going forward"). Adding a name -# here forbids it for both. That is the point: two lists is how they drift. -# -# ⚠️ Every entry must be a value some component actually PRODUCES as a message -# `from`/`to`, or a component name the collector builds pipelines from — not a -# word that merely looked risky. A name in here that nothing emits is a refusal -# with no failure behind it. -[ - # ---- message-layer senders ------------------------------------------- - # The human at the dashboard: a broker recipient (the T4LK box sends - # `{from: "operator", ...}`) and the fallback attribution for an answered - # question. - "operator" - # Helper events (`approval_resolved`, `container_crash`, ...) — the sender an - # agent is told to treat as hyperhive itself rather than as a peer. - # `hive_sh4re::manager::SYSTEM_SENDER`. - "system" - # A due self-scheduled reminder arrives as its own sender, so a wake I asked - # for last week is distinguishable from a peer message. - "reminder" - # Forge notification wakes, delivered by the notify daemon. - "forge" - # A scheduled prompt firing, pushed as a trusted sender. - "scheduled" - # Three synthetic wakes the harness itself produces: an in-container todo, - # the follow-up turn after a self-requested compaction, and the single flush - # turn before a graceful stop. - "todo" - "compact" - "graceful-stop" - - # ---- swarm-tier component owners ------------------------------------- - # The swarm collector names its components `/` and uses the hive - # name as the owner, so a hive called `swarm` would silently replace the - # swarm tier's own pipelines and lose its own — it would keep accepting - # pushes into a pipeline that routes nowhere. Previously enforced only - # against hive names, in `swarm-otel.nix`'s own `reservedOwners`. - "swarm" -] -# Deliberately absent, and both are load-bearing omissions: -# -# `` / `` — routing recipients the ident charset already -# rejects, so no name can ever equal them; listing them would imply a guard -# that never fires. -# -# `ruth` — a real agent, not a literal. A second agent wanting that name is a -# name that is TAKEN, which the roster answers, not this list. diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index bad9edfe..7b7290f1 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -682,20 +682,6 @@ struct CreateAgentRequest { #[derive(Clone, Debug, Serialize, ToSchema)] struct CreateAgentResponse { node_id: u64, - /// Name collisions that were **allowed through**, phrased for the - /// operator who just chose the name. - /// - /// The creation still happened — this is the "loud warning now, refuse - /// after a grace period" step, so a caller that ignores this field gets - /// exactly today's behaviour. It rides on the response rather than - /// living only in the daemon's log because the person who can still fix - /// the name in one keystroke is the one holding this response, and they - /// are not reading the journal. - /// - /// Empty (and omitted from the JSON) in the normal case, so a client - /// that never looks at it is not handed an empty array to reason about. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - warnings: Vec, } /// Queue the whole agent-creation job graph for `name` — two independent @@ -750,61 +736,9 @@ async fn create_agent( // `state.hives` is the swarm directory loaded from `SWARM_CONTROLLER_HIVES` // at startup and never mutated, so this is a scan of a handful of // entries against a value an operator just chose. - // The name is shaped like an identifier; these two ask whether it is - // *available*. Both WARN rather than refuse: an operator with agents - // already created under a colliding name would otherwise be unable to - // re-run creation at all, so the refusal comes after a grace period, - // once the warning has had time to be seen. - // - // Collected rather than logged-and-dropped — see `CreateAgentResponse`. - let mut warnings = Vec::new(); - // The blacklist comes from nix via `HIVE_RESERVED_NAMES` — one file, read - // by this daemon, by hive-c0re and by the swarm collector's own owner - // assertion. An UNSET variable means this process was never told, which - // is not the same as "no name is reserved": staying quiet there would be - // a check that reports clean because it could not run. - let raw = hive_types::reserved_names_raw(); - match raw.as_deref().map(hive_types::parse_reserved_names) { - None => { - tracing::error!( - var = hive_types::RESERVED_NAMES_ENV, - "create_agent: reserved-name check could not run — variable not set" - ); - warnings.push(format!( - "the reserved-name check did not run: {} is unset, so {agent:?} was accepted \ - without being checked against the protocol literals", - hive_types::RESERVED_NAMES_ENV - )); - } - Some(reserved) if hive_types::is_reserved_name(&agent, &reserved) => { - // A protocol literal: the message layer already produces this - // name as a sender or recipient, so wakes from the component and - // messages from the agent become the same broker row. - tracing::warn!(agent = %agent, "create_agent: reserved name"); - warnings.push(format!( - "agent name {agent:?} is a reserved protocol name — messages from this agent will \ - be indistinguishable from hyperhive's own; this will become an error" - )); - } - Some(_) => {} - } - let hive = hive_types::Ident::parse(&req.hive) .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? .into_string(); - - // The roster is the same source the `hive` field is validated against - // immediately below — one scan, asked of the other field. A hive and an - // agent sharing a name collide in the swarm's own directory keys, which - // is what makes a pipeline and a label vanish with no failed assertion. - if state.hives.iter().any(|h| h.name == agent) { - let detail = format!( - "agent name {agent:?} is also a hive in this swarm — they share the swarm's directory \ - keys; this will become an error" - ); - tracing::warn!(agent = %agent, "create_agent: name collides with a hive"); - warnings.push(detail); - } if !state.hives.iter().any(|h| h.name == hive) { let known: Vec<&str> = state.hives.iter().map(|h| h.name.as_str()).collect(); // Name the hives that *would* work: the caller is an operator who @@ -864,10 +798,7 @@ async fn create_agent( let [id] = ids[..] else { unreachable!("exactly one handle was asked for"); }; - Ok(Json(CreateAgentResponse { - node_id: id.get(), - warnings, - })) + Ok(Json(CreateAgentResponse { node_id: id.get() })) } /// Query params for `GET /api/jobq/graph` — `?states=` narrows to root @@ -1666,33 +1597,3 @@ mod tests { } } } - -#[cfg(test)] -mod create_agent_warning_tests { - use super::CreateAgentResponse; - - /// The normal case must not grow a field. A client that has never - /// heard of warnings should see the response it always saw — that is - /// what makes "warn now, refuse later" a non-breaking first step. - #[test] - fn no_warnings_are_omitted_from_the_json_entirely() { - let json = serde_json::to_string(&CreateAgentResponse { - node_id: 7, - warnings: vec![], - }) - .unwrap(); - assert_eq!(json, r#"{"node_id":7}"#); - } - - /// Presence arm — without it, a `skip_serializing_if` that dropped the - /// field unconditionally would pass the test above. - #[test] - fn warnings_are_serialized_when_present() { - let json = serde_json::to_string(&CreateAgentResponse { - node_id: 7, - warnings: vec!["name is reserved".to_owned()], - }) - .unwrap(); - assert_eq!(json, r#"{"node_id":7,"warnings":["name is reserved"]}"#); - } -}