From dd20f7506934f5fe2c1f0048e973cf34af18ea95 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 21:25:36 +0200 Subject: [PATCH 1/2] Fix vite bridge conn swap race, blocking send, and stdin goroutine leak The dev tunnel bridge had three concurrency issues in one file: reconnects swapped tunnelConn without synchronization (and leaked the old connection), the connection:request send could block the tunnel reader and hang shutdown, and each approval prompt spawned a stdin reader goroutine that leaked on timeout and swallowed the next answer. Use atomic.Pointer for tunnelConn (closing the replaced connection), guard the connection request send with stopChan and a timeout, and read stdin with a single persistent goroutine feeding a channel. Existing tests synchronized on time.Sleep with a shared variable, which the race detector flags; they now synchronize on a channel so the package is race-clean. Co-authored-by: Isaac --- libs/apps/vite/bridge.go | 97 +++++++++++----- libs/apps/vite/bridge_test.go | 168 ++++++++++++++++++++++++---- libs/apps/vite/validate_dir_test.go | 21 ++-- 3 files changed, 223 insertions(+), 63 deletions(-) diff --git a/libs/apps/vite/bridge.go b/libs/apps/vite/bridge.go index 0bd665cacbd..5addd672b12 100644 --- a/libs/apps/vite/bridge.go +++ b/libs/apps/vite/bridge.go @@ -14,6 +14,7 @@ import ( "path/filepath" "strings" "sync" + "sync/atomic" "time" "github.com/databricks/cli/libs/cmdio" @@ -75,10 +76,11 @@ type prioritizedMessage struct { } type Bridge struct { - ctx context.Context - w *databricks.WorkspaceClient - appName string - tunnelConn *websocket.Conn + ctx context.Context + w *databricks.WorkspaceClient + appName string + // Atomic because reconnects swap the connection while the writer goroutine reads it. + tunnelConn atomic.Pointer[websocket.Conn] hmrConn *websocket.Conn tunnelID string tunnelWriteChan chan prioritizedMessage @@ -86,6 +88,7 @@ type Bridge struct { stop func() httpClient *http.Client connectionRequests chan *BridgeMessage + stdinLines chan string // Lines read by the persistent stdin reader, consumed by connection prompts port int keepaliveDone chan struct{} // Signals keepalive goroutine to stop on reconnect keepaliveMu sync.Mutex // Protects keepaliveDone @@ -116,6 +119,7 @@ func NewBridge(ctx context.Context, w *databricks.WorkspaceClient, appName strin stopChan: make(chan struct{}), tunnelWriteChan: make(chan prioritizedMessage, 100), // Buffered channel for async writes connectionRequests: make(chan *BridgeMessage, 10), + stdinLines: make(chan string), port: port, autoApprove: autoApprove, } @@ -123,9 +127,9 @@ func NewBridge(ctx context.Context, w *databricks.WorkspaceClient, appName strin b.stop = sync.OnceFunc(func() { close(b.stopChan) - if b.tunnelConn != nil { - _ = b.tunnelConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) - b.tunnelConn.Close() + if conn := b.tunnelConn.Load(); conn != nil { + _ = conn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) + conn.Close() } if b.hmrConn != nil { @@ -219,7 +223,7 @@ func (vb *Bridge) connectToTunnel(appDomain *url.URL) error { return nil }) - vb.tunnelConn = conn + vb.setTunnelConn(conn) // Start keepalive ping goroutine (stop existing one first if any) vb.keepaliveMu.Lock() @@ -235,6 +239,14 @@ func (vb *Bridge) connectToTunnel(appDomain *url.URL) error { return nil } +// setTunnelConn atomically installs a new tunnel connection and closes the +// previous one so a reconnect doesn't leak the old connection. +func (vb *Bridge) setTunnelConn(conn *websocket.Conn) { + if old := vb.tunnelConn.Swap(conn); old != nil { + old.Close() + } +} + // ConnectToTunnelWithRetry attempts to connect to the tunnel with exponential backoff. // This handles cases where the app isn't fully ready yet (e.g., right after deployment). func (vb *Bridge) ConnectToTunnelWithRetry(appDomain *url.URL) error { @@ -346,7 +358,8 @@ func (vb *Bridge) tunnelWriter(ctx context.Context) error { case <-vb.stopChan: return nil case msg := <-vb.tunnelWriteChan: - if err := vb.tunnelConn.WriteMessage(msg.messageType, msg.data); err != nil { + // Load per message so writes follow a reconnect to the new connection. + if err := vb.tunnelConn.Load().WriteMessage(msg.messageType, msg.data); err != nil { log.Errorf(vb.ctx, "[vite_bridge] Failed to write message: %v", err) return fmt.Errorf("failed to write to tunnel: %w", err) } @@ -364,7 +377,7 @@ func (vb *Bridge) handleTunnelMessages(ctx context.Context) error { default: } - _, message, err := vb.tunnelConn.ReadMessage() + _, message, err := vb.tunnelConn.Load().ReadMessage() if err != nil { if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseNoStatusReceived, websocket.CloseAbnormalClosure) { cmdio.LogString(vb.ctx, "🔄 Tunnel closed, reconnecting...") @@ -408,8 +421,16 @@ func (vb *Bridge) handleMessage(msg *BridgeMessage) error { return nil case "connection:request": - vb.connectionRequests <- msg - return nil + // The consumer can be blocked on a stdin prompt (or gone after stop), so a + // bare send here could stall the tunnel reader and hang shutdown. + select { + case vb.connectionRequests <- msg: + return nil + case <-vb.stopChan: + return nil + case <-time.After(wsWriteTimeout): + return errors.New("connection request queue full, dropping request") + } case "fetch": go func(fetchMsg BridgeMessage) { @@ -446,6 +467,27 @@ func (vb *Bridge) handleMessage(msg *BridgeMessage) error { } } +// readStdinLines forwards lines from r to stdinLines until read error or stop. +// A single persistent reader (rather than one goroutine per prompt) ensures a +// prompt that times out doesn't leak a goroutine that then swallows the user's +// answer to the next prompt. The channel is closed on read error so pending and +// future prompts fail instead of waiting for input that will never come. +func (vb *Bridge) readStdinLines(r io.Reader) { + reader := bufio.NewReader(r) + for { + line, err := reader.ReadString('\n') + if err != nil { + close(vb.stdinLines) + return + } + select { + case vb.stdinLines <- line: + case <-vb.stopChan: + return + } + } +} + func (vb *Bridge) handleConnectionRequest(msg *BridgeMessage) error { cmdio.LogString(vb.ctx, "") cmdio.LogString(vb.ctx, "🔔 Connection Request") @@ -458,25 +500,15 @@ func (vb *Bridge) handleConnectionRequest(msg *BridgeMessage) error { } else { cmdio.LogString(vb.ctx, " Approve this connection? (y/n)") - // Read from stdin with timeout to prevent indefinite blocking - inputChan := make(chan string, 1) - errChan := make(chan error, 1) - - go func() { - reader := bufio.NewReader(os.Stdin) - input, err := reader.ReadString('\n') - if err != nil { - errChan <- err - return - } - inputChan <- input - }() - select { - case input := <-inputChan: + case input, ok := <-vb.stdinLines: + if !ok { + return errors.New("failed to read user input: stdin closed") + } approved = strings.ToLower(strings.TrimSpace(input)) == "y" - case err := <-errChan: - return fmt.Errorf("failed to read user input: %w", err) + case <-vb.stopChan: + // Shutting down; no point answering the request. + return nil case <-time.After(BridgeConnTimeout): // Default to denying after timeout cmdio.LogString(vb.ctx, "⏱️ Timeout waiting for response, denying connection") @@ -907,7 +939,7 @@ func (vb *Bridge) Start() error { readyChan := make(chan error, 1) go func() { for vb.tunnelID == "" { - _, message, err := vb.tunnelConn.ReadMessage() + _, message, err := vb.tunnelConn.Load().ReadMessage() if err != nil { readyChan <- err return @@ -953,6 +985,11 @@ func (vb *Bridge) Start() error { return nil }) + // Single persistent stdin reader for connection prompts; see readStdinLines. + if !vb.autoApprove { + go vb.readStdinLines(os.Stdin) + } + // Connection request handler - not in errgroup to avoid blocking other handlers go func() { for { diff --git a/libs/apps/vite/bridge_test.go b/libs/apps/vite/bridge_test.go index 17f9fb4464b..2b75d2db435 100644 --- a/libs/apps/vite/bridge_test.go +++ b/libs/apps/vite/bridge_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -192,6 +193,18 @@ func TestBridgeHandleMessage(t *testing.T) { } } +// waitForMessage waits for the websocket test server to deliver a message. +// Tests must synchronize on a channel (not a sleep) so they pass under -race. +func waitForMessage(t *testing.T, received <-chan []byte) []byte { + select { + case message := <-received: + return message + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for message") + return nil + } +} + func TestBridgeHandleFileReadRequest(t *testing.T) { // Create a temporary directory structure tmpDir := t.TempDir() @@ -211,7 +224,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { w := &databricks.WorkspaceClient{} // Create a mock tunnel connection using httptest - var lastMessage []byte + received := make(chan []byte, 1) upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) @@ -227,7 +240,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { t.Errorf("failed to read message: %v", err) return } - lastMessage = message + received <- message })) defer server.Close() @@ -239,7 +252,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { defer conn.Close() vb := NewBridge(ctx, w, "test-app", 5173, false) - vb.tunnelConn = conn + vb.tunnelConn.Store(conn) go func() { _ = vb.tunnelWriter(ctx) }() @@ -252,12 +265,9 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { err = vb.handleFileReadRequest(msg) require.NoError(t, err) - // Give the message time to be sent - time.Sleep(100 * time.Millisecond) - // Parse the response var response BridgeMessage - err = json.Unmarshal(lastMessage, &response) + err = json.Unmarshal(waitForMessage(t, received), &response) require.NoError(t, err) assert.Equal(t, "file:read:response", response.Type) @@ -270,7 +280,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) w := &databricks.WorkspaceClient{} - var lastMessage []byte + received := make(chan []byte, 1) upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) @@ -285,7 +295,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { t.Errorf("failed to read message: %v", err) return } - lastMessage = message + received <- message })) defer server.Close() @@ -296,7 +306,7 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { defer conn.Close() vb := NewBridge(ctx, w, "test-app", 5173, false) - vb.tunnelConn = conn + vb.tunnelConn.Store(conn) go func() { _ = vb.tunnelWriter(ctx) }() @@ -309,11 +319,8 @@ func TestBridgeHandleFileReadRequest(t *testing.T) { err = vb.handleFileReadRequest(msg) require.NoError(t, err) - // Give the message time to be sent - time.Sleep(100 * time.Millisecond) - var response BridgeMessage - err = json.Unmarshal(lastMessage, &response) + err = json.Unmarshal(waitForMessage(t, received), &response) require.NoError(t, err) assert.Equal(t, "file:read:response", response.Type) @@ -368,11 +375,134 @@ func TestNewBridge_AutoApprove(t *testing.T) { assert.True(t, vb.autoApprove) } +// newWSConn starts a websocket server that discards inbound messages and +// returns a client connection to it. +func newWSConn(t *testing.T) *websocket.Conn { + upgrader := websocket.Upgrader{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + defer conn.Close() + for { + if _, _, err := conn.ReadMessage(); err != nil { + return + } + } + })) + t.Cleanup(server.Close) + + wsURL := "ws" + server.URL[4:] + conn, resp, err := websocket.DefaultDialer.Dial(wsURL, nil) + require.NoError(t, err) + resp.Body.Close() + t.Cleanup(func() { conn.Close() }) + return conn +} + +func TestBridgeSetTunnelConnSwapDuringWrites(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + w := &databricks.WorkspaceClient{} + + conn1 := newWSConn(t) + conn2 := newWSConn(t) + + vb := NewBridge(ctx, w, "test-app", 5173, false) + vb.tunnelConn.Store(conn1) + + writerDone := make(chan struct{}) + go func() { + defer close(writerDone) + // The writer may return an error if a queued write hits the + // just-closed old connection; this test only checks that the swap is + // race-free and closes the old connection. + _ = vb.tunnelWriter(ctx) + }() + + for i := range 100 { + vb.tunnelWriteChan <- prioritizedMessage{ + messageType: websocket.TextMessage, + data: []byte("payload"), + priority: 1, + } + if i == 50 { + // Simulate the reconnect path swapping in a fresh connection. + vb.setTunnelConn(conn2) + } + } + + close(vb.stopChan) + select { + case <-writerDone: + case <-time.After(5 * time.Second): + t.Fatal("tunnel writer did not stop") + } + + // setTunnelConn must close the connection it replaced. + err := conn1.WriteMessage(websocket.TextMessage, []byte("x")) + require.Error(t, err) +} + +func TestBridgeConnectionRequestSendDoesNotBlockAfterStop(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + w := &databricks.WorkspaceClient{} + + vb := NewBridge(ctx, w, "test-app", 5173, false) + + // Fill the queue so an unguarded send would block forever. + for range cap(vb.connectionRequests) { + vb.connectionRequests <- &BridgeMessage{Type: "connection:request"} + } + + vb.Stop() + + done := make(chan error, 1) + go func() { + done <- vb.handleMessage(&BridgeMessage{Type: "connection:request"}) + }() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("handleMessage blocked on a full connectionRequests queue after stop") + } +} + +func TestBridgeConnectionRequestSequentialPrompts(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + w := &databricks.WorkspaceClient{} + + vb := NewBridge(ctx, w, "test-app", 5173, false) + go vb.readStdinLines(strings.NewReader("y\nn\n")) + + // Both prompts must get their own line; the old per-prompt reader leaked a + // goroutine on timeout that could swallow the answer to the next prompt. + require.NoError(t, vb.handleConnectionRequest(&BridgeMessage{Type: "connection:request", Viewer: "a@example.com", RequestID: "req-1"})) + require.NoError(t, vb.handleConnectionRequest(&BridgeMessage{Type: "connection:request", Viewer: "b@example.com", RequestID: "req-2"})) + + var responses []BridgeMessage + for range 2 { + msg := <-vb.tunnelWriteChan + var response BridgeMessage + require.NoError(t, json.Unmarshal(msg.data, &response)) + responses = append(responses, response) + } + + assert.Equal(t, "connection:response", responses[0].Type) + assert.Equal(t, "req-1", responses[0].RequestID) + assert.True(t, responses[0].Approved) + assert.Equal(t, "connection:response", responses[1].Type) + assert.Equal(t, "req-2", responses[1].RequestID) + assert.False(t, responses[1].Approved) +} + func TestBridgeHandleConnectionRequest_AutoApproveSkipsStdin(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) w := &databricks.WorkspaceClient{} - var received []byte + received := make(chan []byte, 1) upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) @@ -387,7 +517,7 @@ func TestBridgeHandleConnectionRequest_AutoApproveSkipsStdin(t *testing.T) { t.Errorf("failed to read message: %v", err) return } - received = message + received <- message })) defer server.Close() @@ -398,7 +528,7 @@ func TestBridgeHandleConnectionRequest_AutoApproveSkipsStdin(t *testing.T) { defer conn.Close() vb := NewBridge(ctx, w, "test-app", 5173, true) - vb.tunnelConn = conn + vb.tunnelConn.Store(conn) go func() { _ = vb.tunnelWriter(ctx) }() @@ -410,10 +540,8 @@ func TestBridgeHandleConnectionRequest_AutoApproveSkipsStdin(t *testing.T) { require.NoError(t, vb.handleConnectionRequest(msg)) - time.Sleep(100 * time.Millisecond) - var response BridgeMessage - require.NoError(t, json.Unmarshal(received, &response)) + require.NoError(t, json.Unmarshal(waitForMessage(t, received), &response)) assert.Equal(t, "connection:response", response.Type) assert.Equal(t, "req-auto", response.RequestID) assert.True(t, response.Approved) diff --git a/libs/apps/vite/validate_dir_test.go b/libs/apps/vite/validate_dir_test.go index a2850c98f4f..3a1c810ab43 100644 --- a/libs/apps/vite/validate_dir_test.go +++ b/libs/apps/vite/validate_dir_test.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" "testing" - "time" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/databricks-sdk-go" @@ -123,7 +122,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) w := &databricks.WorkspaceClient{} - var lastMessage []byte + received := make(chan []byte, 1) upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) @@ -138,7 +137,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { t.Errorf("failed to read message: %v", err) return } - lastMessage = message + received <- message })) defer server.Close() @@ -149,7 +148,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { defer conn.Close() vb := NewBridge(ctx, w, "test-app", 5173, false) - vb.tunnelConn = conn + vb.tunnelConn.Store(conn) go func() { _ = vb.tunnelWriter(ctx) }() @@ -162,10 +161,8 @@ func TestBridgeHandleDirListRequest(t *testing.T) { err = vb.handleDirListRequest(msg) require.NoError(t, err) - time.Sleep(100 * time.Millisecond) - var response BridgeMessage - err = json.Unmarshal(lastMessage, &response) + err = json.Unmarshal(waitForMessage(t, received), &response) require.NoError(t, err) assert.Equal(t, "dir:list:response", response.Type) @@ -188,7 +185,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) w := &databricks.WorkspaceClient{} - var lastMessage []byte + received := make(chan []byte, 1) upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { conn, err := upgrader.Upgrade(w, r, nil) @@ -203,7 +200,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { t.Errorf("failed to read message: %v", err) return } - lastMessage = message + received <- message })) defer server.Close() @@ -214,7 +211,7 @@ func TestBridgeHandleDirListRequest(t *testing.T) { defer conn.Close() vb := NewBridge(ctx, w, "test-app", 5173, false) - vb.tunnelConn = conn + vb.tunnelConn.Store(conn) go func() { _ = vb.tunnelWriter(ctx) }() @@ -227,10 +224,8 @@ func TestBridgeHandleDirListRequest(t *testing.T) { err = vb.handleDirListRequest(msg) require.NoError(t, err) - time.Sleep(100 * time.Millisecond) - var response BridgeMessage - err = json.Unmarshal(lastMessage, &response) + err = json.Unmarshal(waitForMessage(t, received), &response) require.NoError(t, err) assert.Equal(t, "dir:list:response", response.Type) From 5c54ed262e9aec7a69c661a7da8d2405adbbd84a Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 23:40:49 +0200 Subject: [PATCH 2/2] Remove redundant comments Co-authored-by: Isaac --- libs/apps/vite/bridge.go | 16 +++++----------- libs/apps/vite/bridge_test.go | 12 +++--------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/libs/apps/vite/bridge.go b/libs/apps/vite/bridge.go index 5addd672b12..7b658e83c7d 100644 --- a/libs/apps/vite/bridge.go +++ b/libs/apps/vite/bridge.go @@ -239,8 +239,7 @@ func (vb *Bridge) connectToTunnel(appDomain *url.URL) error { return nil } -// setTunnelConn atomically installs a new tunnel connection and closes the -// previous one so a reconnect doesn't leak the old connection. +// setTunnelConn installs a new tunnel connection, closing the old one so reconnects don't leak it. func (vb *Bridge) setTunnelConn(conn *websocket.Conn) { if old := vb.tunnelConn.Swap(conn); old != nil { old.Close() @@ -421,8 +420,7 @@ func (vb *Bridge) handleMessage(msg *BridgeMessage) error { return nil case "connection:request": - // The consumer can be blocked on a stdin prompt (or gone after stop), so a - // bare send here could stall the tunnel reader and hang shutdown. + // The consumer may be blocked on a stdin prompt or gone after stop; a bare send could hang the tunnel reader. select { case vb.connectionRequests <- msg: return nil @@ -467,11 +465,9 @@ func (vb *Bridge) handleMessage(msg *BridgeMessage) error { } } -// readStdinLines forwards lines from r to stdinLines until read error or stop. -// A single persistent reader (rather than one goroutine per prompt) ensures a -// prompt that times out doesn't leak a goroutine that then swallows the user's -// answer to the next prompt. The channel is closed on read error so pending and -// future prompts fail instead of waiting for input that will never come. +// readStdinLines forwards lines from r to stdinLines, closing the channel on read +// error so prompts fail instead of hanging. A single persistent reader keeps a +// timed-out prompt from leaking a goroutine that swallows the next prompt's answer. func (vb *Bridge) readStdinLines(r io.Reader) { reader := bufio.NewReader(r) for { @@ -507,7 +503,6 @@ func (vb *Bridge) handleConnectionRequest(msg *BridgeMessage) error { } approved = strings.ToLower(strings.TrimSpace(input)) == "y" case <-vb.stopChan: - // Shutting down; no point answering the request. return nil case <-time.After(BridgeConnTimeout): // Default to denying after timeout @@ -985,7 +980,6 @@ func (vb *Bridge) Start() error { return nil }) - // Single persistent stdin reader for connection prompts; see readStdinLines. if !vb.autoApprove { go vb.readStdinLines(os.Stdin) } diff --git a/libs/apps/vite/bridge_test.go b/libs/apps/vite/bridge_test.go index 2b75d2db435..cadd70f0828 100644 --- a/libs/apps/vite/bridge_test.go +++ b/libs/apps/vite/bridge_test.go @@ -194,7 +194,6 @@ func TestBridgeHandleMessage(t *testing.T) { } // waitForMessage waits for the websocket test server to deliver a message. -// Tests must synchronize on a channel (not a sleep) so they pass under -race. func waitForMessage(t *testing.T, received <-chan []byte) []byte { select { case message := <-received: @@ -375,8 +374,7 @@ func TestNewBridge_AutoApprove(t *testing.T) { assert.True(t, vb.autoApprove) } -// newWSConn starts a websocket server that discards inbound messages and -// returns a client connection to it. +// newWSConn returns a client connection to a test server that discards inbound messages. func newWSConn(t *testing.T) *websocket.Conn { upgrader := websocket.Upgrader{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -414,9 +412,7 @@ func TestBridgeSetTunnelConnSwapDuringWrites(t *testing.T) { writerDone := make(chan struct{}) go func() { defer close(writerDone) - // The writer may return an error if a queued write hits the - // just-closed old connection; this test only checks that the swap is - // race-free and closes the old connection. + // A queued write may hit the just-closed old connection, so the error is ignored. _ = vb.tunnelWriter(ctx) }() @@ -427,7 +423,6 @@ func TestBridgeSetTunnelConnSwapDuringWrites(t *testing.T) { priority: 1, } if i == 50 { - // Simulate the reconnect path swapping in a fresh connection. vb.setTunnelConn(conn2) } } @@ -477,8 +472,7 @@ func TestBridgeConnectionRequestSequentialPrompts(t *testing.T) { vb := NewBridge(ctx, w, "test-app", 5173, false) go vb.readStdinLines(strings.NewReader("y\nn\n")) - // Both prompts must get their own line; the old per-prompt reader leaked a - // goroutine on timeout that could swallow the answer to the next prompt. + // Each prompt must consume exactly one line; a leaked reader would swallow the next prompt's answer. require.NoError(t, vb.handleConnectionRequest(&BridgeMessage{Type: "connection:request", Viewer: "a@example.com", RequestID: "req-1"})) require.NoError(t, vb.handleConnectionRequest(&BridgeMessage{Type: "connection:request", Viewer: "b@example.com", RequestID: "req-2"}))