feat(health): compare speaker /sources with service Sources.xml

For each device, probe http://<ip>:8090/sources and compare the set of source types against the service-side Sources.xml. The two documents have *different* schemas (sourceItem attributes vs. source elements with sourceKey children), so we compare the extracted type sets rather than diffing XML directly. Two finding shapes: - WARN: service advertises types the speaker doesn't have (e.g. TUNEIN, RADIO_BROWSER missing after a factory reset). Includes a copyable POST /notification command that triggers a sourcesUpdated refresh without a reboot. - INFO: speaker has types the service doesn't know about (mostly harmless — usually AUX or BLUETOOTH-style local-only sources). Surfaces it so operators notice managed sources that drifted out of the service config. Recurring debug pattern from issues #175, #195, #214, #218, #236, disc #315. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-08-18 16:46:17 +00:00 · 2026-05-19 23:20:40 +02:00
parent 195403f42a
commit 255dd655b5
3 changed files with 419 additions and 0 deletions
@@ -106,6 +106,7 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red

 	health.RegisterSourcesXMLPresent(s.healthRegistry, ds)
 	health.RegisterSpeakerInfoReachable(s.healthRegistry, ds)
+	health.RegisterSourcesXMLDiff(s.healthRegistry, ds)

 	return s
 }
@@ -0,0 +1,211 @@
+package health
+
+import (
+	"context"
+	"encoding/xml"
+	"fmt"
+	"sort"
+	"strings"
+	"time"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+// CheckIDSourcesXMLDiff is the registry id of the speaker-vs-service
+// sources comparison check.
+const CheckIDSourcesXMLDiff = "sources_xml_diff"
+
+// speakerSourcesXML mirrors the speaker's /sources response.
+// Schema example:
+//
+//	<sources deviceID="DEVICEID01">
+//	  <sourceItem source="TUNEIN" status="READY" ... />
+//	  <sourceItem source="AUX" sourceAccount="AUX" ... />AUX IN</sourceItem>
+//	</sources>
+//
+// Note this is a *different* schema from the service-side
+// Sources.xml, which is why we compare the set of source-key types
+// rather than diffing the XML byte-for-byte.
+type speakerSourcesXML struct {
+	XMLName xml.Name `xml:"sources"`
+	Items   []struct {
+		Source string `xml:"source,attr"`
+		Status string `xml:"status,attr"`
+	} `xml:"sourceItem"`
+}
+
+// RegisterSourcesXMLDiff registers the sources_xml_diff check
+// against r. For each known device it fetches the speaker's
+// /sources, compares the source-type set against the service's
+// Sources.xml, and emits findings for asymmetries.
+func RegisterSourcesXMLDiff(r *Registry, ds *datastore.DataStore) {
+	r.Register(Check{
+		ID:    CheckIDSourcesXMLDiff,
+		Title: "Speaker /sources matches service Sources.xml",
+		Run: func() []Finding {
+			return runSourcesXMLDiff(ds)
+		},
+	})
+}
+
+func runSourcesXMLDiff(ds *datastore.DataStore) []Finding {
+	if ds == nil {
+		return nil
+	}
+
+	devices, err := ds.ListAllDevices()
+	if err != nil {
+		return []Finding{{
+			Severity: SeverityError,
+			Message:  "Could not enumerate devices: " + err.Error(),
+		}}
+	}
+
+	var findings []Finding
+
+	for i := range devices {
+		dev := &devices[i]
+		if dev.IPAddress == "" || dev.AccountID == "" || dev.DeviceID == "" {
+			continue
+		}
+
+		findings = append(findings, diffSourcesForDevice(ds, dev.AccountID, dev.DeviceID, dev.IPAddress)...)
+	}
+
+	return findings
+}
+
+func diffSourcesForDevice(ds *datastore.DataStore, account, deviceID, ipAddress string) []Finding {
+	probeURL := fmt.Sprintf("http://%s:8090/sources", ipAddress)
+	return diffSourcesForDeviceWithURL(ds, account, deviceID, ipAddress, probeURL)
+}
+
+// diffSourcesForDeviceWithURL is the same as diffSourcesForDevice
+// but takes the speaker URL explicitly. Used by tests that point
+// at an httptest.Server bound to a random port.
+func diffSourcesForDeviceWithURL(ds *datastore.DataStore, account, deviceID, ipAddress, probeURL string) []Finding {
+	target := Target{Account: account, Device: deviceID}
+
+	// Service side: read configured sources (if Sources.xml is
+	// missing, leave the set empty — the sources_xml_present check
+	// already flags that case).
+	serviceSet := map[string]bool{}
+
+	if ds.HasConfiguredSources(account, deviceID) {
+		sources, err := ds.GetConfiguredSources(account, deviceID)
+		if err == nil {
+			for i := range sources {
+				if t := sources[i].SourceKey.Type; t != "" {
+					serviceSet[t] = true
+				}
+			}
+		}
+	}
+
+	res := ProbeGet(context.Background(), probeURL, 2*time.Second)
+	if !res.Reachable {
+		// Don't double-warn — the speaker_info_reachable check
+		// will already have flagged unreachable speakers. Surface
+		// only the manual command for this specific endpoint.
+		return []Finding{{
+			Severity: SeverityInfo,
+			Target:   target,
+			Message:  "Couldn't fetch /sources from the speaker; can't compare.",
+			ManualCommands: []ManualCommand{{
+				Label:   "Compare manually:",
+				Command: res.CurlCommand,
+				Hint:    "Paste the result here in a future revision, or just diff the source list against the service's Sources.xml by eye.",
+			}},
+		}}
+	}
+
+	if res.Status != 200 {
+		return []Finding{{
+			Severity: SeverityInfo,
+			Target:   target,
+			Message:  fmt.Sprintf("Speaker /sources returned HTTP %d.", res.Status),
+		}}
+	}
+
+	speakerSet, parseErr := parseSpeakerSources(res.Body)
+	if parseErr != nil {
+		return []Finding{{
+			Severity: SeverityWarning,
+			Target:   target,
+			Message:  "Speaker /sources reply isn't valid XML.",
+			Details:  parseErr.Error(),
+		}}
+	}
+
+	missingOnSpeaker := setDifference(serviceSet, speakerSet)
+	missingOnService := setDifference(speakerSet, serviceSet)
+
+	var findings []Finding
+
+	if len(missingOnSpeaker) > 0 {
+		notifyCmd := fmt.Sprintf(
+			"curl -sS -X POST 'http://%s:8090/notification' -H 'Content-Type: application/xml' -d '<updates deviceID=\"%s\"><sourcesUpdated/></updates>'",
+			ipAddress, deviceID,
+		)
+
+		findings = append(findings, Finding{
+			Severity: SeverityWarning,
+			Target:   target,
+			Message: fmt.Sprintf(
+				"Speaker is missing %d source type(s) the service advertises: %s.",
+				len(missingOnSpeaker), strings.Join(missingOnSpeaker, ", "),
+			),
+			Details: "After a power-cycle the speaker fetches /full from the service and re-registers its source list. Forcing a sourcesUpdated notification triggers the same refresh without rebooting.",
+			ManualCommands: []ManualCommand{{
+				Label:   "Trigger a sources refresh on the speaker:",
+				Command: notifyCmd,
+				Hint:    "Run on a host that can reach the speaker on port 8090. A power-cycle is sometimes also required for the new source types to fully register (see docs/reference/radio-browser.md:21).",
+			}},
+		})
+	}
+
+	if len(missingOnService) > 0 {
+		findings = append(findings, Finding{
+			Severity: SeverityInfo,
+			Target:   target,
+			Message: fmt.Sprintf(
+				"Speaker advertises %d source type(s) the service doesn't know about: %s.",
+				len(missingOnService), strings.Join(missingOnService, ", "),
+			),
+			Details: "Usually harmless — the speaker can keep AUX or other local sources without the service knowing. But if a managed source is in this list, check the service Sources.xml.",
+		})
+	}
+
+	return findings
+}
+
+func parseSpeakerSources(body []byte) (map[string]bool, error) {
+	var parsed speakerSourcesXML
+	if err := xml.Unmarshal(body, &parsed); err != nil {
+		return nil, err
+	}
+
+	out := make(map[string]bool, len(parsed.Items))
+	for i := range parsed.Items {
+		if s := parsed.Items[i].Source; s != "" {
+			out[s] = true
+		}
+	}
+
+	return out, nil
+}
+
+// setDifference returns the keys in a that are not in b, sorted.
+func setDifference(a, b map[string]bool) []string {
+	out := make([]string, 0)
+
+	for k := range a {
+		if !b[k] {
+			out = append(out, k)
+		}
+	}
+
+	sort.Strings(out)
+
+	return out
+}
@@ -0,0 +1,207 @@
+package health
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+func newSourcesDiffDS(t *testing.T, account, device string) *datastore.DataStore {
+	t.Helper()
+
+	tempDir, err := os.MkdirTemp("", "sources-diff-test-*")
+	if err != nil {
+		t.Fatalf("temp dir: %v", err)
+	}
+
+	t.Cleanup(func() { os.RemoveAll(tempDir) })
+
+	ds := datastore.NewDataStore(tempDir)
+
+	if err := ds.SaveDeviceInfo(account, device, &models.ServiceDeviceInfo{
+		DeviceID:  device,
+		AccountID: account,
+	}); err != nil {
+		t.Fatalf("SaveDeviceInfo: %v", err)
+	}
+
+	return ds
+}
+
+func setServiceSources(t *testing.T, ds *datastore.DataStore, account, device string, types ...string) {
+	t.Helper()
+
+	sources := make([]models.ConfiguredSource, 0, len(types))
+	for i, ty := range types {
+		var src models.ConfiguredSource
+		src.ID = "1000" + string(rune('0'+i))
+		src.Type = "Audio"
+		src.SourceKey.Type = ty
+		sources = append(sources, src)
+	}
+
+	if err := ds.SaveConfiguredSources(account, device, sources); err != nil {
+		t.Fatalf("SaveConfiguredSources: %v", err)
+	}
+}
+
+func stubSpeakerSourcesServer(t *testing.T, sources ...string) string {
+	t.Helper()
+
+	var body strings.Builder
+	body.WriteString(`<?xml version="1.0" encoding="UTF-8"?>` + "\n")
+	body.WriteString(`<sources deviceID="DEVICEID01">` + "\n")
+	for _, s := range sources {
+		body.WriteString(`  <sourceItem source="` + s + `" status="READY"/>` + "\n")
+	}
+	body.WriteString(`</sources>`)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/sources" {
+			http.NotFound(w, r)
+			return
+		}
+		w.Header().Set("Content-Type", "application/xml")
+		_, _ = w.Write([]byte(body.String()))
+	}))
+	t.Cleanup(srv.Close)
+
+	u, _ := url.Parse(srv.URL)
+
+	return "http://" + u.Host + "/sources"
+}
+
+func TestSourcesDiff_FlagsMissingOnSpeaker(t *testing.T) {
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	setServiceSources(t, ds, account, device, "TUNEIN", "RADIO_BROWSER", "AUX")
+
+	speakerURL := stubSpeakerSourcesServer(t, "AUX") // missing TUNEIN, RADIO_BROWSER
+
+	got := diffSourcesForDeviceWithURL(ds, account, device, "192.0.2.10", speakerURL)
+	if len(got) == 0 {
+		t.Fatalf("expected at least one finding")
+	}
+
+	var foundMissing bool
+	for _, f := range got {
+		if strings.Contains(f.Message, "missing") && f.Severity == SeverityWarning {
+			foundMissing = true
+			if !strings.Contains(f.Message, "TUNEIN") || !strings.Contains(f.Message, "RADIO_BROWSER") {
+				t.Errorf("expected TUNEIN + RADIO_BROWSER in message, got %q", f.Message)
+			}
+
+			if len(f.ManualCommands) != 1 || !strings.Contains(f.ManualCommands[0].Command, "/notification") {
+				t.Errorf("expected notify command, got %+v", f.ManualCommands)
+			}
+
+			if !strings.Contains(f.ManualCommands[0].Command, "192.0.2.10") {
+				t.Errorf("notify command should target the device IP, got %q", f.ManualCommands[0].Command)
+			}
+		}
+	}
+
+	if !foundMissing {
+		t.Errorf("expected a 'missing on speaker' warning, got %+v", got)
+	}
+}
+
+func TestSourcesDiff_FlagsMissingOnService(t *testing.T) {
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	setServiceSources(t, ds, account, device, "AUX")
+
+	speakerURL := stubSpeakerSourcesServer(t, "AUX", "BLUETOOTH")
+
+	got := diffSourcesForDeviceWithURL(ds, account, device, "192.0.2.10", speakerURL)
+
+	var foundExtra bool
+	for _, f := range got {
+		if strings.Contains(f.Message, "doesn't know about") && f.Severity == SeverityInfo {
+			foundExtra = true
+			if !strings.Contains(f.Message, "BLUETOOTH") {
+				t.Errorf("expected BLUETOOTH in message, got %q", f.Message)
+			}
+		}
+	}
+
+	if !foundExtra {
+		t.Errorf("expected an info finding for sources missing on service, got %+v", got)
+	}
+}
+
+func TestSourcesDiff_NoFindingsWhenMatched(t *testing.T) {
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	setServiceSources(t, ds, account, device, "TUNEIN", "AUX")
+
+	speakerURL := stubSpeakerSourcesServer(t, "TUNEIN", "AUX")
+
+	got := diffSourcesForDeviceWithURL(ds, account, device, "192.0.2.10", speakerURL)
+	if len(got) != 0 {
+		t.Errorf("expected no findings, got %+v", got)
+	}
+}
+
+func TestSourcesDiff_UnreachableSpeakerEmitsManualCommand(t *testing.T) {
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	setServiceSources(t, ds, account, device, "TUNEIN")
+
+	// Refused port; probe fails.
+	got := diffSourcesForDeviceWithURL(ds, account, device, "127.0.0.1", "http://127.0.0.1:1/sources")
+	if len(got) != 1 || got[0].Severity != SeverityInfo {
+		t.Fatalf("expected one info finding for unreachable speaker, got %+v", got)
+	}
+
+	if len(got[0].ManualCommands) != 1 {
+		t.Errorf("expected a manual command, got %+v", got[0].ManualCommands)
+	}
+}
+
+func TestSourcesDiff_MalformedXMLWarns(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte("not xml"))
+	}))
+	defer srv.Close()
+
+	u, _ := url.Parse(srv.URL)
+	probeURL := "http://" + u.Host + "/sources"
+
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	setServiceSources(t, ds, account, device, "TUNEIN")
+
+	got := diffSourcesForDeviceWithURL(ds, account, device, "192.0.2.10", probeURL)
+	if len(got) != 1 || got[0].Severity != SeverityWarning {
+		t.Fatalf("expected one warning for malformed XML, got %+v", got)
+	}
+}
+
+func TestSourcesDiff_EmptyServiceSetDoesNotDoubleWarn(t *testing.T) {
+	account, device := "1000001", "DEVICEID01"
+
+	ds := newSourcesDiffDS(t, account, device)
+	// Don't write any Sources.xml — sources_xml_present already flags this.
+
+	speakerURL := stubSpeakerSourcesServer(t, "AUX", "TUNEIN")
+
+	got := diffSourcesForDeviceWithURL(ds, account, device, "192.0.2.10", speakerURL)
+
+	for _, f := range got {
+		if f.Severity == SeverityWarning && strings.Contains(f.Message, "missing") {
+			t.Errorf("should not emit a 'missing on speaker' warning when service set is empty, got %+v", f)
+		}
+	}
+}