From 89f0e8d7dcb993830b76b936f27956cf124b6484 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 20 Jul 2026 21:23:28 -0700 Subject: [PATCH] fix(test): enforce the Node version pin on every test run (#7619) engine-strict (#7613) only fires during npm install/ci, not on a subsequent npm run against already-installed node_modules -- a node_modules built while on the pinned Node, followed by simply switching the active `node` (nvm/homebrew default change) with no reinstall, sails straight past engine-strict on every later npm run. Confirmed directly: npm run engines-nvmrc:check succeeded on Node 26 despite engine-strict=true and a tightened engines.node. Add scripts/check-node-version.mjs, checking process.version against root package.json's engines.node with a clear, actionable message. Wire it as a pretest*/preui:test hook on test, test:ci, test:coverage, test:workers, and ui:test -- the commands that actually run vitest -- so each fails immediately, before doing any real work, regardless of node_modules' install state. Verified directly on Node 26: npm test, npm run test:ci, and npm run ui:test each now fail immediately with the clear message instead of running the full command chain first. Closes #7619 --- package.json | 6 +++ scripts/check-node-version.d.mts | 8 ++++ scripts/check-node-version.mjs | 36 ++++++++++++++++ test/unit/check-node-version-script.test.ts | 47 +++++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 scripts/check-node-version.d.mts create mode 100644 scripts/check-node-version.mjs create mode 100644 test/unit/check-node-version-script.test.ts diff --git a/package.json b/package.json index 74a15da08..310762f59 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ "ui:preview": "npm run ui:build && wrangler dev --config apps/loopover-ui/dist/server/wrangler.json --ip 127.0.0.1 --port 4173 --local", "ui:lint": "npm run ui:kit:build && npm --workspace @loopover/ui run lint && npm --workspace @loopover/ui-miner run lint", "ui:typecheck": "npm run ui:kit:build && npm --workspace @loopover/ui run typecheck && npm --workspace @loopover/ui-miner run typecheck", + "preui:test": "npm run check-node-version", "ui:test": "npm run ui:kit:build && npm --workspace @loopover/ui run test && npm --workspace @loopover/ui-miner run test && npm --workspace @loopover/miner-extension run test", "ui:openapi": "tsx scripts/write-ui-openapi.ts", "ui:openapi:check": "tsx scripts/write-ui-openapi.ts --check", @@ -88,6 +89,8 @@ "mcp:release-due": "node scripts/check-mcp-release-due.mjs --json", "mcp:release-candidate": "node scripts/check-mcp-release-candidate.mjs", "typecheck": "tsc --noEmit", + "check-node-version": "node scripts/check-node-version.mjs", + "pretest": "npm run check-node-version", "test": "vitest run", "test:unit": "vitest run test/unit", "test:integration": "vitest run test/integration", @@ -96,13 +99,16 @@ "test:live-gate-parity": "vitest run test/contract/live-gate-parity.test.ts", "test:driver-parity": "vitest run test/contract/coding-agent-driver-parity.test.ts", "test:changed": "vitest run --changed=origin/main", + "pretest:workers": "npm run check-node-version", "test:workers": "vitest run --config vitest.workers.config.ts", + "pretest:coverage": "npm run check-node-version", "test:coverage": "vitest run --coverage --pool=forks", "test:smoke:production": "node scripts/smoke-production.mjs", "test:smoke:observability": "node scripts/smoke-observability-traces.mjs", "test:smoke:observability:metrics": "node scripts/smoke-observability-metrics.mjs", "test:smoke:browser:install": "playwright install chromium", "test:smoke:browser": "node scripts/smoke-ui-browser.mjs", + "pretest:ci": "npm run check-node-version", "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run build:mcp && npm run test:mcp-pack && npm run build:miner && npm run test:miner-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build", "test:release": "npm run test:ci && npm run changelog:check", "test:release:mcp": "npm run test:ci", diff --git a/scripts/check-node-version.d.mts b/scripts/check-node-version.d.mts new file mode 100644 index 000000000..c836d2f3b --- /dev/null +++ b/scripts/check-node-version.d.mts @@ -0,0 +1,8 @@ +export function checkNodeVersion(options?: { + nodeVersion?: string; + readFile?: () => string; +}): { + ok: boolean; + requiredRange: string | undefined; + nodeVersion?: string; +}; diff --git a/scripts/check-node-version.mjs b/scripts/check-node-version.mjs new file mode 100644 index 000000000..7d8606889 --- /dev/null +++ b/scripts/check-node-version.mjs @@ -0,0 +1,36 @@ +#!/usr/bin/env node +// Fails fast, with a clear message, when the running Node doesn't satisfy root package.json's +// engines.node -- even when node_modules is already installed. The root .npmrc's engine-strict=true only +// fires during npm install/ci (dependency resolution); a node_modules installed while on the pinned Node, +// followed by simply switching the active `node` (nvm/homebrew default change) with no reinstall, sails +// straight past engine-strict on every later `npm run`. That's exactly the shape of gap that let the +// Node 26 jsdom/localStorage bug (#7592/#7597/#7612) go unnoticed the first two times: a pile of +// confusing downstream test failures instead of one clear "wrong Node version" message up front. Wired as +// a `pretest*` hook (see package.json) on the commands people actually run vitest through. +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import semver from "semver"; + +const PACKAGE_JSON_URL = new URL("../package.json", import.meta.url); + +export function checkNodeVersion({ nodeVersion = process.version, readFile = () => readFileSync(PACKAGE_JSON_URL, "utf8") } = {}) { + const pkg = JSON.parse(readFile()); + const requiredRange = pkg.engines?.node; + if (!requiredRange) return { ok: true, requiredRange: undefined }; + + const ok = semver.satisfies(nodeVersion, requiredRange); + return { ok, requiredRange, nodeVersion }; +} + +function main() { + const { ok, requiredRange, nodeVersion } = checkNodeVersion(); + if (!ok) { + console.error( + `\nRunning Node ${nodeVersion}, but this repo requires ${requiredRange} (see .nvmrc / package.json engines).\n` + + `Switch to the pinned Node version (e.g. \`nvm use\`) before running this command.\n`, + ); + process.exit(1); + } +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) main(); diff --git a/test/unit/check-node-version-script.test.ts b/test/unit/check-node-version-script.test.ts new file mode 100644 index 000000000..a691d3a6b --- /dev/null +++ b/test/unit/check-node-version-script.test.ts @@ -0,0 +1,47 @@ +import { execFileSync } from "node:child_process"; +import { describe, expect, it } from "vitest"; +import { checkNodeVersion } from "../../scripts/check-node-version.mjs"; + +describe("check-node-version script", () => { + it("passes when the running Node satisfies the declared engines.node range", () => { + const result = checkNodeVersion({ + nodeVersion: "v22.23.1", + readFile: () => JSON.stringify({ engines: { node: ">=22.0.0 <23.0.0" } }), + }); + + expect(result).toEqual({ ok: true, requiredRange: ">=22.0.0 <23.0.0", nodeVersion: "v22.23.1" }); + }); + + it("fails when the running Node is outside the declared engines.node range (the Node 26 case)", () => { + const result = checkNodeVersion({ + nodeVersion: "v26.5.0", + readFile: () => JSON.stringify({ engines: { node: ">=22.0.0 <23.0.0" } }), + }); + + expect(result).toEqual({ ok: false, requiredRange: ">=22.0.0 <23.0.0", nodeVersion: "v26.5.0" }); + }); + + it("passes trivially when package.json declares no engines.node at all", () => { + const result = checkNodeVersion({ + nodeVersion: "v26.5.0", + readFile: () => JSON.stringify({}), + }); + + expect(result).toEqual({ ok: true, requiredRange: undefined }); + }); + + // Regression guard: proves the process actually running this test suite -- CI's own Node, or whatever a + // contributor is running locally -- genuinely satisfies the real repo's engines.node. If this fails, the + // repo is being tested on the wrong Node right now. + it("the real repo's engines.node is satisfied by whatever Node is running this suite", () => { + const result = checkNodeVersion(); + + expect(result.ok).toBe(true); + }); + + it("prints nothing and exits 0 as a subprocess on a satisfying Node", () => { + const output = execFileSync(process.execPath, ["scripts/check-node-version.mjs"], { encoding: "utf8" }); + + expect(output).toBe(""); + }); +});