openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-08 20:40:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: run project pre-tool hooks after trusted hooks

Browse Source
This commit is contained in:
viyatb-oai
2026-04-07 12:25:00 -07:00
parent ab48990728
commit ac0800ed36
9 changed files with 271 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/config/src/state.rs
+4
View File
@@ -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()
}
codex-rs/hooks/src/engine/discovery.rs
+46 -19
View File
@@ -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<ConfiguredHandler>,
warnings: &mut Vec<String>,
display_order: &mut i64,
source_path: &Path,
event_name: codex_protocol::protocol::HookEventName,
context: AppendGroupContext<'_>,
matcher: Option<&str>,
group_handlers: Vec<HookHandlerConfig>,
) {
@@ -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<ConfiguredHandler>,
warnings: &mut Vec<String>,
display_order: &mut i64,
source_path: &Path,
is_project: bool,
event_name: codex_protocol::protocol::HookEventName,
groups: Vec<MatcherGroup>,
) {
@@ -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(),
codex-rs/hooks/src/engine/dispatcher.rs
+111
View File
@@ -82,6 +82,47 @@ pub(crate) async fn execute_handlers<T>(
.collect()
}
pub(crate) fn select_handlers_by_trust_precedence(
handlers: &[ConfiguredHandler],
event_name: HookEventName,
matcher_input: Option<&str>,
) -> Vec<Vec<ConfiguredHandler>> {
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<String>,
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::<Vec<_>>()
})
.collect::<Vec<_>>(),
vec![vec![0, 1, 2], vec![3]],
);
}
}
codex-rs/hooks/src/engine/mod.rs
+1
View File
@@ -35,6 +35,7 @@ pub(crate) struct ConfiguredHandler {
pub timeout_sec: u64,
pub status_message: Option<String>,
pub source_path: PathBuf,
pub is_project: bool,
pub display_order: i64,
}
codex-rs/hooks/src/events/post_tool_use.rs
+1
View File
@@ -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,
}
}
codex-rs/hooks/src/events/pre_tool_use.rs
+105 -8
View File
@@ -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<HookCompletedEvent>) -> 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,
}
}
codex-rs/hooks/src/events/session_start.rs
+1
View File
@@ -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,
}
}
codex-rs/hooks/src/events/stop.rs
+1
View File
@@ -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,
}
}
codex-rs/hooks/src/events/user_prompt_submit.rs
+1
View File
@@ -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,
}
}