[codex] Remove legacy shell output formatting paths#22706
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60433a4b65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| input | ||
| self.input.clone() |
There was a problem hiding this comment.
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 👍 / 👎.
| } | ||
|
|
||
| input | ||
| self.input.clone() |
There was a problem hiding this comment.
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 👍 / 👎.
pakrym-oai
changed the title
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69615a9fd8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
2 participants
Why
The client and tool pipeline still carried compatibility code for legacy structured shell output. Current shell and apply_patch responses are already plain text for model consumption, so keeping a JSON-serialization path plus shell-item rewrite logic makes the request formatter and tests preserve a format we do not need anymore.
What Changed
core/src/client_common.rs.freeformswitch so tool emitters use one model-facing formatter.ApplyPatchModelOutput::ShellCommandViaHeredoccompatibility input shape, but no longer treats it as a separate output-format mode.Validation
cargo test -p codex-core client_commoncargo test -p codex-core shell_serializationcargo test -p codex-core apply_patch_clijust fix -p codex-coreDocumentation
No external Codex documentation update is needed.