fix(studio-server): undo and redo on Windows wait out a briefly held file (#6559)

* fix(studio-server): undo and redo on Windows wait out a briefly held file

* style(core): format the atomic write test

packages/core/package-subpaths.json

@@ -74,6 +74,18 @@
       "types": "./dist/removeTree.d.ts",
       "environments": ["bun", "node"]
     },
+    "./rename-retry": {
+      "source": "./src/renameWithRetry.ts",
+      "runtime": "./dist/renameWithRetry.js",
+      "types": "./dist/renameWithRetry.d.ts",
+      "environments": ["bun", "node"]
+    },
+    "./remove-scratch": {
+      "source": "./src/removeScratch.ts",
+      "runtime": "./dist/removeScratch.js",
+      "types": "./dist/removeScratch.d.ts",
+      "environments": ["bun", "node"]
+    },
     "./project-folders": {
       "source": "./src/projectFolders.ts",
       "runtime": "./dist/projectFolders.js",

packages/core/package.json

@@ -86,6 +86,18 @@
       "import": "./src/removeTree.ts",
       "types": "./src/removeTree.ts"
     },
+    "./rename-retry": {
+      "bun": "./src/renameWithRetry.ts",
+      "node": "./dist/renameWithRetry.js",
+      "import": "./src/renameWithRetry.ts",
+      "types": "./src/renameWithRetry.ts"
+    },
+    "./remove-scratch": {
+      "bun": "./src/removeScratch.ts",
+      "node": "./dist/removeScratch.js",
+      "import": "./src/removeScratch.ts",
+      "types": "./src/removeScratch.ts"
+    },
     "./project-folders": {
       "bun": "./src/projectFolders.ts",
       "node": "./dist/projectFolders.js",
@@ -648,6 +660,14 @@
         "import": "./dist/removeTree.js",
         "types": "./dist/removeTree.d.ts"
       },
