Repository navigation
Use Blacksmith as macOS CI default - #5019
lawrencecchen wants to merge 7 commits into
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.
|
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughReplace WarpBuild macOS runner fallbacks with Blacksmith labels across workflows, remove Warp labels from the actionlint allowlist, update docs to reflect MACOS_RUNNER_* → Blacksmith fallbacks, and tighten CI guard tests to accept only Blacksmith or repo-variable runner references. ChangesmacOS runner migration from WarpBuild to Blacksmith
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (16 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 hardens the macOS CI runner choice by replacing all
Confidence Score: 5/5Safe to merge; all changes are CI infrastructure, with no application code modified. Every workflow, guard test, and doc update is internally consistent. The auto dispatcher logic, Depot identity guard, SPM cache keys, concurrency groups, and actionlint allowlist all use the same hardcoded Blacksmith labels. No logic errors or mismatched expressions were found across the 13 changed files. No files require special attention. The guard test and release-lane assertions were updated in lockstep with the workflow changes. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Workflow triggered] --> B{inputs.runner}
B -->|empty or 'auto'| C[blacksmith-6vcpu-macos-15]
B -->|explicit choice| D{Choice}
D --> E[blacksmith-6vcpu-macos-15]
D --> F[blacksmith-6vcpu-macos-26]
D --> G[blacksmith-6vcpu-macos-latest]
D --> H[depot-macos-latest / depot-macos-14]
C --> I{starts with depot-macos-?}
E --> I
F --> I
G --> I
H --> I
I -->|Yes| J[Validate Depot runner identity]
I -->|No| K[Skip identity guard]
J --> L[Run job on selected runner]
K --> L
Reviews (5): Last reviewed commit: "Hardcode Blacksmith runners; drop MACOS_..." | Re-trigger Greptile |
| Remove the override and use the checked-in Blacksmith defaults: | ||
|
|
||
| ```bash | ||
| gh variable delete MACOS_RUNNER_15 --repo manaflow-ai/cmux | ||
| gh variable delete MACOS_RUNNER_26 --repo manaflow-ai/cmux | ||
| ``` |
There was a problem hiding this comment.
No documented escape hatch for Blacksmith capacity outages
The PR description explicitly cites a past event where Blacksmith queues hit 55–126 minutes and caused a full revert (PR #4926). The old docs told operators to gh variable set … warp-macos-15-arm64-6x; that guidance is now gone, and Warp is removed from all workflow dropdowns and the actionlint allowlist. If Blacksmith queues surge again, an operator in a time-sensitive release has no documented runner to fall back to. The only option—setting MACOS_RUNNER_15/MACOS_RUNNER_26 to a different provider—is correct in principle, but the docs no longer say what labels are valid alternatives or where to find them.
Consider adding a brief "Emergency override" section that names at least one alternative runner label (e.g., a Warp or GitHub-hosted fallback) and notes that the label must also be added to .github/actionlint.yaml before use.
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!
|
Paused merge after the refreshed PR run stayed queued on Blacksmith macOS runners for over 11 minutes without a runner assignment. Live repo variables have been restored to Warp for now: |
# Conflicts: # .github/workflows/nightly.yml # .github/workflows/release.yml # docs/macos-ci-runners.md
# Conflicts: # .github/workflows/ci.yml # .github/workflows/release.yml
There was a problem hiding this comment.
2 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/tmux-corpus.yml">
<violation number="1" location=".github/workflows/tmux-corpus.yml:64">
P2: Hardcoded runner label removes the `vars.MACOS_RUNNER_15` override, contradicting the PR's stated design of "Auto resolution now follows MACOS_RUNNER_15 first, then Blacksmith." The original pattern allowed operators to redirect this job by setting a repo variable if Blacksmith has queue or capacity issues; the new hardcoded label removes that flexibility entirely.</violation>
</file>
<file name="docs/macos-ci-runners.md">
<violation number="1" location="docs/macos-ci-runners.md:3">
P2: The docs overstate runner behavior: Blacksmith is the default, but manual workflows can still run on Depot runners.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| terminal-nightly: | ||
| if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' | ||
| runs-on: ${{ vars.MACOS_RUNNER_15 || 'warp-macos-15-arm64-6x' }} | ||
| runs-on: blacksmith-6vcpu-macos-15 |
There was a problem hiding this comment.
P2: Hardcoded runner label removes the vars.MACOS_RUNNER_15 override, contradicting the PR's stated design of "Auto resolution now follows MACOS_RUNNER_15 first, then Blacksmith." The original pattern allowed operators to redirect this job by setting a repo variable if Blacksmith has queue or capacity issues; the new hardcoded label removes that flexibility entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/tmux-corpus.yml, line 64:
<comment>Hardcoded runner label removes the `vars.MACOS_RUNNER_15` override, contradicting the PR's stated design of "Auto resolution now follows MACOS_RUNNER_15 first, then Blacksmith." The original pattern allowed operators to redirect this job by setting a repo variable if Blacksmith has queue or capacity issues; the new hardcoded label removes that flexibility entirely.</comment>
<file context>
@@ -61,7 +61,7 @@ jobs:
terminal-nightly:
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
- runs-on: ${{ vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15' }}
+ runs-on: blacksmith-6vcpu-macos-15
timeout-minutes: 30
steps:
</file context>
| runs-on: blacksmith-6vcpu-macos-15 | |
| runs-on: ${{ vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15' }} |
| # macOS CI runners | ||
|
|
||
| All paid macOS CI/CD jobs pick their runner from two repository variables instead of a hardcoded label: | ||
| All paid macOS CI/CD jobs run on Blacksmith. The runner labels are hardcoded in the workflows (git-tracked); there is no repo-variable override. |
There was a problem hiding this comment.
P2: The docs overstate runner behavior: Blacksmith is the default, but manual workflows can still run on Depot runners.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/macos-ci-runners.md, line 3:
<comment>The docs overstate runner behavior: Blacksmith is the default, but manual workflows can still run on Depot runners.</comment>
<file context>
@@ -1,46 +1,22 @@
# macOS CI runners
-All paid macOS CI/CD jobs pick their runner from two repository variables instead of a hardcoded label:
+All paid macOS CI/CD jobs run on Blacksmith. The runner labels are hardcoded in the workflows (git-tracked); there is no repo-variable override.
-- `MACOS_RUNNER_15` for jobs that build the real universal Release app, including nightly, stable release, `release-build`, and the e2e/perf defaults.
</file context>
| All paid macOS CI/CD jobs run on Blacksmith. The runner labels are hardcoded in the workflows (git-tracked); there is no repo-variable override. | |
| All paid macOS CI/CD jobs default to Blacksmith. Runner labels are git-tracked in workflows, manual workflows can still choose explicit runner options, and there is no repo-variable override. |
Summary
Make Blacksmith the checked-in default for paid macOS CI jobs when
MACOS_RUNNER_15/MACOS_RUNNER_26are unset. Remove Warp runner choices from manual workflows and actionlint config. Update runner docs and the CI guard to treat Blacksmith as the supported default.I also set the live repo variables to
blacksmith-6vcpu-macos-15andblacksmith-6vcpu-macos-26, so newmainruns use Blacksmith immediately.Queue check
blacksmith-6vcpu-macos-15: 16s queue, probe run canceled after startblacksmith-6vcpu-macos-26: 14s queue, probe run canceled after startTesting
tests/test_ci_self_hosted_guard.shactionlint -shellcheck= -config-file .github/actionlint.yaml .github/workflows/ci.yml .github/workflows/ci-macos-compat.yml .github/workflows/build-ghosttykit.yml .github/workflows/nightly.yml .github/workflows/release.yml .github/workflows/test-depot.yml .github/workflows/tmux-corpus.yml .github/workflows/perf-activation.yml .github/workflows/test-e2e.ymlFull
actionlintwith shellcheck still reports existing shellcheck findings in unrelated workflow scripts.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes where all paid macOS builds and tests run (release, nightly, CI) with no repo-variable escape hatch; misconfiguration or Blacksmith outages require a workflow PR to fix.
Overview
Paid macOS CI/CD now hardcodes Blacksmith runner labels in workflows instead of routing through
MACOS_RUNNER_15/MACOS_RUNNER_26with Warp fallbacks. Typical jobs useblacksmith-6vcpu-macos-15; release/CI app SDK validation and compat’s macOS 26 matrix leg useblacksmith-6vcpu-macos-26. Warp labels are removed fromactionlintand from manualrunnerchoices inperf-activation.ymlandtest-e2e.yml;autoand related run-name, concurrency, and SPM cache keys resolve toblacksmith-6vcpu-macos-15.docs/macos-ci-runners.mdis rewritten for git-tracked labels and PR-based provider changes. Guard scripts require Blacksmith labels only and assert the macOS 15 helper / macOS 26 app split on release and CI lanes.Reviewed by Cursor Bugbot for commit 638a8c0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Hardcodes Blacksmith as the paid macOS CI/CD runner and removes the
MACOS_RUNNER_15/MACOS_RUNNER_26override variables. Manual “auto” choices, cache keys, and run names now resolve toblacksmith-6vcpu-macos-15/blacksmith-6vcpu-macos-26; Warp labels are removed.Refactors
runs-onfallbacks with Blacksmith labels in CI, release, nightly, compat matrix, e2e, perf, GhosttyKit, Depot tests, and tmux workflows.perf-activation.ymlandtest-e2e.yml“auto” now uses Blacksmith; updated run names, concurrency groups, and SPM cache keys to match..github/actionlint.yamland manualrunnerinputs; keptdepot-macos-*options and the Depot identity guard.Docs & Tests
docs/macos-ci-runners.mdto state runners are hardcoded in git; switching providers requires a PR; added actionlint label guidance.tests/test_ci_self_hosted_guard.shto require Blacksmith labels; updatedtests/test_ci_release_sdk_lane.shto assert Blacksmith defaults for macOS 15 helper and macOS 26 app builds.Written for commit 638a8c0. Summary will update on new commits.
Summary by CodeRabbit
Chores
Documentation
Tests