Skip to content

[codex] Remove legacy shell output formatting paths #22706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pakrym-oai wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[codex] Remove legacy shell output formatting paths #22706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60433a4
Remove legacy shell output rewrite
pakrym-oai May 14, 2026
69615a9
Unify exec output formatting
pakrym-oai May 14, 2026
f093561
codex: fix CI failure on PR #22706
pakrym-oai May 15, 2026
3593e45
Merge remote-tracking branch 'origin/main' into pakrym/delete-shell-r…
pakrym-oai May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 1 addition & 110 deletions codex-rs/core/src/client_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@ pub use codex_api::ResponseEvent;
use codex_config::types::Personality;
use codex_protocol::error::Result;
use codex_protocol::models::BaseInstructions;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::ResponseItem;
use codex_tools::ToolSpec;
use futures::Stream;
use serde::Deserialize;
use serde_json::Value;
use std::collections::HashSet;
use std::pin::Pin;
use std::task::Context;
use std::task::Poll;
Expand Down Expand Up @@ -64,116 +61,10 @@ impl Default for Prompt {

impl Prompt {
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
let mut input = self.input.clone();

// when using the *Freeform* apply_patch tool specifically, tool outputs
// should be structured text, not json. Do NOT reserialize when using
// the Function tool - note that this differs from the check above for
// instructions. We declare the result as a named variable for clarity.
let is_freeform_apply_patch_tool_present = self.tools.iter().any(|tool| match tool {
ToolSpec::Freeform(f) => f.name == "apply_patch",
_ => false,
});
if is_freeform_apply_patch_tool_present {
reserialize_shell_outputs(&mut input);
}

input
self.input.clone()
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve freeform apply_patch output formatting

When the freeform apply_patch tool runs, ToolEmitter::ApplyPatch still falls through to format_exec_output_for_model_structured in codex-rs/core/src/tools/events.rs, so the tool result is initially a JSON string containing output and metadata. This removed formatting pass was the only conversion before the next Responses request; in the existing shell_serialization coverage for ApplyPatchModelOutput::Freeform, the captured custom_tool_call_output is expected to begin with Exit code: 0, but this clone now sends the raw JSON wrapper to the model after any successful patch.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep formatting legacy shell history on resume

For resumed conversations that already contain legacy local_shell_call/shell outputs serialized as {"output":...,"metadata":...}, the old pass converted those historical tool results back to the plain Exit code/Wall time text whenever a current freeform apply_patch tool was present. Cloning the history here now sends the raw JSON wrapper back to the model for those older transcripts, even though context_manager/normalize.rs still preserves local-shell calls and their FunctionCallOutput pairs for prompt history, so users resuming pre-freeform-shell sessions get a different and less readable model context.

Useful? React with 👍 / 👎.

Comment thread
pakrym-oai marked this conversation as resolved.
}
}

fn reserialize_shell_outputs(items: &mut [ResponseItem]) {
let mut shell_call_ids: HashSet<String> = HashSet::new();

items.iter_mut().for_each(|item| match item {
ResponseItem::LocalShellCall { call_id, id, .. } => {
if let Some(identifier) = call_id.clone().or_else(|| id.clone()) {
shell_call_ids.insert(identifier);
}
}
ResponseItem::CustomToolCall {
id: _,
status: _,
call_id,
name,
input: _,
} => {
if name == "apply_patch" {
shell_call_ids.insert(call_id.clone());
}
}
ResponseItem::FunctionCall { name, call_id, .. }
if is_shell_tool_name(name) || name == "apply_patch" =>
{
shell_call_ids.insert(call_id.clone());
}
ResponseItem::FunctionCallOutput {
call_id, output, ..
}
| ResponseItem::CustomToolCallOutput {
call_id, output, ..
} => {
if shell_call_ids.remove(call_id)
&& let Some(structured) = output
.text_content()
.and_then(parse_structured_shell_output)
{
output.body = FunctionCallOutputBody::Text(structured);
}
}
_ => {}
})
}

fn is_shell_tool_name(name: &str) -> bool {
name == "shell"
}

#[derive(Deserialize)]
struct ExecOutputJson {
output: String,
metadata: ExecOutputMetadataJson,
}

#[derive(Deserialize)]
struct ExecOutputMetadataJson {
exit_code: i32,
duration_seconds: f32,
}

