From 2981cdb5a9a219ebff569a88b7b40f01630bf68f Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Wed, 26 Aug 2026 13:15:31 -0300 Subject: [PATCH] fix(combos): use the routing-alias prefix, not raw providerId, when serializing combo model steps (#11433) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildPrecisionComboModelStep() rebuilt a step's `model` field as `${providerId}/${modelId}` from the canonical provider id. For the no-auth "OpenCode Free" provider (id "opencode") this produced "opencode/", but "opencode" also doubles as a manual routing-prefix override (open-sse/services/model.ts) that resolves to the unrelated paid "OpenCode Zen" provider. So every step built this way silently misrouted, even though step.providerId correctly said "opencode". Thread the provider's already-computed routing-alias prefix (e.g. "oc") through an optional modelPrefix param, and pass it from the three affected call sites: the precision single-select picker and handleAddBuilderStep in combos/page.tsx, buildGlobalModelList (global search), and buildManualComboModelStep (manual "oc/" entry, which now preserves the typed prefix instead of collapsing it back to the canonical id). step.providerId keeps carrying the canonical id unconditionally, so routing/duplicate-detection identity is unaffected. findNextSuggestedConnectionId is unaffected since its duplicate check keys off entry.providerId, not the parsed model prefix. ALIAS_TO_PROVIDER_ID / resolveProviderAlias() / the routingPrefix computation in builderOptions.ts are untouched — they were already correct (#2901) and are the source of truth this fix threads through. --- .../fixes/11433-opencode-alias-collision.md | 1 + src/app/(dashboard)/dashboard/combos/page.tsx | 6 ++ src/lib/combos/builderDraft.ts | 29 ++++++- tests/unit/combo-builder-draft.test.ts | 8 +- ...sue-11433-opencode-alias-collision.test.ts | 83 +++++++++++++++++++ 5 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 changelog.d/fixes/11433-opencode-alias-collision.md create mode 100644 tests/unit/issue-11433-opencode-alias-collision.test.ts diff --git a/changelog.d/fixes/11433-opencode-alias-collision.md b/changelog.d/fixes/11433-opencode-alias-collision.md new file mode 100644 index 0000000000..abc21d3a16 --- /dev/null +++ b/changelog.d/fixes/11433-opencode-alias-collision.md @@ -0,0 +1 @@ +- **fix(combos):** the combo builder's precision-select, global-model-search, and manual-entry flows now serialize a model step's `model` string using the provider's already-computed routing-alias prefix (e.g. `oc/`) instead of rebuilding it from the raw canonical `providerId`, fixing the no-auth "OpenCode Free" provider (`opencode`) being routed to the unrelated paid "OpenCode Zen" provider (`opencode-zen`) because `opencode` doubles as a manual routing-prefix override ([#11433](https://github.com/diegosouzapw/OmniRoute/issues/11433)). diff --git a/src/app/(dashboard)/dashboard/combos/page.tsx b/src/app/(dashboard)/dashboard/combos/page.tsx index 04f73d54a0..df2d30f9a8 100644 --- a/src/app/(dashboard)/dashboard/combos/page.tsx +++ b/src/app/(dashboard)/dashboard/combos/page.tsx @@ -2177,6 +2177,9 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo builderConnectionId !== COMBO_BUILDER_AUTO_CONNECTION ? builderConnectionId : null, connectionLabel: selectedBuilderConnection?.label || null, allowedConnectionIds: builderEffectiveAllowedConnectionIds, + // #11433: use the already-corrected routing prefix (e.g. "oc" for + // OpenCode Free) instead of letting it default to the raw providerId. + modelPrefix: parseQualifiedModel(selectedBuilderModel.qualifiedModel)?.providerId, }) : null; const builderHasDuplicate = @@ -2501,6 +2504,9 @@ function ComboFormModal({ isOpen, combo, onClose, onSave, activeProviders, combo builderConnectionId !== COMBO_BUILDER_AUTO_CONNECTION ? builderConnectionId : null, connectionLabel: selectedBuilderConnection?.label || null, allowedConnectionIds: builderEffectiveAllowedConnectionIds, + // #11433: use the already-corrected routing prefix (e.g. "oc" for + // OpenCode Free) instead of letting it default to the raw providerId. + modelPrefix: parseQualifiedModel(selectedBuilderModel.qualifiedModel)?.providerId, }); if (hasExactModelStepDuplicate(models, nextStep)) { diff --git a/src/lib/combos/builderDraft.ts b/src/lib/combos/builderDraft.ts index 1a4a093118..3e4c7d7606 100644 --- a/src/lib/combos/builderDraft.ts +++ b/src/lib/combos/builderDraft.ts @@ -83,6 +83,7 @@ export function buildPrecisionComboModelStep({ connectionLabel, allowedConnectionIds = null, weight = 0, + modelPrefix, }: { providerId: string; modelId: string; @@ -91,9 +92,22 @@ export function buildPrecisionComboModelStep({ /** #3266: account allowlist scoping round-robin to a subset of connections. */ allowedConnectionIds?: string[] | null; weight?: number; + /** + * #11433: the routing-prefix segment to serialize into `model` (e.g. "oc" + * for the no-auth OpenCode Free provider), when it differs from the + * canonical `providerId`. Some canonical provider ids collide with an + * unrelated manual `ALIAS_TO_PROVIDER_ID` routing override (`opencode` → + * `opencode-zen`), so reconstructing `model` from the raw `providerId` + * alone can round-trip to the wrong provider on request routing. Falls + * back to `providerId` when omitted/blank. `step.providerId` always stays + * the canonical id regardless, so routing/duplicate-detection identity is + * unaffected. + */ + modelPrefix?: string | null; }): ComboModelStep { const normalizedProviderId = toTrimmedString(providerId) || "provider"; const normalizedModelId = toTrimmedString(modelId) || "model"; + const normalizedModelPrefix = toTrimmedString(modelPrefix) || normalizedProviderId; const normalizedConnectionId = toTrimmedString(connectionId); const normalizedConnectionLabel = toTrimmedString(connectionLabel); // A pinned single connection wins over an allowlist, so only carry the allowlist @@ -110,7 +124,7 @@ export function buildPrecisionComboModelStep({ return { kind: "model", providerId: normalizedProviderId, - model: `${normalizedProviderId}/${normalizedModelId}`, + model: `${normalizedModelPrefix}/${normalizedModelId}`, ...(normalizedConnectionId ? { connectionId: normalizedConnectionId } : {}), ...(normalizedConnectionLabel ? { label: normalizedConnectionLabel } : {}), ...(normalizedAllowed.length > 0 ? { allowedConnectionIds: normalizedAllowed } : {}), @@ -160,10 +174,15 @@ export function buildManualComboModelStep({ const providerId = resolveComboBuilderProviderId(parsed.providerId, providers); if (!providerId) return null; + // #11433: preserve the user-typed prefix (e.g. "oc") as the routing prefix + // instead of letting buildPrecisionComboModelStep rebuild `model` from the + // resolved canonical providerId, which can collide with an unrelated + // manual alias override (e.g. "opencode" -> "opencode-zen"). return buildPrecisionComboModelStep({ providerId, modelId: parsed.modelId, weight, + modelPrefix: parsed.providerId, }); } @@ -225,7 +244,7 @@ type ComboBuilderGlobalProvider = { displayName?: unknown; connectionCount?: unknown; connections?: unknown[]; - models?: Array<{ id?: unknown; name?: unknown }>; + models?: Array<{ id?: unknown; name?: unknown; qualifiedModel?: unknown }>; }; /** @@ -252,12 +271,18 @@ export function buildGlobalModelList( const modelId = toTrimmedString(model?.id); if (!modelId) return; const modelName = toTrimmedString(model?.name) || modelId; + // #11433: derive the routing prefix from the model's already-corrected + // `qualifiedModel` (e.g. "oc/" for the OpenCode Free provider) + // instead of defaulting to the raw providerId, which can collide with + // an unrelated manual alias override. + const modelPrefix = parseQualifiedModel(model?.qualifiedModel)?.providerId || providerId; const step = buildPrecisionComboModelStep({ providerId, modelId, connectionId: null, connectionLabel: null, allowedConnectionIds: [], + modelPrefix, }); list.push({ providerId, diff --git a/tests/unit/combo-builder-draft.test.ts b/tests/unit/combo-builder-draft.test.ts index 1f74545202..6acf0d6da7 100644 --- a/tests/unit/combo-builder-draft.test.ts +++ b/tests/unit/combo-builder-draft.test.ts @@ -42,6 +42,12 @@ test("buildPrecisionComboModelStep preserves provider/model/account triple", () }); test("buildManualComboModelStep resolves provider aliases and uses dynamic account", () => { + // #11433: `providerId` resolves to the canonical id ("codex") for + // duplicate-detection/routing identity, but the serialized `model` string + // now preserves the user-typed prefix ("cx/") verbatim instead of + // collapsing back to the canonical id — some canonical ids (e.g. + // "opencode") collide with an unrelated manual routing-alias override, so + // rebuilding `model` from the canonical id alone can silently misroute. assert.deepEqual( builderDraft.buildManualComboModelStep({ value: "cx/gpt-5.5", @@ -50,7 +56,7 @@ test("buildManualComboModelStep resolves provider aliases and uses dynamic accou { kind: "model", providerId: "codex", - model: "codex/gpt-5.5", + model: "cx/gpt-5.5", weight: 0, } ); diff --git a/tests/unit/issue-11433-opencode-alias-collision.test.ts b/tests/unit/issue-11433-opencode-alias-collision.test.ts new file mode 100644 index 0000000000..674a25c592 --- /dev/null +++ b/tests/unit/issue-11433-opencode-alias-collision.test.ts @@ -0,0 +1,83 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + buildPrecisionComboModelStep, + buildGlobalModelList, + buildManualComboModelStep, +} from "../../src/lib/combos/builderDraft.ts"; +import { resolveProviderAlias, parseModel } from "../../open-sse/services/model.ts"; + +// Issue #11433: the combo builder's precision-select path builds a step's +// `model` string as `${providerId}/${modelId}` using the CANONICAL provider id. +// For the no-auth "opencode" (OpenCode Free) provider this produces +// `model: "opencode/"`, but `opencode` is ALSO a manual routing-prefix +// override (`ALIAS_TO_PROVIDER_ID["opencode"] = "opencode-zen"`) intended only +// for user-typed `opencode/` prefixes referring to the OpenCode Zen (api-key) +// tier. Parsing the step's own `model` string therefore resolves to a +// DIFFERENT provider than the one recorded in `step.providerId`. + +test('sanity: resolveProviderAlias("opencode") is the manual override causing the collision', () => { + // Documents the root cause directly: the manual alias override in + // open-sse/services/model.ts unconditionally rewrites "opencode" to + // "opencode-zen", even though "opencode" is also a registered canonical + // provider id (src/shared/constants/providers/noauth.ts). + assert.equal(resolveProviderAlias("opencode"), "opencode-zen"); +}); + +test("issue #11433 fix: buildPrecisionComboModelStep honors an explicit modelPrefix override", () => { + // The combo builder call sites now thread through the already-computed + // routing-alias prefix (e.g. "oc") instead of letting the step default to + // the raw providerId, so the serialized `model` field round-trips to the + // correct provider. + const step = buildPrecisionComboModelStep({ + providerId: "opencode", + modelId: "big-pickle", + modelPrefix: "oc", + }); + + assert.equal(step.providerId, "opencode"); + assert.equal(step.model, "oc/big-pickle"); + + const parsed = parseModel(step.model); + assert.equal(parsed.provider, step.providerId); +}); + +test("issue #11433 fix: buildGlobalModelList derives modelPrefix from qualifiedModel for the no-auth OpenCode Free provider", () => { + // Mirrors what src/lib/combos/builderOptions.ts::rewriteQualifiedModelPrefix + // produces for the no-auth "opencode" provider entry: `qualifiedModel` is + // already rewritten to the "oc/" alias prefix, but (pre-fix) + // buildGlobalModelList ignored it and rebuilt `model` from the raw + // providerId, producing "opencode/big-pickle" which parses back to the + // wrong provider ("opencode-zen"). + const [entry] = buildGlobalModelList([ + { + providerId: "opencode", + displayName: "OpenCode Free", + connectionCount: 0, + connections: [], + models: [{ id: "big-pickle", name: "Big Pickle", qualifiedModel: "oc/big-pickle" }], + }, + ]); + + assert.equal(entry.step.providerId, "opencode"); + assert.equal(entry.step.model, "oc/big-pickle"); + assert.equal(parseModel(entry.step.model).provider, entry.step.providerId); +}); + +test("issue #11433 fix: buildManualComboModelStep preserves a user-typed oc/ prefix", () => { + // buildManualComboModelStep resolves the typed alias ("oc") back to the + // canonical providerId ("opencode") before building the step. Pre-fix, it + // then handed that canonical id straight to buildPrecisionComboModelStep, + // which rebuilt `model` from it and collapsed "oc/" back down to + // "opencode/" — reproducing the same collision for manual entry. + const step = buildManualComboModelStep({ + value: "oc/big-pickle", + providers: [{ providerId: "opencode", alias: "oc" }], + }); + + assert.ok(step); + assert.equal(step?.providerId, "opencode"); + assert.equal(step?.model, "oc/big-pickle"); + assert.equal(parseModel(step!.model).provider, step!.providerId); +});