Repository navigation
feat(core): bridge preserved film HTML to native capture - #5131
Conversation
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Reviewed at 30f6c08aa, focused on the opaque iframe protocol. The core checks are right:
- the sandbox refuses
allow-same-origin; event.source === iframe.contentWindowandorigin === "null"are both required;- frame replies are matched on
seq.
Two should-fixes:
1. A reloaded runner is never loaded again (filmBridge.ts:66-72).
loadedstays true for the bridge's lifetime. If the film document navigates or reloads (film code callinglocation.reload(), or a link inside the sandbox), the new document'scontentWindowis the same browsing context, so itshellopasses the source/origin checks. Because!loadedis false, it never getsload.- The next
renderpostsframeto an unloaded runner and fails after the 15 s timeout. - (Inference, not browser-tested) A reply already queued from the old document for the current
seqcould also be accepted. - Fix: treat a second
helloas a new document generation. Fail the bridge fast, or rejectpendingand redo the handshake, rather than waiting on the timeout.
2. A send failure stalls instead of failing (filmBridge.ts:69, :100).
postMessage({ ...load, ... })throwsDataCloneErrorfor any non-cloneable value, andloadis typedRecord<string, unknown>.- The throw escapes the message handler with
loadedalready true, soreadyhangs until the 20 s startup timeout. - Fix: wrap both
postMessagecalls and route a throw tofail(). The same applies to anullcontentWindowatrendertime, which currently waits out the 15 s.
— Jerrai
…-bridge # Conflicts: # packages/core/src/runtime/entry.ts
|
Both findings are valid and fixed in d3e7bcb; parent fixes are carried forward without rewriting existing commits.
Regression tests cover reload before readiness and during a pending frame, old acknowledgements, DataCloneError on load, frame-send exceptions, and missing runner windows. Focused core tests: 55 passed. Final-stack validation: 4,252 core, 591 SDK, and 51 real-browser tests passed, plus core build/typecheck and commit hooks. Actual partner HTML preview/export acceptance remains unverified under the previously reported browser policy block. |
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed at d3e7bcb52. Both findings are fixed.
- Reload: a second
hellofails the bridge, rejects ready and any pending frame, and clears both timers. Later messages can't revive it. Recreating the bridge on reload is the right call, since old-document replies can't be told apart. - Failed sends: both
postMessagesites go through the guarded send, so a clone error or a missingcontentWindowfails immediately. All three cases have tests.
— Jerrai
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed at d3e7bcb52, focused on the message protocol and its failure paths.
What holds up:
allow-same-originis refused at construction (filmBridge.ts:16-19).- Both
event.sourceand the opaque"null"origin are checked (:91), and acknowledgements match onseq, so a spoofed or stale reply cannot release capture. - The new
send()wrapper turns a missing window or aDataCloneErrorinto an immediate failure, and a secondhellonow fails fast instead of waiting out the deadline.
One minor item, not blocking:
Minor: one slow frame ends the bridge for the life of the page.
- The frame deadline calls
fail()(:110), which setsfailure(:38). From then onreceivedrops every message (:91) and everyrenderthrows (:105). frame-sources.mdsays a later successful seek recovers from a frame failure, andimported-film-html.mdsays aframe-errorrecovers. A timeout is the one frame failure that never does.- Export is unaffected: the barrier fails that capture either way. Preview is where it shows. One frame over 15 s (a cold asset fetch, or a runner that waits on
requestAnimationFramewhile the editor tab is in the background) leaves the scene frozen until a full reload, with nothing in the UI saying why. - Fix: on a frame timeout, reject only that request and clear
pending, the wayframe-errordoes at:62. Keep startup failures terminal. A late reply for the oldseqis already ignored by the sequence check.
CI at this head: 28 passing. regression and Player perf were cancelled at this sha, not failed; rerun them before merge.
Verdict: APPROVE
Reasoning: sender, origin and sequence validation are correct, and every send and startup failure now fails promptly. The open item is a recovery path in preview, not wrong output.
A composition that follows the frame-source contract registers no GSAP timeline, so lint reported missing_timeline_registry and every render waited 45 s for timeline registration (17.6 s -> 1 min 47.9 s for the 14 s imported film sample). Document the existing data-no-timeline opt-out for the host and a timeline-less root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Imported film wrappers have no GSAP timeline. Without data-no-timeline on the scene hosts and root, lint fails with missing_timeline_registry and each render waits 45 s for timeline registration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Approved at c4b8763c7. The parent merge carries exactly the #5130 delta (init.ts +3, the browser test, and the frame-sources doc line), plus one doc line in imported-film-html.md requiring data-no-timeline on the film host. Nothing else changed since my last approval.
The regression, Perf and Preview parity failures here were cancelled runs ("Change detection did not succeed"), not test failures. I have re-run them.
— Jerrai
The base branch was changed.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
jerrai-bot-heygen
left a comment
There was a problem hiding this comment.
Approved again at 5541b4383, after the retarget to main.
I checked the new diff against main, file by file, against this PR's own delta at my last approval (29a5c02b4...c4b8763c7). It is the same 7 files, and every added and removed line is identical:
filmBridge.ts: 123 changed lines.filmBridge.test.ts: 202.imported-film-html.md: 32.entry.ts: 3.entry.test.ts,window.d.tsanddocs.json: 1 each.
So the merge of main kept the film bridge exactly as it was reviewed, and nothing else entered the diff.
— Jerrai
Edit accuracy: accurate 2059 (base branch 2059), smooth 1664 of thoseThe gate passes. Quarantined, measured but not gated (0) |
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-stamp at 5541b4383, now based on main after #5130 landed. Since my review at d3e7bcb52, this PR's own diff changed only in imported-film-html.md (hosts and timeline-less roots take data-no-timeline). The transport-redraw fix arrived through #5130 and is on main. Bridge code is unchanged, so the earlier findings and the open minor (a frame timeout is terminal in preview) stand as written.
CI: 85 passing, 1 red. The red is Studio: timeline viewport gate: interaction p95 was 71.7 ms on the first attempt and 58.9 ms on the second, against a 58.3 ms budget. Main failed the same gate on the #5130 merge commit a17d0bfc0 and passed it on the next two commits. This PR touches no Studio code, so I am not counting it against the change; a rerun should clear it.
Verdict: APPROVE
Reasoning: the bridge is unchanged since the reviewed head and the delta is documentation.
What
Add
window.__hyperframes.createFilmBridgeto drive preserved film-runner HTML through its currentappifact-film:protocol. The wrapper can keep the runner, scene modules and assets unchanged while HyperFrames owns time and screenshot capture.Why
Current Claude Motion previews already expose a sandboxed runner with explicit frame requests. Supporting that protocol avoids requiring a new export format or translating animation code into GSAP.
Related work
Depends on #5130; merge it first. Second of three PRs; #5132 adds bounded scene timing. Somansh owns MCP extraction, asset packaging and import/export.
How
Install the message listener before setting
srcdoc, retain the opaqueallow-scriptssandbox, and validate both source window and origin. Load after hello, wait for ready, then await matching frame sequence IDs. Bound startup/frame waits and reject fatal errors or disposal; individual frame errors can recover on later seeks. No separate rendering service or autoplay clock is introduced.Test plan
Core build/typecheck, changed-file lint/format, 4,240 core tests passed (
--maxWorkers=4 --testTimeout=30000). Focused bridge/runtime tests rerun after simplifying the message handlers. The original partner HTML is not committed. Audio/video extraction and manual editing inside the iframe are outside this slice.Complexity report: highest new message handler is CC 8; no new function exceeds 10.