Skip to content

fix(desktop): force rebuild when packaged app is missing node-pty native binary (closes #62462) - #62564

Closed
Snowdchike wants to merge 1 commit into
NousResearch:mainfrom
Snowdchike:fix/desktop-linux-node-pty-missing
Closed

Snowdchike wants to merge 1 commit into
NousResearch:mainfrom
Snowdchike:fix/desktop-linux-node-pty-missing

Conversation

@Snowdchike

Copy link
Copy Markdown

Summary

hermes desktop on Linux could silently launch a packaged Electron app whose pty.node was missing, then crash in the main process with Cannot find module pty.node. The content-hash stamp only proved the source tree was unchanged — it did NOT verify the packaged artifact actually carried a usable native binary, so a previously-built (but broken) artifact could satisfy the stamp forever.

This adds two layers of defense at build-time so the failure surfaces as a loud rebuild, not a runtime crash.

Root cause

The user's reported behavior matches exactly what happens when both:

  • prebuilds/linux-x64/ is empty (no matching prebuild for Electron's Node ABI), AND
  • build/Release/ is empty (@electron/rebuild was never run)

…because both paths fall through, node-pty ships into the package with its JS surface intact but no .node binary, and Electron dies loading unixTerminal.js at startup.

The existing stamp check in _desktop_build_needed() would happily mark the broken package "up to date (content stamp matches)" because the source tree hadn't changed since the broken build was produced.

Changes

apps/desktop/scripts/stage-native-deps.mjs — fail loud, not silent

  • New validation step that throws when both build/Release/ and prebuilds/<platform>-<arch>/ lack a *.node binary or spawn-helper.
  • Error message points to the exact remediation: npx @electron/rebuild -w node-pty --platform <p> --arch <a>.

hermes_cli/main.py — native-deps gate alongside content stamp

  • New _desktop_native_deps_present() checks the same paths node-pty's loader checks (build/Release/, prebuilds/-/) plus lib/index.js for defensive coverage.
  • New _native_arch_suffix() returns the canonical <platform>-<arch> dir name (linux-x64, linux-arm64, darwin-arm64, win32-x64, …) without duplicating the lookup logic in 5 places.
  • New _desktop_unpacked_resources_root() centralizes how we locate app.asar.unpacked/.../resources/dist/node_modules/node-pty — stays in sync with _desktop_packaged_executable.
  • _desktop_build_needed() now returns True when native deps are missing, forcing a rebuild.

tests/hermes_cli/test_gui_command.py — coverage

  • _make_packaged_executable helper updated to stage realistic node-pty payload (lib/index.js + build/Release/pty.node) by default; with_native_deps=False opt-out for the missing-deps test.
  • 4 new tests: missing native binary forces rebuild even with fresh stamp; prebuild-only layout is accepted; lib/index.js regression is caught; arch-suffix mapping covers linux/darwin/win32.

Verification

$ ./venv/bin/python -m pytest tests/hermes_cli/test_gui_command.py -v
============================== 65 passed in 1.13s ==============================

End-to-end smoke test of the mjs change confirmed the new validation throws with the expected error message when both build/Release/ and prebuilds/linux-x64/ are empty.

Notes

  • Sir's machine has the binary at the correct path (apps/desktop/release/linux-unpacked/resources/app.asar.unpacked/dist/node_modules/node-pty/build/Release/pty.node) — this only affects installs where the staging step produced an incomplete payload.
  • The fix is conservative: it does NOT modify electron-builder config, asarUnpack, or before-pack.mjs. Those would be a larger change touching every platform's packaging flow. This PR isolates the change to: (1) fail loud during staging, and (2) detect + rebuild at the CLI gate.

Closes #62462

Fixes #62462 — `hermes desktop` would silently launch a packaged Electron
app whose `pty.node` was missing on Linux, then crash in the main process
with "Cannot find module pty.node". The content-hash stamp only proved
the source tree was unchanged; it did NOT verify the packaged artifact
actually carried a usable native binary, so a previously-built (but broken)
artifact could satisfy the stamp forever.

Two layers of defense, mirroring the issue's "likely cause" list:

1. `stage-native-deps.mjs` now throws (instead of warn) when no native
   binary is staged for the target. Empty build/Release/ AND empty
   prebuilds/<platform>-<arch>/ is the broken state we want to catch.

2. `_desktop_build_needed()` adds a native-deps gate alongside the
   content stamp. Looks at the same paths the renderer-side node-pty
   loader checks (build/Release/, prebuilds/<platform>-<arch>/) plus
   lib/index.js so a future layout change can't silently regress.

Tests: 65/65 pass (61 existing + 4 new covering missing-deps detection,
prebuild-only layout, lib/index.js regression, arch-suffix mapping).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #61798 (competing/complementary fix for the same missing-pty.node Linux Desktop crash family — that PR makes @electron/rebuild traverse the workspace root to produce the binary; this PR validates the packaged artifact and forces a rebuild when it's absent), #62462 (the bug this closes), and #62171 (bug spec). Different mechanisms — a maintainer should pick which layer to take (or both).

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists 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 Desktop crashes on Linux because node-pty pty.node is missing

2 participants