Skip to content

fix(dashboard): stop Node 25 installs from failing at nanoid - #84397

Open
fangliquanflq wants to merge 8 commits into
NousResearch:mainfrom
fangliquanflq:fix/dashboard-node-engine-alignment
Open

fix(dashboard): stop Node 25 installs from failing at nanoid#84397
fangliquanflq wants to merge 8 commits into
NousResearch:mainfrom
fangliquanflq:fix/dashboard-node-engine-alignment

Conversation

@fangliquanflq

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents hermes dashboard installs on Node 25 from reaching a transitive nanoid@6 engine failure with no Hermes-specific remediation. Hermes now declares the Node release lines its dependency tree actually supports, and both installers replace unsupported odd-numbered releases with the managed LTS runtime.

Symptom

With engine-strict=true, running the dashboard install under Node 25.2.1 and npm 11.6.2 fails with EBADENGINE at nanoid@6.0.0, whose Node range excludes Node 25. The former Hermes range, >=22.22.0, incorrectly advertised Node 25 as supported.

Impact

Users who update or install the dashboard with Node 25 cannot complete the workspace dependency install. Without this change, the installer accepts that runtime and lets the failure surface later from a transitive dependency rather than selecting a working managed runtime.

Bug Cause

Trigger: package.json:engines.node and scripts/install.sh:node_satisfies_build / scripts/install.ps1:Test-NodeVersionOk

Causal chain:

  1. A user runs the dashboard install with Node 25 and npm engine checks enabled.
  2. Hermes's manifest and installer preflight accept every Node release at or above 22.22, while nanoid@6 supports Node 22, 24, and 26 or newer.
  3. npm proceeds until dependency validation and aborts with a transitive nanoid EBADENGINE error.

Why it is wrong: Hermes's continuous minimum-version range was looser than the release-line compatibility of its dependency tree, and the installer duplicated the same overly broad policy.

Working sibling / contrast: Node 22.22+, Node 24, and Node 26+ satisfy both Hermes and the transitive dependency; the installers can also provision the supported managed Node 24 runtime.

Ruled out: Disabling engine-strict lets the same dependency tree install, which rules out a missing package or dashboard build failure and isolates the failure to incompatible engine policy.

Fix

Declare Node 22.22+, 24, and 26+ consistently in the root and Desktop manifests and their lockfile mirrors. Apply the same release-line policy in the POSIX and Windows installer preflight, emit actionable supported-line diagnostics, and add JavaScript invariants for manifest/lockfile alignment plus Python coverage for the managed runtime relationship.

Related Issue

Closes #84359

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • package.json, apps/desktop/package.json, and package-lock.json - align the declared Node engine range with supported release lines.
  • scripts/install.sh and scripts/install.ps1 - reject unsupported odd-numbered Node releases and select the managed runtime with actionable diagnostics.
  • tests-js/node-engine-alignment.test.ts - verify supported and rejected releases across manifests and lockfile mirrors.
  • tests/test_engines_satisfiable.py - verify the managed installer runtime satisfies the declared range.

How to Test

  1. With Node 25.2.1 and npm 11.6.2, run npm ci --ignore-scripts --no-audit --no-fund on the base revision and observe the nanoid@6.0.0 EBADENGINE failure.
  2. Run the same command on this branch and observe the earlier Hermes engine contract naming the supported Node lines, with no transitive nanoid error.
  3. Exercise both installer version gates and confirm Node 22.22, 24, and 26 pass while Node 22.21, 23, and 25 select the managed runtime.
  4. Run the targeted automated tests:
npm exec --workspace tests-js -- vitest run node-engine-alignment.test.ts
scripts/run_tests.sh tests/test_engines_satisfiable.py tests/hermes_cli/test_npm_engine.py

The JavaScript suite passed 8 tests and the targeted Python suite passed 30 tests on the reviewed tip.

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 added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 10 with real Node 25.2.1 and npm 11.6.2

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) - N/A; the supported range is declared in the package manifests and installer diagnostics
  • I've updated cli-config.yaml.example if I added/changed config keys - N/A; no config keys changed
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows - N/A; no architecture or workflow changed
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior - N/A; no model tool changed

