diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index d1bcf646..e4675b06 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -486,6 +486,14 @@ fn base64_encode(content: &str) -> String { /// `hive-c0re::lifecycle::setup::initial_agent_nix` writes at the per-hive /// level (this process has no access to that function across the crate /// boundary, and it's three lines — not worth a shared crate for). +/// +/// Deliberately does **not** record which hive the agent belongs to, even +/// though `POST /api/agents` now takes one: an agent's config should carry +/// no reference to the hive it runs on. The hive is an address the swarm +/// routes on, not a property of the agent — a +/// config that named its own hive would be a second place stating where +/// the agent lives, free to disagree with the queue that actually +/// delivers to it. fn initial_agent_nix(name: &str) -> String { format!( "{{ config, pkgs, lib, ... }}:\n{{\n # Per-agent overrides for {name}. This is a regular NixOS module\n # — add packages, services, modules, imports as needed.\n #\n # imports = [ ./extra-module.nix ];\n # environment.systemPackages = with pkgs; [ ];\n}}\n", diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 090194e9..20a7c509 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -64,7 +64,25 @@ enum SwarmNodeKind { AddRepoMember { repo: String, agent: String }, /// Seed `repo` with `agent.nix` + `flake.nix`. See /// `forge::Client::seed_agent_config`. - InitAgentConfigRepo { repo: String, agent: String }, + /// + /// `hive` is the **address** this agent's creation was aimed at — the + /// hive a deploy message gets sent to over the queue — and not a + /// property of the agent. Seeding does not use it: it rides on the + /// graph because that is the only thing carrying the operator's choice + /// forward from the API boundary, where it was checked against the + /// roster once. The node that routes on it is the deploy node in the + /// deploy-coordination work; until that lands the field is carried and + /// rendered, not acted on. + /// + /// Carrying the validated value rather than re-reading the roster at + /// node time is deliberate — the roster is loaded at startup, and a + /// node that looked it up again would be answering a question the + /// operator already answered. + InitAgentConfigRepo { + repo: String, + agent: String, + hive: String, + }, } impl hive_jobq_wire::WireNode for SwarmNodeKind { @@ -85,10 +103,16 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind { SwarmNodeKind::CreateRepo { repo } => { serde_json::json!({ "repo": repo }) } - SwarmNodeKind::AddRepoMember { repo, agent } - | SwarmNodeKind::InitAgentConfigRepo { repo, agent } => { + SwarmNodeKind::AddRepoMember { repo, agent } => { serde_json::json!({ "repo": repo, "agent": agent }) } + // Rendered with `hive` because it is the one place the graph + // states where this creation was aimed — nothing persists it, + // so a viewer watching the job is the only reader there is + // until the deploy node routes on it. + SwarmNodeKind::InitAgentConfigRepo { repo, agent, hive } => { + serde_json::json!({ "repo": repo, "agent": agent, "hive": hive }) + } } } } @@ -167,7 +191,10 @@ async fn run_swarm_node( Err(e) => Outcome::Failed(format!("{e:#}")), }, }, - SwarmNodeKind::InitAgentConfigRepo { repo, agent } => match deps.forge { + // `hive` is deliberately not destructured: seeding writes no hive + // into the agent's config, so this node carries the address without + // consuming it. See the variant's doc. + SwarmNodeKind::InitAgentConfigRepo { repo, agent, .. } => match deps.forge { None => Outcome::Failed( "no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \ SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)" @@ -547,13 +574,27 @@ async fn get_hives_status( } } -/// Body of `POST /api/agents` — the agent name to create. The repo name -/// inside `forge::AGENTS_ORG` is the same string: one repo per agent, -/// named after it, same convention `hive-c0re::forge` already uses for its -/// own single-hive `CreateRepo` path. +/// Body of `POST /api/agents` — the agent name to create, and the hive the +/// creation is aimed at. The repo name inside `forge::AGENTS_ORG` is the +/// same string as `name`: one repo per agent, named after it, same +/// convention `hive-c0re::forge` already uses for its own single-hive +/// `CreateRepo` path. +/// +/// `hive` is an **address**, not an attribute of the agent: it is where a +/// deploy message goes over the queue. Nothing here writes it into the +/// agent's config — a config naming its own hive would be a second +/// statement of where the agent lives, free to drift from the queue that +/// actually delivers to it. +/// +/// It is required rather than optional because it is only knowable here, +/// at creation, from the operator making the choice. An optional field +/// would have been the friendlier migration and would have left every +/// creation in the state this endpoint exists to avoid — one nothing can +/// address. #[derive(Clone, Debug, Deserialize, ToSchema)] struct CreateAgentRequest { name: String, + hive: String, } /// Where the queued job landed — a caller polls `/api/jobq/graph` (or @@ -597,7 +638,7 @@ struct CreateAgentResponse { request_body = CreateAgentRequest, responses( (status = 200, description = "job chain queued", body = CreateAgentResponse), - (status = 400, description = "`name` is not a valid identifier (problem+json)", body = String), + (status = 400, description = "`name` or `hive` is not a valid identifier, or `hive` is not in this swarm (problem+json)", body = String), (status = 500, description = "the job chain could not be queued (problem+json)", body = String), ), tag = "agents" @@ -611,6 +652,33 @@ async fn create_agent( .into_string(); let repo = agent.clone(); + // Two checks, and the second is the one that makes the field worth + // having: `Ident::parse` says the string is *shaped* like a hive name, + // and the roster says it *is* one. Without the roster check a typo is + // accepted, the agent gets created, and nothing notices until a deploy + // message is addressed to a hive that does not exist — by which point + // the operator who could have corrected it in one keystroke is gone. + // + // `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. + let hive = hive_types::Ident::parse(&req.hive) + .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? + .into_string(); + 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 + // just picked one, and "not in this swarm" without the roster + // leaves them guessing at a typo they cannot see. + let known = if known.is_empty() { + "(none configured)".to_owned() + } else { + known.join(", ") + }; + let detail = format!("hive {hive:?} is not in this swarm — known hives: {known}"); + return Err(error_problem(axum::http::StatusCode::BAD_REQUEST, &detail)); + } + let mut sched = state .jobq .lock() @@ -633,7 +701,7 @@ async fn create_agent( }) .after_ok(create_repo); let _init_config = b - .node(SwarmNodeKind::InitAgentConfigRepo { repo, agent }) + .node(SwarmNodeKind::InitAgentConfigRepo { repo, agent, hive }) .after_ok(create_repo); vec![create_identity.guid()] }) @@ -976,6 +1044,103 @@ mod tests { /// SAFETY: single-threaded mutation of the two `forge` env vars this /// test itself owns, restored before returning — no other test in this /// crate reads them. + /// One roster, one agent name, two hives — the accepted one and a + /// typo. Built per test so neither can see the other's queue. + /// The scheduler handle these tests hold onto so they can assert on + /// what the endpoint queued. Aliased because the full type is three + /// nested generics deep and reads worse inline than named. + type SharedSched = std::sync::Arc< + std::sync::Mutex< + hive_jobq::scheduler::Scheduler, + >, + >; + + fn state_with_roster() -> (super::AppState, SharedSched) { + let sched = + std::sync::Arc::new(std::sync::Mutex::new(hive_jobq::scheduler::Scheduler::new( + hive_jobq::Graph::new(), + hive_jobq::resources::ResourceTable::new(), + ))); + let state = super::AppState { + hives: std::sync::Arc::new(vec![HiveEntry { + name: "pr1ma".to_owned(), + domain: "pr1ma.example".to_owned(), + }]), + links: std::sync::Arc::new(Vec::new()), + status: None, + jobq: std::sync::Arc::clone(&sched), + webhook_secret: None, + }; + (state, sched) + } + + /// The roster check is the half that makes the recorded hive worth + /// having, so assert it by EFFECT rather than by the message: a hive + /// that is not in this swarm must be refused **before anything is + /// queued**. A version that queued the graph and then complained would + /// satisfy an assertion on the status code alone while still creating + /// the agent — which is the failure this exists to stop. + #[tokio::test] + async fn a_hive_outside_the_roster_is_refused_before_anything_is_queued() { + let (state, sched) = state_with_roster(); + + let err = super::create_agent( + axum::extract::State(state), + axum::Json(super::CreateAgentRequest { + name: "atlas".to_owned(), + hive: "pr1maa".to_owned(), + }), + ) + .await + .expect_err("a hive outside the roster must be refused"); + + // Names what would have worked: an operator who just mistyped a + // hive cannot see the roster from the error otherwise. + let rendered = format!("{err:?}"); + assert!( + rendered.contains("pr1ma"), + "the refusal should name the known hives, got: {rendered}" + ); + + let queued = sched + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .graph() + .nodes() + .count(); + assert_eq!(queued, 0, "a refused creation must queue no work"); + } + + /// The control for the arm above: the same call with a hive that IS in + /// the roster gets through and queues the graph. Without this, "refused" + /// could equally mean the endpoint refuses everything. + #[tokio::test] + async fn a_hive_in_the_roster_is_accepted_and_queues_the_graph() { + let (state, sched) = state_with_roster(); + + // The response is bound rather than asserted on: node ids start at + // zero, so every property I reached for first ("> 0") was a claim + // about the id allocator rather than about this endpoint. What the + // accept arm is actually for is the queue count below. + let _queued = super::create_agent( + axum::extract::State(state), + axum::Json(super::CreateAgentRequest { + name: "atlas".to_owned(), + hive: "pr1ma".to_owned(), + }), + ) + .await + .expect("a hive in the roster must be accepted"); + + let queued = sched + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .graph() + .nodes() + .count(); + assert!(queued > 0, "an accepted creation must queue work"); + } + #[tokio::test] async fn create_repo_node_runs_end_to_end_and_fails_without_forge_configured() { unsafe {