diff --git a/changelog.d/fixes/15458-priority-order-vs-stickiness.md b/changelog.d/fixes/15458-priority-order-vs-stickiness.md new file mode 100644 index 000000000000..ab7ddf37b084 --- /dev/null +++ b/changelog.d/fixes/15458-priority-order-vs-stickiness.md @@ -0,0 +1 @@ +- **fix(combo):** keep declared priority order ahead of session stickiness ([#15458](https://github.com/diegosouzapw/OmniRoute/pull/15458)) — thanks @skygunner diff --git a/open-sse/services/combo/sessionStickiness.ts b/open-sse/services/combo/sessionStickiness.ts index 7347730a6028..6929dff9b62d 100644 --- a/open-sse/services/combo/sessionStickiness.ts +++ b/open-sse/services/combo/sessionStickiness.ts @@ -85,9 +85,7 @@ interface StickyEntry { * Injectable saturation fetcher seam (for unit tests). * Returns HeadroomSaturation or undefined when unknown. */ -export type SaturationFetcher = ( - connectionId: string -) => Promise; +export type SaturationFetcher = (connectionId: string) => Promise; // ─── Saturation fetcher seam ───────────────────────────────────────────────── @@ -187,9 +185,7 @@ export type QuotaExhaustionChecker = (connectionId: string) => boolean; let _quotaExhaustionOverride: QuotaExhaustionChecker | null = null; /** Test-only: inject the quota-exhaustion checker; pass null to restore default. */ -export function __setStickinessQuotaCheckerForTests( - checker: QuotaExhaustionChecker | null -): void { +export function __setStickinessQuotaCheckerForTests(checker: QuotaExhaustionChecker | null): void { _quotaExhaustionOverride = checker; } @@ -450,6 +446,16 @@ export interface ApplyStickinessResult { stuck: boolean; } +/** + * #15241: when the caller needs to preserve the combo's declared first target, + * a sticky binding pointing at a mid-list connection must not silently become + * runtime try-slot #1. The binding's hygiene gates below still run; only the + * promotion is barred. + */ +export interface ApplyStickinessOptions { + respectDeclaredOrder?: boolean; +} + /** * Attempt to promote the sticky connection to the front of `orderedTargets`. * @@ -469,12 +475,15 @@ export interface ApplyStickinessResult { * @param orderedTargets Targets already ordered by the combo strategy. * @param messages Request body.messages. * @param namespace Combo identity that owns this sticky binding. + * @param options #15241 — `respectDeclaredOrder` bars a mid-list promotion + * for strategies whose first target is operator-declared. * @returns Result with (possibly reordered) targets. */ export async function applySessionStickiness( orderedTargets: ResolvedComboTarget[], messages: Array<{ role?: string; content?: unknown }> | null | undefined, - namespace?: string + namespace?: string, + options?: ApplyStickinessOptions ): Promise { const noOp: ApplyStickinessResult = { targets: orderedTargets, messageHash: null, stuck: false }; @@ -530,6 +539,12 @@ export async function applySessionStickiness( return { targets: orderedTargets, messageHash, stuck: false }; } + // #15241: a caller preserving an operator-declared head keeps its order — + // a mid-list sticky connection stays mid-list. + if (options?.respectDeclaredOrder && stickyIdx > 0) { + return { targets: orderedTargets, messageHash, stuck: false }; + } + // Promote the sticky target to position 0 const reordered = [ orderedTargets[stickyIdx], diff --git a/open-sse/services/combo/targetResolution.ts b/open-sse/services/combo/targetResolution.ts index e55f9ac767bd..cf79de23b46b 100644 --- a/open-sse/services/combo/targetResolution.ts +++ b/open-sse/services/combo/targetResolution.ts @@ -535,7 +535,12 @@ async function applyContinuityFilters( // #7270: normalize both wire shapes (.messages / Responses-API .input) so the // stickiness key is derivable on the /v1/responses surface, not just Chat Completions. normalizeStickinessMessages(body as { messages?: unknown; input?: unknown }), - combo.name + combo.name, + // #15241: priority is an operator-declared failover order. A mid-list + // success must not silently become runtime try-slot #1 while the stored + // hop list still names another head. Other strategies keep their existing + // session-stickiness behavior. + { respectDeclaredOrder: strategy === "priority" } ); let orderedTargets = sticky.targets; if (!cacheStrategyAffinityApplied) { diff --git a/tests/unit/combo-session-stickiness.test.ts b/tests/unit/combo-session-stickiness.test.ts index 2d4d93e14dbd..85e8e86b837a 100644 --- a/tests/unit/combo-session-stickiness.test.ts +++ b/tests/unit/combo-session-stickiness.test.ts @@ -37,7 +37,9 @@ const { // ─── helpers ───────────────────────────────────────────────────────────────── -function makeTarget(connectionId: string): import("../../open-sse/services/combo/types.ts").ResolvedComboTarget { +function makeTarget( + connectionId: string +): import("../../open-sse/services/combo/types.ts").ResolvedComboTarget { return { kind: "model", stepId: `step-${connectionId}`, @@ -466,3 +468,48 @@ test("peekStickyConnectionId: reflects the current binding without mutating it", // Peeking again must not clear or otherwise mutate the binding. assert.equal(peekStickyConnectionId(hash), "conn-peek"); }); + +// ─── #15241: operator-declared order outranks stickiness promotion ─────────── + +test("#15241: respectDeclaredOrder keeps the declared head over a mid-list sticky connection", async () => { + const messages = [{ role: "user", content: "Priority order" }]; + const hash = deriveMessageHash(messages)!; + recordStickyBinding(hash, "conn-B"); + + const targets = [makeTarget("conn-A"), makeTarget("conn-B"), makeTarget("conn-C")]; + const result = await applySessionStickiness(targets, messages, undefined, { + respectDeclaredOrder: true, + }); + + assert.equal(result.stuck, false, "nothing was promoted, so sticky must not report stuck"); + assert.equal(result.targets[0].connectionId, "conn-A", "declared head stays first"); + assert.equal(result.targets[1].connectionId, "conn-B"); + // the binding survives — only the reorder is barred, a later success refreshes it + assert.equal(peekStickyConnectionId(hash), "conn-B"); +}); + +test("#15241: respectDeclaredOrder leaves a sticky connection already at the head untouched", async () => { + const messages = [{ role: "user", content: "Already head" }]; + const hash = deriveMessageHash(messages)!; + recordStickyBinding(hash, "conn-A"); + + const targets = [makeTarget("conn-A"), makeTarget("conn-B")]; + const result = await applySessionStickiness(targets, messages, undefined, { + respectDeclaredOrder: true, + }); + + assert.equal(result.stuck, true, "declared head equals the sticky pin — sticky still applied"); + assert.equal(result.targets[0].connectionId, "conn-A"); +}); + +test("#15241: without respectDeclaredOrder a mid-list sticky connection still promotes", async () => { + const messages = [{ role: "user", content: "Legacy behavior" }]; + const hash = deriveMessageHash(messages)!; + recordStickyBinding(hash, "conn-B"); + + const targets = [makeTarget("conn-A"), makeTarget("conn-B"), makeTarget("conn-C")]; + const result = await applySessionStickiness(targets, messages); + + assert.equal(result.stuck, true, "strategies without a declared head keep the reorder"); + assert.equal(result.targets[0].connectionId, "conn-B"); +}); diff --git a/tests/unit/combo-target-resolution-split.test.ts b/tests/unit/combo-target-resolution-split.test.ts index 9f80ff17fda7..13ae1ac22931 100644 --- a/tests/unit/combo-target-resolution-split.test.ts +++ b/tests/unit/combo-target-resolution-split.test.ts @@ -22,6 +22,8 @@ const { saveModelsDevCapabilities, clearModelsDevCapabilities } = await import("../../src/lib/modelsDevSync.ts"); const { resolveComboTargetPipeline } = await import("../../open-sse/services/combo/targetResolution.ts"); +const { clearAllStickyBindings, recordStickyBinding } = + await import("../../open-sse/services/combo/sessionStickiness.ts"); test.after(() => { core.resetDbInstance(); @@ -35,6 +37,7 @@ test.after(() => { test.beforeEach(() => { clearModelsDevCapabilities(); + clearAllStickyBindings(); }); const noopLog = { info() {}, warn() {}, error() {}, debug() {} } as never; @@ -94,6 +97,36 @@ test("priority strategy resolves combo models into orderedTargets in declared or ); }); +test("#15241 priority pipeline keeps its declared head after a mid-list sticky success", async () => { + const priorityWithConnections = { + id: "c-sticky-priority", + name: "sticky-priority", + strategy: "priority", + models: [ + { model: "openai/gpt-4o", connectionId: "conn-priority-a" }, + { model: "anthropic/claude-3", connectionId: "conn-priority-b" }, + ], + config: {}, + }; + const first = await resolveComboTargetPipeline(deps({ combo: priorityWithConnections })); + assert.ok(!("earlyResponse" in first)); + if ("earlyResponse" in first) return; + assert.ok(first.sticky.messageHash, "pipeline derives the conversation stickiness key"); + assert.ok(first.orderedTargets[1]?.connectionId, "second target has a connection id to pin"); + + recordStickyBinding(first.sticky.messageHash!, first.orderedTargets[1]!.connectionId!); + + const repeated = await resolveComboTargetPipeline(deps({ combo: priorityWithConnections })); + assert.ok(!("earlyResponse" in repeated)); + if ("earlyResponse" in repeated) return; + assert.deepEqual( + repeated.orderedTargets.map((target) => target.modelStr), + ["openai/gpt-4o", "anthropic/claude-3"], + "a prior mid-list success must not become runtime try-slot #1 for priority" + ); + assert.equal(repeated.sticky.stuck, false); +}); + test("returns the derived values the attempt loop consumes", async () => { const result = await resolveComboTargetPipeline(deps()); assert.ok(!("earlyResponse" in result));