Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions codex-rs/core/src/hook_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,18 @@ pub(crate) async fn run_pending_session_start_hooks(
.await
}

/// Runs matching `PreToolUse` hooks before a tool executes.
///
/// `tool_name` is the hook-facing matcher name that will also be serialized to
/// hook stdin. Callers should pass the handler-provided compatibility name
/// rather than deriving it from the internal registry key; otherwise existing
/// matchers such as `^Bash$` can stop matching even though the underlying tool
/// is still shell-like.
pub(crate) async fn run_pre_tool_use_hooks(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
tool_use_id: String,
tool_name: String,
command: String,
) -> Option<String> {
let request = PreToolUseRequest {
Expand All @@ -140,7 +148,7 @@ pub(crate) async fn run_pre_tool_use_hooks(
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
tool_name: "Bash".to_string(),
tool_name,
tool_use_id,
command,
};
Expand Down Expand Up @@ -190,10 +198,17 @@ pub(crate) async fn run_permission_request_hooks(
decision
}

/// Runs matching `PostToolUse` hooks after a tool has produced a successful output.
///
/// The `tool_name`, `command`, and `tool_response` values are already adapted
/// by the tool handler into the stable hook contract. Passing raw internal tool
/// data here would leak implementation details into user hook matchers and hook
/// logs.
pub(crate) async fn run_post_tool_use_hooks(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
tool_use_id: String,
tool_name: String,
command: String,
tool_response: Value,
) -> PostToolUseOutcome {
Expand All @@ -204,7 +219,7 @@ pub(crate) async fn run_post_tool_use_hooks(
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
tool_name: "Bash".to_string(),
tool_name,
tool_use_id,
command,
tool_response,
Expand Down
11 changes: 11 additions & 0 deletions codex-rs/core/src/tools/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ pub trait ToolOutput: Send {

fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem;

/// Returns the stable value exposed to `PostToolUse` hooks for this tool output.
///
/// Tool handlers decide whether a tool participates in `PostToolUse`, but
/// this method lets the output type own any conversion from model-facing
/// response content to hook-facing data. Returning `None` means the output
/// should not produce a post-use hook payload, not merely that the tool had
/// empty output.
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
None
}
Expand Down Expand Up @@ -303,6 +310,10 @@ impl ToolOutput for ApplyPatchToolOutput {
)
}

fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
Some(JsonValue::String(self.text.clone()))
}

fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue {
JsonValue::Object(serde_json::Map::new())
}
Expand Down
39 changes: 39 additions & 0 deletions codex-rs/core/src/tools/handlers/apply_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@ use crate::tools::context::ApplyPatchToolOutput;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::SharedTurnDiffTracker;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::events::ToolEmitter;
use crate::tools::events::ToolEventCtx;
use crate::tools::handlers::apply_granted_turn_permissions;
use crate::tools::handlers::parse_arguments;
use crate::tools::orchestrator::ToolOrchestrator;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolArgumentDiffConsumer;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
Expand Down Expand Up @@ -206,6 +209,21 @@ fn write_permissions_for_paths(
normalize_additional_permissions(permissions).ok()
}

/// Extracts the raw patch text used as the command-shaped hook input for apply_patch.
///
/// The apply_patch tool can arrive as the older JSON/function shape or as a
/// freeform custom tool call. Both represent the same file edit operation, so
/// hooks see the raw patch body in `tool_input.command` either way.
fn apply_patch_payload_command(payload: &ToolPayload) -> Option<String> {
match payload {
ToolPayload::Function { arguments } => parse_arguments::<ApplyPatchToolArgs>(arguments)
.ok()
.map(|args| args.input),
ToolPayload::Custom { input } => Some(input.clone()),
_ => None,
}
}

async fn effective_patch_permissions(
session: &Session,
turn: &TurnContext,
Expand Down Expand Up @@ -260,6 +278,27 @@ impl ToolHandler for ApplyPatchHandler {
Some(Box::<ApplyPatchArgumentDiffConsumer>::default())
}

fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
apply_patch_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: "apply_patch".to_string(),
command,
})
}

fn post_tool_use_payload(
&self,
call_id: &str,
payload: &ToolPayload,
result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
Some(PostToolUsePayload {
tool_name: "apply_patch".to_string(),
command: apply_patch_payload_command(payload)?,
tool_response,
})
}

async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
Expand Down
83 changes: 83 additions & 0 deletions codex-rs/core/src/tools/handlers/apply_patch_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,92 @@ use codex_protocol::protocol::SandboxPolicy;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::Arc;
use tempfile::TempDir;
use tokio::sync::Mutex;

use crate::session::tests::make_session_and_context;
use crate::tools::context::ToolInvocation;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::turn_diff_tracker::TurnDiffTracker;

fn sample_patch() -> &'static str {
r#"*** Begin Patch
*** Add File: hello.txt
+hello
*** End Patch"#
}

async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation {
let (session, turn) = make_session_and_context().await;
ToolInvocation {
session: session.into(),
turn: turn.into(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-apply-patch".to_string(),
tool_name: codex_tools::ToolName::plain("apply_patch"),
payload,
}
}

#[tokio::test]
async fn pre_tool_use_payload_uses_json_patch_input() {
let patch = sample_patch();
let payload = ToolPayload::Function {
arguments: json!({ "input": patch }).to_string(),
};
let invocation = invocation_for_payload(payload).await;
let handler = ApplyPatchHandler;

assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: "apply_patch".to_string(),
command: patch.to_string(),
})
);
}

