From ac876ecef21bd92fc10550243a47b25dcd64b135 Mon Sep 17 00:00:00 2001 From: M Date: Fri, 27 Mar 2026 18:13:38 +0600 Subject: [PATCH 1/3] fix: surface provider model fetch failures during login Return provider-specific errors when configured providers fail to refresh credentials or fetch models instead of silently skipping them. This keeps OpenAI-compatible and Ollama login/model-selection failures visible to the user instead of degrading into an empty model list. --- crates/forge_api/src/api.rs | 4 ++-- crates/forge_app/src/app.rs | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/forge_api/src/api.rs b/crates/forge_api/src/api.rs index 51ee3153ec..61ed75b368 100644 --- a/crates/forge_api/src/api.rs +++ b/crates/forge_api/src/api.rs @@ -23,8 +23,8 @@ 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 fail to - /// return models are silently skipped. + /// Provides models from all configured providers. + /// Returns an error if any configured provider fails to return models. async fn get_all_provider_models(&self) -> Result>; /// Provides a list of agents available in the current environment diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index 9dae5b259f..60d24c1294 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -1,7 +1,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::Local; use forge_domain::*; use forge_stream::MpscStream; @@ -274,8 +274,8 @@ impl ForgeApp { /// Gets available models from all configured providers concurrently. /// /// Returns a list of `ProviderModels` for each configured provider. - /// All providers are queried in parallel; providers that fail to - /// return models are silently skipped. + /// All providers are queried in parallel and the first provider error is + /// returned to the caller. pub async fn get_all_provider_models(&self) -> Result> { let all_providers = self.services.get_all_providers().await?; @@ -291,21 +291,21 @@ impl ForgeApp { .provider_auth_service() .refresh_provider_credential(provider) .await - .ok()?; - let models = services.models(refreshed).await.ok()?; - Some(ProviderModels { provider_id, models }) + .with_context(|| { + format!("Failed to refresh credentials for provider '{provider_id}'") + })?; + let models = services.models(refreshed).await.with_context(|| { + format!("Failed to fetch models for provider '{provider_id}'") + })?; + + Ok(ProviderModels { provider_id, models }) } }) .collect(); - // Execute all provider fetches concurrently and collect successful results - let results = futures::future::join_all(futures) - .await - .into_iter() - .flatten() - .collect(); - - Ok(results) + // Execute all provider fetches concurrently and fail fast on errors so + // callers such as login/model selection can surface the root cause. + futures::future::try_join_all(futures).await } pub async fn read_workflow(&self, path: Option<&Path>) -> Result { From 34d7eee2189638e39e3cd2179241c9adbbc9e036 Mon Sep 17 00:00:00 2001 From: M Date: Sat, 28 Mar 2026 23:12:50 +0600 Subject: [PATCH 2/3] propagate credential refresh errors instead of silently swallowing --- crates/forge_services/src/provider_auth.rs | 58 ++++++++++++++-------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/crates/forge_services/src/provider_auth.rs b/crates/forge_services/src/provider_auth.rs index 72b8693632..0e5edf06b0 100644 --- a/crates/forge_services/src/provider_auth.rs +++ b/crates/forge_services/src/provider_auth.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use std::time::Duration; +use anyhow::Context; use forge_app::{AuthStrategy, ProviderAuthService, StrategyFactory}; use forge_domain::{ AuthContextRequest, AuthContextResponse, AuthMethod, Provider, ProviderId, ProviderRepository, @@ -136,7 +137,8 @@ where /// Checks if credential needs refresh (5 minute buffer before expiry), /// iterates through provider's auth methods, and attempts to refresh. /// Returns the provider with updated credentials, or original if refresh - /// fails or isn't needed. + /// isn't needed. Returns an error if refresh is needed but fails for all + /// configured auth methods. async fn refresh_provider_credential( &self, mut provider: Provider, @@ -146,6 +148,8 @@ where let buffer = chrono::Duration::minutes(5); if credential.needs_refresh(buffer) { + let mut last_error: Option = None; + // Iterate through auth methods and try to refresh for auth_method in &provider.auth_methods { match auth_method { @@ -169,37 +173,47 @@ where }; // Create strategy and refresh credential - if let Ok(strategy) = self.infra.create_auth_strategy( + let strategy = match self.infra.create_auth_strategy( provider.id.clone(), auth_method.clone(), required_params, ) { - match strategy.refresh(&existing_credential).await { - Ok(refreshed) => { - // Store refreshed credential - if self - .infra - .upsert_credential(refreshed.clone()) - .await - .is_err() - { - continue; - } - - // Update provider with refreshed credential - provider.credential = Some(refreshed); - break; // Success, stop trying other methods - } - Err(_) => { - // If refresh fails, continue with - // existing credentials - } + Ok(s) => s, + Err(err) => { + last_error = Some(err); + continue; + } + }; + + match strategy.refresh(&existing_credential).await { + Ok(refreshed) => { + // Store refreshed credential + self.infra + .upsert_credential(refreshed.clone()) + .await?; + + // Update provider with refreshed credential + provider.credential = Some(refreshed); + return Ok(provider); + } + Err(err) => { + last_error = Some(err); } } } _ => {} } } + + // If we got here, all auth methods failed + if let Some(err) = last_error { + return Err(err).with_context(|| { + format!( + "Failed to refresh credentials for provider '{}'", + provider.id + ) + }); + } } } From 57f8d7b00fc2a3baf192313ed955be106201f3b1 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 28 Mar 2026 17:14:38 +0000 Subject: [PATCH 3/3] [autofix.ci] apply automated fixes --- crates/forge_services/src/provider_auth.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/forge_services/src/provider_auth.rs b/crates/forge_services/src/provider_auth.rs index 0e5edf06b0..7f796d28ef 100644 --- a/crates/forge_services/src/provider_auth.rs +++ b/crates/forge_services/src/provider_auth.rs @@ -188,9 +188,7 @@ where match strategy.refresh(&existing_credential).await { Ok(refreshed) => { // Store refreshed credential - self.infra - .upsert_credential(refreshed.clone()) - .await?; + self.infra.upsert_credential(refreshed.clone()).await?; // Update provider with refreshed credential provider.credential = Some(refreshed);