Rebuild withNodeAdapter's response on a real Writable so Node middleware (Next.js, compression, send) runs against it

[kriszyp]

Request.withNodeAdapter() handed middleware an EventEmitter with hand-forwarded write/end/drain, but real Node middleware treats a ServerResponse as a Writable and relies on _header, _implicitHeader() and writeHead() ordering. Next.js therefore returned 500s, and larger responses hung once the obvious missing members were patched. The response is now NodeAdapterResponse extends PassThrough, and that exact object is the body the adapter resolves with, so backpressure and stream lifecycle events are Node's own.

Headers commit once through this.writeHead; disconnects and handler failures close or error the stream instead of leaving it open; and Headers.delete is case-insensitive, so compression can remove send's stale Content-Length. Post-review maintenance also makes setHeaders() accept Harper Headers, preserves an own __proto__ request header without changing the Node-compatible plain-object prototype, and forwards explicit and server-default timeout events with their socket argument. The public Promise<{ status, headers, body: PassThrough }> shape is unchanged.

Implementation and evidence are in NodeAdapterResponse.ts, Request.ts, Headers.ts, the Request contract tests, Headers regression test, and real-middleware/wire tests. The non-obvious contract is recorded in DESIGN.md; the real middleware fixtures are documented in dependencies.md and added through package.json plus its package-lock.json.

For the human reviewer

1. Express is an explicit non-goal. Express replaces the response prototype with one rooted at http.ServerResponse.prototype, which a Writable-derived response cannot survive. Next.js, compression, send, on-finished, on-headers, finalhandler, serve-static, h3 and fastify duck-type the response without replacing its prototype. Supporting Express requires a separate design.
2. Client hangup semantics are deliberate. After headers, a disconnect destroys the body without an error; before headers, the response promise rejects. This matches Node's dropped-connection behavior and lets pipeBodyToResponse treat the premature close as routine. One consequence to weigh: the rejection carries an AbortError, which has no statusCode, so a consumer that routes it into server/http.ts's onError gets a 500 written and harperLogger.error for a routine hangup. Giving the rejection a classified status is the one-line alternative.
3. The adapter owns an error listener. res.writeHead(200); res.destroy(err) emits before an awaiting caller can attach one. The error remains in the stream's state for pipeline(), finished() or async iteration, and the immediate-destroy regression proves the process does not crash.
4. A second writeHead() throws ERR_HTTP_HEADERS_SENT. This now matches Node, as do late setHeader, appendHeader and removeHeader calls; the contract test pins the change.
5. A handler failure after res.end() is logged at warn, sync or async. It cannot reach the client or the already-resolved promise, so both paths use the same operator signal.
6. Timeout forwarding follows listener ownership. The adapter attaches to the real response only while middleware has a timeout listener, so it does not accidentally mark an unhandled timeout as handled. Both setTimeout() forwarding and server-default forwarding without setTimeout() preserve the socket argument and detach on close.
7. An already-aborted request still invokes the handler against a destroyed response. Node can invoke a handler after its client is gone; writes then fail through normal stream semantics. Returning early is the one-line alternative if this compatibility choice changes.
8. Informational responses use the real Node response. writeContinue, writeProcessing and writeEarlyHints delegate when the transport provides a response and call back without hanging in the response-less fixture path.
9. The added packages are test-only real middleware. Next.js vendors compression 1.7.4, which gates on _header/_implicitHeader(), while 1.8 gates on headersSent/writeHead(). The matrix runs both; send was already a runtime dependency.
10. The in-repo wire proof is intentionally below a mounted Harper route. It uses a real http.createServer, Node request/response objects, compression + send, and Harper's pipeBodyToResponse. The out-of-tree @harperfast/nextjs CI matrix is the mounted consumer this unblocks.
11. strictContentLength is declared but not enforced. implements ServerResponse requires the property, and the adapter's default false matches Node exactly (verified on Node v26: a 10-byte Content-Length ended with 5 bytes throws ERR_HTTP_CONTENT_LENGTH_MISMATCH only when the flag is set to true). A middleware that opts in by setting it true would get no enforcement here. No supported middleware sets it; enforcing it means counting bytes per write().

Two earlier suggestions remain declined: adding statusMessage to the resolved public shape, and routing write()/end() through flushHeaders() instead of _implicitHeader().

Verification

