From cc923d634c42070f0789ee534e35817a40fd6dce Mon Sep 17 00:00:00 2001
From: Paulo Aboim Pinto <aboimpinto@gmail.com>
Date: Mon, 1 Jun 2026 21:53:23 +0200
Subject: [PATCH] =?UTF-8?q?fix:=20address=20greptile=20review=20comments?=
 =?UTF-8?q?=20=E2=80=94=20remove=20double-firing,=20wrap=20blocking=20exec?=
 =?UTF-8?q?ute=20in=20spawn=5Fblocking?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Remove ToolCallBefore observer firing from tool_routing.rs (the turn-loop gate now handles it) to prevent double-firing hooks for each tool call (greptile P1).

- Wrap hook_executor.execute() call in tokio::task::spawn_blocking so the Tokio worker thread is not blocked by child.wait_timeout() during hook execution (greptile P1).
---
 crates/tui/src/core/engine/turn_loop.rs | 14 ++++++++++++--
 crates/tui/src/tui/tool_routing.rs      | 18 +++++-------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs
index 8c576a7b..7b6a8faa 100644
--- a/crates/tui/src/core/engine/turn_loop.rs
+++ b/crates/tui/src/core/engine/turn_loop.rs
@@ -1309,8 +1309,18 @@ impl Engine {
                         .with_workspace(self.session.workspace.clone())
                         .with_model(&self.config.model)
                         .with_session_id(&self.session.id);
-                    let hook_results = hook_executor
-                        .execute(crate::hooks::HookEvent::ToolCallBefore, &hook_context);
+                    // 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))
diff --git a/crates/tui/src/tui/tool_routing.rs b/crates/tui/src/tui/tool_routing.rs
index e0e35bdd..f76e1cd1 100644
--- a/crates/tui/src/tui/tool_routing.rs
+++ b/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();