mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
Generated
+1
@@ -3291,6 +3291,7 @@ dependencies = [
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-output-truncation",
|
||||
"codex-utils-pty",
|
||||
"futures",
|
||||
"pretty_assertions",
|
||||
"regex",
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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<u32>,
|
||||
#[cfg(windows)]
|
||||
job: Option<JobObject>,
|
||||
}
|
||||
|
||||
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<i32>,
|
||||
stdout: String,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<T>(
|
||||
shell: &CommandShell,
|
||||
runtime: &CommandHookRuntime,
|
||||
handlers: Vec<ConfiguredHandler>,
|
||||
input_json: String,
|
||||
cwd: &Path,
|
||||
@@ -100,7 +100,7 @@ pub(crate) async fn execute_handlers<T>(
|
||||
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))
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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<ConfiguredHandler>,
|
||||
warnings: Vec<String>,
|
||||
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<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
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<String>,
|
||||
) -> 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<HookRunSummary> {
|
||||
@@ -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<HookRunSummary> {
|
||||
@@ -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<String>) -> Option<String> {
|
||||
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<codex_protocol::items::HookPromptFragment>,
|
||||
) -> Vec<codex_protocol::items::HookPromptFragment> {
|
||||
self.output_spiller
|
||||
.maybe_spill_prompt_fragments(session_id, fragments)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -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()]);
|
||||
|
||||
@@ -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");
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<HookRunSummary> {
|
||||
|
||||
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(),
|
||||
|
||||
@@ -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<String>,
|
||||
) -> 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;
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user