From 1568269e0f95417fbd448e2c16acac45df8f309c Mon Sep 17 00:00:00 2001 From: Pahud Hsieh Date: Fri, 9 Jan 2026 19:31:31 -0500 Subject: [PATCH] feat(hooks): Add HookResponse parsing and user confirmation support - Add HookDecision enum with Allow, Ask, and Block variants for hook responses - Add HookResponse struct to parse JSON responses from hook stdout - Implement from_stdout() method to parse decision and optional message from JSON - Update HookOutput type to include parsed HookResponse as third tuple element - Add comprehensive unit tests for HookResponse parsing with various scenarios - Add prompt_hook_confirmation() function to prompt users for hook confirmation - Update hook execution to parse and return HookResponse from successful hook runs - Cached hooks now return None for HookResponse since Ask decisions aren't cached - Enable hooks to communicate decisions (allow/ask/block) back to the CLI for tool execution control --- crates/chat-cli/src/cli/agent/hook.rs | 110 ++++++++++++++++++++++ crates/chat-cli/src/cli/chat/cli/hooks.rs | 48 ++++++++-- crates/chat-cli/src/cli/chat/mod.rs | 51 +++++++++- docs/hooks.md | 38 +++++++- 4 files changed, 236 insertions(+), 11 deletions(-) diff --git a/crates/chat-cli/src/cli/agent/hook.rs b/crates/chat-cli/src/cli/agent/hook.rs index 0ad4dba483..1dc502957d 100644 --- a/crates/chat-cli/src/cli/agent/hook.rs +++ b/crates/chat-cli/src/cli/agent/hook.rs @@ -100,3 +100,113 @@ impl Hook { DEFAULT_CACHE_TTL_SECONDS } } + +/// Decision returned by a PreToolUse hook +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum HookDecision { + /// Allow tool execution (default behavior) + #[default] + Allow, + /// Prompt user for confirmation before executing + Ask, + /// Block tool execution + Block, +} + +/// Response from a hook that can include a decision and message +#[derive(Debug, Clone, Default)] +pub struct HookResponse { + pub decision: HookDecision, + pub message: Option, +} + +impl HookResponse { + /// Try to parse a JSON response from hook stdout + /// Returns None if the output is not valid JSON or doesn't contain a decision field + pub fn from_stdout(stdout: &str) -> Option { + let trimmed = stdout.trim(); + if trimmed.is_empty() || !trimmed.starts_with('{') { + return None; + } + + let json: serde_json::Value = serde_json::from_str(trimmed).ok()?; + let decision_str = json.get("decision")?.as_str()?; + + let decision = match decision_str.to_lowercase().as_str() { + "allow" => HookDecision::Allow, + "ask" => HookDecision::Ask, + "block" => HookDecision::Block, + _ => return None, + }; + + let message = json.get("message").and_then(|m| m.as_str()).map(String::from); + + Some(Self { decision, message }) + } +} + + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_hook_response_from_stdout_ask() { + let stdout = r#"{"decision": "ask", "message": "⚠️ This command uses sudo. Allow?"}"#; + let response = HookResponse::from_stdout(stdout).unwrap(); + assert_eq!(response.decision, HookDecision::Ask); + assert_eq!(response.message, Some("⚠️ This command uses sudo. Allow?".to_string())); + } + + #[test] + fn test_hook_response_from_stdout_block() { + let stdout = r#"{"decision": "block", "message": "Blocked by policy"}"#; + let response = HookResponse::from_stdout(stdout).unwrap(); + assert_eq!(response.decision, HookDecision::Block); + assert_eq!(response.message, Some("Blocked by policy".to_string())); + } + + #[test] + fn test_hook_response_from_stdout_allow() { + let stdout = r#"{"decision": "allow"}"#; + let response = HookResponse::from_stdout(stdout).unwrap(); + assert_eq!(response.decision, HookDecision::Allow); + assert_eq!(response.message, None); + } + + #[test] + fn test_hook_response_from_stdout_case_insensitive() { + let stdout = r#"{"decision": "ASK", "message": "Confirm?"}"#; + let response = HookResponse::from_stdout(stdout).unwrap(); + assert_eq!(response.decision, HookDecision::Ask); + } + + #[test] + fn test_hook_response_from_stdout_empty() { + assert!(HookResponse::from_stdout("").is_none()); + } + + #[test] + fn test_hook_response_from_stdout_not_json() { + assert!(HookResponse::from_stdout("not json").is_none()); + } + + #[test] + fn test_hook_response_from_stdout_no_decision() { + let stdout = r#"{"message": "some message"}"#; + assert!(HookResponse::from_stdout(stdout).is_none()); + } + + #[test] + fn test_hook_response_from_stdout_invalid_decision() { + let stdout = r#"{"decision": "invalid"}"#; + assert!(HookResponse::from_stdout(stdout).is_none()); + } + + #[test] + fn test_hook_response_from_stdout_with_whitespace() { + let stdout = " \n{\"decision\": \"ask\", \"message\": \"test\"}\n "; + let response = HookResponse::from_stdout(stdout).unwrap(); + assert_eq!(response.decision, HookDecision::Ask); + } +} diff --git a/crates/chat-cli/src/cli/chat/cli/hooks.rs b/crates/chat-cli/src/cli/chat/cli/hooks.rs index fa83288c4c..48afa78b70 100644 --- a/crates/chat-cli/src/cli/chat/cli/hooks.rs +++ b/crates/chat-cli/src/cli/chat/cli/hooks.rs @@ -33,6 +33,7 @@ use spinners::{ use crate::cli::agent::hook::{ Hook, + HookResponse, HookTrigger, }; use crate::cli::agent::is_mcp_tool_ref; @@ -48,9 +49,10 @@ use crate::theme::StyledText; use crate::util::MCP_SERVER_TOOL_DELIMITER; use crate::util::pattern_matching::matches_any_pattern; -/// Hook execution result: (exit_code, output) +/// Hook execution result: (exit_code, output, hook_response) /// Output is stdout if exit_code is 0, stderr otherwise. -pub type HookOutput = (i32, String); +/// hook_response is parsed from stdout JSON if present. +pub type HookOutput = (i32, String, Option); /// Check if a hook matches a tool name based on its matcher pattern fn hook_matches_tool(hook: &Hook, tool_name: &str) -> bool { @@ -139,7 +141,8 @@ impl HookExecutor { if let Some(cache) = self.get_cache(&hook) { // Note: we only cache successful hook run. hence always using 0 as exit code for cached hook - cached.push((hook.clone(), (0, cache))); + // Cached hooks don't have HookResponse since we don't cache Ask decisions + cached.push((hook.clone(), (0, cache, None))); continue; } futures.push(self.run_hook(hook, cwd, prompt, tool_context.clone())); @@ -194,7 +197,7 @@ impl HookExecutor { } // Process results regardless of output enabled - if let Ok((exit_code, hook_output)) = &result { + if let Ok((exit_code, hook_output, _)) = &result { // Print warning if exit code is not 0 if *exit_code != 0 { queue!( @@ -243,7 +246,7 @@ impl HookExecutor { drop(futures); // Fill cache with executed results, skipping what was already from cache - for ((trigger, hook), (exit_code, output)) in &results { + for ((trigger, hook), (exit_code, output, _)) in &results { if *exit_code != 0 { continue; // Only cache successful hooks } @@ -351,7 +354,13 @@ impl HookExecutor { "" } ); - Ok((exit_code, formatted_output)) + // Parse JSON response from stdout if exit code is 0 + let hook_response = if exit_code == 0 { + HookResponse::from_stdout(&formatted_output) + } else { + None + }; + Ok((exit_code, formatted_output, hook_response)) }, Ok(Err(err)) => Err(eyre!("failed to execute command: {}", err)), Err(_) => Err(eyre!("command timed out after {} ms", timeout.as_millis())), @@ -384,6 +393,33 @@ fn sanitize_user_prompt(input: &str) -> String { truncated.replace(|c: char| c.is_control() && c != '\n' && c != '\r' && c != '\t', "") } +/// Prompt user for confirmation with a custom message from a hook +/// Returns true if user confirms (y/yes), false otherwise +pub fn prompt_hook_confirmation(message: &str) -> std::io::Result { + use std::io::{ + Write, + stdin, + stdout, + }; + + let mut stdout = stdout(); + queue!( + stdout, + style::Print("\n"), + StyledText::warning_fg(), + style::Print(message), + StyledText::reset(), + style::Print("\n[y/N]: "), + )?; + stdout.flush()?; + + let mut input = String::new(); + stdin().read_line(&mut input)?; + let input = input.trim().to_lowercase(); + + Ok(input == "y" || input == "yes") +} + #[deny(missing_docs)] #[derive(Debug, PartialEq, Args)] #[command( diff --git a/crates/chat-cli/src/cli/chat/mod.rs b/crates/chat-cli/src/cli/chat/mod.rs index 099c0c8761..d3e3d6ae19 100644 --- a/crates/chat-cli/src/cli/chat/mod.rs +++ b/crates/chat-cli/src/cli/chat/mod.rs @@ -71,7 +71,10 @@ use clap::{ ValueEnum, }; use cli::compact::CompactStrategy; -use cli::hooks::ToolContext; +use cli::hooks::{ + ToolContext, + prompt_hook_confirmation, +}; use cli::model::{ find_model, get_available_models, @@ -3397,12 +3400,15 @@ impl ChatSession { .await?; // Here is how we handle the preToolUse hook output: - // Exit code is 0: nothing. stdout is not shown to user. + // Exit code is 0: check for JSON decision response + // - decision: allow (or no JSON) -> allow tool execution + // - decision: ask -> prompt user for confirmation + // - decision: block -> block tool execution // Exit code is 2: block the tool use. return stderr to LLM. show warning to user // Other error: show warning to user. // Check for exit code 2 and add to tool_results - for (_, (exit_code, output)) in &hook_results { + for (_, (exit_code, output, hook_response)) in &hook_results { if *exit_code == 2 { tool_results.push(ToolUseResult { tool_use_id: tool.id.clone(), @@ -3412,6 +3418,45 @@ impl ChatSession { ))], status: ToolResultStatus::Error, }); + } else if *exit_code == 0 { + // Check for JSON decision response + if let Some(response) = hook_response { + use crate::cli::agent::hook::HookDecision; + match response.decision { + HookDecision::Block => { + let msg = response.message.as_deref().unwrap_or("Hook blocked execution"); + tool_results.push(ToolUseResult { + tool_use_id: tool.id.clone(), + content: vec![ToolUseResultBlock::Text(format!( + "PreToolHook blocked the tool execution: {}", + msg + ))], + status: ToolResultStatus::Error, + }); + }, + HookDecision::Ask => { + let msg = response.message.as_deref().unwrap_or("Hook requests confirmation"); + match prompt_hook_confirmation(msg) { + Ok(true) => { + // User approved, continue with tool execution + }, + Ok(false) | Err(_) => { + // User denied or error reading input + tool_results.push(ToolUseResult { + tool_use_id: tool.id.clone(), + content: vec![ToolUseResultBlock::Text( + "User denied tool execution after hook prompt".to_string(), + )], + status: ToolResultStatus::Error, + }); + }, + } + }, + HookDecision::Allow => { + // Explicitly allowed, continue + }, + } + } } } } diff --git a/docs/hooks.md b/docs/hooks.md index f342780c62..fa2f962ab8 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -80,7 +80,7 @@ Runs when user submits a prompt. Output is added to conversation context. ### PreToolUse -Runs before tool execution. Can validate and block tool usage. +Runs before tool execution. Can validate, block, or prompt for confirmation. **Hook Event** ```json @@ -100,10 +100,44 @@ Runs before tool execution. Can validate and block tool usage. ``` **Exit Code Behavior:** -- **0**: Allow tool execution. +- **0**: Allow tool execution (or check JSON response for decision). - **2**: Block tool execution, return STDERR to LLM. - **Other**: Show STDERR warning to user, allow tool execution. +#### JSON Decision Response + +PreToolUse hooks can output a JSON response to STDOUT to control tool execution: + +```json +{ + "decision": "ask", + "message": "⚠️ This command uses sudo. Allow execution?" +} +``` + +| Decision | Behavior | +|----------|----------| +| `allow` | Allow tool execution (same as empty output) | +| `ask` | Prompt user for confirmation (y/n) | +| `block` | Block tool execution, return message to LLM | + +**Example: Sudo confirmation hook** + +```python +#!/usr/bin/env python3 +import json, sys, re + +input_data = json.load(sys.stdin) +command = input_data.get("tool_input", {}).get("command", "") + +if re.search(r'(^|;|&&|\|\||\|)\s*sudo(\s|$)', command): + print(json.dumps({ + "decision": "ask", + "message": f"⚠️ This command uses sudo:\n\n {command}\n\nAllow execution?" + })) +sys.exit(0) +``` + ### PostToolUse Runs after tool execution with access to tool results.