mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-13 18:52:18 +03:00
* fix: complete Z.ai web browser transport * refactor: address Z.ai review feedback * test(zai-web): reconcile the #8014 endpoint guard with the chats/new + signed flow Rebasing onto release/v3.8.49 pulled in #8503, which repointed CHAT_URL to /api/v2/chat/completions and added an endpoint probe. This branch already targets v2, so the executor conflict resolved to this branch's superset (NEW_CHAT_URL + signature constants alongside the same v2 CHAT_URL). The two tests needed adapting, because #8503's assertions assume the pre-rework flow: - executor-zai-web.test.ts: the completion URL now carries the request signature as a query string, so an exact-equality check on the endpoint can never match. Assert the v2 prefix instead. - zai-web-chat-endpoint-8014-probe.test.ts: the probe drove the executor with a bare cookie credential and no captcha proof, which now routes through the browser transport — fetch was never called and the probe captured nothing. Supplied a direct-path credential, and matched on pathname across all requests (the executor also probes the homepage for the frontend version and calls /api/v1/chats/new first). The guard's intent is unchanged and slightly strengthened: it now asserts no request reaches the stale unversioned path and that exactly one completions request is issued, against v2. 54/54 across the zai suites; typecheck:core and eslint clean. * fix(zai-web): surface upstream error frames instead of finishing empty Reported on this PR: HTTP 200, `out=0`, stream "complete", no content and no diagnosis. Cause. HTTP-level failures are already handled — fetchUpstream turns any !ok response into a makeErrorResult with the sanitized body. The gap is a 200 whose SSE body carries an error payload: parseZaiFrame returns null for it, drainSseDeltas drops it, and buildZaiStreamingBody then closes with an empty assistant message + stop + [DONE]. The caller reads that as a successful empty completion, so a rejected signature, an expired captcha and a stale token all look identical — which is why this had to be diagnosed by reading code rather than logs. Hard Rule #6. Fix. parseZaiFrame now classifies an affirmatively error-shaped frame (`error` at the top level or under `data`, string or {detail|message|msg}) as a terminal delta, checked before the delta paths so it cannot fall through to the "no usable delta" null. The stream emits it as `[Z.ai error] <message>`, matching the mid-stream convention the other web executors already use (zed-hosted's createErrorChunk) — the 200 is on the wire, so the status cannot change, but the caller must not be left reading a blank success. Content streamed before the failure is preserved. Message goes through sanitizeErrorMessage (Rule #12). Deliberately NOT changed: a contentless frame still parses to null. That is live-validated behaviour, not an oversight — z.ai emits phase frames with no delta_content, and executor-zai-web.test.ts pins it ("returns null for frames with no usable delta"). Treating "nothing parseable arrived" as a failure would invent policy on top of an observed protocol and risk false errors on the happy path, so this only adds recognition of explicit error frames. Tests (TDD, RED then GREEN): zai-web-silent-empty-repro.test.ts — 7 cases. Error frame classified and terminal; surfaced through the stream with the upstream's own text; surfaced after partial content without losing it; plus a REGRESSION GUARD that contentless/phase-only frames are still skipped, and two controls that the happy path and reasoning-only output are untouched. The guard and controls passed before the fix; the four error cases did not. 94/94 across the zai + stream suites; typecheck:core, eslint and check:file-size clean. * refactor(sse): extract the zai-web transports so the complexity ratchet holds The v3.8.49 merge-train rebaseline (#8686) set the ceiling to the tip's own measurement, leaving zero headroom, so this branch's +5 cyclomatic / +3 cognitive own-growth had nowhere to sit once rebased onto it. Eight violations, all in code this branch introduces, resolved by extraction — no behaviour change: - `execute` (152 lines, complexity 25, cognitive 20) now delegates to `resolveZaiRequest()` for the four client-error rejections and to a `fetchViaSignedApi()` method for the CAPTCHA/signature path, so it reads as "validate, pick a transport, shape the response". - `fetchThroughBrowser` (126 lines, cognitive 16) hands its image decoding to `resolveZaiBrowserAttachments()`, its Playwright options to `buildZaiBrowserChatOptions()`, and its call-log payload to `buildZaiBrowserAuditBody()`. - `configureZaiBrowserEffort` (cognitive 35 — the worst of the set) repeated a wrap-and-relabel try/catch four times inside an if/else. `runStage`, which already existed one function below, is now module-scoped and reused, and the toggle collapses to `checked !== config.enabled` (same four cases). - `validateWebCookieProvider` (complexity 19) moves its can-we-probe-this cascade into `resolveWebCookieProbe()`, which returns either a rejection or the URL + headers to use. - `acquireBrowserContext`'s creation closure (complexity 17) hands cookie and localStorage seeding to `seedContextSession()`. That last extraction also clears a violation that predates this branch — `acquireBrowserContext` was already over the 80-line ceiling — so cyclomatic lands at 2187 against a baseline of 2188. Verified: check:complexity-ratchets green both metrics; typecheck:core clean; ESLint clean on all four files; 85 tests across the zai-web, web-cookie validation, browser-pool and model-test-runner suites pass. * fix(zai-web): surface upstream errors on the non-streaming path collectZaiNonStreaming ignored delta.error — a 200 whose SSE body carries an error frame (rejected signature, expired captcha, stale token) came back as a successful empty completion. Now it throws on an error frame, matching the streaming path's [Z.ai error] convention; the caller's existing try/catch returns makeErrorResult(502) instead of an empty 200. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: backryun <busan011@ormbiz.co.kr> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
108 lines
4.6 KiB
TypeScript
108 lines
4.6 KiB
TypeScript
// Regression test for #7058 — zai-web (and every other entry-bearing web-cookie
|
|
// provider) never honored a configured HTTP/SOCKS proxy during connection-test /
|
|
// cookie validation.
|
|
//
|
|
// Root cause: validateWebCookieProvider() probed `${baseUrl}/models` via
|
|
// directHttpsRequest(), which hardcodes `bypassProxyPatch: true` — forcing
|
|
// safeOutboundFetch to use the pre-patch native fetch and skip proxy-context/
|
|
// env-var resolution entirely. That bypass was introduced in #3226 as a narrow,
|
|
// documented exception for a single NVIDIA NIM workaround
|
|
// (see tests/unit/proxy-bypass-scope-guard-3226.test.ts) but validateWebCookieProvider
|
|
// adopted it as its default transport from inception (#4023), silently extending the
|
|
// bypass to every web-cookie provider with a registry entry (zai-web among them).
|
|
//
|
|
// This test proves the cookie-validation probe reaches a local forward proxy
|
|
// (via a real CONNECT tunnel — the same mechanism undici uses for both HTTP and
|
|
// HTTPS targets) when one is configured via HTTP_PROXY, exactly like the
|
|
// specialty web-cookie validators (chatgpt-web, grok-web, ...) already do via
|
|
// validationRead/validationWrite.
|
|
import test from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import http from "node:http";
|
|
import net from "node:net";
|
|
|
|
const { validateWebCookieProvider } = await import("../../src/lib/providers/validation.ts");
|
|
const { REGISTRY } = await import("../../open-sse/config/providerRegistry.ts");
|
|
const { clearDispatcherCache } = await import("../../open-sse/utils/proxyDispatcher.ts");
|
|
|
|
const zaiWebEntry = REGISTRY["zai-web"] as { baseUrl?: string } | undefined;
|
|
const ORIGINAL_BASE_URL = zaiWebEntry?.baseUrl;
|
|
const ORIGINAL_HTTP_PROXY = process.env.HTTP_PROXY;
|
|
|
|
test.after(() => {
|
|
if (zaiWebEntry && ORIGINAL_BASE_URL !== undefined) {
|
|
zaiWebEntry.baseUrl = ORIGINAL_BASE_URL;
|
|
}
|
|
if (ORIGINAL_HTTP_PROXY === undefined) {
|
|
delete process.env.HTTP_PROXY;
|
|
} else {
|
|
process.env.HTTP_PROXY = ORIGINAL_HTTP_PROXY;
|
|
}
|
|
clearDispatcherCache();
|
|
});
|
|
|
|
test("zai-web cookie validation routes through the configured HTTP_PROXY (#7058)", async () => {
|
|
assert.ok(
|
|
zaiWebEntry,
|
|
"zai-web must have a providerRegistry entry for this test to be meaningful"
|
|
);
|
|
|
|
// Stand-in for chat.z.ai's /models probe target.
|
|
let targetPath = "";
|
|
let targetAuthorization = "";
|
|
const target = http.createServer((req, res) => {
|
|
targetPath = req.url ?? "";
|
|
targetAuthorization = req.headers.authorization ?? "";
|
|
res.writeHead(200, { "content-type": "application/json" });
|
|
res.end("{}");
|
|
});
|
|
await new Promise<void>((resolve) => target.listen(0, () => resolve()));
|
|
const targetPort = (target.address() as net.AddressInfo).port;
|
|
|
|
// Minimal forward proxy that only speaks CONNECT (like a real corporate proxy) and
|
|
// always tunnels to the local target above, regardless of the requested host — this
|
|
// lets the "upstream" host be a non-resolvable placeholder without any real DNS
|
|
// dependency, while still proving the request actually reached the proxy.
|
|
let sawConnect = false;
|
|
const proxy = http.createServer((_req, res) => {
|
|
res.writeHead(501);
|
|
res.end("CONNECT only");
|
|
});
|
|
proxy.on("connect", (_req, socket) => {
|
|
sawConnect = true;
|
|
const upstream = net.connect(targetPort, "127.0.0.1", () => {
|
|
socket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
|
|
upstream.pipe(socket);
|
|
socket.pipe(upstream);
|
|
});
|
|
upstream.on("error", () => socket.destroy());
|
|
socket.on("error", () => upstream.destroy());
|
|
});
|
|
await new Promise<void>((resolve) => proxy.listen(0, () => resolve()));
|
|
const proxyPort = (proxy.address() as net.AddressInfo).port;
|
|
|
|
// A non-local-looking hostname: isLocalAddress()/resolveProxyForRequest() force a
|
|
// direct connection for any 127.*/localhost/LAN target, which would defeat this test.
|
|
zaiWebEntry!.baseUrl = "http://zai-web-validation-probe-7058.invalid";
|
|
process.env.HTTP_PROXY = `http://127.0.0.1:${proxyPort}`;
|
|
clearDispatcherCache();
|
|
|
|
try {
|
|
const result = await validateWebCookieProvider({ provider: "zai-web", apiKey: "token=fake" });
|
|
|
|
assert.equal(
|
|
sawConnect,
|
|
true,
|
|
"BUG #7058: zai-web cookie validation never reached the configured HTTP_PROXY " +
|
|
"(bypassProxyPatch:true unconditionally uses the native, unpatched fetch)"
|
|
);
|
|
assert.equal(result.valid, true, `expected a valid session, got ${JSON.stringify(result)}`);
|
|
assert.equal(targetPath, "/api/models");
|
|
assert.equal(targetAuthorization, "Bearer fake");
|
|
} finally {
|
|
target.close();
|
|
proxy.close();
|
|
clearDispatcherCache();
|
|
}
|
|
});
|