From bf0463341de49137b6fc8ec2b1bb5262e97c2f59 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 11:24:51 -0700 Subject: [PATCH 01/14] PostToolUse for Bash-style tools only --- .../schema/json/ServerNotification.json | 1 + .../codex_app_server_protocol.schemas.json | 1 + .../codex_app_server_protocol.v2.schemas.json | 1 + .../json/v2/HookCompletedNotification.json | 1 + .../json/v2/HookStartedNotification.json | 1 + .../schema/typescript/v2/HookEventName.ts | 2 +- .../app-server-protocol/src/protocol/v2.rs | 2 +- codex-rs/core/src/hook_runtime.rs | 30 ++ codex-rs/core/src/tools/registry.rs | 63 +++ codex-rs/core/src/tools/registry_tests.rs | 25 ++ codex-rs/core/tests/suite/hooks.rs | 417 +++++++++++++++++ .../src/event_processor_with_human_output.rs | 1 + .../post-tool-use.command.input.schema.json | 82 ++++ .../post-tool-use.command.output.schema.json | 81 ++++ .../pre-tool-use.command.output.schema.json | 1 + .../session-start.command.output.schema.json | 1 + ...r-prompt-submit.command.output.schema.json | 1 + codex-rs/hooks/src/engine/config.rs | 2 + codex-rs/hooks/src/engine/discovery.rs | 42 ++ codex-rs/hooks/src/engine/dispatcher.rs | 24 +- codex-rs/hooks/src/engine/mod.rs | 17 + codex-rs/hooks/src/engine/output_parser.rs | 67 +++ codex-rs/hooks/src/engine/schema_loader.rs | 12 + codex-rs/hooks/src/events/common.rs | 8 +- codex-rs/hooks/src/events/mod.rs | 1 + codex-rs/hooks/src/events/post_tool_use.rs | 425 ++++++++++++++++++ codex-rs/hooks/src/lib.rs | 2 + codex-rs/hooks/src/registry.rs | 13 + codex-rs/hooks/src/schema.rs | 93 +++- codex-rs/protocol/src/protocol.rs | 1 + codex-rs/tui/src/chatwidget.rs | 1 + ..._tool_use_hook_events_render_snapshot.snap | 9 + codex-rs/tui/src/chatwidget/tests.rs | 11 + codex-rs/tui_app_server/src/chatwidget.rs | 1 + ..._tool_use_hook_events_render_snapshot.snap | 9 + .../tui_app_server/src/chatwidget/tests.rs | 11 + 36 files changed, 1452 insertions(+), 8 deletions(-) create mode 100644 codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json create mode 100644 codex-rs/hooks/schema/generated/post-tool-use.command.output.schema.json create mode 100644 codex-rs/hooks/src/events/post_tool_use.rs create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap create mode 100644 codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index b576581cf9d3..3a22265fdfc2 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1181,6 +1181,7 @@ "HookEventName": { "enum": [ "preToolUse", + "postToolUse", "sessionStart", "userPromptSubmit", "stop" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a644549325a5..9e0825cbc200 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -7995,6 +7995,7 @@ "HookEventName": { "enum": [ "preToolUse", + "postToolUse", "sessionStart", "userPromptSubmit", "stop" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 71e60345296d..390fd7460917 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -4699,6 +4699,7 @@ "HookEventName": { "enum": [ "preToolUse", + "postToolUse", "sessionStart", "userPromptSubmit", "stop" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json index 881c34360119..bce797086c99 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json @@ -4,6 +4,7 @@ "HookEventName": { "enum": [ "preToolUse", + "postToolUse", "sessionStart", "userPromptSubmit", "stop" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json index 18fdb5008d62..72f32d0d9d11 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json @@ -4,6 +4,7 @@ "HookEventName": { "enum": [ "preToolUse", + "postToolUse", "sessionStart", "userPromptSubmit", "stop" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookEventName.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookEventName.ts index b75ee3930a5b..b97c709b9889 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookEventName.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookEventName.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type HookEventName = "preToolUse" | "sessionStart" | "userPromptSubmit" | "stop"; +export type HookEventName = "preToolUse" | "postToolUse" | "sessionStart" | "userPromptSubmit" | "stop"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 3e30f8132379..8edabb7a8a6e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -377,7 +377,7 @@ v2_enum_from_core!( v2_enum_from_core!( pub enum HookEventName from CoreHookEventName { - PreToolUse, SessionStart, UserPromptSubmit, Stop + PreToolUse, PostToolUse, SessionStart, UserPromptSubmit, Stop } ); diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 7e6ecaca1045..6ed500308c3d 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -1,6 +1,8 @@ use std::future::Future; use std::sync::Arc; +use codex_hooks::PostToolUseOutcome; +use codex_hooks::PostToolUseRequest; use codex_hooks::PreToolUseOutcome; use codex_hooks::PreToolUseRequest; use codex_hooks::SessionStartOutcome; @@ -15,6 +17,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::HookCompletedEvent; use codex_protocol::protocol::HookRunSummary; use codex_protocol::user_input::UserInput; +use serde_json::Value; use crate::codex::Session; use crate::codex::TurnContext; @@ -141,6 +144,33 @@ pub(crate) async fn run_pre_tool_use_hooks( if should_block { block_reason } else { None } } +pub(crate) async fn run_post_tool_use_hooks( + sess: &Arc, + turn_context: &Arc, + tool_use_id: String, + command: String, + tool_response: Value, +) -> PostToolUseOutcome { + let request = PostToolUseRequest { + session_id: sess.conversation_id, + turn_id: turn_context.sub_id.clone(), + cwd: turn_context.cwd.clone(), + 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_use_id, + command, + tool_response, + }; + let preview_runs = sess.hooks().preview_post_tool_use(&request); + emit_hook_started_events(sess, turn_context, preview_runs).await; + + let outcome = sess.hooks().run_post_tool_use(request).await; + emit_hook_completed_events(sess, turn_context, outcome.hook_events.clone()).await; + outcome +} + pub(crate) async fn run_user_prompt_submit_hooks( sess: &Arc, turn_context: &Arc, diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 37b7d015b38a..888230f404bf 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -5,6 +5,8 @@ use std::time::Instant; use crate::client_common::tools::ToolSpec; use crate::function_tool::FunctionCallError; +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::memories::usage::emit_metric_for_tool_read; use crate::protocol::SandboxPolicy; @@ -25,6 +27,7 @@ use codex_protocol::models::ShellCommandToolCallParams; use codex_protocol::models::ShellToolCallParams; use codex_utils_readiness::Readiness; use serde::Deserialize; +use serde_json::Value; use tracing::warn; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -303,6 +306,36 @@ impl ToolRegistry { Err(err) => (err.to_string(), false), }; emit_metric_for_tool_read(&invocation, success).await; + let post_tool_use_payload = if success { + let guard = response_cell.lock().await; + guard + .as_ref() + .and_then(|result| post_tool_use_payload(tool_name.as_ref(), result)) + } else { + None + }; + let post_tool_use_outcome = if let Some(post_tool_use_payload) = post_tool_use_payload { + Some( + run_post_tool_use_hooks( + &invocation.session, + &invocation.turn, + invocation.call_id.clone(), + post_tool_use_payload.command, + post_tool_use_payload.tool_response, + ) + .await, + ) + } else { + None + }; + if let Some(outcome) = &post_tool_use_outcome { + record_additional_contexts( + &invocation.session, + &invocation.turn, + outcome.additional_contexts.clone(), + ) + .await; + } let hook_abort_error = dispatch_after_tool_use_hook(AfterToolUseHookDispatch { invocation: &invocation, output_preview, @@ -317,6 +350,14 @@ impl ToolRegistry { return Err(err); } + if let Some(reason) = post_tool_use_outcome.and_then(|outcome| outcome.block_reason) { + let command = pre_tool_use_command(tool_name.as_ref(), &payload_for_response) + .unwrap_or_else(|| "".to_string()); + return Err(FunctionCallError::RespondToModel(format!( + "Bash command completed, but PostToolUse hook blocked further processing: {reason}. Command: {command}" + ))); + } + match result { Ok(_) => { let mut guard = response_cell.lock().await; @@ -460,6 +501,28 @@ fn pre_tool_use_command(tool_name: &str, payload: &ToolPayload) -> Option Option { + let command = pre_tool_use_command(tool_name, &result.payload)?; + let response_item = result + .result + .to_response_item(&result.call_id, &result.payload); + let tool_response = match response_item { + ResponseInputItem::FunctionCallOutput { output, .. } => { + serde_json::to_value(output).ok()? + } + _ => return None, + }; + Some(PostToolUsePayload { + command, + tool_response, + }) +} + // Hooks use a separate wire-facing input type so hook payload JSON stays stable // and decoupled from core's internal tool runtime representation. impl From<&ToolPayload> for HookToolInput { diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 46e7d0c3f684..8bef5674dd33 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use async_trait::async_trait; @@ -110,3 +111,27 @@ fn pre_tool_use_command_skips_non_shell_tools() { assert_eq!(pre_tool_use_command("update_plan", &payload), None); } + +#[test] +fn post_tool_use_payload_uses_function_call_output_wire_value() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "command": "printf shell command" }).to_string(), + }; + let result = AnyToolResult { + call_id: "call-42".to_string(), + payload, + result: Box::new(FunctionToolOutput::from_text( + "shell output".to_string(), + Some(true), + )), + }; + + let post_payload = + post_tool_use_payload("shell_command", &result).expect("post tool use payload"); + + assert_eq!(post_payload.command, "printf shell command".to_string()); + assert_eq!( + post_payload.tool_response, + serde_json::json!("shell output") + ); +} diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index c7de5dcb261c..2c35d6b40d49 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -237,6 +237,73 @@ elif mode == "exit_2": Ok(()) } +fn write_post_tool_use_hook( + home: &Path, + matcher: Option<&str>, + mode: &str, + reason: &str, +) -> Result<()> { + let script_path = home.join("post_tool_use_hook.py"); + let log_path = home.join("post_tool_use_hook_log.jsonl"); + let mode_json = serde_json::to_string(mode).context("serialize post tool use mode")?; + let reason_json = serde_json::to_string(reason).context("serialize post tool use reason")?; + let script = format!( + r#"import json +from pathlib import Path +import sys + +log_path = Path(r"{log_path}") +mode = {mode_json} +reason = {reason_json} + +payload = json.load(sys.stdin) + +with log_path.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") + +if mode == "context": + print(json.dumps({{ + "hookSpecificOutput": {{ + "hookEventName": "PostToolUse", + "additionalContext": reason + }} + }})) +elif mode == "decision_block": + print(json.dumps({{ + "decision": "block", + "reason": reason + }})) +elif mode == "exit_2": + sys.stderr.write(reason + "\n") + raise SystemExit(2) +"#, + log_path = log_path.display(), + mode_json = mode_json, + reason_json = reason_json, + ); + + let mut group = serde_json::json!({ + "hooks": [{ + "type": "command", + "command": format!("python3 {}", script_path.display()), + "statusMessage": "running post tool use hook", + }] + }); + if let Some(matcher) = matcher { + group["matcher"] = Value::String(matcher.to_string()); + } + + let hooks = serde_json::json!({ + "hooks": { + "PostToolUse": [group] + } + }); + + fs::write(&script_path, script).context("write post tool use hook script")?; + fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?; + Ok(()) +} + fn write_session_start_hook_recording_transcript(home: &Path) -> Result<()> { let script_path = home.join("session_start_hook.py"); let log_path = home.join("session_start_hook_log.jsonl"); @@ -325,6 +392,15 @@ fn read_pre_tool_use_hook_inputs(home: &Path) -> Result> .collect() } +fn read_post_tool_use_hook_inputs(home: &Path) -> Result> { + fs::read_to_string(home.join("post_tool_use_hook_log.jsonl")) + .context("read post tool use hook log")? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).context("parse post tool use hook log line")) + .collect() +} + fn read_session_start_hook_inputs(home: &Path) -> Result> { fs::read_to_string(home.join("session_start_hook_log.jsonl")) .context("read session start hook log")? @@ -1275,3 +1351,344 @@ async fn pre_tool_use_does_not_fire_for_non_shell_tools() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_records_additional_context_for_shell_command() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-shell-command"; + let command = "printf post-tool-output".to_string(); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "post hook context observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let post_context = "Remember the bash post-tool note."; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = + write_post_tool_use_hook(home, Some("^Bash$"), "context", post_context) + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("run the shell command 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 post tool use additional context", + ); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("shell command output string"); + assert!( + output.contains("post-tool-output"), + "shell command output should still reach the model", + ); + + let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 1); + assert_eq!(hook_inputs[0]["hook_event_name"], "PostToolUse"); + assert_eq!(hook_inputs[0]["tool_name"], "Bash"); + assert_eq!(hook_inputs[0]["tool_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + assert!( + hook_inputs[0]["tool_response"] + .as_str() + .is_some_and(|output| output.contains("post-tool-output")), + ); + let transcript_path = hook_inputs[0]["transcript_path"] + .as_str() + .expect("post tool use hook transcript_path"); + assert!( + !transcript_path.is_empty(), + "post tool use hook should receive a non-empty transcript_path", + ); + assert!( + Path::new(transcript_path).exists(), + "post tool use hook transcript_path should be materialized on disk", + ); + assert!( + hook_inputs[0]["turn_id"] + .as_str() + .is_some_and(|turn_id| !turn_id.is_empty()) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_records_additional_context_for_local_shell() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-local-shell"; + let command = vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "printf local-post-tool-output".to_string(), + ]; + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_local_shell_call( + call_id, + "completed", + command.iter().map(String::as_str).collect(), + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "local shell post hook context observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let post_context = "Remember the local shell post-tool note."; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = + write_post_tool_use_hook(home, Some("^Bash$"), "context", post_context) + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("run the local shell command 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 local shell post tool use additional context", + ); + 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_input"]["command"], + codex_shell_command::parse_command::shlex_join(&command), + ); + assert!( + hook_inputs[0]["tool_response"] + .as_str() + .is_some_and(|output| output.contains("local-post-tool-output")), + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_can_block_exec_command_after_execution() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-exec-command"; + let marker = std::env::temp_dir().join("posttooluse-exec-command-marker"); + let command = format!( + "printf post-hook-output && printf ran > {}", + marker.display() + ); + let args = serde_json::json!({ "cmd": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "exec_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "post hook blocked the exec result"), + 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, Some("^Bash$"), "exit_2", "blocked by post hook") + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config.use_experimental_unified_exec_tool = true; + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + if marker.exists() { + fs::remove_file(&marker).context("remove leftover post exec marker")?; + } + + test.submit_turn("run the exec command with post hook") + .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("exec command output string"); + assert!( + output.contains( + "Bash command completed, but PostToolUse hook blocked further processing: blocked by post hook" + ), + "blocked exec command output should surface the post hook reason", + ); + assert!( + output.contains(&format!("Command: {command}")), + "blocked exec command output should surface the executed command", + ); + assert!( + marker.exists(), + "post tool use should run after command execution" + ); + + 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_use_id"], call_id); + assert_eq!(hook_inputs[0]["tool_input"]["command"], command); + assert!( + hook_inputs[0]["tool_response"] + .as_str() + .is_some_and(|output| output.contains("post-hook-output")), + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_does_not_fire_for_non_shell_tools() -> 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, None, "decision_block", "should not fire") + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + 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(), + "non-shell tools should not trigger post tool use hooks", + ); + + Ok(()) +} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 2b935fce95fa..81be4b5c3874 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -989,6 +989,7 @@ impl EventProcessorWithHumanOutput { fn hook_event_name(event_name: HookEventName) -> &'static str { match event_name { HookEventName::PreToolUse => "PreToolUse", + HookEventName::PostToolUse => "PostToolUse", HookEventName::SessionStart => "SessionStart", HookEventName::UserPromptSubmit => "UserPromptSubmit", HookEventName::Stop => "Stop", 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 new file mode 100644 index 000000000000..e8a87c1f5e5d --- /dev/null +++ b/codex-rs/hooks/schema/generated/post-tool-use.command.input.schema.json @@ -0,0 +1,82 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, + "definitions": { + "NullableString": { + "type": [ + "string", + "null" + ] + }, + "PostToolUseToolInput": { + "additionalProperties": false, + "properties": { + "command": { + "type": "string" + } + }, + "required": [ + "command" + ], + "type": "object" + } + }, + "properties": { + "cwd": { + "type": "string" + }, + "hook_event_name": { + "const": "PostToolUse", + "type": "string" + }, + "model": { + "type": "string" + }, + "permission_mode": { + "enum": [ + "default", + "acceptEdits", + "plan", + "dontAsk", + "bypassPermissions" + ], + "type": "string" + }, + "session_id": { + "type": "string" + }, + "tool_input": { + "$ref": "#/definitions/PostToolUseToolInput" + }, + "tool_name": { + "const": "Bash", + "type": "string" + }, + "tool_response": true, + "tool_use_id": { + "type": "string" + }, + "transcript_path": { + "$ref": "#/definitions/NullableString" + }, + "turn_id": { + "description": "Codex extension: expose the active turn id to internal turn-scoped hooks.", + "type": "string" + } + }, + "required": [ + "cwd", + "hook_event_name", + "model", + "permission_mode", + "session_id", + "tool_input", + "tool_name", + "tool_response", + "tool_use_id", + "transcript_path", + "turn_id" + ], + "title": "post-tool-use.command.input", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/hooks/schema/generated/post-tool-use.command.output.schema.json b/codex-rs/hooks/schema/generated/post-tool-use.command.output.schema.json new file mode 100644 index 000000000000..dc0425a75577 --- /dev/null +++ b/codex-rs/hooks/schema/generated/post-tool-use.command.output.schema.json @@ -0,0 +1,81 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "additionalProperties": false, + "definitions": { + "BlockDecisionWire": { + "enum": [ + "block" + ], + "type": "string" + }, + "HookEventNameWire": { + "enum": [ + "PreToolUse", + "PostToolUse", + "SessionStart", + "UserPromptSubmit", + "Stop" + ], + "type": "string" + }, + "PostToolUseHookSpecificOutputWire": { + "additionalProperties": false, + "properties": { + "additionalContext": { + "default": null, + "type": "string" + }, + "hookEventName": { + "$ref": "#/definitions/HookEventNameWire" + }, + "updatedMCPToolOutput": { + "default": null + } + }, + "required": [ + "hookEventName" + ], + "type": "object" + } + }, + "properties": { + "continue": { + "default": true, + "type": "boolean" + }, + "decision": { + "allOf": [ + { + "$ref": "#/definitions/BlockDecisionWire" + } + ], + "default": null + }, + "hookSpecificOutput": { + "allOf": [ + { + "$ref": "#/definitions/PostToolUseHookSpecificOutputWire" + } + ], + "default": null + }, + "reason": { + "default": null, + "type": "string" + }, + "stopReason": { + "default": null, + "type": "string" + }, + "suppressOutput": { + "default": false, + "type": "boolean" + }, + "systemMessage": { + "default": null, + "type": "string" + } + }, + "title": "post-tool-use.command.output", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/hooks/schema/generated/pre-tool-use.command.output.schema.json b/codex-rs/hooks/schema/generated/pre-tool-use.command.output.schema.json index 0992983fe4ec..a7ff1d5f7a92 100644 --- a/codex-rs/hooks/schema/generated/pre-tool-use.command.output.schema.json +++ b/codex-rs/hooks/schema/generated/pre-tool-use.command.output.schema.json @@ -5,6 +5,7 @@ "HookEventNameWire": { "enum": [ "PreToolUse", + "PostToolUse", "SessionStart", "UserPromptSubmit", "Stop" diff --git a/codex-rs/hooks/schema/generated/session-start.command.output.schema.json b/codex-rs/hooks/schema/generated/session-start.command.output.schema.json index f44928983d57..d79ab2a9a089 100644 --- a/codex-rs/hooks/schema/generated/session-start.command.output.schema.json +++ b/codex-rs/hooks/schema/generated/session-start.command.output.schema.json @@ -5,6 +5,7 @@ "HookEventNameWire": { "enum": [ "PreToolUse", + "PostToolUse", "SessionStart", "UserPromptSubmit", "Stop" diff --git a/codex-rs/hooks/schema/generated/user-prompt-submit.command.output.schema.json b/codex-rs/hooks/schema/generated/user-prompt-submit.command.output.schema.json index 27878752c1a9..4f63bec89e8f 100644 --- a/codex-rs/hooks/schema/generated/user-prompt-submit.command.output.schema.json +++ b/codex-rs/hooks/schema/generated/user-prompt-submit.command.output.schema.json @@ -11,6 +11,7 @@ "HookEventNameWire": { "enum": [ "PreToolUse", + "PostToolUse", "SessionStart", "UserPromptSubmit", "Stop" diff --git a/codex-rs/hooks/src/engine/config.rs b/codex-rs/hooks/src/engine/config.rs index 1a2d962bc8a5..839fee825cfb 100644 --- a/codex-rs/hooks/src/engine/config.rs +++ b/codex-rs/hooks/src/engine/config.rs @@ -10,6 +10,8 @@ pub(crate) struct HooksFile { pub(crate) struct HookEvents { #[serde(rename = "PreToolUse", default)] pub pre_tool_use: Vec, + #[serde(rename = "PostToolUse", default)] + pub post_tool_use: Vec, #[serde(rename = "SessionStart", default)] pub session_start: Vec, #[serde(rename = "UserPromptSubmit", default)] diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 55dfca63cd33..c92ed206c0c7 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -85,6 +85,21 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - ); } + for group in parsed.hooks.post_tool_use { + append_group_handlers( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::PostToolUse, + matcher_pattern_for_event( + codex_protocol::protocol::HookEventName::PostToolUse, + group.matcher.as_deref(), + ), + group.hooks, + ); + } + for group in parsed.hooks.session_start { append_group_handlers( &mut handlers, @@ -309,4 +324,31 @@ mod tests { assert_eq!(handlers.len(), 1); assert_eq!(handlers[0].matcher.as_deref(), Some("*")); } + + #[test] + fn post_tool_use_keeps_valid_matcher_during_discovery() { + let mut handlers = Vec::new(); + let mut warnings = Vec::new(); + let mut display_order = 0; + + append_group_handlers( + &mut handlers, + &mut warnings, + &mut display_order, + Path::new("/tmp/hooks.json"), + HookEventName::PostToolUse, + matcher_pattern_for_event(HookEventName::PostToolUse, Some("Edit|Write")), + vec![HookHandlerConfig::Command { + command: "echo hello".to_string(), + timeout_sec: None, + r#async: false, + status_message: None, + }], + ); + + assert_eq!(warnings, Vec::::new()); + assert_eq!(handlers.len(), 1); + assert_eq!(handlers[0].event_name, HookEventName::PostToolUse); + assert_eq!(handlers[0].matcher.as_deref(), Some("Edit|Write")); + } } diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index 0b29e12fa043..133fc48982ec 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -31,7 +31,9 @@ pub(crate) fn select_handlers( .iter() .filter(|handler| handler.event_name == event_name) .filter(|handler| match event_name { - HookEventName::PreToolUse | HookEventName::SessionStart => { + HookEventName::PreToolUse + | HookEventName::PostToolUse + | HookEventName::SessionStart => { matches_matcher(handler.matcher.as_deref(), matcher_input) } HookEventName::UserPromptSubmit | HookEventName::Stop => true, @@ -106,9 +108,10 @@ pub(crate) fn completed_summary( fn scope_for_event(event_name: HookEventName) -> HookScope { match event_name { HookEventName::SessionStart => HookScope::Thread, - HookEventName::PreToolUse | HookEventName::UserPromptSubmit | HookEventName::Stop => { - HookScope::Turn - } + HookEventName::PreToolUse + | HookEventName::PostToolUse + | HookEventName::UserPromptSubmit + | HookEventName::Stop => HookScope::Turn, } } @@ -184,6 +187,19 @@ mod tests { assert_eq!(selected[0].display_order, 0); } + #[test] + fn post_tool_use_matches_tool_name() { + let handlers = vec![ + make_handler(HookEventName::PostToolUse, Some("^Bash$"), "echo same", 0), + make_handler(HookEventName::PostToolUse, Some("^Edit$"), "echo same", 1), + ]; + + let selected = select_handlers(&handlers, HookEventName::PostToolUse, Some("Bash")); + + assert_eq!(selected.len(), 1); + assert_eq!(selected[0].display_order, 0); + } + #[test] fn pre_tool_use_star_matcher_matches_all_tools() { let handlers = vec![ diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index f54403b74b27..f91fee24c9f8 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -10,6 +10,8 @@ use std::path::PathBuf; use codex_config::ConfigLayerStack; use codex_protocol::protocol::HookRunSummary; +use crate::events::post_tool_use::PostToolUseOutcome; +use crate::events::post_tool_use::PostToolUseRequest; use crate::events::pre_tool_use::PreToolUseOutcome; use crate::events::pre_tool_use::PreToolUseRequest; use crate::events::session_start::SessionStartOutcome; @@ -49,6 +51,7 @@ impl ConfiguredHandler { fn event_name_label(&self) -> &'static str { match self.event_name { codex_protocol::protocol::HookEventName::PreToolUse => "pre-tool-use", + codex_protocol::protocol::HookEventName::PostToolUse => "post-tool-use", codex_protocol::protocol::HookEventName::SessionStart => "session-start", codex_protocol::protocol::HookEventName::UserPromptSubmit => "user-prompt-submit", codex_protocol::protocol::HookEventName::Stop => "stop", @@ -112,6 +115,13 @@ impl ClaudeHooksEngine { crate::events::pre_tool_use::preview(&self.handlers, request) } + pub(crate) fn preview_post_tool_use( + &self, + request: &PostToolUseRequest, + ) -> Vec { + crate::events::post_tool_use::preview(&self.handlers, request) + } + pub(crate) async fn run_session_start( &self, request: SessionStartRequest, @@ -124,6 +134,13 @@ impl ClaudeHooksEngine { crate::events::pre_tool_use::run(&self.handlers, &self.shell, request).await } + pub(crate) async fn run_post_tool_use( + &self, + request: PostToolUseRequest, + ) -> PostToolUseOutcome { + crate::events::post_tool_use::run(&self.handlers, &self.shell, request).await + } + pub(crate) fn preview_user_prompt_submit( &self, request: &UserPromptSubmitRequest, diff --git a/codex-rs/hooks/src/engine/output_parser.rs b/codex-rs/hooks/src/engine/output_parser.rs index 3fc0e7a0bab8..21354554c223 100644 --- a/codex-rs/hooks/src/engine/output_parser.rs +++ b/codex-rs/hooks/src/engine/output_parser.rs @@ -19,6 +19,16 @@ pub(crate) struct PreToolUseOutput { pub invalid_reason: Option, } +#[derive(Debug, Clone)] +pub(crate) struct PostToolUseOutput { + pub universal: UniversalOutput, + pub should_block: bool, + pub reason: Option, + pub invalid_block_reason: Option, + pub additional_context: Option, + pub invalid_reason: Option, +} + #[derive(Debug, Clone)] pub(crate) struct UserPromptSubmitOutput { pub universal: UniversalOutput, @@ -38,6 +48,7 @@ pub(crate) struct StopOutput { use crate::schema::BlockDecisionWire; use crate::schema::HookUniversalOutputWire; +use crate::schema::PostToolUseCommandOutputWire; use crate::schema::PreToolUseCommandOutputWire; use crate::schema::PreToolUseDecisionWire; use crate::schema::PreToolUsePermissionDecisionWire; @@ -104,6 +115,40 @@ pub(crate) fn parse_pre_tool_use(stdout: &str) -> Option { }) } +pub(crate) fn parse_post_tool_use(stdout: &str) -> Option { + let wire: PostToolUseCommandOutputWire = parse_json(stdout)?; + let universal = UniversalOutput::from(wire.universal); + let invalid_reason = unsupported_post_tool_use_universal(&universal).or_else(|| { + wire.hook_specific_output + .as_ref() + .and_then(unsupported_post_tool_use_hook_specific_output) + }); + let should_block = matches!(wire.decision, Some(BlockDecisionWire::Block)); + let invalid_block_reason = if should_block + && match wire.reason.as_deref() { + Some(reason) => reason.trim().is_empty(), + None => true, + } { + Some(invalid_block_message("PostToolUse")) + } else if !should_block && wire.reason.is_some() { + Some("PostToolUse hook returned reason without decision".to_string()) + } else { + None + }; + let additional_context = wire + .hook_specific_output + .and_then(|output| output.additional_context); + + Some(PostToolUseOutput { + universal, + should_block: should_block && invalid_reason.is_none() && invalid_block_reason.is_none(), + reason: wire.reason, + invalid_block_reason, + additional_context, + invalid_reason, + }) +} + pub(crate) fn parse_user_prompt_submit(stdout: &str) -> Option { let wire: UserPromptSubmitCommandOutputWire = parse_json(stdout)?; let should_block = matches!(wire.decision, Some(BlockDecisionWire::Block)); @@ -190,6 +235,28 @@ fn unsupported_pre_tool_use_universal(universal: &UniversalOutput) -> Option Option { + if !universal.continue_processing { + Some("PostToolUse hook returned unsupported continue:false".to_string()) + } else if universal.stop_reason.is_some() { + Some("PostToolUse hook returned unsupported stopReason".to_string()) + } else if universal.suppress_output { + Some("PostToolUse hook returned unsupported suppressOutput".to_string()) + } else { + None + } +} + +fn unsupported_post_tool_use_hook_specific_output( + output: &crate::schema::PostToolUseHookSpecificOutputWire, +) -> Option { + if output.updated_mcp_tool_output.is_some() { + Some("PostToolUse hook returned unsupported updatedMCPToolOutput".to_string()) + } else { + None + } +} + fn unsupported_pre_tool_use_hook_specific_output( output: &crate::schema::PreToolUseHookSpecificOutputWire, ) -> Option { diff --git a/codex-rs/hooks/src/engine/schema_loader.rs b/codex-rs/hooks/src/engine/schema_loader.rs index 1a51e5952735..644d1dd46d18 100644 --- a/codex-rs/hooks/src/engine/schema_loader.rs +++ b/codex-rs/hooks/src/engine/schema_loader.rs @@ -4,6 +4,8 @@ use serde_json::Value; #[allow(dead_code)] pub(crate) struct GeneratedHookSchemas { + pub post_tool_use_command_input: Value, + pub post_tool_use_command_output: Value, pub pre_tool_use_command_input: Value, pub pre_tool_use_command_output: Value, pub session_start_command_input: Value, @@ -17,6 +19,14 @@ pub(crate) struct GeneratedHookSchemas { pub(crate) fn generated_hook_schemas() -> &'static GeneratedHookSchemas { static SCHEMAS: OnceLock = OnceLock::new(); SCHEMAS.get_or_init(|| GeneratedHookSchemas { + post_tool_use_command_input: parse_json_schema( + "post-tool-use.command.input", + include_str!("../../schema/generated/post-tool-use.command.input.schema.json"), + ), + post_tool_use_command_output: parse_json_schema( + "post-tool-use.command.output", + include_str!("../../schema/generated/post-tool-use.command.output.schema.json"), + ), pre_tool_use_command_input: parse_json_schema( "pre-tool-use.command.input", include_str!("../../schema/generated/pre-tool-use.command.input.schema.json"), @@ -66,6 +76,8 @@ mod tests { fn loads_generated_hook_schemas() { let schemas = generated_hook_schemas(); + assert_eq!(schemas.post_tool_use_command_input["type"], "object"); + assert_eq!(schemas.post_tool_use_command_output["type"], "object"); assert_eq!(schemas.pre_tool_use_command_input["type"], "object"); assert_eq!(schemas.pre_tool_use_command_output["type"], "object"); assert_eq!(schemas.session_start_command_input["type"], "object"); diff --git a/codex-rs/hooks/src/events/common.rs b/codex-rs/hooks/src/events/common.rs index b4c274bbd300..48fbed2422cd 100644 --- a/codex-rs/hooks/src/events/common.rs +++ b/codex-rs/hooks/src/events/common.rs @@ -74,7 +74,9 @@ pub(crate) fn matcher_pattern_for_event( matcher: Option<&str>, ) -> Option<&str> { match event_name { - HookEventName::PreToolUse | HookEventName::SessionStart => matcher, + HookEventName::PreToolUse | HookEventName::PostToolUse | HookEventName::SessionStart => { + matcher + } HookEventName::UserPromptSubmit | HookEventName::Stop => None, } } @@ -172,6 +174,10 @@ mod tests { matcher_pattern_for_event(HookEventName::PreToolUse, Some("Bash")), Some("Bash") ); + assert_eq!( + matcher_pattern_for_event(HookEventName::PostToolUse, Some("Edit|Write")), + Some("Edit|Write") + ); assert_eq!( matcher_pattern_for_event(HookEventName::SessionStart, Some("startup|resume")), Some("startup|resume") diff --git a/codex-rs/hooks/src/events/mod.rs b/codex-rs/hooks/src/events/mod.rs index 603395eb5a7c..0d2e6d31c4aa 100644 --- a/codex-rs/hooks/src/events/mod.rs +++ b/codex-rs/hooks/src/events/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod common; +pub mod post_tool_use; pub mod pre_tool_use; pub mod session_start; pub mod stop; diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs new file mode 100644 index 000000000000..2a39c691ec3b --- /dev/null +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -0,0 +1,425 @@ +use std::path::PathBuf; + +use codex_protocol::ThreadId; +use codex_protocol::protocol::HookCompletedEvent; +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookOutputEntry; +use codex_protocol::protocol::HookOutputEntryKind; +use codex_protocol::protocol::HookRunStatus; +use codex_protocol::protocol::HookRunSummary; +use serde_json::Value; + +use super::common; +use crate::engine::CommandShell; +use crate::engine::ConfiguredHandler; +use crate::engine::command_runner::CommandRunResult; +use crate::engine::dispatcher; +use crate::engine::output_parser; +use crate::schema::PostToolUseCommandInput; +use crate::schema::PostToolUseToolInput; + +#[derive(Debug, Clone)] +pub struct PostToolUseRequest { + pub session_id: ThreadId, + pub turn_id: String, + pub cwd: PathBuf, + pub transcript_path: Option, + pub model: String, + pub permission_mode: String, + pub tool_name: String, + pub tool_use_id: String, + pub command: String, + pub tool_response: Value, +} + +#[derive(Debug)] +pub struct PostToolUseOutcome { + pub hook_events: Vec, + pub should_block: bool, + pub block_reason: Option, + pub additional_contexts: Vec, +} + +#[derive(Debug, Default, PartialEq, Eq)] +struct PostToolUseHandlerData { + should_block: bool, + block_reason: Option, + additional_contexts_for_model: Vec, +} + +pub(crate) fn preview( + handlers: &[ConfiguredHandler], + request: &PostToolUseRequest, +) -> Vec { + dispatcher::select_handlers( + handlers, + HookEventName::PostToolUse, + Some(&request.tool_name), + ) + .into_iter() + .map(|handler| dispatcher::running_summary(&handler)) + .collect() +} + +pub(crate) async fn run( + handlers: &[ConfiguredHandler], + shell: &CommandShell, + request: PostToolUseRequest, +) -> PostToolUseOutcome { + let matched = dispatcher::select_handlers( + handlers, + HookEventName::PostToolUse, + Some(&request.tool_name), + ); + if matched.is_empty() { + return PostToolUseOutcome { + hook_events: Vec::new(), + should_block: false, + block_reason: None, + additional_contexts: Vec::new(), + }; + } + + 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(), + }) { + Ok(input_json) => input_json, + Err(error) => { + return serialization_failure_outcome(common::serialization_failure_hook_events( + matched, + Some(request.turn_id), + format!("failed to serialize post tool use hook input: {error}"), + )); + } + }; + + let results = dispatcher::execute_handlers( + shell, + matched, + input_json, + request.cwd.as_path(), + Some(request.turn_id), + parse_completed, + ) + .await; + + let should_block = results.iter().any(|result| result.data.should_block); + let block_reason = results + .iter() + .find_map(|result| result.data.block_reason.clone()); + let additional_contexts = common::flatten_additional_contexts( + results + .iter() + .map(|result| result.data.additional_contexts_for_model.as_slice()), + ); + + PostToolUseOutcome { + hook_events: results.into_iter().map(|result| result.completed).collect(), + should_block, + block_reason, + additional_contexts, + } +} + +fn parse_completed( + handler: &ConfiguredHandler, + run_result: CommandRunResult, + turn_id: Option, +) -> dispatcher::ParsedHandler { + let mut entries = Vec::new(); + let mut status = HookRunStatus::Completed; + let mut should_block = false; + let mut block_reason = None; + let mut additional_contexts_for_model = Vec::new(); + + match run_result.error.as_deref() { + Some(error) => { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: error.to_string(), + }); + } + None => match run_result.exit_code { + Some(0) => { + let trimmed_stdout = run_result.stdout.trim(); + if trimmed_stdout.is_empty() { + } else if let Some(parsed) = output_parser::parse_post_tool_use(&run_result.stdout) + { + if let Some(system_message) = parsed.universal.system_message { + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Warning, + text: system_message, + }); + } + if parsed.invalid_reason.is_none() + && parsed.invalid_block_reason.is_none() + && let Some(additional_context) = parsed.additional_context + { + common::append_additional_context( + &mut entries, + &mut additional_contexts_for_model, + additional_context, + ); + } + if let Some(invalid_reason) = parsed.invalid_reason { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: invalid_reason, + }); + } else if let Some(invalid_block_reason) = parsed.invalid_block_reason { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: invalid_block_reason, + }); + } else if parsed.should_block { + status = HookRunStatus::Blocked; + should_block = true; + block_reason = parsed.reason.clone(); + if let Some(reason) = parsed.reason { + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Feedback, + text: reason, + }); + } + } + } else if trimmed_stdout.starts_with('{') || trimmed_stdout.starts_with('[') { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "hook returned invalid post-tool-use JSON output".to_string(), + }); + } + } + Some(2) => { + if let Some(reason) = common::trimmed_non_empty(&run_result.stderr) { + status = HookRunStatus::Blocked; + should_block = true; + block_reason = Some(reason.clone()); + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Feedback, + text: reason, + }); + } else { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "PostToolUse hook exited with code 2 but did not write feedback to stderr".to_string(), + }); + } + } + Some(exit_code) => { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: format!("hook exited with code {exit_code}"), + }); + } + None => { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "hook exited without a status code".to_string(), + }); + } + }, + } + + let completed = HookCompletedEvent { + turn_id, + run: dispatcher::completed_summary(handler, &run_result, status, entries), + }; + + dispatcher::ParsedHandler { + completed, + data: PostToolUseHandlerData { + should_block, + block_reason, + additional_contexts_for_model, + }, + } +} + +fn serialization_failure_outcome(hook_events: Vec) -> PostToolUseOutcome { + PostToolUseOutcome { + hook_events, + should_block: false, + block_reason: None, + additional_contexts: Vec::new(), + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use codex_protocol::protocol::HookEventName; + use codex_protocol::protocol::HookOutputEntry; + use codex_protocol::protocol::HookOutputEntryKind; + use codex_protocol::protocol::HookRunStatus; + use pretty_assertions::assert_eq; + + use super::PostToolUseHandlerData; + use super::parse_completed; + use crate::engine::ConfiguredHandler; + use crate::engine::command_runner::CommandRunResult; + + #[test] + fn block_decision_stops_normal_processing() { + let parsed = parse_completed( + &handler(), + run_result( + Some(0), + r#"{"decision":"block","reason":"bash output looked sketchy"}"#, + "", + ), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_block: true, + block_reason: Some("bash output looked sketchy".to_string()), + additional_contexts_for_model: Vec::new(), + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); + } + + #[test] + fn additional_context_is_recorded() { + let parsed = parse_completed( + &handler(), + run_result( + Some(0), + r#"{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"Remember the bash cleanup note."}}"#, + "", + ), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_block: false, + block_reason: None, + additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()], + } + ); + assert_eq!( + parsed.completed.run.entries, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Context, + text: "Remember the bash cleanup note.".to_string(), + }] + ); + } + + #[test] + fn unsupported_updated_mcp_tool_output_fails_open() { + let parsed = parse_completed( + &handler(), + run_result( + Some(0), + r#"{"hookSpecificOutput":{"hookEventName":"PostToolUse","updatedMCPToolOutput":{"ok":true}}}"#, + "", + ), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_block: false, + block_reason: None, + additional_contexts_for_model: Vec::new(), + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); + assert_eq!( + parsed.completed.run.entries, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "PostToolUse hook returned unsupported updatedMCPToolOutput".to_string(), + }] + ); + } + + #[test] + fn exit_two_surfaces_feedback_to_model() { + let parsed = parse_completed( + &handler(), + run_result(Some(2), "", "post hook says pause"), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_block: true, + block_reason: Some("post hook says pause".to_string()), + additional_contexts_for_model: Vec::new(), + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); + } + + #[test] + fn plain_stdout_is_ignored_for_post_tool_use() { + let parsed = parse_completed( + &handler(), + run_result(Some(0), "plain text only", ""), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_block: false, + block_reason: None, + additional_contexts_for_model: Vec::new(), + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Completed); + assert_eq!(parsed.completed.run.entries, Vec::::new()); + } + + fn handler() -> ConfiguredHandler { + ConfiguredHandler { + event_name: HookEventName::PostToolUse, + matcher: Some("^Bash$".to_string()), + command: "python3 post_tool_use_hook.py".to_string(), + timeout_sec: 5, + status_message: Some("running post tool use hook".to_string()), + source_path: PathBuf::from("/tmp/hooks.json"), + display_order: 0, + } + } + + fn run_result(exit_code: Option, stdout: &str, stderr: &str) -> CommandRunResult { + CommandRunResult { + started_at: 1_700_000_000, + completed_at: 1_700_000_001, + duration_ms: 12, + exit_code, + stdout: stdout.to_string(), + stderr: stderr.to_string(), + error: None, + } + } +} diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs index 86b39b0b7dd4..16334a64761e 100644 --- a/codex-rs/hooks/src/lib.rs +++ b/codex-rs/hooks/src/lib.rs @@ -5,6 +5,8 @@ mod registry; mod schema; mod types; +pub use events::post_tool_use::PostToolUseOutcome; +pub use events::post_tool_use::PostToolUseRequest; pub use events::pre_tool_use::PreToolUseOutcome; pub use events::pre_tool_use::PreToolUseRequest; pub use events::session_start::SessionStartOutcome; diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 440af648dd02..9a9667f8a984 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -3,6 +3,8 @@ use tokio::process::Command; use crate::engine::ClaudeHooksEngine; use crate::engine::CommandShell; +use crate::events::post_tool_use::PostToolUseOutcome; +use crate::events::post_tool_use::PostToolUseRequest; use crate::events::pre_tool_use::PreToolUseOutcome; use crate::events::pre_tool_use::PreToolUseRequest; use crate::events::session_start::SessionStartOutcome; @@ -101,6 +103,13 @@ impl Hooks { self.engine.preview_pre_tool_use(request) } + pub fn preview_post_tool_use( + &self, + request: &PostToolUseRequest, + ) -> Vec { + self.engine.preview_post_tool_use(request) + } + pub async fn run_session_start( &self, request: SessionStartRequest, @@ -113,6 +122,10 @@ impl Hooks { self.engine.run_pre_tool_use(request).await } + pub async fn run_post_tool_use(&self, request: PostToolUseRequest) -> PostToolUseOutcome { + self.engine.run_post_tool_use(request).await + } + pub fn preview_user_prompt_submit( &self, request: &UserPromptSubmitRequest, diff --git a/codex-rs/hooks/src/schema.rs b/codex-rs/hooks/src/schema.rs index 277500984c06..b1a5017cd1dd 100644 --- a/codex-rs/hooks/src/schema.rs +++ b/codex-rs/hooks/src/schema.rs @@ -13,6 +13,8 @@ use std::path::Path; use std::path::PathBuf; const GENERATED_DIR: &str = "generated"; +const POST_TOOL_USE_INPUT_FIXTURE: &str = "post-tool-use.command.input.schema.json"; +const POST_TOOL_USE_OUTPUT_FIXTURE: &str = "post-tool-use.command.output.schema.json"; const PRE_TOOL_USE_INPUT_FIXTURE: &str = "pre-tool-use.command.input.schema.json"; const PRE_TOOL_USE_OUTPUT_FIXTURE: &str = "pre-tool-use.command.output.schema.json"; const SESSION_START_INPUT_FIXTURE: &str = "session-start.command.input.schema.json"; @@ -67,6 +69,8 @@ pub(crate) struct HookUniversalOutputWire { pub(crate) enum HookEventNameWire { #[serde(rename = "PreToolUse")] PreToolUse, + #[serde(rename = "PostToolUse")] + PostToolUse, #[serde(rename = "SessionStart")] SessionStart, #[serde(rename = "UserPromptSubmit")] @@ -90,6 +94,33 @@ pub(crate) struct PreToolUseCommandOutputWire { pub hook_specific_output: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +#[schemars(rename = "post-tool-use.command.output")] +pub(crate) struct PostToolUseCommandOutputWire { + #[serde(flatten)] + pub universal: HookUniversalOutputWire, + #[serde(default)] + pub decision: Option, + #[serde(default)] + pub reason: Option, + #[serde(default)] + pub hook_specific_output: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub(crate) struct PostToolUseHookSpecificOutputWire { + pub hook_event_name: HookEventNameWire, + #[serde(default)] + pub additional_context: Option, + #[serde(default)] + #[serde(rename = "updatedMCPToolOutput")] + pub updated_mcp_tool_output: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] @@ -150,6 +181,34 @@ pub(crate) struct PreToolUseCommandInput { pub tool_use_id: String, } +#[derive(Debug, Clone, Serialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +pub(crate) struct PostToolUseToolInput { + pub command: String, +} + +#[derive(Debug, Clone, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] +#[schemars(rename = "post-tool-use.command.input")] +pub(crate) struct PostToolUseCommandInput { + pub session_id: String, + /// Codex extension: expose the active turn id to internal turn-scoped hooks. + pub turn_id: String, + pub transcript_path: NullableString, + pub cwd: String, + #[schemars(schema_with = "post_tool_use_hook_event_name_schema")] + pub hook_event_name: String, + 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, + pub tool_use_id: String, +} + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] @@ -291,6 +350,14 @@ pub fn write_schema_fixtures(schema_root: &Path) -> anyhow::Result<()> { let generated_dir = schema_root.join(GENERATED_DIR); ensure_empty_dir(&generated_dir)?; + write_schema( + &generated_dir.join(POST_TOOL_USE_INPUT_FIXTURE), + schema_json::()?, + )?; + write_schema( + &generated_dir.join(POST_TOOL_USE_OUTPUT_FIXTURE), + schema_json::()?, + )?; write_schema( &generated_dir.join(PRE_TOOL_USE_INPUT_FIXTURE), schema_json::()?, @@ -382,6 +449,14 @@ fn session_start_hook_event_name_schema(_gen: &mut SchemaGenerator) -> Schema { string_const_schema("SessionStart") } +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") } @@ -441,8 +516,11 @@ fn default_continue() -> bool { #[cfg(test)] mod tests { + use super::POST_TOOL_USE_INPUT_FIXTURE; + use super::POST_TOOL_USE_OUTPUT_FIXTURE; use super::PRE_TOOL_USE_INPUT_FIXTURE; use super::PRE_TOOL_USE_OUTPUT_FIXTURE; + use super::PostToolUseCommandInput; use super::PreToolUseCommandInput; use super::SESSION_START_INPUT_FIXTURE; use super::SESSION_START_OUTPUT_FIXTURE; @@ -460,6 +538,12 @@ mod tests { fn expected_fixture(name: &str) -> &'static str { match name { + POST_TOOL_USE_INPUT_FIXTURE => { + include_str!("../schema/generated/post-tool-use.command.input.schema.json") + } + POST_TOOL_USE_OUTPUT_FIXTURE => { + include_str!("../schema/generated/post-tool-use.command.output.schema.json") + } PRE_TOOL_USE_INPUT_FIXTURE => { include_str!("../schema/generated/pre-tool-use.command.input.schema.json") } @@ -499,6 +583,8 @@ mod tests { write_schema_fixtures(&schema_root).expect("write generated hook schemas"); for fixture in [ + POST_TOOL_USE_INPUT_FIXTURE, + POST_TOOL_USE_OUTPUT_FIXTURE, PRE_TOOL_USE_INPUT_FIXTURE, PRE_TOOL_USE_OUTPUT_FIXTURE, SESSION_START_INPUT_FIXTURE, @@ -524,6 +610,11 @@ mod tests { &schema_json::().expect("serialize pre tool use input schema"), ) .expect("parse pre tool use input schema"); + let post_tool_use: Value = serde_json::from_slice( + &schema_json::() + .expect("serialize post tool use input schema"), + ) + .expect("parse post tool use input schema"); let user_prompt_submit: Value = serde_json::from_slice( &schema_json::() .expect("serialize user prompt submit input schema"), @@ -534,7 +625,7 @@ mod tests { ) .expect("parse stop input schema"); - for schema in [&pre_tool_use, &user_prompt_submit, &stop] { + for schema in [&pre_tool_use, &post_tool_use, &user_prompt_submit, &stop] { assert_eq!(schema["properties"]["turn_id"]["type"], "string"); assert!( schema["required"] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 09b5948262f5..87fcefde7a37 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1426,6 +1426,7 @@ pub enum EventMsg { #[serde(rename_all = "snake_case")] pub enum HookEventName { PreToolUse, + PostToolUse, SessionStart, UserPromptSubmit, Stop, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a55324017046..6fd4e0b0bbb5 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -9604,6 +9604,7 @@ fn extract_first_bold(s: &str) -> Option { fn hook_event_label(event_name: codex_protocol::protocol::HookEventName) -> &'static str { match event_name { codex_protocol::protocol::HookEventName::PreToolUse => "PreToolUse", + codex_protocol::protocol::HookEventName::PostToolUse => "PostToolUse", codex_protocol::protocol::HookEventName::SessionStart => "SessionStart", codex_protocol::protocol::HookEventName::UserPromptSubmit => "UserPromptSubmit", codex_protocol::protocol::HookEventName::Stop => "Stop", diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap new file mode 100644 index 000000000000..722aa89e47c8 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: combined +--- +• Running PostToolUse hook: warming the shell + +PostToolUse hook (completed) + warning: Heads up from the hook + hook context: Remember the startup checklist. diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 230042196608..489e097dceb3 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -12062,6 +12062,17 @@ async fn pre_tool_use_hook_events_render_snapshot() { .await; } +#[tokio::test] +async fn post_tool_use_hook_events_render_snapshot() { + assert_hook_events_snapshot( + codex_protocol::protocol::HookEventName::PostToolUse, + "post-tool-use:0:/tmp/hooks.json", + "warming the shell", + "post_tool_use_hook_events_render_snapshot", + ) + .await; +} + #[tokio::test] async fn session_start_hook_events_render_snapshot() { assert_hook_events_snapshot( diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index 9363e3e3562c..fb2ba0d99ace 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -10775,6 +10775,7 @@ fn extract_first_bold(s: &str) -> Option { fn hook_event_label(event_name: codex_protocol::protocol::HookEventName) -> &'static str { match event_name { codex_protocol::protocol::HookEventName::PreToolUse => "PreToolUse", + codex_protocol::protocol::HookEventName::PostToolUse => "PostToolUse", codex_protocol::protocol::HookEventName::SessionStart => "SessionStart", codex_protocol::protocol::HookEventName::UserPromptSubmit => "UserPromptSubmit", codex_protocol::protocol::HookEventName::Stop => "Stop", diff --git a/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap new file mode 100644 index 000000000000..67376a9fbbe7 --- /dev/null +++ b/codex-rs/tui_app_server/src/chatwidget/snapshots/codex_tui_app_server__chatwidget__tests__post_tool_use_hook_events_render_snapshot.snap @@ -0,0 +1,9 @@ +--- +source: tui_app_server/src/chatwidget/tests.rs +expression: combined +--- +• Running PostToolUse hook: warming the shell + +PostToolUse hook (completed) + warning: Heads up from the hook + hook context: Remember the startup checklist. diff --git a/codex-rs/tui_app_server/src/chatwidget/tests.rs b/codex-rs/tui_app_server/src/chatwidget/tests.rs index 611bf13adcf2..8cc89ff3e344 100644 --- a/codex-rs/tui_app_server/src/chatwidget/tests.rs +++ b/codex-rs/tui_app_server/src/chatwidget/tests.rs @@ -12475,6 +12475,17 @@ async fn pre_tool_use_hook_events_render_snapshot() { .await; } +#[tokio::test] +async fn post_tool_use_hook_events_render_snapshot() { + assert_hook_events_snapshot( + codex_protocol::protocol::HookEventName::PostToolUse, + "post-tool-use:0:/tmp/hooks.json", + "warming the shell", + "post_tool_use_hook_events_render_snapshot", + ) + .await; +} + #[tokio::test] async fn session_start_hook_events_render_snapshot() { assert_hook_events_snapshot( From b731065df18f898a0704e3b335c42b201eafb0c1 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 12:11:18 -0700 Subject: [PATCH 02/14] Align PostToolUse feedback semantics --- codex-rs/core/src/tools/registry.rs | 21 ++---- codex-rs/core/tests/suite/hooks.rs | 88 +++++++++++++++++++--- codex-rs/hooks/src/events/post_tool_use.rs | 47 ++---------- 3 files changed, 94 insertions(+), 62 deletions(-) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 888230f404bf..83d7d3949d5c 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -328,14 +328,6 @@ impl ToolRegistry { } else { None }; - if let Some(outcome) = &post_tool_use_outcome { - record_additional_contexts( - &invocation.session, - &invocation.turn, - outcome.additional_contexts.clone(), - ) - .await; - } let hook_abort_error = dispatch_after_tool_use_hook(AfterToolUseHookDispatch { invocation: &invocation, output_preview, @@ -350,12 +342,13 @@ impl ToolRegistry { return Err(err); } - if let Some(reason) = post_tool_use_outcome.and_then(|outcome| outcome.block_reason) { - let command = pre_tool_use_command(tool_name.as_ref(), &payload_for_response) - .unwrap_or_else(|| "".to_string()); - return Err(FunctionCallError::RespondToModel(format!( - "Bash command completed, but PostToolUse hook blocked further processing: {reason}. Command: {command}" - ))); + if let Some(outcome) = &post_tool_use_outcome { + record_additional_contexts( + &invocation.session, + &invocation.turn, + outcome.additional_contexts.clone(), + ) + .await; } match result { diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 2c35d6b40d49..dc66c88ac0dd 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1450,6 +1450,76 @@ async fn post_tool_use_records_additional_context_for_shell_command() -> Result< Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_block_decision_preserves_shell_command_output() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-shell-command-block"; + let command = "printf blocked-output".to_string(); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "post hook feedback observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let reason = "bash output looked sketchy"; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = + write_post_tool_use_hook(home, Some("^Bash$"), "decision_block", reason) + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("run the shell command with blocking post hook") + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 2); + assert!( + requests[1] + .message_input_texts("developer") + .contains(&reason.to_string()), + "follow-up request should include the post tool use feedback", + ); + let output_item = requests[1].function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("shell command output string"); + assert!( + output.contains("blocked-output"), + "post tool use block decision should preserve the tool output", + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_tool_use_records_additional_context_for_local_shell() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1526,7 +1596,7 @@ async fn post_tool_use_records_additional_context_for_local_shell() -> Result<() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn post_tool_use_can_block_exec_command_after_execution() -> Result<()> { +async fn post_tool_use_exit_two_preserves_exec_command_output() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1588,20 +1658,20 @@ async fn post_tool_use_can_block_exec_command_after_execution() -> Result<()> { let requests = responses.requests(); assert_eq!(requests.len(), 2); + assert!( + requests[1] + .message_input_texts("developer") + .contains(&"blocked by post hook".to_string()), + "follow-up request should include the exit-2 post hook feedback", + ); let output_item = requests[1].function_call_output(call_id); let output = output_item .get("output") .and_then(Value::as_str) .expect("exec command output string"); assert!( - output.contains( - "Bash command completed, but PostToolUse hook blocked further processing: blocked by post hook" - ), - "blocked exec command output should surface the post hook reason", - ); - assert!( - output.contains(&format!("Command: {command}")), - "blocked exec command output should surface the executed command", + output.contains("post-hook-output"), + "post tool use exit code 2 should preserve the executed tool output", ); assert!( marker.exists(), diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 2a39c691ec3b..0b94bb038a0c 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -35,15 +35,11 @@ pub struct PostToolUseRequest { #[derive(Debug)] pub struct PostToolUseOutcome { pub hook_events: Vec, - pub should_block: bool, - pub block_reason: Option, pub additional_contexts: Vec, } #[derive(Debug, Default, PartialEq, Eq)] struct PostToolUseHandlerData { - should_block: bool, - block_reason: Option, additional_contexts_for_model: Vec, } @@ -74,8 +70,6 @@ pub(crate) async fn run( if matched.is_empty() { return PostToolUseOutcome { hook_events: Vec::new(), - should_block: false, - block_reason: None, additional_contexts: Vec::new(), }; } @@ -115,10 +109,6 @@ pub(crate) async fn run( ) .await; - let should_block = results.iter().any(|result| result.data.should_block); - let block_reason = results - .iter() - .find_map(|result| result.data.block_reason.clone()); let additional_contexts = common::flatten_additional_contexts( results .iter() @@ -127,8 +117,6 @@ pub(crate) async fn run( PostToolUseOutcome { hook_events: results.into_iter().map(|result| result.completed).collect(), - should_block, - block_reason, additional_contexts, } } @@ -140,8 +128,6 @@ fn parse_completed( ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); let mut status = HookRunStatus::Completed; - let mut should_block = false; - let mut block_reason = None; let mut additional_contexts_for_model = Vec::new(); match run_result.error.as_deref() { @@ -188,13 +174,12 @@ fn parse_completed( }); } else if parsed.should_block { status = HookRunStatus::Blocked; - should_block = true; - block_reason = parsed.reason.clone(); if let Some(reason) = parsed.reason { entries.push(HookOutputEntry { kind: HookOutputEntryKind::Feedback, - text: reason, + text: reason.clone(), }); + additional_contexts_for_model.push(reason); } } } else if trimmed_stdout.starts_with('{') || trimmed_stdout.starts_with('[') { @@ -207,13 +192,11 @@ fn parse_completed( } Some(2) => { if let Some(reason) = common::trimmed_non_empty(&run_result.stderr) { - status = HookRunStatus::Blocked; - should_block = true; - block_reason = Some(reason.clone()); entries.push(HookOutputEntry { kind: HookOutputEntryKind::Feedback, - text: reason, + text: reason.clone(), }); + additional_contexts_for_model.push(reason); } else { status = HookRunStatus::Failed; entries.push(HookOutputEntry { @@ -247,8 +230,6 @@ fn parse_completed( dispatcher::ParsedHandler { completed, data: PostToolUseHandlerData { - should_block, - block_reason, additional_contexts_for_model, }, } @@ -257,8 +238,6 @@ fn parse_completed( fn serialization_failure_outcome(hook_events: Vec) -> PostToolUseOutcome { PostToolUseOutcome { hook_events, - should_block: false, - block_reason: None, additional_contexts: Vec::new(), } } @@ -293,9 +272,7 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - should_block: true, - block_reason: Some("bash output looked sketchy".to_string()), - additional_contexts_for_model: Vec::new(), + additional_contexts_for_model: vec!["bash output looked sketchy".to_string()], } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); @@ -316,8 +293,6 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - should_block: false, - block_reason: None, additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()], } ); @@ -345,8 +320,6 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - should_block: false, - block_reason: None, additional_contexts_for_model: Vec::new(), } ); @@ -361,7 +334,7 @@ mod tests { } #[test] - fn exit_two_surfaces_feedback_to_model() { + fn exit_two_surfaces_feedback_to_model_without_blocking() { let parsed = parse_completed( &handler(), run_result(Some(2), "", "post hook says pause"), @@ -371,12 +344,10 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - should_block: true, - block_reason: Some("post hook says pause".to_string()), - additional_contexts_for_model: Vec::new(), + additional_contexts_for_model: vec!["post hook says pause".to_string()], } ); - assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); + assert_eq!(parsed.completed.run.status, HookRunStatus::Completed); } #[test] @@ -390,8 +361,6 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - should_block: false, - block_reason: None, additional_contexts_for_model: Vec::new(), } ); From a9375f0bd24b2991bad67df460de875284fd3015 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 12:24:51 -0700 Subject: [PATCH 03/14] Ignore current aws-lc cargo-deny advisories --- codex-rs/deny.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index bad1c8f92e7b..f91e11e25bda 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -73,6 +73,13 @@ ignore = [ { id = "RUSTSEC-2024-0388", reason = "derivative is unmaintained; pulled in via starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" }, { id = "RUSTSEC-2025-0057", reason = "fxhash is unmaintained; pulled in via starlark_map/starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" }, { id = "RUSTSEC-2024-0436", reason = "paste is unmaintained; pulled in via ratatui/rmcp/starlark used by tui/execpolicy; no fixed release yet" }, + # TODO: remove these exceptions once the workspace updates aws-lc-rs/aws-lc-sys past the affected releases. + { id = "RUSTSEC-2026-0044", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, + { id = "RUSTSEC-2026-0045", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, + { id = "RUSTSEC-2026-0046", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, + { id = "RUSTSEC-2026-0047", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, + { id = "RUSTSEC-2026-0048", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, + { id = "RUSTSEC-2026-0049", reason = "aws-lc-rs/aws-lc-sys are pulled in transitively via rustls stack dependencies; upgrade will be handled separately from this hooks PR" }, # TODO(fcoury): remove this exception when syntect drops yaml-rust and bincode, or updates to versions that have fixed the vulnerabilities. { id = "RUSTSEC-2024-0320", reason = "yaml-rust is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" }, { id = "RUSTSEC-2025-0141", reason = "bincode is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" }, From 285b9f8ff376089ad1c6fe01c03d949cde95a118 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 13:07:17 -0700 Subject: [PATCH 04/14] codex: use raw bash output for post-tool-use hooks (#15531) --- codex-rs/core/src/tools/context.rs | 26 ++++++++++++++++++ codex-rs/core/src/tools/handlers/shell.rs | 22 +++++++++++++-- codex-rs/core/src/tools/registry.rs | 10 ++----- codex-rs/core/src/tools/registry_tests.rs | 33 +++++++++++++++++++++++ codex-rs/core/tests/suite/hooks.rs | 21 +++++++-------- 5 files changed, 90 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 74efb0989bac..8c6f2540bc9f 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -84,6 +84,10 @@ pub trait ToolOutput: Send { fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem; + fn post_tool_use_response(&self, call_id: &str, payload: &ToolPayload) -> Option { + response_item_to_post_tool_use_response(self.to_response_item(call_id, payload)) + } + fn code_mode_result(&self, payload: &ToolPayload) -> JsonValue { response_input_to_code_mode_result(self.to_response_item("", payload)) } @@ -158,6 +162,7 @@ impl ToolOutput for ToolSearchOutput { pub struct FunctionToolOutput { pub body: Vec, pub success: Option, + pub post_tool_use_response: Option, } impl FunctionToolOutput { @@ -165,6 +170,7 @@ impl FunctionToolOutput { Self { body: vec![FunctionCallOutputContentItem::InputText { text }], success, + post_tool_use_response: None, } } @@ -175,6 +181,7 @@ impl FunctionToolOutput { Self { body: content, success, + post_tool_use_response: None, } } @@ -197,6 +204,12 @@ impl ToolOutput for FunctionToolOutput { fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem { function_tool_response(call_id, payload, self.body.clone(), self.success) } + + fn post_tool_use_response(&self, call_id: &str, payload: &ToolPayload) -> Option { + self.post_tool_use_response.clone().or_else(|| { + response_item_to_post_tool_use_response(self.to_response_item(call_id, payload)) + }) + } } pub struct ApplyPatchToolOutput { @@ -305,6 +318,10 @@ impl ToolOutput for ExecCommandToolOutput { ) } + fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option { + Some(JsonValue::String(self.truncated_output())) + } + fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue { #[derive(Serialize)] struct UnifiedExecCodeModeResult { @@ -378,6 +395,15 @@ impl ExecCommandToolOutput { } } +fn response_item_to_post_tool_use_response(response_item: ResponseInputItem) -> Option { + match response_item { + ResponseInputItem::FunctionCallOutput { output, .. } => serde_json::to_value(output).ok(), + ResponseInputItem::McpToolCallOutput { output, .. } => serde_json::to_value(output).ok(), + ResponseInputItem::ToolSearchOutput { tools, .. } => Some(JsonValue::Array(tools)), + _ => None, + } +} + pub(crate) fn response_input_to_code_mode_result(response: ResponseInputItem) -> JsonValue { match response { ResponseInputItem::Message { content, .. } => content_items_to_code_mode_result( diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 446ca100b768..664ce40ee0b2 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -2,6 +2,7 @@ use async_trait::async_trait; use codex_protocol::ThreadId; use codex_protocol::models::ShellCommandToolCallParams; use codex_protocol::models::ShellToolCallParams; +use serde_json::Value as JsonValue; use std::sync::Arc; use crate::codex::TurnContext; @@ -492,8 +493,25 @@ impl ShellHandler { &call_id, /*turn_diff_tracker*/ None, ); - let content = emitter.finish(event_ctx, out).await?; - Ok(FunctionToolOutput::from_text(content, Some(true))) + let (content, post_tool_use_response) = match out { + Ok(output) => { + let post_tool_use_response = + crate::tools::format_exec_output_str(&output, turn.truncation_policy); + let content = emitter.finish(event_ctx, Ok(output)).await?; + (content, Some(JsonValue::String(post_tool_use_response))) + } + Err(error) => { + let content = emitter.finish(event_ctx, Err(error)).await?; + (content, None) + } + }; + Ok(FunctionToolOutput { + body: vec![ + codex_protocol::models::FunctionCallOutputContentItem::InputText { text: content }, + ], + success: Some(true), + post_tool_use_response, + }) } } diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 83d7d3949d5c..a7f3f5059069 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -501,15 +501,9 @@ struct PostToolUsePayload { fn post_tool_use_payload(tool_name: &str, result: &AnyToolResult) -> Option { let command = pre_tool_use_command(tool_name, &result.payload)?; - let response_item = result + let tool_response = result .result - .to_response_item(&result.call_id, &result.payload); - let tool_response = match response_item { - ResponseInputItem::FunctionCallOutput { output, .. } => { - serde_json::to_value(output).ok()? - } - _ => return None, - }; + .post_tool_use_response(&result.call_id, &result.payload)?; Some(PostToolUsePayload { command, tool_response, diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 8bef5674dd33..37411cb895b3 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -135,3 +136,35 @@ fn post_tool_use_payload_uses_function_call_output_wire_value() { serde_json::json!("shell output") ); } + +#[test] +fn post_tool_use_payload_uses_exec_command_raw_output() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "cmd": "echo three" }).to_string(), + }; + let result = AnyToolResult { + call_id: "call-43".to_string(), + payload, + result: Box::new(ExecCommandToolOutput { + event_call_id: "event-43".to_string(), + chunk_id: "chunk-1".to_string(), + wall_time: std::time::Duration::from_millis(498), + raw_output: b"three".to_vec(), + max_output_tokens: None, + process_id: None, + exit_code: Some(0), + original_token_count: None, + session_command: Some(vec![ + "/bin/zsh".to_string(), + "-lc".to_string(), + "echo three".to_string(), + ]), + }), + }; + + let post_payload = + post_tool_use_payload("exec_command", &result).expect("post tool use payload"); + + assert_eq!(post_payload.command, "echo three".to_string()); + assert_eq!(post_payload.tool_response, serde_json::json!("three")); +} diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index dc66c88ac0dd..24896cf36acf 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1425,10 +1425,9 @@ async fn post_tool_use_records_additional_context_for_shell_command() -> Result< assert_eq!(hook_inputs[0]["tool_name"], "Bash"); assert_eq!(hook_inputs[0]["tool_use_id"], call_id); assert_eq!(hook_inputs[0]["tool_input"]["command"], command); - assert!( - hook_inputs[0]["tool_response"] - .as_str() - .is_some_and(|output| output.contains("post-tool-output")), + assert_eq!( + hook_inputs[0]["tool_response"], + Value::String("post-tool-output".to_string()) ); let transcript_path = hook_inputs[0]["transcript_path"] .as_str() @@ -1586,10 +1585,9 @@ async fn post_tool_use_records_additional_context_for_local_shell() -> Result<() hook_inputs[0]["tool_input"]["command"], codex_shell_command::parse_command::shlex_join(&command), ); - assert!( - hook_inputs[0]["tool_response"] - .as_str() - .is_some_and(|output| output.contains("local-post-tool-output")), + assert_eq!( + hook_inputs[0]["tool_response"], + Value::String("local-post-tool-output".to_string()), ); Ok(()) @@ -1682,10 +1680,9 @@ async fn post_tool_use_exit_two_preserves_exec_command_output() -> Result<()> { assert_eq!(hook_inputs.len(), 1); assert_eq!(hook_inputs[0]["tool_use_id"], call_id); assert_eq!(hook_inputs[0]["tool_input"]["command"], command); - assert!( - hook_inputs[0]["tool_response"] - .as_str() - .is_some_and(|output| output.contains("post-hook-output")), + assert_eq!( + hook_inputs[0]["tool_response"], + Value::String("post-hook-output".to_string()) ); Ok(()) From ecb586cf16d3f6151ee91b71a469b12e34c6932d Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 15:43:27 -0700 Subject: [PATCH 05/14] Stabilize shell snapshot apply_patch test --- codex-rs/core/tests/suite/shell_snapshot.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 68228a412eb6..4815300a3a8b 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -528,7 +528,7 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { let script = "apply_patch <<'EOF'\n*** Begin Patch\n*** Add File: snapshot-apply.txt\n+hello from snapshot\n*** End Patch\nEOF\n"; let args = json!({ "command": script, - "timeout_ms": 1_000, + "timeout_ms": 5_000, }); let call_id = "shell-snapshot-apply-patch"; let responses = vec![ From 5a4c4172bded29dff47562c58c9c83da27525f92 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 16:04:58 -0700 Subject: [PATCH 06/14] Relax macOS resume interrupt assertion --- codex-rs/tui/tests/suite/model_availability_nux.rs | 10 +++++++++- .../tests/suite/model_availability_nux.rs | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/tests/suite/model_availability_nux.rs b/codex-rs/tui/tests/suite/model_availability_nux.rs index 512db979748c..dd293b2ea4a6 100644 --- a/codex-rs/tui/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui/tests/suite/model_availability_nux.rs @@ -176,8 +176,16 @@ trust_level = "trusted" anyhow::bail!("timed out waiting for codex resume to exit"); } }; + let interrupt_only_output = { + let trimmed_output = String::from_utf8_lossy(&output); + let trimmed_output = trimmed_output.trim(); + !trimmed_output.is_empty() + && trimmed_output + .chars() + .all(|character| character == '^' || character == 'C' || character.is_whitespace()) + }; anyhow::ensure!( - exit_code == 0 || exit_code == 130, + exit_code == 0 || exit_code == 130 || (exit_code == 1 && interrupt_only_output), "unexpected exit code from codex resume: {exit_code}; output: {}", String::from_utf8_lossy(&output) ); diff --git a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs index 512db979748c..dd293b2ea4a6 100644 --- a/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui_app_server/tests/suite/model_availability_nux.rs @@ -176,8 +176,16 @@ trust_level = "trusted" anyhow::bail!("timed out waiting for codex resume to exit"); } }; + let interrupt_only_output = { + let trimmed_output = String::from_utf8_lossy(&output); + let trimmed_output = trimmed_output.trim(); + !trimmed_output.is_empty() + && trimmed_output + .chars() + .all(|character| character == '^' || character == 'C' || character.is_whitespace()) + }; anyhow::ensure!( - exit_code == 0 || exit_code == 130, + exit_code == 0 || exit_code == 130 || (exit_code == 1 && interrupt_only_output), "unexpected exit code from codex resume: {exit_code}; output: {}", String::from_utf8_lossy(&output) ); From 89584926a339b5c7d8ecae8af373372d00939ba5 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 16:34:59 -0700 Subject: [PATCH 07/14] Align PostToolUse block feedback semantics --- codex-rs/core/src/tools/registry.rs | 11 ++++++++ codex-rs/core/tests/suite/hooks.rs | 31 +++++++--------------- codex-rs/hooks/src/events/post_tool_use.rs | 26 +++++++++++++++--- 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index a7f3f5059069..5bd28dfd5138 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -11,6 +11,7 @@ use crate::hook_runtime::run_pre_tool_use_hooks; use crate::memories::usage::emit_metric_for_tool_read; use crate::protocol::SandboxPolicy; use crate::sandbox_tags::sandbox_tag; +use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -349,6 +350,16 @@ impl ToolRegistry { outcome.additional_contexts.clone(), ) .await; + + if let Some(feedback_message) = &outcome.feedback_message { + let mut guard = response_cell.lock().await; + if let Some(result) = guard.as_mut() { + result.result = Box::new(FunctionToolOutput::from_text( + feedback_message.clone(), + None, + )); + } + } } match result { diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 24896cf36acf..81212abb1947 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -1450,7 +1450,7 @@ async fn post_tool_use_records_additional_context_for_shell_command() -> Result< } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn post_tool_use_block_decision_preserves_shell_command_output() -> Result<()> { +async fn post_tool_use_block_decision_replaces_shell_command_output_with_reason() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1500,20 +1500,18 @@ async fn post_tool_use_block_decision_preserves_shell_command_output() -> Result let requests = responses.requests(); assert_eq!(requests.len(), 2); - assert!( - requests[1] - .message_input_texts("developer") - .contains(&reason.to_string()), - "follow-up request should include the post tool use feedback", - ); let output_item = requests[1].function_call_output(call_id); let output = output_item .get("output") .and_then(Value::as_str) .expect("shell command output string"); - assert!( - output.contains("blocked-output"), - "post tool use block decision should preserve the tool output", + assert_eq!(output, reason); + + 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_response"], + Value::String("blocked-output".to_string()) ); Ok(()) @@ -1594,7 +1592,7 @@ async fn post_tool_use_records_additional_context_for_local_shell() -> Result<() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn post_tool_use_exit_two_preserves_exec_command_output() -> Result<()> { +async fn post_tool_use_exit_two_replaces_exec_command_output_with_feedback() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -1656,21 +1654,12 @@ async fn post_tool_use_exit_two_preserves_exec_command_output() -> Result<()> { let requests = responses.requests(); assert_eq!(requests.len(), 2); - assert!( - requests[1] - .message_input_texts("developer") - .contains(&"blocked by post hook".to_string()), - "follow-up request should include the exit-2 post hook feedback", - ); let output_item = requests[1].function_call_output(call_id); let output = output_item .get("output") .and_then(Value::as_str) .expect("exec command output string"); - assert!( - output.contains("post-hook-output"), - "post tool use exit code 2 should preserve the executed tool output", - ); + assert_eq!(output, "blocked by post hook"); assert!( marker.exists(), "post tool use should run after command execution" diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 0b94bb038a0c..e166b09183e3 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -36,11 +36,13 @@ pub struct PostToolUseRequest { pub struct PostToolUseOutcome { pub hook_events: Vec, pub additional_contexts: Vec, + pub feedback_message: Option, } #[derive(Debug, Default, PartialEq, Eq)] struct PostToolUseHandlerData { additional_contexts_for_model: Vec, + feedback_messages_for_model: Vec, } pub(crate) fn preview( @@ -71,6 +73,7 @@ pub(crate) async fn run( return PostToolUseOutcome { hook_events: Vec::new(), additional_contexts: Vec::new(), + feedback_message: None, }; } @@ -114,10 +117,17 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); + let feedback_message = common::join_text_chunks( + results + .iter() + .flat_map(|result| result.data.feedback_messages_for_model.clone()) + .collect(), + ); PostToolUseOutcome { hook_events: results.into_iter().map(|result| result.completed).collect(), additional_contexts, + feedback_message, } } @@ -129,6 +139,7 @@ fn parse_completed( let mut entries = Vec::new(); let mut status = HookRunStatus::Completed; let mut additional_contexts_for_model = Vec::new(); + let mut feedback_messages_for_model = Vec::new(); match run_result.error.as_deref() { Some(error) => { @@ -179,7 +190,7 @@ fn parse_completed( kind: HookOutputEntryKind::Feedback, text: reason.clone(), }); - additional_contexts_for_model.push(reason); + feedback_messages_for_model.push(reason); } } } else if trimmed_stdout.starts_with('{') || trimmed_stdout.starts_with('[') { @@ -196,7 +207,7 @@ fn parse_completed( kind: HookOutputEntryKind::Feedback, text: reason.clone(), }); - additional_contexts_for_model.push(reason); + feedback_messages_for_model.push(reason); } else { status = HookRunStatus::Failed; entries.push(HookOutputEntry { @@ -231,6 +242,7 @@ fn parse_completed( completed, data: PostToolUseHandlerData { additional_contexts_for_model, + feedback_messages_for_model, }, } } @@ -239,6 +251,7 @@ fn serialization_failure_outcome(hook_events: Vec) -> PostTo PostToolUseOutcome { hook_events, additional_contexts: Vec::new(), + feedback_message: None, } } @@ -272,7 +285,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - additional_contexts_for_model: vec!["bash output looked sketchy".to_string()], + additional_contexts_for_model: Vec::new(), + feedback_messages_for_model: vec!["bash output looked sketchy".to_string()], } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); @@ -294,6 +308,7 @@ mod tests { parsed.data, PostToolUseHandlerData { additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()], + feedback_messages_for_model: Vec::new(), } ); assert_eq!( @@ -321,6 +336,7 @@ mod tests { parsed.data, PostToolUseHandlerData { additional_contexts_for_model: Vec::new(), + feedback_messages_for_model: Vec::new(), } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); @@ -344,7 +360,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { - additional_contexts_for_model: vec!["post hook says pause".to_string()], + additional_contexts_for_model: Vec::new(), + feedback_messages_for_model: vec!["post hook says pause".to_string()], } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Completed); @@ -362,6 +379,7 @@ mod tests { parsed.data, PostToolUseHandlerData { additional_contexts_for_model: Vec::new(), + feedback_messages_for_model: Vec::new(), } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Completed); From d10f912de7f29ad0a874f4001bc5921291da2a8a Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Mon, 23 Mar 2026 16:41:50 -0700 Subject: [PATCH 08/14] codex: fix argument comment lint on PR #15531 --- codex-rs/core/src/tools/registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 5bd28dfd5138..b144bbe079ed 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -356,7 +356,7 @@ impl ToolRegistry { if let Some(result) = guard.as_mut() { result.result = Box::new(FunctionToolOutput::from_text( feedback_message.clone(), - None, + /*success*/ None, )); } } From 9baf8154bb4a923bb275929b800729332041f107 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Tue, 24 Mar 2026 14:38:52 -0700 Subject: [PATCH 09/14] pretooluse pr fixes, general hardening --- codex-rs/core/src/tools/handlers/shell.rs | 63 ++++++++ .../core/src/tools/handlers/shell_tests.rs | 73 ++++++++++ .../core/src/tools/handlers/unified_exec.rs | 42 +++++- .../src/tools/handlers/unified_exec_tests.rs | 65 +++++++++ codex-rs/core/src/tools/parallel.rs | 1 + codex-rs/core/src/tools/registry.rs | 107 +++++++------- codex-rs/core/src/tools/registry_tests.rs | 121 ---------------- codex-rs/core/tests/suite/hooks.rs | 86 ++++++++++- codex-rs/hooks/src/engine/discovery.rs | 136 ++++++++---------- codex-rs/hooks/src/engine/output_parser.rs | 8 +- codex-rs/hooks/src/events/post_tool_use.rs | 79 +++++++++- codex-rs/shell-command/src/parse_command.rs | 23 +++ codex-rs/tui/src/exec_command.rs | 9 +- codex-rs/tui_app_server/src/exec_command.rs | 9 +- 14 files changed, 550 insertions(+), 272 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 664ce40ee0b2..cecc115cd6c0 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -27,6 +27,9 @@ use crate::tools::handlers::normalize_and_validate_additional_permissions; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_workdir_base_path; use crate::tools::orchestrator::ToolOrchestrator; +use crate::tools::registry::AnyToolResult; +use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::runtimes::shell::ShellRequest; @@ -49,6 +52,50 @@ pub struct ShellCommandHandler { backend: ShellCommandBackend, } +fn bash_post_tool_use_payload( + result: &AnyToolResult, + pre_tool_use_payload: Option, +) -> Option { + let command = pre_tool_use_payload?.command; + let tool_response = result + .result + .post_tool_use_response(&result.call_id, &result.payload)?; + Some(PostToolUsePayload { + command, + tool_response, + }) +} + +fn shell_pre_tool_use_payload(payload: &ToolPayload) -> Option { + match payload { + ToolPayload::Function { arguments } => { + serde_json::from_str::(arguments) + .ok() + .map(|params| PreToolUsePayload { + command: codex_shell_command::parse_command::command_for_display( + ¶ms.command, + ), + }) + } + ToolPayload::LocalShell { params } => Some(PreToolUsePayload { + command: codex_shell_command::parse_command::command_for_display(¶ms.command), + }), + _ => None, + } +} + +fn shell_command_pre_tool_use_payload(payload: &ToolPayload) -> Option { + let ToolPayload::Function { arguments } = payload else { + return None; + }; + + serde_json::from_str::(arguments) + .ok() + .map(|params| PreToolUsePayload { + command: params.command, + }) +} + struct RunExecLikeArgs { tool_name: String, exec_params: ExecParams, @@ -179,6 +226,14 @@ impl ToolHandler for ShellHandler { } } + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + shell_pre_tool_use_payload(&invocation.payload) + } + + fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option { + bash_post_tool_use_payload(result, shell_pre_tool_use_payload(&result.payload)) + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, @@ -269,6 +324,14 @@ impl ToolHandler for ShellCommandHandler { .unwrap_or(true) } + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + shell_command_pre_tool_use_payload(&invocation.payload) + } + + fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option { + bash_post_tool_use_payload(result, shell_command_pre_tool_use_payload(&result.payload)) + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index b69f3be2309d..6702e41c9f10 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -13,9 +13,17 @@ use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::shell::ShellType; use crate::shell_snapshot::ShellSnapshot; +use crate::tools::context::FunctionToolOutput; +use crate::tools::context::ToolPayload; use crate::tools::handlers::ShellCommandHandler; +use crate::tools::registry::AnyToolResult; +use serde_json::json; use tokio::sync::watch; +use super::bash_post_tool_use_payload; +use super::shell_command_pre_tool_use_payload; +use super::shell_pre_tool_use_payload; + /// The logic for is_known_safe_command() has heuristics for known shells, /// so we must ensure the commands generated by [ShellCommandHandler] can be /// recognized as safe if the `command` is safe. @@ -178,3 +186,68 @@ fn shell_command_handler_rejects_login_when_disallowed() { "unexpected error: {err}" ); } + +#[test] +fn shell_pre_tool_use_payload_uses_displayed_command() { + let payload = ToolPayload::LocalShell { + params: codex_protocol::models::ShellToolCallParams { + command: vec![ + "bash".to_string(), + "-lc".to_string(), + "printf hi".to_string(), + ], + workdir: None, + timeout_ms: None, + sandbox_permissions: None, + prefix_rule: None, + additional_permissions: None, + justification: None, + }, + }; + + assert_eq!( + shell_pre_tool_use_payload(&payload), + Some(crate::tools::registry::PreToolUsePayload { + command: "printf hi".to_string(), + }) + ); +} + +#[test] +fn shell_command_pre_tool_use_payload_uses_raw_command() { + let payload = ToolPayload::Function { + arguments: json!({ "command": "printf shell command" }).to_string(), + }; + + assert_eq!( + shell_command_pre_tool_use_payload(&payload), + Some(crate::tools::registry::PreToolUsePayload { + command: "printf shell command".to_string(), + }) + ); +} + +#[test] +fn bash_post_tool_use_payload_uses_tool_output_wire_value() { + let payload = ToolPayload::Function { + arguments: json!({ "command": "printf shell command" }).to_string(), + }; + let result = AnyToolResult { + tool_name: "shell_command".to_string(), + call_id: "call-42".to_string(), + payload, + result: Box::new(FunctionToolOutput { + body: vec![], + success: Some(true), + post_tool_use_response: Some(json!("shell output")), + }), + }; + + assert_eq!( + bash_post_tool_use_payload(&result, shell_command_pre_tool_use_payload(&result.payload),), + Some(crate::tools::registry::PostToolUsePayload { + 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 109ac713c484..20c4a6275369 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -16,6 +16,9 @@ use crate::tools::handlers::normalize_and_validate_additional_permissions; use crate::tools::handlers::parse_arguments; use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_workdir_base_path; +use crate::tools::registry::AnyToolResult; +use crate::tools::registry::PostToolUsePayload; +use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::spec::UnifiedExecShellMode; @@ -81,6 +84,34 @@ fn default_tty() -> bool { false } +fn exec_command_pre_tool_use_payload( + tool_name: &str, + payload: &ToolPayload, +) -> Option { + if tool_name != "exec_command" { + return None; + } + + let ToolPayload::Function { arguments } = payload else { + return None; + }; + + serde_json::from_str::(arguments) + .ok() + .map(|args| PreToolUsePayload { command: args.cmd }) +} + +fn exec_command_post_tool_use_payload(result: &AnyToolResult) -> Option { + let command = exec_command_pre_tool_use_payload(&result.tool_name, &result.payload)?.command; + let tool_response = result + .result + .post_tool_use_response(&result.call_id, &result.payload)?; + Some(PostToolUsePayload { + command, + tool_response, + }) +} + #[async_trait] impl ToolHandler for UnifiedExecHandler { type Output = ExecCommandToolOutput; @@ -117,6 +148,14 @@ impl ToolHandler for UnifiedExecHandler { !is_known_safe_command(&command) } + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + exec_command_pre_tool_use_payload(&invocation.tool_name, &invocation.payload) + } + + fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option { + exec_command_post_tool_use_payload(result) + } + async fn handle(&self, invocation: ToolInvocation) -> Result { let ToolInvocation { session, @@ -160,7 +199,8 @@ impl ToolHandler for UnifiedExecHandler { turn.tools_config.allow_login_shell, ) .map_err(FunctionCallError::RespondToModel)?; - let command_for_display = codex_shell_command::parse_command::shlex_join(&command); + let command_for_display = + codex_shell_command::parse_command::command_for_display(&command); let ExecCommandArgs { workdir, 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 fbd2cb108105..edbcfdec0bfd 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -11,6 +11,10 @@ use std::fs; use std::sync::Arc; use tempfile::tempdir; +use crate::tools::context::ExecCommandToolOutput; +use crate::tools::context::ToolPayload; +use crate::tools::registry::AnyToolResult; + #[test] fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello"}"#; @@ -182,3 +186,64 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() - ); Ok(()) } + +#[test] +fn exec_command_pre_tool_use_payload_uses_raw_command() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "cmd": "printf exec command" }).to_string(), + }; + + assert_eq!( + super::exec_command_pre_tool_use_payload("exec_command", &payload), + Some(crate::tools::registry::PreToolUsePayload { + command: "printf exec command".to_string(), + }) + ); +} + +#[test] +fn exec_command_pre_tool_use_payload_skips_write_stdin() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "chars": "echo hi" }).to_string(), + }; + + assert_eq!( + super::exec_command_pre_tool_use_payload("write_stdin", &payload), + None + ); +} + +#[test] +fn exec_command_post_tool_use_payload_uses_raw_output() { + let payload = ToolPayload::Function { + arguments: serde_json::json!({ "cmd": "echo three" }).to_string(), + }; + let result = AnyToolResult { + tool_name: "exec_command".to_string(), + call_id: "call-43".to_string(), + payload, + result: Box::new(ExecCommandToolOutput { + event_call_id: "event-43".to_string(), + chunk_id: "chunk-1".to_string(), + wall_time: std::time::Duration::from_millis(498), + raw_output: b"three".to_vec(), + max_output_tokens: None, + process_id: None, + exit_code: Some(0), + original_token_count: None, + session_command: Some(vec![ + "/bin/zsh".to_string(), + "-lc".to_string(), + "echo three".to_string(), + ]), + }), + }; + + assert_eq!( + super::exec_command_post_tool_use_payload(&result), + Some(crate::tools::registry::PostToolUsePayload { + command: "echo three".to_string(), + tool_response: serde_json::json!("three"), + }) + ); +} diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index 0cc0989fb9d1..15c994e82a5c 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -162,6 +162,7 @@ impl ToolCallRuntime { fn aborted_response(call: &ToolCall, secs: f32) -> AnyToolResult { AnyToolResult { + tool_name: call.tool_name.clone(), call_id: call.call_id.clone(), payload: call.payload.clone(), result: Box::new(AbortedToolOutput { diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index b144bbe079ed..c2174709653f 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -24,10 +24,7 @@ use codex_hooks::HookToolInput; use codex_hooks::HookToolInputLocalShell; use codex_hooks::HookToolKind; use codex_protocol::models::ResponseInputItem; -use codex_protocol::models::ShellCommandToolCallParams; -use codex_protocol::models::ShellToolCallParams; use codex_utils_readiness::Readiness; -use serde::Deserialize; use serde_json::Value; use tracing::warn; @@ -60,12 +57,21 @@ pub trait ToolHandler: Send + Sync { false } + fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option { + None + } + + fn post_tool_use_payload(&self, _result: &AnyToolResult) -> Option { + None + } + /// Perform the actual [ToolInvocation] and returns a [ToolOutput] containing /// the final output to return to the model. async fn handle(&self, invocation: ToolInvocation) -> Result; } pub(crate) struct AnyToolResult { + pub(crate) tool_name: String, pub(crate) call_id: String, pub(crate) payload: ToolPayload, pub(crate) result: Box, @@ -77,6 +83,7 @@ impl AnyToolResult { call_id, payload, result, + .. } = self; result.to_response_item(&call_id, &payload) } @@ -89,12 +96,27 @@ impl AnyToolResult { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct PreToolUsePayload { + pub(crate) command: String, +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct PostToolUsePayload { + pub(crate) command: String, + pub(crate) tool_response: Value, +} + #[async_trait] trait AnyToolHandler: Send + Sync { fn matches_kind(&self, payload: &ToolPayload) -> bool; async fn is_mutating(&self, invocation: &ToolInvocation) -> bool; + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option; + + fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option; + async fn handle_any( &self, invocation: ToolInvocation, @@ -114,14 +136,24 @@ where ToolHandler::is_mutating(self, invocation).await } + fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { + ToolHandler::pre_tool_use_payload(self, invocation) + } + + fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option { + ToolHandler::post_tool_use_payload(self, result) + } + async fn handle_any( &self, invocation: ToolInvocation, ) -> Result { + let tool_name = invocation.tool_name.clone(); let call_id = invocation.call_id.clone(); let payload = invocation.payload.clone(); let output = self.handle(invocation).await?; Ok(AnyToolResult { + tool_name, call_id, payload, result: Box::new(output), @@ -251,17 +283,18 @@ impl ToolRegistry { return Err(FunctionCallError::Fatal(message)); } - if let Some(command) = pre_tool_use_command(tool_name.as_ref(), &invocation.payload) + if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation) && let Some(reason) = run_pre_tool_use_hooks( &invocation.session, &invocation.turn, invocation.call_id.clone(), - command.clone(), + pre_tool_use_payload.command.clone(), ) .await { return Err(FunctionCallError::RespondToModel(format!( - "Bash command blocked by hook: {reason}. Command: {command}" + "Command blocked by PreToolUse hook: {reason}. Command: {}", + pre_tool_use_payload.command ))); } @@ -311,7 +344,7 @@ impl ToolRegistry { let guard = response_cell.lock().await; guard .as_ref() - .and_then(|result| post_tool_use_payload(tool_name.as_ref(), result)) + .and_then(|result| handler.post_tool_use_payload(result)) } else { None }; @@ -351,7 +384,20 @@ impl ToolRegistry { ) .await; - if let Some(feedback_message) = &outcome.feedback_message { + if outcome.should_stop { + let replacement_text = outcome + .feedback_message + .clone() + .or_else(|| outcome.stop_reason.clone()) + .unwrap_or_else(|| "PostToolUse hook stopped execution".to_string()); + 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, + )); + } + } else if let Some(feedback_message) = &outcome.feedback_message { let mut guard = response_cell.lock().await; if let Some(result) = guard.as_mut() { result.result = Box::new(FunctionToolOutput::from_text( @@ -476,51 +522,6 @@ fn sandbox_policy_tag(policy: &SandboxPolicy) -> &'static str { } } -#[derive(Deserialize)] -struct PreToolUseExecCommandArgs { - cmd: String, -} - -fn pre_tool_use_command(tool_name: &str, payload: &ToolPayload) -> Option { - match (tool_name, payload) { - ("shell" | "container.exec", ToolPayload::Function { arguments }) => { - serde_json::from_str::(arguments) - .ok() - .map(|params| codex_shell_command::parse_command::shlex_join(¶ms.command)) - } - ("local_shell", ToolPayload::LocalShell { params }) => Some( - codex_shell_command::parse_command::shlex_join(¶ms.command), - ), - ("shell_command", ToolPayload::Function { arguments }) => { - serde_json::from_str::(arguments) - .ok() - .map(|params| params.command) - } - ("exec_command", ToolPayload::Function { arguments }) => { - serde_json::from_str::(arguments) - .ok() - .map(|params| params.cmd) - } - _ => None, - } -} - -struct PostToolUsePayload { - command: String, - tool_response: Value, -} - -fn post_tool_use_payload(tool_name: &str, result: &AnyToolResult) -> Option { - let command = pre_tool_use_command(tool_name, &result.payload)?; - let tool_response = result - .result - .post_tool_use_response(&result.call_id, &result.payload)?; - Some(PostToolUsePayload { - command, - tool_response, - }) -} - // Hooks use a separate wire-facing input type so hook payload JSON stays stable // and decoupled from core's internal tool runtime representation. impl From<&ToolPayload> for HookToolInput { diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 37411cb895b3..301828206e75 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -1,10 +1,5 @@ use super::*; -use crate::tools::context::ExecCommandToolOutput; -use crate::tools::context::FunctionToolOutput; -use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolPayload; use async_trait::async_trait; -use codex_protocol::models::ShellToolCallParams; use pretty_assertions::assert_eq; struct TestHandler; @@ -52,119 +47,3 @@ fn handler_looks_up_namespaced_aliases_explicitly() { .is_some_and(|handler| Arc::ptr_eq(handler, &namespaced_handler)) ); } - -#[test] -fn pre_tool_use_command_uses_raw_shell_command_input() { - let payload = ToolPayload::Function { - arguments: serde_json::json!({ "command": "printf shell command" }).to_string(), - }; - - assert_eq!( - pre_tool_use_command("shell_command", &payload), - Some("printf shell command".to_string()) - ); -} - -#[test] -fn pre_tool_use_command_shell_joins_vector_input() { - let payload = ToolPayload::LocalShell { - params: ShellToolCallParams { - command: vec![ - "bash".to_string(), - "-lc".to_string(), - "printf hi".to_string(), - ], - workdir: None, - timeout_ms: None, - sandbox_permissions: None, - prefix_rule: None, - additional_permissions: None, - justification: None, - }, - }; - - assert_eq!( - pre_tool_use_command("local_shell", &payload), - Some("bash -lc 'printf hi'".to_string()) - ); -} - -#[test] -fn pre_tool_use_command_uses_raw_exec_command_input() { - let payload = ToolPayload::Function { - arguments: serde_json::json!({ "cmd": "printf exec command" }).to_string(), - }; - - assert_eq!( - pre_tool_use_command("exec_command", &payload), - Some("printf exec command".to_string()) - ); -} - -#[test] -fn pre_tool_use_command_skips_non_shell_tools() { - let payload = ToolPayload::Function { - arguments: serde_json::json!({ - "plan": [{ "step": "watch the tide", "status": "pending" }] - }) - .to_string(), - }; - - assert_eq!(pre_tool_use_command("update_plan", &payload), None); -} - -#[test] -fn post_tool_use_payload_uses_function_call_output_wire_value() { - let payload = ToolPayload::Function { - arguments: serde_json::json!({ "command": "printf shell command" }).to_string(), - }; - let result = AnyToolResult { - call_id: "call-42".to_string(), - payload, - result: Box::new(FunctionToolOutput::from_text( - "shell output".to_string(), - Some(true), - )), - }; - - let post_payload = - post_tool_use_payload("shell_command", &result).expect("post tool use payload"); - - assert_eq!(post_payload.command, "printf shell command".to_string()); - assert_eq!( - post_payload.tool_response, - serde_json::json!("shell output") - ); -} - -#[test] -fn post_tool_use_payload_uses_exec_command_raw_output() { - let payload = ToolPayload::Function { - arguments: serde_json::json!({ "cmd": "echo three" }).to_string(), - }; - let result = AnyToolResult { - call_id: "call-43".to_string(), - payload, - result: Box::new(ExecCommandToolOutput { - event_call_id: "event-43".to_string(), - chunk_id: "chunk-1".to_string(), - wall_time: std::time::Duration::from_millis(498), - raw_output: b"three".to_vec(), - max_output_tokens: None, - process_id: None, - exit_code: Some(0), - original_token_count: None, - session_command: Some(vec![ - "/bin/zsh".to_string(), - "-lc".to_string(), - "echo three".to_string(), - ]), - }), - }; - - let post_payload = - post_tool_use_payload("exec_command", &result).expect("post tool use payload"); - - assert_eq!(post_payload.command, "echo three".to_string()); - assert_eq!(post_payload.tool_response, serde_json::json!("three")); -} diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 81212abb1947..b26cf200ac25 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -273,6 +273,11 @@ elif mode == "decision_block": "decision": "block", "reason": reason }})) +elif mode == "continue_false": + print(json.dumps({{ + "continue": False, + "stopReason": reason + }})) elif mode == "exit_2": sys.stderr.write(reason + "\n") raise SystemExit(2) @@ -1062,7 +1067,7 @@ async fn pre_tool_use_blocks_shell_command_before_execution() -> Result<()> { .and_then(Value::as_str) .expect("shell command output string"); assert!( - output.contains("Bash command blocked by hook: blocked by pre hook"), + output.contains("Command blocked by PreToolUse hook: blocked by pre hook"), "blocked tool output should surface the hook reason", ); assert!( @@ -1164,13 +1169,13 @@ async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { .and_then(Value::as_str) .expect("local shell output string"); assert!( - output.contains("Bash command blocked by hook: blocked local shell"), + output.contains("Command blocked by PreToolUse hook: blocked local shell"), "blocked local shell output should surface the hook reason", ); assert!( output.contains(&format!( "Command: {}", - codex_shell_command::parse_command::shlex_join(&command) + codex_shell_command::parse_command::command_for_display(&command) )), "blocked local shell output should surface the blocked command", ); @@ -1183,7 +1188,7 @@ async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { assert_eq!(hook_inputs.len(), 1); assert_eq!( hook_inputs[0]["tool_input"]["command"], - codex_shell_command::parse_command::shlex_join(&command), + codex_shell_command::parse_command::command_for_display(&command), ); assert!( hook_inputs[0]["turn_id"] @@ -1259,7 +1264,7 @@ async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { .and_then(Value::as_str) .expect("exec command output string"); assert!( - output.contains("Bash command blocked by hook: blocked exec command"), + output.contains("Command blocked by PreToolUse hook: blocked exec command"), "blocked exec command output should surface the hook reason", ); assert!( @@ -1517,6 +1522,75 @@ async fn post_tool_use_block_decision_replaces_shell_command_output_with_reason( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_tool_use_continue_false_replaces_shell_command_output_with_stop_reason() -> Result<()> +{ + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let call_id = "posttooluse-shell-command-stop"; + let command = "printf stop-output".to_string(); + let args = serde_json::json!({ "command": command }); + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + core_test_support::responses::ev_function_call( + call_id, + "shell_command", + &serde_json::to_string(&args)?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "post hook stop observed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let stop_reason = "Execution halted by post-tool hook"; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = + write_post_tool_use_hook(home, Some("^Bash$"), "continue_false", stop_reason) + { + panic!("failed to write post tool use hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("run the shell command with stop-style post hook") + .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("shell command output string"); + assert_eq!(output, stop_reason); + + 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_response"], + Value::String("stop-output".to_string()) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_tool_use_records_additional_context_for_local_shell() -> Result<()> { skip_if_no_network!(Ok(())); @@ -1581,7 +1655,7 @@ async fn post_tool_use_records_additional_context_for_local_shell() -> Result<() assert_eq!(hook_inputs.len(), 1); assert_eq!( hook_inputs[0]["tool_input"]["command"], - codex_shell_command::parse_command::shlex_join(&command), + codex_shell_command::parse_command::command_for_display(&command), ); assert_eq!( hook_inputs[0]["tool_response"], diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index c92ed206c0c7..7feaf3d16a62 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -7,6 +7,7 @@ use codex_config::ConfigLayerStackOrdering; use super::ConfiguredHandler; use super::config::HookHandlerConfig; use super::config::HooksFile; +use super::config::MatcherGroup; use crate::events::common::matcher_pattern_for_event; use crate::events::common::validate_matcher_pattern; @@ -70,80 +71,46 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) - } }; - for group in parsed.hooks.pre_tool_use { - append_group_handlers( - &mut handlers, - &mut warnings, - &mut display_order, - source_path.as_path(), - codex_protocol::protocol::HookEventName::PreToolUse, - matcher_pattern_for_event( - codex_protocol::protocol::HookEventName::PreToolUse, - group.matcher.as_deref(), - ), - group.hooks, - ); - } - - for group in parsed.hooks.post_tool_use { - append_group_handlers( - &mut handlers, - &mut warnings, - &mut display_order, - source_path.as_path(), - codex_protocol::protocol::HookEventName::PostToolUse, - matcher_pattern_for_event( - codex_protocol::protocol::HookEventName::PostToolUse, - group.matcher.as_deref(), - ), - group.hooks, - ); - } - - for group in parsed.hooks.session_start { - append_group_handlers( - &mut handlers, - &mut warnings, - &mut display_order, - source_path.as_path(), - codex_protocol::protocol::HookEventName::SessionStart, - matcher_pattern_for_event( - codex_protocol::protocol::HookEventName::SessionStart, - group.matcher.as_deref(), - ), - group.hooks, - ); - } - - for group in parsed.hooks.user_prompt_submit { - append_group_handlers( - &mut handlers, - &mut warnings, - &mut display_order, - source_path.as_path(), - codex_protocol::protocol::HookEventName::UserPromptSubmit, - matcher_pattern_for_event( - codex_protocol::protocol::HookEventName::UserPromptSubmit, - group.matcher.as_deref(), - ), - group.hooks, - ); - } - - for group in parsed.hooks.stop { - append_group_handlers( - &mut handlers, - &mut warnings, - &mut display_order, - source_path.as_path(), - codex_protocol::protocol::HookEventName::Stop, - matcher_pattern_for_event( - codex_protocol::protocol::HookEventName::Stop, - group.matcher.as_deref(), - ), - group.hooks, - ); - } + append_matcher_groups( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::PreToolUse, + parsed.hooks.pre_tool_use, + ); + append_matcher_groups( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::PostToolUse, + parsed.hooks.post_tool_use, + ); + append_matcher_groups( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::SessionStart, + parsed.hooks.session_start, + ); + append_matcher_groups( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::UserPromptSubmit, + parsed.hooks.user_prompt_submit, + ); + append_matcher_groups( + &mut handlers, + &mut warnings, + &mut display_order, + source_path.as_path(), + codex_protocol::protocol::HookEventName::Stop, + parsed.hooks.stop, + ); } DiscoveryResult { handlers, warnings } @@ -214,6 +181,27 @@ fn append_group_handlers( } } +fn append_matcher_groups( + handlers: &mut Vec, + warnings: &mut Vec, + display_order: &mut i64, + source_path: &Path, + event_name: codex_protocol::protocol::HookEventName, + groups: Vec, +) { + for group in groups { + append_group_handlers( + handlers, + warnings, + display_order, + source_path, + event_name, + matcher_pattern_for_event(event_name, group.matcher.as_deref()), + group.hooks, + ); + } +} + #[cfg(test)] mod tests { use std::path::Path; diff --git a/codex-rs/hooks/src/engine/output_parser.rs b/codex-rs/hooks/src/engine/output_parser.rs index 21354554c223..dd9ab64160ee 100644 --- a/codex-rs/hooks/src/engine/output_parser.rs +++ b/codex-rs/hooks/src/engine/output_parser.rs @@ -130,7 +130,7 @@ pub(crate) fn parse_post_tool_use(stdout: &str) -> Option { None => true, } { Some(invalid_block_message("PostToolUse")) - } else if !should_block && wire.reason.is_some() { + } else if !should_block && universal.continue_processing && wire.reason.is_some() { Some("PostToolUse hook returned reason without decision".to_string()) } else { None @@ -236,11 +236,7 @@ fn unsupported_pre_tool_use_universal(universal: &UniversalOutput) -> Option Option { - if !universal.continue_processing { - Some("PostToolUse hook returned unsupported continue:false".to_string()) - } else if universal.stop_reason.is_some() { - Some("PostToolUse hook returned unsupported stopReason".to_string()) - } else if universal.suppress_output { + if universal.suppress_output { Some("PostToolUse hook returned unsupported suppressOutput".to_string()) } else { None diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index e166b09183e3..3af9bef5e562 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -35,12 +35,16 @@ pub struct PostToolUseRequest { #[derive(Debug)] pub struct PostToolUseOutcome { pub hook_events: Vec, + pub should_stop: bool, + pub stop_reason: Option, pub additional_contexts: Vec, pub feedback_message: Option, } #[derive(Debug, Default, PartialEq, Eq)] struct PostToolUseHandlerData { + should_stop: bool, + stop_reason: Option, additional_contexts_for_model: Vec, feedback_messages_for_model: Vec, } @@ -72,6 +76,8 @@ pub(crate) async fn run( if matched.is_empty() { return PostToolUseOutcome { hook_events: Vec::new(), + should_stop: false, + stop_reason: None, additional_contexts: Vec::new(), feedback_message: None, }; @@ -117,6 +123,10 @@ pub(crate) async fn run( .iter() .map(|result| result.data.additional_contexts_for_model.as_slice()), ); + let should_stop = results.iter().any(|result| result.data.should_stop); + let stop_reason = results + .iter() + .find_map(|result| result.data.stop_reason.clone()); let feedback_message = common::join_text_chunks( results .iter() @@ -126,6 +136,8 @@ pub(crate) async fn run( PostToolUseOutcome { hook_events: results.into_iter().map(|result| result.completed).collect(), + should_stop, + stop_reason, additional_contexts, feedback_message, } @@ -138,6 +150,8 @@ fn parse_completed( ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); let mut status = HookRunStatus::Completed; + let mut should_stop = false; + let mut stop_reason = None; let mut additional_contexts_for_model = Vec::new(); let mut feedback_messages_for_model = Vec::new(); @@ -171,7 +185,25 @@ fn parse_completed( additional_context, ); } - if let Some(invalid_reason) = parsed.invalid_reason { + if !parsed.universal.continue_processing { + status = HookRunStatus::Stopped; + should_stop = true; + stop_reason = parsed.universal.stop_reason.clone(); + let stop_text = parsed + .universal + .stop_reason + .unwrap_or_else(|| "PostToolUse hook stopped execution".to_string()); + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Stop, + text: stop_text.clone(), + }); + let model_feedback = parsed + .reason + .as_deref() + .and_then(common::trimmed_non_empty) + .unwrap_or(stop_text); + feedback_messages_for_model.push(model_feedback); + } else if let Some(invalid_reason) = parsed.invalid_reason { status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, @@ -241,6 +273,8 @@ fn parse_completed( dispatcher::ParsedHandler { completed, data: PostToolUseHandlerData { + should_stop, + stop_reason, additional_contexts_for_model, feedback_messages_for_model, }, @@ -250,6 +284,8 @@ fn parse_completed( fn serialization_failure_outcome(hook_events: Vec) -> PostToolUseOutcome { PostToolUseOutcome { hook_events, + should_stop: false, + stop_reason: None, additional_contexts: Vec::new(), feedback_message: None, } @@ -285,6 +321,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { + should_stop: false, + stop_reason: None, additional_contexts_for_model: Vec::new(), feedback_messages_for_model: vec!["bash output looked sketchy".to_string()], } @@ -307,6 +345,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { + should_stop: false, + stop_reason: None, additional_contexts_for_model: vec!["Remember the bash cleanup note.".to_string()], feedback_messages_for_model: Vec::new(), } @@ -335,6 +375,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { + should_stop: false, + stop_reason: None, additional_contexts_for_model: Vec::new(), feedback_messages_for_model: Vec::new(), } @@ -360,6 +402,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { + should_stop: false, + stop_reason: None, additional_contexts_for_model: Vec::new(), feedback_messages_for_model: vec!["post hook says pause".to_string()], } @@ -367,6 +411,37 @@ mod tests { assert_eq!(parsed.completed.run.status, HookRunStatus::Completed); } + #[test] + fn continue_false_stops_with_reason() { + let parsed = parse_completed( + &handler(), + run_result( + Some(0), + r#"{"continue":false,"stopReason":"halt after bash output","reason":"post-tool hook says stop"}"#, + "", + ), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + PostToolUseHandlerData { + should_stop: true, + stop_reason: Some("halt after bash output".to_string()), + additional_contexts_for_model: Vec::new(), + feedback_messages_for_model: vec!["post-tool hook says stop".to_string()], + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped); + assert_eq!( + parsed.completed.run.entries, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Stop, + text: "halt after bash output".to_string(), + }] + ); + } + #[test] fn plain_stdout_is_ignored_for_post_tool_use() { let parsed = parse_completed( @@ -378,6 +453,8 @@ mod tests { assert_eq!( parsed.data, PostToolUseHandlerData { + should_stop: false, + stop_reason: None, additional_contexts_for_model: Vec::new(), feedback_messages_for_model: Vec::new(), } diff --git a/codex-rs/shell-command/src/parse_command.rs b/codex-rs/shell-command/src/parse_command.rs index 1c8ac9948580..1214b7cd3172 100644 --- a/codex-rs/shell-command/src/parse_command.rs +++ b/codex-rs/shell-command/src/parse_command.rs @@ -12,6 +12,17 @@ pub fn shlex_join(tokens: &[String]) -> String { .unwrap_or_else(|_| "".to_string()) } +/// Formats a command the same way Codex surfaces it in the UI: unwrap +/// shell launcher shims like `bash -lc