Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 44 additions & 1 deletion crates/buzz-acp/src/acp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
Expand Down
72 changes: 72 additions & 0 deletions crates/buzz-acp/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand Down
32 changes: 32 additions & 0 deletions crates/buzz-acp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down