Merge pull request #892 from Hmbown/codex/fix-env-key-auth-runtime
fix(auth): explain env-only key failures in runtime
This commit is contained in:
@@ -25,7 +25,7 @@ use crate::client::DeepSeekClient;
|
||||
use crate::compaction::{
|
||||
CompactionConfig, compact_messages_safe, merge_system_prompts, should_compact,
|
||||
};
|
||||
use crate::config::{Config, DEFAULT_MAX_SUBAGENTS, DEFAULT_TEXT_MODEL};
|
||||
use crate::config::{ApiProvider, Config, DEFAULT_MAX_SUBAGENTS, DEFAULT_TEXT_MODEL};
|
||||
use crate::cycle_manager::{
|
||||
CycleBriefing, CycleConfig, StructuredState, archive_cycle, build_seed_messages,
|
||||
estimate_briefing_tokens, produce_briefing, should_advance_cycle,
|
||||
@@ -294,6 +294,7 @@ pub struct Engine {
|
||||
config: EngineConfig,
|
||||
deepseek_client: Option<DeepSeekClient>,
|
||||
deepseek_client_error: Option<String>,
|
||||
api_key_env_only_recovery: Option<String>,
|
||||
session: Session,
|
||||
subagent_manager: SharedSubAgentManager,
|
||||
shell_manager: SharedShellManager,
|
||||
@@ -346,6 +347,42 @@ impl Engine {
|
||||
}
|
||||
}
|
||||
|
||||
fn env_only_api_key_recovery_hint(api_config: &Config) -> Option<String> {
|
||||
if !crate::config::active_provider_uses_env_only_api_key(api_config) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let provider = api_config.api_provider();
|
||||
let env_var = match provider {
|
||||
ApiProvider::Deepseek | ApiProvider::DeepseekCN => "DEEPSEEK_API_KEY",
|
||||
ApiProvider::NvidiaNim => "NVIDIA_API_KEY/NVIDIA_NIM_API_KEY",
|
||||
ApiProvider::Openrouter => "OPENROUTER_API_KEY",
|
||||
ApiProvider::Novita => "NOVITA_API_KEY",
|
||||
ApiProvider::Fireworks => "FIREWORKS_API_KEY",
|
||||
ApiProvider::Sglang => "SGLANG_API_KEY",
|
||||
ApiProvider::Vllm => "VLLM_API_KEY",
|
||||
};
|
||||
|
||||
Some(format!(
|
||||
"The rejected key came from {env_var}; no saved config key is present.\n\
|
||||
Run `deepseek auth set --provider {provider}` to save a valid key in ~/.deepseek/config.toml, \
|
||||
or remove the stale export and open a fresh shell.",
|
||||
provider = provider.as_str()
|
||||
))
|
||||
}
|
||||
|
||||
pub(super) fn decorate_auth_error_message(&self, message: String) -> String {
|
||||
let Some(hint) = self.api_key_env_only_recovery.as_ref() else {
|
||||
return message;
|
||||
};
|
||||
if crate::error_taxonomy::classify_error_message(&message) != ErrorCategory::Authentication
|
||||
|| message.contains("no saved config key is present")
|
||||
{
|
||||
return message;
|
||||
}
|
||||
format!("{message}\n\n{hint}")
|
||||
}
|
||||
|
||||
/// Create a new engine with the given configuration
|
||||
pub fn new(config: EngineConfig, api_config: &Config) -> (Self, EngineHandle) {
|
||||
let (tx_op, rx_op) = mpsc::channel(32);
|
||||
@@ -362,6 +399,7 @@ impl Engine {
|
||||
Ok(client) => (Some(client), None),
|
||||
Err(err) => (None, Some(err.to_string())),
|
||||
};
|
||||
let api_key_env_only_recovery = Self::env_only_api_key_recovery_hint(api_config);
|
||||
|
||||
let mut session = Session::new(
|
||||
config.model.clone(),
|
||||
@@ -471,6 +509,7 @@ impl Engine {
|
||||
config,
|
||||
deepseek_client,
|
||||
deepseek_client_error,
|
||||
api_key_env_only_recovery,
|
||||
session,
|
||||
subagent_manager,
|
||||
shell_manager,
|
||||
|
||||
@@ -13,6 +13,8 @@ use tempfile::tempdir;
|
||||
const WORKING_SET_SUMMARY_MARKER: &str = "## Repo Working Set";
|
||||
static CAPACITY_MEMORY_ENV_LOCK: LazyLock<tokio::sync::Mutex<()>> =
|
||||
LazyLock::new(|| tokio::sync::Mutex::new(()));
|
||||
static API_KEY_ENV_LOCK: LazyLock<std::sync::Mutex<()>> =
|
||||
LazyLock::new(|| std::sync::Mutex::new(()));
|
||||
|
||||
struct ScopedCapacityMemoryDir {
|
||||
previous: Option<OsString>,
|
||||
@@ -43,6 +45,35 @@ impl Drop for ScopedCapacityMemoryDir {
|
||||
}
|
||||
}
|
||||
|
||||
struct ScopedDeepSeekApiKey {
|
||||
previous: Option<OsString>,
|
||||
}
|
||||
|
||||
impl ScopedDeepSeekApiKey {
|
||||
fn set(value: &str) -> Self {
|
||||
let previous = std::env::var_os("DEEPSEEK_API_KEY");
|
||||
// Safety: tests using this helper serialize with API_KEY_ENV_LOCK and
|
||||
// restore the original value in Drop.
|
||||
unsafe {
|
||||
std::env::set_var("DEEPSEEK_API_KEY", value);
|
||||
}
|
||||
Self { previous }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for ScopedDeepSeekApiKey {
|
||||
fn drop(&mut self) {
|
||||
// Safety: tests using this helper serialize with API_KEY_ENV_LOCK.
|
||||
unsafe {
|
||||
if let Some(previous) = self.previous.take() {
|
||||
std::env::set_var("DEEPSEEK_API_KEY", previous);
|
||||
} else {
|
||||
std::env::remove_var("DEEPSEEK_API_KEY");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn build_engine_with_capacity(capacity: CapacityControllerConfig) -> Engine {
|
||||
let engine_config = EngineConfig {
|
||||
capacity,
|
||||
@@ -52,6 +83,36 @@ fn build_engine_with_capacity(capacity: CapacityControllerConfig) -> Engine {
|
||||
engine
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn env_only_auth_error_gets_recovery_hint() {
|
||||
let _guard = API_KEY_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _env = ScopedDeepSeekApiKey::set("stale-env-key");
|
||||
let (engine, _handle) = Engine::new(EngineConfig::default(), &Config::default());
|
||||
|
||||
let message =
|
||||
engine.decorate_auth_error_message("Authentication failed: invalid API key".to_string());
|
||||
|
||||
assert!(message.contains("DEEPSEEK_API_KEY"));
|
||||
assert!(message.contains("no saved config key is present"));
|
||||
assert!(message.contains("deepseek auth set --provider deepseek"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_auth_error_does_not_blame_env() {
|
||||
let _guard = API_KEY_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let _env = ScopedDeepSeekApiKey::set("stale-env-key");
|
||||
let cfg = Config {
|
||||
api_key: Some("fresh-config-key".to_string()),
|
||||
..Config::default()
|
||||
};
|
||||
let (engine, _handle) = Engine::new(EngineConfig::default(), &cfg);
|
||||
|
||||
let message =
|
||||
engine.decorate_auth_error_message("Authentication failed: invalid API key".to_string());
|
||||
|
||||
assert_eq!(message, "Authentication failed: invalid API key");
|
||||
}
|
||||
|
||||
fn make_plan(
|
||||
read_only: bool,
|
||||
supports_parallel: bool,
|
||||
|
||||
@@ -270,7 +270,7 @@ impl Engine {
|
||||
s
|
||||
}
|
||||
Err(e) => {
|
||||
let message = e.to_string();
|
||||
let message = self.decorate_auth_error_message(e.to_string());
|
||||
if is_context_length_error_message(&message)
|
||||
&& context_recovery_attempts < MAX_CONTEXT_RECOVERY_ATTEMPTS
|
||||
&& self
|
||||
@@ -410,7 +410,7 @@ impl Engine {
|
||||
}
|
||||
Err(e) => {
|
||||
stream_errors = stream_errors.saturating_add(1);
|
||||
let message = e.to_string();
|
||||
let message = self.decorate_auth_error_message(e.to_string());
|
||||
// #103: when the stream errors before any content was
|
||||
// streamed AND we still have retry budget, transparently
|
||||
// resend the request. DeepSeek has not billed for any
|
||||
@@ -440,7 +440,9 @@ impl Engine {
|
||||
continue;
|
||||
}
|
||||
Err(retry_err) => {
|
||||
let retry_msg = format!("Stream retry failed: {retry_err}");
|
||||
let retry_msg = self.decorate_auth_error_message(format!(
|
||||
"Stream retry failed: {retry_err}"
|
||||
));
|
||||
turn_error.get_or_insert(retry_msg.clone());
|
||||
let _ = self
|
||||
.tx_event
|
||||
|
||||
Reference in New Issue
Block a user