fix(player): mark sources stale on a failed read, not on a timer

The source inventory was expired by a 30s read-time TTL evaluated inside
DeviceConnection.Status(). The status poll ticker is also exactly 30s
(discovery.go), so the two ran in lockstep and every source button went
disabled for the moment before each refresh.

Deriving staleness per read had a second cost: two reads at the same
Revision could disagree about sourcesStale, which is the only reason the
browser needed mergeDerivedStatus to carry that one bit across an
otherwise-rejected frame. Dropping the TTL removes that whole special
case, and with it a class of "equal revision, different derived state"
reasoning.

Sources are now stale exactly when the last /sources read failed, recorded
at merge time so every change advances Revision. An unreachable device
fails its polls, so the case the TTL was meant to cover is still covered.
The newer-failure-fences-older-success ordering is unchanged: the merge
still runs through CompleteFieldPoll(FieldSources, ...).

Drops SourcesReadAt, sourceCacheTTL, sourceCacheStatusAt, the Status()
projection and mergeDerivedStatus. The TTL-boundary tests go with them;
the fencing and recovery tests stay, and one of them no longer has to pin
its read times to the wall clock to avoid the TTL.

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

pkg/service/soundtouchweb/browser_compatibility_test.go

@@ -232,8 +232,15 @@ window.revisionChecks = {
   equalRejected: equal === snapshot,
   olderRejected: older === equal,
   newerAccepted: newer !== older && newer.speaker.status.revision === 6 && newer.speaker.status.nowPlaying.Track === 'newest',
-  derivedStaleAccepted: stale !== newer && stale.speaker.status.sourcesStale === true &&
-    stale.speaker.status.nowPlaying.Track === 'newest',
+  staleAtEqualRevisionRejected: stale === newer,
+  staleAtNewerRevisionAccepted: (() => {
+    const applied = mergeStatusUpdate(newer, 'speaker', {
+      revision: 7,
+      sourcesStale: true,
+      nowPlaying: { Track: 'newest' },
+    });
+    return applied !== newer && applied.speaker.status.sourcesStale === true;
+  })(),
   staleCannotClearAtEqualRevision: mergeStatusUpdate(stale, 'speaker', {
     revision: 6,
     sourcesStale: false,
@@ -267,7 +274,10 @@ window.revisionChecks = {
 	}
 }
 
-func TestDerivedSourceExpiryDisablesCommandsAtEqualRevision(t *testing.T) {
+// TestSourceExpiryDisablesCommandsOnNewerRevision: a stale marker pushed over
+// the socket must reach the buttons and make them unclickable, without the
+// projection ever showing the source as selected.
+func TestSourceExpiryDisablesCommandsOnNewerRevision(t *testing.T) {
 	const sourceExpiryFixture = `
 import { h, render } from 'preact';
 import { mergeStatusUpdate } from '/app/static/js/app.js';
@@ -293,11 +303,11 @@ function redraw() {
 }
 window.expireSources = () => {
   devices = mergeStatusUpdate(devices, 'speaker', {
-    revision: 5,
+    revision: 6,
     nowPlayingRevision: 5,
     sourcesStale: true,
     sources: { SourceItem: ready },
-    nowPlaying: { Source: 'AUX', SourceAccount: 'AUX1' },
+    nowPlaying: { Source: 'STANDBY', SourceAccount: '' },
   });
   redraw();
 };
@@ -333,7 +343,7 @@ redraw();
 	mu.Lock()
 	defer mu.Unlock()
 	if writes != 0 || trackSource != "STANDBY" {
-		t.Fatalf("derived expiry writes=%d projected source=%q, want 0 and STANDBY", writes, trackSource)
+		t.Fatalf("stale source expiry writes=%d projected source=%q, want 0 and STANDBY", writes, trackSource)
 	}
 }
 

pkg/service/soundtouchweb/source_cache_test.go

@@ -2,11 +2,9 @@ package soundtouchweb
 
 import (
 	"encoding/json"
-	"errors"
 	"net/http"
 	"net/http/httptest"
 	"testing"
-	"time"
 
 	"github.com/gesellix/bose-soundtouch/pkg/client"
 	"github.com/gesellix/bose-soundtouch/pkg/models"
@@ -44,38 +42,57 @@ func TestUpdateDeviceStatusDoesNotRefreshNowPlayingRevisionOnFailure(t *testing.
 	}
 }
 
-// TestUpdateSourcesCacheRetainsFailureAndRefreshesSuccess: a failed refresh
-// keeps the previous inventory and its read time but marks it stale; the next
-// success replaces both and restores actionability.
-func TestUpdateSourcesCacheRetainsFailureAndRefreshesSuccess(t *testing.T) {
-	conn := webtypes.NewDeviceConnection(nil, nil)
-	// Read times must sit inside sourceCacheTTL of now: Status() derives
-	// staleness against the wall clock, so fixed calendar dates would read
-	// back as expired no matter what this test does.
-	firstRead := time.Now()
-	oldSources := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}}
+// TestUpdateDeviceStatusMarksSourcesStaleOnFailedRead: unlike every other
+// field, a FAILED /sources read is still merged -- the last known inventory
+// stays visible but is marked unusable, because offering source buttons the
+// speaker no longer confirms is worse than offering none.
+func TestUpdateDeviceStatusMarksSourcesStaleOnFailedRead(t *testing.T) {
+	sourcesOK := true
+	speaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/sources" {
+			if !sourcesOK {
+				http.Error(w, "unavailable", http.StatusServiceUnavailable)
 
-	updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), oldSources, nil, firstRead)
+				return
+			}
 
-	if !updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), nil,
-		errors.New("temporary read failure"), firstRead.Add(time.Second)) {
-		t.Fatal("failed source readback was not recorded")
+			w.Header().Set("Content-Type", "application/xml")
+			_, _ = w.Write([]byte(`<sources><sourceItem source="AUX" sourceAccount="AUX1" status="READY" isLocal="true">Aux 1</sourceItem></sources>`))
+
+			return
+		}
+
+		w.Header().Set("Content-Type", "application/xml")
+		_, _ = w.Write([]byte(`<volume><targetvolume>35</targetvolume><actualvolume>35</actualvolume><muteenabled>false</muteenabled></volume>`))
+	}))
+	defer speaker.Close()
+
+	app := NewWebApp()
+	conn := webtypes.NewDeviceConnection(client.NewClient(&client.Config{Host: speaker.URL}), nil)
+
+	app.UpdateDeviceStatus("speaker", conn)
+
+	fresh := conn.Status()
+	if fresh.SourcesStale || fresh.Sources == nil || len(fresh.Sources.SourceItem) != 1 {
+		t.Fatalf("successful source read was not merged as actionable: %+v", fresh)
 	}
 
-	status := conn.Status()
-	if status.Sources != oldSources || !status.SourcesReadAt.Equal(firstRead) || !status.SourcesStale {
-		t.Fatalf("failed source readback changed the cache: %+v", status)
+	sourcesOK = false
+	app.UpdateDeviceStatus("speaker", conn)
+
+	stale := conn.Status()
+	if !stale.SourcesStale {
+		t.Fatalf("failed source read did not mark the inventory stale: %+v", stale)
+	}
+	if stale.Sources != fresh.Sources {
+		t.Fatalf("failed source read discarded the last known inventory: %+v", stale)
 	}
 
-	secondRead := time.Now()
-	newSources := &models.Sources{SourceItem: []models.SourceItem{{Source: "PRODUCT"}}}
-	if !updateSourcesCache(conn, conn.BeginFieldPoll(webtypes.FieldSources), newSources, nil, secondRead) {
-		t.Fatal("successful source readback was not reported as an update")
-	}
+	sourcesOK = true
+	app.UpdateDeviceStatus("speaker", conn)
 
-	status = conn.Status()
-	if status.Sources != newSources || !status.SourcesReadAt.Equal(secondRead) || status.SourcesStale {
-		t.Fatalf("successful source readback did not refresh the cache: %+v", status)
+	if recovered := conn.Status(); recovered.SourcesStale {
+		t.Fatalf("successful source read did not clear staleness: %+v", recovered)
 	}
 }
 

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

@@ -37,20 +37,6 @@ function acceptsNewerStatus(current, incoming) {
     return incomingRevision !== null && incomingRevision > currentRevision;
 }
 
-// The server derives sourcesStale at read time, so two reads at the same
-// revision can disagree about it. Carry that one derived bit forward without
-// letting an otherwise-stale frame replace the canonical state.
-function mergeDerivedStatus(current, incoming) {
-    const currentRevision = statusRevision(current);
-    const incomingRevision = statusRevision(incoming);
-    if (currentRevision === null || incomingRevision !== currentRevision ||
-        current?.sourcesStale === true || incoming?.sourcesStale !== true) {
-        return current;
-    }
-
-    return { ...current, sourcesStale: true };
-}
-
 export function mergeDevicesSnapshot(previous, snapshot) {
     return Object.fromEntries(Object.entries(snapshot || {}).map(([deviceId, incoming]) => {
         const current = Object.prototype.hasOwnProperty.call(previous, deviceId)
@@ -58,10 +44,10 @@ export function mergeDevicesSnapshot(previous, snapshot) {
         if (!current || acceptsNewerStatus(current.status, incoming?.status)) {
             return [deviceId, incoming];
         }
-        return [deviceId, {
-            ...incoming,
-            status: mergeDerivedStatus(current.status, incoming?.status),
-        }];
+        // Keep the newer status we already hold, but take the rest of the
+        // incoming entry: info/stereoPair travel with the snapshot, not with
+        // the status revision.
+        return [deviceId, { ...incoming, status: current.status }];
     }));
 }
 
@@ -79,13 +65,7 @@ export function mergeStatusUpdate(previous, deviceId, status) {
     // check despite not being a real, known device.
     if (!Object.prototype.hasOwnProperty.call(previous, deviceId) ||
         !acceptsNewerStatus(previous[deviceId]?.status, status)) {
-        const current = previous[deviceId]?.status;
-        const merged = mergeDerivedStatus(current, status);
-        if (merged === current) return previous;
-        return replaceDevice(previous, deviceId, {
-            ...previous[deviceId],
-            status: merged,
-        });
+        return previous;
     }
     return replaceDevice(previous, deviceId, {
         ...previous[deviceId],

pkg/service/soundtouchweb/websocket.go

@@ -752,7 +752,6 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection,
 	volume, volumeErr := conn.Client.GetVolume()
 	presets, presetsErr := conn.Client.GetPresets()
 	sources, sourcesErr := conn.Client.GetSources()
-	sourcesReadAt := time.Now()
 	bass, bassErr := conn.Client.GetBass()
 
 	var (
@@ -800,11 +799,24 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection,
 		anyFetchSucceeded = true
 	}
 
-	// Unlike the other fields, a FAILED /sources read is merged too: the last
-	// known inventory stays visible but is marked stale so it cannot be acted
-	// on. Running through CompleteFieldPoll is what makes a newer failure
-	// fence an older, still-in-flight success.
-	updateSourcesCache(conn, sourcesGen, sources, sourcesErr, sourcesReadAt)
+	// Unlike the other fields, a FAILED /sources read is merged too. The
+	// inventory drives which source buttons the player offers, and acting on
+	// an inventory the speaker no longer confirms is worse than offering
+	// nothing: the last known list stays visible, but SourcesStale disables
+	// it until a read succeeds again. Running through CompleteFieldPoll (not
+	// a bare UpdateStatus) is what makes a newer failure fence an older,
+	// still-in-flight success rather than being silently overwritten by it.
+	conn.CompleteFieldPoll(webtypes.FieldSources, sourcesGen, func(s *webtypes.DeviceStatus) {
+		if sourcesErr != nil {
+			s.SourcesStale = true
+
+			return
+		}
+
+		s.Sources = sources
+		s.SourcesStale = false
+		s.LastActivity = time.Now()
+	})
 
 	if bassErr == nil {
 		anyFetchSucceeded = true
@@ -851,30 +863,6 @@ func (app *WebApp) updateDeviceStatus(_ string, conn *webtypes.DeviceConnection,
 	}
 }
 
-// updateSourcesCache records a source refresh, successful or not. A failure
-// keeps the previous inventory and its read time, but marks it stale so the
-// player stops offering it until a read succeeds again.
-func updateSourcesCache(
-	conn *webtypes.DeviceConnection,
-	generation uint64,
-	sources *models.Sources,
-	err error,
-	readAt time.Time,
-) bool {
-	return conn.CompleteFieldPoll(webtypes.FieldSources, generation, func(s *webtypes.DeviceStatus) {
-		if err != nil {
-			s.SourcesStale = true
-
-			return
-		}
-
-		s.Sources = sources
-		s.SourcesReadAt = readAt
-		s.SourcesStale = false
-		s.LastActivity = time.Now()
-	})
-}
-
 func (app *WebApp) applyGroupUpdatedEvent(
 	conn *webtypes.DeviceConnection,
 	event *models.GroupUpdatedEvent,

pkg/service/soundtouchweb/webtypes/source_cache_test.go

@@ -4,52 +4,63 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/gesellix/bose-soundtouch/pkg/models"
 )
 
-func TestSourceCacheStatusAtTTLBoundary(t *testing.T) {
-	readAt := time.Date(2026, time.August, 30, 12, 0, 0, 0, time.UTC)
-	sources := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}}
-	status := &DeviceStatus{
-		Sources:       sources,
-		SourcesReadAt: readAt,
+// TestSourcesFailureFencesOlderOverlappingSuccess is the ordering property
+// that makes the stale marker trustworthy: a /sources read that FAILED must
+// not be undone by an older, still-in-flight read that happens to succeed
+// after it. Both go through CompleteFieldPoll(FieldSources, ...), so the
+// failure's newer generation wins.
+func TestSourcesFailureFencesOlderOverlappingSuccess(t *testing.T) {
+	conn := NewDeviceConnection(nil, nil)
+	retained := &models.Sources{SourceItem: []models.SourceItem{{Source: "PRODUCT"}}}
+
+	firstGeneration := conn.BeginFieldPoll(FieldSources)
+	secondGeneration := conn.BeginFieldPoll(FieldSources)
+
+	conn.CompleteFieldPoll(FieldSources, secondGeneration, func(status *DeviceStatus) {
+		status.Sources = retained
+		status.SourcesStale = true
+	})
+
+	older := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}}
+	if conn.CompleteFieldPoll(FieldSources, firstGeneration, func(status *DeviceStatus) {
+		status.Sources = older
+		status.SourcesStale = false
+	}) {
+		t.Fatal("older successful source poll was accepted after the newer failure")
 	}
 
-	fresh := sourceCacheStatusAt(status, readAt.Add(sourceCacheTTL-time.Nanosecond), sourceCacheTTL)
-	if fresh.SourcesStale {
-		t.Fatal("source cache became stale before its TTL elapsed")
-	}
-
-	stale := sourceCacheStatusAt(status, readAt.Add(sourceCacheTTL), sourceCacheTTL)
-	if !stale.SourcesStale {
-		t.Fatal("source cache was not stale at its TTL boundary")
-	}
-	if stale.Sources != sources {
-		t.Fatal("stale projection did not retain the last successful source list")
-	}
-
-	refreshed := *stale
-	refreshed.SourcesReadAt = readAt.Add(sourceCacheTTL + time.Second)
-	refreshed.SourcesStale = false
-
-	got := sourceCacheStatusAt(&refreshed, refreshed.SourcesReadAt, sourceCacheTTL)
-	if got.SourcesStale {
-		t.Fatal("successful source readback did not clear staleness immediately")
+	status := conn.Status()
+	if status.Sources != retained || !status.SourcesStale {
+		t.Fatalf("older success cleared the newer source failure: %+v", status)
 	}
 }
 
-func TestSourceCacheWithoutSuccessfulReadIsNotStale(t *testing.T) {
-	status := &DeviceStatus{Sources: &models.Sources{}}
+// TestSourcesStaleSurvivesAnUnrelatedFieldMerge guards the reason staleness is
+// stored rather than derived per read: an unrelated field's merge copies the
+// status, and must carry the marker along.
+func TestSourcesStaleSurvivesAnUnrelatedFieldMerge(t *testing.T) {
+	conn := NewDeviceConnection(nil, nil)
+	conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) {
+		status.SourcesStale = true
+	})
 
-	got := sourceCacheStatusAt(status, time.Now().Add(time.Hour), time.Nanosecond)
-	if got.SourcesStale {
-		t.Fatal("source cache without a recorded successful read was marked stale")
+	conn.CompleteFieldPoll(FieldVolume, conn.BeginFieldPoll(FieldVolume), func(status *DeviceStatus) {
+		status.Volume = &models.Volume{ActualVolume: 35}
+	})
+
+	if !conn.Status().SourcesStale {
+		t.Fatal("an unrelated field merge cleared the source stale marker")
 	}
 }
 
-func TestSourceCacheFailureWithoutInventoryIsExplicit(t *testing.T) {
+// TestSourcesFailureWithoutInventoryIsExplicit covers the first-poll case: the
+// player has no inventory at all AND cannot trust one, and the browser has to
+// see that in the JSON.
+func TestSourcesFailureWithoutInventoryIsExplicit(t *testing.T) {
 	conn := NewDeviceConnection(nil, nil)
 	conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) {
 		status.SourcesStale = true
@@ -69,45 +80,30 @@ func TestSourceCacheFailureWithoutInventoryIsExplicit(t *testing.T) {
 	}
 }
 
-// TestSourceCacheFailureFencesOlderOverlappingSuccess: a /sources read that
-// FAILED must not be undone by an older, still-in-flight read that happens to
-// succeed after it. Both go through CompleteFieldPoll(FieldSources, ...), so
-// the failure's newer generation wins.
-func TestSourceCacheFailureFencesOlderOverlappingSuccess(t *testing.T) {
+// TestSourcesSuccessClearsStale is the recovery half: once a read succeeds the
+// inventory is actionable again, with no TTL to wait out.
+func TestSourcesSuccessClearsStale(t *testing.T) {
 	conn := NewDeviceConnection(nil, nil)
-	readAt := time.Date(2026, time.August, 30, 12, 0, 0, 0, time.UTC)
-	retained := &models.Sources{SourceItem: []models.SourceItem{{Source: "AUX"}}}
-
-	firstGeneration := conn.BeginFieldPoll(FieldSources)
-	secondGeneration := conn.BeginFieldPoll(FieldSources)
-
-	conn.CompleteFieldPoll(FieldSources, secondGeneration, func(status *DeviceStatus) {
-		status.Sources = retained
-		status.SourcesReadAt = readAt
+	conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) {
 		status.SourcesStale = true
 	})
 
-	older := &models.Sources{SourceItem: []models.SourceItem{{Source: "PRODUCT"}}}
-	if conn.CompleteFieldPoll(FieldSources, firstGeneration, func(status *DeviceStatus) {
-		status.Sources = older
-		status.SourcesStale = false
-	}) {
-		t.Fatal("older success was accepted after newer failure")
-	}
-
-	if status := conn.Status(); !status.SourcesStale || status.Sources != retained {
-		t.Fatalf("older success cleared newer failure: %+v", status)
-	}
-
-	newer := &models.Sources{SourceItem: []models.SourceItem{{Source: "BLUETOOTH"}}}
-	newerRead := time.Now()
+	fresh := &models.Sources{SourceItem: []models.SourceItem{{Source: "BLUETOOTH"}}}
 	conn.CompleteFieldPoll(FieldSources, conn.BeginFieldPoll(FieldSources), func(status *DeviceStatus) {
-		status.Sources = newer
-		status.SourcesReadAt = newerRead
+		status.Sources = fresh
 		status.SourcesStale = false
 	})
 
-	if status := conn.Status(); status.SourcesStale || status.Sources != newer {
-		t.Fatalf("newer source success did not restore actionability: %+v", status)
+	status := conn.Status()
+	if status.SourcesStale || status.Sources != fresh {
+		t.Fatalf("successful source readback did not restore actionability: %+v", status)
+	}
+
+	encoded, err := json.Marshal(status)
+	if err != nil {
+		t.Fatalf("marshal refreshed sources: %v", err)
+	}
+	if got := string(encoded); strings.Contains(got, "sourcesStale") {
+		t.Fatalf("cleared stale marker should be omitted from JSON: %s", got)
 	}
 }

pkg/service/soundtouchweb/webtypes/types.go

@@ -112,7 +112,6 @@ type DeviceStatus struct {
 	Presets                *models.Presets         `json:"presets,omitempty"`
 	Sources                *models.Sources         `json:"sources,omitempty"`
 	SourcesStale           bool                    `json:"sourcesStale,omitempty"`
-	SourcesReadAt          time.Time               `json:"-"`
 	Bass                   *models.Bass            `json:"bass,omitempty"`
 	Group                  *models.Group           `json:"group,omitempty"`
 	Connectivity           Connectivity            `json:"connectivity"`
@@ -152,10 +151,6 @@ const (
 	offlineGracePeriod      = 60 * time.Second
 )
 
-// sourceCacheTTL bounds how long a successfully-read source inventory stays
-// actionable without a fresh confirmation from the speaker.
-const sourceCacheTTL = 30 * time.Second
-
 // SpeakerConnectionState is the network state reported by the speaker.
 type SpeakerConnectionState struct {
 	State  string `json:"state"`
@@ -265,20 +260,7 @@ func (c *DeviceConnection) Info() *models.DeviceInfo {
 // mutated. Use UpdateStatus or SetStatus to apply changes. Never returns
 // nil for connections built via NewDeviceConnection.
 func (c *DeviceConnection) Status() *DeviceStatus {
-	return sourceCacheStatusAt(c.status.Load(), time.Now(), sourceCacheTTL)
-}
-
-func sourceCacheStatusAt(status *DeviceStatus, now time.Time, ttl time.Duration) *DeviceStatus {
-	stale := status.SourcesStale || status.Sources != nil && !status.SourcesReadAt.IsZero() &&
-		!now.Before(status.SourcesReadAt.Add(ttl))
-	if stale == status.SourcesStale {
-		return status
-	}
-
-	next := *status
-	next.SourcesStale = stale
-
-	return &next
+	return c.status.Load()
 }
 
 // Done returns a channel that is closed when the connection is removed