fix: harden sync prune recovery

Author: ndycode

index.ts

@@ -3465,6 +3465,7 @@ while (attempted.size < Math.max(1, accountCount)) {
 									index: number;
 									account: AccountStorageV3["accounts"][number];
 								}> = [];
+								let rollbackStorage: AccountStorageV3 | null = null;
 								await withAccountStorageTransaction(async (loadedStorage, persist) => {
 									const currentStorage =
 										loadedStorage ??
@@ -3488,6 +3489,7 @@ while (attempted.size < Math.max(1, accountCount)) {
 									if (removedTargets.length === 0) {
 										return;
 									}
+									rollbackStorage = structuredClone(currentStorage);
 
 									const activeAccountIdentity = {
 										refreshToken:
@@ -3554,21 +3556,41 @@ while (attempted.size < Math.max(1, accountCount)) {
 											}),
 										),
 									);
-									await withFlaggedAccountsTransaction(async (currentFlaggedStorage, persist) => {
-										await persist({
-											version: 1,
-											accounts: currentFlaggedStorage.accounts.filter(
-												(flagged) =>
-													!removedFlaggedKeys.has(
-														getSyncRemovalTargetKey({
-															refreshToken: flagged.refreshToken,
-															organizationId: flagged.organizationId,
-															accountId: flagged.accountId,
-														}),
-													),
-											),
+									try {
+										await withFlaggedAccountsTransaction(async (currentFlaggedStorage, persist) => {
+											await persist({
+												version: 1,
+												accounts: currentFlaggedStorage.accounts.filter(
+													(flagged) =>
+														!removedFlaggedKeys.has(
+															getSyncRemovalTargetKey({
+																refreshToken: flagged.refreshToken,
+																organizationId: flagged.organizationId,
+																accountId: flagged.accountId,
+															}),
+														),
+												),
+											});
 										});
-									});
+									} catch (flaggedError) {
+										if (rollbackStorage) {
+											try {
+												await withAccountStorageTransaction(async (_current, persist) => {
+													await persist(rollbackStorage as AccountStorageV3);
+												});
+											} catch (restoreError) {
+												const flaggedMessage =
+													flaggedError instanceof Error ? flaggedError.message : String(flaggedError);
+												const restoreMessage =
+													restoreError instanceof Error ? restoreError.message : String(restoreError);
+												throw new Error(
+													`Failed to remove flagged sync entries after account removal: ${flaggedMessage}; ` +
+														`failed to restore removed accounts: ${restoreMessage}`,
+												);
+											}
+										}
+										throw flaggedError;
+									}
 									invalidateAccountManagerCache();
 								}
 							};
@@ -3754,7 +3776,12 @@ while (attempted.size < Math.max(1, accountCount)) {
 									console.log("");
 								} catch (error) {
 									const message = error instanceof Error ? error.message : String(error);
-									const backupHint = backupPath ? `\nBackup: ${backupPath}` : "";
+									const cleanupBackupPath =
+										error instanceof Error &&
+										typeof (error as Error & { backupPath?: unknown }).backupPath === "string"
+											? ((error as Error & { backupPath: string }).backupPath)
+											: undefined;
+									const backupHint = cleanupBackupPath ? `\nBackup: ${cleanupBackupPath}` : "";
 									console.log(`\nCleanup failed: ${message}${backupHint}\n`);
 								}
 							};

lib/codex-multi-auth-sync.ts

