Conversation
Switching Codex accounts in the segmented layout cleared the published usage and left the open menu card blank: the account-scoped refresh's phase callbacks called the generic open-menu refresh, which defers parent rebuilds while tracking and re-defers on its retry pass, so the fetched usage was only rendered after closing and reopening the menu. Route the phase callbacks through the switcher rebuild path instead. It coalesces the rapid phase notifications into one in-place rebuild, closes hosted subviews first like the selection rebuild, and clears the tracking deferral, so the open card adopts the new account's usage as soon as it arrives. Adds a regression test asserting a post-fetch rebuild while the menu stays open, and updates the provider gatekeeper allowlist for the rerouted call. Fixes steipete#3709.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed September 18, 2026, 8:56 AM ET / 12:56 UTC (Revision 15). ClawSweeper reviewWhat this changesRefreshes the selected Codex account’s usage card while its menu remains open, preserving submenu interactions and adding regression coverage and native proof tooling. Merge readiness✅ Ready for maintainer review The PR remains necessary: current main retains the deferred-refresh behavior. No actionable defects remain, and the native before/after evidence sufficiently demonstrates the fix. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s menu controller turns account-scoped usage updates into visible account cards. This change schedules those updates during macOS menu tracking while preserving selection and interaction guards. flowchart TD
A[Account tab selected] --> B[Account usage refresh]
B --> C[Check current account and provider]
C --> D[Coalesced tracking scheduler]
D --> E{Submenu or command highlighted?}
E -->|Yes| F[Wait for interaction to finish]
F --> D
E -->|No| G[Update open account card]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Technical reviewBest possible solution: Use the existing tracking scheduler for selected-account updates and explicit interaction resumptions while preserving ordinary refresh ordering. Do we have a high-confidence way to reproduce the issue? Yes. Current main routes delayed account updates through a tracking-deferred rebuild, matching the reported blank-card sequence and supplied native baseline; this review did not execute the scenario. Is this the best way to solve the issue? Yes. Reusing the existing run-loop scheduler repairs rendering without expanding account fetching, changing credentials, or adding settings. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 0ead0d6787e8. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (14 earlier review cycles; latest 8 shown)
|
Address review feedback on the first attempt: routing the refresh phase callbacks through the switcher path would dismiss an open hosted chart submenu when a delayed phase landed. Add a dedicated helper that bumps the content version as a data-only invalidation and schedules an in-place rebuild: coalesced, deferred during native highlight, skipped while a hosted submenu is open, and reconciled when it closes. Also seed the changelog entry and cover the helper's submenu preservation contract with a focused regression test.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
The account-scoped refresh can publish unrelated data-only invalidations after the final rebuild (widget snapshots, readiness resyncs), so asserting a fully-settled state at the end over-constrains the regression. The defect's crisp signal is a rebuild happening after the fetch resolves while the menu stays open.
Follow the repo's native-proof convention (AboutUpdateCommandProofTests, ClaudeWeeklyLabelsNativeProofTests): when CODEXBAR_ACCOUNT_SWITCH_PROOF_DIR is set with keychain prompt suppression, the test runs the real account- switch scheduling path with card rendering enabled and captures before/ after PNGs of the usage card plus a transcript, asserting the fetched usage is published and the open hosted submenu survives. A dispatch-only workflow runs the proof on a macOS runner and uploads the artifacts as merge evidence.
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
Native macOS proof (revision 3) — request fulfilled Ran the account-switch flow on a real macOS 26 GitHub Actions runner (real AppKit, menu card rendering enabled, no test scheduler or rebuild mocks for the scheduling path): workflow run. Transcript (from Assertions in the run (all passing): new account's usage published into the still-open menu, hosted chart submenu preserved across the delayed rebuild, parent menu never closed, deferred parent rebuild lands after the submenu closes. Before/after renders of the actual menu card are captured as PNGs. Also validated in the same macOS run: |
clawsweeper
Bot
removed
the
merge-risk: 🚨 automation
steipete
changed the title
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Native before/after verification used a signed test host on macOS 27.0 (26A428), an actual tracking status menu, its attached B button, and exactly one held synthetic account fetch. The baseline publishes B's 17% usage but leaves the card blank; candidate Both complete captures were inspected and contain only synthetic data. No real provider or credential access, scheduler override, manual menu rebuild, or reparented card was used. Full-suite follow-up and CI are still in progress.
Image SHA-256: before |
clawsweeper
Bot
added
proof: 📸 screenshot
|
Final local validation passed on
The full-suite check exposed an ordering regression in an earlier draft: applying the tracking scheduler to every ordinary refresh could rebuild plugin cards before queued store observations settled. The final change limits that scheduler to account updates and explicit interaction resumptions, preserving the original ordinary refresh ordering. The unchanged plugin regression now passes. An adjacent cleanup test now awaits its owned sequence tasks instead of polling for an impossible load count. Baseline after-fetch capture remains unchanged. The new final-source capture below was fully inspected and contains only synthetic data. Its clean source tree is Final-head CI is still pending; this PR has not been merged yet.
|
2 participants
Fixes #3709. Selecting another Codex account could leave its usage card blank even after fetching succeeded because the refresh callback deferred the root-menu rebuild until tracking ended.
Schedule account data updates through the existing event-tracking/default run loop, preserving request coalescing, hosted submenus, and highlighted native commands. Recheck account and provider ownership before enqueueing and before rebuilding. Explicit interaction resumptions use the same tracking-safe path; ordinary plugin/manual refreshes retain their existing yielding task order. Fetching, credentials, and account activation stay in their existing layers.
Validation on
b12e07cdd8f82b955f0c756c27d3be6e9dda8424:make testpassed all 1,185 selections across 108 groups on the first attempt, without retries or timeouts;make check, 119 focused tests, and independent P0–P2 review passed. A freshly signed native test selects the attached B button in an actual tracking status menu, holds one synthetic fetch, then requires B and 17% in the attached card's screenshot/OCR before tracking returns. The baseline publishes that same usage but leaves the card blank. It uses contained fixtures and the real publication/ownership flow, without real provider access, scheduler overrides, manual rebuilds, or reparenting. Cleanup waits for the entire scoped refresh and widget task.The images below were completely inspected and contain only synthetic data. Final native proof and source/binary provenance. CI for the validated fix passed. The subsequent main integration at
befa45d8dabe4effd5809809f2720da85c8f098dretains the fix and native harness unchanged, with cleanmake checkand independent P0–P2 integration review. A combined integration run at22f32150a3461d28e9cea58d74123680b830d894also passed all 1,200 selections across 111 groups without retries or timeouts, including this unchanged menu fix, the Claude progress candidate, and current provider changes. Current-head CI passed all checks.Thanks @sandeep780049 for the original fix and investigation.