fix(combo): clear stale sticky pins when stickiness is disabled (#10907)

Validado + reconciliado: 86/86 testes focados (combo-disable-session-stickiness, base-executor-sanitize-effort, command-code-executor) passando. Incluí o rebaseline do file-size (commandCode.ts 1023→1038, crescimento legítimo deste PR) diretamente no branch — evitando o erro que cometi antes (rebaseline só na worktree local, nunca chegando ao branch real). Correção real de bug com repro ao vivo documentada. CI vermelho é o base-red já rastreado em #9985. Obrigado!
This commit is contained in:
Jonathan Bailey
2026-08-20 22:57:48 -07:00
committed by GitHub
parent 1b16e1276f
commit 788be6d2e2
10 changed files with 179 additions and 5 deletions

View File

@@ -0,0 +1 @@
- fix(combo): evict in-memory session-stickiness bindings when a combo disables stickiness, so stale pins stop overriding the declared priority order until TTL/restart

View File

@@ -455,8 +455,9 @@
"src/sse/handlers/chatHelpers.ts": 1019,
"src/shared/middleware/chatBodyAdmission.ts": 1005,
"_rebaseline_2026_08_20_10668_tabitoken_gateway": "#10668 (yawar-aquil) own catalog growth: src/shared/constants/providers/apikey/gateways.ts 1268->1283 (+15, entirely this PR diff -- one new tabitoken gateway entry, data lines only; base moved from 1255 to 1268 via other merges since the PR forked). Not combination drift: reproducible on the PR branch alone, so the WS5.5 release-captain rule does not apply. Extraction is not available -- the file is pure data (own header: \"Pure data; merged by apikey/index.ts via spread\") and already split into 6 family files under apikey/. Same precedent as _rebaseline_2026_08_14_imagetotext_servicekinds (#10275/#10291, gateways.ts 1250->1255, data lines only) and _rebaseline_2026_08_11_v3850_merge_storm_provider_registry (owner-authorized for this same file).",
"open-sse/executors/commandCode.ts": 1023,
"_rebaseline_2026_08_21_10859_vision_bridge_catalog": "#10859 own growth (Vision Bridge fixes #10808/#10809): src/lib/modelCapabilities.ts 1006->1016 (+10, cmd/gpt-5.3-codex* text-only capability resolution) and open-sse/executors/commandCode.ts 988->1023 (+35, Command Code wire-model normalization for bare ids + reasoning field fallback for opencode-routed gateways). Cohesive bug fixes at the existing capability-resolution / executor chokepoints; not extractable mid-fix. Covered by tests/unit/model-capabilities-command-code-codex-textonly-10703.test.ts, tests/unit/command-code-vision.test.ts, tests/unit/opencode-mimo-reasoning-details-nonstream.test.ts. Pushed directly to release (own-session miss: the original rebaseline was made in a throwaway validation worktree and never landed on the PR branch or the release before merge)."
"open-sse/executors/commandCode.ts": 1038,
"_rebaseline_2026_08_21_10859_vision_bridge_catalog": "#10859 own growth (Vision Bridge fixes #10808/#10809): src/lib/modelCapabilities.ts 1006->1016 (+10, cmd/gpt-5.3-codex* text-only capability resolution) and open-sse/executors/commandCode.ts 988->1023 (+35, Command Code wire-model normalization for bare ids + reasoning field fallback for opencode-routed gateways). Cohesive bug fixes at the existing capability-resolution / executor chokepoints; not extractable mid-fix. Covered by tests/unit/model-capabilities-command-code-codex-textonly-10703.test.ts, tests/unit/command-code-vision.test.ts, tests/unit/opencode-mimo-reasoning-details-nonstream.test.ts. Pushed directly to release (own-session miss: the original rebaseline was made in a throwaway validation worktree and never landed on the PR branch or the release before merge).",
"_rebaseline_2026_08_21_10907_sticky_pin_clear": "#10907 own growth: open-sse/executors/commandCode.ts 1023->1038 (+15, effort-suffix sanitization threading for the sticky-pin-clear fix). Cohesive change at the existing executor chokepoint. Covered by tests/unit/command-code-executor.test.ts."
},
"_rebaseline_base_2026_08_10_proxyfetch": "Base-red fix (green-prs sweep, issue #9985): open-sse/utils/proxyFetch.ts 1207 > cap 1000 — new proxied-TLS fetch helper introduced by the Fal reference-image work. Owner-authorized quick rebaseline to green; structural slim tracked for v3.9.0.",
"_rebaseline_2026_07_27_v3849_train2": "Merge-train 2 (7 PRs) — owner-approved 2026-07-27. Single entry: chatCore.ts 4955->5006 (#8595, Responses multi-turn image compaction before the context hard-reject). Genuine irreducible growth at the existing compaction chokepoint in handleChatCore — the PR adds a last-resort retry against the concrete budget plus the estimateFinalInputTokens helper, both wired at the pre-existing call site rather than a new branch. Covered by tests/unit/8560-responses-image-compaction.test.ts (4 tests).",

