fix(dev): cancel readiness retries on abort - #2109
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Small, well-scoped test-only fix. The change correctly addresses both sides of the lingering handle problem:
createServer((socket) => socket.destroy())—waitForPortopens a real TCP connection (src/io/port.ts:60) that the server accepts. Previously nothing destroyed the server-side accepted socket, so its handle could keep Bun's event loop alive on Windows.- Awaiting
server.close()via its callback ensures the listener is fully released before the test returns, rather than relying on the previous fire-and-forget call.
The PR description accurately explains the Windows CI hang, the fix matches the described root cause, and no production code is touched. Nothing to change here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2109 +/- ##
============================================
- Coverage 97.42% 97.41% -0.02%
============================================
Files 429 429
Lines 26314 26330 +16
============================================
+ Hits 25637 25649 +12
- Misses 677 681 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Related: #2110 |
Problem
waitForPortleft active sockets and scheduled retries alive after abort. In CI, the supervisor's 60ms abort test instead ran for about 60 seconds and timed out; the original #2041 Windows run eventually lost its runner.Solution
Abort now destroys the active socket, clears the pending retry, removes its listener, and rejects immediately. The TCP test also closes accepted sockets and awaits listener shutdown.
Testing
tsc --noEmitpassed.