mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 21:32:10 +03:00
fix(fusion): judge replayed a panel answer via idempotency-key collision (#6558)
Merged — thank you, @developerjillur! Namespaces the idempotency key by target provider/model + a messages digest so fusion panel/judge sub-requests can't collide on a shared client Idempotency-Key. Existing chatCore extracted-module tests were aligned to the composed-key contract. Integrated into release/v3.8.47.
This commit is contained in:
@@ -26,6 +26,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
||||
- **fix(providers):** custom models a provider actually has are no longer dropped from the Free Provider Rankings when both **"Configured only"** and **"Available only"** filters are applied ([#6368](https://github.com/diegosouzapw/OmniRoute/issues/6368)), follow-up to #6150 — `freeProviderRankings.ts::getProviderModels()` only ever walked the static `open-sse/config/providerRegistry.ts` catalog, so a user-added custom model (e.g. a Puter `claude-fable-5` model saved as "Claude Fable 5") never entered the candidate model list the ranking scores against, and could never survive the #6150 configured/available filters even when actually configured and available. It now additively merges the provider's custom models (`db/models.ts::getCustomModels`) into that candidate list via a new pure, de-duping `mergeProviderModels()` helper, before scoring/filtering runs — catalog free/paid filtering elsewhere is untouched. Regression guard: `tests/unit/free-provider-rankings-custom-models-6368.test.ts`. (thanks @shabeer)
|
||||
- **fix(providers):** `cloudflare-ai` no longer silently drops image/non-text content parts ([#6390](https://github.com/diegosouzapw/OmniRoute/issues/6390)) — `transformRequest()`'s `flattenContent()` (added for #2539 to satisfy the Workers AI `/ai/v1/chat/completions` plain-string `content` requirement) mapped any non-text OpenAI content part (e.g. `image_url`) to `""` and joined the rest, so a request carrying an image quietly went out as text-only with the attachment gone and no error surfaced. It now throws a clear error on the first non-text part instead of dropping it silently, which the existing top-level `chatCore.ts` catch already routes through `buildErrorBody()`/`sanitizeErrorMessage()` (same pattern as `buildUrl()`'s missing-Account-ID error). Regression guard: `tests/unit/cloudflare-ai-image-parts-6390.test.ts`.
|
||||
- **fix(providers):** stop Antigravity connections from falsely reporting **all-accounts quota-exhausted** ([#6295](https://github.com/diegosouzapw/OmniRoute/issues/6295)) — `genericQuotaFetcher.ts::percentUsedForQuota()` ignored the `fractionReported` flag and defaulted an unreported model's `remainingPercentage` to 0, which computed as 100% used; since `convertUsageToQuotaInfo()` takes the worst-case window across a connection, a single model with no reported fraction dragged the whole account into `limitReached` and `quotaPreflight` skipped it. `percentUsedForQuota()` now returns `null` (unknown, window ignored) whenever `fractionReported === false`, before falling back to `remainingPercentage`. Regression guard: `tests/unit/generic-quota-fetcher.test.ts`.
|
||||
- **fix(fusion):** the fusion judge no longer replays a panel member's answer via an idempotency-key collision — fusion's panel + judge sub-requests re-enter `chatCore` sharing the client's headers, so they derived the same `Idempotency-Key`/`x-request-id` and a panel answer saved under the key was replayed by the judge's check ~1ms later (inside the 5s window), returning a panel member's answer instead of the judge synthesis (observed live on `nexa/conversation-fusion`). `composeIdempotencyKey()` now namespaces the key by target provider/model + a digest of the request messages, so sub-requests can't collide while a genuine client retry (same key/model/body) still replays. Regression guard: `tests/unit/idempotency-fusion-collision.test.ts`. ([#6558](https://github.com/diegosouzapw/OmniRoute/pull/6558)) — see PR. (thanks @developerjillur)
|
||||
- **fix(providers):** grok-cli (Grok Build) now strips `reasoning_effort`/`reasoning` before forwarding the request ([#6288](https://github.com/diegosouzapw/OmniRoute/issues/6288)) — Claude Code sends `reasoning_effort` on every request (routing the Opus slot), which Grok Build's upstream chat-proxy endpoint rejects with a 400; `transformRequest()`'s existing `UNSUPPORTED` sampling-param strip list (#5273) never covered it. Regression guard: `tests/unit/grok-cli-reasoning-strip-6288.test.ts`.
|
||||
- **fix(cli):** `omniroute serve` no longer hangs silently on a readiness timeout ([#6321](https://github.com/diegosouzapw/OmniRoute/issues/6321)) — the child server's stdout was piped to `"ignore"` whenever `--log`/`OMNIROUTE_SHOW_LOG` wasn't set (the default), discarding any debug output, and `runWithSupervisor`'s `waitForServer(...).then((up) => { if (up) {...} })` had no `else` branch, so a boot that never became ready produced zero further output after "⏳ Starting server...". Stdout is now buffered alongside stderr (`ServerSupervisor.getRecentLog()`), and a timeout prints a clear diagnostic plus the buffered output instead of staying silent. Does not by itself explain why boot never completes on a given machine — see the issue for further reproduction. Regression guard: `tests/unit/cli-serve-readiness-timeout-6321.test.ts`.
|
||||
- **fix(pricing):** Pricing Sync dashboard no longer stuck on "Next Sync: Never" / "Synced Models: 0" ([#6325](https://github.com/diegosouzapw/OmniRoute/issues/6325)) — `pricingSync.ts` kept sync state (`lastSyncTime`, `lastSyncModelCount`) in module-level vars, but the background periodic sync (`instrumentation-node.ts`) and the dashboard status route (`/api/pricing/sync`) each import the module from separate Next.js standalone webpack chunks, giving each its own independent state; `getSyncStatus()` read the (empty) API-route instance's vars. Sync status is now additionally persisted to a new `pricing_sync_status` `key_value` namespace and `getSyncStatus()` falls back to it when the local module instance never ran a sync itself. Regression guard: `tests/unit/pricing-sync-cross-instance.test.ts`.
|
||||
|
||||
@@ -562,6 +562,10 @@ export async function handleChatCore({
|
||||
clientRawRequest,
|
||||
provider,
|
||||
model,
|
||||
// NEXA fusion-idempotency fix: body.messages feeds the key digest so combo-internal
|
||||
// sub-requests (fusion panel + judge re-enter chatCore sharing the client's headers)
|
||||
// can never collide on the raw Idempotency-Key/x-request-id header key.
|
||||
body,
|
||||
effectiveServiceTier,
|
||||
startTime,
|
||||
log,
|
||||
|
||||
@@ -1,7 +1,45 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { getIdempotencyKey, checkIdempotency } from "@/lib/idempotencyLayer";
|
||||
import { calculateCost } from "@/lib/usage/costCalculator";
|
||||
import { attachOmniRouteMetaHeaders } from "@/domain/omnirouteResponseMeta";
|
||||
|
||||
/**
|
||||
* NEXA fusion-idempotency fix: compose the effective idempotency key from the raw
|
||||
* header key + target provider/model + a digest of the request messages.
|
||||
*
|
||||
* Why: combo-internal sub-requests (fusion panel members AND the judge) re-enter
|
||||
* chatCore SHARING the client's headers, so the raw `Idempotency-Key`/`x-request-id`
|
||||
* key was identical for all of them. A panel answer saved under the key and the
|
||||
* judge's check (~1ms later, well inside the 5s window) replayed it — the client
|
||||
* received a panel member's answer instead of the judge synthesis. Namespacing by
|
||||
* model separates panel members; the messages digest separates the judge even when
|
||||
* it reuses a panel member's model (the judge body appends the judge directive
|
||||
* turn). A genuine client retry (same key, same model, same body) still replays.
|
||||
*/
|
||||
export function composeIdempotencyKey({
|
||||
rawKey,
|
||||
provider,
|
||||
model,
|
||||
messages,
|
||||
}: {
|
||||
rawKey: string | null | undefined;
|
||||
provider: string;
|
||||
model: string;
|
||||
messages: unknown;
|
||||
}): string | null {
|
||||
if (!rawKey) return null;
|
||||
let digest = "";
|
||||
try {
|
||||
digest = createHash("sha256")
|
||||
.update(JSON.stringify(messages ?? ""))
|
||||
.digest("hex")
|
||||
.slice(0, 16);
|
||||
} catch {
|
||||
digest = "nodigest";
|
||||
}
|
||||
return `${rawKey}|${provider}|${model}|${digest}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the request's idempotency key once and check the idempotency store. Returns the
|
||||
* resolved `idempotencyKey` alongside the cache `hit` so the caller can reuse the SAME key
|
||||
@@ -12,6 +50,7 @@ export async function checkIdempotencyCache({
|
||||
clientRawRequest,
|
||||
provider,
|
||||
model,
|
||||
body,
|
||||
effectiveServiceTier,
|
||||
startTime,
|
||||
log,
|
||||
@@ -19,19 +58,26 @@ export async function checkIdempotencyCache({
|
||||
clientRawRequest: unknown;
|
||||
provider: string;
|
||||
model: string;
|
||||
body?: unknown;
|
||||
effectiveServiceTier: unknown;
|
||||
startTime: number;
|
||||
log: unknown;
|
||||
}): Promise<{ hit: { success: true; response: Response } | null; idempotencyKey: string }> {
|
||||
const idempotencyKey = getIdempotencyKey(clientRawRequest?.headers);
|
||||
}): Promise<{ hit: { success: true; response: Response } | null; idempotencyKey: string | null }> {
|
||||
// NEXA fusion-idempotency fix: namespace the raw header key (see composeIdempotencyKey).
|
||||
const rawIdempotencyKey = getIdempotencyKey(clientRawRequest?.headers);
|
||||
const idempotencyKey = composeIdempotencyKey({
|
||||
rawKey: rawIdempotencyKey,
|
||||
provider,
|
||||
model,
|
||||
messages: (body as { messages?: unknown } | undefined)?.messages,
|
||||
});
|
||||
const cachedIdemp = checkIdempotency(idempotencyKey);
|
||||
if (cachedIdemp) {
|
||||
log?.debug?.("IDEMPOTENCY", `Hit for key=${idempotencyKey?.slice(0, 12)}...`);
|
||||
const idempotentUsage =
|
||||
cachedIdemp.response && typeof cachedIdemp.response === "object"
|
||||
? ((cachedIdemp.response as Record<string, unknown>).usage as
|
||||
| Record<string, unknown>
|
||||
| undefined)
|
||||
Record<string, unknown> | undefined)
|
||||
: undefined;
|
||||
const idempotentCost = idempotentUsage
|
||||
? await calculateCost(provider, model, idempotentUsage as Record<string, number>, {
|
||||
|
||||
@@ -11,7 +11,10 @@ import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
import { sanitizeChatRequestBody } from "../../open-sse/handlers/chatCore/sanitization.ts";
|
||||
import { checkIdempotencyCache } from "../../open-sse/handlers/chatCore/idempotency.ts";
|
||||
import {
|
||||
checkIdempotencyCache,
|
||||
composeIdempotencyKey,
|
||||
} from "../../open-sse/handlers/chatCore/idempotency.ts";
|
||||
import { FORMATS } from "../../open-sse/translator/formats.ts";
|
||||
import { saveIdempotency } from "../../src/lib/idempotencyLayer.ts";
|
||||
|
||||
@@ -74,14 +77,32 @@ test("checkIdempotencyCache returns { hit:null, idempotencyKey } on a miss", asy
|
||||
log: undefined,
|
||||
});
|
||||
assert.equal(result.hit, null);
|
||||
assert.equal(result.idempotencyKey, "idem-miss-3821");
|
||||
// #6558: the raw header key is now namespaced by provider/model + a messages
|
||||
// digest (composeIdempotencyKey) so fusion panel/judge sub-requests can't collide.
|
||||
assert.equal(
|
||||
result.idempotencyKey,
|
||||
composeIdempotencyKey({
|
||||
rawKey: "idem-miss-3821",
|
||||
provider: "openai",
|
||||
model: "gpt-4.1",
|
||||
messages: undefined,
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
test("checkIdempotencyCache returns a hit Response reusing the same key after a save", async () => {
|
||||
const key = "idem-hit-3821";
|
||||
const rawKey = "idem-hit-3821";
|
||||
// #6558: the store is keyed by the COMPOSED key, so the save site saves under the
|
||||
// same derivation checkIdempotencyCache returns — reproduce that here.
|
||||
const key = composeIdempotencyKey({
|
||||
rawKey,
|
||||
provider: "openai",
|
||||
model: "gpt-4.1",
|
||||
messages: undefined,
|
||||
})!;
|
||||
saveIdempotency(key, { object: "chat.completion", choices: [], usage: {} }, 200);
|
||||
|
||||
const headers = new Headers({ "idempotency-key": key });
|
||||
const headers = new Headers({ "idempotency-key": rawKey });
|
||||
const result = await checkIdempotencyCache({
|
||||
clientRawRequest: { headers },
|
||||
provider: "openai",
|
||||
|
||||
99
tests/unit/idempotency-fusion-collision.test.ts
Normal file
99
tests/unit/idempotency-fusion-collision.test.ts
Normal file
@@ -0,0 +1,99 @@
|
||||
/**
|
||||
* Regression: fusion judge must not replay a panel member's cached response.
|
||||
*
|
||||
* The idempotency layer keys on the client's `Idempotency-Key` / `x-request-id`
|
||||
* header with a 5s replay window. Fusion's internal panel + judge sub-requests
|
||||
* re-enter chatCore SHARING the client's headers, so they all derived the SAME
|
||||
* key: a panel answer saved under the key, and ~1ms later the judge's check hit
|
||||
* it — the client received a panel member's answer (labeled with the judge's
|
||||
* meta headers) instead of the judge synthesis. Observed live on
|
||||
* "nexa/conversation-fusion" (body = Gemini panel answer verbatim,
|
||||
* X-OmniRoute-Idempotent: true, judge "latency" ~0ms).
|
||||
*
|
||||
* Fix: namespace the composed key by target provider/model AND a digest of the
|
||||
* request messages. Panel members differ by model; the judge differs by model
|
||||
* AND by messages (it appends the judge directive turn), so sub-requests can
|
||||
* never collide — while a genuine client retry (same key, same model, same
|
||||
* body) still replays.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { composeIdempotencyKey } from "../../open-sse/handlers/chatCore/idempotency.ts";
|
||||
|
||||
const MSGS = [{ role: "user", content: "Client asks about our PST coverage" }];
|
||||
const JUDGE_MSGS = [...MSGS, { role: "user", content: "You are the judge. Synthesize: ..." }];
|
||||
|
||||
test("no raw header -> null (idempotency disabled for the request)", () => {
|
||||
assert.equal(
|
||||
composeIdempotencyKey({
|
||||
rawKey: null,
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: MSGS,
|
||||
}),
|
||||
null
|
||||
);
|
||||
});
|
||||
|
||||
test("panel members (same raw key, same body, different models) get DIFFERENT keys", () => {
|
||||
const base = { rawKey: "req-1", messages: MSGS };
|
||||
const opus = composeIdempotencyKey({ ...base, provider: "cc", model: "claude-opus-4-6" });
|
||||
const gemini = composeIdempotencyKey({
|
||||
...base,
|
||||
provider: "antigravity",
|
||||
model: "gemini-3.1-pro-high",
|
||||
});
|
||||
const gpt = composeIdempotencyKey({ ...base, provider: "cx", model: "gpt-5.5-high" });
|
||||
assert.ok(opus && gemini && gpt);
|
||||
assert.notEqual(opus, gemini);
|
||||
assert.notEqual(gemini, gpt);
|
||||
assert.notEqual(opus, gpt);
|
||||
});
|
||||
|
||||
test("judge (same raw key, different model AND extra judge turn) never collides with a panel member", () => {
|
||||
const panel = composeIdempotencyKey({
|
||||
rawKey: "req-1",
|
||||
provider: "antigravity",
|
||||
model: "gemini-3.1-pro-high",
|
||||
messages: MSGS,
|
||||
});
|
||||
const judge = composeIdempotencyKey({
|
||||
rawKey: "req-1",
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: JUDGE_MSGS,
|
||||
});
|
||||
assert.notEqual(judge, panel);
|
||||
});
|
||||
|
||||
test("judge that reuses a panel member's model still differs (messages digest separates them)", () => {
|
||||
const panel = composeIdempotencyKey({
|
||||
rawKey: "req-1",
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: MSGS,
|
||||
});
|
||||
const judge = composeIdempotencyKey({
|
||||
rawKey: "req-1",
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: JUDGE_MSGS,
|
||||
});
|
||||
assert.notEqual(judge, panel);
|
||||
});
|
||||
|
||||
test("genuine client retry (same key + model + body) -> SAME key (replay semantics preserved)", () => {
|
||||
const a = composeIdempotencyKey({
|
||||
rawKey: "retry-9",
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: MSGS,
|
||||
});
|
||||
const b = composeIdempotencyKey({
|
||||
rawKey: "retry-9",
|
||||
provider: "cc",
|
||||
model: "claude-opus-4-8",
|
||||
messages: MSGS,
|
||||
});
|
||||
assert.equal(a, b);
|
||||
});
|
||||
Reference in New Issue
Block a user