feat: support remote MCP server migration (streamable-http, sse)#245
feat: support remote MCP server migration (streamable-http, sse)#245kiloconnect[bot] wants to merge 1 commit intodevfrom
Conversation
Add support for migrating remote MCP server configurations (streamable-http, sse) from .kilocode/mcp_settings.json and .kilocode/mcp.json. Previously, the migrator only handled stdio/local servers and would fail with 'The "file" argument must be of type string. Received undefined' when encountering remote servers that have a url field instead of command/args. Changes: - Extended KilocodeMcpServer interface with optional type, url, and headers fields - Added isRemote() helper to detect remote server configs - Updated convertServer() to produce Config.McpRemote for remote servers - Made command field optional since remote servers don't have one - Added comprehensive tests for streamable-http, sse, mixed configs, and headers
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const REMOTE_TYPES = new Set(["streamable-http", "sse"]) | ||
|
|
||
| function isRemote(server: KilocodeMcpServer): boolean { | ||
| return !!server.url || REMOTE_TYPES.has(server.type ?? "") |
There was a problem hiding this comment.
CRITICAL: isRemote() can classify a server as remote even when url is missing
If server.type is "streamable-http"/"sse" but url is absent, convertServer() will still take the remote branch and emit { type: 'remote', url: undefined } (via non-null assertions), which can break config validation downstream. Consider requiring url when type indicates remote (or treat missing url as skipped + warning).
| if (isRemote(server)) { | ||
| const mcpConfig: Config.Mcp = { | ||
| type: "remote", | ||
| url: server.url!, |
There was a problem hiding this comment.
CRITICAL: Non-null assertion on server.url can produce invalid remote config
url: server.url! will silently become url: undefined at runtime if the input config has type set but no url. A defensive guard (return null and record a warning/skipped reason) would avoid emitting an invalid Config.McpRemote.
|
|
||
| // Build the MCP config object | ||
| // Local/stdio servers | ||
| const command = [server.command!, ...(server.args ?? [])] |
There was a problem hiding this comment.
WARNING: Non-null assertion on server.command can create [undefined, ...] for local servers
Since command is now optional on KilocodeMcpServer, a malformed/partial local config could reach this branch and produce an invalid command array. Consider validating server.command before constructing the local config (and skipping + warning if missing).
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (2 files)
|
Summary
Adds support for migrating remote MCP server configurations (streamable-http, sse) from
.kilocode/mcp_settings.jsonand.kilocode/mcp.json.Problem
When a user configures a remote streamable-http MCP server in
.kilocode/mcp_settings.json:{ "mcpServers": { "local-mcp": { "type": "streamable-http", "url": "http://localhost:4321/mcp" } } }The migrator fails with:
The "file" argument must be of type string. Received undefinedThis happens because
convertServer()unconditionally treats every server as a local/stdio server, accessingserver.command(which isundefinedfor remote servers) and passing it to the command array.Changes
packages/opencode/src/kilocode/mcp-migrator.tsKilocodeMcpServerinterface with optionaltype,url, andheadersfields for remote serverscommandoptional since remote servers don't have oneisRemote()helper that detects remote servers by checking for aurlfield or a remotetype(streamable-http,sse)convertServer()to produceConfig.McpRemote(type: "remote") for remote servers, passing throughurlandheaderspackages/opencode/test/kilocode/mcp-migrator.test.tsurlbut no explicittypeBackward Compatibility
All 16 existing tests continue to pass. Stdio/local server migration is unchanged.
Built for Marius Wichtner by Kilo for Slack