fix(studio): stop three crash-boundary trips in the editor

packages/studio/src/components/sidebar/LeftSidebar.storage.test.ts

@@ -0,0 +1,28 @@
+// @vitest-environment happy-dom
+
+import { afterEach, expect, it, vi } from "vitest";
+import { getPersistedTab } from "./LeftSidebar";
+
+vi.mock("../../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
+
+const originalDescriptor = Object.getOwnPropertyDescriptor(globalThis, "localStorage");
+
+afterEach(() => {
+  if (originalDescriptor) Object.defineProperty(globalThis, "localStorage", originalDescriptor);
+});
+
+it("falls back to the default tab when localStorage is blocked", () => {
+  // Chrome throws on the property read itself when site data is blocked for
+  // the document. This runs as a useState initializer, so throwing here takes
+  // Studio to the crash boundary.
+  Object.defineProperty(globalThis, "localStorage", {
+    configurable: true,
+    get() {
+      throw new Error(
+        "Failed to read the 'localStorage' property from 'Window': Access is denied for this document.",
+      );
+    },
+  });
+
+  expect(getPersistedTab()).toBe("compositions");
+});

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

@@ -10,6 +10,7 @@ import {
 import { CompositionsTab } from "./CompositionsTab";
 import { AssetsTab } from "./AssetsTab";
 import { trackStudioEvent } from "../../utils/studioTelemetry";
+import { safeLocalStorage } from "../../utils/safeStorage";
 import { BlocksTab, type BlockPreviewInfo } from "./BlocksTab";
 import { FileTree } from "../editor/FileTree";
 import { Tooltip } from "../ui";
@@ -23,8 +24,18 @@ export interface LeftSidebarHandle {
 
 const STORAGE_KEY = "hf-studio-sidebar-tab";
 
-function getPersistedTab(): SidebarTab {
-  const stored = localStorage.getItem(STORAGE_KEY);
+// Both the `localStorage` reference and `getItem` itself can throw when the
+// browsing context is partitioned or site data is blocked — the same case
+// telemetry/config.ts documents. This runs as a `useState` initializer, so an
+// unguarded throw here takes the whole editor to the crash boundary rather
+// than losing one remembered tab.
+export function getPersistedTab(): SidebarTab {
+  let stored: string | null = null;
+  try {
+    stored = safeLocalStorage()?.getItem(STORAGE_KEY) ?? null;
+  } catch {
+    /* storage unavailable — fall back to the default tab */
+  }
   if (stored === "assets") return "assets";
   if (stored === "code") return "code";
   if (stored === "blocks") return "blocks";
@@ -104,7 +115,11 @@ export const LeftSidebar = memo(
 
     const selectTab = useCallback((t: SidebarTab) => {
       setTab(t);
-      localStorage.setItem(STORAGE_KEY, t);
+      try {
+        safeLocalStorage()?.setItem(STORAGE_KEY, t);
+      } catch {
+        /* storage unavailable — the tab just won't be remembered */
+      }
       trackStudioEvent("tab_switch", { panel: "left_sidebar", tab: t });
     }, []);
 

packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts

@@ -1,12 +1,16 @@
-import { describe, it, expect, beforeEach } from "vitest";
+import { describe, it, expect, beforeEach, vi } from "vitest";
 import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
 import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore";
 import {
   clearKeyframeCacheForElement,
+  elementCacheKeys,
   pruneKeyframeCacheToFiles,
   replaceKeyframeCacheForFile,
   updateKeyframeCacheFromParsed,
 } from "./gsapKeyframeCacheHelpers";
+import { trackStudioEvent } from "../utils/studioTelemetry";
+
+vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
 
 const entry = (): KeyframeCacheEntry => ({
   format: "percentage",
@@ -30,6 +34,48 @@ const animWithKeyframes = (id: string): GsapAnimation => ({
 
 beforeEach(() => {
   usePlayerStore.setState({ keyframeCache: new Map(), gsapAnimations: new Map(), elements: [] });
+  vi.mocked(trackStudioEvent).mockClear();
+});
+
+describe("non-string cache keys", () => {
+  // `s.indexOf is not a function` in pruneKeyframeCacheToFiles, decoded from the
+  // released 0.7.90 bundle. Some producer reaches elementCacheKeys with a
+  // non-string id; the bare-id key was written through raw, so both maps ended
+  // up holding a key that prune's `key.indexOf("#")` cannot handle.
+  const badId = 42 as unknown as string;
+
+  it("keeps every written key a string", () => {
+    expect(elementCacheKeys("comp.html", badId).every((k) => typeof k === "string")).toBe(true);
+  });
+
+  it("reports the offending value instead of swallowing it", () => {
+    elementCacheKeys("comp.html", badId);
+
+    expect(trackStudioEvent).toHaveBeenCalledWith(
+      "cache_key_non_string",
+      expect.objectContaining({
+        value_type: "number",
+        constructor_name: "Number",
+        source_file: "comp.html",
+      }),
+    );
+  });
+
+  it("stays silent on the normal string path", () => {
+    elementCacheKeys("comp.html", "box");
+
+    expect(trackStudioEvent).not.toHaveBeenCalled();
+  });
+
+  it("survives a prune after a write with a non-string id", () => {
+    replaceKeyframeCacheForFile(
+      "stale.html",
+      new Map([[badId, entry()]]),
+      new Map([[badId, [animWithKeyframes("box")]]]),
+    );
+
+    expect(() => pruneKeyframeCacheToFiles(["kept.html"])).not.toThrow();
+  });
 });
 
 describe("clearKeyframeCacheForElement", () => {

packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts

@@ -4,6 +4,7 @@
  */
 import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
 import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore";
+import { trackStudioEvent } from "../utils/studioTelemetry";
 import { resolveClipTimingBasis, resolveSelectorElementIds, toClipKeyframes } from "./gsapShared";
 import {
   deduplicateKeyframes,
@@ -226,11 +227,38 @@ export function scopedElementKey(element: {
   return `${element.sourceFile || "index.html"}#${element.id}`;
 }
 
-/** Every cache key a write for this element sets, in read-preference order. */
+/**
+ * The one gate every cache write passes through, so it is also the one place
+ * that can guarantee `keyframeCache` / `gsapAnimations` really are keyed by
+ * string the way their types claim.
+ *
+ * Two of the three keys are template literals, which coerce on their own. The
+ * bare-id key was passed through raw, so a non-string `elementId` reaching here
+ * put a non-string key in both maps — and `pruneKeyframeCacheToFiles` then threw
+ * `s.indexOf is not a function` on it, taking Studio to the crash boundary.
+ *
+ * Which caller supplies a non-string id is still unknown: every writer traced
+ * from here produces a string. So this coerces rather than guesses, and reports
+ * the offending value's shape instead of swallowing it — the next occurrence
+ * names its own producer.
+ */
 export function elementCacheKeys(sourceFile: string, elementId: string): string[] {
+  const id = typeof elementId === "string" ? elementId : coerceCacheKeyId(elementId, sourceFile);
   return sourceFile === "index.html"
-    ? [`index.html#${elementId}`, elementId]
-    : [`${sourceFile}#${elementId}`, `index.html#${elementId}`, elementId];
+    ? [`index.html#${id}`, id]
+    : [`${sourceFile}#${id}`, `index.html#${id}`, id];
+}
+
+function coerceCacheKeyId(elementId: unknown, sourceFile: string): string {
+  trackStudioEvent("cache_key_non_string", {
+    value_type: typeof elementId,
+    // An Element lands here as "HTMLDivElement", a boxed id as "Number" — enough
+    // to name the producer without shipping user content to telemetry.
+    constructor_name: (elementId as { constructor?: { name?: string } })?.constructor?.name ?? null,
+    is_array: Array.isArray(elementId),
+    source_file: sourceFile,
+  });
+  return String(elementId);
 }
 
 /** Replace one file's complete cache snapshot with one atomic store publish. */

packages/studio/src/hooks/useGsapTweenCache.ts

@@ -4,6 +4,7 @@ import { usePlayerStore } from "../player/store/playerStore";
 import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeBridge";
 import {
   clearKeyframeCacheForElement,
+  elementCacheKeys,
   pruneKeyframeCacheToFiles,
   publishKeyframeCache,
   writeGsapAnimationsForElement,
@@ -302,8 +303,9 @@ export function useGsapAnimationsForElement(
       // scan already cached. Only clear when no source cached this element —
       // otherwise selecting it would wipe its diamonds.
       const { keyframeCache } = usePlayerStore.getState();
-      const hasCached =
-        keyframeCache.has(`${sourceFile}#${elementId}`) || keyframeCache.has(elementId);
+      const hasCached = elementCacheKeys(sourceFile, elementId).some((key) =>
+        keyframeCache.has(key),
+      );
       if (!hasCached) clearKeyframeCacheForElement(sourceFile, elementId);
       return;
     }
@@ -314,14 +316,16 @@ export function useGsapAnimationsForElement(
       ...(ease ? { ease } : {}),
       ...(easeEach ? { easeEach } : {}),
     };
-    // PropertyPanel reads the cache by bare elementId (without sourceFile
-    // prefix), so the same entry is written under the bare key for
-    // cross-component lookups. Both keys land in one publish: a reader that woke
-    // between two separate writes saw the prefixed key updated and the bare one
-    // still stale.
+    // elementCacheKeys owns the key-variant list every writer sets (prefixed,
+    // index.html fallback, bare id). Building it by hand here is what let this
+    // site drift: it omitted the fallback key, and it wrote the bare id without
+    // the string coercion that keeps prune from throwing on a non-string. All
+    // keys land in one publish: a reader that woke between two separate writes
+    // saw the prefixed key updated and the bare one still stale.
     publishKeyframeCache((draft) => {
-      draft.keyframeCache.set(`${sourceFile}#${elementId}`, merged);
-      draft.keyframeCache.set(elementId, merged);
+      for (const key of elementCacheKeys(sourceFile, elementId)) {
+        draft.keyframeCache.set(key, merged);
+      }
     });
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [elementId, sourceFile, animations, domClipChildrenKey]);
@@ -426,13 +430,8 @@ export function usePopulateKeyframeCacheForFile(
       // in between re-rendered against a cache only partly filled in.
       publishKeyframeCache((draft) => {
         for (const [id, data] of scanned) {
-          const cacheKey = `${sf}#${id}`;
-          const fallbackKey = `index.html#${id}`;
-          const alreadyCached =
-            draft.keyframeCache.has(cacheKey) ||
-            draft.keyframeCache.has(fallbackKey) ||
-            draft.keyframeCache.has(id);
-          if (alreadyCached) continue;
+          const keys = elementCacheKeys(sf, id);
+          if (keys.some((key) => draft.keyframeCache.has(key))) continue;
           // Skip position-only set tweens from runtime too, same filter as AST path
           const isPosOnly =
             data.keyframes.length === 1 &&
@@ -445,9 +444,7 @@ export function usePopulateKeyframeCacheForFile(
             keyframes: data.keyframes,
             ...(data.easeEach ? { easeEach: data.easeEach } : {}),
           };
-          draft.keyframeCache.set(cacheKey, entry);
-          if (sf !== "index.html") draft.keyframeCache.set(fallbackKey, entry);
-          draft.keyframeCache.set(id, entry);
+          for (const key of keys) draft.keyframeCache.set(key, entry);
         }
       });
       runtimeScanDoneRef.current = `kf-cache:${projectId}:${sf}:${version}`;

packages/studio/src/player/components/Player.test.ts

@@ -100,6 +100,19 @@ describe("preview errors", () => {
     );
   });
 
+  it("unmounts cleanly when the player element is already detached", async () => {
+    const { player } = await mountPlayer();
+
+    // A container re-render, a crossfade swap, or a page-translation extension
+    // can detach the element before React tears the Player down. Cleanup must
+    // not throw NotFoundError — the error boundary turns that into a
+    // full-screen "Something went wrong".
+    player.remove();
+
+    expect(() => act(() => root?.unmount())).not.toThrow();
+    root = null;
+  });
+
   it("attaches lifecycle listeners before navigating the player", async () => {
     await mountPlayer();
     const srcIndex = lifecycleLog.indexOf("src");

packages/studio/src/player/components/Player.tsx

@@ -295,7 +295,13 @@ export const Player = forwardRef<HTMLIFrameElement, PlayerProps>(
           player.removeEventListener("error", handleError);
           if (assetPollRef.current) clearInterval(assetPollRef.current);
           assetPollRef.current = null;
-          container.removeChild(player);
+          // `remove()` rather than `container.removeChild(player)`: by the time
+          // this cleanup runs the element may already be detached — React can
+          // re-render the container, a crossfade refresh can swap it, or a
+          // translation/extension can reparent it. `removeChild` then throws
+          // NotFoundError, which the error boundary turns into a full-screen
+          // "Something went wrong". `remove()` is a no-op when already detached.
+          player.remove();
           if (retryPreviewRef.current === retryPreview) retryPreviewRef.current = null;
           // Clear the forwarded ref only if it still points to THIS iframe.
           // During crossfade refreshes the retiring Player unmounts after the

packages/studio/src/utils/clipboard.ts

@@ -26,7 +26,7 @@ function copyWithSelection(text: string): boolean {
   } catch {
     return false;
   } finally {
-    document.body.removeChild(textarea);
+    textarea.remove();
   }
 }