From b4a78cbe246fddfb185d26680bb4a2e01f17114a Mon Sep 17 00:00:00 2001 From: Test User Date: Fri, 25 Jul 2025 16:38:41 +0900 Subject: [PATCH] feat: add user confirmation before executing hooks --- src/infrastructure/hooks.rs | 112 +++++++++++++++-- src/infrastructure/mod.rs | 2 +- tests/unit/infrastructure/hooks.rs | 193 ++++++++++++++++++++++++++++- 3 files changed, 298 insertions(+), 9 deletions(-) diff --git a/src/infrastructure/hooks.rs b/src/infrastructure/hooks.rs index 80a47a7..a9fdc45 100644 --- a/src/infrastructure/hooks.rs +++ b/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); + } } diff --git a/src/infrastructure/mod.rs b/src/infrastructure/mod.rs index d9f7539..6ff4492 100644 --- a/src/infrastructure/mod.rs +++ b/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; diff --git a/tests/unit/infrastructure/hooks.rs b/tests/unit/infrastructure/hooks.rs index 2725541..81c3c1b 100644 --- a/tests/unit/infrastructure/hooks.rs +++ b/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(()) +}