From 5ca4912af13bf2495606045f85d1522ac796e7c5 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 10:29:19 -0700 Subject: [PATCH 1/5] fix(converters): preserve user config when writing MCP servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The install and convert paths for Codex, Copilot, and Qwen were overwriting the entire config file with only plugin-managed MCP servers, wiping user settings (model preferences, custom MCP servers, etc.). Codex: Use managed block markers (BEGIN/END) so plugin MCP config coexists with user TOML. Extract shared mergeCodexConfig() used by both the install and sync paths. Migrate previous-generation markers and the unmarked legacy format (# Generated by compound-plugin). Copilot/Qwen: Track plugin-managed server names in a _compound_managed_mcp array within the JSON config. On re-install, only managed servers are replaced — user-added servers are preserved. Stale managed servers (renamed/removed from plugin) are cleaned up. Legacy configs without tracking key treat all existing servers as plugin-managed for safe first migration. All three targets now converge correctly when the plugin ships zero MCP servers (stale managed config is removed). Also skips malformed MCP servers (neither command nor url) instead of emitting empty TOML sections. Fixes #478 Co-Authored-By: Claude Opus 4.6 (1M context) --- src/sync/codex.ts | 65 ++------- src/targets/codex.ts | 79 +++++++++- src/targets/copilot.ts | 64 +++++++- src/targets/qwen.ts | 62 +++++++- tests/codex-writer.test.ts | 273 ++++++++++++++++++++++++++++++++++- tests/copilot-writer.test.ts | 110 ++++++++++++++ tests/qwen-writer.test.ts | 101 +++++++++++++ tests/sync-codex.test.ts | 29 +++- 8 files changed, 711 insertions(+), 72 deletions(-) create mode 100644 tests/qwen-writer.test.ts diff --git a/src/sync/codex.ts b/src/sync/codex.ts index b7b894ef7..bf0cc8175 100644 --- a/src/sync/codex.ts +++ b/src/sync/codex.ts @@ -1,15 +1,11 @@ import fs from "fs/promises" import path from "path" import type { ClaudeHomeConfig } from "../parsers/claude-home" -import { renderCodexConfig } from "../targets/codex" +import { mergeCodexConfig, renderCodexConfig } from "../targets/codex" import { writeTextSecure } from "../utils/files" import { syncCodexCommands } from "./commands" import { syncSkills } from "./skills" -const CURRENT_START_MARKER = "# BEGIN compound-plugin Claude Code MCP" -const CURRENT_END_MARKER = "# END compound-plugin Claude Code MCP" -const LEGACY_MARKER = "# MCP servers synced from Claude Code" - export async function syncToCodex( config: ClaudeHomeConfig, outputRoot: string, @@ -17,52 +13,19 @@ export async function syncToCodex( await syncSkills(config.skills, path.join(outputRoot, "skills")) await syncCodexCommands(config, outputRoot) - // Write MCP servers to config.toml (TOML format) - if (Object.keys(config.mcpServers).length > 0) { - const configPath = path.join(outputRoot, "config.toml") - const mcpToml = renderCodexConfig(config.mcpServers) - if (!mcpToml) { - return - } - - // Read existing config and merge idempotently - let existingContent = "" - try { - existingContent = await fs.readFile(configPath, "utf-8") - } catch (err) { - if ((err as NodeJS.ErrnoException).code !== "ENOENT") { - throw err - } + // Write MCP servers to config.toml, or clean up stale managed block if none remain + const configPath = path.join(outputRoot, "config.toml") + let existingContent = "" + try { + existingContent = await fs.readFile(configPath, "utf-8") + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + throw err } - - const managedBlock = [ - CURRENT_START_MARKER, - mcpToml.trim(), - CURRENT_END_MARKER, - "", - ].join("\n") - - const withoutCurrentBlock = existingContent.replace( - new RegExp( - `${escapeForRegex(CURRENT_START_MARKER)}[\\s\\S]*?${escapeForRegex(CURRENT_END_MARKER)}\\n?`, - "g", - ), - "", - ).trimEnd() - - const legacyMarkerIndex = withoutCurrentBlock.indexOf(LEGACY_MARKER) - const cleaned = legacyMarkerIndex === -1 - ? withoutCurrentBlock - : withoutCurrentBlock.slice(0, legacyMarkerIndex).trimEnd() - - const newContent = cleaned - ? `${cleaned}\n\n${managedBlock}` - : `${managedBlock}` - - await writeTextSecure(configPath, newContent) } -} - -function escapeForRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") + const mcpToml = renderCodexConfig(config.mcpServers) + const merged = mergeCodexConfig(existingContent, mcpToml) + if (merged !== null) { + await writeTextSecure(configPath, merged) + } } diff --git a/src/targets/codex.ts b/src/targets/codex.ts index 25c6780a1..a0277da07 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -1,9 +1,17 @@ +import fs from "fs/promises" import path from "path" -import { backupFile, copySkillDir, ensureDir, sanitizePathName, writeText } from "../utils/files" +import { backupFile, copySkillDir, ensureDir, sanitizePathName, writeText, writeTextSecure } from "../utils/files" import type { CodexBundle } from "../types/codex" import type { ClaudeMcpServer } from "../types/claude" import { transformContentForCodex } from "../utils/codex-content" +const MANAGED_START_MARKER = "# BEGIN Compound Engineering plugin MCP -- do not edit this block" +const MANAGED_END_MARKER = "# END Compound Engineering plugin MCP" +const PREV_START_MARKER = "# BEGIN compound-plugin Claude Code MCP" +const PREV_END_MARKER = "# END compound-plugin Claude Code MCP" +const LEGACY_MARKER = "# MCP servers synced from Claude Code" +const UNMARKED_LEGACY_MARKER = "# Generated by compound-plugin" + export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): Promise { const codexRoot = resolveCodexRoot(outputRoot) await ensureDir(codexRoot) @@ -35,14 +43,16 @@ export async function writeCodexBundle(outputRoot: string, bundle: CodexBundle): } } - const config = renderCodexConfig(bundle.mcpServers) - if (config) { - const configPath = path.join(codexRoot, "config.toml") + const configPath = path.join(codexRoot, "config.toml") + const existingConfig = await readFileSafe(configPath) + const mcpToml = renderCodexConfig(bundle.mcpServers) + const merged = mergeCodexConfig(existingConfig, mcpToml) + if (merged !== null) { const backupPath = await backupFile(configPath) if (backupPath) { console.log(`Backed up existing config to ${backupPath}`) } - await writeText(configPath, config) + await writeTextSecure(configPath, merged) } } @@ -53,9 +63,11 @@ function resolveCodexRoot(outputRoot: string): string { export function renderCodexConfig(mcpServers?: Record): string | null { if (!mcpServers || Object.keys(mcpServers).length === 0) return null - const lines: string[] = ["# Generated by compound-plugin", ""] + const lines: string[] = [] for (const [name, server] of Object.entries(mcpServers)) { + if (!server.command && !server.url) continue + const key = formatTomlKey(name) lines.push(`[mcp_servers.${key}]`) @@ -83,7 +95,60 @@ export function renderCodexConfig(mcpServers?: Record): lines.push("") } - return lines.join("\n") + return lines.length > 0 ? lines.join("\n") : null +} + +async function readFileSafe(filePath: string): Promise { + try { + return await fs.readFile(filePath, "utf-8") + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + throw err + } + return "" + } +} + +export function mergeCodexConfig(existingContent: string, mcpToml: string | null): string | null { + // Strip current and previous managed blocks + let stripped = existingContent + for (const [start, end] of [[MANAGED_START_MARKER, MANAGED_END_MARKER], [PREV_START_MARKER, PREV_END_MARKER]]) { + stripped = stripped.replace( + new RegExp(`${escapeForRegex(start)}[\\s\\S]*?${escapeForRegex(end)}\\n?`, "g"), + "", + ) + } + stripped = stripped.trimEnd() + + // Strip from legacy markers to end of content (old formats wrote everything after the marker) + let cleaned = stripped + for (const marker of [LEGACY_MARKER, UNMARKED_LEGACY_MARKER]) { + const idx = cleaned.indexOf(marker) + if (idx !== -1) { + cleaned = cleaned.slice(0, idx).trimEnd() + } + } + + // No MCP servers to write — return cleaned content, or null only if there was never a file + if (!mcpToml) { + if (!existingContent) return null + return cleaned + } + + const managedBlock = [ + MANAGED_START_MARKER, + mcpToml.trim(), + MANAGED_END_MARKER, + "", + ].join("\n") + + return cleaned + ? `${cleaned}\n\n${managedBlock}` + : `${managedBlock}` +} + +function escapeForRegex(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") } function formatTomlString(value: string): string { diff --git a/src/targets/copilot.ts b/src/targets/copilot.ts index ca1a303ad..45e8b3466 100644 --- a/src/targets/copilot.ts +++ b/src/targets/copilot.ts @@ -1,5 +1,5 @@ import path from "path" -import { backupFile, copySkillDir, ensureDir, sanitizePathName, writeJson, writeText } from "../utils/files" +import { backupFile, copySkillDir, ensureDir, pathExists, readJson, sanitizePathName, writeJsonSecure, writeText } from "../utils/files" import { transformContentForCopilot } from "../converters/claude-to-copilot" import type { CopilotBundle } from "../types/copilot" @@ -28,13 +28,69 @@ export async function writeCopilotBundle(outputRoot: string, bundle: CopilotBund } } - if (bundle.mcpConfig && Object.keys(bundle.mcpConfig).length > 0) { - const mcpPath = path.join(paths.githubDir, "copilot-mcp-config.json") + const mcpPath = path.join(paths.githubDir, "copilot-mcp-config.json") + const merged = await mergeCopilotMcpConfig(mcpPath, bundle.mcpConfig ?? {}) + if (merged !== null) { const backupPath = await backupFile(mcpPath) if (backupPath) { console.log(`Backed up existing copilot-mcp-config.json to ${backupPath}`) } - await writeJson(mcpPath, { mcpServers: bundle.mcpConfig }) + await writeJsonSecure(mcpPath, merged) + } +} + +const MANAGED_KEY = "_compound_managed_mcp" + +async function mergeCopilotMcpConfig( + configPath: string, + incoming: Record, +): Promise | null> { + let existing: Record = {} + if (await pathExists(configPath)) { + try { + const parsed = await readJson(configPath) + if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) { + existing = parsed as Record + } + } catch { + // Unparseable file — proceed with incoming only + } + } + + const existingMcp = (typeof existing.mcpServers === "object" && existing.mcpServers !== null && !Array.isArray(existing.mcpServers)) + ? { ...(existing.mcpServers as Record) } + : {} + + // Remove previously-managed plugin servers that are no longer in the bundle. + // Legacy migration: if no tracking key exists, assume all existing servers are plugin-managed + // (the old writer overwrote the entire file, so there are no user servers to preserve). + const prevManaged = Array.isArray(existing[MANAGED_KEY]) + ? existing[MANAGED_KEY] as string[] + : Object.keys(existingMcp) + for (const name of prevManaged) { + if (!(name in incoming)) { + delete existingMcp[name] + } + } + + const mergedMcp = { ...existingMcp, ...incoming } + const incomingKeys = Object.keys(incoming) + + // Nothing to write — no user servers, no plugin servers, no existing file + if (Object.keys(mergedMcp).length === 0 && Object.keys(existing).length === 0) { + return null + } + + // Strip tracking key when plugin has no servers and no managed entries remain + const { [MANAGED_KEY]: _, ...rest } = existing + if (incomingKeys.length === 0 && Object.keys(mergedMcp).length === 0) { + return { ...rest, mcpServers: {} } + } + + return { + ...existing, + mcpServers: mergedMcp, + [MANAGED_KEY]: incomingKeys, } } diff --git a/src/targets/qwen.ts b/src/targets/qwen.ts index 7a4e9c1cb..398bcdd7c 100644 --- a/src/targets/qwen.ts +++ b/src/targets/qwen.ts @@ -1,18 +1,19 @@ import path from "path" -import { backupFile, copyDir, ensureDir, resolveCommandPath, sanitizePathName, writeJson, writeText } from "../utils/files" +import { backupFile, copyDir, ensureDir, readJson, resolveCommandPath, sanitizePathName, pathExists, writeJsonSecure, writeText } from "../utils/files" import type { QwenBundle, QwenExtensionConfig } from "../types/qwen" export async function writeQwenBundle(outputRoot: string, bundle: QwenBundle): Promise { const qwenPaths = resolveQwenPaths(outputRoot) await ensureDir(qwenPaths.root) - // Write qwen-extension.json config + // Merge qwen-extension.json config, preserving existing user MCP servers const configPath = qwenPaths.configPath const backupPath = await backupFile(configPath) if (backupPath) { console.log(`Backed up existing config to ${backupPath}`) } - await writeJson(configPath, bundle.config) + const merged = await mergeQwenConfig(configPath, bundle.config) + await writeJsonSecure(configPath, merged) // Write context file (QWEN.md) if (bundle.contextFile) { @@ -45,6 +46,61 @@ export async function writeQwenBundle(outputRoot: string, bundle: QwenBundle): P } } +const MANAGED_KEY = "_compound_managed_mcp" + +async function mergeQwenConfig( + configPath: string, + incoming: QwenExtensionConfig, +): Promise { + let existing: Record = {} + if (await pathExists(configPath)) { + try { + const parsed = await readJson(configPath) + if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) { + existing = parsed as Record + } + } catch { + // Unparseable file — proceed with incoming only + } + } + + const existingMcp = (typeof existing.mcpServers === "object" && existing.mcpServers !== null && !Array.isArray(existing.mcpServers)) + ? { ...(existing.mcpServers as Record) } + : {} + + // Remove previously-managed plugin servers that are no longer in the bundle. + // Legacy migration: if no tracking key exists, assume all existing servers are plugin-managed + // (the old writer overwrote the entire file, so there are no user servers to preserve). + const prevManaged = Array.isArray(existing[MANAGED_KEY]) + ? existing[MANAGED_KEY] as string[] + : Object.keys(existingMcp) + const incomingMcp = incoming.mcpServers ?? {} + for (const name of prevManaged) { + if (!(name in incomingMcp)) { + delete existingMcp[name] + } + } + + const mergedMcp = { ...existingMcp, ...incomingMcp } + const { mcpServers: _, ...incomingRest } = incoming + const merged = { ...existing, ...incomingRest } as QwenExtensionConfig & Record + + if (Object.keys(mergedMcp).length > 0) { + merged.mcpServers = mergedMcp as QwenExtensionConfig["mcpServers"] + } else { + delete merged.mcpServers + } + + const incomingKeys = Object.keys(incomingMcp) + if (incomingKeys.length > 0) { + merged[MANAGED_KEY] = incomingKeys + } else { + delete merged[MANAGED_KEY] + } + + return merged as QwenExtensionConfig +} + function resolveQwenPaths(outputRoot: string) { return { root: outputRoot, diff --git a/tests/codex-writer.test.ts b/tests/codex-writer.test.ts index 6e587071b..eb3b6904c 100644 --- a/tests/codex-writer.test.ts +++ b/tests/codex-writer.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test" import { promises as fs } from "fs" import path from "path" import os from "os" -import { writeCodexBundle } from "../src/targets/codex" +import { mergeCodexConfig, renderCodexConfig, writeCodexBundle } from "../src/targets/codex" import type { CodexBundle } from "../src/types/codex" async function exists(filePath: string): Promise { @@ -44,6 +44,8 @@ describe("writeCodexBundle", () => { expect(await exists(configPath)).toBe(true) const config = await fs.readFile(configPath, "utf8") + expect(config).toContain("# BEGIN Compound Engineering plugin MCP -- do not edit this block") + expect(config).toContain("# END Compound Engineering plugin MCP") expect(config).toContain("[mcp_servers.local]") expect(config).toContain("command = \"echo\"") expect(config).toContain("args = [\"hello\"]") @@ -74,12 +76,12 @@ describe("writeCodexBundle", () => { expect(await exists(path.join(codexRoot, "skills", "skill-one", "SKILL.md"))).toBe(true) }) - test("backs up existing config.toml before overwriting", async () => { + test("preserves existing user config when writing MCP servers", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-backup-")) const codexRoot = path.join(tempRoot, ".codex") const configPath = path.join(codexRoot, "config.toml") - // Create existing config + // Create existing config with user settings await fs.mkdir(codexRoot, { recursive: true }) const originalContent = "# My original config\n[custom]\nkey = \"value\"\n" await fs.writeFile(configPath, originalContent) @@ -93,11 +95,17 @@ describe("writeCodexBundle", () => { await writeCodexBundle(codexRoot, bundle) - // New config should be written const newConfig = await fs.readFile(configPath, "utf8") + // Plugin MCP servers should be present in a managed block expect(newConfig).toContain("[mcp_servers.test]") - - // Backup should exist with original content + expect(newConfig).toContain("# BEGIN Compound Engineering plugin MCP -- do not edit this block") + expect(newConfig).toContain("# END Compound Engineering plugin MCP") + // User's original config should be preserved + expect(newConfig).toContain("# My original config") + expect(newConfig).toContain("[custom]") + expect(newConfig).toContain('key = "value"') + + // Backup should still exist with original content const files = await fs.readdir(codexRoot) const backupFileName = files.find((f) => f.startsWith("config.toml.bak.")) expect(backupFileName).toBeDefined() @@ -106,6 +114,120 @@ describe("writeCodexBundle", () => { expect(backupContent).toBe(originalContent) }) + test("is idempotent — running twice does not duplicate managed block", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-idempotent-")) + const codexRoot = path.join(tempRoot, ".codex") + const configPath = path.join(codexRoot, "config.toml") + + await fs.mkdir(codexRoot, { recursive: true }) + await fs.writeFile(configPath, "[user]\nmodel = \"gpt-4.1\"\n") + + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + mcpServers: { test: { command: "echo" } }, + } + + await writeCodexBundle(codexRoot, bundle) + await writeCodexBundle(codexRoot, bundle) + + const config = await fs.readFile(configPath, "utf8") + expect(config.match(/# BEGIN Compound Engineering plugin MCP/g)?.length).toBe(1) + expect(config.match(/# END Compound Engineering plugin MCP/g)?.length).toBe(1) + expect(config).toContain("[user]") + }) + + test("migrates old managed block markers to new ones", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-migrate-")) + const codexRoot = path.join(tempRoot, ".codex") + const configPath = path.join(codexRoot, "config.toml") + + await fs.mkdir(codexRoot, { recursive: true }) + await fs.writeFile(configPath, [ + "[user]", + 'model = "gpt-4.1"', + "", + "# BEGIN compound-plugin Claude Code MCP", + "[mcp_servers.old]", + 'command = "old"', + "# END compound-plugin Claude Code MCP", + ].join("\n")) + + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + mcpServers: { fresh: { command: "new" } }, + } + + await writeCodexBundle(codexRoot, bundle) + + const config = await fs.readFile(configPath, "utf8") + expect(config).not.toContain("# BEGIN compound-plugin Claude Code MCP") + expect(config).toContain("# BEGIN Compound Engineering plugin MCP") + expect(config).not.toContain("[mcp_servers.old]") + expect(config).toContain("[mcp_servers.fresh]") + expect(config).toContain("[user]") + }) + + test("migrates unmarked legacy format (# Generated by compound-plugin)", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-unmarked-")) + const codexRoot = path.join(tempRoot, ".codex") + const configPath = path.join(codexRoot, "config.toml") + + // Simulate old writer output: entire file was just the generated config + await fs.mkdir(codexRoot, { recursive: true }) + await fs.writeFile(configPath, [ + "# Generated by compound-plugin", + "", + "[mcp_servers.old]", + 'command = "old"', + "", + ].join("\n")) + + const bundle: CodexBundle = { + prompts: [], + skillDirs: [], + generatedSkills: [], + mcpServers: { fresh: { command: "new" } }, + } + + await writeCodexBundle(codexRoot, bundle) + + const config = await fs.readFile(configPath, "utf8") + expect(config).not.toContain("# Generated by compound-plugin") + expect(config).not.toContain("[mcp_servers.old]") + expect(config).toContain("# BEGIN Compound Engineering plugin MCP") + expect(config).toContain("[mcp_servers.fresh]") + // Should have exactly one BEGIN marker (no duplication) + expect(config.match(/# BEGIN Compound Engineering plugin MCP/g)?.length).toBe(1) + }) + + test("strips stale managed block when plugin has no MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-stale-")) + const codexRoot = path.join(tempRoot, ".codex") + const configPath = path.join(codexRoot, "config.toml") + + await fs.mkdir(codexRoot, { recursive: true }) + await fs.writeFile(configPath, [ + "[user]", + 'model = "gpt-4.1"', + "", + "# BEGIN Compound Engineering plugin MCP -- do not edit this block", + "[mcp_servers.stale]", + 'command = "should-be-removed"', + "# END Compound Engineering plugin MCP", + ].join("\n")) + + await writeCodexBundle(codexRoot, { prompts: [], skillDirs: [], generatedSkills: [] }) + + const config = await fs.readFile(configPath, "utf8") + expect(config).not.toContain("mcp_servers.stale") + expect(config).not.toContain("# BEGIN Compound Engineering") + expect(config).toContain("[user]") + }) + test("transforms copied SKILL.md files using Codex invocation targets", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "codex-skill-transform-")) const sourceSkillDir = path.join(tempRoot, "source-skill") @@ -265,3 +387,142 @@ Workflow handoff: expect(installedSkill).not.toContain("https://prompts:www.proofeditor.ai") }) }) + +describe("renderCodexConfig", () => { + test("skips servers with neither command nor url", () => { + const result = renderCodexConfig({ broken: {} }) + expect(result).toBeNull() + }) + + test("skips malformed servers but keeps valid ones", () => { + const result = renderCodexConfig({ + valid: { command: "echo" }, + broken: {}, + alsoValid: { url: "https://example.com/mcp" }, + }) + expect(result).not.toBeNull() + expect(result).toContain("[mcp_servers.valid]") + expect(result).toContain("[mcp_servers.alsoValid]") + expect(result).not.toContain("[mcp_servers.broken]") + }) + + test("returns null for empty or undefined input", () => { + expect(renderCodexConfig(undefined)).toBeNull() + expect(renderCodexConfig({})).toBeNull() + }) +}) + +describe("mergeCodexConfig", () => { + test("returns managed block when no existing content", () => { + const result = mergeCodexConfig("", "[mcp_servers.test]\ncommand = \"echo\"") + expect(result).toContain("# BEGIN Compound Engineering plugin MCP") + expect(result).toContain("[mcp_servers.test]") + expect(result).toContain("# END Compound Engineering plugin MCP") + }) + + test("preserves user content and replaces managed block", () => { + const existing = [ + "[user]", + 'model = "gpt-4.1"', + "", + "# BEGIN Compound Engineering plugin MCP -- do not edit this block", + "[mcp_servers.old]", + 'command = "old"', + "# END Compound Engineering plugin MCP", + "", + "[after]", + 'key = "value"', + ].join("\n") + + const result = mergeCodexConfig(existing, "[mcp_servers.new]\ncommand = \"new\"")! + expect(result).toContain("[user]") + expect(result).toContain("[after]") + expect(result).not.toContain("[mcp_servers.old]") + expect(result).toContain("[mcp_servers.new]") + }) + + test("strips previous-generation markers", () => { + const existing = [ + "[user]", + 'model = "gpt-4.1"', + "", + "# BEGIN compound-plugin Claude Code MCP", + "[mcp_servers.old]", + 'command = "old"', + "# END compound-plugin Claude Code MCP", + ].join("\n") + + const result = mergeCodexConfig(existing, "[mcp_servers.new]\ncommand = \"new\"")! + expect(result).not.toContain("# BEGIN compound-plugin Claude Code MCP") + expect(result).not.toContain("[mcp_servers.old]") + expect(result).toContain("# BEGIN Compound Engineering plugin MCP") + expect(result).toContain("[mcp_servers.new]") + }) + + test("returns cleaned content (no block) when mcpToml is null", () => { + const existing = [ + "[user]", + 'model = "gpt-4.1"', + "", + "# BEGIN Compound Engineering plugin MCP -- do not edit this block", + "[mcp_servers.stale]", + 'command = "stale"', + "# END Compound Engineering plugin MCP", + ].join("\n") + + const result = mergeCodexConfig(existing, null)! + expect(result).toContain("[user]") + expect(result).not.toContain("mcp_servers.stale") + expect(result).not.toContain("# BEGIN") + }) + + test("strips unmarked legacy format (# Generated by compound-plugin)", () => { + const existing = [ + "# Generated by compound-plugin", + "", + "[mcp_servers.old]", + 'command = "old"', + "", + ].join("\n") + + const result = mergeCodexConfig(existing, "[mcp_servers.new]\ncommand = \"new\"")! + expect(result).not.toContain("# Generated by compound-plugin") + expect(result).not.toContain("[mcp_servers.old]") + expect(result).toContain("# BEGIN Compound Engineering plugin MCP") + expect(result).toContain("[mcp_servers.new]") + }) + + test("preserves user config before unmarked legacy format", () => { + const existing = [ + "[user]", + 'model = "gpt-4.1"', + "", + "# Generated by compound-plugin", + "", + "[mcp_servers.old]", + 'command = "old"', + ].join("\n") + + const result = mergeCodexConfig(existing, "[mcp_servers.new]\ncommand = \"new\"")! + expect(result).toContain("[user]") + expect(result).not.toContain("# Generated by compound-plugin") + expect(result).not.toContain("[mcp_servers.old]") + expect(result).toContain("[mcp_servers.new]") + }) + + test("returns null when no existing content and no mcpToml", () => { + expect(mergeCodexConfig("", null)).toBeNull() + }) + + test("returns empty string when file was only a managed block and mcpToml is null", () => { + const existing = [ + "# BEGIN Compound Engineering plugin MCP -- do not edit this block", + "[mcp_servers.stale]", + 'command = "stale"', + "# END Compound Engineering plugin MCP", + ].join("\n") + + const result = mergeCodexConfig(existing, null) + expect(result).toBe("") + }) +}) diff --git a/tests/copilot-writer.test.ts b/tests/copilot-writer.test.ts index d87a45f4e..61e95e278 100644 --- a/tests/copilot-writer.test.ts +++ b/tests/copilot-writer.test.ts @@ -203,6 +203,116 @@ Run these research agents: expect(installedSkill).not.toContain("Task compound-engineering:") }) + test("removes stale plugin MCP servers on re-install", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-converge-")) + const githubRoot = path.join(tempRoot, ".github") + + const bundle1: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + mcpConfig: { old: { type: "local", command: "old-server", tools: ["*"] } }, + } + const bundle2: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + mcpConfig: { fresh: { type: "local", command: "new-server", tools: ["*"] } }, + } + + await writeCopilotBundle(tempRoot, bundle1) + await writeCopilotBundle(tempRoot, bundle2) + + const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) + expect(result.mcpServers.fresh).toBeDefined() + expect(result.mcpServers.old).toBeUndefined() + }) + + test("cleans up all plugin MCP servers when bundle has none", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-zero-")) + const githubRoot = path.join(tempRoot, ".github") + + const bundle1: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + mcpConfig: { old: { type: "local", command: "old-server", tools: ["*"] } }, + } + const bundle2: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + // No mcpConfig + } + + await writeCopilotBundle(tempRoot, bundle1) + await writeCopilotBundle(tempRoot, bundle2) + + const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) + expect(result.mcpServers.old).toBeUndefined() + expect(result._compound_managed_mcp).toBeUndefined() + }) + + test("preserves user-added MCP servers across re-installs", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-user-mcp-")) + const githubRoot = path.join(tempRoot, ".github") + await fs.mkdir(githubRoot, { recursive: true }) + + // User has their own MCP server alongside plugin-managed ones (tracking key present) + await fs.writeFile( + path.join(githubRoot, "copilot-mcp-config.json"), + JSON.stringify({ + mcpServers: { "user-tool": { type: "local", command: "my-tool", tools: ["*"] } }, + _compound_managed_mcp: [], + }), + ) + + const bundle: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + mcpConfig: { plugin: { type: "local", command: "plugin-server", tools: ["*"] } }, + } + + await writeCopilotBundle(githubRoot, bundle) + + const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result.mcpServers.plugin).toBeDefined() + }) + + test("prunes stale servers from legacy config without tracking key", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-legacy-")) + const githubRoot = path.join(tempRoot, ".github") + await fs.mkdir(githubRoot, { recursive: true }) + + // Simulate old writer output: has mcpServers but no _compound_managed_mcp + await fs.writeFile( + path.join(githubRoot, "copilot-mcp-config.json"), + JSON.stringify({ + mcpServers: { + old: { type: "local", command: "old-server", tools: ["*"] }, + renamed: { type: "local", command: "renamed-server", tools: ["*"] }, + }, + }), + ) + + const bundle: CopilotBundle = { + agents: [], + generatedSkills: [], + skillDirs: [], + mcpConfig: { fresh: { type: "local", command: "new-server", tools: ["*"] } }, + } + + await writeCopilotBundle(githubRoot, bundle) + + const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) + expect(result.mcpServers.fresh).toBeDefined() + expect(result.mcpServers.old).toBeUndefined() + expect(result.mcpServers.renamed).toBeUndefined() + expect(result._compound_managed_mcp).toEqual(["fresh"]) + }) + test("creates skill directories with SKILL.md", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-genskill-")) const bundle: CopilotBundle = { diff --git a/tests/qwen-writer.test.ts b/tests/qwen-writer.test.ts new file mode 100644 index 000000000..8687017d0 --- /dev/null +++ b/tests/qwen-writer.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from "bun:test" +import { promises as fs } from "fs" +import os from "os" +import path from "path" +import { writeQwenBundle } from "../src/targets/qwen" +import type { QwenBundle } from "../src/types/qwen" + +function makeBundle(mcpServers?: Record): QwenBundle { + return { + config: { + name: "test-plugin", + version: "1.0.0", + commands: "commands", + skills: "skills", + agents: "agents", + mcpServers, + }, + agents: [], + commandFiles: [], + skillDirs: [], + } +} + +describe("writeQwenBundle", () => { + test("removes stale plugin MCP servers on re-install", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-converge-")) + + await writeQwenBundle(tempRoot, makeBundle({ old: { command: "old-server" } })) + await writeQwenBundle(tempRoot, makeBundle({ fresh: { command: "new-server" } })) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.mcpServers.fresh).toBeDefined() + expect(result.mcpServers.old).toBeUndefined() + }) + + test("preserves user-added MCP servers across re-installs", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-user-mcp-")) + + // User has their own MCP server alongside plugin-managed ones (tracking key present) + await fs.writeFile( + path.join(tempRoot, "qwen-extension.json"), + JSON.stringify({ + name: "user-project", + mcpServers: { "user-tool": { command: "my-tool" } }, + _compound_managed_mcp: [], + }), + ) + + await writeQwenBundle(tempRoot, makeBundle({ plugin: { command: "plugin-server" } })) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result.mcpServers.plugin).toBeDefined() + }) + + test("preserves unknown top-level keys from existing config", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-preserve-")) + + await fs.writeFile( + path.join(tempRoot, "qwen-extension.json"), + JSON.stringify({ name: "user-project", customField: "should-survive" }), + ) + + await writeQwenBundle(tempRoot, makeBundle({ plugin: { command: "p" } })) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.customField).toBe("should-survive") + }) + + test("prunes stale servers from legacy config without tracking key", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-legacy-")) + + // Simulate old writer output: has mcpServers but no _compound_managed_mcp + await fs.writeFile( + path.join(tempRoot, "qwen-extension.json"), + JSON.stringify({ + name: "old-project", + mcpServers: { old: { command: "old-server" }, renamed: { command: "renamed-server" } }, + }), + ) + + await writeQwenBundle(tempRoot, makeBundle({ fresh: { command: "new-server" } })) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.mcpServers.fresh).toBeDefined() + expect(result.mcpServers.old).toBeUndefined() + expect(result.mcpServers.renamed).toBeUndefined() + expect(result._compound_managed_mcp).toEqual(["fresh"]) + }) + + test("cleans up all plugin MCP servers when bundle has none", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-zero-")) + + await writeQwenBundle(tempRoot, makeBundle({ old: { command: "old-server" } })) + await writeQwenBundle(tempRoot, makeBundle()) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.mcpServers).toBeUndefined() + expect(result._compound_managed_mcp).toBeUndefined() + }) +}) diff --git a/tests/sync-codex.test.ts b/tests/sync-codex.test.ts index 9714ba842..cd9157b2e 100644 --- a/tests/sync-codex.test.ts +++ b/tests/sync-codex.test.ts @@ -56,9 +56,36 @@ describe("syncToCodex", () => { expect(content).toContain("[mcp_servers.remote]") expect(content).toContain("url = \"https://example.com/mcp\"") expect(content).toContain("http_headers") - expect(content.match(/# BEGIN compound-plugin Claude Code MCP/g)?.length).toBe(1) + // Old markers should be replaced with new ones + expect(content).not.toContain("# BEGIN compound-plugin Claude Code MCP") + expect(content.match(/# BEGIN Compound Engineering plugin MCP/g)?.length).toBe(1) const perms = (await fs.stat(configPath)).mode & 0o777 expect(perms).toBe(0o600) }) + + test("cleans up stale managed block when syncing with zero MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "sync-codex-zero-")) + const fixtureSkillDir = path.join(import.meta.dir, "fixtures", "sample-plugin", "skills", "skill-one") + const configPath = path.join(tempRoot, "config.toml") + + // First sync with MCP servers + const configWithServers: ClaudeHomeConfig = { + skills: [{ name: "skill-one", sourceDir: fixtureSkillDir, skillPath: path.join(fixtureSkillDir, "SKILL.md") }], + mcpServers: { old: { command: "old-server" } }, + } + await syncToCodex(configWithServers, tempRoot) + expect(await fs.readFile(configPath, "utf8")).toContain("[mcp_servers.old]") + + // Second sync with zero MCP servers + const configEmpty: ClaudeHomeConfig = { + skills: [{ name: "skill-one", sourceDir: fixtureSkillDir, skillPath: path.join(fixtureSkillDir, "SKILL.md") }], + mcpServers: {}, + } + await syncToCodex(configEmpty, tempRoot) + + const content = await fs.readFile(configPath, "utf8") + expect(content).not.toContain("[mcp_servers.old]") + expect(content).not.toContain("# BEGIN") + }) }) From 8007603208b5dc28286250ede71ea6bed9e0b731 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 10:44:30 -0700 Subject: [PATCH 2/5] fix(converters): preserve Qwen tracking key when plugin has zero MCP servers Always write _compound_managed_mcp (even as []) so the legacy fallback doesn't treat user servers as plugin-managed on a future install. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/qwen.ts | 9 +++------ tests/qwen-writer.test.ts | 26 +++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/targets/qwen.ts b/src/targets/qwen.ts index 398bcdd7c..6ce284d03 100644 --- a/src/targets/qwen.ts +++ b/src/targets/qwen.ts @@ -91,12 +91,9 @@ async function mergeQwenConfig( delete merged.mcpServers } - const incomingKeys = Object.keys(incomingMcp) - if (incomingKeys.length > 0) { - merged[MANAGED_KEY] = incomingKeys - } else { - delete merged[MANAGED_KEY] - } + // Always write the tracking key (even as []) so the legacy fallback + // doesn't treat user servers as plugin-managed on a future install. + merged[MANAGED_KEY] = Object.keys(incomingMcp) return merged as QwenExtensionConfig } diff --git a/tests/qwen-writer.test.ts b/tests/qwen-writer.test.ts index 8687017d0..5b1382e86 100644 --- a/tests/qwen-writer.test.ts +++ b/tests/qwen-writer.test.ts @@ -96,6 +96,30 @@ describe("writeQwenBundle", () => { const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) expect(result.mcpServers).toBeUndefined() - expect(result._compound_managed_mcp).toBeUndefined() + expect(result._compound_managed_mcp).toEqual([]) + }) + + test("preserves user servers across zero-MCP-then-MCP round trip", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-roundtrip-")) + + // 1. Install with plugin MCP + await writeQwenBundle(tempRoot, makeBundle({ plugin: { command: "plugin-server" } })) + + // 2. User adds their own server (with tracking key present) + const configPath = path.join(tempRoot, "qwen-extension.json") + const afterInstall = JSON.parse(await fs.readFile(configPath, "utf8")) + afterInstall.mcpServers["user-tool"] = { command: "my-tool" } + await fs.writeFile(configPath, JSON.stringify(afterInstall)) + + // 3. Install with zero plugin MCP + await writeQwenBundle(tempRoot, makeBundle()) + + // 4. Install with plugin MCP again + await writeQwenBundle(tempRoot, makeBundle({ new_plugin: { command: "new-plugin" } })) + + const result = JSON.parse(await fs.readFile(configPath, "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result.mcpServers.new_plugin).toBeDefined() + expect(result.mcpServers.plugin).toBeUndefined() }) }) From 19a07ed26fa91f19ddc844f639f13ee9756b4f14 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 11:08:44 -0700 Subject: [PATCH 3/5] fix(converters): guard legacy fallback against zero-MCP pruning in Copilot and Qwen When no tracking key exists AND incoming MCP is empty, skip the legacy prune path that assumes all existing servers are plugin-managed. Also always retain the tracking key (as []) to prevent the legacy fallback from firing on future installs. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/copilot.ts | 20 ++++++------ src/targets/qwen.ts | 13 +++++--- tests/copilot-writer.test.ts | 60 +++++++++++++++++++++++++++++++++++- tests/qwen-writer.test.ts | 20 ++++++++++++ 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/src/targets/copilot.ts b/src/targets/copilot.ts index 45e8b3466..bb678f10a 100644 --- a/src/targets/copilot.ts +++ b/src/targets/copilot.ts @@ -62,11 +62,15 @@ async function mergeCopilotMcpConfig( : {} // Remove previously-managed plugin servers that are no longer in the bundle. - // Legacy migration: if no tracking key exists, assume all existing servers are plugin-managed - // (the old writer overwrote the entire file, so there are no user servers to preserve). - const prevManaged = Array.isArray(existing[MANAGED_KEY]) + // Legacy migration: if no tracking key exists AND plugin has servers, assume all + // existing servers are plugin-managed (the old writer overwrote the entire file). + // When incoming is empty, skip pruning — there's nothing to migrate and we'd + // wrongly delete user servers from a pre-existing untracked config. + const incomingKeys = Object.keys(incoming) + const hasTrackingKey = Array.isArray(existing[MANAGED_KEY]) + const prevManaged = hasTrackingKey ? existing[MANAGED_KEY] as string[] - : Object.keys(existingMcp) + : incomingKeys.length > 0 ? Object.keys(existingMcp) : [] for (const name of prevManaged) { if (!(name in incoming)) { delete existingMcp[name] @@ -74,19 +78,13 @@ async function mergeCopilotMcpConfig( } const mergedMcp = { ...existingMcp, ...incoming } - const incomingKeys = Object.keys(incoming) // Nothing to write — no user servers, no plugin servers, no existing file if (Object.keys(mergedMcp).length === 0 && Object.keys(existing).length === 0) { return null } - // Strip tracking key when plugin has no servers and no managed entries remain - const { [MANAGED_KEY]: _, ...rest } = existing - if (incomingKeys.length === 0 && Object.keys(mergedMcp).length === 0) { - return { ...rest, mcpServers: {} } - } - + // Always write tracking key (even as []) to prevent legacy fallback on future installs return { ...existing, mcpServers: mergedMcp, diff --git a/src/targets/qwen.ts b/src/targets/qwen.ts index 6ce284d03..2909ce0c9 100644 --- a/src/targets/qwen.ts +++ b/src/targets/qwen.ts @@ -69,12 +69,15 @@ async function mergeQwenConfig( : {} // Remove previously-managed plugin servers that are no longer in the bundle. - // Legacy migration: if no tracking key exists, assume all existing servers are plugin-managed - // (the old writer overwrote the entire file, so there are no user servers to preserve). - const prevManaged = Array.isArray(existing[MANAGED_KEY]) - ? existing[MANAGED_KEY] as string[] - : Object.keys(existingMcp) + // Legacy migration: if no tracking key exists AND plugin has servers, assume all + // existing servers are plugin-managed (the old writer overwrote the entire file). + // When incoming is empty, skip pruning — there's nothing to migrate and we'd + // wrongly delete user servers from a pre-existing untracked config. const incomingMcp = incoming.mcpServers ?? {} + const hasTrackingKey = Array.isArray(existing[MANAGED_KEY]) + const prevManaged = hasTrackingKey + ? existing[MANAGED_KEY] as string[] + : Object.keys(incomingMcp).length > 0 ? Object.keys(existingMcp) : [] for (const name of prevManaged) { if (!(name in incomingMcp)) { delete existingMcp[name] diff --git a/tests/copilot-writer.test.ts b/tests/copilot-writer.test.ts index 61e95e278..aaee54ca5 100644 --- a/tests/copilot-writer.test.ts +++ b/tests/copilot-writer.test.ts @@ -250,7 +250,65 @@ Run these research agents: const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) expect(result.mcpServers.old).toBeUndefined() - expect(result._compound_managed_mcp).toBeUndefined() + expect(result._compound_managed_mcp).toEqual([]) + }) + + test("does not prune untracked user config when plugin has zero MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-untracked-")) + const githubRoot = path.join(tempRoot, ".github") + await fs.mkdir(githubRoot, { recursive: true }) + + // Pre-existing user config with no tracking key (never had the plugin before) + await fs.writeFile( + path.join(githubRoot, "copilot-mcp-config.json"), + JSON.stringify({ + mcpServers: { "user-tool": { type: "local", command: "my-tool", tools: ["*"] } }, + }), + ) + + // Plugin installs with zero MCP servers + await writeCopilotBundle(githubRoot, { + agents: [], + generatedSkills: [], + skillDirs: [], + }) + + const result = JSON.parse(await fs.readFile(path.join(githubRoot, "copilot-mcp-config.json"), "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result._compound_managed_mcp).toEqual([]) + }) + + test("preserves user servers across zero-MCP-then-MCP round trip", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "copilot-roundtrip-")) + const githubRoot = path.join(tempRoot, ".github") + const mcpPath = path.join(githubRoot, "copilot-mcp-config.json") + + // 1. Install with plugin MCP + await writeCopilotBundle(tempRoot, { + agents: [], generatedSkills: [], skillDirs: [], + mcpConfig: { plugin: { type: "local", command: "plugin-server", tools: ["*"] } }, + }) + + // 2. User adds their own server + const afterInstall = JSON.parse(await fs.readFile(mcpPath, "utf8")) + afterInstall.mcpServers["user-tool"] = { type: "local", command: "my-tool", tools: ["*"] } + await fs.writeFile(mcpPath, JSON.stringify(afterInstall)) + + // 3. Install with zero plugin MCP + await writeCopilotBundle(tempRoot, { + agents: [], generatedSkills: [], skillDirs: [], + }) + + // 4. Install with plugin MCP again + await writeCopilotBundle(tempRoot, { + agents: [], generatedSkills: [], skillDirs: [], + mcpConfig: { new_plugin: { type: "local", command: "new-plugin", tools: ["*"] } }, + }) + + const result = JSON.parse(await fs.readFile(mcpPath, "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result.mcpServers.new_plugin).toBeDefined() + expect(result.mcpServers.plugin).toBeUndefined() }) test("preserves user-added MCP servers across re-installs", async () => { diff --git a/tests/qwen-writer.test.ts b/tests/qwen-writer.test.ts index 5b1382e86..5c0eeb30b 100644 --- a/tests/qwen-writer.test.ts +++ b/tests/qwen-writer.test.ts @@ -88,6 +88,26 @@ describe("writeQwenBundle", () => { expect(result._compound_managed_mcp).toEqual(["fresh"]) }) + test("does not prune untracked user config when plugin has zero MCP servers", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-untracked-")) + + // Pre-existing user config with no tracking key (never had the plugin before) + await fs.writeFile( + path.join(tempRoot, "qwen-extension.json"), + JSON.stringify({ + name: "user-project", + mcpServers: { "user-tool": { command: "my-tool" } }, + }), + ) + + // Plugin installs with zero MCP servers + await writeQwenBundle(tempRoot, makeBundle()) + + const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) + expect(result.mcpServers["user-tool"]).toBeDefined() + expect(result._compound_managed_mcp).toEqual([]) + }) + test("cleans up all plugin MCP servers when bundle has none", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-zero-")) From 211a375210dd37e97ab56acf18290fcc3d73f13f Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 11:24:22 -0700 Subject: [PATCH 4/5] fix(converters): prune stale Qwen top-level keys when plugin config changes Track plugin-written top-level keys via _compound_managed_keys, same pattern as _compound_managed_mcp for MCP servers. Keys removed from the plugin config (e.g. settings) are pruned on re-install; user-added keys are preserved. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/targets/qwen.ts | 19 +++++++++++++++++-- tests/qwen-writer.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/targets/qwen.ts b/src/targets/qwen.ts index 2909ce0c9..0694efcf7 100644 --- a/src/targets/qwen.ts +++ b/src/targets/qwen.ts @@ -47,6 +47,8 @@ export async function writeQwenBundle(outputRoot: string, bundle: QwenBundle): P } const MANAGED_KEY = "_compound_managed_mcp" +const MANAGED_KEYS_KEY = "_compound_managed_keys" +const TRACKING_KEYS = new Set([MANAGED_KEY, MANAGED_KEYS_KEY]) async function mergeQwenConfig( configPath: string, @@ -86,6 +88,19 @@ async function mergeQwenConfig( const mergedMcp = { ...existingMcp, ...incomingMcp } const { mcpServers: _, ...incomingRest } = incoming + const incomingTopKeys = Object.keys(incomingRest).filter((k) => !TRACKING_KEYS.has(k)) + + // Prune top-level keys from previous installs that are no longer in the incoming bundle. + // Only prune keys we previously tracked; skip on first install (no tracking key yet). + const prevManagedKeys = Array.isArray(existing[MANAGED_KEYS_KEY]) + ? existing[MANAGED_KEYS_KEY] as string[] + : [] + for (const key of prevManagedKeys) { + if (!incomingTopKeys.includes(key) && key in existing) { + delete existing[key] + } + } + const merged = { ...existing, ...incomingRest } as QwenExtensionConfig & Record if (Object.keys(mergedMcp).length > 0) { @@ -94,9 +109,9 @@ async function mergeQwenConfig( delete merged.mcpServers } - // Always write the tracking key (even as []) so the legacy fallback - // doesn't treat user servers as plugin-managed on a future install. + // Always write tracking keys (even as []) so future installs know what to prune. merged[MANAGED_KEY] = Object.keys(incomingMcp) + merged[MANAGED_KEYS_KEY] = incomingTopKeys return merged as QwenExtensionConfig } diff --git a/tests/qwen-writer.test.ts b/tests/qwen-writer.test.ts index 5c0eeb30b..a255f4f69 100644 --- a/tests/qwen-writer.test.ts +++ b/tests/qwen-writer.test.ts @@ -142,4 +142,38 @@ describe("writeQwenBundle", () => { expect(result.mcpServers.new_plugin).toBeDefined() expect(result.mcpServers.plugin).toBeUndefined() }) + + test("prunes stale top-level plugin keys when incoming config drops them", async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "qwen-stale-keys-")) + + // First install with settings + const bundleWithSettings: QwenBundle = { + config: { + name: "test-plugin", + version: "1.0.0", + commands: "commands", + skills: "skills", + agents: "agents", + settings: [{ name: "api-key", description: "API key", envVar: "API_KEY", sensitive: true }], + }, + agents: [], + commandFiles: [], + skillDirs: [], + } + await writeQwenBundle(tempRoot, bundleWithSettings) + + // User adds their own top-level key + const configPath = path.join(tempRoot, "qwen-extension.json") + const afterInstall = JSON.parse(await fs.readFile(configPath, "utf8")) + afterInstall.userCustom = "should-survive" + await fs.writeFile(configPath, JSON.stringify(afterInstall)) + + // Second install without settings + await writeQwenBundle(tempRoot, makeBundle()) + + const result = JSON.parse(await fs.readFile(configPath, "utf8")) + expect(result.settings).toBeUndefined() + expect(result.userCustom).toBe("should-survive") + expect(result.name).toBe("test-plugin") + }) }) From 25ebe0e860bfb3ccf151edb051be295f02c88551 Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 11:25:48 -0700 Subject: [PATCH 5/5] test(converters): verify _compound_managed_keys tracking in Qwen preservation test Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/qwen-writer.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qwen-writer.test.ts b/tests/qwen-writer.test.ts index a255f4f69..53f861a1a 100644 --- a/tests/qwen-writer.test.ts +++ b/tests/qwen-writer.test.ts @@ -65,6 +65,9 @@ describe("writeQwenBundle", () => { const result = JSON.parse(await fs.readFile(path.join(tempRoot, "qwen-extension.json"), "utf8")) expect(result.customField).toBe("should-survive") + // Tracking key should be written so future installs can prune stale plugin keys + expect(result._compound_managed_keys).toBeInstanceOf(Array) + expect(result._compound_managed_keys).not.toContain("customField") }) test("prunes stale servers from legacy config without tracking key", async () => {