refactor: migrate the last two hand-rolled retry loops to withRetry

lib/config.ts

@@ -880,37 +880,32 @@ export async function savePluginConfig(
 			// writer. Mirrors the unified-settings save path (writeSettingsRecordAsync
 			// CAS).
 			await withConfigFileLock(envPath, async () => {
-				for (let attempt = 0; attempt < 3; attempt += 1) {
-					const expectedMtimeMs = await getConfigFileMtimeMs(envPath);
-					const envConfigState = await readConfigRecordForSave(envPath);
-					if (envConfigState.status === "unreadable") {
-						throw new Error(
-							`Aborting config save because ${envPath} is unreadable.`,
-						);
-					}
-					const existingConfig =
-						envConfigState.status === "ok"
-							? sanitizeStoredPluginConfigRecord(envConfigState.record)
-							: null;
-					const merged = {
-						...(existingConfig ?? {}),
-						...sanitizedPatch,
-					};
-					try {
+				// CAS retry: ESTALE means the file's mtime moved between our stat and
+				// the write, so each attempt re-stats, re-reads, and re-merges against
+				// the latest on-disk state before writing again.
+				await withRetry(
+					async () => {
+						const expectedMtimeMs = await getConfigFileMtimeMs(envPath);
+						const envConfigState = await readConfigRecordForSave(envPath);
+						if (envConfigState.status === "unreadable") {
+							throw new Error(
+								`Aborting config save because ${envPath} is unreadable.`,
+							);
+						}
+						const existingConfig =
+							envConfigState.status === "ok"
+								? sanitizeStoredPluginConfigRecord(envConfigState.record)
+								: null;
+						const merged = {
+							...(existingConfig ?? {}),
+							...sanitizedPatch,
+						};
 						await writeJsonFileAtomicWithRetry(envPath, merged, {
 							expectedMtimeMs,
 						});
-						return;
-					} catch (error) {
-						if (
-							(error as NodeJS.ErrnoException).code !== "ESTALE" ||
-							attempt >= 2
-						) {
-							throw error;
-						}
-						// Loop: re-stat, re-read, re-merge against the latest on-disk state.
-					}
-				}
+					},
+					{ maxAttempts: 3, backoffMs: 0, retryableCodes: ["ESTALE"] },
+				);
 			});
 		});
 		return;

lib/storage.ts

@@ -1838,24 +1838,17 @@ async function saveAccountsUnlocked(storage: AccountStorageV3): Promise<void> {
 				fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }),
 			),
 		statTemp: (tempPath: string) => fs.stat(tempPath),
-		renameTempToPath: async (tempPath: string) => {
-			let lastError: NodeJS.ErrnoException | null = null;
-			for (let attempt = 0; attempt < 5; attempt++) {
-				try {
-					await fs.rename(tempPath, path);
-					return;
-				} catch (renameError) {
-					const code = (renameError as NodeJS.ErrnoException).code;
-					if (code === "EPERM" || code === "EBUSY") {
-						lastError = renameError as NodeJS.ErrnoException;
-						await new Promise((r) => setTimeout(r, 10 * 2 ** attempt));
-						continue;
-					}
-					throw renameError;
-				}
-			}
-			if (lastError) throw lastError;
-		},
+		renameTempToPath: (tempPath: string) =>
+			// Windows can hold the destination briefly (AV/indexer); retry only the
+			// lock codes rename actually surfaces there, on the original
+			// 10ms-doubling schedule. (The hand-rolled loop this replaces also
+			// slept once more after the final failure; withRetry rethrows
+			// immediately instead.)
+			withRetry(() => fs.rename(tempPath, path), {
+				maxAttempts: 5,
+				backoffMs: (attempt) => 10 * 2 ** (attempt - 1),
+				retryableCodes: ["EPERM", "EBUSY"],
+			}),
 		cleanupResetMarker: async () => {
 			try {
 				await fs.unlink(resetMarkerPath);

test/storage.test.ts

@@ -3976,6 +3976,87 @@ describe("storage", () => {
 			}
 		});
 
+		it("retries the temp-to-final rename on transient EPERM and succeeds", async () => {
+			const now = Date.now();
+			const storagePath = getStoragePath();
+
+			// A predecessor that failed mid-test can leak its own fs.rename spy;
+			// restore first so the passthrough binding captures the real rename
+			// instead of recursing into this test's mock.
+			vi.restoreAllMocks();
+			const originalRename = fs.rename.bind(fs);
+			let tempRenameAttempts = 0;
+			const renameSpy = vi
+				.spyOn(fs, "rename")
+				.mockImplementation(async (oldPath, newPath) => {
+					const sourcePath = String(oldPath);
+					if (sourcePath.endsWith(".tmp") && String(newPath) === storagePath) {
+						tempRenameAttempts += 1;
+						if (tempRenameAttempts <= 2) {
+							const err = new Error(
+								"EPERM temp rename",
+							) as NodeJS.ErrnoException;
+							err.code = "EPERM";
+							throw err;
+						}
+					}
+					return originalRename(oldPath as string, newPath as string);
+				});
+			try {
+				await saveAccounts({
+					version: 3 as const,
+					activeIndex: 0,
+					accounts: [{ refreshToken: "token", addedAt: now, lastUsed: now }],
+				});
+
+				expect(tempRenameAttempts).toBe(3);
+				const saved = JSON.parse(
+					await fs.readFile(storagePath, "utf-8"),
+				) as {
+					accounts?: Array<{ refreshToken?: string }>;
+				};
+				expect(saved.accounts?.[0]?.refreshToken).toBe("token");
+			} finally {
+				renameSpy.mockRestore();
+			}
+		});
+
+		it("does not retry the temp-to-final rename on non-lock errors", async () => {
+			const now = Date.now();
+			const storagePath = getStoragePath();
+
+			// Same leaked-spy guard as the EPERM retry case above.
+			vi.restoreAllMocks();
+			const originalRename = fs.rename.bind(fs);
+			let tempRenameAttempts = 0;
+			const renameSpy = vi
+				.spyOn(fs, "rename")
+				.mockImplementation(async (oldPath, newPath) => {
+					const sourcePath = String(oldPath);
+					if (sourcePath.endsWith(".tmp") && String(newPath) === storagePath) {
+						tempRenameAttempts += 1;
+						const err = new Error(
+							"ENOSPC temp rename",
+						) as NodeJS.ErrnoException;
+						err.code = "ENOSPC";
+						throw err;
+					}
+					return originalRename(oldPath as string, newPath as string);
+				});
+			try {
+				await expect(
+					saveAccounts({
+						version: 3 as const,
+						activeIndex: 0,
+						accounts: [{ refreshToken: "token", addedAt: now, lastUsed: now }],
+					}),
+				).rejects.toThrow(/ENOSPC temp rename|Failed to save accounts/);
+				expect(tempRenameAttempts).toBe(1);
+			} finally {
+				renameSpy.mockRestore();
+			}
+		});
+
 		it("rotates backups and retains historical snapshots", async () => {
 			const now = Date.now();
 			const storagePath = getStoragePath();