From 38b7044b850113731d2b4b4b9533cc5d0da308f8 Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Thu, 30 Jul 2026 12:56:19 +1000 Subject: [PATCH 1/4] fix(acp): deliver plain replies for shared compute agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buzz-agent's contract is that output is tool calls: streamed assistant text is observability only and never published. Capable models honour that and call `send_message`. Small local models served over shared compute do not always — on multi-step turns they run their tools, then write the answer as prose and end the turn, so the turn reports success with nothing in the channel. Retain the turn's streamed text in AcpClient (bounded at 8 KiB) alongside a flag for whether a publish tool call was seen, and on EndTurn publish that text as a threaded channel reply when the agent published nothing itself. Gated on EndTurn only: MaxTokens / MaxTurnRequests mean the turn was truncated, so the text is not a deliberate answer. Bare acknowledgements ("OK", "Done") are filtered, and taking the buffer clears it so the same text can never post twice. Off by default via --deliver-plain-replies / BUZZ_ACP_DELIVER_PLAIN_REPLIES: for cloud models, which reliably publish their own replies, posting streamed text would double-post. The desktop shared-compute preset opts in, so this stays scoped to the case that needs it. Verified end to end against a live relay with stub ACP agents that pin each branch deterministically: a prose-only agent gets its answer delivered, and an agent that calls send_message while also streaming prose posts exactly once with the fallback silent. Forward-ported onto main from the unmerged origin/micspiral/mesh-0-74-gemma branch (bb4af4189). This exact bug was hit in production by the "Local LLM (llm1)" self-hosted mesh-llm/llama-server agent: turns ended with EndTurn and correct prose visible in the ACP activity log, but nothing was published to the channel because send_message was never called. Signed-off-by: Michael Neale with a helping hand from Claude Code Signed-off-by: Brett Meehan --- crates/buzz-acp/src/acp.rs | 129 ++++++++++++++++++ crates/buzz-acp/src/config.rs | 16 +++ crates/buzz-acp/src/lib.rs | 3 + crates/buzz-acp/src/pool.rs | 90 +++++++++++- .../src/managed_agents/relay_mesh.rs | 28 ++++ 5 files changed, 262 insertions(+), 4 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 700d5e8dcfd..3723e9463fe 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -211,6 +211,56 @@ pub struct AcpClient { /// deltas. Both goose and buzz-agent emit this notification; goose gates /// on client capability advertisement, buzz-agent emits unconditionally. goose_usage: UsageTracker, + /// Plain assistant text streamed during the current turn, and whether the + /// agent published a reply itself. Small local models routinely finish a + /// multi-step turn by writing the answer as prose instead of calling + /// `send_message`; ACP treats streamed text as observability only, so that + /// answer is dropped. The pool's mesh-gated delivery fallback consumes + /// these via [`take_undelivered_turn_text`](Self::take_undelivered_turn_text). + turn_text: String, + /// Whether a message-publish tool call was observed during this turn. + turn_published: bool, +} + +/// Maximum assistant text retained per turn for the delivery fallback. +/// +/// Bounded so a rogue agent streaming forever cannot grow the buffer without +/// limit; a chat reply that exceeds this is truncated rather than dropped. +const MAX_TURN_TEXT_BYTES: usize = 8 * 1024; + +/// Whether an ACP `tool_call` update represents the agent publishing a message. +/// +/// Matches both shapes an agent can use: the first-class `send_message` tool +/// (any `dev__`/server prefix) and a shell command that runs the CLI directly. +fn is_message_publish(title: &str, raw_input: &str) -> bool { + title.ends_with("send_message") + || raw_input.contains("messages send") + || raw_input.contains("social publish") +} + +/// Whether text is a bare acknowledgement not worth publishing as a reply. +/// +/// Guards the fallback against posting filler like "OK" or "Done." when the +/// agent had nothing substantive to say. +fn is_bare_acknowledgement(text: &str) -> bool { + let normalized: String = text + .chars() + .filter(|c| c.is_alphanumeric() || c.is_whitespace()) + .collect::() + .trim() + .to_ascii_lowercase(); + matches!( + normalized.as_str(), + "" | "ok" + | "okay" + | "done" + | "sure" + | "got it" + | "understood" + | "acknowledged" + | "will do" + | "thanks" + ) } /// Recursively merge `overlay` into `base`, with `overlay` winning on scalar/shape @@ -550,6 +600,8 @@ impl AcpClient { steering_supported: false, steer_rx: None, goose_usage: UsageTracker::default(), + turn_text: String::new(), + turn_published: false, }) } @@ -777,6 +829,11 @@ impl AcpClient { // misattributed to this turn. self.goose_usage.begin_turn(session_id); + // Reset per-turn delivery-fallback state alongside usage: text streamed + // by a previous turn must never be republished by this one. + self.turn_text.clear(); + self.turn_published = false; + self.last_prompt_id = Some(self.next_id); let id = self.next_id; self.next_id += 1; @@ -881,6 +938,23 @@ impl AcpClient { self.goose_usage.take() } + /// Take the turn's assistant text if the agent never published a reply. + /// + /// Returns `None` when the agent published via `send_message` (or the CLI) + /// itself, or when the only text was a bare acknowledgement. Consuming + /// clears the buffer so the same text can never be posted twice. + pub fn take_undelivered_turn_text(&mut self) -> Option { + let text = std::mem::take(&mut self.turn_text); + if self.turn_published { + return None; + } + let trimmed = text.trim(); + if trimmed.is_empty() || is_bare_acknowledgement(trimmed) { + return None; + } + Some(trimmed.to_string()) + } + /// Install a per-turn steer request channel for goose-native /// non-cancelling mid-turn delivery. /// @@ -1732,6 +1806,15 @@ impl AcpClient { "agent_message_chunk" => { if let Some(text) = update["content"]["text"].as_str() { tracing::info!(target: "acp::stream", "{text}"); + // Retain for the mesh-gated delivery fallback (bounded). + let remaining = MAX_TURN_TEXT_BYTES.saturating_sub(self.turn_text.len()); + if remaining > 0 { + let mut end = text.len().min(remaining); + while end > 0 && !text.is_char_boundary(end) { + end -= 1; + } + self.turn_text.push_str(&text[..end]); + } } false } @@ -1745,6 +1828,15 @@ impl AcpClient { .and_then(|v| v.as_str()) .unwrap_or("unknown"); tracing::info!(target: "acp::tool", "tool_call: {title} ({kind})"); + if !self.turn_published { + let raw_input = update + .get("rawInput") + .map(|v| v.to_string()) + .unwrap_or_default(); + if is_message_publish(title, &raw_input) { + self.turn_published = true; + } + } true } "tool_call_update" => { @@ -2256,6 +2348,43 @@ fn configure_no_window(cmd: &mut tokio::process::Command) { mod tests { use super::*; + #[test] + fn publish_detection_matches_tool_and_shell_shapes() { + // First-class tool, with the MCP server prefix the agent reports. + assert!(is_message_publish("dev__send_message", "{}")); + assert!(is_message_publish("send_message", "{}")); + // Shell shapes: the CLI invoked directly. + assert!(is_message_publish( + "dev__shell", + r#"{"command":"buzz messages send --channel c --content hi"}"# + )); + assert!(is_message_publish( + "dev__shell", + r#"{"command":"buzz social publish --content hi"}"# + )); + // Unrelated tools must not suppress the fallback. + assert!(!is_message_publish("dev__todo", "{}")); + assert!(!is_message_publish("dev__shell", r#"{"command":"ls -la"}"#)); + // `read_file` ends in neither name and must not match. + assert!(!is_message_publish("dev__read_file", r#"{"path":"a"}"#)); + } + + #[test] + fn bare_acknowledgements_are_not_worth_publishing() { + for text in ["OK", "ok.", "Done!", " Sure ", "Got it", "will do", ""] { + assert!( + is_bare_acknowledgement(text), + "expected {text:?} to be a bare ack" + ); + } + for text in ["12", "The capital is Paris.", "OK, the answer is 12"] { + assert!( + !is_bare_acknowledgement(text), + "expected {text:?} to be substantive" + ); + } + } + #[test] fn stop_reason_parses_all_known_values() { assert_eq!(StopReason::from_str("end_turn"), Some(StopReason::EndTurn)); diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 35aaec188db..5b4f5cdd181 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -409,6 +409,17 @@ pub struct CliArgs { #[arg(long, env = "BUZZ_ACP_NO_BASE_PROMPT")] pub no_base_prompt: bool, + /// Publish the agent's plain reply text when it ends a turn without calling + /// `send_message`. + /// + /// Off by default: capable models always publish their own replies, and + /// posting streamed text unconditionally would double-post. Small local + /// models served over shared compute reliably finish multi-step turns by + /// writing the answer as prose instead, so the desktop shared-compute + /// preset opts in. + #[arg(long, env = "BUZZ_ACP_DELIVER_PLAIN_REPLIES")] + pub deliver_plain_replies: bool, + /// Path to a custom base prompt file. Overrides the compiled-in default. /// Mutually exclusive with --no-base-prompt. #[arg( @@ -568,6 +579,9 @@ pub struct Config { /// `from_cli()`. `None` when using the compiled-in default or when /// `--no-base-prompt` is set. pub base_prompt_content: Option, + /// Publish the agent's plain reply text when a turn ends without a + /// `send_message` call. Opt-in; see the CLI flag for rationale. + pub deliver_plain_replies: bool, } /// Maximum length, in characters, of a session title sent to the adapter. @@ -1110,6 +1124,7 @@ impl Config { agent_owner: args.agent_owner.map(|s| s.trim().to_ascii_lowercase()), no_base_prompt: args.no_base_prompt, base_prompt_content, + deliver_plain_replies: args.deliver_plain_replies, }; Ok(config) @@ -1480,6 +1495,7 @@ mod tests { lazy_pool: false, agent_owner: None, no_base_prompt: false, + deliver_plain_replies: false, base_prompt_content: None, } } diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 811253e4ac0..033f04e6f3c 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1610,6 +1610,7 @@ async fn tokio_main() -> Result<()> { .as_deref() .and_then(|hex| nostr::PublicKey::from_hex(hex).ok()), memory_enabled: config.memory_enabled, + deliver_plain_replies: config.deliver_plain_replies, harness_name: crate::config::normalize_agent_command_identity(&config.agent_command), relay_url: config.relay_url.clone(), }); @@ -5136,6 +5137,7 @@ mod build_mcp_servers_tests { lazy_pool: false, agent_owner: None, no_base_prompt: false, + deliver_plain_replies: false, base_prompt_content: None, } } @@ -5358,6 +5360,7 @@ mod error_outcome_emission_tests { lazy_pool: false, agent_owner: None, no_base_prompt: false, + deliver_plain_replies: false, base_prompt_content: None, } } diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index ddc0330d9f2..be7f2a8d532 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -557,6 +557,11 @@ pub struct PromptContext { /// `[Agent Memory — core]` section. On by default; disabled via /// `--no-memory` / `BUZZ_ACP_NO_MEMORY`. pub memory_enabled: bool, + /// Whether to publish the agent's plain reply text when a turn ends without + /// a `send_message` call. Off by default; the desktop shared-compute preset + /// opts in because small local models routinely finish a multi-step turn by + /// writing the answer as prose instead of calling the tool. + pub deliver_plain_replies: bool, /// Harness identity string for NIP-AM `harness` field. Derived from the /// configured `agent_command` at startup (e.g. `"goose"`, `"buzz-agent"`). pub harness_name: String, @@ -1416,6 +1421,14 @@ pub async fn run_prompt_task( .as_ref() .map(|b| b.events.iter().map(|be| be.event.id.to_hex()).collect()) .unwrap_or_default(); + // Capture the reply target now: `batch` is moved into the requeue/outcome + // arms below, but the plain-reply fallback runs after the turn completes. + // Heartbeats have no batch and therefore no fallback target. + let plain_reply_target = if ctx.deliver_plain_replies { + batch.as_ref().and_then(plain_reply_target_for) + } else { + None + }; agent.acp.observe( "turn_started", serde_json::json!({ @@ -2171,6 +2184,31 @@ pub async fn run_prompt_task( agent.state.invalidate(&source); } + // Deliver the turn's plain text when the agent ended the turn + // without publishing a reply itself. Gated on EndTurn only: + // MaxTokens / MaxTurnRequests mean the turn was truncated, so the + // text is not a deliberate answer. `take_*` clears the buffer, so + // this can never post the same text twice. + if matches!(stop_reason, StopReason::EndTurn) { + if let Some(target) = plain_reply_target.as_ref() { + if let Some(text) = agent.acp.take_undelivered_turn_text() { + tracing::warn!( + target: "pool::prompt", + channel = %target.channel_id, + "agent ended turn without publishing; delivering its plain text as a channel reply" + ); + post_threaded_message( + &ctx.rest_client, + target.channel_id, + &target.thread_tags, + &text, + "plain-reply delivery", + ) + .await; + } + } + } + let core_stop = acp_stop_to_core(&stop_reason); let usage = agent.acp.take_turn_usage(); publish_agent_turn_metric( @@ -3865,6 +3903,34 @@ pub(crate) async fn reaction_add(rest: &crate::relay::RestClient, event_id: &str } } +/// Where a plain-reply fallback should post, captured before `batch` is moved. +#[derive(Debug, Clone)] +pub(crate) struct PlainReplyTarget { + channel_id: Uuid, + thread_tags: ThreadTags, +} + +/// Derive the plain-reply target from the batch's triggering event. +/// +/// Anchors to the trigger's own thread when it is itself a reply; otherwise the +/// trigger becomes the thread root, so the fallback reply lands in the same +/// place the agent's own `send_message` would have. +pub(crate) fn plain_reply_target_for(batch: &FlushBatch) -> Option { + let last = batch.events.last()?; + let parsed = crate::queue::parse_thread_tags(&last.event); + let trigger_hex = last.event.id.to_hex(); + let root = parsed.root_event_id.clone().unwrap_or(trigger_hex.clone()); + let parent = parsed.parent_event_id.clone().unwrap_or(trigger_hex); + Some(PlainReplyTarget { + channel_id: batch.channel_id, + thread_tags: ThreadTags { + root_event_id: Some(root), + parent_event_id: Some(parent), + mentioned_pubkeys: Vec::new(), + }, + }) +} + /// Best-effort: post a visible failure notice (kind:9) to a channel after a /// batch is dead-lettered. Replies into the thread of `thread_tags` when the /// triggering event was threaded. Errors are logged and swallowed — the @@ -3874,6 +3940,21 @@ pub(crate) async fn post_failure_notice( channel_id: Uuid, thread_tags: &ThreadTags, content: &str, +) { + post_threaded_message(rest, channel_id, thread_tags, content, "failure notice").await; +} + +/// Best-effort: post a threaded kind:9 message to a channel. +/// +/// Shared by the dead-letter failure notice and the plain-reply delivery +/// fallback; `what` only labels the log lines so the two are distinguishable. +/// Errors are logged and swallowed — publishing must never take down the loop. +async fn post_threaded_message( + rest: &crate::relay::RestClient, + channel_id: Uuid, + thread_tags: &ThreadTags, + content: &str, + what: &str, ) { let thread_ref = thread_tags.root_event_id.as_deref().and_then(|root| { let root_id = nostr::EventId::from_hex(root).ok()?; @@ -3891,21 +3972,21 @@ pub(crate) async fn post_failure_notice( match buzz_sdk::build_message(channel_id, content, thread_ref.as_ref(), &[], false, &[]) { Ok(b) => b, Err(e) => { - tracing::warn!(channel = %channel_id, "failure notice: build failed: {e}"); + tracing::warn!(channel = %channel_id, "{what}: build failed: {e}"); return; } }; let event = match builder.sign_with_keys(&rest.keys) { Ok(e) => e, Err(e) => { - tracing::warn!(channel = %channel_id, "failure notice: sign failed: {e}"); + tracing::warn!(channel = %channel_id, "{what}: sign failed: {e}"); return; } }; match tokio::time::timeout(Duration::from_secs(5), rest.submit_event(&event)).await { Ok(Ok(_)) => {} - Ok(Err(e)) => tracing::warn!(channel = %channel_id, "failure notice failed: {e}"), - Err(_) => tracing::warn!(channel = %channel_id, "failure notice timed out"), + Ok(Err(e)) => tracing::warn!(channel = %channel_id, "{what} failed: {e}"), + Err(_) => tracing::warn!(channel = %channel_id, "{what} timed out"), } } @@ -6540,6 +6621,7 @@ mod tests { agent_keys: agent_keys.clone(), agent_owner_pubkey: owner_pubkey, memory_enabled: false, + deliver_plain_replies: false, harness_name: "goose".to_string(), relay_url: "ws://127.0.0.1:3000".to_string(), } diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 5c246feedc5..961ab7f5101 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -42,6 +42,16 @@ pub fn apply_relay_mesh_env( RELAY_MESH_PREFER_MESH_FOR_AUTO_ENV.to_string(), "1".to_string(), ); + // Shared compute runs small local models, which reliably finish a + // multi-step turn by writing the answer as prose instead of calling + // `send_message` — the reply is then never published. Opt this preset into + // the harness fallback that delivers that text. Mesh-only: cloud models + // publish their own replies, where the fallback would double-post. + // Value is `true`, not `1`: these are clap bool flags, which reject `1`. + env.insert( + "BUZZ_ACP_DELIVER_PLAIN_REPLIES".to_string(), + "true".to_string(), + ); // Keep the requested response inside smaller local-model context windows. // These are defaults, not policy: the effective agent/persona/global env // may deliberately choose a smaller cap or a different effort. This function @@ -133,6 +143,24 @@ mod tests { .map(String::as_str), Some("1") ); + // Small local models end multi-step turns with prose instead of a + // `send_message` call, so shared compute opts into harness delivery. + assert_eq!( + env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES").map(String::as_str), + // `true`, not `1`: clap bool flags reject `1` and the harness + // exits at startup with "invalid value '1'". + Some("true") + ); + } + + #[test] + fn non_mesh_provider_does_not_opt_into_plain_reply_delivery() { + let mut env = BTreeMap::new(); + apply_relay_mesh_env(&mut env, Some("anthropic"), Some("claude-sonnet-4")); + + // Cloud models publish their own replies; delivering streamed text + // there would double-post. + assert_eq!(env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES"), None); } #[test] From 66dfd760f6ed9e9fbaf949ca80e7037e9f14dbb0 Mon Sep 17 00:00:00 2001 From: Brett Meehan Date: Wed, 5 Aug 2026 16:27:31 +0800 Subject: [PATCH 2/4] fix(desktop): default self-hosted openai-compat agents into plain-reply delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plain-reply-delivery fallback landed for the built-in relay-mesh ("Buzz shared compute") preset only, gated on provider == "relay-mesh" in apply_relay_mesh_env. A self-hosted model reached the same way small local models always are — the "OpenAI-compatible" provider pointed at a user's own llama.cpp/vLLM/mesh-llm endpoint instead of the built-in preset — never opted in, so the exact same silent-drop bug reproduced: the model answers in prose, ACP's activity log shows a correct reply, and nothing is ever published to the channel. Default BUZZ_ACP_DELIVER_PLAIN_REPLIES to "true" for effective provider "openai-compat" too, in the same effective-env assembly step, right after the relay-mesh translation. Scoped narrowly to "openai-compat": plain "openai" (the real OpenAI cloud API) is left untouched, since cloud models reliably call send_message themselves and enabling the fallback there would risk double-posting. An explicit user-set value (including an intentional "false") is preserved, matching the emptyness convention used elsewhere in this module. Reproduced against the "Local LLM (llm1)" agent in production: provider "openai-compat", OPENAI_COMPAT_BASE_URL pointed at a remote llama-server instance — confirmed via the agent's own config screenshots that it is not on the relay-mesh preset, so relay_mesh.rs's existing default never applied to it. with a helping hand from Claude Code Signed-off-by: Brett Meehan --- .../src-tauri/src/managed_agents/readiness.rs | 139 ++++++++++++++++-- 1 file changed, 124 insertions(+), 15 deletions(-) diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index c072448ff13..b09af319ba0 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -278,6 +278,29 @@ fn resolve_effective_agent_env_with_def( effective_model.as_deref(), ); + // The user's own self-hosted OpenAI-compatible endpoint (llama.cpp, vLLM, + // a directly-configured mesh-llm instance, etc. — distinct from the + // built-in `relay-mesh` preset above) shares relay-mesh's failure mode: + // small local models often finish a turn by writing the answer as prose + // instead of calling `send_message`, and unlike cloud providers there is + // no reliable way to make them self-correct. Opt this provider into the + // same harness delivery fallback, unless a lower layer already set an + // explicit value. An empty string counts as unset, matching the emptiness + // convention used throughout this module (e.g. `buzz_agent_requirements`). + // Scoped to `openai-compat` only: plain `openai` is the real OpenAI cloud + // API, which — like other cloud providers — reliably calls tools, so + // enabling the fallback there would risk double-posting. + if effective_provider.as_deref().map(str::trim) == Some("openai-compat") + && env + .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .is_none_or(|v| v.trim().is_empty()) + { + env.insert( + "BUZZ_ACP_DELIVER_PLAIN_REPLIES".to_string(), + "true".to_string(), + ); + } + EffectiveAgentEnv { env, config_file_path: runtime.and_then(|r| r.config_file_path), @@ -1463,20 +1486,12 @@ mod tests { // ── resolve_effective_agent_env ───────────────────────────────────────── - #[test] - fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { - // A record whose env_vars explicitly set provider/model must win over - // any baked defaults. In OSS test builds the baked map is empty, so - // this test validates the user-env layer is present in the output. - let mut env_vars = BTreeMap::new(); - env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); - env_vars.insert( - "BUZZ_AGENT_MODEL".to_string(), - "claude-opus-4-5".to_string(), - ); - - // Minimal record: only the fields resolve_effective_agent_env reads. - let record = crate::managed_agents::types::ManagedAgentRecord { + /// Minimal `ManagedAgentRecord` carrying only the fields + /// `resolve_effective_agent_env` reads, with caller-supplied `env_vars`. + fn minimal_record( + env_vars: BTreeMap, + ) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { pubkey: "test-pubkey".to_string(), name: "test-agent".to_string(), persona_id: None, @@ -1530,8 +1545,22 @@ mod tests { definition_respond_to_allowlist: Vec::new(), definition_parallelism: None, relay_mesh: None, - }; + } + } + + #[test] + fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { + // A record whose env_vars explicitly set provider/model must win over + // any baked defaults. In OSS test builds the baked map is empty, so + // this test validates the user-env layer is present in the output. + let mut env_vars = BTreeMap::new(); + env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); + env_vars.insert( + "BUZZ_AGENT_MODEL".to_string(), + "claude-opus-4-5".to_string(), + ); + let record = minimal_record(env_vars); let runtime = known_acp_runtime_exact("buzz-agent"); let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); @@ -1546,6 +1575,86 @@ mod tests { ); } + // ── openai-compat plain-reply delivery default ───────────────────────── + // + // Self-hosted OpenAI-compatible endpoints (llama.cpp, vLLM, a directly + // configured mesh-llm instance — the "Local LLM" case, distinct from the + // built-in relay-mesh preset) hit the same silent-drop bug relay-mesh's + // fallback exists for: a small local model answers in prose and never + // calls `send_message`. These tests lock in that this provider opts into + // the fallback by default, that the real OpenAI cloud API does not, and + // that an explicit user override always wins. + + #[test] + fn openai_compat_provider_opts_into_plain_reply_delivery() { + // The normalized provider dropdown persists to `record.provider` + // (what `resolve_effective_model_provider` actually reads), not to a + // raw `BUZZ_AGENT_PROVIDER` env var — that env var is itself derived + // from this typed field by an earlier env layer in real usage. + let mut env_vars = BTreeMap::new(); + env_vars.insert( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://llm1.example.ts.net:37073/v1".to_string(), + ); + let mut record = minimal_record(env_vars); + record.provider = Some("openai-compat".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective + .env + .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .map(String::as_str), + Some("true"), + "openai-compat should default to delivering plain-text replies" + ); + } + + #[test] + fn real_openai_provider_does_not_opt_into_plain_reply_delivery() { + // Plain "openai" (the real OpenAI cloud API) must NOT get the + // fallback: cloud models reliably call `send_message` themselves, and + // enabling it there risks double-posting. + let mut record = minimal_record(BTreeMap::new()); + record.provider = Some("openai".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective.env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES"), + None, + "real OpenAI cloud provider must not opt into plain-reply delivery" + ); + } + + #[test] + fn explicit_deliver_plain_replies_override_is_preserved_for_openai_compat() { + // A user (or a lower env layer) that explicitly disabled the fallback + // must not be silently overridden back to "true". + let mut env_vars = BTreeMap::new(); + env_vars.insert( + "BUZZ_ACP_DELIVER_PLAIN_REPLIES".to_string(), + "false".to_string(), + ); + let mut record = minimal_record(env_vars); + record.provider = Some("openai-compat".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective + .env + .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .map(String::as_str), + Some("false"), + "explicit user override must survive the openai-compat default" + ); + } + // ── provider-specific model fallback tests ──────────────────────────── #[test] From 6deceac2a8dd6571c529f26735a46db4c92f9a31 Mon Sep 17 00:00:00 2001 From: repudi8or Date: Fri, 7 Aug 2026 12:19:13 +0000 Subject: [PATCH 3/4] fix(buzz-agent): sanitize MCP tool schemas for openai-compat backends rmcp/schemars-derived tool schemas (buzz-dev-mcp's shell, read_file, str_replace, etc.) routinely carry JSON Schema metadata and assertions like $schema, title, format, and additionalProperties. Anthropic and the official OpenAI API tolerate these, but some openai-compat backends reached via a self-hosted endpoint or a multi-provider proxy (observed via a litellm-fronted OpenRouter route) hard-reject the whole tool-call request with "unsupported assertions or reserved metadata" instead of ignoring the extra keys. Add sanitize_tool_schema, applied once at tool registration in mcp.rs alongside the existing cap_schema size cap, to strip a denylist of pure-metadata and format/range-assertion keywords from every object in the schema tree. Every stripped keyword only narrows or documents what the schema accepts, never widens it, so removal can loosen validation but never break a previously-valid tool call. Structural/semantic keywords (type, properties, required, items, description, enum) are left untouched. Signed-off-by: repudi8or --- crates/buzz-agent/src/mcp.rs | 147 ++++++++++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 1 deletion(-) diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index 9ae125a0b76..21c73f774c6 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -285,7 +285,10 @@ impl McpRegistry { t.description.as_deref().unwrap_or("").to_owned(), MAX_DESCRIPTION_BYTES, ), - input_schema: cap_schema(&qname, Value::Object((*t.input_schema).clone())), + input_schema: cap_schema( + &qname, + sanitize_tool_schema(Value::Object((*t.input_schema).clone())), + ), }); reg.by_qname.insert(qname, Entry { server_idx, bare }); } @@ -860,6 +863,61 @@ fn timeout_msg(stage: &str, name: &str, t: Duration) -> String { format!("{stage} {name}: timeout after {}s", t.as_secs()) } +/// JSON Schema keywords that are pure metadata or format/range assertions, +/// not part of the safe common subset every OpenAI-compatible function-calling +/// backend accepts. `rmcp`/`schemars`-derived tool schemas routinely carry +/// `$schema`, `format` (e.g. `"format": "uint32"` on integer fields), and +/// similar — some gateways hard-reject a tool schema for carrying them at all +/// ("unsupported assertions or reserved metadata"), even though they're +/// harmless everywhere else. Stripping is always safe: every key here only +/// narrows or documents what the schema accepts, never widens it, so removal +/// can loosen validation but never break a previously-valid tool call. +const SCHEMA_DENYLIST: &[&str] = &[ + "$schema", + "$id", + "$comment", + "title", + "examples", + "default", + "readOnly", + "writeOnly", + "deprecated", + "format", + "pattern", + "minLength", + "maxLength", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minItems", + "maxItems", + "uniqueItems", + "minProperties", + "maxProperties", + "additionalProperties", + "contentEncoding", + "contentMediaType", +]; + +/// Recursively strip [`SCHEMA_DENYLIST`] keys from every object in the schema +/// tree, applied once at tool registration so every downstream request +/// builder (`anthropic_body`, `openai_body`, `responses_body`) inherits the +/// sanitized form without re-deriving it per turn. +fn sanitize_tool_schema(value: Value) -> Value { + match value { + Value::Object(map) => Value::Object( + map.into_iter() + .filter(|(k, _)| !SCHEMA_DENYLIST.contains(&k.as_str())) + .map(|(k, v)| (k, sanitize_tool_schema(v))) + .collect(), + ), + Value::Array(arr) => Value::Array(arr.into_iter().map(sanitize_tool_schema).collect()), + other => other, + } +} + fn cap_schema(qname: &str, schema: Value) -> Value { let size = serde_json::to_vec(&schema).map(|b| b.len()).unwrap_or(0); if size <= MAX_SCHEMA_BYTES { @@ -1201,4 +1259,91 @@ mod content_tests { // the resulting spawn wouldn't OOM (build+flag-set is the full contract here). // The real protection is the cfg-gated production path in spawn_one(). } + + #[test] + fn sanitize_tool_schema_strips_root_metadata() { + let schema = serde_json::json!({ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "ShellArgs", + "type": "object", + "properties": {}, + }); + let out = sanitize_tool_schema(schema); + assert_eq!( + out, + serde_json::json!({ "type": "object", "properties": {} }) + ); + } + + #[test] + fn sanitize_tool_schema_strips_nested_property_assertions() { + // schemars commonly emits "format" on integer fields (e.g. "uint32") + // and range/length assertions — these must be stripped wherever they + // appear in the tree, not just at the root. + let schema = serde_json::json!({ + "type": "object", + "properties": { + "timeout_ms": { "type": "integer", "format": "uint32", "minimum": 0 }, + "command": { "type": "string", "minLength": 1, "pattern": "^.+$" }, + }, + "additionalProperties": false, + }); + let out = sanitize_tool_schema(schema); + assert_eq!( + out, + serde_json::json!({ + "type": "object", + "properties": { + "timeout_ms": { "type": "integer" }, + "command": { "type": "string" }, + }, + }) + ); + } + + #[test] + fn sanitize_tool_schema_preserves_structural_and_semantic_keywords() { + // type/properties/required/items/description/enum are load-bearing — + // they must survive untouched. + let schema = serde_json::json!({ + "type": "object", + "description": "does a thing", + "properties": { + "mode": { "type": "string", "enum": ["fast", "slow"] }, + "tags": { "type": "array", "items": { "type": "string" } }, + }, + "required": ["mode"], + }); + let out = sanitize_tool_schema(schema.clone()); + assert_eq!(out, schema, "no safe keyword should be removed"); + } + + #[test] + fn sanitize_tool_schema_recurses_into_arrays() { + // anyOf/array-of-schemas members must each be sanitized too. + let schema = serde_json::json!({ + "properties": { + "x": { + "anyOf": [ + { "type": "string", "format": "uuid" }, + { "type": "null" } + ] + } + } + }); + let out = sanitize_tool_schema(schema); + assert_eq!( + out, + serde_json::json!({ + "properties": { + "x": { + "anyOf": [ + { "type": "string" }, + { "type": "null" } + ] + } + } + }) + ); + } } From 9832cf6422007c0b66ce1606d9dc9c7d3e81d90a Mon Sep 17 00:00:00 2001 From: repudi8or Date: Fri, 7 Aug 2026 12:58:53 +0000 Subject: [PATCH 4/4] fix(desktop): split readiness.rs test module under the file-size ratchet An earlier commit on this branch grew readiness.rs past its grandfathered 1742-line ratchet allowance (to 1850 lines), blocking the pre-push desktop-check hook. Following this repo's existing convention for oversized test modules (migration.rs's migration_tests.rs, migration_command_tests.rs, etc., and readiness.rs's own readiness_goose_file_config_tests.rs), extract the inline `mod tests` block into sibling files declared via #[path]. The extracted module still exceeded the 1000-line cap for new files, so it's split further into readiness_tests.rs and readiness_provider_tests.rs. readiness.rs itself drops to 690 lines. Also includes a cargo fmt fix to relay_mesh.rs (a pre-existing line-wrap picked up by running fmt across the whole crate), and reformats the two new test files to match. Signed-off-by: repudi8or --- .../src-tauri/src/managed_agents/readiness.rs | 1176 +---------------- .../readiness_provider_tests.rs | 473 +++++++ .../src/managed_agents/readiness_tests.rs | 710 ++++++++++ .../src/managed_agents/relay_mesh.rs | 3 +- 4 files changed, 1195 insertions(+), 1167 deletions(-) create mode 100644 desktop/src-tauri/src/managed_agents/readiness_provider_tests.rs create mode 100644 desktop/src-tauri/src/managed_agents/readiness_tests.rs diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index b09af319ba0..12acc28c77b 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -674,1174 +674,18 @@ fn goose_requirements( } // ── Tests ───────────────────────────────────────────────────────────────────── - +// +// The bulk of readiness's test coverage lives in a sibling file so this +// module stays under the desktop file-size ratchet. #[cfg(test)] -mod tests { - use std::collections::BTreeMap; - - use super::*; - use crate::managed_agents::discovery::known_acp_runtime_exact; - - /// 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); - EffectiveAgentEnv { - env, - config_file_path: runtime.and_then(|r| r.config_file_path), - effective_command: command.to_string(), - } - } - - fn env_with(pairs: &[(&str, &str)]) -> BTreeMap { - pairs - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect() - } - - // ── buzz-agent tests ────────────────────────────────────────────────── - - #[test] - fn buzz_agent_missing_provider_returns_not_ready_with_normalized_field() { - let env = make_env( - "buzz-agent", - env_with(&[("BUZZ_AGENT_MODEL", "claude-opus-4-5")]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "missing BUZZ_AGENT_PROVIDER should be NotReady" - ); - let reqs = result.requirements(); - assert!( - reqs.contains(&Requirement::NormalizedField { - field: "provider".to_string() - }), - "requirements should include NormalizedField(provider); got {reqs:?}" - ); - } - - #[test] - fn buzz_agent_missing_model_returns_not_ready_with_normalized_field() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("ANTHROPIC_API_KEY", "sk-test"), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result - .requirements() - .contains(&Requirement::NormalizedField { - field: "model".to_string() - })); - } - - #[test] - fn buzz_agent_missing_anthropic_key_returns_not_ready_with_env_key() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string() - })); - } - - #[test] - fn buzz_agent_missing_openai_key_returns_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openai"), - ("BUZZ_AGENT_MODEL", "gpt-4o"), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "OPENAI_COMPAT_API_KEY".to_string() - })); - } - - #[test] - fn buzz_agent_anthropic_with_all_fields_is_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), - ("ANTHROPIC_API_KEY", "sk-test"), - ]), - ); - assert!(agent_readiness(&env).is_ready()); - } - - #[test] - fn buzz_agent_databricks_with_host_and_model_is_ready_without_token() { - // DATABRICKS_TOKEN is NOT required — OAuth PKCE is the normal path. - // No token present, no OAuth cache present → still Ready because we - // cannot evaluate OAuth state from the env map alone. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks"), - ("BUZZ_AGENT_MODEL", "dbrx-instruct"), - ("DATABRICKS_HOST", "https://dbc.example.com"), - // NOTE: no DATABRICKS_TOKEN - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "Databricks with HOST+model but no TOKEN should still be Ready (OAuth path)" - ); - } - - #[test] - fn buzz_agent_databricks_missing_host_returns_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks"), - ("BUZZ_AGENT_MODEL", "dbrx-instruct"), - // NOTE: no DATABRICKS_HOST - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "DATABRICKS_HOST".to_string() - })); - } - - #[test] - fn buzz_agent_databricks_v2_missing_host_returns_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks_v2"), - ( - "BUZZ_AGENT_MODEL", - "databricks/meta-llama-4-maverick-17b-instruct", - ), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "DATABRICKS_HOST".to_string() - })); - } - - // ── goose tests ─────────────────────────────────────────────────────── - - #[test] - fn goose_missing_provider_returns_not_ready() { - // Call goose_requirements directly with None file config so the test is - // deterministic — the `agent_readiness` path reads the real - // ~/.config/goose/config.yaml which may silence requirements on - // developer machines. - let env = make_env("goose", env_with(&[("GOOSE_MODEL", "claude-opus-4-5")])); - let reqs = goose_requirements(&env, None); - assert!( - !reqs.is_empty(), - "missing GOOSE_PROVIDER with no file config must produce requirements" - ); - assert!( - reqs.contains(&Requirement::NormalizedField { - field: "provider".to_string() - }), - "requirements must include NormalizedField(provider); got {reqs:?}" - ); - } - - #[test] - fn goose_with_provider_and_model_and_key_is_ready() { - let env = make_env( - "goose", - env_with(&[ - ("GOOSE_PROVIDER", "anthropic"), - ("GOOSE_MODEL", "claude-opus-4-5"), - ("ANTHROPIC_API_KEY", "sk-test"), - ]), - ); - assert!(agent_readiness(&env).is_ready()); - } - - // ── empty-string semantics ──────────────────────────────────────────── - // - // A key present with an empty value ("") must be treated as MISSING, to - // match the dialog's (envVars[key] ?? "").length === 0 emptiness check. - - #[test] - fn buzz_agent_empty_string_provider_is_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", ""), - ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "empty-string BUZZ_AGENT_PROVIDER must be treated as missing" - ); - assert!(result - .requirements() - .contains(&Requirement::NormalizedField { - field: "provider".to_string() - })); - } - - #[test] - fn buzz_agent_empty_string_model_is_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("BUZZ_AGENT_MODEL", ""), - ("ANTHROPIC_API_KEY", "sk-test"), - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "empty-string BUZZ_AGENT_MODEL must be treated as missing" - ); - assert!(result - .requirements() - .contains(&Requirement::NormalizedField { - field: "model".to_string() - })); - } - - #[test] - fn buzz_agent_empty_string_anthropic_key_is_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), - ("ANTHROPIC_API_KEY", ""), - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "empty-string ANTHROPIC_API_KEY must be treated as missing" - ); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string() - })); - } - - #[test] - fn buzz_agent_empty_string_databricks_host_is_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks"), - ("BUZZ_AGENT_MODEL", "dbrx-instruct"), - ("DATABRICKS_HOST", ""), - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "empty-string DATABRICKS_HOST must be treated as missing" - ); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "DATABRICKS_HOST".to_string() - })); - } - - #[test] - fn goose_empty_string_provider_is_not_ready() { - // Call goose_requirements directly with None file config so the test is - // deterministic — the `agent_readiness` path reads the real - // ~/.config/goose/config.yaml which may silence requirements on - // developer machines. - let env = make_env( - "goose", - env_with(&[("GOOSE_PROVIDER", ""), ("GOOSE_MODEL", "claude-opus-4-5")]), - ); - let reqs = goose_requirements(&env, None); - assert!( - !reqs.is_empty(), - "empty-string GOOSE_PROVIDER must be treated as missing" - ); - assert!( - reqs.contains(&Requirement::NormalizedField { - field: "provider".to_string() - }), - "requirements must include NormalizedField(provider); got {reqs:?}" - ); - } - - #[test] - fn goose_empty_string_anthropic_key_is_not_ready() { - // Call goose_requirements directly with None file config so the test is - // deterministic — the `agent_readiness` path reads the real - // ~/.config/goose/config.yaml which may silence requirements on - // developer machines. - let env = make_env( - "goose", - env_with(&[ - ("GOOSE_PROVIDER", "anthropic"), - ("GOOSE_MODEL", "claude-opus-4-5"), - ("ANTHROPIC_API_KEY", ""), - ]), - ); - let reqs = goose_requirements(&env, None); - assert!( - !reqs.is_empty(), - "empty-string ANTHROPIC_API_KEY must be treated as missing (goose)" - ); - assert!( - reqs.contains(&Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string() - }), - "requirements must include ANTHROPIC_API_KEY; got {reqs:?}" - ); - } - - // ── codex tests ─────────────────────────────────────────────────────── - - #[test] - fn codex_not_ready_copy_does_not_mention_openai_api_key() { - // codex uses its own credential store via `codex login` (OAuth or API key). - // The nudge copy must NOT say "set OPENAI_API_KEY". - // Use a not-installed runtime so the requirement is always emitted - // regardless of whether codex is on the test machine's PATH. - let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], None); - let reqs = cli_login::requirements(&["codex", "login", "status"], "run `codex login`", &rt); - // Whether codex is installed or not, the copy (if any) must not mention OPENAI_API_KEY. - for req in &reqs { - if let Requirement::CliLogin { setup_copy, .. } = req { - assert!( - !setup_copy.contains("OPENAI_API_KEY"), - "codex nudge copy must not mention OPENAI_API_KEY; got: {setup_copy:?}" - ); - assert!( - setup_copy.contains("codex login"), - "codex nudge copy should mention `codex login`; got: {setup_copy:?}" - ); - } - } - } - - // ── cli_login_requirements: resolve_command integration ───────────── - - /// Construct a minimal `KnownAcpRuntime` stub for testing cli_login_requirements. - /// `commands` are the adapter binaries; `underlying_cli` is the CLI name. - fn make_cli_runtime( - commands: &'static [&'static str], - underlying_cli: Option<&'static str>, - ) -> KnownAcpRuntime { - KnownAcpRuntime { - id: "test-cli-runtime", - 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, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, - } - } - - /// Returns the absolute path of the currently-running test binary as a `&'static str`. - /// Host-portable stand-in for a "present" binary: absolute path so `find_command` resolves - /// it via `path.exists()`. Leaked allocation is intentional — process exits after tests. - fn present_binary_str() -> &'static str { - let path = std::env::current_exe().expect("current_exe must be available in tests"); - Box::leak(path.to_string_lossy().into_owned().into_boxed_str()) - } - - /// Leak a runtime slice of `'static` strs for use in `make_cli_runtime`. - fn static_commands(commands: Vec<&'static str>) -> &'static [&'static str] { - Box::leak(commands.into_boxed_slice()) - } - - #[test] - fn cli_login_requirements_missing_binary_is_not_ready() { - // Both adapter and underlying CLI are nonexistent → NotInstalled state - // → must return a CliLogin requirement with availability=NotInstalled. - let rt = make_cli_runtime( - &["__buzz_nonexistent_adapter_abc123__"], - Some("__buzz_nonexistent_cli_abc123__"), - ); - let reqs = cli_login::requirements( - &["__buzz_nonexistent_binary_abc123__", "status"], - "install the tool first", - &rt, - ); - assert!( - !reqs.is_empty(), - "missing binary must produce a CliLogin requirement (NotReady)" - ); - assert!( - matches!(reqs[0], Requirement::CliLogin { .. }), - "requirement must be CliLogin; got {:?}", - reqs[0] - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::NotInstalled, - "both missing → NotInstalled" - ); - } - } - - #[test] - fn cli_login_requirements_adapter_missing_emits_adapter_missing() { - // Underlying CLI present (use the running test binary as a portable - // stand-in — it's always present and resolves via absolute path), - // adapter absent. - // → AdapterMissing state → no probe run → CliLogin{AdapterMissing}. - let exe = present_binary_str(); - let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], Some(exe)); - let reqs = cli_login::requirements(&[exe, "--list"], "install the adapter", &rt); - assert!( - !reqs.is_empty(), - "adapter missing must produce a CliLogin requirement" - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::AdapterMissing, - "adapter absent, CLI present → AdapterMissing" - ); - } - } - - #[test] - fn cli_login_requirements_cli_missing_emits_cli_missing() { - // Adapter present (use the running test binary as a portable stand-in), - // underlying CLI absent. - // → CliMissing state → no probe run → CliLogin{CliMissing}. - let exe = present_binary_str(); - let rt = make_cli_runtime( - static_commands(vec![exe]), // adapter found via absolute path - Some("__buzz_nonexistent_cli_abc123__"), // underlying CLI missing - ); - let reqs = cli_login::requirements(&[exe, "--list"], "install the CLI", &rt); - assert!( - !reqs.is_empty(), - "CLI missing must produce a CliLogin requirement" - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::CliMissing, - "adapter present, CLI absent → CliMissing" - ); - } - } - - #[test] - fn cli_login_requirements_resolvable_binary_runs_probe_at_resolved_path() { - // Both adapter and CLI present (use the running test binary as a - // portable stand-in — always present, resolves via absolute path), - // probe exits 0 (run with `--list` which lists tests and exits 0). - // → logged_in = true → requirements is empty (Ready). - let exe = present_binary_str(); - let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); - let reqs = cli_login::requirements( - &[exe, "--list"], - "this should not show (probe exits 0)", - &rt, - ); - assert!( - reqs.is_empty(), - "expected Ready (no requirements) when probe binary resolves and exits 0; \ - got {:?}", - reqs - ); - } - - #[test] - fn cli_login_requirements_logged_out_emits_available() { - // Both adapter and CLI present, but probe exits non-zero (logged out). - // Use the test binary with an unrecognized argument as the probe — - // libtest exits non-zero for unknown flags on all platforms. - // → CliLogin{Available} (tooling installed, needs login). - let exe = present_binary_str(); - let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); - let reqs = - cli_login::requirements(&[exe, "--buzz-probe-fail-xyz"], "run `tool login`", &rt); - assert!( - !reqs.is_empty(), - "non-zero probe must produce a CliLogin requirement (logged out)" - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::Available, - "tooling installed, probe fails → Available (logged-out)" - ); - } - } - - // ── codex readiness version gate ─────────────────────────────────────── - - /// Build a minimal `KnownAcpRuntime` for testing the codex version gate. - /// `adapter_commands` are the exact strings passed to `find_command` — use - /// `&["codex-acp"]` when the binary is on PATH, or `&[]` - /// when resolving via absolute path. `underlying_cli` is a portable - /// stand-in so the adapter is not misclassified as `CliMissing`. - fn make_codex_runtime( - adapter_commands: &'static [&'static str], - underlying_cli: Option<&'static str>, - ) -> KnownAcpRuntime { - KnownAcpRuntime { - id: "codex", - 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, - max_rounds_env_var: None, - required_normalized_fields: &[], - login_hint: None, - auth_probe_args: None, - } - } - - /// Build a temp dir containing a `codex-acp` script with the given body, - /// prepend it to PATH, and clear the resolve cache. Returns the temp dir - /// and the original PATH string for restoration. - #[cfg(unix)] - fn setup_temp_codex_acp(script_body: &str) -> (tempfile::TempDir, String) { - use std::os::unix::fs::PermissionsExt; - - let dir = tempfile::tempdir().expect("create temp dir"); - let bin = dir.path().join("codex-acp"); - std::fs::write(&bin, script_body).expect("write script"); - std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) - .expect("chmod script"); - - let original_path = std::env::var("PATH").unwrap_or_default(); - let new_path = format!("{}:{}", dir.path().display(), original_path); - std::env::set_var("PATH", &new_path); - crate::managed_agents::clear_resolve_cache(); - - (dir, original_path) - } +#[path = "readiness_tests.rs"] +mod tests; - #[cfg(unix)] - fn leaked_adapter_commands(bin: &std::path::Path) -> &'static [&'static str] { - let command = Box::leak(bin.display().to_string().into_boxed_str()); - Box::leak(vec![command as &'static str].into_boxed_slice()) - } - - /// Restore PATH and clear the resolve cache after a PATH-mutating test. - #[cfg(unix)] - fn restore_path(original: &str) { - std::env::set_var("PATH", original); - crate::managed_agents::clear_resolve_cache(); - } - - /// Codex readiness: outdated adapter (exits non-zero) → AdapterOutdated, - /// login probe skipped. - #[cfg(unix)] - #[test] - fn cli_login_requirements_codex_outdated_adapter_emits_adapter_outdated() { - let _guard = crate::managed_agents::lock_path_mutex(); - - let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\nexit 1\n"); - let exe = present_binary_str(); - // Use the fixture's absolute adapter path here. Bare `codex-acp` - // intentionally prefers Buzz's managed npm shim when it exists, which - // would make this version-gate regression test depend on machine state. - let rt = make_codex_runtime( - leaked_adapter_commands(&dir.path().join("codex-acp")), - Some(exe), - ); - let reqs = cli_login::requirements( - &[exe, "--buzz-probe-must-not-run-xyz"], - "run `codex login`", - &rt, - ); - - restore_path(&orig); - drop(dir); - - assert!( - !reqs.is_empty(), - "outdated codex adapter must produce a requirement; got {reqs:?}" - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::AdapterOutdated, - "0.x codex adapter must yield AdapterOutdated; got {availability:?}" - ); - } else { - panic!("expected CliLogin requirement; got {:?}", reqs[0]); - } - } - - /// Codex readiness: adapter exits 0 but output is not a parseable version - /// → AdapterOutdated (garbage output treated as outdated, same as non-zero). - #[cfg(unix)] - #[test] - fn cli_login_requirements_codex_garbage_version_output_emits_adapter_outdated() { - let _guard = crate::managed_agents::lock_path_mutex(); - - let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\necho 'not a version string'\nexit 0\n"); - let exe = present_binary_str(); - let rt = make_codex_runtime( - leaked_adapter_commands(&dir.path().join("codex-acp")), - Some(exe), - ); - let reqs = cli_login::requirements( - &[exe, "--buzz-probe-must-not-run-xyz"], - "run `codex login`", - &rt, - ); - - restore_path(&orig); - drop(dir); - - assert!( - !reqs.is_empty(), - "garbage version output must produce a requirement; got {reqs:?}" - ); - if let Requirement::CliLogin { - ref availability, .. - } = reqs[0] - { - assert_eq!( - *availability, - crate::managed_agents::AcpAvailabilityStatus::AdapterOutdated, - "unparseable version output must yield AdapterOutdated; got {availability:?}" - ); - } else { - panic!("expected CliLogin requirement; got {:?}", reqs[0]); - } - } - - // ── custom/unknown command ───────────────────────────────────────────── - - #[test] - fn unknown_command_is_always_ready() { - // Since Phase B-7 (readiness exec-check), unknown/custom commands that are - // not resolvable in PATH produce a MissingBinary requirement rather than - // being unconditionally Ready. A command that IS resolvable should be Ready. - // Use a known-present binary so the test is not environment-sensitive. - let env = make_env("sh", BTreeMap::new()); - assert!( - agent_readiness(&env).is_ready(), - "unknown/custom command present in PATH should be Ready" - ); - } - - #[test] - fn unknown_command_missing_from_path_is_not_ready() { - let env = make_env("my-custom-harness-that-does-not-exist", BTreeMap::new()); - let readiness = agent_readiness(&env); - assert!( - !readiness.is_ready(), - "unknown/custom command absent from PATH should be NotReady" - ); - let reqs = readiness.requirements(); - assert_eq!(reqs.len(), 1); - assert!( - matches!(&reqs[0], Requirement::MissingBinary { command } if command == "my-custom-harness-that-does-not-exist"), - "should surface MissingBinary requirement" - ); - } - - // ── AgentReadiness helpers ───────────────────────────────────────────── - - #[test] - fn agent_readiness_ready_has_empty_requirements() { - assert!(AgentReadiness::Ready.requirements().is_empty()); - } - - #[test] - fn agent_readiness_not_ready_exposes_requirements() { - let r = AgentReadiness::NotReady { - requirements: vec![Requirement::EnvKey { - key: "FOO".to_string(), - }], - }; - assert!(!r.is_ready()); - assert_eq!(r.requirements().len(), 1); - } - - // ── Requirement serialization ───────────────────────────────────────── - - #[test] - fn requirement_serializes_with_surface_tag() { - let r = Requirement::NormalizedField { - field: "provider".to_string(), - }; - let json = serde_json::to_value(&r).unwrap(); - assert_eq!(json["surface"], "normalized_field"); - assert_eq!(json["field"], "provider"); - } - - #[test] - fn git_bash_requirement_serializes_correctly() { - let json = serde_json::to_value(Requirement::GitBash).unwrap(); - assert_eq!(json, serde_json::json!({ "surface": "git_bash" })); - } - - #[test] - fn env_key_requirement_serializes_correctly() { - let r = Requirement::EnvKey { - key: "ANTHROPIC_API_KEY".to_string(), - }; - let json = serde_json::to_value(&r).unwrap(); - assert_eq!(json["surface"], "env_key"); - assert_eq!(json["key"], "ANTHROPIC_API_KEY"); - } - - #[test] - fn cli_login_requirement_serializes_correctly() { - let r = Requirement::CliLogin { - probe_args: vec![ - "codex".to_string(), - "login".to_string(), - "status".to_string(), - ], - setup_copy: "run `codex login`".to_string(), - availability: crate::managed_agents::AcpAvailabilityStatus::Available, - }; - let json = serde_json::to_value(&r).unwrap(); - assert_eq!(json["surface"], "cli_login"); - assert!(json["probe_args"].is_array()); - assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); - } - - // ── resolve_effective_agent_env ───────────────────────────────────────── - - /// Minimal `ManagedAgentRecord` carrying only the fields - /// `resolve_effective_agent_env` reads, with caller-supplied `env_vars`. - fn minimal_record( - env_vars: BTreeMap, - ) -> crate::managed_agents::types::ManagedAgentRecord { - crate::managed_agents::types::ManagedAgentRecord { - pubkey: "test-pubkey".to_string(), - name: "test-agent".to_string(), - persona_id: None, - private_key_nsec: String::new(), - auth_tag: None, - relay_url: String::new(), - avatar_url: None, - acp_command: "buzz-acp".to_string(), - agent_command: "buzz-agent".to_string(), - agent_command_override: None, - agent_args: vec![], - mcp_command: String::new(), - turn_timeout_seconds: 320, - idle_timeout_seconds: None, - max_turn_duration_seconds: None, - parallelism: 1, - system_prompt: None, - model: None, - provider: None, - persona_source_version: None, - env_vars, - start_on_app_launch: false, - auto_restart_on_config_change: true, - runtime_pid: None, - backend: Default::default(), - backend_agent_id: None, - provider_binary_path: None, - team_id: None, - persona_team_dir: None, - persona_name_in_team: None, - created_at: String::new(), - updated_at: String::new(), - last_started_at: None, - last_stopped_at: None, - last_exit_code: None, - last_error: None, - last_error_code: None, - respond_to: Default::default(), - respond_to_allowlist: vec![], - display_name: None, - slug: None, - runtime: None, - name_pool: Vec::new(), - is_builtin: false, - is_active: true, - shared: false, - source_team: None, - source_team_persona_slug: None, - catalog_source: None, - definition_respond_to: None, - definition_respond_to_allowlist: Vec::new(), - definition_parallelism: None, - relay_mesh: None, - } - } - - #[test] - fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { - // A record whose env_vars explicitly set provider/model must win over - // any baked defaults. In OSS test builds the baked map is empty, so - // this test validates the user-env layer is present in the output. - let mut env_vars = BTreeMap::new(); - env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); - env_vars.insert( - "BUZZ_AGENT_MODEL".to_string(), - "claude-opus-4-5".to_string(), - ); - - let record = minimal_record(env_vars); - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - // User env_vars must be present in the output (last-write-wins). - assert_eq!( - effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), - Some("anthropic") - ); - assert_eq!( - effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), - Some("claude-opus-4-5") - ); - } - - // ── openai-compat plain-reply delivery default ───────────────────────── - // - // Self-hosted OpenAI-compatible endpoints (llama.cpp, vLLM, a directly - // configured mesh-llm instance — the "Local LLM" case, distinct from the - // built-in relay-mesh preset) hit the same silent-drop bug relay-mesh's - // fallback exists for: a small local model answers in prose and never - // calls `send_message`. These tests lock in that this provider opts into - // the fallback by default, that the real OpenAI cloud API does not, and - // that an explicit user override always wins. - - #[test] - fn openai_compat_provider_opts_into_plain_reply_delivery() { - // The normalized provider dropdown persists to `record.provider` - // (what `resolve_effective_model_provider` actually reads), not to a - // raw `BUZZ_AGENT_PROVIDER` env var — that env var is itself derived - // from this typed field by an earlier env layer in real usage. - let mut env_vars = BTreeMap::new(); - env_vars.insert( - "OPENAI_COMPAT_BASE_URL".to_string(), - "http://llm1.example.ts.net:37073/v1".to_string(), - ); - let mut record = minimal_record(env_vars); - record.provider = Some("openai-compat".to_string()); - - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - assert_eq!( - effective - .env - .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") - .map(String::as_str), - Some("true"), - "openai-compat should default to delivering plain-text replies" - ); - } - - #[test] - fn real_openai_provider_does_not_opt_into_plain_reply_delivery() { - // Plain "openai" (the real OpenAI cloud API) must NOT get the - // fallback: cloud models reliably call `send_message` themselves, and - // enabling it there risks double-posting. - let mut record = minimal_record(BTreeMap::new()); - record.provider = Some("openai".to_string()); - - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - assert_eq!( - effective.env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES"), - None, - "real OpenAI cloud provider must not opt into plain-reply delivery" - ); - } - - #[test] - fn explicit_deliver_plain_replies_override_is_preserved_for_openai_compat() { - // A user (or a lower env layer) that explicitly disabled the fallback - // must not be silently overridden back to "true". - let mut env_vars = BTreeMap::new(); - env_vars.insert( - "BUZZ_ACP_DELIVER_PLAIN_REPLIES".to_string(), - "false".to_string(), - ); - let mut record = minimal_record(env_vars); - record.provider = Some("openai-compat".to_string()); - - let runtime = known_acp_runtime_exact("buzz-agent"); - let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - - assert_eq!( - effective - .env - .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") - .map(String::as_str), - Some("false"), - "explicit user override must survive the openai-compat default" - ); - } - - // ── provider-specific model fallback tests ──────────────────────────── - - #[test] - fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { - // The baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL. - // An agent with only DATABRICKS_MODEL must pass the readiness gate. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks_v2"), - ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), - ("DATABRICKS_HOST", "https://dbc.example.com"), - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "DATABRICKS_MODEL must satisfy the model requirement for databricks_v2" - ); - } - - #[test] - fn buzz_agent_databricks_v2_hyphen_alias_with_databricks_model_is_ready() { - // buzz-agent accepts both "databricks_v2" and "databricks-v2". The - // readiness gate must recognize the hyphen alias and accept DATABRICKS_MODEL. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks-v2"), - ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), - ("DATABRICKS_HOST", "https://dbc.example.com"), - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "databricks-v2 alias with DATABRICKS_MODEL must be Ready" - ); - } - - #[test] - fn buzz_agent_databricks_hyphen_alias_missing_host_returns_not_ready() { - // The hyphen alias "databricks-v2" requires DATABRICKS_HOST just like - // the underscore variants. Without it the agent cannot reach the endpoint. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks-v2"), - ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), - // DATABRICKS_HOST intentionally absent - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "databricks-v2 without DATABRICKS_HOST must be NotReady" - ); - let reqs = result.requirements(); - assert!( - reqs.iter() - .any(|r| matches!(r, Requirement::EnvKey { key } if key == "DATABRICKS_HOST")), - "missing requirements must include DATABRICKS_HOST; got {reqs:?}" - ); - } - - #[test] - fn buzz_agent_databricks_v1_with_databricks_model_but_no_buzz_agent_model_is_ready() { - // V1 (Model Serving) also resolves DATABRICKS_MODEL — same fallback applies. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks"), - ("DATABRICKS_MODEL", "dbrx-instruct"), - ("DATABRICKS_HOST", "https://dbc.example.com"), - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "DATABRICKS_MODEL must satisfy the model requirement for databricks (V1)" - ); - } - - #[test] - fn buzz_agent_anthropic_with_anthropic_model_but_no_buzz_agent_model_is_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "anthropic"), - ("ANTHROPIC_MODEL", "claude-opus-4-5"), - ("ANTHROPIC_API_KEY", "sk-test"), - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "ANTHROPIC_MODEL must satisfy the model requirement for anthropic" - ); - } - - #[test] - fn buzz_agent_openai_with_openai_compat_model_but_no_buzz_agent_model_is_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openai"), - ("OPENAI_COMPAT_MODEL", "gpt-4o"), - ("OPENAI_COMPAT_API_KEY", "sk-test"), - ]), - ); - assert!( - agent_readiness(&env).is_ready(), - "OPENAI_COMPAT_MODEL must satisfy the model requirement for openai" - ); - } - - #[test] - fn buzz_agent_empty_provider_model_fallback_key_is_not_ready() { - // An empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must still be NotReady. - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "databricks_v2"), - ("DATABRICKS_MODEL", ""), - ("DATABRICKS_HOST", "https://dbc.example.com"), - ]), - ); - let result = agent_readiness(&env); - assert!( - !result.is_ready(), - "empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must be NotReady" - ); - assert!(result - .requirements() - .contains(&Requirement::NormalizedField { - field: "model".to_string() - })); - } - - // ── OpenRouter readiness ───────────────────────────────────────────── - - #[test] - fn buzz_agent_openrouter_with_all_fields_is_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openrouter"), - ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), - ("OPENROUTER_API_KEY", "sk-or-test-key"), - ]), - ); - let result = agent_readiness(&env); - assert!( - result.is_ready(), - "openrouter with all fields should be ready" - ); - } - - #[test] - fn buzz_agent_openrouter_missing_key_returns_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openrouter"), - ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "OPENROUTER_API_KEY".to_string() - })); - } - - #[test] - fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openrouter"), - ("OPENROUTER_MODEL", "google/gemini-2.5-flash"), - ("OPENROUTER_API_KEY", "sk-or-test-key"), - ]), - ); - let result = agent_readiness(&env); - assert!( - result.is_ready(), - "OPENROUTER_MODEL fallback should satisfy model requirement" - ); - } -} +// Provider/model-fallback and CLI-adjacent requirement tests continue in a +// second sibling file — the split above still landed over the new-file cap. +#[cfg(test)] +#[path = "readiness_provider_tests.rs"] +mod provider_tests; // Goose file-config-aware requirement tests live in a sibling file so this // module stays under the desktop file-size ratchet. diff --git a/desktop/src-tauri/src/managed_agents/readiness_provider_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_provider_tests.rs new file mode 100644 index 00000000000..e7eb9cfc8c1 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_provider_tests.rs @@ -0,0 +1,473 @@ +use std::collections::BTreeMap; + +use super::*; +use crate::managed_agents::discovery::known_acp_runtime_exact; + +/// 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); + EffectiveAgentEnv { + env, + config_file_path: runtime.and_then(|r| r.config_file_path), + effective_command: command.to_string(), + } +} + +fn env_with(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +// ── custom/unknown command ───────────────────────────────────────────── + +#[test] +fn unknown_command_is_always_ready() { + // Since Phase B-7 (readiness exec-check), unknown/custom commands that are + // not resolvable in PATH produce a MissingBinary requirement rather than + // being unconditionally Ready. A command that IS resolvable should be Ready. + // Use a known-present binary so the test is not environment-sensitive. + let env = make_env("sh", BTreeMap::new()); + assert!( + agent_readiness(&env).is_ready(), + "unknown/custom command present in PATH should be Ready" + ); +} + +#[test] +fn unknown_command_missing_from_path_is_not_ready() { + let env = make_env("my-custom-harness-that-does-not-exist", BTreeMap::new()); + let readiness = agent_readiness(&env); + assert!( + !readiness.is_ready(), + "unknown/custom command absent from PATH should be NotReady" + ); + let reqs = readiness.requirements(); + assert_eq!(reqs.len(), 1); + assert!( + matches!(&reqs[0], Requirement::MissingBinary { command } if command == "my-custom-harness-that-does-not-exist"), + "should surface MissingBinary requirement" + ); +} + +// ── AgentReadiness helpers ───────────────────────────────────────────── + +#[test] +fn agent_readiness_ready_has_empty_requirements() { + assert!(AgentReadiness::Ready.requirements().is_empty()); +} + +#[test] +fn agent_readiness_not_ready_exposes_requirements() { + let r = AgentReadiness::NotReady { + requirements: vec![Requirement::EnvKey { + key: "FOO".to_string(), + }], + }; + assert!(!r.is_ready()); + assert_eq!(r.requirements().len(), 1); +} + +// ── Requirement serialization ───────────────────────────────────────── + +#[test] +fn requirement_serializes_with_surface_tag() { + let r = Requirement::NormalizedField { + field: "provider".to_string(), + }; + let json = serde_json::to_value(&r).unwrap(); + assert_eq!(json["surface"], "normalized_field"); + assert_eq!(json["field"], "provider"); +} + +#[test] +fn git_bash_requirement_serializes_correctly() { + let json = serde_json::to_value(Requirement::GitBash).unwrap(); + assert_eq!(json, serde_json::json!({ "surface": "git_bash" })); +} + +#[test] +fn env_key_requirement_serializes_correctly() { + let r = Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string(), + }; + let json = serde_json::to_value(&r).unwrap(); + assert_eq!(json["surface"], "env_key"); + assert_eq!(json["key"], "ANTHROPIC_API_KEY"); +} + +#[test] +fn cli_login_requirement_serializes_correctly() { + let r = Requirement::CliLogin { + probe_args: vec![ + "codex".to_string(), + "login".to_string(), + "status".to_string(), + ], + setup_copy: "run `codex login`".to_string(), + availability: crate::managed_agents::AcpAvailabilityStatus::Available, + }; + let json = serde_json::to_value(&r).unwrap(); + assert_eq!(json["surface"], "cli_login"); + assert!(json["probe_args"].is_array()); + assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); +} + +// ── resolve_effective_agent_env ───────────────────────────────────────── + +/// Minimal `ManagedAgentRecord` carrying only the fields +/// `resolve_effective_agent_env` reads, with caller-supplied `env_vars`. +fn minimal_record( + env_vars: BTreeMap, +) -> crate::managed_agents::types::ManagedAgentRecord { + crate::managed_agents::types::ManagedAgentRecord { + pubkey: "test-pubkey".to_string(), + name: "test-agent".to_string(), + persona_id: None, + private_key_nsec: String::new(), + auth_tag: None, + relay_url: String::new(), + avatar_url: None, + acp_command: "buzz-acp".to_string(), + agent_command: "buzz-agent".to_string(), + agent_command_override: None, + agent_args: vec![], + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + model: None, + provider: None, + persona_source_version: None, + env_vars, + start_on_app_launch: false, + auto_restart_on_config_change: true, + runtime_pid: None, + backend: Default::default(), + backend_agent_id: None, + provider_binary_path: None, + team_id: None, + persona_team_dir: None, + persona_name_in_team: None, + created_at: String::new(), + updated_at: String::new(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + respond_to: Default::default(), + respond_to_allowlist: vec![], + display_name: None, + slug: None, + runtime: None, + name_pool: Vec::new(), + is_builtin: false, + is_active: true, + shared: false, + source_team: None, + source_team_persona_slug: None, + catalog_source: None, + definition_respond_to: None, + definition_respond_to_allowlist: Vec::new(), + definition_parallelism: None, + relay_mesh: None, + } +} + +#[test] +fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { + // A record whose env_vars explicitly set provider/model must win over + // any baked defaults. In OSS test builds the baked map is empty, so + // this test validates the user-env layer is present in the output. + let mut env_vars = BTreeMap::new(); + env_vars.insert("BUZZ_AGENT_PROVIDER".to_string(), "anthropic".to_string()); + env_vars.insert( + "BUZZ_AGENT_MODEL".to_string(), + "claude-opus-4-5".to_string(), + ); + + let record = minimal_record(env_vars); + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + // User env_vars must be present in the output (last-write-wins). + assert_eq!( + effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("anthropic") + ); + assert_eq!( + effective.env.get("BUZZ_AGENT_MODEL").map(String::as_str), + Some("claude-opus-4-5") + ); +} + +// ── openai-compat plain-reply delivery default ───────────────────────── +// +// Self-hosted OpenAI-compatible endpoints (llama.cpp, vLLM, a directly +// configured mesh-llm instance — the "Local LLM" case, distinct from the +// built-in relay-mesh preset) hit the same silent-drop bug relay-mesh's +// fallback exists for: a small local model answers in prose and never +// calls `send_message`. These tests lock in that this provider opts into +// the fallback by default, that the real OpenAI cloud API does not, and +// that an explicit user override always wins. + +#[test] +fn openai_compat_provider_opts_into_plain_reply_delivery() { + // The normalized provider dropdown persists to `record.provider` + // (what `resolve_effective_model_provider` actually reads), not to a + // raw `BUZZ_AGENT_PROVIDER` env var — that env var is itself derived + // from this typed field by an earlier env layer in real usage. + let mut env_vars = BTreeMap::new(); + env_vars.insert( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://llm1.example.ts.net:37073/v1".to_string(), + ); + let mut record = minimal_record(env_vars); + record.provider = Some("openai-compat".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective + .env + .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .map(String::as_str), + Some("true"), + "openai-compat should default to delivering plain-text replies" + ); +} + +#[test] +fn real_openai_provider_does_not_opt_into_plain_reply_delivery() { + // Plain "openai" (the real OpenAI cloud API) must NOT get the + // fallback: cloud models reliably call `send_message` themselves, and + // enabling it there risks double-posting. + let mut record = minimal_record(BTreeMap::new()); + record.provider = Some("openai".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective.env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES"), + None, + "real OpenAI cloud provider must not opt into plain-reply delivery" + ); +} + +#[test] +fn explicit_deliver_plain_replies_override_is_preserved_for_openai_compat() { + // A user (or a lower env layer) that explicitly disabled the fallback + // must not be silently overridden back to "true". + let mut env_vars = BTreeMap::new(); + env_vars.insert( + "BUZZ_ACP_DELIVER_PLAIN_REPLIES".to_string(), + "false".to_string(), + ); + let mut record = minimal_record(env_vars); + record.provider = Some("openai-compat".to_string()); + + let runtime = known_acp_runtime_exact("buzz-agent"); + let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); + + assert_eq!( + effective + .env + .get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .map(String::as_str), + Some("false"), + "explicit user override must survive the openai-compat default" + ); +} + +// ── provider-specific model fallback tests ──────────────────────────── + +#[test] +fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { + // The baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL. + // An agent with only DATABRICKS_MODEL must pass the readiness gate. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks_v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "DATABRICKS_MODEL must satisfy the model requirement for databricks_v2" + ); +} + +#[test] +fn buzz_agent_databricks_v2_hyphen_alias_with_databricks_model_is_ready() { + // buzz-agent accepts both "databricks_v2" and "databricks-v2". The + // readiness gate must recognize the hyphen alias and accept DATABRICKS_MODEL. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks-v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "databricks-v2 alias with DATABRICKS_MODEL must be Ready" + ); +} + +#[test] +fn buzz_agent_databricks_hyphen_alias_missing_host_returns_not_ready() { + // The hyphen alias "databricks-v2" requires DATABRICKS_HOST just like + // the underscore variants. Without it the agent cannot reach the endpoint. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks-v2"), + ("DATABRICKS_MODEL", "goose-claude-4-6-sonnet"), + // DATABRICKS_HOST intentionally absent + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "databricks-v2 without DATABRICKS_HOST must be NotReady" + ); + let reqs = result.requirements(); + assert!( + reqs.iter() + .any(|r| matches!(r, Requirement::EnvKey { key } if key == "DATABRICKS_HOST")), + "missing requirements must include DATABRICKS_HOST; got {reqs:?}" + ); +} + +#[test] +fn buzz_agent_databricks_v1_with_databricks_model_but_no_buzz_agent_model_is_ready() { + // V1 (Model Serving) also resolves DATABRICKS_MODEL — same fallback applies. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks"), + ("DATABRICKS_MODEL", "dbrx-instruct"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "DATABRICKS_MODEL must satisfy the model requirement for databricks (V1)" + ); +} + +#[test] +fn buzz_agent_anthropic_with_anthropic_model_but_no_buzz_agent_model_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("ANTHROPIC_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "ANTHROPIC_MODEL must satisfy the model requirement for anthropic" + ); +} + +#[test] +fn buzz_agent_openai_with_openai_compat_model_but_no_buzz_agent_model_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openai"), + ("OPENAI_COMPAT_MODEL", "gpt-4o"), + ("OPENAI_COMPAT_API_KEY", "sk-test"), + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "OPENAI_COMPAT_MODEL must satisfy the model requirement for openai" + ); +} + +#[test] +fn buzz_agent_empty_provider_model_fallback_key_is_not_ready() { + // An empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must still be NotReady. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks_v2"), + ("DATABRICKS_MODEL", ""), + ("DATABRICKS_HOST", "https://dbc.example.com"), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must be NotReady" + ); + assert!(result + .requirements() + .contains(&Requirement::NormalizedField { + field: "model".to_string() + })); +} + +// ── OpenRouter readiness ───────────────────────────────────────────── + +#[test] +fn buzz_agent_openrouter_with_all_fields_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), + ("OPENROUTER_API_KEY", "sk-or-test-key"), + ]), + ); + let result = agent_readiness(&env); + assert!( + result.is_ready(), + "openrouter with all fields should be ready" + ); +} + +#[test] +fn buzz_agent_openrouter_missing_key_returns_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("BUZZ_AGENT_MODEL", "anthropic/claude-sonnet-4"), + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "OPENROUTER_API_KEY".to_string() + })); +} + +#[test] +fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openrouter"), + ("OPENROUTER_MODEL", "google/gemini-2.5-flash"), + ("OPENROUTER_API_KEY", "sk-or-test-key"), + ]), + ); + let result = agent_readiness(&env); + assert!( + result.is_ready(), + "OPENROUTER_MODEL fallback should satisfy model requirement" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/readiness_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_tests.rs new file mode 100644 index 00000000000..797106c455d --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_tests.rs @@ -0,0 +1,710 @@ +use std::collections::BTreeMap; + +use super::*; +use crate::managed_agents::discovery::known_acp_runtime_exact; + +/// 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); + EffectiveAgentEnv { + env, + config_file_path: runtime.and_then(|r| r.config_file_path), + effective_command: command.to_string(), + } +} + +fn env_with(pairs: &[(&str, &str)]) -> BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() +} + +// ── buzz-agent tests ────────────────────────────────────────────────── + +#[test] +fn buzz_agent_missing_provider_returns_not_ready_with_normalized_field() { + let env = make_env( + "buzz-agent", + env_with(&[("BUZZ_AGENT_MODEL", "claude-opus-4-5")]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "missing BUZZ_AGENT_PROVIDER should be NotReady" + ); + let reqs = result.requirements(); + assert!( + reqs.contains(&Requirement::NormalizedField { + field: "provider".to_string() + }), + "requirements should include NormalizedField(provider); got {reqs:?}" + ); +} + +#[test] +fn buzz_agent_missing_model_returns_not_ready_with_normalized_field() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result + .requirements() + .contains(&Requirement::NormalizedField { + field: "model".to_string() + })); +} + +#[test] +fn buzz_agent_missing_anthropic_key_returns_not_ready_with_env_key() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string() + })); +} + +#[test] +fn buzz_agent_missing_openai_key_returns_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openai"), + ("BUZZ_AGENT_MODEL", "gpt-4o"), + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "OPENAI_COMPAT_API_KEY".to_string() + })); +} + +#[test] +fn buzz_agent_anthropic_with_all_fields_is_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + assert!(agent_readiness(&env).is_ready()); +} + +#[test] +fn buzz_agent_databricks_with_host_and_model_is_ready_without_token() { + // DATABRICKS_TOKEN is NOT required — OAuth PKCE is the normal path. + // No token present, no OAuth cache present → still Ready because we + // cannot evaluate OAuth state from the env map alone. + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks"), + ("BUZZ_AGENT_MODEL", "dbrx-instruct"), + ("DATABRICKS_HOST", "https://dbc.example.com"), + // NOTE: no DATABRICKS_TOKEN + ]), + ); + assert!( + agent_readiness(&env).is_ready(), + "Databricks with HOST+model but no TOKEN should still be Ready (OAuth path)" + ); +} + +#[test] +fn buzz_agent_databricks_missing_host_returns_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks"), + ("BUZZ_AGENT_MODEL", "dbrx-instruct"), + // NOTE: no DATABRICKS_HOST + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "DATABRICKS_HOST".to_string() + })); +} + +#[test] +fn buzz_agent_databricks_v2_missing_host_returns_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks_v2"), + ( + "BUZZ_AGENT_MODEL", + "databricks/meta-llama-4-maverick-17b-instruct", + ), + ]), + ); + let result = agent_readiness(&env); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "DATABRICKS_HOST".to_string() + })); +} + +// ── goose tests ─────────────────────────────────────────────────────── + +#[test] +fn goose_missing_provider_returns_not_ready() { + // Call goose_requirements directly with None file config so the test is + // deterministic — the `agent_readiness` path reads the real + // ~/.config/goose/config.yaml which may silence requirements on + // developer machines. + let env = make_env("goose", env_with(&[("GOOSE_MODEL", "claude-opus-4-5")])); + let reqs = goose_requirements(&env, None); + assert!( + !reqs.is_empty(), + "missing GOOSE_PROVIDER with no file config must produce requirements" + ); + assert!( + reqs.contains(&Requirement::NormalizedField { + field: "provider".to_string() + }), + "requirements must include NormalizedField(provider); got {reqs:?}" + ); +} + +#[test] +fn goose_with_provider_and_model_and_key_is_ready() { + let env = make_env( + "goose", + env_with(&[ + ("GOOSE_PROVIDER", "anthropic"), + ("GOOSE_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + assert!(agent_readiness(&env).is_ready()); +} + +// ── empty-string semantics ──────────────────────────────────────────── +// +// A key present with an empty value ("") must be treated as MISSING, to +// match the dialog's (envVars[key] ?? "").length === 0 emptiness check. + +#[test] +fn buzz_agent_empty_string_provider_is_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", ""), + ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty-string BUZZ_AGENT_PROVIDER must be treated as missing" + ); + assert!(result + .requirements() + .contains(&Requirement::NormalizedField { + field: "provider".to_string() + })); +} + +#[test] +fn buzz_agent_empty_string_model_is_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("BUZZ_AGENT_MODEL", ""), + ("ANTHROPIC_API_KEY", "sk-test"), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty-string BUZZ_AGENT_MODEL must be treated as missing" + ); + assert!(result + .requirements() + .contains(&Requirement::NormalizedField { + field: "model".to_string() + })); +} + +#[test] +fn buzz_agent_empty_string_anthropic_key_is_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "anthropic"), + ("BUZZ_AGENT_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", ""), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty-string ANTHROPIC_API_KEY must be treated as missing" + ); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string() + })); +} + +#[test] +fn buzz_agent_empty_string_databricks_host_is_not_ready() { + let env = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "databricks"), + ("BUZZ_AGENT_MODEL", "dbrx-instruct"), + ("DATABRICKS_HOST", ""), + ]), + ); + let result = agent_readiness(&env); + assert!( + !result.is_ready(), + "empty-string DATABRICKS_HOST must be treated as missing" + ); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "DATABRICKS_HOST".to_string() + })); +} + +#[test] +fn goose_empty_string_provider_is_not_ready() { + // Call goose_requirements directly with None file config so the test is + // deterministic — the `agent_readiness` path reads the real + // ~/.config/goose/config.yaml which may silence requirements on + // developer machines. + let env = make_env( + "goose", + env_with(&[("GOOSE_PROVIDER", ""), ("GOOSE_MODEL", "claude-opus-4-5")]), + ); + let reqs = goose_requirements(&env, None); + assert!( + !reqs.is_empty(), + "empty-string GOOSE_PROVIDER must be treated as missing" + ); + assert!( + reqs.contains(&Requirement::NormalizedField { + field: "provider".to_string() + }), + "requirements must include NormalizedField(provider); got {reqs:?}" + ); +} + +#[test] +fn goose_empty_string_anthropic_key_is_not_ready() { + // Call goose_requirements directly with None file config so the test is + // deterministic — the `agent_readiness` path reads the real + // ~/.config/goose/config.yaml which may silence requirements on + // developer machines. + let env = make_env( + "goose", + env_with(&[ + ("GOOSE_PROVIDER", "anthropic"), + ("GOOSE_MODEL", "claude-opus-4-5"), + ("ANTHROPIC_API_KEY", ""), + ]), + ); + let reqs = goose_requirements(&env, None); + assert!( + !reqs.is_empty(), + "empty-string ANTHROPIC_API_KEY must be treated as missing (goose)" + ); + assert!( + reqs.contains(&Requirement::EnvKey { + key: "ANTHROPIC_API_KEY".to_string() + }), + "requirements must include ANTHROPIC_API_KEY; got {reqs:?}" + ); +} + +// ── codex tests ─────────────────────────────────────────────────────── + +#[test] +fn codex_not_ready_copy_does_not_mention_openai_api_key() { + // codex uses its own credential store via `codex login` (OAuth or API key). + // The nudge copy must NOT say "set OPENAI_API_KEY". + // Use a not-installed runtime so the requirement is always emitted + // regardless of whether codex is on the test machine's PATH. + let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], None); + let reqs = cli_login::requirements(&["codex", "login", "status"], "run `codex login`", &rt); + // Whether codex is installed or not, the copy (if any) must not mention OPENAI_API_KEY. + for req in &reqs { + if let Requirement::CliLogin { setup_copy, .. } = req { + assert!( + !setup_copy.contains("OPENAI_API_KEY"), + "codex nudge copy must not mention OPENAI_API_KEY; got: {setup_copy:?}" + ); + assert!( + setup_copy.contains("codex login"), + "codex nudge copy should mention `codex login`; got: {setup_copy:?}" + ); + } + } +} + +// ── cli_login_requirements: resolve_command integration ───────────── + +/// Construct a minimal `KnownAcpRuntime` stub for testing cli_login_requirements. +/// `commands` are the adapter binaries; `underlying_cli` is the CLI name. +fn make_cli_runtime( + commands: &'static [&'static str], + underlying_cli: Option<&'static str>, +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "test-cli-runtime", + 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, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + } +} + +/// Returns the absolute path of the currently-running test binary as a `&'static str`. +/// Host-portable stand-in for a "present" binary: absolute path so `find_command` resolves +/// it via `path.exists()`. Leaked allocation is intentional — process exits after tests. +fn present_binary_str() -> &'static str { + let path = std::env::current_exe().expect("current_exe must be available in tests"); + Box::leak(path.to_string_lossy().into_owned().into_boxed_str()) +} + +/// Leak a runtime slice of `'static` strs for use in `make_cli_runtime`. +fn static_commands(commands: Vec<&'static str>) -> &'static [&'static str] { + Box::leak(commands.into_boxed_slice()) +} + +#[test] +fn cli_login_requirements_missing_binary_is_not_ready() { + // Both adapter and underlying CLI are nonexistent → NotInstalled state + // → must return a CliLogin requirement with availability=NotInstalled. + let rt = make_cli_runtime( + &["__buzz_nonexistent_adapter_abc123__"], + Some("__buzz_nonexistent_cli_abc123__"), + ); + let reqs = cli_login::requirements( + &["__buzz_nonexistent_binary_abc123__", "status"], + "install the tool first", + &rt, + ); + assert!( + !reqs.is_empty(), + "missing binary must produce a CliLogin requirement (NotReady)" + ); + assert!( + matches!(reqs[0], Requirement::CliLogin { .. }), + "requirement must be CliLogin; got {:?}", + reqs[0] + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::NotInstalled, + "both missing → NotInstalled" + ); + } +} + +#[test] +fn cli_login_requirements_adapter_missing_emits_adapter_missing() { + // Underlying CLI present (use the running test binary as a portable + // stand-in — it's always present and resolves via absolute path), + // adapter absent. + // → AdapterMissing state → no probe run → CliLogin{AdapterMissing}. + let exe = present_binary_str(); + let rt = make_cli_runtime(&["__buzz_nonexistent_adapter_xyz789__"], Some(exe)); + let reqs = cli_login::requirements(&[exe, "--list"], "install the adapter", &rt); + assert!( + !reqs.is_empty(), + "adapter missing must produce a CliLogin requirement" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::AdapterMissing, + "adapter absent, CLI present → AdapterMissing" + ); + } +} + +#[test] +fn cli_login_requirements_cli_missing_emits_cli_missing() { + // Adapter present (use the running test binary as a portable stand-in), + // underlying CLI absent. + // → CliMissing state → no probe run → CliLogin{CliMissing}. + let exe = present_binary_str(); + let rt = make_cli_runtime( + static_commands(vec![exe]), // adapter found via absolute path + Some("__buzz_nonexistent_cli_abc123__"), // underlying CLI missing + ); + let reqs = cli_login::requirements(&[exe, "--list"], "install the CLI", &rt); + assert!( + !reqs.is_empty(), + "CLI missing must produce a CliLogin requirement" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::CliMissing, + "adapter present, CLI absent → CliMissing" + ); + } +} + +#[test] +fn cli_login_requirements_resolvable_binary_runs_probe_at_resolved_path() { + // Both adapter and CLI present (use the running test binary as a + // portable stand-in — always present, resolves via absolute path), + // probe exits 0 (run with `--list` which lists tests and exits 0). + // → logged_in = true → requirements is empty (Ready). + let exe = present_binary_str(); + let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); + let reqs = cli_login::requirements( + &[exe, "--list"], + "this should not show (probe exits 0)", + &rt, + ); + assert!( + reqs.is_empty(), + "expected Ready (no requirements) when probe binary resolves and exits 0; \ + got {:?}", + reqs + ); +} + +#[test] +fn cli_login_requirements_logged_out_emits_available() { + // Both adapter and CLI present, but probe exits non-zero (logged out). + // Use the test binary with an unrecognized argument as the probe — + // libtest exits non-zero for unknown flags on all platforms. + // → CliLogin{Available} (tooling installed, needs login). + let exe = present_binary_str(); + let rt = make_cli_runtime(static_commands(vec![exe]), Some(exe)); + let reqs = cli_login::requirements(&[exe, "--buzz-probe-fail-xyz"], "run `tool login`", &rt); + assert!( + !reqs.is_empty(), + "non-zero probe must produce a CliLogin requirement (logged out)" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::Available, + "tooling installed, probe fails → Available (logged-out)" + ); + } +} + +// ── codex readiness version gate ─────────────────────────────────────── + +/// Build a minimal `KnownAcpRuntime` for testing the codex version gate. +/// `adapter_commands` are the exact strings passed to `find_command` — use +/// `&["codex-acp"]` when the binary is on PATH, or `&[]` +/// when resolving via absolute path. `underlying_cli` is a portable +/// stand-in so the adapter is not misclassified as `CliMissing`. +fn make_codex_runtime( + adapter_commands: &'static [&'static str], + underlying_cli: Option<&'static str>, +) -> KnownAcpRuntime { + KnownAcpRuntime { + id: "codex", + 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, + max_rounds_env_var: None, + required_normalized_fields: &[], + login_hint: None, + auth_probe_args: None, + } +} + +/// Build a temp dir containing a `codex-acp` script with the given body, +/// prepend it to PATH, and clear the resolve cache. Returns the temp dir +/// and the original PATH string for restoration. +#[cfg(unix)] +fn setup_temp_codex_acp(script_body: &str) -> (tempfile::TempDir, String) { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().expect("create temp dir"); + let bin = dir.path().join("codex-acp"); + std::fs::write(&bin, script_body).expect("write script"); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).expect("chmod script"); + + let original_path = std::env::var("PATH").unwrap_or_default(); + let new_path = format!("{}:{}", dir.path().display(), original_path); + std::env::set_var("PATH", &new_path); + crate::managed_agents::clear_resolve_cache(); + + (dir, original_path) +} + +#[cfg(unix)] +fn leaked_adapter_commands(bin: &std::path::Path) -> &'static [&'static str] { + let command = Box::leak(bin.display().to_string().into_boxed_str()); + Box::leak(vec![command as &'static str].into_boxed_slice()) +} + +/// Restore PATH and clear the resolve cache after a PATH-mutating test. +#[cfg(unix)] +fn restore_path(original: &str) { + std::env::set_var("PATH", original); + crate::managed_agents::clear_resolve_cache(); +} + +/// Codex readiness: outdated adapter (exits non-zero) → AdapterOutdated, +/// login probe skipped. +#[cfg(unix)] +#[test] +fn cli_login_requirements_codex_outdated_adapter_emits_adapter_outdated() { + let _guard = crate::managed_agents::lock_path_mutex(); + + let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\nexit 1\n"); + let exe = present_binary_str(); + // Use the fixture's absolute adapter path here. Bare `codex-acp` + // intentionally prefers Buzz's managed npm shim when it exists, which + // would make this version-gate regression test depend on machine state. + let rt = make_codex_runtime( + leaked_adapter_commands(&dir.path().join("codex-acp")), + Some(exe), + ); + let reqs = cli_login::requirements( + &[exe, "--buzz-probe-must-not-run-xyz"], + "run `codex login`", + &rt, + ); + + restore_path(&orig); + drop(dir); + + assert!( + !reqs.is_empty(), + "outdated codex adapter must produce a requirement; got {reqs:?}" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::AdapterOutdated, + "0.x codex adapter must yield AdapterOutdated; got {availability:?}" + ); + } else { + panic!("expected CliLogin requirement; got {:?}", reqs[0]); + } +} + +/// Codex readiness: adapter exits 0 but output is not a parseable version +/// → AdapterOutdated (garbage output treated as outdated, same as non-zero). +#[cfg(unix)] +#[test] +fn cli_login_requirements_codex_garbage_version_output_emits_adapter_outdated() { + let _guard = crate::managed_agents::lock_path_mutex(); + + let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\necho 'not a version string'\nexit 0\n"); + let exe = present_binary_str(); + let rt = make_codex_runtime( + leaked_adapter_commands(&dir.path().join("codex-acp")), + Some(exe), + ); + let reqs = cli_login::requirements( + &[exe, "--buzz-probe-must-not-run-xyz"], + "run `codex login`", + &rt, + ); + + restore_path(&orig); + drop(dir); + + assert!( + !reqs.is_empty(), + "garbage version output must produce a requirement; got {reqs:?}" + ); + if let Requirement::CliLogin { + ref availability, .. + } = reqs[0] + { + assert_eq!( + *availability, + crate::managed_agents::AcpAvailabilityStatus::AdapterOutdated, + "unparseable version output must yield AdapterOutdated; got {availability:?}" + ); + } else { + panic!("expected CliLogin requirement; got {:?}", reqs[0]); + } +} diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 961ab7f5101..9bece9e9046 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -146,7 +146,8 @@ mod tests { // Small local models end multi-step turns with prose instead of a // `send_message` call, so shared compute opts into harness delivery. assert_eq!( - env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES").map(String::as_str), + env.get("BUZZ_ACP_DELIVER_PLAIN_REPLIES") + .map(String::as_str), // `true`, not `1`: clap bool flags reject `1` and the harness // exits at startup with "invalid value '1'". Some("true")