+      "./rename-retry": {
+        "import": "./dist/renameWithRetry.js",
+        "types": "./dist/renameWithRetry.d.ts"
+      },
+      "./remove-scratch": {
+        "import": "./dist/removeScratch.js",
+        "types": "./dist/removeScratch.d.ts"
+      },
       "./project-folders": {
         "import": "./dist/projectFolders.js",
         "types": "./dist/projectFolders.d.ts"

packages/core/src/atomicFile.test.ts

@@ -1,5 +1,5 @@
 // @vitest-environment node
-import { afterEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import * as crypto from "node:crypto";
 import * as fs from "node:fs";
 import { lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
@@ -381,9 +381,15 @@ describe("createFileAtomically", () => {
   });
 });
 
+// Only Windows holds a file a moment after its reader is done, so only there is a refused rename tried again.
 describe("replaceFileAtomically on a busy target", () => {
   const dirs: string[] = [];
+  const platform = process.platform;
+  beforeEach(() =>
+    Object.defineProperty(process, "platform", { value: "win32", configurable: true }),
+  );
   afterEach(() => {
+    Object.defineProperty(process, "platform", { value: platform, configurable: true });
     for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
   });
 
@@ -414,11 +420,11 @@ describe("replaceFileAtomically on a busy target", () => {
     expect(attempts()).toBe(3);
   });
 
-  it("gives up after five attempts and keeps the old file", () => {
+  it("gives up after eight attempts and keeps the old file", () => {
     const { dir, file, operations, attempts } = renamingAfter(Infinity, "EBUSY");
 
     expect(() => replaceFileAtomically(file, "new", 0o644, operations)).toThrow("EBUSY");
-    expect(attempts()).toBe(5);
+    expect(attempts()).toBe(8);
     expect(readFileSync(file, "utf-8")).toBe("old");
     expect(fs.readdirSync(dir)).toEqual(["index.html"]);
   });

packages/core/src/atomicFile.ts

@@ -2,6 +2,7 @@ import * as fs from "node:fs";
 import { createHash, randomBytes } from "node:crypto";
 import { basename, dirname, isAbsolute, join, resolve, sep } from "node:path";
 import { realpath } from "./safePath.js";
+import { renameWithRetry } from "./renameWithRetry.js";
 
 type SiblingFileSystem = Pick<typeof fs, "writeFileSync" | "chmodSync" | "unlinkSync">;
 type DirectFileSystem = SiblingFileSystem & Pick<typeof fs, "openSync" | "closeSync">;
@@ -9,9 +10,6 @@ type DirectFileSystem = SiblingFileSystem & Pick<typeof fs, "openSync" | "closeS
 // Codes a volume without hard links (FAT, exFAT, some network shares) answers link() with.
 // EISDIR: libuv maps Windows ERROR_INVALID_FUNCTION (FAT/exFAT refusing a link) to it; nodejs/node#65817.
 const NO_HARD_LINKS = new Set(["EPERM", "ENOTSUP", "EOPNOTSUPP", "EXDEV", "EISDIR"]);
-// Windows refuses a rename while another process briefly holds the target open.
-const BUSY_RENAME = new Set(["EPERM", "EBUSY", "EACCES"]);
-const RENAME_RETRY_DELAYS_MS = [10, 20, 30, 40];
 const MAX_LINK_HOPS = 40;
 const TEMP_NAME_TRIES = 3;
 const TEMP_SUFFIX = /\.hf[0-9a-f]{6}\.tmp$/;
@@ -52,7 +50,7 @@ export function replaceFileAtomically(
   traceWrite(filePath, content);
   // Node fs.rename uses libuv uv_fs_rename; win32 calls MoveFileExW with MOVEFILE_REPLACE_EXISTING.
   publishSibling(filePath, content, mode, operations, (tempPath) =>
-    renameWithRetry(operations, tempPath, filePath),
+    renameWithRetry(tempPath, filePath, { rename: operations.renameSync }),
   );
 }
 
@@ -174,23 +172,6 @@ function writeTempSibling(
   }
 }
 
-function renameWithRetry(
-  operations: Pick<typeof fs, "renameSync">,
-  from: string,
-  to: string,
-): void {
-  for (let attempt = 0; ; attempt++) {
-    try {
-      operations.renameSync(from, to);
-      return;
-    } catch (error) {
-      const delay = RENAME_RETRY_DELAYS_MS[attempt];
-      if (delay === undefined || !BUSY_RENAME.has(errorCode(error))) throw error;
-      Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, delay);
-    }
-  }
-}
-
 function errorCode(error: unknown): string {
   return String((error as NodeJS.ErrnoException)?.code);
 }

packages/core/src/removeScratch.test.ts

@@ -1,3 +1,4 @@
+// @vitest-environment node
 import { mkdtempSync, readFileSync, writeFileSync, existsSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";

packages/core/src/renameWithRetry.test.ts

@@ -1,3 +1,4 @@
+// @vitest-environment node
 import { describe, expect, it } from "vitest";
 import { renameWithRetry, renameWithRetryAsync } from "./renameWithRetry.js";
 

packages/engine/src/index.ts

@@ -398,8 +398,8 @@ export {
 } from "./utils/ffmpegBinaries.js";
 
 export { trackChildProcess, killTrackedProcesses } from "./utils/processTracker.js";
-export { renameWithRetry, renameWithRetryAsync } from "./utils/renameWithRetry.js";
-export { removeScratch, collectScratchLeftovers } from "./utils/removeScratch.js";
+export { renameWithRetry, renameWithRetryAsync } from "@hyperframes/core/rename-retry";
+export { removeScratch, collectScratchLeftovers } from "@hyperframes/core/remove-scratch";
 
 // drawElement self-verify comparison — shared by the streaming drain
 // (producer) and the parallel disk-path verify (parallelCoordinator).

packages/engine/src/services/aacEncode.ts

@@ -5,7 +5,7 @@
 
 import { execFile } from "node:child_process";
 import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
-import { removeScratch } from "../utils/removeScratch.js";
+import { removeScratch } from "@hyperframes/core/remove-scratch";
 import { dirname, join } from "node:path";
 import { getFfmpegBinary } from "../utils/ffmpegBinaries.js";
 import { runFfmpeg, type RunFfmpegResult } from "../utils/runFfmpeg.js";

packages/engine/src/services/audioFxRender.ts

@@ -29,7 +29,7 @@ import { pathToFileURL } from "node:url";
 import { getAudioFxRuntimeScript } from "@hyperframes/core/audio-fx-runtime";
 import { enabledAudioFxNodes, type HfAudioFxChain } from "@hyperframes/core/audio-fx";
 import { serializeAutomation, type HfAutomation } from "@hyperframes/core/audio-automation";
-import { removeScratch } from "../utils/removeScratch.js";
+import { removeScratch } from "@hyperframes/core/remove-scratch";
 import { acquireBrowser, buildChromeArgs } from "./browserManager.js";
 import { createEnvelopeWalker } from "./audioVolumeEnvelope.js";
 import { riffChunks, wavFormatTag } from "./wavChunks.js";

packages/engine/src/services/audioMixer.ts

@@ -1,8 +1,8 @@
 // fallow-ignore-file complexity code-duplication
 import { isSelfOrAncestorHidden, memberGroupKey, isMemberGroupHidden } from "./mediaHidden.js";
 import { closeSync, existsSync, mkdirSync, mkdtempSync, openSync, rmSync, writeFileSync } from "fs";
-import { renameWithRetry } from "../utils/renameWithRetry.js";
-import { removeScratch } from "../utils/removeScratch.js";
+import { renameWithRetry } from "@hyperframes/core/rename-retry";
+import { removeScratch } from "@hyperframes/core/remove-scratch";
 import { join, dirname, isAbsolute, relative } from "path";
 import { parseHTML } from "linkedom";
 import { extractAudioMetadata, type AudioMetadata } from "../utils/ffprobe.js";

packages/engine/src/services/audioVolumeEnvelope.ts

@@ -16,7 +16,7 @@
  */
 
 import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "fs";
-import { renameWithRetry } from "../utils/renameWithRetry.js";
+import { renameWithRetry } from "@hyperframes/core/rename-retry";
 import { dirname, join } from "path";
 import type { AudioVolumeKeyframe } from "./audioMixer.types.js";
 import { normaliseEnvelope } from "@hyperframes/core/media-volume-envelope";

packages/engine/src/services/extractionCache.ts

@@ -44,7 +44,7 @@ import {
   utimesSync,
   writeFileSync,
 } from "node:fs";
-import { renameWithRetry } from "../utils/renameWithRetry.js";
+import { renameWithRetry } from "@hyperframes/core/rename-retry";
 import { join } from "node:path";
 import type { VideoMetadata } from "../utils/ffprobe.js";
 import { FRAME_FILENAME_PREFIX, framePathsFromDirectory } from "./extractedFrameIndex.js";

packages/engine/src/services/parallelCoordinator.ts

@@ -8,7 +8,7 @@
 import { cpus, freemem } from "os";
 import { existsSync, mkdirSync, readdirSync } from "fs";
 import { copyFile, readFile } from "fs/promises";
-import { renameWithRetryAsync } from "../utils/renameWithRetry.js";
+import { renameWithRetryAsync } from "@hyperframes/core/rename-retry";
 import { join } from "path";
 import { getHeapStatistics } from "v8";
 

packages/studio-server/src/history/blobStore.test.ts

@@ -0,0 +1,74 @@
+// @vitest-environment node
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+vi.mock("node:fs", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:fs")>();
+  return { ...actual, renameSync: vi.fn(actual.renameSync) };
+});
+vi.mock("node:fs/promises", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:fs/promises")>();
+  return { ...actual, rename: vi.fn(actual.rename) };
+});
+
+import * as fs from "node:fs";
+import * as fsp from "node:fs/promises";
+import { openBlobStore } from "./blobStore.js";
+
+const { renameSync: realRenameSync } = await vi.importActual<typeof import("node:fs")>("node:fs");
+const { rename: realRename } =
+  await vi.importActual<typeof import("node:fs/promises")>("node:fs/promises");
+const busy = () => Object.assign(new Error("EPERM: operation not permitted"), { code: "EPERM" });
+const platform = process.platform;
+
+// Undo and redo on Windows: a file the preview or an editor holds open refuses a rename over it for a moment.
+describe("the history store on Windows, while another process briefly holds a file", () => {
+  let root: string;
+  beforeEach(() => {
+    root = mkdtempSync(join(tmpdir(), "hf-blob-busy-"));
+    Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+  });
+  afterEach(() => {
+    Object.defineProperty(process, "platform", { value: platform, configurable: true });
+    vi.mocked(fs.renameSync).mockImplementation(realRenameSync);
+    vi.mocked(fsp.rename).mockImplementation(realRename);
+    rmSync(root, { recursive: true, force: true });
+  });
+
+  it("writes a file back into the project once the hold clears", async () => {
+    const source = join(root, "source.html");
+    writeFileSync(source, "<p>before</p>");
+    const store = await openBlobStore(join(root, "history", "blobs"));
+    const hash = await store.put(source);
+    const target = join(root, "index.html");
+    writeFileSync(target, "<p>after</p>");
+    let refused = 0;
+    vi.mocked(fs.renameSync).mockImplementation((from, to) => {
+      if (refused++ < 2) throw busy();
+      return realRenameSync(from, to);
+    });
+
+    await store.writeTo(hash, target);
+
+    expect(readFileSync(target, "utf8")).toBe("<p>before</p>");
+    expect(refused).toBe(3);
+  });
+
+  it("files a copy in the store once the hold clears", async () => {
+    const source = join(root, "clip.html");
+    writeFileSync(source, "<p>kept</p>");
+    const store = await openBlobStore(join(root, "history", "blobs"));
+    let refused = 0;
+    vi.mocked(fsp.rename).mockImplementation(async (from, to) => {
+      if (refused++ < 2) throw busy();
+      return realRename(from, to);
+    });
+
+    const hash = await store.put(source);
+
+    expect((await store.read(hash)).toString("utf8")).toBe("<p>kept</p>");
+    expect(refused).toBe(3);
+  });
+});

packages/studio-server/src/history/blobStore.ts

@@ -1,9 +1,10 @@
 import { createHash, randomUUID } from "node:crypto";
-import { constants, createReadStream, createWriteStream, renameSync } from "node:fs";
-import { copyFile, mkdir, readdir, readFile, rename, rm, stat } from "node:fs/promises";
+import { constants, createReadStream, createWriteStream } from "node:fs";
+import { copyFile, mkdir, readdir, readFile, rm, stat } from "node:fs/promises";
 import { dirname, join } from "node:path";
 import { pipeline } from "node:stream/promises";
 import { atomicTempPath } from "@hyperframes/core/atomic-file";
+import { renameWithRetry, renameWithRetryAsync } from "@hyperframes/core/rename-retry";
 
 /** File contents stored once by sha256, text and binary alike. */
 export interface BlobStore {
@@ -53,7 +54,7 @@ async function cloneOrCopy(from: string, to: string, beforeReplace?: () => void)
   try {
     await copyFile(from, temp, constants.COPYFILE_FICLONE);
     beforeReplace?.();
-    renameSync(temp, to);
+    renameWithRetry(temp, to);
   } catch (error) {
     await rm(temp, { force: true });
     throw error;
@@ -117,7 +118,7 @@ export async function openBlobStore(dir: string): Promise<BlobStore> {
       }
       await mkdir(dirname(blobPath(dir, hash)), { recursive: true });
       const size = (await stat(temp)).size;
-      await rename(temp, blobPath(dir, hash));
+      await renameWithRetryAsync(temp, blobPath(dir, hash));
       sizes.set(hash, size);
       total += size;
       return hash;
@@ -144,7 +145,7 @@ export async function openBlobStore(dir: string): Promise<BlobStore> {
       await mkdir(dirname(to), { recursive: true });
       const moved =
         remove &&
-        (await rename(absPath, to).then(
+        (await renameWithRetryAsync(absPath, to).then(
           () => true,
           () => false,
         ));
@@ -154,7 +155,7 @@ export async function openBlobStore(dir: string): Promise<BlobStore> {
           await rm(temp, { force: true });
           throw error;
         });
-        await rename(temp, to);
+        await renameWithRetryAsync(temp, to);
       }
       asideSizes.set(hash, (await stat(to)).size);
       holds.set(hash, new Set([path]));