fix(provider): prevent retrying 401/403 errors and enhance error mess…

crates/forge_repo/src/provider/openai.rs

@@ -20,6 +20,21 @@ use crate::provider::utils::{create_headers, format_http_context, join_url};
 
 /// Enhances error messages with provider-specific helpful information
 fn enhance_error(error: anyhow::Error, provider_id: &ProviderId) -> anyhow::Error {
+    // Check for 401/403 errors — these indicate an invalid API key or
+    // insufficient permissions. Retrying won't fix these.
+    let status = crate::provider::retry::get_api_status_code(&error);
+    if status == Some(401) || status == Some(403) {
+        let provider_name = provider_id.to_string();
+        let action = if status == Some(401) {
+            format!("Your {provider_name} API key is invalid. Update it in settings and try again.")
+        } else {
+            format!(
+                "Your {provider_name} API key does not have permission to access this resource. Check your account permissions."
+            )
+        };
+        return error.context(action);
+    }
+
     // GitHub Copilot specific error enhancements
     if *provider_id == ProviderId::GITHUB_COPILOT {
         let error_string = format!("{:#}", error);
@@ -364,11 +379,12 @@ impl<F: HttpInfra + EnvironmentInfra<Config = forge_config::ForgeConfig> + 'stat
 
     async fn models(&self, provider: Provider<Url>) -> anyhow::Result<Vec<Model>> {
         let retry_config = self.infra.get_config()?.retry.unwrap_or_default();
+        let provider_id = provider.id.clone();
         let provider_client = OpenAIProvider::new(provider, self.infra.clone());
         provider_client
             .models()
             .await
-            .map_err(|e| into_retry(e, &retry_config))
+            .map_err(|e| enhance_error(into_retry(e, &retry_config), &provider_id))
             .context("Failed to fetch models from OpenAI-compatible provider")
     }
 }

crates/forge_repo/src/provider/retry.rs

@@ -6,13 +6,24 @@ use forge_config::RetryConfig;
 const TRANSPORT_ERROR_CODES: [&str; 3] = ["ERR_STREAM_PREMATURE_CLOSE", "ECONNRESET", "ETIMEDOUT"];
 const OPENAI_OVERLOADED_ERROR_CODE: &str = "server_is_overloaded";
 
+/// HTTP status codes that must never be retried — they indicate a configuration
+/// problem (bad API key, insufficient permissions) that no amount of retrying
+/// will fix.
+const NEVER_RETRY_STATUS_CODES: [u16; 2] = [401, 403];
+
 pub fn into_retry(error: anyhow::Error, retry_config: &RetryConfig) -> anyhow::Error {
+    // 401/403 are always fatal — retrying won't fix a bad API key or missing
+    // permissions.
     if let Some(code) = get_req_status_code(&error)
         .or(get_event_req_status_code(&error))
         .or(get_api_status_code(&error))
-        && retry_config.status_codes.contains(&code)
     {
-        return DomainError::Retryable(error).into();
+        if NEVER_RETRY_STATUS_CODES.contains(&code) {
+            return error;
+        }
+        if retry_config.status_codes.contains(&code) {
+            return DomainError::Retryable(error).into();
+        }
     }
 
     if is_api_transport_error(&error)
@@ -188,6 +199,22 @@ mod tests {
         anyhow::Error::from(Error::Response(error))
     }
 
+    #[test]
+    fn test_never_retry_401_403_even_if_in_config() {
+        // 401/403 must never be retryable even if the config includes them
+        let retry_config = fixture_retry_config(vec![401, 403, 429, 500]);
+
+        let error_401 = fixture_response_error(Some(401));
+        assert!(!is_retryable(into_retry(error_401, &retry_config)));
+
+        let error_403 = fixture_response_error(Some(403));
+        assert!(!is_retryable(into_retry(error_403, &retry_config)));
+
+        // 500 should still be retryable when in config
+        let error_500 = fixture_response_error(Some(500));
+        assert!(is_retryable(into_retry(error_500, &retry_config)));
+    }
+
     #[test]
     fn test_into_retry_with_status_codes() {
         let retry_config = fixture_retry_config(vec![429, 500, 502, 503, 504]);