Skip to content

fix(desktop): make managed Node authoritative for Windows install scripts - #91188

Closed
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/windows-managed-node-authority
Closed

andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/windows-managed-node-authority

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #82309 and supersedes stale/conflicting PR #82355 while preserving its exact implementation and original author identity.

Hermes can invoke npm through its managed Node while npm lifecycle children resolve a different, older system node from inherited PATH. Electron postinstall then fails with ERR_REQUIRE_ESM, even though Hermes already provisioned a compatible runtime.

This current-main rebase makes with_hermes_node_path() authoritative:

  • normalize PATH entries using Windows path semantics when applicable;
  • remove all existing case/trailing-separator variants of Hermes-managed Node directories;
  • prepend the canonical managed directories ahead of system Node;
  • preserve unrelated PATH order;
  • test both a stripped PATH and a managed path already sitting behind system Node.

Provenance

This is the exact #82355 commit by @fangliquanflq rebased through GitHub onto current main. The rebased commit retains @fangliquanflq as the Git author:

The rebase applied cleanly despite intervening hermes_constants.py changes; Git preserved the current file and placed the original hunk at its new location. This restores a mergeable delivery route without replacing contributor provenance.

Exact-head verification

All hosted workflows attached directly to 150b1973... are green:

No status is inherited from #82355 or its former base.

Scope

This closes managed-Node PATH authority for install/build subprocesses. The package-level electron-builder runtime handoff remains independently enforced by #91079; together they cover both dependency-install lifecycle children and the terminal builder wrapper. This does not claim the broader transactional deployment architecture in #88683 is complete.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows duplicate This issue or pull request already exists labels Aug 21, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #82355. The live diffs are the same managed-Node PATH normalization and tests; #82355 is already open.

Copy link
Copy Markdown
Contributor Author

Topology correction: #82355 remains open and currently mergeable, and this PR carries the same three-file managed-Node PATH surface. The rebased head here served as current-main verification, but it should not replace @fangliquanflq's original PR.

I posted the exact-head validation receipt back to #82355 and am closing this duplicate. The verified rebased evidence remains linked from the canonical PR.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] Desktop dependency install fails with ERR_REQUIRE_ESM when a system Node precedes the managed Node on PATH

3 participants