feat(core): request/response body lifecycle.

[Wahbeh-Mohammad]

Summary

Adds the request/response body lifecycle to @dexpace/core, satisfying BODY-1–BODY-37 and HTTP-36–HTTP-52 (minus the file-backed-body cluster, deferred to Phase 8).

Body model

• Body interface plus five implementations: ByteArrayBody, StringBody, FormUrlEncodedBody, StreamBody, MultipartBody (with MultipartBodyBuilder)
• materialize(body) — drains a single-use body into a replayable one, race-safe consumed-once guard
• Exact-length verification on StreamBody.writeTo (EndOfStreamError on a short stream)
• Multipart framing: shared length/bytes routine, RFC-2046 boundary validation, header quoting/escaping

Request/Response

• Request.body narrowed from unknown to Body | undefined
• Response.body: ReadableStream | null, plus text(), bytes(), close(), Symbol.asyncDispose
• TypedResponse — lazy parsed-response wrapper, parse-once memoized (success and failure), concurrent-first-call serialization, exposes status/headers/protocol/reason/request without touching the body

Logging tees (@internal, unwired until Phase 7)

• withRequestLogging — tee-on-write with a bounded, clearing tap
• withResponseLogging — drain-once wrapper with fits-cap and exceeds-cap regimes, cached drain-failure surfacing

Error handling

• HttpStatusError / toHttpError(response) — 1 MiB bounded error-body buffering, 4xx/5xx only, replayable re-served body, non-consuming preview
• New leaves ConsumedBodyError, MultipartBoundaryError (+ isBodyError)
• Retrofit: flattened io/errors.ts's leftover 3-tier shape (IoError children now extend DexpaceError directly), added isIoError

Public API

• Promotes Body, its variant types/factories, MultipartPart, materialize, TypedResponse, HttpStatusError/toHttpError, ConsumedBodyError/MultipartBoundaryError/isBodyError from packages/core/src/index.ts
• Concrete body classes exported as types only, never as constructible values
• core.api.md regenerated; changeset added (minor — pre-1.0 policy)

[OmarAlJarrah]

Reviewed §6 against the branch, running it rather than reading it. Structurally this is in good shape — all nine blocking gates pass (typecheck, lint, build, api:ci, lint:publish, verify:dual-consumption, verify:seam-1, verify:runtime-floor, audit) and 411 tests are green.

A lot of the hard parts are right. Multipart's declared contentLength matches the bytes actually written to the byte (222 = 222 on a two-part fixture), so HTTP-51's shared-framing-routine requirement is genuinely doing its job. BODY-3's consume-once guard is properly race-safe — setting #consumed before the first await is the correct shape for this runtime. toHttpError gets the whole HTTP-52/BODY-30 cluster right: the 1 MiB cap holds, it keeps draining past the cap to release the connection, buffering happens inside the close-guaranteeing scope, and the isError gate has a genuinely sharp comment about why code < 400 would be wrong for a 6xx. Form encoding is correct (q=a+b&plus=c%2Bd). HTTP-44's null-success and async-rejection memoization both work. And both StreamBody paths consistently leave the caller's stream open — I suspected pipeTo would cancel it and was wrong.

Two of the findings below are security defects with working proofs, and I'd treat them as blocking. The rest are correctness and polish.

One correction to my own probing, in fairness to the code: name and filename are properly defended. My first assertion there was over-strict — the injected text survives as inert characters inside the quoted string, but the CR/LF are stripped and the framing holds. The defense works; it just doesn't extend to mediaType.

[OmarAlJarrah]

Header injection: mediaType is interpolated raw while name and filename go through quoteParam. HTTP-51's framing defense has a hole.

Body.mediaType is a plain unvalidated string, and byteArrayBody(bytes, mediaType) / stringBody(text, mediaType) accept anything. Run on this branch:

const evil = byteArrayBody(new Uint8Array([120]), 'text/plain\r\nX-Injected: pwned');
multipartBody([{name: 'f', body: evil}], 'BOUNDARY');

--BOUNDARY\r\n
Content-Disposition: form-data; name="f"\r\n
Content-Type: text/plain\r\n
X-Injected: pwned\r\n        <-- attacker-controlled header
\r\n

It gets worse, because the value can close the header block outright:

byteArrayBody(data, 'text/plain\r\n\r\nSMUGGLED-BODY\r\n--BOUNDARY--')

Content-Type: text/plain\r\n
\r\n
SMUGGLED-BODY\r\n
--BOUNDARY--\r\n             <-- forged terminator; the real part body lands outside the multipart

