[miguel-heygen]

What broke

Picking a nested element inside a sub-scene (for example a label with no id inside a b-roll scene) and deleting it could empty index.html: the root composition and every clip gone, with no error.

Why

The preview picker (packages/core/src/runtime/picker.ts, buildElementSelector) named an element with a selector that was unique only among its siblings. A nested div with no id got the bare selector div. Anything that re-found the pick with document.querySelector got the first div in the preview, which is index.html's root composition, so deleting "the pick" removed the root. The server then wrote what it was asked to.

Fix

1. Picker: a pick's selector now matches only the picked element in its document. The short forms (#id, [data-composition-id], ...) are kept when they are already unique; otherwise the selector is a child path from the nearest ancestor whose short form is unique, e.g. [data-composition-id="broll"] > [data-hf-inner-root] > div > div > div.
2. Studio server: one guard, rootCompositionRefusal (helpers/rootCompositionGuard.ts), refuses a save that leaves an HTML file which had a composition with none at all: 422 with a plain message, file and backups untouched. Renaming a root composition and non-HTML files save as before. It runs in the three writers that can drop elements: the mutation routes (writeMutationResult), the file PUT every client save goes through, and applyFileMutations (element patch batches answer 422 too). Not wired: the undo restore and GSAP rollback routes (they write back content the client recorded) and the path-rename rewrite (attribute values only); freeze-frame and split only add or clone.

Tests

• picker.test.ts: every element's pick selector matches exactly that element (including prefixed tags like o:p), and the nested label's no longer resolves to the root. Fails on main (the label's selector resolved to the root).

• files.rootComposition.test.ts: deleting the root of index.html, deleting a sub-scene file's own root inside its <template>, a PUT with an emptied body, and an element patch that strips the last composition are each refused with 422 and the file left byte-identical (the first three fail on main with 200). Deleting a nested sub-scene child, deleting a sub-scene host, renaming the root in the code editor and editing a non-HTML file that mentions a composition all still save.

• applyFileMutations.test.ts: a mutation that drops the last composition throws and leaves the file untouched. In a two-file batch where only the second file loses its composition, the check runs for every file before the first backup is taken, so both files, the first file's write receipt and its full ten-entry backup journal are unchanged (fails on the previous head: the first file's oldest backup was pruned).

• Mutations: guard off fails exactly the refusal tests; HTML-only check removed fails only the non-HTML test; the 422 mapping removed fails only the patch test (500); tag escaping removed or uniqueness forced true fails only the picker test.

• Also folds in a flake fix met on this PR's CI: coreRuntimeBrowser.test.ts browser tests ran on vitest's 5 s default and one timed out at 5048 ms on a slow runner (main peaks at 1.77 s over 5 runs; that runner stretched tests up to 5.1x), so both browser describe blocks now get an explicit 15 s timeout.

Before / After

Fixture project: index.html (root main, a title clip, a b-roll sub-scene host) and compositions/broll.html (a nested label with no id). Headless Studio: ask the preview picker for the label's selector, re-find it with querySelector, delete what that resolves to through Studio.

