Repository navigation
Migrate macOS CI/CD runners to Blacksmith - #4902
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates GitHub Actions workflows, the ActionLint config, and the CI guard from WarpBuild/Depot runner labels to Blacksmith macOS runner labels and changes the perf-activation benchmark to run under the console user's Aqua session. ChangesBlacksmith runner migration
🎯 3 (Moderate) | ⏱️ ~20 minutes
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (15 passed)
✨ 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 migrates all macOS CI/CD jobs from WarpBuild (
Confidence Score: 5/5Safe to merge — only CI runner labels change; no application code is touched. Every changed file is a GitHub Actions workflow or a test script that validates those workflows. The runner label swap is 1:1 in size and macOS version. The most complex addition — the launchctl asuser double-sudo pattern in perf-activation.yml — is a well-known workaround for running GUI benchmarks from a non-Aqua CI process, and is correctly structured. SPM cache keys are properly namespaced per runner to prevent cross-contamination. No application logic, signing configuration, or secrets handling is altered. No files require special attention. The perf-activation.yml Aqua session wrapper and the test_ci_self_hosted_guard.sh awk update are the most non-trivial changes but both are correctly implemented. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before["Before (WarpBuild / Depot)"]
W1[warp-macos-15-arm64-6x]
W2[warp-macos-26-arm64-6x]
D1[depot-macos-latest]
end
subgraph After["After (Blacksmith)"]
B1[blacksmith-6vcpu-macos-15]
B2[blacksmith-6vcpu-macos-26]
B3[blacksmith-6vcpu-macos-latest]
end
W1 -->|"tests, tests-build-and-lag,\nui-regressions, build-ghosttykit,\ncompat-15, terminal-nightly,\ntest-depot, perf-activation"| B1
W2 -->|"release-build, build-sign-notarize,\nbuild-sign-notarize-nightly, compat-26"| B2
D1 -->|"test-e2e default"| B1
B1 -.->|dispatch option| B2
B1 -.->|dispatch option| B3
subgraph Special["perf-activation Aqua fix"]
AQ["launchctl asuser CONSOLE_UID\n→ sudo -u CONSOLE_USER\n→ bench-window-visibility.swift\n(CGWindowListCopyWindowInfo\nnow sees on-screen windows)"]
end
B1 --> AQ
Reviews (5): Last reviewed commit: "Harden macOS runner guard diagnostics" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| default: true | ||
| type: boolean | ||
| runner: | ||
| description: "Runner OS (Depot runners for GUI activation support)" | ||
| description: "Runner OS" | ||
| required: false | ||
| default: "depot-macos-latest" | ||
| default: "blacksmith-6vcpu-macos-latest" | ||
| type: choice | ||
| options: | ||
| - blacksmith-6vcpu-macos-latest | ||
| - blacksmith-6vcpu-macos-26 | ||
| - blacksmith-6vcpu-macos-15 | ||
| - depot-macos-latest | ||
| - depot-macos-14 | ||
|
|
||
| jobs: | ||
| e2e: | ||
| runs-on: ${{ inputs.runner || 'depot-macos-latest' }} | ||
| runs-on: ${{ inputs.runner || 'blacksmith-6vcpu-macos-latest' }} | ||
| timeout-minutes: 20 | ||
| env: |
There was a problem hiding this comment.
The old description explicitly flagged that Depot runners were required for GUI activation support. The new default (blacksmith-6vcpu-macos-latest) silently drops that note, but E2E tests that depend on virtual display or GUI activation will silently regress if Blacksmith's latest image doesn't provide the same capability. The ci-macos-compat.yml matrix sets virtual_display: false for macOS 26, so if latest resolves to 26, any E2E step requiring a virtual display will break without a clear failure signal at the runner-selection level.
| terminal-nightly: | ||
| if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' | ||
| runs-on: [self-hosted, warp-macos-15-arm64-6x] | ||
| runs-on: blacksmith-6vcpu-macos-15 |
There was a problem hiding this comment.
self-hosted label silently removed
The original runner spec was [self-hosted, warp-macos-15-arm64-6x], meaning the job required a self-hosted runner. Switching to blacksmith-6vcpu-macos-15 (a managed cloud label) drops the self-hosted routing constraint entirely. If any network policy, secret scope, or tool availability was tied to the self-hosted runner context for terminal-nightly, the job may silently acquire a different environment without failing fast.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 161: Actionlint flags the new custom runner labels like "runs-on:
blacksmith-6vcpu-macos-15" as unknown; update your actionlint configuration to
allow them by adding the specific labels (e.g., blacksmith-6vcpu-macos-15) or a
matching pattern (e.g., /^blacksmith-.*$/) to the allowed self-hosted labels in
actionlint.yaml, or alternatively change the workflow runner selection to a
standard set of labels; look for the "runs-on: blacksmith-6vcpu-macos-15"
occurrences in the CI workflow and the allowed_labels/self-hosted list in
actionlint.yaml (or the equivalent key) and add the labels/pattern so actionlint
stops reporting unknown runner labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df91a6eb-59f4-4359-a1c1-6316cf4b853a
📒 Files selected for processing (10)
.github/workflows/build-ghosttykit.yml.github/workflows/ci-macos-compat.yml.github/workflows/ci.yml.github/workflows/nightly.yml.github/workflows/perf-activation.yml.github/workflows/release.yml.github/workflows/test-depot.yml.github/workflows/test-e2e.yml.github/workflows/tmux-corpus.ymltests/test_ci_self_hosted_guard.sh
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/perf-activation.yml (1)
85-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude runner identifier in SPM cache keys to prevent cross-version cache collisions.
The cache key omits the runner identifier, but the workflow allows selecting between
blacksmith-6vcpu-macos-15,-26, and-latestviaworkflow_dispatch. Different macOS versions use different Xcode/Swift SDKs and may produce incompatible compiled Swift packages. A cache entry populated on macOS 15 could be incorrectly restored on macOS 26, causing build failures or SDK mismatches. Context snippet 4 shows that test-e2e.yml correctly includes the runner in its SPM cache key pattern.🔧 Align cache keys with test-e2e.yml pattern
- name: Cache Swift packages uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 with: path: .ci-source-packages - key: spm-${{ hashFiles('cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} - restore-keys: spm- + key: spm-${{ inputs.runner || 'blacksmith-6vcpu-macos-15' }}-${{ hashFiles('cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} + restore-keys: spm-${{ inputs.runner || 'blacksmith-6vcpu-macos-15' }}-🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/perf-activation.yml around lines 85 - 90, The SPM cache key (key: spm-${{ hashFiles('cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }}) can collide across different runner images; update the actions/cache key to include the runner identifier (e.g., prepend or interpolate matrix.runs-on or github.runner) so it becomes spm-${{ matrix.runs-on }}-${{ hashFiles(...) }} (and adjust restore-keys similarly), keeping the same hashFiles expression and leaving actions/cache usage intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/perf-activation.yml:
- Around line 133-152: The screencapture probe in the "Diagnose GUI session"
step writes /tmp/probe.png but never removes it; add cleanup by removing the
probe file after the probe command (e.g., run rm -f /tmp/probe.png or set a trap
to remove /tmp/probe.png on exit) so the workflow doesn't leave stray files;
locate the screencapture invocation (screencapture -x /tmp/probe.png ...) and
add the cleanup command or trap immediately after it.
---
Outside diff comments:
In @.github/workflows/perf-activation.yml:
- Around line 85-90: The SPM cache key (key: spm-${{
hashFiles('cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved')
}}) can collide across different runner images; update the actions/cache key to
include the runner identifier (e.g., prepend or interpolate matrix.runs-on or
github.runner) so it becomes spm-${{ matrix.runs-on }}-${{ hashFiles(...) }}
(and adjust restore-keys similarly), keeping the same hashFiles expression and
leaving actions/cache usage intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 09659382-28b5-4352-8a77-2b249a9c463b
📒 Files selected for processing (1)
.github/workflows/perf-activation.yml
Switch all macOS jobs to blacksmith-6vcpu-macos-{15,26,latest}:
- ci.yml (tests, tests-build-and-lag, release-build, ui-regressions)
- build-ghosttykit, nightly, release, test-depot, perf-activation,
tmux-corpus, ci-macos-compat
- test-e2e default → blacksmith-6vcpu-macos-latest; existing
workflow_dispatch input keeps depot-macos-* as fallback options
Update tests/test_ci_self_hosted_guard.sh to assert Blacksmith runners
on the paid jobs from issue #385.
Cmd-Tab activation bench times out waiting for a visible CGWindow on Blacksmith macOS runners. perf-activation-session.py works because it talks to the debug socket, so the app process launches fine, but the Cmd-Tab bench uses CGWindowListCopyWindowInfo([.optionOnScreenOnly,...]) which only returns windows the WindowServer is rendering. Print enough runner state to decide whether to launchctl asuser the bench step or move it back to a Warp/Depot GUI-enabled runner.
On Blacksmith macOS runners (and likely any future GitHub-image-based hosted runner), the runner job runs in launchd's system bootstrap even though it is uid 501. Apps launched via NSWorkspace.openApplication still attach to the WindowServer, but CGWindowListCopyWindowInfo([.optionOnScreenOnly,...]) from a system-bootstrap process does not see those windows in the console user's Aqua session, so the Cmd-Tab benchmark times out waiting for an initial visible window. Re-enter the console user's launchd domain with launchctl asuser before invoking the swift bench so visibility polling sees the real on-screen windows. perf-activation-session.py was unaffected because it talks to the debug socket and does not need WindowServer visibility.
699370c to
7405c27
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_ci_self_hosted_guard.sh`:
- Around line 22-25: The output strings in the test (the echo lines that
currently say "FAIL: $job in $(basename "$file") must use a Blacksmith macOS
runner" and "PASS: $job Blacksmith runner is present") leak an internal provider
name; update those messages to use a generic, user-facing phrase instead (e.g.,
"macOS runner" or "expected macOS runner") so both the FAIL and PASS echoes
reference $job and $(basename "$file") but no longer include "Blacksmith".
- Around line 15-20: The AWK job-boundary test (the in_job && /^ [^[:space:]]/
branch) is too broad and treats any two-space line (including comments) as a job
end; update the boundary detection in the AWK snippet that uses the job variable
and in_job flag so it only treats a new top-level YAML key as the end of the job
(i.e., require the two-space prefix followed by non-space characters and a
trailing colon) instead of any two-space line, so comments or blank lines inside
the job do not prematurely clear in_job.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 267b18cd-e368-43dc-a5c0-f40ebf28d968
📒 Files selected for processing (11)
.github/actionlint.yaml.github/workflows/build-ghosttykit.yml.github/workflows/ci-macos-compat.yml.github/workflows/ci.yml.github/workflows/nightly.yml.github/workflows/perf-activation.yml.github/workflows/release.yml.github/workflows/test-depot.yml.github/workflows/test-e2e.yml.github/workflows/tmux-corpus.ymltests/test_ci_self_hosted_guard.sh
|
@coderabbitai resume @coderabbitai review |
|
✅ Actions performedReviews resumed. Review triggered.
|
Stale CodeRabbit review. Addressed in a61c666; fresh CodeRabbit status check completed successfully.
Re-applies the WarpBuild/Depot -> Blacksmith migration that landed as #4902 and was reverted by #4926. The revert was purely a Blacksmith macOS capacity problem (every SKU queued 8m+/65m+ while GitHub-hosted picked up in <10s), not a defect. A push-triggered probe on this branch confirmed all three Blacksmith macOS SKUs (15/26/latest) now pick up jobs in ~16s, so the migration is safe to re-attempt. Switches every macOS job to blacksmith-6vcpu-macos-{15,26,latest}: - ci.yml (tests, tests-build-and-lag, release-build, ui-regressions) - build-ghosttykit, nightly, release, test-depot, perf-activation, tmux-corpus (drops the self-hosted label), ci-macos-compat - test-e2e default -> blacksmith-6vcpu-macos-15; keeps depot-macos-* as fallback dispatch options and the Depot identity guard (now correctly skipped on the Blacksmith default) - re-adds .github/actionlint.yaml allowlist for the Blacksmith labels - re-adds the launchctl asuser Aqua-session wrapper for the Cmd-Tab perf bench (Blacksmith runners run in launchd's system bootstrap) - tests/test_ci_self_hosted_guard.sh asserts Blacksmith macOS runners Removes the temporary capacity probe used to gate this re-attempt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-applies the WarpBuild/Depot -> Blacksmith migration (originally #4902, reverted by #4926 for Blacksmith macOS capacity, now recovered). Every paid macOS job routes through the MACOS_RUNNER_15/MACOS_RUNNER_26 repo variables with a Warp literal fallback, so the provider can be flipped via a repo-variable change with no PR. Manual perf/e2e default to 'auto' and follow the variable too, with Warp selectable. See docs/macos-ci-runners.md.
This branch was successfully deployed
1 participant
Summary
Switches every macOS CI job from WarpBuild (
warp-macos-*-arm64-6x) and Depot (depot-macos-latest) to Blacksmith (blacksmith-6vcpu-macos-{15,26,latest}).Workflows touched
ci.yml— tests, tests-build-and-lag, release-build, ui-regressionsbuild-ghosttykit.ymlci-macos-compat.yml(matrix os values)nightly.yml(build-sign-notarize-nightly)release.yml(build-sign-notarize)test-depot.ymlperf-activation.yml(default + choice options)tmux-corpus.yml(terminal-nightly; also drops theself-hostedlabel)test-e2e.yml(defaultblacksmith-6vcpu-macos-latest, keepsdepot-macos-{latest,14}as fallback options on the workflow_dispatch input)tests/test_ci_self_hosted_guard.shupdated to assert Blacksmith runners on the paid jobs from #385.Size mapping is 1:1: every 6x WarpBuild job becomes a 6vcpu Blacksmith job. macOS 15 vs 26 preserved per job.
Test plan
tests/test_ci_self_hosted_guard.sh(passes locally)ci-macos-compat.ymlto confirm both Blacksmith images bootnightly.ymlwithforce=trueand confirm sign+notarize completestest-e2e.ymlwith the new default runnerrelease.ymldry run (or next real release) onblacksmith-6vcpu-macos-26Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches signing/notarization, release, and nightly pipelines plus GUI perf benchmarks; misconfigured runners or the Aqua-session workaround could break macOS CI or artifact publishing until validated on Blacksmith.
Overview
Moves macOS CI/CD off WarpBuild (
warp-macos-*-arm64-6x) and Depot defaults onto Blacksmith labels (blacksmith-6vcpu-macos-15,-26,-latest) across build, test, release, nightly, perf, e2e, and corpus workflows, with a 1:1 mapping from former 6x Warp jobs to 6vcpu Blacksmith jobs.Adds
.github/actionlint.yamlso actionlint accepts the new self-hosted runner labels.tests/test_ci_self_hosted_guard.shnow requires Blacksmith runners on the paid macOS jobs from issue #385 (including matrixosin compat).perf-activation.ymlchanges defaults andworkflow_dispatchrunner choices to Blacksmith, scopes SwiftPM cache keys by runner, and runs the Cmd-Tab visibility bench inside the console user’s Aqua session vialaunchctl asusersoCGWindowListsees on-screen windows.test-e2e.ymldefaults to Blacksmith while keeping Depot as optional dispatch choices; SPM cache keys follow the selected runner.tmux-corpus.ymlterminal-nightlydrops theself-hosted+ Warp label pair in favor of a single Blacksmith runner.Reviewed by Cursor Bugbot for commit a61c666. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Chores
Tests