From a98b20836689fcaea5e598a08ef1abe549190036 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 12:16:16 -0700 Subject: [PATCH 1/4] fix(ci): a pull request whose head predates the package-size check skips it visibly instead of failing build-storybook The workflow comes from the merge ref and the job checks out the head, so a head cut before the check has no package-size.mjs. The measurement step now skips with a notice there, and the comparison steps run only after a measurement. size-base finds the merge base without merge-base.sh on such a head. --- .../ci-package-size-predating-head.test.mjs | 101 ++++++++++++++++++ .github/workflows/ci.yml | 22 +++- 2 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 .github/scripts/ci-package-size-predating-head.test.mjs diff --git a/.github/scripts/ci-package-size-predating-head.test.mjs b/.github/scripts/ci-package-size-predating-head.test.mjs new file mode 100644 index 0000000000000..205ce761cec62 --- /dev/null +++ b/.github/scripts/ci-package-size-predating-head.test.mjs @@ -0,0 +1,101 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file ci-package-size-predating-head.test.mjs + * Contract: a pull request whose head predates the package-size check (the + * workflow comes from the merge ref, the checkout from the head) skips the + * check visibly instead of failing build-storybook, and the comparison steps + * after it run only when the head was measured. size-base finds the merge + * base without merge-base.sh on such a head. + */ + +import {execFileSync, spawnSync} from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import {afterEach, describe, expect, it} from 'vitest'; +import yaml from 'yaml'; + +const ROOT = path.resolve(import.meta.dirname, '../..'); +const ci = yaml.parse( + fs.readFileSync(path.join(ROOT, '.github/workflows/ci.yml'), 'utf8'), +); +const step = (job, name) => + ci.jobs[job].steps.find(entry => entry.name === name); +const roots = []; +afterEach(() => { + for (const root of roots.splice(0)) + fs.rmSync(root, {recursive: true, force: true}); +}); + +/** Runs a step's script in a fresh directory; returns its exit, stdout and outputs. */ +function runStep(script, setup = () => {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'predating-')); + roots.push(root); + setup(root); + const output = path.join(root, 'github-output'); + fs.writeFileSync(output, ''); + const result = spawnSync('bash', ['-e', '-c', script], { + cwd: root, + encoding: 'utf8', + env: {...process.env, GITHUB_OUTPUT: output, BASE_REF: 'main'}, + }); + return {...result, outputs: fs.readFileSync(output, 'utf8')}; +} + +describe('package size on a head that predates it', () => { + const measure = step('build-storybook', 'Measure published package size'); + + it('skips with a notice and no measurement when the head has no script', () => { + const result = runStep(measure.run); + expect(result.status).toBe(0); + expect(result.stdout).toContain( + '::notice title=Head predates the package-size check::', + ); + expect(result.outputs).not.toContain('measured=true'); + }); + + it('measures, and says so, when the head has the script', () => { + const result = runStep(measure.run, root => { + fs.mkdirSync(path.join(root, '.github/scripts'), {recursive: true}); + fs.writeFileSync( + path.join(root, '.github/scripts/package-size.mjs'), + "import fs from 'node:fs';\nfs.writeFileSync('package-size-head.json', '{}');\n", + ); + }); + expect(result.status).toBe(0); + expect(result.outputs).toContain('measured=true'); + }); + + it('runs the comparison steps only after a measurement', () => { + for (const name of [ + 'Find the merge base for package size', + 'Compare published package size', + ]) { + expect(step('build-storybook', name).if).toContain( + "steps.package-size.outputs.measured == 'true'", + ); + } + }); + + it('finds the merge base without merge-base.sh in size-base', () => { + const script = step('size-base', 'Check out the merge base').run; + const result = runStep( + script.replace( + /git worktree add[\s\S]*$/, + 'echo "sha=$BASE_SHA" >> "$GITHUB_OUTPUT"', + ), + root => { + const git = (...args) => + execFileSync('git', args, {cwd: root, encoding: 'utf8'}); + git('init', '-q', '-b', 'main'); + git('config', 'user.email', 'test@test.co'); + git('config', 'user.name', 'Test'); + git('commit', '-q', '--allow-empty', '-m', 'base'); + git('update-ref', 'refs/remotes/origin/main', 'HEAD'); + }, + ); + expect(result.status).toBe(0); + expect(result.outputs).toMatch(/^sha=[0-9a-f]{40}$/m); + }); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8f5db03721062..68d17cc525e84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1083,11 +1083,20 @@ jobs: - name: Measure published package size id: package-size if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && github.event_name == 'pull_request' && (steps.scope.outputs.touches == 'true' || steps.scope.outputs.full_suite != 'false') - run: node .github/scripts/package-size.mjs measure --output package-size-head.json + # A head cut before the check has no script to run (the workflow comes + # from the merge ref, the checkout from the head): it skips, visibly, + # until it merges main. + run: | + if [ ! -f .github/scripts/package-size.mjs ]; then + echo '::notice title=Head predates the package-size check::Skipping the package-size check; merge main to run it.' + exit 0 + fi + node .github/scripts/package-size.mjs measure --output package-size-head.json + echo "measured=true" >> "$GITHUB_OUTPUT" - name: Find the merge base for package size id: package-size-merge-base - if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.package-size.outcome == 'success' + if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.package-size.outputs.measured == 'true' env: BASE_REF: ${{ github.base_ref }} run: echo "sha=$(.github/scripts/merge-base.sh "$BASE_REF" 'the package-size comparison')" >> "$GITHUB_OUTPUT" @@ -1121,7 +1130,7 @@ jobs: # Advisory: a package over its budget is flagged in the job summary and # the PR report, and fails nothing. - name: Compare published package size - if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.package-size.outcome == 'success' + if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && steps.package-size.outputs.measured == 'true' env: HEAD_SHA: ${{ github.event.pull_request.head.sha }} BASE_SHA: ${{ steps.package-size-merge-base.outputs.sha }} @@ -1219,7 +1228,12 @@ jobs: env: BASE_REF: ${{ github.base_ref }} run: | - BASE_SHA=$(.github/scripts/merge-base.sh "$BASE_REF" 'base size measurement' || true) + # A head cut before merge-base.sh finds the merge base directly. + if [ -x .github/scripts/merge-base.sh ]; then + BASE_SHA=$(.github/scripts/merge-base.sh "$BASE_REF" 'base size measurement' || true) + else + BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD || true) + fi if [ -z "$BASE_SHA" ]; then echo "found=false" >> "$GITHUB_OUTPUT" exit 0 From da5d0a6139ba9f5f90077c8b61c9b6758bfe4d8a Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 13:15:52 -0700 Subject: [PATCH 2/4] fix(ci): drop the size-base fallback (size-base goes away with Main's size records); keep the build-storybook skip --- .../ci-package-size-predating-head.test.mjs | 26 ++----------------- .github/workflows/ci.yml | 7 +---- 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/.github/scripts/ci-package-size-predating-head.test.mjs b/.github/scripts/ci-package-size-predating-head.test.mjs index 205ce761cec62..5447a6f47be74 100644 --- a/.github/scripts/ci-package-size-predating-head.test.mjs +++ b/.github/scripts/ci-package-size-predating-head.test.mjs @@ -5,11 +5,10 @@ * Contract: a pull request whose head predates the package-size check (the * workflow comes from the merge ref, the checkout from the head) skips the * check visibly instead of failing build-storybook, and the comparison steps - * after it run only when the head was measured. size-base finds the merge - * base without merge-base.sh on such a head. + * after it run only when the head was measured. */ -import {execFileSync, spawnSync} from 'node:child_process'; +import {spawnSync} from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -77,25 +76,4 @@ describe('package size on a head that predates it', () => { ); } }); - - it('finds the merge base without merge-base.sh in size-base', () => { - const script = step('size-base', 'Check out the merge base').run; - const result = runStep( - script.replace( - /git worktree add[\s\S]*$/, - 'echo "sha=$BASE_SHA" >> "$GITHUB_OUTPUT"', - ), - root => { - const git = (...args) => - execFileSync('git', args, {cwd: root, encoding: 'utf8'}); - git('init', '-q', '-b', 'main'); - git('config', 'user.email', 'test@test.co'); - git('config', 'user.name', 'Test'); - git('commit', '-q', '--allow-empty', '-m', 'base'); - git('update-ref', 'refs/remotes/origin/main', 'HEAD'); - }, - ); - expect(result.status).toBe(0); - expect(result.outputs).toMatch(/^sha=[0-9a-f]{40}$/m); - }); }); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68d17cc525e84..66705e16bbce5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1228,12 +1228,7 @@ jobs: env: BASE_REF: ${{ github.base_ref }} run: | - # A head cut before merge-base.sh finds the merge base directly. - if [ -x .github/scripts/merge-base.sh ]; then - BASE_SHA=$(.github/scripts/merge-base.sh "$BASE_REF" 'base size measurement' || true) - else - BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD || true) - fi + BASE_SHA=$(.github/scripts/merge-base.sh "$BASE_REF" 'base size measurement' || true) if [ -z "$BASE_SHA" ]; then echo "found=false" >> "$GITHUB_OUTPUT" exit 0 From 8ae797a7b99b21ed2e0f7d5ea19d767f2020c565 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 14:09:21 -0700 Subject: [PATCH 3/4] fix(ci): gate the package-size check on the scope step's requires-head-file (new head_file_present output) instead of an inline file check --- .github/actions/test-scope/action.yml | 19 ++++++++++--- .../ci-package-size-predating-head.test.mjs | 21 ++++++++------ .github/scripts/test-scope-action.test.mjs | 28 +++++++++++++++++++ .github/workflows/ci.yml | 12 ++++---- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/.github/actions/test-scope/action.yml b/.github/actions/test-scope/action.yml index b583b0b31567c..f0e9ac1af993c 100644 --- a/.github/actions/test-scope/action.yml +++ b/.github/actions/test-scope/action.yml @@ -47,7 +47,8 @@ inputs: requires-head-file: description: >- Optional path the work needs at the PR head. A head cut before that file - existed, whose diff does not touch it, does not apply (with a notice). + existed, whose diff does not touch it, does not apply (with a notice), + and `head_file_present` says so whether or not `touches` holds. required: false default: '' @@ -93,6 +94,11 @@ outputs: A file listing the PR's changed paths, one per line (with `component-audit`); empty when the step could not list them. value: ${{ steps.scope.outputs.changed_files_path }} + head_file_present: + description: >- + `false` when the PR head lacks `requires-head-file` and its diff does not + touch it (a head cut before the file existed); `true` otherwise. + value: ${{ steps.scope.outputs.head_file_present }} applies: description: >- The job's work applies: not a specialized lane (spec, docsite or tooling @@ -125,6 +131,7 @@ runs: release_bump: 'false', touches: 'true', applies: 'true', + head_file_present: 'true', decided: 'true', }; // Outside a pull request, or when undecidable, every component audit @@ -225,17 +232,21 @@ runs: } const specialized = ['spec_only', 'docsite_only', 'tooling_only'].some(key => classified[key] === 'true'); let applies = String(!specialized && touches === 'true'); + // Whether the head has the file the work needs, decided whether or + // not `touches` holds; a diff that touches the file keeps it. + let headFilePresent = 'true'; const headFile = process.env.SCOPE_HEAD_FILE; - if (applies === 'true' && headFile && !files.some(f => f.filename === headFile || f.previous_filename === headFile)) { + if (!specialized && headFile && !files.some(f => f.filename === headFile || f.previous_filename === headFile)) { try { await github.rest.repos.getContent({...context.repo, path: headFile, ref: pr.head.sha}); } catch (error) { if (error.status === 404) { core.notice(`This pull request's head has no ${headFile}; merge main to run this check.`); - applies = 'false'; + headFilePresent = 'false'; } } } + if (headFilePresent === 'false') applies = 'false'; let componentScope = {}; if (audit) { if (specialized) { @@ -265,4 +276,4 @@ runs: fs.writeFileSync(changedFilesPath, `${changed.join('\n')}\n`); componentScope.changed_files_path = changedFilesPath; } - report({...everything, ...classified, ...componentScope, release_bump: 'false', touches, applies}); + report({...everything, ...classified, ...componentScope, release_bump: 'false', touches, applies, head_file_present: headFilePresent}); diff --git a/.github/scripts/ci-package-size-predating-head.test.mjs b/.github/scripts/ci-package-size-predating-head.test.mjs index 5447a6f47be74..ea9fb82fbcdc6 100644 --- a/.github/scripts/ci-package-size-predating-head.test.mjs +++ b/.github/scripts/ci-package-size-predating-head.test.mjs @@ -4,8 +4,9 @@ * @file ci-package-size-predating-head.test.mjs * Contract: a pull request whose head predates the package-size check (the * workflow comes from the merge ref, the checkout from the head) skips the - * check visibly instead of failing build-storybook, and the comparison steps - * after it run only when the head was measured. + * check instead of failing build-storybook: the scope step's + * `requires-head-file` notes the missing script (test-scope-action.test.mjs), + * and the comparison steps after it run only when the head was measured. */ import {spawnSync} from 'node:child_process'; @@ -43,18 +44,20 @@ function runStep(script, setup = () => {}) { } describe('package size on a head that predates it', () => { + const job = ci.jobs['build-storybook']; const measure = step('build-storybook', 'Measure published package size'); - it('skips with a notice and no measurement when the head has no script', () => { - const result = runStep(measure.run); - expect(result.status).toBe(0); - expect(result.stdout).toContain( - '::notice title=Head predates the package-size check::', + it("asks the scope step whether the head has the check's script", () => { + const scope = job.steps.find(entry => entry.id === 'scope'); + expect(scope.with['requires-head-file']).toBe( + '.github/scripts/package-size.mjs', + ); + expect(measure.if).toContain( + "steps.scope.outputs.head_file_present != 'false'", ); - expect(result.outputs).not.toContain('measured=true'); }); - it('measures, and says so, when the head has the script', () => { + it('records a measurement only after measuring', () => { const result = runStep(measure.run, root => { fs.mkdirSync(path.join(root, '.github/scripts'), {recursive: true}); fs.writeFileSync( diff --git a/.github/scripts/test-scope-action.test.mjs b/.github/scripts/test-scope-action.test.mjs index d87010c59feea..d1e835c9b7912 100644 --- a/.github/scripts/test-scope-action.test.mjs +++ b/.github/scripts/test-scope-action.test.mjs @@ -158,6 +158,7 @@ describe('per-job scope step', () => { expect(Object.keys(action.outputs).sort()).toEqual( [ 'applies', + 'head_file_present', 'changed_files_path', 'decided', 'docsite_only', @@ -366,6 +367,33 @@ describe('per-job scope step', () => { expect(outputs.applies).toBe('true'); }); + it('reports a head without the file whether or not touches holds', async () => { + for (const file of ['packages/core/src/Button/Button.tsx', 'README.md']) { + const {outputs} = await decide({files: changed(file), inputs}); + expect(outputs.head_file_present, file).toBe('false'); + } + const {outputs} = await decide({ + files: changed('README.md'), + headFiles: ['playwright.interaction.config.ts'], + inputs, + }); + expect(outputs.head_file_present).toBe('true'); + }); + + it('keeps the file present for a pull request that renames or deletes it', async () => { + for (const file of [ + { + filename: 'playwright.old-interaction.config.ts', + previous_filename: 'playwright.interaction.config.ts', + status: 'renamed', + }, + {filename: 'playwright.interaction.config.ts', status: 'removed'}, + ]) { + const {outputs} = await decide({files: [file], inputs}); + expect(outputs.head_file_present).toBe('true'); + } + }); + it('still applies to a pull request that deletes the file, so the check fails', async () => { const {outputs} = await decide({ files: [ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66705e16bbce5..d2441b05afdf6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -951,6 +951,8 @@ jobs: # infrastructure, `full_suite`, runs it too); the build itself # follows the specialized-lane outputs. paths: '^(packages/|pnpm-lock\.yaml$|package\.json$|tsconfig\.json$|scripts/|\.github/workflows/ci\.yml$|\.github/scripts/package-size\.mjs$|\.github/scripts/lib/package-size\.js$|\.github/scripts/merge-base\.sh$)' + # A head cut before the package-size check has no script for it. + requires-head-file: .github/scripts/package-size.mjs - name: Skip heavy work for a specialized lane if: steps.scope.outputs.docsite_only == 'true' || steps.scope.outputs.spec_only == 'true' || steps.scope.outputs.tooling_only == 'true' @@ -1082,15 +1084,11 @@ jobs: # package or the lockfile runs it. - name: Measure published package size id: package-size - if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && github.event_name == 'pull_request' && (steps.scope.outputs.touches == 'true' || steps.scope.outputs.full_suite != 'false') # A head cut before the check has no script to run (the workflow comes - # from the merge ref, the checkout from the head): it skips, visibly, - # until it merges main. + # from the merge ref, the checkout from the head): the scope step notes + # it, and the check skips until the branch merges main. + if: steps.scope.outputs.docsite_only != 'true' && steps.scope.outputs.spec_only != 'true' && steps.scope.outputs.tooling_only != 'true' && github.event_name == 'pull_request' && steps.scope.outputs.head_file_present != 'false' && (steps.scope.outputs.touches == 'true' || steps.scope.outputs.full_suite != 'false') run: | - if [ ! -f .github/scripts/package-size.mjs ]; then - echo '::notice title=Head predates the package-size check::Skipping the package-size check; merge main to run it.' - exit 0 - fi node .github/scripts/package-size.mjs measure --output package-size-head.json echo "measured=true" >> "$GITHUB_OUTPUT" From bfd36eaa4aae98952876cf9823312fa38ab751c8 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 31 Oct 2026 14:39:42 -0700 Subject: [PATCH 4/4] ci: the head-file notice holds for every job that asks for it --- .github/actions/test-scope/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/test-scope/action.yml b/.github/actions/test-scope/action.yml index f0e9ac1af993c..07e051427deeb 100644 --- a/.github/actions/test-scope/action.yml +++ b/.github/actions/test-scope/action.yml @@ -241,7 +241,7 @@ runs: await github.rest.repos.getContent({...context.repo, path: headFile, ref: pr.head.sha}); } catch (error) { if (error.status === 404) { - core.notice(`This pull request's head has no ${headFile}; merge main to run this check.`); + core.notice(`This pull request's head has no ${headFile}, so this job skips what needs it; merging main brings it in.`); headFilePresent = 'false'; } }