github/Bose-SoundTouch
1
0
Fork 0
mirror of https://github.com/gesellix/Bose-SoundTouch.git synced 2026-09-07 15:07:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(setup): warn about a shared account instead of refusing migration

Browse Source 
Migration was refused whenever the rendered account held more than one
device. One account holding every speaker in the household is the normal Bose
arrangement, so this blocked most setups, and there was no override: the
check runs unconditionally at the top of MigrateSpeaker.

It also misfired on genuinely single-speaker setups.
handleDiscoveredDeviceFallback writes a second device directory keyed by the
host address under the same account whenever /info momentarily fails, and its
cleanup only runs when d.SerialNo is set, which discovery never populates. A
stale entry left behind by a DHCP lease change then blocked migration
permanently.

The evidence does not support a hard block either. Issue #614 concluded the
shared-account preset wipe is empirical rather than a proven mechanism, with
the root cause still open, and the troubleshooting entry added there is
labelled a workaround. The guide's own remediation was unreachable in normal
use, since discovery re-adds the other devices.

The check now reports it as a warning, naming the device count, carried into
the migration log the UI already shows alongside its other "Warning:" lines.
The provable checks (persisted snapshot present and valid, presets equal
across snapshot, live /presets and rendered /full) still refuse.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-09-05 21:55:20 +02:00
co-authored by Claude Opus 5
parent ba7e2da7b3
commit c46bc4898a
4 changed files with 80 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/content/docs/guides/MIGRATION-GUIDE.md
+8 -6
View File
@@ -179,12 +179,14 @@ account and device and verifies that its rendered account data preserves every
live preset slot. If the migration page asks for Data Sync, sync the device and
retry instead of bypassing the check.
Migration is also refused while the rendered account contains another device.
Some speaker firmware wipes its presets after a reboot-triggered resync of a
shared account even when `/full` contains the correct data. Move the speaker to
a dedicated account, run Data Sync for it, and then retry migration. Merely
removing the other devices is not sufficient because discovery can add them
again before the speaker fetches `/full` after reboot.
If the account already contains other devices, migration proceeds but the log
says so. Some speaker firmware has been reported to wipe its presets after a
reboot-triggered resync of a shared account even when `/full` contains the
correct data (see issue #614, where the root cause is still open). One account
holding every speaker in the household is the normal arrangement, so this is a
warning rather than a refusal; if you do hit the preset wipe, moving that
speaker to a dedicated account and running Data Sync for it is the known
workaround.
---
pkg/service/setup/migration_readiness.go
+32 -25
View File
@@ -30,31 +30,30 @@ func migrationDataNotReadyf(format string, args ...any) error {
return &MigrationDataNotReadyError{Reason: fmt.Sprintf(format, args...)}
}
func migrationDataNotReadyWithAction(reason, action string) error {
return &MigrationDataNotReadyError{Reason: reason, Action: action}
}
// checkMigrationDataReady proves that redirecting the speaker to this service
// will not replace its live presets with missing, stale, or filtered account
// data. Every operation in this check is read-only.
func (m *Manager) checkMigrationDataReady(deviceIP string) error {
//
// It returns warnings for conditions worth telling the user about but not
// worth refusing over, and an error only for the ones it can actually prove.
func (m *Manager) checkMigrationDataReady(deviceIP string) ([]string, error) {
if m.DataStore == nil {
// CLI callers do not own the service datastore and cannot enforce this
// check. ExecuteInitPlan is a separate onboarding flow which establishes
// account state only after its intentional URL rewrite.
return nil
return nil, nil
}
info, err := m.GetLiveDeviceInfo(deviceIP)
if err != nil {
return migrationDataNotReadyf("cannot read live /info: %v", err)
return nil, migrationDataNotReadyf("cannot read live /info: %v", err)
}
deviceID := strings.TrimSpace(info.DeviceID)
accountID := strings.TrimSpace(info.MargeAccountUUID)
if deviceID == "" {
return migrationDataNotReadyf("live /info has no deviceID")
return nil, migrationDataNotReadyf("live /info has no deviceID")
}
if accountID == "" {
@@ -67,69 +66,77 @@ func (m *Manager) checkMigrationDataReady(deviceIP string) error {
// factory-reset device. Data Sync cannot unblock it either, since it
// files an account-less device under "default", which never matches an
// empty live account.
return nil
return nil, nil
}
if !datastore.IsSafeIdentifier(accountID) || !datastore.IsSafeIdentifier(deviceID) {
return migrationDataNotReadyf("live /info contains an invalid account or device identifier")
return nil, migrationDataNotReadyf("live /info contains an invalid account or device identifier")
}
persistedInfo, err := m.DataStore.GetExactDeviceInfo(accountID, deviceID)
if err != nil {
return migrationDataNotReadyf("DeviceInfo.xml is not persisted under account %q and device %q", accountID, deviceID)
return nil, migrationDataNotReadyf("DeviceInfo.xml is not persisted under account %q and device %q", accountID, deviceID)
}
if persistedInfo.DeviceID != deviceID {
return migrationDataNotReadyf("persisted DeviceInfo.xml identifies device %q instead of %q", persistedInfo.DeviceID, deviceID)
return nil, migrationDataNotReadyf("persisted DeviceInfo.xml identifies device %q instead of %q", persistedInfo.DeviceID, deviceID)
}
snapshot, err := m.DataStore.ReadPresetSnapshot(accountID, deviceID)
if err != nil {
return migrationDataNotReadyf("cannot read the persisted preset snapshot: %v", err)
return nil, migrationDataNotReadyf("cannot read the persisted preset snapshot: %v", err)
}
if snapshot.State != datastore.PresetSnapshotValid {
return migrationDataNotReadyf("persisted Presets.xml is %s", snapshot.State)
return nil, migrationDataNotReadyf("persisted Presets.xml is %s", snapshot.State)
}
if snapshot.NeedsRewrite {
return migrationDataNotReadyf("persisted Presets.xml uses a legacy format that must be refreshed")
return nil, migrationDataNotReadyf("persisted Presets.xml uses a legacy format that must be refreshed")
}
livePresets, err := m.fetchLivePresets(deviceIP)
if err != nil {
return migrationDataNotReadyf("cannot read live /presets: %v", err)
return nil, migrationDataNotReadyf("cannot read live /presets: %v", err)
}
fullXML, err := marge.AccountFullToXMLReadOnly(m.DataStore, accountID)
if err != nil {
return migrationDataNotReadyf("cannot render account /full: %v", err)
return nil, migrationDataNotReadyf("cannot render account /full: %v", err)
}
fullPresets, accountDeviceCount, err := migrationFullPresets(fullXML, deviceID)
if err != nil {
return migrationDataNotReadyf("rendered account /full is incomplete: %v", err)
return nil, migrationDataNotReadyf("rendered account /full is incomplete: %v", err)
}
var warnings []string
// Not a refusal. One account holding every speaker in the household is the
// normal Bose topology, so blocking it would block most setups, and issue
// #614 concluded the shared-account preset wipe is empirical rather than a
// proven mechanism with an open root cause. A stale duplicate entry left
// by a failed /info read or a DHCP lease change would also trip it on a
// genuinely single-speaker setup. Say what was found and let the user
// decide.
if accountDeviceCount != 1 {
return migrationDataNotReadyWithAction(
fmt.Sprintf("rendered account /full contains %d devices; shared-account firmware resync can wipe presets after reboot", accountDeviceCount),
"Move this speaker to a dedicated account, run Data Sync for it, then retry migration.",
)
warnings = append(warnings, fmt.Sprintf(
"migrating into an account that contains %d devices; some firmware has been reported to wipe presets after a reboot-triggered resync of a shared account (issue #614, root cause open)",
accountDeviceCount))
}
persisted := migrationPresetIdentities(snapshot.Presets)
live := migrationPresetIdentities(livePresets)
if mismatch := compareMigrationPresets("persisted snapshot", persisted, "rendered /full", fullPresets); mismatch != "" {
return migrationDataNotReadyf("%s", mismatch)
return nil, migrationDataNotReadyf("%s", mismatch)
}
if mismatch := compareMigrationPresets("live /presets", live, "rendered /full", fullPresets); mismatch != "" {
return migrationDataNotReadyf("%s", mismatch)
return nil, migrationDataNotReadyf("%s", mismatch)
}
return nil
return warnings, nil
}
type migrationPresetIdentity struct {
pkg/service/setup/migration_readiness_test.go
+27 -19
View File
@@ -79,6 +79,8 @@ func livePresetsXML(presets ...models.ServicePreset) string {
return xml.String()
}
func discardWarnings(_ []string, err error) error { return err }
func requireMigrationNotReady(t *testing.T, err error) *MigrationDataNotReadyError {
t.Helper()
if err == nil {
@@ -126,7 +128,7 @@ func TestMigrationDataReadinessBlocksPartialAndFilteredPresets(t *testing.T) {
t.Fatalf("SavePresets: %v", err)
}
requireMigrationNotReady(t, m.checkMigrationDataReady(deviceIP))
requireMigrationNotReady(t, discardWarnings(m.checkMigrationDataReady(deviceIP)))
})
t.Run("preset filtered from full", func(t *testing.T) {
@@ -138,13 +140,13 @@ func TestMigrationDataReadinessBlocksPartialAndFilteredPresets(t *testing.T) {
t.Fatalf("SavePresets: %v", err)
}
notReady := requireMigrationNotReady(t, m.checkMigrationDataReady(deviceIP))
notReady := requireMigrationNotReady(t, discardWarnings(m.checkMigrationDataReady(deviceIP)))
if !strings.Contains(notReady.Reason, "rendered /full") {
t.Fatalf("reason = %q, want rendered /full mismatch", notReady.Reason)
}
})
t.Run("shared account", func(t *testing.T) {
t.Run("shared account warns but does not refuse", func(t *testing.T) {
m, ds, deviceIP := newMigrationReadinessFixture(t, `<presets/>`)
if err := ds.SavePresets(readinessAccount, readinessDevice, nil); err != nil {
t.Fatalf("SavePresets: %v", err)
@@ -158,27 +160,23 @@ func TestMigrationDataReadinessBlocksPartialAndFilteredPresets(t *testing.T) {
t.Fatalf("SaveDeviceInfo sibling: %v", err)
}
notReady := requireMigrationNotReady(t, m.checkMigrationDataReady(deviceIP))
if !strings.Contains(notReady.Reason, "2 devices") {
t.Fatalf("reason = %q, want shared-account device count", notReady.Reason)
warnings, err := m.checkMigrationDataReady(deviceIP)
if err != nil {
t.Fatalf("shared account refused migration: %v", err)
}
if !strings.Contains(notReady.Action, "dedicated account") {
t.Fatalf("action = %q, want dedicated-account guidance", notReady.Action)
}
if !strings.Contains(notReady.Action, "Data Sync") {
t.Fatalf("action = %q, want Data Sync guidance", notReady.Action)
if len(warnings) != 1 || !strings.Contains(warnings[0], "2 devices") {
t.Fatalf("warnings = %v, want one naming the device count", warnings)
}
})
}
func TestMigrationDataReadinessAllowsValidEmptyAndSyncedPresets(t *testing.T) {
t.Run("valid empty", func(t *testing.T) {
m, ds, deviceIP := newMigrationReadinessFixture(t, `<presets/>`)
if err := ds.SavePresets(readinessAccount, readinessDevice, nil); err != nil {
t.Fatalf("SavePresets: %v", err)
}
if err := m.checkMigrationDataReady(deviceIP); err != nil {
if _, err := m.checkMigrationDataReady(deviceIP); err != nil {
t.Fatalf("checkMigrationDataReady: %v", err)
}
})
@@ -190,7 +188,7 @@ func TestMigrationDataReadinessAllowsValidEmptyAndSyncedPresets(t *testing.T) {
t.Fatalf("SavePresets: %v", err)
}
if err := m.checkMigrationDataReady(deviceIP); err != nil {
if _, err := m.checkMigrationDataReady(deviceIP); err != nil {
t.Fatalf("checkMigrationDataReady: %v", err)
}
})
@@ -209,13 +207,13 @@ func TestMigrationDataReadinessDoesNotRewritePresetSnapshots(t *testing.T) {
t.Fatalf("read target snapshot: %v", err)
}
if err = m.checkMigrationDataReady(deviceIP); err != nil {
if _, err = m.checkMigrationDataReady(deviceIP); err != nil {
t.Fatalf("checkMigrationDataReady: %v", err)
}
assertPresetSnapshotUnchanged(t, targetPath, before)
})
t.Run("rejected shared account", func(t *testing.T) {
t.Run("shared account leaves a sibling snapshot alone", func(t *testing.T) {
m, ds, deviceIP := newMigrationReadinessFixture(t, `<presets/>`)
if err := ds.SavePresets(readinessAccount, readinessDevice, nil); err != nil {
t.Fatalf("SavePresets target: %v", err)
@@ -236,7 +234,17 @@ func TestMigrationDataReadinessDoesNotRewritePresetSnapshots(t *testing.T) {
t.Fatalf("write legacy sibling snapshot: %v", err)
}
requireMigrationNotReady(t, m.checkMigrationDataReady(deviceIP))
// Reading the sibling's account to count devices must not canonicalise
// its legacy Presets.xml, which is the point of this case; the shared
// account itself is only a warning.
warnings, err := m.checkMigrationDataReady(deviceIP)
if err != nil {
t.Fatalf("shared account refused migration: %v", err)
}
if len(warnings) != 1 || !strings.Contains(warnings[0], "2 devices") {
t.Fatalf("warnings = %v, want one naming the device count", warnings)
}
assertPresetSnapshotUnchanged(t, siblingPath, legacy)
})
}
@@ -272,7 +280,7 @@ func TestMigrationDataReadinessAllowsUnpairedSpeaker(t *testing.T) {
m := NewManager("http://aftertouch.example:8000", datastore.NewDataStore(t.TempDir()), nil)
if err := m.checkMigrationDataReady(strings.TrimPrefix(speaker.URL, "http://")); err != nil {
if _, err := m.checkMigrationDataReady(strings.TrimPrefix(speaker.URL, "http://")); err != nil {
t.Fatalf("unpaired speaker was refused migration: %v", err)
}
}
pkg/service/setup/setup.go
+13 -3
View File
@@ -873,10 +873,18 @@ func (m *Manager) firstCACertBodyLine() (string, bool) {
// MigrateSpeaker configures the speaker at the given IP to use this service.
func (m *Manager) MigrateSpeaker(deviceIP, targetURL, proxyURL string, options map[string]string, method MigrationMethod) (string, error) {
if err := m.checkMigrationDataReady(deviceIP); err != nil {
readinessWarnings, err := m.checkMigrationDataReady(deviceIP)
if err != nil {
return "", err
}
// Surfaced in the migration log the UI shows, alongside the other
// "Warning:" lines, so an advisory reaches the user without blocking them.
var preflightLogs string
for _, warning := range readinessWarnings {
preflightLogs += fmt.Sprintf("Warning: %s\n", warning)
}
if targetURL == "" {
targetURL = m.ServerURL
}
@@ -890,10 +898,12 @@ func (m *Manager) MigrateSpeaker(deviceIP, targetURL, proxyURL string, options m
// rooted via remote_services.
if method == MigrationMethodTelnet {
urls := telnetURLsFromOptions(targetURL, options)
return m.migrateViaTelnet(deviceIP, targetURL, urls)
telnetLogs, telnetErr := m.migrateViaTelnet(deviceIP, targetURL, urls)
return preflightLogs + telnetLogs, telnetErr
}
var logs string
logs := preflightLogs
// 0. Off-device backup for safety
if backupErr := m.BackupConfigOffDevice(deviceIP); backupErr != nil {