diff --git a/packages/cli/src/server/fileWatcher.atomic.test.ts b/packages/cli/src/server/fileWatcher.atomic.test.ts new file mode 100644 index 00000000000..76631f8a548 --- /dev/null +++ b/packages/cli/src/server/fileWatcher.atomic.test.ts @@ -0,0 +1,64 @@ +// @vitest-environment node +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; +import { replaceFileAtomically } from "@hyperframes/core/atomic-file"; +import { openProjectHistory } from "@hyperframes/studio-server"; +import { createProjectWatcher } from "./fileWatcher.js"; + +function tempDir(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + onTestFinished(() => rmSync(dir, { recursive: true, force: true })); + return dir; +} + +function watchedProject() { + const dir = tempDir("hf-watch-atomic-"); + const index = join(dir, "index.html"); + writeFileSync(index, "

Hello

"); + const watcher = createProjectWatcher(dir); + const heard: string[] = []; + watcher.addListener((path) => heard.push(path)); + onTestFinished(() => watcher.close()); + return { dir, index, heard }; +} + +// Past the watcher's 300 ms burst window, so every event of the write has been delivered. +const quiet = () => new Promise((settle) => setTimeout(settle, 600)); + +async function heardOnly(heard: string[], path: string) { + await vi.waitFor(() => expect(heard).toContain(path)); + await quiet(); + expect(new Set(heard)).toEqual(new Set([path])); +} + +describe("the project watcher, on a real file system", () => { + it("hears a save only as the file it replaced, never its temp file", async () => { + const { index, heard } = watchedProject(); + replaceFileAtomically(index, "

Bye

"); + await heardOnly(heard, "index.html"); + }); + + it("hears a history step's write only as the file it restored, never its temp file", async () => { + const { dir, index, heard } = watchedProject(); + const history = await openProjectHistory({ + projectDir: dir, + historyRoot: tempDir("hf-hist-"), + quietMs: 30, + }); + onTestFinished(() => history.close()); + writeFileSync(index, "

Bye

"); + history.noteChange("index.html"); + await vi.waitFor(() => expect(history.list()).toHaveLength(1)); + await quiet(); + heard.length = 0; + + const undone = await history.undo(history.list()[0]!.id, { + who: { kind: "person", name: "You" }, + }); + expect(undone).toMatchObject({ ok: true }); + expect(readFileSync(index, "utf-8")).toBe("

Hello

"); + await heardOnly(heard, "index.html"); + }); +}); diff --git a/packages/cli/src/server/fileWatcher.test.ts b/packages/cli/src/server/fileWatcher.test.ts index 7555b5fa18a..05d1b495e25 100644 --- a/packages/cli/src/server/fileWatcher.test.ts +++ b/packages/cli/src/server/fileWatcher.test.ts @@ -47,6 +47,13 @@ describe("shouldWatchProjectFile", () => { expect(shouldWatchProjectFile(".thumbnails/frame.jpg")).toBe(false); expect(shouldWatchProjectFile(".waveform-cache/peaks.json")).toBe(false); }); + + it("skips the temp file of a save in flight, but not a user's own .tmp file", () => { + expect(shouldWatchProjectFile("index.html.hf0a1b2c.tmp")).toBe(false); + expect(shouldWatchProjectFile("compositions/intro.html.hf0a1b2c.tmp")).toBe(false); + expect(shouldWatchProjectFile("foo.12345678.tmp")).toBe(true); + expect(shouldWatchProjectFile("notes.tmp")).toBe(true); + }); }); describe("createProjectWatcher", () => { diff --git a/packages/cli/src/server/fileWatcher.ts b/packages/cli/src/server/fileWatcher.ts index 50d7b010e64..1f04477724b 100644 --- a/packages/cli/src/server/fileWatcher.ts +++ b/packages/cli/src/server/fileWatcher.ts @@ -1,5 +1,6 @@ import { lstatSync, readdirSync, watch, type FSWatcher } from "node:fs"; import { join, relative, sep } from "node:path"; +import { isAtomicTempPath } from "@hyperframes/core/atomic-file"; import { affectsProjectSignature } from "@hyperframes/studio-server"; export type FileChangeListener = (relativePath: string) => void; @@ -32,7 +33,7 @@ const QUIET_MS = 30; const BURST_MS = 300; export function shouldWatchProjectFile(filename: string): boolean { - if (!filename) return false; + if (!filename || isAtomicTempPath(filename)) return false; const parts = filename.split(/[\\/]+/); return !parts.some((part) => WATCHER_EXCLUDED_DIRS.has(part)); } diff --git a/packages/core/src/atomicFile.test.ts b/packages/core/src/atomicFile.test.ts index 6b65d9b34e6..cfff0831fd0 100644 --- a/packages/core/src/atomicFile.test.ts +++ b/packages/core/src/atomicFile.test.ts @@ -5,19 +5,34 @@ import * as fs from "node:fs"; import { lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { createFileAtomically, replaceFileAtomically, resolveWritePath } from "./atomicFile.js"; +import { + atomicTempPath, + createFileAtomically, + isAtomicTempPath, + replaceFileAtomically, + resolveWritePath, +} from "./atomicFile.js"; vi.mock("node:crypto", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, randomBytes: vi.fn(actual.randomBytes) }; }); -/** `.<8 hex>.tmp`, checked without building a regex from a path (Windows paths hold backslashes). */ +/** `.hf<6 hex>.tmp`, checked without building a regex from a path (Windows paths hold backslashes). */ function expectTempSiblingOf(tempPath: string, file: string): void { expect(tempPath.startsWith(`${file}.`)).toBe(true); - expect(tempPath.slice(file.length)).toMatch(/^\.[0-9a-f]{8}\.tmp$/); + expect(tempPath.slice(file.length)).toMatch(/^\.hf[0-9a-f]{6}\.tmp$/); } +describe("isAtomicTempPath", () => { + it("knows the temp files it names, and no one else's", () => { + expect(isAtomicTempPath(atomicTempPath("/p/index.html"))).toBe(true); + expect(isAtomicTempPath("compositions/intro.html.hf0a1b2c.tmp")).toBe(true); + expect(isAtomicTempPath("foo.12345678.tmp")).toBe(false); + expect(isAtomicTempPath("index.html.hf0a1b2c.tmp.bak")).toBe(false); + }); +}); + describe("replaceFileAtomically", () => { const dirs: string[] = []; @@ -271,21 +286,21 @@ describe("createFileAtomically", () => { it("takes a fresh temporary name when another writer holds one, leaving theirs alone", () => { const dir = tempDir(); const file = join(dir, "index.html"); - writeFileSync(`${file}.deadbeef.tmp`, "theirs"); - nextTempNames("deadbeef"); + writeFileSync(`${file}.hfdeadbe.tmp`, "theirs"); + nextTempNames("deadbe"); createFileAtomically(file, "html"); expect(readFileSync(file, "utf-8")).toBe("html"); - expect(readFileSync(`${file}.deadbeef.tmp`, "utf-8")).toBe("theirs"); + expect(readFileSync(`${file}.hfdeadbe.tmp`, "utf-8")).toBe("theirs"); }); it("gives up after three taken temporary names without touching them", () => { const dir = tempDir(); const file = join(dir, "index.html"); - for (const name of ["00000001", "00000002", "00000003"]) - writeFileSync(`${file}.${name}.tmp`, "theirs"); - nextTempNames("00000001", "00000002", "00000003"); + for (const name of ["000001", "000002", "000003"]) + writeFileSync(`${file}.hf${name}.tmp`, "theirs"); + nextTempNames("000001", "000002", "000003"); expect(() => createFileAtomically(file, "html")).toThrow( expect.objectContaining({ code: "EEXIST" }), @@ -403,21 +418,23 @@ describe.skipIf(process.platform === "win32")("resolveWritePath", () => { ); }); - it("reads a link through a linked folder and .. the way the system does", () => { - const base = linkedFolder(); + function linkCompThroughDotDot(base: string) { fs.mkdirSync(join(base, "real/a/b"), { recursive: true }); symlinkSync(join(base, "real/a/b"), join(base, "root/x")); - writeFileSync(join(base, "real/a/t.html"), "old"); symlinkSync("x/../t.html", join(base, "root/comp.html")); + } + + it("reads a link through a linked folder and .. the way the system does", () => { + const base = linkedFolder(); + linkCompThroughDotDot(base); + writeFileSync(join(base, "real/a/t.html"), "old"); expect(resolveWritePath(join(base, "root/comp.html"))).toBe(join(base, "real/a/t.html")); }); it("follows a dangling link through a linked folder and .. the way the system does", () => { const base = linkedFolder(); - fs.mkdirSync(join(base, "real/a/b"), { recursive: true }); - symlinkSync(join(base, "real/a/b"), join(base, "root/x")); - symlinkSync("x/../t.html", join(base, "root/comp.html")); + linkCompThroughDotDot(base); expect(resolveWritePath(join(base, "root/comp.html"))).toBe(join(base, "real/a/t.html")); }); diff --git a/packages/core/src/atomicFile.ts b/packages/core/src/atomicFile.ts index d33210e51bf..cbfa60fd59f 100644 --- a/packages/core/src/atomicFile.ts +++ b/packages/core/src/atomicFile.ts @@ -13,6 +13,16 @@ 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$/; + +/** A fresh temp sibling for a write that publishes `filePath`. 13 bytes, so a 242-byte name still fits. */ +export function atomicTempPath(filePath: string): string { + return `${filePath}.hf${randomBytes(3).toString("hex")}.tmp`; +} + +export function isAtomicTempPath(path: string): boolean { + return TEMP_SUFFIX.test(path); +} /** Replace a file only after the complete sibling temp file is written. No mode: the default one. */ export function replaceFileAtomically( @@ -100,8 +110,7 @@ function writeTempSibling( operations: SiblingFileSystem, ): string { for (let attempt = 1; ; attempt++) { - // Short, so a name near the filesystem's limit still fits. - const tempPath = `${filePath}.${randomBytes(4).toString("hex")}.tmp`; + const tempPath = atomicTempPath(filePath); try { operations.writeFileSync(tempPath, content, { encoding: "utf-8", mode, flag: "wx" }); return tempPath; diff --git a/packages/studio-server/src/helpers/projectSignature.test.ts b/packages/studio-server/src/helpers/projectSignature.test.ts index 266173ef811..401f66139ff 100644 --- a/packages/studio-server/src/helpers/projectSignature.test.ts +++ b/packages/studio-server/src/helpers/projectSignature.test.ts @@ -7,11 +7,16 @@ import { mkdtempSync, openSync, rmSync, + writeFileSync, writeSync, } from "node:fs"; import { tmpdir } from "node:os"; import { resolve } from "node:path"; -import { affectsProjectSignature, createProjectSignature } from "./projectSignature.js"; +import { + affectsProjectSignature, + createProjectSignature, + listProjectFiles, +} from "./projectSignature.js"; const temporaryProjects: string[] = []; @@ -54,12 +59,32 @@ describe("affectsProjectSignature", () => { expect(affects(".hyperframes/cache/blob.bin")).toBe(false); }); + it("rejects the temp file of a save in flight, but not a user's own .tmp file", () => { + expect(affects("index.html.hf0a1b2c.tmp")).toBe(false); + expect(affects("foo.12345678.tmp")).toBe(true); + }); + it("rejects a path outside the project", () => { expect(affectsProjectSignature(PROJECT, resolve("/projects/other/index.html"))).toBe(false); expect(affectsProjectSignature(PROJECT, PROJECT)).toBe(false); }); }); +describe("listProjectFiles", () => { + it("leaves out the temp file of a save in flight", () => { + const project = mkdtempSync(resolve(tmpdir(), "hf-signature-")); + temporaryProjects.push(project); + writeFileSync(resolve(project, "index.html"), "

Hello

"); + writeFileSync(resolve(project, "index.html.hf0a1b2c.tmp"), "

Bye

"); + writeFileSync(resolve(project, "foo.12345678.tmp"), "mine"); + + expect(listProjectFiles(project).map((file) => file.path)).toEqual([ + "foo.12345678.tmp", + "index.html", + ]); + }); +}); + describe("createProjectSignature", () => { it("changes after same-size content is written with the original mtime restored", () => { const project = mkdtempSync(resolve(tmpdir(), "hf-signature-")); diff --git a/packages/studio-server/src/helpers/projectSignature.ts b/packages/studio-server/src/helpers/projectSignature.ts index 7978a9f86a2..630cb56599d 100644 --- a/packages/studio-server/src/helpers/projectSignature.ts +++ b/packages/studio-server/src/helpers/projectSignature.ts @@ -1,6 +1,7 @@ import { createHash } from "node:crypto"; import { lstatSync, readFileSync, readdirSync } from "node:fs"; import { extname, isAbsolute, relative, resolve, sep } from "node:path"; +import { isAtomicTempPath } from "@hyperframes/core/atomic-file"; import type { ResolvedProject, StudioApiAdapter } from "../types.js"; const SIGNATURE_TEXT_EXTENSIONS = new Set([ @@ -59,7 +60,12 @@ export const STUDIO_SIGNATURE_MANIFEST_PATHS = [ */ export function affectsProjectSignature(projectDir: string, changedPath: string): boolean { const relativePath = relative(resolve(projectDir), resolve(changedPath)); - if (relativePath === "" || relativePath.startsWith("..") || isAbsolute(relativePath)) { + if ( + relativePath === "" || + relativePath.startsWith("..") || + isAbsolute(relativePath) || + isAtomicTempPath(relativePath) + ) { return false; } const segments = relativePath.split(sep); @@ -96,6 +102,9 @@ function isTextContentEligible(file: string, size: number): boolean { ); } +const isSkippedEntry = (entry: string) => + SIGNATURE_EXCLUDED_DIRS.has(entry) || isAtomicTempPath(entry); + function collectProjectSignatureFiles( projectDir: string, dir: string, @@ -109,7 +118,7 @@ function collectProjectSignatureFiles( } for (const entry of entries) { - if (SIGNATURE_EXCLUDED_DIRS.has(entry)) continue; + if (isSkippedEntry(entry)) continue; const file = resolve(dir, entry); if (!isPathWithin(projectDir, file)) continue; let stat: ReturnType; diff --git a/packages/studio-server/src/history/blobStore.ts b/packages/studio-server/src/history/blobStore.ts index db07532efda..a7e923697ef 100644 --- a/packages/studio-server/src/history/blobStore.ts +++ b/packages/studio-server/src/history/blobStore.ts @@ -2,6 +2,7 @@ import { createHash, randomUUID } from "node:crypto"; import { constants, createReadStream, renameSync } from "node:fs"; import { copyFile, mkdir, readdir, readFile, rename, rm, stat } from "node:fs/promises"; import { dirname, join } from "node:path"; +import { atomicTempPath } from "@hyperframes/core/atomic-file"; /** File contents stored once by sha256, text and binary alike. */ export interface BlobStore { @@ -26,7 +27,7 @@ async function hashFile(path: string): Promise { async function cloneOrCopy(from: string, to: string, beforeReplace?: () => void): Promise { await mkdir(dirname(to), { recursive: true }); - const temp = `${to}.${randomUUID()}.tmp`; + const temp = atomicTempPath(to); try { await copyFile(from, temp, constants.COPYFILE_FICLONE); beforeReplace?.(); diff --git a/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs b/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs index 29a43962442..90ef8e611ef 100644 --- a/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs +++ b/packages/studio/tests/e2e/edit-accuracy/ratchet.mjs @@ -56,8 +56,6 @@ export const QUARANTINED = { "sequndo-none-pct-r0-root-z100": "#4853", "sequndo-none-px-r0-nested-z100": "#4853", "sequndo-none-px-r0-root-z100": "#4853", - "seqnudge-none-pct-r0-nested-z100": "#4857", - "seqnudge-none-pct-r0-root-z100": "#4857", "seqrepeat-none-px-r0-nested-z100": "part C (#4807 stack)", };