diff --git a/changelog.d/fixes/8370-priority-affinity-reorder.md b/changelog.d/fixes/8370-priority-affinity-reorder.md new file mode 100644 index 0000000000..851e000a82 --- /dev/null +++ b/changelog.d/fixes/8370-priority-affinity-reorder.md @@ -0,0 +1 @@ +- fix(backend): stop prompt-cache affinity from silently reordering an explicit priority combo across models (#8370) diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index ed1a3aece5..5441ebf8f5 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -90,6 +90,7 @@ import { expandPromptCacheAffinityTargets, expandPromptCacheAffinityTargetsFromConnections, resolvePromptCacheAffinityKey, + shouldProtectOriginalFirst, } from "./combo/promptCacheAffinity.ts"; import type { CompressionMode } from "./compression/types.ts"; import { getCachedProviderConnections } from "../../src/lib/db/readCache"; @@ -1390,10 +1391,7 @@ export async function handleComboChat({ ); if (promptCacheAffinity.applied) { const protectedOriginal = - (_sticky.stuck || - autoUsedExplicitRouter || - strategy === "quota-share" || - strategy === "weighted") && + shouldProtectOriginalFirst(_sticky.stuck, autoUsedExplicitRouter, strategy) && orderedTargets[0]; const protectedFirst = protectedOriginal ? (promptCacheAffinity.targets.find( diff --git a/open-sse/services/combo/promptCacheAffinity.ts b/open-sse/services/combo/promptCacheAffinity.ts index fdfe9e1352..070e50f84e 100644 --- a/open-sse/services/combo/promptCacheAffinity.ts +++ b/open-sse/services/combo/promptCacheAffinity.ts @@ -237,6 +237,34 @@ export function expandPromptCacheAffinityTargetsFromConnections( return expandedTargets; } +/** + * #8370: decide whether the strategy-selected first target must be re-pinned + * ahead of a global, cross-model prompt-cache-affinity reorder. + * + * `priority`, `fill-first`, and `lkgp` are deterministic, operator-ordered + * strategies: their first target is a meaningful choice (explicit priority + * order, first-available slot, last-known-good pin) that a rendezvous-hash + * reorder must not silently override, even though affinity is still free to + * pick among the remaining/fallback targets. `quota-share` and `weighted` + * were already protected before this fix; session stickiness and an explicit + * auto-router pin remain independently protected via their own flags. + */ +export function shouldProtectOriginalFirst( + stickyStuck: boolean, + autoUsedExplicitRouter: boolean, + strategy: string +): boolean { + return ( + stickyStuck || + autoUsedExplicitRouter || + strategy === "quota-share" || + strategy === "weighted" || + strategy === "priority" || + strategy === "fill-first" || + strategy === "lkgp" + ); +} + /** * Order eligible targets using rendezvous hashing. The original order is used * as the final tie-breaker, so targets sharing one account identity remain diff --git a/tests/unit/8370-priority-affinity-reorder.test.ts b/tests/unit/8370-priority-affinity-reorder.test.ts new file mode 100644 index 0000000000..760cd77071 --- /dev/null +++ b/tests/unit/8370-priority-affinity-reorder.test.ts @@ -0,0 +1,178 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + applyPromptCacheAffinity, + expandPromptCacheAffinityTargetsFromConnections, + shouldProtectOriginalFirst, +} from "../../open-sse/services/combo/promptCacheAffinity.ts"; +import type { ResolvedComboTarget } from "../../open-sse/services/combo/types.ts"; + +// #8370: a `priority` combo declares an explicit, operator-chosen model order. +// Cross-model prompt-cache affinity (a global rendezvous-hash sort over the +// fully expanded, model-blind target list) was silently reordering that +// declaration, letting a lower-priority model's single account jump ahead of +// every account belonging to the highest-priority model. This file is the +// permanent regression guard for that fix (`shouldProtectOriginalFirst` in +// `open-sse/services/combo/promptCacheAffinity.ts`, wired into +// `open-sse/services/combo.ts`'s `protectedOriginal` gate). + +function modelTarget( + stepId: string, + modelStr: string, + provider: string, + allowedConnectionIds?: string[] +): ResolvedComboTarget { + return { + kind: "model", + stepId, + executionKey: stepId, + modelStr, + provider, + providerId: null, + connectionId: null, + weight: 1, + label: null, + ...(allowedConnectionIds ? { allowedConnectionIds } : {}), + } as ResolvedComboTarget; +} + +// Mirrors combo.ts's exact application of the fix: expand model-level targets +// to concrete accounts, run affinity, then re-pin the strategy's declared +// first target ahead of the affinity-sorted list when the strategy warrants it. +function applyComboLikeAffinityPin( + orderedTargets: ResolvedComboTarget[], + connectionsByProvider: Map>>, + body: Record, + strategy: string +): ResolvedComboTarget[] { + const expanded = expandPromptCacheAffinityTargetsFromConnections( + orderedTargets, + connectionsByProvider + ); + const affinity = applyPromptCacheAffinity(expanded, body, true); + if (!affinity.applied) return affinity.targets; + + const protectedOriginal = shouldProtectOriginalFirst(false, false, strategy) && orderedTargets[0]; + + const protectedFirst = protectedOriginal + ? (affinity.targets.find( + (target) => + target === protectedOriginal || + target.executionKey === protectedOriginal.executionKey || + target.executionKey.startsWith(`${protectedOriginal.executionKey}@`) + ) ?? protectedOriginal) + : null; + + return protectedFirst + ? [protectedFirst, ...affinity.targets.filter((target) => target !== protectedFirst)] + : affinity.targets; +} + +function buildCrossModelScenario() { + // Model A (priority 1) has 5 accounts; models B and C (priority 2/3) have 1 each — + // mirrors the issue's reported 3-models-expanded-to-N-accounts shape. + const orderedTargets = [ + modelTarget("step-a", "antigravity/gemini-3-pro", "antigravity"), + modelTarget("step-b", "ollamacloud/minimax-m3", "ollamacloud"), + modelTarget("step-c", "oc/deepseek-v4", "oc"), + ]; + const connectionsByProvider = new Map>>([ + [ + "antigravity", + [ + { id: "antigravity-acct-1" }, + { id: "antigravity-acct-2" }, + { id: "antigravity-acct-3" }, + { id: "antigravity-acct-4" }, + { id: "antigravity-acct-5" }, + ], + ], + ["ollamacloud", [{ id: "minimax-acct-1" }]], + ["oc", [{ id: "deepseek-acct-1" }]], + ]); + return { orderedTargets, connectionsByProvider }; +} + +// Brute-force a prompt_cache_key whose rendezvous winner is NOT one of model +// A's accounts, so the bug (if unfixed) is guaranteed to reproduce rather +// than passing by chance of the hash landing on model A anyway. +function findKeyThatWinsOutsideModelA( + connectionsByProvider: Map>>, + orderedTargets: ResolvedComboTarget[] +): string { + const expanded = expandPromptCacheAffinityTargetsFromConnections( + orderedTargets, + connectionsByProvider + ); + for (let i = 0; i < 500; i++) { + const key = `probe-key-${i}`; + const ranked = applyPromptCacheAffinity(expanded, { prompt_cache_key: key }, true); + if (ranked.targets[0]?.provider !== "antigravity") return key; + } + throw new Error("could not find a probe key whose rendezvous winner is outside model A"); +} + +test("BUG #8370: priority combo keeps its declared model-1-first order despite cross-model affinity", () => { + const { orderedTargets, connectionsByProvider } = buildCrossModelScenario(); + const key = findKeyThatWinsOutsideModelA(connectionsByProvider, orderedTargets); + const body = { prompt_cache_key: key }; + + // Sanity: without the fix's protection, raw affinity really does let a + // model B/C account win the global sort (proves the scenario reproduces). + const expanded = expandPromptCacheAffinityTargetsFromConnections( + orderedTargets, + connectionsByProvider + ); + const rawAffinity = applyPromptCacheAffinity(expanded, body, true); + assert.notEqual( + rawAffinity.targets[0]?.provider, + "antigravity", + "test setup invariant: raw affinity must pick outside model A for this key" + ); + + const result = applyComboLikeAffinityPin(orderedTargets, connectionsByProvider, body, "priority"); + + assert.equal( + result[0]?.provider, + "antigravity", + "priority combo must keep its declared highest-priority model first, not the rendezvous winner" + ); +}); + +test("shouldProtectOriginalFirst covers priority, fill-first, and lkgp", () => { + for (const strategy of ["priority", "fill-first", "lkgp"]) { + assert.equal( + shouldProtectOriginalFirst(false, false, strategy), + true, + `expected ${strategy} to be protected` + ); + } +}); + +test("shouldProtectOriginalFirst still covers the pre-existing quota-share/weighted/sticky/auto-router cases", () => { + assert.equal(shouldProtectOriginalFirst(false, false, "quota-share"), true); + assert.equal(shouldProtectOriginalFirst(false, false, "weighted"), true); + assert.equal(shouldProtectOriginalFirst(true, false, "round-robin"), true); + assert.equal(shouldProtectOriginalFirst(false, true, "round-robin"), true); +}); + +test("round-robin combo is NOT protected — it still gets full cross-model affinity reordering", () => { + const { orderedTargets, connectionsByProvider } = buildCrossModelScenario(); + const key = findKeyThatWinsOutsideModelA(connectionsByProvider, orderedTargets); + const body = { prompt_cache_key: key }; + + assert.equal(shouldProtectOriginalFirst(false, false, "round-robin"), false); + + const result = applyComboLikeAffinityPin( + orderedTargets, + connectionsByProvider, + body, + "round-robin" + ); + + assert.notEqual( + result[0]?.provider, + "antigravity", + "round-robin combo must still let prompt-cache affinity pick the winning account across models" + ); +});