Fix cost not refreshing on menu-open refresh - #2388
Conversation
![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: 37c272878f
ℹ️ 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".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@clawsweeper review please |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 4:04 PM ET / 20:04 UTC. ClawSweeper reviewWhat this changesThis PR extends the existing opt-in all-provider menu-open refresh to schedule a coalesced, rate-limited token-cost scan so displayed costs refresh too. Merge readinessKeep open: the final head implements the maintainer-approved, rate-limited menu-open cost refresh and has sufficient signed-app proof. Likely related people: steipete (high confidence). Priority: P2 Review scores
Verification
How this fits togetherWhen CodexBar’s status menu opens, its refresh plan decides which enabled providers to update. The usage store runs local token-cost scans and publishes fresh snapshots that update the visible cost rows. flowchart LR
A[Menu opens] --> B[Refresh-all preference]
B --> C[Menu-open refresh plan]
C --> D[Provider quota refresh]
C --> E[Forced cost request]
E --> F[60-second floor]
F --> G[Coalesce or queue]
G --> H[Updated cost snapshot]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Merge the shared-scheduler implementation with its 60-second floor intact after required checks complete. Do we have a high-confidence way to reproduce the issue? Yes. Current main’s plan omits cost refresh, while the final-head proof and source define the initial request, sub-minute suppression, and later allowed scan path. Is this the best way to solve the issue? Yes. Extending the existing shared usage-store scheduler is the narrowest maintainable solution and avoids a separate timer or blocking AppKit menu tracking. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 52a092180d35. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (16 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review Addressed the P2 finding in 61ee444: a menu-open forced cost request that arrives while a non-forced token sequence is running is now kept pending and runs as one coalesced forced all-provider pass after that sequence completes, instead of being dropped. Added regression coverage for the queued follow-up and the coalescing cases, and the PR body now includes runtime log proof from a real menu session on this branch's build. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
status: 👀 ready for maintainer look
|
Maintainer decision (product call resolved): yes — the existing "Refresh all providers on menu open" preference should also force token-cost scans, with a floor: keep a minimum interval (60s) between forced cost scans so rapid menu open/close cycles can't hammer the scanner (we just landed #2646 bounding exactly that subsystem's memory behavior; let's not reintroduce pressure via the UI path). @betive37 — could you rebase onto current main and add that floor (bypass the TTL on menu open only if the last forced scan is >60s old)? With that, this is mergeable. If you don't have time, say so and we'll carry the rebase over the line with credit preserved. |
f716c62 to
29c7f84
Compare
|
Rebased onto current main (ported to the The timestamp is recorded when any forced all-provider pass starts, so a manual Refresh resets the floor too — opening the menu right after a manual refresh is a no-op instead of a second scan. Covered by three new tests in |
|
@steipete Could you check the code after the rebase? |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
merge-risk: 🚨 availability
scheduleForcedTokenRefresh dropped the menu-open request whenever any token sequence was in flight, but a non-forced sequence can skip TTL-fresh providers, so the promised forced rescan was lost until the next TTL tick or manual refresh. Keep the request pending and run one coalesced forced all-provider pass when the active sequence completes; re-requests during an in-flight forced all-provider pass (or the forced-refresh enrichment tail) still coalesce into that pass.
Menu open bypasses the token fetch TTL, so rapid open/close cycles could hammer the cost scanner. Record when a forced all-provider pass starts and drop menu-open requests (including the pending-queue path) while the last forced scan is under 60 seconds old. Manual Refresh runs the same forced all-provider pass, so it resets the floor too.
29c7f84 to
8905ccf
Compare
|
@clawsweeper re-review Final maintainer pass is pushed at |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
|
Merged as Proof:
Signed runtime proof used a Developer-ID-signed DEBUG bundle and the real status-menu refresh path. Peekaboo verified each menu click. With isolated empty Codex/Claude history roots, the sanitized event trace was: A real-history run additionally confirmed that opens after the floor coalesce into a still-active forced pass rather than starting duplicates. The temporary DEBUG observer, isolated data roots, and debug preference were removed before publication. The branch preserves #2848’s removed standalone timer and 15-minute ordinary automatic cost TTL. |
2 participants
Summary
When Refresh all providers on menu open is enabled, provider quotas refresh on open but token-cost data previously stayed stale until the automatic cost TTL or manual Refresh.
This change makes the existing opt-in preference refresh cost too:
refreshTokenCostdecision to the menu-open plan only for refresh-all mode with enabled providersThe patch preserves #2848’s idle-efficiency ownership: there is no standalone token timer, and ordinary automatic cost work retains its 15-minute TTL floor.
Validation
CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter 'MenuOpenRefreshPlanTests|UsageStoreManualTokenRefreshTests'— 18/18 passed; repeated five consecutive times after hardening an adjacent timing-sensitive test (90/90)CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter ProviderArchitectureGatekeeperTests— 38/38 passedSpendDashboardControllerTestsauth-rotation fixture recover on retry, while the affected refresh suite remained first-pass cleanmake check— zero violationsSigned-app proof
A Developer-ID-signed DEBUG bundle exercised the actual
StatusItemController.scheduleOpenMenuRefresh→UsageStore.scheduleForcedTokenRefreshpath.codesign --verify --deep --strictpassed with:Peekaboo independently verified each real menu-bar click. A temporary DEBUG-only observer wrote fixed event names and timestamps only—no providers, accounts, costs, paths, or settings values. With isolated empty Codex and Claude history roots so each real scanner pass completed deterministically:
This proves:
A real-history run also showed later opens coalescing into a still-active forced scan rather than starting duplicates.
The temporary observer and isolated debug preference were removed before publication, and the worktree was verified clean. The signed proof-bearing commit was
c639adbc1084a4dbe3142acf9f8d7ffbb2959e50; final head8905ccf0only rebases that unchanged cost-refresh diff over the later Agent Sessions-onlymaincommit and preserves both changelog entries.