fix(cli): omit senderIPAddress on master's /addGroup payload

The speaker's GroupService state machine uses the presence of
<senderIPAddress> in the addGroup payload to decide whether it should
form the group as master or join as slave: "SenderIp is provided, I am
the slave". Sending the same XML to both speakers (with senderIP set to
the master's IP) made the master also conclude it was the slave, enter
AddingSlave, time out after 5 s waiting for a master that never
confirmed, and revert. The slave briefly showed GROUP_OK before
following the master back to NoGroup -- the "stereo pair appears for a
few seconds, then disappears" symptom reported in #252.

Send two distinct payloads from propagateAddGroup: the master receives
the base request with no senderIPAddress, the slave receives a copy
with senderIPAddress set to the master's IP. The base request built by
createGroup no longer carries senderIPAddress; the per-role injection
is contained inside propagateAddGroup where the master/slave roles are
unambiguous.

Update TestPropagateAddGroup_BothSucceed to assert the master's body
has no <senderIPAddress> while the slave's body does, so any future
regression on either side fails the test.

Refs #252

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-15 09:05:14 +02:00
co-authored by Claude Opus 4.7
parent f89b2243c2
commit cf62057a26
2 changed files with 36 additions and 9 deletions
+18 -3
View File
@@ -84,7 +84,8 @@ func createGroup(c *cli.Context) error {
{DeviceID: rightInfo.DeviceID, Role: "RIGHT", IPAddress: rightIP},
},
},
SenderIPAddress: leftIP,
// SenderIPAddress is intentionally omitted on the base request.
// propagateAddGroup adds it to the slave's copy only — see comment there.
}
leftClient, err := clientForHost(c, leftIP)
@@ -139,7 +140,21 @@ type addGroupOutcome struct {
// propagateAddGroup POSTs /addGroup to both speakers concurrently and returns
// the (LEFT, RIGHT) outcomes. A non-GROUP_OK Status in the response is
// reported as an error so callers don't have to re-inspect the body.
//
// The two POSTs carry different payloads: the master (LEFT) receives the base
// request with no senderIPAddress so its state machine forms the group as the
// master, while the slave (RIGHT) receives a copy with senderIPAddress set to
// the master's IP so its state machine joins as the slave. Sending the same
// payload to both makes both speakers think they're the slave — they enter
// AddingSlave, wait for a master that never confirms, time out after 5 s, and
// revert (issue #252).
func propagateAddGroup(left, right *client.Client, leftIP, rightIP string, req *models.Group) (addGroupOutcome, addGroupOutcome) {
masterReq := *req
masterReq.SenderIPAddress = ""
slaveReq := *req
slaveReq.SenderIPAddress = leftIP
var (
wg sync.WaitGroup
leftOut, rightOut addGroupOutcome
@@ -150,13 +165,13 @@ func propagateAddGroup(left, right *client.Client, leftIP, rightIP string, req *
go func() {
defer wg.Done()
leftOut = postAddGroup(left, leftIP, req)
leftOut = postAddGroup(left, leftIP, &masterReq)
}()
go func() {
defer wg.Done()
rightOut = postAddGroup(right, rightIP, req)
rightOut = postAddGroup(right, rightIP, &slaveReq)
}()
wg.Wait()
+18 -6
View File
@@ -61,7 +61,8 @@ func sampleGroupRequest(leftIP, rightIP string) *models.Group {
{DeviceID: "F45EAB3115DA", Role: "RIGHT", IPAddress: rightIP},
},
},
SenderIPAddress: leftIP,
// senderIPAddress is intentionally not set here; propagateAddGroup
// adds it to the slave's copy only.
}
}
@@ -95,19 +96,30 @@ func TestPropagateAddGroup_BothSucceed(t *testing.T) {
t.Errorf("RIGHT group = %+v, want status=GROUP_OK", rightOut.group)
}
// Both speakers must have received the same payload, including senderIPAddress.
for _, bodies := range []*[]string{leftBodies, rightBodies} {
// Both speakers must have received the roles, but only the slave's payload
// carries senderIPAddress — see propagateAddGroup for the why.
for label, bodies := range map[string]*[]string{"LEFT": leftBodies, "RIGHT": rightBodies} {
if len(*bodies) != 1 {
t.Fatalf("expected exactly one POST, got %d", len(*bodies))
t.Fatalf("%s: expected exactly one POST, got %d", label, len(*bodies))
}
body := (*bodies)[0]
for _, want := range []string{"<role>LEFT</role>", "<role>RIGHT</role>", "<senderIPAddress>192.168.1.131</senderIPAddress>"} {
for _, want := range []string{"<role>LEFT</role>", "<role>RIGHT</role>"} {
if !strings.Contains(body, want) {
t.Errorf("body missing %q\nbody:\n%s", want, body)
t.Errorf("%s body missing %q\nbody:\n%s", label, want, body)
}
}
}
leftBody := (*leftBodies)[0]
if strings.Contains(leftBody, "<senderIPAddress>") {
t.Errorf("LEFT (master) body must NOT carry <senderIPAddress>, otherwise the master flips into slave mode (issue #252)\nbody:\n%s", leftBody)
}
rightBody := (*rightBodies)[0]
if !strings.Contains(rightBody, "<senderIPAddress>192.168.1.131</senderIPAddress>") {
t.Errorf("RIGHT (slave) body must carry <senderIPAddress>192.168.1.131</senderIPAddress>\nbody:\n%s", rightBody)
}
}
func TestPropagateAddGroup_RightFails(t *testing.T) {