diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index b29bf3d2f..3a8b82cc4 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -752,6 +752,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. @@ -901,6 +904,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()) @@ -1139,10 +1152,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), @@ -1158,6 +1170,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 { @@ -1450,13 +1475,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 a963de1e7..e4754ac38 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -106,7 +106,7 @@ impl Llm { .await .and_then(parse_openai_with_reasoning_details) } - Provider::OpenAi | Provider::Databricks => { + Provider::OpenAi | Provider::OpenAiCompat | Provider::Databricks => { self.openai_request(cfg, effective_model, |use_responses, request_model| { // Normalize effort for model-specific availability. Startup no longer rejects // `max` for pure OpenAI/Databricks; this per-model table is the single authority @@ -246,7 +246,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 { @@ -471,14 +471,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 @@ -2071,7 +2076,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 => { @@ -2097,9 +2102,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, } } @@ -5725,6 +5732,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 183f27dba..d5552fbfb 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)] @@ -696,7 +716,6 @@ use databricks::{ should_start_interactive_auth, }; use databricks::{discover_databricks_models, DatabricksAuthIntent}; - /// Update mutable fields on an existing managed agent record. /// /// Does NOT auto-restart the agent. Runtime config changes (system prompt, @@ -724,10 +743,8 @@ pub async fn update_managed_agent( for pubkey in &exited_pubkeys { state.clear_agent_session_caches(pubkey); } - let record = find_managed_agent_mut(&mut records, &input.pubkey)?; let previous_record = record.clone(); - let mut name_changed = false; if let Some(name_update) = input.name { let trimmed = name_update.trim().to_string(); @@ -785,7 +802,6 @@ pub async fn update_managed_agent( crate::managed_agents::validate_user_env_keys(&env_vars)?; record.env_vars = env_vars; } - // Native provider/model fields are authoritative. Keep the typed marker // derived for new records while retaining legacy typed records for // non-native providers. @@ -800,7 +816,6 @@ pub async fn update_managed_agent( record.model = Some(model_ref.clone()); record.relay_mesh = Some(crate::managed_agents::RelayMeshConfig { model_ref }); } - // Inbound author gate: merge patch onto current values, then validate // the merged state. This lets a single update switch to Allowlist AND // supply pubkeys atomically. @@ -823,21 +838,16 @@ pub async fn update_managed_agent( if input.respond_to_allowlist.is_some() { record.respond_to_allowlist = prospective_allowlist; } - record.updated_at = now_iso(); - save_managed_agents(&app, &records)?; - let record = records .iter() .find(|r| r.pubkey == input.pubkey) .ok_or_else(|| format!("agent {} not found", input.pubkey))?; - // Publish the edit to the relay. After-save, inside the lock, before // any .await. The retention upsert hashes the opt-IN projection, so an // update that touched only runtime/local fields is a no-op publish. super::agents::retain_managed_agent_pending(&app, &state, record); - let sync_params = if name_changed { let agent_keys = Keys::parse(&record.private_key_nsec) .map_err(|e| format!("failed to parse agent keys: {e}"))?; @@ -862,7 +872,6 @@ pub async fn update_managed_agent( } else { None }; - let summary = { let personas = load_personas(&app).unwrap_or_default(); build_managed_agent_summary( @@ -876,9 +885,7 @@ pub async fn update_managed_agent( let rollback = name_changed.then(|| AgentUpdateRollback::new(previous_record, record)); (summary, sync_params, rollback) }; // lock dropped here - try_regenerate_nest(&app); - // Phase 2: relay profile sync (async, outside lock). A rename is committed // only when this succeeds; otherwise restore the complete pre-edit record // so Desktop and the relay keep one authoritative name. @@ -902,15 +909,12 @@ pub async fn update_managed_agent( )); } } - Ok(UpdateManagedAgentResponse { agent: summary, profile_sync_error: None, }) } - // ── 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), @@ -927,10 +931,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() { @@ -956,7 +958,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") { @@ -978,9 +979,7 @@ pub(super) fn normalize_agent_models( } } } - let supports_switching = !models.is_empty(); - AgentModelsResponse { agent_name, agent_version, @@ -990,7 +989,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 0a40b6bd8..9ce8e7ab0 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 79dd7263c..37c5ce56e 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -105,6 +105,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!( @@ -917,3 +933,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 c072448ff..ff73820b8 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() { // A record whose env_vars explicitly set provider/model must win over @@ -1474,7 +1488,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(), @@ -1531,10 +1544,8 @@ mod tests { definition_parallelism: None, relay_mesh: 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), @@ -1545,9 +1556,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() { // The baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL. @@ -1565,7 +1574,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 @@ -1583,7 +1591,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 @@ -1608,7 +1615,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. @@ -1625,7 +1631,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( @@ -1641,7 +1646,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( @@ -1657,7 +1661,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. @@ -1680,9 +1683,7 @@ mod tests { field: "model".to_string() })); } - // ── OpenRouter readiness ───────────────────────────────────────────── - #[test] fn buzz_agent_openrouter_with_all_fields_is_ready() { let env = make_env( @@ -1699,7 +1700,6 @@ mod tests { "openrouter with all fields should be ready" ); } - #[test] fn buzz_agent_openrouter_missing_key_returns_not_ready() { let env = make_env( @@ -1715,7 +1715,6 @@ mod tests { key: "OPENROUTER_API_KEY".to_string() })); } - #[test] fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { let env = make_env( @@ -1733,7 +1732,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 11f68e8a5..198765324 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 { BUZZ_AGENT_THINKING_EFFORT, @@ -97,7 +102,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, @@ -105,7 +109,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"; @@ -120,7 +123,6 @@ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { showUnavailableEffortOptions: full, } as const; } - export function shouldRevealDependentConfigFields({ disclosure, providerFieldVisible, @@ -136,7 +138,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, @@ -144,7 +145,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. @@ -173,7 +173,6 @@ export function shouldRenderModelControl({ // Omit only on confirmed successful empty — not on failure/unavailable. return !modelDiscoverySuccessfulEmpty; } - export type AgentConfigFieldsProps = { bakedEnv: BakedEnvEntry[]; selectedRuntime: AcpRuntimeCatalogEntry | undefined; @@ -209,7 +208,6 @@ export type AgentConfigFieldsProps = { useCustomSelect?: boolean; useChevronSelectIcon?: boolean; }; - export function AgentConfigFields({ bakedEnv, selectedRuntime, @@ -239,7 +237,6 @@ export function AgentConfigFields({ showRequiredIndicators, showUnavailableEffortOptions, } = resolveDisclosure(disclosure); - const fieldModel = React.useMemo( () => deriveAgentConfigFieldModel({ @@ -254,7 +251,6 @@ export function AgentConfigFields({ effortField?.currentPersistence.kind === "envVar" ? effortField.currentPersistence.key : null; - const numericDescriptors = fieldModel.fields.filter( (d): d is NumericDescriptor => (d.kind === "maxOutputTokens" || @@ -306,7 +302,6 @@ export function AgentConfigFields({ ]), [bakedEnv, allStructuredKeys], ); - const providerValue = providerFieldVisible ? (config.provider ?? "") : ""; const providerForDiscovery = providerFieldVisible && !isCustomProvider @@ -342,6 +337,7 @@ export function AgentConfigFields({ apiKeyEnvVar, apiKeyFileSatisfied, apiKeyInherited, + apiKeyRequired, apiKeyValue, credentialsValid, } = getGlobalAgentCredentialState({ @@ -351,12 +347,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, @@ -382,7 +389,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. @@ -400,7 +406,6 @@ export function AgentConfigFields({ const mayMutateDependentFieldsRef = React.useRef(false); mayMutateDependentFieldsRef.current = healOnMount || userEditedProviderRef.current; - const autoSelectedModelScopeRef = React.useRef(null); React.useEffect(() => { if (!autoSelectModelOnProviderChange) return; @@ -414,12 +419,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 }); @@ -433,11 +436,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 @@ -451,7 +452,6 @@ export function AgentConfigFields({ const currentModel = (config.model ?? "").trim(); if (currentModel.length === 0) return; if (modelDiscoveryLoading) return; - const catalogMiss = discoveredModelOptions !== null && !discoveredModelOptions.some( @@ -460,7 +460,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); @@ -477,7 +476,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 @@ -495,7 +493,6 @@ export function AgentConfigFields({ ) { return; } - const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; onCustomModelEditingChange(false); @@ -521,7 +518,6 @@ export function AgentConfigFields({ onConfigChange({ ...config, env_vars: nextEnvVars }); }, }); - function handleProviderChange(value: string) { userEditedProviderRef.current = true; const previousApiKey = getProviderApiKeyEnvVar(effectiveProvider); @@ -548,7 +544,6 @@ export function AgentConfigFields({ delete nextEnvVars[previousApiKey]; } const providerChanged = nextProvider !== (config.provider ?? null); - onIsCustomProviderChange(false); onConfigChange({ ...config, @@ -562,28 +557,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. @@ -608,7 +598,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); @@ -622,7 +611,6 @@ export function AgentConfigFields({ } return "Select a provider"; }, [bakedProvider, providerOptions]); - const implicitEffortProvider = selectedRuntimeId === "claude" ? "anthropic" @@ -638,7 +626,6 @@ export function AgentConfigFields({ ? (config.env_vars[effortPersistenceKey] ?? "") : ""; const effortFieldVisible = showEffortField && effortField !== undefined; - const progressiveDefaults = disclosure === "progressive-defaults"; const fieldClassName = unstyled ? progressiveDefaults @@ -707,7 +694,6 @@ export function AgentConfigFields({ ))} ); - const providerContent = providerFieldVisible ? (