fix(codex): preserve routed models in desktop picker - #1056
Conversation
📝 WalkthroughWalkthroughAdded opt-in Codex Desktop native-alias combos. The change updates configuration, validation, CLI and GUI handling, catalog construction and synchronization, model visibility, disabled-model migration, tests, and multilingual documentation. ChangesNative alias configuration and client handling
Catalog and management flow
Validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
Review readiness checklistThis PR is kept in draft until every requirement below is fulfilled. The tickable checklist has been added to your PR description — tick all four boxes there.
✅ 4/4 boxes ticked. ✅ All four boxes are ticked. |
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Superseded maintainer note: the current status is recorded in the latest update below.
The original run links and |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/cli/agents.md`:
- Around line 60-62: Document the bare-ID routing boundary in both locations: in
docs-site/src/content/docs/reference/cli/agents.md lines 60-62, state that
--native-alias applies only to the supported unqualified bare model ID and does
not alter account-qualified or provider-qualified OpenAI routes; in
docs-site/src/content/docs/reference/configuration/routing.md lines 55-68,
replace the broad canonical-routing precedence wording with wording limiting
precedence to the bare native ID. Preserve the distinction between bare native
IDs using Codex login routes and qualified IDs remaining separate routes.
In `@gui/src/combo-workspace-data.ts`:
- Around line 297-315: The native-alias checks in validateComboDraft currently
rely on the broad NATIVE_OPENAI_FAMILY_RE and must instead enforce the exact
seven supported native OpenAI slugs from SUPPORTED_NATIVE_OPENAI_SLUGS or its
shared source. Update both alias validation and the item.nativeAlias validation
while preserving namespace and display-name rules, then add a regression case in
combo-workspace-data.test.ts covering an unsupported value such as gpt-unknown.
In `@src/cli/combo.ts`:
- Line 19: Update the CLI usage text near the option declaration to show
--native-alias and --display-name as independent options, and document that
--display-name accepts the "-" value that clears displayName, matching the
parser behavior in the display-name handling flow.
In `@tests/codex-catalog.test.ts`:
- Around line 417-426: Update the config fixture used by
filterCatalogVisibleModels to make providers.combo a valid OcxProviderConfig,
including both adapter and baseUrl. Preserve the disabledModels assertions and
use an existing valid provider fixture or established cast pattern rather than
leaving combo as an empty object.
In `@tests/combo-management-api.test.ts`:
- Around line 545-546: Gate old-model migration on previous?.nativeAlias !==
true so bare native slugs such as "gpt-5.6-sol" are not rewritten; update the
migration logic around oldPublicModel/migratedModels while preserving migration
for non-native aliases. Change the disabledModels expectation to ["gpt-5.6-sol",
"combo/new"] and add assertions covering preservation in subagentModels,
injectionModel, shadowCallIntercept.model, and all claudeCode model fields.
- Around line 685-698: Update the test setup in “GET models exposes one
native-alias row and uses only its canonical disable selector” to set providers
b and c liveModels to false as well as provider a, ensuring every configured
provider is excluded from live discovery and no outbound requests occur.
In `@tests/combo-workspace-data.test.ts`:
- Around line 245-264: Update the native-alias validation in combo workspace
data to return distinct keys for unsupported aliases, missing displayName
values, and invalid displayName values instead of reusing invalidAlias. Add
translations for all new keys in every locale, and update the related validation
expectations in the combo workspace tests.
In `@tests/combos.test.ts`:
- Around line 406-427: Extend the test around the native alias case by
configuring the openai provider and the account namespace main: "`@main`" in
baseConfig. Add an assertion for routeModel(config, "main/gpt-5.6-sol") that
expects providerName "openai", modelId "gpt-5.6-sol", and no combo, ensuring
account-qualified routing occurs before combo selection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec835ccc-fe71-48e0-aef1-cd60ed64e7ca
📒 Files selected for processing (46)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ja/guides/combos.mddocs-site/src/content/docs/ja/reference/configuration/routing.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/combos.mddocs-site/src/content/docs/ko/reference/configuration/routing.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/combos.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/configuration/routing.mdgui/src/combo-workspace-data.tsgui/src/components/combo-workspace-detail-panel.tsxsrc/cli/combo.tssrc/codex/catalog.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/effort.tssrc/codex/catalog/kinds.tssrc/codex/catalog/metadata.tssrc/codex/catalog/native-models.tssrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/combos/index.tssrc/combos/types.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/model-routes.tssrc/server/management/model-rows.tssrc/types.tsstructure/03_catalog-and-subagents.mdtests/cli-headless-parity.test.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/combo-management-api.test.tstests/combo-workspace-data.test.tstests/combos.test.tstests/model-visibility-management-api.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a7cc03ee2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (item.nativeAlias | ||
| && (!alias || alias.includes("/") || !NATIVE_OPENAI_FAMILY_RE.test(alias) || !displayName)) { |
There was a problem hiding this comment.
When a combo was created with nativeAlias via the CLI/API, the dashboard now preserves that hidden flag but does not expose any control to unset it; if the user edits the alias to a normal combo alias or clears it, this validation path keeps rejecting the draft because item.nativeAlias remains true. The same conversion is valid through the management API by omitting nativeAlias, so the dashboard can no longer perform an otherwise supported edit for native-alias combos; clear nativeAlias/displayName when the alias leaves the native-alias contract or add explicit controls for those fields.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ce998b2. Dashboard edits that leave the native-alias contract now clear nativeAlias and displayName in the normalized draft and PUT payload; conversion is covered by regression tests.
| // Desktop's remote allowlist ignores `visibility: "hide"`. Keep current/pristine native recovery | ||
| // sources beside the on-disk rows so re-enabling a model restores its real metadata. Routed and | ||
| // user-authored rows still come only from the on-disk catalog. | ||
| const nativeRecoverySources = [catalog.models ?? [], readCatalogBackup(catalogPath)?.models ?? []]; |
There was a problem hiding this comment.
This recovery source is only used by writeRetainedCatalogSync, but dashboard/API visibility edits call the management convergence path, whose prepareCatalog still seeds native slugs from the active catalog. After a native-alias sync omits a disabled unshadowed native row (for example gpt-5.5 while another native alias remains configured), enabling that row removes it from disabledModels but the active catalog has no row to seed, so the refreshed catalog still omits it until a separate retained sync runs; use the same bundled/backup native recovery in convergence before writing management refreshes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ce998b2. Native catalog recovery is shared through the bundled/backup recovery helper and passed into management convergence as well as retained sync; the convergence regression covers omitted native restoration.
| if (isNativeAliasCatalogEntry(m)) { | ||
| return typeof m.slug === "string" && exactComboSlugs.has(m.slug); | ||
| } |
There was a problem hiding this comment.
When provider discovery transiently returns no routed rows, this branch appears to keep an existing native-alias combo, but the later virtual-combo filter keeps combo-owned entries only if their slug is in freshSlugs; with routedEntries.length === 0, the preserved native-alias row is dropped. In that empty-gather scenario the genuine native row is also suppressed by the alias, so the Desktop-compatible bare slug disappears from the catalog until a successful fetch occurs; carry preserved native aliases through the combo-owned filter or mark them as retained fresh rows.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ce998b2. Empty routed gathers retain an existing configured native-alias row through the combo-owned filter; the catalog regression verifies the row and metadata remain present.
Adds devlog/_plan/260805_issue_pr_triage: a prior-art index over the whole tracked devlog corpus, a frozen surface inventory, per-item verdicts for 17 bug-class issues and all 25 open PRs, an issue-PR cross-link matrix, and a dependency-ordered action queue. The prior-art pass is the load-bearing part: 8 issues and 8 PRs had never been investigated, while #92 has passed through 26 devlog units and is still open. Seven of the eight new issues are real defects; #1045 is already fixed on dev. #1043 and #1024 share one root cause, and #999/#1056 are complementary halves of #241 rather than duplicates. No code, no GitHub writes.
9976f2b to
1ba06b3
Compare
1ba06b3 to
7ce998b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Maintainer update for PR #1056 (head
Both fork workflow runs are still held at |
|
@codex review |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/catalog/sync.ts (1)
632-639: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReject native-alias rows explicitly instead of relying on a malformed provider string.
Line 633 now admits bare native-alias slugs into this filter. Line 635 then runs
m.slug.slice(0, m.slug.indexOf("/"))on a slug with no/, soindexOfreturns-1andslice(0, -1)produces the slug minus its last character, for examplegpt-5.6-so. Line 636 tests that string againstgatheredProviderNames.The current result is still correct, but only by coincidence: the malformed lookup always misses, and line 639 rejects the row because line 470 classifies every native-alias entry as OpenCodex-authored. The two behaviors cancel. If
isOcxAuthoredRoutedEntryis later narrowed, this malformed provider name becomes a live comparison against real provider names.An early return states the intent directly and removes the arithmetic on a bare slug.
♻️ Proposed refactor: explicit rejection
const preservedForeignRouted = catalogModels.filter(m => { if (typeof m.slug !== "string" || (!m.slug.includes("/") && !isNativeAliasCatalogEntry(m))) return false; if (trustedAccountBoundNativeCatalogSlug(m) !== undefined) return false; + // A native alias is always OpenCodex-authored, never a foreign row to preserve. + // Return here so the provider split below only ever runs on a namespaced slug. + if (isNativeAliasCatalogEntry(m)) return false; const provider = m.slug.slice(0, m.slug.indexOf("/")); if (gatheredProviderNames.has(provider) || freshSlugs.has(m.slug)) return false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/catalog/sync.ts` around lines 632 - 639, Update the filter callback in preservedForeignRouted to explicitly reject native-alias rows before deriving provider from the slug: return false when isNativeAliasCatalogEntry(m) is true, then require a slash-qualified string before calling indexOf/slice. Preserve the existing foreign routed-entry preservation behavior for valid provider/slugs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/codex-app-models.md`:
- Around line 117-119: Update the documentation sentence describing dashboard
switches to clarify that only unshadowed native rows remain switchable; shadowed
bare native slugs are omitted from the Models page and have no available switch.
In `@docs-site/src/content/docs/ja/guides/combos.md`:
- Around line 178-181: Update the combo option descriptions in
docs-site/src/content/docs/ja/guides/combos.md lines 178-181 and
docs-site/src/content/docs/ko/guides/combos.md lines 178-180 to document that
“-” clears --effort, --alias, and --display-name, and that combo subcommands are
also available under ocx route combo. Keep the existing localized descriptions
and supported-option semantics intact.
In `@gui/src/components/combo-workspace-detail-panel.tsx`:
- Line 205: Extend the combo editor component around the existing alias control
and draft update flow to render i18n-backed controls for nativeAlias and
displayName, preserving both values through create, edit, and save operations.
Bind the toggle and display-name input to draft state and ensure their values
remain consistent with management API response mapping and toPutBody. Add
regression coverage for creating and editing these fields, and define all
user-visible labels in the appropriate i18n locale files.
In `@src/codex/catalog/sync.ts`:
- Around line 498-505: Update the recovery merge loop in
mergeCatalogModelsWithNativeRecovery so each recovered entry is copied before
being added to merged, including a separate copy of supported_reasoning_levels,
preventing ensureUltraReasoningLevel from mutating catalog or backup source rows
while preserving deduplication by recoveredNativeSlugs.
In `@src/server/index.ts`:
- Around line 721-725: Update the no-selector native model path in the
visibleNatives calculation to suppress native-alias entries targeting supported
bare natives, matching the shadowedNativeSlugs filtering used by the selector
branch and visibleNativeSlugs(config). Preserve selectorNativeSlugs filtering
for selector-based discovery and ensure raw /v1/models output does not emit
duplicate alias IDs.
In `@tests/combo-management-api.test.ts`:
- Around line 563-566: Extend the routing assertions near the existing
gpt-5.6-terra check to verify that routeModel(config, "gpt-5.6-sol") no longer
resolves to the renamed combo. Keep the new alias assertion unchanged and assert
the released slug does not return the combo mapping for providerName "a" and
modelId "m1".
---
Outside diff comments:
In `@src/codex/catalog/sync.ts`:
- Around line 632-639: Update the filter callback in preservedForeignRouted to
explicitly reject native-alias rows before deriving provider from the slug:
return false when isNativeAliasCatalogEntry(m) is true, then require a
slash-qualified string before calling indexOf/slice. Preserve the existing
foreign routed-entry preservation behavior for valid provider/slugs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 91740a90-97af-42a0-8153-543d4bb132e3
📒 Files selected for processing (54)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ja/guides/combos.mddocs-site/src/content/docs/ja/reference/configuration/routing.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/combos.mddocs-site/src/content/docs/ko/reference/configuration/routing.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/combos.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/configuration/routing.mdgui/src/combo-workspace-data.tsgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tssrc/cli/combo.tssrc/codex/catalog.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/effort.tssrc/codex/catalog/kinds.tssrc/codex/catalog/metadata.tssrc/codex/catalog/native-models.tssrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/combos/index.tssrc/combos/types.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/model-routes.tssrc/server/management/model-rows.tssrc/types.tsstructure/03_catalog-and-subagents.mdtests/cli-headless-parity.test.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/combo-workspace-data.test.tstests/combos.test.tstests/model-visibility-management-api.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
| - With at least one native-alias combo configured, disabled bare native rows are omitted rather than | ||
| retained hidden because affected Desktop releases ignore the hidden flag. The dashboard keeps | ||
| their static switches and sync restores native metadata when a row is re-enabled. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify which native switches remain in the dashboard.
Line 119 says that the dashboard keeps static switches. src/codex/catalog/metadata.ts lines 196-203 remove native-alias slugs from nativeModelRows. The switch for a shadowed bare native slug is not available.
State that only unshadowed native rows remain switchable. This prevents users from looking for a switch that the Models page does not render.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/guides/codex-app-models.md` around lines 117 -
119, Update the documentation sentence describing dashboard switches to clarify
that only unshadowed native rows remain switchable; shadowed bare native slugs
are omitted from the Models page and have no available switch.
Source: Path instructions
| `set` は、`--strategy`、`--sticky`、`--effort`、`--alias`、`--native-alias`、 | ||
| `--display-name`、および `--rename-from` も受け入れます。`--native-alias` には現在 | ||
| サポートされている bare native alias と空でない display name が必要です。`create` および `update` は `set` の | ||
| エイリアス、`delete` は `remove` のエイリアスです。 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Restore the missing CLI semantics in both localized guides.
The English guide documents that - clears --effort, --alias, and --display-name. It also documents that combo subcommands are available under ocx route combo. Both localized guides omit these supported behaviors.
docs-site/src/content/docs/ja/guides/combos.md#L178-L181: Document-clearing semantics andocx route combosubcommand availability.docs-site/src/content/docs/ko/guides/combos.md#L178-L180: Document-clearing semantics andocx route combosubcommand availability.
📍 Affects 2 files
docs-site/src/content/docs/ja/guides/combos.md#L178-L181(this comment)docs-site/src/content/docs/ko/guides/combos.md#L178-L180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/ja/guides/combos.md` around lines 178 - 181,
Update the combo option descriptions in
docs-site/src/content/docs/ja/guides/combos.md lines 178-181 and
docs-site/src/content/docs/ko/guides/combos.md lines 178-180 to document that
“-” clears --effort, --alias, and --display-name, and that combo subcommands are
also available under ocx route combo. Keep the existing localized descriptions
and supported-option semantics intact.
Source: Path instructions
| alias: e.target.value.trim() ? e.target.value : null, | ||
| model: comboPublicModelId(d.id, e.target.value), | ||
| }))} | ||
| onChange={(e) => updateDraft((d) => updateComboAliasDraft(d, e.target.value))} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Expose native-alias fields in the dashboard editor.
ComboItem and toPutBody now carry nativeAlias and displayName in gui/src/combo-workspace-data.ts, but this panel renders no control for either field. New drafts start with nativeAlias: false and displayName: null. Line 205 only edits the alias.
Therefore, the dashboard cannot enable native-alias mode or set its required display name. Add an i18n-backed native-alias toggle and display-name input. Keep both fields in draft state and add create/edit/save regression coverage.
As per path instructions, GUI state changes must stay consistent with management API responses, and user-visible strings must use i18n locale files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/components/combo-workspace-detail-panel.tsx` at line 205, Extend the
combo editor component around the existing alias control and draft update flow
to render i18n-backed controls for nativeAlias and displayName, preserving both
values through create, edit, and save operations. Bind the toggle and
display-name input to draft state and ensure their values remain consistent with
management API response mapping and toPutBody. Add regression coverage for
creating and editing these fields, and define all user-visible labels in the
appropriate i18n locale files.
Source: Path instructions
| for (const source of nativeRecoverySources) { | ||
| for (const entry of source) { | ||
| const slug = recoverableNativeSlug(entry); | ||
| if (slug === null || recoveredNativeSlugs.has(slug)) continue; | ||
| merged.push(entry); | ||
| recoveredNativeSlugs.add(slug); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine whether the merge path mutates nested arrays shared with recovery sources.
set -euo pipefail
echo '--- ensureUltraReasoningLevel definition ---'
ast-grep run --pattern 'function ensureUltraReasoningLevel($$$) { $$$ }' --lang typescript src
echo '--- ensureStrictCatalogFields copy semantics ---'
ast-grep run --pattern 'export function ensureStrictCatalogFields($$$) { $$$ }' --lang typescript src
echo '--- normalizeServiceTiers copy semantics ---'
ast-grep run --pattern 'export function normalizeServiceTiers($$$) { $$$ }' --lang typescript src
echo '--- any push into supported_reasoning_levels ---'
rg -nP --type=ts -C4 'supported_reasoning_levels.*\.push\(|levels\.push\(' srcRepository: lidge-jun/opencodex
Length of output: 202
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- candidate files ---'
fd -a 'sync\.ts$|catalog.*sync|catalog' src tests | sed 's#^\./##' | head -100
echo
echo '--- sync imports/matches ---'
rg -n 'mergeCatalogModelsWithNativeRecovery|ensureUltraReasoningLevel|supported_reasoning_levels|recoverableNativeSlug|writeRetainedCatalogSync|mergeCatalogEntriesForSync|ensureStrictCatalogFields|normalizeServiceTiers' src tests -S || trueRepository: lidge-jun/opencodex
Length of output: 20769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/codex/catalog/sync.ts relevant sections ---'
sed -n '470,530p' src/codex/catalog/sync.ts
echo
sed -n '558,578p' src/codex/catalog/sync.ts
echo
sed -n '850,888p' src/codex/catalog/sync.ts
echo
sed -n '950,970p' src/codex/catalog/sync.ts
echo
echo '--- src/codex/catalog/parsing.ts relevant sections ---'
sed -n '190,290p' src/codex/catalog/parsing.ts
echo
echo '--- src/codex/catalog/effort.ts relevant sections ---'
sed -n '185,225p' src/codex/catalog/effort.ts
echo
echo '--- tests relevant native recovery assertions ---'
sed -n '190,218p' tests/codex-catalog-sync-hardening.test.ts
sed -n '212,224p' tests/codex-catalog-sync-hardening.test.tsRepository: lidge-jun/opencodex
Length of output: 14054
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- parsed RawEntry definition/type locations ---'
rg -n 'interface RawEntry|type RawEntry|export type RawEntry|export interface RawEntry|export namespace RawEntry|namespace RawEntry' src tests -S
echo
echo '--- all RawEntry declarations ---'
rg -n --type ts -C3 'interface RawEntry|type RawEntry|RawEntry =' src tests | head -200
echo
echo '--- behavior probe: JS shallow copy of reasoning levels ---'
node - <<'JS'
function ensureUltraReasoningLevel(entry) {
const levels = Array.isArray(entry.supported_reasoning_levels)
? entry.supported_reasoning_levels
: [];
if (levels.length === 0) return;
for (const effort of ["max", "ultra"]) {
if (levels.some(level => level.effort === effort)) continue;
levels.push({ effort, description: `${effort} reasoning` });
}
entry.supported_reasoning_levels = levels;
}
const original = [{ effort: "low" }];
const source = {
slug: "gpt-5.5",
display_name: "5.5",
supported_reasoning_levels: original,
};
const preserved = { ...source, priority: 5 };
ensureUltraReasoningLevel(preserved);
console.log(JSON.stringify({ preservedUltras: preserved.supported_reasoning_levels.map(l => l.effort), sourceLevels: source.supported_reasoning_levels.map(l => l.effort), sameRef: preserved.supported_reasoning_levels === source.supported_reasoning_levels }));
JS
echo
echo '--- search for tests around backup/source mutation during recovery ---'
rg -n -C6 'original (Sol|gpt-5.5|metadata)|restored|catalog.models.*supported_reasoning_levels|restore|recovery|pristine' tests/codex-catalog-sync-hardening.test.ts src/codex/catalog tests/codex-catalog.test.ts tests/native-model-toggle.test.ts -SRepository: lidge-jun/opencodex
Length of output: 25383
Assign recovered reasoning levels instead of pushing into the source array.
ensureUltraReasoningLevel() mutates whichever array entry.supported_reasoning_levels currently references, including arrays carried by catalog.models or the backup catalog when mergeCatalogModelsWithNativeRecovery() appends source rows by reference. The recovery sources are meant to provide pristine metadata for native model restore, so each pushed row should be copied such that later reasoning-level mutation does not alter the backup/source object.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/codex/catalog/sync.ts` around lines 498 - 505, Update the recovery merge
loop in mergeCatalogModelsWithNativeRecovery so each recovered entry is copied
before being added to merged, including a separate copy of
supported_reasoning_levels, preventing ensureUltraReasoningLevel from mutating
catalog or backup source rows while preserving deduplication by
recoveredNativeSlugs.
| const visibleNatives = includeNativeOpenAi | ||
| ? accountSelectors.length > 0 ? selectorNativeSlugs : visibleNativeSlugs(config) | ||
| ? accountSelectors.length > 0 | ||
| ? selectorNativeSlugs.filter(slug => !shadowedNativeSlugs.has(slug)) | ||
| : visibleNativeSlugs(config) | ||
| : []; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine whether visibleNativeSlugs already excludes alias-claimed bare natives.
set -euo pipefail
echo '--- visibleNativeSlugs ---'
ast-grep run --pattern 'export function visibleNativeSlugs($$$) { $$$ }' --lang typescript src/codex/catalog/metadata.ts
echo '--- configuredNativeAliasSlugs ---'
ast-grep run --pattern 'export function configuredNativeAliasSlugs($$$) { $$$ }' --lang typescript src/codex/catalog/metadata.ts
echo '--- desktopVisibleNativeSlugs for comparison ---'
ast-grep run --pattern 'export function desktopVisibleNativeSlugs($$$) { $$$ }' --lang typescript src/codex/catalog/metadata.ts
echo '--- existing raw /v1/models duplicate-id coverage ---'
rg -nP --type=ts -C4 'object: "list"|uniqueCatalogModelsForRawPublicList' testsRepository: lidge-jun/opencodex
Length of output: 184
🏁 Script executed:
#!/bin/bash
# Inspect the relevant server code and metadata functions around native/model selection.
set -euo pipefail
echo '--- locate files ---'
git ls-files | rg '(^src/server/index\.ts$|catalog/metadata|NATIVE_OPENAI_MODELS|visibleNativeSlugs)' || true
echo '--- server index relevant lines ---'
sed -n '680,750p' src/server/index.ts
echo '--- search visibleNativeSlugs definitions/usages ---'
rg -n --type=ts 'function visibleNativeSlugs|const visibleNativeSlugs|export (async )?function visibleNativeSlugs|visibleNativeSlugs\(' src tests || true
echo '--- search native alias/slugs functions ---'
rg -n --type=ts 'configuredNativeAliasSlugs|shadowedNativeSlugs|visibleNativeSlugs|NATIVE_OPENAI_MODELS|desktopVisibleNativeSlugs|accountSelectors' src/server/index.ts src/codex || trueRepository: lidge-jun/opencodex
Length of output: 12084
🏁 Script executed:
#!/bin/bash
# Inspect the catalog functions and evidence from tests around alias shadowing.
set -euo pipefail
echo '--- src/codex/catalog/metadata.ts functions ---'
sed -n '1,230p' src/codex/catalog/metadata.ts
echo '--- relevant tests ---'
sed -n '200,280p' tests/claude-models-discovery.test.ts || true
sed -n '40,80p' tests/native-model-toggle.test.ts || true
echo '--- read-only invariant probe under file as data ---'
python3 - <<'PY'
from pathlib import Path
import re
metadata = Path("src/codex/catalog/metadata.ts").read_text()
patterns = {
"configuredNativeAliasSlugs": r"export \?\?function\s+configuredNativeAliasSlugs\s*\([^)]*\)[^{]*\{(?P<body>.*?)\n}",
"visibleNativeSlugs": r"export \?\?function\s+visibleNativeSlugs\s*\([^)]*\)[^{]*\{(?P<body>.*?)\n}",
}
for name, pat in patterns.items():
m = re.search(pat, metadata, re.S)
print(f"\n--- {name}:")
print(m.group(0) if m else "NOT FOUND")
index = Path("src/server/index.ts").read_text()
m = re.search(r"const visibleNatives = includeNativeOpenAi\s*\?\s*(?P<body>.*?)\s*$", index, re.S)
print("\n--- src/server/index.ts visibleNatives block:")
print(m.group(0) if m else "NOT FOUND")
print("\n--- alias-shadowing checks in visibleNativeSlugs/configuredNativeAliasSlugs:")
for name, body in [("configuredNativeAliasSlugs", re.search(patterns["configuredNativeAliasSlugs"], metadata, re.S).group("body")),
("visibleNativeSlugs", re.search(patterns["visibleNativeSlugs"], metadata, re.S).group("body"))]:
print(f"{name}: contains shadowed check? {any(k in body for k in ['configuredNativeAliasSlugs', 'shadowed', 'NATIVE_OPENAI_MODELS.filter', 'filter(slug => !shadowed)'])}")
print(body)
PYRepository: lidge-jun/opencodex
Length of output: 45754
🏁 Script executed:
#!/bin/bash
# Inspect raw public model list catalog handling and run a direct source-derived duplicate-id probe.
set -euo pipefail
echo '--- catalog metadata remaining functions ---'
sed -n '230,380p' src/codex/catalog/metadata.ts
echo '--- catalog exports ---'
cat src/codex/catalog.ts
echo '--- static duplicate-ID invariant probe from source text ---'
python3 - <<'PY'
from pathlib import Path
import re
# Extract function bodies from the TypeScript source text without evaluating repository code.
metadata = Path("src/codex/catalog/metadata.ts").read_text()
raw = re.search(r"export \(async\) function (uniqueCatalogModelsForRawPublicList)\([^)]*\)[^{]*\{(?P<body>.*?)\n\}", metadata, re.S)
print(raw.group(0) if raw else "uniqueCatalogModelsForRawPublicList NOT FOUND")
print("- contains alias native filter? ", "alias_is_native" in raw.group("body") if raw else False)
print("- contains suppression variable? ", "nativeSuppressedSlugs" in raw.group("body") if raw else False)
# Model the duplicate-only branches: raw public list includes aliases whose target is a bare native.
# This should not mutate repo files; if a test fixture contains such an alias, derive the expected IDs.
examples = {
"raw_alias": {"alias": "gpt-5.6-sol", "owned_by": "combo", "reasoningEfforts": [], "defaultReasoningEffort": None},
"bare": "gpt-5.6-sol",
}
ids = [examples["bare"], f"{examples['raw_alias']['alias']}"]
print("- derived no-selector raw IDs:", ids)
print("- derived duplicate id?", len(set(ids)) != len(ids))
PYRepository: lidge-jun/opencodex
Length of output: 5101
🏁 Script executed:
#!/bin/bash
# Inspect the catalog aggregation functions that build raw public model lists.
set -euo pipefail
echo '--- locate aggregation file ---'
git ls-files | rg 'catalog/aggregation|aggregation' || true
echo '--- source for uniqueCatalogModelsForRawPublicList and alias handling ---'
grep -Rn --include='*.ts' --include='*.tsx' "uniqueCatalogModelsForRawPublicList\|alias_is_native\|nativeSuppressedSlugs\|isNativeAliasCombo\|alias as string" src/codex src || true
echo '--- read candidate files ---'
while IFS= read -r f; do
echo "===== $f ====="
wc -l "$f"
sed -n '1,260p' "$f"
done < <(git ls-files 'src/codex/catalog/aggregation.ts')Repository: lidge-jun/opencodex
Length of output: 2554
🏁 Script executed:
#!/bin/bash
# Read the exact raw public list implementation around line 379.
set -euo pipefail
echo '--- uniqueCatalogModelsForRawPublicList implementation context ---'
sed -n '350,395p' src/codex/catalog/aggregation.ts
echo '--- nearby alias-suppression helper if present ---'
rg -n -C3 "uniqueCatalogModelsForPublicList|alias_is_native|nativeSuppressedSlugs|SUPPORTED_NATIVE_OPENAI_SLUGS|configuredNativeAliasSlugs" src/codex/catalog/aggregation.ts || trueRepository: lidge-jun/opencodex
Length of output: 2226
Deduplicate raw /v1/models native aliases the same way selector discovery does.
src/codex/catalog/aggregation.ts:379 drops duplicate raw IDs only after catalog model deduplication, so a native alias entry for a bare native can still emit the same alias id as visibleNativeSlugs(config) because visibleNativeSlugs(config) filters shadowedNativeSlugs in src/codex/catalog/metadata.ts:162-167. Remove the native-alias entries whose target is a supported bare native, or otherwise make the no-selector raw list match the selector branch’s duplicate bare-native suppression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/index.ts` around lines 721 - 725, Update the no-selector native
model path in the visibleNatives calculation to suppress native-alias entries
targeting supported bare natives, matching the shadowedNativeSlugs filtering
used by the selector branch and visibleNativeSlugs(config). Preserve
selectorNativeSlugs filtering for selector-based discovery and ensure raw
/v1/models output does not emit duplicate alias IDs.
| expect(routeModel(config, "gpt-5.6-terra")).toMatchObject({ | ||
| providerName: "a", | ||
| modelId: "m1", | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Also assert that the released bare slug stops resolving to the combo.
Lines 563-566 prove the new alias gpt-5.6-terra routes to a/m1. They do not prove the old alias gpt-5.6-sol was released.
That second half is the assertion that guards the migration fix. The reason the config keeps a bare gpt-5.6-sol in subagentModels, injectionModel, shadowCallIntercept.model, and the claudeCode fields is that the slug is expected to fall through to native OpenAI routing after the rename. If a future change makes the renamed combo keep capturing the old alias, every assertion at lines 553-562 still passes while those config references silently point at the combo again.
Add one assertion that routeModel(config, "gpt-5.6-sol") no longer returns the combo.
💚 Proposed additional assertion
expect(routeModel(config, "gpt-5.6-terra")).toMatchObject({
providerName: "a",
modelId: "m1",
});
+ // The renamed combo must release the old bare slug back to native routing;
+ // the preserved bare references above depend on this fall-through.
+ expect(routeModel(config, "gpt-5.6-sol")).not.toMatchObject({
+ providerName: "a",
+ modelId: "m1",
+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/combo-management-api.test.ts` around lines 563 - 566, Extend the
routing assertions near the existing gpt-5.6-terra check to verify that
routeModel(config, "gpt-5.6-sol") no longer resolves to the renamed combo. Keep
the new alias assertion unchanged and assert the released slug does not return
the combo mapping for providerName "a" and modelId "m1".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ce998b27e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| nativeAlias, | ||
| exactComboSlugs, | ||
| ); | ||
| routed.opencodex_catalog_kind = CODEX_NATIVE_ALIAS_CATALOG_KIND; |
There was a problem hiding this comment.
When this emits a combo-owned native alias at a bare slug, the rest of the backup/admission code still classifies routed catalogs only by slug.includes("/") (catalogHasRoutedEntries and the convergence hasRoutedEntries). In a setup with only a native-alias combo and no other namespaced rows, the next retained or management convergence sync treats the active catalog as pristine and can publish a backup containing the owned_by: "combo" alias row; later alias removal or native recovery can restore that combo row instead of the genuine native metadata. Include CODEX_NATIVE_ALIAS_CATALOG_KIND in routed/pristine detection before these rows can be backed up as native.
Useful? React with 👍 / 👎.
| && !disabledNative.has(entry.slug) | ||
| && !preservedNativeAliasSlugs.has(entry.slug) |
There was a problem hiding this comment.
This convergence path only excludes disabled bare-native keys and aliases already preserved from disk. When a native-alias combo is disabled via combo/<id> (or has not yet produced a preserved alias row), bundled/native recovery still admits the real gpt-* slug here and buildCatalogEntries emits the genuine native row, undoing the Desktop allowlist suppression that retained sync and live /v1/models?client_version apply. Use the configured native-alias suppression set in this path as well.
Useful? React with 👍 / 👎.
| exactComboSlugs, | ||
| ); | ||
| routed.opencodex_catalog_kind = CODEX_NATIVE_ALIAS_CATALOG_KIND; | ||
| const rankHit = rank.get(slug) ?? rank.get(`${nativeAlias.provider}/${nativeAlias.id}`); |
There was a problem hiding this comment.
When account selectors are active, featuring a bare native alias in subagentModels now ranks the combo row here, but the selector loop still inherits the same rank.get(nativeSlug) for genuine main/<native> account rows. With a native-alias combo for gpt-5.6-sol, pool account selectors, and subagentModels: ["gpt-5.6-sol"], the priority-sorted first-five spawn_agent catalog can include both the routed combo and the native account clone, consuming slots and potentially sending subagents to OpenAI instead of the compatibility route; account rows should only inherit exact selector/native ranks when the bare slug is shadowed by a native alias.
Useful? React with 👍 / 👎.
| const nativeAlias = entry.opencodex_catalog_kind === CODEX_NATIVE_ALIAS_CATALOG_KIND; | ||
| const upstream = nativeAlias | ||
| ? undefined | ||
| : UPSTREAM_NATIVE_ENTRIES.get(trustedAccountBoundNativeCatalogSlug(entry) ?? slug); |
There was a problem hiding this comment.
This special-case makes default mode treat native-alias replacement rows as unpinned instead of applying the upstream pin for the bare slug they replace. For gpt-5.6-sol/terra with the native v2 feature off, the alias row loses the upstream multi_agent_version: "v2" that Codex validates before spawn_agent; for gpt-5.6-luna with the feature on, it can be stamped v2 instead of the upstream v1. Native aliases should still read the upstream pin for their native slug before falling back to feature defaults.
Useful? React with 👍 / 👎.
|
Reviewed in a sweep of the To be clear about what this is: #241's root cause is upstream — Codex Desktop filters the model list against its own remote native-ID allowlist, so routed rows that the app-server and CLI both load correctly never reach the picker. We cannot fix that from here. What this PR does is give a user an opt-in way to route through a native ID that Desktop already accepts. That is a workaround, and workarounds usually rot because they hide what they are doing. This one does not: aliases are bounded to the seven supported native IDs, Please drop the Second item: the head is 44 commits behind Worth noting for anyone reading later: this does not supersede #999, and #999 does not supersede this. That PR documents the Desktop limitation; this one gives users a way around it. #241 stays open either way, because the cause is still upstream. |
2 participants
Summary
available_modelsfiltering. Bare native IDs route through the configured combo, while account-qualified IDs remain genuine OpenAI routes.Verification
bun run prepushafter rebasing ontoupstream/devat791e0fbf: 9,086 passed, 8 skipped, 0 failed; privacy scan passed; React Doctor reported no issues.@bitkyc08/opencodexpackage locally;/healthzstayed healthy./v1/modelsand a fresh Codex app-servermodel/listreturned eight Nova routes and zero native OpenAI rows.Nova1picker options and no native OpenAI entries.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I fixed all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation