Repository navigation
fix: catalog effort ladders, v2 pin precedence, and combo owned_by (#2410 #2409 #2401) - #2630
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
Warning Review limit reachedNext included review available in 46 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
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 |
⏳ DRAFT
What to do
Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required |
![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: 461eddd363
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (next && requiresGlobalV2Disabled(cfg.multiAgentMode, true)) { | ||
| const transition = transitionMultiAgentV2(false, enabled => runCodexFeatures(enabled ? "enable" : "disable", deps)); |
There was a problem hiding this comment.
When a user has configured features.multi_agent_v2.max_concurrent_threads_per_session, enabling the hybrid pin calls transitionMultiAgentV2(false). That helper treats the change as a complete switch to V1: it deletes the V2 limit and writes a translated [agents].max_threads value. Routed catalog rows remain explicitly V2 in hybrid mode, so their configured concurrency limit disappears and they fall back to the upstream default. Disable the global override without removing the V2-specific limit, retaining limits for both surfaces while the hybrid is active.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| const requestedFlag = wantsFlag | ||
| ? body.enabled as boolean | ||
| : modeFlag ?? (wantsKeepNative && hybridPinActive ? false : undefined); |
There was a problem hiding this comment.
With multiAgentMode: "v2" and the hybrid pin active, the dashboard clears the pin by sending only keepNativeChatGptOnV1: false. Here hybridPinActive becomes false, so this expression produces undefined and no transition runs, leaving the global feature disabled even though ordinary V2 mode normally enables it; the CLI's keep-native-v1 off path has the same one-directional behavior. Users must unexpectedly reselect V2 or run ocx v2 on to restore the standard state, so reconcile the flag in both directions when the pin changes.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
…idge-jun#2410 lidge-jun#2409 lidge-jun#2401) (lidge-jun#2630) * fix(catalog): expose opencode-go reasoning efforts * fix(models): advertise combo rows via OpenAI adapter * fix(codex): honor native v1 hybrid pin
…idge-jun#2410 lidge-jun#2409 lidge-jun#2401) (lidge-jun#2630) * fix(catalog): expose opencode-go reasoning efforts * fix(models): advertise combo rows via OpenAI adapter * fix(codex): honor native v1 hybrid pin
Summary
Three community bug issues, each verified and falsified independently before merging.
#2410 —
reasoningEffortsmissing from the live catalog for newer opencode-go modelsThe reporter suspected a static table lagging new models, and that was right.
gpt-5.6-lunaandqwen3.8-maxwere missing from opencode-go's registry map while the same model ids exposed full effort ladders under OpenAI API and Alibaba Token Plan. Tooling infers "supports no efforts" from an empty ladder (src/server/management/shared.ts:260), so agent harnesses selecting programmatically skipped these models.Filling today's holes alone would have left the asymmetry undetectable, so this also adds a cross-provider parity check — a same-model ladder mismatch now fails a test instead of waiting for the next report. The generated-metadata pipeline cannot supply this: it discards source
thinkingladders and keeps only a booleanreasoningflag (scripts/generate-model-metadata.ts:69).#2409 —
keepNativeChatGptOnV1ineffective when global V2 is enabledOpenCodex was already writing the catalog split correctly. The precedence is in the Codex client:
multi_agent_version_override()returns V2 whenever the global feature is on, beforemulti_agent_version_for_model()ever consults the row pin. So no amount of catalog stamping could fix this — the reporter's fresh-process A/B was measuring a client-side override winning.Rather than write a row that cannot take effect, the pin and the global flag are now reconciled where OpenCodex actually owns the decision: enabling the pin disables the global override before syncing,
ocx v2 mode v2honours an existing pin, and a laterocx v2 onis rejected with an explanation of the precedence instead of silently producing the broken hybrid. Same contract onPUT /api/v2.A user who set
keepNativeChatGptOnV1and gotunreadable_encrypted_agent_taskdeserves to be told why — that is the actual fix here.#2401 — combo rows expose
owned_by: "combo""combo"is not a real provider, so OpenAI-compatible clients that select a dispatch adapter byowned_byfail (registration.adapter.prepareCall is not a function). Combo routing itself was fine; only the metadata was wrong.The canonical catalog row keeps
provider: "combo"— internal sync and cleanup depend on that marker, and changing it would break working behavior. Only the plain/v1/modelsserialization changes:owned_by: "openai"because the endpoint is OpenAI-compatible regardless of which target a failover picks, plusis_combo: trueso the dashboard still classifies aliases correctly.Named tradeoff:
owned_bynow describes the compatible inbound adapter rather than literal ownership. The primary target was rejected as the value because it becomes false after failover and would select the wrong wire adapter — worse than the current bug, silently.Verification
Each fix was falsified separately: removing the registry rows reddens the ladder and parity tests; removing the v2 precedence hunks reddens four cases (pin left global on, explicit enable accepted,
mode v2re-enabling, API reportingenabled: true); reverting the serialization reddens both the server row shape and the GUI classification.Checklist
devCloses #2410. Closes #2409. Closes #2401.