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 <base href> 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.
This commit is contained in:
Miguel Ángel
2026-09-18 13:06:09 -04:00
committed by GitHub
parent cc36905c29
commit a0e7c43921
7 changed files with 137 additions and 21 deletions
File diff suppressed because one or more lines are too long
+12 -3
View File
@@ -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);
});
});
+16 -3
View File
@@ -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 <base> 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<string, Buffer<ArrayBuffer>>();
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" };
}
/**
@@ -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", () => {
+4
View File
@@ -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 <base href> and dead relative asset references, so removing it is the fix, not the regression",
],
]);
export function parseBase(argv, fallback = "origin/main") {
+68
View File
@@ -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<typeof import("./generate-catalog-previews.js")>()),
prepareProjectDir: vi.fn(),
}));
vi.mock("./catalog-payload-assets.ts", async (importOriginal) => ({
...(await importOriginal<typeof import("./catalog-payload-assets.ts")>()),
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"), "<html></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();
});
});
+21 -4
View File
@@ -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 <base> 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.