diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2497c9699..0c75cb0408 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,13 +12,15 @@ on: - 'main' - 'release/**' merge_group: - # A daily heartbeat for the macOS and Windows lanes ONLY. Those two are - # gated on `merge_group`, and the merge queue is not enabled on this - # repository — no queue run since 2026-07-02 — so they had stopped running - # entirely: skipped on every pull request, and never reached afterwards. The - # pull-request gate below catches the diffs a path list can recognise; this - # catches everything it cannot, one day later, on `main`. Every other job - # here excludes `schedule` explicitly, so a nightly run is exactly two jobs. + # The macOS and Windows lanes' ONLY remaining trigger, and therefore this + # repository's only signal about a host that is not Linux with a GNU + # userland. Those two are otherwise gated on `merge_group`, and the merge + # queue is not enabled here — no queue run since 2026-07-02 — while their + # pull-request trigger is off until the standing Windows failures are fixed + # (see test_macos). A regression therefore surfaces here, one day later, on + # `main`, and nowhere else: treat a red nightly as a blocker, not as noise. + # Every other job here excludes `schedule` explicitly, so a nightly run is + # exactly two jobs. schedule: - cron: '17 19 * * *' workflow_dispatch: @@ -152,71 +154,6 @@ jobs: echo "ubuntu_runner=${ubuntu_runner}" >> "${GITHUB_OUTPUT}" echo "Selected Linux runner: ${ubuntu_runner}" - # Does this pull request need the macOS and Windows lanes? They are the only - # signal this repository has about a host that is not Linux with a GNU - # userland, and they are expensive, so they run on the diffs whose behaviour - # the HOST decides — shell, CI definitions, the script layer, the runner - # config, the platform-coupled subtrees. The classifier is a net, not a - # proof; the scheduled run on `main` is what covers everything a path list - # cannot see. - # - # Its own job, not a step in classify_pr: that job's outputs pick the Linux - # runner for everything else, and a new failure mode there (this one needs a - # checkout, on a pool whose workspace other jobs can poison) would take the - # whole run's routing with it. Here a failure costs one skipped - # classification, which the gate reads as "run the lanes". - # - # Hosted, and the checkout is of the pull request's BASE commit: this runs - # before any review, so checking out the contributor's head would run their - # classifier with this job's token, and staying off the ECS pool keeps it - # away from the poisoned-workspace class entirely. - classify_platform: - name: 'Classify platform sensitivity' - if: "${{ github.event_name == 'pull_request' }}" - runs-on: 'ubuntu-latest' - timeout-minutes: 5 - continue-on-error: true - permissions: - contents: 'read' - pull-requests: 'read' - outputs: - platform_sensitive: '${{ steps.platform.outputs.platform_sensitive }}' - steps: - - name: 'Check out the classifier from the base branch' - uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - with: - ref: '${{ github.event.pull_request.base.sha }}' - persist-credentials: false - sparse-checkout: '.github/scripts/ci' - - - name: 'Classify platform sensitivity' - id: 'platform' - env: - GH_TOKEN: '${{ github.token }}' - PR_NUMBER: '${{ github.event.pull_request.number }}' - IS_SAME_REPO_PR: '${{ github.event.pull_request.head.repo.full_name == github.repository }}' - run: |- - set -uo pipefail - # Fail-safe in every direction: only a confident `false` from the - # classifier skips the lanes. A classifier error, an unexpected - # word, a fork PR, or this whole job failing all end as "run" — a - # gate that fails open stops testing without ever saying so. - sensitive=true - if [ "${IS_SAME_REPO_PR}" = 'true' ]; then - set +e - classified="$(.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform)" - rc=$? - set -e - case "${rc}:${classified}" in - 0:true|0:false) sensitive="${classified}" ;; - *) echo "::warning::Platform-sensitivity classifier returned rc=${rc} '${classified}'; running the macOS and Windows lanes." ;; - esac - else - echo "Fork PR detected; running the macOS and Windows lanes." - fi - echo "platform_sensitive=${sensitive}" >> "${GITHUB_OUTPUT}" - echo "Platform-sensitive: ${sensitive}" - # # Test: Node # @@ -916,13 +853,35 @@ jobs: # `realpath -m` in a workflow guard, with the suite that pinned it red on # every Mac). # - # Three triggers now, in cost order: a pull request whose diff the - # platform-sensitivity classifier recognises (shell, CI definitions, the - # script layer, the runner config, the platform-coupled subtrees), the merge - # queue if it is ever enabled again, and a nightly run on `main` for - # everything a path list cannot see. The pull-request gate is fail-safe — - # only a confident `false` skips, so a broken classifier costs runner minutes - # rather than coverage. + # The pull-request trigger is OFF, and this is the whole reason the nightly + # above exists. Over the 18 hours measured before it was pulled, the Windows + # lane reported 13 failures and 0 successes on pull requests — one standing + # set of Windows-only path and symlink failures (resolved paths into + # read_many_files, releaseWorktree through an ancestor symlink, the SHA-256 + # review worktree) repeating across unrelated PRs — and macOS queued for a + # p90 of 42 minutes, 159 at worst, on a hosted pool this repository does not + # saturate by itself. Neither lane gates a merge: the `main` ruleset carries + # no required status check. So the cost landed entirely on contributors, who + # read a red X no diff of theirs caused and waited on checks that never had + # to pass. + # + # Two triggers remain, in cost order: the merge queue if it is ever enabled + # again, and a nightly run on `main` — which is now the only thing keeping + # macOS and Windows visible at all, so it is load-bearing rather than a + # backstop. Read a red nightly as a blocker; nothing else will report it. + # + # Restoring the pull-request trigger is the last step of fixing those + # failures, not a separate decision — and it is a REVERT of the commit that + # carried this change, not an edit: the classifier job, its `if` arms on + # both lanes, and the contract pins in scripts/tests/ci-platform-lanes. + # test.js all moved together, and that suite now fails a half-restoration + # (a trigger without its classifier, or the reverse). The classifier script + # `classify-platform-sensitivity.mjs`, the `platform` mode of + # `classify-pr-profile.sh` and both their test files were left untouched + # precisely so the revert stays clean. Worth revisiting at the same time: + # that classifier calls every fork pull request platform-sensitive, which + # is most of the traffic here and most of what the macOS queue was spent + # on. # # Two named jobs, not a matrix: a skipped matrix job reports one collapsed # check name, never the per-OS contexts, so a required-check configuration @@ -932,17 +891,13 @@ jobs: # currently insurance, not a live constraint.) test_macos: name: 'Test (macos-latest, Node 22.x)' - needs: - - 'classify_pr' - - 'classify_platform' + needs: 'classify_pr' if: |- ${{ !cancelled() && ( github.event_name == 'merge_group' || github.event_name == 'schedule' || - github.event_name == 'workflow_dispatch' || - (github.event_name == 'pull_request' && - needs.classify_platform.outputs.platform_sensitive != 'false') + github.event_name == 'workflow_dispatch' ) }} runs-on: 'macos-latest' @@ -1022,28 +977,24 @@ jobs: # Windows counterpart of test_macos (see that job's note). ECS is the default # with a windows-2022 kill-switch fallback; the check name stays unchanged so - # it matches the required-status-check context. The lane now runs on pull - # requests too, but every pull request runs on hosted windows-2022: a - # pull_request run executes the workflow YAML from the PR's own merge commit, - # so any PR this lane admits could rewrite `runs-on` in the same diff that - # reaches it. A gate the gated tree controls is no gate; the pool is reached - # only by triggers an unreviewed PR cannot open — the post-approval merge - # queue, schedule and dispatch. ECS-only tuning is gated on runner.environment; - # the hosted fallback is the pre-ECS job plus the checkout guard and a - # job-level timeout-minutes. + # it matches the required-status-check context. The `!= 'pull_request'` arm + # of `runs-on` is inert while the pull-request trigger is off, and stays for + # when it returns: a pull_request run executes the workflow YAML from the + # PR's own merge commit, so any PR this lane admits could rewrite `runs-on` + # in the same diff that reaches it. A gate the gated tree controls is no + # gate; the pool is reached only by triggers an unreviewed PR cannot open — + # the post-approval merge queue, schedule and dispatch. ECS-only tuning is + # gated on runner.environment; the hosted fallback is the pre-ECS job plus + # the checkout guard and a job-level timeout-minutes. test_windows: name: 'Test (windows-latest, Node 22.x)' - needs: - - 'classify_pr' - - 'classify_platform' + needs: 'classify_pr' if: |- ${{ !cancelled() && ( github.event_name == 'merge_group' || github.event_name == 'schedule' || - github.event_name == 'workflow_dispatch' || - (github.event_name == 'pull_request' && - needs.classify_platform.outputs.platform_sensitive != 'false') + github.event_name == 'workflow_dispatch' ) }} runs-on: '${{ vars.MAINTAINER_ECS_RUNNER_DISABLED != ''true'' && github.event_name != ''pull_request'' && fromJSON(''["self-hosted", "Windows", "X64", "ecs-win"]'') || fromJSON(''["windows-2022"]'') }}' diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js index 056f43f09c..6be3996d7e 100644 --- a/scripts/tests/ci-platform-lanes.test.js +++ b/scripts/tests/ci-platform-lanes.test.js @@ -12,8 +12,19 @@ // repository's only non-Linux, non-GNU signal was silently off, and a macOS // failure shipped and sat in `main` (#9220). Nothing here can prove a lane // ran; what these tests hold is the wiring that lets it: the triggers, the -// fail-safe direction of the gate, the nightly's blast radius, and the -// alerting that makes a nightly failure visible. +// nightly's blast radius, and the alerting that makes a nightly failure +// visible. +// +// The pull-request trigger and its platform-sensitivity classifier are OFF +// while the standing Windows failures are being fixed (see the note above +// test_macos in ci.yml): on pull requests the Windows lane was reporting +// failures on every PR for defects no PR caused, and neither lane gates a +// merge. That leaves the nightly as the lanes' ONLY live trigger, so the +// assertions here are the wiring that keeps it alive — the schedule exists, +// both lanes accept it, nothing else rides it, and its failure files an +// issue. Restoring the pull-request path means reverting the commit that +// carried this change; the classifier script and its tests were left in +// place so that stays a revert. import { readFileSync } from 'node:fs'; import { describe, expect, it } from 'vitest'; @@ -39,18 +50,25 @@ describe('platform lanes — triggers', () => { }); for (const lane of LANES) { - it(`${lane} runs on the schedule, the queue, a dispatch, and a sensitive PR`, () => { + it(`${lane} runs on the schedule, the queue, and a dispatch`, () => { const cond = condOf(lane); // Presence AND the disjunction between clauses: an `&&` where a `||` // belongs leaves the gate unsatisfiable for a trigger (event_name is // single-valued) while a presence-only check stays green. - expect(cond).toMatch(/event_name == 'schedule'\s*\|\|/); expect(cond).toMatch(/event_name == 'merge_group'\s*\|\|/); - expect(cond).toMatch(/event_name == 'workflow_dispatch'\s*\|\|/); - expect(cond).toContain("github.event_name == 'pull_request'"); - expect(cond).toContain( - 'needs.classify_platform.outputs.platform_sensitive', - ); + expect(cond).toMatch(/event_name == 'schedule'\s*\|\|/); + expect(cond).toContain("github.event_name == 'workflow_dispatch'"); + }); + + it(`${lane} stays off the pull-request path while it is red there`, () => { + // Half a restoration is worse than none: a pull_request arm put back + // without its classifier (or the reverse) either runs the lanes on + // every PR or consults an output no job produces. Restore the trigger + // by reverting the commit that removed it, not by editing one side. + const cond = condOf(lane); + expect(cond).not.toContain("'pull_request'"); + expect(cond).not.toContain('platform_sensitive'); + expect(ci.jobs[lane].needs).not.toContain('classify_platform'); }); it(`${lane}'s triggers are alternatives, not requirements`, () => { @@ -67,11 +85,8 @@ describe('platform lanes — triggers', () => { cond.lastIndexOf(')'), ); expect(group).toContain("github.event_name == 'schedule'"); - expect(group.split('||').length).toBeGreaterThanOrEqual(4); - // The only `&&` allowed inside the group is the one binding the - // pull-request clause to its classifier output. + expect(group.split('||').length).toBeGreaterThanOrEqual(3); for (const clause of group.split('||')) { - if (clause.includes('platform_sensitive')) continue; expect( clause, `event clause is conjoined: ${clause.trim()}`, @@ -79,17 +94,10 @@ describe('platform lanes — triggers', () => { } }); - it(`${lane} skips only on a confident 'false'`, () => { - // The fail-safe direction is the whole design: `== 'true'` would turn - // every classifier error, every skipped classify job and every empty - // output into a silently skipped lane. `!= 'false'` spends runner - // minutes instead of coverage. - const cond = condOf(lane); - expect(cond).toContain("platform_sensitive != 'false'"); - expect(cond).not.toContain("platform_sensitive == 'true'"); - // And the gate must survive a skipped or failed classifier job. - expect(cond).toContain('!cancelled()'); - expect(ci.jobs[lane].needs).toContain('classify_platform'); + it(`${lane} survives an upstream skip`, () => { + // classify_pr is still a `needs` edge; without `!cancelled()` a skip + // or failure there would skip the lane on the nightly too. + expect(condOf(lane)).toContain('!cancelled()'); }); } @@ -166,75 +174,26 @@ describe('platform lanes — triggers', () => { }); }); -describe('platform lanes — the sensitivity classifier job', () => { - const job = ci.jobs.classify_platform; - - it('exists, is cheap, and cannot take the run down with it', () => { - expect(job).toBeDefined(); - expect(job['continue-on-error']).toBe(true); - expect(job['timeout-minutes']).toBeLessThanOrEqual(10); - // Hosted on purpose: it needs a checkout, and the persistent pool's - // workspace is exactly what other jobs have poisoned before. - expect(job['runs-on']).toBe('ubuntu-latest'); - expect(job.outputs.platform_sensitive).toContain( - 'steps.platform.outputs.platform_sensitive', - ); +describe('platform lanes — the retired sensitivity classifier', () => { + it('is gone from the workflow, whole', () => { + // Off with the pull-request trigger it fed: nothing consumes its output, + // so a surviving job would spend a hosted runner per pull request on a + // classification no gate reads — and a surviving reference would consult + // a job that no longer exists. Deleted means deleted everywhere. + expect(ci.jobs.classify_platform).toBeUndefined(); + expect(JSON.stringify(ci)).not.toContain('classify_platform'); }); - it('checks out the base commit, never the pull request head', () => { - // This job runs before any review and executes a script from the tree it - // checks out. The contributor's head would be the contributor's - // classifier, running with this job's token. - const checkout = job.steps.find((s) => - String(s.uses ?? '').includes('actions/checkout'), - ); - expect(checkout).toBeDefined(); - expect(checkout.with.ref).toBe('${{ github.event.pull_request.base.sha }}'); - expect(checkout.with.ref).not.toContain('head'); - expect(checkout.with['persist-credentials']).toBe(false); - }); - - it('answers "run the lanes" for anything it is not sure about', () => { - const run = job.steps.find((s) => s.id === 'platform').run; - // A fork PR is not classified at all — the listing call is the same one - // the profile gate restricts to same-repo PRs. - expect(run).toContain('IS_SAME_REPO_PR'); - expect(run).toContain('sensitive=true'); - // Only the two words the classifier is allowed to say are accepted; a - // non-zero exit or anything else warns and runs the lanes. - expect(run).toContain('0:true|0:false'); - expect(run).toMatch(/::warning::.*running the macOS and Windows lanes/); - // The wrapper call is wrapped in `set +e`/`set -e`: the runner invokes - // `shell: bash` steps with `-e`, so without the guard a non-zero exit - // aborts the step at the assignment and the warn-and-run case above is - // dead code. Same shape as the sibling Classify CI profile step. - expect(run).toContain( - [ - ' set +e', - ' classified="$(.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform)"', - ' rc=$?', - ' set -e', - ].join('\n'), - ); - }); - - it('drives the classifier through the shared listing wrapper', () => { - // Not a second listing: the wrapper's comment declares itself the single - // home of that contract, and two call sites listing separately is how the - // same PR ends up classified differently in two places. - const run = job.steps.find((s) => s.id === 'platform').run; - expect(run).toContain( - '.github/scripts/ci/classify-pr-profile.sh "${GITHUB_REPOSITORY}" "${PR_NUMBER}" platform', - ); - }); - - it('runs the classifier unit tests in CI', () => { - // The helper-test list is the single place both the github_ci_only step - // and the full Test step read; a classifier not named there is untested - // on every profile. + it('keeps its classifier script tested for the restoration', () => { + // The script layer stayed in place precisely so restoring the + // pull-request trigger is a revert. A classifier that rotted untested in + // the meantime would make that revert a regression instead. expect(ci.env.HELPER_TESTS).toContain( '.github/scripts/ci/classify-platform-sensitivity.test.mjs', ); + expect(ci.env.HELPER_TESTS).toContain( + '.github/scripts/ci/classify-pr-profile.test.mjs', + ); }); });