Skip to content

fix(docker): install Node 22 from NodeSource to match install.sh - #22481

Closed
wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/dockerfile-node-22
Closed

wesleysimplicio wants to merge 1 commit into
NousResearch:mainfrom
wesleysimplicio:fix/dockerfile-node-22

Conversation

@wesleysimplicio

@wesleysimplicio wesleysimplicio commented May 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Closes #21656.

Root cause

Debian 13's apt-shipped nodejs package is 20.x. At least one transitive
web/ dep (language-tags@2.1.0) requires Node >= 22. npm only WARNs on
engine mismatch, so the install completes but the resolver chain breaks
silently and the Vite build dies with a misleading can't resolve clsx
error.

scripts/install.sh already pins NODE_VERSION="22". The Dockerfile
was still installing Node from apt, so the container build drifted from
the host installer.

Fix

Dockerfile now installs Node from NodeSource's setup_22.x script
(adds gnupg + ca-certificates to the base apt layer for the GPG key
fetch). No other layer touched.

Why this shape

This shape mirrors #29640 so reviewers can quickly compare scope, root cause, fix, tests, and related context without having to decode a custom PR description.

Tests

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

Related PRs / issues

Closes #21656.

Original body

Summary

Closes #21656.

What Changed

  • Standardized this PR body to the current Hermes Turbo template.
  • Preserved the original detailed description below for reference.

Fluxo

A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.

Visão

A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.

Test Plan

  • Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.
Original body

What does this PR do?

Closes #21656.

Problem

Debian 13's apt-shipped nodejs package is 20.x. At least one transitive
web/ dep (language-tags@2.1.0) requires Node >= 22. npm only WARNs on
engine mismatch, so the install completes but the resolver chain breaks
silently and the Vite build dies with a misleading can't resolve clsx
error.

scripts/install.sh already pins NODE_VERSION="22". The Dockerfile
was still installing Node from apt, so the container build drifted from
the host installer.

Fix

Dockerfile now installs Node from NodeSource's setup_22.x script
(adds gnupg + ca-certificates to the base apt layer for the GPG key
fetch). No other layer touched.

Regression guards

tests/test_dockerfile_node_version.py adds two pure-text guards (no
docker build, no subprocess):

  1. test_dockerfile_installs_node_from_nodesource_not_debian_apt — fails
    if any apt-get install line co-installs nodejs and npm without a
    NodeSource setup script in the same layer.
  2. test_dockerfile_node_major_matches_installer — extracts
    NODE_VERSION="<major>" from scripts/install.sh and asserts the
    Dockerfile contains deb.nodesource.com/setup_<major>.x. If
    install.sh bumps in the future and the Dockerfile lags, CI fails
    instead of CI passing and the container build breaking on prod
    deploys.

Verification

Stash-verified — both tests fail on baseline, pass with the fix:

$ git stash push Dockerfile
$ ./scripts/run_tests.sh tests/test_dockerfile_node_version.py -v
... 2 failed
$ git stash pop
$ ./scripts/run_tests.sh tests/test_dockerfile_node_version.py -v
... 2 passed in 1.45s

Solution Sketch

  • fix the root cause in the touched subsystem instead of layering a broad workaround around the symptom
  • keep surrounding behavior stable and avoid unrelated refactors while the area is under review
  • prove the change with focused checks on the exact path that regressed

Related Issue

Closes #21656.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • preserved the existing technical rationale and validation notes inside the template body
  • scoped this PR description to the implementation already present on the branch
  • aligned the delivery format with .github/PULL_REQUEST_TEMPLATE.md

How to Test

  1. Review the existing validation notes preserved in this PR body.
  2. Run the focused checks for the touched area.
  3. Confirm the scoped change still behaves as described above.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

  • N/A.

Generated by Hermes Turbo


Generated by Hermes Turbo

Debian 13's apt-shipped nodejs is 20.x. At least one transitive web/
dep (language-tags@2.1.0) requires Node >= 22, but npm only WARNs on
engine mismatch — the install completes, the resolver chain breaks
silently, and the Vite build fails with a misleading 'can't resolve
clsx' error.

scripts/install.sh already pins NODE_VERSION="22"; this aligns the
Dockerfile so the container build matches the host installer.

Adds two static guards in tests/test_dockerfile_node_version.py:
  * Dockerfile must install Node from NodeSource (not Debian apt)
  * NodeSource major must equal install.sh's NODE_VERSION pin

Closes NousResearch#21656
Copilot AI review requested due to automatic review settings May 9, 2026 10:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns the container build with the project’s installer by ensuring the Docker image installs Node.js 22 from NodeSource (matching scripts/install.sh), preventing web/ dependency engine mismatches that can surface later as misleading Vite/Rollup errors.

Changes:

  • Update Dockerfile to install Node.js via NodeSource setup_22.x (and add required apt deps for key retrieval).
  • Add static (text-only) pytest guards to ensure the Dockerfile uses NodeSource and stays in sync with scripts/install.sh’s NODE_VERSION.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Dockerfile Switches Node installation from Debian apt Node 20.x to NodeSource Node 22 to match installer expectations.
tests/test_dockerfile_node_version.py Adds CI guards to prevent Node version/source drift between Dockerfile and scripts/install.sh.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Dockerfile
# Node 22 is installed from NodeSource so the image matches the Node version
# pinned by scripts/install.sh (NODE_VERSION="22"). Debian 13's apt-shipped
# nodejs is 20.x, and at least one transitive web/ dep (language-tags@2.1.0)
# requires Node ≥22. npm only WARN-s on engine mismatch, so the install
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/docker Docker image, Compose, packaging labels May 11, 2026
@benbarclay

Copy link
Copy Markdown
Contributor

Closing — the underlying issue (Debian 13 ships Node 20.x and web/'s transitives need Node 22+) has already been fixed on main, but via a different mechanism than this PR proposes.

Main's Dockerfile (line 9) uses a multi-stage source image:

FROM node:22-bookworm-slim@sha256:7af03b14a13c8cdd38e45058fd957bf00a72bbe17feac43b1c15a689c029c732 AS node_source
...
COPY --chmod=0755 --from=node_source /usr/local/bin/node /usr/local/bin/
COPY --from=node_source /usr/local/lib/node_modules/npm /usr/local/lib/node_modules/npm

Landed in commit 27a29ee (feat(docker): upgrade Node to 22 LTS via multi-stage from node:22-bookworm-slim (#4977)). That gives us SHA-pinned Node 22 from the official upstream image without adding a NodeSource apt repo or fetching a GPG key at build time.

Your test guard (test_dockerfile_node_major_matches_installer) wouldn't apply to that shape (no deb.nodesource.com/setup_22.x line to match), so the diff isn't salvageable as-is. Thanks for the catch — sorry it sat unrebased while #4977 went through review.

@benbarclay benbarclay closed this May 28, 2026
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 comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dockerfile installs Node 20 (apt nodejs), but scripts/install.sh pins Node 22; web/ build broken on debian:13.4 base in v2026.4.30

4 participants