Address PR review feedback (#479)

- Strip unmarked legacy format (# Generated by compound-plugin) during
  merge to prevent duplicate TOML tables on first upgrade from old writer

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

Author: tmchow

src/targets/codex.ts

@@ -10,6 +10,7 @@ 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<void> {
   const codexRoot = resolveCodexRoot(outputRoot)
@@ -119,10 +120,14 @@ export function mergeCodexConfig(existingContent: string, mcpToml: string | null
   }
   stripped = stripped.trimEnd()
 
-  const legacyMarkerIndex = stripped.indexOf(LEGACY_MARKER)
-  const cleaned = legacyMarkerIndex === -1
-    ? stripped
-    : stripped.slice(0, legacyMarkerIndex).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) {

tests/codex-writer.test.ts

@@ -171,6 +171,39 @@ describe("writeCodexBundle", () => {
     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")
@@ -443,6 +476,40 @@ describe("mergeCodexConfig", () => {
     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()
   })