test: make the #39621 reset fixture report its fd layout and bind its dead port

[robobun]

Problem

• Test-only follow-up to usockets: skip the poll-error close for a socket a handler already closed #39621, from its self-review. Its fixture only reaches the stale SO_ERROR read it was written for when the connect it opens from close() gets the accepted socket's fd number. Nothing checked that. If the runtime starts to open one more fd on that path, the test keeps passing without covering the bug (checked: holding one extra fd at the right moment makes the fixture print ECONNREFUSED with and without the usockets: skip the poll-error close for a socket a handler already closed #39621 guard).
• The fixture's dead port was the local port of a connect() that bound its port automatically. On Linux such a port can be handed out again as the automatic local port of the connect that is meant to be refused, which then connects to itself and the fixture sees open. Measured with the fixture's construction: 9 self-connects in 120000 connects, so about 1 in 13k runs. This is the defect test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400 describes for deadPort() in proxy-stress-helpers.ts. That helper is left to test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400; an earlier revision of this PR changed it too, and d80854a took that back.

Fix

• The fixture reports its precondition next to the outcome: the accepted socket's fd number is closed when close() runs, and open again right after Bun.connect() returns (fstatSync). The test compares the whole JSON line. On Windows, where socket handles are not fd numbers, the fixture reports only the outcome, and the comment above the describe says so.
• The holder connects with localAddress: "127.0.0.1", so it binds its port itself. A port that bind() handed out is not handed out again by a later automatic bind: 0 self-connects in 120000 with this change. This is the same property test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400 uses (it returns a bind(0) port). The fixture runs in a child process and cannot import the helper, so it keeps this one line.
• Verified: with the usockets: skip the poll-error close for a socket a handler already closed #39621 guard removed from loop.c the test prints {"acceptedFdFreeInClose":true,"acceptedFdReusedByConnect":true,"outcome":"ECONNRESET"} and fails on the outcome. With main it passes, 10 of 10 runs.
• This PR changes no runtime code, so there is nothing for it to fail against on main. The fail case it guards is the removed guard above.

Background

• Dead port: a port nothing listens on, held so that no other test can listen on it. The fixture holds it as the local port of an established connection. Connecting to it is refused because nothing listens there.
• Automatic local ports: a connect() without a local binding gets a port from the kernel, and a later connect() may reuse such a port as long as the four-tuple differs. A connect to its own port with the same tuple is a TCP self-connect, which succeeds. Ports that bind() handed out are skipped by that allocator (test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400 has the kernel details).
• The fixture's fd arrangement: POSIX gives a new socket the lowest free fd number. The fixture reserves a number before the peer connects and frees it before the accept, so the accepted socket gets a lower number than the peer, and the connect opened from close() takes the accepted socket's number.

Notes

Other places build refused ports by hand as well (closedPort() in test/js/sql/wire-frames.ts, deadRegistryHref() in test/cli/install/bun-audit.test.ts, bind-then-close in a few tests). A shared helper in test/harness.ts would be the place to carry the rule once #35400 has landed. Not done here.

Measurements, release binary on Linux 6.17 (ip_local_port_range 32768 to 60999): holder bound by connect(): 7 and 2 self-connects in two runs of 60000 (Bun.connect holder), 2 in 60000 (net.connect holder, the deadPort() construction). Holder bound with localAddress: 0 in 60000, 0 in 60000, 0 in 60000. The connect()-bound ports seen were all even and the bind()-bound ones all odd, which is how the two allocators stay apart.

---

no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 minutes

Limit details: You’ve used all 5 included reviews currently available.

You can run this review on demand instead of waiting.

_{On-demand reviews are free until September 18, 2026. After that, they cost $0.25 per reviewed file.}

• Run review for free

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f1159277-1c61-4329-acac-0898979cfe3b

📥 Commits

Reviewing files that changed from the base of the PR and between 4199361 and d80854a.

📒 Files selected for processing (1)

• test/js/bun/net/socket.test.ts

