Give claude-swap accounts explicit precedence over Claude token-account cards#1913
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 11:22 AM ET / 15:22 UTC. Summary Reproducibility: yes. from source inspection: current main computes token-account display and returns from the stacked token-account branch before reaching claude-swap cards when multiple Claude token accounts are configured. I did not run the macOS app in this read-only review. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this narrow precedence fix after ordinary maintainer review and green CI so claude-swap rows are the only Claude account presentation when multiple swap accounts are available. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main computes token-account display and returns from the stacked token-account branch before reaching claude-swap cards when multiple Claude token accounts are configured. I did not run the macOS app in this read-only review. Is this the best way to solve the issue? Yes. Centralizing the precedence predicate and applying it at both the token display and card rendering seams is a narrow maintainable fix for the observed ordering bug. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 45b000c135b0. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
…nt cards Address Codex review: with multiple Claude token accounts in stacked layout, the token-account branch rendered first and silently hid the adapter's rows. Active claude-swap accounts now suppress Claude token-account presentation at the display seam (stacked cards and the segmented switcher) and render first in addMenuCards, with coverage in ClaudeSwapMenuPrecedenceTests. Also add the packaged synthetic proof screenshot (fake cswap executable, no real accounts or credentials) to docs/claude.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
steipete
force-pushed
the
fix/claude-swap-token-account-precedence
branch
from
15fbf68 to
d77749a
Compare
![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: d77749aa2f
ℹ️ 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".
| if ClaudeSwapMenuPrecedence.prefersClaudeSwap( | ||
| provider: context.currentProvider, | ||
| accountCount: self.store.claudeSwapAccountSnapshots.count) | ||
| { |
There was a problem hiding this comment.
When Claude token accounts are configured in stacked mode and claude-swap later reports multiple accounts, this new precedence path renders all claude-swap account cards after tokenAccountMenuDisplay(for:) has been forced to nil. Because openAIWebContext is still computed only from token/codex showAll state, showAllAccounts becomes false and users with Claude cost history enabled get the aggregate menuCardCost/web adjunct appended under the per-account claude-swap cards; the previous stacked token-account path suppressed those aggregate rows. Treat this claude-swap precedence case as an all-accounts menu when building the context, or suppress those adjuncts before returning here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 414c742: active claude-swap rows now count as an all-accounts menu — ClaudeSwapMenuPrecedence.prefersClaudeSwap is wired into the showAllAccounts computation, so aggregate cost/web adjunct rows are suppressed beneath the per-account cards exactly like the stacked token-account path. Docs updated accordingly.
|
Landed after maintainer hardening. Thank you, @optimiz-r! Final proof on exact contributor head
The landed version centralizes the precedence rule in a pure seam, removes brittle live-status-bar test setup, preserves zero/single-account and non-Claude behavior, and keeps the synthetic UI proof/docs aligned with the actual contract. Squash merge: Changelog credit: |
2 participants
Summary
Follow-up to #1909. Multiple Claude token accounts in stacked layout rendered before the claude-swap branch and silently hid its rows.
NSStatusBarconstruction in headless tests.cswapexecutable—no real accounts, credentials, or provider calls.Validation
Final exact head:
d77749aa2f33221703efd10ea96c78b7cbf65557swift test --filter 'ClaudeSwap|TokenAccountMenuDisplay|MenuCardClaudeSwapAccountTests|StatusMenuHostedSubmenuRefreshTests'— 40 tests passed across 6 suites.make test— all 48 shards passed.make check— documentation/locales/repository checks passed; 0 formatting or lint violations.