Skip to content

ci: spread pull-request app-host shards over several runner pools - #14115

Closed
teamleaderleo wants to merge 1 commit into
mainfrom
claude/practical-wright-mgh070
Closed

teamleaderleo wants to merge 1 commit into
mainfrom
claude/practical-wright-mgh070

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Problem. MACOS_RUNNER_PR moves the whole pull-request lane to one pool. At 02:24Z it was set to macos-15. From then on every PR macOS job went to hosted macos-15 (3 vCPU), and Blacksmith macos-15, Blacksmith macos-26 and hosted macos-26 took no PR work (finding on #14097). A runs-on label array must match every label, so one job cannot target "any of these pools". #14097 closed with only the probe, so no routing change existed.

Change. For same-repository pull requests, app-host-unit-tests reads a new repository variable, MACOS_RUNNER_PR_SHARDS. It is a JSON object from shard number to runner label:

gh variable set MACOS_RUNNER_PR_SHARDS --repo manaflow-ai/cmux -b \
  '{"1":"blacksmith-6vcpu-macos-15","2":"blacksmith-6vcpu-macos-26","3":"macos-15","4":"macos-26","5":"blacksmith-6vcpu-macos-15","6":"blacksmith-6vcpu-macos-26","7":"macos-26","8":"macos-15"}'
  • A shard the object doesn't name, a fork PR, and every other event keep the existing MACOS_RUNNER_PR expression. Unset means today's behavior, and rollback is unsetting the variable.
  • The lookup is keyed on matrix.shard and doesn't touch the matrix line, so it composes with ci: run only the edited suites when a diff edits cmuxTests/ #14083: its shard 8 gets a pool from key "8".
  • Compile admission still runs once, on MACOS_RUNNER_PR. app_host_test_products.py restore checks only revision, exact xcodebuild -version and architecture, so one product feeds shards on every pool, as long as each pool carries the Xcode that CMUX_CI_XCODE_APP_PR pins (currently /Applications/Xcode_26.3.app).
  • Hosted-label safety. The map may name bare macos-26, which fleet runners also carry. A new first step, Validate shard runner pool, runs before checkout and fails unless a macos-* request landed with runner.environment == github-hosted. It also writes the requested and actual runner for each shard to the step summary, so pool behavior can be compared from real runs.
  • runner_label_policy.py (health report) reads the object label by label. It accepts bare macos-NN only in this variable, and it reports an unreadable object as drift, not as clean.
  • check_app_host_shard_pool_identity in tests/test_ci_self_hosted_guard.sh requires the fork gate, the matching expression in REQUESTED_RUNNER, and an assertion that runs before checkout.
  • swift-package-tests stays on its dual-Xcode lane. docs/ci-runners.md documents the variable.

Not verified: whether Blacksmith macos-26 ships Xcode_26.3.app. #13923 records that image shipping 26.5. If 26.3 is missing, shards mapped there fail at Select Xcode within their first minute; remove them from the object. The first mixed-pool run settles this.

Overlap: #14107 edits the same runs-on lines to add a fork clause. Whichever lands second needs a small textual rebase. The fork gate here is already inside the new clause.

Testing

  • tests/test_runner_label_policy.py: 27 pass, including 6 new shard-map cases (the four-pool map is clean, bare macos-26 alone is still drift, a fleet label in the map names its shard, malformed maps count as drift).
  • tests/test_ci_self_hosted_guard.sh passes. I also checked that the new guard fails when the github-hosted assertion is broken.
  • All tests/test_ci_*.py / tests/test_ci_*.sh pass except test_ci_canonical_build_root.py and test_ci_sparkle_build_monotonic.sh, which fail the same way on unmodified main.
  • actionlint 1.7.12: clean on ci-macos.yml and ci-health-report.yml.
  • Not yet run on a Mac. The end-to-end check is: merge, set the variable, then watch one full-suite PR's shards land on all four pools and restore the single admission product.

Demo Video

N/A (CI routing only).

Checklist

  • I tested the change locally
  • I added or updated tests for behavior changes
  • I updated docs/changelog if needed

🤖 Generated with Claude Code

https://claude.ai/code/session_01EduXdN9PKnGsMQztJK7WeE


Generated by Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Spreads app-host-unit-tests pull-request shards over several runner pools instead of sending the whole lane to one pool via MACOS_RUNNER_PR, so hosted macos-15 stops hogging every PR job while Blacksmith 15/26 and hosted macos-26 sit idle.

  • Adds MACOS_RUNNER_PR_SHARDS, a JSON map from shard number to runner label, read for same-repository pull requests; unset keys, fork PRs, and all other events keep the existing MACOS_RUNNER_PR lane, and unsetting the variable rolls back.
  • Compile admission still runs once; a shard restores the product on any pool carrying the Xcode that CMUX_CI_XCODE_APP_PR pins. Whether Blacksmith macos-26 ships that Xcode is unverified, so mapped shards may fail in their first minute until confirmed.
  • The map may name bare GitHub-hosted labels like macos-26, which self-hosted fleet runners also carry; a new step fails before checkout unless a macos-* request lands github-hosted, then records requested vs actual runner in the step summary.
  • The runner label policy checks the map label by label, accepts bare macos-NN only there, and reports malformed maps as drift; tests and docs/ci-runners.md cover the new behavior.

Written for commit d6b1b78. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Same-repository pull request macOS test shards can now be assigned to different runner pools individually. Shards without a specific assignment continue to use the existing runner selection.
    • Runner assignments are checked before checkout to ensure macOS runner labels use the required hosted environment.
  • Documentation
    • Added guidance on configuring shard-specific runner pools and the requirements they must meet.

MACOS_RUNNER_PR moves the whole pull-request lane to one pool, so hosted
macos-15 took every PR job while Blacksmith 15/26 and hosted macos-26 sat
idle. A runs-on label array has to match every label, so it cannot mean
"any of these pools".

app-host-unit-tests now reads MACOS_RUNNER_PR_SHARDS, a JSON object from
shard number to runner label, for same-repository pull requests. A shard
it does not name, a fork, and every other event keep the existing lane.
Compile admission still runs once; restore already accepts a product on
any pool with the same xcodebuild -version, so CMUX_CI_XCODE_APP_PR must
name an Xcode every listed pool carries.

The object may name bare macos-26, which self-hosted fleet runners also
carry. The shard's first step fails before checkout unless a macos-*
request landed github-hosted, and records requested vs actual runner in
the step summary. The runner label policy reads the object label by label
and accepts bare macos-NN only there; a new guard keeps the fork gate and
the assertion in place.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EduXdN9PKnGsMQztJK7WeE
@github-actions

Copy link
Copy Markdown
Contributor

Caution

Pull Request opener is not an author or co-author of any commit in this PR.

This check is blocked to guard against commits being submitted under a trusted identity the submitter does not control. If this PR is a legitimate cherry-pick, release-engineering submission, or mailing-list-style patch delivery, the repository maintainer can opt out of this check by setting require-opener-as-author: 'false' on the CLA-assistant step in the repository's workflow.


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document v2.2 and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
✅ teamleaderleo
❌ claude
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9033224c-8024-4ea0-b24a-d634ce85e49b

📥 Commits

Reviewing files that changed from the base of the PR and between 407ee86 and d6b1b78.

📒 Files selected for processing (6)
  • .github/workflows/ci-health-report.yml
  • .github/workflows/ci-macos.yml
  • docs/ci-runners.md
  • scripts/ci/runner_label_policy.py
  • tests/test_ci_self_hosted_guard.sh
  • tests/test_runner_label_policy.py
 _____________________________________________________________________
< No matter how far down the wrong road you have gone, turn back now. >
 ---------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

CAN'T DEAL WITH CLAUDE BEING THE SOLE COMMITTER WHY ISN'T IT CO-AUTHORING

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #14097, which stripes the same app-host shards across the four pools and is already running the full suite. MACOS_RUNNER_PR_SHARDS was only read by this branch and can be unset.


Generated by Claude Code

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.

2 participants