mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
refactor(agents): remove the lazy spawn parameter — eager is structurally unrepresentable
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 <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
d390b01039
commit
c7624cb41f
@@ -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,
|
||||
)
|
||||
}) {
|
||||
|
||||
@@ -427,7 +427,6 @@ pub fn spawn_agent_child(
|
||||
app: &AppHandle,
|
||||
record: &ManagedAgentRecord,
|
||||
relay_url: &str,
|
||||
lazy: bool,
|
||||
owner_hex: Option<&str>,
|
||||
) -> Result<crate::managed_agents::ManagedAgentProcess, String> {
|
||||
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(),
|
||||
|
||||
@@ -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::<Vec<_>>()
|
||||
.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."
|
||||
);
|
||||
}
|
||||
|
||||
@@ -224,7 +224,7 @@ pub(crate) fn start_managed_agent_runtime_pair_lazy(
|
||||
relay_url: String,
|
||||
app: AppHandle,
|
||||
) -> Result<ManagedAgentRuntimeStatus, String> {
|
||||
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<ManagedAgentRuntimeStatus, String> {
|
||||
@@ -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<ManagedAgentRuntimeStatus, String> {
|
||||
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(),
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user