chore: add tests for nbd cleanup

[jakubno]

No description provided.

[cursor]

PR Summary

Medium Risk
Adds new concurrency- and OS-dependent NBD shutdown tests (including root-only dd integration runs), which could be flaky or slow in CI even though no production logic changes.

Overview
Adds regression coverage to ensure NBD shutdown/cleanup reliably follows a cancel → Drain() → socket close sequence without triggering "use of closed network connection" errors when many async reads/writes are in flight, including both a net.Pipe()-based unit test harness and root-only end-to-end dd tests that shut down the device mid-transfer.

^{Written by Cursor Bugbot for commit 7441183. This will update automatically on new commits. Configure here.}

[claude]

The respErr return value is discarded here (magic, _, handle, err), and then the code unconditionally tries to drain readSize bytes of data payload for every response (lines 125–127). The test comment acknowledges that error responses have no data, but the drain always runs regardless.

If the dispatch layer converts context-cancelled reads into error responses (error code != 0, no payload), then io.ReadFull(clientConn, dataBuf) will consume bytes from subsequent response headers as "data" (since all responses are already buffered in the net.Pipe). For example, with 20 × 16-byte error headers buffered, after reading header #1 the drain would consume the remaining 304 bytes (headers 2–20), leaving the outer loop with an empty buffer — only 1 response would be counted instead of 20.

The fix is straightforward: capture respErr and only drain the payload for success responses:

magic, respErr, handle, err := readNBDResponse(clientConn)
// ...
if respErr == 0 {
    dataBuf := make([]byte, readSize)
    clientConn.SetReadDeadline(time.Now().Add(500 * time.Millisecond))
    _, _ = io.ReadFull(clientConn, dataBuf)
}

[cursor]

Cursor Bugbot has reviewed your changes and found 4 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

[cursor]

Use t.Context() instead of context.Background() in tests

Low Severity

context.WithCancel(context.Background()) is used where context.WithCancel(t.Context()) would be preferred. The WithCancel wrapper is not redundant here since cancel() is called at a specific point mid-test, but the base context passed to WithCancel can be t.Context() instead of context.Background() so the context is also auto-canceled if the test fails early.

Additional Locations (1)

• packages/orchestrator/pkg/sandbox/nbd/dispatch_shutdown_test.go#L161-L162

 

^{Triggered by learned rule: Use t.Context() instead of context.Background() in Go tests}

[cursor]

Use t.Context() instead of context.Background() in tests

Low Severity

nbdCtx := context.Background() is used where t.Context() would be preferred. The context does not need to outlive the test — deviceCleanup.Run already uses context.WithoutCancel internally, and pool cancellation is handled explicitly by the cleaner steps.

Additional Locations (1)

• packages/orchestrator/pkg/sandbox/nbd/path_direct_test.go#L246-L247

 

^{Triggered by learned rule: Use t.Context() instead of context.Background() in Go tests}