---

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

[robobun]

Status: test-only change pushed, waiting for CI. Head is d80854a. The diff is now test/js/bun/net/socket.test.ts only: the deadPort() change this PR carried earlier duplicated #35400, which has fixed that helper since July, so it was taken back and #35400 is the fix for the helper.

What it guards: the fd report was checked both ways. With the #39621 guard removed from loop.c the test prints the two fd flags as true and the outcome as ECONNRESET, and holding one extra fd at the wrong moment turns acceptedFdReusedByConnect false. The self-connect was measured with the fixture's construction: 9 of 120000 connects without localAddress, 0 of 180000 with it. On main the test passes 10 of 10.

This PR has no runtime change, so it cannot fail on main by design. Follow-up to #39621 and #39600.

[claude]

LGTM — test-only hardening that fixes a rare self-connect flake and makes the #39621 fixture assert its own fd precondition.

What was reviewed:

• Bun.connect accepts localAddress (parsed at src/runtime/socket/Listener.rs:1122) and net.connect accepts it via node compat, so both holders bind explicitly as intended.
• socket.fd and isWindows are already used/imported in socket.test.ts; the JSON key insertion order in report matches expectedReport so the string comparison is stable.
• The parenthesization of ?? inside the ternary in proxy-stress-helpers.ts is a formatting no-op.

Extended reasoning...

Overview

Test-only follow-up to #39621 touching two files: test/js/bun/http/proxy-stress-helpers.ts (adds localAddress: "127.0.0.1" to the deadPort() holder plus a doc comment; one Prettier-style paren change) and test/js/bun/net/socket.test.ts (adds localAddress to the Bun.connect holder in the reset fixture, and makes the fixture report whether the fd-reuse precondition actually held via fstatSync probes, with the test asserting the full JSON line). No runtime code is touched.

Security risks

None. Changes are confined to test fixtures and a test helper; they add a connect option and read fd state via fstatSync inside a spawned subprocess.

Level of scrutiny

Low-to-medium. It's test infrastructure, but the changes rely on subtle kernel behavior (Linux ephemeral-port pool split between bind() and connect() auto-bind; POSIX lowest-free-fd semantics). I verified the concrete claims against the codebase: Bun.connect reads localAddress from options (Listener.rs:1122), socket.fd is a real property used elsewhere in the same test file, isWindows is already imported at line 13, and the report object's key insertion order (acceptedFdFreeInClose → acceptedFdReusedByConnect → outcome) matches the expectedReport literal so JSON.stringify produces identical strings. The Windows branch correctly skips the fd checks since socket handles there aren't fd numbers.

Other factors

The PR description documents empirical verification: 0 self-connects in 180k with localAddress vs 9-in-120k / 2-in-60k without; the fixture prints the expected fd flags both with and without the #39621 guard; 10/10 passes on main; proxy-stress-errors.test.ts (the deadPort() consumers) still passes. The change follows harness conventions (concurrent describe, tempDir, bunEnv/bunExe, all pipes drained via Promise.all, exit code asserted last). The new fd assertion is intentionally strict — if the runtime later opens an extra fd on that path, the test will fail loudly rather than silently losing coverage, which is the stated goal.

[claude]

LGTM — test-only de-flake with tighter precondition assertions.

What was reviewed:

