Skip to content

fix(cli): doctor shows platform-correct install hints - #319

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
Dylanwooo:fix/doctor-platform-hints
Apr 18, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
Dylanwooo:fix/doctor-platform-hints

Conversation

@Dylanwooo

Copy link
Copy Markdown
Contributor

Summary

hyperframes doctor shipped two bugs that misled non-Debian-Linux and Windows users:

  1. Wrong FFmpeg install hint. When FFmpeg was missing, the hint was hardcoded as sudo apt install ffmpeg for every non-macOS platform. Windows users saw an apt command that doesn't exist on their system; Red Hat / Arch users got the wrong package manager.

  2. FFprobe check broken on Windows. checkFFprobe called which ffprobe via execSync. which isn't a command on Windows (cmd uses where), so the check always reported "Not found" even when ffprobe was on PATH.

Fix

  • checkFFmpeg now uses getFFmpegInstallHint() — the helper already exists in browser/ffmpeg.ts, already handles darwin / linux / win32 correctly, and is already used by render.ts. Just reuse it here.
  • checkFFprobe now runs ffprobe -version directly instead of shelling out to which. This works on every platform where ffprobe is resolvable on PATH, and surfaces the version string in the same style as the FFmpeg check.
  • The "not found" hint for ffprobe now falls through to the same platform-aware ffmpeg install hint (they ship together).

Before / After

Before (on Windows, ffprobe present):

✗ FFprobe    Not found
             Installed with ffmpeg

After:

✓ FFprobe    ffprobe version 8.1 Copyright (c) 2007-2026 …

Before (on Windows, FFmpeg missing):

✗ FFmpeg     Not found
             sudo apt install ffmpeg      ← wrong OS

After:

✗ FFmpeg     Not found
             https://ffmpeg.org/download.html

Test plan

  • bunx oxlint packages/cli/src/commands/doctor.ts — clean
  • bunx oxfmt --check packages/cli/src/commands/doctor.ts — clean
  • Pre-commit typecheck hook passes
  • bunx tsx packages/cli/src/cli.ts doctor runs locally on macOS — FFprobe now reports version instead of path, all other behavior unchanged

Notes

Scope intentionally kept small. findFFmpeg() in browser/ffmpeg.ts still uses which ffmpeg which has the same Windows issue — happy to fix in a follow-up PR if the maintainers want it, but it has a wider blast radius (used by render.ts too) so I left it out of this one.

The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg`
for any non-macOS platform — Windows users would see an apt command that
doesn't exist on their system, and Red Hat / Arch users got the wrong
package manager too.

`getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is
already used by render.ts) and handles darwin / linux / win32 correctly.
Use it here too.

Also rewrite checkFFprobe:
- it previously used `which ffprobe` which is not available on Windows
  (cmd uses `where`), so on Windows the check always reported "Not
  found" even when ffprobe was on PATH
- run `ffprobe -version` directly instead, which works cross-platform
  whenever ffprobe is resolvable on PATH, and surfaces the version
  string in the same style as the FFmpeg check

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the clean fix and the thorough write-up! The before/after examples and the scoping note made this really easy to review.

Approving as-is. This is a strict improvement: Windows users go from a wrong hint + broken ffprobe check to correct hints + a working check, and reusing getFFmpegInstallHint() is exactly the right move.

Good call leaving findFFmpeg() out — the which issue there has a wider blast radius (render.ts depends on it too) and deserves its own PR. I'll open follow-up issues for that one, plus one for making the Linux install hint distro-aware (the helper still returns sudo apt install ffmpeg for all Linux, so Arch/RHEL folks get the wrong package manager — not something to fix here).

Appreciate the contribution!

@jrusso1020
jrusso1020 merged commit 64e3735 into heygen-com:main Apr 18, 2026
12 checks passed
@Dylanwooo
Dylanwooo deleted the fix/doctor-platform-hints branch April 19, 2026 02:57
Zollicoff pushed a commit to Zollicoff/hyperframes that referenced this pull request Jul 1, 2026
The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg`
for any non-macOS platform — Windows users would see an apt command that
doesn't exist on their system, and Red Hat / Arch users got the wrong
package manager too.

`getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is
already used by render.ts) and handles darwin / linux / win32 correctly.
Use it here too.

Also rewrite checkFFprobe:
- it previously used `which ffprobe` which is not available on Windows
  (cmd uses `where`), so on Windows the check always reported "Not
  found" even when ffprobe was on PATH
- run `ffprobe -version` directly instead, which works cross-platform
  whenever ffprobe is resolvable on PATH, and surfaces the version
  string in the same style as the FFmpeg check
dahans-msft2 pushed a commit to dahans-msft2/hyperframes that referenced this pull request Aug 6, 2026
The "FFmpeg not found" hint was hardcoded to `sudo apt install ffmpeg`
for any non-macOS platform — Windows users would see an apt command that
doesn't exist on their system, and Red Hat / Arch users got the wrong
package manager too.

`getFFmpegInstallHint()` already exists in browser/ffmpeg.ts (and is
already used by render.ts) and handles darwin / linux / win32 correctly.
Use it here too.

Also rewrite checkFFprobe:
- it previously used `which ffprobe` which is not available on Windows
  (cmd uses `where`), so on Windows the check always reported "Not
  found" even when ffprobe was on PATH
- run `ffprobe -version` directly instead, which works cross-platform
  whenever ffprobe is resolvable on PATH, and surfaces the version
  string in the same style as the FFmpeg check
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.

2 participants