From c59745367d1fea3de8c4f9ff1fd56e902e449362 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Thu, 3 Sep 2026 10:42:40 +0200 Subject: [PATCH 1/4] fix(server): don't 404 non-websocket upgrade probes like h2c Node fires the "upgrade" event (not "request") for any request with a Connection: Upgrade header, regardless of the target protocol. OkHttp (used by langchain4j) speculatively sends Upgrade: h2c on plain requests to probe for HTTP/2 cleartext, which made GET /api/tags land in the WebSocket-only upgrade handler and get an unconditional 404. Only treat the request as a WebSocket upgrade when Upgrade: websocket is actually present; otherwise fall through to the normal HTTP pipeline so routes like /api/tags still work. --- src/__tests__/ollama.test.ts | 53 ++++++++++++++++++++++++++++++++++++ src/server.ts | 19 +++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/__tests__/ollama.test.ts b/src/__tests__/ollama.test.ts index e409825e..df4688b8 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,43 @@ function postRaw(url: string, raw: string): Promise<{ status: number; body: stri }); } +// Sends a raw GET 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 getWithH2cUpgradeHeaders(url: 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, () => { + socket.write( + `GET ${parsed.pathname} HTTP/1.1\r\n` + + `Host: ${parsed.host}\r\n` + + `Upgrade: h2c\r\n` + + `Connection: Upgrade\r\n` + + `\r\n`, + ); + }); + let data = ""; + socket.on("data", (chunk: Buffer) => { + data += chunk.toString(); + }); + socket.on("error", reject); + socket.on("close", () => { + const [head, ...rest] = data.split("\r\n\r\n"); + const statusLine = head.split("\r\n")[0] ?? ""; + const status = Number(statusLine.split(" ")[1] ?? 0); + // Body may be chunked; strip chunk-size lines for the assertions below. + const body = rest + .join("\r\n\r\n") + .split("\r\n") + .filter((line) => !/^[0-9a-f]+$/i.test(line.trim())) + .join(""); + resolve({ status, body }); + }); + }); +} + function parseNDJSON(body: string): object[] { return body .split("\n") @@ -882,6 +920,21 @@ 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 getWithH2cUpgradeHeaders(`${instance.url}/api/tags`); + + 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"); + }); }); // ─── Integration tests: journal ───────────────────────────────────────────── diff --git a/src/server.ts b/src/server.ts index 7a77f46f..57f27d11 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3145,6 +3145,25 @@ export async function createServerWithResolvedAuth( socket: import("node:net").Socket, head: Buffer, ): Promise { + // Node emits "upgrade" (never "request") for ANY request carrying a + // `Connection: Upgrade` header, regardless of what's being upgraded to. + // Some HTTP clients (e.g. OkHttp, used by langchain4j) speculatively send + // `Upgrade: h2c` + `Connection: Upgrade` on plain requests to probe for + // HTTP/2 cleartext support. Only actual WebSocket upgrades belong on this + // path — anything else must fall through to the normal HTTP pipeline so + // routes like `/api/tags` still work. + if ((req.headers.upgrade ?? "").toLowerCase() !== "websocket") { + if (head.length > 0) socket.unshift(head); + const res = new http.ServerResponse(req); + res.assignSocket(socket); + res.on("finish", () => { + res.detachSocket(socket); + socket.end(); + }); + await handleHttpRequest(req, res); + return; + } + const parsedUrl = new URL(req.url ?? "/", `http://${req.headers.host ?? "localhost"}`); let pathname = parsedUrl.pathname; From aeef742584d0662cc30154c072b812a791fc7b67 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Thu, 3 Sep 2026 10:46:35 +0200 Subject: [PATCH 2/4] fix(server): don't 404 non-websocket upgrade probes like h2c Node fires the "upgrade" event (not "request") for any request with a Connection: Upgrade header, regardless of the target protocol. OkHttp (used by langchain4j) speculatively sends Upgrade: h2c on plain requests to probe for HTTP/2 cleartext, which made GET /api/tags land in the WebSocket-only upgrade handler and get an unconditional 404. Only treat the request as a WebSocket upgrade when Upgrade: websocket is actually present; otherwise fall through to the normal HTTP pipeline so routes like /api/tags still work. --- src/__tests__/ollama.test.ts | 53 ++++++++++++++++++++++++++++++++++++ src/server.ts | 19 +++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/__tests__/ollama.test.ts b/src/__tests__/ollama.test.ts index e409825e..df4688b8 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,43 @@ function postRaw(url: string, raw: string): Promise<{ status: number; body: stri }); } +// Sends a raw GET 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 getWithH2cUpgradeHeaders(url: 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, () => { + socket.write( + `GET ${parsed.pathname} HTTP/1.1\r\n` + + `Host: ${parsed.host}\r\n` + + `Upgrade: h2c\r\n` + + `Connection: Upgrade\r\n` + + `\r\n`, + ); + }); + let data = ""; + socket.on("data", (chunk: Buffer) => { + data += chunk.toString(); + }); + socket.on("error", reject); + socket.on("close", () => { + const [head, ...rest] = data.split("\r\n\r\n"); + const statusLine = head.split("\r\n")[0] ?? ""; + const status = Number(statusLine.split(" ")[1] ?? 0); + // Body may be chunked; strip chunk-size lines for the assertions below. + const body = rest + .join("\r\n\r\n") + .split("\r\n") + .filter((line) => !/^[0-9a-f]+$/i.test(line.trim())) + .join(""); + resolve({ status, body }); + }); + }); +} + function parseNDJSON(body: string): object[] { return body .split("\n") @@ -882,6 +920,21 @@ 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 getWithH2cUpgradeHeaders(`${instance.url}/api/tags`); + + 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"); + }); }); // ─── Integration tests: journal ───────────────────────────────────────────── diff --git a/src/server.ts b/src/server.ts index 7a77f46f..57f27d11 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3145,6 +3145,25 @@ export async function createServerWithResolvedAuth( socket: import("node:net").Socket, head: Buffer, ): Promise { + // Node emits "upgrade" (never "request") for ANY request carrying a + // `Connection: Upgrade` header, regardless of what's being upgraded to. + // Some HTTP clients (e.g. OkHttp, used by langchain4j) speculatively send + // `Upgrade: h2c` + `Connection: Upgrade` on plain requests to probe for + // HTTP/2 cleartext support. Only actual WebSocket upgrades belong on this + // path — anything else must fall through to the normal HTTP pipeline so + // routes like `/api/tags` still work. + if ((req.headers.upgrade ?? "").toLowerCase() !== "websocket") { + if (head.length > 0) socket.unshift(head); + const res = new http.ServerResponse(req); + res.assignSocket(socket); + res.on("finish", () => { + res.detachSocket(socket); + socket.end(); + }); + await handleHttpRequest(req, res); + return; + } + const parsedUrl = new URL(req.url ?? "/", `http://${req.headers.host ?? "localhost"}`); let pathname = parsedUrl.pathname; From dc89fecc49f1f8044acc6578a580b6033bd6cbdd Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Thu, 3 Sep 2026 14:38:21 +0200 Subject: [PATCH 3/4] fix(server): stop dropping the body of h2c-probed POST requests The earlier h2c-probe fix (rebuilding a ServerResponse via assignSocket) only worked for bodyless requests like GET /api/tags. Node detaches its HTTP parser from the socket the moment "upgrade" fires, so any body bytes land in the `head` buffer instead of `req`'s stream; unshifting them back onto the socket never reconnects them to `req`, so readBody(req) resolved empty. A POST like /api/chat then failed JSON parsing ("Unexpected end of JSON input") instead of surfacing the real validation error for the request that was actually sent. Rebuild the request line and headers with Upgrade/Connection dropped, replay them plus `head` on the socket, and let Node re-parse the connection from scratch via server.emit("connection", socket). This reuses Node's own parser for the body (Content-Length or chunked) instead of hand-rolling body buffering. --- src/__tests__/ollama.test.ts | 76 ++++++++++++++++++++++++++++-------- src/server.ts | 34 ++++++++++------ 2 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/__tests__/ollama.test.ts b/src/__tests__/ollama.test.ts index df4688b8..48ef877f 100644 --- a/src/__tests__/ollama.test.ts +++ b/src/__tests__/ollama.test.ts @@ -107,40 +107,63 @@ function postRaw(url: string, raw: string): Promise<{ status: number; body: stri }); } -// Sends a raw GET with `Upgrade: h2c` + `Connection: Upgrade` headers, as +// 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 getWithH2cUpgradeHeaders(url: string): Promise<{ status: number; body: string }> { +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( - `GET ${parsed.pathname} HTTP/1.1\r\n` + + `${method} ${parsed.pathname} HTTP/1.1\r\n` + `Host: ${parsed.host}\r\n` + + bodyHeaders + `Upgrade: h2c\r\n` + `Connection: Upgrade\r\n` + - `\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(); - }); - socket.on("error", reject); - socket.on("close", () => { - const [head, ...rest] = data.split("\r\n\r\n"); + 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); - // Body may be chunked; strip chunk-size lines for the assertions below. - const body = rest - .join("\r\n\r\n") - .split("\r\n") - .filter((line) => !/^[0-9a-f]+$/i.test(line.trim())) - .join(""); - resolve({ status, body }); + 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); }); } @@ -928,7 +951,7 @@ describe("GET /api/tags", () => { // 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 getWithH2cUpgradeHeaders(`${instance.url}/api/tags`); + const res = await requestWithH2cUpgradeHeaders(`${instance.url}/api/tags`, "GET"); expect(res.status).toBe(200); const body = JSON.parse(res.body); @@ -937,6 +960,27 @@ describe("GET /api/tags", () => { }); }); +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); + }); +}); + // ─── Integration tests: journal ───────────────────────────────────────────── describe("POST /api/chat (journal)", () => { diff --git a/src/server.ts b/src/server.ts index 57f27d11..a2388bda 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3148,19 +3148,29 @@ export async function createServerWithResolvedAuth( // Node emits "upgrade" (never "request") for ANY request carrying a // `Connection: Upgrade` header, regardless of what's being upgraded to. // Some HTTP clients (e.g. OkHttp, used by langchain4j) speculatively send - // `Upgrade: h2c` + `Connection: Upgrade` on plain requests to probe for - // HTTP/2 cleartext support. Only actual WebSocket upgrades belong on this - // path — anything else must fall through to the normal HTTP pipeline so - // routes like `/api/tags` still work. + // `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 detaches its HTTP parser from the socket the moment "upgrade" + // fires, so `req` never receives a body: any bytes already read land in + // `head` instead, and reconnecting them to `req` (e.g. via `req.push`) + // would still leave chunked bodies and further framing unhandled. 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 plus `head` onto the socket, and + // let Node re-parse the connection from scratch as an ordinary request. if ((req.headers.upgrade ?? "").toLowerCase() !== "websocket") { - if (head.length > 0) socket.unshift(head); - const res = new http.ServerResponse(req); - res.assignSocket(socket); - res.on("finish", () => { - res.detachSocket(socket); - socket.end(); - }); - await handleHttpRequest(req, res); + 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(head); + socket.unshift(rebuilt); + server.emit("connection", socket); return; } From fe3aa01e84ae56a84fa9626c8fc60255ccef3b6b Mon Sep 17 00:00:00 2001 From: Jordan Ritter Date: Tue, 8 Sep 2026 17:15:53 -0700 Subject: [PATCH 4/4] fix(server): replay the upgrade-probe body from wherever Node parked it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on @dennisoelkers' fix (#410), whose diagnosis and approach are right: Node fires "upgrade" for any `Connection: Upgrade`, so langchain4j's h2c probe took an unconditional 404, and replaying the request with `Upgrade`/`Connection` stripped is the fix. The gap is WHERE THE BODY IS. That differs by Node version, and the original replay forwarded only `head`: - Node <= 24 detaches the parser at "upgrade": `req` yields nothing and the body lands in `head` (or arrives on the socket afterwards if sent later). - Node >= 26 parses the body onto `req` and leaves `head` EMPTY. The bytes are on neither the socket nor `head`. So on Node 26 a `head`-only replay left the re-parsed request waiting on a `Content-Length` worth of bytes that never arrived: no `data`, no `end`, no error — the connection hung open instead of answering. That is worse than the 404 it replaced, because a hang blocks a caller until its own timeout, and `engines` declares `>=20.15.0` so Node 26 is supported. Measured directly on a dependency-free reproduction: node 20/22/24 GET 200 | POST 200 | POST delayed 200 | POST 200KB 200 node 26 GET 200 | POST (NO RESPONSE) x3 Drain `req` and concat it into the replay. It is already ended in both cases, so this never waits on the network: on Node <= 24 it yields zero bytes and `head` carries the body exactly as before. All four probe shapes now pass on 20, 22, 24 and 26. Two tests added beside the original. The existing one asserts the body was PARSED — but a validation error only needs enough of the body to see a field is missing, so it would still pass on a TRUNCATED replay. The new ones assert the body arrived INTACT (matching a fixture on its content, which truncation cannot fake) and that a 200KB body survives, which no single read can carry. Mutation-tested: dropping `parsedBody` from the concat reds 3 of the 95 tests in the file; restored, 95 pass. Also adds Node 26 to the unit and pytest CI matrices. It sits inside the declared `engines` range and was untested, which is exactly why the body-loss case passed CI. typecheck (all three configs) exit 0; full suite 183 files / 5694 tests; eslint and prettier clean. --- .github/workflows/test-pytest.yml | 2 +- .github/workflows/test-unit.yml | 2 +- CHANGELOG.md | 2 ++ src/__tests__/ollama.test.ts | 41 ++++++++++++++++++++++ src/server.ts | 56 ++++++++++++++++++++++++++----- 5 files changed, 92 insertions(+), 11 deletions(-) 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 02e8aba4..3fd24558 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ ### Fixed +- **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 48ef877f..14787f16 100644 --- a/src/__tests__/ollama.test.ts +++ b/src/__tests__/ollama.test.ts @@ -979,6 +979,47 @@ describe("POST /api/chat (h2c upgrade probe)", () => { 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 97296f0d..d6e0afc3 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3140,6 +3140,31 @@ 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, @@ -3149,15 +3174,29 @@ export async function createServerWithResolvedAuth( // ones with a body — to probe for HTTP/2 cleartext support. Only actual // WebSocket upgrades belong on this path. // - // Node detaches its HTTP parser from the socket the moment "upgrade" - // fires, so `req` never receives a body: any bytes already read land in - // `head` instead, and reconnecting them to `req` (e.g. via `req.push`) - // would still leave chunked bodies and further framing unhandled. 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 plus `head` onto the socket, and + // 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) { @@ -3165,8 +3204,7 @@ export async function createServerWithResolvedAuth( headerLines.push(`${req.rawHeaders[i]}: ${req.rawHeaders[i + 1]}`); } const rebuilt = Buffer.from(requestLine + headerLines.join("\r\n") + "\r\n\r\n"); - socket.unshift(head); - socket.unshift(rebuilt); + socket.unshift(Buffer.concat([rebuilt, parsedBody, head])); server.emit("connection", socket); return; }