From fe85eb299c5bc7867c68e39042ed4e8b84a67100 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 12:09:55 -0400 Subject: [PATCH 1/5] ci: fix owned pool review items before the fleet is switched on (a) A re-run of failed jobs reuses attempt 1's changes outputs, so it went back to the owned pool with no watcher. A persistent choice now also names retry_runner, the Blacksmith pool the same rule picks on the lane's Xcode, and every pull request macOS runs-on (and the app-host shards) takes it from attempt 2 on. (b) The picker now runs after the suite choice and counts this run's peak macOS jobs from its routing (up to 11 for a full suite) instead of CI_OWNED_POOL_JOBS_PER_RUN, which is removed. The rescue marker carries the peak and pool; with owned pools on, the janitor reads it into a per-pool committed count, so a run whose later jobs do not exist yet still holds their machines. Runs replayed since the snapshot are charged the largest peak, so a miscount leaves minis idle instead of queueing. (c) An owned-pool run never publishes a persistent-compile route request. (d) The janitor's behavior on owned pools is documented. (e) Bad CI_OWNED_POOL_SLOTS entries each raise a workflow warning and a summary line while owned pools are on. The guard's picker route check covers the retry runner and the marker name. Everything stays dormant until CI_PR_POOL_OWNED is 1. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-macos.yml | 21 ++- .github/workflows/ci-queue-janitor.yml | 3 + .github/workflows/ci.yml | 122 ++++++++------ .github/workflows/cli-pipe-regressions.yml | 9 +- .github/workflows/remote-daemon.yml | 9 +- docs/ci-runners.md | 51 +++++- scripts/ci/owned_pool_rescue.py | 13 +- scripts/ci/pr_runner_pool.py | 180 +++++++++++++++------ scripts/ci/queue_janitor.py | 80 ++++++++- tests/test_ci_change_areas.py | 6 +- tests/test_ci_owned_pool_rescue.py | 2 +- tests/test_ci_pr_runner_pool.py | 171 +++++++++++++++++--- tests/test_ci_self_hosted_guard.sh | 31 +++- 13 files changed, 541 insertions(+), 157 deletions(-) diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index cc7e84e09643..107bd1ccb48b 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -85,6 +85,13 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent (an owned Mac pool): the + # Blacksmith pool on the same Xcode that a re-run of failed jobs takes, + # because such a re-run reuses attempt 1's pick (pr_runner_pool.py). + pr_retry_runner: + required: false + default: "" + type: string pr_xcode_app: required: false default: "" @@ -111,7 +118,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') && (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 && 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 }} @@ -152,7 +159,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') && (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 && 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 @@ -1052,7 +1059,9 @@ 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. - runs-on: ${{ needs.macos-compile-admission.outputs.runner }} + # 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 }} timeout-minutes: 75 strategy: # A pull request wants every shard's failures in one run. A merge group @@ -1134,7 +1143,7 @@ jobs: - name: Verify GitHub-hosted route env: RUNNER_ENVIRONMENT: ${{ runner.environment }} - REQUESTED_RUNNER: ${{ needs.macos-compile-admission.outputs.runner }} + REQUESTED_RUNNER: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} RUNNER_CONTEXT_NAME: ${{ runner.name }} run: | set -euo pipefail @@ -2748,7 +2757,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') && (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 && 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 }} @@ -2761,7 +2770,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') && (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 && 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-queue-janitor.yml b/.github/workflows/ci-queue-janitor.yml index 7d3f9fc7fe15..6089a3f627ba 100644 --- a/.github/workflows/ci-queue-janitor.yml +++ b/.github/workflows/ci-queue-janitor.yml @@ -74,6 +74,9 @@ jobs: PR_POOL_OVERFLOW: ${{ vars.CI_PR_POOL_OVERFLOW }} PR_POOL_ORDER: ${{ vars.CI_PR_POOL_ORDER }} PR_POOL_MAX_QUEUED: ${{ vars.CI_PR_POOL_MAX_QUEUED }} + # Owned pools on: read each candidate run's owned-pool marker for the + # peak it declared. Not copied into the snapshot; forks never use them. + PR_POOL_OWNED: ${{ vars.CI_PR_POOL_OWNED }} run: python3 scripts/ci/queue_janitor.py # What this sweep saw on each macOS pool. ci.yml's `changes` job reads diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6db6e4edafae..fa7b85f60bc1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,6 +86,10 @@ jobs: # pull request, and any uncertainty. macos_pr_runner: ${{ steps.macos-pool.outputs.runner }} macos_pr_xcode_app: ${{ steps.macos-pool.outputs.xcode_app }} + # Set only when the pool is persistent (owned Macs): the Blacksmith pool + # 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 }} permissions: actions: read contents: read @@ -96,58 +100,6 @@ jobs: with: fetch-depth: 2 - - name: Choose the pull request macOS pool - id: macos-pool - # Fail-safe: any error leaves the outputs empty, which is today's route. - continue-on-error: true - timeout-minutes: 1 - env: - GH_TOKEN: ${{ github.token }} - EVENT_NAME: ${{ github.event_name }} - HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} - DEFAULT_RUNNER: ${{ vars.MACOS_RUNNER_PR }} - POOL_OVERFLOW: ${{ vars.CI_PR_POOL_OVERFLOW }} - POOL_ORDER: ${{ vars.CI_PR_POOL_ORDER }} - POOL_MAX_QUEUED: ${{ vars.CI_PR_POOL_MAX_QUEUED }} - # Owned Mac pools (off unless 1), their machine counts, and the lane - # 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 }} - OWNED_JOBS_PER_RUN: ${{ vars.CI_OWNED_POOL_JOBS_PER_RUN }} - 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 }} - run: python3 scripts/ci/pr_runner_pool.py - - # A job queued on a persistent pool (owned Macs) waits for it however long - # it stays busy. This marker tells ci-owned-pool-rescue.yml, running from - # main, to watch the run and re-run it on Blacksmith when a job waits past - # CI_OWNED_POOL_RESCUE_SECONDS. No persistent pool, no marker, no watching. - # Fail-safe like the picker: a missing marker only means the run is not - # watched, which is how every run behaved before the rescue existed. - - name: Mark a run on a persistent macOS pool - id: macos-pool-marker - if: ${{ steps.macos-pool.outputs.persistent == 'true' }} - continue-on-error: true - env: - POOL: ${{ steps.macos-pool.outputs.runner }} - run: | - set -euo pipefail - marker="$RUNNER_TEMP/macos-pool-persistent.json" - POOL="$POOL" MARKER="$marker" python3 -c 'import json,os; json.dump({"pool": os.environ["POOL"]}, open(os.environ["MARKER"], "w"))' - echo "path=$marker" >> "$GITHUB_OUTPUT" - - - name: Upload the persistent pool marker - if: ${{ steps.macos-pool-marker.outputs.path != '' }} - continue-on-error: true - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: macos-pool-persistent-${{ github.run_id }}-${{ github.run_attempt }} - path: ${{ steps.macos-pool-marker.outputs.path }} - if-no-files-found: error - retention-days: 1 - compression-level: 0 - - name: Record GitHub-selected source identity id: source-identity run: | @@ -621,6 +573,67 @@ jobs: ${files_args[@]+"${files_args[@]}"} \ --github-output "$GITHUB_OUTPUT" + - name: Choose the pull request macOS pool + id: macos-pool + # Fail-safe: any error leaves the outputs empty, which is today's route. + continue-on-error: true + timeout-minutes: 1 + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + DEFAULT_RUNNER: ${{ vars.MACOS_RUNNER_PR }} + POOL_OVERFLOW: ${{ vars.CI_PR_POOL_OVERFLOW }} + POOL_ORDER: ${{ vars.CI_PR_POOL_ORDER }} + POOL_MAX_QUEUED: ${{ vars.CI_PR_POOL_MAX_QUEUED }} + # Owned Mac pools (off unless 1), their machine counts, and the lane + # 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 }} + 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 }} + # This run's macOS jobs, from the routing above, so an owned pool is + # taken only when every job of the run gets a machine at once. + RUN_MACOS: ${{ steps.detect.outputs.macos }} + 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_CLAUDE_WRAPPER: ${{ steps.standalone.outputs.claude_wrapper }} + RUN_CLI: ${{ steps.detect.outputs.cli }} + RUN_REMOTE_DAEMON: ${{ steps.standalone.outputs.remote_daemon }} + run: python3 scripts/ci/pr_runner_pool.py + + # A job queued on a persistent pool (owned Macs) waits for it however long + # it stays busy. This marker tells ci-owned-pool-rescue.yml, running from + # main, to watch the run and re-run it on Blacksmith when a job waits past + # CI_OWNED_POOL_RESCUE_SECONDS. No persistent pool, no marker, no watching. + # Fail-safe like the picker: a missing marker only means the run is not + # watched, which is how every run behaved before the rescue existed. + - name: Mark a run on a persistent macOS pool + id: macos-pool-marker + if: ${{ steps.macos-pool.outputs.persistent == 'true' }} + continue-on-error: true + env: + POOL: ${{ steps.macos-pool.outputs.runner }} + run: | + set -euo pipefail + marker="$RUNNER_TEMP/macos-pool-persistent.json" + POOL="$POOL" MARKER="$marker" python3 -c 'import json,os; json.dump({"pool": os.environ["POOL"]}, open(os.environ["MARKER"], "w"))' + echo "path=$marker" >> "$GITHUB_OUTPUT" + + - name: Upload the persistent pool marker + if: ${{ steps.macos-pool-marker.outputs.path != '' }} + continue-on-error: true + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + # The janitor reads the run's peak and pool from the name. + name: macos-pool-persistent-${{ github.run_id }}-${{ github.run_attempt }}-${{ steps.macos-pool.outputs.jobs }}-${{ steps.macos-pool.outputs.runner }} + path: ${{ steps.macos-pool-marker.outputs.path }} + if-no-files-found: error + retention-days: 1 + compression-level: 0 + - name: Route Linux guard suites id: linux_guards env: @@ -897,6 +910,7 @@ jobs: uses: ./.github/workflows/remote-daemon.yml with: pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} + pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} native_tests: ${{ needs.changes.outputs.remote_daemon_native == 'true' || contains(github.event.pull_request.labels.*.name, 'full-ci') }} cli: @@ -905,6 +919,7 @@ jobs: uses: ./.github/workflows/cli-pipe-regressions.yml with: pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} + pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} pr_xcode_app: ${{ needs.changes.outputs.macos_pr_xcode_app }} web: @@ -921,7 +936,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' && (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 && 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 @@ -1052,6 +1067,7 @@ jobs: cache_backend: ${{ inputs.cache_backend }} release_archs: ${{ inputs.release_archs }} pr_runner: ${{ needs.changes.outputs.macos_pr_runner }} + pr_retry_runner: ${{ needs.changes.outputs.macos_pr_retry_runner }} 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 dc02ca9d3dfe..78e39b5271a7 100644 --- a/.github/workflows/cli-pipe-regressions.yml +++ b/.github/workflows/cli-pipe-regressions.yml @@ -11,6 +11,13 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent (an owned Mac pool): the + # Blacksmith pool on the same Xcode that a re-run of failed jobs takes, + # because such a re-run reuses attempt 1's pick (pr_runner_pool.py). + pr_retry_runner: + required: false + default: "" + type: string pr_xcode_app: required: false default: "" @@ -26,7 +33,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' && (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 && 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 8f41184fe5c6..dcbe532ae111 100644 --- a/.github/workflows/remote-daemon.yml +++ b/.github/workflows/remote-daemon.yml @@ -14,6 +14,13 @@ on: required: false default: "" type: string + # Set only when pr_runner is persistent (an owned Mac pool): the + # Blacksmith pool on the same Xcode that a re-run of failed jobs takes, + # because such a re-run reuses attempt 1's pick (pr_runner_pool.py). + pr_retry_runner: + required: false + default: "" + type: string push: branches: [main] paths: @@ -87,7 +94,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' && (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 && 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/docs/ci-runners.md b/docs/ci-runners.md index 333283c0ce51..d663bc02afed 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -175,9 +175,17 @@ Xcode on it. With `CI_PR_POOL_OWNED=1` the default order is (16 GB), then the Blacksmith pools as overflow. An owned pool's capacity is its entry in `CI_OWNED_POOL_SLOTS`, and the janitor's snapshot counts the jobs queued and running on that label. A pull request run puts several macOS jobs on its pool -at once, so a run takes the owned pool only when `CI_OWNED_POOL_JOBS_PER_RUN` -machines (default 3) are still free after the jobs already there and the runs -created since the snapshot. It is skipped when the snapshot is older than 20 +at once, each on its own machine, so a run takes the owned pool only when its +own peak is free at once. The picker runs after the suite choice and counts +that peak from the run's routing: the Claude wrapper, CLI pipe and remote +daemon lanes, beside the larger of compile admission alone or what follows it +(a full suite's seven app-host shards and tests-build-and-lag, 11 jobs in all; +a changed-suites run's one shard). Taken is the larger of the jobs the janitor +saw on the pool and `committed`, the peaks the runs holding it declared, so a +run whose later jobs do not exist yet still counts them. A run created since +the snapshot has an unknown peak: any that could have taken the pool is +assumed to, and charged 11 machines, so a wrong guess leaves minis idle +rather than queueing a job there. It is skipped when the snapshot is older than 20 minutes or the label has no slots, and it is never the fewest-queued fallback. Fork runs and retry attempts never take it. While `CI_PR_POOL_OWNED` is off, owned labels in `CI_PR_POOL_ORDER` are dropped and the rest of the order is @@ -190,19 +198,46 @@ names no owned pool. | --- | --- | --- | | `CI_PR_POOL_OWNED` | unset (off) | `1` puts owned pools first and turns on the rescue below | | `CI_OWNED_POOL_SLOTS` | unset (no slots) | JSON, owned pool label to machine count, the `conforming_count` from `glaeda-mini-fleet pools --json`: `{"glaeda-std-xcode-26.6": 11, "glaeda-light-xcode-26.6": 2}` | -| `CI_OWNED_POOL_JOBS_PER_RUN` | `3` | machines a run needs free to take an owned pool (1 to 10) | + +Each entry of `CI_OWNED_POOL_SLOTS` that is not an owned label with a positive +whole number of machines counts as none. While owned pools are on, the +`changes` job raises a workflow warning and a summary line for each such entry, +so a typo shows up on every run instead of quietly leaving a pool unused. An owned pool is persistent, which needs one more rule because GitHub never re-routes a queued job: one queued there waits for that pool however long it stays busy. An offline mini still counts as a slot, and the snapshot can be minutes old. When the picker chooses a persistent pool, `changes` -uploads a `macos-pool-persistent--` marker, and +uploads a `macos-pool-persistent----` marker (the +janitor reads the run's peak and pool from its name), and `ci-owned-pool-rescue.yml` (from `main`, with Actions write) watches that run. If one of its jobs waits for a persistent runner longer than `CI_OWNED_POOL_RESCUE_SECONDS` (default 90, 30 to 600), the watcher confirms the pull request head has not moved, cancels the run, and re-runs it. A retry attempt never takes a persistent pool, so the re-run lands on Blacksmith as a -whole, and so does any manual re-run after a job failed on an owned Mac. +whole, and so does a manual "Re-run all jobs". + +"Re-run failed jobs" is different: `changes` passed, so it is not re-run, and +the failed jobs read attempt 1's outputs, owned pool included, with no watcher +(the rescue follows attempt 1 only). So a persistent choice also names +`retry_runner`, the Blacksmith pool the same rule picks on the lane's own +Xcode, which is also the Xcode the owned label names. Every pull request macOS +`runs-on`, and the app-host shards that otherwise inherit compile admission's +pool, reads `github.run_attempt > 1 && inputs.pr_retry_runner` first. It is +empty for a run on Blacksmith, so those re-run where they ran. + +An owned-pool run never publishes a persistent-compile route request +(`CI_PERSISTENT_MAC_COMPILE`): its compile admission already runs on an owned +Mac, and the two would compete for the same machines. + +The queue janitor treats an owned label as one more macOS pool. A stale pull +request run (category b: closed, merged or superseded) is cancelled there on +every sweep whatever the queue, which frees minis for current work. The other +categories cancel only while more than `CI_JANITOR_QUEUE_THRESHOLD` jobs queue +on a pool the run holds, owned pools included. With `CI_PR_POOL_OWNED=1` the +janitor also lists the artifacts of each in-flight attempt-1, same-repository +pull request CI run that has no macOS job on another pool, one request per +run, to read its marker's peak into `committed`. | Variable | Default | Effect | | --- | --- | --- | @@ -222,8 +257,8 @@ jobs only as `pr_runner` or on a `pull_request` `runs-on` branch. `CI_PR_POOL_ORDER` is the one variable that may name owned labels (the guard's `owned` pattern, which must match `pr_runner_pool.OWNED_LABEL`), and the CI health report checks every other entry in it against the workflow policy. -Owned pools stay off until `CI_PR_POOL_OWNED`, `CI_OWNED_POOL_SLOTS` and -`CI_PR_POOL_ORDER` are all set. +Owned pools stay off until `CI_PR_POOL_OWNED` is 1 and `CI_OWNED_POOL_SLOTS` +gives the lane's owned label machines. `MACOS_RUNNER_PR` does not move a lane on its own. A runner change and its Xcode pin still have to agree, because `scripts/select-ci-xcode.sh` exits diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 1fd8d224e394..8ab96ab26f94 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -9,7 +9,8 @@ The script waits for ci.yml's `changes` job, which runs the picker. When the picker chose a persistent pool, that job uploads a marker artifact -(`macos-pool-persistent--`); no marker means the run is on an +(`macos-pool-persistent----`, the jobs and pool +for the janitor's count); no marker means the run is on an ephemeral pool and the watch ends. Otherwise it watches the run's jobs until the run finishes. If a job on the persistent pool is still queued with no runner after the budget (CI_OWNED_POOL_RESCUE_SECONDS, 90 by default), it confirms the @@ -181,9 +182,10 @@ def jobs(self, run_id: int, attempt: int) -> list[Mapping[str, Any]]: break return found - def has_artifact(self, run_id: int, name: str) -> bool: - data = self.request("GET", f"/actions/runs/{run_id}/artifacts?name={name}&per_page=10") - return any(item.get("name") == name for item in (data or {}).get("artifacts") or []) + def has_artifact(self, run_id: int, prefix: str) -> bool: + """Whether the run uploaded an artifact whose name starts with `prefix`.""" + data = self.request("GET", f"/actions/runs/{run_id}/artifacts?per_page=100") + return any(str(item.get("name") or "").startswith(prefix) for item in (data or {}).get("artifacts") or []) def pull(self, number: int) -> Mapping[str, Any]: return self.request("GET", f"/pulls/{number}") @@ -226,7 +228,8 @@ def target_from_event(event: Mapping[str, Any], repository: str) -> Target | str def marker_name(target: Target) -> str: - return f"{MARKER_PREFIX}-{target.run_id}-{target.attempt}" + """The marker's name up to its jobs and pool, which only the janitor reads.""" + return f"{MARKER_PREFIX}-{target.run_id}-{target.attempt}-" READ_ERRORS = (urllib.error.URLError, http.client.HTTPException, OSError, ValueError) diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index bcc4269f3d4e..d0c3e855d498 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -39,12 +39,17 @@ capacity is the number of machines the fleet manifest gives each label, published as vars.CI_OWNED_POOL_SLOTS (JSON, `{"glaeda-std-xcode-26.6": 11}`). The janitor's snapshot counts the jobs queued and running on each owned label -from the job listings it already makes, so no token beyond GITHUB_TOKEN is -needed. An owned pool is skipped when it has no slot count, or when the +from the job listings it already makes, and `committed`: what the runs +holding the pool need at their peak, read from the marker each one uploads +(`macos-pool-persistent----`), so a run whose later +jobs do not exist yet still counts them. No token beyond GITHUB_TOKEN is +needed. A run takes an owned pool only when its own peak (run_jobs) is free +at once. An owned pool is skipped when it has no slot count, or when the snapshot is older than OWNED_MAX_AGE_MINUTES. An offline machine still counts -as a slot; what that and the snapshot's age get wrong, -ci-owned-pool-rescue.yml catches: a run whose job waits on an owned pool past -its budget is re-run on Blacksmith. The owned order is `std` (48 GB minis), +as a slot; what that gets wrong, ci-owned-pool-rescue.yml catches: a run whose +job waits on an owned pool past its budget is re-run on Blacksmith. A re-run +of failed jobs reuses this run's outputs, so a persistent choice also names +`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. The queue comes from the queue janitor, which lists every in-flight run's @@ -117,14 +122,16 @@ PR_XCODE_VARIABLE = "CMUX_CI_XCODE_APP_PR" OWNED_VARIABLE = "CI_PR_POOL_OWNED" SLOTS_VARIABLE = "CI_OWNED_POOL_SLOTS" -# A pull request run puts several macOS jobs on its pool at once (compile -# admission, tests-build-and-lag, the Claude wrapper, CLI pipe and remote -# daemon lanes), each on its own owned machine. A run takes an owned pool only -# when that many machines are free, so its later jobs do not queue there and -# trip the rescue. -JOBS_PER_RUN_VARIABLE = "CI_OWNED_POOL_JOBS_PER_RUN" -DEFAULT_JOBS_PER_RUN = 3 -MAX_JOBS_PER_RUN = 10 +# 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 +# daemon lanes; once admission passes, a full suite adds APP_HOST_SHARDS +# shards and tests-build-and-lag, and a changed-suites run one shard. A run +# takes an owned pool only when its own peak (run_jobs) is free, so none of +# its jobs queues there and trips the rescue. A run whose peak is unknown, and +# every run replayed since the snapshot, is charged MAX_RUN_JOBS. +APP_HOST_SHARDS = 7 +SIDE_LANES = 3 +MAX_RUN_JOBS = SIDE_LANES + APP_HOST_SHARDS + 1 # A snapshot older than this is not trusted to place a run on an owned pool. OWNED_MAX_AGE_MINUTES = 20 # Pools whose machines are discarded after each job; the only ones a fork run may use. @@ -151,7 +158,6 @@ class Settings: order: tuple[str, ...] = DEFAULT_ORDER max_queued: int = DEFAULT_MAX_QUEUED - jobs_per_run: int = DEFAULT_JOBS_PER_RUN # Owned labels the order named for another Xcode than the lane's pin. stale: tuple[str, ...] = () @@ -174,11 +180,35 @@ class Choice: runner: str # "" keeps every job's own fallback expression xcode_app: str # "" keeps every job's own Xcode pin reason: str + # 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 = "" + + +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, + 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. + + 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. + """ + side = sum(flag(lane) for lane in (cli, remote_daemon)) + full = flag(macos) and flag(full_suite) + side += flag(claude_wrapper) or full + if not flag(macos): + return side + after = APP_HOST_SHARDS + 1 if full else int(flag(unit_suite) and not flag(unit_in_admission)) + return side + max(1, after) def settings(overflow: str | None, order: str | None, max_queued: str | None, - owned: str | None = None, pr_xcode_app: str | None = None, - jobs_per_run: str | None = None) -> Settings | None: + owned: str | None = None, pr_xcode_app: str | None = None) -> Settings | None: """Settings from repository variables; None when turned off or invalid. Owned pools are dropped from the order unless `owned` is "1", even when @@ -206,12 +236,11 @@ def settings(overflow: str | None, order: str | None, max_queued: str | None, return None try: limit = int(max_queued) if (max_queued or "").strip() else DEFAULT_MAX_QUEUED - weight = int(jobs_per_run) if (jobs_per_run or "").strip() else DEFAULT_JOBS_PER_RUN except ValueError: return None - if limit < 1 or not 1 <= weight <= MAX_JOBS_PER_RUN: + if limit < 1: return None - return Settings(labels, limit, weight, stale) + return Settings(labels, limit, stale) def parse_time(value: str | None) -> dt.datetime | None: @@ -232,27 +261,50 @@ def snapshot_age_minutes(snapshot: Mapping[str, Any], now: dt.datetime) -> float def slots(raw: str | None) -> dict[str, int]: """CI_OWNED_POOL_SLOTS: owned pool label -> machines. Anything malformed counts as none.""" + return _slots(raw)[0] + + +def slot_problems(raw: str | None) -> list[str]: + """Why CI_OWNED_POOL_SLOTS, or an entry of it, counts as no machines. + + The picker treats all of these as zero slots, which is safe but silent: a + mistyped label or a count of "11" or 11.0 just leaves the pool unused. + main() turns each one into a workflow warning. + """ + return _slots(raw)[1] + + +def _slots(raw: str | None) -> tuple[dict[str, int], list[str]]: + if not (raw or "").strip(): + return {}, [] try: - data = json.loads(raw or "{}") - except ValueError: - return {} + data = json.loads(raw or "") + except ValueError as error: + return {}, [f"{SLOTS_VARIABLE} is not JSON ({error})"] if not isinstance(data, Mapping): - return {} - counted = {} + return {}, [f"{SLOTS_VARIABLE} is not a JSON object"] + counted, problems = {}, [] for label, count in data.items(): - if persistent(str(label)) and isinstance(count, int) and not isinstance(count, bool) and count > 0: + if not persistent(str(label)): + problems.append(f"{SLOTS_VARIABLE} entry {label!r} is not an owned pool label (glaeda--xcode-)") + elif not isinstance(count, int) or isinstance(count, bool) or count <= 0: + problems.append(f"{SLOTS_VARIABLE} entry {label!r} has {count!r} machines, not a positive whole number") + else: counted[str(label)] = count - return counted + return counted, problems def pool(snapshot: Mapping[str, Any], label: str, owned_slots: Mapping[str, int] | None = None) -> Mapping[str, int]: """One pool's counts; a pool the janitor saw no job on is empty, not unknown. `capacity` is POOL_CAPACITY for a Blacksmith pool and the slot count for - an owned pool (0 when CI_OWNED_POOL_SLOTS gives it none). + an owned pool (0 when CI_OWNED_POOL_SLOTS gives it none). `committed` is + what the janitor counted the runs holding an owned pool to need at their + peak, including jobs they have not created yet. """ entry = (snapshot.get("pools") or {}).get(label) or {} - counts = {key: int(entry.get(key) or 0) for key in ("queued", "running", "reserved_queued", "oldest_queued_minutes")} + counts = {key: int(entry.get(key) or 0) + for key in ("queued", "running", "reserved_queued", "oldest_queued_minutes", "committed")} counts["capacity"] = int((owned_slots or {}).get(label) or 0) if persistent(label) else POOL_CAPACITY return counts @@ -279,23 +331,30 @@ def effective_queue(counts: Mapping[str, int], added: int) -> int: return counts["queued"] + max(0, added - idle) -def owned_free(counts: Mapping[str, int], added_runs: int, jobs_per_run: int) -> int: - """Machines of an owned pool still free once `added_runs` more runs took theirs.""" - return counts.get("capacity", 0) - counts["running"] - counts["queued"] - added_runs * jobs_per_run +def owned_free(counts: Mapping[str, int], added_runs: int) -> int: + """Machines of an owned pool still free once `added_runs` more runs took theirs. + + Taken is the larger of the jobs the janitor saw and what the runs holding + the pool will need at their peak, so a run whose later jobs do not exist + yet still counts them. Each run replayed since the snapshot is charged + MAX_RUN_JOBS, since its peak is unknown here. + """ + taken = max(counts["running"] + counts["queued"], counts.get("committed", 0)) + return counts.get("capacity", 0) - taken - added_runs * MAX_RUN_JOBS def pick(load: Mapping[str, Mapping[str, int]], added: Mapping[str, int], usable: Sequence[str], - max_queued: int, jobs_per_run: int = DEFAULT_JOBS_PER_RUN) -> tuple[str, bool]: + max_queued: int, jobs: int = MAX_RUN_JOBS) -> tuple[str, bool]: """The rule itself: first usable pool with headroom, else the fewest queued. An owned pool has headroom only while every job of this run gets a machine - at once (jobs_per_run of them): a job queued there waits for that pool + at once (`jobs` of them, its peak): a job queued there waits for that pool alone. It is never the fewest-queued fallback. """ queued = {label: effective_queue(load[label], added[label]) for label in usable} for label in usable: if persistent(label): - if owned_free(load[label], added[label], jobs_per_run) >= jobs_per_run: + if owned_free(load[label], added[label]) >= max(1, jobs): return label, True elif queued[label] < max_queued: return label, True @@ -314,6 +373,7 @@ def decide( placed: Mapping[str, int] | None = None, choose_from: Sequence[str] | None = None, owned_slots: Mapping[str, int] | None = None, + jobs: int = MAX_RUN_JOBS, ) -> Choice: """The preference rule over a janitor snapshot. Uncertainty keeps today's route. @@ -323,7 +383,10 @@ def decide( job each. `auto_xcode` (a fork run, which has no pins) lets every pool fall back to each job selecting its pool's newest SDK 26 Xcode. `choose_from` limits the final pick to some pools of the order (E2E stays - on macOS 26) while the replay still spreads over the whole order. + on macOS 26) while the replay still spreads over the whole order. `jobs` + is this run's peak machine count, which an owned pool must have free. + Replayed runs are placed as if they needed one machine (so any that could + have taken an owned pool is assumed to) and charged MAX_RUN_JOBS there. """ if not isinstance(snapshot, Mapping) or not isinstance(snapshot.get("pools"), Mapping): return Choice("", "", "no readable pool snapshot") @@ -359,23 +422,31 @@ def counted(label: str) -> bool: note = f"; skipped {', '.join(skipped)} (reserved, no Xcode pin, or owned without slots or a fresh snapshot)" if skipped else "" added = {label: max(0, int((placed or {}).get(label) or 0)) for label in usable} for _ in range(max(0, routed_since)): - earlier, _ = pick(load, added, usable, limits.max_queued, limits.jobs_per_run) + earlier, _ = pick(load, added, usable, limits.max_queued, jobs=1) added[earlier] += 1 - label, headroom = pick(load, added, candidates, limits.max_queued, limits.jobs_per_run) + label, headroom = pick(load, added, candidates, limits.max_queued, jobs) 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()) replay = f" after replaying {replayed} newer run(s)" if replayed else "" if headroom and persistent(label): - free = owned_free(load[label], added[label], limits.jobs_per_run) - why = f"first pool in order with headroom ({free} of {load[label]['capacity']} owned machines free){replay}" + free = owned_free(load[label], added[label]) + why = (f"first pool in order with headroom ({free} of {load[label]['capacity']} owned machines free, " + f"this run needs {max(1, jobs)}){replay}") elif headroom: why = f"first pool in order with headroom (< {limits.max_queued} queued){replay}" else: why = f"no pool has headroom{replay}; fewest queued" if limits.stale: note += f"; dropped {', '.join(limits.stale)} (not the lane's Xcode pin)" - return Choice(label, xcode(label) or "", why + note) + retry = "" + if persistent(label): + # A re-run of failed jobs keeps this run's outputs, so it needs a pool + # named now: the Blacksmith pool this rule would take on the lane's + # 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) def choose( @@ -390,7 +461,7 @@ def choose( xcode_pins: Mapping[str, str], owned: str | None = None, owned_slots: str | None = None, - jobs_per_run: str | None = None, + jobs: int = MAX_RUN_JOBS, fetch: Callable[[], Mapping[str, Any] | None], count_routed: Callable[[str], int] = lambda since: 0, now: dt.datetime, @@ -405,7 +476,7 @@ def choose( if not fork: if (default_runner or "").strip() != DEFAULT_RUNNER: return Choice("", "", f"MACOS_RUNNER_PR is {default_runner or 'unset'}, not {DEFAULT_RUNNER}"), None - limits = settings(overflow, order, max_queued, owned, xcode_pins.get(PR_XCODE_VARIABLE), jobs_per_run) + limits = settings(overflow, order, max_queued, owned, xcode_pins.get(PR_XCODE_VARIABLE)) if limits is None: return Choice("", "", f"{OVERFLOW_VARIABLE} is 0, or {ORDER_VARIABLE}/{MAX_QUEUED_VARIABLE} " "is invalid"), None @@ -443,7 +514,8 @@ def choose( except Exception as error: # noqa: BLE001 - every failure keeps the default return Choice("", "", f"could not count runs since the snapshot ({error})"), snapshot choice = decide(snapshot, limits, now=now, xcode_pins={} if fork else xcode_pins, - routed_since=routed, auto_xcode=fork, owned_slots={} if fork else slots(owned_slots)) + routed_since=routed, auto_xcode=fork, owned_slots={} if fork else slots(owned_slots), + jobs=jobs) if fork and choice.runner: choice = dataclasses.replace(choice, reason=f"fork head; {choice.reason}") if retry and choice.runner: @@ -545,11 +617,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) -> str: + owned_slots: Mapping[str, int] | None = None, problems: 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}`") + for problem in problems: + lines.append(f"- **Warning:** {problem}; that pool gets no machines") if isinstance(snapshot, Mapping) and isinstance(snapshot.get("pools"), Mapping): age = snapshot_age_minutes(snapshot, now) lines.append(f"- Queue seen by the janitor at {snapshot.get('generated_at')}" @@ -586,6 +662,12 @@ def count_routed(since: str) -> int: return 0 return client().pull_request_runs_since(since, exclude_run_id=int(run_id) if run_id.isdigit() else None) + # 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( + 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")) choice, snapshot = choose( event=env.get("EVENT_NAME") or "", repo=repo, @@ -596,7 +678,7 @@ def count_routed(since: str) -> int: max_queued=env.get("POOL_MAX_QUEUED"), owned=env.get("POOL_OWNED"), owned_slots=env.get("OWNED_SLOTS"), - jobs_per_run=env.get("OWNED_JOBS_PER_RUN"), + jobs=jobs, xcode_pins={variable: env.get(variable) or "" for variable in {*POOLS.values(), PR_XCODE_VARIABLE} if variable}, fetch=fetch, @@ -604,7 +686,10 @@ def count_routed(since: str) -> int: now=now, run_attempt=int(attempt) if attempt.isdigit() else 1, ) - text = summary(choice, snapshot, now=now, owned_slots=slots(env.get("OWNED_SLOTS"))) + 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) print(text) if env.get("GITHUB_STEP_SUMMARY"): with open(env["GITHUB_STEP_SUMMARY"], "a", encoding="utf-8") as handle: @@ -612,7 +697,8 @@ def count_routed(since: str) -> int: if env.get("GITHUB_OUTPUT"): 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"persistent={'true' if persistent(choice.runner) else 'false'}\n" + f"retry_runner={choice.retry_runner}\njobs={jobs}\n") return 0 diff --git a/scripts/ci/queue_janitor.py b/scripts/ci/queue_janitor.py index bbfe209f0a69..3912be501f78 100644 --- a/scripts/ci/queue_janitor.py +++ b/scripts/ci/queue_janitor.py @@ -50,6 +50,14 @@ pick a pull request run's pool (and, through it, e2e_runner_pool.py an E2E run's) without listing every in-flight run's jobs itself. +An owned Mac pool (``glaeda--xcode-``) is one more pool +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 +``macos-pool-persistent----`` marker, read with one +artifact listing per run that may hold one. + Orphaned runs are a separate pass (find_orphans): a job the runner scheduler lost holds nothing on any pool, so that pass ignores the queue threshold, has its own cap, and may end main schedules, nightly and TestFlight runs, @@ -74,6 +82,7 @@ from typing import Any sys.path.insert(0, str(Path(__file__).resolve().parent)) +from pr_runner_pool import MAX_RUN_JOBS # noqa: E402 from pr_runner_pool import persistent as owned_pool # noqa: E402 @@ -343,12 +352,43 @@ def macos_usage(jobs: Iterable[Mapping[str, Any]]) -> MacosUsage: } +# ci.yml's `changes` job uploads this marker when the picker chose an owned +# pool: macos-pool-persistent----. +OWNED_MARKER = re.compile(r"macos-pool-persistent-(?P[0-9]+)-(?P[0-9]+)-(?P[0-9]+)-(?P.+)") + + +def owned_marker(run: Mapping[str, Any], names: Iterable[str]) -> tuple[str, int] | None: + """(pool, peak jobs) from this attempt's owned-pool marker, or None.""" + for name in names: + match = OWNED_MARKER.fullmatch(str(name)) + if (match and int(match["run"]) == run.get("id") and int(match["attempt"]) == (run.get("run_attempt") or 1) + and owned_pool(match["pool"])): + return match["pool"], min(int(match["jobs"]), MAX_RUN_JOBS) + return None + + +def may_hold_owned_pool(run: Mapping[str, Any], jobs: Sequence[Mapping[str, Any]]) -> bool: + """A run whose marker is worth one artifact listing: it may hold an owned pool. + + Only attempt 1 of a same-repository pull request run of CI can (a retry + never takes one), and not once any of its macOS jobs asked for another pool. + """ + if run.get("event") != "pull_request" or (run.get("run_attempt") or 1) != 1: + return False + if (run.get("head_repository") or {}).get("id") != (run.get("repository") or {}).get("id"): + return False + if not str(run.get("path") or "").endswith("/ci.yml"): + return False + return not any(is_macos_job(job) and not owned_label(job) for job in jobs) + + def pool_load_snapshot( runs: Sequence[Mapping[str, Any]], jobs_by_run: Mapping[int, Sequence[Mapping[str, Any]]], *, now: dt.datetime, settings: Mapping[str, str] | None = None, + markers: Mapping[int, tuple[str, int]] | None = None, ) -> dict[str, Any]: """Per-pool macOS demand from the jobs this sweep already listed. @@ -363,12 +403,26 @@ def pool_load_snapshot( A job on an owned pool (`glaeda--xcode-`) is keyed by that label (runner_pool); its counts are how pr_runner_pool.py knows how - many of the pool's machines are taken. + many of the pool's machines are taken. An owned pool also gets + `committed`: for each run holding it, the larger of the jobs seen there + and the peak its marker declares (`markers`, run id -> (pool, jobs)), so + a run whose later jobs do not exist yet still counts them. """ pools: dict[str, dict[str, Any]] = {} oldest: dict[str, dt.datetime] = {} + committed: dict[str, int] = {} for run in runs: reserved = bool(RESERVED_POOL_WORKFLOW.search(f"{run.get('name') or ''} {run.get('path') or ''}")) + seen: dict[str, int] = {} + for job in jobs_by_run.get(run.get("id"), ()): + if is_macos_job(job) and owned_label(job) and job.get("status") in ( + 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": + seen[marker[0]] = max(seen.get(marker[0], 0), marker[1]) + for label, count in seen.items(): + committed[label] = committed.get(label, 0) + count for job in jobs_by_run.get(run.get("id"), ()): if not is_macos_job(job): continue @@ -389,6 +443,9 @@ def pool_load_snapshot( oldest[pool] = created for pool, created in oldest.items(): pools[pool]["oldest_queued_minutes"] = max(0, int((now - created).total_seconds() // 60)) + for pool, count in committed.items(): + pools.setdefault(pool, {"queued": 0, "running": 0, "reserved_queued": 0, + "oldest_queued_minutes": 0})["committed"] = count return { "version": POOL_LOAD_VERSION, "generated_at": now.strftime("%Y-%m-%dT%H:%M:%SZ"), @@ -1141,6 +1198,11 @@ def in_flight_runs(self) -> list[dict[str, Any]]: break return list(runs.values()) + def artifact_names(self, run_id: int) -> list[str]: + query = urllib.parse.urlencode({"per_page": 100}) + payload = self.request("GET", f"/repos/{self.repo}/actions/runs/{run_id}/artifacts?{query}") + return [str(item.get("name") or "") for item in payload.get("artifacts") or []] + def jobs(self, run_id: int) -> list[dict[str, Any]]: jobs: list[dict[str, Any]] = [] for page in range(1, MAX_JOB_PAGES + 1): @@ -1248,8 +1310,22 @@ def main(argv: Sequence[str] | None = None) -> int: # Before any cancellation: pr_runner_pool.py wants the demand a new run # would queue behind, and the janitor's cancels are capped anyway. pool_settings = {key: os.environ.get(name, "") for name, key in POOL_SETTINGS_ENV.items()} + # Owned pools on: one artifact listing per run that may hold one, for + # the peak its marker declares. Off: no request at all. + markers: dict[int, tuple[str, int]] = {} + if os.environ.get("PR_POOL_OWNED", "").strip() == "1": + for run in runs: + if run.get("id") in jobs_by_run and may_hold_owned_pool(run, jobs_by_run[run["id"]]): + try: + found = owned_marker(run, github.artifact_names(run["id"])) + except RuntimeError as error: + print(f"queue-janitor: owned-pool marker for run {run['id']}: {error}", file=sys.stderr) + continue + if found: + markers[run["id"]] = found args.pool_load.write_text( - json.dumps(pool_load_snapshot(runs, jobs_by_run, now=now, settings=pool_settings), indent=2) + "\n", + json.dumps(pool_load_snapshot(runs, jobs_by_run, now=now, settings=pool_settings, markers=markers), + indent=2) + "\n", encoding="utf-8") plan = build_plan( diff --git a/tests/test_ci_change_areas.py b/tests/test_ci_change_areas.py index 95fc32b8a701..2913844b0d3d 100755 --- a/tests/test_ci_change_areas.py +++ b/tests/test_ci_change_areas.py @@ -4110,7 +4110,11 @@ def app_host_product_consumers(workflow: dict) -> dict[str, dict]: } -PRODUCT_RUNNER_OUTPUT = "${{ needs.macos-compile-admission.outputs.runner }}" +# 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 }}" +) PRODUCT_XCODE_OUTPUT = "${{ needs.macos-compile-admission.outputs.xcode_app }}" diff --git a/tests/test_ci_owned_pool_rescue.py b/tests/test_ci_owned_pool_rescue.py index 26813c82ac79..bdb7177f7902 100644 --- a/tests/test_ci_owned_pool_rescue.py +++ b/tests/test_ci_owned_pool_rescue.py @@ -184,7 +184,7 @@ def test_ephemeral_run_stops_after_the_marker_check(self): api = FakeAPI(clock, lambda s: [changes()(s), job("macos / macOS compile admission", labels=[BLACKSMITH])]) code, summary = run_main(api, clock) self.assertEqual(code, 0) - self.assertEqual(api.calls, ["jobs", f"artifact:macos-pool-persistent-{RUN_ID}-1"]) + self.assertEqual(api.calls, ["jobs", f"artifact:macos-pool-persistent-{RUN_ID}-1-"]) self.assertIn("the run is on an ephemeral pool", summary) def test_waits_for_the_picker_before_looking_for_the_marker(self): diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 07d5bb3b6485..013afa3e5648 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_per_run=""): + owned_slots="", jobs=pool.MAX_RUN_JOBS): 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_per_run=jobs_per_run, + owned_slots=owned_slots, jobs=jobs, fetch=fetch or (lambda: snap), count_routed=count_routed, now=NOW, run_attempt=attempt, )[0] @@ -244,7 +244,8 @@ def test_main_writes_outputs_and_summary(self): self.assertEqual(pool.main(["--snapshot", str(snap_path)], env), 0) finally: sys.stdout = old - self.assertEqual(out.read_text(), f"runner={LARGE}\nxcode_app=\npersistent=false\n") + self.assertEqual(out.read_text(), f"runner={LARGE}\nxcode_app=\npersistent=false\n" + f"retry_runner=\njobs={pool.MAX_RUN_JOBS}\n") text = summary.read_text() self.assertIn(f"Pool: `{LARGE}`", text) self.assertIn(f"{SMALL}: 21 queued, 10 running", text) @@ -319,6 +320,47 @@ def test_counts_jobs_on_an_owned_pool_label(self): self.assertEqual((snap["pools"][mini]["running"], snap["pools"][mini]["queued"]), (2, 1)) self.assertEqual(set(snap["pools"]), {mini}) + def test_owned_pool_commitments_count_jobs_not_created_yet(self): + mini = "glaeda-std-xcode-26.6" + runs = [{"id": 1, "status": "in_progress", "path": ".github/workflows/ci.yml"}, + {"id": 2, "status": "in_progress", "path": ".github/workflows/ci.yml"}, + {"id": 3, "status": "completed", "path": ".github/workflows/ci.yml"}] + jobs = {1: [self.job(mini, "in_progress")], + 2: [self.job(mini, "in_progress"), self.job(mini, "queued")], + 3: []} + # Run 1 declared 9 at its peak; run 2 has no marker; run 3 finished. + markers = {1: (mini, 9), 3: (mini, 11)} + snap = janitor.pool_load_snapshot(runs, jobs, now=NOW, markers=markers) + self.assertEqual(snap["pools"][mini]["committed"], 9 + 2) + self.assertEqual((snap["pools"][mini]["running"], snap["pools"][mini]["queued"]), (2, 1)) + # A marked run with no job created yet still reserves its peak. + snap = janitor.pool_load_snapshot([runs[0]], {1: []}, now=NOW, markers={1: (mini, 4)}) + self.assertEqual(snap["pools"][mini], {"queued": 0, "running": 0, "reserved_queued": 0, + "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) + + def test_owned_marker_names_this_attempts_pool_and_peak(self): + run = {"id": 42, "run_attempt": 1} + name = "macos-pool-persistent-42-1-5-glaeda-std-xcode-26.6" + self.assertEqual(janitor.owned_marker(run, ["other", name]), ("glaeda-std-xcode-26.6", 5)) + self.assertIsNone(janitor.owned_marker({"id": 42, "run_attempt": 2}, [name])) + self.assertIsNone(janitor.owned_marker({"id": 4, "run_attempt": 1}, [name])) + self.assertIsNone(janitor.owned_marker(run, ["macos-pool-persistent-42-1-5-blacksmith-6vcpu-macos-26"])) + self.assertEqual(janitor.owned_marker(run, ["macos-pool-persistent-42-1-99-glaeda-std-xcode-26.6"]), + ("glaeda-std-xcode-26.6", pool.MAX_RUN_JOBS)) + + def test_only_runs_that_may_hold_an_owned_pool_cost_a_listing(self): + repo = {"id": 7} + run = {"event": "pull_request", "run_attempt": 1, "path": ".github/workflows/ci.yml", + "repository": repo, "head_repository": repo} + self.assertTrue(janitor.may_hold_owned_pool(run, [])) + self.assertTrue(janitor.may_hold_owned_pool(run, [self.job("glaeda-std-xcode-26.6", "queued")])) + self.assertFalse(janitor.may_hold_owned_pool(run, [self.job(SMALL, "queued")])) + for change in ({"event": "push"}, {"run_attempt": 2}, {"head_repository": {"id": 8}}, + {"path": ".github/workflows/nightly.yml"}): + self.assertFalse(janitor.may_hold_owned_pool({**run, **change}, []), change) + def test_workflow_publishes_the_snapshot(self): workflow = yaml.safe_load((WORKFLOWS / "ci-queue-janitor.yml").read_text()) steps = workflow["jobs"]["sweep"]["steps"] @@ -330,6 +372,8 @@ def test_workflow_publishes_the_snapshot(self): self.assertEqual(sweep["env"]["PR_POOL_OVERFLOW"], "${{ vars.CI_PR_POOL_OVERFLOW }}") self.assertEqual(sweep["env"]["PR_POOL_ORDER"], "${{ vars.CI_PR_POOL_ORDER }}") self.assertEqual(sweep["env"]["PR_POOL_MAX_QUEUED"], "${{ vars.CI_PR_POOL_MAX_QUEUED }}") + self.assertEqual(sweep["env"]["PR_POOL_OWNED"], "${{ vars.CI_PR_POOL_OWNED }}") + self.assertNotIn("PR_POOL_OWNED", janitor.POOL_SETTINGS_ENV) upload = next(step for step in steps if "upload-artifact" in str(step.get("uses"))) self.assertEqual(upload["with"]["name"], pool.ARTIFACT_NAME) self.assertTrue(upload["with"]["path"].endswith(pool.SNAPSHOT_FILE)) @@ -340,6 +384,8 @@ def test_workflow_publishes_the_snapshot(self): 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'") PR_XCODE = "/Applications/Xcode_26.6.app" MINI = "glaeda-std-xcode-26.6" LIGHT = "glaeda-light-xcode-26.6" @@ -354,6 +400,8 @@ def fleet(busy=0, queued=0, age=2, **kwargs) -> dict: def owned_choice(snap, *, owned="1", machines=11, **kwargs): kwargs.setdefault("owned_slots", json.dumps({MINI: machines})) + # A compile-only run: admission beside the CLI pipe and remote daemon lanes. + kwargs.setdefault("jobs", 3) return choose(snap, pins=OWNED_PINS, owned=owned, **kwargs) @@ -382,7 +430,10 @@ def test_order_naming_an_owned_pool_is_ignored_while_off(self): def test_first_when_on_and_a_whole_run_fits(self): choice = owned_choice(fleet(busy=8)) self.assertEqual((choice.runner, choice.xcode_app), (MINI, "")) - self.assertIn("3 of 11 owned machines free", choice.reason) + self.assertIn("3 of 11 owned machines free, this run needs 3", choice.reason) + # The re-run of failed jobs is named now, on the lane's own Xcode. + self.assertEqual(choice.retry_runner, LARGE) + self.assertEqual(owned_choice(fleet(), owned="").retry_runner, "") def test_light_is_the_second_owned_pool(self): snap = fleet(busy=9) @@ -396,21 +447,42 @@ def test_light_is_the_second_owned_pool(self): def test_a_run_needs_a_machine_for_each_of_its_jobs(self): # 11 machines, 9 busy: one run's 3 jobs would not all start. self.assertEqual(owned_choice(fleet(busy=9)).runner, LARGE) - self.assertEqual(owned_choice(fleet(busy=9), jobs_per_run="2").runner, MINI) + self.assertEqual(owned_choice(fleet(busy=9), jobs=2).runner, MINI) + # A full suite needs all 11 at once. + self.assertEqual(owned_choice(fleet(), jobs=11).runner, MINI) + self.assertEqual(owned_choice(fleet(busy=1), jobs=11).runner, LARGE) + + def test_a_runs_peak_comes_from_its_routing(self): + def jobs(**flags): + base = dict(macos="true", full_suite="false", unit_suite="false", unit_in_admission="false", + claude_wrapper="false", cli="false", remote_daemon="false") + return pool.run_jobs(**{**base, **flags}) + self.assertEqual(jobs(), 1) + self.assertEqual(jobs(cli="true", remote_daemon="true"), 3) + self.assertEqual(jobs(unit_suite="true"), 1) + self.assertEqual(jobs(unit_suite="true", unit_in_admission="true", cli="true"), 2) + # Full suite: seven shards and tests-build-and-lag after admission, and the Claude wrapper. + self.assertEqual(jobs(full_suite="true", cli="true", remote_daemon="true"), pool.MAX_RUN_JOBS) + self.assertEqual(jobs(macos="false", claude_wrapper="true", cli="true"), 2) + self.assertEqual(jobs(macos="false"), 0) + + def test_committed_peaks_count_jobs_not_created_yet(self): + # Two machines busy, but the runs holding them declared 9 at their peak. + snap = fleet(busy=2) + snap["pools"][MINI]["committed"] = 9 + self.assertEqual(owned_choice(snap).runner, LARGE) + self.assertEqual(owned_choice(snap, jobs=2).runner, MINI) def test_queued_jobs_take_machines_without_closing_the_pool(self): self.assertEqual(owned_choice(fleet(busy=5, queued=3)).runner, MINI) self.assertEqual(owned_choice(fleet(busy=5, queued=4)).runner, LARGE) - def test_jobs_per_run_must_be_1_to_10(self): - for value in ("0", "11", "x"): - self.assertEqual(owned_choice(fleet(), jobs_per_run=value).runner, "", value) - - def test_replayed_runs_fill_idle_runners_first(self): - # Two idle runners; two runs created since the snapshot took them. - # 11 machines, 2 busy: two newer runs took 6, leaving 3 for this one; a third takes those. - self.assertEqual(owned_choice(fleet(busy=2), routed=2).runner, MINI) - self.assertEqual(owned_choice(fleet(busy=2), routed=3).runner, LARGE) + def test_replayed_runs_are_charged_the_largest_peak(self): + # A run created since the snapshot has an unknown peak: any that could + # have taken the pool is assumed to, and charged MAX_RUN_JOBS there. + self.assertEqual(owned_choice(fleet(), machines=22, routed=1).runner, MINI) + self.assertEqual(owned_choice(fleet(), routed=1).runner, LARGE) + self.assertEqual(owned_choice(fleet(busy=10), routed=1).runner, LARGE) def test_stale_snapshot_or_no_slots_skips_the_pool(self): self.assertEqual(owned_choice(fleet(age=pool.OWNED_MAX_AGE_MINUTES + 1)).runner, LARGE) @@ -420,6 +492,28 @@ def test_stale_snapshot_or_no_slots_skips_the_pool(self): def test_a_pool_the_janitor_saw_no_job_on_is_idle(self): self.assertEqual(owned_choice(backlog()).runner, MINI) + def test_bad_slot_entries_are_named(self): + problems = pool.slot_problems('{"%s": 11, "glaeda-std-xcode-26.6x": 2, "%s": "3", "%s": 11.0}' + % (MINI, LIGHT, "glaeda-xl-xcode-26.6")) + self.assertEqual(len(problems), 3, problems) + self.assertTrue(any("glaeda-std-xcode-26.6x" in p and "not an owned pool label" in p for p in problems)) + self.assertTrue(any(LIGHT in p and "'3'" in p for p in problems)) + self.assertEqual(pool.slot_problems(""), []) + self.assertEqual(pool.slot_problems('{"%s": 11}' % MINI), []) + self.assertIn("not JSON", pool.slot_problems("nope")[0]) + self.assertIn("not a JSON object", pool.slot_problems("[1]")[0]) + + def test_main_warns_about_bad_slots_only_while_owned_pools_are_on(self): + for owned, warned in (("1", True), ("", False)): + with tempfile.TemporaryDirectory() as tmp: + stdout = io.StringIO() + env = {"EVENT_NAME": "push", "POOL_OWNED": owned, "OWNED_SLOTS": '{"glaeda-std": 3}', + "GITHUB_STEP_SUMMARY": str(Path(tmp, "summary"))} + with unittest.mock.patch("sys.stdout", stdout): + pool.main([], env) + self.assertEqual("::warning title=CI_OWNED_POOL_SLOTS::" in stdout.getvalue(), warned, owned) + self.assertEqual("**Warning:**" in Path(tmp, "summary").read_text(), warned, owned) + def test_slots_ignore_anything_malformed(self): self.assertEqual(pool.slots('{"%s": 11, "blacksmith-6vcpu-macos-26": 5, "glaeda-std-xcode-26.3": 0,' ' "glaeda-light-xcode-26.6": true}' % MINI), {MINI: 11}) @@ -464,16 +558,18 @@ def output(self, attempt, owned="1"): "HEAD_REPO": "manaflow-ai/cmux", "DEFAULT_RUNNER": SMALL, "POOL_OWNED": owned, "OWNED_SLOTS": json.dumps({MINI: 3}), "CMUX_CI_XCODE_APP_PR": PR_XCODE, "CMUX_CI_XCODE_APP_MACOS_15": XCODE_15, - "GITHUB_RUN_ATTEMPT": str(attempt), "GITHUB_OUTPUT": str(out)} + "GITHUB_RUN_ATTEMPT": str(attempt), "GITHUB_OUTPUT": str(out), + "RUN_MACOS": "true"} 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_reports_a_persistent_choice(self): first = self.output(1) - self.assertEqual((first["runner"], first["persistent"]), (MINI, "true")) + self.assertEqual((first["runner"], first["persistent"], first["retry_runner"]), (MINI, "true", LARGE)) + self.assertEqual(first["jobs"], "1") retried = self.output(2) - self.assertEqual((retried["runner"], retried["persistent"]), (LARGE, "false")) + self.assertEqual((retried["runner"], retried["persistent"], retried["retry_runner"]), (LARGE, "false", "")) self.assertEqual(self.output(1, owned="")["persistent"], "false") @@ -498,7 +594,24 @@ def test_a_persistent_choice_publishes_the_rescue_marker(self): mark = next(step for step in steps if step.get("id") == "macos-pool-marker") self.assertEqual(mark["if"], "${{ steps.macos-pool.outputs.persistent == 'true' }}") upload = next(step for step in steps if step.get("name") == "Upload the persistent pool marker") - self.assertEqual(upload["with"]["name"], "macos-pool-persistent-${{ github.run_id }}-${{ github.run_attempt }}") + self.assertEqual(upload["with"]["name"], "macos-pool-persistent-${{ github.run_id }}-${{ github.run_attempt }}" + "-${{ steps.macos-pool.outputs.jobs }}-${{ steps.macos-pool.outputs.runner }}") + + def test_the_picker_reads_the_runs_routing(self): + changes = self.workflow("ci.yml")["jobs"]["changes"] + ids = [step.get("id") for step in changes["steps"]] + for step_id in ("detect", "standalone", "suite"): + self.assertLess(ids.index(step_id), ids.index("macos-pool"), step_id) + env = changes["steps"][ids.index("macos-pool")]["env"] + self.assertEqual(env["RUN_FULL_SUITE"], "${{ steps.suite.outputs.full_suite }}") + self.assertEqual(env["RUN_CLI"], "${{ steps.detect.outputs.cli }}") + self.assertNotIn("OWNED_JOBS_PER_RUN", env) + self.assertEqual(changes["outputs"]["macos_pr_retry_runner"], "${{ steps.macos-pool.outputs.retry_runner }}") + + def test_an_owned_pool_run_never_requests_the_persistent_compile_route(self): + steps = self.workflow("ci.yml")["jobs"]["changes"]["steps"] + request = next(step for step in steps if step.get("id") == "persistent-route-request") + self.assertIn("steps.macos-pool.outputs.persistent != 'true'", request["if"]) def lanes(self, name): text = (WORKFLOWS / name).read_text() @@ -507,15 +620,22 @@ 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": "inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15'", - "cli-pipe-regressions.yml": "inputs.pr_runner || vars.MACOS_RUNNER_PR || 'blacksmith-6vcpu-macos-15'", - "remote-daemon.yml": "inputs.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, } for name, lane in expected.items(): lanes = self.lanes(name) self.assertTrue(lanes, name) self.assertEqual(set(lanes), {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 " + "|| 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) + def test_callers_pass_the_choice(self): jobs = self.workflow("ci.yml")["jobs"] runner = "${{ needs.changes.outputs.macos_pr_runner }}" @@ -525,6 +645,9 @@ def test_callers_pass_the_choice(self): self.assertEqual(jobs["cli"]["with"]["pr_runner"], runner) self.assertEqual(jobs["cli"]["with"]["pr_xcode_app"], xcode) self.assertEqual(jobs["remote-daemon"]["with"]["pr_runner"], runner) + retry = "${{ needs.changes.outputs.macos_pr_retry_runner }}" + for name in ("macos", "cli", "remote-daemon"): + self.assertEqual(jobs[name]["with"]["pr_retry_runner"], retry, name) for name in ("macos", "cli", "remote-daemon", "claude-wrapper"): needs = jobs[name]["needs"] self.assertIn("changes", [needs] if isinstance(needs, str) else needs, name) @@ -559,9 +682,9 @@ def test_build_input_fingerprint_keys_on_the_chosen_xcode(self): "|| vars.CMUX_CI_XCODE_APP_MACOS_15 }}", step_id) def test_reusable_inputs_default_to_todays_route(self): - for name, keys in (("ci-macos.yml", ("pr_runner", "pr_xcode_app")), - ("cli-pipe-regressions.yml", ("pr_runner", "pr_xcode_app")), - ("remote-daemon.yml", ("pr_runner",))): + for name, keys in (("ci-macos.yml", ("pr_runner", "pr_retry_runner", "pr_xcode_app")), + ("cli-pipe-regressions.yml", ("pr_runner", "pr_retry_runner", "pr_xcode_app")), + ("remote-daemon.yml", ("pr_runner", "pr_retry_runner"))): # PyYAML reads the `on:` key as True. inputs = self.workflow(name)[True]["workflow_call"]["inputs"] for key in keys: diff --git a/tests/test_ci_self_hosted_guard.sh b/tests/test_ci_self_hosted_guard.sh index ec658d374d8d..8f0eb854f8de 100755 --- a/tests/test_ci_self_hosted_guard.sh +++ b/tests/test_ci_self_hosted_guard.sh @@ -44,8 +44,9 @@ 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. - in_job && /runs-on:[[:space:]]*\$\{\{ needs\.macos-compile-admission\.outputs\.runner \}\}/ { saw=1 } + # check covers on its own, or on a re-run 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 && /os:.*(vars\.MACOS_RUNNER|blacksmith-[0-9]+vcpu-macos-|warp-macos-[0-9]+-arm64|depot-macos-)/ { saw=1 } END { exit !(saw) } ' "$file"; then @@ -1340,8 +1341,14 @@ from pathlib import Path import yaml PICKED = "steps.macos-pool.outputs.runner" +RETRY_PICKED = "steps.macos-pool.outputs.retry_runner" OUTPUT = "needs.changes.outputs.macos_pr_runner" +RETRY_OUTPUT = "needs.changes.outputs.macos_pr_retry_runner" PASSED = "${{ needs.changes.outputs.macos_pr_runner }}" +# Each input the picked pools reach a reusable workflow through, and its value. +INPUTS = {"pr_runner": PASSED, "pr_retry_runner": "${{ " + RETRY_OUTPUT + " }}"} +MARKER = ("macos-pool-persistent-${{ github.run_id }}-${{ github.run_attempt }}" + "-${{ steps.macos-pool.outputs.jobs }}-${{ steps.macos-pool.outputs.runner }}") # The runs-on branches that may read the picked pool, each behind its # pull_request condition; a fork head keeps only a Blacksmith pick. GUARDED = ( @@ -1350,6 +1357,9 @@ 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", ) @@ -1373,20 +1383,25 @@ for file in sorted(Path(sys.argv[1]).glob("*.y*ml")): allowed = (file.name == "ci.yml" and ( path == ("jobs", "changes", "outputs", "macos_pr_runner") and value == "${{ " + PICKED + " }}" or path[:3] == ("jobs", "changes", "steps") and path[-2:] == ("env", "POOL") - and value == "${{ " + PICKED + " }}")) + and value == "${{ " + PICKED + " }}" + or path[:3] == ("jobs", "changes", "steps") and path[-2:] == ("with", "name") and value == MARKER)) if not allowed: violations.append(f"{where}: reads the picker's runner outside macos_pr_runner and the rescue marker") - if path[-1:] == ("pr_runner",) and len(path) >= 3 and path[-2] == "with": - if value != PASSED or file.name != "ci.yml": - violations.append(f"{where}: pr_runner must be exactly {PASSED}") + if RETRY_PICKED in value and not ( + file.name == "ci.yml" and path == ("jobs", "changes", "outputs", "macos_pr_retry_runner") + and value == "${{ " + RETRY_PICKED + " }}"): + violations.append(f"{where}: reads the picker's retry runner outside macos_pr_retry_runner") + if len(path) >= 3 and path[-2] == "with" and path[-1] in INPUTS: + if value != INPUTS[path[-1]] or file.name != "ci.yml": + violations.append(f"{where}: {path[-1]} must be exactly {INPUTS[path[-1]]}") continue - if OUTPUT not in value: + if OUTPUT not in value and RETRY_OUTPUT not in value: continue if path[-1:] == ("runs-on",): rest = value for branch in GUARDED: rest = rest.replace(branch, "") - if OUTPUT not in rest: + if OUTPUT not in rest and RETRY_OUTPUT not in rest: continue violations.append(f"{where}: reads macos_pr_runner outside pr_runner or a pull_request runs-on branch") print("\n".join(violations)) From 8b78568e0d5474303f9be39cb3c539460c623ef9 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 12:20:24 -0400 Subject: [PATCH 2/5] ci: read every candidate run's owned-pool marker and page artifact lists Review of #14252: the janitor skipped the marker of any run with a macOS job on another pool, and swift-package-tests always runs on Blacksmith beside a full suite, so full-suite runs on an owned pool were counted at their current jobs, not their peak. Every attempt-1 same-repository CI run is now a candidate. The janitor and the rescue page through a run's artifacts instead of reading only the first 100, and the app-host test rerun maps an owned-pool admission to the macOS 26 pool, whose Xcode is the lane pin the owned label carries. Co-Authored-By: Claude Opus 5.5 --- docs/ci-runners.md | 6 ++++-- scripts/ci/app_host_test_rerun.py | 4 ++++ scripts/ci/owned_pool_rescue.py | 12 +++++++++--- scripts/ci/queue_janitor.py | 28 ++++++++++++++++++---------- tests/test_app_host_test_rerun.py | 5 +++++ tests/test_ci_pr_runner_pool.py | 3 ++- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index d663bc02afed..c7e4d297cb43 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -236,8 +236,10 @@ every sweep whatever the queue, which frees minis for current work. The other categories cancel only while more than `CI_JANITOR_QUEUE_THRESHOLD` jobs queue on a pool the run holds, owned pools included. With `CI_PR_POOL_OWNED=1` the janitor also lists the artifacts of each in-flight attempt-1, same-repository -pull request CI run that has no macOS job on another pool, one request per -run, to read its marker's peak into `committed`. +pull request CI run (one request per run, more only past 100 artifacts) to +read its marker's peak into `committed`. A run's other macOS jobs do not rule +it out: `swift-package-tests` always runs on Blacksmith beside a full suite on +an owned pool. | Variable | Default | Effect | | --- | --- | --- | diff --git a/scripts/ci/app_host_test_rerun.py b/scripts/ci/app_host_test_rerun.py index 34597430ec18..0ebdc80c1d96 100644 --- a/scripts/ci/app_host_test_rerun.py +++ b/scripts/ci/app_host_test_rerun.py @@ -183,6 +183,10 @@ def product_runner(repository: str, run_id: str, api: Callable[[str], dict], pag # test-e2e.yml compiles in its `build` job. if job.get("name", "").endswith(ADMISSION_JOB) or job.get("name") == "build": for label in job.get("labels", []): + # An owned Mac pool carries the pull-request lane's Xcode, + # which is the macOS 26 pools' pin (pr_runner_pool.py). + if re.fullmatch(r"glaeda-(?:xl|std|light)-xcode-[0-9.]+", label): + return PRODUCT_RUNNERS["26"] match = re.search(r"macos-(\d+)", label) if match and match.group(1) in PRODUCT_RUNNERS: return PRODUCT_RUNNERS[match.group(1)] diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 8ab96ab26f94..675746192f6d 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -182,10 +182,16 @@ def jobs(self, run_id: int, attempt: int) -> list[Mapping[str, Any]]: break return found - def has_artifact(self, run_id: int, prefix: str) -> bool: + def has_artifact(self, run_id: int, prefix: str, pages: int = 5) -> bool: """Whether the run uploaded an artifact whose name starts with `prefix`.""" - data = self.request("GET", f"/actions/runs/{run_id}/artifacts?per_page=100") - return any(str(item.get("name") or "").startswith(prefix) for item in (data or {}).get("artifacts") or []) + for page in range(1, pages + 1): + data = self.request("GET", f"/actions/runs/{run_id}/artifacts?per_page=100&page={page}") + names = [str(item.get("name") or "") for item in (data or {}).get("artifacts") or []] + if any(name.startswith(prefix) for name in names): + return True + if len(names) < 100: + return False + return False def pull(self, number: int) -> Mapping[str, Any]: return self.request("GET", f"/pulls/{number}") diff --git a/scripts/ci/queue_janitor.py b/scripts/ci/queue_janitor.py index 3912be501f78..244604cee650 100644 --- a/scripts/ci/queue_janitor.py +++ b/scripts/ci/queue_janitor.py @@ -352,6 +352,7 @@ def macos_usage(jobs: Iterable[Mapping[str, Any]]) -> MacosUsage: } +MAX_ARTIFACT_PAGES = 5 # ci.yml's `changes` job uploads this marker when the picker chose an owned # pool: macos-pool-persistent----. OWNED_MARKER = re.compile(r"macos-pool-persistent-(?P[0-9]+)-(?P[0-9]+)-(?P[0-9]+)-(?P.+)") @@ -368,18 +369,17 @@ def owned_marker(run: Mapping[str, Any], names: Iterable[str]) -> tuple[str, int def may_hold_owned_pool(run: Mapping[str, Any], jobs: Sequence[Mapping[str, Any]]) -> bool: - """A run whose marker is worth one artifact listing: it may hold an owned pool. + """A run whose marker is worth an artifact listing: it may hold an owned pool. Only attempt 1 of a same-repository pull request run of CI can (a retry - never takes one), and not once any of its macOS jobs asked for another pool. + never takes one). Its other macOS jobs say nothing: swift-package-tests + always runs on a Blacksmith pool beside a run on an owned one. """ if run.get("event") != "pull_request" or (run.get("run_attempt") or 1) != 1: return False if (run.get("head_repository") or {}).get("id") != (run.get("repository") or {}).get("id"): return False - if not str(run.get("path") or "").endswith("/ci.yml"): - return False - return not any(is_macos_job(job) and not owned_label(job) for job in jobs) + return str(run.get("path") or "").endswith("/ci.yml") def pool_load_snapshot( @@ -1198,10 +1198,17 @@ def in_flight_runs(self) -> list[dict[str, Any]]: break return list(runs.values()) - def artifact_names(self, run_id: int) -> list[str]: - query = urllib.parse.urlencode({"per_page": 100}) - payload = self.request("GET", f"/repos/{self.repo}/actions/runs/{run_id}/artifacts?{query}") - return [str(item.get("name") or "") for item in payload.get("artifacts") or []] + def artifact_names(self, run_id: int, *, stop: str) -> list[str]: + """The run's artifact names, page by page until one starts with `stop`.""" + names: list[str] = [] + for page in range(1, MAX_ARTIFACT_PAGES + 1): + query = urllib.parse.urlencode({"per_page": 100, "page": page}) + payload = self.request("GET", f"/repos/{self.repo}/actions/runs/{run_id}/artifacts?{query}") + batch = [str(item.get("name") or "") for item in payload.get("artifacts") or []] + names.extend(batch) + if len(batch) < 100 or any(name.startswith(stop) for name in batch): + break + return names def jobs(self, run_id: int) -> list[dict[str, Any]]: jobs: list[dict[str, Any]] = [] @@ -1317,7 +1324,8 @@ def main(argv: Sequence[str] | None = None) -> int: for run in runs: if run.get("id") in jobs_by_run and may_hold_owned_pool(run, jobs_by_run[run["id"]]): try: - found = owned_marker(run, github.artifact_names(run["id"])) + found = owned_marker(run, github.artifact_names( + run["id"], stop=f"macos-pool-persistent-{run['id']}-{run.get('run_attempt') or 1}-")) except RuntimeError as error: print(f"queue-janitor: owned-pool marker for run {run['id']}: {error}", file=sys.stderr) continue diff --git a/tests/test_app_host_test_rerun.py b/tests/test_app_host_test_rerun.py index a3da65d218eb..5d51c9a88a43 100644 --- a/tests/test_app_host_test_rerun.py +++ b/tests/test_app_host_test_rerun.py @@ -297,6 +297,11 @@ def test_github_hosted_admission_maps_to_the_same_macos(self) -> None: api = self.api_for([{"name": "macos / macOS compile admission", "labels": ["macos-26"]}]) self.assertEqual(rerun.product_runner("o/r", "5", api), "blacksmith-6vcpu-macos-26") + def test_owned_pool_admission_maps_to_the_lane_xcode_pool(self) -> None: + # An owned Mac carries the pull-request lane's Xcode, the macOS 26 pools' pin. + api = self.api_for([{"name": "macos / macOS compile admission", "labels": ["glaeda-std-xcode-26.6"]}]) + self.assertEqual(rerun.product_runner("o/r", "5", api), "blacksmith-6vcpu-macos-26") + def test_macos_15_admission_and_unknown_producers_stay_on_macos_15(self) -> None: api = self.api_for([{"name": "macos / macOS compile admission", "labels": ["blacksmith-6vcpu-macos-15"]}]) self.assertEqual(rerun.product_runner("o/r", "5", api), "blacksmith-6vcpu-macos-15") diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 013afa3e5648..96242bfacc1e 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -356,7 +356,8 @@ def test_only_runs_that_may_hold_an_owned_pool_cost_a_listing(self): "repository": repo, "head_repository": repo} self.assertTrue(janitor.may_hold_owned_pool(run, [])) self.assertTrue(janitor.may_hold_owned_pool(run, [self.job("glaeda-std-xcode-26.6", "queued")])) - self.assertFalse(janitor.may_hold_owned_pool(run, [self.job(SMALL, "queued")])) + # swift-package-tests sits on Blacksmith beside a full-suite run on an owned pool. + self.assertTrue(janitor.may_hold_owned_pool(run, [self.job(OLD, "queued")])) for change in ({"event": "push"}, {"run_attempt": 2}, {"head_repository": {"id": 8}}, {"path": ".github/workflows/nightly.yml"}): self.assertFalse(janitor.may_hold_owned_pool({**run, **change}, []), change) From a479a3b4fa3649951f6c1d37e3cec2f1356a7ec8 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 12:38:16 -0400 Subject: [PATCH 3/5] ci: re-run refused owned-pool jobs, count cli-product-tests, charge replays less Review of #14252 at 439d9f5: 1. cli-product-tests (new on main since #14211) inherits compile admission's pool like the app-host shards, so it now reads pr_retry_runner first too. 2. run_jobs counted neither cli-product-tests nor the compile admission a CLI-only run pays for. A full suite peaks at 12 machines, a CLI-only run at 2. 3. An owned runner that refuses a job (glaeda's job-started hook exits 1 on a held host lock) fails it in seconds, and GitHub never retries. The rescue now treats a job on the persistent pool that failed within 120 s with no workflow step succeeded as refused: it checks the head, cancels the run if still going, and re-runs the failed jobs, which take retry_runner on Blacksmith and keep what passed. 4. A run replayed since the snapshot is charged 4 machines (a compile-only run with every side lane) instead of 11, now that a miscount is refused or queued and moved by the rescue instead of stranding a job. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-macos.yml | 4 +- .github/workflows/ci-owned-pool-rescue.yml | 5 +- docs/ci-runners.md | 20 +++++-- scripts/ci/owned_pool_rescue.py | 68 +++++++++++++++++++--- scripts/ci/pr_runner_pool.py | 30 ++++++---- tests/test_ci_owned_pool_rescue.py | 61 +++++++++++++++++++ tests/test_ci_pr_runner_pool.py | 20 +++++-- 7 files changed, 175 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 107bd1ccb48b..a8b9faba7d3a 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -2035,7 +2035,7 @@ jobs: # 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: ${{ needs.macos-compile-admission.outputs.runner }} + runs-on: ${{ github.run_attempt > 1 && 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 }} @@ -2051,7 +2051,7 @@ jobs: - name: Verify GitHub-hosted route env: RUNNER_ENVIRONMENT: ${{ runner.environment }} - REQUESTED_RUNNER: ${{ needs.macos-compile-admission.outputs.runner }} + REQUESTED_RUNNER: ${{ github.run_attempt > 1 && inputs.pr_retry_runner || needs.macos-compile-admission.outputs.runner }} RUNNER_CONTEXT_NAME: ${{ runner.name }} run: | set -euo pipefail diff --git a/.github/workflows/ci-owned-pool-rescue.yml b/.github/workflows/ci-owned-pool-rescue.yml index f336ba4c5d50..4b6f6167f92e 100644 --- a/.github/workflows/ci-owned-pool-rescue.yml +++ b/.github/workflows/ci-owned-pool-rescue.yml @@ -6,7 +6,10 @@ run-name: owned-pool-rescue-${{ github.event.workflow_run.id }}-${{ github.event # busy the run would wait for it indefinitely. This workflow watches such a run # and, when one of its jobs has waited past CI_OWNED_POOL_RESCUE_SECONDS, # cancels the run and re-runs it; the retry attempt never takes a persistent -# pool, so it lands on Blacksmith. scripts/ci/owned_pool_rescue.py has the rule. +# pool, so it lands on Blacksmith. A job an owned runner refused at job start +# (it failed in seconds, before any step succeeded) gets its failed jobs re-run +# instead, which moves them to the picker's retry_runner on Blacksmith. +# scripts/ci/owned_pool_rescue.py has the rules. # # It needs actions: write, so it is triggered by workflow_run and its code comes # from main: a pull request cannot change it. Only attempt 1 is watched, in the diff --git a/docs/ci-runners.md b/docs/ci-runners.md index c7e4d297cb43..f63a8c46e4c3 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -179,13 +179,17 @@ at once, each on its own machine, so a run takes the owned pool only when its own peak is free at once. The picker runs after the suite choice and counts that peak from the run's routing: the Claude wrapper, CLI pipe and remote daemon lanes, beside the larger of compile admission alone or what follows it -(a full suite's seven app-host shards and tests-build-and-lag, 11 jobs in all; -a changed-suites run's one shard). Taken is the larger of the jobs the janitor +(a full suite's seven app-host shards, tests-build-and-lag and +cli-product-tests, 12 jobs in all; a changed-suites run's one shard; a CLI +change's cli-product-tests). Taken is the larger of the jobs the janitor saw on the pool and `committed`, the peaks the runs holding it declared, so a run whose later jobs do not exist yet still counts them. A run created since the snapshot has an unknown peak: any that could have taken the pool is -assumed to, and charged 11 machines, so a wrong guess leaves minis idle -rather than queueing a job there. It is skipped when the snapshot is older than 20 +assumed to, and charged 4 machines, a compile-only run with every side lane, +which is what the default pull request policy runs. A full-suite run among +them is under-counted until the next snapshot; a job that then finds its +mini busy is refused or queued, and the rescue below moves it to Blacksmith. +It is skipped when the snapshot is older than 20 minutes or the label has no slots, and it is never the fewest-queued fallback. Fork runs and retry attempts never take it. While `CI_PR_POOL_OWNED` is off, owned labels in `CI_PR_POOL_ORDER` are dropped and the rest of the order is @@ -217,6 +221,14 @@ pull request head has not moved, cancels the run, and re-runs it. A retry attempt never takes a persistent pool, so the re-run lands on Blacksmith as a whole, and so does a manual "Re-run all jobs". +An owned runner can also refuse a job: glaeda's job-started hook exits 1 when +the host is busy, and the job fails within seconds. GitHub does not retry it. +The watcher treats a job on the persistent pool that failed within 120 +seconds of starting, with no workflow step succeeded, as refused. It confirms +the head has not moved, cancels the run if it is still going, and re-runs its +failed jobs, so nobody has to. That attempt 2 keeps what passed and sends the +rest to `retry_runner` (below). + "Re-run failed jobs" is different: `changes` passed, so it is not re-run, and the failed jobs read attempt 1's outputs, owned pool included, with no watcher (the rescue follows attempt 1 only). So a persistent choice also names diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 675746192f6d..8eec24f9e889 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -21,6 +21,17 @@ 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. +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 +within seconds, before any step of the workflow succeeds. GitHub does not +retry it, so the pull request would stay red until someone re-ran it. A job +on the persistent pool that failed within REFUSAL_SECONDS of starting with no +workflow step succeeded counts as refused: the watcher confirms the head has +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. + A job's wait is measured from the later of its `created_at` and the first time the watcher saw it queued, so a job record created before its `needs` were met can never count as already past the budget. @@ -75,6 +86,11 @@ MARKER_PREFIX = "macos-pool-persistent" CANCEL_WAIT_SECONDS = 180 FORCE_CANCEL_AFTER_SECONDS = 90 +# A refused job fails in seconds; a real failure of the first step after +# checkout takes longer than this, and one that does not is cheap to retry. +REFUSAL_SECONDS = 120 +# The runner's own steps, which run before glaeda's hook decides. +SETUP_STEPS = frozenset({"Set up job", "Set up runner"}) MAX_JOB_PAGES = 3 API = "https://api.github.com" @@ -118,6 +134,17 @@ def queued_seconds(job: Mapping[str, Any], now: dt.datetime, first_seen: dt.date return 0.0 if since is None else max(0.0, (now - since).total_seconds()) +def refused(job: Mapping[str, Any]) -> bool: + """A job the owned runner refused at job start (see the module docstring).""" + if not job_pool(job) or job.get("status") != "completed" or job.get("conclusion") != "failure": + return False + started, completed = parse_time(job.get("started_at")), parse_time(job.get("completed_at")) + if started is None or completed is None or (completed - started).total_seconds() > REFUSAL_SECONDS: + return False + return not any(step.get("conclusion") == "success" and step.get("name") not in SETUP_STEPS + for step in job.get("steps") or [] if isinstance(step, Mapping)) + + def picker_finished(jobs: Sequence[Mapping[str, Any]]) -> bool: picker = [job for job in jobs if job.get("name") == PICKER_JOB] return bool(picker) and all(job.get("status") == "completed" for job in picker) @@ -129,7 +156,7 @@ def run_finished(jobs: Sequence[Mapping[str, Any]]) -> bool: @dataclasses.dataclass(frozen=True) class Look: - action: str # "rescue" or "watch" + action: str # "rescue" (cancel, re-run all), "refused" (re-run failed jobs) or "watch" reason: str waiting: bool = False # a persistent-pool job has no runner yet @@ -144,6 +171,10 @@ def assess(jobs: Sequence[Mapping[str, Any]], *, now: dt.datetime, budget_second names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in stuck)) return Look("rescue", f"{names} queued on {job_pool(stuck[0])} for at least " f"{budget_seconds}s with no runner") + turned_away = [job for job in jobs if refused(job)] + if turned_away: + names = ", ".join(sorted(str(job.get("name") or job.get("id")) for job in turned_away)) + return Look("refused", f"{names} refused by {job_pool(turned_away[0])} at job start") if waiting: return Look("watch", f"{len(waiting)} job(s) waiting for a persistent runner", waiting=True) return Look("watch", "no job is waiting for a persistent runner") @@ -205,6 +236,9 @@ def force_cancel(self, run_id: int) -> None: def rerun(self, run_id: int) -> None: self.request("POST", f"/actions/runs/{run_id}/rerun") + def rerun_failed(self, run_id: int) -> None: + self.request("POST", f"/actions/runs/{run_id}/rerun-failed-jobs") + @dataclasses.dataclass class Target: @@ -276,6 +310,10 @@ def watch(api: GitHub, target: Target, *, budget_seconds: int, return "stop", "the run finished before the pool choice" interval = POLL_SECONDS if on_persistent: + if any(refused(job) for job in jobs): + look = assess(jobs, now=now(), budget_seconds=budget_seconds, first_seen=first_seen) + log(f"look {looks}: {look.reason}") + return look.action, look.reason if run_finished(jobs) and read(lambda: api.run(target.run_id), sleep, log).get("status") == "completed": return "stop", "the run finished" seen_at = now() @@ -284,8 +322,8 @@ def watch(api: GitHub, target: Target, *, budget_seconds: int, first_seen.setdefault(job.get("id"), seen_at) look = assess(jobs, now=seen_at, budget_seconds=budget_seconds, first_seen=first_seen) log(f"look {looks}: {look.reason}") - if look.action == "rescue": - return "rescue", look.reason + if look.action in ("rescue", "refused"): + return look.action, look.reason if not look.waiting: interval = IDLE_POLL_SECONDS if (now() - started).total_seconds() >= WATCH_LIMIT_SECONDS: @@ -305,14 +343,23 @@ def pull_moved(api: GitHub, target: Target, sleep: Callable[[float], None], def rescue(api: GitHub, target: Target, *, now: Callable[[], dt.datetime], sleep: Callable[[float], None], - log: Callable[[str], None]) -> str: - """Cancel and re-run, unless the pull request has moved on. Returns what happened.""" + log: Callable[[str], None], failed_only: bool = False) -> str: + """Cancel and re-run, unless the pull request has moved on. Returns what happened. + + `failed_only` (a refused job) re-runs only the failed and cancelled jobs, + keeping what passed, and needs no cancel when the run already finished. + """ moved = pull_moved(api, target, sleep, log) if moved: return f"not rescued: {moved}" run = read(lambda: api.run(target.run_id), sleep, log) + if int(run.get("run_attempt") or 0) != target.attempt: + return "not rescued: someone else already re-ran the run" if run.get("status") == "completed": - return "not rescued: the run already finished" + if not failed_only: + return "not rescued: the run already finished" + api.rerun_failed(target.run_id) + return f"re-ran the failed jobs of run {target.run_id}; attempt {target.attempt + 1} takes retry_runner" api.cancel(target.run_id) log(f"cancelled run {target.run_id}") started = now() @@ -336,6 +383,9 @@ def rescue(api: GitHub, target: Target, *, now: Callable[[], dt.datetime], sleep moved = pull_moved(api, target, sleep, log) if moved: return f"cancelled but not re-run: {moved}" + if failed_only: + api.rerun_failed(target.run_id) + return f"re-ran the failed jobs of run {target.run_id}; attempt {target.attempt + 1} takes retry_runner" api.rerun(target.run_id) return f"re-ran run {target.run_id}; attempt {target.attempt + 1} takes an ephemeral pool" @@ -376,10 +426,10 @@ def finish(outcome: str) -> int: log(f"watching run {target.run_id} of pull request #{target.pr_number} (budget {seconds}s)") try: outcome, reason = watch(client, target, budget_seconds=seconds, now=clock, sleep=sleep, log=log) - if outcome != "rescue": + if outcome not in ("rescue", "refused"): return finish(f"stopped: {reason}") - log(f"rescue: {reason}") - return finish(rescue(client, target, now=clock, sleep=sleep, log=log)) + log(f"{'rescue' if outcome == 'rescue' else 'refused'}: {reason}") + return finish(rescue(client, target, now=clock, sleep=sleep, log=log, failed_only=outcome == "refused")) except (*READ_ERRORS, Aborted) as error: # A failed watch leaves the run exactly as GitHub scheduled it. finish(f"gave up: {error}") diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index d0c3e855d498..25a4cfb0c403 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -125,13 +125,19 @@ # 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 # daemon lanes; once admission passes, a full suite adds APP_HOST_SHARDS -# shards and tests-build-and-lag, and a changed-suites run one shard. A run -# takes an owned pool only when its own peak (run_jobs) is free, so none of -# its jobs queues there and trips the rescue. A run whose peak is unknown, and -# every run replayed since the snapshot, is charged MAX_RUN_JOBS. +# shards, tests-build-and-lag and cli-product-tests, a changed-suites run one +# shard, and a CLI change cli-product-tests. A run takes an owned pool only +# when its own peak (run_jobs) is free, so none of its jobs queues there. A +# run whose peak is unknown is charged MAX_RUN_JOBS. A run replayed since the +# snapshot is charged REPLAYED_RUN_JOBS, the peak of a compile-only run with +# every side lane, which is what the default pull request policy runs: a +# full-suite run among them is under-counted until the next snapshot, and a +# job that then finds its mini busy is refused or queued, and moved to +# Blacksmith by ci-owned-pool-rescue.yml. APP_HOST_SHARDS = 7 SIDE_LANES = 3 -MAX_RUN_JOBS = SIDE_LANES + APP_HOST_SHARDS + 1 +MAX_RUN_JOBS = SIDE_LANES + APP_HOST_SHARDS + 2 +REPLAYED_RUN_JOBS = SIDE_LANES + 1 # A snapshot older than this is not trusted to place a run on an owned pool. OWNED_MAX_AGE_MINUTES = 20 # Pools whose machines are discarded after each job; the only ones a fork run may use. @@ -196,14 +202,16 @@ def run_jobs(*, macos: str | None, full_suite: str | None, unit_suite: str | Non 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. + 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. """ side = sum(flag(lane) for lane in (cli, remote_daemon)) full = flag(macos) and flag(full_suite) side += flag(claude_wrapper) or full - if not flag(macos): + if not (flag(macos) or flag(cli)): return side - after = APP_HOST_SHARDS + 1 if full else int(flag(unit_suite) and not flag(unit_in_admission)) + 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) @@ -337,10 +345,10 @@ def owned_free(counts: Mapping[str, int], added_runs: int) -> int: Taken is the larger of the jobs the janitor saw and what the runs holding the pool will need at their peak, so a run whose later jobs do not exist yet still counts them. Each run replayed since the snapshot is charged - MAX_RUN_JOBS, since its peak is unknown here. + REPLAYED_RUN_JOBS, since its own peak is unknown here. """ taken = max(counts["running"] + counts["queued"], counts.get("committed", 0)) - return counts.get("capacity", 0) - taken - added_runs * MAX_RUN_JOBS + return counts.get("capacity", 0) - taken - added_runs * REPLAYED_RUN_JOBS def pick(load: Mapping[str, Mapping[str, int]], added: Mapping[str, int], usable: Sequence[str], @@ -386,7 +394,7 @@ def decide( on macOS 26) while the replay still spreads over the whole order. `jobs` is this run's peak machine count, which an owned pool must have free. Replayed runs are placed as if they needed one machine (so any that could - have taken an owned pool is assumed to) and charged MAX_RUN_JOBS there. + have taken an owned pool is assumed to) and charged REPLAYED_RUN_JOBS there. """ if not isinstance(snapshot, Mapping) or not isinstance(snapshot.get("pools"), Mapping): return Choice("", "", "no readable pool snapshot") diff --git a/tests/test_ci_owned_pool_rescue.py b/tests/test_ci_owned_pool_rescue.py index bdb7177f7902..73f39a46d634 100644 --- a/tests/test_ci_owned_pool_rescue.py +++ b/tests/test_ci_owned_pool_rescue.py @@ -99,6 +99,9 @@ def force_cancel(self, run_id): def rerun(self, run_id): self.calls.append("rerun") + def rerun_failed(self, run_id): + self.calls.append("rerun-failed") + def event(**overrides): run = {"id": RUN_ID, "path": ".github/workflows/ci.yml", "event": "pull_request", "run_attempt": 1, @@ -137,6 +140,64 @@ def jobs(seconds): return jobs +def refused_job(name="macos / macOS compile admission", *, seconds=8, steps=None, labels=(MINI,)): + found = job(name, status="completed", labels=labels, created=40, runner="mini-1") + found.update(conclusion="failure", started_at=stamp(41), completed_at=stamp(41 + seconds), + steps=[{"name": "Set up job", "conclusion": "failure"}] if steps is None else steps) + return found + + +def refusing_run(refused_at=60, **kwargs): + def jobs(seconds): + found = [changes()(seconds)] + if seconds >= refused_at: + found.append(refused_job(**kwargs)) + elif seconds >= 40: + found.append(job("macos / macOS compile admission", labels=[MINI], created=40)) + return found + return jobs + + +class Refusal(unittest.TestCase): + def test_what_counts_as_a_refusal(self): + self.assertTrue(rescue.refused(refused_job())) + self.assertTrue(rescue.refused(refused_job(steps=[]))) + self.assertTrue(rescue.refused(refused_job(steps=[{"name": "Set up job", "conclusion": "success"}, + {"name": "Runner hook", "conclusion": "failure"}]))) + # A step of the workflow ran, the job ran too long, it is not on an owned pool, or it did not fail. + self.assertFalse(rescue.refused(refused_job(steps=[{"name": "Set up job", "conclusion": "success"}, + {"name": "Checkout", "conclusion": "success"}, + {"name": "Build", "conclusion": "failure"}]))) + self.assertFalse(rescue.refused(refused_job(seconds=rescue.REFUSAL_SECONDS + 1))) + self.assertFalse(rescue.refused(refused_job(labels=(BLACKSMITH,)))) + self.assertFalse(rescue.refused({**refused_job(), "conclusion": "cancelled"})) + + def test_a_refused_job_reruns_the_failed_jobs_after_cancelling(self): + clock = Clock() + api = FakeAPI(clock, refusing_run(), marker=True) + code, summary = run_main(api, clock) + self.assertEqual(code, 0) + self.assertEqual(api.calls[-4:], ["cancel", "run", "pull", "rerun-failed"]) + self.assertNotIn("rerun", api.calls) + self.assertIn(f"refused by {MINI} at job start", summary) + self.assertIn("attempt 2 takes retry_runner", summary) + + def test_a_finished_run_with_a_refusal_needs_no_cancel(self): + clock = Clock() + api = FakeAPI(clock, refusing_run(), marker=True, finished=lambda seconds: seconds >= 60) + _, summary = run_main(api, clock) + self.assertNotIn("cancel", api.calls) + self.assertEqual(api.calls[-1], "rerun-failed") + self.assertIn("re-ran the failed jobs", summary) + + def test_a_refusal_on_a_moved_head_is_left_alone(self): + clock = Clock() + api = FakeAPI(clock, refusing_run(), marker=True, head="b" * 40) + _, summary = run_main(api, clock) + self.assertNotIn("rerun-failed", api.calls) + self.assertIn("not rescued", summary) + + class Scope(unittest.TestCase): def test_owned_pools_off_makes_no_request(self): for value in ("", "0"): diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 96242bfacc1e..4ea8f8ebe188 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -461,10 +461,16 @@ def jobs(**flags): self.assertEqual(jobs(), 1) self.assertEqual(jobs(cli="true", remote_daemon="true"), 3) self.assertEqual(jobs(unit_suite="true"), 1) + # A CLI change adds the pipe lane and cli-product-tests after admission. self.assertEqual(jobs(unit_suite="true", unit_in_admission="true", cli="true"), 2) - # Full suite: seven shards and tests-build-and-lag after admission, and the Claude wrapper. + self.assertEqual(jobs(unit_suite="true", cli="true"), 3) + # Full suite: seven shards, tests-build-and-lag and cli-product-tests after + # admission, beside the three side lanes. self.assertEqual(jobs(full_suite="true", cli="true", remote_daemon="true"), pool.MAX_RUN_JOBS) - self.assertEqual(jobs(macos="false", claude_wrapper="true", cli="true"), 2) + self.assertEqual(pool.MAX_RUN_JOBS, 12) + # A CLI-only run still compiles, then tests the bundled CLI. + self.assertEqual(jobs(macos="false", cli="true"), 2) + self.assertEqual(jobs(macos="false", claude_wrapper="true", cli="true"), 3) self.assertEqual(jobs(macos="false"), 0) def test_committed_peaks_count_jobs_not_created_yet(self): @@ -478,11 +484,13 @@ def test_queued_jobs_take_machines_without_closing_the_pool(self): self.assertEqual(owned_choice(fleet(busy=5, queued=3)).runner, MINI) self.assertEqual(owned_choice(fleet(busy=5, queued=4)).runner, LARGE) - def test_replayed_runs_are_charged_the_largest_peak(self): + def test_replayed_runs_are_charged_a_compile_only_peak(self): # A run created since the snapshot has an unknown peak: any that could - # have taken the pool is assumed to, and charged MAX_RUN_JOBS there. - self.assertEqual(owned_choice(fleet(), machines=22, routed=1).runner, MINI) - self.assertEqual(owned_choice(fleet(), routed=1).runner, LARGE) + # have taken the pool is assumed to, and charged REPLAYED_RUN_JOBS (4). + self.assertEqual(pool.REPLAYED_RUN_JOBS, 4) + # 11 machines: two newer runs take 8, leaving 3 for this 3-job run. + self.assertEqual(owned_choice(fleet(), routed=2).runner, MINI) + self.assertEqual(owned_choice(fleet(), routed=2, jobs=4).runner, LARGE) self.assertEqual(owned_choice(fleet(busy=10), routed=1).runner, LARGE) def test_stale_snapshot_or_no_slots_skips_the_pool(self): From d5a5d8179427343080e621efcdc1bd111d1dfad0 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 12:50:49 -0400 Subject: [PATCH 4/5] ci: teach the seed test's expression evaluator numeric comparisons tests/test_seed_derived_data.py evaluates compile admission's runs-on with its own Actions expression subset, which could not parse the `github.run_attempt > 1` that pr_retry_runner added, and failed the workflow guard tests. It now compares <, >, <= and >= as numbers the way Actions coerces, the test context carries github.run_attempt, and a new case checks that attempt 2 of an owned-pool run takes pr_retry_runner. Also records why splitting a refused run is sound: the minis and Blacksmith's macOS 26 images carried the same Xcode 26.6 build (17F113) on 2026-09-24, and the slot example now shows the 12 std minis. Co-Authored-By: Claude Opus 5.5 --- docs/ci-runners.md | 7 +++-- scripts/ci/owned_pool_rescue.py | 7 ++++- scripts/ci/pr_runner_pool.py | 2 +- tests/test_seed_derived_data.py | 50 +++++++++++++++++++++++++++++---- 4 files changed, 57 insertions(+), 9 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index f63a8c46e4c3..043a714f3720 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -201,7 +201,7 @@ names no owned pool. | Variable | Default | Effect | | --- | --- | --- | | `CI_PR_POOL_OWNED` | unset (off) | `1` puts owned pools first and turns on the rescue below | -| `CI_OWNED_POOL_SLOTS` | unset (no slots) | JSON, owned pool label to machine count, the `conforming_count` from `glaeda-mini-fleet pools --json`: `{"glaeda-std-xcode-26.6": 11, "glaeda-light-xcode-26.6": 2}` | +| `CI_OWNED_POOL_SLOTS` | unset (no slots) | JSON, owned pool label to machine count, the `conforming_count` from `glaeda-mini-fleet pools --json`: `{"glaeda-std-xcode-26.6": 12, "glaeda-light-xcode-26.6": 2}` | Each entry of `CI_OWNED_POOL_SLOTS` that is not an owned label with a positive whole number of machines counts as none. While owned pools are on, the @@ -227,7 +227,10 @@ The watcher treats a job on the persistent pool that failed within 120 seconds of starting, with no workflow step succeeded, as refused. It confirms the head has not moved, cancels the run if it is still going, and re-runs its failed jobs, so nobody has to. That attempt 2 keeps what passed and sends the -rest to `retry_runner` (below). +rest to `retry_runner` (below). Products built on a mini are then tested on +Blacksmith, which is sound only while both carry the same Xcode build: on +2026-09-24 the minis and Blacksmith's 6vcpu and 12vcpu macOS 26 images all +reported Xcode 26.6 build 17F113 (jobs 107712770707 and 107710434810). "Re-run failed jobs" is different: `changes` passed, so it is not re-run, and the failed jobs read attempt 1's outputs, owned pool included, with no watcher diff --git a/scripts/ci/owned_pool_rescue.py b/scripts/ci/owned_pool_rescue.py index 8eec24f9e889..531c443fcf5f 100644 --- a/scripts/ci/owned_pool_rescue.py +++ b/scripts/ci/owned_pool_rescue.py @@ -30,7 +30,12 @@ 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. +(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 +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 +here instead (rescue with failed_only=False). A job's wait is measured from the later of its `created_at` and the first time the watcher saw it queued, so a job record created before its `needs` diff --git a/scripts/ci/pr_runner_pool.py b/scripts/ci/pr_runner_pool.py index 25a4cfb0c403..1758c79871eb 100644 --- a/scripts/ci/pr_runner_pool.py +++ b/scripts/ci/pr_runner_pool.py @@ -37,7 +37,7 @@ outlives the job. They take part only when `vars.CI_PR_POOL_OWNED == '1'`, and then go first in the default order so Blacksmith is overflow. Their capacity is the number of machines the fleet manifest gives each label, -published as vars.CI_OWNED_POOL_SLOTS (JSON, `{"glaeda-std-xcode-26.6": 11}`). +published as vars.CI_OWNED_POOL_SLOTS (JSON, `{"glaeda-std-xcode-26.6": 12}`). The janitor's snapshot counts the jobs queued and running on each owned label from the job listings it already makes, and `committed`: what the runs holding the pool need at their peak, read from the marker each one uploads diff --git a/tests/test_seed_derived_data.py b/tests/test_seed_derived_data.py index 7a89bdd3c929..7f3f2f8c867d 100644 --- a/tests/test_seed_derived_data.py +++ b/tests/test_seed_derived_data.py @@ -329,7 +329,7 @@ def named(step_list, name): return matches[0], step_list[matches[0]] -TOKEN = re.compile(r"\s*(\|\||&&|==|!=|!|\(|\)|,|'(?:[^']|'')*'|[A-Za-z_][A-Za-z0-9_.-]*)") +TOKEN = re.compile(r"\s*(\|\||&&|==|!=|>=|<=|>|<|!|\(|\)|,|'(?:[^']|'')*'|[0-9]+(?:\.[0-9]+)?|[A-Za-z_][A-Za-z0-9_.-]*)") def evaluate(expression, context): @@ -338,6 +338,8 @@ def evaluate(expression, context): `a && b` is b when a is truthy, else a; `a || b` is a when truthy, else b. Names resolve by dotted path in `context`; a missing one is null, which compares equal to ''. startsWith() compares case-insensitively. + `<`, `>`, `<=` and `>=` compare as numbers, the way Actions coerces: null + and '' are 0, and a string that is not a number never compares true. """ text = expression.strip() if text.startswith("${{") and text.endswith("}}"): @@ -371,6 +373,8 @@ def primary(): return token[1:-1].replace("''", "'") if token in ("true", "false"): return token == "true" + if token[0].isdigit(): + return float(token) if token == "startsWith" and peek() == "(": take() haystack = either() @@ -386,12 +390,26 @@ def primary(): value = value.get(part) if isinstance(value, dict) else None return value + def number(value): + if value is None or value == "": + return 0.0 + if isinstance(value, bool): + return float(value) + try: + return float(value) + except (TypeError, ValueError): + return float("nan") + def comparison(): left = primary() - while peek() in ("==", "!="): + while peek() in ("==", "!=", ">", "<", ">=", "<="): operator, right = take(), primary() - equal = ("" if left is None else str(left)) == ("" if right is None else str(right)) - left = equal if operator == "==" else not equal + if operator in ("==", "!="): + equal = ("" if left is None else str(left)) == ("" if right is None else str(right)) + left = equal if operator == "==" else not equal + else: + a, b = number(left), number(right) + left = {">": a > b, "<": a < b, ">=": a >= b, "<=": a <= b}[operator] return left def both(): @@ -418,7 +436,7 @@ def either(): def github_context(event_name, ref="refs/heads/main", **variables): return { - "github": {"event_name": event_name, "ref": ref, "repository_owner": "manaflow-ai"}, + "github": {"event_name": event_name, "ref": ref, "repository_owner": "manaflow-ai", "run_attempt": "1"}, "vars": { "MACOS_RUNNER_PR": "pool-pr", "MACOS_RUNNER_15": "pool-15-paid", @@ -709,6 +727,23 @@ def test_fork_pull_request_admission_stays_on_blacksmith(self): "/Applications/Xcode-pr.app" if head == "manaflow-ai/cmux" else "/Applications/Xcode-15.app", ) + 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"), + ): + 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): + self.assertEqual(evaluate(admission["runs-on"], context), runner) + self.assertEqual(evaluate(admission["env"]["CMUX_PRODUCT_RUNNER"], context), runner) + def test_the_expression_evaluator_follows_actions_semantics(self): context = {"vars": {"A": "a", "EMPTY": ""}} self.assertEqual(evaluate("${{ vars.A && 'x' || 'y' }}", context), "x") @@ -718,6 +753,11 @@ def test_the_expression_evaluator_follows_actions_semantics(self): self.assertIs(evaluate("${{ (vars.MISSING || '1') != '0' }}", context), True) self.assertIs(evaluate("${{ vars.A != 'b' && vars.A == 'a' }}", context), True) self.assertIs(evaluate("${{ startsWith(vars.A, 'A') }}", context), True) + numbers = {"github": {"run_attempt": "2"}, "vars": {"A": "a"}} + self.assertIs(evaluate("${{ github.run_attempt > 1 }}", numbers), True) + self.assertIs(evaluate("${{ github.run_attempt > 2 }}", numbers), False) + self.assertIs(evaluate("${{ vars.MISSING > 0 }}", numbers), False) + self.assertIs(evaluate("${{ vars.A > 0 || vars.A < 1 }}", numbers), False) self.assertIs(evaluate("${{ startsWith(vars.MISSING, 'a') }}", context), False) def test_no_workflow_compares_a_bare_variable_with_zero(self): From 0673aab8a40199308974d7f149bda7db6f3b958a Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 24 Sep 2026 13:06:22 -0400 Subject: [PATCH 5/5] ci: drop the persistent-compile route gate, retired on main in #14232 Main removed the route request step with the pilot, so the owned-pool condition added for it, its test and its doc paragraph go too. Co-Authored-By: Claude Opus 5.5 --- docs/ci-runners.md | 4 ---- tests/test_ci_pr_runner_pool.py | 5 ----- 2 files changed, 9 deletions(-) diff --git a/docs/ci-runners.md b/docs/ci-runners.md index 043a714f3720..2b8158265f9d 100644 --- a/docs/ci-runners.md +++ b/docs/ci-runners.md @@ -241,10 +241,6 @@ Xcode, which is also the Xcode the owned label names. Every pull request macOS pool, reads `github.run_attempt > 1 && inputs.pr_retry_runner` first. It is empty for a run on Blacksmith, so those re-run where they ran. -An owned-pool run never publishes a persistent-compile route request -(`CI_PERSISTENT_MAC_COMPILE`): its compile admission already runs on an owned -Mac, and the two would compete for the same machines. - The queue janitor treats an owned label as one more macOS pool. A stale pull request run (category b: closed, merged or superseded) is cancelled there on every sweep whatever the queue, which frees minis for current work. The other diff --git a/tests/test_ci_pr_runner_pool.py b/tests/test_ci_pr_runner_pool.py index 4ea8f8ebe188..2f6c9d7f6144 100644 --- a/tests/test_ci_pr_runner_pool.py +++ b/tests/test_ci_pr_runner_pool.py @@ -617,11 +617,6 @@ def test_the_picker_reads_the_runs_routing(self): self.assertNotIn("OWNED_JOBS_PER_RUN", env) self.assertEqual(changes["outputs"]["macos_pr_retry_runner"], "${{ steps.macos-pool.outputs.retry_runner }}") - def test_an_owned_pool_run_never_requests_the_persistent_compile_route(self): - steps = self.workflow("ci.yml")["jobs"]["changes"]["steps"] - request = next(step for step in steps if step.get("id") == "persistent-route-request") - self.assertIn("steps.macos-pool.outputs.persistent != 'true'", request["if"]) - def lanes(self, name): text = (WORKFLOWS / name).read_text() return [match.group("lane") for match in PR_ROUTE.finditer(text)]