diff --git a/AGENTS.md b/AGENTS.md index b0c168e..a5a7abf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,7 +55,7 @@ test/ binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (69 tests) managedLifecycle.test.ts Managed install with real file I/O (26 tests) - mcpConfig.test.ts MCP config with real temp directories (12 tests) + mcpConfig.test.ts MCP config with real temp directories (14 tests) managedInstall.test.ts Managed Update compares latest vs managed binary (10 tests) mcpRegister.test.ts Native MCP definition helper for binary path (2 tests) statusRefresh.test.ts Status and MCP refresh order after input change (1 test) diff --git a/README.md b/README.md index d0e8494..5169a35 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ Run `Patchloom: Setup Workspace` to walk through everything your project needs: - **Cursor** (`.cursor/mcp.json`) - **Windsurf** (`~/.codeium/windsurf/mcp_config.json`) -When configuring, pick **Full tool inventory** (default) or **Core pack**. Core sets `PATCHLOOM_MCP_SURFACE=core` on the server entry. +When configuring, pick **Full tool inventory** (default) or **Core pack**. Core sets `PATCHLOOM_MCP_SURFACE=core` on the server entry. Existing servers in JSON or JSONC (`//` comments, trailing commas) stay in the file. A config that is not an object is left unchanged and the command reports an error. CLI **0.31.0** (and 0.24+) exposes **58** MCP tools by default (including `list_files` and `apply_fragment`). The core pack is 11 tools: `read_file`, `search_files`, `list_files`, `replace_text`, `batch_replace`, `doc_get`, `doc_set`, `doc_query`, `md_replace_section`, `execute_plan`, `server_info`. `search_files` accepts `files_without_match` (CLI 0.29+). `apply_patch` accepts unified diffs, Codex `*** Begin Patch`, and Aider SEARCH/REPLACE (CLI 0.30+). Absolute paths that resolve inside the MCP workspace root are allowed; empty paths, `../`, and outside paths still reject with stable `error_kind` peels. diff --git a/package-lock.json b/package-lock.json index c1ac344..c381a1e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,6 +8,9 @@ "name": "patchloom", "version": "0.5.0", "license": "MIT", + "dependencies": { + "jsonc-parser": "^3.3.1" + }, "devDependencies": { "@types/mocha": "^10.0.10", "@types/node": "^26.4.0", @@ -4969,7 +4972,6 @@ "version": "3.3.1", "resolved": "https://registry.npmjs.org/jsonc-parser/-/jsonc-parser-3.3.1.tgz", "integrity": "sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ==", - "dev": true, "license": "MIT" }, "node_modules/jsonfile": { diff --git a/package.json b/package.json index 8ca5524..4777b95 100644 --- a/package.json +++ b/package.json @@ -270,5 +270,8 @@ "ovsx": "^1.1.1", "typescript": "^7.0.2", "vscode-extension-tester": "^8.24.0" + }, + "dependencies": { + "jsonc-parser": "^3.3.1" } } diff --git a/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index 1016b3f..dbcb1a0 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -3,8 +3,9 @@ import * as path from "node:path"; import * as vscode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; import { configureMcpTargets, inspectMcpTargets } from "../mcp/config.js"; -import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; import { refreshStatusBar } from "../status/statusBar.js"; +import { formatError } from "../util.js"; +import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; export async function configureMcp(): Promise { const binaryPath = await ensurePatchloomReadyOrNotify("Patchloom needs a working binary before MCP setup can continue."); @@ -63,24 +64,30 @@ export async function configureMcp(): Promise { } const selectedKinds = selections.map((selection) => selection.target.kind); - const results = await configureMcpTargets({ - workspaceFolderPath, - includeKinds: selectedKinds, - includeUserTarget: environment.supportsUserMcpConfig, - patchloomPathSetting: binaryPath, - mcpSurface: surfacePick.surface, - readFile: async (filePath) => { - try { - return await fs.readFile(filePath, "utf8"); - } catch { - return undefined; + let results; + try { + results = await configureMcpTargets({ + workspaceFolderPath, + includeKinds: selectedKinds, + includeUserTarget: environment.supportsUserMcpConfig, + patchloomPathSetting: binaryPath, + mcpSurface: surfacePick.surface, + readFile: async (filePath) => { + try { + return await fs.readFile(filePath, "utf8"); + } catch { + return undefined; + } + }, + writeFile: async (filePath, content) => { + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, content, "utf8"); } - }, - writeFile: async (filePath, content) => { - await fs.mkdir(path.dirname(filePath), { recursive: true }); - await fs.writeFile(filePath, content, "utf8"); - } - }); + }); + } catch (error) { + await vscode.window.showErrorMessage(`Failed to configure MCP: ${formatError(error)}`); + return; + } const applied = results; const changed = applied.filter((result) => result.changed); diff --git a/src/mcp/config.ts b/src/mcp/config.ts index 906ab4b..fa05f1a 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -1,4 +1,5 @@ import * as path from "node:path"; +import { parse, type ParseError } from "jsonc-parser"; import { configuredBinaryPathFromSetting } from "../binary/patchloom.js"; export type McpTargetKind = "vscode-workspace" | "cursor-workspace" | "windsurf-user"; @@ -46,11 +47,18 @@ export async function inspectMcpTargets(inputs: McpInspectionInputs): Promise { : {}; } -function parseJsonObject(content: string | undefined): Record { +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function parseJsonObject(content: string | undefined, filePath: string): Record { if (!content || !content.trim()) { return {}; } - try { - const parsed = JSON.parse(content) as unknown; - return parsed && typeof parsed === "object" && !Array.isArray(parsed) - ? { ...(parsed as Record) } - : {}; - } catch { - return {}; + const errors: ParseError[] = []; + const parsed: unknown = parse(content, errors, { allowTrailingComma: true }); + if (errors.length > 0 || !isPlainObject(parsed)) { + throw new Error(`Cannot parse MCP config ${filePath}: invalid JSONC or not a JSON object`); } + return { ...parsed }; } async function defaultReadFile(filePath: string): Promise { diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index 3b4adfb..4d15fc6 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -85,6 +85,47 @@ test("configureMcpTargets writes core surface env when requested", async () => { }); }); +test("configureMcpTargets preserves sibling servers in JSONC mcp.json", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + const filePath = path.join(vscodeDir, "mcp.json"); + await fs.writeFile( + filePath, + `{ + // comment + "servers": { + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github"] + }, + } +} +`, + "utf8" + ); + + await configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + readFile: async (targetPath) => { + try { return await fs.readFile(targetPath, "utf8"); } catch { return undefined; } + }, + writeFile: async (targetPath, content) => { + await fs.mkdir(path.dirname(targetPath), { recursive: true }); + await fs.writeFile(targetPath, content, "utf8"); + } + }); + + const written = await readJson(filePath); + const servers = written.servers as Record; + assert.ok(servers.github, "existing github server should be preserved"); + assert.ok(servers.patchloom, "patchloom server should be added"); + }); +}); + test("configureMcpTargets preserves existing servers in the config file", async () => { await withTempDir(async (workspace) => { const vscodeDir = path.join(workspace, ".vscode"); @@ -195,29 +236,61 @@ test("configureMcpTargets is idempotent on second call", async () => { }); }); -test("configureMcpTargets handles invalid JSON in existing file gracefully", async () => { +test("configureMcpTargets refuses garbage JSON and leaves the file unchanged", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + const filePath = path.join(vscodeDir, "mcp.json"); + const original = "not json {{{"; + await fs.writeFile(filePath, original, "utf8"); + + let wrote = false; + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + readFile: async (targetPath) => { + try { return await fs.readFile(targetPath, "utf8"); } catch { return undefined; } + }, + writeFile: async (targetPath, content) => { + wrote = true; + await fs.mkdir(path.dirname(targetPath), { recursive: true }); + await fs.writeFile(targetPath, content, "utf8"); + } + }), + (err: unknown) => { + assert.ok(err instanceof Error); + assert.match(err.message, /mcp\.json/); + return true; + } + ); + + assert.equal(wrote, false, "garbage config must not be overwritten"); + const after = await fs.readFile(filePath, "utf8"); + assert.equal(after, original); + }); +}); + +test("inspectMcpTargets reports unconfigured when existing file is not valid JSONC", async () => { await withTempDir(async (workspace) => { const vscodeDir = path.join(workspace, ".vscode"); await fs.mkdir(vscodeDir, { recursive: true }); await fs.writeFile(path.join(vscodeDir, "mcp.json"), "not json {{{", "utf8"); - const results = await configureMcpTargets({ + const targets = await inspectMcpTargets({ workspaceFolderPath: workspace, homeDir: workspace, - includeKinds: ["vscode-workspace"], - patchloomPathSetting: "patchloom", - readFile: async (filePath) => { - try { return await fs.readFile(filePath, "utf8"); } catch { return undefined; } - }, - writeFile: async (filePath, content) => { - await fs.mkdir(path.dirname(filePath), { recursive: true }); - await fs.writeFile(filePath, content, "utf8"); + readFile: async (targetPath) => { + try { return await fs.readFile(targetPath, "utf8"); } catch { return undefined; } } }); - assert.equal(results[0].changed, true); - const written = await readJson(path.join(vscodeDir, "mcp.json")); - assert.ok((written.servers as Record).patchloom); + const vscodeTarget = targets.find((t) => t.kind === "vscode-workspace"); + assert.ok(vscodeTarget); + assert.equal(vscodeTarget.exists, true); + assert.equal(vscodeTarget.configured, false); }); });