feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, view. TeeSink. by Wahbeh-Mohammad · Pull Request #31 · dexpace/nodejs-sdk

Wahbeh-Mohammad · 2026-08-11T07:05:52Z

Implement product-spec §5 byte-streaming primitives in a new packages/core/src/io/,
per docs/superpowers/specs/2026-07-24-phase3a-io-contracts-design.md:

ByteQueue: synchronous FIFO source+sink over a chunk list (IO-7–IO-10), with
snapshot/copyTo non-consuming and an AllocationLimitError ceiling (IO-9).
RetentionWindow: pulled-byte window plus cursor set behind peek/slice, trimming
to min(live cursors). Deliberately uncapped — every cap the spec mandates sits in
§6 and belongs to Phase 3b.
BufferedSource/BufferedSink over Web Streams: typed reads, exact reads, line reads,
skip, charset decode/encode, flush vs emit, Web Streams bridges (IO-1–IO-24, IO-16).
TeeSink: mirror-before-forward decorator whose primary always receives the full
untruncated payload (IO-25–IO-29).
writeAll pump (IO-17) and the IO-30 factory half; provider resolution, IO-31–IO-36
and IO-39 are not built — no registry exists, same simplification as SEAM-5–SEAM-10.
IoError tree rooted at DexpaceError; argument errors go through a new invariant().

Nothing enters the public barrel. Every export is @internal behind src/io/index.ts,
and packages/core/etc/core.api.md is byte-identical, so Phase 3b can decide whether
BODY-1's write-to-sink names BufferedSink or the platform's WritableStream.

Teardown is close() only: Symbol.asyncDispose postdates the engines.node >=18.17
floor and TypeScript does not polyfill it for a declaring library. No AbortSignal
and no timer anywhere in src/io/ — IO-40 assigns deadlines to the transport that
owns the socket.

…, TeeSink.

…ection helper.

OmarAlJarrah

A second pass focused on §5 conformance and on the teardown paths, after the round of inline comments already on this PR. Five things below that aren't covered by those: two lifecycle defects, one comment that documents behaviour the platform doesn't have, and two tests that don't test what they claim.

The rest of the section reads as faithful. readUtf8Line's CRLF-versus-lone-\r handling matches IO-14 exactly (including the empty-line and bare-\n cases), IO-2's zero-count-before-exhaustion ordering is right in both ByteQueue.read and BufferedSource.read, IO-21's lazy slice-offset overflow surfaces as EOF on first read as required, and IO-22/IO-24 invalidation fails loudly rather than serving stale bytes. The deliberate non-implementation of the resolution half of IO-30 and of IO-31–IO-36 is recorded in the checklist and matches the SEAM-5–SEAM-10 decision, so that's consistent.

OmarAlJarrah

Deeper pass, this time running the branch rather than reading it. Summary first, because most of it is good news.

Everything structural checks out. All ten blocking gates pass on this branch — typecheck, lint, build, api:ci, lint:publish, verify:dual-consumption, verify:seam-1, verify:runtime-floor, audit, and the suite (140 pass, 95% lines). The PR body's claims hold up under checking: core.api.md really is byte-identical to 3-phase-3, there is genuinely no AbortSignal, setTimeout or AbortController anywhere in src/io/ (only prose mentioning their absence), every new file carries SPDX on line 1, and every test file cites its IO-N ids.

I probed the requirements that tests can't easily reach, and they hold. Closing a source while a read is parked mid-reader.read() surfaces ClosedResourceError rather than stale bytes or a hang — IO-38's JS analogue works, via the assertUsable() recheck after each await in pullThrough. IO-37's explicit allowance for concurrent use of independent views works: a parent and a peek read concurrently without cross-talk. IO-23's slice-of-slice composes additively and caps at the outer budget. IO-21's far-offset slice reads empty rather than throwing. IO-3 ordering is right — a negative count on a closed source raises the argument error, not the state error. Line reading survives chunk boundaries, including a UTF-8 sequence split mid-character, and EndOfStreamError reports delivered N of M.

I confirmed the Buffer.slice finding empirically — worth stating plainly because it is the most consequential thing on this PR. Buffer.prototype.slice aliases where Uint8Array.prototype.slice copies, so writeBytes(buf) and overBytes(buf) both leave the caller's buffer wired into the queue: mutate the input afterwards and the queue's contents change. IO-30's independence guarantee is broken for the single most likely input type in a Node SDK.

