[Bug]: Preserve operator-owned static OAuth catalogs during reconciliation

[yusoofsh]

Summary

• Preserve provider model and capability metadata when an operator explicitly sets liveModels: false.
• Prevent OAuth startup reconciliation from replacing an operator-owned static catalog with the registry preset.
• Keep legacy Antigravity migration behavior unchanged.
• Add regression coverage for a customized Command Code static OAuth catalog.
• Fixes [Bug]: ocx sync drops configured combo members and omits failover catalog entry #1308.

Verification

• bun run test tests/oauth-provider-reconcile.test.ts — 6 passed.
• bun run typecheck — passed.
• bun run privacy:scan — passed.
• git diff --check — passed.
• The full bun run test suite was started through Nub and reached broad coverage, but was stopped after unrelated GUI dependency warnings for missing react packages in the checkout; the focused reconciliation test and all relevant reported tests passed.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (not user-facing API; no docs change needed).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Contributor readiness

• Local focused CI is green.
• Branch is based on the latest available dev checkout.
• All correct Codex and CodeRabbit findings are fixed.
• Ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Preserved operator-configured OAuth model catalogs during preset reconciliation.
  • Prevented static model settings and context-window mappings from being unintentionally modified.
  • Reconciliation now correctly reports no changes when these catalogs are already configured.

[coderabbitai]

📝 Walkthrough

Walkthrough

OAuth reconciliation now detects operator-owned static catalogs with liveModels === false, skips preset reconciliation for those providers, and verifies that configured models and context-window mappings remain unchanged.

Changes

Static OAuth catalog preservation

Layer / File(s)	Summary
Detect, skip, and validate static catalogs src/oauth/index.ts, tests/oauth-provider-reconcile.test.ts	The reconciliation logic identifies operator-owned static catalogs and skips OAuth preset reconciliation. The test verifies that custom command-code models and context-window mappings remain unchanged and that no modification is reported.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• lidge-jun/opencodex#406: Introduced provider registry metadata used by static OAuth catalog handling.
• lidge-jun/opencodex#652: Defined liveModels discovery semantics in src/oauth/index.ts.
• lidge-jun/opencodex#1178: Extended OAuth reconciliation handling for static catalogs and configured mappings.

Suggested labels: bug

Suggested reviewers: lidge-jun, wibias, luvs01

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change preserves configured static provider models and metadata, which supports retaining failover combo members when live discovery is incomplete [#1308].
Out of Scope Changes check	✅ Passed	The source change and regression test directly address static OAuth catalog reconciliation and contain no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes preserving operator-owned static OAuth catalogs during reconciliation, which is the main change.

✨ 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.}

[Wibias]

Verdict: Request changes

The Command Code fix is useful and the diff is small, but this is not safe to merge as-is.

Blocker — liveModels: false is not reliable operator-ownership provenance

isOperatorOwnedStaticCatalog() now treats every provider.liveModels === false as operator-owned and skips all OAuth preset reconciliation.

That breaks Kiro. Kiro is an OAuth provider whose registry preset itself intentionally sets liveModels: false because its CodeWhisperer wire has no compatible live /models endpoint. deriveOAuthProviderConfig() materialises that registry value into the saved provider config. Therefore a normal, untouched Kiro config now looks “operator-owned” and will hit the early continue forever.

Result: existing Kiro users stop receiving future registry updates to its static model list and associated context/reasoning metadata. This directly conflicts with the Kiro preset's purpose: its static catalog is authoritative and must be maintained by OpenCodex as models/capabilities change.

Please distinguish registry-owned false from operator-owned false. At minimum, false cannot mean operator-owned when the current OAuth preset also has liveModels: false. If operators must also be able to freeze/customise Kiro's catalog independently, this needs explicit ownership/provenance rather than overloading the discovery flag.

Contract risk — the guard freezes much more than model discovery

The early continue skips every field in OAUTH_RECONCILE_FIELDS, including context/output limits, input modalities, reasoning maps, noVisionModels, parameter restrictions, etc. Public config semantics describe liveModels as the switch for live catalog discovery; this patch silently turns liveModels: false into an opt-out from future bundled capability maintenance.

That can leave capability metadata stale after upgrades. In particular, stale vision metadata can change whether requests use native vision or the vision sidecar, which has behaviour/cost/privacy implications. I found no direct secret leak or auth bypass in this diff, but freezing these capability facts is too broad without an explicit contract.

Tests needed

Please add coverage for at least:

• a normal Kiro registry-owned liveModels: false config with stale models/context/reasoning metadata still reconciling to the current preset;
• the intended Command Code operator-owned static catalog remaining untouched;
• the intended policy for the other capability fields currently skipped by the early continue (and whether stale defaultModel healing should also be skipped).

CI on the current head is green, including the test shards, typecheck, GUI tests, privacy scan and macOS job. I also found no new resource-retention/concurrency issue in the two-file diff. The blocker is semantic/provenance correctness, not CI hygiene.

Verdict: REQUEST CHANGES. Fix the ownership distinction before merge.