Audit #67 remediation: milestones 1–5 (#68–#82) - #98
Merged
Conversation
`DecodeTarget.admitsNull` (`packages/core/src/seams/serde.ts:126-145`) is the caller's opt-in that `T` includes `null`, and `jsonSerde()` honours it at `packages/codec-json/src/json-serde.ts:170`. Two `@public` TSDoc blocks still said the rejection "cannot be conditional" and that `tristate(inner)` is a field combinator only -- the text that predates the flag, shipped in both packages' `.d.ts`. Both now describe the default and the opt-in. Pinned already by `json-serde.test.ts:581,591`; no behaviour change. Part of audit #67 / #68 (SERDE-5, SERDE-13).
Each was true when written and is not now: - `context/instrumentation.ts` `tracerFactory` said it returns "the started span". It returns a **tracer**; both consumers narrow it and call `startSpan()` themselves (`pipeline/runtime.ts:52-57`, `observability/logging-step.ts:263-269`). - `io/index.ts` said `isIoError`, `AllocationLimitError`, `ClosedResourceError` and `SourceContractViolationError` "remain internal". `1f48926` put all four on the barrel (`core/src/index.ts:39-47`) and in `core.api.md`. - `http/request-conditions.ts` said reconciling HTTP-48's obs-text permission against HTTP-18 is "left to a later phase". Item 15 of `sdk-design-nodejs/10` decided it: the strict outbound path stays, no relaxed emit path. - `body/request-body-logging.ts` ended mid-sentence before its `@internal` tag, which has been true since `e3ba885`. Completed with what `snapshot()` actually guarantees (`io/byte-queue.ts:96`). - `retry/retry-step.ts` said `RequestOptionsBuilder.maxRetries` "rejects only a negative value ... it admits Infinity, NaN, and fractions". `http/request-options.ts:212-219` rejects all three, pinned by that file's `maxRetries validation (HTTP-35)` block. The `invariant` stays, and now says why. - `pagination/strategies.ts` said the template "never changes across the walk". `pageNumberStrategy` returns the next one and `paginator.ts:213` installs it; `strategy.ts:10-15` already had it right. Doc-only; no signature moved, so no `etc/*.api.md` changed. Part of audit #67 / #68 (CTX-14, HTTP-18, HTTP-35, BODY-19, PAGE-17, TRANSPORT-20).
`buildRequest` assembles through `Request.Builder.build()`, so a descriptor pairing a body with GET/HEAD/TRACE/CONNECT throws `RequestBodyNotAllowedError` (`http/request.ts:221`). Its `@throws` list named only `OperationAssemblyError` and `UrlConstructionError`, so the class was unreachable from the shipped `.d.ts` for a caller trying to catch it. No test pinned the path either. Added one; confirmed it fails when the assertion is inverted to `.not.toThrow()`. Part of audit #67 / #68 (HTTP-7, SEAM-26).
…er drives The file header listed `TRANSPORT-20..27` and `registerFailureRows`'s describe was named `TRANSPORT-4/5/6/20/22`, but no row in it forces an adaptation throw -- its four rows are a dead port and three timeout cases. Forcing one needs a hook into the native response, so each adapter asserts it against its own (`transport-fetch/src/fetch-transport.test.ts:118`, `transport-undici/src/undici-transport.test.ts:503`). Both header and describe now say so. Comment-only; the suite runs the same rows. Part of audit #67 / #68 (TRANSPORT-22).
Every `file:line` in the sections this audit touches now points at the text it cites, verified line by line against the worktree: - **Item 5** quoted `deserialize<T>(data, schema, typeName?)` at `serde.ts:145` and the `Serde` interface at `:182`. The signature took its witness in a `DecodeTarget` on 2026-09-04; it is `:194`, `Serde` is `:241`, `deserializeFrom` is `:221`, and `DecodeTarget` is `:122-124`. The codec anchors move from `json-serde.ts:236,242` to `:244,250`. The deviation is unchanged -- a schema value still stands in for a reflected type token. - **Item 17**'s hierarchy lines were four to six off (`29,49,65,80` -> `29,51,67,83`; `isIoError` `102` -> `108`; `TransportFailureError` `126` -> `132`). Its "cause-walk returns retryable for any `IoError`" reads as covering all five I/O classes and does not: `classify.ts:73` tests `instanceof IoError`, which the flat leaves fail. Recorded as an anchor correction with the rule left to #78, which decides `instanceof` vs. `isIoError` and rewrites the rationale. - **The OBS-29 row** said "the 1:1 binding is NOT met" and cited only the per-attempt span in `logging-step.ts`. `pipeline/runtime.ts:33-48,154-157,171-175` opens one span per logical operation outside every pillar, and `send`'s own `@remarks` states the binding. Marked IN PROGRESS for #80, which owns what a caller can reach. - **Items 3 and 15** are re-anchored because this branch's own source edits shifted them (`retry-step.ts:137` -> `:142`, `request-conditions.ts:129-142` -> `:133-146`). Two counts deleted rather than corrected, per CLAUDE.md: the section intro's "Four rows as of 2026-09-02" above a five-row table, and the `invariant()` cell's "fifteen" for a `pipeline/` that ships three. The cell now carries the command to re-derive them instead. Part of audit #67 / #68 (SERDE-5, SERDE-13, OBS-29, TRANSPORT-20, HTTP-18).
- `write-a-response-handler.md` said `decodeSuccessResponse` delegates to `toHttpError` "otherwise", and that `HttpStatusError` carries "the status, the headers and a bounded body preview". `serde/response-handlers.ts:189-201` routes 4xx/5xx there and closes-then-raises a `DeserializationError` for every other non-2xx; `body/http-status-error.ts` has `status`, `body()` and `preview()` and no headers accessor. - `write-a-paging-strategy.md` twice described `template` as "the request the walk started from". It is the request that fetched this page and it advances (`pagination/paginator.ts:165,213`, `strategy.ts:10-15`) -- the same wrong reading `strategies.ts` carried. - `auth.md` named the source of `perCall` and `client` but not `operation`. It is `RequestOptions.operationAuth` (`auth/auth-step.ts:252,756`, `auth/resolve.ts:19`), which has had a source only since `1f48926`. `probe.mjs` and `check-fences.mjs` both pass. The credential-shape example in `auth.md` is left for #71. Part of audit #67 / #68 (SERDE-28, PAGE-5, PAGE-17, AUTH-4).
`noopInstrumentationBundle.activeSpan` was `undefined`. CTX-15 asks for "a no-op span" beside the no-op tracer factory, and `createInstrumentationBundle` has always used `NOOP_SPAN` for the enabled bundle, so the two constructors of one interface disagreed. Phase 4a shipped the gap deliberately and ledgered it as partial: no `Span` type existed then. `Span`/`NOOP_SPAN` landed in Phase 7b, so the reason expired. The one-line value change needs a module split to land. `observability/ tracing.ts` imports `InstrumentationBundle` for `createInstrumentationBundle`'s return type, so importing `NOOP_SPAN` back into `context/instrumentation.ts` closes a cycle -- and `verify:import-cycles` counts type-only edges on purpose. Its failure message prescribes the fix taken here: the inert declarations (`SpanContext`, `Span`, `Tracer`, `NOOP_SPAN`, `NOOP_TRACER`) move to a leaf module `observability/span.ts` that imports nothing, and `tracing.ts` re-exports all five. No import path and no line of `etc/core.api.md` changes; `bun run api` is unchanged. `span.ts`'s module header is line comments that never write the internal-marker JSDoc tag out. `stripInternal` (on, from gts) tests it by substring-scanning every leading comment range of a declaration, line comments included, so a header merely mentioning the tag deleted `SpanContext` from the emitted `.d.ts` with no `tsc` diagnostic -- the build failed one package later on an unresolved name inside core's own `dist/`. Decided as D3 of the audit #67 decision ledger: fix, not a deviations.md row.
…nce reading (#69) Two places where the port reads a MUST differently from the spec's literal text, kept deliberately and now held by a test rather than by nobody. Both readings land as rows in docs/deviations.md in the next commit. REDIR-3 says eligibility is measured against the ORIGINAL request method; the port measures it against the CURRENT hop's (`decide.ts:241` into `codes.ts:69`). The readings diverge on exactly one chain: an opted-in 303 rewrites POST to GET, and a following 301 is then eligible under the default {GET, HEAD} set. `decide.test.ts` now drives that chain end to end. Mutation- checked: forcing `isEligibleByCode` back to the seed method turns the new case red with `return-current`. PAGE-19's conformance note offers `<not a url>; rel=next` as an example of "stream ends, no exception". Under WHATWG URL -- the only RFC 3986 resolver available without a runtime dependency (SEAM-1) -- a supplied base makes that a valid relative path reference, so the port follows it to `/repo/not%20a%20url`. The requirement's normative sentence is about a target that CANNOT resolve, and that half was already pinned; this adds the half the fixture disagrees with. Mutation-checked the same way. Both files' `Exercises:` headers name the new readings and point at docs/deviations.md.
#69) `deviations.md`'s scope paragraph said its collection section was empty and that the 2026-08-31 sweep found no unrecorded deviation. Both were true of that sweep and neither is true now: that sweep read the registers, and the 2026-09-04 code audit (#67) read the shipped code. It found MUST-level narrowings and undecided readings that lived only in a phase design's ledger, only in a test comment, or nowhere. The paragraph is rewritten to say so, and deliberately states no count -- a count in a collection point is wrong on the next append. Nine rows appended, each with verified `file:line` evidence: - PIPE-37 -- the outermost pre-redirect status-mapping step was never built. `statusMappingStep` is a `ResponseStep`, not a staged pipeline `Step`; Phase 4's checklist handed the wiring to Phase 5 and Phase 5 shipped without it. Ledgered, not implemented: it is public pipeline surface, and the petstore spike's finding 2 wants the same work from the other side. (D4) - REDIR-3 -- eligibility reads the current hop's method, not the original's. (D5) - PAGE-19 -- the spec's own `<not a url>` fixture resolves as a relative reference here and is followed; the normative clause is satisfied. (D6) - HTTP-46 -- `Request.equals` compares the body by identity: reading a single-use body to compare it would make equality destructive. - IO-13 -- write-side encodings are UTF-8 and ISO-8859-1 only. - BODY-9 -- `StreamBody` is always single-use; the SHOULD's condition is unmeetable on `ReadableStream`. - BODY-34 -- the shared preview cap covers the two logging tees, not `toHttpError`, whose cap HTTP-52 fixes. - IO-38 -- cross-thread close visibility has no subject on this platform; recorded as not applicable rather than as satisfied. - transport `reasonPhrase` -- fetch sets it, undici has no value to read. Sits beside item 13's `Protocol.HTTP_1_1` gap, which does not name it. (D7 for the last six.) Also re-anchors the OBS-29 row's `Tracer` citation, which the previous commit moved from `observability/tracing.ts` to `observability/span.ts`. Phase records under docs/work/ keep their pre-move paths, as that tree is never retro-edited.
…pInternal trap
Completes the file against this task's own criterion — every `file:line` points at the text it cites. Each replacement was pinned with `sed -n` before it was written, and the whole file re-swept afterwards. | Item | Was | Now | |---|---|---| | 1 | `context.ts:104,125,149` — `:104` unrelated prose, `:125` a signature close | `:112` is the `Symbol()` mint; `:128`/`:149` are the two per-flavor key defaults | | 3 | `engine.ts:354`, inside `runWithRetry`'s TSDoc | `:358`, the declaration | | 4 | `configuration.ts:72`, which is `.emit();` | `:100`, `export interface Configuration` | | 8 | `"sideEffects": false` at `:20`/`:21`/`:21` — all three a bare `}` | `packages/core/package.json:25`, both transports at `:26` | | 12 | `auth-step.ts:387-390`, the HTTPS-guard paragraph | the marker read at `:395` and the two branches it gates, `:401` and `:786`, plus `OutboundPlan.crossOrigin` at `:375` for why the answer survives the dispatch | Item 4's "**61 interfaces** against 58 classes" is deleted rather than corrected, per CLAUDE.md — the real figures had drifted to 65 and 71, and the sentence never needed a number to make its point. Part of audit #67 / #68, round 2.
…ts two unknowns `packages/core/src/context/instrumentation.ts:8-13` justified `activeSpan` and `tracerFactory` being typed `unknown` with "nothing in the package consumes either yet, pending Phase 7a". Phase 7a landed in `bd37a08` and shipped `Span` and `Tracer` in `observability/tracing.ts`; it did not narrow these two. Measured rather than assumed, and the two members differ: - `tracerFactory` IS consumed — `pipeline/runtime.ts:52` and `observability/logging-step.ts:263`, both reaching a `Tracer` through a cast. - `activeSpan` is filled by `createInstrumentationBundle` (`observability/tracing.ts:222`) and read back by nothing in the package, so half the original sentence was true and half was not. The note now states the real reason they stay `unknown`: narrowing a published member widens what a caller may pass and narrows what they receive, which is a version-bump decision. `tracerFactory`'s own PROVISIONAL line stops citing a phase that has shipped and defers to that note. The `activeSpan` member TSDoc and `noopInstrumentationBundle` are deliberately untouched — #69 owns both. Part of audit #67 / #68, round 2 (CTX-14, CTX-15, OBS-29).
…mediation-67 # Conflicts: # docs/deviations.md
`SchemeDowngradeError` and `NonReplayableBodyError` interpolated their raw `from`/`to` URLs into their messages. Every other redirect event sends its URLs through `redactUrl()`, but `http.redirect.rejected` hands the decision error to `LogEvent.cause()`, which renders it as `name: message` (`observability/logger.ts:74`) -- so userinfo and query-string tokens reached the log record in clear text, past a field cap of 8192 that truncated nothing. Redacted at construction rather than at the log site (ledger D8): the message is what every logger, `cause` chain and consumer `console.error` renders, and this SDK owns none of those. `redactUrl` is total under OBS-15, so a constructor can call it unguarded; an input that never parsed yields `[malformed url]`, which is the safe reading of "unparseable" rather than "carries no secret". `targetUrl` / `fromUrl` / `toUrl` stay RAW -- program code reads the property, log and human output read the message. That split is now stated in each class's TSDoc. OBS-11, OBS-12, OBS-15, REDIR-28, XCUT-19(a)/(b).
The rejection record named no URL at all, so the only hop a reader could recover from it was the raw one interpolated into the cause's message. It now carries `url.full` through `redactUrl()` -- the same field name and the same policy `http.redirect.hop`, `loopDetected` and `downgradePermitted` already use -- on both the `fail` and the `return-current` branch. Defence in depth over the message fix, not a replacement for it: this covers a future error class on this path that forgets to redact, while the message fix covers the `cause` chain this step cannot reach. REDIR-28, OBS-11, OBS-12, XCUT-19.
One XCUT-19 row over the real two-origin fixture and the composed retry+redirect+auth+logging pipeline: a one-shot body makes a 307 unfollowable, so `decide()` fails with the redirect target in the message, and the target carries a credential-shaped query value the test itself supplies. Asserts the captured `http.redirect.rejected` record shows `access_token=***`, and that no field of ANY record the whole pipeline emitted carries the secret in clear text -- which is XCUT-19's actual claim, not just this one event's. Needs a Location with a secret in it, which no existing fixture route serves, so `/redirect-secret-target` joins `server.ts`; it echoes the caller's own `?secret=` back into the Location so the test owns the string it then greps for. XCUT-19(a)/(b), OBS-11, OBS-12, REDIR-28.
…uide (#70) `errors.md` listed both redirect errors in its table and said nothing about which of their URL surfaces is safe to log. A consumer reading `error.message` into their own logger now knows it is already redacted, and that `fromUrl`/`toUrl`/`targetUrl` are the raw values for program use.
…#71) `BasicCredential` and `DigestCredential` shipped as structural interfaces with a public `readonly password: string`, so `util.inspect` of an `AuthCredentialSet` printed `password: 'hunter2'` beside `ApiKeyCredential{key=***}` and `JSON.stringify` serialized both passwords. Both are now classes in `auth/credential.ts` on exactly the pattern `ApiKeyCredential`/`NameKeyCredential`/`BearerToken` already use: a `#password` field, a redacted `toString()`, the `nodejs.util.inspect.custom` hook, and an in-package `credentialPassword()` friend hook as the sole read path -- the mirror of `credentialKey()`, and `buildHandlers` is its only caller. AUTH-8's redaction clause enumerates bearer, API key and name key only; reading it as covering every credential type is a deliberate widening, ledgered separately. Validation is deliberately NOT duplicated onto the classes: AUTH-14's non-empty-whitespace-permitted rule and AUTH-16's acceptable-set rule stay single-sourced in `basicHandler()`/`digestHandler()`, which `authStep()` builds at construction, so a blank password still fails synchronously from that factory. Public shape change: `{username, password}` object literals no longer type-check. Free before the first version bump.
) `guardReplayScheme` tested two header NAMES -- `Authorization` and `Proxy-Authorization`. `ApiKeyCredentialConfig.headerName` lets this very step stamp any header the caller names, so a `challengeHook` answering a 401 with `X-Api-Key: SECRET` over a downgraded `http://` URL was dispatched in clear text with no `PlaintextCredentialError`. `OutboundPlan` now carries `guarded` -- whether the outbound pass ran `requireHttps` on this hop -- and the replay inherits it: once guarded, always guarded, without inspecting a single header name. Deriving the set of credential-carrying names from configuration was rejected; a hook may invent a carrier this step has never been told about, so no enumeration can be complete, whereas "this hop is credentialed" is a fact the outbound pass already decided. The header test survives as a second arm: a `NO_AUTH` hop is never guarded outbound, and a hook answering its challenge with an `Authorization` header is attaching a credential all the same. XCUT-16's carve-out is preserved -- a genuinely credential-free re-issue over any scheme still proceeds.
…UT-16 readings (#71) `auth.md`'s `AuthCredentialSet` sketch still showed `{username, password}` object literals directly above the claim that a credential cannot leak into a log line by accident, which was the false half of the pair the audit found. It now shows the five classes, states that there is no `password` property to read back and why, and documents the "once guarded, always guarded" replay rule under the HTTPS-guard heading. Two rows appended to `deviations.md` under "Deviations recorded outside a phase": AUTH-8's redaction clause read as covering every credential type rather than the three it enumerates, and XCUT-16's replay guard keyed on whether the hop was guarded rather than on whether the replacement looks credentialed -- the second is knowingly wider than the requirement's letter.
…tion-67 # Conflicts: # tests/conformance/xcut/security-by-default.conformance.test.ts
`README.md:53` still listed `Content-Length`, `Host`, `Transfer-Encoding` and `Connection`. `FETCH_FORBIDDEN_HEADERS` gained `Expect`, `Keep-Alive` and `Upgrade` earlier on this branch, so the sentence has been false since f02dd8e. Rewritten as three bullets rather than one: which names the client computes and which the layer underneath refuses; why the refused three are dropped rather than forwarded (WHATWG names all four forbidden, the implementations enforce none of it, and they disagree about what happens instead -- Node's undici-backed `fetch` fails the send with the RETRYABLE `TransportFailureError`, Bun 1.3.14 forwards two to the wire and hangs on `Upgrade`); and that a non-token header name degrades to the same drop, which the README had never said at all. `packages/transport-undici/README.md` needed nothing: 6df9645 already widened its enumeration, including the `Connection`-value split undici alone has. Refs #81, #67.
`degradeInboundHeaders`' inbound-value gate read `/[\x00-\x08\x0B-\x1F\x7F]/u`, which skips `\x0A` along with the intended `\x09`. Nothing observable changed — `Headers.addInbound` applies core's own `hasForbiddenInboundValueByte`, which does reject LF, and the `try`/`catch` two lines down records the same drop — so the two gates were redundant and only one of them was right. The class is now `/[\x00-\x08\x0A-\x1F\x7F]/u`, identical to core's. The constant is exported from the module (not from the barrel) so its test can read the character class directly: a test that went through `degradeInboundHeaders` would have passed against the broken class, which is how this survived from Phase 8a to audit #67 / #82. Found by: audit #67 / #82.
`@dexpace/transport-fetch` wrapped every native rejection as `TransportFailureError`, which `retry/classify.ts` reports retryable for being an `IoError`. `Request` accepts any absolute URL, so `ftp://example.com` reached `fetch`, was refused permanently, and spent the caller's whole retry budget re-proving it. `@dexpace/transport-undici` refused the same condition through `TERMINAL_ARGUMENT_CODES`: the two adapters classified one condition oppositely. The decision moves to `@dexpace/transport-shared`'s new `dispatch-classification.ts` — the precedent is `abort-mapping.ts` — as `isPermanentDispatchFailure` plus `toDispatchFailure`, and both adapters call it. It is an allow-list of three positive recognitions, so an unrecognised rejection stays the retryable `TransportFailureError` TRANSPORT-20 makes a MUST: - a terminal argument code on the error or its immediate cause, which is undici's whole dispatcher leg (`UND_ERR_INVALID_ARG`, `UND_ERR_NOT_SUPPORTED`) and Bun's `fetch` (`ERR_INVALID_ARG_VALUE` and friends); - a `TypeError` with no `cause`, which is how undici's `fetch` — Node's global `fetch` — reports argument validation, network failures always carrying one; - a cause naming one of three WHATWG scheme refusals, which is the only way that same `fetch` can report `ftp://` at all. `bad port` is deliberately excluded: port 1 is on WHATWG's blocked list, so TRANSPORT-20's own dead-port probe arrives with that reason and must stay retryable. A `DexpaceError` is passed through unchanged — it was classified at its source. Rows added, red against the unfixed fetch transport and already green against undici: `an unsupported URL scheme fails outside the IoError tree` in the shared suite (asserting `isIoError(e) === false`, which is exactly what the retry engine asks), the Node-runtime twin in `tests/node-conformance/transport.test.mjs` because the two runtimes use entirely different error shapes for it, five unit rows in `transport-fetch`, and twelve in `transport-shared`. Found by: audit #67 / #82.
…apters 204, 304, 205, 101, 103, every HEAD and a 2xx CONNECT can carry no body, and three of the four native combinations the two adapters meet disagreed about how to say so. undici's dispatcher always hands back a `BodyReadable`, so `@dexpace/transport-undici` wrapped an empty stream; Node's `fetch` returns `null` per the spec; Bun 1.3.14's `fetch` returns a live `ReadableStream` for all three (measured 2026-09-05), so `@dexpace/transport-fetch` was reporting the runtime's answer rather than the contract's. `hasNoResponseBody(method, status)` in `@dexpace/transport-shared` is now the rule and both adapters apply it, so `body === null` is a property of the SDK on every runtime. It is the WHATWG shape and the one `http/response.ts:18` already types; the rejected alternative, an empty stream on both, makes a consumer read to learn there is nothing to read. Each adapter releases the native handle it declines to expose — `cancel()` on fetch's, `dump()` on undici's. `Response.close()` is a no-op on a null body, so nobody else would, and an undrained `BodyReadable` holds the pooled connection open until the dispatcher times it out (TRANSPORT-25, SEAM-30). Rows: 204, 304 and HEAD in the shared suite, each asserting `body === null`, the `content-length` the case does or does not justify, and `reasonPhrase` as `undefined`-or-string — the fetch/undici divergence there is D7's ledger row beside §10 item 13 and is not re-ledgered. A GET over the same route is the twin, so nulling a body-less response cannot quietly null an ordinary one. All six were red on both adapters. `tests/node-conformance/transport.test.mjs` gets the runtime-divergent case, red on undici and green on fetch there, which is the asymmetry the Bun rows cannot show. `fixtures.ts`'s `route` passed the 70-line cap, so the three body-less fixtures are their own function. Found by: audit #67 / #82.
When a streaming request-body producer lost the race in `#dispatch`, `send()`
rejected while the native call was still pending, and nothing cancelled it. A
response arriving afterwards was dropped with its body neither read nor
released — TRANSPORT-9's leak, from the request side. `abandon` unwound the
producer; it could not reach the fork.
It could not reach the fork because for a send with no caller signal and no
composed timeout there was none: `forkSignal(undefined)` returned
`{signal: undefined}` and both transports dispatched with no signal at all,
which is exactly the case with nothing left to cancel with. `ForkedSignal.signal`
is now always a live `AbortSignal` — one controller nobody may ever abort, and
indistinguishable to the native client from no signal — and the interface gains
`abort(reason)`. `detach()` latches it, so the new direction cannot become the
SEAM-16 violation the fork's original direction exists to prevent.
Both transports read whether the *caller* aborted before pulling the fork
themselves; reading it after would surface every producer failure as a
`CancellationError`.
`producerFailure` now classifies its own rejection as the retryable
`TransportFailureError`, which is what both catches already produced for it.
That is not cosmetic: the same catch now runs native rejections through a table
that reads a bare `TypeError` as a permanent misconfiguration, and a producer
that threw one would have been mistaken for the wire refusing the request.
`prepareBody`'s buffered branch has classified the same failure at its source
since Phase 8a.
Instrumented rows on both transports — a `FetchLike` and a bring-your-own
`Dispatcher` whose native call resolves 30ms after the producer fails — assert
the dispatched signal is aborted with the producer's error and that the late
response never settles into the send. Both were red. Their twins assert a
delivered response leaves the fork unaborted. `signal-fork.test.ts` gains four
rows for the two-way fork and the latch. The fetch `defaultTimeoutMs` row is
rewritten: "a signal was handed over" no longer discriminates anything, so it
asserts the deadline is honoured instead.
Found by: audit #67 / #82.
`defaultTimeoutMs` was unchecked on both transports and reached `AbortSignal.timeout()` untouched. Node throws `RangeError` on `1.5`, `2**32` and `-1`; Bun 1.3.14 accepts the first two. The same misconfigured transport therefore failed every send on one runtime and used a deadline nobody asked for on the other. It is also the last such path. `RequestOptionsBuilder.timeoutMs` has enforced the integer `1 .. 2**32 - 1` range at its setter since audit #67 / #76, on HTTP-35's reading that a timeout a setter accepted and a transport then refused belongs at the call site. `requireValidDefaultTimeoutMs` in `@dexpace/transport-shared` applies the identical rule with the identical wording, and both factories call it first thing — before `selectDispatchers` allocates, so a refusal cannot leak an `Agent` with no transport to close it through. A `TypeError`, matching the two construction-time refusals `undiciTransport` already raises and asserted the same way. `@throws` on both factories, and the `defaultTimeoutMs` TSDoc now states the range. `TransportCapabilities` gains a required `buildWithDefaultTimeoutMs(value)`: required rather than a flag because §17 assumes every transport has a default (TRANSPORT-5 is written against one), and typed `number` because every value the rows supply legitimately is one. Twelve rows, red on both adapters, plus the in-range twin that proves narrowing did not reject a legitimate default, plus the Node-runtime case — that one matters because Node is the runtime that used to fail late and loudly where Bun failed silently. Carried from audit #67 / #76 (D14's hand-off). Found by: audit #67 / #82.
`docs/deviations.md` gains one row for the reading the classification table rests on: `TRANSPORT-20`'s "any transport failure that produced no HTTP response" is read as an exchange that failed, not as a request the native client refused to make, and such a refusal is reported outside the `IoError` tree so `retry/classify.ts`'s allow-list makes it non-retryable. The reading is not new — undici has applied it since Phase 8a — but it lived only in that phase's checklist, and until this run `@dexpace/transport-fetch` did the opposite for the identical condition. The row records that the MUST is still the default: the table is an allow-list, and `bad port` is excluded by name because TRANSPORT-20's own dead-port probe arrives with that reason on Node. `reasonPhrase` is deliberately not re-ledgered — it is already a row beside §10 item 13. Two citations this branch moved are re-anchored: the SOCKS row's `undici-transport.ts:138,151-158,192` -> `:147,160-167,201` and `fetch-transport.ts:76-79` -> `:79-82`. `run-suite.ts`'s header re-anchors the two TRANSPORT-22 test citations for the same reason, and says why TRANSPORT-9's producer race is not a shared row: only an instrumented native client can show that a pending call was cancelled. READMEs: `transport-shared`'s module table gains the three new modules and the fork's second direction; both adapters' behaviour lists gain the classification rule, the body-less contract and the `defaultTimeoutMs` range.
This was referenced Sep 5, 2026
Closed
Closed
Contributor
Author
|
Subtask issues closed 2026-09-05, each pointing here. Task → PR that landed it on this branch:
Umbrella #67 stays open until this PR merges. Every wave was preflighted ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remediation of the 2026-09-04 audit of
mvp(#67). Fifteen subtasks, each landed as its own PR into this branch after a fullrun-ci.mjs --cleanpreflight on the merged tree. Every decision the run took, and everything it deferred, is indocs/audit-67-decisions.md(D0–D20). Basemvpis untouched at1f48926.What changed, per task
Milestone 1 — docs
docs/deviations.mdand fix public TSDoc that contradicts the code #68 (PR docs: re-anchor deviations.md and fix public TSDoc that contradicts the code (#68) #83):docs/deviations.mdre-anchored; public TSDoc that contradicted the code corrected (Deserializer,jsonSerde(),tracerFactory,buildRequest,RequestConditions.applyTo).noopInstrumentationBundle.activeSpanisNOOP_SPAN;Span/Tracertypes moved toobservability/span.tsto break an import cycle.Milestone 2 — security and log hygiene
http.redirect.rejectedlog path #70 (PR fix: redact URLs in the http.redirect.rejected log path (#70) #85): redirect rejection errors build their messages fromredactUrl();http.redirect.rejectedcarries a redactedurl.full.BasicCredential/DigestCredentialare classes with a#password, redactedtoStringandinspect; the replay HTTPS guard applies to every guarded hop, not only the three enumerated header names.Milestone 3 — contract decisions
SuppressedErrorwrapper #72 (PR Retry surfaces the typed error (#72) #88): retry surfaces the final attempt's own error, never aSuppressedErrorwrapper; earlier attempts are read through the newretryAttempts(error)export. A cancelled backoff is nowinstanceof CancellationError.dispatchWithRecoverysplit intoprepareRequest+dispatchPrepared.WWW-Authenticate, Digest-sesswithoutqop, emptynonce/realm#74 (PR Auth challenge handling: repeated headers, -sess cnonce, empty realm/nonce (#74) #89): everyWWW-Authenticate/Proxy-Authenticatevalue is parsed, not only the first;cnonceis emitted for-sessalgorithms withoutqop; an emptyrealmornonceis declined.sseEvents$and the caller-owned stream #75 (PR RxJS SSE ownership: keep the transfer, ledger it (#75) #87):sseEvents$/typedSse$keep ownership of the stream; documented outright and ledgered as an ASYNC-21 departure, with tests counting the underlying resource's release.Milestone 4 — correctness
Date, fractionaltimeoutMs, lone surrogates, frozengetAll#76 (PR Domain-model input validation: prototype keys, invalid Date, fractional timeoutMs, lone surrogates, frozen getAll (#76) #92): prototype keys no longer satisfy a path placeholder; invalidDaterejected;timeoutMsmust be an integer in1..2**32-1; lone surrogates rejected atQueryParamsBuilder.addand path substitution;getAllalways frozen;TeeSinkrejects a fractional limit.streamBody, multipart boundary quoting,withResponseLoggingafterclose()#77 (PR Body lifecycle: empty chunks, multipart boundary quoting, logging tap after close (#77) #91): an empty chunk during an exact-length body copy is aSourceContractViolationError; the multipart boundary is quoted when it is not a token; the response logging tap is safe afterclose(); 22 new Node-conformance cases for the Web Streams bridges.instanceof IoErrorafter the flat tier, non-finitedelayOverride, backoffNaN#78 (PR Retry engine and error classification: IoError rule, non-finite delayOverride, backoff NaN (#78) #93):instanceof IoErrorin the retry classifier now means "the wire failed" and is pinned per class; a non-finitedelayOverridefalls back like a throwing one;computeDelaynever returnsNaN.deserializeFromabort, paginator leak on malformedPageInfo,tristatenull cast, SSE double-reported release failure #79 (PR Serde and pagination: abortable reads, paginator close on malformed PageInfo, tristate null, SSE single report (#79) #94):deserializeFrom/serializeTorace pending reads against the abort signal; the paginator closes the response on a malformedPageInfo;tristate()never yields a presentnull; an SSE release failure is reported once; a server-supplied cursor with a lone surrogate throwsUrlConstructionError.enterWithleak, operation span reachable from the public API, context-store eviction, body-drain diagnostics,DEXPACE_LOG_LEVELwiring #80 (PR Instrumentation: context restored after send, public instrumentation option, store hygiene, body-drain diagnostics, log-level wiring (#80) #95):send()scopes its async stores withAsyncLocalStorage.run, so the caller's context is restored afterawait; new publicPipelineOptions {instrumentation, operationName}onPipelineBuilderandstandardResilience(); context-store entries no longer leak on a throwingtracerFactory;span.end()runs once; body-drain failures emithttp.instrumentation.bodyCaptureFailed;LoggingStepSettings.configKey;DEXPACE_LOG_LEVELwiring documented.Milestone 5 — transport parity
fileBodyshort-write detection, SOCKS proxy type #81 (PR undici transport parity: header rejections degrade, fileBody short-write detection, SOCKS refusal (#81) #96): undici degradesExpect,Keep-Alive,Upgrade, non-token names and unforwardableConnectionvalues to logged drops (the fetch drop set widened by the same three names);fileBodygoes throughwriteToon both transports so a short write is detected; a SOCKS proxy type is refused at the factory.CONTROL_BYTEregex, producer-failure race #82 (PR Transport parity: permanent-error classification, body-less responses, CONTROL_BYTE, producer-failure race, defaultTimeoutMs (#82) #97): one classification table intransport-shareddecides permanent vs retryable native failures for both transports; body-less responses (HEAD, 204, 304, …) reportbody === nullon both;CONTROL_BYTEcovers LF; a producer failure aborts the native call it raced;defaultTimeoutMsvalidated at both factories.Open questions
ProxyTypeunion. Core resolvessocks4/socks5(CFG-22 requires the model to carry them) but neither transport supports them; the factory refuses with aTypeError. Narrow the union before the first version bump, or keep it and rely on the refusal?DEXPACE_LOG_LEVELdefault. The global configuration stays empty by default, so the variable is inert untilsetGlobalConfiguration(defaultConfiguration())is called (now documented). ShoulddefaultConfiguration()become the default, reading the environment at import time?close()fails while an iterator is parked in a read, the failure reaches bothonReleaseFailureand the thrown error'ssuppressed. Left as is (the listener cannot know); decide whether that is acceptable.streamBody'spipeTopath still forwards empty chunks; only the exact-length copy rejects them. Intercepting it would change cancellation ownership (BODY-8).packages/core/src/io/index.tsis a dead barrel nothing imports, and its header comment ships a brokendist/io/index.d.tsviastripInternal. Delete it?PRE_REDIRECTstatus mapping) is ledgered as a gap; needs a maintainer-opened tracking issue.Deferred — release machinery
No changesets, version bumps or
docs/first-release.mdedits were made (D1). The per-PR list of changesets to write is the "Deferred — release machinery" table at the end ofdocs/audit-67-decisions.md.