From ddd468723ac1f0663abd19d23ca3484b2dd40bf5 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Wed, 29 Jul 2026 13:08:32 -0400 Subject: [PATCH] revert(acp): remove dead GOOSE_ACP_SCHEDULER_DISABLED env injection (#3576) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary [block/buzz#3144](https://github.com/block/buzz/pull/3144) injected `GOOSE_ACP_SCHEDULER_DISABLED=true` into every `AcpClient::spawn` call as a forward-compatible no-op, intended to suppress the cron scheduler in goose ACP children once the matching reader landed in goose. That reader only ever existed in [aaif-goose/goose#10738](https://github.com/aaif-goose/goose/pull/10738), which was closed unmerged. [goose#10781](https://github.com/aaif-goose/goose/pull/10781) (Lifei Zhou, merged 2026-07-29) disables the ACP scheduler by default at the source: `goose acp` now requires `--enable-scheduler` to start a scheduler. Buzz-spawned children therefore get no scheduler with zero configuration — making the `GOOSE_ACP_SCHEDULER_DISABLED` injection permanently dead code. ## What changes Removes from `crates/buzz-acp/src/acp.rs`: - `GOOSE_SCHEDULER_DISABLED_ENV` constant - `cmd.env(GOOSE_SCHEDULER_DISABLED_ENV, "true")` injection in `AcpClient::spawn` - `spawn_injects_scheduler_disabled_env_by_default` test - `spawn_scheduler_disabled_env_overrides_conflicting_extra_env` test - `spawn_and_read_child_env` helper (unreferenced once the two tests above are gone) No other files are affected. ## Why now Leaving dead code that references an env var no reader will ever consume misleads future maintainers about the actual scheduler-isolation mechanism. The isolation is now an upstream default, not a Buzz injection. Reverts: [block/buzz#3144](https://github.com/block/buzz/pull/3144) Related: [aaif-goose/goose#10781](https://github.com/aaif-goose/goose/pull/10781) Signed-off-by: Will Pfleger --- crates/buzz-acp/src/acp.rs | 85 -------------------------------------- 1 file changed, 85 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 351458051..8a698954a 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -20,10 +20,6 @@ use crate::usage::{TurnUsage, UsageTracker}; /// Lines exceeding this limit are rejected to prevent OOM from rogue agents. const MAX_LINE_SIZE: usize = 10_000_000; // 10 MB -/// Env var that tells a goose ACP child not to start its cron scheduler. -/// Injected unconditionally by [`AcpClient::spawn`]; see the call site for why. -pub(crate) const GOOSE_SCHEDULER_DISABLED_ENV: &str = "GOOSE_ACP_SCHEDULER_DISABLED"; - /// An MCP server configuration passed to `session/new`. /// /// Corresponds to the `McpServerStdio` variant in the ACP schema. @@ -517,16 +513,6 @@ impl AcpClient { cmd.env("CODEX_CONFIG", merged); } - // Buzz-managed agents must never execute the operator's personal cron - // schedule. A goose ACP child starts a scheduler over the shared - // `schedule.json`, so a pool of N children fires every scheduled job N - // times — under the wrong identity and racing standalone goose. - // - // Set last, and with no operator-wins escape hatch, so it beats both a - // conflicting persona `extra_env` entry and any inherited parent value. - // Agent builds that don't recognize the variable ignore it. - cmd.env(GOOSE_SCHEDULER_DISABLED_ENV, "true"); - // Spawn the agent in its own process group so SIGKILL doesn't propagate // to the harness's own process group on Unix. // tokio::process::Command::process_group is a stable tokio API (no extra imports needed). @@ -2866,46 +2852,6 @@ mod tests { .expect("failed to spawn test script") } - /// Spawn a script that echoes the named env vars as the child observes - /// them, one per line. `` means the child did not receive the var. - async fn spawn_and_read_child_env( - vars: &[&str], - extra_env: &[(String, String)], - ) -> Vec { - let script = vars - .iter() - .map(|var| format!("printf '%s\\n' \"${{{var}:-}}\"")) - .collect::>() - .join("\n"); - let mut client = AcpClient::spawn("bash", &["-c".into(), script], extra_env, false) - .await - .expect("failed to spawn env probe script"); - let mut observed = Vec::with_capacity(vars.len()); - for var in vars { - observed.push( - client - .reader - .next() - .await - .unwrap_or_else(|| panic!("child produced no output for {var}")) - .expect("child stdout was not readable"), - ); - } - observed - } - - /// Every spawned agent must be told not to run the operator's cron - /// schedule, without the caller having to opt in. - #[tokio::test] - async fn spawn_injects_scheduler_disabled_env_by_default() { - let observed = spawn_and_read_child_env(&[GOOSE_SCHEDULER_DISABLED_ENV], &[]).await; - assert_eq!( - observed, - vec!["true"], - "{GOOSE_SCHEDULER_DISABLED_ENV} must be injected into every spawn" - ); - } - /// Spawn a probe script whose file name carries a runtime identity (e.g. /// `hermes-acp`) and return the value of `var` as the child observed it. /// `` means the child did not receive the var. @@ -2978,37 +2924,6 @@ mod tests { ); } - /// Persona config must not be able to re-enable the scheduler: this is a - /// correctness invariant, not an operator-tunable default, so the - /// injection is set after (and therefore wins over) the `extra_env` loop. - /// - /// The control var pins that `extra_env` really did reach the child, so a - /// pass here means the conflicting entry lost the fight rather than - /// `extra_env` being dropped wholesale. - #[tokio::test] - async fn spawn_scheduler_disabled_env_overrides_conflicting_extra_env() { - let extra_env = vec![ - ( - GOOSE_SCHEDULER_DISABLED_ENV.to_string(), - "false".to_string(), - ), - ( - "BUZZ_ENV_PROBE_CONTROL".to_string(), - "delivered".to_string(), - ), - ]; - let observed = spawn_and_read_child_env( - &[GOOSE_SCHEDULER_DISABLED_ENV, "BUZZ_ENV_PROBE_CONTROL"], - &extra_env, - ) - .await; - assert_eq!( - observed, - vec!["true", "delivered"], - "a persona extra_env entry must not override {GOOSE_SCHEDULER_DISABLED_ENV}" - ); - } - #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await;