Cap Codex session display when weekly quota is exhausted#1930
Conversation
|
Codex review: needs changes before merge. Reviewed July 6, 2026, 3:53 AM ET / 07:53 UTC. Summary Reproducibility: yes. From source, a persisted Codex widget entry with session and weekly windows both exhausted and both resetsAt values <= now still maps row.window.remainingPercent to 0 before the weekly-cap guard returns inactive rows. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the weekly-cap display behavior after widget rows share the menu-bar projection’s reset-aware handling for expired persisted quota windows. Do we have a high-confidence way to reproduce the issue? Yes. From source, a persisted Codex widget entry with session and weekly windows both exhausted and both resetsAt values <= now still maps row.window.remainingPercent to 0 before the weekly-cap guard returns inactive rows. Is this the best way to solve the issue? No, not yet. The projection and menu-bar path are narrow, but the widget row path still needs reset-aware filtering for expired saved windows before this is the best maintainable fix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 76a63dc55939. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a952efd50
ℹ️ 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".
| guard self.weeklyCapsSession(weekly: weekly, now: now), | ||
| session.remainingPercent > 0 |
There was a problem hiding this comment.
Use weekly reset even when session is already empty
When the weekly bucket is exhausted and the 5-hour bucket is also already at 0%, this extra session.remainingPercent > 0 guard returns the raw session window, so the session row keeps showing the 5-hour reset instead of the later weekly reset even though the weekly cap is still binding. Users who have depleted both buckets will still see the misleading short reset this change is trying to avoid; the weekly reset should be substituted whenever weeklyCapsSession is true, not only when the session has remaining quota.
Useful? React with 👍 / 👎.
| resetsAt: weekly?.resetsAt ?? session.resetsAt, | ||
| resetDescription: weekly?.resetDescription ?? session.resetDescription, |
There was a problem hiding this comment.
Avoid showing a 5h reset when weekly reset is unknown
When the Codex CLI/parser gives an exhausted weekly window without reset metadata, weeklyCapsSession still treats it as binding, but this fallback copies the session reset into the synthetic capped row. In that scenario the UI shows 0% session quota with the 5-hour reset time, implying the cap clears in 5h even though the weekly reset is unknown; the synthetic window should leave reset metadata unknown unless it can use the weekly reset.
Useful? React with 👍 / 👎.
| private static func weeklyCapsSession(weekly: RateWindow?, now: Date) -> Bool { | ||
| guard let weekly else { return false } | ||
| guard weekly.remainingPercent <= 0 else { return false } | ||
| return weekly.resetsAt.map { $0 > now } ?? true |
There was a problem hiding this comment.
Compare the weekly reset against render time
This expiry check depends entirely on Context.now, but several display paths build Codex projections with snapshot.updatedAt rather than the current render time (for example the icon resolver and widget rows). If the snapshot was captured before the weekly reset and no successful refresh lands afterward, $0 > now stays true for those paths, so the session/icon/widget remain capped at 0 even after the weekly reset has passed; display projections need to evaluate this cap against the current timeline/render time.
Useful? React with 👍 / 👎.
|
Addressed the review feedback in 417b8b2:
Added regressions for the both-exhausted, unknown-weekly-reset, and stale-snapshot cases. |
When the weekly lane is the binding cap, show 0% session remaining and the weekly reset time so the menu card and menu bar do not imply usable 5-hour quota. Co-authored-by: Cursor <cursoragent@cursor.com>
Retarget the session reset whenever the weekly cap is active, omit the 5h reset when weekly reset metadata is missing, and evaluate cap expiry against render time. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
417b8b2 to
6c136ef
Compare
Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c136ef4f4
ℹ️ 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".
| WidgetUsageRow( | ||
| id: row.id, | ||
| title: row.title, | ||
| percentLeft: row.window?.remainingPercent ?? row.percentLeft) |
There was a problem hiding this comment.
Stop rendering expired Codex widget quotas
When the widget reloads from a persisted Codex snapshot after reset times have passed, this still renders each row from the raw row.window?.remainingPercent. If both the session and weekly buckets were exhausted in the saved snapshot and both resetsAt values have elapsed while the app has not written a fresh snapshot, the full/switcher widgets keep showing both quotas at 0%; applyingCodexWeeklyCap only handles an active weekly cap and then returns these stale zeros. Filter or ignore expired exhausted windows here, matching the menu-bar projection's reset-aware behavior.
Useful? React with 👍 / 👎.
|
Maintainer proof for exact head
Validation used deterministic fixtures only; no real provider, browser, Keychain, or account mutation. |
Summary
CodexConsumerProjectionnow shows the session lane as 0% remaining and uses the weekly reset time instead of a misleading 99% / 5-hour countdown.secondaryGloballyCapsPrimary).Upstream Codex UI (misleading)
CodexBar after this fix
Upstream report: openai/codex#31214
Test plan
swift test --filter CodexConsumerProjectionTestsmake check