ci: force rustup proxy on PATH for macos-14 x86_64 builds#1151
ci: force rustup proxy on PATH for macos-14 x86_64 builds#1151carlos-alm wants to merge 6 commits into
Conversation
macos-14 ARM runners intermittently expose a stale cargo binary that resolves to rustup-init, breaking napi-rs `cargo metadata` calls during the x86_64-apple-darwin build. Add a verification step after dtolnay/rust-toolchain that re-prepends $HOME/.cargo/bin and runs `cargo --version` so the failure surfaces immediately rather than deep inside napi build. Fixes #1136
|
Claude finished @carlos-alm's task —— View job PR Review CompleteRoot Cause AnalysisProblem Being Solved: macos-14 ARM runners intermittently expose a stale Why This Approach: The fix adds explicit PATH management after Risk Assessment: Very low risk. Changes are minimal, targeted, and properly conditional. Only affects macOS jobs and adds early validation without changing core build logic. Backlog Compliance
Code Quality Assessment✅ Strengths:
📍 Technical Details:
Critical ConcernsNone identified. This is a well-targeted infrastructure fix. Final Recommendation
Recommendation: Approve and merge. Consider monitoring CI logs to confirm the fix resolves the intermittent failures on macos-14 runners. |
Greptile SummaryThis PR adds a targeted workaround step to both
Confidence Score: 5/5Safe to merge — the change is a narrowly scoped CI step that only runs on macOS, cannot affect build outputs, and fails fast if the Rust toolchain is unhealthy. The workaround is minimal and well-understood: prepending a path entry via GITHUB_PATH is idempotent, the health checks use absolute paths so they work correctly within the same step, and the guard condition limits the step to macOS matrix entries. No production code is touched. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Setup Rust\ndtolnay/rust-toolchain] --> B{runner.os == 'macOS'?}
B -- No --> D[Rust cache\nSwatinem/rust-cache]
B -- Yes --> C["Verify cargo resolves to rustup proxy\n• echo $HOME/.cargo/bin >> $GITHUB_PATH\n• $HOME/.cargo/bin/cargo --version\n• $HOME/.cargo/bin/rustc --version"]
C -- "fail-fast: version cmd exits non-zero" --> E[❌ Build fails immediately]
C -- pass --> D
D --> F[Build native addon\nnapi build --release]
Reviews (6): Last reviewed commit: "ci: clarify macOS rustup shim workaround..." | Re-trigger Greptile |
| targets: ${{ matrix.target }} | ||
|
|
||
| - name: Verify cargo resolves to rustup proxy (macos-14 shim workaround) | ||
| if: runner.os == 'macOS' |
There was a problem hiding this comment.
Condition covers all macOS runners, not just
macos-14
runner.os == 'macOS' also matches the macos-latest / aarch64-apple-darwin matrix entry, so the PATH fixup runs for the native ARM build too. This is harmless and arguably a safety net, but the step name ("macos-14 shim workaround") and the PR description ("specifically for macos-14") are slightly misleading. If the intent is to guard only the known-broken matrix entry you could use matrix.os == 'macos-14' instead. The same applies to publish.yml.
There was a problem hiding this comment.
Addressed in 6680ffe — renamed the step to "macOS shim workaround" and updated the comment to note that the workaround intentionally applies to all macOS matrix entries. Issue #1136 also recorded an aarch64-apple-darwin occurrence (PR #1127 run), so keeping the broader runner.os == 'macOS' guard is the right call as a safety net rather than narrowing to matrix.os == 'macos-14'. The step/comment wording now matches that intent.
Greptile noted that the step name ("macos-14 shim workaround") and PR description suggested the fix only targets macos-14, while runner.os == 'macOS' also covers macos-latest/aarch64-native. Issue #1136 also recorded an aarch64-apple-darwin failure, so keeping the broader guard is correct. Update the step name and comment to reflect that the workaround intentionally applies to all macOS matrix entries as a safety net.
Summary
cargoon PATH that resolves torustup-init, breaking napi-rscargo metadatacalls during theBuild x86_64-apple-darwinjob (ci: Build x86_64-apple-darwin sporadically fails — cargo resolves to rustup-init on macos-14 #1136).dtolnay/rust-toolchainthat re-prepends$HOME/.cargo/binto$GITHUB_PATHand runscargo --version/rustc --versionso any future shim regressions fail fast and obviously instead of deep insidenapi build.build-native.ymlandpublish.yml(the matrix entry is duplicated across the two workflows).Test plan
Build Nativeagainst this branch and confirmBuild x86_64-apple-darwinsucceeds.Verify cargo resolves to rustup proxystep runs only on macOS jobs and is skipped on Linux/Windows.Closes #1136