diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 4e3ec919b5..3ec9e7b63f 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -534,6 +534,8 @@ impl Config { let openai_key = env("OPENAI_API_KEY"); let legacy_openai_key = env("OPENAI_COMPAT_API_KEY"); let openai_base_url = env("OPENAI_COMPAT_BASE_URL"); + let selected_openai_key = + first_nonempty(openai_key.as_deref(), legacy_openai_key.as_deref()); let provider = resolve_provider( normalize_legacy_openai_provider( requested_provider.as_deref(), @@ -543,7 +545,7 @@ impl Config { ) .as_deref(), env("ANTHROPIC_API_KEY").as_deref(), - openai_key.as_deref().or(legacy_openai_key.as_deref()), + selected_openai_key, env("OPENROUTER_API_KEY").as_deref(), )?; @@ -571,10 +573,8 @@ impl Config { OpenAiApi::Auto, // unused for Anthropic ), Provider::OpenAi => ( - openai_key - .clone() - .or_else(|| legacy_openai_key.clone()) - .filter(|value| !value.trim().is_empty()) + selected_openai_key + .map(str::to_string) .ok_or_else(|| "config: OPENAI_API_KEY required".to_string())?, resolve_model( buzz_agent_model.as_deref(), @@ -817,6 +817,12 @@ fn resolve_model( explicit_override.or(provider_default).map(str::to_owned) } +fn first_nonempty<'a>(primary: Option<&'a str>, fallback: Option<&'a str>) -> Option<&'a str> { + primary + .filter(|value| !value.trim().is_empty()) + .or_else(|| fallback.filter(|value| !value.trim().is_empty())) +} + fn present_nonempty(v: Option<&str>) -> bool { v.map(str::trim).is_some_and(|s| !s.is_empty()) } @@ -1233,6 +1239,12 @@ mod tests { ); } + #[test] + fn empty_official_key_falls_back_to_populated_legacy_key() { + assert_eq!(first_nonempty(Some(""), Some("legacy")), Some("legacy")); + assert_eq!(first_nonempty(Some(" "), Some("legacy")), Some("legacy")); + } + #[test] fn resolve_provider_requires_only_official_openai_key() { let err = resolve_provider(Some("anthropic"), None, None, None).unwrap_err(); diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index 55610f12e2..7b2cee78a6 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -99,9 +99,9 @@ pub async fn get_agent_models( let mut merged_env = discovery_env_with_baked_floor(merged_env); // Resolve against the baked/process env when the record saved no provider, // so a build-provided provider still gets live discovery. - let effective_provider = + let mut effective_provider = effective_discovery_provider(saved_provider.as_deref(), provider_env_var, &merged_env); - crate::managed_agents::readiness::canonicalize_openai_provider_env( + effective_provider.value = crate::managed_agents::readiness::canonicalize_openai_provider_env( &mut merged_env, effective_provider.as_deref(), ); @@ -238,12 +238,12 @@ pub async fn discover_agent_models( let mut merged_env = discovery_env_with_baked_floor(merged_env); // Recover a build-provided provider when the form has none, so the create // dialog discovers live models instead of falling through to the subprocess. - let effective_provider = effective_discovery_provider( + let mut effective_provider = effective_discovery_provider( input.provider.as_deref(), runtime_meta.and_then(|meta| meta.provider_env_var), &merged_env, ); - crate::managed_agents::readiness::canonicalize_openai_provider_env( + effective_provider.value = crate::managed_agents::readiness::canonicalize_openai_provider_env( &mut merged_env, effective_provider.as_deref(), ); diff --git a/desktop/src-tauri/src/commands/agent_models_env.rs b/desktop/src-tauri/src/commands/agent_models_env.rs index 95a576a212..a2bbcf54b4 100644 --- a/desktop/src-tauri/src/commands/agent_models_env.rs +++ b/desktop/src-tauri/src/commands/agent_models_env.rs @@ -61,8 +61,12 @@ pub(super) fn openai_compatible_models_url_for_discovery( ) -> Result { let is_compat = provider.is_some_and(|value| value.trim().eq_ignore_ascii_case("openai-compat")); - let configured_base_url = - env_or_process_override(env, "OPENAI_COMPAT_BASE_URL").filter(|value| !value.is_empty()); + let configured_base_url = if is_compat { + env_or_process_override(env, "OPENAI_COMPAT_BASE_URL") + } else { + env_value(env, "OPENAI_COMPAT_BASE_URL") + } + .filter(|value| !value.is_empty()); if !is_compat { if configured_base_url .as_deref() @@ -106,7 +110,7 @@ pub(super) fn redaction_env_with_value( /// turn the subprocess catalog into `config: ANTHROPIC_API_KEY required`. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct DiscoveryProvider { - value: Option, + pub(super) value: Option, inferred: bool, } diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 30144841a1..5214dd5a27 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -2,8 +2,6 @@ //! projections and their NIP-09 tombstones. Extracted from the parent module to //! keep it under the file-size cap. -use std::collections::BTreeMap; - use tauri::{AppHandle, Emitter, Manager}; use crate::{ @@ -511,32 +509,6 @@ fn event_d_tag(event: &nostr::Event) -> Result { .ok_or_else(|| "inbound event missing d-tag".to_string()) } -fn normalize_inbound_openai_provider( - provider: Option, - local_env: &BTreeMap, -) -> Option { - let is_legacy_openai = provider - .as_deref() - .is_some_and(|value| value.trim().eq_ignore_ascii_case("openai")) - && local_env - .get("OPENAI_API_KEY") - .is_none_or(|value| value.trim().is_empty()) - && local_env - .get("OPENAI_COMPAT_API_KEY") - .is_some_and(|value| !value.trim().is_empty()); - let has_custom_origin = local_env - .get("OPENAI_COMPAT_BASE_URL") - .map(String::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()) - .is_some_and(|value| value.trim_end_matches('/') != "https://api.openai.com/v1"); - if is_legacy_openai && has_custom_origin { - Some("openai-compat".to_string()) - } else { - provider - } -} - /// Merge a parsed inbound persona into the local set: patch the matching record /// in place, or push it when none matches. /// @@ -558,7 +530,7 @@ fn apply_inbound_persona(personas: &mut Vec, inbound: AgentDefi local.system_prompt = inbound.system_prompt; local.runtime = inbound.runtime; local.model = inbound.model; - local.provider = normalize_inbound_openai_provider(inbound.provider, &local.env_vars); + local.provider = inbound.provider; local.name_pool = inbound.name_pool; local.respond_to = inbound.respond_to; local.respond_to_allowlist = inbound.respond_to_allowlist; @@ -604,7 +576,7 @@ fn apply_inbound_managed_agent( if !definition_linked { local.system_prompt = inbound.system_prompt; local.model = inbound.model; - local.provider = normalize_inbound_openai_provider(inbound.provider, &local.env_vars); + local.provider = inbound.provider; local.persona_source_version = inbound.persona_source_version; } local.parallelism = inbound.parallelism; diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 0c3411f680..4567afa840 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -52,7 +52,7 @@ fn inbound_legacy_openai_provider_keeps_local_custom_origin_compatible() { apply_inbound_persona(&mut personas, inbound); - assert_eq!(personas[0].provider.as_deref(), Some("openai-compat")); + assert_eq!(personas[0].provider.as_deref(), Some("openai")); } #[test] diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 388b11d34f..536ea28937 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -269,7 +269,7 @@ fn resolve_effective_agent_env_with_def( ); env.extend(user_env); - canonicalize_openai_provider_env(&mut env, effective_provider.as_deref()); + let _ = canonicalize_openai_provider_env(&mut env, effective_provider.as_deref()); // Buzz shared compute is a native Buzz provider. Translate it to buzz-agent's // OpenAI-compatible transport only in the effective runtime environment. @@ -290,12 +290,27 @@ fn resolve_effective_agent_env_with_def( pub(crate) fn canonicalize_openai_provider_env( env: &mut BTreeMap, provider: Option<&str>, -) { - match provider - .map(str::trim) - .map(str::to_ascii_lowercase) - .as_deref() - { +) -> Option { + let mut provider = provider.map(str::trim).map(str::to_ascii_lowercase); + if provider.as_deref() == Some("openai") { + let has_official_key = env + .get("OPENAI_API_KEY") + .is_some_and(|value| !value.trim().is_empty()); + let has_legacy_key = env + .get("OPENAI_COMPAT_API_KEY") + .is_some_and(|value| !value.trim().is_empty()); + let has_custom_origin = env + .get("OPENAI_COMPAT_BASE_URL") + .map(String::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .is_some_and(|value| value.trim_end_matches('/') != "https://api.openai.com/v1"); + if !has_official_key && has_legacy_key && has_custom_origin { + provider = Some("openai-compat".to_string()); + } + } + + match provider.as_deref() { Some("openai") => { env.remove("OPENAI_COMPAT_BASE_URL"); env.remove("OPENAI_COMPAT_API_KEY"); @@ -306,6 +321,10 @@ pub(crate) fn canonicalize_openai_provider_env( } _ => {} } + if let (Some(value), Some(slot)) = (provider.as_ref(), env.get_mut("BUZZ_AGENT_PROVIDER")) { + *slot = value.clone(); + } + provider } // ── Requirement types ───────────────────────────────────────────────────────── diff --git a/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs index c0cbd8a7c9..9a47dd5056 100644 --- a/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs +++ b/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs @@ -39,6 +39,27 @@ fn provider_env_canonicalization_isolates_consumers() { ); } +#[test] +fn inherited_legacy_custom_configuration_normalizes_per_consumer() { + let mut env = BTreeMap::from([ + ("BUZZ_AGENT_PROVIDER".into(), "openai".into()), + ("OPENAI_COMPAT_API_KEY".into(), "legacy".into()), + ( + "OPENAI_COMPAT_BASE_URL".into(), + "http://localhost:11434/v1".into(), + ), + ]); + assert_eq!( + canonicalize_openai_provider_env(&mut env, Some("openai")).as_deref(), + Some("openai-compat") + ); + assert_eq!( + env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("openai-compat") + ); + assert!(!env.contains_key("OPENAI_API_KEY")); +} + #[test] fn keyless_compatible_provider_gets_explicit_blank_credential() { let mut env = BTreeMap::from([( diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index cb764bbf7b..9652e2a0bf 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -24,7 +24,7 @@ pub(crate) use super::access_policy::{build_respond_to_env_with_policy, RespondT mod metadata; pub(crate) use metadata::{ apply_agent_display_env, resolve_session_title, runtime_metadata_env_vars, - scrub_unconfigured_openai_compat_key, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, + scrub_ambient_openai_env, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, }; mod stop; @@ -711,7 +711,11 @@ pub fn spawn_agent_child( let mesh_model_id = effective_cfg.relay_mesh_model_id(); let effective_prompt = effective_cfg.system_prompt.value; let effective_model = effective_cfg.model.value; - let effective_provider = effective_cfg.provider.value; + let effective_provider = descriptor + .env + .get("BUZZ_AGENT_PROVIDER") + .cloned() + .or(effective_cfg.provider.value); if let Some(prompt) = &effective_prompt { command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt); @@ -810,11 +814,7 @@ pub fn spawn_agent_child( command.env(key, value); } - scrub_unconfigured_openai_compat_key( - &mut command, - effective_provider.as_deref(), - &descriptor.env, - ); + scrub_ambient_openai_env(&mut command, effective_provider.as_deref(), &descriptor.env); // (ANTHROPIC_MODEL is applied post-loop for the same reason). When effort_level is // None there is no canonical value to assert, so env passthrough stands — user env diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index 5cd66ffb08..da21338716 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -24,16 +24,23 @@ pub(crate) fn runtime_metadata_env_vars<'a>( vars } -pub(crate) fn scrub_unconfigured_openai_compat_key( +pub(crate) fn scrub_ambient_openai_env( command: &mut std::process::Command, effective_provider: Option<&str>, env: &std::collections::BTreeMap, ) { - if effective_provider - .is_some_and(|provider| provider.trim().eq_ignore_ascii_case("openai-compat")) - && !env.contains_key("OPENAI_COMPAT_API_KEY") - { - command.env_remove("OPENAI_COMPAT_API_KEY"); + match effective_provider.map(str::trim) { + Some(provider) if provider.eq_ignore_ascii_case("openai") => { + command.env_remove("OPENAI_COMPAT_API_KEY"); + command.env_remove("OPENAI_COMPAT_BASE_URL"); + } + Some(provider) if provider.eq_ignore_ascii_case("openai-compat") => { + command.env_remove("OPENAI_API_KEY"); + if !env.contains_key("OPENAI_COMPAT_API_KEY") { + command.env_remove("OPENAI_COMPAT_API_KEY"); + } + } + _ => {} } } @@ -89,14 +96,14 @@ pub(crate) fn resolve_session_title(display_name: Option<&str>, name: &str) -> O #[cfg(test)] mod tests { - use super::{resolve_session_title, scrub_unconfigured_openai_compat_key}; + use super::{resolve_session_title, scrub_ambient_openai_env}; use std::collections::BTreeMap; #[test] fn keyless_openai_compat_scrubs_parent_process_credential() { let mut command = std::process::Command::new("echo"); command.env("OPENAI_COMPAT_API_KEY", "ambient-secret"); - scrub_unconfigured_openai_compat_key(&mut command, Some("openai-compat"), &BTreeMap::new()); + scrub_ambient_openai_env(&mut command, Some("openai-compat"), &BTreeMap::new()); assert!(command .get_envs() .any(|(key, value)| key == "OPENAI_COMPAT_API_KEY" && value.is_none())); @@ -106,12 +113,25 @@ mod tests { fn configured_openai_compat_key_is_preserved() { let mut command = std::process::Command::new("echo"); let env = BTreeMap::from([("OPENAI_COMPAT_API_KEY".to_string(), "key".to_string())]); - scrub_unconfigured_openai_compat_key(&mut command, Some("openai-compat"), &env); + scrub_ambient_openai_env(&mut command, Some("openai-compat"), &env); assert!(!command .get_envs() .any(|(key, value)| key == "OPENAI_COMPAT_API_KEY" && value.is_none())); } + #[test] + fn official_openai_scrubs_ambient_compat_routing() { + let mut command = std::process::Command::new("echo"); + command.env("OPENAI_COMPAT_API_KEY", "ambient-secret"); + command.env("OPENAI_COMPAT_BASE_URL", "http://localhost:11434/v1"); + scrub_ambient_openai_env(&mut command, Some("openai"), &BTreeMap::new()); + for key in ["OPENAI_COMPAT_API_KEY", "OPENAI_COMPAT_BASE_URL"] { + assert!(command + .get_envs() + .any(|(name, value)| name == key && value.is_none())); + } + } + #[test] fn resolve_session_title_prefers_display_name() { assert_eq!( diff --git a/desktop/src-tauri/src/migration/openai_credentials.rs b/desktop/src-tauri/src/migration/openai_credentials.rs index e26b814f0d..d9700dd36d 100644 --- a/desktop/src-tauri/src/migration/openai_credentials.rs +++ b/desktop/src-tauri/src/migration/openai_credentials.rs @@ -399,7 +399,7 @@ fn set_provider(store: &mut Store, owner: Owner, provider: &str) -> Result<(), S fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { let consumers = collect_consumers(&store)?; - let mut provider_targets: HashMap = HashMap::new(); + let mut provider_targets: HashMap, bool)> = HashMap::new(); for consumer in &consumers { let expected = if consumer.identity == Identity::Official { "openai" @@ -408,21 +408,19 @@ fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { }; match provider_targets.entry(consumer.provider_owner.clone()) { std::collections::hash_map::Entry::Vacant(entry) => { - entry.insert((consumer.identity, consumer.provider != expected)); + entry.insert((Some(consumer.identity), consumer.provider != expected)); } std::collections::hash_map::Entry::Occupied(mut entry) => { - if entry.get().0 != consumer.identity { - return Err(format!( - "provider owner {:?} feeds mixed official/custom consumers", - consumer.provider_owner - )); + if entry.get().0 != Some(consumer.identity) { + entry.get_mut().0 = None; + } else { + entry.get_mut().1 |= consumer.provider != expected; } - entry.get_mut().1 |= consumer.provider != expected; } } } for (owner, (identity, needs_change)) in provider_targets { - if needs_change { + if let (Some(identity), true) = (identity, needs_change) { set_provider( &mut store, owner, @@ -435,35 +433,27 @@ fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { } } - let mut legacy_targets: HashMap = HashMap::new(); + let mut official_legacy_targets = Vec::new(); for consumer in &consumers { - if consumer.identity == Identity::Official - && consumer + if consumer.identity != Identity::Official + || consumer .official_key .as_ref() .is_some_and(|v| !v.value.is_empty()) { continue; } - let Some(legacy) = &consumer.legacy_key else { - continue; - }; - if let Some(previous) = legacy_targets.insert(legacy.owner.clone(), consumer.identity) { - if previous != consumer.identity { - return Err(format!( - "credential owner {:?} feeds mixed official/custom consumers", - legacy.owner - )); - } + if let Some(legacy) = &consumer.legacy_key { + official_legacy_targets.push(legacy.owner.clone()); } } - for (owner, identity) in legacy_targets { - if identity == Identity::Official { - let env = env_mut(&mut store, owner)?; - if env.get(OPENAI_API_KEY).is_none_or(|value| value.is_empty()) { - if let Some(value) = env.remove(OPENAI_COMPAT_API_KEY) { - env.insert(OPENAI_API_KEY.into(), value); - } + official_legacy_targets.sort_by_key(|owner| format!("{owner:?}")); + official_legacy_targets.dedup(); + for owner in official_legacy_targets { + let env = env_mut(&mut store, owner)?; + if env.get(OPENAI_API_KEY).is_none_or(|value| value.is_empty()) { + if let Some(value) = env.get(OPENAI_COMPAT_API_KEY).cloned() { + env.insert(OPENAI_API_KEY.into(), value); } } } @@ -566,17 +556,6 @@ fn verify_post_split(before: &[Consumer], after: &Store) -> Result<(), String> { let record_index = old .record_index .ok_or("managed consumer has no record index")?; - let config = - match resolve_effective_config(&after.records[record_index], &defs, &after.global) { - EffectiveConfigResult::Resolved(c) => c, - _ => return Err(format!("consumer {:?} became unresolved", old.label)), - }; - if config.provider.value.as_deref() != Some(expected_provider) { - return Err(format!( - "consumer {:?} resolves provider {:?}, expected {expected_provider}", - old.label, config.provider.value - )); - } let descriptor = crate::managed_agents::resolve_effective_harness_descriptor( &after.records[record_index], &defs, diff --git a/desktop/src-tauri/src/migration/openai_credentials_tests.rs b/desktop/src-tauri/src/migration/openai_credentials_tests.rs index 0c45d46b71..f73cfebb9b 100644 --- a/desktop/src-tauri/src/migration/openai_credentials_tests.rs +++ b/desktop/src-tauri/src/migration/openai_credentials_tests.rs @@ -63,6 +63,16 @@ fn whole_store_matrix_classifies_origins_and_moves_only_official_credentials() { .map(String::as_str), Some("secret") ); + if expected_provider == "openai" { + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("secret"), + "migration copies the shared legacy key during compatibility" + ); + } if expected_provider == "openai" && origin.is_some() { assert_eq!( store.records[0] @@ -173,7 +183,7 @@ fn existing_official_key_wins_without_copying_legacy_secret() { } #[test] -fn mixed_global_provider_owner_is_rejected() { +fn mixed_global_provider_owner_is_resolved_per_consumer() { let global = GlobalAgentConfig { provider: Some("openai".into()), ..Default::default() @@ -190,9 +200,22 @@ fn mixed_global_provider_owner_is_rejected() { OPENAI_COMPAT_BASE_URL.into(), "https://gateway.example/v1".into(), ); - assert!(planned(global, vec![official, custom]) - .unwrap_err() - .contains("mixed official/custom")); + let store = planned(global, vec![official, custom]).unwrap(); + assert_eq!(store.global.provider.as_deref(), Some("openai")); + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("a") + ); + assert_eq!( + store.records[1] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("b") + ); } #[test] @@ -236,7 +259,7 @@ fn linked_provider_snapshot_is_ignored_and_definition_is_mutated() { } #[test] -fn linked_local_identity_that_definition_cannot_represent_is_rejected() { +fn linked_local_identity_is_resolved_per_consumer() { let mut def = definition("shared"); def.provider = Some("openai".into()); let mut official = record("official", "official"); @@ -253,10 +276,21 @@ fn linked_local_identity_that_definition_cannot_represent_is_rejected() { OPENAI_COMPAT_BASE_URL.into(), "https://gateway.example/v1".into(), ); - assert!( - planned(GlobalAgentConfig::default(), vec![def, official, custom]) - .unwrap_err() - .contains("mixed official/custom") + let store = planned(GlobalAgentConfig::default(), vec![def, official, custom]).unwrap(); + assert_eq!(store.records[0].provider.as_deref(), Some("openai")); + assert_eq!( + store.records[1] + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("a") + ); + assert_eq!( + store.records[2] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("b") ); }