diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index da1dc579..fbad9eec 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -2549,6 +2549,37 @@ async fn test_provider_connection(req: TestConnectionRequest) -> TestConnectionR let base = req.base_url.trim_end_matches('/'); match req.adapter.as_str() { + "nearai" if base.contains("private") => { + // NEAR AI private endpoints use /health for connectivity checks. + let url = format!("{base}/health"); + let mut builder = client.get(&url); + if let Some(key) = req.api_key.as_deref().filter(|k| !k.is_empty()) { + builder = builder.header("Authorization", format!("Bearer {key}")); + } + match builder.send().await { + Ok(r) if r.status().is_success() => TestConnectionResponse { + ok: true, + message: format!("Connected ({})", r.status()), + }, + Ok(r) + if r.status() == reqwest::StatusCode::UNAUTHORIZED + || r.status() == reqwest::StatusCode::FORBIDDEN => + { + TestConnectionResponse { + ok: false, + message: format!("Authentication failed ({})", r.status()), + } + } + Ok(r) => TestConnectionResponse { + ok: false, + message: format!("Server returned {}", r.status()), + }, + Err(e) => TestConnectionResponse { + ok: false, + message: format!("Connection failed: {e}"), + }, + } + } "ollama" => { let url = format!("{base}/api/tags"); match client.get(&url).send().await { @@ -2625,9 +2656,10 @@ fn interpret_chat_response( message: format!("Authentication failed ({})", status), } } else if status == reqwest::StatusCode::BAD_REQUEST + || status == reqwest::StatusCode::NOT_FOUND || status == reqwest::StatusCode::UNPROCESSABLE_ENTITY { - // 400/422 = server reachable, likely wrong model name — still a success for connectivity + // 400/404/422 = server reachable, likely wrong model name or endpoint variant — still a success for connectivity TestConnectionResponse { ok: true, message: format!("Server reachable ({})", status), @@ -2737,13 +2769,15 @@ async fn fetch_provider_models(req: ListModelsRequest) -> ListModelsResponse { } } _ => { - // OpenAI-compatible and Anthropic both support GET /models - let effective_base = - if req.adapter == "anthropic" && !base.ends_with("/v1") && !base.contains("/v1/") { - format!("{base}/v1") - } else { - base.to_string() - }; + // OpenAI-compatible, Anthropic, and NEAR AI all support GET /models. + // NEAR AI private endpoints and Anthropic need a /v1 prefix. + let effective_base = if (req.adapter == "nearai" && base.contains("private")) + || (req.adapter == "anthropic" && !base.ends_with("/v1") && !base.contains("/v1/")) + { + format!("{base}/v1") + } else { + base.to_string() + }; let url = format!("{effective_base}/models"); let mut builder = client.get(&url); if req.adapter == "anthropic" { @@ -2804,27 +2838,25 @@ async fn fetch_provider_models(req: ListModelsRequest) -> ListModelsResponse { /// The frontend uses these as fallback values when the DB has no overrides. /// API keys are never returned — only a boolean `has_api_key`. async fn llm_env_defaults_handler() -> Json { + use crate::config::helpers::optional_env; use crate::llm::registry::ProviderRegistry; let registry = ProviderRegistry::load(); let mut defaults = serde_json::Map::new(); + // Helper: read env var via optional_env (checks real env + injected overlay). + let read_env = |key: &str| -> Option { optional_env(key).ok().flatten() }; + // NEAR AI is a special case (not in the registry) { let mut entry = serde_json::Map::new(); - if let Ok(key) = std::env::var("NEARAI_API_KEY") - && !key.is_empty() - { + if let Some(key) = read_env("NEARAI_API_KEY") { entry.insert("api_key".to_string(), serde_json::Value::String(key)); } - if let Ok(model) = std::env::var("NEARAI_MODEL") - && !model.is_empty() - { + if let Some(model) = read_env("NEARAI_MODEL") { entry.insert("model".to_string(), serde_json::Value::String(model)); } - if let Ok(url) = std::env::var("NEARAI_BASE_URL") - && !url.is_empty() - { + if let Some(url) = read_env("NEARAI_BASE_URL") { entry.insert("base_url".to_string(), serde_json::Value::String(url)); } defaults.insert("nearai".to_string(), serde_json::Value::Object(entry)); @@ -2835,21 +2867,17 @@ async fn llm_env_defaults_handler() -> Json { let mut entry = serde_json::Map::new(); if let Some(ref api_key_env) = def.api_key_env - && let Ok(key) = std::env::var(api_key_env) - && !key.is_empty() + && let Some(key) = read_env(api_key_env) { entry.insert("api_key".to_string(), serde_json::Value::String(key)); } - if let Ok(model) = std::env::var(&def.model_env) - && !model.is_empty() - { + if let Some(model) = read_env(&def.model_env) { entry.insert("model".to_string(), serde_json::Value::String(model)); } if let Some(ref base_url_env) = def.base_url_env - && let Ok(url) = std::env::var(base_url_env) - && !url.is_empty() + && let Some(url) = read_env(base_url_env) { entry.insert("base_url".to_string(), serde_json::Value::String(url)); } diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 46164e7c..ed60c2c9 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -6318,7 +6318,8 @@ function renderProviders() { ? '' : ''; const envDef = _envDefaults[p.id] || {}; - const effectiveBaseUrl = envDef.base_url || p.base_url; + const overrideBaseUrl = p.builtin && _builtinOverrides[p.id] ? (_builtinOverrides[p.id].base_url || '') : ''; + const effectiveBaseUrl = overrideBaseUrl || envDef.base_url || p.base_url; const baseUrlText = effectiveBaseUrl ? '' + escapeHtml(effectiveBaseUrl) + '' : ''; @@ -6421,25 +6422,22 @@ function configureBuiltinProvider(id) { const titleEl = document.getElementById('provider-form-title'); titleEl.textContent = I18n.t('config.configureProvider') + ': ' + (p.name || id); titleEl.removeAttribute('data-i18n'); - // Hide name/id/adapter rows; show base-url as read-only for reference + // Hide name/id/adapter rows; show base-url as editable document.getElementById('provider-name-row').style.display = 'none'; document.getElementById('provider-id-row').style.display = 'none'; document.getElementById('provider-adapter-row').style.display = 'none'; const baseUrlInput = document.getElementById('provider-base-url'); - const envBaseUrl = (_envDefaults[id] || {}).base_url; - const effectiveBaseUrl = envBaseUrl || p.base_url; - if (effectiveBaseUrl) { - document.getElementById('provider-base-url-row').style.display = ''; - baseUrlInput.value = effectiveBaseUrl; - baseUrlInput.readOnly = true; - baseUrlInput.style.opacity = '0.6'; - } else { - document.getElementById('provider-base-url-row').style.display = 'none'; - } - document.getElementById('provider-api-key-row').style.display = p.api_key_required !== false ? '' : 'none'; - document.getElementById('fetch-models-btn').style.display = p.can_list_models ? '' : 'none'; const override = _builtinOverrides[id] || {}; const envDef = _envDefaults[id] || {}; + // Priority: db override > env > hardcoded default + const effectiveBaseUrl = override.base_url || envDef.base_url || p.base_url; + document.getElementById('provider-base-url-row').style.display = ''; + baseUrlInput.value = effectiveBaseUrl || ''; + baseUrlInput.readOnly = false; + baseUrlInput.style.opacity = ''; + baseUrlInput.placeholder = p.base_url || ''; + document.getElementById('provider-api-key-row').style.display = p.api_key_required !== false ? '' : 'none'; + document.getElementById('fetch-models-btn').style.display = p.can_list_models ? '' : 'none'; const apiKeyInput = document.getElementById('provider-api-key'); apiKeyInput.value = override.api_key || envDef.api_key || ''; apiKeyInput.placeholder = ''; @@ -6487,12 +6485,13 @@ document.getElementById('test-provider-btn').addEventListener('click', () => { const apiKey = document.getElementById('provider-api-key').value.trim(); const model = document.getElementById('provider-model').value.trim(); - // For built-in providers, use the hardcoded adapter/base_url from BUILTIN_PROVIDERS + // For built-in providers, use the hardcoded adapter from BUILTIN_PROVIDERS. + // base_url comes from the form which already reflects: env > hardcoded default. if (_configuringBuiltinId) { const p = BUILTIN_PROVIDERS.find((x) => x.id === _configuringBuiltinId); if (p) { adapter = p.adapter; - baseUrl = p.base_url; + if (!baseUrl) baseUrl = p.base_url; } } @@ -6529,10 +6528,12 @@ document.getElementById('save-provider-btn').addEventListener('click', () => { if (_configuringBuiltinId) { const apiKey = document.getElementById('provider-api-key').value.trim(); const model = document.getElementById('provider-model').value.trim(); + const baseUrl = document.getElementById('provider-base-url').value.trim(); const id = _configuringBuiltinId; const override = {}; if (apiKey) override.api_key = apiKey; if (model) override.model = model; + if (baseUrl) override.base_url = baseUrl; const prev = _builtinOverrides[id]; _builtinOverrides[id] = override; const isActive = id === _activeLlmBackend; @@ -6664,12 +6665,13 @@ document.getElementById('fetch-models-btn').addEventListener('click', () => { let baseUrl = document.getElementById('provider-base-url').value.trim(); const apiKey = document.getElementById('provider-api-key').value.trim(); - // For built-in providers, use the hardcoded base_url and adapter from BUILTIN_PROVIDERS + // For built-in providers, use the hardcoded adapter from BUILTIN_PROVIDERS. + // base_url comes from the form which already reflects: env > hardcoded default. if (_configuringBuiltinId) { const p = BUILTIN_PROVIDERS.find((x) => x.id === _configuringBuiltinId); if (p) { adapter = p.adapter; - baseUrl = p.base_url; + if (!baseUrl) baseUrl = p.base_url; } } diff --git a/src/config/llm.rs b/src/config/llm.rs index 125907ce..efbeaf08 100644 --- a/src/config/llm.rs +++ b/src/config/llm.rs @@ -46,14 +46,16 @@ impl LlmConfig { } } - /// Resolve a model name from env var -> settings.selected_model -> hardcoded default. + /// Resolve a model name from settings.selected_model -> env var -> hardcoded default. fn resolve_model( env_var: &str, settings: &Settings, default: &str, ) -> Result { - Ok(optional_env(env_var)? - .or_else(|| settings.selected_model.clone()) + Ok(settings + .selected_model + .clone() + .or_else(|| optional_env(env_var).ok().flatten()) .unwrap_or_else(|| default.to_string())) } @@ -129,18 +131,31 @@ impl LlmConfig { }; // Always resolve NEAR AI config (used for embeddings even when not the primary backend) - let nearai_api_key = optional_env("NEARAI_API_KEY")?.map(SecretString::from); + // Priority: DB (builtin_overrides) > env > default + let nearai_override = settings.llm_builtin_overrides.get("nearai"); + let nearai_api_key = nearai_override + .and_then(|o| o.api_key.as_ref()) + .map(|k| SecretString::from(k.clone())) + .or_else(|| { + optional_env("NEARAI_API_KEY") + .ok() + .flatten() + .map(SecretString::from) + }); let nearai = NearAiConfig { model: Self::resolve_model("NEARAI_MODEL", settings, crate::llm::DEFAULT_MODEL)?, cheap_model: optional_env("NEARAI_CHEAP_MODEL")?, base_url: { - let url = optional_env("NEARAI_BASE_URL")?.unwrap_or_else(|| { - if nearai_api_key.is_some() { - "https://cloud-api.near.ai".to_string() - } else { - "https://private.near.ai".to_string() - } - }); + let url = nearai_override + .and_then(|o| o.base_url.clone()) + .or_else(|| optional_env("NEARAI_BASE_URL").ok().flatten()) + .unwrap_or_else(|| { + if nearai_api_key.is_some() { + "https://cloud-api.near.ai".to_string() + } else { + "https://private.near.ai".to_string() + } + }); validate_base_url(&url, "NEARAI_BASE_URL")?; url }, @@ -177,20 +192,27 @@ impl LlmConfig { }; let bedrock = if is_bedrock { - let explicit_region = - optional_env("BEDROCK_REGION")?.or_else(|| settings.bedrock_region.clone()); + let explicit_region = settings + .bedrock_region + .clone() + .or_else(|| optional_env("BEDROCK_REGION").ok().flatten()); if explicit_region.is_none() { tracing::info!("BEDROCK_REGION not set, defaulting to us-east-1"); } let region = explicit_region.unwrap_or_else(|| "us-east-1".to_string()); - let model = optional_env("BEDROCK_MODEL")? - .or_else(|| settings.selected_model.clone()) + let model = settings + .selected_model + .clone() + .or_else(|| optional_env("BEDROCK_MODEL").ok().flatten()) .ok_or_else(|| ConfigError::MissingRequired { key: "BEDROCK_MODEL".to_string(), - hint: "Set BEDROCK_MODEL when LLM_BACKEND=bedrock".to_string(), + hint: "Set BEDROCK_MODEL or selected_model when LLM_BACKEND=bedrock" + .to_string(), })?; - let cross_region = optional_env("BEDROCK_CROSS_REGION")? - .or_else(|| settings.bedrock_cross_region.clone()); + let cross_region = settings + .bedrock_cross_region + .clone() + .or_else(|| optional_env("BEDROCK_CROSS_REGION").ok().flatten()); if let Some(ref cr) = cross_region && !matches!(cr.as_str(), "us" | "eu" | "apac" | "global") { @@ -202,7 +224,10 @@ impl LlmConfig { ), }); } - let profile = optional_env("AWS_PROFILE")?.or_else(|| settings.bedrock_profile.clone()); + let profile = settings + .bedrock_profile + .clone() + .or_else(|| optional_env("AWS_PROFILE").ok().flatten()); Some(BedrockConfig { region, model, @@ -215,10 +240,12 @@ impl LlmConfig { // Resolve OpenAI Codex config let openai_codex = if is_openai_codex { - // Model: OPENAI_CODEX_MODEL > OPENAI_MODEL > settings.selected_model > default - let model = optional_env("OPENAI_CODEX_MODEL")? - .or(optional_env("OPENAI_MODEL")?) - .or_else(|| settings.selected_model.clone()) + // Model: settings.selected_model > OPENAI_CODEX_MODEL > OPENAI_MODEL > default + let model = settings + .selected_model + .clone() + .or_else(|| optional_env("OPENAI_CODEX_MODEL").ok().flatten()) + .or_else(|| optional_env("OPENAI_MODEL").ok().flatten()) .unwrap_or_else(|| "gpt-5.3-codex".to_string()); let auth_endpoint = optional_env("OPENAI_CODEX_AUTH_URL")? .unwrap_or_else(|| "https://auth.openai.com".to_string()); @@ -322,8 +349,10 @@ impl LlmConfig { tracing::warn!(id = %custom.id, "Custom provider has no base_url configured — requests will fail"); } - let model = optional_env("LLM_MODEL")? - .or_else(|| settings.selected_model.clone()) + let model = settings + .selected_model + .clone() + .or_else(|| optional_env("LLM_MODEL").ok().flatten()) .or_else(|| custom.default_model.clone()) .unwrap_or_default(); if model.is_empty() { @@ -423,14 +452,13 @@ impl LlmConfig { } Some(creds.token) } else if let Some(env_var) = api_key_env { - // Resolve API key: env var (including secrets store overlay) > settings override - optional_env(env_var)?.map(SecretString::from).or_else(|| { - settings - .llm_builtin_overrides - .get(backend) - .and_then(|o| o.api_key.as_ref()) - .map(|k| SecretString::from(k.clone())) - }) + // Resolve API key: settings override (DB) > env var (including secrets store overlay) + settings + .llm_builtin_overrides + .get(backend) + .and_then(|o| o.api_key.as_ref()) + .map(|k| SecretString::from(k.clone())) + .or_else(|| optional_env(env_var).ok().flatten().map(SecretString::from)) } else { None }; @@ -446,18 +474,18 @@ impl LlmConfig { } } - // Resolve base URL: codex override > env var > settings (backward compat) > registry default + // Resolve base URL: codex override > builtin_overrides (DB) > legacy settings (DB) > env var > registry default let is_codex_chatgpt = codex_base_url_override.is_some(); let base_url = codex_base_url_override .or_else(|| { - if let Some(env_var) = base_url_env { - optional_env(env_var).ok().flatten() - } else { - None - } + // DB settings: per-provider base_url override + settings + .llm_builtin_overrides + .get(backend) + .and_then(|o| o.base_url.clone()) }) .or_else(|| { - // Backward compat: check legacy settings fields + // DB settings: legacy settings fields match backend { "ollama" => settings.ollama_base_url.clone(), "openai_compatible" | "openrouter" => { @@ -466,6 +494,13 @@ impl LlmConfig { _ => None, } }) + .or_else(|| { + if let Some(env_var) = base_url_env { + optional_env(env_var).ok().flatten() + } else { + None + } + }) .or_else(|| default_base_url.map(String::from)) .unwrap_or_default(); @@ -485,15 +520,17 @@ impl LlmConfig { validate_base_url(&base_url, field)?; } - // Resolve model: env var > selected_model (/model command) > per-provider override > registry default - let model = optional_env(model_env)? - .or_else(|| settings.selected_model.clone()) + // Resolve model: selected_model (DB) > per-provider override (DB) > env var > registry default + let model = settings + .selected_model + .clone() .or_else(|| { settings .llm_builtin_overrides .get(backend) .and_then(|o| o.model.clone()) }) + .or_else(|| optional_env(model_env).ok().flatten()) .unwrap_or_else(|| default_model.to_string()); // Resolve extra headers @@ -658,7 +695,7 @@ mod tests { } #[test] - fn openai_compatible_llm_model_env_overrides_selected_model() { + fn openai_compatible_selected_model_overrides_env() { let _guard = lock_env(); clear_openai_compatible_env(); // SAFETY: Under ENV_MUTEX. @@ -676,7 +713,10 @@ mod tests { let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); let provider = cfg.provider.expect("provider config should be present"); - assert_eq!(provider.model, "openai/gpt-5-codex"); + assert_eq!( + provider.model, "openai/gpt-5.1-codex", + "DB selected_model should take priority over LLM_MODEL env var" + ); // SAFETY: Under ENV_MUTEX. unsafe { @@ -799,7 +839,7 @@ mod tests { } #[test] - fn ollama_model_env_overrides_selected_model() { + fn ollama_selected_model_overrides_env() { let _guard = lock_env(); clear_ollama_env(); // SAFETY: Under ENV_MUTEX. @@ -816,7 +856,10 @@ mod tests { let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); let provider = cfg.provider.expect("provider config should be present"); - assert_eq!(provider.model, "mistral:latest"); + assert_eq!( + provider.model, "llama3.2", + "DB selected_model should take priority over OLLAMA_MODEL env var" + ); // SAFETY: Under ENV_MUTEX. unsafe { @@ -1079,28 +1122,31 @@ mod tests { ..Default::default() }; - let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); - let provider = cfg.provider.expect("should have provider config"); - assert_eq!( - provider.base_url, "http://localhost:8000/v1", - "env var should take priority over settings" - ); - - // Now without env var, settings should win over registry default - unsafe { - std::env::remove_var("LLM_BASE_URL"); - } - + // DB settings should take priority over env var let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); let provider = cfg.provider.expect("should have provider config"); assert_eq!( provider.base_url, "http://localhost:9000/v1", - "settings should take priority over registry default" + "DB settings should take priority over env var" + ); + + // Without DB settings, env var should win over registry default + let settings_no_base = Settings { + llm_backend: Some("openai_compatible".to_string()), + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings_no_base).expect("resolve should succeed"); + let provider = cfg.provider.expect("should have provider config"); + assert_eq!( + provider.base_url, "http://localhost:8000/v1", + "env var should take priority over registry default when DB has no base_url" ); // SAFETY: Under ENV_MUTEX. unsafe { std::env::remove_var("LLM_BACKEND"); + std::env::remove_var("LLM_BASE_URL"); } } @@ -1427,6 +1473,7 @@ mod tests { crate::settings::LlmBuiltinOverride { api_key: None, model: Some("llama-3.1-8b-instant".to_string()), + base_url: None, }, ); let settings = Settings { @@ -1478,6 +1525,7 @@ mod tests { crate::settings::LlmBuiltinOverride { api_key: None, model: Some("llama-3.1-8b-instant".to_string()), + base_url: None, }, ); let settings = Settings { @@ -1535,6 +1583,7 @@ mod tests { crate::settings::LlmBuiltinOverride { api_key: Some("gsk_test_key".to_string()), model: Some("llama-3.3-70b-versatile".to_string()), + base_url: None, }, ); let settings = Settings { @@ -1654,4 +1703,367 @@ mod tests { std::env::remove_var("OPENAI_CODEX_AUTH_URL"); } } + + // ── DB > ENV priority tests ───────────────────────────────────── + + #[test] + fn builtin_override_api_key_wins_over_env_var() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("GROQ_API_KEY", "gsk_from_env"); + std::env::remove_var("GROQ_MODEL"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "groq".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: Some("gsk_from_db".to_string()), + model: None, + base_url: None, + }, + ); + let settings = Settings { + llm_backend: Some("groq".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + let provider = cfg.provider.expect("provider config should be present"); + use secrecy::ExposeSecret as _; + assert_eq!( + provider + .api_key + .as_ref() + .map(|k| k.expose_secret().to_string()), + Some("gsk_from_db".to_string()), + "DB builtin_override api_key must take priority over GROQ_API_KEY env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("GROQ_API_KEY"); + } + } + + #[test] + fn builtin_override_model_wins_over_env_var() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("GROQ_MODEL", "model-from-env"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "groq".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: None, + model: Some("model-from-db".to_string()), + base_url: None, + }, + ); + let settings = Settings { + llm_backend: Some("groq".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + let provider = cfg.provider.expect("provider config should be present"); + assert_eq!( + provider.model, "model-from-db", + "DB builtin_override model must take priority over GROQ_MODEL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("GROQ_MODEL"); + } + } + + #[test] + fn custom_provider_selected_model_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("LLM_MODEL", "model-from-env"); + } + + let settings = Settings { + llm_backend: Some("myprovider".to_string()), + selected_model: Some("model-from-db".to_string()), + llm_custom_providers: vec![crate::settings::CustomLlmProviderSettings { + id: "myprovider".to_string(), + name: "My Provider".to_string(), + adapter: "open_ai_completions".to_string(), + base_url: Some("https://api.example.com/v1".to_string()), + default_model: Some("default-model".to_string()), + api_key: None, + builtin: false, + }], + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + let provider = cfg.provider.expect("provider config should be present"); + assert_eq!( + provider.model, "model-from-db", + "DB selected_model must take priority over LLM_MODEL env var for custom providers" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_MODEL"); + } + } + + #[test] + fn openai_codex_selected_model_wins_over_env() { + let _guard = lock_env(); + clear_openai_codex_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::set_var("OPENAI_CODEX_MODEL", "codex-from-env"); + } + + let settings = Settings { + llm_backend: Some("openai_codex".to_string()), + selected_model: Some("codex-from-db".to_string()), + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + let codex = cfg.openai_codex.expect("codex config should be present"); + assert_eq!( + codex.model, "codex-from-db", + "DB selected_model must take priority over OPENAI_CODEX_MODEL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("OPENAI_CODEX_MODEL"); + } + } + + #[test] + fn nearai_selected_model_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("NEARAI_MODEL", "nearai-from-env"); + } + + let settings = Settings { + llm_backend: Some("nearai".to_string()), + selected_model: Some("nearai-from-db".to_string()), + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.model, "nearai-from-db", + "DB selected_model must take priority over NEARAI_MODEL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("NEARAI_MODEL"); + } + } + + #[test] + fn nearai_override_base_url_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("NEARAI_BASE_URL", "http://localhost:9001"); + std::env::remove_var("NEARAI_API_KEY"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "nearai".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: None, + model: None, + base_url: Some("http://localhost:9002".to_string()), + }, + ); + let settings = Settings { + llm_backend: Some("nearai".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.base_url, "http://localhost:9002", + "DB builtin_overrides base_url must take priority over NEARAI_BASE_URL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("NEARAI_BASE_URL"); + } + } + + #[test] + fn nearai_env_base_url_used_when_no_override() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("NEARAI_BASE_URL", "http://localhost:9001"); + std::env::remove_var("NEARAI_API_KEY"); + } + + let settings = Settings { + llm_backend: Some("nearai".to_string()), + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.base_url, "http://localhost:9001", + "NEARAI_BASE_URL env var should be used when no DB override exists" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("NEARAI_BASE_URL"); + } + } + + #[test] + fn nearai_override_api_key_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("NEARAI_API_KEY", "key-from-env"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "nearai".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: Some("key-from-db".to_string()), + model: None, + base_url: None, + }, + ); + let settings = Settings { + llm_backend: Some("nearai".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + use secrecy::ExposeSecret as _; + assert_eq!( + cfg.nearai + .api_key + .as_ref() + .map(|k| k.expose_secret().to_string()), + Some("key-from-db".to_string()), + "DB builtin_overrides api_key must take priority over NEARAI_API_KEY env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("NEARAI_API_KEY"); + } + } + + #[test] + fn nearai_base_url_auto_selects_when_no_override_or_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::remove_var("NEARAI_BASE_URL"); + std::env::remove_var("NEARAI_API_KEY"); + } + + // No API key → should default to private.near.ai + let settings = Settings { + llm_backend: Some("nearai".to_string()), + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.base_url, "https://private.near.ai", + "Without API key, should default to private.near.ai" + ); + + // With API key → should default to cloud-api.near.ai + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "nearai".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: Some("some-key".to_string()), + model: None, + base_url: None, + }, + ); + let settings_with_key = Settings { + llm_backend: Some("nearai".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings_with_key).expect("resolve should succeed"); + assert_eq!( + cfg.nearai.base_url, "https://cloud-api.near.ai", + "With API key, should default to cloud-api.near.ai" + ); + } + + #[test] + fn registry_provider_override_base_url_wins_over_env() { + let _guard = lock_env(); + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("LLM_BACKEND"); + std::env::set_var("GROQ_BASE_URL", "http://localhost:9003"); + std::env::remove_var("GROQ_API_KEY"); + std::env::remove_var("GROQ_MODEL"); + } + + let mut overrides = std::collections::HashMap::new(); + overrides.insert( + "groq".to_string(), + crate::settings::LlmBuiltinOverride { + api_key: None, + model: None, + base_url: Some("http://localhost:9004".to_string()), + }, + ); + let settings = Settings { + llm_backend: Some("groq".to_string()), + llm_builtin_overrides: overrides, + ..Default::default() + }; + + let cfg = LlmConfig::resolve(&settings).expect("resolve should succeed"); + let provider = cfg.provider.expect("provider config should be present"); + assert_eq!( + provider.base_url, "http://localhost:9004", + "DB builtin_overrides base_url must take priority over GROQ_BASE_URL env var" + ); + + // SAFETY: Under ENV_MUTEX. + unsafe { + std::env::remove_var("GROQ_BASE_URL"); + } + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 68b23ab2..96086743 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,6 +1,6 @@ //! Configuration for IronClaw. //! -//! Settings are loaded with priority: env var > database > default. +//! Settings are loaded with priority: database > env var > default. //! `DATABASE_URL` lives in `~/.ironclaw/.env` (loaded via dotenvy early //! in startup). Everything else comes from env vars, the DB settings //! table, or auto-detection. @@ -188,7 +188,7 @@ impl Config { /// Load configuration from environment variables and the database. /// - /// Priority: env var > TOML config file > DB settings > default. + /// Priority: DB settings > env var > TOML config file > default. /// This is the primary way to load config after DB is connected. pub async fn from_db( store: &(dyn crate::db::SettingsStore + Sync), @@ -198,6 +198,10 @@ impl Config { } /// Load from DB with an optional TOML config file overlay. + /// + /// Priority: DB settings > env var > TOML config file > default. + /// TOML is loaded first as a base, then DB values are merged on top + /// so that DB always wins over TOML. pub async fn from_db_with_toml( store: &(dyn crate::db::SettingsStore + Sync), user_id: &str, @@ -206,19 +210,22 @@ impl Config { let _ = dotenvy::dotenv(); crate::bootstrap::load_ironclaw_env(); - // Load all settings from DB into a Settings struct - let mut db_settings = match store.get_all_settings(user_id).await { - Ok(map) => Settings::from_db_map(&map), + // Start with TOML config as a base (lowest priority among the two). + let mut settings = Settings::default(); + Self::apply_toml_overlay(&mut settings, toml_path)?; + + // Overlay DB settings on top so DB values win over TOML. + match store.get_all_settings(user_id).await { + Ok(map) => { + let db_settings = Settings::from_db_map(&map); + settings.merge_from(&db_settings); + } Err(e) => { tracing::warn!("Failed to load settings from DB, using defaults: {}", e); - Settings::default() } }; - // Overlay TOML config file (values win over DB settings) - Self::apply_toml_overlay(&mut db_settings, toml_path)?; - - Self::build(&db_settings).await + Self::build(&settings).await } /// Load configuration from environment variables only (no database). @@ -294,11 +301,13 @@ impl Config { toml_path: Option<&std::path::Path>, ) -> Result<(), ConfigError> { let settings = if let Some(store) = store { - let mut s = match store.get_all_settings(user_id).await { - Ok(map) => Settings::from_db_map(&map), - Err(_) => Settings::default(), - }; + // TOML as base, then DB on top (DB wins). + let mut s = Settings::default(); Self::apply_toml_overlay(&mut s, toml_path)?; + if let Ok(map) = store.get_all_settings(user_id).await { + let db_settings = Settings::from_db_map(&map); + s.merge_from(&db_settings); + } s } else { Settings::default() diff --git a/src/settings.rs b/src/settings.rs index e79b08d5..2f683b13 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -41,16 +41,19 @@ pub struct CustomLlmProviderSettings { /// Stored as `llm_builtin_overrides` in the settings store, keyed by provider ID /// (e.g. `"openai"`, `"gemini"`). Resolved at startup during `LlmConfig::resolve()`. /// -/// Note: Environment variables and the global `selected_model` (if set) take -/// precedence over these per-provider overrides. +/// Note: The global `selected_model` (if set) takes precedence over these +/// per-provider overrides, which in turn take precedence over environment variables. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct LlmBuiltinOverride { - /// API key override used when no API key is provided via environment variables. + /// API key override. Takes precedence over environment variables. #[serde(default, skip_serializing_if = "Option::is_none")] pub api_key: Option, - /// Default model override used when no global `selected_model` is configured. + /// Model override. Takes precedence over environment variables but not `selected_model`. #[serde(default, skip_serializing_if = "Option::is_none")] pub model: Option, + /// Base URL override. Takes precedence over environment variables. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub base_url: Option, } /// User settings persisted to disk. @@ -2327,4 +2330,65 @@ mod tests { assert_eq!(current.embeddings.provider, "nearai"); assert_eq!(current.embeddings.model, "text-embedding-3-large"); } + + /// DB values must win over TOML values when both set the same field. + /// + /// This mirrors the merge order in `Config::from_db_with_toml`: + /// TOML is loaded as the base, then DB is merged on top. + #[test] + fn db_settings_win_over_toml_settings() { + // Simulate TOML base: has llm_backend and selected_model + let mut base = Settings { + llm_backend: Some("openai".to_string()), + selected_model: Some("toml-model".to_string()), + ..Default::default() + }; + + // Simulate DB overlay: has different llm_backend and selected_model + let db = Settings { + llm_backend: Some("anthropic".to_string()), + selected_model: Some("db-model".to_string()), + ..Default::default() + }; + + // Merge DB on top of TOML (same order as from_db_with_toml) + base.merge_from(&db); + + assert_eq!( + base.llm_backend.as_deref(), + Some("anthropic"), + "DB llm_backend must win over TOML" + ); + assert_eq!( + base.selected_model.as_deref(), + Some("db-model"), + "DB selected_model must win over TOML" + ); + } + + /// When DB has no value (default), TOML value should be preserved. + #[test] + fn toml_settings_used_when_db_has_no_value() { + let mut base = Settings { + llm_backend: Some("openai".to_string()), + selected_model: Some("toml-model".to_string()), + ..Default::default() + }; + + // DB has no llm_backend or selected_model (both default/None) + let db = Settings::default(); + + base.merge_from(&db); + + assert_eq!( + base.llm_backend.as_deref(), + Some("openai"), + "TOML llm_backend should be preserved when DB has no value" + ); + assert_eq!( + base.selected_model.as_deref(), + Some("toml-model"), + "TOML selected_model should be preserved when DB has no value" + ); + } }