From 148b4372655494c3473e5eba32bcd953b4ea3365 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 14 Nov 2026 09:17:22 -0500 Subject: [PATCH 1/3] fix(studio-server): save an edit Windows briefly holds without freezing the app --- docs/contributing/testing-local-changes.mdx | 8 ++ packages/core/src/renameWithRetry.ts | 21 ++++ .../src/history/blobStore.test.ts | 21 ++-- .../studio-server/src/history/blobStore.ts | 15 +-- .../src/routes/files.heldFile.test.ts | 105 +++++++++++++++++- packages/studio-server/src/routes/files.ts | 45 +++++++- 6 files changed, 192 insertions(+), 23 deletions(-) diff --git a/docs/contributing/testing-local-changes.mdx b/docs/contributing/testing-local-changes.mdx index ea911bdfb46..9ce15836e23 100644 --- a/docs/contributing/testing-local-changes.mdx +++ b/docs/contributing/testing-local-changes.mdx @@ -71,6 +71,14 @@ npx ./hyperframes-.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: -sTCP:LISTEN)" +``` + ## What to verify Test the user-visible outcome, not only the exit code: diff --git a/packages/core/src/renameWithRetry.ts b/packages/core/src/renameWithRetry.ts index b6bbf1310f7..ef551d8aa08 100644 --- a/packages/core/src/renameWithRetry.ts +++ b/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,25 @@ export function isWindowsHold(error: unknown, platform = process.platform): bool return RETRYABLE.has((error as NodeJS.ErrnoException | undefined)?.code ?? ""); } +const holdScope = new AsyncLocalStorage<{ open: boolean }>(); + +class HeldRename { + constructor(readonly error: unknown) {} +} + +export async function catchHeldRename(work: () => Promise): Promise { + const scope = { open: true }; + try { + await holdScope.run(scope, work); + return null; + } catch (thrown) { + if (thrown instanceof HeldRename) return thrown.error; + throw thrown; + } finally { + scope.open = false; + } +} + /** 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 +68,7 @@ export function renameWithRetry( } catch (error) { const step = nextStep(error, attempt, options, platform); if (step === "throw") throw error; + if (holdScope.getStore()?.open) throw new HeldRename(error); if (step === "wait") { sleepSync(BACKOFF_MS[attempt]!); continue; diff --git a/packages/studio-server/src/history/blobStore.test.ts b/packages/studio-server/src/history/blobStore.test.ts index a9f421fca5b..6372eea2d07 100644 --- a/packages/studio-server/src/history/blobStore.test.ts +++ b/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(); - return { ...actual, renameSync: vi.fn(actual.renameSync) }; -}); vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); 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("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, "

after

"); - 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, "

after

"); 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, "

after

