Skip to content

Fix API key fallback priority and improve error messages #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix API key fallback priority and improve error messages #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08badc9
Initial plan
Copilot Oct 6, 2025
46692e9
Initial analysis: API key fallback and error message improvements
Copilot Oct 6, 2025
d86f06f
Fix API key fallback priority and improve error messages
Copilot Oct 6, 2025
9a100d0
Update commit.rs
oleander Oct 6, 2025
f54ab7d
Fix CI test failure: make API key fallback test network-free
Copilot Oct 6, 2025
0829bf6
Eliminate all OpenAI library dependencies from API key fallback test
Copilot Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

106 changes: 57 additions & 49 deletions src/commit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use crate::multi_step_integration::{generate_commit_message_local, generate_comm
const INSTRUCTION_TEMPLATE: &str = include_str!("../resources/prompt.md");

/// Returns the instruction template for the AI model.
/// This template guides the model in generating appropriate commit messages.
///
/// # Returns
/// * `Result<String>` - The rendered template or an error
Expand Down Expand Up @@ -85,58 +84,32 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
.and_then(|s| s.max_commit_length)
.or(config::APP_CONFIG.max_commit_length);

// Check if we have a valid API key configuration
let has_valid_api_key = if let Some(custom_settings) = settings {
custom_settings
.openai_api_key
.as_ref()
.map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
.unwrap_or(false)
} else {
// Check environment variable or config
config::APP_CONFIG
.openai_api_key
.as_ref()
.map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
.unwrap_or(false)
|| std::env::var("OPENAI_API_KEY")
.map(|key| !key.is_empty())
.unwrap_or(false)
};

if !has_valid_api_key {
bail!("OpenAI API key not configured. Please set your API key using:\n git-ai config set openai-api-key <your-key>\nor set the OPENAI_API_KEY environment variable.");
}

// Use custom settings if provided
if let Some(custom_settings) = settings {
if let Some(api_key) = &custom_settings.openai_api_key {
if !api_key.is_empty() && api_key != "<PLACE HOLDER FOR YOUR API KEY>" {
match openai::create_openai_config(custom_settings) {
Ok(config) => {
let client = Client::with_config(config);
let model_str = model.to_string();
// Always try to create config - this will handle API key validation and fallback
match openai::create_openai_config(custom_settings) {
Ok(config) => {
let client = Client::with_config(config);
let model_str = model.to_string();

match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
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") {
bail!("Invalid OpenAI API key. Please check your API key configuration.");
}
log::warn!("Multi-step generation with custom settings failed: {e}");
if let Some(session) = debug_output::debug_session() {
session.set_multi_step_error(e.to_string());
}
}
}
}
match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
Ok(message) => return Ok(openai::Response { response: message }),
Err(e) => {
// If config creation fails due to API key, propagate the error
return 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") {
bail!("Invalid OpenAI API key. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable");
}
log::warn!("Multi-step generation with custom settings failed: {e}");
if let Some(session) = debug_output::debug_session() {
session.set_multi_step_error(e.to_string());
}
}
}
}
Err(e) => {
// If config creation fails due to API key, propagate the error
return Err(e);
}
}
} else {
// Try with default settings
Expand All @@ -150,7 +123,7 @@ pub async fn generate(patch: String, remaining_tokens: usize, model: Model, sett
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") {
bail!("Invalid OpenAI API key. Please check your API key configuration.");
bail!("Invalid OpenAI API key. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable");
}
log::warn!("Multi-step generation failed: {e}");
if let Some(session) = debug_output::debug_session() {
Expand Down Expand Up @@ -247,7 +220,7 @@ mod tests {
assert!(result.is_err());
let error_message = result.unwrap_err().to_string();
assert!(
error_message.contains("OpenAI API key not configured"),
error_message.contains("OpenAI API key not found") || error_message.contains("git-ai config set openai-api-key"),
"Expected error message about missing API key, got: {}",
error_message
);
Expand Down Expand Up @@ -276,9 +249,44 @@ mod tests {
assert!(result.is_err());
let error_message = result.unwrap_err().to_string();
assert!(
error_message.contains("OpenAI API key not configured"),
error_message.contains("OpenAI API key not found") || error_message.contains("git-ai config set openai-api-key"),
"Expected error message about invalid API key, got: {}",
error_message
);
}

#[test]
fn test_api_key_fallback_to_env() {
// Test the API key resolution logic without creating OpenAI config objects

// Set a valid-looking API key in environment
std::env::set_var("OPENAI_API_KEY", "sk-test123456789012345678901234567890123456789012345678");

// Test case 1: Placeholder in config should fall back to env var
let placeholder_key = Some("<PLACE HOLDER FOR YOUR API KEY>".to_string());
if let Some(key) = &placeholder_key {
if !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>" {
panic!("Should not use placeholder key");
} else {
// Should fall back to env var
let env_result = std::env::var("OPENAI_API_KEY");
assert!(env_result.is_ok(), "Environment variable should be available");
assert_eq!(env_result.unwrap(), "sk-test123456789012345678901234567890123456789012345678");
}
}

// Test case 2: Empty config should fall back to env var
let empty_key: Option<String> = None;
if empty_key.is_none() {
let env_result = std::env::var("OPENAI_API_KEY");
assert!(env_result.is_ok(), "Environment variable should be available for empty config");
}

// Clean up
std::env::remove_var("OPENAI_API_KEY");

// Verify environment variable is removed
let env_result_after_cleanup = std::env::var("OPENAI_API_KEY");
assert!(env_result_after_cleanup.is_err(), "Environment variable should be removed");
}
}
30 changes: 23 additions & 7 deletions src/openai.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,29 @@ pub async fn generate_commit_message(diff: &str) -> Result<String> {

/// Creates an OpenAI configuration from application settings
pub fn create_openai_config(settings: &AppConfig) -> Result<OpenAIConfig> {
let api_key = settings
.openai_api_key
.as_ref()
.ok_or_else(|| anyhow!("OpenAI API key not configured"))?;
// Try config first, then environment variable
let api_key = if let Some(key) = &settings.openai_api_key {
if !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>" {
key.clone()
} else {
// Try environment variable as fallback
std::env::var("OPENAI_API_KEY")
.map_err(|_| anyhow!(
"OpenAI API key not found. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable"
))?
}
} else {
// No config key, try environment variable
std::env::var("OPENAI_API_KEY")
.map_err(|_| anyhow!(
"OpenAI API key not found. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable"
))?
};

if api_key.is_empty() || api_key == "<PLACE HOLDER FOR YOUR API KEY>" {
return Err(anyhow!("Invalid OpenAI API key"));
if api_key.is_empty() {
return Err(anyhow!(
"OpenAI API key cannot be empty. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable"
));
}

let config = OpenAIConfig::new().with_api_key(api_key);
Expand Down Expand Up @@ -342,7 +358,7 @@ pub async fn call_with_config(request: Request, config: OpenAIConfig) -> Result<
// Check if it's an API key error - fail immediately without retrying
if let OpenAIError::ApiError(ref api_err) = &last_error.as_ref().unwrap() {
if api_err.code.as_deref() == Some("invalid_api_key") {
let error_msg = format!("Invalid OpenAI API key: {}", api_err.message);
let error_msg = format!("Invalid OpenAI API key: {}. Set via:\n1. git-ai config set openai-api-key <key>\n2. OPENAI_API_KEY environment variable", api_err.message);
log::error!("{error_msg}");
return Err(anyhow!(error_msg));
}
Expand Down
4 changes: 2 additions & 2 deletions tests/model_validation_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ fn test_model_as_ref() {
s.as_ref().to_string()
}

assert_eq!(takes_str_ref(&Model::GPT41), "gpt-4.1");
assert_eq!(takes_str_ref(&Model::GPT41Mini), "gpt-4.1-mini");
assert_eq!(takes_str_ref(Model::GPT41), "gpt-4.1");
assert_eq!(takes_str_ref(Model::GPT41Mini), "gpt-4.1-mini");
}

#[test]
Expand Down
Loading