Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds normalization logic for the owned_by field of fetched models to map them to the plugin's providerId, preventing OpenCode from creating separate credential-less provider entries. The review feedback suggests removing redundant array length checks before iterating over rawModels to simplify the code, and points out the repository style guide requirement to include unit tests for production code changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // ── Normalize owned_by ──────────────────────────────────────────────── | ||
| // OpenCode auto-discovers providers by grouping models under each unique | ||
| // `owned_by` value from /v1/models. When OmniRoute returns models with | ||
| // `owned_by: "combo"` (or any other non-omniroute value), OpenCode creates | ||
| // a separate provider entry without credentials, causing "No credentials | ||
| // for provider: combo" errors. Fix: normalize all `owned_by` values to the | ||
| // plugin's `providerId` so they stay under the same provider block. |
There was a problem hiding this comment.
According to the repository style guide, tests must always be included when changing production code (such as files under src/). Please add unit tests to verify that the owned_by normalization logic correctly maps various owned_by values to the resolved providerId.
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
| if (rawModels.length > 0) { | ||
| for (const model of rawModels) { | ||
| if (model && typeof model.owned_by === "string" && model.owned_by !== resolved.providerId) { | ||
| model.owned_by = resolved.providerId; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The check if (rawModels.length > 0) is redundant because the for...of loop will naturally not execute if the array is empty. Removing this check simplifies the code and reduces nesting.
for (const model of rawModels) {
if (model && typeof model.owned_by === "string" && model.owned_by !== resolved.providerId) {
model.owned_by = resolved.providerId;
}
}| if (rawModels.length > 0) { | ||
| for (const model of rawModels) { | ||
| if (model && typeof model.owned_by === "string" && model.owned_by !== resolved.providerId) { | ||
| model.owned_by = resolved.providerId; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The check if (rawModels.length > 0) is redundant because the for...of loop will naturally not execute if the array is empty. Removing this check simplifies the code and reduces nesting.
for (const model of rawModels) {
if (model && typeof model.owned_by === "string" && model.owned_by !== resolved.providerId) {
model.owned_by = resolved.providerId;
}
}|
Thanks @herjarsa! The 1. The timeout bump is currently a no-op. You raised the default 2. No tests (repo Hard Rule #18). 3. It's The |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
* chore(release): open v3.8.20 development cycle * fix(images): prefer bare combos over image aliases (cheaper-inference#3527) Integrated into release/v3.8.20 * fix(translator): map Codex local_shell tool (cheaper-inference#3534) Integrated into release/v3.8.20 * fix(usage): make opencode-go quota fetcher fail-open instead of throwing 500 (cheaper-inference#3522) Integrated into release/v3.8.20 * Fix Runtime page breaker state rendering (cheaper-inference#3533) Integrated into release/v3.8.20 * Expose provider breaker degradation threshold setting (cheaper-inference#3535) Integrated into release/v3.8.20 * fix(executor): strip provider prefix from versioned built-in tool model field (cheaper-inference#3532) Integrated into release/v3.8.20 * feat(providers): add Claude Fable 5 support (cheaper-inference#3524) Integrated into release/v3.8.20 * feat(resilience): add global provider cooldown tracking to prevent combo re-walking (cheaper-inference#3556) Integrated into release/v3.8.20 (default OFF, opt-in) * fix(translator): scope thoughtSignature bypass to Antigravity/CLI only (cheaper-inference#3560) Integrated into release/v3.8.20. Co-authored-by: Six7Day <six7day@gmail.com> * fix(routing): normalize thinking:disabled for combo-substituted models that reject it (cheaper-inference#3554) (cheaper-inference#3563) Integrated into release/v3.8.20 * fix(usage): accept 0/empty budget limits so the dashboard can save and clear (cheaper-inference#3537) (cheaper-inference#3564) Integrated into release/v3.8.20 * docs(changelog): credit @Six7Day for cheaper-inference#3560 thoughtSignature fix (cheaper-inference#3414) The cheaper-inference#3560 squash co-author trailer landed inline (unparsed by GitHub), so add an explicit CHANGELOG credit ensuring @Six7Day (original cheaper-inference#3414) and @oyi77 are on the public record for the Gemini thoughtSignature fix. * fix(gamification): dedup badge unlock via user_badges so events don't re-fire every request (cheaper-inference#3472) (cheaper-inference#3565) Integrated into release/v3.8.20 * fix(routing): pass through 'auto' keyword on codex /v1/responses instead of rewriting to codex/auto (cheaper-inference#3509) (cheaper-inference#3566) Integrated into release/v3.8.20 * fix(cli-tools): normalize apiKey null in guide-settings schema so cloud-mode config saves (cheaper-inference#3552) (cheaper-inference#3567) Integrated into release/v3.8.20 * fix(catalog): reclassify PublicAI from keyless to one-time-initial (requires API key) (cheaper-inference#3558) (cheaper-inference#3568) Integrated into release/v3.8.20 * fix(gemini-web): surface missing Playwright browser as actionable 503 + cooldown hint, not a retryable 500 loop (cheaper-inference#3516) (cheaper-inference#3570) Integrated into release/v3.8.20 * fix(security): sanitize raw err.message in web executors + embeddings/search response bodies (Rule #12) (cheaper-inference#3494, cheaper-inference#3495) (cheaper-inference#3573) Integrated into release/v3.8.20 * fix(dashboard): point CustomHostsManager + FeatureFlagsGrid at real routes (cheaper-inference#3486, cheaper-inference#3487) (cheaper-inference#3574) Integrated into release/v3.8.20 * chore(providers): remove dead krutrim entry (cheaper-inference#3483) + docs(api): fix agent-bridge per-agent state route (cheaper-inference#3489) (cheaper-inference#3575) Integrated into release/v3.8.20 * docs(api): correct API_REFERENCE.md paths for skills/plugins/admin/cache/acp/system-info (cheaper-inference#3497) (cheaper-inference#3577) Integrated into release/v3.8.20 * fix(proxy): drive SOCKS5 UI option from runtime ENABLE_SOCKS5_PROXY, not build-time NEXT_PUBLIC (cheaper-inference#3508) (cheaper-inference#3579) Integrated into release/v3.8.20 * fix(playground): filter playground models by node prefix so custom-endpoint models appear (cheaper-inference#3505) (cheaper-inference#3581) Integrated into release/v3.8.20 * fix(usage): show an informative message instead of a blank Kiro quota card when no usage breakdown (cheaper-inference#3506) (cheaper-inference#3582) Integrated into release/v3.8.20 * docs(changelog): add the cheaper-inference#3506 Kiro quota entry (missed in cheaper-inference#3582 due to a stale-base CHANGELOG anchor) (cheaper-inference#3583) Integrated into release/v3.8.20 * fix(auto-update): use stable PROJECT_ROOT walker, not frozen process.cwd() (cheaper-inference#3561) Integrated into release/v3.8.20. Auto-update PROJECT_ROOT now uses a stable __dirname-anchored upward walker instead of the no-op process.cwd() resolver. * fix: address PR cheaper-inference#3518 review comments (lifecycle hooks, regex, indentation, route params) (cheaper-inference#3562) Integrated into release/v3.8.20. Addresses cheaper-inference#3518 review: regex literals, logs/[id] route params (Next 16), indentation, and wires plugin lifecycle hooks (onInstall/onActivate/onDeactivate/onUninstall) in the loader so manager.ts can register them. Adds Rule cheaper-inference#18 regression test. * docs(changelog): credit @ViFigueiredo (cheaper-inference#3423) for PROJECT_ROOT + log cheaper-inference#3561/cheaper-inference#3562 (v3.8.20) * fix: openai to gemini incorrectly translates historical tool calls into text (cheaper-inference#3569) Integrated into release/v3.8.20. Standard Gemini direct path now maps historical tool calls to native functionCall/functionResponse parts (signaturelessToolCallMode: native) instead of inert text — validated against the real Gemini API (gemini-2.5-flash returns 200 for signatureless native functionCall, even with tools+thinking; Hard Rule cheaper-inference#18). Eliminates the text-serialization leak. Antigravity/CLI sentinel path (cheaper-inference#3560) untouched. * docs(changelog)+test: reconcile standard-Gemini native mode (cheaper-inference#3569) — update round-2 rationale comment + log VPS validation * docs(changelog): reconcile v3.8.20 — add 9 missing bullets + move [Unreleased] to versioned section * docs(changelog): complete v3.8.20 reconciliation — 27 bullets, 11 contributors --------- Co-authored-by: Alexander Averyanov <alex@averyan.ru> Co-authored-by: Hakan Kurşun <bykamaka@gmail.com> Co-authored-by: Wilson <pedbookmed@gmail.com> Co-authored-by: Randi <55005611+rdself@users.noreply.github.com> Co-authored-by: Giorgos Giakoumettis <giorgos@yiakoumettis.gr> Co-authored-by: PizzaV <103120356+pizzav-xyz@users.noreply.github.com> Co-authored-by: Paijo <14921983+oyi77@users.noreply.github.com> Co-authored-by: Markus Hartung <mail@hartmark.se>
…rence#3587) Reasoning models (deepseek-v4-flash, nemotron, etc.) consume ALL max_tokens for reasoning_tokens, leaving content empty. The fix: 1. Detects reasoning models via supportsReasoning() before sending requests 2. Adds 50% + 1000 token buffer to max_tokens for reasoning models 3. Detects reasoning-only empty content in validateResponseQuality() 4. Applies buffer in both main combo loop and round-robin handler
aaffc95 to
52e52ba
Compare
|
Superseded by #3588 — this branch was force-pushed with a clean commit history |
Problem
Three related issues cause failures in OpenCode when using the OmniRoute plugin:
1. "No credentials for provider: combo"
OpenCode auto-discovers providers by grouping models under each unique
owned_byvalue from/v1/models. When OmniRoute returns models withowned_by: "combo", OpenCode creates a separate provider entry without credentials:2. "Unable to determine provider for model"
OpenCode parses model IDs like
combo/masterand extractscomboas the provider name, ignoring the plugin'sproviderID: "omniroute". This causes authentication failures and the error:3. AbortError on slow connections
The default 10s timeout for
/v1/models,/api/combos, and other fetch endpoints causesAbortErroron WSL and other slow network setups where responses can exceed 270KB and take >10s to transfer.Root Cause
owned_by mismatch: The
/v1/modelsendpoint returns models with differentowned_byvalues (e.g.,combo,opencode-zen,kilocode). OpenCode's auto-discovery creates separate providers for each, but onlyomniroutehas credentials.Combo prefix mismatch:
buildComboKeygenerates model IDs likecombo/master. OpenCode's AI SDK parses this and tries to route to acomboprovider, which doesn't exist.Timeout too aggressive: 10s timeout is insufficient for large responses on slow networks.
Solution
Fix 1: Normalize owned_by
Normalize all
owned_byvalues to the plugin'sproviderId(e.g.,omniroute) after fetching from/v1/models. This ensures all models stay under the same provider block with valid credentials.Fix 2: Use omniroute/ prefix for combos
Change
buildComboKeyto generateomniroute/{slug}instead ofcombo/{slug}. This ensures OpenCode's AI SDK correctly routes combo models to the registeredomnirouteprovider with valid credentials.Fix 3: Increase timeouts
Increase default timeouts from 10s to 60s for main fetchers, and from 5s to 30s for enrichment/autoCombos.
Changes
createOmniRouteConfigHookafter fetching modelscreateOmniRouteProviderHookfor consistencybuildComboKeyto useomniroute/prefix instead ofcombo/Testing
/v1/modelsresponseImpact
This is a backward-compatible fix that doesn't change the plugin's API or behavior for users who don't experience these issues. The normalization only affects the internal
owned_byfield used by OpenCode's auto-discovery mechanism.