diff --git a/.github/workflows/test-pytest.yml b/.github/workflows/test-pytest.yml index 94a94444..8646c659 100644 --- a/.github/workflows/test-pytest.yml +++ b/.github/workflows/test-pytest.yml @@ -22,7 +22,7 @@ jobs: strategy: matrix: python-version: ["3.10", "3.11", "3.12", "3.13"] - node-version: [20, 22, 24] + node-version: [20, 22, 24, 26] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: { persist-credentials: false } diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index adf8610d..591e2020 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [20, 22, 24] + node-version: [20, 22, 24, 26] steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: { persist-credentials: false } diff --git a/CHANGELOG.md b/CHANGELOG.md index a7002585..ea9fa192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - **`X-AIMock-Strict` is parsed case-insensitively and tolerates surrounding whitespace.** `resolveStrictMode` compared the raw header value against `"true"` / `"false"` / `"1"` / `"0"` literally, so the variants clients, proxies and shell snippets actually emit — `True`, `TRUE`, `" true "` — matched nothing and fell through to the server default. A caller on a `--strict false` server asking for `X-AIMock-Strict: True` kept getting `404` instead of `503`, and on a `--strict true` server `X-AIMock-Strict: False` stayed strict; the same header gates reasoning suppression, so the mismatch could also surface as a flaky reasoning assertion. The value is now trimmed and lower-cased before comparison. Unrecognised values still fall back to the server default, and trimming is end-only — `"tr ue"` is still not a match (#408) +- **A non-WebSocket upgrade probe no longer 404s, and its request body survives on every supported Node.** Node fires `"upgrade"` rather than `"request"` for ANY `Connection: Upgrade` header, whatever protocol is named, so langchain4j's OkHttp client — which speculatively sends `Upgrade: h2c` on plain requests to probe for HTTP/2 cleartext — had `GET /api/tags` and `POST /api/chat` land in the WebSocket-only handler and take an unconditional `404`. Such a request is now replayed onto the socket with `Upgrade`/`Connection` stripped and re-parsed as an ordinary request. **Where the body lives depends on the Node version, and both are replayed:** Node <= 24 detaches the parser at `"upgrade"` and leaves body bytes in `head`, while Node >= 26 parses them onto `req` and leaves `head` empty — with the bytes on neither the socket nor `head`, so a `head`-only replay left the re-parsed request waiting on a `Content-Length` worth of bytes that never arrived and the connection hung open instead of answering. Real WebSocket upgrades are untouched (the guard matches the `Upgrade` token case-insensitively, so `websocket` and `WebSocket` both still get `101`), the replayed request still passes the `AIMOCK_API_KEYS` boundary rather than bypassing it, and keep-alive reuse of the same socket afterwards still works. The unit and pytest CI matrices gain Node 26, which is inside the declared `engines` range (`>=20.15.0`) and was previously untested — the gap that let the body-loss case pass CI (#410) - **The AG-UI drift collector no longer reports "clean" for a failure it could not read.** `collectAgUiDriftEntries` recognizes three failure-message shapes; any FAILED assertion matching none of them was dropped outright — no entry, no counter, no warning — and a FAILED assertion carrying no message at all was skipped before it was even looked at. Both paths produced zero entries, exit 0 and `conclusion: "clean"`, so a genuinely failing `agui-schema.drift.ts` assertion (`should parse aimock event types` fails with a bare `expect` message that matches no shape) certified AG-UI as drift-free. Every such failure now takes the collector's EXISTING quarantine lane — held for review at exit 5, the same "never silently swallowed" contract the HTTP leg already used — per-assertion and unconditional, so an unreadable failure survives a mixed run in which other failures DID parse. Separately, a ZERO-exit AG-UI run whose stdout is not vitest JSON now THROWS instead of returning an empty (`"no failures"`) result, matching what the HTTP twin `runDriftTests()` has always done on the same condition: the two legs no longer disagree about whether garbage output is clean. Drift-harness tooling, not runtime behavior — no published surface changes (#391) - **The AG-UI drift suite's canonical-schema parser no longer drops the field declared after a trailing comment.** `extractExtendFields` stripped whole-line comments only (`/^\s*\/\/.*$/gm`). Upstream's `STATE_DELTA` declares `delta: z.array(z.any()), // JSON Patch (RFC 6902)`, and after the top-level comma split that trailing comment heads the NEXT entry, so the field-name match failed and the entry was discarded silently — which is why the drift report read canonical as declaring `subagentRunId` on 23 events rather than 24, and flagged `STATE_DELTA.subagentRunId` as aimock-only. Comments are now stripped wherever they appear; no canonical schema literal contains `//`, so the unconditional strip is safe. This is drift-harness tooling, not runtime behavior — no published surface changes (#391) diff --git a/src/__tests__/ollama.test.ts b/src/__tests__/ollama.test.ts index e409825e..14787f16 100644 --- a/src/__tests__/ollama.test.ts +++ b/src/__tests__/ollama.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, afterEach } from "vitest"; import * as http from "node:http"; +import * as net from "node:net"; import type { Fixture, HandlerDefaults } from "../types.js"; import { createServer, type ServerInstance } from "../server.js"; import { ollamaToCompletionRequest, handleOllama, handleOllamaGenerate } from "../ollama.js"; @@ -106,6 +107,66 @@ function postRaw(url: string, raw: string): Promise<{ status: number; body: stri }); } +// Sends a raw request with `Upgrade: h2c` + `Connection: Upgrade` headers, as +// OkHttp (and thus langchain4j) speculatively does even for plain requests. +// http.request() can't produce this from the client side, so we use a raw +// socket and parse the response ourselves. +function requestWithH2cUpgradeHeaders( + url: string, + method: string, + body?: string, +): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const parsed = new URL(url); + const port = parsed.port ? Number(parsed.port) : 80; + const socket = net.connect(port, parsed.hostname, () => { + const bodyHeaders = body + ? `Content-Type: application/json\r\nContent-Length: ${Buffer.byteLength(body)}\r\n` + : ""; + socket.write( + `${method} ${parsed.pathname} HTTP/1.1\r\n` + + `Host: ${parsed.host}\r\n` + + bodyHeaders + + `Upgrade: h2c\r\n` + + `Connection: Upgrade\r\n` + + `\r\n` + + (body ?? ""), + ); + }); + let data = ""; + // The server keeps the connection alive (we don't send a real + // `Connection: close`), so completion must be detected from the response + // framing itself rather than waiting for the socket to close. + socket.on("data", (chunk: Buffer) => { + data += chunk.toString(); + const headerEnd = data.indexOf("\r\n\r\n"); + if (headerEnd === -1) return; + const head = data.slice(0, headerEnd); + const rest = data.slice(headerEnd + 4); + const isChunked = /transfer-encoding:\s*chunked/i.test(head); + const contentLengthMatch = head.match(/content-length:\s*(\d+)/i); + const done = isChunked + ? rest.endsWith("0\r\n\r\n") + : contentLengthMatch + ? Buffer.byteLength(rest) >= Number(contentLengthMatch[1]) + : false; + if (!done) return; + + const statusLine = head.split("\r\n")[0] ?? ""; + const status = Number(statusLine.split(" ")[1] ?? 0); + const responseBody = isChunked + ? rest + .split("\r\n") + .filter((line) => !/^[0-9a-f]+$/i.test(line.trim())) + .join("") + : rest.slice(0, Number(contentLengthMatch![1])); + socket.destroy(); + resolve({ status, body: responseBody }); + }); + socket.on("error", reject); + }); +} + function parseNDJSON(body: string): object[] { return body .split("\n") @@ -882,6 +943,83 @@ describe("GET /api/tags", () => { const names = body.models.map((m: { name: string }) => m.name); expect(names).toContain("gpt-4"); }); + + // Regression test: langchain4j's OkHttp-based client probes for HTTP/2 + // cleartext support by sending `Upgrade: h2c` + `Connection: Upgrade` on + // its plain GET /api/tags request. Node's http module treats any + // `Connection: Upgrade` header as a protocol-upgrade request, so this must + // not be swallowed by the WebSocket-upgrade path and 404. + it("responds normally when the request carries h2c upgrade probe headers", async () => { + instance = await createServer(allFixtures); + const res = await requestWithH2cUpgradeHeaders(`${instance.url}/api/tags`, "GET"); + + expect(res.status).toBe(200); + const body = JSON.parse(res.body); + const names = body.models.map((m: { name: string }) => m.name); + expect(names).toContain("llama3"); + }); +}); + +describe("POST /api/chat (h2c upgrade probe)", () => { + // Regression test: the same h2c probe headers on a POST with a body used to + // reach the request handler with an empty body (Node detaches its parser + // from `req` once "upgrade" fires, so bytes buffered in `head` never became + // part of `req`'s stream), producing a "Malformed JSON body" error instead + // of the real validation error for the request that was actually sent. + it("still parses the JSON body and returns the real validation error", async () => { + instance = await createServer(allFixtures); + const res = await requestWithH2cUpgradeHeaders( + `${instance.url}/api/chat`, + "POST", + JSON.stringify({ model: "llama3.2" }), + ); + + expect(res.status).toBe(400); + const body = JSON.parse(res.body); + expect(body.error.message).toMatch(/messages/i); + expect(body.error.message).not.toMatch(/Malformed JSON/i); + }); + + // The test above proves the body was PARSED. This one proves it arrived + // INTACT and was matched on: a validation error only needs enough of the + // body to know a field is missing, so it would still pass if the replay + // delivered a truncated body. Matching a fixture on `userMessage` cannot. + it("delivers the body intact — the request matches a fixture on its content", async () => { + instance = await createServer(allFixtures); + const res = await requestWithH2cUpgradeHeaders( + `${instance.url}/api/chat`, + "POST", + JSON.stringify({ + model: "llama3", + messages: [{ role: "user", content: "hello" }], + stream: false, + }), + ); + + expect(res.status).toBe(200); + expect(JSON.parse(res.body).message.content).toBe("Hi there!"); + }); + + // A body far larger than any single read, so the replay cannot pass by + // happening to fit in one buffer. Node >= 26 parses the body onto `req` and + // leaves `head` empty, so a replay that forwards only `head` stalls here + // forever rather than failing — which is why this asserts a real response. + it("delivers a body far larger than one read", async () => { + instance = await createServer(allFixtures); + const filler = "x".repeat(200_000); + const res = await requestWithH2cUpgradeHeaders( + `${instance.url}/api/chat`, + "POST", + JSON.stringify({ + model: "llama3", + messages: [{ role: "user", content: `hello ${filler}` }], + stream: false, + }), + ); + + expect(res.status).toBe(200); + expect(JSON.parse(res.body).message.content).toBe("Hi there!"); + }); }); // ─── Integration tests: journal ───────────────────────────────────────────── diff --git a/src/server.ts b/src/server.ts index ece104ac..9fb60c10 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3147,11 +3147,75 @@ export async function createServerWithResolvedAuth( }, ); + /** + * Body bytes the platform already parsed onto `req` for an upgrade request. + * + * Node >= 26 delivers an upgrade request's body here and leaves `head` + * empty; Node <= 24 leaves the body in `head` and ends `req` with nothing. + * The stream has already ended in both cases, so this resolves on the next + * tick either way — it never waits on the network. + */ + function drainParsedRequestBody(req: http.IncomingMessage): Promise { + return new Promise((resolve) => { + const chunks: Buffer[] = []; + let settled = false; + const done = (): void => { + if (settled) return; + settled = true; + resolve(Buffer.concat(chunks)); + }; + req.on("data", (c: Buffer) => chunks.push(c)); + req.on("end", done); + req.on("error", done); + req.on("close", done); + req.resume(); + }); + } + async function handleUpgradeRequest( req: http.IncomingMessage, socket: import("node:net").Socket, head: Buffer, ): Promise { + // `Upgrade: h2c` + `Connection: Upgrade` on plain requests — including + // ones with a body — to probe for HTTP/2 cleartext support. Only actual + // WebSocket upgrades belong on this path. + // + // Node fires "upgrade" instead of "request" for any `Connection: Upgrade`, + // whatever protocol is named, and the request never reaches the normal + // pipeline. Rather than reimplementing body parsing, rebuild the request + // line and headers with `Upgrade`/`Connection` dropped — the only thing + // that made this look like an upgrade — replay them onto the socket, and + // let Node re-parse the connection from scratch as an ordinary request. + // + // WHERE THE BODY LIVES DEPENDS ON THE NODE VERSION, and both cases must be + // replayed or the re-parsed request stalls forever waiting on a + // `Content-Length` worth of bytes that will never arrive: + // + // - Node <= 24 detaches the parser at "upgrade", so `req` yields nothing + // and any body bytes already read land in `head` (or, if the client + // sent them later, still arrive on the socket afterwards). + // - Node >= 26 parses the body onto `req` and leaves `head` EMPTY. The + // bytes are not on the socket either — draining `req` is the only way + // to get them back. + // + // Draining `req` is safe on both: on the older behaviour it ends + // immediately with zero bytes, so the concat below is a no-op there and + // `head` carries the body as before. + if ((req.headers.upgrade ?? "").toLowerCase() !== "websocket") { + const parsedBody = await drainParsedRequestBody(req); + const requestLine = `${req.method} ${req.url} HTTP/${req.httpVersion}\r\n`; + const headerLines: string[] = []; + for (let i = 0; i < req.rawHeaders.length; i += 2) { + if (/^(?:upgrade|connection)$/i.test(req.rawHeaders[i])) continue; + headerLines.push(`${req.rawHeaders[i]}: ${req.rawHeaders[i + 1]}`); + } + const rebuilt = Buffer.from(requestLine + headerLines.join("\r\n") + "\r\n\r\n"); + socket.unshift(Buffer.concat([rebuilt, parsedBody, head])); + server.emit("connection", socket); + return; + } + const parsedUrl = new URL(req.url ?? "/", `http://${req.headers.host ?? "localhost"}`); let pathname = parsedUrl.pathname;