Three new items below.

…ecycle, and encoding symmetry (IO-1..IO-42).

… TeeSink. (#31) * feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, views, TeeSink. * test(core): close per-file coverage gaps in invariant, io/errors, rejection helper. * fix(core): resolve I/O contract review findings — copy semantics, lifecycle, and encoding symmetry (IO-1..IO-42).

…body lifecycle (#34) * feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, view. TeeSink. (#31) * feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, views, TeeSink. * test(core): close per-file coverage gaps in invariant, io/errors, rejection helper. * fix(core): resolve I/O contract review findings — copy semantics, lifecycle, and encoding symmetry (IO-1..IO-42). * feat(core): request/response body lifecycle. (#32) * feat(core): add request/response body lifecycle: Body model, materialize(), TypedResponse, HttpStatusError, logging tees (BODY-1..37, HTTP-36..52). * fix(core): resolve body-lifecycle review findings (BODY-3..37, HTTP-26/39/42/43/44/51). * test(core): cover withBodyWriter teardown paths (RECOV-12, RETRY-2). * fix(core): phase 3 review — sink ownership, close guarantees, HTTP-2 Nine defects found reviewing the shipped io/ and body/ layers against the phase 3a/3b plans. Seven are wire-correctness or public-API. Sink ownership, three sites and one root cause. A WritableStream adapter declaring write and close but no abort silently swallows the delegate's abort — the default abort algorithm is a no-op — so withRequestLogging left the caller's sink open, locked, and never told the message was broken. It now forwards both teardown paths, and closes on behalf of a delegate that resolves without doing so. StreamBody.writeTo passes preventCancel: true: pipeTo's default cancels the caller's source when the sink fails, taking ownership BODY-8 leaves with the caller, and disagreeing with the declared-length path, which only releases its reader. Close guarantees. Response.bytes/text and toHttpError acquire the body reader inside the try. getReader() itself throws when an external consumer holds the lock — which BODY-15 forbids assuming away — so the one failure BODY-16's guarantee most needs to cover was the one that skipped the close and held the connection. Declared length vs bytes written. MultipartBody.writeTo verifies its total against contentLength, refusing an overrunning chunk before it is written. The shared framing routine keeps the framing honest but takes each part's own length on trust, and MultipartPart.body is the public Body interface, so a caller implementation could report one length and write another (HTTP-51). Every Body variant is frozen at construction, so contentLength cannot be reassigned after the fact (HTTP-1, XCUT-15). Public API. Response regained the private constructor and createResponse friend hook a rewrite had dropped, which had published a field-wise constructor (HTTP-2). [Symbol.asyncDispose] is removed from Response and LoggedResponseBody: it postdates engines.node ">=18.17", where the computed key binds the method to the string "undefined", and its type reached this package only through a dev-only global — so the published .d.ts did not compile for a consumer on the lib this package itself declares. The API report is back to zero (undocumented) members, from 62. New blocking gate verify:consumer-types compiles a throwaway consumer against the built .d.ts on the declared lib with types: [], which is the gate whose absence let the asyncDispose defect clear every other one. Verified to fail on the reintroduced defect and pass once reverted. Also: assertCount single-sourced in io/limits.ts and applied to TeeSink, the fourth size-taking surface, which had none (IO-3); BODY-25's zero-chunk rule applied on the exceeds-cap tail path, not only the drain; an over-reporting primitive source raises SourceContractViolationError instead of surfacing as an exhausted stream (IO-17); http/charset.ts's decodeText renamed decodeBodyText so it stops colliding with io/text-codec's deliberately different one. * docs: expand phase 3 open findings, correct checkpoint status The phase 3b plan lists the 2026-07-25 checkpoint as a signed-off prerequisite. It has no commit and every box is unchecked — but §5.1 landed in bunfig.toml and half of §5.3 landed in errors.ts, which is exactly what made the claim look true to a spot check. Records the measured status of all twelve §5 items rather than the flat "it did not run". Grows the phase 3b execution findings from two rows to seven. E1 and E2 gain verified version numbers and measured blast radius; E3-E7 are new: §5.3 applied to 2 of 10 error leaves and stopped, §5.7 no isolated linker configured, §5.9 no test:node script exists although the 3b plan's own gate sequence calls it, §5.10 none of the eleven model files carries the #private justification, §5.8 stale NFR-14 reason. Resolves phase 4b's F1 to branch (b). Two of its premises were false: the floor was never raised, and SuppressedError arrived in Node 24.0.0 with the full Explicit Resource Management proposal rather than in the 18.18.0/20.4.0 symbols backport — so branch (a) means dropping Node 18, 20 and 22 outright. esnext.disposable supplies Symbol.asyncDispose's type but not SuppressedError's runtime, so E1's floor bump does not fix F1 and must not be read as doing so, including by 5a, 6b and 6c. Adds the phase 3b checklist, which was missing entirely, and records this phase's own residuals separately from the checkpoint's — among them the multipart boundary non-appearance limitation, which is documented rather than partially checked because a StreamBody part's bytes do not exist until the write. * chore: date-prefix changeset filenames `@changesets/write` names every changeset with a random `human-id` (`dry-candles-unite.md`), and there is no config knob for it — the ID comes from a hardcoded `humanId()` call, and `.changeset/config.json`'s schema has no filename field. The names were already being hand-corrected after the fact. Add `scripts/changeset.mjs`, wired as `bun run changeset`: it forwards every argument to the CLI, then renames whatever changeset the run produced to `YYYY-MM-DD-<slug>.md`, matching `docs/superpowers/{specs,plans}`. The slug is prompted for and defaults to the changeset's own first sentence; a non-TTY caller takes that default rather than hanging on a prompt nobody can answer. Subcommands that create nothing (`version`, `status`, `publish`, `tag`, `pre`, `init`) pass straight through. Renaming after the fact is safe because nothing reads the filename back: the CLI globs `.changeset/*.md` and takes every decision from the frontmatter. The seven existing changesets are backfilled with the date of the commit that added each one. No CI gate — a changeset written by hand or by another tool is not checked. * ci(test): add the Node-runtime conformance suite, close checkpoint §5.9 `bun test` runs the whole unit suite on Bun's runtime and proves nothing about the runtime this SDK ships to. Audited before writing anything: 319 of the 516 unit tests, across 21 of 43 files, exercise a runtime-divergent surface — Web Streams, AbortSignal, async iteration, or ByteQueue's Uint8Array handling — against exactly two assertions of Node coverage in scripts/verify-node-floor.mjs, neither of which touched io/. The ci job pinned no Node at all, so its three node-executed gates ran on an undeclared runner default, and node-floor-conformance pinned 18.17.0 alone, leaving current LTS unexercised against sdk-design-nodejs/09:52-54's "in addition to current LTS". Implements §5.9's own prescription rather than a substitute. bun test stays the unit runner untouched — docs/knowledge/testing.md mandates bun:test symbol imports, setSystemTime and --concurrent, so migrating to node:test would be a styleguide deviation plus a whole-suite rewrite — and is now scoped to packages/ via bunfig's [test] root so the two layers cannot blur. Without that scoping bun test collected the new .mjs files too, which would have run the Node-only layer on Bun and erased the distinction it exists to draw. Adds test/node-conformance/: 30 `node --test` cases over the BUILT artifact, never src/. Public surface arrives through the @dexpace/core specifier, the path a real consumer takes; io/ is @internal with no public subpath in exports, so it is reached by direct dist/ file path. Seeded with composeSignal, Phase 3a's byte-stream surface (chunk-straddling CRLF, slice views not advancing the parent, reader-lock release on close, tee mirror-and-forward, writeAll), and Phase 3b's body surface (reader-lock discipline on bytes/text/close, pipeTo preventCancel ownership, multipart framing through Web Crypto, toHttpError buffering). scripts/verify-node-floor.mjs is retired and its two AbortSignal.any assertions folded in as the suite's first cases, per §5.9:375's "rather than keeping two parallel Node entry points". The CI job is renamed node-conformance and is now a fail-fast:false matrix over ['18.17.0', 'lts/*']; lts/* resolves at run time so the LTS half cannot go stale as LTS moves. The 3b plan's Task 13 Step 3 called `bun run test:node` when no such script existed, so that gate sequence could not be executed as written; it is corrected, along with the two blocking gates it had never listed. NOTE: the CI job name changed. Branch protection requiring `node-floor-conformance` must be updated to `node-conformance`. * chore: add an empty changeset for the Node conformance suite `changeset --empty`, deliberately, rather than no changeset at all. Commit e3d0b18 touched zero files under packages/ — everything in it is repository infrastructure that ships to nobody — so there is nothing for @dexpace/core to bump, and a patch would put a changelog line in front of consumers that means nothing to them. The empty changeset is what distinguishes "this change needs no release" from "somebody forgot a changeset". `changeset status` is unchanged by it: still one minor for @dexpace/core, from the five existing non-empty changesets. Created through scripts/changeset.mjs so the filename follows the repo's YYYY-MM-DD-<slug> convention; --empty produces no summary to derive a slug from, so the wrapper fell back to a generic name and it was renamed using the wrapper's own toSlug logic once the summary was written. * fix(core): raise the Node floor to 20.3, close two floor defects The PR's node-conformance job failed on the pinned floor and passed on `lts/*`. Two unrelated defects, both invisible to `bun test` by construction. `MultipartBody` generates its boundary from `crypto.getRandomValues` — a bare global — while `engines.node` declared `">=18.17"`. Node exposes `globalThis.crypto` unflagged only from 19.0.0, and never to an ES module on any 18.x release: verified on 18.17.0 and 18.20.8, where `typeof globalThis.crypto` is `undefined` in `.mjs` and an object in CJS. So every `multipartBody(...)` call threw `ReferenceError: crypto is not defined` on the version the package promised, and a CommonJS probe would have reported that floor as satisfied. Bun supplies the global, which is why 516 unit tests never saw it and E5's suite caught it the first time it ran the built artifact on the pinned floor. The floor moves to `>=20.3`, chosen over the two options that keep Node 18. A `node:crypto` fallback puts a Node-only specifier in a package documented as running on browsers, Deno, Bun and Workers, and cannot be reached synchronously from the constructor that needs it. A non-crypto RNG silently downgrades the unguessable-boundary mitigation HTTP-51 leans on against multipart injection, on exactly the runtime CI pins. Node 18 went EOL in April 2025, so no supported runtime is dropped. 20.3 and not 20.0: `AbortSignal.any()` — `composeSignal`'s own floor-defining call, backported to 18.17.0 — reached the 20.x line only in 20.3.0, confirmed by running the suite against a pinned 20.0.0. `lib`/`target` move to ES2023 so `verify:runtime-floor` stays consistent; its `es2023` row is amended to `>=20.3` with the built-ins, not the syntax, named as the reason the floor sits above the language level's own minimum. The CI matrix pin moves 18.17.0 -> 20.3.0, and `seams.test.mjs` gains a case asserting `globalThis.crypto.getRandomValues` is a function *in ESM* — verified to fail on 18.17.0 and pass on 20.3.0 — so this cannot regress silently. Second defect: `seams.test.mjs` awaited an `AbortSignal.timeout()` abort with nothing else scheduled. That timer is unref'd on every Node version by design, so the loop drained before it fired and 18.17.0's runner cancelled the rest of the file (`Promise resolution is still pending but the event loop has already resolved`). Newer runners hold the loop open through handles of their own, which is the whole reason it passed on LTS. It now holds a ref'd deadline that both keeps the loop alive and fails the case if the abort never arrives. `sdk-design-nodejs/02`'s runtime line claimed Node >=18.17 supplies `globalThis.crypto.subtle`; corrected. Recorded as roadmap finding E8, which also renumbers E1: Symbol.dispose/asyncDispose reached the 20.x line in 20.4.0, so §5.4's bump now reads `>=20.3` -> `>=20.4`. The symbol is still declared nowhere. Gates: typecheck, lint, build, bun test (516), api, lint:publish, verify:dual-consumption, verify:consumer-types, verify:seam-1, verify:runtime-floor, audit, and test:node on both 20.3.0 and current Node.

feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, views…

6f847fb

…, TeeSink.

Wahbeh-Mohammad requested a review from OmarAlJarrah August 11, 2026 07:05

Wahbeh-Mohammad changed the title ~~feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, views…~~ feat(core): add I/O contracts — ByteQueue, BufferedSource/Sink, view. TeeSink. Aug 11, 2026

test(core): close per-file coverage gaps in invariant, io/errors, rej…

3f7978a

…ection helper.

Wahbeh-Mohammad self-assigned this Aug 11, 2026