fix(sdk,core): css tokenizer, override-set replay, setattribute safety, persist errors (#1350)

* fix(sdk,core): css tokenizer, override-set replay, setattribute safety, persist errors

* test(sdk,ci): smoke test + explicit sdk-tests CI gate

Smoke test covers the full public surface:
  openComposition → setStyle/setText/dispatch(moveElement) → serialize
  applyPatches + ORIGIN_APPLY_PATCHES tagging
  batch() coalescing + transactional rollback on throw
  undo/redo round-trip
  persist adapter write + persist:error surfacing
  T3 embedded mode: override-set apply on open + getOverrides round-trip

Adds sdk-tests CI job so SDK coverage is explicitly named and required —
prevents a repeat of the demo-next vitest-never-ran incident.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sdk): export adapter types, awaitable flush(), never-coalesce mode

- Export PersistAdapter, PreviewAdapter, PersistVersionEntry from package
  root — callers can now write typed fakes without reaching into internals
- Add flush(): Promise<void> to Composition interface + CompositionImpl —
  app-close handlers can await a clean drain of the persist queue
- coalesceMs <= 0 disables coalescing entirely in createHistory — enables
  deterministic test scenarios without per-entry timestamp manipulation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(sdk): p2 edge cases — setText no-text-node, override-remove non-existent, flush in smoke

- setText on element with no prior text node (firstTextIdx=-1 path)
- applyOverrideSet null removal on non-existent prop is a no-op (no throw)
- smoke persist test uses comp.flush() instead of setTimeout
- can() JSDoc clarifies Phase 3b false-return is intentional feature-detection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: trigger regression suite

* fix(ci): add packages/sdk/package.json to Dockerfile.test workspace copy

bun install --frozen-lockfile fails in the regression Docker build because
the lockfile references the sdk workspace member but its package.json was
not copied into the image before the install step.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: vanceingalls

.github/workflows/ci.yml

@@ -218,6 +218,18 @@ jobs:
       - run: bun run --cwd packages/core build:hyperframes-runtime
       - run: bun run --filter '!@hyperframes/producer' test
 
+  sdk-tests:
+    name: "SDK: unit + contract + smoke"
+    needs: changes
+    if: needs.changes.outputs.code == 'true'
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+      - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
+      - run: bun install --frozen-lockfile
+      - run: bun run --filter @hyperframes/sdk test
+
   test-runtime-contract:
     name: "Test: runtime contract"
     needs: changes

Dockerfile.test

@@ -80,6 +80,7 @@ COPY packages/studio/package.json packages/studio/package.json
 COPY packages/shader-transitions/package.json packages/shader-transitions/package.json
 COPY packages/aws-lambda/package.json packages/aws-lambda/package.json
 COPY packages/gcp-cloud-run/package.json packages/gcp-cloud-run/package.json
+COPY packages/sdk/package.json packages/sdk/package.json
 RUN bun install --frozen-lockfile
 
 # Copy source

packages/core/src/studio-api/helpers/sourceMutation.ts

@@ -223,10 +223,35 @@ function isSafeAttributeValue(name: string, value: string): boolean {
   return true;
 }
 
