fix(catalog): keep failover combos when live discovery omits members

[Wibias]

Summary

• When authoritative provider /models omits configured ids that a combo still targets, retain those rows instead of dropping them from the live catalog.
• Prevents ocx sync from omitting failover combos as "member capabilities are incomplete" (OCX-111 / [Bug]: ocx sync drops configured combo members and omits failover catalog entry #1308), including the openrouter + opencode-go + command-code repro.
• Builds on the fix(catalog): synthesize incomplete combo members with context fallback (#1163) #1305 synthesis fallback; this keeps combo-target configured models visible and selectable too.

Closes #1308

Verification

• bun test ./tests/codex-catalog.test.ts -t "OCX-111|successful live discovery|managed Kimi|combo catalog"
• bun run typecheck

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed.
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

• Bug Fixes
  • Configured combo target models are now retained when live provider discovery returns different model lists.
  • Combo-only target models remain available even when providers do not list them independently.
  • Failover combo models preserve their expected context window and no longer generate unnecessary omission warnings.
  • Model catalogs remain consistent across live, cached, stale, and failed discovery results.
  • Concurrent discovery requests with different combo configurations now return the correct model availability.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: af1c1bb6-1979-4023-ab44-4cb9c99fc110

📥 Commits

Reviewing files that changed from the base of the PR and between 28bbad1 and 5168505.

📒 Files selected for processing (3)

• src/codex/catalog/provider-fetch.ts
• tests/codex-catalog.test.ts
• tests/codex-gather-authority.test.ts

---

📝 Walkthrough

Walkthrough

Configured combo targets are grouped by provider and passed into provider discovery. Discovery retains them across live, cached, stale, and failure paths. Shared merging preserves configured aliases and defaults. Tests cover failover synthesis, warm-cache behavior, and concurrent retention sets.

Changes

Configured combo target retention

Layer / File(s)	Summary
Build and wire provider retention mapping src/codex/catalog/provider-fetch.ts, src/codex/catalog.ts	Provider capture stores configured model sets. Provider graph identity includes retention IDs. The catalog facade re-exports configuredComboTargetModelsByProvider.
Merge configured models across discovery paths src/codex/catalog/provider-fetch.ts	mergeConfiguredModelsIntoLiveCatalog centralizes retention for combo targets, dated aliases, Vertex defaults, and compatibility models. Live and fallback paths reapply current retention while cache entries remain independent of the current combo set.
Validate retention behavior tests/codex-catalog.test.ts, tests/codex-gather-authority.test.ts	Tests verify omitted targets, synthesized failover metadata, 128,000-token capability intersection, warm-cache retention, warning suppression, and separate concurrent gathers.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• lidge-jun/opencodex#1305: Both changes address incomplete combo members in catalog handling.
• lidge-jun/opencodex#1244: Both changes preserve configured combo targets during provider discovery.
• lidge-jun/opencodex#1212: Both changes modify combo-target retention and discovery behavior.

Suggested labels: review-ready

Suggested reviewers: lidge-jun, ingwannu

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: retaining failover combo members when live discovery omits them.
Linked Issues check	✅ Passed	The changes address issue #1308 by retaining configured combo targets across live, cached, stale, and fallback catalog paths, with regression tests.
Out of Scope Changes check	✅ Passed	The catalog changes and regression tests are directly related to retaining configured failover combo members and preventing catalog omission.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@src/codex/catalog/provider-fetch.ts`:
- Around line 1287-1291: Update the live-discovery logic around the
provider-model candidate construction in
src/codex/catalog/provider-fetch.ts#L1287-L1291 to create CatalogModel rows for
IDs in captured.retainConfiguredModelIds, not only entries from prov.models,
applying the available provider configuration hints; in
tests/codex-catalog.test.ts#L1552-L1573, remove a configured target from its
provider models, preserve its metadata through provider hints, and assert both
the provider row and combo/failover remain present.

🪄 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: 249397c5-47a5-47a5-879c-11cd3002ed45

📥 Commits

Reviewing files that changed from the base of the PR and between 4bb249b and e8a21dc.

📒 Files selected for processing (3)

• src/codex/catalog.ts
• src/codex/catalog/provider-fetch.ts
• tests/codex-catalog.test.ts

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/codex/catalog/provider-fetch.ts (2)

463-471: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Add combo retention to the provider collect-flight key.

gatherRoutedModelsWithAuth reuses a pending collect flight when providerGraphIdentity matches, but captureGatherFlight builds that identity from omitProviderTransportExecutor(provider.provider) and omits the per-provider retainConfiguredModelIds set. When the same config is issued again with a different combo target for the same provider, the flight can return cached provider rows built without the new retention IDs. Include a deterministic, sorted copy of prov.retainConfiguredModelIds in the provider collect-flight identity.

🤖 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/provider-fetch.ts` around lines 463 - 471, Update
captureGatherFlight’s provider collect-flight identity to include a
deterministic sorted copy of prov.retainConfiguredModelIds alongside
omitProviderTransportExecutor(prov.provider). Ensure differing retention sets
produce distinct providerGraphIdentity values while identical sets remain
reusable.

---

1297-1301: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Apply combo model retention before every cached or failed result is returned.

configuredComboTargetModelsByProvider passes combo targets into captureProviderGather, and the live /models retain rule uses captured.retainConfiguredModelIds at src/codex/catalog/provider-fetch.ts:1297-1301. The Cursor cache returns at src/codex/catalog/provider-fetch.ts:1093-1095 and stale returns at src/codex/catalog/provider-fetch.ts:1098-1101, while Google stale returns at src/codex/catalog/provider-fetch.ts:1146-1150 and discovery fallbacks at src/codex/catalog/provider-fetch.ts:1162 and src/codex/catalog/provider-fetch.ts:1177 all skip that rule. Add a shared retention helper for cached/stale/failure returns, or invalidate model cache when combo retention changes, and cover this with a warm-cache regression case.

🤖 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/provider-fetch.ts` around lines 1297 - 1301, Ensure
combo-target model retention is applied before every cached, stale, or
discovery-fallback result is returned, not only in the live `/models` path
around `shouldRetainConfiguredProviderModel` and
`captured.retainConfiguredModelIds`. Add a shared retention helper for the
Cursor cache returns, Google stale returns, and discovery fallbacks, or
invalidate the model cache whenever combo retention changes; include a
warm-cache regression case.

🤖 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/catalog/provider-fetch.ts`:
- Around line 463-471: Update captureGatherFlight’s provider collect-flight
identity to include a deterministic sorted copy of prov.retainConfiguredModelIds
alongside omitProviderTransportExecutor(prov.provider). Ensure differing
retention sets produce distinct providerGraphIdentity values while identical
sets remain reusable.
- Around line 1297-1301: Ensure combo-target model retention is applied before
every cached, stale, or discovery-fallback result is returned, not only in the
live `/models` path around `shouldRetainConfiguredProviderModel` and
`captured.retainConfiguredModelIds`. Add a shared retention helper for the
Cursor cache returns, Google stale returns, and discovery fallbacks, or
invalidate the model cache whenever combo retention changes; include a
warm-cache regression case.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7245acb8-e6cf-4ae5-a047-4980aa7e80cc

📥 Commits

Reviewing files that changed from the base of the PR and between e8a21dc and dbdae1e.

📒 Files selected for processing (2)

• src/codex/catalog/provider-fetch.ts
• tests/codex-catalog.test.ts

Dismissed only to correct malformed newline escaping in the review body; the technical findings are unchanged.

[Ingwannu]

The direction is valuable and much closer to the right fix than #1310, but I am requesting changes because the retention policy is not yet part of every catalog reuse boundary.

Two blockers remain on the current head:

1. captureGatherFlight hashes the provider row into providerGraphIdentity but omits retainConfiguredModelIds. Two concurrent gathers with the same provider config and different combo targets can therefore share a flight built for the wrong retention set. Add a deterministic sorted retention list to that identity and test that different sets do not coalesce.

2. The retention rule runs only after a successful live /models response. Fresh cache, stale cache, cooldown, and discovery-failure returns happen earlier and can return a catalog captured before the combo target existed. Apply one shared configured-target retention helper to every cached/fallback return, or make the relevant cache identity/invalidation include the retention set. Add a warm-cache regression that gathers once without the combo and again with it inside the TTL.

I do not require exposing combo-only IDs as standalone provider rows: the existing #1305 synthesis path intentionally keeps those scoped to combo derivation, and the new test correctly asserts no standalone leak. The configured models[] members do need consistent retention across live, cached, stale, and in-flight paths.

Once these two cache/flight boundaries are fixed and exact-head CI is green, this should remain a strong merge candidate for #1308.

[Ingwannu]

Re-reviewed exact head 516850518acc. Both requested catalog reuse boundaries are now covered:

• the sorted combo-retention set participates in providerGraphIdentity, with a concurrent-flight regression;
• one shared retention helper is applied across live, fresh-cache, stale/cooldown, and discovery-fallback returns, while cache writes deliberately avoid freezing combo-only state.

Local exact-head validation passed: TypeScript typecheck and 173 focused catalog/gather tests. GitHub CI, macOS, all Linux shards, packaging smoke checks, hygiene, and CodeRabbit are green. Approved for dev.