Files
qiming/qimingclaw/docs/review-mcp-bridge-and-proxy.md

3.6 KiB
Raw Blame History

Code Review: MCP Bridge + qiming-mcp-stdio-proxy 统一方案

结论

整体设计清晰,逻辑正确。 临时与持久化 server 统一通过单一 mcp-proxyqiming-mcp-stdio-proxy聚合bridge 入口并入 proxy 后不再需要独立 mcp-bridge-client 脚本,可维护性更好。以下为可改进点与注意事项。


做得好的地方

  1. 单一入口getAgentMcpConfig() 只返回一个 mcp-proxyconfig 内同时包含 stdio 与 bridge 条目Agent 只 spawn 一个进程。
  2. 健康与 URL:持久化 server 仅当 getBridgeUrl(name) 非空(即 healthy时才加入 config避免把不可用 bridge 暴露给引擎。
  3. fallback:无 proxy 脚本时回退到「仅临时 server 的 stdio 配置」,行为明确。
  4. 测试mcp.test.ts 覆盖「仅 persistent」「bridge 运行时的 url」「混合临时+持久化」「无 proxy 时 fallback」等场景。
  5. 持久化 BridgeHTTP body 限制、session 清理、自动重启、stale session 清理等都有考虑。

建议改进

1. syncMcpConfigToProxyAndReload 可能丢失 persistent 标记

位置mcp.tssyncMcpConfigToProxyAndReload

问题:入参类型为 Record<string, { command, args?, env? }>,没有 persistent。内部用 merged = { ...DEFAULT_MCP_PROXY_CONFIG.mcpServers, ...realOnly },若 UI 传入的 realOnly 里含有与默认同名的 serverchrome-devtools)但未带 persistent: true,会覆盖默认项,导致该 server 的 persistent 被抹掉。

建议

  • 若 UI/调用方会传回完整 server 列表(含是否持久化),在类型中增加 persistent?: boolean,并在合并时保留该字段;或
  • 合并时对「已知持久化 server 的 name」做白名单保留其 persistent: true,避免被未带标记的覆盖。

2. createHttpSessionmcpServer.connect(transport) 未 await

位置persistentMcpBridge.ts 第 472474 行。

现状mcpServer.connect(transport).catch(...) 后立即 return { server, transport },调用方可能在 connect 完成前就发请求。

影响:取决于 MCP SDK 的 StreamableHTTPServerTransport 是否在「未 connect 完成」时排队或拒绝请求;若首包在 connect 前到达,理论上存在竞态。

建议:若 SDK 不保证「首请求一定在 connect 之后」,可考虑改为在 handleRequest 前 await 某个「session ready」状态或文档中注明「首请求可能在 connect 完成前到达,由 transport 内部处理」。


3. qiming-mcp-stdio-proxy README 链接

已处理README 中 bridge 示例链接原指向 modelcontextprotocol.io已改为「Electron apps PersistentMcpBridge」描述避免误导。


边界情况确认

场景 行为 评价
仅有 persistent、bridge 未运行 proxyServers 为空 → 返回 null ✓ 合理
仅有 persistent、bridge 运行 仅含 { url } 条目,单 proxy 启动 ✓ 正确
persistentMcpBridge.start() 失败 打 log不抛主流程继续 ✓ 可接受getAgentMcpConfig 仍可能因 isRunning() 为 true 而带 bridge若 HTTP 已起)
打包后 proxy 脚本路径 先 app.getAppPath(),再 app.asar.unpacked ✓ 与 electron-builder 常见布局一致

小结

  • 可合并/发布:当前实现和测试足以支撑现有用法;上述 1、2 为改进项,非阻断。
  • 建议优先:若 UI 会编辑并回写 MCP 配置,建议按 1 处理 persistent,避免用户把 chrome-devtools 改为非持久化后无法恢复。