From c94de6034e3438090892e21eebf4c09be1ea3780 Mon Sep 17 00:00:00 2001
From: Vance Ingalls <vance@heygen.com>
Date: Thu, 9 Jul 2026 11:07:24 -0700
Subject: [PATCH] fix(sdk): address PR #2100 review feedback on attachSync
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Script-mirror filter changed from an exact "/script/gsap" match to
  path.startsWith("/script/") — the documented contract is "never mirror
  script-tag rewrites," not just today's one known path; startsWith covers
  any future script-kind patch under the same intent.
- _syncDetach is now cleared when the caller invokes the returned detach
  function directly, not only on the next attachSync call — avoids holding
  a stale (already-unsubscribed) reference between an explicit detach() and
  a later attachSync(other).
- The initial applyOverrideSet call is now wrapped in try/catch: a bad
  initial snapshot no longer prevents the ongoing patch subscription from
  attaching, matching the SDK's existing swallow-and-warn precedent for
  silent-failure paths (adapters/iframe.ts's tainted-canvas warning).
- Added a test proving declareVariable/removeVariable (the /variable-decls/
  patches PR #2098 introduces) mirror onto the live document's
  data-composition-variables attribute — the existing suite only covered
  setVariableValue's CSS-custom-property path, not the schema-metadata path.
---
 packages/sdk/src/adapters/iframe.sync.test.ts | 15 +++++++++++
 packages/sdk/src/adapters/iframe.ts           | 25 +++++++++++++++----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/packages/sdk/src/adapters/iframe.sync.test.ts b/packages/sdk/src/adapters/iframe.sync.test.ts
index 7b80e2105..cde99bcfd 100644
--- a/packages/sdk/src/adapters/iframe.sync.test.ts
+++ b/packages/sdk/src/adapters/iframe.sync.test.ts
@@ -219,6 +219,21 @@ window.__timelines = { t: tl };</script>
     expect(liveRoot.style.getPropertyValue("--accent")).toBe("#0f0");
   });
 
+  it("mirrors declareVariable/removeVariable onto the live document's schema attribute", async () => {
+    const iframe = mountIframe(BASE_HTML); // no data-composition-variables at all
+    const comp = await openComposition(BASE_HTML);
+    const adapter = createIframePreviewAdapter(iframe);
+    adapter.attachSync(comp);
+
+    comp.declareVariable({ id: "accent", type: "string", label: "Accent", default: "#fff" });
+
+    const liveDocEl = iframe.contentDocument!.documentElement;
+    expect(liveDocEl.getAttribute("data-composition-variables")).toContain("accent");
+
+    comp.removeVariable("accent");
+    expect(liveDocEl.getAttribute("data-composition-variables")).not.toContain("accent");
+  });
+
   it("mirrors setTiming onto the live element's data-start/data-end attributes", async () => {
     const iframe = mountIframe(BASE_HTML);
     const comp = await openComposition(BASE_HTML);
diff --git a/packages/sdk/src/adapters/iframe.ts b/packages/sdk/src/adapters/iframe.ts
index f5c1595f2..e83736b7c 100644
--- a/packages/sdk/src/adapters/iframe.ts
+++ b/packages/sdk/src/adapters/iframe.ts
@@ -754,20 +754,35 @@ class IframePreviewAdapter implements PreviewAdapter {
 
     const doc = this.iframe.contentDocument;
     if (doc) {
-      applyOverrideSet({ document: doc, wrapped: false, stamped: "" }, comp.getOverrides());
+      try {
+        applyOverrideSet({ document: doc, wrapped: false, stamped: "" }, comp.getOverrides());
+      } catch (err) {
+        // Don't let a bad initial snapshot prevent the ongoing subscription
+        // below from attaching — future patches should still mirror even if
+        // this composition's current overrides couldn't be applied.
+        console.warn("[hyperframes] attachSync: initial override sync failed:", err);
+      }
     }
 
-    const unsubscribe = comp.on("patch", ({ patches }) => {
+    const rawUnsubscribe = comp.on("patch", ({ patches }) => {
       const liveDoc = this.iframe.contentDocument;
       if (!liveDoc) return;
       applyPatchesToDocument(
         { document: liveDoc, wrapped: false, stamped: "" },
-        patches.filter((p) => p.path !== "/script/gsap"),
+        // "Never mirror script-tag rewrites" is the documented contract, not
+        // just today's one known path — startsWith so a future script kind
+        // (e.g. "/script/label") is covered by the same intent, not just an
+        // exact string this filter happens to know about today.
+        patches.filter((p) => !p.path.startsWith("/script/")),
       );
     });
 
-    this._syncDetach = unsubscribe;
-    return unsubscribe;
+    const detach = (): void => {
+      rawUnsubscribe();
+      if (this._syncDetach === detach) this._syncDetach = null;
+    };
+    this._syncDetach = detach;
+    return detach;
   }
 }