fix(executor): auto-fallback to API key when OAuth capacity exhausted#537
fix(executor): auto-fallback to API key when OAuth capacity exhausted#537RyderFreeman4Logos merged 1 commit intomainfrom
Conversation
…y exhausted When gemini-cli ACP transport fails with "No capacity available" or similar rate-limit errors, CSA now automatically: 1. Detects the capacity/quota error pattern in the result 2. Reads api_key from [tools.gemini-cli] config 3. Injects GEMINI_API_KEY env var and retries ONCE 4. Logs warning when fallback is used The first attempt always uses OAuth (no API key injected). Only on capacity exhaustion is the stored API key used as fallback. Closes #533 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates crate versions and enhances the Gemini retry and fallback logic in the csa-executor crate. The changes introduce comprehensive tracing and warning logs for the three-phase authentication fallback mechanism (OAuth to API Key) in both LegacyTransport and AcpTransport, and refactor associated unit tests into a dedicated file. Feedback suggests refactoring the duplicated retry initialization logic into a shared handler to improve maintainability and consistency across transport methods. Additionally, a test should be updated to use a dynamic temporary directory instead of a hardcoded path to ensure robustness.
| let max_attempts = gemini_max_attempts(extra_env); | ||
| let has_fallback_key = extra_env | ||
| .is_some_and(|env| env.contains_key(csa_core::gemini::API_KEY_FALLBACK_ENV_KEY)); | ||
| let auth_mode = gemini_auth_mode(extra_env).unwrap_or("unknown"); | ||
| tracing::debug!( | ||
| max_attempts, | ||
| has_fallback_key, | ||
| auth_mode, | ||
| "gemini-cli ACP retry chain initialized" | ||
| ); | ||
|
|
||
| let mut attempt = 1u8; | ||
| loop { | ||
| // Build ACP args for this attempt, injecting model override in phase 3. | ||
| let mut args = self.acp_args.clone(); | ||
| if let Some(model) = gemini_retry_model(attempt) { | ||
| tracing::info!(attempt, model, "gemini-cli ACP: overriding model for retry"); | ||
| args.extend(["-m".into(), model.into()]); | ||
| } | ||
|
|
||
| // Phase 2+: inject API key auth if available, otherwise keep original env. | ||
| let api_key_env = if gemini_should_use_api_key(attempt) { | ||
| gemini_inject_api_key_fallback(extra_env) | ||
| let injected = gemini_inject_api_key_fallback(extra_env); | ||
| if injected.is_none() { | ||
| tracing::warn!( | ||
| attempt, | ||
| auth_mode, | ||
| has_fallback_key, | ||
| "gemini-cli ACP: API key fallback unavailable for retry \ | ||
| (auth_mode must be 'oauth' and _CSA_API_KEY_FALLBACK must be set); \ | ||
| retrying with original auth" | ||
| ); | ||
| } | ||
| injected | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
There's significant code duplication here for initializing the retry chain and injecting the API key. This logic is nearly identical to the implementation in LegacyTransport::execute_in (lines 241-272).
Additionally, the LegacyTransport::execute method (lines 317-359 in the full file) which also contains retry logic, has not been updated with this new tracing and fallback checking, leading to inconsistent behavior.
To improve maintainability and ensure consistency, consider refactoring this duplicated logic into a shared helper function or a dedicated struct (e.g., GeminiRetryHandler). This would centralize the retry logic and make it easier to apply to all relevant transport methods.
| let result = transport | ||
| .execute_in( | ||
| "test api key fallback", | ||
| std::path::Path::new("/tmp"), |
There was a problem hiding this comment.
Using a hardcoded path like /tmp can make tests brittle as it might not be available or writable on all systems. The test already creates a temporary directory via setup_fake_gemini_environment on line 53, but it's currently unused (_temp).
Please use the provided temporary directory to make the test more robust. You'll also need to change _temp to temp on line 53.
| std::path::Path::new("/tmp"), | |
| temp.path(), |
Audit: Bot Findings AssessmentFinding 1: Code duplication in retry chain (transport.rs:636) — DismissedThe retry initialization between
The current approach favors clarity and independence over DRY. Each transport type owns its complete execution lifecycle. Finding 2: Hardcoded /tmp in test (transport_tests_gemini_fallback.rs:64) — Accepted (pre-existing)The 🤖 Arbitrated by Claude Code orchestrator |
…llback fix(executor): auto-fallback to API key when OAuth capacity exhausted
1 participant
Summary
[tools.gemini-cli].api_keyconfigCloses #533
Test plan
csa reviewwith gemini-cli succeeds when OAuth has capacity (no API key injected)csa reviewwith gemini-cli auto-falls-back to API key when OAuth returns "No capacity available"has_fallback_key: bool)🤖 Generated with Claude Code