diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index a34ccc6cdb..7f6916ad0a 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -99,6 +99,10 @@ impl ConfigLayerEntry { self.disabled_reason.is_some() } + pub fn is_project_layer(&self) -> bool { + matches!(self.name, ConfigLayerSource::Project { .. }) + } + pub fn raw_toml(&self) -> Option<&str> { self.raw_toml.as_deref() } diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 465caa376d..9434287be9 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -94,6 +94,7 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - &mut warnings, &mut display_order, source_path.as_path(), + layer.is_project_layer(), event_name, groups, ); @@ -107,8 +108,7 @@ fn append_group_handlers( handlers: &mut Vec, warnings: &mut Vec, display_order: &mut i64, - source_path: &Path, - event_name: codex_protocol::protocol::HookEventName, + context: AppendGroupContext<'_>, matcher: Option<&str>, group_handlers: Vec, ) { @@ -117,7 +117,7 @@ fn append_group_handlers( { warnings.push(format!( "invalid matcher {matcher:?} in {}: {err}", - source_path.display() + context.source_path.display() )); return; } @@ -133,46 +133,55 @@ fn append_group_handlers( if r#async { warnings.push(format!( "skipping async hook in {}: async hooks are not supported yet", - source_path.display() + context.source_path.display() )); continue; } if command.trim().is_empty() { warnings.push(format!( "skipping empty hook command in {}", - source_path.display() + context.source_path.display() )); continue; } let timeout_sec = timeout_sec.unwrap_or(600).max(1); handlers.push(ConfiguredHandler { - event_name, + event_name: context.event_name, matcher: matcher.map(ToOwned::to_owned), command, timeout_sec, status_message, - source_path: source_path.to_path_buf(), + source_path: context.source_path.to_path_buf(), + is_project: context.is_project, display_order: *display_order, }); *display_order += 1; } HookHandlerConfig::Prompt {} => warnings.push(format!( "skipping prompt hook in {}: prompt hooks are not supported yet", - source_path.display() + context.source_path.display() )), HookHandlerConfig::Agent {} => warnings.push(format!( "skipping agent hook in {}: agent hooks are not supported yet", - source_path.display() + context.source_path.display() )), } } } +#[derive(Clone, Copy)] +struct AppendGroupContext<'a> { + source_path: &'a Path, + is_project: bool, + event_name: codex_protocol::protocol::HookEventName, +} + fn append_matcher_groups( handlers: &mut Vec, warnings: &mut Vec, display_order: &mut i64, source_path: &Path, + is_project: bool, event_name: codex_protocol::protocol::HookEventName, groups: Vec, ) { @@ -181,8 +190,11 @@ fn append_matcher_groups( handlers, warnings, display_order, - source_path, - event_name, + AppendGroupContext { + source_path, + is_project, + event_name, + }, matcher_pattern_for_event(event_name, group.matcher.as_deref()), group.hooks, ); @@ -197,6 +209,7 @@ mod tests { use codex_protocol::protocol::HookEventName; use pretty_assertions::assert_eq; + use super::AppendGroupContext; use super::ConfiguredHandler; use super::HookHandlerConfig; use super::append_group_handlers; @@ -212,8 +225,11 @@ mod tests { &mut handlers, &mut warnings, &mut display_order, - Path::new("/tmp/hooks.json"), - HookEventName::UserPromptSubmit, + AppendGroupContext { + source_path: Path::new("/tmp/hooks.json"), + is_project: false, + event_name: HookEventName::UserPromptSubmit, + }, matcher_pattern_for_event(HookEventName::UserPromptSubmit, Some("[")), vec![HookHandlerConfig::Command { command: "echo hello".to_string(), @@ -233,6 +249,7 @@ mod tests { timeout_sec: 600, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, }] ); @@ -248,8 +265,11 @@ mod tests { &mut handlers, &mut warnings, &mut display_order, - Path::new("/tmp/hooks.json"), - HookEventName::PreToolUse, + AppendGroupContext { + source_path: Path::new("/tmp/hooks.json"), + is_project: false, + event_name: HookEventName::PreToolUse, + }, matcher_pattern_for_event(HookEventName::PreToolUse, Some("^Bash$")), vec![HookHandlerConfig::Command { command: "echo hello".to_string(), @@ -269,6 +289,7 @@ mod tests { timeout_sec: 600, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, }] ); @@ -284,8 +305,11 @@ mod tests { &mut handlers, &mut warnings, &mut display_order, - Path::new("/tmp/hooks.json"), - HookEventName::PreToolUse, + AppendGroupContext { + source_path: Path::new("/tmp/hooks.json"), + is_project: false, + event_name: HookEventName::PreToolUse, + }, matcher_pattern_for_event(HookEventName::PreToolUse, Some("*")), vec![HookHandlerConfig::Command { command: "echo hello".to_string(), @@ -310,8 +334,11 @@ mod tests { &mut handlers, &mut warnings, &mut display_order, - Path::new("/tmp/hooks.json"), - HookEventName::PostToolUse, + AppendGroupContext { + source_path: Path::new("/tmp/hooks.json"), + is_project: false, + event_name: HookEventName::PostToolUse, + }, matcher_pattern_for_event(HookEventName::PostToolUse, Some("Edit|Write")), vec![HookHandlerConfig::Command { command: "echo hello".to_string(), diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index 642231ff19..b5bc3c9462 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -82,6 +82,47 @@ pub(crate) async fn execute_handlers( .collect() } +pub(crate) fn select_handlers_by_trust_precedence( + handlers: &[ConfiguredHandler], + event_name: HookEventName, + matcher_input: Option<&str>, +) -> Vec> { + let mut non_project = Vec::new(); + let mut project = Vec::new(); + for handler in select_handlers(handlers, event_name, matcher_input) { + if handler.is_project { + project.push(handler); + } else { + non_project.push(handler); + } + } + + let mut tiers = Vec::new(); + if !non_project.is_empty() { + tiers.push(non_project); + } + if !project.is_empty() { + tiers.push(project); + } + tiers +} + +pub(crate) fn skipped_completed_event( + handler: &ConfiguredHandler, + turn_id: Option, + message: String, +) -> HookCompletedEvent { + let mut run = running_summary(handler); + run.status = HookRunStatus::Failed; + run.completed_at = Some(run.started_at); + run.duration_ms = Some(0); + run.entries = vec![codex_protocol::protocol::HookOutputEntry { + kind: codex_protocol::protocol::HookOutputEntryKind::Error, + text: message, + }]; + HookCompletedEvent { turn_id, run } +} + pub(crate) fn completed_summary( handler: &ConfiguredHandler, run_result: &CommandRunResult, @@ -123,6 +164,7 @@ mod tests { use super::ConfiguredHandler; use super::select_handlers; + use super::select_handlers_by_trust_precedence; fn make_handler( event_name: HookEventName, @@ -137,6 +179,26 @@ mod tests { timeout_sec: 5, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, + display_order, + } + } + + fn make_handler_with_source( + event_name: HookEventName, + matcher: Option<&str>, + command: &str, + source_path: &str, + display_order: i64, + ) -> ConfiguredHandler { + ConfiguredHandler { + event_name, + matcher: matcher.map(str::to_owned), + command: command.to_string(), + timeout_sec: 5, + status_message: None, + source_path: PathBuf::from(source_path), + is_project: source_path.contains("/project/"), display_order, } } @@ -334,4 +396,53 @@ mod tests { assert_eq!(selected[1].command, "second"); assert_eq!(selected[2].command, "third"); } + + #[test] + fn select_handlers_by_trust_precedence_runs_non_project_before_project() { + let handlers = vec![ + make_handler_with_source( + HookEventName::PreToolUse, + Some("^Bash$"), + "echo system", + "/etc/codex/hooks.json", + 0, + ), + make_handler_with_source( + HookEventName::PreToolUse, + Some("^Bash$"), + "echo user one", + "/tmp/home/.codex/hooks.json", + 1, + ), + make_handler_with_source( + HookEventName::PreToolUse, + Some("^Bash$"), + "echo user two", + "/tmp/home/.codex/hooks.json", + 2, + ), + make_handler_with_source( + HookEventName::PreToolUse, + Some("^Bash$"), + "echo project", + "/tmp/project/.codex/hooks.json", + 3, + ), + ]; + + let tiers = + select_handlers_by_trust_precedence(&handlers, HookEventName::PreToolUse, Some("Bash")); + + assert_eq!( + tiers + .iter() + .map(|tier| { + tier.iter() + .map(|handler| handler.display_order) + .collect::>() + }) + .collect::>(), + vec![vec![0, 1, 2], vec![3]], + ); + } } diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index f91fee24c9..427d711ee9 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -35,6 +35,7 @@ pub(crate) struct ConfiguredHandler { pub timeout_sec: u64, pub status_message: Option, pub source_path: PathBuf, + pub is_project: bool, pub display_order: i64, } diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 3af9bef5e5..fbf4bad8d6 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -471,6 +471,7 @@ mod tests { timeout_sec: 5, status_message: Some("running post tool use hook".to_string()), source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, } } diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index 8366bb632c..c7fd8d5de5 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -98,15 +98,43 @@ pub(crate) async fn run( } }; - let results = dispatcher::execute_handlers( - shell, - matched, - input_json, - request.cwd.as_path(), - Some(request.turn_id), - parse_completed, + let mut results = Vec::new(); + let mut tiers = dispatcher::select_handlers_by_trust_precedence( + &matched, + HookEventName::PreToolUse, + Some(&request.tool_name), ) - .await; + .into_iter() + .peekable(); + while let Some(tier) = tiers.next() { + let tier_results = dispatcher::execute_handlers( + shell, + tier, + input_json.clone(), + request.cwd.as_path(), + Some(request.turn_id.clone()), + parse_completed, + ) + .await; + let tier_should_block = tier_results.iter().any(|result| result.data.should_block); + results.extend(tier_results); + if tier_should_block { + let skipped_message = + "skipped because a higher-precedence PreToolUse hook blocked the command" + .to_string(); + for skipped_handler in tiers.flatten() { + results.push(dispatcher::ParsedHandler { + completed: dispatcher::skipped_completed_event( + &skipped_handler, + Some(request.turn_id.clone()), + skipped_message.clone(), + ), + data: PreToolUseHandlerData::default(), + }); + } + break; + } + } let should_block = results.iter().any(|result| result.data.should_block); let block_reason = results @@ -232,6 +260,7 @@ fn serialization_failure_outcome(hook_events: Vec) -> PreToo mod tests { use std::path::PathBuf; + use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; @@ -239,7 +268,9 @@ mod tests { use pretty_assertions::assert_eq; use super::PreToolUseHandlerData; + use super::PreToolUseRequest; use super::parse_completed; + use crate::engine::CommandShell; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; @@ -453,6 +484,71 @@ mod tests { ); } + #[tokio::test] + async fn higher_precedence_block_skips_lower_precedence_handlers() -> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let marker_path = temp.path().join("project-ran"); + let handlers = vec![ + ConfiguredHandler { + event_name: HookEventName::PreToolUse, + matcher: Some("^Bash$".to_string()), + command: "printf 'blocked by policy' >&2; exit 2".to_string(), + timeout_sec: 5, + status_message: None, + source_path: PathBuf::from("/tmp/home/.codex/hooks.json"), + is_project: false, + display_order: 0, + }, + ConfiguredHandler { + event_name: HookEventName::PreToolUse, + matcher: Some("^Bash$".to_string()), + command: "touch project-ran".to_string(), + timeout_sec: 5, + status_message: None, + source_path: PathBuf::from("/tmp/project/.codex/hooks.json"), + is_project: true, + display_order: 1, + }, + ]; + + let outcome = super::run( + &handlers, + &CommandShell { + program: "/bin/sh".to_string(), + args: vec!["-c".to_string()], + }, + PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + cwd: temp.path().to_path_buf(), + transcript_path: None, + model: "gpt-5".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + tool_use_id: "tool-1".to_string(), + command: "echo hello".to_string(), + }, + ) + .await; + + assert!(outcome.should_block); + assert_eq!(outcome.block_reason, Some("blocked by policy".to_string())); + assert!(!marker_path.exists()); + assert_eq!(outcome.hook_events.len(), 2); + assert_eq!(outcome.hook_events[0].run.status, HookRunStatus::Blocked); + assert_eq!(outcome.hook_events[1].run.status, HookRunStatus::Failed); + assert_eq!( + outcome.hook_events[1].run.entries, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "skipped because a higher-precedence PreToolUse hook blocked the command" + .to_string(), + }] + ); + + Ok(()) + } + fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::PreToolUse, @@ -461,6 +557,7 @@ mod tests { timeout_sec: 5, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, } } diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index 057c5d0e7c..d2fa57b50b 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -358,6 +358,7 @@ mod tests { timeout_sec: 600, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, } } diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 5eba1414b9..1417f045cd 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -527,6 +527,7 @@ mod tests { timeout_sec: 600, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, } } diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index b909c183be..8fd195e5aa 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -418,6 +418,7 @@ mod tests { timeout_sec: 5, status_message: None, source_path: PathBuf::from("/tmp/hooks.json"), + is_project: false, display_order: 0, } }