[codex] rename rollout budget error to session budget error - #29744
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f838ee5543
ℹ️ 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".
| pub enum CodexErrorInfo { | ||
| ContextWindowExceeded, | ||
| RolloutBudgetExceeded, | ||
| SessionBudgetExceeded, |
There was a problem hiding this comment.
When resuming or reading a rollout written before this rename, the persisted ErrorEvent still contains "codex_error_info":"rollout_budget_exceeded"; after this enum variant rename there is no serde(alias = "rollout_budget_exceeded"), so RolloutRecorder::load_rollout_items fails to deserialize that JSONL line and skips it. In the budget-exhaustion path an Error event is followed by TurnComplete, so dropping only the error makes the rebuilt thread history look completed instead of failed and hides the original budget error; keep the new serialized name, but accept the old one for rollout compatibility.
AGENTS.md reference: AGENTS.md:L103-L111
Useful? React with 👍 / 👎.
| pub enum CodexErrorInfo { | ||
| ContextWindowExceeded, | ||
| RolloutBudgetExceeded, | ||
| SessionBudgetExceeded, |
There was a problem hiding this comment.
For app-server v2 clients that already switch on the documented codexErrorInfo string, this changes the server-emitted value from rolloutBudgetExceeded to sessionBudgetExceeded; those clients will miss the budget-exhaustion classification even though the rest of the error/turn payload shape is unchanged. If this is meant to be a Rust naming cleanup, keep the old wire spelling with a targeted serde rename (or otherwise version the API change) so existing app-server consumers continue to classify this error.
AGENTS.md reference: AGENTS.md:L103-L111
Useful? React with 👍 / 👎.
Summary
RolloutBudgetExceededtoSessionBudgetExceededsessionBudgetExceededThis is a naming-only follow-up to #29715 based on Pavel's review suggestion. Runtime behavior is unchanged.
Tests
just test -p codex-core rollout_budgetjust test -p codex-app-server-protocoljust fmtjust write-app-server-schema