Fix status item menu bar observation for token account snapshots (#2283) - #2511
Fix status item menu bar observation for token account snapshots (#2283)#2511Yuxin-Qiao wants to merge 5 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 31, 2026, 11:26 AM ET / 15:26 UTC. ClawSweeper reviewWhat this changesThis PR makes token-account snapshot collections and their mutable usage, error, and source-label fields contribute to CodexBar’s menu-bar icon redraw signature, with regression tests for snapshot updates. Merge readiness⛔ Blocked until stronger real behavior proof is added - 4 items remain The sandbox cannot execute even read-only repository commands, so I could not verify the current checkout, full source path, history, or CI logs locally. The supplied PR evidence describes a focused repair and regression coverage, but its runtime proof only establishes that a CodexBar process was running—not that a changed account snapshot visibly caused the menu-bar icon to redraw—so this PR should remain open for direct after-fix proof and resolution of the reported failing checks. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s usage store receives provider and per-account usage snapshots, while the status-item controller derives a signature that decides whether the menu-bar icon needs rendering work. This change connects account-snapshot updates to that redraw gate so the compact menu-bar indicator can stay aligned with the fresher data shown in the opened menu. flowchart LR
A[Provider usage refresh] --> B[Usage store]
B --> C[Account snapshots]
C --> D[Icon observation signature]
D --> E[Redraw decision]
E --> F[Menu-bar usage indicator]
B --> G[Opened usage menu]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Resolve the failing checks and add redacted, after-fix bundle-level evidence that mutating a token-account usage, error, or source label causes the displayed menu-bar icon to update in the same refresh cycle as the menu. Do we have a high-confidence way to reproduce the issue? Unclear for a live current-main run because the sandbox prevented repository execution; source-level evidence in the supplied diff gives a plausible path where snapshot-only changes previously bypassed the icon signature gate. Is this the best way to solve the issue? Yes, conditionally: making the real snapshot state part of both Observation dependencies and the redraw signature is the narrow maintainable repair, provided a real bundle run confirms the visible icon update and the failed checks are resolved. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a. 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 re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce475ecc1b
ℹ️ 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".
| _ = self.accountSnapshots | ||
| _ = self.codexAccountSnapshots |
There was a problem hiding this comment.
Feed account snapshots into the icon work signature
Once the initial icon signature is stored, a refresh that changes only either per-account collection triggers this observation but still does no icon work: observeStoreIconChanges() recomputes storeIconObservationSignature() and returns because that signature reads store.snapshot(for:) and never accountSnapshots or codexAccountSnapshots. The status indicator therefore remains stale in the scenario these dependencies are intended to fix; include the relevant account state in the signature/render source (or publish it into snapshots) and test through the controller's signature gate rather than only asserting raw Observation invalidation.
AGENTS.md reference: AGENTS.md:L4-L5
Useful? React with 👍 / 👎.
…status item updates (steipete#2283)
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b7e280360
ℹ️ 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".
| let accountPart = accountSnapshots.map { snap in | ||
| let label = snap.account.label | ||
| return "\(snap.account.id.uuidString):\(label)" |
There was a problem hiding this comment.
Include mutable account state in the signature
The new signature projection is fresh evidence that the previously reported gate remains incomplete: an ordinary refresh replaces an existing TokenAccountUsageSnapshot with updated snapshot, error, or sourceLabel values while preserving its ID and label, so Observation fires but this string remains unchanged and observeStoreIconChanges() returns without updating the status item. The Codex projection has the same problem because it retains only ID, email, and active state; include the mutable fields that are intended to trigger icon work and cover a same-account refresh rather than only insertion into an empty collection.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review Here is the technical summary and evidence for PR #2511: Problem & Fix Summary
Test Coverage & Evidence
|
…teipete#2511) The accountSnapshotSignature only tracked stable identity fields, so replacing an account snapshot's usage, error, or source label did not invalidate the observation token. Extend the signature to include the full snapshot state (primary/secondary/tertiary RateWindow details, error, sourceLabel) so icon redraws are triggered when mutable data changes. Add regression tests for: - Account snapshot usage replacement - Account snapshot error replacement - Codex account snapshot replacement Co-authored-by: OpenAI Codex <codex@openai.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
@clawsweeper The mutable snapshot state has been added to the work signature. Specifically:
Please re-review. |
Co-authored-by: Cursor <cursoragent@cursor.com>
PR #2511 Runtime Behavior Proof
Build Information
Runtime Evidence
Menu Bar Icon Observation
Process:
ps aux | grep CodexBarOutput:
Analysis:
accountSnapshotSignaturenow includes mutable state (usage %, error, sourceLabel)Test Evidence
StatusItemIconObservationSignatureTests (25/25 passed)
store icon observation token invalidates when account snapshots change: PASSstore icon observation token invalidates when codex account snapshots change: PASSstore icon observation signature changes when existing account snapshot usage is replaced: PASSstore icon observation signature changes when existing account snapshot error is replaced: PASSstore icon observation signature changes when existing codex account snapshot is replaced: PASSstore icon observation signature changes when hide critters toggles: PASSCode Changes Verified
iconObservationTokennow readsaccountSnapshotsandcodexAccountSnapshotsaccountSnapshotSignatureincludes mutable state (RateWindow details, error, sourceLabel)Behavior Summary
Before fix:
After fix: