From 5202e5c5baec931146288c27442e0355c69d1a2c Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 27 Aug 2026 11:02:35 +0200 Subject: [PATCH 1/3] types: reserve the protocol names an agent must not be called An agent's name was checked for shape and never for meaning: `Ident::parse` is 1-63 chars of [a-z0-9-] and there was no reserved-name list anywhere in the tree. So an agent could be called `operator`, `forge` or `todo` -- names the message layer already produces as a sender -- and a wake from that component became indistinguishable, at the broker, from a message sent by the agent. Adds `RESERVED_NAMES` + `is_reserved_name` to `hive-types`, the zero-dep leaf both `hive-c0re` and `swarm-controller` already depend on, so neither grows a dependency to use it. Every entry is a value some component actually produces as a message `from`/`to`, taken from `hive-sh4re`'s own sentinel constants rather than guessed: operator, system, reminder, forge, scheduled, todo, compact, graceful-stop. Two sentinels are deliberately absent -- `` and `` are unreachable as agent names because the charset rejects them, and `ruth` is a real agent, so wanting that name is a name being *taken*, which the roster answers. 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 is used for today. `create_agent` now warns on both halves -- a reserved name, and a name that is also a hive in the roster -- and does not refuse. The warnings ride on `CreateAgentResponse` rather than only the daemon's log, because the person who can still fix the name in one keystroke is holding the response, not reading the journal. `skip_serializing_if` keeps the no-warning JSON byte-identical to before, so this is a non-breaking first step toward refusing later. `hive-sh4re` gains a drift test tying its sentinel constants to the list: two crates that cannot import each other's intent now fail loudly if a sentinel is added without being reserved. Mutation-verified -- forcing the predicate false, forcing it true, and dropping a single entry each turn a different test red. --- hive-sh4re/src/manager.rs | 49 ++++++++++++++++++++ hive-types/src/lib.rs | 86 +++++++++++++++++++++++++++++++++++- swarm-controller/src/main.rs | 82 +++++++++++++++++++++++++++++++++- 3 files changed, 215 insertions(+), 2 deletions(-) diff --git a/hive-sh4re/src/manager.rs b/hive-sh4re/src/manager.rs index dcfd72e8..02ef7b44 100644 --- a/hive-sh4re/src/manager.rs +++ b/hive-sh4re/src/manager.rs @@ -131,3 +131,52 @@ 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, is_reserved_name}; + + /// The sentinels declared here and the reserved-name list in + /// `hive-types` are two spellings of one fact, in crates 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() { + for sentinel in [OPERATOR_RECIPIENT, SYSTEM_SENDER] { + assert!( + is_reserved_name(sentinel), + "{sentinel:?} is a sentinel an agent could be named — it must be in RESERVED_NAMES" + ); + } + } + + /// 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() { + 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)); + } + } + + /// `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() { + assert!(Ident::parse(MANAGER_AGENT).is_ok()); + assert!(!is_reserved_name(MANAGER_AGENT)); + } +} diff --git a/hive-types/src/lib.rs b/hive-types/src/lib.rs index 40d04857..0abb2037 100644 --- a/hive-types/src/lib.rs +++ b/hive-types/src/lib.rs @@ -6,6 +6,62 @@ //! and get serde-validated parsing at the socket boundary for free — with //! no cross-crate coupling and without growing `hive-sh4re`. +/// Names that already mean something to the message layer, and so are not +/// available as an agent name. +/// +/// Every entry is a value some component *produces* as a message `from` or +/// `to`, not a word that merely looked risky. An agent holding one of these +/// is indistinguishable, at the broker, from the thing that normally sends +/// it: a wake from `forge` and a wake from an agent named `forge` are the +/// same row. +/// +/// 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. +pub const RESERVED_NAMES: &[&str] = &[ + // The human at the dashboard. Both a broker recipient (the T4LK box + // sends `{from: "operator", to, body}`) 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. Named in `hive_sh4re::manager::SYSTEM_SENDER`. + "system", + // A due self-scheduled reminder arrives as its own sender, so that 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", +]; + +// Two sentinels are deliberately absent. `` and `` are +// routing recipients that `Ident::parse` already rejects on charset, so no +// name can ever equal them — listing them would imply a guard that never +// fires. And `ruth` (the manager) is a real agent, not a literal: a second +// agent wanting that name is a name that is *taken*, which is the roster +// check's job, not this list's. `hive-sh4re` pins both claims in a test. + +/// Whether `name` is already a protocol literal — see [`RESERVED_NAMES`]. +/// +/// 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) -> bool { + RESERVED_NAMES.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 @@ -98,7 +154,7 @@ impl<'de> serde::Deserialize<'de> for Ident { #[cfg(test)] mod ident_tests { - use super::Ident; + use super::{Ident, RESERVED_NAMES, is_reserved_name}; #[test] fn accepts_canonical_shapes() { @@ -135,6 +191,34 @@ mod ident_tests { } } + #[test] + fn reserved_names_are_flagged_and_ordinary_names_are_not() { + // Presence arm: every entry must actually be reported. + for name in RESERVED_NAMES { + assert!(is_reserved_name(name), "{name:?} should be reserved"); + } + // 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), "{ok:?} must NOT be reserved"); + } + } + + #[test] + fn every_reserved_name_is_a_valid_ident() { + // A reserved name that `Ident::parse` already rejects is dead + // weight — nothing could ever have been created with it, so + // listing it implies a guard that is doing nothing. `graceful-stop` + // is the one that makes this worth asserting: it is hyphenated, and + // a charset tightening would silently retire it. + for name in RESERVED_NAMES { + assert!( + Ident::parse(name).is_ok(), + "{name:?} is reserved but not a parseable ident — one of the two is wrong" + ); + } + } + #[test] fn round_trips_and_serde_validates() { let id = Ident::parse("damocles").unwrap(); diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 7b7290f1..f7106e0b 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -682,6 +682,20 @@ 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 @@ -736,9 +750,42 @@ 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(); + if hive_types::is_reserved_name(&agent) { + // 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. + let detail = 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" + ); + tracing::warn!(agent = %agent, "create_agent: reserved name"); + warnings.push(detail); + } + 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 @@ -798,7 +845,10 @@ async fn create_agent( let [id] = ids[..] else { unreachable!("exactly one handle was asked for"); }; - Ok(Json(CreateAgentResponse { node_id: id.get() })) + Ok(Json(CreateAgentResponse { + node_id: id.get(), + warnings, + })) } /// Query params for `GET /api/jobq/graph` — `?states=` narrows to root @@ -1597,3 +1647,33 @@ 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"]}"#); + } +} From 7bb68fe819987fff5975823be803d797313d14d6 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 27 Aug 2026 11:26:26 +0200 Subject: [PATCH 2/3] 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 From 27932ec631b847e8ea89d38e7243fff40370a785 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 27 Aug 2026 15:15:33 +0200 Subject: [PATCH 3/3] types: let nix own the reserved-name blacklist One list, in nix/reserved-names.nix, handed to everything that needs it as HIVE_RESERVED_NAMES. Keeping it current becomes a config change rather than a rebuild, and hive names and agent names -- one namespace going forward -- are checked against the same file: swarm-otel.nix's hand-written reservedOwners is gone. Whitespace-separated rather than JSON, deliberately, unlike the structured env vars beside it. Every entry is an Ident ([a-z0-9-]), so whitespace cannot occur inside a name and the encoding is provably lossless; JSON would mean either a parser dependency in a crate whose purpose is to have none, or a copy of the parse in every consumer. An UNSET variable is not "nothing is reserved". Both creation sites log an error and return a warning saying the check did not run, so a misconfigured deployment says so instead of silently accepting every name. A blank value folds into unset: nix always renders a non-empty list, so present-but-empty is a rendering fault, not a declaration. Two guards whose subject moved out of their own file now assert their own case is still in it, because a guard that can be retired by an edit elsewhere is not a guard: - swarm-otel.nix asserts reserved-names.nix still contains its swarmTierName. - hive-sh4re's sentinel drift test PANICS when the variable is missing rather than skipping -- a drift test that quietly does nothing still reports green. checks.nix and devshell.nix both export it so CI and a local cargo test agree. Verified as a pair: with the variable set, 8 tests pass; with it unset, exactly the 4 drift tests fail and the unrelated ones still pass. --- .../src/socket_server/config_approvals.rs | 35 +++-- hive-sh4re/src/manager.rs | 73 ++++++++-- hive-types/src/lib.rs | 129 ++++++++++-------- nix/checks.nix | 8 ++ nix/devshell.nix | 6 + nix/host-modules/hive-c0re/environment.nix | 15 ++ nix/host-modules/swarm-controller.nix | 7 + nix/host-modules/swarm-otel.nix | 30 +++- nix/reserved-names.nix | 67 +++++++++ swarm-controller/src/main.rs | 39 ++++-- 10 files changed, 324 insertions(+), 85 deletions(-) create mode 100644 nix/reserved-names.nix diff --git a/hive-c0re/src/socket_server/config_approvals.rs b/hive-c0re/src/socket_server/config_approvals.rs index 01ef9dff..abb28b64 100644 --- a/hive-c0re/src/socket_server/config_approvals.rs +++ b/hive-c0re/src/socket_server/config_approvals.rs @@ -38,14 +38,33 @@ pub(super) fn handle_request_init_config( // 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() + // + // 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, diff --git a/hive-sh4re/src/manager.rs b/hive-sh4re/src/manager.rs index 02ef7b44..a5cdf017 100644 --- a/hive-sh4re/src/manager.rs +++ b/hive-sh4re/src/manager.rs @@ -137,19 +137,68 @@ mod reserved_name_tests { use super::{ CHILDREN_RECIPIENT, MANAGER_AGENT, OPERATOR_RECIPIENT, PARENT_RECIPIENT, SYSTEM_SENDER, }; - use hive_types::{Ident, is_reserved_name}; + use hive_types::{Ident, RESERVED_NAMES_ENV, is_reserved_name}; - /// The sentinels declared here and the reserved-name list in - /// `hive-types` are two spellings of one fact, in crates 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. + /// 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), - "{sentinel:?} is a sentinel an agent could be named — it must be in RESERVED_NAMES" + 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" ); } } @@ -161,12 +210,14 @@ mod reserved_name_tests { /// 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)); + assert!(!is_reserved_name(sentinel, &reserved)); } } @@ -176,7 +227,9 @@ mod reserved_name_tests { /// 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)); + assert!(!is_reserved_name(MANAGER_AGENT, &reserved)); } } diff --git a/hive-types/src/lib.rs b/hive-types/src/lib.rs index 0abb2037..d1a51508 100644 --- a/hive-types/src/lib.rs +++ b/hive-types/src/lib.rs @@ -6,14 +6,14 @@ //! and get serde-validated parsing at the socket boundary for free — with //! no cross-crate coupling and without growing `hive-sh4re`. -/// Names that already mean something to the message layer, and so are not -/// available as an agent name. +/// The environment variable nix uses to hand this process the blacklist of +/// names that already mean something to the message layer. /// -/// Every entry is a value some component *produces* as a message `from` or -/// `to`, not a word that merely looked risky. An agent holding one of these -/// is indistinguishable, at the broker, from the thing that normally sends -/// it: a wake from `forge` and a wake from an agent named `forge` are the -/// same row. +/// **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 @@ -21,45 +21,47 @@ /// refusal — which is a stronger action than the warning this list is /// currently used for. Creation sites call [`is_reserved_name`]; readers do /// not. -pub const RESERVED_NAMES: &[&str] = &[ - // The human at the dashboard. Both a broker recipient (the T4LK box - // sends `{from: "operator", to, body}`) 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. Named in `hive_sh4re::manager::SYSTEM_SENDER`. - "system", - // A due self-scheduled reminder arrives as its own sender, so that 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", -]; +/// +/// ⚠️ 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"; -// Two sentinels are deliberately absent. `` and `` are -// routing recipients that `Ident::parse` already rejects on charset, so no -// name can ever equal them — listing them would imply a guard that never -// fires. And `ruth` (the manager) is a real agent, not a literal: a second -// agent wanting that name is a name that is *taken*, which is the roster -// check's job, not this list's. `hive-sh4re` pins both claims in a test. +/// 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() +} -/// Whether `name` is already a protocol literal — see [`RESERVED_NAMES`]. +/// 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) -> bool { - RESERVED_NAMES.contains(&name) +pub fn is_reserved_name(name: &str, reserved: &[&str]) -> bool { + reserved.contains(&name) } /// A validated hive identifier: 1-63 chars of `[a-z0-9-]`. @@ -154,7 +156,7 @@ impl<'de> serde::Deserialize<'de> for Ident { #[cfg(test)] mod ident_tests { - use super::{Ident, RESERVED_NAMES, is_reserved_name}; + use super::{Ident, is_reserved_name, parse_reserved_names}; #[test] fn accepts_canonical_shapes() { @@ -193,29 +195,48 @@ 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_NAMES { - assert!(is_reserved_name(name), "{name:?} should be reserved"); + 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), "{ok:?} must NOT be reserved"); + 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 every_reserved_name_is_a_valid_ident() { - // A reserved name that `Ident::parse` already rejects is dead - // weight — nothing could ever have been created with it, so - // listing it implies a guard that is doing nothing. `graceful-stop` - // is the one that makes this worth asserting: it is hyphenated, and - // a charset tightening would silently retire it. - for name in RESERVED_NAMES { - assert!( - Ident::parse(name).is_ok(), - "{name:?} is reserved but not a parseable ident — one of the two is wrong" - ); + 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"); } } diff --git a/nix/checks.nix b/nix/checks.nix index e9e01878..0944d240 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -62,6 +62,14 @@ 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 36c07af8..0d3400aa 100644 --- a/nix/devshell.nix +++ b/nix/devshell.nix @@ -5,6 +5,12 @@ { 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 92b612e8..c1df0c6e 100644 --- a/nix/host-modules/hive-c0re/environment.nix +++ b/nix/host-modules/hive-c0re/environment.nix @@ -143,6 +143,21 @@ 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 8a94855f..5ba54251 100644 --- a/nix/host-modules/swarm-controller.nix +++ b/nix/host-modules/swarm-controller.nix @@ -648,6 +648,13 @@ 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 4408ab49..9d5ebd00 100644 --- a/nix/host-modules/swarm-otel.nix +++ b/nix/host-modules/swarm-otel.nix @@ -55,9 +55,16 @@ 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 `` 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 ]; + # 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; # 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 @@ -581,6 +588,23 @@ 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 new file mode 100644 index 00000000..f7fbb6b2 --- /dev/null +++ b/nix/reserved-names.nix @@ -0,0 +1,67 @@ +# 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 f7106e0b..bad9edfe 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -758,16 +758,35 @@ async fn create_agent( // // Collected rather than logged-and-dropped — see `CreateAgentResponse`. let mut warnings = Vec::new(); - if hive_types::is_reserved_name(&agent) { - // 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. - let detail = 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" - ); - tracing::warn!(agent = %agent, "create_agent: reserved name"); - warnings.push(detail); + // 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)