diff --git a/packages/cli/src/capture/assetDownloader.test.ts b/packages/cli/src/capture/assetDownloader.test.ts index 9209e4a685..fa9da1c65d 100644 --- a/packages/cli/src/capture/assetDownloader.test.ts +++ b/packages/cli/src/capture/assetDownloader.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { isPrivateUrl, safeFetch } from "./assetDownloader.js"; +import { isPrivateUrl, safeFetch, toStandaloneSvg } from "./assetDownloader.js"; describe("isPrivateUrl — SSRF denylist (security: F-003)", () => { it("blocks loopback, private, and metadata IPv4", () => { @@ -91,3 +91,39 @@ describe("safeFetch — re-validates the denylist on every redirect hop (securit expect(fetchMock).not.toHaveBeenCalled(); }); }); + +describe("toStandaloneSvg — scraped inline SVGs must survive as .svg files", () => { + it("adds the SVG namespace that outerHTML omits for inline SVG", () => { + const inline = ''; + const out = toStandaloneSvg(inline); + expect(out).toContain('xmlns="http://www.w3.org/2000/svg"'); + // Nothing else may change — the path geometry is the brand mark. + expect(out).toContain(''); + expect(out.endsWith("")).toBe(true); + }); + + it("leaves an SVG that already declares xmlns untouched", () => { + const already = ''; + expect(toStandaloneSvg(already)).toBe(already); + }); + + it("declares xmlns:xlink only when an xlink: attribute is actually used", () => { + const withXlink = ''; + expect(toStandaloneSvg(withXlink)).toContain('xmlns:xlink="http://www.w3.org/1999/xlink"'); + const without = ''; + expect(toStandaloneSvg(without)).not.toContain("xmlns:xlink"); + }); + + it("is idempotent and preserves attributes on the root", () => { + const inline = ''; + const once = toStandaloneSvg(inline); + expect(toStandaloneSvg(once)).toBe(once); + for (const attr of ['class="logo"', 'width="120"', 'height="24"', 'fill="currentColor"']) { + expect(once).toContain(attr); + } + }); + + it("returns non-SVG input unchanged rather than corrupting it", () => { + expect(toStandaloneSvg("
not an svg
")).toBe("
not an svg
"); + }); +}); diff --git a/packages/cli/src/capture/assetDownloader.ts b/packages/cli/src/capture/assetDownloader.ts index a3f13bb378..67b2c0f750 100644 --- a/packages/cli/src/capture/assetDownloader.ts +++ b/packages/cli/src/capture/assetDownloader.ts @@ -17,6 +17,32 @@ function svgContentHashSlug(svgSource: string | Buffer, isLogo: boolean): string return isLogo ? `logo-${hash}` : `svg-${hash}`; } +/** + * Make a scraped inline `` usable as a standalone `.svg` file. + * + * An inline SVG in an HTML document inherits the SVG namespace from the parser, so the DOM's + * `outerHTML` does not serialize `xmlns`. That string is fine pasted back into HTML but is NOT + * a valid standalone document: `` renders a broken-image icon, which + * is how these assets are actually consumed downstream. Declare the namespaces on the way to disk. + * + * `xlink:href` is deprecated but still emitted by plenty of sites; an undeclared `xlink:` prefix + * is a parse error in a standalone document, so declare that too — but only when it is used. + */ +export function toStandaloneSvg(outerHTML: string): string { + const open = outerHTML.match(/]*>/i); + if (!open) return outerHTML; + const original = open[0]; + let tag = original; + const add: string[] = []; + if (!/\sxmlns\s*=/i.test(tag)) add.push('xmlns="http://www.w3.org/2000/svg"'); + if (/\sxlink:[a-z-]+\s*=/i.test(outerHTML) && !/\sxmlns:xlink\s*=/i.test(tag)) { + add.push('xmlns:xlink="http://www.w3.org/1999/xlink"'); + } + if (!add.length) return outerHTML; + tag = tag.replace(/^ = {}, +) { + const evaluate = vi.fn(async (script?: unknown) => + String(script).includes("scrollHeight") ? docHeight : undefined, + ); + const screenshot = vi.fn(async (_opts?: unknown) => pngBuffer(plateHeight)); + return { page: { evaluate, screenshot, ...overrides } as unknown as Page, evaluate, screenshot }; +} + +describe("captureFullPagePlate — the scroll shot's plate", () => { + it("writes one full-page png and returns its relative path", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + const { page, screenshot } = fakePage({ docHeight: 10962, plateHeight: 10962 }); + + const out = await captureFullPagePlate(page, dir); + + expect(out).toBe("screenshots/full-page.png"); + expect(screenshot).toHaveBeenCalledWith({ type: "png", fullPage: true }); + expect(pngHeight(readFileSync(join(dir, "full-page.png")))).toBe(10962); + }); + + it("stays 1x: it never touches the viewport's deviceScaleFactor", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + const setViewport = vi.fn(async () => undefined); + const { page } = fakePage({}, { setViewport }); + + await captureFullPagePlate(page, dir); + + // A 2x plate would exceed the cap on exactly the long pages that want a scroll shot. + expect(setViewport).not.toHaveBeenCalled(); + }); + + it("skips a page taller than Chrome can capture, instead of writing a clipped plate", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + const { page, screenshot } = fakePage({ docHeight: MAX_PLATE_HEIGHT_PX + 1 }); + + const out = await captureFullPagePlate(page, dir); + + expect(out).toBeNull(); + expect(screenshot).not.toHaveBeenCalled(); + expect(existsSync(join(dir, "full-page.png"))).toBe(false); + }); + + it("neutralises sticky/fixed chrome for the shot and restores it afterwards", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + const { page, evaluate, screenshot } = fakePage(); + + await captureFullPagePlate(page, dir); + + const scripts = evaluate.mock.calls.map((c) => String(c[0])); + // neutralise, height probe, restore — the probe sits after neutralisation because + // forcing fixed/sticky to `static` puts those elements back in flow and grows the page. + expect(scripts).toHaveLength(3); + expect(scripts[0]).toContain("'fixed'"); + expect(scripts[0]).toContain("'sticky'"); + expect(scripts[0]).toContain("data-hf-plate-position"); + expect(scripts[1]).toContain("scrollHeight"); + // Then hand the page back unchanged: the caller keeps reading the DOM after this. + expect(scripts[2]).toContain("removeAttribute"); + expect(scripts[2]).toContain("data-hf-plate-position"); + expect(evaluate.mock.invocationCallOrder[1]).toBeLessThan( + screenshot.mock.invocationCallOrder[0]!, + ); + expect(screenshot.mock.invocationCallOrder[0]).toBeLessThan( + evaluate.mock.invocationCallOrder[2]!, + ); + }); + + it("restores the page even when the screenshot throws", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + const screenshot = vi.fn(async (_opts?: unknown) => { + throw new Error("capture failed"); + }); + const { page, evaluate } = fakePage({}, { screenshot }); + + await expect(captureFullPagePlate(page, dir)).rejects.toThrow("capture failed"); + // A page left with every sticky element forced static would corrupt the extraction + // passes that run after this one. + expect(String(evaluate.mock.calls.at(-1)?.[0])).toContain("removeAttribute"); + }); +}); + +describe("captureFullPagePlate — guards against a silently clipped plate", () => { + it("measures the height itself, after lazy content has grown the page", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + // A page that measured 9000 before scrolling but is 20000 once lazy images land: the + // pre-scroll number would have passed the guard and emitted a clipped plate. + const { page, screenshot } = fakePage({ docHeight: 20000 }); + + expect(await captureFullPagePlate(page, dir)).toBeNull(); + expect(screenshot).not.toHaveBeenCalled(); + }); + + it("discards a plate Chrome clipped, even when the measurement passed", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + // Measurement said 16000, but the capture itself triggered more loading and came back + // over the cap. Emitting it would be undetectable downstream. + const { page } = fakePage({ docHeight: 16000, plateHeight: MAX_PLATE_HEIGHT_PX + 500 }); + + expect(await captureFullPagePlate(page, dir)).toBeNull(); + expect(existsSync(join(dir, "full-page.png"))).toBe(false); + }); + + it("survives a restore that throws — the real error is what propagates", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + let call = 0; + const evaluate = vi.fn(async (script?: unknown) => { + call++; + if (String(script).includes("scrollHeight")) return 8000; + if (String(script).includes("removeAttribute")) throw new Error("page crashed"); + return undefined; + }); + const screenshot = vi.fn(async (_opts?: unknown) => { + throw new Error("capture failed"); + }); + const page = { evaluate, screenshot } as unknown as Page; + + // Without the try/catch in `finally`, "page crashed" would mask "capture failed". + await expect(captureFullPagePlate(page, dir)).rejects.toThrow("capture failed"); + expect(call).toBeGreaterThanOrEqual(3); + }); +}); + +describe("pngHeight", () => { + it("reads the height out of the IHDR chunk", () => { + expect(pngHeight(pngBuffer(10962))).toBe(10962); + }); + + it("returns null for anything that is not a PNG", () => { + expect(pngHeight(Buffer.from("not a png at all, definitely not"))).toBeNull(); + expect(pngHeight(Buffer.alloc(4))).toBeNull(); + }); +}); + +describe("captureFullPagePlate — the guard sees the post-neutralisation page (Magi's case)", () => { + it("skips when the initial height is under the cap but the final height is over it", async () => { + const dir = mkdtempSync(join(tmpdir(), "hf-plate-")); + // Pre-traversal the page measured 9000. Lazy content and un-fixing the sticky header push + // it over the cap by the time the plate would be shot. Probing before either step would + // have passed the guard and emitted a clipped plate. + let neutralised = false; + const evaluate = vi.fn(async (script?: unknown) => { + const src = String(script); + if (src.includes("'sticky'")) { + neutralised = true; + return undefined; + } + if (src.includes("scrollHeight")) return neutralised ? 20000 : 9000; + return undefined; + }); + const screenshot = vi.fn(async (_opts?: unknown) => pngBuffer(20000)); + const page = { evaluate, screenshot } as unknown as Page; + + expect(await captureFullPagePlate(page, dir)).toBeNull(); + expect(screenshot).not.toHaveBeenCalled(); + expect(existsSync(join(dir, "full-page.png"))).toBe(false); + // Bailing out early must still hand the page back unmodified. + expect(String(evaluate.mock.calls.at(-1)?.[0])).toContain("removeAttribute"); + }); +}); diff --git a/packages/cli/src/capture/screenshotCapture.ts b/packages/cli/src/capture/screenshotCapture.ts index 05c294991e..41b09b8c52 100644 --- a/packages/cli/src/capture/screenshotCapture.ts +++ b/packages/cli/src/capture/screenshotCapture.ts @@ -21,6 +21,92 @@ import { join } from "node:path"; * elements — screenshots show the page in its natural browsing state with * scroll-triggered animations fired. */ +/** + * Chrome caps a screenshot at 16384px per side (Skia's max texture dimension); past that the + * capture comes back clipped or fails outright. Long marketing pages do reach this. + */ +export const MAX_PLATE_HEIGHT_PX = 16384; + +/** + * Pixel height Chrome actually produced, read from the PNG's IHDR chunk: 8-byte signature, + * then 4 length + 4 type + 4 width + 4 height. Null when the buffer isn't a PNG. + */ +export function pngHeight(buf: Uint8Array): number | null { + // Byte math rather than Buffer helpers: page.screenshot() resolves to a Uint8Array. + if (buf.length < 24) return null; + if (buf[12] !== 0x49 || buf[13] !== 0x48 || buf[14] !== 0x44 || buf[15] !== 0x52) return null; // "IHDR" + return ((buf[20]! << 24) | (buf[21]! << 16) | (buf[22]! << 8) | buf[23]!) >>> 0; +} + +/** + * One tall image of the whole document — the plate a scroll shot slides its viewport over. + * + * This is deliberately 1x. A 2x plate is what you'd want for pushing in without softening + * text, but doubling a long marketing page blows past `MAX_PLATE_HEIGHT_PX` exactly on the + * pages that most want a scroll shot; a frame that needs 2x should capture its own region + * instead. At 1x a 1920-wide plate is pixel-exact for a 1920x1080 viewport travelling down it. + * + * Two things have to be true for the plate to be usable, and both are why the earlier + * `full-page.png` was worth removing rather than keeping as-is: + * · It must be taken AFTER the scroll traversal, so lazy images have loaded and + * scroll-triggered reveals have fired. A plate shot on arrival is full of blank bands. + * · Sticky/fixed chrome has to be neutralised first. `fullPage` bakes a fixed header in at + * one position, so a nav ends up frozen across the middle of the plate. The viewport + * shots keep sticky on purpose (natural browsing state); the plate cannot. + * + * Returns the relative path, or null when the page is too tall to capture in one piece. + */ +export async function captureFullPagePlate( + page: Page, + screenshotsDir: string, +): Promise { + // Record the inline value before overwriting so the page is handed back unchanged — the + // caller keeps using it (asset extraction, DOM reads) after this returns. + await page.evaluate( + `document.querySelectorAll('*').forEach((el) => { + const p = getComputedStyle(el).position; + if (p === 'fixed' || p === 'sticky') { + el.setAttribute('data-hf-plate-position', el.style.position || ''); + el.style.position = 'static'; + } + })`, + ); + try { + // Measured here — after the caller's scroll traversal AND after neutralisation — never + // taken from the caller. Both steps grow the document: lazy content loads as the page is + // scrolled, and forcing fixed/sticky elements to `static` drops them back into flow. A + // height read before either one is low on exactly the long pages this guard exists for, + // which would pass the check and let a clipped plate through. + const docHeight = (await page.evaluate( + `Math.max(document.body.scrollHeight, document.documentElement.scrollHeight)`, + )) as number; + if (docHeight > MAX_PLATE_HEIGHT_PX) return null; + + const buffer = await page.screenshot({ type: "png", fullPage: true }); + // Confirm what Chrome produced instead of trusting the measurement: the capture itself can + // trigger another round of lazy loading. A clipped plate is undetectable downstream — the + // skill only teaches the tile fallback when the file is *absent* — so emit nothing rather + // than something silently wrong. + const produced = pngHeight(buffer); + if (produced != null && produced > MAX_PLATE_HEIGHT_PX) return null; + writeFileSync(join(screenshotsDir, "full-page.png"), buffer); + return "screenshots/full-page.png"; + } finally { + // A page that broke mid-capture will fail this too; letting that escape would replace the + // real error with a cleanup one. Nothing to restore if the page is already gone. + try { + await page.evaluate( + `document.querySelectorAll('[data-hf-plate-position]').forEach((el) => { + el.style.position = el.getAttribute('data-hf-plate-position'); + el.removeAttribute('data-hf-plate-position'); + })`, + ); + } catch { + /* page unusable — the restore is moot */ + } + } +} + export async function captureScrollScreenshots(page: Page, outputDir: string): Promise { const screenshotsDir = join(outputDir, "screenshots"); mkdirSync(screenshotsDir, { recursive: true }); @@ -151,7 +237,13 @@ export async function captureScrollScreenshots(page: Page, outputDir: string): P await page.evaluate(`window.scrollTo(0, 0)`); await new Promise((r) => setTimeout(r, 200)); - // full-page.png removed — 1/8 agents read it, contact sheet covers the same content + // The scroll plate, last: everything above has loaded the page and fired its reveals, which + // is the only state a full-page shot is worth taking in. (An earlier full-page.png was + // dropped because 1/8 agents read it and the contact sheet covered the same ground — that + // was about it as a *comprehension* artifact. The scroll shot is a different consumer: it + // needs one continuous plate, which no set of viewport tiles can substitute for.) + const plate = await captureFullPagePlate(page, screenshotsDir); + if (plate) filePaths.push(plate); } catch { /* scroll screenshots are non-critical */ } diff --git a/skills-manifest.json b/skills-manifest.json index 10bf33ab9c..08d4a2bca2 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -6,8 +6,8 @@ "files": 140 }, "faceless-explainer": { - "hash": "b772a9b6c8118c2c", - "files": 22 + "hash": "72acdcb31403531d", + "files": 23 }, "figma": { "hash": "517e4dc53c13ea05", @@ -58,12 +58,12 @@ "files": 132 }, "pr-to-video": { - "hash": "44a9877e7ea1289e", - "files": 29 + "hash": "922c2714750d5a54", + "files": 30 }, "product-launch-video": { - "hash": "1a14737e16f6a154", - "files": 26 + "hash": "a0f6b1f4c8131ed2", + "files": 27 }, "remotion-to-hyperframes": { "hash": "3a0e6c2affb9f74e", diff --git a/skills/faceless-explainer/scripts/assemble-index.mjs b/skills/faceless-explainer/scripts/assemble-index.mjs index 8a7554500a..e772ddbc45 100644 --- a/skills/faceless-explainer/scripts/assemble-index.mjs +++ b/skills/faceless-explainer/scripts/assemble-index.mjs @@ -63,6 +63,9 @@ const flag = (name, def) => { const i = argv.indexOf(`--${name}`); return i >= 0 && i + 1 < argv.length ? argv[i + 1] : def; }; +// Deliberate escape from the bgm_pending refusal below — for previewing while a detached +// generate is still running. Off by default so a silent film can't ship by accident. +const allowPendingBgm = argv.includes("--allow-pending-bgm"); function die(msg) { console.error(`✗ assemble-index.mjs: ${msg}`); process.exit(1); @@ -349,7 +352,14 @@ let audio = { bgm: null, voices: [], sfx: [] }; if (existsSync(audioMetaPath)) { try { const parsed = JSON.parse(readFileSync(audioMetaPath, "utf8")); - audio = { bgm: parsed.bgm ?? null, voices: parsed.voices ?? [], sfx: parsed.sfx ?? [] }; + // bgm_pending rides along: without it this step cannot tell a detached generate that has + // not landed yet from a film that is silent by design, and it would build the silent one. + audio = { + bgm: parsed.bgm ?? null, + bgm_pending: !!parsed.bgm_pending, + voices: parsed.voices ?? [], + sfx: parsed.sfx ?? [], + }; } catch (e) { die(`audio_meta.json parse: ${e.message}`); } @@ -431,6 +441,20 @@ if (audio.bgm?.path) { } else { anomalies.push(`bgm ${audio.bgm.path} not on disk — skipped`); } +} else if (audio.bgm_pending) { + // The distinction the flag exists to make. A warning is not enough: assemble is re-run on + // rework long after the audio step's own warning scrolled past, and it would happily build a + // silent film from a snapshot whose JSON says the bed is still generating. + if (!allowPendingBgm) { + die( + "audio_meta.json says bgm_pending — the music bed is still generating and is NOT in this " + + "assembly. Wait for the track, re-run the audio step, then assemble again. To assemble a " + + "deliberately silent preview anyway, pass --allow-pending-bgm.", + ); + } + anomalies.push( + "bgm still generating (bgm_pending) — assembled without a bed per --allow-pending-bgm", + ); } // (track 2) captions — captions.mjs writes this or legally skips; key off existence. diff --git a/skills/faceless-explainer/scripts/assemble-index.test.mjs b/skills/faceless-explainer/scripts/assemble-index.test.mjs new file mode 100644 index 0000000000..bd60c87115 --- /dev/null +++ b/skills/faceless-explainer/scripts/assemble-index.test.mjs @@ -0,0 +1,72 @@ +import assert from "node:assert/strict"; +import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; +import test from "node:test"; + +const assembleScript = new URL("./assemble-index.mjs", import.meta.url).pathname; + +// ── bgm_pending at the assembly boundary ───────────────────────────────────── +// Regression: the flag survived into audio_meta.json but assemble rebuilt its audio object +// from three named keys and dropped it, so the step that actually builds the film could not +// tell "not ready yet" from "silent by design" and would ship the silent one. + +function assembleWith({ audioMeta, extraArgs = [] }) { + const dir = mkdtempSync(join(tmpdir(), "product-launch-assemble-")); + writeFileSync( + join(dir, "STORYBOARD.md"), + "---\nformat: 1920x1080\nmessage: T\n---\n\n## Frame 1 — A\n- duration: 3s\n- src: compositions/frames/01-a.html\n", + ); + mkdirSync(join(dir, "compositions", "frames"), { recursive: true }); + writeFileSync( + join(dir, "compositions", "frames", "01-a.html"), + '
' + + '
', + ); + if (audioMeta) writeFileSync(join(dir, "audio_meta.json"), JSON.stringify(audioMeta)); + const r = spawnSync( + process.execPath, + [ + assembleScript, + "--storyboard", + join(dir, "STORYBOARD.md"), + "--hyperframes", + dir, + ...extraArgs, + ], + { encoding: "utf8" }, + ); + return { dir, r }; +} + +test("assemble REFUSES while bgm_pending and no bed on disk", () => { + const { dir, r } = assembleWith({ + audioMeta: { bgm: null, bgm_pending: true, voices: [], sfx: [] }, + }); + + assert.notEqual(r.status, 0, "should not assemble a silent film over a pending bed"); + assert.match(r.stderr, /bgm_pending/); + // Refusing means producing nothing, not a half-built index. + assert.equal(existsSync(join(dir, "index.html")), false); +}); + +test("--allow-pending-bgm assembles anyway, and says so", () => { + const { dir, r } = assembleWith({ + audioMeta: { bgm: null, bgm_pending: true, voices: [], sfx: [] }, + extraArgs: ["--allow-pending-bgm"], + }); + + assert.equal(r.status, 0, r.stderr); + assert.equal(existsSync(join(dir, "index.html")), true); + assert.match(r.stdout + r.stderr, /pending/i); +}); + +test("a film that is silent BY DESIGN still assembles untouched", () => { + // The whole point of carrying the flag: this case must stay distinguishable from the above. + const { dir, r } = assembleWith({ audioMeta: { bgm: null, voices: [], sfx: [] } }); + + assert.equal(r.status, 0, r.stderr); + assert.equal(existsSync(join(dir, "index.html")), true); + assert.doesNotMatch(r.stderr, /bgm_pending/); +}); diff --git a/skills/faceless-explainer/scripts/audio.mjs b/skills/faceless-explainer/scripts/audio.mjs index 3bae9e2430..84a878a97a 100644 --- a/skills/faceless-explainer/scripts/audio.mjs +++ b/skills/faceless-explainer/scripts/audio.mjs @@ -105,6 +105,11 @@ function toProductLaunchMeta(neutral) { duration_s: neutral.bgm.duration_s ?? null, } : null; + // bgm_pending must survive the neutral → skill translation. A detached generate + // (Lyria/MusicGen) leaves `bgm: null, bgm_pending: true` until the track lands; dropping the + // flag makes "not ready yet" indistinguishable from "silent by design", so a later + // `fetch-sfx` snapshot turns a still-generating bed into no music at all with no signal. + const bgmPending = !!neutral.bgm_pending; const sfx = (neutral.sfx ?? []).map((s) => ({ frame: Number(s.id), file: s.file, @@ -112,7 +117,7 @@ function toProductLaunchMeta(neutral) { duration_s: s.duration_s ?? 1, volume: s.volume ?? 0.35, })); - return { bgm, voices, sfx }; + return { bgm, bgm_pending: bgmPending, voices, sfx }; } // ── generate (TTS + BGM) ──────────────────────────────────────────────────── @@ -193,12 +198,16 @@ function runFetchSfx(argv) { const manifest = parseStoryboard(readFileSync(storyboardPath, "utf8")); // Per-frame `sfx:` cues (comma-separated) → engine lines carrying only sfx. + // `filter(Boolean)` alone is not enough: a storyboard that spells "no SFX here" as + // `sfx: none` reaches the engine as a cue literally NAMED "none", which then fails to + // resolve. The absence sentinels are part of the storyboard vocabulary, so drop them. + const SFX_NONE = new Set(["none", "no", "n/a", "na", "skip", "-", "—", "–"]); const lines = []; for (const f of manifest.frames) { const names = (f.extra?.sfx ?? "") .split(",") .map((s) => s.trim()) - .filter(Boolean); + .filter((s) => s && !SFX_NONE.has(s.toLowerCase())); if (names.length && f.number != null) lines.push({ id: pad2(f.number), sfx: names }); } @@ -212,6 +221,15 @@ function runFetchSfx(argv) { const meta = toProductLaunchMeta(JSON.parse(readFileSync(neutral, "utf8"))); writeFileSync(outPath, JSON.stringify(meta, null, 2)); console.log(`✓ audio fetch-sfx: ${meta.sfx.length} SFX cue(s) → ${outPath}`); + // This pass rewrites audio_meta.json from the neutral sidecar. If a detached BGM generate is + // still running, the bed it eventually writes is NOT folded back in — the snapshot we just + // took has no music. Say so instead of leaving a silent film behind. + if (meta.bgm_pending && !meta.bgm) { + console.warn( + "⚠ audio fetch-sfx: a detached BGM generate is still pending, so this snapshot has no bed. " + + "Re-run `fetch-sfx` (or re-point audio_meta.json at the track) once it lands, before assembling.", + ); + } } // ── sync-durations (local; rewrites STORYBOARD.md) ──────────────────────────── diff --git a/skills/faceless-explainer/scripts/audio.test.mjs b/skills/faceless-explainer/scripts/audio.test.mjs index a7731f108b..cdad04f621 100644 --- a/skills/faceless-explainer/scripts/audio.test.mjs +++ b/skills/faceless-explainer/scripts/audio.test.mjs @@ -89,3 +89,88 @@ test("a storyboard music mood still retrieves BGM (marker is exact, not fuzzy)", assert.equal(request.bgm.mode, "retrieve"); assert.equal(request.bgm.query, "upbeat synthwave with heavy drums"); }); + +// ── fetch-sfx ──────────────────────────────────────────────────────────────── +// Regressions found while running the full product-launch workflow end to end +// (linear.app site showcase, 2026-07-30). + +/** Runs the fetch-sfx subcommand. `neutralOut` is what the stub engine writes to --out. */ +function runFetchSfx({ storyboard, neutralOut = { voices: [], bgm: null, sfx: [] } }) { + const dir = mkdtempSync(join(tmpdir(), "product-launch-sfx-")); + const engine = join(dir, "engine.mjs"); + writeFileSync(join(dir, "STORYBOARD.md"), storyboard); + writeFileSync( + engine, + `import { readFileSync, writeFileSync } from "node:fs"; +const argv = process.argv.slice(2); +const flag = (name) => argv[argv.indexOf(name) + 1]; +const request = JSON.parse(readFileSync(flag("--request"), "utf8")); +writeFileSync(new URL("request.json", import.meta.url), JSON.stringify(request)); +writeFileSync(flag("--out"), ${JSON.stringify(JSON.stringify(neutralOut))}); +`, + ); + const result = spawnSync( + process.execPath, + [script, "fetch-sfx", "--hyperframes", dir, "--storyboard", join(dir, "STORYBOARD.md")], + { encoding: "utf8", env: { ...process.env, HF_MEDIA_ENGINE: engine } }, + ); + return { dir, result }; +} + +const FRAME_WITH_SFX = (sfx) => + `---\nmessage: Test\n---\n\n## Frame 1 — Hook\n- duration: 3s\n- sfx: ${sfx}\n`; + +test("fetch-sfx: `sfx: none` is an absence marker, not a cue named none", () => { + const { dir, result } = runFetchSfx({ storyboard: FRAME_WITH_SFX("none") }); + + assert.equal(result.status, 0, result.stderr); + const request = JSON.parse(readFileSync(join(dir, "request.json"), "utf8")); + // Used to reach the engine as { sfx: ["none"] } — a cue that cannot resolve. + assert.deepEqual(request.lines, []); +}); + +test("fetch-sfx: the other absence spellings are markers too", () => { + for (const spelling of ["None", "n/a", "NA", "skip", "-", "—"]) { + const { dir, result } = runFetchSfx({ storyboard: FRAME_WITH_SFX(spelling) }); + assert.equal(result.status, 0, result.stderr); + const request = JSON.parse(readFileSync(join(dir, "request.json"), "utf8")); + assert.deepEqual(request.lines, [], `spelling: ${spelling}`); + } +}); + +test("fetch-sfx: a real cue still reaches the engine, and mixed lists drop only the marker", () => { + const { dir, result } = runFetchSfx({ storyboard: FRAME_WITH_SFX("whoosh, none, click") }); + + assert.equal(result.status, 0, result.stderr); + const request = JSON.parse(readFileSync(join(dir, "request.json"), "utf8")); + assert.deepEqual(request.lines, [{ id: "01", sfx: ["whoosh", "click"] }]); +}); + +test("fetch-sfx: carries bgm_pending through and warns that the snapshot has no bed", () => { + const { dir, result } = runFetchSfx({ + storyboard: FRAME_WITH_SFX("whoosh"), + // A detached Lyria/MusicGen generate that has not landed yet. + neutralOut: { voices: [], bgm: null, bgm_pending: true, sfx: [] }, + }); + + assert.equal(result.status, 0, result.stderr); + const meta = JSON.parse(readFileSync(join(dir, "audio_meta.json"), "utf8")); + // The flag used to be dropped in the neutral → PL translation, making "not ready yet" + // indistinguishable from "silent by design". + assert.equal(meta.bgm_pending, true); + assert.equal(meta.bgm, null); + assert.match(result.stderr + result.stdout, /pending/i); +}); + +test("fetch-sfx: a resolved bed reports bgm_pending false and no warning", () => { + const { dir, result } = runFetchSfx({ + storyboard: FRAME_WITH_SFX("whoosh"), + neutralOut: { voices: [], bgm: { path: "assets/bgm/track.mp3", volume: 0.12 }, sfx: [] }, + }); + + assert.equal(result.status, 0, result.stderr); + const meta = JSON.parse(readFileSync(join(dir, "audio_meta.json"), "utf8")); + assert.equal(meta.bgm_pending, false); + assert.equal(meta.bgm.path, "assets/bgm/track.mp3"); + assert.doesNotMatch(result.stderr, /pending/i); +}); diff --git a/skills/pr-to-video/scripts/assemble-index.mjs b/skills/pr-to-video/scripts/assemble-index.mjs index adb1318f21..e1c200ef6a 100644 --- a/skills/pr-to-video/scripts/assemble-index.mjs +++ b/skills/pr-to-video/scripts/assemble-index.mjs @@ -64,6 +64,9 @@ const flag = (name, def) => { const i = argv.indexOf(`--${name}`); return i >= 0 && i + 1 < argv.length ? argv[i + 1] : def; }; +// Deliberate escape from the bgm_pending refusal below — for previewing while a detached +// generate is still running. Off by default so a silent film can't ship by accident. +const allowPendingBgm = argv.includes("--allow-pending-bgm"); function die(msg) { console.error(`✗ assemble-index.mjs: ${msg}`); process.exit(1); @@ -355,7 +358,14 @@ let audio = { bgm: null, voices: [], sfx: [] }; if (existsSync(audioMetaPath)) { try { const parsed = JSON.parse(readFileSync(audioMetaPath, "utf8")); - audio = { bgm: parsed.bgm ?? null, voices: parsed.voices ?? [], sfx: parsed.sfx ?? [] }; + // bgm_pending rides along: without it this step cannot tell a detached generate that has + // not landed yet from a film that is silent by design, and it would build the silent one. + audio = { + bgm: parsed.bgm ?? null, + bgm_pending: !!parsed.bgm_pending, + voices: parsed.voices ?? [], + sfx: parsed.sfx ?? [], + }; } catch (e) { die(`audio_meta.json parse: ${e.message}`); } @@ -437,6 +447,20 @@ if (audio.bgm?.path) { } else { anomalies.push(`bgm ${audio.bgm.path} not on disk — skipped`); } +} else if (audio.bgm_pending) { + // The distinction the flag exists to make. A warning is not enough: assemble is re-run on + // rework long after the audio step's own warning scrolled past, and it would happily build a + // silent film from a snapshot whose JSON says the bed is still generating. + if (!allowPendingBgm) { + die( + "audio_meta.json says bgm_pending — the music bed is still generating and is NOT in this " + + "assembly. Wait for the track, re-run the audio step, then assemble again. To assemble a " + + "deliberately silent preview anyway, pass --allow-pending-bgm.", + ); + } + anomalies.push( + "bgm still generating (bgm_pending) — assembled without a bed per --allow-pending-bgm", + ); } // (track 2) captions — captions.mjs writes this or legally skips; key off existence. diff --git a/skills/pr-to-video/scripts/assemble-index.test.mjs b/skills/pr-to-video/scripts/assemble-index.test.mjs new file mode 100644 index 0000000000..91c52eaad8 --- /dev/null +++ b/skills/pr-to-video/scripts/assemble-index.test.mjs @@ -0,0 +1,75 @@ +import assert from "node:assert/strict"; +import { existsSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { spawnSync } from "node:child_process"; +import test from "node:test"; + +const assembleScript = new URL("./assemble-index.mjs", import.meta.url).pathname; + +// ── bgm_pending at the assembly boundary ───────────────────────────────────── +// Regression: the flag survived into audio_meta.json but assemble rebuilt its audio object +// from three named keys and dropped it, so the step that actually builds the film could not +// tell "not ready yet" from "silent by design" and would ship the silent one. + +function assembleWith({ audioMeta, extraArgs = [] }) { + const dir = mkdtempSync(join(tmpdir(), "product-launch-assemble-")); + writeFileSync( + join(dir, "STORYBOARD.md"), + "---\nformat: 1920x1080\nmessage: T\n---\n\n## Frame 1 — A\n- duration: 3s\n- src: compositions/frames/01-a.html\n", + ); + mkdirSync(join(dir, "compositions", "frames"), { recursive: true }); + writeFileSync( + join(dir, "compositions", "frames", "01-a.html"), + // pr-to-video's frame contract wants one bare