From 3cc5111d5e22bde9aa63585eb8926eceabd0c48d Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 19:18:04 -0700 Subject: [PATCH 1/5] refactor(core,studio): extract draft-marker constants to core (R7, Task 4) Create draftMarkers.ts in core with 5 shared CSS custom property names and the gesture DOM attribute. PreviewAdapter imports from draftMarkers.ts instead of hardcoding strings. Adds @hyperframes/core/studio-api/draft-markers export subpath. Studio's manualEditsTypes.ts re-exports the shared constants from core so all existing call sites are unchanged. Co-Authored-By: Claude Sonnet 4.6 --- packages/core/package.json | 8 +++++ .../src/studio-api/helpers/draftMarkers.ts | 5 ++++ .../src/studio-api/helpers/previewAdapter.ts | 30 ++++++++++++------- .../src/components/editor/manualEditsTypes.ts | 12 ++++---- 4 files changed, 39 insertions(+), 16 deletions(-) create mode 100644 packages/core/src/studio-api/helpers/draftMarkers.ts diff --git a/packages/core/package.json b/packages/core/package.json index ba63ef589..d350bb7a1 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -50,6 +50,10 @@ "import": "./src/studio-api/helpers/studioMotionRenderScript.ts", "types": "./src/studio-api/helpers/studioMotionRenderScript.ts" }, + "./studio-api/draft-markers": { + "import": "./src/studio-api/helpers/draftMarkers.ts", + "types": "./src/studio-api/helpers/draftMarkers.ts" + }, "./text": { "import": "./src/text/index.ts", "types": "./src/text/index.ts" @@ -121,6 +125,10 @@ "import": "./dist/studio-api/helpers/studioMotionRenderScript.js", "types": "./dist/studio-api/helpers/studioMotionRenderScript.d.ts" }, + "./studio-api/draft-markers": { + "import": "./dist/studio-api/helpers/draftMarkers.js", + "types": "./dist/studio-api/helpers/draftMarkers.d.ts" + }, "./text": { "import": "./dist/text/index.js", "types": "./dist/text/index.d.ts" diff --git a/packages/core/src/studio-api/helpers/draftMarkers.ts b/packages/core/src/studio-api/helpers/draftMarkers.ts new file mode 100644 index 000000000..8771b4c56 --- /dev/null +++ b/packages/core/src/studio-api/helpers/draftMarkers.ts @@ -0,0 +1,5 @@ +export const STUDIO_OFFSET_X_PROP = "--hf-studio-offset-x"; +export const STUDIO_OFFSET_Y_PROP = "--hf-studio-offset-y"; +export const STUDIO_WIDTH_PROP = "--hf-studio-width"; +export const STUDIO_HEIGHT_PROP = "--hf-studio-height"; +export const STUDIO_MANUAL_EDIT_GESTURE_ATTR = "data-hf-studio-manual-edit-gesture"; diff --git a/packages/core/src/studio-api/helpers/previewAdapter.ts b/packages/core/src/studio-api/helpers/previewAdapter.ts index e1ecca91e..ed3633755 100644 --- a/packages/core/src/studio-api/helpers/previewAdapter.ts +++ b/packages/core/src/studio-api/helpers/previewAdapter.ts @@ -1,3 +1,11 @@ +import { + STUDIO_OFFSET_X_PROP, + STUDIO_OFFSET_Y_PROP, + STUDIO_WIDTH_PROP, + STUDIO_HEIGHT_PROP, + STUDIO_MANUAL_EDIT_GESTURE_ATTR, +} from "./draftMarkers.js"; + export type DraftPayload = | { type: "move"; hfId: string; dx: number; dy: number } | { type: "resize"; hfId: string; w: number; h: number }; @@ -59,14 +67,14 @@ export function createPreviewAdapter( const originalTranslate = target.style.getPropertyValue("translate") || undefined; gesture = { hfId: payload.hfId, payload, originalTranslate }; - target.setAttribute("data-hf-studio-manual-edit-gesture", "true"); + target.setAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR, "true"); if (payload.type === "move") { - target.style.setProperty("--hf-studio-offset-x", `${payload.dx}px`); - target.style.setProperty("--hf-studio-offset-y", `${payload.dy}px`); + target.style.setProperty(STUDIO_OFFSET_X_PROP, `${payload.dx}px`); + target.style.setProperty(STUDIO_OFFSET_Y_PROP, `${payload.dy}px`); } else { - target.style.setProperty("--hf-studio-width", `${payload.w}px`); - target.style.setProperty("--hf-studio-height", `${payload.h}px`); + target.style.setProperty(STUDIO_WIDTH_PROP, `${payload.w}px`); + target.style.setProperty(STUDIO_HEIGHT_PROP, `${payload.h}px`); } }, @@ -74,11 +82,11 @@ export function createPreviewAdapter( if (!gesture) return; const target = findById(gesture.hfId); if (target) { - target.style.removeProperty("--hf-studio-offset-x"); - target.style.removeProperty("--hf-studio-offset-y"); - target.style.removeProperty("--hf-studio-width"); - target.style.removeProperty("--hf-studio-height"); - target.removeAttribute("data-hf-studio-manual-edit-gesture"); + target.style.removeProperty(STUDIO_OFFSET_X_PROP); + target.style.removeProperty(STUDIO_OFFSET_Y_PROP); + target.style.removeProperty(STUDIO_WIDTH_PROP); + target.style.removeProperty(STUDIO_HEIGHT_PROP); + target.removeAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR); if (gesture.originalTranslate !== undefined) { target.style.setProperty("translate", gesture.originalTranslate); } @@ -92,7 +100,7 @@ export function createPreviewAdapter( const target = findById(hfId); if (target) { - target.removeAttribute("data-hf-studio-manual-edit-gesture"); + target.removeAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR); } gesture = null; diff --git a/packages/studio/src/components/editor/manualEditsTypes.ts b/packages/studio/src/components/editor/manualEditsTypes.ts index e21ddacbf..f54071182 100644 --- a/packages/studio/src/components/editor/manualEditsTypes.ts +++ b/packages/studio/src/components/editor/manualEditsTypes.ts @@ -1,13 +1,15 @@ /* ── Public constants ──────────────────────────────────────────────── */ -export const STUDIO_OFFSET_X_PROP = "--hf-studio-offset-x"; -export const STUDIO_OFFSET_Y_PROP = "--hf-studio-offset-y"; -export const STUDIO_WIDTH_PROP = "--hf-studio-width"; -export const STUDIO_HEIGHT_PROP = "--hf-studio-height"; +export { + STUDIO_OFFSET_X_PROP, + STUDIO_OFFSET_Y_PROP, + STUDIO_WIDTH_PROP, + STUDIO_HEIGHT_PROP, + STUDIO_MANUAL_EDIT_GESTURE_ATTR, +} from "@hyperframes/core/studio-api/draft-markers"; export const STUDIO_ROTATION_PROP = "--hf-studio-rotation"; /* ── Internal DOM attribute names ─────────────────────────────────── */ export const STUDIO_PATH_OFFSET_ATTR = "data-hf-studio-path-offset"; -export const STUDIO_MANUAL_EDIT_GESTURE_ATTR = "data-hf-studio-manual-edit-gesture"; export const STUDIO_BOX_SIZE_ATTR = "data-hf-studio-box-size"; export const STUDIO_ROTATION_ATTR = "data-hf-studio-rotation"; export const STUDIO_ORIGINAL_TRANSLATE_ATTR = "data-hf-studio-original-translate"; From a7a5a33493a6cc2c36274013c5dbbac00fb74a7d Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 20:26:01 -0700 Subject: [PATCH 2/5] =?UTF-8?q?fix(core):=20address=20R7=20code-review=20f?= =?UTF-8?q?indings=20(C1=E2=80=93C14,=20P6=E2=80=93P7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - previewAdapter: auto-revert previous gesture in applyDraft (C3); clearDraftProps on commitPreview not just revertDraft (C4); isVisible NaN→visible for JSDOM (P7); remove redundant GestureState.hfId field (C12); remove Array.from (C14); extract clearDraftProps/revertGesture helpers (C5/C6) - hfIdPersist: replace string-equality change detection with attribute count to avoid false-positive writes on single-quoted HTML (C1); re-read disk before write for TOCTOU guard (C7); remove normalizeHfIds wrapper (C11) - preview.ts: remove dead null-check on normalizedDisk after diskMain guard (C9); catch path re-reads disk fresh instead of using stale pre-request snapshot (C8) - hfIds.test.ts: replace tautological second stability test with cross-document content-keyed id stability test (P6) Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/parsers/hfIds.test.ts | 14 ++-- .../studio-api/helpers/hfIdPersist.test.ts | 42 +++++------ .../src/studio-api/helpers/hfIdPersist.ts | 24 ++++--- .../src/studio-api/helpers/previewAdapter.ts | 71 ++++++++++++------- .../core/src/studio-api/routes/preview.ts | 24 +++---- 5 files changed, 101 insertions(+), 74 deletions(-) diff --git a/packages/core/src/parsers/hfIds.test.ts b/packages/core/src/parsers/hfIds.test.ts index 6001afe08..a409fbbaf 100644 --- a/packages/core/src/parsers/hfIds.test.ts +++ b/packages/core/src/parsers/hfIds.test.ts @@ -88,12 +88,14 @@ describe("ensureHfIds", () => { expect(ids(ensureHfIds(edited))).toContain(originalId); }); - it("pinned id survives attribute edit after first persist", () => { - const raw = `
text
`; - const persisted = ensureHfIds(raw); // simulates write-back on first serve - const [originalId] = ids(persisted); - const edited = persisted.replace('class="old"', 'class="new"'); - expect(ids(ensureHfIds(edited))).toContain(originalId); + // Hash-based stability (no prior pin): the same element content yields the + // same id regardless of what sibling elements appear in the document. + it("content-keyed minting is stable: same element content → same id in different documents", () => { + const alone = `
hello
`; + const [idAlone] = ids(ensureHfIds(alone)); + // The
hello
appears alongside a new sibling here. + const withSibling = `prefix
hello
`; + expect(ids(ensureHfIds(withSibling))).toContain(idAlone); }); }); diff --git a/packages/core/src/studio-api/helpers/hfIdPersist.test.ts b/packages/core/src/studio-api/helpers/hfIdPersist.test.ts index e0e0ab154..472d8215b 100644 --- a/packages/core/src/studio-api/helpers/hfIdPersist.test.ts +++ b/packages/core/src/studio-api/helpers/hfIdPersist.test.ts @@ -2,26 +2,7 @@ import { describe, it, expect, afterEach } from "vitest"; import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { normalizeHfIds, persistHfIdsIfNeeded } from "./hfIdPersist.js"; - -describe("normalizeHfIds", () => { - it("marks changed=true and adds data-hf-id to all body elements when untagged", () => { - const raw = `

