-
Notifications
You must be signed in to change notification settings - Fork 459
fix(catalog): omit bare OpenAI models without openai provider #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After a successful no-OpenAI sync leaves a catalog containing only routed rows, the next 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 | ||
|
|
@@ -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); | ||
|
|
@@ -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] : []), | ||
|
|
@@ -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"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the AGENTS.md reference: AGENTS.md:L93-L95 Useful? React with 👍 / 👎. |
||
| this.name = "NoEnabledOpenAiProviderError"; | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the canonical
openaiprovider 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 asuser-native, contradicting the existing preservation case intests/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 👍 / 👎.