fix(electron): include @ngrok/ngrok in packaged app so tunnel feature works#2037
fix(electron): include @ngrok/ngrok in packaged app so tunnel feature works#2037bsradcliffe wants to merge 10 commits intoMCPJam:mainfrom
Conversation
… works The VitePlugin only packs .vite/build/ into app.asar — no top-level node_modules. @ngrok/ngrok was marked external in vite.main.config.ts (can't bundle native addons), so it was never reachable at runtime and caused 'Cannot find module' on startup. Three changes to forge.config.ts: 1. afterCopy: copy the @ngrok scope dir into .vite/build/node_modules/ so it lands inside the asar alongside main.cjs. Uses require.resolve() to handle npm workspace hoisting instead of hardcoding the local node_modules path. 2. postPackage hook: rebuild the asar with unpack: "*.node" after packaging. The asar.unpack packagerConfig option only evaluates files present before afterCopy runs, so .node binaries added by afterCopy were never promoted to app.asar.unpacked. Native addons cannot be dlopen'd from inside an asar archive, so this step is required. 3. Fix asar.unpack pattern from "**/*.node" to "*.node" — @electron/asar uses minimatch with matchBase:true against absolute paths, so patterns with "/" don't match.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…ling universal fat binary
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dosubot
Bot
added
the
size:M
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Hidden review stack artifact:WalkthroughThe changes refactor Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
mcpjam-inspector/forge.config.ts (1)
202-204: ⚡ Quick winDocument or guard the ASAR rebuild to prevent future breakage with asarIntegrity.
The
postPackagehook rebuildsapp.asar, which is safe today since noasarIntegrityoption is present inpackagerConfig. However,EnableEmbeddedAsarIntegrityValidationis already enabled (line 243), and addingasarIntegrityto the packager config would cause the rebuilt ASAR to silently fail integrity validation when loaded—the header hash embedded by electron-packager would no longer match.Preempt this by adding a comment explaining the constraint alongside the fuse configuration, or implement a guard in
postPackagethat updates theInfo.plistheader hash if one exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mcpjam-inspector/forge.config.ts` around lines 202 - 204, The postPackage hook currently rebuilds app.asar via asar.createPackageWithOptions in postPackage which will break electron-packager's asarIntegrity validation if packagerConfig.asarIntegrity is later set; either add a clear comment next to the fuse/EnableEmbeddedAsarIntegrityValidation configuration documenting that rebuilding app.asar is incompatible with asarIntegrity, or implement a guard in postPackage: detect if packagerConfig.asarIntegrity (or an embedded asar header hash in Info.plist) exists and skip the rebuild or recompute and update the stored header hash in Info.plist after calling asar.createPackageWithOptions; reference postPackage, asar.createPackageWithOptions, EnableEmbeddedAsarIntegrityValidation, packagerConfig.asarIntegrity and the Info.plist header hash when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcpjam-inspector/forge.config.ts`:
- Around line 111-114: The code currently silently skips copying when src
doesn't exist, leading to a cryptic runtime MODULE_NOT_FOUND; update the block
that checks existsSync(src) around the cpSync(src, join(dest, pkg), { recursive:
true }) call to emit a clear build-time warning (e.g. console.warn) when the
platform package (pkg / src) is missing, including the offending pkg and dest
values so users can see that the `@ngrok/`<pkg> binary was not found instead of
silently proceeding.
- Around line 192-207: postPackage currently renames the original asar
(renameSync(asarPath, `${asarPath}.bak`)) before repackaging and never removes
the extracted tmpDir, so failures leave the app broken and temp files behind;
wrap the sequence around asar.extractAll(tmpDir), renameSync, and
asar.createPackageWithOptions in a try/finally: create tmpDir, extract (remove
the needless await on asar.extractAll if synchronous), then rename the original
to .bak only immediately before createPackageWithOptions, and in finally ensure
tmpDir is removed (use rmSync or similar) and if createPackageWithOptions failed
restore the original by renaming `${asarPath}.bak` back to asarPath; only
unlinkSync(`${asarPath}.bak`) after successful packaging. Ensure you reference
tmpDir, asarPath, asar.extractAll, renameSync, asar.createPackageWithOptions,
unlinkSync and rmSync when making the change.
- Around line 109-115: The loop that copies packages using cpSync (iterating
over pkgsToCopy with src built from ngrokSrc) can throw and currently those
errors are unhandled; wrap the cpSync call (and the surrounding per-package
logic) in a try/catch block and on error call the provided callback with the
error (callback(err)) after logging context (e.g., which pkg/src/dest failed),
so permission/full-disk/symlink errors are surfaced instead of crashing or
hanging.
---
Nitpick comments:
In `@mcpjam-inspector/forge.config.ts`:
- Around line 202-204: The postPackage hook currently rebuilds app.asar via
asar.createPackageWithOptions in postPackage which will break
electron-packager's asarIntegrity validation if packagerConfig.asarIntegrity is
later set; either add a clear comment next to the
fuse/EnableEmbeddedAsarIntegrityValidation configuration documenting that
rebuilding app.asar is incompatible with asarIntegrity, or implement a guard in
postPackage: detect if packagerConfig.asarIntegrity (or an embedded asar header
hash in Info.plist) exists and skip the rebuild or recompute and update the
stored header hash in Info.plist after calling asar.createPackageWithOptions;
reference postPackage, asar.createPackageWithOptions,
EnableEmbeddedAsarIntegrityValidation, packagerConfig.asarIntegrity and the
Info.plist header hash when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b22290fc-0d05-446f-956a-93aa3e3a8232
📒 Files selected for processing (1)
mcpjam-inspector/forge.config.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcpjam-inspector/forge.config.ts`:
- Around line 202-209: Move the temporary directory creation and the asar
extraction into the guarded try/finally so tmpDir is always cleaned up on
failure: create tmpDir with mkdirSync and call asar.extractAll(asarPath, tmpDir)
inside the try block (not before it) and remove the unnecessary await on
asar.extractAll since it is synchronous; keep the finally block to delete tmpDir
and preserve the existing succeeded flag handling around the extraction and
rebuild steps.
- Around line 185-225: The postPackage hook is rebuilding app.asar after signing
and integrity embedding (breaking EnableEmbeddedAsarIntegrityValidation /
OnlyLoadAppFromAsar and code signatures); remove the asar extract/recreate logic
from the postPackage function and either (a) add
`@electron-forge/plugin-auto-unpack-natives` to the Forge config so unpack:
"*.node" is applied before signing, or (b) move the current asar unpack/repack
steps from the postPackage hook into the afterPack hook so the archive is
rebuilt prior to integrity embedding/signing (update/remove the code inside
postPackage and relocate the
renameSync/asar.extractAll/asar.createPackageWithOptions/unlinkSync/rmSync logic
accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d18b34fb-e400-4a86-a475-6d81e9662deb
📒 Files selected for processing (1)
mcpjam-inspector/forge.config.ts
dosubot
Bot
added
size:L
…ore asarApp The postPackage hook was added to ensure .node files end up in app.asar.unpacked, but it is unnecessary: afterCopy fires inside electron-packager's copyTemplate() step, which runs before asarApp() creates the archive. So packagerConfig.asar.unpack: "*.node" already sees the @ngrok binaries that afterCopy placed in .vite/build/node_modules and marks them as unpacked at asar creation time. Verified empirically: bypassing the postPackage body with `continue` still produced app.asar.unpacked/.vite/build/node_modules/@ngrok/.../*.node. Removes the @electron/asar import and the hooks block entirely.
dosubot
Bot
added
size:M
|
@coderabbitai resume |
|
Tip For best results, initiate chat on the files or code changes.
[resume] |
2 participants
Fixes #2036.
Summary
Fixes
Cannot find module '@ngrok/ngrok'crash on startup of the packaged Mac app.The VitePlugin only packs
.vite/build/intoapp.asar— nonode_modules. Since@ngrok/ngrokis listed as external invite.main.config.ts(native addons can't be bundled by Vite), it was completely absent from the packaged app. The existingafterCopyhook was writing tobuildPath/node_modules/@ngrok, which is outside what VitePlugin packs — effectively a no-op.Changes (
mcpjam-inspector/forge.config.ts)afterCopy: copy the@ngrokscope dir into.vite/build/node_modules/so it lands inside the asar alongsidemain.cjs. Usesrequire.resolve()to find the package accounting for npm workspace hoisting. Copies only the platform-matched native sub-package (e.g.ngrok-darwin-arm64) to avoid bundling redundant binaries for other platforms.asar.unpackglob:"**/*.node"→"*.node".@electron/asarcallsminimatch(absPath, pattern, { matchBase: true })— patterns containing/don't match absolute paths, so only basename-only patterns work.afterCopyruns beforeasarApp()creates the archive, so the.nodefiles placed by the hook are correctly seen and marked as unpacked at creation time.Test plan
electron-forge packageand confirmapp.asar.unpacked/.vite/build/node_modules/@ngrok/ngrok-darwin-arm64/ngrok.darwin-arm64.nodeexists.appand confirm noCannot find module '@ngrok/ngrok'error on startup