From cdb4a37004d3e06978d9e3e020eef5c6f93139b6 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:19:35 +0800 Subject: [PATCH 01/10] fix(storage): fail export when current pool is empty --- lib/storage.ts | 57 ++++++++++++++++++- lib/storage/account-port.ts | 4 +- test/account-port.test.ts | 61 +++++++++++++++++++-- test/experimental-sync-target-entry.test.ts | 2 +- 4 files changed, 113 insertions(+), 11 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 1a6e0585..fcb6f5e3 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1432,6 +1432,58 @@ async function loadAccountsInternal( } } +async function loadAccountsForExport(): Promise { + const path = getStoragePath(); + const resetMarkerPath = getIntentionalResetMarkerPath(path); + const migratedLegacyStorage = + await migrateLegacyProjectStorageIfNeeded(saveAccountsUnlocked); + + if (existsSync(resetMarkerPath)) { + return createEmptyStorageWithMetadata(false, "intentional-reset"); + } + + try { + const { normalized, storedVersion, schemaErrors } = await loadAccountsFromPath( + path, + { + normalizeAccountStorage, + isRecord, + }, + ); + if (schemaErrors.length > 0) { + log.warn("Account storage schema validation warnings", { + errors: schemaErrors.slice(0, 5), + }); + } + if (normalized && storedVersion !== normalized.version) { + log.info("Migrating account storage to v3", { + from: storedVersion, + to: normalized.version, + }); + try { + await saveAccountsUnlocked(normalized); + } catch (saveError) { + log.warn("Failed to persist migrated storage", { + error: String(saveError), + }); + } + } + if (existsSync(resetMarkerPath)) { + return createEmptyStorageWithMetadata(false, "intentional-reset"); + } + return normalized; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (existsSync(resetMarkerPath)) { + return createEmptyStorageWithMetadata(false, "intentional-reset"); + } + if (code === "ENOENT") { + return migratedLegacyStorage; + } + throw error; + } +} + async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { const path = getStoragePath(); const resetMarkerPath = getIntentionalResetMarkerPath(path); @@ -1788,9 +1840,8 @@ export async function exportAccounts( force, currentStoragePath, transactionState: getTransactionSnapshotState(), - loadAccountsInternal: () => loadAccountsInternal(saveAccountsUnlocked), - readCurrentStorage: () => - withAccountStorageTransaction((current) => Promise.resolve(current)), + readCurrentStorageUnlocked: loadAccountsForExport, + readCurrentStorage: () => withStorageLock(loadAccountsForExport), exportAccountsToFile, beforeCommit, logInfo: (message, details) => { diff --git a/lib/storage/account-port.ts b/lib/storage/account-port.ts index b14f29f5..814b1ce6 100644 --- a/lib/storage/account-port.ts +++ b/lib/storage/account-port.ts @@ -11,7 +11,7 @@ export async function exportAccountsSnapshot(params: { snapshot: AccountStorageV3 | null; } | undefined; - loadAccountsInternal: () => Promise; + readCurrentStorageUnlocked: () => Promise; readCurrentStorage: () => Promise; exportAccountsToFile: (args: { resolvedPath: string; @@ -28,7 +28,7 @@ export async function exportAccountsSnapshot(params: { params.transactionState.storagePath === params.currentStoragePath ? params.transactionState.snapshot : params.transactionState?.active - ? await params.loadAccountsInternal() + ? await params.readCurrentStorageUnlocked() : await params.readCurrentStorage(); await params.exportAccountsToFile({ diff --git a/test/account-port.test.ts b/test/account-port.test.ts index 4dc3ece1..8d459a8e 100644 --- a/test/account-port.test.ts +++ b/test/account-port.test.ts @@ -5,8 +5,17 @@ import { } from "../lib/storage/account-port.js"; describe("account port helpers", () => { - it("exports transaction snapshot when active", async () => { + it("exports transaction snapshot when active for the current storage path", async () => { const exportAccountsToFile = vi.fn(async () => undefined); + const snapshot = { + version: 3 as const, + accounts: [{ refreshToken: "snapshot-token" }], + activeIndex: 0, + activeIndexByFamily: {}, + }; + const readCurrentStorageUnlocked = vi.fn(); + const readCurrentStorage = vi.fn(); + await exportAccountsSnapshot({ resolvedPath: "/tmp/out.json", force: true, @@ -14,19 +23,61 @@ describe("account port helpers", () => { transactionState: { active: true, storagePath: "/tmp/accounts.json", + snapshot, + }, + readCurrentStorageUnlocked, + readCurrentStorage, + exportAccountsToFile, + logInfo: vi.fn(), + }); + expect(readCurrentStorageUnlocked).not.toHaveBeenCalled(); + expect(readCurrentStorage).not.toHaveBeenCalled(); + expect(exportAccountsToFile).toHaveBeenCalledWith( + expect.objectContaining({ + storage: snapshot, + }), + ); + }); + + it("reads current storage without reusing a stale transaction snapshot from another path", async () => { + const exportAccountsToFile = vi.fn(async () => undefined); + const readCurrentStorageUnlocked = vi.fn(async () => ({ + version: 3 as const, + accounts: [{ refreshToken: "live-token" }], + activeIndex: 0, + activeIndexByFamily: {}, + })); + const readCurrentStorage = vi.fn(); + + await exportAccountsSnapshot({ + resolvedPath: "/tmp/out.json", + force: true, + currentStoragePath: "/tmp/accounts.json", + transactionState: { + active: true, + storagePath: "/tmp/other.json", snapshot: { version: 3, - accounts: [], + accounts: [{ refreshToken: "stale-token" }], activeIndex: 0, activeIndexByFamily: {}, }, }, - loadAccountsInternal: vi.fn(), - readCurrentStorage: vi.fn(), + readCurrentStorageUnlocked, + readCurrentStorage, exportAccountsToFile, logInfo: vi.fn(), }); - expect(exportAccountsToFile).toHaveBeenCalled(); + + expect(readCurrentStorageUnlocked).toHaveBeenCalledTimes(1); + expect(readCurrentStorage).not.toHaveBeenCalled(); + expect(exportAccountsToFile).toHaveBeenCalledWith( + expect.objectContaining({ + storage: expect.objectContaining({ + accounts: [{ refreshToken: "live-token" }], + }), + }), + ); }); it("imports through transaction helper and logs result", async () => { diff --git a/test/experimental-sync-target-entry.test.ts b/test/experimental-sync-target-entry.test.ts index cd4e4e22..f62c727a 100644 --- a/test/experimental-sync-target-entry.test.ts +++ b/test/experimental-sync-target-entry.test.ts @@ -67,7 +67,7 @@ describe("experimental sync target entry", () => { expect(capturedReadJson).toBeDefined(); expect(readFileWithRetry).toHaveBeenCalledWith("C:\\state.json", { - retryableCodes: new Set(["EBUSY", "EPERM", "EAGAIN", "ENOTEMPTY", "EACCES"]), + retryableCodes: new Set(["EBUSY", "EPERM", "EAGAIN"]), maxAttempts: 4, sleep, }); From c95016d4348b417233a89a94b52029a56fe7b25e Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 17:52:46 +0800 Subject: [PATCH 02/10] fix(storage): harden export review followups --- lib/storage.ts | 98 ++++++++------ test/account-port.test.ts | 32 +++++ test/experimental-sync-target-entry.test.ts | 63 ++++++++- test/storage.test.ts | 138 ++++++++++++++++++++ 4 files changed, 283 insertions(+), 48 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index fcb6f5e3..d0037569 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -709,9 +709,12 @@ function getLegacyFlaggedAccountsPath(): string { ); } -async function migrateLegacyProjectStorageIfNeeded( - persist: (storage: AccountStorageV3) => Promise = saveAccounts, -): Promise { +async function migrateLegacyProjectStorageIfNeeded(options?: { + persist?: (storage: AccountStorageV3) => Promise; + commit?: boolean; +}): Promise { + const persist = options?.persist ?? saveAccounts; + const commit = options?.commit ?? true; const state = getStoragePathState(); if (!state.currentStoragePath) { return null; @@ -782,43 +785,49 @@ async function migrateLegacyProjectStorageIfNeeded( ); const fallbackStorage = targetStorage ?? legacyStorage; - try { - await persist(mergedStorage); - targetStorage = mergedStorage; - migrated = true; - } catch (error) { - targetStorage = fallbackStorage; - log.warn("Failed to persist migrated account storage", { + if (commit) { + try { + await persist(mergedStorage); + targetStorage = mergedStorage; + migrated = true; + } catch (error) { + targetStorage = fallbackStorage; + log.warn("Failed to persist migrated account storage", { + from: legacyPath, + to: state.currentStoragePath, + error: String(error), + }); + continue; + } + + try { + await fs.unlink(legacyPath); + log.info("Removed legacy account storage file after migration", { + path: legacyPath, + }); + } catch (unlinkError) { + const code = (unlinkError as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn( + "Failed to remove legacy account storage file after migration", + { + path: legacyPath, + error: String(unlinkError), + }, + ); + } + } + + log.info("Migrated legacy project account storage", { from: legacyPath, to: state.currentStoragePath, - error: String(error), + accounts: mergedStorage.accounts.length, }); continue; } - try { - await fs.unlink(legacyPath); - log.info("Removed legacy account storage file after migration", { - path: legacyPath, - }); - } catch (unlinkError) { - const code = (unlinkError as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - log.warn( - "Failed to remove legacy account storage file after migration", - { - path: legacyPath, - error: String(unlinkError), - }, - ); - } - } - - log.info("Migrated legacy project account storage", { - from: legacyPath, - to: state.currentStoragePath, - accounts: mergedStorage.accounts.length, - }); + targetStorage = mergedStorage; + migrated = true; } if (migrated) { @@ -1263,7 +1272,7 @@ async function loadAccountsInternal( const resetMarkerPath = getIntentionalResetMarkerPath(path); await cleanupStaleRotatingBackupArtifacts(path); const migratedLegacyStorage = persistMigration - ? await migrateLegacyProjectStorageIfNeeded(persistMigration) + ? await migrateLegacyProjectStorageIfNeeded({ persist: persistMigration }) : null; try { @@ -1432,11 +1441,17 @@ async function loadAccountsInternal( } } -async function loadAccountsForExport(): Promise { +async function loadAccountsForExport(options?: { + persistMigrations?: boolean; +}): Promise { + const persistMigrations = options?.persistMigrations ?? true; const path = getStoragePath(); const resetMarkerPath = getIntentionalResetMarkerPath(path); - const migratedLegacyStorage = - await migrateLegacyProjectStorageIfNeeded(saveAccountsUnlocked); + const migratedLegacyStorage = await migrateLegacyProjectStorageIfNeeded( + persistMigrations + ? { persist: saveAccountsUnlocked } + : { commit: false }, + ); if (existsSync(resetMarkerPath)) { return createEmptyStorageWithMetadata(false, "intentional-reset"); @@ -1455,7 +1470,7 @@ async function loadAccountsForExport(): Promise { errors: schemaErrors.slice(0, 5), }); } - if (normalized && storedVersion !== normalized.version) { + if (persistMigrations && normalized && storedVersion !== normalized.version) { log.info("Migrating account storage to v3", { from: storedVersion, to: normalized.version, @@ -1840,8 +1855,9 @@ export async function exportAccounts( force, currentStoragePath, transactionState: getTransactionSnapshotState(), - readCurrentStorageUnlocked: loadAccountsForExport, - readCurrentStorage: () => withStorageLock(loadAccountsForExport), + readCurrentStorageUnlocked: () => + loadAccountsForExport({ persistMigrations: false }), + readCurrentStorage: () => withStorageLock(() => loadAccountsForExport()), exportAccountsToFile, beforeCommit, logInfo: (message, details) => { diff --git a/test/account-port.test.ts b/test/account-port.test.ts index 8d459a8e..825fd390 100644 --- a/test/account-port.test.ts +++ b/test/account-port.test.ts @@ -80,6 +80,38 @@ describe("account port helpers", () => { ); }); + it("reads current storage via the locked reader when no transaction is active", async () => { + const exportAccountsToFile = vi.fn(async () => undefined); + const readCurrentStorageUnlocked = vi.fn(); + const readCurrentStorage = vi.fn(async () => ({ + version: 3 as const, + accounts: [{ refreshToken: "locked-read-token" }], + activeIndex: 0, + activeIndexByFamily: {}, + })); + + await exportAccountsSnapshot({ + resolvedPath: "/tmp/out.json", + force: true, + currentStoragePath: "/tmp/accounts.json", + transactionState: undefined, + readCurrentStorageUnlocked, + readCurrentStorage, + exportAccountsToFile, + logInfo: vi.fn(), + }); + + expect(readCurrentStorageUnlocked).not.toHaveBeenCalled(); + expect(readCurrentStorage).toHaveBeenCalledTimes(1); + expect(exportAccountsToFile).toHaveBeenCalledWith( + expect.objectContaining({ + storage: expect.objectContaining({ + accounts: [{ refreshToken: "locked-read-token" }], + }), + }), + ); + }); + it("imports through transaction helper and logs result", async () => { const result = await importAccountsSnapshot({ resolvedPath: "/tmp/in.json", diff --git a/test/experimental-sync-target-entry.test.ts b/test/experimental-sync-target-entry.test.ts index f62c727a..00b4aac9 100644 --- a/test/experimental-sync-target-entry.test.ts +++ b/test/experimental-sync-target-entry.test.ts @@ -40,9 +40,27 @@ describe("experimental sync target entry", () => { }); }); - it("wires windows-safe retry options through readJson", async () => { + it("provides windows lock retry policy to the injected reader", async () => { const sleep = vi.fn(async () => undefined); - const readFileWithRetry = vi.fn(async () => '{"hello":"world"}'); + const retryAttempts: string[] = []; + const readFileWithRetry = vi.fn(async (_path, options) => { + expect(options.retryableCodes.has("EBUSY")).toBe(true); + expect(options.retryableCodes.has("EPERM")).toBe(true); + expect(options.retryableCodes.has("EAGAIN")).toBe(true); + expect(options.retryableCodes.has("EACCES")).toBe(false); + expect(options.retryableCodes.has("ENOTEMPTY")).toBe(false); + expect(options.maxAttempts).toBe(4); + + while (retryAttempts.length < 2) { + retryAttempts.push("EBUSY"); + if (!options.retryableCodes.has("EBUSY")) { + throw Object.assign(new Error("busy"), { code: "EBUSY" }); + } + await options.sleep(25 * retryAttempts.length); + } + + return '{"hello":"world"}'; + }); const normalizeAccountStorage = vi.fn(() => null); let capturedReadJson: ((path: string) => Promise) | undefined; @@ -66,14 +84,45 @@ describe("experimental sync target entry", () => { }); expect(capturedReadJson).toBeDefined(); - expect(readFileWithRetry).toHaveBeenCalledWith("C:\\state.json", { - retryableCodes: new Set(["EBUSY", "EPERM", "EAGAIN"]), - maxAttempts: 4, - sleep, - }); + expect(readFileWithRetry).toHaveBeenCalledTimes(1); + expect(sleep).toHaveBeenNthCalledWith(1, 25); + expect(sleep).toHaveBeenNthCalledWith(2, 50); expect(normalizeAccountStorage).toHaveBeenCalledWith({ hello: "world" }); }); + it("propagates fail-fast lock errors that are outside the retryable set", async () => { + const sleep = vi.fn(async () => undefined); + const readFileWithRetry = vi.fn(async (_path, options) => { + expect(options.retryableCodes.has("EACCES")).toBe(false); + expect(options.retryableCodes.has("ENOTEMPTY")).toBe(false); + throw Object.assign(new Error("denied"), { code: "EACCES" }); + }); + const normalizeAccountStorage = vi.fn(() => null); + + const loadExperimentalSyncTargetState = vi.fn(async (args) => { + await args.readJson("C:\\state.json"); + return { + kind: "target" as const, + detection: { kind: "target" as const }, + destination: null, + }; + }); + + await expect( + loadExperimentalSyncTargetEntry({ + loadExperimentalSyncTargetState, + detectTarget: createDetectedTarget, + readFileWithRetry, + normalizeAccountStorage, + sleep, + }), + ).rejects.toMatchObject({ code: "EACCES" }); + + expect(readFileWithRetry).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + expect(normalizeAccountStorage).not.toHaveBeenCalled(); + }); + it("propagates malformed json parse failures to the caller", async () => { const readFileWithRetry = vi.fn(async () => "not-valid-json{{{"); const normalizeAccountStorage = vi.fn(() => null); diff --git a/test/storage.test.ts b/test/storage.test.ts index 4903d606..6fe40516 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js"; +import { setStoragePathState } from "../lib/storage/path-state.js"; import { buildNamedBackupPath, clearAccounts, @@ -1297,6 +1298,143 @@ describe("storage", () => { } }); + it("exports legacy-migrated storage without persisting it during another storage transaction", async () => { + const transactionStoragePath = join(testWorkDir, "accounts-transaction.json"); + const currentStoragePath = join(testWorkDir, "accounts-current.json"); + const legacyStoragePath = join(testWorkDir, "accounts-legacy.json"); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(transactionStoragePath); + try { + await withAccountStorageTransaction(async () => { + setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + await exportAccounts(exportPath); + }); + + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts).toEqual([ + expect.objectContaining({ refreshToken: "legacy-token" }), + ]); + expect(existsSync(currentStoragePath)).toBe(false); + expect(existsSync(legacyStoragePath)).toBe(true); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + + it("does not persist v3 normalization while export reads storage unlocked", async () => { + const transactionStoragePath = join(testWorkDir, "accounts-transaction.json"); + const currentStoragePath = join(testWorkDir, "accounts-v1.json"); + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 1, + activeIndex: 0, + accounts: [ + { + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(transactionStoragePath); + try { + await withAccountStorageTransaction(async () => { + setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: null, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + await exportAccounts(exportPath); + }); + + const onDisk = JSON.parse( + await fs.readFile(currentStoragePath, "utf-8"), + ); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(onDisk.version).toBe(1); + expect(exported.version).toBe(3); + expect(exported.accounts).toEqual([ + expect.objectContaining({ refreshToken: "legacy-token" }), + ]); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + + it.each(["EBUSY", "EPERM"] as const)( + "rethrows %s when export cannot read the current storage file", + async (code) => { + const lockedStoragePath = join(testWorkDir, `accounts-${code}.json`); + await fs.writeFile( + lockedStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "locked", + refreshToken: "locked-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + const actualStorageParser = await vi.importActual< + typeof import("../lib/storage/storage-parser.js") + >("../lib/storage/storage-parser.js"); + vi.resetModules(); + vi.doMock("../lib/storage/storage-parser.js", () => ({ + ...actualStorageParser, + loadAccountsFromPath: vi.fn(async (path, deps) => { + if (path === lockedStoragePath) { + throw Object.assign(new Error(`locked ${code}`), { code }); + } + return actualStorageParser.loadAccountsFromPath(path, deps); + }), + })); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + isolatedStorageModule.setStoragePathDirect(lockedStoragePath); + await expect( + isolatedStorageModule.exportAccounts(exportPath), + ).rejects.toMatchObject({ code }); + } finally { + vi.doUnmock("../lib/storage/storage-parser.js"); + vi.resetModules(); + setStoragePathDirect(testStoragePath); + } + }, + ); + it("should fail import when file does not exist", async () => { const { importAccounts } = await import("../lib/storage.js"); const nonexistentPath = join(testWorkDir, "nonexistent-file.json"); From 3756bd9363c7481f630dd38e303eb92d3631d064 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:03:24 +0800 Subject: [PATCH 03/10] fix(storage): clarify locked export migration path --- lib/storage.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index d0037569..47e8f9f4 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1441,10 +1441,13 @@ async function loadAccountsInternal( } } -async function loadAccountsForExport(options?: { - persistMigrations?: boolean; -}): Promise { - const persistMigrations = options?.persistMigrations ?? true; +async function loadAccountsForExport( + mode: "locked" | "unlocked" = "locked", +): Promise { + // Export reuses this helper from both paths in `exportAccounts()`. Only the + // locked path may persist migrations; unlocked reads must remain side-effect + // free while another storage transaction holds the mutex for a different file. + const persistMigrations = mode === "locked"; const path = getStoragePath(); const resetMarkerPath = getIntentionalResetMarkerPath(path); const migratedLegacyStorage = await migrateLegacyProjectStorageIfNeeded( @@ -1855,9 +1858,9 @@ export async function exportAccounts( force, currentStoragePath, transactionState: getTransactionSnapshotState(), - readCurrentStorageUnlocked: () => - loadAccountsForExport({ persistMigrations: false }), - readCurrentStorage: () => withStorageLock(() => loadAccountsForExport()), + readCurrentStorageUnlocked: () => loadAccountsForExport("unlocked"), + readCurrentStorage: () => + withStorageLock(() => loadAccountsForExport("locked")), exportAccountsToFile, beforeCommit, logInfo: (message, details) => { From 05c201ffb72dd252e8b0aa37d34b402500c28168 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:27:44 +0800 Subject: [PATCH 04/10] fix(storage): defer export legacy migration --- lib/storage.ts | 13 +++-- test/storage.test.ts | 118 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 47e8f9f4..35ad67a4 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1450,11 +1450,9 @@ async function loadAccountsForExport( const persistMigrations = mode === "locked"; const path = getStoragePath(); const resetMarkerPath = getIntentionalResetMarkerPath(path); - const migratedLegacyStorage = await migrateLegacyProjectStorageIfNeeded( - persistMigrations - ? { persist: saveAccountsUnlocked } - : { commit: false }, - ); + const migrationOptions = persistMigrations + ? { persist: saveAccountsUnlocked } + : { commit: false }; if (existsSync(resetMarkerPath)) { return createEmptyStorageWithMetadata(false, "intentional-reset"); @@ -1496,6 +1494,11 @@ async function loadAccountsForExport( return createEmptyStorageWithMetadata(false, "intentional-reset"); } if (code === "ENOENT") { + const migratedLegacyStorage = + await migrateLegacyProjectStorageIfNeeded(migrationOptions); + if (existsSync(resetMarkerPath)) { + return createEmptyStorageWithMetadata(false, "intentional-reset"); + } return migratedLegacyStorage; } throw error; diff --git a/test/storage.test.ts b/test/storage.test.ts index 6fe40516..41d94419 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1234,7 +1234,6 @@ describe("storage", () => { }); it("should fail export when no accounts exist", async () => { - const { exportAccounts } = await import("../lib/storage.js"); setStoragePathDirect(testStoragePath); await clearAccounts(); await expect(exportAccounts(exportPath)).rejects.toThrow( @@ -1302,6 +1301,22 @@ describe("storage", () => { const transactionStoragePath = join(testWorkDir, "accounts-transaction.json"); const currentStoragePath = join(testWorkDir, "accounts-current.json"); const legacyStoragePath = join(testWorkDir, "accounts-legacy.json"); + await fs.writeFile( + transactionStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "transaction", + refreshToken: "transaction-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); await fs.writeFile( legacyStoragePath, JSON.stringify({ @@ -1332,9 +1347,15 @@ describe("storage", () => { }); const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + const transactionStorage = JSON.parse( + await fs.readFile(transactionStoragePath, "utf-8"), + ); expect(exported.accounts).toEqual([ expect.objectContaining({ refreshToken: "legacy-token" }), ]); + expect(transactionStorage.accounts).toEqual([ + expect.objectContaining({ refreshToken: "transaction-token" }), + ]); expect(existsSync(currentStoragePath)).toBe(false); expect(existsSync(legacyStoragePath)).toBe(true); } finally { @@ -1435,6 +1456,101 @@ describe("storage", () => { }, ); + it("does not revive legacy accounts when the current storage exists but is empty", async () => { + const currentStoragePath = join(testWorkDir, "accounts-empty-current.json"); + const legacyStoragePath = join(testWorkDir, "accounts-empty-legacy.json"); + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [], + }), + ); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(currentStoragePath); + try { + setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await expect(exportAccounts(exportPath)).rejects.toThrow( + /No accounts to export/, + ); + + const currentStorage = JSON.parse( + await fs.readFile(currentStoragePath, "utf-8"), + ); + expect(currentStorage.accounts).toEqual([]); + expect(existsSync(legacyStoragePath)).toBe(true); + expect(existsSync(exportPath)).toBe(false); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + + it("does not revive legacy accounts when the current storage has an intentional reset marker", async () => { + const currentStoragePath = join(testWorkDir, "accounts-reset-current.json"); + const legacyStoragePath = join(testWorkDir, "accounts-reset-legacy.json"); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(currentStoragePath); + await clearAccounts(); + try { + setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await expect(exportAccounts(exportPath)).rejects.toThrow( + /No accounts to export/, + ); + + expect(existsSync(currentStoragePath)).toBe(false); + expect(existsSync(legacyStoragePath)).toBe(true); + expect(existsSync(exportPath)).toBe(false); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + it("should fail import when file does not exist", async () => { const { importAccounts } = await import("../lib/storage.js"); const nonexistentPath = join(testWorkDir, "nonexistent-file.json"); From f07b31dab8c57a90ec005893ac10cf75d5aa441a Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:55:54 +0800 Subject: [PATCH 05/10] fix(storage): keep export reads side-effect free --- lib/storage.ts | 45 +++--------- test/storage.test.ts | 163 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 34 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 35ad67a4..dbc0fc59 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1441,49 +1441,27 @@ async function loadAccountsInternal( } } -async function loadAccountsForExport( - mode: "locked" | "unlocked" = "locked", -): Promise { - // Export reuses this helper from both paths in `exportAccounts()`. Only the - // locked path may persist migrations; unlocked reads must remain side-effect - // free while another storage transaction holds the mutex for a different file. - const persistMigrations = mode === "locked"; +async function loadAccountsForExport(): Promise { + // Export reuses this helper from both paths in `exportAccounts()`. Keep the + // read side effect free so export never clears a reset marker or races with + // concurrent writers while normalizing legacy storage for the snapshot. const path = getStoragePath(); const resetMarkerPath = getIntentionalResetMarkerPath(path); - const migrationOptions = persistMigrations - ? { persist: saveAccountsUnlocked } - : { commit: false }; if (existsSync(resetMarkerPath)) { return createEmptyStorageWithMetadata(false, "intentional-reset"); } try { - const { normalized, storedVersion, schemaErrors } = await loadAccountsFromPath( - path, - { - normalizeAccountStorage, - isRecord, - }, - ); + const { normalized, schemaErrors } = await loadAccountsFromPath(path, { + normalizeAccountStorage, + isRecord, + }); if (schemaErrors.length > 0) { log.warn("Account storage schema validation warnings", { errors: schemaErrors.slice(0, 5), }); } - if (persistMigrations && normalized && storedVersion !== normalized.version) { - log.info("Migrating account storage to v3", { - from: storedVersion, - to: normalized.version, - }); - try { - await saveAccountsUnlocked(normalized); - } catch (saveError) { - log.warn("Failed to persist migrated storage", { - error: String(saveError), - }); - } - } if (existsSync(resetMarkerPath)) { return createEmptyStorageWithMetadata(false, "intentional-reset"); } @@ -1495,7 +1473,7 @@ async function loadAccountsForExport( } if (code === "ENOENT") { const migratedLegacyStorage = - await migrateLegacyProjectStorageIfNeeded(migrationOptions); + await migrateLegacyProjectStorageIfNeeded({ commit: false }); if (existsSync(resetMarkerPath)) { return createEmptyStorageWithMetadata(false, "intentional-reset"); } @@ -1861,9 +1839,8 @@ export async function exportAccounts( force, currentStoragePath, transactionState: getTransactionSnapshotState(), - readCurrentStorageUnlocked: () => loadAccountsForExport("unlocked"), - readCurrentStorage: () => - withStorageLock(() => loadAccountsForExport("locked")), + readCurrentStorageUnlocked: () => loadAccountsForExport(), + readCurrentStorage: () => withStorageLock(() => loadAccountsForExport()), exportAccountsToFile, beforeCommit, logInfo: (message, details) => { diff --git a/test/storage.test.ts b/test/storage.test.ts index 41d94419..86fec453 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1407,6 +1407,41 @@ describe("storage", () => { } }); + it("does not persist v3 normalization while export reads storage with the lock", async () => { + const currentStoragePath = join(testWorkDir, "accounts-v1-locked.json"); + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 1, + activeIndex: 0, + accounts: [ + { + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(currentStoragePath); + try { + await exportAccounts(exportPath); + + const onDisk = JSON.parse( + await fs.readFile(currentStoragePath, "utf-8"), + ); + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(onDisk.version).toBe(1); + expect(exported.version).toBe(3); + expect(exported.accounts).toEqual([ + expect.objectContaining({ refreshToken: "legacy-token" }), + ]); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + it.each(["EBUSY", "EPERM"] as const)( "rethrows %s when export cannot read the current storage file", async (code) => { @@ -1456,6 +1491,92 @@ describe("storage", () => { }, ); + it.each(["EBUSY", "EPERM"] as const)( + "does not write an export file when %s happens while reading another storage path during a transaction", + async (code) => { + const transactionStoragePath = join( + testWorkDir, + `accounts-transaction-${code}.json`, + ); + const currentStoragePath = join(testWorkDir, `accounts-live-${code}.json`); + await fs.writeFile( + transactionStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "transaction", + refreshToken: "transaction-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "live", + refreshToken: "live-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + const actualStorageParser = await vi.importActual< + typeof import("../lib/storage/storage-parser.js") + >("../lib/storage/storage-parser.js"); + vi.resetModules(); + vi.doMock("../lib/storage/storage-parser.js", () => ({ + ...actualStorageParser, + loadAccountsFromPath: vi.fn(async (path, deps) => { + if (path === currentStoragePath) { + throw Object.assign(new Error(`locked ${code}`), { code }); + } + return actualStorageParser.loadAccountsFromPath(path, deps); + }), + })); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + const isolatedPathState = await import("../lib/storage/path-state.js"); + isolatedStorageModule.setStoragePathDirect(transactionStoragePath); + await expect( + isolatedStorageModule.withAccountStorageTransaction(async () => { + isolatedPathState.setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: null, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + await isolatedStorageModule.exportAccounts(exportPath); + }), + ).rejects.toMatchObject({ code }); + + const transactionStorage = JSON.parse( + await fs.readFile(transactionStoragePath, "utf-8"), + ); + expect(transactionStorage.accounts).toEqual([ + expect.objectContaining({ refreshToken: "transaction-token" }), + ]); + expect(existsSync(exportPath)).toBe(false); + } finally { + vi.doUnmock("../lib/storage/storage-parser.js"); + vi.resetModules(); + setStoragePathDirect(testStoragePath); + } + }, + ); + it("does not revive legacy accounts when the current storage exists but is empty", async () => { const currentStoragePath = join(testWorkDir, "accounts-empty-current.json"); const legacyStoragePath = join(testWorkDir, "accounts-empty-legacy.json"); @@ -1509,6 +1630,48 @@ describe("storage", () => { } }); + it("exports legacy storage without persisting it when current storage is missing", async () => { + const currentStoragePath = join(testWorkDir, "accounts-missing-current.json"); + const legacyStoragePath = join(testWorkDir, "accounts-missing-legacy.json"); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + setStoragePathDirect(currentStoragePath); + try { + setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await exportAccounts(exportPath); + + const exported = JSON.parse(await fs.readFile(exportPath, "utf-8")); + expect(exported.accounts).toEqual([ + expect.objectContaining({ refreshToken: "legacy-token" }), + ]); + expect(existsSync(currentStoragePath)).toBe(false); + expect(existsSync(legacyStoragePath)).toBe(true); + } finally { + setStoragePathDirect(testStoragePath); + } + }); + it("does not revive legacy accounts when the current storage has an intentional reset marker", async () => { const currentStoragePath = join(testWorkDir, "accounts-reset-current.json"); const legacyStoragePath = join(testWorkDir, "accounts-reset-legacy.json"); From 3600dc834bf891e91ea9d722b36229899cf13282 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 19:36:17 +0800 Subject: [PATCH 06/10] test(storage): cover eagain export lock reads --- test/storage.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/storage.test.ts b/test/storage.test.ts index 86fec453..46014b87 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1442,7 +1442,7 @@ describe("storage", () => { } }); - it.each(["EBUSY", "EPERM"] as const)( + it.each(["EBUSY", "EPERM", "EAGAIN"] as const)( "rethrows %s when export cannot read the current storage file", async (code) => { const lockedStoragePath = join(testWorkDir, `accounts-${code}.json`); @@ -1491,7 +1491,7 @@ describe("storage", () => { }, ); - it.each(["EBUSY", "EPERM"] as const)( + it.each(["EBUSY", "EPERM", "EAGAIN"] as const)( "does not write an export file when %s happens while reading another storage path during a transaction", async (code) => { const transactionStoragePath = join( From 97a8b1851f689d0c64d8bb5e56b869bf433b057a Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 19:44:35 +0800 Subject: [PATCH 07/10] fix(storage): honor reappeared export storage --- lib/storage.ts | 35 +++++++++++++++---- test/storage.test.ts | 80 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index dbc0fc59..5dd406ba 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -719,6 +719,7 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { if (!state.currentStoragePath) { return null; } + const currentStoragePath = state.currentStoragePath; const candidatePaths = [ state.currentLegacyWorktreeStoragePath, @@ -743,10 +744,8 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { return null; } - let targetStorage = await loadNormalizedStorageFromPath( - state.currentStoragePath, - "current account storage", - { + const loadCurrentStorageForMigration = async (): Promise => + loadNormalizedStorageFromPath(currentStoragePath, "current account storage", { loadAccountsFromPath: (path) => loadAccountsFromPath(path, { normalizeAccountStorage, @@ -755,11 +754,29 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { logWarn: (message, details) => { log.warn(message, details); }, - }, - ); + }); + const readOnlyExportCurrentStorage = async (): Promise<{ + exists: boolean; + storage: AccountStorageV3 | null; + }> => { + if (commit || !existsSync(currentStoragePath)) { + return { exists: false, storage: null }; + } + return { + exists: true, + storage: await loadCurrentStorageForMigration(), + }; + }; + + let targetStorage = await loadCurrentStorageForMigration(); let migrated = false; for (const legacyPath of existingCandidatePaths) { + const liveCurrentStorageBeforeMerge = await readOnlyExportCurrentStorage(); + if (liveCurrentStorageBeforeMerge.exists) { + return liveCurrentStorageBeforeMerge.storage; + } + const legacyStorage = await loadNormalizedStorageFromPath( legacyPath, "legacy account storage", @@ -778,6 +795,12 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { continue; } + const liveCurrentStorageAfterLegacyRead = + await readOnlyExportCurrentStorage(); + if (liveCurrentStorageAfterLegacyRead.exists) { + return liveCurrentStorageAfterLegacyRead.storage; + } + const mergedStorage = mergeStorageForMigration( targetStorage, legacyStorage, diff --git a/test/storage.test.ts b/test/storage.test.ts index 46014b87..ba64f86c 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1672,6 +1672,86 @@ describe("storage", () => { } }); + it("does not revive legacy accounts when the current storage reappears before export merges legacy storage", async () => { + const currentStoragePath = join( + testWorkDir, + "accounts-reappeared-current.json", + ); + const legacyStoragePath = join( + testWorkDir, + "accounts-reappeared-legacy.json", + ); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + const actualStorageParser = await vi.importActual< + typeof import("../lib/storage/storage-parser.js") + >("../lib/storage/storage-parser.js"); + let recreateCurrentStorage = true; + vi.resetModules(); + vi.doMock("../lib/storage/storage-parser.js", () => ({ + ...actualStorageParser, + loadAccountsFromPath: vi.fn(async (path, deps) => { + if (path === currentStoragePath && recreateCurrentStorage) { + recreateCurrentStorage = false; + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [], + }), + ); + throw Object.assign(new Error("missing current storage"), { + code: "ENOENT", + }); + } + return actualStorageParser.loadAccountsFromPath(path, deps); + }), + })); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + const isolatedPathState = await import("../lib/storage/path-state.js"); + isolatedPathState.setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await expect( + isolatedStorageModule.exportAccounts(exportPath), + ).rejects.toThrow(/No accounts to export/); + + const currentStorage = JSON.parse( + await fs.readFile(currentStoragePath, "utf-8"), + ); + expect(currentStorage.accounts).toEqual([]); + expect(existsSync(legacyStoragePath)).toBe(true); + expect(existsSync(exportPath)).toBe(false); + } finally { + vi.doUnmock("../lib/storage/storage-parser.js"); + vi.resetModules(); + setStoragePathDirect(testStoragePath); + } + }); + it("does not revive legacy accounts when the current storage has an intentional reset marker", async () => { const currentStoragePath = join(testWorkDir, "accounts-reset-current.json"); const legacyStoragePath = join(testWorkDir, "accounts-reset-legacy.json"); From 736d4eba4c0cb161dfa5343a2e526a0656b9507e Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 20:23:25 +0800 Subject: [PATCH 08/10] fix(storage): rethrow reappeared export locks --- lib/storage.ts | 35 ++++++++++++++---- test/storage.test.ts | 87 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 5dd406ba..56812c63 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -755,24 +755,45 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { log.warn(message, details); }, }); - const readOnlyExportCurrentStorage = async (): Promise<{ + const readLiveCurrentStorageIfExportMode = async (): Promise<{ exists: boolean; storage: AccountStorageV3 | null; }> => { if (commit || !existsSync(currentStoragePath)) { return { exists: false, storage: null }; } - return { - exists: true, - storage: await loadCurrentStorageForMigration(), - }; + try { + const { normalized, schemaErrors } = await loadAccountsFromPath( + currentStoragePath, + { + normalizeAccountStorage, + isRecord, + }, + ); + if (schemaErrors.length > 0) { + log.warn("current account storage schema validation warnings", { + path: currentStoragePath, + errors: schemaErrors.slice(0, 5), + }); + } + return { + exists: true, + storage: normalized, + }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return { exists: false, storage: null }; + } + throw error; + } }; let targetStorage = await loadCurrentStorageForMigration(); let migrated = false; for (const legacyPath of existingCandidatePaths) { - const liveCurrentStorageBeforeMerge = await readOnlyExportCurrentStorage(); + const liveCurrentStorageBeforeMerge = + await readLiveCurrentStorageIfExportMode(); if (liveCurrentStorageBeforeMerge.exists) { return liveCurrentStorageBeforeMerge.storage; } @@ -796,7 +817,7 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { } const liveCurrentStorageAfterLegacyRead = - await readOnlyExportCurrentStorage(); + await readLiveCurrentStorageIfExportMode(); if (liveCurrentStorageAfterLegacyRead.exists) { return liveCurrentStorageAfterLegacyRead.storage; } diff --git a/test/storage.test.ts b/test/storage.test.ts index ba64f86c..e993ed66 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1752,6 +1752,93 @@ describe("storage", () => { } }); + it.each(["EBUSY", "EPERM", "EAGAIN"] as const)( + "rethrows %s when the current storage reappears locked during export fallback", + async (code) => { + const currentStoragePath = join( + testWorkDir, + `accounts-reappeared-locked-${code}.json`, + ); + const legacyStoragePath = join( + testWorkDir, + `accounts-reappeared-legacy-${code}.json`, + ); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + const actualStorageParser = await vi.importActual< + typeof import("../lib/storage/storage-parser.js") + >("../lib/storage/storage-parser.js"); + let currentReadCount = 0; + vi.resetModules(); + vi.doMock("../lib/storage/storage-parser.js", () => ({ + ...actualStorageParser, + loadAccountsFromPath: vi.fn(async (path, deps) => { + if (path === currentStoragePath) { + currentReadCount += 1; + if (currentReadCount === 1) { + await fs.writeFile( + currentStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [], + }), + ); + throw Object.assign( + new Error("missing current storage"), + { code: "ENOENT" }, + ); + } + throw Object.assign(new Error(`locked ${code}`), { code }); + } + return actualStorageParser.loadAccountsFromPath(path, deps); + }), + })); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + const isolatedPathState = await import("../lib/storage/path-state.js"); + isolatedPathState.setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await expect( + isolatedStorageModule.exportAccounts(exportPath), + ).rejects.toMatchObject({ code }); + + const currentStorage = JSON.parse( + await fs.readFile(currentStoragePath, "utf-8"), + ); + expect(currentStorage.accounts).toEqual([]); + expect(existsSync(legacyStoragePath)).toBe(true); + expect(existsSync(exportPath)).toBe(false); + } finally { + vi.doUnmock("../lib/storage/storage-parser.js"); + vi.resetModules(); + setStoragePathDirect(testStoragePath); + } + }, + ); + it("does not revive legacy accounts when the current storage has an intentional reset marker", async () => { const currentStoragePath = join(testWorkDir, "accounts-reset-current.json"); const legacyStoragePath = join(testWorkDir, "accounts-reset-legacy.json"); From a81b9c8e2d2c68cc96cf15c7a2ba00845a1b1b41 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 20:32:56 +0800 Subject: [PATCH 09/10] test(sync): remove unreachable retry mock branch --- test/experimental-sync-target-entry.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/experimental-sync-target-entry.test.ts b/test/experimental-sync-target-entry.test.ts index 00b4aac9..41652fd3 100644 --- a/test/experimental-sync-target-entry.test.ts +++ b/test/experimental-sync-target-entry.test.ts @@ -53,9 +53,6 @@ describe("experimental sync target entry", () => { while (retryAttempts.length < 2) { retryAttempts.push("EBUSY"); - if (!options.retryableCodes.has("EBUSY")) { - throw Object.assign(new Error("busy"), { code: "EBUSY" }); - } await options.sleep(25 * retryAttempts.length); } From d38106331d2746c08b0a388536053bdf3844b206 Mon Sep 17 00:00:00 2001 From: ndycode <405533+ndycode@users.noreply.github.com> Date: Mon, 23 Mar 2026 20:57:39 +0800 Subject: [PATCH 10/10] fix-export-reset-fallback --- lib/storage.ts | 6 ++-- test/storage.test.ts | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 56812c63..93154513 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -838,7 +838,7 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { targetStorage = fallbackStorage; log.warn("Failed to persist migrated account storage", { from: legacyPath, - to: state.currentStoragePath, + to: currentStoragePath, error: String(error), }); continue; @@ -864,7 +864,7 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { log.info("Migrated legacy project account storage", { from: legacyPath, - to: state.currentStoragePath, + to: currentStoragePath, accounts: mergedStorage.accounts.length, }); continue; @@ -877,7 +877,7 @@ async function migrateLegacyProjectStorageIfNeeded(options?: { if (migrated) { return targetStorage; } - if (targetStorage && !existsSync(state.currentStoragePath)) { + if (targetStorage && !existsSync(currentStoragePath)) { return targetStorage; } return null; diff --git a/test/storage.test.ts b/test/storage.test.ts index e993ed66..929a30b6 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -5,6 +5,7 @@ import { dirname, join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js"; import { setStoragePathState } from "../lib/storage/path-state.js"; +import { getIntentionalResetMarkerPath } from "../lib/storage/backup-paths.js"; import { buildNamedBackupPath, clearAccounts, @@ -1672,6 +1673,73 @@ describe("storage", () => { } }); + it("does not export legacy accounts when an intentional reset marker appears during export fallback migration", async () => { + const currentStoragePath = join( + testWorkDir, + "accounts-reset-during-fallback-current.json", + ); + const legacyStoragePath = join( + testWorkDir, + "accounts-reset-during-fallback-legacy.json", + ); + const resetMarkerPath = + getIntentionalResetMarkerPath(currentStoragePath); + await fs.writeFile( + legacyStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [ + { + accountId: "legacy", + refreshToken: "legacy-token", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + const actualStorageParser = await vi.importActual< + typeof import("../lib/storage/storage-parser.js") + >("../lib/storage/storage-parser.js"); + vi.resetModules(); + vi.doMock("../lib/storage/storage-parser.js", () => ({ + ...actualStorageParser, + loadAccountsFromPath: vi.fn(async (path, deps) => { + if (path === legacyStoragePath && !existsSync(resetMarkerPath)) { + await fs.writeFile(resetMarkerPath, ""); + } + return actualStorageParser.loadAccountsFromPath(path, deps); + }), + })); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + const isolatedPathState = await import("../lib/storage/path-state.js"); + isolatedPathState.setStoragePathState({ + currentStoragePath, + currentLegacyProjectStoragePath: legacyStoragePath, + currentLegacyWorktreeStoragePath: null, + currentProjectRoot: null, + }); + + await expect( + isolatedStorageModule.exportAccounts(exportPath), + ).rejects.toThrow(/No accounts to export/); + + expect(existsSync(currentStoragePath)).toBe(false); + expect(existsSync(resetMarkerPath)).toBe(true); + expect(existsSync(legacyStoragePath)).toBe(true); + expect(existsSync(exportPath)).toBe(false); + } finally { + vi.doUnmock("../lib/storage/storage-parser.js"); + vi.resetModules(); + setStoragePathDirect(testStoragePath); + } + }); + it("does not revive legacy accounts when the current storage reappears before export merges legacy storage", async () => { const currentStoragePath = join( testWorkDir,