Merge pull request #836 from lidge-jun/agent/fix-default-catalog-preservation-814

fix(catalog): preserve on-disk rows on default sync

Author: Ingwannu

src/codex/catalog/sync.ts

@@ -543,6 +543,12 @@ export async function syncCatalogModels(config: OcxConfig): Promise<{
   const catalog = loadCatalogForSync(catalogPath);
   if (!catalog) return { added: 0, path: catalogPath, catalogWritten: false, comboOmissions: [] };
 
+  // The bundled catalog is a reliable native template on the default path, but it is not the
+  // merge source. Preservation must inspect the file that this sync is about to overwrite;
+  // otherwise an empty/partial provider gather cannot see routed or user-native rows on disk.
+  const onDiskCatalog = readCatalog(catalogPath);
+  const catalogModelsForMerge = onDiskCatalog?.models ?? catalog.models ?? [];
+
   const template = findNativeTemplate(catalog);
 
   const comboOmissions: ComboCatalogOmission[] = [];
@@ -585,7 +591,7 @@ export async function syncCatalogModels(config: OcxConfig): Promise<{
   // bare gpt-* rows that hard-404 via NoEnabledOpenAiProviderError. Keep natives when no
   // providers are configured yet (fresh install / catalog bootstrap tests).
   const includeNativeOpenAi = enabledProviders.length === 0 || hasCanonicalOpenai;
-  catalog.models = mergeCatalogEntriesForSync(catalog.models ?? [], goEntries, baseline, featured, wsEnabled, goIds, template, disabledNativeSlugs(config), gatheredProviderNames, multiAgentMode, exactComboSlugs, hasPhysicalComboProvider, includeNativeOpenAi);
+  catalog.models = mergeCatalogEntriesForSync(catalogModelsForMerge, goEntries, baseline, featured, wsEnabled, goIds, template, disabledNativeSlugs(config), gatheredProviderNames, multiAgentMode, exactComboSlugs, hasPhysicalComboProvider, includeNativeOpenAi);
   clampCatalogModelsToCodexSupport(catalog.models);
 
   atomicWriteFile(catalogPath, JSON.stringify(catalog, null, 2) + "\n");

structure/03_catalog-and-subagents.md

@@ -20,6 +20,12 @@
   rather than assuming a single file;
 - invalidates `$CODEX_HOME/models_cache.json` when model visibility changes.
 
+On the default `opencodex-catalog.json` path, sync deliberately uses two catalog sources: Codex's
+bundled catalog supplies a current native entry template, while the actual on-disk catalog supplies
+the rows being merged. This split is required because empty or partial provider discovery must
+preserve routed entries and genuine user-native rows from the file that will be overwritten; a
+bundled catalog never contains those rows.
+
 Codex App model picker visibility comes from this shared catalog, not from patching the App.
 
 Provider live-model lists are cached with a configured TTL (`src/codex/model-cache.ts`). Adding,

tests/codex-catalog-sync-hardening.test.ts

@@ -1,21 +1,49 @@
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
 import { spawnSync } from "node:child_process";
 import { tmpdir } from "node:os";
 import { dirname, join, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
 
 const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url)));
 
-function runScript(codexHome: string, opencodexHome: string, script: string): { stdout: string; status: number; stderr: string } {
+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 },
+    env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome, ...extraEnv },
     encoding: "utf8",
   });
   return { stdout: result.stdout?.trim() ?? "", stderr: result.stderr ?? "", status: result.status ?? 1 };
 }
 
+function createCodexCatalogFixture(dir: string): string {
+  const scriptPath = join(dir, "codex-catalog-fixture.js");
+  const bundled = JSON.stringify({ models: [nativeEntry("gpt-5.5", 0)] });
+  writeFileSync(scriptPath, [
+    'if (process.argv.includes("--version")) {',
+    '  console.log("codex-cli 0.999.0");',
+    '} else {',
+    `  process.stdout.write(${JSON.stringify(bundled)});`,
+    '}',
+  ].join("\n"), "utf8");
+
+  if (process.platform === "win32") {
+    const commandPath = join(dir, "codex-catalog-fixture.cmd");
+    writeFileSync(commandPath, `@echo off\r\n"${process.execPath}" "${scriptPath}" %*\r\n`, "utf8");
+    return commandPath;
+  }
+
+  const commandPath = join(dir, "codex-catalog-fixture");
+  writeFileSync(commandPath, `#!/bin/sh\nexec "${process.execPath}" "${scriptPath}" "$@"\n`, "utf8");
+  chmodSync(commandPath, 0o755);
+  return commandPath;
+}
+
 function nativeEntry(slug: string, priority: number): Record<string, unknown> {
   return {
     slug,
@@ -118,6 +146,35 @@ describe("Codex catalog sync hardening", () => {
     expect(slugs).toContain("gpt-5.5");
   });
 
+  test("default catalog path merges from disk instead of replacing it with bundled rows", () => {
+    const catalogPath = join(codexHome, "opencodex-catalog.json");
+    writeFileSync(join(codexHome, "config.toml"), 'openai_base_url = "http://127.0.0.1:10100/v1"\n', "utf8");
+    writeFileSync(catalogPath, JSON.stringify({
+      models: [
+        nativeEntry("gpt-5.5", 0),
+        nativeEntry("user-native", 4),
+        routedEntry("kiro/claude-opus-4.8", 5),
+        routedEntry("opencode-go/glm-5.2", 6),
+      ],
+    }, null, 2) + "\n");
+
+    // Force the default-path bundled shortcut to succeed. The fixture intentionally returns only
+    // a native row so this test fails if sync uses the bundled catalog as its merge input.
+    const codexCliPath = createCodexCatalogFixture(opencodexHome);
+    const r = runScript(codexHome, opencodexHome, `
+      const { syncCatalogModels } = require("./src/codex/catalog");
+      syncCatalogModels({ providers: {} }).then(res => console.log(JSON.stringify(res)));
+    `, { CODEX_CLI_PATH: codexCliPath });
+    expect(r.status).toBe(0);
+    expect(r.stderr).toContain("routed model fetch returned empty; preserving 2 existing routed entries");
+
+    const slugs = (JSON.parse(readFileSync(catalogPath, "utf8")).models as Array<{ slug: string }>).map(m => m.slug);
+    expect(slugs).toContain("gpt-5.5");
+    expect(slugs).toContain("user-native");
+    expect(slugs).toContain("kiro/claude-opus-4.8");
+    expect(slugs).toContain("opencode-go/glm-5.2");
+  });
+
   test("empty routed refresh drops compatibility-excluded rows while preserving other routed entries", () => {
     const catalogPath = join(codexHome, "catalog.json");
     writeFileSync(join(codexHome, "config.toml"), 'model_catalog_json = "catalog.json"\n', "utf8");