testutils: make GetFreePort return distinct ports per process

[evanphx]

🤖 Authored autonomously by Claude Code (no human co-author). Opened via the /bot-friendly-pr skill; a human reviews before it lands.

What

Fixes the TestEtcdComponentIntegration port-collision flake (MIR-720) at its root: testutils.GetFreePort.

GetFreePort finds a free port by binding to :0 and then releasing it before returning the number, so two calls in quick succession could hand back the same port — the OS is free to re-offer the just-freed one. In the reported failure both clientPort and peerPort came back as 36517; etcd tried to bind its client and peer URLs to one port, failed with address already in use, and the test timed out waiting for readiness.

How

Remember every port handed out in this process (guarded by a mutex) and skip a repeat, rebinding on the next iteration to rotate to a fresh ephemeral port. This resolves the collision for all call sites at once — the etcd test takes two/three ports in a row — rather than patching each call, and preserves the existing "free for both TCP and UDP" guarantee. Bumped the attempt budget 10 → 20 to leave headroom for the extra rejections.

Chose this over the ticket's per-test options (assert-and-retry in the test, or clientPort+1) because the bug is in the helper's contract — a "get a free port" helper returning the same port twice — so fixing it there is both more correct and narrower in aggregate.

Verification

go test ./pkg/testutils/ — new TestGetFreePortReturnsDistinctPorts requests 200 ports and asserts every one is unique; run with -count=5 to confirm the fix isn't itself flaky. Package builds and vets clean.

The flake is probabilistic (two calls occasionally collide), so there's no deterministic red-first test to show; the new test instead pins the distinctness invariant the old implementation didn't guarantee. I did not run the full TestEtcdComponentIntegration (it needs the etcd integration harness), but the fix is entirely within the port helper it depends on.

Part of MIR-720

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f894e39b-7be8-4401-9e78-79b618886f7a

📥 Commits

Reviewing files that changed from the base of the PR and between 7f611d5 and 30001ac.

📒 Files selected for processing (2)

• pkg/testutils/port.go
• pkg/testutils/port_test.go

---

📝 Walkthrough

Walkthrough

GetFreePort now maintains synchronized process-wide state for ports it has returned, rejects previously issued candidates, and retries up to 20 times before failing. A new test calls the function 200 times, verifies each port is positive, and detects duplicate results.

---

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

[miren-code-agent]

🍪 biscuit: ✅ ready to merge — auto-review, non-blocking

This is a focused, well-reasoned fix for a real flake. The root cause (two quick GetFreePort calls getting the same port back from the OS because the first listener was already released) is correctly diagnosed, and the per-process deduplication map is a clean way to eliminate it. The test directly pins the guarantee by calling GetFreePort 200 times and asserting all ports are distinct — exactly the right kind of regression test for this class of bug.

A few things I'd flag for conscious consideration before this graduates to a human reviewer:

The "seen" map is never pruned. In a long test run that calls GetFreePort tens of thousands of times the map grows unboundedly. The ephemeral port range on most systems is roughly 28 000–32 000 ports (Linux default), so after enough calls the map could approach or exceed the usable range, turning the 20-attempt loop into a spin that always hits the t.Fatalf. For typical test suites this is unlikely to matter, but it's worth being aware of.

The retry budget (20 attempts) is now shared between two rejection reasons — UDP unavailable and "already seen" — without distinguishing them. If many ports were previously handed out, legitimate "UDP not free" failures silently eat into the same budget as deduplication retries. This is a minor point, but a comment noting that 20 was chosen to cover both cases would help future readers.

One subtle race window remains. Between udpConn.Close() and the caller actually binding the returned port, another process can still grab it. This was true before the PR too, so it's not a regression — just worth noting that the deduplication protects against intra-process re-use, not inter-process races.

None of these are blockers. The fix is correct for the documented failure mode, the deduplication logic is sound and properly locked, and the test is genuinely meaningful. This is ready to hand to a human reviewer.

---

🍪 full review note · comment /biscuit review to run biscuit again.

[evanphx]

🤖 Settled and ready for human review. Automated round-trip summary:

• CI: green — 21 checks pass, 1 skipped, 0 failures.
• biscuit (miren-code-agent): ✅ ready to merge on the first pass, no blockers, no inline comments. Three non-blocking advisories left for the human reviewer to weigh (not addressed here, since each is either a design choice or pre-existing):
  1. the per-process handedOutPorts map is never pruned — unbounded over tens of thousands of calls (biscuit: unlikely to matter for typical suites);
  2. the 20-attempt budget now covers both "UDP unavailable" and "already seen" rejections;
  3. the intra-process dedup doesn't close the inter-process TOCTOU window (pre-existing, not a regression).
• CodeRabbit: no review posted within the polling window — appears to be skipping this draft. Not blocking.

Still a draft — a human decides readiness and merge. End of the bot's autonomous pass.