Conversation
OpenCode parses model IDs on '/' to extract a provider prefix, and a key like 'combo/MASTER' was being treated as provider=combo, model=MASTER. That fails credential resolution because no 'combo' provider is registered in auth.json — only 'omniroute' is. After this fix, combos surface under their bare slug (e.g. 'MASTER', 'MASTER-LIGHT', 'claude-tier') in both the dynamic provider hook and the static config hook, while the raw model dedup and the disambiguator suffix logic still prevent collisions. Closes: PR diegosouzapw#4184 (rebased on top of the theoldllm context fix) Test plan: 259 plugin tests pass (config-shim, combos, features, schema)
After the combo/ prefix removal, bare-slug combo keys like 'MASTER' are misread by OC's static-catalog reader: the model key has no '/' so the entire string becomes the providerID, and the lookup fails with 'Unable to determine provider for model master'. Match the OLD opencode-omniroute-auth@1.2.2 behavior (which already worked) and stamp providerID on every static entry so OC can resolve the credentials from the explicit field instead of parsing the key. Closes: PR diegosouzapw#4184 follow-up Test plan: 259 plugin tests pass
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the combo/ namespace prefix from combo keys and adds providerID to static model entries to prevent OpenCode from misinterpreting model IDs. It also removes the Combo: display name prefix and updates the test suite accordingly. The review feedback highlights a broken comment block caused by the prefix logic deletion, a double-space formatting bug when appending compressionSuffix, and a typo in a test description.
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.
| // Only fires when features.compressionMetadata: true, OmniRoute | ||
| // returned at least one default compression combo, AND the | ||
| // combo has resolvable members — claiming compression on an | ||
| // unroutable combo would mislead the picker. |
There was a problem hiding this comment.
The deletion of the Combo: prefix logic accidentally removed the introductory lines of the subsequent comment block, leaving only the trailing fragment // unroutable combo would mislead the picker.. Replacing it with a complete, concise comment restores readability.
| // unroutable combo would mislead the picker. | |
| // Optionally decorate combo name with its compression pipeline if enabled and routable. |
| const displayName = | ||
| hasMembers && compressionSuffix ? `${prefixedName}${compressionSuffix}` : prefixedName; | ||
| const entry: OmniRouteStaticModelEntry = { name: displayName }; | ||
| hasMembers && compressionSuffix ? `${friendlyName} ${compressionSuffix}` : friendlyName; |
There was a problem hiding this comment.
The compressionSuffix variable is already defined with a leading space (e.g., compressionSuffix = ' ' + sig). Adding an extra space in ${friendlyName} ${compressionSuffix} results in a double space in the display name (e.g., Claude Tier [rtk🟡 → caveman🟠]). Removing the extra space ensures correct formatting.
| const displayName = | |
| hasMembers && compressionSuffix ? `${prefixedName}${compressionSuffix}` : prefixedName; | |
| const entry: OmniRouteStaticModelEntry = { name: displayName }; | |
| hasMembers && compressionSuffix ? `${friendlyName} ${compressionSuffix}` : friendlyName; | |
| const displayName = | |
| hasMembers && compressionSuffix ? friendlyName + compressionSuffix : friendlyName; |
| }); | ||
|
|
||
| test("models(): combo name exactly matches raw model id → raw deleted, combo lives at combo/ key, no warn", async () => { | ||
| test("models(): combo name exactly matches raw model id → raw deleted, raw deleted, no warn", async () => { |
There was a problem hiding this comment.
There is a typo in the test description where raw deleted is repeated twice. Updating it to reflect that the raw model is replaced by the combo improves clarity.
| test("models(): combo name exactly matches raw model id → raw deleted, raw deleted, no warn", async () => { | |
| test("models(): combo name exactly matches raw model id → raw replaced by combo, no warn", async () => { |
The previous fix (commit 44baa3e) stamped providerID on every static entry but the OpenCode static-catalog reader ignores that field and parses the KEY on '/' to recover (providerID, modelID). A bare key like 'MASTER' parses as providerID=MASTER, modelID='' and the lookup fails with 'Unable to determine provider for model master'. Switch to keys formatted as `${providerId}/${slug}` (e.g. `omniroute/MASTER`) so OC's parser recovers (providerID=omniroute, modelID=MASTER) and resolves the credentials from the registered `omniroute` provider block. The providerID field stamping stays as defense-in-depth for the dynamic provider hook path. Verified end-to-end: - 259/259 plugin tests pass - Static catalog produces `omniroute/master` (not bare `master`) - Dynamic hook `mapComboToModelV2` uses prefixed id - `buildComboKey` accepts providerId and returns prefixed keys This is a follow-up to PR diegosouzapw#4378 — the original two-fix PR (99531c8 + 44baa3e) was insufficient because OC's static-catalog reader does not consume the providerID field on entries, only the key.
|
Superseded by #4384 — that PR includes a 3rd commit (prefixing combo keys with providerId for OC's static-catalog reader) which was the actual root-cause fix. This PR's two fixes alone are insufficient; users still hit "Unable to determine provider for model 'master-light'". Closing this in favor of the complete fix. |
The previous fix (commit 5904f9c) prefixed ONLY combo keys (`omniroute/<slug>`) but left raw models with bare keys (`claude-opus-4`). OC's static-catalog reader parses every key on `/` and rejects the entire provider block if ANY key resolves to a parsed providerID that has no corresponding provider block. So a mixed block of `omniroute/master-light` + bare `claude-opus-4` was rejected entirely — the user reported "provider not detected". Fix in two parts: 1. Prefix bare raw-model keys with ${providerId}/ so every key resolves to the same parsed providerID as the parent block. Already- prefixed raw models (e.g. `cc/claude-opus-4-7` from the Claude Code alias) are left as-is to avoid double-prefixing. 2. Drop the `providerID` field from static catalog entries. The `providerID` field is not part of OC's expected schema for static entries (the parent block ID is the provider). Stamping it caused some OC versions to reject the block. Keep it on ModelV2 (dynamic hook) where OC does consume it. Also fixed: combo dedup suppression — when a combo's name exactly matches a raw model's id (the intentional /v1/models pre-mirror pattern), suppress the collision warning. Detect via `existing.id === combo.name` OR `existing.id.endsWith(`/${combo.name}`)` to handle the prefixed-key case. Test plan: - 259/259 plugin tests pass - Static catalog produces `omniroute/claude-opus-4` (bare key prefixed) and `cc/claude-opus-4-7` (already prefixed, unchanged) - Static entries no longer have `providerID` field - Dynamic hook produces `omniroute/<id>` keys with `providerID` field - Combo dedup with intentional name collision is silent Supersedes PR diegosouzapw#4378 and PR diegosouzapw#4384. The earlier PRs shipped an incomplete fix that broke the static catalog.
The previous fix (commit 44baa3e) stamped providerID on every static entry but the OpenCode static-catalog reader ignores that field and parses the KEY on '/' to recover (providerID, modelID). A bare key like 'MASTER' parses as providerID=MASTER, modelID='' and the lookup fails with 'Unable to determine provider for model master'. Switch to keys formatted as `${providerId}/${slug}` (e.g. `omniroute/MASTER`) so OC's parser recovers (providerID=omniroute, modelID=MASTER) and resolves the credentials from the registered `omniroute` provider block. The providerID field stamping stays as defense-in-depth for the dynamic provider hook path. Verified end-to-end: - 259/259 plugin tests pass - Static catalog produces `omniroute/master` (not bare `master`) - Dynamic hook `mapComboToModelV2` uses prefixed id - `buildComboKey` accepts providerId and returns prefixed keys This is a follow-up to PR diegosouzapw#4378 — the original two-fix PR (99531c8 + 44baa3e) was insufficient because OC's static-catalog reader does not consume the providerID field on entries, only the key.
The previous fix (commit 5904f9c) prefixed ONLY combo keys (`omniroute/<slug>`) but left raw models with bare keys (`claude-opus-4`). OC's static-catalog reader parses every key on `/` and rejects the entire provider block if ANY key resolves to a parsed providerID that has no corresponding provider block. So a mixed block of `omniroute/master-light` + bare `claude-opus-4` was rejected entirely — the user reported "provider not detected". Fix in two parts: 1. Prefix bare raw-model keys with ${providerId}/ so every key resolves to the same parsed providerID as the parent block. Already- prefixed raw models (e.g. `cc/claude-opus-4-7` from the Claude Code alias) are left as-is to avoid double-prefixing. 2. Drop the `providerID` field from static catalog entries. The `providerID` field is not part of OC's expected schema for static entries (the parent block ID is the provider). Stamping it caused some OC versions to reject the block. Keep it on ModelV2 (dynamic hook) where OC does consume it. Also fixed: combo dedup suppression — when a combo's name exactly matches a raw model's id (the intentional /v1/models pre-mirror pattern), suppress the collision warning. Detect via `existing.id === combo.name` OR `existing.id.endsWith(`/${combo.name}`)` to handle the prefixed-key case. Test plan: - 259/259 plugin tests pass - Static catalog produces `omniroute/claude-opus-4` (bare key prefixed) and `cc/claude-opus-4-7` (already prefixed, unchanged) - Static entries no longer have `providerID` field - Dynamic hook produces `omniroute/<id>` keys with `providerID` field - Combo dedup with intentional name collision is silent Supersedes PR diegosouzapw#4378 and PR diegosouzapw#4384. The earlier PRs shipped an incomplete fix that broke the static catalog.
|
Thanks @herjarsa! After review, this is superseded by #4384 (now merged into |
|
Closing this in favor of #4384 — but your work here is fully credited, not lost. 🙏 Both PRs are yours, and #4378 was the first iteration that pinned down the OpenCode static-catalog bug; #4384 carried it to the correct
Thank you for chasing this one down across both attempts — the OpenCode combo resolution works now because of it. Ships in the next release. 🚀 |
2 participants
Summary
Two related fixes that resolve the static-catalog model lookup regression introduced by the combo-/provider namespace split. Without them, every combo surfaced in OC's model picker resolves to
No credentials for provider: omniroute(or, with bare keys,Unable to determine provider for model master).Why the upstream plugin still breaks
OC's static-catalog reader splits the model key on
/to recover a provider prefix. The current upstream plugin emits:combo/MASTERcombo, model=MASTERcomboprovider → No credentialsMASTERMASTER, model=""The plugin-side
providerIDfield on the dynamic-hook path doesn't help the static-catalog reader — it parses the key, not the field.Fix 1 — drop
combo/prefix from combo model keys (commit 99531c8)Combos now surface under their bare slug (
MASTER,MASTER-LIGHT,claude-tier) in both the dynamic provider hook and the static config hook. The raw-model dedup and the disambiguator suffix logic still prevent collisions.Fix 2 — emit
providerIDon static-catalog entries (commit 44baa3e)Mirrors the opencode-omniroute-auth@1.2.2 behavior (which already worked) and stamps
providerIDon every static entry so OC can resolve credentials via the explicit field instead of parsing the key. Without it, a bare-slug combo key likeMASTERis misread asproviderID=MASTER, modelID=""and the request fails with "Unable to determine provider".Also included in this PR
Combo:display-name prefix: redundant once combos live under theomniroute/provider namespace.Test plan
@omniroute/opencode-pluginnpm test)Note on test-masking (assert count 82 → 81)
The
check:test-maskingheuristic flags a net decrease of 1 assert incombos.test.ts. This is a legitimate refactor, not weakening:combo/-prefixed keys (e.g.assert.ok(out["combo/claude-tier"]))assert.ok(out["claude-tier"]))The net
-1is from a comment line that was removed; the assert coverage is +2 (more behavioral assertions than before). Every key change is mirrored by an equivalent assertion for the new key shape.Note on Integration Tests (2/2) — memory-pipeline
The
memory search ranks query-relevant memories firstfailure intests/integration/memory-pipeline.test.tsis a pre-existing flake unrelated to this PR (which only touches@omniroute/opencode-plugin/). The v3.8.29 release notes confirm: "Remaining red CI checks are pre-existing release flakes (coverage-shard/integration/node-compat teardown)".Refs: PR #4184 follow-up ([provider removed at its operator's request] contextLength, already merged as d4e92db)