Enforce the Node pin: xtask preflight, engine-strict, docs (bd-lh30hlvd) - #664
Merged
Conversation
…nv skips it Root cause: Node >= 25 defines a `localStorage` accessor on globalThis that returns undefined without --localstorage-file; vitest 4.x's jsdom environment skips window keys already present on the global unless allowlisted, and the allowlist lacks localStorage/sessionStorage (fixed upstream only in vitest 5). Recurrence of the May 2026 incident (ca6d47c): the Node 24 pin is advisory (no version manager, engine-strict off) and a Sep 4 `brew upgrade` relinked node to 26.8.1. Plan skeleton + probe, prototype setupFiles shim (62/62 affected tests green on Node 26), and fix/enforcement design questions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Gr8MUcZtbVAz8wzokihQV
Decision (2026-09-08): keep engines.node at ^24.0.0, fix the machine (fnm from .nvmrc), and make the pin enforced (xtask verify/dev-setup check, engine-strict). Defer the vitest 5 / jsdom-Storage shim to the LTS bump. Phase A (machine) is done and recorded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Gr8MUcZtbVAz8wzokihQV
The repo has pinned Node 24 since ca6d47c (.nvmrc + engines.node), but the pin was advisory: `.nvmrc` needs a version manager, `engines` only warns at `npm install`, and xtask never looked. A `brew upgrade` on 2026-09-04 relinked `node` to 26.8.1 and the hub-client vitest suite went red four days later with 23 unrelated-looking `localStorage` failures (Node >= 25 defines a localStorage accessor that vitest 4.x's jsdom environment refuses to overwrite; fixed upstream only in vitest 5). Decision: keep engines.node at ^24.0.0 and make the pin enforced instead. - crates/xtask/src/node_version.rs (new): reads engines.node from the root package.json, probes `node --version`, and classifies the result. Ranges are interpreted with npm's own grammar (nodejs-semver) — Cargo's semver crate rejects the space-separated `>=24 <25` form. An unparsable range is an error, never a silent pass. 17 unit tests, written first. - cargo xtask verify: "Preflight: Node toolchain" before Step 1 — fails in seconds on a mismatch or missing node with an actionable message, prints the version it found on success, skips itself when every npm-driven step is disabled. Q2_ALLOW_NODE_MISMATCH=1 warns and continues (deliberate experiments only). - cargo xtask dev-setup: warn-only check with fnm install hints per platform. - .npmrc: engine-strict=true, so `npm install`/`npm ci` fail with EBADENGINE under a Node outside the range (verified under 26.8.1) and succeed under 24 (fresh `npm ci` in the worktree). - claude-notes/instructions/node-version.md: the pin, what enforces it, fnm setup (why ~/.zprofile rather than ~/.zshenv on macOS), the Homebrew relink trap, and how to bump the pin (incl. the vitest 5 / shim item, bd-s84z961e). Pointers from CLAUDE.md and .claude/rules/worktrees.md. Full `cargo xtask verify` under fnm's Node 24.20.0: all 14 steps green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Gr8MUcZtbVAz8wzokihQV
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Gr8MUcZtbVAz8wzokihQV
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The repo has pinned Node 24 since
ca6d47c8(.nvmrc+engines.node), but the pin was advisory:.nvmrcneeds a version manager,enginesonly warns atnpm install, and xtask never looked. Abrew upgradeon 2026-09-04 relinkednodeto 26.8.1 on a dev machine and the hub-client vitest suite went red four days later with 23 unrelated-lookinglocalStoragefailures (Node ≥ 25 defines alocalStorageaccessor that vitest 4.x's jsdom environment refuses to overwrite; fixed upstream only in vitest 5).Decision: keep
engines.nodeat^24.0.0for now and make the pin enforced.crates/xtask/src/node_version.rs(new): readsengines.nodefrom the rootpackage.json, probesnode --version, classifies the result. Ranges use npm's own grammar (nodejs-semver) — Cargo'ssemvercrate rejects the space-separated>=24 <25form. An unparsable range is an error, never a silent pass. 17 unit tests, written first.cargo xtask verify: "Preflight: Node toolchain" before Step 1 — fails in seconds on a mismatch or missingnodewith an actionable message; prints the version found on success; skips itself when every npm-driven step is disabled.Q2_ALLOW_NODE_MISMATCH=1warns and continues (deliberate experiments only).cargo xtask dev-setup: warn-only check with per-platform fnm install hints..npmrc:engine-strict=true—npm install/npm cifail withEBADENGINEunder a Node outside the range (verified under 26.8.1) and succeed under 24 (freshnpm ci).claude-notes/instructions/node-version.md(the pin, what enforces it, fnm setup incl. why~/.zprofilenot~/.zshenvon macOS, the Homebrew relink trap, bump procedure); pointers fromCLAUDE.mdand.claude/rules/worktrees.md.Investigation record, timeline and fix-candidate table:
claude-notes/plans/2026-09-08-node26-vitest-localstorage.md. Deferred vitest 5 / shim work for the eventual Node 26 bump: bd-s84z961e.Verification
cargo xtask verifyunder Node 24.20.0: all 14 steps green (13,757 Rust tests; hub-client 1084/119/133; trace-viewer, shared preview-*, hub MCP suites).cargo xtask verifystops at the preflight with exit 1 before any Rust build.-D warnings),cargo fmt,cargo xtask lintclean.CI runs
actions/setup-nodewithnode-version: '24'in every workflow, soengine-strictand the preflight are no-ops there — this PR is mainly to confirm that.🤖 Generated with Claude Code
https://claude.ai/code/session_017Gr8MUcZtbVAz8wzokihQV