fix(core): address review — gate early diagnostic, fix empty crossOrigin, document gaps

Author: miguel-heygen

packages/core/package-subpaths.json

@@ -236,6 +236,12 @@
       "types": "./dist/runtime/stackingContext.d.ts",
       "environments": ["browser", "bun", "node"]
     },
+    "./runtime/web-audio-route": {
+      "source": "./src/runtime/webAudioRoute.ts",
+      "runtime": "./dist/runtime/webAudioRoute.js",
+      "types": "./dist/runtime/webAudioRoute.d.ts",
+      "environments": ["browser", "bun", "node"]
+    },
     "./compiler/html-document": {
       "source": "./src/compiler/htmlDocument.ts",
       "runtime": "./dist/compiler/htmlDocument.js",

packages/core/package.json

@@ -245,6 +245,12 @@
       "import": "./src/runtime/stackingContext.ts",
       "types": "./src/runtime/stackingContext.ts"
     },
+    "./runtime/web-audio-route": {
+      "bun": "./src/runtime/webAudioRoute.ts",
+      "node": "./dist/runtime/webAudioRoute.js",
+      "import": "./src/runtime/webAudioRoute.ts",
+      "types": "./src/runtime/webAudioRoute.ts"
+    },
     "./compiler/html-document": {
       "bun": "./src/compiler/htmlDocument.ts",
       "node": "./dist/compiler/htmlDocument.js",
@@ -539,6 +545,10 @@
         "import": "./dist/runtime/stackingContext.js",
         "types": "./dist/runtime/stackingContext.d.ts"
       },