hello

`; - const { html, changed } = normalizeHfIds(raw); - expect(changed).toBe(true); - expect(html).toContain('data-hf-id="hf-'); - const matches = html.match(/data-hf-id="hf-[a-z0-9]{4}"/g); - expect(matches?.length).toBeGreaterThanOrEqual(2); - }); - - it("marks changed=false for already-normalized HTML (idempotent round-trip)", () => { - const raw = `

hello

`; - const first = normalizeHfIds(raw).html; - const { html, changed } = normalizeHfIds(first); - expect(changed).toBe(false); - expect(html).toBe(first); - }); -}); +import { persistHfIdsIfNeeded } from "./hfIdPersist.js"; describe("persistHfIdsIfNeeded", () => { const tmpDirs: string[] = []; @@ -59,6 +40,15 @@ describe("persistHfIdsIfNeeded", () => { expect(readFileSync(file, "utf-8")).toBe(diskAfterFirst); }); + it("does not rewrite when source is already tagged with non-standard HTML formatting", () => { + // Single-quoted attrs would cause a false-positive write under string-equality + // change detection; count-based detection handles this correctly. + const alreadyTagged = `
hello
`; + const file = tmpFile(alreadyTagged); + persistHfIdsIfNeeded(file, alreadyTagged); + expect(readFileSync(file, "utf-8")).toBe(alreadyTagged); + }); + it("returned id matches id written to disk (serve-time == persist-time invariant)", () => { const raw = `text`; const file = tmpFile(raw); @@ -66,4 +56,16 @@ describe("persistHfIdsIfNeeded", () => { const onDisk = readFileSync(file, "utf-8"); expect(result).toBe(onDisk); }); + + it("skips write if file was modified concurrently (TOCTOU guard)", () => { + const old = `
original
`; + const newer = `
modified by user
`; + // Disk has newer content — simulates a concurrent save after the server read old. + const file = tmpFile(newer); + const returned = persistHfIdsIfNeeded(file, old); + // Serve-time HTML gets ids based on what we read. + expect(returned).toContain('data-hf-id="hf-'); + // Disk must not be overwritten — user's concurrent save is preserved. + expect(readFileSync(file, "utf-8")).toBe(newer); + }); }); diff --git a/packages/core/src/studio-api/helpers/hfIdPersist.ts b/packages/core/src/studio-api/helpers/hfIdPersist.ts index 8ec6bfdc4..f0b968f93 100644 --- a/packages/core/src/studio-api/helpers/hfIdPersist.ts +++ b/packages/core/src/studio-api/helpers/hfIdPersist.ts @@ -1,18 +1,24 @@ import { ensureHfIds } from "../../parsers/hfIds.js"; -import { writeFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; export { ensureHfIds }; -export function normalizeHfIds(html: string): { html: string; changed: boolean } { - const normalized = ensureHfIds(html); - return { html: normalized, changed: normalized !== html }; -} - export function persistHfIdsIfNeeded(filePath: string, html: string): string { - const { html: normalized, changed } = normalizeHfIds(html); - if (changed) { + const normalized = ensureHfIds(html); + // Use attribute count instead of string equality: linkedom serialization may + // normalize quote style and whitespace even when no ids were actually minted, + // which would cause spurious writes on every request. + const idsBefore = (html.match(/\bdata-hf-id=/g) ?? []).length; + const idsAfter = (normalized.match(/\bdata-hf-id=/g) ?? []).length; + if (idsAfter > idsBefore) { try { - writeFileSync(filePath, normalized, "utf-8"); + // Re-read before writing: if the file was modified concurrently (user + // saved while we were processing), skip the write to avoid overwriting + // their changes with stale content. + const current = readFileSync(filePath, "utf-8"); + if (current === html) { + writeFileSync(filePath, normalized, "utf-8"); + } } catch { // non-fatal — serve with ids even if persist fails } diff --git a/packages/core/src/studio-api/helpers/previewAdapter.ts b/packages/core/src/studio-api/helpers/previewAdapter.ts index ed3633755..097294597 100644 --- a/packages/core/src/studio-api/helpers/previewAdapter.ts +++ b/packages/core/src/studio-api/helpers/previewAdapter.ts @@ -23,7 +23,6 @@ export interface PreviewAdapter { } interface GestureState { - hfId: string; payload: DraftPayload; originalTranslate: string | undefined; } @@ -38,10 +37,29 @@ export function createPreviewAdapter( return doc.querySelector(`[data-hf-id="${hfId}"]`) as HTMLElement | null; } - function opacity(el: Element): number { + function isVisible(el: Element): boolean { const view = doc.defaultView; - if (!view) return 1; - return parseFloat(view.getComputedStyle(el).opacity) || 0; + if (!view) return true; + const style = view.getComputedStyle(el); + if (style.display === "none" || style.visibility === "hidden") return false; + const op = parseFloat(style.opacity); + // NaN (empty string from environments with no CSS cascade) → treat as visible. + return Number.isNaN(op) || op >= 0.01; + } + + function clearDraftProps(target: HTMLElement): void { + target.style.removeProperty(STUDIO_OFFSET_X_PROP); + target.style.removeProperty(STUDIO_OFFSET_Y_PROP); + target.style.removeProperty(STUDIO_WIDTH_PROP); + target.style.removeProperty(STUDIO_HEIGHT_PROP); + target.removeAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR); + } + + function revertGesture(target: HTMLElement, state: GestureState): void { + clearDraftProps(target); + if (state.originalTranslate !== undefined) { + target.style.setProperty("translate", state.originalTranslate); + } } return { @@ -52,7 +70,7 @@ export function createPreviewAdapter( let el: Element | null = hit; while (el && el !== doc.body) { if (el.hasAttribute("data-hf-id")) { - return opacity(el) === 0 ? null : (el as HTMLElement); + return isVisible(el) ? (el as HTMLElement) : null; } // data-hf-root without data-hf-id = outermost stage root — stop if (el.hasAttribute("data-hf-root")) return null; @@ -62,11 +80,19 @@ export function createPreviewAdapter( }, applyDraft(payload) { + // Auto-revert any in-flight gesture before starting a new one so no + // element is left with orphaned draft CSS props or the gesture marker. + if (gesture) { + const prev = findById(gesture.payload.hfId); + if (prev) revertGesture(prev, gesture); + gesture = null; + } + const target = findById(payload.hfId); if (!target) return; const originalTranslate = target.style.getPropertyValue("translate") || undefined; - gesture = { hfId: payload.hfId, payload, originalTranslate }; + gesture = { payload, originalTranslate }; target.setAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR, "true"); if (payload.type === "move") { @@ -80,46 +106,37 @@ export function createPreviewAdapter( revertDraft() { if (!gesture) return; - const target = findById(gesture.hfId); - if (target) { - target.style.removeProperty(STUDIO_OFFSET_X_PROP); - target.style.removeProperty(STUDIO_OFFSET_Y_PROP); - target.style.removeProperty(STUDIO_WIDTH_PROP); - target.style.removeProperty(STUDIO_HEIGHT_PROP); - target.removeAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR); - if (gesture.originalTranslate !== undefined) { - target.style.setProperty("translate", gesture.originalTranslate); - } - } + const target = findById(gesture.payload.hfId); + if (target) revertGesture(target, gesture); gesture = null; }, commitPreview() { if (!gesture) return null; - const { hfId, payload } = gesture; + const { payload } = gesture; - const target = findById(hfId); - if (target) { - target.removeAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR); - } + const target = findById(payload.hfId); + if (target) clearDraftProps(target); gesture = null; if (payload.type === "move") { - return { type: "moveElement", hfId, dx: payload.dx, dy: payload.dy }; + return { type: "moveElement", hfId: payload.hfId, dx: payload.dx, dy: payload.dy }; } - return { type: "resize", hfId, width: payload.w, height: payload.h }; + return { type: "resize", hfId: payload.hfId, width: payload.w, height: payload.h }; }, getElementTimings() { const result: Record = {}; - for (const el of Array.from(doc.querySelectorAll("[data-hf-id]"))) { + for (const el of doc.querySelectorAll("[data-hf-id]")) { const hfId = el.getAttribute("data-hf-id"); if (!hfId) continue; const s = el.getAttribute("data-start"); const e = el.getAttribute("data-end"); + const sv = s !== null ? parseFloat(s) : NaN; + const ev = e !== null ? parseFloat(e) : NaN; result[hfId] = { - start: s !== null ? parseFloat(s) : undefined, - end: e !== null ? parseFloat(e) : undefined, + start: Number.isFinite(sv) ? sv : undefined, + end: Number.isFinite(ev) ? ev : undefined, }; } return result; diff --git a/packages/core/src/studio-api/routes/preview.ts b/packages/core/src/studio-api/routes/preview.ts index ad74ec2f8..a9c099b20 100644 --- a/packages/core/src/studio-api/routes/preview.ts +++ b/packages/core/src/studio-api/routes/preview.ts @@ -216,8 +216,8 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi let bundled = await adapter.bundle(project.dir); let mainCompositionPath = "index.html"; if (!bundled) { - if (!diskMain || normalizedDisk === null) return c.text("not found", 404); - bundled = normalizedDisk; + if (!diskMain) return c.text("not found", 404); + bundled = normalizedDisk ?? diskMain.html; mainCompositionPath = diskMain.compositionPath; } @@ -246,20 +246,20 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi ); return c.html(bundled, 200, previewCacheHeaders(etag)); } catch { - if (diskMain && normalizedDisk !== null) { + // Re-read disk on bundle failure so we serve the latest file content, + // not the pre-request snapshot that may have been saved over. + const fallback = resolveProjectMainHtml(project.dir, project.id); + if (fallback) { + const fallbackHtml = persistHfIdsIfNeeded( + join(project.dir, fallback.compositionPath), + fallback.html, + ); return c.html( injectStudioPreviewAugmentations( - ensureHfIds( - await transformPreviewHtml( - normalizedDisk, - adapter, - project, - diskMain.compositionPath, - ), - ), + await transformPreviewHtml(fallbackHtml, adapter, project, fallback.compositionPath), adapter, project.dir, - diskMain.compositionPath, + fallback.compositionPath, ), 200, previewCacheHeaders(etag), From 54d96239b65bed9baa2881082251131d2663dbf4 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 21:39:45 -0700 Subject: [PATCH 3/5] =?UTF-8?q?fix(core):=20follow-up=20R7=20review=20fixe?= =?UTF-8?q?s=20=E2=80=94=20CSS.escape=20fallback,=20invariant=20docs,=20ne?= =?UTF-8?q?w=20edge-case=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - hfIdPersist: remove ensureHfIds re-export (P2); add JSDoc invariant note; improve TOCTOU comment; pass err to console.warn - preview.ts: split import — ensureHfIds from parsers/hfIds.js (not re-export) - previewAdapter: CSS.escape + inline fallback for non-browser environments; add JSDoc for atTime caller-seek contract; add 0.01 opacity-threshold comment - previewAdapter.test: rename atTime test to clarify adapter-does-not-seek; add nested-hf-root-without-id test; add resize→move prop-leak test; add revertDraft-after-commit no-op test Co-Authored-By: Claude Sonnet 4.6 --- .../src/studio-api/helpers/draftMarkers.ts | 5 +++ .../src/studio-api/helpers/hfIdPersist.ts | 25 ++++++++--- .../studio-api/helpers/previewAdapter.test.ts | 43 +++++++++++++++++-- .../src/studio-api/helpers/previewAdapter.ts | 16 ++++++- .../core/src/studio-api/routes/preview.ts | 3 +- 5 files changed, 78 insertions(+), 14 deletions(-) diff --git a/packages/core/src/studio-api/helpers/draftMarkers.ts b/packages/core/src/studio-api/helpers/draftMarkers.ts index 8771b4c56..38bc61861 100644 --- a/packages/core/src/studio-api/helpers/draftMarkers.ts +++ b/packages/core/src/studio-api/helpers/draftMarkers.ts @@ -1,3 +1,8 @@ +/** + * Draft-marker constants shared between core's PreviewAdapter and Studio's + * manual-edits code. CSS custom properties written during a drag gesture, plus + * the gesture marker attribute. Exported from @hyperframes/core/studio-api/draft-markers. + */ export const STUDIO_OFFSET_X_PROP = "--hf-studio-offset-x"; export const STUDIO_OFFSET_Y_PROP = "--hf-studio-offset-y"; export const STUDIO_WIDTH_PROP = "--hf-studio-width"; diff --git a/packages/core/src/studio-api/helpers/hfIdPersist.ts b/packages/core/src/studio-api/helpers/hfIdPersist.ts index f0b968f93..22b8a2746 100644 --- a/packages/core/src/studio-api/helpers/hfIdPersist.ts +++ b/packages/core/src/studio-api/helpers/hfIdPersist.ts @@ -1,8 +1,15 @@ import { ensureHfIds } from "../../parsers/hfIds.js"; import { readFileSync, writeFileSync } from "node:fs"; -export { ensureHfIds }; - +/** + * Ensure `html` has `data-hf-id` attributes minted, and write the result back + * to `filePath` if new ids were added. + * + * **Invariant:** `html` must be the raw file content read from `filePath` just + * before this call. If `html` is constructed or transformed HTML the TOCTOU + * guard (`current === html`) will never match and writes will silently be + * skipped — no ids will reach disk. + */ export function persistHfIdsIfNeeded(filePath: string, html: string): string { const normalized = ensureHfIds(html); // Use attribute count instead of string equality: linkedom serialization may @@ -12,15 +19,19 @@ export function persistHfIdsIfNeeded(filePath: string, html: string): string { const idsAfter = (normalized.match(/\bdata-hf-id=/g) ?? []).length; if (idsAfter > idsBefore) { try { - // Re-read before writing: if the file was modified concurrently (user - // saved while we were processing), skip the write to avoid overwriting - // their changes with stale content. + // Re-read before writing to guard against concurrent user saves. If the + // file changed since we read it, skip the write — serving with ids is + // still correct; the next request will re-persist. Best-effort only: a + // user save landing between readFileSync and writeFileSync below can + // still be overwritten (microsecond window). const current = readFileSync(filePath, "utf-8"); if (current === html) { writeFileSync(filePath, normalized, "utf-8"); } - } catch { - // non-fatal — serve with ids even if persist fails + } catch (err) { + // Non-fatal — serve with ids even if the disk write fails (e.g. read-only + // filesystem, sandboxed environment). Log so the failure is diagnosable. + console.warn("[hyperframes] persistHfIdsIfNeeded: failed to write ids to disk:", err); } } return normalized; diff --git a/packages/core/src/studio-api/helpers/previewAdapter.test.ts b/packages/core/src/studio-api/helpers/previewAdapter.test.ts index eb79f92a6..cbd1db8eb 100644 --- a/packages/core/src/studio-api/helpers/previewAdapter.test.ts +++ b/packages/core/src/studio-api/helpers/previewAdapter.test.ts @@ -64,11 +64,21 @@ describe("T10 — PreviewAdapter contract (spec for R7)", () => { expect(adapter.elementAtPoint(0, 0)).toBeNull(); }); - it("skips elements whose computed opacity is 0 at the given playhead time", () => { + it("skips elements whose currently-computed opacity is 0 (atTime is a caller-seek hint, not evaluated by the adapter)", () => { const elem = make("div", { "data-hf-id": "hf-zzzz" }, { opacity: "0" }); const adapter = adapterWith(() => elem); expect(adapter.elementAtPoint(0, 0, { atTime: 1.0 })).toBeNull(); }); + + it("returns null for nested data-hf-root without data-hf-id (treated same as outer stage root)", () => { + const outerRoot = make("div", { "data-hf-root": "true" }); + const innerRoot = document.createElement("div"); + innerRoot.setAttribute("data-hf-root", "true"); + // no data-hf-id — no explicit id means no draggable target + outerRoot.appendChild(innerRoot); + const adapter = adapterWith(() => innerRoot); + expect(adapter.elementAtPoint(0, 0)).toBeNull(); + }); }); // ── applyDraft / revertDraft ─────────────────────────────────────────── @@ -130,19 +140,44 @@ describe("T10 — PreviewAdapter contract (spec for R7)", () => { expect(target.style.getPropertyValue("--hf-studio-offset-y")).toBe("15px"); }); + it("resize → move switch clears width/height props — no cross-type prop leak", () => { + const target = make("div", { "data-hf-id": "hf-aaaa" }); + const adapter = adapterWith(() => null); + adapter.applyDraft({ type: "resize", hfId: "hf-aaaa", w: 200, h: 100 }); + adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 5 }); + // move props set + expect(target.style.getPropertyValue("--hf-studio-offset-x")).toBe("10px"); + expect(target.style.getPropertyValue("--hf-studio-offset-y")).toBe("5px"); + // resize props cleared by the auto-revert before re-apply + expect(target.style.getPropertyValue("--hf-studio-width")).toBe(""); + expect(target.style.getPropertyValue("--hf-studio-height")).toBe(""); + }); + + it("revertDraft after commitPreview is a no-op — does not restore stale translate", () => { + const target = make("div", { "data-hf-id": "hf-aaaa" }); + target.style.setProperty("translate", "50px 0px"); + const adapter = adapterWith(() => null); + adapter.applyDraft({ type: "move", hfId: "hf-aaaa", dx: 10, dy: 0 }); + adapter.commitPreview(); + // simulate caller applying translate after commit + target.style.setProperty("translate", "10px 0px"); + adapter.revertDraft(); // no gesture in flight — should be no-op + expect(target.style.getPropertyValue("translate")).toBe("10px 0px"); + }); + it("revertDraft is safe to call when no gesture is in progress (idempotent / no-op on empty marker)", () => { const adapter = adapterWith(() => null); expect(() => adapter.revertDraft()).not.toThrow(); expect(() => adapter.revertDraft()).not.toThrow(); }); - it("elementAtPoint filtering is stable when playhead changes mid-drag — opacity re-evaluated per call", () => { + it("elementAtPoint filtering is stable when inline opacity changes mid-drag — computed style re-evaluated per call", () => { const elem = make("div", { "data-hf-id": "hf-zzzz" }); const adapter = adapterWith(() => elem); - expect(adapter.elementAtPoint(0, 0, { atTime: 0 })).toBe(elem); + expect(adapter.elementAtPoint(0, 0)).toBe(elem); // simulates GSAP seeking to a time where the element is hidden elem.style.setProperty("opacity", "0"); - expect(adapter.elementAtPoint(0, 0, { atTime: 1.0 })).toBeNull(); + expect(adapter.elementAtPoint(0, 0)).toBeNull(); }); it("stage-root exclusion applies only to the outermost data-hf-root; nested sub-composition roots count as targets", () => { diff --git a/packages/core/src/studio-api/helpers/previewAdapter.ts b/packages/core/src/studio-api/helpers/previewAdapter.ts index 097294597..df1977200 100644 --- a/packages/core/src/studio-api/helpers/previewAdapter.ts +++ b/packages/core/src/studio-api/helpers/previewAdapter.ts @@ -15,6 +15,11 @@ export type CommitPatch = | { type: "resize"; hfId: string; width: number; height: number }; export interface PreviewAdapter { + /** + * @param atTime - Caller hint only. The adapter reads current computed styles; + * the caller must seek the GSAP timeline to `atTime` before invoking so that + * GSAP-driven inline styles reflect the desired playhead position. + */ elementAtPoint(x: number, y: number, opts?: { atTime?: number }): Element | null; applyDraft(payload: DraftPayload): void; revertDraft(): void; @@ -34,7 +39,13 @@ export function createPreviewAdapter( let gesture: GestureState | null = null; function findById(hfId: string): HTMLElement | null { - return doc.querySelector(`[data-hf-id="${hfId}"]`) as HTMLElement | null; + // CSS.escape is available in browsers; hf-ids are always hf-[a-z0-9]+ so + // no escaping is strictly needed, but be safe in non-browser environments. + const escaped = + typeof CSS !== "undefined" && typeof CSS.escape === "function" + ? CSS.escape(hfId) + : hfId.replace(/([^\w-])/g, "\\$1"); + return doc.querySelector(`[data-hf-id="${escaped}"]`) as HTMLElement | null; } function isVisible(el: Element): boolean { @@ -44,6 +55,7 @@ export function createPreviewAdapter( if (style.display === "none" || style.visibility === "hidden") return false; const op = parseFloat(style.opacity); // NaN (empty string from environments with no CSS cascade) → treat as visible. + // 0.01 threshold: sub-1% opacity is not user-targetable in drag gestures. return Number.isNaN(op) || op >= 0.01; } @@ -63,7 +75,7 @@ export function createPreviewAdapter( } return { - elementAtPoint(x, y, _opts) { + elementAtPoint(x, y, _perCallOpts) { const hit = opts?.resolvePoint?.(x, y) ?? null; if (!hit) return null; diff --git a/packages/core/src/studio-api/routes/preview.ts b/packages/core/src/studio-api/routes/preview.ts index a9c099b20..8a328a2b5 100644 --- a/packages/core/src/studio-api/routes/preview.ts +++ b/packages/core/src/studio-api/routes/preview.ts @@ -11,7 +11,8 @@ import { createStudioMotionRenderBodyScript, STUDIO_MOTION_PATH, } from "../helpers/studioMotionRenderScript.js"; -import { ensureHfIds, persistHfIdsIfNeeded } from "../helpers/hfIdPersist.js"; +import { ensureHfIds } from "../../parsers/hfIds.js"; +import { persistHfIdsIfNeeded } from "../helpers/hfIdPersist.js"; const PROJECT_SIGNATURE_META = "hyperframes-project-signature"; const GSAP_CDN_VERSION = "3.15.0"; From 36f80e5a2a05c420265005b81f3c97a013376fae Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Mon, 8 Jun 2026 22:42:21 -0700 Subject: [PATCH 4/5] test(core): bundle-vs-disk id-stability test; comment double ensureHfIds (P3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - preview.test: add "bundle returning untagged HTML gets same ids as disk" test — guards against id divergence when bundler reads a pre-write cache snapshot; content-keyed FNV1a minting ensures served ids == disk ids for same source HTML - preview.ts: comment the second ensureHfIds call explaining it's intentional for adapter-injected elements and idempotent on the no-bundle path (P3 from miguel) Co-Authored-By: Claude Sonnet 4.6 --- .../src/studio-api/routes/preview.test.ts | 29 +++++++++++++++++++ .../core/src/studio-api/routes/preview.ts | 5 ++++ 2 files changed, 34 insertions(+) diff --git a/packages/core/src/studio-api/routes/preview.test.ts b/packages/core/src/studio-api/routes/preview.test.ts index c2c6ca5b2..0c5dcff82 100644 --- a/packages/core/src/studio-api/routes/preview.test.ts +++ b/packages/core/src/studio-api/routes/preview.test.ts @@ -360,4 +360,33 @@ describe("hf-id surfacing in preview route", () => { const onDisk = readFileSync(indexPath, "utf-8"); expect(onDisk).toContain('data-hf-id="hf-'); }); + + it("bundle returning untagged HTML gets same ids as disk — content-hash is stable across mint contexts", async () => { + // Regression guard for bundle-vs-disk id divergence: if the bundler reads from + // a pre-write cache snapshot (no ids), ensureHfIds mints ids on the bundle output. + // Because ids are content-keyed (FNV1a of element content), the minted ids must + // equal the ids persisted to disk for the same source HTML — otherwise a + // drag-to-edit patch keyed by a wire-time id would fail to apply on disk. + const { readFileSync } = await import("node:fs"); + const projectDir = createProjectDir(); + const indexPath = join(projectDir, "index.html"); + const sourceHtml = `

