[pull] main from heygen-com:main#73
Merged
Merged
Conversation
…mps (#1931) Sub-composition scripts run inside a wrapper that passes the SCOPED __hyperframes (per-instance getVariables) as a bare script param, while `window` is a Proxy. That proxy intercepted only __timelines, so `window.__hyperframes` fell through to the HOST page's base __hyperframes — whose getVariables reads the host's variables, not this instance's. So the two documented spellings diverged: the bare `__hyperframes.getVariables()` param returned the correct per-instance values, but `window.__hyperframes.getVariables()` returned the wrong (host / empty) ones, silently rendering every reused instance with the first instance's content (or defaults). docs/concepts/variables.mdx already promises both forms "work in both top-level and sub-composition scripts ... each instance sees its own resolved values" — the runtime just didn't honor it. Reported directly (a user lost significant debugging time across three parametrized sub-comps before discovering the bare param was the only form that worked), and matches an earlier deferred finding that getVariables() returns {} for reused sub-comp instances. Fix: the scoped `window` proxy now returns the scoped __hyperframes for `prop === "__hyperframes"`, so window.__hyperframes.getVariables() and the bare param resolve identically to this composition's own variables. The scoped variant is Object.assign({}, base, { getVariables }), so all other __hyperframes members still pass through to the base unchanged. Test: two new executed-wrapper cases (new Function(...)(fakeWindow)) — window.__hyperframes.getVariables() now returns the per-comp variables instead of the TOP-LEVEL-LEAK host value, and a non-getVariables member (fitTextFontSize) still reaches the base. Full core suite (1092) passes.
…ries (#1913) * fix(cli): purge stale/partial browser installs instead of wedging retries Two independent reports of the same failure: a `chrome-headless-shell` zip extraction gets interrupted (Windows AV lock, sleep/wake, ctrl-C) and leaves only the alphabetically-early files (ABOUT/LICENSE) in the target directory, no executable. Every subsequent `browser ensure` (or implicit re-download from `findBrowser`/`ensureBrowser`) sees the directory already exists and hands it straight to @puppeteer/browsers' install(), which throws "folder exists but the executable is missing" without re-extracting -- permanently wedging the machine until someone manually deletes the directory. `--force` didn't help because it was a phantom flag: `browser.ts` never declared it, so it silently did nothing (mentioned only in an error-message string). Root cause: `findFromCache()` already detects this exact case (dir exists, exe missing) and returns it as `staleHyperframesCachePath`, but `findBrowser()`/`ensureBrowser()` fed that straight into a re-download without ever deleting the stale directory first, so install() hit the same "exists" branch every time. Fix: - `findFromCache()` also returns `staleInstallPath` (InstalledBrowser's `.path` -- the actual install-folder root, not the missing executablePath) for the stale case. - Both `findBrowser()` and `ensureBrowser()` now purge that directory (`rmSync`, inside the existing `withInstallLock` mutex from #1866 so a purge can't race a concurrent installer) before retrying, so install() actually re-extracts instead of erroring. - Wired up a real `--force` flag on `hyperframes browser ensure`: it purges the whole HF-managed cache (reusing the already-tested `clearBrowser()`) and skips every cache/system shortcut, so it always gets a fresh download regardless of what's currently on disk -- matching what the existing (previously false) help text already claimed it did. Not fixed here (separate root cause, flagged for later): neither report's machine had a usable auto-detected system Chrome fallback on Windows -- `SYSTEM_CHROME_PATHS` only lists macOS/Linux paths, so `findFromSystem()` can never succeed on win32. Both reporters worked around this manually via HYPERFRAMES_BROWSER_PATH, which still works fine; adding real Windows system-Chrome detection is a distinct, larger change. Test: extended manager.test.ts's existing stale-cache-redownload test to include a populated stale install directory and assert it's gone before the mocked install() is called (was previously only asserting the redownload happened, not that the fix's purge step ran). Added a new test for `ensureBrowser({force: true})` purging the cache and bypassing a healthy cache/system-Chrome shortcut. Also fixed the shared fs mock's `rmSync` to actually simulate recursive deletion (drop nested tracked paths too), which the new tests need and the old ones never exercised. Full CLI suite (1222 tests) passes. * fix(cli): serialize force browser cache purge
…and line (#1890) * fix(engine): write the audio mix filter graph to a file, not the command line mixAudioTracks built the ffmpeg -filter_complex argument as one inline string scaling linearly with track count. Reported in the wild at 146 timed audio clips: the resulting command line exceeded the OS length limit and spawn failed with ENAMETOOLONG, dropping audio entirely until the user manually consolidated clips to reduce the count. FFmpeg supports -filter_complex_script specifically for this - the same filter graph read from a file instead of inlined as an argument. The -i pairs for each track still scale with count but stay short and fixed-size each, so the one component that actually grew unbounded (the filter string) no longer sits on the command line at all. The temp file is cleaned up immediately after ffmpeg exits, matching the existing sibling temp-file convention in audioVolumeEnvelope.ts. Verified end-to-end against a real ffmpeg binary (not just mocked): a two-track mix produced correct output audio with no leftover temp files. * fix(engine): create audio filter scripts safely
…ition tail (#1889) * fix(video-workflows): pad the frame's own duration to match the transition tail transitions.mjs extends the index.html WRAPPER's data-duration to cover an outgoing transition's tail, but the frame's own internal composition file kept its shorter, content-only data-duration (authored per frame-worker.md's "duration is fixed upstream" instruction). The render engine clip-gates a sub-composition's visible content at its own declared duration, so content vanished abruptly at content-end instead of fading through the wrapper's extended fade-out tween. A user root-caused and verified this themselves: padding the frame's own duration to match the wrapper fixed it, project-wide, across every non-final frame. transitions.mjs already computes the correct padded duration for the wrapper - it now writes the same value into the matching frame's own file at inject time. Extracted to a shared lib/pad-frame-duration.mjs (mirroring the existing lib/transition-registry.mjs convention) since transitions.mjs's own top-level CLI dispatch runs on import, making it untestable directly. Duplicated identically across pr-to-video, faceless-explainer, and product-launch-video, whose transitions.mjs copies are otherwise byte-identical (confirmed via diff) - one root cause, one fix, applied everywhere it lives. * fix(skills): avoid duration helper file race
…scripts (#1929) `validate` navigated the page with a hardcoded 10s timeout that ignored the --timeout option. A composition that loads GSAP (or any library) from a CDN <script> in <head> blocks `domcontentloaded` until that script finishes downloading; on a slow network that exceeds 10s and validate fails with an opaque "Navigation timeout of 10000ms exceeded" — even though the full render (much larger budget) rides it out fine, and even though --timeout (the documented "wait longer for slow loads" knob) had no effect on navigation. The only recourse was to change the composition (vendor the script locally). Reported precisely, with the exact error and the observation that render's 60s budget masks it while validate's 10s trips. Fix: - resolveNavigationTimeoutMs(optTimeout) = max(10s floor, --timeout), so --timeout now also extends the navigation budget. Default behavior is unchanged: the default --timeout (3000) stays clamped to the 10s floor. - navigationTimeoutHint() replaces Puppeteer's opaque timeout error with an actionable message naming the likely cause (a blocking CDN <script>) and the two fixes (vendor locally / raise --timeout). Any non-timeout error is rethrown unchanged. - --timeout help text updated to note it also governs navigation. Both helpers are pure and exported; validateInBrowser wires them around the single page.goto. No behavior change for compositions that navigate within 10s. Test: resolveNavigationTimeoutMs (floor kept for unset/small/zero, raised past the floor) and navigationTimeoutHint (rewrites a nav-timeout error with CDN + --timeout guidance; returns null for other errors so the caller rethrows as-is). validate suite 14 tests pass.
…es (#1895) * fix(cli): validate seeks the runtime player directly, not raw timelines validate's seekTo() only checked for window.__hf.seek (a bridge object the producer's render-pipeline file server injects) before falling back to grabbing window.__timelines and calling .seek() on each raw GSAP timeline directly. validate serves compositions through a plain static file server that never injects that bridge, so this fallback ran on every single validate invocation. Seeking a raw timeline moves the animation state but skips the runtime's own [data-start]/[data-duration] visibility sync (syncMediaForCurrentState in packages/core/src/runtime/init.ts), which is what sets an off-window clip's inline visibility/display styles. Skipping it left elements outside their timeline window looking fully visible to any check that reads computed style afterward at that seek time. This surfaced as validate's WCAG contrast audit (contrast-audit.browser.js) flagging text in off-window clips against whatever background happened to be behind them, since its own visibility filtering trusts the runtime to have already hidden them. Fix: prefer window.__player.renderSeek, which the composition runtime exposes directly on every page load (no bridge required) and which does run the visibility sync, before falling back to the __hf/raw timeline paths. No changes needed to contrast-audit.browser.js itself since its existing visibility check now sees correct computed style. No new test added: seekTo's branch selection runs entirely inside a page.evaluate() callback, which Puppeteer serializes via .toString() for the browser context, so it can't import and call a project-local window.__player stub from a jsdom/vitest test without testing a copy of the logic rather than the shipped code. Verified instead by reading the runtime chain end-to-end: window.__player.renderSeek is always set by packages/core/src/runtime/init.ts's createPlayerApiCompat, calls through to player.renderSeek, which calls syncMediaForCurrentState(). * fix(cli): wait for runtime seek target in validate
The audio engine shells out to `python3` for ElevenLabs TTS
(tts.mjs) and the local Lyria/MusicGen BGM paths (bgm.mjs). `python3`
is correct on macOS/Linux, but a standard python.org install on
Windows only creates `python.exe` plus the `py` launcher -- there is
no `python3.exe` (only the Microsoft Store build adds one). So every
`spawn("python3", ...)`/`spawnSync("python3", ...)` ENOENTs on a normal
Windows Python setup, silently disabling all Python-backed audio
features until the user hand-creates a `python3.exe` shim (reported: a
user copied python.exe to python3.exe to work around it, and separately
another had to target a python3 stub specifically).
Fix: a shared lib/python.mjs resolver probes the platform's candidates
in order and returns the argv prefix that actually launches Python 3 --
`["python3"]` / `["python"]` / `["py", "-3"]` on win32, `["python3"]`
then `["python"]` elsewhere -- resolved once per process. All direct
`python3` spawn sites in tts.mjs (elevenlabs probe + synth) and bgm.mjs
(pyOk probe, Lyria recipe, MusicGen script) now route through it. On
macOS/Linux `python3` still wins first, so behavior there is unchanged;
if nothing probes OK the resolver falls back to `python3` so the spawn
fails loudly exactly as before, never worse.
Scope: only the direct python3 invocations. bgm.mjs's pipInstall()
still shells `pip` -- switching that to `<python> -m pip` is the
separate concern of the open PR #1894 (draft); noting the overlap so
the two don't collide. Windows-specific whisper.cpp-vs-openai-whisper
detection and the npm_execpath/npx spawn issue from the same report are
distinct root causes, not addressed here.
Test: python.test.mjs (node:test) covers every platform/probe branch
with an injected probe -- no real interpreter spawned: non-win32 picks
python3; win32 prefers python3, falls back to python, then to `py -3`;
the py launcher is probed as `py -3 --version`; nothing-runs falls back
to the canonical python3; and pythonInvocation keeps the launcher's -3
ahead of caller args. Existing tts.spawn.test.mjs still passes (6/6).
… bare pip (#1894) pipInstall() spawned a bare "pip" binary. Many Homebrew/system Python installs expose only python3/pip3 on PATH, so the spawn silently ENOENTs and the documented "auto-installed on demand" local MusicGen path never actually installs anything - the failure is invisible since spawnSync's status just comes back non-zero like any other install failure. Switched to `python3 -m pip install`, matching this same file's own pyOk() convention of always invoking python3 explicitly. This also closes a second latent bug: a bare pip/pip3 could resolve to a different Python installation than the python3 binary pyOk() probes against if more than one is on PATH, so `-m pip` guarantees the install lands in the exact interpreter being checked. Manually verified `python3 -m pip --version` succeeds in this environment. No automated test added - this is a literal command-array swap with no new branching logic, and spawnSync is a named import from node:child_process with no clean mocking path available without a larger refactor disproportionate to the fix's size.
* fix(producer): avoid reviving hidden DOM in HDR layers * fix(producer): filter transition HDR DOM masks * fix(producer): keep hidden timed descendants masked
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )