Repository navigation
Conversation
Let providers advertise native forks while a later source turn is running. Codex supports completed fork boundaries in that state, so retain its native history at dispatch and startup instead of creating a portable handoff. Providers without this capability keep the existing fallback. Co-Authored-By: GPT-6.1 Sol at high effort <noreply@openai.com> Co-Authored-By: GPT-6.1 Sol at xhigh effort, review subagent for busy Codex forks <noreply@openai.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
vouch:unvouched
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — Busy Codex forks will automatically preserve native history instead of using portable context summaries. Human confirmation is required for this change to the default fork behavior. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Fork capability contract and Codex declaration packages/contracts/src/orchestrationV2.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts |
The thread capability schema adds optional canForkWhileRunning. Codex advertises this capability. |
Fork selection, turn-start handling, and tests apps/server/src/orchestration-v2/CommandPolicy.ts, apps/server/src/orchestration-v2/Orchestrator.ts, apps/server/src/orchestration-v2/ProviderTurnStartService.ts, apps/server/src/orchestration-v2/CommandPolicy.test.ts, apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts |
For a busy source, the policy selects a native fork only when the provider supports forking while running and a native turn boundary exists. Otherwise, the turn-start path uses portable context. Tests cover idle and busy sources, capability values, and native, synthetic, or missing boundaries. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant Orchestrator
participant CommandPolicyV2
participant ProviderTurnStartService
participant CodexSession
Orchestrator->>CommandPolicyV2: Pass source busy state and native turn boundary
CommandPolicyV2->>ProviderTurnStartService: Select native fork or portable context
ProviderTurnStartService->>CodexSession: Fork thread when capability and native boundary permit
Suggested reviewers: t3dotgg, juliusmarminge
Merge Risk: 
c38a7
-
The busy-fork boundary is now checked before native forking. No actionable merge-blocking risk remains from the supplied evidence.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the server fix and the main behavior change: preserving native history for busy Codex forks. Description check The description includes all required sections. It explains the problem, change, scope, approval status, detailed verification, baseline failures, and affected checks. Maintainer approval remains pend… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes all required sections. It explains the problem, change, scope, approval status, detailed verification, baseline failures, and affected checks. Maintainer approval remains pending, but the description documents that status clearly.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Requesting maintainer confirmation on the intended Codex behavior. #26901 explicitly notes that Codex forks of a running source previously forked natively, before the generic busy-source fallback was added for providers that cannot fork a running session. Codex's app-server supports copying history through a completed turn while a later turn remains active. The isolated runtime check in this PR verified that the selected history is retained, the later turn is excluded, and the parent continues running. This PR restores that native Codex path while preserving the portable fallback for other providers. Please confirm whether restoring native history for busy Codex forks at completed boundaries is the approved direction before merging. |
![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: 0ebd6b2a5b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep the portable fallback when a completed source turn has no native turn reference, including synthetic goal replies. Require that boundary at both dispatch and startup so a running parent cannot add later history to the fork. Co-Authored-By: GPT-6.1 Sol at high effort <noreply@openai.com>
Problem
Forking a completed Codex turn while a later turn runs in the parent silently replaces the native conversation with a bounded portable handoff. The busy-source guard introduced in #26901 applies to every provider, although Codex can natively fork through a completed turn while the source remains active. In the reported case the handoff selected five conversation items and omitted 41, and did not carry native reasoning or tool state.
Reproduction: finish a Codex turn, start another request in the parent, fork from the completed turn, and send a request in the fork before the parent finishes.
Change
Add an optional provider capability for native forks while the source runs, enable it for Codex, and consult it both when dispatch plans the fork and when startup rechecks the source. Busy native forks also require the selected turn's native reference, so synthetic replies such as
/goaland missing cursors keep the portable fallback. Providers and older capability snapshots without explicit support keep their existing fallback; idle behavior stays unchanged.Scope and approval
This is a focused restoration of the existing native-fork workflow lost in #26901, with no new user setting or workflow. Following Macroscope's approvability review, maintainer confirmation of the intended Codex behavior has been requested; that approval remains pending.
Verification
Before the fix, the policy and startup regression cases both failed: dispatch selected
portable_context, and startup never called the native fork. The idle-source case passed.Official Codex app-server documentation permits a completed
lastTurnIdwhile the source is mid-turn; only an in-progress explicit boundary is rejected.Native runtime check on Windows with Codex 0.166.0, a disposable Codex home and workspace, and a local mock Responses server (no real model calls):
The review follow-up reproduced all three missing-boundary cases on
0ebd6b2: dispatch with no native boundary, startup with a synthetic turn, and startup with no provider turn. After correction,vp test run src/orchestration-v2/CommandPolicy.test.ts src/orchestration-v2/ProviderTurnStartService.test.ts -t 'forks a busy source|planned native side chat fork'fromapps/serverpassed all nine selected cases. The native positive case also asserts that the selected provider turn ID is passed to the fork.Restored every changed file to unchanged upstream
47f2c8d29e98and ranvp test run src/orchestration-v2/ProviderTurnStartService.test.ts -t 'recovers the local or pushed thread tip|preserves recreation interrupts|coordinates provider startup'— the same four failures reproduced: recreated worktree ENOENT, an interruption reported as success, and the two startup/cleanup ordering assertions. Restored the committed fix afterward and verified a clean worktree. These failures are not introduced by this PR.vp run check:changed upstream/mainonc38a7d4d9f— formatting, lint, both unused-code checks, GraphQL, and all 35 affected package typechecks passed. The selected tests had 218 passing and only the same four confirmed baseline Windows failures, so the overall command exited 1. No checks or assertions were weakened.Implemented with GPT-6.1 Sol at high effort in the Codex harness; the initial patch was independently reviewed with GPT-6.1 Sol at xhigh effort through T3 Code. Codex and CodeRabbit's boundary findings were reproduced and corrected in the follow-up.