From 2c1ac79ddd58c17544cda6efcc34a8393ce809a8 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 5 Sep 2026 20:13:02 +0200 Subject: [PATCH] fix(player): never confirm a source the speaker reports as not playing PROVIDER_SOURCES covers the sources known to produce the stub now-playing. It cannot cover the ones we have not tested, ALEXA among them, and a source list is whatever the speaker chooses to advertise. So the readback now refuses to confirm the stub itself, wherever it comes from: a now-playing that names the source we asked for but reports no location, no play status, and an item name echoing the source is reported as a failure rather than a success. That shape is what a speaker returns for a select it accepted but cannot act on. All three conditions are required together. A physical input reports no location and no item name of its own yet is genuinely playing, so any one condition alone would reject real selections; a test covers exactly that case, confirming with PlayStatus set and no location. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser_compatibility_test.go | 65 +++++++++++++++++++ .../static/js/components/Sources.js | 40 +++++++++++- 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index 3be29471..b7cf0e39 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -702,6 +702,71 @@ func TestProviderSourceNavigatesWhenRecentsFail(t *testing.T) { } } +// TestStubNowPlayingIsNotReportedAsSuccess: PROVIDER_SOURCES only covers the +// sources known to produce the stub. For any other, the readback must not +// confirm a now-playing that names the source but reports nothing playing: +// no location, no play status, item name echoing the source. +func TestStubNowPlayingIsNotReportedAsSuccess(t *testing.T) { + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":9,"nowPlayingRevision":9,` + + `"webSocketConnected":true,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1",` + + `"PlayStatus":"","ContentItem":{"Source":"AUX","Type":"","Location":"",` + + `"ItemName":"AUX","IsPresetable":false}}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + var statusText string + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-btn:nth-child(1)').classList.contains('failed')`, nil), + chromedp.Text(`.source-command-status`, &statusText, chromedp.ByQuery), + ); err != nil { + t.Fatalf("exercise stub now-playing readback: %v", err) + } + + if !strings.Contains(statusText, "nothing playing") { + t.Errorf("status = %q, want it to report that nothing is playing", statusText) + } +} + +// TestPlayingSourceWithoutLocationStillConfirms guards the backstop's own +// blast radius: a physical input reports no location, and must still confirm +// as long as the speaker says it is playing. +func TestPlayingSourceWithoutLocationStillConfirms(t *testing.T) { + server := newPlayerFixtureServer(t, sourceFixtureScript, func(r chi.Router) { + r.Post("/api/control/devices/speaker/action/source", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true}`)) + }) + r.Get("/api/control/devices/speaker/now-playing", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":9,"nowPlayingRevision":9,` + + `"webSocketConnected":true,"nowPlaying":{"Source":"AUX","SourceAccount":"AUX1",` + + `"PlayStatus":"PLAY_STATE","ContentItem":{"Source":"AUX","Type":"","Location":"",` + + `"ItemName":"AUX","IsPresetable":false}}}}}`)) + }) + }) + + ctx := newHeadlessChromeContext(t) + if err := chromedp.Run(ctx, + chromedp.Navigate(server.URL+"/fixture"), + chromedp.WaitVisible(`.source-btn`, chromedp.ByQuery), + chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery), + chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected'`, nil), + ); err != nil { + t.Fatalf("exercise playing source with no location: %v", err) + } +} + func TestSourceSelectionStopsReadbacksOnceTheEventStreamConfirms(t *testing.T) { var mu sync.Mutex reads := 0 diff --git a/pkg/service/soundtouchweb/static/js/components/Sources.js b/pkg/service/soundtouchweb/static/js/components/Sources.js index 95514eca..d8ca701d 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -44,6 +44,26 @@ function isErrorSource(source) { return source === 'INVALID_SOURCE' || source?.endsWith('_ERROR'); } +// A speaker given a select it cannot act on parks on a stub now-playing +// rather than refusing: the source name echoed back as the item name, no +// location, and no play status, while whatever was already playing carries +// on. Confirmed on real hardware for RADIO_BROWSER and LOCAL_INTERNET_RADIO. +// +// PROVIDER_SOURCES prevents the selects we know produce this. This check is +// the backstop for the ones we do not know about, ALEXA among them: reporting +// such a readback as success is worse than reporting nothing, because the +// source list then shows a source the speaker is demonstrably not playing. +// +// All three conditions are required. A genuinely playing source reports a +// play status even when it has no location to report, a physical input for +// instance, so no single condition would be safe on its own. +function isStubNowPlaying(nowPlaying) { + const item = nowPlaying?.ContentItem; + if (!item) return false; + + return !item.Location && !nowPlaying.PlayStatus && item.ItemName === nowPlaying.Source; +} + function sourceAccountIdentity(source, account) { return account && account !== source ? account : ''; } @@ -101,6 +121,7 @@ export function Sources({ const matches = currentSource === command.source && sourceAccountsMatch(command.source, currentAccount, command.account); + const stub = currentSource === command.source && isStubNowPlaying(status?.nowPlaying); if (command.outcome === 'final-confirmed') { if (nowPlayingRevision > command.confirmedRevision && !matches) { setCommand(previous => previous?.generation === command.generation @@ -108,11 +129,15 @@ export function Sources({ } return; } - if (isErrorSource(currentSource)) { + if (isErrorSource(currentSource) || stub) { clearReadbacks(); commandRef.current.active = null; setCommand(previous => previous?.generation === command.generation - ? { ...previous, outcome: 'failed', error: currentSource } + ? { + ...previous, + outcome: 'failed', + error: stub ? 'speaker reported nothing playing' : currentSource, + } : previous); } else if (matches && command.outcome === 'pending') { setCommand(previous => previous?.generation === command.generation @@ -250,6 +275,17 @@ export function Sources({ error: nowPlaying.Source, startNowPlayingRevision: nowPlayingRevision, }); + } else if (revisionIsNewer && nowPlaying?.Source === target.source && + isStubNowPlaying(nowPlaying)) { + clearReadbacks(); + commandRef.current.active = null; + setCommand({ + ...target, + generation, + outcome: 'failed', + error: 'speaker reported nothing playing', + startNowPlayingRevision: nowPlayingRevision, + }); } else if (revisionIsNewer && nowPlaying?.Source === target.source && sourceAccountsMatch(target.source, nowPlaying?.SourceAccount, target.account)) { // A match is not the end of the story: /select answers