Skip to content

fix(server): fork Codex threads at the native turn boundary - #11490

Open
saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:work/ov2-20260913-02
Open

fix(server): fork Codex threads at the native turn boundary#11490
saphid wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
saphid:work/ov2-20260913-02

Conversation

@saphid

@saphid saphid commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • CodexAdapterV2.forkThread forked the full native Codex thread and then issued thread/rollback to reach the selected turn. Newer Codex versions default to paginated durable threads, which reject thread/rollback, so retry/edit of an earlier turn created the fork but failed the following provider turn.
  • The adapter now resolves the selected provider turn's nativeTurnRef.nativeId and passes it as lastTurnId on the original thread/fork request. Codex's inclusive boundary semantics exclude later turns atomically, so no rollback is issued on that path.
  • The historical fork-then-rollback fallback is kept only where it is valid: when no native turn reference is available, and only on forks whose historyMode is legacy. A paginated fallback fork now fails with an explicit ProviderAdapterForkThreadError instead of sending a request Codex rejects.
  • rollbackThread now probes historyMode first and returns an honest ProviderAdapterRollbackThreadError for paginated threads rather than issuing an unsupported thread/rollback. Legacy-thread rollback is unchanged.

Focused proof

  • Real replay fixtures updated and driven through the actual orchestration path: thread_fork_native (latest-turn boundary), thread_fork_native_prior_turn (inclusive earlier boundary, later source turns absent from the emitted fork, no rollback frames), thread_merge_back_continue / thread_merge_back_siblings (boundary on merge-back forks), and thread_rollback (history-mode probe + preserved legacy rollback).
  • New unit tests cover boundary resolution: latest/prior turn, missing nativeId, missing provider turn, missing source-turn history, and turn-not-in-source.
  • New replay-level tests cover paginated fallback fork failure, paginated direct rollback failure, and fork-request error propagation.
  • vp test on the touched suites: 84 passed / 1 skipped across CodexAdapterV2.test.ts (71), ThreadFork (7), ThreadMergeBack (4), CodexReplayFixtures (3); broader replay/contract suites previously green (81). Typecheck exit 0.

Credit

Adapts the approach first proposed in #9169 by @Yusuf007R, including the lastTurnId boundary and updated fork fixtures, reworked for the current adapter shape with the paginated-history probes added.

Work performed by SWE-2 (Devin) in T3 Code.
Coordination trace: thread 5c350af2-1c4e-4243-9aa9-de40f39b548d, campaign https://github.com/saphid/t3code-personal/issues/298

Retry/edit of an earlier turn in a paginated native Codex thread forked at
head and then issued thread/rollback, which paginated threads reject. Resolve
the selected provider turn nativeTurnRef and pass it as lastTurnId to
thread/fork so later turns are excluded atomically and no rollback is needed.
Fall back to fork-then-rollback only when no native reference is available,
and fail honestly when that fallback hits a paginated fork. rollbackThread now
probes historyMode and reports an explicit unsupported error for paginated
threads instead of sending a request Codex will reject.

Adapts the approach from pingdotgg#9169 by Yusuf007R.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 13, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ebe77e1

Macroscope's review found this PR approvable — The production change is confined to the Codex adapter and corrects existing fork-boundary behavior using the provider’s native turn ID, with legacy and paginated-history cases explicitly covered by tests. Other modifications are test fixtures or documentation, with no schema, deployment, security, billing, or product-default impact.

You can add or adjust custom eligibility rules. Learn more.

@saphid

saphid commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

CI note for reviewers: the three failing checks are pre-existing on the base branch, not caused by this diff.

  • Check: knip flags apps/server/scripts/verify-background-live.ts (untouched here) as unused.
  • Test: apps/mobile/src/state/thread-order.test.ts pending-move tests — this PR touches only server orchestration and docs.
  • Release Smoke: vp install --lockfile-only --ignore-scripts infrastructure failure.

Check and Test fail identically on run 34659363738 for an unrelated PR head (e03e376a3) targeting the same base. All server test shards, Rust, and Macroscope are green on this head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant