-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add custom MCP server installation support #8061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add "Add Custom MCP" button to marketplace MCP tab - Create CustomMcpDialog component for configuring custom servers - Include Serena MCP server as example configuration - Implement backend handler for custom MCP installation - Add comprehensive tests for the new functionality - Fix linting issues Fixes #8059
| const [showSerenaExample, setShowSerenaExample] = useState(false) | ||
|
|
||
| const handleSubmit = async () => { | ||
| // Validate inputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider internationalizing the error messages used in validation (e.g., 'Server name is required' and 'Command is required') by using the translation function t(), to ensure consistency with the rest of the UI.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| onSuccess: () => void | ||
| } | ||
|
|
||
| export function CustomMcpDialog({ onClose, onSuccess }: CustomMcpDialogProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider wrapping this dialog component in an error boundary to gracefully handle unexpected errors. Currently, if an error occurs outside the try-catch in handleSubmit, it could crash the UI.
| await safeWriteJson(mcpSettingsPath, mcpSettings) | ||
|
|
||
| // Refresh MCP connections to load the new server | ||
| await mcpHub.refreshAllConnections() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The success message shows even if refreshAllConnections() fails. Consider wrapping the refresh in a try-catch to handle failures gracefully and provide appropriate user feedback.
| return | ||
| } | ||
| if (!command.trim()) { | ||
| setError("Command is required") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command field accepts any input without validation. Could we validate that it's an executable command or at least show a warning about potential security implications of running arbitrary commands?
| <Button variant="outline" onClick={onClose} disabled={isSubmitting}> | ||
| {t("marketplace:customMcp.cancel")} | ||
| </Button> | ||
| <Button onClick={handleSubmit} disabled={isSubmitting}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a loading spinner or progress indicator here. Currently users only see the button text change to "Adding..." which might not be enough feedback for slower operations.
| placeholder={t("marketplace:customMcp.serverNamePlaceholder")} | ||
| disabled={isSubmitting} | ||
| /> | ||
| {serverName === "" && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "Looking for Serena MCP?" link only appears when serverName is empty. Since Serena was the specific request in the issue, could we make this example more prominent - perhaps always visible at the top of the dialog?
| const trimmedLine = line.trim() | ||
| if (trimmedLine && trimmedLine.includes("=")) { | ||
| const [key, ...valueParts] = trimmedLine.split("=") | ||
| envObj[key.trim()] = valueParts.join("=").trim() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good handling of environment variables with multiple equals signs! Consider adding a tooltip or help text to document this behavior for users (e.g., "API_KEY=value=with=equals works correctly").
This PR addresses Issue #8059 by adding support for custom MCP server installation.
Summary
Implements a feature that allows users to add custom MCP servers through the Roo Code marketplace interface, with specific support for the Serena MCP server requested by the issue reporter.
Changes
UI Components
CustomMcpDialogcomponent for configuring custom MCP serversBackend Implementation
addCustomMcpServermessage handler inwebviewMessageHandler.tssafeWriteJsonutilityTesting
Features
Testing
All tests pass:
src/core/webview/__tests__/webviewMessageHandler.customMcp.spec.tswebview-ui/src/components/marketplace/components/__tests__/CustomMcpDialog.spec.tsxScreenshots
The implementation adds a prominent "Add Custom MCP" button in the MCP tab of the marketplace, which opens a dialog where users can configure their custom MCP servers.
Fixes #8059
Important
Add support for custom MCP server installations with UI and backend changes, including tests for new functionality.
MarketplaceListView.tsx.CustomMcpDialogcomponent for configuring custom MCP servers.CustomMcpDialog.tsx.addCustomMcpServermessage handler inwebviewMessageHandler.ts.safeWriteJsonutility.CustomMcpDialoginCustomMcpDialog.spec.tsx.webviewMessageHandler.customMcp.spec.ts.This description was created by
for 3894345. You can customize this summary. It will automatically update as commits are pushed.