refactor(soundtouch-web): encapsulate WebApp device registry behind methods

The Devices map on WebApp was written from the startup goroutine, the
/api/discover POST handler, and addDevice, while being read from every
HTTP handler and the WebSocket periodic-update loop — all without any
mutex. The Go runtime panics with "fatal error: concurrent map writes"
or "concurrent map read and map write" on any actual collision, so this
was a latent crash, not a tearing issue.

Hide the map behind a sync.RWMutex and a small API:

  GetDevice(id) (*DeviceConnection, bool)
  DeviceSnapshot() []DeviceEntry
  DeviceCount() int
  AddDevice(id, conn) bool        // atomic insert-or-touch
  TouchDevice(id) bool            // fast-path LastSeen bump

Update every caller — handlers, websocket, main, tests — to go through
the API. addDevice's existing-host fast path uses TouchDevice; the
final insert uses AddDevice so a race with another writer is rejected
cleanly instead of silently overwriting.

Add a TestRegistryConcurrent stress test that runs 64 goroutines doing
12,800 operations across writers, touchers, and two reader patterns.
It exists to give `-race` (already on in CI) a concrete shape to catch
if the encapsulation ever leaks back out.

Struct-field races on conn.Status.* are not addressed by this change;
they need their own follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cmd/soundtouch-web/handlers/handlers.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/gesellix/bose-soundtouch/cmd/soundtouch-web/webtypes"
 	"github.com/gesellix/bose-soundtouch/pkg/models"
@@ -17,18 +18,34 @@ import (
 	"github.com/gorilla/websocket"
 )
 
-// WebApp holds the application state and dependencies
+// WebApp holds the application state and dependencies.
+//
+// The device registry (devices map + devicesMu) is encapsulated:
+// callers go through GetDevice / DeviceSnapshot / AddDevice /
+// TouchDevice / DeviceCount instead of touching the map directly.
+// This prevents the concurrent-map-read/write panic that would
+// otherwise be reachable any time an HTTP handler runs while
+// discovery or the /api/discover endpoint is registering devices.
 type WebApp struct {
-	Devices   map[string]*webtypes.DeviceConnection
+	devicesMu sync.RWMutex
+	devices   map[string]*webtypes.DeviceConnection
+
 	Upgrader  websocket.Upgrader
 	WSClients map[*websocket.Conn]bool
 	WSMutex   sync.RWMutex
 }
 
