Skip to content

fix(cli): no macOS developer-tools prompt from git, xcrun or cc checks on a Mac without the tools - #7257

Merged
miguel-heygen merged 4 commits into
mainfrom
fix/macos-tool-stubs
Nov 9, 2026
Merged

miguel-heygen merged 4 commits into
mainfrom
fix/macos-tool-stubs

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

What

On a Mac without the developer tools, the CLI no longer opens macOS's "install the command line developer tools" dialog when it checks for git, Swift or a C compiler. It now sees that the tool isn't there and skips, saying why, as it already does when git is missing on other systems.

Why

A report: on macOS without Xcode's tools, macOS popped the install-developer-tools dialog during a CLI run. Until those tools are installed, macOS puts stubs in /usr/bin for git, xcrun, cc and others: running one opens that dialog and fails. Three CLI checks run one just to learn whether the tool exists:

  • skills (and init/update, which install skills) ran git --version.
  • The audio describe provider (Apple SoundAnalysis) ran xcrun --find swiftc to see whether it can build its helper.
  • The whisper install found /usr/bin/git with which, then ran git clone, then a cmake build that compiles with /usr/bin/cc.

I traced beats <file> --json, the command in the report, on a macOS runner with CLI 0.9.16: it starts only which, ffprobe, ffmpeg and the telemetry flush, none of them a stub. So these three checks are what can open the dialog, not beats itself.

