fix: make account clear reset atomic

Author: ndycode

lib/storage/account-clear.ts

@@ -7,32 +7,43 @@ export async function clearAccountStorageArtifacts(params: {
 	backupPaths: string[];
 	logError: (message: string, details: Record<string, unknown>) => void;
 }): Promise<void> {
-	await fs.writeFile(
-		params.resetMarkerPath,
-		JSON.stringify({ version: 1, createdAt: Date.now() }),
-		{ encoding: "utf-8", mode: 0o600 },
-	);
-	const clearPath = async (targetPath: string): Promise<void> => {
+	const clearPath = async (
+		targetPath: string,
+		required: boolean,
+	): Promise<void> => {
 		try {
 			await fs.unlink(targetPath);
 		} catch (error) {
 			const code = (error as NodeJS.ErrnoException).code;
-			if (code !== "ENOENT") {
+			if (code === "ENOENT") {
+				return;
+			}
+			if (required) {
 				params.logError("Failed to clear account storage artifact", {
 					path: targetPath,
 					error: String(error),
 				});
+				throw error;
 			}
+			params.logError("Failed to clear account storage artifact", {
+				path: targetPath,
+				error: String(error),
+			});
 		}
 	};
 
-	try {
-		await Promise.all([
-			clearPath(params.path),
-			clearPath(params.walPath),
-			...params.backupPaths.map(clearPath),
-		]);
-	} catch {
-		// Individual path cleanup is already best-effort with per-artifact logging.
+	await clearPath(params.path, true);
+	await clearPath(params.walPath, true);
+	await fs.writeFile(
+		params.resetMarkerPath,
+		JSON.stringify({ version: 1, createdAt: Date.now() }),
+		{ encoding: "utf-8", mode: 0o600 },
+	);
+	for (const backupPath of params.backupPaths) {
+		try {
+			await clearPath(backupPath, false);
+		} catch {
+			// Non-critical artifacts are already logged best-effort.
+		}
 	}
 }

test/storage.test.ts

@@ -2961,14 +2961,24 @@ describe("storage", () => {
 			await fs.rm(testWorkDir, { recursive: true, force: true });
 		});
 
-		it("logs but does not throw on non-ENOENT errors", async () => {
-			const readOnlyDir = join(testWorkDir, "readonly");
-			await fs.mkdir(readOnlyDir, { recursive: true });
-			const readOnlyFile = join(readOnlyDir, "accounts.json");
-			await fs.writeFile(readOnlyFile, "{}");
-			setStoragePathDirect(readOnlyFile);
+		it("throws and does not write reset marker when the primary storage file cannot be removed", async () => {
+			await fs.writeFile(testStoragePath, "{}", "utf-8");
+			const resetMarkerPath = getIntentionalResetMarkerPath(testStoragePath);
+			const unlinkSpy = vi
+				.spyOn(fs, "unlink")
+				.mockImplementation(async (targetPath) => {
+					if (String(targetPath) === testStoragePath) {
+						const error = Object.assign(new Error("locked"), { code: "EPERM" });
+						throw error;
+					}
+					return Promise.resolve();
+				});
 
-			await expect(clearAccounts()).resolves.not.toThrow();
+			await expect(clearAccounts()).rejects.toMatchObject({ code: "EPERM" });
+			expect(existsSync(testStoragePath)).toBe(true);
+			expect(existsSync(resetMarkerPath)).toBe(false);
+
+			unlinkSpy.mockRestore();
 		});
 	});
 
@@ -4122,7 +4132,7 @@ describe("storage", () => {
 					Object.assign(new Error("EACCES error"), { code: "EACCES" }),
 				);
 
-			await clearAccounts();
+			await expect(clearAccounts()).rejects.toMatchObject({ code: "EACCES" });
 
 			expect(unlinkSpy).toHaveBeenCalled();
 			unlinkSpy.mockRestore();