[robobun]

Problem

A node:quic session created without the cc option gets lsquic's default congestion controller, which is es_cc_algo 3, "adaptive". Node's default is cubic (ngtcp2's default; src/js/internal/quic/quic.ts only validates cc and passes undefined through, and apply_transport_params in src/runtime/node/quic/endpoint.rs only called cc_algo() when the option was given).

Adaptive mode is also the wrong default for these engines on its own merits. It commits each connection to Cubic or BBRv1 on the connection's first RTT sample (send_ctl_select_cc, threshold 1.5 ms), and both node:quic engines are only ticked from the event loop, so that sample measures whatever JS ran between two ticks. A server that does any work in its session callback, or a client that does any work right after connect(), gets BBRv1 (BUN_DEBUG_lsquic=1: select BBRv1 congestion controller), and BBRv1 then keeps modelling the loop as the path: the only bandwidth it can observe is one window per loop iteration while its min RTT is the real loopback RTT, so on a busy loop the window shrinks to BBR's 4-packet floor and stays there. With the released binary, a session sending 512 KiB chunks from a loop paced at 12 ms per iteration reports session.stats.cwnd === 5840n within 20 iterations (20/20 runs) and moves about 5.9 KB per iteration from then on; the same transfer on Cubic keeps a window of 100 to 240 KiB (bounded by the receiver's UDP buffer) and moves 4 to 170 times as much. Which of the two a given session got depended on what the loop happened to be doing during its handshake. #37455 makes the same change for the Bun.serve({http3}) and fetch engines in quic.c and deliberately left node:quic out; this is the node:quic half.

Fix

apply_transport_params sets Cubic before looking at the options, so the default and the options is-not-an-object path both get it, and the cc mapping is now bbr to BBRv1 and everything else (cubic, and reno, which lsquic does not ship) to Cubic. The previous catch-all arm mapped to 0, which is adaptive again; nothing reaches it through the public API (the JS layer validates cc), but it is gone too. Both engines go through this function (build_engine, from listen() and connect()).

Verification

test/js/node/quic/quic-endpoint.test.ts, "congestion control default": one test per engine (the listen() one sends to the test's client, the connect() one sends to the test's server), with the busy sender in test/js/node/quic/busy-loop-sender-fixture.ts. Two things about the fixture are worth calling out:

• Getting adaptive mode to pick BBRv1 reliably on loopback: lsquic subtracts the peer's reported ACK delay from its RTT samples, and the first full-connection sample is usually taken while the loop is still inside the handshake burst, so merely pacing the loop with 5 ms sleeps still produced first samples of 150 to 1300 us and Cubic 8 times out of 8. The fixture instead blocks the thread for 30 ms while the first round trip is in flight (in the listen() session callback, and right after connect(), which has already put the Initial on the wire), which makes that sample about 30 ms and the pick BBRv1 every time; with the fix there is no pick to make.
• Getting BBRv1 to actually collapse: its min RTT comes from raw ACK timing, so the connection has to have been ACKed while the loop was genuinely free. The fixture free-runs until 256 KiB have been sent (a few rounds of ACKs) rather than idling for a fixed delay, which was not enough on a loaded machine, then paces 60 iterations of 12 ms and reports the smallest stats.cwnd it saw over the last 30 of them. The tests require 16 KiB: Cubic settles just under what the receiver's UDP buffer holds (100 KiB and up with Linux's 208 KiB default, about 50 to 64 KiB behind Windows' 64 KiB default), and the unfixed default reports exactly 5840.

Results:

• Unfixed (USE_SYSTEM_BUN=1, 1.4.0): both tests fail with Received: 5840 in 10/10 runs of the file, and in 3/3 runs with the whole thing pinned to one CPU.
• Fixed debug build: 10/10 runs pass, plus 6/6 with three copies of the file running at once and 6/6 pinned to one or two CPUs; each test takes 3.5 to 4 s, almost all of it the fixture loading node:quic in a debug build (hence the explicit timeout). test/js/node/quic/ passes, and the upstream test-quic-cc-algorithm, -session-stats-detailed, -stream-bidi-varchunklen, -flow-control-uni, -datagram and -connection-concurrent tests pass with their flags.
• An explicit cc: 'bbr' run of the fixture on the fixed build reports the same 5840, which is how the fixture was checked against the thing it rules out; it is not asserted on in the suite because, starved to a single core, BBR did not always get all the way down within the window (one run out of three reported 20 KiB).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 7 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5e61c5e2-3b8b-4a25-a423-16815ee57f69

📥 Commits

Reviewing files that changed from the base of the PR and between da3851e and a588120.

📒 Files selected for processing (3)

• src/runtime/node/quic/endpoint.rs
• test/js/node/quic/busy-loop-sender-fixture.ts
• test/js/node/quic/quic-endpoint.test.ts

---

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

[robobun]

Status

Reproduced with the released binary (1.4.0): USE_SYSTEM_BUN=1 bun test test/js/node/quic/quic-endpoint.test.ts -t "congestion control" fails both tests (one per engine) with the sending session's stats.cwnd at 5840, BBR's 4-packet floor, in 10/10 runs; BUN_DEBUG_lsquic=1 on the sending process shows select BBRv1 congestion controller. With this branch the same tests pass on a debug build (10/10 runs, plus the loaded and CPU-pinned runs listed in the description).

Fix: apply_transport_params (src/runtime/node/quic/endpoint.rs) defaults es_cc_algo to Cubic, Node's default, instead of leaving lsquic's adaptive mode in place; cc: 'bbr' still selects BBRv1. Companion of #37455, which does the same for the Bun.serve/fetch HTTP/3 engines.

[github-actions]

This PR may be a duplicate of:

1. quic: use Cubic for the HTTP/3 engines and fix its pacing rate wrapping on Windows #37455 - Same root cause and same fix (pin lsquic es_cc_algo to Cubic so the adaptive default stops latching BBRv1 and collapsing cwnd to the 5840-byte floor), applied to the usockets HTTP/3 server and fetch engines instead of apply_transport_params in endpoint.rs.

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #37455: that PR changes the two engines in packages/bun-usockets/src/quic.c (Bun.serve HTTP/3 and the fetch client) and says it leaves node:quic alone. This one is the node:quic half, in src/runtime/node/quic/endpoint.rs where the listen()/connect() engines are configured and where the user-facing cc option is mapped. The two do not overlap in code and can land in either order.

[claude]

LGTM — one-line default in apply_transport_params aligns node:quic with Node's documented cc: 'cubic' default, mirroring #37455's fix for the other engines.

What was reviewed:

• CC_ALGO_CUBIC=1/CC_ALGO_BBR=2 match lsquic's es_cc_algo enum; the default is set before the !options.is_object() early return so both paths get it.
• The catch-all arm now maps to Cubic instead of 0 (adaptive) — unreachable through the JS layer's validateOneOf, but safer.
• Fixture wires early process exit to reject the awaited promises, uses await using for cleanup, and the negative cc: 'bbr' test proves the fixture actually detects the collapse the other two tests rule out.

Extended reasoning...

Overview

The PR changes apply_transport_params in src/runtime/node/quic/endpoint.rs to set es_cc_algo to Cubic (1) before parsing options, so a session created without cc gets Node's documented default instead of lsquic's "adaptive" mode. The explicit-cc match is simplified: bbr → BBRv1, everything else → Cubic (previously the catch-all mapped to 0, which is adaptive again). Two named constants replace the raw integers. The rest of the diff is a new test block in quic-endpoint.test.ts plus a subprocess fixture that reproduces BBRv1's cwnd collapse on a busy event loop.

Security risks

None. This changes a congestion-control default on an experimental node:quic module; no auth, crypto, parsing, or memory-ownership changes. The setter (us_nq_settings_set_cc_algo) already existed and is called with compile-time constants.

Level of scrutiny

Low-to-medium. The runtime change is effectively one line (s.cc_algo(CC_ALGO_CUBIC)) placed before the existing early-return, plus a defensive tightening of an already-validated match arm. It is the node:quic companion to #37455, which made the same change for Bun.serve({http3})/fetch. The constant values (1=Cubic, 2=BBRv1, 3=Adaptive default) line up with lsquic's es_cc_algo documentation and with the pre-existing comment this PR replaces.

Other factors

The test is timing-oriented by necessity — it is measuring congestion-controller behavior — but is designed against flakiness: it takes the minimum cwnd over the last 30 of 60 iterations (tolerating BBR's ACK-bunching bumps), the 32 KiB threshold sits well between the observed BBR floor (5840) and Cubic's 100+ KiB, and a third test with cc: 'bbr' asserts the collapse is observed by the same fixture. The PR description reports 10/10 failure on USE_SYSTEM_BUN=1 and 16/16 pass on the debug build, plus passing upstream Node QUIC tests. The subprocess helper wires proc.exited into a rejection so a fixture crash fails fast rather than hanging, and resources are released with await using. Bun.sleepSync here is the mechanism under test (a busy loop between engine ticks), not a wait-for-condition, and each site is commented per the review guidance.

[robobun]

^{Updated 4:24 AM PT - Aug 11th, 2026}

✅ @robobun, your commit a588120d5593ba373dbc150f29948c51f41edb70 passed in Build #92185! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37468

That installs a local version of the PR into your bun-37468 executable, so you can run:

bun-37468 --bun

[claude]

I reviewed this PR and didn't find any bugs. The Rust change is small and correct (Cubic default set ahead of option parsing, named constants match the lsquic es_cc_algo values, and the _ => 0 adaptive fall-through is gone). Because the new tests assert on congestion-window behaviour under a busy event loop — timing-sensitive by nature, even with the 3× margin and the condition-based free-run phase — a human look at the fixture's CI stability across platforms would still be worthwhile.

What was reviewed:

• apply_transport_params: default cc_algo(CC_ALGO_CUBIC) mirrors the existing idle_timeout(10) pattern and covers the !options.is_object() early-return path.
• cc mapping: bbr → 2, everything else → 1; the JS layer restricts cc to 'reno'|'cubic'|'bbr', so no value silently changes meaning.
• Fixture: free-run awaits bytesSent (not a fixed sleep), died race prevents hangs on fixture crash, resources released via await using, graceful session.close() lets both sides finish.

Extended reasoning...

Overview

Three files: src/runtime/node/quic/endpoint.rs (+7/−8 net), a new 113-line test fixture busy-loop-sender-fixture.ts, and ~110 new lines in quic-endpoint.test.ts. The runtime change sets lsquic's es_cc_algo to Cubic before reading options (so both the no-options path and the cc-unset path get it), replaces magic numbers with CC_ALGO_CUBIC/CC_ALGO_BBR constants, and drops the _ => 0 (adaptive) catch-all. This is the node:quic half of the same fix #37455 applies to the usockets HTTP/3 engines.

Security risks

None identified. The change only picks a congestion-control algorithm; it doesn't touch TLS, certificate handling, or input validation. The cc option was already validated in the JS layer before reaching this match.

Level of scrutiny

Moderate. The Rust diff is trivial and follows the exact pattern used two lines above for idle_timeout. What warrants a human look is the test: it spawns a fixture that blocks the thread for 30 ms during handshake, free-runs until 256 KiB are ACKed, then paces 60 iterations of 12 ms sleepSync and asserts the minimum stats.cwnd over the last 30 stays ≥ 16 KiB. The author stress-tested this (10/10 runs, 6/6 with three concurrent copies, 6/6 pinned to 1–2 CPUs) and the 16 KiB threshold sits ~3× below Cubic's smallest observed floor (≈50 KiB on Windows' 64 KiB UDP buffer), but congestion-control behaviour on shared CI runners across linux/macos/windows is inherently variable and worth a maintainer's judgement call.

Other factors

• The comment-cop bot flagged long comments five times; the author iterated down to one-line docs (all threads resolved).
• describe.concurrent keeps the two ~4 s tests overlapping; the 30 s timeout is justified by debug-build node:quic load time.
• spawnSender races IPC against proc.exited so a crashed fixture fails the test with its stderr instead of hanging — good pattern.
• The h3blast.c comment es_cc_algo = 3; // BBRv1 disagrees with this PR's (and the pre-existing endpoint.rs comment's) reading of 3 = Adaptive; that file isn't touched here and its comment is likely just stale, but noting it in case #37455's review didn't already.
• This changes a user-visible default in a Node compat module; aligning with Node's cubic default is the right call, but a human should confirm they're happy landing this independently of #37455.