From 58c26e1629d618981e517001aeeb654dc70deade Mon Sep 17 00:00:00 2001
From: rcourtman <8825017+rcourtman@users.noreply.github.com>
Date: Mon, 5 Oct 2026 09:57:11 +0100
Subject: [PATCH] Stop the SSE connect-snapshot test racing a queued duplicate

TestSSEBroadcaster_AddClientWritesCurrentStatusBeforeReturning failed
under -race in CI on unrelated frontend PRs (2479, 2485). The test
broadcast a stage and connected straight away. When the broadcast loop
picked that message up after the client had registered, it queued the
stage for the new client too, so the client's writer goroutine wrote
the httptest recorder while the test read it without a lock.

AddClient itself is correct: it writes the preamble and current status
before returning, and a stage racing the connect can reach the client
twice, which the update modal already ignores. The test now lets the
broadcast reach an earlier client before connecting, so nothing else
writes the recorder and the status in the body can only come from the
synchronous snapshot write.

Reproduced by injecting the CI schedule through -overlay (broadcast
loop held until the client registers, caller paused as AddClient
returns): the old test failed 199/200 runs with DATA RACE, the new one
0/200. A mutant that moves the snapshot write onto the writer goroutine
still fails the new test 100/100.
---
 internal/updates/sse_test.go | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/internal/updates/sse_test.go b/internal/updates/sse_test.go
index 15da113887..4e97fb6fc8 100644
--- a/internal/updates/sse_test.go
+++ b/internal/updates/sse_test.go
@@ -249,7 +249,23 @@ func TestSSEBroadcaster_AddClientWritesCurrentStatusBeforeReturning(t *testing.T
 	broadcaster := NewSSEBroadcaster()
 	defer broadcaster.Close()
 
+	// The stage was broadcast before this client connects, so let it reach
+	// an existing client first. While a broadcast is still in flight the
+	// broadcast loop can queue it for the new client as well, and that
+	// client's writer goroutine would then write the recorder concurrently
+	// with the unsynchronized reads below.
+	earlier := &syncFlushWriter{}
+	if client := broadcaster.AddClient(earlier, "client-earlier"); client == nil {
+		t.Fatal("AddClient returned nil")
+	}
 	broadcaster.Broadcast(UpdateStatus{Status: "extracting", Progress: 40, Message: "Extracting update..."})
+	deadline := time.Now().Add(2 * time.Second)
+	for !strings.Contains(earlier.String(), `"status":"extracting"`) {
+		if time.Now().After(deadline) {
+			t.Fatalf("broadcast never reached the earlier client: %q", earlier.String())
+		}
+		time.Sleep(time.Millisecond)
+	}
 
 	w := &mockFlushWriter{ResponseRecorder: httptest.NewRecorder()}
 	if client := broadcaster.AddClient(w, "client-now"); client == nil {