Skip to content

chore(docker): align Node toolchain on 24 - #74890

Closed
DeliciousHouse wants to merge 1 commit into
NousResearch:mainfrom
DeliciousHouse:chore/t_f68cb417-node-24-toolchain
Closed

DeliciousHouse wants to merge 1 commit into
NousResearch:mainfrom
DeliciousHouse:chore/t_f68cb417-node-24-toolchain

Conversation

@DeliciousHouse

Copy link
Copy Markdown

Summary

  • Upgrade the Docker Node source stage from Node 22 to the SHA-pinned Node 24 Bookworm slim image.
  • Add .nvmrc with Node 24 and require node >=24 in both package.json and the root lockfile metadata.
  • Keep the Dockerfile maintenance notes aligned with the new single-major toolchain contract.

Test Coverage

  • Canonical Hermes per-file runner: 1/1 temporary toolchain contract test passed under Node v24.15.0; the temporary test was removed after verification.
  • npm install --package-lock-only --ignore-scripts --no-audit --no-fund --dry-run: passed under Node v24.15.0 / npm 11.12.1 without changing the committed tree.
  • Docker Hub registry verification: the pinned image index resolves to an amd64 config declaring NODE_VERSION=24.18.0.
  • Local Docker image build was not run because Docker is unavailable on the Windows host and WSL returned WSL/Service/E_UNEXPECTED; the existing Linux Docker CI remains the authoritative image build.

Pre-Landing Review

No unresolved issues found. One stale Dockerfile version-bump comment was corrected during review.

Design Review

No frontend files changed; design review skipped.

Scope Drift

Scope Check: CLEAN

Plan Completion

No plan file detected.

Base Freshness

  • Initial task-required base: freshly fetched fork origin/main at 1001cd2d3469236219ae25ddea131299bd48258f, 0 commits behind.
  • Final PR target base: freshly fetched upstream/main at 36e41c09ed02bd783c1186564bf08cca5c8e821d, 0 commits behind. The feature commit was rebased onto the real upstream target after detecting that the fork default branch was stale.

Test plan

  • Node 24 toolchain contract passes under Node v24.15.0.
  • Package manifest and lockfile engine metadata agree on >=24.
  • Pinned Docker image digest resolves to a Node 24 runtime.
  • Pre-landing review reports no unresolved issues.
  • Linux Docker build and container smoke tests (CI).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for aligning the Docker stage and repository metadata around Node 24. The Docker premise is real: current main still uses node:22-bookworm-slim at Dockerfile:51.

Problems

  • package.json:47 raises the root engine requirement to Node 24, but Hermes still provisions and tests Node 22: scripts/install.sh:60, scripts/install.ps1:153, and .github/workflows/js-tests.yml:18 / :64. The user docs also promise Node 22 at website/docs/getting-started/installation.md:93 and website/docs/user-guide/windows-native.md:59. This leaves the claimed single-major contract inconsistent across supported installation and CI paths.

Suggested changes

  • Update the installers, Node CI setup, and affected docs to Node 24 together with this engine change, then validate those paths under Node 24.

Automated hermes-sweeper review.

Comment thread package.json
},
"engines": {
"node": ">=20.0.0"
"node": ">=24"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This root requirement conflicts with current supported paths: scripts/install.sh:60 and scripts/install.ps1:153 provision Node 22, while .github/workflows/js-tests.yml:18 and :64 run npm CI/checks on Node 22. Please update those toolchain contracts and their user docs in the same change.

@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change area/docker Docker image, Compose, packaging dependencies Pull requests that update a dependency file P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation labels Jul 30, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026

@DeliciousHouse DeliciousHouse left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Merge-gate disposition at exact head ac2abe4ebcdda8dc226e873a023d5eff59c4e10b: close unmerged as the redundant stream.

PR #74250 is the canonical Node 24 candidate. It contains this PR’s same .nvmrc, root engine/lockfile, and digest-pinned node:24-bookworm-slim source-stage behavior, plus all six actions/setup-node consumers, desktop engine/lock metadata, change classification, and a repository contract test. This four-file branch adds no unique executable behavior.

It also leaves CI on Node 22 while changing the root engine to >=24, and leaves the desktop engine on its older Node 20/22-compatible range. No exact-head checks are reported. The canonical PR has entered its single correction cycle for the broader installer/bootstrap/healing/Nix/docs Node contract and fresh-base CI. Closing this overlapping draft prevents the same implementation from being merged twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging dependencies Pull requests that update a dependency file needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants