Skip to content

fix(builtin-mcp): make image-gen server runnable by external node in packaged app#1584

Merged
IceyLiu merged 4 commits intomainfrom
zynx/fix/builtin-mcp-asar-unpacked
Mar 20, 2026
Merged

fix(builtin-mcp): make image-gen server runnable by external node in packaged app#1584
IceyLiu merged 4 commits intomainfrom
zynx/fix/builtin-mcp-asar-unpacked

Conversation

@piorpua
Copy link
Copy Markdown
Contributor

@piorpua piorpua commented Mar 20, 2026

Summary

  • In packaged builds, the builtin MCP image-gen server was invisible to Claude because the script path pointed inside app.asar and all npm dependencies were externalized — neither accessible to a standalone node process
  • Fix redirect getBuiltinMcpBaseDir() to app.asar.unpacked when app.isPackaged is true, add the script to asarUnpack, and re-bundle it with esbuild so all dependencies are inlined

Changes

src/process/initStorage.ts

  • getBuiltinMcpBaseDir(): replace app.asar with app.asar.unpacked in the returned path when app.isPackaged === true

electron-builder.yml

  • Add out/main/builtin-mcp-image-gen.js to asarUnpack so the file is physically present on disk and accessible to external processes

scripts/build-mcp-servers.js (new)

  • esbuild config (JS API) that produces a fully self-contained CJS bundle with all npm deps inlined; uses JS API instead of CLI flags to avoid shell-quoting issues with --define values containing //

scripts/build-with-builder.js

  • After the electron-vite build step, invoke build-mcp-servers.js to overwrite the externalized bundle with the self-contained one

Root Cause

externalizeDepsPlugin (used by electron-vite) turns every npm package into a require() call. Inside Electron this works because Electron patches require() to support ASAR virtual paths. But the MCP server runs as a child process via node <path> — that process has no ASAR support, so it can't find any of the dependencies (@modelcontextprotocol/sdk, zod, @office-ai/aioncli-core, etc.).

Related

Follow-up fix for #1243 (introduced the builtin image-gen MCP server).

Test Plan

  • Run bun run build — confirm 📦 Bundling builtin MCP servers (self-contained)... step succeeds
  • Confirm app.asar.unpacked/out/main/builtin-mcp-image-gen.js physically exists in the built .app
  • Launch the release build, open a Claude session, run /mcp — confirm Aionui-image-generation status is connected (not failed)
  • Verify image generation still works in the packaged app

fix(builtin-mcp): make image-gen server runnable by external node in …
cb88802 
…packaged app

In packaged builds (app.isPackaged === true), the builtin MCP image-gen
server was not visible to Claude because of two issues:

1. getBuiltinMcpBaseDir() returned a path inside app.asar. External node
   processes have no Electron ASAR virtual FS support, so `node <path>`
   failed silently. Fix: replace 'app.asar' with 'app.asar.unpacked' when
   app.isPackaged is true.

2. electron-vite's externalizeDepsPlugin leaves all npm packages as
   require() calls. These resolve fine inside Electron (ASAR patches
   require()) but cannot be found by a standalone node process launched
   from app.asar.unpacked. Fix: after electron-vite build, re-bundle
   imageGenServer.ts with esbuild (--bundle) so all dependencies are
   inlined into a single self-contained CJS file.

Changes:
- electron-builder.yml: add out/main/builtin-mcp-image-gen.js to
  asarUnpack so the file is physically present on disk
- src/process/initStorage.ts: redirect getBuiltinMcpBaseDir() to
  app.asar.unpacked when packaged
- scripts/build-mcp-servers.js: new esbuild config script that produces
  a fully self-contained bundle (uses JS API to avoid shell-quoting
  issues with define values containing '/')
- scripts/build-with-builder.js: invoke build-mcp-servers.js after the
  electron-vite build step
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 20, 2026
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 20, 2026
View reviewed changes
@piorpua
Copy link
Copy Markdown
Contributor Author

piorpua commented Mar 20, 2026