• Fails on base: building origin/main's adapter and running the new middleware suite produced 16 failures / 3 passes: compression 1.7 lacked _implicitHeader, send/on-finished paths timed out, header-ordering assertions failed, and an immediate post-writeHead destroy escaped as an uncaught error.
• Current focused gates: npm ci, npm run build, npm run test:types, npm run lint:required, npm run format:check, and git diff --check pass. npx mocha unitTests/server/serverHelpers/Request.test.js unitTests/server/serverHelpers/Headers.test.js unitTests/server/serverHelpers/nodeAdapterMiddleware.test.js reports 126 passing.
• Windows gate: npm run test:unit:windows passes all 9 groups / 3,762 tests on the final head.
• Broader local gates (on the merge): npm run test:unit:main reports 5,582 passing / 197 pending with two failures, both environmental and both confirmed by re-running them alone: the known long-worktree domain-socket path case in configValidator, and stuckWorkerDiagnostics (a main test from 6d725818c, untouched here) which counts two diagnostic samples and loses the race when the box is loaded — it passes on its own, 5 passing.
• Review: the original design gate concluded chosen-approach-sound. Four further full cross-model passes ran on this head lineage (claude + gemini + Harper-domain adjudication each time), converging at round 11 with verdict LGTM and adjudicated severity minor. They produced two real fixes, both included here: a duplicate node_modules/@harperfast/rocksdb-js-linux-x64-musl key in package-lock.json (this branch and main had each restored the entry, and the merge kept both, leaving which metadata wins parser-dependent), and a functional bug where writeHead(302, undefined, { Location }) dropped the third argument's headers because the else branch assigned over them unconditionally — Node uses obj ??= reason, verified against a real http.createServer, and the regression test fails on the previous behaviour. Three reviewer claims were rejected on executed evidence rather than argument: setHeaders taking Headers | Map is Node's contract (Node v26 rejects a plain object with ERR_INVALID_ARG_TYPE); req.headers having Object.prototype matches Node, which also lets a Constructor: header shadow it; and strictContentLength is enforced by Node only when set to true, so the adapter's false default matches.
• CI (head 6bb377753): 45 checks pass, none fail. The branch was brought current with main by a merge commit rather than a rebase. That cleared four checks that had been red at the previous head and were all reproducing failures from the merge-base 3a3c6f1ff, each already fixed on main: the Docker smoke job's shrinkwrap pin comparison (fixed by #2560; the root pins here are identical to main's, and the same job failed on main at the merge-base and two later commits), and the eviction-secondary-index 500 plus the txnlog-restart-reclaim double-cleanup-pass case on shards 2 and 6 (both fixed by bf0d17d78). The Next.js adapter matrix passes on Node 20/22/24, every integration shard passes on Node 24, Bun, uWS and Windows, and the Windows unit gate passes.

Refs #2527

— Claude Fable 5.1

🤖 Generated with Claude Code

https://claude.ai/code/session_01BL9FvWh4XHTxvPgsdak2Ni

Complexity: complicated

_{Review-Coverage: authored=codex; ran=gemini,claude; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=11; full=4 @ 6bb3777}

_{Human-Review-Need: 4 (decisions: response-is-body, disconnect-settlement, timeout-relay, post-end-handler-failure, middleware-boundary, verification-boundary) @ 6bb3777}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	on-headers@​1.1.0
	compression@​1.7.4
	compression@​1.8.1

View full report

[gemini-code-assist]

Code Review

This pull request refactors the withNodeAdapter implementation by introducing a dedicated NodeAdapterResponse class that implements the Node.js ServerResponse contract. This improves robustness around client disconnects, backpressure, and handler failures. It also adds a case-insensitive delete method to the custom Headers class, adds devDependencies for testing with real Node middleware, and introduces comprehensive unit tests. The review feedback correctly identifies a TypeScript type mismatch in the setHeaders method of NodeAdapterResponse where passing the custom ResponseHeaders class would cause a compilation error, and provides a clear code suggestion to resolve it.

[claude]

Reviewed; no blockers found.

[dawsontoth]

I read through it, and things roughly make sense to me. I imagine this is something for fixing up the Next.js component, or people building more middleware on top of Harper? I had one question about a unit test completeness, but nothing necessary. The failing unit tests look unrelated to the changes made here, probably tackled in a different PR already.

[dawsontoth]

(Rebasing or merging the latest from main into this branch may tackle the merge conflict and the failing MQTT tests, though I haven't checked that to make sure.)