fix(tui): pass --expose-gc as node argv instead of NODE_OPTIONS - #29998
Merged
Conversation
Node refuses to start when NODE_OPTIONS contains --expose-gc:
node: --expose-gc is not allowed in NODE_OPTIONS
NODE_OPTIONS is restricted to a small allowlist of flags that are safe
to inject via env (since any process able to set env vars on a node
child could otherwise enable arbitrary capabilities). --expose-gc is
not on that list and never has been -- it must be passed as a direct
CLI flag.
_launch_tui() was appending --expose-gc to NODE_OPTIONS before spawning
the TUI's node process, which made `hermes --tui` fail to start on
every modern node release. The intent (manual GC for long sessions to
avoid fatal-OOM) is preserved by inserting --expose-gc directly into
the node argv in _make_tui_argv() -- same effect, but actually allowed.
--max-old-space-size=8192 stays in NODE_OPTIONS: it *is* allowlisted,
and keeping it there means downstream node spawns inherit the same
heap cap without having to re-thread the flag through every spawn site.
The dev paths (`tsx src/entry.tsx` and `npm start` fallback) are left
alone -- they don't accept node flags directly, and the production
dist path is the one users actually hit via `hermes --tui`.
Repro before fix:
$ hermes --tui
/usr/bin/node: --expose-gc is not allowed in NODE_OPTIONS
The original PR fixed the ext_dir and built-tui paths but missed the sibling pip-wheel path at line 1155. Without this, wheel installs would lose --expose-gc entirely (the env-var append at the call site was already removed). All three production node-launch sites now pass --expose-gc via argv consistently.
Contributor
🔎 Lint report:
|
3 tasks
alt-glitch
added
type/bug
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #21711 by @YarrowQiao onto current main + sibling-site fix.
Summary
hermes --tuifails to start on Node versions that enforce theNODE_OPTIONSallowlist:--expose-gcis not in Node's allowlist forNODE_OPTIONS(security: env-var-set flags must be safe to inject) and must be passed as a direct CLI flag. The fix moves it from theNODE_OPTIONSbuilder in_launch_tui()into the node argv built by_make_tui_argv().--max-old-space-size=8192stays inNODE_OPTIONS(it is allowlisted) so downstream node spawns inherit the heap cap.Changes
hermes_cli/main.py— drop--expose-gcfromNODE_OPTIONS, insert it as argv[1] on all three production node-launch paths (ext_dir, wheel-bundled, built tui_dir).scripts/release.py— mapyichengqiao21@gmail.com→YarrowQiaofor changelog attribution.Sibling-site fix added during salvage
Original PR covered the
ext_dirandtui_dirpaths but missed the pip-wheel path (bundled). Without that, wheel installs would have lost--expose-gcentirely. Now all three sites are consistent.Validation
origin/mainbefore the fix (grep shows--expose-gcappended toNODE_OPTIONS).NODE_OPTIONS='--expose-gc' node ...rejected on affected Node builds;node --expose-gc ...accepted universally.py_compileclean.hermes --tuifails with--expose-gc is not allowed in NODE_OPTIONS#17187 and Discord report.Credit to @YarrowQiao — original commit preserved via cherry-pick.
Infographic