diff --git a/codex-rs/core/src/tools/code_mode/wait_handler.rs b/codex-rs/core/src/tools/code_mode/wait_handler.rs index fccf516b1..d0c0453df 100644 --- a/codex-rs/core/src/tools/code_mode/wait_handler.rs +++ b/codex-rs/core/src/tools/code_mode/wait_handler.rs @@ -2,9 +2,12 @@ use serde::Deserialize; use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; +use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::context::boxed_tool_output; use crate::tools::registry::CoreToolRuntime; +use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -110,4 +113,22 @@ impl ToolExecutor for CodeModeWaitHandler { } } -impl CoreToolRuntime for CodeModeWaitHandler {} +impl CoreToolRuntime for CodeModeWaitHandler { + fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option { + // Code-mode `wait` is runtime control for an existing code cell, not a + // standalone user action. Tool calls made from code mode still flow + // through normal dispatch, but hooks should not block or rewrite the + // wait loop itself. + None + } + + fn post_tool_use_payload( + &self, + _invocation: &ToolInvocation, + _result: &dyn ToolOutput, + ) -> Option { + // The wait result feeds code-mode control flow, so do not let + // PostToolUse replace it with model-facing hook feedback. + None + } +} diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 470b32bea..8c9f55c46 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -4,17 +4,12 @@ use codex_tools::ConversationHistory; use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolName; use codex_tools::ToolSpec; -use serde_json::Value; use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; -use crate::tools::flat_tool_name; -use crate::tools::hook_names::HookToolName; use crate::tools::registry::CoreToolRuntime; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; pub(crate) struct ExtensionToolAdapter(Arc>); @@ -23,13 +18,6 @@ impl ExtensionToolAdapter { pub(crate) fn new(executor: Arc>) -> Self { Self(executor) } - - fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> { - let ToolPayload::Function { arguments } = payload else { - return None; - }; - Some(arguments) - } } #[async_trait::async_trait] @@ -60,30 +48,7 @@ impl ToolExecutor for ExtensionToolAdapter { impl CoreToolRuntime for ExtensionToolAdapter { fn matches_kind(&self, payload: &ToolPayload) -> bool { - self.arguments_from_payload(payload).is_some() - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - let arguments = self.arguments_from_payload(&invocation.payload)?; - Some(PreToolUsePayload { - tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()), - tool_input: extension_tool_hook_input(arguments), - }) - } - - fn post_tool_use_payload( - &self, - invocation: &ToolInvocation, - result: &dyn ToolOutput, - ) -> Option { - let arguments = self.arguments_from_payload(&invocation.payload)?; - Some(PostToolUsePayload { - tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()), - tool_use_id: invocation.call_id.clone(), - tool_input: extension_tool_hook_input(arguments), - tool_response: result - .post_tool_use_response(&invocation.call_id, &invocation.payload)?, - }) + matches!(payload, ToolPayload::Function { .. }) } } @@ -100,14 +65,6 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { } } -fn extension_tool_hook_input(arguments: &str) -> Value { - if arguments.trim().is_empty() { - return Value::Object(serde_json::Map::new()); - } - - serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string())) -} - #[cfg(test)] mod tests { use std::sync::Arc; diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 5f6261323..0d5c15827 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -9,10 +9,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::context::boxed_tool_output; use crate::tools::flat_tool_name; -use crate::tools::hook_names::HookToolName; use crate::tools::registry::CoreToolRuntime; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use crate::tools::registry::ToolTelemetryTags; use crate::tools::tool_search_entry::ToolSearchInfo; @@ -23,8 +20,6 @@ use codex_tools::ToolName; use codex_tools::ToolSearchSourceInfo; use codex_tools::ToolSpec; use codex_tools::mcp_tool_to_responses_api_tool; -use serde_json::Map; -use serde_json::Value; pub struct McpHandler { tool_info: ToolInfo, @@ -143,58 +138,6 @@ impl CoreToolRuntime for McpHandler { tags }) } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - let ToolPayload::Function { arguments } = &invocation.payload else { - return None; - }; - - Some(PreToolUsePayload { - tool_name: HookToolName::new(self.tool_name().to_string()), - tool_input: mcp_hook_tool_input(arguments), - }) - } - - fn with_updated_hook_input( - &self, - mut invocation: ToolInvocation, - updated_input: Value, - ) -> Result { - invocation.payload = match invocation.payload { - ToolPayload::Function { .. } => ToolPayload::Function { - arguments: serde_json::to_string(&updated_input).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to serialize rewritten MCP arguments: {err}" - )) - })?, - }, - payload => { - return Err(FunctionCallError::RespondToModel(format!( - "tool {} does not support hook input rewriting for payload {payload:?}", - self.tool_name() - ))); - } - }; - Ok(invocation) - } - fn post_tool_use_payload( - &self, - invocation: &ToolInvocation, - result: &dyn crate::tools::context::ToolOutput, - ) -> Option { - let ToolPayload::Function { .. } = &invocation.payload else { - return None; - }; - - let tool_response = - result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; - Some(PostToolUsePayload { - tool_name: HookToolName::new(self.tool_name().to_string()), - tool_use_id: invocation.call_id.clone(), - tool_input: result.post_tool_use_input(&invocation.payload)?, - tool_response, - }) - } } fn create_tool_spec(tool_info: &ToolInfo) -> Result { @@ -223,14 +166,6 @@ fn create_tool_spec(tool_info: &ToolInfo) -> Result })) } -fn mcp_hook_tool_input(raw_arguments: &str) -> Value { - if raw_arguments.trim().is_empty() { - return Value::Object(Map::new()); - } - - serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string())) -} - fn build_mcp_search_text(info: &ToolInfo) -> String { let tool_name = info.canonical_tool_name(); let mut schema_properties = info @@ -288,6 +223,9 @@ mod tests { use super::*; use crate::session::tests::make_session_and_context; use crate::tools::context::ToolCallSource; + use crate::tools::hook_names::HookToolName; + use crate::tools::registry::PostToolUsePayload; + use crate::tools::registry::PreToolUsePayload; use crate::turn_diff_tracker::TurnDiffTracker; use pretty_assertions::assert_eq; use serde_json::json; @@ -447,11 +385,6 @@ mod tests { ); } - #[test] - fn mcp_hook_tool_input_defaults_empty_args_to_object() { - assert_eq!(mcp_hook_tool_input(" "), json!({})); - } - #[test] fn mcp_read_only_hint_supports_parallel_calls_without_server_opt_in() { let mut read_only_info = tool_info("foo", "mcp__foo__", "read"); diff --git a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs index dfa4240eb..a639ea006 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs @@ -5,6 +5,7 @@ use crate::tools::context::boxed_tool_output; use crate::tools::handlers::parse_arguments; use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; use crate::unified_exec::WriteStdinRequest; use codex_protocol::protocol::EventMsg; @@ -101,11 +102,20 @@ impl CoreToolRuntime for WriteStdinHandler { matches!(payload, ToolPayload::Function { .. }) } + fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option { + // `write_stdin` is transport for an existing exec session. Empty writes + // are background polls, and non-empty writes continue a command that + // already ran PreToolUse as Bash, so do not emit a second pre hook here. + None + } + fn post_tool_use_payload( &self, invocation: &ToolInvocation, result: &dyn crate::tools::context::ToolOutput, ) -> Option { + // A `write_stdin` poll can observe final completion for the original + // `exec_command`; emit that command's matching Bash PostToolUse. post_unified_exec_tool_use_payload(invocation, result) } } diff --git a/codex-rs/core/src/tools/hook_names.rs b/codex-rs/core/src/tools/hook_names.rs index 9d3b6c240..92ebe8aa5 100644 --- a/codex-rs/core/src/tools/hook_names.rs +++ b/codex-rs/core/src/tools/hook_names.rs @@ -38,6 +38,18 @@ impl HookToolName { } } + /// Returns the hook identity for spawning sub-agents. + /// + /// The serialized name remains `spawn_agent`, while `Agent` is accepted as + /// a matcher alias for compatibility with hook configurations that describe + /// sub-agent creation using Claude Code-style names. + pub(crate) fn spawn_agent() -> Self { + Self { + name: "spawn_agent".to_string(), + matcher_aliases: vec!["Agent".to_string()], + } + } + /// Returns the hook identity historically used for shell-like tools. pub(crate) fn bash() -> Self { Self::new("Bash") diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 411ce1435..a08d5c783 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -19,6 +19,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::flat_tool_name; +use crate::tools::handlers::multi_agents_spec::MULTI_AGENT_V1_NAMESPACE; use crate::tools::hook_names::HookToolName; use crate::tools::lifecycle::notify_tool_finish; use crate::tools::lifecycle::notify_tool_start; @@ -26,6 +27,7 @@ use crate::tools::tool_dispatch_trace::ToolDispatchTrace; use crate::tools::tool_search_entry::ToolSearchInfo; use crate::util::error_or_panic; use codex_extension_api::ToolCallOutcome; +use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::EventMsg; use codex_tools::ToolName; @@ -64,14 +66,47 @@ pub(crate) trait CoreToolRuntime: ToolExecutor { fn post_tool_use_payload( &self, - _invocation: &ToolInvocation, - _result: &dyn ToolOutput, + invocation: &ToolInvocation, + result: &dyn ToolOutput, ) -> Option { - None + let ToolPayload::Function { arguments } = &invocation.payload else { + return None; + }; + + Some(PostToolUsePayload { + tool_name: function_hook_tool_name(invocation), + tool_use_id: result.post_tool_use_id(&invocation.call_id), + tool_input: result + .post_tool_use_input(&invocation.payload) + .unwrap_or_else(|| function_hook_tool_input(arguments)), + tool_response: result + .post_tool_use_response(&invocation.call_id, &invocation.payload) + .or_else(|| { + // Most function tools can expose their model-facing output + // as the hook response. Outputs with a more stable hook + // contract should override post_tool_use_response above. + let ResponseInputItem::FunctionCallOutput { + output: FunctionCallOutputPayload { body, .. }, + .. + } = result.to_response_item(&invocation.call_id, &invocation.payload) + else { + return None; + }; + + serde_json::to_value(body).ok() + })?, + }) } - fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option { - None + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + let ToolPayload::Function { arguments } = &invocation.payload else { + return None; + }; + + Some(PreToolUsePayload { + tool_name: function_hook_tool_name(invocation), + tool_input: function_hook_tool_input(arguments), + }) } /// Rebuilds a tool invocation from hook-facing `tool_input`. @@ -80,12 +115,25 @@ pub(crate) trait CoreToolRuntime: ToolExecutor { /// hook contract they expose from `pre_tool_use_payload`. fn with_updated_hook_input( &self, - _invocation: ToolInvocation, - _updated_input: Value, + invocation: ToolInvocation, + updated_input: Value, ) -> Result { - Err(FunctionCallError::RespondToModel( - "tool does not support hook input rewriting".to_string(), - )) + let ToolPayload::Function { .. } = &invocation.payload else { + return Err(FunctionCallError::RespondToModel( + "hook input rewrite received unsupported function tool payload".to_string(), + )); + }; + + let arguments = serde_json::to_string(&updated_input).map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to serialize rewritten {} arguments: {err}", + flat_tool_name(&invocation.tool_name) + )) + })?; + Ok(ToolInvocation { + payload: ToolPayload::Function { arguments }, + ..invocation + }) } /// Creates an optional consumer for streamed tool argument diffs. @@ -133,6 +181,29 @@ impl AnyToolResult { } } +struct PostToolUseFeedbackOutput { + original: Box, + model_visible: FunctionToolOutput, +} + +impl ToolOutput for PostToolUseFeedbackOutput { + fn log_preview(&self) -> String { + self.original.log_preview() + } + + fn success_for_logging(&self) -> bool { + self.original.success_for_logging() + } + + fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem { + self.model_visible.to_response_item(call_id, payload) + } + + fn code_mode_result(&self, payload: &ToolPayload) -> Value { + self.original.code_mode_result(payload) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PreToolUsePayload { /// Hook-facing tool name model. @@ -538,11 +609,15 @@ impl ToolRegistry { }; if let Some(replacement_text) = replacement_text { let mut guard = response_cell.lock().await; - if let Some(result) = guard.as_mut() { - result.result = Box::new(FunctionToolOutput::from_text( - replacement_text, - /*success*/ None, - )); + if let Some(mut result) = guard.take() { + result.result = Box::new(PostToolUseFeedbackOutput { + original: result.result, + model_visible: FunctionToolOutput::from_text( + replacement_text, + /*success*/ None, + ), + }); + *guard = Some(result); } } } @@ -618,6 +693,27 @@ async fn handle_any_tool( }) } +fn function_hook_tool_name(invocation: &ToolInvocation) -> HookToolName { + if invocation.tool_name.name == "spawn_agent" + && matches!( + invocation.tool_name.namespace.as_deref(), + None | Some(MULTI_AGENT_V1_NAMESPACE) + ) + { + return HookToolName::spawn_agent(); + } + + HookToolName::new(flat_tool_name(&invocation.tool_name).into_owned()) +} + +fn function_hook_tool_input(arguments: &str) -> Value { + if arguments.trim().is_empty() { + return Value::Object(serde_json::Map::new()); + } + + serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string())) +} + fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String { match payload { ToolPayload::Custom { .. } => format!("unsupported custom tool call: {tool_name}"), diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 8517730d3..f2566251c 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -172,6 +172,198 @@ fn handler_looks_up_namespaced_aliases_explicitly() { ); } +#[tokio::test] +async fn function_tools_expose_default_hook_payloads_and_rewrites() -> anyhow::Result<()> { + let (session, turn) = crate::session::tests::make_session_and_context().await; + let tool_name = codex_tools::ToolName::namespaced("functions.", "echo"); + let handler = TestHandler { + tool_name: tool_name.clone(), + }; + let invocation = ToolInvocation { + payload: ToolPayload::Function { + arguments: serde_json::json!({ "message": "hello" }).to_string(), + }, + ..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name) + }; + let output = + crate::tools::context::FunctionToolOutput::from_text("echoed".to_string(), Some(true)); + + assert_eq!( + handler.pre_tool_use_payload(&invocation), + Some(PreToolUsePayload { + tool_name: HookToolName::new("functions.echo"), + tool_input: serde_json::json!({ "message": "hello" }), + }) + ); + assert_eq!( + handler.post_tool_use_payload(&invocation, &output), + Some(PostToolUsePayload { + tool_name: HookToolName::new("functions.echo"), + tool_use_id: "call-1".to_string(), + tool_input: serde_json::json!({ "message": "hello" }), + tool_response: serde_json::json!("echoed"), + }) + ); + + let invocation = handler + .with_updated_hook_input(invocation, serde_json::json!({ "message": "rewritten" }))?; + let ToolPayload::Function { arguments } = invocation.payload else { + panic!("generic rewritten function payload should remain function-shaped"); + }; + assert_eq!( + serde_json::from_str::(&arguments)?, + serde_json::json!({ "message": "rewritten" }) + ); + + Ok(()) +} + +#[tokio::test] +async fn function_hook_input_defaults_empty_arguments_to_object() { + let (session, turn) = crate::session::tests::make_session_and_context().await; + let tool_name = codex_tools::ToolName::plain("echo"); + let handler = TestHandler { + tool_name: tool_name.clone(), + }; + let invocation = ToolInvocation { + payload: ToolPayload::Function { + arguments: " ".to_string(), + }, + ..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name) + }; + + assert_eq!( + handler.pre_tool_use_payload(&invocation), + Some(PreToolUsePayload { + tool_name: HookToolName::new("echo"), + tool_input: serde_json::json!({}), + }) + ); +} + +#[tokio::test] +async fn spawn_agent_function_tools_use_agent_matcher_alias() { + let (session, turn) = crate::session::tests::make_session_and_context().await; + let session = Arc::new(session); + let turn = Arc::new(turn); + + let hook_payloads = [ + codex_tools::ToolName::plain("spawn_agent"), + codex_tools::ToolName::namespaced(MULTI_AGENT_V1_NAMESPACE, "spawn_agent"), + ] + .into_iter() + .map(|tool_name| { + let handler = TestHandler { + tool_name: tool_name.clone(), + }; + let invocation = ToolInvocation { + payload: ToolPayload::Function { + arguments: serde_json::json!({ "message": "inspect this repo" }).to_string(), + }, + ..test_invocation(Arc::clone(&session), Arc::clone(&turn), "call-1", tool_name) + }; + handler.pre_tool_use_payload(&invocation) + }) + .collect::>(); + + assert_eq!( + hook_payloads, + vec![ + Some(PreToolUsePayload { + tool_name: HookToolName::spawn_agent(), + tool_input: serde_json::json!({ "message": "inspect this repo" }), + }), + Some(PreToolUsePayload { + tool_name: HookToolName::spawn_agent(), + tool_input: serde_json::json!({ "message": "inspect this repo" }), + }), + ] + ); +} + +#[tokio::test] +async fn code_mode_wait_does_not_expose_default_hook_payloads() { + let (session, turn) = crate::session::tests::make_session_and_context().await; + let output = crate::tools::context::FunctionToolOutput::from_text("ok".to_string(), Some(true)); + + let wait = crate::tools::handlers::CodeModeWaitHandler; + let wait_invocation = test_invocation( + Arc::new(session), + Arc::new(turn), + "wait-call", + wait.tool_name(), + ); + assert_eq!(wait.pre_tool_use_payload(&wait_invocation), None); + assert_eq!(wait.post_tool_use_payload(&wait_invocation, &output), None); +} + +#[tokio::test] +async fn write_stdin_does_not_expose_default_pre_tool_use_payload() { + let (session, turn) = crate::session::tests::make_session_and_context().await; + + let write_stdin = crate::tools::handlers::WriteStdinHandler; + let invocation = test_invocation( + Arc::new(session), + Arc::new(turn), + "write-stdin-call", + write_stdin.tool_name(), + ); + + assert_eq!(write_stdin.pre_tool_use_payload(&invocation), None); +} + +#[test] +fn post_tool_use_feedback_output_keeps_code_mode_result_typed() { + let result = AnyToolResult { + call_id: "call-1".to_string(), + payload: ToolPayload::Function { + arguments: "{}".to_string(), + }, + result: Box::new(PostToolUseFeedbackOutput { + original: Box::new(codex_tools::JsonToolOutput::new( + serde_json::json!({ "typed": true }), + )), + model_visible: crate::tools::context::FunctionToolOutput::from_text( + "hook feedback".to_string(), + /*success*/ None, + ), + }), + post_tool_use_payload: None, + }; + + assert_eq!( + result.into_response(), + ResponseInputItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: codex_protocol::models::FunctionCallOutputPayload::from_text( + "hook feedback".to_string() + ), + } + ); + + let result = AnyToolResult { + call_id: "call-1".to_string(), + payload: ToolPayload::Function { + arguments: "{}".to_string(), + }, + result: Box::new(PostToolUseFeedbackOutput { + original: Box::new(codex_tools::JsonToolOutput::new( + serde_json::json!({ "typed": true }), + )), + model_visible: crate::tools::context::FunctionToolOutput::from_text( + "hook feedback".to_string(), + /*success*/ None, + ), + }), + post_tool_use_payload: None, + }; + + assert_eq!( + result.code_mode_result(), + serde_json::json!({ "typed": true }) + ); +} + #[tokio::test] async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> { let (mut session, turn) = crate::session::tests::make_session_and_context().await; diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index ee1826ceb..379174655 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -3463,42 +3463,35 @@ async fn pre_tool_use_blocks_apply_patch_with_write_alias() -> Result<()> { } #[tokio::test] -async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { +async fn pre_tool_use_blocks_local_function_tool_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let call_id = "pretooluse-update-plan"; - let args = serde_json::json!({ - "plan": [{ - "step": "watch the tide", - "status": "pending", - }] - }); + let call_id = "pretooluse-local-function-tool"; + let args = serde_json::json!({}); let responses = mount_sse_sequence( &server, vec![ sse(vec![ ev_response_created("resp-1"), - core_test_support::responses::ev_function_call( - call_id, - "update_plan", - &serde_json::to_string(&args)?, - ), + ev_function_call(call_id, "test_sync_tool", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), sse(vec![ ev_response_created("resp-2"), - ev_assistant_message("msg-1", "plan updated"), + ev_assistant_message("msg-1", "local function hook blocked it"), ev_completed("resp-2"), ]), ], ) .await; + let reason = "blocked local function pre hook"; let mut builder = test_codex() + .with_model("test-gpt-5.1-codex") .with_pre_build_hook(|home| { if let Err(error) = - write_pre_tool_use_hook(home, /*matcher*/ None, "json_deny", "should not fire") + write_pre_tool_use_hook(home, Some("^test_sync_tool$"), "json_deny", reason) { panic!("failed to write pre tool use hook test fixture: {error}"); } @@ -3506,7 +3499,8 @@ async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { .with_config(trust_discovered_hooks); let test = builder.build(&server).await?; - test.submit_turn("update the plan").await?; + test.submit_turn("call the local function tool with the pre hook") + .await?; let requests = responses.requests(); assert_eq!(requests.len(), 2); @@ -3514,17 +3508,85 @@ async fn pre_tool_use_does_not_fire_for_plan_tool() -> Result<()> { let output = output_item .get("output") .and_then(Value::as_str) - .expect("update plan output string"); + .expect("blocked local function tool output string"); assert!( - !output.contains("should not fire"), - "non-shell tool output should not be blocked by PreToolUse", + output.contains(&format!( + "Tool call blocked by PreToolUse hook: {reason}. Tool: test_sync_tool" + )), + "blocked local function output should surface the hook reason and tool name", ); - let hook_log_path = test.codex_home_path().join("pre_tool_use_hook_log.jsonl"); - assert!( - !hook_log_path.exists(), - "plan tool should not trigger pre tool use hooks", - ); + let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse"); + assert_eq!(hook_inputs[0]["tool_name"], "test_sync_tool"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"], args); + + Ok(()) +} + +#[tokio::test] +async fn pre_tool_use_rewrites_local_function_tool_before_execution() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "pretooluse-local-function-tool-rewrite"; + let original_args = serde_json::json!({ + "barrier": { + "id": "pretooluse-local-function-invalid-barrier", + "participants": 0, + } + }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + call_id, + "test_sync_tool", + &serde_json::to_string(&original_args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "local function hook rewrote it"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let updated_input = serde_json::json!({}); + let mut builder = test_codex() + .with_model("test-gpt-5.1-codex") + .with_pre_build_hook(move |home| { + if let Err(error) = + write_updating_pre_tool_use_hook(home, "^test_sync_tool$", &updated_input) + { + panic!("failed to write updating pre tool use hook test fixture: {error}"); + } + }) + .with_config(trust_discovered_hooks); + let test = builder.build(&server).await?; + + test.submit_turn("call the local function tool with the pre hook rewrite") + .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("rewritten local function tool output string"); + assert_eq!(output, "ok"); + + 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_input"], original_args); Ok(()) } @@ -4133,73 +4195,3 @@ async fn post_tool_use_records_apply_patch_context_with_edit_alias() -> Result<( Ok(()) } - -#[tokio::test] -async fn post_tool_use_does_not_fire_for_plan_tool() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let call_id = "posttooluse-update-plan"; - let args = serde_json::json!({ - "plan": [{ - "step": "watch the tide", - "status": "pending", - }] - }); - let responses = mount_sse_sequence( - &server, - vec![ - sse(vec![ - ev_response_created("resp-1"), - core_test_support::responses::ev_function_call( - call_id, - "update_plan", - &serde_json::to_string(&args)?, - ), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-1", "plan updated"), - ev_completed("resp-2"), - ]), - ], - ) - .await; - - let mut builder = test_codex() - .with_pre_build_hook(|home| { - if let Err(error) = write_post_tool_use_hook( - home, - /*matcher*/ None, - "decision_block", - "should not fire", - ) { - panic!("failed to write post tool use hook test fixture: {error}"); - } - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - test.submit_turn("update the plan").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("update plan output string"); - assert!( - !output.contains("should not fire"), - "non-shell tool output should not be affected by PostToolUse", - ); - - let hook_log_path = test.codex_home_path().join("post_tool_use_hook_log.jsonl"); - assert!( - !hook_log_path.exists(), - "plan tool should not trigger post tool use hooks", - ); - - Ok(()) -}