"); - vi.mocked(fs.renameSync).mockImplementation(() => { + vi.mocked(fsp.rename).mockImplementation(async () => { throw busy(); }); const heard: string[] = []; diff --git a/packages/studio-server/src/history/blobStore.ts b/packages/studio-server/src/history/blobStore.ts index 5ad4ef113d8..a4389a1b575 100644 --- a/packages/studio-server/src/history/blobStore.ts +++ b/packages/studio-server/src/history/blobStore.ts @@ -7,6 +7,7 @@ import { open, readdir, readFile, + rename, rm, stat, writeFile, @@ -14,11 +15,7 @@ import { 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); diff --git a/packages/studio-server/src/routes/files.heldFile.test.ts b/packages/studio-server/src/routes/files.heldFile.test.ts index 62dc7756a9a..39c0a6e1e25 100644 --- a/packages/studio-server/src/routes/files.heldFile.test.ts +++ b/packages/studio-server/src/routes/files.heldFile.test.ts @@ -1,6 +1,13 @@ -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { Hono } from "hono"; import { type ChildProcess, spawn, spawnSync } from "node:child_process"; + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, renameSync: vi.fn(actual.renameSync) }; +}); + +import * as fs from "node:fs"; import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -12,7 +19,7 @@ afterEach(async () => { for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); }); -function fixture(name: string) { +function fixture(name: string, onError?: (error: Error) => void) { const project = mkdtempSync(join(tmpdir(), "hf-held-file-")); mkdirSync(join(project, "sub")); const file = join(project, name); @@ -28,6 +35,11 @@ function fixture(name: string) { startRender: () => ({ id: "job", status: "rendering", progress: 0, outputPath: "out.mp4" }), }; const app = new Hono(); + if (onError) + app.onError((error, c) => { + onError(error); + return c.json({ error: "failed" }, 500); + }); registerFileRoutes(app, adapter); const get = (query = "") => app.request(`http://localhost/projects/demo/files/${name}${query}`); const put = () => @@ -119,3 +131,92 @@ describe.runIf(process.platform === "win32")("a file another program holds open expect(await answered(await put())).toEqual({ status: 403, why: "permission" }); }); }); + +/** Windows refusing every replace of `file` (EBUSY, as a scanner's open handle does) for `ms`. */ +function holdFor(file: string, ms: number) { + const platform = process.platform; + const realRenameSync = vi.mocked(fs.renameSync).getMockImplementation()!; + const releaseAt = Date.now() + ms; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + vi.mocked(fs.renameSync).mockImplementation((from, to) => { + if (to === file && Date.now() < releaseAt) { + throw Object.assign(new Error("EBUSY: resource busy or locked"), { code: "EBUSY" }); + } + return realRenameSync(from, to); + }); + cleanups.push(() => { + Object.defineProperty(process, "platform", { value: platform, configurable: true }); + vi.mocked(fs.renameSync).mockImplementation(realRenameSync); + }); +} + +/** The longest the event loop went without running a 5 ms timer while `work` ran. */ +async function longestStall(work: () => Promise) { + let last = performance.now(); + let longest = 0; + const tick = setInterval(() => { + const now = performance.now(); + longest = Math.max(longest, now - last); + last = now; + }, 5); + try { + await work(); + } finally { + clearInterval(tick); + } + return longest; +} + +describe("an element edit while Windows briefly holds the file", () => { + const edited = '
kept
'; + + it.each([200, 3_000])( + "still saves the edit when the hold clears after %i ms, and reports nothing", + async (ms) => { + const reported: unknown[] = []; + const { file, remove } = fixture("held.html", (error) => reported.push(error)); + writeFileSync(file, `${edited}

tag

`); + holdFor(file, ms); + expect((await remove("gone")).status).toBe(200); + expect(readFileSync(file, "utf8")).not.toContain("tag"); + expect(reported).toEqual([]); + }, + 10_000, + ); + + // fails without the fix: each retry slept the whole process, up to 640 ms in one go. + it("keeps the app answering while it waits", async () => { + const { file, remove } = fixture("held.html"); + writeFileSync(file, `${edited}

tag

`); + holdFor(file, 1_500); + expect( + await longestStall(async () => expect((await remove("gone")).status).toBe(200)), + ).toBeLessThan(100); + }, 10_000); + + it("answers 423 busy once the hold outlasts the wait, reports it once, and leaves the file as it was", async () => { + const reported: string[] = []; + const { file, remove } = fixture("held.html", (error) => + reported.push(String((error as NodeJS.ErrnoException).code)), + ); + writeFileSync(file, `${edited}

tag

`); + holdFor(file, 60_000); + expect(await answered(await remove("gone"))).toEqual({ status: 423, why: "busy" }); + expect(readFileSync(file, "utf8")).toBe(`${edited}

tag

`); + expect(reported).toEqual(["EBUSY"]); + }, 15_000); + + it.skipIf(process.getuid?.() === 0)( + "refuses a read-only file at once, without waiting", + async () => { + const { file, remove } = fixture("readonly.html"); + writeFileSync(file, `${edited}

tag

`); + chmodSync(file, 0o444); + cleanups.push(() => chmodSync(file, 0o644)); + holdFor(file, 0); + const started = performance.now(); + expect((await remove("gone")).status).toBe(403); + expect(performance.now() - started).toBeLessThan(500); + }, + ); +}); diff --git a/packages/studio-server/src/routes/files.ts b/packages/studio-server/src/routes/files.ts index c7abf56f492..e49012e98b5 100644 --- a/packages/studio-server/src/routes/files.ts +++ b/packages/studio-server/src/routes/files.ts @@ -31,7 +31,7 @@ import type { StudioApiAdapter } from "../types.js"; import { isAudioFile } from "../helpers/mime.js"; import { lengthIsTimeline } from "../helpers/compositionInputs.js"; import { createFileAtomically, replaceFileAtomically } from "@hyperframes/core/atomic-file"; -import { retryWindowsHold } from "@hyperframes/core/rename-retry"; +import { catchHeldRename, retryWindowsHold } from "@hyperframes/core/rename-retry"; import { generateWaveformCache } from "../helpers/waveform.js"; import { validateUploadedMediaBuffer } from "../helpers/mediaValidation.js"; import { @@ -2797,6 +2797,28 @@ async function processUploadedFiles( const MAX_TEXT_READ_BYTES = 32 * 1024 * 1024; const GIT_BINARY_SNIFF_BYTES = 8000; +const HELD_EDIT_WAITS = [10, 20, 40, 80, 160, 320, 640, 1280, 2560]; +const EDIT_ROUTE = /\/projects\/[^/]+\/(files|file-mutations)\//; +const REPLAYED = "x-hyperframes-held-replay"; +const HELD = "x-hyperframes-held"; + +function isEdit(request: Request): boolean { + return ( + !["GET", "HEAD"].includes(request.method) && + EDIT_ROUTE.test(new URL(request.url).pathname) && + !(request.headers.get("content-type") ?? "").startsWith("multipart/") + ); +} + +function keptForReplay(request: Request): () => Request { + const kept = request.clone(); + return () => { + const again = new Request(kept.clone(), { headers: new Headers(kept.headers) }); + again.headers.set(REPLAYED, "1"); + return again; + }; +} + function heldFileAnswer(c: RouteContext, error: unknown) { const code = (error as NodeJS.ErrnoException | null)?.code; if (code === "EBUSY") return c.json({ error: "file in use", why: "busy" }, 423); @@ -2813,6 +2835,27 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { if (refusedWrite) c.res = refusedWrite; }); + api.use("/projects/:id/*", async (c, next) => { + if (!isEdit(c.req.raw)) return next(); + const replayed = c.req.raw.headers.has(REPLAYED); + const replay = replayed ? null : keptForReplay(c.req.raw); + const held = await catchHeldRename(next); + if (held === null) return; + if (!replay) { + c.res = new Response(null, { status: 423, headers: { [HELD]: "1" } }); + return; + } + for (const ms of HELD_EDIT_WAITS) { + await new Promise((resolve) => setTimeout(resolve, ms)); + const again = await api.fetch(replay()); + if (!again.headers.has(HELD)) { + c.res = again; + return; + } + } + throw held; + }); + // ── Read ── api.get("/projects/:id/files/*", async (c) => { From 10aa434833052253dbe1ba15b6d6d3ae5f499daf Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 14 Nov 2026 10:14:39 -0500 Subject: [PATCH 2/3] fix(studio-server): keep later edits behind a held one, one backup per replay --- .../src/helpers/backupJournal.ts | 16 ++-- .../src/routes/files.heldFile.test.ts | 29 ++++++- .../src/routes/files.heldReplay.test.ts | 84 +++++++++++++++++++ packages/studio-server/src/routes/files.ts | 33 +++++--- .../src/routes/freezeFrame.test.ts | 17 ++++ .../studio-server/src/routes/freezeFrame.ts | 12 ++- 6 files changed, 169 insertions(+), 22 deletions(-) create mode 100644 packages/studio-server/src/routes/files.heldReplay.test.ts diff --git a/packages/studio-server/src/helpers/backupJournal.ts b/packages/studio-server/src/helpers/backupJournal.ts index 5199eba3e33..0848fa346fc 100644 --- a/packages/studio-server/src/helpers/backupJournal.ts +++ b/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 { diff --git a/packages/studio-server/src/routes/files.heldFile.test.ts b/packages/studio-server/src/routes/files.heldFile.test.ts index 39c0a6e1e25..1c028d2da27 100644 --- a/packages/studio-server/src/routes/files.heldFile.test.ts +++ b/packages/studio-server/src/routes/files.heldFile.test.ts @@ -54,7 +54,16 @@ function fixture(name: string, onError?: (error: Error) => void) { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ targets: [{ id }] }), }); - return { file, get, put, remove }; + const patch = (color: string) => + app.request(`http://localhost/projects/demo/file-mutations/patch-element/${name}`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + target: { id: "title" }, + operations: [{ type: "inline-style", property: "color", value: color }], + }), + }); + return { file, get, put, remove, patch }; } const answered = async (response: Response) => ({ @@ -184,6 +193,24 @@ describe("an element edit while Windows briefly holds the file", () => { 10_000, ); + it("keeps a later edit to the project behind the held one, so the later edit stands", async () => { + const { file, patch } = fixture("held.html"); + writeFileSync(file, '

A

'); + holdFor(file, 1_000); + const order: string[] = []; + const send = (color: string) => + patch(color).then((response) => { + order.push(color); + return response.status; + }); + const first = send("red"); + await new Promise((resolve) => setTimeout(resolve, 1_050)); + const second = send("blue"); + expect([await first, await second]).toEqual([200, 200]); + expect(order).toEqual(["red", "blue"]); + expect(readFileSync(file, "utf8")).toContain("color: blue"); + }, 10_000); + // fails without the fix: each retry slept the whole process, up to 640 ms in one go. it("keeps the app answering while it waits", async () => { const { file, remove } = fixture("held.html"); diff --git a/packages/studio-server/src/routes/files.heldReplay.test.ts b/packages/studio-server/src/routes/files.heldReplay.test.ts new file mode 100644 index 00000000000..5b76fd2ab19 --- /dev/null +++ b/packages/studio-server/src/routes/files.heldReplay.test.ts @@ -0,0 +1,84 @@ +// @vitest-environment node +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, renameSync: vi.fn(actual.renameSync) }; +}); + +import * as fs from "node:fs"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createStudioApi } from "../createStudioApi"; +import { fileContentVersion } from "../helpers/fileVersion"; +import { openProjectHistory } from "../history/projectHistory"; +import type { StudioApiAdapter } from "../types"; + +const cleanups: Array<() => unknown> = []; +afterEach(async () => { + for (const cleanup of cleanups.splice(0).reverse()) await cleanup(); +}); + +function tempDir(prefix: string) { + const dir = mkdtempSync(join(tmpdir(), prefix)); + cleanups.push(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +/** Windows refusing every replace of `file` (EBUSY) for `ms`. */ +function holdFor(file: string, ms: number) { + const platform = process.platform; + const realRenameSync = vi.mocked(fs.renameSync).getMockImplementation()!; + const releaseAt = Date.now() + ms; + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + vi.mocked(fs.renameSync).mockImplementation((from, to) => { + if (to === file && Date.now() < releaseAt) { + throw Object.assign(new Error("EBUSY: resource busy or locked"), { code: "EBUSY" }); + } + return realRenameSync(from, to); + }); + cleanups.push(() => { + Object.defineProperty(process, "platform", { value: platform, configurable: true }); + vi.mocked(fs.renameSync).mockImplementation(realRenameSync); + }); +} + +describe("an element edit sent again after Windows held the file", () => { + it("applies once, keeps one backup and leaves one history entry", async () => { + const projectDir = tempDir("hf-held-replay-"); + const index = join(projectDir, "index.html"); + const before = '

A

'; + writeFileSync(index, before); + const history = await openProjectHistory({ + projectDir, + historyRoot: tempDir("hf-held-replay-history-"), + }); + cleanups.push(() => history.close()); + await history.flush(); + const api = createStudioApi({ + listProjects: () => [], + resolveProject: (id: string) => (id === "demo" ? { id, dir: projectDir } : null), + history: () => history, + } as unknown as StudioApiAdapter); + const post = (path: string, body: object) => + api.request(`/projects/demo${path}`, { method: "POST", body: JSON.stringify(body) }); + + holdFor(index, 200); + const patched = await post("/file-mutations/patch-element/index.html", { + target: { id: "title" }, + operations: [{ type: "inline-style", property: "z-index", value: "2" }], + }); + expect(patched.status).toBe(200); + await post("/history/claim", { + label: "Moved Title", + paths: ["index.html"], + overwrote: { "index.html": fileContentVersion(before) }, + }); + await history.flush(); + + expect(readFileSync(index, "utf8").match(/z-index/g)).toHaveLength(1); + expect(readdirSync(join(projectDir, ".hyperframes", "backup"))).toHaveLength(1); + expect(history.list().map((entry) => entry.label)).toEqual(["Moved Title"]); + }); +}); diff --git a/packages/studio-server/src/routes/files.ts b/packages/studio-server/src/routes/files.ts index e49012e98b5..98d05061a3e 100644 --- a/packages/studio-server/src/routes/files.ts +++ b/packages/studio-server/src/routes/files.ts @@ -2798,13 +2798,14 @@ const MAX_TEXT_READ_BYTES = 32 * 1024 * 1024; const GIT_BINARY_SNIFF_BYTES = 8000; const HELD_EDIT_WAITS = [10, 20, 40, 80, 160, 320, 640, 1280, 2560]; -const EDIT_ROUTE = /\/projects\/[^/]+\/(files|file-mutations)\//; +const EDIT_ROUTE = + /\/projects\/[^/]+\/(files|file-mutations|gsap-mutations(-batch)?|gsap-mutation-rollback)\//; const REPLAYED = "x-hyperframes-held-replay"; const HELD = "x-hyperframes-held"; function isEdit(request: Request): boolean { return ( - !["GET", "HEAD"].includes(request.method) && + ["POST", "PUT"].includes(request.method) && EDIT_ROUTE.test(new URL(request.url).pathname) && !(request.headers.get("content-type") ?? "").startsWith("multipart/") ); @@ -2819,6 +2820,15 @@ function keptForReplay(request: Request): () => Request { }; } +async function sendAgainOnceFree(api: Hono, replay: () => Request): Promise { + for (const ms of HELD_EDIT_WAITS) { + await new Promise((resolve) => setTimeout(resolve, ms)); + const again = await api.fetch(replay()); + if (!again.headers.has(HELD)) return again; + } + return null; +} + function heldFileAnswer(c: RouteContext, error: unknown) { const code = (error as NodeJS.ErrnoException | null)?.code; if (code === "EBUSY") return c.json({ error: "file in use", why: "busy" }, 423); @@ -2835,9 +2845,12 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { if (refusedWrite) c.res = refusedWrite; }); + const heldEdits = new Map>(); api.use("/projects/:id/*", async (c, next) => { if (!isEdit(c.req.raw)) return next(); + const project = c.req.param("id") ?? ""; const replayed = c.req.raw.headers.has(REPLAYED); + while (!replayed && heldEdits.has(project)) await heldEdits.get(project); const replay = replayed ? null : keptForReplay(c.req.raw); const held = await catchHeldRename(next); if (held === null) return; @@ -2845,15 +2858,13 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { c.res = new Response(null, { status: 423, headers: { [HELD]: "1" } }); return; } - for (const ms of HELD_EDIT_WAITS) { - await new Promise((resolve) => setTimeout(resolve, ms)); - const again = await api.fetch(replay()); - if (!again.headers.has(HELD)) { - c.res = again; - return; - } - } - throw held; + const waiting = sendAgainOnceFree(api, replay); + heldEdits.set(project, waiting); + const again = await waiting.finally(() => { + if (heldEdits.get(project) === waiting) heldEdits.delete(project); + }); + if (!again) throw held; + c.res = again; }); // ── Read ── diff --git a/packages/studio-server/src/routes/freezeFrame.test.ts b/packages/studio-server/src/routes/freezeFrame.test.ts index 92ed21877f7..c2ca0a25338 100644 --- a/packages/studio-server/src/routes/freezeFrame.test.ts +++ b/packages/studio-server/src/routes/freezeFrame.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { Hono } from "hono"; import { + chmodSync, existsSync, mkdirSync, mkdtempSync, @@ -173,6 +174,22 @@ describe("freeze-frame route", () => { expect(freezeFiles(dir)).toEqual([]); }); + it.skipIf(process.getuid?.() === 0)( + "removes the still it extracted when the write throws, as for a read-only page", + async () => { + const { dir, post } = setup(pngWriter([])); + chmodSync(join(dir, "index.html"), 0o444); + const res = await post({ + path: "index.html", + expectedVersion: fileContentVersion(html), + target: { id: "talk" }, + playhead: 2.5, + }); + expect(res.status).toBe(500); + expect(freezeFiles(dir)).toEqual([]); + }, + ); + it("keeps a traversal clip id inside assets/freeze, for the ffmpeg output and the still's src", async () => { const evil = html.replace('id="talk"', 'id="../../../../outside/frame"'); const calls: string[][] = []; diff --git a/packages/studio-server/src/routes/freezeFrame.ts b/packages/studio-server/src/routes/freezeFrame.ts index 14d0193e9c3..bc22d9ddb49 100644 --- a/packages/studio-server/src/routes/freezeFrame.ts +++ b/packages/studio-server/src/routes/freezeFrame.ts @@ -218,9 +218,15 @@ export function registerFreezeFrameRoutes( return c.json({ error }, status); }; if (!folded) return failAndRemoveStill("Freeze target was not found in the file", 400); - const written = writeFolded(project.dir, absPath, body.path, before, folded.html, { - token: body.transactionToken ?? c.req.header("X-Hyperframes-Write-Token"), - }); + let written: ReturnType; + try { + written = writeFolded(project.dir, absPath, body.path, before, folded.html, { + token: body.transactionToken ?? c.req.header("X-Hyperframes-Write-Token"), + }); + } catch (error) { + rmSync(imagePath, { force: true }); + throw error; + } if ("error" in written) return failAndRemoveStill(written.error, written.status); const { version, writeToken, backupPath } = written; return c.json({ From f55285eec4b151f14b1390c2171e56f7739fcd8e Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 14 Nov 2026 10:42:44 -0500 Subject: [PATCH 3/3] fix(studio-server): send a held cut again, and keep the sync retry for rollbacks --- packages/core/src/renameWithRetry.test.ts | 18 ++++++++++++- packages/core/src/renameWithRetry.ts | 19 ++++++++----- .../src/routes/files.heldFile.test.ts | 27 ++++++++++++++++++- packages/studio-server/src/routes/files.ts | 2 +- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/packages/core/src/renameWithRetry.test.ts b/packages/core/src/renameWithRetry.test.ts index e7627bc9268..883b1a1f168 100644 --- a/packages/core/src/renameWithRetry.test.ts +++ b/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); + }); +}); diff --git a/packages/core/src/renameWithRetry.ts b/packages/core/src/renameWithRetry.ts index ef551d8aa08..93677f60c76 100644 --- a/packages/core/src/renameWithRetry.ts +++ b/packages/core/src/renameWithRetry.ts @@ -19,23 +19,30 @@ export function isWindowsHold(error: unknown, platform = process.platform): bool return RETRYABLE.has((error as NodeJS.ErrnoException | undefined)?.code ?? ""); } -const holdScope = new AsyncLocalStorage<{ open: boolean }>(); +const holdScope = new AsyncLocalStorage<{ open: boolean; held: unknown }>(); class HeldRename { constructor(readonly error: unknown) {} } export async function catchHeldRename(work: () => Promise): Promise { - const scope = { open: true }; + const scope: { open: boolean; held: unknown } = { open: true, held: null }; try { await holdScope.run(scope, work); - return null; } catch (thrown) { - if (thrown instanceof HeldRename) return thrown.error; - throw 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. */ @@ -68,7 +75,7 @@ export function renameWithRetry( } catch (error) { const step = nextStep(error, attempt, options, platform); if (step === "throw") throw error; - if (holdScope.getStore()?.open) throw new HeldRename(error); + handOverHold(error); if (step === "wait") { sleepSync(BACKOFF_MS[attempt]!); continue; diff --git a/packages/studio-server/src/routes/files.heldFile.test.ts b/packages/studio-server/src/routes/files.heldFile.test.ts index 1c028d2da27..43bbbc4771d 100644 --- a/packages/studio-server/src/routes/files.heldFile.test.ts +++ b/packages/studio-server/src/routes/files.heldFile.test.ts @@ -12,6 +12,7 @@ import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from "node:os"; import { join } from "node:path"; import { registerFileRoutes } from "./files"; +import { fileContentVersion } from "../helpers/fileVersion"; import type { StudioApiAdapter } from "../types"; const cleanups: Array<() => void | Promise> = []; @@ -63,7 +64,22 @@ function fixture(name: string, onError?: (error: Error) => void) { operations: [{ type: "inline-style", property: "color", value: color }], }), }); - return { file, get, put, remove, patch }; + const cut = (version: string) => + app.request("http://localhost/projects/demo/file-mutations/split-batch", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + transactionToken: "cut", + files: [ + { + path: name, + expectedVersion: version, + targets: [{ target: { id: "a" }, splitTime: 2, elementStart: 0, elementDuration: 4 }], + }, + ], + }), + }); + return { file, get, put, remove, patch, cut }; } const answered = async (response: Response) => ({ @@ -193,6 +209,15 @@ describe("an element edit while Windows briefly holds the file", () => { 10_000, ); + it("still saves a cut, whose route answers a failed write itself, when the hold clears", async () => { + const { file, cut } = fixture("held.html"); + const before = '
A
'; + writeFileSync(file, before); + holdFor(file, 200); + expect((await cut(fileContentVersion(before))).status).toBe(200); + expect(readFileSync(file, "utf8").match(/data-duration/g)).toHaveLength(2); + }, 10_000); + it("keeps a later edit to the project behind the held one, so the later edit stands", async () => { const { file, patch } = fixture("held.html"); writeFileSync(file, '

A

'); diff --git a/packages/studio-server/src/routes/files.ts b/packages/studio-server/src/routes/files.ts index 98d05061a3e..856abf45788 100644 --- a/packages/studio-server/src/routes/files.ts +++ b/packages/studio-server/src/routes/files.ts @@ -2853,7 +2853,7 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void { while (!replayed && heldEdits.has(project)) await heldEdits.get(project); const replay = replayed ? null : keptForReplay(c.req.raw); const held = await catchHeldRename(next); - if (held === null) return; + if (held === null || (c.finalized && c.res.ok)) return; if (!replay) { c.res = new Response(null, { status: 423, headers: { [HELD]: "1" } }); return;