From f9dfd3d50c3bdffbad5a1f16aaed3983fe28ce82 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Fri, 1 May 2026 16:18:20 -0700 Subject: [PATCH] Keep PreToolUse core boundary narrow --- codex-rs/core/src/hook_runtime.rs | 31 +++++++++++++++--- codex-rs/core/src/tools/registry.rs | 49 +++++++++-------------------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index fff20a6abd..01d5ce1534 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -140,7 +140,7 @@ pub(crate) async fn run_pre_tool_use_hooks( tool_use_id: String, tool_name: &HookToolName, tool_input: &Value, -) -> PreToolUseOutcome { +) -> Option { let request = PreToolUseRequest { session_id: sess.conversation_id, turn_id: turn_context.sub_id.clone(), @@ -157,10 +157,31 @@ pub(crate) async fn run_pre_tool_use_hooks( let preview_runs = hooks.preview_pre_tool_use(&request); emit_hook_started_events(sess, turn_context, preview_runs).await; - let outcome = hooks.run_pre_tool_use(request).await; - emit_hook_completed_events(sess, turn_context, outcome.hook_events.clone()).await; - record_additional_contexts(sess, turn_context, outcome.additional_contexts.clone()).await; - outcome + let PreToolUseOutcome { + hook_events, + should_block, + block_reason, + additional_contexts, + } = hooks.run_pre_tool_use(request).await; + emit_hook_completed_events(sess, turn_context, hook_events).await; + record_additional_contexts(sess, turn_context, additional_contexts).await; + + if should_block { + block_reason.map(|reason| { + if (tool_name.name() == "Bash" || tool_name.name() == "apply_patch") + && let Some(command) = tool_input.get("command").and_then(Value::as_str) + { + format!("Command blocked by PreToolUse hook: {reason}. Command: {command}") + } else { + format!( + "Tool call blocked by PreToolUse hook: {reason}. Tool: {}", + tool_name.name() + ) + } + }) + } else { + None + } } // PermissionRequest hooks share the same preview/start/completed event flow as diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 86e88e1d22..87b36ff17e 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -354,41 +354,20 @@ impl ToolRegistry { return Err(err); } - let pre_tool_use_outcome = - if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation) { - let outcome = run_pre_tool_use_hooks( - &invocation.session, - &invocation.turn, - invocation.call_id.clone(), - &pre_tool_use_payload.tool_name, - &pre_tool_use_payload.tool_input, - ) - .await; - if outcome.should_block - && let Some(reason) = outcome.block_reason.as_deref() - { - let message = if (pre_tool_use_payload.tool_name.name() == "Bash" - || pre_tool_use_payload.tool_name.name() == "apply_patch") - && let Some(command) = pre_tool_use_payload - .tool_input - .get("command") - .and_then(Value::as_str) - { - format!("Command blocked by PreToolUse hook: {reason}. Command: {command}") - } else { - format!( - "Tool call blocked by PreToolUse hook: {reason}. Tool: {}", - pre_tool_use_payload.tool_name.name() - ) - }; - let err = FunctionCallError::RespondToModel(message); - dispatch_trace.record_failed(&err); - return Err(err); - } - Some(outcome) - } else { - None - }; + if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation) + && let Some(message) = run_pre_tool_use_hooks( + &invocation.session, + &invocation.turn, + invocation.call_id.clone(), + &pre_tool_use_payload.tool_name, + &pre_tool_use_payload.tool_input, + ) + .await + { + let err = FunctionCallError::RespondToModel(message); + dispatch_trace.record_failed(&err); + return Err(err); + } let is_mutating = handler.is_mutating(&invocation).await; let response_cell = tokio::sync::Mutex::new(None);