diff --git a/.github/dependabot.yml b/.github/dependabot.yml index db4b08a..16fe6ee 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -5,6 +5,18 @@ # enabled here so dependency updates stay visible, and so the two tools can be # compared side by side before we settle on one. Expect duplicate PRs until then # -- that is intentional, not a misconfiguration. +# +# Node pin (issue #126): Dependabot has NO equivalent of Renovate's +# `constraintsFiltering`. Its ignore/allow/versioning-strategy/groups levers all +# operate on semver update *type*, never on runtime compatibility, so it cannot be +# told "only propose updates whose engines.node overlaps ours". +# +# Two mitigations, since it stays enabled for coverage: +# 1. `@types/node` majors are ignored below, so it cannot re-propose the +# typings-ahead-of-runtime drift that issue #126 exists to fix. +# 2. Everything else relies on CI: the `node-pin` job fails on declaration +# drift, and npm's `engine-strict` (server/.npmrc) fails the install itself +# on a wrong runtime. Treat any Dependabot npm PR as needing an engines glance. version: 2 updates: @@ -20,6 +32,10 @@ updates: update-types: - "minor" - "patch" + ignore: + # Node pin (issue #126): @types/node tracks the runtime major, never leads it. + - dependency-name: "@types/node" + update-types: ["version-update:semver-major"] - package-ecosystem: "github-actions" directory: "/" diff --git a/.github/workflows/build-server.yml b/.github/workflows/build-server.yml index 5d6dc10..674a530 100644 --- a/.github/workflows/build-server.yml +++ b/.github/workflows/build-server.yml @@ -39,10 +39,12 @@ jobs: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Setup Node.js 22 + # The runtime resolved here is the one bundled into the release artifact + # below, so .nvmrc pins what ships to users, not just what CI builds with. + - name: Setup Node.js (pinned by .nvmrc) uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: 22 + node-version-file: .nvmrc - name: Install dependencies working-directory: server diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0047bf5..0b19b97 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,12 @@ on: pull_request: branches: [main] +# Least privilege. Declared at workflow level rather than on one job: every job here +# only checks out and runs npm, and ci.yml was the sole workflow without a permissions +# block, so all four jobs were inheriting whatever the repo/org default grants. +permissions: + contents: read + jobs: typecheck: runs-on: ubuntu-latest @@ -12,7 +18,7 @@ jobs: - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: 22 + node-version-file: .nvmrc - name: Install dependencies working-directory: server @@ -20,7 +26,31 @@ jobs: - name: Type-check working-directory: server - run: npx tsc --noEmit + run: npm run typecheck + + - name: Type-check scripts + working-directory: server + run: npm run typecheck:scripts + + node-pin: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version-file: .nvmrc + + - name: Install dependencies + working-directory: server + run: npm ci + + # `npm ci` above already rejects a wrong runtime via engine-strict. This + # catches the other half: declarations that are each valid but have drifted + # apart -- the @types/node-ahead-of-runtime shape from issue #126. + - name: Check Node pin + working-directory: server + run: npm run check:node-pin test: runs-on: ubuntu-latest @@ -29,7 +59,7 @@ jobs: - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: 22 + node-version-file: .nvmrc - name: Install dependencies working-directory: server @@ -46,7 +76,7 @@ jobs: - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: 22 + node-version-file: .nvmrc - name: Install dependencies working-directory: server diff --git a/.github/workflows/dependency-health.yml b/.github/workflows/dependency-health.yml index 1d94b3b..e62397f 100644 --- a/.github/workflows/dependency-health.yml +++ b/.github/workflows/dependency-health.yml @@ -18,7 +18,7 @@ jobs: - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: 22 + node-version-file: .nvmrc cache: npm cache-dependency-path: server/package-lock.json diff --git a/.nvmrc b/.nvmrc new file mode 100644 index 0000000..a45fd52 --- /dev/null +++ b/.nvmrc @@ -0,0 +1 @@ +24 diff --git a/CLAUDE.md b/CLAUDE.md index 7ebb7ca..6d79ad7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,13 +8,15 @@ DMXr bridges DMX lighting fixtures into SignalRGB as first-class canvas devices. ``` DMXr/ +├── .nvmrc # Node major (24 LTS) -- single source of truth, read by CI + version managers ├── DMXr.js # SignalRGB plugin (UDP/HTTP color transport) ├── DMXr.qml # SignalRGB settings panel UI ├── docs/images/ # SVG logos, fixture icons └── server/ + ├── .npmrc # engine-strict=true -- a wrong Node major fails `npm ci` ├── src/ # All server TypeScript │ ├── bootstrap/ # Startup orchestration (DMX, library, shutdown) - │ ├── config/ # Settings, remap-preset, server-config stores + │ ├── config/ # Settings, remap-preset, server-config stores, Node-pin check │ ├── dmx/ # Universe manager, dispatcher, connection pool, monitor, driver factory │ ├── fixtures/ # Fixture/group/user-fixture stores, color pipeline, channel mapper │ ├── libraries/ # Library registry (OFL + user fixtures) @@ -31,6 +33,7 @@ DMXr/ │ ├── ui/ # Frontend helpers (channel labels, CSS theming, OFL conversion) │ ├── ui-tests/ # Playwright E2E tests (grid, CRUD, settings, multi-select) │ └── utils/ # Formatting, validation helpers + ├── scripts/ # Dev/ops entry points (service installers, check-node-pin) ├── public/ # Alpine.js web UI (no build step) │ ├── js/ # app.js + 31 mixin files │ └── css/ # Feature-scoped CSS files @@ -66,11 +69,20 @@ Browser (http://localhost:8080) SignalRGB Plugin (DMXr.js) ## Development +**Node 24 LTS is required and enforced** (issue #126). `.nvmrc` is the single source of +truth; `server/.npmrc` sets `engine-strict=true`, so `npm ci` **fails** on any other major +rather than warning. Use a version manager that reads `.nvmrc` (`fnm use` / `nvm use` / +`mise install`). Never bump the Node major as a routine dependency PR -- it is one +coordinated change across `.nvmrc`, `engines.node`, `@types/node`, CI, and the runtime that +`build-server.yml` bundles into the release artifact. + ```bash cd server -npm test # vitest run (tests co-located: *.test.ts next to source) -npx tsc --noEmit # type check (strict mode) -- the clean-check; no separate ESLint -npm run build # tsc -> dist/ +npm test # vitest run (tests co-located: *.test.ts next to source) +npm run typecheck # tsc --noEmit (strict mode) -- the clean-check; no separate ESLint +npm run typecheck:scripts # same strict flags over scripts/ (outside the build's rootDir) +npm run check:node-pin # assert every Node declaration names the same major +npm run build # tsc -> dist/ ``` ## Key Conventions diff --git a/renovate.json b/renovate.json index b67d648..4d427ec 100644 --- a/renovate.json +++ b/renovate.json @@ -15,6 +15,28 @@ { "matchManagers": ["npm"], "groupName": "npm-server" + }, + { + "description": "Node pin (issue #126): only propose npm releases whose own engines.node overlaps ours. The constraint is auto-detected from engines.node and .nvmrc -- deliberately NOT declared in a `constraints` block, because Renovate treats a manually-set constraint as fixed and will never offer to bump it, forking the source of truth. Scoped to runtime dependencies per Renovate's guidance that strict filtering across devDependencies filters far more than most users expect.", + "matchManagers": ["npm"], + "matchDepTypes": ["dependencies"], + "constraintsFiltering": "strict" + }, + { + "description": "Node pin (issue #126): never propose a Node MAJOR bump. Moving LTS majors touches .nvmrc, engines.node, @types/node, CI and the runtime bundled into the release artifact -- a coordinated decision, not a routine dependency PR. Minor/patch within the pinned major stays enabled so security patches still flow.", + "matchDepNames": ["node"], + "matchUpdateTypes": ["major"], + "enabled": false + }, + { + "description": "Node pin (issue #126): keep .nvmrc and any workflow node-version on the pinned major. Bounded rather than disabled so 24.x patches still arrive.", + "matchDatasources": ["node-version"], + "allowedVersions": "<25.0.0" + }, + { + "description": "Node pin (issue #126): @types/node must track the runtime major, never lead it. A typings-only bump past the runtime lets tsc accept APIs that do not exist at execution -- this repo was sitting at @types/node 25 against a Node 22 CI, and Node 25 is EOL and never was an LTS.", + "matchPackageNames": ["@types/node"], + "allowedVersions": "<25.0.0" } ] } diff --git a/server/.npmrc b/server/.npmrc new file mode 100644 index 0000000..b6f27f1 --- /dev/null +++ b/server/.npmrc @@ -0,0 +1 @@ +engine-strict=true diff --git a/server/package-lock.json b/server/package-lock.json index 6fc9fa7..b60a699 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -21,20 +21,22 @@ "devDependencies": { "@playwright/test": "^1.60.0", "@types/better-sqlite3": "^7.6.13", - "@types/node": "^25.7.0", + "@types/node": "^24.13.3", "@types/pngjs": "^6.0.5", + "@types/semver": "^7.7.1", "@vitest/coverage-v8": "^4.1.6", "fast-check": "^4.8.0", "pixelmatch": "^7.1.0", "pngjs": "^7.0.0", "puppeteer": "^24.43.1", + "semver": "^7.8.5", "tsx": "^4.21.0", "typescript": "^6.0.2", "vitest": "^4.1.3", "zod": "^4.4.3" }, "engines": { - "node": ">=18.0.0" + "node": ">=24 <25" } }, "node_modules/@babel/code-frame": { @@ -1630,12 +1632,12 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "25.7.0", - "resolved": "https://registry.npmjs.org/@types/node/-/node-25.7.0.tgz", - "integrity": "sha512-z+pdZyxE+RTQE9AcboAZCb4otwcrvgHD+GlBpPgn0emDVt0ohrTMhAwlr2Wd9nZ+nihhYFxO2pThz3C5qSu2Eg==", + "version": "24.13.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.3.tgz", + "integrity": "sha512-Dh8vAsV36ig5wa9OX4pXvMc9D3Veibfw2wix0CUwYODLD8nkj9UsLjASr49nPg+2eKzxhBV+v7L8pXvT4e639Q==", "license": "MIT", "dependencies": { - "undici-types": "~7.21.0" + "undici-types": "~7.18.0" } }, "node_modules/@types/pngjs": { @@ -1648,6 +1650,13 @@ "@types/node": "*" } }, + "node_modules/@types/semver": { + "version": "7.7.1", + "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.7.1.tgz", + "integrity": "sha512-FmgJfu+MOcQ370SD0ev7EI8TlCAfKYU+B4m5T3yXc1CiRN94g/SZPtsCkk506aUDtlMnFZvasDwHHUcZUEaYuA==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/triple-beam": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/@types/triple-beam/-/triple-beam-1.3.5.tgz", @@ -4635,9 +4644,9 @@ "license": "BSD-3-Clause" }, "node_modules/semver": { - "version": "7.7.4", - "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz", - "integrity": "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA==", + "version": "7.8.5", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.8.5.tgz", + "integrity": "sha512-Y7/KDsb8LjooZpwaqGyulO6DQlksgCncchHGk+sZIY4SBvUocMBEFH5Ur1fI4dV+Jvl0w6cjvucaIi40puRioA==", "license": "ISC", "bin": { "semver": "bin/semver.js" @@ -5188,9 +5197,9 @@ } }, "node_modules/undici-types": { - "version": "7.21.0", - "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.21.0.tgz", - "integrity": "sha512-w9IMgQrz4O0YN1LtB7K5P63vhlIOvC7opSmouCJ+ZywlPAlO9gIkJ+otk6LvGpAs2wg4econaCz3TvQ9xPoyuQ==", + "version": "7.18.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", + "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", "license": "MIT" }, "node_modules/util-deprecate": { diff --git a/server/package.json b/server/package.json index 9d5a341..a4823c2 100644 --- a/server/package.json +++ b/server/package.json @@ -15,7 +15,10 @@ "test:ui:watch": "vitest --config vitest.config.ui.ts", "test:ui:update": "UPDATE_BASELINES=1 vitest run --config vitest.config.ui.ts", "test:contract": "vitest run --config vitest.config.contract.ts", - "test:e2e": "playwright test" + "test:e2e": "playwright test", + "typecheck": "tsc --noEmit", + "typecheck:scripts": "tsc -p tsconfig.scripts.json", + "check:node-pin": "tsx scripts/check-node-pin.ts" }, "dependencies": { "@fastify/cors": "^11.2.0", @@ -30,19 +33,21 @@ "devDependencies": { "@playwright/test": "^1.60.0", "@types/better-sqlite3": "^7.6.13", - "@types/node": "^25.7.0", + "@types/node": "^24.13.3", "@types/pngjs": "^6.0.5", + "@types/semver": "^7.7.1", "@vitest/coverage-v8": "^4.1.6", "fast-check": "^4.8.0", "pixelmatch": "^7.1.0", "pngjs": "^7.0.0", "puppeteer": "^24.43.1", + "semver": "^7.8.5", "tsx": "^4.21.0", "typescript": "^6.0.2", "vitest": "^4.1.3", "zod": "^4.4.3" }, "engines": { - "node": ">=18.0.0" + "node": ">=24 <25" } } diff --git a/server/scripts/check-node-pin.ts b/server/scripts/check-node-pin.ts new file mode 100644 index 0000000..d2f1f00 --- /dev/null +++ b/server/scripts/check-node-pin.ts @@ -0,0 +1,112 @@ +/** + * Fails the build when DMXr's Node version declarations drift apart (issue #126). + * + * This is the I/O shell only — the comparison logic lives in + * `src/config/node-pin.ts`, where tsconfig typechecks it and vitest covers it. + * + * Run with `npm run check:node-pin`. + */ +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { checkNodePin } from "../src/config/node-pin.js"; +import type { NodePinInputs } from "../src/config/node-pin.js"; + +const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)); +const SERVER_DIR = join(SCRIPT_DIR, ".."); +const REPO_ROOT = join(SERVER_DIR, ".."); + +/** Reads a UTF-8 file, naming the path in the error so a bad checkout is obvious. */ +function read(path: string): string { + try { + return readFileSync(path, "utf8"); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`Cannot read ${path}: ${reason}`); + } +} + +/** The slice of `server/package.json` the pin cares about. */ +interface ServerManifest { + readonly engines?: { readonly node?: string }; + readonly devDependencies?: Readonly>; +} + +/** Parses a package manifest, distinguishing "unreadable" from "not valid JSON". */ +function readManifest(path: string): ServerManifest { + try { + return JSON.parse(read(path)) as ServerManifest; + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`Cannot parse ${path} as JSON: ${reason}`); + } +} + +/** + * Gathers the four declarations from disk and the current process. + * + * A missing `engines.node` or `@types/node` throws rather than defaulting: the pin + * cannot be satisfied by a declaration that is not there, and silently passing would + * defeat the gate. + */ +function collectInputs(): NodePinInputs { + const manifestPath = join(SERVER_DIR, "package.json"); + const manifest = readManifest(manifestPath); + + const enginesNode = manifest.engines?.node; + if (enginesNode === undefined) { + throw new Error(`${manifestPath} declares no engines.node — the pin needs one.`); + } + + const typesNode = manifest.devDependencies?.["@types/node"]; + if (typesNode === undefined) { + throw new Error( + `${manifestPath} declares no devDependencies["@types/node"] — the pin needs one.`, + ); + } + + return { + nvmrc: read(join(REPO_ROOT, ".nvmrc")), + enginesNode, + typesNode, + runtimeVersion: process.version, + }; +} + +/** Runs the check and prints either a one-line pass or a full declaration table. */ +function main(): void { + const result = checkNodePin(collectInputs()); + + if (result.consistent) { + console.log( + `node-pin: consistent — Node ${result.major} across ` + + `${result.declarations.length} declarations and the running runtime.`, + ); + return; + } + + console.error("node-pin: FAILED — Node version declarations disagree.\n"); + for (const declaration of result.declarations) { + console.error( + ` ${declaration.source.padEnd(38)} ${declaration.raw.trim()}` + + `${declaration.major === null ? " (unparseable)" : ""}`, + ); + } + console.error(""); + for (const problem of result.problems) { + console.error(` - ${problem}`); + } + console.error( + "\nSee issue #126. `.nvmrc` is the source of truth; bring the others to match it.", + ); + process.exitCode = 1; +} + +try { + main(); +} catch (error) { + console.error( + `node-pin: could not run — ${error instanceof Error ? error.message : String(error)}`, + ); + process.exitCode = 1; +} diff --git a/server/src/config/README.md b/server/src/config/README.md index 8d43790..f25ad7e 100644 --- a/server/src/config/README.md +++ b/server/src/config/README.md @@ -14,6 +14,15 @@ - Auto-generates `serverId` (UUID) on first load if missing - Always returns defensive copies (`{ ...current }`) to prevent external mutation +### node-pin.ts +- `parseMajor(raw)`, `isRangeConfinedToMajor(range, major)`, `checkNodePin(inputs)` -> `NodePinCheckResult` +- Pure comparison logic for the Node 24 LTS pin (issue #126): asserts `.nvmrc`, + `engines.node`, `@types/node` and the running interpreter all name the same major, + and that `engines.node` and `@types/node` are each confined to that major + (`rangeProblems` checks both) rather than being open-ended or spanning two majors +- No I/O -- `server/scripts/check-node-pin.ts` supplies the files and the exit code. + Lives here so tsconfig typechecks it and vitest covers it; `scripts/` is outside both. + ### remap-preset-store.ts - `createRemapPresetStore(filePath)` -> `RemapPresetStore { load, getAll, get, upsert, remove, save }` - Stores named channel-remap presets (channelCount + offset mapping) diff --git a/server/src/config/node-pin.test.ts b/server/src/config/node-pin.test.ts new file mode 100644 index 0000000..93010c6 --- /dev/null +++ b/server/src/config/node-pin.test.ts @@ -0,0 +1,191 @@ +import { describe, it, expect } from "vitest"; +import { parseMajor, isRangeConfinedToMajor, checkNodePin } from "./node-pin.js"; +import type { NodePinInputs } from "./node-pin.js"; + +const consistentInputs: NodePinInputs = { + nvmrc: "24", + enginesNode: ">=24 <25", + typesNode: "^24.13.3", + runtimeVersion: "v24.18.1", +}; + +describe("parseMajor", () => { + it("reads a bare major", () => { + expect(parseMajor("24")).toBe(24); + }); + + it("reads a v-prefixed runtime version", () => { + expect(parseMajor("v24.18.1")).toBe(24); + }); + + it("reads a caret range", () => { + expect(parseMajor("^24.13.3")).toBe(24); + }); + + it("reads the lower bound of a bounded range", () => { + expect(parseMajor(">=24 <25")).toBe(24); + }); + + it("reads a floor range", () => { + expect(parseMajor(">=18.0.0")).toBe(18); + }); + + it("tolerates surrounding whitespace and newlines", () => { + expect(parseMajor(" 24\n")).toBe(24); + }); + + it("returns null when no major is present", () => { + expect(parseMajor("latest")).toBeNull(); + expect(parseMajor("")).toBeNull(); + }); +}); + +describe("isRangeConfinedToMajor", () => { + it("accepts an explicit single-major range", () => { + expect(isRangeConfinedToMajor(">=24 <25", 24)).toBe(true); + }); + + it("accepts caret, tilde and exact pins inside the major", () => { + expect(isRangeConfinedToMajor("^24.13.3", 24)).toBe(true); + expect(isRangeConfinedToMajor("~24.13.0", 24)).toBe(true); + expect(isRangeConfinedToMajor("24.13.3", 24)).toBe(true); + }); + + it("rejects an open-ended floor", () => { + expect(isRangeConfinedToMajor(">=24", 24)).toBe(false); + expect(isRangeConfinedToMajor(">=18.0.0", 24)).toBe(false); + }); + + // Regression, fixed in 2fa65a6: the syntactic predicate this replaced returned + // true here, because the range has an upper bound and starts with 24 — yet Node + // 25 satisfies it. + it("rejects a bounded range that still spans two majors", () => { + expect(isRangeConfinedToMajor(">=24 <26", 24)).toBe(false); + }); + + it("rejects a disjunction that reaches past the major", () => { + expect(isRangeConfinedToMajor("^24 || ^25", 24)).toBe(false); + }); + + it("rejects a wildcard", () => { + expect(isRangeConfinedToMajor("*", 24)).toBe(false); + }); + + it("rejects a range confined to a different major", () => { + expect(isRangeConfinedToMajor("^22.0.0", 24)).toBe(false); + }); + + it("returns false for a malformed range instead of throwing", () => { + expect(isRangeConfinedToMajor("not-a-range", 24)).toBe(false); + expect(isRangeConfinedToMajor("", 24)).toBe(false); + }); +}); + +describe("checkNodePin", () => { + it("passes when every declaration names the same major", () => { + const result = checkNodePin(consistentInputs); + + expect(result.consistent).toBe(true); + expect(result.major).toBe(24); + expect(result.problems).toEqual([]); + }); + + it("reports every declaration it inspected", () => { + const result = checkNodePin(consistentInputs); + + expect(result.declarations).toHaveLength(4); + expect(result.declarations.map((d) => d.source)).toEqual([ + ".nvmrc", + "server/package.json → engines.node", + "server/package.json → @types/node", + "running interpreter", + ]); + }); + + it("fails when @types/node runs ahead of the runtime", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: "^25.7.0" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("@types/node"); + }); + + it("fails when the running interpreter is a different major", () => { + const result = checkNodePin({ ...consistentInputs, runtimeVersion: "v26.4.0" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("running interpreter"); + }); + + it("fails when .nvmrc disagrees with the engines range", () => { + const result = checkNodePin({ ...consistentInputs, nvmrc: "22" }); + + expect(result.consistent).toBe(false); + }); + + it("fails an open-ended engines range even when every major agrees", () => { + const result = checkNodePin({ ...consistentInputs, enginesNode: ">=24" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("bounded"); + }); + + it("fails an unparseable declaration rather than silently skipping it", () => { + const result = checkNodePin({ ...consistentInputs, nvmrc: "lts/*" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain(".nvmrc"); + }); + + it("names the offending majors so the failure is actionable", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: "^25.7.0" }); + + expect(result.problems.join(" ")).toContain("25"); + expect(result.problems.join(" ")).toContain("24"); + }); + + // Regression, fixed in 2fa65a6: adversarial review of PR #127 (Codex + // gpt-5.6-sol) found the gate accepted ranges that pass a first-number or + // has-an-upper-bound check but still admit a different major -- the exact + // drift it exists to prevent. + it("fails an engines range that admits a second major", () => { + const result = checkNodePin({ ...consistentInputs, enginesNode: ">=24 <26" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("engines.node"); + }); + + it("fails an unbounded @types/node range", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: ">=24" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("@types/node"); + }); + + it("fails a disjunctive @types/node range that reaches past the pin", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: "^24 || ^25" }); + + expect(result.consistent).toBe(false); + expect(result.problems.join(" ")).toContain("@types/node"); + }); + + it("still accepts an exact pinned @types/node version", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: "24.13.3" }); + + expect(result.consistent).toBe(true); + }); + + it("still accepts a tilde @types/node range inside the pin", () => { + const result = checkNodePin({ ...consistentInputs, typesNode: "~24.13.0" }); + + expect(result.consistent).toBe(true); + }); + + it("does not mutate its input", () => { + const inputs: NodePinInputs = { ...consistentInputs }; + const snapshot = JSON.stringify(inputs); + + checkNodePin(inputs); + + expect(JSON.stringify(inputs)).toBe(snapshot); + }); +}); diff --git a/server/src/config/node-pin.ts b/server/src/config/node-pin.ts new file mode 100644 index 0000000..4a9eb27 --- /dev/null +++ b/server/src/config/node-pin.ts @@ -0,0 +1,174 @@ +/** + * Node version pin (issue #126). + * + * DMXr declares its Node major in four independent places. This module holds the + * pure comparison logic that asserts they all agree; `scripts/check-node-pin.ts` + * supplies the file I/O and the exit code. + * + * Two of those declarations are semver *ranges*, and a range is where a pin + * quietly rots: `">=24 <26"` looks bounded and `">=24"` starts with the right + * number, yet both admit Node 25. So ranges are checked semantically -- the whole + * range must be contained in the pinned major -- rather than by reading their + * first number or looking for a `<`. Adversarial review of PR #127 caught the + * earlier syntactic version accepting exactly those two shapes. + */ +import { subset } from "semver"; + +/** One place the Node major is declared. */ +export interface NodePinDeclaration { + /** Human-readable location, used verbatim in failure messages. */ + readonly source: string; + /** The declaration exactly as written. */ + readonly raw: string; + /** Major version, or null when `raw` names no parseable major. */ + readonly major: number | null; +} + +/** The four declarations, read from disk and the running process. */ +export interface NodePinInputs { + /** Contents of `.nvmrc` — the source of truth. A bare major, e.g. `24`. */ + readonly nvmrc: string; + /** `engines.node` from `server/package.json`. A semver range. */ + readonly enginesNode: string; + /** `devDependencies["@types/node"]` from `server/package.json`. A semver range. */ + readonly typesNode: string; + /** `process.version` of the interpreter running the check. */ + readonly runtimeVersion: string; +} + +export interface NodePinCheckResult { + readonly consistent: boolean; + /** The pinned major, or null when anything disagrees. */ + readonly major: number | null; + readonly declarations: readonly NodePinDeclaration[]; + /** Empty when consistent; otherwise one actionable sentence per problem. */ + readonly problems: readonly string[]; +} + +/** Matches the first version number in a version string. */ +const FIRST_MAJOR = /(\d+)/; + +/** + * Extracts the major version from a concrete version (`24`, `v24.18.1`) or, for + * display purposes, from a range. + * + * Returns null when no digit is present (`lts/*`, `latest`, `""`), so callers can + * fail loudly instead of treating an unreadable declaration as agreement. + * + * Do **not** use this to validate a range — `">=24 <26"` yields 24 while still + * admitting 25. Use {@link isRangeConfinedToMajor}. + */ +export function parseMajor(raw: string): number | null { + const match = FIRST_MAJOR.exec(raw.trim()); + if (match === null) return null; + + const major = Number.parseInt(match[1], 10); + return Number.isNaN(major) ? null : major; +} + +/** + * True when *every* version satisfying `range` falls inside `major`. + * + * This is the check that actually holds the pin. `^24.13.3`, `~24.13.0`, + * `24.13.3` and `">=24 <25"` pass; `">=24"`, `">=24 <26"`, `"^24 || ^25"` and + * `"*"` all fail, because each admits a major we never validated. + * + * Returns false for a malformed range rather than throwing — an unreadable + * declaration is a failure, not a crash. + */ +export function isRangeConfinedToMajor(range: string, major: number): boolean { + try { + return subset(range, `>=${major}.0.0 <${major + 1}.0.0`); + } catch { + return false; + } +} + +/** Builds a declaration record, parsing its major for display in failure output. */ +function declare(source: string, raw: string): NodePinDeclaration { + return { source, raw, major: parseMajor(raw) }; +} + +/** + * Reports a range declaration that is not wholly inside the pinned major. + * Returns an empty list when the range is fine, so callers can spread it. + */ +function rangeProblems( + source: string, + range: string, + pinned: number, +): readonly string[] { + if (isRangeConfinedToMajor(range, pinned)) return []; + + return [ + `${source} is "${range.trim()}", which is not bounded to Node ${pinned} — ` + + `it admits at least one other major. Use a range wholly inside the pin, ` + + `such as ">=${pinned} <${pinned + 1}" or "^${pinned}.0.0".`, + ]; +} + +/** + * Reports the interpreter actually executing the check being off the pin, with the + * remedy — this is the failure a contributor on the wrong runtime will hit first. + */ +function runtimeProblems(version: string, pinned: number): readonly string[] { + const major = parseMajor(version); + if (major === pinned) return []; + + const named = major === null ? "no readable major" : `Node ${major}`; + return [ + `running interpreter reports ${named} but the pin is Node ${pinned} ` + + `(declared "${version.trim()}"). Switch runtimes with fnm/nvm/mise — ` + + `they read .nvmrc.`, + ]; +} + +/** + * Asserts every declaration agrees on one Node major. + * + * `.nvmrc` is the reference, because it is the file both version managers and + * `setup-node` read — it is what actually selects the runtime. The other three + * are measured against it. + */ +export function checkNodePin(inputs: NodePinInputs): NodePinCheckResult { + const declarations: readonly NodePinDeclaration[] = [ + declare(".nvmrc", inputs.nvmrc), + declare("server/package.json → engines.node", inputs.enginesNode), + declare("server/package.json → @types/node", inputs.typesNode), + declare("running interpreter", inputs.runtimeVersion), + ]; + + const pinned = parseMajor(inputs.nvmrc); + if (pinned === null) { + return { + consistent: false, + major: null, + declarations, + problems: [ + `.nvmrc declares "${inputs.nvmrc.trim()}", which names no Node major. ` + + `It must be a bare major, e.g. "24".`, + ], + }; + } + + const problems = [ + ...rangeProblems( + "server/package.json → engines.node", + inputs.enginesNode, + pinned, + ), + ...rangeProblems( + "server/package.json → @types/node", + inputs.typesNode, + pinned, + ), + ...runtimeProblems(inputs.runtimeVersion, pinned), + ]; + + return { + consistent: problems.length === 0, + major: problems.length === 0 ? pinned : null, + declarations, + problems, + }; +} diff --git a/server/tsconfig.json b/server/tsconfig.json index 766dc77..9a5f07b 100644 --- a/server/tsconfig.json +++ b/server/tsconfig.json @@ -15,5 +15,9 @@ "isolatedModules": true }, "include": ["src/**/*.ts"], - "exclude": ["node_modules", "dist", "**/*.test.ts"] + // node-pin.ts is a build-time gate, not server code. It is excluded from emit so + // its `semver` import stays a devDependency and never reaches the pruned release + // bundle that build-server.yml ships. It is still typechecked (tsconfig.scripts.json) + // and still unit-tested (vitest), so excluding it costs no coverage. + "exclude": ["node_modules", "dist", "**/*.test.ts", "src/config/node-pin.ts"] } diff --git a/server/tsconfig.scripts.json b/server/tsconfig.scripts.json new file mode 100644 index 0000000..065d80c --- /dev/null +++ b/server/tsconfig.scripts.json @@ -0,0 +1,16 @@ +// Typechecks `scripts/` under the same strict flags as `src/`. +// +// The base config sets `rootDir: "src"` and emits to `dist/`, so it cannot also +// cover `scripts/` without changing the build output. This config is check-only: +// it widens the root and turns emit off, so a broken script fails CI instead of +// failing the first time someone runs it. +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "rootDir": ".", + "noEmit": true, + "declaration": false, + "sourceMap": false + }, + "include": ["scripts/**/*.ts", "src/**/*.ts"] +}