fix(codex): preserve routed models in desktop picker - #1244
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNative-alias combos are now opt-in and require supported native slugs plus display names. Catalog generation, visibility, recovery, routing, CLI, GUI editing, localized documentation, and regression coverage now support the behavior. ChangesNative alias compatibility
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI_GUI
participant ComboConfig
participant CatalogSync
participant ModelVisibility
CLI_GUI->>ComboConfig: Set nativeAlias and displayName
ComboConfig->>CatalogSync: Provide normalized native-alias combo
CatalogSync->>ModelVisibility: Apply shadowing and disabled selectors
ModelVisibility-->>CLI_GUI: Return visible combo and native rows
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
|
@codex review Please review this replacement PR against current |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🤖 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 @.github/workflows/maintainer-full-verify-1244.yml:
- Around line 3-6: Add a branch-scoped concurrency group to the workflow’s
top-level configuration so pushes to the same branch run serially. Update the
cleanup/removal step to tolerate the workflow already having been successfully
removed by an earlier run, while preserving normal failure reporting for other
errors.
- Around line 33-34: Update the high-severity dependency audit invoked by the
“High severity dependency audit” workflow step to also run `bun audit
--audit-level=high` from the `docs-site` directory, while preserving the
existing root and gui audits.
- Around line 16-21: Replace the mutable action references in the workflow’s
checkout and Bun setup steps with verified full commit SHA references, reusing
the corresponding SHA-pinned versions from the CI workflow while preserving
their existing configuration.
- Around line 8-9: The full verification job grants excessive repository write
access and persists checkout credentials. Change its permissions to contents:
read and configure actions/checkout with persist-credentials: false; move only
the trusted maintainer cleanup into a separate job with contents: write,
ensuring it does not execute repository-provided install, build, audit, or
verification code, and protect the maintainer/supersede-1056-native-alias branch
and its writers.
In `@docs-site/src/content/docs/zh-cn/guides/codex-app-models.md`:
- Around line 85-86: Update the native-alias behavior description in the Chinese
guide to state that shadowed bare native rows are omitted from the Models page
and have no native switch; only unshadowed native rows retain a switch and can
restore native metadata when re-enabled.
In `@gui/tests/combo-native-alias-editor.test.tsx`:
- Around line 86-95: Re-query the `#cwi-edit-display-name` input after
nativeAlias.click() and before calling setInputValue, while retaining the
existing pre-toggle expect(displayName).toBeTruthy() assertion. Use the
refreshed element for the post-toggle input event so the test targets the
current DetailPanel DOM node.
- Around line 96-98: Update ComboWorkspaceDetailPanel’s create and save action
buttons with stable selectors such as the specified IDs or data-testid
attributes, then revise the corresponding test queries around the create and
save clicks to use those selectors instead of rendered textContent.
In `@src/codex/catalog/aggregation.ts`:
- Around line 258-266: Update src/codex/catalog/aggregation.ts lines 258-266 in
the combo visibility logic to use only the canonical combo/<id> selector for
native-alias combos, while retaining the bare alias selector for native-row
visibility. Update src/codex/catalog/provider-fetch.ts lines 1199-1207 so native
target metadata remains available to native-alias combos when that bare alias is
disabled for the native row; add regression coverage for a disabled bare alias
with an openai/<alias> combo target.
In `@src/codex/catalog/metadata.ts`:
- Line 217: Update mergeCatalogEntriesForSync() so preserved native-alias
entries also pass through the exactComboCatalogSlugs() disabled-combo filtering
before or during the final merge. Ensure native aliases whose combo/<id> is in
disabledModels are removed from the transitory saved-route result instead of
remaining visible, while retaining enabled aliases.
In `@src/codex/catalog/sync.ts`:
- Around line 495-520: Update mergeCatalogModelsWithNativeRecovery to create a
fully independent clone of each recovered entry before adding it to merged,
replacing the selective supported_reasoning_levels copy. Use the existing
structured-clone mechanism available in the project so nested fields such as
input_modalities, service_tiers, and metadata cannot remain shared with
nativeRecoverySources.
- Around line 337-344: Update the nativeAliasesBySlug construction in the sync
flow to detect duplicate bare aliases among COMBO_NAMESPACE native aliases
instead of overwriting earlier models. Preserve the first occurrence for each
alias, add subsequent matching combo models to collisionSkipped, and ensure they
are excluded or warned consistently before routed aliases are emitted.
- Around line 662-665: Update the input_modalities filtering logic near the
preserved-entry check to retain bare native aliases whose slug is present in
exactComboSlugs, even when input_modalities is missing or empty. Ensure this
retention is applied consistently with retainedNativeAlias so these rows are not
dropped during disk-loaded catalog filtering.
- Line 976: Update buildCatalogEntries() to exclude suppressedBareNativeSlugs
from nativeEntries before creating account-bound selector/native-slug clones,
preventing disabled desktop-hidden native slugs from being emitted with list
visibility. Ensure clones for slugs removed from suppression are restored before
applyNativeVisibility() runs, while preserving existing behavior for
unsuppressed native entries.
In `@src/codex/convergence.ts`:
- Around line 186-195: Replace the intermediate preservedNativeAliases array
with direct construction of preservedNativeAliasSlugs from active.models,
retaining the existing preservingExistingRouted guard and both predicates. Leave
the independently pushed-row logic unchanged.
In `@src/server/management/combo-routes.ts`:
- Around line 127-138: Update the PUT response in the combo route to return the
persisted `stored` object instead of the pre-strip `normalized` object, while
preserving the existing storage behavior in the normalization block. This keeps
PUT responses consistent with GET responses and ensures both use the same
key-omitting shape.
In `@tests/codex-catalog.test.ts`:
- Line 424: Loosen the warning assertion in the test around the preservation
case so it checks only the stable message portion, such as the preservation
count and context, rather than exact singular wording. Update the expect call
for warning to avoid coupling it to pluralization while still verifying that one
existing routed entry is preserved.
- Around line 769-816: Update the native-alias assertions in the test to derive
context windows, input modalities, reasoning efforts, and default reasoning
efforts from the native snapshot helpers used by completeNativeAliasMembers,
including nativeOpenAiContextWindow, nativeInputModalities, and
nativeReasoningEfforts. Assert that each alias inherits those source values
rather than duplicating literals, while preserving the existing alias-specific
expectations.
- Around line 351-367: Refactor the test setup around mergeCatalogEntriesForSync
so this 15-argument call uses a test-local helper with named options. Ensure
disabledModels, goIds, gatheredProviderNames, and exactComboSlugs are explicitly
assigned to their intended parameters, preserving the gpt-5.6-sol survival
assertion as a disabled-model scenario rather than relying on positional set
ordering.
In `@tests/codex-convergence-contract.test.ts`:
- Around line 234-269: The test fixture does not prove that alias generation or
routed-entry backup handling executes. Update the convergence test around
liveConfig, gatherCodexCatalogCandidate, and commitCodexCatalogCandidate to
assert a generation-only field such as the routed description or distinct
priority, add a case with the active catalog cleared, and verify the published
backup; ensure the fixture exercises bare-slug alias routing and exposes
incorrect catalogHasRoutedEntries behavior.
- Line 274: Strengthen the gpt-5.5 assertion in the recovery test by locating
the matching entry and asserting its display_name equals the bundled fixture
value "GPT-5.5", rather than only checking that an entry exists. Keep the test
focused on metadata recovered through mergeCatalogModelsWithNativeRecovery.
- Line 219: Replace the `as never` cast in the call to
`setBundledCatalogCacheForTests` with the directly typed `bundled` value. If
needed, annotate `bundled` as `RawCatalog & { models: RawEntry[] }` at its
declaration, preserving the existing models data while allowing TypeScript to
validate future signature changes.
In `@tests/combo-management-api.test.ts`:
- Around line 338-357: Rename the test case around comboApi PUT validation to
“PUT rejects a native alias without a displayName,” matching the omitted
displayName and nativeAlias: true inputs. Optionally add a sibling test for a
native alias that omits nativeAlias while preserving the existing no-mutation
assertions.
In `@tests/model-visibility-management-api.test.ts`:
- Around line 283-289: Update the disable-step assertion in the test around put
and loadConfig to compare the complete disabledModels array with toEqual,
including both the preserved bare native key gpt-5.6-sol and the newly disabled
combo/nova entry in the handler’s actual order.
In `@tests/native-alias-maintainer-regressions.test.ts`:
- Around line 45-51: Update the test around mergeCatalogModelsWithNativeRecovery
to locate the recovered row by its expected slug instead of assuming merged[0]
is the target. Assert that the slug-matched row exists before accessing
supported_reasoning_levels, then keep the deep-copy and source-immutability
assertions unchanged.
In `@tests/native-profile-drain-server.test.ts`:
- Around line 290-293: Extract the repeated bounded polling logic into a shared
helper in the test module, preserving the 2,000 ms deadline and Bun.sleep(10)
interval. Replace both loops near the upstream-close checks with calls to that
helper, including the loops around the existing upstream close assertions, while
preserving their current conditions and behavior.
🪄 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: 50ee5be5-27f1-4608-b56a-3c2f1b207cf0
📒 Files selected for processing (58)
.github/workflows/maintainer-full-verify-1244.ymldocs-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.tsgui/tests/combo-native-alias-editor.test.tsxgui/tests/combo-workspace-dirty.test.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/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-alias-maintainer-regressions.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: 2402848a83
ℹ️ 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".
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/convergence.ts (1)
253-258: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep preserved native aliases without
input_modalities.When routed discovery returns no rows, Lines 241-251 preserve exact native aliases from the active catalog. Lines 253-258 then remove those same rows if
input_modalitiesis absent or empty.A transient discovery failure can therefore delete a valid bare native alias from the converged catalog. Retain
isNativeAliasCatalogEntry(entry)rows whenpreservingExistingRoutedandexact.has(entry.slug)are both true. Add a regression test with an active native-alias row that has noinput_modalities.Proposed fix
if (!hasPhysicalComboProvider) { const exact = exactComboSlugs; - catalog.models = entries.filter(entry => ( - typeof entry.slug !== "string" || !exact.has(entry.slug) - || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0) - )); + catalog.models = entries.filter(entry => { + const slug = typeof entry.slug === "string" ? entry.slug : ""; + const retainedNativeAlias = preservingExistingRouted + && isNativeAliasCatalogEntry(entry) + && exact.has(slug); + return retainedNativeAlias + || !exact.has(slug) + || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0); + }); }This follows the preserved-alias contract in
src/codex/catalog/sync.tsLines 673-681.🤖 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/convergence.ts` around lines 253 - 258, Update the filtering logic in the hasPhysicalComboProvider block of convergence so entries satisfying preservingExistingRouted, exact.has(entry.slug), and isNativeAliasCatalogEntry(entry) are retained even when input_modalities is missing or empty. Preserve the existing filtering behavior for other entries, and add a regression test covering an active bare native-alias catalog row during routed discovery failure.
🤖 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.
Outside diff comments:
In `@src/codex/convergence.ts`:
- Around line 253-258: Update the filtering logic in the
hasPhysicalComboProvider block of convergence so entries satisfying
preservingExistingRouted, exact.has(entry.slug), and
isNativeAliasCatalogEntry(entry) are retained even when input_modalities is
missing or empty. Preserve the existing filtering behavior for other entries,
and add a regression test covering an active bare native-alias catalog row
during routed discovery failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c7e602f-d8bc-4de4-a007-f02560c31361
📒 Files selected for processing (13)
docs-site/src/content/docs/zh-cn/guides/codex-app-models.mdgui/src/components/combo-workspace-detail-panel.tsxgui/tests/combo-native-alias-editor.test.tsxsrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/server/management/combo-routes.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/model-visibility-management-api.test.tstests/native-alias-maintainer-regressions.test.tstests/native-profile-drain-server.test.ts
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/codex/convergence.ts (1)
241-252: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
prepareCatalogdrops preserved native aliases that carry noinput_modalities;mergeCatalogEntriesForSynckeeps them.Line 245 preserves an alias row from the active catalog when its slug is in
exactComboSlugs. The filter at Line 253-258 then removes any entry whose slug is inexactComboSlugsunlessinput_modalitiesis a non-empty array. The preserved alias row has no exemption, so a disk row withoutinput_modalitiesis preserved and then discarded in the same function.
src/codex/catalog/sync.tsadded exactly that exemption at Line 673-681 (retainedNativeAlias || !exactComboSlugs.has(slug) || ...). The two catalog paths therefore disagree for the same input.tests/native-alias-maintainer-regressions.test.tsLine 90-104 covers only themergeCatalogEntriesForSyncpath, so the convergence path has no regression test for it.Apply the same retention rule here and add a matching case to
tests/codex-convergence-contract.test.ts.🐛 Proposed fix — mirror the sync.ts retention rule
if (!hasPhysicalComboProvider) { const exact = exactComboSlugs; catalog.models = entries.filter(entry => ( typeof entry.slug !== "string" || !exact.has(entry.slug) + || (preservingExistingRouted && isNativeAliasCatalogEntry(entry)) || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0) )); } else {As per path instructions: "A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
🤖 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/convergence.ts` around lines 241 - 252, Update prepareCatalog’s preserved-entry filtering to retain native alias entries in exactComboSlugs when they lack input_modalities, matching the retainedNativeAlias exemption used by mergeCatalogEntriesForSync; preserve existing filtering for other entries. Add a focused regression case to tests/codex-convergence-contract.test.ts covering a disk/active native alias without input_modalities and asserting it remains in the converged catalog.Source: Path instructions
src/codex/catalog/sync.ts (1)
663-681: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a test for generated exact combo aliases without
inputModalities.
deriveEntrysetspreserveExactInputModalitiesfor exact combo rows, andensureStrictCatalogFieldsonly normalizes to["text"]wheninput_modalitiesis absent if that option is false. Since the later filter keeps exact combo rows only wheninput_modalitiesis a non-empty array, a combo catalog model with noinputModalitiesdrops the generated alias after the existing bare native alias has been suppressed. Add a regression test that exercises a bare exact combo native alias withmodel.inputModalitiesmissing/empty.🤖 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 663 - 681, Add a regression test covering a bare exact combo native alias where model.inputModalities is missing or empty. Exercise the sync flow through deriveEntry and ensureStrictCatalogFields, verify the existing bare native alias is suppressed, and assert the generated exact combo alias is retained despite lacking input_modalities.
🤖 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/combos.md`:
- Around line 101-104: Update the routing explanation around the exact bare slug
to state that only gpt-5.6-sol resolves to combo/nova-sol. Explicitly document
that openai-apikey/gpt-5.6-sol remains a provider-qualified API-key route and
never falls through to the native alias, while preserving the existing
account-qualified route behavior.
In `@docs-site/src/content/docs/reference/configuration/routing.md`:
- Around line 67-68: Update the routing configuration documentation around
nativeAlias to explicitly state that bare gpt-5.6-* aliases use Codex
Pool/Direct credentials, while openai-apikey/gpt-5.6-* uses API keys and never
falls through to the native alias; retain the existing distinction for
account-qualified and provider-qualified routes.
In `@docs-site/src/content/docs/ru/reference/configuration/routing.md`:
- Around line 67-68: Update the routing documentation at
docs-site/src/content/docs/ru/reference/configuration/routing.md lines 67-68 to
state that bare gpt-5.6-* IDs use Codex Pool/Direct credentials, while
openai-apikey/gpt-5.6-* uses API keys and never falls through. Apply the same
authentication and routing distinction at
docs-site/src/content/docs/ru/guides/combos.md lines 266-267, while clarifying
that account-qualified and provider-qualified identities remain unchanged.
In `@gui/src/i18n/de.ts`:
- Around line 1742-1743: Update the German translation value for
cws.field.nativeAliasHint to use the feminine article “diese Combo” instead of
“diesen Combo”, leaving the rest of the hint unchanged.
In `@gui/tests/combo-native-alias-editor.test.tsx`:
- Around line 9-29: Update the global-state setup in beforeEach and cleanup in
afterEach to save each key’s complete property descriptor via
Object.getOwnPropertyDescriptor(globalThis, key). Restore existing descriptors
with Object.defineProperty, and delete keys that had no original descriptor
instead of recreating them with undefined values. Keep the current testWindow
lifecycle and IS_REACT_ACT_ENVIRONMENT handling intact.
---
Outside diff comments:
In `@src/codex/catalog/sync.ts`:
- Around line 663-681: Add a regression test covering a bare exact combo native
alias where model.inputModalities is missing or empty. Exercise the sync flow
through deriveEntry and ensureStrictCatalogFields, verify the existing bare
native alias is suppressed, and assert the generated exact combo alias is
retained despite lacking input_modalities.
In `@src/codex/convergence.ts`:
- Around line 241-252: Update prepareCatalog’s preserved-entry filtering to
retain native alias entries in exactComboSlugs when they lack input_modalities,
matching the retainedNativeAlias exemption used by mergeCatalogEntriesForSync;
preserve existing filtering for other entries. Add a focused regression case to
tests/codex-convergence-contract.test.ts covering a disk/active native alias
without input_modalities and asserting it remains in the converged catalog.
🪄 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: ca79901d-7cd1-4806-b683-f19110beebda
📒 Files selected for processing (57)
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.tsgui/tests/combo-native-alias-editor.test.tsxgui/tests/combo-workspace-dirty.test.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/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-alias-maintainer-regressions.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-site/src/content/docs/ru/guides/combos.md (1)
224-226: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the existing clear-value syntax.
src/cli/combo.tssupports-to clear--effort,--alias, and--display-name(src/cli/combo.ts, Lines 68-99). The English page documents this behavior, but the Russian CLI section omits it.Add the equivalent Russian sentence:
Proposed documentation change
`set` также принимает `--strategy`, `--sticky`, `--effort`, `--alias`, `--native-alias`, `--display-name` и `--rename-from`. Для `--native-alias` нужны поддерживаемый сейчас bare native alias и непустой display name. +Значение `-` у `--effort`, `--alias` и `--display-name` очищает соответствующее поле.As per path instructions, localized documentation must stay synchronized with actual CLI/API behavior.
🤖 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/ru/guides/combos.md` around lines 224 - 226, Update the Russian CLI documentation near the `set` option description to state that `-` clears the `--effort`, `--alias`, and `--display-name` values, matching the behavior implemented in `src/cli/combo.ts` and documented on the English page.Source: Path instructions
🤖 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.
Outside diff comments:
In `@docs-site/src/content/docs/ru/guides/combos.md`:
- Around line 224-226: Update the Russian CLI documentation near the `set`
option description to state that `-` clears the `--effort`, `--alias`, and
`--display-name` values, matching the behavior implemented in `src/cli/combo.ts`
and documented on the English page.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: acb35fdb-e3f1-4dda-8f23-edb66940c93e
📒 Files selected for processing (6)
docs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mdgui/src/i18n/de.tsgui/tests/combo-native-alias-editor.test.tsx
b79a526 to
15545b3
Compare
|
@Wibias Thanks for rebasing this onto CI is red at
Two shapes of fix are available and the choice is yours, since these are your call sites: default the destructured input, or update every caller to pass it. Defaulting is smaller but hides a future missing caller; updating callers is louder. Whichever you pick, the same class of gap may exist for the other inputs added in the same commit, so it's worth grepping rather than fixing just this one. Holding the merge until Cross-platform CI is green at the exact head. Ping me when it is and I'll take another look. |
|
@Wibias CI is green at And your fix was better than what I suggested. I proposed either defaulting the destructured input at Reviewed the whole diff on your head. The native-alias design is well covered — actual Three things before I merge, none of them large: 1. The branch is 20 commits behind 2. One locale is out of sync. English documents that 3. CI evidence standard, and this one is about our infrastructure rather than your patch. I filed #1302 today: Linux test shards intermittently hang for ~15 minutes and get killed, reporting One request rather than a blocker: the screenshot in the description is carried forward from #1056, and this branch is a reconstruction rather than that code. A fresh Desktop picker capture at the current head would be worth more — bare native alias routing to the combo, account-qualified natives staying separate, and provider-qualified OpenAI not falling through. |
…ign (#1307) Five units, and the honest summary is that adversarial review changed my answer far more often than it polished it. 023 WP15 — three contributor fixes republished on dev with Co-authored-by preserved. Records that I read #1244's CI as in_progress and built a "watch" disposition on a run that had already concluded failure. 024 WP16 — #1273 is two defects. I designed the second fix three times and review caught the first two losing user data: whole-array reconciliation resurrects a deleted provider's rows, and keying on routedSlug duplicates renamed ones. Shipped defect 1, left defect 2 open with a diagnosis rather than landing a third attempt in a config-persistence path. 025 WP3 — #1185 was red and right; its crash was a Bun EEXIST in a file its one-file diff cannot reach. Also records a workflow snippet I suggested that interpolated an expression straight into shell, while reviewing a security-class change. 026 WP5 — #1244's author answered my defect report with neither fix I proposed, and was right. My stated reason for agreeing was wrong too: tsconfig.json has include:[src], so the typecheck I cited as caller-sweep evidence never read tests/ at all. 027 lane D — closes the catalog sequence, and records six PRs merged to dev without the approval MAINTAINERS.md requires. I logged every workflow-run approval meticulously against its head SHA, which is what made the missing pull-request approval feel handled. Filed as #1306 rather than back-filled.
|
Heads-up: this went You are now 25 behind, and the overlap is two files:
I am not rebasing this for you. The mechanical rebases I did today were ones where the author's intent was unambiguous; this is not that. The three conditions from my earlier review still stand, and the rebase now folds the first one in: the branch is stale (and the description still says "0 commits behind" and "remains draft", both untrue), the Russian locale is missing the Ping me once it is rebased and I will review promptly — the change itself reviewed well and I would rather it landed than aged. |
…ng (#1316) Published devlog 028/029 as #1314, rebased #1301 onto current dev, and reported #1244's new conflict to its author. The part worth keeping is a retraction, and its own correction. I claimed two CI runs shared a stall signature -- cli-native-profile handing off to cli-restart-health with 'killed 1 dangling process' last -- and posted it to #1302 as a narrowing. False: run 31152916419 never hung at all. It hit the Bun epoll_ctl error while loading autostart-health, kept running, and finished as a failure in 85 seconds. I had compared two logs by memory of what one of them said. Then the retraction itself was wrong twice. It named claude-messages-endpoint as the first affected file in 31263738953, when that is where output stops and baseten-provider is where the error first appears -- two different questions I had collapsed into one column. And it said the EEXIST has 'two outcomes', which asserts causation the logs do not establish: the error appears in all three runs, one of which completed normally. Three passes to state three sentences accurately. Both corrections are public on #1302 with the chain left visible. Also corrected here: 'two cancelled runs at the exact head' was wrong -- they were consecutive heads, before and after the rebase.
df11918 to
a802019
Compare
Squash-rebased PR #1244 onto current dev after conflict reconciliation and verification.
218839c to
9defc5a
Compare
|
@lidge-jun Rebased and the blocking review conditions are now covered.
The PR body is updated with the exact evidence. The fresh Desktop picker capture remains the non-blocking UI-evidence request; the screenshot currently shown is explicitly labelled historical. |
… closed (#1322) Two of the four contributor-held PRs resolved. @Wibias met every condition on #1244 -- rebase, Russian locale parity, two completed non-cancelled CI runs at the same SHA -- including resolving the conflict I created by merging #1305. Verified independently: two CI successes at d5e70a2 and a local full suite of 10120 pass / 0 fail on that head. Landed as c75e68e, 58 files. #241 closed with its chain named, since #1244 references #1056 rather than #241. #1301 rebased again after drifting 33 behind, then merged with --admin over a red test 1/4. The failure is a 5000ms timeout in tests/crash-guard.test.ts, which my one-file diff to tests/ci-workflows.test.ts cannot reach, and which passes 14/14 locally three times. Logged as MERGE-DESPITE-CI with the reason rather than as a clean green -- and recorded that this is the same shape of reasoning I criticised as 'rerun until green' earlier. #1272 hit a tenth #1302 occurrence and was rerun but NOT merged: it is still a draft and those boxes are the contributor's attestation. A broken CI is not a reason to tick them -- it is a reason not to, since they assert what CI can no longer confirm. Method note: rerun-failed-jobs overwrites the job log, so I destroyed the #1302 evidence by fetching it after the rerun. Capture first, then rerun.
2 participants
Summary
Supersedes #1056, which had diverged 362 commits behind
dev.This maintainer rebuild transplants the original native-alias feature onto current
devwith a true three-way merge, then hardens the resulting catalog/routing behavior against the still-valid bot/reviewer findings and current-dev integration changes.available_modelsfiltering. Bare supported native IDs route through the configured combo; account-qualified/provider-qualified OpenAI routes remain separate.nativeAliasanddisplayNamein the dashboard editor with localized labels and create/edit/save coverage.Maintainer verification
devatde6f3db97and resolved the fix(catalog): synthesize incomplete combo members with context fallback (#1163) #1305 overlap semantically: current-dev incomplete combo-member synthesis is preserved, while native-alias metadata remains the preferred fallback before the generic synthesized fallback.d5e70a29e. GitHub reports the PR mergeable, and compare againstdevshows 0 commits behind / 4 commits ahead.d5e70a29e.d5e70a29e, satisfying the two non-cancelled same-SHA runs requested because of [Bug] CI: Linux test shards intermittently hang ~15 minutes and are killed, leaving an orphan bun process #1302.-clear semantics and theocx route combocommand alias, and the Russian routing reference explicitly states that native aliases capture only the exact bare id.UI verification
The original implementation was verified in an isolated Codex Desktop instance; the routed native aliases remained in the picker while native OpenAI rows were suppressed as intended.
The screenshot below is historical evidence carried forward from #1056, not a capture of the current #1244 head. A fresh current-head Desktop picker capture has been requested by maintainers and should show bare native alias routing to the combo while account-qualified natives remain separate and provider-qualified OpenAI routes do not fall through.
Security / integrity review
Manual diff review covers alias validation and namespace boundaries, bare-vs-qualified routing separation, catalog backup/recovery trust, disabled-model migration, live/provider discovery isolation, duplicate/suppressed native rows, metadata mutation, and model-ranking behavior. No new secret-bearing or authentication material is introduced by this feature.
Review readiness
This PR is ready for maintainer review. The requested rebase, Russian documentation parity, and two completed non-cancelled same-SHA Cross-platform CI runs are complete. The fresh Desktop screenshot remains requested as stronger UI evidence but was explicitly described as non-blocking.
Supersedes #1056.
Summary by CodeRabbit