From 7b78e446490cd2a250a6417e6bdb41ac39059ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Thu, 8 Oct 2026 01:59:14 +0800 Subject: [PATCH 1/7] fix(core): fence stale account-pool saves during OAuth updates --- packages/core/src/account-manager.test.ts | 166 +++- packages/core/src/account-manager.ts | 79 +- packages/opencode-v2/src/plugin.ts | 271 +++++-- packages/opencode-v2/test/plugin.test.ts | 879 +++++++++++++++++++++- 4 files changed, 1339 insertions(+), 56 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index 3146ed90..777294f5 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'bun:test' +import { describe, expect, it, jest } from 'bun:test' import { AccountManager } from './account-manager.ts' import type { AccountStorageStore } from './account-storage.ts' import type { AccountStorageV4 } from './account-types.ts' @@ -439,6 +439,170 @@ describe('core AccountManager', () => { await manager.dispose() expect(memory.mergedSaves()).toBe(1) }) + + it('stops stale deferred saves before an external pool mutation', async () => { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: memory.store, + }) + manager.requestSaveToDisk() + const pendingFlush = manager.flushSaveToDisk().then( + () => 'resolved', + (error: unknown) => String(error), + ) + + await manager.stopSaving() + expect(await pendingFlush).toContain('persistence has stopped') + manager.requestSaveToDisk() + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await manager.dispose() + expect(memory.mergedSaves()).toBe(0) + }) + + it('waits for an in-flight save before stopping persistence', async () => { + jest.useFakeTimers() + try { + let saveCalls = 0 + let releaseSave!: () => void + let markSaveStarted!: () => void + const saveStarted = new Promise((resolve) => { + markSaveStarted = resolve + }) + const store: AccountStorageStore = { + load: async () => stored, + saveMerged: async (_path, next) => { + saveCalls++ + markSaveStarted() + await new Promise((resolve) => { + releaseSave = resolve + }) + return next + }, + mutate: async (_path, fn) => (await fn(stored)) ?? stored, + clear: async () => {}, + } + const manager = new AccountManager(undefined, stored, { store }) + manager.requestSaveToDisk() + await jest.advanceTimersByTime(1000) + await saveStarted + + let stopped = false + const stopping = manager.stopSaving().then(() => { + stopped = true + }) + await Promise.resolve() + expect(stopped).toBe(false) + + releaseSave() + await stopping + expect(stopped).toBe(true) + expect(saveCalls).toBe(1) + + manager.requestSaveToDisk() + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await jest.advanceTimersByTime(1000) + expect(saveCalls).toBe(1) + } finally { + jest.useRealTimers() + } + }) + + it('waits for direct saves and rejects both direct save APIs after retirement', async () => { + let release!: () => void + let started!: () => void + const saveStarted = new Promise((resolve) => { + started = resolve + }) + let calls = 0 + const memory = createStore(stored) + const store: AccountStorageStore = { + ...memory.store, + saveMerged: async (_path, snapshot) => { + calls++ + started() + await new Promise((resolve) => { + release = resolve + }) + return snapshot + }, + } + const manager = new AccountManager(undefined, stored, { store }) + const saving = manager.saveToDisk() + await saveStarted + let stopped = false + const firstStop = manager.stopSaving().then(() => { + stopped = true + }) + const secondStop = manager.stopSaving() + await Promise.resolve() + expect(stopped).toBe(false) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await expect(manager.saveToDiskReplace()).rejects.toThrow( + 'persistence has stopped', + ) + release() + await Promise.all([saving, firstStop, secondStop]) + expect(calls).toBe(1) + expect(memory.mutations()).toBe(0) + }) + + it('preserves non-fatal lock contention for ordinary flushes and rejects strict transition flushes', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + throw new Error('ELOCKED') + }, + }, + }) + manager.requestSaveToDisk() + const ordinary = manager.flushSaveToDisk() + const strict = manager.flushSaveToDisk({ strict: true }) + await jest.advanceTimersByTime(1000) + await expect(ordinary).resolves.toBeUndefined() + await expect(strict).rejects.toThrow('ELOCKED') + await expect(manager.flushSaveToDisk()).resolves.toBeUndefined() + await expect(manager.flushSaveToDisk({ strict: true })).rejects.toThrow( + 'ELOCKED', + ) + await manager.stopSaving() + } finally { + jest.useRealTimers() + } + }) + + it('inspects retained non-contention failures only for strict flushes', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + throw new Error('disk full') + }, + }, + }) + manager.requestSaveToDisk() + await jest.advanceTimersByTime(1000) + await expect(manager.flushSaveToDisk()).resolves.toBeUndefined() + await expect(manager.flushSaveToDisk({ strict: true })).rejects.toThrow( + 'disk full', + ) + await manager.stopSaving() + } finally { + jest.useRealTimers() + } + }) }) describe('AccountManager instance dependencies', () => { diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 9be86b47..343e121c 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -349,10 +349,14 @@ export class AccountManager { private savePending = false private saveTimeout: ReturnType | null = null private saveInFlight: Promise | null = null + private directSaves = new Set>() + private saveFailure: unknown = null private disposed = false + private persistenceStopped = false private savePromiseResolvers: Array<{ resolve: () => void reject: (err: unknown) => void + strict: boolean }> = [] private sessionStartTime: number @@ -1726,7 +1730,18 @@ export class AccountManager { } async saveToDisk(): Promise { - await this.store.saveMerged(this.storagePath, this.buildStorageSnapshot()) + if (this.persistenceStopped) + throw new Error('Account manager persistence has stopped') + const save = this.store.saveMerged( + this.storagePath, + this.buildStorageSnapshot(), + ) + this.directSaves.add(save) + try { + await save + } finally { + this.directSaves.delete(save) + } } /** @@ -1735,8 +1750,18 @@ export class AccountManager { * mergeAccountStorage re-reading it from disk. */ async saveToDiskReplace(): Promise { + if (this.persistenceStopped) + throw new Error('Account manager persistence has stopped') const snapshot = this.buildStorageSnapshot() - await this.store.mutate(this.storagePath, () => snapshot) + const save = this.store + .mutate(this.storagePath, () => snapshot) + .then(() => {}) + this.directSaves.add(save) + try { + await save + } finally { + this.directSaves.delete(save) + } } requestSaveToDisk(): void { @@ -1751,15 +1776,54 @@ export class AccountManager { }, 1000) } - async flushSaveToDisk(): Promise { + async flushSaveToDisk(options: { strict?: boolean } = {}): Promise { + if (this.persistenceStopped) { + throw new Error('Account manager persistence has stopped') + } if (!this.savePending) { await this.saveInFlight + if (this.saveFailure && options.strict) throw this.saveFailure return } return new Promise((resolve, reject) => { - this.savePromiseResolvers.push({ resolve, reject }) + this.savePromiseResolvers.push({ + resolve, + reject, + strict: options.strict === true, + }) }) } + + /** + * Retire a manager before a lock-held pool mutation. A stale manager must + * never merge a deferred snapshot back over the newly mutated pool. + * The caller should flush intended state first, then await this method + * before starting the mutation. + */ + async stopSaving(): Promise { + if (this.persistenceStopped) { + await this.saveInFlight + await Promise.all( + [...this.directSaves].map((save) => save.catch(() => {})), + ) + return + } + this.persistenceStopped = true + this.disposed = true + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + this.savePending = false + const resolvers = this.savePromiseResolvers + this.savePromiseResolvers = [] + for (const { reject } of resolvers) { + reject(new Error('Account manager persistence has stopped')) + } + await this.saveInFlight + await Promise.all([...this.directSaves].map((save) => save.catch(() => {}))) + } + async dispose(): Promise { if (this.disposed) return this.disposed = true @@ -1782,10 +1846,12 @@ export class AccountManager { try { await this.saveToDisk() + this.saveFailure = null for (const { resolve } of resolvers) { resolve() } } catch (error) { + this.saveFailure = error if (isStorageLockContention(error)) { this.onDiagnostic?.( 'Skipped account-state persist due to storage lock contention', @@ -1793,8 +1859,9 @@ export class AccountManager { error: String(error), }, ) - for (const { resolve } of resolvers) { - resolve() + for (const waiter of resolvers) { + if (waiter.strict) waiter.reject(error) + else waiter.resolve() } return } diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index f8da7d6f..743859f1 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -19,14 +19,17 @@ import { dirname, join } from 'node:path' import { AccountManager, + type AccountStorageV4, + type AgyRequestScope, AgyRequestSessionStore, - applyClaudeTransforms, ANTIGRAVITY_ENDPOINT_FALLBACKS, - CLAUDE_THINKING_MAX_OUTPUT_TOKENS, + type AntigravityTokenExchangeResult, + applyClaudeTransforms, authorizeAntigravity, buildAgyAgentRequestMetadata, - buildImageGenerationConfig, buildAntigravityHarnessUserAgent, + buildImageGenerationConfig, + CLAUDE_THINKING_MAX_OUTPUT_TOKENS, defaultAccountStorageStore, ensureProjectContext, exchangeAntigravity, @@ -35,21 +38,18 @@ import { getModelFamily, isImageGenerationModel, loadAccountStorage, + type ManagedAccount, mutateAccountStorage, normalizeGeminiTools, + type OAuthAuthDetails, orderAgyRequestPayloadInPlace, parseRateLimitReason, parseRefreshParts, refreshAntigravityToken, resolveModelForHeaderStyle, - sanitizeCrossModelPayloadInPlace, SKIP_THOUGHT_SIGNATURE, + sanitizeCrossModelPayloadInPlace, toGeminiSchema, - type AccountStorageV4, - type AgyRequestScope, - type AntigravityTokenExchangeResult, - type ManagedAccount, - type OAuthAuthDetails, } from '@cortexkit/antigravity-auth-core' import type { Credential, @@ -137,6 +137,7 @@ interface OpenCodeV2Dependencies { authorizeAntigravity: typeof authorizeAntigravity ensureProjectContext: typeof ensureProjectContext exchangeAntigravity: typeof exchangeAntigravity + loadAccountStorage: typeof loadAccountStorage mutateAccountStorage: typeof mutateAccountStorage refreshAntigravityToken: typeof refreshAntigravityToken waitForAntigravityCode: typeof waitForAntigravityCode @@ -607,6 +608,7 @@ export function createOpenCodeV2AntigravityPlugin( ensureProjectContext: overrides.ensureProjectContext ?? ensureProjectContext, exchangeAntigravity: overrides.exchangeAntigravity ?? exchangeAntigravity, + loadAccountStorage: overrides.loadAccountStorage ?? loadAccountStorage, mutateAccountStorage: overrides.mutateAccountStorage ?? mutateAccountStorage, refreshAntigravityToken: @@ -627,41 +629,136 @@ export function createOpenCodeV2AntigravityPlugin( const activeControllers = new Set() const registrations: Registration[] = [] - let accounts = await loadAccountStorage(ACCOUNTS_FILE).catch((error) => { - log('accounts-load-error', errorMessage(error)) - return null - }) + let accounts = await dependencies + .loadAccountStorage(ACCOUNTS_FILE) + .catch((error) => { + log('accounts-load-error', errorMessage(error)) + return null + }) let manager = new AccountManager(undefined, accounts, { store: defaultAccountStorageStore, storagePath: ACCOUNTS_FILE, }) + let poolGeneration = 0 + let poolUnavailable = false + let transition: Promise | null = null + let finishTransition: (() => void) | null = null + let mutationTail: Promise = Promise.resolve() log('setup-start', 'accounts', manager.getTotalAccountCount()) - const reloadPool = async ( - options: { flushCurrent?: boolean } = {}, + const waitForPool = async (signal?: AbortSignal): Promise => { + if (signal?.aborted) throw signal.reason + while (transition) { + const pending = transition + if (!signal) await pending + else + await new Promise((resolve, reject) => { + const onAbort = (): void => { + signal.removeEventListener('abort', onAbort) + reject(signal.reason) + } + signal.addEventListener('abort', onAbort, { once: true }) + pending.then(() => { + signal.removeEventListener('abort', onAbort) + resolve() + }) + if (signal.aborted) onAbort() + }) + } + if (poolUnavailable) + throw new Error('Antigravity account pool is unavailable') + } + + const installAuthoritativePool = async (): Promise => { + accounts = await dependencies.loadAccountStorage(ACCOUNTS_FILE) + manager = new AccountManager(undefined, accounts, { + store: defaultAccountStorageStore, + storagePath: ACCOUNTS_FILE, + }) + poolGeneration += 1 + poolUnavailable = false + } + + const upsertPool = async ( + result: OAuthSuccess, + now: number, ): Promise => { - const previous = manager - if (options.flushCurrent !== false) await previous.flushSaveToDisk() + const predecessor = mutationTail + let release!: () => void + mutationTail = new Promise((resolve) => { + release = resolve + }) + await predecessor try { - accounts = await loadAccountStorage(ACCOUNTS_FILE) - manager = new AccountManager(undefined, accounts, { - store: defaultAccountStorageStore, - storagePath: ACCOUNTS_FILE, - }) - await previous.dispose().catch((error) => { - log('previous-pool-dispose-error', errorMessage(error)) + if (poolUnavailable) { + try { + await installAuthoritativePool() + } catch (error) { + log('pool-recovery-read-error', errorMessage(error)) + throw new Error('Antigravity account pool is unavailable', { + cause: error, + }) + } + } + await waitForPool() + transition = new Promise((resolve) => { + finishTransition = resolve }) - log('pool-reloaded', manager.getTotalAccountCount()) - } catch (error) { - log('reload-pool-error', errorMessage(error)) - throw error + poolGeneration += 1 + const previous = manager + try { + await previous.flushSaveToDisk({ strict: true }) + await previous.stopSaving() + await dependencies.mutateAccountStorage(ACCOUNTS_FILE, (current) => + upsertOAuthAccount(current, result, now), + ) + await installAuthoritativePool() + const refreshToken = parseRefreshParts(result.refresh).refreshToken + const saved = accounts?.accounts.find((account) => + result.email + ? account.email === result.email + : account.refreshToken === refreshToken, + ) + if ( + !saved || + saved.refreshToken !== refreshToken || + saved.enabled === false + ) + throw new Error( + 'Antigravity OAuth account was not confirmed on disk', + ) + log('pool-reloaded', manager.getTotalAccountCount()) + } catch (error) { + await previous.stopSaving().catch((stopError) => { + log('pool-saver-stop-error', errorMessage(stopError)) + }) + try { + await installAuthoritativePool() + } catch (readError) { + poolUnavailable = true + log('pool-readback-error', errorMessage(readError)) + throw new Error('Antigravity account pool is unavailable', { + cause: error, + }) + } + throw error + } finally { + finishTransition?.() + finishTransition = null + transition = null + } + } finally { + release() } } async function accessFor( account: ManagedAccount, force = false, - ): Promise { + generation = poolGeneration, + ): Promise { + if (generation !== poolGeneration || transition || poolUnavailable) + return null if ( !force && account.access && @@ -673,6 +770,8 @@ export function createOpenCodeV2AntigravityPlugin( const refreshed = await dependencies.refreshAntigravityToken( account.parts.refreshToken, ) + if (generation !== poolGeneration || transition || poolUnavailable) + return null const auth: OAuthAuthDetails = { type: 'oauth', refresh: formatRefreshParts({ @@ -721,14 +820,55 @@ export function createOpenCodeV2AntigravityPlugin( job: PendingJob, signal?: AbortSignal, ): Promise<{ response: Response; account: ManagedAccount }> { + await waitForPool(signal) const family = familyFor(job.modelID) const requested = job.resolved.actualModel const identity = { id: job.sessionID ?? 'default', parentId: null } - const excluded = new Set() - const poolSize = Math.max(1, manager.getEnabledAccounts().length) + let excluded = new Set() + let observedGeneration = poolGeneration let failure: unknown = null + let dispatched = false + let dispatchedError: unknown = null + + const generationChanged = (generation: number): boolean => { + if (generation === poolGeneration) return false + if (dispatched) + throw ( + dispatchedError ?? + new Error( + 'Antigravity request changed account pool after dispatch', + ) + ) + return true + } - for (let attempt = 0; attempt < poolSize + 2; attempt += 1) { + const waitForSelection = async ( + generation: number, + ): Promise => { + try { + await waitForPool(signal) + } catch (error) { + if (signal?.aborted) throw error + generationChanged(generation) + throw error + } + return generationChanged(generation) + } + + for ( + let attempt = 0; + attempt < Math.max(1, manager.getEnabledAccounts().length) + 2; + attempt += 1 + ) { + await waitForSelection(observedGeneration) + if (observedGeneration !== poolGeneration) { + generationChanged(observedGeneration) + observedGeneration = poolGeneration + excluded = new Set() + attempt = 0 + } + const selectedGeneration = poolGeneration + const selectedManager = manager const account = manager.getCurrentOrNextForFamily( family, requested, @@ -743,24 +883,31 @@ export function createOpenCodeV2AntigravityPlugin( if (!account) break let auth: OAuthAuthDetails + let accessed: OAuthAuthDetails | null try { - auth = await accessFor(account) + accessed = await accessFor(account, false, selectedGeneration) } catch (error) { + generationChanged(selectedGeneration) log('token-error', `#${account.index}`, errorMessage(error)) excluded.add(account.index) failure = error continue } + if (await waitForSelection(selectedGeneration)) continue + if (!accessed) continue + auth = accessed let context: Awaited> try { context = await dependencies.ensureProjectContext(auth) } catch (error) { + generationChanged(selectedGeneration) log('project-error', `#${account.index}`, errorMessage(error)) excluded.add(account.index) failure = error continue } + if (await waitForSelection(selectedGeneration)) continue const sessionKey = requestSessionKey(job) const scope = requestSessions.beginRequest(sessionKey) @@ -795,16 +942,20 @@ export function createOpenCodeV2AntigravityPlugin( ) { const endpoint = ANTIGRAVITY_ENDPOINT_FALLBACKS[endpointIndex] if (!endpoint) continue + if (await waitForSelection(selectedGeneration)) break let response: Response try { + dispatched = true response = await send(envelope, auth, endpoint, signal, job.kind) } catch (error) { + dispatchedError = error log( 'transport-error', `#${account.index}`, endpoint, errorMessage(error), ) + generationChanged(selectedGeneration) failure = error continue } @@ -817,14 +968,26 @@ export function createOpenCodeV2AntigravityPlugin( ) if (response.ok) { - manager.markRequestSuccess(account) - manager.markAccountUsed(account.index) - manager.recordRequest(account.index, family) - manager.requestSaveToDisk() + if ( + selectedGeneration === poolGeneration && + selectedManager === manager + ) { + manager.markRequestSuccess(account) + manager.markAccountUsed(account.index) + manager.recordRequest(account.index, family) + manager.requestSaveToDisk() + } return { response, account } } + dispatchedError = new Error(`Antigravity HTTP ${response.status}`) + generationChanged(selectedGeneration) + const { reason, message } = await readErrorDetails(response) + dispatchedError = new Error( + `Antigravity HTTP ${response.status}${reason ? ` (${reason})` : ''}`, + ) + generationChanged(selectedGeneration) log('upstream-error', response.status, reason ?? '', message) if ( response.status === 404 && @@ -835,11 +998,19 @@ export function createOpenCodeV2AntigravityPlugin( if (response.status === 401) { if (!forcedRefresh) { try { - auth = await accessFor(account, true) + const refreshed = await accessFor( + account, + true, + selectedGeneration, + ) + generationChanged(selectedGeneration) + if (!refreshed) break + auth = refreshed forcedRefresh = true endpointIndex -= 1 continue } catch (error) { + generationChanged(selectedGeneration) failure = error } } @@ -849,7 +1020,13 @@ export function createOpenCodeV2AntigravityPlugin( if (response.status === 403 && reason === 'ACCOUNT_INELIGIBLE') { manager.markAccountIneligible(account.index, reason) - await manager.flushSaveToDisk() + try { + await manager.flushSaveToDisk() + } catch (error) { + generationChanged(selectedGeneration) + throw error + } + generationChanged(selectedGeneration) excluded.add(account.index) failure = new Error('Antigravity account is ineligible') break @@ -857,7 +1034,13 @@ export function createOpenCodeV2AntigravityPlugin( if (response.status === 403 && reason === 'VALIDATION_REQUIRED') { manager.markAccountVerificationRequired(account.index, reason) - await manager.flushSaveToDisk() + try { + await manager.flushSaveToDisk() + } catch (error) { + generationChanged(selectedGeneration) + throw error + } + generationChanged(selectedGeneration) excluded.add(account.index) failure = new Error('Antigravity account requires validation') break @@ -908,6 +1091,7 @@ export function createOpenCodeV2AntigravityPlugin( excluded.add(account.index) break } + if (generationChanged(selectedGeneration)) continue // Transport failures may exhaust every endpoint without producing an // HTTP response. Move to another account instead of selecting the same // account again in the outer loop. @@ -1323,12 +1507,7 @@ export function createOpenCodeV2AntigravityPlugin( 'Antigravity token exchange returned no refresh token', ) } - await manager.flushSaveToDisk() - await dependencies.mutateAccountStorage( - ACCOUNTS_FILE, - (current) => upsertOAuthAccount(current, result, now), - ) - await reloadPool({ flushCurrent: false }) + await upsertPool(result, now) log('account-added', 'pool', manager.getTotalAccountCount()) const credential: Credential.OAuth = { type: 'oauth', diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index 926cb56c..c90ca07d 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -1,7 +1,12 @@ -import { describe, expect, test } from 'bun:test' -import { mkdirSync, writeFileSync } from 'node:fs' +import { describe, expect, spyOn, test } from 'bun:test' +import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' - +import { + type AccountStorageV4, + defaultAccountStorageStore, + loadAccountStorage, + mutateAccountStorage, +} from '@cortexkit/antigravity-auth-core' import type { SessionHttpRequest } from '@opencode-ai/plugin/promise/session' import plugin, { @@ -25,7 +30,875 @@ function successResponse(): Response { ) } +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +function seedPool(accounts: AccountStorageV4['accounts']): string { + const dir = process.env.OPENCODE_CONFIG_DIR + if (!dir) throw new Error('OPENCODE_CONFIG_DIR is not set') + mkdirSync(dir, { recursive: true }) + const path = join(dir, 'antigravity-accounts.json') + writeFileSync( + path, + `${JSON.stringify({ version: 4, accounts, activeIndex: 0 })}\n`, + ) + return path +} + +function readPool(path: string): AccountStorageV4 { + return JSON.parse(readFileSync(path, 'utf8')) as AccountStorageV4 +} + +async function setupPoolAdapter( + overrides: Parameters[0], +) { + type OAuthMethod = { + authorize: () => Promise<{ callback: Promise }> + } + let oauth: OAuthMethod | undefined + let hook: ((event: SessionHttpRequest) => Promise) | undefined + const registration = { dispose: async () => {} } + const adapter = createOpenCodeV2AntigravityPlugin(overrides) + const cleanup = await adapter.setup({ + session: { + hook: async (name: string, callback: unknown) => { + if (name === 'http.request') hook = callback as typeof hook + return registration + }, + }, + integration: { + transform: async (transform: unknown) => { + ;( + transform as (draft: { + method: { update: (method: OAuthMethod) => void } + }) => void + )({ + method: { + update: (method) => { + oauth = method + }, + }, + }) + return registration + }, + }, + } as never) + if (!oauth || !hook || !cleanup) throw new Error('plugin setup incomplete') + return { oauth, hook, cleanup } +} + +async function titleRequest( + hook: (event: SessionHttpRequest) => Promise, + signal?: AbortSignal, +): Promise { + const event = { + sessionID: 'pool-test-session', + agent: 'pool-test-agent', + model: { providerID: 'google', id: 'gemini-3.5-flash-lite' }, + kind: 'title', + request: new Request( + 'https://generativelanguage.googleapis.com/v1beta/models/gemini-3.5-flash-lite:streamGenerateContent?alt=sse', + { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + contents: [{ role: 'user', parts: [{ text: 'title' }] }], + }), + }, + ), + } + await hook(event as never) + return fetch(event.request, { signal }) +} + describe('opencode-v2-antigravity-auth plugin entry', () => { + test('serializes same-email OAuth saves and keeps the other enabled account', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const firstMutation = deferred() + const enteredMutation = deferred() + let writes = 0 + let exchanges = 0 + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => { + exchanges++ + return { + type: 'success', + refresh: `new-a-${exchanges}`, + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + } + }, + mutateAccountStorage: async (file, mutator) => { + writes++ + if (writes === 1) { + enteredMutation.resolve() + await firstMutation.promise + } + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return successResponse() + }, + }) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + const first = (await captured.oauth.authorize()).callback + await enteredMutation.promise + const cancelled = new AbortController() + const waitingRequest = titleRequest(captured.hook, cancelled.signal) + await Bun.sleep(20) + cancelled.abort() + await expect(waitingRequest).rejects.toThrow() + expect(sent).toEqual(['old-a']) + const second = (await captured.oauth.authorize()).callback + firstMutation.resolve() + await Promise.all([first, second]) + await Bun.sleep(1100) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a-2', 'token-b']) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + expect(writes).toBe(2) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.slice(1)).not.toContain('old-a') + } finally { + firstMutation.resolve() + await captured.cleanup() + } + }) + + test('recovers the live pool from disk after a failed OAuth write', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: false, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async () => { + throw new Error('disk write failed') + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return successResponse() + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'disk write failed', + ) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['old-a', 'token-b']) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['token-b']) + } finally { + await captured.cleanup() + } + }) + + test('strict OAuth flush stops before mutation when queued state hits storage contention', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let mutations = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + mutations++ + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async () => successResponse(), + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockRejectedValue(new Error('ELOCKED')) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'ELOCKED', + ) + expect(mutations).toBe(0) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['old-a', 'token-b']) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + } finally { + save.mockRestore() + await captured.cleanup() + } + }) + + test('reconciles a write-then-error without replaying OAuth mutation', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let writes = 0 + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + writes++ + await mutateAccountStorage(file, mutator) + throw new Error('write result lost') + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return successResponse() + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'write result lost', + ) + expect(writes).toBe(1) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).not.toContain('old-a') + } finally { + await captured.cleanup() + } + }) + + test('does not report OAuth success when storage returns without the new account', async () => { + const path = seedPool([ + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file) => (await loadAccountStorage(file))!, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'not confirmed on disk', + ) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['token-b']) + } finally { + await captured.cleanup() + } + }) + + test('reload failure reads back the durable OAuth write before serving requests', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let loads = 0 + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + loadAccountStorage: async (file) => { + loads++ + if (loads === 2) throw new Error('reload failed') + return loadAccountStorage(file) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return successResponse() + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'reload failed', + ) + expect(loads).toBe(3) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).not.toContain('old-a') + } finally { + await captured.cleanup() + } + }) + + test('discards a refresh result from a retired pool before sending', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const refreshStarted = deferred() + const releaseRefresh = deferred() + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => { + if (refresh === 'old-a') { + refreshStarted.resolve() + await releaseRefresh.promise + } + return { refresh, access: refresh, expires: Date.now() + 600_000 } + }, + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return successResponse() + }, + }) + try { + const pendingRequest = titleRequest(captured.hook) + await refreshStarted.promise + await (await captured.oauth.authorize()).callback + releaseRefresh.resolve() + expect((await pendingRequest).ok).toBe(true) + expect(sent).not.toContain('old-a') + } finally { + releaseRefresh.resolve() + await captured.cleanup() + } + }) + + for (const outcome of ['http-error', 'transport-error'] as const) { + test(`does not replay an already sent request after ${outcome} crosses OAuth transition`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const sent = deferred() + const upstream = deferred() + const accesses: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + accesses.push(auth.access ?? '') + sent.resolve() + return upstream.promise + }, + }) + try { + const pendingRequest = titleRequest(captured.hook) + await sent.promise + expect(accesses).toEqual(['old-a']) + await (await captured.oauth.authorize()).callback + if (outcome === 'http-error') + upstream.resolve( + new Response(JSON.stringify({ error: { message: 'limited' } }), { + status: 429, + }), + ) + else upstream.reject(new Error('transport failed')) + const response = await pendingRequest + expect(response.status).toBe(502) + expect(await response.text()).toContain( + outcome === 'http-error' + ? 'Antigravity HTTP 429' + : 'transport failed', + ) + expect(accesses).toEqual(['old-a']) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + } finally { + upstream.resolve(successResponse()) + await captured.cleanup() + } + }) + } + + test('does not retry a sent 401 after forced refresh crosses OAuth transition', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const forcedStarted = deferred() + const releaseForced = deferred() + let refreshes = 0 + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => { + refreshes++ + if (refresh === 'old-a' && refreshes === 2) { + forcedStarted.resolve() + await releaseForced.promise + } + return { refresh, access: refresh, expires: Date.now() + 600_000 } + }, + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return new Response(JSON.stringify({ error: { message: 'expired' } }), { + status: 401, + }) + }, + }) + try { + const pendingRequest = titleRequest(captured.hook) + await forcedStarted.promise + await (await captured.oauth.authorize()).callback + releaseForced.resolve() + const response = await pendingRequest + expect(response.status).toBe(502) + expect(await response.text()).toContain('Antigravity HTTP 401') + expect(sent).toEqual(['old-a']) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + } finally { + releaseForced.resolve() + await captured.cleanup() + } + }) + + for (const status of [404, 503]) { + test(`does not send a fallback endpoint after a sent ${status} crosses OAuth transition`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const sent = deferred() + const releaseBody = deferred() + const accesses: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + accesses.push(auth.access ?? '') + const response = new Response( + new ReadableStream({ + async start(controller) { + await releaseBody.promise + controller.enqueue( + new TextEncoder().encode( + JSON.stringify({ + error: { message: 'upstream unavailable' }, + }), + ), + ) + controller.close() + }, + }), + { status }, + ) + sent.resolve() + return response + }, + }) + try { + const pendingRequest = titleRequest(captured.hook) + await sent.promise + await (await captured.oauth.authorize()).callback + releaseBody.resolve() + const response = await pendingRequest + expect(response.status).toBe(502) + expect(await response.text()).toContain(`Antigravity HTTP ${status}`) + expect(accesses).toEqual(['old-a']) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + } finally { + releaseBody.resolve() + await captured.cleanup() + } + }) + } + + test('reports pool unavailable when OAuth write and authoritative readback both fail', async () => { + const path = seedPool([ + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let loads = 0 + let writes = 0 + let readable = false + let sends = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + loadAccountStorage: async (file) => { + loads++ + if (loads > 1 && !readable) throw new Error('readback failed') + return loadAccountStorage(file) + }, + mutateAccountStorage: async (file, mutator) => { + writes++ + if (writes === 1) throw new Error('disk write failed') + return mutateAccountStorage(file, mutator) + }, + send: async () => { + sends++ + return successResponse() + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'pool is unavailable', + ) + const response = await titleRequest(captured.hook) + expect(response.status).toBe(502) + expect(await response.text()).toContain('pool is unavailable') + expect(sends).toBe(0) + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'pool is unavailable', + ) + expect(writes).toBe(1) + readable = true + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + expect(writes).toBe(2) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['token-b', 'new-a']) + } finally { + await captured.cleanup() + } + }) test('exports the plugin contract shape', () => { expect(plugin).toBeTypeOf('object') expect(plugin.id).toBe('cortexkit.antigravity-auth') From 8365a8b107997092acc18d66d01dfc10b9190e4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Thu, 8 Oct 2026 15:40:34 +0800 Subject: [PATCH 2/7] bugfix: preserve account state across pool transitions and save failures --- packages/core/src/account-manager.test.ts | 807 ++++++++++++++++++- packages/core/src/account-manager.ts | 383 ++++++++- packages/core/src/index.ts | 6 +- packages/opencode-v2/src/plugin.ts | 136 +++- packages/opencode-v2/test/plugin.test.ts | 900 +++++++++++++++++++++- 5 files changed, 2153 insertions(+), 79 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index 777294f5..707d0c46 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it, jest } from 'bun:test' -import { AccountManager } from './account-manager.ts' +import { + AccountManager, + AccountManagerPersistenceError, +} from './account-manager.ts' import type { AccountStorageStore } from './account-storage.ts' +import { mergeAccountStorage } from './account-storage.ts' import type { AccountStorageV4 } from './account-types.ts' function createStore(initial: AccountStorageV4 | null = null) { @@ -440,6 +444,807 @@ describe('core AccountManager', () => { expect(memory.mergedSaves()).toBe(1) }) + it('retires a clean manager without rewriting external pool metadata', async () => { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: memory.store, + }) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external edit' } : account, + ), + })) + await manager.flushAndStopSaving() + expect(memory.mergedSaves()).toBe(0) + expect(memory.state()?.accounts[0]?.label).toBe('external edit') + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + }) + + it('keeps a lock-failed deferred save retryable on the same manager', async () => { + const memory = createStore(stored) + let locked = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + if (locked) throw new Error('ELOCKED') + return memory.store.saveMerged(path, next) + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toBeInstanceOf( + AccountManagerPersistenceError, + ) + locked = false + await manager.flushAndStopSaving() + expect(memory.mutations()).toBe(1) + expect(memory.state()?.accounts).toHaveLength(2) + }) + + it('lets an ordinary save continue after a retryable fence', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let locked = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + if (locked) throw new Error('ELOCKED') + return memory.store.saveMerged(path, next) + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'retryable', + }) + locked = false + manager.requestSaveToDisk() + const ordinary = manager.flushSaveToDisk({ strict: true }) + await jest.advanceTimersByTime(1000) + await expect(ordinary).resolves.toBeUndefined() + await manager.flushAndStopSaving() + expect(memory.state()?.accounts).toHaveLength(2) + } finally { + jest.useRealTimers() + } + }) + + it('confirms a post-write error by read-back without replaying it', async () => { + const memory = createStore(stored) + let calls = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + calls++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + }, + }, + }) + manager.requestSaveToDisk() + await manager.flushAndStopSaving() + expect(calls).toBe(1) + }) + + it('holds an ambiguous write fenced until read-back can confirm it', async () => { + const memory = createStore(stored) + let readable = false + let loadCalls = 0 + let calls = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + loadCalls++ + if (loadCalls === 1) return memory.store.load(path) + if (!readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + calls++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + readable = true + await manager.flushAndStopSaving() + expect(calls).toBe(1) + }) + + it('confirms the original nested snapshot and then saves late intent', async () => { + const memory = createStore(stored) + let releaseRead!: () => void + let readStarted!: () => void + const reading = new Promise((resolve) => { + readStarted = resolve + }) + let loadCalls = 0 + let saves = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + loadCalls++ + if (loadCalls === 2) { + readStarted() + await new Promise((resolve) => { + releaseRead = resolve + }) + } + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + saves++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + if (saves === 1) throw new Error('response lost') + return next + }, + }, + }) + manager.requestSaveToDisk() + const fence = manager.flushAndStopSaving() + await reading + manager.getAccounts()[0]!.rateLimitResetTimes.claude = 999 + manager.requestSaveToDisk() + releaseRead() + await fence + expect(saves).toBe(2) + expect(memory.state()?.accounts[0]?.rateLimitResetTimes?.claude).toBe(999) + }) + + it('does not require a readable baseline for an ordinary successful save', async () => { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async () => { + throw new Error('read unavailable') + }, + }, + }) + manager.requestSaveToDisk() + await manager.flushAndStopSaving() + expect(memory.mergedSaves()).toBe(1) + }) + + it('does not infer a failed write from read-back without its baseline', async () => { + const memory = createStore(stored) + let loadCalls = 0 + let saves = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + loadCalls++ + if (loadCalls === 1) throw new Error('baseline unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + saves++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(saves).toBe(1) + }) + + it('closes direct-save admission and drains a late requested save', async () => { + const memory = createStore(stored) + let release!: () => void + let started!: () => void + const firstStarted = new Promise((resolve) => { + started = resolve + }) + let calls = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + calls++ + if (calls === 1) { + started() + await new Promise((resolve) => { + release = resolve + }) + } + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + return next + }, + }, + }) + const direct = manager.saveToDisk() + await firstStarted + const fence = manager.flushAndStopSaving() + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + manager.getAccounts()[0]!.label = 'late' + manager.requestSaveToDisk() + release() + await Promise.all([direct, fence]) + expect(calls).toBe(2) + expect(memory.state()?.accounts[0]?.label).toBe('late') + }) + + it('keeps a failed replacement fenced when read-back differs from both states', async () => { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + mutate: async (path, fn) => { + await memory.store.mutate(path, fn) + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external' } : account, + ), + })) + throw new Error('response lost') + }, + }, + }) + const replacement = manager.saveToDiskReplace() + await expect(replacement).rejects.toThrow('response lost') + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts[0]?.label).toBe('external') + await expect(manager.saveToDiskReplace()).rejects.toThrow( + 'persistence has stopped', + ) + }) + + it('does not replay a failed replacement over a changed pool', async () => { + const memory = createStore(stored) + let locked = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + mutate: async (path, fn) => { + if (locked) throw new Error('ELOCKED') + return memory.store.mutate(path, fn) + }, + }, + }) + manager.removeAccountByIndex(0) + await expect(manager.saveToDiskReplace()).rejects.toThrow('ELOCKED') + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external' } : account, + ), + })) + locked = false + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['r1', 'r2']) + expect(memory.state()?.accounts[0]?.label).toBe('external') + }) + + it('retries the exact failed replacement and keeps the removal', async () => { + const memory = createStore(stored) + let locked = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + mutate: async (path, fn) => { + if (locked) throw new Error('ELOCKED') + return memory.store.mutate(path, fn) + }, + }, + }) + manager.removeAccountByIndex(0) + await expect(manager.saveToDiskReplace()).rejects.toThrow('ELOCKED') + locked = false + await manager.flushAndStopSaving() + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['r2']) + expect(memory.mergedSaves()).toBe(0) + }) + + it('rejects a failed merge retry after external token rotation', async () => { + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + throw new Error('ELOCKED') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'retryable', + }) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, refreshToken: 'rotated' } : account, + ), + })) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['rotated', 'r2']) + expect(memory.mutations()).toBe(2) + }) + + for (const kind of ['merge', 'replace'] as const) { + it(`retains a newer ${kind} intent when reconciliation fails again`, async () => { + const memory = createStore(stored) + let locked = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + if (locked) throw new Error('ELOCKED') + return memory.store.saveMerged(path, next) + }, + mutate: async (path, fn) => { + if (locked) throw new Error('ELOCKED') + return memory.store.mutate(path, fn) + }, + }, + }) + await expect(manager.saveToDisk()).rejects.toThrow('ELOCKED') + manager.getAccounts()[1]!.label = 'retained newer intent' + manager.getAccounts()[1]!.rateLimitResetTimes.claude = 999 + if (kind === 'replace') manager.removeAccountByIndex(0) + const newer = + kind === 'replace' ? manager.saveToDiskReplace() : manager.saveToDisk() + await expect(newer).rejects.toThrow('ELOCKED') + locked = false + await manager.flushAndStopSaving() + expect( + memory + .state() + ?.accounts.find((account) => account.refreshToken === 'r2'), + ).toMatchObject({ + label: 'retained newer intent', + rateLimitResetTimes: { claude: 999 }, + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(kind === 'replace' ? ['r2'] : ['r1', 'r2']) + }) + } + + it('keeps a deferred replacement from overwriting a concurrent edit after recovery', async () => { + const memory = createStore(stored) + let locked = true + let mutations = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + throw new Error('ELOCKED') + }, + mutate: async (path, fn) => { + if (locked) throw new Error('ELOCKED') + mutations++ + if (mutations === 2) { + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 + ? { ...account, refreshToken: 'externally-rotated' } + : account, + ), + })) + } + return memory.store.mutate(path, fn) + }, + }, + }) + await expect(manager.saveToDisk()).rejects.toThrow('ELOCKED') + manager.removeAccountByIndex(0) + await expect(manager.saveToDiskReplace()).rejects.toThrow('ELOCKED') + locked = false + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['externally-rotated', 'r2']) + expect(manager.getAccounts()[0]?.parts.refreshToken).toBe('r2') + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + }) + + it('serializes a queued new save with fence reconciliation of an older failure', async () => { + const memory = createStore(stored) + let retryCalls = 0 + let mergedCalls = 0 + let releaseRetry!: () => void + let retryStarted!: () => void + const retrying = new Promise((resolve) => { + retryStarted = resolve + }) + const gate = new Promise((resolve) => { + releaseRetry = resolve + }) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + mergedCalls++ + if (mergedCalls === 1) throw new Error('ELOCKED') + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + return next + }, + mutate: async (path, fn) => { + retryCalls++ + if (retryCalls === 1) { + retryStarted() + await gate + } + return memory.store.mutate(path, fn) + }, + }, + }) + await expect(manager.saveToDisk()).rejects.toThrow('ELOCKED') + manager.getAccounts()[0]!.label = 'new intent' + const second = manager.saveToDisk() + const fence = manager.flushAndStopSaving() + await retrying + await Promise.resolve() + await Promise.resolve() + const callsBeforeRelease = retryCalls + releaseRetry() + const results = await Promise.allSettled([second, fence]) + expect(callsBeforeRelease).toBe(1) + expect(results.map((result) => result.status)).toEqual([ + 'fulfilled', + 'fulfilled', + ]) + expect(memory.state()?.accounts[0]?.label).toBe('new intent') + }) + + for (const kind of ['merge', 'replace'] as const) { + it(`captures queued ${kind} save snapshots at invocation time`, async () => { + const initial: AccountStorageV4 = { + version: 4, + accounts: [ + ...stored.accounts, + { refreshToken: 'r3', projectId: 'p3', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + const memory = createStore(initial) + let releaseFirst!: () => void + let firstStarted!: () => void + const firstStartedPromise = new Promise((resolve) => { + firstStarted = resolve + }) + const firstGate = new Promise((resolve) => { + releaseFirst = resolve + }) + const writes: Array<{ operation: string; state: AccountStorageV4 }> = [] + const record = async ( + operation: string, + state: AccountStorageV4, + ): Promise => { + const attempt = writes.length + 1 + writes.push({ operation, state: structuredClone(state) }) + if (attempt === 1) { + firstStarted() + await firstGate + } + if (attempt === 2) throw new Error('ELOCKED') + } + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + await record('merge', next) + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + return next + }, + mutate: async (path, fn) => { + const current = memory.state() ?? { + version: 4 as const, + accounts: [], + activeIndex: 0, + } + const next = await fn(current) + if (!next) return current + await record('replace', next) + return memory.store.mutate(path, () => next) + }, + }, + }) + // Keep collection edits local to the explicit saves under observation. + manager.requestSaveToDisk = () => {} + + const first = manager.saveToDisk() + await firstStartedPromise + + if (kind === 'replace') manager.removeAccountByIndex(0) + const secondAccount = manager + .getAccounts() + .find((account) => account.parts.refreshToken === 'r3')! + secondAccount.label = 'second snapshot' + secondAccount.rateLimitResetTimes.claude = 333 + const second = + kind === 'replace' ? manager.saveToDiskReplace() : manager.saveToDisk() + const secondExpectedTokens = + kind === 'replace' ? ['r2', 'r3'] : ['r1', 'r2', 'r3'] + manager.removeAccountByIndex(0) + const thirdAccount = manager + .getAccounts() + .find((account) => account.parts.refreshToken === 'r3')! + thirdAccount.label = 'third snapshot' + thirdAccount.rateLimitResetTimes.claude = 777 + const third = + kind === 'replace' ? manager.saveToDiskReplace() : manager.saveToDisk() + const fence = manager.flushAndStopSaving() + + releaseFirst() + const results = await Promise.allSettled([first, second, third, fence]) + expect(results.map((result) => result.status)).toEqual([ + 'fulfilled', + 'rejected', + 'fulfilled', + 'fulfilled', + ]) + expect((results[1] as PromiseRejectedResult).reason).toMatchObject({ + message: 'ELOCKED', + }) + expect(writes.map((write) => write.operation)).toEqual( + kind === 'merge' + ? ['merge', 'merge', 'replace', 'replace'] + : ['merge', 'replace', 'replace', 'replace'], + ) + expect( + writes[1]!.state.accounts.map((account) => account.refreshToken), + ).toEqual(secondExpectedTokens) + expect( + writes[2]!.state.accounts.find( + (account) => account.refreshToken === 'r3', + )?.label, + ).toBe('second snapshot') + expect( + writes[3]!.state.accounts.map((account) => account.refreshToken), + ).toEqual(kind === 'replace' ? ['r3'] : ['r1', 'r2', 'r3']) + expect( + writes[3]!.state.accounts.find( + (account) => account.refreshToken === 'r3', + ), + ).toMatchObject({ + label: 'third snapshot', + rateLimitResetTimes: { claude: 777 }, + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(kind === 'replace' ? ['r3'] : ['r1', 'r2', 'r3']) + expect( + memory + .state() + ?.accounts.find((account) => account.refreshToken === 'r3'), + ).toMatchObject({ + label: 'third snapshot', + rateLimitResetTimes: { claude: 777 }, + }) + }) + } + + for (const kind of ['merge', 'replace'] as const) { + it(`fence retry keeps a failed ${kind} intent after later memory changes`, async () => { + const initial: AccountStorageV4 = { + version: 4, + accounts: [ + ...stored.accounts, + { refreshToken: 'r3', projectId: 'p3', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + const memory = createStore(initial) + let releaseFirst!: () => void + let firstStarted!: () => void + const firstStartedPromise = new Promise((resolve) => { + firstStarted = resolve + }) + const firstGate = new Promise((resolve) => { + releaseFirst = resolve + }) + const writes: Array<{ operation: string; state: AccountStorageV4 }> = [] + const record = async ( + operation: string, + state: AccountStorageV4, + ): Promise => { + const attempt = writes.length + 1 + writes.push({ operation, state: structuredClone(state) }) + if (attempt === 1) { + firstStarted() + await firstGate + } + if (attempt === 2) throw new Error('ELOCKED') + } + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + await record('merge', next) + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + return next + }, + mutate: async (path, fn) => { + const current = memory.state() ?? { + version: 4 as const, + accounts: [], + activeIndex: 0, + } + const next = await fn(current) + if (!next) return current + await record('replace', next) + return memory.store.mutate(path, () => next) + }, + }, + }) + manager.requestSaveToDisk = () => {} + + const first = manager.saveToDisk() + await firstStartedPromise + + if (kind === 'replace') manager.removeAccountByIndex(0) + const secondAccount = manager + .getAccounts() + .find((account) => account.parts.refreshToken === 'r3')! + secondAccount.label = 'second snapshot' + secondAccount.rateLimitResetTimes.claude = 333 + const second = + kind === 'replace' ? manager.saveToDiskReplace() : manager.saveToDisk() + const secondExpectedTokens = + kind === 'replace' ? ['r2', 'r3'] : ['r1', 'r2', 'r3'] + + // These edits happen after the second intent is frozen and before it + // reaches its failing write. + manager.removeAccountByIndex(0) + const changedAccount = manager + .getAccounts() + .find((account) => account.parts.refreshToken === 'r3')! + changedAccount.label = 'changed before failure' + changedAccount.rateLimitResetTimes.claude = 555 + + releaseFirst() + await first + await expect(second).rejects.toThrow('ELOCKED') + + // Mutate again after ELOCKED, without admitting another direct save. + changedAccount.label = 'changed after failure' + changedAccount.rateLimitResetTimes.claude = 999 + await manager.flushAndStopSaving() + + expect(writes.map((write) => write.operation)).toEqual( + kind === 'merge' + ? ['merge', 'merge', 'replace'] + : ['merge', 'replace', 'replace'], + ) + expect( + writes[1]!.state.accounts.map((account) => account.refreshToken), + ).toEqual(secondExpectedTokens) + expect( + writes[1]!.state.accounts.find( + (account) => account.refreshToken === 'r3', + ), + ).toMatchObject({ + label: 'second snapshot', + rateLimitResetTimes: { claude: 333 }, + }) + expect( + writes[2]!.state.accounts.map((account) => account.refreshToken), + ).toEqual(secondExpectedTokens) + expect( + writes[2]!.state.accounts.find( + (account) => account.refreshToken === 'r3', + ), + ).toMatchObject({ + label: 'second snapshot', + rateLimitResetTimes: { claude: 333 }, + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(kind === 'replace' ? ['r2', 'r3'] : ['r1', 'r2', 'r3']) + expect( + memory + .state() + ?.accounts.find((account) => account.refreshToken === 'r3'), + ).toMatchObject({ + label: 'second snapshot', + rateLimitResetTimes: { claude: 333 }, + }) + }) + } + + it('accepts a later successful save after an earlier admitted save fails', async () => { + const memory = createStore(stored) + let releaseFirst!: () => void + let firstStarted!: () => void + const started = new Promise((resolve) => { + firstStarted = resolve + }) + const gate = new Promise((resolve) => { + releaseFirst = resolve + }) + let saves = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + saves++ + if (saves === 1) { + firstStarted() + await gate + throw new Error('ELOCKED') + } + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + return next + }, + }, + }) + const first = manager.saveToDisk() + await started + manager.getAccounts()[0]!.label = 'later intent' + const second = manager.saveToDisk() + const fence = manager.flushAndStopSaving() + releaseFirst() + const results = await Promise.allSettled([first, second, fence]) + expect(results.map((result) => result.status)).toEqual([ + 'rejected', + 'fulfilled', + 'fulfilled', + ]) + expect(memory.state()?.accounts[0]?.label).toBe('later intent') + }) + it('stops stale deferred saves before an external pool mutation', async () => { const memory = createStore(stored) const manager = new AccountManager(undefined, stored, { diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 343e121c..77a286c3 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -1,6 +1,10 @@ import { createHash } from 'node:crypto' +import { isDeepStrictEqual } from 'node:util' import type { AccountStorageStore } from './account-storage.ts' -import { AccountStorageLockContentionError } from './account-storage.ts' +import { + AccountStorageLockContentionError, + mergeAccountStorage, +} from './account-storage.ts' import type { AccountMetadataV3, AccountSelectionStrategy, @@ -48,6 +52,28 @@ function isStorageLockContention(error: unknown): boolean { ) } +function sameStoredState( + left: AccountStorageV4 | null, + right: AccountStorageV4 | null, +): boolean { + // Storage writes JSON, which omits undefined optional fields. Compare + // durable representations while ignoring object key insertion order. + return isDeepStrictEqual( + JSON.parse(JSON.stringify(left)), + JSON.parse(JSON.stringify(right)), + ) +} + +export class AccountManagerPersistenceError extends Error { + constructor( + readonly state: 'retryable' | 'unconfirmed', + readonly originalError: unknown, + ) { + super(`Account manager persistence ${state}`) + this.name = 'AccountManagerPersistenceError' + } +} + export interface AccountManagerOptions { store: AccountStorageStore storagePath?: string @@ -353,6 +379,21 @@ export class AccountManager { private saveFailure: unknown = null private disposed = false private persistenceStopped = false + private storageAdmission: 'open' | 'fencing' | 'stopped' = 'open' + private fenceInFlight: Promise | null = null + private saveQueue: Promise = Promise.resolve() + private acceptedSaves = new Set>() + private acceptedSaveFailure: unknown = null + private pendingSaveIntents: Array<{ + kind: 'merge' | 'replace' + snapshot: AccountStorageV4 + }> = [] + private failedSaveAttempt: { + kind: 'merge' | 'replace' + baseline: AccountStorageV4 | null | undefined + snapshot: AccountStorageV4 + error: unknown + } | null = null private savePromiseResolvers: Array<{ resolve: () => void reject: (err: unknown) => void @@ -1730,18 +1771,8 @@ export class AccountManager { } async saveToDisk(): Promise { - if (this.persistenceStopped) - throw new Error('Account manager persistence has stopped') - const save = this.store.saveMerged( - this.storagePath, - this.buildStorageSnapshot(), - ) - this.directSaves.add(save) - try { - await save - } finally { - this.directSaves.delete(save) - } + this.assertSaveAdmission() + await this.enqueueSave('merge') } /** @@ -1750,25 +1781,206 @@ export class AccountManager { * mergeAccountStorage re-reading it from disk. */ async saveToDiskReplace(): Promise { - if (this.persistenceStopped) + this.assertSaveAdmission() + await this.enqueueSave('replace') + } + + private assertSaveAdmission(): void { + if (this.storageAdmission !== 'open' || this.persistenceStopped) { throw new Error('Account manager persistence has stopped') - const snapshot = this.buildStorageSnapshot() - const save = this.store - .mutate(this.storagePath, () => snapshot) - .then(() => {}) + } + } + + private enqueueSave(kind: 'merge' | 'replace'): Promise { + const intent = { + kind, + snapshot: structuredClone(this.buildStorageSnapshot()), + } + const save = this.saveQueue.then(async () => { + // Retain this intent before reconciling older work: reconciliation + // can fail without ever attempting this new snapshot. Capture happened + // at invocation time, while queue insertion stays in serial order. + this.pendingSaveIntents.push(intent) + await this.drainSaveIntents() + }) + this.acceptedSaves.add(save) this.directSaves.add(save) + void save.then( + () => { + this.acceptedSaveFailure = null + }, + (error) => { + this.acceptedSaveFailure = error + }, + ) + void save + .finally(() => { + this.acceptedSaves.delete(save) + this.directSaves.delete(save) + }) + .catch(() => {}) + this.saveQueue = save.then( + () => {}, + () => {}, + ) + return save + } + + private async drainSaveIntents( + recoveredBaseline?: AccountStorageV4, + ): Promise { + let guardedBaseline = recoveredBaseline + const failed = this.failedSaveAttempt + if (failed) { + const result = await this.readBackFailedSave() + if (result === 'missing') { + guardedBaseline = await this.retryFailedSave() + } else if (result === 'saved') { + guardedBaseline = + failed.kind === 'merge' + ? mergeAccountStorage( + failed.baseline ?? { version: 4, accounts: [], activeIndex: 0 }, + failed.snapshot, + ) + : failed.snapshot + } + } + while (this.pendingSaveIntents.length > 0) { + const intent = this.pendingSaveIntents[0]! + const { kind, snapshot } = intent + let baseline: AccountStorageV4 | null | undefined = guardedBaseline + if (baseline === undefined) { + // Preserve ordinary saves when baseline reads are unavailable. + try { + baseline = structuredClone(await this.store.load(this.storagePath)) + } catch (error) { + this.onDiagnostic?.('Could not capture account-state baseline', { + error: String(error), + }) + } + } + try { + let saved: AccountStorageV4 + if (guardedBaseline !== undefined) { + const expected = guardedBaseline + saved = await this.store.mutate(this.storagePath, (current) => { + if (!sameStoredState(current, expected)) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account state changed before a deferred save'), + ) + } + return kind === 'merge' + ? mergeAccountStorage(current, snapshot) + : snapshot + }) + } else if (kind === 'merge') { + saved = await this.store.saveMerged(this.storagePath, snapshot) + } else { + saved = await this.store.mutate(this.storagePath, (current) => { + baseline = structuredClone(current) + return snapshot + }) + } + this.pendingSaveIntents.shift() + // Further retained intents must not overwrite changes made between + // recovery and their lock-held write. + guardedBaseline = structuredClone(saved) + this.acceptedSaveFailure = null + } catch (error) { + this.pendingSaveIntents.shift() + this.failedSaveAttempt = { kind, baseline, snapshot, error } + throw error + } + } + } + + private async readBackFailedSave(): Promise<'none' | 'saved' | 'missing'> { + const attempt = this.failedSaveAttempt + if (!attempt) return 'none' + if (isStorageLockContention(attempt.error)) { + return 'missing' + } + let current: AccountStorageV4 | null try { - await save - } finally { - this.directSaves.delete(save) + current = await this.store.load(this.storagePath) + } catch (error) { + throw new AccountManagerPersistenceError('unconfirmed', error) + } + if (attempt.baseline === undefined) { + throw new AccountManagerPersistenceError('unconfirmed', attempt.error) + } + const expected = + attempt.kind === 'merge' + ? mergeAccountStorage( + attempt.baseline ?? { version: 4, accounts: [], activeIndex: 0 }, + attempt.snapshot, + ) + : attempt.snapshot + if (sameStoredState(current, expected)) { + this.failedSaveAttempt = null + this.acceptedSaveFailure = null + return 'saved' } + if (sameStoredState(current, attempt.baseline)) { + return 'missing' + } + throw new AccountManagerPersistenceError('unconfirmed', attempt.error) } - requestSaveToDisk(): void { - if (this.disposed || this.savePending) { - return + /** Retry only the exact failed intent under a lock-held state check. */ + private async retryFailedSave(): Promise { + const attempt = this.failedSaveAttempt + if (!attempt) return + if (attempt.baseline === undefined) { + throw new AccountManagerPersistenceError('unconfirmed', attempt.error) } - this.savePending = true + const baseline = attempt.baseline + try { + const saved = await this.store.mutate(this.storagePath, (current) => { + if ( + !sameStoredState( + current, + baseline ?? { version: 4, accounts: [], activeIndex: 0 }, + ) + ) { + throw new AccountManagerPersistenceError('unconfirmed', attempt.error) + } + return attempt.kind === 'merge' + ? mergeAccountStorage(current, attempt.snapshot) + : attempt.snapshot + }) + this.failedSaveAttempt = null + this.saveFailure = null + this.acceptedSaveFailure = null + return structuredClone(saved) + } catch (error) { + if (error instanceof AccountManagerPersistenceError) throw error + attempt.error = error + throw error + } + } + + private enqueueFailedSaveReconciliation( + recoveredBaseline?: AccountStorageV4, + ): Promise { + const reconcile = this.saveQueue.then(() => + this.drainSaveIntents(recoveredBaseline), + ) + this.saveQueue = reconcile.then( + () => {}, + () => {}, + ) + return reconcile + } + + private schedulePendingSave(): void { + if ( + !this.savePending || + this.saveTimeout || + this.storageAdmission !== 'open' + ) + return this.saveTimeout = setTimeout(() => { this.saveInFlight = this.executeSave().finally(() => { this.saveInFlight = null @@ -1776,10 +1988,19 @@ export class AccountManager { }, 1000) } + requestSaveToDisk(): void { + if (this.disposed || this.persistenceStopped) { + return + } + this.savePending = true + this.schedulePendingSave() + } + async flushSaveToDisk(options: { strict?: boolean } = {}): Promise { - if (this.persistenceStopped) { + if (this.storageAdmission !== 'open' || this.persistenceStopped) { throw new Error('Account manager persistence has stopped') } + this.schedulePendingSave() if (!this.savePending) { await this.saveInFlight if (this.saveFailure && options.strict) throw this.saveFailure @@ -1794,6 +2015,112 @@ export class AccountManager { }) } + /** Close admission now, then confirm accepted writes before retiring. */ + flushAndStopSaving(): Promise { + if (this.fenceInFlight) return this.fenceInFlight + if (this.storageAdmission === 'stopped' || this.persistenceStopped) { + return Promise.reject( + new Error('Account manager persistence has stopped'), + ) + } + this.storageAdmission = 'fencing' + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + const fence = this.finishFlushAndStopSaving() + this.fenceInFlight = fence + void fence + .finally(() => { + this.fenceInFlight = null + }) + .catch(() => {}) + return fence + } + + private async finishFlushAndStopSaving(): Promise { + try { + const hasAcceptedWork = + this.acceptedSaves.size > 0 || + this.saveInFlight !== null || + this.savePending + if ( + !hasAcceptedWork && + (this.failedSaveAttempt || this.pendingSaveIntents.length > 0) + ) { + await this.enqueueFailedSaveReconciliation() + } + while (true) { + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + if (this.savePending) { + this.saveInFlight = this.executeSave().finally(() => { + this.saveInFlight = null + }) + } + await this.saveQueue + await this.saveInFlight + const failed = this.failedSaveAttempt + const readBack = await this.readBackFailedSave() + if ( + readBack === 'saved' && + failed && + this.pendingSaveIntents.length > 0 + ) { + const recovered = + failed.kind === 'merge' + ? mergeAccountStorage( + failed.baseline ?? { + version: 4, + accounts: [], + activeIndex: 0, + }, + failed.snapshot, + ) + : failed.snapshot + await this.enqueueFailedSaveReconciliation(recovered) + continue + } + if (readBack === 'missing') { + throw new AccountManagerPersistenceError( + 'retryable', + this.failedSaveAttempt?.error ?? this.acceptedSaveFailure, + ) + } + if (readBack === 'none' && this.acceptedSaveFailure) { + if ( + this.acceptedSaveFailure instanceof AccountManagerPersistenceError + ) { + throw this.acceptedSaveFailure + } + throw new AccountManagerPersistenceError( + 'unconfirmed', + this.acceptedSaveFailure, + ) + } + if (this.savePending) continue + this.storageAdmission = 'stopped' + this.persistenceStopped = true + this.disposed = true + return + } + } catch (error) { + if (error instanceof AccountManagerPersistenceError) { + if (error.state === 'retryable') { + this.storageAdmission = 'open' + } + throw error + } + if (isStorageLockContention(error)) { + this.storageAdmission = 'open' + throw new AccountManagerPersistenceError('retryable', error) + } + throw new AccountManagerPersistenceError('unconfirmed', error) + } + } + /** * Retire a manager before a lock-held pool mutation. A stale manager must * never merge a deferred snapshot back over the newly mutated pool. @@ -1809,6 +2136,7 @@ export class AccountManager { return } this.persistenceStopped = true + this.storageAdmission = 'stopped' this.disposed = true if (this.saveTimeout) { clearTimeout(this.saveTimeout) @@ -1845,7 +2173,8 @@ export class AccountManager { this.savePromiseResolvers = [] try { - await this.saveToDisk() + if (this.storageAdmission === 'open') await this.saveToDisk() + else await this.enqueueSave('merge') this.saveFailure = null for (const { resolve } of resolvers) { resolve() diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 27e35f57..6a992046 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,7 +6,11 @@ export type { AccountSessionIdentity, ManagedAccount, } from './account-manager.ts' -export { AccountManager, resolveQuotaGroup } from './account-manager.ts' +export { + AccountManager, + AccountManagerPersistenceError, + resolveQuotaGroup, +} from './account-manager.ts' export * from './account-storage.ts' export * from './account-types.ts' export * from './agy-request-metadata.ts' diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index 743859f1..f87f0486 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -644,6 +644,7 @@ export function createOpenCodeV2AntigravityPlugin( let transition: Promise | null = null let finishTransition: (() => void) | null = null let mutationTail: Promise = Promise.resolve() + let shuttingDown = false log('setup-start', 'accounts', manager.getTotalAccountCount()) const waitForPool = async (signal?: AbortSignal): Promise => { @@ -669,20 +670,47 @@ export function createOpenCodeV2AntigravityPlugin( throw new Error('Antigravity account pool is unavailable') } - const installAuthoritativePool = async (): Promise => { - accounts = await dependencies.loadAccountStorage(ACCOUNTS_FILE) - manager = new AccountManager(undefined, accounts, { + const installPool = (stored: AccountStorageV4 | null): void => { + const replacement = new AccountManager(undefined, stored, { store: defaultAccountStorageStore, storagePath: ACCOUNTS_FILE, }) + accounts = stored + manager = replacement poolGeneration += 1 poolUnavailable = false } + const findConfirmedOAuthAccount = ( + stored: AccountStorageV4 | null, + result: OAuthSuccess, + ) => { + const refreshToken = parseRefreshParts(result.refresh).refreshToken + return stored?.accounts.find((account) => + result.email + ? account.email === result.email + : account.refreshToken === refreshToken, + ) + } + + const isConfirmedOAuthAccount = ( + stored: AccountStorageV4 | null, + result: OAuthSuccess, + ): boolean => { + const refreshToken = parseRefreshParts(result.refresh).refreshToken + const saved = findConfirmedOAuthAccount(stored, result) + return !!( + saved && + saved.refreshToken === refreshToken && + saved.enabled !== false + ) + } + const upsertPool = async ( result: OAuthSuccess, now: number, ): Promise => { + if (shuttingDown) throw new Error('Antigravity plugin is shutting down') const predecessor = mutationTail let release!: () => void mutationTail = new Promise((resolve) => { @@ -690,9 +718,11 @@ export function createOpenCodeV2AntigravityPlugin( }) await predecessor try { + if (shuttingDown) + throw new Error('Antigravity plugin is shutting down') if (poolUnavailable) { try { - await installAuthoritativePool() + installPool(await dependencies.loadAccountStorage(ACCOUNTS_FILE)) } catch (error) { log('pool-recovery-read-error', errorMessage(error)) throw new Error('Antigravity account pool is unavailable', { @@ -707,33 +737,29 @@ export function createOpenCodeV2AntigravityPlugin( poolGeneration += 1 const previous = manager try { - await previous.flushSaveToDisk({ strict: true }) - await previous.stopSaving() + await previous.flushAndStopSaving() + } catch (error) { + log('pool-save-fence-error', errorMessage(error)) + finishTransition?.() + finishTransition = null + transition = null + throw error + } + + try { await dependencies.mutateAccountStorage(ACCOUNTS_FILE, (current) => upsertOAuthAccount(current, result, now), ) - await installAuthoritativePool() - const refreshToken = parseRefreshParts(result.refresh).refreshToken - const saved = accounts?.accounts.find((account) => - result.email - ? account.email === result.email - : account.refreshToken === refreshToken, - ) - if ( - !saved || - saved.refreshToken !== refreshToken || - saved.enabled === false - ) + const stored = await dependencies.loadAccountStorage(ACCOUNTS_FILE) + if (!isConfirmedOAuthAccount(stored, result)) throw new Error( 'Antigravity OAuth account was not confirmed on disk', ) + installPool(stored) log('pool-reloaded', manager.getTotalAccountCount()) } catch (error) { - await previous.stopSaving().catch((stopError) => { - log('pool-saver-stop-error', errorMessage(stopError)) - }) try { - await installAuthoritativePool() + installPool(await dependencies.loadAccountStorage(ACCOUNTS_FILE)) } catch (readError) { poolUnavailable = true log('pool-readback-error', errorMessage(readError)) @@ -842,6 +868,24 @@ export function createOpenCodeV2AntigravityPlugin( return true } + const persistAccountState = async ( + selectedManager: AccountManager, + account: ManagedAccount, + generation: number, + ): Promise => { + generationChanged(generation) + try { + await selectedManager.flushSaveToDisk() + } catch (error) { + log( + 'account-state-save-error', + `#${account.index}`, + errorMessage(error), + ) + } + generationChanged(generation) + } + const waitForSelection = async ( generation: number, ): Promise => { @@ -1019,28 +1063,27 @@ export function createOpenCodeV2AntigravityPlugin( } if (response.status === 403 && reason === 'ACCOUNT_INELIGIBLE') { - manager.markAccountIneligible(account.index, reason) - try { - await manager.flushSaveToDisk() - } catch (error) { - generationChanged(selectedGeneration) - throw error - } - generationChanged(selectedGeneration) + selectedManager.markAccountIneligible(account.index, reason) + await persistAccountState( + selectedManager, + account, + selectedGeneration, + ) excluded.add(account.index) failure = new Error('Antigravity account is ineligible') break } if (response.status === 403 && reason === 'VALIDATION_REQUIRED') { - manager.markAccountVerificationRequired(account.index, reason) - try { - await manager.flushSaveToDisk() - } catch (error) { - generationChanged(selectedGeneration) - throw error - } - generationChanged(selectedGeneration) + selectedManager.markAccountVerificationRequired( + account.index, + reason, + ) + await persistAccountState( + selectedManager, + account, + selectedGeneration, + ) excluded.add(account.index) failure = new Error('Antigravity account requires validation') break @@ -1481,6 +1524,8 @@ export function createOpenCodeV2AntigravityPlugin( label: 'Google Antigravity (add account)', }, authorize: async () => { + if (shuttingDown) + throw new Error('Antigravity plugin is shutting down') const authorization = await dependencies.authorizeAntigravity() const state = new URL(authorization.url).searchParams.get('state') if (!state) { @@ -1558,6 +1603,7 @@ export function createOpenCodeV2AntigravityPlugin( ) return async () => { + shuttingDown = true for (const registration of registrations.reverse()) { await registration .dispose() @@ -1570,12 +1616,20 @@ export function createOpenCodeV2AntigravityPlugin( for (const timer of jobTimers.values()) clearTimeout(timer) jobTimers.clear() for (const controller of activeControllers) controller.abort() + await mutationTail server.closeAllConnections?.() await new Promise((resolve) => server.close(() => resolve())) requestSessions.clear() - await manager - .flushSaveToDisk() - .catch((error) => log('pool-flush-error', errorMessage(error))) + try { + await manager.flushAndStopSaving() + } catch (error) { + log('pool-flush-error', errorMessage(error)) + await manager + .stopSaving() + .catch((stopError) => + log('pool-saver-stop-error', errorMessage(stopError)), + ) + } await manager .dispose() .catch((error) => log('pool-dispose-error', errorMessage(error))) diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index c90ca07d..38150407 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -2,10 +2,13 @@ import { describe, expect, spyOn, test } from 'bun:test' import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { + AccountManagerPersistenceError, type AccountStorageV4, + buildAntigravityHarnessUserAgent, defaultAccountStorageStore, loadAccountStorage, mutateAccountStorage, + saveAccountStorage, } from '@cortexkit/antigravity-auth-core' import type { SessionHttpRequest } from '@opencode-ai/plugin/promise/session' @@ -209,6 +212,71 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + test('shutdown drains an accepted pool transition and rejects late OAuth writes', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + ]) + const enteredMutation = deferred() + const releaseMutation = deferred() + let writes = 0 + let exchanges = 0 + let accepted: Promise | undefined + let cleanup: Promise | undefined + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => { + exchanges++ + return { + type: 'success', + refresh: `new-a-${exchanges}`, + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + } + }, + mutateAccountStorage: async (file, mutator) => { + writes++ + enteredMutation.resolve() + await releaseMutation.promise + return mutateAccountStorage(file, mutator) + }, + }) + let cleanupFinished = false + try { + accepted = (await captured.oauth.authorize()).callback + await enteredMutation.promise + cleanup = Promise.resolve(captured.cleanup()).then(() => { + cleanupFinished = true + }) + await Bun.sleep(20) + expect(cleanupFinished).toBe(false) + releaseMutation.resolve() + await accepted + await cleanup + + await expect(captured.oauth.authorize()).rejects.toThrow() + expect(writes).toBe(1) + expect(exchanges).toBe(1) + } finally { + releaseMutation.resolve() + await accepted?.catch(() => {}) + await cleanup?.catch(() => {}) + if (!cleanupFinished) await captured.cleanup() + } + }) + test('recovers the live pool from disk after a failed OAuth write', async () => { const path = seedPool([ { @@ -329,9 +397,14 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { ).mockRejectedValue(new Error('ELOCKED')) try { expect((await titleRequest(captured.hook)).ok).toBe(true) - await expect((await captured.oauth.authorize()).callback).rejects.toThrow( - 'ELOCKED', + const callback = (await captured.oauth.authorize()).callback + await expect(callback).rejects.toBeInstanceOf( + AccountManagerPersistenceError, ) + await expect(callback).rejects.toMatchObject({ + state: 'retryable', + originalError: expect.objectContaining({ message: 'ELOCKED' }), + }) expect(mutations).toBe(0) expect( readPool(path).accounts.map((account) => account.refreshToken), @@ -343,6 +416,103 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + test('keeps dirty account state and the live pool after a retryable OAuth fence failure', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let mutations = 0 + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + mutations++ + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return sent.length === 1 + ? new Response(JSON.stringify({ error: { message: 'limited' } }), { + status: 429, + headers: { 'retry-after': '120' }, + }) + : successResponse() + }, + }) + const save = spyOn(defaultAccountStorageStore, 'saveMerged') + .mockRejectedValueOnce(new Error('ELOCKED')) + .mockImplementation((file, next) => + mutateAccountStorage(file, () => next), + ) + try { + const requestStartedAt = Date.now() + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a', 'token-b']) + const callback = (await captured.oauth.authorize()).callback + await expect(callback).rejects.toBeInstanceOf( + AccountManagerPersistenceError, + ) + await expect(callback).rejects.toMatchObject({ state: 'retryable' }) + expect(mutations).toBe(0) + expect(readPool(path).accounts[0]?.lastUsed).toBe(1) + + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.slice(2)).toEqual(['token-b']) + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + expect(mutations).toBe(1) + const persisted = readPool(path) + expect(persisted.accounts.map((account) => account.refreshToken)).toEqual( + ['new-a', 'token-b'], + ) + expect( + Object.values(persisted.accounts[0]?.rateLimitResetTimes ?? {}).some( + (resetAt) => + typeof resetAt === 'number' && resetAt > requestStartedAt, + ), + ).toBe(true) + } finally { + save.mockRestore() + await captured.cleanup() + } + }) + test('reconciles a write-then-error without replaying OAuth mutation', async () => { const path = seedPool([ { @@ -412,8 +582,15 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) - test('does not report OAuth success when storage returns without the new account', async () => { + test('keeps B available when account-state write and readback fail before OAuth mutation', async () => { const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, { email: 'b@example.test', refreshToken: 'token-b', @@ -422,6 +599,9 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { enabled: true, }, ]) + let loads = 0 + let mutations = 0 + const sent: string[] = [] const captured = await setupPoolAdapter({ authorizeAntigravity: async () => ({ url: 'https://accounts.example/authorize?state=pool-state', @@ -437,15 +617,717 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { email: 'a@example.test', projectId: 'project', }), - mutateAccountStorage: async (file) => (await loadAccountStorage(file))!, + loadAccountStorage: async (file) => { + loads++ + if (loads > 1) throw new Error('unexpected pool reload') + return loadAccountStorage(file) + }, + mutateAccountStorage: async (file, mutator) => { + mutations++ + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return sent.length === 1 + ? new Response(JSON.stringify({ error: { message: 'limited' } }), { + status: 429, + headers: { 'retry-after': '120' }, + }) + : successResponse() + }, + }) + const storeLoad = spyOn( + defaultAccountStorageStore, + 'load', + ).mockRejectedValue(new Error('account-state readback failed')) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + await mutateAccountStorage(file, () => next) + throw new Error('account-state write result lost') }) try { - await expect((await captured.oauth.authorize()).callback).rejects.toThrow( - 'not confirmed on disk', - ) + const requestStartedAt = Date.now() + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a', 'token-b']) + const callback = (await captured.oauth.authorize()).callback + await expect(callback).rejects.toMatchObject({ + state: 'unconfirmed', + originalError: expect.objectContaining({ + message: 'account-state readback failed', + }), + }) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.slice(2)).toEqual(['token-b']) + expect(mutations).toBe(0) + expect(loads).toBe(1) + expect(readPool(path).accounts[0]?.refreshToken).toBe('old-a') expect( - readPool(path).accounts.map((account) => account.refreshToken), - ).toEqual(['token-b']) + Object.values( + readPool(path).accounts[0]?.rateLimitResetTimes ?? {}, + ).some( + (resetAt) => + typeof resetAt === 'number' && resetAt > requestStartedAt, + ), + ).toBe(true) + } finally { + storeLoad.mockRestore() + save.mockRestore() + await captured.cleanup() + } + }) + + for (const reason of ['ACCOUNT_INELIGIBLE', 'VALIDATION_REQUIRED'] as const) { + test(`continues with B after ${reason} while account persistence is unconfirmed`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const sent: string[] = [] + let loads = 0 + let saveAttempts = 0 + let oauthMutations = 0 + let accountStateBeforeOAuth: + | AccountStorageV4['accounts'][number] + | undefined + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'token-c', + access: 'access-c', + expires: Date.now() + 600_000, + email: 'c@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + oauthMutations++ + const current = await loadAccountStorage(file) + accountStateBeforeOAuth = current?.accounts.find( + (account) => account.email === 'a@example.test', + ) + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + if (sent.length === 2) + return new Response( + JSON.stringify({ + error: { status: reason, message: 'account blocked' }, + }), + { status: 403 }, + ) + return successResponse() + }, + }) + + const storeLoad = spyOn( + defaultAccountStorageStore, + 'load', + ).mockImplementation(async (file) => { + loads++ + if (loads === 2) throw new Error('state readback unavailable') + return loadAccountStorage(file) + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveAttempts++ + await saveAccountStorage(file, next) + throw new Error('state write result unknown') + }) + + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a']) + + await expect( + (await captured.oauth.authorize()).callback, + ).rejects.toMatchObject({ state: 'unconfirmed' }) + expect(oauthMutations).toBe(0) + expect(saveAttempts).toBe(1) + + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a', 'old-a', 'token-b']) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.slice(-1)).toEqual(['token-b']) + expect(saveAttempts).toBe(1) + expect(oauthMutations).toBe(0) + + storeLoad.mockRestore() + save.mockRestore() + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + expect(oauthMutations).toBe(1) + expect(accountStateBeforeOAuth).toMatchObject( + reason === 'ACCOUNT_INELIGIBLE' + ? { + enabled: false, + accountIneligible: true, + accountIneligibleReason: reason, + } + : { + enabled: false, + verificationRequired: true, + verificationRequiredReason: reason, + }, + ) + const finalPool = readPool(path) + expect( + finalPool.accounts.map((account) => account.refreshToken), + ).toEqual(['old-a', 'token-b', 'token-c']) + expect(finalPool.accounts[0]).toMatchObject( + reason === 'ACCOUNT_INELIGIBLE' + ? { + enabled: false, + accountIneligible: true, + accountIneligibleReason: reason, + } + : { + enabled: false, + verificationRequired: true, + verificationRequiredReason: reason, + }, + ) + } finally { + storeLoad.mockRestore() + save.mockRestore() + await captured.cleanup() + } + }) + } + + test('continues with B after an ordinary eligibility-save failure and persists A when saving succeeds', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return sent.length === 1 + ? new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockRejectedValue(new Error('ordinary account-state save failure')) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a', 'token-b']) + expect(readPool(path).accounts[0]?.accountIneligible).toBeUndefined() + } finally { + save.mockRestore() + await captured.cleanup() + } + + const savedPath = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const savedSent: string[] = [] + const saved = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + savedSent.push(auth.access ?? '') + return savedSent.length === 1 + ? new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + try { + expect((await titleRequest(saved.hook)).ok).toBe(true) + expect(savedSent).toEqual(['old-a', 'token-b']) + expect(readPool(savedPath).accounts[0]).toMatchObject({ + enabled: false, + accountIneligible: true, + accountIneligibleReason: 'ACCOUNT_INELIGIBLE', + }) + } finally { + await saved.cleanup() + } + }) + + test('preserves A’s upstream eligibility error when no B account exists', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + ]) + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockRejectedValue(new Error('ordinary account-state save failure')) + try { + const response = await titleRequest(captured.hook) + expect(response.status).toBe(502) + expect(await response.text()).toContain( + 'Antigravity account is ineligible', + ) + expect(sent).toEqual(['old-a']) + } finally { + save.mockRestore() + await captured.cleanup() + } + }) + + test('keeps a sent 403 terminal when its account-state flush crosses OAuth transition', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const saveStarted = deferred() + const releaseSave = deferred() + const sent: string[] = [] + let sendCount = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sendCount++ + sent.push(auth.access ?? '') + if (sendCount === 2) + return new Response( + JSON.stringify({ + error: { + status: 'ACCOUNT_INELIGIBLE', + message: 'account blocked', + }, + }), + { status: 403 }, + ) + return successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveStarted.resolve() + await releaseSave.promise + return saveAccountStorage(file, next) + }) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + const pending403 = titleRequest(captured.hook) + await saveStarted.promise + const oauthCallback = (await captured.oauth.authorize()).callback + await Bun.sleep(10) + releaseSave.resolve() + + const response = await pending403 + await oauthCallback + expect(response.status).toBe(502) + expect(await response.text()).toContain( + 'Antigravity HTTP 403 (ACCOUNT_INELIGIBLE)', + ) + expect(sent).toEqual(['old-a', 'old-a']) + } finally { + releaseSave.resolve() + save.mockRestore() + await captured.cleanup() + } + }) + + test('does not dispatch B after cancellation while persisting a 403 account flag', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const saveStarted = deferred() + const releaseSave = deferred() + const sent: string[] = [] + let sendCount = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sendCount++ + sent.push(auth.access ?? '') + return sendCount === 2 + ? new Response( + JSON.stringify({ + error: { + status: 'ACCOUNT_INELIGIBLE', + message: 'account blocked', + }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveStarted.resolve() + await releaseSave.promise + return saveAccountStorage(file, next) + }) + const controller = new AbortController() + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + const pending403 = titleRequest(captured.hook, controller.signal) + await saveStarted.promise + controller.abort() + releaseSave.resolve() + await expect(pending403).rejects.toThrow() + expect(sent).toEqual(['old-a', 'old-a']) + } finally { + releaseSave.resolve() + save.mockRestore() + await captured.cleanup() + } + }) + + test('does not report OAuth success when storage returns without the new account', async () => { + const path = seedPool([ + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file) => (await loadAccountStorage(file))!, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'not confirmed on disk', + ) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['token-b']) + } finally { + await captured.cleanup() + } + }) + + test('does not install a rejected OAuth readback or leave its fingerprint saver orphaned', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + fingerprint: { + deviceId: 'device', + sessionToken: 'session', + userAgent: buildAntigravityHarnessUserAgent(), + apiClient: 'antigravity-cli', + clientMetadata: { + ideType: 'ANTIGRAVITY', + platform: 'MACOS', + pluginType: 'ANTIGRAVITY', + }, + createdAt: 1, + }, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let loads = 0 + let writes = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + loadAccountStorage: async (file) => { + loads++ + const loaded = await loadAccountStorage(file) + if (loads !== 2 || !loaded) return loaded + return { + ...loaded, + accounts: loaded.accounts.map((account) => + account.email === 'a@example.test' + ? { + ...account, + fingerprint: { + deviceId: 'stale-device', + sessionToken: 'stale-session', + userAgent: 'antigravity/cli/1.0.0 (legacy)', + apiClient: 'antigravity-cli', + clientMetadata: { + ideType: 'ANTIGRAVITY', + platform: 'MACOS', + pluginType: 'ANTIGRAVITY', + }, + createdAt: 2, + }, + } + : account, + ), + } + }, + mutateAccountStorage: async (file, mutator) => { + writes++ + if (writes === 1) return (await loadAccountStorage(file))! + return mutateAccountStorage(file, mutator) + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'not confirmed on disk', + ) + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + await Bun.sleep(1100) + expect( + readPool(path).accounts.map((account) => account.refreshToken), + ).toEqual(['new-a', 'token-b']) + expect(writes).toBe(2) } finally { await captured.cleanup() } From 7b3685b3fc9b1e7a2fae48c319807a4ae323fd24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Thu, 8 Oct 2026 16:37:52 +0800 Subject: [PATCH 3/7] bugfix: harden account pool transitions and disposal --- packages/core/src/account-manager.test.ts | 208 ++++++++++++ packages/core/src/account-manager.ts | 56 +++- packages/opencode-v2/src/plugin.ts | 29 +- packages/opencode-v2/test/plugin.test.ts | 384 ++++++++++++++++++++++ 4 files changed, 660 insertions(+), 17 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index 707d0c46..2bdff0a3 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -444,6 +444,214 @@ describe('core AccountManager', () => { expect(memory.mergedSaves()).toBe(1) }) + it.each([ + ['merge', (manager: AccountManager) => manager.saveToDisk()], + ['replace', (manager: AccountManager) => manager.saveToDiskReplace()], + ] as const)('dispose waits for an in-flight direct %s save', async (_kind, save) => { + let release!: () => void + let started!: () => void + const saveStarted = new Promise((resolve) => { + started = resolve + }) + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + started() + await new Promise((resolve) => { + release = resolve + }) + return memory.store.saveMerged(path, next) + }, + mutate: async (path, fn) => { + started() + await new Promise((resolve) => { + release = resolve + }) + return memory.store.mutate(path, fn) + }, + }, + }) + + const saving = save(manager) + await saveStarted + let disposed = false + const disposing = manager.dispose().then(() => { + disposed = true + }) + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + expect(disposed).toBe(false) + + release() + await Promise.all([saving, disposing]) + expect(disposed).toBe(true) + }) + + it('dispose drains queued direct saves across repeated calls', async () => { + let releaseFirst!: () => void + let startedFirst!: () => void + let startedSecond!: () => void + const firstStarted = new Promise((resolve) => { + startedFirst = resolve + }) + const secondStarted = new Promise((resolve) => { + startedSecond = resolve + }) + let calls = 0 + const memory = createStore(stored) + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + calls++ + startedFirst() + await new Promise((resolve) => { + releaseFirst = resolve + }) + return memory.store.saveMerged(path, next) + }, + mutate: async (path, fn) => { + calls++ + startedSecond() + return memory.store.mutate(path, fn) + }, + }, + }) + + const firstSave = manager.saveToDisk() + const queuedSave = manager.saveToDiskReplace() + await firstStarted + let firstDisposeDone = false + const firstDispose = manager.dispose().then(() => { + firstDisposeDone = true + }) + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + const secondDispose = manager.dispose() + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + expect(firstDisposeDone).toBe(false) + releaseFirst() + await secondStarted + let secondDisposeDone = false + const observedSecondDispose = secondDispose.then(() => { + secondDisposeDone = true + }) + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + expect(secondDisposeDone).toBe(false) + await Promise.all([ + firstSave, + queuedSave, + firstDispose, + observedSecondDispose, + ]) + expect(calls).toBe(2) + }) + + it('dispose waits through a failed direct save without adding another write', async () => { + let release!: () => void + let started!: () => void + const saveStarted = new Promise((resolve) => { + started = resolve + }) + const memory = createStore(stored) + let calls = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + calls++ + started() + await new Promise((resolve) => { + release = resolve + }) + throw new Error('direct save failed') + }, + }, + }) + + const saving = manager.saveToDisk() + await saveStarted + let disposed = false + const disposing = manager.dispose().then(() => { + disposed = true + }) + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + expect(disposed).toBe(false) + release() + await expect(saving).rejects.toThrow('direct save failed') + await disposing + expect(disposed).toBe(true) + expect(calls).toBe(1) + expect(memory.mergedSaves()).toBe(0) + }) + + it.each([ + true, + false, + ])('dispose waits for an accepted fence reconciliation (retrySucceeds=%s)', async (retrySucceeds) => { + const memory = createStore(stored) + let releaseRetry!: () => void + let retryStarted!: () => void + const retryStartedPromise = new Promise((resolve) => { + retryStarted = resolve + }) + let mergeAttempts = 0 + let retryAttempts = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + mergeAttempts++ + throw new Error('ELOCKED') + }, + mutate: async (path, fn) => { + retryAttempts++ + retryStarted() + await new Promise((resolve, reject) => { + releaseRetry = () => { + if (!retrySucceeds) { + reject(new Error('retry failed')) + return + } + resolve() + } + }) + return memory.store.mutate(path, fn) + }, + }, + }) + + await expect(manager.saveToDisk()).rejects.toThrow('ELOCKED') + const fence = manager.flushAndStopSaving() + await retryStartedPromise + let fenceSettled = false + const observedFence = fence.then( + () => { + fenceSettled = true + return 'resolved' + }, + () => { + fenceSettled = true + return 'rejected' + }, + ) + let disposed = false + const disposing = manager.dispose().then(() => { + disposed = true + }) + for (let turn = 0; turn < 5; turn++) await Promise.resolve() + expect(fenceSettled).toBe(false) + expect(disposed).toBe(false) + + releaseRetry() + expect(await observedFence).toBe(retrySucceeds ? 'resolved' : 'rejected') + await disposing + expect(disposed).toBe(true) + expect(mergeAttempts).toBe(1) + expect(retryAttempts).toBe(1) + expect(memory.mutations()).toBe(retrySucceeds ? 1 : 0) + }) + it('retires a clean manager without rewriting external pool metadata', async () => { const memory = createStore(stored) const manager = new AccountManager(undefined, stored, { diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 77a286c3..9525b79f 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -378,6 +378,7 @@ export class AccountManager { private directSaves = new Set>() private saveFailure: unknown = null private disposed = false + private disposeInFlight: Promise | null = null private persistenceStopped = false private storageAdmission: 'open' | 'fencing' | 'stopped' = 'open' private fenceInFlight: Promise | null = null @@ -2152,17 +2153,54 @@ export class AccountManager { await Promise.all([...this.directSaves].map((save) => save.catch(() => {}))) } - async dispose(): Promise { - if (this.disposed) return + dispose(): Promise { + if (this.disposeInFlight) return this.disposeInFlight this.disposed = true - if (this.saveTimeout) { - clearTimeout(this.saveTimeout) - this.saveTimeout = null - } - if (this.savePending) { - await this.executeSave() + const disposing = this.finishDispose() + this.disposeInFlight = disposing + void disposing + .finally(() => { + this.disposeInFlight = null + }) + .catch(() => {}) + return disposing + } + + private async finishDispose(): Promise { + while (true) { + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + if (this.savePending && !this.saveInFlight) { + this.saveInFlight = this.executeSave().finally(() => { + this.saveInFlight = null + }) + } + + const directSaves = [...this.directSaves].map((save) => + save.catch(() => {}), + ) + const inFlight = this.saveInFlight + const saveQueue = this.saveQueue + const fenceInFlight = this.fenceInFlight?.catch(() => {}) + await Promise.all([ + ...directSaves, + ...(inFlight ? [inFlight] : []), + saveQueue, + ...(fenceInFlight ? [fenceInFlight] : []), + ]) + + if ( + !this.savePending && + !this.saveInFlight && + this.directSaves.size === 0 && + this.saveQueue === saveQueue && + !this.fenceInFlight + ) { + return + } } - await this.saveInFlight } private async executeSave(): Promise { diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index f87f0486..9834a787 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -681,6 +681,17 @@ export function createOpenCodeV2AntigravityPlugin( poolUnavailable = false } + const recoverAndInstallPool = async ( + predecessor: AccountManager, + ): Promise => { + const stored = await dependencies.loadAccountStorage(ACCOUNTS_FILE) + if (stored === null && predecessor.getTotalAccountCount() > 0) { + log('pool-readback-missing', 'populated pool') + throw new Error('Antigravity account pool readback is missing') + } + installPool(stored) + } + const findConfirmedOAuthAccount = ( stored: AccountStorageV4 | null, result: OAuthSuccess, @@ -722,7 +733,7 @@ export function createOpenCodeV2AntigravityPlugin( throw new Error('Antigravity plugin is shutting down') if (poolUnavailable) { try { - installPool(await dependencies.loadAccountStorage(ACCOUNTS_FILE)) + await recoverAndInstallPool(manager) } catch (error) { log('pool-recovery-read-error', errorMessage(error)) throw new Error('Antigravity account pool is unavailable', { @@ -734,7 +745,6 @@ export function createOpenCodeV2AntigravityPlugin( transition = new Promise((resolve) => { finishTransition = resolve }) - poolGeneration += 1 const previous = manager try { await previous.flushAndStopSaving() @@ -745,6 +755,7 @@ export function createOpenCodeV2AntigravityPlugin( transition = null throw error } + poolGeneration += 1 try { await dependencies.mutateAccountStorage(ACCOUNTS_FILE, (current) => @@ -759,7 +770,7 @@ export function createOpenCodeV2AntigravityPlugin( log('pool-reloaded', manager.getTotalAccountCount()) } catch (error) { try { - installPool(await dependencies.loadAccountStorage(ACCOUNTS_FILE)) + await recoverAndInstallPool(previous) } catch (readError) { poolUnavailable = true log('pool-readback-error', errorMessage(readError)) @@ -883,7 +894,7 @@ export function createOpenCodeV2AntigravityPlugin( errorMessage(error), ) } - generationChanged(generation) + await waitForSelection(generation) } const waitForSelection = async ( @@ -999,7 +1010,7 @@ export function createOpenCodeV2AntigravityPlugin( endpoint, errorMessage(error), ) - generationChanged(selectedGeneration) + await waitForSelection(selectedGeneration) failure = error continue } @@ -1014,7 +1025,9 @@ export function createOpenCodeV2AntigravityPlugin( if (response.ok) { if ( selectedGeneration === poolGeneration && - selectedManager === manager + selectedManager === manager && + !transition && + !poolUnavailable ) { manager.markRequestSuccess(account) manager.markAccountUsed(account.index) @@ -1025,13 +1038,13 @@ export function createOpenCodeV2AntigravityPlugin( } dispatchedError = new Error(`Antigravity HTTP ${response.status}`) - generationChanged(selectedGeneration) + await waitForSelection(selectedGeneration) const { reason, message } = await readErrorDetails(response) dispatchedError = new Error( `Antigravity HTTP ${response.status}${reason ? ` (${reason})` : ''}`, ) - generationChanged(selectedGeneration) + await waitForSelection(selectedGeneration) log('upstream-error', response.status, reason ?? '', message) if ( response.status === 404 && diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index 38150407..cfb0db98 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -1117,6 +1117,91 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + test('continues to B after a retryable pool retirement fence fails', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const saveStarted = deferred() + const releaseSave = deferred() + const sent: string[] = [] + let saveCalls = 0 + let mutations = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + mutations++ + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + if (sent.length === 1) { + return new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + } + return successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async () => { + saveCalls++ + saveStarted.resolve() + await releaseSave.promise + throw new Error('retryable save failure') + }) + try { + const pendingRequest = titleRequest(captured.hook) + await saveStarted.promise + const oauthCallback = (await captured.oauth.authorize()).callback + releaseSave.reject(new Error('retryable save failure')) + await expect(oauthCallback).rejects.toThrow('persistence retryable') + expect((await pendingRequest).ok).toBe(true) + expect(sent).toEqual(['old-a', 'token-b']) + expect(saveCalls).toBe(1) + expect(mutations).toBe(0) + } finally { + releaseSave.resolve() + save.mockRestore() + await captured.cleanup() + } + }) + test('does not dispatch B after cancellation while persisting a 403 account flag', async () => { seedPool([ { @@ -1198,6 +1283,182 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + test('does not persist request success from a pool while its retirement is pending', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + ]) + const saveStarted = deferred() + const releaseSave = deferred() + const sentAgain = deferred() + const releaseSecondResponse = deferred() + let sendCount = 0 + let saveCount = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async () => { + sendCount++ + if (sendCount === 2) { + sentAgain.resolve() + return releaseSecondResponse.promise + } + return successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveCount++ + if (saveCount === 1) { + saveStarted.resolve() + await releaseSave.promise + } + return saveAccountStorage(file, next) + }) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + await saveStarted.promise + const pendingRequest = titleRequest(captured.hook) + await sentAgain.promise + const oauthCallback = (await captured.oauth.authorize()).callback + await Bun.sleep(10) + releaseSecondResponse.resolve(successResponse()) + expect((await pendingRequest).ok).toBe(true) + expect(saveCount).toBe(1) + releaseSave.resolve() + await oauthCallback + expect(saveCount).toBe(1) + } finally { + releaseSave.resolve() + releaseSecondResponse.resolve(successResponse()) + save.mockRestore() + await captured.cleanup() + } + }) + + test('aborts a sent failure while it is waiting for pool retirement', async () => { + seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + const saveStarted = deferred() + const releaseSave = deferred() + const failureStarted = deferred() + const releaseFailure = deferred() + const sent: string[] = [] + let sendCount = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sendCount++ + sent.push(auth.access ?? '') + if (sendCount === 2) { + failureStarted.resolve() + return releaseFailure.promise + } + return successResponse() + }, + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveStarted.resolve() + await releaseSave.promise + return saveAccountStorage(file, next) + }) + const cancelled = new AbortController() + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + await saveStarted.promise + const pendingRequest = titleRequest(captured.hook, cancelled.signal) + await failureStarted.promise + const oauthCallback = (await captured.oauth.authorize()).callback + await Bun.sleep(10) + releaseFailure.resolve( + new Response(JSON.stringify({ error: { message: 'limited' } }), { + status: 429, + }), + ) + await Bun.sleep(10) + cancelled.abort() + await expect(pendingRequest).rejects.toThrow() + releaseSave.resolve() + await oauthCallback + expect(sent).toEqual(['old-a', 'old-a']) + } finally { + cancelled.abort() + releaseFailure.resolve(successResponse()) + releaseSave.resolve() + save.mockRestore() + await captured.cleanup() + } + }) + test('does not report OAuth success when storage returns without the new account', async () => { const path = seedPool([ { @@ -1781,6 +2042,129 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { await captured.cleanup() } }) + + test('keeps a populated pool unavailable after missing OAuth readback until concrete recovery', async () => { + seedPool([ + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let loads = 0 + let writes = 0 + let readable = false + const recoveredPool: AccountStorageV4 = { + version: 4, + accounts: [], + activeIndex: 0, + } + let sends = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + loadAccountStorage: async (file) => { + loads++ + if (loads > 1 && !readable) return null + if (readable && loads === 5) return recoveredPool + return loadAccountStorage(file) + }, + mutateAccountStorage: async (file, mutator) => { + writes++ + return mutateAccountStorage(file, mutator) + }, + send: async () => { + sends++ + return successResponse() + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).rejects.toMatchObject({ + message: 'Antigravity account pool is unavailable', + cause: expect.objectContaining({ + message: 'Antigravity OAuth account was not confirmed on disk', + }), + }) + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'unavailable', + ) + expect(writes).toBe(1) + expect(sends).toBe(0) + readable = true + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + expect(writes).toBe(2) + expect( + readPool( + join( + process.env.OPENCODE_CONFIG_DIR ?? '', + 'antigravity-accounts.json', + ), + ).accounts.map((account) => account.refreshToken), + ).toEqual(['token-b', 'new-a']) + } finally { + await captured.cleanup() + } + }) + + test('keeps known-empty first-run recovery compatible with missing reads', async () => { + let loads = 0 + let writes = 0 + const captured = await setupPoolAdapter({ + loadAccountStorage: async (file) => { + loads++ + if (loads === 1 || loads === 2 || loads === 3) return null + return loadAccountStorage(file) + }, + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + writes++ + return mutateAccountStorage(file, mutator) + }, + }) + try { + await expect((await captured.oauth.authorize()).callback).rejects.toThrow( + 'not confirmed', + ) + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toBeDefined() + expect(writes).toBe(2) + expect(loads).toBe(4) + } finally { + await captured.cleanup() + } + }) test('exports the plugin contract shape', () => { expect(plugin).toBeTypeOf('object') expect(plugin.id).toBe('cortexkit.antigravity-auth') From bfdc9847b4fef7f6f15a7f9b602ca971210e9b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Thu, 8 Oct 2026 19:12:49 +0800 Subject: [PATCH 4/7] bugfix(core): recover saving after failed account pool fences --- packages/core/src/account-manager.test.ts | 701 ++++++++++++++++++++++ packages/core/src/account-manager.ts | 216 ++++++- packages/opencode-v2/src/plugin.ts | 1 + packages/opencode-v2/test/plugin.test.ts | 165 +++++ 4 files changed, 1076 insertions(+), 7 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index 2bdff0a3..c9a52043 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -778,6 +778,707 @@ describe('core AccountManager', () => { expect(calls).toBe(1) }) + it('recovers a failed fence only after explicit enablement and a new flush', async () => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + writes++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + if (writes === 1) throw new Error('response lost') + return next + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted' + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(writes).toBe(1) + const enable = Reflect.get(manager, 'enableSavingRecovery') + expect(typeof enable).toBe('function') + if (typeof enable !== 'function') return + Reflect.apply(enable, manager, []) + readable = true + manager.getAccounts()[0]!.label = 'latest' + manager.requestSaveToDisk() + await manager.flushSaveToDisk({ strict: true }) + expect(writes).toBe(1) + expect(memory.state()?.accounts[0]?.label).toBe('latest') + await manager.saveToDisk() + }) + + it('does not enable recovery while the first fence is still in progress', async () => { + const memory = createStore(stored) + let releaseWrite!: () => void + let markWriteStarted!: () => void + const writeStarted = new Promise((resolve) => { + markWriteStarted = resolve + }) + let reads = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + markWriteStarted() + await new Promise((resolve) => { + releaseWrite = resolve + }) + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + const fence = manager.flushAndStopSaving() + await writeStarted + manager.enableSavingRecovery() + releaseWrite() + await expect(fence).rejects.toMatchObject({ state: 'unconfirmed' }) + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await manager.dispose() + }) + + it('retries a missing accepted intent under a guarded baseline, then flushes without dirty memory', async () => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + writes++ + throw new Error('write lost') + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted' + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + await manager.flushSaveToDisk({ strict: true }) + expect(writes).toBe(1) + expect(memory.mutations()).toBe(1) + expect(memory.state()?.accounts[0]?.label).toBe('accepted') + }) + + it('starts enabled recovery from a requested save after the debounce', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let readable = false + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, snapshot) => { + writes++ + if (writes === 1) throw new Error('write lost') + return memory.store.saveMerged(path, snapshot) + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + manager.getAccounts()[0]!.label = 'latest' + manager.requestSaveToDisk() + await jest.advanceTimersByTime(1000) + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.label).toBe('latest') + } finally { + jest.useRealTimers() + } + }) + + it('keeps a conflicting or unavailable failed baseline fenced without overwrite', async () => { + for (const changed of [false, true]) { + const memory = createStore(stored) + let readable = false + let reads = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + throw new Error('write lost') + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted' + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + if (changed) { + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external' } : account, + ), + })) + readable = true + } + const before = memory.state() + await expect(manager.flushSaveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()).toEqual(before) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await manager.dispose() + } + }) + + it('does not replay a failed intent when its original baseline was unreadable', async () => { + const memory = createStore(stored) + let readable = false + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + if (!readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + writes++ + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + await expect(manager.flushSaveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(writes).toBe(1) + expect(memory.mutations()).toBe(0) + await manager.dispose() + }) + + it('keeps repeated unreadable requests single-flight and waits for an explicit retry', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let readable = false + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + writes++ + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + for (let index = 0; index < 20; index++) manager.requestSaveToDisk() + const first = manager.flushSaveToDisk() + const second = manager.flushSaveToDisk() + const failures = await Promise.allSettled([first, second]) + expect(failures.map((result) => result.status)).toEqual([ + 'rejected', + 'rejected', + ]) + const readsAfterFailure = reads + await jest.advanceTimersByTime(5000) + expect(reads).toBe(readsAfterFailure) + expect(writes).toBe(1) + readable = true + await manager.flushSaveToDisk() + expect(memory.state()?.accounts).toHaveLength(2) + } finally { + jest.useRealTimers() + } + }) + + it('keeps changes made during the recovery write for the next ordinary save', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseWrite!: () => void + let markWriteStarted!: () => void + const writeStarted = new Promise((resolve) => { + markWriteStarted = resolve + }) + let blockRecoveryWrite = false + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + }, + mutate: async (path, fn) => { + if (blockRecoveryWrite) { + blockRecoveryWrite = false + markWriteStarted() + await new Promise((resolve) => { + releaseWrite = resolve + }) + } + return memory.store.mutate(path, fn) + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted' + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + blockRecoveryWrite = true + manager.getAccounts()[0]!.label = 'captured' + manager.requestSaveToDisk() + const recovery = manager.flushSaveToDisk() + await writeStarted + manager.getAccounts()[0]!.label = 'late' + manager.requestSaveToDisk() + releaseWrite() + await recovery + expect(memory.state()?.accounts[0]?.label).toBe('captured') + await jest.advanceTimersByTime(1000) + expect(memory.state()?.accounts[0]?.label).toBe('late') + } finally { + jest.useRealTimers() + } + }) + + it('holds a late flush until its dirty update after an active recovery write', async () => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseWrite!: () => void + let markWriteStarted!: () => void + const writeStarted = new Promise((resolve) => { + markWriteStarted = resolve + }) + let blockRecoveryWrite = false + let originalWrite = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + if (originalWrite) { + originalWrite = false + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + } + return memory.store.saveMerged(path, next) + }, + mutate: async (path, fn) => { + if (blockRecoveryWrite) { + blockRecoveryWrite = false + markWriteStarted() + await new Promise((resolve) => { + releaseWrite = resolve + }) + } + return memory.store.mutate(path, fn) + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + blockRecoveryWrite = true + manager.getAccounts()[0]!.label = 'A' + manager.requestSaveToDisk() + const firstFlush = manager.flushSaveToDisk() + await writeStarted + manager.getAccounts()[0]!.label = 'B' + manager.requestSaveToDisk() + let lateResolved = false + const lateFlush = manager.flushSaveToDisk().then(() => { + lateResolved = true + }) + releaseWrite() + await firstFlush + await Promise.resolve() + expect(memory.state()?.accounts[0]?.label).toBe('A') + expect(lateResolved).toBe(false) + await jest.advanceTimersByTime(1000) + await lateFlush + expect(memory.state()?.accounts[0]?.label).toBe('B') + } finally { + jest.useRealTimers() + } + }) + + it.each([ + false, + true, + ])('confirms a committed recovery write before %s later dirty update', async (lateDirty) => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + throw new Error('original write lost') + }, + mutate: async (path, fn) => { + writes++ + const saved = await memory.store.mutate(path, fn) + if (writes === 2) { + if (lateDirty) { + manager.getAccounts()[0]!.label = 'B' + manager.requestSaveToDisk() + } + throw new Error('recovery response lost') + } + return saved + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + manager.getAccounts()[0]!.label = 'A' + manager.requestSaveToDisk() + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'recovery response lost', + ) + expect(memory.state()?.accounts[0]?.label).toBe('A') + await manager.flushSaveToDisk() + expect(writes).toBe(lateDirty ? 3 : 2) + expect(memory.state()?.accounts[0]?.label).toBe(lateDirty ? 'B' : 'A') + }) + + it('dispose preserves a late requested save accepted by an active ordinary fence', async () => { + const memory = createStore(stored) + let releaseSave!: () => void + let markSaveStarted!: () => void + const saveStarted = new Promise((resolve) => { + markSaveStarted = resolve + }) + let first = true + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async (path, snapshot) => { + if (first) { + first = false + markSaveStarted() + await new Promise((resolve) => { + releaseSave = resolve + }) + } + return memory.store.saveMerged(path, snapshot) + }, + }, + }) + manager.requestSaveToDisk() + const fence = manager.flushAndStopSaving() + await saveStarted + manager.getAccounts()[0]!.label = 'late' + manager.requestSaveToDisk() + const disposing = manager.dispose() + releaseSave() + await Promise.all([fence, disposing]) + expect(memory.state()?.accounts[0]?.label).toBe('late') + }) + + it('dispose drains a failed active fence after rejecting its late work', async () => { + const memory = createStore(stored) + let releaseSave!: () => void + let markSaveStarted!: () => void + const saveStarted = new Promise((resolve) => { + markSaveStarted = resolve + }) + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + writes++ + markSaveStarted() + await new Promise((resolve) => { + releaseSave = resolve + }) + throw new Error('response lost') + }, + }, + }) + manager.requestSaveToDisk() + const initialFlush = manager + .flushSaveToDisk({ strict: true }) + .catch((error: unknown) => error) + const fence = manager.flushAndStopSaving() + await saveStarted + manager.getAccounts()[0]!.label = 'late' + manager.requestSaveToDisk() + const lateFlush = manager.flushSaveToDisk().catch((error: unknown) => error) + const disposing = manager.dispose() + releaseSave() + expect(String(await initialFlush)).toContain('response lost') + expect(String(await lateFlush)).toContain('persistence has stopped') + await expect(fence).rejects.toMatchObject({ state: 'unconfirmed' }) + const observedFailure = Reflect.get(manager, 'failedFenceUnconfirmed') + // The old implementation spins in dispose after this failure; release it + // before asserting RED so the test process can exit. + if (!observedFailure) await manager.stopSaving() + await disposing + expect(observedFailure).toBe(true) + expect(writes).toBe(1) + expect(memory.mutations()).toBe(0) + }) + + it('invalidates a started recovery on dispose and drains it across repeated calls', async () => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseRead!: () => void + let markReadStarted!: () => void + const readStarted = new Promise((resolve) => { + markReadStarted = resolve + }) + let blockRecoveryRead = false + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (blockRecoveryRead) { + blockRecoveryRead = false + markReadStarted() + await new Promise((resolve) => { + releaseRead = resolve + }) + } + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.requestSaveToDisk() + blockRecoveryRead = true + const recovery = manager.flushSaveToDisk() + await readStarted + let disposed = false + const firstDispose = manager.dispose().then(() => { + disposed = true + }) + const secondDispose = manager.dispose() + expect(disposed).toBe(false) + readable = true + releaseRead() + await expect(recovery).rejects.toThrow('persistence has stopped') + await Promise.all([firstDispose, secondDispose]) + expect(disposed).toBe(true) + expect(memory.mutations()).toBe(0) + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + }) + + it('invalidates a started recovery before stopSaving returns', async () => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseRead!: () => void + let markReadStarted!: () => void + const readStarted = new Promise((resolve) => { + markReadStarted = resolve + }) + let blockRecoveryRead = false + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (blockRecoveryRead) { + blockRecoveryRead = false + markReadStarted() + await new Promise((resolve) => { + releaseRead = resolve + }) + } + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.requestSaveToDisk() + blockRecoveryRead = true + const recovery = manager.flushSaveToDisk() + await readStarted + let stopped = false + const stopping = manager.stopSaving().then(() => { + stopped = true + }) + expect(stopped).toBe(false) + readable = true + releaseRead() + await expect(recovery).rejects.toThrow('persistence has stopped') + await stopping + expect(stopped).toBe(true) + expect(memory.mutations()).toBe(0) + }) + + it('lets a new fence supersede recovery and keeps the retired manager closed', async () => { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseRead!: () => void + let markReadStarted!: () => void + const readStarted = new Promise((resolve) => { + markReadStarted = resolve + }) + let blockRecoveryRead = false + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (blockRecoveryRead) { + blockRecoveryRead = false + markReadStarted() + await new Promise((resolve) => { + releaseRead = resolve + }) + } + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + blockRecoveryRead = true + const recovery = manager.flushSaveToDisk() + await readStarted + readable = true + const fence = manager.flushAndStopSaving() + releaseRead() + await expect(recovery).rejects.toThrow('persistence has stopped') + await fence + const writes = memory.mutations() + manager.enableSavingRecovery() + manager.requestSaveToDisk() + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + expect(memory.mutations()).toBe(writes) + }) + it('confirms the original nested snapshot and then saves late intent', async () => { const memory = createStore(stored) let releaseRead!: () => void diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 9525b79f..c69f4a0e 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -373,6 +373,12 @@ export class AccountManager { private lastToastTime = 0 private savePending = false + private saveRequestVersion = 0 + private recoveryConfirmedVersion = 0 + private failedRecoverySnapshot: { + snapshot: AccountStorageV4 + requestVersion: number + } | null = null private saveTimeout: ReturnType | null = null private saveInFlight: Promise | null = null private directSaves = new Set>() @@ -382,6 +388,10 @@ export class AccountManager { private persistenceStopped = false private storageAdmission: 'open' | 'fencing' | 'stopped' = 'open' private fenceInFlight: Promise | null = null + private failedFenceUnconfirmed = false + private savingRecoveryEnabled = false + private savingRecoveryGeneration = 0 + private savingRecoveryInFlight: Promise | null = null private saveQueue: Promise = Promise.resolve() private acceptedSaves = new Set>() private acceptedSaveFailure: unknown = null @@ -1829,11 +1839,13 @@ export class AccountManager { private async drainSaveIntents( recoveredBaseline?: AccountStorageV4, - ): Promise { + assertActive?: () => void, + ): Promise { let guardedBaseline = recoveredBaseline const failed = this.failedSaveAttempt if (failed) { const result = await this.readBackFailedSave() + assertActive?.() if (result === 'missing') { guardedBaseline = await this.retryFailedSave() } else if (result === 'saved') { @@ -1847,6 +1859,7 @@ export class AccountManager { } } while (this.pendingSaveIntents.length > 0) { + assertActive?.() const intent = this.pendingSaveIntents[0]! const { kind, snapshot } = intent let baseline: AccountStorageV4 | null | undefined = guardedBaseline @@ -1860,6 +1873,7 @@ export class AccountManager { }) } } + assertActive?.() try { let saved: AccountStorageV4 if (guardedBaseline !== undefined) { @@ -1894,6 +1908,7 @@ export class AccountManager { throw error } } + return guardedBaseline } private async readBackFailedSave(): Promise<'none' | 'saved' | 'missing'> { @@ -1965,9 +1980,9 @@ export class AccountManager { private enqueueFailedSaveReconciliation( recoveredBaseline?: AccountStorageV4, ): Promise { - const reconcile = this.saveQueue.then(() => - this.drainSaveIntents(recoveredBaseline), - ) + const reconcile = this.saveQueue.then(async () => { + await this.drainSaveIntents(recoveredBaseline) + }) this.saveQueue = reconcile.then( () => {}, () => {}, @@ -1979,25 +1994,190 @@ export class AccountManager { if ( !this.savePending || this.saveTimeout || - this.storageAdmission !== 'open' + (this.storageAdmission !== 'open' && + !(this.storageAdmission === 'fencing' && this.savingRecoveryEnabled)) || + this.savingRecoveryInFlight ) return this.saveTimeout = setTimeout(() => { + if (this.storageAdmission === 'fencing' && this.savingRecoveryEnabled) { + this.saveTimeout = null + void this.startSavingRecovery().catch(() => {}) + return + } this.saveInFlight = this.executeSave().finally(() => { this.saveInFlight = null }) }, 1000) } + /** Permit a live failed fence to reconcile on its next save request. */ + enableSavingRecovery(): void { + if ( + this.storageAdmission !== 'fencing' || + !this.failedFenceUnconfirmed || + this.persistenceStopped || + this.disposed + ) + return + this.savingRecoveryEnabled = true + } + + private invalidateSavingRecovery(): void { + this.savingRecoveryGeneration++ + this.savingRecoveryEnabled = false + } + + private discardFailedFenceForDispose(): void { + if ( + !this.disposed || + this.storageAdmission !== 'fencing' || + !this.failedFenceUnconfirmed + ) + return + this.invalidateSavingRecovery() + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + this.savePending = false + const resolvers = this.savePromiseResolvers + this.savePromiseResolvers = [] + for (const { reject } of resolvers) { + reject(new Error('Account manager persistence has stopped')) + } + } + + private assertSavingRecovery(generation: number): void { + if ( + generation !== this.savingRecoveryGeneration || + !this.savingRecoveryEnabled || + this.storageAdmission !== 'fencing' || + this.persistenceStopped || + this.disposed + ) { + throw new Error('Account manager persistence has stopped') + } + } + + private startSavingRecovery(): Promise { + if (this.savingRecoveryInFlight) return this.savingRecoveryInFlight + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } + const generation = this.savingRecoveryGeneration + const recovery = this.saveQueue.then(async () => { + this.assertSavingRecovery(generation) + // Resolve the exact accepted failure and every retained intent before + // considering memory that changed while admission was fenced. + const failedAttempt = this.failedSaveAttempt + const baseline = await this.drainSaveIntents(undefined, () => + this.assertSavingRecovery(generation), + ) + this.assertSavingRecovery(generation) + if ( + this.failedRecoverySnapshot && + failedAttempt?.snapshot === this.failedRecoverySnapshot.snapshot && + !this.failedSaveAttempt + ) { + if ( + this.saveRequestVersion === this.failedRecoverySnapshot.requestVersion + ) { + this.savePending = false + } + this.failedRecoverySnapshot = null + } + if (baseline === undefined) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + this.acceptedSaveFailure, + ) + } + if (this.savePending) { + const snapshot = structuredClone(this.buildStorageSnapshot()) + const capturedVersion = this.saveRequestVersion + this.savePending = false + try { + await this.store.mutate(this.storagePath, (current) => { + if (!sameStoredState(current, baseline)) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account state changed before a recovery save'), + ) + } + return mergeAccountStorage(current, snapshot) + }) + } catch (error) { + this.failedSaveAttempt = { + kind: 'merge', + baseline, + snapshot, + error, + } + this.savePending = true + this.failedRecoverySnapshot = { + snapshot, + requestVersion: capturedVersion, + } + throw error + } + this.assertSavingRecovery(generation) + this.recoveryConfirmedVersion = capturedVersion + } else { + this.recoveryConfirmedVersion = this.saveRequestVersion + } + this.assertSavingRecovery(generation) + this.storageAdmission = 'open' + this.savingRecoveryEnabled = false + this.failedFenceUnconfirmed = false + this.saveFailure = null + this.acceptedSaveFailure = null + }) + this.saveQueue = recovery.then( + () => {}, + () => {}, + ) + this.savingRecoveryInFlight = recovery + let succeeded = false + void recovery + .then( + () => { + succeeded = true + }, + (error) => { + this.saveFailure = error + }, + ) + .finally(() => { + if (this.savingRecoveryInFlight === recovery) { + this.savingRecoveryInFlight = null + } + if (succeeded && this.storageAdmission === 'open' && !this.disposed) { + this.schedulePendingSave() + } + }) + return recovery + } + requestSaveToDisk(): void { if (this.disposed || this.persistenceStopped) { return } this.savePending = true + this.saveRequestVersion++ this.schedulePendingSave() } async flushSaveToDisk(options: { strict?: boolean } = {}): Promise { + if (this.storageAdmission === 'fencing' && this.savingRecoveryEnabled) { + const requestedVersion = this.saveRequestVersion + await this.startSavingRecovery() + if (requestedVersion > this.recoveryConfirmedVersion) { + await this.flushSaveToDisk({ strict: true }) + } + return + } if (this.storageAdmission !== 'open' || this.persistenceStopped) { throw new Error('Account manager persistence has stopped') } @@ -2024,6 +2204,8 @@ export class AccountManager { new Error('Account manager persistence has stopped'), ) } + this.invalidateSavingRecovery() + this.failedFenceUnconfirmed = false this.storageAdmission = 'fencing' if (this.saveTimeout) { clearTimeout(this.saveTimeout) @@ -2105,12 +2287,15 @@ export class AccountManager { this.storageAdmission = 'stopped' this.persistenceStopped = true this.disposed = true + this.failedFenceUnconfirmed = false return } } catch (error) { if (error instanceof AccountManagerPersistenceError) { if (error.state === 'retryable') { this.storageAdmission = 'open' + } else { + this.failedFenceUnconfirmed = true } throw error } @@ -2118,6 +2303,7 @@ export class AccountManager { this.storageAdmission = 'open' throw new AccountManagerPersistenceError('retryable', error) } + this.failedFenceUnconfirmed = true throw new AccountManagerPersistenceError('unconfirmed', error) } } @@ -2129,8 +2315,11 @@ export class AccountManager { * before starting the mutation. */ async stopSaving(): Promise { + this.invalidateSavingRecovery() if (this.persistenceStopped) { await this.saveInFlight + await this.savingRecoveryInFlight?.catch(() => {}) + this.savePending = false await Promise.all( [...this.directSaves].map((save) => save.catch(() => {})), ) @@ -2150,12 +2339,15 @@ export class AccountManager { reject(new Error('Account manager persistence has stopped')) } await this.saveInFlight + await this.savingRecoveryInFlight?.catch(() => {}) + this.savePending = false await Promise.all([...this.directSaves].map((save) => save.catch(() => {}))) } dispose(): Promise { if (this.disposeInFlight) return this.disposeInFlight this.disposed = true + this.discardFailedFenceForDispose() const disposing = this.finishDispose() this.disposeInFlight = disposing void disposing @@ -2168,11 +2360,16 @@ export class AccountManager { private async finishDispose(): Promise { while (true) { + this.discardFailedFenceForDispose() if (this.saveTimeout) { clearTimeout(this.saveTimeout) this.saveTimeout = null } - if (this.savePending && !this.saveInFlight) { + if ( + this.savePending && + !this.saveInFlight && + this.storageAdmission === 'open' + ) { this.saveInFlight = this.executeSave().finally(() => { this.saveInFlight = null }) @@ -2184,19 +2381,24 @@ export class AccountManager { const inFlight = this.saveInFlight const saveQueue = this.saveQueue const fenceInFlight = this.fenceInFlight?.catch(() => {}) + const recoveryInFlight = this.savingRecoveryInFlight?.catch(() => {}) await Promise.all([ ...directSaves, ...(inFlight ? [inFlight] : []), saveQueue, ...(fenceInFlight ? [fenceInFlight] : []), + ...(recoveryInFlight ? [recoveryInFlight] : []), ]) + this.discardFailedFenceForDispose() + if ( !this.savePending && !this.saveInFlight && this.directSaves.size === 0 && this.saveQueue === saveQueue && - !this.fenceInFlight + !this.fenceInFlight && + !this.savingRecoveryInFlight ) { return } diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index 9834a787..c1ceccee 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -750,6 +750,7 @@ export function createOpenCodeV2AntigravityPlugin( await previous.flushAndStopSaving() } catch (error) { log('pool-save-fence-error', errorMessage(error)) + previous.enableSavingRecovery() finishTransition?.() finishTransition = null transition = null diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index cfb0db98..8afbcb00 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -513,6 +513,171 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + for (const failedWrite of ['missing', 'already-committed'] as const) { + test(`recovers a failed OAuth fence on the next successful request when storage becomes readable (${failedWrite})`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'old-a', + addedAt: 1, + lastUsed: 1, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 2, + lastUsed: 2, + enabled: true, + }, + ]) + let oauthMutations = 0 + let storeLoadCalls = 0 + let unreadableLoadCalls = 0 + let saveCalls = 0 + let recoveryMutations = 0 + let storageReadable = true + const persisted = deferred() + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'new-a', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'project', + }), + mutateAccountStorage: async (file, mutator) => { + oauthMutations++ + return mutateAccountStorage(file, mutator) + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + ensureProjectContext: async (auth) => ({ + auth, + projectId: 'project', + effectiveProjectId: 'project', + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return sent.length === 1 + ? new Response(JSON.stringify({ error: { message: 'limited' } }), { + status: 429, + headers: { 'retry-after': '120' }, + }) + : successResponse() + }, + }) + const storeLoad = spyOn( + defaultAccountStorageStore, + 'load', + ).mockImplementation(async (file) => { + storeLoadCalls++ + if (!storageReadable) { + unreadableLoadCalls++ + throw new Error('temporary account state readback failure') + } + return loadAccountStorage(file) + }) + const save = spyOn( + defaultAccountStorageStore, + 'saveMerged', + ).mockImplementation(async (file, next) => { + saveCalls++ + if (saveCalls === 1) { + if (failedWrite === 'already-committed') + await saveAccountStorage(file, next) + storageReadable = false + throw new Error('account state save result unknown') + } + return saveAccountStorage(file, next) + }) + const storeMutate = spyOn( + defaultAccountStorageStore, + 'mutate', + ).mockImplementation(async (file, mutator, options) => { + const result = await mutateAccountStorage(file, mutator, options) + recoveryMutations++ + persisted.resolve() + return result + }) + try { + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['old-a', 'token-b']) + const callback = (await captured.oauth.authorize()).callback + let fenceError: unknown + try { + await callback + } catch (error) { + fenceError = error + } + expect(fenceError).toBeInstanceOf(AccountManagerPersistenceError) + expect(fenceError).toMatchObject({ + state: 'unconfirmed', + originalError: expect.objectContaining({ + message: 'temporary account state readback failure', + }), + }) + expect(oauthMutations).toBe(0) + expect(unreadableLoadCalls).toBeGreaterThan(0) + expect(readPool(path).accounts[1]?.enabled).toBe(true) + if (failedWrite === 'missing') + expect( + readPool(path).accounts[0]?.rateLimitResetTimes, + ).toBeUndefined() + else + expect( + Object.values( + readPool(path).accounts[0]?.rateLimitResetTimes ?? {}, + ).some((resetAt) => typeof resetAt === 'number'), + ).toBe(true) + + const idleLoadCount = storeLoadCalls + const idleSaveCount = saveCalls + await Bun.sleep(1050) + expect(storeLoadCalls).toBe(idleLoadCount) + expect(saveCalls).toBe(idleSaveCount) + + storageReadable = true + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.slice(-1)).toEqual(['token-b']) + await Promise.race([ + persisted.promise, + Bun.sleep(3000).then(() => { + throw new Error('successful request did not recover account state') + }), + ]) + await Bun.sleep(10) + expect(oauthMutations).toBe(0) + if (failedWrite === 'already-committed') + expect(recoveryMutations).toBe(1) + else expect(recoveryMutations).toBeGreaterThan(0) + const recovered = readPool(path) + expect( + Object.values(recovered.accounts[0]?.rateLimitResetTimes ?? {}).some( + (resetAt) => typeof resetAt === 'number', + ), + ).toBe(true) + expect(recovered.accounts[1]?.lastUsed).toBeGreaterThan(2) + } finally { + storeLoad.mockRestore() + save.mockRestore() + storeMutate.mockRestore() + await captured.cleanup() + } + }) + } + test('reconciles a write-then-error without replaying OAuth mutation', async () => { const path = seedPool([ { From 2f685d0d14593638c378a7488090d73e8b8ecb57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Fri, 9 Oct 2026 00:46:05 +0800 Subject: [PATCH 5/7] bugfix: harden account pool recovery and OAuth confirmation --- packages/core/src/account-manager.test.ts | 498 +++++++++++++++++++-- packages/core/src/account-manager.ts | 75 ++-- packages/opencode-v2/src/plugin.ts | 81 ++-- packages/opencode-v2/test/plugin.test.ts | 507 +++++++++++++++++++++- 4 files changed, 1047 insertions(+), 114 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index c9a52043..f0babd6c 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, jest } from 'bun:test' +import { spawn } from 'node:child_process' import { AccountManager, AccountManagerPersistenceError, @@ -807,10 +808,7 @@ describe('core AccountManager', () => { state: 'unconfirmed', }) expect(writes).toBe(1) - const enable = Reflect.get(manager, 'enableSavingRecovery') - expect(typeof enable).toBe('function') - if (typeof enable !== 'function') return - Reflect.apply(enable, manager, []) + manager.enableSavingRecovery() readable = true manager.getAccounts()[0]!.label = 'latest' manager.requestSaveToDisk() @@ -1184,6 +1182,127 @@ describe('core AccountManager', () => { } }) + it.each([ + undefined, + false, + true, + ] as const)('preserves strict=%s for a late flush after recovery when the next save hits ELOCKED', async (strict) => { + jest.useFakeTimers() + try { + const memory = createStore(stored) + let readable = false + let reads = 0 + let releaseWrite!: () => void + let markWriteStarted!: () => void + const writeStarted = new Promise((resolve) => { + markWriteStarted = resolve + }) + let blockRecoveryWrite = false + let firstWrite = true + let lateAttempts = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1 && !readable) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async (path, snapshot) => { + if (firstWrite) { + firstWrite = false + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, snapshot), + ) + throw new Error('response lost') + } + lateAttempts++ + throw new Error('ELOCKED') + }, + mutate: async (path, fn) => { + if (blockRecoveryWrite) { + blockRecoveryWrite = false + markWriteStarted() + await new Promise((resolve) => { + releaseWrite = resolve + }) + } + return memory.store.mutate(path, fn) + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + readable = true + blockRecoveryWrite = true + manager.getAccounts()[0]!.label = 'A' + manager.requestSaveToDisk() + const firstFlush = manager.flushSaveToDisk() + await writeStarted + manager.getAccounts()[0]!.label = 'B' + manager.requestSaveToDisk() + const lateFlush = ( + strict === undefined + ? manager.flushSaveToDisk() + : manager.flushSaveToDisk({ strict }) + ).then( + () => 'resolved', + (error: unknown) => String(error), + ) + releaseWrite() + await firstFlush + await jest.advanceTimersByTime(1000) + const outcome = await lateFlush + if (strict === true) expect(outcome).toContain('ELOCKED') + else expect(outcome).toBe('resolved') + expect(lateAttempts).toBe(1) + expect(memory.state()?.accounts[0]?.label).toBe('A') + await manager.stopSaving() + } finally { + jest.useRealTimers() + } + }) + + it.each([ + undefined, + false, + true, + ] as const)('rejects strict=%s flush on unconfirmed recovery read-back without another write', async (strict) => { + const memory = createStore(stored) + let reads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + reads++ + if (reads > 1) throw new Error('read unavailable') + return memory.store.load(path) + }, + saveMerged: async () => { + writes++ + throw new Error('write lost') + }, + }, + }) + manager.requestSaveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + const flush = + strict === undefined + ? manager.flushSaveToDisk() + : manager.flushSaveToDisk({ strict }) + await expect(flush).rejects.toMatchObject({ state: 'unconfirmed' }) + expect(writes).toBe(1) + expect(memory.mutations()).toBe(0) + await manager.dispose() + }) + it.each([ false, true, @@ -1269,54 +1388,128 @@ describe('core AccountManager', () => { }) it('dispose drains a failed active fence after rejecting its late work', async () => { - const memory = createStore(stored) - let releaseSave!: () => void - let markSaveStarted!: () => void - const saveStarted = new Promise((resolve) => { - markSaveStarted = resolve - }) - let reads = 0 - let writes = 0 - const manager = new AccountManager(undefined, stored, { - store: { - ...memory.store, - load: async (path) => { + const sourceUrl = + process.env.ANTIGRAVITY_MANAGER_DISPOSE_TEST_SOURCE ?? + new URL('./account-manager.ts', import.meta.url).href + const script = ` + const { AccountManager } = await import(${JSON.stringify(sourceUrl)}) + const initial = { + version: 4, + accounts: [{ refreshToken: 'r1', projectId: 'p1', addedAt: 1, lastUsed: 0 }], + activeIndex: 0, + } + let state = structuredClone(initial) + let reads = 0 + let writes = 0 + let mutations = 0 + let releaseSave + let markSaveStarted + const saveStarted = new Promise((resolve) => { markSaveStarted = resolve }) + const store = { + load: async () => { reads++ if (reads > 1) throw new Error('read unavailable') - return memory.store.load(path) + return state }, saveMerged: async () => { writes++ markSaveStarted() - await new Promise((resolve) => { - releaseSave = resolve - }) + await new Promise((resolve) => { releaseSave = resolve }) throw new Error('response lost') }, - }, - }) - manager.requestSaveToDisk() - const initialFlush = manager - .flushSaveToDisk({ strict: true }) - .catch((error: unknown) => error) - const fence = manager.flushAndStopSaving() - await saveStarted - manager.getAccounts()[0]!.label = 'late' - manager.requestSaveToDisk() - const lateFlush = manager.flushSaveToDisk().catch((error: unknown) => error) - const disposing = manager.dispose() - releaseSave() - expect(String(await initialFlush)).toContain('response lost') - expect(String(await lateFlush)).toContain('persistence has stopped') - await expect(fence).rejects.toMatchObject({ state: 'unconfirmed' }) - const observedFailure = Reflect.get(manager, 'failedFenceUnconfirmed') - // The old implementation spins in dispose after this failure; release it - // before asserting RED so the test process can exit. - if (!observedFailure) await manager.stopSaving() - await disposing - expect(observedFailure).toBe(true) - expect(writes).toBe(1) - expect(memory.mutations()).toBe(0) + mutate: async (_path, fn) => { + mutations++ + state = (await fn(state)) ?? state + return state + }, + clear: async () => { state = null }, + } + const outcome = (promise) => promise.then( + () => ({ status: 'fulfilled' }), + (error) => ({ status: 'rejected', state: error?.state, message: String(error) }), + ) + const manager = new AccountManager(undefined, initial, { store }) + manager.requestSaveToDisk() + const firstFlush = outcome(manager.flushSaveToDisk({ strict: true })) + const fence = outcome(manager.flushAndStopSaving()) + await saveStarted + manager.getAccounts()[0].label = 'late' + manager.requestSaveToDisk() + const lateFlush = outcome(manager.flushSaveToDisk()) + const disposing = outcome(manager.dispose()) + process.stdout.write('entered\\n') + releaseSave() + const [first, late, fenced, disposed] = await Promise.all([ + firstFlush, lateFlush, fence, disposing, + ]) + process.stdout.write(JSON.stringify({ first, late, fenced, disposed, writes, mutations }) + '\\n') + ` + const child = spawn(process.execPath, ['-e', script], { + cwd: process.cwd(), + env: { ...process.env, TMPDIR: '/private/tmp' }, + stdio: ['ignore', 'pipe', 'pipe'], + }) + let stdout = '' + let stderr = '' + let spawnError: Error | null = null + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', (chunk: string) => { + stdout += chunk + }) + child.stderr.on('data', (chunk: string) => { + stderr += chunk + }) + child.once('error', (error) => { + spawnError = error + }) + const closed = new Promise<{ + code: number | null + signal: NodeJS.Signals | null + }>((resolve) => { + child.once('close', (code, signal) => resolve({ code, signal })) + }) + let timedOut = false + const deadline = setTimeout(() => { + timedOut = true + child.kill('SIGKILL') + }, 3000) + try { + const exit = await closed + expect(timedOut).toBe(false) + expect(spawnError).toBeNull() + expect(exit).toEqual({ code: 0, signal: null }) + const lines = stdout.trim().split('\n') + expect(lines[0]).toBe('entered') + const result = JSON.parse(lines[1] ?? '') + expect(result.first).toMatchObject({ + status: 'rejected', + message: 'Error: response lost', + }) + expect(result.late).toMatchObject({ + status: 'rejected', + message: 'Error: Account manager persistence has stopped', + }) + expect(result.fenced).toMatchObject({ + status: 'rejected', + state: 'unconfirmed', + }) + expect(result.disposed).toEqual({ status: 'fulfilled' }) + expect(result.writes).toBe(1) + expect(result.mutations).toBe(0) + } catch (error) { + throw new Error( + `Disposal child failed (timedOut=${timedOut}, entered=${stdout.includes('entered')}, stderr=${stderr.slice(0, 1000)}): ${String(error)}`, + ) + } finally { + clearTimeout(deadline) + if (child.exitCode === null && child.signalCode === null) { + child.kill('SIGKILL') + } + await closed + child.stdout.destroy() + child.stderr.destroy() + } }) it('invalidates a started recovery on dispose and drains it across repeated calls', async () => { @@ -2266,6 +2459,209 @@ describe('core AccountManager', () => { expect(memory.mutations()).toBe(0) }) + it('waits for a failed direct save to reconcile before retiring the old manager', async () => { + const memory = createStore(stored) + let releaseRetry!: () => void + let retryStarted!: () => void + const retrying = new Promise((resolve) => { + retryStarted = resolve + }) + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + writes++ + throw new Error('ELOCKED') + }, + mutate: async (path, update) => { + retryStarted() + await new Promise((resolve) => { + releaseRetry = resolve + }) + writes++ + return memory.store.mutate(path, update) + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted before stop' + await expect(manager.saveToDisk()).rejects.toThrow('ELOCKED') + + const fence = manager.flushAndStopSaving() + await retrying + let stopped = false + const firstStop = manager.stopSaving().then(() => { + stopped = true + }) + const secondStop = manager.stopSaving() + await new Promise((resolve) => setImmediate(resolve)) + expect(stopped).toBe(false) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + + releaseRetry() + await Promise.all([fence, firstStop, secondStop]) + expect(memory.state()?.accounts[0]?.label).toBe('accepted before stop') + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external successor' } : account, + ), + })) + manager.getAccounts()[0]!.label = 'stale after stop' + manager.requestSaveToDisk() + await manager.dispose() + expect(writes).toBe(2) + expect(memory.state()?.accounts[0]?.label).toBe('external successor') + }) + + it('drains reconciliation enqueued by a fence after stop snapshots its queue', async () => { + const memory = createStore(stored) + let releaseRead!: () => void + let readStarted!: () => void + const reading = new Promise((resolve) => { + readStarted = resolve + }) + let releaseReconciliation!: () => void + let reconciliationStarted!: () => void + const reconciling = new Promise((resolve) => { + reconciliationStarted = resolve + }) + let loads = 0 + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + load: async (path) => { + loads++ + if (loads === 2) { + readStarted() + await new Promise((resolve) => { + releaseRead = resolve + }) + throw new Error('read unavailable') + } + return memory.store.load(path) + }, + saveMerged: async (path, next) => { + writes++ + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, next), + ) + throw new Error('response lost') + }, + mutate: async (path, update) => { + reconciliationStarted() + await new Promise((resolve) => { + releaseReconciliation = resolve + }) + writes++ + return memory.store.mutate(path, update) + }, + }, + }) + manager.getAccounts()[0]!.label = 'first accepted' + await expect(manager.saveToDisk()).rejects.toThrow('response lost') + manager.getAccounts()[1]!.label = 'second accepted' + const second = manager.saveToDisk() + await reading + const fence = manager.flushAndStopSaving() + let stopped = false + const stopping = manager.stopSaving().then(() => { + stopped = true + }) + releaseRead() + await expect(second).rejects.toMatchObject({ state: 'unconfirmed' }) + await reconciling + expect(stopped).toBe(false) + releaseReconciliation() + await Promise.all([fence, stopping]) + expect(writes).toBe(2) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'first accepted', + 'second accepted', + ]) + manager.getAccounts()[1]!.label = 'stale after stop' + manager.requestSaveToDisk() + await manager.dispose() + expect(writes).toBe(2) + }) + + it.each([ + 'domain retryable', + 'raw lock contention', + ] as const)('keeps admission closed when an accepted fence ends in %s after stop', async (failurePath) => { + const memory = createStore(stored) + let releaseFailure!: () => void + let failureStarted!: () => void + const failing = new Promise((resolve) => { + failureStarted = resolve + }) + let writes = 0 + const manager = new AccountManager(undefined, stored, { + store: { + ...memory.store, + saveMerged: async () => { + writes++ + if (failurePath === 'domain retryable') { + failureStarted() + await new Promise((resolve) => { + releaseFailure = resolve + }) + } + throw new Error('ELOCKED') + }, + mutate: async () => { + writes++ + failureStarted() + await new Promise((resolve) => { + releaseFailure = resolve + }) + throw new Error('ELOCKED') + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted intent' + const original = manager.saveToDisk() + if (failurePath === 'raw lock contention') { + await expect(original).rejects.toThrow('ELOCKED') + } + const fence = manager.flushAndStopSaving() + await failing + let stopped = false + const stopping = manager.stopSaving().then(() => { + stopped = true + }) + expect(stopped).toBe(false) + releaseFailure() + if (failurePath === 'domain retryable') { + await expect(original).rejects.toThrow('ELOCKED') + } + await expect(fence).rejects.toMatchObject({ state: 'retryable' }) + await stopping + expect(stopped).toBe(true) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await expect(manager.saveToDiskReplace()).rejects.toThrow( + 'persistence has stopped', + ) + await expect(manager.flushSaveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + await expect(manager.flushAndStopSaving()).rejects.toThrow( + 'persistence has stopped', + ) + manager.requestSaveToDisk() + await manager.dispose() + expect(writes).toBe(failurePath === 'domain retryable' ? 1 : 2) + expect(memory.state()?.accounts[0]?.label).toBeUndefined() + }) + it('preserves non-fatal lock contention for ordinary flushes and rejects strict transition flushes', async () => { jest.useFakeTimers() try { @@ -2279,11 +2675,17 @@ describe('core AccountManager', () => { }, }) manager.requestSaveToDisk() - const ordinary = manager.flushSaveToDisk() - const strict = manager.flushSaveToDisk({ strict: true }) + const ordinary = manager.flushSaveToDisk().then( + () => 'resolved', + (error: unknown) => String(error), + ) + const strict = manager.flushSaveToDisk({ strict: true }).then( + () => 'resolved', + (error: unknown) => String(error), + ) await jest.advanceTimersByTime(1000) - await expect(ordinary).resolves.toBeUndefined() - await expect(strict).rejects.toThrow('ELOCKED') + expect(await ordinary).toBe('resolved') + expect(await strict).toContain('ELOCKED') await expect(manager.flushSaveToDisk()).resolves.toBeUndefined() await expect(manager.flushSaveToDisk({ strict: true })).rejects.toThrow( 'ELOCKED', diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index c69f4a0e..88b835a1 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -393,6 +393,7 @@ export class AccountManager { private savingRecoveryGeneration = 0 private savingRecoveryInFlight: Promise | null = null private saveQueue: Promise = Promise.resolve() + private stopInFlight: Promise | null = null private acceptedSaves = new Set>() private acceptedSaveFailure: unknown = null private pendingSaveIntents: Array<{ @@ -2174,7 +2175,7 @@ export class AccountManager { const requestedVersion = this.saveRequestVersion await this.startSavingRecovery() if (requestedVersion > this.recoveryConfirmedVersion) { - await this.flushSaveToDisk({ strict: true }) + await this.flushSaveToDisk(options) } return } @@ -2293,14 +2294,14 @@ export class AccountManager { } catch (error) { if (error instanceof AccountManagerPersistenceError) { if (error.state === 'retryable') { - this.storageAdmission = 'open' + if (!this.persistenceStopped) this.storageAdmission = 'open' } else { this.failedFenceUnconfirmed = true } throw error } if (isStorageLockContention(error)) { - this.storageAdmission = 'open' + if (!this.persistenceStopped) this.storageAdmission = 'open' throw new AccountManagerPersistenceError('retryable', error) } this.failedFenceUnconfirmed = true @@ -2314,34 +2315,56 @@ export class AccountManager { * The caller should flush intended state first, then await this method * before starting the mutation. */ - async stopSaving(): Promise { + stopSaving(): Promise { this.invalidateSavingRecovery() - if (this.persistenceStopped) { - await this.saveInFlight - await this.savingRecoveryInFlight?.catch(() => {}) + if (!this.persistenceStopped) { + this.persistenceStopped = true + this.storageAdmission = 'stopped' + this.disposed = true + if (this.saveTimeout) { + clearTimeout(this.saveTimeout) + this.saveTimeout = null + } this.savePending = false + const resolvers = this.savePromiseResolvers + this.savePromiseResolvers = [] + for (const { reject } of resolvers) { + reject(new Error('Account manager persistence has stopped')) + } + } + if (this.stopInFlight) return this.stopInFlight + const stopping = this.finishStopSaving() + this.stopInFlight = stopping + return stopping + } + + private async finishStopSaving(): Promise { + while (true) { + const inFlight = this.saveInFlight + const directSaves = [...this.directSaves] + const saveQueue = this.saveQueue + const fenceInFlight = this.fenceInFlight + const recoveryInFlight = this.savingRecoveryInFlight await Promise.all( - [...this.directSaves].map((save) => save.catch(() => {})), + [ + ...directSaves, + ...(inFlight ? [inFlight] : []), + saveQueue, + ...(fenceInFlight ? [fenceInFlight] : []), + ...(recoveryInFlight ? [recoveryInFlight] : []), + ].map((task) => task.catch(() => {})), ) - return - } - this.persistenceStopped = true - this.storageAdmission = 'stopped' - this.disposed = true - if (this.saveTimeout) { - clearTimeout(this.saveTimeout) - this.saveTimeout = null - } - this.savePending = false - const resolvers = this.savePromiseResolvers - this.savePromiseResolvers = [] - for (const { reject } of resolvers) { - reject(new Error('Account manager persistence has stopped')) + this.savePending = false + if ( + !this.saveInFlight && + this.directSaves.size === 0 && + this.saveQueue === saveQueue && + !this.fenceInFlight && + !this.savingRecoveryInFlight + ) { + return + } } - await this.saveInFlight - await this.savingRecoveryInFlight?.catch(() => {}) - this.savePending = false - await Promise.all([...this.directSaves].map((save) => save.catch(() => {}))) } dispose(): Promise { diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index c1ceccee..55f9eccd 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -148,31 +148,62 @@ export type OpenCodeV2DependencyOverrides = Partial type OAuthSuccess = Extract +interface OAuthAccountFields { + email?: string + refreshToken: string + projectId?: string + managedProjectId?: string + label?: string +} + +function parseOAuthAccountFields(result: OAuthSuccess): OAuthAccountFields { + const refreshParts = parseRefreshParts(result.refresh) + if (!refreshParts.refreshToken) { + throw new Error('Antigravity token exchange returned no refresh token') + } + return { + email: result.email, + refreshToken: refreshParts.refreshToken, + projectId: refreshParts.projectId || result.projectId || undefined, + managedProjectId: refreshParts.managedProjectId, + label: result.label, + } +} + +function isOAuthAccount( + account: AccountStorageV4['accounts'][number], + fields: OAuthAccountFields, +): boolean { + return fields.email + ? account.email === fields.email + : account.refreshToken === fields.refreshToken +} + +function findOAuthAccount( + accounts: AccountStorageV4['accounts'], + fields: OAuthAccountFields, +) { + return accounts.find((account) => isOAuthAccount(account, fields)) +} + export function upsertOAuthAccount( current: AccountStorageV4, result: OAuthSuccess, now: number, ): AccountStorageV4 { - const refreshParts = parseRefreshParts(result.refresh) - if (!refreshParts.refreshToken) { - throw new Error('Antigravity token exchange returned no refresh token') - } + const fields = parseOAuthAccountFields(result) const existingIndex = current.accounts.findIndex((account) => - result.email - ? account.email === result.email - : account.refreshToken === refreshParts.refreshToken, + isOAuthAccount(account, fields), ) const existing = existingIndex >= 0 ? current.accounts[existingIndex] : undefined const account = { ...existing, - email: result.email ?? existing?.email, - label: result.label ?? existing?.label, - refreshToken: refreshParts.refreshToken, - projectId: - refreshParts.projectId || result.projectId || existing?.projectId, - managedProjectId: - refreshParts.managedProjectId ?? existing?.managedProjectId, + email: fields.email ?? existing?.email, + label: fields.label ?? existing?.label, + refreshToken: fields.refreshToken, + projectId: fields.projectId ?? existing?.projectId, + managedProjectId: fields.managedProjectId ?? existing?.managedProjectId, addedAt: existing?.addedAt ?? now, lastUsed: now, enabled: true, @@ -694,26 +725,26 @@ export function createOpenCodeV2AntigravityPlugin( const findConfirmedOAuthAccount = ( stored: AccountStorageV4 | null, - result: OAuthSuccess, + fields: OAuthAccountFields, ) => { - const refreshToken = parseRefreshParts(result.refresh).refreshToken - return stored?.accounts.find((account) => - result.email - ? account.email === result.email - : account.refreshToken === refreshToken, - ) + return stored ? findOAuthAccount(stored.accounts, fields) : undefined } const isConfirmedOAuthAccount = ( stored: AccountStorageV4 | null, result: OAuthSuccess, ): boolean => { - const refreshToken = parseRefreshParts(result.refresh).refreshToken - const saved = findConfirmedOAuthAccount(stored, result) + const fields = parseOAuthAccountFields(result) + const saved = findConfirmedOAuthAccount(stored, fields) return !!( saved && - saved.refreshToken === refreshToken && - saved.enabled !== false + saved.refreshToken === fields.refreshToken && + saved.enabled !== false && + (fields.projectId === undefined || + saved.projectId === fields.projectId) && + (fields.managedProjectId === undefined || + saved.managedProjectId === fields.managedProjectId) && + (fields.label == null || saved.label === fields.label) ) } diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index 8afbcb00..174578b7 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, spyOn, test } from 'bun:test' +import { describe, expect, jest, spyOn, test } from 'bun:test' import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { @@ -59,6 +59,53 @@ function readPool(path: string): AccountStorageV4 { return JSON.parse(readFileSync(path, 'utf8')) as AccountStorageV4 } +type OAuthMetadataField = 'projectId' | 'managedProjectId' | 'label' + +function setOAuthMetadataField( + account: AccountStorageV4['accounts'][number], + field: OAuthMetadataField, + value: string, +): AccountStorageV4['accounts'][number] { + switch (field) { + case 'projectId': + return { ...account, projectId: value } + case 'managedProjectId': + return { ...account, managedProjectId: value } + case 'label': + return { ...account, label: value } + } +} + +const oauthMetadataCases = [ + { + field: 'projectId', + name: 'projectId', + refresh: 'same-token|requested-project', + projectId: 'fallback-project', + label: undefined, + expected: 'requested-project', + stale: 'old-project', + }, + { + field: 'managedProjectId', + name: 'managedProjectId', + refresh: 'same-token|old-project|requested-managed', + projectId: '', + label: undefined, + expected: 'requested-managed', + stale: 'old-managed', + }, + { + field: 'label', + name: 'label', + refresh: 'same-token|old-project|old-managed', + projectId: '', + label: '', + expected: '', + stale: 'old-label', + }, +] as const + async function setupPoolAdapter( overrides: Parameters[0], ) { @@ -536,9 +583,17 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { let unreadableLoadCalls = 0 let saveCalls = 0 let recoveryMutations = 0 + let recoveryMutationAttempts = 0 let storageReadable = true const persisted = deferred() const sent: string[] = [] + const t0 = Date.now() + const t1 = t0 + 5_000 + let restoreDateNow: (() => void) | undefined + let fakeTimersEnabled = false + let timersRestoredMocks = false + let persistenceDeadline: ReturnType | undefined + let restoreStoreMutate: (() => void) | undefined const captured = await setupPoolAdapter({ authorizeAntigravity: async () => ({ url: 'https://accounts.example/authorize?state=pool-state', @@ -602,18 +657,27 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } return saveAccountStorage(file, next) }) - const storeMutate = spyOn( + const initialStoreMutate = spyOn( defaultAccountStorageStore, 'mutate', ).mockImplementation(async (file, mutator, options) => { + recoveryMutationAttempts++ const result = await mutateAccountStorage(file, mutator, options) recoveryMutations++ - persisted.resolve() return result }) + restoreStoreMutate = () => initialStoreMutate.mockRestore() try { + const useFirstRequestTime = spyOn(Date, 'now').mockReturnValue(t0) + restoreDateNow = () => useFirstRequestTime.mockRestore() expect((await titleRequest(captured.hook)).ok).toBe(true) + restoreDateNow() + restoreDateNow = undefined expect(sent).toEqual(['old-a', 'token-b']) + + jest.useFakeTimers() + fakeTimersEnabled = true + jest.setSystemTime(t0) const callback = (await captured.oauth.authorize()).callback let fenceError: unknown try { @@ -641,23 +705,58 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { readPool(path).accounts[0]?.rateLimitResetTimes ?? {}, ).some((resetAt) => typeof resetAt === 'number'), ).toBe(true) + expect(readPool(path).accounts[1]?.lastUsed).toBe( + failedWrite === 'already-committed' ? t0 : 2, + ) const idleLoadCount = storeLoadCalls const idleSaveCount = saveCalls - await Bun.sleep(1050) + const idleRecoveryCount = recoveryMutations + const idleMutationAttemptCount = recoveryMutationAttempts + await jest.advanceTimersByTime(1_000) expect(storeLoadCalls).toBe(idleLoadCount) expect(saveCalls).toBe(idleSaveCount) + expect(recoveryMutations).toBe(idleRecoveryCount) + expect(recoveryMutationAttempts).toBe(idleMutationAttemptCount) + jest.useRealTimers() + fakeTimersEnabled = false + timersRestoredMocks = true + restoreStoreMutate = undefined storageReadable = true + const storeMutate = spyOn( + defaultAccountStorageStore, + 'mutate', + ).mockImplementation(async (file, mutator, options) => { + recoveryMutationAttempts++ + const result = await mutateAccountStorage(file, mutator, options) + recoveryMutations++ + if ( + result.accounts.some( + (account) => + account.email === 'b@example.test' && account.lastUsed === t1, + ) + ) + persisted.resolve() + return result + }) + restoreStoreMutate = () => storeMutate.mockRestore() + const useRecoveryRequestTime = spyOn(Date, 'now').mockReturnValue(t1) + restoreDateNow = () => useRecoveryRequestTime.mockRestore() expect((await titleRequest(captured.hook)).ok).toBe(true) expect(sent.slice(-1)).toEqual(['token-b']) - await Promise.race([ - persisted.promise, - Bun.sleep(3000).then(() => { - throw new Error('successful request did not recover account state') - }), - ]) - await Bun.sleep(10) + const completionDeadline = new Promise((_resolve, reject) => { + persistenceDeadline = setTimeout(() => { + reject( + new Error( + 'successful request did not commit the T1 account state', + ), + ) + }, 3_000) + }) + await Promise.race([persisted.promise, completionDeadline]) + restoreDateNow() + restoreDateNow = undefined expect(oauthMutations).toBe(0) if (failedWrite === 'already-committed') expect(recoveryMutations).toBe(1) @@ -668,11 +767,19 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { (resetAt) => typeof resetAt === 'number', ), ).toBe(true) - expect(recovered.accounts[1]?.lastUsed).toBeGreaterThan(2) + expect(recovered.accounts[1]?.lastUsed).toBe(t1) } finally { - storeLoad.mockRestore() - save.mockRestore() - storeMutate.mockRestore() + if (persistenceDeadline) clearTimeout(persistenceDeadline) + restoreDateNow?.() + if (fakeTimersEnabled) { + jest.useRealTimers() + timersRestoredMocks = true + } + if (!timersRestoredMocks) { + storeLoad.mockRestore() + save.mockRestore() + } + restoreStoreMutate?.() await captured.cleanup() } }) @@ -1663,6 +1770,376 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + for (const metadataCase of oauthMetadataCases) { + test(`rejects a no-op OAuth write when ${metadataCase.name} is not confirmed`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + projectId: 'old-project', + managedProjectId: 'old-managed', + label: 'old-label', + addedAt: 1, + lastUsed: 2, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 3, + lastUsed: 4, + enabled: true, + }, + ]) + const sent: string[] = [] + let oauthWrites = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: metadataCase.refresh, + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: metadataCase.projectId, + ...(metadataCase.label === undefined + ? {} + : { label: metadataCase.label }), + }), + mutateAccountStorage: async (file) => { + oauthWrites++ + // Return the actual storage state without running the proposed update. + return (await loadAccountStorage(file))! + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return auth.access === 'same-token' + ? new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).rejects.toThrow('not confirmed on disk') + expect(oauthWrites).toBe(1) + expect(readPool(path).accounts[0]?.refreshToken).toBe('same-token') + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.at(-1)).toBe('token-b') + } finally { + await captured.cleanup() + } + }) + + test(`rejects an overwritten OAuth write when ${metadataCase.name} is stale`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + projectId: 'old-project', + managedProjectId: 'old-managed', + label: 'old-label', + addedAt: 1, + lastUsed: 2, + enabled: true, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 3, + lastUsed: 4, + enabled: true, + }, + ]) + const sent: string[] = [] + let oauthWrites = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: metadataCase.refresh, + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: metadataCase.projectId, + ...(metadataCase.label === undefined + ? {} + : { label: metadataCase.label }), + }), + mutateAccountStorage: async (file, mutator) => { + oauthWrites++ + const committed = await mutateAccountStorage(file, mutator) + await mutateAccountStorage(file, (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.email === 'a@example.test' + ? setOAuthMetadataField( + account, + metadataCase.field, + metadataCase.stale, + ) + : account, + ), + })) + return committed + }, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return auth.access === 'same-token' + ? new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).rejects.toThrow('not confirmed on disk') + expect(oauthWrites).toBe(1) + expect(readPool(path).accounts[0]?.[metadataCase.field]).toBe( + metadataCase.stale, + ) + expect(readPool(path).accounts[0]?.refreshToken).toBe('same-token') + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent.at(-1)).toBe('token-b') + } finally { + await captured.cleanup() + } + }) + } + + test('confirms already-durable explicit OAuth fields without invoking the mutator', async () => { + const path = seedPool([ + { + email: 'existing@example.test', + refreshToken: 'same-token', + projectId: 'packed-project', + managedProjectId: 'managed-project', + label: '', + addedAt: 1, + lastUsed: 2, + fingerprint: { + deviceId: 'device-before', + sessionToken: 'session-before', + userAgent: buildAntigravityHarnessUserAgent(), + apiClient: 'antigravity-cli', + clientMetadata: { + ideType: 'ANTIGRAVITY', + platform: 'MACOS', + pluginType: 'ANTIGRAVITY', + }, + createdAt: 1, + }, + }, + ]) + let oauthWrites = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'same-token|packed-project|managed-project', + access: 'access', + expires: Date.now() + 600_000, + projectId: 'fallback-project', + label: '', + }), + mutateAccountStorage: async (file) => { + oauthWrites++ + // A storage adapter may report success without invoking the mutator. + return (await loadAccountStorage(file))! + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toMatchObject({ + refresh: 'same-token|packed-project|managed-project', + }) + expect(oauthWrites).toBe(1) + expect(readPool(path).accounts[0]).toMatchObject({ + email: 'existing@example.test', + refreshToken: 'same-token', + projectId: 'packed-project', + managedProjectId: 'managed-project', + label: '', + }) + expect(readPool(path).accounts[0]?.enabled).toBeUndefined() + } finally { + await captured.cleanup() + } + }) + + test('confirms explicit OAuth fields despite unrelated durable changes before readback', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + projectId: 'old-project', + managedProjectId: 'old-managed', + label: 'old-label', + addedAt: 1, + lastUsed: 2, + enabled: true, + rateLimitResetTimes: { gemini: 10 }, + fingerprint: { + deviceId: 'device-before', + sessionToken: 'session-before', + userAgent: buildAntigravityHarnessUserAgent(), + apiClient: 'antigravity-cli', + clientMetadata: { + ideType: 'ANTIGRAVITY', + platform: 'MACOS', + pluginType: 'ANTIGRAVITY', + }, + createdAt: 1, + }, + }, + ]) + let oauthWrites = 0 + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'same-token|packed-project|managed-project', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: 'fallback-project', + label: '', + }), + mutateAccountStorage: async (file, mutator) => { + oauthWrites++ + const committed = await mutateAccountStorage(file, mutator) + await mutateAccountStorage(file, (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.email === 'a@example.test' + ? { + ...account, + lastUsed: 999, + rateLimitResetTimes: { gemini: 20 }, + fingerprint: { + deviceId: 'device-after', + sessionToken: 'session-after', + userAgent: buildAntigravityHarnessUserAgent(), + apiClient: 'antigravity-cli', + clientMetadata: { + ideType: 'ANTIGRAVITY', + platform: 'MACOS', + pluginType: 'ANTIGRAVITY', + }, + createdAt: 999, + }, + } + : account, + ), + })) + return committed + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toMatchObject({ + refresh: 'same-token|packed-project|managed-project', + }) + expect(oauthWrites).toBe(1) + expect(readPool(path).accounts[0]).toMatchObject({ + refreshToken: 'same-token', + projectId: 'packed-project', + managedProjectId: 'managed-project', + label: '', + lastUsed: 999, + rateLimitResetTimes: { gemini: 20 }, + fingerprint: { deviceId: 'device-after' }, + }) + } finally { + await captured.cleanup() + } + }) + + test('inherits omitted and empty OAuth metadata without comparing inherited values', async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + projectId: 'kept-project', + managedProjectId: 'kept-managed', + label: 'kept-label', + addedAt: 1, + lastUsed: 2, + enabled: true, + }, + ]) + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'same-token||', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: '', + }), + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toMatchObject({ refresh: 'same-token|' }) + expect(readPool(path).accounts[0]).toMatchObject({ + projectId: 'kept-project', + managedProjectId: 'kept-managed', + label: 'kept-label', + }) + } finally { + await captured.cleanup() + } + }) + test('does not install a rejected OAuth readback or leave its fingerprint saver orphaned', async () => { const path = seedPool([ { From 666cef6d7dbd4341c34a0dddef9420b9ef38c8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Fri, 9 Oct 2026 10:37:17 +0800 Subject: [PATCH 6/7] bugfix(core): scope account pool retries after lock contention --- packages/core/src/account-manager.test.ts | 650 +++++++++++++++++++++- packages/core/src/account-manager.ts | 397 +++++++++++-- packages/opencode-v2/test/plugin.test.ts | 9 +- 3 files changed, 990 insertions(+), 66 deletions(-) diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index f0babd6c..8e9bfe85 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -1,12 +1,24 @@ import { describe, expect, it, jest } from 'bun:test' import { spawn } from 'node:child_process' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { AccountManager, AccountManagerPersistenceError, } from './account-manager.ts' import type { AccountStorageStore } from './account-storage.ts' -import { mergeAccountStorage } from './account-storage.ts' +import { + AccountStorageLockContentionError, + defaultAccountStorageStore, + loadAccountStorage, + mergeAccountStorage, + mutateAccountStorage, + saveAccountStorage, +} from './account-storage.ts' import type { AccountStorageV4 } from './account-types.ts' +import { acquireFencedFileLock } from './file-lock.ts' +import { generateFingerprint } from './fingerprint.ts' function createStore(initial: AccountStorageV4 | null = null) { let state = initial @@ -47,6 +59,624 @@ const stored: AccountStorageV4 = { } describe('core AccountManager', () => { + for (const externalChange of [ + 'metadata', + 'rotation', + 'removal', + 'addition', + ] as const) { + it(`preserves independent B ${externalChange} through typed retry and later saves`, async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let firstSave = true + let external!: AccountStorageV4 + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path, next) => { + if (firstSave) { + firstSave = false + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: + externalChange === 'removal' + ? current.accounts.slice(0, 1) + : externalChange === 'addition' + ? [ + ...current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'external B' } + : account, + ), + { + refreshToken: 'external-new', + addedAt: 2, + lastUsed: 0, + }, + ] + : current.accounts.map((account) => + account.refreshToken === 'r2' + ? { + ...account, + label: 'external B', + projectId: 'external project', + refreshToken: + externalChange === 'rotation' + ? 'rotated-B' + : account.refreshToken, + } + : account, + ), + activeIndex: externalChange === 'removal' ? 0 : 1, + activeIndexByFamily: { + claude: externalChange === 'removal' ? 0 : 1, + gemini: 0, + }, + })) + external = structuredClone(memory.state()!) + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + } + return memory.store.saveMerged(path, next) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.label).toBe('local A') + expect(memory.state()?.accounts.slice(1)).toEqual( + external.accounts.slice(1), + ) + expect(memory.state()?.activeIndexByFamily).toEqual( + external.activeIndexByFamily, + ) + manager.getAccounts()[0]!.lastUsed = 321 + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.lastUsed).toBe(321) + expect(memory.state()?.accounts.slice(1)).toEqual( + external.accounts.slice(1), + ) + expect(memory.state()?.activeIndex).toBe(external.activeIndex) + await manager.flushAndStopSaving() + expect(memory.state()?.accounts.slice(1)).toEqual( + external.accounts.slice(1), + ) + }) + } + + it('retains omitted durable metadata on A and unchanged B during initialization maintenance', async () => { + const retained = { + cachedPerModelQuota: [ + { modelId: 'model', group: 'claude', remainingFraction: 0.5 }, + ], + coolingDownUntil: 123_456, + cooldownReason: 'network-error' as const, + lastSwitchReason: 'rotation' as const, + } + const initial = structuredClone(stored) + initial.accounts = initial.accounts.map((account) => ({ + ...account, + ...structuredClone(retained), + })) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await manager.saveToDisk() + for (const account of memory.state()!.accounts) + expect(account).toMatchObject(retained) + manager.getAccounts()[0]!.lastUsed = 123 + await manager.saveToDisk() + for (const account of memory.state()!.accounts) + expect(account).toMatchObject(retained) + } finally { + await manager.stopSaving() + } + }) + + it('keeps B changes that precede the failed save baseline read', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let firstRead = true + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + load: async (path) => { + if (firstRead) { + firstRead = false + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { + ...account, + label: 'B before baseline', + fingerprint: generateFingerprint(), + } + : account, + ), + })) + } + return memory.store.load(path) + }, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + const externalB = structuredClone(memory.state()!.accounts[1]) + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.label).toBe('local A') + expect(memory.state()?.accounts[1]).toEqual(externalB) + await manager.stopSaving() + }) + + it('preserves B between a recovered intent and the queued newer intent', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let enter!: () => void + let release!: () => void + const entered = new Promise((resolve) => { + enter = resolve + }) + const gate = new Promise((resolve) => { + release = resolve + }) + let mutations = 0 + const observed: string[] = [] + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + enter() + await gate + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, fn) => { + mutations++ + if (mutations === 2) { + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'B between intents' } + : account, + ), + })) + } + const result = await memory.store.mutate(path, fn) + observed.push(result.accounts[0]!.label!) + return result + }, + }, + }) + manager.getAccounts()[0]!.label = 'first A' + const first = manager.saveToDisk() + void first.catch(() => {}) + await entered + manager.getAccounts()[0]!.label = 'queued A' + const queued = manager.saveToDisk() + void queued.catch(() => {}) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'B while locked' } + : account, + ), + })) + release() + await expect(first).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await queued + expect(observed).toEqual(['first A', 'queued A']) + expect(memory.state()?.accounts[1]?.label).toBe('B between intents') + await manager.flushAndStopSaving() + }) + + for (const conflict of ['metadata', 'rotation', 'removal'] as const) { + it(`keeps a typed retry unconfirmed when A has an external ${conflict}`, async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: + conflict === 'removal' + ? current.accounts.slice(1) + : current.accounts.map((account) => + account.refreshToken === 'r1' + ? { + ...account, + label: 'external A', + refreshToken: + conflict === 'rotation' + ? 'rotated-A' + : account.refreshToken, + } + : account, + ), + })) + const external = structuredClone(memory.state()) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()).toEqual(external) + await manager.stopSaving() + }) + } + + for (const structural of ['replace', 'rotation', 'selection'] as const) { + it(`guards a later ${structural} against preserved external B state`, async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'external B' } + : account, + ), + })) + await manager.saveToDisk() + const external = structuredClone(memory.state()) + if (structural === 'replace') manager.removeAccountByIndex(1) + if (structural === 'rotation') + manager.updateFromAuth(manager.getAccounts()[0]!, { + type: 'oauth', + refresh: 'new-A|p1', + }) + if (structural === 'selection') { + manager.getCurrentOrNextForFamily('claude', null, 'round-robin') + expect( + manager.getCurrentOrNextForFamily('claude', null, 'round-robin') + ?.index, + ).toBe(1) + } + await expect( + structural === 'replace' + ? manager.saveToDiskReplace() + : manager.saveToDisk(), + ).rejects.toMatchObject({ state: 'unconfirmed' }) + expect(memory.state()).toEqual(external) + await manager.stopSaving() + }) + } + + it('retains typed retries across repeated contention without advancing disk provenance', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let retryLocked = true + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, fn) => { + if (retryLocked) + throw new AccountStorageLockContentionError('retry contention', { + path, + attempts: 6, + }) + return memory.store.mutate(path, fn) + }, + }, + }) + manager.getAccounts()[0]!.label = 'first A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + manager.getAccounts()[0]!.label = 'newer A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + expect(memory.state()?.accounts[0]?.label).toBeUndefined() + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'external B' } + : account, + ), + })) + retryLocked = false + await manager.flushAndStopSaving() + expect(memory.state()?.accounts[0]?.label).toBe('newer A') + expect(memory.state()?.accounts[1]?.label).toBe('external B') + }) + + it('preserves B in the enabled recovery tail and after recovery reopens saves', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let rejectBeforeCommit = true + let scopedWrites = 0 + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, fn) => { + if (rejectBeforeCommit) { + rejectBeforeCommit = false + await fn(structuredClone(memory.state()!)) + throw new Error('commit unavailable') + } + scopedWrites++ + if (scopedWrites === 2) { + await memory.store.mutate(path, (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'B before recovery tail' } + : account, + ), + })) + } + return memory.store.mutate(path, fn) + }, + }, + }) + manager.getAccounts()[0]!.label = 'accepted A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.getAccounts()[0]!.label = 'late A' + manager.requestSaveToDisk() + await manager.flushSaveToDisk({ strict: true }) + expect(memory.state()?.accounts[0]?.label).toBe('late A') + expect(memory.state()?.accounts[1]?.label).toBe('B before recovery tail') + manager.getAccounts()[0]!.lastUsed = 456 + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.lastUsed).toBe(456) + expect(memory.state()?.accounts[1]?.label).toBe('B before recovery tail') + await manager.flushAndStopSaving() + }) + + for (const concurrentAfterWrite of [false, true]) { + it(`reconciles an ambiguous scoped write conservatively (externalAfterWrite=${concurrentAfterWrite})`, async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + let writes = 0 + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, fn) => { + writes++ + const result = await memory.store.mutate(path, fn) + if (writes === 1) throw new Error('response lost after write') + return result + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'external B' } + : account, + ), + })) + await expect(manager.saveToDisk()).rejects.toThrow( + 'response lost after write', + ) + expect(memory.state()?.accounts[0]?.label).toBe('local A') + if (concurrentAfterWrite) { + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account) => + account.refreshToken === 'r2' + ? { ...account, label: 'B changed after ambiguous write' } + : account, + ), + })) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(writes).toBe(1) + await manager.stopSaving() + } else { + await manager.saveToDisk() + manager.getAccounts()[0]!.lastUsed = 789 + await manager.saveToDisk() + expect(memory.state()?.accounts[0]?.lastUsed).toBe(789) + expect(memory.state()?.accounts[1]?.label).toBe('external B') + await manager.flushAndStopSaving() + } + }) + } + + for (const initialKind of ['fallback', 'invalid', 'duplicate'] as const) { + it(`retains the whole-pool guard for ${initialKind} initialization`, async () => { + const initial = structuredClone(stored) + if (initialKind === 'invalid') + initial.accounts.push({ refreshToken: '', addedAt: 1, lastUsed: 0 }) + if (initialKind === 'duplicate') + initial.accounts.push({ ...initial.accounts[0]! }) + const memory = createStore(initial) + const manager = new AccountManager( + initialKind === 'fallback' + ? { type: 'oauth', refresh: 'fallback|project' } + : undefined, + initial, + { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }, + ) + manager.getAccounts()[0]!.label = 'local A' + if (initialKind === 'duplicate') + manager.getAccounts()[2]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await manager.flushAndStopSaving() + expect(memory.state()?.accounts[0]?.label).toBe('local A') + const tokens = memory + .state() + ?.accounts.map((account) => account.refreshToken) + expect(tokens).toEqual( + initialKind === 'fallback' ? ['r1', 'r2', 'fallback'] : ['r1', 'r2'], + ) + }) + } + + it('retries a real pre-write file-lock failure without replacing B or its fingerprint', async () => { + const root = await mkdtemp(join(tmpdir(), 'agy-scoped-lock-')) + const path = join(root, 'accounts.json') + const initial = structuredClone(stored) + initial.accounts[0]!.fingerprint = generateFingerprint() + initial.accounts[0]!.fingerprint!.userAgent = 'old fingerprint version' + let first = true + let external!: AccountStorageV4 + let manager: AccountManager | undefined + try { + await saveAccountStorage(path, initial) + const loaded = await loadAccountStorage(path) + manager = new AccountManager(undefined, loaded, { + storagePath: path, + store: { + ...defaultAccountStorageStore, + saveMerged: async (target, snapshot) => { + if (!first) return saveAccountStorage(target, snapshot) + first = false + const held = await acquireFencedFileLock({ + path: target, + name: 'accounts', + ttlMs: 10_000, + renew: false, + }) + expect(held).not.toBeNull() + try { + external = JSON.parse( + await readFile(target, 'utf8'), + ) as AccountStorageV4 + external.accounts[1] = { + ...external.accounts[1]!, + label: 'real external B', + fingerprint: generateFingerprint(), + } + await writeFile(target, JSON.stringify(external)) + return await mutateAccountStorage( + target, + (current) => mergeAccountStorage(current, snapshot), + { sleep: async () => {} }, + ) + } finally { + await held!.release() + } + }, + }, + }) + manager.getAccounts()[0]!.label = 'real local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await manager.saveToDisk() + const result = await loadAccountStorage(path) + expect(result?.accounts[0]?.label).toBe('real local A') + expect(result?.accounts[0]?.fingerprint?.userAgent).toBe( + manager.getAccounts()[0]?.fingerprint?.userAgent, + ) + expect(result?.accounts[1]).toEqual(external.accounts[1]) + manager.getAccounts()[0]!.lastUsed = 123 + await manager.saveToDisk() + await manager.flushAndStopSaving() + expect((await loadAccountStorage(path))?.accounts[1]).toEqual( + external.accounts[1], + ) + } finally { + await manager?.stopSaving() + await rm(root, { recursive: true, force: true }) + } + }) + it('constructs from stored and fallback auth', () => { const memory = createStore(stored) const manager = new AccountManager( @@ -894,6 +1524,10 @@ describe('core AccountManager', () => { let readable = false let reads = 0 let writes = 0 + let observeRecovery!: () => void + const recoveryObserved = new Promise((resolve) => { + observeRecovery = resolve + }) const manager = new AccountManager(undefined, stored, { store: { ...memory.store, @@ -902,6 +1536,11 @@ describe('core AccountManager', () => { if (reads > 1 && !readable) throw new Error('read unavailable') return memory.store.load(path) }, + mutate: async (path, fn) => { + const saved = await memory.store.mutate(path, fn) + observeRecovery() + return saved + }, saveMerged: async (path, snapshot) => { writes++ if (writes === 1) throw new Error('write lost') @@ -918,6 +1557,8 @@ describe('core AccountManager', () => { manager.getAccounts()[0]!.label = 'latest' manager.requestSaveToDisk() await jest.advanceTimersByTime(1000) + await recoveryObserved + await manager.flushSaveToDisk({ strict: true }) await manager.saveToDisk() expect(memory.state()?.accounts[0]?.label).toBe('latest') } finally { @@ -1059,6 +1700,10 @@ describe('core AccountManager', () => { markWriteStarted = resolve }) let blockRecoveryWrite = false + let observeLateWrite!: () => void + const lateWriteObserved = new Promise((resolve) => { + observeLateWrite = resolve + }) const manager = new AccountManager(undefined, stored, { store: { ...memory.store, @@ -1071,6 +1716,7 @@ describe('core AccountManager', () => { await memory.store.mutate(path, (current) => mergeAccountStorage(current, next), ) + if (next.accounts[0]?.label === 'late') observeLateWrite() throw new Error('response lost') }, mutate: async (path, fn) => { @@ -1103,6 +1749,7 @@ describe('core AccountManager', () => { await recovery expect(memory.state()?.accounts[0]?.label).toBe('captured') await jest.advanceTimersByTime(1000) + await lateWriteObserved expect(memory.state()?.accounts[0]?.label).toBe('late') } finally { jest.useRealTimers() @@ -1446,7 +2093,6 @@ describe('core AccountManager', () => { ` const child = spawn(process.execPath, ['-e', script], { cwd: process.cwd(), - env: { ...process.env, TMPDIR: '/private/tmp' }, stdio: ['ignore', 'pipe', 'pipe'], }) let stdout = '' diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 88b835a1..28e19ce9 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -64,6 +64,61 @@ function sameStoredState( ) } +type AccountChange = { + before: AccountMetadataV3 + after: AccountMetadataV3 + fields: Array +} + +type SaveIntent = { + kind: 'merge' | 'replace' + snapshot: AccountStorageV4 + changes: AccountChange[] + structural: boolean +} + +type SaveAttempt = SaveIntent & { + baseline: AccountStorageV4 | null | undefined + expected?: AccountStorageV4 + error: unknown + scoped?: boolean + wholePool?: boolean + appliedTokens?: string[] + appliedPointers?: boolean +} + +function sameDurableValue(left: unknown, right: unknown): boolean { + return isDeepStrictEqual( + JSON.parse(JSON.stringify({ value: left })), + JSON.parse(JSON.stringify({ value: right })), + ) +} + +function accountChange( + before: AccountMetadataV3, + after: AccountMetadataV3, +): AccountChange { + const fields = [...new Set([...Object.keys(before), ...Object.keys(after)])] + .map((key) => key as keyof AccountMetadataV3) + .filter((key) => !sameDurableValue(before[key], after[key])) + return { before, after, fields } +} + +function applyAccountChange( + current: AccountMetadataV3, + change: AccountChange, +): AccountMetadataV3 { + const incoming = { + ...current, + ...Object.fromEntries(change.fields.map((key) => [key, change.after[key]])), + } + // Retain the existing project, eligibility, quota-reset and last-used rules. + return mergeAccountStorage( + { version: 4, accounts: [current], activeIndex: 0 }, + { version: 4, accounts: [incoming], activeIndex: 0 }, + ).accounts[0]! +} + export class AccountManagerPersistenceError extends Error { constructor( readonly state: 'retryable' | 'unconfirmed', @@ -396,16 +451,16 @@ export class AccountManager { private stopInFlight: Promise | null = null private acceptedSaves = new Set>() private acceptedSaveFailure: unknown = null - private pendingSaveIntents: Array<{ - kind: 'merge' | 'replace' - snapshot: AccountStorageV4 - }> = [] - private failedSaveAttempt: { - kind: 'merge' | 'replace' - baseline: AccountStorageV4 | null | undefined - snapshot: AccountStorageV4 - error: unknown - } | null = null + private pendingSaveIntents: SaveIntent[] = [] + private failedSaveAttempt: SaveAttempt | null = null + // Local capture and confirmed disk provenance are deliberately separate: + // an unrelated account read from disk has not been incorporated into memory. + private localSaveCheckpoint!: AccountStorageV4 + private persistedOrigin: AccountStorageV4 | null = null + private initialMaintenance: AccountChange[] = [] + private initialPointerMaintenance: AccountStorageV4 | null = null + private initialStructurePending = false + private scopedPersistence = false private savePromiseResolvers: Array<{ resolve: () => void reject: (err: unknown) => void @@ -432,6 +487,8 @@ export class AccountManager { stored: AccountStorageV4 | null | undefined, options: AccountManagerOptions, ) { + // Constructor normalization can mutate fingerprints shared with `stored`. + this.persistedOrigin = structuredClone(stored ?? null) this.store = options.store this.storagePath = options.storagePath ?? '' this.onDiagnostic = options.onDiagnostic @@ -446,6 +503,7 @@ export class AccountManager { if (stored && stored.accounts.length === 0) { this.accounts = [] this.cursorByFamily = { claude: 0, gemini: 0 } + this.initializeSaveProvenance() return } @@ -567,6 +625,7 @@ export class AccountManager { } } + this.initializeSaveProvenance() return } @@ -593,6 +652,218 @@ export class AccountManager { this.currentAccountIndexByFamily.gemini = 0 } } + this.initializeSaveProvenance() + } + + private initializeSaveProvenance(): void { + this.localSaveCheckpoint = structuredClone(this.buildStorageSnapshot()) + const original = this.persistedOrigin + const originalTokens = original?.accounts.map((a) => a.refreshToken) ?? [] + const normalizedTokens = this.localSaveCheckpoint.accounts.map( + (a) => a.refreshToken, + ) + this.initialStructurePending = + !sameDurableValue(originalTokens, normalizedTokens) || + new Set(originalTokens).size !== originalTokens.length + for (const normalized of this.localSaveCheckpoint.accounts) { + const before = original?.accounts.find( + (a) => a.refreshToken === normalized.refreshToken, + ) + if (before) { + const change = accountChange(before, normalized) + // A field omitted by the manager projection is not a deletion intent. + // Ordinary merges preserve it from the authoritative stored account. + change.fields = change.fields.filter((key) => + Object.hasOwn(normalized, key), + ) + if (change.fields.length > 0) this.initialMaintenance.push(change) + } + } + if ( + original && + (!sameDurableValue( + original.activeIndex, + this.localSaveCheckpoint.activeIndex, + ) || + !sameDurableValue( + original.activeIndexByFamily, + this.localSaveCheckpoint.activeIndexByFamily, + )) + ) { + this.initialPointerMaintenance = this.localSaveCheckpoint + } + } + + private captureSaveIntent(kind: 'merge' | 'replace'): SaveIntent { + const snapshot = structuredClone(this.buildStorageSnapshot()) + const previous = this.localSaveCheckpoint + const changes: AccountChange[] = [] + for (const after of snapshot.accounts) { + const before = previous.accounts.find( + (a) => a.refreshToken === after.refreshToken, + ) + if (before) { + const change = accountChange(before, after) + if (change.fields.length > 0) changes.push(change) + } + } + const structural = + kind === 'replace' || + !sameDurableValue( + previous.accounts.map((a) => a.refreshToken), + snapshot.accounts.map((a) => a.refreshToken), + ) || + !sameDurableValue(previous.activeIndex, snapshot.activeIndex) || + !sameDurableValue( + previous.activeIndexByFamily, + snapshot.activeIndexByFamily, + ) + this.localSaveCheckpoint = snapshot + return { kind, snapshot, changes, structural } + } + + private confirmSavedAttempt( + attempt: SaveAttempt, + saved: AccountStorageV4, + ): void { + if (!attempt.scoped || attempt.wholePool) { + this.persistedOrigin = structuredClone(saved) + this.initialStructurePending = false + } else if (this.persistedOrigin) { + for (const token of attempt.appliedTokens ?? []) { + const index = this.persistedOrigin.accounts.findIndex( + (a) => a.refreshToken === token, + ) + const account = saved.accounts.find((a) => a.refreshToken === token) + if (index >= 0 && account) { + this.persistedOrigin.accounts[index] = structuredClone(account) + } + } + if (attempt.appliedPointers) { + this.persistedOrigin.activeIndex = saved.activeIndex + this.persistedOrigin.activeIndexByFamily = structuredClone( + saved.activeIndexByFamily, + ) + } + } + this.initialMaintenance = [] + this.initialPointerMaintenance = null + } + + private expectedSaveResult(attempt: SaveAttempt): AccountStorageV4 { + if (attempt.expected) return attempt.expected + return attempt.kind === 'merge' + ? mergeAccountStorage( + attempt.baseline ?? { version: 4, accounts: [], activeIndex: 0 }, + attempt.snapshot, + ) + : attempt.snapshot + } + + private async writeScopedIntent( + intent: SaveIntent, + ): Promise { + const attempt: SaveAttempt = { + ...intent, + baseline: undefined, + expected: undefined, + error: undefined, + scoped: true, + appliedPointers: false, + wholePool: intent.structural || this.initialStructurePending, + appliedTokens: [], + } + try { + const saved = await this.store.mutate(this.storagePath, (current) => { + attempt.baseline = structuredClone(current) + const origin = this.persistedOrigin ?? { + version: 4, + accounts: [], + activeIndex: 0, + } + let next: AccountStorageV4 + if (attempt.wholePool) { + if (!sameStoredState(current, origin)) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account pool changed before a structural save'), + ) + } + next = + intent.kind === 'merge' + ? mergeAccountStorage(current, intent.snapshot) + : intent.snapshot + } else { + // Check explicit intent before applying optional initialization work. + for (const change of intent.changes) { + const token = change.before.refreshToken + const expected = origin.accounts.filter( + (a) => a.refreshToken === token, + ) + const actual = current.accounts.filter( + (a) => a.refreshToken === token, + ) + if ( + expected.length !== 1 || + actual.length !== 1 || + !sameDurableValue(actual[0], expected[0]) + ) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account changed before a deferred merge'), + ) + } + } + next = structuredClone(current) + for (const maintenance of this.initialMaintenance) { + const index = next.accounts.findIndex( + (a) => a.refreshToken === maintenance.before.refreshToken, + ) + if ( + index >= 0 && + sameDurableValue(current.accounts[index], maintenance.before) + ) { + next.accounts[index] = applyAccountChange( + next.accounts[index]!, + maintenance, + ) + attempt.appliedTokens!.push(maintenance.before.refreshToken) + } + } + for (const change of intent.changes) { + const index = next.accounts.findIndex( + (a) => a.refreshToken === change.before.refreshToken, + ) + next.accounts[index] = applyAccountChange( + next.accounts[index]!, + change, + ) + attempt.appliedTokens!.push(change.before.refreshToken) + } + if ( + this.initialPointerMaintenance && + sameStoredState(current, origin) + ) { + next.activeIndex = this.initialPointerMaintenance.activeIndex + next.activeIndexByFamily = structuredClone( + this.initialPointerMaintenance.activeIndexByFamily, + ) + attempt.appliedPointers = true + } + } + attempt.expected = structuredClone(next) + return next + }) + this.confirmSavedAttempt(attempt, saved) + this.failedSaveAttempt = null + this.saveFailure = null + this.acceptedSaveFailure = null + return structuredClone(saved) + } catch (error) { + attempt.error = error + this.failedSaveAttempt = attempt + throw error + } } getAccountCount(): number { @@ -1804,10 +2075,7 @@ export class AccountManager { } private enqueueSave(kind: 'merge' | 'replace'): Promise { - const intent = { - kind, - snapshot: structuredClone(this.buildStorageSnapshot()), - } + const intent = this.captureSaveIntent(kind) const save = this.saveQueue.then(async () => { // Retain this intent before reconciling older work: reconciliation // can fail without ever attempting this new snapshot. Capture happened @@ -1850,13 +2118,7 @@ export class AccountManager { if (result === 'missing') { guardedBaseline = await this.retryFailedSave() } else if (result === 'saved') { - guardedBaseline = - failed.kind === 'merge' - ? mergeAccountStorage( - failed.baseline ?? { version: 4, accounts: [], activeIndex: 0 }, - failed.snapshot, - ) - : failed.snapshot + guardedBaseline = this.expectedSaveResult(failed) } } while (this.pendingSaveIntents.length > 0) { @@ -1877,7 +2139,9 @@ export class AccountManager { assertActive?.() try { let saved: AccountStorageV4 - if (guardedBaseline !== undefined) { + if (this.scopedPersistence) { + saved = await this.writeScopedIntent(intent) + } else if (guardedBaseline !== undefined) { const expected = guardedBaseline saved = await this.store.mutate(this.storagePath, (current) => { if (!sameStoredState(current, expected)) { @@ -1898,6 +2162,12 @@ export class AccountManager { return snapshot }) } + if (!this.scopedPersistence) { + this.confirmSavedAttempt( + { ...intent, baseline, error: undefined }, + saved, + ) + } this.pendingSaveIntents.shift() // Further retained intents must not overwrite changes made between // recovery and their lock-held write. @@ -1905,7 +2175,12 @@ export class AccountManager { this.acceptedSaveFailure = null } catch (error) { this.pendingSaveIntents.shift() - this.failedSaveAttempt = { kind, baseline, snapshot, error } + if (!this.scopedPersistence) { + this.failedSaveAttempt = { ...intent, baseline, error } + } + if (error instanceof AccountStorageLockContentionError) { + this.scopedPersistence = true + } throw error } } @@ -1924,17 +2199,15 @@ export class AccountManager { } catch (error) { throw new AccountManagerPersistenceError('unconfirmed', error) } - if (attempt.baseline === undefined) { + if ( + attempt.baseline === undefined || + (attempt.scoped && !attempt.expected) + ) { throw new AccountManagerPersistenceError('unconfirmed', attempt.error) } - const expected = - attempt.kind === 'merge' - ? mergeAccountStorage( - attempt.baseline ?? { version: 4, accounts: [], activeIndex: 0 }, - attempt.snapshot, - ) - : attempt.snapshot + const expected = this.expectedSaveResult(attempt) if (sameStoredState(current, expected)) { + this.confirmSavedAttempt(attempt, expected) this.failedSaveAttempt = null this.acceptedSaveFailure = null return 'saved' @@ -1949,6 +2222,13 @@ export class AccountManager { private async retryFailedSave(): Promise { const attempt = this.failedSaveAttempt if (!attempt) return + if ( + attempt.scoped || + attempt.error instanceof AccountStorageLockContentionError + ) { + this.scopedPersistence = true + return this.writeScopedIntent(attempt) + } if (attempt.baseline === undefined) { throw new AccountManagerPersistenceError('unconfirmed', attempt.error) } @@ -1967,6 +2247,7 @@ export class AccountManager { ? mergeAccountStorage(current, attempt.snapshot) : attempt.snapshot }) + this.confirmSavedAttempt(attempt, saved) this.failedSaveAttempt = null this.saveFailure = null this.acceptedSaveFailure = null @@ -2096,25 +2377,37 @@ export class AccountManager { ) } if (this.savePending) { - const snapshot = structuredClone(this.buildStorageSnapshot()) + const intent = this.captureSaveIntent('merge') + const { snapshot } = intent const capturedVersion = this.saveRequestVersion this.savePending = false try { - await this.store.mutate(this.storagePath, (current) => { - if (!sameStoredState(current, baseline)) { - throw new AccountManagerPersistenceError( - 'unconfirmed', - new Error('Account state changed before a recovery save'), - ) - } - return mergeAccountStorage(current, snapshot) - }) + if (this.scopedPersistence) { + await this.writeScopedIntent(intent) + } else { + const saved = await this.store.mutate( + this.storagePath, + (current) => { + if (!sameStoredState(current, baseline)) { + throw new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account state changed before a recovery save'), + ) + } + return mergeAccountStorage(current, snapshot) + }, + ) + this.confirmSavedAttempt( + { ...intent, baseline, error: undefined }, + saved, + ) + } } catch (error) { - this.failedSaveAttempt = { - kind: 'merge', - baseline, - snapshot, - error, + if (!this.scopedPersistence) { + this.failedSaveAttempt = { ...intent, baseline, error } + } + if (error instanceof AccountStorageLockContentionError) { + this.scopedPersistence = true } this.savePending = true this.failedRecoverySnapshot = { @@ -2253,17 +2546,7 @@ export class AccountManager { failed && this.pendingSaveIntents.length > 0 ) { - const recovered = - failed.kind === 'merge' - ? mergeAccountStorage( - failed.baseline ?? { - version: 4, - accounts: [], - activeIndex: 0, - }, - failed.snapshot, - ) - : failed.snapshot + const recovered = this.expectedSaveResult(failed) await this.enqueueFailedSaveReconciliation(recovered) continue } diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index 174578b7..19051a04 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -591,7 +591,6 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { const t1 = t0 + 5_000 let restoreDateNow: (() => void) | undefined let fakeTimersEnabled = false - let timersRestoredMocks = false let persistenceDeadline: ReturnType | undefined let restoreStoreMutate: (() => void) | undefined const captured = await setupPoolAdapter({ @@ -721,7 +720,6 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { jest.useRealTimers() fakeTimersEnabled = false - timersRestoredMocks = true restoreStoreMutate = undefined storageReadable = true const storeMutate = spyOn( @@ -773,12 +771,9 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { restoreDateNow?.() if (fakeTimersEnabled) { jest.useRealTimers() - timersRestoredMocks = true - } - if (!timersRestoredMocks) { - storeLoad.mockRestore() - save.mockRestore() } + storeLoad.mockRestore() + save.mockRestore() restoreStoreMutate?.() await captured.cleanup() } From 98220b4b5adb2f40560efd9e36f544400581e713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=99=9E=E6=89=AC=28=E6=8A=80=E6=9C=AF=E4=BA=A7=E5=93=81?= =?UTF-8?q?=E4=B8=AD=E5=BF=83/=E7=AE=97=E6=B3=95=E9=83=A8=29?= Date: Fri, 9 Oct 2026 12:37:46 +0800 Subject: [PATCH 7/7] bugfix(auth): preserve ordered saves and confirm OAuth state --- packages/core/src/account-manager.test.ts | 615 +++++++++++++++++- packages/core/src/account-manager.ts | 430 +++++++----- packages/opencode-v2/src/plugin.ts | 2 + packages/opencode-v2/test/plugin.test.ts | 120 ++++ .../src/plugin/accounts-storage-race.test.ts | 170 +++++ packages/opencode/src/plugin/accounts.test.ts | 146 ++++- packages/opencode/src/plugin/accounts.ts | 23 +- packages/opencode/src/plugin/storage.test.ts | 113 ++++ packages/opencode/src/plugin/storage.ts | 38 +- 9 files changed, 1449 insertions(+), 208 deletions(-) create mode 100644 packages/opencode/src/plugin/accounts-storage-race.test.ts diff --git a/packages/core/src/account-manager.test.ts b/packages/core/src/account-manager.test.ts index 8e9bfe85..8ee2a944 100644 --- a/packages/core/src/account-manager.test.ts +++ b/packages/core/src/account-manager.test.ts @@ -59,6 +59,618 @@ const stored: AccountStorageV4 = { } describe('core AccountManager', () => { + it('lets independent B save while A has a known prewrite conflict, then recovers A after restoration', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.getAccounts()[1]!.label = 'local B' + await manager.saveToDisk() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + 'local B', + ]) + expect(memory.state()?.accounts[0]?.fingerprint).toBeUndefined() + expect(memory.state()?.accounts[1]?.fingerprint).toBeDefined() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: undefined } : account, + ), + })) + await manager.flushAndStopSaving() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'local A', + 'local B', + ]) + expect(memory.state()?.accounts[0]?.fingerprint).toBeDefined() + } finally { + await manager.stopSaving() + } + }) + + it('retains two account conflicts while C saves and preserves later same-account order', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'first A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index < 2 ? { ...account, label: `external ${index}` } : account, + ), + })) + manager.getAccounts()[1]!.label = 'local B' + await expect(manager.saveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.getAccounts()[2]!.label = 'local C' + await manager.saveToDisk() + manager.getAccounts()[0]!.label = 'later A' + await expect(manager.saveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external 0', + 'external 1', + 'local C', + ]) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index < 2 ? structuredClone(initial.accounts[index]!) : account, + ), + })) + await manager.flushAndStopSaving() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'later A', + 'local B', + 'local C', + ]) + } finally { + await manager.stopSaving() + } + }) + + it('retains a skipped later A intent when independent B succeeds', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const observedA: Array = [] + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, update) => { + const saved = await memory.store.mutate(path, update) + observedA.push(saved.accounts[0]?.label) + return saved + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'first A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.getAccounts()[0]!.label = 'later A' + await expect(manager.saveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.getAccounts()[1]!.label = 'independent B' + await manager.saveToDisk() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + 'independent B', + ]) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? structuredClone(initial.accounts[0]!) : account, + ), + })) + await manager.flushAndStopSaving() + expect(observedA.slice(-2)).toEqual(['first A', 'later A']) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'later A', + 'independent B', + ]) + } finally { + await manager.stopSaving() + } + }) + + it('rejects a strict requested flush when B saves but A remains conflicted', async () => { + jest.useFakeTimers() + try { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.getAccounts()[1]!.label = 'local B' + manager.requestSaveToDisk() + const ordinary = manager.flushSaveToDisk() + const strict = manager.flushSaveToDisk({ strict: true }) + void strict.catch(() => {}) + await jest.advanceTimersByTime(1000) + await expect(ordinary).resolves.toBeUndefined() + await expect(strict).rejects.toMatchObject({ state: 'unconfirmed' }) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + 'local B', + ]) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await manager.dispose() + } finally { + jest.useRealTimers() + } + }) + + it('validates every account in a captured intent before applying any part of it', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'first A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await manager.saveToDisk() + const durableB = structuredClone(memory.state()!.accounts[1]!) + manager.getAccounts()[0]!.label = 'second A' + manager.getAccounts()[1]!.label = 'local B' + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 1 ? { ...account, label: 'external B' } : account, + ), + })) + await expect(manager.saveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'first A', + 'external B', + undefined, + ]) + manager.getAccounts()[2]!.label = 'local C' + await manager.saveToDisk() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'first A', + 'external B', + 'local C', + ]) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 1 ? structuredClone(durableB) : account, + ), + })) + await manager.flushAndStopSaving() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'second A', + 'local B', + 'local C', + ]) + } finally { + await manager.stopSaving() + } + }) + + it('keeps a replacement and its later merge behind an older account conflict', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.removeAccountByIndex(1) + await expect(manager.saveToDiskReplace()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.getAccounts()[1]!.label = 'later C' + await expect(manager.saveToDisk()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['r1', 'r2', 'r3']) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? structuredClone(initial.accounts[0]!) : account, + ), + })) + await manager.flushAndStopSaving() + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['r1', 'r3']) + expect(memory.state()?.accounts[1]?.label).toBe('later C') + } finally { + await manager.stopSaving() + } + }) + + it('treats a wrapped prewrite validation error as ambiguous and blocks B', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + mutate: async (path, update) => { + try { + return await memory.store.mutate(path, update) + } catch { + throw new Error('wrapped validation result') + } + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.getAccounts()[1]!.label = 'local B' + await expect(manager.saveToDisk()).rejects.toThrow( + 'wrapped validation result', + ) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + undefined, + ]) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + } finally { + await manager.stopSaving() + } + }) + + it('retains a conflicted A through recovery tail and disposal after B succeeds', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.getAccounts()[1]!.label = 'local B' + await manager.saveToDisk() + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.getAccounts()[2]!.label = 'pending C' + manager.requestSaveToDisk() + await expect( + manager.flushSaveToDisk({ strict: true }), + ).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + 'local B', + 'pending C', + ]) + await manager.dispose() + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'external A', + 'local B', + 'pending C', + ]) + await expect(manager.saveToDisk()).rejects.toThrow( + 'persistence has stopped', + ) + }) + + it('keeps a same-account recovery tail behind conflicted A until A is restored', async () => { + const initial = structuredClone(stored) + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'first A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.getAccounts()[0]!.label = 'later A' + manager.requestSaveToDisk() + await expect( + manager.flushSaveToDisk({ strict: true }), + ).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts[0]?.label).toBe('external A') + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? structuredClone(initial.accounts[0]!) : account, + ), + })) + await manager.flushSaveToDisk({ strict: true }) + expect(memory.state()?.accounts[0]?.label).toBe('later A') + await manager.flushAndStopSaving() + } finally { + await manager.stopSaving() + } + }) + + it('keeps an independent recovery tail behind a retained replacement', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path) => { + throw new AccountStorageLockContentionError('typed contention', { + path, + attempts: 6, + }) + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 0 ? { ...account, label: 'external A' } : account, + ), + })) + manager.removeAccountByIndex(1) + await expect(manager.saveToDiskReplace()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.getAccounts()[1]!.label = 'tail C' + manager.requestSaveToDisk() + await expect( + manager.flushSaveToDisk({ strict: true }), + ).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect( + memory.state()?.accounts.map((account) => account.refreshToken), + ).toEqual(['r1', 'r2', 'r3']) + expect(memory.state()?.accounts[2]?.label).toBeUndefined() + } finally { + await manager.dispose() + } + }) + + it('keeps a recovery tail behind ambiguous persistence until read-back resolves', async () => { + const initial: AccountStorageV4 = { + ...structuredClone(stored), + accounts: [ + ...structuredClone(stored.accounts), + { refreshToken: 'r3', addedAt: 1, lastUsed: 0 }, + ], + } + const memory = createStore(initial) + const manager = new AccountManager(undefined, initial, { + store: { + ...memory.store, + saveMerged: async (path, snapshot) => { + await memory.store.mutate(path, (current) => + mergeAccountStorage(current, snapshot), + ) + throw new Error('response lost') + }, + }, + }) + try { + manager.getAccounts()[0]!.label = 'local A' + await expect(manager.saveToDisk()).rejects.toThrow('response lost') + await memory.store.mutate('', (current) => ({ + ...current, + accounts: current.accounts.map((account, index) => + index === 1 ? { ...account, label: 'external B' } : account, + ), + })) + await expect(manager.flushAndStopSaving()).rejects.toMatchObject({ + state: 'unconfirmed', + }) + manager.enableSavingRecovery() + manager.getAccounts()[2]!.label = 'tail C' + manager.requestSaveToDisk() + await expect( + manager.flushSaveToDisk({ strict: true }), + ).rejects.toMatchObject({ + state: 'unconfirmed', + }) + expect(memory.state()?.accounts.map((account) => account.label)).toEqual([ + 'local A', + 'external B', + undefined, + ]) + } finally { + await manager.dispose() + } + }) + for (const externalChange of [ 'metadata', 'rotation', @@ -2553,7 +3165,8 @@ describe('core AccountManager', () => { expect( memory.state()?.accounts.map((account) => account.refreshToken), ).toEqual(['rotated', 'r2']) - expect(memory.mutations()).toBe(2) + // Authoritative read-back sees the rotation before a retry is attempted. + expect(memory.mutations()).toBe(1) }) for (const kind of ['merge', 'replace'] as const) { diff --git a/packages/core/src/account-manager.ts b/packages/core/src/account-manager.ts index 28e19ce9..3510918b 100644 --- a/packages/core/src/account-manager.ts +++ b/packages/core/src/account-manager.ts @@ -71,6 +71,7 @@ type AccountChange = { } type SaveIntent = { + sequence: number kind: 'merge' | 'replace' snapshot: AccountStorageV4 changes: AccountChange[] @@ -85,6 +86,8 @@ type SaveAttempt = SaveIntent & { wholePool?: boolean appliedTokens?: string[] appliedPointers?: boolean + prewriteValidationError?: unknown + knownNotWritten?: boolean } function sameDurableValue(left: unknown, right: unknown): boolean { @@ -430,10 +433,7 @@ export class AccountManager { private savePending = false private saveRequestVersion = 0 private recoveryConfirmedVersion = 0 - private failedRecoverySnapshot: { - snapshot: AccountStorageV4 - requestVersion: number - } | null = null + private recoveryTailVersions = new Map() private saveTimeout: ReturnType | null = null private saveInFlight: Promise | null = null private directSaves = new Set>() @@ -452,7 +452,8 @@ export class AccountManager { private acceptedSaves = new Set>() private acceptedSaveFailure: unknown = null private pendingSaveIntents: SaveIntent[] = [] - private failedSaveAttempt: SaveAttempt | null = null + private failedSaveAttempts: SaveAttempt[] = [] + private nextSaveSequence = 0 // Local capture and confirmed disk provenance are deliberately separate: // an unrelated account read from disk has not been incorporated into memory. private localSaveCheckpoint!: AccountStorageV4 @@ -719,7 +720,13 @@ export class AccountManager { snapshot.activeIndexByFamily, ) this.localSaveCheckpoint = snapshot - return { kind, snapshot, changes, structural } + return { + sequence: ++this.nextSaveSequence, + kind, + snapshot, + changes, + structural, + } } private confirmSavedAttempt( @@ -746,8 +753,16 @@ export class AccountManager { ) } } - this.initialMaintenance = [] - this.initialPointerMaintenance = null + if (attempt.wholePool || !attempt.scoped) { + this.initialMaintenance = [] + this.initialPointerMaintenance = null + } else { + const applied = new Set(attempt.appliedTokens ?? []) + this.initialMaintenance = this.initialMaintenance.filter( + (change) => !applied.has(change.before.refreshToken), + ) + if (attempt.appliedPointers) this.initialPointerMaintenance = null + } } private expectedSaveResult(attempt: SaveAttempt): AccountStorageV4 { @@ -760,6 +775,44 @@ export class AccountManager { : attempt.snapshot } + private retainFailedAttempt(attempt: SaveAttempt, error: unknown): void { + attempt.error = error + attempt.knownNotWritten = + error instanceof AccountStorageLockContentionError || + (attempt.prewriteValidationError !== undefined && + error === attempt.prewriteValidationError) + const index = this.failedSaveAttempts.findIndex( + (failed) => failed.sequence === attempt.sequence, + ) + if (index >= 0) this.failedSaveAttempts[index] = attempt + else this.failedSaveAttempts.push(attempt) + this.failedSaveAttempts.sort( + (left, right) => left.sequence - right.sequence, + ) + } + + private confirmFailedAttempt(attempt: SaveAttempt): void { + this.failedSaveAttempts = this.failedSaveAttempts.filter( + (failed) => failed.sequence !== attempt.sequence, + ) + if (this.failedSaveAttempts.length === 0) { + this.saveFailure = null + this.acceptedSaveFailure = null + } + } + + private unresolvedFailure(): AccountManagerPersistenceError | null { + const attempt = this.failedSaveAttempts[0] + if (!attempt) return null + return new AccountManagerPersistenceError( + attempt.knownNotWritten && + attempt.error instanceof AccountStorageLockContentionError + ? 'retryable' + : 'unconfirmed', + attempt.error, + ) + } + private async writeScopedIntent( intent: SaveIntent, ): Promise { @@ -768,11 +821,17 @@ export class AccountManager { baseline: undefined, expected: undefined, error: undefined, + prewriteValidationError: undefined, + knownNotWritten: false, scoped: true, appliedPointers: false, wholePool: intent.structural || this.initialStructurePending, appliedTokens: [], } + const rejectPrewrite = (error: AccountManagerPersistenceError): never => { + attempt.prewriteValidationError = error + throw error + } try { const saved = await this.store.mutate(this.storagePath, (current) => { attempt.baseline = structuredClone(current) @@ -784,9 +843,11 @@ export class AccountManager { let next: AccountStorageV4 if (attempt.wholePool) { if (!sameStoredState(current, origin)) { - throw new AccountManagerPersistenceError( - 'unconfirmed', - new Error('Account pool changed before a structural save'), + return rejectPrewrite( + new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account pool changed before a structural save'), + ), ) } next = @@ -808,9 +869,11 @@ export class AccountManager { actual.length !== 1 || !sameDurableValue(actual[0], expected[0]) ) { - throw new AccountManagerPersistenceError( - 'unconfirmed', - new Error('Account changed before a deferred merge'), + return rejectPrewrite( + new AccountManagerPersistenceError( + 'unconfirmed', + new Error('Account changed before a deferred merge'), + ), ) } } @@ -855,13 +918,10 @@ export class AccountManager { return next }) this.confirmSavedAttempt(attempt, saved) - this.failedSaveAttempt = null - this.saveFailure = null - this.acceptedSaveFailure = null + this.confirmFailedAttempt(attempt) return structuredClone(saved) } catch (error) { - attempt.error = error - this.failedSaveAttempt = attempt + this.retainFailedAttempt(attempt, error) throw error } } @@ -2081,13 +2141,14 @@ export class AccountManager { // can fail without ever attempting this new snapshot. Capture happened // at invocation time, while queue insertion stays in serial order. this.pendingSaveIntents.push(intent) - await this.drainSaveIntents() + await this.drainSaveIntents(undefined, undefined, intent.sequence) }) this.acceptedSaves.add(save) this.directSaves.add(save) void save.then( () => { - this.acceptedSaveFailure = null + if (this.failedSaveAttempts.length === 0) + this.acceptedSaveFailure = null }, (error) => { this.acceptedSaveFailure = error @@ -2109,23 +2170,71 @@ export class AccountManager { private async drainSaveIntents( recoveredBaseline?: AccountStorageV4, assertActive?: () => void, + requestedSequence?: number, ): Promise { let guardedBaseline = recoveredBaseline - const failed = this.failedSaveAttempt - if (failed) { - const result = await this.readBackFailedSave() + const blockedTokens = new Set() + let structuralBarrier = false + const ordered = [ + ...this.failedSaveAttempts, + ...this.pendingSaveIntents, + ].sort((left, right) => left.sequence - right.sequence) + for (const intent of ordered) { assertActive?.() - if (result === 'missing') { - guardedBaseline = await this.retryFailedSave() - } else if (result === 'saved') { - guardedBaseline = this.expectedSaveResult(failed) + if (structuralBarrier) continue + const failed = this.failedSaveAttempts.find( + (attempt) => attempt.sequence === intent.sequence, + ) + if ( + intent.structural || + this.initialStructurePending || + intent.changes.some((change) => + blockedTokens.has(change.before.refreshToken), + ) + ) { + if (blockedTokens.size > 0) { + if (intent.structural || this.initialStructurePending) + structuralBarrier = true + continue + } } - } - while (this.pendingSaveIntents.length > 0) { - assertActive?.() - const intent = this.pendingSaveIntents[0]! + if (failed) { + try { + const result = await this.readBackFailedSave(failed) + assertActive?.() + if (result === 'missing') { + guardedBaseline = await this.retryFailedSave(failed) + } else if (result === 'saved') { + guardedBaseline = this.expectedSaveResult(failed) + } + } catch (error) { + const currentFailure = this.failedSaveAttempts.find( + (attempt) => attempt.sequence === intent.sequence, + ) + if ( + currentFailure?.knownNotWritten && + !currentFailure.structural && + !this.initialStructurePending + ) { + for (const change of currentFailure.changes) { + blockedTokens.add(change.before.refreshToken) + } + this.scopedPersistence = true + continue + } + throw error + } + continue + } + if ( + !this.pendingSaveIntents.some( + (pending) => pending.sequence === intent.sequence, + ) + ) + continue const { kind, snapshot } = intent let baseline: AccountStorageV4 | null | undefined = guardedBaseline + let prewriteValidationError: AccountManagerPersistenceError | undefined if (baseline === undefined) { // Preserve ordinary saves when baseline reads are unavailable. try { @@ -2145,10 +2254,11 @@ export class AccountManager { const expected = guardedBaseline saved = await this.store.mutate(this.storagePath, (current) => { if (!sameStoredState(current, expected)) { - throw new AccountManagerPersistenceError( + prewriteValidationError = new AccountManagerPersistenceError( 'unconfirmed', new Error('Account state changed before a deferred save'), ) + throw prewriteValidationError } return kind === 'merge' ? mergeAccountStorage(current, snapshot) @@ -2168,29 +2278,66 @@ export class AccountManager { saved, ) } - this.pendingSaveIntents.shift() + this.pendingSaveIntents = this.pendingSaveIntents.filter( + (pending) => pending.sequence !== intent.sequence, + ) // Further retained intents must not overwrite changes made between // recovery and their lock-held write. guardedBaseline = structuredClone(saved) - this.acceptedSaveFailure = null + if (this.failedSaveAttempts.length === 0) + this.acceptedSaveFailure = null } catch (error) { - this.pendingSaveIntents.shift() + this.pendingSaveIntents = this.pendingSaveIntents.filter( + (pending) => pending.sequence !== intent.sequence, + ) if (!this.scopedPersistence) { - this.failedSaveAttempt = { ...intent, baseline, error } + this.retainFailedAttempt( + { ...intent, baseline, error, prewriteValidationError }, + error, + ) } if (error instanceof AccountStorageLockContentionError) { this.scopedPersistence = true } + if (error === prewriteValidationError) this.scopedPersistence = true throw error } } + if ( + requestedSequence !== undefined && + this.failedSaveAttempts.some( + (attempt) => attempt.sequence === requestedSequence, + ) + ) { + const failure = this.failedSaveAttempts.find( + (attempt) => attempt.sequence === requestedSequence, + )! + if (failure.error instanceof AccountStorageLockContentionError) + throw failure.error + throw this.unresolvedFailure() + } + if ( + requestedSequence !== undefined && + this.pendingSaveIntents.some( + (pending) => pending.sequence === requestedSequence, + ) + ) { + const blocking = this.failedSaveAttempts[0] + if (blocking?.error instanceof AccountStorageLockContentionError) + throw blocking.error + throw ( + this.unresolvedFailure() ?? + new AccountManagerPersistenceError('unconfirmed', undefined) + ) + } return guardedBaseline } - private async readBackFailedSave(): Promise<'none' | 'saved' | 'missing'> { - const attempt = this.failedSaveAttempt + private async readBackFailedSave( + attempt = this.failedSaveAttempts[0], + ): Promise<'none' | 'saved' | 'missing'> { if (!attempt) return 'none' - if (isStorageLockContention(attempt.error)) { + if (attempt.knownNotWritten) { return 'missing' } let current: AccountStorageV4 | null @@ -2208,8 +2355,7 @@ export class AccountManager { const expected = this.expectedSaveResult(attempt) if (sameStoredState(current, expected)) { this.confirmSavedAttempt(attempt, expected) - this.failedSaveAttempt = null - this.acceptedSaveFailure = null + this.confirmFailedAttempt(attempt) return 'saved' } if (sameStoredState(current, attempt.baseline)) { @@ -2219,9 +2365,9 @@ export class AccountManager { } /** Retry only the exact failed intent under a lock-held state check. */ - private async retryFailedSave(): Promise { - const attempt = this.failedSaveAttempt - if (!attempt) return + private async retryFailedSave( + attempt: SaveAttempt, + ): Promise { if ( attempt.scoped || attempt.error instanceof AccountStorageLockContentionError @@ -2233,6 +2379,7 @@ export class AccountManager { throw new AccountManagerPersistenceError('unconfirmed', attempt.error) } const baseline = attempt.baseline + attempt.prewriteValidationError = undefined try { const saved = await this.store.mutate(this.storagePath, (current) => { if ( @@ -2241,20 +2388,24 @@ export class AccountManager { baseline ?? { version: 4, accounts: [], activeIndex: 0 }, ) ) { - throw new AccountManagerPersistenceError('unconfirmed', attempt.error) + const validationError = new AccountManagerPersistenceError( + 'unconfirmed', + attempt.error, + ) + attempt.prewriteValidationError = validationError + throw validationError } return attempt.kind === 'merge' ? mergeAccountStorage(current, attempt.snapshot) : attempt.snapshot }) this.confirmSavedAttempt(attempt, saved) - this.failedSaveAttempt = null - this.saveFailure = null - this.acceptedSaveFailure = null + this.confirmFailedAttempt(attempt) return structuredClone(saved) } catch (error) { - if (error instanceof AccountManagerPersistenceError) throw error - attempt.error = error + this.retainFailedAttempt(attempt, error) + if (error === attempt.prewriteValidationError) + this.scopedPersistence = true throw error } } @@ -2351,82 +2502,45 @@ export class AccountManager { const generation = this.savingRecoveryGeneration const recovery = this.saveQueue.then(async () => { this.assertSavingRecovery(generation) - // Resolve the exact accepted failure and every retained intent before - // considering memory that changed while admission was fenced. - const failedAttempt = this.failedSaveAttempt - const baseline = await this.drainSaveIntents(undefined, () => - this.assertSavingRecovery(generation), - ) - this.assertSavingRecovery(generation) - if ( - this.failedRecoverySnapshot && - failedAttempt?.snapshot === this.failedRecoverySnapshot.snapshot && - !this.failedSaveAttempt - ) { - if ( - this.saveRequestVersion === this.failedRecoverySnapshot.requestVersion - ) { - this.savePending = false - } - this.failedRecoverySnapshot = null + const capturedVersion = this.saveRequestVersion + // Capture the fenced request before reconciling older work. Its + // account scope decides whether it can progress past a conflict. + if (this.savePending) { + const intent = this.captureSaveIntent('merge') + this.recoveryTailVersions.set(intent.sequence, capturedVersion) + this.pendingSaveIntents.push(intent) + this.savePending = false } - if (baseline === undefined) { + let baseline: AccountStorageV4 | undefined + try { + baseline = await this.drainSaveIntents(undefined, () => + this.assertSavingRecovery(generation), + ) + } finally { + this.confirmRecoveryTailVersions() + } + this.assertSavingRecovery(generation) + const unresolved = this.unresolvedFailure() + if (unresolved) throw unresolved + if (this.pendingSaveIntents.length > 0) throw new AccountManagerPersistenceError( 'unconfirmed', this.acceptedSaveFailure, ) - } - if (this.savePending) { - const intent = this.captureSaveIntent('merge') - const { snapshot } = intent - const capturedVersion = this.saveRequestVersion - this.savePending = false - try { - if (this.scopedPersistence) { - await this.writeScopedIntent(intent) - } else { - const saved = await this.store.mutate( - this.storagePath, - (current) => { - if (!sameStoredState(current, baseline)) { - throw new AccountManagerPersistenceError( - 'unconfirmed', - new Error('Account state changed before a recovery save'), - ) - } - return mergeAccountStorage(current, snapshot) - }, - ) - this.confirmSavedAttempt( - { ...intent, baseline, error: undefined }, - saved, - ) - } - } catch (error) { - if (!this.scopedPersistence) { - this.failedSaveAttempt = { ...intent, baseline, error } - } - if (error instanceof AccountStorageLockContentionError) { - this.scopedPersistence = true - } - this.savePending = true - this.failedRecoverySnapshot = { - snapshot, - requestVersion: capturedVersion, - } - throw error - } - this.assertSavingRecovery(generation) - this.recoveryConfirmedVersion = capturedVersion - } else { - this.recoveryConfirmedVersion = this.saveRequestVersion - } + if (baseline === undefined) + throw new AccountManagerPersistenceError( + 'unconfirmed', + this.acceptedSaveFailure, + ) + this.recoveryConfirmedVersion = capturedVersion this.assertSavingRecovery(generation) this.storageAdmission = 'open' this.savingRecoveryEnabled = false this.failedFenceUnconfirmed = false - this.saveFailure = null - this.acceptedSaveFailure = null + if (this.failedSaveAttempts.length === 0) { + this.saveFailure = null + this.acceptedSaveFailure = null + } }) this.saveQueue = recovery.then( () => {}, @@ -2454,6 +2568,23 @@ export class AccountManager { return recovery } + private confirmRecoveryTailVersions(): void { + for (const [sequence, requestVersion] of this.recoveryTailVersions) { + if ( + this.pendingSaveIntents.some( + (intent) => intent.sequence === sequence, + ) || + this.failedSaveAttempts.some((attempt) => attempt.sequence === sequence) + ) + continue + this.recoveryConfirmedVersion = Math.max( + this.recoveryConfirmedVersion, + requestVersion, + ) + this.recoveryTailVersions.delete(sequence) + } + } + requestSaveToDisk(): void { if (this.disposed || this.persistenceStopped) { return @@ -2478,7 +2609,11 @@ export class AccountManager { this.schedulePendingSave() if (!this.savePending) { await this.saveInFlight - if (this.saveFailure && options.strict) throw this.saveFailure + if (options.strict) { + if (this.saveFailure) throw this.saveFailure + const unresolved = this.unresolvedFailure() + if (unresolved) throw unresolved + } return } return new Promise((resolve, reject) => { @@ -2517,16 +2652,10 @@ export class AccountManager { private async finishFlushAndStopSaving(): Promise { try { - const hasAcceptedWork = + const hadAcceptedWork = this.acceptedSaves.size > 0 || this.saveInFlight !== null || this.savePending - if ( - !hasAcceptedWork && - (this.failedSaveAttempt || this.pendingSaveIntents.length > 0) - ) { - await this.enqueueFailedSaveReconciliation() - } while (true) { if (this.saveTimeout) { clearTimeout(this.saveTimeout) @@ -2539,24 +2668,33 @@ export class AccountManager { } await this.saveQueue await this.saveInFlight - const failed = this.failedSaveAttempt - const readBack = await this.readBackFailedSave() + let recoveredBaseline: AccountStorageV4 | undefined + if (hadAcceptedWork && this.failedSaveAttempts.length > 0) { + for (const attempt of [...this.failedSaveAttempts]) { + const result = await this.readBackFailedSave(attempt) + if (result === 'saved') + recoveredBaseline = this.expectedSaveResult(attempt) + if (result === 'missing') { + if ( + attempt.knownNotWritten && + !(attempt.error instanceof AccountStorageLockContentionError) + ) + throw this.unresolvedFailure() + throw new AccountManagerPersistenceError( + 'retryable', + attempt.error, + ) + } + } + } if ( - readBack === 'saved' && - failed && + this.failedSaveAttempts.length > 0 || this.pendingSaveIntents.length > 0 - ) { - const recovered = this.expectedSaveResult(failed) - await this.enqueueFailedSaveReconciliation(recovered) - continue - } - if (readBack === 'missing') { - throw new AccountManagerPersistenceError( - 'retryable', - this.failedSaveAttempt?.error ?? this.acceptedSaveFailure, - ) - } - if (readBack === 'none' && this.acceptedSaveFailure) { + ) + await this.enqueueFailedSaveReconciliation(recoveredBaseline) + const unresolved = this.unresolvedFailure() + if (unresolved) throw unresolved + if (this.acceptedSaveFailure) { if ( this.acceptedSaveFailure instanceof AccountManagerPersistenceError ) { @@ -2721,9 +2859,11 @@ export class AccountManager { try { if (this.storageAdmission === 'open') await this.saveToDisk() else await this.enqueueSave('merge') - this.saveFailure = null - for (const { resolve } of resolvers) { - resolve() + if (this.failedSaveAttempts.length === 0) this.saveFailure = null + const unresolved = this.unresolvedFailure() + for (const waiter of resolvers) { + if (waiter.strict && unresolved) waiter.reject(unresolved) + else waiter.resolve() } } catch (error) { this.saveFailure = error diff --git a/packages/opencode-v2/src/plugin.ts b/packages/opencode-v2/src/plugin.ts index 55f9eccd..76223581 100644 --- a/packages/opencode-v2/src/plugin.ts +++ b/packages/opencode-v2/src/plugin.ts @@ -740,6 +740,8 @@ export function createOpenCodeV2AntigravityPlugin( saved && saved.refreshToken === fields.refreshToken && saved.enabled !== false && + saved.accountIneligible !== true && + saved.verificationRequired !== true && (fields.projectId === undefined || saved.projectId === fields.projectId) && (fields.managedProjectId === undefined || diff --git a/packages/opencode-v2/test/plugin.test.ts b/packages/opencode-v2/test/plugin.test.ts index 19051a04..4c1d0c2f 100644 --- a/packages/opencode-v2/test/plugin.test.ts +++ b/packages/opencode-v2/test/plugin.test.ts @@ -1765,6 +1765,126 @@ describe('opencode-v2-antigravity-auth plugin entry', () => { } }) + for (const blockedState of [ + { accountIneligible: true }, + { verificationRequired: true }, + ]) { + const flag = Object.keys(blockedState)[0] + test(`rejects a no-op OAuth readback with ${flag} still set`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + addedAt: 1, + lastUsed: 2, + enabled: true, + ...blockedState, + }, + { + email: 'b@example.test', + refreshToken: 'token-b', + addedAt: 3, + lastUsed: 4, + enabled: true, + }, + ]) + const sent: string[] = [] + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'same-token', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: '', + }), + mutateAccountStorage: async (file) => (await loadAccountStorage(file))!, + refreshAntigravityToken: async (refresh) => ({ + refresh, + access: refresh, + expires: Date.now() + 600_000, + }), + send: async ({ auth }) => { + sent.push(auth.access ?? '') + return auth.access === 'same-token' + ? new Response( + JSON.stringify({ + error: { status: 'ACCOUNT_INELIGIBLE', message: 'blocked' }, + }), + { status: 403 }, + ) + : successResponse() + }, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).rejects.toThrow('not confirmed on disk') + expect(readPool(path).accounts[0]).toMatchObject({ + enabled: true, + ...blockedState, + }) + expect((await titleRequest(captured.hook)).ok).toBe(true) + expect(sent).toEqual(['same-token', 'token-b']) + } finally { + await captured.cleanup() + } + }) + } + + for (const clearState of [ + {}, + { accountIneligible: false }, + { verificationRequired: false }, + ]) { + test(`confirms no-op OAuth readback with clear eligibility flags ${JSON.stringify(clearState)}`, async () => { + const path = seedPool([ + { + email: 'a@example.test', + refreshToken: 'same-token', + addedAt: 1, + lastUsed: 2, + enabled: true, + ...clearState, + }, + ]) + const captured = await setupPoolAdapter({ + authorizeAntigravity: async () => ({ + url: 'https://accounts.example/authorize?state=pool-state', + verifier: 'verifier', + projectId: '', + }), + waitForAntigravityCode: async () => 'code', + exchangeAntigravity: async () => ({ + type: 'success', + refresh: 'same-token', + access: 'access', + expires: Date.now() + 600_000, + email: 'a@example.test', + projectId: '', + }), + mutateAccountStorage: async (file) => (await loadAccountStorage(file))!, + }) + try { + await expect( + (await captured.oauth.authorize()).callback, + ).resolves.toMatchObject({ refresh: 'same-token|' }) + expect(readPool(path).accounts[0]).toMatchObject({ + enabled: true, + ...clearState, + }) + } finally { + await captured.cleanup() + } + }) + } + for (const metadataCase of oauthMetadataCases) { test(`rejects a no-op OAuth write when ${metadataCase.name} is not confirmed`, async () => { const path = seedPool([ diff --git a/packages/opencode/src/plugin/accounts-storage-race.test.ts b/packages/opencode/src/plugin/accounts-storage-race.test.ts new file mode 100644 index 00000000..fc31f474 --- /dev/null +++ b/packages/opencode/src/plugin/accounts-storage-race.test.ts @@ -0,0 +1,170 @@ +import { afterEach, expect, it, mock } from 'bun:test' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + AccountStorageLockContentionError, + type AccountStorageStore, + type AccountStorageV4, +} from '@cortexkit/antigravity-auth-core' + +const realStorage = await import('./storage') +const realCreateAccountStorageStore = realStorage.createAccountStorageStore +const realSaveAccountsReplace = realStorage.saveAccountsReplace + +function deferred() { + let resolve!: (value: T | PromiseLike) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +let storagePath = '' +let managerCallbackArmed = false +let managerCallbackEntered = deferred() +let releaseManagerCallback = deferred() +let legacyReplaceArmed = false +let legacyReplaceEntered = deferred() +let releaseLegacyReplace = deferred() + +mock.module('./storage', () => ({ + ...realStorage, + getStoragePath: () => storagePath, + saveAccountsReplace: async (next: AccountStorageV4) => { + if (legacyReplaceArmed) { + legacyReplaceEntered.resolve() + await releaseLegacyReplace.promise + } + return realSaveAccountsReplace(next) + }, + createAccountStorageStore: () => { + const store = realCreateAccountStorageStore() + const wrappedStore: AccountStorageStore = { + ...store, + mutate: (path, mutate, options) => + store.mutate( + path, + async (current) => { + if (managerCallbackArmed) { + managerCallbackArmed = false + managerCallbackEntered.resolve() + await releaseManagerCallback.promise + } + return mutate(current) + }, + options, + ), + } + return wrappedStore + }, +})) + +const { AccountManager } = await import('./accounts') + +afterEach(() => { + managerCallbackArmed = false + managerCallbackEntered = deferred() + releaseManagerCallback = deferred() + legacyReplaceArmed = false + legacyReplaceEntered = deferred() + releaseLegacyReplace = deferred() +}) + +it('keeps scoped manager and concurrent writer changes after the first lock contention', async () => { + const configDir = await mkdtemp(join(tmpdir(), 'antigravity-manager-race-')) + storagePath = join(configDir, 'antigravity-accounts.json') + const previousConfigDir = process.env.OPENCODE_CONFIG_DIR + process.env.OPENCODE_CONFIG_DIR = configDir + const realStore = realCreateAccountStorageStore() + const initial: AccountStorageV4 = { + version: 4, + accounts: [ + { refreshToken: 'a', addedAt: 1, lastUsed: 0 }, + { refreshToken: 'b', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + const lockEntered = deferred() + const releaseLock = deferred() + const manager = new AccountManager(undefined, initial, { storagePath }) + + try { + await realStore.mutate(storagePath, () => initial) + manager.getAccounts()[0]!.label = 'A updated' + + const initialLock = realStore.mutate(storagePath, async (current) => { + lockEntered.resolve() + await releaseLock.promise + return current + }) + await lockEntered.promise + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + + releaseLock.resolve() + await initialLock + managerCallbackArmed = true + legacyReplaceArmed = true + + const secondSaveDone = deferred() + const secondSave = manager + .saveToDisk() + .finally(() => secondSaveDone.resolve()) + + const progress = await Promise.race([ + managerCallbackEntered.promise.then(() => 'locked-callback' as const), + legacyReplaceEntered.promise.then(() => 'legacy-overwrite' as const), + ]) + + const externalContention = deferred() + const externalWrite = realStore.mutate( + storagePath, + (current) => { + current.accounts[1]!.label = 'B updated' + current.accounts.push({ + refreshToken: 'oauth-added', + addedAt: 2, + lastUsed: 0, + }) + return current + }, + { + sleep: async () => { + externalContention.resolve() + await secondSaveDone.promise + }, + }, + ) + + if (progress === 'locked-callback') { + await externalContention.promise + releaseManagerCallback.resolve() + await secondSave + await externalWrite + } else { + await externalWrite + releaseLegacyReplace.resolve() + await secondSave + } + + const persisted = await realStore.load(storagePath) + expect(persisted?.accounts).toEqual([ + expect.objectContaining({ refreshToken: 'a', label: 'A updated' }), + expect.objectContaining({ refreshToken: 'b', label: 'B updated' }), + expect.objectContaining({ refreshToken: 'oauth-added' }), + ]) + } finally { + releaseLock.resolve() + releaseManagerCallback.resolve() + releaseLegacyReplace.resolve() + await manager.dispose() + if (previousConfigDir === undefined) { + delete process.env.OPENCODE_CONFIG_DIR + } else { + process.env.OPENCODE_CONFIG_DIR = previousConfigDir + } + await rm(configDir, { recursive: true, force: true }) + } +}) diff --git a/packages/opencode/src/plugin/accounts.test.ts b/packages/opencode/src/plugin/accounts.test.ts index 84231302..07847e6a 100644 --- a/packages/opencode/src/plugin/accounts.test.ts +++ b/packages/opencode/src/plugin/accounts.test.ts @@ -5,9 +5,12 @@ import { expect, it, jest, - mock, spyOn, } from 'bun:test' +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { AccountStorageLockContentionError } from '@cortexkit/antigravity-auth-core' import { AccountManager, @@ -16,23 +19,13 @@ import { parseRateLimitReason, resolveQuotaGroup, } from './accounts' -// Mock storage to prevent test data from leaking to real config files. -// Bun's `mock.module` doesn't support the `importOriginal` callback that -// Vitest exposes, so we capture the real exports first and merge. -import * as realStorage from './storage' import { type AccountStorageV4, - saveAccounts, - saveAccountsReplace, + createAccountStorageStore, + loadAccounts, } from './storage' import type { OAuthAuthDetails } from './types' -mock.module('./storage', () => ({ - ...realStorage, - saveAccounts: mock().mockResolvedValue(undefined), - saveAccountsReplace: mock().mockResolvedValue(undefined), -})) - describe('AccountManager', () => { beforeEach(() => { jest.clearAllMocks() @@ -44,6 +37,96 @@ describe('AccountManager', () => { globalThis.unstubAllGlobals() }) + it('persists through the configured storagePath', async () => { + const configDir = await mkdtemp(join(tmpdir(), 'antigravity-manager-path-')) + const storagePath = join(configDir, 'custom-accounts.json') + const stored: AccountStorageV4 = { + version: 4, + accounts: [ + { refreshToken: 'custom-path-account', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + const manager = new AccountManager(undefined, stored, { storagePath }) + + try { + await manager.saveToDiskReplace() + const onDisk = JSON.parse(await readFile(storagePath, 'utf8')) + expect(onDisk.accounts).toHaveLength(1) + expect(onDisk.accounts[0]).toMatchObject({ + refreshToken: 'custom-path-account', + addedAt: 1, + lastUsed: 0, + }) + } finally { + await manager.dispose() + await rm(configDir, { recursive: true, force: true }) + } + }) + + it('retries a scoped save after real lock contention without dropping pool changes', async () => { + const configDir = await mkdtemp( + join(tmpdir(), 'antigravity-manager-retry-'), + ) + const storagePath = join(configDir, 'antigravity-accounts.json') + const store = createAccountStorageStore() + const initial: AccountStorageV4 = { + version: 4, + accounts: [ + { refreshToken: 'a', addedAt: 1, lastUsed: 0 }, + { refreshToken: 'b', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + let enteredLock!: () => void + const lockEntered = new Promise((resolve) => { + enteredLock = resolve + }) + let releaseLock!: () => void + const lockRelease = new Promise((resolve) => { + releaseLock = resolve + }) + const manager = new AccountManager(undefined, initial, { storagePath }) + + try { + await store.mutate(storagePath, () => initial) + manager.getAccounts()[0]!.label = 'A updated' + + const concurrentWrite = store.mutate(storagePath, async (current) => { + enteredLock() + await lockRelease + current.accounts[1]!.label = 'B updated' + current.accounts.push({ + refreshToken: 'oauth-added', + addedAt: 2, + lastUsed: 0, + }) + return current + }) + await lockEntered + + await expect(manager.saveToDisk()).rejects.toBeInstanceOf( + AccountStorageLockContentionError, + ) + releaseLock() + await concurrentWrite + + // The typed first failure switches AccountManager to scoped persistence. + await manager.saveToDisk() + + const persisted = await store.load(storagePath) + expect(persisted?.accounts).toEqual([ + expect.objectContaining({ refreshToken: 'a', label: 'A updated' }), + expect.objectContaining({ refreshToken: 'b', label: 'B updated' }), + expect.objectContaining({ refreshToken: 'oauth-added' }), + ]) + } finally { + releaseLock() + await manager.dispose() + await rm(configDir, { recursive: true, force: true }) + } + }) + it('treats on-disk storage as source of truth, even when empty', () => { const fallback: OAuthAuthDetails = { type: 'oauth', @@ -85,17 +168,11 @@ describe('AccountManager', () => { expect(manager.setAccountEnabled(0, true)).toBe(false) await manager.saveToDisk() - expect(saveAccounts as any).toHaveBeenCalledWith( - expect.objectContaining({ - accounts: [ - expect.objectContaining({ - enabled: false, - accountIneligible: true, - eligibilityStateUpdatedAt: 1_000, - }), - ], - }), - ) + expect((await loadAccounts())?.accounts[0]).toMatchObject({ + enabled: false, + accountIneligible: true, + eligibilityStateUpdatedAt: 1_000, + }) jest.setSystemTime(2_000) expect(manager.clearAccountAccessBlocks(0, true)).toBe(true) @@ -105,6 +182,11 @@ describe('AccountManager', () => { eligibilityStateUpdatedAt: 2_000, }) await manager.saveToDisk() + expect((await loadAccounts())?.accounts[0]).toMatchObject({ + enabled: true, + accountIneligible: false, + eligibilityStateUpdatedAt: 2_000, + }) jest.clearAllTimers() }) @@ -509,9 +591,6 @@ describe('AccountManager', () => { }) it('persists account removal via replace (no merge) so deletions stick', async () => { - ;(saveAccounts as any).mockClear() - ;(saveAccountsReplace as any).mockClear() - const stored: AccountStorageV4 = { version: 4, accounts: [ @@ -529,13 +608,9 @@ describe('AccountManager', () => { await manager.saveToDiskReplace() - // Must use replace (full overwrite), not merge — merge re-reads the file and - // resurrects the deleted account. - expect(saveAccountsReplace).toHaveBeenCalledTimes(1) - const saved = (saveAccountsReplace as any).mock.calls.at(-1)?.[0] - expect( - saved?.accounts.map((a: { refreshToken: string }) => a.refreshToken), - ).toEqual(['r1']) + expect((await loadAccounts())?.accounts.map((a) => a.refreshToken)).toEqual( + ['r1'], + ) }) it('keeps round-robin cursors separate by model family', () => { @@ -565,7 +640,6 @@ describe('AccountManager', () => { it('does not persist transient cooldown and switch metadata', async () => { jest.useFakeTimers() jest.setSystemTime(new Date(0)) - ;(saveAccounts as any).mockClear() const stored: AccountStorageV4 = { version: 4, @@ -582,7 +656,7 @@ describe('AccountManager', () => { await manager.saveToDisk() - const saved = (saveAccounts as any).mock.calls.at(-1)?.[0] + const saved = await loadAccounts() expect(saved?.accounts[0]).not.toHaveProperty('lastSwitchReason') expect(saved?.accounts[0]).not.toHaveProperty('coolingDownUntil') expect(saved?.accounts[0]).not.toHaveProperty('cooldownReason') diff --git a/packages/opencode/src/plugin/accounts.ts b/packages/opencode/src/plugin/accounts.ts index df31f210..c522c330 100644 --- a/packages/opencode/src/plugin/accounts.ts +++ b/packages/opencode/src/plugin/accounts.ts @@ -5,10 +5,9 @@ import { import { debugLogToFile } from './debug' import { + createAccountStorageStore, getStoragePath, loadAccounts, - saveAccounts, - saveAccountsReplace, } from './storage' import type { OAuthAuthDetails } from './types' @@ -27,24 +26,8 @@ export { resolveQuotaGroup, } from '@cortexkit/antigravity-auth-core' -const openCodeStore: AccountManagerOptions['store'] = { - load: async () => loadAccounts(), - saveMerged: async (_path, next) => { - await saveAccounts(next) - return next - }, - mutate: async (_path, fn) => { - const current = (await loadAccounts()) ?? { - version: 4, - accounts: [], - activeIndex: 0, - } - const next = (await fn(current)) ?? current - await saveAccountsReplace(next) - return next - }, - clear: async () => {}, -} +const openCodeStore: AccountManagerOptions['store'] = + createAccountStorageStore() export class AccountManager extends CoreAccountManager { constructor( diff --git a/packages/opencode/src/plugin/storage.test.ts b/packages/opencode/src/plugin/storage.test.ts index 0825bad3..6f6c45f4 100644 --- a/packages/opencode/src/plugin/storage.test.ts +++ b/packages/opencode/src/plugin/storage.test.ts @@ -6,6 +6,7 @@ import { type AccountMetadataV2 as AccountMetadata, type AccountStorageV2 as AccountStorage, type AccountStorageV4, + createAccountStorageStore, deduplicateAccountsByEmail, ensureGitignore, ensureGitignoreSync, @@ -14,6 +15,118 @@ import { migrateV2ToV3, } from './storage' +function deferred() { + let resolve!: (value: T | PromiseLike) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +describe('OpenCode AccountManager storage adapter', () => { + it('holds the real file lock across the callback and preserves concurrent pool changes', async () => { + const configDir = await mkdtemp(join(tmpdir(), 'antigravity-store-lock-')) + const path = join(configDir, 'antigravity-accounts.json') + const store = createAccountStorageStore() + const initial: AccountStorageV4 = { + version: 4, + accounts: [ + { refreshToken: 'a', addedAt: 1, lastUsed: 0 }, + { refreshToken: 'b', addedAt: 1, lastUsed: 0 }, + ], + activeIndex: 0, + } + const firstEntered = deferred() + const releaseFirst = deferred() + const firstFinished = deferred() + const contenderSawLock = deferred() + let secondCallbackEntered = false + + try { + await store.mutate(path, () => initial) + + const first = store + .mutate(path, async (current) => { + firstEntered.resolve() + await releaseFirst.promise + current.accounts[0]!.label = 'A updated' + return current + }) + .finally(() => firstFinished.resolve()) + + await firstEntered.promise + const second = store.mutate( + path, + (current) => { + secondCallbackEntered = true + current.accounts[1]!.label = 'B updated' + current.accounts.push({ + refreshToken: 'oauth-added', + addedAt: 2, + lastUsed: 0, + }) + return current + }, + { + sleep: async () => { + contenderSawLock.resolve() + await firstFinished.promise + }, + }, + ) + + await contenderSawLock.promise + expect(secondCallbackEntered).toBe(false) + + releaseFirst.resolve() + const [firstSaved, secondSaved] = await Promise.all([first, second]) + const onDisk = await store.load(path) + if (!onDisk) throw new Error('Storage disappeared after concurrent saves') + + expect(secondSaved).toEqual(onDisk) + expect(firstSaved.accounts[0]?.label).toBe('A updated') + expect(secondSaved.accounts).toEqual([ + expect.objectContaining({ refreshToken: 'a', label: 'A updated' }), + expect.objectContaining({ refreshToken: 'b', label: 'B updated' }), + expect.objectContaining({ refreshToken: 'oauth-added' }), + ]) + } finally { + releaseFirst.resolve() + await rm(configDir, { recursive: true, force: true }) + } + }) + + it('returns the actual merged state committed by saveMerged', async () => { + const configDir = await mkdtemp(join(tmpdir(), 'antigravity-store-merge-')) + const path = join(configDir, 'antigravity-accounts.json') + const store = createAccountStorageStore() + + try { + await store.mutate(path, () => ({ + version: 4, + accounts: [{ refreshToken: 'existing', addedAt: 1, lastUsed: 0 }], + activeIndex: 0, + })) + + const saved = await store.saveMerged(path, { + version: 4, + accounts: [{ refreshToken: 'incoming', addedAt: 2, lastUsed: 0 }], + activeIndex: 1, + }) + const onDisk = await store.load(path) + if (!onDisk) throw new Error('Storage disappeared after merged save') + + expect(saved).toEqual(onDisk) + expect(saved.accounts.map((account) => account.refreshToken)).toEqual([ + 'existing', + 'incoming', + ]) + } finally { + await rm(configDir, { recursive: true, force: true }) + } + }) +}) + describe('deduplicateAccountsByEmail', () => { it('returns empty array for empty input', () => { const result = deduplicateAccountsByEmail([]) diff --git a/packages/opencode/src/plugin/storage.ts b/packages/opencode/src/plugin/storage.ts index 86502519..d62b31af 100644 --- a/packages/opencode/src/plugin/storage.ts +++ b/packages/opencode/src/plugin/storage.ts @@ -30,6 +30,7 @@ import type { AccountMetadataV2, AccountMetadataV3, AccountModelFamily, + AccountStorageStore, AccountStorageUnreadableReason, AccountStorageV2, AccountStorageV4, @@ -344,6 +345,35 @@ export async function loadAccounts(): Promise { return coreLoadAccountStorage(path) } +async function prepareAccountStoragePath(path: string): Promise { + const configDir = dirname(path) + await fs.mkdir(configDir, { recursive: true }) + await ensureGitignore(configDir) +} + +/** + * Build the OpenCode adapter for AccountManager's path-aware store contract. + * Every read/modify/write operation uses core's lock-held persistence entry + * point, and returns the exact state committed by that operation. + */ +export function createAccountStorageStore(): AccountStorageStore { + return { + load: async (path) => { + await ensureGitignore(dirname(path)) + return coreLoadAccountStorage(path) + }, + saveMerged: async (path, storage) => { + await prepareAccountStoragePath(path) + return coreSaveAccountStorage(path, storage) + }, + mutate: async (path, mutate, options) => { + await prepareAccountStoragePath(path) + return coreMutateAccountStorage(path, mutate, options) + }, + clear: async (path) => coreClearAccountStorage(path), + } +} + /** * Merge `storage` into the persisted pool. Use this for non-destructive * writes (quota cache, eligibility, last-used) so concurrent writers @@ -351,9 +381,7 @@ export async function loadAccounts(): Promise { */ export async function saveAccounts(storage: AccountStorageV4): Promise { const path = getStoragePath() - const configDir = dirname(path) - await fs.mkdir(configDir, { recursive: true }) - await ensureGitignore(configDir) + await prepareAccountStoragePath(path) await coreSaveAccountStorage(path, storage) } @@ -366,9 +394,7 @@ export async function saveAccountsReplace( storage: AccountStorageV4, ): Promise { const path = getStoragePath() - const configDir = dirname(path) - await fs.mkdir(configDir, { recursive: true }) - await ensureGitignore(configDir) + await prepareAccountStoragePath(path) await coreSaveAccountStorageReplace(path, storage) }