From 7abf8b062fbc9557adc63e8c08c1aad5a93b6581 Mon Sep 17 00:00:00 2001 From: David Tivris <163727302+tivris@users.noreply.github.com> Date: Fri, 10 Apr 2026 19:38:43 +0300 Subject: [PATCH 1/6] fix(app): tolerate partial provider failures in get_all_provider_models() The function used .collect::>>() which fails on the first error, discarding all successful results from other providers. This meant a single stale provider (e.g. Bedrock with expired credentials) would block model fetching for all providers, causing confusing errors after an unrelated provider login. Replace with a loop that collects successes and logs warnings for individual failures. Only returns an error when every configured provider fails, matching the existing docstring contract. Fixes #2935 --- crates/forge_app/src/app.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index c8fec71741..5bbf45242c 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -326,10 +326,28 @@ impl> ForgeAp }) .collect(); - // Execute all provider fetches concurrently. - futures::future::join_all(futures) - .await - .into_iter() - .collect::>>() + // Execute all provider fetches concurrently, tolerating partial failures. + let mut first_error: Option = None; + let mut successes = Vec::new(); + + for result in futures::future::join_all(futures).await { + match result { + Ok(provider_models) => successes.push(provider_models), + Err(err) => { + tracing::warn!(error = %err, "failed to fetch models from a configured provider"); + if first_error.is_none() { + first_error = Some(err); + } + } + } + } + + if successes.is_empty() { + if let Some(err) = first_error { + return Err(err); + } + } + + Ok(successes) } } From a01263c7f18bf9119397428d5af47f5221f7758b Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 16:40:59 +0000 Subject: [PATCH 2/6] [autofix.ci] apply automated fixes --- crates/forge_app/src/app.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index 5bbf45242c..12d6b317f0 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -342,11 +342,10 @@ impl> ForgeAp } } - if successes.is_empty() { - if let Some(err) = first_error { + if successes.is_empty() + && let Some(err) = first_error { return Err(err); } - } Ok(successes) } From afed37f4b087a26d0660ead54ad4af981dd0aa35 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 16:42:51 +0000 Subject: [PATCH 3/6] [autofix.ci] apply automated fixes (attempt 2/3) --- crates/forge_app/src/app.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index 12d6b317f0..d2ed57a4ea 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -343,9 +343,10 @@ impl> ForgeAp } if successes.is_empty() - && let Some(err) = first_error { - return Err(err); - } + && let Some(err) = first_error + { + return Err(err); + } Ok(successes) } From ebecca305d243270595b39184b359a82c3d0253f Mon Sep 17 00:00:00 2001 From: David Tivris <163727302+tivris@users.noreply.github.com> Date: Fri, 10 Apr 2026 21:26:53 +0300 Subject: [PATCH 4/6] refactor(app): surface per-provider errors to UI callers Change get_all_provider_models() to return Vec> so individual provider failures (e.g. stale credentials) are shown to the user instead of being silently swallowed or failing the entire operation. - Errors are written to stderr (preserves --porcelain stdout contract) - Full error chain shown via alternate Display ({:#}) - All-providers-failed still surfaces as a real error - Addresses review feedback on #2936 Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/forge_api/src/api.rs | 11 ++++---- crates/forge_api/src/forge_api.rs | 2 +- crates/forge_app/src/app.rs | 37 ++++++------------------ crates/forge_main/src/ui.rs | 47 +++++++++++++++++++++++++++---- 4 files changed, 56 insertions(+), 41 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index ceef7975d9..e0013f0b42 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -23,12 +23,11 @@ pub trait API: Sync + Send { /// Provides a list of models available in the current environment async fn get_models(&self) -> Result>; - /// Provides models from all configured providers. Providers that - /// successfully return models are included in the result. If every - /// configured provider fails (e.g. due to an invalid API key), the - /// first error is returned so the caller sees the real underlying cause - /// rather than an empty list. - async fn get_all_provider_models(&self) -> Result>; + /// Provides models from all configured providers. Each element is + /// either a successful `ProviderModels` or an error for a provider + /// that failed (e.g. due to stale credentials), so callers can show + /// partial results alongside per-provider errors. + async fn get_all_provider_models(&self) -> Result>>; /// Provides a list of agents available in the current environment async fn get_agents(&self) -> Result>; diff --git a/crates/forge_api/src/forge_api.rs b/crates/forge_api/src/forge_api.rs index 8569b21d6b..129014eee5 100644 --- a/crates/forge_api/src/forge_api.rs +++ b/crates/forge_api/src/forge_api.rs @@ -84,7 +84,7 @@ impl< self.app().get_models().await } - async fn get_all_provider_models(&self) -> Result> { + async fn get_all_provider_models(&self) -> Result>> { self.app().get_all_provider_models().await } diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index d2ed57a4ea..7be35e0ff4 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -297,11 +297,11 @@ impl> ForgeAp /// Gets available models from all configured providers concurrently. /// - /// Returns a list of `ProviderModels` for each configured provider that - /// successfully returned models. If every configured provider fails (e.g. - /// due to an invalid API key), the first error encountered is returned so - /// the caller receives the real underlying cause rather than an empty list. - pub async fn get_all_provider_models(&self) -> Result> { + /// Returns one `Result` per configured provider so the + /// caller can display partial results alongside per-provider errors + /// (e.g. stale credentials on one provider should not hide models from + /// others). + pub async fn get_all_provider_models(&self) -> Result>> { let all_providers = self.services.get_all_providers().await?; // Build one future per configured provider, preserving the error on failure. @@ -312,6 +312,7 @@ impl> ForgeAp let provider_id = provider.id.clone(); let services = self.services.clone(); async move { + let pid = provider_id.clone(); let result: Result = async { let refreshed = services .provider_auth_service() @@ -321,33 +322,11 @@ impl> ForgeAp Ok(ProviderModels { provider_id, models }) } .await; - result + result.map_err(|e| e.context(format!("provider '{pid}'"))) } }) .collect(); - // Execute all provider fetches concurrently, tolerating partial failures. - let mut first_error: Option = None; - let mut successes = Vec::new(); - - for result in futures::future::join_all(futures).await { - match result { - Ok(provider_models) => successes.push(provider_models), - Err(err) => { - tracing::warn!(error = %err, "failed to fetch models from a configured provider"); - if first_error.is_none() { - first_error = Some(err); - } - } - } - } - - if successes.is_empty() - && let Some(err) = first_error - { - return Err(err); - } - - Ok(successes) + Ok(futures::future::join_all(futures).await) } } diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index fde42605d5..67a3515c64 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -19,7 +19,8 @@ use forge_app::{CommitResult, ToolResolver}; use forge_config::ForgeConfig; use forge_display::MarkdownFormat; use forge_domain::{ - AuthMethod, ChatResponseContent, ConsoleWriter, ContextMessage, Role, TitleFormat, UserCommand, + AuthMethod, ChatResponseContent, ConsoleWriter, ContextMessage, ProviderModels, Role, + TitleFormat, UserCommand, }; use forge_fs::ForgeFS; use forge_select::ForgeWidget; @@ -129,6 +130,37 @@ impl A + Send + Sync> UI self.spinner.ewrite_ln(title) } + /// Partitions provider model results into successes, writing + /// per-provider errors to stderr. If every provider failed, the + /// first error is returned so callers surface a real failure rather + /// than silently treating it as "no models configured". + fn collect_provider_models( + &mut self, + results: Vec>, + ) -> anyhow::Result> { + let mut models = Vec::new(); + let mut first_error: Option = None; + for result in results { + match result { + Ok(pm) => models.push(pm), + Err(err) => { + self.writeln_to_stderr( + TitleFormat::error(format!("{err:#}")).display().to_string(), + )?; + if first_error.is_none() { + first_error = Some(err); + } + } + } + } + if models.is_empty() { + if let Some(err) = first_error { + return Err(err); + } + } + Ok(models) + } + /// Helper to get provider for an optional agent, defaulting to the current /// active agent's provider async fn get_provider(&self, agent_id: Option) -> Result> { @@ -1217,13 +1249,15 @@ impl A + Send + Sync> UI async fn on_show_models(&mut self, porcelain: bool) -> anyhow::Result<()> { self.spinner.start(Some("Fetching Models"))?; - let mut all_provider_models = match self.api.get_all_provider_models().await { - Ok(provider_models) => provider_models, + let results = match self.api.get_all_provider_models().await { + Ok(results) => results, Err(err) => { self.spinner.stop(None)?; return Err(err); } }; + self.spinner.stop(None)?; + let mut all_provider_models = self.collect_provider_models(results)?; if all_provider_models.is_empty() { return Ok(()); @@ -2158,8 +2192,9 @@ impl A + Send + Sync> UI // Fetch models from ALL configured providers (matches shell plugin's // `forge list models --porcelain`), then optionally filter by provider. self.spinner.start(Some("Loading"))?; - let mut all_provider_models = self.api.get_all_provider_models().await?; + let results = self.api.get_all_provider_models().await?; self.spinner.stop(None)?; + let mut all_provider_models = self.collect_provider_models(results)?; // When a provider filter is specified (e.g. during onboarding after a // provider was just selected), restrict the list to that provider's @@ -2928,7 +2963,8 @@ impl A + Send + Sync> UI let (needs_model_selection, compatible_model) = match current_model { None => (true, None), Some(current_model) => { - let provider_models = self.api.get_all_provider_models().await?; + let results = self.api.get_all_provider_models().await?; + let provider_models = self.collect_provider_models(results)?; let model_available = provider_models .iter() .find(|pm| pm.provider_id == provider.id) @@ -3821,6 +3857,7 @@ impl A + Send + Sync> UI .get_all_provider_models() .await? .into_iter() + .filter_map(Result::ok) .find(|pm| &pm.provider_id == provider_id) .with_context(|| { format!("Provider '{provider_id}' not found or returned no models") From 2e4df8655e181ddb4cbee65730fa768d9d831fc8 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:31:25 +0000 Subject: [PATCH 5/6] [autofix.ci] apply automated fixes --- crates/forge_main/src/ui.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 67a3515c64..a46156b84a 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -153,11 +153,10 @@ impl A + Send + Sync> UI } } } - if models.is_empty() { - if let Some(err) = first_error { + if models.is_empty() + && let Some(err) = first_error { return Err(err); } - } Ok(models) } From 5f11d58f29b8cc3acd3c7b129858285f6b2f2c29 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 18:33:19 +0000 Subject: [PATCH 6/6] [autofix.ci] apply automated fixes (attempt 2/3) --- crates/forge_main/src/ui.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index a46156b84a..5363ed8dca 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -154,9 +154,10 @@ impl A + Send + Sync> UI } } if models.is_empty() - && let Some(err) = first_error { - return Err(err); - } + && let Some(err) = first_error + { + return Err(err); + } Ok(models) }