From 9daa0cc0bb8f3bbdd281e286573281f1f926cb2a Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Mon, 10 Aug 2026 06:13:14 +0000 Subject: [PATCH 1/2] Make hook execution working directory explicit --- crates/git-smee-cli/src/commands/run.rs | 73 +++++++++++++- crates/git-smee-cli/tests/cli_integration.rs | 101 ++++++++++++++++++- crates/git-smee-core/src/executor.rs | 61 ++++++++++- crates/git-smee-core/src/executor/runner.rs | 66 ++++++++++-- 4 files changed, 281 insertions(+), 20 deletions(-) diff --git a/crates/git-smee-cli/src/commands/run.rs b/crates/git-smee-cli/src/commands/run.rs index c5102b0..8f88e16 100644 --- a/crates/git-smee-cli/src/commands/run.rs +++ b/crates/git-smee-cli/src/commands/run.rs @@ -18,12 +18,22 @@ pub(crate) fn run_hook( hook: &str, hook_args: &[String], ) -> Result<(), Box> { - repository::ensure_in_repo_root()?; + let repository_root = repository::find_git_root()?; let phase = LifeCyclePhase::from_str(hook)?; let stdin_payload = read_hook_stdin_for_phase(phase)?; - let config = read_config_file(config_path)?; - let summary = - executor::execute_hook_with_summary(&config, phase, hook_args, stdin_payload.as_deref())?; + let config_path = if config_path.is_relative() { + repository_root.join(config_path) + } else { + config_path.to_path_buf() + }; + let config = read_config_file(&config_path)?; + let summary = executor::execute_hook_with_summary_in_directory( + &config, + phase, + &repository_root, + hook_args, + stdin_payload.as_deref(), + )?; for line in summary.text_lines(phase) { println!("{line}"); } @@ -94,8 +104,63 @@ fn stdin_sentinel_read_limit(max_hook_stdin_bytes: u64) -> u64 { #[cfg(test)] mod tests { + use std::{ + fs, + path::PathBuf, + sync::{LazyLock, Mutex}, + }; + + use tempfile::tempdir; + use super::*; + static CURRENT_DIR_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); + + struct CurrentDirGuard(PathBuf); + + impl Drop for CurrentDirGuard { + fn drop(&mut self) { + env::set_current_dir(&self.0).expect("failed to restore current directory"); + } + } + + fn assert_run_hook_preserves_current_dir(command: &str, expect_success: bool) { + let _lock = CURRENT_DIR_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let repo = tempdir().unwrap(); + git2::Repository::init(repo.path()).unwrap(); + let nested = repo.path().join("nested"); + fs::create_dir(&nested).unwrap(); + let config_path = repo.path().join("config.toml"); + fs::write( + &config_path, + format!("[[proc-receive]]\ncommand = {command:?}\n"), + ) + .unwrap(); + + let original_dir = env::current_dir().unwrap(); + let _current_dir_guard = CurrentDirGuard(original_dir); + env::set_current_dir(&nested).unwrap(); + + let result = run_hook(&config_path, "proc-receive", &[]); + + assert_eq!(result.is_ok(), expect_success); + assert_eq!(env::current_dir().unwrap(), nested.canonicalize().unwrap()); + } + + #[test] + fn successful_run_preserves_callers_current_directory() { + let command = if cfg!(windows) { "exit /b 0" } else { "true" }; + assert_run_hook_preserves_current_dir(command, true); + } + + #[test] + fn failed_run_preserves_callers_current_directory() { + let command = if cfg!(windows) { "exit /b 9" } else { "exit 9" }; + assert_run_hook_preserves_current_dir(command, false); + } + #[test] fn default_limit_uses_human_readable_display() { assert_eq!( diff --git a/crates/git-smee-cli/tests/cli_integration.rs b/crates/git-smee-cli/tests/cli_integration.rs index 8b2383c..1f292f7 100644 --- a/crates/git-smee-cli/tests/cli_integration.rs +++ b/crates/git-smee-cli/tests/cli_integration.rs @@ -1065,6 +1065,33 @@ command = "echo from-invocation-config" .success(); } +#[test] +fn given_nested_directory_when_running_then_hook_command_uses_repository_root() { + let test_repo = common::TestRepo::default(); + let nested_dir = test_repo.path.join("nested/deep"); + fs::create_dir_all(&nested_dir).expect("failed to create nested invocation dir"); + let command = if cfg!(windows) { + "echo repo-root> hook-cwd.txt" + } else { + "printf 'repo-root\\n' > hook-cwd.txt" + }; + test_repo.write_config(&format!("[[pre-commit]]\ncommand = {command:?}\n")); + + Command::new(cargo::cargo_bin!("git-smee")) + .current_dir(&nested_dir) + .args(["run", "pre-commit"]) + .assert() + .success(); + + assert_eq!( + fs::read_to_string(test_repo.path.join("hook-cwd.txt")) + .unwrap() + .trim(), + "repo-root" + ); + assert!(!nested_dir.join("hook-cwd.txt").exists()); +} + #[test] fn given_successful_hook_when_running_then_cli_stdout_contains_hook_output_and_summary() { let test_repo = common::TestRepo::default(); @@ -1400,12 +1427,14 @@ command = "exit 1" fn given_bare_repo_when_running_then_hook_executes() { let bare_repo = TempDir::new().expect("failed to create bare repo temp dir"); git2::Repository::init_bare(bare_repo.path()).expect("failed to init bare repo"); + let command = if cfg!(windows) { + "echo bare-root> bare-cwd.txt" + } else { + "printf 'bare-root\\n' > bare-cwd.txt" + }; fs::write( bare_repo.path().join(".git-smee.toml"), - r#" -[[pre-receive]] -command = "echo bare-run" -"#, + format!("[[pre-receive]]\ncommand = {command:?}\n"), ) .expect("failed to write config"); @@ -1414,6 +1443,70 @@ command = "echo bare-run" .args(["run", "pre-receive"]) .assert() .success(); + + assert_eq!( + fs::read_to_string(bare_repo.path().join("bare-cwd.txt")) + .unwrap() + .trim(), + "bare-root" + ); +} + +#[test] +fn given_linked_worktree_when_running_then_hook_command_uses_linked_worktree_root() { + let test_repo = common::TestRepo::default(); + let command = if cfg!(windows) { + "echo linked-root> linked-cwd.txt" + } else { + "printf 'linked-root\\n' > linked-cwd.txt" + }; + test_repo.write_config(&format!("[[pre-commit]]\ncommand = {command:?}\n")); + + let repository = git2::Repository::open(&test_repo.path).unwrap(); + let mut index = repository.index().unwrap(); + index + .add_path(Path::new(".git-smee.toml")) + .expect("failed to stage config"); + index.write().unwrap(); + let tree_id = index.write_tree().unwrap(); + let tree = repository.find_tree(tree_id).unwrap(); + let signature = git2::Signature::now("git-smee test", "git-smee@example.invalid").unwrap(); + repository + .commit( + Some("HEAD"), + &signature, + &signature, + "linked worktree fixture", + &tree, + &[], + ) + .unwrap(); + + let worktree_parent = TempDir::new().expect("failed to create worktree parent"); + let linked_root = worktree_parent.path().join("linked"); + let status = StdCommand::new("git") + .current_dir(&test_repo.path) + .args(["worktree", "add", "--detach"]) + .arg(&linked_root) + .status() + .expect("failed to create linked worktree"); + assert!(status.success(), "git worktree add failed with {status}"); + let nested_dir = linked_root.join("nested"); + fs::create_dir(&nested_dir).unwrap(); + + Command::new(cargo::cargo_bin!("git-smee")) + .current_dir(&nested_dir) + .args(["run", "pre-commit"]) + .assert() + .success(); + + assert_eq!( + fs::read_to_string(linked_root.join("linked-cwd.txt")) + .unwrap() + .trim(), + "linked-root" + ); + assert!(!test_repo.path.join("linked-cwd.txt").exists()); } #[test] diff --git a/crates/git-smee-core/src/executor.rs b/crates/git-smee-core/src/executor.rs index 5e7a909..1d288f9 100644 --- a/crates/git-smee-core/src/executor.rs +++ b/crates/git-smee-core/src/executor.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use thiserror::Error; pub(crate) mod redaction; @@ -7,7 +9,7 @@ mod summary; use crate::{SmeeConfig, config::LifeCyclePhase, platform::Platform}; -use runner::{CommandRunner, PlatformCommandRunner}; +use runner::{CommandRunner, PlatformCommandRunner, WorkingDirectory}; use scheduler::{run_hooks_with_runner, run_hooks_with_runner_with_summary}; pub use summary::{CommandPhase, CommandRun, HookRunSummary}; @@ -63,6 +65,24 @@ pub fn execute_hook_with_summary( let platform = Platform::current(); let runner = PlatformCommandRunner { platform: &platform, + working_directory: WorkingDirectory::Inherited, + }; + execute_hook_with_runner_and_summary(smee_config, phase, &runner, hook_args, stdin_payload) +} + +/// Executes a hook and captures its summary with every child command rooted at +/// `working_directory`. +pub fn execute_hook_with_summary_in_directory( + smee_config: &SmeeConfig, + phase: LifeCyclePhase, + working_directory: &Path, + hook_args: &[String], + stdin_payload: Option<&[u8]>, +) -> Result { + let platform = Platform::current(); + let runner = PlatformCommandRunner { + platform: &platform, + working_directory: WorkingDirectory::Explicit(working_directory), }; execute_hook_with_runner_and_summary(smee_config, phase, &runner, hook_args, stdin_payload) } @@ -93,6 +113,7 @@ pub fn execute_hook_with_platform_and_args_and_stdin( ) -> Result<(), Error> { let runner = PlatformCommandRunner { platform: &platform, + working_directory: WorkingDirectory::Inherited, }; execute_hook_with_runner(smee_config, phase, &runner, hook_args, stdin_payload) } @@ -134,7 +155,7 @@ mod tests { collections::{HashMap, VecDeque}, env, ffi::OsString, - io, + fs, io, process::Command, sync::{Arc, Barrier, Mutex}, time::Duration, @@ -282,6 +303,42 @@ mod tests { assert_eq!(runner.calls(), vec!["run-pre-commit"]); } + #[test] + fn given_explicit_working_directory_when_executing_then_child_uses_it() { + let working_directory = tempfile::tempdir().unwrap(); + let command = if cfg!(windows) { + "echo hook-cwd> hook-cwd.txt" + } else { + "printf 'hook-cwd\\n' > hook-cwd.txt" + }; + let mut hooks_map = HashMap::new(); + hooks_map.insert( + LifeCyclePhase::PreCommit, + vec![HookDefinition { + command: command.try_into().unwrap(), + parallel_execution_allowed: false, + }], + ); + let config = SmeeConfig::try_new(hooks_map).unwrap(); + + let summary = execute_hook_with_summary_in_directory( + &config, + LifeCyclePhase::PreCommit, + working_directory.path(), + &[], + None, + ) + .unwrap(); + + assert!(summary.error().is_none()); + assert_eq!( + fs::read_to_string(working_directory.path().join("hook-cwd.txt")) + .unwrap() + .trim(), + "hook-cwd" + ); + } + #[test] fn given_hook_args_when_executing_then_all_commands_receive_forwarded_args() { let mut hooks_map = std::collections::HashMap::new(); diff --git a/crates/git-smee-core/src/executor/runner.rs b/crates/git-smee-core/src/executor/runner.rs index 768f3c3..25fede6 100644 --- a/crates/git-smee-core/src/executor/runner.rs +++ b/crates/git-smee-core/src/executor/runner.rs @@ -1,14 +1,13 @@ use std::{ env, io::{self, ErrorKind, Write}, - process::Stdio, + path::{Path, PathBuf}, + process::{Command, Stdio}, thread, }; #[cfg(windows)] use std::os::windows::process::CommandExt; -#[cfg(windows)] -use std::path::PathBuf; use crate::{config::HookCommand, platform::Platform}; @@ -24,6 +23,13 @@ pub(super) trait CommandRunner: Sync { pub(super) struct PlatformCommandRunner<'a> { pub(super) platform: &'a Platform, + pub(super) working_directory: WorkingDirectory<'a>, +} + +#[derive(Clone, Copy)] +pub(super) enum WorkingDirectory<'a> { + Inherited, + Explicit(&'a Path), } impl CommandRunner for PlatformCommandRunner<'_> { @@ -55,11 +61,7 @@ impl CommandRunner for PlatformCommandRunner<'_> { if stdin_payload.is_some() { shell_command.stdin(Stdio::piped()); } - - #[cfg(windows)] - if let Some(current_dir) = cmd_compatible_current_dir()? { - shell_command.current_dir(current_dir); - } + apply_working_directory(&mut shell_command, self.working_directory)?; let mut child = shell_command.spawn()?; if let Some(stdin_payload) = stdin_payload { @@ -92,6 +94,43 @@ impl CommandRunner for PlatformCommandRunner<'_> { } } +fn apply_working_directory( + shell_command: &mut Command, + working_directory: WorkingDirectory<'_>, +) -> io::Result<()> { + match working_directory { + WorkingDirectory::Explicit(path) => { + shell_command.current_dir(command_working_directory(path)); + } + WorkingDirectory::Inherited => { + if let Some(current_dir) = inherited_compatible_working_directory()? { + shell_command.current_dir(current_dir); + } + } + } + Ok(()) +} + +#[cfg(not(windows))] +fn command_working_directory(path: &Path) -> PathBuf { + path.to_path_buf() +} + +#[cfg(windows)] +fn command_working_directory(path: &Path) -> PathBuf { + cmd_compatible_path(path) +} + +#[cfg(not(windows))] +fn inherited_compatible_working_directory() -> io::Result> { + Ok(None) +} + +#[cfg(windows)] +fn inherited_compatible_working_directory() -> io::Result> { + cmd_compatible_current_dir() +} + pub(super) fn create_windows_command_script( command: &str, ) -> Result { @@ -165,8 +204,15 @@ pub(super) fn apply_hook_arg_env(shell_command: &mut std::process::Command, hook #[cfg(windows)] pub(super) fn cmd_compatible_current_dir() -> Result, std::io::Error> { let current_dir = env::current_dir()?; - let current_dir = current_dir.to_string_lossy(); - Ok(current_dir.strip_prefix(r"\\?\").map(PathBuf::from)) + let compatible = cmd_compatible_path(¤t_dir); + Ok((compatible != current_dir).then_some(compatible)) +} + +#[cfg(windows)] +fn cmd_compatible_path(path: &Path) -> PathBuf { + let path = path.to_string_lossy(); + path.strip_prefix(r"\\?\") + .map_or_else(|| PathBuf::from(path.as_ref()), PathBuf::from) } pub(super) fn is_hook_arg_env_key(key: &str) -> bool { From 78c9aecfb22e02427582ec85316461f0b34b31f7 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Mon, 10 Aug 2026 06:18:52 +0000 Subject: [PATCH 2/2] Fix Windows path normalization tests --- crates/git-smee-cli/src/commands/run.rs | 3 ++- crates/git-smee-core/src/executor.rs | 20 +++++++++++++++++++- crates/git-smee-core/src/executor/runner.rs | 11 +++++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/crates/git-smee-cli/src/commands/run.rs b/crates/git-smee-cli/src/commands/run.rs index 8f88e16..52090d3 100644 --- a/crates/git-smee-cli/src/commands/run.rs +++ b/crates/git-smee-cli/src/commands/run.rs @@ -142,11 +142,12 @@ mod tests { let original_dir = env::current_dir().unwrap(); let _current_dir_guard = CurrentDirGuard(original_dir); env::set_current_dir(&nested).unwrap(); + let invocation_dir = env::current_dir().unwrap(); let result = run_hook(&config_path, "proc-receive", &[]); assert_eq!(result.is_ok(), expect_success); - assert_eq!(env::current_dir().unwrap(), nested.canonicalize().unwrap()); + assert_eq!(env::current_dir().unwrap(), invocation_dir); } #[test] diff --git a/crates/git-smee-core/src/executor.rs b/crates/git-smee-core/src/executor.rs index 1d288f9..8d68dfc 100644 --- a/crates/git-smee-core/src/executor.rs +++ b/crates/git-smee-core/src/executor.rs @@ -156,6 +156,7 @@ mod tests { env, ffi::OsString, fs, io, + path::{Path, PathBuf}, process::Command, sync::{Arc, Barrier, Mutex}, time::Duration, @@ -171,7 +172,8 @@ mod tests { use super::redaction::redact_command; use super::runner::{ - apply_hook_arg_env, is_hook_arg_env_key, windows_cmd_quote_hook_arg, windows_command_script, + apply_hook_arg_env, cmd_compatible_path, is_hook_arg_env_key, windows_cmd_quote_hook_arg, + windows_command_script, }; use super::scheduler::execute_command; use super::summary::{CommandOutcome, CommandRun}; @@ -373,6 +375,22 @@ mod tests { assert_eq!(script, "@echo off\r\nif \"%1\"==\"alpha\" exit /b 0\r\n"); } + #[test] + fn given_windows_verbatim_drive_path_when_normalizing_then_prefix_is_removed() { + assert_eq!( + cmd_compatible_path(Path::new(r"\\?\C:\repo")), + PathBuf::from(r"C:\repo") + ); + } + + #[test] + fn given_windows_verbatim_unc_path_when_normalizing_then_absolute_unc_is_preserved() { + assert_eq!( + cmd_compatible_path(Path::new(r"\\?\UNC\server\share\repo")), + PathBuf::from(r"\\server\share\repo") + ); + } + #[test] fn given_windows_hook_arg_with_cmd_metachar_when_quoting_then_it_is_wrapped() { assert_eq!( diff --git a/crates/git-smee-core/src/executor/runner.rs b/crates/git-smee-core/src/executor/runner.rs index 25fede6..603eab0 100644 --- a/crates/git-smee-core/src/executor/runner.rs +++ b/crates/git-smee-core/src/executor/runner.rs @@ -208,9 +208,16 @@ pub(super) fn cmd_compatible_current_dir() -> Result, std::io::E Ok((compatible != current_dir).then_some(compatible)) } -#[cfg(windows)] -fn cmd_compatible_path(path: &Path) -> PathBuf { +#[cfg(any(windows, test))] +pub(super) fn cmd_compatible_path(path: &Path) -> PathBuf { let path = path.to_string_lossy(); + const VERBATIM_UNC_PREFIX: &str = r"\\?\UNC\"; + if path + .get(..VERBATIM_UNC_PREFIX.len()) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case(VERBATIM_UNC_PREFIX)) + { + return PathBuf::from(format!(r"\\{}", &path[VERBATIM_UNC_PREFIX.len()..])); + } path.strip_prefix(r"\\?\") .map_or_else(|| PathBuf::from(path.as_ref()), PathBuf::from) }