feat(auth): add refresh-token support to TokenStore + PKCE provider

src/auth/errors.ts

@@ -10,6 +10,12 @@ export type AuthErrorCode =
     | 'AUTH_TOKEN_EXCHANGE_FAILED'
     | 'AUTH_STORE_WRITE_FAILED'
     | 'AUTH_STORE_READ_FAILED'
+    /** Refresh token rejected — typically `invalid_grant`. Forces re-login. */
+    | 'AUTH_REFRESH_EXPIRED'
+    /** Refresh attempt failed transiently (network, 5xx, non-JSON). Caller may retry. */
+    | 'AUTH_REFRESH_TRANSIENT'
+    /** No refresh token stored, or provider doesn't implement `refreshToken`. */
+    | 'AUTH_REFRESH_UNAVAILABLE'
     | 'NOT_AUTHENTICATED'
     | 'TOKEN_FROM_ENV'
     | 'NO_ACCOUNT_SELECTED'

src/auth/flow.test.ts

@@ -22,19 +22,43 @@ import { execFile } from 'node:child_process'
 import { readFileSync } from 'node:fs'
 import openBrowserModule from 'open'
 import { type RunOAuthFlowOptions, runOAuthFlow } from './flow.js'
-import type { AuthProvider, TokenStore } from './types.js'
+import type { AuthProvider, TokenBundle, TokenStore } from './types.js'
 
 type Account = { id: string; label?: string; email: string }
+type FakeStoreState = {
+    account: Account
+    bundle: TokenBundle
+    /** Literal options arg seen by `setBundle` (`undefined` when omitted). */
+    setBundleOptions: { promoteDefault?: boolean } | undefined
+}
 
