From 8cd907407a60387cb85edbd5e15e230cf6644afa Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 8 Feb 2026 10:54:10 +0800 Subject: [PATCH] fix: remove Option from model listing return types, propagate errors Remove the Option wrapper from fetch_supported_models and fetch_recommended_models. An empty vec already means "no models" so the Option added complexity with no value. Dynamic providers now propagate errors instead of swallowing them into Ok(None). This ensures misconfigured providers surface errors to the desktop and CLI rather than silently returning no models. Signed-off-by: Adrian Cole --- crates/goose-cli/src/commands/configure.rs | 6 +- .../src/routes/config_management.rs | 16 +---- crates/goose/src/agents/reply_parts.rs | 5 +- crates/goose/src/providers/anthropic.rs | 13 ++-- crates/goose/src/providers/auto_detect.rs | 4 +- crates/goose/src/providers/base.rs | 15 ++--- crates/goose/src/providers/bedrock.rs | 4 ++ .../canonical/build_canonical_models.rs | 9 +-- crates/goose/src/providers/chatgpt_codex.rs | 12 ++-- crates/goose/src/providers/claude_code.rs | 7 +++ crates/goose/src/providers/codex.rs | 6 +- crates/goose/src/providers/cursor_agent.rs | 7 +++ crates/goose/src/providers/databricks.rs | 61 +++++++------------ crates/goose/src/providers/gcpvertexai.rs | 4 +- crates/goose/src/providers/gemini_cli.rs | 7 +++ crates/goose/src/providers/githubcopilot.rs | 13 ++-- crates/goose/src/providers/google.rs | 16 +++-- crates/goose/src/providers/lead_worker.rs | 20 ++---- crates/goose/src/providers/litellm.rs | 16 ++--- crates/goose/src/providers/ollama.rs | 4 +- crates/goose/src/providers/openai.rs | 4 +- .../goose/src/providers/openai_compatible.rs | 23 +++---- crates/goose/src/providers/openrouter.rs | 48 ++++++--------- crates/goose/src/providers/snowflake.rs | 7 +++ crates/goose/src/providers/tetrate.rs | 40 +++++------- crates/goose/src/providers/venice.rs | 4 +- crates/goose/tests/providers.rs | 24 ++++---- 27 files changed, 177 insertions(+), 218 deletions(-) diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index ab0568729ec1..29b9650bf8d2 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -691,15 +691,15 @@ pub async fn configure_provider_dialog() -> anyhow::Result { }; spin.stop(style("Model fetch complete").green()); - // Select a model: on fetch error show styled error and abort; if Some(models), show list; if None, free-text input + // Select a model: on fetch error show styled error and abort; if models available, show list; otherwise free-text input let model: String = match models_res { Err(e) => { // Provider hook error cliclack::outro(style(e.to_string()).on_red().white())?; return Ok(false); } - Ok(Some(models)) => select_model_from_list(&models, provider_meta)?, - Ok(None) => { + Ok(models) if !models.is_empty() => select_model_from_list(&models, provider_meta)?, + Ok(_) => { let default_model = std::env::var("GOOSE_MODEL").unwrap_or(provider_meta.default_model.clone()); cliclack::input("Enter a model from that provider:") diff --git a/crates/goose-server/src/routes/config_management.rs b/crates/goose-server/src/routes/config_management.rs index 6c12b6c0e1f9..79e62aa28f53 100644 --- a/crates/goose-server/src/routes/config_management.rs +++ b/crates/goose-server/src/routes/config_management.rs @@ -372,19 +372,6 @@ pub async fn providers() -> Result>, ErrorResponse> { pub async fn get_provider_models( Path(name): Path, ) -> Result>, ErrorResponse> { - let loaded_provider = goose::config::declarative_providers::load_provider(name.as_str()).ok(); - // TODO(Douwe): support a get models url for custom providers - if let Some(loaded_provider) = loaded_provider { - return Ok(Json( - loaded_provider - .config - .models - .into_iter() - .map(|m| m.name) - .collect::>(), - )); - } - let all = get_providers().await.into_iter().collect::>(); let Some((metadata, provider_type)) = all.into_iter().find(|(m, _)| m.name == name) else { return Err(ErrorResponse::bad_request(format!( @@ -405,8 +392,7 @@ pub async fn get_provider_models( let models_result = provider.fetch_recommended_models().await; match models_result { - Ok(Some(models)) => Ok(Json(models)), - Ok(None) => Ok(Json(Vec::new())), + Ok(models) => Ok(Json(models)), Err(provider_error) => Err(provider_error.into()), } } diff --git a/crates/goose/src/agents/reply_parts.rs b/crates/goose/src/agents/reply_parts.rs index ada2410be70f..ff8670eea15e 100644 --- a/crates/goose/src/agents/reply_parts.rs +++ b/crates/goose/src/agents/reply_parts.rs @@ -31,9 +31,12 @@ async fn enhance_model_error(error: ProviderError, provider: &Arc) return error; } - let Ok(Some(models)) = provider.fetch_recommended_models().await else { + let Ok(models) = provider.fetch_recommended_models().await else { return error; }; + if models.is_empty() { + return error; + } ProviderError::RequestFailed(format!( "{}. Available models for this provider: {}", diff --git a/crates/goose/src/providers/anthropic.rs b/crates/goose/src/providers/anthropic.rs index dbc2026e0877..8a48bc218521 100644 --- a/crates/goose/src/providers/anthropic.rs +++ b/crates/goose/src/providers/anthropic.rs @@ -256,7 +256,7 @@ impl Provider for AnthropicProvider { Ok((message, provider_usage)) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let response = self.api_client.request(None, "v1/models").api_get().await?; if response.status != StatusCode::OK { @@ -267,17 +267,18 @@ impl Provider for AnthropicProvider { } let json = response.payload.unwrap_or_default(); - let arr = match json.get("data").and_then(|v| v.as_array()) { - Some(arr) => arr, - None => return Ok(None), - }; + let arr = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + ProviderError::RequestFailed( + "Missing 'data' array in Anthropic models response".to_string(), + ) + })?; let mut models: Vec = arr .iter() .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) .collect(); models.sort(); - Ok(Some(models)) + Ok(models) } async fn stream( diff --git a/crates/goose/src/providers/auto_detect.rs b/crates/goose/src/providers/auto_detect.rs index 0513fd928be7..43f13ea21e4c 100644 --- a/crates/goose/src/providers/auto_detect.rs +++ b/crates/goose/src/providers/auto_detect.rs @@ -31,7 +31,9 @@ pub async fn detect_provider_from_api_key(api_key: &str) -> Option<(String, Vec< }) .await { - Ok(Some(models)) => Some((provider_name.to_string(), models)), + Ok(models) if !models.is_empty() => { + Some((provider_name.to_string(), models)) + } _ => None, } } diff --git a/crates/goose/src/providers/base.rs b/crates/goose/src/providers/base.rs index ab343750e1a9..67ac0e5f9f2e 100644 --- a/crates/goose/src/providers/base.rs +++ b/crates/goose/src/providers/base.rs @@ -447,16 +447,13 @@ pub trait Provider: Send + Sync { RetryConfig::default() } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - Ok(None) + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(vec![]) } /// Fetch models filtered by canonical registry and usability - async fn fetch_recommended_models(&self) -> Result>, ProviderError> { - let all_models = match self.fetch_supported_models().await? { - Some(models) => models, - None => return Ok(None), - }; + async fn fetch_recommended_models(&self) -> Result, ProviderError> { + let all_models = self.fetch_supported_models().await?; let registry = CanonicalModelRegistry::bundled().map_err(|e| { ProviderError::ExecutionError(format!("Failed to load canonical registry: {}", e)) @@ -501,9 +498,9 @@ pub trait Provider: Send + Sync { .collect(); if recommended_models.is_empty() { - Ok(Some(all_models)) + Ok(all_models) } else { - Ok(Some(recommended_models)) + Ok(recommended_models) } } diff --git a/crates/goose/src/providers/bedrock.rs b/crates/goose/src/providers/bedrock.rs index 3551a54c9835..898112eba658 100644 --- a/crates/goose/src/providers/bedrock.rs +++ b/crates/goose/src/providers/bedrock.rs @@ -301,6 +301,10 @@ impl Provider for BedrockProvider { self.model.clone() } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(BEDROCK_KNOWN_MODELS.iter().map(|s| s.to_string()).collect()) + } + #[tracing::instrument( skip(self, model_config, system, messages, tools), fields(model_config, input, output, input_tokens, output_tokens, total_tokens) diff --git a/crates/goose/src/providers/canonical/build_canonical_models.rs b/crates/goose/src/providers/canonical/build_canonical_models.rs index e53cd7310a9c..0fe11dffec59 100644 --- a/crates/goose/src/providers/canonical/build_canonical_models.rs +++ b/crates/goose/src/providers/canonical/build_canonical_models.rs @@ -493,14 +493,10 @@ async fn check_provider( }; let fetched_models = match provider.fetch_supported_models().await { - Ok(Some(models)) => { + Ok(models) => { println!(" ✓ Fetched {} models", models.len()); models } - Ok(None) => { - println!(" ⚠ Provider does not support model listing"); - Vec::new() - } Err(e) => { println!(" ⚠ Failed to fetch models: {}", e); println!(" This is expected if credentials are not configured."); @@ -509,11 +505,10 @@ async fn check_provider( }; let recommended_models = match provider.fetch_recommended_models().await { - Ok(Some(models)) => { + Ok(models) => { println!(" ✓ Found {} recommended models", models.len()); models } - Ok(None) => Vec::new(), Err(e) => { println!(" ⚠ Failed to fetch recommended models: {}", e); Vec::new() diff --git a/crates/goose/src/providers/chatgpt_codex.rs b/crates/goose/src/providers/chatgpt_codex.rs index 0206ac5ca473..8dacdcbb4c97 100644 --- a/crates/goose/src/providers/chatgpt_codex.rs +++ b/crates/goose/src/providers/chatgpt_codex.rs @@ -985,13 +985,11 @@ impl Provider for ChatGptCodexProvider { Ok(()) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - Ok(Some( - CHATGPT_CODEX_KNOWN_MODELS - .iter() - .map(|s| s.to_string()) - .collect(), - )) + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(CHATGPT_CODEX_KNOWN_MODELS + .iter() + .map(|s| s.to_string()) + .collect()) } } diff --git a/crates/goose/src/providers/claude_code.rs b/crates/goose/src/providers/claude_code.rs index b7148f8864a5..33ba2f3b3372 100644 --- a/crates/goose/src/providers/claude_code.rs +++ b/crates/goose/src/providers/claude_code.rs @@ -493,6 +493,13 @@ impl Provider for ClaudeCodeProvider { self.model.clone() } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(CLAUDE_CODE_KNOWN_MODELS + .iter() + .map(|s| s.to_string()) + .collect()) + } + #[tracing::instrument( skip(self, model_config, system, messages, tools), fields(model_config, input, output, input_tokens, output_tokens, total_tokens) diff --git a/crates/goose/src/providers/codex.rs b/crates/goose/src/providers/codex.rs index fc03c8a14f1e..8c44edcc8019 100644 --- a/crates/goose/src/providers/codex.rs +++ b/crates/goose/src/providers/codex.rs @@ -662,10 +662,8 @@ impl Provider for CodexProvider { )) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - Ok(Some( - CODEX_KNOWN_MODELS.iter().map(|s| s.to_string()).collect(), - )) + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(CODEX_KNOWN_MODELS.iter().map(|s| s.to_string()).collect()) } } diff --git a/crates/goose/src/providers/cursor_agent.rs b/crates/goose/src/providers/cursor_agent.rs index ad593f158bf0..9131d9c8344a 100644 --- a/crates/goose/src/providers/cursor_agent.rs +++ b/crates/goose/src/providers/cursor_agent.rs @@ -355,6 +355,13 @@ impl Provider for CursorAgentProvider { self.model.clone() } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(CURSOR_AGENT_KNOWN_MODELS + .iter() + .map(|s| s.to_string()) + .collect()) + } + #[tracing::instrument( skip(self, model_config, system, messages, tools), fields(model_config, input, output, input_tokens, output_tokens, total_tokens) diff --git a/crates/goose/src/providers/databricks.rs b/crates/goose/src/providers/databricks.rs index ca3d1c0722e6..5f6f2b823759 100644 --- a/crates/goose/src/providers/databricks.rs +++ b/crates/goose/src/providers/databricks.rs @@ -391,51 +391,38 @@ impl Provider for DatabricksProvider { .map_err(|e| ProviderError::ExecutionError(e.to_string())) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - let response = match self + async fn fetch_supported_models(&self) -> Result, ProviderError> { + let response = self .api_client .request(None, "api/2.0/serving-endpoints") .response_get() .await - { - Ok(resp) => resp, - Err(e) => { - tracing::warn!("Failed to fetch Databricks models: {}", e); - return Ok(None); - } - }; + .map_err(|e| { + ProviderError::RequestFailed(format!("Failed to fetch Databricks models: {}", e)) + })?; if !response.status().is_success() { let status = response.status(); - if let Ok(error_text) = response.text().await { - tracing::warn!( - "Failed to fetch Databricks models: {} - {}", - status, - error_text - ); - } else { - tracing::warn!("Failed to fetch Databricks models: {}", status); - } - return Ok(None); + let detail = response.text().await.unwrap_or_default(); + return Err(ProviderError::RequestFailed(format!( + "Failed to fetch Databricks models: {} {}", + status, detail + ))); } - let json: Value = match response.json().await { - Ok(json) => json, - Err(e) => { - tracing::warn!("Failed to parse Databricks API response: {}", e); - return Ok(None); - } - }; + let json: Value = response.json().await.map_err(|e| { + ProviderError::RequestFailed(format!("Failed to parse Databricks API response: {}", e)) + })?; - let endpoints = match json.get("endpoints").and_then(|v| v.as_array()) { - Some(endpoints) => endpoints, - None => { - tracing::warn!( + let endpoints = json + .get("endpoints") + .and_then(|v| v.as_array()) + .ok_or_else(|| { + ProviderError::RequestFailed( "Unexpected response format from Databricks API: missing 'endpoints' array" - ); - return Ok(None); - } - }; + .to_string(), + ) + })?; let models: Vec = endpoints .iter() @@ -447,11 +434,7 @@ impl Provider for DatabricksProvider { }) .collect(); - if models.is_empty() { - Ok(None) - } else { - Ok(Some(models)) - } + Ok(models) } } diff --git a/crates/goose/src/providers/gcpvertexai.rs b/crates/goose/src/providers/gcpvertexai.rs index db7b74df43e9..ac30055bc5e8 100644 --- a/crates/goose/src/providers/gcpvertexai.rs +++ b/crates/goose/src/providers/gcpvertexai.rs @@ -695,10 +695,10 @@ impl Provider for GcpVertexAIProvider { })) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let models: Vec = KNOWN_MODELS.iter().map(|s| s.to_string()).collect(); let filtered = self.filter_by_org_policy(models).await; - Ok(Some(filtered)) + Ok(filtered) } } diff --git a/crates/goose/src/providers/gemini_cli.rs b/crates/goose/src/providers/gemini_cli.rs index b2c6ff829797..9d7186d8549d 100644 --- a/crates/goose/src/providers/gemini_cli.rs +++ b/crates/goose/src/providers/gemini_cli.rs @@ -267,6 +267,13 @@ impl Provider for GeminiCliProvider { self.model.clone() } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(GEMINI_CLI_KNOWN_MODELS + .iter() + .map(|s| s.to_string()) + .collect()) + } + #[tracing::instrument( skip(self, _model_config, system, messages, tools), fields(model_config, input, output, input_tokens, output_tokens, total_tokens) diff --git a/crates/goose/src/providers/githubcopilot.rs b/crates/goose/src/providers/githubcopilot.rs index 821f57dea000..9c91dcd8575c 100644 --- a/crates/goose/src/providers/githubcopilot.rs +++ b/crates/goose/src/providers/githubcopilot.rs @@ -495,7 +495,7 @@ impl Provider for GithubCopilotProvider { stream_openai_compat(response, log) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let (endpoint, token) = self.get_api_info().await?; let url = format!("{}/models", endpoint); @@ -515,10 +515,11 @@ impl Provider for GithubCopilotProvider { let json: serde_json::Value = response.json().await?; - let arr = match json.get("data").and_then(|v| v.as_array()) { - Some(arr) => arr, - None => return Ok(None), - }; + let arr = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + ProviderError::RequestFailed( + "Missing 'data' array in GitHub Copilot models response".to_string(), + ) + })?; let mut models: Vec = arr .iter() .filter_map(|m| { @@ -532,7 +533,7 @@ impl Provider for GithubCopilotProvider { }) .collect(); models.sort(); - Ok(Some(models)) + Ok(models) } async fn configure_oauth(&self) -> Result<(), ProviderError> { diff --git a/crates/goose/src/providers/google.rs b/crates/goose/src/providers/google.rs index 736340d38ef6..68a9bd28a750 100644 --- a/crates/goose/src/providers/google.rs +++ b/crates/goose/src/providers/google.rs @@ -187,24 +187,28 @@ impl Provider for GoogleProvider { Ok((message, provider_usage)) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let response = self .api_client .request(None, "v1beta/models") .response_get() .await?; let json: serde_json::Value = response.json().await?; - let arr = match json.get("models").and_then(|v| v.as_array()) { - Some(arr) => arr, - None => return Ok(None), - }; + let arr = json + .get("models") + .and_then(|v| v.as_array()) + .ok_or_else(|| { + ProviderError::RequestFailed( + "Missing 'models' array in Google models response".to_string(), + ) + })?; let mut models: Vec = arr .iter() .filter_map(|m| m.get("name").and_then(|v| v.as_str())) .map(|name| name.split('/').next_back().unwrap_or(name).to_string()) .collect(); models.sort(); - Ok(Some(models)) + Ok(models) } fn supports_streaming(&self) -> bool { diff --git a/crates/goose/src/providers/lead_worker.rs b/crates/goose/src/providers/lead_worker.rs index 2dbc965957b7..4d53f58a05af 100644 --- a/crates/goose/src/providers/lead_worker.rs +++ b/crates/goose/src/providers/lead_worker.rs @@ -445,22 +445,14 @@ impl Provider for LeadWorkerProvider { final_result } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { // Combine models from both providers - let lead_models = self.lead_provider.fetch_supported_models().await?; + let mut all_models = self.lead_provider.fetch_supported_models().await?; let worker_models = self.worker_provider.fetch_supported_models().await?; - - match (lead_models, worker_models) { - (Some(lead), Some(worker)) => { - let mut all_models = lead; - all_models.extend(worker); - all_models.sort(); - all_models.dedup(); - Ok(Some(all_models)) - } - (Some(models), None) | (None, Some(models)) => Ok(Some(models)), - (None, None) => Ok(None), - } + all_models.extend(worker_models); + all_models.sort(); + all_models.dedup(); + Ok(all_models) } fn supports_embeddings(&self) -> bool { diff --git a/crates/goose/src/providers/litellm.rs b/crates/goose/src/providers/litellm.rs index 4d5af0d38d07..32e9a93be817 100644 --- a/crates/goose/src/providers/litellm.rs +++ b/crates/goose/src/providers/litellm.rs @@ -223,17 +223,11 @@ impl Provider for LiteLLMProvider { self.model.model_name.to_lowercase().contains("claude") } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - match self.fetch_models().await { - Ok(models) => { - let model_names: Vec = models.into_iter().map(|m| m.name).collect(); - Ok(Some(model_names)) - } - Err(e) => { - tracing::warn!("Failed to fetch models from LiteLLM: {}", e); - Ok(None) - } - } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + let models = self.fetch_models().await.map_err(|e| { + ProviderError::RequestFailed(format!("Failed to fetch models from LiteLLM: {}", e)) + })?; + Ok(models.into_iter().map(|m| m.name).collect()) } } diff --git a/crates/goose/src/providers/ollama.rs b/crates/goose/src/providers/ollama.rs index 12f06eb67708..56618c77eb6d 100644 --- a/crates/goose/src/providers/ollama.rs +++ b/crates/goose/src/providers/ollama.rs @@ -307,7 +307,7 @@ impl Provider for OllamaProvider { stream_ollama(response, log) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let response = self .api_client .request(None, "api/tags") @@ -340,7 +340,7 @@ impl Provider for OllamaProvider { model_names.sort(); - Ok(Some(model_names)) + Ok(model_names) } } diff --git a/crates/goose/src/providers/openai.rs b/crates/goose/src/providers/openai.rs index 87edb4b6435f..63d46a5ebe6e 100644 --- a/crates/goose/src/providers/openai.rs +++ b/crates/goose/src/providers/openai.rs @@ -353,7 +353,7 @@ impl Provider for OpenAiProvider { } } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let models_path = self.base_path.replace("v1/chat/completions", "v1/models"); let response = self .api_client @@ -377,7 +377,7 @@ impl Provider for OpenAiProvider { .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) .collect(); models.sort(); - Ok(Some(models)) + Ok(models) } fn supports_embeddings(&self) -> bool { diff --git a/crates/goose/src/providers/openai_compatible.rs b/crates/goose/src/providers/openai_compatible.rs index 2b77d09b52fe..3e703c54dc8b 100644 --- a/crates/goose/src/providers/openai_compatible.rs +++ b/crates/goose/src/providers/openai_compatible.rs @@ -112,7 +112,7 @@ impl Provider for OpenAiCompatibleProvider { Ok((message, ProviderUsage::new(response_model, usage))) } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let response = self .api_client .response_get(None, "models") @@ -128,18 +128,15 @@ impl Provider for OpenAiCompatibleProvider { return Err(ProviderError::Authentication(msg.to_string())); } - let data = json.get("data").and_then(|v| v.as_array()); - match data { - Some(arr) => { - let mut models: Vec = arr - .iter() - .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) - .collect(); - models.sort(); - Ok(Some(models)) - } - None => Ok(None), - } + let arr = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + ProviderError::RequestFailed("Missing 'data' array in models response".to_string()) + })?; + let mut models: Vec = arr + .iter() + .filter_map(|m| m.get("id").and_then(|v| v.as_str()).map(str::to_string)) + .collect(); + models.sort(); + Ok(models) } fn supports_streaming(&self) -> bool { diff --git a/crates/goose/src/providers/openrouter.rs b/crates/goose/src/providers/openrouter.rs index 0bdcd198b267..a54fc46af8a0 100644 --- a/crates/goose/src/providers/openrouter.rs +++ b/crates/goose/src/providers/openrouter.rs @@ -309,39 +309,35 @@ impl Provider for OpenRouterProvider { } /// Fetch supported models from OpenRouter API (only models with tool support) - async fn fetch_supported_models(&self) -> Result>, ProviderError> { - // Handle request failures gracefully - // If the request fails, fall back to manual entry - let response = match self + async fn fetch_supported_models(&self) -> Result, ProviderError> { + let response = self .api_client .request(None, "api/v1/models") .response_get() .await - { - Ok(response) => response, - Err(e) => { - tracing::warn!("Failed to fetch models from OpenRouter API: {}, falling back to manual model entry", e); - return Ok(None); - } - }; + .map_err(|e| { + ProviderError::RequestFailed(format!( + "Failed to fetch models from OpenRouter API: {}", + e + )) + })?; - // Handle JSON parsing failures gracefully - let json: serde_json::Value = match response.json().await { - Ok(json) => json, - Err(e) => { - tracing::warn!("Failed to parse OpenRouter API response as JSON: {}, falling back to manual model entry", e); - return Ok(None); - } - }; + let json: serde_json::Value = response.json().await.map_err(|e| { + ProviderError::RequestFailed(format!( + "Failed to parse OpenRouter API response as JSON: {}", + e + )) + })?; - // Check for error in response if let Some(err_obj) = json.get("error") { let msg = err_obj .get("message") .and_then(|v| v.as_str()) .unwrap_or("unknown error"); - tracing::warn!("OpenRouter API returned an error: {}", msg); - return Ok(None); + return Err(ProviderError::RequestFailed(format!( + "OpenRouter API returned an error: {}", + msg + ))); } let data = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { @@ -380,14 +376,8 @@ impl Provider for OpenRouterProvider { }) .collect(); - // If no models with tool support were found, fall back to manual entry - if models.is_empty() { - tracing::warn!("No models with tool support found in OpenRouter API response, falling back to manual model entry"); - return Ok(None); - } - models.sort(); - Ok(Some(models)) + Ok(models) } async fn supports_cache_control(&self) -> bool { diff --git a/crates/goose/src/providers/snowflake.rs b/crates/goose/src/providers/snowflake.rs index f0fcb01e262f..03c236904d69 100644 --- a/crates/goose/src/providers/snowflake.rs +++ b/crates/goose/src/providers/snowflake.rs @@ -328,6 +328,13 @@ impl Provider for SnowflakeProvider { self.model.clone() } + async fn fetch_supported_models(&self) -> Result, ProviderError> { + Ok(SNOWFLAKE_KNOWN_MODELS + .iter() + .map(|s| s.to_string()) + .collect()) + } + #[tracing::instrument( skip(self, model_config, system, messages, tools), fields(model_config, input, output, input_tokens, output_tokens, total_tokens) diff --git a/crates/goose/src/providers/tetrate.rs b/crates/goose/src/providers/tetrate.rs index 5dbf55f8559c..cb0d0fad790d 100644 --- a/crates/goose/src/providers/tetrate.rs +++ b/crates/goose/src/providers/tetrate.rs @@ -244,7 +244,7 @@ impl Provider for TetrateProvider { } /// Fetch supported models from Tetrate Agent Router Service API (only models with tool support) - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { // Use the existing api_client which already has authentication configured let response = match self .api_client @@ -261,14 +261,12 @@ impl Provider for TetrateProvider { } }; - // Handle JSON parsing failures gracefully - let json: serde_json::Value = match response.json().await { - Ok(json) => json, - Err(e) => { - tracing::warn!("Failed to parse Tetrate Agent Router Service API response as JSON: {}, falling back to manual model entry", e); - return Ok(None); - } - }; + let json: serde_json::Value = response.json().await.map_err(|e| { + ProviderError::ExecutionError(format!( + "Failed to parse Tetrate API response: {}. Please check your API key and account at {}", + e, TETRATE_DOC_URL + )) + })?; // Check for error in response if let Some(err_obj) = json.get("error") { @@ -276,10 +274,6 @@ impl Provider for TetrateProvider { .get("message") .and_then(|v| v.as_str()) .unwrap_or("unknown error"); - tracing::warn!( - "Tetrate Agent Router Service API returned an error: {}", - msg - ); return Err(ProviderError::ExecutionError(format!( "Tetrate API error: {}. Please check your API key and account at {}", msg, TETRATE_DOC_URL @@ -288,13 +282,12 @@ impl Provider for TetrateProvider { // The response format from /v1/models is expected to be OpenAI-compatible // It should have a "data" field with an array of model objects - let data = match json.get("data").and_then(|v| v.as_array()) { - Some(data) => data, - None => { - tracing::warn!("Tetrate Agent Router Service API response missing 'data' field, falling back to manual model entry"); - return Ok(None); - } - }; + let data = json.get("data").and_then(|v| v.as_array()).ok_or_else(|| { + ProviderError::ExecutionError(format!( + "Tetrate API response missing 'data' field. Please check your API key and account at {}", + TETRATE_DOC_URL + )) + })?; let mut models: Vec = data .iter() @@ -312,13 +305,8 @@ impl Provider for TetrateProvider { }) .collect(); - if models.is_empty() { - tracing::warn!("No models found in Tetrate Agent Router Service API response, falling back to manual model entry"); - return Ok(None); - } - models.sort(); - Ok(Some(models)) + Ok(models) } fn supports_streaming(&self) -> bool { diff --git a/crates/goose/src/providers/venice.rs b/crates/goose/src/providers/venice.rs index e42b1cb00715..b8187c76cb97 100644 --- a/crates/goose/src/providers/venice.rs +++ b/crates/goose/src/providers/venice.rs @@ -239,7 +239,7 @@ impl Provider for VeniceProvider { self.model.clone() } - async fn fetch_supported_models(&self) -> Result>, ProviderError> { + async fn fetch_supported_models(&self) -> Result, ProviderError> { let response = self .api_client .request(None, &self.models_path) @@ -264,7 +264,7 @@ impl Provider for VeniceProvider { }) .collect::>(); models.sort(); - Ok(Some(models)) + Ok(models) } #[tracing::instrument( diff --git a/crates/goose/tests/providers.rs b/crates/goose/tests/providers.rs index 46a667954591..0656bb3a743d 100644 --- a/crates/goose/tests/providers.rs +++ b/crates/goose/tests/providers.rs @@ -374,19 +374,17 @@ impl ProviderTester { dbg!(&models); println!("==================="); - if let Some(models) = models { - assert!(!models.is_empty(), "Expected non-empty model list"); - let model_name = &self.provider.get_model_config().model_name; - // Some providers (e.g. Ollama) return names with tags like "qwen3:latest" - // while the configured model name may be just "qwen3". - assert!( - models - .iter() - .any(|m| m == model_name || m.starts_with(&format!("{}:", model_name))), - "Expected model '{}' in supported models", - model_name - ); - } + assert!(!models.is_empty(), "Expected non-empty model list"); + let model_name = &self.provider.get_model_config().model_name; + // Some providers (e.g. Ollama) return names with tags like "qwen3:latest" + // while the configured model name may be just "qwen3". + assert!( + models + .iter() + .any(|m| m == model_name || m.starts_with(&format!("{}:", model_name))), + "Expected model '{}' in supported models", + model_name + ); Ok(()) }