revert(acp): remove dead GOOSE_ACP_SCHEDULER_DISABLED env injection (#3576)

## 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 <pfleger.will@gmail.com>
This commit is contained in:
Will Pfleger
2026-07-29 13:08:32 -04:00
committed by GitHub
parent 4a1ebf25c7
commit ddd468723a
-85
View File
@@ -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. `<unset>` means the child did not receive the var.
async fn spawn_and_read_child_env(
vars: &[&str],
extra_env: &[(String, String)],
) -> Vec<String> {
let script = vars
.iter()
.map(|var| format!("printf '%s\\n' \"${{{var}:-<unset>}}\""))
.collect::<Vec<_>>()
.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.
/// `<unset>` 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;