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(stereo): drop unjustified same-Marge-account Create restriction

Browse Source 
validateCreateCandidates rejected LEFT/RIGHT pairs whenever their
MargeAccountUUID differed, even though no hardware or design
requirement calls for it: the pre-lifecycle CLI's direct /addGroup
calls never checked Marge accounts, and the existing read/display
projection has no account awareness either. Confirmed via live
hardware testing that this newly rejects a real, previously-working
cross-account stereo pair.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-09-05 11:08:04 +02:00
co-authored by Claude Sonnet 5
parent 3cdd20883d
commit 536b465a51
2 changed files with 74 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/stereopair/coordinator.go
-5
View File
@@ -489,11 +489,6 @@ func validateCreateCandidates(states []memberState) {
setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT device IDs must be distinct", ErrInvalidRequest))
}
if strings.TrimSpace(states[0].info.MargeAccountUUID) == "" ||
states[0].info.MargeAccountUUID != states[1].info.MargeAccountUUID {
setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT speakers must use the same Marge account", ErrConflict))
}
if !SameMargeBackend(states[0].info.MargeURL, states[1].info.MargeURL) {
setPreflightError(&states[1], fmt.Errorf("%w: LEFT and RIGHT speakers must use the same Marge backend", ErrConflict))
}
pkg/stereopair/coordinator_test.go
+74 -54
View File
@@ -191,12 +191,6 @@ func TestCreateRejectsInvalidPreflightWithoutMutation(t *testing.T) {
right.capabilities.DeviceID = "OTHER-ID"
},
},
{
name: "different Marge accounts",
mutate: func(_ *fakeClient, right *fakeClient) {
right.info.MargeAccountUUID = "ACCOUNT2"
},
},
{
name: "different Marge backends",
mutate: func(_ *fakeClient, right *fakeClient) {
@@ -238,6 +232,24 @@ func TestCreateRejectsInvalidPreflightWithoutMutation(t *testing.T) {
}
}
// TestCreateAllowsDifferingMargeAccounts guards against reintroducing an
// unjustified same-Marge-account requirement: a real, working cross-account
// stereo pair (created pre-lifecycle-API via the CLI's direct /addGroup
// calls, which never checked Marge accounts) must remain creatable through
// this API too. See i655 code-review finding #0.
func TestCreateAllowsDifferingMargeAccounts(t *testing.T) {
left, right, coordinator := newCreateCoordinator()
right.info.MargeAccountUUID = "ACCOUNT2"
result, err := coordinator.Create(CreateRequest{LeftIPAddress: leftIP, RightIPAddress: rightIP, Name: "Pair"})
if err != nil || result.Status != StatusSucceeded {
t.Fatalf("result = %+v, err = %v; want succeeded despite differing Marge accounts", result, err)
}
if left.addRequest == nil || right.addRequest == nil {
t.Fatal("addGroup not called for a valid cross-account pair")
}
}
func TestCreatePartialFailureIsCompensatedAndReported(t *testing.T) {
left, right, coordinator := newCreateCoordinator()
right.addErr = errors.New("right add failed")
@@ -734,56 +746,64 @@ func TestRenamePersistsVerifiedGeneration(t *testing.T) {
}
}
func TestRenameRejectsSplitMargeOwnershipWithoutMutation(t *testing.T) {
for _, test := range []struct {
name string
mutate func(*models.DeviceInfo)
}{
{
name: "account",
mutate: func(info *models.DeviceInfo) {
info.MargeAccountUUID = "OTHER-ACCOUNT"
},
func TestRenameRejectsSplitMargeBackendWithoutMutation(t *testing.T) {
group := configuredGroup("Old Name")
left := readyClient(leftID, "Left")
right := readyClient(rightID, "Right")
left.group = cloneGroup(group)
right.group = cloneGroup(group)
right.info.MargeURL = "http://other-aftertouch.example"
persistenceCalls := 0
coordinator := NewWithGenerationLifecyclePersistence(
factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}),
nil,
nil,
func(_ GenerationRef, _ string) error {
persistenceCalls++
return nil
},
{
name: "backend",
mutate: func(info *models.DeviceInfo) {
info.MargeURL = "http://other-aftertouch.example"
},
},
} {
t.Run(test.name, func(t *testing.T) {
group := configuredGroup("Old Name")
left := readyClient(leftID, "Left")
right := readyClient(rightID, "Right")
left.group = cloneGroup(group)
right.group = cloneGroup(group)
test.mutate(right.info)
persistenceCalls := 0
coordinator := NewWithGenerationLifecyclePersistence(
factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}),
nil,
nil,
func(_ GenerationRef, _ string) error {
persistenceCalls++
return nil
},
)
)
result, err := coordinator.Rename(RenameRequest{
MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name",
})
if err == nil || result.Status != StatusFailed {
t.Fatalf("result = %+v, err = %v; want failed ownership preflight", result, err)
}
if left.updateRequest != nil || right.updateRequest != nil || persistenceCalls != 0 {
t.Fatalf("ownership mismatch mutated pair: left=%+v right=%+v persistence=%d",
left.updateRequest, right.updateRequest, persistenceCalls)
}
if len(result.Members) != 2 || !errors.Is(result.Members[1].PreflightError, ErrConflict) {
t.Fatalf("members = %+v; want peer ownership conflict", result.Members)
}
})
result, err := coordinator.Rename(RenameRequest{
MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name",
})
if err == nil || result.Status != StatusFailed {
t.Fatalf("result = %+v, err = %v; want failed ownership preflight", result, err)
}
if left.updateRequest != nil || right.updateRequest != nil || persistenceCalls != 0 {
t.Fatalf("ownership mismatch mutated pair: left=%+v right=%+v persistence=%d",
left.updateRequest, right.updateRequest, persistenceCalls)
}
if len(result.Members) != 2 || !errors.Is(result.Members[1].PreflightError, ErrConflict) {
t.Fatalf("members = %+v; want peer ownership conflict", result.Members)
}
}
// TestRenameAllowsDifferingMargeAccounts guards against reintroducing an
// unjustified same-Marge-account requirement into the shared loadPair
// preflight used by Rename/Dissolve. See i655 code-review finding #0.
func TestRenameAllowsDifferingMargeAccounts(t *testing.T) {
group := configuredGroup("Old Name")
left := readyClient(leftID, "Left")
right := readyClient(rightID, "Right")
left.group = cloneGroup(group)
right.group = cloneGroup(group)
right.info.MargeAccountUUID = "OTHER-ACCOUNT"
coordinator := NewWithGenerationLifecyclePersistence(
factoryFor(map[string]*fakeClient{leftIP: left, rightIP: right}),
nil,
nil,
func(_ GenerationRef, _ string) error { return nil },
)
result, err := coordinator.Rename(RenameRequest{
MemberIPAddress: leftIP, ExpectedGroupID: "PAIR-ID", Name: "New Name",
})
if err != nil || result.Status != StatusSucceeded {
t.Fatalf("result = %+v, err = %v; want succeeded despite differing Marge accounts", result, err)
}
if left.updateRequest == nil || right.updateRequest == nil {
t.Fatal("updateGroup not called for a valid cross-account pair")
}
}