fix(catalog): write every slug exactly once from retained sync (#4730)

src/codex/catalog/retained-sync.ts

@@ -51,7 +51,7 @@ import { bundledCatalogCacheState, loadBundledCodexCatalog } from "./bundled";
 import { isMultiAgentV2Enabled } from "../features";
 import { clampCatalogModelsToCodexSupport } from "./effort";
 import { filterCatalogVisibleModels, gatherRoutedModels, type CatalogGatherProviderModelOutcome } from "./provider-fetch";
-import { exactComboCatalogSlugs, type ComboCatalogOmission } from "./aggregation";
+import { exactComboCatalogSlugs, safeCatalogWarningLabel, type ComboCatalogOmission } from "./aggregation";
 import {
   withCatalogWriteSerialization,
   type CatalogWritePermit,
@@ -520,6 +520,30 @@ function writeRetainedCatalogSync({
       warningPolicy: "emit",
     },
   });
+  const dedupedCatalogModels = dedupeCatalogEntriesBySlug(catalog.models);
+  if (dedupedCatalogModels.length !== catalog.models.length) {
+    // A dropped row that differs from the kept one means two emit paths disagree about
+    // the same slug's content. First-win still stands (the merge ranked the kept row),
+    // but the operator needs to see WHICH slugs diverged instead of silently losing data.
+    const keptBySlug = new Map(catalog.models.flatMap(entry => (
+      typeof entry.slug === "string" ? [[entry.slug, entry] as const] : []
+    )));
+    const divergentSlugs = new Set<string>();
+    for (const entry of catalog.models) {
+      if (typeof entry.slug !== "string") continue;
+      const kept = keptBySlug.get(entry.slug);
+      if (kept && kept !== entry && JSON.stringify(kept) !== JSON.stringify(entry)) {
+        divergentSlugs.add(entry.slug);
+      }
+    }
+    const divergentNote = divergentSlugs.size > 0
+      ? `; divergent content on: ${[...divergentSlugs].slice(0, 5).map(safeCatalogWarningLabel).join(", ")}${divergentSlugs.size > 5 ? ", …" : ""}`
+      : "";
+    console.warn(
+      `[opencodex] catalog sync dropped ${catalog.models.length - dedupedCatalogModels.length} duplicate slug row(s), keeping the first occurrence of each slug (#4730)${divergentNote}.`,
+    );
+    catalog.models = dedupedCatalogModels;
+  }
   clampCatalogModelsToCodexSupport(catalog.models);
   finalizeAutoReviewModelOverride(catalog.models, catalogModelsForMerge, config);
 
@@ -553,6 +577,30 @@ function writeRetainedCatalogSync({
   };
 }
 
+/**
+ * Final guard for the written catalog: every slug must appear exactly once (#4730).
+ *
+ * Two emit paths can hand the merge the same model under the same Codex-facing slug and the
+ * equivalence-key merge keeps both (a slash-less `model.alias` slug is an "exact" key, so the
+ * aliased and canonical rows of one provider model never collapse). Observed on 2.56.0: a sync
+ * produced 507 rows for 72 unique slugs, every duplicate byte-identical. Keep the FIRST
+ * occurrence — the merge already ranked it — and never touch distinct slugs.
+ */
+export function dedupeCatalogEntriesBySlug(models: RawEntry[]): RawEntry[] {
+  const seen = new Set<string>();
+  const out: RawEntry[] = [];
+  for (const entry of models) {
+    if (typeof entry.slug !== "string") {
+      out.push(entry);
+      continue;
+    }
+    if (seen.has(entry.slug)) continue;
+    seen.add(entry.slug);
+    out.push(entry);
+  }
+  return out;
+}
+
 export async function syncCatalogModels(
   config: OcxConfig,
   options?: CodexCatalogSyncOptions,

tests/codex-integration/catalog-duplicate-slug-dedup.test.ts

@@ -0,0 +1,56 @@
+import { describe, expect, test } from "bun:test";
+import { dedupeCatalogEntriesBySlug } from "../../src/codex/catalog/retained-sync";
+import type { RawEntry } from "../../src/codex/catalog/parsing";
+
+/**
+ * #4730: one sync on 2.56.0 wrote 507 catalog rows for 72 unique slugs — the aliased
+ * (`CC-x`) and canonical (`command-code/x`) emit paths of the same provider model both
+ * survived the equivalence-key merge as byte-identical rows. The written catalog must
+ * carry every slug exactly once, and the guard must be inert for catalogs that are
+ * already unique.
+ */
+
+const row = (slug: string, display?: string): RawEntry => ({
+  slug,
+  ...(display ? { display_name: display } : {}),
+} as RawEntry);
+
+describe("dedupeCatalogEntriesBySlug", () => {
+  test("keeps the first occurrence and drops later byte-identical rows", () => {
+    const models = [row("CC-MiniMaxAI-MiniMax-M3", "first"), row("CC-MiniMaxAI-MiniMax-M3", "first"), row("CC-MiniMaxAI-MiniMax-M3", "first")];
+    const out = dedupeCatalogEntriesBySlug(models);
+    expect(out).toHaveLength(1);
+    expect(out[0]).toBe(models[0]);
+  });
+
+  test("never drops distinct slugs, including alias/canonical pairs", () => {
+    const models = [
+      row("CC-MiniMaxAI-MiniMax-M3"),
+      row("command-code/MiniMaxAI-MiniMax-M3"),
+      row("gpt-5.6-luna"),
+    ];
+    expect(dedupeCatalogEntriesBySlug(models)).toHaveLength(3);
+  });
+
+  test("preserves row order", () => {
+    const models = [row("b"), row("a"), row("b"), row("c"), row("a")];
+    expect(dedupeCatalogEntriesBySlug(models).map(entry => entry.slug)).toEqual(["b", "a", "c"]);
+  });
+
+  test("passes through rows without a string slug untouched", () => {
+    const odd = { display_name: "no slug" } as unknown as RawEntry;
+    const models = [odd, row("x"), odd];
+    const out = dedupeCatalogEntriesBySlug(models);
+    expect(out).toHaveLength(3);
+    expect(out[0]).toBe(odd);
+    expect(out[1]).toBe(models[1]);
+    expect(out[2]).toBe(odd);
+  });
+
+  test("is inert for an already-unique catalog", () => {
+    const models = [row("a"), row("b"), row("c")];
+    const out = dedupeCatalogEntriesBySlug(models);
+    expect(out).toHaveLength(3);
+    expect(out[0]).toBe(models[0]);
+  });
+});

tests/codex-integration/catalog-modelalias-unique-sync.test.ts

@@ -0,0 +1,145 @@
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { spawnSync } from "node:child_process";
+import { tmpdir } from "node:os";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+import { removeTreeWithRetry } from "../helpers/remove-tree";
+
+// Integration regression for #4730: whatever the upstream merge emits, the catalog a sync
+// WRITES must carry every slug exactly once, and the guard must not collapse distinct
+// slugs — the aliased (`CC-…`) and canonical (`command-code/…`) rows of one provider model
+// are different public names and both must survive. Runs the real sync twice (idempotence)
+// in an isolated CODEX_HOME/OPENCODEX_HOME with the reporter's config shape: provider
+// `alias: "CC"` plus `modelAliases` mappings.
+
+const repoRoot = dirname(fileURLToPath(new URL("../../package.json", import.meta.url)));
+
+function runScript(codexHome: string, opencodexHome: string, script: string, extraEnv: Record<string, string> = {}): { stdout: string; status: number; stderr: string } {
+  const result = spawnSync(process.execPath, ["--eval", script], {
+    cwd: repoRoot,
+    env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome, ...extraEnv },
+    encoding: "utf8",
+  });
+  const diagnostics = [result.stderr ?? ""];
+  if (result.error) {
+    const code = "code" in result.error ? String(result.error.code) : result.error.name;
+    diagnostics.push(`[spawn error: ${code}] ${result.error.stack ?? result.error.message}`);
+  }
+  if (result.signal) diagnostics.push(`[spawn signal] ${result.signal}`);
+  return { stdout: result.stdout?.trim() ?? "", stderr: diagnostics.filter(Boolean).join("\n"), status: result.status ?? 1 };
+}
+
+function createCodexCatalogFixture(dir: string): string {
+  const scriptPath = join(dir, "codex-catalog-fixture.js");
+  const bundled = JSON.stringify({ models: [{
+    slug: "gpt-5.5", display_name: "gpt-5.5", description: "native", priority: 0,
+    visibility: "list", shell_type: "shell_command", comp_hash: "native-comp-hash",
+    model_messages: { instructions_template: "You are Codex." },
+    base_instructions: "You are Codex, a coding agent based on GPT-5.",
+    supported_reasoning_levels: [{ effort: "medium", description: "m" }],
+  }] });
+  writeFileSync(scriptPath, [
+    'if (process.argv.includes("--version")) {',
+    '  console.log("codex-cli 0.999.0");',
+    '} else {',
+    `  process.stdout.write(${JSON.stringify(bundled)});`,
+    '}',
+  ].join("\n"), "utf8");
+  const commandPath = join(dir, "codex-catalog-fixture");
+  writeFileSync(commandPath, `#!/bin/sh\nexec "${process.execPath}" "${scriptPath}" "$@"\n`, "utf8");
+  return commandPath;
+}
+
+function routedEntry(slug: string, priority: number, display?: string): Record<string, unknown> {
+  return {
+    slug, display_name: display ?? slug, description: "routed", priority,
+    visibility: "list", supported_reasoning_levels: [],
+    base_instructions: "You are Codex, a coding agent based on GPT-5.",
+  };
+}
+
+describe("modelAliases sync writes unique slugs (#4730)", () => {
+  let codexHome: string;
+  let opencodexHome: string;
+
+  beforeEach(() => {
+    codexHome = mkdtempSync(join(tmpdir(), "ocx-alias-home-"));
+    opencodexHome = mkdtempSync(join(tmpdir(), "ocx-alias-ocx-"));
+  });
+
+  afterEach(() => {
+    if (existsSync(codexHome)) removeTreeWithRetry(codexHome);
+    if (existsSync(opencodexHome)) removeTreeWithRetry(opencodexHome);
+  });
+
+  test("real sync dedups duplicate rows and keeps the alias/canonical pair distinct", () => {
+    const catalogPath = join(codexHome, "catalog.json");
+    writeFileSync(join(codexHome, "config.toml"), 'model_catalog_json = "catalog.json"\n');
+    // Baseline carries duplicate rows of the SAME slug (the #4730 symptom) next to the
+    // alias/canonical pair of one model and a native — the pair must NOT be collapsed.
+    writeFileSync(catalogPath, JSON.stringify({ models: [
+      routedEntry("command-code/MiniMaxAI-MiniMax-M3", 5),
+      routedEntry("command-code/MiniMaxAI-MiniMax-M3", 5),
+      routedEntry("CC-MiniMaxAI-MiniMax-M3", 5),
+      routedEntry("CC-MiniMaxAI-MiniMax-M3", 5),
+      routedEntry("command-code/deepseek-deepseek-v4-flash", 6),
+    ] }));
+    const runtime = createCodexCatalogFixture(opencodexHome);
+    const config = {
+      providers: {
+        // The forward surface is what keeps includeNativeOpenAi true; without it the merge
+        // drops every slash-less baseline row before the write guard ever sees them.
+        openai: { adapter: "openai-responses", authMode: "forward", baseUrl: "https://chatgpt.com/backend-api/codex" },
+        "command-code": {
+          adapter: "openai-chat",
+          baseUrl: "https://catalog-fixture.invalid/v1",
+          authMode: "key",
+          apiKey: "fixture-key",
+          liveModels: false,
+          models: ["MiniMaxAI/MiniMax-M3", "deepseek/deepseek-v4-flash"],
+          alias: "CC",
+          modelAliases: {
+            "MiniMaxAI/MiniMax-M3": "CC-MiniMaxAI-MiniMax-M3",
+            "deepseek/deepseek-v4-flash": "CC-deepseek-deepseek-v4-flash",
+          },
+        },
+      },
+    };
+    writeFileSync(join(opencodexHome, "config.json"), JSON.stringify(config));
+    const passesPath = join(opencodexHome, "alias-sync-passes.json");
+    const r = runScript(codexHome, opencodexHome, `
+      const { readFileSync, writeFileSync } = require("node:fs");
+      const { syncCatalogModels } = require("./src/codex/catalog");
+      const config = ${JSON.stringify(config)};
+      const passes = [];
+      for (let pass = 0; pass < 2; pass++) {
+        const result = await syncCatalogModels(config);
+        passes.push({
+          written: result.catalogWritten,
+          catalog: JSON.parse(readFileSync(${JSON.stringify(catalogPath)}, "utf8")).models,
+        });
+      }
+      writeFileSync(${JSON.stringify(passesPath)}, JSON.stringify(passes));
+    `, { CODEX_CLI_PATH: runtime });
+    expect(r.status, r.stderr).toBe(0);
+    const passes = JSON.parse(readFileSync(passesPath, "utf8")) as Array<{
+      written: boolean;
+      catalog: Array<{ slug: string }>;
+    }>;
+    expect(passes).toHaveLength(2);
+    expect(passes[0]!.written).toBe(true);
+    // Slug-level idempotence: the same public names land in the same order every pass. Row
+    // bodies may legitimately differ between passes (native metadata refresh), so equality
+    // is asserted on the slug sequence, not on full rows.
+    expect(passes[1]!.catalog.map(row => row.slug)).toEqual(passes[0]!.catalog.map(row => row.slug));
+    for (const pass of passes) {
+      const slugs = pass.catalog.map(row => row.slug);
+      // The write-path guard: whatever the merge/retention emitted, every slug lands once.
+      expect(new Set(slugs).size).toBe(slugs.length);
+      // Distinct public names of the same provider model both survive, once each.
+      expect(slugs).toContain("CC-MiniMaxAI-MiniMax-M3");
+      expect(slugs).toContain("command-code/MiniMaxAI-MiniMax-M3");
+      expect(slugs).toContain("command-code/deepseek-deepseek-v4-flash");
+    }
+  }, { timeout: 20_000 });
+});