-
Notifications
You must be signed in to change notification settings - Fork 459
fix(catalog): carry bare-OpenAI catalog omit onto dig2-go (#643) #681
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 | ||
| .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; | ||
|
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 every configured provider is disabled—especially an existing canonical 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"); | ||
|
|
||
| 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`, | ||
|
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 canonical 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.
For any configuration with enabled providers but no canonical
openai, settingincludeNativeOpenAito false makes this ternary discard every bare catalog row, not just the supported OpenAI-native slugs. That deletes user-added entries such asuser-native, whose preservation is an explicit existing invariant intests/codex-catalog-sync-hardening.test.ts:57-90andtests/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 👍 / 👎.