From 150765dbe329ef0296bbeb07279ccaaefda27dac Mon Sep 17 00:00:00 2001 From: jimmyfraiture Date: Tue, 30 Sep 2025 17:19:23 +0100 Subject: [PATCH] V2 --- codex-rs/Cargo.lock | 5 + codex-rs/Cargo.toml | 2 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 285 +----------------- codex-rs/core/src/tools/context.rs | 9 +- .../core/src/tools/handlers/apply_patch.rs | 33 +- codex-rs/core/src/tools/handlers/mod.rs | 2 +- codex-rs/core/src/tools/handlers/shell.rs | 5 +- codex-rs/core/src/tools/mod.rs | 252 ++++++++++++++++ codex-rs/core/src/tools/router.rs | 1 - codex-rs/core/src/tools/spec.rs | 12 +- codex-rs/utils/string/Cargo.toml | 7 + codex-rs/utils/string/src/lib.rs | 38 +++ 13 files changed, 354 insertions(+), 298 deletions(-) create mode 100644 codex-rs/utils/string/Cargo.toml create mode 100644 codex-rs/utils/string/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 49f4a148a8..793970979c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -846,6 +846,7 @@ dependencies = [ "codex-otel", "codex-protocol", "codex-rmcp-client", + "codex-utils-string", "core_test_support", "dirs", "env-flags", @@ -1237,6 +1238,10 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-utils-string" +version = "0.0.0" + [[package]] name = "color-eyre" version = "0.6.5" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 66d8073ca3..783ef097c1 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -31,6 +31,7 @@ members = [ "git-apply", "utils/json-to-toml", "utils/readiness", + "utils/string", ] resolver = "2" @@ -69,6 +70,7 @@ codex-rmcp-client = { path = "rmcp-client" } codex-tui = { path = "tui" } codex-utils-json-to-toml = { path = "utils/json-to-toml" } codex-utils-readiness = { path = "utils/readiness" } +codex-utils-string = { path = "utils/string" } core_test_support = { path = "core/tests/common" } mcp-types = { path = "mcp-types" } mcp_test_support = { path = "mcp-server/tests/common" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index ff1103c3d2..253fd57aed 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -25,6 +25,7 @@ codex-mcp-client = { workspace = true } codex-rmcp-client = { workspace = true } codex-protocol = { workspace = true } codex-otel = { workspace = true, features = ["otel"] } +codex-utils-string = { workspace = true } dirs = { workspace = true } env-flags = { workspace = true } eventsource-stream = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 974828f9ea..4a35fd9221 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -110,7 +110,7 @@ use crate::state::SessionServices; use crate::tasks::CompactTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; -use crate::tools::Router; +use crate::tools::{format_exec_output_str, Router}; use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::UnifiedExecSessionManager; use crate::user_instructions::UserInstructions; @@ -149,13 +149,6 @@ pub struct CodexSpawnOk { pub(crate) const INITIAL_SUBMIT_ID: &str = ""; pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 64; -// Model-formatting limits: clients get full streams; oonly content sent to the model is truncated. -pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB -pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines -pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; -pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 -pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; - impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub async fn spawn( @@ -250,10 +243,10 @@ use crate::state::SessionState; /// A session has at most 1 running task at a time, and can be interrupted by user input. pub(crate) struct Session { conversation_id: ConversationId, - tx_event: Sender, + pub(crate) tx_event: Sender, state: Mutex, pub(crate) active_turn: Mutex>, - services: SessionServices, + pub(crate) services: SessionServices, next_internal_sub_id: AtomicU64, } @@ -919,7 +912,7 @@ impl Session { /// command even on error. /// /// Returns the output of the exec tool call. - async fn run_exec_with_events( + pub(crate) async fn run_exec_with_events( &self, turn_diff_tracker: &mut TurnDiffTracker, prepared: PreparedExec, @@ -2270,275 +2263,6 @@ async fn handle_response_item( } } -pub(crate) async fn handle_container_exec_with_params( - tool_name: &str, - params: ExecParams, - sess: &Session, - turn_context: &TurnContext, - turn_diff_tracker: &mut TurnDiffTracker, - sub_id: String, - call_id: String, -) -> Result { - let otel_event_manager = turn_context.client.get_otel_event_manager(); - - if params.with_escalated_permissions.unwrap_or(false) - && !matches!(turn_context.approval_policy, AskForApproval::OnRequest) - { - return Err(FunctionCallError::RespondToModel(format!( - "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", - policy = turn_context.approval_policy - ))); - } - - // check if this was a patch, and apply it if so - let apply_patch_exec = match maybe_parse_apply_patch_verified(¶ms.command, ¶ms.cwd) { - MaybeApplyPatchVerified::Body(changes) => { - match apply_patch::apply_patch(sess, turn_context, &sub_id, &call_id, changes).await { - InternalApplyPatchInvocation::Output(item) => return item, - InternalApplyPatchInvocation::DelegateToExec(apply_patch_exec) => { - Some(apply_patch_exec) - } - } - } - MaybeApplyPatchVerified::CorrectnessError(parse_error) => { - // It looks like an invocation of `apply_patch`, but we - // could not resolve it into a patch that would apply - // cleanly. Return to model for resample. - return Err(FunctionCallError::RespondToModel(format!( - "error: {parse_error:#?}" - ))); - } - MaybeApplyPatchVerified::ShellParseError(error) => { - trace!("Failed to parse shell command, {error:?}"); - None - } - MaybeApplyPatchVerified::NotApplyPatch => None, - }; - - let command_for_display = if let Some(exec) = apply_patch_exec.as_ref() { - vec!["apply_patch".to_string(), exec.action.patch.clone()] - } else { - params.command.clone() - }; - - let exec_command_context = ExecCommandContext { - sub_id: sub_id.clone(), - call_id: call_id.clone(), - command_for_display: command_for_display.clone(), - cwd: params.cwd.clone(), - apply_patch: apply_patch_exec.as_ref().map( - |ApplyPatchExec { - action, - user_explicitly_approved_this_action, - }| ApplyPatchCommandContext { - user_explicitly_approved_this_action: *user_explicitly_approved_this_action, - changes: convert_apply_patch_to_protocol(action), - }, - ), - tool_name: tool_name.to_string(), - otel_event_manager, - }; - - let mode = match apply_patch_exec { - Some(exec) => ExecutionMode::ApplyPatch(exec), - None => ExecutionMode::Shell, - }; - - sess.services.executor.update_environment( - turn_context.sandbox_policy.clone(), - turn_context.cwd.clone(), - ); - - let prepared_exec = PreparedExec::new( - exec_command_context, - params, - command_for_display, - mode, - Some(StdoutStream { - sub_id: sub_id.clone(), - call_id: call_id.clone(), - tx_event: sess.tx_event.clone(), - }), - turn_context.shell_environment_policy.use_profile, - ); - - let output_result = sess - .run_exec_with_events( - turn_diff_tracker, - prepared_exec, - turn_context.approval_policy, - ) - .await; - - match output_result { - Ok(output) => { - let ExecToolCallOutput { exit_code, .. } = &output; - let content = format_exec_output(&output); - if *exit_code == 0 { - Ok(content) - } else { - Err(FunctionCallError::RespondToModel(content)) - } - } - Err(ExecError::Function(err)) => Err(err), - Err(ExecError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) => Err( - FunctionCallError::RespondToModel(format_exec_output(&output)), - ), - Err(ExecError::Codex(err)) => Err(FunctionCallError::RespondToModel(format!( - "execution error: {err:?}" - ))), - } -} - -fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { - let ExecToolCallOutput { - aggregated_output, .. - } = exec_output; - - // Head+tail truncation for the model: show the beginning and end with an elision. - // Clients still receive full streams; only this formatted summary is capped. - - let mut s = &aggregated_output.text; - let prefixed_str: String; - - if exec_output.timed_out { - prefixed_str = format!( - "command timed out after {} milliseconds\n", - exec_output.duration.as_millis() - ) + s; - s = &prefixed_str; - } - - let total_lines = s.lines().count(); - if s.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES { - return s.to_string(); - } - - let lines: Vec<&str> = s.lines().collect(); - let head_take = MODEL_FORMAT_HEAD_LINES.min(lines.len()); - let tail_take = MODEL_FORMAT_TAIL_LINES.min(lines.len().saturating_sub(head_take)); - let omitted = lines.len().saturating_sub(head_take + tail_take); - - // Join head and tail blocks (lines() strips newlines; reinsert them) - let head_block = lines - .iter() - .take(head_take) - .cloned() - .collect::>() - .join("\n"); - let tail_block = if tail_take > 0 { - lines[lines.len() - tail_take..].join("\n") - } else { - String::new() - }; - let marker = format!("\n[... omitted {omitted} of {total_lines} lines ...]\n\n"); - - // Byte budgets for head/tail around the marker - let mut head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES); - let tail_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(head_budget + marker.len()); - if tail_budget == 0 && marker.len() >= MODEL_FORMAT_MAX_BYTES { - // Degenerate case: marker alone exceeds budget; return a clipped marker - return take_bytes_at_char_boundary(&marker, MODEL_FORMAT_MAX_BYTES).to_string(); - } - if tail_budget == 0 { - // Make room for the marker by shrinking head - head_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(marker.len()); - } - - // Enforce line-count cap by trimming head/tail lines - let head_lines_text = head_block; - let tail_lines_text = tail_block; - // Build final string respecting byte budgets - let head_part = take_bytes_at_char_boundary(&head_lines_text, head_budget); - let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(s.len())); - - result.push_str(head_part); - result.push_str(&marker); - - let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len()); - let tail_budget_final = remaining; - let tail_part = take_last_bytes_at_char_boundary(&tail_lines_text, tail_budget_final); - result.push_str(tail_part); - - result -} - -// Truncate a &str to a byte budget at a char boundary (prefix) -#[inline] -fn take_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { - if s.len() <= maxb { - return s; - } - let mut last_ok = 0; - for (i, ch) in s.char_indices() { - let nb = i + ch.len_utf8(); - if nb > maxb { - break; - } - last_ok = nb; - } - &s[..last_ok] -} - -// Take a suffix of a &str within a byte budget at a char boundary -#[inline] -fn take_last_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { - if s.len() <= maxb { - return s; - } - let mut start = s.len(); - let mut used = 0usize; - for (i, ch) in s.char_indices().rev() { - let nb = ch.len_utf8(); - if used + nb > maxb { - break; - } - start = i; - used += nb; - if start == 0 { - break; - } - } - &s[start..] -} - -/// Exec output is a pre-serialized JSON payload -fn format_exec_output(exec_output: &ExecToolCallOutput) -> String { - let ExecToolCallOutput { - exit_code, - duration, - .. - } = exec_output; - - #[derive(Serialize)] - struct ExecMetadata { - exit_code: i32, - duration_seconds: f32, - } - - #[derive(Serialize)] - struct ExecOutput<'a> { - output: &'a str, - metadata: ExecMetadata, - } - - // round to 1 decimal place - let duration_seconds = ((duration.as_secs_f32()) * 10.0).round() / 10.0; - - let formatted_output = format_exec_output_str(exec_output); - - let payload = ExecOutput { - output: &formatted_output, - metadata: ExecMetadata { - exit_code: *exit_code, - duration_seconds, - }, - }; - - #[expect(clippy::expect_used)] - serde_json::to_string(&payload).expect("serialize ExecOutput") -} - pub(super) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -> Option { responses.iter().rev().find_map(|item| { if let ResponseItem::Message { role, content, .. } = item { @@ -2665,6 +2389,7 @@ mod tests { use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; + use crate::tools::handle_container_exec_with_params; use codex_protocol::mcp_protocol::AuthMode; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index dd4aef3d60..08c78fb9f5 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -1,12 +1,12 @@ -use std::borrow::Cow; -use std::collections::HashMap; -use std::path::PathBuf; use codex_otel::otel_event_manager::OtelEventManager; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ShellToolCallParams; use codex_protocol::protocol::FileChange; use mcp_types::CallToolResult; +use std::borrow::Cow; +use std::collections::HashMap; +use std::path::PathBuf; use crate::codex::Session; use crate::codex::TurnContext; @@ -92,7 +92,6 @@ impl ToolOutput { } } - #[derive(Clone, Debug)] pub(crate) struct ExecCommandContext { pub(crate) sub_id: String, @@ -108,4 +107,4 @@ pub(crate) struct ExecCommandContext { pub(crate) struct ApplyPatchCommandContext { pub(crate) user_explicitly_approved_this_action: bool, pub(crate) changes: HashMap, -} \ No newline at end of file +} diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 4a38de4ef1..72daff6485 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -1,15 +1,42 @@ use std::collections::HashMap; -use async_trait::async_trait; - +use crate::apply_patch; +use crate::apply_patch::ApplyPatchExec; +use crate::apply_patch::InternalApplyPatchInvocation; +use crate::apply_patch::convert_apply_patch_to_protocol; +use crate::codex::ApplyPatchCommandContext; +use crate::codex::ExecCommandContext; +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::error::CodexErr; +use crate::error::SandboxErr; use crate::exec::ExecParams; +use crate::exec::ExecToolCallOutput; +use crate::exec::StdoutStream; +use crate::executor::ExecutionMode; +use crate::executor::errors::ExecError; +use crate::executor::linkers::PreparedExec; use crate::function_tool::FunctionCallError; +use crate::tools::{handle_container_exec_with_params, MODEL_FORMAT_HEAD_BYTES}; +use crate::tools::MODEL_FORMAT_HEAD_LINES; +use crate::tools::MODEL_FORMAT_MAX_BYTES; +use crate::tools::MODEL_FORMAT_MAX_LINES; +use crate::tools::MODEL_FORMAT_TAIL_LINES; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::spec::ApplyPatchToolArgs; +use crate::turn_diff_tracker::TurnDiffTracker; +use async_trait::async_trait; +use codex_apply_patch::MaybeApplyPatchVerified; +use codex_apply_patch::maybe_parse_apply_patch_verified; +use codex_protocol::protocol::AskForApproval; +use codex_utils_string::take_bytes_at_char_boundary; +use codex_utils_string::take_last_bytes_at_char_boundary; +use serde::Serialize; +use tracing::trace; pub struct ApplyPatchHandler; @@ -66,7 +93,7 @@ impl ToolHandler for ApplyPatchHandler { justification: None, }; - let content = crate::codex::handle_container_exec_with_params( + let content = handle_container_exec_with_params( tool_name.as_str(), exec_params, session, diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 93249f8873..80596807a6 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -1,4 +1,4 @@ -mod apply_patch; +pub mod apply_patch; mod exec_stream; mod mcp; mod plan; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 5e98a79add..907f193fe1 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -8,6 +8,7 @@ use crate::function_tool::FunctionCallError; 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::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -62,7 +63,7 @@ impl ToolHandler for ShellHandler { )) })?; let exec_params = Self::to_exec_params(params, turn); - let content = crate::codex::handle_container_exec_with_params( + let content = handle_container_exec_with_params( tool_name.as_str(), exec_params, session, @@ -79,7 +80,7 @@ impl ToolHandler for ShellHandler { } ToolPayload::LocalShell { params } => { let exec_params = Self::to_exec_params(params, turn); - let content = crate::codex::handle_container_exec_with_params( + let content = handle_container_exec_with_params( tool_name.as_str(), exec_params, session, diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index ee78cafdfa..23c5d459b8 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -4,4 +4,256 @@ pub mod registry; pub mod router; pub mod spec; +use serde::Serialize; +use tracing::trace; +use codex_apply_patch::{maybe_parse_apply_patch_verified, MaybeApplyPatchVerified}; +use codex_protocol::protocol::AskForApproval; +use codex_utils_string::{take_bytes_at_char_boundary, take_last_bytes_at_char_boundary}; pub use router::Router; +use crate::apply_patch; +use crate::apply_patch::{convert_apply_patch_to_protocol, ApplyPatchExec, InternalApplyPatchInvocation}; +use crate::codex::{ApplyPatchCommandContext, ExecCommandContext, Session, TurnContext}; +use crate::error::{CodexErr, SandboxErr}; +use crate::exec::{ExecParams, ExecToolCallOutput, StdoutStream}; +use crate::executor::errors::ExecError; +use crate::executor::ExecutionMode; +use crate::executor::linkers::PreparedExec; +use crate::function_tool::FunctionCallError; +use crate::turn_diff_tracker::TurnDiffTracker; + +// Model-formatting limits: clients get full streams; oonly content sent to the model is truncated. +pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB +pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines +pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; +pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 +pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; + + +pub(crate) async fn handle_container_exec_with_params( + tool_name: &str, + params: ExecParams, + sess: &Session, + turn_context: &TurnContext, + turn_diff_tracker: &mut TurnDiffTracker, + sub_id: String, + call_id: String, +) -> Result { + let otel_event_manager = turn_context.client.get_otel_event_manager(); + + if params.with_escalated_permissions.unwrap_or(false) + && !matches!(turn_context.approval_policy, AskForApproval::OnRequest) + { + return Err(FunctionCallError::RespondToModel(format!( + "approval policy is {policy:?}; reject command — you should not ask for escalated permissions if the approval policy is {policy:?}", + policy = turn_context.approval_policy + ))); + } + + // check if this was a patch, and apply it if so + let apply_patch_exec = match maybe_parse_apply_patch_verified(¶ms.command, ¶ms.cwd) { + MaybeApplyPatchVerified::Body(changes) => { + match apply_patch::apply_patch(sess, turn_context, &sub_id, &call_id, changes).await { + InternalApplyPatchInvocation::Output(item) => return item, + InternalApplyPatchInvocation::DelegateToExec(apply_patch_exec) => { + Some(apply_patch_exec) + } + } + } + MaybeApplyPatchVerified::CorrectnessError(parse_error) => { + // It looks like an invocation of `apply_patch`, but we + // could not resolve it into a patch that would apply + // cleanly. Return to model for resample. + return Err(FunctionCallError::RespondToModel(format!( + "error: {parse_error:#?}" + ))); + } + MaybeApplyPatchVerified::ShellParseError(error) => { + trace!("Failed to parse shell command, {error:?}"); + None + } + MaybeApplyPatchVerified::NotApplyPatch => None, + }; + + let command_for_display = if let Some(exec) = apply_patch_exec.as_ref() { + vec!["apply_patch".to_string(), exec.action.patch.clone()] + } else { + params.command.clone() + }; + + let exec_command_context = ExecCommandContext { + sub_id: sub_id.clone(), + call_id: call_id.clone(), + command_for_display: command_for_display.clone(), + cwd: params.cwd.clone(), + apply_patch: apply_patch_exec.as_ref().map( + |ApplyPatchExec { + action, + user_explicitly_approved_this_action, + }| ApplyPatchCommandContext { + user_explicitly_approved_this_action: *user_explicitly_approved_this_action, + changes: convert_apply_patch_to_protocol(action), + }, + ), + tool_name: tool_name.to_string(), + otel_event_manager, + }; + + let mode = match apply_patch_exec { + Some(exec) => ExecutionMode::ApplyPatch(exec), + None => ExecutionMode::Shell, + }; + + sess.services.executor.update_environment( + turn_context.sandbox_policy.clone(), + turn_context.cwd.clone(), + ); + + let prepared_exec = PreparedExec::new( + exec_command_context, + params, + command_for_display, + mode, + Some(StdoutStream { + sub_id: sub_id.clone(), + call_id: call_id.clone(), + tx_event: sess.tx_event.clone(), + }), + turn_context.shell_environment_policy.use_profile, + ); + + let output_result = sess + .run_exec_with_events( + turn_diff_tracker, + prepared_exec, + turn_context.approval_policy, + ) + .await; + + match output_result { + Ok(output) => { + let ExecToolCallOutput { exit_code, .. } = &output; + let content = format_exec_output_apply_patch(&output); + if *exit_code == 0 { + Ok(content) + } else { + Err(FunctionCallError::RespondToModel(content)) + } + } + 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:?}" + ))), + } +} + +pub fn format_exec_output_apply_patch(exec_output: &ExecToolCallOutput) -> String { + let ExecToolCallOutput { + exit_code, + duration, + .. + } = exec_output; + + #[derive(Serialize)] + struct ExecMetadata { + exit_code: i32, + duration_seconds: f32, + } + + #[derive(Serialize)] + struct ExecOutput<'a> { + output: &'a str, + metadata: ExecMetadata, + } + + // round to 1 decimal place + let duration_seconds = ((duration.as_secs_f32()) * 10.0).round() / 10.0; + + let formatted_output = format_exec_output_str(exec_output); + + let payload = ExecOutput { + output: &formatted_output, + metadata: ExecMetadata { + exit_code: *exit_code, + duration_seconds, + }, + }; + + #[expect(clippy::expect_used)] + serde_json::to_string(&payload).expect("serialize ExecOutput") +} + +pub fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { + let ExecToolCallOutput { + aggregated_output, .. + } = exec_output; + + // Head+tail truncation for the model: show the beginning and end with an elision. + // Clients still receive full streams; only this formatted summary is capped. + + let mut s = &aggregated_output.text; + let prefixed_str: String; + + if exec_output.timed_out { + prefixed_str = format!( + "command timed out after {} milliseconds\n", + exec_output.duration.as_millis() + ) + s; + s = &prefixed_str; + } + + let total_lines = s.lines().count(); + if s.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES { + return s.to_string(); + } + + let lines: Vec<&str> = s.lines().collect(); + let head_take = MODEL_FORMAT_HEAD_LINES.min(lines.len()); + let tail_take = MODEL_FORMAT_TAIL_LINES.min(lines.len().saturating_sub(head_take)); + let omitted = lines.len().saturating_sub(head_take + tail_take); + + // Join head and tail blocks (lines() strips newlines; reinsert them) + let head_block = lines + .iter() + .take(head_take) + .cloned() + .collect::>() + .join("\n"); + let tail_block = if tail_take > 0 { + lines[lines.len() - tail_take..].join("\n") + } else { + String::new() + }; + let marker = format!("\n[... omitted {omitted} of {total_lines} lines ...]\n\n"); + + // Byte budgets for head/tail around the marker + let mut head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES); + let tail_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(head_budget + marker.len()); + if tail_budget == 0 && marker.len() >= MODEL_FORMAT_MAX_BYTES { + // Degenerate case: marker alone exceeds budget; return a clipped marker + return take_bytes_at_char_boundary(&marker, MODEL_FORMAT_MAX_BYTES).to_string(); + } + if tail_budget == 0 { + // Make room for the marker by shrinking head + head_budget = MODEL_FORMAT_MAX_BYTES.saturating_sub(marker.len()); + } + + // Enforce line-count cap by trimming head/tail lines + let head_lines_text = head_block; + let tail_lines_text = tail_block; + // Build final string respecting byte budgets + let head_part = take_bytes_at_char_boundary(&head_lines_text, head_budget); + let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(s.len())); + + result.push_str(head_part); + result.push_str(&marker); + + let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len()); + let tail_budget_final = remaining; + let tail_part = take_last_bytes_at_char_boundary(&tail_lines_text, tail_budget_final); + result.push_str(tail_part); + + result +} \ No newline at end of file diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 372b652605..403f5a5c78 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -10,7 +10,6 @@ use crate::tools::spec::ToolSpec; use crate::tools::spec::ToolsConfig; use crate::tools::spec::build_specs; use crate::turn_diff_tracker::TurnDiffTracker; -use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::LocalShellAction; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 6a2351f727..42814b1b57 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -505,7 +505,7 @@ pub(crate) fn build_specs( if config.experimental_unified_exec_tool { specs.push(create_unified_exec_tool()); - builder.register_handler("unified_exec", unified_exec_handler.clone()); + builder.register_handler("unified_exec", unified_exec_handler); } else { match &config.shell_type { ConfigShellToolType::Default => { @@ -522,7 +522,7 @@ pub(crate) fn build_specs( create_write_stdin_tool_for_responses_api(), )); builder.register_handler(EXEC_COMMAND_TOOL_NAME, exec_stream_handler.clone()); - builder.register_handler(WRITE_STDIN_TOOL_NAME, exec_stream_handler.clone()); + builder.register_handler(WRITE_STDIN_TOOL_NAME, exec_stream_handler); } } } @@ -530,11 +530,11 @@ pub(crate) fn build_specs( // Always register shell aliases so older prompts remain compatible. builder.register_handler("shell", shell_handler.clone()); builder.register_handler("container.exec", shell_handler.clone()); - builder.register_handler("local_shell", shell_handler.clone()); + builder.register_handler("local_shell", shell_handler); if config.plan_tool { specs.push(PLAN_TOOL.clone()); - builder.register_handler("update_plan", plan_handler.clone()); + builder.register_handler("update_plan", plan_handler); } if let Some(apply_patch_tool_type) = &config.apply_patch_tool_type { @@ -546,7 +546,7 @@ pub(crate) fn build_specs( specs.push(create_apply_patch_json_tool()); } } - builder.register_handler("apply_patch", apply_patch_handler.clone()); + builder.register_handler("apply_patch", apply_patch_handler); } if config.web_search_request { @@ -555,7 +555,7 @@ pub(crate) fn build_specs( if config.include_view_image_tool { specs.push(create_view_image_tool()); - builder.register_handler("view_image", view_image_handler.clone()); + builder.register_handler("view_image", view_image_handler); } if let Some(mcp_tools) = mcp_tools { diff --git a/codex-rs/utils/string/Cargo.toml b/codex-rs/utils/string/Cargo.toml new file mode 100644 index 0000000000..698c4b2f6f --- /dev/null +++ b/codex-rs/utils/string/Cargo.toml @@ -0,0 +1,7 @@ +[package] +edition.workspace = true +name = "codex-utils-string" +version.workspace = true + +[lints] +workspace = true diff --git a/codex-rs/utils/string/src/lib.rs b/codex-rs/utils/string/src/lib.rs new file mode 100644 index 0000000000..f7299d4372 --- /dev/null +++ b/codex-rs/utils/string/src/lib.rs @@ -0,0 +1,38 @@ +// Truncate a &str to a byte budget at a char boundary (prefix) +#[inline] +pub fn take_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { + if s.len() <= maxb { + return s; + } + let mut last_ok = 0; + for (i, ch) in s.char_indices() { + let nb = i + ch.len_utf8(); + if nb > maxb { + break; + } + last_ok = nb; + } + &s[..last_ok] +} + +// Take a suffix of a &str within a byte budget at a char boundary +#[inline] +pub fn take_last_bytes_at_char_boundary(s: &str, maxb: usize) -> &str { + if s.len() <= maxb { + return s; + } + let mut start = s.len(); + let mut used = 0usize; + for (i, ch) in s.char_indices().rev() { + let nb = ch.len_utf8(); + if used + nb > maxb { + break; + } + start = i; + used += nb; + if start == 0 { + break; + } + } + &s[start..] +}