fix(studio-server): save an edit Windows briefly holds without freezing the app

docs/contributing/testing-local-changes.mdx

@@ -71,6 +71,14 @@ npx ./hyperframes-<version>.tgz --help
 
 Run the archive against an isolated project as well as the command you changed.
 
+A `hyperframes preview` started through `npx` keeps its server running when the
+`npx` process is stopped. Stop the server by the process listening on its port,
+or the next run talks to the old one:
+
+```bash
+kill "$(lsof -ti tcp:<port> -sTCP:LISTEN)"
+```
+
 ## What to verify
 
 Test the user-visible outcome, not only the exit code:

packages/core/src/renameWithRetry.test.ts

@@ -1,6 +1,6 @@
 // @vitest-environment node
 import { describe, expect, it } from "vitest";
-import { renameWithRetry, renameWithRetryAsync } from "./renameWithRetry.js";
+import { catchHeldRename, renameWithRetry, renameWithRetryAsync } from "./renameWithRetry.js";
 
 function failing(code: string, times: number) {
   let calls = 0;
@@ -88,3 +88,19 @@ describe("renameWithRetry", () => {
     expect(copied).toEqual([]);
   });
 });
+
+describe("catchHeldRename", () => {
+  it("hands back the first hold, even one a caller swallowed, and rides later holds out as before", async () => {
+    const rename = failing("EBUSY", 2);
+    const held = await catchHeldRename(async () => {
+      try {
+        renameWithRetry("a", "b", { platform: "win32", rename: rename.fn });
+      } catch {
+        // a route that answers a failed write itself
+      }
+      renameWithRetry("a", "b", { platform: "win32", rename: rename.fn });
+    });
+    expect(held).toMatchObject({ code: "EBUSY" });
+    expect(rename.calls()).toBe(3);
+  });
+});

packages/core/src/renameWithRetry.ts

@@ -1,3 +1,4 @@
+import { AsyncLocalStorage } from "node:async_hooks";
 import { copyFileSync, renameSync } from "node:fs";
 import { copyFile, rename } from "node:fs/promises";
 import { removeScratch } from "./removeScratch.js";
@@ -18,6 +19,32 @@ export function isWindowsHold(error: unknown, platform = process.platform): bool
   return RETRYABLE.has((error as NodeJS.ErrnoException | undefined)?.code ?? "");
 }
 
