* fix(cli): reject directory --composition and add --browser-timeout (#1199)
Two unrelated symptoms from issue #1199, fixed together:
1. `--composition .` (or any directory path) used to slip past the
existsSync check in render.ts and explode downstream as
`EISDIR: illegal operation on a directory, read` when the producer
readFileSync'd the entry. The CLI now treats `.` / `""` as "omit
the flag" (falls back to index.html) and rejects other directory
paths with an actionable error pointing at the .html shape.
2. The 60s Puppeteer page.goto timeout in frameCapture.ts was hard-
coded, so heavy compositions (many videos / fonts / asset requests)
could not complete `domcontentloaded` in time. Add a configurable
`pageNavigationTimeout` to EngineConfig (default 60_000, env
fallback PRODUCER_PAGE_NAVIGATION_TIMEOUT_MS) and expose it as
`--browser-timeout <seconds>` on `hyperframes render`. The flag
threads through both renderLocal (via resolveConfig) and the
docker bridge (via buildDockerRunArgs).
Tests:
- render.test.ts: forwards/omits pageNavigationTimeout into resolveConfig
- dockerRunArgs.test.ts: forwards/omits --browser-timeout (seconds)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): address PR #1200 review — extract validators, tighten bounds
Addresses Vai's blockers and Miguel's nits on PR #1200:
- Vai blocker 1 (fallow CRAP) + blocker 3 (no argv tests):
Extract --browser-timeout and --composition validators into pure
helpers in utils/renderArgs.ts with a structured-result discriminant.
Drops ~45 lines of inline validation from run(), reducing its CRAP
score 1290→978 and cyclomatic 75→65. 19 new unit tests cover the
parse branches (sub-ms, overflow, NaN, Infinity, empty, negative,
".", "./", whitespace, directory, missing, ../escape, sibling-prefix).
- Vai blocker 2 (sub-ms → timeout:0 = "no timeout"): reject inputs
that round to <1 ms. Puppeteer treats page.goto({timeout:0}) as
wait-forever, so --browser-timeout 0.0004 silently flipped the
semantics. Now rejected with an explicit "rounds to 0 ms" error.
- Vai important 5 (1e10 accepted → setTimeout overflow): cap at
86_400s (24h). Above Node's TIMEOUT_MAX ≈ 2^31-1 ms setTimeout
fires immediately, the opposite of "long timeout."
- Vai important 4 (related timeouts unmentioned): CLI help and docs
now flag PRODUCER_PUPPETEER_PROTOCOL_TIMEOUT_MS and the 45s
playerReadyTimeout as the other knobs heavy compositions may need.
- Vai nit 7 (s/ms unit mismatch): help text and docs row both call
out the SECONDS-vs-MILLISECONDS difference between flag and env.
- Vai nit 8 / Miguel nit (composition flag discoverability): the
--composition description now says "Pass `.` (or omit the flag)
to render the project's index.html."
- Miguel nit (dead branch): the entryFile === "" unreachable branch
is gone. New helper uses `if (!trimmed || trimmed === ".")`.
Also adds a trailing-separator guard on the project-containment check
(sibling-prefix bypass: /proj-evil/x.html no longer slips past
startsWith('/proj')) — flagged by the code review.
The three remaining fallow complexity findings on render.ts (run,
renderDocker, trackRenderMetrics) are inherited from main; this PR
reduces run() but does not refactor it. Suppressed with
fallow-ignore-next-line markers and inline rationale.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): diverge --browser-timeout error messages per Vai nit 5
The `not-a-number` and `not-positive` branches in browserTimeoutErrorMessage
shared the generic "Must be a positive number of seconds" message even
though the discriminant carried distinct kinds. Diverge them so users see
the specific failure mode:
--browser-timeout abc → "Got \"abc\", which is not a number."
--browser-timeout -5 → "Got \"-5\" seconds, which is not positive."
The shared hint ("pass a positive number of seconds, e.g. 180") is
preserved on both branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(producer): localize remote <img> sources + await image readiness
Producer's frame-capture has `pollVideosReady` (waits readyState >= 2 for
every <video>) but no equivalent for <img>. Combined with htmlCompiler's
`collectExternalAssets` explicitly skipping http(s) URLs (line 805-806),
agent-pipeline-generated compositions (astral / daphne / hyperion
multi-v2 outputs with raw S3 <img src>) reach Chrome with a network
dependency that races the readiness gate AND can be evicted mid-render.
Either path produces blank-frame flicker.
Reproduction (02_kobe agent output, 42s render @ 30fps): scene_02's
remote S3 background-image painted from t=7.0s, vanished at t=10.5s
(frame size 139KB vs 700-940KB neighbors), back at t=11.0s. GSAP
timeline said opacity:1 throughout — Chrome simply didn't have the
pixels.
Two-layer fix:
1. **Producer** — `localizeRemoteImageSources` in `htmlCompiler.ts`
mirrors the existing `localizeRemoteMediaSources` (video/audio) +
`localizeRemoteFontFaces` pattern, reusing `downloadAndRewriteUrls`
and the `_remote_media/` subdir. Wired into `compileForRender`
between the media and font localize steps. Once the file is local,
Chrome's image cache is bounded by disk reads, not S3 latency.
2. **Engine** — `pollImagesReady` + `decodeAllImages` helpers in
`frameCapture.ts` parallel to `pollVideosReady`. Waits for every
`<img>` (skipping data: URIs) to have `complete && naturalWidth > 0`,
then forces GPU upload via `img.decode()`. Called from both the
classic-xvfb path and the BeginFrame path after their respective
video readiness checks. Defense-in-depth — Layer 1 closes the
symptom for current+future agent-pipeline outputs; Layer 2 protects
any future code path that leaves a remote URL in place.
Tests: 7 new cases in `htmlCompiler.test.ts` covering happy-path
rewrite, 404 fallback, dedup of duplicate URLs, non-HTTP and data:
URI passthrough, both quote styles, and the agent-pipeline shape where
`src` is not the first attribute. All pass alongside the existing 56
htmlCompiler tests.
* fix(producer): scope remote-img regex to real src; correct stale comments
Review follow-ups on the remote-<img> localization fix:
- Tighten REMOTE_IMG_TAG_RE with a (?<![\w-]) lookbehind so it matches a
real `src` attribute only. The previous `\bsrc` also matched `data-src`
(and `data-*-src`) lazy-loader placeholders, which would download/rewrite
a URL the render never paints. Added a regression test; `srcset` stays
excluded by the `\s*=` requirement.
- Fix comments that claimed frameCapture has "no pollImagesReady analog" —
this PR adds exactly that, so the docstrings were self-contradictory.
Reframed localization as the primary fix and pollImagesReady as the
defense-in-depth layer, and documented the <img src>-only scope
(srcset / <picture> / SVG <image> / CSS background-image are follow-ups).
Verified locally end-to-end on the 02_kobe repro: all 4 remote S3 <img>
URLs localize to _remote_media/, the render completes, and the frame at
t~10.5s that was a 139KB blank in the broken render now paints the trophy
background in every native-fps frame. htmlCompiler.test.ts 64 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(engine): pollImagesReady broken-image escape + skip decode on in-flight
Addresses two real bugs Magi caught in review on hf#1197:
1. pollImagesReady would spin the full pageReadyTimeout (45s default)
for any <img> that settled with an error — Chrome marks 404 / decode
failure / CORS rejection with (complete=true, naturalWidth=0), and
the previous predicate `complete && naturalWidth > 0` returned false
for those, so the poll ran to timeout. This is the HTMLImageElement
equivalent of pollVideosReady's `ve.error` early-exit. Add a
`complete && naturalWidth === 0` branch that treats settled-with-
error as done — waiting won't make it load. Particularly relevant
because localizeRemoteImageSources falls back to the original URL on
download failure; that failed URL is now hit by a 45s stall instead
of the broken-image marker rendering immediately.
2. decodeAllImages called img.decode() on every image, including those
still in flight after pollImagesReady timed out. Per the WHATWG spec,
decode() on a loading image awaits the fetch — never resolving
until the network completes or puppeteer's evaluate timeout fires
and throws an uncaught error that aborts the render. Pre-filter to
only call decode() on images that successfully loaded.
Test coverage: new frameCapture-pollImagesReady.test.ts with 8 cases
covering empty docs, all-loaded, broken (complete + naturalWidth=0),
data: URI, empty src, in-flight → resolves, in-flight → timeout, and
the mixed batch. The broken-image test explicitly asserts elapsed <
500ms on a 1000ms timeout — guards against the regression Magi flagged.
* docs(engine): clarify decodeAllImages prevents init race, not eviction
Vai correctly noted that decode() forces initial GPU upload but does not
prevent Chrome from evicting decoded pixels mid-render. The producer-side
localizeRemoteImageSources is what bounds the eviction risk (local
file-server paging vs S3 re-fetch). Comment updated to reflect that split
of responsibilities.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(engine): fast-fail on zero duration instead of 45s timeout
When a composition's runtime finishes initializing but reports zero
duration (no GSAP timeline and no data-duration attribute), the engine
previously polled for the full 45-second timeout before failing.
Now, after 10 seconds of polling, the engine checks whether the runtime
has finished (window.__renderReady === true) with a working seek
function but zero duration. If so, it fails immediately with a
diagnostic message explaining what's wrong and how to fix it.
This also improves the generic timeout error message to include runtime
state (whether __player exists, __hf.seek, GSAP timelines, declared
duration) so users can self-diagnose.
PostHog data: 555-1,234 occurrences/day, each wasting 45s of user time.
* fix(engine): throttle diagnostic polls and tighten zero-duration fast-fail
Two nit fixes in pollHfReady:
1. Throttle evaluateHfDiagnostic calls to once per ~1000ms after the 10s
mark. Previously called on every 100ms loop tick, generating ~350 CDP
round-trips per failed render. One check per second is sufficient to
detect a permanently-zero composition.
2. Change fast-fail condition from 'duration === 0' to
'!hasTimeline && declaredDuration <= 0'. A composition with a GSAP
timeline but no data-duration attribute should not be fast-failed —
GSAP sets duration synchronously before __renderReady via __timelines,
so a non-empty __timelines is a reliable signal that duration will
eventually be non-zero. Only compositions with NEITHER a GSAP timeline
NOR a declared duration are permanently zero.
GSAP's volume tween on an <audio> element causes Chrome to construct an
AudioContext. In headless Chrome, the autoplay policy blocks AudioContext
startup with "The AudioContext was not allowed to start" — the frame-capture
loop then waits for it indefinitely and deadlocks before the BeginFrame
fallback can recover. The render hangs at "Starting frame capture" with
0 output frames and times out.
Adding --autoplay-policy=no-user-gesture-required lets the AudioContext start
without a user gesture, which is safe in the headless rendering context where
no real user interaction is possible anyway.
Applied to both the main Chrome launch (browserManager) and the HDR capture
path (hdrCapture).
Fixes#1176. Reported by Abhai (Infinity agent, external).
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(producer): localize remote @font-face src URLs before render
Remote font URLs in @font-face blocks fail with a CORS rejection when
the renderer fetches them from http://localhost:PORT (S3 does not echo
the local origin in Access-Control-Allow-Origin). Chrome falls back to
the next font in the stack (e.g. Arial), producing wrong typography.
localizeRemoteFontFaces() scans <style> blocks, extracts HTTP url()
references inside @font-face rules, downloads them in parallel into
_remote_media/, and rewrites the CSS url() references to local paths —
the same pattern as localizeRemoteMediaSources() for <video>/<audio>.
Background url() references outside @font-face blocks are intentionally
left untouched to avoid downloading arbitrary images.
The shared download+rewrite logic is extracted into downloadAndRewriteUrls()
to eliminate duplication between the two localize functions.
Reported via the Beasty Style caption template (Komika Axis .ttf from S3
falling back to Arial on every cloud render).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(engine): add SSRF guard to downloadToTemp (blocks private/IMDS addresses)
Customer-supplied compositions can author @font-face src URLs (and <video>/
<audio> src attrs via the existing localize path) that point to private
infrastructure. Without a guard, the producer's downloadToTemp would fetch
http://169.254.169.254/... (AWS IMDS), RFC1918, loopback, etc., save the
response to _remote_media/, and expose it via the local file server.
assertPublicHttpsUrl() rejects:
- Non-HTTPS (http://) — all composition fetches must use HTTPS
- 169.254.x (AWS link-local / IMDS)
- 127.x / localhost / 0.x (loopback / unspecified)
- 10.x, 172.16–172.31, 192.168.x (RFC1918)
- [::1], [fc...], [fd...] (IPv6 loopback + unique-local)
The guard fires before the cache check so a blocked URL never gets into
the in-flight map. Applies to both the font-face localize path (PR #1155)
and the existing video/audio localize path (PR #1146) since both call
downloadToTemp.
Note: DNS-rebinding bypasses are not closed by this check (hostname
comparison only, no DNS resolution). Acceptable risk for current threat
model; server-side DNS validation can be layered on later.
12 unit tests covering all blocked ranges + the allowed edge cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(engine): fix TypeScript strict-mode error in urlDownloader SSRF guard
m[1] from RegExp.match() is typed string | undefined; parseInt requires string.
Use nullish coalescing to satisfy tsc without changing runtime behavior —
the regex guarantees m[1] is always defined when the match succeeds.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(engine): use vitest import in urlDownloader test
bun:test is not available in CI — the engine package runs tests via vitest.
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
processCompositionAudio prepares all tracks in parallel (Promise.all),
so for N tracks the mix call lands at index N, not index 1. The 3-track
test was reading calls[1] (the second prepare call) instead of calls[3]
(the mix call), causing indexOf("-filter_complex") to return -1 and the
subsequent assertions to read the wrong args.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(engine): remove amix normalize=0 to fix audio on FFmpeg 4.x/6.x
amix's normalize=0 option is absent from many FFmpeg builds (e.g.
FFmpeg 4.2 on Ubuntu 20.04). When the option is not recognized, FFmpeg
fails the entire filter graph initialization, processCompositionAudio
returns success:false, and the assembled video has no audio stream.
Replace normalize=0 + weights='1...' with the amix default behavior
(normalize=true, divides by track count) and multiply the master output
gain by the track count to restore the original per-track volumes.
The net volume is identical across all FFmpeg versions.
Fixes #1136-adjacent: reported as 'audio doesn't play' in rendered MP4.
* fix(producer): strip img crossorigin + fix audioExtractor normalize=0
Two follow-up fixes:
1. htmlCompiler: strip crossorigin attribute from <img> elements during
compilation. External images (e.g. S3) with crossorigin='anonymous'
force CORS-mode requests against the renderer's localhost file server,
which S3 rejects → images render blank. Matches the existing video
strip at line 261.
2. audioExtractor: same amix normalize=0 bug as audioMixer.ts. The
audioExtractor path is used for <video data-has-audio='true'> mixing
in the CLI's local render pipeline; on FFmpeg 4.x it would also drop
audio silently. Fix: remove normalize=0, compensate with volume=N.
* test(engine,producer): pin amix normalize contract + img crossorigin strip
- audioMixer.test.ts: assert filter has no normalize=/weights=; add
3-track test confirming compensatedGain = masterGain × N = 3
- htmlCompiler.test.ts: parallel tests for img and video crossorigin
strip (covers both elements, not just video)
Two perf fixes caught in #1118 review:
1. Cache guard: probeAndCacheVolumeKeyframes now short-circuits when
the element is already in volumeKeyframeCache. Without the guard
every bindMediaMetadataListeners call (every 30 RAF ticks) re-probed
all bound elements — N elements × full-composition timeline seeks at
60 Hz regardless of whether keyframes were already known.
bindRootTimelineIfAvailable still clears the cache on a new timeline
capture so keyframes stay fresh when the composition is rebound.
2. PCM cursor: audioVolumeEnvelope.ts had the incremental segment
cursor (O(N+M) overall) before #1118 extracted the interpolation into
interpolateVolumeGain. The shared function restarts from segment=0 on
each call — fine for the preview path (one call per RAF tick) but
O(N×M) for the PCM path (one call per sample: 48 kHz × duration).
Napkin math: a 10-min render went from ~30M to ~460M ops. Restored
the inline incremental scan in the engine bake loop; engine now only
imports normaliseEnvelope from core.
Preview audio with GSAP volume fades (e.g. data-volume="0" with a
gsap.to("#bgm", {volume:0.25, ...})) played ~1s then silenced. Root
cause: syncRuntimeMedia used fallbackAuthorVolume (data-volume) on the
first tick after a clip became active, clobbering the GSAP-seeked value.
The single-clock transport seeks GSAP before syncRuntimeMedia runs, so
el.volume already holds the animated value — we just need to trust it.
Fix — three layers, matching the renderer's approach (PR #1117):
1. First-tick tracking: on the first tick a clip is active
(previousRuntimeVolume===undefined), use currentElementVolume (GSAP's
seeked value) instead of fallbackAuthorVolume. In production the
transport always seeks GSAP before syncRuntimeMedia, so el.volume is
already at the correct animated position.
2. Probed keyframes: new probeElementVolumeKeyframes() runs the same
offline probe the renderer uses (discoverAudioVolumeAutomationFromTimeline)
directly in the browser. init.ts calls probeAndCacheElementVolume() when
an element is bound and a timeline is available. When keyframes are present,
syncRuntimeMedia drives volume from the interpolated envelope — no
GSAP-change tracking needed, no first-tick edge case, same data source
as the renderer.
3. Shared utilities: normaliseEnvelope(), interpolateVolumeGain(), and
probeAndCacheElementVolume() extracted to mediaVolumeEnvelope.ts and
exported from @hyperframes/core/media-volume-envelope. The engine's
audioVolumeEnvelope.ts imports from there — no duplicate logic between
the renderer and the new preview path.
Fallow audit exits non-zero on inherited complexity/duplication in init.ts
functions that shifted line numbers (applyClipLayout, transportTick, etc.),
unchanged by this PR — same known false-positive pattern noted in #1117.
Lint, format, typecheck, and unit tests all pass.
53 core/media tests pass (3 updated to pre-set el.volume to match the
runtime's bindMediaMetadataListeners — corrects a missing setup step).
audioVolumeEnvelope tests (6) still pass.
Animated media volume (GSAP/JS fades) dropped the audio track entirely for dense
fades. The 60 Hz timeline probe emits 100-300 keyframes for a multi-second fade,
which were folded into an FFmpeg `volume` expression nesting one `if(lt(t,...))`
per keyframe. Past ~95 nested levels (build-dependent, lower on some Linux ffmpeg
builds) the expression overflows FFmpeg's evaluator, fails filter-graph init,
fails the whole mix, and the muxer omits audio — so a `data-volume="0"` fade-in
rendered with no audio at all (follow-up to #1066; this is why #1064's own
scenario regressed once the fade was dense enough).
Apply volume automation as sample-accurate gain, layered so audio is never lost:
1. Primary: bake the envelope into the prepared PCM samples in-process
(audioVolumeEnvelope.ts). The track WAV is always pcm_s16le/48k/stereo;
multiply its samples by the interpolated envelope and atomically rename the
result into place, then mix at unity. No expression, no keyframe ceiling,
exact at every sample, and the downstream ffmpeg amix/AAC encode is untouched
so golden baselines only change where a fade is applied. The RIFF parser
scans chunks order-independently and accepts only 16-bit PCM, falling back
otherwise. The output is written to a random-named sibling and renamed, so a
crash can't leave a truncated WAV and there's no predictable-path write.
2. Fallback: RDP-bounded ffmpeg `volume` expression (0.5% tolerance, capped at
32 segments) for the rare case a WAV is not 16-bit PCM. 0.5% keeps the
rendered envelope within ~0.2 dB of the source curve.
3. Backstop: if an automated mix still fails, retry once at base volume and
surface the degradation rather than dropping the track.
This mirrors how OSS NLEs render automation (sample-level gain): MoviePy,
Kdenlive/Shotcut (MLT), Remotion.
Verified end-to-end: a 297-keyframe fade that rendered with no audio now bakes
all 297 keyframes sample-accurately. Adds unit tests for sample-accurate gain,
track-start offset, base/tail holds, thousands of keyframes, order-independent
chunk parsing, and format rejection, plus mixer regression tests for bounded
nesting and the base-volume backstop.
* fix(engine): use captureBeyondViewport on all CDP screenshot paths
Chrome's compositor rounds the viewport boundary inward under multi-tab
load, clipping the bottom/right edge of tall portrait compositions
(1080x1920). The explicit clip rect already constrains output to exact
composition dimensions, making the viewport-boundary pre-clip from
captureBeyondViewport:false both redundant and unreliable.
Set captureBeyondViewport:true on all three CDP screenshot call sites:
pageScreenshotCapture, captureScreenshotWithAlpha, and captureAlphaPng.
Add portrait-edge-bleed regression test: 1080x1920 grid with bright
magenta bottom rows, rendered with 4 workers. Any compositor clipping
at the bottom edge drops PSNR sharply against the golden baseline.
Closes#1009
* fix(engine): address review feedback on captureBeyondViewport
- Add backref comments on captureScreenshotWithAlpha and captureAlphaPng
pointing to pageScreenshotCapture for the rationale, so the next reader
doesn't treat the flag as unintentional copy-paste
- Note in test meta.json that the static grid fixture covers the
capture-side clipping path but not the video-element compositor surface
timing that produces the t≈37s self-healing in #1009
* test(producer): use video element in portrait-edge-bleed regression test
Replace the static CSS grid with a 1080x1920 portrait video element —
matches the original bug report shape where the compositor surface
allocation timing causes the bottom-edge clipping. The video has a dark
top region and bright magenta bottom 480px, so any viewport clipping at
the bottom edge drops PSNR sharply. Baseline regenerated in Docker with
4 workers.
* fix(engine): disable browser pool for parallel capture in BeginFrame mode
BeginFrame's compositor is process-global — when multiple pages in the
same Chrome instance drive HeadlessExperimental.beginFrame concurrently,
they race the compositor and crash with "Protocol error: Target closed".
Only disable the pool when BeginFrame mode would actually be active
(Linux + headless-shell + not forceScreenshot). Screenshot mode
(macOS/Windows) is unaffected and keeps the pool for memory efficiency.
Also extracts the frame capture loop into captureFrameRange to reduce
function complexity in executeWorkerTask.
* fix(engine): include supersampling in BeginFrame-mode predicate
Match the full capture-mode predicate from createCaptureSession:
DPR > 1 (supersampling) forces screenshot mode, which is pool-safe.
Without this check, supersampled parallel renders on Linux would
unnecessarily launch separate browsers.
Two follow-ups to the ancestor-visibility skip in `injectVideoFramesBatch`
and `syncVideoFrameVisibility`.
1. **Mask defence.** Both ancestor-hidden branches previously wrote a plain
`img.style.visibility = "hidden"`. `applyDomLayerMask` writes the
stylesheet rule `#${showId} *{visibility:visible !important}`, and CSS
cascade puts important stylesheet author above non-important inline
author — so a sub-comp host landing in the active layer's `show` set
would revive a stale `__render_frame__` and let it bleed onto the
layer composite. Write the hide via
`style.setProperty("visibility", "hidden", "important")` instead;
important inline beats important stylesheet.
2. **Caller cache hygiene.** `createVideoFrameInjector` unconditionally
wrote `lastInjectedFrameByVideo.set(id, frameIndex)` after calling
`injectVideoFramesBatch`, even for videos the page silently skipped due
to a hidden visual ancestor. On the next call at the same frameIndex —
common with source-fps < output-fps, paused source frames, or
non-frame-aligned host starts — the cache short-circuited the second
inject and the host's first visible frame painted blank because the
replacement `<img>` was never created.
Make `injectVideoFramesBatch` return `string[]` (the subset of ids it
actually painted) and have the caller cache only those. The cli-side
`snapshot.ts` consumer is unaffected: its local `InjectFn` types the
return as `Promise<void>`, which is structurally compatible with
`Promise<string[]>` under TS void-return assignment rules.
Tests: linkedom doesn't preserve `!important` in cssText, so the two new
mask-defence cases spy on the live `<img>`'s `style.setProperty` and assert
the 3-arg call shape. The cache-hygiene case stubs the page-side primitives
via `vi.mock`, drives the hook twice at the same frameIndex with a stubbed
"injected nothing" first response, and verifies the second call still
issues an inject. A counter-test pins the happy-path cache hit so a future
refactor can't trade the skip bug for a never-cache regression.
`isVisualAncestorHidden` was treating any `visibility: hidden` ancestor as a
signal to skip injecting the replacement frame. That's too broad — for plain
`[data-start]` containers, the replacement `<img>`'s explicit
`visibility: visible` correctly overrides the ancestor per CSS spec, and
consumers rely on that to hold the final GSAP-driven frame when an authored
`data-duration` outlives the composition's GSAP timeline (e.g.
`style-9-prod`, where the runtime truncates the host to `visibility: hidden`
after the timeline ends and the replacement frame must paint through).
Restrict the `visibility: hidden` skip to ancestors that carry
`data-composition-src` or `data-composition-file` — the actual sub-composition
hosts this guard was added for. `display: none` keeps the broad behavior:
it takes the whole subtree out of layout and a child override cannot escape.
Update the existing regression suite to mark the host as a sub-composition,
and add two new cases pinning the plain-`[data-start]` behavior: both
`injectVideoFramesBatch` and `syncVideoFrameVisibility` must still produce a
visible replacement `<img>` when the host is `visibility: hidden` but does
not carry a sub-composition attribute.
The screenshotService.test.ts regression-suite comment pointed at the
author's fork branch as backstory. Strip the line so upstream code
doesn't carry a fork-relative reference; the surrounding paragraph
already explains the bug end-to-end without it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`injectVideoFramesBatch` and `syncVideoFrameVisibility` iterate every
`video[data-start]` whose raw time window covers the current seek.
Inner `<video>` elements inside `[data-composition-src]`
sub-compositions get `data-start="0"` auto-injected by
`compileTimingAttrs` and probed-duration cover the entire timeline,
so they look "active" even when their host has not yet started.
When the runtime then hides the host with `visibility: hidden` (its
out-of-window lifecycle), the inner video inherits hidden via the CSS
cascade — but our injector responded by painting a replacement
`<img class="__render_frame__" style="visibility: visible">` next to
the video. `visibility: visible` on the descendant defeats the parent
`visibility: hidden` cascade, and because the host has not been
morphed by GSAP yet the video's bounding box is its CSS default
(usually full-bleed). The result is one full-bleed frame per inactive
sub-comp painted over whichever moment is *actually* visible — the
overlay symptom the upstream agentic-finecut project saw.
Walk ancestors in both functions; if any has `display: none` or
`visibility: hidden`, skip the inject and hide any stale
`__render_frame__` sibling. The render is now correctly empty for
hidden hosts, which is what the surrounding CSS cascade already
intends.
Tests:
- `screenshotService.test.ts`: cover the new guard for both
visibility:hidden and display:none hosts, both for the fresh-img and
the stale-img paths, plus `syncVideoFrameVisibility` for the case
where the time window calls a video "active" but a hidden ancestor
still requires its frame to stay hidden. Each test fails against
pre-fix `screenshotService.ts`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace require("child_process") with static import (same ESM fix
as config.ts — require is undefined in native ESM)
- Unify cap: both VRAM probe and heuristic paths now cap at 16GB
- Add comment noting the one-time blocking execSync is cached
On NVIDIA systems, spawns nvidia-smi once (cached) to read actual GPU
memory. Uses real VRAM for the Chrome GPU budget instead of guessing
from total system RAM. Falls back to total/2 on non-NVIDIA systems or
when nvidia-smi is unavailable.
No other headless Chrome renderer probes GPU memory — Remotion, Puppeteer,
and Playwright all ignore --force-gpu-mem-available-mb entirely.
Scale GPU budget to half of total RAM (capped at 16GB) instead of
hardcoding 4096MB. A 32GB machine now gets 16GB GPU budget; a 64GB
machine gets 16GB (Chrome's practical limit). Low-memory tiers unchanged.
Replace dynamic require("os") with static import — require is undefined
in native ESM, causing the try/catch to silently return the 16GB
fallback on every machine. The cache scaling was dead code.
Addresses review feedback: freemem() is misleading on macOS where
aggressive file caching reports low free memory even on high-spec
machines. Switched to totalmem()-based thresholds consistent with
calculateOptimalWorkers in parallelCoordinator.ts.
Thresholds now based on total RAM:
- <4GB total: GPU=512MB, V8=256MB, cache=32/128MB
- <8GB total: GPU=1024MB, V8=512MB, cache=64/256MB
- >=8GB total: unchanged (4096MB GPU, no V8 cap, 256/1500MB cache)
