From 514ef6e117908032ab61d86d11d4793cdbedae73 Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 19:18:19 +0800 Subject: [PATCH 01/10] feat: add sync storage and config foundation Add the config flag, storage primitives, atomic flagged persistence, import preparation hook, and sync-prune backup helper needed by the codex-multi-auth sync flow. Co-authored-by: Codex --- lib/config.ts | 98 +++++++++++++++++- lib/schemas.ts | 5 + lib/storage.ts | 182 +++++++++++++++++++++++++-------- lib/sync-prune-backup.ts | 45 ++++++++ test/sync-prune-backup.test.ts | 96 +++++++++++++++++ 5 files changed, 380 insertions(+), 46 deletions(-) create mode 100644 lib/sync-prune-backup.ts create mode 100644 test/sync-prune-backup.test.ts diff --git a/lib/config.ts b/lib/config.ts index af93ee73..d3241d11 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -1,5 +1,5 @@ -import { readFileSync, existsSync } from "node:fs"; -import { join } from "node:path"; +import { promises as fs, readFileSync, existsSync } from "node:fs"; +import { dirname, join } from "node:path"; import { homedir } from "node:os"; import type { PluginConfig } from "./types.js"; import { @@ -16,6 +16,7 @@ const TUI_GLYPH_MODES = new Set(["ascii", "unicode", "auto"]); const REQUEST_TRANSFORM_MODES = new Set(["native", "legacy"]); const UNSUPPORTED_CODEX_POLICIES = new Set(["strict", "fallback"]); const RETRY_PROFILES = new Set(["conservative", "balanced", "aggressive"]); +let configMutationMutex: Promise = Promise.resolve(); export type UnsupportedCodexPolicy = "strict" | "fallback"; @@ -111,7 +112,39 @@ function stripUtf8Bom(content: string): string { } function isRecord(value: unknown): value is Record { - return value !== null && typeof value === "object"; + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +type RawPluginConfig = Record; + +function withConfigMutationLock(fn: () => Promise): Promise { + const previous = configMutationMutex; + let release: () => void; + configMutationMutex = new Promise((resolve) => { + release = resolve; + }); + return previous.then(fn).finally(() => release()); +} + +async function renameConfigWithWindowsRetry(sourcePath: string, destinationPath: string): Promise { + let lastError: NodeJS.ErrnoException | null = null; + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + await fs.rename(sourcePath, destinationPath); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (process.platform === "win32" && (code === "EPERM" || code === "EBUSY")) { + lastError = error as NodeJS.ErrnoException; + await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + continue; + } + throw error; + } + } + if (lastError) { + throw lastError; + } } /** @@ -501,3 +534,62 @@ export function getStreamStallTimeoutMs(pluginConfig: PluginConfig): number { { min: 1_000 }, ); } + +async function savePluginConfigMutation( + mutate: (current: RawPluginConfig) => RawPluginConfig, +): Promise { + await withConfigMutationLock(async () => { + await fs.mkdir(dirname(CONFIG_PATH), { recursive: true }); + const current = existsSync(CONFIG_PATH) + ? await (async () => { + const raw = stripUtf8Bom(await fs.readFile(CONFIG_PATH, "utf-8")); + let parsed: unknown; + try { + parsed = JSON.parse(raw) as unknown; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Invalid JSON in config file ${CONFIG_PATH}: ${message}`); + } + if (!isRecord(parsed)) { + throw new Error(`Config file must contain a JSON object: ${CONFIG_PATH}`); + } + return { ...parsed }; + })() + : {}; + const next = mutate(current); + const tempPath = `${CONFIG_PATH}.${process.pid}.${Date.now()}.tmp`; + try { + await fs.writeFile(tempPath, `${JSON.stringify(next, null, 2)}\n`, { + encoding: "utf-8", + mode: 0o600, + }); + await renameConfigWithWindowsRetry(tempPath, CONFIG_PATH); + } catch (error) { + try { + await fs.unlink(tempPath); + } catch { + // Best effort cleanup only. + } + throw error; + } + }); +} + +export function getSyncFromCodexMultiAuthEnabled(pluginConfig: PluginConfig): boolean { + return pluginConfig.experimental?.syncFromCodexMultiAuth?.enabled === true; +} + +export async function setSyncFromCodexMultiAuthEnabled(enabled: boolean): Promise { + await savePluginConfigMutation((current) => { + const experimental = isRecord(current.experimental) ? { ...current.experimental } : {}; + const syncSettings = isRecord(experimental.syncFromCodexMultiAuth) + ? { ...experimental.syncFromCodexMultiAuth } + : {}; + syncSettings.enabled = enabled; + experimental.syncFromCodexMultiAuth = syncSettings; + return { + ...current, + experimental, + }; + }); +} diff --git a/lib/schemas.ts b/lib/schemas.ts index 6028246d..714e9368 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -52,6 +52,11 @@ export const PluginConfigSchema = z.object({ pidOffsetEnabled: z.boolean().optional(), fetchTimeoutMs: z.number().min(1_000).optional(), streamStallTimeoutMs: z.number().min(1_000).optional(), + experimental: z.object({ + syncFromCodexMultiAuth: z.object({ + enabled: z.boolean().optional(), + }).optional(), + }).optional(), }); export type PluginConfigFromSchema = z.infer; diff --git a/lib/storage.ts b/lib/storage.ts index 151e2213..bcd90f5d 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -51,6 +51,11 @@ export interface ImportAccountsOptions { backupMode?: ImportBackupMode; } +type PrepareImportStorage = ( + normalized: AccountStorageV3, + existing: AccountStorageV3 | null, +) => AccountStorageV3; + export type ImportBackupStatus = "created" | "skipped" | "failed"; export interface ImportAccountsResult { @@ -1009,14 +1014,53 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { }; } -export async function loadFlaggedAccounts(): Promise { +async function saveFlaggedAccountsUnlocked(storage: FlaggedAccountStorageV1): Promise { + const path = getFlaggedAccountsPath(); + const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; + const tempPath = `${path}.${uniqueSuffix}.tmp`; + try { + await fs.mkdir(dirname(path), { recursive: true }); + const content = JSON.stringify(normalizeFlaggedStorage(storage), null, 2); + await fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }); + await renameWithWindowsRetry(tempPath, path); + } catch (error) { + try { + await fs.unlink(tempPath); + } catch { + // Ignore cleanup failures. + } + log.error("Failed to save flagged account storage", { path, error: String(error) }); + throw error; + } +} + +async function loadFlaggedAccountsUnlocked(): Promise { const path = getFlaggedAccountsPath(); const empty: FlaggedAccountStorageV1 = { version: 1, accounts: [] }; + const removeOrphanedFlaggedAccounts = async ( + storage: FlaggedAccountStorageV1, + ): Promise => { + const accounts = await loadAccountsInternal(saveAccountsUnlocked); + if (!accounts) { + return storage; + } + const activeRefreshTokens = new Set((accounts?.accounts ?? []).map((account) => account.refreshToken)); + const filteredAccounts = storage.accounts.filter((flagged) => activeRefreshTokens.has(flagged.refreshToken)); + if (filteredAccounts.length === storage.accounts.length) { + return storage; + } + const cleaned = { + version: 1 as const, + accounts: filteredAccounts, + }; + await saveFlaggedAccountsUnlocked(cleaned); + return cleaned; + }; try { const content = await fs.readFile(path, "utf-8"); const data = JSON.parse(content) as unknown; - return normalizeFlaggedStorage(data); + return await removeOrphanedFlaggedAccounts(normalizeFlaggedStorage(data)); } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { @@ -1035,7 +1079,7 @@ export async function loadFlaggedAccounts(): Promise { const legacyData = JSON.parse(legacyContent) as unknown; const migrated = normalizeFlaggedStorage(legacyData); if (migrated.accounts.length > 0) { - await saveFlaggedAccounts(migrated); + await saveFlaggedAccountsUnlocked(migrated); } try { await fs.unlink(legacyPath); @@ -1047,7 +1091,7 @@ export async function loadFlaggedAccounts(): Promise { to: path, accounts: migrated.accounts.length, }); - return migrated; + return await removeOrphanedFlaggedAccounts(migrated); } catch (error) { log.error("Failed to migrate legacy flagged account storage", { from: legacyPath, @@ -1058,26 +1102,35 @@ export async function loadFlaggedAccounts(): Promise { } } -export async function saveFlaggedAccounts(storage: FlaggedAccountStorageV1): Promise { +export async function loadFlaggedAccounts(): Promise { + return withStorageLock(async () => loadFlaggedAccountsUnlocked()); +} + +export async function withFlaggedAccountsTransaction( + handler: ( + current: FlaggedAccountStorageV1, + persist: (storage: FlaggedAccountStorageV1) => Promise, + ) => Promise, +): Promise { return withStorageLock(async () => { - const path = getFlaggedAccountsPath(); - const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; - const tempPath = `${path}.${uniqueSuffix}.tmp`; + const current = await loadFlaggedAccountsUnlocked(); + return handler(current, saveFlaggedAccountsUnlocked); + }); +} - try { - await fs.mkdir(dirname(path), { recursive: true }); - const content = JSON.stringify(normalizeFlaggedStorage(storage), null, 2); - await fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }); - await renameWithWindowsRetry(tempPath, path); - } catch (error) { - try { - await fs.unlink(tempPath); - } catch { - // Ignore cleanup failures. - } - log.error("Failed to save flagged account storage", { path, error: String(error) }); - throw error; - } +export async function loadAccountAndFlaggedStorageSnapshot(): Promise<{ + accounts: AccountStorageV3 | null; + flagged: FlaggedAccountStorageV1; +}> { + return withStorageLock(async () => ({ + accounts: await loadAccountsInternal(saveAccountsUnlocked), + flagged: await loadFlaggedAccountsUnlocked(), + })); +} + +export async function saveFlaggedAccounts(storage: FlaggedAccountStorageV1): Promise { + return withStorageLock(async () => { + await saveFlaggedAccountsUnlocked(storage); }); } @@ -1155,26 +1208,62 @@ export async function previewImportAccounts( const { normalized } = await readAndNormalizeImportFile(filePath); return withAccountStorageTransaction((existing) => { - const existingAccounts = existing?.accounts ?? []; - const merged = [...existingAccounts, ...normalized.accounts]; - - if (merged.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { - const deduped = deduplicateAccountsForStorage(merged); - if (deduped.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { - throw new Error( - `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts (would have ${deduped.length})`, - ); - } + return Promise.resolve(previewImportAccountsAgainstExistingNormalized(normalized, existing)); + }); +} + +export async function previewImportAccountsWithExistingStorage( + filePath: string, + existing: AccountStorageV3 | null | undefined, +): Promise<{ imported: number; total: number; skipped: number }> { + const { normalized } = await readAndNormalizeImportFile(filePath); + return previewImportAccountsAgainstExistingNormalized(normalized, existing); +} + +function previewImportAccountsAgainstExistingNormalized( + normalized: AccountStorageV3, + existing: AccountStorageV3 | null | undefined, +): { imported: number; total: number; skipped: number } { + const existingAccounts = existing?.accounts ?? []; + const merged = [...existingAccounts, ...normalized.accounts]; + + if (merged.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { + const deduped = deduplicateAccountsForStorage(merged); + if (deduped.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { + throw new Error( + `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts (would have ${deduped.length})`, + ); } + } - const deduplicatedAccounts = deduplicateAccountsForStorage(merged); - const imported = deduplicatedAccounts.length - existingAccounts.length; - const skipped = normalized.accounts.length - imported; - return Promise.resolve({ - imported, - total: deduplicatedAccounts.length, - skipped, - }); + const deduplicatedAccounts = deduplicateAccountsForStorage(merged); + const imported = Math.max(0, deduplicatedAccounts.length - existingAccounts.length); + const skipped = normalized.accounts.length - imported; + return { + imported, + total: deduplicatedAccounts.length, + skipped, + }; +} + +export async function backupRawAccountsFile(filePath: string, force = true): Promise { + await withStorageLock(async () => { + const resolvedPath = resolvePath(filePath); + + if (!force && existsSync(resolvedPath)) { + throw new Error(`File already exists: ${resolvedPath}`); + } + + await migrateLegacyProjectStorageIfNeeded(saveAccountsUnlocked); + const storagePath = getStoragePath(); + if (!existsSync(storagePath)) { + throw new Error("No accounts to back up"); + } + + await fs.mkdir(dirname(resolvedPath), { recursive: true }); + await fs.copyFile(storagePath, resolvedPath); + await fs.chmod(resolvedPath, 0o600).catch(() => undefined); + log.info("Backed up raw accounts storage", { path: resolvedPath, source: storagePath }); }); } @@ -1213,6 +1302,7 @@ export async function exportAccounts(filePath: string, force = true): Promise { const { resolvedPath, normalized } = await readAndNormalizeImportFile(filePath); const backupMode = options.backupMode ?? "none"; @@ -1227,6 +1317,12 @@ export async function importAccounts( backupError, } = await withAccountStorageTransaction(async (existing, persist) => { + const prepared = prepare ? prepare(normalized, existing) : normalized; + const preparedNormalized = normalizeAccountStorage(prepared); + if (!preparedNormalized) { + throw new Error("prepare() returned invalid account storage"); + } + const skippedByPrepare = Math.max(0, normalized.accounts.length - preparedNormalized.accounts.length); const existingStorage: AccountStorageV3 = existing ?? ({ @@ -1262,7 +1358,7 @@ export async function importAccounts( } } - const merged = [...existingAccounts, ...normalized.accounts]; + const merged = [...existingAccounts, ...preparedNormalized.accounts]; if (merged.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { const deduped = deduplicateAccountsForStorage(merged); @@ -1309,8 +1405,8 @@ export async function importAccounts( await persist(newStorage); - const imported = deduplicatedAccounts.length - existingAccounts.length; - const skipped = normalized.accounts.length - imported; + const imported = Math.max(0, deduplicatedAccounts.length - existingAccounts.length); + const skipped = skippedByPrepare + Math.max(0, preparedNormalized.accounts.length - imported); return { imported, total: deduplicatedAccounts.length, diff --git a/lib/sync-prune-backup.ts b/lib/sync-prune-backup.ts new file mode 100644 index 00000000..16dc666f --- /dev/null +++ b/lib/sync-prune-backup.ts @@ -0,0 +1,45 @@ +import type { AccountStorageV3 } from "./storage.js"; + +type FlaggedSnapshot = { + version: 1; + accounts: TAccount[]; +}; + +type TokenRedacted = + Omit & { + accessToken?: undefined; + refreshToken?: undefined; + idToken?: undefined; + }; + +function cloneWithoutTokens(account: TAccount): TokenRedacted { + const clone = structuredClone(account) as TokenRedacted; + delete clone.accessToken; + delete clone.refreshToken; + delete clone.idToken; + return clone; +} + +export function createSyncPruneBackupPayload( + currentAccountsStorage: AccountStorageV3, + currentFlaggedStorage: FlaggedSnapshot, +): { + version: 1; + accounts: Omit & { + accounts: Array>; + }; + flagged: FlaggedSnapshot>; +} { + return { + version: 1, + accounts: { + ...currentAccountsStorage, + accounts: currentAccountsStorage.accounts.map((account) => cloneWithoutTokens(account)), + activeIndexByFamily: { ...(currentAccountsStorage.activeIndexByFamily ?? {}) }, + }, + flagged: { + ...currentFlaggedStorage, + accounts: currentFlaggedStorage.accounts.map((flagged) => cloneWithoutTokens(flagged)), + }, + }; +} diff --git a/test/sync-prune-backup.test.ts b/test/sync-prune-backup.test.ts new file mode 100644 index 00000000..a7102dd2 --- /dev/null +++ b/test/sync-prune-backup.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from "vitest"; +import { createSyncPruneBackupPayload } from "../lib/sync-prune-backup.js"; +import type { AccountStorageV3 } from "../lib/storage.js"; + +describe("sync prune backup payload", () => { + it("omits live tokens from the prune backup payload", () => { + const storage: AccountStorageV3 = { + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "org-sync", + organizationId: "org-sync", + accountIdSource: "org", + refreshToken: "refresh-token", + accessToken: "access-token", + idToken: "id-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }; + const payload = createSyncPruneBackupPayload(storage, { + version: 1, + accounts: [ + { + refreshToken: "refresh-token", + accessToken: "flagged-access-token", + idToken: "flagged-id-token", + }, + ], + }); + + expect(payload.accounts.accounts[0]).not.toHaveProperty("accessToken"); + expect(payload.accounts.accounts[0]).not.toHaveProperty("refreshToken"); + expect(payload.accounts.accounts[0]).not.toHaveProperty("idToken"); + expect(payload.flagged.accounts[0]).not.toHaveProperty("accessToken"); + expect(payload.flagged.accounts[0]).not.toHaveProperty("refreshToken"); + expect(payload.flagged.accounts[0]).not.toHaveProperty("idToken"); + }); + + it("deep-clones nested metadata so later mutations do not leak into the snapshot", () => { + const storage: AccountStorageV3 = { + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "org-sync", + organizationId: "org-sync", + accountIdSource: "org", + refreshToken: "refresh-token", + accessToken: "access-token", + idToken: "id-token", + accountTags: ["work"], + addedAt: 1, + lastUsed: 1, + lastSelectedModelByFamily: { + codex: "gpt-5.4", + }, + }, + ], + }; + const flagged = { + version: 1 as const, + accounts: [ + { + refreshToken: "refresh-token", + accessToken: "flagged-access-token", + idToken: "flagged-id-token", + metadata: { + source: "flagged", + }, + }, + ], + }; + + const payload = createSyncPruneBackupPayload(storage, flagged); + + storage.accounts[0]!.accountTags?.push("mutated"); + storage.accounts[0]!.lastSelectedModelByFamily = { codex: "gpt-5.5" }; + flagged.accounts[0]!.metadata.source = "mutated"; + + expect(payload.accounts.accounts[0]?.accountTags).toEqual(["work"]); + expect(payload.accounts.accounts[0]?.lastSelectedModelByFamily).toEqual({ codex: "gpt-5.4" }); + expect(payload.accounts.accounts[0]).not.toHaveProperty("idToken"); + expect(payload.flagged.accounts[0]).toMatchObject({ + metadata: { + source: "flagged", + }, + }); + expect(payload.flagged.accounts[0]).not.toHaveProperty("refreshToken"); + expect(payload.flagged.accounts[0]).not.toHaveProperty("idToken"); + }); +}); From 8be2d48af504e800dd03020c966d01167ccb1363 Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 19:33:05 +0800 Subject: [PATCH 02/10] fix: harden foundation review gaps Align config rename retry behavior with storage, log backup permission failures, and add regression coverage for the new foundation storage and config APIs. Co-authored-by: Codex --- lib/config.ts | 2 +- lib/storage.ts | 7 ++- test/plugin-config.test.ts | 51 ++++++++++++++++++++++ test/storage.test.ts | 87 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 2 deletions(-) diff --git a/lib/config.ts b/lib/config.ts index d3241d11..ec0af6f1 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -134,7 +134,7 @@ async function renameConfigWithWindowsRetry(sourcePath: string, destinationPath: return; } catch (error) { const code = (error as NodeJS.ErrnoException).code; - if (process.platform === "win32" && (code === "EPERM" || code === "EBUSY")) { + if (code === "EPERM" || code === "EBUSY") { lastError = error as NodeJS.ErrnoException; await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); continue; diff --git a/lib/storage.ts b/lib/storage.ts index bcd90f5d..410440f0 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1262,7 +1262,12 @@ export async function backupRawAccountsFile(filePath: string, force = true): Pro await fs.mkdir(dirname(resolvedPath), { recursive: true }); await fs.copyFile(storagePath, resolvedPath); - await fs.chmod(resolvedPath, 0o600).catch(() => undefined); + await fs.chmod(resolvedPath, 0o600).catch((chmodErr) => { + log.warn("Failed to restrict backup file permissions", { + path: resolvedPath, + error: String(chmodErr), + }); + }); log.info("Backed up raw accounts storage", { path: resolvedPath, source: storagePath }); }); } diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 1cf69951..0dd59fb0 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -20,6 +20,8 @@ import { getRequestTransformMode, getFetchTimeoutMs, getStreamStallTimeoutMs, + getSyncFromCodexMultiAuthEnabled, + setSyncFromCodexMultiAuthEnabled, } from '../lib/config.js'; import type { PluginConfig } from '../lib/types.js'; import * as fs from 'node:fs'; @@ -739,5 +741,54 @@ describe('Plugin Configuration', () => { delete process.env.CODEX_AUTH_STREAM_STALL_TIMEOUT_MS; }); }); + + describe('sync config mutation', () => { + it('persists the sync toggle via temp file rename', async () => { + mockExistsSync.mockReturnValue(true); + const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue( + JSON.stringify({ experimental: { syncFromCodexMultiAuth: { enabled: false } } }) as never + ); + const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); + const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); + const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); + + try { + await setSyncFromCodexMultiAuthEnabled(true); + expect(readSpy).toHaveBeenCalled(); + expect(writeSpy).toHaveBeenCalled(); + expect(renameSpy).toHaveBeenCalled(); + expect(unlinkSpy).not.toHaveBeenCalled(); + } finally { + readSpy.mockRestore(); + mkdirSpy.mockRestore(); + writeSpy.mockRestore(); + renameSpy.mockRestore(); + unlinkSpy.mockRestore(); + } + }); + + it('surfaces malformed config files instead of replacing them', async () => { + mockExistsSync.mockReturnValue(true); + const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue('{ invalid json' as never); + const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); + const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + + try { + await expect(setSyncFromCodexMultiAuthEnabled(true)).rejects.toThrow(/Invalid JSON in config file/); + expect(writeSpy).not.toHaveBeenCalled(); + expect(renameSpy).not.toHaveBeenCalled(); + } finally { + readSpy.mockRestore(); + writeSpy.mockRestore(); + renameSpy.mockRestore(); + } + }); + + it('reads the persisted sync toggle flag', () => { + expect(getSyncFromCodexMultiAuthEnabled({ experimental: { syncFromCodexMultiAuth: { enabled: true } } })).toBe(true); + expect(getSyncFromCodexMultiAuthEnabled({})).toBe(false); + }); + }); }); diff --git a/test/storage.test.ts b/test/storage.test.ts index 94d20141..7724a328 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -19,8 +19,12 @@ import { exportAccounts, importAccounts, previewImportAccounts, + previewImportAccountsWithExistingStorage, createTimestampedBackupPath, withAccountStorageTransaction, + withFlaggedAccountsTransaction, + loadAccountAndFlaggedStorageSnapshot, + backupRawAccountsFile, } from "../lib/storage.js"; // Mocking the behavior we're about to implement for TDD @@ -1511,6 +1515,89 @@ describe("storage", () => { expect(loaded.accounts).toHaveLength(1); expect(loaded.accounts[0]?.refreshToken).toBe("flagged-ebusy"); }); + + it("updates flagged storage inside withFlaggedAccountsTransaction", async () => { + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + refreshToken: "flagged-keep", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + { + refreshToken: "flagged-drop", + flaggedAt: 2, + addedAt: 2, + lastUsed: 2, + }, + ], + }); + + await withFlaggedAccountsTransaction(async (current, persist) => { + await persist({ + version: 1, + accounts: current.accounts.filter((account) => account.refreshToken !== "flagged-drop"), + }); + }); + + const loaded = await loadFlaggedAccounts(); + expect(loaded.accounts.map((account) => account.refreshToken)).toEqual(["flagged-keep"]); + }); + + it("loads account and flagged storage from one snapshot helper", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + refreshToken: "account-refresh", + accountId: "account-id", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + refreshToken: "account-refresh", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const snapshot = await loadAccountAndFlaggedStorageSnapshot(); + expect(snapshot.accounts?.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + expect(snapshot.flagged.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + }); + + it("copies the raw accounts file for backup", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + refreshToken: "backup-refresh", + accountId: "backup-id", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const backupPath = join(testWorkDir, "backup.json"); + await backupRawAccountsFile(backupPath); + + const raw = JSON.parse(await fs.readFile(backupPath, "utf-8")) as { accounts: Array<{ refreshToken: string }> }; + expect(raw.accounts[0]?.refreshToken).toBe("backup-refresh"); + }); }); describe("setStoragePath", () => { From 7ae17db5d43bd27e5932194dfad97d92fcf58044 Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 19:52:54 +0800 Subject: [PATCH 03/10] fix: address foundation review follow-ups Document the non-reentrant flagged transaction helper and stub mkdir in the malformed config test. Co-authored-by: Codex --- lib/storage.ts | 5 +++++ test/plugin-config.test.ts | 2 ++ 2 files changed, 7 insertions(+) diff --git a/lib/storage.ts b/lib/storage.ts index 410440f0..b631acf0 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1106,6 +1106,11 @@ export async function loadFlaggedAccounts(): Promise { return withStorageLock(async () => loadFlaggedAccountsUnlocked()); } +/** + * Runs `handler` while the storage lock is held. + * Do not call lock-acquiring storage helpers from inside `handler`; + * use only `persist` for writes while the transaction is active. + */ export async function withFlaggedAccountsTransaction( handler: ( current: FlaggedAccountStorageV1, diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 0dd59fb0..b86525fd 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -771,6 +771,7 @@ describe('Plugin Configuration', () => { it('surfaces malformed config files instead of replacing them', async () => { mockExistsSync.mockReturnValue(true); const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue('{ invalid json' as never); + const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined as never); const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); @@ -780,6 +781,7 @@ describe('Plugin Configuration', () => { expect(renameSpy).not.toHaveBeenCalled(); } finally { readSpy.mockRestore(); + mkdirSpy.mockRestore(); writeSpy.mockRestore(); renameSpy.mockRestore(); } From 78f0e5a4e7902bac36600ca2bb7c13a5ea16eb7b Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 20:00:28 +0800 Subject: [PATCH 04/10] fix: reuse accounts snapshot for flagged cleanup Avoid reopening the accounts file while building the combined snapshot and cover the single-read path with a regression test. Co-authored-by: Codex --- lib/storage.ts | 17 ++++++++++----- test/storage.test.ts | 52 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index b631acf0..b8849b93 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1034,13 +1034,15 @@ async function saveFlaggedAccountsUnlocked(storage: FlaggedAccountStorageV1): Pr } } -async function loadFlaggedAccountsUnlocked(): Promise { +async function loadFlaggedAccountsUnlocked( + accountsSnapshot?: AccountStorageV3 | null, +): Promise { const path = getFlaggedAccountsPath(); const empty: FlaggedAccountStorageV1 = { version: 1, accounts: [] }; const removeOrphanedFlaggedAccounts = async ( storage: FlaggedAccountStorageV1, ): Promise => { - const accounts = await loadAccountsInternal(saveAccountsUnlocked); + const accounts = accountsSnapshot ?? (await loadAccountsInternal(saveAccountsUnlocked)); if (!accounts) { return storage; } @@ -1127,10 +1129,13 @@ export async function loadAccountAndFlaggedStorageSnapshot(): Promise<{ accounts: AccountStorageV3 | null; flagged: FlaggedAccountStorageV1; }> { - return withStorageLock(async () => ({ - accounts: await loadAccountsInternal(saveAccountsUnlocked), - flagged: await loadFlaggedAccountsUnlocked(), - })); + return withStorageLock(async () => { + const accounts = await loadAccountsInternal(saveAccountsUnlocked); + return { + accounts, + flagged: await loadFlaggedAccountsUnlocked(accounts), + }; + }); } export async function saveFlaggedAccounts(storage: FlaggedAccountStorageV1): Promise { diff --git a/test/storage.test.ts b/test/storage.test.ts index 7724a328..9126ec2b 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1577,6 +1577,58 @@ describe("storage", () => { expect(snapshot.flagged.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); }); + it("reuses the loaded accounts snapshot when pruning orphaned flagged entries", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + refreshToken: "account-refresh", + accountId: "account-id", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + refreshToken: "account-refresh", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + { + refreshToken: "orphan-refresh", + flaggedAt: 2, + addedAt: 2, + lastUsed: 2, + }, + ], + }); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile"); + let accountReadCount = 0; + readSpy.mockImplementation(async (path, options) => { + if (String(path) === testStoragePath) { + accountReadCount++; + } + return originalReadFile(path as Parameters[0], options as never); + }); + + try { + const snapshot = await loadAccountAndFlaggedStorageSnapshot(); + expect(snapshot.accounts?.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + expect(snapshot.flagged.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + expect(accountReadCount).toBe(1); + } finally { + readSpy.mockRestore(); + } + }); + it("copies the raw accounts file for backup", async () => { await saveAccounts({ version: 3, From 0164cf5f39ef799809955e74fbb794b7c6f2b5e4 Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 20:14:04 +0800 Subject: [PATCH 05/10] fix: harden storage review follow-ups Preserve null account snapshots, retry raw backup copies on Windows lock errors, and constrain prepare() to filtered imports with regression coverage. Co-authored-by: Codex --- lib/storage.ts | 34 ++++++++++- test/storage.test.ts | 133 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 2 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index b8849b93..cb38f040 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -159,6 +159,30 @@ async function renameWithWindowsRetry(sourcePath: string, destinationPath: strin } } +async function copyFileWithWindowsRetry(sourcePath: string, destinationPath: string): Promise { + let lastError: NodeJS.ErrnoException | null = null; + + for (let attempt = 0; attempt < WINDOWS_RENAME_RETRY_ATTEMPTS; attempt += 1) { + try { + await fs.copyFile(sourcePath, destinationPath); + return; + } catch (error) { + if (isWindowsLockError(error)) { + lastError = error; + await new Promise((resolve) => + setTimeout(resolve, WINDOWS_RENAME_RETRY_BASE_DELAY_MS * 2 ** attempt), + ); + continue; + } + throw error; + } + } + + if (lastError) { + throw lastError; + } +} + async function writeFileWithTimeout(filePath: string, content: string, timeoutMs: number): Promise { const controller = new AbortController(); const timeoutHandle = setTimeout(() => controller.abort(), timeoutMs); @@ -1042,7 +1066,10 @@ async function loadFlaggedAccountsUnlocked( const removeOrphanedFlaggedAccounts = async ( storage: FlaggedAccountStorageV1, ): Promise => { - const accounts = accountsSnapshot ?? (await loadAccountsInternal(saveAccountsUnlocked)); + const accounts = + accountsSnapshot === undefined + ? await loadAccountsInternal(saveAccountsUnlocked) + : accountsSnapshot; if (!accounts) { return storage; } @@ -1271,7 +1298,7 @@ export async function backupRawAccountsFile(filePath: string, force = true): Pro } await fs.mkdir(dirname(resolvedPath), { recursive: true }); - await fs.copyFile(storagePath, resolvedPath); + await copyFileWithWindowsRetry(storagePath, resolvedPath); await fs.chmod(resolvedPath, 0o600).catch((chmodErr) => { log.warn("Failed to restrict backup file permissions", { path: resolvedPath, @@ -1337,6 +1364,9 @@ export async function importAccounts( if (!preparedNormalized) { throw new Error("prepare() returned invalid account storage"); } + if (preparedNormalized.accounts.length > normalized.accounts.length) { + throw new Error("prepare() must return a subset of normalized import accounts"); + } const skippedByPrepare = Math.max(0, normalized.accounts.length - preparedNormalized.accounts.length); const existingStorage: AccountStorageV3 = existing ?? diff --git a/test/storage.test.ts b/test/storage.test.ts index 9126ec2b..41e7a543 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -193,6 +193,33 @@ describe("storage", () => { expect(loaded?.accounts[0]?.accountId).toBe("existing"); }); + it("should preview import results against a provided existing snapshot", async () => { + const existing = { + version: 3 as const, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref1", addedAt: 1, lastUsed: 2 }], + }; + + await fs.writeFile( + exportPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { accountId: "existing", refreshToken: "ref1", addedAt: 3, lastUsed: 4 }, + { accountId: "preview", refreshToken: "ref2", addedAt: 5, lastUsed: 6 }, + ], + }), + ); + + const preview = await previewImportAccountsWithExistingStorage(exportPath, existing); + expect(preview.imported).toBe(1); + expect(preview.skipped).toBe(1); + expect(preview.total).toBe(2); + }); + it("creates timestamped backup paths in storage backups directory", () => { const path = createTimestampedBackupPath(); const expectedBackupDir = join(dirname(testStoragePath), "backups"); @@ -641,6 +668,62 @@ describe("storage", () => { await expect(importAccounts(exportPath)).rejects.toThrow(/Invalid account storage format/); }); + it("filters import accounts through prepare()", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }); + + await fs.writeFile( + exportPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { accountId: "drop-me", refreshToken: "ref-drop", addedAt: 2, lastUsed: 2 }, + { accountId: "keep-me", refreshToken: "ref-keep", addedAt: 3, lastUsed: 3 }, + ], + }), + ); + + const result = await importAccounts(exportPath, {}, (normalized) => ({ + ...normalized, + accounts: normalized.accounts.filter((account) => account.accountId === "keep-me"), + })); + + expect(result.imported).toBe(1); + expect(result.skipped).toBe(1); + expect(result.total).toBe(2); + + const loaded = await loadAccounts(); + expect(loaded?.accounts.map((account) => account.accountId)).toEqual(["existing", "keep-me"]); + }); + + it("rejects prepare() results that expand the import account set", async () => { + await fs.writeFile( + exportPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }), + ); + + await expect( + importAccounts(exportPath, {}, (normalized) => ({ + ...normalized, + accounts: [ + ...normalized.accounts, + { accountId: "extra", refreshToken: "ref-extra", addedAt: 2, lastUsed: 2 }, + ], + })), + ).rejects.toThrow(/prepare\(\) must return a subset/); + }); + it("continues import in best-effort mode when pre-import backup write is locked", async () => { await saveAccounts({ version: 3, @@ -736,6 +819,56 @@ describe("storage", () => { expect(loaded?.accounts).toHaveLength(1); expect(loaded?.accounts[0]?.accountId).toBe("existing"); }); + + it("throws when backing up raw accounts without force and the destination exists", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }); + + const backupPath = join(testWorkDir, "raw-backup.json"); + await fs.writeFile(backupPath, "exists"); + + await expect(backupRawAccountsFile(backupPath, false)).rejects.toThrow(/File already exists/); + }); + + it("retries raw backup copy on EBUSY and succeeds", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }); + + const backupPath = join(testWorkDir, "raw-backup-retry.json"); + const originalCopyFile = fs.copyFile.bind(fs); + let attemptCount = 0; + const copySpy = vi.spyOn(fs, "copyFile").mockImplementation(async (source, destination, mode) => { + attemptCount += 1; + if (attemptCount < 3) { + const err = new Error("copy locked") as NodeJS.ErrnoException; + err.code = "EBUSY"; + throw err; + } + return originalCopyFile( + source as Parameters[0], + destination as Parameters[1], + mode as Parameters[2], + ); + }); + + try { + await backupRawAccountsFile(backupPath); + } finally { + copySpy.mockRestore(); + } + + expect(attemptCount).toBe(3); + const raw = JSON.parse(await fs.readFile(backupPath, "utf-8")) as { accounts: Array<{ accountId?: string }> }; + expect(raw.accounts[0]?.accountId).toBe("existing"); + }); }); describe("filename migration (TDD)", () => { From c0e8f97102bb62cf24c862148bcb383fe1069e53 Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 20:18:51 +0800 Subject: [PATCH 06/10] fix: tighten config and import invariants Guard config mutations with a lockfile and enforce prepare() as a true subset of imported accounts. Co-authored-by: Codex --- lib/config.ts | 45 ++++++++++++++++++++++++++++++-------- lib/storage.ts | 11 +++++++++- test/plugin-config.test.ts | 7 +++++- test/storage.test.ts | 22 +++++++++++++++++++ 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/lib/config.ts b/lib/config.ts index ec0af6f1..ee1f8482 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -16,6 +16,7 @@ const TUI_GLYPH_MODES = new Set(["ascii", "unicode", "auto"]); const REQUEST_TRANSFORM_MODES = new Set(["native", "legacy"]); const UNSUPPORTED_CODEX_POLICIES = new Set(["strict", "fallback"]); const RETRY_PROFILES = new Set(["conservative", "balanced", "aggressive"]); +const CONFIG_LOCK_PATH = `${CONFIG_PATH}.lock`; let configMutationMutex: Promise = Promise.resolve(); export type UnsupportedCodexPolicy = "strict" | "fallback"; @@ -126,6 +127,32 @@ function withConfigMutationLock(fn: () => Promise): Promise { return previous.then(fn).finally(() => release()); } +async function withConfigProcessLock(fn: () => Promise): Promise { + let lastError: NodeJS.ErrnoException | null = null; + + for (let attempt = 0; attempt < 20; attempt += 1) { + try { + const handle = await fs.open(CONFIG_LOCK_PATH, "wx", 0o600); + try { + return await fn(); + } finally { + await handle.close(); + await fs.unlink(CONFIG_LOCK_PATH).catch(() => undefined); + } + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "EEXIST") { + lastError = error as NodeJS.ErrnoException; + await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + continue; + } + throw error; + } + } + + throw lastError ?? new Error(`Timed out acquiring config lock ${CONFIG_LOCK_PATH}`); +} + async function renameConfigWithWindowsRetry(sourcePath: string, destinationPath: string): Promise { let lastError: NodeJS.ErrnoException | null = null; for (let attempt = 0; attempt < 5; attempt += 1) { @@ -536,12 +563,12 @@ export function getStreamStallTimeoutMs(pluginConfig: PluginConfig): number { } async function savePluginConfigMutation( - mutate: (current: RawPluginConfig) => RawPluginConfig, + mutate: (current: RawPluginConfig) => RawPluginConfig, ): Promise { - await withConfigMutationLock(async () => { - await fs.mkdir(dirname(CONFIG_PATH), { recursive: true }); - const current = existsSync(CONFIG_PATH) - ? await (async () => { + await withConfigMutationLock(async () => withConfigProcessLock(async () => { + await fs.mkdir(dirname(CONFIG_PATH), { recursive: true }); + const current = existsSync(CONFIG_PATH) + ? await (async () => { const raw = stripUtf8Bom(await fs.readFile(CONFIG_PATH, "utf-8")); let parsed: unknown; try { @@ -569,10 +596,10 @@ async function savePluginConfigMutation( await fs.unlink(tempPath); } catch { // Best effort cleanup only. - } - throw error; - } - }); + } + throw error; + } + })); } export function getSyncFromCodexMultiAuthEnabled(pluginConfig: PluginConfig): boolean { diff --git a/lib/storage.ts b/lib/storage.ts index cb38f040..22352b13 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -183,6 +183,10 @@ async function copyFileWithWindowsRetry(sourcePath: string, destinationPath: str } } +function getAccountSubsetIdentity(account: AccountMetadataV3): string { + return [account.organizationId ?? "", account.accountId ?? "", account.refreshToken, account.email ?? ""].join("\u0000"); +} + async function writeFileWithTimeout(filePath: string, content: string, timeoutMs: number): Promise { const controller = new AbortController(); const timeoutHandle = setTimeout(() => controller.abort(), timeoutMs); @@ -1364,7 +1368,12 @@ export async function importAccounts( if (!preparedNormalized) { throw new Error("prepare() returned invalid account storage"); } - if (preparedNormalized.accounts.length > normalized.accounts.length) { + const normalizedIdentities = new Set(normalized.accounts.map((account) => getAccountSubsetIdentity(account))); + const preparedIdentities = preparedNormalized.accounts.map((account) => getAccountSubsetIdentity(account)); + if ( + preparedNormalized.accounts.length > normalized.accounts.length || + preparedIdentities.some((identity) => !normalizedIdentities.has(identity)) + ) { throw new Error("prepare() must return a subset of normalized import accounts"); } const skippedByPrepare = Math.max(0, normalized.accounts.length - preparedNormalized.accounts.length); diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index b86525fd..936672b9 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -748,6 +748,9 @@ describe('Plugin Configuration', () => { const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue( JSON.stringify({ experimental: { syncFromCodexMultiAuth: { enabled: false } } }) as never ); + const openSpy = vi.spyOn(fs.promises, 'open').mockResolvedValue({ + close: vi.fn(async () => undefined), + } as never); const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined); const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); @@ -755,12 +758,14 @@ describe('Plugin Configuration', () => { try { await setSyncFromCodexMultiAuthEnabled(true); + expect(openSpy).toHaveBeenCalled(); expect(readSpy).toHaveBeenCalled(); expect(writeSpy).toHaveBeenCalled(); expect(renameSpy).toHaveBeenCalled(); - expect(unlinkSpy).not.toHaveBeenCalled(); + expect(unlinkSpy).toHaveBeenCalled(); } finally { readSpy.mockRestore(); + openSpy.mockRestore(); mkdirSpy.mockRestore(); writeSpy.mockRestore(); renameSpy.mockRestore(); diff --git a/test/storage.test.ts b/test/storage.test.ts index 41e7a543..66e21895 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -724,6 +724,28 @@ describe("storage", () => { ).rejects.toThrow(/prepare\(\) must return a subset/); }); + it("rejects prepare() results that rewrite import account identities", async () => { + await fs.writeFile( + exportPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }), + ); + + await expect( + importAccounts(exportPath, {}, (normalized) => ({ + ...normalized, + accounts: normalized.accounts.map((account) => ({ + ...account, + accountId: "rewritten", + })), + })), + ).rejects.toThrow(/prepare\(\) must return a subset/); + }); + it("continues import in best-effort mode when pre-import backup write is locked", async () => { await saveAccounts({ version: 3, From 787cfe7145125d29b682bd5feee9201e4e6d462d Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 20:20:47 +0800 Subject: [PATCH 07/10] fix: avoid flagged transaction double-read Reuse the loaded accounts snapshot in flagged transactions and cover the single-read path with a regression test. Co-authored-by: Codex --- lib/storage.ts | 3 ++- test/storage.test.ts | 51 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/lib/storage.ts b/lib/storage.ts index 22352b13..a2c510cc 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1151,7 +1151,8 @@ export async function withFlaggedAccountsTransaction( ) => Promise, ): Promise { return withStorageLock(async () => { - const current = await loadFlaggedAccountsUnlocked(); + const accountsSnapshot = await loadAccountsInternal(saveAccountsUnlocked); + const current = await loadFlaggedAccountsUnlocked(accountsSnapshot); return handler(current, saveFlaggedAccountsUnlocked); }); } diff --git a/test/storage.test.ts b/test/storage.test.ts index 66e21895..2768b1dc 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1784,6 +1784,57 @@ describe("storage", () => { } }); + it("reuses the loaded accounts snapshot inside withFlaggedAccountsTransaction", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + refreshToken: "account-refresh", + accountId: "account-id", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + refreshToken: "account-refresh", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + { + refreshToken: "orphan-refresh", + flaggedAt: 2, + addedAt: 2, + lastUsed: 2, + }, + ], + }); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile"); + let accountReadCount = 0; + readSpy.mockImplementation(async (path, options) => { + if (String(path) === testStoragePath) { + accountReadCount++; + } + return originalReadFile(path as Parameters[0], options as never); + }); + + try { + const result = await withFlaggedAccountsTransaction(async (current) => current); + expect(result.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + expect(accountReadCount).toBe(1); + } finally { + readSpy.mockRestore(); + } + }); + it("copies the raw accounts file for backup", async () => { await saveAccounts({ version: 3, From ed0b33c3c242a084db650321cbb428f0caf1b37d Mon Sep 17 00:00:00 2001 From: ndycode Date: Tue, 10 Mar 2026 23:57:57 +0800 Subject: [PATCH 08/10] fix: harden config mutation follow-ups Bound config lock retries, create the config directory before locking, and isolate tests around the lockfile and prune backup clone checks. Co-authored-by: Codex --- lib/config.ts | 14 ++++++++++++-- lib/storage.ts | 7 ++++++- test/plugin-config.test.ts | 6 ++++++ test/sync-prune-backup.test.ts | 2 +- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/config.ts b/lib/config.ts index ee1f8482..f808563b 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -17,6 +17,9 @@ const REQUEST_TRANSFORM_MODES = new Set(["native", "legacy"]); const UNSUPPORTED_CODEX_POLICIES = new Set(["strict", "fallback"]); const RETRY_PROFILES = new Set(["conservative", "balanced", "aggressive"]); const CONFIG_LOCK_PATH = `${CONFIG_PATH}.lock`; +const CONFIG_LOCK_RETRY_ATTEMPTS = 5; +const CONFIG_LOCK_RETRY_BASE_DELAY_MS = 10; +const CONFIG_LOCK_RETRY_MAX_DELAY_MS = 200; let configMutationMutex: Promise = Promise.resolve(); export type UnsupportedCodexPolicy = "strict" | "fallback"; @@ -130,7 +133,9 @@ function withConfigMutationLock(fn: () => Promise): Promise { async function withConfigProcessLock(fn: () => Promise): Promise { let lastError: NodeJS.ErrnoException | null = null; - for (let attempt = 0; attempt < 20; attempt += 1) { + await fs.mkdir(dirname(CONFIG_PATH), { recursive: true }); + + for (let attempt = 0; attempt < CONFIG_LOCK_RETRY_ATTEMPTS; attempt += 1) { try { const handle = await fs.open(CONFIG_LOCK_PATH, "wx", 0o600); try { @@ -143,7 +148,12 @@ async function withConfigProcessLock(fn: () => Promise): Promise { const code = (error as NodeJS.ErrnoException).code; if (code === "EEXIST") { lastError = error as NodeJS.ErrnoException; - await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + await new Promise((resolve) => + setTimeout( + resolve, + Math.min(CONFIG_LOCK_RETRY_BASE_DELAY_MS * 2 ** attempt, CONFIG_LOCK_RETRY_MAX_DELAY_MS), + ), + ); continue; } throw error; diff --git a/lib/storage.ts b/lib/storage.ts index a2c510cc..86f1c76c 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1364,7 +1364,12 @@ export async function importAccounts( backupError, } = await withAccountStorageTransaction(async (existing, persist) => { - const prepared = prepare ? prepare(normalized, existing) : normalized; + const prepared = prepare + ? prepare( + structuredClone(normalized), + existing ? structuredClone(existing) : existing, + ) + : normalized; const preparedNormalized = normalizeAccountStorage(prepared); if (!preparedNormalized) { throw new Error("prepare() returned invalid account storage"); diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 936672b9..c7b54900 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -776,9 +776,13 @@ describe('Plugin Configuration', () => { it('surfaces malformed config files instead of replacing them', async () => { mockExistsSync.mockReturnValue(true); const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue('{ invalid json' as never); + const openSpy = vi.spyOn(fs.promises, 'open').mockResolvedValue({ + close: vi.fn(async () => undefined), + } as never); const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined as never); const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); try { await expect(setSyncFromCodexMultiAuthEnabled(true)).rejects.toThrow(/Invalid JSON in config file/); @@ -786,9 +790,11 @@ describe('Plugin Configuration', () => { expect(renameSpy).not.toHaveBeenCalled(); } finally { readSpy.mockRestore(); + openSpy.mockRestore(); mkdirSpy.mockRestore(); writeSpy.mockRestore(); renameSpy.mockRestore(); + unlinkSpy.mockRestore(); } }); diff --git a/test/sync-prune-backup.test.ts b/test/sync-prune-backup.test.ts index a7102dd2..2c6ddb12 100644 --- a/test/sync-prune-backup.test.ts +++ b/test/sync-prune-backup.test.ts @@ -79,7 +79,7 @@ describe("sync prune backup payload", () => { const payload = createSyncPruneBackupPayload(storage, flagged); storage.accounts[0]!.accountTags?.push("mutated"); - storage.accounts[0]!.lastSelectedModelByFamily = { codex: "gpt-5.5" }; + storage.accounts[0]!.lastSelectedModelByFamily!.codex = "gpt-5.5"; flagged.accounts[0]!.metadata.source = "mutated"; expect(payload.accounts.accounts[0]?.accountTags).toEqual(["work"]); From 7a214dde6834924bdb94604c8aae376caf19390c Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 11 Mar 2026 00:44:35 +0800 Subject: [PATCH 09/10] fix: handle stale config lockfiles Retry through stale config lockfiles and cover first-run config-guard paths with focused tests. Co-authored-by: Codex --- lib/config.ts | 13 ++++++++ test/plugin-config.test.ts | 63 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/lib/config.ts b/lib/config.ts index f808563b..bf2adb71 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -20,6 +20,7 @@ const CONFIG_LOCK_PATH = `${CONFIG_PATH}.lock`; const CONFIG_LOCK_RETRY_ATTEMPTS = 5; const CONFIG_LOCK_RETRY_BASE_DELAY_MS = 10; const CONFIG_LOCK_RETRY_MAX_DELAY_MS = 200; +const CONFIG_LOCK_STALE_MS = 30_000; let configMutationMutex: Promise = Promise.resolve(); export type UnsupportedCodexPolicy = "strict" | "fallback"; @@ -147,6 +148,18 @@ async function withConfigProcessLock(fn: () => Promise): Promise { } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code === "EEXIST") { + try { + const stat = await fs.stat(CONFIG_LOCK_PATH); + if (Date.now() - stat.mtimeMs > CONFIG_LOCK_STALE_MS) { + await fs.unlink(CONFIG_LOCK_PATH).catch(() => undefined); + continue; + } + } catch (statError) { + const statCode = (statError as NodeJS.ErrnoException).code; + if (statCode === "ENOENT") { + continue; + } + } lastError = error as NodeJS.ErrnoException; await new Promise((resolve) => setTimeout( diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index c7b54900..2a8b891b 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -798,6 +798,69 @@ describe('Plugin Configuration', () => { } }); + it('surfaces non-object config files instead of replacing them', async () => { + mockExistsSync.mockReturnValue(true); + const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue('[]' as never); + const openSpy = vi.spyOn(fs.promises, 'open').mockResolvedValue({ + close: vi.fn(async () => undefined), + } as never); + const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined as never); + const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); + const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); + + try { + await expect(setSyncFromCodexMultiAuthEnabled(true)).rejects.toThrow(/Config file must contain a JSON object/); + expect(writeSpy).not.toHaveBeenCalled(); + expect(renameSpy).not.toHaveBeenCalled(); + } finally { + readSpy.mockRestore(); + openSpy.mockRestore(); + mkdirSpy.mockRestore(); + writeSpy.mockRestore(); + renameSpy.mockRestore(); + unlinkSpy.mockRestore(); + } + }); + + it('removes a stale lockfile before retrying config mutation', async () => { + mockExistsSync.mockReturnValue(true); + const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue( + JSON.stringify({ experimental: { syncFromCodexMultiAuth: { enabled: false } } }) as never + ); + const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined as never); + const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); + const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); + const closeSpy = vi.fn(async () => undefined); + const staleError = Object.assign(new Error('lock exists'), { code: 'EEXIST' }); + const openSpy = vi.spyOn(fs.promises, 'open') + .mockRejectedValueOnce(staleError as never) + .mockResolvedValue({ + close: closeSpy, + } as never); + const statSpy = vi.spyOn(fs.promises, 'stat').mockResolvedValue({ + mtimeMs: Date.now() - 31_000, + } as never); + + try { + await setSyncFromCodexMultiAuthEnabled(true); + expect(statSpy).toHaveBeenCalled(); + expect(openSpy).toHaveBeenCalledTimes(2); + expect(unlinkSpy).toHaveBeenCalled(); + expect(writeSpy).toHaveBeenCalled(); + expect(renameSpy).toHaveBeenCalled(); + } finally { + readSpy.mockRestore(); + mkdirSpy.mockRestore(); + writeSpy.mockRestore(); + renameSpy.mockRestore(); + unlinkSpy.mockRestore(); + openSpy.mockRestore(); + statSpy.mockRestore(); + } + }); + it('reads the persisted sync toggle flag', () => { expect(getSyncFromCodexMultiAuthEnabled({ experimental: { syncFromCodexMultiAuth: { enabled: true } } })).toBe(true); expect(getSyncFromCodexMultiAuthEnabled({})).toBe(false); From 21144919f4bfe1039e53242ed0521a5d75ac5914 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 16 Mar 2026 00:28:26 +0800 Subject: [PATCH 10/10] fix: close storage and config review gaps --- lib/config.ts | 13 +++++-- lib/storage.ts | 29 +++++++++++----- test/plugin-config.test.ts | 40 +++++++++++++++++++++ test/storage.test.ts | 71 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 12 deletions(-) diff --git a/lib/config.ts b/lib/config.ts index bf2adb71..968d6997 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -133,10 +133,11 @@ function withConfigMutationLock(fn: () => Promise): Promise { async function withConfigProcessLock(fn: () => Promise): Promise { let lastError: NodeJS.ErrnoException | null = null; + let attempt = 0; await fs.mkdir(dirname(CONFIG_PATH), { recursive: true }); - for (let attempt = 0; attempt < CONFIG_LOCK_RETRY_ATTEMPTS; attempt += 1) { + while (attempt < CONFIG_LOCK_RETRY_ATTEMPTS) { try { const handle = await fs.open(CONFIG_LOCK_PATH, "wx", 0o600); try { @@ -167,6 +168,7 @@ async function withConfigProcessLock(fn: () => Promise): Promise { Math.min(CONFIG_LOCK_RETRY_BASE_DELAY_MS * 2 ** attempt, CONFIG_LOCK_RETRY_MAX_DELAY_MS), ), ); + attempt += 1; continue; } throw error; @@ -178,7 +180,7 @@ async function withConfigProcessLock(fn: () => Promise): Promise { async function renameConfigWithWindowsRetry(sourcePath: string, destinationPath: string): Promise { let lastError: NodeJS.ErrnoException | null = null; - for (let attempt = 0; attempt < 5; attempt += 1) { + for (let attempt = 0; attempt < CONFIG_LOCK_RETRY_ATTEMPTS; attempt += 1) { try { await fs.rename(sourcePath, destinationPath); return; @@ -186,7 +188,12 @@ async function renameConfigWithWindowsRetry(sourcePath: string, destinationPath: const code = (error as NodeJS.ErrnoException).code; if (code === "EPERM" || code === "EBUSY") { lastError = error as NodeJS.ErrnoException; - await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + await new Promise((resolve) => + setTimeout( + resolve, + Math.min(CONFIG_LOCK_RETRY_BASE_DELAY_MS * 2 ** attempt, CONFIG_LOCK_RETRY_MAX_DELAY_MS), + ), + ); continue; } throw error; diff --git a/lib/storage.ts b/lib/storage.ts index 86f1c76c..6c3c674c 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1,4 +1,4 @@ -import { promises as fs, existsSync } from "node:fs"; +import { constants as fsConstants, promises as fs, existsSync } from "node:fs"; import { randomBytes } from "node:crypto"; import { dirname, join } from "node:path"; import { ACCOUNT_LIMITS } from "./constants.js"; @@ -159,12 +159,16 @@ async function renameWithWindowsRetry(sourcePath: string, destinationPath: strin } } -async function copyFileWithWindowsRetry(sourcePath: string, destinationPath: string): Promise { +async function copyFileWithWindowsRetry( + sourcePath: string, + destinationPath: string, + mode = 0, +): Promise { let lastError: NodeJS.ErrnoException | null = null; for (let attempt = 0; attempt < WINDOWS_RENAME_RETRY_ATTEMPTS; attempt += 1) { try { - await fs.copyFile(sourcePath, destinationPath); + await fs.copyFile(sourcePath, destinationPath, mode); return; } catch (error) { if (isWindowsLockError(error)) { @@ -1136,7 +1140,10 @@ async function loadFlaggedAccountsUnlocked( } export async function loadFlaggedAccounts(): Promise { - return withStorageLock(async () => loadFlaggedAccountsUnlocked()); + return withStorageLock(async () => { + const accountsSnapshot = await loadAccountsInternal(saveAccountsUnlocked); + return loadFlaggedAccountsUnlocked(accountsSnapshot); + }); } /** @@ -1291,10 +1298,7 @@ function previewImportAccountsAgainstExistingNormalized( export async function backupRawAccountsFile(filePath: string, force = true): Promise { await withStorageLock(async () => { const resolvedPath = resolvePath(filePath); - - if (!force && existsSync(resolvedPath)) { - throw new Error(`File already exists: ${resolvedPath}`); - } + const copyMode = force ? 0 : fsConstants.COPYFILE_EXCL; await migrateLegacyProjectStorageIfNeeded(saveAccountsUnlocked); const storagePath = getStoragePath(); @@ -1303,7 +1307,14 @@ export async function backupRawAccountsFile(filePath: string, force = true): Pro } await fs.mkdir(dirname(resolvedPath), { recursive: true }); - await copyFileWithWindowsRetry(storagePath, resolvedPath); + try { + await copyFileWithWindowsRetry(storagePath, resolvedPath, copyMode); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "EEXIST") { + throw new Error(`File already exists: ${resolvedPath}`); + } + throw error; + } await fs.chmod(resolvedPath, 0o600).catch((chmodErr) => { log.warn("Failed to restrict backup file permissions", { path: resolvedPath, diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 2a8b891b..554ab482 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -861,6 +861,46 @@ describe('Plugin Configuration', () => { } }); + it('does not consume retry budget when stale lock cleanup succeeds repeatedly', async () => { + mockExistsSync.mockReturnValue(true); + const readSpy = vi.spyOn(fs.promises, 'readFile').mockResolvedValue( + JSON.stringify({ experimental: { syncFromCodexMultiAuth: { enabled: false } } }) as never + ); + const mkdirSpy = vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined as never); + const writeSpy = vi.spyOn(fs.promises, 'writeFile').mockResolvedValue(undefined); + const renameSpy = vi.spyOn(fs.promises, 'rename').mockResolvedValue(undefined); + const unlinkSpy = vi.spyOn(fs.promises, 'unlink').mockResolvedValue(undefined); + const closeSpy = vi.fn(async () => undefined); + const staleError = Object.assign(new Error('lock exists'), { code: 'EEXIST' }); + const openSpy = vi.spyOn(fs.promises, 'open'); + for (let attempt = 0; attempt < 5; attempt += 1) { + openSpy.mockRejectedValueOnce(staleError as never); + } + openSpy.mockResolvedValue({ + close: closeSpy, + } as never); + const statSpy = vi.spyOn(fs.promises, 'stat').mockResolvedValue({ + mtimeMs: Date.now() - 31_000, + } as never); + + try { + await setSyncFromCodexMultiAuthEnabled(true); + expect(statSpy).toHaveBeenCalledTimes(5); + expect(openSpy).toHaveBeenCalledTimes(6); + expect(unlinkSpy).toHaveBeenCalledTimes(6); + expect(writeSpy).toHaveBeenCalled(); + expect(renameSpy).toHaveBeenCalled(); + } finally { + readSpy.mockRestore(); + mkdirSpy.mockRestore(); + writeSpy.mockRestore(); + renameSpy.mockRestore(); + unlinkSpy.mockRestore(); + openSpy.mockRestore(); + statSpy.mockRestore(); + } + }); + it('reads the persisted sync toggle flag', () => { expect(getSyncFromCodexMultiAuthEnabled({ experimental: { syncFromCodexMultiAuth: { enabled: true } } })).toBe(true); expect(getSyncFromCodexMultiAuthEnabled({})).toBe(false); diff --git a/test/storage.test.ts b/test/storage.test.ts index 2768b1dc..2082fdb6 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1784,6 +1784,57 @@ describe("storage", () => { } }); + it("reuses the loaded accounts snapshot inside loadFlaggedAccounts", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + refreshToken: "account-refresh", + accountId: "account-id", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await saveFlaggedAccounts({ + version: 1, + accounts: [ + { + refreshToken: "account-refresh", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + { + refreshToken: "orphan-refresh", + flaggedAt: 2, + addedAt: 2, + lastUsed: 2, + }, + ], + }); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile"); + let accountReadCount = 0; + readSpy.mockImplementation(async (path, options) => { + if (String(path) === testStoragePath) { + accountReadCount++; + } + return originalReadFile(path as Parameters[0], options as never); + }); + + try { + const flagged = await loadFlaggedAccounts(); + expect(flagged.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]); + expect(accountReadCount).toBe(1); + } finally { + readSpy.mockRestore(); + } + }); + it("reuses the loaded accounts snapshot inside withFlaggedAccountsTransaction", async () => { await saveAccounts({ version: 3, @@ -1856,6 +1907,26 @@ describe("storage", () => { const raw = JSON.parse(await fs.readFile(backupPath, "utf-8")) as { accounts: Array<{ refreshToken: string }> }; expect(raw.accounts[0]?.refreshToken).toBe("backup-refresh"); }); + + it("surfaces a copy-time EEXIST when backing up raw accounts without force", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [{ accountId: "existing", refreshToken: "ref-existing", addedAt: 1, lastUsed: 1 }], + }); + + const backupPath = join(testWorkDir, "raw-backup-race.json"); + const copySpy = vi.spyOn(fs, "copyFile").mockRejectedValue( + Object.assign(new Error("exists"), { code: "EEXIST" }) as never, + ); + + try { + await expect(backupRawAccountsFile(backupPath, false)).rejects.toThrow(/File already exists/); + } finally { + copySpy.mockRestore(); + } + }); }); describe("setStoragePath", () => {