From dd916428cd738001049e95d13854b80b78878065 Mon Sep 17 00:00:00 2001 From: Abhinav Date: Sat, 8 Aug 2026 03:43:04 +0000 Subject: [PATCH] Terminate timed-out hook process trees (#37527) ## Why Timing out a command hook can leave descendant processes running after the hook itself is stopped. ## What changed - Run hook commands in a process group on Unix and a job object on Windows. - Terminate the full process tree when hook execution is cancelled or fails, with `taskkill` as a Windows fallback. - Preserve descendants when a hook completes successfully so intentionally detached helpers can continue running. GitOrigin-RevId: 1a18653d34c6f1685ef8c4f8e8546663b276292a --- codex-rs/Cargo.lock | 1 + codex-rs/hooks/Cargo.toml | 3 + codex-rs/hooks/src/engine/command_runner.rs | 119 +++++++++++++++--- .../hooks/src/engine/command_runner_tests.rs | 5 +- codex-rs/hooks/src/engine/dispatcher.rs | 6 +- codex-rs/hooks/src/engine/mod.rs | 87 +++++-------- codex-rs/hooks/src/engine/mod_tests.rs | 69 +++++----- codex-rs/hooks/src/events/compact.rs | 16 +-- .../hooks/src/events/permission_request.rs | 6 +- codex-rs/hooks/src/events/post_tool_use.rs | 11 +- codex-rs/hooks/src/events/pre_tool_use.rs | 11 +- codex-rs/hooks/src/events/session_end.rs | 6 +- codex-rs/hooks/src/events/session_start.rs | 11 +- codex-rs/hooks/src/events/stop.rs | 6 +- .../hooks/src/events/user_prompt_submit.rs | 11 +- codex-rs/hooks/src/registry.rs | 10 +- 16 files changed, 223 insertions(+), 155 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 024b7255a2..4c0cc0e6eb 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3291,6 +3291,7 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-output-truncation", + "codex-utils-pty", "futures", "pretty_assertions", "regex", diff --git a/codex-rs/hooks/Cargo.toml b/codex-rs/hooks/Cargo.toml index 1bd7f455f2..5f5a180766 100644 --- a/codex-rs/hooks/Cargo.toml +++ b/codex-rs/hooks/Cargo.toml @@ -33,3 +33,6 @@ uuid = { workspace = true, features = ["v4"] } pretty_assertions = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread", "time"] } + +[target.'cfg(any(unix, windows))'.dependencies] +codex-utils-pty = { workspace = true } diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index 6afa84bb43..635defb831 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -4,6 +4,8 @@ use std::process::Stdio; use std::time::Duration; use std::time::Instant; +#[cfg(windows)] +use codex_utils_pty::JobObject; use tokio::io::AsyncWriteExt; use tokio::process::Command; use tokio::time::timeout; @@ -17,9 +19,30 @@ use super::dispatcher::hook_handler_type_label; use super::dispatcher::hook_scope_label; use super::dispatcher::hook_source_label; use super::dispatcher::scope_for_event; +use crate::output_spill::HookOutputSpiller; use codex_protocol::protocol::HookExecutionMode; use codex_protocol::protocol::HookHandlerType; +/// Owns the shell and output spiller shared by a session's command hooks. +#[derive(Clone)] +pub(crate) struct CommandHookRuntime { + shell: CommandShell, + output_spiller: HookOutputSpiller, +} + +impl CommandHookRuntime { + pub(crate) fn new(shell: CommandShell) -> Self { + Self { + shell, + output_spiller: HookOutputSpiller::new(), + } + } + + pub(crate) fn output_spiller(&self) -> &HookOutputSpiller { + &self.output_spiller + } +} + #[derive(Debug)] pub(crate) struct CommandRunResult { pub started_at: i64, @@ -48,7 +71,7 @@ pub(crate) struct CommandRunResult { ) )] pub(crate) async fn run_command( - shell: &CommandShell, + runtime: &CommandHookRuntime, handler: &ConfiguredHandler, configured_order: usize, input_json: &str, @@ -57,7 +80,7 @@ pub(crate) async fn run_command( let started_at = chrono::Utc::now().timestamp(); let started = Instant::now(); - let mut command = build_command(shell, handler); + let mut command = build_command(&runtime.shell, handler); command .current_dir(cwd) .stdin(Stdio::piped()) @@ -65,7 +88,27 @@ pub(crate) async fn run_command( .stderr(Stdio::piped()) .kill_on_drop(true); - let mut child = match command.spawn() { + #[cfg(unix)] + command.process_group(0); + + #[cfg(windows)] + let mut process_tree_job = JobObject::create().ok(); + #[cfg(windows)] + let child = match process_tree_job.as_ref() { + Some(job) => match job.spawn_contained(&mut command) { + Ok(child) => Ok(child), + Err(_) => { + process_tree_job = None; + command.creation_flags(0); + command.spawn() + } + }, + None => command.spawn(), + }; + #[cfg(not(windows))] + let child = command.spawn(); + + let mut child = match child { Ok(child) => child, Err(err) => { return finish_command_run( @@ -82,6 +125,12 @@ pub(crate) async fn run_command( } }; + let mut process_tree_guard = ProcessTreeGuard { + process_id: child.id(), + #[cfg(windows)] + job: process_tree_job, + }; + if let Some(mut stdin) = child.stdin.take() && let Err(err) = stdin.write_all(input_json.as_bytes()).await && err.kind() != ErrorKind::BrokenPipe @@ -102,17 +151,25 @@ pub(crate) async fn run_command( let timeout_duration = Duration::from_secs(handler.timeout_sec); match timeout(timeout_duration, child.wait_with_output()).await { - Ok(Ok(output)) => finish_command_run( - started_at, - started, - CommandRunCompletion { - exit_code: output.status.code(), - stdout: String::from_utf8_lossy(&output.stdout).to_string(), - stderr: String::from_utf8_lossy(&output.stderr).to_string(), - error: None, - outcome: "completed", - }, - ), + Ok(Ok(output)) => { + // Successfully completed hooks may intentionally leave detached helpers running. + #[cfg(windows)] + if let Some(job) = process_tree_guard.job.as_ref() { + let _ = job.preserve_descendants(); + } + process_tree_guard.process_id = None; + finish_command_run( + started_at, + started, + CommandRunCompletion { + exit_code: output.status.code(), + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + error: None, + outcome: "completed", + }, + ) + } Ok(Err(err)) => finish_command_run( started_at, started, @@ -138,6 +195,40 @@ pub(crate) async fn run_command( } } +// Needed only until command hooks move to the exec server, which owns process-tree cleanup. +struct ProcessTreeGuard { + process_id: Option, + #[cfg(windows)] + job: Option, +} + +impl Drop for ProcessTreeGuard { + fn drop(&mut self) { + let Some(process_id) = self.process_id else { + return; + }; + + #[cfg(unix)] + { + let _ = codex_utils_pty::process_group::kill_process_group(process_id); + } + + #[cfg(windows)] + { + if let Some(job) = self.job.as_ref() { + let _ = job.terminate(); + } else { + let _ = std::process::Command::new("taskkill") + .args(["/PID", &process_id.to_string(), "/T", "/F"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn(); + } + } + } +} + struct CommandRunCompletion { exit_code: Option, stdout: String, diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index 3ed720c62e..6a22fd84a3 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -8,6 +8,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::tempdir; +use super::CommandHookRuntime; use super::CommandShell; use super::ConfiguredHandler; use super::run_command; @@ -51,7 +52,7 @@ async fn cmd_shell_runs_quoted_hook_command_path() { for shell in shells { let result = run_command( - &shell, + &CommandHookRuntime::new(shell), &handler, /*configured_order*/ 0, "{}", @@ -89,7 +90,7 @@ async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() { let input_json = format!(r#"{{"padding":"{}"}}"#, "x".repeat(1024 * 1024)); let result = run_command( - &shell, + &CommandHookRuntime::new(shell), &handler, /*configured_order*/ 0, &input_json, diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index a5c7552067..4855bbf333 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -11,8 +11,8 @@ use codex_protocol::protocol::HookRunStatus; use codex_protocol::protocol::HookRunSummary; use codex_protocol::protocol::HookScope; -use super::CommandShell; use super::ConfiguredHandler; +use super::command_runner::CommandHookRuntime; use super::command_runner::CommandRunResult; use super::command_runner::run_command; use crate::events::common::matches_matcher; @@ -88,7 +88,7 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary { } pub(crate) async fn execute_handlers( - shell: &CommandShell, + runtime: &CommandHookRuntime, handlers: Vec, input_json: String, cwd: &Path, @@ -100,7 +100,7 @@ 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(shell, &handler, configured_order, &input_json, cwd).await; + let result = run_command(runtime, &handler, configured_order, &input_json, cwd).await; (configured_order, parse(&handler, result, turn_id)) }); } diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index d0c1aba1fe..cfa87a239c 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -23,10 +23,8 @@ use crate::events::stop::StopRequest; use crate::events::user_prompt_submit::UserPromptSubmitOutcome; use crate::events::user_prompt_submit::UserPromptSubmitRequest; use crate::output_spill::AdditionalContextLimit; -use crate::output_spill::HookOutputSpiller; use codex_config::ConfigLayerStack; use codex_plugin::PluginHookSource; -use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookHandlerType; use codex_protocol::protocol::HookRunSummary; @@ -36,6 +34,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; use std::time::Duration; +use command_runner::CommandHookRuntime; + #[derive(Debug, Clone)] pub(crate) struct CommandShell { pub program: String, @@ -107,8 +107,7 @@ pub struct HookListEntry { pub(crate) struct ClaudeHooksEngine { handlers: Vec, warnings: Vec, - shell: CommandShell, - output_spiller: HookOutputSpiller, + command_runtime: CommandHookRuntime, } impl ClaudeHooksEngine { @@ -118,14 +117,13 @@ impl ClaudeHooksEngine { config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: Vec, plugin_hook_load_warnings: Vec, - shell: CommandShell, + command_runtime: CommandHookRuntime, ) -> Self { if !enabled { return Self { handlers: Vec::new(), warnings: Vec::new(), - shell, - output_spiller: HookOutputSpiller::new(), + command_runtime, }; } @@ -136,11 +134,11 @@ impl ClaudeHooksEngine { plugin_hook_load_warnings, bypass_hook_trust, ); + Self { handlers: discovered.handlers, warnings: discovered.warnings, - shell, - output_spiller: HookOutputSpiller::new(), + command_runtime, } } @@ -189,26 +187,19 @@ impl ClaudeHooksEngine { request: SessionStartRequest, turn_id: Option, ) -> SessionStartOutcome { - crate::events::session_start::run( - &self.handlers, - &self.shell, - &self.output_spiller, - request, - turn_id, - ) - .await + crate::events::session_start::run(&self.handlers, &self.command_runtime, 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.shell, &self.output_spiller, request) - .await + crate::events::pre_tool_use::run(&self.handlers, &self.command_runtime, request).await } pub(crate) async fn run_permission_request( &self, request: PermissionRequestRequest, ) -> PermissionRequestOutcome { - crate::events::permission_request::run(&self.handlers, &self.shell, request).await + crate::events::permission_request::run(&self.handlers, &self.command_runtime, request).await } pub(crate) async fn run_post_tool_use( @@ -216,16 +207,16 @@ impl ClaudeHooksEngine { request: PostToolUseRequest, ) -> PostToolUseOutcome { let session_id = request.session_id; - let mut outcome = crate::events::post_tool_use::run( - &self.handlers, - &self.shell, - &self.output_spiller, - request, - ) - .await; - outcome.feedback_message = self - .maybe_spill_text(session_id, outcome.feedback_message) - .await; + let mut outcome = + crate::events::post_tool_use::run(&self.handlers, &self.command_runtime, request).await; + if let Some(feedback_message) = outcome.feedback_message.take() { + outcome.feedback_message = Some( + self.command_runtime + .output_spiller() + .maybe_spill_text(session_id, feedback_message) + .await, + ); + } outcome } @@ -234,7 +225,7 @@ impl ClaudeHooksEngine { } pub(crate) async fn run_pre_compact(&self, request: PreCompactRequest) -> PreCompactOutcome { - crate::events::compact::run_pre(&self.handlers, &self.shell, request).await + crate::events::compact::run_pre(&self.handlers, &self.command_runtime, request).await } pub(crate) fn preview_post_compact(&self, request: &PostCompactRequest) -> Vec { @@ -245,7 +236,7 @@ impl ClaudeHooksEngine { &self, request: PostCompactRequest, ) -> StatelessHookOutcome { - crate::events::compact::run_post(&self.handlers, &self.shell, request).await + crate::events::compact::run_post(&self.handlers, &self.command_runtime, request).await } pub(crate) fn preview_user_prompt_submit( @@ -259,13 +250,7 @@ impl ClaudeHooksEngine { &self, request: UserPromptSubmitRequest, ) -> UserPromptSubmitOutcome { - crate::events::user_prompt_submit::run( - &self.handlers, - &self.shell, - &self.output_spiller, - request, - ) - .await + crate::events::user_prompt_submit::run(&self.handlers, &self.command_runtime, request).await } pub(crate) fn preview_stop(&self, request: &StopRequest) -> Vec { @@ -277,34 +262,20 @@ impl ClaudeHooksEngine { } pub(crate) async fn run_session_end(&self, request: SessionEndRequest) -> SessionEndOutcome { - crate::events::session_end::run(&self.handlers, &self.shell, request).await + crate::events::session_end::run(&self.handlers, &self.command_runtime, request).await } pub(crate) async fn run_stop(&self, request: StopRequest) -> StopOutcome { let session_id = request.session_id; - let mut outcome = crate::events::stop::run(&self.handlers, &self.shell, request).await; + let mut outcome = + crate::events::stop::run(&self.handlers, &self.command_runtime, request).await; outcome.continuation_fragments = self + .command_runtime + .output_spiller() .maybe_spill_prompt_fragments(session_id, outcome.continuation_fragments) .await; outcome } - - async fn maybe_spill_text(&self, session_id: ThreadId, text: Option) -> Option { - match text { - Some(text) => Some(self.output_spiller.maybe_spill_text(session_id, text).await), - None => None, - } - } - - async fn maybe_spill_prompt_fragments( - &self, - session_id: ThreadId, - fragments: Vec, - ) -> Vec { - self.output_spiller - .maybe_spill_prompt_fragments(session_id, fragments) - .await - } } #[cfg(test)] diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index e323fffe53..11612a984a 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -29,6 +29,7 @@ use pretty_assertions::assert_eq; use tempfile::tempdir; use super::ClaudeHooksEngine; +use super::CommandHookRuntime; use super::CommandShell; use crate::events::pre_tool_use::PreToolUseRequest; @@ -202,10 +203,10 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -309,10 +310,10 @@ async fn requirements_managed_hooks_execute_windows_command_override() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); let outcome = engine @@ -389,10 +390,10 @@ fn unknown_requirement_source_hooks_stay_managed() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert_eq!(engine.handlers.len(), 1); @@ -472,10 +473,10 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert_eq!(engine.handlers.len(), 1); @@ -538,10 +539,10 @@ fn user_disablement_does_not_filter_managed_layer_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert_eq!(engine.handlers.len(), 1); @@ -700,10 +701,10 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -756,10 +757,10 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -810,10 +811,10 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -880,10 +881,10 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.handlers.is_empty()); @@ -919,10 +920,10 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { Some(&config_layer_stack), plugin_hook_sources, Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.handlers.is_empty()); @@ -991,10 +992,10 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -1101,10 +1102,10 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().iter().any(|warning| { @@ -1196,10 +1197,10 @@ fn profile_user_layers_load_shared_hooks_json_once() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.warnings().is_empty()); @@ -1270,10 +1271,10 @@ fn malformed_hooks_json_is_reported_as_startup_warning() { Some(&config_layer_stack), Vec::new(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert!(engine.handlers.is_empty()); @@ -1342,10 +1343,10 @@ print(json.dumps({ Some(&config_layer_stack), plugin_hook_sources.clone(), Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); let preview = engine.preview_pre_tool_use(&PreToolUseRequest { @@ -1462,10 +1463,10 @@ fn plugin_hook_sources_expand_plugin_placeholders() { Some(&config_layer_stack), plugin_hook_sources, Vec::new(), - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert_eq!( @@ -1506,10 +1507,10 @@ fn plugin_hook_load_warnings_are_startup_warnings() { /*config_layer_stack*/ None, Vec::new(), vec!["failed plugin hook".to_string()], - CommandShell { + CommandHookRuntime::new(CommandShell { program: String::new(), args: Vec::new(), - }, + }), ); assert_eq!(engine.warnings(), &["failed plugin hook".to_string()]); diff --git a/codex-rs/hooks/src/events/compact.rs b/codex-rs/hooks/src/events/compact.rs index 3542172056..b6765f9835 100644 --- a/codex-rs/hooks/src/events/compact.rs +++ b/codex-rs/hooks/src/events/compact.rs @@ -10,8 +10,8 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; @@ -71,7 +71,7 @@ pub(crate) fn preview_pre( pub(crate) async fn run_pre( handlers: &[ConfiguredHandler], - shell: &CommandShell, + runtime: &CommandHookRuntime, request: PreCompactRequest, ) -> PreCompactOutcome { let matched = dispatcher::select_handlers( @@ -103,7 +103,7 @@ pub(crate) async fn run_pre( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -153,7 +153,7 @@ pub(crate) fn preview_post( pub(crate) async fn run_post( handlers: &[ConfiguredHandler], - shell: &CommandShell, + runtime: &CommandHookRuntime, request: PostCompactRequest, ) -> StatelessHookOutcome { let matched = dispatcher::select_handlers( @@ -185,7 +185,7 @@ pub(crate) async fn run_post( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -436,7 +436,8 @@ mod tests { #[test] fn pre_compact_input_includes_lifecycle_metadata() { - let input_json = pre_command_input_json(&pre_request()).expect("serialize command input"); + let request = pre_request(); + let input_json = pre_command_input_json(&request).expect("serialize command input"); let input: serde_json::Value = serde_json::from_str(&input_json).expect("parse command input"); @@ -456,7 +457,8 @@ mod tests { #[test] fn post_compact_input_includes_lifecycle_metadata() { - let input_json = post_command_input_json(&post_request()).expect("serialize command input"); + let request = post_request(); + let input_json = post_command_input_json(&request).expect("serialize command input"); let input: serde_json::Value = serde_json::from_str(&input_json).expect("parse command input"); diff --git a/codex-rs/hooks/src/events/permission_request.rs b/codex-rs/hooks/src/events/permission_request.rs index db7970f02d..91b50a2f2e 100644 --- a/codex-rs/hooks/src/events/permission_request.rs +++ b/codex-rs/hooks/src/events/permission_request.rs @@ -16,8 +16,8 @@ use std::path::PathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; @@ -86,7 +86,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, + runtime: &CommandHookRuntime, request: PermissionRequestRequest, ) -> PermissionRequestOutcome { let matcher_inputs = common::matcher_inputs(&request.tool_name, &request.matcher_aliases); @@ -119,7 +119,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index ce0f449008..2cac61f9ee 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -11,13 +11,12 @@ use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; -use crate::output_spill::HookOutputSpiller; use crate::schema::PostToolUseCommandInput; use crate::schema::SubagentCommandInputFields; @@ -71,8 +70,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, - output_spiller: &HookOutputSpiller, + runtime: &CommandHookRuntime, request: PostToolUseRequest, ) -> PostToolUseOutcome { let session_id = request.session_id; @@ -105,7 +103,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -119,7 +117,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = output_spiller + let additional_contexts = runtime + .output_spiller() .maybe_spill_additional_contexts(session_id, additional_contexts) .await; let should_block = results.iter().any(|result| result.data.should_block); diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index b630798b66..273ff9b44a 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -11,13 +11,12 @@ use codex_utils_absolute_path::AbsolutePathBuf; use serde_json::Value; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; -use crate::output_spill::HookOutputSpiller; use crate::schema::PreToolUseCommandInput; use crate::schema::SubagentCommandInputFields; @@ -72,8 +71,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, - output_spiller: &HookOutputSpiller, + runtime: &CommandHookRuntime, request: PreToolUseRequest, ) -> PreToolUseOutcome { let session_id = request.session_id; @@ -107,7 +105,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -125,7 +123,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = output_spiller + let additional_contexts = runtime + .output_spiller() .maybe_spill_additional_contexts(session_id, additional_contexts) .await; let updated_input = if should_block { diff --git a/codex-rs/hooks/src/events/session_end.rs b/codex-rs/hooks/src/events/session_end.rs index 32a46b607d..310d340591 100644 --- a/codex-rs/hooks/src/events/session_end.rs +++ b/codex-rs/hooks/src/events/session_end.rs @@ -10,8 +10,8 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::schema::NullableString; @@ -49,7 +49,7 @@ pub(crate) fn preview(handlers: &[ConfiguredHandler]) -> Vec { pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, + runtime: &CommandHookRuntime, request: SessionEndRequest, ) -> SessionEndOutcome { let matched = dispatcher::select_handlers( @@ -81,7 +81,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index a26df2426a..2386354edc 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -10,13 +10,12 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; -use crate::output_spill::HookOutputSpiller; use crate::schema::NullableString; use crate::schema::SessionStartCommandInput; use crate::schema::SubagentStartCommandInput; @@ -109,8 +108,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, - output_spiller: &HookOutputSpiller, + runtime: &CommandHookRuntime, request: SessionStartRequest, turn_id: Option, ) -> SessionStartOutcome { @@ -185,7 +183,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -203,7 +201,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = output_spiller + let additional_contexts = runtime + .output_spiller() .maybe_spill_additional_contexts(session_id, additional_contexts) .await; diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index a17e305b3c..64e6e04473 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -11,8 +11,8 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; @@ -94,7 +94,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, + runtime: &CommandHookRuntime, request: StopRequest, ) -> StopOutcome { let matched = dispatcher::select_handlers( @@ -178,7 +178,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index e6ef8f6afd..5cec1d63e3 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -10,13 +10,12 @@ use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::AbsolutePathBuf; use super::common; -use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandHookRuntime; use crate::engine::command_runner::CommandRunResult; use crate::engine::dispatcher; use crate::engine::output_parser; use crate::output_spill::AdditionalContext; -use crate::output_spill::HookOutputSpiller; use crate::schema::NullableString; use crate::schema::SubagentCommandInputFields; use crate::schema::UserPromptSubmitCommandInput; @@ -64,8 +63,7 @@ pub(crate) fn preview( pub(crate) async fn run( handlers: &[ConfiguredHandler], - shell: &CommandShell, - output_spiller: &HookOutputSpiller, + runtime: &CommandHookRuntime, request: UserPromptSubmitRequest, ) -> UserPromptSubmitOutcome { let session_id = request.session_id; @@ -107,7 +105,7 @@ pub(crate) async fn run( }; let results = dispatcher::execute_handlers( - shell, + runtime, matched, input_json, request.cwd.as_path(), @@ -125,7 +123,8 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); - let additional_contexts = output_spiller + let additional_contexts = runtime + .output_spiller() .maybe_spill_additional_contexts(session_id, additional_contexts) .await; diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index f729e7a0f2..2506782d24 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -6,6 +6,7 @@ use tokio::process::Command; use crate::engine::ClaudeHooksEngine; use crate::engine::CommandShell; use crate::engine::HookListEntry; +use crate::engine::command_runner::CommandHookRuntime; use crate::events::compact::PostCompactRequest; use crate::events::compact::PreCompactOutcome; use crate::events::compact::PreCompactRequest; @@ -67,16 +68,17 @@ impl Hooks { .map(crate::notify_hook) .into_iter() .collect(); + let command_runtime = CommandHookRuntime::new(CommandShell { + program: config.shell_program.unwrap_or_default(), + args: config.shell_args, + }); let engine = ClaudeHooksEngine::new( config.feature_enabled, config.bypass_hook_trust, config.config_layer_stack.as_ref(), config.plugin_hook_sources, config.plugin_hook_load_warnings, - CommandShell { - program: config.shell_program.unwrap_or_default(), - args: config.shell_args, - }, + command_runtime, ); Self { after_agent,