Skip to content

fix(install): require Node >=20.19/22.12 for the desktop build - #38255

Merged
OutThisLife merged 1 commit into
mainfrom
bb/installer-desktop-build-logging
Jun 3, 2026
Merged

fix(install): require Node >=20.19/22.12 for the desktop build#38255
OutThisLife merged 1 commit into
mainfrom
bb/installer-desktop-build-logging

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

Summary

The "Build desktop app" install step fails with an opaque exit code 1 on machines with an old Node (reported on Apple Silicon by @Pixo14 / others). I reproduced the exact failure rather than guessing.

Reproduced — same repo, same deps, only Node version changed:

Node npm run build (tsc + vite)
v22.22.3 ✅ builds in ~10s
v20.5.1 exit code 1

On v20.5.1:

You are using Node.js 20.5.1. Vite requires Node.js version 20.19+ or 22.12+. Please upgrade your Node.js version.
SyntaxError: The requested module 'node:util' does not provide an export named 'styleText'

Root cause. Vite 8 (via rolldown) imports styleText from node:util, which doesn't exist before Node 20.12 — so vite build hard-crashes before producing the app. It dies right after write-build-stamp / stage-native-deps, matching the reporter's screenshot. The installer accepted any pre-existing Node with no version floor:

check_node() {
    log_info "Checking Node.js (for browser tools)..."

    if command -v node &> /dev/null; then
        local found_ver=$(node --version)
        log_success "Node.js $found_ver found"
        HAS_NODE=true
        return 0

Test-Node in install.ps1 had the identical gap. So a too-old system Node was used for the build instead of the bundled Node 22.

Fix

  • Add a version floor matching Vite's range (^20.19 || >=22.12) to check_node (install.sh) and Test-Node (install.ps1). A too-old system Node is replaced with the Hermes-managed Node 22 LTS the installer already knows how to fetch.
  • Make the desktop build stage re-resolve Node (it previously only did so when npm was entirely missing, so an old system Node slipped through).
  • Declare "engines": { "node": "^20.19.0 || >=22.12.0" } in apps/desktop/package.json.

Test plan

  • Reproduced the failure on Node 20.5.1 and the pass on Node 22 (above)
  • npm run build still green on Node 22 after the change
  • bash -n scripts/install.sh clean
  • Unit-tested the node_satisfies_build floor across boundary versions — rejects 18.x / 20.5.1 / 20.18.9 / 21.7.0 / 22.11.0; accepts 20.19 / 20.20 / 22.12 / 22.22 / 24.0
  • install.ps1 not run — no pwsh on my machine; the PowerShell mirrors the bash logic 1:1 but needs a Windows/old-Node run to confirm

Scope note

This fixes the build failure itself. It does not touch the separate "the failure produced no readable log" diagnosability issue — intentionally kept out of scope.

The "Build desktop app" install step failed with an opaque "exit code 1"
on machines with an old Node, and nothing in the logs explained it.

Reproduced: on Node 20.5.1, `npm run pack`'s `vite build` crashes with

  You are using Node.js 20.5.1. Vite requires Node.js version 20.19+ or 22.12+.
  SyntaxError: The requested module 'node:util' does not provide an
  export named 'styleText'

Vite 8 (rolldown) imports node:util.styleText, which doesn't exist before
Node 20.12, so the build dies before producing the app. The installer's
check_node / Test-Node accepted ANY pre-existing Node with no version
floor, so a too-old system Node was used for the build instead of the
bundled Node 22.

Add a version floor (^20.19 || >=22.12) to check_node (install.sh) and
Test-Node (install.ps1): a too-old system Node is replaced with the
Hermes-managed Node 22 LTS, and the desktop stage re-resolves Node so the
build always runs on a satisfying version. Declare the same range in
apps/desktop/package.json engines.

Verified: build succeeds on Node 22, fails on 20.5.1 with the error above;
the floor logic matches Vite's range across boundary versions (20.18/20.19,
21.x, 22.11/22.12).
@OutThisLife
OutThisLife requested a review from a team June 3, 2026 14:19
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: bb/installer-desktop-build-logging vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9733 on HEAD, 9733 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5043 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@OutThisLife
OutThisLife enabled auto-merge June 3, 2026 14:24
@alt-glitch alt-glitch added type/bug Something isn't working area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jun 3, 2026
@austinpickett
austinpickett requested a review from Copilot June 3, 2026 14:37
@OutThisLife
OutThisLife merged commit 12ea7fc into main Jun 3, 2026
20 checks passed
@OutThisLife
OutThisLife deleted the bb/installer-desktop-build-logging branch June 3, 2026 14:38

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

Note

Copilot was unable to run its full agentic suite in this review.

Updates the install scripts to enforce a minimum Node.js version required by the desktop build (Vite ^8), reducing opaque build failures on older Node versions.

Changes:

  • Add Node version “floor” checks (^20.19 || >=22.12) and prefer/install a Hermes-managed Node when the system Node is too old.
  • Make desktop install stages re-resolve Node in each stage/process to avoid relying on prior stage state.
  • Declare the desktop app’s Node engine requirement in apps/desktop/package.json.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
scripts/install.sh Adds Node version gating + forces re-check before desktop build to avoid Vite-on-old-Node failures.
scripts/install.ps1 Adds version gating and re-checks Node in desktop stage across separate PowerShell processes.
apps/desktop/package.json Declares Node engine requirement matching Vite’s supported Node range.
Comments suppressed due to low confidence (1)

scripts/install.sh:1

  • node --version (and the Hermes-managed equivalent) is invoked multiple times in the same branch. Capture the version once into a local variable and reuse it for both node_satisfies_build and logging to avoid redundant subprocess calls and make the logic easier to read.
#!/bin/bash

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

Comment thread scripts/install.sh
Comment on lines 722 to +725
check_node() {
log_info "Checking Node.js (for browser tools)..."

if command -v node &> /dev/null; then
local found_ver=$(node --version)
log_success "Node.js $found_ver found"
if command -v node &> /dev/null && node_satisfies_build "$(node --version)"; then
Comment thread scripts/install.sh
Comment on lines +725 to +726
if command -v node &> /dev/null && node_satisfies_build "$(node --version)"; then
log_success "Node.js $(node --version) found"
Comment thread scripts/install.ps1
Comment on lines 737 to +739
if (Get-Command node -ErrorAction SilentlyContinue) {
$version = node --version
Write-Success "Node.js $version found"
$script:HasNode = $true
return $true
if (Test-NodeVersionOk $version) {
Comment thread scripts/install.ps1
Comment on lines +749 to 750
if ((Test-Path $managedNode) -and (Test-NodeVersionOk (& $managedNode --version))) {
$version = & $managedNode --version
davidgut1982 pushed a commit to davidgut1982/hermes-agent that referenced this pull request Jun 5, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
@abiboe

abiboe commented Jun 7, 2026

Copy link
Copy Markdown

Thank you @OutThisLife for the quick fix! This was exactly the issue I was encountering. The version floor check properly catches the Node 22.11.0 incompatibility now. Really appreciate the detailed PR and quick merge.

alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
…d-logging

fix(install): require Node >=20.19/22.12 for the desktop build
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…desktop-build-logging

fix(install): require Node >=20.19/22.12 for the desktop build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles 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.

5 participants