Skip to content

fix(deps): allow node-pty's install script so its native binary gets built - #554

Open
RonenMars wants to merge 1 commit into
mainfrom
fix/allowscripts-node-pty
Open

fix(deps): allow node-pty's install script so its native binary gets built#554
RonenMars wants to merge 1 commit into
mainfrom
fix/allowscripts-node-pty

Conversation

@RonenMars

Copy link
Copy Markdown
Owner

A clean npm install on 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 allowScripts names the package.
node-pty builds its native addon through exactly that mechanism and was not in the map, so its node-gyp rebuild never ran and node_modules/node-pty/build/Release/pty.node was never produced.

How it presents

The install exits 0.
The only signal is one line among several warnings:

npm warn install-scripts   node-pty@1.1.0 (install: node-gyp rebuild)
npm warn install-scripts Run `npm install-scripts ls` to review, or `npm install-scripts approve <pkg>` to allow.

Then every session start fails, because node-pty cannot 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-pty does not fix it. It is blocked by the same rule and still reports rebuilt dependencies successfully. Only a direct npx node-gyp rebuild inside node_modules/node-pty produces the binary.

Verified

Observed 2026-08-12 on npm 12.0.2 while refreshing dependencies before deploying 1.48.0:

better-sqlite3: build/Release/better_sqlite3.node   ✓   (allow-listed)
node-pty:       MISSING                             ✗   (not allow-listed)

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 allowScripts to solve this same npm 12 breakage for better-sqlite3, and covered only that package.
This adds the other native dependency.

The test

__tests__/allow-scripts.test.ts asserts 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-pty line turns × node-pty is allow-listed so its build actually runs red.

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.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant