Skip to content

fix: narrow over-broad default pack exclude patterns#249

Open
adityasingh2400 wants to merge 1 commit into
modelcontextprotocol:mainfrom
adityasingh2400:fix/pack-exclude-patterns-too-broad
Open

fix: narrow over-broad default pack exclude patterns#249
adityasingh2400 wants to merge 1 commit into
modelcontextprotocol:mainfrom
adityasingh2400:fix/pack-exclude-patterns-too-broad

Conversation

@adityasingh2400
Copy link
Copy Markdown

The default EXCLUDE_PATTERNS in mcpb pack drop real runtime code from the bundle in two cases, so the packed .mcpb installs but crashes at module-load time.

*.map matches any path component ending in .map, including directories. Packages such as es-iterator-helpers ship directories named Iterator.prototype.map/ and Iterator.prototype.flatMap/ whose contents are ordinary .js files. The whole directory was excluded while sibling directories (Iterator.prototype.filter/, etc.) were kept, producing a Cannot find module .../Iterator.prototype.map/index.js error at startup. This change narrows the pattern to actual source maps: *.js.map, *.cjs.map, *.mjs.map, *.css.map, *.ts.map.

node_modules/.bin excluded all CLI shims. Some packages reference these at module-load time (for example @salesforce/code-analyzer-retirejs-engine calls findCommand("retire") at the top level of its executor.js) and throw if the shim is missing. Removing it from the defaults keeps the shims in the bundle. Users who do not want them can still add node_modules/.bin to their own .mcpbignore. node_modules/.cache stays excluded, since it only holds build caches.

Testing:

  • Added regression tests in test/mcpbignore.test.ts asserting that source maps are still excluded, that .map-suffixed directories and their files are kept, and that node_modules/.bin shims are kept. Two of these use getAllFiles against a temp tree that reproduces the es-iterator-helpers layout.
  • yarn build, yarn lint, and yarn test all pass (132 tests).

Fixes #241

The default EXCLUDE_PATTERNS dropped real runtime code from packed
bundles in two cases:

- "*.map" matched any path component ending in ".map", including
  directories. Packages like es-iterator-helpers ship directories named
  Iterator.prototype.map/ whose contents are ordinary .js files, so the
  whole directory was excluded and the bundle crashed at module load.
  This narrows the pattern to actual source maps (*.js.map, *.cjs.map,
  *.mjs.map, *.css.map, *.ts.map).
- "node_modules/.bin" excluded the CLI shim directory, which some
  packages reference (for example via existsSync) at module-load time.
  Removing it from the defaults keeps those shims in the bundle; users
  who want it gone can add it to their own .mcpbignore.

Adds regression tests covering both cases.

Fixes modelcontextprotocol#241
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

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.

Default EXCLUDE_PATTERNS in mcpb pack is too broad

1 participant