feat(sdk)!: add options.headers, fetchOptions, and fetch - #456
Conversation
The SDK called the global `fetch` directly, so consumers had no way to influence how requests are made — ClientOptions exposed only maxRetries. `options.fetch` accepts any fetch-compatible function and is used for every request, retries included. The motivating case: undici 8.8.0-8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds (nodejs/undici#5600, fixed in 8.10.0), and Node 26 bundles 8.9.0. A consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can supply an undici dispatcher with a tuned keepAliveTimeout and carry on. The same hook covers the ordinary reasons SDKs grow one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional through to HttpContext rather than being defaulted at construction, so the default path still calls the global directly. That keeps it late-bound — replacing globalThis.fetch after a client exists still works, which is what vi.stubGlobal does — and avoids invoking a detached `fetch` reference, which is not universally safe. Both properties are pinned by tests. Streaming is unaffected: .stream()/.liveQuery() go through EventSource, which this does not replace. Documented as such. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014EJvon2pyU4zaSHTGJj8fE
Documentation sync for the preceding commit — a new public ClientOptions field on a published package, which AGENTS.md §Documentation Sync requires under [Unreleased]. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014EJvon2pyU4zaSHTGJj8fE
Pre-push review found the advertised type surface didn't match what
shipped, in two ways.
FetchLike was declared and referenced by ClientOptions but never listed
in index.ts's export block, so `import type { FetchLike } from
"@wavehouse/sdk"` did not resolve -- while the CHANGELOG claimed it was
exported. It's now in the export list; verified in dist/index.d.ts.
The documented undici example did not compile. Reproduced with the
repo's own tsc against undici 7.28: under the default lib (which
includes DOM, and is what this package's own tsconfig uses) the wrapper
fails with TS2322 on Response -- undici declares its own Request and
Response, separate from the ones behind the global fetch -- plus TS2345
on the input arg. Assigning undici's fetch export directly fails under
Node-only libs too. So the "satisfies it without casting" claim was
wrong in general, not just at the edges. The example now carries the
casts that make it typecheck (verified clean under both lib configs
against the built package), and the JSDoc says plainly that such
implementations need a cast, and why.
Also corrected: liveQuery's initial backfill is an ordinary request and
does go through the supplied fetch -- only the SSE connection is exempt.
The docs, JSDoc and CHANGELOG all said otherwise, and streaming.md
already described the backfill correctly, so the pages disagreed.
The docs section had been inserted between the ClientConfig table and
that section's trailing content, orphaning the auth note and the
baseURL path-prefix subsection under an h3 about fetch; it now sits
after them. Adds the install note for the undici import, states the
contract an implementer must satisfy (string URL, plain RequestInit,
only .ok/.status/.headers/.text() read back, rejection -> retried
NETWORK_ERROR, AbortError -> ABORTED), and leads with a middleware
example that compiles anywhere.
The override test asserted only that init.method was some string; it
now pins the full init -- string URL, POST, Content-Type, Authorization,
serialized body -- which is what proxy and middleware consumers rely on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Second pre-push review round. The contract sentence added in the last commit was itself wrong, and the type didn't accept the shape it described. The contract claimed the SDK reads "only .ok/.status/.headers/.text()" off the response. The error path also calls res.json() and reads res.statusText (errors.ts:7,17). A response object built to the documented spec doesn't crash -- parseErrorResponse swallows the TypeError -- it silently degrades, yielding a WaveHouseError whose message is typed string but is undefined at runtime, on every error response. Contract now lists all six members and says which are read when. FetchLike was typeof fetch, which rejects the very middleware signature the docs tell you to write: (url: string, init?: RequestInit) => Promise<Response> is not assignable to a parameter accepting RequestInfo | URL, so it fails on contravariance with an error that has nothing to do with the cause the docs give. Narrowed to the signature the SDK actually calls -- a string URL is all it ever passes. That's strictly more permissive: the global fetch still assigns, and declared middleware now assigns with no cast. Verified against the built package under both a DOM-inclusive and a Node-only lib config. Abort handling is now stated: ABORTED requires a DOMException named AbortError, so an implementation that signals abort another way -- node-fetch throws its own AbortError class -- is retried as NETWORK_ERROR instead. That was worth saying out loud given the JSDoc names node-fetch as a supported implementation. Corrected the severity attributed to nodejs/undici#5600. It was described as stalling "for seconds" on a socket idle for seconds; the issue is titled "stalls up to ~500ms on an idle event loop" and the reported measurements are ~450-465ms against a 10ms server, with the repro making back-to-back requests. The trigger is an idle event loop, not a seconds-idle socket. Also says plainly that 8.10.0 fixes it and upgrading is the real answer -- the page previously steered readers toward a permanent code change for a bug already fixed upstream. The undici snippet used FetchLike and createClient with no SDK import, so it didn't compile as pasted; it now imports what it uses. Config table notes SSE streams are excluded, since that carve-out was three sections below where most readers stop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
…inert Third pre-push review round. The docs prescribed a workaround for nodejs/undici#5600 that does not work, which I confirmed by measuring it rather than reasoning about it. Against a 10ms local server on undici 8.9.0 (Node 22.23.2), with 1.5s idle gaps, per-request latency: default Agent() 22 1513 1493 584 Agent({ keepAliveTimeout: 1_000 }) 13 1500 1496 1513 Agent({ keepAliveTimeout: 1 }) 15 1484 1511 1500 Agent({ pipelining: 0 }) 13 13 12 12 same script on undici 8.10.0 22 15 13 13 So the tuning the page recommended changes nothing: the socket retirement timer is starved by the same idle event loop that triggers the bug, so the socket is still pooled and still reused. What actually made the documented snippet fast is that `npm install undici` resolves to 8.10.0 -- a fixed implementation -- not the dispatcher argument. Reframed the remedy accordingly: options.fetch is how you route through an undici you install yourself, so the copy bundled with your Node never handles the request. Dropped the keepAliveTimeout argument and its "retire idle sockets sooner" comment, and said plainly that the knob does not help, since it is the obvious thing for a reader to try. For anyone genuinely pinned to an affected undici, pipelining: 0 is documented instead -- measured effective, at a connection per request. The setGlobalDispatcher fallback is now attributed correctly: the mechanism works, but only helps when the installed undici is 8.10.0+. Also corrected the severity in the other direction from last commit. The upstream issue title says ~500ms and I took it at face value; the stall is actually erratic and idle-gap dependent, measuring ~100ms to ~2.5s across gaps. The docs now give the upstream number and our own range rather than a single figure. Contract precision: .status is only read on the failure path (http.ts:80, errors.ts:19-22) -- the success branch touches only .text() and .headers. Listing it as always-read was conservative but wrong for a paragraph that is an explicit spec. Verified the rewritten snippet by running it, not just typechecking it: against a live server it returns data on the success path and, on a 404, a WaveHouseError whose message is "Not Found" -- i.e. the .statusText fallback the contract documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Fourth pre-push review round. The setGlobalDispatcher caveat ended "since it's the bundled implementation doing the stalling", which argues for the opposite conclusion: if the bundled copy is what stalls, a reader infers that swapping the global dispatcher cannot help -- which would also undercut the options.fetch remedy above it. The mechanism is that setGlobalDispatcher hands connection handling to the installed copy, so that copy is the one that has to carry the fix. Reworded to say so. Also adds stubbing HTTP in your own tests to the list of reasons to reach for the option. It is probably the most common one, and the CHANGELOG entry already called it out while the page did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
The text claimed our measurements ranged "~100 ms to ~2.5 s". That ceiling is an artifact of one setup: npm-installed undici 8.9.0 on Node 22, where gaps of 4s+ show no stall at all because the default 4s keepAliveTimeout has already retired the socket, so there is nothing to reuse. Measuring against Node 26's bundled 8.9.0 through the SDK tells a different story -- 6s gaps against a stub server answering instantly produced [12, 23990, 2203, 2100, 2099] ms. I can't reproduce that from Node 22 with an npm-installed undici, so rather than assert a bound I can't defend in both directions, the docs now say severity varies with runtime and idle gap, quote the upstream ~450-465ms figure, and give our own range as ~100ms to tens of seconds. This also keeps the entry consistent with #455's account of the same bug, which reported multi-second stalls from the e2e harness's multi-second lingers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe TypeScript SDK adds configurable REST fetch behavior through ChangesConfigurable REST fetch transport
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SDKClient
participant HTTPRequest
participant ConfiguredFetch
participant GlobalFetch
SDKClient->>HTTPRequest: Create request with client options
HTTPRequest->>HTTPRequest: Merge headers and fetchOptions
alt Custom fetch configured
HTTPRequest->>ConfiguredFetch: Send URL and protected RequestInit
HTTPRequest->>ConfiguredFetch: Retry request when required
else No custom fetch configured
HTTPRequest->>GlobalFetch: Resolve and send request at invocation time
end
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📚 Docs preview is live → https://f1ea3dcb-wavehouse-docs.wave-rf.workers.dev
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6d6d5e4-2aa4-4ac6-8d94-42c47e15434a
📒 Files selected for processing (7)
CHANGELOG.mdclients/ts/src/client.test.tsclients/ts/src/client.tsclients/ts/src/http.tsclients/ts/src/index.tsclients/ts/src/types.tsdocs/src/content/docs/sdk/index.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
clients/ts/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
The TypeScript SDK in
clients/ts/is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Files:
clients/ts/src/index.tsclients/ts/src/client.test.tsclients/ts/src/http.tsclients/ts/src/types.tsclients/ts/src/client.ts
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Documentation prose under the Starlight docs site must stay accurate against code, include runnable examples where relevant, and reflect code↔docs sync for changed behavior.
Files:
docs/src/content/docs/sdk/index.mdx
🧠 Learnings (10)
📓 Common learnings
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.test.tsCHANGELOG.mdclients/ts/src/types.tsdocs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.test.tsclients/ts/src/types.tsdocs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.test.tsclients/ts/src/http.tsclients/ts/src/types.tsclients/ts/src/client.ts
📚 Learning: 2026-08-12T05:38:52.277Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: tests/e2e/sdk/helpers.ts:214-221
Timestamp: 2026-08-12T05:38:52.277Z
Learning: In `tests/e2e/sdk/helpers.ts`, `chQuery` must reclassify a caught error as a request timeout or caller abort only when the error is an abort error. On Node 22, `AbortSignal.timeout()` produces an error named `TimeoutError`, `AbortController.abort()` and caller cancellation through `AbortSignal.any()` produce `AbortError`, and `JSON.parse()` failures produce `SyntaxError`.
Applied to files:
clients/ts/src/client.test.ts
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to internal/stream/**/*.{go} : Streaming/SSE code must preserve the hub’s per-role projection model, subscriber queues, bucket fan-out, heartbeating, and metrics semantics.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to internal/observability/**/*.{go} : Observability code must preserve the repo’s logging and telemetry invariants, including stdout fan-out, OTLP behavior, lazy dialing, and the private Prometheus registry.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to docs/src/content/docs/configuration.md : Document every new or changed configuration option in the configuration docs.
Applied to files:
docs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-08-11T15:22:23.813Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.
Applied to files:
docs/src/content/docs/sdk/index.mdx
🪛 LanguageTool
docs/src/content/docs/sdk/index.mdx
[style] ~355-~355: Since ownership is already implied, this phrasing may be redundant.
Context: ...h client certificates, wrap requests in your own middleware (logging, tracing, circuit b...
(PRP_OWN)
[style] ~356-~356: Since ownership is already implied, this phrasing may be redundant.
Context: ...racing, circuit breaking), stub HTTP in your own tests without monkey-patching a global,...
(PRP_OWN)
🔇 Additional comments (7)
clients/ts/src/types.ts (1)
83-134: LGTM!Also applies to: 328-329
clients/ts/src/index.ts (1)
31-32: LGTM!clients/ts/src/client.ts (1)
40-40: LGTM!clients/ts/src/http.ts (1)
59-69: LGTM!clients/ts/src/client.test.ts (1)
9-9: LGTM!Also applies to: 147-149, 155-191, 201-208
CHANGELOG.md (1)
14-15: LGTM!docs/src/content/docs/sdk/index.mdx (1)
330-330: LGTM!Also applies to: 352-429, 436-442
Both from CodeRabbit on #456. The override tests still carried `as unknown as FetchLike` from when FetchLike was typeof fetch and a bare vi.fn() couldn't satisfy it. Since the narrowing that is no longer true, so the casts were vestigial -- and a cast in the test undercuts the claim that a consumer needs none. Now `vi.fn<FetchLike>(...)`, which also types the recorded call, so the init assertions are checked against RequestInit instead of any. The pinned-to-an-affected-undici escape was described in prose but not shown, and it is the one case where you do have to merge a dispatcher into the init the SDK hands you -- the main example no longer does, since the remedy there is to bring your own fixed undici. Added the snippet; typechecked under both lib configs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
|
Scope expanded — this PR took the rest of #269's HTTP-customization surface rather than shipping the
Rationale: all three answer the same question ("how do I talk to a WaveHouse behind a gate?"), and shipping them separately would have meant three passes over the same config surface and docs section, with the first two each half-answering the case that motivated the issue. Also landed here, beyond the original scope:
Split out and not in this PR: per-call overrides and dynamic header callbacks (#459), the (Edited 2026-08-12 — the original text listed |
Completes #269's REST-side scope. Three knobs now, shaped after the conventions in Supabase's, OpenAI's and Anthropic's clients rather than invented here. options.headers adds static headers to every REST request -- the motivating case being a Cloudflare Access service token in front of WaveHouse. Names match case-insensitively, as HTTP requires, and the merge is deliberately asymmetric: configured headers sit underneath the SDK's own, and a collision drops the configured one rather than joining the two. auth keeps Authorization; a request's Content-Type and Accept can't be displaced by a global. Both rules are lifted from bugs other SDKs shipped -- a global Content-Type joined with an upload's own produced `application/json, image/png` and 415s, and a case-sensitive Authorization check let a lowercase spelling ride alongside the canonical one. options.fetchOptions merges extra RequestInit fields, for what isn't a header and doesn't warrant replacing the transport: credentials: "include" for a cookie-authenticated origin, plus mode/cache and runtime extensions like Next.js's next: { tags }. The fields the SDK controls -- method, headers, body, signal -- are applied after the spread, so this can't corrupt the request. fetchOptions.headers is ignored rather than merged; options.headers is the header channel, and merging both would give two precedence stories for one concept. FetchLike widens to the standard fetch signature. It was narrowed earlier in this branch on the theory that it accepted strictly more, which was only true in one direction: a value typed as the narrow form is rejected by every other SDK's wide option, so a shared wrapper couldn't be handed to both. Every client surveyed uses the wide shape and none carry complaints about it. Still written out rather than `typeof fetch`, which resolves differently with and without DOM in lib. REST-only, stated plainly in the docs rather than left implicit: SSE takes neither headers nor a fetch, so a header-gated deployment can query but not stream until #203. The same gap in Supabase's realtime client was found by a user whose RLS policies silently stopped matching, which is the failure mode worth pre-empting. Verified on the wire against a live server, not just against a mocked fetch: the CF-Access header is sent, auth beats a lowercase `authorization` impostor, and Content-Type stays application/json. Per-call overrides and dynamic header callbacks are deferred to #459. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
…widening
Review round on the expanded scope. Three real defects, two of them
caused by the FetchLike widening in the previous commit.
mergeHeaders matched case-insensitively against the SDK's headers but
not within the configured set, so `{ "x-tenant": "a", "X-Tenant": "b" }`
survived as two keys and the Headers constructor comma-joined them at
fetch time -- `x-tenant: a, b`. Verified on Node. That is the exact
corruption the function's own docblock cites as its reason to exist, so
it was worth closing rather than documenting. Configured names now
replace each other, last spelling wins, with a test.
Widening FetchLike broke both undici snippets and I did not re-check
them -- I verified the new headers/fetchOptions examples and assumed the
existing ones were unaffected. They were not: `url` is now
`string | URL | Request`, which is not assignable to undici's
RequestInfo whenever DOM is in lib, i.e. the TypeScript default.
Reproduced as TS2345 against undici 7.28 and undici-types 8.3. Both
snippets and the matching JSDoc now cast the URL, and the
"casts are load-bearing" paragraph explains why there are two.
The CHANGELOG entry still carried the pre-widening rationale and
contradicted itself a few sentences apart -- claiming FetchLike both
matches the standard signature and is deliberately narrower than it.
The narrow claim is now false against the shipped type. CHANGELOG.md
sits outside scripts/docs-prose.sh, so no gate would have caught it.
Also corrected in the docs:
- credentials: "include" was the headline fetchOptions example, and it
cannot work against a stock WaveHouse. corsMiddleware deliberately
never emits Access-Control-Allow-Credentials (it is a Bearer-token
API, asserted by a test in router_test.go), and the default
cors_allowed_origins "*" makes include a hard browser failure anyway.
Now led by cache/keepalive, with credentials kept as a qualified note
pointing at the proxy-owns-CORS case.
- Custom headers from a browser must pass CORS preflight, and
Access-Control-Allow-Headers is a fixed list with no config knob, so
a CF-Access header only works server-side or behind a proxy that owns
CORS. Said outright rather than left for someone to discover.
- "Values replace, never append" described nothing observable, since a
collision drops the configured value rather than replacing the SDK's.
Reworded to what actually happens.
- mode/cache/keepalive were lumped in with "not declared on the
standard RequestInit". They are standard; only the runtime
extensions need a cast.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Docs review found that the documented workaround does not do what it
claims, and measurement confirms it. Third time the undici guidance in
this branch has been wrong in a way only measurement caught.
The section said: install undici 8.10.0 yourself and route through it,
"so the version bundled with your Node never handles them". That is not
what happens. undici keeps its connection pool on a shared globalThis
symbol -- Symbol(undici.globalDispatcher.2) -- and the first copy loaded
claims it, which on an affected runtime is the bundled one. fetch()
resolves that global when no dispatcher is passed, so an installed
8.10.0's fetch dispatches through the bundled 8.9.0's pool.
Reproduced with both copies loaded side by side. new.getGlobalDispatcher()
returns an instance of old.Agent, and the timings follow, 1.5s idle gaps
against a 10ms server:
no explicit dispatcher 21 1514 1495 583
explicit new Agent() 17 14 12 13
So the snippet now passes the dispatcher explicitly, which never
consults the shared symbol, and a caution block explains why with those
numbers. Verified end to end through the SDK with 8.9.0 loaded first:
24, 15, 13, 14 ms.
This also collapses the section, since the pinned-to-an-affected-version
case was already passing an explicit dispatcher -- it is now the same
snippet with different Agent options rather than a second one. The
setGlobalDispatcher note gains the reason it works: an explicit write
wins the same race.
Also from the same review:
- "Both casts are load-bearing" undercounted the snippet, which has
three. init as never was left unexplained, which is the one that looks
most alarming. All three are now named, with what each bridges.
- Next.js next: { tags } was described as per-deployment cache control.
It tags a cache entry for on-demand revalidation via revalidateTag().
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Review round on the previous fix. The caution said an explicit dispatcher and an explicit setGlobalDispatcher both win "because it never consults the shared symbol". That is only true of the dispatcher. setGlobalDispatcher wins the opposite way -- it overwrites the symbol after the first copy claimed it -- which is what the paragraph further down already said, so the page contradicted itself and left no way to understand why a call on the installed copy affects the bundled fetch at all. Reasons are now split. The casts paragraph still spelled the init cast `init as never`, which was the pre-dispatcher snippet's text. The snippet now casts the merged object. Beyond not matching, copying the cast as the prose spelled it would drop `dispatcher` and land back on the stalling global pool that the caution three lines above exists to prevent. Also took the reviewer's suggestion on the pinned-version paragraph: it reuses a snippet whose comment reads "8.10.0+", which is the opposite of that reader's situation, so it now says to use your pinned version with pipelining: 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
The caution said the shared globalThis symbol is claimed by "the first copy loaded -- which on an affected runtime is the bundled one". The second half is wrong, and wrong in the ordering the snippet above it creates. Node does not claim the symbol at startup; it loads its bundled undici on the first call to the built-in fetch. Verified locally: no undici symbol exists on globalThis at startup, and Symbol(undici.globalDispatcher.1) appears only after a fetch() call. Since ESM imports evaluate before any user code runs, `import ... from "undici"` normally makes the INSTALLED copy the claimant. My earlier measurement forced the bundled-first ordering with an explicit `import "old"`, which is why it looked deterministic. The measurement sentence already says "with 8.9.0 loaded first", so it was honest; the surrounding claim was not. Reworded to what holds: whichever copy loads first claims it, Node claims for the bundled copy on the first built-in fetch rather than at startup, so ownership comes down to a load order you don't really control -- which is a better argument for passing the dispatcher explicitly than the deterministic version was, since it removes the dependency instead of betting on it. Same softening in the CHANGELOG; the JSDoc already hedged correctly. Also labels the measurement block's units. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Two leftovers from the previous round. The caution named a built-in fetch() call as what makes Node claim the shared dispatcher symbol. Too narrow. Verified on Node 22: new Headers(), new Response(), new Request(), new FormData() and even reading globalThis.WebSocket all claim it; only reading globalThis.fetch without calling does not. That matters because the caution exists to stop a reader reasoning about the ordering -- someone who audits "I never call the built-in fetch, everything goes through undiciFetch" gets a false all-clear. Worth saying outright that this SDK is one such claimant: http.ts constructs a Headers on the abort and retry-exhausted paths. The CHANGELOG still said undici needs casts on "both the URL argument and the return value". There are three -- the init cast is unavoidable regardless of the DOM-lib split, since dispatcher isn't a RequestInit field at all. Same undercount already fixed in the docs and already correct in the JSDoc, so the PR was contradicting itself across its own artifacts. CHANGELOG.md is excluded from scripts/docs-prose.sh by name, which is why this is the second factual drift to survive there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6c5b3cc9-35af-4bb6-9d93-2c65619151da
📒 Files selected for processing (10)
CHANGELOG.mdclients/ts/src/client.test.tsclients/ts/src/client.tsclients/ts/src/http.tsclients/ts/src/index.tsclients/ts/src/pipes.tsclients/ts/src/query-builder.tsclients/ts/src/table.tsclients/ts/src/types.tsdocs/src/content/docs/sdk/index.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Integration tests
- GitHub Check: Docs build
- GitHub Check: E2E tests
- GitHub Check: Lint
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (2)
clients/ts/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
The TypeScript SDK in
clients/ts/is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Files:
clients/ts/src/index.tsclients/ts/src/client.tsclients/ts/src/table.tsclients/ts/src/query-builder.tsclients/ts/src/client.test.tsclients/ts/src/types.tsclients/ts/src/http.tsclients/ts/src/pipes.ts
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Documentation prose under the Starlight docs site must stay accurate against code, include runnable examples where relevant, and reflect code↔docs sync for changed behavior.
Files:
docs/src/content/docs/sdk/index.mdx
🧠 Learnings (24)
📓 Common learnings
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: docs/src/content/docs/sdk/index.mdx:0-0
Timestamp: 2026-08-12T15:28:20.891Z
Learning: For `docs/src/content/docs/sdk/index.mdx`, the documented workaround for the undici idle-event-loop keep-alive stall is to upgrade to undici 8.10.0 or later. If a consumer is pinned to an affected version, `new Agent({ pipelining: 0 })` must be merged as `dispatcher` into the SDK-provided `RequestInit`; this disables keep-alive reuse. Configuring `keepAliveTimeout` does not mitigate this stall because the socket retirement timer is starved by the same idle event loop.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.tsclients/ts/src/table.tsclients/ts/src/query-builder.tsclients/ts/src/client.test.tsCHANGELOG.mdclients/ts/src/types.tsclients/ts/src/http.tsdocs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.tsclients/ts/src/query-builder.tsclients/ts/src/client.test.tsCHANGELOG.mdclients/ts/src/types.tsclients/ts/src/http.tsdocs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/index.tsclients/ts/src/client.tsclients/ts/src/table.tsclients/ts/src/query-builder.tsclients/ts/src/client.test.tsclients/ts/src/types.tsclients/ts/src/http.tsclients/ts/src/pipes.ts
📚 Learning: 2026-08-11T21:56:03.206Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/query_builder.go:278-291
Timestamp: 2026-08-11T21:56:03.206Z
Learning: In `clients/go/query_builder.go`, `fetchNextTyped` intentionally treats a failed JSON decode of a non-object typed `Row` as normal end-of-pagination. This behavior matches the existing “cursor column was not in the projection” path and TypeScript SDK parity. The broader behavior change is tracked in GitHub issue `#452`.
Applied to files:
clients/ts/src/table.tsclients/ts/src/query-builder.ts
📚 Learning: 2026-08-11T21:56:06.521Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: docs/src/content/docs/sdk/go/queries.md:356-362
Timestamp: 2026-08-11T21:56:06.521Z
Learning: In Wave-RF/WaveHouse Go SDK cursor pagination, `fetchNextTyped` uses only the first `QueryBuilder.OrderBy` column and a strict `gt` or `lt` filter. Duplicate values at a page boundary can skip rows. The Go SDK documentation must require a unique ordering column until the shared Go and TypeScript composite-cursor or tie-breaker implementation tracked in GitHub issue `#452` is available.
Applied to files:
clients/ts/src/query-builder.ts
📚 Learning: 2026-08-12T05:38:52.277Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: tests/e2e/sdk/helpers.ts:214-221
Timestamp: 2026-08-12T05:38:52.277Z
Learning: In `tests/e2e/sdk/helpers.ts`, `chQuery` must reclassify a caught error as a request timeout or caller abort only when the error is an abort error. On Node 22, `AbortSignal.timeout()` produces an error named `TimeoutError`, `AbortController.abort()` and caller cancellation through `AbortSignal.any()` produce `AbortError`, and `JSON.parse()` failures produce `SyntaxError`.
Applied to files:
clients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-05-19T14:41:38.228Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/scripts/screenshot.mjs:31-31
Timestamp: 2026-05-19T14:41:38.228Z
Learning: `docs/scripts/screenshot.mjs` in the Wave-RF/WaveHouse repo is an intentionally manual dev-iteration tool (not wired into CI). Response-status validation and retry logic are deliberately deferred until the script is promoted to a CI visual-regression workflow. Do not flag the absence of `response.ok()` checks as an issue in this file.
Applied to files:
clients/ts/src/client.test.ts
📚 Learning: 2026-08-12T05:35:22.804Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: clients/ts/README.md:11-11
Timestamp: 2026-08-12T05:35:22.804Z
Learning: For Wave-RF/WaveHouse, CI currently tests Node 22 only through `.nvmrc` and `.github/actions/setup-env` using `node-version-file`. Local `make ci` verification on Node 26 is not CI coverage and must not be documented as continuously tested runtime support.
Applied to files:
clients/ts/src/client.test.ts
📚 Learning: 2026-08-12T15:28:20.891Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: docs/src/content/docs/sdk/index.mdx:0-0
Timestamp: 2026-08-12T15:28:20.891Z
Learning: For `docs/src/content/docs/sdk/index.mdx`, the documented workaround for the undici idle-event-loop keep-alive stall is to upgrade to undici 8.10.0 or later. If a consumer is pinned to an affected version, `new Agent({ pipelining: 0 })` must be merged as `dispatcher` into the SDK-provided `RequestInit`; this disables keep-alive reuse. Configuring `keepAliveTimeout` does not mitigate this stall because the socket retirement timer is starved by the same idle event loop.
Applied to files:
CHANGELOG.mdclients/ts/src/types.tsdocs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-07-07T12:38:15.328Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:15.328Z
Learning: Repo: Wave-RF/WaveHouse. WaveHouse deliberately does not log or trace any client IP address anywhere in the codebase. `middleware.RealIP` was removed in PR `#332` due to IP-spoofing GHSAs, and trusted-proxy-aware client-IP extraction for logs/traces is tracked as a future cross-cutting effort in issue `#333`. Do not suggest adding `r.RemoteAddr` or naive `X-Forwarded-For`-derived IPs to logs (e.g., audit logs in internal/auth/auth.go for the operator-key path) until `#333` lands with proper trusted-proxy handling.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-08-11T15:22:47.380Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: docs/src/content/docs/sdk/index.mdx:330-334
Timestamp: 2026-08-11T15:22:47.380Z
Learning: In WaveHouse Go server authentication, `internal/auth/auth.go` `bearerToken` returns from the `Authorization` header path before modifying `r.URL`. It removes the `token` query parameter only when authentication uses the query parameter without an `Authorization` header. Documentation must state that this protects WaveHouse's own logs only; reverse proxies, CDNs, load balancers, and other upstream intermediaries require query-string redaction.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to internal/auth/**/*.{go} : JWT auth middleware must always run, verify with either HMAC or JWKS (not both), pin accepted `alg` to the active verifier, and keep authN/authZ decoupled except for the sanctioned operator key.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-05-13T20:41:09.256Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 125
File: internal/api/health_test.go:100-163
Timestamp: 2026-05-13T20:41:09.256Z
Learning: In `internal/api/health_test.go` (WaveHouse), every handler test explicitly asserts `Content-Type: application/json` and `X-Content-Type-Options: nosniff` headers, including on 503 responses. This is deliberate regression coverage: the comment in `TestHealth_Readiness_PingFails` explains that without the 503-path header test, a future refactor moving header setup into the success branch would silently drop headers on error responses. New boot-degraded tests should follow the same pattern.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to internal/observability/**/*.{go} : Observability code must preserve the repo’s logging and telemetry invariants, including stdout fan-out, OTLP behavior, lazy dialing, and the private Prometheus registry.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to internal/stream/**/*.{go} : Streaming/SSE code must preserve the hub’s per-role projection model, subscriber queues, bucket fan-out, heartbeating, and metrics semantics.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-05-25T11:24:24.022Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 180
File: internal/cache/local.go:0-0
Timestamp: 2026-05-25T11:24:24.022Z
Learning: In the WaveHouse codebase (`internal/cache/local.go` and related packages), package-level `var` declarations of immutable `metric.MeasurementOption` / OTel attribute sets (e.g., `cacheL1Attrs = metric.WithAttributes(attribute.String("tier", "L1"))`) are intentional and acceptable. These are pre-allocated constants analogous to `regexp.MustCompile(...)`, not mutable global state. The AGENTS.md "no global state / constructor injection" rule applies to application dependencies (Cache, Publisher, Deduplicator), not to stateless OTel metric attribute options. Do not flag these as violations of the constructor-injection guideline.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-05-20T01:02:03.228Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:03.228Z
Learning: In the WaveHouse project (`internal/api/**/*_test.go`), the convention for testing `RequireRole` middleware is to inject `ContextKeyRole` directly into the request context rather than using `testutil.MakeJWT`. JWT token parsing is covered separately in `middleware_test.go` (17 dedicated tests). Do not suggest switching role-gate tests to JWT-driven tests — the separation of concerns is intentional to keep failure surfaces isolated.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-05-25T11:25:11.992Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 180
File: internal/observability/instruments.go:40-117
Timestamp: 2026-05-25T11:25:11.992Z
Learning: In the WaveHouse project (Go), package-level `var` declarations of OTel metric instruments (e.g., `metric.Float64Histogram`, `metric.Int64Counter`) created via `Meter().Float64Histogram(...)` / `Meter().Int64Counter(...)` are idiomatic and intentional — they follow the OTel Go SDK global proxy pattern and are NOT considered "global state" violations under the AGENTS.md constructor-injection rule. That rule targets swappable application-level interface dependencies (Cache, Publisher, Subscriber, Deduplicator), not OTel proxy instruments. Do not suggest wrapping these into an `Instruments` struct for injection.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-05-20T20:30:22.556Z
Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 172
File: internal/api/pipes_test.go:106-118
Timestamp: 2026-05-20T20:30:22.556Z
Learning: In WaveHouse's pipes authorization fix (PR `#172`), fixing the AllowedRoles fail-open bug requires two changes: (1) remove the outer `if role != ""` guard in PipesHandler.Execute so empty roles are evaluated against the allowlist, AND (2) add an `ar != ""` guard inside the allowlist scan (i.e., `ar != "" && ar == role`) so a malformed allowlist containing empty strings (e.g., `[""]`) cannot match an empty role via `"" == ""`. Doing only (1) is insufficient and would make `[""]` fail-open.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-06-26T15:07:28.749Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 0
File: :0-0
Timestamp: 2026-06-26T15:07:28.749Z
Learning: In the Go SSE implementation in `internal/api/stream.go`, keepalive frames from `internal/stream.Heartbeater` are only written from the post-replay select loop. The replay/gap-fill step is synchronous before entering that loop, so registering the `internal/stream.Subscriber` before replay does not materially improve idle-time coverage during replay; it can at most buffer one heartbeat in the subscriber's capacity-1 queue. Covering a genuinely long replay would require interleaving replay with the select loop and is tied to the broader delivery-path rework tracked by Issue `#294`.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
CHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to docs/src/content/docs/configuration.md : Document every new or changed configuration option in the configuration docs.
Applied to files:
docs/src/content/docs/sdk/index.mdx
📚 Learning: 2026-08-11T15:22:23.813Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.
Applied to files:
docs/src/content/docs/sdk/index.mdx
🪛 LanguageTool
docs/src/content/docs/sdk/index.mdx
[style] ~389-~389: Since ownership is already implied, this phrasing may be redundant.
Context: ...he SDK's value stands alone, and two of your own entries differing only in case collapse...
(PRP_OWN)
[style] ~518-~518: Since ownership is already implied, this phrasing may be redundant.
Context: ... you don't really control, and auditing your own code for fetch calls won't tell you: ...
(PRP_OWN)
[style] ~539-~539: Since ownership is already implied, this phrasing may be redundant.
Context: ... one underlying reason: undici declares its own request/response types, separate from t...
(PRP_OWN)
[style] ~543-~543: Consider using the typographical ellipsis character here instead.
Context: ...RequestInfonames its *own*Request; { ...init, dispatcher } as never` covers the ...
(ELLIPSIS)
[style] ~545-~545: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ither spelling, so one snippet compiles whether or not your lib includes DOM); and the retur...
(WHETHER)
🔇 Additional comments (17)
clients/ts/src/types.ts (1)
86-160: LGTM!Also applies to: 334-342, 360-366
clients/ts/src/index.ts (1)
32-46: LGTM!clients/ts/src/http.ts (1)
5-5: LGTM!Also applies to: 28-56, 62-62, 84-106
CHANGELOG.md (1)
14-14: LGTM!Also applies to: 28-28
clients/ts/src/client.ts (1)
41-42: LGTM!clients/ts/src/client.test.ts (1)
150-152: LGTM!Also applies to: 167-173, 197-199, 212-289, 291-331
docs/src/content/docs/sdk/index.mdx (5)
330-336: LGTM!
358-401: LGTM!
403-435: LGTM!
470-474: LGTM!
493-562: LGTM!clients/ts/src/pipes.ts (1)
4-4: LGTM!clients/ts/src/query-builder.ts (3)
12-12: LGTM!
140-140: LGTM!
242-242: LGTM!clients/ts/src/table.ts (2)
9-9: LGTM!
61-61: LGTM!
Closes #464. From CodeRabbit on #456. PipeRef.fetch took the same RequestOptions as the query builder, which carries `limit`, but forwarded only `signal` -- so wh.pipe('x').fetch({ limit: 10 }) compiled, ran, and quietly did nothing. QueryBuilder.fetch and TableRef.fetch both honour it, so the inconsistency sat inside one shared type. There is nothing to forward: internal/api/pipes.go binds the request body as the pipe's parameters via pipes.BindParams, so a row cap only exists if the pipe's own SQL declares one. The fix is the surface, not the plumbing -- fetch now takes Pick<RequestOptions, "signal">, and the JSDoc points at wh.pipe(name, { limit }) as the real route. A @ts-expect-error test pins the rejection so it can't quietly widen back. Pre-existing rather than introduced here, and I first filed it to keep this diff focused. That was the wrong call: this PR renames that exact type and gives it a JSDoc describing it as the options for `.fetch()`, which makes the false advertisement more prominent, not less. The new test also exposed a flaw in the suite's own fixture. fetchSpy resolved a single shared Response, and a Response body can only be read once -- so any test making two requests had the second fail and retry, and an assertion on mock.calls[1] was reading that retry rather than a second request. "adds configured headers to every request" was passing for exactly that wrong reason. The mock now builds a fresh Response per call, and that test asserts a call count of 2 so the distinction is pinned rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Both reviewers caught the same miss on the previous commit: it changed
a public method's signature and documented that only in the JSDoc.
CHANGELOG had no entry at all, which is worse than an omission here --
the existing BREAKING entry for the FetchOptions rename lists pipes.ts
in its file set and says "renaming the import is the whole migration",
so a consumer reading [Unreleased] end to end would conclude the only
pipes.ts change was cosmetic and their .fetch({ limit }) still
compiles. Added its own BREAKING entry with the migration, and
qualified the rename entry so the two don't contradict.
sdk/pipes.md documented the method as a bare `.fetch(opts?)` with opts
undefined, while sdk/queries.md documents an identically-titled
`.fetch(opts?)` with an options table including limit. Someone learning
opts on the queries page and carrying it to pipes got a type error with
nothing in the docs explaining it. The pipes page now states the
narrower type and redirects to the pipe-parameter route, and the API
tree in reference.md annotates the entry, which previously rendered
byte-identically to the two that do take limit.
Also fixed "bound a pipe with a {{limit}} parameter" in the JSDoc,
which read as the noun rather than an imperative.
No prose anywhere showed `.fetch({ limit })` on a pipe, so no example
was broken -- both docs and README already passed limit as a pipe
parameter, which is the route the narrowing pushes people toward.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 170f75a9-7b41-4e0c-81b7-4ce77900c8bf
📒 Files selected for processing (5)
CHANGELOG.mdclients/ts/src/client.test.tsclients/ts/src/pipes.tsdocs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/reference.md
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: Integration tests
- GitHub Check: E2E tests
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Docs build
- GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (2)
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Documentation prose under the Starlight docs site must stay accurate against code, include runnable examples where relevant, and reflect code↔docs sync for changed behavior.
Files:
docs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/reference.md
clients/ts/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
The TypeScript SDK in
clients/ts/is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Files:
clients/ts/src/pipes.tsclients/ts/src/client.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/client.test.ts:0-0
Timestamp: 2026-08-12T20:34:04.763Z
Learning: In the TypeScript SDK, `clients/ts/src/types.ts` defines `FetchLike` as `(input: string | URL | Request, init?: RequestInit) => Promise<Response>`. This explicit standard fetch-compatible signature avoids `typeof fetch` differences when DOM library types are absent and allows fetch wrappers to be shared with SDKs that use the standard wide input type.
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:28-28
Timestamp: 2026-08-12T20:33:30.744Z
Learning: In the TypeScript SDK, `PipeRef.fetch` in `clients/ts/src/pipes.ts` accepts only a signal option. Pipe row limits are not generic request options. A pipe SQL definition can declare a `{{limit}}` parameter, and callers provide that parameter through `wh.pipe(name, { limit })`. The API binds the pipe request body as pipe parameters through `pipes.BindParams` in `internal/api/pipes.go`.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
📚 Learning: 2026-08-12T20:33:30.744Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:28-28
Timestamp: 2026-08-12T20:33:30.744Z
Learning: In the TypeScript SDK, `PipeRef.fetch` in `clients/ts/src/pipes.ts` accepts only a signal option. Pipe row limits are not generic request options. A pipe SQL definition can declare a `{{limit}}` parameter, and callers provide that parameter through `wh.pipe(name, { limit })`. The API binds the pipe request body as pipe parameters through `pipes.BindParams` in `internal/api/pipes.go`.
Applied to files:
docs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/reference.mdclients/ts/src/pipes.tsclients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-06-29T14:21:45.067Z
Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 343
File: internal/api/pipe_deps.go:0-0
Timestamp: 2026-06-29T14:21:45.067Z
Learning: In `internal/api/pipes.go`, direct table-function reads and direct cross-database table reads are intentionally omitted from the pipe dependency set and continue using the normal query-derived TTL; only resolved-but-unmaintainable dependencies (such as unknown or unfoldable view-derived names) trigger the unresolved-dependency TTL cap.
Applied to files:
docs/src/content/docs/sdk/pipes.mdclients/ts/src/pipes.tsclients/ts/src/client.test.ts
📚 Learning: 2026-06-10T15:01:09.027Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.
Applied to files:
docs/src/content/docs/sdk/pipes.mddocs/src/content/docs/sdk/reference.mdCHANGELOG.md
📚 Learning: 2026-08-12T20:34:04.763Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/client.test.ts:0-0
Timestamp: 2026-08-12T20:34:04.763Z
Learning: In the TypeScript SDK, `clients/ts/src/types.ts` defines `FetchLike` as `(input: string | URL | Request, init?: RequestInit) => Promise<Response>`. This explicit standard fetch-compatible signature avoids `typeof fetch` differences when DOM library types are absent and allows fetch wrappers to be shared with SDKs that use the standard wide input type.
Applied to files:
clients/ts/src/pipes.tsclients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-08-11T21:56:06.521Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: docs/src/content/docs/sdk/go/queries.md:356-362
Timestamp: 2026-08-11T21:56:06.521Z
Learning: In Wave-RF/WaveHouse Go SDK cursor pagination, `fetchNextTyped` uses only the first `QueryBuilder.OrderBy` column and a strict `gt` or `lt` filter. Duplicate values at a page boundary can skip rows. The Go SDK documentation must require a unique ordering column until the shared Go and TypeScript composite-cursor or tie-breaker implementation tracked in GitHub issue `#452` is available.
Applied to files:
clients/ts/src/pipes.tsCHANGELOG.md
📚 Learning: 2026-08-11T21:56:03.206Z
Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/query_builder.go:278-291
Timestamp: 2026-08-11T21:56:03.206Z
Learning: In `clients/go/query_builder.go`, `fetchNextTyped` intentionally treats a failed JSON decode of a non-object typed `Row` as normal end-of-pagination. This behavior matches the existing “cursor column was not in the projection” path and TypeScript SDK parity. The broader behavior change is tracked in GitHub issue `#452`.
Applied to files:
clients/ts/src/pipes.tsclients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-05-20T20:30:22.556Z
Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 172
File: internal/api/pipes_test.go:106-118
Timestamp: 2026-05-20T20:30:22.556Z
Learning: In WaveHouse's pipes authorization fix (PR `#172`), fixing the AllowedRoles fail-open bug requires two changes: (1) remove the outer `if role != ""` guard in PipesHandler.Execute so empty roles are evaluated against the allowlist, AND (2) add an `ar != ""` guard inside the allowlist scan (i.e., `ar != "" && ar == role`) so a malformed allowlist containing empty strings (e.g., `[""]`) cannot match an empty role via `"" == ""`. Doing only (1) is insufficient and would make `[""]` fail-open.
Applied to files:
clients/ts/src/pipes.ts
📚 Learning: 2026-08-11T15:22:20.507Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.
Applied to files:
clients/ts/src/pipes.tsclients/ts/src/client.test.ts
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/**/*.{ts,tsx} : The TypeScript SDK in `clients/ts/` is the canonical client; when backend API changes affect users, update the SDK surface, auth handling, query builder, streaming helpers, pipes/policy helpers, or regenerated types as needed.
Applied to files:
clients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-07-07T16:19:29.374Z
Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-07-07T16:19:29.374Z
Learning: Applies to clients/ts/README.md : Keep the TypeScript SDK README in sync with SDK-facing changes and public client behavior.
Applied to files:
clients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-08-12T05:38:52.277Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: tests/e2e/sdk/helpers.ts:214-221
Timestamp: 2026-08-12T05:38:52.277Z
Learning: In `tests/e2e/sdk/helpers.ts`, `chQuery` must reclassify a caught error as a request timeout or caller abort only when the error is an abort error. On Node 22, `AbortSignal.timeout()` produces an error named `TimeoutError`, `AbortController.abort()` and caller cancellation through `AbortSignal.any()` produce `AbortError`, and `JSON.parse()` failures produce `SyntaxError`.
Applied to files:
clients/ts/src/client.test.tsCHANGELOG.md
📚 Learning: 2026-05-19T14:41:38.228Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 142
File: docs/scripts/screenshot.mjs:31-31
Timestamp: 2026-05-19T14:41:38.228Z
Learning: `docs/scripts/screenshot.mjs` in the Wave-RF/WaveHouse repo is an intentionally manual dev-iteration tool (not wired into CI). Response-status validation and retry logic are deliberately deferred until the script is promoted to a CI visual-regression workflow. Do not flag the absence of `response.ok()` checks as an issue in this file.
Applied to files:
clients/ts/src/client.test.ts
📚 Learning: 2026-08-12T05:35:22.804Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: clients/ts/README.md:11-11
Timestamp: 2026-08-12T05:35:22.804Z
Learning: For Wave-RF/WaveHouse, CI currently tests Node 22 only through `.nvmrc` and `.github/actions/setup-env` using `node-version-file`. Local `make ci` verification on Node 26 is not CI coverage and must not be documented as continuously tested runtime support.
Applied to files:
clients/ts/src/client.test.ts
📚 Learning: 2026-08-12T15:28:20.891Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: docs/src/content/docs/sdk/index.mdx:0-0
Timestamp: 2026-08-12T15:28:20.891Z
Learning: For `docs/src/content/docs/sdk/index.mdx`, the documented workaround for the undici idle-event-loop keep-alive stall is to upgrade to undici 8.10.0 or later. If a consumer is pinned to an affected version, `new Agent({ pipelining: 0 })` must be merged as `dispatcher` into the SDK-provided `RequestInit`; this disables keep-alive reuse. Configuring `keepAliveTimeout` does not mitigate this stall because the socket retirement timer is starved by the same idle event loop.
Applied to files:
CHANGELOG.md
🔇 Additional comments (9)
CHANGELOG.md (1)
28-30: LGTM!docs/src/content/docs/sdk/pipes.md (1)
24-30: LGTM!docs/src/content/docs/sdk/reference.md (1)
66-66: LGTM!clients/ts/src/client.test.ts (5)
9-19: LGTM!
122-132: LGTM!
163-226: LGTM!
228-307: LGTM!
309-349: LGTM!clients/ts/src/pipes.ts (1)
4-4: LGTM!
From CodeRabbit on #456. The previous fix was half a fix. Pick<RequestOptions, "signal"> only rejects a fresh object literal. TypeScript's excess-property check does not apply to a variable, so: const opts: RequestOptions = { signal, limit: 10 }; wh.pipe('x').fetch(opts); // compiled, limit silently dropped Verified with tsc: the literal form errored while the named form passed. That leaves the original defect intact for anyone who builds options once and shares them, which is the more realistic shape in real code than an inline literal. Replaced with an exported PipeRequestOptions declaring signal and limit?: never. `never` rather than omitting the property, because omission is exactly what leaves the named case open. Both forms are now pinned by @ts-expect-error tests. The tradeoff, measured rather than assumed: a RequestOptions value now fails to assign even when it carries no limit at runtime, since the declared type still permits one. That is correct -- the compiler cannot know the value is limit-free, and the two option sets genuinely differ -- but it is a real ergonomic cost for a caller sharing one options object across query and pipe fetches, so it is worth stating plainly. Note this reverses a judgement from an earlier review round, which read Pick<>'s permissiveness as a compatibility feature for the wrapper case. It is the same behaviour; the disagreement is whether admitting a silently-ignored limit is forgiving or defective. Given the whole point of #464 is that the type advertised something it ignored, defective. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Both reviewers caught the same under-description. The docs and CHANGELOG
framed the new compile error as conditional on passing a limit. It is
not: limit?: never makes any value *declared* RequestOptions
unassignable, limit or no limit, because assignability is decided on the
declared type. Verified -- a limit-free `const o: RequestOptions =
{ signal }` fails with "Type 'number | undefined' is not assignable to
type 'undefined'", which reads like an SDK bug when the value plainly
has no limit.
That is the error consumers will meet, so it now leads with the way
out. Verified both halves of the advice rather than asserting them:
a PipeRequestOptions value is accepted by the pipe, table AND
query-builder .fetch(), so it works as a single shared type; and an
un-annotated `const o = { signal }` passes by inference.
Also documented a reassuring property I had not checked, from the code
reviewer: method parameters compare bivariantly, so PipeRef still
satisfies a structural `interface Fetchable { fetch(opts?:
RequestOptions) }`. Verified. Only direct argument passing breaks, which
shrinks the blast radius considerably and is worth saying, since a
reader otherwise has to guess.
Separately: RequestOptions had become a term of art in the docs without
being named anywhere it is defined -- queries.md documented the query
builder's options as an untitled table -- so the pipes page contrasted
against a name the linked page never used. queries.md now names the
type at its home and points at the pipes variant.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM
Completes the previous commit, which fixed the escaping DOMException but left the classification keyed off the error object — so the answer depended on maxRetries. An implementation throwing something other than a DOMException named AbortError (AbortSignal.timeout raises a TimeoutError, node-fetch its own class) fell through to NETWORK_ERROR at the fetch site, and was only reclassified as ABORTED if a retry remained for the backoff sleep to notice the signal. Measured: same abort, NETWORK_ERROR at maxRetries=0 and ABORTED at 2. The outer check now also consults opts.signal?.aborted, which collapses every case to ABORTED and makes the rule simpler to state: the SDK reads the signal, not your error type. Pinned at both retry settings. Three public docs asserted the opposite, all written or edited by this branch — the FetchLike TSDoc that ships in dist/index.d.ts, the supplying-your-own-fetch section, and the abort clause of the #456 CHANGELOG entry. All corrected. Adds the ### Fixed CHANGELOG entry the abort fix needed: it is a user-visible break of the SDK's never-throws contract on the REST path, reachable with the default maxRetries and any AbortSignal, so it clears the "any notable change" bar rather than riding along unmentioned. Docs: anchors the closing SSE_AUTH_ERROR paragraph to the rule it actually excepts — caller-side failures are terminal — instead of claiming exception status without naming one.
…cs say The previous commit added `opts.signal?.aborted` but kept the error-type check beside it, so the docs' new rule — "decided from the signal, not the rejection's type" — was only half implemented. The surviving clause was reachable in exactly one situation: an `options.fetch` enforcing its own per-attempt deadline, which aborts an internal controller and rejects with a genuine AbortError while the caller's signal is untouched. That returned a terminal ABORTED where the docs promise a retried NETWORK_ERROR — and the docs are right on the merits. Nobody asked to cancel; a middleware timeout is transient. So the clause is gone rather than documented, which also makes the sentence true as written. The inner catch around the backoff sleep now uses the same predicate, so the two classifications fifteen lines apart cannot drift. No existing test covered the dropped path — every abort test passes an already-aborted signal — so a new one pins it: a middleware AbortError with no caller signal retries and succeeds. Mutation-verified. CHANGELOG: the #456 entry still claimed `FetchLike` "matches the standard `fetch` signature", which this branch's own BREAKING entry three items below negates — the URL parameter narrows to `string`, and the standard first parameter is `RequestInfo | URL`. Third stale clause reconciled in that paragraph; the other two were the REST-only carve-out and the abort rule.
## Summary Moves `.stream()` / `.liveQuery()` off `EventSource` and onto `fetch`, so the JWT travels as `Authorization: Bearer` instead of `?token=` in the request URI — where every proxy, CDN, and load balancer in front of WaveHouse is free to log it. **Server untouched.** `bearerToken()` has always preferred the header, and the CORS preflight has allow-listed `Authorization` + `Last-Event-ID` since #215 — `internal/api/router_test.go:352` is an existing test naming #203 as its reason. `?token=` stays accepted for clients that genuinely can't set headers. **Advances #203; does not close it.** Tasks 1 and 2 of 3 are done (header auth, cURL flow). The third — retiring `?token=` server-side — is #468, and closing that closes this. **This does not make streams authenticated, and (regarding #203) does not close it.** `/v1/stream` stays ungated: an expired or missing token still resolves to `default_role` and gets a filtered `200`, never a `401`. What changes is that the JWT stops appearing in request URIs, and therefore in proxy, CDN, and load-balancer logs. Enforcing expiry on a live stream is #239 and is deliberately not delivered here. Part of #194. ## Decisions worth your eyes **1. Redirects are refused only when the request carries a credential.** Platforms strip `Authorization` on a cross-origin redirect ([whatwg/fetch#1544](whatwg/fetch#1544) — the mitigation for the class behind [CVE-2022-1650](GHSA-6h5x-7c5m-7cr7), which was this exact bug in the `eventsource` package) while forwarding other headers intact. A credentialed hop therefore either silently downgrades the stream to `default_role` — this endpoint answers an unauthenticated caller rather than rejecting them — or hands a configured proxy secret to whatever the redirect names. Refusing costs nothing, because following would never have produced an *authenticated* stream anyway. With no credential there's nothing to protect, so redirects are followed and CDN canonicalization, geo/LB indirection, and http→https upgrades all work. `options.fetch` overrides `redirect` if you need the credentialed case followed regardless. `manual` rather than `error`: `error` rejects with a bare `TypeError` indistinguishable from a connection failure, which the reconnect loop would retry forever against a redirect that will never stop happening. **2. `FetchLike`'s URL parameter narrows to `string`.** The design note on #269 said the type was narrow so hand-written `(url: string, init?) => Promise<Response>` middleware would assign. What shipped was `string | URL | Request`, which — parameters being contravariant — rejects exactly that. Purely additive for implementers; only code that *imported* `FetchLike` and called through it with a `URL` breaks. **3. The SDK gains its first runtime dependency: `eventsource-parser@^3.1.0`.** MIT, **zero transitive deps**, npm provenance-attested (SLSA v1), no install scripts, 61.7M downloads/week, same maintainer as the canonical `eventsource`. Dual CJS/ESM, so it composes with our `dist/index.cjs`. Measured cost in the CDN IIFE bundle: 3402 B minified, 1430 B gzipped. A caret rather than an exact pin because pinning in a *published library* duplicates the package in any consumer tree already resolving a 3.x and freezes them out of patch/security releases until we cut one — our lockfile still governs CI. The range stops below the ESM-only 4.0.0. Why rent rather than hand-roll: the correct parser is ~440 lines, and the parts that matter — reassembling a frame split across chunk boundaries without quadratic recopying, disambiguating a trailing `\r` at a chunk boundary between a bare-CR terminator and half a split `\r\n`, and capping buffered input against a hostile stream — are what a naive version gets wrong, and are unavoidable even though we control the server, since HTTP/2 and intermediaries re-chunk freely. The buffer cap is set explicitly to 16 MiB; the parser defaults to unbounded. **This retires the "zero-dependency" claim** everywhere it appeared — both READMEs, AGENTS.md invariant 14, four site pages, and an older `Unreleased` CHANGELOG entry that would otherwise have shipped in the same release notes as the entry introducing the dependency. The only surviving mentions are in released CHANGELOG history, which is a record rather than a claim. ## What this fixes beyond the headline - **Expired-token silent downgrade.** `auth()` was called once and baked into the URL; `EventSource` then reconnected forever with that token. Once expired the stream stayed open serving a reduced view. This is why #203 is a **prerequisite for #239**. - **Blank `id:` clearing resumption.** The hub emits `id: ` for passthrough payloads; per spec an empty `id` *clears* the last-event-id. The transport retains the last **non-empty** id. - **Real errors.** A rejection carries its actual status and message instead of `EventSource`'s status-free `onerror` — which, in the old transport, meant a gateway `401` surfaced as a silent `closed` with no `error` callback at all. Note the limit: a browser stream going cross-origin only sees the status if the rejection passes CORS and the gateway answered the `Authorization` preflight; otherwise it degrades to a retryable network error. - **Non-SSE `200`s refused** (`SSE_BAD_CONTENT_TYPE`). An auth gateway's login page would otherwise feed HTML to the parser, which per the SSE grammar parses to *nothing* — leaving the stream live and permanently silent. - **No Node polyfill.** `polyfills.ts` and the `eventsource` devDependency are deleted. - **`options.fetch` / `headers` / `fetchOptions` reach streams**, closing the carve-out documented in #456. ## Behavior changes to be aware of - **A credentialed cross-origin browser stream now preflights on the initial connect.** `EventSource` never preflighted at all (its request isn't a `fetch()`, so Fetch's unsafe-request flag is never set). A proxy that answers CORS itself must allow `Authorization` on `OPTIONS /v1/stream` or the stream never opens. Documented in `reverse-proxy.mdx`. - **A proxy that strips `Authorization` on `/v1/stream`, or redirects it while credentialed, now breaks.** Both previously "worked" by accident. - **Resumption is at-least-once and time-bounded** — this was always true; the docs now say so. The last event you saw is *certainly* redelivered (the id is a `received_timestamp` and replay is inclusive), the SDK does not dedupe live frames, and replay is capped by `mq.gap_window_minutes` (15 default). ## Testing `sse.test.ts` is rewritten against an injected fetch returning a scripted `ReadableStream`. The old harness stubbed a global `FakeEventSource` and could only assert on URL strings — framing, reconnect, and resumption had **no coverage at all**. 204 tests now, with 14 behavioral fixes mutation-verified: reverting each one makes a test fail, and — after review caught a case where it didn't — fail on the assertion that names it. The e2e auth test was rewritten to be discriminating: `anon` is denied `payload` on the events table, so dropping the `Authorization` header flips the assertion. The previous version could not fail. ## Beyond the nominal scope Two REST-path fixes ride along, both surfaced by review of the streaming work and both in `clients/ts/src/http.ts`. Flagging them because an SSE PR is not where you would look for them, and either can be split out on request. - **A cancelled request could throw instead of returning `ABORTED`.** The network-error backoff is the one `sleep` inside `request()`'s catch, so its rejection had no handler and escaped as a raw `DOMException`. Nothing wraps `request()`, so it reached callers as an unhandled rejection — and the `AbortController` example in our own reference demonstrated a branch that could not be taken against an unreachable server. - **Abort is now classified from the signal, not the rejection's type.** Keying off the error made the outcome depend on `maxRetries`: `AbortSignal.timeout()` raises a `TimeoutError`, so it reported `NETWORK_ERROR` at `maxRetries: 0` and `ABORTED` at `2`. It also mis-handled middleware — an `options.fetch` enforcing its own per-attempt deadline aborts an internal controller while the caller never cancelled, which is transient and should be retried, not reported as a terminal `ABORTED`. The same rule then had to be applied to the stream transport, where the old error-type check was worse: an `AbortError` from `auth()` or a custom `fetch` ended the stream terminally and emitted **nothing**. ## Review Fifteen pre-push rounds against both gating reviewers, who verify by executing the code rather than reading it. Worth knowing what they caught, since none of it was reachable by CI: | | | |---|---| | Behavior bugs | `SSE_CONNECT_ERROR` never reaching a subscriber; a closed stream stuck reporting `live`; a consumed-body guard that didn't guard; an unhandled rejection that killed the host process; a stranded reconnect timer | | Regression vs `EventSource` | `close()` from inside a handler no longer stopped delivery | | Coverage holes | deleting the bearer half of the credentialed-redirect rule left the suite green | | False claims in docs | `EventSource` "preflighted on reconnect" (it never preflights); "WaveHouse does not reject a stream" (it 400s on a missing table); "the SDK isolates a throwing handler" (true only inside the transport — several paths outside it are not, now enumerated in the SDK reference and filed as #473) | Two recurring shapes, both worth knowing before you read the diff. **In the code: a guard or cleanup applied to N−1 of N call sites.** Six defects shared it. `if (this._closed) return` now appears eight times in `sse.ts`, several added a round apart. Assume any new early-exit path in this transport is the one that got missed. **In the prose: a true mechanism attached to a wider case set than it holds for.** This accounts for essentially every documentation defect found here, and it recurred for eight consecutive rounds — three times *inside the sentence written to fix the previous instance*. The reviewers' diagnosis is the useful part: none of these were factual errors about the system, they were missing quantifiers. Nearly every claim in this area is a function of a variable the docs cannot name — the reader's token-provider latency, which origin, which credentials mode — so the domain lives only in the author's head at the moment of writing, and the next revision reaches for the deepest true mechanism and silently re-attaches it to the whole case set. The empirical tell was sharp: the rule-shaped sentences never needed correcting; the value-shaped ones were corrected every round. The Live Queries failure section is written to that conclusion — it states rules and gives the reader a test, rather than reporting which outcome is typical. Two amplifiers were also removed: sentences that counted table rows (a ninth row would have silently falsified five of them) and facts restated independently in four or five files. **If you are reviewing prose here, the question that finds bugs is "for which cases is this true?", not "is this true?"** Four axes account for essentially every defect found on this branch, and a claim that is silent about which side it means is the shape to distrust: 1. **Who rejected it** — WaveHouse (a `400` on the stream route; a `404`/`405` off it) versus something in front. Never "the server said 401": `/v1/stream` is ungated. 2. **Where the caller runs** — server-side or same-origin (statuses visible) versus browser cross-origin, where CORS can make any rejection, including a rejected preflight, indistinguishable from a network drop. 3. **Whether the request carries a credential** — decides `redirect: "manual"` versus `"follow"`, and the test is `Authorization`-or-`headers`, so cookies are *not* credentials by it (#478). 4. **Whether the failure is semantically transient** — the 4xx-is-terminal rule is a transport mechanism, not a claim about the world (#469). One structural note so it isn't rediscovered: `CHANGELOG.md` is denylisted from the docs-prose gate (`scripts/docs-prose.sh:38`), so that entry has never been read by the automated docs reviewer. It is worth reading at docs scrutiny rather than skimming as boilerplate — a false claim survived three rounds there for exactly that reason. **The Go diff is one comment, so this looks deployment-free. It isn't.** A credentialed cross-origin browser stream now preflights where `EventSource` never did, so a proxy that answers CORS itself must allow `Authorization` on `OPTIONS /v1/stream` or streams stop opening — silently, in a retry loop, not with a visible error. That break is documented in `reverse-proxy.mdx`; a reviewer reading only `internal/` will conclude nothing operational changed. ## Follow-ups filed Design work deferred out of this PR: - **#465** — gzip on `/v1/stream` (per-frame flush; measure before adopting) - **#466** — normalize a schemeless `baseURL`, with a loopback exception - **#467** — binary framing negotiated via `Accept`, sequenced behind #465 - **#468** — retire `?token=` server-side; closes #203 - **#204** — commented with the POST-body analysis that unblocks multiplexing Defects found by review of this branch and left unfixed here, each because the fix lands outside the transport or carries a design question I didn't want to answer unilaterally in a PR about auth: - **#469** — a stream `429` is terminal; should honor `Retry-After` - **#471** — a rejected resumption preflight leaves a stream re-dialing forever - **#473** — a throwing subscriber silently stops delivery to the others. Widened during review to cover every path outside the transport's guard — `.subscribe()`'s initial unguarded `status` call (worse via `liveQuery()`, which returns no handle at all), the fan-out dropping the event for a concurrent `for await` and leaving it un-terminated on a terminal close, a throwing `status` handler making `.connected()` time out against a live stream, and `liveQuery()`'s backfill flush discarding its buffer. The docs and CHANGELOG now describe the real contract rather than the one I first wrote. - **#476** — `http.ts`'s `sleep()` leaks an abort listener when the timer wins - **#484** — transport hardening against a non-conforming peer, raised as "what I'd watch" in the final review: a parser-buffer overflow re-dials at a flat rate forever because the backoff reset counts the overflowing connection as healthy, and `FetchLike`'s contract never states that `init.signal` must be honored. Neither is reachable with a conforming peer; both are cheap now. - **#477** — `StreamController` retains every event when nothing iterates. The buffer's only drain is the async iterator's `next()`, so a `.subscribe()`-only consumer — the pattern the docs lead with — holds every event it has ever received, unbounded. Doubled on a filtered or live stream, since both controller layers buffer. - **#478** — a cookie-authenticated stream bypasses the redirect guard. `credentialed` tests for a bearer token or configured `headers`; cookies are neither, so the request follows a redirect and can arrive unauthenticated. - **#449** — pre-existing liveQuery dedup boundary, re-confirmed by review - **#445** — not touched here, but surfaced again while reviewing the streaming docs and worth a person's eye: `wh.pipe(name).stream()` is documented as working in three places while `pipes.ts` streams `?table=<pipeName>`, so it subscribes to a topic nothing publishes and silently yields nothing. The DLQ variant carries an inline caveat for the same class of gap; the pipe one doesn't. ## Reviewer notes The interesting file is `clients/ts/src/stream/sse.ts`. `controller.ts` is untouched — the transport sits behind the same `StreamTransport` interface — so the diff is scoped to the transport, its tests, and the docs the change invalidated. `clients/ts/src/stream/live-query.test.ts` is new and is the first test coverage `LiveQuery` has had. It pins one thing worth knowing about: the backfill, not the stream, spends the first `auth()` call, and that ordering is emergent from four independent details rather than declared anywhere. One added `await` on the REST path silently swaps the two failure modes the docs describe, so the test exists to make that a red build rather than a documentation drift. Merged with `main` at `1064a4fe`, which brought #381 (per-subscriber SSE row filtering) and #457. Both conflicted textually with this branch and both were resolved keeping each side; #381 adds no new status code, so this PR's claim that `/v1/stream` raises exactly one 4xx itself still holds. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three catch-up changes, all client-side. The server is untouched. Routes: main merged every admin-gated endpoint under /v1/ops (#479) with no aliases, so thirteen call sites were 404ing against a current server — schema list/refresh, DLQ stats, raw SQL, policy get/put/validate, and pipes CRUD, plus the codegen CLI's schema fetch. Rewrote them along with the tests, the shared wire_cases.json fixture, and the Go SDK docs. The fixture is replayed by both conformance runners, so the stale paths broke the TypeScript half too; `make test-conformance-ts` is back to 45/45. ClientOptions.Headers: the TypeScript SDK gained options.headers in #456 and Go had no equivalent. Headers now apply to every request the client makes, REST and SSE alike — which is also how an operator sends the server's non-JWT X-Operator-Key. The SDK's own headers are set afterwards and win a collision; net/http canonicalizes names, so matching is case-insensitive; the map is copied at construction so later mutation can't reach into requests. SSE robustness, mirroring main's fetch-based rewrite (#470). The Go SDK already authenticated by header, so that part was never stale, but three gaps were: - A credentialed stream followed redirects. net/http drops Authorization on a cross-host hop while forwarding custom headers verbatim, so a redirect either downgraded the stream to default_role in silence or handed configured secrets to wherever it pointed. Now refused with a terminal SSE_REDIRECT. Uncredentialed streams still follow. - A 200 with any content type was treated as an event stream, so an auth gateway's login page left the stream sitting in StatusLive delivering nothing. Now a terminal SSE_BAD_CONTENT_TYPE. - Every failure collapsed into one retryable SSE_ERROR, and malformed frames came back as a bare fmt.Errorf, so errors.As and IsRetryable didn't work on them. Replaced with the taxonomy the TypeScript SDK uses — SSE_AUTH_ERROR, SSE_NETWORK_ERROR, SSE_CONNECT_ERROR, SSE_REDIRECT, SSE_BAD_CONTENT_TYPE, SSE_PARSE_ERROR, SSE_READ_ERROR — each with its own retryable flag, all delivered as *Error. Also documents what main changed underneath the Go SDK without changing its code: DateTime values arrive canonicalized to RFC 3339 UTC (#402), SSE applies policy row-filters per subscriber and fails closed (#381, #457), /v1/stream is ungated so WaveHouse never 401s a stream, and /v1/ops/dlq/stats is absent (404) when the DLQ is disabled rather than returning empty stats. Tests: terminal-failure table (bad content type, missing content type, credentialed redirect, non-HTTP scheme), redirect-followed-when- uncredentialed, typed retryable parse errors, and header precedence and copying on both transports.
Closes #269. Closes #464. Adds the SDK's HTTP-customization surface —
options.headers,options.fetchOptions, andoptions.fetch— so a WaveHouse behind a gate is reachable from the client.Why
ClientConfigexposed onlybaseURL,auth, andoptions.maxRetries. A WaveHouse fronted by a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) simply couldn't be talked to — a defense-in-depth gate forced consumers off the SDK entirely. Found via WaveHouse-Stats dogfooding.The second driver is a runtime bug consumers can't fix from inside the SDK: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused on an idle event loop (nodejs/undici#5600, fixed in 8.10.0), and Node 26 bundles 8.9.0. Severity varies with runtime and idle gap — upstream measured ~450–465ms; on Node 26.7.0 with 6s gaps against a stub server answering instantly:
What landed
options.headersoptions.fetchOptionsRequestInitmerged in —cache,keepalive,credentials, Next.jsnext: { tags }options.fetchShaped after Supabase (
global.fetch/global.headers) and OpenAI/Anthropic (fetch/fetchOptions/defaultHeaders) rather than invented here.Header precedence, lowest to highest:
options.headers→auth→ SDK-computed (Content-Type,Accept). Names match case-insensitively, and a collision drops the configured value rather than joining it. Both rules come from bugs other SDKs shipped: a globalContent-Typejoined with an upload's own producedapplication/json, image/pngand 415s, and a case-sensitiveAuthorizationcheck let a lowercase spelling ride alongside the canonical one. Two configured spellings of one header collapse to the last, soHeaderscan't comma-join them on the wire.fetchOptionscan't corrupt the request —method,headers,body, andsignalare applied after the spread.fetchOptions.headersis ignored rather than merged;options.headersis the header channel, and merging both would give one concept two precedence stories and a side door aroundauth.The undici workaround took three tries to get right
Worth reading before you follow it elsewhere, because the obvious forms don't work. Measured on undici 8.9.0, 10ms server, 1.5s idle gaps (per-request ms):
Lowering
keepAliveTimeoutdoes nothing — the retirement timer is starved by the same idle event loop that causes the bug.Worse, importing a fixed undici isn't enough on its own. undici keeps its connection pool on a shared
globalThissymbol claimed by whichever copy loads first — the bundled one, on an affected runtime. With 8.9.0 loaded first and 8.10.0'sfetchdoing the work:So the documented snippet passes
dispatcherexplicitly. Verified end-to-end through the SDK with 8.9.0 loaded first:24, 15, 13, 14ms.Breaking changes
PipeRef.fetchno longer acceptslimit— closes #464, raised by CodeRabbit here. It took the shared per-call options type, which carrieslimit, but forwarded onlysignal, sowh.pipe('x').fetch({ limit: 10 })compiled and silently did nothing. Nothing to forward: the endpoint binds the body as the pipe's parameters (internal/api/pipes.go→pipes.BindParams), so a row cap belongs in the pipe's SQL as{{limit}}, passed viawh.pipe(name, { limit })— which is what the docs already showed. Now a dedicated exportedPipeRequestOptions—signal?: AbortSignal; limit?: never.The
neveris load-bearing. The first attempt usedPick<RequestOptions, "signal">, which CodeRabbit correctly flagged as only half a fix: TypeScript's excess-property check is a freshness heuristic, so it rejects a fresh literal but not a variable.const opts: RequestOptions = { signal, limit: 10 }; wh.pipe('x').fetch(opts)still compiled and still dropped the limit — the original defect, in the shape real code is more likely to take. Both forms are now pinned by@ts-expect-errortests.Collateral effect, and the half you'll actually hit: a value declared
RequestOptionsno longer assigns to a pipe.fetch()at all, even carrying no limit at runtime, since the declared type permits one. Type a shared options object asPipeRequestOptions— the table and query-builder.fetch()accept it too, so it works everywhere — or inline{ signal }. Structural wrappers are unaffected: method parameters compare bivariantly, sointerface Fetchable { fetch(opts?: RequestOptions): … }is still satisfied byPipeRef(verified).Pre-existing on main; folded in here because this PR renames that exact type and gives it a JSDoc describing it as the options for
.fetch(), which made the false advertisement more prominent rather than less.FetchOptions→RequestOptions(the per-call type accepted by.fetch()), no deprecated alias. The old name collided conceptually with the newoptions.fetchOptions, which — per the ecosystem — means "extraRequestInit", not "options for our.fetch()method". Nothing consumes it pre-1.0; renaming the import is the whole migration for this one — thePipeRef.fetchnarrowing above is a separate, behavioural break in the same file. The module-privateRequestOptionsinhttp.tsbecameRequestSpecto free the name.Design notes
fetchstays optional all the way to the internalHttpContextrather than being resolved at construction, so the default path calls the global directly. That keeps it late-bound (replacingglobalThis.fetchafter a client exists still works — whatvi.stubGlobaldoes) and avoids invoking a detachedfetchreference, which throws "Illegal invocation" on browsers, workerd, and Bun. Both are pinned by tests. Supabase'sresolveFetchindependently converges on the same closure-per-call shape; the SDKs that capture at construction are the ones with stale-fetch bug reports.FetchLikeis the standardfetchsignature, written out rather than astypeof fetchbecause that resolves differently depending on whether the consumer'slibincludes DOM.Scope
REST only.
.stream()and.liveQuery()'s live connection go throughEventSource, which accepts neither headers nor afetch— so a header-gated deployment can query but not stream until #203 changes that transport. Called out in a:::cautionon the docs page rather than a footnote: the equivalent gap in Supabase's realtime client was found by a user whose RLS policies silently stopped matching..liveQuery()'s initial backfill is an ordinary REST call and is covered.Browser callers should know custom headers must also pass CORS preflight, and WaveHouse allow-lists a fixed set with no config knob — documented.
Tests
158 pass, 1 skipped across the package (30 in
client.test.ts), covering: routing through a supplied fetch with the fullRequestInitasserted; global fallback; late-binding after construction; retries using the override; headers applied to every request;Content-Typenot displaceable;authbeating a lowercaseauthorization; two casings collapsing to one;fetchOptionsmerged;fetchOptionsunable to touchmethod/body/headers; and two@ts-expect-errorpins thatPipeRef.fetchrejectslimit, as a literal and via a namedRequestOptionsvalue.Beyond unit tests, every documented snippet was compiled against the built package under DOM-inclusive and Node-only
libconfigs, and the header behaviour was verified on the wire against a live server — the CF-Access header sent,authbeating an impostor,Content-Typeintact. That last step matters here: compile-plus-happy-path is exactly the verification that let the implied-dispatcher bug through twice.Related
headers/fetchOptions/fetchand dynamic header callbacks, deliberately deferredauth()per-request contract, split outconnection: close, right for a test helper, wrong for a production client — hence this escape hatch)