From 99c25d130bda05140927eb2a19991c8e65666043 Mon Sep 17 00:00:00 2001 From: allen-munsch Date: Wed, 20 May 2026 17:43:17 -0500 Subject: [PATCH 1/2] feat: auto-detect provider from API key env vars When a user sets an API key env var (e.g. DEEPSEEK_API_KEY) but doesn't explicitly configure a provider, dirge now auto-detects the matching provider instead of falling back to openrouter. Resolution order: CLI flag > config file > env var detection > openrouter Supports: DEEPSEEK_API_KEY, OPENAI_API_KEY, ANTHROPIC_API_KEY, GEMINI_API_KEY, GLM_API_KEY, OLLAMA_API_KEY, OPENROUTER_API_KEY Signed-off-by: allen-munsch --- src/cli.rs | 1 + src/provider.rs | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/cli.rs b/src/cli.rs index 6be70a42..e9acae2f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -153,6 +153,7 @@ impl Cli { .as_deref() .or(cfg.provider.as_deref()) .map(CompactString::new) + .or_else(|| crate::provider::auto_detect_provider().map(CompactString::new)) .unwrap_or_else(|| CompactString::new("openrouter")) } diff --git a/src/provider.rs b/src/provider.rs index b1c5fa50..958d45a9 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -145,6 +145,29 @@ fn provider_env_var(kind: ProviderKind) -> &'static str { } } +/// Auto-detect provider from environment variables when none is explicitly configured. +/// Returns the provider name string (e.g. "deepseek") for the first matching env var. +pub fn auto_detect_provider() -> Option<&'static str> { + let candidates: &[(&str, &str)] = &[ + ("DEEPSEEK_API_KEY", "deepseek"), + ("OPENAI_API_KEY", "openai"), + ("ANTHROPIC_API_KEY", "anthropic"), + ("GEMINI_API_KEY", "gemini"), + ("GLM_API_KEY", "glm"), + ("OLLAMA_API_KEY", "ollama"), + ("OPENROUTER_API_KEY", "openrouter"), + ]; + for (env_var, provider_name) in candidates { + if std::env::var(env_var) + .map(|v| !v.is_empty()) + .unwrap_or(false) + { + return Some(provider_name); + } + } + None +} + fn resolve_api_key( kind: ProviderKind, api_key_env_override: Option<&str>, @@ -531,3 +554,58 @@ pub async fn build_agent( AnyModel::Custom(m) => build_inner!(m, Custom), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn clean_env() { + for var in &[ + "DEEPSEEK_API_KEY", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", + "GLM_API_KEY", + "OLLAMA_API_KEY", + "OPENROUTER_API_KEY", + ] { + unsafe { std::env::remove_var(var) }; + } + } + + #[test] + fn auto_detect_returns_none_when_no_vars_set() { + clean_env(); + assert_eq!(auto_detect_provider(), None); + } + + #[test] + fn auto_detect_finds_deepseek_when_key_set() { + clean_env(); + unsafe { std::env::set_var("DEEPSEEK_API_KEY", "sk-test-123") }; + assert_eq!(auto_detect_provider(), Some("deepseek")); + } + + #[test] + fn auto_detect_finds_openai_when_key_set() { + clean_env(); + unsafe { std::env::set_var("OPENAI_API_KEY", "sk-test-456") }; + assert_eq!(auto_detect_provider(), Some("openai")); + } + + #[test] + fn auto_detect_skips_empty_var() { + clean_env(); + unsafe { std::env::set_var("DEEPSEEK_API_KEY", "") }; + unsafe { std::env::set_var("OPENAI_API_KEY", "sk-test-789") }; + assert_eq!(auto_detect_provider(), Some("openai")); + } + + #[test] + fn auto_detect_returns_first_match_in_order() { + clean_env(); + unsafe { std::env::set_var("DEEPSEEK_API_KEY", "sk-ds") }; + unsafe { std::env::set_var("OPENAI_API_KEY", "sk-oai") }; + assert_eq!(auto_detect_provider(), Some("deepseek")); + } +} From fb7468a5bbb0395fbaec777c2f4ebc683bccab9f Mon Sep 17 00:00:00 2001 From: Yogthos Date: Thu, 21 May 2026 01:31:55 -0400 Subject: [PATCH 2/2] test: replace env-var mutation with pure function + mock env lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original tests used `std::env::set_var` / `remove_var` which mutate process-wide state. Rust runs tests in parallel by default, so the tests raced under `cargo test` and 2 of 5 failed intermittently. Refactor: extract `auto_detect_provider_from(env_lookup)` as a pure helper. Production `auto_detect_provider()` passes `std::env::var`; tests pass a HashMap-backed closure via `mock_env(&[...])`. No process env mutation, no race. The candidate list moves to a module-level `PROVIDER_AUTODETECT_ORDER` so tests reference the same source of truth — adds an isolation test (`auto_detect_each_provider_in_isolation`) that exercises every entry, guarding against accidental drops or reorders. 6 tests now pass deterministically (was 5 flaky). --- src/provider.rs | 114 +++++++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 44 deletions(-) diff --git a/src/provider.rs b/src/provider.rs index 958d45a9..9140f1bf 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -145,22 +145,42 @@ fn provider_env_var(kind: ProviderKind) -> &'static str { } } -/// Auto-detect provider from environment variables when none is explicitly configured. -/// Returns the provider name string (e.g. "deepseek") for the first matching env var. +/// Auto-detect provider from environment variables when none is +/// explicitly configured. Returns the provider name string +/// (e.g. "deepseek") for the first matching `*_API_KEY` env var +/// with a non-empty value. Returns `None` if no known key is set. +/// +/// Resolution order is fixed (see `PROVIDER_AUTODETECT_ORDER`). +/// When multiple keys are present, the FIRST in that list wins so +/// the behavior is deterministic — important for users who have +/// several keys in their shell environment. pub fn auto_detect_provider() -> Option<&'static str> { - let candidates: &[(&str, &str)] = &[ - ("DEEPSEEK_API_KEY", "deepseek"), - ("OPENAI_API_KEY", "openai"), - ("ANTHROPIC_API_KEY", "anthropic"), - ("GEMINI_API_KEY", "gemini"), - ("GLM_API_KEY", "glm"), - ("OLLAMA_API_KEY", "ollama"), - ("OPENROUTER_API_KEY", "openrouter"), - ]; - for (env_var, provider_name) in candidates { - if std::env::var(env_var) - .map(|v| !v.is_empty()) - .unwrap_or(false) + auto_detect_provider_from(|name| std::env::var(name).ok()) +} + +/// Provider candidate list for autodetect. Listed in priority +/// order — first key with a non-empty value wins. Extracted as a +/// module item so tests reference the same source of truth and +/// adding a provider only touches one place. +const PROVIDER_AUTODETECT_ORDER: &[(&str, &str)] = &[ + ("DEEPSEEK_API_KEY", "deepseek"), + ("OPENAI_API_KEY", "openai"), + ("ANTHROPIC_API_KEY", "anthropic"), + ("GEMINI_API_KEY", "gemini"), + ("GLM_API_KEY", "glm"), + ("OLLAMA_API_KEY", "ollama"), + ("OPENROUTER_API_KEY", "openrouter"), +]; + +/// Pure helper that drives `auto_detect_provider` from a +/// caller-supplied env lookup. Production calls +/// `auto_detect_provider()` which passes `std::env::var`; tests +/// pass a closure backed by a HashMap so they don't mutate +/// process-wide env vars (which races under parallel `cargo test`). +fn auto_detect_provider_from Option>(env: F) -> Option<&'static str> { + for (env_var, provider_name) in PROVIDER_AUTODETECT_ORDER { + if let Some(v) = env(env_var) + && !v.is_empty() { return Some(provider_name); } @@ -558,54 +578,60 @@ pub async fn build_agent( #[cfg(test)] mod tests { use super::*; - - fn clean_env() { - for var in &[ - "DEEPSEEK_API_KEY", - "OPENAI_API_KEY", - "ANTHROPIC_API_KEY", - "GEMINI_API_KEY", - "GLM_API_KEY", - "OLLAMA_API_KEY", - "OPENROUTER_API_KEY", - ] { - unsafe { std::env::remove_var(var) }; - } + use std::collections::HashMap; + + /// Build an env-lookup closure backed by a HashMap. Avoids + /// mutating process-wide env vars — `std::env::set_var` is + /// thread-unsafe and the previous test suite raced under + /// parallel `cargo test`, producing intermittent failures. + fn mock_env(vars: &[(&str, &str)]) -> impl Fn(&str) -> Option + use<> { + let map: HashMap = vars + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(); + move |name: &str| map.get(name).cloned() } #[test] fn auto_detect_returns_none_when_no_vars_set() { - clean_env(); - assert_eq!(auto_detect_provider(), None); + assert_eq!(auto_detect_provider_from(mock_env(&[])), None); } #[test] fn auto_detect_finds_deepseek_when_key_set() { - clean_env(); - unsafe { std::env::set_var("DEEPSEEK_API_KEY", "sk-test-123") }; - assert_eq!(auto_detect_provider(), Some("deepseek")); + let env = mock_env(&[("DEEPSEEK_API_KEY", "sk-test-123")]); + assert_eq!(auto_detect_provider_from(env), Some("deepseek")); } #[test] fn auto_detect_finds_openai_when_key_set() { - clean_env(); - unsafe { std::env::set_var("OPENAI_API_KEY", "sk-test-456") }; - assert_eq!(auto_detect_provider(), Some("openai")); + let env = mock_env(&[("OPENAI_API_KEY", "sk-test-456")]); + assert_eq!(auto_detect_provider_from(env), Some("openai")); } #[test] fn auto_detect_skips_empty_var() { - clean_env(); - unsafe { std::env::set_var("DEEPSEEK_API_KEY", "") }; - unsafe { std::env::set_var("OPENAI_API_KEY", "sk-test-789") }; - assert_eq!(auto_detect_provider(), Some("openai")); + let env = mock_env(&[("DEEPSEEK_API_KEY", ""), ("OPENAI_API_KEY", "sk-test-789")]); + assert_eq!(auto_detect_provider_from(env), Some("openai")); } #[test] fn auto_detect_returns_first_match_in_order() { - clean_env(); - unsafe { std::env::set_var("DEEPSEEK_API_KEY", "sk-ds") }; - unsafe { std::env::set_var("OPENAI_API_KEY", "sk-oai") }; - assert_eq!(auto_detect_provider(), Some("deepseek")); + let env = mock_env(&[("DEEPSEEK_API_KEY", "sk-ds"), ("OPENAI_API_KEY", "sk-oai")]); + assert_eq!(auto_detect_provider_from(env), Some("deepseek")); + } + + /// Cover every provider in the autodetect list — guards + /// against accidentally dropping or reordering an entry. + #[test] + fn auto_detect_each_provider_in_isolation() { + for &(env_var, expected) in PROVIDER_AUTODETECT_ORDER { + let env = mock_env(&[(env_var, "sk-x")]); + assert_eq!( + auto_detect_provider_from(env), + Some(expected), + "env_var={env_var}", + ); + } } }