diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index b667179855..9716db0362 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -173,7 +173,7 @@ pub enum HookHandlerConfig { McpTool { server: String, tool: String, - #[serde(default)] + #[serde(default, deserialize_with = "deserialize_mcp_tool_input")] input: serde_json::Map, #[serde(default, rename = "timeout")] timeout_sec: Option, @@ -186,6 +186,22 @@ pub enum HookHandlerConfig { Agent {}, } +// Reject values such as null that cannot be represented in TOML for trust hashing. +fn deserialize_mcp_tool_input<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let input = serde_json::Map::deserialize(deserializer)?; + toml::Value::try_from(&input).map_err(|error| { + serde::de::Error::custom(format!( + "MCP hook input must be representable as TOML: {error}" + )) + })?; + Ok(input) +} + #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ManagedHooksRequirementsToml { pub managed_dir: Option, diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs index fcf7b91901..bd71a7f0e3 100644 --- a/codex-rs/config/src/hooks_tests.rs +++ b/codex-rs/config/src/hooks_tests.rs @@ -68,7 +68,7 @@ fn hooks_file_deserializes_mcp_tool_handler_with_json_inputs() { "tool": "scan", "input": { "file_path": "${tool_input.file_path}", - "optional": null, + "include_ignored": false, }, "timeout": 30, "statusMessage": "Scanning file", @@ -88,7 +88,10 @@ fn hooks_file_deserializes_mcp_tool_handler_with_json_inputs() { "file_path".to_string(), serde_json::Value::String("${tool_input.file_path}".to_string()), ), - ("optional".to_string(), serde_json::Value::Null), + ( + "include_ignored".to_string(), + serde_json::Value::Bool(false) + ), ]), timeout_sec: Some(30), status_message: Some("Scanning file".to_string()), @@ -96,6 +99,36 @@ fn hooks_file_deserializes_mcp_tool_handler_with_json_inputs() { ); } +#[test] +fn hooks_file_rejects_mcp_tool_handler_with_null_input() { + for input in [ + serde_json::json!({ "optional": null }), + serde_json::json!({ "metadata": { "optional": null } }), + serde_json::json!({ "values": [null] }), + ] { + let error = serde_json::from_value::(serde_json::json!({ + "hooks": { + "PostToolUse": [{ + "hooks": [{ + "type": "mcp_tool", + "server": "security", + "tool": "scan", + "input": input, + }], + }], + }, + })) + .expect_err("literal null MCP hook arguments should be rejected"); + + assert!( + error + .to_string() + .contains("MCP hook input must be representable as TOML"), + "unexpected parse error: {error}" + ); + } +} + #[test] fn hooks_file_rejects_events_outside_hooks_object() { let error = serde_json::from_str::( diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index d9bef0151d..662de697e6 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::io::ErrorKind; use std::path::Path; use std::process::Stdio; @@ -20,6 +21,8 @@ use tracing::Span; use super::CommandShell; use super::ConfiguredHandler; +use super::ConfiguredHandlerKind; +use super::HandlerRunResult; use super::dispatcher::ParsedHandler; use super::dispatcher::hook_event_name_label; use super::dispatcher::hook_execution_mode_label; @@ -99,7 +102,7 @@ impl CommandHookRuntime { input_json: String, cwd: std::path::PathBuf, turn_id: Option, - parse: fn(&ConfiguredHandler, CommandRunResult, Option) -> ParsedHandler, + parse: fn(&ConfiguredHandler, HandlerRunResult, Option) -> ParsedHandler, ) { let mut state = self.lock_state(); if self.result_sender.is_closed() || state.concurrency_limit.is_closed() { @@ -114,7 +117,11 @@ impl CommandHookRuntime { let Ok(_permit) = concurrency_limit.acquire_owned().await else { return; }; - let result = run_command(&runtime, &handler, &input_json, &cwd).await; + let result = match &handler.kind { + ConfiguredHandlerKind::Command { command, env, .. } => { + run_command(&runtime, &handler, command, env, &input_json, &cwd).await + } + }; let mut hook_result = parse(&handler, result, turn_id).completed; let mut entries = Vec::new(); let mut warnings = Vec::new(); @@ -161,17 +168,6 @@ impl CommandHookRuntime { } } -#[derive(Debug)] -pub(crate) struct CommandRunResult { - pub started_at: i64, - pub completed_at: i64, - pub duration_ms: i64, - pub exit_code: Option, - pub stdout: String, - pub stderr: String, - pub error: Option, -} - #[tracing::instrument( name = "codex.hooks.command", level = "trace", @@ -179,7 +175,7 @@ pub(crate) struct CommandRunResult { fields( hook.event_name = hook_event_name_label(handler.event_name), hook.handler_type = hook_handler_type_label(HookHandlerType::Command), - hook.execution_mode = hook_execution_mode_label(handler.execution_mode), + hook.execution_mode = hook_execution_mode_label(handler.execution_mode()), hook.scope = hook_scope_label(scope_for_event(handler.event_name)), hook.source = hook_source_label(handler.source), hook.display_order = handler.display_order, @@ -190,13 +186,15 @@ pub(crate) struct CommandRunResult { pub(crate) async fn run_command( runtime: &CommandHookRuntime, handler: &ConfiguredHandler, + command: &str, + env: &HashMap, input_json: &str, cwd: &Path, -) -> CommandRunResult { +) -> HandlerRunResult { let started_at = chrono::Utc::now().timestamp(); let started = Instant::now(); - let mut command = build_command(&runtime.shell, handler); + let mut command = build_command(&runtime.shell, command, env); command .current_dir(cwd) .stdin(Stdio::piped()) @@ -357,9 +355,9 @@ fn finish_command_run( started_at: i64, started: Instant, completion: CommandRunCompletion, -) -> CommandRunResult { +) -> HandlerRunResult { Span::current().record("hook.command_outcome", completion.outcome); - CommandRunResult { + HandlerRunResult { started_at, completed_at: chrono::Utc::now().timestamp(), duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), @@ -370,7 +368,11 @@ fn finish_command_run( } } -fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { +fn build_command( + shell: &CommandShell, + command_line: &str, + env: &HashMap, +) -> Command { let mut command = if shell.program.is_empty() { default_shell_command() } else { @@ -378,24 +380,24 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command { }; if shell.program.is_empty() { #[cfg(windows)] - command.raw_arg(format!(r#""{}""#, handler.command)); + command.raw_arg(format!(r#""{command_line}""#)); #[cfg(not(windows))] - command.arg(&handler.command); + command.arg(command_line); } else { command.args(&shell.args); #[cfg(windows)] if shell.args.iter().any(|arg| arg.eq_ignore_ascii_case("/c")) { - command.raw_arg(format!(r#""{}""#, handler.command)); + command.raw_arg(format!(r#""{command_line}""#)); } else { - command.arg(&handler.command); + command.arg(command_line); } #[cfg(not(windows))] - command.arg(&handler.command); + command.arg(command_line); } - command.envs(&handler.env); + command.envs(env); scrub_non_inheritable_env_vars(command.as_std_mut()); command } diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index 177b0f0a60..bc7ab3a8ad 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -8,7 +8,6 @@ use async_channel::Receiver; use codex_protocol::ThreadId; use codex_protocol::protocol::HookCompletedEvent; use codex_protocol::protocol::HookEventName; -use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -20,6 +19,8 @@ use tempfile::tempdir; use tokio::time::sleep; use tokio::time::timeout; +use super::super::ClaudeHooksEngine; +use super::super::ConfiguredHandlerKind; use super::CommandHookRuntime; use super::CommandShell; use super::ConfiguredHandler; @@ -41,18 +42,22 @@ async fn cmd_shell_runs_quoted_hook_command_path() { .expect("write hook command"); let source_path = AbsolutePathBuf::try_from(hook_path.clone()).expect("absolute hook command path"); + let command = format!(r#""{}" notify"#, hook_path.display()); + let env = HashMap::new(); let handler = ConfiguredHandler { event_name: HookEventName::SessionStart, - execution_mode: HookExecutionMode::Sync, matcher: None, - command: format!(r#""{}" notify"#, hook_path.display()), timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), source_path, source: HookSource::User, display_order: 0, - env: HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: command.clone(), + r#async: false, + env: env.clone(), + }, }; let shells = [ CommandShell { @@ -68,7 +73,7 @@ async fn cmd_shell_runs_quoted_hook_command_path() { for shell in shells { let (result_sender, _result_receiver) = async_channel::unbounded(); let runtime = CommandHookRuntime::new(shell, ThreadId::new(), result_sender); - let result = run_command(&runtime, &handler, "{}", temp.path()).await; + let result = run_command(&runtime, &handler, &command, &env, "{}", temp.path()).await; assert_eq!(result.exit_code, Some(0), "stderr: {}", result.stderr); assert_eq!(result.stdout.trim(), "hook-ran"); @@ -81,23 +86,27 @@ async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() { let temp = tempdir().expect("create temp dir"); let source_path = AbsolutePathBuf::try_from(temp.path().join("hooks.json")) .expect("absolute hook configuration path"); + let command = "echo hook-ran"; + let env = HashMap::new(); let handler = ConfiguredHandler { event_name: HookEventName::SessionStart, - execution_mode: HookExecutionMode::Sync, matcher: None, - command: "echo hook-ran".to_string(), timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), source_path, source: HookSource::User, display_order: 0, - env: HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: command.to_string(), + r#async: false, + env: env.clone(), + }, }; let input_json = format!(r#"{{"padding":"{}"}}"#, "x".repeat(1024 * 1024)); let (runtime, _result_receiver) = runtime(); - let result = run_command(&runtime, &handler, &input_json, temp.path()).await; + let result = run_command(&runtime, &handler, command, &env, &input_json, temp.path()).await; assert_eq!(result.exit_code, Some(0), "stderr: {}", result.stderr); assert_eq!(result.stdout.trim(), "hook-ran"); @@ -125,9 +134,7 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler { std::fs::write(&script_path, source).expect("write async test hook"); ConfiguredHandler { event_name: HookEventName::UserPromptSubmit, - execution_mode: HookExecutionMode::Async, matcher: None, - command: format!("python3 {}", script_path.display()), timeout_sec: 10, status_message: None, additional_context_limit: Default::default(), @@ -135,15 +142,22 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler { .expect("absolute test hook path"), source: HookSource::User, display_order: 0, - env: HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: format!("python3 {}", script_path.display()), + r#async: true, + env: HashMap::new(), + }, } } async fn schedule(runtime: &CommandHookRuntime, handler: ConfiguredHandler, cwd: &Path) { - crate::events::user_prompt_submit::run( - &[handler], - runtime, - UserPromptSubmitRequest { + let engine = ClaudeHooksEngine { + handlers: vec![handler], + warnings: Vec::new(), + command_runtime: runtime.clone(), + }; + engine + .run_user_prompt_submit(UserPromptSubmitRequest { session_id: ThreadId::new(), turn_id: "async-test-turn".to_string(), subagent: None, @@ -152,9 +166,8 @@ async fn schedule(runtime: &CommandHookRuntime, handler: ConfiguredHandler, cwd: model: "test-model".to_string(), permission_mode: "default".to_string(), prompt: "test prompt".to_string(), - }, - ) - .await; + }) + .await; } #[tokio::test] diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 651f83ed9f..9bfaf952d3 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -22,6 +22,7 @@ use serde::Deserialize; use serde::Serialize; use super::ConfiguredHandler; +use super::ConfiguredHandlerKind; use super::HookListEntry; use crate::config_rules::hook_states_from_stack; use crate::events::common::matcher_pattern_for_event; @@ -52,6 +53,14 @@ struct HookHandlerSource<'a> { plugin_id: Option, } +struct NormalizedHandler { + config: HookHandlerConfig, + kind: ConfiguredHandlerKind, + timeout_sec: u64, + status_message: Option, + additional_context_limit: Option, +} + #[derive(Clone, Copy)] struct HookDiscoveryPolicy { allow_managed_hooks_only: bool, @@ -460,7 +469,7 @@ fn append_matcher_groups( continue; } for (handler_index, handler) in group.hooks.iter().cloned().enumerate() { - match handler { + let normalized = match handler { HookHandlerConfig::Command { command, command_windows, @@ -487,14 +496,9 @@ fn append_matcher_groups( source.path.as_path(), warnings, ); - let execution_mode = if r#async - && event_name != codex_protocol::protocol::HookEventName::SessionEnd - { - HookExecutionMode::Async - } else { - HookExecutionMode::Sync - }; - if r#async && execution_mode == HookExecutionMode::Sync { + let runs_async = r#async + && event_name != codex_protocol::protocol::HookEventName::SessionEnd; + if r#async && !runs_async { warnings.push(format!( "running async SessionEnd hook synchronously in {}", source.path.display() @@ -520,7 +524,7 @@ fn append_matcher_groups( }; let normalized_additional_context_limit = additional_context_limit .filter(|limit| *limit != DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT); - let normalized_handler = HookHandlerConfig::Command { + let config = HookHandlerConfig::Command { command: command.clone(), command_windows: None, timeout_sec: Some(timeout_sec), @@ -528,76 +532,106 @@ fn append_matcher_groups( status_message: status_message.clone(), additional_context_limit: normalized_additional_context_limit, }; - let current_hash = - command_hook_hash(event_name, matcher, &group, normalized_handler); let command = source.env.iter().fold(command, |command, (key, value)| { command.replace(&format!("${{{key}}}"), value) }); - // TODO(abhinav): replace this positional suffix with a durable hook id. - let key = - crate::hook_key(&source.key_source, event_name, group_index, handler_index); - let state = source.hook_states.get(&key); - let enabled = hook_enabled(source.is_managed, state); - let trusted_hash = hook_trusted_hash(source.is_managed, state); - let trust_status = - hook_trust_status(source.is_managed, ¤t_hash, trusted_hash); - hook_entries.push(HookListEntry { - key, - event_name, - handler_type: HookHandlerType::Command, - matcher: matcher.map(ToOwned::to_owned), - command: Some(command.clone()), - timeout_sec, - status_message: status_message.clone(), - additional_context_limit, - source_path: source.path.clone(), - source: source.source, - plugin_id: source.plugin_id.clone(), - display_order: *display_order, - enabled, - is_managed: source.is_managed, - current_hash, - trust_status, - execution_mode, - }); - if enabled - && (source.bypass_hook_trust - || matches!( - trust_status, - HookTrustStatus::Managed | HookTrustStatus::Trusted - )) - { - handlers.push(ConfiguredHandler { - event_name, - execution_mode, - matcher: matcher.map(ToOwned::to_owned), + NormalizedHandler { + config, + kind: ConfiguredHandlerKind::Command { command, - timeout_sec, - status_message, - additional_context_limit: AdditionalContextLimit::from_config( - additional_context_limit, - ), - source_path: source.path.clone(), - source: source.source, - display_order: *display_order, env: source.env.clone(), - }); + r#async: runs_async, + }, + timeout_sec, + status_message, + additional_context_limit, } - *display_order += 1; } - HookHandlerConfig::McpTool { .. } => warnings.push(format!( - "skipping MCP tool hook in {}: MCP tool hooks are not supported yet", - source.path.display() - )), - HookHandlerConfig::Prompt {} => warnings.push(format!( - "skipping prompt hook in {}: prompt hooks are not supported yet", - source.path.display() - )), - HookHandlerConfig::Agent {} => warnings.push(format!( - "skipping agent hook in {}: agent hooks are not supported yet", - source.path.display() - )), + HookHandlerConfig::McpTool { .. } => { + warnings.push(format!( + "skipping MCP tool hook in {}: MCP tool hooks are not supported yet", + source.path.display() + )); + continue; + } + HookHandlerConfig::Prompt {} => { + warnings.push(format!( + "skipping prompt hook in {}: prompt hooks are not supported yet", + source.path.display() + )); + continue; + } + HookHandlerConfig::Agent {} => { + warnings.push(format!( + "skipping agent hook in {}: agent hooks are not supported yet", + source.path.display() + )); + continue; + } + }; + + let NormalizedHandler { + config, + kind, + timeout_sec, + status_message, + additional_context_limit, + } = normalized; + let current_hash = hook_hash(event_name, matcher, &group, config); + let key = crate::hook_key(&source.key_source, event_name, group_index, handler_index); + let state = source.hook_states.get(&key); + let enabled = hook_enabled(source.is_managed, state); + let trusted_hash = hook_trusted_hash(source.is_managed, state); + let trust_status = hook_trust_status(source.is_managed, ¤t_hash, trusted_hash); + let ConfiguredHandlerKind::Command { command, .. } = &kind; + let execution_mode = + if matches!(kind, ConfiguredHandlerKind::Command { r#async: true, .. }) { + HookExecutionMode::Async + } else { + HookExecutionMode::Sync + }; + + hook_entries.push(HookListEntry { + key, + event_name, + handler_type: HookHandlerType::Command, + matcher: matcher.map(ToOwned::to_owned), + command: Some(command.clone()), + timeout_sec, + status_message: status_message.clone(), + additional_context_limit, + source_path: source.path.clone(), + source: source.source, + plugin_id: source.plugin_id.clone(), + display_order: *display_order, + enabled, + is_managed: source.is_managed, + current_hash, + trust_status, + execution_mode, + }); + if enabled + && (source.bypass_hook_trust + || matches!( + trust_status, + HookTrustStatus::Managed | HookTrustStatus::Trusted + )) + { + handlers.push(ConfiguredHandler { + event_name, + matcher: matcher.map(ToOwned::to_owned), + timeout_sec, + status_message, + additional_context_limit: AdditionalContextLimit::from_config( + additional_context_limit, + ), + source_path: source.path.clone(), + source: source.source, + display_order: *display_order, + kind, + }); } + *display_order += 1; } } } @@ -635,7 +669,7 @@ struct NormalizedHookIdentity { group: MatcherGroup, } -fn command_hook_hash( +fn hook_hash( event_name: codex_protocol::protocol::HookEventName, matcher: Option<&str>, group: &MatcherGroup, @@ -733,6 +767,7 @@ mod tests { use pretty_assertions::assert_eq; use super::ConfiguredHandler; + use super::ConfiguredHandlerKind; use super::HookListEntry; use super::append_matcher_groups; use crate::output_spill::AdditionalContextLimit; @@ -958,16 +993,18 @@ mod tests { handlers, vec![ConfiguredHandler { event_name: HookEventName::UserPromptSubmit, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: None, - command: "echo hello".to_string(), timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), source_path: source_path.clone(), source: hook_source(), display_order: 0, - env: std::collections::HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: "echo hello".to_string(), + r#async: false, + env: std::collections::HashMap::new(), + }, }] ); } @@ -995,16 +1032,18 @@ mod tests { handlers, vec![ConfiguredHandler { event_name: HookEventName::PreToolUse, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: Some("^Bash$".to_string()), - command: "echo hello".to_string(), timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), source_path: source_path.clone(), source: hook_source(), display_order: 0, - env: std::collections::HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: "echo hello".to_string(), + r#async: false, + env: std::collections::HashMap::new(), + }, }] ); } @@ -1269,11 +1308,16 @@ mod tests { assert_eq!(warnings, Vec::::new()); assert_eq!(handlers.len(), 1); assert_eq!( - handlers[0].command, - if cfg!(windows) { - "echo windows" - } else { - "echo unix" + handlers[0].kind, + ConfiguredHandlerKind::Command { + command: if cfg!(windows) { + "echo windows" + } else { + "echo unix" + } + .to_string(), + env: std::collections::HashMap::new(), + r#async: false, } ); } diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index 2caabf04e3..0858c409e4 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -11,9 +11,10 @@ use codex_protocol::protocol::HookRunStatus; use codex_protocol::protocol::HookRunSummary; use codex_protocol::protocol::HookScope; +use super::ClaudeHooksEngine; use super::ConfiguredHandler; -use super::command_runner::CommandHookRuntime; -use super::command_runner::CommandRunResult; +use super::ConfiguredHandlerKind; +use super::HandlerRunResult; use super::command_runner::run_command; use crate::events::common::matches_matcher; @@ -72,8 +73,8 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary { HookRunSummary { id: handler.run_id(), event_name: handler.event_name, - handler_type: HookHandlerType::Command, - execution_mode: handler.execution_mode, + handler_type: handler.handler_type(), + execution_mode: handler.execution_mode(), scope: scope_for_event(handler.event_name), source_path: handler.source_path.clone(), source: handler.source, @@ -88,17 +89,17 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary { } pub(crate) async fn execute_handlers( - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, handlers: Vec, input_json: String, cwd: &Path, turn_id: Option, - parse: fn(&ConfiguredHandler, CommandRunResult, Option) -> ParsedHandler, + parse: fn(&ConfiguredHandler, HandlerRunResult, Option) -> ParsedHandler, ) -> Vec> { let mut pending = FuturesUnordered::new(); for (configured_order, handler) in handlers.into_iter().enumerate() { - if handler.execution_mode == HookExecutionMode::Async { - runtime.schedule_async_hook( + if handler.execution_mode() == HookExecutionMode::Async { + engine.command_runtime.schedule_async_hook( handler, input_json.clone(), cwd.to_path_buf(), @@ -110,7 +111,19 @@ pub(crate) async fn execute_handlers( let input_json = input_json.clone(); let turn_id = turn_id.clone(); pending.push(async move { - let result = run_command(runtime, &handler, &input_json, cwd).await; + let result = match &handler.kind { + ConfiguredHandlerKind::Command { command, env, .. } => { + run_command( + &engine.command_runtime, + &handler, + command, + env, + &input_json, + cwd, + ) + .await + } + }; (configured_order, parse(&handler, result, turn_id)) }); } @@ -128,15 +141,15 @@ pub(crate) async fn execute_handlers( pub(crate) fn completed_summary( handler: &ConfiguredHandler, - run_result: &CommandRunResult, + run_result: &HandlerRunResult, status: HookRunStatus, entries: Vec, ) -> HookRunSummary { HookRunSummary { id: handler.run_id(), event_name: handler.event_name, - handler_type: HookHandlerType::Command, - execution_mode: handler.execution_mode, + handler_type: handler.handler_type(), + execution_mode: handler.execution_mode(), scope: scope_for_event(handler.event_name), source_path: handler.source_path.clone(), source: handler.source, @@ -231,6 +244,7 @@ mod tests { use pretty_assertions::assert_eq; use super::ConfiguredHandler; + use super::ConfiguredHandlerKind; use super::select_handlers; use super::select_handlers_for_matcher_inputs; @@ -242,16 +256,18 @@ mod tests { ) -> ConfiguredHandler { ConfiguredHandler { event_name, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: matcher.map(str::to_owned), - command: command.to_string(), timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: HookSource::User, display_order, - env: std::collections::HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: command.to_string(), + r#async: false, + env: std::collections::HashMap::new(), + }, } } @@ -511,9 +527,6 @@ mod tests { let selected = select_handlers(&handlers, HookEventName::Stop, /*matcher_input*/ None); - assert_eq!(selected.len(), 3); - assert_eq!(selected[0].command, "first"); - assert_eq!(selected[1].command, "second"); - assert_eq!(selected[2].command, "third"); + assert_eq!(selected, handlers); } } diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 49aa7e93fd..997803f599 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -46,22 +46,47 @@ pub(crate) struct CommandShell { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ConfiguredHandler { pub event_name: codex_protocol::protocol::HookEventName, - pub execution_mode: HookExecutionMode, pub matcher: Option, - pub command: String, pub timeout_sec: u64, pub status_message: Option, pub additional_context_limit: AdditionalContextLimit, pub source_path: AbsolutePathBuf, pub source: HookSource, pub display_order: i64, - pub env: HashMap, + pub kind: ConfiguredHandlerKind, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum ConfiguredHandlerKind { + Command { + command: String, + env: HashMap, + r#async: bool, + }, +} + +#[derive(Debug)] +pub(crate) struct HandlerRunResult { + pub started_at: i64, + pub completed_at: i64, + pub duration_ms: i64, + pub exit_code: Option, + pub stdout: String, + pub stderr: String, + pub error: Option, } impl ConfiguredHandler { + pub(crate) fn execution_mode(&self) -> HookExecutionMode { + match self.kind { + ConfiguredHandlerKind::Command { r#async: true, .. } => HookExecutionMode::Async, + ConfiguredHandlerKind::Command { r#async: false, .. } => HookExecutionMode::Sync, + } + } + /// Only synchronous hooks can apply control effects. pub(crate) fn can_apply_control_effects(&self) -> bool { - self.execution_mode == HookExecutionMode::Sync + self.execution_mode() == HookExecutionMode::Sync } pub fn run_id(&self) -> String { @@ -88,6 +113,12 @@ impl ConfiguredHandler { codex_protocol::protocol::HookEventName::Stop => "stop", } } + + fn handler_type(&self) -> HookHandlerType { + match &self.kind { + ConfiguredHandlerKind::Command { .. } => HookHandlerType::Command, + } + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -113,7 +144,7 @@ pub struct HookListEntry { #[derive(Clone)] pub(crate) struct ClaudeHooksEngine { - handlers: Vec, + pub(crate) handlers: Vec, warnings: Vec, pub(crate) command_runtime: CommandHookRuntime, } @@ -178,7 +209,7 @@ impl ClaudeHooksEngine { .iter() .filter(|handler| { handler.event_name == HookEventName::PermissionRequest - && handler.execution_mode == HookExecutionMode::Sync + && handler.can_apply_control_effects() }) .map(|handler| handler.timeout_sec) .max() @@ -198,27 +229,25 @@ impl ClaudeHooksEngine { request: SessionStartRequest, turn_id: Option, ) -> SessionStartOutcome { - crate::events::session_start::run(&self.handlers, &self.command_runtime, request, turn_id) - .await + crate::events::session_start::run(self, request, turn_id).await } pub(crate) async fn run_pre_tool_use(&self, request: PreToolUseRequest) -> PreToolUseOutcome { - crate::events::pre_tool_use::run(&self.handlers, &self.command_runtime, request).await + crate::events::pre_tool_use::run(self, request).await } pub(crate) async fn run_permission_request( &self, request: PermissionRequestRequest, ) -> PermissionRequestOutcome { - crate::events::permission_request::run(&self.handlers, &self.command_runtime, request).await + crate::events::permission_request::run(self, request).await } pub(crate) async fn run_post_tool_use( &self, request: PostToolUseRequest, ) -> PostToolUseOutcome { - let mut outcome = - crate::events::post_tool_use::run(&self.handlers, &self.command_runtime, request).await; + let mut outcome = crate::events::post_tool_use::run(self, request).await; if let Some(feedback_message) = outcome.feedback_message.take() { outcome.feedback_message = Some( self.command_runtime @@ -235,7 +264,7 @@ impl ClaudeHooksEngine { } pub(crate) async fn run_pre_compact(&self, request: PreCompactRequest) -> PreCompactOutcome { - crate::events::compact::run_pre(&self.handlers, &self.command_runtime, request).await + crate::events::compact::run_pre(self, request).await } pub(crate) fn preview_post_compact(&self, request: &PostCompactRequest) -> Vec { @@ -246,7 +275,7 @@ impl ClaudeHooksEngine { &self, request: PostCompactRequest, ) -> StatelessHookOutcome { - crate::events::compact::run_post(&self.handlers, &self.command_runtime, request).await + crate::events::compact::run_post(self, request).await } pub(crate) fn preview_user_prompt_submit( @@ -260,7 +289,7 @@ impl ClaudeHooksEngine { &self, request: UserPromptSubmitRequest, ) -> UserPromptSubmitOutcome { - crate::events::user_prompt_submit::run(&self.handlers, &self.command_runtime, request).await + crate::events::user_prompt_submit::run(self, request).await } pub(crate) fn preview_stop(&self, request: &StopRequest) -> Vec { @@ -272,12 +301,11 @@ impl ClaudeHooksEngine { } pub(crate) async fn run_session_end(&self, request: SessionEndRequest) -> SessionEndOutcome { - crate::events::session_end::run(&self.handlers, &self.command_runtime, request).await + crate::events::session_end::run(self, request).await } pub(crate) async fn run_stop(&self, request: StopRequest) -> StopOutcome { - let mut outcome = - crate::events::stop::run(&self.handlers, &self.command_runtime, request).await; + let mut outcome = crate::events::stop::run(self, request).await; outcome.continuation_fragments = self .command_runtime .output_spiller() diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 3fede5ac5e..d8dd2a8dda 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -22,7 +22,6 @@ use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; -use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -35,6 +34,7 @@ use super::ClaudeHooksEngine; use super::CommandHookRuntime; use super::CommandShell; use super::ConfiguredHandler; +use super::ConfiguredHandlerKind; use crate::events::pre_tool_use::PreToolUseRequest; fn cwd() -> AbsolutePathBuf { @@ -59,22 +59,31 @@ fn permission_request_timeout_only_counts_synchronous_handlers() { args: Vec::new(), }), ); + let command = "echo synchronous permission hook"; let synchronous_handler = ConfiguredHandler { event_name: HookEventName::PermissionRequest, - execution_mode: HookExecutionMode::Sync, matcher: None, - command: "echo synchronous permission hook".to_string(), timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), source_path: cwd().join("hooks.json"), source: HookSource::User, display_order: 0, - env: HashMap::new(), + kind: ConfiguredHandlerKind::Command { + command: command.to_string(), + r#async: false, + env: HashMap::new(), + }, + }; + let asynchronous_handler = ConfiguredHandler { + timeout_sec: 600, + kind: ConfiguredHandlerKind::Command { + command: command.to_string(), + r#async: true, + env: HashMap::new(), + }, + ..synchronous_handler.clone() }; - let mut asynchronous_handler = synchronous_handler.clone(); - asynchronous_handler.execution_mode = HookExecutionMode::Async; - asynchronous_handler.timeout_sec = 600; engine.handlers = vec![synchronous_handler, asynchronous_handler.clone()]; assert_eq!( @@ -772,7 +781,14 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { tool_input: serde_json::json!({ "command": "echo hello" }), }); assert_eq!(preview.len(), 1); - assert_eq!(engine.handlers[0].command, "echo hi"); + assert_eq!( + engine.handlers[0].kind, + ConfiguredHandlerKind::Command { + command: "echo hi".to_string(), + r#async: false, + env: HashMap::new(), + } + ); assert_eq!( engine.handlers[0].source_path, AbsolutePathBuf::try_from(missing_dir).expect("absolute missing dir") @@ -1052,14 +1068,16 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { engine .handlers .iter() - .map(|handler| handler.command.as_str()) + .map(|handler| match &handler.kind { + ConfiguredHandlerKind::Command { command, .. } => Some(command.as_str()), + }) .collect::>(), vec![ - "python3 /tmp/requirements-hook.py", - "python3 /tmp/mdm-hook.py", - "python3 /tmp/system-hook.py", - "python3 /tmp/legacy-file-hook.py", - "python3 /tmp/legacy-mdm-hook.py", + Some("python3 /tmp/requirements-hook.py"), + Some("python3 /tmp/mdm-hook.py"), + Some("python3 /tmp/system-hook.py"), + Some("python3 /tmp/legacy-file-hook.py"), + Some("python3 /tmp/legacy-mdm-hook.py"), ] ); let discovered = super::discovery::discover_handlers( @@ -1519,32 +1537,32 @@ fn plugin_hook_sources_expand_plugin_placeholders() { ); assert_eq!( - engine.handlers[0].command, - format!( - "run {} {} {} {}", - plugin_root.display(), - plugin_root.display(), - plugin_data_root.display(), - plugin_data_root.display() - ) - ); - assert_eq!( - engine.handlers[0].env, - HashMap::from([ - ("PLUGIN_ROOT".to_string(), plugin_root.display().to_string()), - ( - "CLAUDE_PLUGIN_ROOT".to_string(), - plugin_root.display().to_string() + engine.handlers[0].kind, + ConfiguredHandlerKind::Command { + command: format!( + "run {} {} {} {}", + plugin_root.display(), + plugin_root.display(), + plugin_data_root.display(), + plugin_data_root.display() ), - ( - "PLUGIN_DATA".to_string(), - plugin_data_root.display().to_string() - ), - ( - "CLAUDE_PLUGIN_DATA".to_string(), - plugin_data_root.display().to_string() - ), - ]) + r#async: false, + env: HashMap::from([ + ("PLUGIN_ROOT".to_string(), plugin_root.display().to_string()), + ( + "CLAUDE_PLUGIN_ROOT".to_string(), + plugin_root.display().to_string(), + ), + ( + "PLUGIN_DATA".to_string(), + plugin_data_root.display().to_string(), + ), + ( + "CLAUDE_PLUGIN_DATA".to_string(), + plugin_data_root.display().to_string(), + ), + ]), + } ); } diff --git a/codex-rs/hooks/src/events/compact.rs b/codex-rs/hooks/src/events/compact.rs index 86bc360735..2013d6c762 100644 --- a/codex-rs/hooks/src/events/compact.rs +++ b/codex-rs/hooks/src/events/compact.rs @@ -10,9 +10,9 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::schema::PostCompactCommandInput; @@ -70,12 +70,11 @@ pub(crate) fn preview_pre( } pub(crate) async fn run_pre( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: PreCompactRequest, ) -> PreCompactOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, HookEventName::PreCompact, Some(request.trigger.as_str()), ); @@ -103,7 +102,7 @@ pub(crate) async fn run_pre( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -152,12 +151,11 @@ pub(crate) fn preview_post( } pub(crate) async fn run_post( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: PostCompactRequest, ) -> StatelessHookOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, HookEventName::PostCompact, Some(request.trigger.as_str()), ); @@ -185,7 +183,7 @@ pub(crate) async fn run_post( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -227,7 +225,7 @@ struct CompactHandlerData { fn parse_pre_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { parse_completed( @@ -241,7 +239,7 @@ fn parse_pre_completed( fn parse_post_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { parse_completed( @@ -255,7 +253,7 @@ fn parse_post_completed( fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, event_label: &'static str, parse_output: fn(&str) -> Option, @@ -360,7 +358,7 @@ mod tests { use super::post_command_input_json; use super::pre_command_input_json; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; #[test] fn pre_compact_input_includes_lifecycle_metadata() { @@ -526,21 +524,23 @@ mod tests { fn handler(event_name: HookEventName) -> ConfiguredHandler { ConfiguredHandler { event_name, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: None, - command: "python3 compact_hook.py".to_string(), timeout_sec: 5, status_message: Some("running compact hook".to_string()), additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "python3 compact_hook.py".to_string(), + r#async: false, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1_700_000_000, completed_at: 1_700_000_001, duration_ms: 12, diff --git a/codex-rs/hooks/src/events/permission_request.rs b/codex-rs/hooks/src/events/permission_request.rs index 1c8e32dcbd..58da6c8e4d 100644 --- a/codex-rs/hooks/src/events/permission_request.rs +++ b/codex-rs/hooks/src/events/permission_request.rs @@ -16,9 +16,9 @@ use std::path::PathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::schema::PermissionRequestCommandInput; @@ -85,13 +85,12 @@ pub(crate) fn preview( } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: PermissionRequestRequest, ) -> PermissionRequestOutcome { let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases); let matched = dispatcher::select_handlers_for_matcher_inputs( - handlers, + &engine.handlers, HookEventName::PermissionRequest, &matcher_inputs, ); @@ -119,7 +118,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -188,7 +187,7 @@ fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestC fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 9db324c137..5f1dfa5850 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -11,9 +11,9 @@ use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; @@ -69,13 +69,12 @@ pub(crate) fn preview( } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: PostToolUseRequest, ) -> PostToolUseOutcome { let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases); let matched = dispatcher::select_handlers_for_matcher_inputs( - handlers, + &engine.handlers, HookEventName::PostToolUse, &matcher_inputs, ); @@ -102,7 +101,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -116,7 +115,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = runtime + let additional_contexts = engine + .command_runtime .output_spiller() .maybe_spill_additional_contexts(additional_contexts) .await; @@ -168,7 +168,7 @@ fn command_input_json(request: &PostToolUseRequest) -> Result, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); @@ -319,7 +319,6 @@ fn serialization_failure_outcome(hook_events: Vec) -> PostTo mod tests { use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; - use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -333,7 +332,7 @@ mod tests { use super::parse_completed; use super::preview; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; use crate::events::common; use crate::output_spill::AdditionalContext; use crate::output_spill::AdditionalContextLimit; @@ -433,8 +432,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(0), stdout, ""), @@ -586,23 +584,29 @@ mod tests { } fn handler() -> ConfiguredHandler { + handler_with_async(/*async*/ false) + } + + fn handler_with_async(r#async: bool) -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PostToolUse, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: Some("^Bash$".to_string()), - command: "python3 post_tool_use_hook.py".to_string(), timeout_sec: 5, status_message: Some("running post tool use hook".to_string()), additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "python3 post_tool_use_hook.py".to_string(), + r#async, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1_700_000_000, completed_at: 1_700_000_001, duration_ms: 12, diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index 64394c7a23..7233c014a9 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -11,9 +11,9 @@ use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; @@ -70,13 +70,12 @@ pub(crate) fn preview( } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: PreToolUseRequest, ) -> PreToolUseOutcome { let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases); let matched = dispatcher::select_handlers_for_matcher_inputs( - handlers, + &engine.handlers, HookEventName::PreToolUse, &matcher_inputs, ); @@ -104,7 +103,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -122,7 +121,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = runtime + let additional_contexts = engine + .command_runtime .output_spiller() .maybe_spill_additional_contexts(additional_contexts) .await; @@ -192,7 +192,7 @@ fn command_input_json(request: &PreToolUseRequest) -> Result, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); @@ -323,7 +323,6 @@ fn serialization_failure_outcome(hook_events: Vec) -> PreToo mod tests { use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; - use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -337,7 +336,7 @@ mod tests { use super::parse_completed; use super::preview; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; use crate::events::common; use crate::output_spill::AdditionalContext; use crate::output_spill::AdditionalContextLimit; @@ -466,8 +465,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(0), stdout, ""), @@ -770,23 +768,29 @@ mod tests { } fn handler() -> ConfiguredHandler { + handler_with_async(/*async*/ false) + } + + fn handler_with_async(r#async: bool) -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PreToolUse, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: Some("^Bash$".to_string()), - command: "echo hook".to_string(), timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "echo hook".to_string(), + r#async, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1, completed_at: 2, duration_ms: 1, diff --git a/codex-rs/hooks/src/events/session_end.rs b/codex-rs/hooks/src/events/session_end.rs index 310d340591..708d0e53d9 100644 --- a/codex-rs/hooks/src/events/session_end.rs +++ b/codex-rs/hooks/src/events/session_end.rs @@ -10,9 +10,9 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::schema::NullableString; use crate::schema::SessionEndCommandInput; @@ -48,12 +48,11 @@ pub(crate) fn preview(handlers: &[ConfiguredHandler]) -> Vec { } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: SessionEndRequest, ) -> SessionEndOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, HookEventName::SessionEnd, Some(SESSION_END_REASON), ); @@ -81,7 +80,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -96,7 +95,7 @@ pub(crate) async fn run( fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler<()> { let (status, entries) = match (run_result.error.as_deref(), run_result.exit_code) { diff --git a/codex-rs/hooks/src/events/session_end_tests.rs b/codex-rs/hooks/src/events/session_end_tests.rs index 1b9a44682f..d2cf0183b7 100644 --- a/codex-rs/hooks/src/events/session_end_tests.rs +++ b/codex-rs/hooks/src/events/session_end_tests.rs @@ -10,7 +10,7 @@ use pretty_assertions::assert_eq; use super::parse_completed; use super::preview; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; #[test] fn session_end_matches_other_reason() { @@ -42,7 +42,7 @@ fn session_end_matches_other_reason() { fn session_end_ignores_successful_output() { let completed = parse_completed( &handler(/*matcher*/ None), - CommandRunResult { + HandlerRunResult { started_at: 1, completed_at: 2, duration_ms: 1, @@ -61,15 +61,17 @@ fn session_end_ignores_successful_output() { fn handler(matcher: Option<&str>) -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::SessionEnd, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: matcher.map(str::to_string), - command: "echo hook".to_string(), timeout_sec: 2, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: HookSource::User, display_order: 0, - env: HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "echo hook".to_string(), + r#async: false, + env: HashMap::new(), + }, } } diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index 2a74fad014..784ab1d074 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -10,9 +10,9 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; @@ -107,13 +107,12 @@ pub(crate) fn preview( } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: SessionStartRequest, turn_id: Option, ) -> SessionStartOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, request.target.event_name(), Some(request.target.matcher_input()), ); @@ -182,7 +181,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -200,7 +199,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = runtime + let additional_contexts = engine + .command_runtime .output_spiller() .maybe_spill_additional_contexts(additional_contexts) .await; @@ -221,7 +221,7 @@ pub(crate) async fn run( /// `continue:false`; `SubagentStart` stays context-injection-only. fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); @@ -364,7 +364,7 @@ mod tests { use super::SessionStartHandlerData; use super::parse_completed; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; use crate::output_spill::AdditionalContext; use crate::output_spill::AdditionalContextLimit; @@ -541,21 +541,23 @@ mod tests { fn handler_for(event_name: HookEventName) -> ConfiguredHandler { ConfiguredHandler { event_name, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: None, - command: "echo hook".to_string(), timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "echo hook".to_string(), + r#async: false, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1, completed_at: 2, duration_ms: 1, diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 408e78fb6c..5316867065 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -11,9 +11,9 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::schema::NullableString; @@ -92,13 +92,9 @@ pub(crate) fn preview( .collect() } -pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, - request: StopRequest, -) -> StopOutcome { +pub(crate) async fn run(engine: &ClaudeHooksEngine, request: StopRequest) -> StopOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, request.target.event_name(), request.target.matcher_input(), ); @@ -178,7 +174,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -201,7 +197,7 @@ pub(crate) async fn run( fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); @@ -412,7 +408,6 @@ fn serialization_failure_outcome(hook_events: Vec) -> StopOu #[cfg(test)] mod tests { use codex_protocol::protocol::HookEventName; - use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -426,7 +421,7 @@ mod tests { use super::aggregate_results; use super::parse_completed; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; #[test] fn block_decision_with_reason_sets_continuation_prompt() { @@ -474,8 +469,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(0), r#"{"decision":"block"}"#, ""), @@ -592,8 +586,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(0), "not json", ""), @@ -642,23 +635,29 @@ mod tests { } fn handler() -> ConfiguredHandler { + handler_with_async(/*async*/ false) + } + + fn handler_with_async(r#async: bool) -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::Stop, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: None, - command: "echo hook".to_string(), timeout_sec: 600, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "echo hook".to_string(), + r#async, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1, completed_at: 2, duration_ms: 1, diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index f6b0a36fbf..ebc63bbfa1 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -10,9 +10,9 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; +use crate::engine::ClaudeHooksEngine; use crate::engine::ConfiguredHandler; -use crate::engine::command_runner::CommandHookRuntime; -use crate::engine::command_runner::CommandRunResult; +use crate::engine::HandlerRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; @@ -62,12 +62,11 @@ pub(crate) fn preview( } pub(crate) async fn run( - handlers: &[ConfiguredHandler], - runtime: &CommandHookRuntime, + engine: &ClaudeHooksEngine, request: UserPromptSubmitRequest, ) -> UserPromptSubmitOutcome { let matched = dispatcher::select_handlers( - handlers, + &engine.handlers, HookEventName::UserPromptSubmit, /*matcher_input*/ None, ); @@ -104,7 +103,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - runtime, + engine, matched, input_json, request.cwd.as_path(), @@ -122,7 +121,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = runtime + let additional_contexts = engine + .command_runtime .output_spiller() .maybe_spill_additional_contexts(additional_contexts) .await; @@ -137,7 +137,7 @@ pub(crate) async fn run( fn parse_completed( handler: &ConfiguredHandler, - run_result: CommandRunResult, + run_result: HandlerRunResult, turn_id: Option, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); @@ -286,7 +286,6 @@ fn serialization_failure_outcome(hook_events: Vec) -> UserPr #[cfg(test)] mod tests { use codex_protocol::protocol::HookEventName; - use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; @@ -297,7 +296,7 @@ mod tests { use super::UserPromptSubmitHandlerData; use super::parse_completed; use crate::engine::ConfiguredHandler; - use crate::engine::command_runner::CommandRunResult; + use crate::engine::HandlerRunResult; use crate::output_spill::AdditionalContext; #[test] @@ -405,8 +404,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(0), stdout, ""), @@ -447,8 +445,7 @@ mod tests { }] ); - let mut async_handler = handler(); - async_handler.execution_mode = HookExecutionMode::Async; + let async_handler = handler_with_async(/*async*/ true); let parsed = parse_completed( &async_handler, run_result(Some(2), "", "blocked by policy\n"), @@ -459,23 +456,29 @@ mod tests { } fn handler() -> ConfiguredHandler { + handler_with_async(/*async*/ false) + } + + fn handler_with_async(r#async: bool) -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::UserPromptSubmit, - execution_mode: codex_protocol::protocol::HookExecutionMode::Sync, matcher: None, - command: "echo hook".to_string(), timeout_sec: 5, status_message: None, additional_context_limit: Default::default(), source_path: test_path_buf("/tmp/hooks.json").abs(), source: codex_protocol::protocol::HookSource::User, display_order: 0, - env: std::collections::HashMap::new(), + kind: crate::engine::ConfiguredHandlerKind::Command { + command: "echo hook".to_string(), + r#async, + env: std::collections::HashMap::new(), + }, } } - fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { - CommandRunResult { + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> HandlerRunResult { + HandlerRunResult { started_at: 1, completed_at: 2, duration_ms: 1,