From c65a698d932d02e5fb4a948db4d000e21ed6ba4f Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Wed, 1 Apr 2026 11:46:57 -0700 Subject: [PATCH] fix(converters): preserve user config when writing MCP servers (#479) Co-authored-by: Claude Opus 4.6 (1M context) --- src/sync/codex.ts | 65 ++------- src/targets/codex.ts | 79 +++++++++- src/targets/copilot.ts | 62 +++++++- src/targets/qwen.ts | 77 +++++++++- tests/codex-writer.test.ts | 271 ++++++++++++++++++++++++++++++++++- tests/copilot-writer.test.ts | 168 ++++++++++++++++++++++ tests/qwen-writer.test.ts | 182 +++++++++++++++++++++++ tests/sync-codex.test.ts | 29 +++- 8 files changed, 862 insertions(+), 71 deletions(-) create mode 100644 tests/qwen-writer.test.ts diff --git a/src/sync/codex.ts b/src/sync/codex.ts index b7b894e..bf0cc81 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 + // 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 } - - // 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 - } - } - - 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) + } + const mcpToml = renderCodexConfig(config.mcpServers) + const merged = mergeCodexConfig(existingContent, mcpToml) + if (merged !== null) { + await writeTextSecure(configPath, merged) } } - -function escapeForRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") -} diff --git a/src/targets/codex.ts b/src/targets/codex.ts index 25c6780..a0277da 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 ca1a303..bb678f1 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,67 @@ 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 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[] + : incomingKeys.length > 0 ? Object.keys(existingMcp) : [] + for (const name of prevManaged) { + if (!(name in incoming)) { + delete existingMcp[name] + } + } + + const mergedMcp = { ...existingMcp, ...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 + } + + // Always write tracking key (even as []) to prevent legacy fallback on future installs + return { + ...existing, + mcpServers: mergedMcp, + [MANAGED_KEY]: incomingKeys, } } diff --git a/src/targets/qwen.ts b/src/targets/qwen.ts index 7a4e9c1..0694efc 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,76 @@ 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, + 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 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] + } + } + + 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) { + merged.mcpServers = mergedMcp as QwenExtensionConfig["mcpServers"] + } else { + delete merged.mcpServers + } + + // 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 +} + function resolveQwenPaths(outputRoot: string) { return { root: outputRoot, diff --git a/tests/codex-writer.test.ts b/tests/codex-writer.test.ts index 6e58707..eb3b690 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]") + 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 exist with original content + // 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 d87a45f..aaee54c 100644 --- a/tests/copilot-writer.test.ts +++ b/tests/copilot-writer.test.ts @@ -203,6 +203,174 @@ 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).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 () => { + 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 0000000..53f861a --- /dev/null +++ b/tests/qwen-writer.test.ts @@ -0,0 +1,182 @@ +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") + // 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 () => { + 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("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-")) + + 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).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() + }) + + 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") + }) +}) diff --git a/tests/sync-codex.test.ts b/tests/sync-codex.test.ts index 9714ba8..cd9157b 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") + }) })