mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-23 15:22:30 +03:00
Compare commits
2 Commits
fix/13963-
...
fix/13154-
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
20348a276f | ||
|
|
7047a5c0fd |
1
changelog.d/fixes/13154-compression-worker-clone-gate.md
Normal file
1
changelog.d/fixes/13154-compression-worker-clone-gate.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(sse): widen compression worker eligibility gate to accept structured-clone-safe `undefined`/Date/Map/Set/RegExp values, restoring worker offload for real requests (#13154)
|
||||
@@ -4,10 +4,36 @@ import {
|
||||
applyStackedCompression,
|
||||
type StackedCompressionStep,
|
||||
} from "./strategySelector.ts";
|
||||
import { adaptBodyForCompression } from "./bodyAdapter.ts";
|
||||
import type {
|
||||
CompressionWorkerJob,
|
||||
CompressionWorkerMessage,
|
||||
} from "./compressionWorkerProtocol.ts";
|
||||
import type { CompressionResult } from "./types.ts";
|
||||
|
||||
// #13154 follow-up: `applyCompression`'s sync/in-process "stacked" branch runs every body
|
||||
// through `adaptBodyForCompression` first (Responses `input[]` and Kiro `conversationState`
|
||||
// envelopes get flattened to `messages[]`, then restored after compression) — see
|
||||
// strategySelector.ts's `runCompression`. Before the worker-eligibility gate widening in this
|
||||
// same fix, essentially no real "stacked" call ever reached the worker (any `undefined`
|
||||
// option key rejected it), so this branch calling `applyStackedCompression` directly on the
|
||||
// raw body was dead code. Now that eligible calls are actually routed here, it must mirror
|
||||
// that same adapt/restore step or Responses/Kiro bodies get miscompressed (wrong shape, and
|
||||
// hard-budget post-pass warnings silently lost) only when the worker happens to run them.
|
||||
function runStackedJob(
|
||||
job: CompressionWorkerJob,
|
||||
onEngineStep: (step: StackedCompressionStep) => void
|
||||
): CompressionResult {
|
||||
const adapter = adaptBodyForCompression(
|
||||
job.body,
|
||||
job.options?.config?.codexResponsesConfig?.preserveToolNames
|
||||
);
|
||||
const result = applyStackedCompression(adapter.body, job.options?.config?.stackedPipeline, {
|
||||
...job.options,
|
||||
onEngineStep,
|
||||
});
|
||||
return adapter.adapted ? { ...result, body: adapter.restore(result.body) } : result;
|
||||
}
|
||||
|
||||
if (!parentPort) throw new Error("compressionWorker must run in a worker thread");
|
||||
parentPort.on("message", (job: CompressionWorkerJob) => {
|
||||
@@ -20,10 +46,7 @@ parentPort.on("message", (job: CompressionWorkerJob) => {
|
||||
} satisfies CompressionWorkerMessage);
|
||||
const result =
|
||||
job.mode === "stacked"
|
||||
? applyStackedCompression(job.body, job.options?.config?.stackedPipeline, {
|
||||
...job.options,
|
||||
onEngineStep,
|
||||
})
|
||||
? runStackedJob(job, onEngineStep)
|
||||
: applyCompression(job.body, job.mode, job.options);
|
||||
parentPort.postMessage({
|
||||
id: job.id,
|
||||
|
||||
@@ -33,24 +33,34 @@ function isPlainObject(value: object): value is Record<string, unknown> {
|
||||
return prototype === Object.prototype || prototype === null;
|
||||
}
|
||||
|
||||
// Anything that is not a (non-null) object is either a structured-clone-safe primitive
|
||||
// or an unsupported value (e.g. a non-finite number, a function, a symbol). Isolated
|
||||
// from `isStrictlySerializable` so the recursive walk below stays flat.
|
||||
function isClonablePrimitive(value: unknown): boolean {
|
||||
if (value === null || value === undefined) return true;
|
||||
if (typeof value === "string" || typeof value === "boolean") return true;
|
||||
if (typeof value === "number") return Number.isFinite(value);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Date/Map/Set/RegExp are copied natively by structuredClone (not walked as plain
|
||||
// objects), so they are always structured-clone-safe regardless of their contents.
|
||||
const NATIVELY_CLONABLE_CTORS = [Date, Map, Set, RegExp] as const;
|
||||
function isNativelyClonable(value: object): boolean {
|
||||
return NATIVELY_CLONABLE_CTORS.some((ctor) => value instanceof ctor);
|
||||
}
|
||||
|
||||
// `seen` tracks only the current recursion PATH (ancestors), not every node ever visited:
|
||||
// add before descending, remove after returning. That way a real cycle (a node reachable
|
||||
// from itself) is still rejected, but two sibling branches that happen to reference the
|
||||
// SAME non-cyclic sub-object (a false positive with a globally-shared `seen` set) are not.
|
||||
export function isStrictlySerializable(value: unknown, seen = new Set<object>()): boolean {
|
||||
if (
|
||||
value === null ||
|
||||
typeof value === "string" ||
|
||||
typeof value === "boolean" ||
|
||||
typeof value === "number"
|
||||
) {
|
||||
return typeof value !== "number" || Number.isFinite(value);
|
||||
}
|
||||
if (typeof value !== "object") return false;
|
||||
if (value === null || typeof value !== "object") return isClonablePrimitive(value);
|
||||
if (seen.has(value)) return false;
|
||||
seen.add(value);
|
||||
try {
|
||||
if (Array.isArray(value)) return value.every((entry) => isStrictlySerializable(entry, seen));
|
||||
if (isNativelyClonable(value)) return true;
|
||||
if (!isPlainObject(value)) return false;
|
||||
return Object.values(value).every((entry) => isStrictlySerializable(entry, seen));
|
||||
} finally {
|
||||
|
||||
69
tests/unit/compression-worker-gate-13154-repro.test.ts
Normal file
69
tests/unit/compression-worker-gate-13154-repro.test.ts
Normal file
@@ -0,0 +1,69 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { describe, it } from "node:test";
|
||||
import {
|
||||
isCompressionWorkerEligible,
|
||||
isStrictlySerializable,
|
||||
} from "../../open-sse/services/compression/compressionWorkerProtocol.ts";
|
||||
import type { CompressionConfig } from "../../open-sse/services/compression/types.ts";
|
||||
|
||||
const body = {
|
||||
model: "gpt-test",
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
};
|
||||
const config = {
|
||||
enabled: true,
|
||||
defaultMode: "stacked",
|
||||
autoTriggerTokens: 1,
|
||||
cacheMinutes: 0,
|
||||
preserveSystemPrompt: true,
|
||||
stackedPipeline: [{ engine: "rtk" }, { engine: "caveman" }],
|
||||
} as CompressionConfig;
|
||||
|
||||
describe("#13154: compression worker gate rejects structured-cloneable bodies", () => {
|
||||
it("structuredClone accepts a workerOptions shape with an explicit `undefined` key", () => {
|
||||
const workerOptions = {
|
||||
model: "gpt-test",
|
||||
supportsVision: true,
|
||||
providerTransport: "direct" as const,
|
||||
provider: undefined,
|
||||
imageTransportFidelity: "unknown" as const,
|
||||
sourceFormat: "chat" as const,
|
||||
targetFormat: "chat" as const,
|
||||
compressionStage: "pre-translation" as const,
|
||||
config,
|
||||
};
|
||||
assert.doesNotThrow(() => structuredClone({ body, mode: "stacked", options: workerOptions }));
|
||||
});
|
||||
|
||||
it("the gate should accept that same structured-cloneable shape", () => {
|
||||
const workerOptions = {
|
||||
model: "gpt-test",
|
||||
supportsVision: true,
|
||||
providerTransport: "direct" as const,
|
||||
provider: undefined,
|
||||
imageTransportFidelity: "unknown" as const,
|
||||
sourceFormat: "chat" as const,
|
||||
targetFormat: "chat" as const,
|
||||
compressionStage: "pre-translation" as const,
|
||||
config,
|
||||
};
|
||||
assert.equal(isStrictlySerializable({ body, mode: "stacked", options: workerOptions }), true);
|
||||
assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
|
||||
});
|
||||
|
||||
it("realistic runCompressionAsync-shaped call (only `provider` unset) should be eligible", () => {
|
||||
const workerOptions = {
|
||||
model: "gpt-test",
|
||||
supportsVision: undefined,
|
||||
providerTransport: undefined,
|
||||
provider: undefined,
|
||||
imageTransportFidelity: undefined,
|
||||
sourceFormat: undefined,
|
||||
targetFormat: undefined,
|
||||
compressionStage: undefined,
|
||||
config,
|
||||
};
|
||||
assert.doesNotThrow(() => structuredClone({ body, mode: "stacked", options: workerOptions }));
|
||||
assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
|
||||
});
|
||||
});
|
||||
@@ -79,17 +79,8 @@ describe("compression worker eligibility", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects functions, symbols, classes, special objects, cycles, and non-finite numbers", () => {
|
||||
for (const value of [
|
||||
() => undefined,
|
||||
Symbol("x"),
|
||||
new Date(),
|
||||
new Map(),
|
||||
new Set(),
|
||||
/x/,
|
||||
NaN,
|
||||
Infinity,
|
||||
]) {
|
||||
it("rejects functions, symbols, cycles, and non-finite numbers", () => {
|
||||
for (const value of [() => undefined, Symbol("x"), NaN, Infinity]) {
|
||||
assert.equal(isStrictlySerializable(value), false);
|
||||
}
|
||||
const cyclic: Record<string, unknown> = {};
|
||||
@@ -97,6 +88,34 @@ describe("compression worker eligibility", () => {
|
||||
assert.equal(isStrictlySerializable(cyclic), false);
|
||||
});
|
||||
|
||||
it("#13154: accepts structured-clone-native Date/Map/Set/RegExp values", () => {
|
||||
for (const value of [new Date(), new Map(), new Set(), /x/]) {
|
||||
assert.equal(isStrictlySerializable(value), true);
|
||||
}
|
||||
});
|
||||
|
||||
it("#13154: accepts `undefined` values instead of rejecting the whole tree", () => {
|
||||
assert.equal(isStrictlySerializable(undefined), true);
|
||||
assert.equal(isStrictlySerializable({ provider: undefined, model: "gpt-test" }), true);
|
||||
});
|
||||
|
||||
it("#13154: accepts strategySelector.ts's exact 9-key workerOptions shape with `provider` unset", () => {
|
||||
// Mirrors runCompressionAsync's workerOptions object: all 9 keys always present,
|
||||
// `provider` commonly unresolved (undefined) at call time.
|
||||
const workerOptions = {
|
||||
model: "gpt-test",
|
||||
supportsVision: undefined,
|
||||
providerTransport: undefined,
|
||||
provider: undefined,
|
||||
imageTransportFidelity: undefined,
|
||||
sourceFormat: undefined,
|
||||
targetFormat: undefined,
|
||||
compressionStage: undefined,
|
||||
config,
|
||||
};
|
||||
assert.equal(isCompressionWorkerEligible(body, "stacked", workerOptions), true);
|
||||
});
|
||||
|
||||
it("#13154: does not misread a shared (non-cyclic) sub-object referenced by two sibling branches as a cycle", () => {
|
||||
// Original bug: a single `seen` set shared across the whole recursion tree (never
|
||||
// backtracked) meant visiting the SAME object twice via two different, non-cyclic
|
||||
|
||||
Reference in New Issue
Block a user