feat: add subscription description from Keychain - #237
Conversation
Greptile SummaryThis PR adds Keychain comment (
Confidence Score: 3/5The core parsing and description-extraction helpers look correct and are well-tested, but the label-building step in readAllClaudeAccounts has a logic gap that can produce confusing UI labels in the mixed-description scenario this PR is specifically designed to solve. When a user has two "Claude Pro" accounts where one carries a Keychain comment and the other doesn't, the undescribed account is labelled "Claude Pro 2" — the numeric suffix is derived from a count that includes described accounts, so there is no visible "Claude Pro 1" in the picker. The PR's stated goal is exactly this disambiguation scenario, and the broken numbering undercuts it for the most common real-world split (one named, one anonymous). Files Needing Attention: src/keychain.ts — specifically the label-construction block inside readAllClaudeAccounts starting at the baseLabels/numberedLabels split
|
| Filename | Overview |
|---|---|
| src/keychain.ts | Adds parseKeychainDump/deriveKeychainDescription helpers and refactors readAllClaudeAccounts to use Keychain comments for labels; the label-numbering logic misfires when only some accounts of the same plan type carry descriptions |
| src/keychain.test.ts | Adds comprehensive tests for parseKeychainDump and deriveKeychainDescription covering NULL attributes, multi-entry dedup, and edge cases; no issues found |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[readAllClaudeAccounts] --> B{darwin?}
B -- No --> C[readCredentialsFile]
C --> D[buildAccountLabels]
D --> E[Return file account]
B -- Yes --> F[listClaudeKeychainEntries]
F --> G[execSync security dump-keychain]
G --> H[parseKeychainDump]
H --> I[Filter startsWith PRIMARY_SERVICE]
I --> J[Dedup by service, primary first]
J --> K[Return ordered KeychainEntry list]
K --> L[readKeychainService per entry]
L --> M[parseCredentials]
M --> N[deriveKeychainDescription]
N --> O{has description?}
O -- Yes --> P["label = baseLabels[i] + description"]
O -- No --> Q["label = numberedLabels[i] ⚠️ counts described accts too"]
P --> R[Return ClaudeAccount]
Q --> R
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/keychain.ts:291-313
**Misleading numeric suffix when described and undescribed accounts are mixed**
`numberedLabels` is built from **all** raw accounts, including those that already have a Keychain description. That means when two "Claude Pro" accounts exist and only one has a comment, `buildAccountLabels` counts both and produces `["Claude Pro 1", "Claude Pro 2"]`. The described account consumes slot 0 silently, so the undescribed one ends up labelled `"Claude Pro 2"` — implying a "Claude Pro 1" that never appears in the UI.
Concrete case: accounts `[{Pro, desc="Work"}, {Pro, no desc}]` → user sees `"Claude Pro - Work"` and `"Claude Pro 2"`. Only passing credentials from *undescribed* accounts to `buildAccountLabels` (then stitching the labels back at the right indices) would produce the correct `"Claude Pro"` (or `"Claude Pro 1"` / `"Claude Pro 2"` when multiple undescribed entries share a plan).
Reviews (1): Last reviewed commit: "feat: add subscription description from ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Build base labels (e.g. "Claude Pro") without disambiguating numbers. | ||
| const baseLabels = rawAccounts.map((a) => { | ||
| const sub = a.credentials.subscriptionType | ||
| if (sub) return `Claude ${sub.charAt(0).toUpperCase() + sub.slice(1)}` | ||
| return "Claude" | ||
| }) | ||
|
|
||
| // Only fall back to numeric suffixes for entries without a Keychain comment. | ||
| const numberedLabels = buildAccountLabels( | ||
| rawAccounts.map((a) => a.credentials), | ||
| ) | ||
|
|
||
| return rawAccounts.map((a, i) => ({ | ||
| label: labels[i], | ||
| // Append the user-set Keychain comment after the plan so the UI shows | ||
| // e.g. "Claude Pro - Claude Sub Duncan". Without a comment, fall back to | ||
| // the disambiguated label (e.g. "Claude Max 2"). | ||
| label: a.description | ||
| ? `${baseLabels[i]} - ${a.description}` | ||
| : numberedLabels[i], | ||
| source: a.source, | ||
| credentials: a.credentials, | ||
| description: a.description, | ||
| })) |
There was a problem hiding this comment.
numberedLabels is built from all raw accounts, including those that already have a Keychain description. That means when two "Claude Pro" accounts exist and only one has a comment, buildAccountLabels counts both and produces ["Claude Pro 1", "Claude Pro 2"]. The described account consumes slot 0 silently, so the undescribed one ends up labelled "Claude Pro 2" — implying a "Claude Pro 1" that never appears in the UI.
Concrete case: accounts [{Pro, desc="Work"}, {Pro, no desc}] → user sees "Claude Pro - Work" and "Claude Pro 2". Only passing credentials from undescribed accounts to buildAccountLabels (then stitching the labels back at the right indices) would produce the correct "Claude Pro" (or "Claude Pro 1" / "Claude Pro 2" when multiple undescribed entries share a plan).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/keychain.ts
Line: 291-313
Comment:
**Misleading numeric suffix when described and undescribed accounts are mixed**
`numberedLabels` is built from **all** raw accounts, including those that already have a Keychain description. That means when two "Claude Pro" accounts exist and only one has a comment, `buildAccountLabels` counts both and produces `["Claude Pro 1", "Claude Pro 2"]`. The described account consumes slot 0 silently, so the undescribed one ends up labelled `"Claude Pro 2"` — implying a "Claude Pro 1" that never appears in the UI.
Concrete case: accounts `[{Pro, desc="Work"}, {Pro, no desc}]` → user sees `"Claude Pro - Work"` and `"Claude Pro 2"`. Only passing credentials from *undescribed* accounts to `buildAccountLabels` (then stitching the labels back at the right indices) would produce the correct `"Claude Pro"` (or `"Claude Pro 1"` / `"Claude Pro 2"` when multiple undescribed entries share a plan).
How can I resolve this? If you propose a fix, please make it concise.
2 participants
Summary
Append the comment field in Keychain if not empty, otherwise keep enumerating. This is useful to select the right subscription if multiple Claude subscriptions of the same kind (ie. "Claude Pro") are found in Keychain.
Related issue
Testing
opencode auth login -p anthropicChecklist
feat:,fix:,docs:,chore:, etc.)make allpasses locally (runs lint, build, and test)README or docs updated where applicable