diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2f13c5ba91..79cd265c93 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2400,6 +2400,7 @@ mod tests { use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; + use crate::tools::ExecResponseFormat; use crate::tools::MODEL_FORMAT_HEAD_LINES; use crate::tools::MODEL_FORMAT_MAX_BYTES; use crate::tools::MODEL_FORMAT_MAX_LINES; @@ -3089,6 +3090,7 @@ mod tests { &mut turn_diff_tracker, sub_id, call_id, + ExecResponseFormat::LegacyJson, ) .await; @@ -3115,6 +3117,7 @@ mod tests { &mut turn_diff_tracker, "test-sub".to_string(), "test-call-2".to_string(), + ExecResponseFormat::LegacyJson, ) .await; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index ced0898adb..5a9880b228 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -8,6 +8,7 @@ use crate::client_common::tools::ToolSpec; use crate::exec::ExecParams; use crate::function_tool::FunctionCallError; use crate::openai_tools::JsonSchema; +use crate::tools::ExecResponseFormat; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -50,16 +51,16 @@ impl ToolHandler for ApplyPatchHandler { payload, } = invocation; - let patch_input = match payload { + let (patch_input, response_format) = match payload { ToolPayload::Function { arguments } => { let args: ApplyPatchToolArgs = serde_json::from_str(&arguments).map_err(|e| { FunctionCallError::RespondToModel(format!( "failed to parse function arguments: {e:?}" )) })?; - args.input + (args.input, ExecResponseFormat::LegacyJson) } - ToolPayload::Custom { input } => input, + ToolPayload::Custom { input } => (input, ExecResponseFormat::StructuredText), _ => { return Err(FunctionCallError::RespondToModel( "apply_patch handler received unsupported payload".to_string(), @@ -84,6 +85,7 @@ impl ToolHandler for ApplyPatchHandler { tracker, sub_id.to_string(), call_id.clone(), + response_format, ) .await?; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index fbcb493e24..1614a267fb 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -5,10 +5,12 @@ use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; use crate::function_tool::FunctionCallError; +use crate::tools::ExecResponseFormat; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::handle_container_exec_with_params; +use crate::tools::handlers::apply_patch::ApplyPatchToolType; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -54,6 +56,14 @@ impl ToolHandler for ShellHandler { payload, } = invocation; + // When using the freeform apply_patch tool type, shell tool output should also be raw + // output, not json-encoded. + let response_format = match turn.tools_config.apply_patch_tool_type { + Some(ApplyPatchToolType::Freeform) => ExecResponseFormat::StructuredText, + Some(ApplyPatchToolType::Function) => ExecResponseFormat::LegacyJson, + None => ExecResponseFormat::LegacyJson, + }; + match payload { ToolPayload::Function { arguments } => { let params: ShellToolCallParams = @@ -71,6 +81,7 @@ impl ToolHandler for ShellHandler { tracker, sub_id.to_string(), call_id.clone(), + response_format, ) .await?; Ok(ToolOutput::Function { @@ -88,6 +99,7 @@ impl ToolHandler for ShellHandler { tracker, sub_id.to_string(), call_id.clone(), + response_format, ) .await?; Ok(ToolOutput::Function { diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 5a120d0907..c8313ecf30 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -44,6 +44,12 @@ pub(crate) const TELEMETRY_PREVIEW_MAX_LINES: usize = 64; // lines pub(crate) const TELEMETRY_PREVIEW_TRUNCATION_NOTICE: &str = "[... telemetry preview truncated ...]"; +#[derive(Clone, Copy)] +pub(crate) enum ExecResponseFormat { + LegacyJson, + StructuredText, +} + // TODO(jif) break this down pub(crate) async fn handle_container_exec_with_params( tool_name: &str, @@ -53,6 +59,7 @@ pub(crate) async fn handle_container_exec_with_params( turn_diff_tracker: &mut TurnDiffTracker, sub_id: String, call_id: String, + response_format: ExecResponseFormat, ) -> Result { let otel_event_manager = turn_context.client.get_otel_event_manager(); @@ -148,7 +155,7 @@ pub(crate) async fn handle_container_exec_with_params( match output_result { Ok(output) => { let ExecToolCallOutput { exit_code, .. } = &output; - let content = format_exec_output_apply_patch(&output); + let content = format_exec_output_with_style(&output, response_format); if *exit_code == 0 { Ok(content) } else { @@ -156,12 +163,14 @@ pub(crate) async fn handle_container_exec_with_params( } } Err(ExecError::Function(err)) => Err(err), - Err(ExecError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) => Err( - FunctionCallError::RespondToModel(format_exec_output_apply_patch(&output)), - ), - Err(ExecError::Codex(err)) => Err(FunctionCallError::RespondToModel(format!( - "execution error: {err:?}" - ))), + Err(ExecError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) => { + Err(FunctionCallError::RespondToModel( + format_exec_output_with_style(&output, response_format), + )) + } + Err(ExecError::Codex(err)) => Err(FunctionCallError::RespondToModel( + format_unexpected_exec_error(err, response_format), + )), } } @@ -201,6 +210,155 @@ pub fn format_exec_output_apply_patch(exec_output: &ExecToolCallOutput) -> Strin serde_json::to_string(&payload).expect("serialize ExecOutput") } +fn format_exec_output_with_style( + exec_output: &ExecToolCallOutput, + response_format: ExecResponseFormat, +) -> String { + match response_format { + ExecResponseFormat::LegacyJson => format_exec_output_apply_patch(exec_output), + ExecResponseFormat::StructuredText => format_exec_output_structured(exec_output), + } +} + +fn format_unexpected_exec_error(err: CodexErr, response_format: ExecResponseFormat) -> String { + match response_format { + ExecResponseFormat::LegacyJson => format!("execution error: {err:?}"), + ExecResponseFormat::StructuredText => format_structured_error(&format!("{err:?}")), + } +} + +fn format_structured_error(message: &str) -> String { + let lines = vec![ + "Exit code: N/A".to_string(), + "Wall time: N/A seconds".to_string(), + format!("Error: {message}"), + "Output:".to_string(), + String::new(), + ]; + lines.join("\n") +} + +fn format_wall_time(duration: std::time::Duration) -> String { + format_significant_digits(duration.as_secs_f64(), 4) +} + +fn format_significant_digits(value: f64, digits: usize) -> String { + if !value.is_finite() { + return value.to_string(); + } + if value == 0.0 { + return "0".to_string(); + } + + let abs = value.abs(); + let initial_exponent = abs.log10().floor() as i32; + let rounded_value = if value == 0.0 { + 0.0 + } else { + let scale = 10_f64.powf((digits as f64 - 1.0) - initial_exponent as f64); + (value * scale).round() / scale + }; + + let abs_rounded = rounded_value.abs(); + let exponent = if abs_rounded == 0.0 { + 0 + } else { + abs_rounded.log10().floor() as i32 + }; + let use_exp = exponent < -4 || exponent >= digits as i32; + if use_exp { + return format!("{rounded_value:.prec$e}", prec = digits.saturating_sub(1)); + } + + let decimal_places = (digits as i32 - exponent - 1).max(0) as usize; + let mut s = format!("{rounded_value:.decimal_places$}"); + if s.contains('.') { + while s.ends_with('0') { + s.pop(); + } + if s.ends_with('.') { + s.pop(); + } + } + s +} + +pub fn format_exec_output_structured(exec_output: &ExecToolCallOutput) -> String { + let ExecToolCallOutput { + exit_code, + duration, + aggregated_output, + .. + } = exec_output; + + let mut sections = Vec::new(); + sections.push(format!("Exit code: {exit_code}")); + sections.push(format!( + "Wall time: {} seconds", + format_wall_time(*duration) + )); + + if let Some(total_lines) = aggregated_output.truncated_after_lines { + sections.push(format!("Total output lines: {total_lines}")); + } + + sections.push("Output:".to_string()); + sections.push(format_exec_output_str(exec_output)); + + sections.join("\n") +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::exec::StreamOutput; + use pretty_assertions::assert_eq; + use std::time::Duration; + + fn sample_output() -> ExecToolCallOutput { + ExecToolCallOutput { + exit_code: 0, + stdout: StreamOutput::new("stdout".to_string()), + stderr: StreamOutput::new("stderr".to_string()), + aggregated_output: StreamOutput::new("stdout\nstderr".to_string()), + duration: Duration::from_secs_f64(1.2345), + timed_out: false, + } + } + + #[test] + fn structured_format_basic() { + let formatted = format_exec_output_structured(&sample_output()); + let expected = "Exit code: 0\nWall time: 1.235 seconds\nOutput:\nstdout\nstderr"; + assert_eq!(formatted, expected); + } + + #[test] + fn structured_format_includes_truncation_metadata() { + let mut output = sample_output(); + output.aggregated_output.truncated_after_lines = Some(200); + let formatted = format_exec_output_structured(&output); + assert!(formatted.contains("Total output lines: 200")); + } + + #[test] + fn significant_digit_formatting_matches_expectations() { + assert_eq!(format_significant_digits(0.0, 4), "0"); + assert_eq!(format_significant_digits(1.23456, 4), "1.235"); + assert_eq!(format_significant_digits(12345.0, 4), "1.235e4"); + assert_eq!(format_significant_digits(0.000123456, 4), "0.0001235"); + } + + #[test] + fn structured_error_includes_metadata() { + let error = format_structured_error("unexpected failure"); + assert_eq!( + error, + "Exit code: N/A\nWall time: N/A seconds\nError: unexpected failure\nOutput:\n" + ); + } +} + pub fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { let ExecToolCallOutput { aggregated_output, .. diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 323cc879fe..3ad25b6159 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -71,6 +71,10 @@ pub struct Cli { #[arg(long = "include-plan-tool", default_value_t = false)] pub include_plan_tool: bool, + /// Force-enable the apply_patch tool even for models that do not opt into it by default. + #[arg(long = "custom-apply-patch", default_value_t = false)] + pub custom_apply_patch: bool, + /// Specifies file where the last message from the agent should be written. #[arg(long = "output-last-message", short = 'o', value_name = "FILE")] pub last_message_file: Option, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index e3bf6e52aa..74673fa6a7 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -69,6 +69,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any prompt, output_schema: output_schema_path, include_plan_tool, + custom_apply_patch, config_overrides, } = cli; @@ -177,7 +178,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any codex_linux_sandbox_exe, base_instructions: None, include_plan_tool: Some(include_plan_tool), - include_apply_patch_tool: Some(true), + include_apply_patch_tool: custom_apply_patch.then_some(true), include_view_image_tool: None, show_raw_agent_reasoning: oss.then_some(true), tools_web_search_request: None, diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index f0630a34c5..0d62925b46 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -72,6 +72,10 @@ pub struct Cli { #[arg(long = "search", default_value_t = false)] pub web_search: bool, + /// Force-enable the apply_patch tool even for models that do not opt into it by default. + #[arg(long = "custom-apply-patch", default_value_t = false)] + pub custom_apply_patch: bool, + #[clap(skip)] pub config_overrides: CliConfigOverrides, } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 28d2a3f08b..294095db7c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -142,7 +142,7 @@ pub async fn run_main( codex_linux_sandbox_exe, base_instructions: None, include_plan_tool: Some(true), - include_apply_patch_tool: None, + include_apply_patch_tool: cli.custom_apply_patch.then_some(true), include_view_image_tool: None, show_raw_agent_reasoning: cli.oss.then_some(true), tools_web_search_request: cli.web_search.then_some(true),