refactor(auth): add UserRecordStore adapter over current flat config (foundation for cli-core keyring store) - #229
Merged
Conversation
Foundation for the upcoming swap to cli-core's createKeyringTokenStore (PR β follows). The adapter translates twist's current flat config fields (authUserId / authUserName / authMode / authScope / token) into cli-core's UserRecord list shape without changing the on-disk schema. - list() returns at most one record built from the flat fields. - upsert() writes the flat fields, replacing the token with the record's fallbackToken (or stripping it when absent, per cli-core's replace-not-merge contract). - remove(id) clears the auth fields when the id matches; no-op otherwise. - getDefaultId / setDefaultId map to authUserId; null clears it. No consumer yet — the adapter is unused. Zero behaviour change. Existing TwistTokenStore + auth.ts probe/save/clear stay in place until PR β swaps them out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop six tests that only verified single-line code (`list` empty branch, field-rename happy paths, single-line accessor getters). The remaining four lock in the cli-core contracts the consumer relies on: - round-trip via upsert + list (covers both happy paths in one test) - upsert replace-not-merge for the token slot - remove no-op on mismatch - setDefaultId(null) preserves non-auth fields Any other regression would surface in PR β's integration tests against the live `KeyringTokenStore`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scottlovegrove
changed the title
Collaborator
Author
|
@doistbot /review |
doistbot
reviewed
May 17, 2026
doistbot
left a comment
doistbot
left a comment
Member
There was a problem hiding this comment.
This PR introduces the UserRecordStore adapter to map the existing flat auth configuration into cli-core's expected structure, laying a solid groundwork for the upcoming keyring migration. The approach safely prepares for the transition without altering the current on-disk schema. A few adjustments are needed before the final swap, specifically to preserve legacy token-only authentication sessions, ensure atomic configuration updates, and consolidate duplicated token normalization and state management logic.
P1: legacy `tw auth token` users have no `authUserId` but do have
`authMode` (+ a token in the keyring). Previously `buildRecords`
returned `[]` for them, which would orphan their keyring entry once
PR β wires the adapter into `KeyringTokenStore` — effectively a
silent logout on upgrade. Now synthesises a record with `id: ''`
when any auth metadata or token is present.
Also addressed:
- Switch get+setConfig to atomic `updateConfig({field: undefined})`,
reducing the read-write race window from full-config to just the
auth fields.
- Trim the persisted `fallbackToken` to match `saveApiToken`'s
normalisation.
- `setDefaultId` is now a no-op for the single-user adapter: clearing
via `null` would orphan the token + label, and setting an unknown
id would synthesise a phantom record (would surface as a blank-label
account on the next `list()`).
- Short-circuit `remove` / `setDefaultId` to skip the disk write when
the operation is a no-op.
- Simplify `getDefaultId` to read the id from the synthesised record
(correctly returns `''` for the legacy token-only case).
Tests updated: kept round-trip, replace-not-merge, remove-mismatch;
added token-only synthesis, remove-matching, and `setDefaultId`
no-op coverage (six tests total).
Declined the shared-serializer nit: the existing
`createTwistTokenStore().set()` path goes away in PR β, so abstracting
a shared `TwistAccount → config` mapping now would couple two code
paths that only briefly coexist between α and β.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
🎉 This PR is included in version 2.40.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
2 participants
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
Foundation PR for adopting cli-core's
createKeyringTokenStore. PR β will swap twist's customTwistTokenStore+lib/auth.tsprobe/save/clear over to cli-core's keyring-backed store; this PR adds theUserRecordStoreadapter the swap will hand to cli-core.src/lib/user-records.ts—createTwistUserRecordStore()returns aUserRecordStore<TwistAccount>that translates twist's current flat config fields (authUserId/authUserName/authMode/authScope/token) into cli-core'sUserRecordlist shape. Single-user today:list()returns at most one record;upsertandremovewrite or strip those flat fields;getDefaultId/setDefaultIdmap toauthUserId.TwistTokenStoreandauth.tsprobe/save/clear stay exactly as they are. Zero behaviour change.tokenis surfaced asfallbackToken(cli-core's name for the plaintext copy persisted when the OS keyring is unreachable at write time) — preserves twist's existing keyring-fallback behaviour.Test plan
npm run type-checknpm run lint:checknpm test— 10 new adapter tests (list / upsert / remove / getDefaultId / setDefaultId, happy + edge cases including replace-not-merge for the token slot); 624 total passnpm run buildFollow-up (PR β)
createTwistTokenStorewithcreateKeyringTokenStore({ serviceName: 'twist-cli', accountForUser: () => 'api-token', userRecords: createTwistUserRecordStore(), recordsLocation: getConfigPath() }).accountForUserto the existing'api-token'keyring slot so tokens already in users' keychains stay readable — no keyring slot migration needed.lib/auth.tsprobe/save/clear; rewire callers (config view,doctor,api, etc.) to usestore.active()/store.set()/store.clear()directly.getApiToken()shim withTWIST_API_TOKENenv precedence (cli-core's keyring store doesn't know about the env var).🤖 Generated with Claude Code