@@ -121,6 +121,7 @@ interface PreparedCodexMultiAuthPreviewStorage {
 
 const TEMP_CLEANUP_RETRY_DELAYS_MS = [100, 250, 500] as const;
 const STALE_TEMP_CLEANUP_RETRY_DELAY_MS = 150;
+const STALE_TEMP_SWEEP_RETRYABLE_CODES = new Set(["EBUSY", "EAGAIN", "EACCES", "EPERM", "ENOTEMPTY"]);
 
 function sleepAsync(ms: number): Promise<void> {
 	return new Promise((resolve) => setTimeout(resolve, ms));
@@ -311,7 +312,7 @@ async function cleanupStaleNormalizedImportTempDirs(
 					continue;
 				}
 				let message = error instanceof Error ? error.message : String(error);
-				if (code === "EBUSY" || code === "EACCES" || code === "EPERM") {
+				if (code && STALE_TEMP_SWEEP_RETRYABLE_CODES.has(code)) {
 					await sleepAsync(STALE_TEMP_CLEANUP_RETRY_DELAY_MS);
 					try {
 						await fs.rm(candidateDir, { recursive: true, force: true });
@@ -1212,6 +1213,7 @@ export async function cleanupCodexMultiAuthSyncedOverlaps(
 			activeIndex: 0,
 			activeIndexByFamily: {},
 		};
+		let writtenBackupPath: string | undefined;
 		if (backupPath) {
 			await fs.mkdir(dirname(backupPath), { recursive: true });
 			const tempBackupPath = `${backupPath}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`;
@@ -1221,6 +1223,7 @@ export async function cleanupCodexMultiAuthSyncedOverlaps(
 					mode: 0o600,
 				});
 				await fs.rename(tempBackupPath, backupPath);
+				writtenBackupPath = backupPath;
 			} catch (error) {
 				try {
 					await fs.unlink(tempBackupPath);
@@ -1230,10 +1233,17 @@ export async function cleanupCodexMultiAuthSyncedOverlaps(
 				throw error;
 			}
 		}
-		const plan = buildCodexMultiAuthOverlapCleanupPlan(fallback);
-		if (plan.nextStorage) {
-			await persist(plan.nextStorage);
+		try {
+			const plan = buildCodexMultiAuthOverlapCleanupPlan(fallback);
+			if (plan.nextStorage) {
+				await persist(plan.nextStorage);
+			}
+			return plan.result;
+		} catch (error) {
+			if (writtenBackupPath && error instanceof Error) {
+				(error as Error & { backupPath?: string }).backupPath = writtenBackupPath;
+			}
+			throw error;
 		}
-		return plan.result;
 	});
 }

lib/sync-prune-backup.ts

@@ -5,41 +5,22 @@ type FlaggedSnapshot<TAccount extends object> = {
 	accounts: TAccount[];
 };
 
-type TokenRedacted<TAccount extends object> =
-	Omit<TAccount, "accessToken" | "refreshToken" | "idToken"> & {
-		accessToken?: undefined;
-		refreshToken?: undefined;
-		idToken?: undefined;
-	};
-
-function cloneWithoutTokens<TAccount extends object>(account: TAccount): TokenRedacted<TAccount> {
-	const clone = structuredClone(account) as TokenRedacted<TAccount>;
-	delete clone.accessToken;
-	delete clone.refreshToken;
-	delete clone.idToken;
-	return clone;
-}
-
 export function createSyncPruneBackupPayload<TFlaggedAccount extends object>(
 	currentAccountsStorage: AccountStorageV3,
 	currentFlaggedStorage: FlaggedSnapshot<TFlaggedAccount>,
 ): {
 	version: 1;
-	accounts: Omit<AccountStorageV3, "accounts"> & {
-		accounts: Array<TokenRedacted<AccountStorageV3["accounts"][number]>>;
-	};
-	flagged: FlaggedSnapshot<TokenRedacted<TFlaggedAccount>>;
+	accounts: AccountStorageV3;
+	flagged: FlaggedSnapshot<TFlaggedAccount>;
 } {
 	return {
 		version: 1,
-		accounts: {
+		accounts: structuredClone({
 			...currentAccountsStorage,
-			accounts: currentAccountsStorage.accounts.map((account) => cloneWithoutTokens(account)),
 			activeIndexByFamily: { ...(currentAccountsStorage.activeIndexByFamily ?? {}) },
-		},
-		flagged: {
+		}),
+		flagged: structuredClone({
 			...currentFlaggedStorage,
-			accounts: currentFlaggedStorage.accounts.map((flagged) => cloneWithoutTokens(flagged)),
-		},
+		}),
 	};
 }

test/codex-multi-auth-sync.test.ts

@@ -808,7 +808,9 @@ describe("codex-multi-auth sync", () => {
 				skipped: 0,
 				total: 4,
 			});
-			expect(rmSpy).toHaveBeenCalledTimes(3);
+			expect(
+				rmSpy.mock.calls.filter(([path]) => String(path).includes("oc-chatgpt-multi-auth-sync-")),
+			).toHaveLength(3);
 			expect(vi.mocked(loggerModule.logWarn)).not.toHaveBeenCalledWith(
 				expect.stringContaining("Failed to remove temporary codex sync directory"),
 			);
@@ -961,7 +963,9 @@ describe("codex-multi-auth sync", () => {
 		}
 	});
 
-	it("retries stale temp sweep once on transient Windows lock errors", async () => {
+	it.each(["EBUSY", "ENOTEMPTY", "EAGAIN"] as const)(
+		"retries stale temp sweep once on transient Windows %s cleanup errors",
+		async (code) => {
 		const rootDir = join(process.cwd(), ".tmp-codex-multi-auth");
 		const fakeHome = await fs.promises.mkdtemp(join(os.tmpdir(), "codex-sync-home-"));
 		process.env.CODEX_MULTI_AUTH_DIR = rootDir;
@@ -987,7 +991,7 @@ describe("codex-multi-auth sync", () => {
 		const rmSpy = vi.spyOn(fs.promises, "rm").mockImplementation(async (path, options) => {
 			if (!staleSweepBlocked && String(path) === staleDir) {
 				staleSweepBlocked = true;
-				throw Object.assign(new Error("busy"), { code: "EBUSY" });
+				throw Object.assign(new Error("busy"), { code });
 			}
 			return originalRm(path, options as never);
 		});
@@ -1701,6 +1705,67 @@ describe("codex-multi-auth sync", () => {
 		},
 	);
 
+	it("annotates overlap cleanup failures with the written backup path", async () => {
+		const storageModule = await import("../lib/storage.js");
+		const persist = vi.fn(async () => {
+			throw new Error("persist failed");
+		});
+		vi.mocked(storageModule.withAccountStorageTransaction).mockImplementationOnce(async (handler) =>
+			handler(
+				{
+					version: 3,
+					activeIndex: 0,
+					activeIndexByFamily: {},
+					accounts: [
+						{
+							accountId: "org-local",
+							organizationId: "org-local",
+							accountIdSource: "org",
+							email: "shared@example.com",
+							refreshToken: "rt-local",
+							addedAt: 5,
+							lastUsed: 5,
+						},
+						{
+							accountTags: ["codex-multi-auth-sync"],
+							email: "shared@example.com",
+							refreshToken: "rt-sync",
+							addedAt: 4,
+							lastUsed: 4,
+						},
+					],
+				},
+				persist,
+			),
+		);
+		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);
+
+		try {
+			const { cleanupCodexMultiAuthSyncedOverlaps } = await import("../lib/codex-multi-auth-sync.js");
+			let thrown: unknown;
+			try {
+				await cleanupCodexMultiAuthSyncedOverlaps("/tmp/overlap-cleanup-backup.json");
+			} catch (error) {
+				thrown = error;
+			}
+
+			expect(mkdirSpy).toHaveBeenCalledWith("/tmp", { recursive: true });
+			expect(writeSpy).toHaveBeenCalled();
+			expect(renameSpy).toHaveBeenCalled();
+			expect(thrown).toBeInstanceOf(Error);
+			expect(thrown).toMatchObject({
+				message: "persist failed",
+				backupPath: "/tmp/overlap-cleanup-backup.json",
+			});
+		} finally {
+			mkdirSpy.mockRestore();
+			writeSpy.mockRestore();
+			renameSpy.mockRestore();
+		}
+	});
+
 
 	it("limits overlap cleanup to accounts tagged from codex-multi-auth sync", async () => {
 			const storageModule = await import("../lib/storage.js");