feat: support add custom mcp server #1731
Conversation
github-actions
bot
added
documentation
WalkthroughAdds Model Context Protocol (MCP) support to the robot plugin: new MCPHost service manages MCP connections/transports and tool calls, composable changes for per-server lifecycle and tool routing, custom MCP server configuration and UI toggle wiring, a small tool-utils mutation, and an MCP SDK dependency. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as McpServer.vue
participant Composable as useMcp
participant MCPHost as MCPHost
participant MCP as MCP Server
User->>UI: toggle server (enable/disable)
UI->>Composable: updateMcpServerToggle(server, enabled)
alt enable server
Composable->>Composable: updateCustomMcpServers()
Composable->>MCPHost: connectToServer(serverConfig)
MCPHost->>MCP: establish transport (SSE / StreamableHTTP)
MCP-->>MCPHost: connection + tools list
MCPHost-->>Composable: { id, tools }
Composable->>Composable: update toolsMap and server addState/tools
Composable-->>UI: updated state
else disable server
Composable->>MCPHost: disconnect(serverId)
MCPHost->>MCP: close connection
MCPHost-->>Composable: disconnected
Composable->>Composable: remove tools from toolsMap / update addState
Composable-->>UI: updated state
end
sequenceDiagram
actor LLM as AI Plugin
participant Composable as useMcp
participant Tools as toolsMap
participant MCPHost as MCPHost
participant MCP as MCP Server
LLM->>Composable: callTool(toolId, args)
Composable->>Tools: resolve toolId -> { serverId, toolName }
Composable->>MCPHost: callTool({ serverId, toolName, toolArgs: args })
MCPHost->>MCP: invoke tool
MCP-->>MCPHost: tool result
MCPHost-->>Composable: result
Composable-->>LLM: return tool output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
packages/plugins/robot/src/composables/features/useMcp.ts (2)
9-9: Module-level singleton may cause issues in testing or SSR.The
MCPHostinstance is created at module load time, which makes it difficult to mock in tests and could cause issues in SSR environments. Consider using a factory function or dependency injection pattern if these scenarios are relevant.
211-226: ConsiderPromise.allSettledfor resilience ingetLLMTools.Using
Promise.allmeans if one MCP server fails to list tools, the entire operation fails. UsingPromise.allSettledwould allow successful servers to return their tools while logging failures for problematic servers.🔎 Proposed approach
const getLLMTools = async () => { const servers = inUseMcpServers.value.filter((server) => server.enabled && server.tools.length > 0) - const tools = await Promise.all( + const results = await Promise.allSettled( servers.map(async (server) => { const enabledTools = server.tools?.filter((tool) => tool.enabled).map((tool) => tool.id || tool.name) || [] const client = mcpHost.getClient(server.id) if (client) { const listToolResult: { tools: McpTool[] } = await client.listTools() return listToolResult.tools.filter((tool) => enabledTools.includes(tool.name)) } return [] }) ) + const tools = results + .filter((r): r is PromiseFulfilledResult<McpTool[]> => r.status === 'fulfilled') + .map((r) => r.value) return convertMCPToOpenAITools(tools.flat()) }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/advanced-features/new-ai-plugin-usage.mdpackages/common/composable/mcp/toolUtils.tspackages/plugins/robot/meta.jspackages/plugins/robot/package.jsonpackages/plugins/robot/src/components/footer-extension/McpServer.vuepackages/plugins/robot/src/composables/features/useMcp.tspackages/plugins/robot/src/services/MCPHost.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-30T07:51:10.036Z
Learnt from: chilingling
Repo: opentiny/tiny-engine PR: 837
File: packages/vue-generator/src/plugins/genDependenciesPlugin.js:66-66
Timestamp: 2024-09-30T07:51:10.036Z
Learning: In the `tiny-engine` project, `opentiny/tiny-engine-dsl-vue` refers to the current package itself, and importing types from it may cause circular dependencies.
Applied to files:
packages/plugins/robot/package.json
📚 Learning: 2024-12-14T05:53:28.501Z
Learnt from: gene9831
Repo: opentiny/tiny-engine PR: 917
File: docs/开始/快速上手.md:31-31
Timestamp: 2024-12-14T05:53:28.501Z
Learning: The latest stable version of `opentiny/tiny-engine-cli` is `2.0.0`, and documentation should reference this version instead of any release candidates.
Applied to files:
packages/plugins/robot/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (7)
docs/advanced-features/new-ai-plugin-usage.md (1)
251-266: Documentation looks good.The new MCP server configuration section clearly documents the
mcpConfig.mcpServersstructure with a practical example. The supported transport types (SSE and StreamableHttp) and required properties (type, url) are well-documented.packages/plugins/robot/src/services/MCPHost.ts (2)
109-121: Clarify whyclient.disconnect()is commented out.The commented-out
client.disconnect()at line 114 may indicate incomplete cleanup. If the MCP SDK doesn't require explicit client disconnection when closing the transport, consider adding a comment explaining this. Otherwise, uncommenting it may be necessary to properly release resources.
123-134: Cleanup and utility methods look good.The
cleanup()method properly iterates through all transports, closes them, and clears both maps. ThehasActiveSessions()helper is a clean utility.packages/plugins/robot/meta.js (1)
18-28: Configuration structure looks good.The new
mcpConfig.mcpServersconfiguration surface is well-structured with a helpful commented example showing the expected format. This aligns with the MCPHost implementation and documentation.packages/plugins/robot/src/components/footer-extension/McpServer.vue (1)
50-61: Good refactor to delegate toggle handling.The change from direct mutation (
plugin.enabled = enabled) to usingupdateMcpServerToggle(plugin, enabled)properly centralizes the async server connection/disconnection logic in the composable. This improves maintainability and encapsulation.packages/plugins/robot/src/composables/features/useMcp.ts (2)
72-93: Engine server toggle doesn't update tool states.For
ENGINE_MCP_SERVER, the toggle only setsserver.enabledwithout callingupdateEngineServer()to sync tool states. This differs fromupdateMcpServerStatuswhich does callupdateEngineServer. Is this inconsistency intentional?
202-241: Computed properties and exports look good.The
toolscomputed property correctly filters and converts enabled tools from enabled servers. TheisToolsEnabledcomputed provides a clean boolean for UI state. The exported API surface is comprehensive for the MCP server management use case.
hexqi
force-pushed
the
feat/custom-mcp
branch
from
acb288d to
fe7bc52
Compare
packages/plugins/robot/src/components/footer-extension/McpServer.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/plugins/robot/src/composables/features/useMcp.ts`:
- Around line 217-232: In getLLMTools, enabledTools is built from PluginTool as
tool.id || tool.name but the filter against McpTool currently checks only
tool.name, which drops tools whose id differs from name; update the filter
inside the servers.map (where listToolResult.tools is filtered) to check
enabledTools.includes(tool.id || tool.name) so it matches by id first then name
(i.e., use tool.id || tool.name when comparing), keeping the rest of the logic
(mcpHost.getClient, Promise.all, convertMCPToOpenAITools) unchanged.
🧹 Nitpick comments (1)
packages/plugins/robot/src/composables/features/useMcp.ts (1)
81-102: Log failures when toggling custom MCP servers.
Silent failures make diagnosing connectivity/config issues harder; a brief error log would help.📝 Suggested tweak
} catch (error) { inuseServer.enabled = false + console.error('MCP server toggle failed', { serverId: inuseServer.id, enabled, error }) } }
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.