From 0a1583d44e3073f943134eefa9e06ad3f5993cc2 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 7 Jun 2026 20:09:21 -0700 Subject: [PATCH 01/10] feat(core): clip-model hf- ids minted at parse, emitted as data-hf-id (R1) --- packages/core/src/generators/hyperframes.ts | 1 + packages/core/src/parsers/htmlParser.ts | 7 +++++-- packages/core/src/parsers/stableIds.test.ts | 12 ++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/core/src/generators/hyperframes.ts b/packages/core/src/generators/hyperframes.ts index 68307921b..9028013c1 100644 --- a/packages/core/src/generators/hyperframes.ts +++ b/packages/core/src/generators/hyperframes.ts @@ -447,6 +447,7 @@ function generateZoomGsapAnimations( function generateElementHtml(element: TimelineElement, keyframes?: Keyframe[]): string { const baseAttrs = [ `id="${element.id}"`, + `data-hf-id="${element.id}"`, `data-start="${element.startTime}"`, `data-end="${element.startTime + element.duration}"`, `data-layer="${element.zIndex}"`, diff --git a/packages/core/src/parsers/htmlParser.ts b/packages/core/src/parsers/htmlParser.ts index 92864e04e..d0aac41cf 100644 --- a/packages/core/src/parsers/htmlParser.ts +++ b/packages/core/src/parsers/htmlParser.ts @@ -11,6 +11,7 @@ import type { CompositionVariable, } from "../core.types"; import { validateCompositionGsap } from "./gsapSerialize"; +import { ensureHfIds } from "./hfIds.js"; import type { ValidationResult } from "../core.types"; const MEDIA_TYPES = new Set(["video", "image", "audio"]); @@ -156,8 +157,9 @@ function resolveResolutionFromDimensions(width: number, height: number): CanvasR } export function parseHtml(html: string): ParsedHtml { + const withIds = ensureHfIds(html); const parser = new DOMParser(); - const doc = parser.parseFromString(html, "text/html"); + const doc = parser.parseFromString(withIds, "text/html"); const elements: TimelineElement[] = []; const keyframes: Record = {}; @@ -190,7 +192,8 @@ export function parseHtml(html: string): ParsedHtml { duration = 5; } - const id = el.id || `element-${++idCounter}`; + // R1: stable hf- id minted by ensureHfIds above; clips just read it. + const id = el.getAttribute("data-hf-id") || el.id || `element-${++idCounter}`; const name = getElementName(el); const zIndex = getZIndex(el); diff --git a/packages/core/src/parsers/stableIds.test.ts b/packages/core/src/parsers/stableIds.test.ts index a3887f04f..4656f98c8 100644 --- a/packages/core/src/parsers/stableIds.test.ts +++ b/packages/core/src/parsers/stableIds.test.ts @@ -18,7 +18,7 @@ import { serialize } from "./test-utils.js"; describe("T2 — stable element ids (spec for R1)", () => { // --- Spec (red until R1) --- - it.fails("[spec] elements without an id get a hf- prefixed id at parse", () => { + it("[spec] elements without an id get a hf- prefixed id at parse", () => { const html = `
Text
@@ -29,7 +29,7 @@ describe("T2 — stable element ids (spec for R1)", () => { } }); - it.fails("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => { + it("[spec] generated hf- ids match /^hf-[a-z0-9]{4}$/", () => { const html = `
X
@@ -41,7 +41,7 @@ describe("T2 — stable element ids (spec for R1)", () => { } }); - it.fails("[spec] adding an element before existing ones does not change existing ids", () => { + it("[spec] adding an element before existing ones does not change existing ids", () => { const base = `
A
B
@@ -62,12 +62,12 @@ describe("T2 — stable element ids (spec for R1)", () => { // --- Baseline (already pass, must not regress) --- - it("elements with an existing id keep it unchanged", () => { + it("existing data-hf-id is pinned and becomes the clip id (never re-minted)", () => { const html = `
-
Hi
+
Hi
`; const { elements } = parseHtml(html); - expect(elements.some((e) => e.id === "my-title")).toBe(true); + expect(elements.some((e) => e.id === "hf-anch")).toBe(true); }); it("ids are deterministic: same input produces same ids on re-parse", () => { From 2d1d724746ed998bb7eac16cd1eea9472777fc27 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 15:25:51 -0700 Subject: [PATCH 02/10] docs(core): document legacy-id round-trip in clip-model readback (R1 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Rames' review on #1270: clarifies that a pre-R1 clip authored with id="my-title" round-trips as data-hf-id="my-title" (non-hf-shaped but stable, exact-match) by design — targeting uses exact [data-hf-id="…"] match and does not require the hf- shape; legacy values re-mint only at the R7 write-back. Not a bug. Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/parsers/htmlParser.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/core/src/parsers/htmlParser.ts b/packages/core/src/parsers/htmlParser.ts index d0aac41cf..fd4c912cf 100644 --- a/packages/core/src/parsers/htmlParser.ts +++ b/packages/core/src/parsers/htmlParser.ts @@ -193,6 +193,13 @@ export function parseHtml(html: string): ParsedHtml { } // R1: stable hf- id minted by ensureHfIds above; clips just read it. + // Legacy/migration note: ensureHfIds pins a pre-existing `data-hf-id`, and + // the generator emits `data-hf-id="${element.id}"`. So a clip authored + // before R1 with `id="my-title"` round-trips as `data-hf-id="my-title"` — + // a non-`hf-`-shaped but still stable, exact-match handle. This is the + // intended migration: targeting uses exact `[data-hf-id="…"]` match (it does + // not require the hf- shape), and legacy values are re-minted only once the + // R7 write-back persists freshly-minted ids to source. Not a bug. const id = el.getAttribute("data-hf-id") || el.id || `element-${++idCounter}`; const name = getElementName(el); const zIndex = getZIndex(el); From 9ddd1454794c2285dd7ce7459c0b35793539412d Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 23:05:44 -0700 Subject: [PATCH 03/10] docs(core): fix misleading legacy-id migration comment in htmlParser.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original comment said legacy data-hf-id values "are re-minted only once the R7 write-back persists freshly-minted ids to source" — which is incorrect. ensureHfIds skips elements that already carry data-hf-id, so legacy values (e.g. data-hf-id="my-title") persist indefinitely and are NOT automatically re-minted. Exact-match targeting still works correctly. Update comment to reflect actual behaviour. Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/parsers/htmlParser.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/parsers/htmlParser.ts b/packages/core/src/parsers/htmlParser.ts index fd4c912cf..47859badf 100644 --- a/packages/core/src/parsers/htmlParser.ts +++ b/packages/core/src/parsers/htmlParser.ts @@ -196,10 +196,11 @@ export function parseHtml(html: string): ParsedHtml { // Legacy/migration note: ensureHfIds pins a pre-existing `data-hf-id`, and // the generator emits `data-hf-id="${element.id}"`. So a clip authored // before R1 with `id="my-title"` round-trips as `data-hf-id="my-title"` — - // a non-`hf-`-shaped but still stable, exact-match handle. This is the - // intended migration: targeting uses exact `[data-hf-id="…"]` match (it does - // not require the hf- shape), and legacy values are re-minted only once the - // R7 write-back persists freshly-minted ids to source. Not a bug. + // a non-`hf-`-shaped but still stable, exact-match handle. This is safe + // indefinitely: targeting uses exact `[data-hf-id="…"]` match (it does not + // require the hf- prefix). ensureHfIds skips elements that already carry + // data-hf-id, so legacy values are NOT re-minted automatically — they + // persist until the user re-saves the composition through Studio. Not a bug. const id = el.getAttribute("data-hf-id") || el.id || `element-${++idCounter}`; const name = getElementName(el); const zIndex = getZIndex(el); From a8e05a66422852f94be59887d6e1db583f0e6082 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 7 Jun 2026 20:49:10 -0700 Subject: [PATCH 04/10] feat(studio): sourcePatcher data-hf-id targeting (R1, T3) --- .../studio/src/utils/sourcePatcher.test.ts | 71 ++++++++++++++-- packages/studio/src/utils/sourcePatcher.ts | 80 +++++++++---------- 2 files changed, 102 insertions(+), 49 deletions(-) diff --git a/packages/studio/src/utils/sourcePatcher.test.ts b/packages/studio/src/utils/sourcePatcher.test.ts index 7697d24e6..67c01b3a7 100644 --- a/packages/studio/src/utils/sourcePatcher.test.ts +++ b/packages/studio/src/utils/sourcePatcher.test.ts @@ -517,17 +517,72 @@ describe("motion attribute round-trip via sourcePatcher", () => { }); }); -// T3 — id-based targeting (spec for R1). -// R1 adds `hfId?: string` to PatchTarget and a `[data-hf-id="…"]` lookup branch -// in findTagByTarget. Convert from it.todo to real assertions in the R1 PR. +// T3 — id-based targeting (R1). describe("T3 — hfId targeting (spec for R1)", () => { - it.todo("updates inline style by data-hf-id"); + it("updates inline style by data-hf-id", () => { + const html = `

Hello

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-x7k2" }, + { + type: "inline-style", + property: "color", + value: "blue", + }, + ); + expect(result).toContain("color: blue"); + expect(result).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("updates text content by data-hf-id"); + it("updates text content by data-hf-id", () => { + const html = `

Old text

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-a1b2" }, + { + type: "text-content", + property: "", + value: "New text", + }, + ); + expect(result).toContain(">New text<"); + }); - it.todo("updates attribute by data-hf-id"); + it("updates attribute by data-hf-id", () => { + const html = `
`; + const result = applyPatchByTarget( + html, + { hfId: "hf-c3d4" }, + { + type: "attribute", + property: "start", + value: "2.5", + }, + ); + expect(result).toContain('data-start="2.5"'); + }); - it.todo("data-hf-id attribute is preserved after a style patch"); + it("data-hf-id attribute is preserved after a style patch", () => { + const html = `

Hello

`; + const patched = applyPatchByTarget( + html, + { hfId: "hf-x7k2" }, + { + type: "inline-style", + property: "color", + value: "blue", + }, + ); + expect(readAttributeByTarget(patched, { hfId: "hf-x7k2" }, "data-hf-id")).toBe("hf-x7k2"); + }); - it.todo("hfId lookup falls through to selector when hfId not found"); + it("hfId lookup falls through to selector when hfId not found", () => { + const html = `

Hello

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-missing", selector: ".headline" }, + { type: "inline-style", property: "color", value: "blue" }, + ); + expect(result).toContain("color: blue"); + }); }); diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index 2d89eea2e..e081622e7 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -94,6 +94,7 @@ export interface PatchOperation { export interface PatchTarget { id?: string | null; + hfId?: string; selector?: string; selectorIndex?: number; } @@ -232,61 +233,58 @@ function replaceTagAtMatch(html: string, match: TagMatch, newTag: string): strin return `${html.slice(0, match.start)}${newTag}${html.slice(match.end)}`; } -export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null { - if (target.id) { - const idPattern = new RegExp(`(<[^>]*\\bid=(["'])${escapeRegex(target.id)}\\2[^>]*)>`, "i"); - const match = idPattern.exec(html); - if (match?.index != null) { +function execDataAttrPattern(html: string, attr: string, value: string): TagMatch | null { + const pattern = new RegExp(`(<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\2[^>]*)>`, "i"); + const match = pattern.exec(html); + return match?.index != null + ? { tag: match[1], start: match.index, end: match.index + match[1].length } + : null; +} + +function findTagByClass(html: string, target: PatchTarget): TagMatch | null { + const classMatch = target.selector?.match(/^\.([a-zA-Z0-9_-]+)$/); + if (!classMatch) return null; + const cls = classMatch[1]; + const pattern = new RegExp( + `(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`, + "gi", + ); + const selectorIndex = target.selectorIndex ?? 0; + let match: RegExpExecArray | null; + let currentIndex = 0; + while ((match = pattern.exec(html)) !== null) { + if (currentIndex === selectorIndex && match.index != null) { return { tag: match[1], start: match.index, end: match.index + match[1].length, }; } + currentIndex += 1; + } + return null; +} + +export function findTagByTarget(html: string, target: PatchTarget): TagMatch | null { + if (target.hfId) { + const result = execDataAttrPattern(html, "data-hf-id", target.hfId); + if (result) return result; + } + + if (target.id) { + const result = execDataAttrPattern(html, "id", target.id); + if (result) return result; } if (!target.selector) return null; const compositionIdMatch = target.selector.match(/^\[data-composition-id="([^"]+)"\]$/); if (compositionIdMatch) { - const compId = compositionIdMatch[1]; - const pattern = new RegExp( - `(<[^>]*\\bdata-composition-id=(["'])${escapeRegex(compId)}\\2[^>]*)>`, - "i", - ); - const match = pattern.exec(html); - if (match?.index != null) { - return { - tag: match[1], - start: match.index, - end: match.index + match[1].length, - }; - } + const result = execDataAttrPattern(html, "data-composition-id", compositionIdMatch[1]); + if (result) return result; } - const classMatch = target.selector.match(/^\.([a-zA-Z0-9_-]+)$/); - if (classMatch) { - const cls = classMatch[1]; - const pattern = new RegExp( - `(<[^>]*\\bclass=(["'])[^"']*\\b${escapeRegex(cls)}\\b[^"']*\\2[^>]*)>`, - "gi", - ); - const selectorIndex = target.selectorIndex ?? 0; - let match: RegExpExecArray | null; - let currentIndex = 0; - while ((match = pattern.exec(html)) !== null) { - if (currentIndex === selectorIndex && match.index != null) { - return { - tag: match[1], - start: match.index, - end: match.index + match[1].length, - }; - } - currentIndex += 1; - } - } - - return null; + return findTagByClass(html, target); } export function readAttributeByTarget( From d423170bd56c90206b7aed49ca59563c3c56e99a Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 15:20:08 -0700 Subject: [PATCH 05/10] fix(studio): warn on duplicate match in execDataAttrPattern (R1, T3 review) Addresses Rames' review on #1271: execDataAttrPattern returned the first regex match without checking for a second. A duplicate id/data-hf-id in source (id drift) would silently patch one element and leave the other stale. Now warns when more than one element matches. By the mint contract it should never fire. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/studio/src/utils/sourcePatcher.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/studio/src/utils/sourcePatcher.ts b/packages/studio/src/utils/sourcePatcher.ts index e081622e7..9d19114d4 100644 --- a/packages/studio/src/utils/sourcePatcher.ts +++ b/packages/studio/src/utils/sourcePatcher.ts @@ -236,9 +236,18 @@ function replaceTagAtMatch(html: string, match: TagMatch, newTag: string): strin function execDataAttrPattern(html: string, attr: string, value: string): TagMatch | null { const pattern = new RegExp(`(<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\2[^>]*)>`, "i"); const match = pattern.exec(html); - return match?.index != null - ? { tag: match[1], start: match.index, end: match.index + match[1].length } - : null; + if (match?.index == null) return null; + // Defensive: a second exact match means a duplicate id/attr in the source + // (id drift). Don't silently patch the first while leaving the other stale — + // surface it. By the mint contract this should never fire. + const all = html.match(new RegExp(`<[^>]*\\b${attr}=(["'])${escapeRegex(value)}\\1[^>]*>`, "gi")); + if (all && all.length > 1) { + // eslint-disable-next-line no-console + console.warn( + `sourcePatcher: ${attr}="${value}" matched ${all.length} elements; patching the first. ids/attrs must be unique per document.`, + ); + } + return { tag: match[1], start: match.index, end: match.index + match[1].length }; } function findTagByClass(html: string, target: PatchTarget): TagMatch | null { From dfee19642af48484ee262ad698f67177af01328a Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 23:06:51 -0700 Subject: [PATCH 06/10] test(studio): pin hfId-is-authoritative-over-selector contract (R1, T3 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test: "hfId match is authoritative — selector is not used as a narrowing filter". When hfId matches element A and selector points at element B, findTagByTarget returns A without consulting selector as a narrowing filter. Pins the intended behaviour so a future refactor cannot silently start narrowing by selector. Co-Authored-By: Claude Sonnet 4.6 --- packages/studio/src/utils/sourcePatcher.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/studio/src/utils/sourcePatcher.test.ts b/packages/studio/src/utils/sourcePatcher.test.ts index 67c01b3a7..ee4ab514c 100644 --- a/packages/studio/src/utils/sourcePatcher.test.ts +++ b/packages/studio/src/utils/sourcePatcher.test.ts @@ -585,4 +585,20 @@ describe("T3 — hfId targeting (spec for R1)", () => { ); expect(result).toContain("color: blue"); }); + + it("hfId match is authoritative — selector is not used as a narrowing filter", () => { + // hfId matches h1; selector points at h2. hfId wins — patch lands on h1, h2 untouched. + const html = `

A

B

`; + const result = applyPatchByTarget( + html, + { hfId: "hf-x7k2", selector: ".b" }, + { type: "inline-style", property: "color", value: "blue" }, + ); + expect(result).toContain('data-hf-id="hf-x7k2"'); + const h1End = result.indexOf(""); + const bluePos = result.indexOf("color: blue"); + expect(bluePos).toBeGreaterThan(-1); + expect(bluePos).toBeLessThan(h1End); + expect(result).toContain('

B

'); + }); }); From 227b7e9725007add02af9d6450126c3e8636c6cb Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 7 Jun 2026 20:51:42 -0700 Subject: [PATCH 07/10] feat(core): sourceMutation data-hf-id targeting (R1, T7) --- .../studio-api/helpers/sourceMutation.test.ts | 47 +++++++++++++++++-- .../src/studio-api/helpers/sourceMutation.ts | 8 +++- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.test.ts b/packages/core/src/studio-api/helpers/sourceMutation.test.ts index c23b45911..d3b4552d0 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.test.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.test.ts @@ -368,13 +368,50 @@ describe("probeElementInSource", () => { // Covers the same surface as T3 (Studio sourcePatcher) — Core sourceMutation supports // all patch types (inline-style, attribute, text-content) via patchElementInHtml. describe("T7 — data-hf-id targeting (spec for R1)", () => { - it.todo("updates inline style by data-hf-id when no HTML id attribute is present"); + it("updates inline style by data-hf-id when no HTML id attribute is present", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-x7k2" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("updates text content by data-hf-id"); + it("updates text content by data-hf-id", () => { + const source = `

Old text

`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-a1b2" }, [ + { type: "text-content", property: "", value: "New text" }, + ]); + expect(matched).toBe(true); + expect(html).toContain("New text"); + }); - it.todo("updates attribute by data-hf-id"); + it("updates attribute by data-hf-id", () => { + const source = `
`; + const { html, matched } = patchElementInHtml(source, { hfId: "hf-c3d4" }, [ + { type: "attribute", property: "start", value: "2.5" }, + ]); + expect(matched).toBe(true); + expect(html).toContain('data-start="2.5"'); + }); - it.todo("data-hf-id attribute survives the patch (can be targeted again)"); + it("data-hf-id attribute survives the patch (can be targeted again)", () => { + const source = `

Hello

`; + const { html } = patchElementInHtml(source, { hfId: "hf-x7k2" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); - it.todo("hfId lookup falls through to selector when hfId is not found in the document"); + it("hfId lookup falls through to selector when hfId is not found in the document", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml( + source, + { hfId: "hf-missing", selector: ".headline" }, + [{ type: "inline-style", property: "color", value: "blue" }], + ); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + }); }); diff --git a/packages/core/src/studio-api/helpers/sourceMutation.ts b/packages/core/src/studio-api/helpers/sourceMutation.ts index 8cdc5ae05..6f762377f 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.ts @@ -2,6 +2,7 @@ import { parseHTML } from "linkedom"; export interface SourceMutationTarget { id?: string | null; + hfId?: string; selector?: string; selectorIndex?: number; } @@ -32,6 +33,11 @@ function querySelectorAllWithTemplates(root: Document | Element, selector: strin } function findTargetElement(document: Document, target: SourceMutationTarget): Element | null { + if (target.hfId) { + const matches = querySelectorAllWithTemplates(document, `[data-hf-id="${target.hfId}"]`); + if (matches[0]) return matches[0]; + } + if (target.id) { const byId = document.getElementById(target.id); if (byId) return byId; @@ -207,7 +213,7 @@ export function patchElementInHtml( } export function probeElementInSource(source: string, target: SourceMutationTarget): boolean { - if (!target.id && !target.selector) return false; + if (!target.id && !target.hfId && !target.selector) return false; const { document } = parseSourceDocument(source); const el = findTargetElement(document, target); return el != null && isHTMLElement(el); From ae506ead257d8d8c51c019ad2caeb1f5f66b1598 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 7 Jun 2026 21:25:38 -0700 Subject: [PATCH 08/10] test(core): update htmlParser baselines for R1 hf- id format Elements now get data-hf-id minted by ensureHfIds; parser reads data-hf-id as model id, so HTML id attrs are no longer the model id. Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/parsers/htmlParser.test.ts | 29 +++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/core/src/parsers/htmlParser.test.ts b/packages/core/src/parsers/htmlParser.test.ts index 949943bd0..2d786a7d3 100644 --- a/packages/core/src/parsers/htmlParser.test.ts +++ b/packages/core/src/parsers/htmlParser.test.ts @@ -26,13 +26,13 @@ describe("parseHtml", () => { const result = parseHtml(html); expect(result.elements).toHaveLength(2); - expect(result.elements[0].id).toBe("text1"); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); expect(result.elements[0].startTime).toBe(0); expect(result.elements[0].duration).toBe(5); expect(result.elements[0].name).toBe("Title"); expect(result.elements[0].type).toBe("text"); - expect(result.elements[1].id).toBe("text2"); + expect(result.elements[1].id).toMatch(/^hf-[a-z0-9]{4}$/); expect(result.elements[1].startTime).toBe(2); expect(result.elements[1].duration).toBe(5); }); @@ -53,7 +53,7 @@ describe("parseHtml", () => { expect(result.elements).toHaveLength(1); expect(result.elements[0].type).toBe("composition"); - expect(result.elements[0].id).toBe("comp1"); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); if (result.elements[0].type === "composition") { expect(result.elements[0].compositionId).toBe("abc123"); expect(result.elements[0].src).toBe("/compositions/abc123"); @@ -76,20 +76,20 @@ describe("parseHtml", () => { expect(result.elements).toHaveLength(3); - const video = result.elements.find((e) => e.id === "vid1"); + const video = result.elements.find((e) => e.type === "video"); expect(video).toBeDefined(); - expect(video?.type).toBe("video"); + expect(video?.id).toMatch(/^hf-[a-z0-9]{4}$/); if (video?.type === "video") { expect(video.src).toBe("video.mp4"); } - const audio = result.elements.find((e) => e.id === "aud1"); + const audio = result.elements.find((e) => e.type === "audio"); expect(audio).toBeDefined(); - expect(audio?.type).toBe("audio"); + expect(audio?.id).toMatch(/^hf-[a-z0-9]{4}$/); - const img = result.elements.find((e) => e.id === "img1"); + const img = result.elements.find((e) => e.type === "image"); expect(img).toBeDefined(); - expect(img?.type).toBe("image"); + expect(img?.id).toMatch(/^hf-[a-z0-9]{4}$/); }); it("handles missing attributes gracefully", () => { @@ -123,7 +123,7 @@ describe("parseHtml", () => { const result = parseHtml(html); expect(result.elements).toHaveLength(1); - expect(result.elements[0].id).toMatch(/^element-\d+$/); + expect(result.elements[0].id).toMatch(/^hf-[a-z0-9]{4}$/); }); it("extracts GSAP script from script tags", () => { @@ -398,9 +398,12 @@ describe("parseHtml", () => { `; const result = parseHtml(html); - expect(result.keyframes["text1"]).toBeDefined(); - expect(result.keyframes["text1"]).toHaveLength(2); - expect(result.keyframes["text1"][0].id).toBe("kf1"); + const elId = result.elements[0]?.id ?? ""; + expect(elId).toMatch(/^hf-[a-z0-9]{4}$/); + const kfs = result.keyframes[elId]; + expect(kfs).toBeDefined(); + expect(kfs).toHaveLength(2); + expect(kfs[0].id).toBe("kf1"); }); it("parses stage zoom keyframes", () => { From a9e87d50c511670d9ffa995579d0416233ba084f Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 14:37:30 -0700 Subject: [PATCH 09/10] test(core): data-hf-id survives id/selector patch (R1, T7) Locks the preservation guarantee the write-back design depends on: a Studio edit targeting by id or selector (it never sends hfId) must not strip an existing data-hf-id, or the stable handle is destroyed by the next edit. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../studio-api/helpers/sourceMutation.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.test.ts b/packages/core/src/studio-api/helpers/sourceMutation.test.ts index d3b4552d0..6a1eabaa9 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.test.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.test.ts @@ -414,4 +414,28 @@ describe("T7 — data-hf-id targeting (spec for R1)", () => { expect(matched).toBe(true); expect(html).toMatch(/color:\s*blue/); }); + + // The Studio edit path targets by id/selector (it never sends hfId). Once a + // persisted data-hf-id exists in source, those edits must NOT strip it — else + // the stable handle is destroyed by the next edit. This is the preservation + // guarantee the write-back design depends on. + it("preserves an existing data-hf-id when the element is patched by id", () => { + const source = `

Hello

`; + const { html, matched } = patchElementInHtml(source, { id: "hero" }, [ + { type: "inline-style", property: "color", value: "blue" }, + ]); + expect(matched).toBe(true); + expect(html).toMatch(/color:\s*blue/); + expect(html).toContain('data-hf-id="hf-x7k2"'); + }); + + it("preserves an existing data-hf-id when the element is patched by selector", () => { + const source = `

Old

`; + const { html, matched } = patchElementInHtml(source, { selector: ".body" }, [ + { type: "text-content", property: "textContent", value: "New" }, + ]); + expect(matched).toBe(true); + expect(html).toContain("New"); + expect(html).toContain('data-hf-id="hf-a1b2"'); + }); }); From 2e8a61d622f64933a5147e336d0a4734e9dc60af Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 15:18:07 -0700 Subject: [PATCH 10/10] fix(core): escape hfId in selector + warn on duplicate match (R1, T7 review) Addresses review on #1272 (Miguel P3 + Rames): findTargetElement interpolated target.hfId raw into a [data-hf-id="..."] selector. Escape it (CSS attr-value injection guard) and warn when a hfId matches more than one element instead of silently patching an arbitrary one. Adds an injection-guard test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../studio-api/helpers/sourceMutation.test.ts | 16 +++++++++ .../src/studio-api/helpers/sourceMutation.ts | 33 +++++++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/core/src/studio-api/helpers/sourceMutation.test.ts b/packages/core/src/studio-api/helpers/sourceMutation.test.ts index 6a1eabaa9..8915d411d 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.test.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.test.ts @@ -415,6 +415,22 @@ describe("T7 — data-hf-id targeting (spec for R1)", () => { expect(html).toMatch(/color:\s*blue/); }); + it("does not break out of the selector on a crafted hfId (CSS injection guard)", () => { + // A value with a quote/bracket must be escaped, not injected — it should + // simply match nothing and leave the source untouched, never throw. + const source = `

A

B

`; + const evil = `x"] , [class="victim`; + const run = () => + patchElementInHtml(source, { hfId: evil }, [ + { type: "text-content", property: "textContent", value: "HACKED" }, + ]); + expect(run).not.toThrow(); + const { html, matched } = run(); + expect(matched).toBe(false); + expect(html).toBe(source); + expect(html).not.toContain("HACKED"); + }); + // The Studio edit path targets by id/selector (it never sends hfId). Once a // persisted data-hf-id exists in source, those edits must NOT strip it — else // the stable handle is destroyed by the next edit. This is the preservation diff --git a/packages/core/src/studio-api/helpers/sourceMutation.ts b/packages/core/src/studio-api/helpers/sourceMutation.ts index 6f762377f..d7c2c2ef7 100644 --- a/packages/core/src/studio-api/helpers/sourceMutation.ts +++ b/packages/core/src/studio-api/helpers/sourceMutation.ts @@ -32,10 +32,39 @@ function querySelectorAllWithTemplates(root: Document | Element, selector: strin return []; } +// Prevent CSS attribute-selector injection via a crafted hfId: escape +// backslashes first, then double-quotes. Keeps a malformed/hostile value from +// breaking out of the `[data-hf-id="…"]` selector once callers beyond the +// internal mint contract (R2+ user flows) pass values here. +function escapeCssAttrValue(value: string): string { + return value.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); +} + +function findByHfId(document: Document, hfId: string): Element | null { + try { + const matches = querySelectorAllWithTemplates( + document, + `[data-hf-id="${escapeCssAttrValue(hfId)}"]`, + ); + if (matches.length > 1) { + // The mint contract guarantees uniqueness; a duplicate means upstream + // id drift. Don't silently patch an arbitrary one — surface it. + // eslint-disable-next-line no-console + console.warn( + `sourceMutation: data-hf-id "${hfId}" matched ${matches.length} elements; using the first. ids must be unique per document.`, + ); + } + return matches[0] ?? null; + } catch { + // Malformed selector despite escaping — let the caller fall back. + return null; + } +} + function findTargetElement(document: Document, target: SourceMutationTarget): Element | null { if (target.hfId) { - const matches = querySelectorAllWithTemplates(document, `[data-hf-id="${target.hfId}"]`); - if (matches[0]) return matches[0]; + const el = findByHfId(document, target.hfId); + if (el) return el; } if (target.id) {