Screenshots / Logs

Real-environment verification reproduced the base failure under Node 25.2.1 and confirmed that this branch rejects it at Hermes's own engine contract. POSIX and Windows installer gates both reject Node 25 and select the managed runtime.

@fangliquanflq
fangliquanflq requested a review from a team August 12, 2026 08:06
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/desktop Electron desktop app (apps/desktop/*) area/install-update Installer, updater, packaging, wheels, doctor P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(dashboard): stop Node 25 installs from failing at nanoid

The new range ^22.22.0 || ^24.0.0 || >=26.0.0 matches nanoid 6.0.0's own engines (^22 || ^24 || >=26, package-lock.json:14488), and install.sh / install.ps1 were updated consistently. Observations:

  1. tests-js/node-engine-alignment.test.ts hand-rolls a semver-range parser (satisfiesClause/satisfiesRange) that understands only ^, >=, <=, >, <, =. If the manifest range ever gains a ~, an x-range, or a prerelease floor (e.g. >=26.0.0-rc.1), the parser will silently return wrong answers (~ falls through to the = default after parseVersion mangles the bound into NaN). Either reject unsupported operators explicitly or add the semver package as a devDep instead of reimplementing the spec.
  2. The comment says "nanoid 6 deliberately excludes odd-numbered Node releases" — strictly, nanoid 6's engine range is ^22 || ^24 || >=26, which excludes 23/25 but allows 27+ via >=26. The repo mirrors that with >=26.0.0, so the range is consistent — but if Node 27 later proves incompatible, both manifests and both installers will need ^26.0.0, and the comment's blanket claim would mislead that decision. Consider tightening the wording.
  3. tests/test_engines_satisfiable.py now checks the managed runtime against the synthetic f"{managed_major}.999.999". That correctly handles the enumerated-LTS shape, but it can only prove the range accepts some release of that major, not the actual floor — a future patch-capped range (e.g. ~22.22.0) would pass this check while real Node 22.22.0 fails it. A comment noting the limitation would help.
  4. Minor: parseVersion discards prerelease tags by splitting on -; fine for the current ranges, but that is another divergence from real semver worth a one-line comment.

@fangliquanflq

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I investigated each point and updated the branch:

  1. Unsupported semver syntax: addressed. The JS alignment helper now rejects unsupported clauses explicitly, validates every || alternative before evaluation, and covers ~, x-ranges, prereleases, and an unsupported clause after an already-matching alternative.
  2. Odd-release wording: addressed. Both installers now state the precise nanoid behavior: Node 23 and 25 are excluded, while the >=26 arm permits later releases.
  3. Managed-major representative: no code change needed. The installer fetches latest-v{major}.x; the Python invariant intentionally checks that the provisioned major belongs to an accepted engine line, rather than claiming that {major}.0.0 is downloaded or validating a minimum patch.
  4. Prerelease handling: addressed. The test helper and both installer gates now reject prerelease versions instead of stripping or silently misclassifying them.

While validating the complete path, I also merged current upstream main, preserved its npm-missing and desktop lockfile behavior, and fixed the Windows installer to require a compatible npm alongside a supported system Node. This prevents accepted Node 24 installations with npm 11.16 from reaching npm install under engine-strict.

Validation completed:

  • npx vitest run tests-js/node-engine-alignment.test.ts — 12 passed
  • scripts/run_tests.sh tests/test_engines_satisfiable.py tests/test_install_sh_node_npm_check.py tests/test_install_ps1_managed_node_swap.py tests/test_install_ps1_node_path_for_npm.py — 19 passed
  • Prettier check, bash -n scripts/install.sh, and git diff --check — passed

Pushed as 94c4f9c60e2c679e9676a3eeb987d3b7d48589bb.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference — not a maintainer.

Thanks for the thorough pass — the explicit rejection of unsupported semver syntax, the tightened odd-release wording, the prerelease handling, and the documented rationale for the managed-major check all address the points. Looks consistent; we'll re-check on the updated branch.

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: hermes dashboard fails on Node 25 after update due to engine-strict and nanoid@6

3 participants