From 1b9294f9d383cf6e1c8fc5c58c9c19c4d36e38b8 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Fri, 5 Jun 2026 11:33:05 -0400 Subject: [PATCH 1/5] Add pnpm pre-ci mirroring PR CI gates, with manifest drift guard --- .github/workflows/tests-pr.yml | 12 +++++++ bin/check-ci-gates.js | 65 ++++++++++++++++++++++++++++++++++ bin/ci-gates.js | 52 +++++++++++++++++++++++++++ bin/pre-ci.js | 42 ++++++++++++++++++++++ dev.yml | 3 ++ package.json | 2 ++ 6 files changed, 176 insertions(+) create mode 100644 bin/check-ci-gates.js create mode 100644 bin/ci-gates.js create mode 100644 bin/pre-ci.js diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 5e35457511b..3609208ec54 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -316,3 +316,15 @@ jobs: run: | echo '::error::Breaking changes detected. See the sticky comment on the PR for details.' exit 1 + + ci-gate-sync: + name: 'CI gate manifest' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v4 + with: + node-version: ${{ env.DEFAULT_NODE_VERSION }} + - name: Check the CI gate manifest matches this workflow + run: node bin/check-ci-gates.js diff --git a/bin/check-ci-gates.js b/bin/check-ci-gates.js new file mode 100644 index 00000000000..569bf3b2a2b --- /dev/null +++ b/bin/check-ci-gates.js @@ -0,0 +1,65 @@ +// Drift guard: fails if the local CI-gate manifest (bin/ci-gates.js) falls out of +// sync with .github/workflows/tests-pr.yml, or if the pinned tool versions in +// dev.yml and tests-pr.yml disagree. Runs in CI (ci-gate-sync job) and locally +// (pnpm check-ci-gates, also invoked by pre-ci). +import {readFileSync} from 'node:fs' +import {fileURLToPath} from 'node:url' +import {dirname, join} from 'node:path' + +import {MANIFEST_JOB_IDS} from './ci-gates.js' + +const root = join(dirname(fileURLToPath(import.meta.url)), '..') +const read = (rel) => readFileSync(join(root, rel), 'utf8') + +const problems = [] + +// 1. Workflow job ids must exactly match the manifest job ids. +const workflow = read('.github/workflows/tests-pr.yml') +const jobsSection = workflow.slice(workflow.search(/^jobs:/m)) +const workflowJobIds = [...jobsSection.matchAll(/^ {2}([A-Za-z0-9_-]+):\s*$/gm)].map((match) => match[1]) + +const manifestSet = new Set(MANIFEST_JOB_IDS) +const workflowSet = new Set(workflowJobIds) +const missingFromManifest = workflowJobIds.filter((id) => !manifestSet.has(id)) +const staleInManifest = MANIFEST_JOB_IDS.filter((id) => !workflowSet.has(id)) + +if (missingFromManifest.length > 0) { + problems.push( + `Workflow jobs not classified in bin/ci-gates.js: ${missingFromManifest.join(', ')}.\n` + + ` Add each to CI_GATES as a 'pre-ci' gate (with a local command) or 'ci-only' (with a reason).`, + ) +} +if (staleInManifest.length > 0) { + problems.push( + `bin/ci-gates.js lists jobs absent from tests-pr.yml: ${staleInManifest.join(', ')}.\n` + + ` Remove them or fix the job id.`, + ) +} + +// 2. Pinned tool versions must agree between dev.yml and tests-pr.yml. +const devYml = read('dev.yml') +const pick = (source, regex, label) => { + const match = source.match(regex) + if (!match) problems.push(`Could not parse ${label}.`) + return match ? match[1] : null +} + +const ciNode = pick(workflow, /DEFAULT_NODE_VERSION:\s*'([^']+)'/, 'DEFAULT_NODE_VERSION in tests-pr.yml') +const devNode = pick(devYml, /node:[\s\S]*?version:\s*([0-9][\w.-]*)/, 'node version in dev.yml') +const ciPnpm = pick(workflow, /PNPM_VERSION:\s*'([^']+)'/, 'PNPM_VERSION in tests-pr.yml') +const devPnpm = pick(devYml, /package_manager:\s*pnpm@([0-9][\w.-]*)/, 'pnpm version in dev.yml') + +if (ciNode && devNode && ciNode !== devNode) { + problems.push(`Node version mismatch: dev.yml ${devNode} vs tests-pr.yml DEFAULT_NODE_VERSION ${ciNode}.`) +} +if (ciPnpm && devPnpm && ciPnpm !== devPnpm) { + problems.push(`pnpm version mismatch: dev.yml ${devPnpm} vs tests-pr.yml PNPM_VERSION ${ciPnpm}.`) +} + +if (problems.length > 0) { + console.error('CI gate manifest is out of sync with the workflow:\n') + for (const problem of problems) console.error(`- ${problem}`) + process.exit(1) +} + +console.log(`CI gate manifest in sync: ${workflowJobIds.length} workflow jobs classified; tool versions match (node ${ciNode}, pnpm ${ciPnpm}).`) diff --git a/bin/ci-gates.js b/bin/ci-gates.js new file mode 100644 index 00000000000..715f50f29a9 --- /dev/null +++ b/bin/ci-gates.js @@ -0,0 +1,52 @@ +// Single source of truth mapping every job in .github/workflows/tests-pr.yml to +// either a local `pre-ci` command (run-what-CI-runs) or an explicit reason it is +// CI-only. `bin/pre-ci.js` runs the pre-ci gates; `bin/check-ci-gates.js` asserts +// this list stays in sync with the workflow so the two cannot silently drift. +// +// `job` is the workflow job id (the key under `jobs:`), which is stable, unlike +// the rendered display name (matrix jobs interpolate `${{ ... }}`). + +export const CI_GATES = [ + // --- gates a contributor can reproduce locally before pushing --- + // Ordered as pre-ci should run them: build precedes the oclif codegen check, + // and the graphql check precedes the oclif check (their whole-repo `git status` + // asserts otherwise cross-contaminate in a single working tree). + {job: 'type-check', kind: 'pre-ci', command: 'pnpm type-check'}, + {job: 'lint', kind: 'pre-ci', command: 'pnpm lint'}, + {job: 'bundle', kind: 'pre-ci', command: 'pnpm build'}, + {job: 'knip', kind: 'pre-ci', command: 'pnpm knip'}, + {job: 'graphql-schema', kind: 'pre-ci', command: 'pnpm codegen:check:graphql'}, + {job: 'oclif-checks', kind: 'pre-ci', command: 'pnpm codegen:check:oclif'}, + {job: 'unit-tests', kind: 'pre-ci', command: 'pnpm test'}, + + // --- CI-only jobs, with the reason they are not part of pre-ci --- + { + job: 'unit-tests-gate', + kind: 'ci-only', + reason: 'Aggregation gate that only collects the unit-tests matrix results; nothing to run locally.', + }, + { + job: 'e2e-tests', + kind: 'ci-only', + reason: 'Needs Playwright browsers and real test stores/credentials; too slow and credentialed for a pre-push check.', + }, + { + job: 'type-diff', + kind: 'ci-only', + reason: 'Diffs the public type surface against the main branch; needs a base checkout, not a single local working tree.', + }, + { + job: 'major-change-check', + kind: 'ci-only', + reason: 'Breaking-change detection against the PR base; advisory and diff-based, not reproducible from one local tree.', + }, + { + job: 'ci-gate-sync', + kind: 'ci-only', + reason: 'Meta gate: runs `pnpm check-ci-gates` to keep this manifest in sync with tests-pr.yml. pre-ci runs the same check locally.', + }, +] + +export const PRE_CI_GATES = CI_GATES.filter((gate) => gate.kind === 'pre-ci') +export const CI_ONLY_GATES = CI_GATES.filter((gate) => gate.kind === 'ci-only') +export const MANIFEST_JOB_IDS = CI_GATES.map((gate) => gate.job) diff --git a/bin/pre-ci.js b/bin/pre-ci.js new file mode 100644 index 00000000000..e148a35d306 --- /dev/null +++ b/bin/pre-ci.js @@ -0,0 +1,42 @@ +// Runs the local subset of PR CI gates ("run what CI runs") so contributors can +// catch failures before pushing. The gate list and its parity with the workflow +// are defined in bin/ci-gates.js and enforced by bin/check-ci-gates.js. +// +// pre-ci mirrors CI's full (`--all`) targets so that green locally implies green +// in CI. It is intentionally slower than the affected-only `dev check`. +import {execSync} from 'node:child_process' + +import {PRE_CI_GATES, CI_ONLY_GATES} from './ci-gates.js' + +const steps = [ + {label: 'CI gate manifest in sync', command: 'pnpm check-ci-gates'}, + ...PRE_CI_GATES.map((gate) => ({label: gate.job, command: gate.command})), +] + +const results = [] +for (const step of steps) { + process.stdout.write(`\n▶ ${step.label}: ${step.command}\n`) + try { + execSync(step.command, {stdio: 'inherit'}) + results.push({...step, ok: true}) + } catch { + results.push({...step, ok: false}) + } +} + +console.log('\n──────── pre-ci summary ────────') +for (const result of results) { + console.log(`${result.ok ? '✓' : '✗'} ${result.label}`) +} + +console.log('\nNot run locally (CI-only):') +for (const gate of CI_ONLY_GATES) { + console.log(`· ${gate.job} — ${gate.reason}`) +} + +const failed = results.filter((result) => !result.ok) +if (failed.length > 0) { + console.error(`\npre-ci failed: ${failed.map((result) => result.label).join(', ')}`) + process.exit(1) +} +console.log('\npre-ci passed. Note: codegen checks regenerate files — review `git status` for any uncommitted generated changes.') diff --git a/dev.yml b/dev.yml index 6c6bffb6bfc..8f64660a55d 100644 --- a/dev.yml +++ b/dev.yml @@ -66,6 +66,9 @@ commands: type-check: desc: 'Type-check the project' run: pnpm run type-check:affected + pre-ci: + desc: 'Run the local subset of PR CI gates (run what CI runs) before pushing' + run: pnpm run pre-ci check: type-check: pnpm nx affected --target=type-check diff --git a/package.json b/package.json index ab8b564e89e..b53670b7e44 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "clean": "nx run-many --target=clean --all --skip-nx-cache && nx reset", "codegen": "pnpm graphql-codegen:get-graphql-schemas && pnpm graphql-codegen && pnpm refresh-manifests && pnpm refresh-code-documentation && pnpm build-dev-docs", "codegen:check:graphql": "pnpm graphql-codegen:get-graphql-schemas && pnpm graphql-codegen && node ./bin/check-codegen-clean.js graphql", + "check-ci-gates": "node bin/check-ci-gates.js", "codegen:check:oclif": "pnpm refresh-manifests && node ./bin/check-codegen-clean.js oclif:manifests && pnpm refresh-readme && node ./bin/check-codegen-clean.js oclif:readme && pnpm refresh-code-documentation && node ./bin/check-codegen-clean.js oclif:code-docs && pnpm build-dev-docs && node ./bin/check-codegen-clean.js oclif:dev-docs", "create-app": "nx build create-app && node packages/create-app/bin/dev.js --package-manager pnpm", "deploy-experimental": "node bin/deploy-experimental.js", @@ -29,6 +30,7 @@ "refresh-readme": "nx run-many --target=refresh-readme --all --skip-nx-cache", "release": "./bin/release", "post-release": "./bin/post-release", + "pre-ci": "node bin/pre-ci.js", "update-observe": "node bin/update-observe.js", "shopify:run": "node packages/cli/bin/dev.js", "shopify": "nx build cli && node packages/cli/bin/dev.js", From 13963489007829d0c9f977d71b30ff614f7f7d16 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Mon, 8 Jun 2026 10:17:49 -0400 Subject: [PATCH 2/5] Tighten dev.yml node version parse to the node block's child line --- bin/check-ci-gates.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/check-ci-gates.js b/bin/check-ci-gates.js index 569bf3b2a2b..a2d42321ab1 100644 --- a/bin/check-ci-gates.js +++ b/bin/check-ci-gates.js @@ -45,7 +45,7 @@ const pick = (source, regex, label) => { } const ciNode = pick(workflow, /DEFAULT_NODE_VERSION:\s*'([^']+)'/, 'DEFAULT_NODE_VERSION in tests-pr.yml') -const devNode = pick(devYml, /node:[\s\S]*?version:\s*([0-9][\w.-]*)/, 'node version in dev.yml') +const devNode = pick(devYml, /node:\s*\n\s+version:\s*([0-9][\w.-]*)/, 'node version in dev.yml') const ciPnpm = pick(workflow, /PNPM_VERSION:\s*'([^']+)'/, 'PNPM_VERSION in tests-pr.yml') const devPnpm = pick(devYml, /package_manager:\s*pnpm@([0-9][\w.-]*)/, 'pnpm version in dev.yml') From 26a4056358dc67c8f44af9346d5d6e17f5268156 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Mon, 8 Jun 2026 10:58:07 -0400 Subject: [PATCH 3/5] Harden the CI-gate guard parsing and add node:test coverage --- .github/workflows/tests-pr.yml | 2 + bin/check-ci-gates.js | 117 +++++++++++++++++++-------------- bin/check-ci-gates.test.js | 54 +++++++++++++++ 3 files changed, 125 insertions(+), 48 deletions(-) create mode 100644 bin/check-ci-gates.test.js diff --git a/.github/workflows/tests-pr.yml b/.github/workflows/tests-pr.yml index 3609208ec54..0a805cf8a7d 100644 --- a/.github/workflows/tests-pr.yml +++ b/.github/workflows/tests-pr.yml @@ -326,5 +326,7 @@ jobs: - uses: actions/setup-node@v4 with: node-version: ${{ env.DEFAULT_NODE_VERSION }} + - name: Test the gate checker + run: node --test bin/check-ci-gates.test.js - name: Check the CI gate manifest matches this workflow run: node bin/check-ci-gates.js diff --git a/bin/check-ci-gates.js b/bin/check-ci-gates.js index a2d42321ab1..98fa0101ca6 100644 --- a/bin/check-ci-gates.js +++ b/bin/check-ci-gates.js @@ -1,65 +1,86 @@ // Drift guard: fails if the local CI-gate manifest (bin/ci-gates.js) falls out of // sync with .github/workflows/tests-pr.yml, or if the pinned tool versions in -// dev.yml and tests-pr.yml disagree. Runs in CI (ci-gate-sync job) and locally -// (pnpm check-ci-gates, also invoked by pre-ci). +// dev.yml and tests-pr.yml disagree. Kept dependency-free (no YAML library) so the +// CI job can run on bare Node; the parsing below is hardened for the formats these +// two files actually use. import {readFileSync} from 'node:fs' -import {fileURLToPath} from 'node:url' +import {fileURLToPath, pathToFileURL} from 'node:url' import {dirname, join} from 'node:path' import {MANIFEST_JOB_IDS} from './ci-gates.js' -const root = join(dirname(fileURLToPath(import.meta.url)), '..') -const read = (rel) => readFileSync(join(root, rel), 'utf8') +// Job ids are the keys directly under `jobs:`. Bound the search to the jobs block +// (up to the next top-level key) and allow a trailing comment after the id. Job +// keys are always bare (their mapping is on following lines), so nested keys — +// indented deeper than 2 spaces — and `key: value` anchors are naturally excluded. +export function parseJobIds(workflow) { + const jobsAt = workflow.search(/^jobs:/m) + if (jobsAt === -1) return [] + const afterHeader = workflow.slice(jobsAt).replace(/^jobs:.*\n/, '') + const nextTopLevel = afterHeader.search(/^\S/m) + const block = nextTopLevel === -1 ? afterHeader : afterHeader.slice(0, nextTopLevel) + return [...block.matchAll(/^ {2}([A-Za-z0-9_-]+):[ \t]*(?:#.*)?$/gm)].map((match) => match[1]) +} -const problems = [] +// Pure and testable: given the two YAML texts and the manifest job ids, return the +// list of human-readable problems (empty when everything is in sync). +export function findProblems({workflow, devYml, manifestJobIds}) { + const problems = [] -// 1. Workflow job ids must exactly match the manifest job ids. -const workflow = read('.github/workflows/tests-pr.yml') -const jobsSection = workflow.slice(workflow.search(/^jobs:/m)) -const workflowJobIds = [...jobsSection.matchAll(/^ {2}([A-Za-z0-9_-]+):\s*$/gm)].map((match) => match[1]) + const workflowJobIds = parseJobIds(workflow) + const manifestSet = new Set(manifestJobIds) + const workflowSet = new Set(workflowJobIds) + const missingFromManifest = workflowJobIds.filter((id) => !manifestSet.has(id)) + const staleInManifest = manifestJobIds.filter((id) => !workflowSet.has(id)) -const manifestSet = new Set(MANIFEST_JOB_IDS) -const workflowSet = new Set(workflowJobIds) -const missingFromManifest = workflowJobIds.filter((id) => !manifestSet.has(id)) -const staleInManifest = MANIFEST_JOB_IDS.filter((id) => !workflowSet.has(id)) + if (missingFromManifest.length > 0) { + problems.push( + `Workflow jobs not classified in bin/ci-gates.js: ${missingFromManifest.join(', ')}.\n` + + ` Add each to CI_GATES as a 'pre-ci' gate (with a local command) or 'ci-only' (with a reason).`, + ) + } + if (staleInManifest.length > 0) { + problems.push(`bin/ci-gates.js lists jobs absent from tests-pr.yml: ${staleInManifest.join(', ')}.`) + } -if (missingFromManifest.length > 0) { - problems.push( - `Workflow jobs not classified in bin/ci-gates.js: ${missingFromManifest.join(', ')}.\n` + - ` Add each to CI_GATES as a 'pre-ci' gate (with a local command) or 'ci-only' (with a reason).`, - ) -} -if (staleInManifest.length > 0) { - problems.push( - `bin/ci-gates.js lists jobs absent from tests-pr.yml: ${staleInManifest.join(', ')}.\n` + - ` Remove them or fix the job id.`, - ) -} + const pick = (source, regex, label) => { + const match = source.match(regex) + if (!match) problems.push(`Could not read ${label}.`) + return match ? match[1] : undefined + } + const ciNode = pick(workflow, /DEFAULT_NODE_VERSION:\s*['"]?([0-9][\w.-]*)/, 'DEFAULT_NODE_VERSION from tests-pr.yml') + const ciPnpm = pick(workflow, /PNPM_VERSION:\s*['"]?([0-9][\w.-]*)/, 'PNPM_VERSION from tests-pr.yml') + // dev.yml pins these on the `version:`/`package_manager:` lines under the `node:` step. + const devNode = pick(devYml, /node:\s*\n\s+version:\s*['"]?([0-9][\w.-]*)/, 'the node version from dev.yml') + const devPnpm = pick(devYml, /package_manager:\s*['"]?pnpm@([0-9][\w.-]*)/, 'the pnpm version from dev.yml') -// 2. Pinned tool versions must agree between dev.yml and tests-pr.yml. -const devYml = read('dev.yml') -const pick = (source, regex, label) => { - const match = source.match(regex) - if (!match) problems.push(`Could not parse ${label}.`) - return match ? match[1] : null + if (ciNode && devNode && ciNode !== devNode) { + problems.push(`Node version mismatch: dev.yml ${devNode} vs tests-pr.yml DEFAULT_NODE_VERSION ${ciNode}.`) + } + if (ciPnpm && devPnpm && ciPnpm !== devPnpm) { + problems.push(`pnpm version mismatch: dev.yml ${devPnpm} vs tests-pr.yml PNPM_VERSION ${ciPnpm}.`) + } + + return {problems, workflowJobIds, ciNode, ciPnpm} } -const ciNode = pick(workflow, /DEFAULT_NODE_VERSION:\s*'([^']+)'/, 'DEFAULT_NODE_VERSION in tests-pr.yml') -const devNode = pick(devYml, /node:\s*\n\s+version:\s*([0-9][\w.-]*)/, 'node version in dev.yml') -const ciPnpm = pick(workflow, /PNPM_VERSION:\s*'([^']+)'/, 'PNPM_VERSION in tests-pr.yml') -const devPnpm = pick(devYml, /package_manager:\s*pnpm@([0-9][\w.-]*)/, 'pnpm version in dev.yml') +// Run as a CLI when invoked directly (not when imported by a test). +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const root = join(dirname(fileURLToPath(import.meta.url)), '..') + const read = (rel) => readFileSync(join(root, rel), 'utf8') -if (ciNode && devNode && ciNode !== devNode) { - problems.push(`Node version mismatch: dev.yml ${devNode} vs tests-pr.yml DEFAULT_NODE_VERSION ${ciNode}.`) -} -if (ciPnpm && devPnpm && ciPnpm !== devPnpm) { - problems.push(`pnpm version mismatch: dev.yml ${devPnpm} vs tests-pr.yml PNPM_VERSION ${ciPnpm}.`) -} + const {problems, workflowJobIds, ciNode, ciPnpm} = findProblems({ + workflow: read('.github/workflows/tests-pr.yml'), + devYml: read('dev.yml'), + manifestJobIds: MANIFEST_JOB_IDS, + }) -if (problems.length > 0) { - console.error('CI gate manifest is out of sync with the workflow:\n') - for (const problem of problems) console.error(`- ${problem}`) - process.exit(1) + if (problems.length > 0) { + console.error('CI gate manifest is out of sync with the workflow:\n') + for (const problem of problems) console.error(`- ${problem}`) + process.exit(1) + } + console.log( + `CI gate manifest in sync: ${workflowJobIds.length} workflow jobs classified; tool versions match (node ${ciNode}, pnpm ${ciPnpm}).`, + ) } - -console.log(`CI gate manifest in sync: ${workflowJobIds.length} workflow jobs classified; tool versions match (node ${ciNode}, pnpm ${ciPnpm}).`) diff --git a/bin/check-ci-gates.test.js b/bin/check-ci-gates.test.js new file mode 100644 index 00000000000..72a838ce747 --- /dev/null +++ b/bin/check-ci-gates.test.js @@ -0,0 +1,54 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import {findProblems, parseJobIds} from './check-ci-gates.js' + +const workflow = (jobIds, {node = '26.1.0', pnpm = '10.11.1'} = {}) => + `name: tests\non: pull_request\nenv:\n DEFAULT_NODE_VERSION: '${node}'\n PNPM_VERSION: '${pnpm}'\njobs:\n` + + jobIds.map((id) => ` ${id}:\n runs-on: ubuntu-latest\n steps: []`).join('\n') + + '\n' + +const devYml = ({node = '26.1.0', pnpm = '10.11.1'} = {}) => + `name: cli\nup:\n - node:\n version: ${node}\n package_manager: pnpm@${pnpm}\n - packages:\n - jq\n` + +test('in sync: no problems', () => { + const {problems} = findProblems({workflow: workflow(['a', 'b']), devYml: devYml(), manifestJobIds: ['a', 'b']}) + assert.deepEqual(problems, []) +}) + +test('workflow job missing from the manifest', () => { + const {problems} = findProblems({workflow: workflow(['a', 'b', 'c']), devYml: devYml(), manifestJobIds: ['a', 'b']}) + assert.match(problems.join('\n'), /not classified.*\bc\b/) +}) + +test('manifest lists a job absent from the workflow', () => { + const {problems} = findProblems({workflow: workflow(['a']), devYml: devYml(), manifestJobIds: ['a', 'b']}) + assert.match(problems.join('\n'), /absent from tests-pr\.yml.*\bb\b/) +}) + +test('node version mismatch is detected', () => { + const {problems} = findProblems({workflow: workflow(['a'], {node: '25.0.0'}), devYml: devYml({node: '26.1.0'}), manifestJobIds: ['a']}) + assert.match(problems.join('\n'), /Node version mismatch/) +}) + +test('pnpm version mismatch is detected', () => { + const {problems} = findProblems({workflow: workflow(['a']), devYml: devYml({pnpm: '9.0.0'}), manifestJobIds: ['a']}) + assert.match(problems.join('\n'), /pnpm version mismatch/) +}) + +test('a missing version pin is reported, not silently passed', () => { + const noEnv = `name: t\non: pull_request\njobs:\n a:\n runs-on: ubuntu-latest\n steps: []\n` + const {problems} = findProblems({workflow: noEnv, devYml: devYml(), manifestJobIds: ['a']}) + assert.match(problems.join('\n'), /Could not read DEFAULT_NODE_VERSION/) +}) + +// Hardening: tolerate a trailing comment after the job id, and ignore deeper-indented +// keys, blank lines, comment lines, and any top-level section after `jobs:`. +test('parseJobIds tolerates comments and ignores non-job lines', () => { + const wf = `env:\n DEFAULT_NODE_VERSION: '26.1.0'\njobs:\n build: # freshness gate\n runs-on: ubuntu-latest\n env:\n NESTED: 1\n\n # a comment line\n test-job:\n steps: []\nconcurrency:\n group: x\n` + assert.deepEqual(parseJobIds(wf), ['build', 'test-job']) +}) + +test('parseJobIds returns empty when there is no jobs block', () => { + assert.deepEqual(parseJobIds('name: t\non: push\n'), []) +}) From 3950431e797dadb571d09c4e8f801d67ebf5e6bb Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Mon, 8 Jun 2026 11:14:35 -0400 Subject: [PATCH 4/5] Normalize CRLF in the gate guard parsing; cover it with a test --- bin/check-ci-gates.js | 9 +++++++-- bin/check-ci-gates.test.js | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/bin/check-ci-gates.js b/bin/check-ci-gates.js index 98fa0101ca6..fd907817210 100644 --- a/bin/check-ci-gates.js +++ b/bin/check-ci-gates.js @@ -13,7 +13,8 @@ import {MANIFEST_JOB_IDS} from './ci-gates.js' // (up to the next top-level key) and allow a trailing comment after the id. Job // keys are always bare (their mapping is on following lines), so nested keys — // indented deeper than 2 spaces — and `key: value` anchors are naturally excluded. -export function parseJobIds(workflow) { +export function parseJobIds(workflowText) { + const workflow = workflowText.replace(/\r\n/g, '\n') const jobsAt = workflow.search(/^jobs:/m) if (jobsAt === -1) return [] const afterHeader = workflow.slice(jobsAt).replace(/^jobs:.*\n/, '') @@ -24,9 +25,13 @@ export function parseJobIds(workflow) { // Pure and testable: given the two YAML texts and the manifest job ids, return the // list of human-readable problems (empty when everything is in sync). -export function findProblems({workflow, devYml, manifestJobIds}) { +export function findProblems({workflow: workflowText, devYml: devYmlText, manifestJobIds}) { const problems = [] + // Normalize line endings so the parsing below is robust to CRLF working trees. + const workflow = workflowText.replace(/\r\n/g, '\n') + const devYml = devYmlText.replace(/\r\n/g, '\n') + const workflowJobIds = parseJobIds(workflow) const manifestSet = new Set(manifestJobIds) const workflowSet = new Set(workflowJobIds) diff --git a/bin/check-ci-gates.test.js b/bin/check-ci-gates.test.js index 72a838ce747..fc48758e5e5 100644 --- a/bin/check-ci-gates.test.js +++ b/bin/check-ci-gates.test.js @@ -52,3 +52,10 @@ test('parseJobIds tolerates comments and ignores non-job lines', () => { test('parseJobIds returns empty when there is no jobs block', () => { assert.deepEqual(parseJobIds('name: t\non: push\n'), []) }) + +test('CRLF line endings are handled', () => { + const wf = workflow(['a', 'b']).replace(/\n/g, '\r\n') + assert.deepEqual(parseJobIds(wf), ['a', 'b']) + const {problems} = findProblems({workflow: wf, devYml: devYml().replace(/\n/g, '\r\n'), manifestJobIds: ['a', 'b']}) + assert.deepEqual(problems, []) +}) From 6a635d0e9b855af21b0518e4a5b7cb168cf72806 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Mon, 8 Jun 2026 15:30:54 -0400 Subject: [PATCH 5/5] Use synthetic version fixtures in the gate-guard test (not the repo's real pins) --- bin/check-ci-gates.test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bin/check-ci-gates.test.js b/bin/check-ci-gates.test.js index fc48758e5e5..21df4c48c6f 100644 --- a/bin/check-ci-gates.test.js +++ b/bin/check-ci-gates.test.js @@ -3,12 +3,14 @@ import test from 'node:test' import {findProblems, parseJobIds} from './check-ci-gates.js' -const workflow = (jobIds, {node = '26.1.0', pnpm = '10.11.1'} = {}) => +// Versions below are arbitrary fixtures, not the repo's real pins — the guard +// reads those from dev.yml / tests-pr.yml at runtime, never hard-coded. +const workflow = (jobIds, {node = '1.2.3', pnpm = '4.5.6'} = {}) => `name: tests\non: pull_request\nenv:\n DEFAULT_NODE_VERSION: '${node}'\n PNPM_VERSION: '${pnpm}'\njobs:\n` + jobIds.map((id) => ` ${id}:\n runs-on: ubuntu-latest\n steps: []`).join('\n') + '\n' -const devYml = ({node = '26.1.0', pnpm = '10.11.1'} = {}) => +const devYml = ({node = '1.2.3', pnpm = '4.5.6'} = {}) => `name: cli\nup:\n - node:\n version: ${node}\n package_manager: pnpm@${pnpm}\n - packages:\n - jq\n` test('in sync: no problems', () => { @@ -27,7 +29,7 @@ test('manifest lists a job absent from the workflow', () => { }) test('node version mismatch is detected', () => { - const {problems} = findProblems({workflow: workflow(['a'], {node: '25.0.0'}), devYml: devYml({node: '26.1.0'}), manifestJobIds: ['a']}) + const {problems} = findProblems({workflow: workflow(['a'], {node: '9.9.9'}), devYml: devYml({node: '1.2.3'}), manifestJobIds: ['a']}) assert.match(problems.join('\n'), /Node version mismatch/) }) @@ -45,7 +47,7 @@ test('a missing version pin is reported, not silently passed', () => { // Hardening: tolerate a trailing comment after the job id, and ignore deeper-indented // keys, blank lines, comment lines, and any top-level section after `jobs:`. test('parseJobIds tolerates comments and ignores non-job lines', () => { - const wf = `env:\n DEFAULT_NODE_VERSION: '26.1.0'\njobs:\n build: # freshness gate\n runs-on: ubuntu-latest\n env:\n NESTED: 1\n\n # a comment line\n test-job:\n steps: []\nconcurrency:\n group: x\n` + const wf = `env:\n DEFAULT_NODE_VERSION: '1.2.3'\njobs:\n build: # freshness gate\n runs-on: ubuntu-latest\n env:\n NESTED: 1\n\n # a comment line\n test-job:\n steps: []\nconcurrency:\n group: x\n` assert.deepEqual(parseJobIds(wf), ['build', 'test-job']) })