Confirmed on real hardware: selecting it bare produces the byte-identical
stub RADIO_BROWSER produced, with the source name echoed as the item name,
empty type and location, isPresetable false, and PlayStatus empty, while the
previously playing stream carries on. Nothing about the speaker's audio
changes; only /now_playing does.
Its fallback browser is Play URL, which is the page in this app that emits
that source: HandlePlayURL builds a ContentItem with
Source "LOCAL_INTERNET_RADIO".
ALEXA is advertised READY as well but stays unlisted: whether a bare select
resumes anything for it is still unverified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A speaker advertises RADIO_BROWSER and TUNEIN in /sources with
status="READY", so the player rendered them as ordinary source buttons and
issued the same bare /select it uses for AUX or SPOTIFY: source and account,
no ContentItem.
They are not selectable inputs. Playing one needs a station ContentItem
carrying a Location, which is what stations.ResolveContentItem builds and
what HandlePlayRadioBrowser sends. Given a bare select the speaker accepts
the command and parks on a stub now-playing instead, observed on real
hardware:
<nowPlaying source="RADIO_BROWSER" sourceAccount="">
<ContentItem source="RADIO_BROWSER" type="" location="" isPresetable="false">
<itemName>RADIO_BROWSER</itemName>
</ContentItem>
</nowPlaying>
Empty type, empty location, itemName echoing the source name, and no
playStatus, while the previous audio keeps playing. The speaker then reports
that stub indefinitely, so the player shows RadioBrowser while Spotify is
audible. Worse, the readback sees the source it asked for and confirms
"Source selected" for a command that produced a dead state.
Clicking such a source now resumes its most recent station, using the
Recents entry's own ContentItem, which is the real item the speaker was
given and carries the Location a bare select cannot supply. With nothing to
resume, or if the lookup fails, the click navigates to that provider's
browser rather than issuing a select known to strand the speaker.
Only RADIO_BROWSER and TUNEIN are treated this way. LOCAL_INTERNET_RADIO and
ALEXA are advertised READY too, but whether a bare select resumes anything
for them is unverified, so they keep today's behaviour.
api.playChecked mirrors api.selectSource: the command path needs a write
whose failure it can see, while api.play keeps its response-level behaviour
for the callers that already rely on it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Selecting a source cost up to ~21 speaker requests. Two multipliers, both
removed here.
A match on the first readback only marked the command provisional and left
the later deadlines running, so the happy path always spent all three. The
reason to keep watching is real: /select answers 200 even for a source the
speaker rejects seconds later, surfacing as a transition to an error source.
But the event stream already reports that transition as it happens, and the
effect watching nowPlayingUpdated already turns it into a failure. Polling
on top is re-asking a question we are subscribed to the answer of. The
remaining readbacks are now kept only when the readback itself reports no
live event stream, which is the case they are actually needed for.
Each readback also fetched the whole device, and HandleAPIDevice runs a full
UpdateDeviceStatus: six sequential speaker calls plus /getGroup on a
stereo-capable model, to answer one question, against a device the readback
may be checking on precisely because it is slow. GET
/devices/{id}/now-playing refreshes only /now_playing and returns the same
shape, under FieldNowPlaying's generation so it still orders against push
events and concurrent polls, and reporting to the health tracker like any
other HTTP round.
A confirmed selection on a speaker with a live event stream now costs one
speaker request instead of about twenty-one. A speaker whose events are not
arriving keeps the full three-readback window, at one request each.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A failed /sources read went through CompleteFieldPoll, so it consumed the
field generation. That let a newer failed read discard an older, still
in-flight read that had succeeded, and since the player renders
disabled=${sourcesStale}, one transient hiccup could disable every source
button until the next fully successful poll, up to 30s later, even though a
valid inventory had just arrived.
A failure carries no inventory, so there is nothing to order and no reason
to spend the generation on it. ApplySourcesRead now splits the two:
- a success is still fenced by generation, so two successful reads keep
their ordering and an older one cannot overwrite a newer one, and it
always clears the marker;
- a failure is counted instead, and only staleSourcesFailureThreshold in
a row marks the inventory unusable, matching how
offlineFailureThreshold already debounces connectivity in this file.
A genuinely unreachable speaker is therefore stale one poll cycle later
than before, and a single dropped read costs nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When a `devices` snapshot lost the revision comparison, the merge kept the
newer status we already held but took the rest of the incoming entry. That
produced a self-inconsistent device: the server derives stereoPair from the
very status.Group the snapshot lost on, so a snapshot captured before a
pair was dissolved restored its projection alongside a status that had
already cleared the group. StereoPair.js then rendered a pair that no
longer exists, with an expectedGroupId pointing at a deleted group.
Keep the entry we hold instead. The cost is that info riding the same
snapshot waits for the next one, bounded at 5s by the periodic devices
frame and in practice much shorter, because whatever produced the newer
status also queued a device-list broadcast.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DeviceStatus.Revision is per-connection and restarts at 0. The browser
compares revisions to decide which frame wins, but nothing in the frame
said which revision sequence it belonged to.
So a device id backed by a fresh DeviceConnection published revisions
starting at 0 while an open tab still held a high revision for that id, and
the tab rejected every later frame for it: a status frozen until reload.
HandleDeleteDevice broadcasts after removal, which covers the ordinary
remove-then-rediscover path, but a discovery sweep re-adding the host
inside that window yields a snapshot that already contains the device at
revision 0, so no device-less snapshot is ever sent.
Every status now carries an Epoch identifying the connection that produced
it, stamped by both SetStatus and UpdateStatus. The browser compares epochs
first and only falls back to revisions within one epoch, so a newer
connection is accepted regardless of its revision and a frame still in
flight from the replaced connection is rejected regardless of its.
nextStatusEpoch is seeded from the wall clock and forced strictly
increasing, so epochs also keep rising across a service restart, where a
plain counter would restart at 0 and reintroduce the same problem. It is
in milliseconds because the browser compares it as a JSON number and a
nanosecond timestamp exceeds Number.MAX_SAFE_INTEGER.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A nowPlayingUpdated event is authoritative evidence that the speaker
switched, and the effect watching it promotes a pending command to
provisional-confirmed. The last readback then overwrote the command
wholesale with outcome: 'unverified', in both its no-match branch and its
catch, without looking at what the command had already become.
So a selection the speaker confirmed by push at ~1s was reported as
"Source selection unverified" when the 10s readback happened to fail or
returned a now-playing that had since moved on.
The readback window closing now settles such a command as confirmed rather
than retracting it; only a command still pending, which nothing ever
confirmed, becomes unverified. A failed command stays failed.
The push-event promotion also has to record confirmedRevision, which it
previously left unset: without it the later-authoritative-source check
would compare against undefined and the projection would never clear.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rebase derives NowPlayingRevision from the existing FieldNowPlaying
generation rather than a counter of its own. Nothing asserted that it
advances down both paths that write the field, a completed poll and a push
event, which is the property the player's readback loop depends on to tell
a fresh now-playing write from an unrelated field's merge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Selecting a source posts once and then confirms by bounded readback. When
the POST itself failed, the write error was captured into active.writeError
and never surfaced: a rejected command reported nothing for the full 10s
readback window and then a bare "Source selection unverified".
Surfacing it needs a distinction the API layer did not make. checkedReq
collapsed every failure into one Error, but the two cases differ:
- 4xx: every 4xx on these endpoints is produced before AfterTouch calls
the speaker (unknown device, unparseable body, empty source, unknown
action), so the command provably never went out. Nothing can confirm
it; report the failure at once, with the server's reason.
- 5xx and transport errors: handleSourceControl reports a failed
Client.SelectSource through sendControlResponse, which maps any
speaker-call error to 500. A request that timed out after the speaker
already switched is indistinguishable from one it never received, so
the readbacks must keep running and the reason is carried into
whatever outcome they reach.
checkedReq now tags thrown errors with `definitive`, and only a definitive
refusal cancels the readbacks. Outcome text appends the reason when there
is one, so a firmware rejection also names the error source it saw.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main already runs the player's static JS unit tests via `make
test-frontend` over pkg/service/soundtouchweb/frontend_test/*.test.mjs,
wired into .github/workflows/browser-tests.yml. That target was added by
PR #672 and this change predates it.
api.test.mjs was placed in static/js/ instead and run by a second `node
--test` invocation bolted onto test-browser, behind a new NODE variable.
That is a parallel test-wiring convention for one file, and it also mixes
a test into the directory that is served to browsers as static assets.
Move the file next to the other frontend tests and revert the Makefile
change; the existing target and CI job pick it up unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `devices` and `status_update` frames are now applied by revision, but
refreshDevices() still replaced the whole map with whatever the REST call
returned. A refresh issued before a socket update but answered after it
would put the older status back.
Route it through mergeDevicesSnapshot as well. Entries the snapshot omits
are still dropped, so device removal keeps working, and info/stereoPair
still come from the snapshot even when its status loses the comparison.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removing the `ready.length === 0` early return meant a device that has not
been polled yet rendered a "Sources" heading with an empty list and a
"Source list unavailable" notice. That reports a problem where there is
none: having read no inventory is not the same as having one we distrust.
The section is hidden again when there is nothing to offer, and the
availability notice is now reserved for the case it was meant for, an
inventory we hold but refuse to act on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The source inventory was expired by a 30s read-time TTL evaluated inside
DeviceConnection.Status(). The status poll ticker is also exactly 30s
(discovery.go), so the two ran in lockstep and every source button went
disabled for the moment before each refresh.
Deriving staleness per read had a second cost: two reads at the same
Revision could disagree about sourcesStale, which is the only reason the
browser needed mergeDerivedStatus to carry that one bit across an
otherwise-rejected frame. Dropping the TTL removes that whole special
case, and with it a class of "equal revision, different derived state"
reasoning.
Sources are now stale exactly when the last /sources read failed, recorded
at merge time so every change advances Revision. An unreachable device
fails its polls, so the case the TTL was meant to cover is still covered.
The newer-failure-fences-older-success ordering is unchanged: the merge
still runs through CompleteFieldPoll(FieldSources, ...).
Drops SourcesReadAt, sourceCacheTTL, sourceCacheStatusAt, the Status()
projection and mergeDerivedStatus. The TTL-boundary tests go with them;
the fencing and recovery tests stay, and one of them no longer has to pin
its read times to the wall clock to avoid the TTL.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Use a single POST command with bounded delayed readback, revision-order
REST and WebSocket state, and explicit pending, confirmed, unverified, or
failed outcomes. Mark stale source inventories unusable until a successful
refresh and preserve legacy API error handling outside this command path.
Rebased onto main. One conflict could not be resolved by picking a side:
this change introduced its own per-field fencing (fieldRevision,
deviceStatusFieldRevisions, MergeNowPlaying/MergeVolume/MergePresets/
MergeSources/MergeBass/MergeIsConnected) over the same six fields that
main's StatusField mechanism (BeginFieldPoll/CompleteFieldPoll/
ApplyFieldEvent, added by the #654/#668 stack) already orders. Landing
both would leave two independent generation counters guarding the same
state, which is the divergent-fencing bug class that stack already had to
fix three times. Resolved in favour of main's mechanism:
- the Merge* methods and their counters are dropped; the WebSocket
handlers and updateDeviceStatus use main's calls;
- NowPlayingRevision is derived from the existing FieldNowPlaying
generation via recordFieldRevision, not a second counter;
- SetStatus derives Revision from the stored status and supersedes all
field generations, so a replacement cannot reset the revision a
browser is ordering on;
- updateSourcesCache records a failed /sources read through
CompleteFieldPoll, keeping the newer-failure-fences-older-success
property the original had via MergeSourcesFailure;
- the three tests that called the dropped API were ported to it.
Everything else in this change is unmodified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HandleGetZoneCandidates read entry.Device.DeviceInfo (the immutable
discovery-time snapshot) directly instead of Info(), missed when every
other handler in this file was migrated to the live-name accessor. A
speaker renamed after discovery kept showing its stale name in the
"add to my zone" picker indefinitely.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refreshDevices() now throws on a failed /api/devices fetch (needed by
removeDeviceAndRefresh's success/failure contract), but StereoPair's
run() awaited it inside the same try that already notified success --
so a refresh/device-list-fetch hiccup after a successful create/rename/
dissolve rolled the UI back into an "operation failed" error state even
though the mutation itself succeeded. Isolate the post-success refresh
so its failure only surfaces as its own, separate notice.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OnTransportState routed connect/disconnect through
applyConnectionStateEvent, which calls ApplyFieldEvent(FieldConnectivity,
...). That unconditionally bumps FieldConnectivity's applied generation,
so a transient WebSocket transport blip could invalidate a
concurrently-completing, genuinely successful HTTP poll's IsConnected
merge -- exactly the kind of cross-mechanism staleness bug the per-field
fencing was built to prevent. ObserveEventStreamTransport already derives
IsConnected (via applyConnectivityLocked) from the same transport signal,
so the extra call was redundant as well as unsafe.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tuneInSearchSection, TuneInSearchNext, and tuneInProfileNavItem each
implemented the same Station/PlayItem/Topic/Program/Profile dispatch,
but disagreed on what to do with anything else: the first two had no
default case and silently dropped the item entirely (hiding real
content with no trace it existed), while the third treated any
unrecognized type as directly playable with no indication that was a
guess.
Consolidated into one tuneInClassifyItem, used by all three, that
takes a middle path instead of picking one side: an unrecognized type
still gets a playback link (nothing is silently hidden), but its
Subtitle is marked ("Unrecognized type, may not play") so a playback
attempt that doesn't pan out reads as an unsupported content type
rather than a mystery broken link. The player UI (TuneInBrowser.js)
already renders Subtitle verbatim, so this is visible to users, not
just present in the API response.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
handlers_bmx_tunein.go's parseTuneInNavigatePath and stations.go's
navigateTuneIn were byte-for-byte identical (confirmed both already
independently implemented the same "profiles" parsing before PR #677;
that fix was applied consistently to both copies rather than
introducing new duplication). Removed the duplicate: HandleTuneInNavigate
now delegates to stations.Navigate(stations.ProviderTuneIn, wildcard),
the same path stations' own callers already use.
Added TestNavigateTuneIn_ProfilesPathDispatch, covering the single-
segment (current), legacy multi-segment, and empty-URI shapes -- this
package (and the handler package's now-removed copy) had zero test
coverage for this logic before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
TuneInSearch, TuneInSearchNext, and TuneInNavigateProfile each
duplicated the identical "Items" (v1.3)-then-"body" (legacy) fallback.
Extracted tuneInRawItems, used by all three. Pure refactor, no
behavior change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The navigate-500 fix replaced a working call to the existing
tuneInSearchSection helper with new hand-rolled container-handling
logic that silently dropped several things the original already did
correctly:
- An empty container (no "Type" field, only "ContainerType") fell
through to a default branch that treats any unrecognized type as
directly playable, turning the container's own non-playable GuideId
into a bogus playback link. Now skipped instead.
- The Pivots.More.Url "load more" pagination cursor was never read, so
a container with more children than fit on one page silently showed
only the first page. Extracted the cursor-link logic already in
tuneInSearchSection into a shared tuneInMoreCursorLink helper, used
by both.
- The legacy lowercase "children" key (tuneInSearchSection's own
fallback for "Children") was dropped entirely.
- The response's self link used "/v1/navigate/profile/" (singular);
no route dispatcher recognizes that, breaking re-navigation via the
link itself.
- base64.URLEncoding (padded) vs. RawURLEncoding (no padding), two
lines apart building the same kind of href -- decodeBase64URI
already tolerates both, which is why this never surfaced as a
decode failure. Standardized on RawURLEncoding, matching every
other encode site in the file.
- The "profiles" path case (duplicated pre-existing in both
handlers_bmx_tunein.go and stations.go) had no fallback for an empty
encoded URI, unlike the sibling "sub" case a few lines above. Both
copies now fall back the same way "sub" does.
Added TestTuneInNavigateProfileHandlesContainerShapes covering all
four tunein.go fixes against a fixture server modeling the real
two-fetch profile/contents shape.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Splitting mutatingGet into two functions during the main rebase
(mutatingGet keeps the existing result-unmarshaling form used by
/removeGroup; the new mutatingGetConfirmStatus validates a <status>
echo instead, for endpoints with no meaningful response body) left
~30 lines of one-shot-transport setup (clone, disable keep-alives,
CheckRedirect) duplicated between them. Extracted newOneShotHTTPClient
so a future fix to that mechanism can't be applied to one copy and
forgotten in the other.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same fix as -service and -player, for consistency: newGroupCoordinator
proactively pushed group cleanup/rename to an external Marge backend
the CLI doesn't own. A speaker's own firmware already self-reports
that create/rename/teardown to whatever Marge backend it's configured
with -- that's the entire reason HandleMargeAddGroup/HandleMargeModifyGroup/
HandleMargeDeleteGroup exist, they're only ever called by speakers.
Extracted the wiring into cliStereoPairGenerationPersistence (mirrors
the -service/-player equivalents): cleanup and rename are now no-ops,
and preflight's read-only dangling-generation check stays but its
failure no longer blocks Create -- it's printed as a warning instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same reasoning as the -service fix: NewWebApp's default generation-
lifecycle wiring (used by the standalone player, and by embedded
-service until SetStereoPairGenerationPersistence overrides it)
proactively pushed group cleanup/rename to an external Marge backend
the player doesn't own. A speaker's own firmware already self-reports
that create/rename/teardown to whatever Marge backend it's configured
with -- that's the entire reason HandleMargeAddGroup/HandleMargeModifyGroup/
HandleMargeDeleteGroup exist, they're only ever called by speakers.
Extracted the wiring into playerStereoPairGenerationPersistence
(mirroring cmd/soundtouch-service's own testable helper): cleanup and
rename are now no-ops, and preflight's read-only dangling-generation
check stays but its failure no longer blocks Create.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A speaker's own firmware self-reports its group create/rename/teardown
to whatever Marge backend it's configured with -- that's the entire
reason HandleMargeAddGroup/HandleMargeModifyGroup/HandleMargeDeleteGroup
exist, they're only ever called by speakers, never by us. Coordinator
cleanup/rename proactively pushing the same update to an external
(non-local) Marge target duplicated that self-report against a backend
we generally can't authenticate to anyway (real Bose cloud, another
instance, ...), for zero benefit.
The one part of the external path with a real, distinct purpose --
preflight's read-only check for a dangling stale generation before a
new Create -- stays, but its failure (network error, wrong
credentials, an unreachable backend) no longer blocks Create. It's a
best-effort safety net on top of the coordinator's own physical
preflight (capability/zone/reachability checks against the live
speakers), not the primary guard, and the live speakers' own state is
the authoritative signal either way.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A speaker migrated at the DNS level keeps reporting its own MargeURL as
the literal Bose cloud hostname (e.g. https://streaming.bose.com) --
DNS migration only changes how that hostname resolves on the network,
never the device's own advertised URL. embeddedStereoPairGenerationPersistence's
isLocal() only matched against this service's own advertised server
URL, so it misclassified such a speaker as "external" and sent the
stereo-pair Create preflight's generation-conflict check out over the
real internet instead of checking the local datastore. Bose's cloud is
still live enough to answer (just not to authenticate us), so the
check failed with HTTP 401 and hard-blocked Create for an otherwise
perfectly normal DNS-migrated setup.
Added a DNS-hijack-aware check: if this service's own DNS hijack is
active and the reported MargeURL's host is one of the known redirected
Bose hostnames (discovery.InterceptedBoseHosts), treat it as local too.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#672 added pkg/service/soundtouchweb/frontend_test/*.test.mjs (Node's
built-in test runner) but never hooked them into anything: not the
Makefile, not any GitHub Actions workflow. They only ran if someone
happened to invoke `node --test` manually, so CI would stay green even
if isSoundTouch10StereoPair or the DeviceDetail notice markup broke.
Added `make test-frontend` and a step in browser-tests.yml (which
already runs the Go-side chromedp player tests) to run them on every
push/PR, matching the pinned actions/setup-node version already used
by update-static-deps.yml.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Constrain device and playback metadata across narrow layouts while retaining complete values through tooltips and a touch-friendly details disclosure, including RAOP tracks.
The test's own comment said "we point the LAN host at 127.0.0.1", but
the resolver stub actually returned 1.2.3.4 -- a real, internet-
routable address, not 127.0.0.1 -- since the test was first written.
Probing an arbitrary internet destination's reachability depends on
the tester's own network path: a transparent proxy, a DPI middlebox,
or "known test IP" sinkholing can all make 1.2.3.4:443 appear
reachable, failing the test's core assumption outside a sandboxed CI
network. See #683.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pkg/stereopair's sameRoles and pkg/service/datastore's
sameGroupGenerationTopology independently reimplemented the same
Role-keyed topology comparison, but normalized IP addresses
differently (net.ParseIP-only vs. plain string equality) -- exactly
the class of disagreement models.SameGroup was already created to fix
for order-sensitivity. Both now delegate their per-role comparison to
a new models.SameGroupRoles, which treats equal-but-differently-
formatted IPs as a match without regressing the common
both-addresses-unset case either implementation relied on.
models.SameGroup and datastore's sameStereoPair stay distinct
(commented why): both are intentionally ID/IP-agnostic for reasons
unrelated to this consolidation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
completeStereoPairMutation's refreshStereoPairMembersAsync ran after
applyStereoPairProjection, and its UpdateDeviceStatus call always
minted a strictly newer group generation via BeginGroupRefresh -- so
ApplyPolledGroup's staleness guard could never reject it, even if its
/getGroup read raced a slower path and was stale relative to the
mutation that had already completed. The refresh now snapshots each
member's post-projection generation and only applies its own read via
ApplyPolledGroupIfBaseline, which requires nothing else (no other
event or poll) to have changed group state in the meantime. Other
UpdateDeviceStatus callers keep their existing always-newer semantics.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
applyDissolve tore down LEFT/RIGHT as independent goroutines with no
compensation path: a partial failure (one member's RemoveGroup
succeeds, the other never verifies empty even after the existing
reverification retries) only ever produced StatusDegraded, requiring
manual operator intervention. compensateDissolve retries RemoveGroup
on any member still unverified, giving it the same chance to converge
that Create's compensateCreate already gets on its own partial
failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
HandleMargeDeleteAccountGroups (DELETE /streaming/account/{id}/group/,
no group ID) had become a pure acknowledgement, leaving
DeleteAllGroupsForAccount dead code. This is the exact request real
firmware sends on factory reset/teardown; skipping the delete leaves a
stale Group_*.xml behind, permanently rejecting the next legitimate
Create for those devices via EnsureNoGroupsForDevices with no operator
remedy.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
