mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(parsers,sdk,studio-server,studio): unify hf-id space across preview, disk, and SDK session (#1981)
* fix(parsers,sdk,studio-server,studio): unify hf-id space across preview, disk, and SDK session Root-causes the setTiming element_not_found resolver-shadow divergence class: timeline edits carry hf-ids read from the live preview DOM, but the preview minted ids AFTER rewriting attributes (and never persisted them for sub-comps), while the SDK session mints from the raw file — content-keyed minting then yields different ids for the same element. Template-based comps were worse: the SDK excluded the whole <template> subtree, so the session had zero elements and every edit diverged. - parsers: ensureHfIds now descends into <template> subtrees (linkedom's querySelectorAll does not), minting and pinning inner ids - sdk: buildRoots/buildElement treat <template> as a transparent container, and resolution (resolveScoped, animation-id map) searches template subtrees via querySelectorAllDeep — template comps now model, resolve, and edit - studio-server: the sub-comp preview route persists hf-ids to the raw file BEFORE the rewrite pipeline (mirrors the main route), pinning one id space across served DOM, disk, and SDK session - studio: resolver-shadow skips structurally-empty sessions (no event, no attempt) and tags fail-open emissions with sourceReadFailed so read errors are distinguishable from unwired readers in telemetry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(parsers,sdk,studio-server,studio): scope template descent, guard persist route Addresses the 10 verified findings from the PR #1981 review: - Restrict template transparency to COMPOSITION templates (<template data-composition-id>) everywhere — ensureHfIds, SDK buildChildren, querySelectorAllDeep. A plain <template> (runtime clone-source) keeps its old fully-excluded behavior: stamping its interior would duplicate one persisted id across every runtime clone, and modeling it would show phantom timeline clips. - Guard the sub-comp persist: only .html files (the wildcard route can serve any project path — stamping an SVG corrupted it on disk), try/catch the read (file-removed race becomes 404, not 500), salt the etag (v2) so pre-fix cached clients don't 304 past the id pin, and thread the stamped content into buildSubCompositionHtml so served ids match the mint even when the disk write is skipped. - Rewrite querySelectorAllDeep as a document-order DOM walk — appending template matches after top-level matches made duplicate-id tiebreaks disagree with the preview's unwrapped DOM (wrong-element edits). - Recurse sourceMutation.querySelectorAllWithTemplates so server-side ops resolve ids at any template depth, matching SDK resolution. - Replace the empty-session silent skip with ONE tagged session_empty event per session — silence would blind the tripwire to exactly the modeling-gap class that exposed the template bug. Attempts stay uncounted (an unmodelable comp can't cut over). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(studio-server): close TOCTOU in sub-comp hf-id persist (CodeQL js/file-system-race) Replace the route-level stat/read/persist sequence with stampFileHfIds: validation (fstat), read, mint, and write-back all go through ONE open file descriptor (O_NOFOLLOW where supported), so the path cannot be swapped between validation and write. Falls back to read-only stamping when the file isn't writable — content-keyed minting means the SDK derives the same ids from the same bytes even without the disk write. Addresses miguel-heygen's blocking review on PR #1981. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(studio-server): linear-time template-attr match (CodeQL js/polynomial-redos) promoteTemplateCompositionId's single-pattern regex backtracked polynomially on crafted input. Two-step match: grab each <template> open tag linearly, then find data-composition-id within that short tag text. Same semantics (first template carrying the attr wins). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
co-authored by
Claude Fable 5
Claude Fable 5
parent
e9076324e7
commit
3a717fa719
@@ -98,6 +98,23 @@ describe("A. Flag gating", () => {
|
||||
expect(trackedEvents.filter((e) => e.event === "sdk_resolver_shadow")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("A2c: empty session → ONE tagged session_empty event per session, no attempt", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
flushAttemptCounts(); // drain counts left by earlier tests
|
||||
const session = await openComposition("<!DOCTYPE html><html><body></body></html>");
|
||||
const ops: PatchOperation[] = [{ type: "inline-style", property: "color", value: "blue" }];
|
||||
runResolverShadow(session, "hf-anything", ops);
|
||||
runResolverShadow(session, "hf-other", ops); // repeat edits do not re-emit
|
||||
const events = trackedEvents.filter((e) => e.event === "sdk_resolver_shadow");
|
||||
// The modeling gap stays VISIBLE (silence would blind the tripwire to the
|
||||
// exact class that exposed the template-comp bug) but is distinguishable
|
||||
// and rate-limited to once per session instance.
|
||||
expect(events).toHaveLength(1);
|
||||
expect(events[0]?.props.sessionEmpty).toBe(true);
|
||||
expect(JSON.stringify(events[0]?.props.mismatches)).toContain("session_empty");
|
||||
expect(flushAttemptCounts()).toBeNull(); // can't cut over → not in the denominator
|
||||
});
|
||||
|
||||
it("A3: shadow depends ONLY on shadow flag, not on STUDIO_SDK_CUTOVER_ENABLED", async () => {
|
||||
// The mock always returns STUDIO_SDK_CUTOVER_ENABLED=false. Use a divergence
|
||||
// (poisoned session) so the flag-on case emits; flag-off must stay silent.
|
||||
@@ -268,7 +285,12 @@ describe("C. Resolver-parity detection", () => {
|
||||
|
||||
it("C8 sourceHfIdCount: emitted element_not_found carries source occurrence count", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
const session = { getElement: () => null, getElements: () => [] } as unknown as Composition;
|
||||
// One unrelated element so the session isn't "empty" (empty sessions are
|
||||
// skipped as structural modeling gaps) — it just can't resolve hf-dup.
|
||||
const session = {
|
||||
getElement: () => null,
|
||||
getElements: () => [{ id: "hf-other" }],
|
||||
} as unknown as Composition;
|
||||
// id present twice in source (duplicate-id ambiguity) but absent from session
|
||||
const source = `<div data-hf-id="hf-dup">a</div><div data-hf-id="hf-dup">b</div>`;
|
||||
runResolverShadow(
|
||||
@@ -282,7 +304,10 @@ describe("C. Resolver-parity detection", () => {
|
||||
|
||||
it("C8 sourceLooseMatchOnly: hfId matches source only as plain text, not a data-hf-id attribute", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
const session = { getElement: () => null, getElements: () => [] } as unknown as Composition;
|
||||
const session = {
|
||||
getElement: () => null,
|
||||
getElements: () => [{ id: "hf-other" }],
|
||||
} as unknown as Composition;
|
||||
// "hf-widget" appears only inside a class name, never as data-hf-id="hf-widget".
|
||||
const source = `<div class="hf-widget-container">no attribute match here</div>`;
|
||||
runResolverShadow(
|
||||
@@ -436,7 +461,7 @@ describe("F. recordResolverParity", () => {
|
||||
expect(ev?.sourceHfIdCount).toBeUndefined();
|
||||
});
|
||||
|
||||
it("fails open: a readSource error still emits (no suppression)", async () => {
|
||||
it("fails open: a readSource error still emits (no suppression), tagged sourceReadFailed", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
const session = await openComposition(BASE_HTML);
|
||||
await recordResolverParity(session, "hf-missing", "setTiming", () =>
|
||||
@@ -445,6 +470,31 @@ describe("F. recordResolverParity", () => {
|
||||
const ev = lastShadow();
|
||||
expect(ev?.mismatchCount).toBe(1);
|
||||
expect(ev?.sourceHfIdCount).toBeUndefined();
|
||||
// Distinguishes "reader threw" from "no reader wired" — every wild emission
|
||||
// of the setTiming class had an absent sourceHfIdCount and the two cases
|
||||
// were indistinguishable in telemetry.
|
||||
expect(ev?.sourceReadFailed).toBe(true);
|
||||
});
|
||||
|
||||
it("does not tag sourceReadFailed when no reader is supplied", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
const session = await openComposition(BASE_HTML);
|
||||
await recordResolverParity(session, "hf-missing", "setTiming");
|
||||
expect(lastShadow()?.sourceReadFailed).toBeUndefined();
|
||||
});
|
||||
|
||||
it("empty session → ONE tagged session_empty event, no attempt, no element_not_found", async () => {
|
||||
mockFlags.STUDIO_SDK_RESOLVER_SHADOW_ENABLED = true;
|
||||
flushAttemptCounts(); // drain any counts left by earlier tests
|
||||
const session = await openComposition("<!DOCTYPE html><html><body></body></html>");
|
||||
await recordResolverParity(session, "hf-anything", "setTiming");
|
||||
await recordResolverParity(session, "hf-other", "setTiming"); // no re-emit
|
||||
const events = trackedEvents.filter((e) => e.event === "sdk_resolver_shadow");
|
||||
expect(events).toHaveLength(1);
|
||||
expect(events[0]?.props.sessionEmpty).toBe(true);
|
||||
expect(JSON.stringify(events[0]?.props.mismatches)).toContain("session_empty");
|
||||
expect(JSON.stringify(events[0]?.props.mismatches)).not.toContain("element_not_found");
|
||||
expect(flushAttemptCounts()).toBeNull(); // can't cut over → not in the denominator
|
||||
});
|
||||
|
||||
it("tags sourceLooseMatchOnly when hfId matches source only as plain text, not a data-hf-id attribute", async () => {
|
||||
|
||||
@@ -24,7 +24,12 @@ import { trackStudioEvent, flushViaBeacon } from "./studioTelemetry";
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
export interface SdkResolverMismatch {
|
||||
kind: "element_not_found" | "value_mismatch" | "dispatch_error" | "animation_not_found";
|
||||
kind:
|
||||
| "element_not_found"
|
||||
| "value_mismatch"
|
||||
| "dispatch_error"
|
||||
| "animation_not_found"
|
||||
| "session_empty";
|
||||
hfId?: string;
|
||||
animationId?: string;
|
||||
property?: string;
|
||||
@@ -347,6 +352,34 @@ function redactMismatches(mismatches: SdkResolverMismatch[]): SdkResolverMismatc
|
||||
* (see below). The session is shared with the cutover path, so it MUST end the
|
||||
* call exactly as it started.
|
||||
*/
|
||||
// Sessions whose empty-session modeling gap has already been reported — one
|
||||
// event per session instance, not one per edit (the per-edit storm is noise;
|
||||
// the EXISTENCE of the gap is the signal).
|
||||
const emptySessionReported = new WeakSet<Composition>();
|
||||
|
||||
/**
|
||||
* An empty session structurally cannot resolve ANY id — a modeling gap (empty
|
||||
* file, comp shape the SDK can't parse into elements), not a resolver
|
||||
* divergence, and it can't cut over either, so it stays out of the attempt
|
||||
* denominator. But silence would blind the tripwire to exactly the class that
|
||||
* exposed the template-comp bug — so emit ONE distinguishable `session_empty`
|
||||
* event per session, then skip. Returns true when the caller should skip.
|
||||
*/
|
||||
function reportEmptySession(session: Composition, opLabel: string): boolean {
|
||||
if (session.getElements().length !== 0) return false;
|
||||
if (!emptySessionReported.has(session)) {
|
||||
emptySessionReported.add(session);
|
||||
trackStudioEvent("sdk_resolver_shadow", {
|
||||
opLabel,
|
||||
sessionEmpty: true,
|
||||
sessionElementCount: 0,
|
||||
mismatchCount: 1,
|
||||
mismatches: JSON.stringify([{ kind: "session_empty" } satisfies SdkResolverMismatch]),
|
||||
});
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function runResolverShadow(
|
||||
session: Composition,
|
||||
hfId: string | null | undefined,
|
||||
@@ -356,6 +389,7 @@ export function runResolverShadow(
|
||||
if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return;
|
||||
if (!hfId) return;
|
||||
try {
|
||||
if (reportEmptySession(session, "dom-edit")) return;
|
||||
recordAttempt("dom-edit");
|
||||
const mismatches = sdkResolverShadowCheck(session, hfId, ops, sourceContent);
|
||||
// Emit only on divergence — parity is silent, matching recordResolverParity
|
||||
@@ -409,6 +443,7 @@ export async function recordResolverParity(
|
||||
if (!STUDIO_SDK_RESOLVER_SHADOW_ENABLED) return;
|
||||
if (!session || !hfId) return;
|
||||
try {
|
||||
if (reportEmptySession(session, opLabel)) return;
|
||||
recordAttempt(opLabel);
|
||||
if (resolveSnapshot(session, hfId)) return; // resolves — parity, nothing to record
|
||||
// Capture BEFORE any await: this call is fire-and-forget (`void recordResolverParity(...)`)
|
||||
@@ -419,11 +454,13 @@ export async function recordResolverParity(
|
||||
const sessionElementCount = session.getElements().length;
|
||||
// Cheap check passed above, so the source read only runs on a real divergence.
|
||||
let source: string | undefined;
|
||||
let sourceReadFailed = false;
|
||||
if (readSource) {
|
||||
try {
|
||||
source = await readSource();
|
||||
} catch {
|
||||
source = undefined; // fail-open: a read error must not drop a real divergence
|
||||
sourceReadFailed = true;
|
||||
}
|
||||
}
|
||||
// Runtime-generated node the static parse can't model — suppress (mirrors the dom-edit path).
|
||||
@@ -444,6 +481,9 @@ export async function recordResolverParity(
|
||||
// Lets telemetry consumers filter this cohort without parsing the
|
||||
// sourceHfIdCount comment above.
|
||||
...(strictCount === 0 ? { sourceLooseMatchOnly: true } : {}),
|
||||
// The reader was wired but threw — distinguishes "read failed, emitted
|
||||
// fail-open without the suppression/count checks" from "no reader wired".
|
||||
...(sourceReadFailed ? { sourceReadFailed: true } : {}),
|
||||
mismatchCount: 1,
|
||||
mismatches: JSON.stringify([
|
||||
{ kind: "element_not_found", hfId } satisfies SdkResolverMismatch,
|
||||
|
||||
Reference in New Issue
Block a user