Skip to content

fix(desktop): rebuild node-pty for Electron when no prebuild covers the target - #63391

Closed
seojacknet-ops wants to merge 1 commit into
NousResearch:mainfrom
seojacknet-ops:main
Closed

seojacknet-ops wants to merge 1 commit into
NousResearch:mainfrom
seojacknet-ops:main

Conversation

@seojacknet-ops

Copy link
Copy Markdown

Problem

On Linux, the packaged desktop app crashes at launch:

Error: Failed to load native module: pty.node, checked: build/Release, build/Debug, prebuilds/linux-x64

node-pty publishes prebuilds for darwin/win32 but not linux, and its compile-on-install script is blocked by the workspace allowScripts policy — so Linux packs ship node-pty with no native binary at all. stage-native-deps.mjs already warns about this, but the build still succeeds and the app crashes at runtime.

Even with the install script approved, node-gyp rebuild compiles against the host Node ABI, which Electron refuses to load ("compiled against a different Node.js version").

Fix

  • New apps/desktop/scripts/ensure-node-pty-electron.mjs, wired into the desktop build script ahead of stage-native-deps.mjs. It's a no-op when a published prebuild exists for the host platform/arch, or when build/Release/pty.node was already rebuilt for the current Electron version (tracked via a stamp file in build/Release, so a fresh npm ci always retriggers). Otherwise it runs @electron/rebuild -w node-pty against the workspace's Electron version and fails the build loudly if that doesn't produce pty.node.
  • Approve node-pty's install script in the root allowScripts.
  • stage-native-deps.mjs: only warn about a missing prebuild when build/Release/pty.node is also missing, so healthy Linux builds don't print a scary warning.

Verification

  • ensure-node-pty-electron.mjs rebuilds on first run, no-ops on the second (stamp match).
  • Full hermes desktop pack on Fedora 44 (linux-x64, Electron 40.10.2): pty.node lands in app.asar.unpacked, and the packaged Electron loads node-pty and spawns a PTY successfully.
  • Packaged app launches and runs; the previous main-process crash is gone.

🤖 Generated with Claude Code

…he target

node-pty publishes no linux prebuild, and its compile-on-install script was
blocked by allowScripts — so Linux packs shipped without pty.node and the
packaged app crashed on launch ("Failed to load native module: pty.node").
Even with the script approved, npm builds against the host Node ABI, which
Electron refuses to load.

- add scripts/ensure-node-pty-electron.mjs: before staging, rebuild node-pty
  via @electron/rebuild when there is no published prebuild for the host
  target and no stamp-verified Electron-ABI build; no-op otherwise
- wire it into the desktop "build" script ahead of stage-native-deps
- approve node-pty's install script in the workspace allowScripts
- stage-native-deps: only warn about a missing prebuild when build/Release
  has no pty.node either

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@seojacknet-ops
seojacknet-ops requested a review from a team July 12, 2026 20:20
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists labels Jul 12, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #61798 (competing fix for the same verified Linux node-pty-missing desktop crash), and issues #62171 (bug spec) / #62462 (dup report). Different mechanism: #61798 makes @electron/rebuild traverse the npm workspace root and adds a launcher completeness gate; this PR adds a dedicated pre-stage ensure-node-pty-electron.mjs rebuild script plus an allowScripts approval. Not a duplicate -- a maintainer should pick between the two approaches.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused Linux desktop packaging fix. This automated hermes-sweeper review found that the requested behavior is already implemented on current main.

  • apps/desktop/scripts/stage-native-deps.mjs:260-305 detects a missing target native payload, runs electron-rebuild -f -w node-pty, fails loudly if rebuilding fails, and stages the resulting build/Release output. This behavior was introduced by c008f41bb979257e0467e771550fd67d77ce75c0.
  • Current main additionally handles target architecture and rejects wrong-platform staged binaries at apps/desktop/scripts/stage-native-deps.mjs:248-309 (7a44a8fdec45cb27ee6f8c24470bdfa6a17272ce).
  • The normal desktop build already runs this staging path at apps/desktop/package.json:22.

The related competing fix, #61798, was also closed by its author as superseded after auditing these landed commits. Closing as implemented on main.

@teknium1 teknium1 closed this Jul 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 15, 2026
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/*) P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants