claude-swap: render the usage fields the adapter already receives, add source-chosen switch targets - #3452
Open
QuantIntellect wants to merge 6 commits into
Conversation
|
🦞👀 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
![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: 2cebc10e00
ℹ️ 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 executablePath = self.settings.claudeSwapExecutablePath | ||
| self.claudeSwapTransientState.switchingStrategy = strategy | ||
| self.runClaudeSwapSwitch(executablePath: executablePath, attributingErrorTo: nil) { |
There was a problem hiding this comment.
When a source-chosen switch fails—such as to: null or an unsupported strategy on an older claude-swap—passing nil here stores lastError with a nil lastErrorAccountID. The only UI consumer in StatusItemController+ClaudeSwapMenu.swift displays this error when lastErrorAccountID == account.id, which can never hold for a real account, so the promised error is silently discarded from the menu. Add a strategy/global error presentation path or associate the failure with a visible row.
Useful? React with 👍 / 👎.
Comment on lines
+166
to
+167
| if let retained = self.retainedAtLimitSnapshot(previous, matching: row, now: now) { | ||
| return .live(retained) |
There was a problem hiding this comment.
If a row previously projected lastGoodUsage with an exhausted, not-yet-reset window and a later refresh reports .unavailable without live usage, this branch reclassifies that retained snapshot as live even though previous.usesLastKnownUsage is true. An active row then passes claudeSwapMenuBarSnapshotOverride and renders the old measurement on the age-less menu-bar icon, defeating the new last-known exclusion. Preserve the previous classification or do not use last-known snapshots in this retention path.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed September 7, 2026, 5:55 AM ET / 09:55 UTC. ClawSweeper reviewWhat this changesThe PR enriches Claude account cards with cached usage, measurement age, spending and disabled-account information, and adds source-selected switching with outcome feedback. Merge readiness⛔ Blocked before merge - 5 items remain The PR remains useful, and the three previous findings are addressed. The new transcript establishes timestamp behavior, but merge readiness still depends on switching-scope approval, visible app feedback, and cache compatibility coverage. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s optional claude-swap adapter converts an external tool’s account and usage output into menu cards and bar indicators. User-initiated switches run through that tool, which owns credential changes, before CodexBar refreshes its display. flowchart TD
A[Account usage from claude-swap] --> B[Parse and retain measurements]
B --> C[Account cards and bar indicators]
D[User selects a switch action] --> E[Serialized external switch]
E --> F[Classify switch outcome]
F --> C
E --> A
Decision needed
Why: The accepted contract authorizes named-slot activation; the branch explicitly proposes broader target selection, and separable commits do not resolve that product/auth choice. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the richer display work independently landable, preserve explicit-slot switching until the proposed extension is approved, and verify retained measurements remain correctly classified across upgrades and relaunches. Do we have a high-confidence way to reproduce the issue? Yes for the display gap: current main drops sentinel-row snapshots and uses refresh time for ordinary measurements. This review established that from source and did not execute the app. Is this the best way to solve the issue? Partly: the parser and projection changes reuse the existing adapter well, and the prior defects are repaired; source-chosen switching remains a separate product choice with incomplete app-level proof. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 0863fe838a77. LabelsLabel changes:
Label 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 (3 earlier review cycles)
|
cswap schema v1 carries more per-account display data than the adapter decoded. Rows whose live `usage` is null projected to a nil snapshot, so a token-expired or keychain-blocked account rendered as an empty card while `cswap list` still showed its numbers. Parse and project the additive display-only fields: - `lastGoodUsage` / `lastGoodFetchedAt` / `lastGoodAgeSeconds`: keep the last known bars on rows with no live usage, instead of blanking the card. - `usageFetchedAt` / `usageAgeSeconds`: date a snapshot by claude-swap's own fetch time, so "updated N ago" reports the measurement's real age rather than the refresh tick. claude-swap serves a per-account cache, so an `ok` row can legitimately be minutes old. - `usage.spend`: project pay-as-you-go spend onto the shared provider cost row, matching the 5h / 7d / spend triple claude-swap's own menu shows. - `disabled`: mirror the "(disabled)" marker for slots held out of claude-swap's rotation. They stay valid explicit switch targets. - `foreign_credential`: recognize the cswap 0.26 status sentinel instead of falling through to "Unrecognized claude-swap status". Last-known measurements are marked on the projection and excluded from the menu-bar icon override. A card states a snapshot's age, but the bar icon has no such affordance and `isStale` tracks provider errors rather than measurement age, so the bar keeps falling back to the ambient snapshot rather than presenting hours-old numbers as current. The at-limit retention path from steipete#2731 is unaffected. All of these are display-only. No new argument array, no credential access, and no change to the Phase 1/2 process authority: the adapter still runs exactly `cswap --list --json`. Additive sections parse leniently so a malformed spend or fallback can never suppress valid rate windows.
The adapter could only activate one explicitly chosen slot. claude-swap's
own menu bar also offers the three switches where it picks the target:
rotate, most quota left, and next account not at its limit.
Add them as a "Switch Using claude-swap" submenu on the claude-swap section,
named apart from the per-card "Switch Account…" action that names its target:
- `ClaudeSwapSwitchStrategy` maps each case to one fixed argument array
(`--switch [--strategy best|next-available] --json`). CodexBar still
forwards no user-supplied arguments and never invokes a shell.
- The switch parser now accepts a null `to`, which only a strategy run
produces; the reader turns it into `noViableTarget` so "nothing to switch
to" surfaces instead of looking like a silent success. A slot-targeted
switch still requires the exact slot it asked for.
- `switched: false` with a target ("already-active") stays a successful
outcome rather than an error.
- Both entry points share one serialized transaction, so a strategy switch
cannot overlap a slot switch, and both run to natural exit.
- `ClaudeSwapSwitchMenuPlanner` decides availability off the account
projection, keeping the rule testable without live NSMenu construction.
Strategies are hidden when claude-swap has nowhere to go, and an account
disabled with `cswap disable` is never chosen on the user's behalf — it
remains an explicit switch target, which is claude-swap's own rule.
- `message` is deliberately not parsed: it embeds the account email and
must not bypass Hide Personal Info. Only the machine `reason` is kept.
Each action is still one explicit user click. This does not start
claude-swap's background `auto` loop, and CodexBar still handles no
credentials.
Strategy arguments require a claude-swap release that supports
`switch --strategy`; verified against cswap 0.26.0.
Update the adapter contract in docs/claude-multi-account-and-status-items.md and the provider notes in docs/CLAUDE.md to match what the adapter now parses and renders, and record the source-chosen switch actions as a proposed extension to Phase 2 rather than an accepted decision. The Phase 1 allow-list gains the additive display-only fields (spend, disabled, measurement freshness, lastGoodUsage), together with the rule that additive sections parse leniently and that claude-swap's human `message` field is never parsed because it embeds the account email. Accepted decision 2 is clarified to say "automatic (background) switching" stays out of scope, and points at the proposal for explicit switches where claude-swap picks the destination slot. That proposal changes no storage, status item migration, process authority, or credential boundary, so it is left for maintainer review rather than folded into the accepted list.
A completed switch was silent. Clicking the action closes the menu, so the only signal was the active marker moving on the next open — and when the target's usage reads the same before and after (a freshly reset account showing 0% either way), nothing visibly changed at all. A `switched: false` "already-active" result produced no feedback whatsoever. Record the parsed switch result as a `ClaudeSwapSwitchOutcome` and let the card it landed on report it: "Switched", or "Already Active" when claude-swap said that slot was already the active one. The confirmation replaces that card's action label for 90 seconds — long enough to still be there when the user reopens the menu after the click that closed it, short enough not to read as a permanent state — and any newer switch supersedes it. The destination is derived from the result rather than the request, so a strategy switch confirms on whichever slot claude-swap chose. A run that named no destination confirms nothing, and failures keep surfacing through the existing per-account switch error. `ClaudeSwapAccountActionPlanner` now owns the whole action-label decision (confirmation, in-flight, active, offer, none) in Core, so the ordering rules are covered by model tests instead of live NSMenu construction; the app layer only maps cases to localized strings.
QuantIntellect
force-pushed
the
feat/claude-swap-richer-usage-and-strategies
branch
from
September 7, 2026 07:19
3c59e96 to
c72d2bb
Compare
clawsweeper
Bot
added
the
merge-risk: 🚨 auth-provider
Review of steipete#3452 found three ways the new switch actions could leave a user with no idea why their active account did not change. 1. `switched: false` was treated as "already active". cswap 0.26 returns the *current* account as `to` for blocked attempts as well as benign no-ops — `candidates-exhausted`, `usage-unavailable`, `no-valid-target`, `only-one-account` and `unmanaged-account` all report a non-null destination — so a strategy that could not move anywhere confirmed "Already Active". `ClaudeSwapSwitchResultClassifier` now separates a real move, a deliberate stay-put (`already-active`, `already-best`) and a blocked attempt by reason, and only the first two confirm on a card. A reason a future claude-swap release adds is treated as blocked and surfaced, rather than silently confirmed as success. 2. Strategy failures had nowhere to render. They carry no requested slot, so the completion stored `lastError` with a nil `lastErrorAccountID`, while the only consumer shows that error when the ID matches an account — which never happened. Unsupported strategy arguments, malformed output and no-target runs therefore vanished when the menu closed. Errors that belong to no single card now render on the section beside the switch actions. 3. At-limit retention promoted stale data to live. An `unavailable` row retains the previous snapshot when a window is still exhausted; when that previous snapshot was itself a last-known fallback, the retained copy was marked live, letting it through the menu-bar exclusion and onto the bar icon as current. Retention now carries provenance forward, and `ClaudeSwapRetainedUsageStore.Record` persists it so a relaunch cannot launder it either. Payloads written before the field predate the fallback, so decoding them as live stays correct. Covered by tests over every blocked and benign reason cswap 0.26 emits, and by a retention case that fails without the provenance fix.
Two things made a completed switch feel like a click that did nothing. Ordering: the completion handler awaited a full ambient Claude refresh — which probes OAuth, the CLI and the web and can take seconds — before bumping the revision that redraws the cards. For that whole window the menu kept rendering the pre-switch state, including the old active marker. Re-list claude-swap first instead: it is one short subprocess and it owns the active marker and the numbers the cards show, so the menu reflects the switch right away. The ambient reconcile still runs, just afterwards, and the transaction marker is held until it finishes so two credential writes cannot overlap. Legibility: the status label sat in the same grey secondary text as a plan name, so among sibling cards that otherwise look identical the active account did not stand out, and a landed switch was easy to miss. `PlanEmphasis` gives the label three weights — plain for a plan name or an offered action, accent for the active account, green for an action that just completed — and the claude-swap cards select one from the action the planner already computes. Highlighted rows keep the selection color, so menu contrast is unchanged.
Contributor
Author
Real behavior proofAll output below is from a build of this branch, against a real claude-swap 0.26.0 install with three accounts. Emails are redacted; nothing else is edited. 1. Cards are dated by claude-swap's own measurement, not the refresh tick
Three distinct 2.
|
Contributor
Author
|
@clawsweeper re-review All three findings from the previous review are addressed, and real behavior proof is posted above.
Also fixed since the last revision: a completed switch was invisible for seconds because the completion handler awaited the full ambient Claude refresh before redrawing the cards. On the merge-risk decision: the split you recommended is already in place. Commit 1 is the display work and stands alone inside the accepted Phase 1 contract; the source-chosen switching is commits 2, 5 and 6 plus the "Proposed: source-chosen switch targets" section, and can be dropped together without touching commit 1. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
steipete
added a commit
that referenced
this pull request
Sep 8, 2026
Preserve optional usageFetchedAt instead of resetting cached measurement age on each poll. Keep valid usage and the existing fallback for missing or malformed timestamps. Extracted from #3452; its remaining last-good display and source-selected switching stay separate. Closes no issue. Co-authored-by: Persian ROss <37_privacy.blends@icloud.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three changes to the opt-in
claude-swapadapter, in separate commits so any of them can be taken alone.1. Render the usage fields the adapter already receives (commit 1 — inside the accepted Phase 1 contract)
cswap --list --jsoncarries more per-account display data than the parser decoded. Most visibly, a row whose liveusageis null projected to a nil snapshot, so a token-expired or Keychain-blocked account rendered as an empty card whilecswap liststill showed its numbers.Now parsed and projected, all display-only:
lastGoodUsage/lastGoodFetchedAt/lastGoodAgeSecondsusageFetchedAt/usageAgeSecondsokrow can legitimately be minutes oldusage.spenddisabledforeign_credentialNo new argument array, no credential access, no change to process authority — the adapter still runs exactly
cswap --list --json. Additive sections parse leniently so a malformedspendor fallback can never suppress valid rate windows. claude-swap's humanmessagefield is deliberately not parsed: it embeds the account email and must not bypass Hide Personal Info.Last-known measurements are marked on the projection and excluded from the menu-bar icon override. A card states a snapshot's age; the bar icon has no such affordance, and
isStaletracks provider errors rather than measurement age, so the bar keeps falling back to the ambient snapshot rather than presenting hours-old numbers as current. The at-limit retention path from #2731 is unaffected.2. Rotate / best / next-available switch actions (commit 2 — a proposed extension to Phase 2)
Phase 2 activates one slot the user names. claude-swap's own menu bar additionally offers three switches where the tool picks the destination. Added as a
Switch Using claude-swapsubmenu, named apart from the per-cardSwitch Account…action:ClaudeSwapSwitchStrategymaps each case to one fixed argument array —--switch [--strategy best|next-available] --json— from a closed enum. No free-form arguments, no shell.autoloop, which stays out of scope.to: null; that surfaces asnoViableTargetrather than a silent no-op.already-activestays a successful outcome. A slot-targeted switch still requires the exact slot it asked for.ClaudeSwapSwitchMenuPlannerdecides availability off the account projection, so the rule is testable without liveNSMenuconstruction. Hidden when there is nowhere to go; an account disabled withcswap disablestays an explicit target but is never chosen on the user's behalf — claude-swap's own rule.docs/claude-multi-account-and-status-items.mdrecords this under "Proposed: source-chosen switch targets" rather than folding it into the accepted-decisions list, since decision 2 currently reads "automatic switching … stays out of scope". It is clarified to "automatic (background) switching". If you'd rather not take this, commits 2 and 4 and that doc section can be dropped; commit 1 stands alone.3. Confirm what a switch actually did (commit 4)
A completed switch was silent. Clicking the action closes the menu, so the only signal was the active marker moving on the next open — and when the target's usage reads the same before and after, nothing visibly changed at all. This is easy to hit in practice: a freshly reset account reports
pct: 0.0on every window with noresetsAt, so it renders identically before and after being activated. Aswitched: false/already-activeresult produced no feedback whatsoever.The parsed result is now recorded as a
ClaudeSwapSwitchOutcome, and the card it landed on reports it: "Switched", or "Already Active". The confirmation replaces that card's action label for 90 seconds — long enough to still be there when the user reopens the menu after the click that closed it, short enough not to read as a permanent state — and any newer switch supersedes it. The destination comes from the result rather than the request, so a strategy switch confirms on whichever slot claude-swap chose. A run that named no destination confirms nothing, and failures keep surfacing through the existing per-account switch error.ClaudeSwapAccountActionPlannernow owns the whole action-label decision (confirmation → in-flight → active → offer → none) in Core, so the ordering rules are covered by model tests instead of liveNSMenuconstruction; the app layer only maps cases to localized strings.4. Report why a switch did not happen (commit 5 — addresses the three review findings)
toforcandidates-exhausted,usage-unavailable,no-valid-target,only-one-accountandunmanaged-account, not justfor benign no-ops, so a strategy that could not move anywhere confirmed "Already Active".
ClaudeSwapSwitchResultClassifiernow separates a real move, a deliberate stay-put (already-active,already-best) and a blocked attempt by reason. A reason a future release adds is treated as blocked andsurfaced, never silently confirmed.
lastErrorAccountIDwas nil and thecard renderer never matched — unsupported arguments, malformed output and no-target runs vanished when the menu
closed. Errors owned by no single card now render on the section beside the switch actions.
unavailablerow retaining a last-known fallback kept the.liveclassification and slipped past the menu-bar exclusion;RetainedUsageStore.Recordalso dropped the flag onthe disk round-trip. Provenance now survives both. The added test fails without the fix.
5. Make a completed switch visible immediately (commit 6)
The completion handler awaited the full ambient Claude refresh — OAuth, CLI and web probes — before bumping the
revision that redraws the cards, so for seconds the menu kept rendering the pre-switch state and the click looked
inert. claude-swap is re-listed first (one short subprocess that owns the active marker), then the ambient reconcile
runs. The adapter configuration is re-checked after each
awaitrather than read once up front, so a configurationchange mid-flight still discards the result as Phase 2 requires. The exact-argv test now pins the whole sequence
(
--switch-to <slot> --json,--version,--list --json), a stronger guarantee than before.The same commit gives the status label three weights via
PlanEmphasis— plain for a plan name, accent for the activeaccount, green for an action that just completed — because among sibling cards that otherwise look identical the
active account did not stand out. Highlighted rows keep the selection color, so menu contrast is unchanged.
Commands run
Tests
Four new suites, fake executables and fixtures only — no real claude-swap install, no credentials, no Keychain access:
ClaudeSwapRichUsageParsingTests— spend (incl. currency default and unusable-limit rejection),disabled, freshness,lastGoodUsage(incl. dropped when empty/untimestamped, and malformed-never-fails-the-row),foreign_credential, and a regression test pinning the reset-timestamp shape cswap actually emits (six fractional digits with a+00:00offset — a parse failure there would reject the whole account list).ClaudeSwapLastGoodProjectionTests— last-known bars survive,updatedAtis the measurement time, spend →providerCost, disabled marker, foreign-credential note.ClaudeSwapSwitchStrategyTests— exact argv per strategy, null-tohandling, already-active, slot-switch strictness, planner availability rules.ClaudeSwapSwitchConfirmationTests— confirmation targeting, expiry boundary, backwards-clock tolerance, in-flight precedence, and outcome derivation from the switch result.ClaudeSwapMenuBarSnapshotTestsgains a case pinning that an active account carrying only last-known usage still falls back to the ambient snapshot on the bar.Verification
Verified against cswap 0.26.0 with a real three-account setup, including a live switch. Strategy arguments require a claude-swap release supporting
switch --strategy; on older builds the run surfaces a parse error rather than switching silently.Related: #1756, #1268, #2444 (same class as the
relogin_requiredgap), and the open layout decision in #3382 — this PR does not touch that layout.