• localAddress on the holder connect: confirmed Bun.connect reads it (Listener.rs:1122) and net.connect supports it natively; the explicit bind avoids the Linux self-connect corner.
• fd-report additions: socket.fd and isWindows are already used in this file; JSON.stringify key order matches insertion order on both sides so the string compare is stable.
• Windows path skips the fd checks (socket handles aren't fd numbers) and asserts only the outcome, matching the described intent.

Extended reasoning...

Overview

Test-only follow-up to #39621 touching two files: test/js/bun/http/proxy-stress-helpers.ts (adds localAddress: "127.0.0.1" to the deadPort() holder connect plus a doc-comment paragraph) and test/js/bun/net/socket.test.ts (same localAddress fix on the fixture's holder, plus fd-layout reporting via fstatSync so the test asserts its own precondition — that the connect opened from close() reuses the accepted socket's fd number). No runtime code is touched.

Security risks

None. Changes are confined to test fixtures and a test helper; no auth, crypto, or user-facing surface.

Level of scrutiny

Low-to-medium. This is a targeted flake fix (~1/13k self-connect on Linux) plus a test-tightening that turns a silently-vacuous test into a loud one if the runtime's fd allocation shifts. I checked that Bun.connect actually honors localAddress (src/runtime/socket/Listener.rs:1122), that socket.fd and isWindows are already imported/used in socket.test.ts, and that the JSON.stringify comparison is order-stable (both sides insert keys in the same sequence: acceptedFdFreeInClose → acceptedFdReusedByConnect → outcome). The Windows branch correctly reduces to { outcome } since checkFds gates both fd writes in the fixture.

Other factors

The PR description documents empirical verification in both directions: passes 10/10 on main, fails on the outcome (with fd flags still true) when the #39621 guard is removed from loop.c, and proxy-stress-errors.test.ts (the deadPort() consumers) still passes. The mechanism (Linux ephemeral-port allocation keeping bind()-assigned and connect()-assigned ports apart) is real and the doc comment explains it so the localAddress line won't be simplified away later. No prior reviewer comments to address.

[claude]

LGTM — test-only hardening of the #39621 fixture.

What was reviewed:

• localAddress is a real Bun.connect option (src/runtime/socket/Listener.rs:1122); forcing an explicit bind avoids the Linux TCP self-connect flake path.
• socket.fd and isWindows are already used/imported in this file; the fd precondition checks are gated off Windows where socket handles aren't fds.
• JSON key insertion order in the fixture's report matches the expectedReport literal on both platform branches, so the string comparison holds.
• The deadPort() change mentioned in the description was dropped in d80854a (deferred to #35400), so the diff is single-file as shown.

Extended reasoning...

Overview

This PR hardens a single regression fixture in test/js/bun/net/socket.test.ts that was added in #39621. Two independent changes:

1. The "dead port" holder now passes localAddress: "127.0.0.1" to Bun.connect, forcing an explicit bind() for the local port. This avoids a ~1/13k Linux flake where the kernel hands the same ephemeral port to the refused connect's automatic bind, producing a TCP self-connect that succeeds instead of being refused.
2. The fixture now records whether its fd-reuse precondition actually held (acceptedFdFreeInClose, acceptedFdReusedByConnect via fstatSync) and asserts on the full JSON report. If the runtime later opens an extra fd on this path, the test fails visibly instead of silently passing without exercising the stale SO_ERROR read it was written for.

No runtime code is touched.

Security risks

None. Test-only change to a subprocess fixture that connects to loopback ports it created itself.

Level of scrutiny

Low-to-medium. It's test-only, but the fixture depends on fairly subtle POSIX fd-numbering and Linux ephemeral-port semantics. I checked that localAddress is a supported Bun.connect option (Listener.rs), that socket.fd is already used elsewhere in this test file, that isWindows is already imported, and that the JSON property insertion order in the fixture matches the object literal order in expectedReport on both the Windows and non-Windows branches (so the string-equality assertion holds). The Windows branch correctly skips the fd checks since socket handles are not fds there.

Other factors

• The change directly implements two REVIEW.md test rules: it de-flakes without weakening the assertion (the outcome is still asserted exactly), and it adds a precondition assertion so the test can't pass for the wrong reason.
• The PR description originally covered a parallel deadPort() fix in proxy-stress-helpers.ts, but the head commit d80854a dropped it and defers it to #35400. The current diff is single-file and self-contained.
• The author reports 10/10 passes on main and verified the fixture still fails on the outcome when the #39621 guard is reverted, and that the fd flags flip when the arrangement is deliberately broken — both preconditions and the negative case were exercised.