diff --git a/flake.nix b/flake.nix index 63818b00..8eef657c 100644 --- a/flake.nix +++ b/flake.nix @@ -149,20 +149,34 @@ nixosConfigurations = let - # These two configs are what hive-c0re extends per agent - # (meta.rs's `mkAgent` does `.extendModules { … }`, so - # the built base and every container stay on one version). - # Nothing deployment-specific may be set here: it would be - # inherited by every container on the hive and collide with - # the per-agent values hive-c0re renders. The agent modules - # are written so a bare evaluation needs no such values — - # service URLs default to `null`, meaning "not configured", - # and the units that would use them simply aren't generated. + # Values the agent modules require but that only a real + # deployment can know. Real containers are built from the + # generated meta flake, where hive-c0re renders these per + # agent from the host's `HIVE_FORGE_URL` (see meta.rs's + # `SERVICE_URL_OPTIONS`) — they never evaluate through + # `self.nixosConfigurations`, so nothing here can reach a + # running agent. These two configs exist only to typecheck + # the modules and to pre-build the container closure + # (`system.extraDependencies`, see hive-c0re/default.nix). + # + # Deliberately a `.invalid` host (RFC 2606: guaranteed not to + # resolve) rather than something plausible like a loopback + # port. If this value ever *did* escape into a runtime path, + # it must fail loudly at DNS instead of quietly connecting to + # whatever happens to be listening — which is the entire + # point of removing the `http://localhost:3000` default this + # replaces. + evalOnlyPlaceholders = { + hyperhive.forge.url = "http://forge.invalid"; + }; mkContainer = module: nixpkgs.lib.nixosSystem { system = "x86_64-linux"; - modules = [ module ]; + modules = [ + module + evalOnlyPlaceholders + ]; }; in { diff --git a/hive-c0re/src/dashboard/build_logs.rs b/hive-c0re/src/dashboard/build_logs.rs index 7c741a52..990af3ef 100644 --- a/hive-c0re/src/dashboard/build_logs.rs +++ b/hive-c0re/src/dashboard/build_logs.rs @@ -207,18 +207,6 @@ struct BuildLogFrame { /// always carries the full accumulated log so far (cursors start at 0); /// subsequent frames carry only new bytes. `done: true` on the final /// frame signals the browser to close the `EventSource`. -#[utoipa::path( - get, - path = "/api/build-logs/id/{id}/stream", - params(("id" = i64, Path, description = "build log row id")), - responses( - (status = 200, description = "server-sent event stream; each event's \ - `data` is a JSON-serialised `BuildLogFrame` \ - (stdout_append/stderr_append/status/done)", - body = String, content_type = "text/event-stream"), - ), - tag = "build_logs" -)] pub(super) async fn get_build_log_stream( State(state): State, AxumPath(id): AxumPath, diff --git a/hive-c0re/src/dashboard/mod.rs b/hive-c0re/src/dashboard/mod.rs index 9e2a12e6..5491e8cd 100644 --- a/hive-c0re/src/dashboard/mod.rs +++ b/hive-c0re/src/dashboard/mod.rs @@ -7,8 +7,10 @@ use std::sync::Arc; use anyhow::{Context, Result}; use axum::{ + Router, http::StatusCode, response::{IntoResponse, Response}, + routing::get, }; use utoipa::OpenApi; use utoipa_axum::{router::OpenApiRouter, routes}; @@ -118,12 +120,27 @@ pub async fn serve( ) -> Result<()> { // API-only: the gateway static-serves the dashboard dist and proxies // non-static requests here (see hive-gateway.nix). Unmatched paths 404. - // No static fallback needed here either. Every route (SSE streams - // included — `utoipa::path` can document an SSE body as an opaque - // `text/event-stream` string, it just can't model individual frame - // shapes) is registered below via `OpenApiRouter`; see the `router`/ - // `api` split. - // + // Every `#[utoipa::path]`-annotated route is registered further down + // via `OpenApiRouter` instead of a plain `.route(...)` call here — see + // the `router`/`api` split below. What's left in this plain chain is + // exactly the SSE/streaming endpoints utoipa can't model (see the + // `ApiDoc` doc comment) plus anything not yet annotated. + let app = Router::new() + .route( + "/api/build-logs/id/{id}/stream", + get(build_logs::get_build_log_stream), + ) + .route( + "/api/dashboard/stream", + get(state_snapshot::dashboard_stream), + ) + .route( + "/api/dashboard/history", + get(state_snapshot::dashboard_history), + ) + // No static fallback — the gateway owns the dist; unmatched paths 404. + ; + // `routes!()` folds every handler it's given into ONE shared // `MethodRouter` for the whole macro invocation — it does NOT group // by path internally, so passing it handlers for more than one @@ -202,9 +219,7 @@ pub async fn serve( .routes(routes!(questions::post_cancel_question)) .routes(routes!(tombstones::post_purge_tombstone)) .routes(routes!(meta_inputs::post_meta_update)) - .routes(routes!(build_logs::get_build_log_stream)) - .routes(routes!(state_snapshot::dashboard_stream)) - .routes(routes!(state_snapshot::dashboard_history)) + .merge(app.into()) .split_for_parts(); let app = router .merge(SwaggerUi::new("/api/docs").url("/api/openapi.json", api)) @@ -438,9 +453,6 @@ mod router_build_probe { .routes(routes!(questions::post_answer_question)) .routes(routes!(questions::post_cancel_question)) .routes(routes!(tombstones::post_purge_tombstone)) - .routes(routes!(meta_inputs::post_meta_update)) - .routes(routes!(build_logs::get_build_log_stream)) - .routes(routes!(state_snapshot::dashboard_stream)) - .routes(routes!(state_snapshot::dashboard_history)); + .routes(routes!(meta_inputs::post_meta_update)); } } diff --git a/hive-c0re/src/dashboard/state_snapshot.rs b/hive-c0re/src/dashboard/state_snapshot.rs index f86000cb..37006cc1 100644 --- a/hive-c0re/src/dashboard/state_snapshot.rs +++ b/hive-c0re/src/dashboard/state_snapshot.rs @@ -18,7 +18,6 @@ use hive_sh4re::Approval; use serde::{Deserialize, Serialize}; use tokio_stream::wrappers::BroadcastStream; use tokio_stream::{Stream, StreamExt}; -use utoipa::IntoParams; use crate::container_view::ContainerView; @@ -656,17 +655,6 @@ fn build_approval_views(approvals: Vec) -> Vec { out } -#[utoipa::path( - get, - path = "/api/dashboard/history", - responses( - (status = 200, description = "`{ seq, events }` — up to the last 200 \ - broker messages as `DashboardEvent::Sent`/`Delivered` JSON, plus \ - `seq`: the dashboard channel's high-water mark at fetch time \ - (used by clients to dedupe against buffered live SSE frames)"), - ), - tag = "state_snapshot" -)] pub(super) async fn dashboard_history(State(state): State) -> Response { // Backfill source for the dashboard terminal. Returns up to ~200 // historical broker messages (no other event kinds are persisted) @@ -743,7 +731,7 @@ pub(super) async fn dashboard_history(State(state): State) -> Response /// Useful for narrow pages (e.g. `flow.js` only cares about `sent` /// / `delivered` / `container_state_changed` / `container_removed`) /// that want to drop the dispatch overhead on every unrelated mutation. -#[derive(Deserialize, Default, IntoParams)] +#[derive(Deserialize, Default)] pub(super) struct DashboardStreamQuery { /// Comma-separated event kinds to forward. Each token is /// trimmed; unknown kinds are silently ignored on lookup @@ -751,18 +739,6 @@ pub(super) struct DashboardStreamQuery { kinds: Option, } -#[utoipa::path( - get, - path = "/api/dashboard/stream", - params(DashboardStreamQuery), - responses( - (status = 200, description = "server-sent event stream; each event's \ - `data` is a JSON-serialised `DashboardEvent` (seq-tagged; pair \ - with `/api/dashboard/history` to backfill + dedupe on connect)", - body = String, content_type = "text/event-stream"), - ), - tag = "state_snapshot" -)] pub(super) async fn dashboard_stream( State(state): State, axum::extract::Query(q): axum::extract::Query, diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 37ba30da..080982b9 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -85,11 +85,6 @@ async fn stage_generated_meta_files(dir: &std::path::Path) -> Result<()> { /// no-op. pub async fn sync_agents(hive: &HiveEnv, agents: &[AgentSpec]) -> Result<()> { let _guard = META_LOCK.lock().await; - // Before anything is written: a hive without a forge URL would deploy a - // whole fleet of agents that silently never log in, because the agent - // option treats "unset" as "no forge configured" rather than erroring. - // This is the layer that knows a forge is mandatory, so it says so here. - require_service_urls(&forwarded_env_vars())?; let dir = crate::paths::meta_root(); std::fs::create_dir_all(&dir).with_context(|| format!("create {}", dir.display()))?; @@ -720,12 +715,8 @@ const SERVICE_URL_OPTIONS: &[(&str, &str)] = &[ /// other under the default parallel test runner. A pure function over the /// already-collected pairs has no such hazard. /// -/// A var that isn't present emits nothing rather than a guess. For an optional -/// service that is the whole point — the agent option defaults to `null`, -/// meaning "not configured", and the units that would use it aren't generated. -/// For a service the hive cannot run without, silence would instead produce a -/// fleet of agents quietly missing an integration, so those are checked by -/// [`require_service_urls`] before this is called. +/// A var that isn't present emits nothing rather than a guess — see the call +/// site for why that silence is the point. fn push_service_url_options(out: &mut String, vars: &[(&'static str, String)]) { use std::fmt::Write as _; for (var, val) in vars { @@ -737,48 +728,6 @@ fn push_service_url_options(out: &mut String, vars: &[(&'static str, String)]) { } } -/// Service URLs a running hive must always supply, checked before rendering. -/// -/// The forge is not optional on a real hive: `hive-c0re.nix` sets -/// `HIVE_FORGE_URL` unconditionally, so its absence means this daemon was -/// started outside the NixOS module. The agent option is nullable — `null` -/// legitimately means "no forge" when the modules are evaluated on their own — -/// which is exactly why the hive has to assert its own requirement here rather -/// than leaning on the module to reject the empty case. -const REQUIRED_SERVICE_URL_VARS: &[&str] = &["HIVE_FORGE_URL"]; - -/// Fails unless every [`REQUIRED_SERVICE_URL_VARS`] entry is present in -/// `vars`. -/// -/// Pure over the already-collected pairs so it can be tested without touching -/// process env (the same reason [`push_service_url_options`] is split out). -/// -/// Checked by `sync_agents` — the point at which the hive commits a rendered -/// flake to disk — rather than inside the renderer. Rendering is a pure string -/// operation that many tests exercise directly; making *it* env-dependent -/// would mean every one of those tests either sets a process-wide var (the -/// parallel-test race this module already avoids) or fails for reasons that -/// have nothing to do with what it asserts. -/// -/// # Errors -/// -/// When a required var is missing. `hive-c0re.nix` sets it unconditionally, so -/// this means the daemon is running outside the NixOS module; refusing to -/// write the flake beats writing one whose agents would silently all lack a -/// forge. -fn require_service_urls(vars: &[(&'static str, String)]) -> Result<()> { - for required in REQUIRED_SERVICE_URL_VARS { - if !vars.iter().any(|(name, _)| name == required) { - anyhow::bail!( - "{required} is unset — hive-c0re.nix sets it unconditionally, so this process \ - was started outside the NixOS module. Refusing to write a meta flake whose \ - agents would every one of them have no forge configured." - ); - } - } - Ok(()) -} - fn forwarded_env_vars() -> Vec<(&'static str, String)> { FORWARDED_VARS .iter() @@ -1228,16 +1177,10 @@ where // ever correct when the callee shares the caller's netns, and the forge // and homeserver are moving to swarm level, possibly on other hosts. // - // Absent vars emit nothing rather than a guess, and the agent option - // treats "unset" as "this service is not configured" rather than - // substituting a loopback address — an absent integration instead of a - // misdirected one. - // - // That is the right default for an optional service and the wrong one for - // the forge, which a running hive always has — so `sync_agents` rejects a - // missing `HIVE_FORGE_URL` before it writes anything (`require_service_urls`). - // The check lives there rather than here because rendering is a pure string - // operation the tests exercise directly. + // Absent vars emit nothing rather than a guess. Once the defaults are + // gone that surfaces as an eval failure, which is the point: better a + // build that stops than an agent quietly talking to a port on the wrong + // machine. push_service_url_options(&mut out, &forwarded_env_vars()); // GitHub integration is on by default in every agent // (`hyperhive.github.enable`); the host turns it off hive-wide via @@ -1957,36 +1900,12 @@ mod tests { ); } - #[test] - fn require_service_urls_accepts_a_rendered_forge_url() { - require_service_urls(&[ - ("HIVE_FORGE_URL", "http://forge.example.test".to_string()), - ("HYPERHIVE_HIVE_NAME", "pr1ma".to_string()), - ]) - .expect("a forwarded forge URL satisfies the requirement"); - } - - #[test] - fn require_service_urls_refuses_to_write_without_a_forge() { - // The agent option is nullable, so nothing downstream would complain: - // every agent would simply come up with no forge login and no way to - // tell that was unintended. The hive asserts its own requirement - // because it is the only layer that knows a forge is mandatory. - let err = require_service_urls(&[("HYPERHIVE_HIVE_NAME", "pr1ma".to_string())]) - .expect_err("a missing forge URL must stop the write"); - assert!( - err.to_string().contains("HIVE_FORGE_URL is unset"), - "the error must name the missing variable: {err}" - ); - } - #[test] fn service_url_options_emit_nothing_when_absent() { - // No guess when the host doesn't say — the agent option stays `null` - // ("not configured") and the units that would use it aren't generated, - // so absence is an absent integration rather than a misdirected one. - // Required services don't reach here: `require_service_urls` rejects - // them first. + // No guess when the host doesn't say. Once the nix-side defaults are + // removed this is what turns a missing value into a build failure + // rather than an agent quietly talking to a port on the wrong machine, + // so the absence has to be as deliberate as the presence. let mut out = String::new(); push_service_url_options(&mut out, &[("HYPERHIVE_HIVE_NAME", "pr1ma".to_string())]); assert!( diff --git a/nix/agent-modules/forge.nix b/nix/agent-modules/forge.nix index a26cf295..0e4eb8e9 100644 --- a/nix/agent-modules/forge.nix +++ b/nix/agent-modules/forge.nix @@ -20,8 +20,7 @@ let in { options.hyperhive.forge.url = lib.mkOption { - type = lib.types.nullOr lib.types.str; - default = null; + type = lib.types.str; example = "http://forge.internal:3000"; description = '' Base URL of the hyperhive-managed Forgejo. Used at container @@ -32,38 +31,28 @@ in forge-token file is missing (i.e. hive-forge isn't running on the host). - **`null` means "no forge", not "guess one".** There is deliberately - no loopback default: the forge may run on a different host from - the agents, and inside an agent's network namespace `localhost` - reaches the agent rather than the forge, so a default would be a - value that builds fine and then talks to the wrong machine. - When this is `null` the tea-login and avatar-sync units are not - generated at all --- an absent integration, never a misdirected - one. - - On a real hive it is always set: hive-c0re renders it into every - agent's config from the host's `HIVE_FORGE_URL` (which - `hive-c0re.nix` sets unconditionally) and refuses to render a meta - flake without it, so `null` only survives where the modules are - evaluated outside a hive --- exactly the case that has no forge. + **Required, deliberately undefaulted.** hive-c0re renders it into + every agent's config from the host's `HIVE_FORGE_URL`, which + `hive-c0re.nix` sets unconditionally --- the forge is mandatory. + A loopback default would be a guess: the forge may run on a + different host from the agents, and inside an agent's network + namespace `localhost` reaches the agent, not the forge. An + unevaluatable config is better than one that builds and then + talks to the wrong machine. ''; }; config = { assertions = [ - # Only a *set* value is constrained. `null` is the legitimate - # "no forge here" state (see the option doc) and is handled by - # not generating the units below, so it must not trip this. - # The empty string, by contrast, is the one non-null value the - # type permits that cannot be a URL --- it is what a caller - # supplies when they have nothing, which is precisely what `null` - # is for, so reject it and name the option. + # The empty string is the one value the type permits that cannot + # be a URL, and it is what a caller supplies when they have + # nothing --- exactly the case the removed loopback default used + # to paper over. Reject it here so the failure names the option. { assertion = - config.hyperhive.forge.url == null - || lib.hasPrefix "http://" config.hyperhive.forge.url + lib.hasPrefix "http://" config.hyperhive.forge.url || lib.hasPrefix "https://" config.hyperhive.forge.url; - message = "hyperhive.forge.url must be an http:// or https:// URL, or null for no forge (got: \"${toString config.hyperhive.forge.url}\")"; + message = "hyperhive.forge.url must be an http:// or https:// URL (got: \"${config.hyperhive.forge.url}\")"; } ]; @@ -116,9 +105,7 @@ in # One-shot: tea config.yml from the seeded forge token. Shape # contract (always exit 0, no set -e, skip-silently, re-runnable): # docs/conventions.md::Best-effort oneshot services. - # Not generated at all when no forge is configured: an absent - # integration rather than one pointed at a guessed address. - systemd.services.tea-login = lib.mkIf (config.hyperhive.forge.url != null) { + systemd.services.tea-login = { description = "configure tea CLI from hive-forge token (best-effort)"; wantedBy = [ "multi-user.target" ]; after = [ "local-fs.target" ]; @@ -201,51 +188,49 @@ in # avatar sync), so the unit only exists when an icon is configured # and needs no librsvg at runtime — Forgejo's Go image library # can't decode SVG, hence PNG. - systemd.services.forge-avatar-sync = - lib.mkIf (config.hyperhive.icon != null && config.hyperhive.forge.url != null) - { - description = "sync agent icon to Forgejo user avatar (best-effort)"; - wantedBy = [ "multi-user.target" ]; - after = [ "tea-login.service" ]; - serviceConfig = { - Type = "oneshot"; - RemainAfterExit = false; - # Pin the journal identity (else it's the `script` store-path wrapper). - SyslogIdentifier = "forge-avatar-sync"; - }; - path = [ - pkgs.curl - pkgs.coreutils - pkgs.jq - ]; - script = '' - FORGE_URL=${lib.escapeShellArg config.hyperhive.forge.url} - # $HYPERHIVE_STATE_DIR is set system-wide by the meta flake - # (systemd.globalEnvironment) to `/agents//state`. - TOKEN_FILE="$HYPERHIVE_STATE_DIR/forge-token" - if [ ! -f "$TOKEN_FILE" ]; then - echo "forge-avatar-sync: no forge-token found; skipping" - exit 0 - fi - TOKEN=$(cat "$TOKEN_FILE") - IMAGE=$(base64 -w 0 < ${iconPng}) - # Forgejo POST /user/avatar expects {"image":""} — just the - # raw base64 string, NOT a data URI (data:image/png;base64,...). - # Use jq to build the payload so the large base64 value is safely quoted. - PAYLOAD=$(jq -n --arg img "$IMAGE" '{image:$img}') - RESP=$(curl -sf --max-time 10 \ - -X POST "$FORGE_URL/api/v1/user/avatar" \ - -H "Authorization: token $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$PAYLOAD" \ - -w "\n%{http_code}" 2>/dev/null || true) - CODE=$(printf '%s' "$RESP" | tail -1) - if [ "$CODE" = "204" ] || [ "$CODE" = "200" ]; then - echo "forge-avatar-sync: avatar uploaded (HTTP $CODE)" - else - echo "forge-avatar-sync: upload returned HTTP $CODE — skipping (non-fatal)" - fi - ''; - }; + systemd.services.forge-avatar-sync = lib.mkIf (config.hyperhive.icon != null) { + description = "sync agent icon to Forgejo user avatar (best-effort)"; + wantedBy = [ "multi-user.target" ]; + after = [ "tea-login.service" ]; + serviceConfig = { + Type = "oneshot"; + RemainAfterExit = false; + # Pin the journal identity (else it's the `script` store-path wrapper). + SyslogIdentifier = "forge-avatar-sync"; + }; + path = [ + pkgs.curl + pkgs.coreutils + pkgs.jq + ]; + script = '' + FORGE_URL=${lib.escapeShellArg config.hyperhive.forge.url} + # $HYPERHIVE_STATE_DIR is set system-wide by the meta flake + # (systemd.globalEnvironment) to `/agents//state`. + TOKEN_FILE="$HYPERHIVE_STATE_DIR/forge-token" + if [ ! -f "$TOKEN_FILE" ]; then + echo "forge-avatar-sync: no forge-token found; skipping" + exit 0 + fi + TOKEN=$(cat "$TOKEN_FILE") + IMAGE=$(base64 -w 0 < ${iconPng}) + # Forgejo POST /user/avatar expects {"image":""} — just the + # raw base64 string, NOT a data URI (data:image/png;base64,...). + # Use jq to build the payload so the large base64 value is safely quoted. + PAYLOAD=$(jq -n --arg img "$IMAGE" '{image:$img}') + RESP=$(curl -sf --max-time 10 \ + -X POST "$FORGE_URL/api/v1/user/avatar" \ + -H "Authorization: token $TOKEN" \ + -H "Content-Type: application/json" \ + -d "$PAYLOAD" \ + -w "\n%{http_code}" 2>/dev/null || true) + CODE=$(printf '%s' "$RESP" | tail -1) + if [ "$CODE" = "204" ] || [ "$CODE" = "200" ]; then + echo "forge-avatar-sync: avatar uploaded (HTTP $CODE)" + else + echo "forge-avatar-sync: upload returned HTTP $CODE — skipping (non-fatal)" + fi + ''; + }; }; }