diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 5af1cdabb2..b5b706f296 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -61,7 +61,21 @@ pub fn handle_git(args: &[String]) { return; } - let parsed = parse_git_cli_args(args); + let raw_parsed = parse_git_cli_args(args); + + // Preserve the built-in read-only fast path before attempting alias + // resolution. Aliases need a repository config lookup, but commands that are + // already known to be read-only should stay as cheap as they were before. + if is_read_only_invocation(&raw_parsed) { + let exit_status = proxy_to_git(args, false, None); + exit_with_status(exit_status); + } + + let repository = find_repository(&raw_parsed.global_args).ok(); + let parsed = repository + .as_ref() + .and_then(|repo| resolve_alias_invocation(&raw_parsed, repo)) + .unwrap_or_else(|| raw_parsed.clone()); // Read-only invocations don't need wrapper state (the daemon fast-paths // their trace events and never processes them through the normalizer). @@ -72,14 +86,7 @@ pub fn handle_git(args: &[String]) { // so that subcommand-gated read-only calls like `git stash list` and // `git worktree list` are also suppressed — these account for thousands // of Zed IDE invocations per session. - let is_read_only = { - let subcommand = parsed.command_args.first().map(String::as_str); - parsed.command.as_deref().is_some_and(|cmd| { - crate::git::command_classification::is_definitely_read_only_invocation(cmd, subcommand) - }) - }; - - if is_read_only { + if is_read_only_invocation(&parsed) { let exit_status = proxy_to_git(args, false, None); exit_with_status(exit_status); } @@ -114,7 +121,6 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } - let repository = find_repository(&parsed.global_args).ok(); let worktree = repository.as_ref().and_then(|r| r.workdir().ok()); let pre_state = worktree @@ -146,7 +152,13 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } -#[cfg(feature = "test-support")] +fn is_read_only_invocation(parsed: &ParsedGitInvocation) -> bool { + let subcommand = parsed.command_args.first().map(String::as_str); + parsed.command.as_deref().is_some_and(|cmd| { + crate::git::command_classification::is_definitely_read_only_invocation(cmd, subcommand) + }) +} + pub fn resolve_alias_invocation( parsed_args: &ParsedGitInvocation, repository: &Repository, @@ -183,7 +195,6 @@ pub fn resolve_alias_invocation( } } -#[cfg(feature = "test-support")] fn parse_alias_tokens(value: &str) -> Option> { let trimmed = value.trim_start(); diff --git a/src/daemon/coordinator.rs b/src/daemon/coordinator.rs index 30d86e9e39..159fd6a9cd 100644 --- a/src/daemon/coordinator.rs +++ b/src/daemon/coordinator.rs @@ -99,7 +99,7 @@ mod tests { CommandScope, Confidence, FamilyKey, NormalizedCommand, RepoContext, }; use crate::daemon::git_backend::{GitBackend, ReflogCut}; - use crate::git::cli_parser::parse_git_cli_args; + use crate::git::cli_parser::{ParsedGitInvocation, parse_git_cli_args}; use std::path::{Path, PathBuf}; use std::sync::Mutex; @@ -150,6 +150,16 @@ mod tests { _worktree: &Path, argv: &[String], ) -> Result, GitAiError> { + Ok(self + .resolve_invocation(_worktree, argv)? + .and_then(|invocation| invocation.command)) + } + + fn resolve_invocation( + &self, + _worktree: &Path, + argv: &[String], + ) -> Result, GitAiError> { let tokens: &[String] = if argv .first() .is_some_and(|value| value == "git" || value == "git.exe") @@ -158,7 +168,7 @@ mod tests { } else { argv }; - Ok(parse_git_cli_args(tokens).command) + Ok(Some(parse_git_cli_args(tokens))) } fn clone_target(&self, _argv: &[String], _cwd_hint: Option<&Path>) -> Option { diff --git a/src/daemon/git_backend.rs b/src/daemon/git_backend.rs index 9df9bc2650..e3a98c1fea 100644 --- a/src/daemon/git_backend.rs +++ b/src/daemon/git_backend.rs @@ -1,6 +1,6 @@ use crate::daemon::domain::{FamilyKey, RefChange, RepoContext}; use crate::error::GitAiError; -use crate::git::cli_parser::parse_git_cli_args; +use crate::git::cli_parser::{ParsedGitInvocation, parse_git_cli_args}; use crate::git::find_repository_in_path; use crate::git::repo_state::common_dir_for_worktree; use crate::git::repository::discover_repository_in_path_no_git_exec; @@ -37,6 +37,12 @@ pub trait GitBackend: Send + Sync + 'static { argv: &[String], ) -> Result, GitAiError>; + fn resolve_invocation( + &self, + worktree: &Path, + argv: &[String], + ) -> Result, GitAiError>; + fn clone_target(&self, argv: &[String], cwd_hint: Option<&Path>) -> Option; fn init_target(&self, argv: &[String], cwd_hint: Option<&Path>) -> Option; @@ -399,22 +405,32 @@ impl GitBackend for SystemGitBackend { worktree: &Path, argv: &[String], ) -> Result, GitAiError> { + Ok(self + .resolve_invocation(worktree, argv)? + .and_then(|invocation| invocation.command)) + } + + fn resolve_invocation( + &self, + worktree: &Path, + argv: &[String], + ) -> Result, GitAiError> { let mut current = parse_git_cli_args(git_invocation_tokens(argv)); let mut seen = HashSet::new(); loop { let Some(command) = current.command.clone() else { - return Ok(None); + return Ok(Some(current)); }; if !seen.insert(command.clone()) { return Ok(None); } if is_builtin_primary_command(&command) { - return Ok(Some(command)); + return Ok(Some(current)); } let alias_value = match self.resolve_alias_cached(worktree, &command)? { Some(value) => value, - None => return Ok(Some(command)), + None => return Ok(Some(current)), }; let Some(alias_tokens) = parse_alias_tokens(&alias_value) else { diff --git a/src/daemon/trace_normalizer.rs b/src/daemon/trace_normalizer.rs index 24a8185bd6..eb773adf11 100644 --- a/src/daemon/trace_normalizer.rs +++ b/src/daemon/trace_normalizer.rs @@ -860,8 +860,24 @@ impl TraceNormalizer { pending.worktree.as_deref(), pending.family_key.as_ref(), )?; - let (invoked_command, invoked_args) = - canonical_invocation(&pending.raw_argv, primary_command.as_deref()); + let resolved_invocation = if let (Some(worktree), Some(_family)) = + (pending.worktree.as_deref(), pending.family_key.as_ref()) + { + self.backend + .resolve_invocation(worktree, &pending.raw_argv)? + } else { + None + }; + let (invoked_command, invoked_args) = match resolved_invocation { + Some(invocation) + if invocation.command.is_some() + && (primary_command.is_none() + || invocation.command.as_deref() == primary_command.as_deref()) => + { + (invocation.command, invocation.command_args) + } + _ => canonical_invocation(&pending.raw_argv, primary_command.as_deref()), + }; if primary_command.is_none() { primary_command = invoked_command.clone(); } @@ -1495,7 +1511,7 @@ fn select_primary_command( mod tests { use super::*; use crate::daemon::domain::RefChange; - use std::collections::HashMap; + use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -1582,20 +1598,48 @@ mod tests { worktree: &Path, argv: &[String], ) -> Result, GitAiError> { + Ok(self + .resolve_invocation(worktree, argv)? + .and_then(|invocation| invocation.command)) + } + + fn resolve_invocation( + &self, + worktree: &Path, + argv: &[String], + ) -> Result, GitAiError> { let raw = argv_primary_command(argv); - let Some(command) = raw else { - return Ok(None); - }; + let tokens = trace_argv_invocation_tokens(argv); + let mut current = parse_git_cli_args(tokens); let worktree_key = normalize_path_key(worktree); - let resolved = self - .alias_by_worktree_command - .lock() - .unwrap() - .get(&worktree_key) - .and_then(|commands| commands.get(&command)) - .cloned() - .unwrap_or(command); - Ok(Some(resolved)) + let mut seen = HashSet::new(); + loop { + let Some(command) = current.command.clone() else { + return if raw.is_some() { + Ok(Some(current)) + } else { + Ok(None) + }; + }; + if !seen.insert(command.clone()) { + return Ok(None); + } + let alias_value = self + .alias_by_worktree_command + .lock() + .unwrap() + .get(&worktree_key) + .and_then(|commands| commands.get(&command)) + .cloned(); + let Some(alias_value) = alias_value else { + return Ok(Some(current)); + }; + let mut expanded_args = Vec::new(); + expanded_args.extend(current.global_args.iter().cloned()); + expanded_args.extend(alias_value.split_whitespace().map(|s| s.to_string())); + expanded_args.extend(current.command_args.iter().cloned()); + current = parse_git_cli_args(&expanded_args); + } } fn clone_target(&self, _argv: &[String], _cwd_hint: Option<&Path>) -> Option { @@ -1841,6 +1885,62 @@ mod tests { assert_eq!(cmd.primary_command.as_deref(), Some("commit")); } + #[test] + fn alias_commit_amend_expands_invoked_args_for_history_analysis() { + let backend = Arc::new(MockBackend::default()); + let temp = tempfile::tempdir().expect("create tempdir"); + let worktree = temp.path().join("repo"); + fs::create_dir_all(worktree.join(".git")).expect("create git dir"); + let worktree_str = worktree.to_str().expect("utf8 worktree"); + backend.set_family(worktree_str, "family"); + backend.set_alias(worktree_str, "ca", "commit --amend"); + let mut normalizer = TraceNormalizer::new(backend); + + let old_head = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + let new_head = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + let start = serde_json::json!({ + "event":"start", + "sid":"alias-amend", + "ts":1, + "argv":["git","ca","-m","msg"], + "worktree":worktree, + "git_ai_family_reflog_start": {"HEAD": 10} + }); + let exit = serde_json::json!({ + "event":"exit", + "sid":"alias-amend", + "ts":2, + "code":0, + "git_ai_family_reflog_end": {"HEAD": 11}, + "git_ai_family_reflog_changes": [{ + "reference": "HEAD", + "old": old_head, + "new": new_head + }] + }); + + assert!(normalizer.ingest_payload(&start).unwrap().is_none()); + let cmd = normalizer.ingest_payload(&exit).unwrap().unwrap(); + assert_eq!(cmd.primary_command.as_deref(), Some("commit")); + assert_eq!(cmd.invoked_command.as_deref(), Some("commit")); + assert!(cmd.invoked_args.iter().any(|arg| arg == "--amend")); + + let analyzer = crate::daemon::analyzers::history::HistoryAnalyzer; + let result = crate::daemon::analyzers::CommandAnalyzer::analyze( + &analyzer, + &cmd, + crate::daemon::analyzers::AnalysisView { + refs: &HashMap::new(), + }, + ) + .unwrap(); + assert!(result.events.iter().any(|event| matches!( + event, + crate::daemon::domain::SemanticEvent::CommitAmended { old_head: old, new_head: new } + if old == old_head && new == new_head + ))); + } + #[test] fn normalizer_errors_on_exit_without_start() { let backend = Arc::new(MockBackend::default()); diff --git a/tests/async_mode.rs b/tests/async_mode.rs index 6e423627db..2647d97148 100644 --- a/tests/async_mode.rs +++ b/tests/async_mode.rs @@ -649,6 +649,34 @@ fn async_mode_post_commit_shows_stats_for_ai_commit() { ); } +#[test] +fn async_mode_post_commit_shows_stats_for_aliased_commit() { + let repo = TestRepo::new_with_mode(GitTestMode::WrapperDaemon); + repo.git(&["config", "alias.ci", "commit"]) + .expect("alias config should succeed"); + + let mut file = repo.filename("alias.txt"); + file.set_contents(crate::lines!["Base"]); + repo.stage_all_and_commit("Base").unwrap(); + + file.insert_at(1, crate::lines!["AI line".ai()]); + repo.git(&["add", "-A"]).expect("add should succeed"); + + let output = repo + .git_with_env( + &["ci", "-m", "AI additions via alias"], + &[("GIT_AI_TEST_FORCE_TTY", "1")], + None, + ) + .expect("aliased commit should succeed"); + + assert!( + output.contains("you") && output.contains("ai"), + "expected stats output for aliased async commit, got:\n{}", + output + ); +} + #[test] fn async_mode_post_commit_quiet_flag_suppresses_stats() { let repo = TestRepo::new_with_mode(GitTestMode::WrapperDaemon); diff --git a/tests/daemon_mode.rs b/tests/daemon_mode.rs index c2b381fcb5..bd2c76c2ac 100644 --- a/tests/daemon_mode.rs +++ b/tests/daemon_mode.rs @@ -1553,6 +1553,66 @@ fn daemon_pure_trace_socket_write_mode_applies_amend_rewrite() { ); } +#[test] +#[serial] +fn daemon_pure_trace_socket_commit_amend_alias_emits_amend_event() { + let repo = + TestRepo::new_with_mode_and_daemon_scope(GitTestMode::Daemon, DaemonTestScope::NoDaemon); + repo.git(&["config", "alias.ca", "commit --amend"]) + .expect("configure commit amend alias"); + let _daemon = DaemonGuard::start(&repo); + let trace_socket = daemon_trace_socket_path(&repo); + let env = git_trace_env(&trace_socket); + let env_refs = [(env[0].0, env[0].1.as_str()), (env[1].0, env[1].1.as_str())]; + let completion_baseline = repo.daemon_total_completion_count(); + let mut expected_top_level_completions = 0u64; + + fs::write(repo.path().join("alias-amend.txt"), "line 1\n").expect("failed to write file"); + traced_git_with_env( + &repo, + &["add", "alias-amend.txt"], + &env_refs, + &mut expected_top_level_completions, + ) + .expect("add should succeed"); + traced_git_with_env( + &repo, + &["commit", "-m", "initial"], + &env_refs, + &mut expected_top_level_completions, + ) + .expect("commit should succeed"); + + fs::write(repo.path().join("alias-amend.txt"), "line 1\nline 2\n") + .expect("failed to update file"); + traced_git_with_env( + &repo, + &["add", "alias-amend.txt"], + &env_refs, + &mut expected_top_level_completions, + ) + .expect("add before amend should succeed"); + traced_git_with_env( + &repo, + &["ca", "-m", "initial amended"], + &env_refs, + &mut expected_top_level_completions, + ) + .expect("alias amend should succeed"); + + wait_for_expected_top_level_completions( + &repo, + completion_baseline, + expected_top_level_completions, + ); + + let amend_events = wait_for_rewrite_event_count(&repo, "\"commit_amend\"", 1); + assert_eq!( + amend_events, 1, + "commit --amend aliases should emit exactly one commit_amend rewrite event" + ); +} + #[test] #[serial] fn daemon_pure_trace_socket_rebase_abort_emits_abort_event() {