Skip to content

ci: force rustup proxy on PATH for macos-14 x86_64 builds#1151

Open
carlos-alm wants to merge 6 commits into
mainfrom
fix/1136-cargo-rustup-init-shim
Open

ci: force rustup proxy on PATH for macos-14 x86_64 builds#1151
carlos-alm wants to merge 6 commits into
mainfrom
fix/1136-cargo-rustup-init-shim

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • macos-14 ARM runners intermittently expose a stale cargo on PATH that resolves to rustup-init, breaking napi-rs cargo metadata calls during the Build x86_64-apple-darwin job (ci: Build x86_64-apple-darwin sporadically fails — cargo resolves to rustup-init on macos-14 #1136).
  • Add a step right after dtolnay/rust-toolchain that re-prepends $HOME/.cargo/bin to $GITHUB_PATH and runs cargo --version / rustc --version so any future shim regressions fail fast and obviously instead of deep inside napi build.
  • Same step added to both build-native.yml and publish.yml (the matrix entry is duplicated across the two workflows).

Test plan

  • Manually dispatch Build Native against this branch and confirm Build x86_64-apple-darwin succeeds.
  • Confirm Verify cargo resolves to rustup proxy step runs only on macOS jobs and is skipped on Linux/Windows.

Closes #1136

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
Copy link
Copy Markdown

claude Bot commented May 18, 2026

Claude finished @carlos-alm's task —— View job


PR Review Complete

Root Cause Analysis

Problem Being Solved: macos-14 ARM runners intermittently expose a stale cargo binary on PATH that resolves to rustup-init instead of the proper rustup proxy, breaking cargo metadata calls during napi-rs builds for the x86_64-apple-darwin target.

Why This Approach: The fix adds explicit PATH management after dtolnay/rust-toolchain setup to ensure $HOME/.cargo/bin takes precedence, with immediate verification that cargo/rustc resolve correctly. This provides fast failure detection instead of cryptic errors deep inside napi build.

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

  • Zero-dep: ✅ No new runtime dependencies
  • Foundation-aligned: ✅ Pure CI infrastructure fix, no core principle violations
  • Problem-fit: 2/5 — CI reliability improvement, doesn't directly address core "AI navigation" problem
  • Breaking: No — purely additive CI fix
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Code Quality Assessment

✅ Strengths:

  • Identical implementation across both workflows (build-native.yml and publish.yml) maintains consistency
  • Proper conditional execution (if: runner.os == 'macOS') prevents unnecessary execution on Linux/Windows
  • Explicit absolute path usage ("$HOME/.cargo/bin/cargo") avoids PATH resolution issues
  • Comprehensive verification of both cargo and rustc
  • Clear, descriptive comments explaining the workaround and issue reference (ci: Build x86_64-apple-darwin sporadically fails — cargo resolves to rustup-init on macos-14 #1136)
  • Fails fast with obvious error rather than obscure failure in napi build

📍 Technical Details:

  • Lines 70-80 in build-native.yml, 179-189 in publish.yml: Step added after rust-toolchain setup
  • Uses echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" to prepend to GitHub Actions PATH
  • Shell verification commands will fail immediately if binary is corrupted/mislinked
  • No impact on non-macOS builds due to conditional execution

⚠️ Minor Considerations:

  • Step name could be more concise, but descriptive name aids debugging
  • Could potentially add Windows verification in future if similar issues arise there
  • No shell security concerns - paths are well-controlled and no user input

Critical Concerns

None identified. This is a well-targeted infrastructure fix.

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Excellent solution to a legitimate infrastructure problem. The implementation is clean, safe, and properly scoped. While this doesn't directly improve AI agent navigation (the core product problem), it's essential CI infrastructure maintenance that prevents build failures. The approach correctly addresses the root cause rather than masking symptoms.

Recommendation: Approve and merge. Consider monitoring CI logs to confirm the fix resolves the intermittent failures on macos-14 runners.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR adds a targeted workaround step to both build-native.yml and publish.yml that re-prepends $HOME/.cargo/bin to GITHUB_PATH immediately after dtolnay/rust-toolchain, then verifies cargo and rustc resolve via their absolute paths. This guards against intermittent macOS runner PATH pollution where a stale cargo resolving to rustup-init was breaking napi-rs cargo metadata calls.

  • The workaround is guarded by runner.os == 'macOS' and applied consistently to both workflow files, covering all macOS matrix entries as a safety net.
  • Health-check invocations use explicit absolute paths (\"$HOME/.cargo/bin/cargo\" --version), which correctly bypasses PATH resolution within the current step while GITHUB_PATH takes effect only for subsequent steps.

Confidence Score: 5/5

Safe 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

Filename Overview
.github/workflows/build-native.yml Adds a macOS shim workaround step that prepends $HOME/.cargo/bin to GITHUB_PATH and runs absolute-path health checks for cargo/rustc after dtolnay/rust-toolchain; logic and placement are correct.
.github/workflows/publish.yml Identical macOS shim workaround step added to the build-native matrix job in publish.yml; mirrors build-native.yml faithfully.

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]
Loading

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

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

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

carlos-alm and others added 5 commits May 17, 2026 19:12
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.
@carlos-alm
Copy link
Copy Markdown
Contributor Author

@greptileai

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.

ci: Build x86_64-apple-darwin sporadically fails — cargo resolves to rustup-init on macos-14

1 participant