rollout: own turn lifecycle replay - #29725
Closed
anp-oai wants to merge 2 commits into
Closed
Conversation
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
2 times, most recently
from
June 23, 2026 21:50
ecfa6f4 to
4bb96c6
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
2 times, most recently
from
June 23, 2026 22:04
4513560 to
1060c61
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 23, 2026 22:04
4bb96c6 to
4bc1eb0
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 23, 2026 22:21
1060c61 to
477f370
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 23, 2026 22:21
4bc1eb0 to
087929d
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 23, 2026 22:53
477f370 to
583cc81
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 23, 2026 22:53
087929d to
251071f
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 23, 2026 23:36
583cc81 to
063c4e1
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 23, 2026 23:36
251071f to
f925fca
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 00:30
063c4e1 to
ef724d0
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 00:30
f925fca to
4afbe9d
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 00:46
ef724d0 to
eb612f1
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 00:46
4afbe9d to
fe000bb
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 03:13
eb612f1 to
2424e1f
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 03:13
fe000bb to
c99a024
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 04:06
2424e1f to
3590cf4
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 04:06
c99a024 to
09e933f
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 05:09
3590cf4 to
1550250
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 05:10
09e933f to
817c284
Compare
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
from
June 24, 2026 19:00
1550250 to
a8788d4
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 19:00
817c284 to
45f10b6
Compare
Contributor
Author
|
@codex review |
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45f10b61f3
ℹ️ 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".
anp-oai
force-pushed
the
codex/split-mcp-elicitation-types
branch
2 times, most recently
from
June 24, 2026 20:42
4b7cb7b to
88479f3
Compare
anp-oai
added a commit
that referenced
this pull request
Jun 24, 2026
## Why Core and tools need to request MCP elicitation without constructing app-server wire payloads. The request should remain a neutral protocol concept until app-server serializes it for a client. ## What changed - Switched core and tools to `codex_protocol::approvals::ElicitationRequest`. - Derived turn and server context inside core instead of carrying app-server request types through lower layers. - Kept the app-server payload unchanged through an explicit boundary conversion. - Removed the remaining production app-server-protocol dependency from tools. ## Stack This is PR 5 of 6, stacked on [PR #29723](#29723). Review only the delta from `codex/split-connector-metadata-types`. Next: [PR #29725](#29725). ## Validation - `codex-core` MCP coverage passed: 87 tests. - Tools elicitation and app-server round-trip coverage passed.
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 20:59
45f10b6 to
d33a0b4
Compare
anp-oai
force-pushed
the
codex/split-rollout-lifecycle
branch
from
June 24, 2026 21:32
d33a0b4 to
9285b7b
Compare
Contributor
Author
|
@codex review |
Contributor
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
jacks0n
pushed a commit
to jacks0n/codex
that referenced
this pull request
Jul 6, 2026
## Why Core and tools need to request MCP elicitation without constructing app-server wire payloads. The request should remain a neutral protocol concept until app-server serializes it for a client. ## What changed - Switched core and tools to `codex_protocol::approvals::ElicitationRequest`. - Derived turn and server context inside core instead of carrying app-server request types through lower layers. - Kept the app-server payload unchanged through an explicit boundary conversion. - Removed the remaining production app-server-protocol dependency from tools. ## Stack This is PR 5 of 6, stacked on [PR openai#29723](openai#29723). Review only the delta from `codex/split-connector-metadata-types`. Next: [PR openai#29725](openai#29725). ## Validation - `codex-core` MCP coverage passed: 87 tests. - Tools elicitation and app-server round-trip coverage passed.
Contributor
Author
|
just going to remake this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Core needs turn lifecycle replay when resuming, forking, and truncating rollouts, but that logic currently comes from app-server's history reducer. Rollout replay should not require an app-server wire dependency.
What changed
RolloutTurnLifecycleTrackerincodex-rollout.Stack
This is PR 6 of 6, stacked on PR #29724. Review only the delta from
codex/split-mcp-elicitation-types.Validation
just test -p codex-rollout: 86 tests passed.just argument-comment-lint: all 690 Bazel targets passed.just fmtcodex-corethread-manager coverage passed: 24 tests.