diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 78db7ff718..d2b5cb8378 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -132,6 +132,56 @@ fn build_initialize_params() -> serde_json::Value { }) } +#[cfg(test)] +fn permission_option_id( + options: &[serde_json::Value], + approve: bool, +) -> Result<(&str, bool), AcpError> { + if approve { + if let Some(option) = options + .iter() + .find(|option| option.get("kind").and_then(|kind| kind.as_str()) == Some("allow_once")) + { + let option_id = option["optionId"] + .as_str() + .ok_or_else(|| AcpError::Protocol("allow_once option missing optionId".into()))?; + return Ok((option_id, true)); + } + } + + let option = options + .iter() + .find(|option| option.get("kind").and_then(|kind| kind.as_str()) == Some("reject_once")) + .ok_or_else(|| { + AcpError::Protocol("no reject_once option available for permission response".into()) + })?; + let option_id = option["optionId"] + .as_str() + .ok_or_else(|| AcpError::Protocol("reject_once option missing optionId".into()))?; + Ok((option_id, false)) +} + +fn permission_option_for_selection<'a>( + options: &'a [serde_json::Value], + selection: &crate::pool::PermissionSelection, +) -> Option<(&'a str, &'a str)> { + options.iter().find_map(|option| { + let option_id = option.get("optionId")?.as_str()?; + let kind = option.get("kind")?.as_str()?; + // Interactive managed-runtime consent is deliberately one-shot. + // Never forward allow_always/reject_always, even if an owner-signed + // control names an exact persistent option offered by the runtime. + if !matches!(kind, "allow_once" | "reject_once") { + return None; + } + let matches = match selection { + crate::pool::PermissionSelection::OptionId(selected) => option_id == selected, + crate::pool::PermissionSelection::Kind(selected) => kind == selected, + }; + matches.then_some((option_id, kind)) + }) +} + /// ACP client that owns an agent subprocess and communicates over its stdio. /// /// One `AcpClient` per agent process. Multiple sessions can be created on the @@ -158,6 +208,16 @@ pub struct AcpClient { /// Guards against double-response if a timeout fires after the allow_once /// response was written but before `pending_permission_id` was cleared. permission_responded: bool, + /// Whether ACP permission requests may be approved with `allow_once`. + /// + /// Buzz historically auto-approves these requests. Managed runtimes may + /// disable that fallback so a headless community prompt cannot silently + /// stand in for user consent. + auto_approve_permissions: bool, + /// Whether owner-signed observer controls may resolve permission requests. + interactive_permissions: bool, + /// Per-turn channel for owner permission decisions. + permission_rx: Option>, /// The JSON-RPC id of the most recently sent `session/prompt` request. /// Used by [`cancel_with_cleanup`] to drain the correct response. /// Set in [`session_prompt_with_idle_timeout`]; consumed in [`cancel_with_cleanup`]. @@ -488,6 +548,9 @@ impl AcpClient { next_id: 0, pending_permission_id: None, permission_responded: false, + auto_approve_permissions: true, + interactive_permissions: false, + permission_rx: None, last_prompt_id: None, current_hard_deadline: None, observer: None, @@ -505,6 +568,38 @@ impl AcpClient { self.observer_agent_index = Some(agent_index); } + /// Control whether `session/request_permission` may select `allow_once`. + /// + /// When disabled, the client selects `reject_once` and never falls back to + /// an allow option. This is set from the managed runtime launch policy at + /// the start of every prompt. + pub fn set_auto_approve_permissions(&mut self, enabled: bool) { + self.auto_approve_permissions = enabled; + } + + /// Control whether owner-signed observer controls may select an exact option + /// offered by the current permission request. + pub fn set_interactive_permissions(&mut self, enabled: bool) { + self.interactive_permissions = enabled; + } + + /// Install the permission-decision channel for one prompt turn. + pub fn install_permission_rx( + &mut self, + rx: tokio::sync::mpsc::Receiver, + ) { + debug_assert!( + self.permission_rx.is_none(), + "install_permission_rx: previous turn receiver was not cleared" + ); + self.permission_rx = Some(rx); + } + + /// Clear any permission receiver before the agent returns to the pool. + pub fn clear_permission_rx(&mut self) { + self.permission_rx = None; + } + /// Update metadata that will be attached to subsequent raw wire events. pub fn set_observer_context(&mut self, context: ObserverContext) { self.observer_context = context; @@ -706,6 +801,19 @@ impl AcpClient { self.current_hard_deadline = None; return Err(e); } + let prompt_dispatched_at = tokio::time::Instant::now(); + self.observe( + "prompt_dispatched", + serde_json::json!({ + "idleTimeoutSeconds": idle_timeout.as_secs(), + "maxDurationSeconds": max_duration.as_secs(), + }), + ); + tracing::info!( + idle_timeout_secs = idle_timeout.as_secs(), + max_duration_secs = max_duration.as_secs(), + "ACP prompt dispatched" + ); let result = self .read_until_response_with_idle_timeout( @@ -716,6 +824,19 @@ impl AcpClient { max_duration, ) .await; + let elapsed_ms = prompt_dispatched_at.elapsed().as_millis() as u64; + self.observe( + "prompt_wait_finished", + serde_json::json!({ + "elapsedMs": elapsed_ms, + "success": result.is_ok(), + }), + ); + tracing::info!( + elapsed_ms, + success = result.is_ok(), + "ACP prompt wait finished" + ); // On timeout errors, leave current_hard_deadline set so cancel_with_cleanup // can inherit the remaining budget. Clear it on all other outcomes. @@ -1224,6 +1345,7 @@ impl AcpClient { let mut idle_deadline = now + idle_timeout; let mut hard_deadline = hard_deadline; let mut last_activity_at = now; + let mut first_activity_observed = false; loop { // Determine which deadline fires first BEFORE sleeping — this is @@ -1411,6 +1533,15 @@ impl AcpClient { continue; } }; + if !first_activity_observed { + first_activity_observed = true; + let elapsed_ms = now.elapsed().as_millis() as u64; + self.observe( + "prompt_first_activity", + serde_json::json!({ "elapsedMs": elapsed_ms }), + ); + tracing::info!(elapsed_ms, "ACP prompt produced first activity"); + } self.observe("acp_read", msg.clone()); let activity_now = Instant::now(); @@ -1668,10 +1799,12 @@ impl AcpClient { } } - /// Auto-approve a `session/request_permission` request from the agent. + /// Respond to a `session/request_permission` request from the agent. /// - /// Finds the option with `kind == "allow_once"` and responds with its `optionId`. - /// If no `allow_once` option exists, falls back to `reject_once`. + /// When auto-approval is enabled, finds the option with + /// `kind == "allow_once"` and responds with its `optionId`. Otherwise it + /// selects `reject_once`. A missing allow option also falls back to + /// `reject_once`; a missing reject option is a protocol error. /// /// **Critical:** Never hardcode `optionId` — always find it dynamically by `kind`. /// @@ -1699,39 +1832,79 @@ impl AcpClient { options.len() ); - // Find allow_once by kind — NEVER hardcode optionId. - let allow_once = options - .iter() - .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("allow_once")); + // Auto-approval preserves the historical behavior for existing + // runtimes. Interactive mode waits only for an owner-signed control + // routed to this exact turn and request id. Any missing, stale, closed, + // or timed-out decision fails closed. + let selection = if self.auto_approve_permissions { + Some(crate::pool::PermissionSelection::Kind( + "allow_once".to_string(), + )) + } else if self.interactive_permissions { + self.wait_for_permission_decision(&id).await + } else { + None + }; - let response = if let Some(opt) = allow_once { - let option_id = opt["optionId"] - .as_str() - .ok_or_else(|| AcpError::Protocol("allow_once option missing optionId".into()))?; - tracing::info!( - target: "acp::permission", - "auto-approving permission id={id} with allow_once optionId={option_id:?}" - ); + // Owner controls carry an exact option ID. Validate it against this + // live request before returning it to the runtime. Any missing or + // unknown selection fails closed to reject_once. + let selected = selection + .as_ref() + .and_then(|selection| permission_option_for_selection(options, selection)); + let (option_id, kind) = match selected { + Some(selected) => selected, + None => { + if selection.is_some() { + tracing::warn!( + target: "acp::permission", + "permission selection did not match the current request — rejecting" + ); + } + permission_option_for_selection( + options, + &crate::pool::PermissionSelection::Kind("reject_once".to_string()), + ) + .ok_or_else(|| { + AcpError::Protocol( + "no reject_once option available for permission response".into(), + ) + })? + } + }; + let approved = !kind.starts_with("reject"); + + let response = if approved { + if self.auto_approve_permissions { + tracing::info!( + target: "acp::permission", + "auto-approving permission id={id} with {kind} optionId={option_id:?}" + ); + } else { + tracing::info!( + target: "acp::permission", + "owner selected permission id={id} with {kind} optionId={option_id:?}" + ); + } permission_response_selected(&id, option_id) } else { - // No allow_once — fall back to reject_once. - tracing::warn!( - target: "acp::permission", - "no allow_once option found in permission request id={id}, falling back to reject_once" - ); - let reject = options - .iter() - .find(|opt| opt.get("kind").and_then(|k| k.as_str()) == Some("reject_once")); - - if let Some(opt) = reject { - let option_id = opt["optionId"].as_str().unwrap_or("reject"); - permission_response_selected(&id, option_id) + if self.auto_approve_permissions && selection.is_some() { + tracing::warn!( + target: "acp::permission", + "no allow_once option found in permission request id={id}, falling back to reject_once" + ); + } else if self.interactive_permissions { + tracing::info!( + target: "acp::permission", + "owner rejected or did not resolve permission request id={id}" + ); } else { - return Err(AcpError::Protocol( - "no suitable permission option found (neither allow_once nor reject_once)" - .into(), - )); + tracing::info!( + target: "acp::permission", + "rejecting permission request id={id} because auto-approval is disabled" + ); } + permission_response_selected(&id, option_id) }; // Write the response first, then mark as responded. @@ -1754,6 +1927,58 @@ impl AcpClient { Ok(()) } + /// Wait for a decision that matches this ACP JSON-RPC request id. + /// + /// The receiver is scoped to one prompt turn. The hard turn deadline also + /// bounds this wait so an unattended prompt cannot stay parked forever. + async fn wait_for_permission_decision( + &mut self, + request_id: &serde_json::Value, + ) -> Option { + let Some(deadline) = self.current_hard_deadline else { + tracing::warn!( + target: "acp::permission", + "interactive permission request has no hard deadline — rejecting" + ); + return None; + }; + let Some(rx) = self.permission_rx.as_mut() else { + tracing::warn!( + target: "acp::permission", + "interactive permission request has no owner decision channel — rejecting" + ); + return None; + }; + + loop { + match tokio::time::timeout_at(deadline, rx.recv()).await { + Ok(Some(decision)) if decision.request_id == *request_id => { + return Some(decision.selection); + } + Ok(Some(_)) => { + tracing::debug!( + target: "acp::permission", + "ignoring permission decision for a different request id" + ); + } + Ok(None) => { + tracing::warn!( + target: "acp::permission", + "permission decision channel closed — rejecting" + ); + return None; + } + Err(_) => { + tracing::info!( + target: "acp::permission", + "interactive permission request timed out — rejecting" + ); + return None; + } + } + } + } + /// Parse `stopReason` from a `session/prompt` result value. fn parse_stop_reason(&self, result: &serde_json::Value) -> Result { let raw = result["stopReason"].as_str().ok_or_else(|| { @@ -2111,6 +2336,109 @@ mod tests { assert_eq!(reject_once.unwrap()["optionId"].as_str(), Some("rej-x")); } + #[test] + fn permission_policy_rejects_when_auto_approval_is_disabled() { + let options = serde_json::json!([ + {"optionId": "reject-this", "kind": "reject_once"}, + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + let (option_id, approved) = permission_option_id(options, false).unwrap(); + + assert_eq!(option_id, "reject-this"); + assert!(!approved); + } + + #[test] + fn permission_policy_preserves_existing_allow_once_behavior_when_enabled() { + let options = serde_json::json!([ + {"optionId": "reject-this", "kind": "reject_once"}, + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + let (option_id, approved) = permission_option_id(options, true).unwrap(); + + assert_eq!(option_id, "allow-this"); + assert!(approved); + } + + #[test] + fn permission_selection_resolves_exact_one_shot_option_id() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = crate::pool::PermissionSelection::OptionId("allow-once".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + Some(("allow-once", "allow_once")) + ); + } + + #[test] + fn permission_selection_rejects_persistent_options() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + { + "optionId": "allow-buzz-messages-in-workspace", + "kind": "allow_always" + }, + {"optionId": "reject-always", "kind": "reject_always"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + + for option_id in ["allow-buzz-messages-in-workspace", "reject-always"] { + let selection = crate::pool::PermissionSelection::OptionId(option_id.to_string()); + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None, + "{option_id} must not be actionable" + ); + } + } + + #[test] + fn permission_selection_by_kind_rejects_persistent_options() { + let options = serde_json::json!([ + {"optionId": "allow-always", "kind": "allow_always"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = crate::pool::PermissionSelection::Kind("allow_always".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None + ); + } + + #[test] + fn permission_selection_rejects_unknown_option_id() { + let options = serde_json::json!([ + {"optionId": "allow-once", "kind": "allow_once"}, + {"optionId": "reject-this", "kind": "reject_once"} + ]); + let selection = + crate::pool::PermissionSelection::OptionId("not-offered-by-agent".to_string()); + + assert_eq!( + permission_option_for_selection(options.as_array().unwrap(), &selection), + None + ); + } + + #[test] + fn permission_policy_never_allows_when_reject_is_unavailable() { + let options = serde_json::json!([ + {"optionId": "allow-this", "kind": "allow_once"} + ]); + let options = options.as_array().unwrap(); + + assert!(permission_option_id(options, false).is_err()); + } + #[test] fn request_has_id_field() { let id: u64 = 42; @@ -2627,6 +2955,48 @@ mod tests { ); } + #[tokio::test] + async fn prompt_timing_observer_events_do_not_capture_prompt_content() { + let script = r#"IFS= read -r _line +printf '%s\n' '{"jsonrpc":"2.0","id":0,"result":{"stopReason":"end_turn"}}'"#; + let mut client = spawn_script(script).await; + let observer = crate::observer::ObserverHandle::in_process(); + client.set_observer(Some(observer.clone()), 0); + + let result = client + .session_prompt_with_idle_timeout( + "test-session", + "SENSITIVE_PROMPT_SENTINEL", + std::time::Duration::from_secs(1), + std::time::Duration::from_secs(5), + ) + .await; + assert!(matches!(result, Ok(StopReason::EndTurn))); + + let events = observer.snapshot(); + for kind in [ + "prompt_dispatched", + "prompt_first_activity", + "prompt_wait_finished", + ] { + assert!( + events.iter().any(|event| event.kind == kind), + "missing {kind} timing event" + ); + } + let timing_payloads = events + .iter() + .filter(|event| event.kind.starts_with("prompt_")) + .map(|event| event.payload.to_string()) + .collect::>() + .join("\n"); + assert!( + !timing_payloads.contains("SENSITIVE_PROMPT_SENTINEL"), + "timing telemetry must never include prompt content" + ); + client.shutdown().await; + } + #[tokio::test] async fn hard_timeout_fires_when_deadline_is_immediate() { let mut client = spawn_script("while true; do echo 'noise'; sleep 0.01; done").await; diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index a38d6faa14..6395b9ef3a 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -361,6 +361,21 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_NO_IGNORE_SELF")] pub no_ignore_self: bool, + /// Seconds to wait after observing a self-authored channel message before + /// closing the exact still-running turn that published it. 0 disables the + /// compatibility recovery. + /// + /// Some ACP adapters can successfully publish their externally visible + /// result and then fail to return `session/prompt`. The grace period lets + /// the adapter finish naturally first; an exact turn-id match prevents a + /// delayed timer from cancelling later work in the same channel. + #[arg( + long = "self-publish-completion-grace", + env = "BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", + default_value_t = 0 + )] + pub self_publish_completion_grace_secs: u64, + /// Maximum number of context messages to include for thread replies and DMs. /// Set to 0 to disable automatic context fetching. Max 100. #[arg(long, env = "BUZZ_ACP_CONTEXT_MESSAGE_LIMIT", default_value_t = 12, @@ -437,6 +452,33 @@ pub struct CliArgs { )] pub permission_mode: PermissionMode, + /// Whether Buzz may answer ACP permission requests with `allow_once`. + /// + /// This preserves the historical harness behavior by default. A managed + /// runtime may enforce `false` when a headless community turn must never + /// substitute for interactive user consent. + #[arg( + long, + env = "BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", + default_value_t = true, + action = clap::ArgAction::Set + )] + pub auto_approve_permissions: bool, + + /// Whether owner-signed observer controls may resolve ACP permission + /// requests interactively with `allow_once` or `reject_once`. + /// + /// Disabled by default so existing runtimes preserve their historical + /// behavior. Managed runtimes that disable auto-approval may enable this + /// owner-consent path without selecting a bypass permission mode. + #[arg( + long, + env = "BUZZ_ACP_INTERACTIVE_PERMISSIONS", + default_value_t = false, + action = clap::ArgAction::Set + )] + pub interactive_permissions: bool, + /// Inbound author gate: which authors' events the harness forwards. /// Modes: owner-only (default), allowlist, anyone, nobody. #[arg( @@ -506,6 +548,9 @@ pub struct Config { pub dedup_mode: DedupMode, pub multiple_event_handling: MultipleEventHandling, pub ignore_self: bool, + /// Runtime compatibility recovery after a self-authored result publish. + /// 0 disables the recovery and preserves historical behavior. + pub self_publish_completion_grace_secs: u64, pub kinds_override: Option>, pub channels_override: Option>, pub no_mention_filter: bool, @@ -524,6 +569,10 @@ pub struct Config { pub model: Option, /// Permission mode to apply after session creation. `Default` = skip. pub permission_mode: PermissionMode, + /// Whether ACP permission requests may select `allow_once`. + pub auto_approve_permissions: bool, + /// Whether owner-signed observer controls may resolve permission requests. + pub interactive_permissions: bool, /// Inbound author gate mode. pub respond_to: RespondTo, /// Validated allowlist of pubkey hex strings (used when respond_to == Allowlist). @@ -616,7 +665,7 @@ pub(crate) fn normalize_agent_command_identity(command: &str) -> String { fn default_agent_args(command: &str) -> Option> { match normalize_agent_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), + "goose" | "devin" => Some(vec!["acp".to_string()]), "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" | "claudecode" | "buzz-agent" => Some(Vec::new()), _ => None, @@ -982,6 +1031,7 @@ impl Config { dedup_mode: args.dedup, multiple_event_handling: args.multiple_event_handling, ignore_self: !args.no_ignore_self, + self_publish_completion_grace_secs: args.self_publish_completion_grace_secs, kinds_override: args.kinds, channels_override: args.channels, no_mention_filter: args.no_mention_filter, @@ -993,6 +1043,8 @@ impl Config { memory_enabled: args.memory && !args.no_memory, model, permission_mode: args.permission_mode, + auto_approve_permissions: args.auto_approve_permissions, + interactive_permissions: args.interactive_permissions, respond_to: args.respond_to, respond_to_allowlist, allowed_respond_to, @@ -1024,7 +1076,7 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} self_publish_completion_grace={}s context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} auto_approve_permissions={} interactive_permissions={} {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1038,6 +1090,7 @@ impl Config { self.dedup_mode, self.multiple_event_handling, self.ignore_self, + self.self_publish_completion_grace_secs, self.context_message_limit, self.max_turns_per_session, self.presence_enabled, @@ -1045,6 +1098,8 @@ impl Config { self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), self.permission_mode, + self.auto_approve_permissions, + self.interactive_permissions, respond_to_detail, allowed_respond_to_detail, ) @@ -1351,6 +1406,7 @@ mod tests { dedup_mode: DedupMode::Queue, multiple_event_handling: MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -1362,6 +1418,8 @@ mod tests { memory_enabled: true, model: None, permission_mode: PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: Vec::new(), @@ -1492,6 +1550,22 @@ mod tests { ); } + #[test] + fn normalizes_devin_args_to_native_acp_subcommand() { + assert_eq!(normalize_agent_args("devin", Vec::new()), vec!["acp"]); + assert_eq!( + normalize_agent_args("/usr/local/bin/devin", vec!["".into()]), + vec!["acp"] + ); + assert_eq!( + normalize_agent_args( + "devin", + vec!["acp".into(), "--agent-type".into(), "review".into()] + ), + vec!["acp", "--agent-type", "review"] + ); + } + #[test] fn normalize_agent_command_identity_variants() { assert_eq!(normalize_agent_command_identity("goose"), "goose"); @@ -2048,6 +2122,57 @@ channels = "ALL" assert!(CliArgs::parse_from(["buzz-acp", "--private-key", &key, "--lazy-pool"]).lazy_pool); } + #[test] + fn self_publish_completion_recovery_defaults_off_and_accepts_a_grace() { + let key = "0".repeat(64); + assert_eq!( + CliArgs::parse_from(["buzz-acp", "--private-key", &key]) + .self_publish_completion_grace_secs, + 0 + ); + assert_eq!( + CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--self-publish-completion-grace", + "30", + ]) + .self_publish_completion_grace_secs, + 30 + ); + } + + #[test] + fn permission_request_auto_approval_defaults_on_and_can_be_disabled() { + let key = "0".repeat(64); + assert!(CliArgs::parse_from(["buzz-acp", "--private-key", &key]).auto_approve_permissions); + assert!( + !CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--auto-approve-permissions=false", + ]) + .auto_approve_permissions + ); + } + + #[test] + fn interactive_permissions_default_off_and_can_be_enabled() { + let key = "0".repeat(64); + assert!(!CliArgs::parse_from(["buzz-acp", "--private-key", &key]).interactive_permissions); + assert!( + CliArgs::parse_from([ + "buzz-acp", + "--private-key", + &key, + "--interactive-permissions=true", + ]) + .interactive_permissions + ); + } + #[test] fn test_summary_includes_agents_and_heartbeat() { let config = test_config(SubscribeMode::Mentions); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 0230ea0875..58369b98c1 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -885,12 +885,106 @@ fn handle_relay_observer_control_event( Some("switch_model") => { handle_switch_model_control(&payload, pool, observer); } + Some("permission_decision") => { + handle_permission_decision_control(&payload, pool, observer); + } _ => { tracing::debug!(payload = %payload, "ignoring unknown observer control frame"); } } } +/// Resolve one ACP permission request from an owner-signed encrypted control. +/// +/// Only per-request `allow_once` and `reject_once` decisions are accepted. +/// Channel, turn, and JSON-RPC request ids must all match the live task. +fn handle_permission_decision_control( + payload: &serde_json::Value, + pool: &mut AgentPool, + observer: Option<&observer::ObserverHandle>, +) { + let Some(channel_id) = payload + .get("channelId") + .and_then(|value| value.as_str()) + .and_then(|value| value.parse::().ok()) + else { + tracing::warn!("permission decision control missing valid channelId"); + return; + }; + let Some(turn_id) = payload.get("turnId").and_then(|value| value.as_str()) else { + tracing::warn!("permission decision control missing turnId"); + return; + }; + let Some(request_id) = payload + .get("requestId") + .filter(|value| value.is_string() || value.is_number()) + .cloned() + else { + tracing::warn!("permission decision control missing valid requestId"); + return; + }; + let selection = match payload.get("optionId").and_then(|value| value.as_str()) { + Some(option_id) if !option_id.is_empty() && option_id.len() <= 512 => { + pool::PermissionSelection::OptionId(option_id.to_string()) + } + Some(_) => { + tracing::warn!("permission optionId must be a non-empty string of at most 512 bytes"); + return; + } + None => match payload.get("decision").and_then(|value| value.as_str()) { + Some("allow_once") => pool::PermissionSelection::Kind("allow_once".to_string()), + Some("reject_once") => pool::PermissionSelection::Kind("reject_once".to_string()), + _ => { + tracing::warn!( + "permission decision must include optionId or a legacy allow_once/reject_once decision" + ); + return; + } + }, + }; + let selected_option_id = match &selection { + pool::PermissionSelection::OptionId(option_id) => Some(option_id.clone()), + pool::PermissionSelection::Kind(_) => None, + }; + let selected_kind = match &selection { + pool::PermissionSelection::Kind(kind) => Some(kind.clone()), + pool::PermissionSelection::OptionId(_) => None, + }; + + let status = match pool.send_permission_decision( + channel_id, + turn_id, + pool::PermissionDecision { + request_id, + selection, + }, + ) { + Ok(()) => "sent", + Err(pool::PermissionDecisionError::NoActiveTurn) => "no_active_turn", + Err(pool::PermissionDecisionError::StaleTurn) => "stale_turn", + Err(pool::PermissionDecisionError::Unavailable) => "unavailable", + }; + + if let Some(observer) = observer { + observer.emit( + "control_result", + None, + &observer::ObserverContext { + channel_id: Some(channel_id.to_string()), + session_id: None, + turn_id: Some(turn_id.to_string()), + started_at: None, + }, + serde_json::json!({ + "type": "permission_decision", + "status": status, + "optionId": selected_option_id, + "decision": selected_kind, + }), + ); + } +} + /// Handle a `cancel_turn` control frame: signal the in-flight task to cancel. fn handle_cancel_turn_control( payload: &serde_json::Value, @@ -1553,6 +1647,8 @@ async fn tokio_main() -> Result<()> { context_message_limit: config.context_message_limit, max_turns_per_session: config.max_turns_per_session, permission_mode: config.permission_mode, + auto_approve_permissions: config.auto_approve_permissions, + interactive_permissions: config.interactive_permissions, agent_keys: config.keys.clone(), agent_owner_pubkey: startup_owner .as_deref() @@ -1627,6 +1723,13 @@ async fn tokio_main() -> Result<()> { // withheld event in `EventQueue::withheld_native_steer` until // `IN_FLIGHT_DEADLINE_SECS` expires. let (steer_ack_tx, mut steer_ack_rx) = mpsc::unbounded_channel::(); + // Runtime-scoped compatibility recovery for ACP adapters that publish a + // visible Buzz result but fail to finish `session/prompt`. One pending + // timer per turn bounds task/channel growth even if the agent publishes + // several messages during the grace window. + let (self_publish_completion_tx, mut self_publish_completion_rx) = + mpsc::unbounded_channel::<(Uuid, String, String)>(); + let mut pending_self_publish_completions = HashSet::::new(); // ── Step 7: Shutdown signal ─────────────────────────────────────────────── let (shutdown_tx, mut shutdown_rx) = watch::channel(()); @@ -1701,6 +1804,11 @@ async fn tokio_main() -> Result<()> { Result(Box), Panic(tokio::task::JoinError), SteerAck(SteerAckEvent), + SelfPublishCompletion { + channel_id: Uuid, + turn_id: String, + event_id: String, + }, Wake(u32, Result), } @@ -1844,6 +1952,15 @@ async fn tokio_main() -> Result<()> { Some(ack_event) = steer_ack_rx.recv() => { Some(PoolEvent::SteerAck(ack_event)) } + Some((channel_id, turn_id, event_id)) = self_publish_completion_rx.recv(), + if config.self_publish_completion_grace_secs > 0 => + { + Some(PoolEvent::SelfPublishCompletion { + channel_id, + turn_id, + event_id, + }) + } Some((attempt, result)) = wake_rx.recv(), if config.lazy_pool && !pool_ready => { Some(PoolEvent::Wake(attempt, result)) } @@ -2024,6 +2141,29 @@ async fn tokio_main() -> Result<()> { continue; } + if buzz_event.event.pubkey.to_hex() == pubkey_hex + && kind_u32 == KIND_STREAM_MESSAGE + && config.self_publish_completion_grace_secs > 0 + { + if let Some(turn_id) = + in_flight_turn_id(&pool, buzz_event.channel_id) + { + let turn_id = turn_id.to_owned(); + if pending_self_publish_completions.insert(turn_id.clone()) { + let tx = self_publish_completion_tx.clone(); + let channel_id = buzz_event.channel_id; + let event_id = buzz_event.event.id.to_hex(); + let grace = Duration::from_secs( + config.self_publish_completion_grace_secs, + ); + tokio::spawn(async move { + tokio::time::sleep(grace).await; + let _ = tx.send((channel_id, turn_id, event_id)); + }); + } + } + } + if config.ignore_self && buzz_event.event.pubkey.to_hex() == pubkey_hex { tracing::debug!(channel_id = %buzz_event.channel_id, "dropping self-authored event"); continue; @@ -2533,6 +2673,34 @@ async fn tokio_main() -> Result<()> { typing_channels.insert(channel_id, thread_tags); } } + Some(PoolEvent::SelfPublishCompletion { + channel_id, + turn_id, + event_id, + }) => { + pending_self_publish_completions.remove(&turn_id); + if signal_exact_in_flight_task( + &mut pool, + channel_id, + &turn_id, + ControlSignal::PublishedResult, + ) { + tracing::warn!( + channel = %channel_id, + turn_id, + event_id, + grace_secs = config.self_publish_completion_grace_secs, + "self-authored result was published but ACP turn remained open — completing exact turn" + ); + } else { + tracing::debug!( + channel = %channel_id, + turn_id, + event_id, + "self-publish completion grace elapsed after turn already finished" + ); + } + } Some(PoolEvent::Wake(attempt, result)) => { let completion = result.as_ref().map(|_| ()).map_err(|error| error.clone()); if let Err(error) = @@ -2728,6 +2896,14 @@ fn is_owner_control_command( // ── signal_in_flight_task ───────────────────────────────────────────────────── +/// Return the turn id currently checked out for `channel_id`. +fn in_flight_turn_id(pool: &AgentPool, channel_id: uuid::Uuid) -> Option<&str> { + pool.task_map() + .values() + .find(|meta| meta.channel_id == Some(channel_id)) + .map(|meta| meta.turn_id.as_str()) +} + /// Decide which [`ControlSignal`] (if any) to send to an in-flight turn when a /// new, already-author-gated event arrives for that channel. /// @@ -2776,6 +2952,36 @@ fn signal_in_flight_task( false } +/// Send a control signal only when both channel and turn id still match. +/// +/// Delayed compatibility timers use this stricter boundary so a stale timer +/// from a completed turn can never cancel later work in the same channel. +fn signal_exact_in_flight_task( + pool: &mut AgentPool, + channel_id: uuid::Uuid, + turn_id: &str, + mode: ControlSignal, +) -> bool { + let entry = pool + .task_map_mut() + .values_mut() + .find(|meta| meta.channel_id == Some(channel_id) && meta.turn_id == turn_id); + + if let Some(meta) = entry { + if let Some(tx) = meta.control_tx.take() { + tracing::info!( + channel = %channel_id, + turn_id, + ?mode, + "exact-turn control signal sent to in-flight task" + ); + let _ = tx.send(mode); + return true; + } + } + false +} + /// Attempt the non-cancelling (ACP) steer for a freshly-queued event. /// /// Caller invariants: @@ -2937,6 +3143,9 @@ fn dispatch_pending( let (tx, rx) = tokio::sync::mpsc::channel::(1); agent.acp.install_steer_rx(rx); let steer_tx = Some(tx); + let (permission_tx, permission_rx) = + tokio::sync::mpsc::channel::(4); + agent.acp.install_permission_rx(permission_rx); // Prompt text is now built inside run_prompt_task (needs async for // context fetching). Pass None for prompt_text; batch carries the data. @@ -2966,6 +3175,7 @@ fn dispatch_pending( recoverable_batch, control_tx: Some(control_tx), steer_tx, + permission_tx: Some(permission_tx), }, ); dispatched_channels.push((channel_id, typing_scope)); @@ -3579,6 +3789,7 @@ fn dispatch_heartbeat( recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); *heartbeat_in_flight = true; @@ -3742,6 +3953,11 @@ async fn initialize_agent_pool( startup: &PoolStartup, mut shutdown: Option>, ) -> Result { + let pool_started_at = std::time::Instant::now(); + tracing::info!( + agents = startup.agents, + "ACP agent pool initialization started" + ); // One agent failing to start must not kill the whole pool. // Attempt each spawn under a 60-second timeout; a partial pool is valid. let mut agent_slots: Vec> = Vec::with_capacity(startup.agents as usize); @@ -3836,7 +4052,11 @@ async fn initialize_agent_pool( startup.agents ); } - tracing::info!("agent_pool_ready agents={}", live_count); + tracing::info!( + agents = live_count, + elapsed_ms = pool_started_at.elapsed().as_millis() as u64, + "agent_pool_ready" + ); Ok(AgentPool::from_slots(agent_slots)) } @@ -4321,6 +4541,7 @@ mod owner_control_command_tests { recoverable_batch: None, control_tx: Some(control_tx), steer_tx: None, + permission_tx: None, }, ); @@ -4341,6 +4562,120 @@ mod owner_control_command_tests { ControlSignal::Rotate )); } + + #[tokio::test] + async fn self_publish_completion_signal_requires_the_exact_turn() { + let mut pool = AgentPool::from_slots(vec![]); + let channel_id = Uuid::new_v4(); + let (control_tx, control_rx) = tokio::sync::oneshot::channel(); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "current-turn".to_string(), + recoverable_batch: None, + control_tx: Some(control_tx), + steer_tx: None, + permission_tx: None, + }, + ); + + assert_eq!(in_flight_turn_id(&pool, channel_id), Some("current-turn")); + assert!(!signal_exact_in_flight_task( + &mut pool, + channel_id, + "stale-turn", + ControlSignal::PublishedResult, + )); + assert!(signal_exact_in_flight_task( + &mut pool, + channel_id, + "current-turn", + ControlSignal::PublishedResult, + )); + assert_eq!(control_rx.await.unwrap(), ControlSignal::PublishedResult); + } + + #[tokio::test] + async fn permission_control_forwards_exact_option_and_preserves_numeric_request_id() { + let mut pool = AgentPool::from_slots(vec![None]); + let channel_id = Uuid::new_v4(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "permission-turn".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + handle_permission_decision_control( + &serde_json::json!({ + "type": "permission_decision", + "channelId": channel_id, + "turnId": "permission-turn", + "requestId": 42, + "optionId": "allow-buzz-messages-in-workspace", + }), + &mut pool, + None, + ); + let decision = permission_rx + .recv() + .await + .expect("exact permission selection should be delivered"); + assert_eq!(decision.request_id, serde_json::json!(42)); + assert_eq!( + decision.selection, + pool::PermissionSelection::OptionId("allow-buzz-messages-in-workspace".to_string()) + ); + } + + #[tokio::test] + async fn permission_control_rejects_invalid_option_ids() { + let mut pool = AgentPool::from_slots(vec![None]); + let channel_id = Uuid::new_v4(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + let abort_handle = pool.join_set.spawn(async {}); + pool.task_map_mut().insert( + abort_handle.id(), + pool::TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "permission-turn".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + handle_permission_decision_control( + &serde_json::json!({ + "type": "permission_decision", + "channelId": channel_id, + "turnId": "permission-turn", + "requestId": 42, + "optionId": "", + }), + &mut pool, + None, + ); + assert!(matches!( + permission_rx.try_recv(), + Err(tokio::sync::mpsc::error::TryRecvError::Empty) + )); + } } #[cfg(test)] @@ -4963,6 +5298,7 @@ mod build_mcp_servers_tests { dedup_mode: config::DedupMode::Queue, multiple_event_handling: config::MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -4974,6 +5310,8 @@ mod build_mcp_servers_tests { memory_enabled: false, model: None, permission_mode: config::PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: config::RespondTo::Anyone, respond_to_allowlist: std::collections::HashSet::new(), allowed_respond_to: vec![], @@ -5129,6 +5467,7 @@ mod error_outcome_emission_tests { dedup_mode: config::DedupMode::Queue, multiple_event_handling: config::MultipleEventHandling::Queue, ignore_self: true, + self_publish_completion_grace_secs: 0, kinds_override: None, channels_override: None, no_mention_filter: false, @@ -5140,6 +5479,8 @@ mod error_outcome_emission_tests { memory_enabled: false, model: None, permission_mode: config::PermissionMode::BypassPermissions, + auto_approve_permissions: true, + interactive_permissions: false, respond_to: config::RespondTo::Anyone, respond_to_allowlist: HashSet::new(), allowed_respond_to: vec![], @@ -5210,6 +5551,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); @@ -5286,6 +5628,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); started_rx.await.unwrap(); @@ -5378,6 +5721,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5469,6 +5813,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5574,6 +5919,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5650,6 +5996,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5744,6 +6091,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let config = test_config(); @@ -5860,6 +6208,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -5999,6 +6348,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6187,6 +6537,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); @@ -6272,6 +6623,7 @@ mod error_outcome_emission_tests { recoverable_batch: None, control_tx: None, steer_tx: None, + permission_tx: None, }, ); let mut queue = EventQueue::new(config::DedupMode::Queue); diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index cc537f8683..b60c127dff 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -65,6 +65,35 @@ pub struct TaskMeta { /// tasks only — all prompt tasks install a steer channel regardless /// of the agent's name. pub steer_tx: Option>, + /// Owner-approved permission decisions for the in-flight turn. + /// + /// Decisions are matched to both `turn_id` and the ACP JSON-RPC request id + /// before the read loop may select `allow_once`. + pub permission_tx: Option>, +} + +/// One owner selection for an ACP `session/request_permission` request. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PermissionDecision { + pub request_id: serde_json::Value, + pub selection: PermissionSelection, +} + +/// How an owner-selected ACP permission option is identified. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PermissionSelection { + /// Exact `optionId` copied from the current permission request. + OptionId(String), + /// Backward-compatible selection by ACP option kind. + Kind(String), +} + +/// Failure to deliver an interactive permission decision to a live turn. +#[derive(Debug, PartialEq, Eq)] +pub enum PermissionDecisionError { + NoActiveTurn, + StaleTurn, + Unavailable, } /// Agent-level model capabilities. Populated on first session creation. @@ -263,6 +292,11 @@ fn apply_completed_before_control_signal( pub enum ControlSignal { /// Stop the current turn and drop its triggering batch. Cancel, + /// The agent already published an externally visible channel result, but + /// its ACP adapter did not finish `session/prompt` within the configured + /// compatibility grace. Stop the exact publishing turn, drop its already + /// satisfied batch, and report successful completion. + PublishedResult, /// Stop the current turn and requeue its triggering batch for a merged /// re-prompt framed as a **supersede**: the new request replaces the old. Interrupt, @@ -510,6 +544,10 @@ pub struct PromptContext { pub max_turns_per_session: u32, /// Permission mode to apply after session creation. `Default` = skip. pub permission_mode: PermissionMode, + /// Whether ACP permission requests may select `allow_once`. + pub auto_approve_permissions: bool, + /// Whether owner-signed controls may resolve ACP permission requests. + pub interactive_permissions: bool, /// Agent identity — used to derive the NIP-AE conversation key at /// session creation for core injection. pub agent_keys: nostr::Keys, @@ -661,6 +699,32 @@ impl AgentPool { .map_err(|e| SteerError::Transport(e.to_string())) } + /// Deliver an owner-approved permission decision to one exact in-flight + /// turn. Matching the turn id prevents a delayed control frame from + /// authorizing a later turn in the same channel. + pub fn send_permission_decision( + &mut self, + channel_id: Uuid, + turn_id: &str, + decision: PermissionDecision, + ) -> Result<(), PermissionDecisionError> { + let Some(meta) = self + .task_map + .values_mut() + .find(|meta| meta.channel_id == Some(channel_id)) + else { + return Err(PermissionDecisionError::NoActiveTurn); + }; + if meta.turn_id != turn_id { + return Err(PermissionDecisionError::StaleTurn); + } + let Some(tx) = meta.permission_tx.as_ref() else { + return Err(PermissionDecisionError::Unavailable); + }; + tx.try_send(decision) + .map_err(|_| PermissionDecisionError::Unavailable) + } + pub fn result_tx(&self) -> mpsc::UnboundedSender { self.result_tx.clone() } @@ -824,7 +888,6 @@ async fn create_session_and_apply_model( ), agent_canvas, ); - let resp = agent .acp .session_new_full( @@ -1241,6 +1304,7 @@ fn send_prompt_result( batch: Option, ) { agent.acp.clear_steer_rx(); + agent.acp.clear_permission_rx(); let _ = result_tx.send(PromptResult { agent, source, @@ -1271,6 +1335,13 @@ pub async fn run_prompt_task( control_rx: Option>, turn_id: String, ) { + agent + .acp + .set_auto_approve_permissions(ctx.auto_approve_permissions); + agent + .acp + .set_interactive_permissions(ctx.interactive_permissions); + // Is this a channel prompt or a heartbeat? let source = match &batch { Some(b) => PromptSource::Channel(b.channel_id), @@ -1576,7 +1647,6 @@ pub async fn run_prompt_task( "isNewSession": is_new_session, }), ); - if is_new_session { if let (PromptSource::Channel(cid), Some(ref initial_msg)) = (&source, &ctx.initial_message) { @@ -1819,7 +1889,6 @@ pub async fn run_prompt_task( .collect(), None => prompt_sections.iter().map(String::as_str).collect(), }; - // When control_rx is Some (channel tasks), wrap the prompt in select! so // the main loop can cancel, interrupt, or rotate it. Heartbeats // (control_rx=None) take the simple await path — they are not controllable. @@ -1848,6 +1917,8 @@ pub async fn run_prompt_task( ) => result, mode = rx => { let control_signal = mode.unwrap_or(ControlSignal::Cancel); + let published_result = + matches!(control_signal, ControlSignal::PublishedResult); // Land the model switch before any cancel/requeue work: setting // `desired_model` here means the fresh session created by the // requeued turn (busy) or the next turn (already-completed) @@ -1878,7 +1949,11 @@ pub async fn run_prompt_task( observer_channel_id, &session_id, &turn_id, - Some(buzz_core::agent_turn_metric::StopReason::Cancelled), + Some(if published_result { + buzz_core::agent_turn_metric::StopReason::EndTurn + } else { + buzz_core::agent_turn_metric::StopReason::Cancelled + }), ) .await; send_prompt_result( @@ -1886,7 +1961,11 @@ pub async fn run_prompt_task( &turn_id, agent, source, - PromptOutcome::Cancelled, + if published_result { + PromptOutcome::Ok(StopReason::EndTurn) + } else { + PromptOutcome::Cancelled + }, retry_batch, ); return; @@ -2991,8 +3070,12 @@ fn requeue_cancelled_batch( let reason = match signal { ControlSignal::Steer => CancelReason::Steer, ControlSignal::Interrupt | ControlSignal::SwitchModel(_) => CancelReason::Interrupt, - // Cancel/Rotate discard the batch — no merged re-prompt. - ControlSignal::Cancel | ControlSignal::Rotate => return None, + // Cancel/Rotate discard the batch — no merged re-prompt. PublishedResult + // also discards it because the agent's self-authored channel event is + // evidence that the triggering work already produced its visible result. + ControlSignal::Cancel | ControlSignal::Rotate | ControlSignal::PublishedResult => { + return None; + } }; requeue_batch_if_queue(ctx, batch).map(|mut b| { b.cancel_reason = Some(reason); @@ -3653,6 +3736,59 @@ mod tests { use nostr::{EventBuilder, Keys, Kind, Tag, Timestamp}; use serde_json::json; + #[tokio::test] + async fn permission_decision_requires_exact_turn_and_delivers_once() { + let channel_id = Uuid::new_v4(); + let mut pool = AgentPool::from_slots(vec![None]); + let task_id = pool.join_set.spawn(async {}).id(); + let (permission_tx, mut permission_rx) = + tokio::sync::mpsc::channel::(1); + pool.task_map_mut().insert( + task_id, + TaskMeta { + agent_index: 0, + channel_id: Some(channel_id), + turn_id: "turn-current".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + permission_tx: Some(permission_tx), + }, + ); + + assert_eq!( + pool.send_permission_decision( + channel_id, + "turn-stale", + PermissionDecision { + request_id: json!("request-1"), + selection: PermissionSelection::Kind("allow_once".to_string()), + }, + ), + Err(PermissionDecisionError::StaleTurn) + ); + + pool.send_permission_decision( + channel_id, + "turn-current", + PermissionDecision { + request_id: json!("request-1"), + selection: PermissionSelection::OptionId("allow-workspace".to_string()), + }, + ) + .expect("matching owner decision should be delivered"); + + let delivered = permission_rx + .recv() + .await + .expect("permission decision should arrive"); + assert_eq!(delivered.request_id, json!("request-1")); + assert_eq!( + delivered.selection, + PermissionSelection::OptionId("allow-workspace".to_string()) + ); + } + // These pin the initial_message dispatch path (run_prompt_task, ~line 855): // a legacy agent WITH a base_prompt must get [Base] prepended to the user // message. This is the exact regression that shipped in the round-2 bug. @@ -4466,6 +4602,7 @@ mod tests { ), (ControlSignal::Cancel, None), (ControlSignal::Rotate, None), + (ControlSignal::PublishedResult, None), ]; let mut ctx = make_prompt_context_no_owner(); ctx.dedup_mode = DedupMode::Queue; @@ -4556,6 +4693,15 @@ mod tests { expected_reason: None, invalidate_all: false, }, + Case { + name: "CancelDrainTimeout + PublishedResult drops the satisfied batch", + error: || AcpError::CancelDrainTimeout(CONTROL_CANCEL_GRACE), + signal: ControlSignal::PublishedResult, + expected_outcome: "CancelDrainTimeout", + batch_preserved: false, + expected_reason: None, + invalidate_all: false, + }, Case { name: "CancelDrainTimeout + Interrupt preserves batch with Interrupt reason", error: || AcpError::CancelDrainTimeout(CONTROL_CANCEL_GRACE), @@ -5302,6 +5448,8 @@ mod tests { context_message_limit: 0, max_turns_per_session: 0, permission_mode: PermissionMode::Default, + auto_approve_permissions: true, + interactive_permissions: false, agent_keys: agent_keys.clone(), agent_owner_pubkey: owner_pubkey, memory_enabled: false, diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index 029bf86dbf..f2026002b3 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -1171,6 +1171,21 @@ fn append_new_thread_reply_instruction(s: &mut String, event_id: &str) { )); } +/// Append the destination instruction for an ordinary top-level DM response. +/// +/// Omitting a reply anchor is not enough for every ACP runtime: an agent may +/// still choose to thread a conversational response. State the main-timeline +/// destination explicitly while preserving an explicit human request to open a +/// thread. +fn append_top_level_dm_reply_instruction(s: &mut String) { + s.push_str( + "\nIMPORTANT: This is a top-level DM message. For ordinary replies in \ + this turn, use `buzz messages send` without `--reply-to` so the answer \ + appears directly in the DM's main timeline. Only use `--reply-to` if \ + the human explicitly asks for a threaded response.", + ); +} + /// Decide whether a turn is human-facing for reply-anchor purposes. /// /// A turn is human-facing when the triggering sender is a human, OR a human @@ -1275,6 +1290,8 @@ fn format_context_hints( if let Some(event_id) = reply_anchor { append_reply_instruction(&mut s, event_id); } + } else { + append_top_level_dm_reply_instruction(&mut s); } s } else if let Some(ref root) = thread_tags.root_event_id { @@ -1463,7 +1480,8 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec + + + diff --git a/desktop/src-tauri/src/commands/agent_auth.rs b/desktop/src-tauri/src/commands/agent_auth.rs index bba0f33860..388a28c7d4 100644 --- a/desktop/src-tauri/src/commands/agent_auth.rs +++ b/desktop/src-tauri/src/commands/agent_auth.rs @@ -68,6 +68,12 @@ pub async fn connect_acp_runtime( } fn discover_acp_auth_methods_blocking(runtime_id: &str) -> Result { + if let Some(runtime) = known_acp_runtime_exact(runtime_id) { + if let Some(methods) = catalog_cli_auth_methods(runtime) { + return Ok(methods); + } + } + let output = run_buzz_acp_auth_command(runtime_id, ["auth-methods", "--json"])?; if !output.status.success() { return Err(command_error("buzz-acp auth-methods", &output)); @@ -77,6 +83,23 @@ fn discover_acp_auth_methods_blocking(runtime_id: &str) -> Result Option { + let command = runtime.auth_login_args?; + Some(AcpAuthMethodsResult { + methods: vec![AcpAuthMethod { + id: "cli-login".to_string(), + name: format!("Sign in to {}", runtime.label), + description: runtime.login_hint.map(str::to_string), + method_type: Some("terminal".to_string()), + args: Vec::new(), + command: command.iter().map(|arg| (*arg).to_string()).collect(), + meta: None, + }], + }) +} + fn connect_acp_runtime_blocking( request: &ConnectAcpRuntimeRequest, ) -> Result { @@ -256,7 +279,7 @@ fn launch_terminal_auth(runtime_id: &str, method: &AcpAuthMethod) -> Result<(), .ok_or_else(|| format!("{} ACP adapter is not installed", runtime.label))?; let fallback_command = adapter_command.1.display().to_string(); let argv = adapter_terminal_argv(runtime.label, method, &fallback_command)?; - launch_visible_terminal(&argv) + launch_visible_terminal(&argv, runtime.scrub_env_vars) } fn adapter_terminal_argv( @@ -361,15 +384,16 @@ fn spawn_without_stdio(mut command: Command) -> Result<(), String> { } #[cfg(target_os = "macos")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { let mut script = tempfile::Builder::new() .prefix("buzz-auth-") .suffix(".command") .tempfile() .map_err(|error| format!("failed to create terminal login script: {error}"))?; + let unset_commands = shell_unset_commands(scrub_env_vars); writeln!( script, - "#!/bin/sh\ntrap 'rm -f -- \"$0\"' EXIT\n{}", + "#!/bin/sh\ntrap 'rm -f -- \"$0\"' EXIT\n{unset_commands}{}", shell_join(argv) ) .map_err(|error| format!("failed to write terminal login script: {error}"))?; @@ -395,8 +419,12 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { } #[cfg(target_os = "linux")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { - let command = shell_join(argv); +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { + let command = format!( + "{}{}", + shell_unset_commands(scrub_env_vars), + shell_join(argv) + ); let candidates: [(&str, &[&str]); 4] = [ ("x-terminal-emulator", &["-e", "sh", "-lc"]), ("gnome-terminal", &["--", "sh", "-lc"]), @@ -406,6 +434,9 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { for (terminal, prefix) in candidates { let mut terminal_command = Command::new(terminal); terminal_command.args(prefix).arg(&command); + for key in scrub_env_vars { + terminal_command.env_remove(key); + } if spawn_without_stdio(terminal_command).is_ok() { return Ok(()); } @@ -414,7 +445,7 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { } #[cfg(target_os = "windows")] -fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(argv: &[String], scrub_env_vars: &[&str]) -> Result<(), String> { use std::os::windows::process::CommandExt; const CREATE_NEW_CONSOLE: u32 = 0x0000_0010; @@ -425,6 +456,9 @@ fn launch_visible_terminal(argv: &[String]) -> Result<(), String> { command .args(windows_terminal_args(argv)) .creation_flags(CREATE_NEW_CONSOLE); + for key in scrub_env_vars { + command.env_remove(key); + } spawn_without_stdio(command) } @@ -436,10 +470,18 @@ fn windows_terminal_args(argv: &[String]) -> Vec { } #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))] -fn launch_visible_terminal(_argv: &[String]) -> Result<(), String> { +fn launch_visible_terminal(_argv: &[String], _scrub_env_vars: &[&str]) -> Result<(), String> { Err("opening a terminal is not supported on this platform".to_string()) } +#[cfg(any(target_os = "macos", target_os = "linux", test))] +fn shell_unset_commands(scrub_env_vars: &[&str]) -> String { + scrub_env_vars + .iter() + .map(|key| format!("unset {}\n", shell_escape(key))) + .collect() +} + fn shell_join(argv: &[String]) -> String { argv.iter() .map(|arg| shell_escape(arg)) @@ -461,11 +503,35 @@ fn shell_escape(arg: &str) -> String { #[cfg(test)] mod tests { use super::{ - adapter_terminal_argv, append_inherited_path, is_claude_subscription_login, - run_buzz_acp_auth_command_with_paths, shell_escape, shell_join, uses_terminal_auth, - windows_terminal_args, AcpAuthMethod, + adapter_terminal_argv, append_inherited_path, catalog_cli_auth_methods, + is_claude_subscription_login, run_buzz_acp_auth_command_with_paths, shell_escape, + shell_join, shell_unset_commands, uses_terminal_auth, windows_terminal_args, AcpAuthMethod, }; + #[test] + fn devin_uses_catalog_declared_visible_terminal_login() { + let runtime = + crate::managed_agents::known_acp_runtime_exact("devin").expect("Devin runtime"); + let result = catalog_cli_auth_methods(runtime).expect("catalog CLI login method"); + + assert_eq!(result.methods.len(), 1); + assert_eq!(result.methods[0].id, "cli-login"); + assert_eq!(result.methods[0].method_type.as_deref(), Some("terminal")); + assert_eq!( + result.methods[0].command, + ["devin", "auth", "login"].map(str::to_string) + ); + } + + #[test] + fn terminal_login_scrubs_only_catalog_declared_identity_overrides() { + assert_eq!( + shell_unset_commands(&["WINDSURF_API_KEY"]), + "unset WINDSURF_API_KEY\n" + ); + assert!(shell_unset_commands(&[]).is_empty()); + } + /// Windows regression: the augmented PATH there holds only Buzz-managed /// dirs and the exe parent (no login-shell PATH, no managed Node), so the /// user's inherited PATH must be appended for npm `.cmd` adapters to find diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 461fed5dbd..43f168c100 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -606,40 +606,8 @@ mod tests { use super::*; use crate::managed_agents::{BackendKind, RespondTo}; - fn goose_runtime() -> &'static KnownAcpRuntime { - &KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - } - } + mod fixtures; + use fixtures::goose_runtime; fn agent_record() -> ManagedAgentRecord { ManagedAgentRecord { diff --git a/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs b/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs new file mode 100644 index 0000000000..1a4ec99c3f --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_config/tests/fixtures.rs @@ -0,0 +1,50 @@ +use super::*; + +pub(super) fn goose_runtime() -> &'static KnownAcpRuntime { + &KnownAcpRuntime { + id: "goose", + label: "Goose", + display_label: "Goose", + sort_priority: 1, + onboarding_visible: false, + commands: &["goose"], + aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, + avatar_url: "", + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + } +} diff --git a/desktop/src-tauri/src/commands/agent_discovery.rs b/desktop/src-tauri/src/commands/agent_discovery.rs index 84a00433c0..75d0019027 100644 --- a/desktop/src-tauri/src/commands/agent_discovery.rs +++ b/desktop/src-tauri/src/commands/agent_discovery.rs @@ -108,9 +108,7 @@ pub async fn save_custom_harness( original_id: Option, app: tauri::AppHandle, ) -> Result { - use crate::managed_agents::{ - custom_harnesses, AcpAvailabilityStatus, AuthStatus, HarnessSource, - }; + use crate::managed_agents::{custom_harnesses, AcpAvailabilityStatus}; use tauri::Manager; // ── Phase 1: full validation before touching the filesystem ───────────── @@ -164,31 +162,13 @@ pub async fn save_custom_harness( let default_args = crate::managed_agents::normalize_agent_args(&definition.command, definition.args.clone()); - Ok(AcpRuntimeCatalogEntry { - id: definition.id, - label: definition.label, - // Security: no user-supplied avatar URL in catalog entries. - avatar_url: String::new(), + Ok(crate::managed_agents::custom_runtime_catalog_entry( + definition, availability, - command: command_opt, + command_opt, binary_path, default_args, - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - install_hint: definition.install_hint, - install_instructions_url: definition.install_instructions_url, - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Custom, - // Carry definition env back so the edit form can read and preserve it. - definition_env: definition.env, - }) + )) } /// Remove a user-defined harness definition from `/custom_harnesses/`. @@ -1291,18 +1271,18 @@ pub async fn discover_managed_agent_prereqs( #[tauri::command] pub async fn list_relay_agents(state: State<'_, AppState>) -> Result, String> { - // Query kind:10100 agent profile events from the relay. + // Kind:30177 carries identity + inbound-author policy; 10100 is channel-add. let events = query_relay( &state, &[serde_json::json!({ - "kinds": [10100], + "kinds": [buzz_core_pkg::kind::KIND_MANAGED_AGENT], })], ) .await?; // The convert helper returns `{"agents": [...]}`. Extract and re-deserialize // into the strongly-typed `Vec` the frontend expects. - let value = nostr_convert::agents_from_events(&events); + let value = nostr_convert::managed_agents_from_events(&events); let agents = value .get("agents") .cloned() diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 3b5ebeca4f..9cbea37949 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -7,11 +7,11 @@ use crate::{ build_managed_agent_summary, current_instance_id, discover_provider_candidates, ensure_persona_is_active, find_managed_agent_mut, load_managed_agents, load_personas, load_teams, managed_agent_avatar_url, managed_agents_base_dir, normalize_agent_args, - provider_deploy, resolve_provider_binary, save_managed_agents, start_managed_agent_process, - stop_managed_agent_process, stop_managed_agent_workspace_pair, + provider_deploy, resolve_agent_parallelism, resolve_provider_binary, save_managed_agents, + start_managed_agent_process, stop_managed_agent_process, stop_managed_agent_workspace_pair, sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, BackendKind, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, - ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, + ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, @@ -872,7 +872,7 @@ pub async fn create_managed_agent( input.parallelism, linked_persona.as_ref(), )?; - + let resolved_parallelism = resolve_agent_parallelism(minted.parallelism, &agent_command); let record = crate::managed_agents::ManagedAgentRecord { pubkey: pubkey.clone(), name: name.clone(), @@ -900,7 +900,7 @@ pub async fn create_managed_agent( // 0 or None → harness uses its own default (320s idle, 3600s max), and the CLI also clamps 0 → minimum. idle_timeout_seconds: input.idle_timeout_seconds.filter(|s| *s > 0), max_turn_duration_seconds: input.max_turn_duration_seconds.filter(|s| *s > 0), - parallelism: minted.parallelism.unwrap_or(DEFAULT_AGENT_PARALLELISM), + parallelism: resolved_parallelism, system_prompt: snapshot_prompt.or_else(|| { input .system_prompt diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index e32fc1cfe4..422b6c990a 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -86,6 +86,27 @@ fn persona_record(id: &str, model: Option<&str>, provider: Option<&str>) -> Agen } } +#[test] +fn created_devin_agents_default_to_one_worker() { + assert_eq!(resolve_agent_parallelism(None, "devin"), 1); +} + +#[test] +fn created_existing_runtime_agents_keep_the_global_default() { + for command in ["goose", "claude-agent-acp", "codex-acp", "buzz-agent"] { + assert_eq!( + resolve_agent_parallelism(None, command), + crate::managed_agents::DEFAULT_AGENT_PARALLELISM, + "{command}" + ); + } +} + +#[test] +fn explicit_or_persona_parallelism_overrides_runtime_default() { + assert_eq!(resolve_agent_parallelism(Some(3), "devin"), 3); +} + /// Auto-archive uses the same NIP-IA wire builder as the explicit GUI action, /// attaches owner consent, and marks a deliberate delete as `retired`. #[test] diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 64405d0440..96efd42992 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -395,6 +395,21 @@ pub fn run() { migration::run_boot_migrations(&app_handle); } + // Reclaim subprocesses left behind by an ungraceful prior exit + // before the webview can send messages. Agent restoration remains + // deferred until apply_workspace installs the correct relay, but + // cleanup needs neither relay nor identity and must not share that + // delay: a stale lazy harness can still subscribe as the same agent + // and consume the first post-relaunch prompt. + // + // The single-instance plugin has already admitted this process. + // Receipt, same-instance marker, and exact-bundle checks keep + // upstream Buzz plus other bundle identifiers out of scope. + let startup_instance_id = managed_agents::current_instance_id(&app_handle); + managed_agents::sweep_orphaned_agent_processes(&app_handle, &[]); + managed_agents::sweep_system_agent_processes(&startup_instance_id, &[]); + managed_agents::sweep_untracked_bundle_harnesses(&[]); + // Resolve persisted identity key (env var → file → generate+save). // This is fatal — the app should not start with an ephemeral identity // that will be lost on restart, as that silently breaks channel diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index ba4407d164..90b6ae66a0 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -102,7 +102,18 @@ pub fn agent_event_content(record: &ManagedAgentRecord) -> ManagedAgentEventCont }, parallelism: record.parallelism, respond_to: record.respond_to, - respond_to_allowlist: record.respond_to_allowlist.clone(), + // The allowlist only means something in allowlist mode. The instance + // record deliberately retains it across mode toggles so an owner can + // flip away and back without retyping entries, but publishing it under + // another mode would advertise pubkeys the owner has already revoked — + // the same reason `apply_persona_behavior` clears it on the definition + // side. Spawn agrees: `build_respond_to_env` emits no allowlist + // variable unless the mode is Allowlist. + respond_to_allowlist: if record.respond_to == RespondTo::Allowlist { + record.respond_to_allowlist.clone() + } else { + Vec::new() + }, } } @@ -336,6 +347,44 @@ mod tests { assert_eq!(a, b); } + /// Revoking an allowlist by switching modes must not keep advertising the + /// revoked pubkeys. The record retains them on purpose (so the owner can + /// toggle back without retyping), so the projection is what has to drop + /// them — otherwise a revoked association stays publicly readable. + #[test] + fn projection_omits_allowlist_for_non_allowlist_modes() { + let mut agent = sample_agent(); + agent.respond_to = RespondTo::Allowlist; + agent.respond_to_allowlist = vec!["a".repeat(64)]; + assert_eq!( + agent_event_content(&agent).respond_to_allowlist, + vec!["a".repeat(64)], + "allowlist mode must still publish its entries" + ); + + // `nobody` is intentionally absent from this enum (harness-only). + for mode in [RespondTo::OwnerOnly, RespondTo::Anyone] { + let mut revoked = agent.clone(); + revoked.respond_to = mode; + let content = agent_event_content(&revoked); + assert!( + content.respond_to_allowlist.is_empty(), + "{mode:?} must not publish a retained allowlist" + ); + assert!( + !serde_json::to_string(&content) + .unwrap() + .contains(&"a".repeat(64)), + "{mode:?} projection must not carry the revoked pubkey on the wire" + ); + assert_eq!( + revoked.respond_to_allowlist, + vec!["a".repeat(64)], + "the local record keeps its entries for mode round-tripping" + ); + } + } + /// Mutating only runtime fields must NOT change the projection — the /// guarantee that operational start/stop never republishes. #[test] diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs index 372d2cfde1..4539848bb7 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader.rs @@ -205,6 +205,8 @@ pub(crate) fn read_config_surface( RuntimeConfigSurface { runtime_id: runtime_meta.map(|m| m.id.to_string()), runtime_label: runtime_meta.map(|m| m.label.to_string()), + supports_buzz_model_config: runtime_meta + .map(|m| m.model_env_var.is_some() || m.supports_acp_model_switching), is_pre_spawn, normalized, advanced, diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4c11cd6c49..c52f6c804d 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -31,9 +31,19 @@ fn test_runtime() -> &'static KnownAcpRuntime { &KnownAcpRuntime { id: "goose", label: "Goose", + display_label: "Goose", + sort_priority: 1, + onboarding_visible: false, commands: &["goose"], aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, avatar_url: "", + superseded_avatar_urls: &[], mcp_command: None, mcp_hooks: false, underlying_cli: None, @@ -46,10 +56,13 @@ fn test_runtime() -> &'static KnownAcpRuntime { adapter_install_hint: "", skill_dir: None, supports_acp_model_switching: false, + accepts_harness_model: true, model_env_var: Some("GOOSE_MODEL"), provider_env_var: Some("GOOSE_PROVIDER"), provider_locked: false, default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], config_file_path: Some("~/.config/goose/config.yaml"), config_file_format: Some("yaml"), supports_acp_native_config: true, @@ -59,6 +72,7 @@ fn test_runtime() -> &'static KnownAcpRuntime { required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, + auth_login_args: None, } } @@ -131,6 +145,18 @@ fn pre_spawn_surface_reports_pending_acp_tiers() { ConfigTierStatus::Pending ); assert_eq!(surface.sources.env_vars, ConfigTierStatus::Available); + assert_eq!(surface.supports_buzz_model_config, Some(true)); +} + +#[test] +fn devin_surface_projects_runtime_owned_model_capability() { + let record = test_record(); + let runtime = + crate::managed_agents::known_acp_runtime("devin").expect("Devin must remain cataloged"); + let surface = read_config_surface(&record, Some(runtime), None, None); + + assert_eq!(surface.runtime_id.as_deref(), Some("devin")); + assert_eq!(surface.supports_buzz_model_config, Some(false)); } #[test] @@ -607,9 +633,19 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { &KnownAcpRuntime { id: "buzz-agent", label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 0, + onboarding_visible: false, commands: &["buzz-agent"], aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, avatar_url: "", + superseded_avatar_urls: &[], mcp_command: None, mcp_hooks: false, underlying_cli: None, @@ -622,10 +658,13 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { adapter_install_hint: "", skill_dir: None, supports_acp_model_switching: true, + accepts_harness_model: true, model_env_var: Some("BUZZ_AGENT_MODEL"), provider_env_var: Some("BUZZ_AGENT_PROVIDER"), provider_locked: false, default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], config_file_path: None, config_file_format: None, supports_acp_native_config: false, @@ -635,6 +674,7 @@ fn buzz_agent_runtime() -> &'static KnownAcpRuntime { required_normalized_fields: &["model", "provider"], login_hint: None, auth_probe_args: None, + auth_login_args: None, } } diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs index 15ccb718e7..33559aff38 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/types.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/types.rs @@ -132,6 +132,8 @@ pub struct ConfigSourceReport { pub struct RuntimeConfigSurface { pub runtime_id: Option, pub runtime_label: Option, + /// Catalog-projected model capability. `None` for unknown runtimes. + pub supports_buzz_model_config: Option, pub is_pre_spawn: bool, pub normalized: NormalizedConfig, pub advanced: Vec, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 2b7264b429..c903000194 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -7,19 +7,21 @@ use std::time::{Duration, Instant}; use crate::managed_agents::{ buzz_managed_command_path, buzz_managed_node_bin_dir, buzz_managed_npm_bin_dir, AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, CommandAvailabilityInfo, - HarnessSource, + HarnessSource, DEFAULT_AGENT_PARALLELISM, }; +mod catalog_projection; +mod runtime_catalog; mod runtime_metadata; +pub(crate) use catalog_projection::custom_runtime_catalog_entry; +use runtime_catalog::KNOWN_ACP_RUNTIMES; +#[cfg(test)] +use runtime_catalog::{ + BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, +}; pub(crate) use runtime_metadata::KnownAcpRuntime; -const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; -const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; -const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; -const BUZZ_AGENT_AVATAR_URL: &str = - "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; - fn common_binary_paths() -> &'static [PathBuf] { static PATHS: OnceLock> = OnceLock::new(); PATHS.get_or_init(|| { @@ -63,140 +65,6 @@ fn common_binary_paths() -> &'static [PathBuf] { }) } -const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ - KnownAcpRuntime { - id: "goose", - label: "Goose", - commands: &["goose"], - aliases: &[], - avatar_url: GOOSE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("goose"), - cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], - // Goose's stable release currently publishes only the Unix installer; - // its official Windows instructions intentionally point at this main-branch script. - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"$env:CONFIGURE='false'; irm https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1 | iex\""], - adapter_install_commands: &[], - cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", - adapter_install_instructions_url: "", - cli_install_hint: "Buzz requires the Goose CLI; the desktop app alone is not enough.", - adapter_install_hint: "", - skill_dir: Some(".goose/skills"), - supports_acp_model_switching: false, - model_env_var: Some("GOOSE_MODEL"), - provider_env_var: Some("GOOSE_PROVIDER"), - provider_locked: false, - default_env: &[("GOOSE_MODE", "auto")], - config_file_path: Some("~/.config/goose/config.yaml"), - config_file_format: Some("yaml"), - supports_acp_native_config: true, - thinking_env_var: Some("GOOSE_THINKING_EFFORT"), - max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), - context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, - KnownAcpRuntime { - id: "claude", - label: "Claude Code", - commands: &["claude-agent-acp", "claude-code-acp"], - aliases: &["claude-code", "claudecode"], - avatar_url: CLAUDE_CODE_AVATAR_URL, - mcp_command: None, - mcp_hooks: false, - underlying_cli: Some("claude"), - cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://claude.ai/install.ps1 | iex\""], - adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], - cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", - cli_install_hint: "Buzz requires the Claude Code CLI; the desktop app alone is not enough.", - adapter_install_hint: "Install the Claude Code ACP adapter via npm.", - skill_dir: Some(".claude/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: true, - default_env: &[], - config_file_path: Some("~/.claude/settings.json"), - config_file_format: Some("json"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run the Claude CLI to complete authentication."), - auth_probe_args: Some(&["claude", "auth", "status"]), - }, - KnownAcpRuntime { - id: "codex", - label: "Codex", - commands: &["codex-acp"], - aliases: &[], - avatar_url: CODEX_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: false, - underlying_cli: Some("codex"), - cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], - cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://chatgpt.com/codex/install.ps1 | iex\""], - adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], - cli_install_instructions_url: "https://developers.openai.com/codex/cli/", - adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", - cli_install_hint: "Buzz requires the Codex CLI; the desktop app alone is not enough.", - adapter_install_hint: "Install the Codex ACP adapter via npm.", - skill_dir: Some(".codex/skills"), - supports_acp_model_switching: false, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - config_file_path: Some("~/.codex/config.toml"), - config_file_format: Some("toml"), - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: Some("Run `codex login` to authenticate."), - // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. - auth_probe_args: Some(&["codex", "login", "status"]), - }, - KnownAcpRuntime { - id: "buzz-agent", - label: "Buzz Agent", - commands: &["buzz-agent"], - aliases: &[], - avatar_url: BUZZ_AGENT_AVATAR_URL, - mcp_command: Some("buzz-dev-mcp"), - mcp_hooks: true, - underlying_cli: None, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "https://github.com/block/buzz", - adapter_install_instructions_url: "https://github.com/block/buzz", - cli_install_hint: "Ships with the Buzz desktop app.", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: true, - model_env_var: Some("BUZZ_AGENT_MODEL"), - provider_env_var: Some("BUZZ_AGENT_PROVIDER"), - provider_locked: false, - default_env: &[], - config_file_path: None, - config_file_format: None, - supports_acp_native_config: false, - thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), - max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), - context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), - required_normalized_fields: &["model", "provider"], - login_hint: None, - auth_probe_args: None, - }, -]; - /// Skill discovery directories declared by known runtimes. pub(crate) fn known_skill_dirs() -> impl Iterator { KNOWN_ACP_RUNTIMES.iter().filter_map(|p| p.skill_dir) @@ -266,6 +134,12 @@ pub(crate) fn known_acp_runtime_exact(id: &str) -> Option<&'static KnownAcpRunti KNOWN_ACP_RUNTIMES.iter().find(|p| p.id == id) } +pub(crate) fn resolve_agent_parallelism(requested: Option, command: &str) -> u32 { + requested + .or_else(|| known_acp_runtime(command).and_then(|runtime| runtime.default_parallelism)) + .unwrap_or(DEFAULT_AGENT_PARALLELISM) +} + /// The agent command a freshly-created agent defaults to when the create /// request supplies none. Resolves the bundled `buzz-agent` from the catalog so /// the default cannot drift from the provider definition. Falls back to the id @@ -455,12 +329,13 @@ pub fn try_record_agent_command( } fn default_agent_args(command: &str) -> Option> { - match normalize_command_identity(command).as_str() { - "goose" => Some(vec!["acp".to_string()]), - "codex" | "codex-acp" | "claude-agent-acp" | "claude-code-acp" | "claude-code" - | "claudecode" | "buzz-agent" => Some(Vec::new()), - _ => None, - } + known_acp_runtime(command).map(|runtime| { + runtime + .default_args + .iter() + .map(|arg| (*arg).to_string()) + .collect() + }) } pub fn normalize_agent_args(command: &str, agent_args: Vec) -> Vec { @@ -486,8 +361,8 @@ pub fn normalize_agent_args(command: &str, agent_args: Vec) -> Vec [PathBuf; 2] { - if cfg!(debug_assertions) { +fn profile_target_dirs(root: &Path, debug_build: bool) -> [PathBuf; 2] { + if debug_build { // `just dev` builds fresh debug sidecars; never prefer stale release output. [root.join("target/debug"), root.join("target/release")] } else { @@ -495,17 +370,31 @@ fn profile_target_dirs(root: &Path) -> [PathBuf; 2] { } } -fn command_search_dirs() -> Vec { - let mut dirs = profile_target_dirs(&workspace_root_dir()).to_vec(); - if let Ok(current_dir) = std::env::current_dir() { - dirs.extend(profile_target_dirs(¤t_dir)); +fn command_search_dirs_for( + workspace_root: &Path, + current_dir: Option<&Path>, + executable_dir: Option<&Path>, + debug_build: bool, +) -> Vec { + let mut dirs = Vec::new(); + + // A packaged release must run the sidecar that was signed and shipped + // beside the desktop executable. Build-machine checkout paths can still + // exist on a developer Mac; searching them first silently mixes an + // installed release with stale target/debug binaries. + if !debug_build { + dirs.extend(executable_dir.map(Path::to_path_buf)); + } + + dirs.extend(profile_target_dirs(workspace_root, debug_build)); + if let Some(current_dir) = current_dir { + dirs.extend(profile_target_dirs(current_dir, debug_build)); + } + + if debug_build { + dirs.extend(executable_dir.map(Path::to_path_buf)); } - dirs.extend( - std::env::current_exe() - .ok() - .and_then(|path| path.parent().map(Path::to_path_buf)), - ); dirs.into_iter().fold(Vec::new(), |mut unique, dir| { if !unique.contains(&dir) { unique.push(dir); @@ -514,6 +403,19 @@ fn command_search_dirs() -> Vec { }) } +fn command_search_dirs() -> Vec { + let current_dir = std::env::current_dir().ok(); + let executable_dir = std::env::current_exe() + .ok() + .and_then(|path| path.parent().map(Path::to_path_buf)); + command_search_dirs_for( + &workspace_root_dir(), + current_dir.as_deref(), + executable_dir.as_deref(), + cfg!(debug_assertions), + ) +} + fn is_executable_file(path: &Path) -> bool { let Ok(metadata) = path.metadata() else { return false; @@ -534,7 +436,7 @@ fn is_executable_file(path: &Path) -> bool { } } -fn resolve_workspace_command(command: &str) -> Option { +pub(crate) fn resolve_workspace_command(command: &str) -> Option { if command_looks_like_path(command) { let path = PathBuf::from(command); return is_executable_file(&path).then_some(path); @@ -1019,16 +921,18 @@ pub(crate) fn is_npm_global_install(cmd: &str) -> bool { /// background threads to prevent pipe-buffer deadlock. On timeout the child is /// killed and `Unknown` is returned; no orphaned threads or processes are left /// behind. Returns `Unknown` on timeout. -fn probe_auth_status(binary_path: &Path, probe_args: &[&str]) -> AuthStatus { +fn probe_auth_status( + binary_path: &Path, + probe_args: &[&str], + scrub_env_vars: &[&str], +) -> AuthStatus { use crate::managed_agents::readiness::cli_probe; let augmented_path = cli_probe::augmented_path(); let mut command = std::process::Command::new(binary_path); command.args(&probe_args[1..]); - if let Some(ref path) = augmented_path { - command.env("PATH", path); - } + cli_probe::configure_probe_environment(&mut command, augmented_path.as_deref(), scrub_env_vars); command .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) @@ -1326,10 +1230,11 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr .and_then(find_command) .map(|p| p.display().to_string()); - let default_args = command - .as_deref() - .map(|cmd| normalize_agent_args(cmd, Vec::new())) - .unwrap_or_default(); + let default_args = runtime + .default_args + .iter() + .map(|arg| (*arg).to_string()) + .collect(); let can_auto_install = !runtime.cli_install_commands_for_os().is_empty() || !runtime.adapter_install_commands.is_empty(); @@ -1376,7 +1281,19 @@ fn discover_acp_runtime_phase1(runtime: &'static KnownAcpRuntime) -> PartialEntr entry: AcpRuntimeCatalogEntry { id: runtime.id.to_string(), label: runtime.label.to_string(), + display_label: runtime.display_label.to_string(), + sort_priority: runtime.sort_priority, + onboarding_visible: runtime.onboarding_visible, + icon_url: runtime.icon_url.to_string(), + icon_scale: runtime.icon_scale, avatar_url: runtime.avatar_url.to_string(), + superseded_avatar_urls: runtime + .superseded_avatar_urls + .iter() + .map(|url| (*url).to_string()) + .collect(), + supports_buzz_model_config: runtime.model_env_var.is_some() + || runtime.supports_acp_model_switching, availability, command, binary_path, @@ -1483,8 +1400,15 @@ fn preset_catalog_entry( AcpRuntimeCatalogEntry { id: def.id.to_string(), label: def.label.to_string(), + display_label: def.label.to_string(), + sort_priority: 100, + onboarding_visible: false, + icon_url: String::new(), + icon_scale: 1.0, // No remote URL — all preset icons are bundled assets. avatar_url: String::new(), + superseded_avatar_urls: Vec::new(), + supports_buzz_model_config: true, availability, command, binary_path, @@ -1669,10 +1593,11 @@ pub fn discover_acp_runtimes_from( // Need the resolved binary path for the CLI (e.g. the actual `claude` binary). let binary_path = resolve_command(probe_args[0])?; let probe_args_owned: Vec = probe_args.iter().map(|s| s.to_string()).collect(); + let scrub_env_vars = partial.runtime.scrub_env_vars; let handle = std::thread::spawn(move || { let refs: Vec<&str> = probe_args_owned.iter().map(String::as_str).collect(); - probe_auth_status(&binary_path, &refs) + probe_auth_status(&binary_path, &refs, scrub_env_vars) }); Some((idx, handle)) }) @@ -1746,37 +1671,13 @@ pub fn discover_acp_runtimes_from( let default_args = normalize_agent_args(&def.command, def.args.clone()); - entries.push(AcpRuntimeCatalogEntry { - id: def.id.clone(), - label: def.label.clone(), - // F1 security fix: never copy user-supplied avatar URL into the catalog. - // All icons are bundled assets; customs fall back to TerminalSquare in the UI. - avatar_url: String::new(), + entries.push(custom_runtime_catalog_entry( + def, availability, command, binary_path, default_args, - // Custom harnesses are plain ACP — no MCP sidecar, no env-var - // model switching, no thinking knobs. - mcp_command: None, - model_env_var: None, - provider_env_var: None, - thinking_env_var: None, - install_hint: def.install_hint.clone(), - install_instructions_url: def.install_instructions_url.clone(), - // Security line: custom definitions carry no install scripts. - can_auto_install: false, - requires_external_cli: false, - underlying_cli_path: None, - node_required: false, - // No auth probe for custom harnesses. - auth_status: AuthStatus::NotApplicable, - login_hint: None, - source: HarnessSource::Custom, - // Carry definition env into the catalog so the edit form can - // read it back — prevents silently erasing env on save. - definition_env: def.env.clone(), - }); + )); } } @@ -1830,5 +1731,19 @@ pub fn managed_agent_avatar_url(command: &str) -> Option { Some(runtime.avatar_url.to_string()) } +/// Replace a superseded catalog-default avatar without touching user-selected +/// images. This is intentionally a read-time normalization: existing records +/// render correctly immediately, their relay profiles reconcile to the new +/// default, and the normalized value is persisted on the next ordinary save. +pub fn normalize_managed_agent_avatar(command: &str, avatar_url: Option) -> Option { + if let (Some(runtime), Some(avatar)) = (known_acp_runtime(command), avatar_url.as_deref()) { + if runtime.superseded_avatar_urls.contains(&avatar) { + return Some(runtime.avatar_url.to_string()); + } + } + + avatar_url +} + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs b/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs new file mode 100644 index 0000000000..71a4eee221 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/catalog_projection.rs @@ -0,0 +1,45 @@ +use crate::managed_agents::{ + custom_harnesses::HarnessDefinition, AcpAvailabilityStatus, AcpRuntimeCatalogEntry, AuthStatus, + HarnessSource, +}; + +pub(crate) fn custom_runtime_catalog_entry( + definition: HarnessDefinition, + availability: AcpAvailabilityStatus, + command: Option, + binary_path: Option, + default_args: Vec, +) -> AcpRuntimeCatalogEntry { + AcpRuntimeCatalogEntry { + id: definition.id, + display_label: definition.label.clone(), + label: definition.label, + sort_priority: 100, + onboarding_visible: false, + icon_url: String::new(), + icon_scale: 1.0, + // User-controlled custom avatar URLs never enter the catalog. + avatar_url: String::new(), + superseded_avatar_urls: Vec::new(), + // Preserve the established custom-harness model surface. + supports_buzz_model_config: true, + availability, + command, + binary_path, + default_args, + mcp_command: None, + model_env_var: None, + provider_env_var: None, + thinking_env_var: None, + install_hint: definition.install_hint, + install_instructions_url: definition.install_instructions_url, + can_auto_install: false, + requires_external_cli: false, + underlying_cli_path: None, + node_required: false, + auth_status: AuthStatus::NotApplicable, + login_hint: None, + source: HarnessSource::Custom, + definition_env: definition.env, + } +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs new file mode 100644 index 0000000000..7416f125fb --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_catalog.rs @@ -0,0 +1,286 @@ +use super::KnownAcpRuntime; + +pub(super) const GOOSE_AVATAR_URL: &str = "https://goose-docs.ai/img/logo_dark.png"; +pub(super) const CLAUDE_CODE_AVATAR_URL: &str = "https://anthropic.gallerycdn.vsassets.io/extensions/anthropic/claude-code/2.1.77/1773707456892/Microsoft.VisualStudio.Services.Icons.Default"; +pub(super) const CODEX_AVATAR_URL: &str = "https://openai.gallerycdn.vsassets.io/extensions/openai/chatgpt/26.5313.41514/1773706730621/Microsoft.VisualStudio.Services.Icons.Default"; +pub(super) const LEGACY_DEVIN_AVATAR_URL: &str = "https://mintcdn.com/cognitionai/Hhrl_8XUBqA4VQ6v/logo/favicon.svg?fit=max&auto=format&n=Hhrl_8XUBqA4VQ6v&q=85&s=ab641f30c01bf5374b90b62209db569e"; +// The official Devin mark is transparent. Keep the profile avatar self-contained +// and add a white canvas so the black mark remains visible in dark themes. +pub(super) const DEVIN_AVATAR_URL: &str = concat!( + "data:image/svg+xml,%3Csvg%20width%3D%22425%22%20height%3D%22425%22%20viewBox%3D%220%200%20425%20425%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E", + "%3Crect%20width%3D%22425%22%20height%3D%22425%22%20fill%3D%22white%22%2F%3E", + "%3Cpath%20d%3D%22M70%20159.333V91.3471C70%2088.3592%2071.594%2085.5983%2074.1816%2084.1044L133.043%2050.1205C135.631%2048.6265%20138.819%2048.6265%20141.407%2050.1205L200.269%2084.1044C202.856%2085.5983%20204.45%2088.3592%20204.45%2091.3471V126.068C204.708%20137.606%20210.806%20148.734%20221.531%20154.926C232.256%20161.117%20244.942%20160.834%20255.063%20155.289L285.132%20137.929C287.719%20136.435%20290.907%20136.435%20293.495%20137.929L352.357%20171.913C354.944%20173.406%20356.538%20176.167%20356.538%20179.155V247.123C356.538%20250.111%20354.944%20252.872%20352.357%20254.366L293.495%20288.35C290.907%20289.844%20287.719%20289.844%20285.132%20288.35L255.306%20271.13C245.146%20265.456%20232.344%20265.117%20221.534%20271.358C210.809%20277.55%20204.711%20288.678%20204.453%20300.215V334.926C204.453%20337.914%20202.859%20340.675%20200.271%20342.169L141.41%20376.153C138.822%20377.647%20135.634%20377.647%20133.046%20376.153L74.1845%20342.169C71.5969%20340.675%2070.0028%20337.914%2070.0028%20334.926V266.959C70.0029%20263.971%2071.5969%20261.21%2074.1845%20259.716L133.046%20225.732C135.634%20224.238%20138.822%20224.238%20141.41%20225.732L171.547%20243.132C181.656%20248.638%20194.306%20248.906%20205.005%20242.729C215.815%20236.488%20221.922%20225.231%20222.088%20213.595C221.83%20202.057%20215.732%20189.737%20205.008%20183.545C194.283%20177.353%20181.597%20177.636%20171.476%20183.181L141.269%20200.72C138.67%20202.229%20135.461%20202.228%20132.864%20200.716L74.1576%20166.562C71.5835%20165.065%2070%20162.311%2070%20159.333Z%22%20fill%3D%22black%22%2F%3E", + "%3C%2Fsvg%3E" +); +pub(super) const BUZZ_AGENT_AVATAR_URL: &str = + "https://raw.githubusercontent.com/block/buzz/refs/heads/main/crates/buzz-agent/buzz-agent.png"; + +pub(super) const KNOWN_ACP_RUNTIMES: &[KnownAcpRuntime] = &[ + KnownAcpRuntime { + id: "goose", + label: "Goose", + display_label: "Goose", + sort_priority: 50, + onboarding_visible: true, + commands: &["goose"], + aliases: &[], + default_args: &["acp"], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/goose.svg", + icon_scale: 1.25, + avatar_url: GOOSE_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("goose"), + cli_install_commands: &["curl -fsSL https://github.com/aaif-goose/goose/releases/download/stable/download_cli.sh | CONFIGURE=false bash"], + // Goose's stable release currently publishes only the Unix installer; + // its official Windows instructions intentionally point at this main-branch script. + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"$env:CONFIGURE='false'; irm https://raw.githubusercontent.com/aaif-goose/goose/main/download_cli.ps1 | iex\""], + adapter_install_commands: &[], + cli_install_instructions_url: "https://goose-docs.ai/docs/getting-started/installation/", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz requires the Goose CLI; the desktop app alone is not enough.", + adapter_install_hint: "", + skill_dir: Some(".goose/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: Some("GOOSE_MODEL"), + provider_env_var: Some("GOOSE_PROVIDER"), + provider_locked: false, + default_env: &[("GOOSE_MODE", "auto")], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.config/goose/config.yaml"), + config_file_format: Some("yaml"), + supports_acp_native_config: true, + thinking_env_var: Some("GOOSE_THINKING_EFFORT"), + max_tokens_env_var: Some("GOOSE_MAX_TOKENS"), + context_limit_env_var: Some("GOOSE_CONTEXT_LIMIT"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + }, + KnownAcpRuntime { + id: "claude", + label: "Claude Code", + display_label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, + commands: &["claude-agent-acp", "claude-code-acp"], + aliases: &["claude-code", "claudecode"], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/claude.png", + icon_scale: 1.1, + avatar_url: CLAUDE_CODE_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("claude"), + cli_install_commands: &["curl -fsSL https://claude.ai/install.sh | bash"], + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://claude.ai/install.ps1 | iex\""], + adapter_install_commands: &["npm install -g @agentclientprotocol/claude-agent-acp"], + cli_install_instructions_url: "https://code.claude.com/docs/en/getting-started", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/claude-agent-acp", + cli_install_hint: "Buzz requires the Claude Code CLI; the desktop app alone is not enough.", + adapter_install_hint: "Install the Claude Code ACP adapter via npm.", + skill_dir: Some(".claude/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: true, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.claude/settings.json"), + config_file_format: Some("json"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run the Claude CLI to complete authentication."), + auth_probe_args: Some(&["claude", "auth", "status"]), + auth_login_args: None, + }, + KnownAcpRuntime { + id: "codex", + label: "Codex", + display_label: "Codex", + sort_priority: 40, + onboarding_visible: true, + commands: &["codex-acp"], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/runtime-icons/codex.png", + icon_scale: 1.1, + avatar_url: CODEX_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: false, + underlying_cli: Some("codex"), + cli_install_commands: &["curl -fsSL https://chatgpt.com/codex/install.sh | sh"], + cli_install_commands_windows: &["powershell.exe -NoProfile -ExecutionPolicy Bypass -Command \"irm https://chatgpt.com/codex/install.ps1 | iex\""], + adapter_install_commands: &["npm install -g @agentclientprotocol/codex-acp"], + cli_install_instructions_url: "https://developers.openai.com/codex/cli/", + adapter_install_instructions_url: "https://github.com/agentclientprotocol/codex-acp", + cli_install_hint: "Buzz requires the Codex CLI; the desktop app alone is not enough.", + adapter_install_hint: "Install the Codex ACP adapter via npm.", + skill_dir: Some(".codex/skills"), + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: Some("~/.codex/config.toml"), + config_file_format: Some("toml"), + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `codex login` to authenticate."), + // Verified: `codex login status` exits 0 when logged in, non-zero otherwise. + auth_probe_args: Some(&["codex", "login", "status"]), + auth_login_args: None, + }, + KnownAcpRuntime { + id: "devin", + label: "Devin", + display_label: "Devin", + sort_priority: 20, + onboarding_visible: true, + commands: &["devin"], + aliases: &[], + default_args: &["acp"], + // One official CLI session is the safe local default. Explicit request + // or persona values still win. + default_parallelism: Some(1), + // The official CLI performs meaningful startup work before its first + // prompt. Pay that cost when the managed runtime starts so the first + // user message does not also become a process-health probe. + defer_agent_start_until_work: false, + // Devin normally emits ACP progress well inside this window. A fully + // silent native server is replaced and the queued batch retried rather + // than appearing to hang under the generic 15-minute tool allowance. + default_idle_timeout_secs: Some(120), + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + avatar_url: DEVIN_AVATAR_URL, + superseded_avatar_urls: &[LEGACY_DEVIN_AVATAR_URL], + mcp_command: None, + mcp_hooks: false, + underlying_cli: Some("devin"), + cli_install_commands: &["curl -fsSL https://cli.devin.ai/install.sh | bash"], + cli_install_commands_windows: &[ + "powershell.exe -NoProfile -Command \"irm https://static.devin.ai/cli/setup.ps1 | iex\"", + ], + adapter_install_commands: &[], + cli_install_instructions_url: "https://docs.devin.ai/cli", + adapter_install_instructions_url: "", + cli_install_hint: "Buzz requires the Devin CLI; the desktop app alone is not enough.", + adapter_install_hint: "", + skill_dir: Some(".devin/skills"), + supports_acp_model_switching: false, + // Devin's native ACP server owns model choice. Passing Buzz's global + // model would imply a capability we do not expose and causes the + // official CLI to reject unrelated Buzz model IDs before falling back. + accepts_harness_model: false, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + // Buzz's harness historically defaults to bypassing ACP permission + // requests. The native Devin runtime must always retain the official + // CLI's safe permission behavior. + enforced_env: &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ], + // WINDSURF_API_KEY: the official CLI gives this legacy ambient key + // precedence over the account established by `devin auth login`. Remove + // it without reading its value so readiness and usage attribution share + // one identity. + // + // ACP_BACKEND: set by the Devin IDE for its own ACP integration. When it + // leaks in — Buzz launched from a terminal inside that IDE, for example + // — `devin acp` switches to "ACP host is the sole source of + // credentials", refuses the stored CLI credentials, and fails every turn + // with "ACP host has not authenticated" even though `devin auth login` + // succeeded. Ambient state must not redefine the adapter's credential + // policy. + scrub_env_vars: &["WINDSURF_API_KEY", "ACP_BACKEND"], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: Some("Run `devin auth login` to authenticate."), + // Verified locally: the command exits 0 for an authenticated CLI. + auth_probe_args: Some(&["devin", "auth", "status"]), + auth_login_args: Some(&["devin", "auth", "login"]), + }, + KnownAcpRuntime { + id: "buzz-agent", + label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 60, + onboarding_visible: true, + commands: &["buzz-agent"], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "/app-icon@2x.png", + icon_scale: 1.1, + avatar_url: BUZZ_AGENT_AVATAR_URL, + superseded_avatar_urls: &[], + mcp_command: Some("buzz-dev-mcp"), + mcp_hooks: true, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "https://github.com/block/buzz", + adapter_install_instructions_url: "https://github.com/block/buzz", + cli_install_hint: "Ships with the Buzz desktop app.", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: true, + accepts_harness_model: true, + model_env_var: Some("BUZZ_AGENT_MODEL"), + provider_env_var: Some("BUZZ_AGENT_PROVIDER"), + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: Some("BUZZ_AGENT_THINKING_EFFORT"), + max_tokens_env_var: Some("BUZZ_AGENT_MAX_OUTPUT_TOKENS"), + context_limit_env_var: Some("BUZZ_AGENT_MAX_CONTEXT_TOKENS"), + required_normalized_fields: &["model", "provider"], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + }, +]; diff --git a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs index 2fb6a471d4..6b215da6d3 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/runtime_metadata.rs @@ -2,9 +2,38 @@ pub(crate) struct KnownAcpRuntime { pub id: &'static str, pub label: &'static str, + /// Compact product label used by runtime pickers. + pub display_label: &'static str, + /// Stable catalog ordering before the label tie-breaker. + pub sort_priority: u16, + /// Whether first-run onboarding should offer this runtime. + pub onboarding_visible: bool, pub commands: &'static [&'static str], pub aliases: &'static [&'static str], + /// Arguments used when the runtime is launched without an explicit argv. + pub default_args: &'static [&'static str], + /// Runtime-specific worker default used when neither the request nor a + /// linked persona specifies parallelism. `None` preserves Buzz's global + /// default. + pub default_parallelism: Option, + /// Whether a desktop-requested lazy harness should defer spawning the ACP + /// subprocess until accepted work is queued. Runtimes with expensive or + /// failure-prone first handshakes can opt out while preserving the lazy + /// relay socket. + pub defer_agent_start_until_work: bool, + /// Runtime-specific idle timeout used only when the agent record, process + /// environment, and merged user environment do not supply an override. + /// `None` preserves the harness default. + pub default_idle_timeout_secs: Option, + /// App-local runtime mark used by catalog-driven frontend surfaces. + pub icon_url: &'static str, + /// Presentation scale for the runtime mark. Kept with the catalog entry so + /// React does not need a harness-ID lookup table. + pub icon_scale: f32, pub avatar_url: &'static str, + /// Catalog-default avatar URLs superseded by `avatar_url`. These are not + /// user-selected images and may be replaced during read-time migration. + pub superseded_avatar_urls: &'static [&'static str], /// Legacy MCP server binary field. Vestigial — all agents now use the bundled CLI /// directly. Will be removed when runtime discovery is simplified. pub mcp_command: Option<&'static str>, @@ -34,14 +63,28 @@ pub(crate) struct KnownAcpRuntime { /// runtime reads the canonical path directly or has no skill support. pub skill_dir: Option<&'static str>, /// Whether this runtime handles model switching via ACP protocol natively. - /// Currently unused — env var injection runs unconditionally regardless of - /// this value. Retained as scaffolding for when ACP model switching matures. - #[allow(dead_code)] + /// Env var injection still handles initial model selection separately. pub supports_acp_model_switching: bool, + /// Whether Buzz should pass its resolved model through the generic + /// `BUZZ_ACP_MODEL` harness setting at process launch. + /// + /// This is intentionally separate from `supports_acp_model_switching` and + /// `model_env_var`: existing adapters may consume the generic bootstrap + /// model without exposing Buzz-side model controls. Native runtimes whose + /// official ACP server owns model selection set this to `false`. + pub accepts_harness_model: bool, pub model_env_var: Option<&'static str>, pub provider_env_var: Option<&'static str>, pub provider_locked: bool, + /// Environment defaults applied only when neither the parent process nor + /// saved agent configuration supplies a value. pub default_env: &'static [(&'static str, &'static str)], + /// Environment values enforced at process launch after inherited and + /// user-configured values have been merged. + pub enforced_env: &'static [(&'static str, &'static str)], + /// Environment variables removed from runtime subprocesses. This prevents + /// ambient process state from overriding catalog-declared identity policy. + pub scrub_env_vars: &'static [&'static str], pub config_file_path: Option<&'static str>, #[allow(dead_code)] // reserved for format-based dispatch when readers are unified pub config_file_format: Option<&'static str>, @@ -62,6 +105,9 @@ pub(crate) struct KnownAcpRuntime { /// CLI args for probing authentication status. `args[0]` is the binary name; /// the remainder are the subcommand. `None` for runtimes with no login step. pub auth_probe_args: Option<&'static [&'static str]>, + /// CLI argv for an interactive login launched in a visible terminal. + /// `None` when authentication is adapter-owned or not applicable. + pub auth_login_args: Option<&'static [&'static str]>, } impl KnownAcpRuntime { @@ -85,6 +131,23 @@ impl KnownAcpRuntime { mod tests { use super::super::known_acp_runtime_exact; + #[test] + fn onboarding_visibility_and_order_are_catalog_owned() { + let mut visible = super::super::KNOWN_ACP_RUNTIMES + .iter() + .filter(|runtime| runtime.onboarding_visible) + .collect::>(); + visible.sort_by_key(|runtime| runtime.sort_priority); + + assert_eq!( + visible + .into_iter() + .map(|runtime| runtime.id) + .collect::>(), + ["devin", "claude", "codex", "goose", "buzz-agent"] + ); + } + #[test] fn vendor_metadata_distinguishes_cli_and_adapter_guidance() { let goose = known_acp_runtime_exact("goose").unwrap(); @@ -120,5 +183,51 @@ mod tests { ); assert!(codex.adapter_install_instructions_url.contains("codex-acp")); assert!(codex.cli_install_hint.contains("desktop app alone")); + + let devin = known_acp_runtime_exact("devin").unwrap(); + assert_eq!(devin.commands, &["devin"]); + assert_eq!(devin.default_args, &["acp"]); + assert_eq!(devin.default_parallelism, Some(1)); + assert!(!devin.defer_agent_start_until_work); + assert_eq!(devin.default_idle_timeout_secs, Some(120)); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!(devin.underlying_cli, Some("devin")); + assert_eq!(devin.skill_dir, Some(".devin/skills")); + assert_eq!( + devin.auth_probe_args, + Some(&["devin", "auth", "status"][..]) + ); + assert_eq!(devin.auth_login_args, Some(&["devin", "auth", "login"][..])); + assert!(devin.default_env.is_empty()); + assert_eq!( + devin.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + assert_eq!(devin.scrub_env_vars, &["WINDSURF_API_KEY", "ACP_BACKEND"]); + assert_eq!( + devin.cli_install_instructions_url, + "https://docs.devin.ai/cli" + ); + assert_eq!( + devin.cli_install_commands, + &["curl -fsSL https://cli.devin.ai/install.sh | bash"] + ); + assert_eq!( + devin.cli_install_commands_windows, + &[ + "powershell.exe -NoProfile -Command \"irm https://static.devin.ai/cli/setup.ps1 | iex\"" + ] + ); + assert!(devin.adapter_install_commands.is_empty()); + assert!(devin.cli_install_hint.contains("desktop app alone")); } } diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 48e8d5479c..726d5e9977 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -6,43 +6,15 @@ use super::{ codex_adapter_is_outdated, create_time_agent_command_override, default_agent_command, effective_agent_command, find_nvm_default_bin, find_via_login_shell, is_login_shell_path_uninit, is_safe_nvm_tag, managed_agent_avatar_url, normalize_agent_args, - parse_semver_tag, preset_catalog_entry, probe_codex_acp_major_version, record_agent_command, - refresh_login_shell_path, try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, - CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL, + normalize_managed_agent_avatar, parse_semver_tag, preset_catalog_entry, + probe_codex_acp_major_version, record_agent_command, refresh_login_shell_path, + try_record_agent_command, PresetHarness, BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, + CODEX_AVATAR_URL, GOOSE_AVATAR_URL, }; use crate::managed_agents::AcpAvailabilityStatus; -#[test] -fn resolves_known_avatar_for_bare_command() { - let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); - - assert_eq!(avatar_url, GOOSE_AVATAR_URL); -} - -#[test] -fn resolves_known_avatar_for_command_paths_and_aliases() { - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/codex-acp"), - Some(CODEX_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("Claude Code"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); - assert_eq!( - managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), - Some(CLAUDE_CODE_AVATAR_URL.to_string()) - ); -} - -#[test] -fn returns_none_for_unknown_commands() { - assert!(managed_agent_avatar_url("custom-agent").is_none()); -} +mod avatar; +mod devin; #[test] fn default_agent_command_resolves_bundled_buzz_agent() { diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs new file mode 100644 index 0000000000..af40349d01 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/avatar.rs @@ -0,0 +1,33 @@ +use super::*; + +#[test] +fn resolves_known_avatar_for_bare_command() { + let avatar_url = managed_agent_avatar_url("goose").expect("goose avatar should resolve"); + + assert_eq!(avatar_url, GOOSE_AVATAR_URL); +} + +#[test] +fn resolves_known_avatar_for_command_paths_and_aliases() { + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/codex-acp"), + Some(CODEX_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("Claude Code"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url(r"C:\Tools\claude-agent-acp.exe"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); + assert_eq!( + managed_agent_avatar_url("/usr/local/bin/claude-code-acp"), + Some(CLAUDE_CODE_AVATAR_URL.to_string()) + ); +} + +#[test] +fn returns_none_for_unknown_commands() { + assert!(managed_agent_avatar_url("custom-agent").is_none()); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs new file mode 100644 index 0000000000..6530414a95 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/devin.rs @@ -0,0 +1,118 @@ +use super::*; + +#[test] +fn resolves_devin_avatar() { + let avatar_url = + managed_agent_avatar_url("/usr/local/bin/devin").expect("Devin avatar should resolve"); + + assert_eq!(avatar_url, super::super::runtime_catalog::DEVIN_AVATAR_URL); + assert!(avatar_url.starts_with("data:image/svg+xml,")); + assert!(avatar_url.contains("fill%3D%22white%22")); +} + +#[test] +fn migrates_only_the_superseded_devin_default_avatar() { + let migrated = normalize_managed_agent_avatar( + "devin", + Some(super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL.to_string()), + ); + assert_eq!( + migrated.as_deref(), + Some(super::super::runtime_catalog::DEVIN_AVATAR_URL) + ); + + let custom = Some("https://example.test/custom-devin.png".to_string()); + assert_eq!( + normalize_managed_agent_avatar("devin", custom.clone()), + custom + ); + + let other_runtime = Some(super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL.to_string()); + assert_eq!( + normalize_managed_agent_avatar("goose", other_runtime.clone()), + other_runtime + ); +} + +#[test] +fn normalizes_devin_args_to_native_acp_subcommand() { + assert_eq!(normalize_agent_args("devin", Vec::new()), vec!["acp"]); + assert_eq!( + normalize_agent_args("/usr/local/bin/devin", vec!["".into()]), + vec!["acp"] + ); + assert_eq!( + normalize_agent_args( + "devin", + vec!["acp".into(), "--agent-type".into(), "review".into()] + ), + vec!["acp", "--agent-type", "review"] + ); +} + +#[test] +fn runtime_catalog_exposes_devin_once() { + let devin_entries = super::super::KNOWN_ACP_RUNTIMES + .iter() + .filter(|runtime| runtime.id == "devin") + .collect::>(); + + assert_eq!(devin_entries.len(), 1); + let devin = devin_entries[0]; + assert_eq!(devin.label, "Devin"); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.commands, &["devin"]); + assert_eq!(devin.default_args, &["acp"]); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!(devin.underlying_cli, Some("devin")); + assert_eq!(devin.skill_dir, Some(".devin/skills")); + assert_eq!( + devin.auth_probe_args, + Some(&["devin", "auth", "status"][..]) + ); + assert!(devin.default_env.is_empty()); + assert_eq!( + devin.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + // ACP_BACKEND must stay scrubbed: inherited from the Devin IDE it flips the + // adapter to host-supplied credentials only, and every turn then fails with + // "ACP host has not authenticated" despite a valid `devin auth login`. + assert_eq!( + devin.scrub_env_vars, + &["WINDSURF_API_KEY", "ACP_BACKEND"], + "ambient IDE state must not redefine Devin's credential policy" + ); + assert!(!devin.supports_acp_model_switching); + assert!(!devin.accepts_harness_model); +} + +#[test] +fn runtime_discovery_exposes_devin_entry() { + let runtimes = super::super::discover_acp_runtimes_from(None); + let devin = runtimes + .iter() + .find(|runtime| runtime.id == "devin") + .expect("runtime discovery must project the Devin catalog entry"); + + assert_eq!(devin.label, "Devin"); + assert_eq!(devin.display_label, "Devin"); + assert_eq!(devin.sort_priority, 20); + assert!(devin.onboarding_visible); + assert_eq!(devin.icon_url, "/runtime-icons/devin.svg"); + assert_eq!(devin.icon_scale, 1.1); + assert_eq!( + devin.superseded_avatar_urls, + [super::super::runtime_catalog::LEGACY_DEVIN_AVATAR_URL] + ); + assert!(!devin.supports_buzz_model_config); + assert_eq!(devin.default_args, ["acp"]); +} diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs index 2f6b038deb..6995754ab5 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests/managed_path_resolution.rs @@ -1,4 +1,38 @@ use crate::managed_agents::discovery::{clear_resolve_cache, resolve_command}; +use std::path::{Path, PathBuf}; + +#[test] +fn packaged_release_prefers_its_bundled_sidecars_over_checkout_targets() { + let bundle = PathBuf::from("/Applications/Buzz.app/Contents/MacOS"); + let dirs = super::super::command_search_dirs_for( + Path::new("/build/buzz"), + Some(Path::new("/Users/developer/buzz")), + Some(&bundle), + false, + ); + + assert_eq!(dirs.first(), Some(&bundle)); + assert_eq!(dirs[1], PathBuf::from("/build/buzz/target/release")); + assert_eq!(dirs[2], PathBuf::from("/build/buzz/target/debug")); +} + +#[test] +fn debug_build_keeps_fresh_workspace_sidecars_ahead_of_executable_dir() { + let bundle = PathBuf::from("/build/buzz/target/debug"); + let dirs = super::super::command_search_dirs_for( + Path::new("/build/buzz"), + Some(Path::new("/build/buzz/desktop")), + Some(&bundle), + true, + ); + + assert_eq!(dirs.first(), Some(&bundle)); + assert_eq!(dirs[1], PathBuf::from("/build/buzz/target/release")); + assert_eq!( + dirs.last(), + Some(&PathBuf::from("/build/buzz/desktop/target/release")) + ); +} #[cfg(unix)] #[test] diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c5480b2479..c2bbc187ca 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -390,9 +390,9 @@ impl AgentReadiness { /// - `openai` → `OPENAI_COMPAT_API_KEY` /// - `databricks` / `databricks_v2` → `DATABRICKS_HOST` (token optional — /// OAuth PKCE is the fallback) -/// * **claude**: a successful `claude auth status` probe. -/// * **codex**: a successful `codex login status` probe (checks the codex -/// credential store — NOT `OPENAI_API_KEY`). +/// * **CLI-login runtimes**: a successful catalog-declared authentication +/// probe (for example `claude auth status`, `codex login status`, or +/// `devin auth status`). /// * **unknown / custom command**: always `Ready` (no requirements known). /// /// Databricks note: `DATABRICKS_TOKEN` is `.unwrap_or_default()` in @@ -427,7 +427,7 @@ fn collect_missing_requirements( return vec![]; }; - match rt.id { + let runtime_specific = match rt.id { "buzz-agent" => buzz_agent_requirements(effective), "goose" => { // Read the file config once at the call site so the inner fn is @@ -435,14 +435,21 @@ fn collect_missing_requirements( let file_cfg = read_goose_file_config(); goose_requirements(effective, file_cfg.as_ref()) } - "claude" => cli_login::requirements( - &["claude", "auth", "status"], - "complete Claude Code authentication by running the Claude CLI", - rt, - ), - "codex" => cli_login::requirements(&["codex", "login", "status"], "run `codex login`", rt), _ => vec![], + }; + if !runtime_specific.is_empty() || matches!(rt.id, "buzz-agent" | "goose") { + return runtime_specific; } + + let Some(probe_args) = rt.auth_probe_args else { + return vec![]; + }; + cli_login::requirements( + probe_args, + rt.login_hint + .unwrap_or("Complete authentication in the runtime CLI."), + rt, + ) } /// Requirements for buzz-agent (provider + model + provider-specific creds). @@ -645,6 +652,9 @@ mod tests { use super::*; use crate::managed_agents::discovery::known_acp_runtime_exact; + mod devin_tests; + mod fixtures; + /// Build a minimal `EffectiveAgentEnv` with the given env map and command. fn make_env(command: &str, env: BTreeMap) -> EffectiveAgentEnv { let runtime = known_acp_runtime_exact(command); @@ -1012,34 +1022,10 @@ mod tests { KnownAcpRuntime { id: "test-cli-runtime", label: "Test CLI", + display_label: "Test CLI", commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, + ..fixtures::known_runtime_fixture() } } @@ -1207,34 +1193,11 @@ mod tests { KnownAcpRuntime { id: "codex", label: "Codex", + display_label: "Codex", commands: adapter_commands, - aliases: &[], - avatar_url: "", - mcp_command: None, - mcp_hooks: false, underlying_cli, - cli_install_commands: &[], - cli_install_commands_windows: &[], - adapter_install_commands: &[], - cli_install_instructions_url: "", - adapter_install_instructions_url: "", - cli_install_hint: "", - adapter_install_hint: "", - skill_dir: None, - supports_acp_model_switching: false, - config_file_path: None, - config_file_format: None, - model_env_var: None, - provider_env_var: None, - provider_locked: false, - default_env: &[], - supports_acp_native_config: false, - thinking_env_var: None, - max_tokens_env_var: None, - context_limit_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, + onboarding_visible: true, + ..fixtures::known_runtime_fixture() } } diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs index 4036d9f239..2c0bbb4312 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_login.rs @@ -10,7 +10,7 @@ use crate::managed_agents::{ use super::{cli_probe, Requirement}; -/// Requirements for CLI-login runtimes (claude, codex). +/// Requirements for runtimes with a catalog-declared CLI authentication probe. pub(super) fn requirements( probe_args: &[&str], setup_copy: &str, @@ -47,7 +47,12 @@ pub(super) fn requirements( )]; }; let augmented_path = cli_probe::augmented_path(); - match cli_probe::login_probe(&binary_path, probe_args, augmented_path.as_deref()) { + match cli_probe::login_probe( + &binary_path, + probe_args, + augmented_path.as_deref(), + runtime.scrub_env_vars, + ) { cli_probe::ProbeOutcome::LoggedIn => vec![], cli_probe::ProbeOutcome::LoggedOut => vec![missing_requirement( probe_args, diff --git a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs index 513da4e2a8..247ddb09a1 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/cli_probe.rs @@ -57,12 +57,11 @@ pub(crate) fn login_probe( binary_path: &Path, probe_args: &[&str], augmented_path: Option<&str>, + scrub_env_vars: &[&str], ) -> ProbeOutcome { let mut command = std::process::Command::new(binary_path); command.args(&probe_args[1..]); - if let Some(path) = augmented_path { - command.env("PATH", path); - } + configure_probe_environment(&mut command, augmented_path, scrub_env_vars); crate::util::configure_no_window(&mut command); match command.output() { @@ -72,6 +71,19 @@ pub(crate) fn login_probe( } } +pub(crate) fn configure_probe_environment( + command: &mut std::process::Command, + augmented_path: Option<&str>, + scrub_env_vars: &[&str], +) { + if let Some(path) = augmented_path { + command.env("PATH", path); + } + for key in scrub_env_vars { + command.env_remove(key); + } +} + /// Classify collected probe output into a `ProbeOutcome`. /// /// Shared between `login_probe` (which has the full `Output`) and the @@ -100,6 +112,18 @@ pub(crate) fn classify_probe_output(stderr_bytes: &[u8], exit_success: bool) -> mod tests { use super::{ProbeOutcome, CONFIG_PARSE_SIGNALS}; + #[test] + fn probe_environment_removes_catalog_declared_identity_overrides() { + let mut command = std::process::Command::new("devin"); + command.env("WINDSURF_API_KEY", "sentinel"); + + super::configure_probe_environment(&mut command, None, &["WINDSURF_API_KEY"]); + + assert!(command + .get_envs() + .any(|(key, value)| { key == "WINDSURF_API_KEY" && value.is_none() })); + } + #[cfg(unix)] #[test] fn login_probe_uses_augmented_path_for_env_shebang_interpreter() { @@ -154,6 +178,7 @@ mod tests { &script_path, &["fake-codex", "login", "status"], Some(&augmented_path), + &[], ), ProbeOutcome::LoggedIn, "the injected augmented PATH should allow /usr/bin/env to find the interpreter" @@ -187,6 +212,7 @@ mod tests { &script_path, &["fake-codex-bad-config", "login", "status"], None, + &[], ); assert!( matches!(outcome, ProbeOutcome::ConfigInvalid { .. }), @@ -225,6 +251,7 @@ mod tests { &script_path, &["fake-codex-logged-out", "login", "status"], None, + &[], ); assert_eq!( outcome, diff --git a/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs b/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs new file mode 100644 index 0000000000..28f1343499 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/tests/devin_tests.rs @@ -0,0 +1,88 @@ +use super::*; + +fn devin_runtime_for_test( + commands: &'static [&'static str], + underlying_cli: Option<&'static str>, + auth_probe_args: &'static [&'static str], +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "devin", + label: "Devin", + default_args: &["acp"], + login_hint: Some("Run `devin auth login` to authenticate."), + auth_probe_args: Some(auth_probe_args), + auth_login_args: Some(&["devin", "auth", "login"]), + ..make_cli_runtime(commands, underlying_cli) + } +} + +#[test] +fn devin_readiness_is_ready_when_auth_probe_succeeds() { + let exe = present_binary_str(); + let runtime = devin_runtime_for_test( + static_commands(vec![exe]), + Some(exe), + static_commands(vec![exe, "--list"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + assert!( + collect_missing_requirements(&effective, Some(&runtime)).is_empty(), + "a successful catalog-declared Devin auth probe must be ready" + ); +} + +#[test] +fn devin_readiness_requires_login_when_auth_probe_fails() { + let exe = present_binary_str(); + let runtime = devin_runtime_for_test( + static_commands(vec![exe]), + Some(exe), + static_commands(vec![exe, "--buzz-probe-fail-xyz"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + let requirements = collect_missing_requirements(&effective, Some(&runtime)); + assert_eq!(requirements.len(), 1); + assert!(matches!( + &requirements[0], + Requirement::CliLogin { + availability: AcpAvailabilityStatus::Available, + setup_copy, + .. + } if setup_copy.contains("devin auth login") + )); +} + +#[test] +fn devin_readiness_reports_missing_cli_before_authentication() { + let missing = "__buzz_nonexistent_devin_xyz789__"; + let runtime = devin_runtime_for_test( + static_commands(vec![missing]), + Some(missing), + static_commands(vec![missing, "auth", "status"]), + ); + let effective = EffectiveAgentEnv { + env: BTreeMap::new(), + config_file_path: None, + effective_command: "devin".to_string(), + }; + + let requirements = collect_missing_requirements(&effective, Some(&runtime)); + assert_eq!(requirements.len(), 1); + assert!(matches!( + requirements[0], + Requirement::CliLogin { + availability: AcpAvailabilityStatus::NotInstalled, + .. + } + )); +} diff --git a/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs b/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs new file mode 100644 index 0000000000..0c5994cb5f --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/tests/fixtures.rs @@ -0,0 +1,50 @@ +use crate::managed_agents::KnownAcpRuntime; + +pub(super) fn known_runtime_fixture() -> KnownAcpRuntime { + KnownAcpRuntime { + id: "test-runtime", + label: "Test runtime", + display_label: "Test runtime", + sort_priority: 100, + onboarding_visible: false, + commands: &[], + aliases: &[], + default_args: &[], + default_parallelism: None, + defer_agent_start_until_work: true, + default_idle_timeout_secs: None, + icon_url: "", + icon_scale: 1.0, + avatar_url: "", + superseded_avatar_urls: &[], + mcp_command: None, + mcp_hooks: false, + underlying_cli: None, + cli_install_commands: &[], + cli_install_commands_windows: &[], + adapter_install_commands: &[], + cli_install_instructions_url: "", + adapter_install_instructions_url: "", + cli_install_hint: "", + adapter_install_hint: "", + skill_dir: None, + supports_acp_model_switching: false, + accepts_harness_model: true, + model_env_var: None, + provider_env_var: None, + provider_locked: false, + default_env: &[], + enforced_env: &[], + scrub_env_vars: &[], + config_file_path: None, + config_file_format: None, + supports_acp_native_config: false, + thinking_env_var: None, + max_tokens_env_var: None, + context_limit_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + auth_login_args: None, + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 32b3b328e9..13413b111e 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -8,8 +8,8 @@ use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, missing_command_message, normalize_agent_args, open_log_file, resolve_command, - spawn_key_refusal, KnownAcpRuntime, ManagedAgentPairRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, ManagedAgentSummary, + spawn_key_refusal, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey, + ManagedAgentSummary, }, util::now_iso, }; @@ -20,9 +20,21 @@ pub(crate) use path::compose_path_entries; pub(crate) use path::should_skip_claude_executable; pub(crate) use path::should_use_inherited; +mod cli_config; +pub(crate) use cli_config::configure_runtime_cli; + mod metadata; pub(crate) use metadata::{resolve_effective_prompt_model_provider, runtime_metadata_env_vars}; +mod env_policy; +use env_policy::{ + apply_runtime_env_policy, child_rust_log_filter, effective_idle_timeout, + harness_model_for_runtime, should_defer_agent_start, +}; + +mod presentation; +use presentation::runtime_presentation_for_summary; + mod stop; pub(crate) use stop::managed_agent_runtime_keys; pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; @@ -293,6 +305,7 @@ pub fn build_managed_agent_summary( .and_then(|r| r.mcp_command) .unwrap_or("") .to_string(); + let runtime_presentation = runtime_presentation_for_summary(&descriptor.command); Ok(ManagedAgentSummary { pubkey: record.pubkey.clone(), @@ -312,6 +325,10 @@ pub fn build_managed_agent_summary( parallelism: record.parallelism, system_prompt: effective_prompt, avatar_url: record.avatar_url.clone(), + runtime_icon_url: runtime_presentation.icon_url, + runtime_avatar_url: runtime_presentation.avatar_url, + runtime_superseded_avatar_urls: runtime_presentation.superseded_avatar_urls, + supports_buzz_model_config: runtime_presentation.supports_buzz_model_config, model: effective_model, model_source, provider: effective_provider, @@ -418,30 +435,6 @@ pub(crate) fn build_respond_to_env( Ok((set, remove)) } -pub(crate) fn configure_runtime_cli( - command: &mut std::process::Command, - runtime: Option<&KnownAcpRuntime>, -) { - let Some(runtime) = runtime else { - return; - }; - if runtime.id != "claude" { - return; - } - if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { - // On Windows, `.cmd` and `.bat` files are batch shims — they cannot be - // passed directly to `CreateProcess` and cause EINVAL when the Claude - // adapter tries to spawn them (issue #2397). Skip setting - // `CLAUDE_CODE_EXECUTABLE` for shim paths so the adapter falls back to - // its own PATH lookup and finds the real binary instead. - // Non-Windows: `.cmd`/`.bat` are valid executables and must be assigned. - if should_skip_claude_executable(&cli_path, cfg!(windows)) { - return; - } - command.env("CLAUDE_CODE_EXECUTABLE", cli_path); - } -} - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -541,6 +534,7 @@ pub fn spawn_agent_child( let resolved_agent_command = resolve_command(effective_command) .map(|p| p.display().to_string()) .unwrap_or_else(|| effective_command.clone()); + let runtime_meta = known_acp_runtime(effective_command); // The caller supplies the explicit canonical pair relay. This is the only // relay this child may connect to, regardless of the record/workspace default. @@ -576,7 +570,11 @@ 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" }); + let defer_agent_start = should_defer_agent_start(lazy, runtime_meta); + command.env( + "BUZZ_ACP_LAZY_POOL", + if defer_agent_start { "true" } else { "false" }, + ); command.env("BUZZ_ACP_AGENT_COMMAND", &resolved_agent_command); command.env("BUZZ_ACP_AGENT_ARGS", agent_args.join(",")); match &resolved_mcp_command { @@ -589,7 +587,6 @@ pub fn spawn_agent_child( } // Enable MCP hook tools (_Stop, _PostCompact) for agents that need them. // Uses "*" because build_mcp_servers() hard-codes the server name to "buzz-mcp". - let runtime_meta = known_acp_runtime(effective_command); if runtime_meta.is_some_and(|r| r.mcp_hooks) { command.env("MCP_HOOK_SERVERS", "*"); } @@ -710,13 +707,16 @@ pub fn spawn_agent_child( ); } } - // 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), - // which is the single source of truth. The previously-emitted - // `BUZZ_ACP_TURN_TIMEOUT` is deprecated upstream and was pinning every - // agent to the desktop's stale default (320s), bypassing harness bumps. - if let Some(idle) = record.idle_timeout_seconds { + // Emit BUZZ_ACP_IDLE_TIMEOUT for an explicit agent override or a + // KnownAcpRuntime catalog default. Otherwise the buzz-acp harness applies + // its generic default. Preserve an inherited process value; the merged + // global/persona/agent environment below can still override a catalog + // default. + if let Some(idle) = effective_idle_timeout( + record.idle_timeout_seconds, + std::env::var_os("BUZZ_ACP_IDLE_TIMEOUT").is_some(), + runtime_meta, + ) { command.env("BUZZ_ACP_IDLE_TIMEOUT", idle.to_string()); } @@ -755,13 +755,14 @@ pub fn spawn_agent_child( let effective_prompt = effective_cfg.system_prompt.value; let effective_model = effective_cfg.model.value; let effective_provider = effective_cfg.provider.value; + let harness_model = harness_model_for_runtime(runtime_meta, effective_model.as_deref()); if let Some(prompt) = &effective_prompt { command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt); } else { command.env_remove("BUZZ_ACP_SYSTEM_PROMPT"); } - if let Some(model) = effective_model.as_deref() { + if let Some(model) = harness_model { command.env("BUZZ_ACP_MODEL", model); } else { command.env_remove("BUZZ_ACP_MODEL"); @@ -868,6 +869,10 @@ pub fn spawn_agent_child( } } + // Runtime identity and safety policy goes last so ambient, global, + // persona, and per-agent values cannot silently override it. + apply_runtime_env_policy(&mut command, runtime_meta); + // Stamp desktop ownership and an unpredictable harness-generation identity. let start_nonce = uuid::Uuid::new_v4().simple().to_string(); command @@ -950,14 +955,6 @@ pub fn spawn_agent_child( }) } -fn child_rust_log_filter() -> String { - match std::env::var("RUST_LOG") { - Ok(existing) if existing.contains("buzz_acp") => existing, - Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), - _ => "buzz_acp=info".to_string(), - } -} - pub fn start_managed_agent_process( app: &AppHandle, record: &mut ManagedAgentRecord, @@ -1015,5 +1012,7 @@ pub fn start_managed_agent_process( Ok(()) } +#[cfg(all(test, unix))] +mod process_tree_tests; #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs b/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs new file mode 100644 index 0000000000..1d2c6d3954 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/cli_config.rs @@ -0,0 +1,22 @@ +use std::process::Command; + +use crate::managed_agents::{resolve_command, KnownAcpRuntime}; + +use super::should_skip_claude_executable; + +pub(crate) fn configure_runtime_cli(command: &mut Command, runtime: Option<&KnownAcpRuntime>) { + let Some(runtime) = runtime else { + return; + }; + if runtime.id != "claude" { + return; + } + if let Some(cli_path) = runtime.underlying_cli.and_then(resolve_command) { + // Windows batch shims cannot be passed directly to CreateProcess. + // Let the adapter use PATH when the resolved CLI is a shim. + if should_skip_claude_executable(&cli_path, cfg!(windows)) { + return; + } + command.env("CLAUDE_CODE_EXECUTABLE", cli_path); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs new file mode 100644 index 0000000000..38000e2f9a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/env_policy.rs @@ -0,0 +1,191 @@ +use std::process::Command; + +use crate::managed_agents::KnownAcpRuntime; + +pub(super) fn child_rust_log_filter() -> String { + match std::env::var("RUST_LOG") { + Ok(existing) if existing.contains("buzz_acp") => existing, + Ok(existing) if !existing.trim().is_empty() => format!("{existing},buzz_acp=info"), + _ => "buzz_acp=info".to_string(), + } +} + +/// Honor the desktop caller's lazy request only when the runtime catalog says +/// deferring the ACP subprocess is appropriate. +pub(super) fn should_defer_agent_start( + requested_lazy: bool, + runtime: Option<&KnownAcpRuntime>, +) -> bool { + requested_lazy + && runtime + .map(|runtime| runtime.defer_agent_start_until_work) + .unwrap_or(true) +} + +/// Resolve the idle timeout that desktop should write into the harness +/// environment. Explicit record values win; an inherited process value remains +/// untouched; otherwise the runtime catalog may provide a safer default. +pub(super) fn effective_idle_timeout( + configured: Option, + inherited_is_set: bool, + runtime: Option<&KnownAcpRuntime>, +) -> Option { + configured.or_else(|| { + (!inherited_is_set) + .then(|| runtime.and_then(|runtime| runtime.default_idle_timeout_secs)) + .flatten() + }) +} + +/// Apply launch-only runtime environment policy after all user environment +/// layers have been merged. +pub(super) fn apply_runtime_env_policy(command: &mut Command, runtime: Option<&KnownAcpRuntime>) { + let Some(runtime) = runtime else { + return; + }; + for key in runtime.scrub_env_vars { + command.env_remove(key); + } + for (key, value) in runtime.enforced_env { + command.env(key, value); + } +} + +/// Resolve the generic harness bootstrap model for a known runtime. +/// +/// Unknown/custom runtimes preserve the historical behavior because Buzz +/// cannot infer their ACP capabilities. Known runtime policy comes only from +/// `KnownAcpRuntime`, so launch code never needs a runtime-ID branch. +pub(super) fn harness_model_for_runtime<'a>( + runtime: Option<&KnownAcpRuntime>, + effective_model: Option<&'a str>, +) -> Option<&'a str> { + if runtime.is_some_and(|runtime| !runtime.accepts_harness_model) { + None + } else { + effective_model + } +} + +#[cfg(test)] +mod tests { + use super::{ + apply_runtime_env_policy, effective_idle_timeout, harness_model_for_runtime, + should_defer_agent_start, + }; + use crate::managed_agents::known_acp_runtime; + + #[test] + fn devin_policy_enforces_safe_permissions_and_stored_login_identity() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("BUZZ_ACP_PERMISSION_MODE", "bypassPermissions"); + command.env("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "true"); + command.env("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "false"); + command.env("WINDSURF_API_KEY", "sentinel"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("devin")); + + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_PERMISSION_MODE" && value.is_some_and(|value| value == "default") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_AUTO_APPROVE_PERMISSIONS" + && value.is_some_and(|value| value == "false") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_INTERACTIVE_PERMISSIONS" && value.is_some_and(|value| value == "true") + })); + assert!(command.get_envs().any(|(key, value)| { + key == "BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE" + && value.is_some_and(|value| value == "30") + })); + assert!(command + .get_envs() + .any(|(key, value)| { key == "WINDSURF_API_KEY" && value.is_none() })); + } + + /// Regression: Buzz launched from a terminal inside the Devin IDE inherits + /// `ACP_BACKEND=windsurf`. Passing it through makes `devin acp` treat the + /// ACP host as the sole credential source, so it refuses the stored CLI + /// credentials and every turn fails with "ACP host has not authenticated" + /// even though `devin auth login` succeeded. + #[test] + fn devin_policy_scrubs_inherited_acp_backend() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("ACP_BACKEND", "windsurf"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("devin")); + + assert!( + command + .get_envs() + .any(|(key, value)| { key == "ACP_BACKEND" && value.is_none() }), + "ACP_BACKEND must be removed before spawning the Devin adapter" + ); + } + + #[test] + fn existing_runtime_policy_remains_unchanged() { + let mut command = std::process::Command::new("buzz-acp"); + command.env("GOOSE_MODE", "custom"); + + apply_runtime_env_policy(&mut command, known_acp_runtime("goose")); + + assert!(command.get_envs().any(|(key, value)| { + key == "GOOSE_MODE" && value.is_some_and(|value| value == "custom") + })); + } + + #[test] + fn devin_owns_its_model_selection_without_changing_existing_runtime_bootstrap() { + let requested = Some("swe-1-7-lightning"); + let devin = known_acp_runtime("devin").expect("Devin must remain cataloged"); + assert_eq!(harness_model_for_runtime(Some(devin), requested), None); + + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + let runtime = + known_acp_runtime(runtime_id).expect("existing runtime must remain cataloged"); + assert_eq!( + harness_model_for_runtime(Some(runtime), requested), + requested, + "{runtime_id} bootstrap behavior must remain unchanged" + ); + } + + assert_eq!( + harness_model_for_runtime(None, requested), + requested, + "custom runtimes preserve historical bootstrap behavior" + ); + } + + #[test] + fn devin_starts_eagerly_without_changing_existing_runtime_startup() { + assert!(!should_defer_agent_start(true, known_acp_runtime("devin"))); + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + assert!( + should_defer_agent_start(true, known_acp_runtime(runtime_id)), + "{runtime_id} must retain lazy startup" + ); + } + assert!(should_defer_agent_start(true, None)); + assert!(!should_defer_agent_start(false, known_acp_runtime("goose"))); + } + + #[test] + fn devin_idle_default_preserves_all_override_layers() { + let devin = known_acp_runtime("devin"); + assert_eq!(effective_idle_timeout(None, false, devin), Some(120)); + assert_eq!(effective_idle_timeout(Some(45), false, devin), Some(45)); + assert_eq!(effective_idle_timeout(None, true, devin), None); + + for runtime_id in ["goose", "claude", "codex", "buzz-agent"] { + assert_eq!( + effective_idle_timeout(None, false, known_acp_runtime(runtime_id)), + None, + "{runtime_id} must retain the harness idle default" + ); + } + assert_eq!(effective_idle_timeout(None, false, None), None); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/path.rs b/desktop/src-tauri/src/managed_agents/runtime/path.rs index efec0c903e..5042ee0c42 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/path.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/path.rs @@ -80,13 +80,15 @@ pub(crate) fn compose_path_entries( /// Assemble the augmented `PATH` for a launched managed-agent child process. /// /// Concatenates, in priority order: -/// 1. `/.local/bin` — bundled CLI symlink -/// 2. Buzz-managed npm prefix bin dir — app-private ACP adapter shims -/// 3. Buzz-managed Node.js bin dir — app-private Node/npm runtime -/// 4. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm) -/// 5. exe parent dir — DMG sidecars under `Contents/MacOS/` -/// 6. user's login-shell `PATH` — runtimes like node/python from other managers -/// 7. the current process `PATH` — appended on every platform when no +/// 1. exe parent dir when it contains the bundled `buzz` sidecar — this keeps +/// agents version-coupled to the app that launched them +/// 2. `/.local/bin` — user-local CLIs and the compatibility Buzz symlink +/// 3. Buzz-managed npm prefix bin dir — app-private ACP adapter shims +/// 4. Buzz-managed Node.js bin dir — app-private Node/npm runtime +/// 5. `nvm_bin` — nvm's default Node.js bin dir (if the user uses nvm) +/// 6. exe parent dir when it does not contain the bundled `buzz` sidecar +/// 7. user's login-shell `PATH` — runtimes like node/python from other managers +/// 8. the current process `PATH` — appended on every platform when no /// login-shell PATH exists, because callers use `Command::env("PATH", …)` /// which *replaces* the child's PATH. This is the steady state on Windows, /// where `login_shell_path()` always returns `None` and without it the @@ -110,9 +112,15 @@ pub(in crate::managed_agents) fn build_augmented_path( let home_added = home.is_some(); let exe_added = exe_parent.is_some(); let has_local_context = home_added || exe_added; + let prefer_exe_parent = exe_parent + .as_deref() + .is_some_and(|parent| parent.join(buzz_binary_name()).is_file()); // Build the managed/prefix entries (everything before login-shell PATH). let mut managed: Vec = Vec::new(); + if prefer_exe_parent { + managed.extend(exe_parent.iter().cloned()); + } if let Some(home) = home { managed.push(home.join(".local").join("bin")); } @@ -130,8 +138,8 @@ pub(in crate::managed_agents) fn build_augmented_path( if let Some(nvm_bin) = nvm_bin { managed.push(nvm_bin); } - if let Some(parent) = exe_parent { - managed.push(parent); + if !prefer_exe_parent { + managed.extend(exe_parent); } // Split the login-shell PATH into individual entries. @@ -156,6 +164,16 @@ pub(in crate::managed_agents) fn build_augmented_path( .map(|s| s.to_string_lossy().into_owned()) } +#[cfg(windows)] +fn buzz_binary_name() -> &'static str { + "buzz.exe" +} + +#[cfg(not(windows))] +fn buzz_binary_name() -> &'static str { + "buzz" +} + #[cfg(test)] mod tests { use super::build_augmented_path; @@ -164,22 +182,20 @@ mod tests { #[cfg(unix)] #[test] fn splits_colon_delimited_shell_path() { + let app_dir = tempfile::tempdir().expect("temp app directory"); // Regression: the shell PATH arrives as one colon-delimited string. It // must be split into segments before join_paths, or join_paths rejects // it and the whole augmented PATH collapses to None (managed agents then // lose `buzz`). let result = build_augmented_path( Some(PathBuf::from("/home/agent")), - Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), + Some(app_dir.path().to_path_buf()), Some("/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin".to_string()), None, ); let result = result.expect("path"); assert!(result.starts_with("/home/agent/.local/bin:"), "{result}"); - assert!( - result.contains(":/Applications/Buzz.app/Contents/MacOS:"), - "{result}" - ); + assert!(result.contains(app_dir.path().to_str().expect("utf-8 path"))); assert!( result.ends_with(":/usr/local/bin:/opt/homebrew/bin:/usr/bin:/bin"), "{result}" @@ -201,9 +217,10 @@ mod tests { #[cfg(unix)] #[test] fn nvm_bin_inserted_after_local_bin_before_exe_parent() { + let app_dir = tempfile::tempdir().expect("temp app directory"); let result = build_augmented_path( Some(PathBuf::from("/home/user")), - Some(PathBuf::from("/Applications/Buzz.app/Contents/MacOS")), + Some(app_dir.path().to_path_buf()), Some("/usr/bin:/bin".to_string()), Some(PathBuf::from("/home/user/.nvm/versions/node/v20.0.0/bin")), ); @@ -213,7 +230,7 @@ mod tests { .find("/home/user/.nvm/versions/node/v20.0.0/bin") .unwrap(); let exe = result - .find("/Applications/Buzz.app/Contents/MacOS") + .find(app_dir.path().to_str().expect("utf-8 path")) .unwrap(); assert!(local < nvm && nvm < exe, "{result}"); assert!(result.ends_with(":/usr/bin:/bin"), "{result}"); @@ -222,6 +239,7 @@ mod tests { #[cfg(unix)] #[test] fn nvm_bin_none_does_not_add_segment() { + let app_dir = tempfile::tempdir().expect("temp app directory"); let _guard = crate::managed_agents::lock_path_mutex(); let previous = std::env::var_os("PATH"); // With no shell_path the inherited process PATH is appended last, so @@ -230,7 +248,7 @@ mod tests { let result = build_augmented_path( Some(PathBuf::from("/home/user")), - Some(PathBuf::from("/usr/local/bin")), + Some(app_dir.path().to_path_buf()), None, None, ); @@ -244,7 +262,10 @@ mod tests { assert!(result.starts_with("/home/user/.local/bin:"), "{result}"); assert!(!result.contains(".nvm"), "no nvm segment: {result}"); assert!( - result.contains(":/usr/local/bin:"), + result.contains(&format!( + ":{}:", + app_dir.path().to_str().expect("utf-8 path") + )), "exe parent must precede the inherited PATH: {result}" ); assert!( @@ -253,6 +274,47 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn bundled_buzz_precedes_another_installations_local_symlink() { + let app_dir = tempfile::tempdir().expect("temp app directory"); + std::fs::File::create(app_dir.path().join("buzz")).expect("bundled buzz sidecar"); + + let result = build_augmented_path( + Some(PathBuf::from("/home/user")), + Some(app_dir.path().to_path_buf()), + Some("/usr/bin:/bin".to_string()), + None, + ) + .expect("path"); + + let bundled = result + .find(app_dir.path().to_str().expect("utf-8 path")) + .unwrap(); + let local = result.find("/home/user/.local/bin").unwrap(); + assert!(bundled < local, "{result}"); + } + + #[cfg(unix)] + #[test] + fn exe_parent_without_bundled_buzz_keeps_existing_path_order() { + let app_dir = tempfile::tempdir().expect("temp app directory"); + + let result = build_augmented_path( + Some(PathBuf::from("/home/user")), + Some(app_dir.path().to_path_buf()), + Some("/usr/bin:/bin".to_string()), + None, + ) + .expect("path"); + + let bundled = result + .find(app_dir.path().to_str().expect("utf-8 path")) + .unwrap(); + let local = result.find("/home/user/.local/bin").unwrap(); + assert!(local < bundled, "{result}"); + } + /// On Unix with no login-shell PATH, `build_augmented_path` must fall back to /// the inherited process PATH — otherwise the child gets only Buzz-managed /// dirs and loses every system binary (`curl`, `sh`, `tar`). diff --git a/desktop/src-tauri/src/managed_agents/runtime/presentation.rs b/desktop/src-tauri/src/managed_agents/runtime/presentation.rs new file mode 100644 index 0000000000..6f4d76dfeb --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/presentation.rs @@ -0,0 +1,53 @@ +use crate::managed_agents::known_acp_runtime; + +pub(super) struct RuntimePresentation { + pub(super) icon_url: Option, + pub(super) avatar_url: Option, + pub(super) superseded_avatar_urls: Vec, + pub(super) supports_buzz_model_config: Option, +} + +pub(super) fn runtime_presentation_for_summary(effective_command: &str) -> RuntimePresentation { + let runtime = known_acp_runtime(effective_command); + RuntimePresentation { + icon_url: runtime.map(|runtime| runtime.icon_url.to_string()), + avatar_url: runtime.map(|runtime| runtime.avatar_url.to_string()), + superseded_avatar_urls: runtime + .map(|runtime| { + runtime + .superseded_avatar_urls + .iter() + .map(|url| (*url).to_string()) + .collect() + }) + .unwrap_or_default(), + supports_buzz_model_config: runtime + .map(|runtime| runtime.model_env_var.is_some() || runtime.supports_acp_model_switching), + } +} + +#[cfg(test)] +mod tests { + use super::runtime_presentation_for_summary; + use crate::managed_agents::known_acp_runtime; + + #[test] + fn runtime_avatar_is_catalog_derived_without_process_state() { + let runtime = known_acp_runtime("devin").expect("Devin must remain a known runtime"); + let presentation = runtime_presentation_for_summary("devin"); + + assert_eq!(presentation.icon_url.as_deref(), Some(runtime.icon_url)); + assert_eq!(presentation.avatar_url.as_deref(), Some(runtime.avatar_url)); + assert_eq!( + presentation.superseded_avatar_urls, + runtime.superseded_avatar_urls + ); + assert_eq!(presentation.supports_buzz_model_config, Some(false)); + + let custom = runtime_presentation_for_summary("custom-agent"); + assert!(custom.icon_url.is_none()); + assert!(custom.avatar_url.is_none()); + assert!(custom.superseded_avatar_urls.is_empty()); + assert_eq!(custom.supports_buzz_model_config, None); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/process.rs b/desktop/src-tauri/src/managed_agents/runtime/process.rs index 37eb5659a4..66fe92ecbb 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/process.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/process.rs @@ -34,14 +34,15 @@ pub(crate) const KNOWN_SCRIPT_INTERPRETERS: &[&str] = &["node"]; /// Uses exact match or prefix-with-separator to avoid false positives /// (e.g. `"goose"` must not match `"mongoose"`). pub(super) fn name_matches_known_binary(name: &str) -> bool { - KNOWN_AGENT_BINARIES.iter().any(|&binary| { - name == binary || { - name.starts_with(binary) && { - let rest = &name[binary.len()..]; - rest.starts_with('-') || rest.starts_with('_') || rest.starts_with('.') + known_acp_runtime(name).is_some() + || KNOWN_AGENT_BINARIES.iter().any(|&binary| { + name == binary || { + name.starts_with(binary) && { + let rest = &name[binary.len()..]; + rest.starts_with('-') || rest.starts_with('_') || rest.starts_with('.') + } } - } - }) + }) } /// Check if a process name is a known script interpreter that may be hosting @@ -243,6 +244,10 @@ fn signal_process_group_or_leader(pid: u32, signal: i32, action: &str) -> Result #[cfg(unix)] pub(crate) fn terminate_process(pid: u32) -> Result<(), String> { + // Reap independently-grouped ACP descendants while their ownership can + // still be proven through the live harness ancestry. + sweep::terminate_owned_descendant_groups(pid); + // Try graceful shutdown first (SIGTERM to the group). signal_process_group_or_leader(pid, libc::SIGTERM, "terminate")?; diff --git a/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs b/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs new file mode 100644 index 0000000000..9f0f4b998b --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/process_tree_tests.rs @@ -0,0 +1,83 @@ +//! Process-tree regression tests that launch subprocesses. + +use std::os::unix::process::CommandExt; +use std::process::Command; + +/// Regression coverage for managed-agent restart teardown. The helper test +/// process acts as `buzz-acp` and starts a child in its own process group, +/// mirroring `AcpClient::spawn`. Terminating the helper must reap both groups. +#[test] +fn terminate_process_reaps_independent_descendant_group() { + let _path_guard = crate::managed_agents::lock_path_mutex(); + let marker_path = std::env::temp_dir().join(format!( + "buzz-process-tree-{}-{}.pid", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("system clock after Unix epoch") + .as_nanos() + )); + let _ = std::fs::remove_file(&marker_path); + + let mut helper = { + let mut command = Command::new(std::env::current_exe().expect("resolve test executable")); + command + .args([ + "--exact", + "managed_agents::runtime::process_tree_tests::process_tree_descendant_helper", + "--nocapture", + ]) + .env("BUZZ_PROCESS_TREE_TEST_MARKER", &marker_path) + .process_group(0); + command.spawn().expect("spawn process-tree helper") + }; + let helper_pid = helper.id(); + + let child_pid = (0..100) + .find_map(|_| { + let value = std::fs::read_to_string(&marker_path).ok(); + if value.is_none() { + std::thread::sleep(std::time::Duration::from_millis(20)); + } + value.and_then(|pid| pid.trim().parse::().ok()) + }) + .expect("helper should report its independent child PID"); + + assert_eq!( + unsafe { libc::getpgid(child_pid as i32) }, + child_pid as i32, + "helper child should lead an independent process group" + ); + + super::terminate_process(helper_pid).expect("terminate complete helper process tree"); + let _ = helper.wait(); + + for _ in 0..50 { + if !super::process_is_running(child_pid) { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + assert!( + !super::process_is_running(child_pid), + "independently-grouped ACP child must not survive harness teardown" + ); + let _ = std::fs::remove_file(marker_path); +} + +/// Subprocess-only half of [`terminate_process_reaps_independent_descendant_group`]. +#[test] +fn process_tree_descendant_helper() { + let Some(marker_path) = std::env::var_os("BUZZ_PROCESS_TREE_TEST_MARKER") else { + return; + }; + let mut child = { + let mut command = Command::new("sh"); + command + .args(["-c", "while :; do sleep 60; done"]) + .process_group(0); + command.spawn().expect("spawn independent helper child") + }; + std::fs::write(marker_path, child.id().to_string()).expect("write helper child PID"); + let _ = child.wait(); +} diff --git a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs index 3060ff6593..87622fdf73 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/sweep.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/sweep.rs @@ -186,6 +186,82 @@ pub(super) fn ppid_of_linux(pid: u32) -> Option { proc_stat_ppid_pgid_linux(pid).map(|(ppid, _)| ppid) } +/// Snapshot every live, same-user descendant of `root_pid` while the root is +/// still running. +/// +/// Managed ACP runtimes may put their own subprocesses in independent process +/// groups. Signalling only the harness group therefore cannot guarantee that +/// the complete runtime tree exits. Callers use this snapshot immediately, +/// before terminating the root, so the ancestor relationship is still +/// available and unrelated same-user processes remain out of scope. +#[cfg(target_os = "macos")] +pub(super) fn collect_live_descendant_pids(root_pid: u32) -> Vec { + let my_uid = unsafe { libc::getuid() }; + collect_all_pids() + .into_iter() + .filter_map(|pid| { + if pid <= 0 || pid as u32 == root_pid { + return None; + } + let upid = pid as u32; + let mut info = std::mem::MaybeUninit::::zeroed(); + let ret = unsafe { + super::proc_pidinfo( + pid, + super::PROC_PIDTBSDINFO, + 0, + info.as_mut_ptr() as *mut libc::c_void, + std::mem::size_of::() as libc::c_int, + ) + }; + if ret <= 0 { + return None; + } + let info = unsafe { info.assume_init() }; + (info.pbi_uid == my_uid && walk_has_tracked_ancestor(upid, &[root_pid], ppid_of_macos)) + .then_some(upid) + }) + .collect() +} + +/// Terminate process groups led by a live, same-user descendant of `root_pid`. +/// +/// This must run while the root is still alive so the bounded ancestry walk +/// can prove ownership before any signal is sent. +#[cfg(unix)] +pub(super) fn terminate_owned_descendant_groups(root_pid: u32) { + let descendant_pids = collect_live_descendant_pids(root_pid) + .into_iter() + .map(|pid| pid as i32) + .collect::>(); + if !descendant_pids.is_empty() { + super::resolve_pgids_and_kill(&descendant_pids); + } +} + +/// Linux variant of [`collect_live_descendant_pids`]. +#[cfg(all(unix, not(target_os = "macos")))] +pub(super) fn collect_live_descendant_pids(root_pid: u32) -> Vec { + let my_uid = unsafe { libc::getuid() }; + let Ok(entries) = std::fs::read_dir("/proc") else { + return Vec::new(); + }; + entries + .flatten() + .filter_map(|entry| { + let pid = entry.file_name().to_str()?.parse::().ok()?; + if pid == 0 || pid == root_pid { + return None; + } + use std::os::unix::fs::MetadataExt; + if entry.metadata().ok()?.uid() != my_uid { + return None; + } + walk_has_tracked_ancestor(pid, &[root_pid], ppid_of_linux).then_some(pid) + }) + .collect() +} + /// True if `pid` is a live descendant of any tracked harness in `skip_pids`. /// /// Three complementary checks: @@ -433,7 +509,7 @@ fn collect_process_snapshots(harness_name: &str) -> Vec { snapshots } -// ── expected_harness_exe_path ───────────────────────────────────────────── +// ── expected_harness_exe_paths ──────────────────────────────────────────── /// Derive the expected path of the `buzz-acp` harness binary next to the /// current executable. Returns `None` if `current_exe()` fails or has no @@ -460,12 +536,29 @@ fn collect_process_snapshots(harness_name: &str) -> Vec { /// the same app (different bundle path, e.g. a prior DMG) will not match /// this path — that class is handled by `sweep_system_agent_processes`, which /// scopes by `BUZZ_MANAGED_AGENT` instance ID rather than exe path. -pub fn expected_harness_exe_path() -> Option { - let exe = std::env::current_exe().ok()?; - let dir = exe.parent()?; - let raw = dir.join("buzz-acp"); - // Canonicalize if possible; fall back to the raw path on failure. - Some(std::fs::canonicalize(&raw).unwrap_or(raw)) +pub fn expected_harness_exe_paths() -> Vec { + let mut paths = Vec::new(); + if let Some(raw) = std::env::current_exe() + .ok() + .and_then(|exe| exe.parent().map(|dir| dir.join("buzz-acp"))) + { + paths.push(std::fs::canonicalize(&raw).unwrap_or(raw)); + } + + // `tauri dev` builds the desktop crate under `desktop/src-tauri/target` + // while Buzz's sidecar build lives under the repository-level `target`. + // The launch resolver intentionally uses that workspace sidecar, so the + // boot sweeper must recognize the same exact path. Release builds remain + // scoped to the sibling binary inside the app bundle. + #[cfg(debug_assertions)] + if let Some(raw) = crate::managed_agents::discovery::resolve_workspace_command("buzz-acp") { + let path = std::fs::canonicalize(&raw).unwrap_or(raw); + if !paths.contains(&path) { + paths.push(path); + } + } + + paths } /// The basename of the harness binary — used for the cheap name pre-filter in @@ -496,19 +589,28 @@ const HARNESS_BINARY_NAME: &str = "buzz-acp"; /// when `resolve_pgids_and_kill` signals the PGID. #[cfg(unix)] pub(crate) fn sweep_untracked_bundle_harnesses(skip_pids: &[u32]) { - let Some(harness_exe) = expected_harness_exe_path() else { + let harness_exes = expected_harness_exe_paths(); + if harness_exes.is_empty() { return; - }; + } let snapshots = collect_process_snapshots(HARNESS_BINARY_NAME); - let to_kill = select_untracked_bundle_harnesses(&snapshots, &harness_exe, skip_pids); + let mut to_kill = std::collections::BTreeSet::new(); + for harness_exe in &harness_exes { + to_kill.extend(select_untracked_bundle_harnesses( + &snapshots, + harness_exe, + skip_pids, + )); + } if to_kill.is_empty() { return; } + let to_kill = to_kill.into_iter().collect::>(); eprintln!( - "buzz-desktop: sweep_untracked_bundle_harnesses: reaping {} stale harness process(es) {:?} (exe: {})", + "buzz-desktop: sweep_untracked_bundle_harnesses: reaping {} stale harness process(es) {:?} (expected exe paths: {:?})", to_kill.len(), to_kill, - harness_exe.display(), + harness_exes, ); // Small snapshot→kill PID-reuse window: a PID in `to_kill` could be // recycled between the snapshot and the kill call. This matches the diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 8deb0c4da9..2ee4cd27df 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1,5 +1,7 @@ use crate::managed_agents::known_acp_runtime; +mod devin; + // ── desktop binary name tests ─────────────────────────────────────────── #[test] @@ -594,45 +596,6 @@ fn name_matches_interpreter_rejects_node_prefix() { assert!(!super::name_matches_interpreter("node-gyp")); } -#[test] -fn claude_spawn_uses_the_probed_cli_executable() { - let _guard = crate::managed_agents::lock_path_mutex(); - let temp = tempfile::tempdir().expect("temp dir"); - let cli = temp - .path() - .join(format!("claude{}", std::env::consts::EXE_SUFFIX)); - std::fs::write(&cli, "").expect("write fake cli"); - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&cli, std::fs::Permissions::from_mode(0o755)) - .expect("make fake cli executable"); - } - let original_path = std::env::var_os("PATH"); - std::env::set_var("PATH", temp.path()); - - let mut command = std::process::Command::new("buzz-acp"); - super::configure_runtime_cli(&mut command, super::known_acp_runtime("claude-agent-acp")); - - if let Some(path) = original_path { - std::env::set_var("PATH", path); - } else { - std::env::remove_var("PATH"); - } - assert!(command - .get_envs() - .any(|(key, value)| { key == "CLAUDE_CODE_EXECUTABLE" && value == Some(cli.as_os_str()) })); -} - -#[test] -fn codex_spawn_does_not_set_a_claude_executable() { - let mut command = std::process::Command::new("buzz-acp"); - super::configure_runtime_cli(&mut command, super::known_acp_runtime("codex-acp")); - assert!(!command - .get_envs() - .any(|(key, _)| key == "CLAUDE_CODE_EXECUTABLE")); -} - /// On Windows, `.cmd` and `.bat` batch shims must NOT be assigned to /// `CLAUDE_CODE_EXECUTABLE` — `CreateProcess` cannot exec them directly and /// returns EINVAL (issue #2397). The adapter must fall back to its own PATH @@ -703,6 +666,8 @@ fn grandchild_inherits_pgid_of_process_group_leader() { use std::os::unix::process::CommandExt; use std::process::Command; + let _path_guard = crate::managed_agents::lock_path_mutex(); + // Spawn a "harness" process in its own process group (mirrors // `command.process_group(0)` in the real spawn path). The harness // spawns an intermediate child which in turn spawns a grandchild. @@ -812,6 +777,8 @@ fn own_group_grandchild_detected_by_ancestor_walk() { use std::os::unix::process::CommandExt; use std::process::Command; + let _path_guard = crate::managed_agents::lock_path_mutex(); + // The test process is the "harness". Spawn an intermediate with its own // process group (mirrors the node shim). It backgrounds a grandchild // (sleep 30) and prints the grandchild PID so we can inspect it. diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs new file mode 100644 index 0000000000..399057efe6 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/tests/devin.rs @@ -0,0 +1,46 @@ +use crate::managed_agents::known_acp_runtime; + +#[test] +fn uses_native_acp_without_mcp_hooks() { + let runtime = known_acp_runtime("/usr/local/bin/devin").expect("should resolve"); + assert_eq!(runtime.id, "devin"); + assert_eq!(runtime.default_args, &["acp"]); + assert!(!runtime.defer_agent_start_until_work); + assert_eq!(runtime.default_idle_timeout_secs, Some(120)); + assert!(runtime.default_env.is_empty()); + assert_eq!( + runtime.enforced_env, + &[ + ("BUZZ_ACP_PERMISSION_MODE", "default"), + ("BUZZ_ACP_AUTO_APPROVE_PERMISSIONS", "false"), + ("BUZZ_ACP_INTERACTIVE_PERMISSIONS", "true"), + ("BUZZ_ACP_SELF_PUBLISH_COMPLETION_GRACE", "30"), + ] + ); + assert_eq!(runtime.scrub_env_vars, &["WINDSURF_API_KEY", "ACP_BACKEND"]); + assert!(!runtime.mcp_hooks); + assert_eq!(runtime.mcp_command, None); +} + +#[test] +fn permission_default_does_not_change_existing_runtimes() { + assert_eq!( + known_acp_runtime("goose") + .expect("Goose runtime") + .default_env, + &[("GOOSE_MODE", "auto")] + ); + for command in ["claude-agent-acp", "codex-acp", "buzz-agent"] { + let runtime = known_acp_runtime(command).expect("existing runtime"); + assert!(runtime.default_env.is_empty()); + assert!(runtime.enforced_env.is_empty()); + assert!(runtime.scrub_env_vars.is_empty()); + assert!(runtime.defer_agent_start_until_work); + assert_eq!(runtime.default_idle_timeout_secs, None); + } +} + +#[test] +fn process_name_resolves_from_the_runtime_catalog() { + assert!(super::super::name_matches_known_binary("devin")); +} diff --git a/desktop/src-tauri/src/managed_agents/storage.rs b/desktop/src-tauri/src/managed_agents/storage.rs index b9c6c7e6cd..0118f6af84 100644 --- a/desktop/src-tauri/src/managed_agents/storage.rs +++ b/desktop/src-tauri/src/managed_agents/storage.rs @@ -11,7 +11,10 @@ use crate::app_state::keyring_service; use crate::managed_agents::{ ManagedAgentRecord, ManagedAgentRuntimeKey, ManagedAgentRuntimeReceipt, }; + +mod normalization; use crate::secret_store::{KeyringProbe, SecretStore}; +use normalization::normalize_runtime_avatars; /// Keyring key name for an agent's nsec, namespaced from the human identity /// key (`"identity"`) which shares the service. @@ -181,7 +184,7 @@ fn load_agent_store(app: &AppHandle) -> Result, String> let content = fs::read_to_string(&path) .map_err(|error| format!("failed to read agent store: {error}"))?; - serde_json::from_str(&content).map_err(|error| { + let mut records: Vec = serde_json::from_str(&content).map_err(|error| { // Fail loudly and preserve the evidence: a later in-app save rewrites // this file wholesale, which would silently destroy a malformed hand // edit. Best-effort file-authoring contract (see managed_agents:: @@ -190,7 +193,11 @@ fn load_agent_store(app: &AppHandle) -> Result, String> // swallowed into an empty store. backup_invalid_store(&path); format!("failed to parse agent store (preserved as .invalid): {error}") - }) + })?; + + normalize_runtime_avatars(&mut records); + + Ok(records) } /// Load the keyed agent *instances*. Key-less definitions (former personas, @@ -590,11 +597,34 @@ fn maybe_rotate_log(path: &Path) { pub(crate) fn open_log_file(path: &Path) -> Result { maybe_rotate_log(path); - OpenOptions::new() - .create(true) - .append(true) + let mut options = OpenOptions::new(); + options.create(true).append(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.mode(0o600); + } + + let file = options .open(path) - .map_err(|error| format!("failed to open log file {}: {error}", path.display())) + .map_err(|error| format!("failed to open log file {}: {error}", path.display()))?; + + // `mode()` applies only when the file is created. Tighten logs written by + // older builds as soon as they are reopened so upgrades do not leave agent + // activity readable by other local accounts. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + file.set_permissions(fs::Permissions::from_mode(0o600)) + .map_err(|error| { + format!( + "failed to secure log file permissions for {}: {error}", + path.display() + ) + })?; + } + + Ok(file) } pub(crate) fn append_log_marker(path: &Path, message: &str) -> Result<(), String> { @@ -789,15 +819,17 @@ pub fn meaningful_agent_error_from_log(path: &Path) -> Option { mod tests { use std::cell::RefCell; use std::collections::HashMap; - use std::io::Write as _; - - use tempfile::NamedTempFile; use super::{ agent_keyring_name, hydrate_keys_with, migrate_inline_key, persist_agent_keys_with, KeyMigration, KeyStore, KeyringProbe, ManagedAgentRecord, }; + mod avatar; + mod log_errors; + #[cfg(unix)] + mod log_permissions; + /// In-memory [`KeyStore`] for testing the migrate decision without the OS /// keyring. `reachable=false` simulates a backend outage; `fail_verify` /// simulates a write whose read-back does not confirm. @@ -1095,12 +1127,6 @@ mod tests { assert!(records[1].private_key_nsec.is_empty()); } - fn write_log(content: &str) -> NamedTempFile { - let mut file = NamedTempFile::new().expect("temp log"); - file.write_all(content.as_bytes()).expect("write log"); - file - } - /// The keyringless fallback write must land `0o600` from the write itself — /// not a post-write `chmod` — so a crash in the umask window can never leave /// plaintext agent nsecs world-readable (Wes storage.rs:239, SECURITY.md:90). @@ -1127,58 +1153,6 @@ mod tests { ); } - #[test] - fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { - let file = write_log( - "noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n", - ); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert!(result.message.contains("llm auth")); - assert_eq!(result.code, Some(-32001)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { - let file = write_log("noise\nllm auth: denied\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm auth: denied"); - assert_eq!(result.code, Some(-32001)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { - let file = write_log("noise\nllm model not found: (some-model) 404\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!( - result.message, - "Agent reported error: llm model not found: (some-model) 404" - ); - assert_eq!(result.code, Some(-32002)); - } - - #[test] - fn meaningful_agent_error_from_log_promotes_legacy_format() { - let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); - let result = super::meaningful_agent_error_from_log(file.path()).unwrap(); - assert_eq!(result.message, "Agent reported error: llm: 500 internal"); - assert_eq!(result.code, None); - } - - #[test] - fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { - let file = write_log("noise before llm auth: denied\n"); - assert!(super::meaningful_agent_error_from_log(file.path()).is_none()); - } - - #[test] - fn strips_ansi_from_typical_tracing_line() { - let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; - assert_eq!( - strip_ansi_escapes::strip_str(input), - "2026-05-27T15:16:32 INFO buzz_acp: starting" - ); - } - // ── keyring-dev-migration tests ──────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/storage/normalization.rs b/desktop/src-tauri/src/managed_agents/storage/normalization.rs new file mode 100644 index 0000000000..741276c7ec --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/normalization.rs @@ -0,0 +1,12 @@ +use crate::managed_agents::{normalize_managed_agent_avatar, ManagedAgentRecord}; + +pub(super) fn normalize_runtime_avatars(records: &mut [ManagedAgentRecord]) { + for record in records { + let command = record + .runtime + .as_deref() + .filter(|value| !value.trim().is_empty()) + .unwrap_or(&record.agent_command); + record.avatar_url = normalize_managed_agent_avatar(command, record.avatar_url.take()); + } +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs b/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs new file mode 100644 index 0000000000..a7179cd902 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/avatar.rs @@ -0,0 +1,28 @@ +use super::super::normalize_runtime_avatars; +use super::record_with_key; + +#[test] +fn loaded_devin_records_replace_only_the_superseded_default_avatar() { + let mut devin = record_with_key(""); + devin.runtime = Some("devin".to_string()); + devin.avatar_url = Some( + "https://mintcdn.com/cognitionai/Hhrl_8XUBqA4VQ6v/logo/favicon.svg?fit=max&auto=format&n=Hhrl_8XUBqA4VQ6v&q=85&s=ab641f30c01bf5374b90b62209db569e" + .to_string(), + ); + + let mut custom = record_with_key(""); + custom.runtime = Some("devin".to_string()); + custom.avatar_url = Some("https://example.test/custom.png".to_string()); + + normalize_runtime_avatars(std::slice::from_mut(&mut devin)); + normalize_runtime_avatars(std::slice::from_mut(&mut custom)); + + assert_eq!( + devin.avatar_url, + crate::managed_agents::managed_agent_avatar_url("devin") + ); + assert_eq!( + custom.avatar_url.as_deref(), + Some("https://example.test/custom.png") + ); +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs b/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs new file mode 100644 index 0000000000..6bb1b2e253 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/log_errors.rs @@ -0,0 +1,60 @@ +use std::io::Write as _; + +use tempfile::NamedTempFile; + +fn write_log(content: &str) -> NamedTempFile { + let mut file = NamedTempFile::new().expect("temp log"); + file.write_all(content.as_bytes()).expect("write log"); + file +} + +#[test] +fn meaningful_agent_error_from_log_promotes_wrapped_llm_auth() { + let file = + write_log("noise\nAgent reported error (code -32001): llm auth: 401 unauthorized: ...\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert!(result.message.contains("llm auth")); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_unwrapped_llm_auth() { + let file = write_log("noise\nllm auth: denied\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm auth: denied"); + assert_eq!(result.code, Some(-32001)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_bare_model_not_found() { + let file = write_log("noise\nllm model not found: (some-model) 404\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!( + result.message, + "Agent reported error: llm model not found: (some-model) 404" + ); + assert_eq!(result.code, Some(-32002)); +} + +#[test] +fn meaningful_agent_error_from_log_promotes_legacy_format() { + let file = write_log("noise\nAgent reported error: llm: 500 internal\n"); + let result = super::super::meaningful_agent_error_from_log(file.path()).unwrap(); + assert_eq!(result.message, "Agent reported error: llm: 500 internal"); + assert_eq!(result.code, None); +} + +#[test] +fn meaningful_agent_error_from_log_does_not_promote_midline_auth_text() { + let file = write_log("noise before llm auth: denied\n"); + assert!(super::super::meaningful_agent_error_from_log(file.path()).is_none()); +} + +#[test] +fn strips_ansi_from_typical_tracing_line() { + let input = "\x1b[2m2026-05-27T15:16:32\x1b[0m \x1b[32m INFO\x1b[0m \x1b[2mbuzz_acp\x1b[0m\x1b[2m:\x1b[0m starting"; + assert_eq!( + strip_ansi_escapes::strip_str(input), + "2026-05-27T15:16:32 INFO buzz_acp: starting" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs b/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs new file mode 100644 index 0000000000..5a2e0fee9c --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/storage/tests/log_permissions.rs @@ -0,0 +1,29 @@ +use std::os::unix::fs::PermissionsExt as _; + +#[test] +fn agent_logs_are_created_owner_only_and_tighten_legacy_permissions() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent.log"); + + drop(super::super::open_log_file(&path).expect("create log")); + assert_eq!( + std::fs::metadata(&path) + .expect("created log metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); + + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644)) + .expect("set legacy permissions"); + drop(super::super::open_log_file(&path).expect("reopen legacy log")); + assert_eq!( + std::fs::metadata(&path) + .expect("reopened log metadata") + .permissions() + .mode() + & 0o777, + 0o600 + ); +} diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index dcb8095a7c..c21d13ff3c 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -489,7 +489,19 @@ pub struct ManagedAgentSummary { pub max_turn_duration_seconds: Option, pub parallelism: u32, pub system_prompt: Option, + /// User/persona avatar snapshot persisted on the agent record. pub avatar_url: Option, + /// App-local presentation mark derived from the effective runtime catalog. + pub runtime_icon_url: Option, + /// Presentation-only fallback derived from the effective runtime catalog. + /// This is never persisted as a user-selected avatar. + pub runtime_avatar_url: Option, + /// Superseded catalog defaults that the frontend must not prefer over the + /// current runtime avatar while a stopped agent's relay profile is stale. + pub runtime_superseded_avatar_urls: Vec, + /// Whether Buzz can apply its configured model to this runtime. `None` + /// preserves the existing display for unknown/custom runtimes. + pub supports_buzz_model_config: Option, pub model: Option, #[serde(skip_serializing_if = "Option::is_none")] pub model_source: Option, @@ -589,7 +601,7 @@ pub enum AuthStatus { #[derive(Debug, Clone, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum HarnessSource { - /// Compiled into the app — one of the four first-class runtimes. + /// Compiled into the app — one of the first-class runtimes. Builtin, /// Static preset entry with bundled logo, PATH-probed, not editable/deletable. Preset, @@ -601,7 +613,14 @@ pub enum HarnessSource { pub struct AcpRuntimeCatalogEntry { pub id: String, pub label: String, + pub display_label: String, + pub sort_priority: u16, + pub onboarding_visible: bool, + pub icon_url: String, + pub icon_scale: f32, pub avatar_url: String, + pub superseded_avatar_urls: Vec, + pub supports_buzz_model_config: bool, pub availability: AcpAvailabilityStatus, pub command: Option, pub binary_path: Option, diff --git a/desktop/src-tauri/src/nostr_convert.rs b/desktop/src-tauri/src/nostr_convert.rs index ec4970e0c9..fe941ca708 100644 --- a/desktop/src-tauri/src/nostr_convert.rs +++ b/desktop/src-tauri/src/nostr_convert.rs @@ -22,6 +22,9 @@ pub use user_search::{ // ── Tag helpers ───────────────────────────────────────────────────────────── +mod managed_agents; +pub use managed_agents::managed_agents_from_events; + /// Find the first tag whose name matches `name` and return its first value. /// /// e.g. for tag `["name", "general"]` with `name="name"` returns `Some("general")`. diff --git a/desktop/src-tauri/src/nostr_convert/managed_agents.rs b/desktop/src-tauri/src/nostr_convert/managed_agents.rs new file mode 100644 index 0000000000..d9da5007e4 --- /dev/null +++ b/desktop/src-tauri/src/nostr_convert/managed_agents.rs @@ -0,0 +1,180 @@ +//! Managed-agent directory projection (kind:30177). +//! +//! Split out of `nostr_convert` so the generic event converters stay readable +//! and this authorization-facing projection has an obvious home. + +use nostr::{Event, ToBech32}; +use serde_json::{json, Value}; + +use super::first_tag_value; + +/// Convert public managed-agent projections into the relay-agent directory +/// shape consumed by the desktop. +/// +/// The event author is the human owner. The managed agent's public key is the +/// parameterized replaceable event's `d` tag, so a content-supplied key is +/// never authoritative. Build a narrow output object instead of forwarding +/// the complete content projection so future fields cannot accidentally cross +/// this frontend boundary. +pub fn managed_agents_from_events(events: &[Event]) -> Value { + let arr: Vec = events + .iter() + .filter_map(|event| { + let agent_pubkey = nostr::PublicKey::from_hex(first_tag_value(event, "d")?).ok()?; + let pubkey = agent_pubkey.to_hex(); + let npub = agent_pubkey.to_bech32().unwrap_or_else(|_| pubkey.clone()); + let content: Value = serde_json::from_str(&event.content).ok()?; + let object = content.as_object()?; + let name = object + .get("name") + .and_then(Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or(&npub); + // Only the modes `RespondTo` can represent may be projected. The + // harness also supports `nobody`, but the desktop enum deliberately + // omits it, so emitting it here would fail deserialization for the + // WHOLE directory response — one agent publishing an unsupported + // mode would hide every other agent. An unrepresentable mode + // becomes absent, which every eligibility check treats as + // not-invocable, so the projection degrades closed. + let respond_to = object + .get("respond_to") + .and_then(Value::as_str) + .filter(|mode| matches!(*mode, "owner-only" | "allowlist" | "anyone")); + + let mut respond_to_allowlist = Vec::new(); + if let Some(values) = object.get("respond_to_allowlist").and_then(Value::as_array) { + for value in values { + let Some(raw) = value.as_str() else { + continue; + }; + let Ok(key) = nostr::PublicKey::from_hex(raw.trim()) else { + continue; + }; + let normalized = key.to_hex(); + if !respond_to_allowlist.contains(&normalized) { + respond_to_allowlist.push(normalized); + } + } + } + + Some(json!({ + "pubkey": pubkey, + "name": name, + "agent_type": "agent", + "channels": [], + "channel_ids": [], + "capabilities": [], + "status": "offline", + "respond_to": respond_to, + "respond_to_allowlist": respond_to_allowlist, + })) + }) + .collect(); + + json!({ "agents": arr }) +} +#[cfg(test)] +mod tests { + use super::*; + use nostr::{EventBuilder, Keys, Kind, Tag}; + + fn ev(kind: u16, content: &str, tags: Vec>) -> Event { + let keys = Keys::generate(); + let tags: Vec = tags + .into_iter() + .map(|t| Tag::parse(t.iter().map(|s| s.to_string()).collect::>()).unwrap()) + .collect(); + EventBuilder::new(Kind::from(kind), content) + .tags(tags) + .sign_with_keys(&keys) + .unwrap() + } + + #[test] + fn managed_agents_use_d_tag_identity_and_preserve_allowlist_metadata() { + let agent_pubkey = "02".repeat(32); + let allowed_pubkey = "03".repeat(32); + let e = ev( + 30177, + &format!( + r#"{{"pubkey":"forged","name":"Scout","parallelism":1,"respond_to":"allowlist","respond_to_allowlist":["{allowed_pubkey}"],"env_vars":{{"SECRET":"do-not-project"}}}}"# + ), + vec![vec!["d", &agent_pubkey]], + ); + let v = managed_agents_from_events(std::slice::from_ref(&e)); + let agents = v.get("agents").cloned().unwrap(); + let parsed: Vec = + serde_json::from_value(agents).unwrap(); + + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0].pubkey, agent_pubkey); + assert_eq!(parsed[0].name, "Scout"); + assert_eq!( + parsed[0].respond_to, + Some(crate::managed_agents::RespondTo::Allowlist) + ); + assert_eq!(parsed[0].respond_to_allowlist, vec![allowed_pubkey]); + assert!( + !v.to_string().contains("SECRET"), + "the directory projection must remain an explicit public-field allowlist" + ); + } + + /// One agent publishing a mode the desktop enum cannot represent must not + /// take down the entire directory. `respond_to` deserializes into + /// `RespondTo`, which has no `nobody` variant, so projecting that string + /// would fail the whole `Vec` and hide every other agent. + #[test] + fn managed_agents_unsupported_mode_does_not_break_other_entries() { + let good_pubkey = "02".repeat(32); + let nobody_pubkey = "03".repeat(32); + let good = ev( + 30177, + r#"{"name":"Good","respond_to":"owner-only"}"#, + vec![vec!["d", &good_pubkey]], + ); + let nobody = ev( + 30177, + r#"{"name":"Nope","respond_to":"nobody"}"#, + vec![vec!["d", &nobody_pubkey]], + ); + let garbage = ev( + 30177, + r#"{"name":"Junk","respond_to":"not-a-mode"}"#, + vec![vec!["d", &"04".repeat(32)]], + ); + + let v = managed_agents_from_events(&[good, nobody, garbage]); + let parsed: Vec = + serde_json::from_value(v.get("agents").cloned().unwrap()) + .expect("an unsupported mode must not fail the whole directory"); + + assert_eq!(parsed.len(), 3, "every entry survives"); + assert_eq!( + parsed[0].respond_to, + Some(crate::managed_agents::RespondTo::OwnerOnly) + ); + // Unrepresentable modes degrade closed: absent, never invocable. + assert_eq!(parsed[1].respond_to, None); + assert_eq!(parsed[2].respond_to, None); + } + + #[test] + fn managed_agents_drop_events_without_a_valid_agent_d_tag() { + let missing = ev( + 30177, + r#"{"name":"Missing","parallelism":1,"respond_to":"owner-only"}"#, + vec![], + ); + let invalid = ev( + 30177, + r#"{"name":"Invalid","parallelism":1,"respond_to":"owner-only"}"#, + vec![vec!["d", "not-a-pubkey"]], + ); + let v = managed_agents_from_events(&[missing, invalid]); + + assert_eq!(v.get("agents").and_then(Value::as_array).unwrap().len(), 0); + } +} diff --git a/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs b/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs index 696a055d20..7554fc5564 100644 --- a/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs +++ b/desktop/src/features/agents/lib/agentCardModelLabel.test.mjs @@ -56,3 +56,34 @@ test("resolveAgentCardModelLabel — non-inherited agent with a blank resolved m }); assert.equal(label, "Default model (claude-sonnet)"); }); + +test("resolveAgentCardModelLabel — runtime-owned model choice ignores stale Buzz model state", () => { + const label = resolveAgentCardModelLabel({ + agent: { modelSource: "definition", model: "stale-model" }, + personaModel: "stale-persona-model", + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: false, + }); + assert.equal(label, "Runtime default"); +}); + +test("resolveAgentCardModelLabel — supported and unknown capabilities preserve existing labels", () => { + assert.equal( + resolveAgentCardModelLabel({ + agent: { modelSource: "definition", model: "gpt-5" }, + personaModel: null, + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: true, + }), + "gpt-5", + ); + assert.equal( + resolveAgentCardModelLabel({ + agent: undefined, + personaModel: null, + defaultModel: "claude-sonnet", + supportsBuzzModelConfig: null, + }), + "Default model (claude-sonnet)", + ); +}); diff --git a/desktop/src/features/agents/lib/agentCardModelLabel.ts b/desktop/src/features/agents/lib/agentCardModelLabel.ts index 4ec06f10c5..c15a606d6a 100644 --- a/desktop/src/features/agents/lib/agentCardModelLabel.ts +++ b/desktop/src/features/agents/lib/agentCardModelLabel.ts @@ -22,7 +22,11 @@ export function resolveAgentCardModelLabel(input: { agent: Pick | undefined; personaModel: string | null | undefined; defaultModel: string; + supportsBuzzModelConfig?: boolean | null; }): string { + if (input.supportsBuzzModelConfig === false) { + return "Runtime default"; + } if (input.agent) { const isInherited = !input.agent.modelSource || input.agent.modelSource === "global"; diff --git a/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs b/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs new file mode 100644 index 0000000000..c320a2ef9e --- /dev/null +++ b/desktop/src/features/agents/lib/agentRespondToUpdate.test.mjs @@ -0,0 +1,32 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { buildAgentRespondToUpdate } from "./agentRespondToUpdate.ts"; + +const ALLOWED_PUBKEY = "c".repeat(64); + +test("instance edits always submit the form's respond-to mode", () => { + assert.deepEqual(buildAgentRespondToUpdate("owner-only", []), { + respondTo: "owner-only", + respondToAllowlist: undefined, + }); +}); + +test("allowlist edits always submit the complete form allowlist", () => { + const allowlist = [ALLOWED_PUBKEY]; + + const update = buildAgentRespondToUpdate("allowlist", allowlist); + + assert.deepEqual(update, { + respondTo: "allowlist", + respondToAllowlist: [ALLOWED_PUBKEY], + }); + assert.notEqual(update.respondToAllowlist, allowlist); +}); + +test("non-allowlist edits do not overwrite a preserved allowlist", () => { + assert.deepEqual(buildAgentRespondToUpdate("anyone", [ALLOWED_PUBKEY]), { + respondTo: "anyone", + respondToAllowlist: undefined, + }); +}); diff --git a/desktop/src/features/agents/lib/agentRespondToUpdate.ts b/desktop/src/features/agents/lib/agentRespondToUpdate.ts new file mode 100644 index 0000000000..fad21a7601 --- /dev/null +++ b/desktop/src/features/agents/lib/agentRespondToUpdate.ts @@ -0,0 +1,29 @@ +import type { + RespondToMode, + UpdateManagedAgentInput, +} from "@/shared/api/types"; + +type RespondToUpdate = Pick< + UpdateManagedAgentInput, + "respondTo" | "respondToAllowlist" +>; + +/** + * Build the authoritative inbound-author policy for an instance edit. + * + * The edit dialog stays mounted while managed-agent polling can replace its + * `agent` prop. Its local form state is therefore the only reliable snapshot + * of what the user is saving. Always send that state instead of diffing it + * against a possibly refreshed prop; Rust validates the merged policy and the + * retention layer suppresses unchanged relay publications. + */ +export function buildAgentRespondToUpdate( + respondTo: RespondToMode, + respondToAllowlist: string[], +): RespondToUpdate { + return { + respondTo, + respondToAllowlist: + respondTo === "allowlist" ? [...respondToAllowlist] : undefined, + }; +} diff --git a/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs b/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs index 597b1b9323..46b03ec6bb 100644 --- a/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs +++ b/desktop/src/features/agents/lib/friendlyAgentLastError.test.mjs @@ -47,6 +47,15 @@ test("generic harness exit message → passthrough", () => { }); }); +test("Devin ACP startup failure remains distinct and actionable", () => { + const startupFailure = + "failed to start Devin ACP: process exited before initialization"; + assert.deepEqual(friendlyAgentLastError(startupFailure), { + severity: "generic", + copy: startupFailure, + }); +}); + test("trims whitespace before matching", () => { const result = friendlyAgentLastError( " Agent reported error: llm auth: nope\n", diff --git a/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs new file mode 100644 index 0000000000..124f0b18a3 --- /dev/null +++ b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.test.mjs @@ -0,0 +1,67 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveAgentCardAvatarUrl } from "./resolveAgentCardAvatarUrl.ts"; + +test("stopped legacy agents use the catalog-projected runtime avatar", () => { + assert.equal( + resolveAgentCardAvatarUrl([null, null, "app-avatar://devin"]), + "app-avatar://devin", + ); +}); + +test("stored and custom profile avatars outrank the runtime fallback", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + " https://relay.example/custom.png ", + "https://stored.example/custom.png", + "app-avatar://devin", + ]), + "https://relay.example/custom.png", + ); + assert.equal( + resolveAgentCardAvatarUrl([ + " ", + "https://stored.example/custom.png", + "app-avatar://devin", + ]), + "https://stored.example/custom.png", + ); +}); + +test("superseded relay defaults fall through to the current runtime avatar", () => { + const legacy = "https://runtime.example/old-default.svg"; + assert.equal( + resolveAgentCardAvatarUrl( + [null, legacy, "data:image/svg+xml,current"], + [legacy], + ), + "data:image/svg+xml,current", + ); +}); + +test("persona callers preserve custom instance avatars before the runtime fallback", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + null, + "https://stored.example/custom.png", + "/runtime-icons/current.svg", + ]), + "https://stored.example/custom.png", + ); +}); + +test("persona-only cards use the catalog runtime icon before an instance exists", () => { + assert.equal( + resolveAgentCardAvatarUrl([ + null, + "/runtime-icons/devin.svg", + "data:image/svg+xml,runtime-avatar", + ]), + "/runtime-icons/devin.svg", + ); +}); + +test("missing custom and runtime avatars preserve the initials fallback", () => { + assert.equal(resolveAgentCardAvatarUrl([undefined, "", null]), null); +}); diff --git a/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts new file mode 100644 index 0000000000..6b31195a66 --- /dev/null +++ b/desktop/src/features/agents/lib/resolveAgentCardAvatarUrl.ts @@ -0,0 +1,21 @@ +/** + * Select the first non-empty avatar source for an agent-management card. + * + * Callers provide sources in presentation-priority order. The final source is + * normally the runtime-catalog fallback projected by the backend, so stopped + * legacy agents still render their runtime logo without overwriting stored or + * relay-published custom avatars. + */ +export function resolveAgentCardAvatarUrl( + candidates: Array, + supersededRuntimeAvatarUrls: readonly string[] = [], +): string | null { + const superseded = new Set( + supersededRuntimeAvatarUrls.map((candidate) => candidate.trim()), + ); + for (const candidate of candidates) { + const trimmed = candidate?.trim(); + if (trimmed && !superseded.has(trimmed)) return trimmed; + } + return null; +} diff --git a/desktop/src/features/agents/ui/AgentConfigPanel.tsx b/desktop/src/features/agents/ui/AgentConfigPanel.tsx index 3236b4d808..dcf5bf5569 100644 --- a/desktop/src/features/agents/ui/AgentConfigPanel.tsx +++ b/desktop/src/features/agents/ui/AgentConfigPanel.tsx @@ -168,6 +168,7 @@ function NormalizedRow({ field, isPreSpawn, configFilePath, + runtimeControlsModel = false, variant = "compact", }: { fieldKey: keyof NormalizedConfig; @@ -175,26 +176,31 @@ function NormalizedRow({ field: NormalizedField; isPreSpawn: boolean; configFilePath: string | null; + runtimeControlsModel?: boolean; variant?: RowVariant; }) { const Icon = NORMALIZED_ICONS[fieldKey]; // ACP-sourced origins only become meaningful post-spawn const isAcpOnly = field.origin === "acpNativeRead" || field.origin === "acpConfigOption"; - const rawDisplayValue = - isPreSpawn && isAcpOnly + const rawDisplayValue = runtimeControlsModel + ? "Runtime default" + : isPreSpawn && isAcpOnly ? "Available after agent starts" : (field.value ?? "—"); const displayValue = fieldKey === "provider" ? providerDisplayLabel(rawDisplayValue) : rawDisplayValue; - const provenance = field.value - ? provenanceSentence(field.origin, field.writeVia, configFilePath) - : null; - const locked = isReadOnlyField(field); + const provenance = runtimeControlsModel + ? "Controlled by runtime" + : field.value + ? provenanceSentence(field.origin, field.writeVia, configFilePath) + : null; + const locked = runtimeControlsModel || isReadOnlyField(field); const isCopyable = variant === "profile" && + !runtimeControlsModel && shouldOfferCopy({ fieldKey, origin: field.origin, @@ -216,7 +222,7 @@ function NormalizedRow({ )} {displayValue} {!(isPreSpawn && isAcpOnly) && field.overriddenValue ? ( @@ -417,6 +423,9 @@ export function AgentConfigPanel({ field={field} isPreSpawn={isPreSpawn} configFilePath={configFilePath} + runtimeControlsModel={ + key === "model" && data.supportsBuzzModelConfig === false + } variant={advancedMode === "flat" ? "profile" : "compact"} /> )) diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 601d57f95d..694d1af10a 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -11,6 +11,7 @@ import { useStartManagedAgentMutation, useUpdateManagedAgentMutation, } from "@/features/agents/hooks"; +import { buildAgentRespondToUpdate } from "@/features/agents/lib/agentRespondToUpdate"; import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; import type { ManagedAgent, @@ -631,6 +632,10 @@ export function AgentInstanceEditDialog({ // all agree. See resolveInheritedRuntimeSubmission. const normalizedSubmitProvider = inheritedSubmission.provider; const submitEnvVars = inheritedSubmission.envVars; + const respondToUpdate = buildAgentRespondToUpdate( + respondTo, + respondToAllowlist, + ); const input: UpdateManagedAgentInput = { pubkey: agent.pubkey, name: name.trim() !== agent.name ? name.trim() : undefined, @@ -688,18 +693,7 @@ export function AgentInstanceEditDialog({ envVars: envVarsEqual(submitEnvVars, agent.envVars) ? undefined : submitEnvVars, - respondTo: respondTo !== agent.respondTo ? respondTo : undefined, - // The allowlist is preserved across mode toggles in local UI state - // (so a user can flip away from allowlist and back without losing - // their entries), but we only send it on the wire when (a) it - // actually changed, AND (b) the saved mode will need it. Sending - // an allowlist while switching to a non-allowlist mode would be - // harmless server-side, but it's noise in the persisted record. - respondToAllowlist: - respondTo === "allowlist" && - respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") - ? respondToAllowlist - : undefined, + ...respondToUpdate, }; const result = await updateMutation.mutateAsync(input); diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index 8e1c47c615..b3039b4f4d 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -142,6 +142,7 @@ export function AgentsView() {
{groups.map((group) => { const profileAgent = pickProfileAgent(group.agents); + const personaRuntime = runtimes.find( + (runtime) => runtime.id === group.persona.runtime, + ); return ( ; onOpenAgentProfile: ( @@ -270,12 +283,28 @@ function AgentPersonaCard({ agent, personaModel: persona.model, defaultModel, + supportsBuzzModelConfig: + runtime?.supportsBuzzModelConfig ?? + agent?.supportsBuzzModelConfig ?? + null, }); const isActive = agent ? isManagedAgentActive(agent) : false; const profileQuery = useUserProfileQuery(agent?.pubkey); const avatarUrl = agent - ? firstAvatarUrl(persona.avatarUrl, profileQuery.data?.avatarUrl) - : persona.avatarUrl; + ? resolveAgentCardAvatarUrl( + [ + persona.avatarUrl, + agent.avatarUrl, + profileQuery.data?.avatarUrl, + runtime?.iconUrl ?? agent.runtimeIconUrl, + runtime?.avatarUrl ?? agent.runtimeAvatarUrl, + ], + runtime?.supersededAvatarUrls ?? agent.runtimeSupersededAvatarUrls, + ) + : resolveAgentCardAvatarUrl( + [persona.avatarUrl, runtime?.iconUrl, runtime?.avatarUrl], + runtime?.supersededAvatarUrls ?? [], + ); const friendlyError = agent ? friendlyAgentLastError(agent.lastError, agent.lastErrorCode)?.copy : null; @@ -368,6 +397,10 @@ function StandaloneAgentCard({ )?.copy; const isActive = isManagedAgentActive(agent); const opensRuntimeTab = Boolean(friendlyError && !isActive); + const avatarUrl = resolveAgentCardAvatarUrl( + [agent.avatarUrl, profileQuery.data?.avatarUrl, agent.runtimeAvatarUrl], + agent.runtimeSupersededAvatarUrls, + ); return ( onStartAgent(agent.pubkey)} /> } - avatarUrl={profileQuery.data?.avatarUrl} + avatarUrl={avatarUrl} dataTestId={`managed-agent-${agent.pubkey}`} label={title} modelLabel={resolveAgentCardModelLabel({ agent, personaModel: null, defaultModel, + supportsBuzzModelConfig: agent.supportsBuzzModelConfig, })} onClick={() => { onOpenAgentProfile( @@ -419,16 +453,6 @@ function StandaloneAgentCard({ ); } -function firstAvatarUrl( - ...candidates: Array -): string | null { - for (const candidate of candidates) { - const trimmed = candidate?.trim(); - if (trimmed) return trimmed; - } - return null; -} - function NewAgentCard({ canChooseCatalog, isPersonasPending, diff --git a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx index 53a00e6463..b099bce290 100644 --- a/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx +++ b/desktop/src/features/agents/ui/activityRenderClasses/LifecycleActivity.tsx @@ -1,5 +1,11 @@ +import * as React from "react"; import { AlertCircle, CheckCircle2, ShieldCheck, XCircle } from "lucide-react"; +import { toast } from "sonner"; +import { resolveManagedAgentPermission } from "@/shared/api/agentControl"; +import { Button } from "@/shared/ui/button"; +import { oneShotPermissionOptions } from "../agentSessionPermission"; +import type { AgentPermissionOption } from "../agentSessionTypes"; import { formatTranscriptTimestampTitle } from "../agentSessionUtils"; import { ActivityRow, ActivityRowLabel } from "./ActivityRow"; import { ToolActivity } from "./ToolActivity"; @@ -37,6 +43,82 @@ function permissionOutcomeTone(outcome: string): "approve" | "deny" | "cancel" { return "cancel"; } +function PermissionDecisionActions({ + agentPubkey, + channelId, + requestId, + turnId, + options, +}: { + agentPubkey: string; + channelId: string; + requestId: string | number; + turnId: string; + options: AgentPermissionOption[]; +}) { + const [submitting, setSubmitting] = React.useState(null); + + const decide = React.useCallback( + async (optionId: string) => { + setSubmitting(optionId); + try { + await resolveManagedAgentPermission( + agentPubkey, + channelId, + turnId, + requestId, + optionId, + ); + } catch { + setSubmitting(null); + toast.error("Couldn’t send the permission decision. Try again."); + } + }, + [agentPubkey, channelId, requestId, turnId], + ); + + const oneShotOptions = oneShotPermissionOptions(options); + const allowOnce = oneShotOptions.find( + (option) => option.kind === "allow_once", + ); + const rejectOnce = oneShotOptions.find( + (option) => option.kind === "reject_once", + ); + + return ( + <> +
+
+ Permission decision + {allowOnce ? ( + + ) : null} + {rejectOnce ? ( + + ) : null} +
+ + ); +} + export function LifecycleActivity(props: ActivityRenderClassItemProps) { if (props.item.type === "tool") { return ; @@ -97,6 +179,17 @@ export function LifecycleActivity(props: ActivityRenderClassItemProps) { {outcome}
+ ) : props.item.permissionRequestId != null && + props.item.channelId && + props.item.turnId && + props.item.permissionOptions?.length ? ( + ) : null}
); diff --git a/desktop/src/features/agents/ui/agentSessionPermission.ts b/desktop/src/features/agents/ui/agentSessionPermission.ts new file mode 100644 index 0000000000..5fc98c163c --- /dev/null +++ b/desktop/src/features/agents/ui/agentSessionPermission.ts @@ -0,0 +1,114 @@ +import type { + AgentActivityDescriptor, + AgentPermissionOption, +} from "./agentSessionTypes"; +import { asRecord, asString } from "./agentSessionUtils"; + +export type PermissionRequestDescription = { + title: string; + text: string; + optionNames: Map; + permissionOptions: AgentPermissionOption[]; + descriptor: AgentActivityDescriptor; +}; + +/** + * Interactive managed-runtime consent is intentionally one-shot. Persistent + * runtime choices stay visible in the transcript for auditability, but they + * are never returned as actionable controls. + */ +export function oneShotPermissionOptions( + options: AgentPermissionOption[], +): AgentPermissionOption[] { + return options.filter( + (option) => option.kind === "allow_once" || option.kind === "reject_once", + ); +} + +export function describePermissionRequest( + payload: Record, +): PermissionRequestDescription { + const params = asRecord(payload.params); + const toolCall = asRecord(params.toolCall); + const title = + asString(toolCall.title) ?? + asString(params.title) ?? + asString(params.message) ?? + asString(params.reason) ?? + "Permission requested"; + const toolCallId = + asString(toolCall.toolCallId) ?? + asString(toolCall.tool_call_id) ?? + asString(params.toolCallId) ?? + asString(params.tool_call_id); + const permissionOptions: AgentPermissionOption[] = Array.isArray( + params.options, + ) + ? params.options + .map((option) => { + const record = asRecord(option); + const optionId = asString(record.optionId); + const kind = asString(record.kind); + if (!optionId || !kind) return null; + return { + optionId, + kind, + name: asString(record.name) ?? kind, + }; + }) + .filter((option): option is AgentPermissionOption => option !== null) + : []; + const detail: string[] = []; + if (title !== "Permission requested") detail.push(title); + if (toolCallId) detail.push(`Tool call: ${toolCallId}`); + if (permissionOptions.length > 0) { + detail.push( + `Options: ${permissionOptions.map((option) => option.name).join(", ")}`, + ); + } + + const optionNames = new Map(); + for (const option of permissionOptions) { + optionNames.set(option.optionId, option.kind); + } + + return { + title, + text: detail.join("\n"), + optionNames, + permissionOptions, + descriptor: { + renderClass: "permission", + label: "Permission requested", + preview: title, + action: { verb: "Requested", object: title }, + tone: "admin", + operation: "session/request_permission", + object: title, + source: "acp", + groupKey: "permission:request", + }, + }; +} + +/** + * Format a human-readable outcome label from a permission response. + * kind values from ACP: allow_once, allow_always, reject_once, reject_always. + * "reject_*" kinds are denials; anything else that is selected is an approval. + */ +export function describePermissionOutcome( + outcome: string, + optionId: string | null, + optionNames: Map, +): string { + if (outcome === "cancelled") { + return "Cancelled"; + } + if (outcome === "selected" && optionId) { + const kind = optionNames.get(optionId) ?? optionId; + const isDenial = kind.startsWith("reject"); + const verb = isDenial ? "Denied" : "Approved"; + return `${verb} (${kind})`; + } + return outcome; +} diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs index cc6f0467d6..0d17629bc8 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs +++ b/desktop/src/features/agents/ui/agentSessionTranscript.test.mjs @@ -6,6 +6,7 @@ import { buildTranscriptDisplayBlocks, flattenDisplayBlocks, } from "./agentSessionTranscriptGrouping.ts"; +import { oneShotPermissionOptions } from "./agentSessionPermission.ts"; import { formatToolTitle } from "./agentSessionToolCatalog.ts"; const baseEvent = { @@ -572,12 +573,20 @@ test("buildTranscript surfaces session/request_permission as a permission lifecy turnId: "turn-1", payload: { jsonrpc: "2.0", + id: 41, method: "session/request_permission", params: { - toolCallId: "tool-1", - title: "Confirm force-with-lease push to block/buzz.", + toolCall: { + toolCallId: "tool-1", + title: "Confirm force-with-lease push to block/buzz.", + }, options: [ { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, { optionId: "reject_once", kind: "reject_once", name: "Reject" }, ], }, @@ -590,6 +599,41 @@ test("buildTranscript surfaces session/request_permission as a permission lifecy assert.equal(transcript[0].renderClass, "permission"); assert.equal(transcript[0].title, "Permission requested"); assert.match(transcript[0].text, /Confirm force-with-lease push/); + assert.equal(transcript[0].permissionRequestId, 41); + assert.deepEqual(transcript[0].permissionOptions, [ + { optionId: "allow_once", kind: "allow_once", name: "Allow" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, + { optionId: "reject_once", kind: "reject_once", name: "Reject" }, + ]); + assert.equal(transcript[0].channelId, "channel-1"); + assert.equal(transcript[0].turnId, "turn-1"); +}); + +test("interactive permission actions exclude persistent runtime choices", () => { + assert.deepEqual( + oneShotPermissionOptions([ + { optionId: "allow_once", kind: "allow_once", name: "Allow once" }, + { + optionId: "allow_workspace", + kind: "allow_always", + name: "Always allow in this workspace", + }, + { + optionId: "reject_always", + kind: "reject_always", + name: "Always reject", + }, + { optionId: "reject_once", kind: "reject_once", name: "Deny once" }, + ]), + [ + { optionId: "allow_once", kind: "allow_once", name: "Allow once" }, + { optionId: "reject_once", kind: "reject_once", name: "Deny once" }, + ], + ); }); test("buildTranscript stamps completedAt when a terminal tool update is inserted first", () => { @@ -821,6 +865,22 @@ test("buildTranscript no-ops on a permission response with an unmatched id", () assert.doesNotMatch(item.text ?? "", /Denied/); }); +test("buildTranscript keeps multiple permission requests in one turn separate", () => { + const transcript = buildTranscript([ + makePermissionRequest(1, "req-first"), + makePermissionResponse(2, "req-first", "selected", "allow_once"), + makePermissionRequest(3, "req-second"), + makePermissionResponse(4, "req-second", "selected", "reject_once"), + ]); + + assert.equal(transcript.length, 2); + assert.notEqual(transcript[0].id, transcript[1].id); + assert.equal(transcript[0].type, "lifecycle"); + assert.equal(transcript[0].outcome, "Approved (allow_once)"); + assert.equal(transcript[1].type, "lifecycle"); + assert.equal(transcript[1].outcome, "Denied (reject_once)"); +}); + test("buildTranscript appends Approved outcome for a numeric JSON-RPC id (selected allow_once)", () => { // JSON-RPC 2.0 allows numeric ids; the ACP runtime preserves them as // serde_json::Value. asString() drops numbers, so this exercises the diff --git a/desktop/src/features/agents/ui/agentSessionTranscript.ts b/desktop/src/features/agents/ui/agentSessionTranscript.ts index 962290c6ca..c24fe61379 100644 --- a/desktop/src/features/agents/ui/agentSessionTranscript.ts +++ b/desktop/src/features/agents/ui/agentSessionTranscript.ts @@ -1,6 +1,7 @@ import type { AgentActivityDescriptor, AgentActivityRenderClass, + AgentPermissionOption, ObserverEvent, PromptSection, ToolStatus, @@ -12,6 +13,10 @@ import { normalizeToolStatus, } from "./agentSessionToolCatalog"; import { classifyTool } from "./agentSessionToolClassifier"; +import { + describePermissionOutcome, + describePermissionRequest, +} from "./agentSessionPermission"; import { asRecord, asString, titleCase } from "./agentSessionUtils"; import { describeTurnStarted, @@ -171,85 +176,6 @@ function stringifyPayload(value: unknown) { } } -function describePermissionRequest(payload: Record) { - const params = asRecord(payload.params); - const title = - asString(params.title) ?? - asString(params.message) ?? - asString(params.reason) ?? - "Permission requested"; - const toolCallId = - asString(params.toolCallId) ?? asString(params.tool_call_id); - const options = Array.isArray(params.options) - ? params.options - .map((option) => { - const record = asRecord(option); - return ( - asString(record.name) ?? - asString(record.kind) ?? - asString(record.optionId) - ); - }) - .filter((option): option is string => Boolean(option)) - : []; - const detail: string[] = []; - if (title !== "Permission requested") detail.push(title); - if (toolCallId) detail.push(`Tool call: ${toolCallId}`); - if (options.length > 0) detail.push(`Options: ${options.join(", ")}`); - - // Build optionId → kind map for outcome labeling on the response. - const optionNames = new Map(); - if (Array.isArray(params.options)) { - for (const option of params.options) { - const record = asRecord(option); - const optionId = asString(record.optionId); - const kind = asString(record.kind); - if (optionId && kind) { - optionNames.set(optionId, kind); - } - } - } - - return { - title, - text: detail.join("\n"), - optionNames, - descriptor: { - renderClass: "permission" as const, - label: "Permission requested", - preview: title, - action: { verb: "Requested", object: title }, - tone: "admin" as const, - operation: "session/request_permission", - object: title, - source: "acp" as const, - groupKey: "permission:request", - }, - }; -} - -/** - * Format a human-readable outcome label from a permission response. - * kind values from ACP: allow_once, allow_always, reject_once, reject_always. - * "reject_*" kinds are denials; anything else that is selected is an approval. - */ -function describePermissionOutcome( - outcome: string, - optionId: string | null, - optionNames: Map, -): string { - if (outcome === "cancelled") { - return "Cancelled"; - } - if (outcome === "selected" && optionId) { - const kind = optionNames.get(optionId) ?? optionId; - const isDenial = kind.startsWith("reject"); - const verb = isDenial ? "Denied" : "Approved"; - return `${verb} (${kind})`; - } - return outcome; -} - /** * Stable map key for a JSON-RPC id, which may be a string or a finite number * per the spec. Using JSON.stringify avoids collisions between the number 1 and @@ -263,6 +189,12 @@ function jsonRpcId(value: unknown): string | null { return null; } +function jsonRpcIdValue(value: unknown): string | number | null { + if (typeof value === "string") return value; + if (typeof value === "number" && Number.isFinite(value)) return value; + return null; +} + function describeFreeformStatus(payload: Record) { const statusType = asString(payload.type) ?? asString(payload.status); const title = @@ -408,6 +340,8 @@ function upsertLifecycleItem( ctx: TranscriptItemContext, acpSource?: string, descriptor?: AgentActivityDescriptor, + permissionRequestId?: string | number, + permissionOptions?: AgentPermissionOption[], ) { const existing = d.itemsById.get(id); if (existing?.type === "lifecycle") { @@ -417,6 +351,8 @@ function upsertLifecycleItem( title, text: joinLifecycleText(existing.text, text), descriptor: descriptor ?? existing.descriptor, + permissionRequestId: permissionRequestId ?? existing.permissionRequestId, + permissionOptions: permissionOptions ?? existing.permissionOptions, channelId: ctx.channelId, turnId: ctx.turnId ?? existing.turnId, sessionId: ctx.sessionId ?? existing.sessionId, @@ -434,6 +370,8 @@ function upsertLifecycleItem( text, timestamp, descriptor, + permissionRequestId, + permissionOptions, channelId: ctx.channelId, turnId: ctx.turnId, sessionId: ctx.sessionId, @@ -792,7 +730,10 @@ export function processTranscriptEvent( if (method === "session/request_permission") { const request = describePermissionRequest(payload); - const itemId = `permission:${ch}:${event.turnId ?? event.seq}`; + const requestId = jsonRpcId(payload.id); + const itemId = `permission:${ch}:${event.turnId ?? event.seq}:${ + requestId ?? event.seq + }`; upsertLifecycleItem( d, itemId, @@ -803,10 +744,11 @@ export function processTranscriptEvent( ctx, "permission_request", request.descriptor, + jsonRpcIdValue(payload.id) ?? undefined, + request.permissionOptions, ); // Index by JSON-RPC id so the response (acp_write with result.outcome, // no method) can correlate by id rather than by turn/seq. - const requestId = jsonRpcId(payload.id); if (requestId) { d.pendingPermissions = new Map(d.pendingPermissions); d.pendingPermissions.set(requestId, { diff --git a/desktop/src/features/agents/ui/agentSessionTypes.ts b/desktop/src/features/agents/ui/agentSessionTypes.ts index 578f98076c..47bcb689ed 100644 --- a/desktop/src/features/agents/ui/agentSessionTypes.ts +++ b/desktop/src/features/agents/ui/agentSessionTypes.ts @@ -68,6 +68,12 @@ export type TranscriptItemIdentity = { channelId?: string | null; }; +export type AgentPermissionOption = { + optionId: string; + name: string; + kind: string; +}; + export type TranscriptItem = | ({ id: string; @@ -109,6 +115,10 @@ export type TranscriptItem = text: string; /** Resolved outcome for permission items (e.g. "Approved (allow_once)", "Denied (reject_once)", "Cancelled"). */ outcome?: string; + /** Raw ACP JSON-RPC id used for an exact interactive decision match. */ + permissionRequestId?: string | number; + /** Exact options offered by the runtime for this live request. */ + permissionOptions?: AgentPermissionOption[]; timestamp: string; descriptor?: AgentActivityDescriptor; acpSource?: TranscriptAcpSource; diff --git a/desktop/src/features/channels/ui/BotActivityBar.tsx b/desktop/src/features/channels/ui/BotActivityBar.tsx index 21ab04f8d6..cb22f40673 100644 --- a/desktop/src/features/channels/ui/BotActivityBar.tsx +++ b/desktop/src/features/channels/ui/BotActivityBar.tsx @@ -57,6 +57,24 @@ export function BotActivityComposerAction({ Boolean(singleWorkingAgent), singleWorkingAgent?.pubkey, ); + const pendingPermission = React.useMemo(() => { + if (!singleWorkingAgent) return null; + const scoped = channelId + ? transcript.filter((item) => item.channelId === channelId) + : transcript; + for (let index = scoped.length - 1; index >= 0; index--) { + const item = scoped[index]; + if ( + item?.type === "lifecycle" && + item.renderClass === "permission" && + !item.outcome && + item.permissionRequestId != null + ) { + return item; + } + } + return null; + }, [channelId, singleWorkingAgent, transcript]); const activityHeadlines = React.useMemo(() => { if (!singleWorkingAgent) { return []; @@ -144,17 +162,21 @@ export function BotActivityComposerAction({ profiles?.[agent.pubkey.toLowerCase()]?.avatarUrl ?? null; const selectedPubkey = openAgentSessionPubkey?.toLowerCase() ?? null; const triggerLabel = - workingAgents.length === 1 - ? `${workingAgents[0]?.name ?? "Agent"} is working` - : `${workingAgents.length} agents working`; + pendingPermission && singleWorkingAgent + ? `${singleWorkingAgent.name} needs permission` + : workingAgents.length === 1 + ? `${workingAgents[0]?.name ?? "Agent"} is working` + : `${workingAgents.length} agents working`; const isInline = variant === "inline"; const visibleStatusLabel = - workingAgents.length === 1 - ? `${workingAgents[0]?.name ?? "Agent"}: ${ - activityHeadlines[headlineIndex % activityHeadlines.length] ?? - "Working" - }` - : `${workingAgents[0]?.name ?? "Agent"} +${workingAgents.length - 1}`; + pendingPermission && singleWorkingAgent + ? `${singleWorkingAgent.name}: Approval required` + : workingAgents.length === 1 + ? `${workingAgents[0]?.name ?? "Agent"}: ${ + activityHeadlines[headlineIndex % activityHeadlines.length] ?? + "Working" + }` + : `${workingAgents[0]?.name ?? "Agent"} +${workingAgents.length - 1}`; return ( @@ -166,11 +188,17 @@ export function BotActivityComposerAction({ isInline ? "h-7 min-w-0 gap-2 overflow-visible border-transparent bg-transparent px-0 text-xs font-semibold leading-none shadow-none hover:border-transparent hover:bg-transparent data-[state=open]:border-transparent data-[state=open]:bg-transparent" : "h-9 min-w-9 gap-1.5 px-2 text-xs", + pendingPermission ? "text-amber-700 dark:text-amber-400" : null, )} data-testid="bot-activity-composer-trigger" onBlur={closeWithDelay} onClick={() => { clearHoverTimer(); + if (pendingPermission && singleWorkingAgent) { + setOpen(false); + onOpenAgentSession(singleWorkingAgent.pubkey, channelId); + return; + } setOpen((current) => !current); }} onFocus={() => setOpen(true)} diff --git a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx index 2debea409b..add4315b08 100644 --- a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx +++ b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx @@ -3,18 +3,6 @@ import { TerminalSquare } from "lucide-react"; import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; import { cn } from "@/shared/lib/cn"; -import { useTheme } from "@/shared/theme/ThemeProvider"; -import { BuzzMark } from "@/shared/ui/buzz-logo/BuzzMark"; -import chatgptLogoUrl from "../assets/harness-logos/chatgpt.png?inline"; -import claudeLogoUrl from "../assets/harness-logos/claude.png?inline"; -import gooseLogoUrl from "../assets/harness-logos/goose.png?inline"; - -// Bundled logos for compiled-in runtimes (inline base64, no network fetch). -const RUNTIME_LOGOS: Record = { - claude: claudeLogoUrl, - codex: chatgptLogoUrl, - goose: gooseLogoUrl, -}; // Public-path logos for bundled presets. Served from /harness-logos/ at runtime. // Keys match the preset `id` values emitted by the backend PRESET_HARNESSES. @@ -28,19 +16,22 @@ export const PRESET_LOGOS: Record = { openclaw: "/harness-logos/openclaw.svg", }; -function isBuzzRuntime(runtime: AcpRuntimeCatalogEntry): boolean { - return runtime.id.trim().toLowerCase() === "buzz-agent"; -} - export function getRuntimeDisplayLabel( runtime: AcpRuntimeCatalogEntry, ): string { - return isBuzzRuntime(runtime) ? "Buzz" : runtime.label; + return runtime.displayLabel; } function getRuntimeLogoUrl(runtime: AcpRuntimeCatalogEntry): string | null { const id = runtime.id.trim().toLowerCase(); - return RUNTIME_LOGOS[id] ?? PRESET_LOGOS[id] ?? null; + if (runtime.source === "builtin") { + return runtime.iconUrl || null; + } + if (runtime.source === "preset") { + return PRESET_LOGOS[id] ?? null; + } + // Never render user-controlled custom avatar URLs in onboarding. + return null; } export function RuntimeIcon({ @@ -51,16 +42,8 @@ export function RuntimeIcon({ runtime: AcpRuntimeCatalogEntry; }) { const [imageFailed, setImageFailed] = React.useState(false); - const { isDark } = useTheme(); - // Only use bundled logo maps — never render user-supplied avatar URLs for - // custom/preset entries (tracking pixel / spoofing vector, security line). const id = runtime.id.trim().toLowerCase(); const imageUrl = getRuntimeLogoUrl(runtime); - const shouldForceForegroundColor = !imageUrl && id === "goose"; - - if (isBuzzRuntime(runtime)) { - return ; - } if (imageUrl && !imageFailed) { return ( @@ -71,11 +54,10 @@ export function RuntimeIcon({ className, id === "omp" && "bg-[#0d0d0d] p-1", id === "grok" && "bg-white p-1", - shouldForceForegroundColor && - (isDark ? "brightness-0 invert" : "brightness-0"), )} onError={() => setImageFailed(true)} src={imageUrl} + style={{ transform: `scale(${runtime.iconScale})` }} /> ); } diff --git a/desktop/src/features/onboarding/ui/agentReadiness.test.mjs b/desktop/src/features/onboarding/ui/agentReadiness.test.mjs index 9d6deafb09..2ce228038d 100644 --- a/desktop/src/features/onboarding/ui/agentReadiness.test.mjs +++ b/desktop/src/features/onboarding/ui/agentReadiness.test.mjs @@ -15,6 +15,9 @@ function makeRuntime(overrides = {}) { binaryPath: "/usr/local/bin/goose", defaultArgs: [], mcpCommand: null, + modelEnvVar: "GOOSE_MODEL", + providerEnvVar: "GOOSE_PROVIDER", + thinkingEnvVar: "GOOSE_THINKING_EFFORT", installHint: "", installInstructionsUrl: "https://example.com", canAutoInstall: false, @@ -40,7 +43,15 @@ function makeConfig(overrides = {}) { // --------------------------------------------------------------------------- test("resolveAgentReadiness_cli_returns_ready_when_preferred_cli_runtime_is_logged_in", () => { - const runtimes = [makeRuntime({ id: "claude", label: "Claude" })]; + const runtimes = [ + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; const result = resolveAgentReadiness( runtimes, makeConfig({ preferred_runtime: "claude" }), @@ -52,9 +63,37 @@ test("resolveAgentReadiness_cli_returns_ready_when_preferred_cli_runtime_is_logg }); }); +test("resolveAgentReadiness_devin_uses_catalog_capabilities_without_an_id_check", () => { + const runtimes = [ + makeRuntime({ + id: "devin", + label: "Devin", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; + const result = resolveAgentReadiness( + runtimes, + makeConfig({ preferred_runtime: "devin" }), + "preferred", + ); + assert.deepEqual(result, { + ready: true, + reason: "cli", + runtimeLabel: "Devin", + }); +}); + test("resolveAgentReadiness_uses_only_the_preferred_runtime", () => { const runtimes = [ - makeRuntime({ id: "claude", label: "Claude" }), + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), makeRuntime({ id: "goose", label: "Goose" }), ]; const result = resolveAgentReadiness(runtimes, makeConfig(), "preferred"); @@ -197,7 +236,15 @@ test("resolveAgentReadiness_neither_returns_not_ready", () => { }); test("resolveAgentReadiness_welcome_readiness_uses_ready_cli_without_preference", () => { - const runtimes = [makeRuntime({ id: "claude", label: "Claude" })]; + const runtimes = [ + makeRuntime({ + id: "claude", + label: "Claude", + modelEnvVar: null, + providerEnvVar: null, + thinkingEnvVar: null, + }), + ]; const result = resolveAgentReadiness( runtimes, makeConfig({ preferred_runtime: null }), diff --git a/desktop/src/features/onboarding/ui/agentReadiness.ts b/desktop/src/features/onboarding/ui/agentReadiness.ts index 86b9721af3..d92d1467c8 100644 --- a/desktop/src/features/onboarding/ui/agentReadiness.ts +++ b/desktop/src/features/onboarding/ui/agentReadiness.ts @@ -12,7 +12,8 @@ export type AgentReadinessResult = /** * Determine whether the user has a working agent path configured. * - * CLI path: the preferred Claude or Codex runtime is available and logged in. + * CLI path: a catalog-declared runtime without provider configuration is + * available and logged in. * Provider path: the preferred Buzz Agent or Goose runtime has provider and * model set, plus all required credential env vars for that provider. * @@ -47,7 +48,7 @@ export function resolveAgentReadiness( } if ( - (preferredRuntime.id === "claude" || preferredRuntime.id === "codex") && + preferredRuntime.providerEnvVar == null && (preferredRuntime.authStatus.status === "logged_in" || preferredRuntime.authStatus.status === "not_applicable") ) { diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs index 221702ebb2..e7c7e459c5 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.test.mjs @@ -8,29 +8,64 @@ import { runtimeIsVisibleInOnboarding, } from "./onboardingRuntimeSelection.ts"; -function runtime(id, availability, status) { - return { id, availability, authStatus: { status } }; +function runtime( + id, + availability, + status, + { onboardingVisible = true, sortPriority = 100 } = {}, +) { + return { + id, + availability, + authStatus: { status }, + onboardingVisible, + sortPriority, + }; } -test("all bundled harnesses are visible in onboarding", () => { - assert.equal(runtimeIsVisibleInOnboarding("claude"), true); - assert.equal(runtimeIsVisibleInOnboarding("codex"), true); - assert.equal(runtimeIsVisibleInOnboarding("goose"), true); - assert.equal(runtimeIsVisibleInOnboarding("buzz-agent"), true); - assert.equal(runtimeIsVisibleInOnboarding("custom"), false); +test("onboarding visibility comes from catalog metadata", () => { + assert.equal( + runtimeIsVisibleInOnboarding(runtime("devin", "available", "logged_in")), + true, + ); + assert.equal( + runtimeIsVisibleInOnboarding( + runtime("future-runtime", "available", "logged_in", { + onboardingVisible: false, + }), + ), + false, + ); }); -test("visible onboarding runtimes use the product order", () => { +test("visible onboarding runtimes use catalog ordering", () => { const runtimes = [ - runtime("buzz-agent", "available", "not_applicable"), - runtime("codex", "available", "logged_in"), - runtime("goose", "available", "not_applicable"), - runtime("claude", "available", "logged_in"), + runtime("buzz-agent", "available", "not_applicable", { + sortPriority: 60, + }), + runtime("codex", "available", "logged_in", { sortPriority: 40 }), + runtime("goose", "available", "not_applicable", { + sortPriority: 50, + }), + runtime("devin", "available", "logged_in", { sortPriority: 20 }), + runtime("claude", "available", "logged_in", { sortPriority: 30 }), ]; assert.deepEqual( getVisibleOnboardingRuntimes(runtimes).map(({ id }) => id), - ["claude", "codex", "goose", "buzz-agent"], + ["devin", "claude", "codex", "goose", "buzz-agent"], + ); +}); + +test("catalog ordering falls back to labels for rolling-upgrade payloads", () => { + const alpha = runtime("alpha", "available", "logged_in"); + alpha.label = "Alpha"; + const beta = runtime("beta", "available", "logged_in"); + beta.label = "Beta"; + + assert.deepEqual( + getVisibleOnboardingRuntimes([beta, alpha]).map(({ id }) => id), + ["alpha", "beta"], ); }); @@ -39,6 +74,10 @@ test("readiness requires an available and authenticated runtime", () => { runtimeIsReadyForOnboarding(runtime("claude", "available", "logged_in")), true, ); + assert.equal( + runtimeIsReadyForOnboarding(runtime("devin", "available", "logged_in")), + true, + ); assert.equal( runtimeIsReadyForOnboarding( runtime("codex", "available", "not_applicable"), @@ -57,11 +96,14 @@ test("readiness requires an available and authenticated runtime", () => { test("ready onboarding runtimes exclude unknown and non-ready harnesses", () => { const runtimes = [ - runtime("goose", "available", "not_applicable"), - runtime("codex", "available", "logged_out"), - runtime("buzz-agent", "available", "not_applicable"), - runtime("claude", "available", "logged_in"), - runtime("custom", "available", "not_applicable"), + runtime("goose", "available", "not_applicable", { sortPriority: 50 }), + runtime("codex", "available", "logged_out", { sortPriority: 40 }), + runtime("buzz-agent", "available", "not_applicable", { sortPriority: 60 }), + runtime("devin", "available", "logged_out", { sortPriority: 20 }), + runtime("claude", "available", "logged_in", { sortPriority: 30 }), + runtime("custom", "available", "not_applicable", { + onboardingVisible: false, + }), ]; assert.deepEqual( diff --git a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts index cd491dfcc5..baef13ac34 100644 --- a/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts +++ b/desktop/src/features/onboarding/ui/onboardingRuntimeSelection.ts @@ -1,18 +1,7 @@ import type { AcpRuntimeCatalogEntry } from "@/shared/api/types"; -export const ONBOARDING_RUNTIME_ORDER = [ - "claude", - "codex", - "goose", - "buzz-agent", -]; - -const VISIBLE_ONBOARDING_RUNTIME_IDS = new Set( - ONBOARDING_RUNTIME_ORDER, -); - -export function runtimeIsVisibleInOnboarding(runtimeId: string) { - return VISIBLE_ONBOARDING_RUNTIME_IDS.has(runtimeId); +export function runtimeIsVisibleInOnboarding(runtime: AcpRuntimeCatalogEntry) { + return runtime.onboardingVisible; } export function runtimeIsReadyForOnboarding(runtime: AcpRuntimeCatalogEntry) { @@ -27,11 +16,13 @@ export function getVisibleOnboardingRuntimes( runtimes: readonly AcpRuntimeCatalogEntry[], ) { return runtimes - .filter((runtime) => runtimeIsVisibleInOnboarding(runtime.id)) + .filter(runtimeIsVisibleInOnboarding) .sort( (left, right) => - ONBOARDING_RUNTIME_ORDER.indexOf(left.id) - - ONBOARDING_RUNTIME_ORDER.indexOf(right.id), + left.sortPriority - right.sortPriority || + (left.displayLabel || left.label || left.id).localeCompare( + right.displayLabel || right.label || right.id, + ), ); } diff --git a/desktop/src/features/profile/ui/ProfileAvatar.tsx b/desktop/src/features/profile/ui/ProfileAvatar.tsx index 3153fb4be1..41f70ff819 100644 --- a/desktop/src/features/profile/ui/ProfileAvatar.tsx +++ b/desktop/src/features/profile/ui/ProfileAvatar.tsx @@ -16,6 +16,7 @@ type ProfileAvatarProps = { className?: string; iconClassName?: string; imageClassName?: string; + imageStyle?: React.CSSProperties; plain?: boolean; testId?: string; }; @@ -27,6 +28,7 @@ export function ProfileAvatar({ className, iconClassName, imageClassName, + imageStyle, plain = false, testId, }: ProfileAvatarProps) { @@ -87,6 +89,7 @@ export function ProfileAvatar({ }} referrerPolicy="no-referrer" src={src} + style={imageStyle} /> ) : null} {shouldShowFallback ? ( diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index c1f713d230..4015ef3180 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -80,6 +80,7 @@ import { resolveAgentInstruction, resolvePanelProfile, resolveProfileDisplayName, + resolveProfileEditTarget, truncatePubkey, type UserProfilePanelProps, useRetainedPersona, @@ -398,12 +399,18 @@ export function UserProfilePanel({ }); const handleEditAgent = React.useCallback(() => { - if (resolvedPersona) { + // See resolveProfileEditTarget: an instance-backed profile must edit the + // instance, whose respond-to pair is the one enforced at spawn. + const target = resolveProfileEditTarget({ + hasManagedInstance: managedAgent !== undefined, + hasDefinition: resolvedPersona !== undefined, + }); + if (target === "definition" && resolvedPersona) { setPersonaDialogState(editPersonaDialogState(resolvedPersona)); return; } setEditAgentOpen(true); - }, [resolvedPersona]); + }, [managedAgent, resolvedPersona]); const { deleteManagedAgentRecord, deleteManagedAgentsForPersona } = useProfileAgentDeletion({ diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs index 89837f6017..3cb9b8b0d7 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.test.mjs @@ -7,6 +7,7 @@ import { personaManagedAgentUpdate, profilePanelTabFromSearch, profilePanelViewFromSearch, + resolveProfileEditTarget, } from "./UserProfilePanelUtils.ts"; function agent(overrides = {}) { @@ -189,3 +190,53 @@ test("profilePanelTabFromSearch falls back to info for invalid values", () => { assert.equal(profilePanelTabFromSearch("missing"), "info"); assert.equal(profilePanelTabFromSearch(null), "info"); }); + +// ── Profile Edit routing: displayed policy must be the enforced policy ─────── +// +// Regression: a persona-linked agent routed Edit to the DEFINITION editor, so +// the dialog showed the definition's inbound-author policy while the running +// agent still enforced the instance's own policy. A definition's behavior +// group is copied onto an instance only at mint time, so an owner who granted +// (or revoked) access there changed nothing about the live agent. + +test("resolveProfileEditTarget: an instance-backed profile edits the instance", () => { + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: true, + hasDefinition: true, + }), + "instance", + "a persona-linked instance must still edit the instance it displays", + ); + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: true, + hasDefinition: false, + }), + "instance", + ); +}); + +test("resolveProfileEditTarget: a definition-only profile edits the definition", () => { + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: false, + hasDefinition: true, + }), + "definition", + "with no minted instance the definition is the only editable record", + ); +}); + +test("resolveProfileEditTarget: no instance and no definition falls back to instance", () => { + // Preserves the pre-existing fallback: the caller renders the instance + // dialog only when a managed agent exists, so this is inert rather than a + // route into a dialog that cannot edit anything. + assert.equal( + resolveProfileEditTarget({ + hasManagedInstance: false, + hasDefinition: false, + }), + "instance", + ); +}); diff --git a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts index 07f57803b4..951a0905f7 100644 --- a/desktop/src/features/profile/ui/UserProfilePanelUtils.ts +++ b/desktop/src/features/profile/ui/UserProfilePanelUtils.ts @@ -259,6 +259,39 @@ export function resolveAgentInstruction( ); } +/** + * Decide which editor the profile panel's Edit action must open. + * + * The profile panel is an *instance* view: it shows this agent's own public + * key, runtime state, and Stop/Restart controls. So when a concrete managed + * instance exists, Edit has to open the instance editor — the instance + * record's `respond_to`/allowlist is the pair `build_respond_to_env` turns + * into `BUZZ_ACP_RESPOND_TO` at spawn, and it is therefore the only policy the + * running agent actually enforces. + * + * Routing an instance-backed profile to the definition editor instead lets the + * dialog display an inbound-author policy that the live agent does not apply: + * a definition's behavior group is copied onto an instance only when a *new* + * instance is minted from it, never onto instances that already exist. An + * owner who added someone to an allowlist there would believe they had granted + * access — and, worse, an owner who removed someone would believe they had + * revoked it — while the running agent kept its original policy. + * + * Definition editing stays reachable: the agent library's actions menu opens + * the definition editor directly, and the instance editor offers a hop to the + * linked definition. + */ +export function resolveProfileEditTarget({ + hasManagedInstance, + hasDefinition, +}: { + hasManagedInstance: boolean; + hasDefinition: boolean; +}): "instance" | "definition" { + if (hasManagedInstance) return "instance"; + return hasDefinition ? "definition" : "instance"; +} + export function personaManagedAgentUpdate( agent: ManagedAgent, persona: AgentPersona, diff --git a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx index 96127876a8..d1d669a532 100644 --- a/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx +++ b/desktop/src/features/settings/ui/DoctorSettingsPanel.tsx @@ -35,25 +35,6 @@ import { SectionHeader } from "@/shared/ui/PageHeader"; import { Spinner } from "@/shared/ui/spinner"; import { Switch } from "@/shared/ui/switch"; -const RUNTIME_LOGO_URLS: Record = { - "buzz-agent": "/app-icon@2x.png", - claude: "/runtime-icons/claude.png", - codex: "/runtime-icons/codex.png", - goose: "/runtime-icons/goose.svg", -}; - -const RUNTIME_LOGO_SCALE: Record = { - "buzz-agent": "scale-110", - claude: "scale-110", - codex: "scale-110", - goose: "scale-125", -}; - -const RUNTIME_SORT_PRIORITY: Record = { - "buzz-agent": 0, - goose: 1, -}; - function runtimeInstallGuideLabel(runtime: AcpRuntimeCatalogEntry) { return runtime.availability === "adapter_missing" || runtime.availability === "adapter_outdated" @@ -81,13 +62,11 @@ function RuntimeLogo({ runtime }: { runtime: AcpRuntimeCatalogEntry }) { ); } - const avatarUrl = RUNTIME_LOGO_URLS[runtime.id] ?? runtime.avatarUrl; - return ( @@ -556,8 +535,10 @@ export function DoctorSettingsPanel() { () => [...(runtimesQuery.data ?? [])].sort( (left, right) => - (RUNTIME_SORT_PRIORITY[left.id] ?? Number.MAX_SAFE_INTEGER) - - (RUNTIME_SORT_PRIORITY[right.id] ?? Number.MAX_SAFE_INTEGER), + left.sortPriority - right.sortPriority || + (left.displayLabel || left.label || left.id).localeCompare( + right.displayLabel || right.label || right.id, + ), ), [runtimesQuery.data], ); diff --git a/desktop/src/shared/api/agentControl.ts b/desktop/src/shared/api/agentControl.ts index 677f0ffad4..2e0cce4a8e 100644 --- a/desktop/src/shared/api/agentControl.ts +++ b/desktop/src/shared/api/agentControl.ts @@ -29,3 +29,24 @@ export async function switchManagedAgentModel( modelId, }); } + +/** + * Resolve one live ACP permission request. The harness accepts only + * owner-signed, encrypted controls that match the exact channel, turn, and + * JSON-RPC request id, then verifies that optionId belongs to that request. + */ +export async function resolveManagedAgentPermission( + pubkey: string, + channelId: string, + turnId: string, + requestId: string | number, + optionId: string, +): Promise { + await sendAgentObserverControl(pubkey, { + type: "permission_decision", + channelId, + turnId, + requestId, + optionId, + }); +} diff --git a/desktop/src/shared/api/runtimeCapabilities.ts b/desktop/src/shared/api/runtimeCapabilities.ts new file mode 100644 index 0000000000..a7062bc4c3 --- /dev/null +++ b/desktop/src/shared/api/runtimeCapabilities.ts @@ -0,0 +1,46 @@ +export type ManagedAgentRuntimeCapabilities = { + runtimeIconUrl: string | null; + runtimeAvatarUrl: string | null; + runtimeSupersededAvatarUrls: string[]; + supportsBuzzModelConfig: boolean | null; +}; + +export type RuntimeCatalogCapabilities = { + displayLabel: string; + sortPriority: number; + onboardingVisible: boolean; + iconUrl: string; + iconScale: number; + supersededAvatarUrls: string[]; + supportsBuzzModelConfig: boolean; +}; + +export type RuntimeConfigCapabilities = { + supportsBuzzModelConfig: boolean | null; +}; + +export type ManagedAgentLog = { + content: string; + logPath: string; +}; + +export type CancelManagedAgentTurnResult = { + status: "sent" | "no_active_turn"; +}; + +/** + * Outcome of a live model-switch control frame, surfaced asynchronously via + * the agent's control-result observer frame. + */ +export type SwitchManagedAgentModelStatus = + | "sent" + | "turn_ending" + | "switched" + | "unsupported_model" + | "no_active_turn"; + +export type ControlResultFrame = { + type: "cancel_turn" | "switch_model" | "permission_decision"; + status: string; + modelId?: string; +}; diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index e8a8b885cf..5bfd1bb088 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -133,6 +133,10 @@ export type RawManagedAgent = { parallelism: number; system_prompt: string | null; avatar_url?: string | null; + runtime_icon_url?: string | null; + runtime_avatar_url?: string | null; + runtime_superseded_avatar_urls?: string[]; + supports_buzz_model_config?: boolean | null; model: string | null; model_source?: ManagedAgent["modelSource"]; provider: string | null; @@ -175,7 +179,21 @@ type RawManagedAgentLog = { export type RawAcpRuntimeCatalogEntry = { id: string; label: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + display_label?: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + sort_priority?: number; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + onboarding_visible?: boolean; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + icon_url?: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + icon_scale?: number; avatar_url: string; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + superseded_avatar_urls?: string[]; + /** Optional only for older E2E fixtures; the Rust catalog always supplies it. */ + supports_buzz_model_config?: boolean; availability: AcpAvailabilityStatus; command: string | null; binary_path: string | null; @@ -710,6 +728,10 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { parallelism: agent.parallelism, systemPrompt: agent.system_prompt, avatarUrl: agent.avatar_url ?? null, + runtimeIconUrl: agent.runtime_icon_url ?? null, + runtimeAvatarUrl: agent.runtime_avatar_url ?? null, + runtimeSupersededAvatarUrls: agent.runtime_superseded_avatar_urls ?? [], + supportsBuzzModelConfig: agent.supports_buzz_model_config ?? null, model: agent.model, modelSource: agent.model_source ?? null, // Fallbacks for pre-feature mocks/fixtures. Real records always carry them. @@ -745,7 +767,14 @@ export function fromRawAcpRuntimeCatalogEntry( return { id: entry.id, label: entry.label, + displayLabel: entry.display_label ?? entry.label, + sortPriority: entry.sort_priority ?? 100, + onboardingVisible: entry.onboarding_visible ?? false, + iconUrl: entry.icon_url ?? entry.avatar_url, + iconScale: entry.icon_scale ?? 1, avatarUrl: entry.avatar_url, + supersededAvatarUrls: entry.superseded_avatar_urls ?? [], + supportsBuzzModelConfig: entry.supports_buzz_model_config ?? true, availability: entry.availability, command: entry.command, binaryPath: entry.binary_path, diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 50842d0d0a..d000177bf3 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -1,3 +1,11 @@ +import type * as RuntimeCapabilities from "./runtimeCapabilities"; +export type { + CancelManagedAgentTurnResult, + ControlResultFrame, + ManagedAgentLog, + SwitchManagedAgentModelStatus, +} from "./runtimeCapabilities"; + export type ChannelType = "stream" | "forum" | "dm"; export type ChannelVisibility = "open" | "private"; export type ChannelRole = "owner" | "admin" | "member" | "guest" | "bot"; @@ -415,7 +423,7 @@ export type ManagedAgent = { * `"allowlist"`. Preserved across mode toggles. */ respondToAllowlist: string[]; -}; +} & RuntimeCapabilities.ManagedAgentRuntimeCapabilities; /** * Inbound author gate mode. Mirrors `buzz-acp`'s `--respond-to` CLI flag. @@ -487,34 +495,6 @@ export type CreateManagedAgentResponse = { spawnError: string | null; }; -export type ManagedAgentLog = { - content: string; - logPath: string; -}; - -export type CancelManagedAgentTurnResult = { - status: "sent" | "no_active_turn"; -}; - -/** - * Outcome of a live `switch_model` control frame, surfaced asynchronously via - * the agent's `control_result` observer frame. Busy path: `sent` (cancel + - * requeue on the new model) or `turn_ending` (oneshot already consumed this - * turn). Idle path: `switched`, `unsupported_model`, or `no_active_turn`. - */ -export type SwitchManagedAgentModelStatus = - | "sent" - | "turn_ending" - | "switched" - | "unsupported_model" - | "no_active_turn"; - -export type ControlResultFrame = { - type: "cancel_turn" | "switch_model"; - status: string; - modelId?: string; -}; - export type GitBashPrerequisite = { available: boolean; path: string | null; @@ -579,7 +559,7 @@ export type AcpRuntimeCatalogEntry = { * for `builtin` and `preset` entries. */ definitionEnv?: Record; -}; +} & RuntimeCapabilities.RuntimeCatalogCapabilities; /** An AcpRuntimeCatalogEntry that is confirmed available — command and binaryPath are non-null. */ export type AcpRuntime = AcpRuntimeCatalogEntry & { @@ -724,7 +704,7 @@ export type RuntimeConfigSurface = { advanced: ConfigField[]; extensions: ExtensionEntry[]; sources: ConfigSourceReport; -}; +} & RuntimeCapabilities.RuntimeConfigCapabilities; export type UpdateManagedAgentInput = { pubkey: string; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 493908c872..8cd9f97346 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -6984,6 +6984,11 @@ async function handleDiscoverAcpRuntimes( { id: "goose", label: "Goose", + display_label: "Goose", + sort_priority: 10, + onboarding_visible: false, + icon_url: "/runtime-icons/goose.svg", + icon_scale: 1.25, avatar_url: "", availability: "available", command: "goose", @@ -7003,6 +7008,11 @@ async function handleDiscoverAcpRuntimes( { id: "claude", label: "Claude Code", + display_label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, + icon_url: "/runtime-icons/claude.png", + icon_scale: 1.1, avatar_url: "", availability: "adapter_missing", command: null, @@ -7020,9 +7030,39 @@ async function handleDiscoverAcpRuntimes( source: "builtin", login_hint: undefined, }, + { + id: "devin", + label: "Devin", + display_label: "Devin", + sort_priority: 20, + onboarding_visible: true, + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + avatar_url: "", + availability: "not_installed", + command: null, + binary_path: null, + default_args: ["acp"], + mcp_command: null, + install_hint: + "Buzz requires the Devin CLI; the desktop app alone is not enough.", + install_instructions_url: "https://docs.devin.ai/cli", + can_auto_install: true, + requires_external_cli: true, + underlying_cli_path: null, + node_required: false, + auth_status: { status: "unknown" }, + login_hint: "Run `devin auth login` to authenticate.", + source: "builtin", + }, { id: "codex", label: "Codex", + display_label: "Codex", + sort_priority: 40, + onboarding_visible: true, + icon_url: "/runtime-icons/codex.png", + icon_scale: 1.1, avatar_url: "", availability: "not_installed", command: null, @@ -7043,6 +7083,11 @@ async function handleDiscoverAcpRuntimes( { id: "buzz-agent", label: "Buzz Agent", + display_label: "Buzz", + sort_priority: 0, + onboarding_visible: false, + icon_url: "/app-icon@2x.png", + icon_scale: 1.1, avatar_url: "", availability: "available", command: "buzz-agent", diff --git a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts index 1c3e86f133..7fee103d1a 100644 --- a/desktop/tests/e2e/onboarding-agent-defaults.spec.ts +++ b/desktop/tests/e2e/onboarding-agent-defaults.spec.ts @@ -3,7 +3,7 @@ import { installMockBridge } from "../helpers/bridge"; import { passThroughBackupStep } from "../helpers/onboarding"; function runtime( - id: "buzz-agent" | "claude" | "codex" | "goose", + id: "buzz-agent" | "claude" | "codex" | "devin" | "goose", availability: string, authStatus: Record, overrides: Record = {}, @@ -17,12 +17,20 @@ function runtime( ? "Claude Code" : id === "codex" ? "Codex" - : "Goose", + : id === "devin" + ? "Devin" + : "Goose", + display_label: id === "buzz-agent" ? "Buzz" : undefined, + source: "builtin", avatar_url: "", + sort_priority: 100, + onboarding_visible: true, + icon_url: "", + icon_scale: 1, availability, command: availability === "available" ? id : null, binary_path: availability === "available" ? `/usr/local/bin/${id}` : null, - default_args: [], + default_args: id === "devin" || id === "goose" ? ["acp"] : [], mcp_command: null, install_hint: `Install ${id}`, install_instructions_url: "https://example.com", @@ -57,15 +65,47 @@ async function readSavedRuntime(page: Parameters[0]) { }); } -test("setup shows all bundled harnesses as detected", async ({ page }) => { +test("setup projects catalog visibility and ordering, including Devin", async ({ + page, +}) => { await installMockBridge( page, { acpRuntimesCatalog: [ - runtime("buzz-agent", "available", { status: "not_applicable" }), - runtime("goose", "available", { status: "not_applicable" }), - runtime("codex", "available", { status: "logged_in" }), - runtime("claude", "available", { status: "logged_in" }), + runtime( + "buzz-agent", + "available", + { status: "not_applicable" }, + { onboarding_visible: true, sort_priority: 60 }, + ), + runtime( + "goose", + "available", + { status: "not_applicable" }, + { onboarding_visible: true, sort_priority: 50 }, + ), + runtime( + "codex", + "available", + { status: "logged_in" }, + { sort_priority: 40 }, + ), + runtime( + "devin", + "available", + { status: "logged_in" }, + { + sort_priority: 20, + icon_url: "/runtime-icons/devin.svg", + icon_scale: 1.1, + }, + ), + runtime( + "claude", + "available", + { status: "logged_in" }, + { sort_priority: 30 }, + ), ], }, { skipCommunitySeed: true, skipOnboardingSeed: true }, @@ -73,11 +113,83 @@ test("setup shows all bundled harnesses as detected", async ({ page }) => { await page.goto("/"); await navigateToSetupPage(page); + await expect(page.getByTestId("onboarding-runtime-devin")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-claude")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-codex")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-goose")).toBeVisible(); await expect(page.getByTestId("onboarding-runtime-buzz-agent")).toBeVisible(); await expect(page.getByRole("checkbox")).toHaveCount(0); + + const visibleRuntimeIds = await page + .locator("[data-testid^='onboarding-runtime-']") + .evaluateAll((elements) => + elements + .map((element) => element.getAttribute("data-testid")) + .filter( + (testId) => + testId != null && + !testId.includes("-ready-") && + !testId.includes("-checkmark-") && + !testId.includes("-instructions-") && + !testId.includes("-install-"), + ), + ); + expect(visibleRuntimeIds).toEqual([ + "onboarding-runtime-devin", + "onboarding-runtime-claude", + "onboarding-runtime-codex", + "onboarding-runtime-goose", + "onboarding-runtime-buzz-agent", + ]); + const devinIcon = page.getByTestId("onboarding-runtime-devin").locator("img"); + await expect(devinIcon).toHaveAttribute("src", "/runtime-icons/devin.svg"); + + const renderedIcon = await devinIcon.evaluate(async (element) => { + const image = element as HTMLImageElement; + await image.decode(); + + const canvas = document.createElement("canvas"); + canvas.width = image.naturalWidth; + canvas.height = image.naturalHeight; + const context = canvas.getContext("2d", { willReadFrequently: true }); + if (!context) throw new Error("2D canvas context unavailable"); + context.drawImage(image, 0, 0); + + const pixels = context.getImageData(0, 0, canvas.width, canvas.height).data; + let whitePixels = 0; + let darkPixels = 0; + let transparentPixels = 0; + for (let offset = 0; offset < pixels.length; offset += 4) { + const red = pixels[offset]; + const green = pixels[offset + 1]; + const blue = pixels[offset + 2]; + const alpha = pixels[offset + 3]; + if (alpha === 0) transparentPixels += 1; + if (alpha === 255 && red > 250 && green > 250 && blue > 250) { + whitePixels += 1; + } + if (alpha === 255 && red < 32 && green < 32 && blue < 32) { + darkPixels += 1; + } + } + + const pixelCount = canvas.width * canvas.height; + return { + naturalHeight: image.naturalHeight, + naturalWidth: image.naturalWidth, + transparentPixels, + whiteRatio: whitePixels / pixelCount, + darkRatio: darkPixels / pixelCount, + corner: Array.from(context.getImageData(0, 0, 1, 1).data), + }; + }); + + expect(renderedIcon.naturalWidth).toBe(425); + expect(renderedIcon.naturalHeight).toBe(425); + expect(renderedIcon.corner).toEqual([255, 255, 255, 255]); + expect(renderedIcon.transparentPixels).toBe(0); + expect(renderedIcon.whiteRatio).toBeGreaterThan(0.5); + expect(renderedIcon.darkRatio).toBeGreaterThan(0.05); }); test("setup distinguishes a missing CLI from an installed desktop app", async ({ diff --git a/desktop/tests/e2e/onboarding.spec.ts b/desktop/tests/e2e/onboarding.spec.ts index bfbfc6f063..dac0a68e60 100644 --- a/desktop/tests/e2e/onboarding.spec.ts +++ b/desktop/tests/e2e/onboarding.spec.ts @@ -1181,6 +1181,8 @@ test("first-community shows the scenario cards for localhost", async ({ { id: "claude", label: "Claude Code", + sort_priority: 30, + onboarding_visible: true, avatar_url: "", availability: "available", command: "claude",