feat: run ToolCallBefore hooks before tool execution

crates/tui/src/core/engine.rs

@@ -166,6 +166,9 @@ pub struct EngineConfig {
     /// Tool restriction from custom slash command frontmatter.
     /// `None` means the current turn may use the normal tool set.
     pub allowed_tools: Option<Vec<String>>,
+    /// Hook executor for control-plane hooks.
+    /// `ToolCallBefore` hooks may deny a tool call with exit code 2.
+    pub hook_executor: Option<std::sync::Arc<crate::hooks::HookExecutor>>,
     /// Resolved BCP-47 locale tag (e.g. `"en"`, `"zh-Hans"`, `"ja"`)
     /// for the `## Environment` block in the system prompt. The
     /// caller resolves this from `Settings` once at engine
@@ -237,6 +240,7 @@ impl Default for EngineConfig {
             strict_tool_mode: false,
             goal_objective: None,
             allowed_tools: None,
+            hook_executor: None,
             locale_tag: "en".to_string(),
             workshop: None,
             search_provider: crate::config::SearchProvider::default(),
@@ -650,6 +654,7 @@ impl Engine {
                     translation_enabled,
                     show_thinking,
                     allowed_tools,
+                    hook_executor,
                 } => {
                     self.handle_send_message(
                         content,
@@ -666,6 +671,7 @@ impl Engine {
                         translation_enabled,
                         show_thinking,
                         allowed_tools,
+                        hook_executor,
                     )
                     .await;
                 }
@@ -884,6 +890,7 @@ impl Engine {
                         self.config.translation_enabled,
                         self.config.show_thinking,
                         self.config.allowed_tools.clone(),
+                        self.config.hook_executor.clone(),
                     )
                     .await;
                 }
@@ -1008,6 +1015,7 @@ impl Engine {
         translation_enabled: bool,
         show_thinking: bool,
         allowed_tools: Option<Vec<String>>,
+        hook_executor: Option<std::sync::Arc<crate::hooks::HookExecutor>>,
     ) {
         // Reset cancel token for fresh turn (in case previous was cancelled)
         self.reset_cancel_token();
@@ -1114,6 +1122,7 @@ impl Engine {
             );
         }
         self.config.allowed_tools = allowed_tools;
+        self.config.hook_executor = hook_executor;
         self.session.reasoning_effort = reasoning_effort;
         self.session.reasoning_effort_auto = reasoning_effort_auto;
         self.session.auto_model = auto_model;

crates/tui/src/core/engine/turn_loop.rs

@@ -1255,13 +1255,17 @@ impl Engine {
                     )));
                 }
 