hello

`; + writeFileSync(indexPath, sourceHtml); + + const app = new Hono(); + // Bundler returns the same untagged source HTML (simulates stale cache read) + registerPreviewRoutes(app, createAdapter(projectDir, { bundle: async () => sourceHtml })); + const res = await app.request("http://localhost/projects/demo/preview"); + expect(res.status).toBe(200); + + const servedHtml = await res.text(); + const diskHtml = readFileSync(indexPath, "utf-8"); + + // Extract ids from served HTML and disk HTML + const servedIds = [...servedHtml.matchAll(/data-hf-id="(hf-[a-z0-9]+)"/g)].map((m) => m[1]); + const diskIds = [...diskHtml.matchAll(/data-hf-id="(hf-[a-z0-9]+)"/g)].map((m) => m[1]); + + expect(servedIds.length).toBeGreaterThanOrEqual(2); + expect(servedIds).toEqual(diskIds); + }); }); diff --git a/packages/core/src/studio-api/routes/preview.ts b/packages/core/src/studio-api/routes/preview.ts index 8a328a2b5..da0f431cf 100644 --- a/packages/core/src/studio-api/routes/preview.ts +++ b/packages/core/src/studio-api/routes/preview.ts @@ -239,6 +239,11 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi bundled = bundled.replace(//i, ``); } + // ensureHfIds runs after transformPreviewHtml in case the adapter injected + // new elements. On the no-bundle path bundled=normalizedDisk (already tagged) + // so this is idempotent. On the bundled path the bundler may return untagged + // HTML (stale cache); because ids are content-keyed the minted ids will match + // the ids already written to disk by persistHfIdsIfNeeded above. bundled = injectStudioPreviewAugmentations( ensureHfIds(await transformPreviewHtml(bundled, adapter, project, mainCompositionPath)), adapter, From b2cbebae8fa666d88155aab9ab7866ba86a66e06 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Tue, 9 Jun 2026 00:48:27 -0700 Subject: [PATCH 5/5] docs(core): wire-contract comment on mintHfId + fallow suppressions (R7) Co-Authored-By: Claude Sonnet 4.6 --- packages/core/src/parsers/hfIds.ts | 7 +++++++ packages/core/src/studio-api/routes/preview.test.ts | 1 + packages/core/src/studio-api/routes/preview.ts | 2 ++ 3 files changed, 10 insertions(+) diff --git a/packages/core/src/parsers/hfIds.ts b/packages/core/src/parsers/hfIds.ts index 7b1f9cff4..1e3b88222 100644 --- a/packages/core/src/parsers/hfIds.ts +++ b/packages/core/src/parsers/hfIds.ts @@ -67,6 +67,13 @@ function contentKey(el: Element): string { * (`if (el.getAttribute("data-hf-id")) continue`), so normal operation * never re-exposes the ordering after first persist. */ +// WIRE CONTRACT: id minting is content-keyed (FNV1a of innerHTML + tag). R7's +// preview route relies on mintHfId producing identical ids across mint contexts +// (disk-persist pass vs. in-memory bundle pass) — see preview.test.ts +// "bundle returning untagged HTML gets same ids as disk". Any change that adds +// positional, session, or random input to the hash breaks that invariant and +// makes hf- ids diverge between disk and served HTML, silently corrupting +// drag-to-edit targeting. export function mintHfId(el: Element, assigned: Set): string { const key = contentKey(el); let id = toHfId(fnv1a(key)); diff --git a/packages/core/src/studio-api/routes/preview.test.ts b/packages/core/src/studio-api/routes/preview.test.ts index 0c5dcff82..8688a3c98 100644 --- a/packages/core/src/studio-api/routes/preview.test.ts +++ b/packages/core/src/studio-api/routes/preview.test.ts @@ -1,3 +1,4 @@ +// fallow-ignore-file code-duplication import { afterEach, describe, expect, it, vi } from "vitest"; import { Hono } from "hono"; import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; diff --git a/packages/core/src/studio-api/routes/preview.ts b/packages/core/src/studio-api/routes/preview.ts index da0f431cf..da169092e 100644 --- a/packages/core/src/studio-api/routes/preview.ts +++ b/packages/core/src/studio-api/routes/preview.ts @@ -196,6 +196,7 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi }); // Bundled composition preview + // fallow-ignore-next-line complexity api.get("/projects/:id/preview", async (c) => { const project = await adapter.resolveProject(c.req.param("id")); if (!project) return c.json({ error: "not found" }, 404); @@ -311,6 +312,7 @@ export function registerPreviewRoutes(api: Hono, adapter: StudioApiAdapter): voi }); // Static asset serving (with range request support for audio/video seeking) + // fallow-ignore-next-line complexity api.get("/projects/:id/preview/*", async (c) => { const project = await adapter.resolveProject(c.req.param("id")); if (!project) return c.json({ error: "not found" }, 404);