Repository navigation
Fix nightly Xcode selection on single-Xcode runners - #5694
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d03dcebc21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| HELPER_DEVELOPER_DIR="$APP_DEVELOPER_DIR" | ||
| HELPER_SDK_VER="$APP_SDK_VER" | ||
| echo "No pre-26 Xcode found for the Ghostty CLI helper; falling back to the app Xcode. The universal helper build and lipo verification remain required." >&2 |
There was a problem hiding this comment.
On single-Xcode runners this fallback sets HELPER_DEVELOPER_DIR to the same Xcode 26 SDK selected for the app, but the very next nightly step runs the real build-ghostty-cli-helper.sh --universal with that value as DEVELOPER_DIR. The repo's release lane still builds this helper on the macOS 15/pre-26 lane because Zig 0.15.2 cannot link it on macOS 26, so the intended single-Xcode nightly will get past selection only to fail during the required helper build rather than producing a nightly.
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR refactors nightly macOS Xcode selection from inline workflow logic into a dedicated script, adds comprehensive regression tests for Xcode selection behavior, updates the universal nightly build validation to cover a unified track instead of dual tracks, and integrates these new tests into the CI guard workflow. ChangesNightly Xcode Selection and Universal Build Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR extracts the inline nightly Xcode selection logic into
Confidence Score: 3/5Safe to merge only after confirming that build-ghostty-cli-helper.sh --universal can produce a valid arm64+x86_64 binary when DEVELOPER_DIR points to a macOS 26 SDK Xcode. The selection script and tests are clean, but the fallback silently reuses the macOS 26 SDK Xcode for the Ghostty CLI helper build. The nightly.yml app-build step still documents that zig must skip the in-Xcode x86_64 cross-link against macOS 26 SDK, yet no equivalent note exists on the standalone helper-build step. If build-ghostty-cli-helper.sh --universal shares the same limitation, the CI failure just moves from Select Xcode to Build universal Ghostty CLI helper and the fix is incomplete. scripts/select-nightly-xcodes.sh (fallback path) and .github/workflows/nightly.yml (Build universal Ghostty CLI helper step). Important Files Changed
Reviews (1): Last reviewed commit: "Fix nightly Xcode selection on single-Xc..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if [ -z "$HELPER_DEVELOPER_DIR" ]; then | ||
| HELPER_DEVELOPER_DIR="$APP_DEVELOPER_DIR" | ||
| HELPER_SDK_VER="$APP_SDK_VER" | ||
| echo "No pre-26 Xcode found for the Ghostty CLI helper; falling back to the app Xcode. The universal helper build and lipo verification remain required." >&2 | ||
| fi |
There was a problem hiding this comment.
The nightly.yml app-build step still carries a comment saying the in-Xcode zig build "would fail to cross-link x86_64 against the macOS 26 SDK" — and that step explicitly sets CMUX_SKIP_ZIG_BUILD=1 to skip it. The "Build universal Ghostty CLI helper" step also invokes a zig-driven universal build under DEVELOPER_DIR: ${{ env.HELPER_DEVELOPER_DIR }}. When the fallback here sets HELPER_DEVELOPER_DIR to the macOS 26 SDK Xcode, the standalone build-ghostty-cli-helper.sh --universal will run with that same macOS 26 SDK. If build-ghostty-cli-helper.sh shares the same zig x86_64 cross-link limitation, the CI will still fail on single-Xcode runners — just at the helper-build step instead of "Select Xcode". A brief inline comment confirming that the standalone script is exempt from the zig macOS 26 SDK limitation would make the fallback logic self-documenting.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| make_xcode "Xcode.app" "26.2" | ||
| ONLY_OUT="$TMP_DIR/only.out" | ||
| ONLY_ENV="$TMP_DIR/only.env" | ||
| run_selector "$ONLY_OUT" "$ONLY_ENV" | ||
| assert_env_line "$ONLY_ENV" "DEVELOPER_DIR=$APPS_DIR/Xcode.app/Contents/Developer" | ||
| assert_env_line "$ONLY_ENV" "HELPER_DEVELOPER_DIR=$APPS_DIR/Xcode.app/Contents/Developer" | ||
| if ! grep -Fq "falling back to the app Xcode" "$ONLY_OUT"; then | ||
| echo "FAIL: one-Xcode selection must explain the helper fallback" >&2 | ||
| cat "$ONLY_OUT" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| make_xcode "Xcode_16.app" "15.5" | ||
| DUAL_OUT="$TMP_DIR/dual.out" | ||
| DUAL_ENV="$TMP_DIR/dual.env" | ||
| run_selector "$DUAL_OUT" "$DUAL_ENV" | ||
| assert_env_line "$DUAL_ENV" "DEVELOPER_DIR=$APPS_DIR/Xcode.app/Contents/Developer" | ||
| assert_env_line "$DUAL_ENV" "HELPER_DEVELOPER_DIR=$APPS_DIR/Xcode_16.app/Contents/Developer" |
There was a problem hiding this comment.
The dual-Xcode case tests one macOS 26 Xcode and one pre-26 Xcode. There is no test with two macOS 26+ Xcodes (e.g. Xcode.app at 26.2 and Xcode_26_3.app at 26.3) to verify that the highest-ranked app Xcode is actually chosen. Without this, a regression in the sdk_rank comparison path for the app side would go undetected.
This branch was successfully deployed
1 participant
Summary
Failure fixed
Select Xcodebecause the runner only had Xcode.app with macOS SDK 26.2.Verification
./tests/test_ci_nightly_xcode_selection.shbash ./tests/test_nightly_universal_build.sh./tests/test_ci_release_sdk_lane.shgit diff --check origin/main...HEADNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes nightly signing/build toolchain selection and helper SDK fallback; mis-selection or a failed universal helper link on SDK 26 could break nightlies, though behavior is regression-tested in CI guards.
Overview
Fixes nightly macOS CI failing on runners that only ship Xcode 26 (macOS 26 SDK), where the old inline Select Xcode step hard-required a separate pre-26 Xcode for the Ghostty CLI helper.
Nightly workflow now calls
scripts/select-nightly-xcodes.shinstead of inlined bash. The script still requires a macOS 26+ SDK Xcode for the Swift app (Liquid Glass on Tahoe) and prefers a pre-26 Xcode for the universal helper when present, but falls back to the app Xcode when no pre-26 install exists—while keeping the universal helper build and lipo arm64/x86_64 checks mandatory. Workflow comments were updated to document that behavior.CI guard tests were added in
ci.yml:test_ci_nightly_xcode_selection.sh(mocked Xcode layouts: one-Xcode, dual-Xcode, missing 26+ failure) and an updatedtest_nightly_universal_build.shwired intoworkflow-guard-tests.test_ci_release_sdk_lane.shcomments now point at these nightly-specific guards instead of inline workflow steps.Reviewed by Cursor Bugbot for commit d03dceb. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fix nightly Xcode selection to work on single‑Xcode runners by moving selection into a tested script with a safe helper fallback. The app still builds with a macOS 26 SDK; the Ghostty helper prefers a pre‑26 SDK but falls back to the app Xcode, with universal build and lipo checks enforced.
Bug Fixes
Refactors
scripts/select-nightly-xcodes.shand invoked it fromnightly.yml(exportsDEVELOPER_DIRandHELPER_DEVELOPER_DIR).test_ci_nightly_xcode_selection.shfor one‑Xcode runners and strongertest_nightly_universal_build.sh; wired intoci.yml.Written for commit d03dceb. Summary will update on new commits.
Summary by CodeRabbit
Chores
Tests