Feature/video artifact verification (#537)

* fix(release): CI wait polls the prod rung; escape the header tooltip apostrophe

get_latest_ci_conclusion defaults to the ladder's head rung, so
wait_for_ci searched slave for a release commit that lives on master
and timed out after 40 minutes with the run already green. The wait
now passes the prod branch explicitly. Also fixes the
react/no-unescaped-entities error that turned master's Panel CI red.

* fix(panel,video): dead dialog triggers behind tooltips; dotted composition ids render

HelpTip nested inside a Dialog/AlertDialog trigger puts the trigger's
click handler on the Tooltip root, which renders no DOM — the agents
Spawn item and the KB Reindex-All / Delete-index confirms were dead.
Tooltips now wrap the triggers. The video renderer accepts interior
single dots in composition ids (release-0.25.0) with '..' still
unrepresentable, and propose_video refuses an unrenderable id at
authoring time.

* fix(dispatch): restart-safe PM review turns

A leaf task in awaiting_pm_review had no periodic pickup: the closure
dispatcher bailed on childless tasks and skipped PR-bearing review
tasks as already-promoted, assuming the submit-time PM session was
still alive — an assumption every restart breaks. Proven live on the
docs-sync leaf after the 0.25.0 redeploy, which also dependency-blocked
its sibling dev task. Childless awaiting_pm_review tasks now flow to
the PM's review turn, and the merge turn respawns its PM when none is
active.

* feat(video): verify the rendered artifact, not the source

The 14s release-0.25.0 cut shipped with only one of four scenes visibly
registering: the dev authored DOM, the smoke asserted DOM, QA read code —
nobody consumed the rendered MP4 before the CEO did. Close that loop, and
the reject loop behind it:

- sidecar frames mode: POST /render with frames=1..32 renders the cut,
  ffprobes the REAL duration, extracts midpoint-sampled keyframe PNGs
  (timestamps in filenames), streams a tar.gz back with X-Video-Duration
- request_render do-verb (developer/QA, request_sandbox's shape): renders
  the caller's ACTUAL composition — dev's own worktree (head_sha/dirty
  provenance), QA a read-only git-archive export of the assembled branch —
  extracts frames to the container-shared .previews/ path, stamps the
  render_preview marker, returns the paths as envelope evidence
- gate: i_am_done on a source=video task refuses without a stamped
  render_preview (Requirement.RENDER_VERIFIED; canonical source string
  moved to foundation as markers.VIDEO_TASK_SOURCE; mirrored in the
  possibilities-matrix fast path so it cannot bypass the check)
- QA claim_review evidence carries video_context (composition id, the
  dev's preview, a re-render instruction) so review checks output
- dev spawn prompt block + a 4th authoring AC order Read-every-frame
  verification before submitting
- reject -> re-author: a CEO reject with a reason opens a fresh authoring
  task carrying the verbatim feedback + a revise-in-place pointer at the
  existing composition (best-effort, never fails the reject) — rejection
  feedback no longer dies on the cancelled draft

E2E: rendered the committed release-0.25.0 composition through the new
frames mode locally — the returned keyframes show exactly the reported
failure (blank frame at 5.8s, only 'Env ladder' by 12.8s), the check the
fleet was missing.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>

video-renderer/render.js

@@ -8,13 +8,17 @@
 // render call produces its own temp file the caller cleans up once it has
 // streamed the response.
 import { createRenderJob, executeRenderJob } from "@hyperframes/producer";
+import { execFile } from "node:child_process";
 import { existsSync } from "node:fs";
 import { cp, mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import { Readable } from "node:stream";
+import { promisify } from "node:util";
 import * as tar from "tar";
 
+const execFileP = promisify(execFile);
+
 // @hyperframes/producer reads each cut's dimensions from the composition HTML
 // itself (data-width/data-height on the stage), so the sidecar no longer
 // passes width/height — it only picks the quality tier.
@@ -247,4 +251,85 @@ export async function renderComposition({
     }
     throw err;
   }
+}
+
+export const MAX_PREVIEW_FRAMES = 32;
+
+// Downscaled so agents reading the frames as images get a small file that
+// still keeps on-screen copy legible (720 wide ≈ half of a 1080 cut).
+const PREVIEW_FRAME_WIDTH = 720;
+
+/**
+ * Render one cut and extract `frameCount` evenly spaced keyframes from it —
+ * the preview surface behind the fleet's `request_render` verb, so an agent
+ * can verify the actual artifact instead of the composition source. Samples
+ * scene MIDPOINTS (duration * (i + 0.5) / N), never t=0 or t=duration, so a
+ * fade-in first frame or an EOF seek can't produce a blank/missing frame.
+ * Returns the frames tarball path, the probed real duration, and a cleanup
+ * callback the caller MUST invoke after streaming.
+ */
+export async function renderFrames({
+  tarBuffer,
+  compositionId,
+  inputProps,
+  orientation,
+  frameCount,
+}) {
+  const { outputLocation, cleanup: cleanupRender } = await renderComposition({
+    tarBuffer,
+    compositionId,
+    inputProps,
+    orientation,
+  });
+  let framesDir;
+  try {
+    // ffprobe the RENDERED file rather than trusting the composition's
+    // data-duration attribute — the whole point is ground truth.
+    const { stdout } = await execFileP("ffprobe", [
+      "-v", "error",
+      "-show_entries", "format=duration",
+      "-of", "csv=p=0",
+      outputLocation,
+    ]);
+    const duration = Number(stdout.trim());
+    if (!Number.isFinite(duration) || duration <= 0) {
+      throw new Error("could not probe rendered video duration");
+    }
+
+    framesDir = await mkdtemp(path.join(tmpdir(), "hyperframes-frames-"));
+    const files = [];
+    for (let i = 0; i < frameCount; i++) {
+      const t = (duration * (i + 0.5)) / frameCount;
+      // Timestamp in the filename — self-describing, no manifest needed.
+      const name = `frame-${String(i + 1).padStart(2, "0")}-of-${frameCount}-at-${t.toFixed(1)}s.png`;
+      await execFileP("ffmpeg", [
+        "-v", "error",
+        "-ss", t.toFixed(3),
+        "-i", outputLocation,
+        "-frames:v", "1",
+        "-vf", `scale=${PREVIEW_FRAME_WIDTH}:-2`,
+        "-y",
+        path.join(framesDir, name),
+      ]);
+      files.push(name);
+    }
+
+    const tarPath = path.join(framesDir, "frames.tar.gz");
+    await tar.create({ gzip: true, cwd: framesDir, file: tarPath }, files);
+    return {
+      tarPath,
+      duration,
+      cleanup: () =>
+        Promise.all([
+          cleanupRender(),
+          rm(framesDir, { recursive: true, force: true }).catch(() => {}),
+        ]),
+    };
+  } catch (err) {
+    await cleanupRender();
+    if (framesDir) {
+      await rm(framesDir, { recursive: true, force: true }).catch(() => {});
+    }
+    throw err;
+  }
 }

video-renderer/server.js

@@ -10,9 +10,12 @@ import express from "express";
 import multer from "multer";
 import rateLimit from "express-rate-limit";
 import { createReadStream } from "node:fs";
+import { fileURLToPath } from "node:url";
 import {
   renderComposition,
+  renderFrames,
   ExtractedSizeExceededError,
+  MAX_PREVIEW_FRAMES,
   RenderTimeoutError,
   UnknownCompositionError,
 } from "./render.js";
@@ -58,6 +61,61 @@ app.get("/health", (_req, res) => {
   res.status(200).json({ status: "ok" });
 });
 
+/**
+ * Validate the optional 'frames' form field. Absent/empty keeps the
+ * existing MP4 behavior (`count: null`); present must be an integer in
+ * 1..MAX_PREVIEW_FRAMES or `error` names the bound for the 400 response.
+ * Pure + exported so the branch is unit-testable without a live server.
+ */
+export function parseFramesField(raw) {
+  if (raw === undefined || raw === null || raw === "") return { count: null };
+  const count = Number(raw);
+  if (!Number.isInteger(count) || count < 1 || count > MAX_PREVIEW_FRAMES) {
+    return {
+      error: `'frames' must be an integer between 1 and ${MAX_PREVIEW_FRAMES}`,
+    };
+  }
+  return { count };
+}
+
+/**
+ * Stream `filePath` as the response body and run `cleanup` once the
+ * response is done — shared by both the MP4 and the frames-tar branches of
+ * /render. See the inline comment below on why cleanup fires from both the
+ * source stream's own "close" and the response's "close" (client abort).
+ */
+function streamFileWithCleanup(res, filePath, { contentType, headers, cleanup }) {
+  res.status(200);
+  res.setHeader("Content-Type", contentType);
+  for (const [name, value] of Object.entries(headers ?? {})) {
+    res.setHeader(name, value);
+  }
+  const stream = createReadStream(filePath);
+  stream.on("error", (err) => {
+    console.error("video-renderer: stream error", err);
+    if (!res.headersSent) {
+      res.status(500);
+    }
+    res.end();
+    cleanup();
+  });
+  stream.on("close", () => {
+    cleanup();
+  });
+  // stream.pipe() never propagates a DESTINATION close back to the
+  // source: if the client aborts, or the orchestrator's retry-on-timeout
+  // hangs up mid-download, `res` closes but the source stream's own
+  // "close" above never fires — leaking this request's render-output
+  // temp dir on every such disconnect. Destroying the still-open source
+  // releases its fd immediately; cleanup() is idempotent (rm force:true)
+  // so also landing here on a normal end-of-stream close is harmless.
+  res.on("close", () => {
+    stream.destroy();
+    cleanup();
+  });
+  stream.pipe(res);
+}
+
 app.post("/render", renderLimiter, upload.single("source"), async (req, res) => {
   const body = req.body ?? {};
   const compositionId = body.composition_id;
@@ -96,40 +154,39 @@ app.post("/render", renderLimiter, upload.single("source"), async (req, res) =>
     return;
   }
 
+  const framesField = parseFramesField(body.frames);
+  if (framesField.error) {
+    res.status(400).json({ error: framesField.error });
+    return;
+  }
+
   try {
+    if (framesField.count !== null) {
+      const { tarPath, duration, cleanup } = await renderFrames({
+        tarBuffer: req.file.buffer,
+        compositionId,
+        inputProps,
+        orientation,
+        frameCount: framesField.count,
+      });
+      streamFileWithCleanup(res, tarPath, {
+        contentType: "application/gzip",
+        headers: { "X-Video-Duration": String(duration) },
+        cleanup,
+      });
+      return;
+    }
+
     const { outputLocation, cleanup } = await renderComposition({
       tarBuffer: req.file.buffer,
       compositionId,
       inputProps,
       orientation,
     });
-
-    res.status(200);
-    res.setHeader("Content-Type", "video/mp4");
-    const stream = createReadStream(outputLocation);
-    stream.on("error", (err) => {
-      console.error("video-renderer: stream error", err);
-      if (!res.headersSent) {
-        res.status(500);
-      }
-      res.end();
-      cleanup();
+    streamFileWithCleanup(res, outputLocation, {
+      contentType: "video/mp4",
+      cleanup,
     });
-    stream.on("close", () => {
-      cleanup();
-    });
-    // stream.pipe() never propagates a DESTINATION close back to the
-    // source: if the client aborts, or the orchestrator's retry-on-timeout
-    // hangs up mid-download, `res` closes but the source stream's own
-    // "close" above never fires — leaking this request's render-output
-    // temp dir on every such disconnect. Destroying the still-open source
-    // releases its fd immediately; cleanup() is idempotent (rm force:true)
-    // so also landing here on a normal end-of-stream close is harmless.
-    res.on("close", () => {
-      stream.destroy();
-      cleanup();
-    });
-    stream.pipe(res);
   } catch (err) {
     if (
       err instanceof UnknownCompositionError ||
@@ -171,6 +228,10 @@ app.use((err, req, res, _next) => {
   res.status(500).json({ error: "internal error" });
 });
 
-app.listen(PORT, () => {
-  console.log(`video-renderer listening on :${PORT}`);
-});
+// Guarded so a test can `import` this module (for parseFramesField) without
+// also binding the real port — only `node server.js` triggers the listen.
+if (process.argv[1] === fileURLToPath(import.meta.url)) {
+  app.listen(PORT, () => {
+    console.log(`video-renderer listening on :${PORT}`);
+  });
+}

video-renderer/server.test.js

@@ -0,0 +1,42 @@
+// node --test: exercises the pure 'frames' form-field validation only —
+// server.js's app.listen is guarded (see server.js) so importing it here
+// never binds the real port.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { parseFramesField } from "./server.js";
+import { MAX_PREVIEW_FRAMES } from "./render.js";
+
+test("parseFramesField treats an absent field as the existing MP4 path", () => {
+  assert.deepEqual(parseFramesField(undefined), { count: null });
+});
+
+test("parseFramesField treats an empty string as the existing MP4 path", () => {
+  assert.deepEqual(parseFramesField(""), { count: null });
+});
+
+test("parseFramesField accepts an in-bounds integer string", () => {
+  assert.deepEqual(parseFramesField("8"), { count: 8 });
+});
+
+test("parseFramesField accepts the boundary values 1 and MAX_PREVIEW_FRAMES", () => {
+  assert.deepEqual(parseFramesField("1"), { count: 1 });
+  assert.deepEqual(parseFramesField(String(MAX_PREVIEW_FRAMES)), {
+    count: MAX_PREVIEW_FRAMES,
+  });
+});
+
+test("parseFramesField rejects zero", () => {
+  assert.ok(parseFramesField("0").error);
+});
+
+test("parseFramesField rejects above MAX_PREVIEW_FRAMES", () => {
+  assert.ok(parseFramesField(String(MAX_PREVIEW_FRAMES + 1)).error);
+});
+
+test("parseFramesField rejects a non-integer value", () => {
+  assert.ok(parseFramesField("4.5").error);
+});
+
+test("parseFramesField rejects a non-numeric value", () => {
+  assert.ok(parseFramesField("abc").error);
+});