fix(player): keep verifying a source write unless the refusal is definitive

Selecting a source posts once and then confirms by bounded readback. When
the POST itself failed, the write error was captured into active.writeError
and never surfaced: a rejected command reported nothing for the full 10s
readback window and then a bare "Source selection unverified".

Surfacing it needs a distinction the API layer did not make. checkedReq
collapsed every failure into one Error, but the two cases differ:

  - 4xx: every 4xx on these endpoints is produced before AfterTouch calls
    the speaker (unknown device, unparseable body, empty source, unknown
    action), so the command provably never went out. Nothing can confirm
    it; report the failure at once, with the server's reason.
  - 5xx and transport errors: handleSourceControl reports a failed
    Client.SelectSource through sendControlResponse, which maps any
    speaker-call error to 500. A request that timed out after the speaker
    already switched is indistinguishable from one it never received, so
    the readbacks must keep running and the reason is carried into
    whatever outcome they reach.

checkedReq now tags thrown errors with `definitive`, and only a definitive
refusal cancels the readbacks. Outcome text appends the reason when there
is one, so a firmware rejection also names the error source it saw.

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

pkg/service/soundtouchweb/browser_compatibility_test.go

@@ -370,6 +370,10 @@ func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) {
 
 			w.Header().Set("Content-Type", "application/json")
 			if body.Source == "SPOTIFY" {
+				// 500 is how a failed Client.SelectSource surfaces. It does not
+				// prove the speaker ignored the command, so the readbacks below
+				// must still run.
+				w.WriteHeader(http.StatusInternalServerError)
 				_ = json.NewEncoder(w).Encode(webtypes.APIResponse{Success: false, Error: "source rejected"})
 				return
 			}
@@ -406,6 +410,7 @@ func TestSourceSelectionUsesOneWriteAndAbsoluteReadbacks(t *testing.T) {
 		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified'`, nil),
 		chromedp.Click(`.source-btn:nth-child(3)`, chromedp.ByQuery),
 		chromedp.Poll(`document.querySelector('.source-btn:nth-child(3)').classList.contains('unverified')`, nil),
+		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection unverified: source rejected'`, nil),
 	); err != nil {
 		t.Fatalf("exercise source commands: %v", err)
 	}
@@ -553,6 +558,55 @@ func TestSourceSelectionReadbacksDoNotWaitForSlowWriteResponse(t *testing.T) {
 	}
 }
 
