mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(acp): disable goose cron scheduler in managed agent children (#3144)
A Buzz install with a scheduled goose recipe fires each cron entry once per `goose acp` child instead of once, because every child unconditionally starts its own cron scheduler over the shared `~/.local/share/goose/schedule.json`. With a pool of N children per harness and multiple harnesses, one scheduled recipe fans out to N × harness_count executions — each running under the managed agent's identity rather than the operator's, and racing the operator's own standalone goose over the same schedule file. This injects `GOOSE_ACP_SCHEDULER_DISABLED=true` into every child spawned by `AcpClient::spawn`, so a managed agent never owns the operator's cron schedule. ## Placement The `cmd.env` call is set last — after the `extra_env` operator-wins loop and after the `CODEX_CONFIG` merge — deliberately with no escape hatch. Managed children not running the operator's schedule is a correctness invariant rather than an operator-tunable default, so the injection must beat both a conflicting persona `extra_env` entry and any value inherited from the parent process. It is injected for all agents, not just goose. Agent builds that don't recognize the variable ignore it. ## Sequencing The goose-side flag that reads this variable and skips scheduler startup lands separately (repo TBD). Until it does, this change is a forward-compatible no-op: it sets an environment variable nothing currently reads. Merging it first means no coordinated release is needed — the fix takes effect as soon as the goose side ships. Related: https://github.com/aaif-goose/goose/pull/10738 Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -20,6 +20,10 @@ 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.
|
||||
@@ -460,6 +464,16 @@ 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).
|
||||
@@ -2653,6 +2667,77 @@ 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"
|
||||
);
|
||||
}
|
||||
|
||||
/// 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;
|
||||
|
||||
Reference in New Issue
Block a user