#[tokio::test]
async fn pre_tool_use_payload_uses_freeform_patch_input() {
let patch = sample_patch();
let payload = ToolPayload::Custom {
input: patch.to_string(),
};
let invocation = invocation_for_payload(payload).await;
let handler = ApplyPatchHandler;

assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: "apply_patch".to_string(),
command: patch.to_string(),
})
);
}

#[test]
fn post_tool_use_payload_uses_patch_input_and_tool_output() {
let patch = sample_patch();
let payload = ToolPayload::Custom {
input: patch.to_string(),
};
let output = ApplyPatchToolOutput::from_text("Success. Updated files.".to_string());
let handler = ApplyPatchHandler;

assert_eq!(
handler.post_tool_use_payload("call-apply-patch", &payload, &output),
Some(PostToolUsePayload {
tool_name: "apply_patch".to_string(),
command: patch.to_string(),
tool_response: json!("Success. Updated files."),
})
);
}

#[test]
fn diff_consumer_does_not_stream_json_tool_call_arguments() {
Expand Down
13 changes: 10 additions & 3 deletions codex-rs/core/src/tools/handlers/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ impl ToolHandler for ShellHandler {
}

fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload { command })
shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: "Bash".to_string(),
command,
})
}

fn post_tool_use_payload(
Expand All @@ -216,6 +219,7 @@ impl ToolHandler for ShellHandler {
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
Some(PostToolUsePayload {
tool_name: "Bash".to_string(),
command: shell_payload_command(payload)?,
tool_response,
})
Expand Down Expand Up @@ -313,8 +317,10 @@ impl ToolHandler for ShellCommandHandler {
}

fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
shell_command_payload_command(&invocation.payload)
.map(|command| PreToolUsePayload { command })
shell_command_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: "Bash".to_string(),
command,
})
}

fn post_tool_use_payload(
Expand All @@ -325,6 +331,7 @@ impl ToolHandler for ShellCommandHandler {
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
Some(PostToolUsePayload {
tool_name: "Bash".to_string(),
command: shell_command_payload_command(payload)?,
tool_response,
})
Expand Down
3 changes: 3 additions & 0 deletions codex-rs/core/src/tools/handlers/shell_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ async fn shell_pre_tool_use_payload_uses_joined_command() {
payload,
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: "Bash".to_string(),
command: "bash -lc 'printf hi'".to_string(),
})
);
Expand All @@ -256,6 +257,7 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() {
payload,
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: "Bash".to_string(),
command: "printf shell command".to_string(),
})
);
Expand All @@ -278,6 +280,7 @@ fn build_post_tool_use_payload_uses_tool_output_wire_value() {
assert_eq!(
handler.post_tool_use_payload("call-42", &payload, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: "Bash".to_string(),
command: "printf shell command".to_string(),
tool_response: json!("shell output"),
})
Expand Down
6 changes: 5 additions & 1 deletion codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ impl ToolHandler for UnifiedExecHandler {

parse_arguments::<ExecCommandArgs>(arguments)
.ok()
.map(|args| PreToolUsePayload { command: args.cmd })
.map(|args| PreToolUsePayload {
tool_name: "Bash".to_string(),
command: args.cmd,
})
}

fn post_tool_use_payload(
Expand All @@ -156,6 +159,7 @@ impl ToolHandler for UnifiedExecHandler {

let tool_response = result.post_tool_use_response(call_id, payload)?;
Some(PostToolUsePayload {
tool_name: "Bash".to_string(),
command: args.cmd,
tool_response,
})
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/tools/handlers/unified_exec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() {
payload,
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: "Bash".to_string(),
command: "printf exec command".to_string(),
})
);
Expand Down Expand Up @@ -266,6 +267,7 @@ fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_co
assert_eq!(
UnifiedExecHandler.post_tool_use_payload("call-43", &payload, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: "Bash".to_string(),
command: "echo three".to_string(),
tool_response: serde_json::json!("three"),
})
Expand Down
18 changes: 18 additions & 0 deletions codex-rs/core/src/tools/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,28 @@ impl AnyToolResult {

#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct PreToolUsePayload {
/// Hook-facing tool name used for matcher selection and hook stdin.
///
/// This is intentionally handler supplied because compatibility names do
/// not always match registry names. Shell-style handlers use `Bash` for
/// historical matcher compatibility, while first-class patch edits use
/// `apply_patch`.
pub(crate) tool_name: String,
/// Command-shaped input exposed at `tool_input.command`.
pub(crate) command: String,
}

#[derive(Debug, Clone, PartialEq)]
pub(crate) struct PostToolUsePayload {
/// Hook-facing tool name used for matcher selection and hook stdin.
///
/// Keep this aligned with the corresponding pre-use payload so external
/// hook consumers can pair events by `tool_use_id` without also needing to
/// normalize tool names.
pub(crate) tool_name: String,
/// Command-shaped input exposed at `tool_input.command`.
pub(crate) command: String,
/// Tool result exposed at `tool_response`.
pub(crate) tool_response: Value,
}

Expand Down Expand Up @@ -321,6 +337,7 @@ impl ToolRegistry {
&invocation.session,
&invocation.turn,
invocation.call_id.clone(),
pre_tool_use_payload.tool_name.clone(),
pre_tool_use_payload.command.clone(),
)
.await
Expand Down Expand Up @@ -391,6 +408,7 @@ impl ToolRegistry {
&invocation.session,
&invocation.turn,
invocation.call_id.clone(),
post_tool_use_payload.tool_name,
post_tool_use_payload.command,
post_tool_use_payload.tool_response,
)
Expand Down
Loading
Loading