[codex] Preserve reviewer when resuming threads - #30278
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9cdee51c7
ℹ️ 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".
| if typesafe_overrides.approvals_reviewer.is_some() | ||
| || request_overrides.is_some_and(|overrides| overrides.contains_key("approvals_reviewer")) |
There was a problem hiding this comment.
This guard assumes Some means the resume request explicitly overrode the reviewer, but the codex exec resume path always fills ThreadResumeParams.approvals_reviewer from the resolved config (codex-rs/exec/src/lib.rs:1094, via a helper that always returns Some at 1144-1147). As a result, resuming an auto-review thread through exec with the current config/default set to user still exits here before reading the persisted rollout reviewer, preserving the exact silent switch this change is meant to prevent for that resume surface.
AGENTS.md reference: AGENTS.md:L102-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 933c9b6. codex exec resume now sends approvals_reviewer only when the caller explicitly supplies that override. Ordinary resumes leave it unset, allowing app-server to restore the reviewer persisted in the rollout. Added a regression test covering both ordinary resume and an explicit reviewer override.
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
4bcca3b to
7132b09
Compare
Keep acknowledged settings persistence from creating a rollout before the first user turn while retaining it for resumable threads.\n\nCo-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Co-authored-by: Codex noreply@openai.com
Why
A thread resumed without an explicit reviewer could pick up the reviewer from the current config instead of preserving the reviewer already in use by the thread. After an app restart, this meant a thread running with auto review could silently switch back to user review, and the next turn could continue under the wrong reviewer.
What changed
Persist the effective reviewer with each turn and restore the latest persisted value when the thread resumes. If the resume request explicitly provides a reviewer, that value still takes precedence.
Test plan
just test -p codex-protocoljust test -p codex-statejust test -p codex-rolloutjust test -p codex-app-server thread_resume_preserves_persisted_approvals_reviewer