From 5ecbcf609b53d910c7e3839676699e0ff3a9ef4f Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Wed, 6 May 2026 14:34:31 -0700 Subject: [PATCH] Move tool hook compatibility under hook runtime --- codex-rs/core/src/hook_runtime.rs | 2 ++ .../tool_compat.rs} | 15 ++++++++++++- .../src/tools/handlers/apply_patch_tests.rs | 8 +++---- codex-rs/core/src/tools/handlers/mcp.rs | 8 +++---- .../core/src/tools/handlers/shell_tests.rs | 10 ++++----- .../src/tools/handlers/unified_exec_tests.rs | 8 +++---- codex-rs/core/src/tools/mod.rs | 1 - codex-rs/core/src/tools/registry.rs | 22 ++++--------------- 8 files changed, 37 insertions(+), 37 deletions(-) rename codex-rs/core/src/{tools/hook_compat.rs => hook_runtime/tool_compat.rs} (95%) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 8b6b28cfc3..7958212856 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -38,6 +38,8 @@ use crate::session::turn_context::TurnContext; use crate::tools::hook_names::HookToolName; use crate::tools::sandboxing::PermissionRequestPayload; +pub(crate) mod tool_compat; + pub(crate) struct HookRuntimeOutcome { pub should_stop: bool, pub additional_contexts: Vec, diff --git a/codex-rs/core/src/tools/hook_compat.rs b/codex-rs/core/src/hook_runtime/tool_compat.rs similarity index 95% rename from codex-rs/core/src/tools/hook_compat.rs rename to codex-rs/core/src/hook_runtime/tool_compat.rs index 5821598e66..f25fb60f4b 100644 --- a/codex-rs/core/src/tools/hook_compat.rs +++ b/codex-rs/core/src/hook_runtime/tool_compat.rs @@ -11,7 +11,20 @@ use crate::tools::handlers::shell_command_payload_command; use crate::tools::handlers::shell_function_payload_command; use crate::tools::handlers::unified_exec::ExecCommandArgs; use crate::tools::hook_names::HookToolName; -use crate::tools::registry::PreToolUsePayload; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct PreToolUsePayload { + /// Hook-facing tool name model. + /// + /// The canonical name is serialized to hook stdin, while aliases are used + /// only for matcher compatibility. + pub(crate) tool_name: HookToolName, + /// Tool-specific input exposed at `tool_input`. + /// + /// Shell-like tools use `{ "command": ... }`; MCP tools use their resolved + /// JSON arguments. + pub(crate) tool_input: Value, +} /// Projects native tool payloads into the stable input shape exposed to hooks. /// diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 4fad39b797..c046c38f5d 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -14,12 +14,12 @@ use std::sync::Arc; use tempfile::TempDir; use tokio::sync::Mutex; +use crate::hook_runtime::tool_compat; +use crate::hook_runtime::tool_compat::PreToolUsePayload; use crate::session::tests::make_session_and_context; use crate::tools::context::ToolInvocation; -use crate::tools::hook_compat; use crate::tools::hook_names::HookToolName; use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; use crate::turn_diff_tracker::TurnDiffTracker; fn sample_patch() -> &'static str { @@ -51,7 +51,7 @@ async fn pre_tool_use_payload_uses_json_patch_input() { }; let invocation = invocation_for_payload(payload).await; assert_eq!( - hook_compat::pre_tool_use_payload(&invocation), + tool_compat::pre_tool_use_payload(&invocation), Some(PreToolUsePayload { tool_name: HookToolName::apply_patch(), tool_input: json!({ "command": patch }), @@ -67,7 +67,7 @@ async fn pre_tool_use_payload_uses_freeform_patch_input() { }; let invocation = invocation_for_payload(payload).await; assert_eq!( - hook_compat::pre_tool_use_payload(&invocation), + tool_compat::pre_tool_use_payload(&invocation), Some(PreToolUsePayload { tool_name: HookToolName::apply_patch(), tool_input: json!({ "command": patch }), diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 4536a00616..bb00ece394 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -104,9 +104,9 @@ impl ToolHandler for McpHandler { #[cfg(test)] mod tests { use super::*; + use crate::hook_runtime::tool_compat; use crate::session::tests::make_session_and_context; use crate::tools::context::ToolCallSource; - use crate::tools::hook_compat; use crate::turn_diff_tracker::TurnDiffTracker; use pretty_assertions::assert_eq; use serde_json::json; @@ -128,7 +128,7 @@ mod tests { }; let (session, turn) = make_session_and_context().await; assert_eq!( - hook_compat::pre_tool_use_payload(&ToolInvocation { + tool_compat::pre_tool_use_payload(&ToolInvocation { session: session.into(), turn: turn.into(), cancellation_token: tokio_util::sync::CancellationToken::new(), @@ -138,7 +138,7 @@ mod tests { source: ToolCallSource::Direct, payload, }), - Some(crate::tools::registry::PreToolUsePayload { + Some(tool_compat::PreToolUsePayload { tool_name: HookToolName::new("mcp__memory__create_entities"), tool_input: json!({ "entities": [{ @@ -214,6 +214,6 @@ mod tests { #[test] fn mcp_hook_tool_input_defaults_empty_args_to_object() { - assert_eq!(hook_compat::mcp_hook_tool_input(" "), json!({})); + assert_eq!(tool_compat::mcp_hook_tool_input(" "), json!({})); } } diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index e3208ffe63..c6e4d70abd 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -7,6 +7,7 @@ use core_test_support::test_path_buf; use pretty_assertions::assert_eq; use crate::exec_env::create_env; +use crate::hook_runtime::tool_compat; use crate::sandboxing::SandboxPermissions; use crate::session::tests::make_session_and_context; use crate::shell::Shell; @@ -17,7 +18,6 @@ use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::ShellCommandHandler; -use crate::tools::hook_compat; use crate::tools::hook_names::HookToolName; use crate::tools::registry::ToolHandler; use crate::turn_diff_tracker::TurnDiffTracker; @@ -223,7 +223,7 @@ async fn local_shell_pre_tool_use_payload_uses_joined_command() { let (session, turn) = make_session_and_context().await; assert_eq!( - hook_compat::pre_tool_use_payload(&ToolInvocation { + tool_compat::pre_tool_use_payload(&ToolInvocation { session: session.into(), turn: turn.into(), cancellation_token: tokio_util::sync::CancellationToken::new(), @@ -233,7 +233,7 @@ async fn local_shell_pre_tool_use_payload_uses_joined_command() { source: crate::tools::context::ToolCallSource::Direct, payload, }), - Some(crate::tools::registry::PreToolUsePayload { + Some(tool_compat::PreToolUsePayload { tool_name: HookToolName::bash(), tool_input: json!({ "command": "bash -lc 'printf hi'" }), }) @@ -248,7 +248,7 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() { let (session, turn) = make_session_and_context().await; assert_eq!( - hook_compat::pre_tool_use_payload(&ToolInvocation { + tool_compat::pre_tool_use_payload(&ToolInvocation { session: session.into(), turn: turn.into(), cancellation_token: tokio_util::sync::CancellationToken::new(), @@ -258,7 +258,7 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() { source: crate::tools::context::ToolCallSource::Direct, payload, }), - Some(crate::tools::registry::PreToolUsePayload { + Some(tool_compat::PreToolUsePayload { tool_name: HookToolName::bash(), tool_input: json!({ "command": "printf shell command" }), }) diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index c654ac98ef..b649fb797d 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -6,12 +6,12 @@ use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::sync::Arc; +use crate::hook_runtime::tool_compat; use crate::session::tests::make_session_and_context; use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; -use crate::tools::hook_compat; use crate::tools::hook_names::HookToolName; use crate::tools::registry::ToolHandler; use crate::turn_diff_tracker::TurnDiffTracker; @@ -186,7 +186,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() { }; let (session, turn) = make_session_and_context().await; assert_eq!( - hook_compat::pre_tool_use_payload(&ToolInvocation { + tool_compat::pre_tool_use_payload(&ToolInvocation { session: session.into(), turn: turn.into(), cancellation_token: tokio_util::sync::CancellationToken::new(), @@ -196,7 +196,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() { source: crate::tools::context::ToolCallSource::Direct, payload, }), - Some(crate::tools::registry::PreToolUsePayload { + Some(tool_compat::PreToolUsePayload { tool_name: HookToolName::bash(), tool_input: serde_json::json!({ "command": "printf exec command" }), }) @@ -210,7 +210,7 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() { }; let (session, turn) = make_session_and_context().await; assert_eq!( - hook_compat::pre_tool_use_payload(&ToolInvocation { + tool_compat::pre_tool_use_payload(&ToolInvocation { session: session.into(), turn: turn.into(), cancellation_token: tokio_util::sync::CancellationToken::new(), diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index eeea95687e..659a7d3e54 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -2,7 +2,6 @@ pub(crate) mod code_mode; pub(crate) mod context; pub(crate) mod events; pub(crate) mod handlers; -pub(crate) mod hook_compat; pub(crate) mod hook_names; pub(crate) mod network_approval; pub(crate) mod orchestrator; diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index c15850449b..749c9f575c 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -9,6 +9,7 @@ use crate::hook_runtime::PreToolUseHookResult; use crate::hook_runtime::record_additional_contexts; use crate::hook_runtime::run_post_tool_use_hooks; use crate::hook_runtime::run_pre_tool_use_hooks; +use crate::hook_runtime::tool_compat; use crate::memory_usage::emit_metric_for_tool_read; use crate::sandbox_tags::permission_profile_policy_tag; use crate::sandbox_tags::permission_profile_sandbox_tag; @@ -17,7 +18,6 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; -use crate::tools::hook_compat; use crate::tools::hook_names::HookToolName; use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use codex_hooks::HookEvent; @@ -131,20 +131,6 @@ impl AnyToolResult { } } -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct PreToolUsePayload { - /// Hook-facing tool name model. - /// - /// The canonical name is serialized to hook stdin, while aliases are used - /// only for matcher compatibility. - pub(crate) tool_name: HookToolName, - /// Tool-specific input exposed at `tool_input`. - /// - /// Shell-like tools use `{ "command": ... }`; MCP tools use their resolved - /// JSON arguments. - pub(crate) tool_input: Value, -} - #[derive(Debug, Clone, PartialEq)] pub(crate) struct PostToolUsePayload { /// Hook-facing tool name model. @@ -342,7 +328,7 @@ impl ToolRegistry { return Err(err); } - if let Some(pre_tool_use_payload) = hook_compat::pre_tool_use_payload(&invocation) { + if let Some(pre_tool_use_payload) = tool_compat::pre_tool_use_payload(&invocation) { match run_pre_tool_use_hooks( &invocation.session, &invocation.turn, @@ -360,9 +346,9 @@ impl ToolRegistry { PreToolUseHookResult::Continue { updated_input: Some(updated_input), } => { - invocation = hook_compat::apply_updated_input(invocation, updated_input)?; + invocation = tool_compat::apply_updated_input(invocation, updated_input)?; } - crate::hook_runtime::PreToolUseHookResult::Continue { + PreToolUseHookResult::Continue { updated_input: None, } => {} }