From 8604f1e6ba3a393d259130c6d4aae1e0a91c2b93 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Wed, 20 May 2026 21:38:37 +0200 Subject: [PATCH] fix(datastore,health): enumerate all stale account dirs per device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User reported "we might have another issue with the account mapping" after the prior commit only handled the default-vs-real case. The backup at /backup/var_20260520_01 showed device A81B6A536A98 living under four directories — accounts/9569497, accounts/default, accounts/1111111, and the top-level default/ — only the third of which currently receives the speaker's PUTs. The authoritative "which account does this device belong to" signal is the URL of the speaker's incoming PUT (per "speaker decides"), which only the live handler observes. mtime is a proxy and can be fooled by backup tools, manual touches, etc., so this commit drops the mtime tiebreaker the previous attempt added. Instead: - ListAllDevices' dedup keeps default-deprioritisation (clear placeholder semantics) but otherwise picks the first real account encountered in stable alphabetical order. No heuristic guessing among real accounts. - New AllAccountsForDevice(deviceID) enumerates every on-disk account directory containing the deviceID. - The consistency check's orphan finding now lists every stale account dir for each device, with the path the operator needs to inspect and a pointer to the service log so they can verify which account the speaker is actually targeting before deleting anything. We don't delete automatically — destructive filesystem actions need explicit operator consent (CLAUDE.md "destructive actions" rule). Co-Authored-By: Claude Sonnet 4.6 --- pkg/service/datastore/datastore.go | 64 ++++++++++++++---- pkg/service/health/checks_consistency.go | 83 +++++++++++++++++------- 2 files changed, 111 insertions(+), 36 deletions(-) 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}