Fix VZ guest exec buffering - #2628
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request fixes a bug where VSock execution output was unreliable due to buffered bytes being lost. By returning the bufio.Reader used during the handshake in primeConnection and passing it to ServeStream, any bytes buffered immediately after the ready ACK are preserved. A regression test was also added to verify this behavior. The review feedback correctly identifies a potential race condition in this new test, where the helper goroutine closes the connection immediately after writing the payload, and suggests waiting for the client's response to ensure test robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Keep the priming VSock buffered reader when handing off from guest readiness to command serving so exec requests buffered after the ready ACK are not lost. Backlog: TASK-12141
Wait for and validate the exec response before closing the helper-side pipe so the test does not rely on a side-effect race. Backlog: TASK-12141
3b50b01 to
3a0cf8a
Compare
PR Summary by QodoPreserve buffered VSock exec requests after guest readiness
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1.
|
Wait for the exec response in the regression test, use a less brittle read deadline, and move the Backlog record to unique TASK-13134 after rebasing onto dev. Backlog: TASK-13134
Summary
TASK-13134.Verification
GOCACHE=/private/tmp/tldw-go-build-cache go test ./internal/guestGOCACHE=/private/tmp/tldw-go-build-cache go test ./...git diff --check2 passed3 passed/private/tmp/tvz-e2e.4iJ3wt/evidence/host-smoke-evidence.jsonfinal_exit_code:0Change Summary
This PR fixes the real VZ Linux exec path by preserving the buffered VSock reader after guest handshake/readiness, preventing the first exec request from being lost when it arrives immediately after the ready response. It adds a regression test for that exact buffering edge case and verifies the fix with the Go agent test suite plus a real local Apple VZ smoke run using a rebuilt Debian arm64 bundle.