fix(studio): journal source writebacks

[miguel-heygen]

Problem

Studio structured edits and file overwrites mutate source HTML immediately. If a user accidentally drags or changes a keyframe-backed element, there is no on-disk recovery point outside the in-memory Studio history.

Fixes #1381.

What this fixes

Adds a bounded .hyperframes/backup/ journal before source overwrites and returns the modified file path from write responses. Structured DOM edits and GSAP script commits now show an Updated <path> toast after a successful write so users can see which source file changed.

Root cause

The file routes wrote new bytes directly with writeFileSync(...) across PUT, DOM mutation, and GSAP mutation paths. The client had enough information to update history, but the server never snapshotted the previous file content and structured edit flows were silent after successful write-back.

Verification

Local checks

• bunx vitest run packages/core/src/studio-api/helpers/backupJournal.test.ts packages/core/src/studio-api/routes/files.test.ts
• bunx oxfmt <touched files>
• bunx oxlint <touched files>
• bun run --filter @hyperframes/core typecheck
• bun run --filter @hyperframes/studio typecheck
• Lefthook pre-commit: filesize, largefiles, lint, format, fallow, typecheck

Browser verification

• Started a real Studio preview for /tmp/hf-1381-studio-project.
• Used agent-browser to open http://localhost:5190, record the flow, and POST a structured patch-element mutation through the live Studio API.
• Confirmed via browser-side API reads that index.html contained After, while .hyperframes/backup/...index.html still contained Before and not After.
• Screenshot: artifacts/1381/studio-loaded.png, artifacts/1381/studio-after-mutation.png (local verification artifacts)
• Recording: artifacts/1381/writeback-backup.webm (local verification artifact)

Notes

• Backup creation is best-effort. A backup write/prune failure logs a warning but does not block the user's save.
• Backups are pruned per file, keeping the newest 10 snapshots.
• The fresh worktree has unrelated LFS pointer noise in packages/producer/tests/webm-transparency/output/output.webm; it was not staged or committed.

[james-russo-rames-d-jusso]

hyperframes#1388 — fix(studio): journal source writebacks — adds .hyperframes/backup/ snapshots before structured writebacks + Updated <path> toasts.

Verified the writeback target/direction: server-side snapshot of pre-overwrite bytes into <projectDir>/.hyperframes/backup/<ts>-<sanitized-path>, pruned to newest 10 per file. Client gets backupPath in the JSON response and shows an Updated <path> toast. CI: 1 failing check — CodeQL: Potential file system race condition on backupJournal.ts:59.

