Refactor error handling and module structure for authentication clarity

Author: oleander

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,9 +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") ||
-                   e.to_string().contains("OpenAI API authentication failed") {
+                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}");
@@ -151,9 +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") ||
-               e.to_string().contains("OpenAI API authentication failed") {
+            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,99 @@
+//! 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.
+
+use anyhow::Error;
+
+/// Checks if an error represents an OpenAI API authentication failure.
+///
+/// This function detects various authentication failure patterns including:
+/// - OpenAI-specific API key errors (invalid_api_key, incorrect API key)
+/// - Generic authentication/authorization failures
+/// - HTTP-level errors that typically indicate authentication issues when calling OpenAI
+///
+/// # 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 {
+  let msg = error.to_string().to_lowercase();
+
+  // OpenAI-specific API key errors
+  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 super::*;
+  use anyhow::anyhow;
+
+  #[test]
+  fn test_detects_invalid_api_key() {
+    let error = anyhow!("invalid_api_key: Incorrect API key provided");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_incorrect_api_key() {
+    let error = anyhow!("Incorrect API key provided: sk-xxxxx");
+    assert!(is_openai_auth_error(&error));
+  }
+
+  #[test]
+  fn test_detects_openai_auth_failed() {
+    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));
+  }
+}

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> {
@@ -90,14 +106,7 @@ pub async fn generate_commit_message_multi_step(
       }
       Err(e) => {
         // Check if it's an API key or authentication 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") ||
-           error_str.contains("authentication") ||
-           error_str.contains("unauthorized") ||
-           // Detect HTTP errors that typically indicate auth issues when calling OpenAI
-           (error_str.contains("http error") && error_str.contains("error sending request")) {
+        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);

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,9 +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") ||
-         e.to_string().contains("OpenAI API authentication failed") {
+      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

@@ -25,15 +25,15 @@ index 0000000..0000000
 
     // 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);
-    
-    // Now it should properly detect authentication failures
+
+    // Verify it returns the specific authentication error message with actionable guidance
     assert!(
-        error_message.contains("OpenAI API authentication failed") || 
-        error_message.contains("API key"),
-        "Expected error message to indicate authentication failure, got: {}",
+        error_message.contains("OpenAI API authentication failed") &&
+        error_message.contains("Please check your API key configuration"),
+        "Expected specific authentication error message with guidance, got: {}",
         error_message
     );
-}
+}