fix(ci): a pull request whose head predates the package-size check skips it visibly instead of failing build-storybook

.github/actions/test-scope/action.yml

@@ -49,7 +49,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: ''
 
@@ -95,6 +96,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
@@ -127,6 +133,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
@@ -240,17 +247,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';
+                core.notice(`This pull request's head has no ${headFile}, so this job skips what needs it; merging main brings it in.`);
+                headFilePresent = 'false';
               }
             }
           }
+          if (headFilePresent === 'false') applies = 'false';
           let componentScope = {};
           if (audit) {
             if (specialized) {
@@ -280,4 +291,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});

.github/scripts/ci-package-size-predating-head.test.mjs

@@ -0,0 +1,82 @@
+// 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 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';
+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 job = ci.jobs['build-storybook'];
+  const measure = step('build-storybook', 'Measure published package size');
+
+  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'",
+    );
+  });
+
+  it('records a measurement only after measuring', () => {
+    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 [
+      "Download main's package size for the merge base",
+      'Compare published package size',
+    ]) {
+      expect(step('build-storybook', name).if).toContain(
+        "steps.package-size.outputs.measured == 'true'",
+      );
+    }
+  });
+});

.github/scripts/test-scope-action.test.mjs

@@ -162,6 +162,7 @@ describe('per-job scope step', () => {
     expect(Object.keys(action.outputs).sort()).toEqual(
       [
         'applies',
+        'head_file_present',
         'changed_files_path',
         'decided',
         'docsite_only',
@@ -410,6 +411,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: [

.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'
@@ -1183,11 +1185,16 @@ 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')
-        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): 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: |
+          node .github/scripts/package-size.mjs measure --output package-size-head.json
+          echo "measured=true" >> "$GITHUB_OUTPUT"
 
       - name: Download main's package size for the 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' && steps.size-records.outputs.package_run_id != ''
+        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' && steps.size-records.outputs.package_run_id != ''
         continue-on-error: true
         uses: actions/download-artifact@v8
         with:
@@ -1199,7 +1206,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.merge-base.outputs.sha }}