From 6e32142334251d39a4b0c26ff26f3a52fbb59fa6 Mon Sep 17 00:00:00 2001
From: Vance Ingalls <vance@heygen.com>
Date: Tue, 16 Jun 2026 23:08:08 -0700
Subject: [PATCH] fix(sdk): resolve composition-id targets + emit canonical
 data-hf-id for GSAP tweens (#1526)

A sub-composition ROOT is addressed by its data-composition-id, but the SDK's
whole element<->tween attribution is data-hf-id based, so the prior fix's
[data-composition-id] selector was invisible to three readers (validateOp/can,
selectorMatchesId -> setTiming + removeElement cascade, buildAnimationIdMap ->
getElement.animationIds), diverging can from apply and orphaning tweens.

Root fix: make composition ids first-class resolvable addresses and emit the
canonical selector everywhere.

- resolveScoped (model.ts): for a bare id with no data-hf-id match, fall back to
  [data-composition-id]. data-hf-id keeps precedence; scoped-path and canonical
  behavior intact. Fixes validateOp gating, findById/getElement, and every op
  handler for comp-root targets in one place.
- gsapTargetSelector (mutate.ts): resolve the target and emit
  [data-hf-id="<resolved host hf-id>"] (canonical). Normal targets unchanged;
  comp-root targets resolve via comp-id -> host -> host hf-id. Defensive
  [data-composition-id] only when the resolved element has no hf-id.
- setTiming syncs the GSAP tween via the resolved element's data-hf-id so a
  comp-root target matches its host tween; removeElement cascade already covers
  the host hf-id via collectSubtreeHfIds.
- export escapeHfId; escape both the querySelector probe and the emitted
  selector string.

Tests: comp-id resolveScoped fallback + precedence (session.subcomp), canonical
selector, validateOp accept, setTiming sync, removeElement cascade, and
getElement.animationIds for comp-root tweens (mutate.gsap). The prior test only
called applyOp, masking all of this.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 packages/sdk/src/engine/model.ts            | 13 +++-
 packages/sdk/src/engine/mutate.gsap.test.ts | 82 +++++++++++++++++++++
 packages/sdk/src/engine/mutate.ts           | 34 ++++++++-
 packages/sdk/src/session.subcomp.test.ts    | 56 ++++++++++++++
 4 files changed, 180 insertions(+), 5 deletions(-)

diff --git a/packages/sdk/src/engine/model.ts b/packages/sdk/src/engine/model.ts
index 2efae69eb..6c61b9e41 100644
--- a/packages/sdk/src/engine/model.ts
+++ b/packages/sdk/src/engine/model.ts
@@ -37,7 +37,7 @@ export function findById(document: Document, id: string): Element | null {
   return resolveScoped(document, id);
 }
 
-function escapeHfId(id: string): string {
+export function escapeHfId(id: string): string {
   return id.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
 }
 
@@ -76,8 +76,15 @@ export function resolveScoped(document: Document, id: string): Element | null {
   if (parts.length === 1) {
     const escaped = escapeHfId(id);
     const matches = Array.from(document.querySelectorAll(`[data-hf-id="${escaped}"]`));
-    if (matches.length === 0) return null;
-    return matches.find((el) => isCanonicalScope(el)) ?? matches[0] ?? null;
+    if (matches.length > 0) {
+      return matches.find((el) => isCanonicalScope(el)) ?? matches[0] ?? null;
+    }
+    // Fall back to a sub-composition ROOT addressed by its composition id. A
+    // host element carries data-hf-id (its own leaf id) AND data-composition-id
+    // (the id studio passes when targeting the sub-comp root). data-hf-id takes
+    // precedence above; only when no hf-id matches do we treat the bare id as a
+    // composition id, making comp-ids first-class resolvable addresses.
+    return document.querySelector(`[data-composition-id="${escaped}"]`);
   }
 
   let context: Element | Document = document;
diff --git a/packages/sdk/src/engine/mutate.gsap.test.ts b/packages/sdk/src/engine/mutate.gsap.test.ts
index c752c8cbd..771eff236 100644
--- a/packages/sdk/src/engine/mutate.gsap.test.ts
+++ b/packages/sdk/src/engine/mutate.gsap.test.ts
@@ -32,6 +32,17 @@ function fresh(script = GSAP_SCRIPT) {
   return parseMutable(makeHtml(script));
 }
 
+// A sub-composition host: data-hf-id="hf-host" (its own leaf id) AND
+// data-composition-id="sub-1" (the id studio passes when targeting the root).
+function freshSubComp(script = GSAP_SCRIPT) {
+  return parseMutable(
+    `<div data-hf-id="hf-stage" data-hf-root style="width: 1280px; height: 720px">
+  <div data-hf-id="hf-host" data-composition-id="sub-1" style="opacity: 0"></div>
+  <script>${script}</script>
+</div>`.trim(),
+  );
+}
+
 function getScript(parsed: ReturnType<typeof parseMutable>): string {
   const doc = serializeDocument(parsed);
   const m = /<script>([\s\S]*?)<\/script>/i.exec(doc);
@@ -188,6 +199,77 @@ describe("addGsapTween", () => {
     });
     expect(result.forward).toHaveLength(0);
   });
+
+  // A normal data-hf-id target keeps the [data-hf-id] selector form.
+  it("emits a [data-hf-id] selector for a normal element target", () => {
+    const result = applyOp(fresh(), {
+      type: "addGsapTween",
+      target: "hf-box",
+      tween: { method: "to", properties: { x: 1 } },
+    });
+    const script = String(result.forward[0]?.value ?? "");
+    expect(script).toContain(`[data-hf-id=\\"hf-box\\"]`);
+    expect(script).not.toContain("data-composition-id");
+  });
+
+  // A sub-composition ROOT is addressed by its composition id, but the SDK's
+  // element↔tween attribution is data-hf-id based. So a comp-id target must
+  // resolve to the host element and emit the CANONICAL [data-hf-id="<host>"]
+  // form — NOT [data-composition-id] (invisible to selectorMatchesId / cascade /
+  // buildAnimationIdMap) and NOT [data-hf-id="<compId>"] (matches no element).
+  it("emits a canonical [data-hf-id] selector for a sub-composition root target", () => {
+    const result = applyOp(freshSubComp(), {
+      type: "addGsapTween",
+      target: "sub-1",
+      tween: { method: "to", properties: { x: 1 } },
+    });
+    const script = String(result.forward[0]?.value ?? "");
+    expect(script).toContain(`[data-hf-id=\\"hf-host\\"]`);
+    expect(script).not.toContain("data-composition-id");
+    expect(script).not.toContain(`[data-hf-id=\\"sub-1\\"]`);
+  });
+
+  // validateOp/can must accept a comp-root target (resolveScoped's comp-id
+  // fallback resolves it) — otherwise can/apply diverge.
+  it("validateOp accepts a sub-composition root target (no E_TARGET_NOT_FOUND)", () => {
+    const r = validateOp(freshSubComp(), {
+      type: "addGsapTween",
+      target: "sub-1",
+      tween: { method: "to", properties: { x: 1 } },
+    });
+    expect(r.ok).toBe(true);
+  });
+
+  // setTiming on the comp-root after adding a tween updates the tween's GSAP
+  // position/duration — selectorMatchesId matches the canonical host hf-id.
+  it("setTiming on a comp-root syncs its tween position/duration", () => {
+    // applyOp mutates parsed.document in place, so chain ops on the same parsed.
+    const parsed = freshSubComp();
+    applyOp(parsed, {
+      type: "addGsapTween",
+      target: "sub-1",
+      tween: { method: "to", duration: 0.5, properties: { x: 1 } },
+    });
+    applyOp(parsed, { type: "setTiming", target: "sub-1", start: 2, duration: 1.5 });
+    const script = getScript(parsed);
+    // The host tween's GSAP position (3rd arg) is now 2 and duration 1.5.
+    expect(script).toContain(`[data-hf-id=\\"hf-host\\"]`);
+    expect(script).toMatch(/duration:\s*1\.5/);
+    expect(script).toMatch(/\},\s*2\)/);
+  });
+
+  // removeElement on the comp-root cascade-removes its tween (not orphaned).
+  it("removeElement on a comp-root cascade-removes its tween", () => {
+    const parsed = freshSubComp();
+    applyOp(parsed, {
+      type: "addGsapTween",
+      target: "sub-1",
+      tween: { method: "to", properties: { x: 1 } },
+    });
+    expect(getScript(parsed)).toContain(`[data-hf-id=\\"hf-host\\"]`);
+    applyOp(parsed, { type: "removeElement", target: "sub-1" });
+    expect(getScript(parsed)).not.toContain(`[data-hf-id=\\"hf-host\\"]`);
+  });
 });
 
 // ─── Tween op test helpers ────────────────────────────────────────────────────
diff --git a/packages/sdk/src/engine/mutate.ts b/packages/sdk/src/engine/mutate.ts
index 5afa455e6..f21f6a1e6 100644
--- a/packages/sdk/src/engine/mutate.ts
+++ b/packages/sdk/src/engine/mutate.ts
@@ -11,6 +11,7 @@ import type { CanResult, EditOp, GsapTweenSpec, HfId, JsonPatchOp } from "../typ
 import type { ParsedDocument } from "./model.js";
 import {
   resolveScoped,
+  escapeHfId,
   findRoot,
   getElementStyles,
   setElementStyles,
@@ -351,9 +352,13 @@ function handleSetTiming(
     // Sync GSAP tween positions: the GSAP script is the source of truth at play time —
     // the timeline rebuilds from it on every seek. Without this, DOM attribute edits
     // have zero playback effect; the script's position/duration silently overrides them.
+    // Match against the resolved element's own data-hf-id (the canonical form
+    // tweens are stored under) so a comp-root target ("sub-1") whose tween lives
+    // at [data-hf-id="hf-host"] still syncs.
+    const matchId = el.getAttribute("data-hf-id") ?? id;
     if (parsedGsap && currentScript) {
       for (const { id: animId, animation } of parsedGsap.located) {
-        if (!selectorMatchesId(animation.targetSelector, id)) continue;
+        if (!selectorMatchesId(animation.targetSelector, matchId)) continue;
         const updates: Partial<GsapAnimation> = {};
         if (timing.start !== undefined && newStart !== null) updates.position = newStart;
         if (timing.duration !== undefined && newDuration !== null) updates.duration = newDuration;
@@ -587,6 +592,31 @@ function gsapScriptChange(oldScript: string, newScript: string): MutationResult
 
 // ─── Phase 3b handlers ───────────────────────────────────────────────────────
 
+// Build the GSAP target selector for an add op. The SDK's whole element↔tween
+// attribution is data-hf-id based (selectorMatchesId, cascadeRemoveAnimations,
+// buildAnimationIdMap), so ALWAYS emit the canonical [data-hf-id="…"] form.
+//
+// Resolve the target first: a normal element resolves to itself (hf-id ==
+// target). A sub-composition ROOT addressed by its composition id resolves —
+// via resolveScoped's comp-id fallback — to the host element, whose own
+// data-hf-id we then emit. The fidelity resolver unifies this with the server
+// writer's [data-composition-id="…"] form because both querySelector to the
+// same host node.
+function gsapTargetSelector(
+  document: Parameters<typeof resolveScoped>[0],
+  bareTarget: string,
+): string {
+  const el = resolveScoped(document, bareTarget);
+  if (!el) return `[data-hf-id="${escapeHfId(bareTarget)}"]`;
+  const hfId = el.getAttribute("data-hf-id");
+  if (hfId) return `[data-hf-id="${escapeHfId(hfId)}"]`;
+  // Resolved a sub-comp root that carries data-composition-id but no own
+  // data-hf-id (rare/defensive) — address it by its composition id.
+  const compId = el.getAttribute("data-composition-id");
+  if (compId) return `[data-composition-id="${escapeHfId(compId)}"]`;
+  return `[data-hf-id="${escapeHfId(bareTarget)}"]`;
+}
+
 // fallow-ignore-next-line complexity
 function handleAddGsapTween(
   parsed: ParsedDocument,
@@ -610,7 +640,7 @@ function handleAddGsapTween(
   // selector — only the leaf part is written as data-hf-id on the DOM element.
   const bareTarget = target.includes("/") ? (target.split("/").at(-1) ?? target) : target;
   const animation: Omit<GsapAnimation, "id"> = {
-    targetSelector: `[data-hf-id="${bareTarget}"]`,
+    targetSelector: gsapTargetSelector(parsed.document, bareTarget),
     method: tween.method,
     position: tween.position ?? 0,
     ...(tween.duration !== undefined ? { duration: tween.duration } : {}),
diff --git a/packages/sdk/src/session.subcomp.test.ts b/packages/sdk/src/session.subcomp.test.ts
index c52182de6..fede50e2b 100644
--- a/packages/sdk/src/session.subcomp.test.ts
+++ b/packages/sdk/src/session.subcomp.test.ts
@@ -50,6 +50,35 @@ describe("resolveScoped — flat id", () => {
     expect(resolveScoped(doc as unknown as Document, "hf-xxxx")).toBeNull();
   });
 
+  // A sub-composition ROOT is addressed by its composition id. When no element
+  // carries that as a data-hf-id, fall back to [data-composition-id]: comp-ids
+  // become first-class resolvable addresses (fixes validate / getElement).
+  it("resolves a bare id to a sub-comp root via data-composition-id fallback", () => {
+    const doc = makeDoc(
+      `<!DOCTYPE html><html><body><div data-hf-id="hf-host" data-composition-id="sub-1"></div></body></html>`,
+    ) as unknown as Document;
+    const viaComp = resolveScoped(doc, "sub-1");
+    const viaHf = resolveScoped(doc, "hf-host");
+    expect(viaComp).not.toBeNull();
+    expect(viaComp?.getAttribute("data-hf-id")).toBe("hf-host");
+    // Both addresses resolve to the SAME host element.
+    expect(viaComp).toBe(viaHf);
+  });
+
+  // data-hf-id MUST take precedence: a bare id that matches a real data-hf-id
+  // never falls back to data-composition-id, even if some other element carries
+  // that string as its composition id.
+  it("data-hf-id takes precedence over data-composition-id for a bare id", () => {
+    const doc = makeDoc(
+      `<!DOCTYPE html><html><body>
+        <div data-hf-id="dup" class="byHfId"></div>
+        <div data-hf-id="hf-host" data-composition-id="dup" class="byCompId"></div>
+      </body></html>`,
+    ) as unknown as Document;
+    const el = resolveScoped(doc, "dup");
+    expect(el?.getAttribute("class")).toBe("byHfId");
+  });
+
   // Regression: findById is the patch-replay/undo resolver. It must agree with
   // resolveScoped (forward dispatch) on an ambiguous bare id — both pick the
   // canonical (top-level) instance — or undo reverts the wrong duplicate.
@@ -305,6 +334,33 @@ describe("dispatch — scoped target", () => {
   });
 });
 
+// ─── 3b. Comp-root GSAP tween attribution ─────────────────────────────────────
+
+describe("sub-comp root GSAP tween — canonical hf-id attribution", () => {
+  it("getElement(host).animationIds includes a tween added by comp-id target", async () => {
+    const html = inlinedHtml(`
+      <div data-hf-id="hf-root" data-hf-root>
+        <div data-hf-id="hf-host" data-composition-id="sub-1" data-composition-file="sub.html">
+          <p data-hf-id="hf-leaf">text</p>
+        </div>
+        <script>var tl = gsap.timeline({ paused: true });
+window.__timelines = { t: tl };</script>
+      </div>
+    `);
+    const comp = await openComposition(html);
+    // Target the sub-comp ROOT by its composition id.
+    const animId = comp.addGsapTween("sub-1", {
+      method: "to",
+      duration: 0.3,
+      properties: { x: 200 },
+    });
+    // The tween is filed under the host's own data-hf-id (canonical form), so
+    // it surfaces on the host element snapshot.
+    const host = comp.getElement("hf-host");
+    expect(host?.animationIds).toContain(animId);
+  });
+});
+
 // ─── 4. Override-set keys for scoped ids ──────────────────────────────────────
 
 describe("override-set — scoped id keys", () => {