mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
[hooks] add non-streaming (non-stdin style) shell-only PostToolUse support (#15531)
CHAINED PR - note that base is eternal/hooks-pretooluse-bash, not main -- so the following PR should be first Matching post-tool hook to the pre-tool functionality here: https://github.com/openai/codex/pull/15211 So, PreToolUse calls for plain shell calls, allows blocking. This PostToolUse call runs after the command executed example run: ``` › as a test, run in parallel the following commands: - echo 'one' - echo '[block-pre-tool-use]' - echo '[block-post-tool-use]' ⚠ MCP startup incomplete (failed: notion, linear) • Cruising through those three commands in parallel now, and I’ll share the exact outputs right after they land. • Running PreToolUse hook: checking the observatory runes • Running PreToolUse hook: checking the observatory runes • Running PreToolUse hook: checking the observatory runes PreToolUse hook (blocked) warning: wizard-tower PreToolUse demo blocked a Bash command on purpose. feedback: PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue. PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo 'one' PreToolUse hook (completed) warning: wizard-tower PreToolUse demo inspected Bash: echo '[block-post-tool-use]' • Ran echo '[block-post-tool-use]' └ [block-post-tool-use] • Ran echo 'one' └ one • Running PostToolUse hook: reading the observatory aftermath • Running PostToolUse hook: reading the observatory aftermath PostToolUse hook (blocked) warning: wizard-tower PostToolUse demo blocked the Bash follow-through on purpose. feedback: PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to continue. PostToolUse hook (completed) warning: wizard-tower PostToolUse demo inspected Bash: echo 'one' hook context: PostToolUse demo saw the Bash result. Response preview: one ────────────────────────────────────────────────────────────────────────────────────────────────────── • Ran them in parallel. Little wave report: - echo 'one' completed and returned one. - echo '[block-pre-tool-use]' was blocked before execution by the PreToolUse hook. - echo '[block-post-tool-use]' ran, but its result was blocked afterward by the PostToolUse hook. The hook messages were: - PreToolUse demo blocked the command. Remove [block-pre-tool-use] to continue. - PostToolUse demo blocked the result after execution. Remove [block-post-tool-use] to continue. › for the blocked messages (correctly blocked in this test) did the tool output land back into context? • Yeah, from this run: - PreToolUse block: the command itself did not execute, so no command output landed in context. Only the block message did. - PostToolUse block: the command did execute, but the actual stdout did not come back into context. Only the post-tool block message did. So the blocked notifications landed in context, but the blocked command payloads themselves did not. Nice clean guardrail, aloha. ```
This commit is contained in:
@@ -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<JsonValue> {
|
||||
None
|
||||
}
|
||||
|
||||
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<FunctionCallOutputContentItem>,
|
||||
pub success: Option<bool>,
|
||||
pub post_tool_use_response: Option<JsonValue>,
|
||||
}
|
||||
|
||||
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,10 @@ 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<JsonValue> {
|
||||
self.post_tool_use_response.clone()
|
||||
}
|
||||
}
|
||||
|
||||
pub struct ApplyPatchToolOutput {
|
||||
@@ -305,6 +316,14 @@ impl ToolOutput for ExecCommandToolOutput {
|
||||
)
|
||||
}
|
||||
|
||||
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
|
||||
if self.process_id.is_some() || self.session_command.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(JsonValue::String(self.truncated_output()))
|
||||
}
|
||||
|
||||
fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue {
|
||||
#[derive(Serialize)]
|
||||
struct UnifiedExecCodeModeResult {
|
||||
|
||||
@@ -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;
|
||||
@@ -16,6 +17,7 @@ use crate::protocol::ExecCommandSource;
|
||||
use crate::shell::Shell;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
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;
|
||||
@@ -23,9 +25,12 @@ use crate::tools::handlers::apply_granted_turn_permissions;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::handlers::implicit_granted_permissions;
|
||||
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::orchestrator::ToolOrchestrator;
|
||||
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;
|
||||
@@ -48,6 +53,28 @@ pub struct ShellCommandHandler {
|
||||
backend: ShellCommandBackend,
|
||||
}
|
||||
|
||||
fn shell_payload_command(payload: &ToolPayload) -> Option<String> {
|
||||
match payload {
|
||||
ToolPayload::Function { arguments } => parse_arguments::<ShellToolCallParams>(arguments)
|
||||
.ok()
|
||||
.map(|params| codex_shell_command::parse_command::shlex_join(¶ms.command)),
|
||||
ToolPayload::LocalShell { params } => Some(codex_shell_command::parse_command::shlex_join(
|
||||
¶ms.command,
|
||||
)),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn shell_command_payload_command(payload: &ToolPayload) -> Option<String> {
|
||||
let ToolPayload::Function { arguments } = payload else {
|
||||
return None;
|
||||
};
|
||||
|
||||
parse_arguments::<ShellCommandToolCallParams>(arguments)
|
||||
.ok()
|
||||
.map(|params| params.command)
|
||||
}
|
||||
|
||||
struct RunExecLikeArgs {
|
||||
tool_name: String,
|
||||
exec_params: ExecParams,
|
||||
@@ -178,6 +205,23 @@ impl ToolHandler for ShellHandler {
|
||||
}
|
||||
}
|
||||
|
||||
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
|
||||
shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload { 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 {
|
||||
command: shell_payload_command(payload)?,
|
||||
tool_response,
|
||||
})
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
@@ -268,6 +312,24 @@ impl ToolHandler for ShellCommandHandler {
|
||||
.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
|
||||
shell_command_payload_command(&invocation.payload)
|
||||
.map(|command| PreToolUsePayload { 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 {
|
||||
command: shell_command_payload_command(payload)?,
|
||||
tool_response,
|
||||
})
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
@@ -493,8 +555,19 @@ impl ShellHandler {
|
||||
&call_id,
|
||||
/*turn_diff_tracker*/ None,
|
||||
);
|
||||
let post_tool_use_response = out
|
||||
.as_ref()
|
||||
.ok()
|
||||
.map(|output| crate::tools::format_exec_output_str(output, turn.truncation_policy))
|
||||
.map(JsonValue::String);
|
||||
let content = emitter.finish(event_ctx, out).await?;
|
||||
Ok(FunctionToolOutput::from_text(content, Some(true)))
|
||||
Ok(FunctionToolOutput {
|
||||
body: vec![
|
||||
codex_protocol::models::FunctionCallOutputContentItem::InputText { text: content },
|
||||
],
|
||||
success: Some(true),
|
||||
post_tool_use_response,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,15 @@ 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::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::handlers::ShellCommandHandler;
|
||||
use crate::tools::handlers::ShellHandler;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use serde_json::json;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::watch;
|
||||
|
||||
/// The logic for is_known_safe_command() has heuristics for known shells,
|
||||
@@ -178,3 +186,88 @@ fn shell_command_handler_rejects_login_when_disallowed() {
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_pre_tool_use_payload_uses_joined_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,
|
||||
},
|
||||
};
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
let handler = ShellHandler;
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&ToolInvocation {
|
||||
session: session.into(),
|
||||
turn: turn.into(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
call_id: "call-41".to_string(),
|
||||
tool_name: "shell".to_string(),
|
||||
tool_namespace: None,
|
||||
payload,
|
||||
}),
|
||||
Some(crate::tools::registry::PreToolUsePayload {
|
||||
command: "bash -lc 'printf hi'".to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shell_command_pre_tool_use_payload_uses_raw_command() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({ "command": "printf shell command" }).to_string(),
|
||||
};
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
let handler = ShellCommandHandler {
|
||||
backend: super::ShellCommandBackend::Classic,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&ToolInvocation {
|
||||
session: session.into(),
|
||||
turn: turn.into(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
call_id: "call-42".to_string(),
|
||||
tool_name: "shell_command".to_string(),
|
||||
tool_namespace: None,
|
||||
payload,
|
||||
}),
|
||||
Some(crate::tools::registry::PreToolUsePayload {
|
||||
command: "printf shell command".to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_post_tool_use_payload_uses_tool_output_wire_value() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: json!({ "command": "printf shell command" }).to_string(),
|
||||
};
|
||||
let output = FunctionToolOutput {
|
||||
body: vec![],
|
||||
success: Some(true),
|
||||
post_tool_use_response: Some(json!("shell output")),
|
||||
};
|
||||
let handler = ShellCommandHandler {
|
||||
backend: super::ShellCommandBackend::Classic,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
handler.post_tool_use_payload("call-42", &payload, &output),
|
||||
Some(crate::tools::registry::PostToolUsePayload {
|
||||
command: "printf shell command".to_string(),
|
||||
tool_response: json!("shell output"),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ use crate::shell::Shell;
|
||||
use crate::shell::get_shell_by_model_provided_path;
|
||||
use crate::tools::context::ExecCommandToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::handlers::apply_granted_turn_permissions;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
@@ -16,6 +17,8 @@ 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::PostToolUsePayload;
|
||||
use crate::tools::registry::PreToolUsePayload;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
use crate::tools::spec::UnifiedExecShellMode;
|
||||
@@ -104,7 +107,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
return true;
|
||||
};
|
||||
|
||||
let Ok(params) = serde_json::from_str::<ExecCommandArgs>(arguments) else {
|
||||
let Ok(params) = parse_arguments::<ExecCommandArgs>(arguments) else {
|
||||
return true;
|
||||
};
|
||||
let command = match get_command(
|
||||
@@ -119,6 +122,42 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
!is_known_safe_command(&command)
|
||||
}
|
||||
|
||||
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
|
||||
if invocation.tool_name != "exec_command" {
|
||||
return None;
|
||||
}
|
||||
|
||||
let ToolPayload::Function { arguments } = &invocation.payload else {
|
||||
return None;
|
||||
};
|
||||
|
||||
parse_arguments::<ExecCommandArgs>(arguments)
|
||||
.ok()
|
||||
.map(|args| PreToolUsePayload { command: args.cmd })
|
||||
}
|
||||
|
||||
fn post_tool_use_payload(
|
||||
&self,
|
||||
call_id: &str,
|
||||
payload: &ToolPayload,
|
||||
result: &dyn ToolOutput,
|
||||
) -> Option<PostToolUsePayload> {
|
||||
let ToolPayload::Function { arguments } = payload else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let args = parse_arguments::<ExecCommandArgs>(arguments).ok()?;
|
||||
if args.tty {
|
||||
return None;
|
||||
}
|
||||
|
||||
let tool_response = result.post_tool_use_response(call_id, payload)?;
|
||||
Some(PostToolUsePayload {
|
||||
command: args.cmd,
|
||||
tool_response,
|
||||
})
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
|
||||
@@ -11,6 +11,14 @@ use std::fs;
|
||||
use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use crate::codex::make_session_and_context;
|
||||
use crate::tools::context::ExecCommandToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
#[test]
|
||||
fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> {
|
||||
let json = r#"{"cmd": "echo hello"}"#;
|
||||
@@ -182,3 +190,133 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() -
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_command_pre_tool_use_payload_uses_raw_command() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: serde_json::json!({ "cmd": "printf exec command" }).to_string(),
|
||||
};
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
let handler = UnifiedExecHandler;
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&ToolInvocation {
|
||||
session: session.into(),
|
||||
turn: turn.into(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
call_id: "call-43".to_string(),
|
||||
tool_name: "exec_command".to_string(),
|
||||
tool_namespace: None,
|
||||
payload,
|
||||
}),
|
||||
Some(crate::tools::registry::PreToolUsePayload {
|
||||
command: "printf exec command".to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn exec_command_pre_tool_use_payload_skips_write_stdin() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: serde_json::json!({ "chars": "echo hi" }).to_string(),
|
||||
};
|
||||
let (session, turn) = make_session_and_context().await;
|
||||
let handler = UnifiedExecHandler;
|
||||
|
||||
assert_eq!(
|
||||
handler.pre_tool_use_payload(&ToolInvocation {
|
||||
session: session.into(),
|
||||
turn: turn.into(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
call_id: "call-44".to_string(),
|
||||
tool_name: "write_stdin".to_string(),
|
||||
tool_namespace: None,
|
||||
payload,
|
||||
}),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_commands() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: serde_json::json!({ "cmd": "echo three", "tty": false }).to_string(),
|
||||
};
|
||||
let output = 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!(
|
||||
UnifiedExecHandler.post_tool_use_payload("call-43", &payload, &output),
|
||||
Some(crate::tools::registry::PostToolUsePayload {
|
||||
command: "echo three".to_string(),
|
||||
tool_response: serde_json::json!("three"),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_post_tool_use_payload_skips_interactive_exec() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: serde_json::json!({ "cmd": "echo three", "tty": true }).to_string(),
|
||||
};
|
||||
let output = ExecCommandToolOutput {
|
||||
event_call_id: "event-44".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!(
|
||||
UnifiedExecHandler.post_tool_use_payload("call-44", &payload, &output),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_post_tool_use_payload_skips_running_sessions() {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: serde_json::json!({ "cmd": "echo three", "tty": false }).to_string(),
|
||||
};
|
||||
let output = ExecCommandToolOutput {
|
||||
event_call_id: "event-45".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: Some(45),
|
||||
exit_code: None,
|
||||
original_token_count: None,
|
||||
session_command: Some(vec![
|
||||
"/bin/zsh".to_string(),
|
||||
"-lc".to_string(),
|
||||
"echo three".to_string(),
|
||||
]),
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
UnifiedExecHandler.post_tool_use_payload("call-45", &payload, &output),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,10 +5,13 @@ 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;
|
||||
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;
|
||||
@@ -21,10 +24,8 @@ 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;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
|
||||
@@ -56,6 +57,19 @@ pub trait ToolHandler: Send + Sync {
|
||||
false
|
||||
}
|
||||
|
||||
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
|
||||
None
|
||||
}
|
||||
|
||||
fn post_tool_use_payload(
|
||||
&self,
|
||||
_call_id: &str,
|
||||
_payload: &ToolPayload,
|
||||
_result: &dyn ToolOutput,
|
||||
) -> Option<PostToolUsePayload> {
|
||||
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<Self::Output, FunctionCallError>;
|
||||
@@ -73,6 +87,7 @@ impl AnyToolResult {
|
||||
call_id,
|
||||
payload,
|
||||
result,
|
||||
..
|
||||
} = self;
|
||||
result.to_response_item(&call_id, &payload)
|
||||
}
|
||||
@@ -85,12 +100,32 @@ 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<PreToolUsePayload>;
|
||||
|
||||
fn post_tool_use_payload(
|
||||
&self,
|
||||
call_id: &str,
|
||||
payload: &ToolPayload,
|
||||
result: &dyn ToolOutput,
|
||||
) -> Option<PostToolUsePayload>;
|
||||
|
||||
async fn handle_any(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
@@ -110,6 +145,19 @@ where
|
||||
ToolHandler::is_mutating(self, invocation).await
|
||||
}
|
||||
|
||||
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
|
||||
ToolHandler::pre_tool_use_payload(self, invocation)
|
||||
}
|
||||
|
||||
fn post_tool_use_payload(
|
||||
&self,
|
||||
call_id: &str,
|
||||
payload: &ToolPayload,
|
||||
result: &dyn ToolOutput,
|
||||
) -> Option<PostToolUsePayload> {
|
||||
ToolHandler::post_tool_use_payload(self, call_id, payload, result)
|
||||
}
|
||||
|
||||
async fn handle_any(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
@@ -247,17 +295,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
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -303,6 +352,33 @@ 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| {
|
||||
handler.post_tool_use_payload(
|
||||
&result.call_id,
|
||||
&result.payload,
|
||||
result.result.as_ref(),
|
||||
)
|
||||
})
|
||||
} 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
|
||||
};
|
||||
// Deprecated: this is the legacy AfterToolUse hook. Prefer the new PostToolUse
|
||||
let hook_abort_error = dispatch_after_tool_use_hook(AfterToolUseHookDispatch {
|
||||
invocation: &invocation,
|
||||
output_preview,
|
||||
@@ -317,6 +393,36 @@ impl ToolRegistry {
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
if let Some(outcome) = &post_tool_use_outcome {
|
||||
record_additional_contexts(
|
||||
&invocation.session,
|
||||
&invocation.turn,
|
||||
outcome.additional_contexts.clone(),
|
||||
)
|
||||
.await;
|
||||
|
||||
let replacement_text = if outcome.should_stop {
|
||||
Some(
|
||||
outcome
|
||||
.feedback_message
|
||||
.clone()
|
||||
.or_else(|| outcome.stop_reason.clone())
|
||||
.unwrap_or_else(|| "PostToolUse hook stopped execution".to_string()),
|
||||
)
|
||||
} else {
|
||||
outcome.feedback_message.clone()
|
||||
};
|
||||
if let Some(replacement_text) = replacement_text {
|
||||
let mut guard = response_cell.lock().await;
|
||||
if let Some(result) = guard.as_mut() {
|
||||
result.result = Box::new(FunctionToolOutput::from_text(
|
||||
replacement_text,
|
||||
/*success*/ None,
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
match result {
|
||||
Ok(_) => {
|
||||
let mut guard = response_cell.lock().await;
|
||||
@@ -431,35 +537,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<String> {
|
||||
match (tool_name, payload) {
|
||||
("shell" | "container.exec", ToolPayload::Function { arguments }) => {
|
||||
serde_json::from_str::<ShellToolCallParams>(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::<ShellCommandToolCallParams>(arguments)
|
||||
.ok()
|
||||
.map(|params| params.command)
|
||||
}
|
||||
("exec_command", ToolPayload::Function { arguments }) => {
|
||||
serde_json::from_str::<PreToolUseExecCommandArgs>(arguments)
|
||||
.ok()
|
||||
.map(|params| params.cmd)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
use super::*;
|
||||
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;
|
||||
@@ -50,63 +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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user