From c5c02b551482e04b2d7089844a692142ec263691 Mon Sep 17 00:00:00 2001 From: Dominic Cooney Date: Fri, 22 May 2026 13:42:20 +0900 Subject: [PATCH] fix(mcp): accept CLI-authored nested transport format, preserve oauth/metadata, improve schema error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Cline CLI (cline mcp add) writes servers in a nested transport format: { transport: { type, url }, disabled, oauth } The VSCode extension only accepted the flat format it writes: { type, url, disabled, autoApprove } This caused all MCP servers to silently disappear with a generic 'Invalid MCP settings schema.' error that told users nothing useful. Changes: - schemas.ts: Add nestedTransportConfigSchema as the first union arm in ServerConfigSchema, placed first so the 'transport:' key acts as an unambiguous discriminator. The transform flattens nested -> flat format with zero downstream impact (connection logic unchanged). - schemas.ts: Add oauth and metadata passthrough fields to BaseConfigSchema so CLI-written OAuth state and metadata survive round-trips when the extension modifies the file (e.g. toggling disabled). - McpHub.ts: Dramatically improve error messages — include file path, per-server breakdown of which fields failed (from Zod error paths), and an 'Open Settings File' button for one-click navigation. - schemas.test.ts: 14 new tests covering nested format, flat format, mixed files, oauth/metadata preservation, and error rejection. --- apps/vscode/src/services/mcp/McpHub.ts | 56 +++- .../services/mcp/__tests__/schemas.test.ts | 275 ++++++++++++++++++ apps/vscode/src/services/mcp/schemas.ts | 63 ++++ apps/vscode/webview-ui/package-lock.json | 32 +- apps/vscode/webview-ui/package.json | 3 +- 5 files changed, 414 insertions(+), 15 deletions(-) create mode 100644 apps/vscode/src/services/mcp/__tests__/schemas.test.ts diff --git a/apps/vscode/src/services/mcp/McpHub.ts b/apps/vscode/src/services/mcp/McpHub.ts index 9f231ad41a..ac58e1a4ef 100644 --- a/apps/vscode/src/services/mcp/McpHub.ts +++ b/apps/vscode/src/services/mcp/McpHub.ts @@ -194,10 +194,20 @@ export class McpHub { try { config = JSON.parse(content) } catch (_error) { - HostProvider.window.showMessage({ - type: ShowMessageType.ERROR, - message: "Invalid MCP settings format. Please ensure your settings follow the correct JSON format.", - }) + HostProvider.window + .showMessage({ + type: ShowMessageType.ERROR, + message: `Invalid JSON in MCP settings file. Please check the syntax.`, + options: { + detail: settingsPath, + items: ["Open Settings File"], + }, + }) + .then((response) => { + if (response.selectedOption === "Open Settings File") { + HostProvider.window.showTextDocument({ path: settingsPath, options: {} }) + } + }) return undefined } @@ -208,10 +218,40 @@ export class McpHub { // Validate against schema const result = McpSettingsSchema.safeParse(config) if (!result.success) { - HostProvider.window.showMessage({ - type: ShowMessageType.ERROR, - message: "Invalid MCP settings schema.", - }) + // Build a human-readable summary of what failed. + // Zod paths look like ["mcpServers", "linear", "transport", "url"] — we want to surface + // the server name (index 1) and the field path so users know exactly what to fix. + const issuesByServer = new Map() + for (const issue of result.error.issues) { + // path[0] === "mcpServers", path[1] === serverName + const serverName = issue.path.length >= 2 ? String(issue.path[1]) : "(unknown server)" + const fieldPath = issue.path.slice(2).join(".") // e.g. "transport.url" or "command" + const detail = fieldPath ? `${fieldPath}: ${issue.message}` : issue.message + if (!issuesByServer.has(serverName)) { + issuesByServer.set(serverName, []) + } + issuesByServer.get(serverName)!.push(detail) + } + + const serverSummaries = Array.from(issuesByServer.entries()) + .map(([server, details]) => ` • ${server}: ${details.join(", ")}`) + .join("\n") + + HostProvider.window + .showMessage({ + type: ShowMessageType.ERROR, + message: `MCP settings schema error — no servers were loaded.`, + options: { + detail: `${settingsPath}\n\n${serverSummaries}`, + modal: false, + items: ["Open Settings File"], + }, + }) + .then((response) => { + if (response.selectedOption === "Open Settings File") { + HostProvider.window.showTextDocument({ path: settingsPath, options: {} }) + } + }) return undefined } diff --git a/apps/vscode/src/services/mcp/__tests__/schemas.test.ts b/apps/vscode/src/services/mcp/__tests__/schemas.test.ts new file mode 100644 index 0000000000..65b1f2d219 --- /dev/null +++ b/apps/vscode/src/services/mcp/__tests__/schemas.test.ts @@ -0,0 +1,275 @@ +import { describe, it } from "mocha" +import "should" +import { McpSettingsSchema, ServerConfigSchema } from "../schemas" + +/** + * Unit tests for MCP settings schema parsing. + * + * Covers three formats: + * 1. "Nested transport" format written by the Cline CLI (`cline mcp add`) + * 2. "Flat" legacy format accepted by the VSCode extension before this change + * 3. Invalid configs that must still be rejected + */ +describe("McpSettingsSchema", () => { + // ------------------------------------------------------------------------- + // Nested transport format (written by the Cline CLI) + // ------------------------------------------------------------------------- + + describe("nested transport format (CLI-authored)", () => { + it("accepts a streamableHttp server written by the CLI", () => { + const input = { + mcpServers: { + linear: { + transport: { + type: "streamableHttp", + url: "https://mcp.linear.app/mcp", + }, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + const server = result.data!.mcpServers["linear"] + server.type.should.equal("streamableHttp") + ;(server as any).url.should.equal("https://mcp.linear.app/mcp") + }) + + it("accepts a SSE server with headers in nested format", () => { + const input = { + mcpServers: { + myServer: { + transport: { + type: "sse", + url: "https://mcp.example.com/sse", + headers: { Authorization: "Bearer tok" }, + }, + disabled: true, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + const server = result.data!.mcpServers["myServer"] + server.type.should.equal("sse") + ;(server as any).url.should.equal("https://mcp.example.com/sse") + ;(server as any).headers.should.deepEqual({ Authorization: "Bearer tok" }) + server.disabled!.should.be.true() + }) + + it("accepts a stdio server in nested format", () => { + const input = { + mcpServers: { + docs: { + transport: { + type: "stdio", + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem"], + }, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + const server = result.data!.mcpServers["docs"] + server.type.should.equal("stdio") + ;(server as any).command.should.equal("npx") + }) + + it("preserves oauth field from CLI-authored nested format", () => { + const oauthState = { + tokens: { access_token: "tok", token_type: "Bearer" }, + lastAuthenticatedAt: 1700000000, + } + const input = { + mcpServers: { + linear: { + transport: { type: "streamableHttp", url: "https://mcp.linear.app/mcp" }, + oauth: oauthState, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + ;(result.data!.mcpServers["linear"] as any).oauth.should.deepEqual(oauthState) + }) + + it("preserves metadata field from CLI-authored nested format", () => { + const metadata = { addedBy: "cline-cli", version: "1.2.3" } + const input = { + mcpServers: { + myServer: { + transport: { type: "streamableHttp", url: "https://mcp.example.com/mcp" }, + metadata, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + ;(result.data!.mcpServers["myServer"] as any).metadata.should.deepEqual(metadata) + }) + + it("preserves autoApprove and timeout alongside nested transport", () => { + const input = { + mcpServers: { + myServer: { + transport: { type: "streamableHttp", url: "https://mcp.example.com/mcp" }, + autoApprove: ["my_tool"], + timeout: 120, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + const server = result.data!.mcpServers["myServer"] + server.autoApprove!.should.deepEqual(["my_tool"]) + server.timeout.should.equal(120) + }) + + it("rejects nested format with an invalid transport type", () => { + const input = { + mcpServers: { + bad: { + transport: { type: "unknownTransport", url: "https://mcp.example.com/mcp" }, + }, + }, + } + McpSettingsSchema.safeParse(input).success.should.be.false() + }) + + it("rejects nested stdio transport with empty command", () => { + const input = { + mcpServers: { + bad: { + transport: { type: "stdio", command: "" }, // empty — min(1) fails + }, + }, + } + McpSettingsSchema.safeParse(input).success.should.be.false() + }) + }) + + // ------------------------------------------------------------------------- + // Flat legacy format (written by the VSCode extension) + // ------------------------------------------------------------------------- + + describe("flat legacy format (extension-authored)", () => { + it("accepts a flat streamableHttp server", () => { + const input = { + mcpServers: { + myServer: { + type: "streamableHttp", + url: "https://mcp.example.com/mcp", + autoApprove: [], + timeout: 60, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + const server = result.data!.mcpServers["myServer"] + server.type.should.equal("streamableHttp") + ;(server as any).url.should.equal("https://mcp.example.com/mcp") + }) + + it("accepts a flat stdio server with no explicit type", () => { + const input = { + mcpServers: { + docs: { + command: "npx", + args: ["-y", "@modelcontextprotocol/server-filesystem"], + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + result.data!.mcpServers["docs"].type.should.equal("stdio") + }) + + it("accepts a flat SSE server with no explicit type (legacy default)", () => { + const input = { + mcpServers: { legacy: { url: "https://mcp.example.com/sse" } }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + // Without a type field, the flat schema defaults to "sse" + result.data!.mcpServers["legacy"].type.should.equal("sse") + }) + + it("preserves oauth on flat-format servers (round-trip from write-back)", () => { + const oauthState = { tokens: { access_token: "tok" } } + const input = { + mcpServers: { + myServer: { + type: "streamableHttp", + url: "https://mcp.example.com/mcp", + oauth: oauthState, + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + ;(result.data!.mcpServers["myServer"] as any).oauth.should.deepEqual(oauthState) + }) + }) + + // ------------------------------------------------------------------------- + // Mixed format — CLI and extension servers in the same file + // ------------------------------------------------------------------------- + + describe("mixed format (CLI and extension servers in the same file)", () => { + it("accepts a file with both nested and flat servers", () => { + const input = { + mcpServers: { + cliServer: { + transport: { type: "streamableHttp", url: "https://mcp.linear.app/mcp" }, + oauth: { tokens: {} }, + }, + extensionServer: { + command: "node", + args: ["server.js"], + autoApprove: ["tool1"], + }, + }, + } + + const result = McpSettingsSchema.safeParse(input) + result.success.should.be.true() + + result.data!.mcpServers["cliServer"].type.should.equal("streamableHttp") + result.data!.mcpServers["extensionServer"].type.should.equal("stdio") + }) + }) + + // ------------------------------------------------------------------------- + // ServerConfigSchema – direct usage + // ------------------------------------------------------------------------- + + describe("ServerConfigSchema direct parse", () => { + it("normalises nested format to flat (no transport key in output)", () => { + const result = ServerConfigSchema.safeParse({ + transport: { type: "streamableHttp", url: "https://mcp.example.com" }, + }) + result.success.should.be.true() + const flat = result.data! + ;(flat as any).type.should.equal("streamableHttp") + ;(flat as any).url.should.equal("https://mcp.example.com") + // `transport` key must NOT be present on the output + ;("transport" in flat).should.be.false() + }) + }) +}) diff --git a/apps/vscode/src/services/mcp/schemas.ts b/apps/vscode/src/services/mcp/schemas.ts index 31c6e939f4..4eb91e9b6f 100644 --- a/apps/vscode/src/services/mcp/schemas.ts +++ b/apps/vscode/src/services/mcp/schemas.ts @@ -11,11 +11,74 @@ export const BaseConfigSchema = z.object({ // Marker for servers that were added by remote config sync. // Used to identify which servers should be removed when they are no longer in the remote config. remoteConfigured: z.boolean().optional(), + // OAuth state written by the CLI — preserved as-is (VSCode doesn't implement OAuth flows yet) + oauth: z.unknown().optional(), + // Arbitrary metadata written by the CLI — preserved as-is + metadata: z.unknown().optional(), }) +// Transport schemas for the nested format (as written by the Cline CLI) +const nestedStdioTransportSchema = z.object({ + type: z.literal("stdio"), + command: z.string().min(1), + args: z.array(z.string()).optional(), + cwd: z.string().optional(), + env: z.record(z.string(), z.string()).optional(), +}) + +const nestedSseTransportSchema = z.object({ + type: z.literal("sse"), + url: z.string().url("URL must be a valid URL format"), + headers: z.record(z.string(), z.string()).optional(), +}) + +const nestedStreamableHttpTransportSchema = z.object({ + type: z.literal("streamableHttp"), + url: z.string().url("URL must be a valid URL format"), + headers: z.record(z.string(), z.string()).optional(), +}) + +/** + * Nested transport format as produced by the Cline CLI (`cline mcp add`). + * + * The CLI writes: + * ```json + * { "transport": { "type": "streamableHttp", "url": "..." }, "disabled": false, "oauth": { ... } } + * ``` + * + * This arm normalises it to the flat format used internally by the extension: + * ```json + * { "type": "streamableHttp", "url": "...", "disabled": false, "oauth": { ... } } + * ``` + * + * Placed first in the union so the `transport` key acts as an unambiguous discriminator. + */ +const nestedTransportConfigSchema = z + .object({ + transport: z.discriminatedUnion("type", [ + nestedStdioTransportSchema, + nestedSseTransportSchema, + nestedStreamableHttpTransportSchema, + ]), + disabled: z.boolean().optional(), + autoApprove: AutoApproveSchema.optional(), + timeout: z.number().min(MIN_MCP_TIMEOUT_SECONDS).optional().default(DEFAULT_MCP_TIMEOUT_SECONDS), + remoteConfigured: z.boolean().optional(), + oauth: z.unknown().optional(), + metadata: z.unknown().optional(), + }) + .transform((data) => { + const { transport, ...rest } = data + // Flatten: hoist transport fields to the top level (matches the flat format) + return { ...transport, ...rest } + }) + // Helper function to create a refined schema with better error messages const createServerTypeSchema = () => { return z.union([ + // Nested transport format (as written by the CLI: { transport: { type, ... }, ... }) + // Must be first so the presence of a `transport` key is an unambiguous discriminator. + nestedTransportConfigSchema, // Stdio config (has command field) BaseConfigSchema.extend({ type: z.literal("stdio").optional(), diff --git a/apps/vscode/webview-ui/package-lock.json b/apps/vscode/webview-ui/package-lock.json index 1ef0047f82..5b2a18d1d1 100644 --- a/apps/vscode/webview-ui/package-lock.json +++ b/apps/vscode/webview-ui/package-lock.json @@ -34,6 +34,7 @@ "fuse.js": "^7.0.0", "fzf": "^0.5.2", "lucide-react": "^0.511.0", + "magic-string": "^0.30.21", "mermaid": "11.12.3", "posthog-js": "^1.224.0", "pretty-bytes": "^6.1.1", @@ -81,7 +82,7 @@ "optionalDependencies": { "@rollup/rollup-linux-arm64-gnu": "^4.40.0", "@rollup/rollup-linux-x64-gnu": "^4.40.0", - "@rollup/rollup-win32-x64-msvc": "^4.40.0", + "@rollup/rollup-win32-x64-msvc": "^4.60.4", "@swc/core-linux-x64-gnu": "^1.11.0", "@tailwindcss/oxide-linux-x64-gnu": "^4.0.1", "lightningcss-linux-x64-gnu": "^1.29.1", @@ -5651,9 +5652,9 @@ ] }, "node_modules/@rollup/rollup-win32-x64-msvc": { - "version": "4.60.0", - "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-x64-msvc/-/rollup-win32-x64-msvc-4.60.0.tgz", - "integrity": "sha512-PrsWNQ8BuE00O3Xsx3ALh2Df8fAj9+cvvX9AIA6o4KpATR98c9mud4XtDWVvsEuyia5U4tVSTKygawyJkjm60w==", + "version": "4.60.4", + "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-x64-msvc/-/rollup-win32-x64-msvc-4.60.4.tgz", + "integrity": "sha512-QVTUovf40zgTqlFVrKA1uXMVvU2QWEFWfAH8Wdc48IxLvrJMQVMBRjuQyUpzZCDkakImib9eVazbWlC6ksWtJw==", "cpu": [ "x64" ], @@ -7357,6 +7358,8 @@ }, "node_modules/class-variance-authority": { "version": "0.7.1", + "resolved": "https://registry.npmjs.org/class-variance-authority/-/class-variance-authority-0.7.1.tgz", + "integrity": "sha512-Ka+9Trutv7G8M6WT6SeiRWz792K5qEqIGEGzXKhAE6xOWAY6pPH8U+9IY3oCMv6kqTmLsv7Xh/2w2RigkePMsg==", "license": "Apache-2.0", "dependencies": { "clsx": "^2.1.1" @@ -9983,8 +9986,9 @@ } }, "node_modules/magic-string": { - "version": "0.30.19", - "dev": true, + "version": "0.30.21", + "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.30.21.tgz", + "integrity": "sha512-vd2F4YUyEXKGcLHoq+TEyCjxueSeHnFxyyjNp80yg0XV4vUhnDer/lvvlqM/arB5bXQN5K2/3oinyCRyx8T2CQ==", "license": "MIT", "dependencies": { "@jridgewell/sourcemap-codec": "^1.5.5" @@ -12147,6 +12151,8 @@ }, "node_modules/react-remark": { "version": "2.1.0", + "resolved": "https://registry.npmjs.org/react-remark/-/react-remark-2.1.0.tgz", + "integrity": "sha512-7dEPxRGQ23sOdvteuRGaQAs9cEOH/BOeCN4CqsJdk3laUDIDYRCWnM6a3z92PzXHUuxIRLXQNZx7SiO0ijUcbw==", "license": "MIT", "dependencies": { "rehype-react": "^6.0.0", @@ -12730,6 +12736,20 @@ "fsevents": "~2.3.2" } }, + "node_modules/rollup/node_modules/@rollup/rollup-win32-x64-msvc": { + "version": "4.60.0", + "resolved": "https://registry.npmjs.org/@rollup/rollup-win32-x64-msvc/-/rollup-win32-x64-msvc-4.60.0.tgz", + "integrity": "sha512-PrsWNQ8BuE00O3Xsx3ALh2Df8fAj9+cvvX9AIA6o4KpATR98c9mud4XtDWVvsEuyia5U4tVSTKygawyJkjm60w==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ] + }, "node_modules/roughjs": { "version": "4.6.6", "license": "MIT", diff --git a/apps/vscode/webview-ui/package.json b/apps/vscode/webview-ui/package.json index 231951ad1b..9ebc031d6a 100644 --- a/apps/vscode/webview-ui/package.json +++ b/apps/vscode/webview-ui/package.json @@ -42,6 +42,7 @@ "fuse.js": "^7.0.0", "fzf": "^0.5.2", "lucide-react": "^0.511.0", + "magic-string": "^0.30.21", "mermaid": "11.12.3", "posthog-js": "^1.224.0", "pretty-bytes": "^6.1.1", @@ -92,7 +93,7 @@ "optionalDependencies": { "@rollup/rollup-linux-arm64-gnu": "^4.40.0", "@rollup/rollup-linux-x64-gnu": "^4.40.0", - "@rollup/rollup-win32-x64-msvc": "^4.40.0", + "@rollup/rollup-win32-x64-msvc": "^4.60.4", "@swc/core-linux-x64-gnu": "^1.11.0", "@tailwindcss/oxide-linux-x64-gnu": "^4.0.1", "lightningcss-linux-x64-gnu": "^1.29.1",