Skip to content

fix(codex): resolve forks of turnless parents instead of retrying forever - #1578

Merged
mariusvniekerk merged 3 commits into
kenn-io:mainfrom
dqtz5vpvj9-create:fix/codex-empty-fork-parent
Sep 3, 2026
Merged

fix(codex): resolve forks of turnless parents instead of retrying forever#1578
mariusvniekerk merged 3 commits into
kenn-io:mainfrom
dqtz5vpvj9-create:fix/codex-empty-fork-parent

Conversation

@dqtz5vpvj9-create

@dqtz5vpvj9-create dqtz5vpvj9-create commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Codex Desktop writes a rollout the moment a thread opens, so a fork taken before the first prompt names a parent that holds only session_meta and settings events. The fork gate treated a parent with no turn ids the same way as a parent it could not find: the child was stamped DataVersionNeedsRetry, re-parsed on every pass, and never reached the current data version. Because a retry counts as a failure in collectAndBatch, one such fork on a reconciliation page aborted that page and every page behind it. A local archive carried six of these forks in one chain, every parent present and readable, and logged the identical abort every fifteen seconds for a day; the deferred-source work in #1490 stops the abort from spreading but leaves those forks permanently deferred.

codexParentResolution now treats any parent that can be read as resolved, whether or not it holds turn ids. A turnless parent never opened a turn, so the fork replayed nothing and the gate has nothing to drop; with an empty parent turn set the gate suppresses nothing either way, so parsing the child as current is the only change. A parent that cannot be read still defers the child, exactly as before.

An earlier revision of this PR kept a turnless parent blocking when the child carried a copied parent session_meta, on the theory that a replay prefix always begins with that copied meta. A survey of a large local Codex archive disproved it: about half of the forks there, including many written by Codex 0.147.0, replay parent turns with no copied parent meta at all. That gate and its test are gone, which is why the net diff is small.

Reviewers should look at codexParentResolution in internal/parser/codex.go and TestCodexProviderTurnlessParentResolvesWithoutRetry, which parses a fork of a meta-only parent and expects a current result with only the child's messages.

Closes #1582

🤖 Generated with Claude Code

https://claude.ai/code/session_0158DH4fPMj7ogzQdPmyRqUZ

…ever

Codex Desktop writes a rollout the moment a thread opens, so a fork taken
before the first prompt names a parent that holds only session_meta and
settings events. The fork gate treated a parent with no turn ids the same
as a parent it could not find: the child was marked DataVersionNeedsRetry
and re-parsed on every pass, and because a retry counts as a failure, one
such fork on a reconciliation page aborted the page and every page behind
it. A local archive carried six of these forks, all with readable parents,
and logged the same abort every fifteen seconds for a day.

A parent that was read but never opened a turn is resolved: the fork
replayed no history, so the gate has nothing to drop and a retry could
never learn more. Only a parent that cannot be read still defers the
child.
@roborev-ci

roborev-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (c657d4a)

Verdict: Two medium-severity issues were identified in Codex replay parsing; another reviewer found no security concerns.

Medium

  • internal/parser/codex.go:1655-1666, 1677-1682 — An empty turn-ID map may represent an incomplete scan, including a valid unterminated final turn_context, rather than a genuinely turnless parent. Treating errCodexIncrementalNeedsFullParse as resolved disables retry and preserves replayed parent messages and token usage. Distinguish these cases and retain retry behavior when turn history cannot be determined.

  • internal/parser/codex.go:215-219 — Clearing only forkGate.active leaves parentResolved set to true. A copied parent session_meta can therefore reactivate the gate and discard child records before or without a new turn_context. Reset the resolved/replay state or introduce an explicit turnless-parent state that prevents reactivation.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 2m58s

…nt blocks

A readable parent with no turn ids does not always mean the fork replayed
nothing: the parent may have written turns the fork copied before its own
file flushed them. The child rollout tells the two apart, because a replay
prefix always begins with the copied parent session_meta. Resolve the fork
as current when that copied meta is absent, and keep deferring it until
the parent advances when it is present, which the added test pins down.
@roborev-ci

roborev-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown

roborev: Combined Review (b062e16)

Verdict: Medium-severity replay-gating bug found in Codex transcript parsing.

Medium

  • internal/parser/codex.go:1682 — A turnless parent without copied metadata is marked current, but the replay gate remains active with an empty turn set. For valid transcripts containing response_item records without turn_context, this suppresses all child messages. Deactivate the gate for this case or pass the scan result into gate initialization, and add a regression test.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 4m52s

…ta gate

The copied-meta rule from the previous commit assumed that a replayed
parent prefix always starts with the parent's copied session_meta, so
its absence proved the fork replayed nothing. Local Codex archives show
otherwise: on Codex 0.147.0, the reporter's version, forks routinely
replay parent turns with no copied parent session_meta at all. About
half of the forks in one archive have no copied meta, and most of those
still carry parent turn ids. The copied meta is not a replay signal.

Without that signal the extra branch only guarded a window of a few
milliseconds between Codex writing a turn to the parent rollout and a
fork copying it, and with zero known parent turn ids the fork gate
suppresses nothing on either branch. So the rule collapses to the first
commit's intent: a parent that can be read is settled, a parent that
cannot be read defers the child. The full-file scan for the copied meta
and the test that pinned the disproven premise are removed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0158DH4fPMj7ogzQdPmyRqUZ
@roborev-ci

roborev-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown

roborev: Combined Review (7680c46)

Verdict: Medium-severity issue found; reviewers did not fully agree the code is clean.

Medium

  • internal/parser/codex.go:1677 — Readable parents with no turn IDs are treated as resolved even when their snapshots may be stale. A child fork can therefore import replayed parent turns as duplicate child messages and token usage. Preserve a deferred/unstable state for empty scans or verify parent snapshot completeness; add a regression test.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 2m19s

@mariusvniekerk

Copy link
Copy Markdown
Collaborator

The medium finding on 7680c46 (internal/parser/codex.go:1677, readable parent with no turn ids may be a stale snapshot) is invalid. Its failure path needs a fork to replay parent turns that the parent's rollout file does not yet contain, and Codex forks read their history from that file.

From upstream openai/codex main:

  • fork_thread in codex-rs/core/src/thread_manager.rs loads history through initial_history_from_rollout_path, which reads the thread by rollout path with include_history: true.
  • attach_history_if_requested in codex-rs/thread-store/src/local/read_thread.rs fills that history with load_history_items(&path) from the parent rollout file on disk.
  • The subagent fork path calls ensure_rollout_materialized and flush_rollout on the parent first ("Persist queued rollout updates before reading the fork snapshot").

So the replayed prefix is a copy of what was already on disk, and rollout files are append-only. A parent with zero turn_context lines at fork time produces a child with zero replayed turns; nothing can later be duplicated.

Two more points:

  • A survey of a local archive of 12,343 Codex rollouts (versions 0.133 through 0.152) found 73 turnless rollouts, none with a user message, and 303 forks, none with a turnless parent. No observed instance of the described state.
  • The prior len(turnIDs) > 0 check was never a completeness check. A parent with one flushed turn out of five was already "resolved" and only that turn was dropped. The zero-turn case is not special, and deferring it again would only restore the forever-retry this PR removes.

Caveat: the source read is current main; older tags were not checked. The archive survey covers those versions and shows no counterexample.

generated by a clanker

@mariusvniekerk
mariusvniekerk merged commit 8be33f1 into kenn-io:main Sep 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Codex fork of a turnless parent is marked NeedsRetry forever

2 participants