+      "./runtime/web-audio-route": {
+        "import": "./dist/runtime/webAudioRoute.js",
+        "types": "./dist/runtime/webAudioRoute.d.ts"
+      },
       "./compiler/html-document": {
         "import": "./dist/compiler/htmlDocument.js",
         "types": "./dist/compiler/htmlDocument.d.ts"

packages/core/src/runtime/init.ts

@@ -42,7 +42,11 @@ import { applyVariableBindings } from "./applyVariableBindings";
 import { createColorGradingRuntime, type RuntimeColorGradingApi } from "./colorGrading";
 import { TransportClock } from "./clock";
 import { WebAudioTransport } from "./webAudioTransport";
-import { classifyWebAudioMediaRoute, reportWebAudioMediaRoute } from "./webAudioRoute.js";
+import {
+  classifyWebAudioMediaRoute,
+  isRouteSelectionSettled,
+  reportWebAudioMediaRoute,
+} from "./webAudioRoute.js";
 import {
   ensureAudioGroupInertStyle,
   HF_AUDIO_GROUP_TAG,
@@ -1840,6 +1844,13 @@ export function initSandboxRuntimeModular(): void {
   // were.
   const reportWebAudioRoute = (mediaEl: HTMLMediaElement) => {
     if (!(mediaEl instanceof HTMLAudioElement)) return;
+    // Before resource selection settles, the verdict is built from `<source>`
+    // children the browser might still pass over — good enough for the
+    // schedule path's conservative withhold, not good enough to put in front
+    // of a human as a diagnostic. Skip; the `loadedmetadata` call to this same
+    // function (see below) always has a settled `currentSrc` and will report
+    // for real once the guess would no longer be one.
+    if (!isRouteSelectionSettled(mediaEl)) return;
     reportWebAudioMediaRoute(mediaEl, classifyWebAudioMediaRoute(mediaEl));
   };
 
@@ -1875,10 +1886,15 @@ export function initSandboxRuntimeModular(): void {
       // schedule time. `hyperframes check` seeks, it never calls play(), so a
       // diagnostic raised from the transport would be invisible to the one
       // gate whose job is to surface exactly this class of silent failure.
-      // Bound twice on purpose: now, so a composition that never plays still
-      // reports, and again at `loadedmetadata`, when `currentSrc` is finally
-      // authoritative and the early read may have been guessing from
-      // `<source>` children. `reportWebAudioMediaRoute` latches per element.
+      // Bound twice on purpose: now, for a `src`/committed-`currentSrc`
+      // element so a composition that never plays still reports promptly, and
+      // again at `loadedmetadata`, when `currentSrc` is unconditionally
+      // authoritative. `reportWebAudioRoute` itself skips the "now" call when
+      // selection hasn't settled (see `isRouteSelectionSettled`) — with only
+      // `<source>` children to go on, the browser could still pick a
+      // different one than the classifier just judged, and a diagnostic is a
+      // claim of fact, not a guess. `reportWebAudioMediaRoute` latches per
+      // element, so the deferred-to-`loadedmetadata` case still reports once.
       mediaEl.addEventListener("loadedmetadata", onMediaLoadedMetadataForRoute);
       reportWebAudioRoute(mediaEl);
       // Reactive (zero-videoWidth) + tertiary (error event) proxy-fallback

packages/core/src/runtime/webAudioRoute.test.ts

@@ -1,6 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import {
   classifyWebAudioMediaRoute,
+  isRouteSelectionSettled,
   nativeUnexpressibleProcessing,
   reportWebAudioMediaRoute,
 } from "./webAudioRoute";
@@ -21,6 +22,18 @@ function withCurrentSrc(el: HTMLAudioElement, currentSrc: string): HTMLAudioElem
   return el;
 }
 
+/**
+ * Shadows the IDL `crossOrigin` accessor with a plain data property, so the
+ * value is visible ONLY via the property — unlike `el.crossOrigin = value`
+ * (which jsdom, like real browsers, reflects straight back to the
+ * `crossorigin` attribute), this simulates a host whose IDL property is
+ * genuinely decoupled from the attribute, per `hasCorsOptIn`'s secondary read.
+ */
+function withUnreflectedCrossOrigin(el: HTMLAudioElement, value: string): HTMLAudioElement {
+  Object.defineProperty(el, "crossOrigin", { value, configurable: true });
+  return el;
+}
+
 describe("classifyWebAudioMediaRoute", () => {
   it("routes same-origin media through Web Audio", () => {
     expect(classifyWebAudioMediaRoute(audio({ src: "/assets/vo.mp3" }))).toEqual({
@@ -97,6 +110,53 @@ describe("classifyWebAudioMediaRoute", () => {
   it("routes an element with no resolvable source through Web Audio", () => {
     expect(classifyWebAudioMediaRoute(audio())).toEqual({ kind: "web-audio" });
   });
+
+  it("treats an unreflected empty-string crossOrigin IDL property as opt-in", () => {
+    // The IDL fallback for `crossorigin=""` / bare `crossorigin` is the empty
+    // string. A host whose property setter doesn't reflect to the attribute
+    // (unlike jsdom's own accessor, which does) must not have that empty
+    // string misread as "no opt-in" — `Boolean("")` is false, which is
+    // exactly the fail-open this test guards against.
+    const el = withUnreflectedCrossOrigin(audio({ src: `${CROSS_ORIGIN}/track.mp3` }), "");
+    expect(el.getAttribute("crossorigin")).toBeNull(); // confirms it's genuinely unreflected
+
+    expect(classifyWebAudioMediaRoute(el)).toEqual({ kind: "web-audio" });
+  });
+
+  it("does not treat an untouched crossOrigin IDL property as opt-in", () => {
+    // The other direction of the same risk: a host must not default
+    // `crossOrigin` to a truthy/string value for elements that never opted
+    // in, or the cross-origin check would be permanently disabled.
+    const el = audio({ src: `${CROSS_ORIGIN}/track.mp3` });
+    expect(el.crossOrigin).toBeNull();
+
+    expect(classifyWebAudioMediaRoute(el)).toEqual({
+      kind: "decode-only",
+      reason: "cross_origin_no_cors",
+      asset: `${CROSS_ORIGIN}/track.mp3`,
+    });
+  });
+});
+
+describe("isRouteSelectionSettled", () => {
+  it("is unsettled for an element with only <source> children", () => {
+    const el = audio();
+    el.innerHTML = `<source src="/assets/first.mp3">`;
+    expect(isRouteSelectionSettled(el)).toBe(false);
+  });
+
+  it("is settled once a src attribute is definitive, even before load", () => {
+    expect(isRouteSelectionSettled(audio({ src: "/assets/vo.mp3" }))).toBe(true);
+  });
+
+  it("is settled once currentSrc has committed", () => {
+    const el = withCurrentSrc(audio(), `${SAME_ORIGIN}/assets/vo.mp3`);
+    expect(isRouteSelectionSettled(el)).toBe(true);
+  });
+
+  it("is unsettled for an element with no source at all", () => {
+    expect(isRouteSelectionSettled(audio())).toBe(false);
+  });
 });
 
 describe("nativeUnexpressibleProcessing", () => {

packages/core/src/runtime/webAudioRoute.ts

@@ -21,6 +21,19 @@ import type { RuntimeJson } from "./types";
  * a pure classifier, so the same verdict can be reached at media-discovery time
  * (to emit a diagnostic) and at schedule time (to actually withhold the node)
  * without those two ever drifting apart.
+ *
+ * That guarantee holds for every caller that routes through this classifier —
+ * it is NOT a runtime-wide interception of `createMediaElementSource`. The
+ * timeline transport (`webAudioTransport.ts`, via `init.ts`) always goes
+ * through it; a UI surface that builds its own throwaway `AudioContext` for
+ * an unrelated purpose (e.g. the asset sidebar's preview player,
+ * `AudioRow.tsx`) has to call it too, and is expected to. Known gap: an
+ * element playing a `MediaStream` via `srcObject` instead of `src`/`<source>`
+ * has no origin for this module to judge — `routeCandidates` only reads
+ * `src`-shaped attributes, so a `srcObject` element always reads as
+ * `web-audio` here, correctly or not. Nothing in this codebase feeds
+ * `createMediaElementSource` from a `srcObject` element today, so this is
+ * recorded as a boundary rather than fixed.
  */
 export type WebAudioMediaRoute =
   /** Same-origin, CORS-opted-in, or a scheme the check doesn't apply to. */
@@ -50,11 +63,23 @@ function hasAttr(el: HTMLMediaElement, name: string): boolean {
  * `anonymous`, so PRESENCE is the opt-in — `crossorigin=""` and even
  * `crossorigin="garbage"` both make the fetch a CORS request. Comparing the
  * value against `"anonymous"` would wrongly block those.
+ *
+ * Two independent reads, because a spec-faithful host and a permissive one
+ * disagree about where the truth lives:
+ *  - `getAttribute` is the primary read and covers every real browser: the
+ *    markup is unambiguous regardless of what the IDL getter does with it.
+ *  - `el.crossOrigin` is a secondary read for a host that sets the IDL
+ *    property without reflecting it back to the attribute — some
+ *    jsdom-style test/preview hosts do this. The check is `!= null`
+ *    (covers both `null` and `undefined`), not a truthiness check, ON
+ *    PURPOSE: `crossorigin=""` is a valid, common opt-in (see above), and
+ *    its IDL fallback value is the empty string — a falsy value that
+ *    `Boolean(el.crossOrigin)` would silently misread as "not opted in",
+ *    reintroducing the exact silent-audio bug this module exists to close.
  */
 function hasCorsOptIn(el: HTMLMediaElement): boolean {
   if (hasAttr(el, "crossorigin")) return true;
-  // Secondary read for a host that set the IDL property without reflecting it.
-  return typeof el.crossOrigin === "string";
+  return el.crossOrigin != null;
 }
 
 function baseUri(el: HTMLMediaElement): string {
@@ -108,6 +133,26 @@ function isCorsSilenced(rawUrl: string, el: HTMLMediaElement): boolean {
   return !hasCorsOptIn(el);
 }
 
+/**
+ * Whether resource selection has settled enough for a verdict to be a FACT
+ * rather than a guess. `currentSrc`/`src` are both definitive per the HTML
+ * resource-selection algorithm (see `routeCandidates` above); before either
+ * is set, a verdict can only be built from `<source>` children, any of which
+ * the browser may still pass over before committing.
+ *
+ * `classifyWebAudioMediaRoute` itself stays unsettled-tolerant on purpose —
+ * the schedule path needs *a* verdict even before selection settles, and
+ * conservatively withholding the node there costs nothing but a decode-only
+ * fallback. This predicate exists for the one caller that must NOT act on a
+ * guess: the discovery-time diagnostic, which drops a message in a human's
+ * lap and only gets to say it once (see `reportWebAudioMediaRoute`'s latch).
+ */
+export function isRouteSelectionSettled(el: HTMLMediaElement): boolean {
+  const current = typeof el.currentSrc === "string" ? el.currentSrc : "";
+  if (current) return true;
+  return hasAttr(el, "src");
+}
+
 /**
  * Pure — no node creation, no diagnostics, no element mutation. Called from
  * both the schedule path (where it withholds the node) and the discovery path
@@ -163,7 +208,17 @@ export function nativeUnexpressibleProcessing(el: HTMLMediaElement): string[] {
  * only `<audio>` ever reaches this module.
  */
 function isRenderMode(): boolean {
-  return typeof window !== "undefined" && !!window.__HF_EXPORT_RENDER_SEEK_CONFIG;
+  // Read through an inline cast rather than the ambient `Window` augmentation
+  // in `window.d.ts`: that augmentation is only in scope for programs that
+  // include it (core's own tsconfig does), and this module is also exported
+  // as `./runtime/web-audio-route` for non-runtime consumers (e.g. the studio
+  // asset sidebar's preview player, `AudioRow.tsx`) whose tsconfig doesn't
+  // pull it in. This is a plain existence check, so the cast costs nothing.
+  return (
+    typeof window !== "undefined" &&
+    !!(window as unknown as { __HF_EXPORT_RENDER_SEEK_CONFIG?: unknown })
+      .__HF_EXPORT_RENDER_SEEK_CONFIG
+  );
 }
 
 // One diagnostic per element. Latched only when something is actually emitted,

packages/studio/src/components/sidebar/AudioRow.tsx

@@ -1,4 +1,5 @@
 import { useState, useRef, useEffect, useCallback } from "react";
+import { classifyWebAudioMediaRoute } from "@hyperframes/core/runtime/web-audio-route";
 import { ContextMenu } from "./AssetContextMenu";
 import { basename, getAudioSubtype, type CopyFeedback } from "./assetHelpers";
 import { TIMELINE_ASSET_MIME } from "../../utils/timelineAssetDrop";
@@ -167,14 +168,24 @@ export function AudioRow({
         setPlaying(false);
         cancelAnimationFrame(animRef.current);
       };
+      // `src` must be set BEFORE classifying: the check reads `currentSrc`/
+      // `src`, and a same-origin `serveUrl` mustn't be judged from a blank
+      // element.
+      el.src = serveUrl;
       audioRef.current = el;
       const analyser = analyserRef.current;
-      if (analyser) {
+      // Same hazard `webAudioRoute.ts` documents for the timeline runtime
+      // (#3458): `createMediaElementSource` on a cross-origin element without
+      // a `crossorigin` opt-in permanently reroutes it to a node that outputs
+      // SILENCE per the Web Audio spec, without throwing. Classify first so
+      // this preview player can't reintroduce that bug — skipping the Web
+      // Audio graph here only costs the frequency-bar visualizer; native
+      // `<audio>` playback below stays audible either way.
+      if (analyser && classifyWebAudioMediaRoute(el).kind === "web-audio") {
         sourceRef.current = actxRef.current.createMediaElementSource(el);
         sourceRef.current.connect(analyser);
         analyser.connect(actxRef.current.destination);
       }
-      el.src = serveUrl;
     }
 
     if (actxRef.current.state === "suspended") await actxRef.current.resume();