-/** Tiny in-memory `TokenStore` so the flow tests don't need disk I/O. */
-function fakeStore(): TokenStore<Account> & { last?: { account: Account; token: string } } {
-    const state: { last?: { account: Account; token: string } } = {}
+/**
+ * Tiny in-memory `TokenStore` so the flow tests don't need disk I/O. Keeps
+ * the full `TokenBundle` and the literal `setBundle` options arg so tests
+ * can verify that `runOAuthFlow` persists refresh + expiry AND opts into
+ * `promoteDefault: true` on the explicit login path.
+ */
+function fakeStore(): TokenStore<Account> & { last?: FakeStoreState } {
+    const state: { last?: FakeStoreState } = {}
     return {
         async active() {
-            return state.last ?? null
+            return state.last
+                ? {
+                      token: state.last.bundle.accessToken,
+                      bundle: state.last.bundle,
+                      account: state.last.account,
+                  }
+                : null
         },
         async set(account, token) {
-            state.last = { account, token }
+            state.last = {
+                account,
+                bundle: { accessToken: token },
+                setBundleOptions: undefined,
+            }
+        },
+        async setBundle(account, bundle, setOptions) {
+            state.last = { account, bundle, setBundleOptions: setOptions }
         },
         async clear() {
             state.last = undefined
@@ -151,10 +175,93 @@ describe('runOAuthFlow', () => {
         expect(openBrowser).toHaveBeenCalledTimes(1)
         expect(await store.active()).toEqual({
             token: 'tok-1',
+            bundle: {
+                accessToken: 'tok-1',
+                refreshToken: undefined,
+                accessTokenExpiresAt: undefined,
+                refreshTokenExpiresAt: undefined,
+            },
             account: { id: '1', email: 'a@b' },
         })
     })
 
+    it('persists the full bundle (refresh + access expiry + refresh expiry) when exchangeCode returns them', async () => {
+        const accessExpiresAt = Date.now() + 3_600_000
+        const refreshExpiresAt = Date.now() + 30 * 24 * 3_600_000
+        const { provider, getRedirect } = instrument({
+            exchangeCode: async () => ({
+                accessToken: 'tok-1',
+                refreshToken: 'rt-1',
+                expiresAt: accessExpiresAt,
+                refreshTokenExpiresAt: refreshExpiresAt,
+            }),
+        })
+        const store = fakeStore()
+
+        await runOAuthFlow<Account>(
+            flowOptions({ provider, store, openBrowser: driveCallback(getRedirect) }),
+        )
+
+        // Regression guard: if `runOAuthFlow` drops any of refresh / access
+        // expiry / refresh expiry on the floor, the snapshot would lose
+        // that field and silent re-auth would break.
+        expect(store.last?.bundle).toEqual({
+            accessToken: 'tok-1',
+            refreshToken: 'rt-1',
+            accessTokenExpiresAt: accessExpiresAt,
+            refreshTokenExpiresAt: refreshExpiresAt,
+        })
+        // Login is the canonical default-promotion trigger — the helper
+        // must thread `promoteDefault: true` through. A regression that
+        // dropped the flag would let an empty-default config silently
+        // stay un-pinned after the first login.
+        expect(store.last?.setBundleOptions).toEqual({ promoteDefault: true })
+    })
+
+    it('falls back to set(token) when the store does not implement setBundle (custom-store back-compat)', async () => {
+        // Custom `TokenStore` implementations predating refresh-token
+        // support only ship `set(account, token)`. The shared
+        // `persistBundle` helper degrades gracefully — login still works,
+        // but refresh metadata is lost (subsequent refreshes throw
+        // AUTH_REFRESH_UNAVAILABLE). Without this test the back-compat
+        // path is unexercised and a regression that always required
+        // `setBundle` would silently break consumers.
+        const setSpy = vi.fn(async (_account: Account, _token: string) => {})
+        const legacyStore: TokenStore<Account> = {
+            async active() {
+                return null
+            },
+            set: setSpy,
+            // No setBundle — that's the whole point.
+            async clear() {},
+            async list() {
+                return []
+            },
+            async setDefault() {},
+        }
+        const { provider, getRedirect } = instrument({
+            exchangeCode: async () => ({
+                accessToken: 'tok-legacy',
+                refreshToken: 'rt-1',
+                expiresAt: Date.now() + 3_600_000,
+            }),
+        })
+
+        await runOAuthFlow<Account>(
+            flowOptions({
+                provider,
+                store: legacyStore,
+                openBrowser: driveCallback(getRedirect),
+            }),
+        )
+
+        // The legacy `set` path receives just the access token — refresh +
+        // expiry get dropped (correct degraded behaviour for stores that
+        // can't persist them).
+        expect(setSpy).toHaveBeenCalledTimes(1)
+        expect(setSpy.mock.calls[0][1]).toBe('tok-legacy')
+    })
+
     it('skips validateToken when exchangeCode returns an account', async () => {
         const validateToken = vi.fn(async () => ({ id: 'WRONG', email: 'x@x' }))
         const { provider, getRedirect } = instrument({
@@ -249,11 +356,17 @@ describe('runOAuthFlow', () => {
         expect(openBrowser).not.toHaveBeenCalled()
     })
 
-    it('halts via AbortSignal: aborting before the callback rejects with AUTH_OAUTH_FAILED and skips store.set', async () => {
+    it('halts via AbortSignal: aborting before the callback rejects with AUTH_OAUTH_FAILED and skips persistence', async () => {
         const controller = new AbortController()
         const { provider, getRedirect } = instrument()
         const store = fakeStore()
+        // Spy on BOTH `set` and `setBundle` so a regression where
+        // `runOAuthFlow` persists via `setBundle` after abort (now the
+        // default path with refresh-token support) wouldn't slip through.
+        // Reading `store.last` is the strongest guard: it stays undefined
+        // iff neither method ran.
         const setSpy = vi.spyOn(store, 'set')
+        const setBundleSpy = vi.spyOn(store, 'setBundle')
 
         await expect(
             runOAuthFlow<Account>(
@@ -272,6 +385,8 @@ describe('runOAuthFlow', () => {
             ),
         ).rejects.toMatchObject({ code: 'AUTH_OAUTH_FAILED' })
         expect(setSpy).not.toHaveBeenCalled()
+        expect(setBundleSpy).not.toHaveBeenCalled()
+        expect(store.last).toBeUndefined()
     })
 
     it('always surfaces the authorize URL via onAuthorizeUrl, even when openBrowser succeeds', async () => {

src/auth/flow.ts

@@ -4,6 +4,7 @@ import { type IncomingMessage, type Server, type ServerResponse, createServer }
 import { promisify } from 'node:util'
 import { CliError, getErrorMessage } from '../errors.js'
 import { isStdoutTTY } from '../terminal.js'
+import { bundleFromExchange, persistBundle } from './persist.js'
 import { generateState } from './pkce.js'
 import type { AuthAccount, AuthProvider, TokenStore } from './types.js'
 
@@ -188,7 +189,13 @@ export async function runOAuthFlow<TAccount extends AuthAccount>(
         checkAborted()
 
         try {
-            await options.store.set(account, exchange.accessToken)
+            await persistBundle(options.store, account, bundleFromExchange(exchange), {
+                // Explicit-login path: pin this account as default when
+                // nothing is pinned yet so the first login on a fresh
+                // config auto-selects it. `refreshAccessToken` omits this
+                // flag so silent refresh never mutates selection.
+                promoteDefault: true,
+            })
         } catch (error) {
             if (error instanceof CliError) throw error
             throw new CliError(

src/auth/index.ts

@@ -1,6 +1,8 @@
 export type { AuthErrorCode } from './errors.js'
 export { runOAuthFlow } from './flow.js'
 export type { RunOAuthFlowOptions, RunOAuthFlowResult } from './flow.js'
+export { refreshAccessToken } from './refresh.js'
+export type { RefreshAccessTokenOptions } from './refresh.js'
 export { attachLoginCommand } from './login.js'
 export type { AttachLoginCommandOptions, AttachLoginContext } from './login.js'
 export { attachLogoutCommand } from './logout.js'
@@ -32,6 +34,8 @@ export type {
     ExchangeResult,
     PrepareInput,
     PrepareResult,
+    RefreshInput,
+    TokenBundle,
     TokenStore,
     ValidateInput,
 } from './types.js'

src/auth/keyring/internal.test.ts

@@ -0,0 +1,59 @@
+import { describe, expect, it, vi } from 'vitest'
+
+import { findById, trySetSecret } from './internal.js'
+import { type SecureStore, SecureStoreUnavailableError } from './secure-store.js'
+
+type Account = { id: string; label?: string; email: string }
+
+describe('findById', () => {
+    const accountA: Account = { id: 'a', email: 'a@x' }
+    const accountB: Account = { id: 'b', email: 'b@x' }
+
+    it('returns the matching record when present', () => {
+        const result = findById([{ account: accountA }, { account: accountB }], 'b')
+        expect(result?.account).toBe(accountB)
+    })
+
+    it('returns undefined when no record matches', () => {
+        const result = findById([{ account: accountA }], 'missing')
+        expect(result).toBeUndefined()
+    })
+
+    it('returns undefined for an empty list', () => {
+        expect(findById<Account>([], 'a')).toBeUndefined()
+    })
+})
+
+describe('trySetSecret', () => {
+    function fakeStore(setImpl: (secret: string) => Promise<void>): SecureStore {
+        return {
+            getSecret: vi.fn(async () => null),
+            setSecret: setImpl,
+            deleteSecret: vi.fn(async () => false),
+        }
+    }
+
+    it('returns true on a successful setSecret', async () => {
+        const store = fakeStore(async () => undefined)
+        expect(await trySetSecret(store, 'tok')).toBe(true)
+    })
+
+    it('returns false on SecureStoreUnavailableError (the documented offline downgrade)', async () => {
+        const store = fakeStore(async () => {
+            throw new SecureStoreUnavailableError('no dbus')
+        })
+        expect(await trySetSecret(store, 'tok')).toBe(false)
+    })
+
+    it('rethrows non-SecureStoreUnavailable errors (no silent downgrade for programming bugs)', async () => {
+        // Without this, an unexpected backend error would be
+        // indistinguishable from "keyring offline", and callers would
+        // silently fall through to the plaintext-fallback path on the
+        // wrong signal. The narrow catch is load-bearing.
+        const cause = new Error('something else went wrong')
+        const store = fakeStore(async () => {
+            throw cause
+        })
+        await expect(trySetSecret(store, 'tok')).rejects.toBe(cause)
+    })
+})

src/auth/keyring/internal.ts

@@ -0,0 +1,36 @@
+import type { AuthAccount } from '../types.js'
+import { type SecureStore, SecureStoreUnavailableError } from './secure-store.js'
+import type { UserRecord } from './types.js'
+
+/**
+ * Find a record by its account id. Trivial wrapper, but every caller used
+ * the same shape (`(await userRecords.list()).find((r) => r.account.id ===
+ * id)`) — extracting it keeps a future change to the lookup (e.g. case-
+ * insensitive ids, lazy indexes) to one spot.
+ */
+export function findById<TAccount extends AuthAccount>(
+    records: UserRecord<TAccount>[],
+    id: string,
+): UserRecord<TAccount> | undefined {
+    return records.find((r) => r.account.id === id)
+}
+
+/**
+ * Try a keyring `setSecret` and tolerate the documented offline failure.
+ * Returns `true` when the secret landed in the keyring, `false` when the
+ * caller should fall back to a plaintext record field. Anything other
+ * than `SecureStoreUnavailableError` is rethrown — programming errors and
+ * unexpected backend failures must not silently downgrade to "no keyring".
+ *
+ * Shared by `writeRecordWithKeyringFallback` and `migrateLegacyAuth`'s
+ * tryInsert path so the offline-tolerance policy lives in one place.
+ */
+export async function trySetSecret(store: SecureStore, secret: string): Promise<boolean> {
+    try {
+        await store.setSecret(secret)
+        return true
+    } catch (error) {
+        if (!(error instanceof SecureStoreUnavailableError)) throw error
+        return false
+    }
+}