Blockers
• packages/core/src/studio-api/routes/files.ts:852 (create-only) and the delete path at :858 are intentionally outside the journal scope, fine — but the upload-media path at :790 (writeFileSync(finalPath, buffer)) also unconditionally overwrites if finalPath already exists, and is NOT snapshotted. If a user re-uploads a media asset under the same filename, the prior bytes vanish with no recovery — which is exactly the user-story this PR is closing (#1381 — "on-disk recovery point outside in-memory history"). Either snapshot it or document in the PR that asset overwrites are deliberately out-of-scope.

Concerns
• packages/core/src/studio-api/helpers/backupJournal.ts:46-58 — CodeQL's TOCTOU flag is technically real (existsSync → statSync → readFileSync), but in the Studio's single-process single-user shape it's not exploitable. Close it explicitly: collapse to one readFileSync in the try and catch ENOENT/EISDIR, which silences the alert AND is racier-correct. Right now the failing required check will block merge anyway.
• backupJournal.ts:62 (nextBackupPath) — timestampPrefix() is Date.now()-ish (new Date().toISOString()) and used purely for filename uniqueness, so the determinism rubric doesn't apply (this never runs at render time, only in the studio-edit server route). Worth a one-line comment noting "filename-only, never read back into render state" so a future reader doesn't flag it.
• backupJournal.ts:78-86 (pruneBackups) — sort key is mtimeMs desc with localeCompare tiebreak. The timestamp prefix is millisecond-resolution, but nextBackupPath adds -2, -3 suffixes on same-millisecond collisions, and the suffix order doesn't match localeCompare's lexicographic order for >=10 collisions. Edge case (you'd need 10 saves in <1ms), but the keepPerFile=3 test masks it — recommend pruneBackups sort by full filename desc (timestamp prefix already sorts correctly) and drop statSync from the hot path. Cheaper too — statSync per candidate during prune is N syscalls you don't need.
• backupJournal.ts:27-29 (sanitizePath) — replace(/[^a-zA-Z0-9._-]/g, "_") collapses unicode + spaces. Two files My File.html and My_File.html collide on the same sanitized stem, which then makes pruneBackups's includes(suffix) cross-prune the wrong file's backups. Low-prob (Studio scaffolds ASCII names) but a real correctness gap if users save with non-ASCII filenames.
• pruneBackups's name.includes(suffix) match (backupJournal.ts:81) is substring, not suffix. A file named foo.html will match backups for foo.html and subdir__foo.html because the latter's tail contains -foo.html too. endsWith(suffix) is the right test.
• Cross-PR coherence with #1366 — #1366 hardens the save-flow (queue, circuit breaker, save_failure diagnostics) on the client. This PR's journal sits on the server response path; the two don't currently interlock. Worth surfacing: if #1366's circuit-breaker pauses save retries, the server-side journal still snapshots on each accepted POST — meaning a user retrying through a paused queue won't accumulate backup garbage (good), but also: if a single client save fans out into multiple structured mutations (DOM patch then GSAP commit), each leaves a backup entry, which can blow through the 10-snapshot cap on a busy edit session. Not a blocker, just confirm the user-mental-model is "10 saves" not "10 edits."
• .gitignore doesn't ignore .hyperframes/backup/ (confirmed against repo .gitignore at HEAD — only skills/hyperframes-* and claude-design-hyperframes-video/ match hyperframes). The journal sits next to source files inside the project, and a user who git add .s their Studio project will commit their backup directory. Either add .hyperframes/ to the per-project .gitignore that Studio scaffolds for new projects, or document it as user-managed. Easy to miss.

Nits
• backupJournal.ts:35 — backupPathForResponse does rel.split("\\").join("/") after relative(), but relative() on POSIX already returns /-separated paths; the Windows path-sep normalization here is only meaningful on Windows runtimes. Not wrong, just dead on Linux/macOS — a // Windows-safe response normalization comment makes intent obvious.
• useDomEditCommits.ts:251 — showToast(\Updated ${patchData.path ?? targetPath}`, "info")fires after *every* structured edit. With high-frequency edits (drag-resize), this may stack toasts in the UI. (nit — Vai's lane to confirm if there's debouncing downstream, flagging for cross-checking) •useGsapScriptCommits.ts:163— same toast pattern, fires per GSAP mutation. (nit, same concern) •useDomEditCommits.ts+useGsapScriptCommits.tsremoved// ── Helpers ──/// ── Types ──/// ── Hook ──` section dividers as drive-by cleanup. Unrelated to this PR's stated scope. (nit)

Questions
• Was the unlink path (DELETE /projects/:id/files/* at files.ts:858) considered for the journal? The PR's framing is "user accidentally drags or changes a keyframe-backed element" — but an accidental delete is the same blast radius with worse recovery. If it's deliberately out of scope, a one-line note in the PR body would help future readers.
• Backup directory is <projectDir>/.hyperframes/backup/ — does the Studio's project zip-export exclude .hyperframes/? If not, projects ship with their backup journal embedded.

What I didn't verify
• Runtime-divergence / HF-player semantics — that's Vai's lane in this review pass. I did NOT trace whether the response shape change (adding path / backupPath) reaches any SDK consumer outside Studio. Vai should confirm.
• Stack-tip #1389 cumulative — peeked at #1389's incremental diff; it's the finiteMutation value-guard work and does not touch backupJournal.ts or hoist any fix from this PR up to the tip. Confirmed independent.
• #1366 cross-read — peeked at #1366's file list; it does NOT modify packages/core/src/studio-api/routes/files.ts or backupJournal.*. The two PRs touch the same conceptual save-flow but at different layers (client save-queue vs server response shape). No conflicts; coherence concern above is a UX/capacity question, not a correctness one.

Review by Rames D Jusso

[james-russo-rames-d-jusso]

Re-classifying per author's request to evaluate long-term-vs-bandaid framing.

Journal mechanism itself is the right long-term shape, but ships covering one of three destructive write paths:

• ✅ structured writebacks (gsap-mutations/*, dom-edit/patch-element/*) — journaled
• ❌ upload-media at files.ts:790 — unconditional overwrite, not journaled. Re-uploading an asset under the same filename silently destroys prior bytes; that's exactly the recovery scenario #1381 closes
• ❌ unlink at files.ts:858 — accidental delete has same blast radius, no recovery

Plus durability gaps inside the journal subsystem itself:

• sanitizePath collision (My File.html and My_File.html collapse to the same stem) → pruneBackups cross-prunes the wrong file's backups
• pruneBackups substring match (name.includes(suffix)) cross-matches between sanitize-related filenames; needs endsWith
• .gitignore doesn't cover .hyperframes/backup/; users running git add . will commit their backup directory
• CodeQL TOCTOU on the journal helper (backupJournal.ts:46-58) is a hard required-check failure

Each is fixable; together they signal the journal hardening isn't done.

Long-term shape: extend journal to all destructive writes (or explicitly scope as "structured-writebacks only" + ship a follow-up plan for upload/delete in the PR body), fix sanitize+prune correctness, scaffold .hyperframes/ into the project gitignore template Studio creates.

See my earlier comment review for full file:line breakdown.

Review by Rames D Jusso

[james-russo-rames-d-jusso]

hyperframes#1388 — R2 verification of fix(studio): journal source writebacks against HEAD 259bb60.

R1 was request-changes framed around "right long-term shape, ships covering one of three destructive write paths + four durability gaps." Walked each item against the new diff.

R1 items — resolution status

• ✅ upload-media at files.ts:790 (R1 blocker) — resolved differently. Author didn't journal it; instead rewrote the path to never overwrite. Lines 766-781 now pick a non-colliding name (2).ext, name (3).ext, … up to 9999 before falling back to skipped. Snapshot becomes architecturally unnecessary on this path because no prior bytes are ever destroyed. Legitimate close — alternative resolution to the operational goal.

• ✅ unlink / delete at files.ts:858 (R1 blocker) — journaled. New snapshotBeforeWrite(res.project.dir, res.absPath) at files.ts:864 runs before rmSync / unlinkSync, and the DELETE response carries backupPath. Test coverage: files.test.ts "backs up the previous file content before delete".

• ✅ sanitizePath collision (My File.html ↔ My_File.html cross-prune) — resolved by construction. sanitizePath is gone entirely; backup key is now Buffer.from(relativePath, "utf-8").toString("base64url") (backupJournal.ts:14-16). base64url is injective per byte sequence, so two different file paths cannot collide on the backup-key namespace. Test coverage: backupJournal.test.ts "does not prune backups for paths with colliding sanitized names".

• ✅ pruneBackups substring match (name.includes(suffix)) — fixed. New filter is name.endsWith(suffix) || numberedSuffix.test(name) with numberedSuffix = new RegExp(\-${backupKey}-\d+$`) (backupJournal.ts:82-86`). Both branches are anchored to the end-of-name so a backup for path A cannot match a prune filter for path B. Combined with the injective base64url key above, this closes the cross-prune class of bugs cleanly.

• ✅ CodeQL TOCTOU on backupJournal.ts:46-58 — fixed as suggested. Collapsed to single readFileSync(absPath) inside the try, with explicit ENOENT / EISDIR handling in catch (backupJournal.ts:40-61). CodeQL check now passes; all 49 required checks green.

• ⚠️ .gitignore doesn't cover .hyperframes/backup/ (R1 concern) — partially resolved. The hyperframes monorepo root .gitignore now lists .hyperframes/backup/ (covers anyone editing inside this repo). However, R1's specific concern was about Studio-scaffolded user projects: hyperframes init my-video produces a project directory, the user git inits it, then git add . sweeps the backup folder in. packages/cli/src/commands/init.ts (the scaffolder) does NOT write any .gitignore into the new project — confirmed by grep at HEAD. Narrow gap, affects only the "user tracks their scaffolded project in git" workflow. Worth a follow-up ticket to either (a) add .hyperframes/ to packages/cli/src/templates/_shared/ so every scaffolded project ignores it, or (b) document in the user docs that .hyperframes/ is local state. Mitigant: safePath.ts:10 now adds .hyperframes to IGNORE_DIRS, so backups are hidden from Studio's file-listing UI — orthogonal to git but reduces user-facing confusion.

R1 long-term framing — addressed

R1 said: "journal mechanism is right; ships covering structured writebacks only" and asked for either (a) extend to all destructive writes or (b) explicitly scope + ship follow-up plan.

The author chose (a) in a smart asymmetric shape:

• PUT /files/* (full overwrite) → journaled ✅
• DELETE /files/* (unlink/rmdir) → journaled ✅
• patch-element / remove-element / update-id / gsap-mutations/* → journaled ✅
• upload-media → architecturally can't overwrite anymore ✅
• POST /files/* (create) → unchanged, 409s if exists (not destructive)
• copy at :1057 → writes to generateCopyPath output, which by design produces a non-colliding name (not destructive)
• updateReferences at :197 → batch-rewrites references when an asset is renamed; this is a multi-file destructive write that isn't journaled. Narrow risk (only fires on rename, and rename itself preserves the renamed file), but worth noting as the one remaining unbacked write path in the route. Not a blocker for this PR's scope.

PR body still frames as "structured edits and file overwrites" — accurate to the new shape, even though scope expanded to cover delete too. Not worth re-asking for a body edit.

Bonus improvements not in R1 ask

• safePath.ts:10 — adds .hyperframes to IGNORE_DIRS, so the backup directory is hidden from walkDir (Studio's project file listing). Backups can't accidentally surface in the file tree, asset picker, or any consumer of walkDir. New safePath.test.ts asserts the behavior.
• Test coverage on the new helpers is solid: backupJournal.test.ts covers happy-path snapshot, zero-byte files, pruning to keepPerFile, and the sanitize-collision regression case explicitly. files.test.ts adds three integration tests for PUT/DELETE/patch-element.

CI

All 49 checks pass (including CodeQL). Required checks all green.

Verdict — stamp-ready

R1 blockers: 2/2 resolved (upload-media differently, unlink directly).
R1 concerns: 4/4 resolved on the journal-correctness axis (sanitize, prune, CodeQL, gitignore-in-repo). 1 narrow partial on per-project-scaffold gitignore — follow-up territory, not blocker.

Author shipped a tighter fix than R1 prescribed: kept journal scoped to truly destructive writes, eliminated overwrite risk on upload-media by changing the write semantics instead of paying for journal overhead, hid the backup dir from project listings.

Recommend: approve and merge. File a follow-up to add .hyperframes/ to packages/cli/src/templates/_shared/.gitignore for new scaffolded projects (smallest possible change to close the last gap).

Review by Rames D Jusso

[vanceingalls]

LGTM

[jrusso1020]

Approved.

[miguel-heygen]

Merge prep update: conflicts are resolved and the branch is git-mergeable, but GitHub still reports reviewDecision=CHANGES_REQUESTED from the earlier james-russo-rames-d-jusso review. The later approval from jrusso1020 does not clear that requester in branch policy. Please approve/dismiss from the original requesting account so this can merge once checks finish green.