Merge pull request #10 from wasabeef/feat-hook-confirm

feat: add user confirmation before executing hooks

Author: wasabeef

src/infrastructure/hooks.rs

@@ -34,6 +34,7 @@ use std::process::Command;
 
 use super::super::config::Config;
 use super::super::constants::*;
+use super::super::ui::UserInterface;
 
 /// Context information passed to hook commands
 ///
@@ -70,16 +71,17 @@ pub struct HookContext {
     pub worktree_path: PathBuf,
 }
 
-/// Executes configured hooks for a specific event type
+/// Executes configured hooks for a specific event type with user confirmation
 ///
 /// This function loads the configuration, looks up hooks for the specified
-/// event type, and executes them in order. Each command is run in a shell
-/// with the worktree directory as the working directory.
+/// event type, asks for user confirmation, and executes them in order.
+/// Each command is run in a shell with the worktree directory as the working directory.
 ///
 /// # Arguments
 ///
 /// * `hook_type` - The type of hook to execute (e.g., "post-create", "pre-remove")
 /// * `context` - Context information about the worktree
+/// * `ui` - User interface for confirmation prompts
 ///
 /// # Hook Types
 ///
@@ -96,16 +98,18 @@ pub struct HookContext {
 /// # Example
 ///
 /// ```no_run
-/// use git_workers::hooks::{execute_hooks, HookContext};
+/// use git_workers::hooks::{execute_hooks_with_ui, HookContext};
+/// use git_workers::ui::DialoguerUI;
 /// use std::path::PathBuf;
 ///
 /// let context = HookContext {
 ///     worktree_name: "feature-branch".to_string(),
 ///     worktree_path: PathBuf::from("/path/to/worktree"),
 /// };
+/// let ui = DialoguerUI;
 ///
 /// // Execute post-create hooks
-/// execute_hooks("post-create", &context).ok();
+/// execute_hooks_with_ui("post-create", &context, &ui).ok();
 /// ```
 ///
 /// # Configuration Loading
@@ -122,17 +126,47 @@ pub struct HookContext {
 ///
 /// Command execution errors (spawn failures) are also handled gracefully,
 /// allowing other hooks to continue even if one command fails to start.
-pub fn execute_hooks(hook_type: &str, context: &HookContext) -> Result<()> {
+pub fn execute_hooks_with_ui(
+    hook_type: &str,
+    context: &HookContext,
+    ui: &dyn UserInterface,
+) -> Result<()> {
     // Always load config from the current directory where the command is executed,
     // not from the newly created worktree which doesn't have a config yet
     let config = Config::load()?;
 
     if let Some(commands) = config.hooks.get(hook_type) {
+        if commands.is_empty() {
+            return Ok(());
+        }
+
+        // Ask for confirmation before running hooks
+        println!();
         println!(
-            "{} {hook_type} hooks...",
+            "{} {hook_type} hooks found:",
             INFO_RUNNING_HOOKS.replace("{}", "").trim()
         );
+        for cmd in commands {
+            let expanded_cmd = cmd
+                .replace(TEMPLATE_WORKTREE_NAME, &context.worktree_name)
+                .replace(
+                    TEMPLATE_WORKTREE_PATH,
+                    &context.worktree_path.display().to_string(),
+                );
+            println!("  • {expanded_cmd}");
+        }
+
+        println!();
+        let confirm = ui
+            .confirm_with_default(&format!("Execute {hook_type} hooks?"), true)
+            .unwrap_or(false);
 
+        if !confirm {
+            println!("Skipping {hook_type} hooks.");
+            return Ok(());
+        }
+
+        println!();
         for cmd in commands {
             // Replace template placeholders with actual values
             let expanded_cmd = cmd
@@ -183,9 +217,40 @@ pub fn execute_hooks(hook_type: &str, context: &HookContext) -> Result<()> {
     Ok(())
 }
 
+/// Executes configured hooks for a specific event type (legacy interface)
+///
+/// This is a convenience wrapper that creates a DialoguerUI instance
+/// for backward compatibility with existing code.
+///
+/// # Arguments
+///
+/// * `hook_type` - The type of hook to execute (e.g., "post-create", "pre-remove")
+/// * `context` - Context information about the worktree
+///
+/// # Example
+///
+/// ```no_run
+/// use git_workers::hooks::{execute_hooks, HookContext};
+/// use std::path::PathBuf;
+///
+/// let context = HookContext {
+///     worktree_name: "feature-branch".to_string(),
+///     worktree_path: PathBuf::from("/path/to/worktree"),
+/// };
+///
+/// // Execute post-create hooks
+/// execute_hooks("post-create", &context).ok();
+/// ```
+pub fn execute_hooks(hook_type: &str, context: &HookContext) -> Result<()> {
+    use super::super::ui::DialoguerUI;
+    let ui = DialoguerUI;
+    execute_hooks_with_ui(hook_type, context, &ui)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::ui::MockUI;
     use tempfile::TempDir;
 
     #[test]
@@ -257,4 +322,37 @@ mod tests {
 
         assert_eq!(expanded, "npm install");
     }
+
+    #[test]
+    fn test_hook_execution_with_confirmation() {
+        let context = HookContext {
+            worktree_name: "test".to_string(),
+            worktree_path: PathBuf::from("/test/path"),
+        };
+
+        // Test with confirmation accepted
+        let ui = MockUI::new().with_confirm(true);
+        // This would require a full test setup with config
+        // but we can test the interface exists
+        let _result = execute_hooks_with_ui("post-create", &context, &ui);
+
+        // Test with confirmation rejected
+        let ui = MockUI::new().with_confirm(false);
+        let _result = execute_hooks_with_ui("post-create", &context, &ui);
+    }
+
+    #[test]
+    fn test_hook_confirmation_prompt_display() {
+        // Test that proper hook information is displayed before confirmation
+        let context = HookContext {
+            worktree_name: "feature-xyz".to_string(),
+            worktree_path: PathBuf::from("/workspace/feature-xyz"),
+        };
+
+        // Mock UI that rejects confirmation
+        let ui = MockUI::new().with_confirm(false);
+
+        // In real usage, this would show hook commands before asking
+        let _result = execute_hooks_with_ui("post-create", &context, &ui);
+    }
 }

src/infrastructure/mod.rs

@@ -15,7 +15,7 @@ pub mod hooks;
 pub use file_copy::copy_configured_files;
 pub use filesystem::{FileSystem, RealFileSystem};
 pub use git::{GitWorktreeManager, WorktreeInfo};
-pub use hooks::{execute_hooks, HookContext};
+pub use hooks::{execute_hooks, execute_hooks_with_ui, HookContext};
 
 // Re-export FilesConfig from config module
 pub use super::config::FilesConfig;

tests/unit/infrastructure/hooks.rs

@@ -4,7 +4,8 @@
 //! and template variable substitution.
 
 use anyhow::Result;
-use git_workers::infrastructure::hooks::{execute_hooks, HookContext};
+use git_workers::infrastructure::hooks::{execute_hooks, execute_hooks_with_ui, HookContext};
+use git_workers::ui::MockUI;
 use std::fs;
 use std::path::PathBuf;
 use tempfile::TempDir;
@@ -236,3 +237,193 @@ fn test_hook_execution_flow_simulation() -> Result<()> {
 
     Ok(())
 }
+
+// ============================================================================
+// Hook Confirmation Tests
+// ============================================================================
+
+#[test]
+#[ignore = "Hook execution requires specific command availability"]
+fn test_hook_confirmation_accepted() -> Result<()> {
+    let temp_dir = TempDir::new()?;
+
+    // Initialize git repository
+    git2::Repository::init(temp_dir.path())?;
+
+    // Create config with hooks
+    let config_content = r#"
+[hooks]
+post-create = ["echo 'test hook'"]
+"#;
+    fs::write(temp_dir.path().join(".git-workers.toml"), config_content)?;
+
+    let original_dir = std::env::current_dir()?;
+    std::env::set_current_dir(temp_dir.path())?;
+
+    let context = HookContext {
+        worktree_name: "test".to_string(),
+        worktree_path: temp_dir.path().to_path_buf(),
+    };
+
+    // Mock UI that accepts confirmation
+    let ui = MockUI::new().with_confirm(true);
+
+    // Execute hooks with UI - should succeed when confirmation is accepted
+    let result = execute_hooks_with_ui("post-create", &context, &ui);
+
+    std::env::set_current_dir(original_dir)?;
+
+    assert!(result.is_ok());
+    Ok(())
+}
+
+#[test]
+#[ignore = "Hook execution requires specific command availability"]
+fn test_hook_confirmation_rejected() -> Result<()> {
+    let temp_dir = TempDir::new()?;
+
+    // Initialize git repository
+    git2::Repository::init(temp_dir.path())?;
+
+    // Create config with hooks
+    let config_content = r#"
+[hooks]
+post-create = ["echo 'test hook'"]
+"#;
+    fs::write(temp_dir.path().join(".git-workers.toml"), config_content)?;
+
+    let original_dir = std::env::current_dir()?;
+    std::env::set_current_dir(temp_dir.path())?;
+
+    let context = HookContext {
+        worktree_name: "test".to_string(),
+        worktree_path: temp_dir.path().to_path_buf(),
+    };
+
+    // Mock UI that rejects confirmation
+    let ui = MockUI::new().with_confirm(false);
+
+    // Execute hooks with UI - should succeed but skip execution
+    let result = execute_hooks_with_ui("post-create", &context, &ui);
+
+    std::env::set_current_dir(original_dir)?;
+
+    // Should still return Ok even when hooks are skipped
+    assert!(result.is_ok());
+    Ok(())
+}
+
+#[test]
+#[ignore = "Hook execution requires specific command availability"]
+fn test_hook_with_template_variables_confirmation() -> Result<()> {
+    let temp_dir = TempDir::new()?;
+
+    // Initialize git repository
+    git2::Repository::init(temp_dir.path())?;
+
+    // Create config with hooks using template variables
+    let config_content = r#"
+[hooks]
+post-create = [
+    "echo 'Created worktree: {{worktree_name}}'",
+    "echo 'At path: {{worktree_path}}'"
+]
+"#;
+    fs::write(temp_dir.path().join(".git-workers.toml"), config_content)?;
+
+    let original_dir = std::env::current_dir()?;
+    std::env::set_current_dir(temp_dir.path())?;
+
+    let context = HookContext {
+        worktree_name: "feature-xyz".to_string(),
+        worktree_path: PathBuf::from("/workspace/feature-xyz"),
+    };
+
+    // Mock UI that accepts confirmation
+    let ui = MockUI::new().with_confirm(true);
+
+    // Execute hooks - template variables should be expanded in display
+    let result = execute_hooks_with_ui("post-create", &context, &ui);
+
+    std::env::set_current_dir(original_dir)?;
+
+    assert!(result.is_ok());
+    Ok(())
+}
+
+#[test]
+#[ignore = "Hook execution requires specific command availability"]
+fn test_multiple_hook_types_with_confirmation() -> Result<()> {
+    let temp_dir = TempDir::new()?;
+
+    // Initialize git repository
+    git2::Repository::init(temp_dir.path())?;
+
+    // Create config with multiple hook types
+    let config_content = r#"
+[hooks]
+post-create = ["echo 'post-create'"]
+pre-remove = ["echo 'pre-remove'"]
+post-switch = ["echo 'post-switch'"]
+"#;
+    fs::write(temp_dir.path().join(".git-workers.toml"), config_content)?;
+
+    let original_dir = std::env::current_dir()?;
+    std::env::set_current_dir(temp_dir.path())?;
+
+    let context = HookContext {
+        worktree_name: "test".to_string(),
+        worktree_path: temp_dir.path().to_path_buf(),
+    };
+
+    // Test each hook type with different confirmation responses
+    let hook_types = vec![
+        ("post-create", true),
+        ("pre-remove", false),
+        ("post-switch", true),
+    ];
+
+    for (hook_type, confirm) in hook_types {
+        let ui = MockUI::new().with_confirm(confirm);
+        let result = execute_hooks_with_ui(hook_type, &context, &ui);
+        assert!(result.is_ok(), "Hook type {hook_type} should succeed");
+    }
+
+    std::env::set_current_dir(original_dir)?;
+    Ok(())
+}
+
+#[test]
+#[ignore = "Hook execution requires specific command availability"]
+fn test_empty_hooks_no_confirmation_needed() -> Result<()> {
+    let temp_dir = TempDir::new()?;
+
+    // Initialize git repository
+    git2::Repository::init(temp_dir.path())?;
+
+    // Create config with empty hooks
+    let config_content = r#"
+[hooks]
+post-create = []
+"#;
+    fs::write(temp_dir.path().join(".git-workers.toml"), config_content)?;
+
+    let original_dir = std::env::current_dir()?;
+    std::env::set_current_dir(temp_dir.path())?;
+
+    let context = HookContext {
+        worktree_name: "test".to_string(),
+        worktree_path: temp_dir.path().to_path_buf(),
+    };
+
+    // Mock UI without any confirmations configured
+    let ui = MockUI::new();
+
+    // Should succeed without asking for confirmation
+    let result = execute_hooks_with_ui("post-create", &context, &ui);
+
+    std::env::set_current_dir(original_dir)?;
+
+    assert!(result.is_ok());
+    Ok(())
+}