← Back to PRs

#8389: fix(test): eliminate flaky chutes-oauth 'bad port' race condition (#8337)

by Glucksberg open 2026-02-04 00:24 View on GitHub →
commands size: S experienced-contributor
Fixes #8337 The `chutes-oauth.test.ts` was failing intermittently with a 'bad port' error due to two race conditions: 1. **TOCTOU race in `getFreePort()`:** The helper bound port 0 to discover a free port, released it, then tried to reuse it — another process could claim it in between. 2. **Server-not-ready race:** `onAuth` fired before the callback server was actually listening. **Changes:** - Removed the `getFreePort()` helper entirely - Test now uses port 0 directly (OS assigns port to the callback server) - Added `onListening` callback to `waitForLocalCallback()` and `loginChutes()` — fires with the actual bound address - Added `readyPromise` so `onAuth` waits for the server to be listening before making requests - Production code changes are additive (new optional callback params) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR fixes intermittent `bad port` failures in the Chutes OAuth test flow by removing the TOCTOU-style “find a free port then reuse it” pattern and instead letting the callback server bind to port `0` (OS-assigned). Production code in `src/commands/chutes-oauth.ts` is updated additively: `waitForLocalCallback()` and `loginChutes()` gain an optional `onListening` callback so callers/tests can learn the actual bound host/port, and `loginChutes()` now waits for the local server to be listening before calling `onAuth()` to avoid a server-not-ready race. Overall this aligns the test harness with how Node’s HTTP server binding works and makes the local OAuth callback flow more deterministic. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge and should reduce test flakiness, with minor edge-case clarity improvements worth considering. - Changes are small and localized: the test now uses OS-assigned ports and production code adds an optional callback plus a readiness gate before triggering auth. The main remaining concern is that the new test can fail confusingly if `onListening` never fires (leaving `boundPort=0`), and the readiness resolver pattern is slightly fragile but not incorrect. - src/commands/chutes-oauth.test.ts (assert `boundPort` set); src/commands/chutes-oauth.ts (readiness resolver pattern) <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs