From c7624cb41f7ca38d1956661b2e5aa30415c6130f Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Date: Sun, 26 Jul 2026 17:38:08 -0400 Subject: [PATCH] =?UTF-8?q?refactor(agents):=20remove=20the=20lazy=20spawn?= =?UTF-8?q?=20parameter=20=E2=80=94=20eager=20is=20structurally=20unrepres?= =?UTF-8?q?entable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round on the lazy flip (Wren + Max, independently): the source-contract test only scanned direct spawn_agent_child call sites, so mutating a start_pair caller from true to false reintroduced eager init for a real Desktop flow while the test stayed green — the guard was narrower than the invariant it claimed. Both reviewers converged on the same best fix: delete the boolean seam instead of widening the parser. spawn_agent_child now writes BUZZ_ACP_LAZY_POOL=true unconditionally (with the full rationale at the write site), and the lazy parameter is gone from spawn_agent_child and start_pair — there are no legitimate eager Desktop callers, so the regression Max mutated into existence can no longer be expressed. The 70-line window scanner shrinks to spawn_lazy_pool_env_is_unconditional: the env write must be exactly one unconditional literal. Mutation-verified: making the write conditional reds the test; restored green. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../src-tauri/src/managed_agents/restore.rs | 15 +-- .../src-tauri/src/managed_agents/runtime.rs | 25 ++--- .../src/managed_agents/runtime/tests.rs | 95 ++++++------------- .../src/managed_agents/runtime_commands.rs | 8 +- 4 files changed, 54 insertions(+), 89 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 191062015..5ec1a7dda 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -324,17 +324,18 @@ pub async fn restore_managed_agents_on_launch( } else { match super::terminate_untracked_pair_runtime(app, &key) .and_then(|()| { - // F1: restore spawns lazy, matching - // reconcile and manual start. Eager on - // restore buys nothing — a crashed - // mid-turn session is not resumed by an - // eager child — and silently reintroduces - // N idle brains on every launch. + // F1: all spawns are lazy — the + // parameter was removed once the + // last eager caller flipped. Eager + // on restore bought nothing — a + // crashed mid-turn session is not + // resumed by an eager child — and + // silently reintroduced N idle + // brains on every launch. spawn_agent_child( app, record, &key.relay_url, - true, owner_hex_ref, ) }) { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 94dd89ce8..7972bb1aa 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -427,7 +427,6 @@ pub fn spawn_agent_child( app: &AppHandle, record: &ManagedAgentRecord, relay_url: &str, - lazy: bool, owner_hex: Option<&str>, ) -> Result { if let Some(error) = spawn_key_refusal(record) { @@ -535,7 +534,18 @@ pub fn spawn_agent_child( command.env("RUST_LOG", child_rust_log_filter()); command.env("BUZZ_PRIVATE_KEY", &record.private_key_nsec); command.env("BUZZ_RELAY_URL", &effective_relay_url); - command.env("BUZZ_ACP_LAZY_POOL", if lazy { "true" } else { "false" }); + // ALWAYS lazy: the harness connects/subscribes to the relay first, + // captures the startup watermark immediately, queues accepted work, and + // initializes the agent pool in the cancellable wake task on first + // flushable work. Eager init (`false`) made buzz-acp serially spawn the + // FULL worker pool before connecting to the relay, so create/start of a + // slow-starting harness (e.g. a gateway-backed adapter at several + // seconds per worker) took minutes — and a mention sent in that window + // predated the watermark and could be missed permanently. There is + // deliberately no parameter for this: no Desktop flow may spawn eager + // (see the F1 note in restore.rs; `spawn_lazy_pool_env_is_unconditional` + // pins the env contract). + command.env("BUZZ_ACP_LAZY_POOL", "true"); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); match &resolved_mcp_command { @@ -953,16 +963,7 @@ pub fn start_managed_agent_process( // Scalar PIDs are migration-only and never establish pair liveness. record.runtime_pid = None; - // Lazy, matching manual start, restart, reconcile, and restore (see the - // F1 note in restore.rs). This was the last eager call site. Eager init - // ran the full serial pool spawn BEFORE the harness connected to the - // relay, so create/start of a slow-starting agent (e.g. a gateway-backed - // harness at several seconds per worker) took minutes — and a mention - // sent in that window predated the startup watermark and could be missed - // permanently. Lazy connects/subscribes first, queues accepted work, and - // initializes the pool in the cancellable wake task on first flushable - // work. - let mut process = spawn_agent_child(app, record, &key.relay_url, true, owner_hex)?; + let mut process = spawn_agent_child(app, record, &key.relay_url, owner_hex)?; let now = now_iso(); let receipt = super::ManagedAgentRuntimeReceipt { key: key.clone(), diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 1f2c9de8d..7515d4c8e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1053,72 +1053,37 @@ fn restart_eligible_false_when_non_orphan_has_no_drift() { assert!(!super::restart_eligible(false, false, false)); } -// ── every spawn_agent_child call site is lazy ──────────────────────── +// ── all Desktop spawns are lazy (BUZZ_ACP_LAZY_POOL) ───────────────── // -// The eager path (`lazy=false` → BUZZ_ACP_LAZY_POOL=false) makes buzz-acp -// serially initialize the FULL worker pool before connecting to the relay. -// For a slow-starting harness that is minutes of startup, and any mention -// sent in that window predates the startup watermark and can be missed -// permanently. Every Desktop flow (create/start, manual start, restart, -// reconcile, restore) must therefore spawn lazy: relay connects first, -// accepted work queues, and the pool initializes in the cancellable wake -// task. +// Eager pool init (`BUZZ_ACP_LAZY_POOL=false`) made buzz-acp serially +// initialize the FULL worker pool before connecting to the relay. For a +// slow-starting harness that is minutes of startup, and any mention sent +// in that window predates the startup watermark and can be missed +// permanently. The fix removed the `lazy` parameter entirely so eager +// spawning is structurally unrepresentable: `spawn_agent_child` writes +// the env var as an unconditional literal. // -// `spawn_agent_child` spawns a real OS process, so this contract is pinned -// at the source level: no call site may pass a literal `false` for the -// `lazy` parameter. If a new call site legitimately needs eager init, -// it must carry a documented exemption here. +// This test pins that contract at the source level (the function spawns +// a real OS process, so it cannot run under `cargo test`): the env write +// must be the unconditional literal `"true"` — no conditional, no +// parameter, no second write site. If a future change reintroduces an +// eager mode, it must be deliberate enough to rewrite this test. #[test] -fn no_spawn_agent_child_call_site_is_eager() { - let sources = [ - ( - "runtime.rs", - include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/managed_agents/runtime.rs" - )), - ), - ( - "runtime_commands.rs", - include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/managed_agents/runtime_commands.rs" - )), - ), - ( - "restore.rs", - include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/managed_agents/restore.rs" - )), - ), - ]; - for (name, source) in sources { - for (idx, line) in source.lines().enumerate() { - let trimmed = line.trim(); - // Skip comments; match only argument-position literal `false` - // in a spawn_agent_child call. The call spans multiple lines in - // restore.rs, so scan a small window after the call token. - if trimmed.starts_with("//") { - continue; - } - if trimmed.contains("spawn_agent_child(") { - let window: String = source - .lines() - .skip(idx) - .take(8) - .collect::>() - .join("\n"); - // The lazy flag is the 4th argument; a literal `false` in the - // window is the eager smell this test exists to catch. - assert!( - !window.contains("false"), - "{name}:{}: spawn_agent_child call site appears to pass \ - lazy=false (eager pool init). All Desktop spawns must be \ - lazy — see this test's doc comment. Call window:\n{window}", - idx + 1 - ); - } - } - } +fn spawn_lazy_pool_env_is_unconditional() { + let source = include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/src/managed_agents/runtime.rs" + )); + let writes: Vec<&str> = source + .lines() + .map(str::trim) + .filter(|line| line.contains("BUZZ_ACP_LAZY_POOL") && !line.starts_with("//")) + .collect(); + assert_eq!( + writes, + vec![r#"command.env("BUZZ_ACP_LAZY_POOL", "true");"#], + "BUZZ_ACP_LAZY_POOL must be written exactly once, as the \ + unconditional literal \"true\" — all Desktop spawns are lazy. \ + See this test's doc comment before changing." + ); } diff --git a/desktop/src-tauri/src/managed_agents/runtime_commands.rs b/desktop/src-tauri/src/managed_agents/runtime_commands.rs index c0e55184b..9c5354d57 100644 --- a/desktop/src-tauri/src/managed_agents/runtime_commands.rs +++ b/desktop/src-tauri/src/managed_agents/runtime_commands.rs @@ -224,7 +224,7 @@ pub(crate) fn start_managed_agent_runtime_pair_lazy( relay_url: String, app: AppHandle, ) -> Result { - start_pair(pubkey, relay_url, true, None, app) + start_pair(pubkey, relay_url, None, app) } #[tauri::command] @@ -239,7 +239,6 @@ pub fn start_managed_agent_runtime( fn start_pair( pubkey: String, relay_url: String, - lazy: bool, expected_updated_at: Option<&str>, app: AppHandle, ) -> Result { @@ -283,7 +282,7 @@ fn start_pair( .lock() .ok() .map(|keys| keys.public_key().to_hex()); - let mut process = spawn_agent_child(&app, record, &key.relay_url, lazy, owner.as_deref())?; + let mut process = spawn_agent_child(&app, record, &key.relay_url, owner.as_deref())?; let now = crate::util::now_iso(); let receipt = ManagedAgentRuntimeReceipt { key: key.clone(), @@ -383,7 +382,7 @@ pub fn restart_managed_agent_runtime( app: AppHandle, ) -> Result { stop_managed_agent_runtime(pubkey.clone(), relay_url.clone(), app.clone())?; - start_pair(pubkey, relay_url, true, None, app) + start_pair(pubkey, relay_url, None, app) } /// Probe whether this agent can operate on `requested_relay_url`. @@ -505,7 +504,6 @@ pub async fn reconcile_managed_agent_runtimes( match start_pair( record.pubkey.clone(), key.relay_url.clone(), - true, Some(&record.updated_at), app.clone(), ) {