+const holdScope = new AsyncLocalStorage<{ open: boolean; held: unknown }>();
+
+class HeldRename {
+  constructor(readonly error: unknown) {}
+}
+
+export async function catchHeldRename(work: () => Promise<void>): Promise<unknown> {
+  const scope: { open: boolean; held: unknown } = { open: true, held: null };
+  try {
+    await holdScope.run(scope, work);
+  } catch (thrown) {
+    if (!(thrown instanceof HeldRename)) throw thrown;
+  } finally {
+    scope.open = false;
+  }
+  return scope.held;
+}
+
+function handOverHold(error: unknown): void {
+  const scope = holdScope.getStore();
+  if (!scope?.open) return;
+  scope.open = false;
+  scope.held = error;
+  throw new HeldRename(error);
+}
+
 /** What a refused rename does next: wait and try again, fall back to a copy, or give up with the error. */
 function nextStep(error: unknown, attempt: number, options: Options, platform: NodeJS.Platform) {
   if (!isWindowsHold(error, platform)) return "throw";
@@ -48,6 +75,7 @@ export function renameWithRetry(
     } catch (error) {
       const step = nextStep(error, attempt, options, platform);
       if (step === "throw") throw error;
+      handOverHold(error);
       if (step === "wait") {
         sleepSync(BACKOFF_MS[attempt]!);
         continue;

packages/studio-server/src/helpers/backupJournal.ts

@@ -50,6 +50,8 @@ export function snapshotBeforeWrite(
       pathKey.length <= 200
         ? pathKey
         : `sha256~${createHash("sha256").update(relativePath).digest("hex")}`;
+    const [newest] = backupsNewestFirst(backupDir, [pathKey, backupKey]);
+    if (newest && readFileSync(newest).equals(content)) return { backupPath: newest };
     const backupPath = nextBackupPath(backupDir, backupKey);
     writeFileSync(backupPath, content, { flag: "wx" });
     pruneBackups(backupDir, [pathKey, backupKey], options.keepPerFile ?? DEFAULT_KEEP_PER_FILE);
@@ -86,17 +88,17 @@ function nextBackupPath(backupDir: string, backupKey: string): string {
   }
 }
 
-function pruneBackups(backupDir: string, backupKeys: string[], keepPerFile: number): void {
-  const keep = Math.max(1, Math.floor(keepPerFile));
+function backupsNewestFirst(backupDir: string, backupKeys: string[]): string[] {
   const suffix = new RegExp(`-(?:${backupKeys.join("|")})(?:-\\d+)?$`);
-  const matches = readdirSync(backupDir)
+  return readdirSync(backupDir)
     .filter((name) => suffix.test(name))
     .map((name) => join(backupDir, name))
-    .sort((a, b) => {
-      return b.localeCompare(a);
-    });
+    .sort((a, b) => b.localeCompare(a));
+}
 
-  for (const file of matches.slice(keep)) {
+function pruneBackups(backupDir: string, backupKeys: string[], keepPerFile: number): void {
+  const keep = Math.max(1, Math.floor(keepPerFile));
+  for (const file of backupsNewestFirst(backupDir, backupKeys).slice(keep)) {
     try {
       unlinkSync(file);
     } catch {

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

@@ -13,10 +13,6 @@ 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 {
@@ -32,7 +28,6 @@ import * as fsp from "node:fs/promises";
 import { onRestoredInPlace, openBlobStore } from "./blobStore.js";
 import { serveWhileOpen } from "../helpers/servedFiles.js";
 
-const { renameSync: realRenameSync } = await vi.importActual<typeof import("node:fs")>("node:fs");
 const {
   rename: realRename,
   copyFile: realCopyFile,
@@ -50,7 +45,7 @@ describe("the history store on Windows, while another process briefly holds a fi
   });
   afterEach(() => {
     Object.defineProperty(process, "platform", { value: platform, configurable: true });
-    vi.mocked(fs.renameSync).mockImplementation(realRenameSync);
+    vi.mocked(fsp.rename).mockImplementation(realRename);
     vi.mocked(fsp.rename).mockImplementation(realRename);
     vi.mocked(fsp.copyFile).mockImplementation(realCopyFile);
     rmSync(root, { recursive: true, force: true });
@@ -63,7 +58,7 @@ describe("the history store on Windows, while another process briefly holds a fi
     const store = await openBlobStore(join(root, "history", "blobs"));
     const hash = await store.put(target);
     writeFileSync(target, "<p>after</p>");
-    vi.mocked(fs.renameSync).mockImplementation(() => {
+    vi.mocked(fsp.rename).mockImplementation(async () => {
       throw busy();
     });
     const heard: string[] = [];
@@ -126,9 +121,9 @@ describe("the history store on Windows, while another process briefly holds a fi
     const target = join(root, "index.html");
     writeFileSync(target, "<p>after</p>");
     let refused = 0;
-    vi.mocked(fs.renameSync).mockImplementation((from, to) => {
+    vi.mocked(fsp.rename).mockImplementation(async (from, to) => {
       if (refused++ < 2) throw busy();
-      return realRenameSync(from, to);
+      return realRename(from, to);
     });
 
     await store.writeTo(hash, target);
@@ -163,7 +158,7 @@ describe("a restore of a file the preview is serving", () => {
     root = mkdtempSync(join(tmpdir(), "hf-blob-served-"));
   });
   afterEach(() => {
-    vi.mocked(fs.renameSync).mockImplementation(realRenameSync);
+    vi.mocked(fsp.rename).mockImplementation(realRename);
     rmSync(root, { recursive: true, force: true });
   });
 
@@ -176,9 +171,9 @@ describe("a restore of a file the preview is serving", () => {
     await new Promise((resolve) => read.once("data", resolve));
     read.pause();
     let openAtReplace = true;
-    vi.mocked(fs.renameSync).mockImplementation((from, to) => {
+    vi.mocked(fsp.rename).mockImplementation(async (from, to) => {
       openAtReplace = !read.closed;
-      return realRenameSync(from, to);
+      return realRename(from, to);
     });
 
     await store.writeTo(hash, target);
@@ -192,7 +187,7 @@ describe("a restore of a file the preview is serving", () => {
     const store = await openBlobStore(join(root, "history", "blobs"));
     const hash = await store.put(target);
     writeFileSync(target, "<p>after</p>");
-    vi.mocked(fs.renameSync).mockImplementation(() => {
+    vi.mocked(fsp.rename).mockImplementation(async () => {
       throw busy();
     });
     const heard: string[] = [];

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

@@ -7,18 +7,15 @@ import {
   open,
   readdir,
   readFile,
+  rename,
   rm,
   stat,
   writeFile,
 } from "node:fs/promises";
 import { dirname, join } from "node:path";
 import { pipeline } from "node:stream/promises";
 import { atomicTempPath, refuseReadOnly } from "@hyperframes/core/atomic-file";
-import {
-  isWindowsHold,
-  renameWithRetry,
-  renameWithRetryAsync,
-} from "@hyperframes/core/rename-retry";
+import { isWindowsHold, renameWithRetryAsync } from "@hyperframes/core/rename-retry";
 import { stopServingAndClose } from "../helpers/servedFiles.js";
 
 /** File contents stored once by sha256, text and binary alike. */
@@ -111,9 +108,13 @@ async function cloneOrCopy(
   try {
     await copyFile(from, temp, constants.COPYFILE_FICLONE);
     await stopServingAndClose(to);
-    beforeReplace?.();
     try {
-      renameWithRetry(temp, to);
+      await renameWithRetryAsync(temp, to, {
+        rename: async (from, to) => {
+          beforeReplace?.();
+          await rename(from, to);
+        },
+      });
     } catch (error) {
       if (!isWindowsHold(error)) throw error;
       await writeInPlace(temp, to, hash);