Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/codex/catalog/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,11 @@ export function mergeCatalogEntriesForSync(
multiAgentMode: MultiAgentMode = "default",
exactComboSlugs: ReadonlySet<string> = new Set(),
hasPhysicalComboProvider = false,
includeNativeOpenAi = true,
): RawEntry[] {
const rank = new Map(featured.map((slug, i) => [slug, i] as const));
const native = catalogModels
const native = includeNativeOpenAi
? catalogModels
Comment on lines +335 to +336

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve non-OpenAI bare catalog entries

When the canonical openai provider is absent or disabled, this ternary discards the entire bare-entry collection, not just OpenAI-native rows. Consequently, a sync permanently removes user-added bare entries such as user-native, contradicting the existing preservation case in tests/codex-catalog-sync-hardening.test.ts; retain non-OpenAI bare entries while conditionally filtering only OpenAI-family natives, and add a focused regression case for the provider-absent path.

AGENTS.md reference: AGENTS.md:L93-L95

Useful? React with 👍 / 👎.

Comment on lines +335 to +336

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the current routed catalog authoritative after native removal

After a successful no-OpenAI sync leaves a catalog containing only routed rows, the next loadCatalogForSync call rejects that current file because it has no native template and loads the pristine backup instead. If that next provider fetch is transiently empty, mergeCatalogEntriesForSync therefore preserves the backup's stale routed rows rather than the latest successfully discovered rows, defeating the existing empty-fetch preservation behavior and making recently added provider models disappear. Preserve the current routed catalog while sourcing only the template from backup, and cover a successful-sync-then-empty-sync sequence.

AGENTS.md reference: AGENTS.md:L93-L95

Useful? React with 👍 / 👎.

.filter(m => typeof m.slug === "string"
&& !(m.slug as string).includes("/")
&& m.owned_by !== COMBO_NAMESPACE
Expand Down Expand Up @@ -367,11 +369,14 @@ export function mergeCatalogEntriesForSync(
// for subagent max spawns; wire-clamped to the model's real top rung).
if (!isGpt56NativeSlug(slug)) ensureUltraReasoningLevel(preserved);
return preserved;
});
})
: [];

// Backfill any native OpenAI slug that the on-disk catalog is missing (e.g. gpt-5.5), so a
// routed provider exposing the same id can never delete the native OpenAI/Codex base row.
// Skip when no enabled canonical openai provider exists (#636) — bare gpt-* would 404.
const nativeSlugs = new Set(native.flatMap(m => typeof m.slug === "string" ? [m.slug] : []));
if (includeNativeOpenAi) {
for (const slug of nativeOpenAiSlugs()) {
if (nativeSlugs.has(slug)) continue;
nativeSlugs.add(slug);
Expand All @@ -382,6 +387,7 @@ export function mergeCatalogEntriesForSync(
: 9;
native.push(deriveEntry(template ? JSON.parse(JSON.stringify(template)) : null, slug, "OpenAI native model (Codex OAuth passthrough).", priority));
}
}

const freshSlugs = new Set(
routedEntries.flatMap(entry => typeof entry.slug === "string" ? [entry.slug] : []),
Expand Down Expand Up @@ -497,7 +503,16 @@ export async function syncCatalogModels(config: OcxConfig): Promise<{
// native AND routed so the advertised flag matches the implemented endpoint (phase 120.4) and a
// native template can never leak supports_websockets while the flag is off.
const wsEnabled = websocketsEnabled(config);
catalog.models = mergeCatalogEntriesForSync(catalog.models ?? [], goEntries, baseline, featured, wsEnabled, goIds, template, disabledNativeSlugs(config), gatheredProviderNames, multiAgentMode, exactComboSlugs, hasPhysicalComboProvider);
const enabledProviders = Object.entries(config.providers ?? {})
.filter(([, prov]) => prov.disabled !== true);
const hasCanonicalOpenai = enabledProviders.some(([name, prov]) =>
name === "openai" && isCanonicalOpenAiForwardProvider(prov),
);
// #636: when the user only configured non-OpenAI providers (e.g. kimi), do not advertise
// 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);
clampCatalogModelsToCodexSupport(catalog.models);

atomicWriteFile(catalogPath, JSON.stringify(catalog, null, 2) + "\n");
Expand Down
5 changes: 4 additions & 1 deletion src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ function activeProviderEntries(config: OcxConfig): [string, OcxProviderConfig][]

export class NoEnabledOpenAiProviderError extends Error {
constructor(modelId: string) {
super(`No enabled OpenAI provider for model: ${modelId}. Run 'ocx init' to configure a provider, or check that your config has an enabled 'openai' provider.`);
super(
`Model ${modelId} requires the canonical openai provider. `
+ `Run: ocx provider add openai && ocx sync && ocx restart`,
);
Comment on lines +296 to +299

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Give disabled providers a working recovery command

When the openai row exists with disabled: true, the new instruction immediately fails: handleAdd rejects an existing provider unless --force is supplied (src/cli/provider.ts:156-158), while the CLI's documented non-destructive enable operation is ocx provider edit openai --enabled on (src/cli/provider-runtime.ts:15-20,42-52). Emit the enable command for this branch instead of always recommending provider add, and cover the disabled-provider message separately from the absent-provider case.

AGENTS.md reference: AGENTS.md:L93-L95

Useful? React with 👍 / 👎.

this.name = "NoEnabledOpenAiProviderError";
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ describe("routeModel registry effort defaults", () => {
expect(routeModel({ ...base, providers: { ...base.providers, openai: { ...forward, codexAccountMode: "direct" } } }, "gpt-5.5"))
.toMatchObject({ providerName: "openai", codexAccountMode: "direct" });
expect(() => routeModel({ ...base, providers: { ...base.providers, openai: { ...forward, disabled: true } } }, "gpt-5.5"))
.toThrow(NoEnabledOpenAiProviderError);
.toThrow(/requires the canonical openai provider/);
const unavailable = { ...base, providers: { "openai-proxy": base.providers["openai-proxy"] } };
expect(() => routeModel(unavailable, "gpt-5.5")).toThrow(NoEnabledOpenAiProviderError);
expect(() => routeModel(unavailable, "gpt-5.5")).toThrow(/ocx provider add openai/);
});

test("rejects legacy chatgpt namespaces even when configured", () => {
Expand Down
Loading