fix(capture,audio,docs): defects found running product-launch-video end to end

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 = '<svg viewBox="0 0 24 24"><path d="M0 0h24v24H0z"/></svg>';
+    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('<path d="M0 0h24v24H0z"/>');
+    expect(out.endsWith("</svg>")).toBe(true);
+  });
+
+  it("leaves an SVG that already declares xmlns untouched", () => {
+    const already = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 8 8"><rect/></svg>';
+    expect(toStandaloneSvg(already)).toBe(already);
+  });
+
+  it("declares xmlns:xlink only when an xlink: attribute is actually used", () => {
+    const withXlink = '<svg viewBox="0 0 8 8"><use xlink:href="#a"/></svg>';
+    expect(toStandaloneSvg(withXlink)).toContain('xmlns:xlink="http://www.w3.org/1999/xlink"');
+    const without = '<svg viewBox="0 0 8 8"><use href="#a"/></svg>';
+    expect(toStandaloneSvg(without)).not.toContain("xmlns:xlink");
+  });
+
+  it("is idempotent and preserves attributes on the root", () => {
+    const inline = '<svg class="logo" width="120" height="24" fill="currentColor"><g/></svg>';
+    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("<div>not an svg</div>")).toBe("<div>not an svg</div>");
+  });
+});

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 `<svg>` 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: `<img src="logo-abc123.svg">` 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(/<svg\b[^>]*>/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(/^<svg\b/i, `<svg ${add.join(" ")}`);
+  return outerHTML.replace(original, tag);
+}
+
 export async function downloadAssets(
   tokens: DesignTokens,
   outputDir: string,
@@ -34,7 +60,9 @@ export async function downloadAssets(
   for (let i = 0; i < tokens.svgs.length && i < 30; i++) {
     const svg = tokens.svgs[i]!;
     if (!svg.outerHTML || svg.outerHTML.length < 50) continue;
-    const slug = svgContentHashSlug(svg.outerHTML, !!svg.isLogo);
+    // Hash the bytes that actually land on disk, so the filename still can't drift from content.
+    const svgFile = toStandaloneSvg(svg.outerHTML);
+    const slug = svgContentHashSlug(svgFile, !!svg.isLogo);
     let finalSlug = slug;
     let suffix = 2;
     while (usedSvgNames.has(finalSlug)) {
@@ -45,7 +73,7 @@ export async function downloadAssets(
     const name = `${finalSlug}.svg`;
     const localPath = `assets/svgs/${name}`;
     try {
-      writeFileSync(join(outputDir, localPath), svg.outerHTML, "utf-8");
+      writeFileSync(join(outputDir, localPath), svgFile, "utf-8");
       assets.push({ url: "", localPath, type: "svg" });
     } catch {
       /* skip */

packages/cli/src/capture/screenshotCapture.test.ts

@@ -0,0 +1,184 @@
+import { describe, expect, it, vi } from "vitest";
+import { existsSync, mkdtempSync, readFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import type { Page } from "puppeteer-core";
+import { captureFullPagePlate, MAX_PLATE_HEIGHT_PX, pngHeight } from "./screenshotCapture.js";
+
+// A real 1920x800 PNG header, so the produced-height guard sees something valid.
+function pngBuffer(height: number, width = 1920): Buffer {
+  const buf = Buffer.alloc(24);
+  Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]).copy(buf, 0);
+  buf.writeUInt32BE(13, 8);
+  buf.write("IHDR", 12, "ascii");
+  buf.writeUInt32BE(width, 16);
+  buf.writeUInt32BE(height, 20);
+  return buf;
+}
+
+// The mocks declare their parameters so `mock.calls[i][0]` is a real slot — a zero-arg
+// vi.fn() types its call tuple as [] and indexing it is a compile error.
+// `docHeight` is what the in-function measurement returns; the plate reads the page height
+// itself now rather than trusting a value the caller measured before scrolling.
+function fakePage(
+  { docHeight = 8000, plateHeight = 8000 }: { docHeight?: number; plateHeight?: number } = {},
+  overrides: Record<string, unknown> = {},
+) {
+  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");
+  });
+});

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<string | null> {
+  // 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<string[]> {
   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 */
   }