diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index 7b726dfe..d1bcf646 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -31,11 +31,6 @@ use crate::webhook::DeliveryKind; /// The forge org that owns agent repos. Same org `hive-c0re::forge` /// already uses for its own single-hive `CreateRepo` path — this is /// the same forge instance, not a separate one, so the same org. -/// -/// There is no per-agent repo-vs-agent naming convention to maintain here: -/// agent names are unique and every agent's repo lives in this one org, so -/// the agent name IS the repo name at every call site (dropped the -/// `agent_repo` identity function this used to go through, per review). pub const AGENTS_ORG: &str = "agents"; /// The `operators` team, whitelisted for the merge gate on every repo @@ -491,14 +486,6 @@ 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 3c39cf79..090194e9 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -56,21 +56,15 @@ mod webhook; enum SwarmNodeKind { /// Ensure `agent` exists as an authelia subject at the swarm level. CreateIdentity { agent: String }, - /// Create the agent's repo in `forge::AGENTS_ORG` with the operator - /// merge gate on its default branch. See `forge::Client::create_repo`. - CreateRepo { agent: String }, - /// Add `agent` as a write collaborator on its own repo. See + /// Create `repo` in `forge::AGENTS_ORG` with the operator merge gate + /// on its default branch. See `forge::Client::create_repo`. + CreateRepo { repo: String }, + /// Add `agent` as a write collaborator on `repo`. See /// `forge::Client::add_repo_member`. - AddRepoMember { agent: String }, - /// Seed the agent's repo with `agent.nix` + `flake.nix`. See + AddRepoMember { repo: String, agent: String }, + /// Seed `repo` with `agent.nix` + `flake.nix`. See /// `forge::Client::seed_agent_config`. - /// - /// Deliberately carries no hive: seeding a config repo is the same - /// work whichever hive the agent is bound for, and an agent's config - /// states nothing about where it runs. The hive is an address the - /// swarm routes a deploy message to — it belongs on the node that - /// sends that message, not on this one. - InitAgentConfigRepo { agent: String }, + InitAgentConfigRepo { repo: String, agent: String }, } impl hive_jobq_wire::WireNode for SwarmNodeKind { @@ -84,18 +78,17 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind { } fn data(&self, _id: hive_jobq_wire::WireId) -> serde_json::Value { - // Every node in this graph is about exactly one agent, so they all - // render the same and `label()` is what distinguishes them. Spelled - // out as an or-pattern rather than a catch-all on purpose: a fifth - // variant then fails to compile here instead of silently rendering - // as an agent name. match self { - SwarmNodeKind::CreateIdentity { agent } - | SwarmNodeKind::CreateRepo { agent } - | SwarmNodeKind::AddRepoMember { agent } - | SwarmNodeKind::InitAgentConfigRepo { agent } => { + SwarmNodeKind::CreateIdentity { agent } => { serde_json::json!({ "agent": agent }) } + SwarmNodeKind::CreateRepo { repo } => { + serde_json::json!({ "repo": repo }) + } + SwarmNodeKind::AddRepoMember { repo, agent } + | SwarmNodeKind::InitAgentConfigRepo { repo, agent } => { + serde_json::json!({ "repo": repo, "agent": agent }) + } } } } @@ -149,13 +142,13 @@ async fn run_swarm_node( Err(e) => Outcome::Failed(format!("{e:#}")), }, }, - SwarmNodeKind::CreateRepo { agent } => match deps.forge { + SwarmNodeKind::CreateRepo { repo } => match deps.forge { None => Outcome::Failed( "no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \ SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)" .to_owned(), ), - Some(client) => match client.create_repo(&agent).await { + Some(client) => match client.create_repo(&repo).await { Ok(full_name) => { tracing::info!(%full_name, "swarm jobq: create_repo done"); Outcome::Done @@ -163,24 +156,24 @@ async fn run_swarm_node( Err(e) => Outcome::Failed(format!("{e:#}")), }, }, - SwarmNodeKind::AddRepoMember { agent } => match deps.forge { + SwarmNodeKind::AddRepoMember { repo, agent } => match deps.forge { None => Outcome::Failed( "no forge configured on this host (SWARM_CONTROLLER_FORGE_URL / \ SWARM_CONTROLLER_FORGE_TOKEN_FILE unset)" .to_owned(), ), - Some(client) => match client.add_repo_member(&agent, &agent).await { + Some(client) => match client.add_repo_member(&repo, &agent).await { Ok(()) => Outcome::Done, Err(e) => Outcome::Failed(format!("{e:#}")), }, }, - SwarmNodeKind::InitAgentConfigRepo { agent } => match deps.forge { + 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)" .to_owned(), ), - Some(client) => match client.seed_agent_config(&agent, &agent).await { + Some(client) => match client.seed_agent_config(&repo, &agent).await { Ok(()) => Outcome::Done, Err(e) => Outcome::Failed(format!("{e:#}")), }, @@ -554,34 +547,13 @@ async fn get_hives_status( } } -/// 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. -/// -/// Validated here and carried no further: none of the nodes this endpoint -/// queues talks to a hive, so none of them needs the address. It reaches -/// its consumer when the node that *sends* a deploy message exists, and -/// that node takes it from this field. Settling the request shape now is -/// the point — it is the breaking half, and doing it once is cheaper for -/// every caller than doing it again later. +/// 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. #[derive(Clone, Debug, Deserialize, ToSchema)] struct CreateAgentRequest { name: String, - hive: String, } /// Where the queued job landed — a caller polls `/api/jobq/graph` (or @@ -625,7 +597,7 @@ struct CreateAgentResponse { request_body = CreateAgentRequest, responses( (status = 200, description = "job chain queued", body = CreateAgentResponse), - (status = 400, description = "`name` or `hive` is not a valid identifier, or `hive` is not in this swarm (problem+json)", body = String), + (status = 400, description = "`name` is not a valid identifier (problem+json)", body = String), (status = 500, description = "the job chain could not be queued (problem+json)", body = String), ), tag = "agents" @@ -637,33 +609,7 @@ async fn create_agent( let agent = hive_types::Ident::parse(&req.name) .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? .into_string(); - - // 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 repo = agent.clone(); let mut sched = state .jobq @@ -675,20 +621,19 @@ async fn create_agent( agent: agent.clone(), }); let create_repo = b - .node(SwarmNodeKind::CreateRepo { - agent: agent.clone(), - }) + .node(SwarmNodeKind::CreateRepo { repo: repo.clone() }) .after_ok(create_identity); // Both fan out from `create_repo` directly — independent // operations on the same repo, no ordering requirement on // each other (see the doc comment above). let _add_repo_member = b .node(SwarmNodeKind::AddRepoMember { + repo: repo.clone(), agent: agent.clone(), }) .after_ok(create_repo); let _init_config = b - .node(SwarmNodeKind::InitAgentConfigRepo { agent }) + .node(SwarmNodeKind::InitAgentConfigRepo { repo, agent }) .after_ok(create_repo); vec![create_identity.guid()] }) @@ -1031,103 +976,6 @@ 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 { @@ -1142,7 +990,7 @@ mod tests { let id = sched .append( SwarmNodeKind::CreateRepo { - agent: "atlas".to_owned(), + repo: "atlas".to_owned(), }, Vec::new(), None,