From 8745db45e7ef56892c61874e7a2757b2c1e3d342 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:40:19 +0800 Subject: [PATCH] =?UTF-8?q?ci:=20validate=20on=20the=20runtime=20we=20publ?= =?UTF-8?q?ish=20from=20=E2=80=94=20pin=20every=20workflow=20to=20Node=202?= =?UTF-8?q?2=20(#3825)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI ran two Node versions at once, and nobody had decided that. All 12 PR gates — Build Core, Test Core, TypeScript Type Check, Dogfood, ESLint, spec liveness, dep validation — were on Node 20, EOL since 2026-04-30, while release.yml, publish-smoke.yml, scaffold-e2e.yml and showcase-smoke.yml were on 22. Code was verified on one runtime and shipped from another, and the runtime guarding every merge no longer received security patches. The split was drift, not policy. release.yml carried the receipt in a comment — "22 (not 20 like the other workflows)" — because a downstream clone pins engines.node >=22 and pnpm aborted on 20. One workflow got bumped to clear one error; the other twelve stayed behind. Node 20 was in fact used for nothing else: every shipped Docker image is already node:22-slim. It surfaced only by accident in #3812, where a test imported better-sqlite3@13 (engines >=22). engines is a declaration, not enforcement, so it loaded on 20 and then killed the vitest worker with a process-level abort — no JS error, so the suite reported "Test Files 22 passed (23)" while 17 cases silently never ran. A green check that had stopped running the tests. All 18 setup-node steps now run Node 22, and quoting is normalised (lint.yml used "20", showcase-smoke.yml a bare 22, so a node-version: '\d+' grep missed both). .nvmrc becomes the single source of truth. It pins contributors' local runtime via nvm use — there was no pin at all before, so a contributor on Node 24 could not reproduce a Node 20 gate failure — and the new check:node-version holds every workflow to it. A version pin is otherwise 18 independent string literals, which is why this drifted unseen; bumping Node is now a one-line .nvmrc edit plus whatever the guard reports. The guard also fails a setup-node step that pins nothing (silently inheriting the runner default) and one that resolves its version from an unevaluatable expression. It lives in lint.yml's ESLint job, which carries no paths filter, so it cannot go dormant. engines.node stays >=18.0.0 across all 49 packages: that is a promise to users about what ships, independent of what CI validates on, and tightening it is a breaking change left for its own decision. Co-Authored-By: Claude --- .changeset/ci-node-22-pin.md | 39 +++++ .github/workflows/ci.yml | 10 +- .github/workflows/coverage-nightly.yml | 2 +- .github/workflows/docs-drift-check.yml | 2 +- .github/workflows/lint.yml | 17 ++- .github/workflows/pr-automation.yml | 2 +- .github/workflows/release.yml | 6 +- .github/workflows/showcase-smoke.yml | 2 +- .github/workflows/spec-liveness-check.yml | 2 +- .github/workflows/validate-deps.yml | 2 +- .nvmrc | 1 + package.json | 3 +- scripts/check-node-version.mjs | 164 ++++++++++++++++++++++ 13 files changed, 235 insertions(+), 17 deletions(-) create mode 100644 .changeset/ci-node-22-pin.md create mode 100644 .nvmrc create mode 100644 scripts/check-node-version.mjs diff --git a/.changeset/ci-node-22-pin.md b/.changeset/ci-node-22-pin.md new file mode 100644 index 0000000000..cedfdfec61 --- /dev/null +++ b/.changeset/ci-node-22-pin.md @@ -0,0 +1,39 @@ +--- +--- + +ci: validate on the runtime we publish from — pin every workflow to Node 22 (#3825) + +CI ran two Node versions at once, and nobody had decided that. All 12 PR gates — +Build Core, Test Core, TypeScript Type Check, Dogfood, ESLint, spec liveness, +dep validation — were on **Node 20**, which reached EOL on **2026-04-30**, while +`release.yml`, `publish-smoke.yml`, `scaffold-e2e.yml` and `showcase-smoke.yml` +were on **22**. So code was verified on one runtime and shipped from another, +and the runtime guarding every merge no longer received security patches. + +The split was drift, not policy. `release.yml` carried the receipt in a comment +— *"22 (not 20 like the other workflows)"* — because a downstream clone pinned +`engines.node >=22` and pnpm aborted on 20. One workflow got bumped to clear one +error; the other twelve stayed behind, and nothing in CI could see the gap. + +It surfaced only by accident in #3812: a test imported `better-sqlite3@13`, +whose `engines` say `>=22`. `engines` is a declaration, not enforcement, so it +loaded on Node 20 and then killed the vitest worker with a **process-level +abort** — no JS error, so the suite reported `Test Files 22 passed (23)` while +**17 cases silently never ran**. A green check that had quietly stopped running +the tests. + +All 18 `setup-node` steps now run Node 22, matching what release already used. + +**`.nvmrc` is now the single source of truth.** It pins contributors' local +runtime via `nvm use` — previously there was no pin at all, so a contributor on +Node 24 could not reproduce a Node 20 gate failure — and `check:node-version` +(new, wired into the unfiltered, always-required `lint` job) holds every +workflow to it. A version pin is otherwise 18 independent string literals, which +is why this drifted invisibly for so long; bumping Node is now a one-line edit +to `.nvmrc` plus whatever the guard reports. The guard also fails a `setup-node` +step that pins *nothing*, which would silently inherit the runner default. + +Nothing about the published packages changes: `engines.node` stays `>=18.0.0` +across all 49 of them. That is a promise to users about what ships, independent +of what CI validates on, and tightening it is a breaking change — left for its +own decision rather than folded in here. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4102eaf029..1fcac697c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,7 +90,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable @@ -192,7 +192,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable @@ -319,7 +319,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable @@ -433,7 +433,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable @@ -491,7 +491,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.github/workflows/coverage-nightly.yml b/.github/workflows/coverage-nightly.yml index cc099feef2..683b0c1e09 100644 --- a/.github/workflows/coverage-nightly.yml +++ b/.github/workflows/coverage-nightly.yml @@ -26,7 +26,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.github/workflows/docs-drift-check.yml b/.github/workflows/docs-drift-check.yml index fc6c5f40cf..03d2cd059e 100644 --- a/.github/workflows/docs-drift-check.yml +++ b/.github/workflows/docs-drift-check.yml @@ -29,7 +29,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Fetch base branch run: git fetch --no-tags origin "${{ github.base_ref }}" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d9198b33ad..a4fcbafe29 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -30,7 +30,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: "20" + node-version: '22' - name: Enable Corepack run: corepack enable @@ -106,6 +106,19 @@ jobs: - name: Release-notes drift guard run: pnpm check:release-notes + # #3825 Node-version drift guard: a runtime pin is 18 separate string + # literals across .github/workflows, so a split is invisible until someone + # greps for it. One did open — every PR gate sat on Node 20 (EOL + # 2026-04-30) while release.yml and publish-smoke.yml ran 22, so code was + # verified on one runtime and shipped from another. Nobody chose that; one + # workflow got bumped to clear one error and the rest stayed behind. It + # surfaced only when a dependency needing >=22 aborted the vitest worker + # at the process level, which vitest reported as a PASSING suite with 17 + # cases silently skipped (#3812). .nvmrc is now the single source of + # truth, and this holds every workflow to it. + - name: Node-version drift guard + run: pnpm check:node-version + typecheck: name: TypeScript Type Check runs-on: ubuntu-latest @@ -119,7 +132,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.github/workflows/pr-automation.yml b/.github/workflows/pr-automation.yml index c961a31293..face29ce72 100644 --- a/.github/workflows/pr-automation.yml +++ b/.github/workflows/pr-automation.yml @@ -63,7 +63,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d5e3ae68c6..9ae76a8bd3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -24,9 +24,9 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - # 22 (not 20 like the other workflows): the downstream hotcrm smoke - # below clones hotcrm@v1.2.0, whose manifest pins engines.node >=22. - # pnpm install aborts with ERR_PNPM_UNSUPPORTED_ENGINE on Node 20. + # Cannot go below 22: the downstream hotcrm smoke below clones + # hotcrm@v1.2.0, whose manifest pins engines.node >=22. pnpm install + # aborts with ERR_PNPM_UNSUPPORTED_ENGINE under that. node-version: '22' - name: Enable Corepack diff --git a/.github/workflows/showcase-smoke.yml b/.github/workflows/showcase-smoke.yml index 7d9d515dbf..eed1fb2118 100644 --- a/.github/workflows/showcase-smoke.yml +++ b/.github/workflows/showcase-smoke.yml @@ -22,7 +22,7 @@ jobs: - uses: pnpm/action-setup@v6 - uses: actions/setup-node@v7 with: - node-version: 22 + node-version: '22' cache: pnpm - run: pnpm install --frozen-lockfile # The smoke's webServer runs `os serve --dev`, which loads the showcase diff --git a/.github/workflows/spec-liveness-check.yml b/.github/workflows/spec-liveness-check.yml index 4961127b3c..efb23919af 100644 --- a/.github/workflows/spec-liveness-check.yml +++ b/.github/workflows/spec-liveness-check.yml @@ -33,7 +33,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.github/workflows/validate-deps.yml b/.github/workflows/validate-deps.yml index d0729d35aa..5adbef38b5 100644 --- a/.github/workflows/validate-deps.yml +++ b/.github/workflows/validate-deps.yml @@ -32,7 +32,7 @@ jobs: - name: Setup Node.js uses: actions/setup-node@v7 with: - node-version: '20' + node-version: '22' - name: Enable Corepack run: corepack enable diff --git a/.nvmrc b/.nvmrc new file mode 100644 index 0000000000..2bd5a0a98a --- /dev/null +++ b/.nvmrc @@ -0,0 +1 @@ +22 diff --git a/package.json b/package.json index aab194ccf1..6167fe0ddc 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,8 @@ "check:org-identifier": "node scripts/check-org-identifier.mjs", "check:authz-resolver": "node scripts/check-single-authz-resolver.mjs", "check:console-sha": "node scripts/check-console-sha.mjs", - "check:release-notes": "node scripts/check-release-notes.mjs" + "check:release-notes": "node scripts/check-release-notes.mjs", + "check:node-version": "node scripts/check-node-version.mjs" }, "keywords": [ "objectstack", diff --git a/scripts/check-node-version.mjs b/scripts/check-node-version.mjs new file mode 100644 index 0000000000..3a4717359d --- /dev/null +++ b/scripts/check-node-version.mjs @@ -0,0 +1,164 @@ +#!/usr/bin/env node +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// check-node-version -- every workflow must run the Node version in .nvmrc. +// +// Before #3825 the repo ran two Node versions at once, and nobody had decided +// that: all 12 PR gates were on Node 20 while release.yml, publish-smoke.yml, +// scaffold-e2e.yml and showcase-smoke.yml were on 22. So code was verified on +// one runtime and shipped from another, and the verifying one had been EOL +// since 2026-04-30 -- no security patches on the runtime guarding every merge. +// +// The split was never a policy, it was drift. release.yml carried the receipt +// in a comment: "22 (not 20 like the other workflows)" because a downstream +// clone pinned engines.node >=22 and pnpm aborted on 20. One workflow got +// bumped to clear one error; the other twelve stayed behind. That is how the +// gates-vs-release gap opened, and nothing in CI could see it -- a version pin +// is 18 independent string literals, so drift is invisible until someone greps. +// +// It surfaced only by accident (#3812): a test imported better-sqlite3@13, +// whose engines say >=22. `engines` is a declaration, not enforcement, so it +// loaded on 20 and then killed the vitest worker with a process-level abort -- +// no JS error, so the suite reported "22 passed (23)" while 17 cases silently +// never ran. A green check that had stopped running the tests. +// +// node scripts/check-node-version.mjs +// +// .nvmrc is the single source of truth: it pins contributors' local runtime via +// `nvm use` AND is what this guard holds every workflow to. Bumping Node is +// therefore a one-line edit to .nvmrc plus whatever this guard then reports. +// +// Deliberately NOT checked: `engines.node` in package.json. That is a promise +// to users about what the published packages support, which is independent of +// what CI validates on, and tightening it is a breaking change. See #3825. + +import { execFileSync } from 'node:child_process'; +import { readFileSync, readdirSync } from 'node:fs'; +import { join } from 'node:path'; + +const WORKFLOW_DIR = '.github/workflows'; +const PIN_FILE = '.nvmrc'; + +const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', +}).trim(); + +// The pin, e.g. "22". Tolerates the "v22" and "lts/jod" forms nvm also accepts, +// but this repo writes the bare major -- that is what setup-node wants too. +const pin = readFileSync(join(root, PIN_FILE), 'utf8').trim(); +if (!pin) { + console.error(`check-node-version: ${PIN_FILE} is empty -- it must pin a Node major, e.g. 22.`); + process.exit(1); +} + +const files = readdirSync(join(root, WORKFLOW_DIR)) + .filter((f) => f.endsWith('.yml') || f.endsWith('.yaml')) + .sort(); + +// A step ends at the next YAML list item; `with:` keys live between the +// `uses: actions/setup-node` line and that boundary. +const SETUP_NODE = /^\s*(?:-\s+)?uses:\s*actions\/setup-node@/; +const NEXT_ITEM = /^\s*-\s/; +const NODE_VERSION = /^\s*node-version:\s*(.+?)\s*$/; +const NODE_VERSION_FILE = /^\s*node-version-file:\s*(.+?)\s*$/; + +const unquote = (v) => v.replace(/^['"]|['"]$/g, '').trim(); + +const offenders = []; +let steps = 0; + +for (const file of files) { + const lines = readFileSync(join(root, WORKFLOW_DIR, file), 'utf8').split('\n'); + for (let i = 0; i < lines.length; i++) { + if (!SETUP_NODE.test(lines[i])) continue; + steps++; + + let found = null; + for (let j = i + 1; j < lines.length; j++) { + // Stop at the next step -- but not on the `- uses:` line we started from. + if (NEXT_ITEM.test(lines[j])) break; + const v = lines[j].match(NODE_VERSION); + if (v) { + found = { line: j + 1, kind: 'node-version', value: unquote(v[1]) }; + break; + } + const f = lines[j].match(NODE_VERSION_FILE); + if (f) { + found = { line: j + 1, kind: 'node-version-file', value: unquote(f[1]) }; + break; + } + } + + const where = `${WORKFLOW_DIR}/${file}`; + if (!found) { + // No pin at all: the step silently inherits whatever Node the runner + // image ships, which GitHub bumps without telling us. + offenders.push({ + where: `${where}:${i + 1}`, + problem: 'declares no Node version -- inherits the runner default', + fix: `add "node-version: '${pin}'"`, + }); + continue; + } + + if (found.kind === 'node-version-file') { + // Pointing at the pin file is the ideal form; anything else is a second + // source of truth. + if (found.value.replace(/^\.\//, '') !== PIN_FILE) { + offenders.push({ + where: `${where}:${found.line}`, + problem: `reads its version from "${found.value}", not ${PIN_FILE}`, + fix: `use "node-version-file: ${PIN_FILE}"`, + }); + } + continue; + } + + // A `${{ }}` expression (matrix input, env, workflow input) cannot be + // resolved from the file, so the guard cannot tell 22 from 20 here. Fail + // closed and say why, rather than either waving it through or reporting the + // raw expression as if it were a version number. A deliberate multi-version + // compatibility matrix is a real thing to want -- it just needs deciding + // out loud, since it is exactly the gates-vs-release split done on purpose. + if (found.value.includes('${{')) { + offenders.push({ + where: `${where}:${found.line}`, + problem: `resolves its version from an expression (${found.value}) that this guard cannot evaluate`, + fix: `pin it literally as '${pin}', or extend this guard if a multi-version matrix is intended`, + }); + continue; + } + + if (found.value !== pin) { + offenders.push({ + where: `${where}:${found.line}`, + problem: `pins Node ${found.value}, but ${PIN_FILE} says ${pin}`, + fix: `change it to '${pin}', or bump ${PIN_FILE} if the whole repo should move`, + }); + } + } +} + +if (offenders.length === 0) { + console.log( + `check-node-version: OK (${steps} setup-node step(s) across ${files.length} workflow(s), all on Node ${pin}).`, + ); + process.exit(0); +} + +const plural = offenders.length === 1 ? 'step disagrees' : 'steps disagree'; +console.error(`check-node-version: ${offenders.length} setup-node ${plural} with ${PIN_FILE} (Node ${pin})\n`); +for (const o of offenders) { + console.error(` • ${o.where} -- ${o.problem}`); + console.error(` ${o.fix}`); +} +console.error(` +Every workflow must run the Node version in ${PIN_FILE}, so that what CI +verifies is what release publishes from. When those drift apart, PR gates +validate code on a runtime nothing ships from -- and a dependency that needs the +newer one can abort the test worker mid-run, which vitest reports as a PASSING +suite with silently missing cases (#3812). + +To move the whole repo to a new Node version, edit ${PIN_FILE} and then update +every step this guard lists.`); +process.exit(1);