From 7f5c4f7b5922f5d09807be724223ee252d88e4b6 Mon Sep 17 00:00:00 2001 From: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Date: Fri, 14 Aug 2026 14:04:04 -0700 Subject: [PATCH 01/17] fix: support keyless openai-compatible endpoints Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> --- crates/buzz-agent/src/config.rs | 57 ++++++++++-- crates/buzz-agent/src/llm.rs | 70 ++++++++++++--- .../src-tauri/src/commands/agent_models.rs | 50 +++++++---- .../src/commands/agent_models_env.rs | 14 +++ .../src/commands/agent_models_tests.rs | 77 ++++++++++++++++ .../src-tauri/src/managed_agents/readiness.rs | 87 ++++++++++--------- .../features/agents/ui/AgentConfigFields.tsx | 82 +++++++++-------- .../agents/ui/AgentDefinitionDialog.tsx | 72 ++++++++------- .../agents/ui/AgentInstanceEditDialog.tsx | 78 +++++++++-------- .../ui/OpenAiCompatibleBaseUrlField.test.mjs | 21 +++++ .../ui/OpenAiCompatibleBaseUrlField.tsx | 75 ++++++++++++++++ .../features/agents/ui/agentConfigOptions.tsx | 2 +- .../ui/globalAgentCredentialState.test.mjs | 27 ++++++ .../agents/ui/globalAgentCredentialState.ts | 7 +- .../ui/providerApiKeyFieldState.test.mjs | 15 ++++ .../agents/ui/providerApiKeyFieldState.ts | 5 +- 16 files changed, 540 insertions(+), 199 deletions(-) create mode 100644 desktop/src/features/agents/ui/OpenAiCompatibleBaseUrlField.test.mjs create mode 100644 desktop/src/features/agents/ui/OpenAiCompatibleBaseUrlField.tsx diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 67d7c593b56..98c840a28ae 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -418,6 +418,9 @@ const DEFAULT_SYSTEM_PROMPT: &str = pub enum Provider { Anthropic, OpenAi, + /// A custom OpenAI-compatible endpoint. Unlike official OpenAI, the base + /// URL is explicit and bearer authentication is optional. + OpenAiCompat, /// Databricks model serving. Routes to `{base_url}/serving-endpoints/{model}/invocations` /// with a dynamically-acquired bearer (OAuth 2.0 PKCE, or static `DATABRICKS_TOKEN`). /// Wire format is OpenAI-chat-compatible — reuses the same body builder and parser. @@ -567,6 +570,16 @@ impl Config { env_or("OPENAI_COMPAT_BASE_URL", "https://api.openai.com/v1"), parse_openai_api(env("OPENAI_COMPAT_API").as_deref())?, ), + Provider::OpenAiCompat => ( + env("OPENAI_COMPAT_API_KEY").unwrap_or_default(), + resolve_model( + buzz_agent_model.as_deref(), + env("OPENAI_COMPAT_MODEL").as_deref(), + ) + .ok_or_else(|| "config: OPENAI_COMPAT_MODEL required".to_string())?, + parse_openai_compat_base_url(env("OPENAI_COMPAT_BASE_URL").as_deref())?, + parse_openai_api(env("OPENAI_COMPAT_API").as_deref())?, + ), Provider::Databricks | Provider::DatabricksV2 => ( env("DATABRICKS_TOKEN").unwrap_or_default(), resolve_model(buzz_agent_model.as_deref(), databricks_model.as_deref()) @@ -806,10 +819,9 @@ fn resolve_provider( "anthropic" => Err( "config: ANTHROPIC_API_KEY required".into(), ), - "openai" | "openai-compat" if present_nonempty(openai_key) => Ok(Provider::OpenAi), - "openai" | "openai-compat" => Err( - "config: OPENAI_COMPAT_API_KEY required".into(), - ), + "openai" if present_nonempty(openai_key) => Ok(Provider::OpenAi), + "openai" => Err("config: OPENAI_COMPAT_API_KEY required".into()), + "openai-compat" => Ok(Provider::OpenAiCompat), "databricks" => Ok(Provider::Databricks), "databricks_v2" | "databricks-v2" => Ok(Provider::DatabricksV2), "openrouter" if present_nonempty(openrouter_key) => Ok(Provider::OpenRouter), @@ -825,6 +837,19 @@ fn resolve_provider( } } +fn parse_openai_compat_base_url(raw: Option<&str>) -> Result { + let value = raw + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or_else(|| "config: OPENAI_COMPAT_BASE_URL required for openai-compat".to_string())?; + let parsed = url::Url::parse(value) + .map_err(|_| "config: OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string())?; + if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + return Err("config: OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string()); + } + Ok(value.trim_end_matches('/').to_string()) +} + /// Parse `OPENAI_COMPAT_API`. Pure (env-free) for testability; the /// caller hands in the raw value. fn parse_openai_api(raw: Option<&str>) -> Result { @@ -1117,13 +1142,31 @@ mod tests { } #[test] - fn resolve_provider_errors_when_requested_provider_key_missing() { - // No fallback — missing key returns an error regardless of Databricks availability. + fn resolve_provider_requires_only_official_openai_key() { let err = resolve_provider(Some("anthropic"), None, None, None).unwrap_err(); assert!(err.contains("ANTHROPIC_API_KEY required"), "{err}"); - let err = resolve_provider(Some("openai-compat"), None, Some(" "), None).unwrap_err(); + let err = resolve_provider(Some("openai"), None, Some(" "), None).unwrap_err(); assert!(err.contains("OPENAI_COMPAT_API_KEY required"), "{err}"); + + assert_eq!( + resolve_provider(Some("openai-compat"), None, None, None).unwrap(), + Provider::OpenAiCompat + ); + } + + #[test] + fn openai_compat_base_url_is_required_and_normalized() { + assert!(parse_openai_compat_base_url(None) + .unwrap_err() + .contains("required for openai-compat")); + assert!(parse_openai_compat_base_url(Some("ftp://localhost/v1")) + .unwrap_err() + .contains("valid HTTP(S) URL")); + assert_eq!( + parse_openai_compat_base_url(Some(" http://localhost:11434/v1/// ")).unwrap(), + "http://localhost:11434/v1" + ); } #[test] diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 47df56a6d37..28c5076ca6d 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -114,9 +114,9 @@ impl Llm { .await .and_then(parse_openai_with_reasoning_details) } - Provider::OpenAi | Provider::Databricks => { + Provider::OpenAi | Provider::OpenAiCompat | Provider::Databricks => { let provider_str = match cfg.provider { - Provider::OpenAi => "openai", + Provider::OpenAi | Provider::OpenAiCompat => "openai", Provider::Databricks => "databricks", _ => unreachable!(), }; @@ -269,7 +269,7 @@ impl Llm { let v = self.post_openrouter(cfg, &body).await?; Ok(parse_openai(v)?.text) } - Provider::OpenAi | Provider::Databricks => { + Provider::OpenAi | Provider::OpenAiCompat | Provider::Databricks => { let r = self .openai_request(cfg, effective_model, |use_responses, request_model| { if use_responses { @@ -494,14 +494,19 @@ impl Llm { // statuses map to `LlmAuth` in `post`: a 403 is indistinguishable from // an expired-token 403 here, so we refresh once and let it propagate. let mut bearer = self.auth.bearer().await.map_err(PostError::from)?; + let use_bearer = cfg.provider != Provider::OpenAiCompat || !bearer.is_empty(); let mut refreshed = false; loop { - match post(&self.http, &url, body_ref, cfg.llm_timeout, |r| { - r.bearer_auth(&bearer) + match post(&self.http, &url, body_ref, cfg.llm_timeout, |request| { + if use_bearer { + request.bearer_auth(&bearer) + } else { + request + } }) .await { - Err(PostError::Agent(AgentError::LlmAuth(_))) if !refreshed => { + Err(PostError::Agent(AgentError::LlmAuth(_))) if use_bearer && !refreshed => { refreshed = true; bearer = self .auth @@ -2076,7 +2081,7 @@ pub(crate) fn databricks_pkce_config(host: &str) -> PkceOAuthConfig { /// flow; subsequent requests use the cache + refresh transparently. pub(crate) fn build_token_source(cfg: &Config) -> Result, AgentError> { match cfg.provider { - Provider::Anthropic | Provider::OpenAi | Provider::OpenRouter => { + Provider::Anthropic | Provider::OpenAi | Provider::OpenAiCompat | Provider::OpenRouter => { Ok(Arc::new(StaticTokenSource::new(cfg.api_key.clone()))) } Provider::Databricks | Provider::DatabricksV2 => { @@ -2102,9 +2107,11 @@ pub(crate) fn build_token_source(cfg: &Config) -> Result, A pub(crate) fn summary_completion_cap(provider: Provider, max_output_tokens: u32) -> u32 { match provider { Provider::OpenRouter => max_output_tokens.saturating_mul(2), - Provider::Anthropic | Provider::OpenAi | Provider::Databricks | Provider::DatabricksV2 => { - max_output_tokens - } + Provider::Anthropic + | Provider::OpenAi + | Provider::OpenAiCompat + | Provider::Databricks + | Provider::DatabricksV2 => max_output_tokens, } } @@ -5781,6 +5788,49 @@ mod tests { } } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn post_openai_compat_omits_authorization_when_key_is_empty() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let captured = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut bytes = Vec::new(); + let mut buffer = [0u8; 4096]; + while !bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let count = socket.read(&mut buffer).await.unwrap(); + if count == 0 { + break; + } + bytes.extend_from_slice(&buffer[..count]); + } + let body = "{\"ok\":true}"; + socket + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), body + ) + .as_bytes(), + ) + .await + .unwrap(); + String::from_utf8_lossy(&bytes).to_ascii_lowercase() + }); + + let llm = llm_with(Arc::new(StaticTokenSource::new(""))); + let mut config = cfg(Provider::OpenAiCompat); + config.base_url = base; + llm.post_openai(&config, "/v1/x", &json!({}), "model") + .await + .unwrap(); + + let headers = captured.await.unwrap(); + assert!(!headers.contains("authorization:"), "{headers}"); + } + /// A single 401 forces exactly one refresh, the retry with the fresh /// token succeeds, and a *later* call gets its own refresh — proving the /// one-shot guard is per-call, not stored on the source. diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index cb809b6c04a..ba81673c97c 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -11,7 +11,8 @@ use super::managed_agent_definition::apply_model_provider_prompt_update; #[cfg(test)] use super::agent_models_env::env_value; use super::agent_models_env::{ - effective_discovery_provider, env_or_process_value, redaction_env_with_value, DiscoveryProvider, + effective_discovery_provider, env_or_process_override, env_or_process_value, + redaction_env_with_value, DiscoveryProvider, }; use super::agent_update_rollback::{rollback_failed_agent_update, AgentUpdateRollback}; @@ -362,10 +363,24 @@ fn openai_compatible_models_url(env: &BTreeMap) -> String { format!("{}/models", base_url.trim_end_matches('/')) } -fn openai_compatible_models_url_for_discovery(env: &BTreeMap) -> String { - let base_url = env_or_process_value(env, "OPENAI_COMPAT_BASE_URL") - .unwrap_or_else(|| "https://api.openai.com/v1".to_string()); - format!("{}/models", base_url.trim_end_matches('/')) +fn openai_compatible_models_url_for_discovery( + provider: Option<&str>, + env: &BTreeMap, +) -> Result { + let base_url = env_or_process_value(env, "OPENAI_COMPAT_BASE_URL"); + let base_url = if provider.map(str::trim) == Some("openai-compat") { + base_url.ok_or_else(|| { + "OPENAI_COMPAT_BASE_URL required for OpenAI-compatible model discovery".to_string() + })? + } else { + base_url.unwrap_or_else(|| "https://api.openai.com/v1".to_string()) + }; + let parsed = url::Url::parse(base_url.trim()) + .map_err(|_| "OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string())?; + if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + return Err("OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string()); + } + Ok(format!("{}/models", base_url.trim().trim_end_matches('/'))) } fn is_agent_text_model_id(id: &str) -> bool { @@ -496,8 +511,11 @@ async fn discover_openai_compatible_models( return Ok(None); } + let is_compat = provider.as_deref().map(str::trim) == Some("openai-compat"); let api_key = if relay_mesh { crate::managed_agents::RELAY_MESH_API_KEY_PLACEHOLDER.to_string() + } else if is_compat { + env_or_process_override(env, "OPENAI_COMPAT_API_KEY").unwrap_or_default() } else { match provider.required_env(env, "OPENAI_COMPAT_API_KEY")? { Some(api_key) => api_key, @@ -508,11 +526,15 @@ async fn discover_openai_compatible_models( let url = if relay_mesh { format!("{}/models", crate::managed_agents::RELAY_MESH_API_BASE_URL) } else { - openai_compatible_models_url_for_discovery(env) + openai_compatible_models_url_for_discovery(provider.as_deref(), env)? }; - let response = client - .get(&url) - .bearer_auth(&api_key) + let request = client.get(&url); + let request = if api_key.is_empty() { + request + } else { + request.bearer_auth(&api_key) + }; + let response = request .send() .await .map_err(|error| format!("OpenAI model discovery request failed: {error}"))?; @@ -673,7 +695,6 @@ async fn discover_anthropic_models( if models.is_empty() { return Err("Anthropic model discovery returned no models".to_string()); } - Ok(Some(AgentModelsResponse { agent_name: provider .as_deref() @@ -687,7 +708,6 @@ async fn discover_anthropic_models( supports_switching: true, })) } - #[path = "agent_models_databricks.rs"] mod databricks; #[cfg(test)] @@ -702,8 +722,8 @@ mod update; pub use update::update_managed_agent; pub(super) use update::{flush_managed_agent_policy, managed_agent_access_policy_changed}; -// ── Model normalization ─────────────────────────────────────────────────────── +// ── Model normalization ─────────────────────────────────────────────────────── /// Normalize raw `buzz-acp models --json` output into a typed DTO for the frontend. /// /// Merges models from both ACP paths (stable configOptions + unstable SessionModelState), @@ -720,10 +740,8 @@ pub(super) fn normalize_agent_models( .as_str() .unwrap_or("unknown") .to_string(); - let mut models: Vec = Vec::new(); let mut seen_ids: HashSet = HashSet::new(); - // 1. Stable configOptions (preferred). Only entries with category "model" // are model options — the CLI pre-filters, but we're defensive here. if let Some(config_options) = raw["stable"]["configOptions"].as_array() { @@ -749,7 +767,6 @@ pub(super) fn normalize_agent_models( } } } - // 2. Unstable availableModels (fallback — skip duplicates from stable). let mut agent_default_model: Option = None; if let Some(unstable) = raw.get("unstable") { @@ -771,9 +788,7 @@ pub(super) fn normalize_agent_models( } } } - let supports_switching = !models.is_empty(); - AgentModelsResponse { agent_name, agent_version, @@ -783,7 +798,6 @@ pub(super) fn normalize_agent_models( supports_switching, } } - #[cfg(test)] #[path = "agent_models_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/commands/agent_models_env.rs b/desktop/src-tauri/src/commands/agent_models_env.rs index 0a40b6bd8ff..9ce8e7ab098 100644 --- a/desktop/src-tauri/src/commands/agent_models_env.rs +++ b/desktop/src-tauri/src/commands/agent_models_env.rs @@ -25,6 +25,20 @@ pub(super) fn env_or_process_value(env: &BTreeMap, key: &str) -> }) } +/// Read a trimmed mapped value even when it is blank, falling back to the +/// process only when the map has no override. Optional credentials use this so +/// an explicit blank means "send no authentication" rather than inheriting an +/// unrelated process secret. +pub(super) fn env_or_process_override(env: &BTreeMap, key: &str) -> Option { + env.get(key) + .map(|value| value.trim().to_string()) + .or_else(|| { + std::env::var(key) + .ok() + .map(|value| value.trim().to_string()) + }) +} + /// Clone `env` with `key` set to the value a request actually used, so error /// redaction masks the inherited process value and not just the mapped one. pub(super) fn redaction_env_with_value( diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index a9e3b677753..35e78608f46 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -149,6 +149,22 @@ fn openai_models_url_uses_openai_default_base_url() { ); } +#[test] +fn openai_compat_models_url_requires_custom_base_url() { + let err = openai_compatible_models_url_for_discovery(Some("openai-compat"), &BTreeMap::new()) + .unwrap_err(); + assert!(err.contains("OPENAI_COMPAT_BASE_URL required"), "{err}"); + + let env = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://localhost:11434/v1/".to_string(), + )]); + assert_eq!( + openai_compatible_models_url_for_discovery(Some("openai-compat"), &env).unwrap(), + "http://localhost:11434/v1/models" + ); +} + #[test] fn anthropic_models_url_uses_anthropic_default_base_url() { assert_eq!( @@ -961,3 +977,64 @@ fn databricks_static_token_error_redacts_echoed_token() { "error lost its remediation: {error}" ); } + +#[tokio::test] +async fn openai_compat_discovery_omits_authorization_without_key() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base_url = format!("http://{}/v1", listener.local_addr().unwrap()); + let captured = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut bytes = Vec::new(); + let mut buffer = [0u8; 4096]; + while !bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let count = socket.read(&mut buffer).await.unwrap(); + if count == 0 { + break; + } + bytes.extend_from_slice(&buffer[..count]); + } + let body = r#"{"data":[{"id":"llama3","created":1}]}"#; + socket + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), + body + ) + .as_bytes(), + ) + .await + .unwrap(); + String::from_utf8_lossy(&bytes).to_ascii_lowercase() + }); + + let provider = effective_discovery_provider(Some("openai-compat"), None, &BTreeMap::new()); + let env = BTreeMap::from([("OPENAI_COMPAT_BASE_URL".to_string(), base_url)]); + let result = discover_openai_compatible_models(&reqwest::Client::new(), &provider, &env, None) + .await + .unwrap() + .unwrap(); + + assert_eq!(result.models[0].id, "llama3"); + let request = captured.await.unwrap(); + assert!(request.starts_with("get /v1/models "), "{request}"); + assert!(!request.contains("authorization:"), "{request}"); +} + +#[test] +fn optional_compat_key_allows_explicit_blank_to_shadow_process_env() { + let key = "BUZZ_TEST_OPENAI_COMPAT_API_KEY_OVERRIDE"; + let prior = std::env::var_os(key); + std::env::set_var(key, "process-secret"); + + let env = BTreeMap::from([(key.to_string(), " ".to_string())]); + assert_eq!(env_or_process_override(&env, key).as_deref(), Some("")); + + match prior { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } +} diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index f7f5d5c5d0e..a84ebf81d78 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -516,6 +516,12 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { key: "OPENAI_COMPAT_API_KEY".to_string(), }); } + Some("openai-compat") + if env_key_missing("OPENAI_COMPAT_BASE_URL") => { + missing.push(Requirement::EnvKey { + key: "OPENAI_COMPAT_BASE_URL".to_string(), + }); + } Some("databricks") | Some("databricks_v2") | Some("databricks-v2") // DATABRICKS_HOST is hard-required; DATABRICKS_TOKEN is optional // (OAuth PKCE is the normal path — see buzz-agent/src/config.rs:143). @@ -630,6 +636,14 @@ fn goose_requirements( key: "OPENAI_COMPAT_API_KEY".to_string(), }); } + Some("openai-compat") + if env_key_missing("OPENAI_COMPAT_BASE_URL") + && !file_key_present("OPENAI_COMPAT_BASE_URL") => + { + missing.push(Requirement::EnvKey { + key: "OPENAI_COMPAT_BASE_URL".to_string(), + }); + } Some("databricks") | Some("databricks_v2") | Some("databricks-v2") if env_key_missing("DATABRICKS_HOST") && !file_key_present("DATABRICKS_HOST") => { @@ -748,6 +762,35 @@ mod tests { })); } + #[test] + fn buzz_agent_openai_compat_requires_url_but_not_key() { + let without_url = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openai-compat"), + ("BUZZ_AGENT_MODEL", "llama3"), + ]), + ); + let result = agent_readiness(&without_url); + assert!(!result.is_ready()); + assert!(result.requirements().contains(&Requirement::EnvKey { + key: "OPENAI_COMPAT_BASE_URL".to_string() + })); + assert!(!result.requirements().contains(&Requirement::EnvKey { + key: "OPENAI_COMPAT_API_KEY".to_string() + })); + + let with_url = make_env( + "buzz-agent", + env_with(&[ + ("BUZZ_AGENT_PROVIDER", "openai-compat"), + ("BUZZ_AGENT_MODEL", "llama3"), + ("OPENAI_COMPAT_BASE_URL", "http://localhost:11434/v1"), + ]), + ); + assert!(agent_readiness(&with_url).is_ready()); + } + #[test] fn buzz_agent_anthropic_with_all_fields_is_ready() { let env = make_env( @@ -1203,9 +1246,7 @@ mod tests { ); } } - // ── 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 `&[]` @@ -1249,48 +1290,40 @@ mod tests { 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` @@ -1305,10 +1338,8 @@ mod tests { "run `codex login`", &rt, ); - restore_path(&orig); drop(dir); - assert!( !reqs.is_empty(), "outdated codex adapter must produce a requirement; got {reqs:?}" @@ -1326,14 +1357,12 @@ mod tests { 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( @@ -1345,10 +1374,8 @@ mod tests { "run `codex login`", &rt, ); - restore_path(&orig); drop(dir); - assert!( !reqs.is_empty(), "garbage version output must produce a requirement; got {reqs:?}" @@ -1366,9 +1393,7 @@ mod tests { 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 @@ -1381,7 +1406,6 @@ mod tests { "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()); @@ -1397,14 +1421,11 @@ mod tests { "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 { @@ -1415,9 +1436,7 @@ mod tests { assert!(!r.is_ready()); assert_eq!(r.requirements().len(), 1); } - // ── Requirement serialization ───────────────────────────────────────── - #[test] fn requirement_serializes_with_surface_tag() { let r = Requirement::NormalizedField { @@ -1427,13 +1446,11 @@ mod tests { 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 { @@ -1443,7 +1460,6 @@ mod tests { assert_eq!(json["surface"], "env_key"); assert_eq!(json["key"], "ANTHROPIC_API_KEY"); } - #[test] fn cli_login_requirement_serializes_correctly() { let r = Requirement::CliLogin { @@ -1460,9 +1476,7 @@ mod tests { assert!(json["probe_args"].is_array()); assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); } - // ── resolve_effective_agent_env ───────────────────────────────────────── - #[test] fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { // User env_vars must win over baked defaults; in OSS builds baked map is empty, @@ -1473,7 +1487,6 @@ mod tests { "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 { pubkey: "test-pubkey".to_string(), @@ -1532,10 +1545,8 @@ mod tests { relay_mesh: None, effort_level: None, }; - 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), @@ -1546,6 +1557,7 @@ mod tests { Some("claude-opus-4-5") ); } + // ── provider-specific model fallback tests ──────────────────────────── #[test] fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { @@ -1564,7 +1576,6 @@ mod tests { "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 @@ -1582,7 +1593,6 @@ mod tests { "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 @@ -1607,7 +1617,6 @@ mod tests { "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. @@ -1624,7 +1633,6 @@ mod tests { "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( @@ -1640,7 +1648,6 @@ mod tests { "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( @@ -1656,7 +1663,6 @@ mod tests { "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. @@ -1679,9 +1685,7 @@ mod tests { field: "model".to_string() })); } - // ── OpenRouter readiness ───────────────────────────────────────────── - #[test] fn buzz_agent_openrouter_with_all_fields_is_ready() { let env = make_env( @@ -1698,7 +1702,6 @@ mod tests { "openrouter with all fields should be ready" ); } - #[test] fn buzz_agent_openrouter_missing_key_returns_not_ready() { let env = make_env( @@ -1714,7 +1717,6 @@ mod tests { key: "OPENROUTER_API_KEY".to_string() })); } - #[test] fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { let env = make_env( @@ -1732,7 +1734,6 @@ mod tests { ); } } - // Goose file-config-aware requirement tests live in a sibling file so this // module stays under the desktop file-size ratchet. #[cfg(test)] diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 295c37f23c8..d175b173aae 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -47,6 +47,11 @@ import { AgentModelField, } from "@/features/agents/ui/agentConfigControls"; import { PersonaProviderApiKeyField } from "@/features/agents/ui/PersonaProviderApiKeyField"; +import { + OPENAI_COMPAT_BASE_URL, + OpenAiCompatibleBaseUrlField, + openAiCompatibleBaseUrlError, +} from "@/features/agents/ui/OpenAiCompatibleBaseUrlField"; import { usePersonaModelDiscovery } from "@/features/agents/ui/usePersonaModelDiscovery"; import { resolveModelLabel } from "@/features/agents/lib/formatAgentModelLabel"; import { @@ -98,7 +103,6 @@ const autoSelectModelOnProviderChange = true; const disableModelSelectDuringDiscovery = false; const preserveCredentialEnvVarsOnProviderChange = true; const requireProviderForModelAndEffort = true; - /** The canonical behavior contract, exported for the contract test. */ export const CANONICAL_CONFIG_BEHAVIORS = { autoSelectModelOnProviderChange, @@ -106,7 +110,6 @@ export const CANONICAL_CONFIG_BEHAVIORS = { preserveCredentialEnvVarsOnProviderChange, requireProviderForModelAndEffort, } as const; - /** Disclosure preset → the eight visibility decisions it owns. Exported for the contract test. */ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { const full = disclosure !== "onboarding-essential"; @@ -121,7 +124,6 @@ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { showUnavailableEffortOptions: full, } as const; } - export function shouldRevealDependentConfigFields({ disclosure, providerFieldVisible, @@ -137,7 +139,6 @@ export function shouldRevealDependentConfigFields({ providerValue.trim().length > 0 ); } - /** Whether the status line under the Model field renders. Discovery warnings bypass onboarding-essential so first-run failures are never invisible. */ export function shouldShowModelStatusMessage( showDescriptions: boolean, @@ -145,7 +146,6 @@ export function shouldShowModelStatusMessage( ): boolean { return showDescriptions || status !== null; } - /** * Renders the Model control given discovery state. Optional-model harnesses omit it while * discovery is loading or after confirmed successful empty; failures keep it for the #2246 UI. @@ -174,7 +174,6 @@ export function shouldRenderModelControl({ // Omit only on confirmed successful empty — not on failure/unavailable. return !modelDiscoverySuccessfulEmpty; } - export type AgentConfigFieldsProps = { bakedEnv: BakedEnvEntry[]; selectedRuntime: AcpRuntimeCatalogEntry | undefined; @@ -210,7 +209,6 @@ export type AgentConfigFieldsProps = { useCustomSelect?: boolean; useChevronSelectIcon?: boolean; }; - export function AgentConfigFields({ bakedEnv, selectedRuntime, @@ -240,7 +238,6 @@ export function AgentConfigFields({ showRequiredIndicators, showUnavailableEffortOptions, } = resolveDisclosure(disclosure); - const fieldModel = React.useMemo( () => deriveAgentConfigFieldModel({ @@ -255,7 +252,6 @@ export function AgentConfigFields({ effortField?.currentPersistence.kind === "envVar" ? effortField.currentPersistence.key : null; - const numericDescriptors = fieldModel.fields.filter( (d): d is NumericDescriptor => (d.kind === "maxOutputTokens" || @@ -307,7 +303,6 @@ export function AgentConfigFields({ ]), [bakedEnv, allStructuredKeys], ); - const providerValue = providerFieldVisible ? (config.provider ?? "") : ""; const providerForDiscovery = providerFieldVisible && !isCustomProvider @@ -343,6 +338,7 @@ export function AgentConfigFields({ apiKeyEnvVar, apiKeyFileSatisfied, apiKeyInherited, + apiKeyRequired, apiKeyValue, credentialsValid, } = getGlobalAgentCredentialState({ @@ -352,12 +348,23 @@ export function AgentConfigFields({ runtimeFileConfig, runtimeId: credentialRuntimeId, }); + const compatibleBaseUrl = config.env_vars[OPENAI_COMPAT_BASE_URL] ?? ""; + const compatibleBaseUrlInherited = + effectiveProvider === "openai-compat" && + compatibleBaseUrl.trim().length === 0 && + credentialsValid; + const compatibleBaseUrlValid = + effectiveProvider !== "openai-compat" || + compatibleBaseUrlInherited || + openAiCompatibleBaseUrlError(compatibleBaseUrl) === null; const configIsValid = - selectedRuntimeId.length > 0 && modelIsValid && credentialsValid; + selectedRuntimeId.length > 0 && + modelIsValid && + credentialsValid && + compatibleBaseUrlValid; React.useEffect(() => { onValidityChange?.(configIsValid); }, [configIsValid, onValidityChange]); - const { discoveredModelOptions, modelDiscoveryLoading, @@ -383,7 +390,6 @@ export function AgentConfigFields({ modelIsOptional, showCustomModelOption, }); - // Mount-time healing policy: onboarding page 4 edits the root config during // first-run (no higher layers to inherit from), so acting on open is safe // and intentional there — it heals stale state and picks a valid model. @@ -401,7 +407,6 @@ export function AgentConfigFields({ const mayMutateDependentFieldsRef = React.useRef(false); mayMutateDependentFieldsRef.current = healOnMount || userEditedProviderRef.current; - const autoSelectedModelScopeRef = React.useRef(null); React.useEffect(() => { if (!autoSelectModelOnProviderChange) return; @@ -415,12 +420,10 @@ export function AgentConfigFields({ if (modelDiscoveryLoading || discoveredModelOptions === null) return; const selectionScope = `${selectedRuntimeId}:${trimmedProvider}`; if (autoSelectedModelScopeRef.current === selectionScope) return; - const firstModel = discoveredModelOptions.find( (option) => option.id.trim().length > 0, ); if (!firstModel) return; - autoSelectedModelScopeRef.current = selectionScope; onCustomModelEditingChange(false); onConfigChange({ ...config, model: firstModel.id }); @@ -434,11 +437,9 @@ export function AgentConfigFields({ providerForDiscovery, selectedRuntimeId, ]); - const currentEffortForAutoClear = effortPersistenceKey ? (config.env_vars[effortPersistenceKey] ?? "") : ""; - // When the selected harness changes outside this component (Back → setup // page → choose a different harness → Next), the saved model can belong to // the old harness. In onboarding, heal that stale value as soon as the new @@ -452,7 +453,6 @@ export function AgentConfigFields({ const currentModel = (config.model ?? "").trim(); if (currentModel.length === 0) return; if (modelDiscoveryLoading) return; - const catalogMiss = discoveredModelOptions !== null && !discoveredModelOptions.some( @@ -461,7 +461,6 @@ export function AgentConfigFields({ const omittedAfterSuccessfulEmpty = modelIsOptional && !modelControlVisible && modelDiscoverySuccessfulEmpty; if (!catalogMiss && !omittedAfterSuccessfulEmpty) return; - const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; onCustomModelEditingChange(false); @@ -478,7 +477,6 @@ export function AgentConfigFields({ healOnMount, effortPersistenceKey, ]); - // Orphan-model clearing follows the mount-time healing policy above: the // backend resolves provider and model independently across layers // (agent → definition → global), so a saved global model WITHOUT a global @@ -496,7 +494,6 @@ export function AgentConfigFields({ ) { return; } - const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; onCustomModelEditingChange(false); @@ -522,7 +519,6 @@ export function AgentConfigFields({ onConfigChange({ ...config, env_vars: nextEnvVars }); }, }); - function handleProviderChange(value: string) { userEditedProviderRef.current = true; const previousApiKey = getProviderApiKeyEnvVar(effectiveProvider); @@ -549,7 +545,6 @@ export function AgentConfigFields({ delete nextEnvVars[previousApiKey]; } const providerChanged = nextProvider !== (config.provider ?? null); - onIsCustomProviderChange(false); onConfigChange({ ...config, @@ -563,28 +558,23 @@ export function AgentConfigFields({ : config.model, }); } - function handleCustomProviderInput(value: string) { onConfigChange({ ...config, provider: value || null }); } - function handleModelChange(value: string) { onConfigChange({ ...config, model: config.provider === "relay-mesh" ? value || "auto" : value || null, }); } - function handleEnvVarsChange(next: Record) { onConfigChange({ ...config, env_vars: next }); } - const handleNumericEnvVarChange = (key: string, value: string) => { const next = { ...config.env_vars, [key]: value }; if (value === "") delete next[key]; onConfigChange({ ...config, env_vars: next }); }; - // On internal Block builds, BUZZ_AGENT_PROVIDER is baked in and a boot // migration rewrites v1→v2. Hide the legacy v1 option so it is not offered // for new selections; OSS builds show it. @@ -609,7 +599,6 @@ export function AgentConfigFields({ const providerSelectValue = isCustomProvider ? CUSTOM_PROVIDER_DROPDOWN_VALUE : providerValue || AUTO_PROVIDER_DROPDOWN_VALUE; - const providerZeroLabel = React.useMemo(() => { if (!bakedProvider) return null; return getBakedProviderInheritLabel(bakedProvider, providerOptions); @@ -623,7 +612,6 @@ export function AgentConfigFields({ } return "Select a provider"; }, [bakedProvider, providerOptions]); - const implicitEffortProvider = selectedRuntimeId === "claude" ? "anthropic" @@ -639,7 +627,6 @@ export function AgentConfigFields({ ? (config.env_vars[effortPersistenceKey] ?? "") : ""; const effortFieldVisible = showEffortField && effortField !== undefined; - const progressiveDefaults = disclosure === "progressive-defaults"; const fieldClassName = unstyled ? progressiveDefaults @@ -708,7 +695,6 @@ export function AgentConfigFields({ ))} ); - const providerContent = providerFieldVisible ? (