fix: persist recovered flagged backups

Author: ndycode

lib/storage.ts

@@ -1806,6 +1806,14 @@ export async function loadFlaggedAccounts(): Promise<FlaggedAccountStorageV1> {
 				isRecord,
 				now: () => Date.now(),
 			}),
+		persistRecoveredBackup: async (storage, resetMarkerPath) =>
+			withStorageLock(async () => {
+				if (existsSync(resetMarkerPath)) {
+					return false;
+				}
+				await saveFlaggedAccountsUnlocked(storage);
+				return true;
+			}),
 		saveFlaggedAccounts,
 		loadFlaggedAccountsState,
 		logError: (message, details) => {

lib/storage/flagged-load-entry.ts

@@ -5,12 +5,20 @@ export async function loadFlaggedAccountsEntry(params: {
 	getLegacyFlaggedAccountsPath: () => string;
 	getIntentionalResetMarkerPath: (path: string) => string;
 	normalizeFlaggedStorage: (data: unknown) => FlaggedAccountStorageV1;
+	persistRecoveredBackup: (
+		storage: FlaggedAccountStorageV1,
+		resetMarkerPath: string,
+	) => Promise<boolean>;
 	saveFlaggedAccounts: (storage: FlaggedAccountStorageV1) => Promise<void>;
 	loadFlaggedAccountsState: (args: {
 		path: string;
 		legacyPath: string;
 		resetMarkerPath: string;
 		normalizeFlaggedStorage: (data: unknown) => FlaggedAccountStorageV1;
+		persistRecoveredBackup: (
+			storage: FlaggedAccountStorageV1,
+			resetMarkerPath: string,
+		) => Promise<boolean>;
 		saveFlaggedAccounts: (storage: FlaggedAccountStorageV1) => Promise<void>;
 		logError: (message: string, details: Record<string, unknown>) => void;
 		logInfo: (message: string, details: Record<string, unknown>) => void;
@@ -24,6 +32,7 @@ export async function loadFlaggedAccountsEntry(params: {
 		legacyPath: params.getLegacyFlaggedAccountsPath(),
 		resetMarkerPath: params.getIntentionalResetMarkerPath(path),
 		normalizeFlaggedStorage: params.normalizeFlaggedStorage,
+		persistRecoveredBackup: params.persistRecoveredBackup,
 		saveFlaggedAccounts: params.saveFlaggedAccounts,
 		logError: params.logError,
 		logInfo: params.logInfo,

lib/storage/flagged-storage-io.ts

@@ -57,6 +57,10 @@ export async function loadFlaggedAccountsState(params: {
 	legacyPath: string;
 	resetMarkerPath: string;
 	normalizeFlaggedStorage: (data: unknown) => FlaggedAccountStorageV1;
+	persistRecoveredBackup: (
+		storage: FlaggedAccountStorageV1,
+		resetMarkerPath: string,
+	) => Promise<boolean>;
 	saveFlaggedAccounts: (storage: FlaggedAccountStorageV1) => Promise<void>;
 	logError: (message: string, details: Record<string, unknown>) => void;
 	logInfo: (message: string, details: Record<string, unknown>) => void;
@@ -84,6 +88,23 @@ export async function loadFlaggedAccountsState(params: {
 				if (existsSync(params.resetMarkerPath)) {
 					return empty;
 				}
+				if (recovered.accounts.length > 0) {
+					try {
+						const persisted = await params.persistRecoveredBackup(
+							recovered,
+							params.resetMarkerPath,
+						);
+						if (!persisted) {
+							return empty;
+						}
+					} catch (persistError) {
+						params.logError("Failed to persist recovered flagged account storage", {
+							from: backupPath,
+							to: params.path,
+							error: String(persistError),
+						});
+					}
+				}
 				params.logInfo("Recovered flagged account storage from backup", {
 					from: backupPath,
 					to: params.path,

test/rotation-integration.test.ts

@@ -1,7 +1,7 @@
 import { promises as fs } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { describe, it, expect, beforeAll, beforeEach, afterAll } from "vitest";
+import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from "vitest";
 import { AccountManager } from "../lib/accounts.js";
 import {
   deduplicateAccounts,
@@ -303,6 +303,28 @@ describe("Multi-Account Rotation Integration", () => {
 					TEST_ACCOUNTS.slice(0, 3).map((account) => account.email),
 				);
 			}, 15_000);
+
+			it("removeWithRetry retries transient windows fs.rm errors", async () => {
+				const rmSpy = vi.spyOn(fs, "rm");
+				const retryCodes = ["EBUSY", "EPERM", "ENOTEMPTY"];
+				for (const code of retryCodes) {
+					rmSpy.mockImplementationOnce(async () => {
+						const error = new Error(code) as NodeJS.ErrnoException;
+						error.code = code;
+						throw error;
+					});
+				}
+
+				let callCount = 0;
+				try {
+					await removeWithRetry(testStoragePath, { force: true });
+					callCount = rmSpy.mock.calls.length;
+				} finally {
+					rmSpy.mockRestore();
+				}
+
+				expect(callCount).toBe(retryCodes.length + 1);
+			});
 		});
 
   describe("Debounced save", () => {

test/storage-recovery-paths.test.ts

@@ -132,6 +132,11 @@ describe("storage recovery paths", () => {
 		const recovered = await loadFlaggedAccounts();
 		expect(recovered.accounts).toHaveLength(1);
 		expect(recovered.accounts[0]?.email).toBe("flagged@example.com");
+
+		const persisted = JSON.parse(await fs.readFile(flaggedPath, "utf-8")) as {
+			accounts?: Array<{ email?: string }>;
+		};
+		expect(persisted.accounts?.[0]?.email).toBe("flagged@example.com");
 	});
 
 	it("recovers flagged accounts from backup file when primary storage is unreadable", async () => {
@@ -157,6 +162,11 @@ describe("storage recovery paths", () => {
 		const recovered = await loadFlaggedAccounts();
 		expect(recovered.accounts).toHaveLength(1);
 		expect(recovered.accounts[0]?.email).toBe("flagged2@example.com");
+
+		const persisted = JSON.parse(await fs.readFile(flaggedPath, "utf-8")) as {
+			accounts?: Array<{ email?: string }>;
+		};
+		expect(persisted.accounts?.[0]?.email).toBe("flagged2@example.com");
 	});
 
 	it("suppresses flagged backup recovery when the reset marker appears mid-read", async () => {

test/unified-settings.test.ts

@@ -193,6 +193,72 @@ describe("unified settings", () => {
 		});
 	});
 
+	it("keeps the last good backup when a concurrent writer updates the primary after a backup-derived read", async () => {
+		const {
+			getUnifiedSettingsPath,
+			loadUnifiedPluginConfigSync,
+			saveUnifiedPluginConfig,
+		} = await import("../lib/unified-settings.js");
+
+		await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 45_000 });
+		await saveUnifiedPluginConfig({ codexMode: false, fetchTimeoutMs: 90_000 });
+		await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8");
+
+		expect(loadUnifiedPluginConfigSync()).toEqual({
+			codexMode: true,
+			fetchTimeoutMs: 45_000,
+		});
+
+		const concurrentPrimary = {
+			version: 1,
+			pluginConfig: {
+				codexMode: false,
+				fetchTimeoutMs: 150_000,
+				retries: 4,
+			},
+		};
+		const copySpy = vi.spyOn(fs, "copyFile");
+		const renameSpy = vi.spyOn(fs, "rename");
+		let injectedConcurrentWrite = false;
+		renameSpy.mockImplementationOnce(async () => {
+			injectedConcurrentWrite = true;
+			await fs.writeFile(
+				getUnifiedSettingsPath(),
+				`${JSON.stringify(concurrentPrimary, null, 2)}\n`,
+				"utf8",
+			);
+			const error = new Error("denied") as NodeJS.ErrnoException;
+			error.code = "EACCES";
+			throw error;
+		});
+
+		try {
+			await expect(
+				saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 120_000 }),
+			).rejects.toThrow();
+		} finally {
+			copySpy.mockRestore();
+			renameSpy.mockRestore();
+		}
+
+		expect(injectedConcurrentWrite).toBe(true);
+		expect(copySpy).not.toHaveBeenCalledWith(
+			getUnifiedSettingsPath(),
+			`${getUnifiedSettingsPath()}.bak`,
+		);
+		const backupRecord = JSON.parse(
+			await fs.readFile(`${getUnifiedSettingsPath()}.bak`, "utf8"),
+		) as { pluginConfig?: Record<string, unknown> };
+		expect(backupRecord.pluginConfig).toEqual({
+			codexMode: true,
+			fetchTimeoutMs: 45_000,
+		});
+		const primaryRecord = JSON.parse(
+			await fs.readFile(getUnifiedSettingsPath(), "utf8"),
+		) as { pluginConfig?: Record<string, unknown> };
+		expect(primaryRecord.pluginConfig).toEqual(concurrentPrimary.pluginConfig);
+	});
+
 	it("resumes snapshotting after a backup-derived write succeeds", async () => {
 		const {
 			getUnifiedSettingsPath,