mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
refactor(sse): restore three executor types the runtime already relied on (#8520)
Three independent root causes in the TS 7 executor slice (#8484), each a declaration that had drifted behind the code using it. All type-only — no behavior change, verified by running the new tests against the parent commit's sources (7/7 pass there too). mimocode — AccountState was missing `proxy`, but syncAccountsFromCredentials() writes it on every account and getProxyDispatcher() reads it. The #3837/#5521 contract ("always present, null when unconfigured") lived only in a comment. Declared as AccountProxyConfig["proxy"] so the two stay in lockstep. (4) opencode — the tools-truncation block narrowed `modifiedBody` with `typeof === "object"` but, unlike the client_metadata block directly above it, omitted `!Array.isArray()` and the Record cast, so `.tools` was unreachable on `object`. Adopted the neighbouring block's idiom. Behavior is identical: the old code reached `.tools` on arrays too and relied on Array.isArray(undefined) short-circuiting. (4) zed-hosted — enqueueSseObject/finish/processLine were annotated ReadableStreamDefaultController but are driven from a TransformStream, whose controller has no close(). All three only ever enqueue, so they are now typed by that single capability (SseEnqueueTarget). (3) 310 -> 299 diagnostics, zero new, on a line-number-agnostic diff of the full tsc error set. Tests: new tests/unit/ts7-executor-shared-shapes.test.ts pins the tools truncation (previously uncovered) and the proxy-always-present contract. The zed-hosted transform is already covered end-to-end by zed-hosted-think-close-marker.test.ts. 300/300 across the 26 affected files. Co-authored-by: backryun <busan011@ormbiz.co.kr>
This commit is contained in:
@@ -100,6 +100,12 @@ interface AccountState {
|
||||
expiresAt: number;
|
||||
cooldownUntil: number;
|
||||
consecutiveFails: number;
|
||||
/**
|
||||
* #3837/#5521: the account's resolved proxy, or `null` when none is configured.
|
||||
* Always present (never `undefined`) so callers can read `acct.proxy` directly —
|
||||
* syncAccountsFromCredentials() writes it on every account on every sync.
|
||||
*/
|
||||
proxy: AccountProxyConfig["proxy"];
|
||||
}
|
||||
|
||||
function parseJwtExp(jwt: string): number {
|
||||
|
||||
@@ -338,13 +338,11 @@ export class OpencodeExecutor extends BaseExecutor {
|
||||
) {
|
||||
delete (modifiedBody as Record<string, unknown>).client_metadata;
|
||||
}
|
||||
if (
|
||||
modifiedBody &&
|
||||
typeof modifiedBody === "object" &&
|
||||
Array.isArray(modifiedBody.tools) &&
|
||||
modifiedBody.tools.length > 128
|
||||
) {
|
||||
modifiedBody.tools = modifiedBody.tools.slice(0, 128);
|
||||
if (modifiedBody && typeof modifiedBody === "object" && !Array.isArray(modifiedBody)) {
|
||||
const mb = modifiedBody as Record<string, unknown>;
|
||||
if (Array.isArray(mb.tools) && mb.tools.length > 128) {
|
||||
mb.tools = mb.tools.slice(0, 128);
|
||||
}
|
||||
}
|
||||
if (modifiedBody && typeof modifiedBody === "object" && !Array.isArray(modifiedBody)) {
|
||||
const mb = modifiedBody as Record<string, unknown>;
|
||||
|
||||
@@ -117,8 +117,18 @@ function createErrorChunk(model: string, message: string): Record<string, unknow
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The single controller capability these SSE helpers use. They only ever enqueue —
|
||||
* never `close()`, never read `desiredSize` — so typing them by that one method lets
|
||||
* the same code serve both stream kinds. The wider
|
||||
* `ReadableStreamDefaultController` annotation rejected every call site, because the
|
||||
* helpers are driven from a TransformStream and `TransformStreamDefaultController`
|
||||
* has no `close()`.
|
||||
*/
|
||||
type SseEnqueueTarget = Pick<ReadableStreamDefaultController<Uint8Array>, "enqueue">;
|
||||
|
||||
function enqueueSseObject(
|
||||
controller: ReadableStreamDefaultController<Uint8Array>,
|
||||
controller: SseEnqueueTarget,
|
||||
encoder: TextEncoder,
|
||||
chunk: unknown
|
||||
): void {
|
||||
@@ -202,7 +212,7 @@ function wrapZedCompletionStream(
|
||||
let buffer = "";
|
||||
let done = false;
|
||||
|
||||
const finish = (controller: ReadableStreamDefaultController<Uint8Array>) => {
|
||||
const finish = (controller: SseEnqueueTarget) => {
|
||||
if (done) return;
|
||||
const finalChunk = convertProviderEvent(provider, null, state);
|
||||
enqueueSseObject(controller, encoder, finalChunk);
|
||||
@@ -210,7 +220,7 @@ function wrapZedCompletionStream(
|
||||
done = true;
|
||||
};
|
||||
|
||||
const processLine = (line: string, controller: ReadableStreamDefaultController<Uint8Array>) => {
|
||||
const processLine = (line: string, controller: SseEnqueueTarget) => {
|
||||
if (done) return;
|
||||
const payload = unwrapZedLine(line);
|
||||
if (!payload) return;
|
||||
|
||||
145
tests/unit/ts7-executor-shared-shapes.test.ts
Normal file
145
tests/unit/ts7-executor-shared-shapes.test.ts
Normal file
@@ -0,0 +1,145 @@
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { OpencodeExecutor } = await import("../../open-sse/executors/opencode.ts");
|
||||
const { MimocodeExecutor } = await import("../../open-sse/executors/mimocode.ts");
|
||||
|
||||
/**
|
||||
* Behavioral guards for the three type-only fixes in the TS 7 executor slice
|
||||
* (see #8484). Each fix restored a type the code already depended on at runtime;
|
||||
* these tests pin the runtime contracts so a future "simplification" of the
|
||||
* annotations cannot silently change behavior.
|
||||
*
|
||||
* The zed-hosted `SseEnqueueTarget` fix is already covered end-to-end by
|
||||
* `zed-hosted-think-close-marker.test.ts`, which drives the same TransformStream
|
||||
* that failed to type-check — no duplicate added here.
|
||||
*/
|
||||
|
||||
describe("OpencodeExecutor — tools truncation survives the narrowing fix", () => {
|
||||
const executor = new OpencodeExecutor("opencode-go");
|
||||
const CREDENTIALS = { apiKey: "k" } as Record<string, unknown>;
|
||||
|
||||
const tools = (n: number) =>
|
||||
Array.from({ length: n }, (_, i) => ({
|
||||
type: "function",
|
||||
function: { name: `tool_${i}`, parameters: {} },
|
||||
}));
|
||||
|
||||
function bodyWith(toolCount: number) {
|
||||
return {
|
||||
model: "oc/kimi-k2.6",
|
||||
stream: true,
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
tools: tools(toolCount),
|
||||
};
|
||||
}
|
||||
|
||||
it("truncates an over-long tools array to 128 entries", () => {
|
||||
const out = executor.transformRequest("oc/kimi-k2.6", bodyWith(200), true, CREDENTIALS) as {
|
||||
tools: unknown[];
|
||||
};
|
||||
assert.equal(out.tools.length, 128, "upstream rejects more than 128 tools");
|
||||
assert.deepEqual(
|
||||
(out.tools[127] as { function: { name: string } }).function.name,
|
||||
"tool_127",
|
||||
"truncation keeps the first 128 in order, not an arbitrary slice"
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves a within-limit tools array untouched", () => {
|
||||
const out = executor.transformRequest("oc/kimi-k2.6", bodyWith(10), true, CREDENTIALS) as {
|
||||
tools: unknown[];
|
||||
};
|
||||
assert.equal(out.tools.length, 10);
|
||||
});
|
||||
|
||||
it("is a no-op when the body carries no tools", () => {
|
||||
const body = {
|
||||
model: "oc/kimi-k2.6",
|
||||
stream: true,
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
};
|
||||
const out = executor.transformRequest("oc/kimi-k2.6", body, true, CREDENTIALS) as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
assert.equal("tools" in out, false);
|
||||
assert.ok(Array.isArray(out.messages), "messages preserved");
|
||||
});
|
||||
|
||||
it("leaves an array-shaped body alone (pins the !Array.isArray guard)", () => {
|
||||
// The pre-fix condition reached `.tools` on any object, arrays included, and
|
||||
// relied on `Array.isArray(undefined)` short-circuiting. The explicit
|
||||
// !Array.isArray() guard must keep that outcome identical.
|
||||
const arrayBody = [{ role: "user", content: "hi" }] as unknown as Record<string, unknown>;
|
||||
const out = executor.transformRequest("oc/kimi-k2.6", arrayBody, true, CREDENTIALS);
|
||||
assert.ok(Array.isArray(out), "array body must pass through as an array");
|
||||
assert.equal((out as unknown[]).length, 1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("MimocodeExecutor — AccountState.proxy is always present (#3837/#5521)", () => {
|
||||
const FP = "fingerprint-1";
|
||||
|
||||
function accountsOf(exec: unknown): Array<Record<string, unknown>> {
|
||||
return (exec as { accounts: Array<Record<string, unknown>> }).accounts;
|
||||
}
|
||||
|
||||
function sync(exec: unknown, credentials: unknown): void {
|
||||
(exec as { syncAccountsFromCredentials(c: unknown): void }).syncAccountsFromCredentials(
|
||||
credentials
|
||||
);
|
||||
}
|
||||
|
||||
it("defaults proxy to null — not undefined — when no accountProxies are configured", () => {
|
||||
const exec = new MimocodeExecutor();
|
||||
accountsOf(exec).length = 0;
|
||||
accountsOf(exec).push({
|
||||
fingerprint: FP,
|
||||
jwt: "",
|
||||
expiresAt: 0,
|
||||
cooldownUntil: 0,
|
||||
consecutiveFails: 0,
|
||||
});
|
||||
|
||||
sync(exec, { providerSpecificData: {} });
|
||||
|
||||
const account = accountsOf(exec)[0];
|
||||
assert.ok("proxy" in account, "every account must expose a proxy key");
|
||||
assert.equal(account.proxy, null, "unconfigured proxy is null, never undefined");
|
||||
});
|
||||
|
||||
it("resolves a configured proxy onto the matching account", () => {
|
||||
const exec = new MimocodeExecutor();
|
||||
accountsOf(exec).length = 0;
|
||||
accountsOf(exec).push({
|
||||
fingerprint: FP,
|
||||
jwt: "",
|
||||
expiresAt: 0,
|
||||
cooldownUntil: 0,
|
||||
consecutiveFails: 0,
|
||||
});
|
||||
|
||||
const proxy = { type: "http", host: "p1.example.com", port: 1080 };
|
||||
sync(exec, { providerSpecificData: { accountProxies: [{ fingerprint: FP, proxy }] } });
|
||||
|
||||
assert.deepEqual(accountsOf(exec)[0].proxy, proxy);
|
||||
});
|
||||
|
||||
it("clears a previously-resolved proxy back to null when config drops it", () => {
|
||||
const exec = new MimocodeExecutor();
|
||||
accountsOf(exec).length = 0;
|
||||
accountsOf(exec).push({
|
||||
fingerprint: FP,
|
||||
jwt: "",
|
||||
expiresAt: 0,
|
||||
cooldownUntil: 0,
|
||||
consecutiveFails: 0,
|
||||
proxy: { type: "http", host: "stale.example.com", port: 8080 },
|
||||
});
|
||||
|
||||
sync(exec, { providerSpecificData: { accountProxies: [] } });
|
||||
|
||||
assert.equal(accountsOf(exec)[0].proxy, null, "a removed proxy must not linger on the account");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user