fix(e2e): raise timeout for agent-socket cleanup - #786
Conversation
✅ Deploy Preview for images-devsy-sh canceled.
|
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for devsydev canceled.
|
The "socket directory is cleaned up after connection close" spec (and the no-forward variant) polled a fresh `devsy ssh` connection under a 5s per-attempt context. A cold connection's tunnel + container-exec setup routinely approaches ~4s locally and exceeds 5s under CI load, so the poll process was killed mid-connect and DevsySSH returned an empty string. The Eventually discarded the error and matched on empty output, so it never observed GONE and timed out at ~30s — even though the socket had already been cleaned up promptly (~3s, verified locally against Docker). Raise the per-poll context to 30s (outer window 90s) so a cold connection can complete, and surface the poll error instead of discarding it so a genuine leak or persistent connection failure fails loudly rather than timing out on empty output.
72c0418 to
47ba2de
Compare
skevetter
changed the title
skevetter
changed the title
1 participant
Summary
The e2e spec "socket directory is cleaned up after connection close" was failing in CI with an empty observed value (not
PRESENT):Root cause
The cleanup assertion polls a fresh
devsy sshconnection under a 5s per-attempt context (agent_forward.go). A cold connection's tunnel + container-exec setup routinely approaches ~4s locally and exceeds 5s under CI load, so the poll's context kills thedevsy sshprocess mid-connect (signal: killed) andDevsySSHreturns an empty string. TheEventuallydiscarded that error and matched on empty output, so it never observedGONEand timed out at ~30s.The socket itself was cleaned up promptly — the connection-closing callback fires on transport EOF from a clean
ssh -O exit. This was verified end-to-end locally against Docker (OrbStack): with unmodified production code the socket wasGONEon the first poll at +3.1s. Tightening the poll budget to 2s reproduced the exact CI empty-output symptom, confirming the diagnosis.Fix
Test-only. Raise the per-poll context to 30s (outer window 90s) so a cold connection can complete, and surface the poll error instead of discarding it — so a genuine socket leak or a persistent connection failure fails loudly rather than timing out on empty output. Applied to both the cleanup spec and the no-forward-cleanup spec.
No production code changes;
#760's keep-alive tolerance is untouched.Verification
agent-forwardlabel suite (5 specs) passes locally against Docker.pkg/ssh/serverunit tests pass.