Code Review：fix(builtin-mcp): make image-gen server runnable by external node in packaged app (#1584)

变更概述

本 PR 修复了打包后的应用中内置 MCP 图片生成服务器无法被外部 node 进程访问的问题。通过三处协同改动：新增独立的 esbuild 打包脚本将 MCP 服务器打成自含 CJS bundle、在 electron-builder 配置中将其加入 asarUnpack、并在 getBuiltinMcpBaseDir() 中将路径从 app.asar 重定向到 app.asar.unpacked，解决了 electron-vite 的 externalizeDepsPlugin 导致依赖 require 调用在外部 node 进程中失败的根本原因。

方案评估

结论：✅ 方案合理

根本原因分析准确：electron-vite 的 externalize 机制依赖 Electron 对 require() 的 ASAR patch，而外部 node 进程不具备此能力。通过 esbuild 重新打包内联所有依赖、配合 asarUnpack 和路径重定向，是处理此类问题的标准做法。使用 esbuild JS API 而非 CLI flags 来规避 --define 中特殊字符的 shell 转义问题，这个设计决策有价值且在代码注释中有清晰说明。

问题清单

🟡 MEDIUM — execSync 中路径未加引号，项目路径含空格时构建将失败

文件：scripts/build-with-builder.js，第 423 行

问题代码：

execSync(`node ${path.join(__dirname, 'build-mcp-servers.js')}`, {
  stdio: 'inherit',
  shell: process.platform === 'win32',
});

问题说明：execSync 始终通过 shell 执行命令（Unix 用 /bin/sh，Windows 用 cmd.exe）。若项目目录路径中含有空格（例如 ~/My Projects/AionUi），shell 会将空格后的部分解析为独立参数，导致 Error: Cannot find module '.../build-mcp-servers.js'，构建中断。同文件中其他 execSync 调用（如 bunx electron-vite build）不含动态路径，无此风险。

修复建议：

execSync(`node "${path.join(__dirname, 'build-mcp-servers.js')}"`, {
  stdio: 'inherit',
  shell: process.platform === 'win32',
});

🔵 LOW — import.meta.url 占位符在 Windows 上可能导致运行时异常

文件：scripts/build-mcp-servers.js，第 32 行

问题代码：

'import.meta.url': JSON.stringify('file:///placeholder'),

问题说明：file:///placeholder 是一个不含驱动器字母的 URL。在 Windows 上，如果 @office-ai/aioncli-core 内部调用了 fileURLToPath(import.meta.url)（Node.js 要求 Windows file URL 必须包含驱动器字母，如 file:///C:/...），会抛出 TypeError: File URL host must be "localhost" or empty on windows，导致 MCP 服务器启动失败。注释说明此值仅用于版本检测，若只做字符串操作则无影响；但若存在路径转换调用，则 Windows 场景有隐患。

修复建议：可使用含驱动器字母的 Windows 兼容占位符：

'import.meta.url': JSON.stringify('file:///C:/placeholder'),

或在 CI 的 Windows 构建中补充验证 MCP 服务器能否成功启动。

汇总

# 严重级别 文件 问题
1 🟡 MEDIUM scripts/build-with-builder.js:423 execSync 路径未加引号，空格路径会失败
2 🔵 LOW scripts/build-mcp-servers.js:32 file:///placeholder 在 Windows 上可能引发 fileURLToPath 异常

结论

⚠️ 有条件批准 — 核心修复方案正确，存在一个在含空格路径的 Windows 构建环境中会实际触发的 MEDIUM 问题，建议修复后合并。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

fix(builtin-mcp): address review issues from PR #1584
c41504e 
- Quote dynamic path in execSync to handle project paths with spaces
- Use Windows-compatible file:///C:/placeholder for import.meta.url define

Review follow-up for #1584
@piorpua
Copy link
Copy Markdown
Contributor Author

piorpua commented Mar 20, 2026

PR Fix 验证报告

原始 PR: #1584
修复方式: 直接推送到 zynx/fix/builtin-mcp-asar-unpacked

# 严重级别 文件 问题 修复方式 状态
1 🟡 MEDIUM scripts/build-with-builder.js:423 execSync 路径未加引号，含空格路径时构建失败 将路径包裹在双引号中：`node "${path.join(...)}"` ✅ 已修复
2 🔵 LOW scripts/build-mcp-servers.js:32 file:///placeholder 在 Windows 上 fileURLToPath 可能抛出异常 改为 file:///C:/placeholder，包含驱动器字母 ✅ 已修复

总结： ✅ 已修复 2 个 | ❌ 未能修复 0 个 | ⏭️ 跳过 0 个

@IceyLiu IceyLiu merged commit 28990fa into main Mar 20, 2026
17 checks passed
@piorpua piorpua deleted the zynx/fix/builtin-mcp-asar-unpacked branch March 20, 2026 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IceyLiu IceyLiu IceyLiu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@piorpua @IceyLiu @github-advanced-security