fn parse_structured_shell_output(raw: &str) -> Option<String> {
let parsed: ExecOutputJson = serde_json::from_str(raw).ok()?;
Some(build_structured_output(&parsed))
}

fn build_structured_output(parsed: &ExecOutputJson) -> String {
let mut sections = Vec::new();
sections.push(format!("Exit code: {}", parsed.metadata.exit_code));
sections.push(format!(
"Wall time: {} seconds",
parsed.metadata.duration_seconds
));

let mut output = parsed.output.clone();
if let Some((stripped, total_lines)) = strip_total_output_header(&parsed.output) {
sections.push(format!("Total output lines: {total_lines}"));
output = stripped.to_string();
}

sections.push("Output:".to_string());
sections.push(output);

sections.join("\n")
}

fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
let after_prefix = output.strip_prefix("Total output lines: ")?;
let (total_segment, remainder) = after_prefix.split_once('\n')?;
let total_lines = total_segment.parse::<u32>().ok()?;
let remainder = remainder.strip_prefix('\n').unwrap_or(remainder);
Some((remainder, total_lines))
}

pub struct ResponseStream {
pub(crate) rx_event: mpsc::Receiver<Result<ResponseEvent>>,
/// Signals the mapper task that the consumer stopped polling before the
Expand Down
63 changes: 0 additions & 63 deletions codex-rs/core/src/client_common_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use codex_api::ResponsesApiRequest;
use codex_api::TextControls;
use codex_api::create_text_param_for_request;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::models::FunctionCallOutputPayload;
use pretty_assertions::assert_eq;

use super::*;
Expand Down Expand Up @@ -166,65 +165,3 @@ fn serializes_flex_service_tier_when_set() {
Some("flex")
);
}

#[test]
fn reserializes_shell_outputs_for_function_and_custom_tool_calls() {
let raw_output = r#"{"output":"hello","metadata":{"exit_code":0,"duration_seconds":0.5}}"#;
let expected_output = "Exit code: 0\nWall time: 0.5 seconds\nOutput:\nhello";
let mut items = vec![
ResponseItem::FunctionCall {
id: None,
name: "shell".to_string(),
namespace: None,
arguments: "{}".to_string(),
call_id: "call-1".to_string(),
},
ResponseItem::FunctionCallOutput {
call_id: "call-1".to_string(),
output: FunctionCallOutputPayload::from_text(raw_output.to_string()),
},
ResponseItem::CustomToolCall {
id: None,
status: None,
call_id: "call-2".to_string(),
name: "apply_patch".to_string(),
input: "*** Begin Patch".to_string(),
},
ResponseItem::CustomToolCallOutput {
call_id: "call-2".to_string(),
name: None,
output: FunctionCallOutputPayload::from_text(raw_output.to_string()),
},
];

reserialize_shell_outputs(&mut items);

assert_eq!(
items,
vec![
ResponseItem::FunctionCall {
id: None,
name: "shell".to_string(),
namespace: None,
arguments: "{}".to_string(),
call_id: "call-1".to_string(),
},
ResponseItem::FunctionCallOutput {
call_id: "call-1".to_string(),
output: FunctionCallOutputPayload::from_text(expected_output.to_string()),
},
ResponseItem::CustomToolCall {
id: None,
status: None,
call_id: "call-2".to_string(),
name: "apply_patch".to_string(),
input: "*** Begin Patch".to_string(),
},
ResponseItem::CustomToolCallOutput {
call_id: "call-2".to_string(),
name: None,
output: FunctionCallOutputPayload::from_text(expected_output.to_string()),
},
]
);
}
16 changes: 2 additions & 14 deletions codex-rs/core/src/tools/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ pub(crate) enum ToolEmitter {
cwd: AbsolutePathBuf,
source: ExecCommandSource,
parsed_cmd: Vec<ParsedCommand>,
freeform: bool,
},
ApplyPatch {
changes: HashMap<PathBuf, FileChange>,
Expand All @@ -132,19 +131,13 @@ pub(crate) enum ToolEmitter {
}

impl ToolEmitter {
pub fn shell(
command: Vec<String>,
cwd: AbsolutePathBuf,
source: ExecCommandSource,
freeform: bool,
) -> Self {
pub fn shell(command: Vec<String>, cwd: AbsolutePathBuf, source: ExecCommandSource) -> Self {
let parsed_cmd = parse_command(&command);
Self::Shell {
command,
cwd,
source,
parsed_cmd,
freeform,
}
}

Expand Down Expand Up @@ -328,12 +321,7 @@ impl ToolEmitter {
output: &ExecToolCallOutput,
ctx: ToolEventCtx<'_>,
) -> String {
match self {
Self::Shell { freeform: true, .. } => {
super::format_exec_output_for_model_freeform(output, ctx.turn.truncation_policy)
}
_ => super::format_exec_output_for_model_structured(output, ctx.turn.truncation_policy),
}
super::format_exec_output_for_model(output, ctx.turn.truncation_policy)
}

pub async fn finish(
Expand Down
9 changes: 1 addition & 8 deletions codex-rs/core/src/tools/handlers/shell.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ struct RunExecLikeArgs {
turn: Arc<TurnContext>,
tracker: crate::tools::context::SharedTurnDiffTracker,
call_id: String,
freeform: bool,
shell_runtime_backend: ShellRuntimeBackend,
}

Expand All @@ -68,7 +67,6 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
turn,
tracker,
call_id,
freeform,
shell_runtime_backend,
} = args;

Expand Down Expand Up @@ -162,12 +160,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
}

let source = ExecCommandSource::Agent;
let emitter = ToolEmitter::shell(
exec_params.command.clone(),
exec_params.cwd.clone(),
source,
freeform,
);
let emitter = ToolEmitter::shell(exec_params.command.clone(), exec_params.cwd.clone(), source);
let event_ctx = ToolEventCtx::new(
session.as_ref(),
turn.as_ref(),
Expand Down
1 change: 0 additions & 1 deletion codex-rs/core/src/tools/handlers/shell/shell_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ impl ToolExecutor<ToolInvocation> for ShellCommandHandler {
turn,
tracker,
call_id,
freeform: true,
shell_runtime_backend: self.shell_runtime_backend(),
})
.await
Expand Down
42 changes: 1 addition & 41 deletions codex-rs/core/src/tools/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use codex_utils_output_truncation::TruncationPolicy;
use codex_utils_output_truncation::formatted_truncate_text;
use codex_utils_output_truncation::truncate_text;
pub use router::ToolRouter;
use serde::Serialize;

// Telemetry preview limits: keep log events smaller than model budgets.
pub(crate) const TELEMETRY_PREVIEW_MAX_BYTES: usize = 2 * 1024; // 2 KiB
Expand All @@ -50,46 +49,7 @@ pub(crate) fn flat_tool_name(tool_name: &ToolName) -> Cow<'_, str> {

/// Format the combined exec output for sending back to the model.
/// Includes exit code and duration metadata; truncates large bodies safely.
pub fn format_exec_output_for_model_structured(
exec_output: &ExecToolCallOutput,
truncation_policy: TruncationPolicy,
) -> 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, truncation_policy);

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_for_model_freeform(
pub fn format_exec_output_for_model(
exec_output: &ExecToolCallOutput,
truncation_policy: TruncationPolicy,
) -> String {
Expand Down
15 changes: 0 additions & 15 deletions codex-rs/core/tests/common/responses.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ use wiremock::http::HeaderValue;
use wiremock::matchers::method;
use wiremock::matchers::path_regex;

use crate::test_codex::ApplyPatchModelOutput;

#[derive(Debug, Clone)]
pub struct ResponseMock {
requests: Arc<Mutex<Vec<ResponsesRequest>>>,
Expand Down Expand Up @@ -883,19 +881,6 @@ pub fn ev_local_shell_call(call_id: &str, status: &str, command: Vec<&str>) -> V
})
}

pub fn ev_apply_patch_call(
call_id: &str,
patch: &str,
output_type: ApplyPatchModelOutput,
) -> Value {
match output_type {
ApplyPatchModelOutput::Freeform => ev_apply_patch_custom_tool_call(call_id, patch),
ApplyPatchModelOutput::ShellCommandViaHeredoc => {
ev_apply_patch_shell_command_call_via_heredoc(call_id, patch)
}
}
}

/// Convenience: SSE event for an `apply_patch` custom tool call with raw patch
/// text. This mirrors the payload produced by the Responses API when the model
/// invokes `apply_patch` directly.
Expand Down
Loading
Loading