codex_exec: apply_patch deletes lose file content in rollout (exec starts threads in Limited persistence mode)
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,mainate3f44ca3b(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) vscodex_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.rsclassifiesEventMsg::PatchApplyEndunderEventPersistenceMode::Extended. InLimitedmode it is filtered out before being written to rollout.codex-rs/tui/src/app_server_session.rspassespersist_extended_history: trueonthread/start,thread/resume, andthread/forkrequests, so TUI rollouts run in Extended mode andPatchApplyEndis preserved.codex-rs/exec/src/lib.rsinthread_start_params_from_configbuildsThreadStartParamswith..ThreadStartParams::default(), which leavespersist_extended_historyat itsbooldefault offalse(the field was added as an experimental opt-in in #11227). Exec rollouts therefore run inLimitedmode and dropPatchApplyEndunconditionally.
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_historyonly controls the on-disk rollout filter (EventPersistenceMode, seecodex-rs/rollout/src/policy.rsandcodex-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
- Reclassify
EventMsg::PatchApplyEndasLimitedinrollout/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 (ExecCommandEndetc.) unresolved. If the long-term desire is to tightenLimitedfor specific events, that's a separate architectural decision. - Enrich
ResponseItem::CustomToolCallat rollout write time with a resolvedchangesstructure. 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. - Change the
*** Begin PatchDSL 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 existingapply_patch/resume/ephemeralsuites.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 emitsevent_msg/patch_apply_endwithchanges[<path>].contentpopulated.
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_historyflag 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 forkimplementation also setspersist_extended_history: truefor 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.
5 Comments
Thanks for reporting this. There is a divergence between
execand TUI behavior here. This is because of a recent regression (introduced by me) wherepersist_extended_history: truewas unintentionally added for the TUI. I'll reverse that change so it mirrors its previous behavior and is consistent with theexcebehavior.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: trueis reverted on TUI, neither originator will carry deleted-file content anywhere in the rollout, because the*** Begin PatchDSL's*** Delete File: <path>is path-only by design and the rawresponse_item/custom_tool_calltherefore 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:
EventMsg::PatchApplyEnddirectly — either via the app-server or thecodex exec --jsonevent stream — instead of reading rollout files post-hoc. If that's the intended surface, is it considered stable enough to build against?persist_extended_historywas aiming at, but positioned as a deliberate audit feature rather than an implementation detail.*** Delete File:to carry the file body (symmetric with*** Add File), so the model-ownedcustom_tool_callis 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.
On closer inspect, it looks like
persist_extended_historyis 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 thepersist_extended_historyflag to understand why this difference in behavior was originally introduced.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:
PatchApplyEnd,ExecCommandEnd, andMcpToolCallEnd. If the concern is rollout size, scoping by event type rather than one big mode might be an alternative worth flagging.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.
I discussed this with my teammates, and we've decided to switch all clients to use
persist_extended_history: false. The problem withpersist_extended_history: trueis 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.