That is a full break of the framing HTTP-51 exists to protect — arbitrary headers, arbitrary part content, and a forged closing boundary, all from a media type string. Any code path where a media type is derived from user input (an uploaded file's declared type is the obvious one) is exploitable.

The fix wants to be validation rather than the strip that quoteParam does, since a media type containing CR/LF is never legitimate: reject it at Body construction. MediaType already enforces exactly this under HTTP-26 ("reject a control character or non-ASCII byte ... using the same predicate as outbound header-value validation, so a media type is always header-safe"). Typing the field as MediaType rather than string, or validating through the same predicate in the body factories, closes it at the source and gets HTTP-26 for free.

Worth noting the contentLength invariant does not catch this — renderPartHeader is shared, so the forged bytes are counted and declared length still matches. The wire is consistently, silently wrong.

[OmarAlJarrah]

The loop writes everything the stream yields and only checks the count afterwards, so a stream longer than declared overruns Content-Length before the error is raised.

HTTP-39/BODY-10 says the copy "MUST write precisely the declared count." Verified on this branch — declared 3, stream yields 8:

declared 3, bytes reaching sink: 8  [1,2,3,4,5,6,7,8]  | threw: EndOfStreamError

All eight bytes are committed to the sink; EndOfStreamError arrives after. Once a transport has stamped Content-Length: 3, those extra five bytes sit on the socket immediately after the body, where the peer parses them as the start of the next message — the classic request-smuggling shape. A thrown error does not recall bytes already written.

The short-stream direction has a related problem: the finally runs writer.close() before the delivered !== declared check, so a truncated body is closed cleanly — signalled to the sink as complete — and only then reported as an error. writer.abort() is the signal that actually tells the transport the message is broken.

Suggest bounding the write inside the loop (if (delivered + value.length > declared) → write only the remainder and fail, or fail immediately on overrun) and aborting rather than closing on any length mismatch, so no mis-framed body ever reaches the wire.

[OmarAlJarrah]

finally { await writer.close() } discards the real failure. This pattern is in all five body implementations and it loses the cause every time.

When the sink rejects — the ordinary "connection died mid-upload" case — the finally calls close() on an already-errored writer, that rejects too, and a throwing finally replaces the in-flight exception. Verified against a sink whose write() throws SOCKET GONE:

body	surfaced error
StringBody	Cannot close a writable stream that is closed or errored
ByteArrayBody	same
MultipartBody	same
StreamBody	same

The real cause is gone in every case — not chained, not suppressed, just replaced.

This is worse than a bad error message, because retry classification reads the cause chain. RETRY-2 defines the retryable set as "any throwable that is, or has anywhere in its cause chain, an I/O error or a timeout error." A genuine network failure that arrives as a TypeError about closing a stream will not match, so Phase 5a's retry stack will decline to retry a failure that is squarely retryable — and it will do so silently, which is the hardest kind of bug to find later.

The shape that preserves it:

let failure: unknown;
try { /* write */ } catch (e) { failure = e; throw e; }
finally {
  try { await writer.close(); }
  catch (closeError) { if (failure === undefined) throw closeError; }
}

Same fix at simple-bodies.ts:75, stream-body.ts:63, and multipart-body.ts:147. RECOV-12 spells out the general rule — a close error must never mask the primary, and should ride along as a suppressed/secondary error where the language supports it.

[OmarAlJarrah]

A parser that throws synchronously is never memoized, so the handler re-runs and re-reads the single-use body.

??= assigns the result of the right-hand side — if #parse throws before returning a promise, the assignment never happens and #memoized stays undefined. Verified: a parser that throws synchronously is invoked 3 times across 3 value() calls. HTTP-44 requires the outcome be memoized "without re-running the handler or re-reading the single-use body," and is explicit that both a null success and a thrown failure are covered.

This is easy to dismiss because the signature says => Promise<T>, but a non-async function returning a promise is perfectly ordinary, and so is validating an argument before the first await. The second call then re-reads a body whose bytes are already gone, so the failure mode is a confusing second error rather than the real one.

Wrapping the call so a synchronous throw becomes a rejected promise fixes it and costs nothing:

value(): Promise<T> {
  this.#memoized ??= (async () => this.#parse(this.#response))();
  return this.#memoized;
}

The async-rejection and null-success paths both memoize correctly today — memoizing the promise rather than the value is the right call and neatly sidesteps HTTP-44's null-success clause.

[OmarAlJarrah]

preview() hardcodes UTF-8 while the media type it needs is sitting in #mediaType.

Verified: a 500 whose body is café in ISO-8859-1, with content-type: text/plain; charset=iso-8859-1 stored on the error, previews as caf\uFFFD. body() uses #mediaType; preview() ignores it.

HTTP-42 sets the rule for exactly this — text reads default to the charset declared in the media type, falling back to UTF-8 when absent or unknown. Reusing that resolution here would make the preview correct and keep one charset rule in the codebase rather than two.

Separately, new TextDecoder(charset) throws RangeError on an unrecognised label, so preview('bogus') throws out of a @public method on an error object — the one place a caller is least able to handle another exception. HTTP-42's "falling back to UTF-8 when ... the declared charset is unknown" is the behaviour to copy: resolve, fall back, never throw.

[OmarAlJarrah]

Non-string values are silently dropped from the form body.

Neither branch matches a number/boolean/Date, so the entry vanishes with no error. Verified: formUrlEncodedBody({count: 5, name: 'x'}) produces name=x — the count field is gone.

TypeScript callers are mostly protected by FormUrlEncodedInput, but this is a @public factory in a published package and types evaporate at runtime; a JavaScript consumer passing {page: 1} gets a silently incomplete request body and a puzzling server-side error. Same for the array branch at :109, which drops non-string elements one at a time.

Either coerce (String(value), which is what most form encoders do and what a caller passing a number expects) or reject with a named error. Silently discarding caller data is the one option that leaves no way to diagnose it — and this codebase is otherwise consistent about that, e.g. HTTP-4's "never silently substituting defaults".

[OmarAlJarrah]

No @throws anywhere in the new public surface, which breaks the convention Phase 1 established.

Counting across the branch: 22 @throws tags across 9 http/ modules, 0 across all 11 body/ modules. CLAUDE.md is explicit — "Anything the barrel exports needs a TSDoc block with @public, plus @throws naming each catchable error class on operations that throw."

Several newly-promoted symbols throw:

• streamBody(...) / StreamBody.writeTo → ConsumedBodyError, EndOfStreamError
• multipartBody(parts, boundary) → MultipartBoundaryError
• materialize(body) → whatever the wrapped writeTo raises
• HttpStatusError.preview(charset) → RangeError (see the separate note)

media-type.ts is the model to follow — it names the error type and enumerates the conditions, which is what makes the tag worth having rather than ceremony.

This one is cheap to fix and worth doing before the surface ships, since api-extractor has already baked these signatures into core.api.md and consumers will start writing catch blocks against them.

[OmarAlJarrah]

Second pass, focused on the two logging tees, Response, and — mostly — on whether the tests actually verify the requirements they name. Line coverage is 95%+, so I ran a mutation campaign instead: break a requirement in the source, run the suite, see if anything goes red.

Eleven MUST-level requirements have tests that genuinely catch a break. Deleting the CR/LF strip in quoteParam, skipping tap.clear(), mis-reporting contentLength, ignoring the media-type charset, raising the 1 MiB error cap to 1 GiB, treating every status as an error, reporting a multipart replayable when a part is not, copying past the tap's remaining room, dropping the staged overflow chunk, making the fits-regime non-repeatable, and returning a non-replayable body from materialize — every one of those goes red. That is a genuinely well-tested phase, and the mutation results are the evidence rather than the coverage number.

Two survived, both the same shape, detailed below. Everything else here is smaller.

Two corrections to my own hypotheses, since I would rather report the check than the guess:

• I expected a BODY-28 violation — a cancel() failure on the fits path becoming a cached drain error and permanently blocking the captured body. It does not happen: by the time closeDelegate runs on that path the stream is already closed, so cancel() is a spec no-op and never reaches the failing algorithm. The captured body serves fine. No finding.
• My first BODY-19 mutant (bypassing the tap.size < cap guard) survived, but it is an equivalent mutant — the room arithmetic below it already clamps, so the guard is redundant rather than untested. Mutating the clamp itself is caught by 2 tests. The tap cap is well covered.

BODY-24's exceeds-cap path also checks out end to end: with cap=2 over chunks [1,2,3,4]/[5,6] the consumer receives all six bytes via prefix + staged tail + live remainder, the preview stays at [1,2], and a second read() throws ConsumedBodyError.

[OmarAlJarrah]

This test has no assertions, and the requirement it names survives having its guard deleted.

The body calls logged.close() twice and ends. There is no expect(), so the only thing it can detect is a thrown exception. Deleting if (state.closed) return; from closeDelegate leaves the entire suite green — verified by mutation.

It passes because the failure mode is not a throw. Cancelling an already-cancelled ReadableStream resolves quietly per the Streams spec, so a wrapper that cancels its delegate two, five, or fifty times looks exactly like one that cancels it once. But BODY-27's requirement is specifically counting: "MUST close the delegate at most once across all close paths ... because some transport streams throw on double-close." The whole point is the transports that are less forgiving than a spec-compliant ReadableStream — which is exactly what a spec-compliant ReadableStream cannot demonstrate.

What would catch it is a delegate that counts cancel() invocations, then asserting the count is 1 after exercising both close paths — the wrapper's own close() and the tail stream's completion, since BODY-27 requires they share one guard. Worth covering both, because the shared-guard half is the part a refactor is most likely to break.

response.test.ts already uses a cancelled flag a few tests along; this just needs the same treatment with a counter.

[OmarAlJarrah]

Same shape as the BODY-27 test, same result: no assertions, and deleting the guard keeps the suite green.

Two await response.close() calls, no expect(). Removing if (this.#closed) return; from Response.close() is caught by nothing.

BODY-15 and HTTP-43 both frame this as a counting property — close "MUST be idempotent" and the underlying resource is released at most once — and idempotence observable only as absence of a throw is not really being tested. Since cancel() on an already-cancelled stream resolves, the guard could vanish in a refactor and nothing would notice until a transport whose cancel is not re-entrant shows up in Phase 8.

The fix is small and the file already knows how: releases the connection even when the body was never read, a few lines below, builds a stream with a cancelled flag. Reuse that shape with a counter and assert it lands on 1 after two closes.

While you are in here — Response.close() sets #closed = true before awaiting cancel(). If cancel() rejects with anything that is not a TypeError, the response is marked closed, the error propagates, and a caller who retries gets a silent early return on a connection that was never released. Same pattern flagged on BufferedSink.close() in #31, so it is worth settling once and applying to both.

[OmarAlJarrah]

This module and RetentionWindow now take opposite positions on the same question, each documented as deliberate.

Here a zero-length chunk is "a legal no-op, not an EOF signal" and the loop continues. In io/retention-window.ts:143 the same input raises SourceContractViolationError, citing IO-17's "never tolerated as EOF and not spun on". Verified: a stream yielding [] then [7] drains cleanly to [7] through withResponseLogging, while the equivalent through BufferedSource throws.

Both readings are defensible, and your reasoning here is sound — ReadableStreamDefaultReader.read() genuinely has no requested-count for "zero bytes for a positive requested count" to apply to. The problem is having both in one package, because the two layers meet: a response body flows through withResponseLogging in Phase 7 and through BufferedSource wherever the body layer reaches for it, so the same upstream would fail in one path and succeed in the other depending on which wrapper it passed through.

Worth picking one and having the other cite it. The tolerant reading looks more defensible for byte streams, which would mean relaxing the io/ side — with the caveat that IO-17's spin risk is real if a source only ever yields empty chunks, so the tolerant version probably wants a bound on consecutive empties rather than an unconditional continue.

[OmarAlJarrah]

snapshot() never triggers a drain, so calling it before the first read() returns empty rather than the captured body. Verified: [] on a wrapper over a three-byte stream.

BODY-22 names the trigger set explicitly — the drain happens "lazily, on the first access (read/snapshot/exception query)" — so on a literal reading snapshot() should start it. BODY-26 then carves out the exception query ("surfaces the cached error ... without triggering a drain"), and error() correctly implements that, with a comment. snapshot() has no such carve-out in the spec and no note here.

I do not think the current behaviour is obviously wrong — for a logging tee, snapshotting before anything has been read is arguably a caller error, and draining from a preview accessor has its own surprise factor. But it is a deliberate divergence from a MUST's stated trigger list, and this project's discipline is that those get recorded rather than absorbed. Either drain from snapshot() to match BODY-22, or add a line saying why it deliberately does not, the way error() does.

[OmarAlJarrah]

materialize() returns a new wrapper sharing the same tap buffer as the original, because wrap closes over tap from the factory scope rather than allocating a fresh one.

Verified: after const mat = await logged.materialize() and writing mat, both mat.snapshot() and logged.snapshot() return the same bytes. Two live wrappers, one tap.

BODY-21 asks that materialize "return a wrapper around the delegate's replayable form (preserving the tap cap)" — the cap, not the buffer. In practice the original is consumed by the materialization so it rarely matters, but Phase 7 is where it would bite: a retry loop holding the pre-materialization wrapper for its first-attempt preview finds those bytes silently rewritten by the second attempt, because tap.clear() at the start of every write (BODY-18) is clearing a buffer two objects believe they own. Confusing to debug, easy to prevent.

Giving wrap its own ByteQueue per invocation, with cap still captured from the factory, keeps BODY-21 and drops the aliasing.