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.

P1 Badge Preserve user-added bare catalog entries

For any configuration with enabled providers but no canonical openai, setting includeNativeOpenAi to false makes this ternary discard every bare catalog row, not just the supported OpenAI-native slugs. That deletes user-added entries such as user-native, whose preservation is an explicit existing invariant in tests/codex-catalog-sync-hardening.test.ts:57-90 and tests/codex-catalog-restore.test.ts:41-62; because sync overwrites the catalog, additions made after the pristine backup can be lost. Keep non-OpenAI bare rows and conditionally filter/backfill only the native OpenAI slug set.

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;

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 Omit native models when all providers are disabled

When every configured provider is disabled—especially an existing canonical openai row—enabledProviders is empty, so this condition sets includeNativeOpenAi to true and sync preserves/backfills bare GPT models. Those entries remain selectable even though routeModel rejects them because openai.disabled === true; distinguish a genuinely empty provider configuration from a configuration with zero enabled providers, and add a focused disabled-OpenAI sync regression.

AGENTS.md reference: AGENTS.md:L112-L114

Useful? React with 👍 / 👎.

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`,

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 Provide a working recovery command for disabled OpenAI

When the canonical openai provider exists but is disabled, this error recommends ocx provider add openai, but handleAdd in src/cli/provider.ts:163-165 exits with Provider "openai" already exists unless --force is supplied. The supported non-destructive recovery shown by the provider CLI is ocx provider edit openai --enabled on, so the error needs to distinguish disabled from missing providers rather than directing affected users to a command that immediately fails.

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