feat(web): remove devices from the player UI (refs #451)

The merge of soundtouch-web into soundtouch-service was asymmetric:
manual device *adds* propagated to the player UI (HandleAddManualDevice
notifies, the hook re-seeds + broadcasts), but *removals* did not. The
datastore-removal handler never notified, and the web registry's sync
only ever added entries — its map was append-only, so a removed device
lingered in the player UI until restart.

This adds the missing removal path:

- DELETE /api/control/devices/{id} (HandleDeleteDevice). The registry is
  keyed by host/IP; the datastore by device ID (MAC), so the handler
  resolves one to the other via the connection's DeviceInfo, cascades to
  the datastore through a new RemoveDeviceHook (embedded build only),
  prunes the in-memory entry, and broadcasts the updated list.
- WebApp.RemoveDevice prunes the registry and stops the per-device
  goroutines (status poller + WebSocket reconnect loop) via a new
  done-channel + Close() on DeviceConnection — previously both ran for
  the life of the process.
- Server.RemoveDeviceByID extracts the cross-account lookup + remove from
  HandleRemoveDevice and now fires notifyDevicesChanged, so the admin
  Devices tab removal also propagates to the player UI.
- Player UI: a quiet per-card Remove control (visible on hover), a
  confirm dialog, optimistic prune, and a note that a still-online
  device may reappear after the next discovery scan (honest v1 — no
  ignore-list).

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

pkg/service/handlers/handlers_device_remove_test.go

@@ -0,0 +1,72 @@
+package handlers
+
+import (
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+// TestRemoveDeviceByID verifies the extracted removal helper deletes the
+// device from the datastore, fires the devices-changed hook (so the
+// embedded web UI re-syncs), and reports not-found without firing the
+// hook when no matching device exists.
+func TestRemoveDeviceByID(t *testing.T) {
+	ds := datastore.NewDataStore(t.TempDir())
+	server := NewServer(ds, nil, "http://127.0.0.1:8000", false, false, false)
+
+	const (
+		account  = "1000001"
+		deviceID = "DEVICEID01"
+	)
+
+	if err := ds.SaveDeviceInfo(account, deviceID, &models.ServiceDeviceInfo{
+		DeviceID:  deviceID,
+		Name:      "Test Speaker",
+		IPAddress: "192.0.2.10",
+	}); err != nil {
+		t.Fatalf("SaveDeviceInfo: %v", err)
+	}
+
+	var hookFired int
+
+	server.SetDevicesChangedHook(func() { hookFired++ })
+
+	found, err := server.RemoveDeviceByID(deviceID)
+	if err != nil {
+		t.Fatalf("RemoveDeviceByID: %v", err)
+	}
+
+	if !found {
+		t.Fatal("RemoveDeviceByID reported the device as not found")
+	}
+
+	if hookFired != 1 {
+		t.Errorf("devices-changed hook fired %d times; want 1", hookFired)
+	}
+
+	devices, err := ds.ListAllDevices()
+	if err != nil {
+		t.Fatalf("ListAllDevices: %v", err)
+	}
+
+	for i := range devices {
+		if devices[i].DeviceID == deviceID {
+			t.Error("device still present in datastore after removal")
+		}
+	}
+
+	// A second removal finds nothing and must not fire the hook again.
+	found, err = server.RemoveDeviceByID(deviceID)
+	if err != nil {
+		t.Fatalf("RemoveDeviceByID (second call): %v", err)
+	}
+
+	if found {
+		t.Error("RemoveDeviceByID reported a removed device as found")
+	}
+
+	if hookFired != 1 {
+		t.Errorf("devices-changed hook fired %d times after no-op removal; want 1", hookFired)
+	}
+}

pkg/service/handlers/handlers_setup.go

@@ -101,6 +101,34 @@ func (s *Server) HandleGetDiscoveryStatus(w http.ResponseWriter, _ *http.Request
 	}
 }
 
+// RemoveDeviceByID removes the device with the given device ID (MAC) from
+// the datastore, searching across all accounts. It returns whether a
+// matching device was found. On a successful removal it notifies
+// observers (e.g. the embedded web UI) so they re-sync — the symmetric
+// counterpart to the notify in HandleAddManualDevice.
+func (s *Server) RemoveDeviceByID(deviceID string) (bool, error) {
+	devices, err := s.ds.ListAllDevices()
+	if err != nil {
+		return false, err
+	}
+
+	for i := range devices {
+		if devices[i].DeviceID == deviceID {
+			if err := s.ds.RemoveDevice(devices[i].AccountID, devices[i].DeviceID); err != nil {
+				return false, err
+			}
+
+			// Let any observer (e.g. the embedded web UI) re-sync from the
+			// datastore so the removal propagates to the player UI.
+			s.notifyDevicesChanged()
+
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
 // HandleRemoveDevice removes a device from the datastore.
 func (s *Server) HandleRemoveDevice(w http.ResponseWriter, r *http.Request) {
 	deviceId := chi.URLParam(r, "deviceId")
@@ -109,29 +137,12 @@ func (s *Server) HandleRemoveDevice(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Find which account this device belongs to.
-	devices, err := s.ds.ListAllDevices()
+	found, err := s.RemoveDeviceByID(deviceId)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
-	var found bool
-
-	for i := range devices {
-		if devices[i].DeviceID == deviceId {
-			err = s.ds.RemoveDevice(devices[i].AccountID, devices[i].DeviceID)
-			if err != nil {
-				http.Error(w, err.Error(), http.StatusInternalServerError)
-				return
-			}
-
-			found = true
-
-			break
-		}
-	}
-
 	if !found {
 		http.Error(w, "Device not found", http.StatusNotFound)
 		return

pkg/service/soundtouchweb/discovery.go

@@ -78,8 +78,13 @@ func (app *WebApp) AddDeviceByHost(host string, port int, source string) {
 		ticker := time.NewTicker(30 * time.Second)
 		defer ticker.Stop()
 
-		for range ticker.C {
-			app.UpdateDeviceStatus(host, conn)
+		for {
+			select {
+			case <-ticker.C:
+				app.UpdateDeviceStatus(host, conn)
+			case <-conn.Done():
+				return
+			}
 		}
 	}()
 

pkg/service/soundtouchweb/handler.go

@@ -63,6 +63,13 @@ type WebApp struct {
 	// soundtouch-web leaves it nil and runs its own sweep.
 	TriggerDiscovery func(ctx context.Context)
 
+	// RemoveDeviceHook, when set, removes a device from the backing store by
+	// its device ID (MAC). The embedded build wires it to the service's
+	// datastore removal so a removal from the player UI also clears the
+	// persisted device; standalone soundtouch-web leaves it nil (no store, so
+	// removal only prunes the in-memory registry).
+	RemoveDeviceHook func(deviceID string) error
+
 	discoveryStatus atomic.Value // stores *webtypes.DiscoveryStatus
 }
 
@@ -108,8 +115,9 @@ func (app *WebApp) GetDevice(id string) (*webtypes.DeviceConnection, bool) {
 // 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.
+// are not reflected. A pointer captured here stays valid even if the
+// device is later removed (RemoveDevice only detaches it from the map
+// and stops its goroutines), so iterating a stale snapshot is safe.
 func (app *WebApp) DeviceSnapshot() []DeviceEntry {
 	app.devicesMu.RLock()
 	defer app.devicesMu.RUnlock()
@@ -165,6 +173,27 @@ func (app *WebApp) TouchDevice(id string) bool {
 	return true
 }
 
+// RemoveDevice removes the device registered under id and stops its
+// background goroutines (status poller + WebSocket reconnect loop) via
+// conn.Close. Returns true if id was present. Close runs outside the
+// registry lock because it performs network I/O (WebSocket disconnect).
+func (app *WebApp) RemoveDevice(id string) bool {
+	app.devicesMu.Lock()
+
+	conn, ok := app.devices[id]
+	if ok {
+		delete(app.devices, id)
+	}
+
+	app.devicesMu.Unlock()
+
+	if ok {
+		conn.Close()
+	}
+
+	return ok
+}
+
 // HandleAPIDevices returns all devices as JSON
 func (app *WebApp) HandleAPIDevices(w http.ResponseWriter, _ *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
@@ -228,6 +257,52 @@ func (app *WebApp) HandleAPIDevice(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// HandleDeleteDevice removes a device from the registry and, in the
+// embedded build, from the service datastore. The registry is keyed by
+// host/IP (the {id} URL param); the datastore is keyed by device ID
+// (MAC), so we resolve one to the other via the connection's DeviceInfo
+// before cascading. A device still live on the network is re-discovered
+// on the next sweep — removal is "remove now", not a permanent ban.
+func (app *WebApp) HandleDeleteDevice(w http.ResponseWriter, r *http.Request) {
+	host := chi.URLParam(r, "id")
+	if host == "" {
+		app.sendError(w, "Device ID required", http.StatusBadRequest)
+		return
+	}
+
+	conn, exists := app.GetDevice(host)
+	if !exists {
+		app.sendError(w, "Device not found", http.StatusNotFound)
+		return
+	}
+
+	// Cascade to the backing store (embedded build only). Standalone
+	// soundtouch-web has no datastore and leaves the hook nil, so removal
+	// only prunes the in-memory registry below.
+	if app.RemoveDeviceHook != nil {
+		deviceID := ""
+		if conn.DeviceInfo != nil {
+			deviceID = conn.DeviceInfo.DeviceID
+		}
+
+		if err := app.RemoveDeviceHook(deviceID); err != nil {
+			log.Printf("Failed to remove device %s from store: %v", sanitizeLog(host), err)
+			app.sendError(w, "Failed to remove device from store", http.StatusBadGateway)
+
+			return
+		}
+	}
+
+	app.RemoveDevice(host)
+	app.BroadcastDeviceList()
+
+	w.Header().Set("Content-Type", "application/json")
+
+	if err := json.NewEncoder(w).Encode(webtypes.APIResponse{Success: true}); err != nil {
+		http.Error(w, "Failed to encode response", http.StatusInternalServerError)
+	}
+}
+
 // HandleAPIControl handles device control commands
 func (app *WebApp) HandleAPIControl(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "id")

pkg/service/soundtouchweb/mount.go

@@ -63,6 +63,7 @@ func (app *WebApp) MountWeb(r chi.Router, discoveryService *discovery.UnifiedDis
 
 			r.Route("/{id}", func(r chi.Router) {
 				r.Get("/", app.HandleAPIDevice)
+				r.Delete("/", app.HandleDeleteDevice)
 				r.Post("/key/{key}", app.HandleDeviceKey)
 				r.Post("/volume/{volume}", app.HandleDirectVolumeControl)
 				r.Post("/power", app.HandleDevicePower)

pkg/service/soundtouchweb/registry_test.go

@@ -117,6 +117,81 @@ func TestDeviceSnapshotAndCount(t *testing.T) {
 	}
 }
 
+func TestRemoveDevice(t *testing.T) {
+	app := NewWebApp()
+
+	if app.RemoveDevice("missing") {
+		t.Error("RemoveDevice returned true for unknown id")
+	}
+
+	conn := newRegistryDevice("first")
+	app.AddDevice("host-1", conn)
+
+	if !app.RemoveDevice("host-1") {
+		t.Fatal("RemoveDevice returned false for known id")
+	}
+
+	if _, ok := app.GetDevice("host-1"); ok {
+		t.Error("device still present after RemoveDevice")
+	}
+
+	if got := app.DeviceCount(); got != 0 {
+		t.Errorf("DeviceCount after removal = %d; want 0", got)
+	}
+
+	// Removing the same id again is a no-op.
+	if app.RemoveDevice("host-1") {
+		t.Error("RemoveDevice returned true on second removal")
+	}
+
+	// The connection's done channel must be closed so its background
+	// goroutines (status poll, WebSocket reconnect) stop.
+	select {
+	case <-conn.Done():
+	default:
+		t.Error("RemoveDevice did not close the connection's Done channel")
+	}
+}
+
+// TestRemoveDeviceConcurrent runs adds and removes of the same ids from
+// many goroutines under `go test -race` to confirm the registry stays
+// race-free when removal is in the mix.
+func TestRemoveDeviceConcurrent(t *testing.T) {
+	app := NewWebApp()
+
+	const (
+		workers      = 16
+		opsPerWorker = 200
+	)
+
+	var wg sync.WaitGroup
+
+	wg.Add(workers * 2)
+
+	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)
+	}
+
+	for w := 0; w < workers; w++ {
+		go func(worker int) {
+			defer wg.Done()
+
+			for i := 0; i < opsPerWorker; i++ {
+				app.RemoveDevice(fmt.Sprintf("w%d-%d", worker, i))
+			}
+		}(w)
+	}
+
+	wg.Wait()
+}
+
 // 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

pkg/service/soundtouchweb/static/css/app.css

@@ -312,8 +312,35 @@ img { display: block; max-width: 100%; }
 }
 .device-card:hover { box-shadow: 0 4px 12px rgba(0,0,0,.12); transform: translateY(-1px); }
 
-.device-header { display: flex; align-items: center; justify-content: space-between; margin-bottom: .25rem; }
+.device-header { display: flex; align-items: center; justify-content: space-between; margin-bottom: .25rem; gap: .5rem; }
 .device-name { font-weight: 600; font-size: .95rem; }
+.device-header-right { display: flex; align-items: center; gap: .5rem; flex-shrink: 0; }
+
+/* Quiet remove affordance: invisible until the card is hovered, then dim,
+   warming to red only on its own hover. Keeps the grid relaxed. */
+.device-remove {
+    border: 0;
+    background: none;
+    padding: 0;
+    width: 1.1rem;
+    height: 1.1rem;
+    line-height: 1;
+    font-size: .8rem;
+    color: var(--text-dim);
+    cursor: pointer;
+    opacity: 0;
+    transition: opacity .15s, color .15s;
+}
+.device-card:hover .device-remove { opacity: .55; }
+.device-remove:hover { opacity: 1; color: var(--offline); }
+.device-remove:focus-visible { opacity: 1; outline: 2px solid var(--offline); outline-offset: 2px; }
+
+.device-list-note {
+    margin: .9rem .15rem 0;
+    font-size: .78rem;
+    color: var(--text-dim);
+    line-height: 1.4;
+}
 .device-type { font-size: .8rem; color: var(--text-dim); margin-bottom: .5rem; display: flex; gap: .4rem; flex-wrap: wrap; }
 .device-ip { color: var(--text); font-family: monospace; font-weight: 500; }
 

pkg/service/soundtouchweb/static/js/api.js

@@ -8,6 +8,7 @@ async function req(url, opts = {}) {
 export const api = {
     devices: () => req('/api/control/devices'),
     device: (id) => req(`/api/control/devices/${id}`),
+    removeDevice: (id) => req(`/api/control/devices/${id}`, { method: 'DELETE' }),
     discover: () => req('/api/control/discover', { method: 'POST' }),
     key: (id, key) => req(`/api/control/devices/${id}/key/${key}`, { method: 'POST' }),
     volume: (id, level) => req(`/api/control/devices/${id}/volume/${level}`, { method: 'POST' }),

pkg/service/soundtouchweb/static/js/app.js

@@ -148,6 +148,25 @@ function App() {
         await api.discover();
     }
 
+    async function removeDevice(id) {
+        const name = devices[id]?.info?.name || id;
+        if (!confirm(`Remove "${name}"?\n\nThis clears it from AfterTouch. A device still online may reappear after the next discovery scan.`)) {
+            return;
+        }
+        // Optimistically drop it; the server's devices broadcast reconciles.
+        setDevices(prev => {
+            const next = { ...prev };
+            delete next[id];
+            return next;
+        });
+        try {
+            const resp = await api.removeDevice(id);
+            showToast(resp?.success ? `Removed "${name}"` : (resp?.error || 'Failed to remove device'));
+        } catch (err) {
+            showToast('Failed to remove device');
+        }
+    }
+
     return html`
         <div class="app">
             <nav class="navbar">
@@ -209,6 +228,7 @@ function App() {
                         isDiscovering=${isDiscovering}
                         onSelect=${(id) => navigate('device', id)}
                         onDiscover=${discover}
+                        onRemove=${removeDevice}
                     />
                 ` : page === 'device' ? html`
                     <${DeviceDetail}

pkg/service/soundtouchweb/static/js/components/DeviceList.js

@@ -3,7 +3,7 @@ import htm from 'htm';
 
 const html = htm.bind(h);
 
-function DeviceCard({ id, device, onSelect }) {
+function DeviceCard({ id, device, onSelect, onRemove }) {
     const { info, status } = device;
     const np = status?.nowPlaying;
     const isPlaying = np?.PlayStatus === 'PLAY_STATE';
@@ -13,7 +13,12 @@ function DeviceCard({ id, device, onSelect }) {
         <div class="device-card" onClick=${() => onSelect(id)}>
             <div class="device-header">
                 <span class="device-name">${info?.name || id}</span>
-                <span class="device-indicator ${status?.isConnected ? 'online' : 'offline'}"></span>
+                <span class="device-header-right">
+                    <span class="device-indicator ${status?.isConnected ? 'online' : 'offline'}"></span>
+                    <button class="device-remove" title="Remove this device"
+                            aria-label="Remove this device"
+                            onClick=${(e) => { e.stopPropagation(); onRemove(id); }}>✕</button>
+                </span>
             </div>
             <div class="device-type">
                 ${info?.type || ''}
@@ -31,7 +36,7 @@ function DeviceCard({ id, device, onSelect }) {
     `;
 }
 
-export function DeviceList({ devices, isDiscovering, onSelect, onDiscover }) {
+export function DeviceList({ devices, isDiscovering, onSelect, onDiscover, onRemove }) {
     const entries = Object.entries(devices);
 
     return html`
@@ -48,9 +53,13 @@ export function DeviceList({ devices, isDiscovering, onSelect, onDiscover }) {
             : html`
                 <div class="device-grid" key="grid">
                     ${entries.map(([id, device]) => html`
-                        <${DeviceCard} key=${id} id=${id} device=${device} onSelect=${onSelect} />
+                        <${DeviceCard} key=${id} id=${id} device=${device} onSelect=${onSelect} onRemove=${onRemove} />
                     `)}
-                </div>`
+                </div>
+                <p class="device-list-note" key="note">
+                    Removing a device clears it here. One that is still online may
+                    reappear after the next discovery scan.
+                </p>`
         }
         </div>
     `;

pkg/service/soundtouchweb/websocket.go

@@ -167,6 +167,13 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device
 	var prevSource string
 
 	for {
+		// Stop if the device was removed from the registry (conn.Close()).
+		select {
+		case <-conn.Done():
+			return
+		default:
+		}
+
 		wsClient := conn.Client.NewWebSocketClient(nil)
 
 		// Setup event handlers. Each handler funnels its change through
@@ -213,7 +220,10 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device
 
 		if err := wsClient.Connect(); err != nil {
 			log.Printf("Failed to connect WebSocket for device %s: %v (retrying in %s)", sanitizeLog(deviceID), err, backoff)
-			time.Sleep(backoff)
+
+			if sleepOrDone(conn, backoff) {
+				return
+			}
 
 			backoff *= 2
 			if backoff > maxBackoff {
@@ -248,7 +258,10 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device
 		})
 
 		log.Printf("WebSocket disconnected for device %s — reconnecting in %s", sanitizeLog(deviceID), backoff)
-		time.Sleep(backoff)
+
+		if sleepOrDone(conn, backoff) {
+			return
+		}
 
 		backoff *= 2
 		if backoff > maxBackoff {
@@ -257,6 +270,21 @@ func (app *WebApp) ConnectDeviceWebSocket(deviceID string, conn *webtypes.Device
 	}
 }
 
+// sleepOrDone waits for d to elapse or for the connection to be closed,
+// whichever comes first. It returns true if the connection was closed
+// (the caller should stop), false if the timer fired normally.
+func sleepOrDone(conn *webtypes.DeviceConnection, d time.Duration) bool {
+	timer := time.NewTimer(d)
+	defer timer.Stop()
+
+	select {
+	case <-timer.C:
+		return false
+	case <-conn.Done():
+		return true
+	}
+}
+
 // UpdateDeviceStatus fetches current status from the device.
 //
 // Network calls run outside the atomic merge so the CAS loop in

pkg/service/soundtouchweb/webtypes/types.go

@@ -2,6 +2,7 @@
 package webtypes
 
 import (
+	"sync"
 	"sync/atomic"
 	"time"
 
@@ -45,6 +46,13 @@ type DeviceConnection struct {
 	LastSeen   time.Time
 
 	status atomic.Pointer[DeviceStatus]
+
+	// done is closed by Close when the device is removed from the
+	// registry, signalling its background goroutines (the status poller
+	// and the WebSocket reconnect loop) to exit. closeOnce keeps Close
+	// idempotent.
+	done      chan struct{}
+	closeOnce sync.Once
 }
 
 // DeviceStatus represents the current device state
@@ -66,6 +74,7 @@ func NewDeviceConnection(c *client.Client, info *models.DeviceInfo) *DeviceConne
 		Client:     c,
 		DeviceInfo: info,
 		LastSeen:   time.Now(),
+		done:       make(chan struct{}),
 	}
 	conn.status.Store(&DeviceStatus{
 		IsConnected:  false,
@@ -84,6 +93,28 @@ func (c *DeviceConnection) Status() *DeviceStatus {
 	return c.status.Load()
 }
 
+// Done returns a channel that is closed when the connection is removed
+// from the registry. The per-device status poller and WebSocket
+// reconnect loop select on it to stop instead of running for the life
+// of the process.
+func (c *DeviceConnection) Done() <-chan struct{} {
+	return c.done
+}
+
+// Close signals the connection's background goroutines to stop and best-
+// effort disconnects the WebSocket so a blocked reconnect loop wakes
+// promptly. Idempotent; safe to call on a connection that never started
+// any goroutine (e.g. a test connection with a nil Client).
+func (c *DeviceConnection) Close() {
+	c.closeOnce.Do(func() {
+		close(c.done)
+
+		if c.WebSocket != nil {
+			_ = c.WebSocket.Disconnect()
+		}
+	})
+}
+
 // SetStatus atomically replaces the entire status. Use sparingly —
 // UpdateStatus is the preferred entry point because it preserves
 // concurrent changes from other goroutines.