fix(deps): allow node-pty's install script so its native binary gets built - #554
Open
RonenMars wants to merge 1 commit into
Open
fix(deps): allow node-pty's install script so its native binary gets built#554RonenMars wants to merge 1 commit into
RonenMars wants to merge 1 commit into
Conversation
…built npm 12 blocks a dependency's install scripts unless allowScripts names it, and node-pty builds its addon through exactly that mechanism. It was missing from the map, so npm install and npm rebuild both skipped its node-gyp rebuild and left node_modules/node-pty with no .node binary. The failure is silent and total: the install exits 0 with one warning among several, and the streamer then cannot spawn a single session, because node-pty fails to import. Observed on npm 12.0.2 while refreshing dependencies for a 1.48.0 deploy; only a direct node-gyp invocation produced the binary. This completes #466, which introduced allowScripts for better-sqlite3 and left the other native dependency uncovered. A test asserts both stay allow-listed, since nothing else would notice an entry disappearing.
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.
A clean
npm installon npm 12 produces a streamer that cannot spawn a single session, and nothing about the install says so.npm 12 blocks a dependency's install scripts unless
allowScriptsnames the package.node-ptybuilds its native addon through exactly that mechanism and was not in the map, so itsnode-gyp rebuildnever ran andnode_modules/node-pty/build/Release/pty.nodewas never produced.How it presents
The install exits 0.
The only signal is one line among several warnings:
Then every session start fails, because
node-ptycannot be imported — and the streamer imports it dynamically precisely so a missing native module degrades instead of crashing the process, which is what keeps this quiet.npm rebuild node-ptydoes not fix it. It is blocked by the same rule and still reportsrebuilt dependencies successfully. Only a directnpx node-gyp rebuildinsidenode_modules/node-ptyproduces the binary.Verified
Observed 2026-08-12 on npm 12.0.2 while refreshing dependencies before deploying 1.48.0:
After the direct node-gyp build,
pty.node(90 712 bytes) appeared and a real spawn round-tripped (pty output: pty-ok, exit 0).Why it was half-fixed
#466 introduced
allowScriptsto solve this same npm 12 breakage forbetter-sqlite3, and covered only that package.This adds the other native dependency.
The test
__tests__/allow-scripts.test.tsasserts each native dependency is both still a runtime dependency and still allow-listed.The two halves are paired so they cannot drift: removing the dependency should remove its entry rather than orphan it.
Verified to fail without the fix — deleting the
node-ptyline turns× node-pty is allow-listed so its build actually runsred.It guards the manifest rather than the build, deliberately: CI installs with its own flags and would not reproduce a developer's blocked-script state, so a test that tried to assert the binary exists would pass in CI while the bug shipped.