diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 8460372abab..ccb07969468 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -505,7 +505,13 @@ impl AcpClient { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; } - if std::env::var_os(key).is_none() { + // BUZZ_RELAY_URL is authoritative: `Config` already resolved it + // with CLI-flag-over-parent-env precedence, so the value in + // `extra_env` must win over a missing AND a stale parent + // BUZZ_RELAY_URL. Without this, a child inheriting a stale parent + // env talks to the wrong relay ("404 no community for this host"). + // All other keys keep operator-wins semantics. + if key == "BUZZ_RELAY_URL" || std::env::var_os(key).is_none() { cmd.env(key, value); } } @@ -2971,6 +2977,43 @@ mod tests { ); } + /// The resolved relay URL (threaded in via `extra_env` by `Config`) is + /// authoritative for every child spawn: it must win when the parent env + /// has no BUZZ_RELAY_URL AND when the parent env carries a stale one. + /// Both scenarios run in one test so the process-global env mutation is + /// sequenced (matching the BUZZ_AUTH_TAG set_var precedent in lib.rs). + #[cfg(unix)] + #[tokio::test] + async fn spawn_asserts_resolved_relay_url_over_missing_and_stale_parent_env() { + const VAR: &str = "BUZZ_RELAY_URL"; + const RESOLVED: &str = "wss://relay.example.com"; + let saved = std::env::var_os(VAR); + + // Scenario 1 — unset parent env: child must see the resolved URL, + // not the ws://localhost:3000 default it would otherwise fall back to. + std::env::remove_var(VAR); + assert_eq!( + spawn_named_and_read_child_env("any-agent", VAR, &[(VAR.into(), RESOLVED.into())]) + .await, + RESOLVED, + "with no parent {VAR}, the child must receive the resolved relay URL" + ); + + // Scenario 2 — stale parent env: the resolved URL must still win. + std::env::set_var(VAR, "http://localhost:3000"); + assert_eq!( + spawn_named_and_read_child_env("any-agent", VAR, &[(VAR.into(), RESOLVED.into())]) + .await, + RESOLVED, + "a stale parent {VAR} must not shadow the resolved relay URL" + ); + + match saved { + Some(v) => std::env::set_var(VAR, v), + None => std::env::remove_var(VAR), + } + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 35aaec188db..bacfcacccb6 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -1047,6 +1047,27 @@ impl Config { let mut persona_env_vars = Vec::new(); let model = args.model; + // Reply-path preflight: a relay URL that cannot even be parsed can + // never deliver a reply. Fail terminally here (missing-config), before + // the harness connects, subscribes, or takes any work — a child that + // consumes a mention and then 404s on the relay loses that mention. + // Only the URL is printed; keys are never included. + if let Err(e) = Url::parse(&args.relay_url) { + return Err(ConfigError::ConfigFile(format!( + "missing-config: --relay-url / BUZZ_RELAY_URL '{}' is not a valid URL ({e})", + args.relay_url + ))); + } + + // The resolved relay URL is authoritative for every ACP child spawn. + // Inject it into the spawn env so children never fall back to a + // missing or stale parent BUZZ_RELAY_URL (which silently pointed them + // at the ws://localhost:3000 default → "404 no community for this + // host"). Unlike ordinary persona vars, `AcpClient::spawn` force-sets + // this key, so the parent environment cannot shadow it. clap has + // already given the CLI flag precedence over the parent env here. + persona_env_vars.push(("BUZZ_RELAY_URL".into(), args.relay_url.clone())); + // Inject CODEX_CONFIG so the @agentclientprotocol/codex-acp adapter (1.x) // opens the Seatbelt network sandbox for buzz-cli (an MCP subprocess). No-op // for non-Codex agents or unparseable relay URLs. @@ -2812,6 +2833,57 @@ channels = "ALL" ); } + // --- resolved relay URL: spawn-env injection + preflight --- + + #[test] + fn from_args_injects_resolved_relay_url_into_spawn_env() { + // The resolved --relay-url must land in persona_env_vars so every ACP + // child spawn carries it (AcpClient::spawn force-sets this key). + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--relay-url", + "wss://relay.example.com", + ]) + .expect("clap should parse args"); + let config = Config::from_args(args).expect("from_args should succeed"); + + assert_eq!(config.relay_url, "wss://relay.example.com"); + assert!( + config + .persona_env_vars + .iter() + .any(|(k, v)| k == "BUZZ_RELAY_URL" && v == "wss://relay.example.com"), + "persona_env_vars must carry the resolved relay URL; got {:?}", + config.persona_env_vars + ); + } + + #[test] + fn from_args_rejects_unparseable_relay_url_as_missing_config() { + // Reply-path preflight: an unresolvable relay URL must fail terminally + // before the harness can subscribe and consume mentions. + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--relay-url", + "", + ]) + .expect("clap should parse args"); + let err = Config::from_args(args).expect_err("empty relay URL must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("missing-config"), + "error should be terminal missing-config: {msg}" + ); + assert!( + !msg.contains(TEST_PRIVATE_KEY), + "preflight error must not leak secrets: {msg}" + ); + } + // --- max_turn_duration ceiling gate --- #[test] diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index fa348eeb3cf..3d1c464a86b 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -4244,6 +4244,38 @@ async fn initialize_agent_pool( Ok(AgentPool::from_slots(agent_slots)) } +#[cfg(test)] +mod pool_startup_tests { + use super::*; + use clap::Parser; + + /// Full chain: CLI `--relay-url` → `Config::from_args` → `PoolStartup:: + /// from_config` → the `extra_env` handed to every `AcpClient::spawn` in + /// `initialize_agent_pool` carries the resolved relay URL. + #[test] + fn pool_startup_extra_env_carries_resolved_relay_url() { + let args = config::CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + "0000000000000000000000000000000000000000000000000000000000000001", + "--relay-url", + "wss://relay.example.com", + ]) + .expect("clap should parse args"); + let config = Config::from_args(args).expect("from_args should succeed"); + let startup = PoolStartup::from_config(&config, None); + + assert!( + startup + .extra_env + .iter() + .any(|(k, v)| k == "BUZZ_RELAY_URL" && v == "wss://relay.example.com"), + "pool spawn env must carry the resolved relay URL; got {:?}", + startup.extra_env + ); + } +} + // ── spawn_and_init ──────────────────────────────────────────────────────────── /// Spawn an agent subprocess and run the MCP `initialize` handshake. ///