Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions open-sse/services/combo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { estimateTokens } from "./contextManager.ts";
import { getSessionConnection } from "./sessionManager.ts";
import {
applySessionStickiness,
normalizeStickinessMessages,
recordStickyBinding,
clearStickyBinding,
peekStickyConnectionId,
Expand Down Expand Up @@ -1207,7 +1208,9 @@ export async function handleComboChat({
? ({ targets: orderedTargets, messageHash: null, stuck: false } as const)
: await applySessionStickiness(
orderedTargets,
body.messages as Array<{ role?: string; content?: unknown }>
// #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 })
);
orderedTargets = _sticky.targets;
orderedTargets = orderTargetsByEvalScores(orderedTargets, config.evalRouting, log);
Expand Down Expand Up @@ -2631,7 +2634,9 @@ async function handleRoundRobinCombo({
? ({ targets: filteredTargets, messageHash: null, stuck: false } as const)
: await applySessionStickiness(
filteredTargets,
body?.messages as Array<{ role?: string; content?: unknown }>
// #7270: normalize both wire shapes (.messages / Responses-API .input) so RR
// stickiness engages on the /v1/responses surface, not just Chat Completions.
normalizeStickinessMessages(body as { messages?: unknown; input?: unknown })
);
let rrStartIndex = startIndex;
if (_rrSessionSticky.stuck) {
Expand Down
29 changes: 29 additions & 0 deletions open-sse/services/combo/sessionStickiness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,35 @@ const stickyMap = new Map<string, StickyEntry>();

// ─── Helpers ─────────────────────────────────────────────────────────────────

/**
* #7270: Normalize a request body's user turns into a `{role, content}[]` view for
* stickiness-key derivation, covering both wire formats:
* - Chat Completions (`/v1/chat/completions`) → turns live in `.messages`.
* - OpenAI Responses API (`/v1/responses`) → turns live in `.input`, which may be a
* plain string OR an array of message items; `.messages` is never populated.
* Combo target ordering runs BEFORE per-target format translation, so without this
* the Responses-API key resolved to null and stickiness silently no-oped for the
* entire surface (round-robin/random/strict-random all re-ordered every turn).
* `.messages` takes precedence when present (Chat Completions), then `.input`.
* Returns null when neither carrier yields turns (fail-open, same as deriveMessageHash).
*/
export function normalizeStickinessMessages(
body: { messages?: unknown; input?: unknown } | null | undefined
): Array<{ role?: string; content?: unknown }> | null {
if (!body || typeof body !== "object") return null;
const { messages, input } = body as { messages?: unknown; input?: unknown };
if (Array.isArray(messages) && messages.length > 0) {
return messages as Array<{ role?: string; content?: unknown }>;
}
if (typeof input === "string" && input.length > 0) {
return [{ role: "user", content: input }];
}
if (Array.isArray(input) && input.length > 0) {
return input as Array<{ role?: string; content?: unknown }>;
}
Comment on lines +229 to +233

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the input field is an array of strings (e.g., string[]), which is a valid format for the Responses API, normalizeStickinessMessages currently returns it directly cast as Array<{ role?: string; content?: unknown }>. This causes deriveMessageHash to fail to find any message with role === "user" (since strings do not have a role property), resulting in a null hash and silently disabling session stickiness. Checking if the first element is a string and mapping the array to message objects resolves this issue.

  if (Array.isArray(input) && input.length > 0) {
    if (typeof input[0] === "string") {
      return input.map((content) => ({ role: "user", content }));
    }
    return input as Array<{ role?: string; content?: unknown }>;
  }

return null;
}

/**
* Derive a stable 16-hex-char session key from the first user message content.
* Returns null when the message cannot be extracted (fail-open).
Expand Down
173 changes: 173 additions & 0 deletions tests/unit/combo-stickiness-responses-input-7270.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/**
* Regression for #7270 — combo session stickiness is a silent no-op on the OpenAI
* Responses API (`/v1/responses`).
*
* The stickiness key was derived exclusively from `body.messages`, but Responses-API
* requests carry their turns in `.input` and never populate `.messages`. So
* deriveMessageHash(body.messages) resolved to null, stickiness failed open, and a
* single conversation was re-ordered every turn as if stickiness were disabled —
* regardless of strategy or the toggle.
*
* This drives the REAL handleComboChat with a round-robin combo and an `.input`-shaped
* body and asserts:
* - a single sessionless Responses-API conversation re-pins to its turn-1 connection
* (FAILS before the fix: the combo rotates A → B → C …);
* - distinct conversations still spread across connections (spreading preserved);
* - the new normalizeStickinessMessages() helper covers both wire shapes.
*/
import test from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";

const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-stick-resp-7270-"));
const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
process.env.DATA_DIR = TEST_DATA_DIR;

const { handleComboChat } = await import("../../open-sse/services/combo.ts");
const stick = await import("../../open-sse/services/combo/sessionStickiness.ts");
const dbCore = await import("../../src/lib/db/core.ts");

function makeLog() {
return { info() {}, warn() {}, debug() {}, error() {} };
}

function rrCombo(name: string) {
return {
name,
strategy: "round-robin",
config: { maxRetries: 0 },
models: [
{ kind: "model", provider: "codex", providerId: "codex", model: "m-a", connectionId: "conn-A", id: `${name}-0` },
{ kind: "model", provider: "codex", providerId: "codex", model: "m-b", connectionId: "conn-B", id: `${name}-1` },
{ kind: "model", provider: "glm-cn", providerId: "glm-cn", model: "m-c", connectionId: "conn-C", id: `${name}-2` },
],
};
}

// Responses-API shape: turns live in `.input` (array of message items with
// input_text content parts), and `.messages` is absent.
function responsesBody(combo: Record<string, unknown>, firstMessage: string) {
return {
model: combo.name,
input: [{ role: "user", content: [{ type: "input_text", text: firstMessage }] }],
stream: false,
};
}

async function dispatchConnection(
combo: Record<string, unknown>,
body: Record<string, unknown>
): Promise<string> {
let conn = "?";
await handleComboChat({
body,
combo,
allCombos: [combo],
isModelAvailable: async () => true,
relayOptions: undefined,
signal: undefined,
settings: {},
log: makeLog(),
handleSingleModel: async (
_b: unknown,
modelStr: string,
target?: { connectionId?: string | null }
) => {
conn = target?.connectionId ?? "?";
return Response.json({ choices: [{ message: { role: "assistant", content: modelStr } }] });
},
});
return conn;
}

test.beforeEach(() => {
stick.clearAllStickyBindings();
// Fail-open saturation (unknown → full headroom) so we exercise the stickiness
// MECHANISM, not the headroom gate.
stick.__setStickinessHeadroomFetcherForTests(async () => undefined);
});

test.after(() => {
stick.__setStickinessHeadroomFetcherForTests(null);
dbCore.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
});

test("normalizeStickinessMessages: covers .messages, .input array and .input string", () => {
const fromMessages = stick.normalizeStickinessMessages({
messages: [{ role: "user", content: "hi" }],
});
assert.deepEqual(fromMessages, [{ role: "user", content: "hi" }]);

const fromInputArray = stick.normalizeStickinessMessages({
input: [{ role: "user", content: [{ type: "input_text", text: "hi" }] }],
});
assert.ok(Array.isArray(fromInputArray) && fromInputArray.length === 1);

const fromInputString = stick.normalizeStickinessMessages({ input: "plain string turn" });
assert.deepEqual(fromInputString, [{ role: "user", content: "plain string turn" }]);
Comment on lines +111 to +112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a unit test case to verify that normalizeStickinessMessages correctly handles and normalizes an input array of strings (string[]).

  const fromInputString = stick.normalizeStickinessMessages({ input: "plain string turn" });
  assert.deepEqual(fromInputString, [{ role: "user", content: "plain string turn" }]);

  const fromInputStringArray = stick.normalizeStickinessMessages({
    input: ["hello", "world"],
  });
  assert.deepEqual(fromInputStringArray, [
    { role: "user", content: "hello" },
    { role: "user", content: "world" },
  ]);


// .messages wins over .input when both present (Chat Completions precedence).
const both = stick.normalizeStickinessMessages({
messages: [{ role: "user", content: "from messages" }],
input: "from input",
});
assert.deepEqual(both, [{ role: "user", content: "from messages" }]);

assert.equal(stick.normalizeStickinessMessages({}), null);
assert.equal(stick.normalizeStickinessMessages(null), null);
});

test("an .input-shaped Responses body yields a stable, non-null stickiness key", () => {
const body = responsesBody(rrCombo("k"), "First turn of the conversation");
// Old behavior: deriveMessageHash(body.messages) is null (bug).
assert.equal(
stick.deriveMessageHash(body.messages as never),
null,
"body.messages is absent on the Responses API — the old key source is null"
);
// New behavior: the normalized view produces a stable key.
const key1 = stick.deriveMessageHash(stick.normalizeStickinessMessages(body));
const key2 = stick.deriveMessageHash(stick.normalizeStickinessMessages(body));
assert.ok(key1 !== null, "normalized Responses body must yield a key");
assert.match(key1!, /^[a-f0-9]{16}$/);
assert.equal(key1, key2, "stable across calls");
});

test("round-robin: a sessionless Responses-API conversation re-pins across turns (#7270)", async () => {
const combo = rrCombo("rr-resp-stick");
const conns: string[] = [];
for (let turn = 0; turn < 5; turn++) {
conns.push(
await dispatchConnection(combo, responsesBody(combo, "Refactor the streaming handler."))
);
}
// Turns 2..5 must all reuse turn 1's connection. Before the fix the RR combo rotates
// (conn-A → conn-B → conn-C → …) because the .input key resolved to null → set size 3.
assert.equal(
new Set(conns.slice(1)).size,
1,
`turns 2..5 must stick to one connection, got: ${conns.join(", ")}`
);
assert.equal(conns[1], conns[0], "turn 2 must reuse turn 1's connection");
});

test("round-robin: distinct Responses-API conversations still spread (#7270 spreading guard)", async () => {
const combo = rrCombo("rr-resp-spread");
const hist: Record<string, number> = {};
for (let i = 0; i < 6; i++) {
const conn = await dispatchConnection(
combo,
responsesBody(combo, `conversation number ${i} — distinct first turn`)
);
hist[conn] = (hist[conn] || 0) + 1;
}
assert.ok(
Object.keys(hist).length > 1,
`distinct conversations must spread across connections, got: ${JSON.stringify(hist)}`
);
});
Loading