diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index cb69f5e506..0d8c0eb4ec 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -233,7 +233,11 @@ const overrides = new Map([ // (if let Some(provider_update) = input.provider { record.provider = provider_update; }). // +8: harness_override thread-through in update_managed_agent so a deliberate // Custom pin routes to update_time_agent_command_override (comment + call). - ["src-tauri/src/commands/agent_models.rs", 1079], + // +54: setup-mode auto-restart in update_managed_agent (stop + respawn a + // live setup-listener process when the edit satisfies readiness) + the + // expanded doc contract. Load-bearing fix for the "needs configuration" + // forever-loop; queued to split with the discovery modules above. + ["src-tauri/src/commands/agent_models.rs", 1133], // draft-persistence predicate: submit-time `loadDraft` check + inline comment // + deps-array entry in submitMessage closes the never-persisted-boundary // defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index f54a1dc114..1425502461 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -13,7 +13,8 @@ use crate::{ build_managed_agent_summary, current_instance_id, default_agent_workdir, discovery_env_with_baked_floor, find_managed_agent_mut, known_acp_runtime, load_managed_agents, load_personas, managed_agent_avatar_url, missing_command_message, - normalize_agent_args, resolve_command, save_managed_agents, sync_managed_agent_processes, + normalize_agent_args, resolve_command, save_managed_agents, setup_mode_now_ready, + start_managed_agent_process, stop_managed_agent_process, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo, AgentModelsResponse, UpdateManagedAgentRequest, UpdateManagedAgentResponse, DEFAULT_ACP_COMMAND, }, @@ -786,9 +787,17 @@ async fn run_agent_models_command( /// Update mutable fields on an existing managed agent record. /// -/// Does NOT auto-restart the agent. Runtime config changes (system prompt, -/// parallelism, commands, toolsets) take effect on the next agent spawn. -/// Name changes are synced to the relay immediately via a kind:0 re-publish. +/// Does NOT auto-restart a healthy agent. Runtime config changes (system +/// prompt, parallelism, commands, toolsets) take effect on the next agent +/// spawn. Name changes are synced to the relay immediately via a kind:0 +/// re-publish. +/// +/// ONE exception: a live process that was spawned in setup mode (readiness +/// failed at spawn, so buzz-acp is a nudge-only setup listener) is stopped +/// and respawned when this edit makes the record pass readiness — a +/// setup-mode harness never re-evaluates its config, so without the respawn +/// the agent would nudge "needs configuration" forever after the user fixed +/// it. See `setup_mode_now_ready`. #[tauri::command] pub async fn update_managed_agent( input: UpdateManagedAgentRequest, @@ -910,6 +919,53 @@ pub async fn update_managed_agent( record.updated_at = now_iso(); + // ── Auto-restart out of setup mode ────────────────────────────────── + // + // A harness spawned with BUZZ_ACP_SETUP_PAYLOAD runs as a nudge-only + // setup listener and NEVER re-evaluates its config — readiness is only + // computed at spawn (see `spawn_agent_child`). Without this, a user + // who fixes the missing model/provider in Edit Agent keeps getting + // "needs configuration" nudges forever because the stale setup-mode + // process is still the one connected to the relay. + // + // So: when this edit makes a live setup-mode process ready, stop it + // and respawn in place. Best-effort — a respawn failure must not fail + // the save itself (the edit already landed); it is surfaced via + // `last_error` on the record like other spawn failures. + let was_setup_mode = runtimes + .get(&record.pubkey) + .is_some_and(|rt| rt.in_setup_mode); + if was_setup_mode { + let personas = load_personas(&app).unwrap_or_default(); + if setup_mode_now_ready(record, &personas, true) { + // Same owner-hex read as `commands::agents::workspace_owner_hex`; + // `state.keys` is acquired inside the store lock elsewhere too + // (see `retain_managed_agent_pending`), so ordering is safe. + let owner_hex = { + let keys = state.keys.lock().map_err(|e| e.to_string())?; + keys.public_key().to_hex() + }; + let restart = + stop_managed_agent_process(&app, record, &mut runtimes).and_then(|()| { + start_managed_agent_process(&app, record, &mut runtimes, Some(&owner_hex)) + }); + match restart { + Ok(()) => eprintln!( + "buzz-desktop: agent {} config now satisfies readiness — \ + auto-restarted out of setup mode", + record.name + ), + Err(e) => { + eprintln!( + "buzz-desktop: auto-restart of agent {} out of setup mode failed: {e}", + record.name + ); + record.last_error = Some(format!("auto-restart failed: {e}")); + } + } + } + } + save_managed_agents(&app, &records)?; let record = records diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs index e253489291..b9d383a09d 100644 --- a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -134,6 +134,7 @@ pub fn finish_spawn( child: std::process::Child, log_path: std::path::PathBuf, spawn_config_hash: u64, + in_setup_mode: bool, agent_name: &str, ) -> super::ManagedAgentProcess { let job = create_job_for_child(child.id()); @@ -147,6 +148,7 @@ pub fn finish_spawn( child, log_path, spawn_config_hash, + in_setup_mode, job, } } diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0e12066961..efdc7f5449 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -1620,7 +1620,7 @@ pub fn spawn_agent_child( // // The JSON format mirrors `setup_mode::SetupPayload` in buzz-acp: // { "agent_name": "...", "agent_pubkey": "...", "requirements": [{ "surface": "...", ... }] } - { + let in_setup_mode = { use crate::managed_agents::{ agent_readiness, resolve_effective_agent_env, AgentReadiness, Requirement, }; @@ -1682,6 +1682,7 @@ pub fn spawn_agent_child( command.env_remove("BUZZ_ACP_SETUP_PAYLOAD"); // Set the payload only when desktop computed NotReady. + let in_setup_mode = setup_payload_json.is_some(); if let Some(json) = setup_payload_json { command.env("BUZZ_ACP_SETUP_PAYLOAD", json); eprintln!( @@ -1689,7 +1690,8 @@ pub fn spawn_agent_child( record.name ); } - } + in_setup_mode + }; // Only emit BUZZ_ACP_IDLE_TIMEOUT when the user has explicitly set an // override. When unset, the buzz-acp harness applies its own default // (see `DEFAULT_IDLE_TIMEOUT_SECS` in crates/buzz-acp/src/config.rs), @@ -1890,6 +1892,7 @@ pub fn spawn_agent_child( child, log_path, spawn_config_hash, + in_setup_mode, &record.name, )); #[cfg(not(windows))] @@ -1897,9 +1900,43 @@ pub fn spawn_agent_child( child, log_path, spawn_config_hash, + in_setup_mode, }) } +/// Returns `true` when a process spawned in setup mode would now pass the +/// readiness check under the record's current configuration — i.e. an edit +/// has satisfied the missing requirements and only a respawn will get the +/// agent out of the nudge-only setup listener (a setup-mode harness never +/// re-evaluates its config; readiness is only computed at spawn, see +/// `spawn_agent_child`). +/// +/// `was_spawned_in_setup_mode` is the flag stamped on +/// [`ManagedAgentProcess::in_setup_mode`] at spawn time. Passing the flag +/// (rather than the process) keeps this unit-testable without a live child. +pub fn setup_mode_now_ready( + record: &ManagedAgentRecord, + personas: &[crate::managed_agents::types::PersonaRecord], + was_spawned_in_setup_mode: bool, +) -> bool { + use crate::managed_agents::{agent_readiness, resolve_effective_agent_env, AgentReadiness}; + + if !was_spawned_in_setup_mode { + return false; + } + // Mirror the spawn-time readiness computation exactly: resolve the + // effective harness (persona wins unless explicitly pinned), then build + // the effective env the child would receive. + let effective_command = crate::managed_agents::effective_agent_command( + record.persona_id.as_deref(), + personas, + record.agent_command_override.as_deref(), + ); + let runtime_meta = known_acp_runtime(&effective_command); + let effective = resolve_effective_agent_env(record, personas, runtime_meta); + matches!(agent_readiness(&effective), AgentReadiness::Ready) +} + fn child_rust_log_filter() -> String { match std::env::var("RUST_LOG") { Ok(existing) if existing.contains("buzz_acp") => existing, diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index f675ed0a10..a61462fc73 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -608,3 +608,60 @@ fn grandchild_inherits_pgid_of_process_group_leader() { unsafe { libc::kill(-harness_pid, libc::SIGTERM) }; let _ = harness.wait(); } + +// ── setup_mode_now_ready tests ─────────────────────────────────────── +// +// The auto-restart gate in `update_managed_agent`: a live process spawned in +// setup mode should be respawned exactly when the edited record now passes +// readiness. These drive the pure seam (`setup_mode_now_ready`) with the +// spawn-time flag injected, so no live child process is needed. + +use super::setup_mode_now_ready; + +/// Record that passes buzz-agent readiness: provider + model + matching +/// credential. No persona and no override, so the effective command resolves +/// to the `buzz-agent` default. +fn ready_record() -> ManagedAgentRecord { + let mut record = fixture(RespondTo::Anyone, vec![], Some("tag".into())); + record.model = Some("claude-opus-4-5".into()); + record.provider = Some("anthropic".into()); + record + .env_vars + .insert("ANTHROPIC_API_KEY".into(), "sk-test".into()); + record +} + +#[test] +fn setup_mode_now_ready_true_when_config_fixed() { + // The Fizz bug: spawned in setup mode (model missing), then the user set + // model/provider in Edit Agent → the edit must trigger a respawn. + let record = ready_record(); + assert!(setup_mode_now_ready(&record, &[], true)); +} + +#[test] +fn setup_mode_now_ready_false_when_still_not_ready() { + // Model still missing → stay in setup mode; a respawn would just land + // back in the setup listener. + let mut record = ready_record(); + record.model = None; + assert!(!setup_mode_now_ready(&record, &[], true)); +} + +#[test] +fn setup_mode_now_ready_false_when_credential_still_missing() { + // Provider/model present but the provider credential is absent — the + // same NotReady the spawn path would compute. + let mut record = ready_record(); + record.env_vars.remove("ANTHROPIC_API_KEY"); + assert!(!setup_mode_now_ready(&record, &[], true)); +} + +#[test] +fn setup_mode_now_ready_false_for_normal_process() { + // A process NOT spawned in setup mode must never be auto-restarted by an + // edit — edits to healthy agents keep the existing "takes effect on next + // spawn" contract (the needs_restart badge covers drift). + let record = ready_record(); + assert!(!setup_mode_now_ready(&record, &[], false)); +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 3b152c0261..02d7b1886a 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -247,6 +247,13 @@ pub struct ManagedAgentProcess { /// `runtime_pid` have no `ManagedAgentProcess` entry, so their spawn /// config is unknown and the badge stays off. pub spawn_config_hash: u64, + /// `true` when this process was spawned with `BUZZ_ACP_SETUP_PAYLOAD` set — + /// i.e. readiness failed at spawn and buzz-acp is running as a nudge-only + /// setup listener instead of the agent pool. Runtime-only — never + /// persisted. `update_managed_agent` uses this to auto-restart the process + /// once an edit satisfies readiness, since a setup-mode harness never + /// re-evaluates its config on its own. + pub in_setup_mode: bool, /// Win32 Job Object owning the harness + its entire process tree. Closing /// the handle (via `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`) kills the whole /// tree — the Windows mirror of the Unix process-group teardown. `None`