Repository navigation
ci(nightly): build-nightly-app and the cold seed take blacksmith-6vcpu-macos-26, not the stalled 12 vCPU label - #27448
lawrencecchen wants to merge 4 commits into
Conversation
pick-classic-sign-runner (nightly.yml) and the DEV daily's runner pick give the signing, verify and delta jobs a label and a re-run alternate. With 6 vCPU moving the alternate is 12 vCPU, and with 6 vCPU stalled the pick itself is 12 vCPU. That label started no job from 2026-10-28 20:32Z; main's run 40834485210 held build-sign-notarize-nightly on it from 07:47Z after a re-run took the alternate. Fails until the next commit.
classic_nightly_build_runner.py takes ONLY (a candidate label or size): that label is both `cloud` and `alternate`, whatever the queue snapshot says. pick-classic-sign-runner (nightly.yml) and the DEV daily's runner pick set ONLY 6vcpu, so neither the first pick (6 vCPU stalled) nor a re-run's alternate sends signing, verify or delta jobs to the 12 vCPU macOS 26 label, which started no job from 2026-10-28 20:32Z. main's run 40834485210 held build-sign-notarize-nightly there from 07:47Z after a re-run took the 12 vCPU alternate. The snapshot cannot see that label's missing capacity while nothing waits on it. The labels stay Blacksmith-only literals; the runs-on expressions do not change. tests/test_classic_nightly_build_runner.py (previous commit) fails on the base and passes here.
…ith-12vcpu-macos-26 build-nightly-app's picker (pick-nightly-app-runner) puts 12 vCPU first, so with no snapshot or both queues moving its `cloud` is 12 vCPU, and its runs-on literal is 12 vCPU. refresh-test-compilation-cache seeds the 12 vCPU sibling of the 6 vCPU pull-request pool. That label started no job from 2026-10-28 20:32Z. The guards pin 6 vCPU; they fail until the next commit.
…u-macos-26 pick-nightly-app-runner sets ONLY 6vcpu, so when the trusted mini is busy or offline, and on every later attempt, build-nightly-app takes 6 vCPU, not its picker's 12 vCPU default. Its runs-on literal is 6 vCPU too. The compilation cache keys on OS, arch and toolchain, not instance size, and 6 vCPU built this app in 8 min on 2026-10-29 (runs 40833785678, 40834485210). refresh-test-compilation-cache seeds the pull-request pool as is (vars.MACOS_RUNNER_PR, seed-derived-data.yml's SEED_POOL_2) instead of its 12 vCPU sibling. blacksmith-12vcpu-macos-26 started no job from 2026-10-28 20:32Z. Stacked on the ONLY knob of #27444.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Probably yours: CI failed on
Matched log linesNot re-run automatically: Written by
|
|
Found 1 test failure on Blacksmith runners: Failure
|
|
Review (independent subagent, correctness first): SHIP at b832824 Scope: this PR's own commits c1e4add and b832824. The first two commits are #27444, which has its own receipt.
Not merged and no |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Pin picker outputs when ONLY is set scripts/ci/classic_nightly_build_runner.py |
When ONLY resolves to a Blacksmith candidate, the picker uses it for both cloud and alternate. Otherwise, it retains the existing FIRST-aware route selection and sets alternate to the other candidate. |
Set workflow runner labels .github/workflows/cmux-next-dev-daily.yml, .github/workflows/nightly.yml |
The workflows restrict the described picker selections and fallbacks to the 6-vCPU label. The cache seeder uses the configured runner value directly. |
Verify runner selection tests/test_classic_nightly_build_runner.py, tests/test_nightly_universal_build.sh |
Tests check picker outputs across queue snapshots and attempts, nightly fallbacks, and the absence of the 12-vCPU label in nightly runner and matrix configuration. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Suggested reviewers: teamleaderleo
Merge Risk: 
Minimal · up to b8328
-
The runner-selection changes have no established merge-blocking issue. Cold-build timing remains unmeasured, but the available evidence does not demonstrate a timeout regression.
Pre-merge checks: 
24 · 
1
-
Failed checks: 1 warning Passed checks: 24 passed-
Full details: Cmux Cloud Persistent Session And Early Input
-
Explanation
PASS: The PR changes only GitHub Actions runner labels, nightly pool selection, the classic nightly build-runner picker, and related tests. The diff contains no Cloud terminal creation, cmux-tui transport, manual renderer, PTY readiness, input ownership, authentication, idempotency, revision, or lease changes covered by the rule.
Full details: Cmux Swift Actor Isolation
-
Explanation
PASS: The pull request changes only two workflow files, one Python script, and two test files. The reviewed diff contains no Swift files or production Swift changes, so the Swift actor-isolation check is not applicable.
Full details: Cmux Swift Blocking Runtime
-
Explanation
PASS: The pull request changes only GitHub workflows, Python CI code/tests, and a shell test. It introduces no Swift, Objective-C, or runtime source changes, so it cannot introduce the specified Swift blocking or timing synchronization.
Full details: Cmux Browser Automation Off-Main
-
Explanation
The PR changes only nightly CI workflows, a Python runner picker, and related tests. The authoritative diff does not modify browser socket automation code,
Sources/TerminalController.swift, orControlCommandExecutionPolicy.swift, and no added or removed line contains the rule's browser/WebKit routing terms. Therefore it introduces no browser automation violation.
Full details: Cmux Expensive Synchronous Load
-
Explanation
The pull request changes only workflow YAML, Python CI logic, and shell/Python tests. The reviewed diff contains no Swift, Objective-C, or Objective-C++ files and no production Swift agent-history load. The custom failure condition is therefore not applicable.
Full details: Cmux Cache Substitution Correctness
-
Explanation
PASS: The pull request changes only CI YAML, Python CI logic/tests, and a shell test. It introduces no production Swift, TypeScript, or JavaScript changes, so the cache-substitution condition does not apply.
Full details: Cmux No Hacky Sleeps
-
Explanation
PASS. The production script change only adds the
ONLYrunner-selection path and updates output handling; it does not add or expand sleeps, timers, polling, retries, or wall-clock waits. The existingtrusted_routesleep loop is unchanged. Addedsleep=lambda seconds: Nonecalls are deterministic test scaffolding. Workflow YAML changes are out of scope under the rule.
Full details: Cmux Algorithmic Complexity
-
Explanation
The PR does not introduce an algorithmic-complexity issue. The production picker change in
scripts/ci/classic_nightly_build_runner.py:207-212performs one dictionary lookup and a membership check against the fixed two-labelCANDIDATEStuple, then avoids the existing snapshot scan whenONLYis set. The workflow edits only select the 6-vCPU label. The added collection loops are test-only. No scalable user-owned collection, nested scan, per-target rescan, sorting/filtering hot path, or unbenchmarked slower algorithm is introduced.
Full details: Cmux Swift Concurrency
-
Explanation
PASS: The pull request changes only workflow YAML, Python, and shell test files. The reviewed diff contains no Swift files or Swift concurrency code, so it does not introduce or expand any listed legacy Swift async pattern.
Full details: Cmux Swift Package Boundaries
-
Explanation
The pull request changes only workflow YAML, CI Python, and test shell/Python files. The authoritative diff contains no Swift files or Swift package changes, so the Swift package boundary rule is not triggered.
Full details: Cmux Swiftpm Lockfiles
-
Explanation
PASS: The PR changes only runner-selection workflows, the picker script, and tests. The diff contains no Package.swift, Package.resolved, Xcode project/workspace, or .gitignore changes. The workflow hunks change runner labels and ONLY values; they do not change SwiftPM dependencies or package references. No SwiftPM lockfile policy condition is introduced.
Full details: Cmux User-Facing Error Privacy
-
Explanation
PASS: The diff changes only CI workflow runner selection, an internal nightly picker, and tests/comments. Its new labels and picker diagnostics write to GitHub Actions outputs, summaries, and logs; no changed text reaches a cmux app UI, product CLI, or product API. The rule permits provider/configuration names in internal CI and operator diagnostics.
Full details: Cmux Full Internationalization
-
Explanation
PASS: The diff changes only CI workflows, a runner-selection script, and regression tests. Added text is operational comments, picker diagnostics, or test content. It does not add or change Swift UI text, app catalogs, web UI/API copy, metadata, rendered markdown, or changelog content. The internationalization rule explicitly allows operational comments, tests, and literal configuration tokens.
Full details: Cmux Architecture Rethink
-
Explanation
PASS: The reviewed range changes only GitHub Actions YAML, a Python runner picker, and tests. It contains no Swift files or SwiftUI/AppKit lifecycle code, so the Swift architectural rethink failure conditions do not apply.
Full details: Cmux Source Artifacts
-
Explanation
The PR changes only intentional workflow configuration, source code, and regression tests. The diff adds no local output, logs, screenshots, recordings, temp or cache directories, dependency checkouts, build output, DerivedData, downloads, or copied artifacts. All five paths fit the rule's allowed source, config, script, or test categories.
Full details: Cmux No Test Or Debug Seam In Production Source
-
Explanation
The reviewed diff changes only workflow YAML, Python CI logic, and shell/Python tests. It contains no Swift file under a production
Sources/path, so the specified production test/debug seam condition is not applicable.
Full details: Description check
-
Explanation
The description is mostly complete and relevant. It includes a detailed Summary, Testing section, observed results, known limitations, Changelog, and scope details. The Proof and Checklist sections are omitted, but these omissions do not prevent review because this is a CI-only change and the testing evidence is extensive.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 6 functions across 3 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Stacked on #27444. The first two commits (4eed88b, 171e793) are #27444, which adds the picker's
ONLYsetting that this PR uses. Review c1e4add (test) and b832824 (fix). Merge #27444 first.Summary
Two more nightly jobs on
maincan still route toblacksmith-12vcpu-macos-26, which hasn't started a job since 2026-10-28 20:32Z. In the last 100nightly.ymlruns (back to 2026-10-29 06:18Z), 0 of the 10 jobs that asked for it got a runner. Main's classic nightly run 40834485210 has waited since 07:47Z on that label for signing (#27444 fixes that path).build-nightly-apptakes the trusted mini first. When the mini is busy or offline, and on every later attempt, it takespick-nightly-app-runner'scloud. The picker puts 12 vCPU first, so with both queues moving, or with no snapshot,cloudis 12 vCPU. Theruns-onliteral is 12 vCPU too. Now the pick setsONLY: 6vcpuand the literal is'blacksmith-6vcpu-macos-26'. The trusted-mini path doesn't change. The compilation cache keys on OS, arch and toolchain, not instance size. 6 vCPU built this app in 8 min today (runs 40833785678, 40834485210).refresh-test-compilation-cache(the scheduled cold seed) put its first matrix pool on the 12 vCPU sibling of the 6 vCPU pull-request pool. Now that entry is the pull-request pool as is:vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15', the same expression asseed-derived-data.yml'sSEED_POOL_2.test_seed_derived_data.pystill holds the nightly pools to a subset of the per-push seed pools.Side effects:
seed_derived_data.py scopeputs the width (j<nproc>) in the key. So the nightly cold seed now writesj6keys, notj12. A 12 vCPU admission still gets the per-pushSEED_POOL_1chain and falls back toj6. WhileCI_PR_POOL_OWNED=1, the schedule skips this job, and only aseed_onlydispatch runs it.owned_pool_rescue.retry_labels()for nightly runs at new SHAs no longer counts the 12 vCPU label.After this PR and #27444, no
runs-onor matrix innightly.ymlnames the 12 vCPU label, and its pickers never produce it.Known nit, left on purpose: the step name "Pick the trusted mini or a moving cloud macOS label" in
pick-nightly-app-runneris now out of date, because the cloud label no longer moves. A rename would need another review round.Left as is:
seed-derived-data.yml'sSEED_POOL_1still seeds the 12 vCPU sibling. That pool also publishes the app-host product, and pull-request admission can still pick 12 vCPU (pr_runner_pool.py). Moving it changes which seed widths exist, so it needs its own PR.Testing
Regression:
python3 tests/test_classic_nightly_build_runner.py. The newBuildAndSeedStayOffTwelveVcpureadspick-nightly-app-runner's real step env and runsmain()over 4 snapshot states and attempts 1 and 2. It also checksbuild-nightly-app's fallback literal and that nonightly.ymlruns-onor matrix names the 12 vCPU label. On c1e4add (test only) it fails 10 checks. On the head, all tests pass.tests/test_nightly_universal_build.sh, which pins the build'sruns-on, also fails on c1e4add and passes on the head.Observed / Not run
Observed, run locally on the head, all pass:
test_classic_nightly_build_runner.pyandtest_seed_derived_data.py.test_ci_owned_pool_rescue.pyand the signing keychain guard.test_ci_publisher_mac_jobs_move.py,test_ci_pr_runner_pool.pyandtest_ci_queue_janitor.py.test_runner_label_policy.py,test_ci_runs_on_values.py,test_ci_no_github_hosted_macos.pyandtest_ci_nightly_off_blacksmith.py.test_ci_pull_request_caches_are_read_only.py,test_delivery_workflows_cancel.py,test_nightly_delivery_concurrency.pyandtest_ci_workflow_concurrency.py.test_seed_decide.py,test_ci_canonical_build_root.py,test_reuse_app_host_products.py,test_ci_cloud_overflow_switch.pyandtest_ci_repo_variable_defaults.py.tests/test_nightly_universal_build.sh,tests/test_ci_test_compilation_cache_seed.shandtests/test_ci_self_hosted_guard.sh.SHELLCHECK_OPTS=-S warning) onnightly.yml: clean.test_ci_owned_pool_sources.pyfails 1 check, the same way onorigin/main18f2c43. That check reads the feat-cmux-next tree, so this PR doesn't cause it.Not run: a nightly or a scheduled seed with this change. A cold build on 6 vCPU was not timed; today's two 6 vCPU builds were warm. The job's timeout stays 90 min.
Changelog
none