clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 06:35:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify bash post-tool-use scope

Browse Source
This commit is contained in:
Andrei Eternal
2026-03-24 15:45:17 -07:00
parent 1d5cdee9d4
commit a10ebabeb5
12 changed files with 88 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
codex-rs/core/src/tools/context.rs
View File

@@ -84,8 +84,8 @@ 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> {
response_item_to_post_tool_use_response(self.to_response_item(call_id, payload))
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
None
}
fn code_mode_result(&self, payload: &ToolPayload) -> JsonValue {
@@ -205,10 +205,8 @@ impl ToolOutput for FunctionToolOutput {
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().or_else(|| {
response_item_to_post_tool_use_response(self.to_response_item(call_id, payload))
})
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
self.post_tool_use_response.clone()
}
}
@@ -318,10 +316,6 @@ impl ToolOutput for ExecCommandToolOutput {
)
}
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
Some(JsonValue::String(self.truncated_output()))
}
fn code_mode_result(&self, _payload: &ToolPayload) -> JsonValue {
#[derive(Serialize)]
struct UnifiedExecCodeModeResult {
@@ -395,15 +389,6 @@ impl ExecCommandToolOutput {
}
}
fn response_item_to_post_tool_use_response(response_item: ResponseInputItem) -> Option<JsonValue> {
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(

39
codex-rs/core/src/tools/handlers/shell.rs
View File

@@ -52,48 +52,49 @@ pub struct ShellCommandHandler {
backend: ShellCommandBackend,
}
fn bash_post_tool_use_payload(
fn build_post_tool_use_payload(
result: &AnyToolResult,
pre_tool_use_payload: Option<PreToolUsePayload>,
command: Option<String>,
) -> Option<PostToolUsePayload> {
let command = pre_tool_use_payload?.command;
let tool_response = result
.result
.post_tool_use_response(&result.call_id, &result.payload)?;
Some(PostToolUsePayload {
command,
command: command?,
tool_response,
})
}
fn shell_pre_tool_use_payload(payload: &ToolPayload) -> Option<PreToolUsePayload> {
fn shell_payload_command(payload: &ToolPayload) -> Option<String> {
match payload {
ToolPayload::Function { arguments } => {
serde_json::from_str::<ShellToolCallParams>(arguments)
.ok()
.map(|params| PreToolUsePayload {
command: codex_shell_command::parse_command::command_for_display(
&params.command,
),
})
.map(|params| codex_shell_command::parse_command::shlex_join(&params.command))
}
ToolPayload::LocalShell { params } => Some(PreToolUsePayload {
command: codex_shell_command::parse_command::command_for_display(&params.command),
}),
ToolPayload::LocalShell { params } => Some(codex_shell_command::parse_command::shlex_join(
&params.command,
)),
_ => None,
}
}
fn shell_command_pre_tool_use_payload(payload: &ToolPayload) -> Option<PreToolUsePayload> {
fn shell_pre_tool_use_payload(payload: &ToolPayload) -> Option<PreToolUsePayload> {
shell_payload_command(payload).map(|command| PreToolUsePayload { command })
}
fn shell_command_payload_command(payload: &ToolPayload) -> Option<String> {
let ToolPayload::Function { arguments } = payload else {
return None;
};
serde_json::from_str::<ShellCommandToolCallParams>(arguments)
.ok()
.map(|params| PreToolUsePayload {
command: params.command,
})
.map(|params| params.command)
}
fn shell_command_pre_tool_use_payload(payload: &ToolPayload) -> Option<PreToolUsePayload> {
shell_command_payload_command(payload).map(|command| PreToolUsePayload { command })
}
struct RunExecLikeArgs {
@@ -231,7 +232,7 @@ impl ToolHandler for ShellHandler {
}
fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option<PostToolUsePayload> {
bash_post_tool_use_payload(result, shell_pre_tool_use_payload(&result.payload))
build_post_tool_use_payload(result, shell_payload_command(&result.payload))
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
@@ -329,7 +330,7 @@ impl ToolHandler for ShellCommandHandler {
}
fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option<PostToolUsePayload> {
bash_post_tool_use_payload(result, shell_command_pre_tool_use_payload(&result.payload))
build_post_tool_use_payload(result, shell_command_payload_command(&result.payload))
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {

11
codex-rs/core/src/tools/handlers/shell_tests.rs
View File

@@ -20,7 +20,7 @@ use crate::tools::registry::AnyToolResult;
use serde_json::json;
use tokio::sync::watch;
use super::bash_post_tool_use_payload;
use super::build_post_tool_use_payload;
use super::shell_command_pre_tool_use_payload;
use super::shell_pre_tool_use_payload;
@@ -188,7 +188,7 @@ fn shell_command_handler_rejects_login_when_disallowed() {
}
#[test]
fn shell_pre_tool_use_payload_uses_displayed_command() {
fn shell_pre_tool_use_payload_uses_joined_command() {
let payload = ToolPayload::LocalShell {
params: codex_protocol::models::ShellToolCallParams {
command: vec![
@@ -208,7 +208,7 @@ fn shell_pre_tool_use_payload_uses_displayed_command() {
assert_eq!(
shell_pre_tool_use_payload(&payload),
Some(crate::tools::registry::PreToolUsePayload {
command: "printf hi".to_string(),
command: "bash -lc 'printf hi'".to_string(),
})
);
}
@@ -228,12 +228,11 @@ fn shell_command_pre_tool_use_payload_uses_raw_command() {
}
#[test]
fn bash_post_tool_use_payload_uses_tool_output_wire_value() {
fn build_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 {
@@ -244,7 +243,7 @@ fn bash_post_tool_use_payload_uses_tool_output_wire_value() {
};
assert_eq!(
bash_post_tool_use_payload(&result, shell_command_pre_tool_use_payload(&result.payload),),
build_post_tool_use_payload(&result, Some("printf shell command".to_string()),),
Some(crate::tools::registry::PostToolUsePayload {
command: "printf shell command".to_string(),
tool_response: json!("shell output"),

20
codex-rs/core/src/tools/handlers/unified_exec.rs
View File

@@ -16,8 +16,6 @@ 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;
@@ -101,17 +99,6 @@ fn exec_command_pre_tool_use_payload(
.map(|args| PreToolUsePayload { command: args.cmd })
}
fn exec_command_post_tool_use_payload(result: &AnyToolResult) -> Option<PostToolUsePayload> {
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;
@@ -152,10 +139,6 @@ impl ToolHandler for UnifiedExecHandler {
exec_command_pre_tool_use_payload(&invocation.tool_name, &invocation.payload)
}
fn post_tool_use_payload(&self, result: &AnyToolResult) -> Option<PostToolUsePayload> {
exec_command_post_tool_use_payload(result)
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
@@ -199,8 +182,7 @@ impl ToolHandler for UnifiedExecHandler {
turn.tools_config.allow_login_shell,
)
.map_err(FunctionCallError::RespondToModel)?;
let command_for_display =
codex_shell_command::parse_command::command_for_display(&command);
let command_for_display = codex_shell_command::parse_command::shlex_join(&command);
let ExecCommandArgs {
workdir,

37
codex-rs/core/src/tools/handlers/unified_exec_tests.rs
View File

@@ -11,9 +11,7 @@ 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<()> {
@@ -212,38 +210,3 @@ fn exec_command_pre_tool_use_payload_skips_write_stdin() {
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"),
})
);
}

1
codex-rs/core/src/tools/parallel.rs
View File

@@ -162,7 +162,6 @@ 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 {

3
codex-rs/core/src/tools/registry.rs
View File

@@ -71,7 +71,6 @@ pub trait ToolHandler: Send + Sync {
}
pub(crate) struct AnyToolResult {
pub(crate) tool_name: String,
pub(crate) call_id: String,
pub(crate) payload: ToolPayload,
pub(crate) result: Box<dyn ToolOutput>,
@@ -148,12 +147,10 @@ where
&self,
invocation: ToolInvocation,
) -> Result<AnyToolResult, FunctionCallError> {
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),

36
codex-rs/core/tests/suite/hooks.rs
View File

@@ -1175,7 +1175,7 @@ async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> {
assert!(
output.contains(&format!(
"Command: {}",
codex_shell_command::parse_command::command_for_display(&command)
codex_shell_command::parse_command::shlex_join(&command)
)),
"blocked local shell output should surface the blocked command",
);
@@ -1188,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::command_for_display(&command),
codex_shell_command::parse_command::shlex_join(&command),
);
assert!(
hook_inputs[0]["turn_id"]
@@ -1655,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::command_for_display(&command),
codex_shell_command::parse_command::shlex_join(&command),
);
assert_eq!(
hook_inputs[0]["tool_response"],
@@ -1666,16 +1666,12 @@ 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_replaces_exec_command_output_with_feedback() -> Result<()> {
async fn post_tool_use_does_not_fire_for_exec_command() -> 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 command = "printf post-hook-output".to_string();
let args = serde_json::json!({ "cmd": command });
let responses = mount_sse_sequence(
&server,
@@ -1691,7 +1687,7 @@ async fn post_tool_use_exit_two_replaces_exec_command_output_with_feedback() ->
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "post hook blocked the exec result"),
ev_assistant_message("msg-1", "exec command completed"),
ev_completed("resp-2"),
]),
],
@@ -1719,10 +1715,6 @@ async fn post_tool_use_exit_two_replaces_exec_command_output_with_feedback() ->
});
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?;
@@ -1733,19 +1725,15 @@ async fn post_tool_use_exit_two_replaces_exec_command_output_with_feedback() ->
.get("output")
.and_then(Value::as_str)
.expect("exec command output string");
assert_eq!(output, "blocked by post hook");
assert!(
marker.exists(),
"post tool use should run after command execution"
output.contains("post-hook-output"),
"exec command output should reach the model unchanged",
);
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_eq!(
hook_inputs[0]["tool_response"],
Value::String("post-hook-output".to_string())
let hook_log_path = test.codex_home_path().join("post_tool_use_hook_log.jsonl");
assert!(
!hook_log_path.exists(),
"exec_command should not trigger post tool use hooks",
);
Ok(())

76
codex-rs/hooks/src/engine/discovery.rs
View File

@@ -71,46 +71,42 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -
}
};
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,
);
let super::config::HookEvents {
pre_tool_use,
post_tool_use,
session_start,
user_prompt_submit,
stop,
} = parsed.hooks;
for (event_name, groups) in [
(
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),
] {
append_matcher_groups(
&mut handlers,
&mut warnings,
&mut display_order,
source_path.as_path(),
event_name,
groups,
);
}
}
DiscoveryResult { handlers, warnings }

23
codex-rs/shell-command/src/parse_command.rs
View File

@@ -12,17 +12,6 @@ pub fn shlex_join(tokens: &[String]) -> String {
.unwrap_or_else(|_| "<command included NUL byte>".to_string())
}
/// Formats a command the same way Codex surfaces it in the UI: unwrap
/// shell launcher shims like `bash -lc <script>` when possible, otherwise
/// fall back to a shell-escaped command line.
pub fn command_for_display(command: &[String]) -> String {
if let Some((_, script)) = extract_shell_command(command) {
script.to_string()
} else {
shlex_try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" "))
}
}
/// Extracts the shell and script from a command, regardless of platform
pub fn extract_shell_command(command: &[String]) -> Option<(&str, &str)> {
extract_bash_command(command).or_else(|| extract_powershell_command(command))
@@ -92,18 +81,6 @@ mod tests {
assert_eq!(out, expected);
}
#[test]
fn command_for_display_unwraps_shell_wrapper() {
let command = vec_str(&["/bin/zsh", "-lc", "echo hello"]);
assert_eq!(command_for_display(&command), "echo hello");
}
#[test]
fn command_for_display_escapes_plain_argv() {
let command = vec_str(&["git", "commit", "-m", "surf's up"]);
assert_eq!(command_for_display(&command), "git commit -m \"surf's up\"");
}
#[test]
fn git_status_is_unknown() {
assert_parsed(

9
codex-rs/tui/src/exec_command.rs
View File

@@ -1,18 +1,19 @@
use std::path::Path;
use std::path::PathBuf;
use codex_shell_command::parse_command::command_for_display;
use codex_shell_command::parse_command::extract_shell_command;
use dirs::home_dir;
#[cfg(test)]
use shlex::try_join;
#[cfg(test)]
pub(crate) fn escape_command(command: &[String]) -> String {
try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" "))
}
pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String {
command_for_display(command)
if let Some((_, script)) = extract_shell_command(command) {
return script.to_string();
}
escape_command(command)
}
/// If `path` is absolute and inside $HOME, return the part *after* the home

9
codex-rs/tui_app_server/src/exec_command.rs
View File

@@ -1,18 +1,19 @@
use std::path::Path;
use std::path::PathBuf;
use codex_shell_command::parse_command::command_for_display;
use codex_shell_command::parse_command::extract_shell_command;
use dirs::home_dir;
#[cfg(test)]
use shlex::try_join;
#[cfg(test)]
pub(crate) fn escape_command(command: &[String]) -> String {
try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" "))
}
pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String {
command_for_display(command)
if let Some((_, script)) = extract_shell_command(command) {
return script.to_string();
}
escape_command(command)
}
/// If `path` is absolute and inside $HOME, return the part *after* the home