From b087f1e3c09d24e3a339ec4bf2f129af39f0a915 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel?= <miguel.sierra@heygen.com>
Date: Thu, 2 Jul 2026 17:45:19 -0700
Subject: [PATCH] fix(cli): validate stops misreporting slow-loading media as
 unreadable (#1849)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two independent post-release feedback reports of validate warning about
audio duration despite an explicit, correct data-duration slot, one of
them naming a timeout explicitly.

Root cause: auditClipDurations reads each <video>/<audio> element's
intrinsic .duration via a single page.evaluate() snapshot taken after a
flat, unconditional page-settle sleep (opts.timeout ?? 3000ms, shared with
other audits). Per the HTML spec, HTMLMediaElement.duration is NaN until
metadata loads. A slow-loading audio file (large narration WAV, remote
source) can still be mid-fetch when that sleep elapses — el.duration is
NaN at that exact instant, which the audit permanently records as
"could not read the duration" even though the render pipeline (which
properly awaits media readiness) handles the same file fine.

Fix: race each not-yet-ready element's loadedmetadata/error event against
a deadline instead of taking one fixed-time snapshot. Elements already
ready resolve immediately (no added latency in the common case); only
genuinely slow elements get a real second chance before the warning fires.

The race/cleanup wiring lives twice by necessity — once inline inside the
page.evaluate() closure (Puppeteer serializes and re-runs that closure in
an isolated browser realm with no access to this module), and once as the
exported, duck-typed raceMediaReady for a real, deterministic unit test
via Node's built-in EventTarget (no browser or DOM library needed). The
comment on raceMediaReady flags that both copies must move together.
---
 packages/cli/src/commands/validate.test.ts | 55 ++++++++++++++-
 packages/cli/src/commands/validate.ts      | 82 ++++++++++++++++++++--
 2 files changed, 129 insertions(+), 8 deletions(-)

diff --git a/packages/cli/src/commands/validate.test.ts b/packages/cli/src/commands/validate.test.ts
index 22f628856..1030612c5 100644
--- a/packages/cli/src/commands/validate.test.ts
+++ b/packages/cli/src/commands/validate.test.ts
@@ -1,7 +1,60 @@
 import { describe, expect, it } from "vitest";
-import { extractCompositionErrorsFromLint, shouldIgnoreRequestFailure } from "./validate.js";
+import {
+  extractCompositionErrorsFromLint,
+  raceMediaReady,
+  shouldIgnoreRequestFailure,
+} from "./validate.js";
 import type { ProjectLintResult } from "../utils/lintProject.js";
 
+// Regression for the validate audio-duration-probe timeout: a slow-loading
+// media element's duration was snapshotted once, at a fixed point in time,
+// and any element still mid-load was permanently misreported as unreadable.
+// raceMediaReady is the extracted wiring auditClipDurations now uses to wait
+// for `loadedmetadata` up to a deadline instead. Node's built-in EventTarget
+// satisfies the same duck-typed shape as a real HTMLMediaElement here, so
+// this is a real test of the race/cleanup logic, not a browser mock.
+describe("raceMediaReady", () => {
+  class FakeMediaElement extends EventTarget {
+    duration = NaN;
+  }
+
+  it("resolves immediately when duration is already available", async () => {
+    const el = new FakeMediaElement();
+    el.duration = 12.5;
+    const start = Date.now();
+    await raceMediaReady(el, Date.now() + 5000);
+    expect(Date.now() - start).toBeLessThan(50);
+  });
+
+  it("resolves as soon as loadedmetadata fires, before the deadline", async () => {
+    const el = new FakeMediaElement();
+    const promise = raceMediaReady(el, Date.now() + 5000);
+    setTimeout(() => {
+      el.duration = 8;
+      el.dispatchEvent(new Event("loadedmetadata"));
+    }, 20);
+    const start = Date.now();
+    await promise;
+    expect(Date.now() - start).toBeLessThan(200);
+  });
+
+  it("resolves on error without hanging until the deadline", async () => {
+    const el = new FakeMediaElement();
+    const promise = raceMediaReady(el, Date.now() + 5000);
+    setTimeout(() => el.dispatchEvent(new Event("error")), 20);
+    const start = Date.now();
+    await promise;
+    expect(Date.now() - start).toBeLessThan(200);
+  });
+
+  it("falls back to the deadline when no event ever fires", async () => {
+    const el = new FakeMediaElement();
+    const start = Date.now();
+    await raceMediaReady(el, Date.now() + 50);
+    expect(Date.now() - start).toBeGreaterThanOrEqual(40);
+  });
+});
+
 describe("shouldIgnoreRequestFailure", () => {
   it("ignores aborted media preload requests", () => {
     expect(
diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts
index 824f5378c..a298860d3 100644
--- a/packages/cli/src/commands/validate.ts
+++ b/packages/cli/src/commands/validate.ts
@@ -74,6 +74,37 @@ async function seekTo(page: import("puppeteer-core").Page, time: number): Promis
   await new Promise((r) => setTimeout(r, SEEK_SETTLE_MS));
 }
 
+/**
+ * Race a media element's `loadedmetadata`/`error` event against a deadline,
+ * whichever comes first. Already-ready elements resolve immediately.
+ *
+ * This is the same wiring as the inline copy inside `auditClipDurations`'s
+ * `page.evaluate()` below — duplicated, not imported, because Puppeteer
+ * serializes that closure's source and re-runs it in an isolated browser
+ * realm with no access to this module's scope. Kept here (duck-typed on
+ * `EventTarget`-shaped objects, not `HTMLMediaElement`) so the actual
+ * race/cleanup logic has a real, deterministic unit test — Node's built-in
+ * `EventTarget` satisfies the same shape without a browser or DOM library.
+ * If you change one copy, change both.
+ */
+export function raceMediaReady(
+  el: EventTarget & { duration: number },
+  deadlineMs: number,
+): Promise<void> {
+  if (Number.isFinite(el.duration) && el.duration > 0) return Promise.resolve();
+  return new Promise<void>((resolve) => {
+    const onReady = () => {
+      el.removeEventListener("loadedmetadata", onReady);
+      el.removeEventListener("error", onReady);
+      clearTimeout(timer);
+      resolve();
+    };
+    el.addEventListener("loadedmetadata", onReady, { once: true });
+    el.addEventListener("error", onReady, { once: true });
+    const timer = setTimeout(onReady, Math.max(0, deadlineMs - Date.now()));
+  });
+}
+
 /**
  * Flag `<video>`/`<audio>` clips whose source is meaningfully shorter than their
  * `data-duration` slot (the slot gets silently shortened in renders). Runs in
@@ -83,8 +114,46 @@ async function seekTo(page: import("puppeteer-core").Page, time: number): Promis
 async function auditClipDurations(
   page: import("puppeteer-core").Page,
   analyzeClipMediaFit: typeof import("@hyperframes/engine").analyzeClipMediaFit,
+  extraWaitMs: number,
 ): Promise<ConsoleEntry[]> {
-  const clips = await page.evaluate(() => {
+  // fallow-ignore-next-line complexity
+  const clips = await page.evaluate(async (maxWaitMs: number) => {
+    const nodes = Array.from(
+      document.querySelectorAll("video[data-duration], audio[data-duration]"),
+    ) as HTMLMediaElement[];
+
+    // The caller's page-settle sleep is a flat, unconditional wait shared with
+    // other audits — it isn't aware of how long any given media element takes
+    // to load metadata. A slow-loading audio file (large narration WAV, remote
+    // source) can still be mid-fetch when that sleep elapses, which read as
+    // el.duration === NaN and was misreported as "could not read the duration"
+    // even though the render pipeline (which properly awaits media readiness)
+    // handles the same file fine. Give still-loading elements one more real
+    // chance via loadedmetadata before giving up, instead of a single fixed-time
+    // snapshot. Elements that already have a duration resolve immediately, so
+    // this adds no latency in the common case.
+    const deadline = Date.now() + maxWaitMs;
+    await Promise.all(
+      nodes.map((el) => {
+        if (Number.isFinite(el.duration) && el.duration > 0) return Promise.resolve();
+        return new Promise<void>((resolve) => {
+          // fallow-ignore-next-line code-duplication
+          const cleanup = () => {
+            el.removeEventListener("loadedmetadata", onReady);
+            el.removeEventListener("error", onReady);
+            clearTimeout(timer);
+          };
+          const onReady = () => {
+            cleanup();
+            resolve();
+          };
+          el.addEventListener("loadedmetadata", onReady, { once: true });
+          el.addEventListener("error", onReady, { once: true });
+          const timer = setTimeout(onReady, Math.max(0, deadline - Date.now()));
+        });
+      }),
+    );
+
     const rows: Array<{
       id: string;
       kind: string;
@@ -93,10 +162,9 @@ async function auditClipDurations(
       duration: number;
       loop: boolean;
     }> = [];
-    document.querySelectorAll("video[data-duration], audio[data-duration]").forEach((node) => {
-      const el = node as HTMLMediaElement;
+    for (const el of nodes) {
       const slot = parseFloat(el.getAttribute("data-duration") ?? "");
-      if (!(slot > 0)) return;
+      if (!(slot > 0)) continue;
       rows.push({
         id: el.id || el.getAttribute("src") || `(${el.tagName.toLowerCase()})`,
         kind: el.tagName === "AUDIO" ? "Audio" : "Video",
@@ -105,9 +173,9 @@ async function auditClipDurations(
         duration: el.duration,
         loop: el.loop || el.getAttribute("data-loop") === "true",
       });
-    });
+    }
     return rows;
-  });
+  }, extraWaitMs);
 
   const warnings: ConsoleEntry[] = [];
   const unreadable: string[] = [];
@@ -288,7 +356,7 @@ async function validateInBrowser(
     await page.goto(server.url, { waitUntil: "domcontentloaded", timeout: 10000 });
     await new Promise((r) => setTimeout(r, opts.timeout ?? 3000));
 
-    for (const w of await auditClipDurations(page, analyzeClipMediaFit)) {
+    for (const w of await auditClipDurations(page, analyzeClipMediaFit, opts.timeout ?? 3000)) {
       warnings.push(w);
     }