Skip to content

fix(claude): honor read-only account adapters - #3846

Closed
steipete wants to merge 1 commit into
mainfrom
triage/20260921-small-prs-c
Closed

steipete wants to merge 1 commit into
mainfrom
triage/20260921-small-prs-c

Conversation

@steipete

Copy link
Copy Markdown
Owner

Claude account adapters can now declare supportsAccountSwitching: false in their schema-v1 list response. CodexBar keeps usage, account details, and active markers while suppressing inactive-account switching and active-account credential repair. Missing capabilities retain the existing true default; present values must be JSON booleans.

Adopts @benedictfrancis's #3658 with strict boolean validation and additional regression coverage. Adjacent quota parsing is shared and two single-use projection wrappers are removed without changing behavior. Production scope is two files with 38 insertions and 38 deletions (net zero). The schema guide, Claude documentation, and 0.64.1 changelog are updated with contributor credit.

Validation:

  • Before the fix: CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter ClaudeSwapListParserTests — 16 tests, two failing tests with seven issues: read-only accounts remained activatable and six nonboolean capability values were accepted.
  • After the fix:
    CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter 'ClaudeSwap(ListParser|RichUsageParsing|AccountProjection|AccountAliasProjection|LastGoodProjection|MeasurementTime|SwitchErrorTiming|AccountMenuDisplay|RichUsagePresentation|RichUsageCard|AccountReader|MenuBarSnapshot|RetainedCacheCompatibility)|MenuCardClaudeSwapAccountTests|CLICardsClaudeSwapTests|ProviderArchitectureGatekeeperTests'
    Test run with 187 tests in 16 suites passed; the separate Linux test target also reported 14 tests in 1 suite passed. This includes the red-to-green parser regressions and a synthetic executable check proving a read-only adapter starts no credential command.
  • make check — passed; SwiftFormat clean and SwiftLint reported 0 violations, 0 serious in 2547 files. An initial pre-change run hit the existing packaged-launch fixture's receipt timing failure; it passed unchanged on retry, and the final complete check passed.
  • Independent review through P2: scoped-clean, no actionable findings.

The CloudKit device-removal proposal was reviewed but is not adopted here. It lacks synthetic proof of the destructive fetch/send/retry lifecycle and permits removing the current device; #3234 remains open for a bounded implementation. No app relaunch, live provider probe, merge, or thread comment was performed.

Closes #3658
Refs #3350
Refs #3234

Respect supportsAccountSwitching=false in schema-v1 account lists while
preserving usage, account details, and active markers. Keep the existing
switching default when the field is absent and reject nonboolean values.

Cover parsing, projection, and suppression of credential commands. Reuse
quota parsing and remove single-use projection wrappers to keep the
production diff at zero net lines.

Adopts #3658.

Co-authored-by: benedictfrancis <benedict@techniciti.eu>
@clawsweeper

clawsweeper Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 22, 2026
@clawsweeper

clawsweeper Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 22, 2026, 1:33 AM ET / 05:33 UTC.

ClawSweeper review

What this changes

Adds a read-only capability for Claude account adapters that preserves usage display while suppressing account switching and credential repair, with strict parsing, regression tests, and documentation.

Merge readiness

✅ Ready for maintainer review

Keep open: this is a useful, bounded addition absent from current main and v0.64.0. No actionable correctness or security defect was found, and the numeric-Boolean defect identified in the adopted proposal is addressed.

Priority: P2
Reviewed head: 86c1bd0b795ecc7342c0a100b2265b8dd00b73fe

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, compatible implementation with useful regression coverage and no blocking findings.
Proof confidence 🌊 off-meta tidepool Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Inspected supplemental tests exercise projection and the production switch guard, including absence of a subprocess call for read-only repair; no live native demonstration is claimed. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Inspected supplemental tests exercise projection and the production switch guard, including absence of a subprocess call for read-only repair; no live native demonstration is claimed. No stored-data contract changes.
Evidence reviewed 8 items Policy and patch boundary: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-notes directory was present. Reviewed all eight introduced files against the pinned base. Applied guidance on bounded changes, provider isolation, synthetic testing, and avoiding Keychain prompts; no builds, tests, or provider probes were executed.
Strict additive capability: The parser defaults an absent capability to true, requires genuine JSON booleans through CFBoolean type identity, and passes the result into the account-list model. Explicit false suppresses activation without dropping accounts or usage.
Command suppression reaches the production boundary: Projection gates canActivate with the capability. Both menu actions and UsageStore check that eligibility; UsageStore returns before creating a switch task when activation is forbidden. Existing exact-argument subprocess execution remains unchanged.
Findings None None.
Security None None.

How this fits together

CodexBar reads account usage from a configured Claude adapter executable and projects it into menu cards. Account eligibility controls whether user interaction inspects usage or launches an explicit credential-switch command.

flowchart TD
 A[Configured Claude adapter] --> B[Account list JSON]
 B --> C[Validate optional capability]
 C --> D[Account usage snapshots]
 D --> E[Usage cards and account details]
 D --> F{Switching supported and account eligible}
 F -->|No| E
 F -->|Yes and explicitly clicked| G[Credential switch command]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +38/-38; tests +75/-5 The capability adds no net production lines and gains focused parser, projection, and command-suppression coverage.

Technical review

Best possible solution:

Keep the capability optional and enforce it through the existing account eligibility path while preserving legacy switching and usage display.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a new adapter capability; source inspection confirms main ignores the proposed field. The author reports red-to-green regression tests, which were inspected but not rerun.

Is this the best way to solve the issue?

Yes. An optional boolean feeding the existing activation gate is a narrow solution, and the adjacent parsing and projection refactors preserve their prior behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b99a91694d78.

Labels

Label changes:

  • add P2: This is a bounded compatibility improvement for optional Claude account adapters.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Inspected supplemental tests exercise projection and the production switch guard, including absence of a subprocess call for read-only repair; no live native demonstration is claimed. No stored-data contract changes.

Label justifications:

  • P2: This is a bounded compatibility improvement for optional Claude account adapters.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The owner-authored PR is exempt from the external-contributor proof gate. Inspected supplemental tests exercise projection and the production switch guard, including absence of a subprocess call for read-only repair; no live native demonstration is claimed. No stored-data contract changes.

Evidence

What I checked:

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete added a commit that referenced this pull request Sep 22, 2026
Respect supportsAccountSwitching=false in schema-v1 account lists while
preserving usage, account details, and active markers. Keep the existing
switching default when the field is absent and reject nonboolean values.

Cover parsing, projection, and suppression of credential commands. Reuse
quota parsing and remove single-use projection wrappers to keep the
production diff at zero net lines.

Adopts #3658.

Co-authored-by: benedictfrancis <benedict@techniciti.eu>
(cherry picked from commit 86c1bd0)
steipete added a commit that referenced this pull request Sep 22, 2026
Respect supportsAccountSwitching=false in schema-v1 account lists while
preserving usage, account details, and active markers. Keep the existing
switching default when the field is absent and reject nonboolean values.

Cover parsing, projection, and suppression of credential commands. Reuse
quota parsing and remove single-use projection wrappers to keep the
production diff at zero net lines.

Adopts #3658.

Co-authored-by: benedictfrancis <benedict@techniciti.eu>
(cherry picked from commit 86c1bd0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant