From eb60d0fb8105dfc1c845c99477ec54ff3604a3e6 Mon Sep 17 00:00:00 2001
From: xj <gh-xj@users.noreply.github.com>
Date: Thu, 19 Feb 2026 02:40:02 -0800
Subject: [PATCH] fix(hooks): address code review findings

- C1: Use real tool success boolean instead of starts_with("Error")
  heuristic in after_tool_call hook
- C2: Wire HookRunner from config into ChannelRuntimeContext so hooks
  actually fire in daemon/channel mode (was hardcoded to None)
- I1: Suppress unused_imports warning on HookHandler public API re-export
- I3: Remove session_memory and boot_script config fields that had no
  backing implementation (YAGNI); keep only command_logger which is wired

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/agent/loop_.rs   | 16 +++++++++-------
 src/channels/mod.rs  | 10 +++++++++-
 src/config/schema.rs |  8 +-------
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/agent/loop_.rs b/src/agent/loop_.rs
index 62a78f473..ff4d1a649 100644
--- a/src/agent/loop_.rs
+++ b/src/agent/loop_.rs
@@ -1720,19 +1720,21 @@ pub(crate) async fn run_tool_call_loop(
                 tool: tool_name.clone(),
             });
             let start = Instant::now();
-            let result = if let Some(tool) = find_tool(tools_registry, &tool_name) {
+            let (result, tool_success) = if let Some(tool) = find_tool(tools_registry, &tool_name) {
                 match tool.execute(tool_args).await {
                     Ok(r) => {
+                        let success = r.success;
                         observer.record_event(&ObserverEvent::ToolCall {
                             tool: tool_name.clone(),
                             duration: start.elapsed(),
-                            success: r.success,
+                            success,
                         });
-                        if r.success {
+                        let output = if r.success {
                             scrub_credentials(&r.output)
                         } else {
                             format!("Error: {}", r.error.unwrap_or_else(|| r.output))
-                        }
+                        };
+                        (output, success)
                     }
                     Err(e) => {
                         observer.record_event(&ObserverEvent::ToolCall {
@@ -1740,17 +1742,17 @@ pub(crate) async fn run_tool_call_loop(
                             duration: start.elapsed(),
                             success: false,
                         });
-                        format!("Error executing {}: {e}", tool_name)
+                        (format!("Error executing {}: {e}", tool_name), false)
                     }
                 }
             } else {
-                format!("Unknown tool: {}", tool_name)
+                (format!("Unknown tool: {}", tool_name), false)
             };
 
             // ── Hook: after_tool_call (void) ─────────────────
             if let Some(hooks) = hooks {
                 let tool_result_obj = crate::tools::ToolResult {
-                    success: !result.starts_with("Error"),
+                    success: tool_success,
                     output: result.clone(),
                     error: None,
                 };
diff --git a/src/channels/mod.rs b/src/channels/mod.rs
index f3cd70386..0b8ac1f78 100644
--- a/src/channels/mod.rs
+++ b/src/channels/mod.rs
@@ -3069,7 +3069,15 @@ pub async fn start_channels(config: Config) -> Result<()> {
         message_timeout_secs,
         interrupt_on_new_message,
         multimodal: config.multimodal.clone(),
-        hooks: None,
+        hooks: if config.hooks.enabled {
+            let mut runner = crate::hooks::HookRunner::new();
+            if config.hooks.builtin.command_logger {
+                runner.register(Box::new(crate::hooks::builtin::CommandLoggerHook::new()));
+            }
+            Some(Arc::new(runner))
+        } else {
+            None
+        },
     });
 
     run_message_dispatch_loop(rx, runtime_ctx, max_in_flight_messages).await;
diff --git a/src/config/schema.rs b/src/config/schema.rs
index 3d5392780..da4bce13b 100644
--- a/src/config/schema.rs
+++ b/src/config/schema.rs
@@ -1783,20 +1783,14 @@ impl Default for HooksConfig {
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct BuiltinHooksConfig {
-    /// Persist selected session memory events via built-in hook handler.
-    pub session_memory: bool,
-    /// Log hook-related command activity for audit/troubleshooting.
+    /// Enable the command-logger hook (logs tool calls for auditing).
     pub command_logger: bool,
-    /// Run boot-time initialization script hook.
-    pub boot_script: bool,
 }
 
 impl Default for BuiltinHooksConfig {
     fn default() -> Self {
         Self {
-            session_memory: true,
             command_logger: false,
-            boot_script: true,
         }
     }
 }