From 2b444e2c9534b2acde43c507ff0346fdd6866815 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 17:50:57 +0100 Subject: [PATCH 1/5] test: share account fixtures + a canonical TokenStore mock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates duplicated test scaffolding across the auth suites: - New src/test-support/accounts.ts: TestAccount type + Ingen account fixtures (alanGrant/ellieSattler/ianMalcolm/johnHammond) and a single stateful buildTokenStore covering the full TokenStore contract (active/set/clear→ClearedAccount/list/setDefault + optional activeBundle/setBundle), with id/email/label matching, an overrides splat, per-method spies and live state. - New src/test-support/cli-harness.ts: installConsoleLogSpy / installStdoutSpy and a buildProgram() Commander scaffold helper. - Migrate account/status/logout/token-view/user-flag/flow/login/refresh/ persist/pkce tests onto the shared fixtures + builder + harness, removing ~10 duplicate Account types, the ad-hoc account fixtures, 8 bespoke store builders, and the repeated console-spy + program scaffold. Keyring tests keep their numeric record-key doubles (id woven into records.get('42')-style assertions; not display identities) and dcr/ internal keep their non-standard account shapes. Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/auth/account.test.ts | 430 ++++++++++++++------------------ src/auth/flow.test.ts | 45 ++-- src/auth/login.test.ts | 15 +- src/auth/logout.test.ts | 85 +++---- src/auth/persist.test.ts | 24 +- src/auth/providers/pkce.test.ts | 5 +- src/auth/refresh.test.ts | 63 ++--- src/auth/status.test.ts | 80 +++--- src/auth/token-view.test.ts | 108 ++++---- src/auth/user-flag.test.ts | 30 +-- src/test-support/accounts.ts | 160 ++++++++++++ src/test-support/cli-harness.ts | 50 ++++ 12 files changed, 579 insertions(+), 516 deletions(-) create mode 100644 src/test-support/accounts.ts create mode 100644 src/test-support/cli-harness.ts diff --git a/src/auth/account.test.ts b/src/auth/account.test.ts index 5dcb832..aea6e15 100644 --- a/src/auth/account.test.ts +++ b/src/auth/account.test.ts @@ -1,8 +1,14 @@ -import { Command } from 'commander' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' import { formatJson, formatNdjson } from '../json.js' +import { + type TestAccount as Account, + alanGrant, + buildTokenStore, + ellieSattler, +} from '../test-support/accounts.js' +import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { type AttachAccountCurrentCommandOptions, type AttachAccountListCommandOptions, @@ -15,83 +21,21 @@ import { } from './account.js' import type { TokenStore } from './types.js' -type Account = { id: string; label?: string; email: string } - -const a1: Account = { id: '1', label: 'Alice', email: 'alice@b' } -const a2: Account = { id: '2', label: 'Bob', email: 'bob@b' } - -type Entry = { account: Account; isDefault: boolean } -const bothAccounts: Entry[] = [ - { account: a1, isDefault: true }, - { account: a2, isDefault: false }, +const bothAccounts = [ + { account: alanGrant, isDefault: true }, + { account: ellieSattler, isDefault: false }, ] -function matches(entry: Entry, ref: string): boolean { - return entry.account.id === ref || entry.account.email === ref || entry.account.label === ref -} - -// A store that models multi-account state: `setDefault(ref)` matches by -// id/email/label and re-points the default marker, so a follow-up `list()` -// reflects the change (exercises `use`'s canonical-id re-read). -function buildStore(initial: Entry[] = bothAccounts): { - store: TokenStore - listSpy: ReturnType - setDefaultSpy: ReturnType - activeSpy: ReturnType - clearSpy: ReturnType -} { - const entries = initial.map((entry) => ({ ...entry })) - const listSpy = vi.fn(async () => - entries.map((entry) => ({ account: entry.account, isDefault: entry.isDefault })), - ) - const setDefaultSpy = vi.fn(async (ref: string) => { - const target = entries.find((entry) => matches(entry, ref)) - if (!target) throw new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${ref}".`) - for (const entry of entries) entry.isDefault = entry === target - }) - // No ref → the default entry (selector-less `current`); a ref → match by - // id/email/label. Returns null on miss so the attachers can translate it. - const activeSpy = vi.fn(async (ref?: string) => { - const target = - ref === undefined - ? entries.find((entry) => entry.isDefault) - : entries.find((entry) => matches(entry, ref)) - return target ? { token: `token-${target.account.id}`, account: target.account } : null - }) - // Token-free removal: match by id/email/label (or the default when no ref), - // drop the entry, and report the removed account + its prior default bit — - // mirroring the `TokenStore.clear` contract `attachAccountRemoveCommand` - // relies on. Returns `null` on a miss (no-op). - const clearSpy = vi.fn( - async (ref?: string): Promise<{ account: Account; wasDefault: boolean } | null> => { - const idx = entries.findIndex((entry) => - ref === undefined ? entry.isDefault : matches(entry, ref), - ) - if (idx === -1) return null - const [removed] = entries.splice(idx, 1) - return { account: removed.account, wasDefault: removed.isDefault } - }, - ) - const store: TokenStore = { - active: activeSpy, - set: vi.fn(), - clear: clearSpy, - list: listSpy, - setDefault: setDefaultSpy, - } - return { store, listSpy, setDefaultSpy, activeSpy, clearSpy } -} - function buildList( overrides: Partial> = {}, store?: TokenStore, -): { program: Command; command: Command } { - const resolvedStore = store ?? buildStore().store - const program = new Command() - program.exitOverride() - const account = program.command('account') - const command = attachAccountListCommand(account, { - store: resolvedStore, +): { + program: ReturnType['program'] + command: ReturnType['parent'] +} { + const { program, parent } = buildProgram('account') + const command = attachAccountListCommand(parent, { + store: store ?? buildTokenStore().store, ...overrides, }) return { program, command } @@ -100,13 +44,13 @@ function buildList( function buildUse( overrides: Partial> = {}, store?: TokenStore, -): { program: Command; command: Command } { - const resolvedStore = store ?? buildStore().store - const program = new Command() - program.exitOverride() - const account = program.command('account') - const command = attachAccountUseCommand(account, { - store: resolvedStore, +): { + program: ReturnType['program'] + command: ReturnType['parent'] +} { + const { program, parent } = buildProgram('account') + const command = attachAccountUseCommand(parent, { + store: store ?? buildTokenStore().store, ...overrides, }) return { program, command } @@ -115,13 +59,13 @@ function buildUse( function buildCurrent( overrides: Partial> = {}, store?: TokenStore, -): { program: Command; command: Command } { - const resolvedStore = store ?? buildStore().store - const program = new Command() - program.exitOverride() - const account = program.command('account') - const command = attachAccountCurrentCommand(account, { - store: resolvedStore, +): { + program: ReturnType['program'] + command: ReturnType['parent'] +} { + const { program, parent } = buildProgram('account') + const command = attachAccountCurrentCommand(parent, { + store: store ?? buildTokenStore().store, ...overrides, }) return { program, command } @@ -130,36 +74,28 @@ function buildCurrent( function buildRemove( overrides: Partial> = {}, store?: TokenStore, -): { program: Command; command: Command } { - const resolvedStore = store ?? buildStore().store - const program = new Command() - program.exitOverride() - const account = program.command('account') - const command = attachAccountRemoveCommand(account, { - store: resolvedStore, +): { + program: ReturnType['program'] + command: ReturnType['parent'] +} { + const { program, parent } = buildProgram('account') + const command = attachAccountRemoveCommand(parent, { + store: store ?? buildTokenStore().store, ...overrides, }) return { program, command } } describe('attachAccountListCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('renders default human lines with a (default) marker only on the default entry', async () => { const { program } = buildList() await program.parseAsync(['node', 'cli', 'account', 'list']) - const emitted = logSpy.mock.calls.map((call: unknown[]) => call[0]) - expect(emitted).toEqual(['Alice (id:1) (default)', 'Bob (id:2)']) + const emitted = logSpy().mock.calls.map((call: unknown[]) => call[0]) + expect(emitted).toEqual(['Alan Grant (id:1) (default)', 'Ellie Sattler (id:2)']) }) it('emits a custom renderText string', async () => { @@ -174,7 +110,7 @@ describe('attachAccountListCommand', () => { view: { json: false, ndjson: false }, flags: {}, }) - expect(logSpy).toHaveBeenCalledWith('one line') + expect(logSpy()).toHaveBeenCalledWith('one line') }) it('emits each line when renderText returns an array', async () => { @@ -183,7 +119,9 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list']) - const emitted = logSpy.mock.calls.map((call: unknown[]) => call[0]).join('\n') + const emitted = logSpy() + .mock.calls.map((call: unknown[]) => call[0]) + .join('\n') expect(emitted).toBe('line 1\nline 2\nline 3') }) @@ -192,11 +130,11 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list', '--json']) - expect(logSpy).toHaveBeenCalledWith( + expect(logSpy()).toHaveBeenCalledWith( formatJson({ accounts: [ - { account: a1, isDefault: true }, - { account: a2, isDefault: false }, + { account: alanGrant, isDefault: true }, + { account: ellieSattler, isDefault: false }, ], default: '1', }), @@ -215,13 +153,21 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list', '--json']) expect(renderJson).toHaveBeenCalledTimes(2) - expect(renderJson).toHaveBeenNthCalledWith(1, { account: a1, isDefault: true, flags: {} }) - expect(renderJson).toHaveBeenNthCalledWith(2, { account: a2, isDefault: false, flags: {} }) - expect(logSpy).toHaveBeenCalledWith( + expect(renderJson).toHaveBeenNthCalledWith(1, { + account: alanGrant, + isDefault: true, + flags: {}, + }) + expect(renderJson).toHaveBeenNthCalledWith(2, { + account: ellieSattler, + isDefault: false, + flags: {}, + }) + expect(logSpy()).toHaveBeenCalledWith( formatJson({ accounts: [ - { name: 'Alice', isDefault: true }, - { name: 'Bob', isDefault: false }, + { name: 'Alan Grant', isDefault: true }, + { name: 'Ellie Sattler', isDefault: false }, ], default: '1', }), @@ -233,10 +179,10 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list', '--ndjson']) - const emitted = logSpy.mock.calls.map((call: unknown[]) => call[0]) + const emitted = logSpy().mock.calls.map((call: unknown[]) => call[0]) expect(emitted).toEqual([ - formatNdjson([{ account: a1, isDefault: true }]), - formatNdjson([{ account: a2, isDefault: false }]), + formatNdjson([{ account: alanGrant, isDefault: true }]), + formatNdjson([{ account: ellieSattler, isDefault: false }]), ]) }) @@ -251,12 +197,20 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list', '--ndjson']) - expect(renderJson).toHaveBeenNthCalledWith(1, { account: a1, isDefault: true, flags: {} }) - expect(renderJson).toHaveBeenNthCalledWith(2, { account: a2, isDefault: false, flags: {} }) - const emitted = logSpy.mock.calls.map((call: unknown[]) => call[0]) + expect(renderJson).toHaveBeenNthCalledWith(1, { + account: alanGrant, + isDefault: true, + flags: {}, + }) + expect(renderJson).toHaveBeenNthCalledWith(2, { + account: ellieSattler, + isDefault: false, + flags: {}, + }) + const emitted = logSpy().mock.calls.map((call: unknown[]) => call[0]) expect(emitted).toEqual([ - formatNdjson([{ name: 'Alice', isDefault: true }]), - formatNdjson([{ name: 'Bob', isDefault: false }]), + formatNdjson([{ name: 'Alan Grant', isDefault: true }]), + formatNdjson([{ name: 'Ellie Sattler', isDefault: false }]), ]) }) @@ -265,12 +219,12 @@ describe('attachAccountListCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'list', '--json', '--ndjson']) - expect(logSpy).toHaveBeenCalledTimes(1) - expect(logSpy).toHaveBeenCalledWith( + expect(logSpy()).toHaveBeenCalledTimes(1) + expect(logSpy()).toHaveBeenCalledWith( formatJson({ accounts: [ - { account: a1, isDefault: true }, - { account: a2, isDefault: false }, + { account: alanGrant, isDefault: true }, + { account: ellieSattler, isDefault: false }, ], default: '1', }), @@ -298,43 +252,45 @@ describe('attachAccountListCommand', () => { }) it('emits an empty envelope under --json when no accounts are stored', async () => { - const { program } = buildList({}, buildStore([]).store) + const { program } = buildList({}, buildTokenStore({ entries: [] }).store) await program.parseAsync(['node', 'cli', 'account', 'list', '--json']) - expect(logSpy).toHaveBeenCalledWith(formatJson({ accounts: [], default: null })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ accounts: [], default: null })) }) it('emits nothing under --ndjson when no accounts are stored', async () => { - const { program } = buildList({}, buildStore([]).store) + const { program } = buildList({}, buildTokenStore({ entries: [] }).store) await program.parseAsync(['node', 'cli', 'account', 'list', '--ndjson']) - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() }) it('emits the default empty-state message in human mode when no accounts are stored', async () => { - const { program } = buildList({}, buildStore([]).store) + const { program } = buildList({}, buildTokenStore({ entries: [] }).store) await program.parseAsync(['node', 'cli', 'account', 'list']) - expect(logSpy).toHaveBeenCalledWith('No accounts stored.') + expect(logSpy()).toHaveBeenCalledWith('No accounts stored.') }) it('reports default null when no entry is marked default', async () => { - const store = buildStore([ - { account: a1, isDefault: false }, - { account: a2, isDefault: false }, - ]).store + const store = buildTokenStore({ + entries: [ + { account: alanGrant, isDefault: false }, + { account: ellieSattler, isDefault: false }, + ], + }).store const { program } = buildList({}, store) await program.parseAsync(['node', 'cli', 'account', 'list', '--json']) - expect(logSpy).toHaveBeenCalledWith( + expect(logSpy()).toHaveBeenCalledWith( formatJson({ accounts: [ - { account: a1, isDefault: false }, - { account: a2, isDefault: false }, + { account: alanGrant, isDefault: false }, + { account: ellieSattler, isDefault: false }, ], default: null, }), @@ -364,64 +320,64 @@ describe('attachAccountListCommand', () => { }) describe('attachAccountUseCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('calls setDefault and echoes the raw ref in the human success line', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildUse({}, built.store) - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b']) + await program.parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com']) - expect(built.setDefaultSpy).toHaveBeenCalledWith('bob@b') - expect(logSpy).toHaveBeenCalledWith('✓ Default account set to bob@b') + expect(built.setDefaultSpy).toHaveBeenCalledWith('ellie@ingen.com') + expect(logSpy()).toHaveBeenCalledWith('✓ Default account set to ellie@ingen.com') }) it('emits the canonical resolved id under --json, not the requested ref', async () => { const { program } = buildUse() - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b', '--json']) + await program.parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com', '--json']) - expect(logSpy).toHaveBeenCalledWith(formatJson({ ok: true, default: '2' })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ ok: true, default: '2' })) }) it('prefers --json over --ndjson when both flags are passed', async () => { const { program } = buildUse() - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b', '--json', '--ndjson']) + await program.parseAsync([ + 'node', + 'cli', + 'account', + 'use', + 'ellie@ingen.com', + '--json', + '--ndjson', + ]) - expect(logSpy).toHaveBeenCalledWith(formatJson({ ok: true, default: '2' })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ ok: true, default: '2' })) }) it('does not re-read the store outside --json', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildUse({}, built.store) - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b']) + await program.parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com']) - expect(built.setDefaultSpy).toHaveBeenCalledWith('bob@b') + expect(built.setDefaultSpy).toHaveBeenCalledWith('ellie@ingen.com') expect(built.listSpy).not.toHaveBeenCalled() }) it('is silent under --ndjson but still calls setDefault', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildUse({}, built.store) - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b', '--ndjson']) + await program.parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com', '--ndjson']) - expect(built.setDefaultSpy).toHaveBeenCalledWith('bob@b') - expect(logSpy).not.toHaveBeenCalled() + expect(built.setDefaultSpy).toHaveBeenCalledWith('ellie@ingen.com') + expect(logSpy()).not.toHaveBeenCalled() }) it('propagates ACCOUNT_NOT_FOUND from setDefault and prints nothing', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildUse({}, built.store) // `ghost` matches no stored id/email/label, so the store throws naturally. @@ -429,7 +385,7 @@ describe('attachAccountUseCommand', () => { program.parseAsync(['node', 'cli', 'account', 'use', 'ghost']), ).rejects.toMatchObject({ constructor: CliError, code: 'ACCOUNT_NOT_FOUND' }) expect(built.listSpy).not.toHaveBeenCalled() - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() }) it('emits the success line before awaiting onDefaultSet', async () => { @@ -441,14 +397,14 @@ describe('attachAccountUseCommand', () => { const { program } = buildUse({ onDefaultSet }) const parsed = program - .parseAsync(['node', 'cli', 'account', 'use', 'bob@b']) + .parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com']) .then(() => 'done') await vi.waitFor(() => expect(onDefaultSet).toHaveBeenCalled()) // Success line is already out, but the command is still parked on the hook. - expect(logSpy).toHaveBeenCalledWith('✓ Default account set to bob@b') + expect(logSpy()).toHaveBeenCalledWith('✓ Default account set to ellie@ingen.com') expect(onDefaultSet).toHaveBeenCalledWith({ - ref: 'bob@b', + ref: 'ellie@ingen.com', view: { json: false, ndjson: false }, flags: {}, }) @@ -463,10 +419,10 @@ describe('attachAccountUseCommand', () => { const { program, command } = buildUse({ onDefaultSet }) command.option('--full', 'Extra') - await program.parseAsync(['node', 'cli', 'account', 'use', 'bob@b', '--full']) + await program.parseAsync(['node', 'cli', 'account', 'use', 'ellie@ingen.com', '--full']) expect(onDefaultSet).toHaveBeenCalledWith({ - ref: 'bob@b', + ref: 'ellie@ingen.com', view: { json: false, ndjson: false }, flags: { full: true }, }) @@ -480,32 +436,24 @@ describe('attachAccountUseCommand', () => { }) describe('attachAccountCurrentCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('renders the default human line with a (default) marker for the active account', async () => { const { program } = buildCurrent() await program.parseAsync(['node', 'cli', 'account', 'current']) - expect(logSpy).toHaveBeenCalledWith('Alice (id:1) (default)') + expect(logSpy()).toHaveBeenCalledWith('Alan Grant (id:1) (default)') }) it('omits the marker when the active account is not the default', async () => { const store: TokenStore = { - active: vi.fn(async () => ({ token: 't', account: a1 })), + active: vi.fn(async () => ({ token: 't', account: alanGrant })), set: vi.fn(), clear: vi.fn(), list: vi.fn(async () => [ - { account: a1, isDefault: false }, - { account: a2, isDefault: true }, + { account: alanGrant, isDefault: false }, + { account: ellieSattler, isDefault: true }, ]), setDefault: vi.fn(), } @@ -513,7 +461,7 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current']) - expect(logSpy).toHaveBeenCalledWith('Alice (id:1)') + expect(logSpy()).toHaveBeenCalledWith('Alan Grant (id:1)') }) it('passes account + isDefault to a custom renderText', async () => { @@ -523,12 +471,12 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current']) expect(renderText).toHaveBeenCalledWith({ - account: a1, + account: alanGrant, isDefault: true, view: { json: false, ndjson: false }, flags: {}, }) - expect(logSpy).toHaveBeenCalledWith('custom line') + expect(logSpy()).toHaveBeenCalledWith('custom line') }) it('emits the default { account, isDefault } payload under --json', async () => { @@ -536,7 +484,7 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current', '--json']) - expect(logSpy).toHaveBeenCalledWith(formatJson({ account: a1, isDefault: true })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ account: alanGrant, isDefault: true })) }) it('shapes the --json payload via renderJson', async () => { @@ -545,8 +493,8 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current', '--json']) - expect(renderJson).toHaveBeenCalledWith({ account: a1, isDefault: true, flags: {} }) - expect(logSpy).toHaveBeenCalledWith(formatJson({ email: 'alice@b' })) + expect(renderJson).toHaveBeenCalledWith({ account: alanGrant, isDefault: true, flags: {} }) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ email: 'alan@ingen.com' })) }) it('emits a single payload object under --ndjson', async () => { @@ -554,7 +502,9 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current', '--ndjson']) - expect(logSpy).toHaveBeenCalledWith(formatNdjson([{ account: a1, isDefault: true }])) + expect(logSpy()).toHaveBeenCalledWith( + formatNdjson([{ account: alanGrant, isDefault: true }]), + ) }) it('prefers --json over --ndjson when both flags are passed', async () => { @@ -562,8 +512,8 @@ describe('attachAccountCurrentCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'current', '--json', '--ndjson']) - expect(logSpy).toHaveBeenCalledOnce() - expect(logSpy).toHaveBeenCalledWith(formatJson({ account: a1, isDefault: true })) + expect(logSpy()).toHaveBeenCalledOnce() + expect(logSpy()).toHaveBeenCalledWith(formatJson({ account: alanGrant, isDefault: true })) }) // Covers both non-serializable shapes: a top-level `undefined` @@ -584,7 +534,10 @@ describe('attachAccountCurrentCommand', () => { it('invokes onNotAuthenticated when nothing is active', async () => { const onNotAuthenticated = vi.fn() - const { program } = buildCurrent({ onNotAuthenticated }, buildStore([]).store) + const { program } = buildCurrent( + { onNotAuthenticated }, + buildTokenStore({ entries: [] }).store, + ) await program.parseAsync(['node', 'cli', 'account', 'current']) @@ -592,11 +545,11 @@ describe('attachAccountCurrentCommand', () => { view: { json: false, ndjson: false }, flags: {}, }) - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() }) it('throws NOT_AUTHENTICATED when nothing is active and no hook is supplied', async () => { - const { program } = buildCurrent({}, buildStore([]).store) + const { program } = buildCurrent({}, buildTokenStore({ entries: [] }).store) await expect( program.parseAsync(['node', 'cli', 'account', 'current']), @@ -604,8 +557,8 @@ describe('attachAccountCurrentCommand', () => { }) it('prefers store.activeAccount over active() + list() when implemented', async () => { - const built = buildStore() - built.store.activeAccount = vi.fn(async () => ({ account: a2, isDefault: false })) + const built = buildTokenStore() + built.store.activeAccount = vi.fn(async () => ({ account: ellieSattler, isDefault: false })) const { program } = buildCurrent({}, built.store) await program.parseAsync(['node', 'cli', 'account', 'current']) @@ -613,7 +566,7 @@ describe('attachAccountCurrentCommand', () => { expect(built.store.activeAccount).toHaveBeenCalledOnce() expect(built.activeSpy).not.toHaveBeenCalled() expect(built.listSpy).not.toHaveBeenCalled() - expect(logSpy).toHaveBeenCalledWith('Bob (id:2)') + expect(logSpy()).toHaveBeenCalledWith('Ellie Sattler (id:2)') }) it('returns the new Command so the consumer can chain', () => { @@ -624,41 +577,33 @@ describe('attachAccountCurrentCommand', () => { }) describe('attachAccountRemoveCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('removes the matched account by ref and marks it as the former default', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildRemove({}, built.store) // Invoked by email; the store matches + clears it token-free. - await program.parseAsync(['node', 'cli', 'account', 'remove', 'alice@b']) + await program.parseAsync(['node', 'cli', 'account', 'remove', 'alan@ingen.com']) - expect(built.clearSpy).toHaveBeenCalledWith('alice@b') - expect(await built.store.list()).toEqual([{ account: a2, isDefault: false }]) - expect(logSpy).toHaveBeenCalledOnce() - expect(logSpy).toHaveBeenCalledWith('✓ Removed Alice (default)') + expect(built.clearSpy).toHaveBeenCalledWith('alan@ingen.com') + expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: false }]) + expect(logSpy()).toHaveBeenCalledOnce() + expect(logSpy()).toHaveBeenCalledWith('✓ Removed Alan Grant (default)') }) it('omits the (default) marker when the removed account was not the default', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildRemove({}, built.store) - await program.parseAsync(['node', 'cli', 'account', 'remove', 'bob@b']) + await program.parseAsync(['node', 'cli', 'account', 'remove', 'ellie@ingen.com']) - expect(logSpy).toHaveBeenCalledOnce() - expect(logSpy).toHaveBeenCalledWith('✓ Removed Bob') + expect(logSpy()).toHaveBeenCalledOnce() + expect(logSpy()).toHaveBeenCalledWith('✓ Removed Ellie Sattler') }) it('throws ACCOUNT_NOT_FOUND and removes nothing when the ref misses', async () => { - const built = buildStore() + const built = buildTokenStore() const { program } = buildRemove({}, built.store) await expect( @@ -668,7 +613,7 @@ describe('attachAccountRemoveCommand', () => { }) it('removes an account whose token is unreadable, never touching active()', async () => { - const built = buildStore() + const built = buildTokenStore() // A broken keyring entry: `active()` would throw AUTH_STORE_READ_FAILED, // but `remove` must still clear it. If the attacher called active() this // would surface that error instead of removing the record. @@ -677,19 +622,19 @@ describe('attachAccountRemoveCommand', () => { }) const { program } = buildRemove({}, built.store) - await program.parseAsync(['node', 'cli', 'account', 'remove', 'alice@b']) + await program.parseAsync(['node', 'cli', 'account', 'remove', 'alan@ingen.com']) expect(built.store.active).not.toHaveBeenCalled() - expect(await built.store.list()).toEqual([{ account: a2, isDefault: false }]) - expect(logSpy).toHaveBeenCalledWith('✓ Removed Alice (default)') + expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: false }]) + expect(logSpy()).toHaveBeenCalledWith('✓ Removed Alan Grant (default)') }) it('emits { ok, removed } with the canonical id under --json', async () => { const { program } = buildRemove() - await program.parseAsync(['node', 'cli', 'account', 'remove', 'bob@b', '--json']) + await program.parseAsync(['node', 'cli', 'account', 'remove', 'ellie@ingen.com', '--json']) - expect(logSpy).toHaveBeenCalledWith(formatJson({ ok: true, removed: '2' })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ ok: true, removed: '2' })) }) it('prefers --json over --ndjson when both flags are passed', async () => { @@ -700,24 +645,31 @@ describe('attachAccountRemoveCommand', () => { 'cli', 'account', 'remove', - 'bob@b', + 'ellie@ingen.com', '--json', '--ndjson', ]) - expect(logSpy).toHaveBeenCalledOnce() - expect(logSpy).toHaveBeenCalledWith(formatJson({ ok: true, removed: '2' })) + expect(logSpy()).toHaveBeenCalledOnce() + expect(logSpy()).toHaveBeenCalledWith(formatJson({ ok: true, removed: '2' })) }) it('is silent under --ndjson but still clears and runs onRemoved', async () => { - const built = buildStore() + const built = buildTokenStore() const onRemoved = vi.fn() const { program } = buildRemove({ onRemoved }, built.store) - await program.parseAsync(['node', 'cli', 'account', 'remove', 'bob@b', '--ndjson']) + await program.parseAsync([ + 'node', + 'cli', + 'account', + 'remove', + 'ellie@ingen.com', + '--ndjson', + ]) - expect(await built.store.list()).toEqual([{ account: a1, isDefault: true }]) - expect(logSpy).not.toHaveBeenCalled() + expect(await built.store.list()).toEqual([{ account: alanGrant, isDefault: true }]) + expect(logSpy()).not.toHaveBeenCalled() expect(onRemoved).toHaveBeenCalledOnce() }) @@ -726,18 +678,18 @@ describe('attachAccountRemoveCommand', () => { const onRemoved = vi.fn() const { program } = buildRemove({ renderText, onRemoved }) - await program.parseAsync(['node', 'cli', 'account', 'remove', 'alice@b']) + await program.parseAsync(['node', 'cli', 'account', 'remove', 'alan@ingen.com']) const expectedCtx = { - account: a1, - ref: 'alice@b', + account: alanGrant, + ref: 'alan@ingen.com', wasDefault: true, view: { json: false, ndjson: false }, flags: {}, } expect(renderText).toHaveBeenCalledWith(expectedCtx) expect(onRemoved).toHaveBeenCalledWith(expectedCtx) - expect(logSpy).toHaveBeenCalledWith('gone') + expect(logSpy()).toHaveBeenCalledWith('gone') }) it('emits the success line before awaiting onRemoved', async () => { @@ -749,12 +701,12 @@ describe('attachAccountRemoveCommand', () => { const { program } = buildRemove({ onRemoved }) const parsed = program - .parseAsync(['node', 'cli', 'account', 'remove', 'bob@b']) + .parseAsync(['node', 'cli', 'account', 'remove', 'ellie@ingen.com']) .then(() => 'done') await vi.waitFor(() => expect(onRemoved).toHaveBeenCalled()) // Success line is already out, but the command is still parked on the hook. - expect(logSpy).toHaveBeenCalledWith('✓ Removed Bob') + expect(logSpy()).toHaveBeenCalledWith('✓ Removed Ellie Sattler') expect(await Promise.race([parsed, Promise.resolve('pending')])).toBe('pending') releaseHook() diff --git a/src/auth/flow.test.ts b/src/auth/flow.test.ts index 8122f90..a1b2b7c 100644 --- a/src/auth/flow.test.ts +++ b/src/auth/flow.test.ts @@ -21,33 +21,18 @@ vi.mock('open', () => ({ default: vi.fn(async () => undefined) })) import { execFile } from 'node:child_process' import { readFileSync } from 'node:fs' import openBrowserModule from 'open' +import { + type TestAccount as Account, + alanGrant, + buildTokenStore, + ellieSattler, +} from '../test-support/accounts.js' import { type RunOAuthFlowOptions, runOAuthFlow } from './flow.js' import type { AuthProvider, TokenBundle, TokenStore } from './types.js' -type Account = { id: string; label?: string; email: string } - /** Tiny in-memory `TokenStore` so the flow tests don't need disk I/O. */ -function fakeStore(): TokenStore & { last?: { account: Account; token: string } } { - const state: { last?: { account: Account; token: string } } = {} - return { - async active() { - return state.last ?? null - }, - async set(account, token) { - state.last = { account, token } - }, - async clear() { - state.last = undefined - return null - }, - async list() { - return state.last ? [{ account: state.last.account, isDefault: true }] : [] - }, - async setDefault() {}, - get last() { - return state.last - }, - } +function fakeStore(): TokenStore { + return buildTokenStore({ entries: [] }).store } const renderSuccess = () => 'ok' @@ -79,7 +64,7 @@ function instrument(provider: Partial> = {}): { return { accessToken: 'tok-1' } }, async validateToken() { - return { id: '1', email: 'a@b' } + return alanGrant }, ...rest, async authorize(input) { @@ -129,7 +114,7 @@ describe('runOAuthFlow', () => { it('drives prepare → authorize → exchange → validate → store and returns the result', async () => { const prepare = vi.fn(async () => ({ handshake: { dcrSecret: 'shh' } })) const exchangeCode = vi.fn(async () => ({ accessToken: 'tok-1' })) - const validateToken = vi.fn(async () => ({ id: '1', email: 'a@b' })) + const validateToken = vi.fn(async () => alanGrant) const { provider, getRedirect } = instrument({ prepare, exchangeCode, validateToken }) const store = fakeStore() const openBrowser = vi.fn(driveCallback(getRedirect)) @@ -145,14 +130,14 @@ describe('runOAuthFlow', () => { ) expect(result.token).toBe('tok-1') - expect(result.account).toEqual({ id: '1', email: 'a@b' }) + expect(result.account).toEqual(alanGrant) expect(prepare).toHaveBeenCalledTimes(1) expect(exchangeCode).toHaveBeenCalledTimes(1) expect(validateToken).toHaveBeenCalledTimes(1) expect(openBrowser).toHaveBeenCalledTimes(1) expect(await store.active()).toEqual({ token: 'tok-1', - account: { id: '1', email: 'a@b' }, + account: alanGrant, }) }) @@ -161,7 +146,7 @@ describe('runOAuthFlow', () => { const { provider, getRedirect } = instrument({ exchangeCode: async () => ({ accessToken: 'tok-1', - account: { id: '99', email: 'right@b' }, + account: ellieSattler, }), validateToken, }) @@ -170,14 +155,14 @@ describe('runOAuthFlow', () => { const result = await runOAuthFlow( flowOptions({ provider, store, openBrowser: driveCallback(getRedirect) }), ) - expect(result.account.id).toBe('99') + expect(result.account.id).toBe('2') expect(validateToken).not.toHaveBeenCalled() }) it('threads prepare-time handshake into validateToken even when authorize forgets to forward it', async () => { const validateToken = vi.fn(async ({ handshake }) => { expect(handshake.dcrSecret).toBe('shh') // came from prepare(), not authorize() - return { id: '1', email: 'a@b' } + return alanGrant }) const { provider, getRedirect } = instrument({ prepare: async () => ({ handshake: { dcrSecret: 'shh' } }), diff --git a/src/auth/login.test.ts b/src/auth/login.test.ts index c9d0bd8..3250450 100644 --- a/src/auth/login.test.ts +++ b/src/auth/login.test.ts @@ -1,7 +1,9 @@ -import { Command } from 'commander' +import type { Command } from 'commander' import { beforeEach, describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' +import { type TestAccount as Account, alanGrant } from '../test-support/accounts.js' +import { buildProgram } from '../test-support/cli-harness.js' import { attachLoginCommand } from './login.js' import type { AuthProvider, TokenStore } from './types.js' @@ -12,9 +14,7 @@ vi.mock('./flow.js', () => ({ const { runOAuthFlow } = await import('./flow.js') const mockedRunOAuthFlow = vi.mocked(runOAuthFlow) -type Account = { id: string; label?: string; email: string } - -const account: Account = { id: '1', label: 'me', email: 'a@b' } +const account = alanGrant const provider = {} as AuthProvider const store = {} as TokenStore @@ -30,9 +30,7 @@ function build( onSuccess: ReturnType resolveScopes: ReturnType } { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const onSuccess = vi.fn() const resolveScopes = vi.fn(() => ['read']) const login = attachLoginCommand(auth, { @@ -225,8 +223,7 @@ describe('attachLoginCommand', () => { }) it('returns the new Command so the consumer can chain', () => { - const program = new Command() - const auth = program.command('auth') + const { parent: auth } = buildProgram('auth') const login = attachLoginCommand(auth, { provider, store, diff --git a/src/auth/logout.test.ts b/src/auth/logout.test.ts index 67890c6..98d4ea2 100644 --- a/src/auth/logout.test.ts +++ b/src/auth/logout.test.ts @@ -1,33 +1,30 @@ -import { Command } from 'commander' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Command } from 'commander' +import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' import { formatJson } from '../json.js' +import { + type TestAccount as Account, + type TokenStoreHarness, + alanGrant, + buildTokenStore, +} from '../test-support/accounts.js' +import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { attachLogoutCommand } from './logout.js' import type { TokenStore } from './types.js' -type Account = { id: string; label?: string; email: string } type LogoutOverrides = Partial>[1]> -const account: Account = { id: '1', label: 'me', email: 'a@b' } +const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok', account }, -): { - store: TokenStore - activeSpy: ReturnType - clearSpy: ReturnType -} { - const activeSpy = vi.fn(async () => initial) - const clearSpy = vi.fn(async () => null) - const store: TokenStore = { - active: activeSpy, - set: vi.fn(), - clear: clearSpy, - list: vi.fn(async () => (initial ? [{ account: initial.account, isDefault: true }] : [])), - setDefault: vi.fn(), - } - return { store, activeSpy, clearSpy } +): TokenStoreHarness { + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, token: initial.token }] + : [], + }) } function build( @@ -39,10 +36,8 @@ function build( logout: Command onCleared: ReturnType } { - const { store } = storeOverride ? { store: storeOverride } : buildStore() - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const store = storeOverride ?? buildStore().store + const { program, parent: auth } = buildProgram('auth') const onCleared = vi.fn() const logout = attachLogoutCommand(auth, { store, @@ -53,15 +48,7 @@ function build( } describe('attachLogoutCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('clears the store and emits the human success line in plain mode', async () => { const built = buildStore() @@ -71,7 +58,7 @@ describe('attachLogoutCommand', () => { expect(built.activeSpy).toHaveBeenCalledWith(undefined) expect(built.clearSpy).toHaveBeenCalledWith(undefined) - expect(logSpy).toHaveBeenCalledWith('✓ Logged out') + expect(logSpy()).toHaveBeenCalledWith('✓ Logged out') expect(onCleared).toHaveBeenCalledWith({ account, ref: undefined, @@ -85,7 +72,7 @@ describe('attachLogoutCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'logout', '--json']) - expect(logSpy).toHaveBeenCalledWith(formatJson({ ok: true })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ ok: true })) expect(onCleared).toHaveBeenCalledWith({ account, ref: undefined, @@ -99,7 +86,7 @@ describe('attachLogoutCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'logout', '--ndjson']) - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() expect(onCleared).toHaveBeenCalledWith({ account, ref: undefined, @@ -145,15 +132,15 @@ describe('attachLogoutCommand', () => { 'logout', '--json', '--user', - 'me@example', + 'alan@ingen.com', '--full', ]) - expect(built.activeSpy).toHaveBeenCalledWith('me@example') - expect(built.clearSpy).toHaveBeenCalledWith('me@example') + expect(built.activeSpy).toHaveBeenCalledWith('alan@ingen.com') + expect(built.clearSpy).toHaveBeenCalledWith('alan@ingen.com') expect(onCleared).toHaveBeenCalledWith({ account, - ref: 'me@example', + ref: 'alan@ingen.com', view: { json: true, ndjson: false }, flags: { full: true }, }) @@ -297,15 +284,15 @@ describe('attachLogoutCommand', () => { 'logout', '--json', '--user', - 'me@example', + 'alan@ingen.com', '--full', ]) - expect(built.activeSpy).toHaveBeenCalledWith('me@example') + expect(built.activeSpy).toHaveBeenCalledWith('alan@ingen.com') expect(revokeToken).toHaveBeenCalledWith({ token: 'tok', account, - ref: 'me@example', + ref: 'alan@ingen.com', view: { json: true, ndjson: false }, flags: { full: true }, }) @@ -346,13 +333,13 @@ describe('attachLogoutCommand', () => { const built = buildStore() const { program, onCleared } = build({}, built.store) - await program.parseAsync(['node', 'cli', 'auth', 'logout', '--user', 'alice@example']) + await program.parseAsync(['node', 'cli', 'auth', 'logout', '--user', 'alan@ingen.com']) - expect(built.activeSpy).toHaveBeenCalledWith('alice@example') - expect(built.clearSpy).toHaveBeenCalledWith('alice@example') + expect(built.activeSpy).toHaveBeenCalledWith('alan@ingen.com') + expect(built.clearSpy).toHaveBeenCalledWith('alan@ingen.com') expect(onCleared).toHaveBeenCalledWith({ account, - ref: 'alice@example', + ref: 'alan@ingen.com', view: { json: false, ndjson: false }, flags: {}, }) @@ -372,7 +359,7 @@ describe('attachLogoutCommand', () => { code: 'ACCOUNT_NOT_FOUND', }) expect(built.clearSpy).not.toHaveBeenCalled() - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() }) it('proceeds with clear(ref) when active(ref) throws AUTH_STORE_READ_FAILED', async () => { @@ -392,7 +379,7 @@ describe('attachLogoutCommand', () => { expect(built.clearSpy).toHaveBeenCalledWith('me') expect(revokeSpy).not.toHaveBeenCalled() - expect(logSpy).toHaveBeenCalledWith('✓ Logged out') + expect(logSpy()).toHaveBeenCalledWith('✓ Logged out') // `account` is null (no readable snapshot) but `ref` is populated, so // consumers can distinguish "nothing was stored" from "cleared an // unreadable record". @@ -418,7 +405,7 @@ describe('attachLogoutCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'logout', '--user', 'me']) expect(built.clearSpy).toHaveBeenCalledWith('me') - expect(logSpy).toHaveBeenCalledWith('✓ Logged out') + expect(logSpy()).toHaveBeenCalledWith('✓ Logged out') }) it('still propagates non-read errors from the snapshot pre-flight', async () => { diff --git a/src/auth/persist.test.ts b/src/auth/persist.test.ts index 91faa91..aa41f39 100644 --- a/src/auth/persist.test.ts +++ b/src/auth/persist.test.ts @@ -1,27 +1,27 @@ import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' +import { + type TestAccount as Account, + buildTokenStore, + ianMalcolm, +} from '../test-support/accounts.js' import { persistBundle } from './persist.js' -import type { AuthAccount, TokenBundle, TokenStore } from './types.js' +import type { TokenBundle, TokenStore } from './types.js' -type Account = AuthAccount & { id: string; email: string } - -const account: Account = { id: '42', email: 'a@b.c' } +const account = ianMalcolm const bundle: TokenBundle = { accessToken: 'tok_access', refreshToken: 'tok_refresh', accessTokenExpiresAt: 1_700_000_000_000, } +// Default to a store that implements neither bundle method, so persistBundle's +// fallback path is the baseline; tests opt into `setBundle`/`set` via overrides. function fakeStore(overrides: Partial> = {}): TokenStore { - return { - active: vi.fn(async () => null), - set: vi.fn(async () => undefined), - clear: vi.fn(async () => null), - list: vi.fn(async () => []), - setDefault: vi.fn(async () => undefined), - ...overrides, - } + return buildTokenStore({ + overrides: { setBundle: undefined, activeBundle: undefined, ...overrides }, + }).store } describe('persistBundle', () => { diff --git a/src/auth/providers/pkce.test.ts b/src/auth/providers/pkce.test.ts index ea37bf6..f49d3f3 100644 --- a/src/auth/providers/pkce.test.ts +++ b/src/auth/providers/pkce.test.ts @@ -1,16 +1,15 @@ import { afterEach, describe, expect, it, vi } from 'vitest' +import { type TestAccount as Account, alanGrant } from '../../test-support/accounts.js' import { createPkceProvider } from './pkce.js' -type Account = { id: string; label?: string; email: string } - const respond = (body: unknown, status = 200): Response => new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' }, }) -const validate = async () => ({ id: '1', email: 'a@b' }) as Account +const validate = async (): Promise => alanGrant describe('createPkceProvider', () => { it('builds an authorize URL with response_type / client_id / redirect_uri / state / S256 code_challenge / scope', async () => { diff --git a/src/auth/refresh.test.ts b/src/auth/refresh.test.ts index eb22c1a..b589eb7 100644 --- a/src/auth/refresh.test.ts +++ b/src/auth/refresh.test.ts @@ -5,6 +5,12 @@ import { setTimeout as sleep } from 'node:timers/promises' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' +import { + type TestAccount as Account, + type TokenStoreHarness, + buildTokenStore, + ianMalcolm, +} from '../test-support/accounts.js' import { refreshAccessToken } from './refresh.js' import type { ActiveBundleSnapshot, @@ -14,9 +20,7 @@ import type { TokenStore, } from './types.js' -type Account = { id: string; label?: string; email: string } - -const account: Account = { id: '42', email: 'a@b' } +const account = ianMalcolm function bundle(overrides: Partial = {}): TokenBundle { return { @@ -27,50 +31,16 @@ function bundle(overrides: Partial = {}): TokenBundle { } } -type StoreState = { - snapshot: ActiveBundleSnapshot | null - activeBundleSpy: ReturnType - setBundleSpy: ReturnType - setBundleCalls: { account: Account; bundle: TokenBundle; options?: unknown }[] -} - function fakeStore( initial: ActiveBundleSnapshot | null, overrides: Partial> = {}, -): { store: TokenStore; state: StoreState } { - const setBundleCalls: StoreState['setBundleCalls'] = [] - const state: StoreState = { - snapshot: initial, - activeBundleSpy: vi.fn(), - setBundleSpy: vi.fn(), - setBundleCalls, - } - state.activeBundleSpy.mockImplementation(async () => state.snapshot) - state.setBundleSpy.mockImplementation( - async (acc: Account, b: TokenBundle, options?: unknown) => { - setBundleCalls.push({ account: acc, bundle: b, options }) - state.snapshot = { account: acc, bundle: b } - }, - ) - const store: TokenStore = { - async active() { - return state.snapshot - ? { token: state.snapshot.bundle.accessToken, account: state.snapshot.account } - : null - }, - activeBundle: state.activeBundleSpy as unknown as TokenStore['activeBundle'], - async set() {}, - setBundle: state.setBundleSpy as unknown as TokenStore['setBundle'], - async clear() { - return null - }, - async list() { - return [] - }, - async setDefault() {}, - ...overrides, - } - return { store, state } +): TokenStoreHarness { + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, bundle: initial.bundle }] + : [], + overrides, + }) } function fakeProvider( @@ -238,10 +208,7 @@ describe('refreshAccessToken', () => { // alongside the call so no background task outlives the test. const holder = (async () => { await sleep(120) - state.snapshot = { - account, - bundle: { accessToken: 'tok_held', refreshToken: 'r_held' }, - } + state.entries[0].bundle = { accessToken: 'tok_held', refreshToken: 'r_held' } await rm(lockPath, { force: true }) })() diff --git a/src/auth/status.test.ts b/src/auth/status.test.ts index 9af64ee..3b54965 100644 --- a/src/auth/status.test.ts +++ b/src/auth/status.test.ts @@ -1,30 +1,31 @@ -import { Command } from 'commander' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Command } from 'commander' +import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' import { formatJson, formatNdjson } from '../json.js' +import { + type TestAccount as Account, + type TokenStoreHarness, + alanGrant, + buildTokenStore, +} from '../test-support/accounts.js' +import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { attachStatusCommand } from './status.js' import type { TokenStore } from './types.js' -type Account = { id: string; label?: string; email: string } - -const account: Account = { id: '1', label: 'me', email: 'a@b' } +const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok', account }, -): { - store: TokenStore - activeSpy: ReturnType -} { - const activeSpy = vi.fn(async () => initial) - const store: TokenStore = { - active: activeSpy, - set: vi.fn(), - clear: vi.fn(), - list: vi.fn(async () => (initial ? [{ account: initial.account, isDefault: true }] : [])), - setDefault: vi.fn(), - } - return { store, activeSpy } +): TokenStoreHarness { + // Drop the bundle read so `fetchLive` resolves via `active()` (access token + // only) — these suites assert the no-`bundle` `fetchLive` ctx shape. + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, token: initial.token }] + : [], + overrides: { activeBundle: undefined }, + }) } function build( @@ -36,9 +37,7 @@ function build( renderText: ReturnType } { const store = storeOverride ?? buildStore().store - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const renderText = vi.fn((ctx: { account: Account }) => `Signed in as ${ctx.account.email}`) attachStatusCommand(auth, { store, @@ -49,15 +48,7 @@ function build( } describe('attachStatusCommand', () => { - let logSpy: ReturnType - - beforeEach(() => { - logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}) - }) - - afterEach(() => { - logSpy.mockRestore() - }) + const logSpy = installConsoleLogSpy() it('emits renderText output in plain mode', async () => { const { program, renderText } = build() @@ -69,7 +60,7 @@ describe('attachStatusCommand', () => { view: { json: false, ndjson: false }, flags: {}, }) - expect(logSpy).toHaveBeenCalledWith('Signed in as a@b') + expect(logSpy()).toHaveBeenCalledWith('Signed in as alan@ingen.com') }) it('emits each line when renderText returns an array', async () => { @@ -78,7 +69,9 @@ describe('attachStatusCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'status']) - const emitted = logSpy.mock.calls.map((call: unknown[]) => call[0]).join('\n') + const emitted = logSpy() + .mock.calls.map((call: unknown[]) => call[0]) + .join('\n') expect(emitted).toBe('line 1\nline 2\nline 3') }) @@ -88,7 +81,7 @@ describe('attachStatusCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'status', '--json']) expect(renderText).not.toHaveBeenCalled() - expect(logSpy).toHaveBeenCalledWith(formatJson(account)) + expect(logSpy()).toHaveBeenCalledWith(formatJson(account)) }) it('emits renderJson payload when supplied under --json', async () => { @@ -101,7 +94,7 @@ describe('attachStatusCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'status', '--json']) expect(renderJson).toHaveBeenCalledWith({ account, flags: {} }) - expect(logSpy).toHaveBeenCalledWith(formatJson({ id: '1', email: 'a@b' })) + expect(logSpy()).toHaveBeenCalledWith(formatJson({ id: '1', email: 'alan@ingen.com' })) }) it('emits a single NDJSON line under --ndjson', async () => { @@ -109,7 +102,7 @@ describe('attachStatusCommand', () => { await program.parseAsync(['node', 'cli', 'auth', 'status', '--ndjson']) - expect(logSpy).toHaveBeenCalledWith(formatNdjson([account])) + expect(logSpy()).toHaveBeenCalledWith(formatNdjson([account])) }) it('does not invoke renderJson in human mode', async () => { @@ -122,7 +115,7 @@ describe('attachStatusCommand', () => { }) it('runs fetchLive and uses its returned account for rendering', async () => { - const live: Account = { id: '1', label: 'me', email: 'live@b' } + const live: Account = { ...alanGrant, email: 'live@ingen.com' } const fetchLive = vi.fn(async () => live) const { program, renderText } = build({ fetchLive }) @@ -139,7 +132,7 @@ describe('attachStatusCommand', () => { view: { json: false, ndjson: false }, flags: {}, }) - expect(logSpy).toHaveBeenCalledWith('Signed in as live@b') + expect(logSpy()).toHaveBeenCalledWith('Signed in as live@ingen.com') }) it('propagates fetchLive throws', async () => { @@ -161,7 +154,7 @@ describe('attachStatusCommand', () => { constructor: CliError, code: 'NOT_AUTHENTICATED', }) - expect(logSpy).not.toHaveBeenCalled() + expect(logSpy()).not.toHaveBeenCalled() }) it('awaits an async onNotAuthenticated when supplied instead of throwing', async () => { @@ -183,9 +176,7 @@ describe('attachStatusCommand', () => { }) it('exposes consumer-attached options in flags but strips --json / --ndjson', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const renderText = vi.fn((ctx: { account: Account }) => `Signed in as ${ctx.account.email}`) const status = attachStatusCommand(auth, { store: buildStore().store, @@ -203,8 +194,7 @@ describe('attachStatusCommand', () => { }) it('returns the new Command so the consumer can chain', () => { - const program = new Command() - const auth = program.command('auth') + const { parent: auth } = buildProgram('auth') const status = attachStatusCommand(auth, { store: buildStore().store, renderText: () => 'ok', @@ -217,9 +207,9 @@ describe('attachStatusCommand', () => { const built = buildStore() const { program, renderText } = build({}, built.store) - await program.parseAsync(['node', 'cli', 'auth', 'status', '--user', 'alice@example']) + await program.parseAsync(['node', 'cli', 'auth', 'status', '--user', 'alan@ingen.com']) - expect(built.activeSpy).toHaveBeenCalledWith('alice@example') + expect(built.activeSpy).toHaveBeenCalledWith('alan@ingen.com') expect(renderText).toHaveBeenCalledWith({ account, view: { json: false, ndjson: false }, diff --git a/src/auth/token-view.test.ts b/src/auth/token-view.test.ts index 2907151..569990f 100644 --- a/src/auth/token-view.test.ts +++ b/src/auth/token-view.test.ts @@ -1,47 +1,36 @@ -import { Command } from 'commander' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' +import { + type TestAccount as Account, + type TokenStoreHarness, + alanGrant, + buildTokenStore, +} from '../test-support/accounts.js' +import { buildProgram, installStdoutSpy } from '../test-support/cli-harness.js' import { attachTokenViewCommand } from './token-view.js' -import type { TokenStore } from './types.js' -type Account = { id: string; label?: string; email: string } - -const account: Account = { id: '1', label: 'me', email: 'a@b' } +const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok-xyz', account }, -): { - store: TokenStore - activeSpy: ReturnType -} { - const activeSpy = vi.fn(async () => initial) - const store: TokenStore = { - active: activeSpy, - set: vi.fn(), - clear: vi.fn(), - list: vi.fn(async () => (initial ? [{ account: initial.account, isDefault: true }] : [])), - setDefault: vi.fn(), - } - return { store, activeSpy } +): TokenStoreHarness { + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, token: initial.token }] + : [], + }) } describe('attachTokenViewCommand', () => { - let stdoutSpy: ReturnType - - beforeEach(() => { - stdoutSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) - }) + const stdoutSpy = installStdoutSpy() afterEach(() => { - stdoutSpy.mockRestore() vi.unstubAllEnvs() }) it('writes exactly the bare token (no trailing newline) when stdout is not a TTY', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore() attachTokenViewCommand(auth, { store }) @@ -56,15 +45,15 @@ describe('attachTokenViewCommand', () => { }) } - const emitted = stdoutSpy.mock.calls.map((call: unknown[]) => call[0]).join('') + const emitted = stdoutSpy() + .mock.calls.map((call: unknown[]) => call[0]) + .join('') expect(emitted).toBe('tok-xyz') - expect(stdoutSpy).toHaveBeenCalledTimes(1) + expect(stdoutSpy()).toHaveBeenCalledTimes(1) }) it('appends a newline only when stdout is a TTY', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore() attachTokenViewCommand(auth, { store }) @@ -79,15 +68,15 @@ describe('attachTokenViewCommand', () => { }) } - const emitted = stdoutSpy.mock.calls.map((call: unknown[]) => call[0]).join('') + const emitted = stdoutSpy() + .mock.calls.map((call: unknown[]) => call[0]) + .join('') expect(emitted).toBe('tok-xyz\n') }) it('throws CliError(TOKEN_FROM_ENV) when envVarName is set and env is populated', async () => { vi.stubEnv('TODOIST_API_TOKEN', 'env-token') - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store, activeSpy } = buildStore() attachTokenViewCommand(auth, { store, envVarName: 'TODOIST_API_TOKEN' }) @@ -96,26 +85,22 @@ describe('attachTokenViewCommand', () => { code: 'TOKEN_FROM_ENV', }) expect(activeSpy).not.toHaveBeenCalled() - expect(stdoutSpy).not.toHaveBeenCalled() + expect(stdoutSpy()).not.toHaveBeenCalled() }) it('prints normally when envVarName is set but env is empty', async () => { vi.stubEnv('TODOIST_API_TOKEN', '') - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore() attachTokenViewCommand(auth, { store, envVarName: 'TODOIST_API_TOKEN' }) await program.parseAsync(['node', 'cli', 'auth', 'token']) - expect(stdoutSpy).toHaveBeenCalledWith('tok-xyz') + expect(stdoutSpy()).toHaveBeenCalledWith('tok-xyz') }) it('throws CliError(NOT_AUTHENTICATED) when the store is empty', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore(null) attachTokenViewCommand(auth, { store }) @@ -123,25 +108,22 @@ describe('attachTokenViewCommand', () => { constructor: CliError, code: 'NOT_AUTHENTICATED', }) - expect(stdoutSpy).not.toHaveBeenCalled() + expect(stdoutSpy()).not.toHaveBeenCalled() }) it('registers under a custom name when supplied', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore() const cmd = attachTokenViewCommand(auth, { store, name: 'view' }) expect(cmd.name()).toBe('view') await program.parseAsync(['node', 'cli', 'auth', 'view']) - expect(stdoutSpy).toHaveBeenCalledWith('tok-xyz') + expect(stdoutSpy()).toHaveBeenCalledWith('tok-xyz') }) it('returns the new Command so the consumer can chain', () => { - const program = new Command() - const auth = program.command('auth') + const { parent: auth } = buildProgram('auth') const { store } = buildStore() const cmd = attachTokenViewCommand(auth, { store }) @@ -149,35 +131,29 @@ describe('attachTokenViewCommand', () => { }) it('threads --user ref to store.active(ref) and prints the matched token', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store, activeSpy } = buildStore() attachTokenViewCommand(auth, { store }) - await program.parseAsync(['node', 'cli', 'auth', 'token', '--user', 'alice@example']) + await program.parseAsync(['node', 'cli', 'auth', 'token', '--user', 'alan@ingen.com']) - expect(activeSpy).toHaveBeenCalledWith('alice@example') - expect(stdoutSpy).toHaveBeenCalledWith('tok-xyz') + expect(activeSpy).toHaveBeenCalledWith('alan@ingen.com') + expect(stdoutSpy()).toHaveBeenCalledWith('tok-xyz') }) it('calls store.active(undefined) when --user is absent', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store, activeSpy } = buildStore() attachTokenViewCommand(auth, { store }) await program.parseAsync(['node', 'cli', 'auth', 'token']) expect(activeSpy).toHaveBeenCalledWith(undefined) - expect(stdoutSpy).toHaveBeenCalledWith('tok-xyz') + expect(stdoutSpy()).toHaveBeenCalledWith('tok-xyz') }) it('throws ACCOUNT_NOT_FOUND when --user does not match a stored account', async () => { - const program = new Command() - program.exitOverride() - const auth = program.command('auth') + const { program, parent: auth } = buildProgram('auth') const { store } = buildStore(null) attachTokenViewCommand(auth, { store }) @@ -187,6 +163,6 @@ describe('attachTokenViewCommand', () => { constructor: CliError, code: 'ACCOUNT_NOT_FOUND', }) - expect(stdoutSpy).not.toHaveBeenCalled() + expect(stdoutSpy()).not.toHaveBeenCalled() }) }) diff --git a/src/auth/user-flag.test.ts b/src/auth/user-flag.test.ts index a2c902c..d61d0b1 100644 --- a/src/auth/user-flag.test.ts +++ b/src/auth/user-flag.test.ts @@ -2,30 +2,30 @@ import { Command } from 'commander' import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' +import { + type TestAccount as Account, + alanGrant, + buildTokenStore, +} from '../test-support/accounts.js' +import { buildProgram } from '../test-support/cli-harness.js' import type { TokenStore } from './types.js' import { attachUserFlag, extractUserRef, requireSnapshotForRef } from './user-flag.js' -type Account = { id: string; label?: string; email: string } - -const account: Account = { id: '1', label: 'me', email: 'a@b' } +const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok', account }, ): TokenStore { - return { - active: vi.fn(async () => initial), - set: vi.fn(), - clear: vi.fn(), - list: vi.fn(async () => (initial ? [{ account: initial.account, isDefault: true }] : [])), - setDefault: vi.fn(), - } + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, token: initial.token }] + : [], + }).store } describe('attachUserFlag', () => { it('attaches `--user ` with a generic description', async () => { - const program = new Command() - program.exitOverride() - const sub = program.command('sub') + const { program, parent: sub } = buildProgram('sub') attachUserFlag(sub).action(() => {}) await program.parseAsync(['node', 'cli', 'sub', '--user', 'alice']) @@ -60,10 +60,10 @@ describe('requireSnapshotForRef', () => { it('returns the snapshot when ref matches', async () => { const store = buildStore({ token: 'tok', account }) - const snapshot = await requireSnapshotForRef(store, 'alice') + const snapshot = await requireSnapshotForRef(store, 'alan@ingen.com') expect(snapshot).toEqual({ token: 'tok', account }) - expect(store.active).toHaveBeenCalledWith('alice') + expect(store.active).toHaveBeenCalledWith('alan@ingen.com') }) it('returns null when ref is undefined and the store is empty', async () => { diff --git a/src/test-support/accounts.ts b/src/test-support/accounts.ts new file mode 100644 index 0000000..abff5f2 --- /dev/null +++ b/src/test-support/accounts.ts @@ -0,0 +1,160 @@ +import { vi } from 'vitest' + +import type { + AccountRef, + ActiveBundleSnapshot, + AuthAccount, + ClearedAccount, + TokenBundle, + TokenStore, +} from '../auth/types.js' +import { CliError } from '../errors.js' + +// Shared account fixtures + a canonical in-memory `TokenStore` mock for the +// auth test suites. Lives under `src/test-support/` so it's excluded from the +// build (per `tsconfig.build.json`) and never reaches consumers via `dist/`. + +export type TestAccount = { id: string; label?: string; email: string } + +export const alanGrant: TestAccount = { id: '1', label: 'Alan Grant', email: 'alan@ingen.com' } +export const ellieSattler: TestAccount = { + id: '2', + label: 'Ellie Sattler', + email: 'ellie@ingen.com', +} +export const ianMalcolm: TestAccount = { id: '3', label: 'Ian Malcolm', email: 'ian@ingen.com' } +export const johnHammond: TestAccount = { id: '4', label: 'John Hammond', email: 'john@ingen.com' } + +export type StoreEntry = { + account: TAccount + isDefault: boolean + /** Access token returned by `active()`. Defaults to `bundle.accessToken` or `token-`. */ + token?: string + /** Full bundle returned by `activeBundle()`. */ + bundle?: TokenBundle +} + +/** Fresh default seed: Alan (default) + Ellie. Copy so callers can mutate safely. */ +export function ingenEntries(): StoreEntry[] { + return [ + { account: alanGrant, isDefault: true }, + { account: ellieSattler, isDefault: false }, + ] +} + +/** Stores own the matching rule; the mock matches by id, email, or label. */ +function matchesRef(account: AuthAccount, ref: string): boolean { + return account.id === ref || account.email === ref || account.label === ref +} + +export type TokenStoreHarness = { + store: TokenStore + activeSpy: ReturnType + setSpy: ReturnType + clearSpy: ReturnType + listSpy: ReturnType + setDefaultSpy: ReturnType + activeBundleSpy: ReturnType + setBundleSpy: ReturnType + state: { + /** Live, mutable entry list — tests may reassign `bundle`/`token` to simulate rotation. */ + entries: StoreEntry[] + setBundleCalls: { account: TAccount; bundle: TokenBundle; options?: unknown }[] + } +} + +/** + * Canonical stateful, multi-account `TokenStore` mock. Models the full contract + * over a mutable entry list: id/email/label matching, default re-pinning, + * token-free removal returning `ClearedAccount`, and optional bundle read/write. + * Pass `overrides` to replace (or delete, via `{ method: undefined }`) any + * method for a specific scenario. Returns the store plus per-method spies and a + * live `state`. + */ +export function buildTokenStore( + opts: { entries?: StoreEntry[]; overrides?: Partial> } = {}, +): TokenStoreHarness { + const seed = (opts.entries ?? (ingenEntries() as unknown as StoreEntry[])).map( + (entry) => ({ ...entry }), + ) + const entries: StoreEntry[] = seed + const setBundleCalls: TokenStoreHarness['state']['setBundleCalls'] = [] + + const tokenFor = (entry: StoreEntry): string => + entry.token ?? entry.bundle?.accessToken ?? `token-${entry.account.id}` + const find = (ref?: AccountRef): StoreEntry | undefined => + ref === undefined + ? entries.find((entry) => entry.isDefault) + : entries.find((entry) => matchesRef(entry.account, ref)) + + const activeSpy = vi.fn(async (ref?: AccountRef) => { + const entry = find(ref) + return entry ? { token: tokenFor(entry), account: entry.account } : null + }) + const setSpy = vi.fn(async (account: TAccount, token: string) => { + const entry = entries.find((e) => e.account.id === account.id) + if (entry) entry.token = token + else entries.push({ account, isDefault: entries.length === 0, token }) + }) + const clearSpy = vi.fn(async (ref?: AccountRef): Promise | null> => { + const idx = entries.findIndex((entry) => + ref === undefined ? entry.isDefault : matchesRef(entry.account, ref), + ) + if (idx === -1) return null + const [removed] = entries.splice(idx, 1) + return { account: removed.account, wasDefault: removed.isDefault } + }) + const listSpy = vi.fn(async () => + entries.map((entry) => ({ account: entry.account, isDefault: entry.isDefault })), + ) + const setDefaultSpy = vi.fn(async (ref: AccountRef) => { + const target = entries.find((entry) => matchesRef(entry.account, ref)) + if (!target) throw new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${ref}".`) + for (const entry of entries) entry.isDefault = entry === target + }) + const activeBundleSpy = vi.fn( + async (ref?: AccountRef): Promise | null> => { + const entry = find(ref) + return entry?.bundle ? { account: entry.account, bundle: entry.bundle } : null + }, + ) + const setBundleSpy = vi.fn( + async (account: TAccount, bundle: TokenBundle, options?: unknown) => { + setBundleCalls.push({ account, bundle, options }) + let entry = entries.find((e) => e.account.id === account.id) + if (entry) entry.bundle = bundle + else { + entry = { account, isDefault: false, bundle } + entries.push(entry) + } + // Honour `promoteDefault: true` (first login) by re-pinning the default; + // a silent refresh omits it so a background rotation can't re-pin selection. + if ((options as { promoteDefault?: boolean } | undefined)?.promoteDefault) { + for (const e of entries) e.isDefault = e === entry + } + }, + ) + + const store: TokenStore = { + active: activeSpy as unknown as TokenStore['active'], + set: setSpy as unknown as TokenStore['set'], + clear: clearSpy as unknown as TokenStore['clear'], + list: listSpy as unknown as TokenStore['list'], + setDefault: setDefaultSpy as unknown as TokenStore['setDefault'], + activeBundle: activeBundleSpy as unknown as TokenStore['activeBundle'], + setBundle: setBundleSpy as unknown as TokenStore['setBundle'], + ...opts.overrides, + } + + return { + store, + activeSpy, + setSpy, + clearSpy, + listSpy, + setDefaultSpy, + activeBundleSpy, + setBundleSpy, + state: { entries, setBundleCalls }, + } +} diff --git a/src/test-support/cli-harness.ts b/src/test-support/cli-harness.ts new file mode 100644 index 0000000..5b8c8f7 --- /dev/null +++ b/src/test-support/cli-harness.ts @@ -0,0 +1,50 @@ +import { Command } from 'commander' +import { afterEach, beforeEach, vi } from 'vitest' + +// Shared test scaffolding for the Commander attacher suites. Internal-only +// (under `src/test-support/`, excluded from the build). + +type Spy = ReturnType + +/** + * Register `beforeEach`/`afterEach` hooks that silence and spy on `console.log`, + * returning a getter for the live spy. Call once at the top of a `describe`: + * + * ```ts + * const logSpy = installConsoleLogSpy() + * it('...', () => { expect(logSpy()).toHaveBeenCalledWith('…') }) + * ``` + */ +export function installConsoleLogSpy(): () => Spy { + let spy: Spy + beforeEach(() => { + spy = vi.spyOn(console, 'log').mockImplementation(() => {}) + }) + afterEach(() => { + spy.mockRestore() + }) + return () => spy +} + +/** Same as {@link installConsoleLogSpy} for `process.stdout.write` (pipe-safe output). */ +export function installStdoutSpy(): () => Spy { + let spy: Spy + beforeEach(() => { + spy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) + }) + afterEach(() => { + spy.mockRestore() + }) + return () => spy +} + +/** + * Build a Commander program with `exitOverride()` and a single named parent + * subcommand to attach to — the boilerplate every attacher suite repeats. + */ +export function buildProgram(parentName: string): { program: Command; parent: Command } { + const program = new Command() + program.exitOverride() + const parent = program.command(parentName) + return { program, parent } +} From 99f1691a98daf14175e779a169ea2c14a000e54a Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 18:00:23 +0100 Subject: [PATCH 2/5] test: trim unused surface from buildTokenStore harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the setSpy / activeBundleSpy / setBundleSpy handles from the returned harness (no test reads them — refresh asserts via state.setBundleCalls, flow makes its own spy) and stop exporting the internal ingenEntries default seed. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/test-support/accounts.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/test-support/accounts.ts b/src/test-support/accounts.ts index abff5f2..6638403 100644 --- a/src/test-support/accounts.ts +++ b/src/test-support/accounts.ts @@ -35,7 +35,7 @@ export type StoreEntry = { } /** Fresh default seed: Alan (default) + Ellie. Copy so callers can mutate safely. */ -export function ingenEntries(): StoreEntry[] { +function ingenEntries(): StoreEntry[] { return [ { account: alanGrant, isDefault: true }, { account: ellieSattler, isDefault: false }, @@ -50,12 +50,9 @@ function matchesRef(account: AuthAccount, ref: string): boolean { export type TokenStoreHarness = { store: TokenStore activeSpy: ReturnType - setSpy: ReturnType clearSpy: ReturnType listSpy: ReturnType setDefaultSpy: ReturnType - activeBundleSpy: ReturnType - setBundleSpy: ReturnType state: { /** Live, mutable entry list — tests may reassign `bundle`/`token` to simulate rotation. */ entries: StoreEntry[] @@ -149,12 +146,9 @@ export function buildTokenStore( return { store, activeSpy, - setSpy, clearSpy, listSpy, setDefaultSpy, - activeBundleSpy, - setBundleSpy, state: { entries, setBundleCalls }, } } From 49bdc10e6ebdb7dc65baefebf632192935a4c782 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 18:08:35 +0100 Subject: [PATCH 3/5] test: drop unused johnHammond fixture Only three account fixtures are needed; remove the fourth. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/test-support/accounts.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test-support/accounts.ts b/src/test-support/accounts.ts index 6638403..ddeb79f 100644 --- a/src/test-support/accounts.ts +++ b/src/test-support/accounts.ts @@ -23,7 +23,6 @@ export const ellieSattler: TestAccount = { email: 'ellie@ingen.com', } export const ianMalcolm: TestAccount = { id: '3', label: 'Ian Malcolm', email: 'ian@ingen.com' } -export const johnHammond: TestAccount = { id: '4', label: 'John Hammond', email: 'john@ingen.com' } export type StoreEntry = { account: TAccount From 68dd05fb9a6fc8a973b84f23db66d1340348b1da Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 18:19:21 +0100 Subject: [PATCH 4/5] test: address review on shared test fixtures - buildTokenStore now mirrors KeyringTokenStore's default-selection: effective default = pinned ?? sole account; promote only when unpinned; set/setBundle replace the prior credential shape (drop the stale slot). Removes states production can't produce (a lone survivor is now the effective default after the pinned account is removed). - Tighten the API with overloads so the default Ingen seed only applies to TestAccount; other TAccounts must pass explicit entries (drops the unsound cast at call sites). - Reuse accountNotFoundError() from auth/user-flag so the mock's ACCOUNT_NOT_FOUND wording can't drift from production. - Add buildSingleEntryStore() and adopt it in logout/status/token-view/ user-flag; share ingenEntries() for account.test's fixture; extract an installSpy() lifecycle factory; name the repeated build-helper return type; assert both write paths stay untouched in flow's abort test. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/auth/account.test.ts | 33 +++------ src/auth/flow.test.ts | 4 + src/auth/logout.test.ts | 8 +- src/auth/persist.test.ts | 2 +- src/auth/status.test.ts | 9 +-- src/auth/token-view.test.ts | 8 +- src/auth/user-flag.test.ts | 8 +- src/test-support/accounts.ts | 127 ++++++++++++++++++++++---------- src/test-support/cli-harness.ts | 36 ++++----- 9 files changed, 133 insertions(+), 102 deletions(-) diff --git a/src/auth/account.test.ts b/src/auth/account.test.ts index aea6e15..e1ca089 100644 --- a/src/auth/account.test.ts +++ b/src/auth/account.test.ts @@ -1,3 +1,4 @@ +import type { Command } from 'commander' import { describe, expect, it, vi } from 'vitest' import { CliError } from '../errors.js' @@ -7,6 +8,7 @@ import { alanGrant, buildTokenStore, ellieSattler, + ingenEntries, } from '../test-support/accounts.js' import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { @@ -21,18 +23,14 @@ import { } from './account.js' import type { TokenStore } from './types.js' -const bothAccounts = [ - { account: alanGrant, isDefault: true }, - { account: ellieSattler, isDefault: false }, -] +const bothAccounts = ingenEntries() + +type BuiltAccountCommand = { program: Command; command: Command } function buildList( overrides: Partial> = {}, store?: TokenStore, -): { - program: ReturnType['program'] - command: ReturnType['parent'] -} { +): BuiltAccountCommand { const { program, parent } = buildProgram('account') const command = attachAccountListCommand(parent, { store: store ?? buildTokenStore().store, @@ -44,10 +42,7 @@ function buildList( function buildUse( overrides: Partial> = {}, store?: TokenStore, -): { - program: ReturnType['program'] - command: ReturnType['parent'] -} { +): BuiltAccountCommand { const { program, parent } = buildProgram('account') const command = attachAccountUseCommand(parent, { store: store ?? buildTokenStore().store, @@ -59,10 +54,7 @@ function buildUse( function buildCurrent( overrides: Partial> = {}, store?: TokenStore, -): { - program: ReturnType['program'] - command: ReturnType['parent'] -} { +): BuiltAccountCommand { const { program, parent } = buildProgram('account') const command = attachAccountCurrentCommand(parent, { store: store ?? buildTokenStore().store, @@ -74,10 +66,7 @@ function buildCurrent( function buildRemove( overrides: Partial> = {}, store?: TokenStore, -): { - program: ReturnType['program'] - command: ReturnType['parent'] -} { +): BuiltAccountCommand { const { program, parent } = buildProgram('account') const command = attachAccountRemoveCommand(parent, { store: store ?? buildTokenStore().store, @@ -587,7 +576,7 @@ describe('attachAccountRemoveCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'remove', 'alan@ingen.com']) expect(built.clearSpy).toHaveBeenCalledWith('alan@ingen.com') - expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: false }]) + expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: true }]) expect(logSpy()).toHaveBeenCalledOnce() expect(logSpy()).toHaveBeenCalledWith('✓ Removed Alan Grant (default)') }) @@ -625,7 +614,7 @@ describe('attachAccountRemoveCommand', () => { await program.parseAsync(['node', 'cli', 'account', 'remove', 'alan@ingen.com']) expect(built.store.active).not.toHaveBeenCalled() - expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: false }]) + expect(await built.store.list()).toEqual([{ account: ellieSattler, isDefault: true }]) expect(logSpy()).toHaveBeenCalledWith('✓ Removed Alan Grant (default)') }) diff --git a/src/auth/flow.test.ts b/src/auth/flow.test.ts index a1b2b7c..8ac5d76 100644 --- a/src/auth/flow.test.ts +++ b/src/auth/flow.test.ts @@ -240,6 +240,9 @@ describe('runOAuthFlow', () => { const { provider, getRedirect } = instrument() const store = fakeStore() const setSpy = vi.spyOn(store, 'set') + // `fakeStore` now implements `setBundle` too, so prove neither write + // path ran — not just `set()`. + const setBundleSpy = vi.spyOn(store, 'setBundle') await expect( runOAuthFlow( @@ -258,6 +261,7 @@ describe('runOAuthFlow', () => { ), ).rejects.toMatchObject({ code: 'AUTH_OAUTH_FAILED' }) expect(setSpy).not.toHaveBeenCalled() + expect(setBundleSpy).not.toHaveBeenCalled() }) it('always surfaces the authorize URL via onAuthorizeUrl, even when openBrowser succeeds', async () => { diff --git a/src/auth/logout.test.ts b/src/auth/logout.test.ts index 98d4ea2..0a33532 100644 --- a/src/auth/logout.test.ts +++ b/src/auth/logout.test.ts @@ -7,7 +7,7 @@ import { type TestAccount as Account, type TokenStoreHarness, alanGrant, - buildTokenStore, + buildSingleEntryStore, } from '../test-support/accounts.js' import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { attachLogoutCommand } from './logout.js' @@ -20,11 +20,7 @@ const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok', account }, ): TokenStoreHarness { - return buildTokenStore({ - entries: initial - ? [{ account: initial.account, isDefault: true, token: initial.token }] - : [], - }) + return buildSingleEntryStore(initial) } function build( diff --git a/src/auth/persist.test.ts b/src/auth/persist.test.ts index aa41f39..fbe9113 100644 --- a/src/auth/persist.test.ts +++ b/src/auth/persist.test.ts @@ -19,7 +19,7 @@ const bundle: TokenBundle = { // Default to a store that implements neither bundle method, so persistBundle's // fallback path is the baseline; tests opt into `setBundle`/`set` via overrides. function fakeStore(overrides: Partial> = {}): TokenStore { - return buildTokenStore({ + return buildTokenStore({ overrides: { setBundle: undefined, activeBundle: undefined, ...overrides }, }).store } diff --git a/src/auth/status.test.ts b/src/auth/status.test.ts index 3b54965..c3e36d4 100644 --- a/src/auth/status.test.ts +++ b/src/auth/status.test.ts @@ -7,7 +7,7 @@ import { type TestAccount as Account, type TokenStoreHarness, alanGrant, - buildTokenStore, + buildSingleEntryStore, } from '../test-support/accounts.js' import { buildProgram, installConsoleLogSpy } from '../test-support/cli-harness.js' import { attachStatusCommand } from './status.js' @@ -20,12 +20,7 @@ function buildStore( ): TokenStoreHarness { // Drop the bundle read so `fetchLive` resolves via `active()` (access token // only) — these suites assert the no-`bundle` `fetchLive` ctx shape. - return buildTokenStore({ - entries: initial - ? [{ account: initial.account, isDefault: true, token: initial.token }] - : [], - overrides: { activeBundle: undefined }, - }) + return buildSingleEntryStore(initial, { activeBundle: undefined }) } function build( diff --git a/src/auth/token-view.test.ts b/src/auth/token-view.test.ts index 569990f..99e7efb 100644 --- a/src/auth/token-view.test.ts +++ b/src/auth/token-view.test.ts @@ -5,7 +5,7 @@ import { type TestAccount as Account, type TokenStoreHarness, alanGrant, - buildTokenStore, + buildSingleEntryStore, } from '../test-support/accounts.js' import { buildProgram, installStdoutSpy } from '../test-support/cli-harness.js' import { attachTokenViewCommand } from './token-view.js' @@ -15,11 +15,7 @@ const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok-xyz', account }, ): TokenStoreHarness { - return buildTokenStore({ - entries: initial - ? [{ account: initial.account, isDefault: true, token: initial.token }] - : [], - }) + return buildSingleEntryStore(initial) } describe('attachTokenViewCommand', () => { diff --git a/src/auth/user-flag.test.ts b/src/auth/user-flag.test.ts index d61d0b1..394bf52 100644 --- a/src/auth/user-flag.test.ts +++ b/src/auth/user-flag.test.ts @@ -5,7 +5,7 @@ import { CliError } from '../errors.js' import { type TestAccount as Account, alanGrant, - buildTokenStore, + buildSingleEntryStore, } from '../test-support/accounts.js' import { buildProgram } from '../test-support/cli-harness.js' import type { TokenStore } from './types.js' @@ -16,11 +16,7 @@ const account = alanGrant function buildStore( initial: { token: string; account: Account } | null = { token: 'tok', account }, ): TokenStore { - return buildTokenStore({ - entries: initial - ? [{ account: initial.account, isDefault: true, token: initial.token }] - : [], - }).store + return buildSingleEntryStore(initial).store } describe('attachUserFlag', () => { diff --git a/src/test-support/accounts.ts b/src/test-support/accounts.ts index ddeb79f..07d306c 100644 --- a/src/test-support/accounts.ts +++ b/src/test-support/accounts.ts @@ -8,11 +8,16 @@ import type { TokenBundle, TokenStore, } from '../auth/types.js' -import { CliError } from '../errors.js' +import { accountNotFoundError } from '../auth/user-flag.js' // Shared account fixtures + a canonical in-memory `TokenStore` mock for the // auth test suites. Lives under `src/test-support/` so it's excluded from the // build (per `tsconfig.build.json`) and never reaches consumers via `dist/`. +// +// The mock mirrors `createKeyringTokenStore`'s default-selection rules so tests +// can't assert states production never produces: the *effective default* is the +// pinned default when present, else the sole stored account; promotion only +// pins when nothing is pinned yet. export type TestAccount = { id: string; label?: string; email: string } @@ -26,6 +31,7 @@ export const ianMalcolm: TestAccount = { id: '3', label: 'Ian Malcolm', email: ' export type StoreEntry = { account: TAccount + /** Seeds the pinned default; after construction the default is tracked separately. */ isDefault: boolean /** Access token returned by `active()`. Defaults to `bundle.accessToken` or `token-`. */ token?: string @@ -33,8 +39,8 @@ export type StoreEntry = { bundle?: TokenBundle } -/** Fresh default seed: Alan (default) + Ellie. Copy so callers can mutate safely. */ -function ingenEntries(): StoreEntry[] { +/** Fresh default seed: Alan (pinned default) + Ellie. Copy so callers can mutate safely. */ +export function ingenEntries(): StoreEntry[] { return [ { account: alanGrant, isDefault: true }, { account: ellieSattler, isDefault: false }, @@ -59,29 +65,48 @@ export type TokenStoreHarness = { } } +export function buildTokenStore(opts?: { + entries?: StoreEntry[] + overrides?: Partial> +}): TokenStoreHarness +export function buildTokenStore(opts: { + entries: StoreEntry[] + overrides?: Partial> +}): TokenStoreHarness /** * Canonical stateful, multi-account `TokenStore` mock. Models the full contract - * over a mutable entry list: id/email/label matching, default re-pinning, - * token-free removal returning `ClearedAccount`, and optional bundle read/write. - * Pass `overrides` to replace (or delete, via `{ method: undefined }`) any - * method for a specific scenario. Returns the store plus per-method spies and a - * live `state`. + * over a mutable entry list — id/email/label matching, effective-default + * resolution, promote-if-unpinned, token-free removal returning `ClearedAccount`, + * and bundle read/write with slot replacement. The default Ingen seed only + * applies to `TestAccount`; other `TAccount`s must pass explicit `entries`. + * Pass `overrides` to replace (or delete, via `{ method: undefined }`) any method. */ export function buildTokenStore( opts: { entries?: StoreEntry[]; overrides?: Partial> } = {}, ): TokenStoreHarness { - const seed = (opts.entries ?? (ingenEntries() as unknown as StoreEntry[])).map( - (entry) => ({ ...entry }), - ) - const entries: StoreEntry[] = seed + const entries: StoreEntry[] = ( + opts.entries ?? (ingenEntries() as unknown as StoreEntry[]) + ).map((entry) => ({ ...entry })) + let pinnedDefaultId: string | null = + entries.find((entry) => entry.isDefault)?.account.id ?? null const setBundleCalls: TokenStoreHarness['state']['setBundleCalls'] = [] + // Pinned default if it still resolves, else the sole stored account, else + // none (no records, or several with no pin) — mirrors `effectiveDefault`. + const effectiveDefault = (): StoreEntry | undefined => { + if (pinnedDefaultId) { + const pinned = entries.find((entry) => entry.account.id === pinnedDefaultId) + if (pinned) return pinned + } + return entries.length === 1 ? entries[0] : undefined + } + const promoteIfUnpinned = (id: string): void => { + if (!pinnedDefaultId) pinnedDefaultId = id + } const tokenFor = (entry: StoreEntry): string => entry.token ?? entry.bundle?.accessToken ?? `token-${entry.account.id}` const find = (ref?: AccountRef): StoreEntry | undefined => - ref === undefined - ? entries.find((entry) => entry.isDefault) - : entries.find((entry) => matchesRef(entry.account, ref)) + ref === undefined ? effectiveDefault() : entries.find((e) => matchesRef(e.account, ref)) const activeSpy = vi.fn(async (ref?: AccountRef) => { const entry = find(ref) @@ -89,24 +114,34 @@ export function buildTokenStore( }) const setSpy = vi.fn(async (account: TAccount, token: string) => { const entry = entries.find((e) => e.account.id === account.id) - if (entry) entry.token = token - else entries.push({ account, isDefault: entries.length === 0, token }) + // A fresh write replaces the prior credential shape: drop any stale bundle. + if (entry) { + entry.token = token + entry.bundle = undefined + } else { + entries.push({ account, isDefault: false, token }) + } + promoteIfUnpinned(account.id) }) const clearSpy = vi.fn(async (ref?: AccountRef): Promise | null> => { - const idx = entries.findIndex((entry) => - ref === undefined ? entry.isDefault : matchesRef(entry.account, ref), - ) - if (idx === -1) return null - const [removed] = entries.splice(idx, 1) - return { account: removed.account, wasDefault: removed.isDefault } + const target = find(ref) + if (!target) return null + const wasDefault = effectiveDefault()?.account.id === target.account.id + entries.splice(entries.indexOf(target), 1) + if (pinnedDefaultId === target.account.id) pinnedDefaultId = null + return { account: target.account, wasDefault } + }) + const listSpy = vi.fn(async () => { + const defaultId = effectiveDefault()?.account.id + return entries.map((entry) => ({ + account: entry.account, + isDefault: entry.account.id === defaultId, + })) }) - const listSpy = vi.fn(async () => - entries.map((entry) => ({ account: entry.account, isDefault: entry.isDefault })), - ) const setDefaultSpy = vi.fn(async (ref: AccountRef) => { - const target = entries.find((entry) => matchesRef(entry.account, ref)) - if (!target) throw new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${ref}".`) - for (const entry of entries) entry.isDefault = entry === target + const target = entries.find((e) => matchesRef(e.account, ref)) + if (!target) throw accountNotFoundError(ref) + pinnedDefaultId = target.account.id }) const activeBundleSpy = vi.fn( async (ref?: AccountRef): Promise | null> => { @@ -117,16 +152,16 @@ export function buildTokenStore( const setBundleSpy = vi.fn( async (account: TAccount, bundle: TokenBundle, options?: unknown) => { setBundleCalls.push({ account, bundle, options }) - let entry = entries.find((e) => e.account.id === account.id) - if (entry) entry.bundle = bundle - else { - entry = { account, isDefault: false, bundle } - entries.push(entry) + const entry = entries.find((e) => e.account.id === account.id) + // A fresh write replaces the prior credential shape: drop any stale token. + if (entry) { + entry.bundle = bundle + entry.token = undefined + } else { + entries.push({ account, isDefault: false, bundle }) } - // Honour `promoteDefault: true` (first login) by re-pinning the default; - // a silent refresh omits it so a background rotation can't re-pin selection. if ((options as { promoteDefault?: boolean } | undefined)?.promoteDefault) { - for (const e of entries) e.isDefault = e === entry + promoteIfUnpinned(account.id) } }, ) @@ -151,3 +186,21 @@ export function buildTokenStore( state: { entries, setBundleCalls }, } } + +/** + * Single-account convenience over {@link buildTokenStore} — the shape the + * `logout` / `status` / `token-view` / `user-flag` suites share. `initial` + * mirrors a `store.active()` snapshot (token + account), or `null` for an + * empty store. + */ +export function buildSingleEntryStore( + initial: { token: string; account: TestAccount } | null, + overrides?: Partial>, +): TokenStoreHarness { + return buildTokenStore({ + entries: initial + ? [{ account: initial.account, isDefault: true, token: initial.token }] + : [], + overrides, + }) +} diff --git a/src/test-support/cli-harness.ts b/src/test-support/cli-harness.ts index 5b8c8f7..195362d 100644 --- a/src/test-support/cli-harness.ts +++ b/src/test-support/cli-harness.ts @@ -7,18 +7,15 @@ import { afterEach, beforeEach, vi } from 'vitest' type Spy = ReturnType /** - * Register `beforeEach`/`afterEach` hooks that silence and spy on `console.log`, - * returning a getter for the live spy. Call once at the top of a `describe`: - * - * ```ts - * const logSpy = installConsoleLogSpy() - * it('...', () => { expect(logSpy()).toHaveBeenCalledWith('…') }) - * ``` + * Own the `beforeEach`/`afterEach` spy lifecycle: `register` creates + configures + * a fresh spy before each test (where the target's types are concrete, so no + * casts), and the spy is restored afterwards. Returns a getter for the live spy + * — call it inside the test body, since a new spy is installed per test. */ -export function installConsoleLogSpy(): () => Spy { +function installSpy(register: () => Spy): () => Spy { let spy: Spy beforeEach(() => { - spy = vi.spyOn(console, 'log').mockImplementation(() => {}) + spy = register() }) afterEach(() => { spy.mockRestore() @@ -26,16 +23,21 @@ export function installConsoleLogSpy(): () => Spy { return () => spy } +/** + * Silence + spy on `console.log`. Call once at the top of a `describe`: + * + * ```ts + * const logSpy = installConsoleLogSpy() + * it('...', () => { expect(logSpy()).toHaveBeenCalledWith('…') }) + * ``` + */ +export function installConsoleLogSpy(): () => Spy { + return installSpy(() => vi.spyOn(console, 'log').mockImplementation(() => {})) +} + /** Same as {@link installConsoleLogSpy} for `process.stdout.write` (pipe-safe output). */ export function installStdoutSpy(): () => Spy { - let spy: Spy - beforeEach(() => { - spy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true) - }) - afterEach(() => { - spy.mockRestore() - }) - return () => spy + return installSpy(() => vi.spyOn(process.stdout, 'write').mockImplementation(() => true)) } /** From 870aef0deb3013ff985067dccaf9ea08ad1dd79a Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 18:24:51 +0100 Subject: [PATCH 5/5] docs: add CODEBASE.md repo map A descriptive "what is where" orientation file (companion to AGENTS.md's "how to change"), modelled on todoist-cli's CODEBASE.md but adapted for a library: subpath-export surface, the auth/keyring layout, the attacher pattern, the TokenStore contract, and the test-support harness. Co-Authored-By: Claude Opus 4.7 (1M context) --- CODEBASE.md | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 CODEBASE.md diff --git a/CODEBASE.md b/CODEBASE.md new file mode 100644 index 0000000..c9eaf29 --- /dev/null +++ b/CODEBASE.md @@ -0,0 +1,195 @@ +# CODEBASE.md — Repo Map + +> **Purpose:** a ~2000-token orientation file so Claude (and humans) can navigate +> this repo without exploring. Describes _what is where_; `AGENTS.md` describes +> _how to change things_. Update when structure shifts, not on every new file. + +## What this project is + +`@doist/cli-core` is a **shared TypeScript library** for the three Doist CLIs +(`@doist/todoist-cli`, `@doist/twist-cli`, `@doist/outline-cli`). It is **not a +binary** — it ships reusable building blocks (error type, config I/O, output +formatters, spinner, OAuth/keyring auth runtime, Commander "attachers") that each +CLI composes into its own `program`. + +ESM-only · Node ≥ 20.18.1 · Commander 14 (optional peer) · vitest · oxlint + +oxfmt (no eslint/prettier) · semantic-release on merge to `main`. + +Heavy/optional deps are **optional peer-deps**, pulled in only by the subpath +that needs them (`commander`, `marked`, `marked-terminal-renderer`, +`oauth4webapi`, `open`, `@napi-rs/keyring`, `vitest`). Only `chalk` + +`yocto-spinner` are hard runtime deps. + +## Top-level layout + +``` +/ +├─ src/ # All source. See tree below. +├─ dist/ # Build output (tsc). Never edit. +├─ AGENTS.md # Prescriptive rules (build, code style, README upkeep) +├─ CODEBASE.md # This file — descriptive map +├─ CLAUDE.md # One-liner forward to AGENTS.md +├─ README.md # Public API docs ("What's in it" table + usage) +├─ tsconfig.json # Includes src + tests (type-check, IDE) +├─ tsconfig.build.json # Excludes *.test.ts + test-support/ + __mocks__/ (build/dev) +├─ vitest.config.ts # { globals, root: 'src', include: ['**/*.test.ts'] } +├─ lefthook.yml # Pre-commit: oxfmt + oxlint + type-check + test +└─ release.config.js # semantic-release config +``` + +## Public API surface (`package.json#exports`) + +Each subpath is an independent entry point so JSON-only consumers don't pay for +markdown/OAuth transitive installs. + +| Subpath | Provides | Optional peers needed | +| -------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------- | +| `.` (root) | `CliError`, config I/O, JSON/NDJSON + `emitView`, `printEmpty`, spinner, terminal detection, global-args parser | — (chalk/yocto bundled) | +| `@doist/cli-core/auth` | OAuth runtime, the `attach*Command` registrars, providers, keyring `TokenStore`, refresh, the `TokenStore`/`AuthProvider` contracts | `commander`, `oauth4webapi`, `open`, `@napi-rs/keyring` | +| `@doist/cli-core/commands` | `registerChangelogCommand`, `registerUpdateCommand` + semver helpers | `commander` | +| `@doist/cli-core/markdown` | `preloadMarkdown`, `renderMarkdown` | `marked`, `marked-terminal-renderer` | +| `@doist/cli-core/testing` | `describeEmptyMachineOutput` (public test helper for consumers) | `vitest` | + +The public surface = every re-export through these entry barrels. `tsc --noEmit` +validates the re-exports; there is no separate runtime pinning test (per AGENTS.md). + +## `src/` tree + +``` +src/ +├─ index.ts # Root barrel (the `.` export) +├─ errors.ts # CliError + CliErrorCode aggregator + getErrorMessage +├─ config.ts # XDG config I/O; CoreConfig / UpdateChannel / ConfigErrorCode +├─ json.ts # formatJson / formatNdjson (throw on non-serializable) +├─ options.ts # ViewOptions type + emitView (json/ndjson/human dispatch) +├─ empty.ts # printEmpty (machine-aware empty-state output) +├─ global-args.ts # parseGlobalArgs + spinner/accessible gate factories + stripUserFlag +├─ spinner.ts # createSpinner factory (yocto-spinner wrapper) +├─ terminal.ts # isStdoutTTY / isStdinTTY / isStderrTTY / isCI +├─ markdown.ts # ./markdown subpath (lazy marked + terminal renderer) +├─ testing.ts # ./testing subpath (describeEmptyMachineOutput) +├─ auth/ # ./auth subpath — see below +├─ commands/ # ./commands subpath (changelog, update + semver helpers) +└─ test-support/ # Internal test helpers — EXCLUDED from build, never shipped + ├─ accounts.ts # TestAccount fixtures (Ingen identities) + buildTokenStore / buildSingleEntryStore + ├─ cli-harness.ts # installConsoleLogSpy / installStdoutSpy / buildProgram + └─ keyring-mocks.ts # buildKeyringMap / buildSingleSlot / buildUserRecords +``` + +Every module has a colocated `.test.ts` (31 test files). Subfolders +(`auth/`, `commands/`) follow the same colocated-test rule. + +## `src/auth/` — the OAuth + token-storage subpath + +``` +auth/ +├─ index.ts # ./auth barrel +├─ types.ts # CONTRACTS: AuthProvider, TokenStore, AuthAccount, +│ # TokenBundle, ActiveBundleSnapshot, ClearedAccount, AccountRef +├─ errors.ts # AuthErrorCode union +├─ flow.ts # runOAuthFlow() — PKCE callback-server flow end-to-end +├─ login.ts / logout.ts / status.ts / token-view.ts # attachCommand registrars +├─ account.ts # attachAccountList/Use/Current/Remove command registrars +├─ user-flag.ts # INTERNAL: --user wiring, requireSnapshotForRef, accountNotFoundError +├─ pkce.ts # PKCE primitives (verifier/challenge/state) +├─ persist.ts # persistBundle / bundleFromExchange (setBundle-or-set fallback) +├─ refresh.ts # refreshAccessToken (silent refresh w/ file lock) +├─ providers/ +│ ├─ pkce.ts # createPkceProvider (standard public-client PKCE) +│ ├─ dcr.ts # createDcrProvider (RFC 7591 dynamic client registration) +│ └─ oauth.ts # shared oauth4webapi glue +└─ keyring/ # OS-keyring-backed TokenStore + ├─ index.ts # barrel for the keyring exports + ├─ secure-store.ts # createSecureStore (thin @napi-rs/keyring wrapper) + ├─ token-store.ts # createKeyringTokenStore — the multi-account TokenStore impl + ├─ record-write.ts # bundle/token slot writes + fallback warnings + ├─ migrate.ts # migrateLegacyAuth (v1 plaintext → v2 keyring) + ├─ slot-naming.ts # keyring service/account slug rules + ├─ internal.ts # shared internals + └─ types.ts # UserRecord / UserRecordStore / SecureStore contracts +``` + +**Auth split:** cli-core owns the OAuth flow, keyring `TokenStore`, and the four +command registrars. A consuming CLI supplies (a) a `UserRecordStore` adapter over +its own config file and (b) a provider `validateToken` that maps the access token +to its account shape. See README "Auth (optional subpath)". + +## Attacher pattern + +`attachCommand(parent, options)` is the shared shape across login / logout / +status / token-view / account-list/use/current/remove: + +- Attaches a subcommand to a `parent` Commander command, returns the new + `Command` for chaining. +- Strips the registrar flags (`--json` / `--ndjson` / `--user`) and exposes the + remainder to consumer callbacks as `flags`. +- Machine output: `--json` wins over `--ndjson`; renderers (`renderText` / + `renderJson`) are consumer hooks, invoked only in the relevant mode. +- Errors throw `CliError` with an `AuthErrorCode`; the consumer's top-level + handler renders it. + +## The `TokenStore` contract (`auth/types.ts`) + +The pivot type every auth helper is generic over. Multi-account-shaped: +`active(ref?)`, `set`, `clear(ref?) → ClearedAccount`, `list()`, `setDefault(ref)`, +plus optional `activeAccount` / `activeBundle` / `setBundle` (refresh + `current` +fast-path). Effective default = pinned default if present, else the sole stored +account. `createKeyringTokenStore` is the shipped impl; CLIs may provide their own. + +## `src/test-support/` — internal test helpers (never shipped) + +Excluded from `dist/` by `tsconfig.build.json` and not matched by vitest's +`**/*.test.ts` include, so these files run as helpers, not suites. + +- **`accounts.ts`** — `TestAccount` type + Ingen fixtures (`alanGrant` id 1, + `ellieSattler` 2, `ianMalcolm` 3); `buildTokenStore()` — the canonical stateful + multi-account `TokenStore` mock (mirrors `createKeyringTokenStore`'s + effective-default + promote-if-unpinned + slot-replacement semantics); + `buildSingleEntryStore()` for the single-account suites; `ingenEntries()` default seed. +- **`cli-harness.ts`** — `installConsoleLogSpy()` / `installStdoutSpy()` (own the + beforeEach/afterEach spy lifecycle, return a getter) + `buildProgram(name)` + (the `new Command().exitOverride().command(name)` scaffold). +- **`keyring-mocks.ts`** — `buildKeyringMap` / `buildSingleSlot` / + `buildUserRecords` for the keyring unit suites. + +## Testing + +- **Runner:** vitest. `npm test` (one-shot), `npm run test:watch`. +- **Location:** colocated `*.test.ts` next to the module under test. +- **Account suites:** import fixtures + `buildTokenStore` / `buildSingleEntryStore` + from `test-support/accounts.js` and the spy/scaffold helpers from + `test-support/cli-harness.js` — do NOT hand-roll account objects or store mocks. +- **Pattern:** `const logSpy = installConsoleLogSpy()` at the top of a `describe`, + build via `buildProgram('auth'|'account')`, drive with + `program.parseAsync(['node','cli',…])`. +- No `restoreMocks` in config — the helpers restore their own spies. + +## Build & release + +- **Build:** `tsc -p tsconfig.build.json` → `dist/`. Two-tsconfig setup: + `tsconfig.json` includes tests (type-check/IDE); `tsconfig.build.json` excludes + `*.test.ts` + `src/test-support/` so test-only code never ships. +- **Type-check:** `npm run type-check` (`tsc --noEmit`). +- **Lint/format:** `npm run check` (`oxlint src && oxfmt --check`), `npm run fix`. + **No ESLint, no Prettier.** `npm run check` is the gate — run before a PR. +- **Release:** semantic-release on merge to `main`; Conventional Commits required. + `next` is the pre-release branch. + +## Conventions (quick) + +- Prefer `type` over `interface` for object shapes (per AGENTS.md). +- No dead exports — anything not reached from an entry barrel or a test is deleted. +- New/renamed/removed public export ⇒ update `README.md` in the same commit + (the "What's in it" table + affected usage block) — AGENTS.md "README maintenance". +- Errors: `new CliError(code, message, { hints? })`; codes come from the + per-area unions folded into `CliErrorCode`. +- Status glyphs (`✓`/`✗`) allowed in user-facing output; otherwise no emojis. + +## Start here if new + +1. `README.md` — the public API, with usage blocks per subpath. +2. `src/index.ts` + `src/options.ts` — the root building blocks (`emitView`, `CliError`). +3. `src/auth/types.ts` — the `TokenStore` / `AuthProvider` contracts everything is generic over. +4. `src/auth/status.ts` — canonical attacher; `src/auth/flow.ts` — the OAuth runtime. +5. `src/test-support/accounts.ts` — the shared test harness. +6. `AGENTS.md` — rules you must follow.