Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions packages/opencode/src/kilocode/mcp-migrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,21 @@ export namespace McpMigrator {

// Kilocode MCP server structure
export interface KilocodeMcpServer {
command: string
command?: string
args?: string[]
env?: Record<string, string>
disabled?: boolean
alwaysAllow?: string[]
// Remote server fields
type?: string
url?: string
headers?: Record<string, string>
}

const REMOTE_TYPES = new Set(["streamable-http", "sse"])

function isRemote(server: KilocodeMcpServer): boolean {
return !!server.url || REMOTE_TYPES.has(server.type ?? "")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

}

export interface KilocodeMcpSettings {
Expand All @@ -38,10 +48,18 @@ export namespace McpMigrator {
// Skip disabled servers
if (server.disabled) return null

// Build command array: [command, ...args]
const command = [server.command, ...(server.args ?? [])]
// Remote servers (streamable-http, sse, or any config with a url)
if (isRemote(server)) {
const mcpConfig: Config.Mcp = {
type: "remote",
url: server.url!,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

...(server.headers && Object.keys(server.headers).length > 0 && { headers: server.headers }),
}
return mcpConfig
}

// Build the MCP config object
// Local/stdio servers
const command = [server.command!, ...(server.args ?? [])]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

const mcpConfig: Config.Mcp = {
type: "local",
command,
Expand Down
212 changes: 212 additions & 0 deletions packages/opencode/test/kilocode/mcp-migrator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -339,4 +339,216 @@ describe("McpMigrator", () => {
expect(Object.keys(result.mcp)).toHaveLength(0)
})
})

describe("remote server migration", () => {
describe("convertServer", () => {
test("converts streamable-http server", () => {
const server = {
type: "streamable-http",
url: "http://localhost:4321/mcp",
} as any

const result = McpMigrator.convertServer("local-mcp", server)

expect(result).toEqual({
type: "remote",
url: "http://localhost:4321/mcp",
})
})

test("converts sse server", () => {
const server = {
type: "sse",
url: "https://mcp.example.com/sse",
} as any

const result = McpMigrator.convertServer("sse-server", server)

expect(result).toEqual({
type: "remote",
url: "https://mcp.example.com/sse",
})
})

test("converts remote server with headers", () => {
const server = {
type: "streamable-http",
url: "https://mcp.example.com/mcp",
headers: {
Authorization: "Bearer token123",
"X-Custom": "value",
},
} as any

const result = McpMigrator.convertServer("auth-server", server)

expect(result).toEqual({
type: "remote",
url: "https://mcp.example.com/mcp",
headers: {
Authorization: "Bearer token123",
"X-Custom": "value",
},
})
})

test("returns null for disabled remote server", () => {
const server = {
type: "streamable-http",
url: "http://localhost:4321/mcp",
disabled: true,
} as any

const result = McpMigrator.convertServer("disabled-remote", server)

expect(result).toBeNull()
})

test("converts remote server without explicit type but with url", () => {
const server = {
url: "http://localhost:4321/mcp",
} as any

const result = McpMigrator.convertServer("url-only", server)

expect(result).toEqual({
type: "remote",
url: "http://localhost:4321/mcp",
})
})

test("omits headers when not provided on remote server", () => {
const server = {
type: "streamable-http",
url: "http://localhost:4321/mcp",
} as any

const result = McpMigrator.convertServer("no-headers", server)

expect(result).not.toHaveProperty("headers")
})
})

describe("migrate", () => {
test("migrates streamable-http server from project settings", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const settingsDir = path.join(dir, ".kilocode")
await Bun.write(
path.join(settingsDir, "mcp.json"),
JSON.stringify({
mcpServers: {
"local-mcp": {
type: "streamable-http",
url: "http://localhost:4321/mcp",
},
},
}),
)
},
})

const result = await McpMigrator.migrate({
projectDir: tmp.path,
skipGlobalPaths: true,
})

expect(result.mcp).toHaveProperty("local-mcp")
expect(result.mcp["local-mcp"]).toEqual({
type: "remote",
url: "http://localhost:4321/mcp",
})
})

test("migrates mixed stdio and remote servers", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const settingsDir = path.join(dir, ".kilocode")
await Bun.write(
path.join(settingsDir, "mcp.json"),
JSON.stringify({
mcpServers: {
filesystem: {
command: "npx",
args: ["-y", "@modelcontextprotocol/server-filesystem"],
},
"remote-api": {
type: "streamable-http",
url: "https://api.example.com/mcp",
headers: { Authorization: "Bearer secret" },
},
"sse-server": {
type: "sse",
url: "https://sse.example.com/mcp",
},
},
}),
)
},
})

const result = await McpMigrator.migrate({
projectDir: tmp.path,
skipGlobalPaths: true,
})

expect(Object.keys(result.mcp)).toHaveLength(3)

// stdio server should be local
expect(result.mcp.filesystem).toEqual({
type: "local",
command: ["npx", "-y", "@modelcontextprotocol/server-filesystem"],
})

// streamable-http server should be remote
expect(result.mcp["remote-api"]).toEqual({
type: "remote",
url: "https://api.example.com/mcp",
headers: { Authorization: "Bearer secret" },
})

// sse server should be remote
expect(result.mcp["sse-server"]).toEqual({
type: "remote",
url: "https://sse.example.com/mcp",
})
})

test("skips disabled remote server and records it", async () => {
await using tmp = await tmpdir({
init: async (dir) => {
const settingsDir = path.join(dir, ".kilocode")
await Bun.write(
path.join(settingsDir, "mcp.json"),
JSON.stringify({
mcpServers: {
"active-remote": {
type: "streamable-http",
url: "http://localhost:4321/mcp",
},
"disabled-remote": {
type: "streamable-http",
url: "http://localhost:9999/mcp",
disabled: true,
},
},
}),
)
},
})

const result = await McpMigrator.migrate({
projectDir: tmp.path,
skipGlobalPaths: true,
})

expect(result.mcp).toHaveProperty("active-remote")
expect(result.mcp).not.toHaveProperty("disabled-remote")
expect(result.skipped).toContainEqual({
name: "disabled-remote",
reason: "Server is disabled",
})
})
})
})
})
Loading