-                if !command_allows_tool(self.config.allowed_tools.as_deref(), &tool_name) {
+                if blocked_error.is_none()
+                    && !command_allows_tool(self.config.allowed_tools.as_deref(), &tool_name)
+                {
                     blocked_error = Some(ToolError::permission_denied(format!(
                         "Tool '{tool_name}' is not in the allowed-tools list for the current command"
                     )));
                 }
 
-                if !caller_allowed_for_tool(tool_caller.as_ref(), tool_def) {
+                if blocked_error.is_none()
+                    && !caller_allowed_for_tool(tool_caller.as_ref(), tool_def)
+                {
                     blocked_error = Some(ToolError::permission_denied(format!(
                         "Tool '{tool_name}' does not allow caller '{}'",
                         caller_type_for_tool_use(tool_caller.as_ref())
@@ -1281,6 +1285,68 @@ impl Engine {
                     )));
                 }
 
+                if blocked_error.is_none()
+                    && let Some(hook_executor) = self.config.hook_executor.as_ref()
+                    && hook_executor.has_hooks_for_event(crate::hooks::HookEvent::ToolCallBefore)
+                {
+                    // Warn if any ToolCallBefore hook is configured as background
+                    // — background hooks return exit_code: None immediately, so
+                    // the denial check (exit_code == Some(2)) can never match.
+                    if hook_executor
+                        .has_background_hooks_for_event(crate::hooks::HookEvent::ToolCallBefore)
+                    {
+                        tracing::warn!(
+                            "ToolCallBefore hook(s) configured with background=true — \
+                             background hooks cannot deny tool calls because they exit \
+                             immediately with no result"
+                        );
+                    }
+
+                    let hook_context = crate::hooks::HookContext::new()
+                        .with_tool_name(&tool_name)
+                        .with_tool_args(&tool_input)
+                        .with_mode(&format!("{mode:?}"))
+                        .with_workspace(self.session.workspace.clone())
+                        .with_model(&self.config.model)
+                        .with_session_id(&self.session.id);
+                    // Run hooks off the Tokio worker thread: `execute()` calls
+                    // `child.wait_timeout()` which is a blocking syscall that
+                    // would stall all other async tasks on this thread.
+                    let executor = hook_executor.clone();
+                    let hook_results = tokio::task::spawn_blocking(move || {
+                        executor.execute(crate::hooks::HookEvent::ToolCallBefore, &hook_context)
+                    })
+                    .await
+                    .unwrap_or_else(|join_err| {
+                        tracing::error!("Hook executor task panicked: {join_err}");
+                        Vec::new()
+                    });
+                    if let Some(denial) = hook_results
+                        .iter()
+                        .find(|result| result.exit_code == Some(2))
+                    {
+                        let reason = denial
+                            .stdout
+                            .trim()
+                            .lines()
+                            .next()
+                            .filter(|line| !line.is_empty())
+                            .or_else(|| {
+                                denial
+                                    .stderr
+                                    .trim()
+                                    .lines()
+                                    .next()
+                                    .filter(|line| !line.is_empty())
+                            })
+                            .or(denial.error.as_deref())
+                            .unwrap_or("ToolCallBefore hook denied tool execution");
+                        blocked_error = Some(ToolError::permission_denied(format!(
+                            "ToolCallBefore hook denied tool '{tool_name}': {reason}"
+                        )));
+                    }
+                }
+
                 if McpPool::is_mcp_tool(&tool_name) {
                     read_only = mcp_tool_is_read_only(&tool_name);
                     supports_parallel = mcp_tool_is_parallel_safe(&tool_name);
@@ -2514,4 +2580,105 @@ mod tests {
         let allowed = vec!["read_file".to_string()];
         assert!(command_allows_tool(Some(&allowed), &tool_name));
     }
+
+    #[test]
+    fn hook_gate_denies_with_exit_code_2() {
+        use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
+
+        let deny_cmd = if cfg!(windows) { "exit /b 2" } else { "exit 2" };
+        let config = HooksConfig {
+            enabled: true,
+            hooks: vec![Hook::new(HookEvent::ToolCallBefore, deny_cmd)],
+            ..HooksConfig::default()
+        };
+        let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
+        let ctx = HookContext::new()
+            .with_tool_name("exec_shell")
+            .with_tool_args(&serde_json::json!({}));
+        let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
+
+        assert_eq!(results.len(), 1);
+        assert_eq!(results[0].exit_code, Some(2));
+    }
+
+    #[test]
+    fn hook_gate_allows_with_exit_code_0() {
+        use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
+
+        let allow_cmd = if cfg!(windows) { "exit /b 0" } else { "exit 0" };
+        let config = HooksConfig {
+            enabled: true,
+            hooks: vec![Hook::new(HookEvent::ToolCallBefore, allow_cmd)],
+            ..HooksConfig::default()
+        };
+        let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
+        let ctx = HookContext::new()
+            .with_tool_name("read_file")
+            .with_tool_args(&serde_json::json!({}));
+        let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
+
+        assert_eq!(results.len(), 1);
+        assert_eq!(results[0].exit_code, Some(0));
+        assert!(results[0].success);
+    }
+
+    #[test]
+    fn hook_gate_failure_exit_code_1_is_not_denial() {
+        use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
+
+        let fail_cmd = if cfg!(windows) { "exit /b 1" } else { "exit 1" };
+        let config = HooksConfig {
+            enabled: true,
+            hooks: vec![Hook::new(HookEvent::ToolCallBefore, fail_cmd)],
+            ..HooksConfig::default()
+        };
+        let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
+        let ctx = HookContext::new()
+            .with_tool_name("write_file")
+            .with_tool_args(&serde_json::json!({}));
+        let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
+
+        assert_eq!(results.len(), 1);
+        assert_eq!(results[0].exit_code, Some(1));
+        assert_ne!(results[0].exit_code, Some(2));
+    }
+
+    #[test]
+    fn hook_gate_no_hooks_returns_no_results() {
+        use crate::hooks::{HookContext, HookEvent, HookExecutor, HooksConfig};
+
+        let config = HooksConfig {
+            enabled: true,
+            hooks: vec![],
+            ..HooksConfig::default()
+        };
+        let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
+        let ctx = HookContext::new().with_tool_name("grep_files");
+        let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
+
+        assert!(results.is_empty());
+    }
+
+    #[test]
+    fn hook_gate_denial_reason_can_come_from_stdout() {
+        use crate::hooks::{Hook, HookContext, HookEvent, HookExecutor, HooksConfig};
+
+        let deny_cmd = if cfg!(windows) {
+            "echo Tool blocked by security policy & exit /b 2"
+        } else {
+            "echo 'Tool blocked by security policy' && exit 2"
+        };
+        let config = HooksConfig {
+            enabled: true,
+            hooks: vec![Hook::new(HookEvent::ToolCallBefore, deny_cmd)],
+            ..HooksConfig::default()
+        };
+        let executor = HookExecutor::new(config, std::path::PathBuf::from("."));
+        let ctx = HookContext::new().with_tool_name("exec_shell");
+        let results = executor.execute(HookEvent::ToolCallBefore, &ctx);
+
+        assert_eq!(results.len(), 1);
+        assert_eq!(results[0].exit_code, Some(2));
+        assert!(results[0].stdout.contains("security"));
+    }
 }

crates/tui/src/core/ops.rs

@@ -35,6 +35,9 @@ pub enum Op {
         /// Tool restriction from custom slash command frontmatter.
         /// `None` means the current turn may use the normal tool set.
         allowed_tools: Option<Vec<String>>,
+        /// Hook executor for control-plane hooks.
+        /// `ToolCallBefore` hooks may deny a tool call with exit code 2.
+        hook_executor: Option<std::sync::Arc<crate::hooks::HookExecutor>>,
     },
 
     /// Cancel the current request

crates/tui/src/hooks.rs

@@ -551,6 +551,23 @@ impl HookExecutor {
         self.config.enabled && self.config.hooks.iter().any(|h| h.event == event)
     }
 
+    /// Check if there are any background hooks configured for a specific event.
+    ///
+    /// Background hooks fire and forget — their `exit_code` is always `None`,
+    /// so they cannot deny tool calls. This is a known limitation; the check
+    /// is used to warn operators when a `ToolCallBefore` hook is configured
+    /// as background but expects to block a tool.
+    #[must_use]
+    pub fn has_background_hooks_for_event(&self, event: HookEvent) -> bool {
+        if !self.config.enabled {
+            return false;
+        }
+        self.config
+            .hooks
+            .iter()
+            .any(|h| h.event == event && h.background)
+    }
+
     /// Run configured `message_submit` hooks as a mutable submit pipeline.
     ///
     /// This is deliberately separate from [`Self::execute`]: most hook events

crates/tui/src/main.rs

@@ -5379,6 +5379,7 @@ async fn run_exec_agent(
         strict_tool_mode: config.strict_tool_mode.unwrap_or(false),
         goal_objective: None,
         allowed_tools: None,
+        hook_executor: None,
         locale_tag: crate::localization::resolve_locale(&settings.locale)
             .tag()
             .to_string(),
@@ -5435,6 +5436,7 @@ async fn run_exec_agent(
             model: effective_model.clone(),
             goal_objective: None,
             allowed_tools: None,
+            hook_executor: None,
             reasoning_effort: effective_reasoning_effort,
             reasoning_effort_auto: auto_model,
             auto_model,

crates/tui/src/runtime_threads.rs

@@ -1654,6 +1654,7 @@ impl RuntimeThreadManager {
                 translation_enabled: false,
                 show_thinking,
                 allowed_tools: None,
+                hook_executor: None,
                 approval_mode: if auto_approve {
                     crate::tui::approval::ApprovalMode::Auto
                 } else {
@@ -2020,6 +2021,7 @@ impl RuntimeThreadManager {
             strict_tool_mode: self.config.strict_tool_mode.unwrap_or(false),
             goal_objective: None,
             allowed_tools: None,
+            hook_executor: None,
             locale_tag: crate::localization::resolve_locale(&settings.locale)
                 .tag()
                 .to_string(),

crates/tui/src/tui/tool_routing.rs

@@ -22,19 +22,11 @@ pub(super) fn handle_tool_call_started(
     name: &str,
     input: &serde_json::Value,
 ) {
-    // #455 (observer-only): fire `tool_call_before` hooks here, before
-    // any UI bookkeeping. Hooks are read-only observers in this slice
-    // — they can log, notify, or audit, but cannot mutate the args.
-    // Fast-path skip when no hooks are configured so per-tool
-    // dispatch doesn't pay for context construction in the common
-    // case (most users have no hooks).
-    if app.hooks.has_hooks_for_event(HookEvent::ToolCallBefore) {
-        let context = app
-            .base_hook_context()
-            .with_tool_name(name)
-            .with_tool_args(input);
-        let _ = app.execute_hooks(HookEvent::ToolCallBefore, &context);
-    }
+    // #2511: ToolCallBefore gate moved to turn-loop planning loop
+    // (Engine::handle_deepseek_turn). Removing observer-only firing
+    // here to avoid double-firing hooks for each tool call.
+    // Hooks that need observation can configure ToolCallBefore on
+    // the turn-loop gate — it processes the denial (exit code 2).
 
     let id = id.to_string();
 

crates/tui/src/tui/ui.rs

@@ -503,6 +503,13 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> {
         .shell_manager
         .clone()
         .unwrap_or_else(|| crate::tools::shell::new_shared_shell_manager(app.workspace.clone()));
+    // #2511: ensure hook_executor is initialized for fresh sessions — it is
+    // only set by apply_workspace_runtime_state (session resume / workspace
+    // switch), so a brand-new session would otherwise leave it None and both
+    // exec_shell shell_env hooks and ToolCallBefore gate would silently no-op.
+    if app.runtime_services.hook_executor.is_none() {
+        app.runtime_services.hook_executor = Some(std::sync::Arc::new(app.hooks.clone()));
+    }
     app.runtime_services = RuntimeToolServices {
         shell_manager: Some(shell_manager),
         task_manager: Some(task_manager.clone()),
@@ -511,8 +518,8 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> {
         active_task_id: None,
         active_thread_id: None,
         // #456: plumb the App's HookExecutor so `exec_shell` can surface
-        // the configured `shell_env` hooks. Wrapped in Arc once and shared.
-        hook_executor: Some(std::sync::Arc::new(app.hooks.clone())),
+        // the configured `shell_env` hooks. Clone the shared Arc.
+        hook_executor: app.runtime_services.hook_executor.clone(),
         handle_store: app.runtime_services.handle_store.clone(),
         rlm_sessions: app.runtime_services.rlm_sessions.clone(),
     };
@@ -754,6 +761,7 @@ fn build_engine_config(app: &App, config: &Config) -> EngineConfig {
         ),
         max_spawn_depth: crate::tools::subagent::DEFAULT_MAX_SPAWN_DEPTH,
         allowed_tools: app.active_allowed_tools.clone(),
+        hook_executor: app.runtime_services.hook_executor.clone(),
         network_policy: config.network.clone().map(|toml_cfg| {
             crate::network_policy::NetworkPolicyDecider::with_default_audit(toml_cfg.into_runtime())
         }),
@@ -4706,6 +4714,7 @@ async fn dispatch_user_message(
             translation_enabled: app.translation_enabled,
             show_thinking: app.show_thinking,
             allowed_tools: app.active_allowed_tools.clone(),
+            hook_executor: app.runtime_services.hook_executor.clone(),
         })
         .await
     {

crates/tui/src/utils.rs

@@ -253,7 +253,7 @@ fn browser_open_command(url: &str) -> Result<Command> {
     {
         let mut cmd = Command::new("cmd");
         cmd.args(["/C", "start", "", url]);
-        return Ok(cmd);
+        Ok(cmd)
     }
 
     #[cfg(not(any(target_os = "macos", target_os = "linux", target_os = "windows")))]