Skip to content

docs: v1 stabilization batch#238

Open
pi0x wants to merge 1 commit into
mainfrom
docs/v1-stabilization
Open

docs: v1 stabilization batch#238
pi0x wants to merge 1 commit into
mainfrom
docs/v1-stabilization

Conversation

@pi0x

@pi0x pi0x commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Docs scope of the v1 stabilization plan. Every technical claim was verified against the code. Does not touch trustProxy docs (PR #229), the ### error / gracefulShutdown rewrites (PR #230), @experimental code removals (PR #230), or 9.aws-lambda.md (AWS agent, F23).

T1 — docs promising APIs that don't exist

  • F54 (delete): removed the server.addr and server.port sections from 3.server.md (neither exists; only server.url) plus the stale #port-required anchor.

T2 — wrong / missing docs

  • F54 wrong docs: 2.handler.md req.pathreq.url; 3.server.md "underlying Bun server instance when running in Deno" → Deno; 5.options.md maxHeadersizemaxHeaderSize.
  • F54 missing options: documented manual, middleware, plugins, serviceWorker in 5.options.md (left error/gracefulShutdown to feat: v1 api freeze #230).
  • F54 subpaths: table for srvx/static, srvx/log, srvx/loader, srvx/cli, srvx/generic, srvx/service-worker.
  • F14: documented hostname ← HOST and silent ← TEST env defaults; fixed the types.ts "server with listen" JSDoc typo.
  • Support matrix: close(true) ignored on Deno, trustProxy adapter scope, maxRequestBodySize (Bun 128 MiB / dropped in loader mode — F57), manual no-op on module workers, Cloudflare env only in module syntax (F29), and no upgrade/WebSocket on any runtime → crossws (locked).
  • srvx/static is Node-API-only (node:fs/node:zlib) — noted in 10.cli.md and the subpaths table.
  • Deno range: documented Deno 2.x (CI 2.7.x) and the mandatory explicit srvx/bunny import (root resolves the deno condition).

T3

  • F53 (D7): _request / _url documented as supported API frozen at v1 (underscore is legacy naming).
  • F11 (D5): toFetchHandler / fetchNodeHandler documented stable; srvx/tracing documented experimental — plus a new Semver policy section (also states F55: ERR_BODY_TOO_LARGE is the only guaranteed error code).
  • F56: per-runtime @types packages (@types/bun, @types/deno, @cloudflare/workers-types, @types/aws-lambda) documented for full typing of options.bun/runtime.cloudflare/etc.
  • CLI: srvx fetch non-2xx → exit 22 documented prominently (diverges from curl). usage.ts fixes: env wording "default" not "override", document --hostname, fetch --host/--tls, --method (+--request alias), exit-22 line; 10.cli.md precedence (flag > module > env > default) and Deno --import drop.
  • Skipped tests: opened tracking issues for the three known protocol-test skips and linked each in a comment: test: enable "response stream error" test under HTTP/2 (TLS) #235 (HTTP/2 response stream error), test: enforce/verify "absolute path in request line" under HTTP/2 (TLS) #236 (HTTP/2 absolute path in request line), test: run HTTP/2 test suite on Deno and Bun #237 (Deno/Bun HTTP/2 suite).

Verification

pnpm lint:fix (automd regenerated the CLI usage block), pnpm typecheck, and the common + CLI vitest suites all pass (914 passed / 32 pre-existing skips). Deno/Bun-only suites not run locally (no runtimes installed; CI covers them).

🤖 Generated with Claude Code

Docs scope of the v1 stabilization plan (F54, F14, F53, F11, F55, F56,
CLI, support matrix, skipped-test issue links).

- server.md: delete non-existent `server.addr` / `server.port` sections
  and the stale `#port-required` anchor (F54); fix "Bun instance in Deno"
  copy-paste error.
- handler.md: `request.runtime.node?.req.path` -> `.url` (F54).
- options.md: `maxHeadersize` -> `maxHeaderSize`; document `hostname`
  HOST env default (F14); document `manual`, `middleware`, `plugins`,
  `serviceWorker`; add per-runtime support matrix (close(true) on Deno,
  trustProxy scope, Bun 128 MiB / loader-drop for maxRequestBodySize,
  manual on workers, Cloudflare env in SW syntax, no upgrade/WebSocket ->
  crossws); Deno 2.x range + explicit `srvx/bunny` import note; public
  subpaths table; per-runtime `@types` note (F56); note `srvx/static` is
  Node-API-only.
- node.md: state `_request`/`_url` are supported API frozen at v1 (D7/F53);
  `toFetchHandler`/`fetchNodeHandler` stable, `srvx/tracing` experimental
  (D5/F11); add Semver policy section incl. ERR_BODY_TOO_LARGE-only code
  guarantee (F55).
- cli: `usage.ts` env wording (default, not override), document
  `--hostname`, fetch `--host`/`--tls`, `--method` (+`--request` alias),
  exit-22 line; cli.md precedence, exit-code divergence from curl, and
  Deno `--import` drop.
- types.ts: fix "server with listen" JSDoc typo + HOST default (F14).
- Link skipped protocol tests to tracking issues (#235, #236, #237).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pi0x pi0x requested a review from pi0 as a code owner July 14, 2026 12:47
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@pi0x, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2973c59b-d495-4d25-9159-71c97c6086fd

📥 Commits

Reviewing files that changed from the base of the PR and between d8523dc and f6154d5.

📒 Files selected for processing (9)
  • docs/1.guide/10.cli.md
  • docs/1.guide/2.handler.md
  • docs/1.guide/3.server.md
  • docs/1.guide/5.options.md
  • docs/1.guide/8.node.md
  • src/cli/usage.ts
  • src/types.ts
  • test/_tests.ts
  • test/node.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/v1-stabilization

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Jul 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/srvx@238

commit: f6154d5

@pi0x pi0x left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: docs v1 stabilization batch (PR #238)

Verdict: Approve. Every technical claim in the new/changed prose was fact-checked against the actual source; I found no factually wrong statement that would mislead users. pnpm lint (oxlint + oxfmt) passes; the automd blocks in every file this PR touches regenerate to "no changes". Two low/informational notes below, neither blocking.

Spec checklist (all verified against code, not just the diff)

F54

  • server.addr / server.port sections deleted — confirmed Server (src/types.ts:230-293) exposes only url, no addr/port anywhere. ✅
  • Stale /guide/options#port-required anchor removed. ✅
  • req.pathreq.urlIncomingMessage has .url, not .path (Express-ism). ✅
  • Bun-in-Deno copy-paste fixed (server.deno.server). ✅
  • maxHeadersizemaxHeaderSize — Node's real option is maxHeaderSize; node opts spread into NodeHttp.ServerOptions (types.ts:196). ✅
  • manual / middleware / plugins / serviceWorker documented; all four options exist (types.ts:78,216, ServerOptions.middleware/plugins) and the examples are valid (server.options.middleware is always an array per types.ts:247; ServerPlugin = (server) => void per types.ts:47; serviceWorker: {url?, scope?} matches types.ts:216-227). ✅
  • Public subpaths table — srvx/static, srvx/log, srvx/loader, srvx/cli, srvx/generic, srvx/service-worker all present in package.json exports, and the listed named exports match (loadServerEntry in loader.ts, main/cliFetch in cli.ts, serve/FastURL/FastResponse in generic.ts, log in log.ts). ✅

F14hostname ← HOST (_utils.ts:15) and silent ← TEST (_utils.ts:34) accurately documented; types.ts:92 "server with listen" typo fixed. ✅

Support matrix — every cell is a correct claim:

  • close(true) ignored on Deno ✅ (F25 behavior)
  • trustProxy scope: Node/AWS/Bun/Deno only — confirmed _trust-proxy is consumed only by those adapters; cloudflare/bunny/generic/service-worker have zero usage. ✅
  • maxRequestBodySize: Bun forwards to native (128 MiB default) bun.ts:84-88; dropped in loader mode (loader early-returns at bun.ts:48-50 before serve opts). ✅
  • manual no-op on module workers ✅; Cloudflare env module-syntax-only ✅ (F29); upgrade/WebSocket unsupported → crossws ✅ (locked decision, stated).

srvx/static Node-API-only — confirmed uses node:fs/node:zlib. ✅
Deno range — "Deno 2.x / CI 2.7.x"; CI pins 2.7.12 (.github/workflows/ci.yml:47). Explicit srvx/bunny import note ✅.

F53/D7_request/_url documented as supported, frozen at v1. ✅
F11/D5 + F55toFetchHandler/fetchNodeHandler stable, exported from srvx/node (node.ts:33-34); srvx/tracing experimental; new Semver-policy section; ERR_BODY_TOO_LARGE stated as the only guaranteed code. ✅

F56 — types packages all real (checked npm): @types/bun@1.3.14 ✅, bun-types@1.3.14 (valid alt) ✅, @types/deno@2.7.0 — real, maintained DefinitelyTyped package (created 2024, updated 2026-05, not deprecated) ✅, @cloudflare/workers-types ✅, @types/aws-lambda@8.10.162 ✅.

CLI — exit-22 (main.ts:52 res.ok ? 0 : 22) ✅; usage "default port/host" wording ✅; --hostname ✅; --method + --request curl alias (main.ts:140 values.method || values.request) ✅; fetch --host/--tls (fetch.ts:117) ✅; --import dropped on Deno (main.ts:92 isNode || isBun) ✅.

Issues#235 (HTTP/2 response-stream-error skip, _tests.ts), #236 (absolute-path-in-request-line skip, _tests.ts), #237 (Deno/Bun HTTP/2 suite skip, node.test.ts) all exist, and each inline comment points at the correct skip with an accurate description. ✅

No collision with in-flight work: the ### onError section (5.options.md:161) and any ### gracefulShutdown/### error section are left untouched (#230's); the ### trustProxy hop-model text is untouched (#229's); 9.aws-lambda.md not edited. ✅

Findings

Low — gracefulShutdown row is beyond the planned matrix scope (docs/1.guide/5.options.md:343). The plan's support-matrix bullet doesn't list gracefulShutdown as a cell, and its per-option docs are #230's territory. The row is factually correct (gracefulShutdownPlugin runs only in node/bun/deno; _plugins.ts:17), so not misleading — just confirm with #230 that a matrix cell + #230's ### gracefulShutdown section won't read as duplicated/conflicting.

Low — same-file overlap risk with #229/#230. The new sections (Per-runtime Support, Public Subpaths, TypeScript Types) and the trustProxy matrix row live in 5.options.md, which #229 (trustProxy section) and #230 (error/gracefulShutdown sections) also edit. Content is orthogonal and the regions differ (new blocks appended at end vs. their mid-file section edits), so a textual merge conflict is unlikely — flagging for awareness only.

Info (not this PR's fault) — automd drift in docs/1.guide/1.index.md. Running automd reformats the examples table there (column alignment) under automd 0.4.3. It's pre-existing on main, unrelated to this PR, and pnpm lint does not run an automd check, so CI won't catch it either. No action needed here.

Nice work — this is exactly the kind of claim-by-claim accuracy the batch needed. The @types/deno recommendation in particular is correct (it's a live DT package), which was the easiest one to get wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants