diff --git a/.github/workflows/windows-render.yml b/.github/workflows/windows-render.yml index db3100e127..aba92da2a3 100644 --- a/.github/workflows/windows-render.yml +++ b/.github/workflows/windows-render.yml @@ -484,10 +484,10 @@ jobs: shell: pwsh run: bunx vitest run packages/producer/src/services/hyperframeLint.file-race.test.ts --maxWorkers=2 - - name: Verify catalog source reads on Windows + - name: Verify catalog file operations on Windows if: matrix.lane == 'studio-engine-cli' shell: pwsh - run: bunx vitest run scripts/catalog/catalog-payload-assets.file-race.test.ts --maxWorkers=2 + run: bunx vitest run scripts/catalog/catalog-payload-assets.file-race.test.ts scripts/catalog/catalog-payload-assets.cache-race.test.ts --maxWorkers=2 - name: Run runtime contract test if: matrix.runtime_contract diff --git a/scripts/catalog-payload-assets.ts b/scripts/catalog-payload-assets.ts index de6462b376..04c019959f 100644 --- a/scripts/catalog-payload-assets.ts +++ b/scripts/catalog-payload-assets.ts @@ -9,8 +9,11 @@ import { createHash } from "node:crypto"; import { - existsSync, mkdirSync, + lstatSync, + linkSync, + mkdtempSync, + rmSync, readdirSync, readFileSync, readSync, @@ -206,6 +209,33 @@ export interface AssetTarget { urlBase: string; } +/** Publish a complete cache entry without replacing a competing file or link. */ +function cacheAsset(bytes: Buffer, ext: string, target: AssetTarget): string { + const name = `${createHash("sha256").update(bytes).digest("hex").slice(0, 16)}${ext}`; + const dest = join(target.dir, name); + // Cache hits need no writable directory. A miss still has to win linkSync. + if (lstatSync(dest, { throwIfNoEntry: false })) return name; + mkdirSync(target.dir, { recursive: true }); + if (!lstatSync(target.dir).isDirectory()) + throw new Error("Catalog cache must be a real directory"); + const staging = mkdtempSync(join(target.dir, ".hf-asset-")); + try { + const staged = join(staging, "content"); + writeFileSync(staged, bytes, { flag: "wx" }); + try { + linkSync(staged, dest); + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "EEXIST")) throw error; + } + } finally { + // Cleanup must not mask a publication error or fail an already published asset. + try { + rmSync(staging, { recursive: true, force: true }); + } catch {} + } + return name; +} + /** * Point every local reference at something the browser can fetch. * @@ -252,12 +282,7 @@ export function processAssets(html: string, projectDir: string, target: AssetTar } if (HOSTED_EXTENSIONS.has(ext)) { - const name = `${createHash("sha256").update(bytes).digest("hex").slice(0, 16)}${ext}`; - const dest = join(target.dir, name); - if (!existsSync(dest)) { - mkdirSync(target.dir, { recursive: true }); - writeFileSync(dest, bytes); - } + const name = cacheAsset(bytes, ext, target); out = out.split(ref).join(`${target.urlBase}/${name}`); hosted += 1; continue; @@ -305,12 +330,7 @@ export function externalizeDataUris( const bytes = Buffer.from(blob, "base64"); if (bytes.length < EXTERNALIZE_MIN_BYTES) return whole; - const name = `${createHash("sha256").update(bytes).digest("hex").slice(0, 16)}${ext}`; - const dest = join(target.dir, name); - if (!existsSync(dest)) { - mkdirSync(target.dir, { recursive: true }); - writeFileSync(dest, bytes); - } + const name = cacheAsset(bytes, ext, target); externalized += 1; return `${target.urlBase}/${name}`; }, diff --git a/scripts/catalog/catalog-payload-assets.cache-race.test.ts b/scripts/catalog/catalog-payload-assets.cache-race.test.ts new file mode 100644 index 0000000000..c461721db8 --- /dev/null +++ b/scripts/catalog/catalog-payload-assets.cache-race.test.ts @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createHash } from "node:crypto"; +import { join, dirname } from "node:path"; +import { tmpdir } from "node:os"; +import { processAssets, externalizeDataUris } from "../catalog-payload-assets.ts"; + +const hooks = vi.hoisted(() => { + const state: { + dest: string; + beforePublish?: () => void; + failWrite: boolean; + failLink: boolean; + failCleanup: boolean; + stagedBytes?: Buffer; + stages: string[]; + } = { dest: "", failWrite: false, failLink: false, failCleanup: false, stages: [] }; + return state; +}); +vi.mock("node:fs", async (importOriginal) => { + const fs = await importOriginal(); + function beforePublish(): void { + const hook = hooks.beforePublish; + hooks.beforePublish = undefined; + hook?.(); + } + return { + ...fs, + mkdtempSync: (...args: Parameters) => { + const dir = fs.mkdtempSync(...args); + hooks.stages.push(String(dir)); + return dir; + }, + writeFileSync: (...args: Parameters) => { + if (String(args[0]) === hooks.dest) beforePublish(); + if (hooks.failWrite) { + fs.writeFileSync(args[0], "partial"); + throw new Error("write failure"); + } + return fs.writeFileSync(...args); + }, + linkSync: (...args: Parameters) => { + hooks.stagedBytes = fs.readFileSync(args[0]); + beforePublish(); + if (hooks.failLink) throw new Error("link failure"); + return fs.linkSync(...args); + }, + rmSync: (...args: Parameters) => { + if (hooks.failCleanup) throw new Error("cleanup failure"); + return fs.rmSync(...args); + }, + }; +}); +const fs = await vi.importActual("node:fs"); +let root: string; +let project: string; +let target: { dir: string; urlBase: string }; +const bytes = Buffer.alloc(4096, 42); +const name = `${createHash("sha256").update(bytes).digest("hex").slice(0, 16)}.png`; +beforeEach(() => { + root = fs.mkdtempSync(join(tmpdir(), "hf-catalog-cache-")); + project = join(root, "project"); + fs.mkdirSync(project); + fs.writeFileSync(join(project, "asset.png"), bytes); + target = { dir: join(root, "cache"), urlBase: "/assets" }; + Object.assign(hooks, { + dest: join(target.dir, name), + beforePublish: undefined, + failWrite: false, + failLink: false, + failCleanup: false, + stagedBytes: undefined, + stages: [], + }); +}); +afterEach(() => fs.rmSync(root, { recursive: true, force: true })); +function publish(kind: string): string { + if (kind === "reference") { + const result = processAssets('', project, target); + expect(result.hosted).toBe(1); + expect(result.inlined).toBe(0); + expect(result.unresolved).toEqual([]); + return result.html; + } + const result = externalizeDataUris( + ``, + target, + ); + expect(result.externalized).toBe(1); + return result.html; +} +function assertNoStages(): void { + for (const stage of hooks.stages) expect(fs.existsSync(stage)).toBe(false); +} + +describe.each(["reference", "data URI"])("catalog %s cache publication", (kind) => { + it("publishes complete bytes with the existing hash and reuses cache hits without staging", () => { + expect(publish(kind)).toBe(``); + expect(hooks.stagedBytes).toEqual(bytes); + expect(fs.readFileSync(hooks.dest)).toEqual(bytes); + assertNoStages(); + hooks.failWrite = true; + const count = hooks.stages.length; + expect(publish(kind)).toBe(``); + expect(hooks.stages.length).toBe(count); + }); + it.each(["file", "symlink", "dangling symlink"])("does not overwrite a competing %s", (entry) => { + const outside = join(root, "outside.png"); + if (entry === "symlink") fs.writeFileSync(outside, "outside"); + hooks.beforePublish = () => { + if (entry === "file") fs.writeFileSync(hooks.dest, "winner"); + else fs.symlinkSync(outside, hooks.dest, "file"); + }; + expect(publish(kind)).toBe(``); + expect(hooks.beforePublish).toBeUndefined(); + if (entry === "file") expect(fs.readFileSync(hooks.dest, "utf8")).toBe("winner"); + else { + expect(fs.lstatSync(hooks.dest).isSymbolicLink()).toBe(true); + if (entry === "symlink") expect(fs.readFileSync(outside, "utf8")).toBe("outside"); + else expect(fs.existsSync(outside)).toBe(false); + } + assertNoStages(); + }); + it("leaves a pre-existing dangling destination link untouched", () => { + fs.mkdirSync(target.dir); + const outside = join(root, "outside.png"); + fs.symlinkSync(outside, hooks.dest, "file"); + publish(kind); + expect(fs.existsSync(outside)).toBe(false); + expect(fs.lstatSync(hooks.dest).isSymbolicLink()).toBe(true); + expect(hooks.stages).toEqual([]); + }); + it("rejects a cache directory linked outside without writing there", () => { + const outside = join(root, "outside"); + fs.mkdirSync(outside); + fs.symlinkSync(outside, target.dir, process.platform === "win32" ? "junction" : "dir"); + expect(() => publish(kind)).toThrow("Catalog cache must be a real directory"); + expect(fs.readdirSync(outside)).toEqual([]); + }); + it("removes a partial staging file after write failure without publishing it", () => { + hooks.failWrite = true; + expect(() => publish(kind)).toThrow("write failure"); + expect(fs.existsSync(hooks.dest)).toBe(false); + assertNoStages(); + }); + it("preserves publication errors and cleans staging", () => { + hooks.failLink = true; + expect(() => publish(kind)).toThrow("link failure"); + expect(fs.existsSync(hooks.dest)).toBe(false); + assertNoStages(); + }); + it("does not replace a publication error with a cleanup error", () => { + hooks.failLink = true; + hooks.failCleanup = true; + expect(() => publish(kind)).toThrow("link failure"); + expect(fs.existsSync(hooks.dest)).toBe(false); + }); + it("keeps a successful publication successful if cleanup fails", () => { + hooks.failCleanup = true; + expect(publish(kind)).toBe(``); + expect(fs.readFileSync(hooks.dest)).toEqual(bytes); + expect(hooks.stages.map((stage) => dirname(stage))).toEqual([target.dir]); + }); +});