From dbd714cd22eafbbff0aa3c9030ee6347da93e8e3 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 19:36:33 -0400 Subject: [PATCH 1/5] ci: place each PR macOS job on a free owned mini, overflow the rest The picker took an owned pool only when a run's whole peak was free, so a full suite with 2 of 11 minis busy went to Blacksmith entirely and queued there while 9 minis sat idle. With CI_PR_POOL_OWNED_SPLIT=1, a run that does not fit takes the owned pool with the most machines free, and a new owned_jobs output names the jobs that fit: compile admission first, then the light jobs (cli-product, cli-pipe, remote-daemon, claude-wrapper). Every other attempt-1 job takes retry_runner, the Blacksmith pool on the lane's Xcode. The marker's is now the owned machines the run holds, so the janitor's committed count covers only the jobs placed there. GUI jobs (app-host shards, tests-build-and-lag) never take an owned pool: the minis have no console session. A persistent pick turns off unit_in_admission, so the changed suites run on a Blacksmith shard. Splitting a run is sound because both sides run Xcode 26.6 build 17F113 (minis cmux15, cmuxs-mac-mini-5, cmux13s and Blacksmith 6vcpu/12vcpu macOS 26 on 2026-09-24). The product only moves from the mini to Blacksmith, and check_xcode refuses a product from a newer Xcode. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-macos.yml | 33 ++-- .github/workflows/ci.yml | 20 ++- .github/workflows/cli-pipe-regressions.yml | 10 +- .github/workflows/remote-daemon.yml | 10 +- scripts/ci/owned_pool_rescue.py | 18 +- scripts/ci/pr_runner_pool.py | 199 ++++++++++++++++++--- scripts/ci/queue_janitor.py | 3 +- tests/test_ci_change_areas.py | 32 +++- tests/test_ci_pr_runner_pool.py | 131 ++++++++++++-- tests/test_ci_self_hosted_guard.sh | 12 +- tests/test_seed_derived_data.py | 26 +-- 11 files changed, 407 insertions(+), 87 deletions(-) diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 80bd1555ce28..093e8a934581 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -92,6 +92,14 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent: the jobs of attempt 1 that take + # it, as " ". Every other job takes pr_retry_runner, so a + # run can use the owned machines that are free and overflow the rest + # (pr_runner_pool.py, CI_PR_POOL_OWNED_SPLIT). + pr_owned_jobs: + required: false + default: "" + type: string pr_xcode_app: required: false default: "" @@ -134,7 +142,7 @@ jobs: # compile on the pool and Xcode seed-derived-data.yml builds with, so both # can adopt its DerivedData seed below. Merge groups and dispatches on # other branches keep the macos-15 lane. - runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' admission ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} # A changed-suites run adds its tests after the compile: the same # 30-minute batch ceiling the separate worker had. timeout-minutes: ${{ inputs.unit_in_admission == 'true' && 105 || 75 }} @@ -177,7 +185,7 @@ jobs: # keys on the toolchain and the build path instead, so a product built # here at the canonical root matches on any pool with the same Xcode. # The macOS runner guard still requires it to track runs-on. - CMUX_PRODUCT_RUNNER: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} + CMUX_PRODUCT_RUNNER: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' admission ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} # What the changed-suites steps at the end of this job read, with the # values `app-host unit tests` gives its changed-suites worker, shard 8. # The compile reads none of them: it runs plain xcodebuild and bakes the @@ -1231,9 +1239,11 @@ jobs: # compile admission to macOS 26. To spread shards over providers again, # every pool involved must carry the admission's exact Xcode; the # restore step refuses an older one with both versions named. - # A re-run of failed shards after compile admission passed on an owned - # pool keeps its outputs; pr_retry_runner moves them to Blacksmith. - runs-on: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} + # When compile admission ran on an owned pool, a shard the picker did not + # place there (none today: the minis have no console session for app-host + # tests), and a re-run of failed shards, take pr_retry_runner: Blacksmith + # on the lane's Xcode, which is the Xcode the owned label names. + runs-on: ${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, format(' shard-{0} ', matrix.shard))) && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} timeout-minutes: 75 strategy: # A pull request wants every shard's failures in one run. A merge group @@ -1315,7 +1325,7 @@ jobs: - name: Verify GitHub-hosted route env: RUNNER_ENVIRONMENT: ${{ runner.environment }} - REQUESTED_RUNNER: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} + REQUESTED_RUNNER: ${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, format(' shard-{0} ', matrix.shard))) && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} RUNNER_CONTEXT_NAME: ${{ runner.name }} run: | set -euo pipefail @@ -2227,8 +2237,9 @@ jobs: # and no GUI console session. It still needs the compiled product, so it # reuses macos-compile-admission's artifact like the app-host shards do, # on the pool and Xcode that built it: the test bundle only loads under - # the Xcode that linked it. - runs-on: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} + # the Xcode that linked it. On an owned-pool run it may take + # pr_retry_runner instead, as the shards do. + runs-on: ${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' cli-product ')) && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} timeout-minutes: 40 env: CMUX_NODE_PRODUCT_CACHE_ROOT: ${{ vars.CMUX_NODE_PRODUCT_CACHE_ROOT }} @@ -2244,7 +2255,7 @@ jobs: - name: Verify GitHub-hosted route env: RUNNER_ENVIRONMENT: ${{ runner.environment }} - REQUESTED_RUNNER: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} + REQUESTED_RUNNER: ${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' cli-product ')) && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} RUNNER_CONTEXT_NAME: ${{ runner.name }} run: | set -euo pipefail @@ -2995,7 +3006,7 @@ jobs: # full-suite dispatch follows admission onto the pull-request pool and # Xcode. The product consumer guard in the CI change-area tests fails when # the two drift. - runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_DISPLAY || 'blacksmith-6vcpu-macos-15') }} + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' lag ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_DISPLAY || 'blacksmith-6vcpu-macos-15') }} timeout-minutes: 75 env: CMUX_NODE_PRODUCT_CACHE_ROOT: ${{ vars.CMUX_NODE_PRODUCT_CACHE_ROOT }} @@ -3008,7 +3019,7 @@ jobs: steps: - name: Validate display runner identity env: - REQUESTED_RUNNER: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_DISPLAY || 'blacksmith-6vcpu-macos-15') }} + REQUESTED_RUNNER: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || (github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main') && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' lag ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_DISPLAY || 'blacksmith-6vcpu-macos-15') }} RUNNER_CONTEXT_NAME: ${{ runner.name }} run: | set -euo pipefail diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bea74baa4192..aa5ce7ed6be9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,12 +76,16 @@ jobs: unit_strict_steps: ${{ steps.suite.outputs.unit_strict_steps }} # Only ever true for a changed-suites run the diff selected, which never # takes the canary drop above. - unit_in_admission: ${{ steps.suite.outputs.unit_in_admission }} + # A compile admission on an owned Mac (a persistent pick) cannot run + # app-host suites: the minis have no console session. The changed suites + # then run on the one Blacksmith shard instead (pr_runner_pool.py). + unit_in_admission: ${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }} coverage_gap: ${{ steps.suite.outputs.coverage_gap }} compile_admitted: ${{ steps.unchanged_inputs.outputs.compile_admitted == 'true' && 'true' || steps.admitted.outputs.compile_admitted }} source_parent1: ${{ steps.source-identity.outputs.parent1 }} # The macOS pool every pull-request macOS job in this run uses, chosen - # once so a run is never split across pools (see pr_runner_pool.py). + # once so a Blacksmith run is never split across pools. On an owned pool + # only the jobs in macos_pr_owned_jobs take it (see pr_runner_pool.py). # Empty keeps each job's own MACOS_RUNNER_PR fallback: every event but a # pull request, and any uncertainty. macos_pr_runner: ${{ steps.macos-pool.outputs.runner }} @@ -90,6 +94,9 @@ jobs: # a re-run of failed jobs takes, since it reuses these outputs and must # not queue on the owned pool unwatched. macos_pr_retry_runner: ${{ steps.macos-pool.outputs.retry_runner }} + # Set only when the pool is persistent: the jobs of attempt 1 that take + # it (" admission shard-1 ... "); every other job takes the retry runner. + macos_pr_owned_jobs: ${{ steps.macos-pool.outputs.owned_jobs }} permissions: actions: read contents: read @@ -590,6 +597,9 @@ jobs: # Xcode whose version names their label (glaeda-std-xcode-26.6). POOL_OWNED: ${{ vars.CI_PR_POOL_OWNED }} OWNED_SLOTS: ${{ vars.CI_OWNED_POOL_SLOTS }} + # 1 lets a run take the owned machines that are free and send its + # other jobs to Blacksmith, instead of all or nothing (off unless 1). + POOL_OWNED_SPLIT: ${{ vars.CI_PR_POOL_OWNED_SPLIT }} CMUX_CI_XCODE_APP_PR: ${{ github.event.pull_request.head.repo.full_name == github.repository && vars.CMUX_CI_XCODE_APP_PR || '' }} # Handed to the macOS 15 pool's jobs only when the run lands there. CMUX_CI_XCODE_APP_MACOS_15: ${{ vars.CMUX_CI_XCODE_APP_MACOS_15 }} @@ -599,6 +609,7 @@ jobs: RUN_FULL_SUITE: ${{ steps.suite.outputs.full_suite }} RUN_UNIT_SUITE: ${{ steps.suite.outputs.unit_suite }} RUN_UNIT_IN_ADMISSION: ${{ steps.suite.outputs.unit_in_admission }} + RUN_UNIT_SELECTORS: ${{ steps.suite.outputs.unit_selectors }} RUN_CLAUDE_WRAPPER: ${{ steps.standalone.outputs.claude_wrapper }} RUN_CLI: ${{ steps.detect.outputs.cli }} RUN_REMOTE_DAEMON: ${{ steps.standalone.outputs.remote_daemon }} @@ -911,6 +922,7 @@ jobs: with: pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} + pr_owned_jobs: ${{ needs.changes.outputs.macos_pr_owned_jobs }} native_tests: ${{ needs.changes.outputs.remote_daemon_native == 'true' || contains(github.event.pull_request.labels.*.name, 'full-ci') }} cli: @@ -920,6 +932,7 @@ jobs: with: pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} + pr_owned_jobs: ${{ needs.changes.outputs.macos_pr_owned_jobs }} pr_xcode_app: ${{ needs.changes.outputs.macos_pr_xcode_app }} web: @@ -936,7 +949,7 @@ jobs: name: Claude wrapper regressions needs: [changes, static-preflight] if: ${{ !cancelled() && needs.changes.result == 'success' && needs.static-preflight.result == 'success' && (needs.changes.outputs.claude_wrapper == 'true' || (needs.changes.outputs.macos == 'true' && needs.changes.outputs.full_suite == 'true')) }} - runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(needs.changes.outputs.macos_pr_runner, 'blacksmith-') && needs.changes.outputs.macos_pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && github.run_attempt > 1 && needs.changes.outputs.macos_pr_retry_runner || github.event_name == 'pull_request' && (needs.changes.outputs.macos_pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(needs.changes.outputs.macos_pr_runner, 'blacksmith-') && needs.changes.outputs.macos_pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && (github.run_attempt > 1 || !contains(needs.changes.outputs.macos_pr_owned_jobs, ' claude-wrapper ')) && needs.changes.outputs.macos_pr_retry_runner || github.event_name == 'pull_request' && (needs.changes.outputs.macos_pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} timeout-minutes: 10 steps: - name: Checkout wrapper and test inputs @@ -1071,6 +1084,7 @@ jobs: release_archs: ${{ inputs.release_archs }} pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} + pr_owned_jobs: ${{ needs.changes.outputs.macos_pr_owned_jobs }} pr_xcode_app: ${{ needs.changes.outputs.macos_pr_xcode_app }} tests: diff --git a/.github/workflows/cli-pipe-regressions.yml b/.github/workflows/cli-pipe-regressions.yml index 274b2c6fd844..cfefb7b63f67 100644 --- a/.github/workflows/cli-pipe-regressions.yml +++ b/.github/workflows/cli-pipe-regressions.yml @@ -18,6 +18,14 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent: the jobs of attempt 1 that take + # it, as " ". Every other job takes pr_retry_runner, so a + # run can use the owned machines that are free and overflow the rest + # (pr_runner_pool.py, CI_PR_POOL_OWNED_SPLIT). + pr_owned_jobs: + required: false + default: "" + type: string pr_xcode_app: required: false default: "" @@ -33,7 +41,7 @@ concurrency: jobs: cli-pipe-regressions: - runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' cli-pipe ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') }} timeout-minutes: 30 env: CMUX_CI_XCODE_APP: ${{ github.event_name == 'pull_request' && (inputs.pr_xcode_app || github.event.pull_request.head.repo.full_name == github.repository && vars.CMUX_CI_XCODE_APP_PR || vars.CMUX_CI_XCODE_APP_MACOS_15) || vars.CMUX_CI_XCODE_APP_MACOS_15 }} diff --git a/.github/workflows/remote-daemon.yml b/.github/workflows/remote-daemon.yml index dcbe532ae111..b5780c58a88e 100644 --- a/.github/workflows/remote-daemon.yml +++ b/.github/workflows/remote-daemon.yml @@ -21,6 +21,14 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent: the jobs of attempt 1 that take + # it, as " ". Every other job takes pr_retry_runner, so a + # run can use the owned machines that are free and overflow the rest + # (pr_runner_pool.py, CI_PR_POOL_OWNED_SPLIT). + pr_owned_jobs: + required: false + default: "" + type: string push: branches: [main] paths: @@ -94,7 +102,7 @@ jobs: # Plain `go test` with no Xcode or GUI: any Mac will do. Follow the same # lanes as the other pull-request macOS jobs instead of pinning the # contended macOS 26 pool. - runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && (github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15' }} + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && (startsWith(inputs.pr_runner, 'blacksmith-') && inputs.pr_runner || 'blacksmith-6vcpu-macos-15') || github.event_name == 'pull_request' && ((github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, ' remote-daemon ')) && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15') || vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15' }} timeout-minutes: 15 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 9afd66082b81..558c43e479ab 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -1,10 +1,11 @@ #!/usr/bin/env python3 """Move a pull request CI run off a busy persistent macOS pool. -pr_runner_pool.py puts every macOS job of a run on one pool. When that pool is -owned (a `glaeda--xcode-` label, pr_runner_pool.persistent), -GitHub never re-routes a queued job: it waits for that pool however long the -pool stays busy. ci-owned-pool-rescue.yml starts this script when a CI run is +pr_runner_pool.py picks one pool per run. When that pool is owned (a +`glaeda--xcode-` label, pr_runner_pool.persistent), the jobs +it names in `owned_jobs` take it and the rest take retry_runner (Blacksmith). +GitHub never re-routes a queued job: one on the owned pool waits for it +however long the pool stays busy. ci-owned-pool-rescue.yml starts this script when a CI run is requested, from the default branch, with Actions write. The script waits for ci.yml's `changes` job, which runs the picker. When the @@ -17,9 +18,7 @@ pull request head has not moved, cancels the run, waits for it to finish, and re-runs it. The re-run is attempt 2, and pr_runner_pool.py never gives a retry attempt a persistent pool, so every macOS job of the re-run lands on -Blacksmith together. A run is never split -across pools, because app-host products only load under the Xcode that linked -them (#14163); that is why the whole run is re-run, not one job. +Blacksmith together. An owned runner can also refuse a job it was handed: glaeda's job-started hook exits 1 when the host is busy (its lock is held), and the job fails @@ -31,8 +30,9 @@ not moved, cancels the run if it is still going, and re-runs its failed jobs. That attempt 2 reuses attempt 1's outputs, so every macOS job in it takes retry_runner, the Blacksmith pool the picker named, and what already passed -(compile admission, say) is kept. That splits the run across machines, which -is sound only because both sides run the same Xcode: retry_runner is a macOS +(compile admission, say) is kept. A run on an owned pool is split across pools +anyway (per-job placement, and GUI jobs never take an owned pool), which is +sound only because both sides run the same Xcode: retry_runner is a macOS 26 pool on the lane's pin, the pin the owned label names, and on 2026-09-24 both the minis and Blacksmith's 6vcpu and 12vcpu macOS 26 images reported Xcode 26.6 build 17F113. If those builds ever differ, re-run the whole run diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index 04aac22b9036..f75d6e2a7c4c 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -4,8 +4,9 @@ ci.yml's `changes` job calls this once per run, and every pull-request macOS job in the run reads the answer: compile admission, the app-host consumers that follow it, tests-build-and-lag, the Claude wrapper, CLI pipe and remote -daemon lanes. A run is never split across pools, because the app-host product -only loads under the Xcode that linked it (#14163). +daemon lanes. A run on a Blacksmith pool is never split across pools, because +the app-host product only loads under the Xcode that linked it (#14163). A run +on an owned pool may be, per job (see "Per-job placement" below). The run takes the first pool in preference order that has headroom: @@ -58,6 +59,35 @@ `retry_runner`, the Blacksmith pool every macOS job takes from attempt 2 on. The owned order is `std` (48 GB minis), then `light` (16 GB), then the Blacksmith pools: one order for every job type. +Per-job placement (`vars.CI_PR_POOL_OWNED_SPLIT == '1'`): without it, a run +takes an owned pool only when its whole peak is free, so a full suite on 9 +idle minis with 2 busy went to Blacksmith entirely and queued there. With it, +when no owned pool fits the whole run, the run takes the owned pool with the +most free machines (at least one), and `owned_jobs` names the jobs that fit, +in priority order (priority()): compile admission first (the heavy compile, +and a mini keeps its warm DerivedData), then the light jobs +(cli-product-tests, the CLI pipe, remote daemon and Claude wrapper lanes). +Each job counts one machine; the jobs after admission reuse its machine. +Every other job of attempt 1 takes +`retry_runner`, the Blacksmith pool on the lane's Xcode. The shards and +cli-product-tests then run compile admission's product on another pool, which +is sound only because both run the same Xcode: the owned label names the +lane's pin, and on 2026-09-24 the minis and Blacksmith's 6vcpu and 12vcpu +macOS 26 images all reported Xcode 26.6 build 17F113. The product only ever +moves from a mini to Blacksmith (admission is always placed first), and +app_host_test_products.check_xcode refuses a product linked by a newer Xcode +than the consumer's, so a drift fails closed instead of crashing in dlopen. +The marker's `` is the owned machines the run holds at its peak, so the +janitor's `committed` counts only the owned jobs actually placed. With the +split off, a run takes an owned pool only when all its owned-eligible jobs fit. + +GUI jobs (app-host shards, tests-build-and-lag) never take an owned pool, in +either mode: the minis have no console session, so XCTest app-host runs fail +there (exit 65, job 107862186541). They take `retry_runner`, and ci.yml turns +off `unit_in_admission` for a persistent pick, so the changed suites a +compile admission would run itself move to a Blacksmith shard. A run's owned +peak (`jobs`, and the marker's) counts only the jobs that may take the pool. + The queue comes from the queue janitor, which lists every in-flight run's jobs each sweep and publishes what it saw as the `macos-pool-load` artifact. Only a copy uploaded by a run on main of this repository counts, so no other @@ -127,6 +157,7 @@ XCODE_APP = re.compile(r"/Xcode_([0-9]+(?:\.[0-9]+)*)\.app/?") PR_XCODE_VARIABLE = "CMUX_CI_XCODE_APP_PR" OWNED_VARIABLE = "CI_PR_POOL_OWNED" +SPLIT_VARIABLE = "CI_PR_POOL_OWNED_SPLIT" SLOTS_VARIABLE = "CI_OWNED_POOL_SLOTS" # A pull request run holds several macOS machines at once, each job on its # own. Beside compile admission run the Claude wrapper, CLI pipe and remote @@ -225,6 +256,9 @@ class Choice: # For a persistent runner only: the Blacksmith pool (the lane's own Xcode) # a re-run of failed jobs takes instead, since it reuses this run's pick. retry_runner: str = "" + # For a persistent runner only: its machines free for this run (capped at + # the run's peak), which place() fills in priority order. + owned_budget: int = 0 @dataclasses.dataclass(frozen=True) @@ -246,24 +280,106 @@ def flag(value: str | None) -> bool: return (value or "").strip() == "true" -def run_jobs(*, macos: str | None, full_suite: str | None, unit_suite: str | None, +# The job keys `owned_jobs` lists; each workflow job tests for its own key. +ADMISSION_JOB = "admission" +# The changed-suites worker is matrix shard 8 (ci-macos.yml app-host-unit-tests). +CHANGED_SUITES_SHARD = 8 + + +@dataclasses.dataclass(frozen=True) +class RunJobs: + """A run's macOS jobs by key: compile admission, what runs after it, and beside it.""" + admission: bool + after: tuple[str, ...] # after admission, in owned priority order; they reuse its machine + side: tuple[str, ...] # beside admission and what follows it + + @property + def peak(self) -> int: + return len(self.side) + (max(1, len(self.after)) if self.admission else 0) + + +def shard_job(index: int) -> str: + return f"shard-{index}" + + +# A full suite with every side lane: what a run whose routing is unknown is charged. +FULL_RUN = RunJobs(True, (*(shard_job(index) for index in range(1, APP_HOST_SHARDS + 1)), "lag", "cli-product"), + ("claude-wrapper", "cli-pipe", "remote-daemon")) + + +def run_plan(*, macos: str | None, full_suite: str | None, unit_suite: str | None, unit_in_admission: str | None, claude_wrapper: str | None, cli: str | None, - remote_daemon: str | None) -> int: - """Most macOS machines this run holds at once, from the changes job's routing. + remote_daemon: str | None, unit_selectors: str | None = None) -> RunJobs: + """This run's macOS jobs, from the changes job's routing. Counted high on purpose: compile admission is assumed to run (the reuse checks come later), and a changed-suites canary that may yet be dropped counts its shard. ci-macos.yml runs admission for a macOS or a CLI change, - and cli-product-tests after it for a CLI change or a full suite. + and cli-product-tests after it for a CLI change or a full suite. A unit + suite with no selectors (the unit-ci label) runs all seven shards; with + selectors, the one changed-suites worker. `unit_selectors` None (a caller + that does not know) counts one shard, as before. """ - side = sum(flag(lane) for lane in (cli, remote_daemon)) full = flag(macos) and flag(full_suite) - side += flag(claude_wrapper) or full + side = tuple(key for key, on in (("claude-wrapper", flag(claude_wrapper) or full), ("cli-pipe", flag(cli)), + ("remote-daemon", flag(remote_daemon))) if on) if not (flag(macos) or flag(cli)): - return side - shards = APP_HOST_SHARDS if full else int(flag(macos) and flag(unit_suite) and not flag(unit_in_admission)) - after = shards + full + (flag(cli) or full) - return side + max(1, after) + return RunJobs(False, (), side) + unit = flag(macos) and flag(unit_suite) and not flag(unit_in_admission) + if full or (unit and unit_selectors is not None and not unit_selectors.strip()): + shards = tuple(shard_job(index) for index in range(1, APP_HOST_SHARDS + 1)) + elif unit: + shards = (shard_job(CHANGED_SUITES_SHARD),) + else: + shards = () + after = shards + (("lag",) if full else ()) + (("cli-product",) if flag(cli) or full else ()) + return RunJobs(True, after, side) + + +def run_jobs(**routing: str | None) -> int: + """Most macOS machines this run holds at once, from the changes job's routing (run_plan).""" + return run_plan(**routing).peak + + +# The jobs that may take an owned pool, in placement priority: the heavy +# compile, then light jobs. GUI jobs (shard-N, lag) never do: the minis have +# no console session for XCTest app-host runs. +LIGHT_JOBS = ("cli-product", "cli-pipe", "remote-daemon", "claude-wrapper") +OWNED_ELIGIBLE = (ADMISSION_JOB, *LIGHT_JOBS) + + +def priority(key: str) -> int: + return OWNED_ELIGIBLE.index(key) + + +def owned_peak(plan: RunJobs) -> int: + """The machines a run holds on an owned pool when every job that may take one does.""" + return place(plan, plan.peak)[1] + + +def place(plan: RunJobs, budget: int) -> tuple[tuple[str, ...], int]: + """The jobs that take the owned pool with `budget` machines free, and the machines they hold at peak. + + Jobs are taken in priority() order while the run's owned peak stays within + `budget`: the side lanes (beside admission) plus the larger of admission + and the jobs after it, which reuse its machine. A job that does not fit is + skipped, and a later one that does is still taken. Admission comes first, + so a run whose admission is not placed places nothing after it. + """ + chosen: list[str] = [] + + def held(keys: Sequence[str]) -> int: + side = sum(1 for key in keys if key in plan.side) + after = sum(1 for key in keys if key in plan.after) + return side + (max(1, after) if ADMISSION_JOB in keys else after) + + keys = ((ADMISSION_JOB,) if plan.admission else ()) + plan.after + plan.side + for key in sorted((key for key in keys if key in OWNED_ELIGIBLE), key=priority): + if key in plan.after and ADMISSION_JOB not in chosen: + continue + if held([*chosen, key]) <= max(0, budget): + chosen.append(key) + return tuple(chosen), held(chosen) def settings(overflow: str | None, order: str | None, max_queued: str | None, @@ -412,12 +528,15 @@ def owned_free(counts: Mapping[str, int], added_runs: int, taken_since: int = 0) def pick(load: Mapping[str, Mapping[str, int]], added: Mapping[str, int], usable: Sequence[str], max_queued: int, jobs: int = MAX_RUN_JOBS, - taken: Mapping[str, int] | None = None) -> tuple[str, bool]: + taken: Mapping[str, int] | None = None, split: bool = False) -> tuple[str, bool]: """The rule itself: first usable pool with headroom, else the shortest queue. An owned pool has headroom only while every job of this run gets a machine at once (`jobs` of them, its peak): a job queued there waits for that pool - alone. It is never the fallback. + alone. With `split`, when no owned pool fits the whole run, the owned pool + with the most machines free (the earlier on a tie) has headroom too, if it + has one: the jobs that do not fit go to Blacksmith (place()). An owned + pool is never the fallback. A Blacksmith pool has headroom while this run's job still finds a free machine there (or at most max_queued queue once it arrives), so a full @@ -425,9 +544,14 @@ def pick(load: Mapping[str, Mapping[str, int]], added: Mapping[str, int], usable shortest queue in rounds, a cold pool (cold()) counting COLD_ROUNDS more. """ queued = {label: effective_queue(load[label], added[label] + 1) for label in usable} + free = {label: owned_free(load[label], added[label], (taken or {}).get(label, 0)) + for label in usable if persistent(label)} + fits = [label for label in free if free[label] >= max(1, jobs)] + if split and not fits and free and max(free.values()) >= 1: + fits = [max(free, key=lambda label: free[label])] for label in usable: if persistent(label): - if owned_free(load[label], added[label], (taken or {}).get(label, 0)) >= max(1, jobs): + if label in fits: return label, True elif queued[label] <= max_queued: return label, True @@ -454,6 +578,7 @@ def decide( choose_from: Sequence[str] | None = None, owned_slots: Mapping[str, int] | None = None, jobs: int = MAX_RUN_JOBS, + split: bool = False, ) -> Choice: """The preference rule over a janitor snapshot. Uncertainty keeps today's route. @@ -470,6 +595,7 @@ def decide( `owned_since` is what runs since the snapshot took on each owned pool, by their markers, and `ephemeral_since` counts runs whose pick finished off the owned pools; those are replayed over the Blacksmith pools only. + `split` lets this run take part of an owned pool (pick(), place()). """ if not isinstance(snapshot, Mapping) or not isinstance(snapshot.get("pools"), Mapping): return Choice("", "", "no readable pool snapshot") @@ -512,7 +638,7 @@ def counted(label: str) -> bool: for _ in range(max(0, routed_since)): earlier, _ = pick(load, added, usable, limits.max_queued, jobs=1, taken=taken) added[earlier] += 1 - label, headroom = pick(load, added, candidates, limits.max_queued, jobs, taken=taken) + label, headroom = pick(load, added, candidates, limits.max_queued, jobs, taken=taken, split=split) if persistent(label) and not headroom: return Choice("", "", "every owned pool this run may take is busy, and no other pool is in the order") replayed = sum(added.values()) @@ -520,10 +646,15 @@ def counted(label: str) -> bool: if any(taken.values()): replay += " and counting " + ", ".join(f"{count} machine(s) newer runs took on {pool_label}" for pool_label, count in taken.items() if count) + free = 0 if headroom and persistent(label): free = owned_free(load[label], added[label], taken.get(label, 0)) - why = (f"first pool in order with headroom ({free} of {load[label]['capacity']} owned machines free, " - f"this run needs {max(1, jobs)}){replay}") + if free >= max(1, jobs): + why = (f"first pool in order with headroom ({free} of {load[label]['capacity']} owned machines free, " + f"this run needs {max(1, jobs)}){replay}") + else: + why = (f"owned pool with the most machines free ({free} of {load[label]['capacity']}, this run needs " + f"{max(1, jobs)}): the jobs that fit run there, the rest on the retry runner{replay}") elif headroom: why = f"first pool in order with a free machine{replay}" if not limits.max_queued else \ f"first pool in order with headroom (<= {limits.max_queued} queued){replay}" @@ -546,7 +677,7 @@ def counted(label: str) -> bool: # own Xcode, which is also the Xcode the owned label names. lane = [pool_label for pool_label in usable if not persistent(pool_label) and not POOLS.get(pool_label)] retry = pick(load, added, lane, limits.max_queued)[0] if lane else DEFAULT_RUNNER - return Choice(label, xcode(label) or "", why + note, retry) + return Choice(label, xcode(label) or "", why + note, retry, min(free, max(1, jobs)) if persistent(label) else 0) def choose( @@ -562,6 +693,7 @@ def choose( owned: str | None = None, owned_slots: str | None = None, jobs: int = MAX_RUN_JOBS, + split: str | None = None, fetch: Callable[[], Mapping[str, Any] | None], count_routed: Callable[[str], "int | Routed"] = lambda since: 0, now: dt.datetime, @@ -617,7 +749,8 @@ def choose( routed = Routed(unknown=int(routed)) choice = decide(snapshot, limits, now=now, xcode_pins={} if fork else xcode_pins, routed_since=routed.unknown, owned_since=routed.owned, ephemeral_since=routed.ephemeral, - auto_xcode=fork, owned_slots={} if fork else slots(owned_slots), jobs=jobs) + auto_xcode=fork, owned_slots={} if fork else slots(owned_slots), jobs=jobs, + split=(split or "").strip() == "1") if fork and choice.runner: choice = dataclasses.replace(choice, reason=f"fork head; {choice.reason}") if retry and choice.runner: @@ -791,13 +924,15 @@ def pull_request_runs_since(self, since: str, *, exclude_run_id: int | None) -> def summary(choice: Choice, snapshot: Mapping[str, Any] | None, *, now: dt.datetime, - owned_slots: Mapping[str, int] | None = None, problems: Sequence[str] = ()) -> str: + owned_slots: Mapping[str, int] | None = None, problems: Sequence[str] = (), + owned_jobs: Sequence[str] = ()) -> str: runner = choice.runner or "each job's default (MACOS_RUNNER_PR or its fallback)" lines = ["### macOS pool for this run", "", f"- Pool: `{runner}`", f"- Why: {choice.reason}"] if choice.xcode_app: lines.append(f"- Xcode: `{choice.xcode_app}`") if choice.retry_runner: - lines.append(f"- A re-run of failed jobs goes to: `{choice.retry_runner}`") + lines.append(f"- Jobs on `{choice.runner}`: {', '.join(owned_jobs) or 'none'}; every other job, " + f"and a re-run of failed jobs, goes to: `{choice.retry_runner}`") for problem in problems: lines.append(f"- **Warning:** {problem}; that pool gets no machines") if isinstance(snapshot, Mapping) and isinstance(snapshot.get("pools"), Mapping): @@ -838,10 +973,14 @@ def count_routed(since: str) -> int: # The changes job's routing, when the step runs after it; without it every # run is charged the most machines any run can hold. - jobs = MAX_RUN_JOBS if "RUN_MACOS" not in env else run_jobs( + plan = FULL_RUN if "RUN_MACOS" not in env else run_plan( macos=env.get("RUN_MACOS"), full_suite=env.get("RUN_FULL_SUITE"), unit_suite=env.get("RUN_UNIT_SUITE"), unit_in_admission=env.get("RUN_UNIT_IN_ADMISSION"), claude_wrapper=env.get("RUN_CLAUDE_WRAPPER"), - cli=env.get("RUN_CLI"), remote_daemon=env.get("RUN_REMOTE_DAEMON")) + cli=env.get("RUN_CLI"), remote_daemon=env.get("RUN_REMOTE_DAEMON"), + unit_selectors=env.get("RUN_UNIT_SELECTORS")) + # What an owned pool must have free for the whole run: its owned-eligible + # jobs at their peak (GUI jobs never take one). + jobs = owned_peak(plan) choice, snapshot = choose( event=env.get("EVENT_NAME") or "", repo=repo, @@ -853,6 +992,7 @@ def count_routed(since: str) -> int: owned=env.get("POOL_OWNED"), owned_slots=env.get("OWNED_SLOTS"), jobs=jobs, + split=env.get("POOL_OWNED_SPLIT"), xcode_pins={variable: env.get(variable) or "" for variable in {*POOLS.values(), PR_XCODE_VARIABLE} if variable}, fetch=fetch, @@ -863,7 +1003,11 @@ def count_routed(since: str) -> int: problems = slot_problems(env.get("OWNED_SLOTS")) if (env.get("POOL_OWNED") or "").strip() == "1" else [] for problem in problems: print(f"::warning title={SLOTS_VARIABLE}::{problem}") - text = summary(choice, snapshot, now=now, owned_slots=slots(env.get("OWNED_SLOTS")), problems=problems) + # A persistent pick names the jobs that take it; every other job of the + # run takes retry_runner. The marker's jobs are the owned machines held. + owned_jobs, held = place(plan, choice.owned_budget) if persistent(choice.runner) else ((), plan.peak) + text = summary(choice, snapshot, now=now, owned_slots=slots(env.get("OWNED_SLOTS")), problems=problems, + owned_jobs=owned_jobs) print(text) if env.get("GITHUB_STEP_SUMMARY"): with open(env["GITHUB_STEP_SUMMARY"], "a", encoding="utf-8") as handle: @@ -872,7 +1016,10 @@ def count_routed(since: str) -> int: with open(env["GITHUB_OUTPUT"], "a", encoding="utf-8") as handle: handle.write(f"runner={choice.runner}\nxcode_app={choice.xcode_app}\n" f"persistent={'true' if persistent(choice.runner) else 'false'}\n" - f"retry_runner={choice.retry_runner}\njobs={jobs}\n") + f"retry_runner={choice.retry_runner}\njobs={held}\n" + # Space-delimited with a space at each end, so each job's + # contains(' ') test matches whole keys only. + f"owned_jobs={' ' + ' '.join(owned_jobs) + ' ' if owned_jobs else ''}\n") return 0 diff --git a/scripts/ci/queue_janitor.py b/scripts/ci/queue_janitor.py index 28988798eac3..5813c4fbbcab 100644 --- a/scripts/ci/queue_janitor.py +++ b/scripts/ci/queue_janitor.py @@ -54,7 +54,8 @@ here: stale pull request runs (b) on it are cancelled whatever its queue, which frees minis, and the other categories only while it is backed up. With CI_PR_POOL_OWNED on, the snapshot also carries each owned pool's -``committed`` machines: the peak each run holding it declared in its +``committed`` machines: the owned machines each run holding it declared at +its peak (the jobs it placed there, not the whole run) in its ``macos-pool-persistent----`` marker, read with one artifact listing per run that may hold one. diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index f1abc36ae64a..9cd606dcb0f1 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -4251,11 +4251,22 @@ def app_host_product_consumers(workflow: dict) -> dict[str, dict]: } -# A re-run of failed shards on a run the picker put on an owned pool moves to -# the Blacksmith pool it named on the same Xcode (pr_runner_pool.py). -PRODUCT_RUNNER_OUTPUT = ( - "${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }}" -) +# On a run the picker put on an owned pool, a consumer it did not place there +# (every GUI job), and any re-run of failed jobs, takes the Blacksmith pool it +# named on the lane's Xcode, which is the Xcode the owned label names +# (pr_runner_pool.py). Each consumer tests its own owned_jobs key. +PRODUCT_RUNNER_KEYS = { + "app-host-unit-tests": "format(' shard-{0} ', matrix.shard)", + "cli-product-tests": "' cli-product '", +} + + +def product_runner_output(key: str) -> str: + return ("${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, " + key + ")) " + "&& inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }}") + + +PRODUCT_RUNNER_OUTPUT = product_runner_output(PRODUCT_RUNNER_KEYS["app-host-unit-tests"]) PRODUCT_XCODE_OUTPUT = "${{ needs.macos-compile-admission.outputs.xcode_app }}" @@ -4279,11 +4290,13 @@ def product_consumer_route_violations(workflow: dict) -> list[str]: for name, job in app_host_product_consumers(workflow).items(): runs_on = job.get("runs-on", "") xcode = (job.get("env") or {}).get("CMUX_CI_XCODE_APP") - if runs_on == PRODUCT_RUNNER_OUTPUT and xcode == PRODUCT_XCODE_OUTPUT: + if name in PRODUCT_RUNNER_KEYS and runs_on == product_runner_output(PRODUCT_RUNNER_KEYS[name]) \ + and xcode == PRODUCT_XCODE_OUTPUT: continue if ( name == "tests-build-and-lag" - and runs_on.replace("vars.MACOS_RUNNER_DISPLAY", "vars.MACOS_RUNNER_15") == producer["runs-on"] + and runs_on.replace("vars.MACOS_RUNNER_DISPLAY", "vars.MACOS_RUNNER_15").replace( + "' lag '", "' admission '") == producer["runs-on"] and xcode == producer["env"]["CMUX_CI_XCODE_APP"] ): continue @@ -4428,7 +4441,10 @@ def outputs(paths: list[str], label: str = "") -> dict[str, str]: assert outputs(["Sources/Workspace.swift"])["unit_in_admission"] == "false" ci = yaml.safe_load(CI_WORKFLOW.read_text(encoding="utf-8")) - assert ci["jobs"]["changes"]["outputs"]["unit_in_admission"] == "${{ steps.suite.outputs.unit_in_admission }}" + # A compile admission on an owned Mac cannot run app-host suites (no + # console session), so a persistent pick moves them to a Blacksmith shard. + assert ci["jobs"]["changes"]["outputs"]["unit_in_admission"] == ( + "${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }}") assert ci["jobs"]["macos"]["with"]["unit_in_admission"] == "${{ needs.changes.outputs.unit_in_admission }}" workflow = yaml.safe_load(MACOS_WORKFLOW.read_text(encoding="utf-8")) diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index dc8ac78778ba..54da2ffc6772 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -56,7 +56,7 @@ def backlog(small=21, large=0, old=4, large_reserved=0, old_reserved=0, age=5, s def choose(snap, *, event="pull_request", head="manaflow-ai/cmux", default=SMALL, overflow="", order="", max_queued="", pins=PINS, fetch=None, routed=0, attempt=1, owned="", - owned_slots="", jobs=pool.MAX_RUN_JOBS): + owned_slots="", jobs=pool.MAX_RUN_JOBS, split=""): def count_routed(since): if isinstance(routed, Exception): raise routed @@ -64,7 +64,7 @@ def count_routed(since): return pool.choose( event=event, repo="manaflow-ai/cmux", head_repo=head, default_runner=default, overflow=overflow, order=order, max_queued=max_queued, xcode_pins=pins, owned=owned, - owned_slots=owned_slots, jobs=jobs, + owned_slots=owned_slots, jobs=jobs, split=split, fetch=fetch or (lambda: snap), count_routed=count_routed, now=NOW, run_attempt=attempt, )[0] @@ -283,7 +283,7 @@ def test_main_writes_outputs_and_summary(self): finally: sys.stdout = old self.assertEqual(out.read_text(), f"runner={LARGE}\nxcode_app=\npersistent=false\n" - f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\n") + f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\nowned_jobs=\n") text = summary.read_text() self.assertIn(f"Pool: `{LARGE}`", text) self.assertIn(f"{SMALL}: 21 queued, 10 running", text) @@ -421,11 +421,13 @@ def test_workflow_publishes_the_snapshot(self): # The pull-request lane, wherever its event condition puts it: compile admission # also takes it for main's full-suite dispatch (#14158), where the inputs are empty. -PR_ROUTE = re.compile(r"&& \((?P[^()]*vars\.MACOS_RUNNER_PR[^()]*)\)") +PR_ROUTE = re.compile(r"&& \((?P(?:[^()]|\((?:[^()]|\([^()]*\))*\))*vars\.MACOS_RUNNER_PR[^()]*)\)") -RETRY_LANE = ("github.run_attempt > 1 && inputs.pr_retry_runner || inputs.pr_runner || vars.MACOS_RUNNER_PR " - "|| 'blacksmith-6vcpu-macos-15'") +def retry_lane(key: str) -> str: + """The pull-request lane of the job whose owned_jobs key is `key`.""" + return (f"(github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, {key})) && inputs.pr_retry_runner " + "|| inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15'") PR_XCODE = "/Applications/Xcode_26.6.app" MINI = "glaeda-std-xcode-26.6" LIGHT = "glaeda-light-xcode-26.6" @@ -702,6 +704,107 @@ def test_main_reports_a_persistent_choice(self): self.assertEqual(self.output(1, owned="")["persistent"], "false") +def routing(**flags): + base = dict(macos="true", full_suite="false", unit_suite="false", unit_in_admission="false", + claude_wrapper="false", cli="false", remote_daemon="false", unit_selectors="") + return pool.run_plan(**{**base, **flags}) + + +FULL = dict(full_suite="true", cli="true", remote_daemon="true") + + +class PerJobPlacement(unittest.TestCase): + """CI_PR_POOL_OWNED_SPLIT: free owned machines take the jobs that fit, Blacksmith the rest.""" + + def test_the_plan_names_each_job(self): + plan = routing(**FULL) + self.assertTrue(plan.admission) + self.assertEqual(plan.after, (*(f"shard-{index}" for index in range(1, 8)), "lag", "cli-product")) + self.assertEqual(plan.side, ("claude-wrapper", "cli-pipe", "remote-daemon")) + self.assertEqual(plan.peak, pool.MAX_RUN_JOBS) + # The unit-ci label runs all seven shards; selected suites one worker, shard 8. + self.assertEqual(routing(unit_suite="true").after, tuple(f"shard-{index}" for index in range(1, 8))) + self.assertEqual(routing(unit_suite="true", unit_selectors="Suite").after, ("shard-8",)) + self.assertEqual(routing(unit_suite="true", unit_in_admission="true", unit_selectors="Suite").after, ()) + self.assertEqual(routing(macos="false", cli="true").side, ("cli-pipe",)) + + def test_admission_then_light_jobs_and_never_gui_jobs(self): + plan = routing(**FULL) + self.assertEqual(pool.place(plan, 0), ((), 0)) + self.assertEqual(pool.place(plan, 1), (("admission", "cli-product"), 1)) + self.assertEqual(pool.place(plan, 2), (("admission", "cli-product", "cli-pipe"), 2)) + everything = ("admission", "cli-product", "cli-pipe", "remote-daemon", "claude-wrapper") + self.assertEqual(pool.place(plan, 4), (everything, 4)) + # More machines never place a shard or tests-build-and-lag. + self.assertEqual(pool.place(plan, 12), (everything, 4)) + self.assertEqual(pool.owned_peak(plan), 4) + # A run without admission places its side lanes alone. + self.assertEqual(pool.place(routing(macos="false", remote_daemon="true"), 1), (("remote-daemon",), 1)) + + def test_split_takes_the_free_machines_instead_of_overflowing(self): + # 11 machines, 9 busy: a 3-machine run does not fit whole. + self.assertEqual(owned_choice(fleet(busy=9)).runner, LARGE) + choice = owned_choice(fleet(busy=9), split="1") + self.assertEqual((choice.runner, choice.retry_runner, choice.owned_budget), (MINI, LARGE, 2)) + self.assertIn("2 of 11", choice.reason) + # A whole fit keeps the old reason and a budget of the run's peak. + whole = owned_choice(fleet(busy=2), split="1") + self.assertEqual((whole.runner, whole.owned_budget), (MINI, 3)) + self.assertIn("first pool in order with headroom", whole.reason) + # No machine free at all: Blacksmith, as before. + self.assertEqual(owned_choice(fleet(busy=11), split="1").runner, LARGE) + + def test_split_prefers_a_pool_the_whole_run_fits(self): + snap = fleet(busy=9) + snap["pools"][LIGHT] = {"queued": 0, "running": 0} + both = json.dumps({MINI: 11, LIGHT: 3}) + self.assertEqual(owned_choice(snap, owned_slots=both, split="1").runner, LIGHT) + # Neither fits: the pool with the most free machines. + snap["pools"][LIGHT] = {"queued": 0, "running": 2} + self.assertEqual(owned_choice(snap, owned_slots=both, split="1", jobs=4).runner, MINI) + # A tie goes to the earlier pool; a full one never takes the run. + snap["pools"][MINI]["running"] = 10 + self.assertEqual(owned_choice(snap, owned_slots=both, split="1", jobs=4).runner, MINI) + snap["pools"][MINI]["running"] = 11 + self.assertEqual(owned_choice(snap, owned_slots=both, split="1", jobs=4).runner, LIGHT) + + def test_split_is_off_unless_1(self): + for value in ("", "0", "true"): + self.assertEqual(owned_choice(fleet(busy=9), split=value).runner, LARGE, value) + + def output(self, *, busy, split="1", **routing_env): + with tempfile.TemporaryDirectory() as tmp: + snapshot = Path(tmp, "snap.json") + fresh = fleet(busy=busy) + fresh["generated_at"] = dt.datetime.now(dt.timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + snapshot.write_text(json.dumps(fresh)) + out = Path(tmp, "out") + env = {"EVENT_NAME": "pull_request", "GITHUB_REPOSITORY": "manaflow-ai/cmux", + "HEAD_REPO": "manaflow-ai/cmux", "DEFAULT_RUNNER": SMALL, "POOL_OWNED": "1", + "POOL_OWNED_SPLIT": split, "OWNED_SLOTS": json.dumps({MINI: 11}), + "CMUX_CI_XCODE_APP_PR": PR_XCODE, "CMUX_CI_XCODE_APP_MACOS_15": XCODE_15, + "GITHUB_RUN_ATTEMPT": "1", "GITHUB_OUTPUT": str(out), "RUN_MACOS": "true", + **routing_env} + with unittest.mock.patch("sys.stdout", io.StringIO()): + pool.main(["--snapshot", str(snapshot)], env) + return dict(line.split("=", 1) for line in out.read_text().splitlines()) + + def test_main_names_the_owned_jobs_and_marks_their_peak(self): + full = {"RUN_FULL_SUITE": "true", "RUN_CLI": "true", "RUN_REMOTE_DAEMON": "true"} + partial = self.output(busy=9, **full) + self.assertEqual((partial["runner"], partial["retry_runner"]), (MINI, LARGE)) + self.assertEqual(partial["owned_jobs"], " admission cli-product cli-pipe ") + # The marker (and so the janitor) counts the owned machines placed. + self.assertEqual(partial["jobs"], "2") + whole = self.output(busy=0, **full) + self.assertEqual(whole["owned_jobs"], " admission cli-product cli-pipe remote-daemon claude-wrapper ") + self.assertEqual(whole["jobs"], "4") + # Split off: the whole-run rule over the owned-eligible jobs only. + self.assertEqual(self.output(busy=7, split="", **full)["runner"], MINI) + off = self.output(busy=8, split="", **full) + self.assertEqual((off["runner"], off["owned_jobs"]), (LARGE, "")) + + class Wiring(unittest.TestCase): """Every pull-request macOS route in one CI run reads the one chosen pool.""" @@ -744,21 +847,25 @@ def lanes(self, name): def test_every_pr_route_in_the_run_reads_the_choice(self): expected = { "ci.yml": "needs.changes.outputs.macos_pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15'", - "ci-macos.yml": RETRY_LANE, - "cli-pipe-regressions.yml": RETRY_LANE, - "remote-daemon.yml": RETRY_LANE, + # Compile admission (and its CMUX_PRODUCT_RUNNER mirror) and + # tests-build-and-lag each test their own owned_jobs key. + "ci-macos.yml": {retry_lane("' admission '"), retry_lane("' lag '")}, + "cli-pipe-regressions.yml": {retry_lane("' cli-pipe '")}, + "remote-daemon.yml": {retry_lane("' remote-daemon '")}, } for name, lane in expected.items(): lanes = self.lanes(name) self.assertTrue(lanes, name) - self.assertEqual(set(lanes), {lane}, name) + self.assertEqual(set(lanes), lane if isinstance(lane, set) else {lane}, name) def test_a_rerun_of_failed_shards_leaves_the_owned_pool(self): shards = self.workflow("ci-macos.yml")["jobs"]["app-host-unit-tests"] - self.assertEqual(shards["runs-on"], "${{ github.run_attempt > 1 && inputs.pr_retry_runner " + self.assertEqual(shards["runs-on"], "${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, " + "format(' shard-{0} ', matrix.shard))) && inputs.pr_retry_runner " "|| needs.macos-compile-admission.outputs.runner }}") wrapper = self.workflow("ci.yml")["jobs"]["claude-wrapper"]["runs-on"] - self.assertIn("github.run_attempt > 1 && needs.changes.outputs.macos_pr_retry_runner", wrapper) + self.assertIn("(github.run_attempt > 1 || !contains(needs.changes.outputs.macos_pr_owned_jobs, " + "' claude-wrapper ')) && needs.changes.outputs.macos_pr_retry_runner", wrapper) def test_callers_pass_the_choice(self): jobs = self.workflow("ci.yml")["jobs"] diff --git a/tests/test_ci_self_hosted_guard.sh b/tests/test_ci_self_hosted_guard.sh index 6283493e45b1..c992470a89c8 100755 --- a/tests/test_ci_self_hosted_guard.sh +++ b/tests/test_ci_self_hosted_guard.sh @@ -44,9 +44,10 @@ check_macos_runner() { in_job && /^ [^[:space:]#][^:]*:[[:space:]]*(#.*)?$/ { in_job=0 } in_job && /runs-on:.*(vars\.MACOS_RUNNER|blacksmith-[0-9]+vcpu-macos-|warp-macos-[0-9]+-arm64|depot-macos-)/ { saw=1 } # A product consumer inherits the compile admission pool, which this - # check covers on its own, or on a re-run the Blacksmith pool the pull + # check covers on its own, or, on a re-run or when the picker did not + # place this shard on the owned pool, the Blacksmith pool the pull # request picker named for a run on an owned pool (pr_retry_runner). - in_job && /runs-on:[[:space:]]*\$\{\{ (github\.run_attempt > 1 && inputs\.pr_retry_runner \|\| )?needs\.macos-compile-admission\.outputs\.runner \}\}/ { saw=1 } + in_job && /runs-on:[[:space:]]*\$\{\{ (\(github\.run_attempt > 1 \|\| !contains\(inputs\.pr_owned_jobs, format\(. shard-\{0\} ., matrix\.shard\)\)\) && inputs\.pr_retry_runner \|\| )?needs\.macos-compile-admission\.outputs\.runner \}\}/ { saw=1 } in_job && /os:.*(vars\.MACOS_RUNNER|blacksmith-[0-9]+vcpu-macos-|warp-macos-[0-9]+-arm64|depot-macos-)/ { saw=1 } END { exit !(saw) } ' "$file"; then @@ -1376,9 +1377,10 @@ GUARDED = ( " || 'blacksmith-6vcpu-macos-15')", "github.event_name == 'pull_request' && (needs.changes.outputs.macos_pr_runner || vars.MACOS_RUNNER_PR" " || 'blacksmith-6vcpu-macos-15')", - # A re-run of failed jobs on an owned-pool run: the Blacksmith pool the - # picker named for it. - "github.event_name == 'pull_request' && github.run_attempt > 1 && needs.changes.outputs.macos_pr_retry_runner", + # A re-run of failed jobs on an owned-pool run, or a job the picker did not + # place on the owned pool: the Blacksmith pool the picker named for it. + "github.event_name == 'pull_request' && (github.run_attempt > 1 || !contains(needs.changes.outputs.macos_pr_owned_jobs," + " ' claude-wrapper ')) && needs.changes.outputs.macos_pr_retry_runner", ) diff --git a/tests/test_seed_derived_data.py b/tests/test_seed_derived_data.py index c775b679426a..60bad72baa2a 100644 --- a/tests/test_seed_derived_data.py +++ b/tests/test_seed_derived_data.py @@ -410,16 +410,17 @@ def primary(): return token == "true" if token[0].isdigit(): return float(token) - if token == "startsWith" and peek() == "(": + if token in ("startsWith", "contains") and peek() == "(": take() haystack = either() if take() != ",": - raise ValueError("startsWith takes two arguments") + raise ValueError(f"{token} takes two arguments") needle = either() if take() != ")": raise ValueError("unbalanced parentheses") - return ("" if haystack is None else str(haystack)).lower().startswith( - ("" if needle is None else str(needle)).lower()) + haystack = ("" if haystack is None else str(haystack)).lower() + needle = ("" if needle is None else str(needle)).lower() + return haystack.startswith(needle) if token == "startsWith" else needle in haystack value = context for part in token.split("."): value = value.get(part) if isinstance(value, dict) else None @@ -809,16 +810,21 @@ def test_a_rerun_of_an_owned_pool_run_takes_the_retry_runner(self): # pr_runner_pool.py names pr_retry_runner only for an owned-pool pick; a # re-run of failed jobs (attempt 2) reuses attempt 1's inputs. admission = load("ci-macos.yml")["jobs"]["macos-compile-admission"] - for attempt, retry, runner in ( - ("1", "blacksmith-12vcpu-macos-26", "glaeda-std-xcode-26.6"), - ("2", "blacksmith-12vcpu-macos-26", "blacksmith-12vcpu-macos-26"), - ("2", "", "glaeda-std-xcode-26.6"), + # Attempt 1 takes the owned pool only when the picker placed admission + # there (pr_owned_jobs); otherwise the retry runner. + for attempt, retry, owned_jobs, runner in ( + ("1", "blacksmith-12vcpu-macos-26", " admission cli-pipe ", "glaeda-std-xcode-26.6"), + ("1", "blacksmith-12vcpu-macos-26", " cli-pipe ", "blacksmith-12vcpu-macos-26"), + ("1", "blacksmith-12vcpu-macos-26", "", "blacksmith-12vcpu-macos-26"), + ("2", "blacksmith-12vcpu-macos-26", " admission ", "blacksmith-12vcpu-macos-26"), + ("2", "", "", "glaeda-std-xcode-26.6"), ): context = github_context("pull_request", ref="refs/pull/1/merge") context["github"].update(repository="manaflow-ai/cmux", run_attempt=attempt, event={"pull_request": {"head": {"repo": {"full_name": "manaflow-ai/cmux"}}}}) - context["inputs"].update(pr_runner="glaeda-std-xcode-26.6", pr_retry_runner=retry) - with self.subTest(attempt=attempt, retry=retry): + context["inputs"].update(pr_runner="glaeda-std-xcode-26.6", pr_retry_runner=retry, + pr_owned_jobs=owned_jobs) + with self.subTest(attempt=attempt, retry=retry, owned_jobs=owned_jobs): self.assertEqual(evaluate(admission["runs-on"], context), runner) self.assertEqual(evaluate(admission["env"]["CMUX_PRODUCT_RUNNER"], context), runner) From 0a9ebd366bb557564ce1ff1938f7ca785384afba Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 19:46:43 -0400 Subject: [PATCH 2/5] ci: let owned minis take GUI jobs, behind CI_PR_POOL_OWNED_GUI The minis' runners are LaunchAgents in the logged-in user's Aqua session, and #14305's changed suites passed inside compile admission on cmuxs-mac-mini-5. Job 107862186541's exit 65 was that PR's own test (CMUXCLICodexUnavailableAdmissionTests), not the environment. Owned placement now goes admission, app-host shards by index, tests-build-and-lag, then the light jobs; the shards queue longest on Blacksmith. CI_PR_POOL_OWNED_GUI=0 keeps GUI jobs off the minis again, and only then does a persistent pick move the changed suites out of admission (new output owned_gui). Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-macos.yml | 6 ++-- .github/workflows/ci.yml | 10 +++--- scripts/ci/owned_pool_rescue.py | 2 +- scripts/ci/pr_runner_pool.py | 60 +++++++++++++++++++++------------ tests/test_ci_change_areas.py | 7 ++-- tests/test_ci_pr_runner_pool.py | 52 ++++++++++++++++++---------- 6 files changed, 86 insertions(+), 51 deletions(-) diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 093e8a934581..e60e3255f4d2 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -1240,9 +1240,9 @@ jobs: # every pool involved must carry the admission's exact Xcode; the # restore step refuses an older one with both versions named. # When compile admission ran on an owned pool, a shard the picker did not - # place there (none today: the minis have no console session for app-host - # tests), and a re-run of failed shards, take pr_retry_runner: Blacksmith - # on the lane's Xcode, which is the Xcode the owned label names. + # place there (no machine left, or CI_PR_POOL_OWNED_GUI=0), and a re-run + # of failed shards, take pr_retry_runner: Blacksmith on the lane's Xcode, + # which is the Xcode the owned label names. runs-on: ${{ (github.run_attempt > 1 || !contains(inputs.pr_owned_jobs, format(' shard-{0} ', matrix.shard))) && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} timeout-minutes: 75 strategy: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa5ce7ed6be9..1f1b9834f953 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,10 +76,10 @@ jobs: unit_strict_steps: ${{ steps.suite.outputs.unit_strict_steps }} # Only ever true for a changed-suites run the diff selected, which never # takes the canary drop above. - # A compile admission on an owned Mac (a persistent pick) cannot run - # app-host suites: the minis have no console session. The changed suites - # then run on the one Blacksmith shard instead (pr_runner_pool.py). - unit_in_admission: ${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }} + # With GUI jobs kept off the owned Macs (CI_PR_POOL_OWNED_GUI=0), a + # compile admission there does not run app-host suites; the changed + # suites run on the one Blacksmith shard instead (pr_runner_pool.py). + unit_in_admission: ${{ (steps.macos-pool.outputs.persistent != 'true' || steps.macos-pool.outputs.owned_gui == 'true') && steps.suite.outputs.unit_in_admission || 'false' }} coverage_gap: ${{ steps.suite.outputs.coverage_gap }} compile_admitted: ${{ steps.unchanged_inputs.outputs.compile_admitted == 'true' && 'true' || steps.admitted.outputs.compile_admitted }} source_parent1: ${{ steps.source-identity.outputs.parent1 }} @@ -600,6 +600,8 @@ jobs: # 1 lets a run take the owned machines that are free and send its # other jobs to Blacksmith, instead of all or nothing (off unless 1). POOL_OWNED_SPLIT: ${{ vars.CI_PR_POOL_OWNED_SPLIT }} + # 0 keeps GUI jobs (app-host shards, tests-build-and-lag) off them. + POOL_OWNED_GUI: ${{ vars.CI_PR_POOL_OWNED_GUI }} CMUX_CI_XCODE_APP_PR: ${{ github.event.pull_request.head.repo.full_name == github.repository && vars.CMUX_CI_XCODE_APP_PR || '' }} # Handed to the macOS 15 pool's jobs only when the run lands there. CMUX_CI_XCODE_APP_MACOS_15: ${{ vars.CMUX_CI_XCODE_APP_MACOS_15 }} diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 558c43e479ab..a4a327b98d11 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -31,7 +31,7 @@ That attempt 2 reuses attempt 1's outputs, so every macOS job in it takes retry_runner, the Blacksmith pool the picker named, and what already passed (compile admission, say) is kept. A run on an owned pool is split across pools -anyway (per-job placement, and GUI jobs never take an owned pool), which is +anyway (per-job placement, CI_PR_POOL_OWNED_SPLIT), which is sound only because both sides run the same Xcode: retry_runner is a macOS 26 pool on the lane's pin, the pin the owned label names, and on 2026-09-24 both the minis and Blacksmith's 6vcpu and 12vcpu macOS 26 images reported diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index f75d6e2a7c4c..f7d4d2fc85b2 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -65,8 +65,9 @@ when no owned pool fits the whole run, the run takes the owned pool with the most free machines (at least one), and `owned_jobs` names the jobs that fit, in priority order (priority()): compile admission first (the heavy compile, -and a mini keeps its warm DerivedData), then the light jobs -(cli-product-tests, the CLI pipe, remote daemon and Claude wrapper lanes). +and a mini keeps its warm DerivedData), then the GUI jobs (app-host shards by +index, tests-build-and-lag), which queue longest on Blacksmith, then the light +jobs (cli-product-tests, the CLI pipe, remote daemon and Claude wrapper lanes). Each job counts one machine; the jobs after admission reuse its machine. Every other job of attempt 1 takes `retry_runner`, the Blacksmith pool on the lane's Xcode. The shards and @@ -81,12 +82,13 @@ janitor's `committed` counts only the owned jobs actually placed. With the split off, a run takes an owned pool only when all its owned-eligible jobs fit. -GUI jobs (app-host shards, tests-build-and-lag) never take an owned pool, in -either mode: the minis have no console session, so XCTest app-host runs fail -there (exit 65, job 107862186541). They take `retry_runner`, and ci.yml turns -off `unit_in_admission` for a persistent pick, so the changed suites a -compile admission would run itself move to a Blacksmith shard. A run's owned -peak (`jobs`, and the marker's) counts only the jobs that may take the pool. +GUI jobs (app-host shards, tests-build-and-lag) take an owned pool unless +`vars.CI_PR_POOL_OWNED_GUI == '0'`: the minis' runners are LaunchAgents in +the logged-in user's Aqua session, and each mini runs one job at a time. With +it 0 they take `retry_runner`, and ci.yml turns off `unit_in_admission` for a +persistent pick (output `owned_gui`), so the changed suites a compile +admission would run itself move to a Blacksmith shard. A run's owned peak +(`jobs`, and the marker's) counts only the jobs that may take the pool. The queue comes from the queue janitor, which lists every in-flight run's jobs each sweep and publishes what it saw as the `macos-pool-load` artifact. @@ -158,6 +160,7 @@ PR_XCODE_VARIABLE = "CMUX_CI_XCODE_APP_PR" OWNED_VARIABLE = "CI_PR_POOL_OWNED" SPLIT_VARIABLE = "CI_PR_POOL_OWNED_SPLIT" +GUI_VARIABLE = "CI_PR_POOL_OWNED_GUI" SLOTS_VARIABLE = "CI_OWNED_POOL_SLOTS" # A pull request run holds several macOS machines at once, each job on its # own. Beside compile admission run the Claude wrapper, CLI pipe and remote @@ -341,30 +344,40 @@ def run_jobs(**routing: str | None) -> int: return run_plan(**routing).peak -# The jobs that may take an owned pool, in placement priority: the heavy -# compile, then light jobs. GUI jobs (shard-N, lag) never do: the minis have -# no console session for XCTest app-host runs. +# Owned placement priority: the heavy compile, then GUI jobs (the longest +# Blacksmith queues), then light jobs. GUI jobs need the mini's console +# session; CI_PR_POOL_OWNED_GUI=0 keeps them off. LIGHT_JOBS = ("cli-product", "cli-pipe", "remote-daemon", "claude-wrapper") -OWNED_ELIGIBLE = (ADMISSION_JOB, *LIGHT_JOBS) -def priority(key: str) -> int: - return OWNED_ELIGIBLE.index(key) +def gui_job(key: str) -> bool: + return key == "lag" or key.startswith("shard-") -def owned_peak(plan: RunJobs) -> int: +def priority(key: str) -> tuple[int, int]: + if key == ADMISSION_JOB: + return 0, 0 + if key.startswith("shard-"): + return 1, int(key.removeprefix("shard-")) + if key == "lag": + return 2, 0 + return 3, LIGHT_JOBS.index(key) + + +def owned_peak(plan: RunJobs, gui: bool = True) -> int: """The machines a run holds on an owned pool when every job that may take one does.""" - return place(plan, plan.peak)[1] + return place(plan, plan.peak, gui)[1] -def place(plan: RunJobs, budget: int) -> tuple[tuple[str, ...], int]: +def place(plan: RunJobs, budget: int, gui: bool = True) -> tuple[tuple[str, ...], int]: """The jobs that take the owned pool with `budget` machines free, and the machines they hold at peak. Jobs are taken in priority() order while the run's owned peak stays within `budget`: the side lanes (beside admission) plus the larger of admission and the jobs after it, which reuse its machine. A job that does not fit is skipped, and a later one that does is still taken. Admission comes first, - so a run whose admission is not placed places nothing after it. + so a run whose admission is not placed places nothing after it. Without + `gui`, GUI jobs (gui_job()) are never placed. """ chosen: list[str] = [] @@ -374,7 +387,7 @@ def held(keys: Sequence[str]) -> int: return side + (max(1, after) if ADMISSION_JOB in keys else after) keys = ((ADMISSION_JOB,) if plan.admission else ()) + plan.after + plan.side - for key in sorted((key for key in keys if key in OWNED_ELIGIBLE), key=priority): + for key in sorted((key for key in keys if gui or not gui_job(key)), key=priority): if key in plan.after and ADMISSION_JOB not in chosen: continue if held([*chosen, key]) <= max(0, budget): @@ -979,8 +992,9 @@ def count_routed(since: str) -> int: cli=env.get("RUN_CLI"), remote_daemon=env.get("RUN_REMOTE_DAEMON"), unit_selectors=env.get("RUN_UNIT_SELECTORS")) # What an owned pool must have free for the whole run: its owned-eligible - # jobs at their peak (GUI jobs never take one). - jobs = owned_peak(plan) + # jobs at their peak. + gui = (env.get("POOL_OWNED_GUI") or "").strip() != "0" + jobs = owned_peak(plan, gui) choice, snapshot = choose( event=env.get("EVENT_NAME") or "", repo=repo, @@ -1005,7 +1019,7 @@ def count_routed(since: str) -> int: print(f"::warning title={SLOTS_VARIABLE}::{problem}") # A persistent pick names the jobs that take it; every other job of the # run takes retry_runner. The marker's jobs are the owned machines held. - owned_jobs, held = place(plan, choice.owned_budget) if persistent(choice.runner) else ((), plan.peak) + owned_jobs, held = place(plan, choice.owned_budget, gui) if persistent(choice.runner) else ((), plan.peak) text = summary(choice, snapshot, now=now, owned_slots=slots(env.get("OWNED_SLOTS")), problems=problems, owned_jobs=owned_jobs) print(text) @@ -1017,6 +1031,8 @@ def count_routed(since: str) -> int: handle.write(f"runner={choice.runner}\nxcode_app={choice.xcode_app}\n" f"persistent={'true' if persistent(choice.runner) else 'false'}\n" f"retry_runner={choice.retry_runner}\njobs={held}\n" + # Whether compile admission may run app-host suites on the pool. + f"owned_gui={'true' if gui else 'false'}\n" # Space-delimited with a space at each end, so each job's # contains(' ') test matches whole keys only. f"owned_jobs={' ' + ' '.join(owned_jobs) + ' ' if owned_jobs else ''}\n") diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index 9cd606dcb0f1..07f016b60111 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -4441,10 +4441,11 @@ def outputs(paths: list[str], label: str = "") -> dict[str, str]: assert outputs(["Sources/Workspace.swift"])["unit_in_admission"] == "false" ci = yaml.safe_load(CI_WORKFLOW.read_text(encoding="utf-8")) - # A compile admission on an owned Mac cannot run app-host suites (no - # console session), so a persistent pick moves them to a Blacksmith shard. + # With GUI jobs kept off the owned Macs, a persistent pick moves the + # changed suites from compile admission to a Blacksmith shard. assert ci["jobs"]["changes"]["outputs"]["unit_in_admission"] == ( - "${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }}") + "${{ (steps.macos-pool.outputs.persistent != 'true' || steps.macos-pool.outputs.owned_gui == 'true') " + "&& steps.suite.outputs.unit_in_admission || 'false' }}") assert ci["jobs"]["macos"]["with"]["unit_in_admission"] == "${{ needs.changes.outputs.unit_in_admission }}" workflow = yaml.safe_load(MACOS_WORKFLOW.read_text(encoding="utf-8")) diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 54da2ffc6772..af550d2adca0 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -283,7 +283,7 @@ def test_main_writes_outputs_and_summary(self): finally: sys.stdout = old self.assertEqual(out.read_text(), f"runner={LARGE}\nxcode_app=\npersistent=false\n" - f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\nowned_jobs=\n") + f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\nowned_gui=true\nowned_jobs=\n") text = summary.read_text() self.assertIn(f"Pool: `{LARGE}`", text) self.assertIn(f"{SMALL}: 21 queued, 10 running", text) @@ -728,16 +728,27 @@ def test_the_plan_names_each_job(self): self.assertEqual(routing(unit_suite="true", unit_in_admission="true", unit_selectors="Suite").after, ()) self.assertEqual(routing(macos="false", cli="true").side, ("cli-pipe",)) - def test_admission_then_light_jobs_and_never_gui_jobs(self): + def test_admission_then_gui_jobs_then_light_jobs(self): plan = routing(**FULL) + shards = tuple(f"shard-{index}" for index in range(1, 8)) self.assertEqual(pool.place(plan, 0), ((), 0)) - self.assertEqual(pool.place(plan, 1), (("admission", "cli-product"), 1)) - self.assertEqual(pool.place(plan, 2), (("admission", "cli-product", "cli-pipe"), 2)) + # Shards reuse admission's machine once it finishes. + self.assertEqual(pool.place(plan, 1), (("admission", "shard-1"), 1)) + self.assertEqual(pool.place(plan, 3), (("admission", "shard-1", "shard-2", "shard-3"), 3)) + self.assertEqual(pool.place(plan, 9), (("admission", *shards, "lag", "cli-product"), 9)) + self.assertEqual(pool.place(plan, 12), (("admission", *shards, "lag", "cli-product", "cli-pipe", + "remote-daemon", "claude-wrapper"), 12)) + self.assertEqual(pool.owned_peak(plan), 12) + self.assertEqual(pool.place(routing(unit_suite="true", unit_selectors="Suite"), 1), + (("admission", "shard-8"), 1)) + + def test_gui_jobs_stay_off_when_switched_off(self): + plan = routing(**FULL) + self.assertEqual(pool.place(plan, 1, gui=False), (("admission", "cli-product"), 1)) + self.assertEqual(pool.place(plan, 2, gui=False), (("admission", "cli-product", "cli-pipe"), 2)) everything = ("admission", "cli-product", "cli-pipe", "remote-daemon", "claude-wrapper") - self.assertEqual(pool.place(plan, 4), (everything, 4)) - # More machines never place a shard or tests-build-and-lag. - self.assertEqual(pool.place(plan, 12), (everything, 4)) - self.assertEqual(pool.owned_peak(plan), 4) + self.assertEqual(pool.place(plan, 12, gui=False), (everything, 4)) + self.assertEqual(pool.owned_peak(plan, gui=False), 4) # A run without admission places its side lanes alone. self.assertEqual(pool.place(routing(macos="false", remote_daemon="true"), 1), (("remote-daemon",), 1)) @@ -772,7 +783,7 @@ def test_split_is_off_unless_1(self): for value in ("", "0", "true"): self.assertEqual(owned_choice(fleet(busy=9), split=value).runner, LARGE, value) - def output(self, *, busy, split="1", **routing_env): + def output(self, *, busy, split="1", gui="", **routing_env): with tempfile.TemporaryDirectory() as tmp: snapshot = Path(tmp, "snap.json") fresh = fleet(busy=busy) @@ -781,7 +792,7 @@ def output(self, *, busy, split="1", **routing_env): out = Path(tmp, "out") env = {"EVENT_NAME": "pull_request", "GITHUB_REPOSITORY": "manaflow-ai/cmux", "HEAD_REPO": "manaflow-ai/cmux", "DEFAULT_RUNNER": SMALL, "POOL_OWNED": "1", - "POOL_OWNED_SPLIT": split, "OWNED_SLOTS": json.dumps({MINI: 11}), + "POOL_OWNED_SPLIT": split, "POOL_OWNED_GUI": gui, "OWNED_SLOTS": json.dumps({MINI: 11}), "CMUX_CI_XCODE_APP_PR": PR_XCODE, "CMUX_CI_XCODE_APP_MACOS_15": XCODE_15, "GITHUB_RUN_ATTEMPT": "1", "GITHUB_OUTPUT": str(out), "RUN_MACOS": "true", **routing_env} @@ -791,17 +802,22 @@ def output(self, *, busy, split="1", **routing_env): def test_main_names_the_owned_jobs_and_marks_their_peak(self): full = {"RUN_FULL_SUITE": "true", "RUN_CLI": "true", "RUN_REMOTE_DAEMON": "true"} - partial = self.output(busy=9, **full) + partial = self.output(busy=8, **full) self.assertEqual((partial["runner"], partial["retry_runner"]), (MINI, LARGE)) - self.assertEqual(partial["owned_jobs"], " admission cli-product cli-pipe ") + self.assertEqual(partial["owned_jobs"], " admission shard-1 shard-2 shard-3 ") # The marker (and so the janitor) counts the owned machines placed. - self.assertEqual(partial["jobs"], "2") - whole = self.output(busy=0, **full) - self.assertEqual(whole["owned_jobs"], " admission cli-product cli-pipe remote-daemon claude-wrapper ") - self.assertEqual(whole["jobs"], "4") + self.assertEqual((partial["jobs"], partial["owned_gui"]), ("3", "true")) + # 11 machines for a 12-machine run: the last light job overflows. + most = self.output(busy=0, **full) + self.assertEqual(most["owned_jobs"].split()[-3:], ["cli-product", "cli-pipe", "remote-daemon"]) + self.assertEqual(most["jobs"], "11") + # GUI jobs off: only admission and the light jobs, and owned_gui says so. + light = self.output(busy=9, gui="0", **full) + self.assertEqual((light["owned_jobs"], light["jobs"], light["owned_gui"]), + (" admission cli-product cli-pipe ", "2", "false")) # Split off: the whole-run rule over the owned-eligible jobs only. - self.assertEqual(self.output(busy=7, split="", **full)["runner"], MINI) - off = self.output(busy=8, split="", **full) + self.assertEqual(self.output(busy=7, split="", gui="0", **full)["runner"], MINI) + off = self.output(busy=8, split="", gui="0", **full) self.assertEqual((off["runner"], off["owned_jobs"]), (LARGE, "")) From 1c85ca6f797d91e5103fa26868c5796581fa1362 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 19:50:17 -0400 Subject: [PATCH 3/5] ci: always move the changed suites out of an owned compile admission glaeda's runner hook gives compile admission the compile token, never the gui token, so app-host suites run inside it on a mini could collide with a GUI shard on the same machine. Every persistent pick now runs them on shard 8 instead, and the picker plans for that shard. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 9 +++++---- scripts/ci/pr_runner_pool.py | 12 ++++++------ tests/test_ci_change_areas.py | 7 +++---- tests/test_ci_pr_runner_pool.py | 13 ++++++++----- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f1b9834f953..043db01e7bf3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,10 +76,11 @@ jobs: unit_strict_steps: ${{ steps.suite.outputs.unit_strict_steps }} # Only ever true for a changed-suites run the diff selected, which never # takes the canary drop above. - # With GUI jobs kept off the owned Macs (CI_PR_POOL_OWNED_GUI=0), a - # compile admission there does not run app-host suites; the changed - # suites run on the one Blacksmith shard instead (pr_runner_pool.py). - unit_in_admission: ${{ (steps.macos-pool.outputs.persistent != 'true' || steps.macos-pool.outputs.owned_gui == 'true') && steps.suite.outputs.unit_in_admission || 'false' }} + # A compile admission on an owned Mac never runs app-host suites: glaeda + # gives it the compile token, not the gui token, so the suites would + # collide with a GUI shard on the same mini. The changed suites run on + # the one shard (shard 8) instead, owned or Blacksmith (pr_runner_pool.py). + unit_in_admission: ${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }} coverage_gap: ${{ steps.suite.outputs.coverage_gap }} compile_admitted: ${{ steps.unchanged_inputs.outputs.compile_admitted == 'true' && 'true' || steps.admitted.outputs.compile_admitted }} source_parent1: ${{ steps.source-identity.outputs.parent1 }} diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index f7d4d2fc85b2..a677f1efb360 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -85,9 +85,9 @@ GUI jobs (app-host shards, tests-build-and-lag) take an owned pool unless `vars.CI_PR_POOL_OWNED_GUI == '0'`: the minis' runners are LaunchAgents in the logged-in user's Aqua session, and each mini runs one job at a time. With -it 0 they take `retry_runner`, and ci.yml turns off `unit_in_admission` for a -persistent pick (output `owned_gui`), so the changed suites a compile -admission would run itself move to a Blacksmith shard. A run's owned peak +it 0 they take `retry_runner`. ci.yml turns off `unit_in_admission` for every +persistent pick, so the changed suites a compile admission would run itself +move to shard 8: glaeda gives admission the compile token, not the gui token. A run's owned peak (`jobs`, and the marker's) counts only the jobs that may take the pool. The queue comes from the queue janitor, which lists every in-flight run's @@ -988,7 +988,9 @@ def count_routed(since: str) -> int: # run is charged the most machines any run can hold. plan = FULL_RUN if "RUN_MACOS" not in env else run_plan( macos=env.get("RUN_MACOS"), full_suite=env.get("RUN_FULL_SUITE"), unit_suite=env.get("RUN_UNIT_SUITE"), - unit_in_admission=env.get("RUN_UNIT_IN_ADMISSION"), claude_wrapper=env.get("RUN_CLAUDE_WRAPPER"), + # A persistent pick moves the changed suites out of admission to + # shard 8 (ci.yml), so the plan always counts that shard. + unit_in_admission="false", claude_wrapper=env.get("RUN_CLAUDE_WRAPPER"), cli=env.get("RUN_CLI"), remote_daemon=env.get("RUN_REMOTE_DAEMON"), unit_selectors=env.get("RUN_UNIT_SELECTORS")) # What an owned pool must have free for the whole run: its owned-eligible @@ -1031,8 +1033,6 @@ def count_routed(since: str) -> int: handle.write(f"runner={choice.runner}\nxcode_app={choice.xcode_app}\n" f"persistent={'true' if persistent(choice.runner) else 'false'}\n" f"retry_runner={choice.retry_runner}\njobs={held}\n" - # Whether compile admission may run app-host suites on the pool. - f"owned_gui={'true' if gui else 'false'}\n" # Space-delimited with a space at each end, so each job's # contains(' ') test matches whole keys only. f"owned_jobs={' ' + ' '.join(owned_jobs) + ' ' if owned_jobs else ''}\n") diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index 07f016b60111..7c3ac804072f 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -4441,11 +4441,10 @@ def outputs(paths: list[str], label: str = "") -> dict[str, str]: assert outputs(["Sources/Workspace.swift"])["unit_in_admission"] == "false" ci = yaml.safe_load(CI_WORKFLOW.read_text(encoding="utf-8")) - # With GUI jobs kept off the owned Macs, a persistent pick moves the - # changed suites from compile admission to a Blacksmith shard. + # A compile admission on an owned Mac holds glaeda's compile token, not the + # gui token, so a persistent pick moves the changed suites to shard 8. assert ci["jobs"]["changes"]["outputs"]["unit_in_admission"] == ( - "${{ (steps.macos-pool.outputs.persistent != 'true' || steps.macos-pool.outputs.owned_gui == 'true') " - "&& steps.suite.outputs.unit_in_admission || 'false' }}") + "${{ steps.macos-pool.outputs.persistent != 'true' && steps.suite.outputs.unit_in_admission || 'false' }}") assert ci["jobs"]["macos"]["with"]["unit_in_admission"] == "${{ needs.changes.outputs.unit_in_admission }}" workflow = yaml.safe_load(MACOS_WORKFLOW.read_text(encoding="utf-8")) diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index af550d2adca0..9ca4248c6652 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -283,7 +283,7 @@ def test_main_writes_outputs_and_summary(self): finally: sys.stdout = old self.assertEqual(out.read_text(), f"runner={LARGE}\nxcode_app=\npersistent=false\n" - f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\nowned_gui=true\nowned_jobs=\n") + f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\nowned_jobs=\n") text = summary.read_text() self.assertIn(f"Pool: `{LARGE}`", text) self.assertIn(f"{SMALL}: 21 queued, 10 running", text) @@ -806,15 +806,18 @@ def test_main_names_the_owned_jobs_and_marks_their_peak(self): self.assertEqual((partial["runner"], partial["retry_runner"]), (MINI, LARGE)) self.assertEqual(partial["owned_jobs"], " admission shard-1 shard-2 shard-3 ") # The marker (and so the janitor) counts the owned machines placed. - self.assertEqual((partial["jobs"], partial["owned_gui"]), ("3", "true")) + self.assertEqual(partial["jobs"], "3") # 11 machines for a 12-machine run: the last light job overflows. most = self.output(busy=0, **full) self.assertEqual(most["owned_jobs"].split()[-3:], ["cli-product", "cli-pipe", "remote-daemon"]) self.assertEqual(most["jobs"], "11") - # GUI jobs off: only admission and the light jobs, and owned_gui says so. + # GUI jobs off: only admission and the light jobs. light = self.output(busy=9, gui="0", **full) - self.assertEqual((light["owned_jobs"], light["jobs"], light["owned_gui"]), - (" admission cli-product cli-pipe ", "2", "false")) + self.assertEqual((light["owned_jobs"], light["jobs"]), (" admission cli-product cli-pipe ", "2")) + # Selected suites a compile admission would run itself move to shard 8. + suites = self.output(busy=0, RUN_UNIT_SUITE="true", RUN_UNIT_IN_ADMISSION="true", + RUN_UNIT_SELECTORS="cmuxTests/SomeSuite") + self.assertEqual(suites["owned_jobs"], " admission shard-8 ") # Split off: the whole-run rule over the owned-eligible jobs only. self.assertEqual(self.output(busy=7, split="", gui="0", **full)["runner"], MINI) off = self.output(busy=8, split="", gui="0", **full) From 644822d1b6b7292339e6b80b49678ea504e2f152 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 19:58:58 -0400 Subject: [PATCH 4/5] ci: free a run's owned machines once its owned jobs finish With per-job placement, a run's owned jobs (admission, light lanes) can finish long before its Blacksmith shards, but the janitor charged the marker's peak until the whole run completed, so idle minis read as busy and new runs overflowed. A marked run whose owned jobs all completed now holds nothing; before its first owned job exists, the marker still reserves its peak. The product-consumer guard also requires tests-build-and-lag to test its own ' lag ' key, so it cannot follow admission's placement by copy-paste. Co-Authored-By: Claude Opus 5.5 --- scripts/ci/queue_janitor.py | 7 ++++++- tests/test_ci_change_areas.py | 2 ++ tests/test_ci_pr_runner_pool.py | 8 ++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/scripts/ci/queue_janitor.py b/scripts/ci/queue_janitor.py index 5813c4fbbcab..0a81062609dc 100644 --- a/scripts/ci/queue_janitor.py +++ b/scripts/ci/queue_janitor.py @@ -425,7 +425,12 @@ def pool_load_snapshot( POOL_QUEUED_JOB_STATUSES | RUNNING_JOB_STATUSES): seen[runner_pool(job)] = seen.get(runner_pool(job), 0) + 1 marker = (markers or {}).get(run.get("id")) - if marker and run.get("status") != "completed": + # A run whose owned jobs all finished holds no owned machine, even while + # its Blacksmith jobs (per-job placement) keep it in flight. Before its + # first owned job exists, the marker still reserves its peak. + owned_jobs = [job for job in jobs_by_run.get(run.get("id"), ()) if is_macos_job(job) and owned_label(job)] + released = bool(owned_jobs) and all(job.get("status") == "completed" for job in owned_jobs) + if marker and run.get("status") != "completed" and not released: seen[marker[0]] = max(seen.get(marker[0], 0), marker[1]) for label, count in seen.items(): committed[label] = committed.get(label, 0) + count diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index 7c3ac804072f..12b76c408743 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -4295,6 +4295,8 @@ def product_consumer_route_violations(workflow: dict) -> list[str]: continue if ( name == "tests-build-and-lag" + # Its own owned_jobs key, so it never follows admission's placement. + and "' lag '" in runs_on and runs_on.replace("vars.MACOS_RUNNER_DISPLAY", "vars.MACOS_RUNNER_15").replace( "' lag '", "' admission '") == producer["runs-on"] and xcode == producer["env"]["CMUX_CI_XCODE_APP"] diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 9ca4248c6652..e5b778956452 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -378,6 +378,14 @@ def test_owned_pool_commitments_count_jobs_not_created_yet(self): "oldest_queued_minutes": 0, "committed": 4}) self.assertEqual(owned_choice(snap, machines=6, order=f"{mini},{LARGE}").runner, LARGE) self.assertEqual(owned_choice(snap, machines=7, order=f"{mini},{LARGE}").runner, mini) + # Its owned jobs done, a run still busy on Blacksmith frees its minis. + done = [self.job(mini, "completed"), self.job(LARGE, "in_progress")] + snap = janitor.pool_load_snapshot([runs[0]], {1: done}, now=NOW, markers={1: (mini, 4)}) + self.assertEqual(snap["pools"].get(mini, {}).get("committed", 0), 0) + # One owned job still running keeps the whole peak reserved. + snap = janitor.pool_load_snapshot([runs[0]], {1: [*done, self.job(mini, "queued")]}, now=NOW, + markers={1: (mini, 4)}) + self.assertEqual(snap["pools"][mini]["committed"], 4) def test_owned_marker_names_this_attempts_pool_and_peak(self): run = {"id": 42, "run_attempt": 1} From 5cadea63d833f4a601c347583f52ee4a39028ef7 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 20:19:23 -0400 Subject: [PATCH 5/5] ci: normalize the owned key in the transport route test; hold a run's minis until its peak finishes The CLI product and app-host shard routes differ only by their owned_jobs key, so the transport test compares them with the key normalized. The janitor now releases a marked run's owned machines only once as many owned jobs as its peak have completed: shard jobs exist only after admission. Co-Authored-By: Claude Opus 5.5 --- scripts/ci/queue_janitor.py | 9 ++++++--- tests/test_ci_parallel_artifact_transport.py | 6 ++++-- tests/test_ci_pr_runner_pool.py | 7 ++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/scripts/ci/queue_janitor.py b/scripts/ci/queue_janitor.py index 0a81062609dc..968c8c389e54 100644 --- a/scripts/ci/queue_janitor.py +++ b/scripts/ci/queue_janitor.py @@ -426,10 +426,13 @@ def pool_load_snapshot( seen[runner_pool(job)] = seen.get(runner_pool(job), 0) + 1 marker = (markers or {}).get(run.get("id")) # A run whose owned jobs all finished holds no owned machine, even while - # its Blacksmith jobs (per-job placement) keep it in flight. Before its - # first owned job exists, the marker still reserves its peak. + # its Blacksmith jobs (per-job placement) keep it in flight. Shard jobs + # exist only after admission finishes, so the marker keeps reserving its + # peak until that many owned jobs have completed. owned_jobs = [job for job in jobs_by_run.get(run.get("id"), ()) if is_macos_job(job) and owned_label(job)] - released = bool(owned_jobs) and all(job.get("status") == "completed" for job in owned_jobs) + done = sum(1 for job in owned_jobs if job.get("status") == "completed") + released = (bool(owned_jobs) and done == len(owned_jobs) + and (not marker or done >= marker[1])) if marker and run.get("status") != "completed" and not released: seen[marker[0]] = max(seen.get(marker[0], 0), marker[1]) for label, count in seen.items(): diff --git a/tests/test_ci_parallel_artifact_transport.py b/tests/test_ci_parallel_artifact_transport.py index cb86f8a38a27..2129b67a1ef5 100644 --- a/tests/test_ci_parallel_artifact_transport.py +++ b/tests/test_ci_parallel_artifact_transport.py @@ -128,9 +128,11 @@ def test_cli_product_lane_keeps_the_consumer_transport_chain(self): jobs = yaml.safe_load(WORKFLOW)["jobs"] # Same permissions: the R2 transport needs id-token to mint its token. self.assertEqual(jobs["cli-product-tests"]["permissions"], jobs["app-host-unit-tests"]["permissions"]) + # The two routes differ only by the job's owned_jobs key (#14318). self.assertEqual( - step_block(block, "Verify GitHub-hosted route"), - step_block(job_block("app-host-unit-tests"), "Verify GitHub-hosted route"), + step_block(block, "Verify GitHub-hosted route").replace("' cli-product '", "KEY"), + step_block(job_block("app-host-unit-tests"), "Verify GitHub-hosted route") + .replace("format(' shard-{0} ', matrix.shard)", "KEY"), ) def test_layer_transport_prefers_parallel_reads_and_keeps_the_stream_fallback(self): diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index e5b778956452..31fd80383bfe 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -378,8 +378,13 @@ def test_owned_pool_commitments_count_jobs_not_created_yet(self): "oldest_queued_minutes": 0, "committed": 4}) self.assertEqual(owned_choice(snap, machines=6, order=f"{mini},{LARGE}").runner, LARGE) self.assertEqual(owned_choice(snap, machines=7, order=f"{mini},{LARGE}").runner, mini) + # Shards exist only after admission: one finished owned job of a peak + # of 4 still reserves the peak. + early = [self.job(mini, "completed"), self.job(LARGE, "in_progress")] + snap = janitor.pool_load_snapshot([runs[0]], {1: early}, now=NOW, markers={1: (mini, 4)}) + self.assertEqual(snap["pools"][mini]["committed"], 4) # Its owned jobs done, a run still busy on Blacksmith frees its minis. - done = [self.job(mini, "completed"), self.job(LARGE, "in_progress")] + done = [*(self.job(mini, "completed") for _ in range(4)), self.job(LARGE, "in_progress")] snap = janitor.pool_load_snapshot([runs[0]], {1: done}, now=NOW, markers={1: (mini, 4)}) self.assertEqual(snap["pools"].get(mini, {}).get("committed", 0), 0) # One owned job still running keeps the whole peak reserved.