From 077a1457caf862ae3bc30cbe15f90702b143c402 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 24 Sep 2026 18:05:38 +0200 Subject: [PATCH 1/2] test: guard the @actions/core import form against regression (#3561) --- CHANGELOG.md | 1 + .../__tests__/actions-core-import.test.js | 126 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 scripts/validation/__tests__/actions-core-import.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index da13b84cdb..3f05195334 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Actions Import Regression Guarded** — A test now fails if a default import of the Actions toolkit returns, which is what blocked the upgrade. ([#3561](https://github.com/lightspeedwp/.github/issues/3561)) - **Labelling Agent Crash on @actions/core 3** — The labelling and project sync agents now load with both current and upcoming versions of the GitHub Actions toolkit. ([#3503](https://github.com/lightspeedwp/.github/pull/3503)) - **Changelog Check Timeouts** — Fixed the changelog check failing at random on slow checkouts; it now reads only the files it needs. (#3520) - **Issue Labelling Floods** — Relabelling issues in bulk no longer queues hundreds of labelling runs or puts back labels that were just removed. ([#3531](https://github.com/lightspeedwp/.github/issues/3531)) diff --git a/scripts/validation/__tests__/actions-core-import.test.js b/scripts/validation/__tests__/actions-core-import.test.js new file mode 100644 index 0000000000..a08e6bca67 --- /dev/null +++ b/scripts/validation/__tests__/actions-core-import.test.js @@ -0,0 +1,126 @@ +/** + * Regression guard for the @actions/core import form (#3561). + * + * PR #3547 changed `import core from "@actions/core"` to + * `import * as core from "@actions/core"` in the labelling agents, because + * the package becomes ESM-only in 3.x and an ESM module has no default + * export. That is what blocked #3503. + * + * `tests/js/import-includes-smoke.test.js` cannot catch a regression here: + * it only regex-matches *relative* specifiers and never executes anything, + * so a bare specifier like `@actions/core` is invisible to it. + * + * This guard checks the two properties the fix actually depends on: + * 1. real source never uses a default import of the package, and + * 2. the installed package still exposes every member those files use, + * verified with native ESM resolution. + * + * Deliberately not importing the agent modules themselves: several of them + * are CommonJS-within-an-ESM-package or run `main()` on import, so loading + * them here would test unrelated behaviour and produce false results. + */ +const fs = require('node:fs'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +const ROOT = path.join(__dirname, '../../..'); +const SPECIFIER = '@actions/core'; +// Bare `import name from "@actions/core"` is the form that breaks on an +// ESM-only release; `import * as core` is the supported form. +const DEFAULT_IMPORT = new RegExp( + String.raw`^\s*import\s+[A-Za-z_$][\w$]*\s+from\s+["']${SPECIFIER}["']`, + 'm' +); +const NAMESPACE_IMPORT = new RegExp( + String.raw`^\s*import\s+\*\s+as\s+\w+\s+from\s+["']${SPECIFIER}["']`, + 'm' +); +// Parked trees are not shipped code: `.jest-skip/**` holds intentionally +// disabled suites and `**/fixtures/**` holds deliberately invalid input. +const SKIP_DIRS = new Set([ + 'node_modules', + '.git', + 'coverage', + 'dist', + 'build', + '.jest-skip', + 'fixtures', +]); +const SCAN_ROOTS = ['scripts', 'agents']; + +/** Real source files that import @actions/core, as repo-relative POSIX paths. */ +function findConsumers(dir = ROOT, found = []) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (dir === ROOT ? SCAN_ROOTS.includes(entry.name) : true) findConsumers(full, found); + } else if (/\.(js|mjs)$/.test(entry.name) && full !== __filename) { + const source = fs.readFileSync(full, 'utf8'); + if (source.includes(SPECIFIER)) { + found.push({ file: path.relative(ROOT, full).split(path.sep).join('/'), source }); + } + } + } + return found; +} + +describe('@actions/core import form (#3561)', () => { + const consumers = findConsumers(); + + test('real source has @actions/core consumers to guard', () => { + expect(consumers.length).toBeGreaterThan(0); + }); + + test('no real source uses a default import of @actions/core', () => { + const offenders = consumers + .filter(({ source }) => DEFAULT_IMPORT.test(source)) + .map(({ file }) => file); + expect(offenders).toEqual([]); + }); + + test('ESM consumers use the namespace form the fix relies on', () => { + const namespaceFiles = consumers.filter(({ source }) => NAMESPACE_IMPORT.test(source)); + // Guard against the form silently reverting to a named import, which is + // also unavailable once the package ships no CommonJS build. + const namedOnly = consumers + .filter(({ source }) => { + const imports = source.match( + new RegExp(String.raw`^\s*import\s+[^;\n]*from\s+["']${SPECIFIER}["']`, 'gm') + ); + return imports && imports.every((line) => !NAMESPACE_IMPORT.test(line)); + }) + .map(({ file }) => file); + expect(namespaceFiles.length).toBeGreaterThan(0); + expect(namedOnly).toEqual([]); + }); + + test('the installed package exposes every member those files call', () => { + const used = new Set(); + for (const { source } of consumers) { + for (const match of source.matchAll(new RegExp(String.raw`\bcore\.(\w+)`, 'g'))) { + used.add(match[1]); + } + } + expect(used.size).toBeGreaterThan(0); + + // Native ESM resolution in a real Node process, so an ESM-only release + // is exercised the way GitHub Actions runs it. + const result = spawnSync( + process.execPath, + [ + '--input-type=module', + '-e', + `const m = await import('${SPECIFIER}'); + const used = ${JSON.stringify([...used])}; + const missing = used.filter((key) => !(key in m)); + if (missing.length) { console.error('missing: ' + missing.join(', ')); process.exit(1); } + console.log('ok ' + used.length);`, + ], + { cwd: ROOT, encoding: 'utf8' } + ); + + expect(result.status).toBe(0); + expect(result.stdout).toContain(`ok ${used.size}`); + }, 120000); +}); From 43287e985be806930fd9eed067edb12ba2e33f9c Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 24 Sep 2026 18:35:07 +0200 Subject: [PATCH 2/2] test: harden the @actions/core import guard against multi-line and cross-statement matches --- .../__tests__/actions-core-import.test.js | 83 ++++++++----------- 1 file changed, 33 insertions(+), 50 deletions(-) diff --git a/scripts/validation/__tests__/actions-core-import.test.js b/scripts/validation/__tests__/actions-core-import.test.js index a08e6bca67..936684f25a 100644 --- a/scripts/validation/__tests__/actions-core-import.test.js +++ b/scripts/validation/__tests__/actions-core-import.test.js @@ -10,31 +10,27 @@ * it only regex-matches *relative* specifiers and never executes anything, * so a bare specifier like `@actions/core` is invisible to it. * - * This guard checks the two properties the fix actually depends on: - * 1. real source never uses a default import of the package, and - * 2. the installed package still exposes every member those files use, - * verified with native ESM resolution. - * - * Deliberately not importing the agent modules themselves: several of them - * are CommonJS-within-an-ESM-package or run `main()` on import, so loading - * them here would test unrelated behaviour and produce false results. + * The contract this guard enforces is narrow and deliberate: an + * `@actions/core` import must not carry a default clause. Namespace and + * named imports are both valid against the ESM-only release (verified on + * 3.0.1), so they are not rejected here. */ const fs = require('node:fs'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); const ROOT = path.join(__dirname, '../../..'); -const SPECIFIER = '@actions/core'; -// Bare `import name from "@actions/core"` is the form that breaks on an -// ESM-only release; `import * as core` is the supported form. -const DEFAULT_IMPORT = new RegExp( - String.raw`^\s*import\s+[A-Za-z_$][\w$]*\s+from\s+["']${SPECIFIER}["']`, - 'm' -); -const NAMESPACE_IMPORT = new RegExp( - String.raw`^\s*import\s+\*\s+as\s+\w+\s+from\s+["']${SPECIFIER}["']`, - 'm' -); +// Matches a whole import declaration for the specifier, including a +// multi-line clause list, and captures the clause before `from`. +// The clause may not contain `;` or the word `import`, so it can never span +// across an earlier import statement and capture unrelated text. The +// specifier is a literal rather than an interpolated pattern: it is a fixed +// string, and interpolation would only add needless complexity. +const IMPORT_CLAUSE = /import\s+((?:(?!\bimport\b)[^;])*?)\s+from\s+["']@actions\/core["']/g; +// A clause beginning with an identifier is a default import, e.g. +// `core` or `core, { info }`. Namespace (`* as core`) and named (`{ info }`) +// clauses start with `*` or `{` and are unaffected. +const HAS_DEFAULT_CLAUSE = /^[A-Za-z_$][\w$]*(\s*,|\s*$)/; // Parked trees are not shipped code: `.jest-skip/**` holds intentionally // disabled suites and `**/fixtures/**` holds deliberately invalid input. const SKIP_DIRS = new Set([ @@ -48,57 +44,44 @@ const SKIP_DIRS = new Set([ ]); const SCAN_ROOTS = ['scripts', 'agents']; -/** Real source files that import @actions/core, as repo-relative POSIX paths. */ -function findConsumers(dir = ROOT, found = []) { +/** Import clauses of @actions/core, per repo-relative file. */ +function findImports(dir = ROOT, found = []) { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { if (SKIP_DIRS.has(entry.name)) continue; const full = path.join(dir, entry.name); if (entry.isDirectory()) { - if (dir === ROOT ? SCAN_ROOTS.includes(entry.name) : true) findConsumers(full, found); + if (dir === ROOT ? SCAN_ROOTS.includes(entry.name) : true) findImports(full, found); } else if (/\.(js|mjs)$/.test(entry.name) && full !== __filename) { const source = fs.readFileSync(full, 'utf8'); - if (source.includes(SPECIFIER)) { - found.push({ file: path.relative(ROOT, full).split(path.sep).join('/'), source }); - } + if (!source.includes('@actions/core')) continue; + const clauses = [...source.matchAll(IMPORT_CLAUSE)].map((match) => match[1].trim()); + found.push({ file: path.relative(ROOT, full).split(path.sep).join('/'), source, clauses }); } } return found; } describe('@actions/core import form (#3561)', () => { - const consumers = findConsumers(); + const consumers = findImports(); + const imports = consumers.flatMap(({ file, clauses }) => + clauses.map((clause) => ({ file, clause })) + ); - test('real source has @actions/core consumers to guard', () => { - expect(consumers.length).toBeGreaterThan(0); + test('real source has @actions/core imports to guard', () => { + expect(imports.length).toBeGreaterThan(0); }); - test('no real source uses a default import of @actions/core', () => { - const offenders = consumers - .filter(({ source }) => DEFAULT_IMPORT.test(source)) - .map(({ file }) => file); + test('no import uses a default clause (breaks on the ESM-only release)', () => { + const offenders = imports + .filter(({ clause }) => HAS_DEFAULT_CLAUSE.test(clause)) + .map(({ file, clause }) => `${file}: ${clause.replace(/\s+/g, ' ')}`); expect(offenders).toEqual([]); }); - test('ESM consumers use the namespace form the fix relies on', () => { - const namespaceFiles = consumers.filter(({ source }) => NAMESPACE_IMPORT.test(source)); - // Guard against the form silently reverting to a named import, which is - // also unavailable once the package ships no CommonJS build. - const namedOnly = consumers - .filter(({ source }) => { - const imports = source.match( - new RegExp(String.raw`^\s*import\s+[^;\n]*from\s+["']${SPECIFIER}["']`, 'gm') - ); - return imports && imports.every((line) => !NAMESPACE_IMPORT.test(line)); - }) - .map(({ file }) => file); - expect(namespaceFiles.length).toBeGreaterThan(0); - expect(namedOnly).toEqual([]); - }); - test('the installed package exposes every member those files call', () => { const used = new Set(); for (const { source } of consumers) { - for (const match of source.matchAll(new RegExp(String.raw`\bcore\.(\w+)`, 'g'))) { + for (const match of source.matchAll(/\bcore\.(\w+)/g)) { used.add(match[1]); } } @@ -111,7 +94,7 @@ describe('@actions/core import form (#3561)', () => { [ '--input-type=module', '-e', - `const m = await import('${SPECIFIER}'); + `const m = await import('@actions/core'); const used = ${JSON.stringify([...used])}; const missing = used.filter((key) => !(key in m)); if (missing.length) { console.error('missing: ' + missing.join(', ')); process.exit(1); }