From 731fda33f9d1679cd6d7ad1c0676cc37501852bf Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 19:25:03 +0100 Subject: [PATCH 1/2] refactor(auth): consolidate attacher context types onto a shared base The seven exported auth attacher context types plus several inline callback ctxs each re-typed the same `view: Required` + `flags: Record` pair, copy-pasting the identical `view` JSDoc ~11 times. Introduce internal `AttachContextBase` and `WithAccount` aliases in `types.ts` (not re-exported) and derive the named + view-bearing inline ctxs from them. Pure type-alias refactor: structural shapes are byte-for-byte identical, so the public API surface, README, and runtime behaviour are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/auth/account.ts | 41 ++++++++++++-------------------------- src/auth/login.ts | 14 ++----------- src/auth/logout.ts | 8 ++------ src/auth/status.ts | 48 +++++++++++++++++++++------------------------ src/auth/types.ts | 15 ++++++++++++++ 5 files changed, 53 insertions(+), 73 deletions(-) diff --git a/src/auth/account.ts b/src/auth/account.ts index b22eb4b..de03a30 100644 --- a/src/auth/account.ts +++ b/src/auth/account.ts @@ -2,18 +2,20 @@ import type { Command } from 'commander' import { CliError } from '../errors.js' import { formatJson, formatNdjson } from '../json.js' import { type ViewOptions, emitView } from '../options.js' -import type { AccountRef, AuthAccount, TokenStore } from './types.js' +import type { + AccountRef, + AttachContextBase, + AuthAccount, + TokenStore, + WithAccount, +} from './types.js' import { accountNotFoundError } from './user-flag.js' -export type AttachAccountListContext = { +export type AttachAccountListContext = AttachContextBase & { /** Every stored account with its default marker, in store order. */ accounts: ReadonlyArray<{ account: TAccount; isDefault: boolean }> /** The default account's ref (its `id`), or `null` when nothing is stored. */ default: AccountRef | null - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** Consumer-attached options. The registrar flags (`--json`, `--ndjson`) are stripped. */ - flags: Record } export type AttachAccountListCommandOptions = { @@ -49,11 +51,7 @@ export type AttachAccountUseCommandOptions - flags: Record - }): void | Promise + onDefaultSet?(ctx: AttachContextBase & { ref: AccountRef }): void | Promise } /** Split the parsed Commander options into the canonical machine-output view + the remaining consumer flags. */ @@ -234,15 +232,9 @@ export function attachAccountUseCommand = { - /** The resolved active account. */ - account: TAccount +export type AttachAccountCurrentContext = WithAccount & { /** Whether the active account is the store's effective default. */ isDefault: boolean - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** Consumer-attached options. The registrar flags (`--json`, `--ndjson`) are stripped. */ - flags: Record } export type AttachAccountCurrentCommandOptions = { @@ -271,10 +263,7 @@ export type AttachAccountCurrentCommandOptions - flags: Record - }): void | Promise + onNotAuthenticated?(ctx: AttachContextBase): void | Promise } /** @@ -329,17 +318,11 @@ export function attachAccountCurrentCommand = { - /** The account that was removed, as reported by `store.clear()`. */ - account: TAccount +export type AttachAccountRemoveContext = WithAccount & { /** The raw `` positional the user typed. */ ref: AccountRef /** Whether the removed account was the default before clearing. */ wasDefault: boolean - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** Consumer-attached options. The registrar flags (`--json`, `--ndjson`) are stripped. */ - flags: Record } export type AttachAccountRemoveCommandOptions = { diff --git a/src/auth/login.ts b/src/auth/login.ts index 48692d8..1788f3b 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -2,19 +2,9 @@ import type { Command } from 'commander' import { CliError } from '../errors.js' import type { ViewOptions } from '../options.js' import { runOAuthFlow } from './flow.js' -import type { AuthAccount, AuthProvider, TokenStore } from './types.js' +import type { AuthAccount, AuthProvider, TokenStore, WithAccount } from './types.js' -export type AttachLoginContext = { - account: TAccount - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** - * Stripped per-CLI flags — the parsed options object with the standard - * registrar flags (`--read-only`, `--callback-port`, `--json`, `--ndjson`) - * removed. Same view `resolveScopes` saw at flow start. - */ - flags: Record -} +export type AttachLoginContext = WithAccount export type AttachLoginCommandOptions = { provider: AuthProvider diff --git a/src/auth/logout.ts b/src/auth/logout.ts index 941debc..d143087 100644 --- a/src/auth/logout.ts +++ b/src/auth/logout.ts @@ -2,10 +2,10 @@ import type { Command } from 'commander' import { CliError } from '../errors.js' import { formatJson } from '../json.js' import type { ViewOptions } from '../options.js' -import type { AccountRef, AuthAccount, TokenStore } from './types.js' +import type { AccountRef, AttachContextBase, AuthAccount, TokenStore } from './types.js' import { attachUserFlag, extractUserRef, requireSnapshotForRef } from './user-flag.js' -export type AttachLogoutContext = { +export type AttachLogoutContext = AttachContextBase & { /** * The account that was active immediately before `clear()` ran, or * `null` if nothing was stored. Also `null` on the `AUTH_STORE_READ_FAILED` @@ -15,10 +15,6 @@ export type AttachLogoutContext = { account: TAccount | null /** The `--user ` value, or `undefined` when not supplied. Always present so consumers can tell "no stored account" from "cleared an unreadable account by ref". */ ref: AccountRef | undefined - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** Consumer-attached options. The registrar flags (`--json`, `--ndjson`, `--user`) are stripped. */ - flags: Record } export type AttachLogoutRevokeContext = Omit< diff --git a/src/auth/status.ts b/src/auth/status.ts index a9d37eb..4d3f5bc 100644 --- a/src/auth/status.ts +++ b/src/auth/status.ts @@ -2,7 +2,14 @@ import type { Command } from 'commander' import { CliError } from '../errors.js' import { formatJson, formatNdjson } from '../json.js' import type { ViewOptions } from '../options.js' -import type { AccountRef, AuthAccount, TokenBundle, TokenStore } from './types.js' +import type { + AccountRef, + AttachContextBase, + AuthAccount, + TokenBundle, + TokenStore, + WithAccount, +} from './types.js' import { accountNotFoundError, attachUserFlag, @@ -55,13 +62,7 @@ async function resolveStatusSnapshot( return snapshot ? { token: snapshot.token, account: snapshot.account } : null } -export type AttachStatusContext = { - account: TAccount - /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ - view: Required - /** Consumer-attached options (e.g. `--full`). The registrar flags (`--json`, `--ndjson`, `--user`) are stripped. */ - flags: Record -} +export type AttachStatusContext = WithAccount export type AttachStatusCommandOptions = { store: TokenStore @@ -72,19 +73,18 @@ export type AttachStatusCommandOptions - flags: Record - }): Promise + fetchLive?( + ctx: AttachContextBase & { + account: TAccount + token: string + /** + * Full bundle when the store implements `activeBundle` — lets a + * consumer render expiry without a second read. Absent when the + * store only exposes `active()` (no refresh-side metadata available). + */ + bundle?: TokenBundle + }, + ): Promise /** * Human-mode renderer. May return a single string or an array of lines; * lines are joined with `\n` on output. @@ -100,11 +100,7 @@ export type AttachStatusCommandOptions - flags: Record - }): void | Promise + onNotAuthenticated?(ctx: AttachContextBase): void | Promise } /** diff --git a/src/auth/types.ts b/src/auth/types.ts index 15d047d..f258415 100644 --- a/src/auth/types.ts +++ b/src/auth/types.ts @@ -1,3 +1,5 @@ +import type { ViewOptions } from '../options.js' + /** A single authenticated identity. `id` is the stable key the store indexes on. */ export type AuthAccount = { id: string @@ -5,6 +7,19 @@ export type AuthAccount = { [key: string]: unknown } +/** The view + flags pair every auth attacher callback receives. */ +export type AttachContextBase = { + /** `--json` / `--ndjson` flag values, both present (defaulted to `false`). */ + view: Required + /** Consumer-attached options. The standard registrar flags (`--json` / `--ndjson`, and `--user` where attached) are stripped. */ + flags: Record +} + +/** `AttachContextBase` plus the resolved account. */ +export type WithAccount = AttachContextBase & { + account: TAccount +} + export type PrepareInput = { redirectUri: string flags: Record From 0e542d58e2bce7a9a0dd2c6f93b1d9c08d466a76 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sat, 23 May 2026 19:30:18 +0100 Subject: [PATCH 2/2] refactor(auth): address review on context-type consolidation Restore the login flags-stripping invariant and the remove-ctx account doc that the base-type rebase had dropped, reuse WithAccount in the status fetchLive ctx, and drop the WithAccount alias comment that restated the code. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/auth/account.ts | 4 +++- src/auth/login.ts | 5 +++++ src/auth/status.ts | 3 +-- src/auth/types.ts | 1 - 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/auth/account.ts b/src/auth/account.ts index de03a30..911b55d 100644 --- a/src/auth/account.ts +++ b/src/auth/account.ts @@ -318,7 +318,9 @@ export function attachAccountCurrentCommand = WithAccount & { +export type AttachAccountRemoveContext = AttachContextBase & { + /** The account that was removed, as reported by `store.clear()`. */ + account: TAccount /** The raw `` positional the user typed. */ ref: AccountRef /** Whether the removed account was the default before clearing. */ diff --git a/src/auth/login.ts b/src/auth/login.ts index 1788f3b..b9c3d6a 100644 --- a/src/auth/login.ts +++ b/src/auth/login.ts @@ -4,6 +4,11 @@ import type { ViewOptions } from '../options.js' import { runOAuthFlow } from './flow.js' import type { AuthAccount, AuthProvider, TokenStore, WithAccount } from './types.js' +/** + * `flags` is the parsed options with the login registrar flags (`--read-only`, + * `--callback-port`, `--json`, `--ndjson`) stripped — the same view + * `resolveScopes` saw at flow start. + */ export type AttachLoginContext = WithAccount export type AttachLoginCommandOptions = { diff --git a/src/auth/status.ts b/src/auth/status.ts index 4d3f5bc..c86d9d0 100644 --- a/src/auth/status.ts +++ b/src/auth/status.ts @@ -74,8 +74,7 @@ export type AttachStatusCommandOptions & { token: string /** * Full bundle when the store implements `activeBundle` — lets a diff --git a/src/auth/types.ts b/src/auth/types.ts index f258415..c7cca04 100644 --- a/src/auth/types.ts +++ b/src/auth/types.ts @@ -15,7 +15,6 @@ export type AttachContextBase = { flags: Record } -/** `AttachContextBase` plus the resolved account. */ export type WithAccount = AttachContextBase & { account: TAccount }