diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index eb8e89f..44238e1 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -635,9 +635,12 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) { // Sort "default" to the back so a real-account entry always // wins the first-seen race. "default" exists as a pre-pair // placeholder; once the speaker pairs with a real account it - // becomes orphan state. Sort returns the same list for tests - // because os.ReadDir is already sorted, and digits sort before - // "default" in ASCII — but be defensive. + // becomes orphan state. We don't try to pick a winner among + // multiple real accounts via mtime or other proxies — the + // authoritative signal is the URL of the speaker's incoming + // PUT, which only the live handler sees. Stale real-account + // entries are flagged as findings by the consistency check + // instead. accounts := make([]os.DirEntry, 0, len(entries)) for _, e := range entries { if e.IsDir() { @@ -677,18 +680,18 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) { continue } - // Don't let a "default" entry replace a real-account - // one. The speaker pairs to a real account; any - // remaining "default" record is orphan state from - // before pairing. + // "default" never replaces a real-account entry. if info.AccountID == accountIDDefault { continue } - // Allow real-account → real-account replacement only - // when the prior was the placeholder "default" or had - // no name (less authoritative). - if entry.account == accountIDDefault || (devices[entry.index].Name == "" && info.Name != "") { + // First real-account encountered wins (sort is + // stable + alphabetical so the result is + // deterministic across runs). Don't pick a different + // winner here — the consistency check enumerates all + // the duplicate account dirs for the operator to + // clean up. + if entry.account == accountIDDefault { devices[entry.index] = info seenIDs[key] = seenEntry{index: entry.index, account: info.AccountID} } @@ -699,6 +702,45 @@ func (ds *DataStore) ListAllDevices() ([]models.ServiceDeviceInfo, error) { return devices, nil } +// AllAccountsForDevice returns every account directory that contains a +// device with the given deviceID, in their on-disk order. Used by the +// consistency check to enumerate stale account entries left behind +// when the speaker was re-paired to a different account. +func (ds *DataStore) AllAccountsForDevice(deviceID string) []string { + if deviceID == "" { + return nil + } + + var hits []string + + seen := map[string]bool{} + + for _, dir := range ds.getPossibleDataDirs() { + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + + for _, acc := range entries { + if !acc.IsDir() { + continue + } + + devicePath := filepath.Join(dir, acc.Name(), "devices", deviceID) + if _, err := os.Stat(devicePath); err != nil { + continue + } + + if !seen[acc.Name()] { + seen[acc.Name()] = true + hits = append(hits, acc.Name()) + } + } + } + + return hits +} + // accountIDDefault is the placeholder account id assigned to records // that exist before a speaker has been paired with a real Marge // account. Treated as fallback in ListAllDevices and skipped in diff --git a/pkg/service/health/checks_consistency.go b/pkg/service/health/checks_consistency.go index 14f4e9d..a56f260 100644 --- a/pkg/service/health/checks_consistency.go +++ b/pkg/service/health/checks_consistency.go @@ -4,7 +4,7 @@ import ( "context" "encoding/xml" "fmt" - "os" + "strconv" "time" "github.com/gesellix/bose-soundtouch/pkg/models" @@ -103,48 +103,81 @@ func runPresetsConsistencyCheck(ds *datastore.DataStore) []Finding { return findings } -// detectOrphanDefaultEntries walks the on-disk account directories -// directly (not through ListAllDevices, which already dedupes "default" -// out) and flags any device that exists under "default" *and* under a -// real account. The fix is to delete accounts/default/devices// — -// we don't do it automatically because filesystem deletions need -// explicit operator consent (CLAUDE.md "destructive actions" rule). +// detectOrphanDefaultEntries flags devices that exist under multiple +// account directories. The speaker decides which account it belongs to +// via the URL of every PUT it sends; any other account entry on disk +// is leftover state from a previous pairing. The active account is +// the one ListAllDevices' dedup currently exposes (with "default" +// already deprioritised); the stale ones get one finding each so the +// operator can see and clean them up. We don't delete automatically +// because filesystem deletions need explicit operator consent +// (CLAUDE.md "destructive actions" rule). func detectOrphanDefaultEntries(ds *datastore.DataStore, paired []models.ServiceDeviceInfo) []Finding { - pairedReal := map[string]string{} // deviceID -> real accountID + activeAccount := map[string]string{} // deviceID -> the account ListAllDevices picked for i := range paired { - if paired[i].AccountID != "" && paired[i].AccountID != "default" && paired[i].DeviceID != "" { - pairedReal[paired[i].DeviceID] = paired[i].AccountID + if paired[i].DeviceID == "" { + continue } - } - defaultDevicesDir := ds.AccountDevicesDir("default") - - entries, err := os.ReadDir(defaultDevicesDir) - if err != nil { - return nil + activeAccount[paired[i].DeviceID] = paired[i].AccountID } var findings []Finding - for _, e := range entries { - if !e.IsDir() { + for deviceID, active := range activeAccount { + allAccounts := ds.AllAccountsForDevice(deviceID) + if len(allAccounts) <= 1 { continue } - deviceID := e.Name() - if realAccount, ok := pairedReal[deviceID]; ok { - findings = append(findings, Finding{ - Severity: SeverityWarning, - Target: Target{Account: "default", Device: deviceID}, - Message: "Orphan \"default\" account entry: device " + deviceID + " is also paired under account " + realAccount + ". The default entry is leftover state from before pairing and is masked from consistency checks. Safe to delete: rm -rf /accounts/default/devices/" + deviceID, - }) + stale := make([]string, 0, len(allAccounts)-1) + + for _, acc := range allAccounts { + if acc != active { + stale = append(stale, acc) + } } + + if len(stale) == 0 { + continue + } + + findings = append(findings, Finding{ + Severity: SeverityWarning, + Target: Target{Device: deviceID}, + Message: "Device " + deviceID + " has state under " + strconv.Itoa(len(allAccounts)) + + " account directories — likely leftover from earlier pairings. The active one (per ListAllDevices' dedup) is " + safeQuoteFinding(active) + + "; stale entries: " + joinAccounts(stale) + + ". Confirm which one the speaker currently PUTs to (check service log for /streaming/account//device/" + deviceID + "/...) and remove the others. Each stale dir lives at /accounts//devices/" + deviceID + "/.", + }) } return findings } +func safeQuoteFinding(s string) string { + if s == "" { + return `""` + } + + return `"` + s + `"` +} + +func joinAccounts(accounts []string) string { + out := "" + + for i, a := range accounts { + if i > 0 { + out += ", " + } + + out += `"` + a + `"` + } + + return out +} + func checkOneDeviceConsistency(ds *datastore.DataStore, account, deviceID, ipAddress string) []Finding { target := Target{Account: account, Device: deviceID}