How

  • utils/appleStubs.ts decides, without running anything, whether a command is one of those stubs and whether the tools are installed. It reads where xcode-select points (DEVELOPER_DIR, else /var/db/xcode_select_link; with neither, /Applications/Xcode.app/Contents/Developer or the Command Line Tools folder, as xcode-select itself falls back) and checks that folder has a usr/bin. A tool outside /usr/bin (Homebrew's git) or off macOS is always fine.
  • Callers pass a path or a bare name. On a Mac only, a bare name is looked up on PATH (the first executable file, nothing run); a name not found counts as the /usr/bin stub. All of it uses POSIX paths, since these are macOS paths on whatever host runs the tests.
  • The three checks ask it first: skills treats a stub git as missing (it already skips with "git not available"), the Apple provider reports "no Swift compiler", and whisper neither clones with a stub git nor builds with a stub compiler.

Test plan

  • Unit tests added/updated

  • Manual testing performed

  • Documentation updated (if applicable)

  • Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug

  • fails without the fix: skills.test.ts "bare skills on a Mac without the developer tools skips without running macOS's git stub" fails with main's skills.ts (expected [ 'npx', 'git' ] to not include 'git': main runs the stub) and passes here, 3 runs in a row.

  • appleStubs.test.ts (6 tests, 3 runs): stubs refused with no tools; allowed with the Command Line Tools, with only Xcode.app and nothing chosen (fails with the Xcode.app fallback removed), with an Xcode chosen by xcode-select or DEVELOPER_DIR; Homebrew git, non-stub /usr/bin tools and other systems always allowed; a bare name resolved on PATH without running anything. The fake Mac uses POSIX paths, so the tests mean the same on Windows (CI's Windows lane is the check; I ran Linux only).

  • The callers' tests: skills.test.ts, skills.installTarget.test.ts, src/whisper, src/audio/analysis/providers (306 pass, 1 skipped). oxfmt --check on the 6 touched files, scripts/comment-ratchet.mjs with the new files included (4 source files at or under their share), and tsc reports no errors in the touched files.

  • skills.test.ts gives every test a developer folder that exists, so its darwin cases test npx on any host. With git resolving to /usr/bin/git (as on CI) the darwin npx test failed without it (expected undefined to be 'npx') and skills.test.ts plus skills.installTarget.test.ts pass with it (51 tests, 3 runs).

  • Not covered: a real Mac without the tools; the checks are pure path reads, tested with a stand-in filesystem. The whisper and provider wiring are one-line guards with no test of their own.

@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.

Requesting changes at 2406e2ab.

The approach is right: deciding from paths, never running the stub. The fails without the fix: line names skills.test.ts "bare skills on a Mac without the developer tools…", and that test does exercise the guard. Three things block it:

  1. CI is red because of this diff. The body says oxfmt --check and the comment ratchet are clean, but on this head:
    • Preflight format:check flags packages/cli/src/utils/appleStubs.test.ts. That fails Windows render verification and preview-regression.
    • Comments: NEW file over its package's comment share: packages/cli/src/utils/appleStubs.ts is 10.2% (6/59), the package is 9.7%.
  2. A Mac that has only Xcode.app is treated as having no tools. developerDir() uses DEVELOPER_DIR, then /var/db/xcode_select_link, then /Library/Developer/CommandLineTools. When nothing has been chosen with xcode-select -s, the link may not exist. That is typical when Xcode comes from the App Store and the separate CLT package was never installed. xcode-select / xcrun then fall back to /Applications/Xcode.app/Contents/Developer, and /usr/bin/git runs fine. With this PR, skills/init/update skip with "git not available", the Apple provider says there is no Swift compiler, and whisper won't build from source, all on a fully provisioned Mac. That turns a dialog fix into a silent capability loss. Please add the /Applications/Xcode.app/Contents/Developer fallback before the CLT default, and add a case for it in appleStubs.test.ts.
  3. Minor, not blocking: onPath splits on : and doesn't check that the file is executable. It's harmless today because the result only matters on darwin. Noting it in case someone reuses it.

Fix 1 and 2 and I'll re-review the new head.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 98 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

Unstable (1)

  • seqrotatenudge-none-pct-r0-root-z100: tracking 0.03, pressJump 0, drop 0, reload 3.03, render 0.04, renderKey -, undo false, teleport true / tracking 0.03, pressJump 0, drop 0, reload 0, render 0.08, renderKey -, undo true, teleport true / tracking 0.03, pressJump 0, drop 0, reload 0, render 0.08, renderKey -, undo true, teleport true

@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.

Requesting changes at 8f6d0c62.

The code changes answer all three earlier points:

  • developerDirs() now falls back to /Applications/Xcode.app/Contents/Developer before the CLT folder, and there's a test for it.
  • onPath checks X_OK and splits PATH on path.delimiter.
  • Formatting and the comment ratchet pass (Comments is green).

The fails without the fix: line still names a valid test.

Blocker: a test this PR breaks fails on every run. In CI, skills.test.ts › "hyperframes skills › uses darwin-compatible npx command for preflight and the full install" fails at skills.test.ts:286 with expected undefined to be 'npx'. It fails in Flake gate (10/10 runs), in Test (cli 2/2), and in both CLI: npx shim jobs (ubuntu + windows). That test calls setPlatform("darwin") on a Linux/Windows runner. There, neither /Applications/Xcode.app/.../usr/bin nor /Library/Developer/CommandLineTools/usr/bin exists, so the new guard treats git as the stub and skills skips before npx ever runs. Give that test a developer folder that exists, for example vi.stubEnv("DEVELOPER_DIR", <tmpdir containing usr/bin>), so it keeps testing the npx command it's meant to test. While you're there, look for any other setPlatform("darwin") cases in the touched suites (skills.installTarget, whisper, the providers) that could hit the same guard.

Once CI is green on the new head I'll approve.

@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.

Requesting changes at 3fdbc808.

The skills-test fix is right. A temp DEVELOPER_DIR with usr/bin is stubbed in beforeEach, and the "fails without the fix" test still overrides it with a nonexistent folder. That test now passes on ubuntu, windows and macOS in Flake gate, Test (cli 2/2) and the three npx shim jobs. CI's first attempt failed in Test (runtime) and Test (cli 1/2), on a browserManager timeout and a Chrome WS-endpoint launch timeout, neither touching this diff. I re-ran those failed jobs and CI is now green.

Blocker: Windows render verification › "Tests on windows-latest: engine-cli-1" fails 4 tests in the new src/utils/appleStubs.test.ts:

  • "runs them once the Command Line Tools are installed"
  • "runs them on a Mac with only Xcode.app…"
  • "follows the developer folder xcode-select chose, or DEVELOPER_DIR"
  • onPath › "finds the first executable on PATH…"

The cause is that appleStubs.ts builds paths with the host's node:path. On Windows, join(dir, "usr", "bin") produces backslashes, so it never matches the fake Mac's /…/usr/bin paths. delimiter is ;, so "/opt/homebrew/bin:/usr/bin" stays one entry. Production on a real Mac is fine, but this is a required lane.

Suggested fix: use node:path/posix (join, dirname, basename) for the macOS developer-folder logic, since those paths are always POSIX. For onPath, either take the delimiter from the host (h.platform === "win32" ? ";" : ":") or give the test's fake host a PATH and delimiter that match. Then re-run the Windows lane.

@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.

Approving at ebcddd43. No blockers remain.

  • Windows fix: appleStubs.ts now uses node:path/posix, and the Mac PATH lookup splits on : inside the module. Callers pass a bare name ("git", "cc", "xcrun"), which resolves on PATH or falls back to /usr/bin/<name>, so the old cross-platform onPath export is gone. Off macOS it still returns true before any lookup. The Homebrew-first vs /usr/bin cases are now covered through runsWithoutInstallPrompt("git", …).
  • Earlier points: the Xcode.app fallback, the executable check on PATH, formatting and the comment share all hold.
  • fails without the fix: the line still names the skills.test.ts stub-git case, and the guard still makes it pass.
  • CI on this head:
    • Green: Windows render verification, Flake gate, Test (cli 1/2 and 2/2), the npx shims (ubuntu, windows, macOS), Comments, preview-regression and CodeQL.
    • Red: only CI › Build, on @hyperframes/producer unpacked size 3112478 bytes is over its budget of 3081613. main's latest CI (968f97fe) fails with the same byte count, and this diff doesn't touch producer, so it comes from main.

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>
@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 9, 2026
Merged via the queue into main with commit c149730 Nov 9, 2026
91 of 93 checks passed
@miguel-heygen
miguel-heygen deleted the fix/macos-tool-stubs branch November 9, 2026 17:22
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 9, 2026
…n-com#7110, heygen-com#7000; advance watermark

Eight commits, PR heygen-com#7452 and ten pin moves are recorded with their evidence
in the continued ledger, DECISIONS and UPSTREAM. Adopted: heygen-com#7249 (template
entries detected structurally), heygen-com#7110 (whisper nan/inf output), heygen-com#7000
(engine probe test on fake timers). Deferred: heygen-com#7444, heygen-com#7452. Not applicable:
heygen-com#7257, heygen-com#7443, heygen-com#7450. The watermark moves to c149730, PR heygen-com#7452, and the
nine moved pins plus heygen-com#7434 are re-pinned at their current heads.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
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