+// fallow-ignore-next-line complexity
 function patchStyleAttrString(style: string, property: string, value: string | null): string {
   const props = new Map<string, string>();
   const order: string[] = [];
-  for (const decl of style.split(";")) {
+  // Tokenize declarations robustly: values can contain ';' inside quoted strings
+  // (e.g. content: ';') and ':' inside values (data URIs, url(), etc.).
+  // Split on ';' only when outside quotes and balanced parens; the first ':' in
+  // the resulting segment is the property/value separator (property names never
+  // contain ':').
+  let i = 0;
+  while (i < style.length) {
+    let depth = 0;
+    let inSingle = false;
+    let inDouble = false;
+    const start = i;
+    while (i < style.length) {
+      const ch = style[i];
+      if (ch === "'" && !inDouble) inSingle = !inSingle;
+      else if (ch === '"' && !inSingle) inDouble = !inDouble;
+      else if (!inSingle && !inDouble) {
+        if (ch === "(") depth++;
+        else if (ch === ")") depth = Math.max(0, depth - 1);
+        else if (ch === ";" && depth === 0) break;
+      }
+      i++;
+    }
+    const decl = style.slice(start, i).trim();
+    i++; // advance past ';'
+    if (!decl) continue;
     const colon = decl.indexOf(":");
     if (colon < 0) continue;
     const key = decl.slice(0, colon).trim();

packages/sdk/src/engine/apply-patches.ts

@@ -59,16 +59,17 @@ function parsePath(path: string): ParsedPath | null {
 
 /**
  * Replay a stored override-set onto a freshly-parsed base document (T3 init).
- * Property keys with null mean "restore base value" — a no-op on a fresh base.
- * Bare element keys with null are removal markers — the element is removed.
+ * A null value means the property was explicitly deleted — emit a remove patch
+ * so the base document matches the session state. (Removing a non-existent
+ * property is a no-op in applyOne, so this is safe against fresh-base misses.)
  */
 export function applyOverrideSet(parsed: ParsedDocument, overrides: OverrideSet): void {
   const patches: JsonPatchOp[] = [];
   for (const [key, value] of Object.entries(overrides)) {
     const path = keyToPath(key);
     if (!path) continue;
     if (value === null) {
-      if (!key.includes(".")) patches.push({ op: "remove", path });
+      patches.push({ op: "remove", path });
       continue;
     }
     patches.push({ op: "replace", path, value });

packages/sdk/src/engine/mutate.test.ts

@@ -140,6 +140,17 @@ describe("setText", () => {
     expect(serializeDocument(parsed)).toBe(before);
   });
 
+  it("creates text node when element has no existing text node", () => {
+    const parsed = parseMutable(
+      '<div data-hf-id="hf-s" data-hf-root><span data-hf-id="hf-empty"></span></div>',
+    );
+    const result = applyOp(parsed, { type: "setText", target: "hf-empty", value: "Added" });
+    const el = parsed.document.querySelector('[data-hf-id="hf-empty"]');
+    expect(el?.textContent).toBe("Added");
+    expect(result.forward[0]?.op).toBe("replace");
+    expect(result.forward[0]?.value).toBe("Added");
+  });
+
   it("override-set key maps correctly", () => {
     expect(pathToKey("/elements/hf-title/text")).toBe("hf-title.text");
   });

packages/sdk/src/engine/mutate.ts

@@ -40,19 +40,57 @@ export interface MutationResult {
 
 const EMPTY: MutationResult = { forward: [], inverse: [] };
 
-/** Ops that require the Phase 3b parser-backed engine (meriyah/css-tree). */
-const PHASE3B_OPS = new Set([
-  "setClassStyle",
-  "addGsapTween",
-  "setGsapTween",
-  "setGsapKeyframe",
-  "addGsapKeyframe",
-  "removeGsapKeyframe",
-  "removeGsapTween",
-  "addLabel",
-  "removeLabel",
+// ─── setAttribute safety ────────────────────────────────────────────────────
+
+// Composition-reserved attributes — changing these breaks element identity or
+// the core/studio data model. Reject before mutating.
+const RESERVED_ATTRS = new Set([
+  "data-hf-id",
+  "data-composition-id",
+  "data-width",
+  "data-height",
+  "data-start",
+  "data-end",
+  "data-track-index",
+  "data-hold-start",
+  "data-hold-end",
+  "data-hold-fill",
 ]);
 
+const DANGEROUS_URI_SCHEMES = /^(?:javascript|vbscript):/i;
+const DANGEROUS_DATA_URI = /^data\s*:\s*text\/html/i;
+const URI_BEARING_ATTRS = new Set([
+  "src",
+  "href",
+  "action",
+  "formaction",
+  "poster",
+  "srcset",
+  "xlink:href",
+]);
+
+function validateSetAttribute(name: string, value: string | null): void {
+  const lower = name.toLowerCase();
+  if (RESERVED_ATTRS.has(lower)) {
+    throw new Error(
+      `setAttribute: "${name}" is a reserved composition attribute and cannot be reassigned. ` +
+        `Use the appropriate typed method (setTiming, setHold, etc.) instead.`,
+    );
+  }
+  if (lower.startsWith("on")) {
+    throw new Error(
+      `setAttribute: event-handler attributes ("${name}") are not permitted — ` +
+        `they produce executable HTML that cannot be safely serialized.`,
+    );
+  }
+  if (value !== null && URI_BEARING_ATTRS.has(lower)) {
+    const trimmed = value.trim();
+    if (DANGEROUS_URI_SCHEMES.test(trimmed) || DANGEROUS_DATA_URI.test(trimmed)) {
+      throw new Error(`setAttribute: unsafe URI value for "${name}".`);
+    }
+  }
+}
+
 export class UnsupportedOpError extends Error {
   // Stable error code — part of the public API contract (F7); hosts switch on
   // err.code rather than the message.
@@ -169,7 +207,11 @@ function handleSetText(parsed: ParsedDocument, ids: HfId[], value: string): Muta
     const oldText = getOwnText(el);
     setOwnText(el, value);
     const path = textPath(id);
-    const p = scalarChange(path, oldText || null, value);
+    // getOwnText always returns string ("" for empty) — use it directly so
+    // the forward patch is always op:'replace', not op:'add'. An op:'add' on
+    // a text path is semantically wrong for external JSON-patch consumers
+    // (the path already exists; add would fail on strict appliers).
+    const p = scalarChange(path, oldText, value);
     result.forward.push(p.forward);
     result.inverse.push(p.inverse);
   }
@@ -182,6 +224,7 @@ function handleSetAttribute(
   name: string,
   value: string | null,
 ): MutationResult {
+  validateSetAttribute(name, value);
   const result: MutationResult = { forward: [], inverse: [] };
   for (const id of ids) {
     const el = findById(parsed.document, id);
@@ -400,9 +443,10 @@ export function validateOp(parsed: ParsedDocument, op: EditOp): boolean {
       return findRoot(parsed.document) !== null;
     case "setCompositionMetadata":
       return true;
-    // Phase 3b — not implemented yet; can() must report false so callers
-    // can feature-detect instead of hitting UnsupportedOpError.
+    // Phase 3b and unknown ops — report false so callers can feature-detect.
+    // An unknown op type must never silently pass validation only to no-op
+    // or throw in applyOp (which would violate the can() contract).
     default:
-      return !PHASE3B_OPS.has(op.type);
+      return false;
   }
 }

packages/sdk/src/engine/patches.ts

@@ -115,7 +115,9 @@ export function keyToPath(key: string): string | null {
   if (text?.[1]) return textPath(text[1]);
 
   const attr = /^([^.]+)\.attr\.(.+)$/.exec(key);
-  if (attr?.[1] && attr[2]) return attrPath(attr[1], attr[2]);
+  // pathToKey stores the RFC 6902-encoded segment verbatim; do NOT call attrPath()
+  // here (it would re-escape '~' → '~0'), just reconstruct the path directly.
+  if (attr?.[1] && attr[2]) return `/elements/${attr[1]}/attributes/${attr[2]}`;
 
   const timing = /^([^.]+)\.timing\.(start|end|trackIndex)$/.exec(key);
   if (timing?.[1]) return timingPath(timing[1], timing[2] as "start" | "end" | "trackIndex");

packages/sdk/src/history.ts

@@ -68,6 +68,7 @@ export function createHistory(session: Composition, opts: HistoryOptions = {}):
   }
 
   function shouldCoalesce(entry: HistoryEntry, incoming: PatchEvent): boolean {
+    if (coalesceMs <= 0) return false;
     if (opTypesKey(entry.opTypes) !== opTypesKey(incoming.opTypes)) return false;
     if (entry.origin !== incoming.origin) return false;
     // Coalesce only when the SAME paths are touched (e.g. slider drag on one

packages/sdk/src/index.ts

@@ -30,3 +30,5 @@ export type { HistoryModule, HistoryOptions, HistoryEntry } from "./history.js";
 
 export { createPersistQueue } from "./persist-queue.js";
 export type { PersistQueueModule, PersistQueueOptions } from "./persist-queue.js";
+
+export type { PersistAdapter, PreviewAdapter, PersistVersionEntry } from "./adapters/types.js";

packages/sdk/src/persist-queue.ts

@@ -8,7 +8,7 @@
  * T3 (embedded) hosts own persistence — do not use this module.
  */
 
-import type { Composition } from "./types.js";
+import type { Composition, PersistErrorEvent } from "./types.js";
 import type { PersistAdapter } from "./adapters/types.js";
 
 export interface PersistQueueModule {
@@ -20,6 +20,8 @@ export interface PersistQueueModule {
 export interface PersistQueueOptions {
   /** Adapter path to write to. Default: "composition.html" */
   path?: string;
+  /** Called when adapter.write() rejects. */
+  onError?: (e: PersistErrorEvent) => void;
 }
 
 export function createPersistQueue(
@@ -48,8 +50,9 @@ export function createPersistQueue(
       if (disposed) return;
       try {
         await adapter.write(path, content);
-      } catch {
-        // error already surfaced via persist:error on the adapter
+      } catch (err) {
+        const message = err instanceof Error ? err.message : String(err);
+        opts.onError?.({ error: { message, cause: err } });
       }
     });
     return writeChain;