fix: do not wipe JSONC MCP configs when adding Patchloom

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)

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.
 

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"
   }
 }

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<void> {
   const binaryPath = await ensurePatchloomReadyOrNotify("Patchloom needs a working binary before MCP setup can continue.");
@@ -63,24 +64,30 @@ export async function configureMcp(): Promise<void> {
   }
 
   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);

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<Mc
 
   for (const target of targets) {
     const content = await readFile(target.filePath);
-    const config = parseJsonObject(content);
+    let configured = false;
+    if (content !== undefined) {
+      try {
+        configured = hasPatchloomEntry(target.kind, parseJsonObject(content, target.filePath));
+      } catch {
+        configured = false;
+      }
+    }
     results.push({
       ...target,
       exists: content !== undefined,
-      configured: hasPatchloomEntry(target.kind, config)
+      configured
     });
   }
 
@@ -68,7 +76,7 @@ export async function configureMcpTargets(inputs: McpApplyInputs): Promise<McpTa
 
   for (const target of targets) {
     const content = await readFile(target.filePath);
-    const original = parseJsonObject(content);
+    const original = parseJsonObject(content, target.filePath);
     const updated = withPatchloomEntry(target.kind, original, patchloomCommand, mcpSurface);
     const serialized = `${JSON.stringify(updated, null, 2)}\n`;
     const previousSerialized = content === undefined ? undefined : `${JSON.stringify(original, null, 2)}\n`;
@@ -176,19 +184,21 @@ function objectValue(value: unknown): Record<string, unknown> {
     : {};
 }
 
-function parseJsonObject(content: string | undefined): Record<string, unknown> {
+function isPlainObject(value: unknown): value is Record<string, unknown> {
+  return value !== null && typeof value === "object" && !Array.isArray(value);
+}
+
+function parseJsonObject(content: string | undefined, filePath: string): Record<string, unknown> {
   if (!content || !content.trim()) {
     return {};
   }
 
-  try {
-    const parsed = JSON.parse(content) as unknown;
-    return parsed && typeof parsed === "object" && !Array.isArray(parsed)
-      ? { ...(parsed as Record<string, unknown>) }
-      : {};
-  } 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<string | undefined> {

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<string, unknown>;
+    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<string, unknown>).patchloom);
+    const vscodeTarget = targets.find((t) => t.kind === "vscode-workspace");
+    assert.ok(vscodeTarget);
+    assert.equal(vscodeTarget.exists, true);
+    assert.equal(vscodeTarget.configured, false);
   });
 });