mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 23:50:17 +00:00
fix: address code review issues in gemini-cli OAuth integration
- Add cache_read_input_tokens/cache_creation_input_tokens fields (value 0) - Implement manual Debug for OAuthCredential to redact tokens - Fix hardcoded /tmp: use GeminiOauthConfig::default_credentials_path() - Replace emoji output with plain text markers - Propagate Client::builder() errors instead of silent fallback - Use tokio::fs for all file I/O in CredentialManager (was std::fs) - Use if let Some(ref pid) to avoid consuming credential.project_id - Extract uses_cloud_code_api() helper; route by major version (gemini-2+) - Concatenate multiple system messages into systemInstruction - Include functionCall parts in assistant message conversion - Add 401 retry loop with allow_retry flag for auth failures - Remove biased from tokio::select! in OAuth callback handler - Remove hardcoded context_length 1M; vary by model family - Change GOOG_API_CLIENT from Node.js spoof to gl-rust/1.0.0 - Implement list_models() with static model list - Move create_gemini_oauth_provider() before test module (clippy) - Fix 9 additional clippy warnings (collapsible_if, map_or, needless_borrow) - Run cargo fmt
This commit is contained in:
+3
-10
@@ -46,8 +46,6 @@ impl std::str::FromStr for CacheRetention {
|
||||
"invalid cache retention '{}', expected one of: none, short, long",
|
||||
s
|
||||
)),
|
||||
s
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -203,6 +201,7 @@ impl LlmConfig {
|
||||
},
|
||||
provider: None,
|
||||
bedrock: None,
|
||||
gemini_oauth: None,
|
||||
request_timeout_secs: 120,
|
||||
}
|
||||
}
|
||||
@@ -334,16 +333,11 @@ impl LlmConfig {
|
||||
|
||||
let request_timeout_secs = parse_optional_env("LLM_REQUEST_TIMEOUT_SECS", 120)?;
|
||||
|
||||
let gemini_oauth = if backend == LlmBackend::GeminiOauth {
|
||||
let gemini_oauth = if backend_lower == "gemini_oauth" || backend_lower == "gemini-oauth" {
|
||||
let model = Self::resolve_model("GEMINI_MODEL", settings, "gemini-2.5-flash")?;
|
||||
let credentials_path = optional_env("GEMINI_CREDENTIALS_PATH")?
|
||||
.map(PathBuf::from)
|
||||
.unwrap_or_else(|| {
|
||||
dirs::home_dir()
|
||||
.unwrap_or_else(|| PathBuf::from("/tmp"))
|
||||
.join(".gemini")
|
||||
.join("oauth_creds.json")
|
||||
});
|
||||
.unwrap_or_else(GeminiOauthConfig::default_credentials_path);
|
||||
Some(GeminiOauthConfig {
|
||||
model,
|
||||
credentials_path,
|
||||
@@ -504,7 +498,6 @@ impl LlmConfig {
|
||||
model,
|
||||
extra_headers,
|
||||
oauth_token,
|
||||
>>>>>>> origin/main
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+579
-417
File diff suppressed because it is too large
Load Diff
+13
-12
@@ -14,8 +14,8 @@ mod bedrock;
|
||||
pub mod circuit_breaker;
|
||||
pub mod costs;
|
||||
pub mod failover;
|
||||
mod nearai_chat;
|
||||
pub mod gemini_oauth;
|
||||
mod nearai_chat;
|
||||
mod provider;
|
||||
mod reasoning;
|
||||
pub mod recording;
|
||||
@@ -31,8 +31,8 @@ pub mod vision_models;
|
||||
|
||||
pub use circuit_breaker::{CircuitBreakerConfig, CircuitBreakerProvider};
|
||||
pub use failover::{CooldownConfig, FailoverProvider};
|
||||
pub use nearai_chat::{ModelInfo, NearAiChatProvider};
|
||||
pub use gemini_oauth::GeminiOauthProvider;
|
||||
pub use nearai_chat::{ModelInfo, NearAiChatProvider};
|
||||
pub use provider::{
|
||||
ChatMessage, CompletionRequest, CompletionResponse, ContentPart, FinishReason, ImageUrl,
|
||||
LlmProvider, ModelMetadata, Role, ToolCall, ToolCompletionRequest, ToolCompletionResponse,
|
||||
@@ -534,6 +534,17 @@ pub async fn build_provider_chain(
|
||||
Ok((llm, cheap_llm, recording_handle))
|
||||
}
|
||||
|
||||
pub fn create_gemini_oauth_provider(config: &LlmConfig) -> Result<Arc<dyn LlmProvider>, LlmError> {
|
||||
let gemini_config = config
|
||||
.gemini_oauth
|
||||
.clone()
|
||||
.ok_or_else(|| LlmError::AuthFailed {
|
||||
provider: "gemini_oauth".to_string(),
|
||||
})?;
|
||||
let provider = gemini_oauth::GeminiOauthProvider::new(gemini_config)?;
|
||||
Ok(Arc::new(provider))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -607,13 +618,3 @@ mod tests {
|
||||
assert!(result.unwrap().is_none());
|
||||
}
|
||||
}
|
||||
|
||||
pub fn create_gemini_oauth_provider(config: &LlmConfig) -> Result<Arc<dyn LlmProvider>, LlmError> {
|
||||
let gemini_config = config
|
||||
.gemini_oauth
|
||||
.clone()
|
||||
.ok_or_else(|| LlmError::AuthFailed {
|
||||
provider: "gemini_oauth".to_string(),
|
||||
})?;
|
||||
Ok(Arc::new(gemini_oauth::GeminiOauthProvider::new(gemini_config)))
|
||||
}
|
||||
|
||||
+131
-135
@@ -982,7 +982,6 @@ impl SetupWizard {
|
||||
self.setup_openai_compatible_generic(&def.id, secret_name, display_name)
|
||||
.await?;
|
||||
}
|
||||
>>>>>>> origin/main
|
||||
}
|
||||
|
||||
Ok(())
|
||||
@@ -1416,7 +1415,13 @@ impl SetupWizard {
|
||||
println!();
|
||||
|
||||
let creds_path = crate::config::GeminiOauthConfig::default_credentials_path();
|
||||
let cred_manager = crate::llm::gemini_oauth::CredentialManager::new(&creds_path);
|
||||
let cred_manager =
|
||||
crate::llm::gemini_oauth::CredentialManager::new(&creds_path).map_err(|e| {
|
||||
SetupError::Config(format!(
|
||||
"Failed to initialize Gemini credential manager: {}",
|
||||
e
|
||||
))
|
||||
})?;
|
||||
|
||||
match cred_manager.get_valid_credential().await {
|
||||
Ok(cred) => {
|
||||
@@ -1461,129 +1466,8 @@ impl SetupWizard {
|
||||
let backend = self.settings.llm_backend.as_deref().unwrap_or("nearai");
|
||||
let registry = crate::llm::ProviderRegistry::load();
|
||||
|
||||
if backend == "nearai" {
|
||||
// NEAR AI: use existing provider list_models()
|
||||
let fetched = self.fetch_nearai_models().await;
|
||||
let default_models: Vec<(String, String)> = vec![
|
||||
(
|
||||
"zai-org/GLM-latest".into(),
|
||||
"GLM Latest (default, fast)".into(),
|
||||
),
|
||||
(
|
||||
"anthropic::claude-sonnet-4-20250514".into(),
|
||||
"Claude Sonnet 4 (best quality)".into(),
|
||||
),
|
||||
(
|
||||
"openai::gpt-5.3-codex".into(),
|
||||
"GPT-5.3 Codex (flagship)".into(),
|
||||
),
|
||||
("openai::gpt-5.2".into(), "GPT-5.2".into()),
|
||||
("openai::gpt-4o".into(), "GPT-4o".into()),
|
||||
];
|
||||
|
||||
let models = if fetched.is_empty() {
|
||||
default_models
|
||||
} else {
|
||||
fetched.iter().map(|m| (m.clone(), m.clone())).collect()
|
||||
};
|
||||
self.select_from_model_list(&models)?;
|
||||
} else if let Some(def) = registry.find(backend) {
|
||||
let can_list = def
|
||||
.setup
|
||||
.as_ref()
|
||||
.map(|s| s.can_list_models())
|
||||
.unwrap_or(false);
|
||||
|
||||
if can_list {
|
||||
// Try to fetch models from the provider's /v1/models endpoint
|
||||
let cached_key = self
|
||||
.llm_api_key
|
||||
.as_ref()
|
||||
.map(|k| k.expose_secret().to_string());
|
||||
|
||||
let models = match backend {
|
||||
"anthropic" => fetch_anthropic_models(cached_key.as_deref()).await,
|
||||
"openai" => fetch_openai_models(cached_key.as_deref()).await,
|
||||
"ollama" => {
|
||||
let base_url = self
|
||||
.settings
|
||||
.ollama_base_url
|
||||
.as_deref()
|
||||
.or(def.default_base_url.as_deref())
|
||||
.unwrap_or("http://localhost:11434");
|
||||
let models = fetch_ollama_models(base_url).await;
|
||||
if models.is_empty() {
|
||||
print_info("No models found. Pull one first: ollama pull llama3");
|
||||
}
|
||||
models
|
||||
}
|
||||
_ => {
|
||||
// Generic OpenAI-compatible model listing
|
||||
let base_url = def.default_base_url.as_deref().unwrap_or("");
|
||||
fetch_openai_compatible_models(base_url, cached_key.as_deref()).await
|
||||
}
|
||||
};
|
||||
|
||||
// Apply models_filter from setup hint (e.g., Groq "chat" filters non-chat models)
|
||||
let models =
|
||||
if let Some(filter) = def.setup.as_ref().and_then(|s| s.models_filter()) {
|
||||
let filter_lower = filter.to_lowercase();
|
||||
models
|
||||
.into_iter()
|
||||
.filter(|(id, _)| id.to_lowercase().contains(&filter_lower))
|
||||
.collect()
|
||||
} else {
|
||||
models
|
||||
};
|
||||
|
||||
if models.is_empty() {
|
||||
// Fall back to manual entry
|
||||
let default = &def.default_model;
|
||||
let model_id = input(&format!("Model name (default: {default})"))
|
||||
.map_err(SetupError::Io)?;
|
||||
let model_id = if model_id.is_empty() {
|
||||
default.clone()
|
||||
} else {
|
||||
model_id
|
||||
};
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
} else {
|
||||
self.select_from_model_list(&models)?;
|
||||
}
|
||||
} else {
|
||||
// Manual model entry
|
||||
let default = &def.default_model;
|
||||
let model_id =
|
||||
input(&format!("Model name (default: {default})")).map_err(SetupError::Io)?;
|
||||
let model_id = if model_id.is_empty() {
|
||||
default.clone()
|
||||
} else {
|
||||
model_id
|
||||
};
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
"gemini_oauth" => {
|
||||
let default_models: Vec<(String, String)> = vec![
|
||||
("gemini-3.1-pro-preview".into(), "Gemini 3.1 Pro (Latest, strongest reasoning)".into()),
|
||||
("gemini-3-flash-preview".into(), "Gemini 3 Flash (Fast preview with thinking)".into()),
|
||||
("gemini-2.5-pro".into(), "Gemini 2.5 Pro (Stable, strong reasoning)".into()),
|
||||
("gemini-2.5-flash".into(), "Gemini 2.5 Flash (Fast, good quality)".into()),
|
||||
("gemini-2.5-flash-lite".into(), "Gemini 2.5 Flash Lite (Fastest, lightweight)".into()),
|
||||
];
|
||||
self.select_from_model_list(&default_models)?;
|
||||
}
|
||||
"bedrock" => {
|
||||
let model_id = input("Bedrock model ID (e.g., anthropic.claude-opus-4-6-v1)")
|
||||
.map_err(SetupError::Io)?;
|
||||
if model_id.is_empty() {
|
||||
return Err(SetupError::Config("Model ID is required".to_string()));
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
_ => {
|
||||
match backend {
|
||||
"nearai" => {
|
||||
// NEAR AI: use existing provider list_models()
|
||||
let fetched = self.fetch_nearai_models().await;
|
||||
let default_models: Vec<(String, String)> = vec![
|
||||
@@ -1610,18 +1494,130 @@ impl SetupWizard {
|
||||
};
|
||||
self.select_from_model_list(&models)?;
|
||||
}
|
||||
"gemini_oauth" | "gemini-oauth" => {
|
||||
let default_models: Vec<(String, String)> = vec![
|
||||
(
|
||||
"gemini-2.0-flash".into(),
|
||||
"Gemini 2.0 Flash (Latest, fast)".into(),
|
||||
),
|
||||
(
|
||||
"gemini-2.0-flash-thinking-exp-1219".into(),
|
||||
"Gemini 2.0 Flash Thinking (Latest, reasoning)".into(),
|
||||
),
|
||||
(
|
||||
"gemini-1.5-pro".into(),
|
||||
"Gemini 1.5 Pro (Stable, strong reasoning)".into(),
|
||||
),
|
||||
(
|
||||
"gemini-1.5-flash".into(),
|
||||
"Gemini 1.5 Flash (Fastest, good quality)".into(),
|
||||
),
|
||||
];
|
||||
self.select_from_model_list(&default_models)?;
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
} else {
|
||||
// Unknown provider, manual entry
|
||||
let model_id = input("Model name (e.g., meta-llama/Llama-3-8b-chat-hf)")
|
||||
.map_err(SetupError::Io)?;
|
||||
if model_id.is_empty() {
|
||||
return Err(SetupError::Config("Model name is required".to_string()));
|
||||
"bedrock" => {
|
||||
let model_id =
|
||||
input("Bedrock model ID (e.g., anthropic.claude-v3-sonnet-20240229-v1:0)")
|
||||
.map_err(SetupError::Io)?;
|
||||
if model_id.is_empty() {
|
||||
return Err(SetupError::Config("Model ID is required".to_string()));
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
_ => {
|
||||
if let Some(def) = registry.find(backend) {
|
||||
let can_list = def
|
||||
.setup
|
||||
.as_ref()
|
||||
.map(|s| s.can_list_models())
|
||||
.unwrap_or(false);
|
||||
|
||||
if can_list {
|
||||
// Try to fetch models from the provider's /v1/models endpoint
|
||||
let cached_key = self
|
||||
.llm_api_key
|
||||
.as_ref()
|
||||
.map(|k| k.expose_secret().to_string());
|
||||
|
||||
let models = match backend {
|
||||
"anthropic" => fetch_anthropic_models(cached_key.as_deref()).await,
|
||||
"openai" => fetch_openai_models(cached_key.as_deref()).await,
|
||||
"ollama" => {
|
||||
let base_url = self
|
||||
.settings
|
||||
.ollama_base_url
|
||||
.as_deref()
|
||||
.or(def.default_base_url.as_deref())
|
||||
.unwrap_or("http://localhost:11434");
|
||||
let models = fetch_ollama_models(base_url).await;
|
||||
if models.is_empty() {
|
||||
print_info(
|
||||
"No models found. Pull one first: ollama pull llama3",
|
||||
);
|
||||
}
|
||||
models
|
||||
}
|
||||
_ => {
|
||||
// Generic OpenAI-compatible model listing
|
||||
let base_url = def.default_base_url.as_deref().unwrap_or("");
|
||||
fetch_openai_compatible_models(base_url, cached_key.as_deref())
|
||||
.await
|
||||
}
|
||||
};
|
||||
|
||||
// Apply models_filter from setup hint (e.g., Groq "chat" filters non-chat models)
|
||||
let models = if let Some(filter) =
|
||||
def.setup.as_ref().and_then(|s| s.models_filter())
|
||||
{
|
||||
let filter_lower = filter.to_lowercase();
|
||||
models
|
||||
.into_iter()
|
||||
.filter(|(id, _)| id.to_lowercase().contains(&filter_lower))
|
||||
.collect()
|
||||
} else {
|
||||
models
|
||||
};
|
||||
|
||||
if models.is_empty() {
|
||||
// Fall back to manual entry
|
||||
let default = &def.default_model;
|
||||
let model_id = input(&format!("Model name (default: {default})"))
|
||||
.map_err(SetupError::Io)?;
|
||||
let model_id = if model_id.is_empty() {
|
||||
default.clone()
|
||||
} else {
|
||||
model_id
|
||||
};
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
} else {
|
||||
self.select_from_model_list(&models)?;
|
||||
}
|
||||
} else {
|
||||
// Manual model entry
|
||||
let default = &def.default_model;
|
||||
let model_id = input(&format!("Model name (default: {default})"))
|
||||
.map_err(SetupError::Io)?;
|
||||
let model_id = if model_id.is_empty() {
|
||||
default.clone()
|
||||
} else {
|
||||
model_id
|
||||
};
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
} else {
|
||||
// Unknown provider, manual entry
|
||||
let model_id = input("Model name (e.g., meta-llama/Llama-3-8b-chat-hf)")
|
||||
.map_err(SetupError::Io)?;
|
||||
if model_id.is_empty() {
|
||||
return Err(SetupError::Config("Model name is required".to_string()));
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
}
|
||||
self.settings.selected_model = Some(model_id.clone());
|
||||
print_success(&format!("Selected {}", model_id));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user