Skip to content

fix(setup): require supported Node releases - #468

Open
LunaMeerkats wants to merge 3 commits into
lacs-project:mainfrom
LunaMeerkats:327-node22-setup
Open

LunaMeerkats wants to merge 3 commits into
lacs-project:mainfrom
LunaMeerkats:327-node22-setup

Conversation

@LunaMeerkats

Copy link
Copy Markdown
Contributor

Summary

Raise the sysknife-setup support floor from Node 18 to Node 22. This intentionally drops support for Node 18 and 20 because those releases no longer receive security fixes; it does not merely remove an untested documentation claim.

The package engine, parse-safe preflight, boundary tests, refusal message, and all six published support statements now move together. This is a public support-floor change and should ship in the next minor release while SysKnife remains in the 0.y series.

Closes #327.

Changes

  • require Node 22 in both engines.node and the preflight guard;
  • reject Node 21 and earlier while accepting Node 22 and later;
  • explain the security-maintenance reason in the actionable refusal;
  • guard the complete published documentation surface against a stale Node 18 claim.

Validation

  • Test-first focused run: 4 of 10 tests failed before the implementation (old floor, accepted Node 18, metadata mismatch, and stale published guidance).
  • node --test packages/setup/tests/node-preflight.test.mjs — 10/10 passed.
  • Linux/WSL npm test --prefix packages/setup on Node 24.16.0 — 127/127 passed (clean baseline: 126/126).
  • Mutation check with only MIN_MAJOR restored to 18 — the boundary and metadata-sync tests both failed; the final file was restored and the focused suite rerun green.
  • python scripts/check_evidence_claims.py and bash tests/release/public-claims.test.sh — passed.
  • markdownlint-cli2 on all five changed Markdown files — 0 issues.
  • Repository completeness, test reachability, release-version consistency, and all 54 action pins — passed.
  • npm pack --dry-run — package contains the preflight and metadata, 11 files total.
  • git diff --check — passed.
  • Security impact considered; this narrows the supported runtime to maintained Node releases and does not touch the SysKnife trust boundary.
  • Rust gate not applicable under the documented non-Rust exemption: the diff contains no .rs files.

Native Windows npm test was also sampled on the clean baseline: 111 passed, 14 failed, and 1 skipped on existing Unix permission/path/getuid assumptions. The final complete package suite above ran in the repository's Linux environment; the focused final test passes on Windows too.

Notes for Reviewers

OpenAI Codex authored and validated this change under the disclosed autonomous workflow. No human pre-submission review is claimed. The branch is based on current main at ba1a7b8f; the active #461 touches separate lines in two documentation files and has no same-scope implementation.

Move the setup wizard support floor to Node 22 and keep its package metadata, refusal message, boundary tests, and published guidance aligned. Older Node releases no longer receive security fixes.\n\nAuthored and validated by autonomous OpenAI Codex; no human pre-submission review is claimed.
vladimirrott
vladimirrott previously approved these changes Sep 22, 2026

@vladimirrott vladimirrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the version of #327 I hoped for and did not assume. I named the decision in the offer without answering it, and you answered it in the PR body: this drops support rather than retiring an untested claim, and you say so in the first paragraph so nobody has to infer it from the diff.

Both guards bite. I checked them separately, because the interesting failure here is a support floor that moves in package.json while the prose keeps promising the old one.

Reverting the production hunk:

$ sed -i 's/var MIN_MAJOR = 22;/var MIN_MAJOR = 18;/' packages/setup/node-preflight.js
$ npm test --prefix packages/setup
not ok 52 - rejects every major below the minimum and accepts every one at or above
  failureType: 'testCodeFailure'
  name: 'AssertionError'

Reverting one of the six published statements, leaving the code alone:

$ sed -i 's/Node 22 or newer/Node 18 or newer/' docs/quickstart.md
$ npm test --prefix packages/setup
not ok 55 - published setup guidance names Node 22 and no longer claims Node 18 support
  error: 'docs/quickstart.md must not retain the old support floor'

Named the file. That second one is the guard that makes this change safe to repeat next time a floor moves, and it is the part most PRs of this shape leave out.

Clean run, 127 tests, 0 failures, restored to 0 after each mutation.

I checked the two couplings the diff cannot show. engines.node and MIN_MAJOR move together, which your own comment on line 17 asks for. And CI runs Node 24 everywhere (ci.yml, release.yml, release-rehearsal.yml), so nothing in the pipeline is now building the wizard on a Node it refuses. Neither needed a change; I would rather say I looked.

The refusal message earned its extra line. "needs Node 22 or newer" tells somebody what to do; "because older releases no longer receive security fixes" tells them why they cannot argue with it, which is the difference between a version bump and a decision.

One thing outside your scope, and I am not asking you to fix it here. docs/developer-guide.md:22 still lists Node.js 20 in the contributor prerequisites table, and docs/contributing/CONTRIBUTING.md:71 repeats it in prose. That is a different surface from the wizard's runtime floor, so leaving it is right for this PR. It does mean the repository now tells a contributor to install Node 20, runs CI on 24, and ships a wizard that refuses anything under 22. @k4its1t is already holding #464 against that same contributing page; I will put this in front of them there rather than widening yours.

The CHANGELOG entry is mine to write and I will add it at merge. This one belongs in Changed rather than Fixed, because a dropped support floor is a thing an existing user can notice.

Approving at 6b550f21. Merging once the board is up to date.

The maintainer's half of the merge, not a review finding. A dropped support
floor is something an existing user notices, so it belongs in Changed rather
than Fixed.

Also merges main, which brought lacs-project#461's test baseline (1,866) into the same
three files this branch edits. Both survive: the Node 22 statements and the
baseline figure, checked file by file rather than trusting the auto-merge.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

packages/setup claims Node 18 support, and Node 18 has been end-of-life since 2025-04-30

2 participants