From 7e25c99242fe741dbda8ab5da259988f12b1266e Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 16 Apr 2026 15:11:59 -0300 Subject: [PATCH] fix(core): emit hooks for apply_patch edits Teach apply_patch to provide PreToolUse and PostToolUse payloads for hook dispatch, covering both JSON and freeform patch calls. Pass handler-supplied hook names through runtime serialization so existing Bash matchers keep working and apply_patch can be matched directly. --- codex-rs/core/src/hook_runtime.rs | 19 +- codex-rs/core/src/tools/context.rs | 11 ++ .../core/src/tools/handlers/apply_patch.rs | 39 ++++ .../src/tools/handlers/apply_patch_tests.rs | 83 +++++++++ codex-rs/core/src/tools/handlers/shell.rs | 13 +- .../core/src/tools/handlers/shell_tests.rs | 3 + .../core/src/tools/handlers/unified_exec.rs | 6 +- .../src/tools/handlers/unified_exec_tests.rs | 2 + codex-rs/core/src/tools/registry.rs | 18 ++ codex-rs/core/tests/suite/hooks.rs | 171 +++++++++++++++++- .../post-tool-use.command.input.schema.json | 1 - .../pre-tool-use.command.input.schema.json | 1 - codex-rs/hooks/src/events/post_tool_use.rs | 52 ++++-- codex-rs/hooks/src/events/pre_tool_use.rs | 50 +++-- codex-rs/hooks/src/schema.rs | 10 - 15 files changed, 428 insertions(+), 51 deletions(-) diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 342fca91e828..e6995bcaf6af 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -127,10 +127,18 @@ pub(crate) async fn run_pending_session_start_hooks( .await } +/// Runs matching `PreToolUse` hooks before a tool executes. +/// +/// `tool_name` is the hook-facing matcher name that will also be serialized to +/// hook stdin. Callers should pass the handler-provided compatibility name +/// rather than deriving it from the internal registry key; otherwise existing +/// matchers such as `^Bash$` can stop matching even though the underlying tool +/// is still shell-like. pub(crate) async fn run_pre_tool_use_hooks( sess: &Arc, turn_context: &Arc, tool_use_id: String, + tool_name: String, command: String, ) -> Option { let request = PreToolUseRequest { @@ -140,7 +148,7 @@ pub(crate) async fn run_pre_tool_use_hooks( transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), permission_mode: hook_permission_mode(turn_context), - tool_name: "Bash".to_string(), + tool_name, tool_use_id, command, }; @@ -190,10 +198,17 @@ pub(crate) async fn run_permission_request_hooks( decision } +/// Runs matching `PostToolUse` hooks after a tool has produced a successful output. +/// +/// The `tool_name`, `command`, and `tool_response` values are already adapted +/// by the tool handler into the stable hook contract. Passing raw internal tool +/// data here would leak implementation details into user hook matchers and hook +/// logs. pub(crate) async fn run_post_tool_use_hooks( sess: &Arc, turn_context: &Arc, tool_use_id: String, + tool_name: String, command: String, tool_response: Value, ) -> PostToolUseOutcome { @@ -204,7 +219,7 @@ pub(crate) async fn run_post_tool_use_hooks( transcript_path: sess.hook_transcript_path().await, model: turn_context.model_info.slug.clone(), permission_mode: hook_permission_mode(turn_context), - tool_name: "Bash".to_string(), + tool_name, tool_use_id, command, tool_response, diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index a2f3a7f7c691..22617cf8ad24 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -85,6 +85,13 @@ pub trait ToolOutput: Send { fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem; + /// Returns the stable value exposed to `PostToolUse` hooks for this tool output. + /// + /// Tool handlers decide whether a tool participates in `PostToolUse`, but + /// this method lets the output type own any conversion from model-facing + /// response content to hook-facing data. Returning `None` means the output + /// should not produce a post-use hook payload, not merely that the tool had + /// empty output. fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { None } @@ -303,6 +310,10 @@ impl ToolOutput for ApplyPatchToolOutput { ) } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { + Some(JsonValue::String(self.text.clone())) + } + fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue { JsonValue::Object(serde_json::Map::new()) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index e713c59b4e0c..66b226d4f9e1 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -14,12 +14,15 @@ use crate::tools::context::ApplyPatchToolOutput; use crate::tools::context::FunctionToolOutput; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolInvocation; +use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::handlers::apply_granted_turn_permissions; use crate::tools::handlers::parse_arguments; use crate::tools::orchestrator::ToolOrchestrator; +use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -206,6 +209,21 @@ fn write_permissions_for_paths( normalize_additional_permissions(permissions).ok() } +/// Extracts the raw patch text used as the command-shaped hook input for apply_patch. +/// +/// The apply_patch tool can arrive as the older JSON/function shape or as a +/// freeform custom tool call. Both represent the same file edit operation, so +/// hooks see the raw patch body in `tool_input.command` either way. +fn apply_patch_payload_command(payload: &ToolPayload) -> Option { + match payload { + ToolPayload::Function { arguments } => parse_arguments::(arguments) + .ok() + .map(|args| args.input), + ToolPayload::Custom { input } => Some(input.clone()), + _ => None, + } +} + async fn effective_patch_permissions( session: &Session, turn: &TurnContext, @@ -260,6 +278,27 @@ impl ToolHandler for ApplyPatchHandler { Some(Box::::default()) } + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + apply_patch_payload_command(&invocation.payload).map(|command| PreToolUsePayload { + tool_name: "apply_patch".to_string(), + command, + }) + } + + fn post_tool_use_payload( + &self, + call_id: &str, + payload: &ToolPayload, + result: &dyn ToolOutput, + ) -> Option { + let tool_response = result.post_tool_use_response(call_id, payload)?; + Some(PostToolUsePayload { + tool_name: "apply_patch".to_string(), + command: apply_patch_payload_command(payload)?, + tool_response, + }) + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, 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 a2c9c23af585..d2ebead36123 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -7,9 +7,92 @@ use codex_protocol::protocol::SandboxPolicy; use core_test_support::PathBufExt; use core_test_support::PathExt; use pretty_assertions::assert_eq; +use serde_json::json; use std::collections::HashMap; use std::path::PathBuf; +use std::sync::Arc; use tempfile::TempDir; +use tokio::sync::Mutex; + +use crate::session::tests::make_session_and_context; +use crate::tools::context::ToolInvocation; +use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; +use crate::turn_diff_tracker::TurnDiffTracker; + +fn sample_patch() -> &'static str { + r#"*** Begin Patch +*** Add File: hello.txt ++hello +*** End Patch"# +} + +async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation { + let (session, turn) = make_session_and_context().await; + ToolInvocation { + session: session.into(), + turn: turn.into(), + tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), + call_id: "call-apply-patch".to_string(), + tool_name: codex_tools::ToolName::plain("apply_patch"), + payload, + } +} + +#[tokio::test] +async fn pre_tool_use_payload_uses_json_patch_input() { + let patch = sample_patch(); + let payload = ToolPayload::Function { + arguments: json!({ "input": patch }).to_string(), + }; + let invocation = invocation_for_payload(payload).await; + let handler = ApplyPatchHandler; + + assert_eq!( + handler.pre_tool_use_payload(&invocation), + Some(PreToolUsePayload { + tool_name: "apply_patch".to_string(), + command: patch.to_string(), + }) + ); +} + +#[tokio::test] +async fn pre_tool_use_payload_uses_freeform_patch_input() { + let patch = sample_patch(); + let payload = ToolPayload::Custom { + input: patch.to_string(), + }; + let invocation = invocation_for_payload(payload).await; + let handler = ApplyPatchHandler; + + assert_eq!( + handler.pre_tool_use_payload(&invocation), + Some(PreToolUsePayload { + tool_name: "apply_patch".to_string(), + command: patch.to_string(), + }) + ); +} + +#[test] +fn post_tool_use_payload_uses_patch_input_and_tool_output() { + let patch = sample_patch(); + let payload = ToolPayload::Custom { + input: patch.to_string(), + }; + let output = ApplyPatchToolOutput::from_text("Success. Updated files.".to_string()); + let handler = ApplyPatchHandler; + + assert_eq!( + handler.post_tool_use_payload("call-apply-patch", &payload, &output), + Some(PostToolUsePayload { + tool_name: "apply_patch".to_string(), + command: patch.to_string(), + tool_response: json!("Success. Updated files."), + }) + ); +} #[test] fn diff_consumer_does_not_stream_json_tool_call_arguments() { diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b6bf4bc7adce..2cd3eed3e3a7 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -205,7 +205,10 @@ impl ToolHandler for ShellHandler { } fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload { command }) + shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload { + tool_name: "Bash".to_string(), + command, + }) } fn post_tool_use_payload( @@ -216,6 +219,7 @@ impl ToolHandler for ShellHandler { ) -> Option { let tool_response = result.post_tool_use_response(call_id, payload)?; Some(PostToolUsePayload { + tool_name: "Bash".to_string(), command: shell_payload_command(payload)?, tool_response, }) @@ -313,8 +317,10 @@ impl ToolHandler for ShellCommandHandler { } fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - shell_command_payload_command(&invocation.payload) - .map(|command| PreToolUsePayload { command }) + shell_command_payload_command(&invocation.payload).map(|command| PreToolUsePayload { + tool_name: "Bash".to_string(), + command, + }) } fn post_tool_use_payload( @@ -325,6 +331,7 @@ impl ToolHandler for ShellCommandHandler { ) -> Option { let tool_response = result.post_tool_use_response(call_id, payload)?; Some(PostToolUsePayload { + tool_name: "Bash".to_string(), command: shell_command_payload_command(payload)?, tool_response, }) diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index 652b305688d3..25dee6340717 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -231,6 +231,7 @@ async fn shell_pre_tool_use_payload_uses_joined_command() { payload, }), Some(crate::tools::registry::PreToolUsePayload { + tool_name: "Bash".to_string(), command: "bash -lc 'printf hi'".to_string(), }) ); @@ -256,6 +257,7 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() { payload, }), Some(crate::tools::registry::PreToolUsePayload { + tool_name: "Bash".to_string(), command: "printf shell command".to_string(), }) ); @@ -278,6 +280,7 @@ fn build_post_tool_use_payload_uses_tool_output_wire_value() { assert_eq!( handler.post_tool_use_payload("call-42", &payload, &output), Some(crate::tools::registry::PostToolUsePayload { + tool_name: "Bash".to_string(), command: "printf shell command".to_string(), tool_response: json!("shell output"), }) diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index ab5279d1b8d2..4bed7feb68ec 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -136,7 +136,10 @@ impl ToolHandler for UnifiedExecHandler { parse_arguments::(arguments) .ok() - .map(|args| PreToolUsePayload { command: args.cmd }) + .map(|args| PreToolUsePayload { + tool_name: "Bash".to_string(), + command: args.cmd, + }) } fn post_tool_use_payload( @@ -156,6 +159,7 @@ impl ToolHandler for UnifiedExecHandler { let tool_response = result.post_tool_use_response(call_id, payload)?; Some(PostToolUsePayload { + tool_name: "Bash".to_string(), command: args.cmd, tool_response, }) 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 c208ed1f378f..598bfa92bc84 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -216,6 +216,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() { payload, }), Some(crate::tools::registry::PreToolUsePayload { + tool_name: "Bash".to_string(), command: "printf exec command".to_string(), }) ); @@ -266,6 +267,7 @@ fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_co assert_eq!( UnifiedExecHandler.post_tool_use_payload("call-43", &payload, &output), Some(crate::tools::registry::PostToolUsePayload { + tool_name: "Bash".to_string(), command: "echo three".to_string(), tool_response: serde_json::json!("three"), }) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 0e0b7b36dd19..20edb8d656e7 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -124,12 +124,28 @@ impl AnyToolResult { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PreToolUsePayload { + /// Hook-facing tool name used for matcher selection and hook stdin. + /// + /// This is intentionally handler supplied because compatibility names do + /// not always match registry names. Shell-style handlers use `Bash` for + /// historical matcher compatibility, while first-class patch edits use + /// `apply_patch`. + pub(crate) tool_name: String, + /// Command-shaped input exposed at `tool_input.command`. pub(crate) command: String, } #[derive(Debug, Clone, PartialEq)] pub(crate) struct PostToolUsePayload { + /// Hook-facing tool name used for matcher selection and hook stdin. + /// + /// Keep this aligned with the corresponding pre-use payload so external + /// hook consumers can pair events by `tool_use_id` without also needing to + /// normalize tool names. + pub(crate) tool_name: String, + /// Command-shaped input exposed at `tool_input.command`. pub(crate) command: String, + /// Tool result exposed at `tool_response`. pub(crate) tool_response: Value, } @@ -321,6 +337,7 @@ impl ToolRegistry { &invocation.session, &invocation.turn, invocation.call_id.clone(), + pre_tool_use_payload.tool_name.clone(), pre_tool_use_payload.command.clone(), ) .await @@ -391,6 +408,7 @@ impl ToolRegistry { &invocation.session, &invocation.turn, invocation.call_id.clone(), + post_tool_use_payload.tool_name, post_tool_use_payload.command, post_tool_use_payload.tool_response, ) diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index ee63ca77b808..2ac61173378f 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -21,6 +21,7 @@ use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +use core_test_support::responses::ev_apply_patch_function_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -1812,7 +1813,84 @@ async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn pre_tool_use_does_not_fire_for_non_shell_tools() -> Result<()> { +async fn pre_tool_use_blocks_apply_patch_before_execution() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "pretooluse-apply-patch"; + let file_name = "pre_tool_use_apply_patch.txt"; + let patch = format!( + r#"*** Begin Patch +*** Add File: {file_name} ++blocked +*** End Patch"# + ); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_function_call(call_id, &patch), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "apply_patch blocked"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = write_pre_tool_use_hook( + home, + Some("^apply_patch$"), + "json_deny", + "blocked apply_patch", + ) { + panic!("failed to write pre tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config.include_apply_patch_tool = true; + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("apply the blocked patch").await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("apply_patch output string"); + assert!( + output.contains("Command blocked by PreToolUse hook: blocked apply_patch"), + "blocked apply_patch output should surface the hook reason", + ); + assert!( + !test.workspace_path(file_name).exists(), + "blocked apply_patch should not create the file" + ); + + let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["tool_name"], "apply_patch"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], patch); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1877,7 +1955,7 @@ async fn pre_tool_use_does_not_fire_for_non_shell_tools() -> Result<()> { let hook_log_path = test.codex_home_path().join("pre_tool_use_hook_log.jsonl"); assert!( !hook_log_path.exists(), - "non-shell tools should not trigger pre tool use hooks", + "plan tool should not trigger pre tool use hooks", ); Ok(()) @@ -2267,7 +2345,92 @@ async fn post_tool_use_exit_two_replaces_one_shot_exec_command_output_with_feedb } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn post_tool_use_does_not_fire_for_non_shell_tools() -> Result<()> { +async fn post_tool_use_records_additional_context_for_apply_patch() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-apply-patch"; + let file_name = "post_tool_use_apply_patch.txt"; + let patch = format!( + r#"*** Begin Patch +*** Add File: {file_name} ++patched +*** End Patch"# + ); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_function_call(call_id, &patch), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "apply_patch post hook context observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let post_context = "Remember the apply_patch post-tool note."; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = + write_post_tool_use_hook(home, Some("^apply_patch$"), "context", post_context) + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config.include_apply_patch_tool = true; + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("apply the patch with post hook").await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[1] + .message_input_texts("developer") + .contains(&post_context.to_string()), + "follow-up request should include apply_patch post tool use context", + ); + assert!( + test.workspace_path(file_name).exists(), + "apply_patch should create the file" + ); + + let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["tool_name"], "apply_patch"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], patch); + let tool_response = hook_inputs[0]["tool_response"] + .as_str() + .context("apply_patch tool_response should be a string")?; + assert_eq!( + serde_json::from_str::(tool_response)?, + serde_json::json!({ + "output": "Success. Updated the following files:\nA post_tool_use_apply_patch.txt\n", + "metadata": { + "exit_code": 0, + "duration_seconds": 0.0, + }, + }) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_does_not_fire_for_plan_tool() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -2335,7 +2498,7 @@ async fn post_tool_use_does_not_fire_for_non_shell_tools() -> Result<()> { let hook_log_path = test.codex_home_path().join("post_tool_use_hook_log.jsonl"); assert!( !hook_log_path.exists(), - "non-shell tools should not trigger post tool use hooks", + "plan tool should not trigger post tool use hooks", ); Ok(()) diff --git a/codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json b/codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json index e8a87c1f5e5d..6a3fcfc47c89 100644 --- a/codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json +++ b/codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json @@ -49,7 +49,6 @@ "$ref": "#/definitions/PostToolUseToolInput" }, "tool_name": { - "const": "Bash", "type": "string" }, "tool_response": true, diff --git a/codex-rs/hooks/schema/generated/pre-tool-use.command.input.schema.json b/codex-rs/hooks/schema/generated/pre-tool-use.command.input.schema.json index 86dfac165c8d..df3bec7ff6e5 100644 --- a/codex-rs/hooks/schema/generated/pre-tool-use.command.input.schema.json +++ b/codex-rs/hooks/schema/generated/pre-tool-use.command.input.schema.json @@ -49,7 +49,6 @@ "$ref": "#/definitions/PreToolUseToolInput" }, "tool_name": { - "const": "Bash", "type": "string" }, "tool_use_id": { diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index f7f40d279475..e70279ceb809 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -86,21 +86,7 @@ pub(crate) async fn run( }; } - let input_json = match serde_json::to_string(&PostToolUseCommandInput { - session_id: request.session_id.to_string(), - turn_id: request.turn_id.clone(), - transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), - cwd: request.cwd.display().to_string(), - hook_event_name: "PostToolUse".to_string(), - model: request.model.clone(), - permission_mode: request.permission_mode.clone(), - tool_name: "Bash".to_string(), - tool_input: PostToolUseToolInput { - command: request.command.clone(), - }, - tool_response: request.tool_response.clone(), - tool_use_id: request.tool_use_id.clone(), - }) { + let input_json = match command_input_json(&request) { Ok(input_json) => input_json, Err(error) => { let hook_events = common::serialization_failure_hook_events_for_tool_use( @@ -153,6 +139,29 @@ pub(crate) async fn run( } } +/// Serializes command stdin for a selected `PostToolUse` hook. +/// +/// The request has already been matched by `tool_name`, so the serialized +/// payload preserves that same name for hook logs and for consumers that pair +/// pre/post events across processes. +fn command_input_json(request: &PostToolUseRequest) -> Result { + serde_json::to_string(&PostToolUseCommandInput { + session_id: request.session_id.to_string(), + turn_id: request.turn_id.clone(), + transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), + cwd: request.cwd.display().to_string(), + hook_event_name: "PostToolUse".to_string(), + model: request.model.clone(), + permission_mode: request.permission_mode.clone(), + tool_name: request.tool_name.clone(), + tool_input: PostToolUseToolInput { + command: request.command.clone(), + }, + tool_response: request.tool_response.clone(), + tool_use_id: request.tool_use_id.clone(), + }) +} + fn parse_completed( handler: &ConfiguredHandler, run_result: CommandRunResult, @@ -314,12 +323,25 @@ mod tests { use serde_json::json; use super::PostToolUseHandlerData; + use super::command_input_json; use super::parse_completed; use super::preview; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; use crate::events::common; + #[test] + fn command_input_uses_request_tool_name() { + let mut request = request_for_tool_use("call-apply-patch"); + request.tool_name = "apply_patch".to_string(); + + let input_json = command_input_json(&request).expect("serialize command input"); + let input: serde_json::Value = + serde_json::from_str(&input_json).expect("parse command input"); + + assert_eq!(input["tool_name"], "apply_patch"); + } + #[test] fn block_decision_stops_normal_processing() { let parsed = parse_completed( diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index f90abc2816e0..158c4879767c 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -77,20 +77,7 @@ pub(crate) async fn run( }; } - let input_json = match serde_json::to_string(&PreToolUseCommandInput { - session_id: request.session_id.to_string(), - turn_id: request.turn_id.clone(), - transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), - cwd: request.cwd.display().to_string(), - hook_event_name: "PreToolUse".to_string(), - model: request.model.clone(), - permission_mode: request.permission_mode.clone(), - tool_name: "Bash".to_string(), - tool_input: crate::schema::PreToolUseToolInput { - command: request.command.clone(), - }, - tool_use_id: request.tool_use_id.clone(), - }) { + let input_json = match command_input_json(&request) { Ok(input_json) => input_json, Err(error) => { let hook_events = common::serialization_failure_hook_events_for_tool_use( @@ -130,6 +117,28 @@ pub(crate) async fn run( } } +/// Serializes command stdin for a selected `PreToolUse` hook. +/// +/// Handler selection and command input must use the same `tool_name`; otherwise +/// a hook may match one name but receive a different name in its JSON payload, +/// which makes audit logs and downstream policy decisions disagree. +fn command_input_json(request: &PreToolUseRequest) -> Result { + serde_json::to_string(&PreToolUseCommandInput { + session_id: request.session_id.to_string(), + turn_id: request.turn_id.clone(), + transcript_path: crate::schema::NullableString::from_path(request.transcript_path.clone()), + cwd: request.cwd.display().to_string(), + hook_event_name: "PreToolUse".to_string(), + model: request.model.clone(), + permission_mode: request.permission_mode.clone(), + tool_name: request.tool_name.clone(), + tool_input: crate::schema::PreToolUseToolInput { + command: request.command.clone(), + }, + tool_use_id: request.tool_use_id.clone(), + }) +} + fn parse_completed( handler: &ConfiguredHandler, run_result: CommandRunResult, @@ -250,12 +259,25 @@ mod tests { use pretty_assertions::assert_eq; use super::PreToolUseHandlerData; + use super::command_input_json; use super::parse_completed; use super::preview; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; use crate::events::common; + #[test] + fn command_input_uses_request_tool_name() { + let mut request = request_for_tool_use("call-apply-patch"); + request.tool_name = "apply_patch".to_string(); + + let input_json = command_input_json(&request).expect("serialize command input"); + let input: serde_json::Value = + serde_json::from_str(&input_json).expect("parse command input"); + + assert_eq!(input["tool_name"], "apply_patch"); + } + #[test] fn permission_decision_deny_blocks_processing() { let parsed = parse_completed( diff --git a/codex-rs/hooks/src/schema.rs b/codex-rs/hooks/src/schema.rs index d7693ac7049d..83b18c74f6f5 100644 --- a/codex-rs/hooks/src/schema.rs +++ b/codex-rs/hooks/src/schema.rs @@ -231,7 +231,6 @@ pub(crate) struct PreToolUseCommandInput { pub model: String, #[schemars(schema_with = "permission_mode_schema")] pub permission_mode: String, - #[schemars(schema_with = "pre_tool_use_tool_name_schema")] pub tool_name: String, pub tool_input: PreToolUseToolInput, pub tool_use_id: String, @@ -286,7 +285,6 @@ pub(crate) struct PostToolUseCommandInput { pub model: String, #[schemars(schema_with = "permission_mode_schema")] pub permission_mode: String, - #[schemars(schema_with = "post_tool_use_tool_name_schema")] pub tool_name: String, pub tool_input: PostToolUseToolInput, pub tool_response: Value, @@ -545,10 +543,6 @@ fn post_tool_use_hook_event_name_schema(_gen: &mut SchemaGenerator) -> Schema { string_const_schema("PostToolUse") } -fn post_tool_use_tool_name_schema(_gen: &mut SchemaGenerator) -> Schema { - string_const_schema("Bash") -} - fn pre_tool_use_hook_event_name_schema(_gen: &mut SchemaGenerator) -> Schema { string_const_schema("PreToolUse") } @@ -557,10 +551,6 @@ fn permission_request_hook_event_name_schema(_gen: &mut SchemaGenerator) -> Sche string_const_schema("PermissionRequest") } -fn pre_tool_use_tool_name_schema(_gen: &mut SchemaGenerator) -> Schema { - string_const_schema("Bash") -} - fn permission_request_tool_name_schema(_gen: &mut SchemaGenerator) -> Schema { string_const_schema("Bash") }