fix: Migrate MCPB build to esbuild with ESM compatibility fixes#360
Closed
wonderwhy-er wants to merge 1 commit intomainfrom
Closed
fix: Migrate MCPB build to esbuild with ESM compatibility fixes#360wonderwhy-er wants to merge 1 commit intomainfrom
wonderwhy-er wants to merge 1 commit intomainfrom
Conversation
Replace tsc+copy build approach with esbuild single-file bundling for faster builds and smaller bundles. Includes three post-processing fixes for esbuild ESM compatibility issues: 1. ESM shims: Inject createRequire, __dirname, __filename so CJS modules bundled into ESM format can use require() and path globals 2. Async propagation: Fix esbuild bug where __esm wrapper callbacks that transitively depend on async modules don't get marked as async, causing 'SyntaxError: Unexpected reserved word' for await in non-async functions 3. isMainModule guard: Strip device.ts auto-start block that incorrectly triggers in the single-file bundle (import.meta.url always matches process.argv[1]), which outputs debug messages to stdout and breaks Claude Desktop's MCP JSON-RPC transport Also: - Replace top-level await in trackTools.ts with sync mkdirSync - Keep native modules (sharp, ripgrep) and CJS-only packages (pdf2md, pdf-lib) as external dependencies installed separately
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Contributor
📝 WalkthroughWalkthroughThe pull request replaces the original TypeScript build process in the build script with a new esbuild-based bundling workflow, including post-processing, manifest validation, and dependency management. Additionally, a logging utility is updated to use synchronous directory creation instead of asynchronous operations. Changes
Possibly related PRs
Suggested labels
Poem
Estimated code review effort: 🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
scripts/build-mcpb.cjs (5)
184-189: No guard if an expected dependency is missing from rootpackage.json.If any of these four packages are removed or renamed in the root
package.json, the version will silently beundefined, producing invalid JSON that causes a confusingnpm installfailure later. Consider adding an early check:Example guard
const requiredExternal = ['sharp', '@vscode/ripgrep', '@opendocsg/pdf2md', 'pdf-lib']; for (const dep of requiredExternal) { if (!packageJson.dependencies?.[dep]) { console.error(`❌ Missing required dependency "${dep}" in root package.json`); process.exit(1); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-mcpb.cjs` around lines 184 - 189, Add an early validation that packageJson.dependencies contains the four required keys ("sharp", "@vscode/ripgrep", "@opendocsg/pdf2md", "pdf-lib") before assembling the dependencies object so you never inject undefined versions into the generated JSON; check packageJson.dependencies for each required dependency, log a clear error including the missing package name (e.g. using console.error), and call process.exit(1) if any are absent—place this guard near the top of the script before the code that builds the dependencies object that references packageJson.dependencies.
156-171:package.jsonis copied then immediately overwritten.Line 157 includes
package.jsoninfilesToCopy, but Step 6 (lines 191-194) overwritesBUNDLE_DIR/package.jsonwith a new object. The initial copy is wasted work and could mislead future maintainers into thinking the rootpackage.jsonis used as-is.Proposed fix
-const filesToCopy = ['package.json', 'README.md', 'LICENSE', 'PRIVACY.md', 'icon.png']; +const filesToCopy = ['README.md', 'LICENSE', 'PRIVACY.md', 'icon.png'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-mcpb.cjs` around lines 156 - 171, The script currently copies package.json via the filesToCopy array then later replaces BUNDLE_DIR/package.json with a generated object, so remove that wasted step by excluding "package.json" from filesToCopy (or add a conditional that skips copying when a later write will overwrite it); update the filesToCopy declaration or the copy loop around the filesToCopy symbol and ensure the later package.json generation code that writes to BUNDLE_DIR (the write that creates the final bundle package.json) remains unchanged.
113-122: Regex forisMainModuleremoval is fragile with nested braces.The trailing
[\s\S]*?\n\}will match the first\n}after theif (isMainModule) {opening. If esbuild ever emits nested braces inside that block (e.g., a try/catch or object literal), this will truncate the removal and leave broken JS in the bundle.Consider adding a post-write syntax check (e.g.,
node --check mcpb-bundle/dist/index.js) after line 124 to catch this class of breakage at build time rather than at runtime.Proposed addition after line 124
fs.writeFileSync(bundlePath, bundleContent); + // Syntax-check the post-processed bundle to catch regex replacement errors early + try { + execSync(`node --check "${bundlePath}"`, { cwd: PROJECT_ROOT, stdio: 'pipe' }); + } catch (syntaxError) { + console.error('❌ Post-processed bundle has syntax errors:', syntaxError.stderr?.toString() || syntaxError.message); + process.exit(1); + } console.log('✅ Post-processing completed');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-mcpb.cjs` around lines 113 - 122, The current isMainModulePattern replacement in build-mcpb.cjs (isMainModulePattern / bundleContent) is brittle because the regex can stop at the first closing brace and leave broken JS when nested braces exist in the removed block; update the fix to robustly remove the auto-start block by either parsing the bundleContent with a JS parser (e.g., acorn/estree) to locate the if (isMainModule) { ... } AST node and remove its full range, or implement a brace-matching routine that consumes nested braces reliably for the if (isMainModule) block in device.ts, and after performing the replacement add a post-write syntax check (run node --check on the emitted bundle) to fail the build if the resulting bundle is invalid.
63-63: Typo:shebanLine→shebangLine.Minor naming nit — "shebang" is the standard spelling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-mcpb.cjs` at line 63, Rename the misspelled variable shebanLine to the correct shebangLine across the script (where it's declared as const shebanLine = '#!/usr/bin/env node'; and any usages) to use the standard "shebang" spelling; update all references (declaration and any reads/writes) to shebangLine so the code compiles and names are consistent.
196-204:--productionflag is redundant with--omit=dev.
npm install --omit=devalready excludes devDependencies. The--productionflag is its legacy equivalent and is deprecated in npm v9+. Using both works but the redundancy may cause confusion.Proposed fix
- execSync('npm install --omit=dev --production', { cwd: BUNDLE_DIR, stdio: 'inherit' }); + execSync('npm install --omit=dev', { cwd: BUNDLE_DIR, stdio: 'inherit' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/build-mcpb.cjs` around lines 196 - 204, In the try/catch block that runs execSync to install dependencies, remove the redundant `--production` flag from the command string so it uses only `npm install --omit=dev` (the execSync call that references BUNDLE_DIR); update the execSync invocation accordingly to avoid mixing legacy and modern flags while keeping stdio and cwd behavior the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build-mcpb.cjs`:
- Around line 89-103: The code uses a magic fallback blockEnd = blockStart +
10000 when no nextInit is found, which can truncate the final __esm block and
miss await checks; update the logic in the loop that uses esmBlockRegex and
bundleContent so that when nextInit === -1 you use bundleContent.length (e.g.,
blockEnd = bundleContent.length) instead of blockStart + 10000, ensuring
blockBody contains the full remaining content before checking for 'await' and
pushing fixes into the fixes array.
---
Nitpick comments:
In `@scripts/build-mcpb.cjs`:
- Around line 184-189: Add an early validation that packageJson.dependencies
contains the four required keys ("sharp", "@vscode/ripgrep",
"@opendocsg/pdf2md", "pdf-lib") before assembling the dependencies object so you
never inject undefined versions into the generated JSON; check
packageJson.dependencies for each required dependency, log a clear error
including the missing package name (e.g. using console.error), and call
process.exit(1) if any are absent—place this guard near the top of the script
before the code that builds the dependencies object that references
packageJson.dependencies.
- Around line 156-171: The script currently copies package.json via the
filesToCopy array then later replaces BUNDLE_DIR/package.json with a generated
object, so remove that wasted step by excluding "package.json" from filesToCopy
(or add a conditional that skips copying when a later write will overwrite it);
update the filesToCopy declaration or the copy loop around the filesToCopy
symbol and ensure the later package.json generation code that writes to
BUNDLE_DIR (the write that creates the final bundle package.json) remains
unchanged.
- Around line 113-122: The current isMainModulePattern replacement in
build-mcpb.cjs (isMainModulePattern / bundleContent) is brittle because the
regex can stop at the first closing brace and leave broken JS when nested braces
exist in the removed block; update the fix to robustly remove the auto-start
block by either parsing the bundleContent with a JS parser (e.g., acorn/estree)
to locate the if (isMainModule) { ... } AST node and remove its full range, or
implement a brace-matching routine that consumes nested braces reliably for the
if (isMainModule) block in device.ts, and after performing the replacement add a
post-write syntax check (run node --check on the emitted bundle) to fail the
build if the resulting bundle is invalid.
- Line 63: Rename the misspelled variable shebanLine to the correct shebangLine
across the script (where it's declared as const shebanLine = '#!/usr/bin/env
node'; and any usages) to use the standard "shebang" spelling; update all
references (declaration and any reads/writes) to shebangLine so the code
compiles and names are consistent.
- Around line 196-204: In the try/catch block that runs execSync to install
dependencies, remove the redundant `--production` flag from the command string
so it uses only `npm install --omit=dev` (the execSync call that references
BUNDLE_DIR); update the execSync invocation accordingly to avoid mixing legacy
and modern flags while keeping stdio and cwd behavior the same.
Comment on lines
+89
to
+103
| while ((match = esmBlockRegex.exec(bundleContent)) !== null) { | ||
| if (match[3]) continue; // Already async | ||
| const modName = match[4]; | ||
| const blockStart = match.index; | ||
| const nextInit = bundleContent.indexOf('\nvar init_', blockStart + match[0].length); | ||
| const blockEnd = nextInit !== -1 ? nextInit : blockStart + 10000; | ||
| const blockBody = bundleContent.slice(blockStart, blockEnd); | ||
| if (blockBody.includes('await ')) { | ||
| fixes.push({ | ||
| position: match.index + match[0].indexOf(`"${modName}"()`), | ||
| oldText: `"${modName}"()`, | ||
| newText: `async "${modName}"()` | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Magic fallback blockStart + 10000 can silently miss await in the last/large __esm block.
If the final __esm block in the bundle has no subsequent \nvar init_ marker and its body exceeds 10,000 characters, the blockBody slice will be truncated and any await past that point won't be detected — leaving a non-async callback that will throw at runtime.
Consider using bundleContent.length as the fallback instead:
Proposed fix
- const blockEnd = nextInit !== -1 ? nextInit : blockStart + 10000;
+ const blockEnd = nextInit !== -1 ? nextInit : bundleContent.length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build-mcpb.cjs` around lines 89 - 103, The code uses a magic fallback
blockEnd = blockStart + 10000 when no nextInit is found, which can truncate the
final __esm block and miss await checks; update the logic in the loop that uses
esmBlockRegex and bundleContent so that when nextInit === -1 you use
bundleContent.length (e.g., blockEnd = bundleContent.length) instead of
blockStart + 10000, ensuring blockBody contains the full remaining content
before checking for 'await' and pushing fixes into the fixes array.
Contributor
|
CodeAnt AI finished reviewing your PR. |
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
1 participant
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.
User description
Summary
Replaces the
tsc + copy dist/MCPB build approach with esbuild single-file bundling for faster builds and smaller bundles. Includes post-processing fixes for three esbuild ESM compatibility issues discovered during testing.Changes
scripts/build-mcpb.cjsnpm run build(tsc) with esbuild to produce a singledist/index.jsfilecreateRequire,__dirname,__filenameat bundle top so CJS require() calls work for Node.js built-insasyncto__esmwrapper callbacks that containawaitbut weren't marked async (esbuild bug — async not propagated to modules that transitively depend on async modules)import.meta.url === process.argv[1]in a single-file bundle), which was outputting debug messages to stdout and crashing Claude Desktop's MCP transportsrc/utils/trackTools.tsawait fs.promises.mkdir()with synchronousfs.mkdirSync()to avoid cascading async propagation issues in esbuild's ESM bundlingTesting
npm run build:mcpbRelated to #336
CodeAnt-AI Description
Bundle MCPB with esbuild and fix ESM compatibility to prevent startup crashes
What Changed
Impact
✅ Clearer MCPB startup (no stray debug output on stdout)✅ Fewer MCPB crashes when installed in Claude Desktop✅ Smaller, single-file bundle that builds faster and installs platform ripgrep correctly💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit