fix(product-launch): hoist approved frame videos during assembly (#2226)

* fix(product-launch): keep media out of frame subcompositions

* fix(product-launch): hoist approved frame videos at assembly

* fix(product-launch): format and refresh media contract

* test(product-launch): harden approved video hoist

* chore: refresh product-launch skill manifest

* fix(product-launch): validate and sanitize hoisted video attrs

* fix(product-launch): allowlist hoisted video attributes

skills-manifest.json

@@ -62,8 +62,8 @@
       "files": 22
     },
     "product-launch-video": {
-      "hash": "158398dbfc4ab652",
-      "files": 20
+      "hash": "cb30ad95ba8863c7",
+      "files": 21
     },
     "remotion-to-hyperframes": {
       "hash": "aa599f027db4b994",

skills/product-launch-video/scripts/assemble-index.mjs

@@ -38,8 +38,8 @@
 // `lint` failures surface HERE instead of after assembly + a wasted render):
 //   ① AUTO-REPAIR — a sub-comp root missing data-width/data-height: inject the canvas
 //      dims (the renderer needs them on the cloned root; else lint root_missing_dimensions).
-//   ② HARD FAIL  — <video>/<audio> inside a sub-comp: the runtime only drives media that
-//      is a DIRECT child of the host root, so sub-comp media renders blank/black.
+//   ② APPROVED VIDEO HOIST — an explicitly marked frame video is moved to the host root;
+//      audio remains orchestrator-owned and unmarked media is still a hard failure.
 //   ③ HARD FAIL  — a timed element (data-start+duration+track-index) that is not the root
 //      and lacks class="clip" (shows the whole frame), or two same-track clips that overlap.
 //
@@ -161,9 +161,79 @@ function findRootTag(html) {
   return firstCompId;
 }
 
+function attrValueFrom(attrs, name) {
+  const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+  const match = attrs.match(new RegExp(`(?:^|\\s)${escaped}\\s*=\\s*(?:"([^"]*)"|'([^']*)')`));
+  return match ? (match[1] ?? match[2]) : null;
+}
+
+function escapeHtmlAttr(value) {
+  return value
+    .replaceAll("&", "&amp;")
+    .replaceAll('"', "&quot;")
+    .replaceAll("<", "&lt;")
+    .replaceAll(">", "&gt;");
+}
+
+function approvedVideoAttrs(attrs) {
+  const forwarded = [];
+  for (const name of ["id", "src", "poster", "preload", "aria-label"]) {
+    const value = attrValueFrom(attrs, name);
+    if (value !== null) forwarded.push(`${name}="${escapeHtmlAttr(value)}"`);
+  }
+  for (const name of ["muted", "playsinline", "loop"]) {
+    if (attrPresent(attrs, name)) forwarded.push(name);
+  }
+  return forwarded.join(" ");
+}
+
+function hoistApprovedVideos(html, label) {
+  const videos = [];
+  const errors = [];
+  const scan = html
+    .replace(/<!--[\s\S]*?-->/g, (match) => " ".repeat(match.length))
+    .replace(/<script\b[\s\S]*?<\/script[^>]*>/gi, (match) => " ".repeat(match.length))
+    .replace(/<style\b[\s\S]*?<\/style[^>]*>/gi, (match) => " ".repeat(match.length));
+  const re = /<video\b((?:[^>"']|"[^"]*"|'[^']*')*)>([\s\S]*?)<\/video\s*>/gi;
+  const repaired = html.replace(re, (full, attrs, inner, offset) => {
+    if (scan[offset] !== "<") return full;
+    if (attrValueFrom(attrs, "data-frame-video") !== "approved") return full;
+    const rawStart = attrValueFrom(attrs, "data-start");
+    const rawDuration = attrValueFrom(attrs, "data-duration");
+    const rawTrack = attrValueFrom(attrs, "data-track-index");
+    if (rawStart === null || rawDuration === null || rawTrack === null) {
+      errors.push(
+        `${label}: approved frame video must declare quoted data-start, data-duration, and data-track-index`,
+      );
+      return full;
+    }
+    const start = Number(rawStart);
+    const duration = Number(rawDuration);
+    const track = Number(rawTrack);
+    if (
+      !Number.isFinite(start) ||
+      !Number.isFinite(duration) ||
+      duration <= 0 ||
+      !Number.isFinite(track)
+    ) {
+      errors.push(
+        `${label}: approved frame video must declare finite data-start, positive data-duration, and data-track-index`,
+      );
+      return full;
+    }
+    videos.push({ attrs: approvedVideoAttrs(attrs), inner, start, duration, track });
+    return "<!-- approved frame video hoisted by assemble-index -->";
+  });
+  return { html: repaired, videos, errors };
+}
+
 // Returns { errors: string[], repairedHtml: string|null, repairNote: string|null }.
 function guardFrame(html, label) {
   const errors = [];
+  const originalHtml = html;
+  const approved = hoistApprovedVideos(html, label);
+  html = approved.html;
+  errors.push(...approved.errors);
   // Scan a copy with comments + <script>/<style> bodies blanked, so a tag-like string
   // in a comment (e.g. "<!-- match the host <video> coords -->") or in GSAP code can't
   // trip ②/③. ① still splices into the ORIGINAL html, so its offsets stay correct.
@@ -224,7 +294,7 @@ function guardFrame(html, label) {
   }
 
   // ① auto-repair: ensure the root carries data-width / data-height.
-  let repairedHtml = null;
+  let repairedHtml = approved.html !== originalHtml ? approved.html : null;
   let repairNote = null;
   const root = findRootTag(html);
   if (root) {
@@ -239,7 +309,7 @@ function guardFrame(html, label) {
     }
   }
 
-  return { errors, repairedHtml, repairNote };
+  return { errors, repairedHtml, repairNote, hoistedVideos: approved.videos };
 }
 
 // ---------- resolve mountable frames in document order ----------
@@ -299,7 +369,12 @@ for (const f of manifest.frames) {
   ) {
     die(`${label}: ${f.src} has no data-composition-id="${compId}" (host/inner id must match)`);
   }
-  mounted.push({ frame: f, compId, durationSeconds: r3(f.durationSeconds) });
+  mounted.push({
+    frame: f,
+    compId,
+    durationSeconds: r3(f.durationSeconds),
+    hoistedVideos: guard.hoistedVideos,
+  });
 }
 if (frameErrors.length) {
   die(
@@ -373,6 +448,25 @@ for (const m of mounted) {
   body.push("");
 }
 
+// Approved frame videos are mounted at the host root after frame clips. Translate
+// frame-relative timing to the global timeline and keep them off audio/frame lanes.
+for (const [frameIndex, m] of mounted.entries()) {
+  for (const video of m.hoistedVideos ?? []) {
+    const globalStart = r3(m.start + video.start);
+    const track = 1000 + frameIndex * 1000 + video.track;
+    const id = /(?:^|\s)id\s*=/.test(video.attrs) ? "" : ` id="el-${m.compId}-video-${frameIndex}"`;
+    body.push(
+      `      <video${id} ${video.attrs}`,
+      `        class="clip"`,
+      `        data-start="${globalStart}"`,
+      `        data-duration="${r3(video.duration)}"`,
+      `        data-track-index="${track}"`,
+      `      >${video.inner}</video>`,
+      "",
+    );
+  }
+}
+
 // (track 11) BGM — duck under narration when any voice is present. Loop-extend a short
 // track to the full video length so the tail isn't silent (libraries return ~15–30s clips).
 let bgmEmitted = false;

skills/product-launch-video/scripts/media-contract.test.mjs

@@ -0,0 +1,94 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+import { tmpdir } from "node:os";
+import { spawnSync } from "node:child_process";
+
+const skillDir = join(dirname(fileURLToPath(import.meta.url)), "..");
+
+test("frame worker documents the approved video-hoist contract", () => {
+  const instructions = readFileSync(join(skillDir, "sub-agents", "frame-worker.md"), "utf8");
+
+  assert.match(instructions, /data-frame-video="approved"/);
+  assert.match(instructions, /assemble-index\.mjs.*hoists it to the host root/i);
+  assert.match(instructions, /Audio remains orchestrator-owned/i);
+});
+
+test("assemble hoists an approved timed frame video to the host root", () => {
+  const project = mkdtempSync(join(tmpdir(), "hf-frame-video-"));
+  mkdirSync(join(project, "compositions"));
+  const framePath = join(project, "compositions", "frame-1.html");
+  writeFileSync(
+    join(project, "STORYBOARD.md"),
+    "---\nformat: 16:9\n---\n\n## Frame 1 — Demo\n- status: built\n- duration: 2s\n- src: compositions/frame-1.html\n",
+  );
+  writeFileSync(
+    framePath,
+    `<html><body><div id="root" data-composition-id="frame-1" data-width="1920" data-height="1080"><video data-frame-video="approved" src="https://cdn.example/clip.mp4" poster="poster.png" preload="auto" muted playsinline loop style="background:url(https://evil.example/x)" nonce="unsafe" onerror="alert(1)" srcdoc="<script>alert(2)</script>" data-start="0.25" data-duration="1.5" data-track-index="7"></video></div><script>window.__timelines = {}; window.__timelines["frame-1"] = gsap.timeline();</script></body></html>`,
+  );
+
+  const result = spawnSync(
+    process.execPath,
+    [join(skillDir, "scripts", "assemble-index.mjs"), "--hyperframes", project],
+    { encoding: "utf8" },
+  );
+  assert.equal(result.status, 0, result.stderr);
+  const index = readFileSync(join(project, "index.html"), "utf8");
+  const frame = readFileSync(framePath, "utf8");
+  assert.match(index, /data-start="0\.25"/);
+  assert.match(index, /data-duration="1\.5"/);
+  assert.match(index, /data-track-index="1007"/);
+  assert.match(index, /src="https:\/\/cdn\.example\/clip\.mp4"/);
+  assert.match(index, /poster="poster\.png"/);
+  assert.match(index, /preload="auto"/);
+  assert.match(index, /\smuted(?:\s|>)/);
+  assert.match(index, /\splaysinline(?:\s|>)/);
+  assert.match(index, /\sloop(?:\s|>)/);
+  assert.doesNotMatch(index, /onerror=/i);
+  assert.doesNotMatch(index, /srcdoc=/i);
+  assert.doesNotMatch(index, /nonce=/i);
+  assert.doesNotMatch(index, /style=/i);
+  assert.doesNotMatch(frame, /<video\b/i);
+});
+
+test("rejects an approved video with missing admission timing", () => {
+  const project = mkdtempSync(join(tmpdir(), "hf-frame-video-missing-"));
+  mkdirSync(join(project, "compositions"));
+  writeFileSync(
+    join(project, "STORYBOARD.md"),
+    "---\nformat: 16:9\n---\n\n## Frame 1\n- status: built\n- duration: 2s\n- src: compositions/frame-1.html\n",
+  );
+  writeFileSync(
+    join(project, "compositions", "frame-1.html"),
+    `<html><body><div id="root" data-composition-id="frame-1" data-width="1920" data-height="1080"><video data-frame-video="approved" src="clip.mp4" data-duration="1" data-track-index="0"></video></div><script>window.__timelines = {}; window.__timelines["frame-1"] = gsap.timeline();</script></body></html>`,
+  );
+  const result = spawnSync(
+    process.execPath,
+    [join(skillDir, "scripts", "assemble-index.mjs"), "--hyperframes", project],
+    { encoding: "utf8" },
+  );
+  assert.notEqual(result.status, 0);
+  assert.match(result.stderr, /must declare quoted data-start/i);
+});
+
+test("does not hoist declarations hidden in comments or scripts", () => {
+  const project = mkdtempSync(join(tmpdir(), "hf-frame-video-hidden-"));
+  mkdirSync(join(project, "compositions"));
+  writeFileSync(
+    join(project, "STORYBOARD.md"),
+    "---\nformat: 16:9\n---\n\n## Frame 1\n- status: built\n- duration: 2s\n- src: compositions/frame-1.html\n",
+  );
+  writeFileSync(
+    join(project, "compositions", "frame-1.html"),
+    `<html><body><div id="root" data-composition-id="frame-1" data-width="1920" data-height="1080"></div><script>window.__timelines = {}; window.__timelines["frame-1"] = gsap.timeline(); const s = '<video data-frame-video="approved" data-start="0" data-duration="1" data-track-index="1"></video>';</script><!-- <video data-frame-video="approved" data-start="0" data-duration="1" data-track-index="2"></video> --></body></html>`,
+  );
+  const result = spawnSync(
+    process.execPath,
+    [join(skillDir, "scripts", "assemble-index.mjs"), "--hyperframes", project],
+    { encoding: "utf8" },
+  );
+  assert.equal(result.status, 0, result.stderr);
+  assert.doesNotMatch(readFileSync(join(project, "index.html"), "utf8"), /data-track-index="1001"/);
+});

skills/product-launch-video/sub-agents/frame-worker.md

@@ -14,7 +14,7 @@
   - the **time-coded shot sequence** — your build spec. A sequence of Scene lines (`Scene 1 (0.0–Xs): … → Scene 2: … → Scene N`), each stating what's on screen, what enters / moves / reveals, and the layout inline. Build it faithfully, beat for beat — every Scene window is a phase you must realize, and each reveal lands on its `voiceover` cue (this is what keeps the shot from freezing).
   - `blueprint:` — an id (or the literal `compose`). The id points to `../hyperframes-animation/blueprints/<id>.md`: the **product-agnostic shot template** this frame instantiates — the overall shape + its signature move. Read it for the shape; `compose` means there's no template, sequence the shot from the Scene lines directly.
   - `focal:` — which candidate is the hero.
-  - `roles:` — each candidate's role: `cutout` foreground / `background` full-bleed / supporting — plus the real media available (each `public/<basename> — description`; a **`[video]`** tag marks a `.mp4` motion clip).
+  - `roles:` — each candidate's role: `cutout` foreground / `background` full-bleed / supporting — plus the real media available (each `public/<basename> — description`; a **`[video]`** tag marks a `.mp4` motion source that cannot be mounted by this sub-composition worker).
   - `sfx:` — the orchestrator's; you mount no audio.
 - `frame.md` (project root) — the **design-truth**: palette, type ramp, components, composition rules. The LOOK. Pull every visual token from here.
 - `RULES_DIR` — absolute path to this skill's local `../hyperframes-animation/rules/`. The **named motion verbs in the Scene lines** (and the moves the blueprint cites) resolve to rule recipes here: `RULES_DIR/<id>.md` is the mechanics for a motion. (A few rules link an optional runnable demo in the shared `../hyperframes-animation/examples/<id>.html` — open it only when a recipe is unclear.)
@@ -46,7 +46,7 @@ Generic seek-safety + structure live in `hyperframes-core` (read it; not restate
 - **Visible text is short motion-graphics copy** — headline / stat / one-word emphasis (`"$83K"`, `"INSTANT"`), never a sentence from the narration. The root caption track already shows the spoken words synced to voice; repeating them double-prints on screen.
 - **Build the whole shot — reveal across the full `duration`, never front-load.** Dumping the whole canvas in the first ~25% then holding it is exactly what reads as a PowerPoint slide. Instead reveal each piece — a line, a card, a stat, an icon — **as the `voiceover` reaches it**, sequencing reveals across the shot and especially the back ~50%, with the macro camera move running underneath. **Only EXITS are banned** — a non-final frame unmounts mid-frame, so an exit tween truncates and reads as a glitch (the root transition IS the exit); mid-shot reveals are free and seek-safe. The lone exception is a note marked as a deliberate hold / stillness frame: there, an entrance + a quiet settle is right (a held read beats bad motion).
 - **Implement the shot sequence faithfully — every Scene is a timeline phase.** The Scene lines ARE the build: map each Scene onto a phase of the one timeline, each piece revealing as the `voiceover` reaches it. For each **named motion** in a Scene, open its rule recipe under `RULES_DIR/<id>.md` and reproduce its mechanics — **never name-guess** (a guess loses the signature move). The **`blueprint:` template** (`../hyperframes-animation/blueprints/<id>.md`) gives the overall shape; read it and keep its **signature move** recognizable, then instantiate it with this frame's content / assets / timing. `compose` → no template; sequence the shot straight from the Scene lines. Whichever, never front-load the whole sequence at `t=0` — pace the reveals to the voiceover.
-- **Place each candidate by its `roles`** (the `focal` is the hero): a `cutout` is a foreground subject — respect the 83% keep-out, lay text around it, not over its face; a `background` is full-bleed and dimmed ~30–50% so foreground content stays legible. **A `[video]` candidate (`.mp4`) is a real motion clip — usually the strongest hero for a motion/demo product.** Render it as a **muted** `<video class="clip">` (`data-start` / `data-duration` / `data-track-index` per the core clip contract), a **direct child of the frame root** — never nested in another timed element, or the renderer freezes it. Keep it muted (the root owns all audio); a `[video-still]` or untagged image → `<img>`.
+- **Place each candidate by its `roles`** (the `focal` is the hero): a `cutout` is a foreground subject — respect the 83% keep-out, lay text around it, not over its face; a `background` is full-bleed and dimmed ~30–50% so foreground content stays legible. Frame files are sub-compositions. Audio remains orchestrator-owned: never author `<audio>` in a frame. An approved `[video]` candidate may be declared as a frame-local `<video data-frame-video="approved" data-start="..." data-duration="..." data-track-index="...">`; `assemble-index.mjs` hoists it to the host root and translates its timing. Do not use this declaration for audio, unapproved URLs, or videos without explicit timing. If no approved video is supplied, use an explicitly supplied static still/key art (`[video-still]` or another image candidate) as `<img>`; do not extract a frame, fabricate a URL, or silently embed an unapproved clip.
 
 ## Workflow