From 35eeeb3969eca4c64413c266001993f7b4b5a9c2 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Mon, 24 Aug 2026 19:38:47 +0800 Subject: [PATCH 1/5] fix(ci): scope workflow-size ratchet to the PR that grew the file (#9904) A workflow that grew on main without a same-PR baseline bump red-walled every other open PR's CI (recurred twice in two weeks). Given the PR's base commit, the growth branch now downgrades to a warning when the PR's copy of the file is byte-identical to the base; a PR that actually changes the file still fails closed, as does any unresolvable base. Co-authored-by: Qwen-Coder --- .github/scripts/check-workflow-size.sh | 31 ++++- .github/workflows/.size-baseline | 2 +- .github/workflows/ci.yml | 5 + .github/workflows/qwen-autofix.md | 10 ++ scripts/tests/workflow-size.test.js | 169 ++++++++++++++++++++++++- 5 files changed, 211 insertions(+), 6 deletions(-) diff --git a/.github/scripts/check-workflow-size.sh b/.github/scripts/check-workflow-size.sh index 5ce28db0769..83f78f2e690 100755 --- a/.github/scripts/check-workflow-size.sh +++ b/.github/scripts/check-workflow-size.sh @@ -31,6 +31,29 @@ GROWTH_ALLOWANCE="${WORKFLOW_SIZE_GROWTH_ALLOWANCE:-4096}" # the slack for the next unreviewed 25 KB. SLACK_BYTES=20000 +# The ratchet compares the worktree against a checked-in baseline, so a +# workflow that grew on main without the same-PR baseline bump leaves every +# OTHER open PR failing a gate on a file it never touched (red-walled the +# queue twice in two weeks: #9747, #9822). When the caller passes the PR's +# base commit in WORKFLOW_SIZE_BASE_SHA, the growth branch below hard-fails +# only if the PR actually changed the file; a byte-identical copy means the +# staleness is main-side drift and earns a warning instead. An unresolvable +# base (local run, fetch failure) falls back to the strict failure — the +# ratchet fails closed, never open. One residual window stays by design: if +# main edits the same workflow again after the PR branched, the comparison +# against the new base sees the PR's older copy as different and fails closed +# until that PR rebases — self-healing, and still fail-closed, so it is left +# alone rather than wiring the PR's changed-files list into a gate that today +# needs no API call. +BASE_SHA="${WORKFLOW_SIZE_BASE_SHA:-}" +file_matches_base() { + local file="$1" + [[ -n "${BASE_SHA}" ]] || return 1 + git rev-parse --verify --quiet "${BASE_SHA}^{commit}" >/dev/null || + git fetch --depth=1 --quiet origin "${BASE_SHA}" || return 1 + git show "${BASE_SHA}:${file}" 2>/dev/null | cmp -s - "${file}" +} + status=0 declare -A baseline=() if [[ -r "${BASELINE_FILE}" ]]; then @@ -75,8 +98,12 @@ for file in .github/workflows/*.yml .github/workflows/*.yaml; do echo "::error file=${file}::${file} has no entry in ${BASELINE_FILE}. Add '${size} ${file##*/}' so its growth is tracked." status=1 elif ((size > base + GROWTH_ALLOWANCE)); then - echo "::error file=${file}::${file} grew to ${size} bytes, $((size - base)) over its recorded ${base} (allowance ${GROWTH_ALLOWANCE}). Move prose into a sibling .md and long steps into .github/scripts/ — or, if the growth is real, update ${BASELINE_FILE} in this PR and say why." - status=1 + if file_matches_base "${file}"; then + echo "::warning file=${file}::${file} is ${size} bytes, $((size - base)) over its recorded ${base}, but the file is unchanged from this PR's base — the baseline went stale on main, not in this PR. Bump ${BASELINE_FILE} on main (a one-line PR saying why); unrelated PRs are not blocked." + else + echo "::error file=${file}::${file} grew to ${size} bytes, $((size - base)) over its recorded ${base} (allowance ${GROWTH_ALLOWANCE}). Move prose into a sibling .md and long steps into .github/scripts/ — or, if the growth is real, update ${BASELINE_FILE} in this PR and say why." + status=1 + fi elif ((size + SLACK_BYTES < base)); then echo "::warning file=${file}::${file} is ${size} bytes, $((base - size)) under its recorded ${base} — lower the entry in ${BASELINE_FILE} so the slack is not banked." fi diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index f920373a836..303e6a5def6 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -18,7 +18,7 @@ 4638 build-and-publish-image.yml 49610 cd-cua-driver.yml 2076 cd-mobile-mcp.yml -69782 ci.yml +74203 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 2a7e4026e9d..1fcb6b0f01b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -317,6 +317,11 @@ jobs: # loop for a day on 2026-08-19. Costs one `wc -c` per workflow file. - name: 'Check workflow file size' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" + env: + # Lets the growth ratchet tell "this PR grew the workflow" apart + # from "the baseline went stale on main" so one author's missing + # baseline bump cannot red-wall every unrelated open PR (#9904). + WORKFLOW_SIZE_BASE_SHA: "${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}" run: '.github/scripts/check-workflow-size.sh' - name: 'Docs-only CI' diff --git a/.github/workflows/qwen-autofix.md b/.github/workflows/qwen-autofix.md index 9846d3ee2bc..3e014680dd6 100644 --- a/.github/workflows/qwen-autofix.md +++ b/.github/workflows/qwen-autofix.md @@ -40,6 +40,16 @@ fails until the number is updated in the same PR. Growing a file is still allowed — the ratchet only insists the growth be visible in review rather than discovered at the wall. +The ratchet's blast radius is scoped to the PR that earned it (#9904). The +comparison above is worktree-vs-checked-in-baseline, so a file that grew on +main without the same-PR bump would otherwise fail every unrelated open PR on +a file it never touched — that red-walled the queue twice in two weeks +(#9747, #9822). Given the PR's base commit (`WORKFLOW_SIZE_BASE_SHA`, wired +in `ci.yml`), the gate hard-fails only when the PR's copy of the file differs +from the base; a byte-identical copy means the staleness is main-side drift +and earns a warning pointing at the one-line baseline-bump PR instead. An +unresolvable base keeps the strict failure — the gate fails closed. + ### Steps that moved out, not just their prose `review-address` · `Push and report` was 626 lines of inline shell — ~41 KB, diff --git a/scripts/tests/workflow-size.test.js b/scripts/tests/workflow-size.test.js index c14256a475b..35586f11523 100644 --- a/scripts/tests/workflow-size.test.js +++ b/scripts/tests/workflow-size.test.js @@ -59,6 +59,12 @@ describe('workflow file size', () => { 'if: "${{ needs.classify_pr.outputs.skip_ci != \'true\' }}"', ); expect(step?.[0]).not.toContain('ci_profile'); + // The ratchet's PR-scope fix (#9904) hangs off this env: without it the + // gate has no base to compare against and silently degrades to the + // pre-fix red-wall, so pin both event arms the script relies on. + expect(step?.[0]).toContain('WORKFLOW_SIZE_BASE_SHA'); + expect(step?.[0]).toContain('github.event.pull_request.base.sha'); + expect(step?.[0]).toContain('github.event.merge_group.base_sha'); }); }); @@ -137,8 +143,13 @@ describe('workflow size growth ratchet', () => { const bashSupportsAssocArrays = spawnSync('bash', ['-c', 'declare -A t=()'], { stdio: 'ignore' }).status === 0; +// The stale-baseline fixtures commit their base with git; a runner without +// git cannot build them. +const gitAvailable = + spawnSync('git', ['--version'], { stdio: 'ignore' }).status === 0; +const canRunGate = bashSupportsAssocArrays && gitAvailable; -describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( +describe.skipIf(process.platform === 'win32' || !canRunGate)( 'check-workflow-size.sh execution', () => { // The block above re-implements the gate's arithmetic in JS; only running @@ -150,7 +161,7 @@ describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( 'scripts', 'check-workflow-size.sh', ); - const runGate = ({ files, baseline }) => { + const runGate = ({ files, baseline, commitBase, dirtyFiles, baseSha }) => { const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-')); try { const fixtureDir = join(dir, WORKFLOW_DIR); @@ -161,7 +172,46 @@ describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( if (baseline !== undefined) { writeFileSync(join(fixtureDir, '.size-baseline'), baseline); } - return spawnSync('bash', [gatePath], { cwd: dir, encoding: 'utf8' }); + const env = { ...process.env }; + // Keep fixtures hermetic: the gate reads three WORKFLOW_SIZE_* knobs, + // and any of them leaking in from the developer's shell must not + // change what the strict-path fixtures assert. + delete env.WORKFLOW_SIZE_BASE_SHA; + delete env.WORKFLOW_SIZE_GATE_BYTES; + delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; + if (commitBase) { + // Stand in for the PR's base commit: the caller may then dirty + // files to simulate what the PR itself changed on top. Point git at + // an empty global config and skip the system one — a developer's + // global commit.gpgsign or hooksPath would otherwise break `git + // commit` silently and flip the warning fixture to the strict path. + const gitconfigPath = join(dir, 'fixture-gitconfig'); + writeFileSync(gitconfigPath, ''); + Object.assign(env, { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: gitconfigPath, + }); + const git = (args) => + spawnSync('git', args, { cwd: dir, encoding: 'utf8', env }); + expect(git(['init', '--quiet']).status, 'git init failed').toBe(0); + git(['config', 'user.email', 'gate-test@example.com']); + git(['config', 'user.name', 'gate-test']); + git(['add', '.']); + expect( + git(['commit', '--quiet', '-m', 'base']).status, + 'git commit failed', + ).toBe(0); + env.WORKFLOW_SIZE_BASE_SHA = + baseSha ?? git(['rev-parse', 'HEAD']).stdout.trim(); + } + for (const [name, bytes] of Object.entries(dirtyFiles ?? {})) { + writeFileSync(join(fixtureDir, name), 'a'.repeat(bytes)); + } + return spawnSync('bash', [gatePath], { + cwd: dir, + encoding: 'utf8', + env, + }); } finally { rmSync(dir, { recursive: true, force: true }); } @@ -212,6 +262,119 @@ describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( expect(result.stdout).toContain('grew to 5000 bytes'); }); + // #9904: a workflow that grew on main without the same-PR baseline bump + // used to red-wall every OTHER open PR. A PR whose copy of the file is + // byte-identical to its base did not cause the drift and must only see a + // warning; the hard failure belongs to the PR that changes the file. + it('warns instead of failing when the PR did not touch the file', () => { + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('::warning'); + expect(result.stdout).toContain('the baseline went stale on main'); + expect(result.stdout).not.toContain('::error'); + }); + + it('still fails when the PR changed the file past the allowance', () => { + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + dirtyFiles: { 'small.yml': 5001 }, + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('grew to 5001 bytes'); + }); + + it('fails closed when the base commit cannot be resolved', () => { + // A base sha that is neither present nor fetchable must keep the + // strict failure — downgrading on an unverifiable base would fail the + // ratchet open. + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + baseSha: '0'.repeat(40), + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('grew to 5000 bytes'); + }); + + it('fetches the base commit when it is not local (CI shallow-clone path)', () => { + // The production path: ci.yml checks out at fetch-depth 1, so the PR's + // base commit is never present locally and the gate must reach it via + // `git fetch --depth=1 origin `. Re-implementing the fixture here + // (rather than reusing runGate, which commits into the same repo) so + // the base commit genuinely has to be fetched. Removing the fetch line + // from the script must turn this test red. + const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-fetch-')); + try { + const env = { ...process.env }; + delete env.WORKFLOW_SIZE_BASE_SHA; + delete env.WORKFLOW_SIZE_GATE_BYTES; + delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; + const gitconfigPath = join(dir, 'fixture-gitconfig'); + writeFileSync(gitconfigPath, ''); + Object.assign(env, { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: gitconfigPath, + }); + const bare = join(dir, 'origin.git'); + const seed = join(dir, 'seed'); + const gateCwd = join(dir, 'checkout'); + const git = (args, cwd) => { + const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + mkdirSync(seed, { recursive: true }); + git(['init', '--quiet', '--bare', bare], dir); + // The bare repo's HEAD defaults to refs/heads/master; point it at the + // branch the seed pushes so the clone checks files out at all. + git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); + git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); + // Seed: a base commit with the grown workflow + a stale baseline, + // then a second commit that changes only an unrelated file, so the + // base sits behind the tip and a depth-1 clone does not contain it. + git(['init', '--quiet'], seed); + git(['config', 'user.email', 'gate-test@example.com'], seed); + git(['config', 'user.name', 'gate-test'], seed); + const seedWorkflows = join(seed, WORKFLOW_DIR); + mkdirSync(seedWorkflows, { recursive: true }); + writeFileSync(join(seedWorkflows, 'small.yml'), 'a'.repeat(5000)); + writeFileSync(join(seedWorkflows, '.size-baseline'), '100 small.yml\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'base'], seed); + const baseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); + writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'unrelated tip'], seed); + git(['remote', 'add', 'origin', bare], seed); + git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); + // Depth-1 clone holds only the tip; the base commit needs a fetch. + // The file:// URL matters: a plain local path ignores --depth and + // copies full history, which would hide the fetch the gate must do. + git( + ['clone', '--quiet', '--depth', '1', `file://${bare}`, gateCwd], + dir, + ); + env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const result = spawnSync('bash', [gatePath], { + cwd: gateCwd, + encoding: 'utf8', + env, + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('::warning'); + expect(result.stdout).toContain('the baseline went stale on main'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('fails a workflow with no baseline entry', () => { const result = runGate({ files: { 'small.yml': 100 }, From 9dabee0475dc59fa254bc41ee02cf8b280378fb1 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Tue, 25 Aug 2026 00:12:21 +0800 Subject: [PATCH 2/5] fix(ci): single-quote the base-sha env value for yamllint The repo's yamllint config requires single-quoted strings; the value contains no single quotes, so the double-quoted form failed the quoted-strings rule in CI. Co-authored-by: Qwen-Coder --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1fcb6b0f01b..478b86ce849 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -321,7 +321,7 @@ jobs: # Lets the growth ratchet tell "this PR grew the workflow" apart # from "the baseline went stale on main" so one author's missing # baseline bump cannot red-wall every unrelated open PR (#9904). - WORKFLOW_SIZE_BASE_SHA: "${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}" + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' run: '.github/scripts/check-workflow-size.sh' - name: 'Docs-only CI' From c8551785b5fe7fb4a366d20564e61d55f609c6d5 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Mon, 24 Aug 2026 16:39:07 +0000 Subject: [PATCH 3/5] fix(ci): align workflow-size ratchet mirror with the PR-scope downgrade (#9931) Address review round 1: - Single-quote the WORKFLOW_SIZE_BASE_SHA value; the double quotes violated yamllint's quoted-strings rule and prettier's singleQuote, hard-failing this PR's own Test lane. - Wire the base SHA into every `npm run test:ci` step and grant the vitest mirror the same stale-baseline leniency as the shell gate; the mirror is the only enforcer on the merge-queue lanes, and without this the red wall just moves from the gate into test:ci. - Apply the downgrade to the missing-entry arm too (same red-wall shape as a stale size). - Return a distinct status for an unresolvable base and add a diagnostic, so a transient fetch failure is not annotated like genuine PR growth. - Qualify the success banner when stale-baseline warnings fired. - Deduplicate the hermetic-env construction, pin the brand-new-file class, and split the git fixtures into their own describe so the strict-path tests still run on git-less runners. --- .github/scripts/check-workflow-size.sh | 64 +++- .github/workflows/ci.yml | 9 +- scripts/tests/workflow-size.test.js | 471 ++++++++++++++++++------- 3 files changed, 390 insertions(+), 154 deletions(-) diff --git a/.github/scripts/check-workflow-size.sh b/.github/scripts/check-workflow-size.sh index 83f78f2e690..9c70304d37a 100755 --- a/.github/scripts/check-workflow-size.sh +++ b/.github/scripts/check-workflow-size.sh @@ -35,26 +35,38 @@ SLACK_BYTES=20000 # workflow that grew on main without the same-PR baseline bump leaves every # OTHER open PR failing a gate on a file it never touched (red-walled the # queue twice in two weeks: #9747, #9822). When the caller passes the PR's -# base commit in WORKFLOW_SIZE_BASE_SHA, the growth branch below hard-fails -# only if the PR actually changed the file; a byte-identical copy means the -# staleness is main-side drift and earns a warning instead. An unresolvable -# base (local run, fetch failure) falls back to the strict failure — the -# ratchet fails closed, never open. One residual window stays by design: if -# main edits the same workflow again after the PR branched, the comparison -# against the new base sees the PR's older copy as different and fails closed -# until that PR rebases — self-healing, and still fail-closed, so it is left -# alone rather than wiring the PR's changed-files list into a gate that today -# needs no API call. +# base commit in WORKFLOW_SIZE_BASE_SHA, the missing-entry and growth +# branches below hard-fail only if the PR actually changed the file; a +# byte-identical copy means the staleness is main-side drift and earns a +# warning instead. An unresolvable base (local run, fetch failure) falls +# back to the strict failure — the ratchet fails closed, never open. One +# residual window stays by design: if main edits the same workflow again +# after the PR branched, the comparison against the new base sees the PR's +# older copy as different and fails closed until that PR rebases — +# self-healing, and still fail-closed, so it is left alone rather than +# wiring the PR's changed-files list into a gate that today needs no API +# call. BASE_SHA="${WORKFLOW_SIZE_BASE_SHA:-}" +# Returns 0 when the worktree copy of $1 is byte-identical to the base +# commit, 1 when it differs (or no base was given), and 2 when the base +# cannot be resolved — the callers add a diagnostic on 2, because a +# transient fetch failure and genuine PR growth need opposite remedies. file_matches_base() { local file="$1" [[ -n "${BASE_SHA}" ]] || return 1 - git rev-parse --verify --quiet "${BASE_SHA}^{commit}" >/dev/null || - git fetch --depth=1 --quiet origin "${BASE_SHA}" || return 1 + if ! git rev-parse --verify --quiet "${BASE_SHA}^{commit}" >/dev/null && + ! git fetch --depth=1 --quiet origin "${BASE_SHA}"; then + return 2 + fi git show "${BASE_SHA}:${file}" 2>/dev/null | cmp -s - "${file}" } +unresolvable_base_note() { + echo "::warning::base ${BASE_SHA} could not be resolved (git fetch failed?) — failing strict; if this PR did not touch ${1}, re-run the job." +} + status=0 +warned_stale=0 declare -A baseline=() if [[ -r "${BASELINE_FILE}" ]]; then # The || clause keeps an unterminated final line, which read reports as a @@ -95,14 +107,30 @@ for file in .github/workflows/*.yml .github/workflows/*.yaml; do base="${baseline[${file##*/}]:-}" if [[ -z "${base}" ]]; then - echo "::error file=${file}::${file} has no entry in ${BASELINE_FILE}. Add '${size} ${file##*/}' so its growth is tracked." - status=1 + file_matches_base "${file}" + match=$? + if ((match == 0)); then + echo "::warning file=${file}::${file} has no entry in ${BASELINE_FILE}, but the file is unchanged from this PR's base — add '${size} ${file##*/}' on main so its growth is tracked; unrelated PRs are not blocked." + warned_stale=1 + else + echo "::error file=${file}::${file} has no entry in ${BASELINE_FILE}. Add '${size} ${file##*/}' so its growth is tracked." + status=1 + if ((match == 2)); then + unresolvable_base_note "${file}" + fi + fi elif ((size > base + GROWTH_ALLOWANCE)); then - if file_matches_base "${file}"; then + file_matches_base "${file}" + match=$? + if ((match == 0)); then echo "::warning file=${file}::${file} is ${size} bytes, $((size - base)) over its recorded ${base}, but the file is unchanged from this PR's base — the baseline went stale on main, not in this PR. Bump ${BASELINE_FILE} on main (a one-line PR saying why); unrelated PRs are not blocked." + warned_stale=1 else echo "::error file=${file}::${file} grew to ${size} bytes, $((size - base)) over its recorded ${base} (allowance ${GROWTH_ALLOWANCE}). Move prose into a sibling .md and long steps into .github/scripts/ — or, if the growth is real, update ${BASELINE_FILE} in this PR and say why." status=1 + if ((match == 2)); then + unresolvable_base_note "${file}" + fi fi elif ((size + SLACK_BYTES < base)); then echo "::warning file=${file}::${file} is ${size} bytes, $((base - size)) under its recorded ${base} — lower the entry in ${BASELINE_FILE} so the slack is not banked." @@ -110,6 +138,10 @@ for file in .github/workflows/*.yml .github/workflows/*.yaml; do done if ((status == 0)); then - echo "✅ every workflow file is under the ${GATE_BYTES}-byte gate and within ${GROWTH_ALLOWANCE} bytes of its recorded baseline" + if ((warned_stale)); then + echo "✅ every workflow file is under the ${GATE_BYTES}-byte gate (stale-baseline warnings above — update ${BASELINE_FILE} on main)" + else + echo "✅ every workflow file is under the ${GATE_BYTES}-byte gate and within ${GROWTH_ALLOWANCE} bytes of its recorded baseline" + fi fi exit "${status}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1fcb6b0f01b..2da50a395b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -321,7 +321,7 @@ jobs: # Lets the growth ratchet tell "this PR grew the workflow" apart # from "the baseline went stale on main" so one author's missing # baseline bump cannot red-wall every unrelated open PR (#9904). - WORKFLOW_SIZE_BASE_SHA: "${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}" + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' run: '.github/scripts/check-workflow-size.sh' - name: 'Docs-only CI' @@ -532,6 +532,11 @@ jobs: id: 'unit_tests' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" env: + # Same base SHA the size gate above receives: the vitest mirror of + # the ratchet applies the identical stale-baseline leniency (#9904), + # so main-side drift warns here too instead of red-walling the + # full-CI lanes the gate just passed. + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' @@ -877,6 +882,7 @@ jobs: - name: 'Run tests and generate reports' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" env: + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' @@ -1007,6 +1013,7 @@ jobs: - name: 'Run tests and generate reports' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" env: + WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' diff --git a/scripts/tests/workflow-size.test.js b/scripts/tests/workflow-size.test.js index 35586f11523..454a77af005 100644 --- a/scripts/tests/workflow-size.test.js +++ b/scripts/tests/workflow-size.test.js @@ -15,7 +15,7 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, win32 } from 'node:path'; -import { describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; // GitHub does not start runs for a workflow file over 500 KB (512,000 bytes) // and reports nothing when it stops — see .github/scripts/check-workflow-size.sh @@ -66,8 +66,58 @@ describe('workflow file size', () => { expect(step?.[0]).toContain('github.event.pull_request.base.sha'); expect(step?.[0]).toContain('github.event.merge_group.base_sha'); }); + + it('wires the base SHA into every lane that runs the vitest mirror', () => { + // The ratchet mirror below is the only enforcer on the merge-queue lanes + // that never run the bash gate; each `npm run test:ci` step must hand it + // the same base SHA the gate gets, or the #9904 stale-baseline leniency + // silently degrades to the pre-fix red wall on exactly those lanes. + const testSteps = [ + ...ciWorkflow.matchAll( + /- name: 'Run tests and generate reports'[\s\S]*?npm run test:ci/g, + ), + ]; + expect(testSteps).toHaveLength(3); + for (const step of testSteps) { + expect(step[0]).toContain('WORKFLOW_SIZE_BASE_SHA'); + expect(step[0]).toContain('github.event.pull_request.base.sha'); + expect(step[0]).toContain('github.event.merge_group.base_sha'); + } + }); }); +// The shell gate receives the PR's base SHA so a stale baseline — growth that +// landed on main without the same-PR baseline bump — warns instead of +// red-walling unrelated PRs (#9904). This mirror is the ONLY enforcer on the +// merge-queue lanes that never run the bash script, so it applies the same +// leniency: an over-allowance file that is byte-identical to the base passes. +// As in the gate, an unset or unresolvable base fails closed. +const fileMatchesBase = (file) => { + const baseSha = (process.env.WORKFLOW_SIZE_BASE_SHA ?? '').trim(); + if (!baseSha) return false; + let resolved = + spawnSync( + 'git', + ['rev-parse', '--verify', '--quiet', `${baseSha}^{commit}`], + { stdio: 'ignore' }, + ).status === 0; + if (!resolved) { + resolved = + spawnSync('git', ['fetch', '--depth=1', '--quiet', 'origin', baseSha], { + stdio: 'ignore', + }).status === 0; + } + if (!resolved) return false; + const baseCopy = spawnSync('git', [ + 'show', + `${baseSha}:${file.split(/[\\/]/).join('/')}`, + ]); + return ( + baseCopy.status === 0 && + Buffer.compare(baseCopy.stdout, readFileSync(file)) === 0 + ); +}; + describe('workflow size growth ratchet', () => { // The absolute gate is a ceiling: it only objects once a file is nearly at // the wall, so growth accrues unremarked until one PR has to pay for @@ -110,7 +160,15 @@ describe('workflow size growth ratchet', () => { it.each(workflowFiles)('%s is within its baseline allowance', (file) => { const bytes = Buffer.byteLength(readFileSync(file)); const recorded = baseline.get(workflowName(file)); - expect(bytes).toBeLessThanOrEqual(recorded + allowance); + if (bytes <= recorded + allowance) return; + // Stale-baseline leniency (#9904), mirroring the shell gate: overage on + // a file byte-identical to the PR's base is main-side drift, not this + // PR's growth. Without this the gate warns but the mirror still fails + // the run, relocating the red wall into `npm run test:ci`. + expect( + fileMatchesBase(file), + `${file} is ${bytes - recorded} bytes over its recorded ${recorded} and differs from the PR's base`, + ).toBe(true); }); it('records no file that no longer exists', () => { @@ -143,13 +201,13 @@ describe('workflow size growth ratchet', () => { const bashSupportsAssocArrays = spawnSync('bash', ['-c', 'declare -A t=()'], { stdio: 'ignore' }).status === 0; -// The stale-baseline fixtures commit their base with git; a runner without -// git cannot build them. +// The stale-baseline fixtures commit their base with git. Only the fixtures +// need it — the strict-path tests above run on a git-less runner too, so +// gate the git block separately instead of folding git into this skip. const gitAvailable = spawnSync('git', ['--version'], { stdio: 'ignore' }).status === 0; -const canRunGate = bashSupportsAssocArrays && gitAvailable; -describe.skipIf(process.platform === 'win32' || !canRunGate)( +describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( 'check-workflow-size.sh execution', () => { // The block above re-implements the gate's arithmetic in JS; only running @@ -161,6 +219,25 @@ describe.skipIf(process.platform === 'win32' || !canRunGate)( 'scripts', 'check-workflow-size.sh', ); + // The gate reads three WORKFLOW_SIZE_* knobs, and the git fixtures commit + // through the developer's git config — scrub both, because a leak from + // the surrounding shell must not change what the fixtures assert: a + // leaked WORKFLOW_SIZE_BASE_SHA flips the fail-closed fixtures to the + // warning path, a leaked WORKFLOW_SIZE_GROWTH_ALLOWANCE flips a + // one-byte-over failure green, and a global commit.gpgsign or hooksPath + // breaks `git commit` silently the same way. + const hermeticGateEnv = (dir) => { + const env = { ...process.env }; + delete env.WORKFLOW_SIZE_BASE_SHA; + delete env.WORKFLOW_SIZE_GATE_BYTES; + delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; + const gitconfigPath = join(dir, 'fixture-gitconfig'); + writeFileSync(gitconfigPath, ''); + return Object.assign(env, { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: gitconfigPath, + }); + }; const runGate = ({ files, baseline, commitBase, dirtyFiles, baseSha }) => { const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-')); try { @@ -172,25 +249,10 @@ describe.skipIf(process.platform === 'win32' || !canRunGate)( if (baseline !== undefined) { writeFileSync(join(fixtureDir, '.size-baseline'), baseline); } - const env = { ...process.env }; - // Keep fixtures hermetic: the gate reads three WORKFLOW_SIZE_* knobs, - // and any of them leaking in from the developer's shell must not - // change what the strict-path fixtures assert. - delete env.WORKFLOW_SIZE_BASE_SHA; - delete env.WORKFLOW_SIZE_GATE_BYTES; - delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; + const env = hermeticGateEnv(dir); if (commitBase) { // Stand in for the PR's base commit: the caller may then dirty - // files to simulate what the PR itself changed on top. Point git at - // an empty global config and skip the system one — a developer's - // global commit.gpgsign or hooksPath would otherwise break `git - // commit` silently and flip the warning fixture to the strict path. - const gitconfigPath = join(dir, 'fixture-gitconfig'); - writeFileSync(gitconfigPath, ''); - Object.assign(env, { - GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: gitconfigPath, - }); + // files to simulate what the PR itself changed on top. const git = (args) => spawnSync('git', args, { cwd: dir, encoding: 'utf8', env }); expect(git(['init', '--quiet']).status, 'git init failed').toBe(0); @@ -223,7 +285,12 @@ describe.skipIf(process.platform === 'win32' || !canRunGate)( baseline: '100 small.yml\n', }); expect(result.status).toBe(0); - expect(result.stdout).toContain('✅'); + // The clean banner keeps its allowance claim; only a run that emitted a + // stale-baseline warning qualifies it — a ✅ that contradicts a warning + // in the same log is how the #9904 drift used to read. + expect(result.stdout).toContain( + 'within 4096 bytes of its recorded baseline', + ); }); it('passes a workflow grown within its allowance', () => { @@ -262,119 +329,6 @@ describe.skipIf(process.platform === 'win32' || !canRunGate)( expect(result.stdout).toContain('grew to 5000 bytes'); }); - // #9904: a workflow that grew on main without the same-PR baseline bump - // used to red-wall every OTHER open PR. A PR whose copy of the file is - // byte-identical to its base did not cause the drift and must only see a - // warning; the hard failure belongs to the PR that changes the file. - it('warns instead of failing when the PR did not touch the file', () => { - const result = runGate({ - files: { 'small.yml': 5000 }, - baseline: '100 small.yml\n', - commitBase: true, - }); - expect(result.status).toBe(0); - expect(result.stdout).toContain('::warning'); - expect(result.stdout).toContain('the baseline went stale on main'); - expect(result.stdout).not.toContain('::error'); - }); - - it('still fails when the PR changed the file past the allowance', () => { - const result = runGate({ - files: { 'small.yml': 5000 }, - baseline: '100 small.yml\n', - commitBase: true, - dirtyFiles: { 'small.yml': 5001 }, - }); - expect(result.status).toBe(1); - expect(result.stdout).toContain('grew to 5001 bytes'); - }); - - it('fails closed when the base commit cannot be resolved', () => { - // A base sha that is neither present nor fetchable must keep the - // strict failure — downgrading on an unverifiable base would fail the - // ratchet open. - const result = runGate({ - files: { 'small.yml': 5000 }, - baseline: '100 small.yml\n', - commitBase: true, - baseSha: '0'.repeat(40), - }); - expect(result.status).toBe(1); - expect(result.stdout).toContain('grew to 5000 bytes'); - }); - - it('fetches the base commit when it is not local (CI shallow-clone path)', () => { - // The production path: ci.yml checks out at fetch-depth 1, so the PR's - // base commit is never present locally and the gate must reach it via - // `git fetch --depth=1 origin `. Re-implementing the fixture here - // (rather than reusing runGate, which commits into the same repo) so - // the base commit genuinely has to be fetched. Removing the fetch line - // from the script must turn this test red. - const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-fetch-')); - try { - const env = { ...process.env }; - delete env.WORKFLOW_SIZE_BASE_SHA; - delete env.WORKFLOW_SIZE_GATE_BYTES; - delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; - const gitconfigPath = join(dir, 'fixture-gitconfig'); - writeFileSync(gitconfigPath, ''); - Object.assign(env, { - GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: gitconfigPath, - }); - const bare = join(dir, 'origin.git'); - const seed = join(dir, 'seed'); - const gateCwd = join(dir, 'checkout'); - const git = (args, cwd) => { - const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); - expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); - return r; - }; - mkdirSync(seed, { recursive: true }); - git(['init', '--quiet', '--bare', bare], dir); - // The bare repo's HEAD defaults to refs/heads/master; point it at the - // branch the seed pushes so the clone checks files out at all. - git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); - git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); - // Seed: a base commit with the grown workflow + a stale baseline, - // then a second commit that changes only an unrelated file, so the - // base sits behind the tip and a depth-1 clone does not contain it. - git(['init', '--quiet'], seed); - git(['config', 'user.email', 'gate-test@example.com'], seed); - git(['config', 'user.name', 'gate-test'], seed); - const seedWorkflows = join(seed, WORKFLOW_DIR); - mkdirSync(seedWorkflows, { recursive: true }); - writeFileSync(join(seedWorkflows, 'small.yml'), 'a'.repeat(5000)); - writeFileSync(join(seedWorkflows, '.size-baseline'), '100 small.yml\n'); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'base'], seed); - const baseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); - writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'unrelated tip'], seed); - git(['remote', 'add', 'origin', bare], seed); - git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); - // Depth-1 clone holds only the tip; the base commit needs a fetch. - // The file:// URL matters: a plain local path ignores --depth and - // copies full history, which would hide the fetch the gate must do. - git( - ['clone', '--quiet', '--depth', '1', `file://${bare}`, gateCwd], - dir, - ); - env.WORKFLOW_SIZE_BASE_SHA = baseSha; - const result = spawnSync('bash', [gatePath], { - cwd: gateCwd, - encoding: 'utf8', - env, - }); - expect(result.status).toBe(0); - expect(result.stdout).toContain('::warning'); - expect(result.stdout).toContain('the baseline went stale on main'); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - }); - it('fails a workflow with no baseline entry', () => { const result = runGate({ files: { 'small.yml': 100 }, @@ -461,6 +415,249 @@ describe.skipIf(process.platform === 'win32' || !canRunGate)( expect(result.status).toBe(1); expect(result.stdout).toContain("past this repo's"); }); + + // #9904: a workflow that grew on main without the same-PR baseline bump + // used to red-wall every OTHER open PR. A PR whose copy of the file is + // byte-identical to its base did not cause the drift and must only see + // a warning; the hard failure belongs to the PR that changes the file. + // These fixtures commit their base with git, hence their own skip gate. + describe.skipIf(!gitAvailable)('#9904 PR-scope downgrade', () => { + it('warns instead of failing when the PR did not touch the file', () => { + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('::warning'); + expect(result.stdout).toContain('the baseline went stale on main'); + expect(result.stdout).not.toContain('::error'); + // The success banner must not claim every file is within allowance + // on the very run that warned it is not. + expect(result.stdout).toContain('stale-baseline warnings above'); + }); + + it('still fails when the PR changed the file past the allowance', () => { + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + dirtyFiles: { 'small.yml': 5001 }, + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('grew to 5001 bytes'); + }); + + it('fails closed when the base commit cannot be resolved', () => { + // A base sha that is neither present nor fetchable must keep the + // strict failure — downgrading on an unverifiable base would fail + // the ratchet open. The annotation must also say which case this + // is: a transient fetch failure and genuine growth need opposite + // remedies (re-run the job vs bump the baseline). + const result = runGate({ + files: { 'small.yml': 5000 }, + baseline: '100 small.yml\n', + commitBase: true, + baseSha: '0'.repeat(40), + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('grew to 5000 bytes'); + expect(result.stdout).toContain('could not be resolved'); + expect(result.stdout).toContain('re-run the job'); + }); + + it('warns on a missing entry when the PR did not touch the file', () => { + // A workflow that reached main without a baseline entry (bypass + // merge, misclassification, gate outage) has the same red-wall + // shape as a stale size: every open PR fails on a bookkeeping fix + // its author cannot perform. Unchanged from base → warning. + const result = runGate({ + files: { 'small.yml': 100 }, + baseline: '# header only\n', + commitBase: true, + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('::warning'); + expect(result.stdout).toContain('has no entry'); + expect(result.stdout).not.toContain('::error'); + }); + + it('fails on a missing entry when the PR changed the file', () => { + const result = runGate({ + files: { 'small.yml': 100 }, + baseline: '# header only\n', + commitBase: true, + dirtyFiles: { 'small.yml': 200 }, + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('has no entry'); + }); + + it('fails closed on a missing entry when the base cannot be resolved', () => { + const result = runGate({ + files: { 'small.yml': 100 }, + baseline: '# header only\n', + commitBase: true, + baseSha: '0'.repeat(40), + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('has no entry'); + expect(result.stdout).toContain('could not be resolved'); + }); + + it('still fails when the PR adds a brand-new file past the allowance', () => { + // "Absent from the base commit" is a CHANGED file, not an unchanged + // one: `git show` fails and pipes an empty copy into cmp, so the PR + // that introduces a grown workflow owns it. A future simplification + // treating a failed `git show` as "nothing to compare" would + // downgrade exactly the PR the ratchet exists to catch. + const result = runGate({ + files: { 'other.yml': 100 }, + baseline: '100 other.yml\n100 small2.yml\n', + commitBase: true, + dirtyFiles: { 'small2.yml': 5000 }, + }); + expect(result.status).toBe(1); + expect(result.stdout).toContain('grew to 5000 bytes'); + }); + + it('fetches the base commit when it is not local (CI shallow-clone path)', () => { + // The production path: ci.yml checks out at fetch-depth 1, so the + // PR's base commit is never present locally and the gate must reach + // it via `git fetch --depth=1 origin `. Re-implementing the + // fixture here (rather than reusing runGate, which commits into the + // same repo) so the base commit genuinely has to be fetched. + // Removing the fetch line from the script must turn this test red. + const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-fetch-')); + try { + const env = hermeticGateEnv(dir); + const bare = join(dir, 'origin.git'); + const seed = join(dir, 'seed'); + const gateCwd = join(dir, 'checkout'); + const git = (args, cwd) => { + const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + mkdirSync(seed, { recursive: true }); + git(['init', '--quiet', '--bare', bare], dir); + // The bare repo's HEAD defaults to refs/heads/master; point it at + // the branch the seed pushes so the clone checks files out at all. + git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); + git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); + // Seed: a base commit with the grown workflow + a stale baseline, + // then a second commit that changes only an unrelated file, so the + // base sits behind the tip and a depth-1 clone does not contain it. + git(['init', '--quiet'], seed); + git(['config', 'user.email', 'gate-test@example.com'], seed); + git(['config', 'user.name', 'gate-test'], seed); + const seedWorkflows = join(seed, WORKFLOW_DIR); + mkdirSync(seedWorkflows, { recursive: true }); + writeFileSync(join(seedWorkflows, 'small.yml'), 'a'.repeat(5000)); + writeFileSync( + join(seedWorkflows, '.size-baseline'), + '100 small.yml\n', + ); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'base'], seed); + const baseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); + writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'unrelated tip'], seed); + git(['remote', 'add', 'origin', bare], seed); + git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); + // Depth-1 clone holds only the tip; the base commit needs a fetch. + // The file:// URL matters: a plain local path ignores --depth and + // copies full history, which would hide the fetch the gate must do. + git( + ['clone', '--quiet', '--depth', '1', `file://${bare}`, gateCwd], + dir, + ); + env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const result = spawnSync('bash', [gatePath], { + cwd: gateCwd, + encoding: 'utf8', + env, + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('::warning'); + expect(result.stdout).toContain('the baseline went stale on main'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + describe('the vitest mirror of the leniency (fileMatchesBase)', () => { + // The mirror is the only enforcer on the merge-queue lanes; these + // fixtures pin its four outcomes on a real git repo, mirroring the + // gate's semantics. + let dir; + let env; + let baseSha; + let restoreCwd; + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'workflow-size-mirror-')); + env = hermeticGateEnv(dir); + mkdirSync(join(dir, WORKFLOW_DIR), { recursive: true }); + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'base content\n'); + const git = (args) => { + const r = spawnSync('git', args, { + cwd: dir, + encoding: 'utf8', + env, + }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + git(['init', '--quiet']); + git(['config', 'user.email', 'gate-test@example.com']); + git(['config', 'user.name', 'gate-test']); + git(['add', '.']); + git(['commit', '--quiet', '-m', 'base']); + baseSha = git(['rev-parse', 'HEAD']).stdout.trim(); + restoreCwd = process.cwd(); + process.chdir(dir); + }); + afterAll(() => { + process.chdir(restoreCwd); + delete process.env.WORKFLOW_SIZE_BASE_SHA; + rmSync(dir, { recursive: true, force: true }); + }); + + it('returns false when no base SHA is set', () => { + delete process.env.WORKFLOW_SIZE_BASE_SHA; + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(false); + }); + + it('returns true for a file byte-identical to the base', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(true); + }); + + it('returns false for a file the PR changed', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const path = join(WORKFLOW_DIR, 'small.yml'); + writeFileSync(path, 'changed by the PR\n'); + try { + expect(fileMatchesBase(path)).toBe(false); + } finally { + writeFileSync(path, 'base content\n'); + } + }); + + it('returns false for a file absent from the base commit', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const path = join(WORKFLOW_DIR, 'brand-new.yml'); + writeFileSync(path, 'added by the PR\n'); + expect(fileMatchesBase(path)).toBe(false); + }); + + it('fails closed on an unresolvable base', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = '0'.repeat(40); + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(false); + }); + }); + }); }, ); From 8ad232689a0f720cc9c69f61e8977206a06f9917 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Mon, 24 Aug 2026 19:03:41 +0000 Subject: [PATCH 4/5] fix(ci): close review gaps in the workflow-size ratchet mirror (#9931) - Apply the #9904 stale-baseline leniency to the mirror's entry test, not only the allowance test: a workflow that reached main without a baseline entry used to warn in the bash gate but hard-fail every unrelated PR in `npm run test:ci`, relocating the exact red wall this PR removes. - Distinguish an unresolvable base from a changed file: fileMatchesBase now throws with "re-run the job" guidance instead of folding into false behind a message blaming the PR's growth, and the fetch stderr reaches the log. - Hoist WORKFLOW_SIZE_BASE_SHA to the workflow-level env block so every lane inherits it; delete the four hand-wired step-level copies and point the tripwire test at the single source. - Bump the ci.yml baseline entry to the file's true post-hoist size. - Harden the mirror's witnesses: fixtures hoisted out of the bash-gated block so they run on the merge-group Windows/macOS lanes, a backslash-path case pinning the pathspec normalization on every lane, a shallow-clone fixture pinning the mirror's fetch arm (the production path), and a parity fixture running the bash gate and the JS mirror on the same repo state. --- .github/workflows/.size-baseline | 2 +- .github/workflows/ci.yml | 18 +- scripts/tests/workflow-size.test.js | 430 ++++++++++++++++++++-------- 3 files changed, 316 insertions(+), 134 deletions(-) diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index 303e6a5def6..0e40ffccd38 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -18,7 +18,7 @@ 4638 build-and-publish-image.yml 49610 cd-cua-driver.yml 2076 cd-mobile-mcp.yml -74203 ci.yml +74315 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 2da50a395b9..8323498f3fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,12 @@ env: # new helper test can't be added to one path and silently dropped from the # other. HELPER_TESTS: '.github/scripts/pr-safety-precheck.test.mjs .github/scripts/cap-release-notes.test.mjs .github/scripts/ci/classify-profile.test.mjs .github/scripts/ci/classify-pr-profile.test.mjs .github/scripts/upsert-bot-comment.test.mjs .github/scripts/ci/main-failure-signature.test.mjs .github/scripts/classify-release-notes.test.mjs .github/scripts/dsw-swe-verified/make-manifest.test.mjs .github/scripts/dsw-swe-verified/make-terminal-bench-manifest.test.mjs .github/scripts/resolve-sandbox-image.test.mjs .github/scripts/web-shell-visuals-publish.test.mjs .github/scripts/web-shell-visuals-compose.test.mjs .github/scripts/serve-ab-diff.test.mjs .github/scripts/serve-ab-drive.test.mjs .github/scripts/qwen-triage-workflow.test.mjs .github/scripts/assign-issue-owner.test.mjs .github/scripts/auto-minimize-spam.test.mjs .github/scripts/ci-runner-routing.test.mjs' + # The growth ratchet and its vitest mirror compare each workflow against + # the PR's base commit to tell "this PR grew the file" apart from "the + # 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 }}' jobs: classify_pr: @@ -317,11 +323,6 @@ jobs: # loop for a day on 2026-08-19. Costs one `wc -c` per workflow file. - name: 'Check workflow file size' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - env: - # Lets the growth ratchet tell "this PR grew the workflow" apart - # from "the baseline went stale on main" so one author's missing - # baseline bump cannot red-wall every unrelated open PR (#9904). - WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' run: '.github/scripts/check-workflow-size.sh' - name: 'Docs-only CI' @@ -532,11 +533,6 @@ jobs: id: 'unit_tests' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' && steps.ci_profile.outputs.ci_profile == 'full' }}" env: - # Same base SHA the size gate above receives: the vitest mirror of - # the ratchet applies the identical stale-baseline leniency (#9904), - # so main-side drift warns here too instead of red-walling the - # full-CI lanes the gate just passed. - WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' @@ -882,7 +878,6 @@ jobs: - name: 'Run tests and generate reports' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" env: - WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' @@ -1013,7 +1008,6 @@ jobs: - name: 'Run tests and generate reports' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" env: - WORKFLOW_SIZE_BASE_SHA: '${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }}' NO_COLOR: true HOME: '${{ runner.temp }}/qwen-ci-home' USERPROFILE: '${{ runner.temp }}/qwen-ci-home' diff --git a/scripts/tests/workflow-size.test.js b/scripts/tests/workflow-size.test.js index 454a77af005..ce6141ed75c 100644 --- a/scripts/tests/workflow-size.test.js +++ b/scripts/tests/workflow-size.test.js @@ -15,6 +15,7 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, win32 } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; // GitHub does not start runs for a workflow file over 500 KB (512,000 bytes) @@ -59,30 +60,20 @@ describe('workflow file size', () => { 'if: "${{ needs.classify_pr.outputs.skip_ci != \'true\' }}"', ); expect(step?.[0]).not.toContain('ci_profile'); - // The ratchet's PR-scope fix (#9904) hangs off this env: without it the - // gate has no base to compare against and silently degrades to the - // pre-fix red-wall, so pin both event arms the script relies on. - expect(step?.[0]).toContain('WORKFLOW_SIZE_BASE_SHA'); - expect(step?.[0]).toContain('github.event.pull_request.base.sha'); - expect(step?.[0]).toContain('github.event.merge_group.base_sha'); }); - it('wires the base SHA into every lane that runs the vitest mirror', () => { - // The ratchet mirror below is the only enforcer on the merge-queue lanes - // that never run the bash gate; each `npm run test:ci` step must hand it - // the same base SHA the gate gets, or the #9904 stale-baseline leniency - // silently degrades to the pre-fix red wall on exactly those lanes. - const testSteps = [ - ...ciWorkflow.matchAll( - /- name: 'Run tests and generate reports'[\s\S]*?npm run test:ci/g, - ), - ]; - expect(testSteps).toHaveLength(3); - for (const step of testSteps) { - expect(step[0]).toContain('WORKFLOW_SIZE_BASE_SHA'); - expect(step[0]).toContain('github.event.pull_request.base.sha'); - expect(step[0]).toContain('github.event.merge_group.base_sha'); - } + it('wires the base SHA once at workflow level so every lane inherits it', () => { + // The ratchet's PR-scope fix (#9904) hangs off this env: without it the + // gate and its vitest mirror have no base to compare against and + // silently degrade to the pre-fix red wall. Declared once at workflow + // level, it reaches the gate step AND every `npm run test:ci` lane — + // including the merge-queue lanes where the mirror is the only ratchet + // enforcer — without each step hand-wiring a copy a future lane could + // forget. + const workflowEnv = ciWorkflow.match(/^env:[\s\S]*?\njobs:/m)?.[0]; + expect(workflowEnv).toContain('WORKFLOW_SIZE_BASE_SHA'); + expect(workflowEnv).toContain('github.event.pull_request.base.sha'); + expect(workflowEnv).toContain('github.event.merge_group.base_sha'); }); }); @@ -90,11 +81,19 @@ describe('workflow file size', () => { // landed on main without the same-PR baseline bump — warns instead of // red-walling unrelated PRs (#9904). This mirror is the ONLY enforcer on the // merge-queue lanes that never run the bash script, so it applies the same -// leniency: an over-allowance file that is byte-identical to the base passes. -// As in the gate, an unset or unresolvable base fails closed. +// leniency to both the over-allowance and the missing-entry arms: a file +// byte-identical to the base passes. An unset base fails closed, and an +// unresolvable base throws instead of returning false: like the gate's +// exit-2 arm, a transient fetch failure and genuine PR growth need opposite +// remedies, so the failure says which one it is instead of blaming the PR's +// growth. const fileMatchesBase = (file) => { const baseSha = (process.env.WORKFLOW_SIZE_BASE_SHA ?? '').trim(); if (!baseSha) return false; + // node:path join emits backslashes on the merge-queue Windows lane; git + // pathspecs want forward slashes, and normalizing once covers the + // readFileSync below on every platform. + const repoPath = file.split(/[\\/]/).join('/'); let resolved = spawnSync( 'git', @@ -104,17 +103,19 @@ const fileMatchesBase = (file) => { if (!resolved) { resolved = spawnSync('git', ['fetch', '--depth=1', '--quiet', 'origin', baseSha], { - stdio: 'ignore', + // stderr inherits so a fetch failure leaves its trace in the log. + stdio: ['ignore', 'ignore', 'inherit'], }).status === 0; } - if (!resolved) return false; - const baseCopy = spawnSync('git', [ - 'show', - `${baseSha}:${file.split(/[\\/]/).join('/')}`, - ]); + if (!resolved) { + throw new Error( + `base ${baseSha} could not be resolved (transient git fetch failure? re-run the job)`, + ); + } + const baseCopy = spawnSync('git', ['show', `${baseSha}:${repoPath}`]); return ( baseCopy.status === 0 && - Buffer.compare(baseCopy.stdout, readFileSync(file)) === 0 + Buffer.compare(baseCopy.stdout, readFileSync(repoPath)) === 0 ); }; @@ -154,7 +155,16 @@ describe('workflow size growth ratchet', () => { }); it.each(workflowFiles)('%s has a baseline entry', (file) => { - expect(baseline.has(workflowName(file))).toBe(true); + if (baseline.has(workflowName(file))) return; + // Stale-baseline leniency (#9904), mirroring the shell gate's + // missing-entry arm: a workflow that reached main without an entry + // (bypass merge, misclassification, gate outage) is main-side drift, and + // hard-failing every unrelated PR here relocates the exact red wall this + // PR removes into `npm run test:ci`. + expect( + fileMatchesBase(file), + `${file} has no entry in .size-baseline and differs from the PR's base`, + ).toBe(true); }); it.each(workflowFiles)('%s is within its baseline allowance', (file) => { @@ -207,37 +217,38 @@ const bashSupportsAssocArrays = const gitAvailable = spawnSync('git', ['--version'], { stdio: 'ignore' }).status === 0; +const gatePath = join( + process.cwd(), + '.github', + 'scripts', + 'check-workflow-size.sh', +); +// The gate reads three WORKFLOW_SIZE_* knobs, and the git fixtures commit +// through the developer's git config — scrub both, because a leak from the +// surrounding shell must not change what the fixtures assert: a leaked +// WORKFLOW_SIZE_BASE_SHA flips the fail-closed fixtures to the warning path, +// a leaked WORKFLOW_SIZE_GROWTH_ALLOWANCE flips a one-byte-over failure +// green, and a global commit.gpgsign or hooksPath breaks `git commit` +// silently the same way. +const hermeticGateEnv = (dir) => { + const env = { ...process.env }; + delete env.WORKFLOW_SIZE_BASE_SHA; + delete env.WORKFLOW_SIZE_GATE_BYTES; + delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; + const gitconfigPath = join(dir, 'fixture-gitconfig'); + writeFileSync(gitconfigPath, ''); + return Object.assign(env, { + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: gitconfigPath, + }); +}; + describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( 'check-workflow-size.sh execution', () => { // The block above re-implements the gate's arithmetic in JS; only running // the real script pins its decision branches (growth, missing entry, // missing baseline, slack warning, malformed line). - const gatePath = join( - process.cwd(), - '.github', - 'scripts', - 'check-workflow-size.sh', - ); - // The gate reads three WORKFLOW_SIZE_* knobs, and the git fixtures commit - // through the developer's git config — scrub both, because a leak from - // the surrounding shell must not change what the fixtures assert: a - // leaked WORKFLOW_SIZE_BASE_SHA flips the fail-closed fixtures to the - // warning path, a leaked WORKFLOW_SIZE_GROWTH_ALLOWANCE flips a - // one-byte-over failure green, and a global commit.gpgsign or hooksPath - // breaks `git commit` silently the same way. - const hermeticGateEnv = (dir) => { - const env = { ...process.env }; - delete env.WORKFLOW_SIZE_BASE_SHA; - delete env.WORKFLOW_SIZE_GATE_BYTES; - delete env.WORKFLOW_SIZE_GROWTH_ALLOWANCE; - const gitconfigPath = join(dir, 'fixture-gitconfig'); - writeFileSync(gitconfigPath, ''); - return Object.assign(env, { - GIT_CONFIG_NOSYSTEM: '1', - GIT_CONFIG_GLOBAL: gitconfigPath, - }); - }; const runGate = ({ files, baseline, commitBase, dirtyFiles, baseSha }) => { const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-')); try { @@ -586,77 +597,254 @@ describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( rmSync(dir, { recursive: true, force: true }); } }); + }); + }, +); - describe('the vitest mirror of the leniency (fileMatchesBase)', () => { - // The mirror is the only enforcer on the merge-queue lanes; these - // fixtures pin its four outcomes on a real git repo, mirroring the - // gate's semantics. - let dir; - let env; - let baseSha; - let restoreCwd; - beforeAll(() => { - dir = mkdtempSync(join(tmpdir(), 'workflow-size-mirror-')); - env = hermeticGateEnv(dir); - mkdirSync(join(dir, WORKFLOW_DIR), { recursive: true }); - writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'base content\n'); - const git = (args) => { - const r = spawnSync('git', args, { - cwd: dir, - encoding: 'utf8', - env, - }); - expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); - return r; - }; - git(['init', '--quiet']); - git(['config', 'user.email', 'gate-test@example.com']); - git(['config', 'user.name', 'gate-test']); - git(['add', '.']); - git(['commit', '--quiet', '-m', 'base']); - baseSha = git(['rev-parse', 'HEAD']).stdout.trim(); - restoreCwd = process.cwd(); - process.chdir(dir); - }); - afterAll(() => { - process.chdir(restoreCwd); - delete process.env.WORKFLOW_SIZE_BASE_SHA; - rmSync(dir, { recursive: true, force: true }); +// The mirror spawns only git, so its fixtures gate on git alone — not on the +// bash assoc-array capability the SCRIPT needs. They must run on the +// merge-group Windows and macOS lanes, where the bash gate never runs and +// this mirror is the only ratchet enforcer. +describe.skipIf(!gitAvailable)( + 'fileMatchesBase — the vitest mirror of the leniency', + () => { + let dir; + let baseSha; + let restoreCwd; + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'workflow-size-mirror-')); + const env = hermeticGateEnv(dir); + mkdirSync(join(dir, WORKFLOW_DIR), { recursive: true }); + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'base content\n'); + const git = (args) => { + const r = spawnSync('git', args, { + cwd: dir, + encoding: 'utf8', + env, }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + git(['init', '--quiet']); + git(['config', 'user.email', 'gate-test@example.com']); + git(['config', 'user.name', 'gate-test']); + git(['add', '.']); + git(['commit', '--quiet', '-m', 'base']); + baseSha = git(['rev-parse', 'HEAD']).stdout.trim(); + restoreCwd = process.cwd(); + process.chdir(dir); + }); + afterAll(() => { + process.chdir(restoreCwd); + delete process.env.WORKFLOW_SIZE_BASE_SHA; + rmSync(dir, { recursive: true, force: true }); + }); - it('returns false when no base SHA is set', () => { - delete process.env.WORKFLOW_SIZE_BASE_SHA; - expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(false); - }); + it('returns false when no base SHA is set', () => { + delete process.env.WORKFLOW_SIZE_BASE_SHA; + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(false); + }); - it('returns true for a file byte-identical to the base', () => { - process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; - expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(true); - }); + it('returns true for a file byte-identical to the base', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(true); + }); - it('returns false for a file the PR changed', () => { - process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; - const path = join(WORKFLOW_DIR, 'small.yml'); - writeFileSync(path, 'changed by the PR\n'); - try { - expect(fileMatchesBase(path)).toBe(false); - } finally { - writeFileSync(path, 'base content\n'); - } - }); + it('returns false for a file the PR changed', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const path = join(WORKFLOW_DIR, 'small.yml'); + writeFileSync(path, 'changed by the PR\n'); + try { + expect(fileMatchesBase(path)).toBe(false); + } finally { + writeFileSync(path, 'base content\n'); + } + }); - it('returns false for a file absent from the base commit', () => { - process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; - const path = join(WORKFLOW_DIR, 'brand-new.yml'); - writeFileSync(path, 'added by the PR\n'); - expect(fileMatchesBase(path)).toBe(false); - }); + it('returns false for a file absent from the base commit', () => { + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + const path = join(WORKFLOW_DIR, 'brand-new.yml'); + writeFileSync(path, 'added by the PR\n'); + expect(fileMatchesBase(path)).toBe(false); + }); - it('fails closed on an unresolvable base', () => { - process.env.WORKFLOW_SIZE_BASE_SHA = '0'.repeat(40); - expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(false); - }); - }); + it('normalizes win32-style paths before asking git or the filesystem', () => { + // On the merge-queue Windows lane join() emits backslashes, which git + // pathspecs reject. Pin the normalization with a backslash path on + // EVERY lane — on POSIX join() never emits one, so without this case + // a mutation deleting the normalization survives every test that runs + // and first fails during a real stale-baseline drift. + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + expect(fileMatchesBase(win32.join(WORKFLOW_DIR, 'small.yml'))).toBe(true); + }); + + it('fails closed on an unresolvable base', () => { + // Throws rather than returning false so the failure says "re-run the + // job" instead of blaming the PR's growth — the gate's exit-2 arm + // separates the same two cases for the same reason. + process.env.WORKFLOW_SIZE_BASE_SHA = '0'.repeat(40); + expect(() => fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toThrow( + /could not be resolved/, + ); + }); + + it('fetches the base commit when it is not local (CI shallow-clone path)', () => { + // Mirror of the gate's shallow-clone fixture: every production lane + // checks out at depth 1, so the base commit is never present locally + // and this fetch arm IS the production path for the mirror. Removing + // the arm must turn this test red. + const fetchDir = mkdtempSync( + join(tmpdir(), 'workflow-size-mirror-fetch-'), + ); + const fetchCwd = process.cwd(); + try { + const env = hermeticGateEnv(fetchDir); + const bare = join(fetchDir, 'origin.git'); + const seed = join(fetchDir, 'seed'); + const checkout = join(fetchDir, 'checkout'); + const git = (args, cwd) => { + const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + mkdirSync(seed, { recursive: true }); + git(['init', '--quiet', '--bare', bare], fetchDir); + // The bare repo's HEAD defaults to refs/heads/master; point it at + // the branch the seed pushes so the clone checks files out at all. + git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); + git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); + // Seed: a base commit, then a second commit that changes only an + // unrelated file, so the base sits behind the tip and a depth-1 + // clone does not contain it. + git(['init', '--quiet'], seed); + git(['config', 'user.email', 'gate-test@example.com'], seed); + git(['config', 'user.name', 'gate-test'], seed); + const seedWorkflows = join(seed, WORKFLOW_DIR); + mkdirSync(seedWorkflows, { recursive: true }); + writeFileSync(join(seedWorkflows, 'small.yml'), 'base content\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'base'], seed); + const fetchBaseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); + writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'unrelated tip'], seed); + git(['remote', 'add', 'origin', bare], seed); + git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); + // Depth-1 clone holds only the tip; the base commit needs a fetch. + // The file:// URL matters: a plain local path ignores --depth and + // copies full history, which would hide the fetch under test. + git( + [ + 'clone', + '--quiet', + '--depth', + '1', + pathToFileURL(bare).href, + checkout, + ], + fetchDir, + ); + process.chdir(checkout); + process.env.WORKFLOW_SIZE_BASE_SHA = fetchBaseSha; + // Pin the precondition: the base is absent locally, so any success + // below must come from the fetch arm, not from local history. + expect( + spawnSync( + 'git', + ['rev-parse', '--verify', '--quiet', `${fetchBaseSha}^{commit}`], + { stdio: 'ignore' }, + ).status, + ).not.toBe(0); + expect(fileMatchesBase(join(WORKFLOW_DIR, 'small.yml'))).toBe(true); + } finally { + process.chdir(fetchCwd); + delete process.env.WORKFLOW_SIZE_BASE_SHA; + rmSync(fetchDir, { recursive: true, force: true }); + } + }); + }, +); + +// One predicate ships as two implementations: the bash gate on the PR lanes +// and this JS mirror on the merge-queue lanes. Nothing else runs both +// against the same repo state, so an edit to the leniency logic that lands +// in only one copy makes one lane warn while the other hard-fails — the +// #9904 red wall recreated on lanes that never show the bash diagnostic — +// and stays green here. One committed fixture, both runtimes, every verdict. +describe.skipIf(!gitAvailable || !bashSupportsAssocArrays)( + 'the gate and the mirror agree on the same repo state', + () => { + it('is lenient on main-side drift and strict on PR growth, in both runtimes', () => { + const dir = mkdtempSync(join(tmpdir(), 'workflow-size-parity-')); + const restoreCwd = process.cwd(); + try { + const env = hermeticGateEnv(dir); + mkdirSync(join(dir, WORKFLOW_DIR), { recursive: true }); + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'a'.repeat(5000)); + writeFileSync( + join(dir, WORKFLOW_DIR, '.size-baseline'), + '100 small.yml\n', + ); + const git = (args) => { + const r = spawnSync('git', args, { + cwd: dir, + encoding: 'utf8', + env, + }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + git(['init', '--quiet']); + git(['config', 'user.email', 'gate-test@example.com']); + git(['config', 'user.name', 'gate-test']); + git(['add', '.']); + git(['commit', '--quiet', '-m', 'base']); + const baseSha = git(['rev-parse', 'HEAD']).stdout.trim(); + env.WORKFLOW_SIZE_BASE_SHA = baseSha; + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; + process.chdir(dir); + const gate = () => + spawnSync('bash', [gatePath], { cwd: dir, encoding: 'utf8', env }); + const mirror = () => fileMatchesBase(join(WORKFLOW_DIR, 'small.yml')); + + // Over-allowance drift: 5000 bytes against a recorded 100, file + // unchanged from the base — both runtimes lenient. + const driftGate = gate(); + expect(driftGate.status).toBe(0); + expect(driftGate.stdout).toContain('the baseline went stale on main'); + expect(mirror()).toBe(true); + + // The same drift with the PR changing the file — both strict. + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'a'.repeat(5001)); + const growthGate = gate(); + expect(growthGate.status).toBe(1); + expect(growthGate.stdout).toContain('grew to 5001 bytes'); + expect(mirror()).toBe(false); + + // Missing-entry drift: the gate's other lenient arm, with the file + // back at its base content — both runtimes lenient. + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'a'.repeat(5000)); + writeFileSync( + join(dir, WORKFLOW_DIR, '.size-baseline'), + '# header only\n', + ); + const missingGate = gate(); + expect(missingGate.status).toBe(0); + expect(missingGate.stdout).toContain('has no entry'); + expect(missingGate.stdout).toContain('unrelated PRs are not blocked'); + expect(mirror()).toBe(true); + + // Missing entry with the PR changing the file — both strict. + writeFileSync(join(dir, WORKFLOW_DIR, 'small.yml'), 'a'.repeat(5001)); + const changedGate = gate(); + expect(changedGate.status).toBe(1); + expect(changedGate.stdout).toContain('has no entry'); + expect(mirror()).toBe(false); + } finally { + process.chdir(restoreCwd); + delete process.env.WORKFLOW_SIZE_BASE_SHA; + rmSync(dir, { recursive: true, force: true }); + } }); }, ); From 8b3b24ed5ce15f25aa60bf3aeb3c198a470073c5 Mon Sep 17 00:00:00 2001 From: qwen-code-dev-bot Date: Mon, 24 Aug 2026 21:04:11 +0000 Subject: [PATCH 5/5] fix(ci): address round-3 review on the workflow-size ratchet (#9931) --- .github/workflows/ci.yml | 11 +- scripts/tests/workflow-size.test.js | 180 +++++++++++++--------------- 2 files changed, 88 insertions(+), 103 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8323498f3fc..6de389a1473 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -267,10 +267,13 @@ jobs: 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 }}" - # Shallow: nothing here walks git history (the verify guard below checks - # head.sha == HEAD, schema/tests touch only the working tree). On the - # in-repo ECS runner a full-history clone is the heaviest transfer and - # chokes the squid egress proxy, flaking checkout. depth 1 is enough. + # 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 + # baseline went stale; everything else touches only the working + # tree). On the in-repo ECS runner a full-history clone is the + # heaviest transfer and chokes the squid egress proxy, flaking + # checkout. depth 1 is enough. fetch-depth: 1 # Guard against a stale checkout (e.g. a caching egress proxy serving an old diff --git a/scripts/tests/workflow-size.test.js b/scripts/tests/workflow-size.test.js index ce6141ed75c..e0d93459848 100644 --- a/scripts/tests/workflow-size.test.js +++ b/scripts/tests/workflow-size.test.js @@ -14,7 +14,7 @@ import { writeFileSync, } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, win32 } from 'node:path'; +import { dirname, join, win32 } from 'node:path'; import { pathToFileURL } from 'node:url'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -71,9 +71,13 @@ describe('workflow file size', () => { // enforcer — without each step hand-wiring a copy a future lane could // forget. const workflowEnv = ciWorkflow.match(/^env:[\s\S]*?\njobs:/m)?.[0]; - expect(workflowEnv).toContain('WORKFLOW_SIZE_BASE_SHA'); - expect(workflowEnv).toContain('github.event.pull_request.base.sha'); - expect(workflowEnv).toContain('github.event.merge_group.base_sha'); + // Anchored to the whole line: substring checks still pass a `||` → `&&` + // mutation (empty on both events — 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). + expect(workflowEnv).toMatch( + /^\s*WORKFLOW_SIZE_BASE_SHA: '\$\{\{ github\.event\.pull_request\.base\.sha \|\| github\.event\.merge_group\.base_sha \}\}'$/m, + ); }); }); @@ -163,13 +167,16 @@ describe('workflow size growth ratchet', () => { // PR removes into `npm run test:ci`. expect( fileMatchesBase(file), - `${file} has no entry in .size-baseline and differs from the PR's base`, + `${file} has no entry in .size-baseline and differs from the PR's base — add its byte size to .size-baseline in this PR so its growth is tracked`, ).toBe(true); }); it.each(workflowFiles)('%s is within its baseline allowance', (file) => { const bytes = Buffer.byteLength(readFileSync(file)); const recorded = baseline.get(workflowName(file)); + // An entry-less file renders NaN/undefined below; the missing-entry test + // above owns that state. + if (recorded === undefined) return; if (bytes <= recorded + allowance) return; // Stale-baseline leniency (#9904), mirroring the shell gate: overage on // a file byte-identical to the PR's base is main-side drift, not this @@ -177,7 +184,7 @@ describe('workflow size growth ratchet', () => { // the run, relocating the red wall into `npm run test:ci`. expect( fileMatchesBase(file), - `${file} is ${bytes - recorded} bytes over its recorded ${recorded} and differs from the PR's base`, + `${file} is ${bytes - recorded} bytes over its recorded ${recorded} and differs from the PR's base — move prose into a sibling .md and long steps into .github/scripts/, or, if the growth is real, update .size-baseline in this PR and say why`, ).toBe(true); }); @@ -243,6 +250,54 @@ const hermeticGateEnv = (dir) => { }); }; +// Both fetch-arm fixtures need the same shape: a bare origin whose base +// commit sits behind an unrelated tip, so a depth-1 clone of the tip lacks +// the base and any success must come from the runtime's own fetch. Building +// it once keeps the gate's fixture and the mirror's from drifting — the clone +// URL spelling already drifted between the two copies. +const seedShallowClone = ({ root, env, seedFiles }) => { + const bare = join(root, 'origin.git'); + const seed = join(root, 'seed'); + const checkout = join(root, 'checkout'); + const git = (args, cwd) => { + const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); + expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); + return r; + }; + mkdirSync(seed, { recursive: true }); + git(['init', '--quiet', '--bare', bare], root); + // The bare repo's HEAD defaults to refs/heads/master; point it at the + // branch the seed pushes so the clone checks files out at all. + git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); + git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); + git(['init', '--quiet'], seed); + git(['config', 'user.email', 'gate-test@example.com'], seed); + git(['config', 'user.name', 'gate-test'], seed); + for (const [relPath, contents] of Object.entries(seedFiles)) { + const filePath = join(seed, relPath); + mkdirSync(dirname(filePath), { recursive: true }); + writeFileSync(filePath, contents); + } + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'base'], seed); + const baseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); + // A second commit touching only an unrelated file pushes the base behind + // the tip, so a depth-1 clone does not contain it. + writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); + git(['add', '.'], seed); + git(['commit', '--quiet', '-m', 'unrelated tip'], seed); + git(['remote', 'add', 'origin', bare], seed); + git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); + // Depth-1 clone holds only the tip; the base commit needs a fetch. The + // file:// URL matters: a plain local path ignores --depth and copies + // full history, which would hide the fetch under test. + git( + ['clone', '--quiet', '--depth', '1', pathToFileURL(bare).href, checkout], + root, + ); + return { checkout, baseSha }; +}; + describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( 'check-workflow-size.sh execution', () => { @@ -535,58 +590,23 @@ describe.skipIf(process.platform === 'win32' || !bashSupportsAssocArrays)( it('fetches the base commit when it is not local (CI shallow-clone path)', () => { // The production path: ci.yml checks out at fetch-depth 1, so the // PR's base commit is never present locally and the gate must reach - // it via `git fetch --depth=1 origin `. Re-implementing the - // fixture here (rather than reusing runGate, which commits into the - // same repo) so the base commit genuinely has to be fetched. + // it via `git fetch --depth=1 origin ` — runGate cannot stage + // that, because it commits into the same repo the script inspects. // Removing the fetch line from the script must turn this test red. const dir = mkdtempSync(join(tmpdir(), 'workflow-size-gate-fetch-')); try { const env = hermeticGateEnv(dir); - const bare = join(dir, 'origin.git'); - const seed = join(dir, 'seed'); - const gateCwd = join(dir, 'checkout'); - const git = (args, cwd) => { - const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); - expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); - return r; - }; - mkdirSync(seed, { recursive: true }); - git(['init', '--quiet', '--bare', bare], dir); - // The bare repo's HEAD defaults to refs/heads/master; point it at - // the branch the seed pushes so the clone checks files out at all. - git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); - git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); - // Seed: a base commit with the grown workflow + a stale baseline, - // then a second commit that changes only an unrelated file, so the - // base sits behind the tip and a depth-1 clone does not contain it. - git(['init', '--quiet'], seed); - git(['config', 'user.email', 'gate-test@example.com'], seed); - git(['config', 'user.name', 'gate-test'], seed); - const seedWorkflows = join(seed, WORKFLOW_DIR); - mkdirSync(seedWorkflows, { recursive: true }); - writeFileSync(join(seedWorkflows, 'small.yml'), 'a'.repeat(5000)); - writeFileSync( - join(seedWorkflows, '.size-baseline'), - '100 small.yml\n', - ); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'base'], seed); - const baseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); - writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'unrelated tip'], seed); - git(['remote', 'add', 'origin', bare], seed); - git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); - // Depth-1 clone holds only the tip; the base commit needs a fetch. - // The file:// URL matters: a plain local path ignores --depth and - // copies full history, which would hide the fetch the gate must do. - git( - ['clone', '--quiet', '--depth', '1', `file://${bare}`, gateCwd], - dir, - ); + const { checkout, baseSha } = seedShallowClone({ + root: dir, + env, + seedFiles: { + [join(WORKFLOW_DIR, 'small.yml')]: 'a'.repeat(5000), + [join(WORKFLOW_DIR, '.size-baseline')]: '100 small.yml\n', + }, + }); env.WORKFLOW_SIZE_BASE_SHA = baseSha; const result = spawnSync('bash', [gatePath], { - cwd: gateCwd, + cwd: checkout, encoding: 'utf8', env, }); @@ -699,59 +719,21 @@ describe.skipIf(!gitAvailable)( const fetchCwd = process.cwd(); try { const env = hermeticGateEnv(fetchDir); - const bare = join(fetchDir, 'origin.git'); - const seed = join(fetchDir, 'seed'); - const checkout = join(fetchDir, 'checkout'); - const git = (args, cwd) => { - const r = spawnSync('git', args, { cwd, encoding: 'utf8', env }); - expect(r.status, `git ${args.join(' ')}: ${r.stderr}`).toBe(0); - return r; - }; - mkdirSync(seed, { recursive: true }); - git(['init', '--quiet', '--bare', bare], fetchDir); - // The bare repo's HEAD defaults to refs/heads/master; point it at - // the branch the seed pushes so the clone checks files out at all. - git(['symbolic-ref', 'HEAD', 'refs/heads/main'], bare); - git(['config', 'uploadpack.allowAnySHA1InWant', 'true'], bare); - // Seed: a base commit, then a second commit that changes only an - // unrelated file, so the base sits behind the tip and a depth-1 - // clone does not contain it. - git(['init', '--quiet'], seed); - git(['config', 'user.email', 'gate-test@example.com'], seed); - git(['config', 'user.name', 'gate-test'], seed); - const seedWorkflows = join(seed, WORKFLOW_DIR); - mkdirSync(seedWorkflows, { recursive: true }); - writeFileSync(join(seedWorkflows, 'small.yml'), 'base content\n'); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'base'], seed); - const fetchBaseSha = git(['rev-parse', 'HEAD'], seed).stdout.trim(); - writeFileSync(join(seed, 'README.md'), 'unrelated tip change\n'); - git(['add', '.'], seed); - git(['commit', '--quiet', '-m', 'unrelated tip'], seed); - git(['remote', 'add', 'origin', bare], seed); - git(['push', '--quiet', 'origin', 'HEAD:refs/heads/main'], seed); - // Depth-1 clone holds only the tip; the base commit needs a fetch. - // The file:// URL matters: a plain local path ignores --depth and - // copies full history, which would hide the fetch under test. - git( - [ - 'clone', - '--quiet', - '--depth', - '1', - pathToFileURL(bare).href, - checkout, - ], - fetchDir, - ); + const { checkout, baseSha } = seedShallowClone({ + root: fetchDir, + env, + seedFiles: { + [join(WORKFLOW_DIR, 'small.yml')]: 'base content\n', + }, + }); process.chdir(checkout); - process.env.WORKFLOW_SIZE_BASE_SHA = fetchBaseSha; + process.env.WORKFLOW_SIZE_BASE_SHA = baseSha; // Pin the precondition: the base is absent locally, so any success // below must come from the fetch arm, not from local history. expect( spawnSync( 'git', - ['rev-parse', '--verify', '--quiet', `${fetchBaseSha}^{commit}`], + ['rev-parse', '--verify', '--quiet', `${baseSha}^{commit}`], { stdio: 'ignore' }, ).status, ).not.toBe(0);