diff --git a/src/codex/catalog/sync.ts b/src/codex/catalog/sync.ts index 4127c5d61..877c18471 100644 --- a/src/codex/catalog/sync.ts +++ b/src/codex/catalog/sync.ts @@ -457,11 +457,12 @@ export function mergeCatalogEntriesForSync( export async function syncCatalogModels(config: OcxConfig): Promise<{ added: number; path: string; + catalogWritten: boolean; comboOmissions: ComboCatalogOmission[]; }> { const catalogPath = readCodexCatalogPath(); const catalog = loadCatalogForSync(catalogPath); - if (!catalog) return { added: 0, path: catalogPath, comboOmissions: [] }; + if (!catalog) return { added: 0, path: catalogPath, catalogWritten: false, comboOmissions: [] }; const template = findNativeTemplate(catalog); @@ -500,7 +501,7 @@ export async function syncCatalogModels(config: OcxConfig): Promise<{ clampCatalogModelsToCodexSupport(catalog.models); atomicWriteFile(catalogPath, JSON.stringify(catalog, null, 2) + "\n"); - return { added: goEntries.length, path: catalogPath, comboOmissions }; + return { added: goEntries.length, path: catalogPath, catalogWritten: true, comboOmissions }; } export function restoreCodexCatalog(): { removed: number; kept: number; path: string } { @@ -531,10 +532,11 @@ export function restoreCodexCatalog(): { removed: number; kept: number; path: st return { removed, kept: native.length, path: catalogPath }; } -export function invalidateCodexModelsCache(): void { +/** Force Codex's models_cache stale from the on-disk catalog. Returns whether a cache write occurred. */ +export function invalidateCodexModelsCache(): boolean { try { const catalogPath = readCodexCatalogPath(); - if (!existsSync(catalogPath)) return; + if (!existsSync(catalogPath)) return false; const catalog = JSON.parse(readFileSync(catalogPath, "utf8")); const models = catalog.models ?? catalog; const wrapper = { @@ -543,5 +545,8 @@ export function invalidateCodexModelsCache(): void { models, }; atomicWriteFile(activeCodexModelsCachePath(), JSON.stringify(wrapper, null, 2) + "\n"); - } catch { /* best-effort */ } + return true; + } catch { + return false; + } } diff --git a/src/codex/refresh.ts b/src/codex/refresh.ts index 7036c99e4..1cb582e8e 100644 --- a/src/codex/refresh.ts +++ b/src/codex/refresh.ts @@ -9,6 +9,7 @@ export interface CodexCatalogRefreshResult { added: number; path: string; catalogExists: boolean; + catalogWritten: boolean; cacheSynced: boolean; comboOmissions: ComboCatalogOmission[]; } @@ -42,8 +43,11 @@ export async function refreshCodexModelCatalog( ): Promise { const result = await deps.syncCatalogModels(config); const catalogExists = deps.existsSync(result.path); + const catalogWritten = result.catalogWritten === true; const comboOmissions = result.comboOmissions ?? []; - if (!catalogExists) return { ...result, catalogExists, cacheSynced: false, comboOmissions }; - deps.invalidateCodexModelsCache(); - return { ...result, catalogExists, cacheSynced: true, comboOmissions }; + if (!catalogExists) { + return { ...result, catalogExists, catalogWritten: false, cacheSynced: false, comboOmissions }; + } + const cacheSynced = deps.invalidateCodexModelsCache(); + return { ...result, catalogExists, catalogWritten, cacheSynced, comboOmissions }; } diff --git a/src/codex/sync.ts b/src/codex/sync.ts index d60aa3215..27040f92d 100644 --- a/src/codex/sync.ts +++ b/src/codex/sync.ts @@ -11,6 +11,7 @@ export interface CodexSyncResult { added: number; catalogPath: string | null; catalogExists: boolean; + catalogWritten: boolean; cacheSynced: boolean; message: string; warning?: string; @@ -61,6 +62,7 @@ export async function syncModelsToCodex( added: 0, catalogPath: null, catalogExists: false, + catalogWritten: false, cacheSynced: false, message: result.message, }; @@ -71,6 +73,7 @@ export async function syncModelsToCodex( let catalogPath: string | null = null; let catalogPathForInjection: string | null | undefined; let catalogExists = false; + let catalogWritten = false; let cacheSynced = false; let warning: string | undefined; let comboOmissions: ComboCatalogOmission[] = []; @@ -79,6 +82,7 @@ export async function syncModelsToCodex( const cat = await deps.refreshCodexModelCatalog(config); added = cat.added; catalogExists = cat.catalogExists; + catalogWritten = cat.catalogWritten; cacheSynced = cat.cacheSynced; catalogPathForInjection = cat.catalogExists ? cat.path : null; catalogPath = catalogPathForInjection; @@ -110,6 +114,7 @@ export async function syncModelsToCodex( added, catalogPath, catalogExists, + catalogWritten, cacheSynced, message: result.message, ...(warning ? { warning } : {}), diff --git a/tests/codex-refresh.test.ts b/tests/codex-refresh.test.ts index 057b7d118..a234b7a40 100644 --- a/tests/codex-refresh.test.ts +++ b/tests/codex-refresh.test.ts @@ -1,4 +1,8 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { invalidateCodexModelsCache, syncCatalogModels } from "../src/codex/catalog"; import { refreshCodexModelCatalog } from "../src/codex/refresh"; import type { OcxConfig } from "../src/types"; @@ -8,12 +12,65 @@ const config = { providers: {}, } as OcxConfig; +const tempHomes: string[] = []; + +function installTempHomes(): { codexHome: string; opencodexHome: string; restore(): void } { + const previousCodexHome = process.env.CODEX_HOME; + const previousOpenCodexHome = process.env.OPENCODEX_HOME; + const codexHome = mkdtempSync(join(tmpdir(), "ocx-refresh-codex-")); + const opencodexHome = mkdtempSync(join(tmpdir(), "ocx-refresh-ocx-")); + tempHomes.push(codexHome, opencodexHome); + process.env.CODEX_HOME = codexHome; + process.env.OPENCODEX_HOME = opencodexHome; + + return { + codexHome, + opencodexHome, + restore() { + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; + if (previousOpenCodexHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOpenCodexHome; + rmSync(codexHome, { recursive: true, force: true }); + rmSync(opencodexHome, { recursive: true, force: true }); + }, + }; +} + +function nativeCatalogFixture(slug = "gpt-5.5"): string { + return JSON.stringify({ + models: [{ + slug, + display_name: slug, + description: "native", + priority: 9, + visibility: "list", + base_instructions: "You are Codex, a coding agent based on GPT-5.", + supported_reasoning_levels: [{ effort: "medium", description: "m" }], + }], + }, null, 2) + "\n"; +} + +afterEach(() => { + for (const path of tempHomes.splice(0)) { + rmSync(path, { recursive: true, force: true }); + } +}); + describe("Codex catalog refresh", () => { test("writes an expired Codex models cache whenever the materialized catalog exists", async () => { let invalidated = 0; const result = await refreshCodexModelCatalog(config, { - syncCatalogModels: async () => ({ added: 0, path: "/tmp/opencodex-catalog.json", comboOmissions: [] }), - invalidateCodexModelsCache: () => { invalidated += 1; }, + syncCatalogModels: async () => ({ + added: 0, + path: "/tmp/opencodex-catalog.json", + catalogWritten: true, + comboOmissions: [], + }), + invalidateCodexModelsCache: () => { + invalidated += 1; + return true; + }, existsSync: () => true, }); @@ -21,6 +78,7 @@ describe("Codex catalog refresh", () => { added: 0, path: "/tmp/opencodex-catalog.json", catalogExists: true, + catalogWritten: true, cacheSynced: true, comboOmissions: [], }); @@ -30,14 +88,130 @@ describe("Codex catalog refresh", () => { test("does not touch the cache when no Codex catalog can be materialized", async () => { let invalidated = 0; const result = await refreshCodexModelCatalog(config, { - syncCatalogModels: async () => ({ added: 0, path: "/tmp/missing-catalog.json", comboOmissions: [] }), - invalidateCodexModelsCache: () => { invalidated += 1; }, + syncCatalogModels: async () => ({ + added: 0, + path: "/tmp/missing-catalog.json", + catalogWritten: false, + comboOmissions: [], + }), + invalidateCodexModelsCache: () => { + invalidated += 1; + return true; + }, existsSync: () => false, }); expect(result.catalogExists).toBe(false); + expect(result.catalogWritten).toBe(false); expect(result.cacheSynced).toBe(false); expect(result.comboOmissions).toEqual([]); expect(invalidated).toBe(0); }); + + test("reports cacheSynced false when invalidate cannot write", async () => { + const result = await refreshCodexModelCatalog(config, { + syncCatalogModels: async () => ({ + added: 0, + path: "/tmp/opencodex-catalog.json", + catalogWritten: true, + comboOmissions: [], + }), + invalidateCodexModelsCache: () => false, + existsSync: () => true, + }); + + expect(result.catalogExists).toBe(true); + expect(result.catalogWritten).toBe(true); + expect(result.cacheSynced).toBe(false); + expect(result.comboOmissions).toEqual([]); + }); + + test("preserves catalogWritten false when the catalog path exists but sync did not write", async () => { + const result = await refreshCodexModelCatalog(config, { + syncCatalogModels: async () => ({ + added: 0, + path: "/tmp/broken-catalog.json", + catalogWritten: false, + comboOmissions: [], + }), + invalidateCodexModelsCache: () => false, + existsSync: () => true, + }); + + expect(result.catalogExists).toBe(true); + expect(result.catalogWritten).toBe(false); + expect(result.cacheSynced).toBe(false); + }); + + test("reports catalogWritten true after syncCatalogModels rewrites a real catalog file", async () => { + const home = installTempHomes(); + try { + const catalogPath = join(home.codexHome, "nested", "catalog.json"); + mkdirSync(join(home.codexHome, "nested"), { recursive: true }); + writeFileSync(join(home.codexHome, "config.toml"), 'model_catalog_json = "nested/catalog.json"\n', "utf8"); + writeFileSync(catalogPath, nativeCatalogFixture("gpt-5.6-sol"), "utf8"); + const before = readFileSync(catalogPath, "utf8"); + + const result = await syncCatalogModels(config); + const after = readFileSync(catalogPath, "utf8"); + const rewritten = JSON.parse(after); + + expect(result.path).toBe(join(realpathSync.native(home.codexHome), "nested", "catalog.json")); + expect(result.catalogWritten).toBe(true); + expect(after).not.toBe(before); + expect(rewritten.models[0].slug).toBe("gpt-5.6-sol"); + expect(rewritten.models[0].display_name).toBe("GPT-5.6-Sol"); + expect(rewritten.models[0].context_window).toBeGreaterThan(0); + } finally { + home.restore(); + } + }); + + test("invalidateCodexModelsCache reports real cache write success and failure cases", () => { + const success = installTempHomes(); + try { + writeFileSync(join(success.codexHome, "config.toml"), 'model_catalog_json = "opencodex-catalog.json"\n', "utf8"); + writeFileSync(join(success.codexHome, "opencodex-catalog.json"), nativeCatalogFixture("gpt-5.6-sol"), "utf8"); + + expect(invalidateCodexModelsCache()).toBe(true); + const cache = JSON.parse(readFileSync(join(success.codexHome, "models_cache.json"), "utf8")); + expect(cache.fetched_at).toBe("2000-01-01T00:00:00Z"); + expect(cache.client_version).toBe("0.0.0"); + expect(cache.models[0].slug).toBe("gpt-5.6-sol"); + } finally { + success.restore(); + } + + const missingCatalog = installTempHomes(); + try { + writeFileSync(join(missingCatalog.codexHome, "config.toml"), 'model_catalog_json = "missing-catalog.json"\n', "utf8"); + + expect(invalidateCodexModelsCache()).toBe(false); + expect(existsSync(join(missingCatalog.codexHome, "models_cache.json"))).toBe(false); + } finally { + missingCatalog.restore(); + } + + const malformedCatalog = installTempHomes(); + try { + writeFileSync(join(malformedCatalog.codexHome, "config.toml"), 'model_catalog_json = "opencodex-catalog.json"\n', "utf8"); + writeFileSync(join(malformedCatalog.codexHome, "opencodex-catalog.json"), "{not-json", "utf8"); + + expect(invalidateCodexModelsCache()).toBe(false); + expect(existsSync(join(malformedCatalog.codexHome, "models_cache.json"))).toBe(false); + } finally { + malformedCatalog.restore(); + } + + const unwritableCache = installTempHomes(); + try { + writeFileSync(join(unwritableCache.codexHome, "config.toml"), 'model_catalog_json = "opencodex-catalog.json"\n', "utf8"); + writeFileSync(join(unwritableCache.codexHome, "opencodex-catalog.json"), nativeCatalogFixture(), "utf8"); + mkdirSync(join(unwritableCache.codexHome, "models_cache.json")); + + expect(invalidateCodexModelsCache()).toBe(false); + } finally { + unwritableCache.restore(); + } + }); }); diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index 1e8d8aed0..31d23ff4b 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -53,6 +53,7 @@ describe("GUI/CLI Codex sync backend", () => { added: 3, path: "/tmp/opencodex-catalog.json", catalogExists: true, + catalogWritten: true, cacheSynced: true, comboOmissions: [], }), @@ -72,6 +73,7 @@ describe("GUI/CLI Codex sync backend", () => { added: 3, catalogPath: "/tmp/opencodex-catalog.json", catalogExists: true, + catalogWritten: true, cacheSynced: true, message: "injected", }); @@ -93,6 +95,7 @@ describe("GUI/CLI Codex sync backend", () => { added: 1, path: "/tmp/opencodex-catalog.json", catalogExists: true, + catalogWritten: true, cacheSynced: true, comboOmissions: [omission], }), @@ -121,6 +124,7 @@ describe("GUI/CLI Codex sync backend", () => { added: 0, path: "/tmp/opencodex-catalog.json", catalogExists: true, + catalogWritten: true, cacheSynced: true, comboOmissions: [omission], }), @@ -192,6 +196,7 @@ describe("GUI/CLI Codex sync backend", () => { added: 0, catalogPath: null, catalogExists: false, + catalogWritten: false, cacheSynced: false, message: "external provider preserved", }); diff --git a/tests/injection-model-api.test.ts b/tests/injection-model-api.test.ts index b3bc8713d..3a555b3d2 100644 --- a/tests/injection-model-api.test.ts +++ b/tests/injection-model-api.test.ts @@ -249,9 +249,9 @@ describe("/api/injection-model guidance kill switch + partial update", () => { await refreshCodexModelCatalog(config, { syncCatalogModels: async syncedConfig => { flagSeenBySync = syncedConfig.multiAgentGuidanceEnabled; - return { added: 0, path: join(tempHome!, "missing-catalog.json") }; + return { added: 0, path: join(tempHome!, "missing-catalog.json"), catalogWritten: false, comboOmissions: [] }; }, - invalidateCodexModelsCache: () => {}, + invalidateCodexModelsCache: () => false, existsSync: () => false, }); expect(flagSeenBySync).toBe(false);