+// DeviceEntry pairs a device id with its connection. Used by
+// DeviceSnapshot so callers can iterate without holding the lock.
+type DeviceEntry struct {
+	ID     string
+	Device *webtypes.DeviceConnection
+}
+
 // NewWebApp creates a new WebApp instance for SPA mode
 func NewWebApp() *WebApp {
 	return &WebApp{
-		Devices:   make(map[string]*webtypes.DeviceConnection),
+		devices:   make(map[string]*webtypes.DeviceConnection),
 		WSClients: make(map[*websocket.Conn]bool),
 		Upgrader: websocket.Upgrader{
 			CheckOrigin: func(_ *http.Request) bool { return true },
@@ -36,17 +53,89 @@ func NewWebApp() *WebApp {
 	}
 }
 
+// GetDevice returns the device for id and whether it exists.
+func (app *WebApp) GetDevice(id string) (*webtypes.DeviceConnection, bool) {
+	app.devicesMu.RLock()
+	defer app.devicesMu.RUnlock()
+
+	device, ok := app.devices[id]
+
+	return device, ok
+}
+
+// DeviceSnapshot returns a list of (id, *DeviceConnection) pairs taken
+// under a single read lock. Callers can iterate the result without
+// holding any registry lock. Devices added or removed after the call
+// are not reflected; the pointers themselves remain valid because
+// nothing deletes from the underlying map today.
+func (app *WebApp) DeviceSnapshot() []DeviceEntry {
+	app.devicesMu.RLock()
+	defer app.devicesMu.RUnlock()
+
+	out := make([]DeviceEntry, 0, len(app.devices))
+	for id, device := range app.devices {
+		out = append(out, DeviceEntry{ID: id, Device: device})
+	}
+
+	return out
+}
+
+// DeviceCount returns the number of registered devices at call time.
+func (app *WebApp) DeviceCount() int {
+	app.devicesMu.RLock()
+	defer app.devicesMu.RUnlock()
+
+	return len(app.devices)
+}
+
+// AddDevice atomically registers conn under id when id is not already
+// known. If id existed, its LastSeen is bumped and AddDevice returns
+// false (the caller should discard conn). Returns true if conn was
+// inserted.
+func (app *WebApp) AddDevice(id string, conn *webtypes.DeviceConnection) bool {
+	app.devicesMu.Lock()
+	defer app.devicesMu.Unlock()
+
+	if existing, ok := app.devices[id]; ok {
+		existing.LastSeen = time.Now()
+		return false
+	}
+
+	app.devices[id] = conn
+
+	return true
+}
+
+// TouchDevice bumps LastSeen for id if it exists; returns true if
+// found. Use this as a fast-path check before doing the network work
+// needed to construct a new DeviceConnection.
+func (app *WebApp) TouchDevice(id string) bool {
+	app.devicesMu.Lock()
+	defer app.devicesMu.Unlock()
+
+	existing, ok := app.devices[id]
+	if !ok {
+		return false
+	}
+
+	existing.LastSeen = time.Now()
+
+	return true
+}
+
 // HandleAPIDevices returns all devices as JSON
 func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 
 	// Return all devices as JSON
-	devices := make(map[string]interface{})
-	for id, device := range app.Devices {
-		devices[id] = map[string]interface{}{
-			"info":     device.DeviceInfo,
-			"status":   device.Status,
-			"lastSeen": device.LastSeen,
+	snapshot := app.DeviceSnapshot()
+	devices := make(map[string]interface{}, len(snapshot))
+
+	for _, entry := range snapshot {
+		devices[entry.ID] = map[string]interface{}{
+			"info":     entry.Device.DeviceInfo,
+			"status":   entry.Device.Status,
+			"lastSeen": entry.Device.LastSeen,
 		}
 	}
 
@@ -68,7 +157,7 @@ func (app *WebApp) HandleAPIDevice(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -107,7 +196,7 @@ func (app *WebApp) HandleAPIControl(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -318,7 +407,7 @@ func (app *WebApp) HandleDeviceKey(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "id")
 	key := chi.URLParam(r, "key")
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -350,7 +439,7 @@ func (app *WebApp) HandleDirectVolumeControl(w http.ResponseWriter, r *http.Requ
 		return
 	}
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -376,7 +465,7 @@ func (app *WebApp) HandleDirectVolumeControl(w http.ResponseWriter, r *http.Requ
 func (app *WebApp) HandleDevicePower(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "id")
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -403,7 +492,7 @@ func (app *WebApp) HandleDevicePower(w http.ResponseWriter, r *http.Request) {
 func (app *WebApp) HandleDevicePowerStatus(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "id")
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, "Device not found", http.StatusNotFound)
 		return
@@ -444,12 +533,14 @@ func (app *WebApp) BroadcastDeviceList() {
 	app.WSMutex.RLock()
 	defer app.WSMutex.RUnlock()
 
-	devices := make(map[string]interface{})
-	for id, device := range app.Devices {
-		devices[id] = map[string]interface{}{
-			"info":     device.DeviceInfo,
-			"status":   device.Status,
-			"lastSeen": device.LastSeen,
+	snapshot := app.DeviceSnapshot()
+	devices := make(map[string]interface{}, len(snapshot))
+
+	for _, entry := range snapshot {
+		devices[entry.ID] = map[string]interface{}{
+			"info":     entry.Device.DeviceInfo,
+			"status":   entry.Device.Status,
+			"lastSeen": entry.Device.LastSeen,
 		}
 	}
 
@@ -598,7 +689,7 @@ func (app *WebApp) HandlePlayTuneIn(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		app.sendError(w, fmt.Sprintf("Device '%s' not found", deviceID), http.StatusNotFound)
 		return

cmd/soundtouch-web/handlers/handlers_test.go

@@ -40,7 +40,7 @@ func createTestApp() *WebApp {
 		},
 	}
 
-	app.Devices["test-device"] = device
+	app.AddDevice("test-device", device)
 	return app
 }
 
@@ -59,13 +59,9 @@ func TestNewWebApp(t *testing.T) {
 	if app == nil {
 		t.Fatal("NewWebApp returned nil")
 	}
-	if app.Devices == nil {
-		t.Fatal("Devices map not initialized")
-	}
 
-	// At this point we know app and app.Devices are not nil
-	if len(app.Devices) != 0 {
-		t.Errorf("Expected empty devices map, got %d devices", len(app.Devices))
+	if count := app.DeviceCount(); count != 0 {
+		t.Errorf("Expected empty device registry, got %d devices", count)
 	}
 }
 
@@ -549,11 +545,11 @@ func BenchmarkHandleAPIDevices(b *testing.B) {
 	// Add more devices for realistic benchmarking
 	for i := 0; i < 10; i++ {
 		deviceID := "device-" + string(rune('0'+i))
-		app.Devices[deviceID] = &webtypes.DeviceConnection{
+		app.AddDevice(deviceID, &webtypes.DeviceConnection{
 			Client:     &client.Client{},
 			DeviceInfo: &models.DeviceInfo{Name: "Test Device " + deviceID},
 			Status:     webtypes.DeviceStatus{IsConnected: true},
-		}
+		})
 	}
 
 	req := httptest.NewRequest("GET", "/api/devices", nil)

cmd/soundtouch-web/handlers/registry_test.go

@@ -0,0 +1,194 @@
+// Package handlers contains tests for the device registry API on
+// WebApp (GetDevice, AddDevice, TouchDevice, DeviceSnapshot,
+// DeviceCount).
+package handlers
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/cmd/soundtouch-web/webtypes"
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+)
+
+func newRegistryDevice(name string) *webtypes.DeviceConnection {
+	return &webtypes.DeviceConnection{
+		DeviceInfo: &models.DeviceInfo{Name: name},
+		Status:     webtypes.DeviceStatus{IsConnected: true},
+	}
+}
+
+func TestAddDevice_Inserts(t *testing.T) {
+	app := NewWebApp()
+	conn := newRegistryDevice("first")
+
+	if !app.AddDevice("host-1", conn) {
+		t.Fatal("AddDevice returned false on first insert")
+	}
+
+	got, ok := app.GetDevice("host-1")
+	if !ok {
+		t.Fatal("GetDevice did not find the device after AddDevice")
+	}
+
+	if got != conn {
+		t.Errorf("GetDevice returned a different pointer than inserted")
+	}
+}
+
+func TestAddDevice_RejectsDuplicateAndBumpsLastSeen(t *testing.T) {
+	app := NewWebApp()
+	original := newRegistryDevice("first")
+	app.AddDevice("host-1", original)
+
+	originalSeen := original.LastSeen
+	replacement := newRegistryDevice("second")
+
+	if app.AddDevice("host-1", replacement) {
+		t.Fatal("AddDevice returned true on duplicate; expected false")
+	}
+
+	got, _ := app.GetDevice("host-1")
+	if got != original {
+		t.Error("Duplicate AddDevice replaced the existing device pointer")
+	}
+
+	if !got.LastSeen.After(originalSeen) {
+		t.Error("Duplicate AddDevice did not bump LastSeen on existing device")
+	}
+}
+
+func TestTouchDevice(t *testing.T) {
+	app := NewWebApp()
+
+	if app.TouchDevice("missing") {
+		t.Error("TouchDevice returned true for unknown id")
+	}
+
+	conn := newRegistryDevice("first")
+	app.AddDevice("host-1", conn)
+	seenBefore := conn.LastSeen
+
+	if !app.TouchDevice("host-1") {
+		t.Fatal("TouchDevice returned false for known id")
+	}
+
+	if !conn.LastSeen.After(seenBefore) {
+		t.Error("TouchDevice did not bump LastSeen")
+	}
+}
+
+func TestDeviceSnapshotAndCount(t *testing.T) {
+	app := NewWebApp()
+
+	if got := app.DeviceCount(); got != 0 {
+		t.Errorf("DeviceCount on empty app = %d; want 0", got)
+	}
+
+	if snap := app.DeviceSnapshot(); len(snap) != 0 {
+		t.Errorf("DeviceSnapshot on empty app = %v; want []", snap)
+	}
+
+	for i := 0; i < 5; i++ {
+		app.AddDevice(fmt.Sprintf("host-%d", i), newRegistryDevice(fmt.Sprintf("n%d", i)))
+	}
+
+	if got := app.DeviceCount(); got != 5 {
+		t.Errorf("DeviceCount after 5 adds = %d; want 5", got)
+	}
+
+	snap := app.DeviceSnapshot()
+	if len(snap) != 5 {
+		t.Errorf("DeviceSnapshot len = %d; want 5", len(snap))
+	}
+
+	// Spot-check that the snapshot ids match what we inserted.
+	seen := map[string]bool{}
+	for _, entry := range snap {
+		seen[entry.ID] = true
+	}
+
+	for i := 0; i < 5; i++ {
+		id := fmt.Sprintf("host-%d", i)
+		if !seen[id] {
+			t.Errorf("DeviceSnapshot missing %s", id)
+		}
+	}
+}
+
+// TestRegistryConcurrent exercises the registry from many goroutines
+// at once. Before the introduction of devicesMu this would either
+// panic with "fatal error: concurrent map read and map write" or be
+// flagged by the race detector. The test runs under `go test -race`
+// in CI so a future regression that re-exposes the underlying map
+// without locking would be caught here.
+func TestRegistryConcurrent(t *testing.T) {
+	app := NewWebApp()
+
+	const workers = 16
+
+	const opsPerWorker = 200
+
+	var wg sync.WaitGroup
+	wg.Add(workers * 4)
+
+	// Writers: insert distinct ids across workers.
+	for w := 0; w < workers; w++ {
+		go func(worker int) {
+			defer wg.Done()
+
+			for i := 0; i < opsPerWorker; i++ {
+				id := fmt.Sprintf("w%d-%d", worker, i)
+				app.AddDevice(id, newRegistryDevice(id))
+			}
+		}(w)
+	}
+
+	// Touchers: bump LastSeen on a shared id (which may or may not
+	// exist yet — both branches are exercised).
+	for w := 0; w < workers; w++ {
+		go func() {
+			defer wg.Done()
+
+			for i := 0; i < opsPerWorker; i++ {
+				app.TouchDevice("shared")
+			}
+		}()
+	}
+
+	// Readers via snapshot.
+	for w := 0; w < workers; w++ {
+		go func() {
+			defer wg.Done()
+
+			for i := 0; i < opsPerWorker; i++ {
+				_ = app.DeviceSnapshot()
+			}
+		}()
+	}
+
+	// Readers via direct lookup.
+	for w := 0; w < workers; w++ {
+		go func() {
+			defer wg.Done()
+
+			for i := 0; i < opsPerWorker; i++ {
+				_, _ = app.GetDevice("shared")
+				_ = app.DeviceCount()
+			}
+		}()
+	}
+
+	wg.Wait()
+
+	// Sanity check: every writer inserted opsPerWorker devices, plus
+	// the "shared" entry was never AddDevice'd so should be absent.
+	if got, want := app.DeviceCount(), workers*opsPerWorker; got != want {
+		t.Errorf("DeviceCount after concurrent inserts = %d; want %d", got, want)
+	}
+
+	if _, ok := app.GetDevice("shared"); ok {
+		t.Error("shared device should not exist (only TouchDevice was called for it)")
+	}
+}

cmd/soundtouch-web/handlers/websocket.go

@@ -35,12 +35,14 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) {
 	app.WSMutex.Unlock()
 
 	// Send initial device list
-	devices := make(map[string]interface{})
-	for id, device := range app.Devices {
-		devices[id] = map[string]interface{}{
-			"info":     device.DeviceInfo,
-			"status":   device.Status,
-			"lastSeen": device.LastSeen,
+	snapshot := app.DeviceSnapshot()
+	devices := make(map[string]interface{}, len(snapshot))
+
+	for _, entry := range snapshot {
+		devices[entry.ID] = map[string]interface{}{
+			"info":     entry.Device.DeviceInfo,
+			"status":   entry.Device.Status,
+			"lastSeen": entry.Device.LastSeen,
 		}
 	}
 
@@ -88,12 +90,12 @@ func (app *WebApp) HandleWebSocket(w http.ResponseWriter, r *http.Request) {
 		}
 
 		// Send periodic status updates
-		for id, device := range app.Devices {
-			if device.Status.IsConnected {
+		for _, entry := range app.DeviceSnapshot() {
+			if entry.Device.Status.IsConnected {
 				statusMessage := webtypes.WebSocketMessage{
 					Type:     "status_update",
-					DeviceID: id,
-					Data:     device.Status,
+					DeviceID: entry.ID,
+					Data:     entry.Device.Status,
 				}
 
 				if err := conn.WriteJSON(statusMessage); err != nil {
@@ -230,7 +232,7 @@ func (app *WebApp) HandleDeviceWebSocket(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	device, exists := app.Devices[deviceID]
+	device, exists := app.GetDevice(deviceID)
 	if !exists {
 		http.Error(w, "Device not found", http.StatusNotFound)
 		return

cmd/soundtouch-web/main.go

@@ -103,7 +103,7 @@ func main() {
 				ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 				defer cancel()
 
-				webApp.BroadcastDiscoveryStatus("starting", len(webApp.Devices))
+				webApp.BroadcastDiscoveryStatus("starting", webApp.DeviceCount())
 
 				for _, host := range manualHosts {
 					addDevice(webApp, host, 8090, "manual")
@@ -111,7 +111,7 @@ func main() {
 
 				discoverDevices(ctx, webApp, discoveryService)
 
-				webApp.BroadcastDiscoveryStatus("completed", len(webApp.Devices))
+				webApp.BroadcastDiscoveryStatus("completed", webApp.DeviceCount())
 				webApp.BroadcastDeviceList()
 			}()
 
@@ -217,8 +217,8 @@ func resolveBindAddr(bindAddr string) (string, error) {
 // mDNS/UPnP. If the host is already known, the existing entry's
 // LastSeen is bumped and the function returns without re-fetching.
 func addDevice(app *handlers.WebApp, host string, port int, source string) {
-	if existing, ok := app.Devices[host]; ok {
-		existing.LastSeen = time.Now()
+	// Fast path: skip the network call if we already know this host.
+	if app.TouchDevice(host) {
 		return
 	}
 
@@ -243,7 +243,12 @@ func addDevice(app *handlers.WebApp, host string, port int, source string) {
 			LastActivity: time.Now(),
 		},
 	}
-	app.Devices[host] = conn
+	if !app.AddDevice(host, conn) {
+		// Lost a race — another goroutine inserted the same host
+		// between TouchDevice and AddDevice. AddDevice bumped LastSeen
+		// on the existing entry; discard our conn.
+		return
+	}
 
 	go app.UpdateDeviceStatus(host, conn)
 
@@ -280,12 +285,12 @@ func setupRoutes(app *handlers.WebApp, discoveryService *discovery.UnifiedDiscov
 			defer cancel()
 
 			// Broadcast discovery start
-			app.BroadcastDiscoveryStatus("starting", len(app.Devices))
+			app.BroadcastDiscoveryStatus("starting", app.DeviceCount())
 
 			discoverDevices(ctx, app, discoveryService)
 
 			// Broadcast discovery completion and updated device list
-			app.BroadcastDiscoveryStatus("completed", len(app.Devices))
+			app.BroadcastDiscoveryStatus("completed", app.DeviceCount())
 			app.BroadcastDeviceList()
 		}()
 	})
@@ -321,7 +326,7 @@ func discoverDevices(ctx context.Context, app *handlers.WebApp, discoveryService
 	devices, err := discoveryService.DiscoverDevices(ctx)
 	if err != nil {
 		log.Printf("Discovery failed: %v", err)
-		app.BroadcastDiscoveryStatus("failed", len(app.Devices))
+		app.BroadcastDiscoveryStatus("failed", app.DeviceCount())
 
 		return
 	}

cmd/soundtouch-web/spa_test.go

@@ -256,7 +256,7 @@ func TestControlAPIValidation(t *testing.T) {
 		LastSeen:   time.Now(),
 		Status:     webtypes.DeviceStatus{IsConnected: true},
 	}
-	app.Devices["testdevice"] = mockDevice
+	app.AddDevice("testdevice", mockDevice)
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {