Skip to content

fix(cli): mention hyperframes doctor after a browser download failure - #7253

Open
mertkayacs wants to merge 1 commit into
heygen-com:mainfrom
mertkayacs:fix/browser-download-doctor-hint
Open

mertkayacs wants to merge 1 commit into
heygen-com:mainfrom
mertkayacs:fix/browser-download-doctor-hint

Conversation

@mertkayacs

@mertkayacs mertkayacs commented Nov 5, 2026 •

Copy link
Copy Markdown

What

When the managed Chrome download fails and there is no system Chrome, the recovery message now ends with the same hyperframes doctor step the installed-Chrome message already has. The existing cross-platform table test covers that branch.

Why

The no-Chrome message tells you to install Chrome and set HYPERFRAMES_BROWSER_PATH, then stops, so there is no way to check that the path is right before re-running. This looks like it was dropped in #6954, which removed the guessed executable paths.

On CI the runners ship Chrome, so downloadStall.process.test.ts takes the installed-Chrome branch and passes. On a machine with no system Chrome it takes the no-Chrome branch and fails on main today ("expected ... to contain 'hyperframes doctor'"); it passes with this change.

How

Reuse the existing hyperframes doctor sentence in the no-Chrome message, after the HYPERFRAMES_BROWSER_PATH instruction, without guessing an executable path.

Test plan

  • fails without the fix: manager.test.ts "offers no path to export when no Chrome is installed, so a dead override is never suggested" (macOS, Windows and Linux cases): with main's manager.ts the message has no hyperframes doctor.

  • No-Chrome table test: 3 passed with the fix.

  • downloadStall.process.test.ts: 2 passed on a host without Chrome (1 of 2 fails on main).

  • The changed test passed 10 of 10 runs under CPU load.

  • Build, typechecks, lint, format, the comment gates, test reachability and the CLI lazy-dependency checks pass.

  • Both CLI shards ran locally. The two failures left (an FFmpeg-dependent HDR assertion and one timeout that passes when run alone) also show up on unmodified main.

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (no command, flag or workflow changed)

  • Comments follow CONTRIBUTING.md "Comments" (no comments added or changed)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · head a954196b

The code change looks right: the no-Chrome message now ends with the same hyperframes doctor step as the installed-Chrome branch, and it still doesn't guess an executable path.

  • blocker (process): this repo requires every fix PR to name, in its body, a test that fails without the fix, on a line that starts with fails without the fix:. This body says the test "fails on main" but has no such line. For example:
    - fails without the fix: manager.test.ts "downloadBrowser — install failure surfaces HYPERFRAMES_BROWSER_PATH…" no-Chrome cases (macOS, Windows, Linux): with main's manager.ts the message lacks "hyperframes doctor".

An edit to the body is enough; no new commit is needed.

Thanks for the contribution!

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE — 1 resolved, 0 open, 0 new · a954196 (body-only edit). The fails without the fix: line is present. I confirmed locally that the three no-Chrome cases in manager.test.ts fail with main's manager.ts and pass on this head (81/81). CI is green.

SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 6, 2026
…view to heygen-com#7258

Adopts heygen-com#7306 (motion-blur composition counts in render_complete telemetry,
adapted to discoverMovers; readEvents stays false without heygen-com#7191) and heygen-com#7318
(a set that interrupts a tween holds at any worker count) together with its
merged prerequisite heygen-com#6851. PR axis advances to heygen-com#7258; heygen-com#7253 and heygen-com#7257 are open
and pinned as monitored (97->99).

Co-authored-by: Vance Ingalls <vance@heygen.com>
Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant