codex_exec: apply_patch deletes lose file content in rollout (exec starts threads in Limited persistence mode)

Resolved 💬 5 comments Opened Apr 19, 2026 by moqimoqidea Closed Apr 30, 2026
💡 Likely answer: A maintainer (etraut-openai, contributor) responded on this thread — see the highlighted reply below.

Summary

When codex_exec (the non-interactive CLI path) executes an apply_patch that deletes a file, the rollout file written to ~/.codex/sessions/<year>/<month>/<day>/rollout-<id>.jsonl retains only the model's raw DSL (*** Delete File: <path>), which by design carries no file body. The TUI originator does not have this problem because TUI threads are started with persist_extended_history: true, which keeps EventMsg::PatchApplyEnd — the event that carries structured FileChange::Delete { content }.

As a result, downstream consumers that read rollout files (audit tooling, code-change exporters, compliance logs) can see that a file was deleted under exec, but not what was deleted.

Environment

  • Repo: openai/codex, main at e3f44ca3b (Fix plugin cache panic when cwd is unavailable, #18499)
  • Platform: observed on macOS (Darwin 25.4.0), reproduces anywhere rollout files are written
  • Codex originators compared: codex-tui (OK) vs codex_exec (broken)

Reproduction

Ask the same Codex model, under each originator, to create and delete a small file, then compare rollout JSONL.

originator: codex-tui — complete

{"type":"event_msg","payload":{
  "type":"patch_apply_end",
  "success":true,
  "changes":{
    "foo.txt":{"type":"delete","content":"hello\nworld\n"}
  }
}}

FileChange::Delete { content: String } carries the full file body — downstream audit readers can consume it directly.

originator: codex_exec — incomplete

{"type":"response_item","payload":{
  "type":"custom_tool_call",
  "name":"apply_patch",
  "call_id":"call_abc",
  "input":"*** Begin Patch\n*** Delete File: foo.txt\n*** End Patch\n"
}}

*** Delete File: in the *** Begin Patch DSL is a path-only hunk — it was never required to ship content. The paired custom_tool_call_output.exit_code === 0 confirms the delete succeeded, but the removed bytes are gone from the on-disk log.

| Scenario | Expected | Actual |
|---|---|---|
| tui + delete | rollout records deleted content | works |
| tui + add/update | rollout records full change | works |
| exec + delete | rollout records deleted content | empty |
| exec + add | *** Add File carries + lines | works |
| exec + update | *** Update File carries hunks | works |

Root Cause

The divergence is not in the apply_patch handler. The handler reads and attaches the deleted file's content on both paths — see codex-rs/apply-patch/src/invocation.rs where the Hunk::DeleteFile branch calls fs.read_file_text(&path, sandbox).await and stores the bytes on ApplyPatchFileChange::Delete { content }. convert_apply_patch_to_protocol (in codex-rs/core/src/apply_patch.rs) then preserves that content into FileChange::Delete { content }. The emitter issues a EventMsg::PatchApplyEnd carrying the structured changes map — in memory the exec path already has the content.

The divergence is in the rollout persistence policy:

  • codex-rs/rollout/src/policy.rs classifies EventMsg::PatchApplyEnd under EventPersistenceMode::Extended. In Limited mode it is filtered out before being written to rollout.
  • codex-rs/tui/src/app_server_session.rs passes persist_extended_history: true on thread/start, thread/resume, and thread/fork requests, so TUI rollouts run in Extended mode and PatchApplyEnd is preserved.
  • codex-rs/exec/src/lib.rs in thread_start_params_from_config builds ThreadStartParams with ..ThreadStartParams::default(), which leaves persist_extended_history at its bool default of false (the field was added as an experimental opt-in in #11227). Exec rollouts therefore run in Limited mode and drop PatchApplyEnd unconditionally.

The user-visible effect: exec rollouts retain only the raw response_item/custom_tool_call (which is always persisted), and that raw record cannot carry delete content because the *** Begin Patch DSL doesn't require it.

Proposed fix (minimal)

Opt codex_exec into Extended rollout persistence at thread start, matching TUI. This is a one-line change in codex-rs/exec/src/lib.rs:

 fn thread_start_params_from_config(config: &Config) -> ThreadStartParams {
     ThreadStartParams {
         model: config.model.clone(),
         model_provider: Some(config.model_provider_id.clone()),
         cwd: Some(config.cwd.to_string_lossy().to_string()),
         approval_policy: Some(config.permissions.approval_policy.value().into()),
         approvals_reviewer: approvals_reviewer_override_from_config(config),
         sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()),
         config: config_request_overrides_from_config(config),
         ephemeral: Some(config.ephemeral),
+        // Match TUI so non-interactive runs keep structured apply_patch/exec
+        // events in rollout (e.g. `FileChange::Delete { content }`), which
+        // audit tooling reads via `EventMsg::PatchApplyEnd`.
+        persist_extended_history: true,
         ..ThreadStartParams::default()
     }
 }

Plus a regression unit test in codex-rs/exec/src/lib_tests.rs that pins thread_start_params_from_config(&config).persist_extended_history == true, following the same ConfigBuilder pattern used by the existing thread_start_params_include_review_policy_* tests.

Side note: #17568's implementation proposal for codex exec fork already specifies persist_extended_history: true for its fork params — this PR would put start/resume on the same footing as that planned fork path, rather than leaving the three branches inconsistent.

Why this is safe

  • persist_extended_history only controls the on-disk rollout filter (EventPersistenceMode, see codex-rs/rollout/src/policy.rs and codex-rs/core/src/session/session.rs). It does not alter what events are emitted on exec's event stream or what the model sees.
  • The additional variants preserved under Extended (ExecCommandEnd, McpToolCallEnd, PatchApplyEnd, GuardianAssessment, Error, WebSearchEnd, ViewImageToolCall, Collab*End, DynamicToolCallRequest/Response) are already being written by TUI sessions today; rollout readers on the resume/fork/thread-export paths already handle them.
  • Rollout files grow only with tool-call volume, not per-token overhead. Exec runs are typically short and bounded.

Alternatives considered

  1. Reclassify EventMsg::PatchApplyEnd as Limited in rollout/src/policy.rs. Smaller persistence footprint, but it picks favorites among Extended-only events and leaves sibling "exec rollout is lossy" reports for other events (ExecCommandEnd etc.) unresolved. If the long-term desire is to tighten Limited for specific events, that's a separate architectural decision.
  2. Enrich ResponseItem::CustomToolCall at rollout write time with a resolved changes structure. Fixes Limited-mode consumers directly but enlarges the rollout schema, introduces a filesystem read on the rollout write path, creates two sources of truth for the same data, and risks divergence if the file has already been removed by the time the write happens. Disproportionate for this bug.
  3. Change the *** Begin Patch DSL to require full content on *** Delete File:. Biggest prompt-contract and back-compat change; rejected.

Option 1 via the exec-side flag (as proposed) is the smallest change that matches what exec consumers typically want: lossless rollouts for resume/fork/export, same as TUI.

Verification

Reference branch (on my fork, ready for review if a PR invitation is extended): https://github.com/moqimoqidea/codex/tree/fix/codex-exec-persist-extended-history

  • cargo test -p codex-exec: 40 lib + 1 doc + 61 integration tests all pass, including the new regression test and the existing apply_patch / resume / ephemeral suites.
  • cargo clippy -p codex-exec --all-targets --no-deps: clean.
  • Manual repro on the branch: the same prompt that previously yielded only *** Delete File: ... in rollout now also emits event_msg/patch_apply_end with changes[<path>].content populated.

Impact

This directly affects anyone running audit / observability / compliance tooling on top of codex rollout files under exec — per-run statistics like "lines deleted" or "content removed by model" are currently underreported for exec but not for TUI, producing inconsistent numbers for the same underlying operation.

Related

  • #11227 — introduced the experimental persist_extended_history flag for app-server clients that want lossless thread reconstruction.
  • #14884 — requests a TUI-side config to toggle persist_extended_history. Complementary but orthogonal: that issue is about TUI user control, this one is about exec's default.
  • #17568 — proposed codex exec fork implementation also sets persist_extended_history: true for fork params. Aligning start/resume makes all three exec thread-entry points consistent.

Happy to share additional repro details, a diff, or more data from our downstream audit tooling if useful. If the team is open to the direction, I'd be glad to prepare an invited PR.

View original on GitHub ↗

5 Comments

etraut-openai contributor · 4 months ago

Thanks for reporting this. There is a divergence between exec and TUI behavior here. This is because of a recent regression (introduced by me) where persist_extended_history: true was unintentionally added for the TUI. I'll reverse that change so it mirrors its previous behavior and is consistent with the exce behavior.

moqimoqidea · 4 months ago

Thanks for the quick triage — that clarifies the direction and I appreciate you flagging the regression context.

Just to raise the concrete downstream gap so I can figure out the right next step: once persist_extended_history: true is reverted on TUI, neither originator will carry deleted-file content anywhere in the rollout, because the *** Begin Patch DSL's *** Delete File: <path> is path-only by design and the raw response_item/custom_tool_call therefore never contains the removed bytes. Audit/compliance tooling built on top of rollout files can then tell that a file was deleted but not what was deleted — for both TUI and exec.

To make sure I'm not pointing our tooling at a surface the project doesn't plan to stabilize, could you point me at the recommended path for this use case? A few options we've been weighing, in increasing order of change:

  1. Live event channel: subscribe to EventMsg::PatchApplyEnd directly — either via the app-server or the codex exec --json event stream — instead of reading rollout files post-hoc. If that's the intended surface, is it considered stable enough to build against?
  2. Opt-in, separate audit log: a distinct, explicitly-named mode (or a sidecar file) for "lossless tool-call history" that doesn't conflate with the normal rollout schema. Essentially what persist_extended_history was aiming at, but positioned as a deliberate audit feature rather than an implementation detail.
  3. DSL change: require *** Delete File: to carry the file body (symmetric with *** Add File), so the model-owned custom_tool_call is self-contained and the rollout is lossless by construction. Biggest change, but the only one that doesn't rely on Codex reading disk at apply time.

Happy to help explore any of these if you have a preferred direction — just want to avoid building another pass on a surface the team isn't planning to keep. Thanks again for the fast response.

etraut-openai contributor · 4 months ago

On closer inspect, it looks like persist_extended_history is set to true for our GUI-based clients (the IDE Extension and Codex app), and it's not immediately clear why there should be any difference in rollout behavior across clients. Before making any change here, I'm going to check with my colleague who originally introduced the persist_extended_history flag to understand why this difference in behavior was originally introduced.

moqimoqidea · 4 months ago

Thank you — really appreciate you pausing to loop in the original author before changing anything. With the IDE Extension, Codex app, and (pre-regression) TUI all defaulting to persist_extended_history: true, exec looking like the outlier matches what we see from the downstream side: our audit tooling has been reading TUI rollouts as a lossless surface for a while with no observed issues from the extra event lines.

A couple of things that might help ground the conversation with your colleague, happy to expand on any:

  • From the consumer side, the Extended-only events our downstream tooling actually depends on are PatchApplyEnd, ExecCommandEnd, and McpToolCallEnd. If the concern is rollout size, scoping by event type rather than one big mode might be an alternative worth flagging.
  • If the flag was originally framed as "for clients that want lossless resume/fork" versus "for audit observers", that framing difference might be exactly what's worth re-examining — audit readers care about historical fidelity regardless of resume behavior.

No rush on our side — we'll hold the downstream tooling in place until the team lands somewhere. Please feel free to come back with whatever conclusion you reach; happy to help validate either direction. Thanks again for handling this carefully.

etraut-openai contributor · 3 months ago

I discussed this with my teammates, and we've decided to switch all clients to use persist_extended_history: false. The problem with persist_extended_history: true is that it significantly increases the size of rollouts in memory and on disk. We've received feedback from many users that they want Codex to consume less memory and disk space, and this is an easy way to get some big gains here.