From a0e7c43921e138a891bc604c8b109230143608f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 18 Sep 2026 13:06:09 -0400 Subject: [PATCH] fix(catalog): fall back to recorded video when directory is over budget (#4084) * fix(catalog): fall back to recorded video when directory is over budget hostItemDirectory() returned "" for both "not needed" and "over the 2MB host budget", so an item that needed its own hosted directory but exceeded the cap (texture-mask-text: 5.6MB of mask PNGs referenced via runtime JS template strings, invisible to the static asset scanner) silently shipped a payload with no and dead relative image references, 404ing on the docs site. It now returns a tagged result so the caller can fall back to the recorded video on over-budget, same as the existing unresolved-assets fallback. * test(catalog): cover the over-budget skip path in the payload generator * chore(catalog): allowlist the deliberate over-budget payload deletion * chore(catalog): dedupe hosted-directory assertions, note pre-existing complexity Fallow's new-only gate flagged two problems this branch introduced: five near-identical hostItemDirectory assertions in the file-race test collapse into a shared expectHosted() helper, and the over-budget branch added to buildPayload pushed it one branch past the complexity threshold on top of already-high pre-existing debt (CRAP 240 before this branch, unrelated to the fix). Suppressed with the repo's existing fallow-ignore-next-line convention rather than refactoring an unrelated 115-line function inside a bugfix PR. --- .../catalog/components/texture-mask-text.json | 1 - scripts/catalog-payload-assets.test.ts | 15 +++- scripts/catalog-payload-assets.ts | 19 +++++- .../catalog-payload-assets.file-race.test.ts | 26 ++++--- scripts/check-no-main-deletions.mjs | 4 ++ scripts/generate-catalog-payloads.test.ts | 68 +++++++++++++++++++ scripts/generate-catalog-payloads.ts | 25 +++++-- 7 files changed, 137 insertions(+), 21 deletions(-) delete mode 100644 docs/public/catalog/components/texture-mask-text.json create mode 100644 scripts/generate-catalog-payloads.test.ts diff --git a/docs/public/catalog/components/texture-mask-text.json b/docs/public/catalog/components/texture-mask-text.json deleted file mode 100644 index dbd5636b5f..0000000000 --- a/docs/public/catalog/components/texture-mask-text.json +++ /dev/null @@ -1 +0,0 @@ -{"html":"\n\n \n \n \n Texture Mask Text - Demo\n \n \n \n \n
\n
\n
Texture Mask Text
\n
66 ambientCG luminance masks
\n
\n
\n
\n\n \n \n \n \n\n"} \ No newline at end of file diff --git a/scripts/catalog-payload-assets.test.ts b/scripts/catalog-payload-assets.test.ts index beaf6b136d..f30a9be9b3 100644 --- a/scripts/catalog-payload-assets.test.ts +++ b/scripts/catalog-payload-assets.test.ts @@ -7,6 +7,7 @@ import { externalizeDataUris, hostItemDirectory, localReferences, + MAX_HOSTED_DIRECTORY_BYTES, probableReferences, processAssets, withBaseHref, @@ -250,9 +251,9 @@ describe("hostItemDirectory", () => { // time, so the layout has to survive, not just the files. const dir = project({ "compositions/components/lava.png": Buffer.from([1]), "demo.html": "x" }); const out = target(); - const base = hostItemDirectory(dir, out.dir, "/public/catalog/items/x/"); + const result = hostItemDirectory(dir, out.dir, "/public/catalog/items/x/"); - assert.equal(base, "/public/catalog/items/x/"); + assert.deepEqual(result, { status: "hosted", baseHref: "/public/catalog/items/x/" }); assert.ok(existsSync(join(out.dir, "compositions/components/lava.png"))); }); @@ -268,7 +269,15 @@ describe("hostItemDirectory", () => { const dir = project({ "scene.glb": Buffer.from([3]) }); const out = target(); - assert.equal(hostItemDirectory(dir, out.dir, "/base/"), ""); + assert.deepEqual(hostItemDirectory(dir, out.dir, "/base/"), { status: "not-needed" }); + }); + + it("reports over-budget distinctly from not-needed when the directory exceeds the cap", () => { + const dir = project({ "big.png": Buffer.alloc(MAX_HOSTED_DIRECTORY_BYTES + 1) }); + const out = target(); + + assert.deepEqual(hostItemDirectory(dir, out.dir, "/base/"), { status: "over-budget" }); + assert.equal(existsSync(join(out.dir, "big.png")), false); }); }); diff --git a/scripts/catalog-payload-assets.ts b/scripts/catalog-payload-assets.ts index 04c019959f..47fa053e09 100644 --- a/scripts/catalog-payload-assets.ts +++ b/scripts/catalog-payload-assets.ts @@ -387,7 +387,20 @@ function downloadMirrorPrefix(projectDir: string): string | null { */ export const MAX_HOSTED_DIRECTORY_BYTES = 2_000_000; -export function hostItemDirectory(projectDir: string, destDir: string, urlBase: string): string { +// "not-needed" and "over-budget" both leave nothing published, but the caller +// must not treat them alike: an over-budget item still needs the directory, +// so it has to fall back to its recorded video instead of shipping with dead +// relative references and no to resolve them against. +export type HostItemDirectoryResult = + | { readonly status: "not-needed" } + | { readonly status: "hosted"; readonly baseHref: string } + | { readonly status: "over-budget" }; + +export function hostItemDirectory( + projectDir: string, + destDir: string, + urlBase: string, +): HostItemDirectoryResult { const mirrorPrefix = downloadMirrorPrefix(projectDir); const files = new Map>(); let total = 0; @@ -399,7 +412,7 @@ export function hostItemDirectory(projectDir: string, destDir: string, urlBase: ); if (bytes === null) continue; total += bytes.length; - if (total > MAX_HOSTED_DIRECTORY_BYTES) return ""; + if (total > MAX_HOSTED_DIRECTORY_BYTES) return { status: "over-budget" }; files.set(path, bytes); } @@ -418,7 +431,7 @@ export function hostItemDirectory(projectDir: string, destDir: string, urlBase: if (mirrorPrefix !== null && path.startsWith(mirrorPrefix)) publish(path.slice(mirrorPrefix.length), bytes); } - return files.size > 0 ? urlBase : ""; + return files.size > 0 ? { status: "hosted", baseHref: urlBase } : { status: "not-needed" }; } /** diff --git a/scripts/catalog/catalog-payload-assets.file-race.test.ts b/scripts/catalog/catalog-payload-assets.file-race.test.ts index cd5cd4b0e2..eda986408b 100644 --- a/scripts/catalog/catalog-payload-assets.file-race.test.ts +++ b/scripts/catalog/catalog-payload-assets.file-race.test.ts @@ -131,6 +131,12 @@ function file(name: string, bytes = "checked") { fs.writeFileSync(path, bytes); return path; } +function expectHosted() { + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ + status: "hosted", + baseHref: "/item/", + }); +} function arm(path: string) { hooks.target = path; hooks.swap = () => { @@ -162,7 +168,7 @@ describe("catalog source reads", () => { (mirror) => { arm(file(mirror ? "_downloads/font.woff2" : "font.woff2")); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe("/item/"); + expectHosted(); expect(fs.readFileSync(join(out.dir, "font.woff2"), "utf8")).toBe("checked"); expect(hooks.swap).toBeUndefined(); }, @@ -173,7 +179,7 @@ describe("catalog source reads", () => { fs.renameSync(path, join(root, "original")); fs.writeFileSync(path, Buffer.alloc(MAX_HOSTED_DIRECTORY_BYTES + 1)); }; - expect(hostItemDirectory(project, out.dir, "/item/")).toBe(""); + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ status: "over-budget" }); expect(hooks.beforeOpen).toBeUndefined(); expect(fs.existsSync(out.dir)).toBe(false); }); @@ -182,7 +188,7 @@ describe("catalog source reads", () => { const path = file("large.mp4", ""); fs.truncateSync(path, 32 * 1024 * 1024); hooks.forbidUnbounded = true; - expect(hostItemDirectory(project, out.dir, "/item/")).toBe(""); + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ status: "over-budget" }); expect(hooks.bytesRead).toBe(MAX_HOSTED_DIRECTORY_BYTES + 1); expect(fs.existsSync(out.dir)).toBe(false); }); @@ -190,7 +196,7 @@ describe("catalog source reads", () => { const path = file("asset.webm", "x"); hooks.afterStat = () => fs.truncateSync(path, MAX_HOSTED_DIRECTORY_BYTES * 2); hooks.forbidUnbounded = true; - expect(hostItemDirectory(project, out.dir, "/item/")).toBe(""); + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ status: "over-budget" }); expect(hooks.afterStat).toBeUndefined(); expect(hooks.bytesRead).toBe(MAX_HOSTED_DIRECTORY_BYTES + 1); expect(fs.existsSync(out.dir)).toBe(false); @@ -199,7 +205,7 @@ describe("catalog source reads", () => { file("font.woff2"); hooks.forbidUnbounded = true; hooks.maxReadSize = 2; - expect(hostItemDirectory(project, out.dir, "/item/")).toBe("/item/"); + expectHosted(); expect(fs.readFileSync(join(out.dir, "font.woff2"), "utf8")).toBe("checked"); expect(hooks.bytesRead).toBe(7); }); @@ -215,14 +221,14 @@ describe("catalog source reads", () => { file("b.png", "b".repeat(MAX_HOSTED_DIRECTORY_BYTES)); fs.mkdirSync(out.dir); fs.writeFileSync(join(out.dir, "a.png"), "existing"); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe(""); + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ status: "over-budget" }); expect(fs.readdirSync(out.dir)).toEqual(["a.png"]); expect(fs.readFileSync(join(out.dir, "a.png"), "utf8")).toBe("existing"); }); it("reuses budgeted bytes for download mirrors after sources change during publication", () => { const path = file("_downloads/font.woff2", "x".repeat(MAX_HOSTED_DIRECTORY_BYTES)); hooks.beforeWrite = () => fs.writeFileSync(path, "unchecked"); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe("/item/"); + expectHosted(); expect(hooks.beforeWrite).toBeUndefined(); const original = Buffer.alloc(MAX_HOSTED_DIRECTORY_BYTES, "x"); expect(fs.readFileSync(join(out.dir, "_downloads/font.woff2")).equals(original)).toBe(true); @@ -235,7 +241,7 @@ describe("catalog source reads", () => { join(project, "_downloads"), process.platform === "win32" ? "junction" : "dir", ); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe("/item/"); + expectHosted(); expect(fs.readFileSync(join(out.dir, "media/font.woff2"), "utf8")).toBe("checked"); expect(fs.readFileSync(join(out.dir, "font.woff2"), "utf8")).toBe("checked"); expect(fs.existsSync(join(out.dir, "_downloads"))).toBe(false); @@ -244,7 +250,7 @@ describe("catalog source reads", () => { it("preserves download mirror precedence over a colliding root asset", () => { file("font.woff2", "root"); file("_downloads/font.woff2", "download"); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe("/item/"); + expectHosted(); expect(fs.readFileSync(join(out.dir, "font.woff2"), "utf8")).toBe("download"); }); it.each(["png", "glb", "html"])("does not publish an external .%s symlink target", (ext) => { @@ -270,7 +276,7 @@ describe("catalog source reads", () => { join(project, "_downloads"), process.platform === "win32" ? "junction" : "dir", ); - expect(hostItemDirectory(project, out.dir, "/item/")).toBe(""); + expect(hostItemDirectory(project, out.dir, "/item/")).toEqual({ status: "not-needed" }); expect(fs.existsSync(out.dir)).toBe(false); }); it("keeps internal file links and a symlinked project root working", () => { diff --git a/scripts/check-no-main-deletions.mjs b/scripts/check-no-main-deletions.mjs index c4f22f493d..89f4296bb9 100644 --- a/scripts/check-no-main-deletions.mjs +++ b/scripts/check-no-main-deletions.mjs @@ -157,6 +157,10 @@ export const ALLOWED_DELETIONS = new Map([ "packages/studio/src/player/store/groupLevels.ts", "#3454 deliberately removes group level-meter state", ], + [ + "docs/public/catalog/components/texture-mask-text.json", + "the item's directory exceeds the host budget and now falls back to its recorded video; the stale payload had no and dead relative asset references, so removing it is the fix, not the regression", + ], ]); export function parseBase(argv, fallback = "origin/main") { diff --git a/scripts/generate-catalog-payloads.test.ts b/scripts/generate-catalog-payloads.test.ts new file mode 100644 index 0000000000..8dfbd1af37 --- /dev/null +++ b/scripts/generate-catalog-payloads.test.ts @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +vi.mock("./generate-catalog-previews.js", async (importOriginal) => ({ + ...(await importOriginal()), + prepareProjectDir: vi.fn(), +})); +vi.mock("./catalog-payload-assets.ts", async (importOriginal) => ({ + ...(await importOriginal()), + hostItemDirectory: vi.fn(), +})); + +import { prepareProjectDir } from "./generate-catalog-previews.js"; +import { hostItemDirectory } from "./catalog-payload-assets.ts"; +import { buildPayload, payloadRoot } from "./generate-catalog-payloads.ts"; + +const cleanupPaths: string[] = []; +function tmpDir(): string { + const dir = mkdtempSync(join(tmpdir(), "hf-catalog-payload-")); + cleanupPaths.push(dir); + return dir; +} + +afterEach(() => { + vi.clearAllMocks(); + for (const path of cleanupPaths) rmSync(path, { recursive: true, force: true }); + cleanupPaths.length = 0; +}); + +describe("buildPayload", () => { + it("skips and drops a stale payload when the item's directory is over the host budget", async () => { + const sourceDir = tmpDir(); + // Declares a local asset so `needsOwnDirectory` routes through `hostItemDirectory`. + writeFileSync( + join(sourceDir, "registry-item.json"), + JSON.stringify({ files: [{ type: "hyperframes:asset" }] }), + ); + const projectDir = tmpDir(); + writeFileSync(join(projectDir, "index.html"), ""); + vi.mocked(prepareProjectDir).mockResolvedValue(projectDir); + vi.mocked(hostItemDirectory).mockReturnValue({ status: "over-budget" }); + + const item = { + name: "test-over-budget-item", + kind: "component" as const, + sourceDir, + entryFile: "index.html", + }; + const outPath = join(payloadRoot, "components", `${item.name}.json`); + cleanupPaths.push(outPath); + mkdirSync(join(payloadRoot, "components"), { recursive: true }); + writeFileSync(outPath, JSON.stringify({ html: "stale" })); + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const result = await buildPayload(item); + + expect(result).toBe("skipped"); + expect(existsSync(outPath)).toBe(false); + expect( + logSpy.mock.calls.some( + (call) => String(call[0]).includes("host budget") && String(call[0]).includes(item.name), + ), + ).toBe(true); + logSpy.mockRestore(); + }); +}); diff --git a/scripts/generate-catalog-payloads.ts b/scripts/generate-catalog-payloads.ts index d0f0f58ce8..53afb4d810 100644 --- a/scripts/generate-catalog-payloads.ts +++ b/scripts/generate-catalog-payloads.ts @@ -42,13 +42,14 @@ import { inlineMountedComposition, HOSTED_EXTENSIONS, hostItemDirectory, + MAX_HOSTED_DIRECTORY_BYTES, processAssets, withBaseHref, } from "./catalog-payload-assets.ts"; const scriptDir = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(scriptDir, ".."); -const payloadRoot = resolve(repoRoot, "docs/public/catalog"); +export const payloadRoot = resolve(repoRoot, "docs/public/catalog"); /** * Inlining budget for a single payload. A payload is fetched when the reader @@ -178,7 +179,10 @@ function renderEntry( return { entry, fromSnippet: true }; } -async function buildPayload(item: CatalogItem): Promise<"written" | "skipped"> { +// Pre-existing complexity from the item's independent skip conditions; the +// over-budget branch added here is one more of the same shape, not a new debt. +// fallow-ignore-next-line complexity +export async function buildPayload(item: CatalogItem): Promise<"written" | "skipped"> { const outPath = join(payloadRoot, typeDir(item.kind), `${item.name}.json`); // An item that stops qualifying has to lose its payload, or the page @@ -239,10 +243,23 @@ async function buildPayload(item: CatalogItem): Promise<"written" | "skipped"> { // duplicate assets already shared by hash and roughly double what the // repository carries, to fix a handful of compositions. const itemUrl = `/public/catalog/items/${item.name}`; - const baseHref = + const hostResult = interactive || needsOwnDirectory(item, unresolved) ? hostItemDirectory(projectDir, join(payloadRoot, "items", item.name), `${itemUrl}/`) - : ""; + : { status: "not-needed" as const }; + + // A directory that is needed but over budget still leaves the composition + // with dead relative references and no to resolve them against — + // that is worse than the recorded video it already has. + if (hostResult.status === "over-budget") { + console.log( + ` – ${item.name}: directory over the ${(MAX_HOSTED_DIRECTORY_BYTES / 1e6).toFixed(1)} MB host budget, keeping the recorded video`, + ); + dropStalePayload(); + return "skipped"; + } + const baseHref = hostResult.status === "hosted" ? hostResult.baseHref : ""; + // An interactive preview keeps its mount, so the component travels inline // and the demo's own pinned values come off — the reader's choices are what // should reach it.