Skip to content

fix(docker): work around npm 10.x's arborist crash in the TS builders - #4787

Open
AmirF194 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AmirF194:fix/4782-docker-npm-arborist-crash
Open

fix(docker): work around npm 10.x's arborist crash in the TS builders#4787
AmirF194 wants to merge 1 commit into
modelcontextprotocol:mainfrom
AmirF194:fix/4782-docker-npm-arborist-crash

Conversation

@AmirF194

@AmirF194 AmirF194 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Bisected this a bit further than the issue's own finding: it isn't @vitest/coverage-v8 specifically, plain vitest alone triggers the crash, even in an otherwise empty package.json. So it's an npm 10.x arborist bug in peer-dependency resolution, not anything about this repo's dependency graph. Node 24's bundled npm 11.19.0 resolves the exact same package.json with no errors and no extra flags; npm 10.9.8 (what the unpinned node:22-alpine tag ships today) still crashes.

release.yml already upgrades past this for a different reason (OIDC trusted publishing needs npm >=11.5.1, and its own comment says node 22's bundled npm is 10.x), so I did the same thing in each builder stage rather than adding a --legacy-peer-deps workaround or bumping the base image's node major. Confirmed npm 10.9.x in the release stage still reads a lockfile that npm 11 wrote via npm ci, so nothing downstream needs to change.

One thing that surprised me: none of the "Build"/"Test" CI jobs actually build these Dockerfiles. They run npm ci from a full monorepo checkout against the checked-in root workspace lockfile, a different install path that never hits this, which is probably why it shipped unnoticed.

Ran docker buildx build --no-cache for all four servers against a fresh main checkout first and got the issue's exact error on every one, then again on this branch where all four build clean. Also ran each built image through a real MCP initialize handshake over stdio to confirm it still starts and responds, and drove filesystem the same way with a mounted directory.

The builder stage's npm install crashes on all four TS servers with
"Cannot read properties of null (reading 'edgesOut')", reproduced on
unmodified main with a fully cleared build cache. Bisected past the
issue's own finding: plain vitest alone triggers it, in an otherwise
empty package.json, so it isn't specific to this repo's dependency
graph. It's an npm 10.x arborist bug; npm 11.19.1 resolves the same
package.json with no errors and no extra flags.

release.yml already upgrades past this for a different reason
(OIDC trusted publishing needs npm >=11.5.1, and node 22's bundled
npm is 10.x). Do the same in each builder stage before npm install.
Confirmed npm 10.9.x in the release stage still reads a lockfile
that npm 11 wrote with npm ci, so nothing downstream needs to change.

Verified with docker buildx build --no-cache for all four servers:
fails on unmodified main with the issue's exact error, builds clean
on this branch. Ran each built image through a real MCP initialize
handshake over stdio to confirm it still starts and responds.

Fixes modelcontextprotocol#4782
Copilot AI balanced review requested due to automatic review settings September 9, 2026 21:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants