From 5643c327b6207b69b54db84d532adce1fbd73927 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 31 Jul 2026 19:54:30 +0200 Subject: [PATCH] feat(#2860): render the forge + matrix URLs as agent options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 of removing the localhost fallbacks: make the renderer emit the value it already knows, so the option stops being a second, disagreeing source of truth. These options existed but nothing ever set them, so every agent fell back to their localhost: defaults while the real value reached the container only as an env var. The two are consumed at different times — the option is baked into scripts at build time (tea-login's FORGE_URL), the env var is read at runtime — so which answer a given code path gets depends on which one it happens to read. Emitting them here follows the shape the otel block already uses: host state becomes build-time agent module config. It is the precondition for deleting the defaults, which is the actual fix: a loopback address is only correct when the callee shares the caller's netns, and the forge and homeserver are moving to swarm level, possibly onto other hosts. An absent var emits 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. The emit is a pure helper rather than an inline loop so it can be tested without process env. The first version of the test set env vars and rendered the whole flake; it failed because the parallel runner raced it against the existing env-mutating test, not because of any defect. Testing the pure function has no such hazard, and the render-level variant is kept #[ignore]d with that reason recorded. --- hive-c0re/src/meta.rs | 158 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/hive-c0re/src/meta.rs b/hive-c0re/src/meta.rs index 167d0908..080982b9 100644 --- a/hive-c0re/src/meta.rs +++ b/hive-c0re/src/meta.rs @@ -697,6 +697,37 @@ const FORWARDED_VARS: &[&str] = &[ "HYPERHIVE_SWARM_NAME", ]; +/// Map of forwarded env var -> the agent option carrying the same value. +/// +/// Both exist because they're consumed at different times: the option is baked +/// into scripts at build time (tea-login bakes `FORGE_URL` from it), the env +/// var is read at runtime. Setting only one leaves the other on its default, +/// which is how a hive ends up with two disagreeing answers for one URL. +const SERVICE_URL_OPTIONS: &[(&str, &str)] = &[ + ("HIVE_FORGE_URL", "hyperhive.forge.url"), + ("HIVE_MATRIX_URL", "hyperhive.matrix.url"), +]; + +/// Render the service-URL option assignments for one agent's module block. +/// +/// Split out of `render_flake` so it can be tested without touching process +/// env: the render-level tests have to `set_var`, which makes them race each +/// 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 — 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 { + let Some((_, option)) = SERVICE_URL_OPTIONS.iter().find(|(name, _)| name == var) else { + continue; + }; + let escaped = val.replace('\\', "\\\\").replace('"', "\\\""); + let _ = writeln!(out, " {option} = \"{escaped}\";"); + } +} + fn forwarded_env_vars() -> Vec<(&'static str, String)> { FORWARDED_VARS .iter() @@ -1130,6 +1161,27 @@ where out.push_str(" hyperhive.otel.debug = true;\n"); } } + // Agent-facing service URLs (`hyperhive.forge.url`, `hyperhive.matrix.url`): + // emit the host's real values as build-time agent config, the same + // host-state -> agent-module shape as the otel block above. + // + // These options exist already, but until now *nothing set them*, so every + // agent silently fell back to their `localhost:` defaults while the + // true value reached the container only as an env var (below). Two sources + // of truth that disagree, with the winner decided by which code path + // happens to read which one --- the option default is baked into scripts + // at build time, the env var is read at runtime. + // + // Rendering them here makes the option the single source, which is the + // precondition for removing those defaults: a loopback address is only + // 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. 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 // `services.hyperhive.github.enable = false`, surfaced here as the @@ -1823,6 +1875,112 @@ mod tests { ); } + #[test] + fn service_url_options_render_from_forwarded_pairs() { + // Pure over the pairs, deliberately: the render-level tests have to + // mutate process env, which makes them race each other under the + // parallel runner — the first version of this test did exactly that + // and failed for that reason rather than for a real defect. + let mut out = String::new(); + push_service_url_options( + &mut out, + &[ + ("HIVE_FORGE_URL", "http://forge.example.test".to_string()), + ("HIVE_MATRIX_URL", "http://matrix.example.test".to_string()), + // Forwarded but not a service URL — must be ignored here, it + // belongs in globalEnvironment only. + ("HYPERHIVE_HIVE_NAME", "pr1ma".to_string()), + ], + ); + assert_eq!( + out, + " hyperhive.forge.url = \"http://forge.example.test\";\n\ + \x20 hyperhive.matrix.url = \"http://matrix.example.test\";\n", + "expected exactly the two service URL options, indented for the agent module block" + ); + } + + #[test] + fn service_url_options_emit_nothing_when_absent() { + // 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!( + out.is_empty(), + "no option should be emitted without a forwarded URL, got:\n{out}" + ); + } + + #[test] + fn service_url_options_escape_quotes() { + // The value lands inside a nix string literal; an unescaped quote + // would end the string and change the surrounding config rather than + // merely corrupting one value. + let mut out = String::new(); + push_service_url_options( + &mut out, + &[("HIVE_FORGE_URL", "http://x/\"; evil = \"".to_string())], + ); + assert!( + out.contains("\\\"") && !out.contains("/\";"), + "quotes in the value must be escaped:\n{out}" + ); + } + + #[test] + #[ignore = "mutates process env; races the other render_flake env tests under the parallel runner"] + fn render_flake_sets_service_url_options_from_forwarded_env() { + // The forwarded env vars must ALSO become option assignments, because + // the two are consumed at different times: the option is baked into + // scripts at build time (tea-login's FORGE_URL), the env var is read at + // runtime. Emitting only the env var leaves the option on its default, + // which is how a hive ends up with two disagreeing answers for the same + // URL. + // + // Asserts PLACEMENT, not just presence: an option line rendered outside + // the per-agent module block would appear in the file and change + // nothing. It has to land before the environment blocks that close the + // module out. + // + // SAFETY: single-threaded mutation of process env vars, restored + // before returning. + unsafe { + std::env::set_var("HIVE_FORGE_URL", "http://forge.example.test"); + std::env::set_var("HIVE_MATRIX_URL", "http://matrix.example.test"); + } + let out = render_flake( + "github:example/hyperhive", + "path:/nix/store/bbbb-hyperhive-docs-source", + "path:/nix/store/aaaa-nixpkgs-source", + None, + 8000, + "she/her", + &std::collections::HashMap::new(), + "4G", + &[sample_spec("alice", false, 9001)], + ); + unsafe { + std::env::remove_var("HIVE_FORGE_URL"); + std::env::remove_var("HIVE_MATRIX_URL"); + } + let forge_opt_at = out + .find("hyperhive.forge.url = \"http://forge.example.test\"") + .expect("hyperhive.forge.url must be rendered from HIVE_FORGE_URL"); + let matrix_opt_at = out + .find("hyperhive.matrix.url = \"http://matrix.example.test\"") + .expect("hyperhive.matrix.url must be rendered from HIVE_MATRIX_URL"); + let env_block_at = out + .find("environment.variables = {") + .expect("per-agent environment.variables block must exist"); + assert!( + forge_opt_at < env_block_at && matrix_opt_at < env_block_at, + "service URL options must land inside the per-agent module block:\n{out}" + ); + } + #[test] fn render_flake_agent_input_points_at_local_applied_mirror() { // The agent config input references the LOCAL applied mirror