gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-08-25 14:53:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(llm): enforce db > env > default config priority for provider setting

Browse Source
This commit is contained in:
italic-jinxin
2026-03-24 20:05:34 +08:00
parent fbfe8cb70e
commit f246cf6bf9
5 changed files with 635 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/channels/web/server.rs
+52 -24
View File
@@ -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<serde_json::Value> {
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<String> { 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<serde_json::Value> {
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));
}
src/channels/web/static/app.js
+20 -18
View File
@@ -6318,7 +6318,8 @@ function renderProviders() {
? '<button class="provider-action-btn" data-action="set-active-provider" data-id="' + escapeHtml(p.id) + '">' + I18n.t('config.useProvider') + '</button>'
: '';
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
? '<span class="provider-url">' + escapeHtml(effectiveBaseUrl) + '</span>'
: '';
@@ -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;
}
}
src/config/llm.rs
+472 -60
View File
@@ -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<String, ConfigError> {
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");
}
}
}
src/config/mod.rs
+23 -14
View File
@@ -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()
src/settings.rs
+68 -4
View File
@@ -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<String>,
/// 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<String>,
/// Base URL override. Takes precedence over environment variables.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub base_url: Option<String>,
}
/// 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"
);
}
}