feat: reimplement MCP connection for Electron#945
feat: reimplement MCP connection for Electron#945lockrush-dev wants to merge 23 commits intomoeru-ai:mainfrom
Conversation
- Implement MCP service in Electron main process using @modelcontextprotocol/sdk - Add Eventa-based IPC communication for MCP commands - Create MCP settings UI component and page - Update tauri-plugin-mcp package to support Electron runtime - Add i18n translations for MCP server settings - Integrate MCP module into settings modules list Closes moeru-ai#783
Summary of ChangesHello @lockrush-dev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request reintroduces the Model Context Protocol (MCP) connection capability into the Electron version of the application, a feature that was previously available in the Tauri build. The changes enable the application to seamlessly communicate with external MCP servers, providing users with a robust interface to configure and manage these connections. This ensures that the application maintains its functionality for interacting with AI models and tools, regardless of the underlying desktop framework. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully re-implements the MCP connection feature for the Electron environment, which is a great step forward after the migration from Tauri. The code is well-structured, introducing a unified plugin that supports both runtimes through platform detection. My review includes a few suggestions to enhance robustness, particularly around resource cleanup, and to improve code clarity and maintainability by refactoring some repetitive logic and simplifying certain implementations. One comment has been updated to reference a guideline about using programmatic identifiers for extensibility. Overall, this is a solid contribution.
✅ Deploy to Cloudflare Workers (Preview) for stage-web ready!
|
- Fix resource leak in disconnectServer using Promise.allSettled and finally block Prevents zombie processes if client.close() fails before transport.close() - Simplify process.env copying using Object.fromEntries and Object.entries Replaces manual for...in loop with modern JavaScript approach - Replace JSON.stringify with discriminated union for content types Improves type safety and client-side handling of MCP tool results Adds support for text, image, and other content types - Make isElectron and isTauri synchronous Removes unnecessary async/await overhead for platform detection - Refactor platform detection to eliminate code duplication (DRY) Centralizes platform-specific logic in getMcpApi factory function - Simplify tauri-plugin-mcp to only serve Tauri Removes Electron support as package should be Tauri-only - Support multiple concurrent MCP server connections Refactors from single state to Map-based connection management Adds serverId to all API calls for connection isolation Improves cleanup on app quit to disconnect all servers
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
The pull request successfully reimplements the MCP connection for Electron, addressing the missing feature after the migration from Tauri. The changes are well-structured, introducing a new Electron service, IPC event definitions, and a UI component for settings. The use of Promise.allSettled and AggregateError for robust disconnection handling is commendable. The lazy loading of the Tauri invoke module is also a good optimization. However, there are a few areas that could be improved for better maintainability and type safety, particularly regarding consistent ID generation and explicit type handling for external data.
- Import process.env from node:process to fix linting error - Add explicit type annotation for tool parameter in listTools - Add @ts-expect-error comments for MCP SDK imports (type resolution) - Fix import order to comply with linting rules Fixes TypeScript compilation errors in the MCP service implementation.
- Fix electron.ts connectServer return type to match main process (returns string/serverId) - Replace Math.random() with nanoid() for generateServerId to prevent collisions - Add runtime validation for inputSchema and content parts instead of unsafe type assertions - Update electron.ts API to support serverId parameters for multi-connection support - disconnectServer(serverId): Promise<void> - listTools(serverId): Promise<Tool[]> - callTool(serverId, name, args): Promise<CallToolResult> - Fix TypeScript errors: - Import process.env from node:process - Add explicit type annotations - Remove unused @ts-expect-error directives Addresses review comments from PR moeru-ai#945.
…om tauri-plugin-mcp - Refactor Electron MCP service into two-layer architecture: - McpServerSession: handles operations on a single MCP server (listTools, callTool, close) - McpManager: manages multiple MCP server connections (connectServer, disconnectServer) - Remove Electron support from tauri-plugin-mcp package: - Delete packages/tauri-plugin-mcp/src/electron.ts - Remove @moeru/eventa dependency from tauri-plugin-mcp - Keep tauri-plugin-mcp Tauri-only as intended - Eventa definitions remain in apps/stage-tamagotchi/src/shared/electron/mcp.ts Addresses PR review comments from @nekomeowww.
…plugin-mcp Remove @moeru/eventa dependency entry from lockfile to match package.json changes.
|
Conflict. |
3 participants
Description
This PR reimplements the MCP (Model Context Protocol) connection feature that was missing after migrating from Tauri to Electron. The implementation supports multiple concurrent MCP server connections and follows a clean two-layer architecture pattern.
Changes
Architecture
McpManager(manages connections) andMcpServerSession(handles individual server operations)McpManager: Manages lifecycle of all MCP server connections (connect, disconnect, session access)McpServerSession: Handles operations on a specific MCP server (list tools, call tools, cleanup)Core Implementation
apps/stage-tamagotchi/src/main/services/electron/mcp.tswith:Map<string, McpServerSession>nanoid()instead ofMath.random()for better collision resistanceinputSchemaand content parts instead of unsafe type assertionsPromise.allSettledin cleanup operationsObject.fromEntriesType Safety & IPC
apps/stage-tamagotchi/src/shared/electron/mcp.tsfor type-safe IPC communicationMcpContentPartdiscriminated union type for better content type handling (text, image, and catch-all)Package Updates
@moeru/eventadependencypackages/tauri-plugin-mcp/src/electron.tspnpm-lock.yamlto reflect dependency removalUI Components
packages/stage-ui/src/components/modules/Mcp.vue) and settings pageCode Quality
process.envfromnode:processTechnical Details
StdioClientTransportfrom MCP SDK to spawn and communicate with MCP server processesserverId)Testing
To test:
pnpm -F @proj-airi/stage-tamagotchi app:devnpx) and arguments (e.g.,-y @modelcontextprotocol/server-filesystem)serverId)Linked Issues
Closes #783