+// TestSourceSelectionDefinitiveRefusalFailsImmediately: a 4xx is produced
+// before AfterTouch ever calls the speaker, so the command provably never went
+// out. There is nothing for the readbacks to confirm; report it at once, with
+// the server's reason, instead of polling for the full readback window.
+func TestSourceSelectionDefinitiveRefusalFailsImmediately(t *testing.T) {
+	var mu sync.Mutex
+	reads := 0
+	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.WriteHeader(http.StatusNotFound)
+			_, _ = w.Write([]byte(`{"success":false,"error":"Device not found"}`))
+		})
+		r.Get("/api/control/devices/speaker", func(w http.ResponseWriter, _ *http.Request) {
+			mu.Lock()
+			reads++
+			mu.Unlock()
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{"success":true,"data":{"status":{"revision":2,"nowPlayingRevision":2,"nowPlaying":{"Source":"STANDBY","SourceAccount":""}}}}`))
+		})
+	})
+
+	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').classList.contains('failed')`, nil),
+		chromedp.Text(`.source-command-status`, &statusText, chromedp.ByQuery),
+		// Well past every readback deadline in this fixture (100/250/500ms), so
+		// a zero read count means the failure came from the write itself.
+		chromedp.Sleep(700*time.Millisecond),
+	); err != nil {
+		t.Fatalf("exercise definitively refused source write: %v", err)
+	}
+
+	if !strings.Contains(statusText, "Source selection failed") ||
+		!strings.Contains(statusText, "Device not found") {
+		t.Errorf("status = %q, want the failure and the server's reason", statusText)
+	}
+
+	mu.Lock()
+	defer mu.Unlock()
+	if reads != 0 {
+		t.Errorf("readbacks after a definitive refusal = %d, want 0", reads)
+	}
+}
+
 func TestSourceSelectionLaterFirmwareErrorOverridesProvisionalConfirmation(t *testing.T) {
 	var mu sync.Mutex
 	reads := 0
@@ -586,7 +640,7 @@ func TestSourceSelectionLaterFirmwareErrorOverridesProvisionalConfirmation(t *te
 		chromedp.Click(`.source-btn:nth-child(1)`, chromedp.ByQuery),
 		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selected, confirming'`, nil),
 		chromedp.Evaluate(`document.querySelector('.source-btn:nth-child(1)').classList.contains('provisional-confirmed')`, &provisional),
-		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection failed'`, nil),
+		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection failed: INVALID_SOURCE'`, nil),
 	); err != nil {
 		t.Fatalf("exercise provisional source rejection: %v", err)
 	}
@@ -658,7 +712,7 @@ func TestSourceSelectionTreatsFirmwareErrorSourceAsFailed(t *testing.T) {
 		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 selection failed'`, nil),
+		chromedp.Poll(`document.querySelector('.source-command-status').textContent === 'Source selection failed: INVALID_SOURCE'`, nil),
 	); err != nil {
 		t.Fatalf("exercise firmware source rejection: %v", err)
 	}

pkg/service/soundtouchweb/frontend_test/api.test.mjs

@@ -23,22 +23,44 @@ test('selectSource posts the exact source and account body', async () => {
     assert.deepEqual(JSON.parse(requests[1].options.body), { source: 'PRODUCT', account: '' });
 });
 
-test('source selection rejects non-2xx responses', async () => {
-    globalThis.fetch = async () => new Response('{"success":false,"error":"offline"}', {
-        status: 503,
+// A 4xx is produced before any speaker call (unknown device, unparseable
+// body, empty source), so the command provably never went out.
+test('source selection reports a 4xx as definitive', async () => {
+    globalThis.fetch = async () => new Response('{"success":false,"error":"Device not found"}', {
+        status: 404,
         headers: { 'Content-Type': 'application/json' },
     });
 
-    await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), /offline/);
+    await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), error => {
+        assert.match(error.message, /Device not found/);
+        assert.equal(error.definitive, true);
+        return true;
+    });
 });
 
-test('source selection rejects application failures on 2xx responses', async () => {
-    globalThis.fetch = async () => new Response('{"success":false,"error":"rejected"}', {
-        status: 200,
+// A 5xx is how a failed Client.SelectSource surfaces, and a request that timed
+// out after the speaker already switched is indistinguishable from one it never
+// received. The caller must keep verifying by readback.
+test('source selection reports a 5xx as non-definitive', async () => {
+    globalThis.fetch = async () => new Response('{"success":false,"error":"speaker unreachable"}', {
+        status: 500,
         headers: { 'Content-Type': 'application/json' },
     });
 
-    await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), /rejected/);
+    await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), error => {
+        assert.match(error.message, /speaker unreachable/);
+        assert.equal(error.definitive, false);
+        return true;
+    });
+});
+
+test('source selection reports an unreadable body as non-definitive on 5xx', async () => {
+    globalThis.fetch = async () => new Response('gateway timeout', { status: 504 });
+
+    await assert.rejects(api.selectSource('speaker', 'AUX', 'AUX1'), error => {
+        assert.equal(error.definitive, false);
+        return true;
+    });
 });
 
 test('legacy API consumers retain response-level error handling', async () => {

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

@@ -5,16 +5,33 @@ async function req(url, opts = {}) {
     return r.json();
 }
 
+// checkedReq turns a failed request into a thrown Error, tagging whether the
+// failure is DEFINITIVE, meaning proof the command never reached the speaker.
+//
+// Only 4xx qualifies. Every 4xx on the control endpoints is produced before
+// any speaker call (missing/unknown device, unparseable body, empty source,
+// unknown action), so nothing was sent onward. A 5xx is NOT proof of anything:
+// handleSourceControl reports a failed Client.SelectSource through
+// sendControlResponse, which maps any speaker-call error to 500, and a request
+// that timed out after the speaker already switched looks exactly like one it
+// never received. Transport errors are ambiguous for the same reason.
+//
+// Callers that verify by readback must keep verifying unless the failure is
+// definitive.
 async function checkedReq(url, opts = {}) {
     const r = await fetch(url, opts);
+    const definitive = r.status >= 400 && r.status < 500;
     let response;
     try {
         response = await r.json();
     } catch (_) {
-        throw new Error(`Request failed (${r.status})`);
+        throw Object.assign(new Error(`Request failed (${r.status})`), { definitive });
     }
     if (!r.ok || response?.success === false) {
-        throw new Error(response?.error || `Request failed (${r.status})`);
+        throw Object.assign(
+            new Error(response?.error || `Request failed (${r.status})`),
+            { definitive },
+        );
     }
     return response;
 }

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

@@ -190,7 +190,23 @@ export function Sources({
         try {
             await api.selectSource(deviceId, target.source, target.account);
         } catch (error) {
-            if (commandRef.current.active === active) active.writeError = error;
+            if (commandRef.current.active !== active) return;
+            // A definitive refusal (4xx) means the speaker never saw the
+            // command, so there is nothing for the readbacks to confirm and
+            // reporting it now beats waiting out the readback window. Anything
+            // else stays pending: a 5xx or a transport error does not tell us
+            // whether the speaker acted, so we keep verifying and carry the
+            // reason into whatever outcome the readbacks reach.
+            if (!error?.definitive) {
+                active.writeError = error;
+
+                return;
+            }
+            clearReadbacks();
+            commandRef.current.active = null;
+            setCommand(previous => previous?.generation === generation
+                ? { ...previous, outcome: 'failed', error: error?.message }
+                : previous);
         }
     }
 
@@ -209,6 +225,12 @@ export function Sources({
         failed: 'Source selection failed',
     };
 
+    function commandMessage(cmd) {
+        if (!cmd) return '';
+        const text = outcomeText[cmd.outcome];
+        return cmd.error ? `${text}: ${cmd.error}` : text;
+    }
+
     return html`
         <div class="sources-section">
             <h3 class="section-title">Sources</h3>
@@ -225,7 +247,7 @@ export function Sources({
                             class="source-btn ${isActive ? 'active' : ''} ${src.IsLocal ? 'local' : ''} ${outcome}"
                             onClick=${() => select(src)}
                             disabled=${sourcesStale}
-                            title=${availabilityMessage || (outcome ? outcomeText[outcome] : src.Source)}
+                            title=${availabilityMessage || (outcome ? commandMessage(command) : src.Source)}
                             aria-describedby=${availabilityId}
                             aria-busy=${outcome === 'pending' || outcome === 'provisional-confirmed' ? 'true' : null}
                         >
@@ -241,7 +263,7 @@ export function Sources({
                 role="status"
                 aria-live="polite"
             >
-                ${availabilityMessage || (command ? outcomeText[command.outcome] : '')}
+                ${availabilityMessage || commandMessage(command)}
             </div>
         </div>
     `;