Address PR review feedback (#479)

- Copilot/Qwen: treat all existing MCP servers as plugin-managed when
  _compound_managed_mcp tracking key is absent (legacy migration)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: tmchow

src/targets/copilot.ts

@@ -61,8 +61,12 @@ async function mergeCopilotMcpConfig(
     ? { ...(existing.mcpServers as Record<string, unknown>) }
     : {}
 
-  // Remove previously-managed plugin servers that are no longer in the bundle
-  const prevManaged = Array.isArray(existing[MANAGED_KEY]) ? existing[MANAGED_KEY] as string[] : []
+  // 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]

src/targets/qwen.ts

@@ -68,8 +68,12 @@ async function mergeQwenConfig(
     ? { ...(existing.mcpServers as Record<string, unknown>) }
     : {}
 
-  // Remove previously-managed plugin servers that are no longer in the bundle
-  const prevManaged = Array.isArray(existing[MANAGED_KEY]) ? existing[MANAGED_KEY] as string[] : []
+  // 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)) {

tests/copilot-writer.test.ts

@@ -258,10 +258,13 @@ Run these research agents:
     const githubRoot = path.join(tempRoot, ".github")
     await fs.mkdir(githubRoot, { recursive: true })
 
-    // User has their own MCP server (not managed by the plugin)
+    // 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: ["*"] } } }),
+      JSON.stringify({
+        mcpServers: { "user-tool": { type: "local", command: "my-tool", tools: ["*"] } },
+        _compound_managed_mcp: [],
+      }),
     )
 
     const bundle: CopilotBundle = {
@@ -278,6 +281,38 @@ Run these research agents:
     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 = {

tests/qwen-writer.test.ts

@@ -36,10 +36,14 @@ describe("writeQwenBundle", () => {
   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 (not managed by the plugin)
+    // 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" } } }),
+      JSON.stringify({
+        name: "user-project",
+        mcpServers: { "user-tool": { command: "my-tool" } },
+        _compound_managed_mcp: [],
+      }),
     )
 
     await writeQwenBundle(tempRoot, makeBundle({ plugin: { command: "plugin-server" } }))
@@ -63,6 +67,27 @@ describe("writeQwenBundle", () => {
     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-"))