From 3d609051c06f8459663fc3542d3ecffce99452d9 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 28 Aug 2026 10:50:03 +0800 Subject: [PATCH 1/4] ci: restore the post-merge push trigger on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ci.yml` dropped its `push` trigger on the premise that the merge queue validates the merged tree before it lands. No merge queue is enabled — no queue run since 2026-07-02 — so nothing validated that tree on either side of the merge: `main` had no gate before a merge, and no check faster than the ~40-minute E2E after one. Accept `push` on `main` in the `test` job alone. Every other job keeps its existing gate, so a post-merge run is exactly Classify PR + Test, and a nightly still resolves to exactly the two platform lanes. The expensive, PR-shaped steps inside `test` — the no-AK integration gate, the checkout-head verifier and the test reporter — already carry their own pull_request / merge_group gates, so the push lane is lint, static analysis and unit tests. Route the lane to the ECS pool: a push to `main` is the most trusted context there is, and Test is the ten-plus-minute job that should not spend a scarce hosted Linux runner on every merge. classify_pr's own `runs-on` keeps no push arm by design — it is the canonical association-routing text that sdk-java.yml and serve-ab.yml mirror, and a push carries no author association to route on — so the two halves disagree on push deliberately; a new routing test pins that asymmetry so a later "drift fix" has to read the reason first. --- .github/scripts/ci-runner-routing.test.mjs | 40 +++++++++++++++++ .github/workflows/ci.yml | 50 ++++++++++++++++------ scripts/tests/ci-platform-lanes.test.js | 6 +-- 3 files changed, 81 insertions(+), 15 deletions(-) diff --git a/.github/scripts/ci-runner-routing.test.mjs b/.github/scripts/ci-runner-routing.test.mjs index a1773eb5706..66c2301cf38 100644 --- a/.github/scripts/ci-runner-routing.test.mjs +++ b/.github/scripts/ci-runner-routing.test.mjs @@ -224,6 +224,46 @@ describe('ci.yml classify_pr runner routing', () => { ); }); + it('a push to main reaches ECS, and the kill-switch still wins', () => { + // The post-merge fast signal (Classify PR + Test on `main`) runs the same + // ten-plus-minute Test job a pull request does, in the most trusted + // context there is: the code is already merged and the YAML is main's + // own. Route it to the pool instead of spending a scarce hosted Linux + // runner on every merge. A push carries no author association, so this + // arm is necessarily event-based. + assert.equal( + runPickRunner({ + ecsDisabled: false, + sameRepo: false, + assoc: '', + eventName: 'push', + }), + ECS, + ); + assert.equal( + runPickRunner({ + ecsDisabled: true, + sameRepo: false, + assoc: '', + eventName: 'push', + }), + HOSTED, + 'kill-switch must revert post-merge runs to hosted', + ); + }); + + it('keeps the push arm out of the classify runs-on expression', () => { + // classify_pr routes twice, and on a push the two halves deliberately + // disagree: the shell step sends downstream Linux jobs to ECS while this + // expression leaves classify_pr itself hosted. Pin the asymmetry so a + // future "drift fix" has to read why first — this expression is the + // canonical association-routing text that sdk-java.yml and serve-ab.yml + // mirror, a push has no association to route on, and classify_pr is a + // seconds-long job whose pool costs nothing either way. The drift guard + // above evaluates both halves on pull_request, where they must agree. + assert.doesNotMatch(classifyRunsOn, /github\.event_name == 'push'/); + }); + it('the runs-on expression keeps the trusted clause and kill-switch', () => { // Structural pins for the expression half of the drift guard — the // simulation above re-implements it, so pin the real text too. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5dd50559f5f..863137e7abc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,10 +3,21 @@ name: 'Qwen Code CI' on: - # No `push` trigger: every job here is gated to pull_request / merge_group, so - # a push to `main` ran nothing (CodeQL was the last push job and moved to its - # own scheduled codeql.yml). The merge queue validates the merged tree before - # it lands, so there is nothing left to run on the post-merge push. + # The reason the `push` trigger was removed no longer holds. It said the + # merge queue validates the merged tree before it lands, so a post-merge run + # would be redundant — but no merge queue is enabled here (no queue run since + # 2026-07-02), so nothing validated that tree on either side of the merge. + # `main` was left with no gate before a merge and no check faster than the + # ~40-minute E2E after one, which is how a regression can sit in `main` for + # hours. Only `test` accepts this trigger (see its `if`); every other job + # still excludes push, so a post-merge run is exactly Classify PR + Test. + # The no-AK integration gate inside `test` stays gated to pull_request / + # merge_group, so this lane is the lint, static-analysis and unit-test + # subset — a fast signal, not a second full gate. Remove this trigger again + # once the merge queue is actually enabled and validates the merged tree. + push: + branches: + - 'main' pull_request: branches: - 'main' @@ -72,7 +83,7 @@ env: jobs: classify_pr: name: 'Classify PR' - if: "${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'workflow_dispatch' }}" + if: "${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'workflow_dispatch' || github.event_name == 'push' }}" # Gate runs on ECS for in-repo PRs, fork PRs whose author has write access # (OWNER/MEMBER/COLLABORATOR association — a write-access author is as # trusted as an in-repo branch), and the merge queue (base-repo context), @@ -127,9 +138,19 @@ jobs: echo "skip_ci=${skip_ci}" # In-repo PRs, fork PRs whose author has write access - # (OWNER/MEMBER/COLLABORATOR association), and the merge queue - # (base-repo context) run the Linux jobs on ECS; other fork PRs stay - # hosted. Disable via repo var MAINTAINER_ECS_RUNNER_DISABLED=true. + # (OWNER/MEMBER/COLLABORATOR association), the merge queue (base-repo + # context) and a push to `main` run the Linux jobs on ECS; other fork + # PRs stay hosted. Disable via repo var + # MAINTAINER_ECS_RUNNER_DISABLED=true. + # + # push is deliberately absent from the `runs-on` expression above, so on + # a post-merge run classify_pr itself stays hosted while the Test job it + # feeds goes to ECS. That asymmetry is intentional: the expression is the + # canonical association-routing text mirrored by sdk-java.yml and + # serve-ab.yml, a push carries no author association to route on, and + # classify_pr is a seconds-long job whose pool costs nothing — while Test + # is the ten-plus-minute job that should not spend a scarce hosted Linux + # runner on every merge. - name: 'Select Linux runner' id: 'pick_runner' env: @@ -148,7 +169,7 @@ jobs: if [[ "${ECS_DISABLED}" != "true" && "${DISPATCH_LINUX_RUNNER}" == "self-hosted" ]]; then ubuntu_runner='["self-hosted", "linux", "x64", "ecs-qwen"]' fi - elif [[ "${ECS_DISABLED}" != "true" && ( "${SAME_REPO}" == "true" || "${trusted_author}" == "true" || "${EVENT_NAME}" == "merge_group" ) ]]; then + elif [[ "${ECS_DISABLED}" != "true" && ( "${SAME_REPO}" == "true" || "${trusted_author}" == "true" || "${EVENT_NAME}" == "merge_group" || "${EVENT_NAME}" == "push" ) ]]; then ubuntu_runner='["self-hosted", "linux", "x64", "ecs-qwen"]' fi echo "ubuntu_runner=${ubuntu_runner}" >> "${GITHUB_OUTPUT}" @@ -162,9 +183,14 @@ jobs: needs: 'classify_pr' # Stay running on release-sync PRs so the required Test contexts still # report; the per-step skip_ci guards below make them no-op (pass) there. - # Not on push: the merge queue already tested the merged tree, so a - # post-merge re-run on `main` would be redundant. - if: "${{ !cancelled() && github.event_name != 'push' && github.event_name != 'schedule' }}" + # Accepts push on `main`: this is the only post-merge check faster than the + # ~40-minute E2E, and with no merge queue enabled nothing validates the + # merged tree before it lands either. The expensive and PR-shaped steps + # stay off it on their own gates — the no-AK integration gate, the + # checkout-head verifier and the test reporter are each already restricted + # to pull_request / merge_group — so a push run is lint, static analysis + # and unit tests. + if: "${{ !cancelled() && github.event_name != 'schedule' }}" runs-on: '${{ fromJSON(needs.classify_pr.outputs.ubuntu_runner || ''["ubuntu-latest"]'') }}' timeout-minutes: 60 outputs: diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js index 6be3996d7e6..2e29d7196ec 100644 --- a/scripts/tests/ci-platform-lanes.test.js +++ b/scripts/tests/ci-platform-lanes.test.js @@ -41,9 +41,9 @@ const condOf = (job) => String(ci.jobs[job].if ?? ''); describe('platform lanes — triggers', () => { it('gives the workflow a scheduled trigger', () => { - // Without it the lanes have no path to `main` at all: `ci.yml` has no - // push trigger by design, so a merge-queue-only gate on a repository with - // no merge queue is an off switch. + // Without it the lanes have no path to `main` at all: `ci.yml`'s push + // trigger is accepted by the `test` job alone, so a merge-queue-only gate + // on a repository with no merge queue is an off switch for these two. expect(triggers.schedule).toBeDefined(); expect(Array.isArray(triggers.schedule)).toBe(true); expect(triggers.schedule[0].cron).toMatch(/^\S+ \S+ \S+ \S+ \S+$/); From 41a9b520ce00f7dac4346ad3ae06002cecaaf874 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 28 Aug 2026 13:13:32 +0800 Subject: [PATCH 2/4] ci: address review findings on the post-merge push lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight findings from the review pass, all confirmed against the tree. R1-1 (Critical): the `test` job's checkout `ref:` fell through to `github.ref` on push — `refs/heads/main` resolved at fetch time, a moving tip — while the check run attaches to `github.sha`. The lane could validate a tree it did not report on, and nothing detected it: the checkout-head verifier is gated to pull_request / merge_group and its ancestor check passes for a newer tip regardless. Regressions would still be caught (the tested tree is a descendant), but attribution breaks — red on a clean commit, autofix filed against the wrong merge. Pin push runs to `github.sha`, mirroring the merge_group arm; `e2e.yml` gets the same guarantee from actions/checkout's default. The five other checkout copies sit in jobs that exclude push. R1-5: the concurrency group was keyed on ref alone, so a post-merge run shared `Qwen Code CI-refs/heads/main` with the nightly (60-minute lanes, neither cancelling on main) and with any PR from a fork branch named literally `refs/heads/main` — a legal ref name that makes the group byte-identical while that run cancels in progress. Scope the group by event, as `e2e.yml` already does. R1-6: `WORKFLOW_SIZE_BASE_SHA` was empty on push, so both size enforcers failed closed there while PRs stayed green on leniency — the next stale baseline would redden every merge with an error naming a PR that does not exist. Add `github.event.before`; `github.sha` stays rejected because workflow_dispatch would compare a file against itself. R1-4: the coverage upload ran on push although its only consumer, post_coverage_comment, is gated to pull_request and nothing else reads the artifact. R1-2: the push lane's YAML wiring had no test witness — the routing tests drive pick_runner's shell directly and stay green if the trigger or either job gate loses its push arm. Add assertions covering the trigger, both accepting jobs, the exclusivity of every other job, and the three fixes above. Each is mutation-checked: removing any one fix reddens exactly one test. R1-3, R1-7 and R1-8 are three comments the trigger falsified: the failure watcher's "SCHEDULED run only" header, af-025's "ci.yml has no push trigger" premise, and this PR's own "only `test` accepts this trigger", which contradicted the classify_pr arm added in the same diff. The added prose put ci.yml over its size baseline; trimmed back under the allowance rather than raising the baseline. --- .github/workflows/ci.yml | 74 ++++++++---- .github/workflows/main-ci-failure-issue.yml | 13 ++- .github/workflows/qwen-autofix.md | 8 +- scripts/tests/ci-platform-lanes.test.js | 106 +++++++++++++++++- .../main-ci-failure-issue-workflow.test.js | 5 +- scripts/tests/workflow-size.test.js | 10 +- 6 files changed, 181 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c68c8577a6..72ad7bb32f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,18 +3,17 @@ name: 'Qwen Code CI' on: - # The reason the `push` trigger was removed no longer holds. It said the - # merge queue validates the merged tree before it lands, so a post-merge run - # would be redundant — but no merge queue is enabled here (no queue run since - # 2026-07-02), so nothing validated that tree on either side of the merge. - # `main` was left with no gate before a merge and no check faster than the - # ~40-minute E2E after one, which is how a regression can sit in `main` for - # hours. Only `test` accepts this trigger (see its `if`); every other job - # still excludes push, so a post-merge run is exactly Classify PR + Test. - # The no-AK integration gate inside `test` stays gated to pull_request / - # merge_group, so this lane is the lint, static-analysis and unit-test - # subset — a fast signal, not a second full gate. Remove this trigger again - # once the merge queue is actually enabled and validates the merged tree. + # This trigger was removed on the premise that the merge queue validates the + # merged tree before it lands. No queue is enabled (none since 2026-07-02), + # so nothing validated that tree on either side of a merge: `main` had no + # gate before one and nothing faster than the ~40-minute E2E after one. + # Only `test` and the `classify_pr` it depends on accept it (see their + # `if`s); every other job excludes push, so a post-merge run is exactly + # Classify PR + Test, and the no-AK gate inside `test` stays on + # pull_request / merge_group — this lane is lint, static analysis and unit + # tests. Removing it once the queue is enabled means dropping the trigger + # AND the push arm from BOTH those `if`s: a half-removal leaves every merge + # running Classify PR alone, publishing outputs nothing consumes. push: branches: - 'main' @@ -51,7 +50,15 @@ on: - 'hosted' concurrency: - group: '${{ github.workflow }}-${{ github.head_ref || github.ref }}' + # Scoped by event as well as ref, like `e2e.yml` and for the same collision + # class. Unscoped, the group collapses onto `Qwen Code CI-refs/heads/main` + # and the post-merge run shares it with (1) the nightly's 60-minute lanes, + # neither cancelling on `main`, so merges inside that window queue behind it + # and supersede each other unreported; and (2) any PR from a fork branch + # named literally `refs/heads/main` — a legal ref name whose `head_ref` makes + # the group byte-identical while that run cancels in progress, letting an + # actor with no repo access kill the signal on every merge. + group: '${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.ref }}' cancel-in-progress: |- ${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/heads/release/') }} @@ -78,7 +85,15 @@ env: # baseline went stale on main" (#9904). Wired once here so every lane # inherits it — the gate step and every `npm run test:ci` step, whatever # it is named — instead of each step hand-wiring a copy. - WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' + # `github.event.before` is the push lane's base: on a squash-merge push it is + # main's previous tip, restoring the PR lane's leniency for a baseline that + # went stale on main. Without it both enforcers see an empty base and fail + # closed, so the next stale baseline reddens every merge while every PR stays + # green — telling you to "update .size-baseline in this PR" when no PR + # exists. `github.sha` is deliberately not used: on workflow_dispatch it + # resolves to the checked-out commit and would compare the file against + # itself, failing open. An all-zeros `before` still fails closed. + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || github.event.before }}' jobs: classify_pr: @@ -143,14 +158,12 @@ jobs: # PRs stay hosted. Disable via repo var # MAINTAINER_ECS_RUNNER_DISABLED=true. # - # push is deliberately absent from the `runs-on` expression above, so on - # a post-merge run classify_pr itself stays hosted while the Test job it - # feeds goes to ECS. That asymmetry is intentional: the expression is the - # canonical association-routing text mirrored by sdk-java.yml and - # serve-ab.yml, a push carries no author association to route on, and - # classify_pr is a seconds-long job whose pool costs nothing — while Test - # is the ten-plus-minute job that should not spend a scarce hosted Linux - # runner on every merge. + # push is deliberately absent from the `runs-on` expression above, so a + # post-merge run leaves classify_pr hosted while the Test job it feeds + # goes to ECS. That is intentional: the expression is the canonical + # association-routing text sdk-java.yml and serve-ab.yml mirror, a push + # has no author association to route on, and classify_pr costs seconds — + # while Test should not spend a scarce hosted runner on every merge. - name: 'Select Linux runner' id: 'pick_runner' env: @@ -307,7 +320,17 @@ jobs: if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: - ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" + # Pin push runs to the event SHA: the `github.ref` fallback resolves + # `refs/heads/main` at fetch time — a moving tip — while the check run + # attaches to the triggering commit, so the lane would validate a tree + # it does not report on. Nothing detects that (the checkout-head + # verifier is gated to pull_request / merge_group, and its ancestor + # check passes for a newer tip regardless), and merges land close + # enough together to hit it routinely. Regressions are still caught — + # the tested tree is a descendant — but attribution breaks: red on a + # clean commit, autofix filed against the wrong merge. `e2e.yml` pins + # the event SHA by taking actions/checkout's default. + ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || (github.event_name == 'push' && github.sha) || github.ref }}" # Shallow: nothing here walks git history except on demand (the # verify guard below checks head.sha == HEAD; the size gate and its # vitest mirror fetch the PR's base commit at depth 1 when the @@ -683,8 +706,11 @@ jobs: name: 'test-results-fork-22.x-ubuntu-latest' path: 'packages/*/junit.xml' + # Excludes push: the only consumer, `post_coverage_comment`, is gated to + # pull_request and nothing else reads `coverage-reports-*`, so uploading + # on every merge spends egress and retention on a lane with no reader. - name: 'Upload coverage reports' - if: "${{ always() && needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" + if: "${{ always() && needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' && github.event_name != 'push' }}" uses: 'actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a' # v7.0.1 with: name: 'coverage-reports-22.x-ubuntu-latest' diff --git a/.github/workflows/main-ci-failure-issue.yml b/.github/workflows/main-ci-failure-issue.yml index 8615ce03e9b..a6d693d1e8f 100644 --- a/.github/workflows/main-ci-failure-issue.yml +++ b/.github/workflows/main-ci-failure-issue.yml @@ -4,10 +4,15 @@ name: 'Main CI Failure Issue' on: workflow_run: - # 'Qwen Code CI' is watched for its SCHEDULED run only (see the event - # filter below): that nightly is the macOS and Windows lanes' only - # trigger outside a pull request, and a lane nobody is told about is a - # lane nobody looks at — which is how those two came to stop running. + # 'Qwen Code CI' is watched for its SCHEDULED run and its post-merge PUSH + # run (see the event filter below — the push arm is not name-scoped, so it + # covers this workflow too). The nightly is the macOS and Windows lanes' + # only trigger outside a pull request, and a lane nobody is told about is a + # lane nobody looks at — which is how those two came to stop running. The + # push arm covers ci.yml's post-merge lane on `main`, whose whole point is + # to surface a regression as it lands; a failure there files an issue with + # `type/bug`, `status/ready-for-agent` and `autofix/approved`, so expect + # one per red merge, not per red nightly only. workflows: ['E2E Tests', 'SDK Python', 'Qwen Code CI'] types: ['completed'] # Trigger-level branch filter, not only the `if` below: 'Qwen Code CI' diff --git a/.github/workflows/qwen-autofix.md b/.github/workflows/qwen-autofix.md index 98bf4c1483f..79875058f0a 100644 --- a/.github/workflows/qwen-autofix.md +++ b/.github/workflows/qwen-autofix.md @@ -975,8 +975,12 @@ signal, NOT proof that main is healthy. MAIN_GREEN_CHECKS is sourced from the last-merged PR's PRE-MERGE check-runs, which ran against that PR merged with main-as-of-then — -never the tree now on main (ci.yml has no push trigger, so main's -squash commits carry no check-runs to read). main breaks here by +never the tree now on main. (ci.yml's post-merge push lane DOES put +check-runs on main's squash commits now, but that lane is lint, static +analysis and unit tests only — the no-AK integration gate and every +platform lane stay off the push trigger — so it is a strictly narrower +signal than a PR's full matrix, and this deliberately stays on the +pre-merge runs.) main breaks here by SEMANTIC CONFLICT: two PRs green apart but broken together. In exactly that state the last-merged PR is green, this signal reads green, and the update would merge a currently-broken main into a healthy PR. The diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js index 2e29d7196ec..e8b6c0f2eec 100644 --- a/scripts/tests/ci-platform-lanes.test.js +++ b/scripts/tests/ci-platform-lanes.test.js @@ -42,8 +42,9 @@ const condOf = (job) => String(ci.jobs[job].if ?? ''); describe('platform lanes — triggers', () => { it('gives the workflow a scheduled trigger', () => { // Without it the lanes have no path to `main` at all: `ci.yml`'s push - // trigger is accepted by the `test` job alone, so a merge-queue-only gate - // on a repository with no merge queue is an off switch for these two. + // trigger is accepted only by `test` and the `classify_pr` it depends on, + // so a merge-queue-only gate on a repository with no merge queue is an off + // switch for these two. expect(triggers.schedule).toBeDefined(); expect(Array.isArray(triggers.schedule)).toBe(true); expect(triggers.schedule[0].cron).toMatch(/^\S+ \S+ \S+ \S+ \S+$/); @@ -174,6 +175,107 @@ describe('platform lanes — triggers', () => { }); }); +// The post-merge push lane on `main`. +// +// `ci.yml` carried no push trigger while no merge queue was enabled, so +// nothing validated the merged tree before it landed and the only check after +// it was the ~40-minute E2E — a regression could sit in `main` for hours. The +// trigger is back, taken by `test` and the `classify_pr` it depends on. +// +// The runner-routing tests in `.github/scripts/ci-runner-routing.test.mjs` +// cannot reach any of this: they drive `pick_runner`'s shell directly with +// EVENT_NAME=push, bypassing the trigger and both job gates, so they stay +// green if either loses its push arm. What is pinned here is the YAML half — +// the same class of invariant as `keeps a nightly run to exactly the two +// lanes` above, and for the same reason: every mutation below leaves the whole +// suite green while silently turning the lane off, widening it, or pointing it +// at the wrong tree. +describe('post-merge push lane', () => { + const PUSH_JOBS = ['classify_pr', 'test']; + const stepOf = (job, name) => + (ci.jobs[job].steps ?? []).find((s) => s.name === name); + + it('is triggered on main and nowhere else', () => { + // Drop the trigger and the post-merge signal is gone with it; widen the + // branch list and every dev branch push spends a pool runner. + expect(triggers.push).toBeDefined(); + expect(triggers.push.branches).toEqual(['main']); + }); + + it('classify_pr admits push in its event allowlist', () => { + // `test` reads `needs.classify_pr.outputs.ubuntu_runner`. If classify_pr + // stops accepting push, `test` still runs but that output is empty and + // `fromJSON(... || '["ubuntu-latest"]')` silently demotes every + // post-merge run to a scarce hosted runner. + expect(condOf('classify_pr')).toContain("github.event_name == 'push'"); + }); + + it('test accepts push while still excluding the nightly', () => { + const cond = condOf('test'); + expect(cond).not.toContain("github.event_name != 'push'"); + expect(cond).toContain("github.event_name != 'schedule'"); + }); + + it('keeps a post-merge run to exactly classify_pr and test', () => { + // The mirror of the nightly assertion: a `push:` trigger fires the whole + // workflow, so every other job must exclude push outright or gate on an + // allowlist that cannot contain it. Otherwise a merge quietly becomes a + // full CI run — desktop_shell's cargo build included. + for (const [name, job] of Object.entries(ci.jobs)) { + if (PUSH_JOBS.includes(name)) continue; + const cond = String(job.if ?? ''); + const excluded = + cond.includes("github.event_name != 'push'") || + /github\.event_name == '(pull_request|merge_group|workflow_dispatch|schedule)'/.test( + cond, + ); + expect(excluded, `${name} would also run on a post-merge push`).toBe( + true, + ); + expect(cond, `${name} admits push explicitly`).not.toContain( + "github.event_name == 'push'", + ); + } + }); + + it('checks out the commit the push reported, not the branch tip', () => { + // `github.ref` resolves `refs/heads/main` at fetch time — a moving tip — + // while the check run attaches to `github.sha`. Without a push arm the + // lane can validate a tree it does not report on, and nothing detects it: + // `Verify checkout includes expected head commit` is gated to + // pull_request / merge_group, and its `merge-base --is-ancestor` check + // passes for a newer tip regardless. Order matters as much as presence — + // the arm has to sit before the `github.ref` fallback to ever be reached. + const checkout = (ci.jobs.test.steps ?? []).find((s) => + String(s.uses ?? '').startsWith('actions/checkout'), + ); + expect(checkout).toBeDefined(); + const ref = String(checkout.with?.ref ?? ''); + const arm = "github.event_name == 'push' && github.sha"; + expect(ref).toContain(arm); + expect(ref.indexOf(arm)).toBeLessThan(ref.lastIndexOf('github.ref ')); + }); + + it('does not upload coverage from a post-merge run', () => { + // The artifact's only consumer, post_coverage_comment, is gated to + // pull_request; nothing else under .github/ reads coverage-reports-*. + // Without this exclusion every merge spends pool egress and artifact + // retention on a lane with no reader. + const step = stepOf('test', 'Upload coverage reports'); + expect(step).toBeDefined(); + expect(String(step.if)).toContain("github.event_name != 'push'"); + }); + + it('scopes the concurrency group by event', () => { + // Unscoped, the group collapses onto `Qwen Code CI-refs/heads/main` and + // the post-merge run shares it with the nightly (60-minute lanes, no + // cancellation on main) and with any PR from a fork branch named + // literally `refs/heads/main` — a legal ref name whose `head_ref` makes + // the group byte-identical while its own run cancels in progress. + expect(String(ci.concurrency.group)).toContain('github.event_name'); + }); +}); + 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, diff --git a/scripts/tests/main-ci-failure-issue-workflow.test.js b/scripts/tests/main-ci-failure-issue-workflow.test.js index d7a75f5c8ca..52bbb12e6be 100644 --- a/scripts/tests/main-ci-failure-issue-workflow.test.js +++ b/scripts/tests/main-ci-failure-issue-workflow.test.js @@ -36,7 +36,10 @@ describe('main CI failure issue workflow', () => { expect(workflow).toContain( "github.event.workflow_run.head_branch == 'main'", ); - // Push covers the other two watched workflows; schedule is scoped to + // Push covers the other two watched workflows AND 'Qwen Code CI''s own + // post-merge lane on `main` — ci.yml restored that trigger, so main's + // squash commits now carry Test check-runs and a red one files an issue + // here. Schedule is scoped to // 'Qwen Code CI' — that nightly is the platform lanes' only trigger // outside a pull request, and the other watched workflows' own // nightlies must not dispatch the autofix agent through this watcher. diff --git a/scripts/tests/workflow-size.test.js b/scripts/tests/workflow-size.test.js index e0d93459848..1e4b331da59 100644 --- a/scripts/tests/workflow-size.test.js +++ b/scripts/tests/workflow-size.test.js @@ -72,11 +72,17 @@ describe('workflow file size', () => { // forget. const workflowEnv = ciWorkflow.match(/^env:[\s\S]*?\njobs:/m)?.[0]; // Anchored to the whole line: substring checks still pass a `||` → `&&` - // mutation (empty on both events — the #9904 red wall returns) and an + // mutation (empty on every event — the #9904 red wall returns) and an // appended `|| github.sha` fallback (workflow_dispatch resolves the base // to the checked-out commit, failing the ratchet open on that lane). + // `github.event.before` is the push lane's arm and is safe where + // `github.sha` is not: on a squash-merge push it is main's PREVIOUS tip, + // so the comparison is against a different tree the way a PR's base is, + // and an all-zeros `before` (branch creation, force push) still fails + // closed. Without it the push lane has no base at all and the ratchet + // fails closed on `main` while every PR stays green on leniency. expect(workflowEnv).toMatch( - /^\s*WORKFLOW_SIZE_BASE_SHA: '\$\{\{ github\.event\.pull_request\.base\.sha \|\| github\.event\.merge_group\.base_sha \}\}'$/m, + /^\s*WORKFLOW_SIZE_BASE_SHA: '\$\{\{ github\.event\.pull_request\.base\.sha \|\| github\.event\.merge_group\.base_sha \|\| github\.event\.before \}\}'$/m, ); }); }); From 8474ae22073e7f614dd2c5d22977c6e73a755c11 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 28 Aug 2026 15:53:40 +0800 Subject: [PATCH 3/4] ci: harden the push-lane guards after round-2 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five follow-up findings, all on the guards added in the previous commit rather than on the lane itself. R1-2 (fix-induced): `test`'s push admission was pinned only negatively — no `!= 'push'` — so rewriting `test.if` into allowlist form drops push while every guard stays green: the exclusivity helper skips `test` because it is an expected push job, and the routing tests inject EVENT_NAME=push straight into pick_runner's shell, bypassing the job gate entirely. On a real merge the run would conclude `success` with the lane silently gone, and the failure watcher — gated on `conclusion == 'failure'` — would file nothing. Require the gate to stay exclusion-shaped. R2-1: the push exclusivity loop was a hand-copy of the nightly one and the two had already diverged — the mirror's allowlist gained `schedule` while the original never gained `push` — so a push-only job would fail the nightly assertion with a message naming the wrong event. Replaced both with one parameterized helper, which also restores the `not.toBe('')` assertion the copy dropped. R2-2: the concurrency assertion pinned only the added `github.event_name` token. Flipping `cancel-in-progress` to `true` was measured invisible to all 1854 test:scripts tests, while consecutive merges would then cancel each other — the history this repo already recorded once, when 67 of 100 push runs were cancelled and only 25 reported. Pin all three components, the way e2e-workflow.test.js pins its own. R2-3: the group's pull_request half was still keyed on `head_ref` alone, so same-named branches across forks — and a fork's default branch is `main`, the commonest head_ref there is — share one group and cancel each other's in-progress runs. Add the source-repo term, matching security-checks.yml and serve-ab.yml. Supersede within a PR is preserved: one branch backs at most one open PR per fork. R2-4: the trigger was pinned by `branches` only; a sibling `tags:` key would widen the lane while the suite stayed green. On a tag-creation push `github.event.before` is all zeros, so WORKFLOW_SIZE_BASE_SHA is unresolvable and both size enforcers take their strict arm, while the failure watcher's `head_branch == 'main'` filter files nothing — a silent red on release day. Pin the whole trigger object. Mutation-checked: each of the five reddens exactly the intended assertion, and the extracted helper now reports the correct event for a push-only job (verified by injecting a probe job — the nightly assertion passes, the push one fails naming push). --- .github/workflows/ci.yml | 17 ++- scripts/tests/ci-platform-lanes.test.js | 142 ++++++++++++++++-------- 2 files changed, 101 insertions(+), 58 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 72ad7bb32f2..137dfbaa7a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,15 +50,14 @@ on: - 'hosted' concurrency: - # Scoped by event as well as ref, like `e2e.yml` and for the same collision - # class. Unscoped, the group collapses onto `Qwen Code CI-refs/heads/main` - # and the post-merge run shares it with (1) the nightly's 60-minute lanes, - # neither cancelling on `main`, so merges inside that window queue behind it - # and supersede each other unreported; and (2) any PR from a fork branch - # named literally `refs/heads/main` — a legal ref name whose `head_ref` makes - # the group byte-identical while that run cancels in progress, letting an - # actor with no repo access kill the signal on every merge. - group: '${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.ref }}' + # Event + source repo + ref, not ref alone. Without the event the group + # collapses onto `Qwen Code CI-refs/heads/main`, shared with the nightly + # (60-minute lanes, neither cancelling on `main`). Without the source repo, + # same-named branches across forks collide — they evaluate in the base + # repo's namespace and PR runs DO cancel in progress, so a fork whose + # default branch is `main` cancels a victim PR's run on every push. + # Supersede within a PR is kept: one branch backs one open PR per fork. + group: '${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.head.repo.full_name || github.repository }}-${{ github.head_ref || github.ref }}' cancel-in-progress: |- ${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/heads/release/') }} diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js index e8b6c0f2eec..a24d2de88e9 100644 --- a/scripts/tests/ci-platform-lanes.test.js +++ b/scripts/tests/ci-platform-lanes.test.js @@ -37,8 +37,36 @@ const failureIssue = parse( // `on:` parses as the boolean true in YAML 1.1. const triggers = ci[true] ?? ci['on']; const LANES = ['test_macos', 'test_windows']; +const PUSH_JOBS = ['classify_pr', 'test']; const condOf = (job) => String(ci.jobs[job].if ?? ''); +// One helper for both "an run reaches exactly these jobs" invariants. +// They started as hand-copies and had already diverged: the push mirror's +// allowlist gained `schedule` while the nightly's never gained `push`, so a +// push-only job would have failed the nightly assertion with the factually +// wrong message ` would also run on the nightly schedule`, pointing the +// fixer at the wrong event. Parameterising makes that class of drift +// impossible — both call sites now share one allowlist shape. +const assertEventReachesOnly = (event, allowedJobs, allowlist) => { + const allowRe = new RegExp( + `github\\.event_name == '(${allowlist.join('|')})'`, + ); + for (const [name, job] of Object.entries(ci.jobs)) { + if (allowedJobs.includes(name)) continue; + const cond = String(job.if ?? ''); + const excluded = + cond.includes(`github.event_name != '${event}'`) || allowRe.test(cond); + expect(excluded, `${name} would also run on ${event}`).toBe(true); + // Mentioning an allowlisted event is not the same as excluding this one: + // `event == 'pull_request' || event == ''` satisfies the check + // above while still running. Require the impossibility. + expect(cond, `${name} admits ${event} explicitly`).not.toContain( + `github.event_name == '${event}'`, + ); + expect(cond, `${name} has no event gate at all`).not.toBe(''); + } +}; + describe('platform lanes — triggers', () => { it('gives the workflow a scheduled trigger', () => { // Without it the lanes have no path to `main` at all: `ci.yml`'s push @@ -152,26 +180,12 @@ describe('platform lanes — triggers', () => { // therefore either exclude `schedule` outright or gate on an event // allowlist that cannot contain it — otherwise the nightly quietly // becomes a full CI run every day. - for (const [name, job] of Object.entries(ci.jobs)) { - if (LANES.includes(name)) continue; - const cond = String(job.if ?? ''); - const excluded = - cond.includes("github.event_name != 'schedule'") || - /github\.event_name == '(pull_request|merge_group|workflow_dispatch)'/.test( - cond, - ); - expect(excluded, `${name} would also run on the nightly schedule`).toBe( - true, - ); - // Mentioning an allowlisted event is not the same as excluding this - // one: `event == 'pull_request' || event == 'schedule'` satisfies the - // check above while running nightly. Require the impossibility. - expect( - cond, - `${name} admits the schedule event explicitly`, - ).not.toContain("github.event_name == 'schedule'"); - expect(cond, `${name} has no event gate at all`).not.toBe(''); - } + assertEventReachesOnly('schedule', LANES, [ + 'pull_request', + 'merge_group', + 'workflow_dispatch', + 'push', + ]); }); }); @@ -191,15 +205,20 @@ describe('platform lanes — triggers', () => { // suite green while silently turning the lane off, widening it, or pointing it // at the wrong tree. describe('post-merge push lane', () => { - const PUSH_JOBS = ['classify_pr', 'test']; const stepOf = (job, name) => (ci.jobs[job].steps ?? []).find((s) => s.name === name); it('is triggered on main and nowhere else', () => { // Drop the trigger and the post-merge signal is gone with it; widen the // branch list and every dev branch push spends a pool runner. - expect(triggers.push).toBeDefined(); - expect(triggers.push.branches).toEqual(['main']); + // The whole object, not just `branches`: a sibling `tags:` key would + // widen the lane to every tag push while `branches` still read `['main']`. + // That also breaks the size ratchet — on a tag-creation push + // `github.event.before` is all zeros, so WORKFLOW_SIZE_BASE_SHA is + // unresolvable and both enforcers take their strict arm — and + // main-ci-failure-issue.yml's `head_branch == 'main'` filter means no + // issue is filed for tag runs: a silent red on release day. + expect(triggers.push).toEqual({ branches: ['main'] }); }); it('classify_pr admits push in its event allowlist', () => { @@ -211,31 +230,35 @@ describe('post-merge push lane', () => { }); it('test accepts push while still excluding the nightly', () => { + // Both halves are load-bearing. The exclusion checks are not enough on + // their own: rewriting `test.if` into allowlist form — `!cancelled() && + // event != 'schedule' && (event == 'pull_request' || event == + // 'merge_group' || event == 'workflow_dispatch')` — contains no + // `!= 'push'`, so it passes them, is skipped by the exclusivity helper + // (test is in PUSH_JOBS), and leaves the routing tests green because they + // inject EVENT_NAME=push into pick_runner's shell directly. On a real + // push, classify_pr would run, test would skip, the run would conclude + // `success`, and the failure watcher — gated on `conclusion == 'failure'` + // — would file nothing: the lane gone with zero red anywhere. Requiring + // the gate to stay exclusion-shaped forecloses that. const cond = condOf('test'); expect(cond).not.toContain("github.event_name != 'push'"); + expect(cond).not.toContain('github.event_name =='); expect(cond).toContain("github.event_name != 'schedule'"); }); it('keeps a post-merge run to exactly classify_pr and test', () => { - // The mirror of the nightly assertion: a `push:` trigger fires the whole - // workflow, so every other job must exclude push outright or gate on an - // allowlist that cannot contain it. Otherwise a merge quietly becomes a - // full CI run — desktop_shell's cargo build included. - for (const [name, job] of Object.entries(ci.jobs)) { - if (PUSH_JOBS.includes(name)) continue; - const cond = String(job.if ?? ''); - const excluded = - cond.includes("github.event_name != 'push'") || - /github\.event_name == '(pull_request|merge_group|workflow_dispatch|schedule)'/.test( - cond, - ); - expect(excluded, `${name} would also run on a post-merge push`).toBe( - true, - ); - expect(cond, `${name} admits push explicitly`).not.toContain( - "github.event_name == 'push'", - ); - } + // The mirror of the nightly assertion, through the same helper: a `push:` + // trigger fires the whole workflow, so every other job must exclude push + // outright or gate on an allowlist that cannot contain it. Otherwise a + // merge quietly becomes a full CI run — desktop_shell's cargo build + // included. + assertEventReachesOnly('push', PUSH_JOBS, [ + 'pull_request', + 'merge_group', + 'workflow_dispatch', + 'schedule', + ]); }); it('checks out the commit the push reported, not the branch tip', () => { @@ -266,13 +289,34 @@ describe('post-merge push lane', () => { expect(String(step.if)).toContain("github.event_name != 'push'"); }); - it('scopes the concurrency group by event', () => { - // Unscoped, the group collapses onto `Qwen Code CI-refs/heads/main` and - // the post-merge run shares it with the nightly (60-minute lanes, no - // cancellation on main) and with any PR from a fork branch named - // literally `refs/heads/main` — a legal ref name whose `head_ref` makes - // the group byte-identical while its own run cancels in progress. - expect(String(ci.concurrency.group)).toContain('github.event_name'); + it('scopes the concurrency group and keeps main uncancellable', () => { + // All three components, the way e2e-workflow.test.js pins its own. + // + // Event: unscoped, the group collapses onto `Qwen Code CI-refs/heads/main` + // and the post-merge run shares it with the nightly (60-minute lanes, no + // cancellation on main) and with any PR from a fork branch named literally + // `refs/heads/main` — a legal ref name. + // + // Repo + ref: `head_ref` alone is not unique. Two PRs with the same head + // branch name — including from different forks, which evaluate groups in + // the base repo's namespace — share a group, and PR runs DO cancel in + // progress, so they cancel each other. A fork's default branch is `main`, + // the most common head_ref there is. Dropping the ref half entirely would + // collapse every PR into one group and leave this suite green. + // + // cancel-in-progress: flipping it to `true` was measured to be invisible + // to all 1854 test:scripts tests, while consecutive merges would cancel + // each other — the history this repo already recorded once, when 67 of + // 100 push runs were cancelled and only 25 ever reported. + const group = String(ci.concurrency.group); + expect(group).toContain('github.event_name'); + expect(group).toContain( + 'github.event.pull_request.head.repo.full_name || github.repository', + ); + expect(group).toContain('github.head_ref || github.ref'); + expect(String(ci.concurrency['cancel-in-progress'])).toContain( + "github.ref != 'refs/heads/main'", + ); }); }); From 11d4c518c085a04e8c5eb32044dd279bd9db2cf5 Mon Sep 17 00:00:00 2001 From: wenshao Date: Fri, 28 Aug 2026 20:49:18 +0800 Subject: [PATCH 4/4] ci: decide push-lane gates by evaluation, pin them by literal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 raised seven findings, all Suggestion, and they are all instances of one root cause the review itself named: the guards asserted on substrings, and a substring cannot see a connective. Rather than adding one more substring per mutation, this replaces the two mechanisms behind them. Gate reachability is now decided by EVALUATING each job's `if` for the event instead of looking for tokens in it — the substitute-then-evaluate technique `.github/scripts/ci-runner-routing.test.mjs` already uses on `runs-on`. That closes R2-1's connective blindness (an `&&`→`||` flip inside an excluded job; an allowlisted equality paired with an event-neutral disjunct under `||`) without the recursive boolean parser the suggestion proposed: a hand-written expression parser is a new fail-open surface of its own, whereas evaluation computes the answer rather than pattern-matching the question. Any context term the substitutions do not model now throws instead of reading as excluded. The helper also asserts the positive direction — that the jobs which must run on an event do — which the token version never did, and which is precisely the "a lane nobody is told about" failure this file's header says it exists to prevent. Every security-relevant gate is pinned as a whole literal rather than by substring: `classify_pr.if`, `test.if`, the checkout `ref:` expression, the coverage step's `if`, and both halves of the concurrency block. One shape closes R3-3, R1-2, R3-4 and R2-2 together — connector flips, added exclusions, deleted clauses and reordering all change the literal. R3-2: the trigger comment claimed the test reporter is restricted to pull_request / merge_group. It carries no event gate at all; it is excluded by a same-repo clause that also keeps fork PRs out, so normalising it to the documented allowlist would run dorny/test-reporter on fork events that lack `checks: write`. Reworded per step, with that trap named. R3-1: bump `.size-baseline` for ci.yml to 85336. The previous two rounds trimmed comments to stay under the allowance, which left 62 bytes of headroom and charged the next PR for growth this one introduced. The baseline file's own header says growth is deliberate and asks for the number to be bumped with a reason — the growth here is the per-finding rationale the review asked for, so it is recorded rather than squeezed out. Mutation matrix across all three rounds: 22 mutations, none escaping — the seven this round probed as surviving now redden, and the twelve from rounds 1 and 2 still do. --- .github/workflows/.size-baseline | 2 +- .github/workflows/ci.yml | 12 ++- scripts/tests/ci-platform-lanes.test.js | 133 +++++++++++++++--------- 3 files changed, 91 insertions(+), 56 deletions(-) diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index 76569a1ca52..b8d4657dadb 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -18,7 +18,7 @@ 9256 build-and-publish-image.yml 49610 cd-cua-driver.yml 2076 cd-mobile-mcp.yml -80980 ci.yml +85336 ci.yml 1482 codeql.yml 9389 comment-attachment-guard.yml 31677 desktop-release.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 137dfbaa7a4..22bdc6a7124 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -198,10 +198,14 @@ jobs: # Accepts push on `main`: this is the only post-merge check faster than the # ~40-minute E2E, and with no merge queue enabled nothing validates the # merged tree before it lands either. The expensive and PR-shaped steps - # stay off it on their own gates — the no-AK integration gate, the - # checkout-head verifier and the test reporter are each already restricted - # to pull_request / merge_group — so a push run is lint, static analysis - # and unit tests. + # stay off it on their own gates, each by its own mechanism — the no-AK + # integration gate and the checkout-head verifier are restricted to + # pull_request / merge_group, while the test reporter carries no event + # gate at all: its same-repo clause (head.repo.full_name == + # github.repository) is empty on push. Do not "normalise" that clause into + # the event allowlist — it also keeps fork PRs out, and dorny/test-reporter + # needs `checks: write`, which fork-event runs do not have. So a push run + # is lint, static analysis and unit tests. if: "${{ !cancelled() && github.event_name != 'schedule' }}" runs-on: '${{ fromJSON(needs.classify_pr.outputs.ubuntu_runner || ''["ubuntu-latest"]'') }}' timeout-minutes: 60 diff --git a/scripts/tests/ci-platform-lanes.test.js b/scripts/tests/ci-platform-lanes.test.js index a24d2de88e9..5f8468f98cc 100644 --- a/scripts/tests/ci-platform-lanes.test.js +++ b/scripts/tests/ci-platform-lanes.test.js @@ -41,29 +41,65 @@ const PUSH_JOBS = ['classify_pr', 'test']; const condOf = (job) => String(ci.jobs[job].if ?? ''); // One helper for both "an run reaches exactly these jobs" invariants. -// They started as hand-copies and had already diverged: the push mirror's -// allowlist gained `schedule` while the nightly's never gained `push`, so a -// push-only job would have failed the nightly assertion with the factually -// wrong message ` would also run on the nightly schedule`, pointing the -// fixer at the wrong event. Parameterising makes that class of drift -// impossible — both call sites now share one allowlist shape. -const assertEventReachesOnly = (event, allowedJobs, allowlist) => { - const allowRe = new RegExp( - `github\\.event_name == '(${allowlist.join('|')})'`, - ); +// +// It decides by EVALUATING each gate for the event, not by looking for tokens +// in it — the same substitute-then-evaluate technique +// `.github/scripts/ci-runner-routing.test.mjs` uses on `runs-on`. Token +// probes are connective-blind: an `&&`→`||` flip inside an excluded job, or an +// allowlisted equality paired with an event-neutral disjunct under `||`, +// changes which jobs run while every substring stays exactly where it was. +// Evaluation sees the change because it computes the answer instead of +// pattern-matching the question, and it needs no expression parser of our own. +// +// Any context term the substitutions do not model throws rather than reading +// as false, so a gate that grows a new input is re-read here instead of +// silently degrading this guard. +const gateRunsOn = (cond, event) => { + if (String(cond ?? '').trim() === '') return true; + let e = String(cond) + .replace(/\$\{\{/g, ' ') + .replace(/\}\}/g, ' ') + .replace(/\s+/g, ' ') + .trim(); + const subs = [ + [/!cancelled\(\)/g, 'true'], + [/cancelled\(\)/g, 'false'], + [/always\(\)/g, 'true'], + [/github\.event_name/g, `"${event}"`], + [/needs\.classify_pr\.outputs\.skip_ci/g, '"false"'], + [/needs\.test\.outputs\.ci_profile/g, '"full"'], + [ + /github\.event\.pull_request\.head\.repo\.full_name/g, + event === 'pull_request' ? '"QwenLM/qwen-code"' : '""', + ], + [/github\.repository/g, '"QwenLM/qwen-code"'], + ]; + for (const [re, v] of subs) e = e.replace(re, v); + e = e + .replace(/!==/g, '@NE@') + .replace(/===/g, '@EQ@') + .replace(/!=/g, '@NE@') + .replace(/==/g, '@EQ@') + .replace(/@NE@/g, '!==') + .replace(/@EQ@/g, '==='); + if (/github\.|needs\.|steps\.|vars\.|inputs\./.test(e)) { + throw new Error(`gate carries a term this guard does not model: ${e}`); + } + return Boolean(new Function(`return (${e});`)()); +}; + +// Asserts both directions: the jobs that must run on `event` do, and every +// other job does not. The positive half matters as much — a gate that stops +// admitting its own event turns the lane off with nothing else going red. +const assertEventReachesOnly = (event, allowedJobs) => { for (const [name, job] of Object.entries(ci.jobs)) { - if (allowedJobs.includes(name)) continue; - const cond = String(job.if ?? ''); - const excluded = - cond.includes(`github.event_name != '${event}'`) || allowRe.test(cond); - expect(excluded, `${name} would also run on ${event}`).toBe(true); - // Mentioning an allowlisted event is not the same as excluding this one: - // `event == 'pull_request' || event == ''` satisfies the check - // above while still running. Require the impossibility. - expect(cond, `${name} admits ${event} explicitly`).not.toContain( - `github.event_name == '${event}'`, - ); - expect(cond, `${name} has no event gate at all`).not.toBe(''); + const reached = gateRunsOn(job.if, event); + expect( + reached, + allowedJobs.includes(name) + ? `${name} no longer runs on ${event}` + : `${name} would also run on ${event}`, + ).toBe(allowedJobs.includes(name)); } }; @@ -180,12 +216,7 @@ describe('platform lanes — triggers', () => { // therefore either exclude `schedule` outright or gate on an event // allowlist that cannot contain it — otherwise the nightly quietly // becomes a full CI run every day. - assertEventReachesOnly('schedule', LANES, [ - 'pull_request', - 'merge_group', - 'workflow_dispatch', - 'push', - ]); + assertEventReachesOnly('schedule', LANES); }); }); @@ -226,7 +257,15 @@ describe('post-merge push lane', () => { // stops accepting push, `test` still runs but that output is empty and // `fromJSON(... || '["ubuntu-latest"]')` silently demotes every // post-merge run to a scarce hosted runner. - expect(condOf('classify_pr')).toContain("github.event_name == 'push'"); + // Whole-literal, not a substring: `||`→`&&` anywhere in this allowlist + // keeps every token in place while skipping classify_pr on a push. `test` + // would still run (its own gate is `!cancelled()`-shaped), but with + // `ubuntu_runner` empty it falls back through + // `fromJSON(... || '["ubuntu-latest"]')` — every post-merge run silently + // demoted to a scarce hosted runner, and `skip_ci`/`ci_profile` empty. + expect(condOf('classify_pr')).toBe( + "${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'workflow_dispatch' || github.event_name == 'push' }}", + ); }); it('test accepts push while still excluding the nightly', () => { @@ -241,10 +280,9 @@ describe('post-merge push lane', () => { // `success`, and the failure watcher — gated on `conclusion == 'failure'` // — would file nothing: the lane gone with zero red anywhere. Requiring // the gate to stay exclusion-shaped forecloses that. - const cond = condOf('test'); - expect(cond).not.toContain("github.event_name != 'push'"); - expect(cond).not.toContain('github.event_name =='); - expect(cond).toContain("github.event_name != 'schedule'"); + expect(condOf('test')).toBe( + "${{ !cancelled() && github.event_name != 'schedule' }}", + ); }); it('keeps a post-merge run to exactly classify_pr and test', () => { @@ -253,12 +291,7 @@ describe('post-merge push lane', () => { // outright or gate on an allowlist that cannot contain it. Otherwise a // merge quietly becomes a full CI run — desktop_shell's cargo build // included. - assertEventReachesOnly('push', PUSH_JOBS, [ - 'pull_request', - 'merge_group', - 'workflow_dispatch', - 'schedule', - ]); + assertEventReachesOnly('push', PUSH_JOBS); }); it('checks out the commit the push reported, not the branch tip', () => { @@ -273,10 +306,9 @@ describe('post-merge push lane', () => { String(s.uses ?? '').startsWith('actions/checkout'), ); expect(checkout).toBeDefined(); - const ref = String(checkout.with?.ref ?? ''); - const arm = "github.event_name == 'push' && github.sha"; - expect(ref).toContain(arm); - expect(ref.indexOf(arm)).toBeLessThan(ref.lastIndexOf('github.ref ')); + expect(String(checkout.with?.ref ?? '')).toBe( + "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || (github.event_name == 'push' && github.sha) || github.ref }}", + ); }); it('does not upload coverage from a post-merge run', () => { @@ -286,7 +318,9 @@ describe('post-merge push lane', () => { // retention on a lane with no reader. const step = stepOf('test', 'Upload coverage reports'); expect(step).toBeDefined(); - expect(String(step.if)).toContain("github.event_name != 'push'"); + expect(String(step.if)).toBe( + "${{ always() && needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' && github.event_name != 'push' }}", + ); }); it('scopes the concurrency group and keeps main uncancellable', () => { @@ -308,14 +342,11 @@ describe('post-merge push lane', () => { // to all 1854 test:scripts tests, while consecutive merges would cancel // each other — the history this repo already recorded once, when 67 of // 100 push runs were cancelled and only 25 ever reported. - const group = String(ci.concurrency.group); - expect(group).toContain('github.event_name'); - expect(group).toContain( - 'github.event.pull_request.head.repo.full_name || github.repository', + expect(String(ci.concurrency.group)).toBe( + '${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.head.repo.full_name || github.repository }}-${{ github.head_ref || github.ref }}', ); - expect(group).toContain('github.head_ref || github.ref'); - expect(String(ci.concurrency['cancel-in-progress'])).toContain( - "github.ref != 'refs/heads/main'", + expect(String(ci.concurrency['cancel-in-progress']).trim()).toBe( + "${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/heads/release/') }}", ); }); });