Add separate menu bar items per provider account - #3251
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![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: e1338e02ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let title = "\(Self.statusItemAccessibilityTitle) — " + | ||
| "\(self.store.metadata(for: key.provider).displayName) — \(context.displayName)" | ||
| existing.button?.setAccessibilityTitle(title) | ||
| existing.button?.toolTip = title |
There was a problem hiding this comment.
When “Hide personal information” is enabled and Codex uses separate account items, context.displayName still contains the account email and is assigned verbatim to the status item's tooltip and accessibility title. This makes the email visible on hover and through accessibility APIs even though the setting promises to obscure email addresses in the menu bar; redact or omit the account display name in both the creation and existing-item paths.
Useful? React with 👍 / 👎.
| let displayText = input.accountScoped && input.snapshot == nil | ||
| ? nil | ||
| : self.menuBarDisplayText(for: input.provider, snapshot: input.snapshot) |
There was a problem hiding this comment.
When Codex is split into separate items and “Show brand icon with percentage” is enabled, this calls the provider-level menuBarDisplayText, whose .menuBar projection can attach the currently active account's live credits and historical pace data. An exhausted sibling can therefore display the active account's credit balance, and pace mode can evaluate the sibling window against the active account's history; use an account-scoped display-text path that suppresses provider-level adjuncts.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
|
Codex review: needs changes before merge. Reviewed August 28, 2026, 6:56 PM ET / 22:56 UTC. ClawSweeper reviewWhat this changesThe branch adds an opt-in per-provider setting to show one macOS menu-bar item per account instead of one combined provider item. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readinessKeep open: the visual proof is sufficient and earlier privacy/data-isolation findings are addressed, but separate account items bypass saved custom menu-bar layouts, creating a P1 compatibility regression. Priority: P2 Review scores
Verification
How this fits togetherCodexBar polls provider account usage and renders it in macOS menu-bar status items. The new preference changes the status-item controller from a combined provider item to account-scoped items, icons, and menus. flowchart LR
A[Provider accounts] --> B[Saved menu-bar preference]
B --> C[Status item controller]
D[Account usage snapshots] --> C
C --> E[Combined provider item]
C --> F[Separate account items]
F --> G[Account-scoped menus]
Decision needed
Why: This PR adds a user-facing configuration mode and status-item behavior beyond a narrow bug fix; maintainers must decide whether that permanent product surface belongs in CodexBar. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Reuse the saved-layout renderer for account-scoped items with each item’s own snapshot, preserve account-data isolation, and add regression coverage for a stored provider layout. Do we have a high-confidence way to reproduce the issue? Yes. Set icon-and-percent mode and save a global or provider-specific layout, then enable separate items for a provider with two accounts; the new account path skips the saved-layout renderer. Is this the best way to solve the issue? No. The account path should use the existing layout renderer with account-scoped inputs rather than silently falling back to the legacy renderer. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9769d7394926. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What 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 (3 earlier review cycles)
|
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
clawsweeper
Bot
added
rating: 🦐 gold shrimp
1 participant
Summary
Menu bar items
Account menu behavior
Account identifiers are redacted.
Verification
swift test --filter 'AccountMenuBarDisplayModeTests|StatusItemControllerSplitLifecycleTests|StatusMenuTokenAccountSwitcherTests|StatusMenuCodexSwitcherTests|StatusMenuCodexSeparateAccountTests|StatusItemIconObservationSignatureTests|ProvidersPaneCoverageTests|CodexAccountsSettingsSectionTests'make format && make check