fix(tui): pass --expose-gc as node argv instead of NODE_OPTIONS - #21711
Closed
YarrowQiao wants to merge 1 commit into
Closed
fix(tui): pass --expose-gc as node argv instead of NODE_OPTIONS#21711YarrowQiao wants to merge 1 commit into
YarrowQiao wants to merge 1 commit into
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
Collaborator
Contributor
|
Merged via PR #29998 — your commit was cherry-picked onto current main with your authorship preserved in git log (2ea7cf2). Added a small sibling-site fix on top (wheel-bundled launch path was missing --expose-gc too). Thanks for the clean fix and the great commit message explaining the NODE_OPTIONS allowlist rationale! Closes #17187 too. |
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
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.
Summary
hermes --tuifails to start on every modern Node release with:_launch_tui()was appending--expose-gctoNODE_OPTIONSbefore spawning the TUI. Node restrictsNODE_OPTIONSto a small allowlist (so anything able to set env vars on a node child can't unilaterally enable arbitrary capabilities) —--expose-gcis not on that list and never has been. It must be passed as a direct argv flag.The intent (manual GC for long sessions to avoid fatal-OOM on large transcripts / reasoning blobs) is preserved by inserting
--expose-gcdirectly into thenodeargv inside_make_tui_argv()— same effect, but actually allowed.--max-old-space-size=8192stays inNODE_OPTIONSbecause it is allowlisted, and keeping it there means downstreamnodespawns inherit the same heap cap without re-threading the flag through every spawn site.The dev paths (
tsx src/entry.tsxandnpm startfallback) are intentionally left alone — they don't acceptnodeflags directly, and the production dist path is the one users actually hit viahermes --tui. The shebang inui-tui/src/entry.tsxalready documents the intended flags for direct execution.Repro (before fix)
Test plan
hermes --tuistarts without the--expose-gc is not allowederror on Node v18.20.8 (Linux/WSL2)import hermes_cli.mainsucceeds (no syntax regressions)global.gc()