From 3bcc7745b62496e8b4a4210aa57d15eccba7feb0 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 13:44:54 +0000 Subject: [PATCH 01/14] Speed up read-only wrapper commands --- src/commands/git_handlers.rs | 66 ++++++++++++++++++++++++------------ src/config.rs | 4 +++ 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 5f96b9a95..58473d618 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -110,25 +110,19 @@ pub fn handle_git(args: &[String]) { return; } + let parsed_args = parse_git_cli_args(args); + + // Direct read-only commands never participate in git-ai hooks, so avoid the + // wrapper's repository/config preflight entirely and behave like a pure + // passthrough. This keeps commands like `git status` fast on Windows. + if should_passthrough_read_only_command(&parsed_args) { + let exit_status = proxy_to_git(args, false, None, None); + exit_with_status(exit_status); + } + // Async mode: wrapper should behave as a pure passthrough to git, // but capture and send authoritative pre/post state to the daemon. if config::Config::get().feature_flags().async_mode { - let parsed = parse_git_cli_args(args); - - // Read-only commands don't need wrapper state (the daemon fast-paths - // their trace events and never processes them through the normalizer). - // Skip the invocation_id so we can also suppress trace2 for them, - // avoiding unnecessary daemon work and wrapper_states memory leaks. - let is_read_only = parsed - .command - .as_deref() - .is_some_and(crate::git::command_classification::is_definitely_read_only_command); - - if is_read_only { - let exit_status = proxy_to_git(args, false, None, None); - exit_with_status(exit_status); - } - // Initialize the daemon telemetry handle so we can send wrapper state if let crate::daemon::telemetry_handle::DaemonTelemetryInitResult::Failed(e) = crate::daemon::telemetry_handle::init_daemon_telemetry_handle() @@ -136,7 +130,7 @@ pub fn handle_git(args: &[String]) { debug_log(&format!("wrapper: daemon telemetry init failed: {}", e)); } - let repository = find_repository(&parsed.global_args).ok(); + let repository = find_repository(&parsed_args.global_args).ok(); let worktree = repository.as_ref().and_then(|r| r.workdir().ok()); let pre_state = worktree @@ -159,16 +153,16 @@ pub fn handle_git(args: &[String]) { // After a successful commit, wait briefly for the daemon to produce an // authorship note so we can show stats inline (same UX as plain wrapper mode). if exit_status.success() - && parsed.command.as_deref() == Some("commit") + && parsed_args.command.as_deref() == Some("commit") && let Some(repo) = repository.as_ref() { - maybe_show_async_post_commit_stats(&parsed, repo); + maybe_show_async_post_commit_stats(&parsed_args, repo); } exit_with_status(exit_status); } - let mut parsed_args = parse_git_cli_args(args); + let mut parsed_args = parsed_args; let mut repository_option = find_repository(&parsed_args.global_args).ok(); @@ -263,6 +257,14 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } +fn should_passthrough_read_only_command(parsed_args: &ParsedGitInvocation) -> bool { + !parsed_args.is_help + && parsed_args + .command + .as_deref() + .is_some_and(crate::git::command_classification::is_definitely_read_only_command) +} + /// Handle alias invocations #[cfg(feature = "test-support")] pub fn resolve_alias_invocation( @@ -985,7 +987,10 @@ fn in_shell_completion_context() -> bool { #[cfg(test)] mod tests { use super::parse_alias_tokens; - use super::{parse_git_cli_args, resolve_child_git_hooks_path_override}; + use super::{ + parse_git_cli_args, resolve_child_git_hooks_path_override, + should_passthrough_read_only_command, + }; use crate::git::find_repository_in_path; use std::process::Command; use tempfile::tempdir; @@ -1112,6 +1117,25 @@ mod tests { ); } + #[test] + fn passthrough_read_only_command_for_status() { + let parsed = parse_git_cli_args(&["status".to_string(), "--short".to_string()]); + assert!(should_passthrough_read_only_command(&parsed)); + } + + #[test] + fn passthrough_read_only_command_rejects_mutating_commands() { + let parsed = + parse_git_cli_args(&["commit".to_string(), "-m".to_string(), "msg".to_string()]); + assert!(!should_passthrough_read_only_command(&parsed)); + } + + #[test] + fn passthrough_read_only_command_rejects_help_invocations() { + let parsed = parse_git_cli_args(&["status".to_string(), "--help".to_string()]); + assert!(!should_passthrough_read_only_command(&parsed)); + } + #[cfg(unix)] #[test] fn exit_status_was_interrupted_on_sigint() { diff --git a/src/config.rs b/src/config.rs index 7311d2dcc..eb32187ad 100644 --- a/src/config.rs +++ b/src/config.rs @@ -209,6 +209,10 @@ impl Config { } pub fn is_allowed_repository(&self, repository: &Option) -> bool { + if self.allow_repositories.is_empty() && self.exclude_repositories.is_empty() { + return true; + } + // Fetch remotes once and reuse for both exclude and allow checks let remotes = repository .as_ref() From 2d1464850a748c0ce619d05ce6981d1d224164b4 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 14:40:43 +0000 Subject: [PATCH 02/14] Speed up wrapper repository preflight --- src/commands/git_handlers.rs | 17 ++- src/git/command_classification.rs | 174 +++++++++++++++++++++++++ src/git/repository.rs | 204 +++++++++++++++++++++++++++--- 3 files changed, 367 insertions(+), 28 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 58473d618..dee578887 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -258,11 +258,7 @@ pub fn handle_git(args: &[String]) { } fn should_passthrough_read_only_command(parsed_args: &ParsedGitInvocation) -> bool { - !parsed_args.is_help - && parsed_args - .command - .as_deref() - .is_some_and(crate::git::command_classification::is_definitely_read_only_command) + crate::git::command_classification::is_read_only_invocation(parsed_args) } /// Handle alias invocations @@ -797,10 +793,7 @@ fn proxy_to_git( // trace2 events for the daemon to match wrapper state entries. let suppress_trace2 = wrapper_invocation_id.is_none() && { let parsed = parse_git_cli_args(args); - parsed - .command - .as_deref() - .is_some_and(crate::git::command_classification::is_definitely_read_only_command) + crate::git::command_classification::is_read_only_invocation(&parsed) }; // Use spawn for interactive commands @@ -1123,6 +1116,12 @@ mod tests { assert!(should_passthrough_read_only_command(&parsed)); } + #[test] + fn passthrough_read_only_command_for_branch_show_current() { + let parsed = parse_git_cli_args(&["branch".to_string(), "--show-current".to_string()]); + assert!(should_passthrough_read_only_command(&parsed)); + } + #[test] fn passthrough_read_only_command_rejects_mutating_commands() { let parsed = diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index 035caeadc..eba6ca24f 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -1,3 +1,5 @@ +use crate::git::cli_parser::ParsedGitInvocation; + /// Returns true if the given git subcommand is guaranteed to never mutate /// repository state (refs, objects, config, worktree). Used to skip expensive /// trace2 ingestion work and suppress trace2 emission for read-only commands. @@ -35,9 +37,145 @@ pub fn is_definitely_read_only_command(command: &str) -> bool { ) } +pub fn is_read_only_invocation(parsed: &ParsedGitInvocation) -> bool { + if parsed.is_help { + return false; + } + + if parsed + .command + .as_deref() + .is_some_and(is_definitely_read_only_command) + { + return true; + } + + match parsed.command.as_deref() { + Some("branch") => is_read_only_branch_invocation(parsed), + Some("stash") => is_read_only_stash_invocation(parsed), + Some("tag") => is_read_only_tag_invocation(parsed), + _ => false, + } +} + +fn command_args_contain_any(command_args: &[String], flags: &[&str]) -> bool { + command_args.iter().any(|arg| { + flags + .iter() + .any(|flag| arg == flag || arg.starts_with(&format!("{flag}="))) + }) +} + +fn is_read_only_branch_invocation(parsed: &ParsedGitInvocation) -> bool { + let mutating_flags = [ + "-c", + "-C", + "-d", + "-D", + "-f", + "-m", + "-M", + "-u", + "--copy", + "--create-reflog", + "--delete", + "--delete-force", + "--edit-description", + "--force", + "--move", + "--no-track", + "--recurse-submodules", + "--set-upstream-to", + "--track", + "--unset-upstream", + ]; + if command_args_contain_any(&parsed.command_args, &mutating_flags) { + return false; + } + + let read_only_listing_flags = [ + "--all", + "--contains", + "--format", + "--ignore-case", + "--list", + "--merged", + "--no-color", + "--no-column", + "--no-contains", + "--no-merged", + "--points-at", + "--remotes", + "--show-current", + "--sort", + "--verbose", + "-a", + "-l", + "-r", + "-v", + ]; + + command_args_contain_any(&parsed.command_args, &read_only_listing_flags) + || parsed.pos_command(0).is_none() +} + +fn is_read_only_stash_invocation(parsed: &ParsedGitInvocation) -> bool { + matches!( + parsed.command_args.first().map(String::as_str), + Some("list" | "show") + ) +} + +fn is_read_only_tag_invocation(parsed: &ParsedGitInvocation) -> bool { + let mutating_flags = [ + "-a", + "-d", + "-e", + "-f", + "-F", + "-m", + "-s", + "-u", + "--annotate", + "--cleanup", + "--create-reflog", + "--delete", + "--edit", + "--file", + "--force", + "--local-user", + "--message", + "--no-sign", + "--sign", + "--trailer", + ]; + if command_args_contain_any(&parsed.command_args, &mutating_flags) { + return false; + } + + let read_only_listing_flags = [ + "--column", + "--contains", + "--format", + "--ignore-case", + "--list", + "--merged", + "--no-column", + "--no-contains", + "--no-merged", + "--points-at", + "--sort", + "-l", + ]; + + command_args_contain_any(&parsed.command_args, &read_only_listing_flags) + || parsed.pos_command(0).is_none() +} + #[cfg(test)] mod tests { use super::*; + use crate::git::cli_parser::parse_git_cli_args; #[test] fn read_only_commands_detected() { @@ -68,4 +206,40 @@ mod tests { assert!(!is_definitely_read_only_command("my-custom-alias")); assert!(!is_definitely_read_only_command("")); } + + #[test] + fn read_only_invocation_detects_branch_show_current() { + let parsed = parse_git_cli_args(&["branch".to_string(), "--show-current".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_branch_listing_without_positionals() { + let parsed = parse_git_cli_args(&["branch".to_string(), "-v".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_branch_creation() { + let parsed = parse_git_cli_args(&["branch".to_string(), "feature".to_string()]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_tag_listing() { + let parsed = parse_git_cli_args(&["tag".to_string(), "--list".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_tag_creation() { + let parsed = parse_git_cli_args(&["tag".to_string(), "v1.2.3".to_string()]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_stash_list() { + let parsed = parse_git_cli_args(&["stash".to_string(), "list".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } } diff --git a/src/git/repository.rs b/src/git/repository.rs index c28419863..606bcbebd 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -95,6 +95,19 @@ fn args_with_disabled_hooks_if_needed(args: &[String]) -> Vec { out } +fn apply_internal_git_command_env(cmd: &mut Command) { + cmd.env_remove("GIT_EXTERNAL_DIFF"); + cmd.env_remove("GIT_DIFF_OPTS"); + + // Internal helper git invocations never need trace2 output. Disabling every + // trace2 stream here avoids extra named-pipe/process overhead on Windows when + // users have global trace2 targets configured for the wrapper daemon. + cmd.env("GIT_TRACE2", "0"); + cmd.env("GIT_TRACE2_EVENT", "0"); + cmd.env("GIT_TRACE2_PERF", "0"); + cmd.env("GIT_TRACE2_BRIEF", "0"); +} + fn first_git_subcommand_index(args: &[String]) -> Option { let mut index = 0usize; @@ -2354,6 +2367,10 @@ impl Repository { } pub fn find_repository(global_args: &[String]) -> Result { + if let Some(repository) = try_find_repository_no_git_exec(global_args)? { + return Ok(repository); + } + let mut rev_parse_args = global_args.to_owned(); rev_parse_args.push("rev-parse".to_string()); // Use --git-dir instead of --absolute-git-dir for compatibility with Git < 2.13 @@ -2440,21 +2457,12 @@ pub fn find_repository(global_args: &[String]) -> Result } // Ensure all internal git commands use a stable repository root consistently. - let mut normalized_global_args = global_args.to_owned(); let command_root = if is_bare { git_dir.display().to_string() } else { workdir.display().to_string() }; - - if normalized_global_args.is_empty() { - normalized_global_args = vec!["-C".to_string(), command_root]; - } else if normalized_global_args.len() == 2 - && normalized_global_args[0] == "-C" - && normalized_global_args[1] != command_root - { - normalized_global_args[1] = command_root; - } + let normalized_global_args = normalize_global_args_for_repo_root(global_args, &command_root); // Canonicalize workdir for reliable path comparisons (especially on Windows) // On Windows, canonical paths use the \\?\ UNC prefix, which makes path.starts_with() @@ -2491,6 +2499,122 @@ pub fn find_repository(global_args: &[String]) -> Result }) } +fn try_find_repository_no_git_exec( + global_args: &[String], +) -> Result, GitAiError> { + let Some(base_dir) = resolve_fast_discovery_base_dir(global_args)? else { + return Ok(None); + }; + + let paths = match discover_repository_paths_no_git_exec(&base_dir) { + Ok(paths) => paths, + Err(_) => return Ok(None), + }; + + let normalized_global_args = + normalize_global_args_for_repo_root(global_args, &paths.command_root.to_string_lossy()); + + repository_from_discovered_paths( + normalized_global_args, + &paths.workdir, + &paths.git_dir, + &paths.git_common_dir, + ) + .map(Some) +} + +fn resolve_fast_discovery_base_dir(global_args: &[String]) -> Result, GitAiError> { + let mut base: Option = None; + let mut idx = 0usize; + + while idx < global_args.len() { + let arg = &global_args[idx]; + + if arg == "-C" { + let path_arg = global_args.get(idx + 1).ok_or_else(|| { + GitAiError::Generic("Missing path after -C in global git args".to_string()) + })?; + base = Some(apply_global_c_arg(base.as_ref(), path_arg)?); + idx += 2; + continue; + } + + if let Some(path_arg) = arg.strip_prefix("-C") + && !path_arg.is_empty() + { + base = Some(apply_global_c_arg(base.as_ref(), path_arg)?); + idx += 1; + continue; + } + + if is_fast_discovery_safe_global_flag(arg) { + idx += 1; + continue; + } + + return Ok(None); + } + + Ok(Some(match base { + Some(base) => base, + None => std::env::current_dir().map_err(GitAiError::IoError)?, + })) +} + +fn apply_global_c_arg(base: Option<&PathBuf>, path_arg: &str) -> Result { + let next_base = PathBuf::from(path_arg); + Ok(if next_base.is_absolute() { + next_base + } else { + let current = match base { + Some(existing) => existing.clone(), + None => std::env::current_dir().map_err(GitAiError::IoError)?, + }; + current.join(next_base) + }) +} + +fn is_fast_discovery_safe_global_flag(arg: &str) -> bool { + matches!( + arg, + "-p" | "--paginate" + | "-P" + | "--no-pager" + | "--no-replace-objects" + | "--no-lazy-fetch" + | "--no-optional-locks" + | "--no-advice" + | "--literal-pathspecs" + | "--glob-pathspecs" + | "--noglob-pathspecs" + | "--icase-pathspecs" + ) +} + +fn normalize_global_args_for_repo_root(global_args: &[String], command_root: &str) -> Vec { + let mut normalized = vec!["-C".to_string(), command_root.to_string()]; + let mut idx = 0usize; + + while idx < global_args.len() { + let arg = &global_args[idx]; + + if arg == "-C" { + idx += 2; + continue; + } + + if arg.starts_with("-C") && arg.len() > 2 { + idx += 1; + continue; + } + + normalized.push(arg.clone()); + idx += 1; + } + + normalized +} + fn resolve_command_base_dir(global_args: &[String]) -> Result { let mut base: Option = None; let mut idx = 0usize; @@ -2807,7 +2931,7 @@ pub fn from_bare_repository(git_dir: &Path) -> Result { } fn repository_from_discovered_paths( - command_root: &Path, + global_args: Vec, workdir: &Path, git_dir: &Path, git_common_dir: &Path, @@ -2847,7 +2971,7 @@ fn repository_from_discovered_paths( }; Ok(Repository { - global_args: vec!["-C".to_string(), command_root.to_string_lossy().to_string()], + global_args, storage, git_dir: git_dir.to_path_buf(), git_common_dir: git_common_dir.to_path_buf(), @@ -2865,8 +2989,10 @@ fn repository_from_discovered_paths( pub fn discover_repository_in_path_no_git_exec(path: &Path) -> Result { let paths = discover_repository_paths_no_git_exec(path)?; + let global_args = + normalize_global_args_for_repo_root(&[], &paths.command_root.to_string_lossy()); repository_from_discovered_paths( - &paths.command_root, + global_args, &paths.workdir, &paths.git_dir, &paths.git_common_dir, @@ -3028,8 +3154,7 @@ pub fn exec_git_allow_nonzero_with_profile( args_with_internal_git_profile(&args_with_disabled_hooks_if_needed(args), profile); let mut cmd = Command::new(config::Config::get().git_cmd()); cmd.args(&effective_args); - cmd.env_remove("GIT_EXTERNAL_DIFF"); - cmd.env_remove("GIT_DIFF_OPTS"); + apply_internal_git_command_env(&mut cmd); #[cfg(windows)] { @@ -3082,8 +3207,7 @@ pub fn exec_git_stdin_with_profile( .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()); - cmd.env_remove("GIT_EXTERNAL_DIFF"); - cmd.env_remove("GIT_DIFF_OPTS"); + apply_internal_git_command_env(&mut cmd); #[cfg(windows)] { @@ -3147,8 +3271,7 @@ pub fn exec_git_stdin_with_env_with_profile( for (k, v) in env.iter() { cmd.env(k, v); } - cmd.env_remove("GIT_EXTERNAL_DIFF"); - cmd.env_remove("GIT_DIFF_OPTS"); + apply_internal_git_command_env(&mut cmd); #[cfg(windows)] { @@ -3806,6 +3929,49 @@ index 0000000..abc1234 100644 assert_eq!(resolved, base.join("nested").join("..").join("repo")); } + #[test] + fn resolve_fast_discovery_base_dir_supports_common_global_flags() { + let temp = tempfile::tempdir().expect("tempdir"); + let base = temp.path().join("root"); + let args = vec![ + "--no-pager".to_string(), + "-C".to_string(), + base.to_string_lossy().to_string(), + "--literal-pathspecs".to_string(), + ]; + + let resolved = resolve_fast_discovery_base_dir(&args).expect("resolve fast discovery dir"); + assert_eq!(resolved, Some(base)); + } + + #[test] + fn resolve_fast_discovery_base_dir_rejects_git_dir_override() { + let args = vec!["--git-dir".to_string(), ".git".to_string()]; + let resolved = resolve_fast_discovery_base_dir(&args).expect("resolve fast discovery dir"); + assert_eq!(resolved, None); + } + + #[test] + fn normalize_global_args_for_repo_root_collapses_chained_c_args() { + let args = vec![ + "--no-pager".to_string(), + "-C".to_string(), + "nested".to_string(), + "--literal-pathspecs".to_string(), + ]; + + let normalized = normalize_global_args_for_repo_root(&args, "/repo/root"); + assert_eq!( + normalized, + vec![ + "-C".to_string(), + "/repo/root".to_string(), + "--no-pager".to_string(), + "--literal-pathspecs".to_string(), + ] + ); + } + #[test] fn find_repository_in_path_supports_bare_repositories() { let temp = tempfile::tempdir().expect("tempdir"); From 806a2d77d893874e8a77e3db5ee51892833926e5 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 15:22:14 +0000 Subject: [PATCH 03/14] Expand passthrough startup fast paths --- src/commands/git_handlers.rs | 10 ++- src/git/command_classification.rs | 129 +++++++++++++++++++++++++++++- src/main.rs | 20 +---- 3 files changed, 139 insertions(+), 20 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index dee578887..30acc624e 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -1130,9 +1130,15 @@ mod tests { } #[test] - fn passthrough_read_only_command_rejects_help_invocations() { + fn passthrough_read_only_command_accepts_help_invocations() { let parsed = parse_git_cli_args(&["status".to_string(), "--help".to_string()]); - assert!(!should_passthrough_read_only_command(&parsed)); + assert!(should_passthrough_read_only_command(&parsed)); + } + + #[test] + fn passthrough_read_only_command_accepts_top_level_version() { + let parsed = parse_git_cli_args(&["--version".to_string()]); + assert!(should_passthrough_read_only_command(&parsed)); } #[cfg(unix)] diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index eba6ca24f..3a164789b 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -38,8 +38,8 @@ pub fn is_definitely_read_only_command(command: &str) -> bool { } pub fn is_read_only_invocation(parsed: &ParsedGitInvocation) -> bool { - if parsed.is_help { - return false; + if parsed.is_help || parsed.command.is_none() { + return true; } if parsed @@ -54,6 +54,10 @@ pub fn is_read_only_invocation(parsed: &ParsedGitInvocation) -> bool { Some("branch") => is_read_only_branch_invocation(parsed), Some("stash") => is_read_only_stash_invocation(parsed), Some("tag") => is_read_only_tag_invocation(parsed), + Some("remote") => is_read_only_remote_invocation(parsed), + Some("config") => is_read_only_config_invocation(parsed), + Some("worktree") => is_read_only_worktree_invocation(parsed), + Some("submodule") => is_read_only_submodule_invocation(parsed), _ => false, } } @@ -172,6 +176,77 @@ fn is_read_only_tag_invocation(parsed: &ParsedGitInvocation) -> bool { || parsed.pos_command(0).is_none() } +fn is_read_only_remote_invocation(parsed: &ParsedGitInvocation) -> bool { + let mutating_subcommands = [ + "add", + "rename", + "remove", + "rm", + "set-head", + "set-branches", + "set-url", + "prune", + "update", + ]; + + match parsed.pos_command(0).as_deref() { + None => true, + Some(subcommand) if mutating_subcommands.contains(&subcommand) => false, + Some("show" | "get-url") => true, + Some(_) => false, + } +} + +fn is_read_only_config_invocation(parsed: &ParsedGitInvocation) -> bool { + let mutating_flags = [ + "--add", + "--replace-all", + "--unset", + "--unset-all", + "--rename-section", + "--remove-section", + "--edit", + ]; + if command_args_contain_any(&parsed.command_args, &mutating_flags) { + return false; + } + + let read_only_flags = [ + "--blob", + "--default", + "--get", + "--get-all", + "--get-regexp", + "--get-urlmatch", + "--includes", + "--list", + "--name-only", + "--no-includes", + "--null", + "--show-origin", + "--show-scope", + "--type", + "-l", + "-z", + ]; + + command_args_contain_any(&parsed.command_args, &read_only_flags) +} + +fn is_read_only_worktree_invocation(parsed: &ParsedGitInvocation) -> bool { + matches!( + parsed.command_args.first().map(String::as_str), + Some("list") + ) +} + +fn is_read_only_submodule_invocation(parsed: &ParsedGitInvocation) -> bool { + matches!( + parsed.command_args.first().map(String::as_str), + Some("status" | "summary") + ) +} + #[cfg(test)] mod tests { use super::*; @@ -242,4 +317,54 @@ mod tests { let parsed = parse_git_cli_args(&["stash".to_string(), "list".to_string()]); assert!(is_read_only_invocation(&parsed)); } + + #[test] + fn read_only_invocation_detects_top_level_version() { + let parsed = parse_git_cli_args(&["--version".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_commit_help() { + let parsed = parse_git_cli_args(&["commit".to_string(), "--help".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_remote_listing() { + let parsed = parse_git_cli_args(&["remote".to_string(), "-v".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_remote_add() { + let parsed = parse_git_cli_args(&[ + "remote".to_string(), + "add".to_string(), + "origin".to_string(), + "https://example.com/repo".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_config_list() { + let parsed = parse_git_cli_args(&[ + "config".to_string(), + "--list".to_string(), + "--show-origin".to_string(), + ]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_config_set() { + let parsed = parse_git_cli_args(&[ + "config".to_string(), + "--add".to_string(), + "demo.key".to_string(), + "value".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } } diff --git a/src/main.rs b/src/main.rs index b7d0f70da..1096c70d4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,16 +1,5 @@ -use clap::Parser; use git_ai::commands; -#[derive(Parser)] -#[command(name = "git-ai")] -#[command(about = "git proxy with AI authorship tracking", long_about = None)] -#[command(disable_help_flag = true, disable_version_flag = true)] -struct Cli { - /// Git command and arguments - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - args: Vec, -} - fn main() { // Get the binary name that was called let binary_name = std::env::args_os() @@ -30,21 +19,20 @@ fn main() { commands::git_hook_handlers::handle_git_hook_invocation(&binary_name, &hook_args); std::process::exit(exit_code); } - - let cli = Cli::parse(); + let args: Vec = std::env::args().skip(1).collect(); #[cfg(debug_assertions)] { if std::env::var("GIT_AI").as_deref() == Ok("git") { - commands::git_handlers::handle_git(&cli.args); + commands::git_handlers::handle_git(&args); return; } } if binary_name == "git-ai" || binary_name == "git-ai.exe" { - commands::git_ai_handlers::handle_git_ai(&cli.args); + commands::git_ai_handlers::handle_git_ai(&args); std::process::exit(0); } - commands::git_handlers::handle_git(&cli.args); + commands::git_handlers::handle_git(&args); } From 00123ba2626c743f947e5318e9acd744bdb0f7af Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 15:43:41 +0000 Subject: [PATCH 04/14] Resolve aliases before passthrough checks --- src/commands/git_handlers.rs | 110 ++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 14 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 30acc624e..e0f17f9d5 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -110,7 +110,7 @@ pub fn handle_git(args: &[String]) { return; } - let parsed_args = parse_git_cli_args(args); + let mut parsed_args = parse_git_cli_args(args); // Direct read-only commands never participate in git-ai hooks, so avoid the // wrapper's repository/config preflight entirely and behave like a pure @@ -120,6 +120,14 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } + let mut repository_option = find_repository(&parsed_args.global_args).ok(); + parsed_args = resolve_wrapper_invocation(&parsed_args, repository_option.as_ref()); + + if should_passthrough_read_only_command(&parsed_args) { + let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); + exit_with_status(exit_status); + } + // Async mode: wrapper should behave as a pure passthrough to git, // but capture and send authoritative pre/post state to the daemon. if config::Config::get().feature_flags().async_mode { @@ -130,8 +138,7 @@ pub fn handle_git(args: &[String]) { debug_log(&format!("wrapper: daemon telemetry init failed: {}", e)); } - let repository = find_repository(&parsed_args.global_args).ok(); - let worktree = repository.as_ref().and_then(|r| r.workdir().ok()); + let worktree = repository_option.as_ref().and_then(|r| r.workdir().ok()); let pre_state = worktree .as_deref() @@ -142,7 +149,12 @@ pub fn handle_git(args: &[String]) { // processes the atexit trace event and starts the wrapper state timeout. send_wrapper_pre_state_to_daemon(&invocation_id, worktree.as_deref(), &pre_state); - let exit_status = proxy_to_git(args, false, None, Some(&invocation_id)); + let exit_status = proxy_to_git( + &parsed_args.to_invocation_vec(), + false, + None, + Some(&invocation_id), + ); let post_state = worktree .as_deref() @@ -154,7 +166,7 @@ pub fn handle_git(args: &[String]) { // authorship note so we can show stats inline (same UX as plain wrapper mode). if exit_status.success() && parsed_args.command.as_deref() == Some("commit") - && let Some(repo) = repository.as_ref() + && let Some(repo) = repository_option.as_ref() { maybe_show_async_post_commit_stats(&parsed_args, repo); } @@ -162,10 +174,6 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } - let mut parsed_args = parsed_args; - - let mut repository_option = find_repository(&parsed_args.global_args).ok(); - let has_repo = repository_option.is_some(); let config = config::Config::get(); @@ -204,10 +212,6 @@ pub fn handle_git(args: &[String]) { let repository = repository_option.as_mut().unwrap(); - if let Some(resolved) = resolve_alias_invocation(&parsed_args, repository) { - parsed_args = resolved; - } - let pre_command_start = Instant::now(); run_pre_command_hooks(&mut command_hooks_context, &mut parsed_args, repository); let pre_command_duration = pre_command_start.elapsed(); @@ -261,6 +265,15 @@ fn should_passthrough_read_only_command(parsed_args: &ParsedGitInvocation) -> bo crate::git::command_classification::is_read_only_invocation(parsed_args) } +fn resolve_wrapper_invocation( + parsed_args: &ParsedGitInvocation, + repository: Option<&Repository>, +) -> ParsedGitInvocation { + repository + .and_then(|repo| resolve_alias_invocation(parsed_args, repo)) + .unwrap_or_else(|| parsed_args.clone()) +} + /// Handle alias invocations #[cfg(feature = "test-support")] pub fn resolve_alias_invocation( @@ -981,7 +994,7 @@ fn in_shell_completion_context() -> bool { mod tests { use super::parse_alias_tokens; use super::{ - parse_git_cli_args, resolve_child_git_hooks_path_override, + parse_git_cli_args, resolve_child_git_hooks_path_override, resolve_wrapper_invocation, should_passthrough_read_only_command, }; use crate::git::find_repository_in_path; @@ -1141,6 +1154,75 @@ mod tests { assert!(should_passthrough_read_only_command(&parsed)); } + #[test] + fn wrapper_resolves_read_only_alias_before_passthrough() { + let temp = tempdir().expect("tempdir should create"); + let output = Command::new("git") + .args(["init", "-q"]) + .current_dir(temp.path()) + .output() + .expect("git init should run"); + assert!( + output.status.success(), + "git init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let alias_output = Command::new("git") + .args(["config", "alias.st", "status --short"]) + .current_dir(temp.path()) + .output() + .expect("git config alias should run"); + assert!( + alias_output.status.success(), + "git config alias failed: {}", + String::from_utf8_lossy(&alias_output.stderr) + ); + + let repo = find_repository_in_path(&temp.path().to_string_lossy()) + .expect("repository should be discovered"); + let parsed = parse_git_cli_args(&["st".to_string()]); + let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + + assert_eq!(resolved.command.as_deref(), Some("status")); + assert!(resolved.command_args.iter().any(|arg| arg == "--short")); + assert!(should_passthrough_read_only_command(&resolved)); + } + + #[test] + fn wrapper_resolves_mutating_alias_before_passthrough() { + let temp = tempdir().expect("tempdir should create"); + let output = Command::new("git") + .args(["init", "-q"]) + .current_dir(temp.path()) + .output() + .expect("git init should run"); + assert!( + output.status.success(), + "git init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let alias_output = Command::new("git") + .args(["config", "alias.ci", "commit"]) + .current_dir(temp.path()) + .output() + .expect("git config alias should run"); + assert!( + alias_output.status.success(), + "git config alias failed: {}", + String::from_utf8_lossy(&alias_output.stderr) + ); + + let repo = find_repository_in_path(&temp.path().to_string_lossy()) + .expect("repository should be discovered"); + let parsed = parse_git_cli_args(&["ci".to_string(), "-m".to_string(), "msg".to_string()]); + let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + + assert_eq!(resolved.command.as_deref(), Some("commit")); + assert!(!should_passthrough_read_only_command(&resolved)); + } + #[cfg(unix)] #[test] fn exit_status_was_interrupted_on_sigint() { From d2f9537e6e9cb59081176f6cabc73740c6234694 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 18:49:42 +0000 Subject: [PATCH 05/14] Tighten wrapper fast-path safety checks --- src/git/command_classification.rs | 38 ++++++++++----- src/git/repository.rs | 81 +++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index 3a164789b..29aaa2e5f 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -211,26 +211,19 @@ fn is_read_only_config_invocation(parsed: &ParsedGitInvocation) -> bool { return false; } - let read_only_flags = [ + // Only explicit query actions are safe to fast-path. Other config flags like + // --type or --show-origin are modifiers and can still participate in writes. + let read_only_actions = [ "--blob", - "--default", "--get", "--get-all", "--get-regexp", "--get-urlmatch", - "--includes", "--list", - "--name-only", - "--no-includes", - "--null", - "--show-origin", - "--show-scope", - "--type", "-l", - "-z", ]; - command_args_contain_any(&parsed.command_args, &read_only_flags) + command_args_contain_any(&parsed.command_args, &read_only_actions) } fn is_read_only_worktree_invocation(parsed: &ParsedGitInvocation) -> bool { @@ -367,4 +360,27 @@ mod tests { ]); assert!(!is_read_only_invocation(&parsed)); } + + #[test] + fn read_only_invocation_rejects_config_write_with_type_modifier() { + let parsed = parse_git_cli_args(&[ + "config".to_string(), + "--type=bool".to_string(), + "demo.enabled".to_string(), + "true".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_accepts_config_get_with_modifiers() { + let parsed = parse_git_cli_args(&[ + "config".to_string(), + "--show-origin".to_string(), + "--type=bool".to_string(), + "--get".to_string(), + "demo.enabled".to_string(), + ]); + assert!(is_read_only_invocation(&parsed)); + } } diff --git a/src/git/repository.rs b/src/git/repository.rs index 606bcbebd..4f99befa8 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -2592,6 +2592,10 @@ fn is_fast_discovery_safe_global_flag(arg: &str) -> bool { } fn normalize_global_args_for_repo_root(global_args: &[String], command_root: &str) -> Vec { + if !global_args_are_safe_to_rebase(global_args) { + return global_args.to_vec(); + } + let mut normalized = vec!["-C".to_string(), command_root.to_string()]; let mut idx = 0usize; @@ -2615,6 +2619,33 @@ fn normalize_global_args_for_repo_root(global_args: &[String], command_root: &st normalized } +fn global_args_are_safe_to_rebase(global_args: &[String]) -> bool { + let mut idx = 0usize; + + while idx < global_args.len() { + let arg = &global_args[idx]; + + if arg == "-C" { + idx += 2; + continue; + } + + if arg.starts_with("-C") && arg.len() > 2 { + idx += 1; + continue; + } + + if is_fast_discovery_safe_global_flag(arg) { + idx += 1; + continue; + } + + return false; + } + + true +} + fn resolve_command_base_dir(global_args: &[String]) -> Result { let mut base: Option = None; let mut idx = 0usize; @@ -3496,6 +3527,7 @@ fn parse_hunk_header(line: &str) -> Option<(Vec, bool)> { #[cfg(test)] mod tests { use super::*; + use serial_test::serial; use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; @@ -3972,6 +4004,55 @@ index 0000000..abc1234 100644 ); } + #[test] + fn normalize_global_args_for_repo_root_preserves_relative_git_dir_and_work_tree() { + let args = vec![ + "--git-dir".to_string(), + "sub/repo/.git".to_string(), + "--work-tree".to_string(), + "sub/repo".to_string(), + ]; + + let normalized = normalize_global_args_for_repo_root(&args, "/repo/root"); + assert_eq!(normalized, args); + } + + #[test] + #[serial] + fn find_repository_preserves_relative_git_dir_and_work_tree_for_internal_commands() { + struct CurrentDirGuard(PathBuf); + + impl Drop for CurrentDirGuard { + fn drop(&mut self) { + std::env::set_current_dir(&self.0).expect("restore current dir"); + } + } + + let temp = tempfile::tempdir().expect("tempdir"); + let outer = temp.path().join("outer"); + let repo_dir = outer.join("sub").join("repo"); + fs::create_dir_all(&repo_dir).expect("create repo dir"); + + run_git(&repo_dir, &["init"]); + + let previous_dir = std::env::current_dir().expect("current dir"); + let _guard = CurrentDirGuard(previous_dir); + std::env::set_current_dir(&outer).expect("set current dir"); + + let args = vec![ + "--git-dir".to_string(), + "sub/repo/.git".to_string(), + "--work-tree".to_string(), + "sub/repo".to_string(), + ]; + let repo = find_repository(&args).expect("find repository"); + let output = repo + .git(&["config", "--get", "core.repositoryformatversion"]) + .expect("internal git command should preserve relative repo args"); + + assert_eq!(output.trim(), "0"); + } + #[test] fn find_repository_in_path_supports_bare_repositories() { let temp = tempfile::tempdir().expect("tempdir"); From ab11b1a01a2b0487bd9c3bdcc2f3c17cb0fc9088 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Thu, 26 Mar 2026 19:01:21 +0000 Subject: [PATCH 06/14] Simplify wrapper passthrough flow --- src/commands/git_handlers.rs | 43 +++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index e0f17f9d5..492c64512 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -112,14 +112,6 @@ pub fn handle_git(args: &[String]) { let mut parsed_args = parse_git_cli_args(args); - // Direct read-only commands never participate in git-ai hooks, so avoid the - // wrapper's repository/config preflight entirely and behave like a pure - // passthrough. This keeps commands like `git status` fast on Windows. - if should_passthrough_read_only_command(&parsed_args) { - let exit_status = proxy_to_git(args, false, None, None); - exit_with_status(exit_status); - } - let mut repository_option = find_repository(&parsed_args.global_args).ok(); parsed_args = resolve_wrapper_invocation(&parsed_args, repository_option.as_ref()); @@ -1223,6 +1215,41 @@ mod tests { assert!(!should_passthrough_read_only_command(&resolved)); } + #[test] + fn wrapper_does_not_passthrough_builtin_name_shadowed_by_mutating_alias() { + let temp = tempdir().expect("tempdir should create"); + let output = Command::new("git") + .args(["init", "-q"]) + .current_dir(temp.path()) + .output() + .expect("git init should run"); + assert!( + output.status.success(), + "git init failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let alias_output = Command::new("git") + .args(["config", "alias.status", "branch aliased HEAD"]) + .current_dir(temp.path()) + .output() + .expect("git config alias should run"); + assert!( + alias_output.status.success(), + "git config alias failed: {}", + String::from_utf8_lossy(&alias_output.stderr) + ); + + let repo = find_repository_in_path(&temp.path().to_string_lossy()) + .expect("repository should be discovered"); + let parsed = parse_git_cli_args(&["status".to_string()]); + + let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + assert_eq!(resolved.command.as_deref(), Some("branch")); + assert!(resolved.command_args.iter().any(|arg| arg == "aliased")); + assert!(!should_passthrough_read_only_command(&resolved)); + } + #[cfg(unix)] #[test] fn exit_status_was_interrupted_on_sigint() { From ea1f4bfec5cbec7e70efe2efd177bac60ccf4924 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Fri, 27 Mar 2026 15:03:53 +0000 Subject: [PATCH 07/14] Fix both Devin review findings on PR #809 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. try_find_repository_no_git_exec: catch construction errors from repository_from_discovered_paths and return Ok(None) so the git-exec fallback in find_repository still runs. Previously, errors like canonicalize failures or bad core.worktree config would propagate and silently skip hooks. 2. is_read_only_branch_invocation: split list-mode triggers from list-output modifiers (-v, --verbose, --no-color, --no-column, --ignore-case). Only triggers should classify as read-only when positional args are present. `git branch -v feature` creates a branch — it is NOT read-only. Co-Authored-By: Claude Opus 4.6 --- src/git/command_classification.rs | 26 +++++++++++++++++++------- src/git/repository.rs | 11 ++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index 29aaa2e5f..9e1bd4069 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -97,29 +97,29 @@ fn is_read_only_branch_invocation(parsed: &ParsedGitInvocation) -> bool { return false; } - let read_only_listing_flags = [ + // Flags that *trigger* list mode — their presence alone means read-only. + let list_mode_triggers = [ "--all", "--contains", "--format", - "--ignore-case", "--list", "--merged", - "--no-color", - "--no-column", "--no-contains", "--no-merged", "--points-at", "--remotes", "--show-current", "--sort", - "--verbose", "-a", "-l", "-r", - "-v", ]; - command_args_contain_any(&parsed.command_args, &read_only_listing_flags) + // Flags that only *modify* list output (e.g. -v, --no-color) but do NOT + // trigger list mode on their own. `git branch -v feature` creates a branch + // named "feature" — -v only means "verbose" in list mode, it does not + // activate list mode. + command_args_contain_any(&parsed.command_args, &list_mode_triggers) || parsed.pos_command(0).is_none() } @@ -293,6 +293,18 @@ mod tests { assert!(!is_read_only_invocation(&parsed)); } + #[test] + fn read_only_invocation_rejects_branch_create_with_verbose_flag() { + // `git branch -v feature` creates a branch; -v is a list-output modifier, + // not a list-mode trigger. + let parsed = parse_git_cli_args(&[ + "branch".to_string(), + "-v".to_string(), + "feature".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + #[test] fn read_only_invocation_detects_tag_listing() { let parsed = parse_git_cli_args(&["tag".to_string(), "--list".to_string()]); diff --git a/src/git/repository.rs b/src/git/repository.rs index 4f99befa8..fbb957688 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -2514,13 +2514,18 @@ fn try_find_repository_no_git_exec( let normalized_global_args = normalize_global_args_for_repo_root(global_args, &paths.command_root.to_string_lossy()); - repository_from_discovered_paths( + // If construction fails (e.g. canonicalize fails due to permissions/symlinks, + // or core.worktree config makes the assumed workdir incorrect), fall back to + // the git-exec discovery path rather than propagating the error. + match repository_from_discovered_paths( normalized_global_args, &paths.workdir, &paths.git_dir, &paths.git_common_dir, - ) - .map(Some) + ) { + Ok(repo) => Ok(Some(repo)), + Err(_) => Ok(None), + } } fn resolve_fast_discovery_base_dir(global_args: &[String]) -> Result, GitAiError> { From c1599b8bf0ebff7d662f706c85688598f1d12f25 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Fri, 27 Mar 2026 15:24:24 +0000 Subject: [PATCH 08/14] Expand read-only command classification for IDE/client plumbing Add high-volume read-only commands that IDEs (VS Code, JetBrains), git clients (GitLens, Graphite CLI), and other tools call frequently: Unconditionally read-only (added to is_definitely_read_only_command): ls-remote, show-ref, cherry, show-branch, whatchanged, verify-pack, check-ref-format, fsck, column, fmt-merge-msg, get-tar-commit-id, patch-id, stripspace Subcommand classifiers (added to is_read_only_invocation): symbolic-ref: read when 1 positional, write when 2+, reject -d/--delete reflog: deny-list (only expire/delete mutate); catches implicit show with refs/flags like `git reflog HEAD`, `git reflog --all` notes: allow-list (list/show/get-ref); must be conservative because --ref flag can precede the subcommand Co-Authored-By: Claude Opus 4.6 --- src/git/command_classification.rs | 245 ++++++++++++++++++++++++++++-- 1 file changed, 233 insertions(+), 12 deletions(-) diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index 9e1bd4069..2801b3529 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -3,37 +3,59 @@ use crate::git::cli_parser::ParsedGitInvocation; /// Returns true if the given git subcommand is guaranteed to never mutate /// repository state (refs, objects, config, worktree). Used to skip expensive /// trace2 ingestion work and suppress trace2 emission for read-only commands. +/// +/// This list covers both porcelain and plumbing commands that IDEs (VS Code, +/// JetBrains), git clients (GitLens, Graphite CLI), and other tools call at +/// high frequency. Only commands that are unconditionally read-only regardless +/// of arguments belong here; commands with mixed read/write modes (symbolic-ref, +/// reflog, notes, etc.) are handled by `is_read_only_invocation` instead. pub fn is_definitely_read_only_command(command: &str) -> bool { matches!( command, + // Porcelain read-only "blame" - | "cat-file" - | "check-attr" - | "check-ignore" - | "check-mailmap" - | "count-objects" + | "cherry" | "describe" | "diff" + | "grep" + | "help" + | "log" + | "shortlog" + | "show" + | "show-branch" + | "status" + | "version" + | "whatchanged" + // Plumbing — object/ref inspection + | "cat-file" | "diff-files" | "diff-index" | "diff-tree" | "for-each-ref" - | "grep" - | "help" - | "log" | "ls-files" + | "ls-remote" | "ls-tree" | "merge-base" | "name-rev" | "rev-list" | "rev-parse" - | "shortlog" - | "show" - | "status" + | "show-ref" | "var" | "verify-commit" + | "verify-pack" | "verify-tag" - | "version" + // Plumbing — query/validation helpers + | "check-attr" + | "check-ignore" + | "check-mailmap" + | "check-ref-format" + | "column" + | "count-objects" + | "fmt-merge-msg" + | "fsck" + | "get-tar-commit-id" + | "patch-id" + | "stripspace" ) } @@ -58,6 +80,9 @@ pub fn is_read_only_invocation(parsed: &ParsedGitInvocation) -> bool { Some("config") => is_read_only_config_invocation(parsed), Some("worktree") => is_read_only_worktree_invocation(parsed), Some("submodule") => is_read_only_submodule_invocation(parsed), + Some("symbolic-ref") => is_read_only_symbolic_ref_invocation(parsed), + Some("reflog") => is_read_only_reflog_invocation(parsed), + Some("notes") => is_read_only_notes_invocation(parsed), _ => false, } } @@ -240,6 +265,39 @@ fn is_read_only_submodule_invocation(parsed: &ParsedGitInvocation) -> bool { ) } +/// `git symbolic-ref HEAD` reads; `git symbolic-ref HEAD refs/heads/main` writes. +/// -d/--delete and -m (reason) with a target are also mutating. +fn is_read_only_symbolic_ref_invocation(parsed: &ParsedGitInvocation) -> bool { + if command_args_contain_any(&parsed.command_args, &["-d", "--delete"]) { + return false; + } + // Read mode: exactly one positional (the ref name to read). + // Write mode: two positionals (ref name + target). + parsed.pos_command(1).is_none() +} + +/// Only `git reflog expire` and `git reflog delete` are mutating. +/// Everything else — bare `git reflog`, `git reflog show`, `git reflog HEAD`, +/// `git reflog --all`, `git reflog exists` — is read-only (bare reflog and +/// unrecognized first args are interpreted by git as `reflog show`). +fn is_read_only_reflog_invocation(parsed: &ParsedGitInvocation) -> bool { + !matches!( + parsed.command_args.first().map(String::as_str), + Some("expire" | "delete") + ) +} + +/// `git notes list`, `git notes show` are read-only. +/// `git notes add/append/copy/edit/merge/prune/remove` are mutating. +/// Bare `git notes` defaults to `list`. +fn is_read_only_notes_invocation(parsed: &ParsedGitInvocation) -> bool { + match parsed.command_args.first().map(String::as_str) { + None => true, + Some("list" | "show" | "get-ref") => true, + _ => false, + } +} + #[cfg(test)] mod tests { use super::*; @@ -254,6 +312,16 @@ mod tests { assert!(is_definitely_read_only_command("log")); assert!(is_definitely_read_only_command("cat-file")); assert!(is_definitely_read_only_command("ls-files")); + // High-volume plumbing used by IDEs and git clients + assert!(is_definitely_read_only_command("ls-remote")); + assert!(is_definitely_read_only_command("show-ref")); + assert!(is_definitely_read_only_command("cherry")); + assert!(is_definitely_read_only_command("show-branch")); + assert!(is_definitely_read_only_command("for-each-ref")); + assert!(is_definitely_read_only_command("verify-pack")); + assert!(is_definitely_read_only_command("check-ref-format")); + assert!(is_definitely_read_only_command("fsck")); + assert!(is_definitely_read_only_command("whatchanged")); } #[test] @@ -395,4 +463,157 @@ mod tests { ]); assert!(is_read_only_invocation(&parsed)); } + + // symbolic-ref classifier + #[test] + fn read_only_invocation_detects_symbolic_ref_read() { + let parsed = parse_git_cli_args(&["symbolic-ref".to_string(), "HEAD".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_symbolic_ref_read_with_short() { + let parsed = parse_git_cli_args(&[ + "symbolic-ref".to_string(), + "--short".to_string(), + "HEAD".to_string(), + ]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_symbolic_ref_write() { + let parsed = parse_git_cli_args(&[ + "symbolic-ref".to_string(), + "HEAD".to_string(), + "refs/heads/main".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_symbolic_ref_delete() { + let parsed = parse_git_cli_args(&[ + "symbolic-ref".to_string(), + "--delete".to_string(), + "HEAD".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + // reflog classifier + #[test] + fn read_only_invocation_detects_bare_reflog() { + let parsed = parse_git_cli_args(&["reflog".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_reflog_show() { + let parsed = + parse_git_cli_args(&["reflog".to_string(), "show".to_string(), "HEAD".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_reflog_implicit_show_with_ref() { + // `git reflog HEAD` is `git reflog show HEAD` + let parsed = parse_git_cli_args(&["reflog".to_string(), "HEAD".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_reflog_implicit_show_with_flags() { + // `git reflog --all` is `git reflog show --all` + let parsed = parse_git_cli_args(&["reflog".to_string(), "--all".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_reflog_exists() { + let parsed = parse_git_cli_args(&[ + "reflog".to_string(), + "exists".to_string(), + "HEAD".to_string(), + ]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_reflog_expire() { + let parsed = parse_git_cli_args(&[ + "reflog".to_string(), + "expire".to_string(), + "--all".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_reflog_delete() { + let parsed = parse_git_cli_args(&[ + "reflog".to_string(), + "delete".to_string(), + "HEAD@{0}".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + // notes classifier + #[test] + fn read_only_invocation_detects_bare_notes() { + let parsed = parse_git_cli_args(&["notes".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_notes_list() { + let parsed = parse_git_cli_args(&["notes".to_string(), "list".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_notes_show() { + let parsed = + parse_git_cli_args(&["notes".to_string(), "show".to_string(), "HEAD".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_notes_add() { + let parsed = parse_git_cli_args(&[ + "notes".to_string(), + "add".to_string(), + "-m".to_string(), + "note".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_rejects_notes_remove() { + let parsed = parse_git_cli_args(&[ + "notes".to_string(), + "remove".to_string(), + "HEAD".to_string(), + ]); + assert!(!is_read_only_invocation(&parsed)); + } + + // ls-remote and show-ref as invocations + #[test] + fn read_only_invocation_detects_ls_remote() { + let parsed = parse_git_cli_args(&[ + "ls-remote".to_string(), + "--heads".to_string(), + "origin".to_string(), + ]); + assert!(is_read_only_invocation(&parsed)); + } + + #[test] + fn read_only_invocation_detects_show_ref() { + let parsed = parse_git_cli_args(&["show-ref".to_string(), "--heads".to_string()]); + assert!(is_read_only_invocation(&parsed)); + } } From 5d8e67aa04d11bbe112f6c1fa07f3f1dbe0d9a51 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Fri, 27 Mar 2026 15:36:40 +0000 Subject: [PATCH 09/14] Skip fast repo discovery when GIT_DIR/GIT_WORK_TREE env vars are set When GIT_DIR, GIT_WORK_TREE, or GIT_CEILING_DIRECTORIES environment variables are set, the filesystem-based fast discovery path may find a different repository than git's own discovery logic. Fall back to the git-exec path (git rev-parse) which honours these env vars. Addresses Devin review feedback on PR #809. Co-Authored-By: Claude Opus 4.6 --- src/git/repository.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/git/repository.rs b/src/git/repository.rs index fbb957688..083b018ad 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -2502,6 +2502,16 @@ pub fn find_repository(global_args: &[String]) -> Result fn try_find_repository_no_git_exec( global_args: &[String], ) -> Result, GitAiError> { + // When GIT_DIR, GIT_WORK_TREE, or GIT_CEILING_DIRECTORIES are set, the + // filesystem walk may disagree with git's own discovery logic. Fall back + // to the git-exec path so those env vars are honoured. + if std::env::var_os("GIT_DIR").is_some() + || std::env::var_os("GIT_WORK_TREE").is_some() + || std::env::var_os("GIT_CEILING_DIRECTORIES").is_some() + { + return Ok(None); + } + let Some(base_dir) = resolve_fast_discovery_base_dir(global_args)? else { return Ok(None); }; @@ -3988,6 +3998,32 @@ index 0000000..abc1234 100644 assert_eq!(resolved, None); } + #[test] + #[serial] + fn fast_discovery_skipped_when_git_dir_env_set() { + struct EnvGuard(&'static str, Option); + impl Drop for EnvGuard { + fn drop(&mut self) { + unsafe { + match &self.1 { + Some(val) => std::env::set_var(self.0, val), + None => std::env::remove_var(self.0), + } + } + } + } + + let prev = std::env::var_os("GIT_DIR"); + let _guard = EnvGuard("GIT_DIR", prev); + unsafe { std::env::set_var("GIT_DIR", "/tmp/other-repo/.git") }; + + let result = try_find_repository_no_git_exec(&[]).expect("should not error"); + assert!( + result.is_none(), + "fast path must be skipped when GIT_DIR is set" + ); + } + #[test] fn normalize_global_args_for_repo_root_collapses_chained_c_args() { let args = vec![ From 5f8ca760f210db0b9c78c15161d0e6684d8f3aa5 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 27 Mar 2026 21:46:30 +0000 Subject: [PATCH 10/14] Restore Clap parsing, add clone/init early-exit, port alias commit test - Revert Clap removal from main.rs to preserve help/usage behavior - Restore is_repo_creating early-exit for clone/init in async mode that was dropped during the refactor, preventing the daemon from receiving misleading wrapper state for repo-creating commands - Port async_mode_post_commit_shows_stats_with_commit_alias test from PR #801 which is now subsumed by this PR's alias resolution changes - Fix clippy match_like_matches_macro warning in is_read_only_notes_invocation Co-Authored-By: Sasha Varlamov --- src/commands/git_handlers.rs | 17 ++++++++++++++++ src/git/command_classification.rs | 9 ++++---- src/main.rs | 20 ++++++++++++++---- tests/async_mode.rs | 34 +++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 492c64512..b05ec14e4 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -120,6 +120,23 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } + // Repo-creating commands (clone, init) have no meaningful pre/post + // repo state — the target repo doesn't exist yet. The wrapper would + // either capture nothing (clone from outside a repo) or the wrong + // repo (clone from inside a different repo). Skip the invocation_id + // so the daemon doesn't wait for wrapper state that never arrives or + // is misleading; trace2 events still flow normally (trace2 suppression + // requires *both* no invocation_id and a read-only command). + if parsed_args + .command + .as_deref() + .is_some_and(|cmd| matches!(cmd, "clone" | "init")) + && !parsed_args.is_help + { + let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); + exit_with_status(exit_status); + } + // Async mode: wrapper should behave as a pure passthrough to git, // but capture and send authoritative pre/post state to the daemon. if config::Config::get().feature_flags().async_mode { diff --git a/src/git/command_classification.rs b/src/git/command_classification.rs index 2801b3529..6bfbb9088 100644 --- a/src/git/command_classification.rs +++ b/src/git/command_classification.rs @@ -291,11 +291,10 @@ fn is_read_only_reflog_invocation(parsed: &ParsedGitInvocation) -> bool { /// `git notes add/append/copy/edit/merge/prune/remove` are mutating. /// Bare `git notes` defaults to `list`. fn is_read_only_notes_invocation(parsed: &ParsedGitInvocation) -> bool { - match parsed.command_args.first().map(String::as_str) { - None => true, - Some("list" | "show" | "get-ref") => true, - _ => false, - } + matches!( + parsed.command_args.first().map(String::as_str), + None | Some("list" | "show" | "get-ref") + ) } #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index 1096c70d4..b7d0f70da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,16 @@ +use clap::Parser; use git_ai::commands; +#[derive(Parser)] +#[command(name = "git-ai")] +#[command(about = "git proxy with AI authorship tracking", long_about = None)] +#[command(disable_help_flag = true, disable_version_flag = true)] +struct Cli { + /// Git command and arguments + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + args: Vec, +} + fn main() { // Get the binary name that was called let binary_name = std::env::args_os() @@ -19,20 +30,21 @@ fn main() { commands::git_hook_handlers::handle_git_hook_invocation(&binary_name, &hook_args); std::process::exit(exit_code); } - let args: Vec = std::env::args().skip(1).collect(); + + let cli = Cli::parse(); #[cfg(debug_assertions)] { if std::env::var("GIT_AI").as_deref() == Ok("git") { - commands::git_handlers::handle_git(&args); + commands::git_handlers::handle_git(&cli.args); return; } } if binary_name == "git-ai" || binary_name == "git-ai.exe" { - commands::git_ai_handlers::handle_git_ai(&args); + commands::git_ai_handlers::handle_git_ai(&cli.args); std::process::exit(0); } - commands::git_handlers::handle_git(&args); + commands::git_handlers::handle_git(&cli.args); } diff --git a/tests/async_mode.rs b/tests/async_mode.rs index f7ef67521..6fe930fde 100644 --- a/tests/async_mode.rs +++ b/tests/async_mode.rs @@ -715,6 +715,40 @@ fn async_mode_post_commit_still_processing_when_no_daemon() { ); } +#[test] +fn async_mode_post_commit_shows_stats_with_commit_alias() { + let repo = TestRepo::new_with_mode(GitTestMode::WrapperDaemon); + + // Configure a git alias: cm = commit (mimics common user setup) + repo.git(&["config", "alias.cm", "commit"]).unwrap(); + + // Base commit (human only). + let mut file = repo.filename("alias_test.txt"); + file.set_contents(crate::lines!["Base line 1", "Base line 2"]); + repo.stage_all_and_commit("Base commit").unwrap(); + + // Add AI-attributed lines. + file.insert_at(2, crate::lines!["AI line 1".ai(), "AI line 2".ai()]); + + repo.git(&["add", "-A"]).expect("add should succeed"); + + // Commit using the alias instead of "commit" directly. + let output = repo + .git_with_env( + &["cm", "-m", "AI additions via alias"], + &[("GIT_AI_TEST_FORCE_TTY", "1")], + None, + ) + .expect("aliased commit should succeed"); + + // The wrapper should resolve the alias and still show the stats bar. + assert!( + output.contains("you") && output.contains("ai"), + "expected stats output when committing via alias, got:\n{}", + output + ); +} + #[test] fn async_mode_post_commit_skips_stats_for_large_commit() { let repo = TestRepo::new_with_mode(GitTestMode::WrapperDaemon); From e9963b3c12ef1b2d1da3cbcaaa27502e56f3467c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 27 Mar 2026 22:12:23 +0000 Subject: [PATCH 11/14] Simplify wrapper: inline redundant wrappers, merge global_args_are_safe_to_rebase into normalize_global_args_for_repo_root, DRY resolve_command_base_dir Co-Authored-By: Sasha Varlamov --- src/commands/git_handlers.rs | 50 ++++++++++++++------------------ src/git/repository.rs | 55 ++++++++---------------------------- 2 files changed, 33 insertions(+), 72 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index b05ec14e4..8cd58c41d 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -113,9 +113,15 @@ pub fn handle_git(args: &[String]) { let mut parsed_args = parse_git_cli_args(args); let mut repository_option = find_repository(&parsed_args.global_args).ok(); - parsed_args = resolve_wrapper_invocation(&parsed_args, repository_option.as_ref()); - if should_passthrough_read_only_command(&parsed_args) { + // Resolve aliases early so the read-only check sees the real command. + if let Some(repo) = repository_option.as_ref() + && let Some(resolved) = resolve_alias_invocation(&parsed_args, repo) + { + parsed_args = resolved; + } + + if crate::git::command_classification::is_read_only_invocation(&parsed_args) { let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); exit_with_status(exit_status); } @@ -270,18 +276,6 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } -fn should_passthrough_read_only_command(parsed_args: &ParsedGitInvocation) -> bool { - crate::git::command_classification::is_read_only_invocation(parsed_args) -} - -fn resolve_wrapper_invocation( - parsed_args: &ParsedGitInvocation, - repository: Option<&Repository>, -) -> ParsedGitInvocation { - repository - .and_then(|repo| resolve_alias_invocation(parsed_args, repo)) - .unwrap_or_else(|| parsed_args.clone()) -} /// Handle alias invocations #[cfg(feature = "test-support")] @@ -1002,10 +996,8 @@ fn in_shell_completion_context() -> bool { #[cfg(test)] mod tests { use super::parse_alias_tokens; - use super::{ - parse_git_cli_args, resolve_child_git_hooks_path_override, resolve_wrapper_invocation, - should_passthrough_read_only_command, - }; + use super::{parse_git_cli_args, resolve_alias_invocation, resolve_child_git_hooks_path_override}; + use crate::git::command_classification::is_read_only_invocation; use crate::git::find_repository_in_path; use std::process::Command; use tempfile::tempdir; @@ -1135,32 +1127,32 @@ mod tests { #[test] fn passthrough_read_only_command_for_status() { let parsed = parse_git_cli_args(&["status".to_string(), "--short".to_string()]); - assert!(should_passthrough_read_only_command(&parsed)); + assert!(is_read_only_invocation(&parsed)); } #[test] fn passthrough_read_only_command_for_branch_show_current() { let parsed = parse_git_cli_args(&["branch".to_string(), "--show-current".to_string()]); - assert!(should_passthrough_read_only_command(&parsed)); + assert!(is_read_only_invocation(&parsed)); } #[test] fn passthrough_read_only_command_rejects_mutating_commands() { let parsed = parse_git_cli_args(&["commit".to_string(), "-m".to_string(), "msg".to_string()]); - assert!(!should_passthrough_read_only_command(&parsed)); + assert!(!is_read_only_invocation(&parsed)); } #[test] fn passthrough_read_only_command_accepts_help_invocations() { let parsed = parse_git_cli_args(&["status".to_string(), "--help".to_string()]); - assert!(should_passthrough_read_only_command(&parsed)); + assert!(is_read_only_invocation(&parsed)); } #[test] fn passthrough_read_only_command_accepts_top_level_version() { let parsed = parse_git_cli_args(&["--version".to_string()]); - assert!(should_passthrough_read_only_command(&parsed)); + assert!(is_read_only_invocation(&parsed)); } #[test] @@ -1191,11 +1183,11 @@ mod tests { let repo = find_repository_in_path(&temp.path().to_string_lossy()) .expect("repository should be discovered"); let parsed = parse_git_cli_args(&["st".to_string()]); - let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + let resolved = resolve_alias_invocation(&parsed, &repo).unwrap_or_else(|| parsed.clone()); assert_eq!(resolved.command.as_deref(), Some("status")); assert!(resolved.command_args.iter().any(|arg| arg == "--short")); - assert!(should_passthrough_read_only_command(&resolved)); + assert!(is_read_only_invocation(&resolved)); } #[test] @@ -1226,10 +1218,10 @@ mod tests { let repo = find_repository_in_path(&temp.path().to_string_lossy()) .expect("repository should be discovered"); let parsed = parse_git_cli_args(&["ci".to_string(), "-m".to_string(), "msg".to_string()]); - let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + let resolved = resolve_alias_invocation(&parsed, &repo).unwrap_or_else(|| parsed.clone()); assert_eq!(resolved.command.as_deref(), Some("commit")); - assert!(!should_passthrough_read_only_command(&resolved)); + assert!(!is_read_only_invocation(&resolved)); } #[test] @@ -1261,10 +1253,10 @@ mod tests { .expect("repository should be discovered"); let parsed = parse_git_cli_args(&["status".to_string()]); - let resolved = resolve_wrapper_invocation(&parsed, Some(&repo)); + let resolved = resolve_alias_invocation(&parsed, &repo).unwrap_or_else(|| parsed.clone()); assert_eq!(resolved.command.as_deref(), Some("branch")); assert!(resolved.command_args.iter().any(|arg| arg == "aliased")); - assert!(!should_passthrough_read_only_command(&resolved)); + assert!(!is_read_only_invocation(&resolved)); } #[cfg(unix)] diff --git a/src/git/repository.rs b/src/git/repository.rs index 44a6f8345..00e4f0df4 100644 --- a/src/git/repository.rs +++ b/src/git/repository.rs @@ -2606,27 +2606,33 @@ fn is_fast_discovery_safe_global_flag(arg: &str) -> bool { ) } +/// Replace any `-C` arguments with a single `-C `, keeping other +/// global flags intact. When the args contain flags that aren't safe to rebase +/// (e.g. relative `--git-dir`/`--work-tree`), the original args are returned +/// unchanged so their meaning is preserved. fn normalize_global_args_for_repo_root(global_args: &[String], command_root: &str) -> Vec { - if !global_args_are_safe_to_rebase(global_args) { - return global_args.to_vec(); - } - let mut normalized = vec!["-C".to_string(), command_root.to_string()]; let mut idx = 0usize; while idx < global_args.len() { let arg = &global_args[idx]; + // Strip existing -C args (we prepended the canonical root above). if arg == "-C" { idx += 2; continue; } - if arg.starts_with("-C") && arg.len() > 2 { idx += 1; continue; } + // If the arg isn't a harmless global flag, we can't safely rebase. + // Return the original args unchanged. + if !is_fast_discovery_safe_global_flag(arg) { + return global_args.to_vec(); + } + normalized.push(arg.clone()); idx += 1; } @@ -2634,33 +2640,6 @@ fn normalize_global_args_for_repo_root(global_args: &[String], command_root: &st normalized } -fn global_args_are_safe_to_rebase(global_args: &[String]) -> bool { - let mut idx = 0usize; - - while idx < global_args.len() { - let arg = &global_args[idx]; - - if arg == "-C" { - idx += 2; - continue; - } - - if arg.starts_with("-C") && arg.len() > 2 { - idx += 1; - continue; - } - - if is_fast_discovery_safe_global_flag(arg) { - idx += 1; - continue; - } - - return false; - } - - true -} - fn resolve_command_base_dir(global_args: &[String]) -> Result { let mut base: Option = None; let mut idx = 0usize; @@ -2670,17 +2649,7 @@ fn resolve_command_base_dir(global_args: &[String]) -> Result existing.clone(), - None => std::env::current_dir().map_err(GitAiError::IoError)?, - }; - current.join(next_base) - }); + base = Some(apply_global_c_arg(base.as_ref(), path_arg)?); idx += 2; continue; } From 6d5d4e3ad6f3c7265154a375b6fd06bdf82ec38d Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 27 Mar 2026 22:16:23 +0000 Subject: [PATCH 12/14] Fix clone passthrough: scope to async-only so post_clone_hook runs in non-async mode Co-Authored-By: Sasha Varlamov --- src/commands/git_handlers.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 8cd58c41d..d9735c08b 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -126,19 +126,8 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } - // Repo-creating commands (clone, init) have no meaningful pre/post - // repo state — the target repo doesn't exist yet. The wrapper would - // either capture nothing (clone from outside a repo) or the wrong - // repo (clone from inside a different repo). Skip the invocation_id - // so the daemon doesn't wait for wrapper state that never arrives or - // is misleading; trace2 events still flow normally (trace2 suppression - // requires *both* no invocation_id and a read-only command). - if parsed_args - .command - .as_deref() - .is_some_and(|cmd| matches!(cmd, "clone" | "init")) - && !parsed_args.is_help - { + // `init` has no post-hooks, so it can always passthrough early. + if parsed_args.command.as_deref() == Some("init") && !parsed_args.is_help { let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); exit_with_status(exit_status); } @@ -146,6 +135,14 @@ pub fn handle_git(args: &[String]) { // Async mode: wrapper should behave as a pure passthrough to git, // but capture and send authoritative pre/post state to the daemon. if config::Config::get().feature_flags().async_mode { + // Clone in async mode has no meaningful pre/post repo state — the + // target repo doesn't exist yet. Skip daemon telemetry so the daemon + // doesn't wait for wrapper state that never arrives or is misleading. + if parsed_args.command.as_deref() == Some("clone") && !parsed_args.is_help { + let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); + exit_with_status(exit_status); + } + // Initialize the daemon telemetry handle so we can send wrapper state if let crate::daemon::telemetry_handle::DaemonTelemetryInitResult::Failed(e) = crate::daemon::telemetry_handle::init_daemon_telemetry_handle() From b7ac0736c022b5fda0398be15a138395fe185383 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 27 Mar 2026 22:36:28 +0000 Subject: [PATCH 13/14] Fix formatting: extra blank line and long import line Co-Authored-By: Sasha Varlamov --- src/commands/git_handlers.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index d9735c08b..79ac43d6a 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -273,7 +273,6 @@ pub fn handle_git(args: &[String]) { exit_with_status(exit_status); } - /// Handle alias invocations #[cfg(feature = "test-support")] pub fn resolve_alias_invocation( @@ -993,7 +992,9 @@ fn in_shell_completion_context() -> bool { #[cfg(test)] mod tests { use super::parse_alias_tokens; - use super::{parse_git_cli_args, resolve_alias_invocation, resolve_child_git_hooks_path_override}; + use super::{ + parse_git_cli_args, resolve_alias_invocation, resolve_child_git_hooks_path_override, + }; use crate::git::command_classification::is_read_only_invocation; use crate::git::find_repository_in_path; use std::process::Command; From 0333fc09e0583bd20c676cbf4a5c279c41d86f84 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 28 Mar 2026 00:46:39 +0000 Subject: [PATCH 14/14] Simplify: use is_definitely_read_only_command instead of is_read_only_invocation in wrapper dispatch Co-Authored-By: Sasha Varlamov --- src/commands/git_handlers.rs | 57 ++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/src/commands/git_handlers.rs b/src/commands/git_handlers.rs index 79ac43d6a..d511ee369 100644 --- a/src/commands/git_handlers.rs +++ b/src/commands/git_handlers.rs @@ -121,7 +121,16 @@ pub fn handle_git(args: &[String]) { parsed_args = resolved; } - if crate::git::command_classification::is_read_only_invocation(&parsed_args) { + // Fast-path: commands that are unconditionally read-only (status, log, diff, + // etc.) or help/version invocations can skip all hooks and daemon state. + let is_read_only = parsed_args.is_help + || parsed_args.command.is_none() + || parsed_args + .command + .as_deref() + .is_some_and(crate::git::command_classification::is_definitely_read_only_command); + + if is_read_only { let exit_status = proxy_to_git(&parsed_args.to_invocation_vec(), false, None, None); exit_with_status(exit_status); } @@ -805,7 +814,12 @@ fn proxy_to_git( // trace2 events for the daemon to match wrapper state entries. let suppress_trace2 = wrapper_invocation_id.is_none() && { let parsed = parse_git_cli_args(args); - crate::git::command_classification::is_read_only_invocation(&parsed) + parsed.is_help + || parsed.command.is_none() + || parsed + .command + .as_deref() + .is_some_and(crate::git::command_classification::is_definitely_read_only_command) }; // Use spawn for interactive commands @@ -995,7 +1009,7 @@ mod tests { use super::{ parse_git_cli_args, resolve_alias_invocation, resolve_child_git_hooks_path_override, }; - use crate::git::command_classification::is_read_only_invocation; + use crate::git::command_classification::is_definitely_read_only_command; use crate::git::find_repository_in_path; use std::process::Command; use tempfile::tempdir; @@ -1124,33 +1138,32 @@ mod tests { #[test] fn passthrough_read_only_command_for_status() { - let parsed = parse_git_cli_args(&["status".to_string(), "--short".to_string()]); - assert!(is_read_only_invocation(&parsed)); - } - - #[test] - fn passthrough_read_only_command_for_branch_show_current() { - let parsed = parse_git_cli_args(&["branch".to_string(), "--show-current".to_string()]); - assert!(is_read_only_invocation(&parsed)); + assert!(is_definitely_read_only_command("status")); } #[test] fn passthrough_read_only_command_rejects_mutating_commands() { - let parsed = - parse_git_cli_args(&["commit".to_string(), "-m".to_string(), "msg".to_string()]); - assert!(!is_read_only_invocation(&parsed)); + assert!(!is_definitely_read_only_command("commit")); } #[test] fn passthrough_read_only_command_accepts_help_invocations() { let parsed = parse_git_cli_args(&["status".to_string(), "--help".to_string()]); - assert!(is_read_only_invocation(&parsed)); + assert!(parsed.is_help); } #[test] fn passthrough_read_only_command_accepts_top_level_version() { + // `git --version` is rewritten to `git version` by the parser. let parsed = parse_git_cli_args(&["--version".to_string()]); - assert!(is_read_only_invocation(&parsed)); + assert!( + parsed.is_help + || parsed.command.is_none() + || parsed + .command + .as_deref() + .is_some_and(is_definitely_read_only_command) + ); } #[test] @@ -1185,7 +1198,9 @@ mod tests { assert_eq!(resolved.command.as_deref(), Some("status")); assert!(resolved.command_args.iter().any(|arg| arg == "--short")); - assert!(is_read_only_invocation(&resolved)); + assert!(is_definitely_read_only_command( + resolved.command.as_deref().unwrap() + )); } #[test] @@ -1219,7 +1234,9 @@ mod tests { let resolved = resolve_alias_invocation(&parsed, &repo).unwrap_or_else(|| parsed.clone()); assert_eq!(resolved.command.as_deref(), Some("commit")); - assert!(!is_read_only_invocation(&resolved)); + assert!(!is_definitely_read_only_command( + resolved.command.as_deref().unwrap() + )); } #[test] @@ -1254,7 +1271,9 @@ mod tests { let resolved = resolve_alias_invocation(&parsed, &repo).unwrap_or_else(|| parsed.clone()); assert_eq!(resolved.command.as_deref(), Some("branch")); assert!(resolved.command_args.iter().any(|arg| arg == "aliased")); - assert!(!is_read_only_invocation(&resolved)); + assert!(!is_definitely_read_only_command( + resolved.command.as_deref().unwrap() + )); } #[cfg(unix)]