diff --git a/src/commands/install_hooks.rs b/src/commands/install_hooks.rs index 384de20c37..c893fdc393 100644 --- a/src/commands/install_hooks.rs +++ b/src/commands/install_hooks.rs @@ -1,6 +1,7 @@ use crate::config; use crate::daemon::DaemonConfig; use crate::error::GitAiError; +use crate::git::repository::{GitAuthorIdentity, global_git_config_committer_identity}; use crate::mdm::agents::get_all_installers; use crate::mdm::hook_installer::HookInstallerParams; use crate::mdm::skills_installer; @@ -8,6 +9,7 @@ use crate::mdm::spinner::{Spinner, print_diff}; use crate::mdm::utils::get_current_binary_path; use std::collections::{HashMap, HashSet}; use std::fs; +use std::io::{IsTerminal, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -379,6 +381,9 @@ fn run_hooks_install(options: &InstallOptions) -> Result // Get absolute path to the current binary let binary_path = get_current_binary_path()?; persist_install_config_with_values(&binary_path, options.dry_run, &install_config)?; + // Prompt before async_run_install so the interactive read never interleaves + // with spinner output. + maybe_prompt_and_save_author_identity(options, &install_config); let params = HookInstallerParams { binary_path }; // Run async operations and convert result. @@ -511,6 +516,132 @@ fn persist_install_config_with_values( Ok(true) } +const AUTHOR_PROMPT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); + +/// Prompt only for interactive, non-dry-run installs where an API key exists +/// (hosted usage) and no git-ai author override is configured yet. +fn should_prompt_for_author( + dry_run: bool, + interactive: bool, + author_configured: bool, + api_key_present: bool, +) -> bool { + !dry_run && interactive && !author_configured && api_key_present +} + +/// Best-effort interactive confirmation of the author identity used for +/// AI-authorship attribution. Never fails the install: any config error or +/// prompt timeout just skips. +fn maybe_prompt_and_save_author_identity(options: &InstallOptions, install_config: &InstallConfig) { + let opted_out = std::env::var("GIT_AI_NO_AUTHOR_PROMPT") + .is_ok_and(|v| !matches!(v.as_str(), "" | "0" | "false" | "False" | "FALSE")); + if opted_out { + return; + } + let interactive = (crate::utils::is_interactive_terminal() && std::io::stdout().is_terminal()) + || std::env::var_os("GIT_AI_TEST_FORCE_TTY").is_some(); + let Ok(file_config) = crate::config::load_file_config_public() else { + return; + }; + let author_configured = file_config + .author + .clone() + .is_some_and(|author| !author.normalized().is_empty()); + let api_key_present = std::env::var("GIT_AI_API_KEY").is_ok_and(|key| !key.is_empty()) + || file_config + .api_key + .as_deref() + .is_some_and(|key| !key.is_empty()) + || install_config.api_key.is_some(); + if !should_prompt_for_author( + options.dry_run, + interactive, + author_configured, + api_key_present, + ) { + return; + } + + // Default to the explicitly configured global git identity only: `git var + // GIT_COMMITTER_IDENT` fabricates a junk identity (user@hostname) on + // machines with no user.name/user.email, and Enter would persist it. + let default = global_git_config_committer_identity().unwrap_or_default(); + let Some(author) = prompt_author_identity( + &default, + || crate::utils::read_line_with_timeout(AUTHOR_PROMPT_TIMEOUT), + &mut std::io::stdout(), + ) else { + return; + }; + // Re-load right before saving: the prompt blocks on human input, and saving + // the pre-prompt snapshot would clobber any concurrent config change. + let Ok(mut file_config) = crate::config::load_file_config_public() else { + return; + }; + file_config.author = Some(author); + if let Err(e) = crate::config::save_file_config(&file_config) { + eprintln!("Warning: could not save author config (non-fatal): {e}"); + } +} + +/// Returns `Some(author)` to save, or `None` to skip (prompt timeout/EOF, or +/// nothing entered and no defaults). +fn prompt_author_identity( + default: &GitAuthorIdentity, + mut read_line: impl FnMut() -> Option, + out: &mut impl Write, +) -> Option { + let _ = writeln!( + out, + "Confirm the author identity git-ai will use for AI-authorship attribution\n\ + (press Enter to accept, or type a new value; each prompt auto-skips after {}s):", + AUTHOR_PROMPT_TIMEOUT.as_secs() + ); + let Some(name) = + prompt_author_field("Author name", default.name.as_deref(), &mut read_line, out) + else { + let _ = writeln!( + out, + " (no input — skipping; set later with `git-ai config set author.name ...`)" + ); + return None; + }; + let Some(email) = prompt_author_field( + "Author email", + default.email.as_deref(), + &mut read_line, + out, + ) else { + let _ = writeln!( + out, + " (no input — skipping; set later with `git-ai config set author.email ...`)" + ); + return None; + }; + let author = config::AuthorConfig { name, email }.normalized(); + (!author.is_empty()).then_some(author) +} + +/// Returns `None` on timeout/EOF (abort the prompt entirely — no further stdin +/// reads are allowed after a timeout), otherwise the entered value or the +/// default when the user just pressed Enter. +fn prompt_author_field( + label: &str, + default_value: Option<&str>, + read_line: &mut impl FnMut() -> Option, + out: &mut impl Write, +) -> Option> { + let _ = write!(out, " {label} [{}]: ", default_value.unwrap_or("")); + let _ = out.flush(); + let input = read_line()?; + let input = input.trim(); + Some(if input.is_empty() { + default_value.map(str::to_string) + } else { + Some(input.to_string()) + }) +} + fn detect_install_git_path(binary_path: &Path) -> Option { let install_dir = binary_path.parent()?; @@ -1465,4 +1596,129 @@ mod tests { assert_eq!(parse_git_version("not a git version"), None); assert_eq!(parse_git_version(""), None); } + + // ========================================================================= + // Author identity prompt + // ========================================================================= + + fn scripted_reader(responses: Vec>) -> impl FnMut() -> Option { + let mut responses: std::collections::VecDeque> = responses + .into_iter() + .map(|r| r.map(str::to_string)) + .collect(); + move || { + responses + .pop_front() + .expect("prompt read past scripted input") + } + } + + fn git_identity(name: Option<&str>, email: Option<&str>) -> GitAuthorIdentity { + GitAuthorIdentity { + name: name.map(str::to_string), + email: email.map(str::to_string), + } + } + + #[test] + fn should_prompt_for_author_requires_all_conditions() { + assert!(should_prompt_for_author(false, true, false, true)); + assert!(!should_prompt_for_author(true, true, false, true)); + assert!(!should_prompt_for_author(false, false, false, true)); + assert!(!should_prompt_for_author(false, true, true, true)); + assert!(!should_prompt_for_author(false, true, false, false)); + } + + #[test] + fn prompt_author_identity_enter_confirms_git_defaults() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + let author = prompt_author_identity( + &default, + scripted_reader(vec![Some(""), Some("")]), + &mut Vec::new(), + ) + .expect("confirming defaults must produce an author to save"); + assert_eq!(author.name.as_deref(), Some("Jane Doe")); + assert_eq!(author.email.as_deref(), Some("jane@example.com")); + } + + #[test] + fn prompt_author_identity_typed_input_overrides_defaults() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + let author = prompt_author_identity( + &default, + scripted_reader(vec![Some("Alice"), Some("alice@example.com")]), + &mut Vec::new(), + ) + .unwrap(); + assert_eq!(author.name.as_deref(), Some("Alice")); + assert_eq!(author.email.as_deref(), Some("alice@example.com")); + } + + #[test] + fn prompt_author_identity_mixed_confirm_and_override() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + let author = prompt_author_identity( + &default, + scripted_reader(vec![Some(""), Some("alice@example.com")]), + &mut Vec::new(), + ) + .unwrap(); + assert_eq!(author.name.as_deref(), Some("Jane Doe")); + assert_eq!(author.email.as_deref(), Some("alice@example.com")); + } + + #[test] + fn prompt_author_identity_timeout_on_first_prompt_skips_and_stops_reading() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + // A second read would panic in the scripted reader: after a timeout no + // further stdin reads are allowed. + let author = prompt_author_identity(&default, scripted_reader(vec![None]), &mut Vec::new()); + assert_eq!(author, None); + } + + #[test] + fn prompt_author_identity_timeout_on_second_prompt_saves_nothing() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + let author = prompt_author_identity( + &default, + scripted_reader(vec![Some("Alice"), None]), + &mut Vec::new(), + ); + assert_eq!(author, None); + } + + #[test] + fn prompt_author_identity_all_empty_returns_none() { + let default = git_identity(None, None); + let author = prompt_author_identity( + &default, + scripted_reader(vec![Some(""), Some("")]), + &mut Vec::new(), + ); + assert_eq!( + author, None, + "nothing to save when defaults and input are empty" + ); + } + + #[test] + fn prompt_author_identity_shows_defaults_in_brackets() { + let default = git_identity(Some("Jane Doe"), Some("jane@example.com")); + let mut out = Vec::new(); + prompt_author_identity( + &default, + scripted_reader(vec![Some(""), Some("")]), + &mut out, + ); + let out = String::from_utf8(out).unwrap(); + assert!( + out.contains("Author name [Jane Doe]:"), + "prompt output:\n{out}" + ); + assert!( + out.contains("Author email [jane@example.com]:"), + "prompt output:\n{out}" + ); + } } diff --git a/tests/integration/install_hooks_comprehensive.rs b/tests/integration/install_hooks_comprehensive.rs index b54270be96..9bc0e61957 100644 --- a/tests/integration/install_hooks_comprehensive.rs +++ b/tests/integration/install_hooks_comprehensive.rs @@ -375,6 +375,164 @@ fn install_hooks_wsl_dry_run_does_not_invoke_wsl() { ); } +// ============================================================================== +// Author identity prompt tests +// ============================================================================== + +fn git_ai_config_json(repo: &TestRepo) -> serde_json::Value { + let path = repo.test_home_path().join(".git-ai").join("config.json"); + serde_json::from_str(&fs::read_to_string(path).unwrap()).unwrap() +} + +fn assert_install_hooks_skips_author_prompt( + output: &std::process::Output, + repo: &TestRepo, + context: &str, +) { + assert!( + output.status.success(), + "install-hooks failed ({context}):\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("Author name"), + "author prompt must be skipped ({context}):\n{stdout}" + ); + assert!( + git_ai_config_json(repo).get("author").is_none(), + "no author must be written to the config ({context})" + ); +} + +#[test] +fn install_hooks_non_tty_skips_author_prompt() { + let repo = TestRepo::new_with_daemon_scope(DaemonTestScope::NoDaemon); + + let output = repo + .git_ai_command_without_pre_sync_for_test( + &["install-hooks"], + &[("GIT_AI_API_KEY", "test-key")], + ) + .output() + .expect("run git-ai install-hooks"); + + assert_install_hooks_skips_author_prompt(&output, &repo, "non-tty stdin"); +} + +#[test] +fn install_hooks_forced_tty_prompts_and_saves_author() { + use std::io::Write as _; + + let repo = TestRepo::new_with_daemon_scope(DaemonTestScope::NoDaemon); + + let mut command = repo.git_ai_command_without_pre_sync_for_test( + &["install-hooks"], + &[ + ("GIT_AI_API_KEY", "test-key"), + ("GIT_AI_TEST_FORCE_TTY", "1"), + ], + ); + command + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + let mut child = command.spawn().expect("spawn git-ai install-hooks"); + child + .stdin + .take() + .unwrap() + .write_all(b"Alice\nalice@example.com\n") + .unwrap(); + let output = child.wait_with_output().expect("wait for install-hooks"); + + assert!( + output.status.success(), + "install-hooks failed:\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Author name") && stdout.contains("Author email"), + "missing author prompt:\n{stdout}" + ); + + let config = git_ai_config_json(&repo); + assert_eq!(config["author"]["name"], "Alice"); + assert_eq!(config["author"]["email"], "alice@example.com"); +} + +#[test] +fn install_hooks_forced_tty_author_already_set_skips_prompt() { + let repo = TestRepo::new_with_daemon_scope(DaemonTestScope::NoDaemon); + let config_path = repo.test_home_path().join(".git-ai").join("config.json"); + let mut config = git_ai_config_json(&repo); + config["author"] = serde_json::json!({"name": "Preset Name"}); + fs::write(&config_path, serde_json::to_string_pretty(&config).unwrap()).unwrap(); + + let output = repo + .git_ai_command_without_pre_sync_for_test( + &["install-hooks"], + &[ + ("GIT_AI_API_KEY", "test-key"), + ("GIT_AI_TEST_FORCE_TTY", "1"), + ], + ) + .output() + .expect("run git-ai install-hooks"); + + assert!( + output.status.success(), + "install-hooks failed:\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + !stdout.contains("Author name"), + "author prompt must be skipped when author.name is already set:\n{stdout}" + ); + assert_eq!( + git_ai_config_json(&repo)["author"], + serde_json::json!({"name": "Preset Name"}), + "a partially-set author must be left untouched" + ); +} + +#[test] +fn install_hooks_forced_tty_without_api_key_skips_prompt() { + let repo = TestRepo::new_with_daemon_scope(DaemonTestScope::NoDaemon); + + let mut command = repo.git_ai_command_without_pre_sync_for_test( + &["install-hooks"], + &[("GIT_AI_TEST_FORCE_TTY", "1")], + ); + command.env_remove("GIT_AI_API_KEY").env_remove("API_KEY"); + let output = command.output().expect("run git-ai install-hooks"); + + assert_install_hooks_skips_author_prompt(&output, &repo, "no api key"); +} + +#[test] +fn install_hooks_dry_run_forced_tty_skips_prompt() { + let repo = TestRepo::new_with_daemon_scope(DaemonTestScope::NoDaemon); + + let output = repo + .git_ai_command_without_pre_sync_for_test( + &["install-hooks", "--dry-run"], + &[ + ("GIT_AI_API_KEY", "test-key"), + ("GIT_AI_TEST_FORCE_TTY", "1"), + ], + ) + .output() + .expect("run git-ai install-hooks --dry-run"); + + assert_install_hooks_skips_author_prompt(&output, &repo, "dry run"); +} + #[test] #[cfg(not(windows))] fn install_hooks_detects_cline_from_vscode_server_extension_manifest() {