• Before (main): the selector is div, it resolves to #root, the delete returns 200, the stage goes black and the timeline is empty.
• After: the selector resolves to the label; Studio removes only the label from compositions/broll.html; index.html is unchanged. Sending the old request (delete #root) now returns 422 and nothing is written.

[miguel-heygen]

Pre-review pass (independent reviewer), and what changed at 13b387a

#	Finding	Rank	Outcome
1	Guard parsed non-HTML files and refused e.g. a Markdown edit	should fix	Fixed: HTML files only; test added
2	A refusal from applyFileMutations surfaced from the element-patch routes as a bare 500	should fix	Fixed: typed error mapped to 422; route test added
3	Over-refusal of everyday deletes was untested	should fix	Fixed: sub-scene host delete test (200)
4	The nested-child delete test passes on main	should fix	Kept on purpose as the over-refusal check; the picker test is the one that fails on main for this bug
5	Renaming the root composition was refused	note	Fixed: the guard now refuses only a save that leaves no composition at all; rename test added
6	Prefixed tags (o:p) produced invalid selectors for every descendant	note	Fixed: tag escaped; covered in the picker test
7	GSAP rollback route unguarded	note	Intended (writes back recorded content); listed in the body
8	Guard costs about 250 ms per write on a 1.9 MB file	note	Now one parse of the new content in the normal case; the old content is parsed only when the new one has no composition
9	Text/rich-text patch on the root keeps the id but drops children; <template data-composition-id> inner root	note	Out of scope for this guard (it checks that a composition survives, not its content)
10	Shadow-root picks still build a light-DOM path	note	Same as main; not changed here

Audited	Trusting	Not exercised
Every picker selector consumer in packages/ (all feed querySelector; source lookup goes by id first)	Split and freeze-frame only add or clone	History undo/step writers
Guard through app.request on remove-elements, PUT /files, patch-elements-batch; applyFileMutations directly	Studio toasts the 422 text	Embedding apps' delete flows
Picker in headless Chromium (SVG, foreignObject, duplicate ids) and jsdom		Full studio-server suite (CI runs it)
Lint, format, tsc on core and studio-server; 5 studio-server files 224/224, picker 51/51

[github-actions]

Edit accuracy: accurate 271 (base branch 271), smooth 120 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

Unstable (1)

• seqheldtwice-tlhold-px-r0-nested-z100: tracking 120.29, pressJump 312.3, drop 0.07, reload 0.07, render 0.03, renderKey -, undo true, teleport false / tracking 0.41, pressJump 0.11, drop 0.39, reload 0.39, render 0.02, renderKey -, undo true, teleport true / tracking 0.41, pressJump 0.11, drop 0.39, reload 0.39, render 0.02, renderKey -, undo true, teleport true

[terencecho]

Reviewed 6250014 against effective merge base 87c6e50 (not the newer API base tip).

What is working well

• The picker checks document-wide uniqueness and element identity before using a short selector, otherwise assembling an escaped, anchored child path (picker.ts). The repeated-node/prefixed-tag fixture checks that selectors resolve to the original same-document elements. This is not a claim that runtime selectors universally preserve source-file ownership.
• The guard deliberately protects composition existence, not a particular root ID or all original descendants. HTML-only checking, template traversal, root-rename/non-HTML controls, and typed 422 handling keep that contract narrow. The inspected ordinary PUT and one-file mutation paths check before their own snapshot.
• The successor adds a named 15,000 timeout option to the two browser-test suites, leaving explicit 30,000 test/hook annotations in place. It changes test configuration, not production deletion code. I did not execute Vitest or establish its installed implementation's exact timeout inheritance, default, or flake resolution.

[P2] Preserve earlier files' recovery history when a later batch file is refused

The new guard runs inside the sequential applyOneMutation loop, rather than in the existing all-files preflight (applyFileMutations.ts:38–59, 105–107). Consequently, rejecting a later file can already have changed an earlier file's retained recovery history.

Static source-trace scenario, not an executed reproduction: submit the supported POST /projects/:id/file-mutations/patch-element-batches with two matched files in order. A.html gets a harmless title="old" → title="new" attribute edit that preserves its composition. B.html receives a schema-valid html-attribute operation setting its sole data-composition-id to null; the inspected mutation fold removes that attribute. This passes existing operation/target/version checks but is then rejected by the new composition guard. No concurrent writer is needed. A starts with ten distinct, chronologically older retained backups, including an oldest recovery point with distinct content. The journal unconditionally writes a new snapshot at a non-colliding timestamp/counter path and immediately prunes; choose ordinary increasing timestamps so the distinct oldest entry is the prune victim. The route accepts/folds these operations and preserves request order (files.ts:448–509, 3449–3472).

A passes its guard, snapshots, and writes. snapshotBeforeWrite immediately prunes to the default ten backups, deleting A's oldest retained snapshot (backupJournal.ts:6, 31–49, 82–100). B then throws the new root-refusal error, which the route maps to 422. Rollback restores A's current source bytes and clears its write receipt, but does not restore the pruned older backup or remove the newly created snapshot (applyFileMutations.ts:71–83). B is refused before its own snapshot.

Thus this rejected operation leaves both source files unchanged/restored but loses an older retained recovery point. This is not current document-byte loss, a whole-video wipe, or proof of irrecoverability from every other backup. Generic journal/rollback nontransactionality predates this PR; the introduced interaction is the new deterministic semantic refusal after earlier-file side effects. On the reviewed BASE batch helper, this otherwise-valid request proceeds through the existing write path without the new root-existence refusal. The PR description's “Fix,” item 2 promises “422 with a plain message, file and backups untouched” and explicitly includes element patch batches; this case violates that stated refusal behavior.

Please ensure a root-refused batch preserves source bytes, receipts, and retained backup inventory/content—for example, preflight every prepared root-loss outcome before the first snapshot/write while preserving per-write freshness checks. Add a two-file refusal regression with a full A journal asserting that invariant. The new one-file refusal fixture does not exercise this interaction.

Scope, verification, and limits

The primary and fresh unprimed adversarial passes read all seven existing changed BASE bodies before all nine changed HEAD bodies, including tests. Source identity/ownership, actual picker consumers, separate ordinary canvas-delete targeting, save/error handling, and recorded-history/GSAP carveouts were traced within bounded supporting source. The ten public author concerns were reconciled individually. No application-wide proof for every SDK/external consumer or terminal restore implementation is claimed. A possible attribute-casing mismatch remains unpromoted because complete application recognition support for uppercase-only composition attributes has not been established.

No application tests, browser/native execution, installs/builds, CI queries/triggers, live/provider calls, merge, release, deployment, or production mutation were performed. Author tests, mutants, screenshots, timing and historical automation comments remain attributed claims, not my runtime results. CI color is not the reason for this verdict.

Workflow review found no approval-event trigger in the 17 inspected repository workflows, which does not exclude the stated external approval→merge loop. A later main push can invoke catalog publication/branch/PR/dispatch automation; release/npm/plugin effects have additional conditions. External consumers, generated-artifact closure, and successful publication/deployment were not verified. This review does not authorize direct merge or triggering those actions.

Process disclosures: current primary source accounting is 54 blobs / 1,070,339 bytes including charged API text; two supporting files remain explicitly partial. Its retained-public-text Reads lacked fresh per-read reservations. The fresh adversary reports 42 blobs / 929,014 API-inclusive bytes; resumed/batched reservations, earlier omitted API charges, retrospective coverage receipts, and an EOF-overshooting final range are disclosed rather than treated as contemporaneously perfect records. The effects inventory comprises 74 acquired Git blobs (42 FULL, three PARTIAL, 29 UNREAD) and six FULL API-text objects, totaling 519,460 acquired bytes; nested instructions were discovered after cache materialization but before semantic effect reads, initial per-call plans were late, and some cap records were stale. The applicable current-pass limits are 100 objects, 4 MiB aggregate, and 400 KiB per object. The figures above report current acquisitions, not procedural clearance; they do not cure the procedural gaps or bound the unknown historical acquisition. An initially primed adversarial attempt was discarded and replaced. The canceled, unposted 13b387a0 attempt's unknown transient acquisition bytes and rejected original read method remain historical caveats, not retroactively repaired by this new review.

— tai

Verdict: REQUEST CHANGES

Reasoning: The picker and single-file refusal handling are targeted improvements, but a later-file root refusal can occur after an earlier file's retained recovery history has been rotated away. Rejected batches need to preserve that recovery history.

[miguel-heygen]

[P2] later batch file refused after an earlier file's backup rotated: fixed at bb46618.

applyFileMutations already had an all-files preflight (the freshness check) before any snapshot. The root-composition check now runs in that same loop, against the bytes the freshness check just read, and is removed from the per-file write step. So a refused batch throws before the first snapshotBeforeWrite: no new backup, no prune, no write, no receipt. The per-write freshness check in applyOneMutation stays; it still rejects a file that changed between preflight and write, which is also what keeps the preflight's verdict valid for the bytes actually written over.

Regression: refuses a batch whose later file loses its root composition before touching any backup. File A has ten distinct prior backups and a harmless title edit; file B loses its only composition. It asserts the 422-mapped error names B, both files are byte-identical, A has no write receipt, and the backup folder's names and contents are exactly as before. On 6250014 it fails on the journal assertion (A's oldest backup pruned, a new one added); with the fix it passes (3 runs in a row), and the studio-server suite is 1355/1355.

The two single-file writers (writeMutationResult and the file PUT) already refuse before their snapshot, so they needed no change.

[heygengenesis]

Verdict: APPROVE · 0 blocker · 0 important · 1 nit · head 66a0673b

@terencecho's P2 (a refused later file rotating an earlier file's backups) is fixed. The root check now runs in the all-files preflight before the first snapshotBeforeWrite, and the two-file test pins the full journal, the receipt and both files' bytes. The PUT and writeMutationResult guards also run before their snapshots. CI is green.

• nit: the PR body still lists the 15 s coreRuntimeBrowser.test.ts timeout fold-in. That change from 62500149 isn't in the diff after the main merges, and main still has the defaults, so please update the body or re-add it.