Publish Codex quota before optional enrichment - #2799
Conversation
9b9fa19 to
7c5a763
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed August 9, 2026, 7:12 PM ET / 23:12 UTC. ClawSweeper reviewWhat this changesThis PR lets an open Codex usage card display refreshed core quota before optional Credits and OpenAI dashboard enrichment completes, while retaining refresh state for incompatible layouts. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 4 items remain Keep open: the supplied current-head context resolves the earlier layout and test-cleanup findings, but the PR still lacks after-fix real CodexBar menu-card proof. Priority: P2 Review scores
Verification
How this fits togetherCodexBar refreshes provider quota into an already-open macOS menu card. The refresh monitor freezes a card while provider work runs, then chooses whether a newly resolved model can update the hosted card or needs a final menu rebuild. flowchart LR
A[Manual refresh] --> B[Codex quota fetch]
B --> C[Refresh monitor]
C --> D{Layout compatible?}
D -->|Yes| E[Update open menu card]
D -->|No| F[Final menu rebuild]
F --> E
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the compatibility-gated early publication and add redacted fresh-bundle evidence showing an open Codex card update before optional enrichment completes. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main begins the provider refresh before quota retrieval and clears the monitor only after the optional Codex enrichment sequence completes. Is this the best way to solve the issue? Yes, conditionally publishing only a layout-compatible core model is the narrowest path; real application proof is still needed to confirm the live-card result. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a0d77c37f9cc. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
![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: 9f0712d2de
ℹ️ 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".
| account: account, | ||
| updater: DisabledUpdaterController(), | ||
| preferencesSelection: PreferencesSelection(), | ||
| statusBar: .system) |
There was a problem hiding this comment.
This test only checks async sequencing and MenuCardRefreshMonitor, but passing .system constructs a real status item and the test never calls releaseStatusItemsForTesting(). On headless macOS test workers this can leave AppKit resources and run-loop activity alive, causing the focused test to hang; cover the behavior through a stable state seam, or use the repository cleanup wrapper if the controller is indispensable.
AGENTS.md reference: AGENTS.md:L27-L27
Useful? React with 👍 / 👎.
| // before its optional credits and OpenAI Web enrichment stages below, matching the all-provider | ||
| // foreground/background split used by the full refresh path. | ||
| if provider == .codex { | ||
| self.menuCardRefreshMonitor.endManualRefresh(for: provider) |
There was a problem hiding this comment.
When the open Codex card was created without quota, or the refreshed snapshot changes its metric layout, MenuCardRefreshMonitor.model rejects the newly resolved model because it is incompatible with the card's tracked layout and continues returning the original fallback. Clearing the monitor here therefore removes the “Refreshing…” indicator without publishing the new quota; the card remains empty or stale until the later dashboard enrichment completes and triggers the final menu rebuild. Reconcile the card at this boundary, or retain the refresh state when the new quota cannot be adopted live.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
9f0712d to
1a4a1d4
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Landed as Proof:
Caveat: none in validation. The squash commit preserves Yuxin Qiao's contributor credit with the requested |
2 participants
Summary
Root cause
Provider-scoped Codex refreshes kept the card frozen through core usage, status, token cost, Credits, and dashboard work. The original patch ended that freeze after core usage, but
MenuCardRefreshMonitorcorrectly rejects a live model whose tracked layout differs from the already-hosted card. Clearing the monitor unconditionally therefore removed “Refreshing…” while leaving the old or empty fallback visible.The monitor now owns the early-publication decision. It ends the provider-scoped loading state only when the frozen and resolved models have compatible tracked layouts, retains the published core model through the enrichment tail, and leaves incompatible cards frozen until the controller’s existing final reconciliation. Provider/account authority and the optional enrichment sequence are unchanged.
The original sequencing fixture also retained its system status item and blocked every Credits retry. It now uses the cleanup wrapper and deliberately blocks only the first enrichment call, allowing legitimate account-reconciliation retries to finish.
Synthetic behavior proof
No provider credentials, network access, or personal data were used. A compiled production-controller seam emitted this deterministic operator trace:
The three trace cases passed. A screenshot probe was not added because producing a signed visual fixture for these synthetic states would require product-only credential/menu plumbing; the controller trace exercises the same production monitor boundary directly.
Validation
swift test --filter StatusMenuScopedCodexRefreshTests— 2 tests passed; the former headless hang is goneswift test --filter StatusMenuPersistentRefreshTests— 43 tests passedmake check— passed; SwiftFormat and SwiftLint reported no issuesmake test— all 833 selections passed in 70/70 groups, with no retries or timeoutsorigin/main— clean, no accepted/actionable findings