diff --git a/packages/core/src/accounts.ts b/packages/core/src/accounts.ts index 4ab389e..23e16e8 100644 --- a/packages/core/src/accounts.ts +++ b/packages/core/src/accounts.ts @@ -82,6 +82,22 @@ export type AccountOperationError = { nextRetryAt?: number retryCount?: number tokenHash?: string + /** + * HTTP status of the underlying refresh/quota failure, when known. Lets + * consumers distinguish a permanently-dead token (400 invalid_grant → + * re-login) from a transient failure (429/5xx → recovers) without a delay + * heuristic. Absent on errors persisted before this field existed. + */ + status?: number + /** + * Explicit dead-token discriminator, set at construction. True ONLY when the + * refresh endpoint returned 400 invalid_grant (token is genuinely dead → + * re-login). False for transient failures AND for retry-exhausted/network + * errors that get a long backoff but are NOT dead — so they are not nagged + * for re-login. Absent on errors persisted before this field existed (those + * fall back to status / the 24h-delay heuristic). + */ + permanent?: boolean } export type AccountQuotaWindow = { @@ -365,6 +381,7 @@ function normalizeOperationError( if (!Number.isFinite(checkedAt)) return undefined const nextRetryAt = Number(value.nextRetryAt) const retryCount = Number(value.retryCount) + const status = Number(value.status) return { message: value.message, checkedAt, @@ -372,6 +389,13 @@ function normalizeOperationError( retryCount: Number.isFinite(retryCount) ? retryCount : undefined, tokenHash: typeof value.tokenHash === 'string' ? value.tokenHash : undefined, + // Preserve the dead-token discriminators across save/load. Without these, + // a retry-exhausted transient (permanent=false, 24h backoff) would lose its + // flag on reload and the 24h-delay heuristic would wrongly re-classify it + // permanent → false "needs re-login" nag. + status: Number.isFinite(status) ? status : undefined, + permanent: + typeof value.permanent === 'boolean' ? value.permanent : undefined, } } @@ -770,6 +794,17 @@ async function saveAccountStateUnlocked( delete next.accounts[id] } } + } else { + // Full save: drop any per-account state whose id is no longer present in + // storage.accounts. The scoped path above only prunes ids it was asked to + // save; on a removal the storage is saved with scope.accounts === true + // (ids === null), so without this branch the removed account's runtime + // state (quota/lastRefreshError/access/refresh/expires) would be orphaned + // in the state file and later merged onto a re-added same-id account. + const present = new Set(storage.accounts.map((account) => account.id)) + for (const id of Object.keys(next.accounts)) { + if (!present.has(id)) delete next.accounts[id] + } } } @@ -1253,13 +1288,63 @@ export function buildRefreshOperationError(input: { } else { delay = NON_TRANSIENT_REFRESH_RETRY_DELAY_MS } + const statusFromError = (input.error as { status?: unknown }).status + const status = + typeof statusFromError === 'number' && Number.isFinite(statusFromError) + ? statusFromError + : undefined + const message = formatErrorMessage(input.error) + // A token is permanently dead ONLY on 400 invalid_grant. The OAuth spec allows + // other 400s (invalid_client / invalid_request / unsupported_grant_type) that + // re-login does NOT fix — those, like a retry-exhausted / network / 429 / 5xx + // error, get a long backoff but must stay permanent=false so they are not + // falsely flagged "needs re-login". ClaudeOAuthRefreshError carries the raw + // OAuth body, and its message embeds it (`...: 400 — `), so check both. + const body = + typeof (input.error as { body?: unknown }).body === 'string' + ? (input.error as { body: string }).body + : '' + const isInvalidGrant = + body.includes('invalid_grant') || message.includes('invalid_grant') return { - message: formatErrorMessage(input.error), + message, checkedAt: input.now, nextRetryAt: input.now + delay, retryCount, tokenHash, + status, + permanent: status === 400 && isInvalidGrant, + } +} + +/** + * True when a refresh error means the token is permanently dead and the account + * needs a re-login (vs a transient failure that recovers). + * + * Precedence: + * 1. the explicit `permanent` flag (set at construction from 400 invalid_grant) + * — the authoritative signal; correctly classifies a retry-exhausted/network + * error (long backoff, but NOT dead) as non-permanent; + * 2. else the captured HTTP `status` — 400 (for errors built before `permanent` + * existed but after `status`); + * 3. else the legacy 24h-delay heuristic — back-compat ONLY for errors persisted + * before either field existed (e.g. an operator's already-dead token: no + * status, ~24h backoff). It still flags those until the next refresh restamps + * the error with the explicit field. + */ +export function isPermanentRefreshError( + error: AccountOperationError | undefined, +): boolean { + if (!error) return false + if (typeof error.permanent === 'boolean') return error.permanent + if (typeof error.status === 'number') return error.status === 400 + if (typeof error.nextRetryAt === 'number') { + return ( + error.nextRetryAt - error.checkedAt >= + NON_TRANSIENT_REFRESH_RETRY_DELAY_MS + ) } + return false } export function refreshBackoffActive( diff --git a/packages/core/src/commands/account.ts b/packages/core/src/commands/account.ts index 26f83ad..44abc19 100644 --- a/packages/core/src/commands/account.ts +++ b/packages/core/src/commands/account.ts @@ -17,7 +17,7 @@ export type AccountCommandAction = authHeader?: 'authorization-bearer' | 'x-api-key' } | { type: 'add-oauth-start' } - | { type: 'add-oauth-finish'; code: string } + | { type: 'add-oauth-finish'; code: string; label?: string } | { type: 'usage' } export function parseAccountCommandAction( @@ -89,8 +89,22 @@ export function parseAccountCommandAction( if (action === 'add-oauth-start') return { type: 'add-oauth-start' } - if (action === 'add-oauth-finish' && rest) - return { type: 'add-oauth-finish', code: rest } + if (action === 'add-oauth-finish' && rest) { + let remaining = rest + + // Parse --label flag (mirrors add-apikey). The OAuth code is opaque (may + // contain a #state segment) so the label is collected via the flag, never + // positionally. + let label: string | undefined + const labelMatch = remaining.match(/--label\s+(.+)/) + if (labelMatch) { + label = labelMatch[1]?.trim() || undefined + remaining = remaining.replace(labelMatch[0], '').trim() + } + + if (!remaining) return { type: 'usage' } + return { type: 'add-oauth-finish', code: remaining, label } + } return { type: 'usage' } } diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index ab9b641..98ce7f9 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -61,6 +61,7 @@ import { isFastModeSupportedModel, isKillswitchEnabled, isOAuthAccount, + isPermanentRefreshError, isValidApiBaseURL, KILLSWITCH_COMMAND_NAME, killswitchPassesPolicy, @@ -661,6 +662,18 @@ export const AnthropicAuthPlugin: Plugin = async (ctx) => { ? (quotaManager.getFallback(account.id, account.access)?.quota ?? null) : null, + // A fallback with a permanently-dead refresh token (400 invalid_grant) + // is dropped by getUsableFallbackAccounts and silently degrades to + // main — surface it as "needs re-login". Only flag truly-dead tokens + // whose backoff is still active, not transient (429/5xx) backoff. + needsReauth: + account.lastRefreshError != null && + refreshBackoffActive( + account.lastRefreshError, + account.refresh, + Date.now(), + ) && + isPermanentRefreshError(account.lastRefreshError), enabled: account.enabled !== false, })), activeId: options.activeId, @@ -1117,9 +1130,13 @@ export const AnthropicAuthPlugin: Plugin = async (ctx) => { } const now = Date.now() + // OAuth accounts have no natural key, so the id stays a UUID even when a + // label is given (label collisions must not collide ids). The label is + // optional — a blank one keeps the UUID-name fallback in the UI. const account: OAuthAccount = { id: randomUUID(), type: 'oauth' as const, + label: action.label || undefined, access: result.access, refresh: result.refresh, expires: result.expires, diff --git a/packages/opencode/src/sidebar-state.ts b/packages/opencode/src/sidebar-state.ts index 9be9975..c4530e9 100644 --- a/packages/opencode/src/sidebar-state.ts +++ b/packages/opencode/src/sidebar-state.ts @@ -14,6 +14,9 @@ export interface SidebarAccountState { label: string | undefined quota: AccountQuota | null enabled: boolean + // True when the account's refresh token is permanently dead (400 + // invalid_grant) and it needs a re-login — distinct from a transient backoff. + needsReauth: boolean } export interface SidebarState { @@ -88,6 +91,8 @@ export function normalizeSidebarState(raw: unknown): SidebarState { label: typeof entry.label === 'string' ? entry.label : undefined, quota: isRecord(entry.quota) ? (entry.quota as AccountQuota) : null, enabled: typeof entry.enabled === 'boolean' ? entry.enabled : false, + needsReauth: + typeof entry.needsReauth === 'boolean' ? entry.needsReauth : false, })) : [] diff --git a/packages/opencode/src/tests/account-command.test.ts b/packages/opencode/src/tests/account-command.test.ts index 814f6ab..8e4e951 100644 --- a/packages/opencode/src/tests/account-command.test.ts +++ b/packages/opencode/src/tests/account-command.test.ts @@ -129,6 +129,33 @@ describe('parseAccountCommandAction', () => { test('garbage returns usage', () => { expect(parseAccountCommandAction('garbage')).toEqual({ type: 'usage' }) }) + + test('add-oauth-finish with code only (no label)', () => { + expect(parseAccountCommandAction('add-oauth-finish abc123')).toEqual({ + type: 'add-oauth-finish', + code: 'abc123', + }) + }) + + test('add-oauth-finish with --label', () => { + expect( + parseAccountCommandAction('add-oauth-finish abc123 --label work'), + ).toEqual({ + type: 'add-oauth-finish', + code: 'abc123', + label: 'work', + }) + }) + + test('add-oauth-finish --label with multi-word label', () => { + expect( + parseAccountCommandAction('add-oauth-finish abc123 --label my work acct'), + ).toEqual({ + type: 'add-oauth-finish', + code: 'abc123', + label: 'my work acct', + }) + }) }) // --------------------------------------------------------------------------- diff --git a/packages/opencode/src/tests/accounts.test.ts b/packages/opencode/src/tests/accounts.test.ts index 2389a0f..aca923a 100644 --- a/packages/opencode/src/tests/accounts.test.ts +++ b/packages/opencode/src/tests/accounts.test.ts @@ -27,6 +27,7 @@ import { isCacheKeepSubagentsEnabled, isCostZeroingEnabled, isFastModePersistentlyEnabled, + isPermanentRefreshError, type KillswitchThresholds, killswitchPassesPolicy, loadAccounts, @@ -1841,6 +1842,235 @@ describe('buildRefreshOperationError', () => { }) expect(result.nextRetryAt).toBe(1000000 + 5 * 60_000) }) + + test('captures the error status on the operation error', () => { + const error = new ClaudeOAuthRefreshError(400, 'invalid_grant') + const result = buildRefreshOperationError({ + error, + now: 1000000, + refreshToken: 'test-token', + }) + expect(result.status).toBe(400) + }) + + test('captures duck-typed status when no ClaudeOAuthRefreshError', () => { + const result = buildRefreshOperationError({ + error: { status: 429 }, + now: 1000000, + refreshToken: 'test-token', + }) + expect(result.status).toBe(429) + }) + + test('sets permanent true only for 400 invalid_grant', () => { + const dead = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(400, 'invalid_grant'), + now: 1000000, + refreshToken: 't', + }) + expect(dead.permanent).toBe(true) + }) + + test('sets permanent false for retry-exhausted transient (no status)', () => { + const exhausted = buildRefreshOperationError({ + error: new Error('Token refresh exhausted all retries'), + now: 1000000, + refreshToken: 't', + }) + expect(exhausted.permanent).toBe(false) + }) + + test('sets permanent false for a 429', () => { + const rateLimited = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(429, 'rate limited'), + now: 1000000, + refreshToken: 't', + }) + expect(rateLimited.permanent).toBe(false) + }) + + test('sets permanent false for a 400 that is NOT invalid_grant', () => { + // The OAuth spec allows 400 invalid_client / invalid_request / + // unsupported_grant_type — none of which re-login fixes. Only invalid_grant + // means the refresh token itself is dead. + const invalidClient = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(400, '{"error":"invalid_client"}'), + now: 1000000, + refreshToken: 't', + }) + expect(invalidClient.status).toBe(400) + expect(invalidClient.permanent).toBe(false) + // Explicit false must short-circuit isPermanentRefreshError before the + // legacy status===400 fallback can wrongly flag it. + expect(isPermanentRefreshError(invalidClient)).toBe(false) + }) + + test('sets permanent true for a 400 invalid_grant from the body JSON', () => { + const dead = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError( + 400, + '{"error":"invalid_grant","error_description":"Refresh token expired"}', + ), + now: 1000000, + refreshToken: 't', + }) + expect(dead.permanent).toBe(true) + expect(isPermanentRefreshError(dead)).toBe(true) + }) +}) + +describe('isPermanentRefreshError', () => { + test('400 (invalid_grant) → permanent (dead token, needs re-login)', () => { + const error = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(400, 'invalid_grant'), + now: 1000000, + refreshToken: 't', + }) + expect(isPermanentRefreshError(error)).toBe(true) + }) + + test('429 (rate limited) → NOT permanent (transient, recovers)', () => { + const error = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(429, 'rate limited'), + now: 1000000, + refreshToken: 't', + }) + expect(isPermanentRefreshError(error)).toBe(false) + }) + + test('500 (server error) → NOT permanent (transient)', () => { + const error = buildRefreshOperationError({ + error: { status: 500 }, + now: 1000000, + refreshToken: 't', + }) + expect(isPermanentRefreshError(error)).toBe(false) + }) + + test('legacy error without status but 24h delay → permanent (heuristic)', () => { + // Mirrors a dead token persisted before the status field existed: a + // non-transient error gets NON_TRANSIENT_REFRESH_RETRY_DELAY_MS (24h). + const checkedAt = 1000000 + const error = { + message: 'invalid_grant', + checkedAt, + nextRetryAt: checkedAt + 24 * 60 * 60_000, + } + expect(isPermanentRefreshError(error)).toBe(true) + }) + + test('legacy error without status, short delay → NOT permanent', () => { + const checkedAt = 1000000 + const error = { + message: 'rate limited', + checkedAt, + nextRetryAt: checkedAt + 5 * 60_000, + } + expect(isPermanentRefreshError(error)).toBe(false) + }) + + test('retry-exhausted transient error → NOT permanent (no false re-login nag)', () => { + // A network glitch that exhausts all retries throws a plain Error with NO + // status. isTransientRefreshError classifies it non-transient (no status, + // message is not "fetch failed"), so it gets the 24h backoff delay — but it + // is NOT a dead token. Built through buildRefreshOperationError, the explicit + // `permanent` flag must be false so it is not nagged for re-login. + const error = buildRefreshOperationError({ + error: new Error('Token refresh exhausted all retries'), + now: 1000000, + refreshToken: 't', + }) + // Sanity: it really did get the 24h non-transient delay (so the legacy + // heuristic alone would have wrongly flagged it permanent). + expect(error.nextRetryAt).toBe(1000000 + 24 * 60 * 60_000) + expect(isPermanentRefreshError(error)).toBe(false) + }) +}) + +describe('isPermanentRefreshError across save/load round-trip', () => { + test('retry-exhausted transient survives round-trip as NON-permanent', async () => { + // The classification only matters once persisted. A retry-exhausted error + // is permanent=false in memory but gets a 24h NON_TRANSIENT backoff — if the + // status/permanent fields are dropped on load, the 24h-delay heuristic wrongly + // re-classifies it permanent and the sidebar falsely nags re-login. + const error = buildRefreshOperationError({ + error: new Error('Token refresh exhausted all retries'), + now: Date.now(), + refreshToken: 'rt-exhausted', + }) + expect(error.permanent).toBe(false) + expect(isPermanentRefreshError(error)).toBe(false) + + const storage = baseStorage() + storage.accounts.push({ + id: 'exhausted', + type: 'oauth', + refresh: 'rt-exhausted', + lastRefreshError: error, + }) + await saveAccounts(storage, accountPath) + + const loaded = await loadAccounts(accountPath) + const reloaded = expectOAuthAccount( + loaded!.accounts.find((a) => a.id === 'exhausted'), + ) + expect(reloaded.lastRefreshError?.permanent).toBe(false) + expect(isPermanentRefreshError(reloaded.lastRefreshError)).toBe(false) + }) + + test('400 invalid_grant survives round-trip as permanent', async () => { + const error = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(400, 'invalid_grant'), + now: Date.now(), + refreshToken: 'rt-dead', + }) + expect(error.permanent).toBe(true) + + const storage = baseStorage() + storage.accounts.push({ + id: 'dead', + type: 'oauth', + refresh: 'rt-dead', + lastRefreshError: error, + }) + await saveAccounts(storage, accountPath) + + const loaded = await loadAccounts(accountPath) + const reloaded = expectOAuthAccount( + loaded!.accounts.find((a) => a.id === 'dead'), + ) + expect(reloaded.lastRefreshError?.status).toBe(400) + expect(reloaded.lastRefreshError?.permanent).toBe(true) + expect(isPermanentRefreshError(reloaded.lastRefreshError)).toBe(true) + }) + + test('400 non-invalid_grant survives round-trip as NON-permanent', async () => { + // A 400 invalid_client (misconfig) must NOT nag re-login — and the explicit + // permanent=false must survive load so the status===400 fallback never fires. + const error = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(400, '{"error":"invalid_client"}'), + now: Date.now(), + refreshToken: 'rt-misconfig', + }) + expect(error.permanent).toBe(false) + + const storage = baseStorage() + storage.accounts.push({ + id: 'misconfig', + type: 'oauth', + refresh: 'rt-misconfig', + lastRefreshError: error, + }) + await saveAccounts(storage, accountPath) + + const loaded = await loadAccounts(accountPath) + const reloaded = expectOAuthAccount( + loaded!.accounts.find((a) => a.id === 'misconfig'), + ) + expect(reloaded.lastRefreshError?.status).toBe(400) + expect(reloaded.lastRefreshError?.permanent).toBe(false) + expect(isPermanentRefreshError(reloaded.lastRefreshError)).toBe(false) + }) }) // --------------------------------------------------------------------------- @@ -2660,6 +2890,59 @@ describe('removeAccountPersistent', () => { false, ) }) + + test('prunes the orphaned per-account state on removal (full-save path)', async () => { + const storage = baseStorage() + storage.accounts.push({ + id: 'doomed', + type: 'oauth', + access: 'access-token', + refresh: 'refresh-token', + expires: Date.now() + 3600_000, + lastRefreshError: { + message: 'boom', + checkedAt: Date.now(), + }, + }) + await saveAccounts(storage, accountPath) + + // Sanity: the state file holds the per-account block before removal. + const statePath = getAccountStatePath(accountPath) + const before = JSON.parse(await readFile(statePath, 'utf8')) + expect(before.accounts?.doomed).toBeDefined() + + expect(await removeAccountPersistent('doomed', accountPath)).toBe(true) + + const after = JSON.parse(await readFile(statePath, 'utf8')) + expect(after.accounts?.doomed).toBeUndefined() + }) + + test('removing one fallback leaves the other fallback state intact', async () => { + const storage = baseStorage() + storage.accounts.push({ + id: 'keep', + type: 'oauth', + access: 'keep-access', + refresh: 'keep-refresh', + expires: Date.now() + 3600_000, + }) + storage.accounts.push({ + id: 'drop', + type: 'oauth', + access: 'drop-access', + refresh: 'drop-refresh', + expires: Date.now() + 3600_000, + }) + await saveAccounts(storage, accountPath) + + expect(await removeAccountPersistent('drop', accountPath)).toBe(true) + + const statePath = getAccountStatePath(accountPath) + const after = JSON.parse(await readFile(statePath, 'utf8')) + expect(after.accounts?.drop).toBeUndefined() + expect(after.accounts?.keep).toBeDefined() + expect(after.accounts?.keep?.refresh).toBe('keep-refresh') + }) }) describe('reorderAccountsPersistent', () => { diff --git a/packages/opencode/src/tests/add-account-flows.test.ts b/packages/opencode/src/tests/add-account-flows.test.ts index cd3a059..aa097ca 100644 --- a/packages/opencode/src/tests/add-account-flows.test.ts +++ b/packages/opencode/src/tests/add-account-flows.test.ts @@ -428,6 +428,102 @@ describe('add-oauth error paths', () => { }) }) +// --------------------------------------------------------------------------- +// add-oauth — label threading (the fix: prompt for a label, default UUID name) +// --------------------------------------------------------------------------- +describe('add-oauth label threading', () => { + async function getPluginWithClient() { + const client = createMockClient() + const { AnthropicAuthPlugin } = await import('../index') + const plugin = (await AnthropicAuthPlugin({ + // @ts-expect-error: minimal mock for testing + client, + })) as any + return { plugin, client } + } + + function extractStateFromPromptCalls( + client: ReturnType, + ): string { + const calls = (client.session.promptAsync as { mock: { calls: any[] } }) + .mock.calls + for (const [req] of calls) { + const text: string | undefined = req?.body?.parts?.[0]?.text + if (!text) continue + const match = text.match(/state=([^\s&]+)/) + if (match?.[1]) return decodeURIComponent(match[1]) + } + throw new Error('no OAuth URL with state captured from prompt calls') + } + + async function runOAuthAddWithLabel(labelArg: string | null) { + const { plugin, client } = await getPluginWithClient() + const sessionId = 'ses_oauth_label' + + // Real add-oauth-start: builds a URL with a real state, stores pending, + // and delivers the URL text via session.promptAsync (TUI not connected). + await executeCommand(plugin, 'claude-account', 'add-oauth-start', sessionId) + const state = extractStateFromPromptCalls(client) + + // Mock the token exchange endpoint to succeed. + const originalFetch = globalThis.fetch + globalThis.fetch = mock((input: any) => { + const url = + typeof input === 'string' + ? input + : input instanceof URL + ? input.toString() + : input.url + if (url.includes('oauth/token') || url.includes('/token')) { + return Promise.resolve( + new Response( + JSON.stringify({ + access_token: 'oauth-access', + refresh_token: 'oauth-refresh', + expires_in: 3600, + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ), + ) + } + return Promise.resolve(new Response(null, { status: 200 })) + }) as unknown as typeof fetch + + try { + const finishArgs = + labelArg == null + ? `add-oauth-finish dummy-code#${state}` + : `add-oauth-finish dummy-code#${state} --label ${labelArg}` + await executeCommand(plugin, 'claude-account', finishArgs, sessionId) + } finally { + globalThis.fetch = originalFetch + } + + const { loadAccounts } = await import('@cortexkit/anthropic-auth-core') + const loaded = await loadAccounts(accountPath) + return loaded + } + + test('add-oauth-finish --label sets the account label', async () => { + const loaded = await runOAuthAddWithLabel('work') + expect(loaded).not.toBeNull() + expect(loaded!.accounts).toHaveLength(1) + const account = loaded!.accounts[0]! + expect(account.type).toBe('oauth') + expect(account.label).toBe('work') + }) + + test('add-oauth-finish without --label leaves label undefined (UUID-name fallback)', async () => { + const loaded = await runOAuthAddWithLabel(null) + expect(loaded).not.toBeNull() + expect(loaded!.accounts).toHaveLength(1) + const account = loaded!.accounts[0]! + expect(account.label).toBeUndefined() + // id is a UUID (no natural key for OAuth) + expect(account.id).toMatch(/^[0-9a-f-]{36}$/) + }) +}) + // --------------------------------------------------------------------------- // security — no secrets in logs // --------------------------------------------------------------------------- diff --git a/packages/opencode/src/tests/index.test.ts b/packages/opencode/src/tests/index.test.ts index 2362b97..a496989 100644 --- a/packages/opencode/src/tests/index.test.ts +++ b/packages/opencode/src/tests/index.test.ts @@ -4,6 +4,8 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { type AccountStorage, + buildRefreshOperationError, + ClaudeOAuthRefreshError, getAccountStatePath, hashRefreshToken, loadAccounts, @@ -182,6 +184,73 @@ async function getPlugin(client?: ReturnType) { })) as Promise } +describe('sidebar needsReauth (dead-fallback indicator)', () => { + function fallbackWithRefreshError(status: number) { + const refresh = 'fallback-refresh' + const now = Date.now() + // A genuinely-dead token returns 400 invalid_grant; only that classifies as + // permanent (a bare 400 / other OAuth errors do not). + const body = status === 400 ? '{"error":"invalid_grant"}' : 'boom' + const error = buildRefreshOperationError({ + error: new ClaudeOAuthRefreshError(status, body), + now, + refreshToken: refresh, + }) + return createFallbackStorage({ + accounts: [ + { + id: 'fallback-1', + type: 'oauth', + access: 'fallback-access', + refresh, + expires: now + 5 * 60 * 60 * 1000, + lastRefreshError: error, + }, + ], + }) + } + + test('dead (400 invalid_grant) fallback → needsReauth true', async () => { + await useTempAccountFile(fallbackWithRefreshError(400)) + const plugin = await getPlugin() + await plugin.auth.loader( + () => + Promise.resolve({ + type: 'oauth', + access: 'main-access', + refresh: 'main-refresh', + expires: Date.now() + 100000, + }), + { models: {} }, + ) + await drainSidebarWrites() + const state = await waitForSidebarState( + (candidate) => candidate.fallbacks[0]?.id === 'fallback-1', + ) + expect(state.fallbacks[0]?.needsReauth).toBe(true) + }) + + test('transient (429 rate-limited) fallback → needsReauth false', async () => { + await useTempAccountFile(fallbackWithRefreshError(429)) + const plugin = await getPlugin() + await plugin.auth.loader( + () => + Promise.resolve({ + type: 'oauth', + access: 'main-access', + refresh: 'main-refresh', + expires: Date.now() + 100000, + }), + { models: {} }, + ) + await drainSidebarWrites() + const state = await waitForSidebarState( + (candidate) => candidate.fallbacks[0]?.id === 'fallback-1', + ) + expect(state.fallbacks[0]?.needsReauth).toBe(false) + }) +}) + describe('package metadata', () => { test('exports a runtime-loadable TUI entrypoint', async () => { const packageJson = JSON.parse( diff --git a/packages/opencode/src/tests/sidebar-state.test.ts b/packages/opencode/src/tests/sidebar-state.test.ts index 86cc10c..f591c5f 100644 --- a/packages/opencode/src/tests/sidebar-state.test.ts +++ b/packages/opencode/src/tests/sidebar-state.test.ts @@ -37,7 +37,13 @@ describe('resolveActiveAccount', () => { const state = make({ activeId: 'fb1', fallbacks: [ - { id: 'fb1', label: 'work', quota: quota(40), enabled: true }, + { + id: 'fb1', + label: 'work', + quota: quota(40), + enabled: true, + needsReauth: false, + }, ], }) const active = resolveActiveAccount(state) @@ -50,7 +56,13 @@ describe('resolveActiveAccount', () => { const state = make({ activeId: 'fb1', fallbacks: [ - { id: 'fb1', label: undefined, quota: quota(5), enabled: true }, + { + id: 'fb1', + label: undefined, + quota: quota(5), + enabled: true, + needsReauth: false, + }, ], }) expect(resolveActiveAccount(state).name).toBe('fb1') @@ -61,7 +73,13 @@ describe('resolveActiveAccount', () => { activeId: 'fb1', main: { quota: quota(12) }, fallbacks: [ - { id: 'fb1', label: 'work', quota: quota(40), enabled: false }, + { + id: 'fb1', + label: 'work', + quota: quota(40), + enabled: false, + needsReauth: false, + }, ], }) const active = resolveActiveAccount(state) @@ -79,7 +97,13 @@ describe('resolveActiveAccount', () => { activeId: 'ghost', main: { quota: null }, fallbacks: [ - { id: 'fb1', label: 'work', quota: quota(40), enabled: true }, + { + id: 'fb1', + label: 'work', + quota: quota(40), + enabled: true, + needsReauth: false, + }, ], }) const active = resolveActiveAccount(state) @@ -382,6 +406,27 @@ describe('normalizeSidebarState', () => { expect(out.fallbacks[0]!.enabled).toBe(false) }) + test('fallbacks: needsReauth defaults to false when missing', () => { + const out = normalizeSidebarState({ + fallbacks: [{ id: 'a', label: 'a', enabled: true }], + }) + expect(out.fallbacks[0]!.needsReauth).toBe(false) + }) + + test('fallbacks: needsReauth is parsed when present', () => { + const out = normalizeSidebarState({ + fallbacks: [{ id: 'a', label: 'a', enabled: true, needsReauth: true }], + }) + expect(out.fallbacks[0]!.needsReauth).toBe(true) + }) + + test('fallbacks: needsReauth defaults to false when non-boolean', () => { + const out = normalizeSidebarState({ + fallbacks: [{ id: 'a', label: 'a', enabled: true, needsReauth: 'yes' }], + }) + expect(out.fallbacks[0]!.needsReauth).toBe(false) + }) + test('relay defaults to null when missing transport', () => { const out = normalizeSidebarState({ relay: { enabled: true } }) expect(out.relay).toBeNull() @@ -461,6 +506,7 @@ describe('normalizeSidebarState', () => { five_hour: { usedPercent: 40, remainingPercent: 60 }, }, enabled: true, + needsReauth: false, }, ], activeId: 'fb1', @@ -530,7 +576,15 @@ describe('getSidebarState malformed file round-trip', () => { quotaBackedOff: false, refreshBackedOff: true, }, - fallbacks: [{ id: 'fb1', label: 'work', quota: null, enabled: true }], + fallbacks: [ + { + id: 'fb1', + label: 'work', + quota: null, + enabled: true, + needsReauth: false, + }, + ], activeId: 'fb1', route: 'fallback', relay: { enabled: false, transport: 'sse' }, diff --git a/packages/opencode/src/tui.tsx b/packages/opencode/src/tui.tsx index f33b21d..ab41631 100644 --- a/packages/opencode/src/tui.tsx +++ b/packages/opencode/src/tui.tsx @@ -304,10 +304,13 @@ function AccountBlock(props: { quota: AccountQuota | null active: boolean pacingEnabled: boolean + needsReauth?: boolean marginTop?: number }) { - const statusWord = () => (props.active ? 'active' : 'idle') - const statusTone = (): Tone => (props.active ? 'ok' : 'muted') + const statusWord = () => + props.needsReauth ? 're-login' : props.active ? 'active' : 'idle' + const statusTone = (): Tone => + props.needsReauth ? 'err' : props.active ? 'ok' : 'muted' const pacingFor = ( window: | { usedPercent: number; remainingPercent: number; resetsAt?: string } @@ -402,6 +405,7 @@ function QuotaDialogContent(props: { quota={fb.quota} active={state().activeId === fb.id} pacingEnabled={prefs().sections.pacing} + needsReauth={fb.needsReauth} marginTop={1} /> )} @@ -572,7 +576,8 @@ function QuotaSidebar(props: { const quotaBackedOff = () => state().main?.quotaBackedOff === true const refreshBackedOff = () => state().main?.refreshBackedOff === true - const degraded = () => quotaBackedOff() || refreshBackedOff() + const needsReauth = () => enabledFallbacks().some((f) => f.needsReauth) + const degraded = () => quotaBackedOff() || refreshBackedOff() || needsReauth() const cacheKeep = () => state().cacheKeep const showCache = () => @@ -683,6 +688,7 @@ function QuotaSidebar(props: { quota={fb.quota} active={state().activeId === fb.id} pacingEnabled={prefs().sections.pacing} + needsReauth={fb.needsReauth} marginTop={1} /> )} diff --git a/packages/opencode/src/tui/command-dialogs.tsx b/packages/opencode/src/tui/command-dialogs.tsx index 804d0e3..b49defa 100644 --- a/packages/opencode/src/tui/command-dialogs.tsx +++ b/packages/opencode/src/tui/command-dialogs.tsx @@ -517,13 +517,13 @@ export function openCommandDialog( openOAuthUrlScreen(oauthUrl) return } - openOAuthCodePrompt() + openOAuthCodePrompt(oauthUrl) }} /> )) } - const openOAuthCodePrompt = () => { + const openOAuthCodePrompt = (oauthUrl: string) => { const DialogPrompt = api.ui.DialogPrompt api.ui.dialog.setSize('xlarge') api.ui.dialog.replace(() => ( @@ -543,15 +543,44 @@ export function openCommandDialog( buildL1() return } - void apply('claude-account', `add-oauth-finish ${trimmed}`).then( - (r) => { - api.ui.toast({ message: r.text }) - updateAccounts(r) - buildL1() - }, - ) + openOAuthLabelPrompt(trimmed, oauthUrl) + }} + // Step BACK to the sign-in URL screen, not L1: the OAuth session + // (PKCE verifier/state) is already minted. Returning to L1 would let a + // retry re-run add-oauth-start and re-mint it, invalidating the URL the + // user is mid-sign-in with. Same session → same URL is preserved. + onCancel={() => openOAuthUrlScreen(oauthUrl)} + /> + )) + } + + const openOAuthLabelPrompt = (code: string, oauthUrl: string) => { + const DialogPrompt = api.ui.DialogPrompt + api.ui.dialog.setSize('xlarge') + api.ui.dialog.replace(() => ( + ( + A short name for this account (optional). + )} + placeholder='e.g. work' + value='' + onConfirm={(value: string) => { + const label = value.trim() + const args = label + ? `add-oauth-finish ${code} --label ${label}` + : `add-oauth-finish ${code}` + void apply('claude-account', args).then((r) => { + api.ui.toast({ message: r.text }) + updateAccounts(r) + buildL1() + }) }} - onCancel={() => buildL1()} + // Step BACK to the code prompt, not L1: the user already obtained an + // auth code to reach this step. Returning to L1 would let a retry + // re-run add-oauth-start, minting a new PKCE verifier/state that + // invalidates the code they already have and forces a full re-auth. + onCancel={() => openOAuthCodePrompt(oauthUrl)} /> )) }