From 4fe5ad50b4d112f18b0bbe15e14dbdfac17c05db Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sat, 5 Sep 2026 19:21:12 +0200 Subject: [PATCH] fix(player): don't retract a push-confirmed source selection A nowPlayingUpdated event is authoritative evidence that the speaker switched, and the effect watching it promotes a pending command to provisional-confirmed. The last readback then overwrote the command wholesale with outcome: 'unverified', in both its no-match branch and its catch, without looking at what the command had already become. So a selection the speaker confirmed by push at ~1s was reported as "Source selection unverified" when the 10s readback happened to fail or returned a now-playing that had since moved on. The readback window closing now settles such a command as confirmed rather than retracting it; only a command still pending, which nothing ever confirmed, becomes unverified. A failed command stays failed. The push-event promotion also has to record confirmedRevision, which it previously left unset: without it the later-authoritative-source check would compare against undefined and the projection would never clear. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser_compatibility_test.go | 37 ++++++++++++++++ .../static/js/components/Sources.js | 44 ++++++++++++------- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/pkg/service/soundtouchweb/browser_compatibility_test.go b/pkg/service/soundtouchweb/browser_compatibility_test.go index 54c4a5a5..c39ad20d 100644 --- a/pkg/service/soundtouchweb/browser_compatibility_test.go +++ b/pkg/service/soundtouchweb/browser_compatibility_test.go @@ -655,6 +655,43 @@ func TestSourceSelectionLaterFirmwareErrorOverridesProvisionalConfirmation(t *te } } +// TestSourceSelectionKeepsPushConfirmationWhenReadbacksFail: a +// nowPlayingUpdated event is authoritative evidence the speaker switched. +// Once it has confirmed the selection, the readback window closing without a +// matching read must not retract that and report "unverified". +func TestSourceSelectionKeepsPushConfirmationWhenReadbacksFail(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}`)) + }) + // Every readback fails, so only the pushed status can confirm anything. + r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + }) + }) + + 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), + // The speaker reports the switch before the first readback deadline. + chromedp.Evaluate(`window.renderStatus('AUX', 'AUX1')`, nil), + chromedp.Poll(`document.querySelector('.source-btn').classList.contains('provisional-confirmed')`, nil), + // Outlast the last readback deadline (500ms in this fixture). + chromedp.Sleep(900*time.Millisecond), + chromedp.Text(`.source-command-status`, &statusText, chromedp.ByQuery), + ); err != nil { + t.Fatalf("exercise push-confirmed selection with failing readbacks: %v", err) + } + + if statusText != "Source selected" { + t.Errorf("status = %q, want the push confirmation to stand as %q", statusText, "Source selected") + } +} + func TestSourceSelectionRejectsReadbackWithOnlyNewerAggregateRevision(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 015a6133..23ac8c7d 100644 --- a/pkg/service/soundtouchweb/static/js/components/Sources.js +++ b/pkg/service/soundtouchweb/static/js/components/Sources.js @@ -89,7 +89,11 @@ export function Sources({ : previous); } else if (matches && command.outcome === 'pending') { setCommand(previous => previous?.generation === command.generation - ? { ...previous, outcome: 'provisional-confirmed' } + ? { + ...previous, + outcome: 'provisional-confirmed', + confirmedRevision: nowPlayingRevision, + } : previous); } }, [command, currentSource, currentAccount, nowPlayingRevision]); @@ -120,6 +124,28 @@ export function Sources({ }); const startedAt = Date.now(); + // Called when the readback window closes without this round matching. + // "Unverified" is only honest for a command nothing ever confirmed: a + // nowPlayingUpdated event may already have confirmed it at t=1s, and a + // failed readback at t=10s must not retract that. Such a command is + // settled as confirmed instead, since no further readback will run. + function settleUnverified(previous) { + if (previous?.generation !== generation) return previous; + if (previous.outcome === 'provisional-confirmed' || + previous.outcome === 'final-confirmed') { + return { ...previous, outcome: 'final-confirmed' }; + } + if (previous.outcome === 'failed') return previous; + + return { + ...target, + generation, + outcome: 'unverified', + error: active.writeError?.message, + startNowPlayingRevision: nowPlayingRevision, + }; + } + readbackDelays.forEach((delay, index) => { const timer = setTimeout(async () => { if (commandRef.current.active !== active) return; @@ -162,25 +188,13 @@ export function Sources({ } } else if (index === readbackDelays.length - 1) { commandRef.current.active = null; - setCommand({ - ...target, - generation, - outcome: 'unverified', - error: active.writeError?.message, - startNowPlayingRevision: nowPlayingRevision, - }); + setCommand(settleUnverified); } } catch (_) { if (commandRef.current.active === active && active.latestReadback === index && index === readbackDelays.length - 1) { commandRef.current.active = null; - setCommand({ - ...target, - generation, - outcome: 'unverified', - error: active.writeError?.message, - startNowPlayingRevision: nowPlayingRevision, - }); + setCommand(settleUnverified); } } }, Math.max(0, delay - (Date.now() - startedAt)));