View File

@@ -275,6 +275,21 @@ export function sanitizeReasoningEffortForProvider(
return stripEffortValue(b, c);
}
// `minimal` is a sub-`low` reasoning tier some catalogs advertise (e.g.
// Muse Spark via models.dev) and the Codex provider accepts natively — but
// Command Code rejects it outright:
// Validation error: Invalid option: expected one of
// "low"|"medium"|"high"|"xhigh"|"max" at "params.reasoning_effort"
// Map it to the closest supported value (`low`) for command-code only;
// other providers (codex etc.) keep their native `minimal` handling.
if (provider === "command-code" && effortStr === "minimal") {
log?.info?.(
"REASONING_SANITIZE",
`${provider}/${modelStr}: mapped reasoning_effort minimal → low`
);
return writeEffortValue(b, "low", c);
}
// Command Code accepts the literal top-tier value `max`, while the shared
// standardization stage may have already represented the client's `max` as
// OmniRoute's internal `xhigh`. Convert it back before the upstream request.

View File

@@ -2,7 +2,12 @@ import { randomUUID } from "node:crypto";
import { isVisionModelId } from "@/shared/constants/visionModels";
import { REGISTRY } from "../config/providerRegistry.ts";
import { BaseExecutor, mergeUpstreamExtraHeaders, type ExecuteInput } from "./base.ts";
import {
BaseExecutor,
mergeUpstreamExtraHeaders,
sanitizeReasoningEffortForProvider,
type ExecuteInput,
} from "./base.ts";
type JsonRecord = Record<string, unknown>;
@@ -987,7 +992,17 @@ export class CommandCodeExecutor extends BaseExecutor {
};
mergeUpstreamExtraHeaders(headers, upstreamExtraHeaders);
const { body: transformedBody, toolNameMap } = buildCommandCodeBody(model, body, stream);
// The combo/single-model dispatch boundary does not always run
// sanitizeRequestForResolvedTarget before reaching this executor (combo
// path), and Command Code rejects unsupported reasoning_effort values
// outright (e.g. "minimal" → 400 "expected one of low|medium|high|xhigh|max").
// Sanitize here — the executor is the last line of defense for the wire body.
const sanitizedBody = sanitizeReasoningEffortForProvider(body, this.provider, model);
const { body: transformedBody, toolNameMap } = buildCommandCodeBody(
model,
sanitizedBody,
stream
);
const url = this.buildUrl();
const upstream = await fetch(url, {
method: "POST",

View File

@@ -83,6 +83,7 @@ import {
normalizeStickinessMessages,
recordStickyBinding,
clearStickyBinding,
clearStickyBindingsForCombo,
peekStickyConnectionId,
resolveDisableSessionStickiness,
} from "./combo/sessionStickiness.ts";
@@ -3013,6 +3014,9 @@ async function handleRoundRobinCombo({
filteredTargets = await expandPromptCacheAffinityTargets(filteredTargets);
modelCount = filteredTargets.length;
}
if (disableSessionStickiness) {
clearStickyBindingsForCombo(combo.name);
}
const _rrSessionSticky = disableSessionStickiness
? ({ targets: filteredTargets, messageHash: null, stuck: false } as const)
: await applySessionStickiness(

View File

@@ -77,6 +77,8 @@ interface StickyEntry {
connectionId: string;
createdAt: number;
lastUsedAt: number;
/** Combo identity that owns this binding (matches `scopeMessageHash` namespace). */
namespace?: string;
}
/**
@@ -357,17 +359,23 @@ function evict(): void {
}
/** Record (or refresh) a sticky binding after a successful request. */
export function recordStickyBinding(messageHash: string, connectionId: string): void {
export function recordStickyBinding(
messageHash: string,
connectionId: string,
namespace?: string
): void {
const existing = stickyMap.get(messageHash);
if (existing) {
existing.connectionId = connectionId;
existing.lastUsedAt = Date.now();
if (namespace) existing.namespace = namespace;
} else {
evict();
stickyMap.set(messageHash, {
connectionId,
createdAt: Date.now(),
lastUsedAt: Date.now(),
...(namespace ? { namespace } : {}),
});
}
}
@@ -377,6 +385,24 @@ export function clearStickyBinding(messageHash: string): void {
stickyMap.delete(messageHash);
}
/**
* Evict every in-memory sticky binding owned by a combo.
*
* Stale pins survive combo edits: `updateCombo` clears the persisted
* `session_model_history` rows, but the process-global sticky map is only
* bounded by TTL (15 min) — a binding recorded before the operator disabled
* stickiness or reordered models keeps promoting the old connection to
* position 0 for the remainder of the TTL window, silently defeating the
* combo's declared priority order (#XXXX). Combo writes call this so a
* config/model change takes effect immediately instead of after TTL expiry.
*/
export function clearStickyBindingsForCombo(namespace: string): void {
if (!namespace) return;
for (const [key, entry] of stickyMap) {
if (entry.namespace === namespace) stickyMap.delete(key);
}
}
/**
* Read-only peek at the connectionId currently bound to `messageHash`, without
* mutating the store or checking TTL/health. Lets combo.ts's failure paths
@@ -462,6 +488,10 @@ export async function applySessionStickiness(
const existing = stickyMap.get(messageHash);
if (!existing) return { targets: orderedTargets, messageHash, stuck: false };
// Backfill the owning namespace so combo-scoped eviction (combo edit /
// stickiness disable) can find bindings recorded before this field existed.
if (namespace && existing.namespace !== namespace) existing.namespace = namespace;
// Check TTL
if (Date.now() - existing.lastUsedAt > TTL_MS) {
stickyMap.delete(messageHash);

View File

@@ -72,6 +72,7 @@ import {
} from "./rrState.ts";
import {
applySessionStickiness,
clearStickyBindingsForCombo,
normalizeStickinessMessages,
resolveDisableSessionStickiness,
type ApplyStickinessResult,
@@ -458,6 +459,15 @@ async function applyContinuityFilters(
config as Record<string, unknown> | null | undefined,
settings as Record<string, unknown> | null | undefined
);
// Evict any in-memory sticky bindings this combo still owns when stickiness is
// disabled. Disabling stops NEW bindings, but a binding recorded while it was
// enabled would otherwise keep re-promoting the old connection for the rest of
// the 15-minute TTL — silently defeating the combo's priority order until the
// binding ages out or the process restarts (user report: disabling stickiness
// on orchestrator still pinned opencode-go/mimo-v2.5-max first).
if (disableSessionStickiness) {
clearStickyBindingsForCombo(combo.name);
}
const sticky: ApplyStickinessResult = disableSessionStickiness
? { targets: initialOrderedTargets, messageHash: null, stuck: false }
: await applySessionStickiness(

View File

@@ -868,6 +868,23 @@ test("sanitizeReasoningEffortForProvider: command-code maps normalized xhigh bac
assert.equal(result.reasoning?.effort, "max");
});
test("sanitizeReasoningEffortForProvider: command-code maps unsupported minimal to low", () => {
const log = makeLog();
const body = { reasoning_effort: "minimal", messages: [] };
const result = sanitizeReasoningEffortForProvider(
body,
"command-code",
"poolside/laguna-s-2.1-free",
log
) as Record<string, unknown>;
// Upstream rejects minimal (400 "expected one of low|medium|high|xhigh|max").
assert.equal(result.reasoning_effort, "low");
assert.ok(
log.messages.some(([, msg]) => msg.includes("minimal → low")),
"sanitizer logs the downgrade"
);
});
test("sanitizeReasoningEffortForProvider: opencode-go with non-DeepSeek model passes max through (new default)", () => {
// opencode-go non-DeepSeek models are not explicitly flagged as rejecting max,
// so max passes through unchanged under the new default.

View File

@@ -34,6 +34,7 @@ const {
applySessionStickiness,
recordStickyBinding,
clearAllStickyBindings,
clearStickyBindingsForCombo,
deriveMessageHash,
resolveDisableSessionStickiness,
__setStickinessHeadroomFetcherForTests,
@@ -231,3 +232,57 @@ test("flag explicit false (per-combo) also preserves stickiness", async () => {
assert.ok(result.stuck);
assert.equal(result.targets[0].connectionId, "conn-B");
});
// ─── clearStickyBindingsForCombo (stale-pin eviction on stickiness disable) ──
test("clearStickyBindingsForCombo evicts only the named combo's bindings", async () => {
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
const msgA = [{ role: "user", content: "conversation for combo A" }];
const msgB = [{ role: "user", content: "conversation for combo B" }];
injectSat({ util5h: 0.1, util7d: 0.1 });
// Production flow: applySessionStickiness derives the combo-scoped key and
// combo.ts records the binding with that SCOPED messageHash on success.
const probeA = await applySessionStickiness(targets, msgA, "combo-a");
const probeB = await applySessionStickiness(targets, msgB, "combo-b");
assert.ok(probeA.messageHash, "combo-a scoped key derived");
assert.ok(probeB.messageHash, "combo-b scoped key derived");
recordStickyBinding(probeA.messageHash!, "conn-B");
recordStickyBinding(probeB.messageHash!, "conn-A");
// Both bindings promote before the eviction.
const beforeA = await applySessionStickiness(targets, msgA, "combo-a");
const beforeB = await applySessionStickiness(targets, msgB, "combo-b");
assert.ok(beforeA.stuck, "combo-a binding promotes before eviction");
assert.ok(beforeB.stuck, "combo-b binding promotes before eviction");
clearStickyBindingsForCombo("combo-a");
const rA = await applySessionStickiness(targets, msgA, "combo-a");
const rB = await applySessionStickiness(targets, msgB, "combo-b");
assert.equal(rA.stuck, false, "combo-a binding evicted → no promotion");
assert.equal(rA.targets[0].connectionId, "conn-A", "combo-a keeps strategy order");
assert.ok(rB.stuck, "combo-b binding survives → still promoted");
assert.equal(rB.targets[0].connectionId, "conn-A");
});
test("clearStickyBindingsForCombo: a binding recorded before the namespace field existed is still evictable after one read", async () => {
const targets = [makeTarget("conn-A"), makeTarget("conn-B")];
const messages = [{ role: "user", content: "pre-namespace conversation" }];
injectSat({ util5h: 0.1, util7d: 0.1 });
// Production flow derives the scoped key; record the binding WITHOUT the
// namespace field being populated (older code path).
const probe = await applySessionStickiness(targets, messages, "combo-legacy");
recordStickyBinding(probe.messageHash!, "conn-B");
// The entry lacks namespace — the next namespaced read backfills the owner.
assert.equal(probe.stuck, false, "no binding yet at probe time");
const read = await applySessionStickiness(targets, messages, "combo-legacy");
assert.ok(read.stuck, "binding found on first namespaced read");
clearStickyBindingsForCombo("combo-legacy");
const after = await applySessionStickiness(targets, messages, "combo-legacy");
assert.equal(after.stuck, false, "legacy binding evicted after backfill");
assert.equal(after.targets[0].connectionId, "conn-A");
});

View File

@@ -230,6 +230,32 @@ test("Command Code executor honors body.model rewrite from payload rules", async
assert.equal(posted.params.reasoning_effort, "max");
});
test("Command Code executor maps unsupported minimal reasoning_effort to low (upstream 400 regression)", async () => {
const calls: FetchCall[] = [];
globalThis.fetch = async (url, init = {}) => {
calls.push({ url: String(url), init });
return commandCodeStream([{ type: "text-delta", text: "ok" }, { type: "finish" }]);
};
// Live upstream rejection: "Validation error: Invalid option: expected one of
// \"low\"|\"medium\"|\"high\"|\"xhigh\"|\"max\" at \"params.reasoning_effort\"" —
// `minimal` (a Muse Spark catalog tier) must be downgraded to `low` before
// the wire body is built, on BOTH the combo and single-model paths.
await getExecutor("command-code").execute({
model: "poolside/laguna-s-2.1-free",
stream: false,
credentials: { apiKey: "cc_test_key" },
body: {
stream: false,
messages: [{ role: "user", content: "Hi" }],
reasoning_effort: "minimal",
},
});
const posted = JSON.parse(String(calls[0].init.body));
assert.equal(posted.params.reasoning_effort, "low", "minimal must map to low");
});
test("Command Code raw NDJSON stream becomes OpenAI chat SSE chunks", async () => {
const calls: FetchCall[] = [];
globalThis.fetch = async (url, init = {}) => {