Fix API key errors: propagate authentication failures as errors instead of warnings

src/commit.rs

@@ -80,7 +80,11 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
     bail!("Maximum token count must be greater than zero")
   }
 
-  // Try multi-step approach first
+  // Try multi-step approach first (see multi_step_integration.rs for details)
+  //
+  // Error handling strategy:
+  // - Authentication errors are propagated immediately using is_openai_auth_error()
+  // - Other errors trigger fallback to local multi-step or single-step generation
   let max_length = settings
     .and_then(|s| s.max_commit_length)
     .or(config::APP_CONFIG.max_commit_length);
@@ -121,7 +125,7 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
               Ok(message) => return Ok(openai::Response { response: message }),
               Err(e) => {
                 // Check if it's an API key error
-                if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
+                if crate::error::is_openai_auth_error(&e) {
                   bail!("Invalid OpenAI API key. Please check your API key configuration.");
                 }
                 log::warn!("Multi-step generation with custom settings failed: {e}");
@@ -149,7 +153,7 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
           Ok(message) => return Ok(openai::Response { response: message }),
           Err(e) => {
             // Check if it's an API key error
-            if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
+            if crate::error::is_openai_auth_error(&e) {
               bail!("Invalid OpenAI API key. Please check your API key configuration.");
             }
             log::warn!("Multi-step generation failed: {e}");

src/error.rs

@@ -0,0 +1,207 @@
+//! Error handling utilities for the git-ai CLI tool.
+//!
+//! This module provides helpers for detecting and handling specific error types,
+//! particularly authentication failures from the OpenAI API.
+//!
+//! # OpenAI Error Structure
+//!
+//! According to the official async-openai documentation:
+//! - `OpenAIError::ApiError(ApiError)` contains structured error information from OpenAI
+//! - `ApiError` has fields: `message`, `type`, `param`, and `code`
+//! - Authentication errors have `code` set to `"invalid_api_key"`
+//! - `OpenAIError::Reqwest(Error)` contains HTTP-level errors (connection issues, etc.)
+//!
+//! Reference: https://docs.rs/async-openai/latest/async_openai/error/
+
+use anyhow::Error;
+use async_openai::error::OpenAIError;
+
+/// Checks if an error represents an OpenAI API authentication failure.
+///
+/// This function detects authentication failures by checking for:
+/// 1. **Structured API errors** (preferred): Checks if the error contains an `OpenAIError::ApiError`
+///    with `code` field set to `"invalid_api_key"` - this is the official OpenAI error code
+///    for authentication failures.
+/// 2. **String-based fallback**: As a fallback, checks for authentication-related keywords in
+///    the error message for cases where the error has been wrapped or converted to a string.
+///
+/// This approach is based on the official OpenAI API error codes documentation and the
+/// async-openai Rust library structure.
+///
+/// # Arguments
+///
+/// * `error` - The error to check
+///
+/// # Returns
+///
+/// `true` if the error appears to be an authentication failure, `false` otherwise
+///
+/// # Examples
+///
+/// ```
+/// use anyhow::anyhow;
+/// use ai::error::is_openai_auth_error;
+///
+/// let error = anyhow!("invalid_api_key: Incorrect API key provided");
+/// assert!(is_openai_auth_error(&error));
+/// ```
+pub fn is_openai_auth_error(error: &Error) -> bool {
+  // First, try to downcast to OpenAIError for accurate detection
+  if let Some(openai_err) = error.downcast_ref::<OpenAIError>() {
+    match openai_err {
+      // Official OpenAI API error with structured error code
+      OpenAIError::ApiError(api_err) => {
+        // Check for the official invalid_api_key error code
+        if api_err.code.as_deref() == Some("invalid_api_key") {
+          return true;
+        }
+        // Also check for authentication-related types
+        if let Some(err_type) = &api_err.r#type {
+          if err_type.contains("authentication") || err_type.contains("invalid_request_error") {
+            // For invalid_request_error, check if the message mentions API key
+            if err_type == "invalid_request_error" && api_err.message.to_lowercase().contains("api key") {
+              return true;
+            }
+          }
+        }
+      }
+      // HTTP-level errors (connection failures, malformed requests, etc.)
+      OpenAIError::Reqwest(_) => {
+        // Reqwest errors for auth issues typically manifest as connection errors
+        // when the API key format is completely invalid (e.g., "dl://BA7...")
+        let msg = error.to_string().to_lowercase();
+        if msg.contains("error sending request") || msg.contains("connection") {
+          return true;
+        }
+      }
+      _ => {}
+    }
+  }
+
+  // Fallback: String-based detection for wrapped errors
+  let msg = error.to_string().to_lowercase();
+
+  // OpenAI-specific API key errors (from API responses)
+  msg.contains("invalid_api_key") ||
+  msg.contains("incorrect api key") ||
+  msg.contains("openai api authentication failed") ||
+
+  // Generic auth failures (scoped to avoid false positives)
+  (msg.contains("authentication") && msg.contains("openai")) ||
+  (msg.contains("unauthorized") && msg.contains("openai")) ||
+
+  // HTTP errors that typically indicate auth issues with OpenAI
+  // This pattern catches connection issues when the API key is malformed
+  (msg.contains("http error") && msg.contains("error sending request"))
+}
+
+#[cfg(test)]
+mod tests {
+  use anyhow::anyhow;
+  use async_openai::error::{ApiError, OpenAIError};
+
+  use super::*;
+
+  // Tests for structured OpenAIError detection (preferred method)
+
+  #[test]
+  fn test_detects_structured_invalid_api_key() {
+    let api_error = ApiError {
+      message: "Incorrect API key provided: dl://BA7...".to_string(),
+      r#type:  Some("invalid_request_error".to_string()),
+      param:   None,
+      code:    Some("invalid_api_key".to_string())
+    };
+    let openai_error = OpenAIError::ApiError(api_error);
+    let error: anyhow::Error = openai_error.into();
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_invalid_request_with_api_key_message() {
+    let api_error = ApiError {
+      message: "You must provide a valid API key".to_string(),
+      r#type:  Some("invalid_request_error".to_string()),
+      param:   None,
+      code:    None
+    };
+    let openai_error = OpenAIError::ApiError(api_error);
+    let error: anyhow::Error = openai_error.into();
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_reqwest_error_sending_request() {
+    // Simulate a wrapped reqwest error by using anyhow
+    // In production, malformed API keys cause "error sending request" from reqwest
+    let error = anyhow!("http error: error sending request");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_ignores_structured_non_auth_error() {
+    let api_error = ApiError {
+      message: "Model not found".to_string(),
+      r#type:  Some("invalid_request_error".to_string()),
+      param:   Some("model".to_string()),
+      code:    Some("model_not_found".to_string())
+    };
+    let openai_error = OpenAIError::ApiError(api_error);
+    let error: anyhow::Error = openai_error.into();
+    assert!(!is_openai_auth_error(&error));
+  }
+
+  // Tests for string-based fallback detection (for wrapped errors)
+
+  #[test]
+  fn test_detects_invalid_api_key_string() {
+    let error = anyhow!("invalid_api_key: Incorrect API key provided");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_incorrect_api_key_string() {
+    let error = anyhow!("Incorrect API key provided: sk-xxxxx");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_openai_auth_failed_string() {
+    let error = anyhow!("OpenAI API authentication failed: http error");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_http_error_sending_request() {
+    let error = anyhow!("http error: error sending request");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_openai_specific_auth() {
+    let error = anyhow!("OpenAI authentication failed");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_ignores_generic_auth_errors() {
+    // Should not match generic auth errors without OpenAI context
+    let error = anyhow!("Database authentication timeout");
+    assert!(!is_openai_auth_error(&error));
+
+    let error = anyhow!("OAuth2 unauthorized redirect");
+    assert!(!is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_ignores_unrelated_errors() {
+    let error = anyhow!("File not found");
+    assert!(!is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_ignores_non_auth_openai_errors() {
+    let error = anyhow!("OpenAI rate limit exceeded");
+    assert!(!is_openai_auth_error(&error));
+  }
+}

src/lib.rs

@@ -1,17 +1,18 @@
 pub mod commit;
 pub mod config;
-pub mod hook;
-pub mod style;
-pub mod model;
+pub mod debug_output;
+pub mod error;
 pub mod filesystem;
-pub mod openai;
-pub mod profiling;
 pub mod function_calling;
+pub mod generation;
+pub mod hook;
+pub mod model;
 pub mod multi_step_analysis;
 pub mod multi_step_integration;
+pub mod openai;
+pub mod profiling;
 pub mod simple_multi_step;
-pub mod debug_output;
-pub mod generation;
+pub mod style;
 
 // Re-exports
 pub use profiling::Profile;

src/multi_step_integration.rs

@@ -20,6 +20,22 @@ pub struct ParsedFile {
 }
 
 /// Main entry point for multi-step commit message generation
+///
+/// This function uses a sophisticated divide-and-conquer approach:
+/// 1. Parse the diff into individual files
+/// 2. Analyze each file concurrently using the OpenAI API
+/// 3. Calculate impact scores for each file
+/// 4. Generate multiple commit message candidates
+/// 5. Select the best message based on impact scores
+///
+/// # Error Handling
+///
+/// Authentication failures are detected early and propagated immediately to provide
+/// clear feedback to users. Uses [`crate::error::is_openai_auth_error`] to identify
+/// API key issues and other authentication problems.
+///
+/// Other errors (non-auth) are logged as warnings and processing continues with
+/// remaining files to maximize the chance of generating a useful commit message.
 pub async fn generate_commit_message_multi_step(
   client: &Client<OpenAIConfig>, model: &str, diff_content: &str, max_length: Option<usize>
 ) -> Result<String> {
@@ -89,10 +105,12 @@ pub async fn generate_commit_message_multi_step(
         file_analyses.push((file, analysis));
       }
       Err(e) => {
-        // Check if it's an API key error - if so, propagate it immediately
-        let error_str = e.to_string();
-        if error_str.contains("invalid_api_key") || error_str.contains("Incorrect API key") || error_str.contains("Invalid API key") {
-          return Err(e);
+        // Check if it's an API key or authentication error - if so, propagate it immediately
+        if crate::error::is_openai_auth_error(&e) {
+          return Err(anyhow::anyhow!(
+            "OpenAI API authentication failed: {}. Please check your API key configuration.",
+            e
+          ));
         }
         log::warn!("Failed to analyze file {}: {}", file.path, e);
         // Continue with other files even if one fails

src/openai.rs

@@ -198,6 +198,16 @@ fn truncate_to_fit(text: &str, max_tokens: usize, model: &Model) -> Result<Strin
 }
 
 /// Calls the OpenAI API with the provided configuration
+///
+/// Implements a fallback strategy:
+/// 1. Try multi-step analysis approach (default)
+/// 2. Fall back to single-step if multi-step fails (except for auth errors)
+///
+/// # Error Handling
+///
+/// Authentication errors detected by [`crate::error::is_openai_auth_error`] are
+/// propagated immediately without attempting fallback, ensuring users get clear
+/// feedback about API key issues rather than confusing secondary errors.
 pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result<Response> {
   profile!("OpenAI API call with custom config");
 
@@ -209,7 +219,7 @@ pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result<
     Ok(message) => return Ok(Response { response: message }),
     Err(e) => {
       // Check if it's an API key error and propagate it
-      if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
+      if crate::error::is_openai_auth_error(&e) {
         return Err(e);
       }
       log::warn!("Multi-step approach failed, falling back to single-step: {e}");

tests/api_key_error_test.rs

@@ -0,0 +1,45 @@
+use ai::commit;
+use ai::config::AppConfig;
+use ai::model::Model;
+
+#[tokio::test]
+async fn test_invalid_api_key_propagates_error() {
+  // Initialize logging to capture warnings
+  let _ = env_logger::builder().is_test(true).try_init();
+
+  // Ensure no API key is available from environment to force early validation failure
+  let original_key = std::env::var("OPENAI_API_KEY").ok();
+  std::env::remove_var("OPENAI_API_KEY");
+
+  // Create settings with an invalid API key that fails early validation (no network calls)
+  let settings = AppConfig {
+    openai_api_key: Some("".to_string()), // Empty string triggers early validation failure
+    model: Some("gpt-4o-mini".to_string()),
+    max_tokens: Some(1024),
+    max_commit_length: Some(72),
+    timeout: Some(30)
+  };
+
+  let example_diff = "diff --git a/test.txt b/test.txt\n+Hello World".to_string();
+
+  // This should fail with an API key error, not log a warning and continue
+  let result = commit::generate(example_diff, 1024, Model::GPT41Mini, Some(&settings)).await;
+
+  // Restore original environment variable if it existed
+  if let Some(key) = original_key {
+    std::env::set_var("OPENAI_API_KEY", key);
+  }
+
+  // Verify the behavior - it should return an error, not continue with other files
+  assert!(result.is_err(), "Expected API key error to be propagated as error, not warning");
+
+  let error_message = result.unwrap_err().to_string();
+  println!("Actual error message: '{}'", error_message);
+
+  // The error should indicate that the API key is not configured (early validation without network calls)
+  assert!(
+    error_message.contains("OpenAI API key not configured") || error_message.contains("Invalid OpenAI API key"),
+    "Expected error message to indicate API key configuration issue, got: {}",
+    error_message
+  );
+}