fix(cli): stop pushing to Marge, make its preflight read advisory

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>
This commit is contained in:
Tobias Gesellchen
2026-09-05 12:29:37 +02:00
co-authored by Claude Sonnet 5
parent cb87d86a0b
commit 369887f642
2 changed files with 83 additions and 9 deletions
+39 -9
View File
@@ -182,21 +182,51 @@ func newGroupCoordinator(config *ClientConfig) *stereopair.Coordinator {
}
cleanupClient := &http.Client{Timeout: lifecycleConfig.Timeout}
cleanup, preflight, rename := cliStereoPairGenerationPersistence(cleanupClient)
return stereopair.NewWithGenerationLifecyclePersistence(
groupClientFactory(&lifecycleConfig),
func(ref stereopair.GenerationRef) error {
return stereopair.DeleteMargeGroupGeneration(cleanupClient, ref)
},
func(refs []stereopair.GenerationRef) error {
return stereopair.EnsureMargeNoGroupGenerations(cleanupClient, refs)
},
func(ref stereopair.GenerationRef, name string) error {
return stereopair.RenameMargeGroupGeneration(cleanupClient, ref, name)
},
cleanup, preflight, rename,
)
}
// cliStereoPairGenerationPersistence wires generation-lifecycle hooks for
// the CLI: cleanup and rename are no-ops, and preflight's read-only
// dangling-generation check is advisory (mirrors -service's and -player's
// own equivalents).
//
// A speaker self-reports its own 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. Proactively pushing the
// same update ourselves would duplicate that against a backend we generally
// can't authenticate to anyway (real Bose cloud, another AfterTouch/SoundCork
// instance, ...). The one part with a distinct purpose -- checking for a
// dangling stale generation before a new Create -- is still attempted, but
// its failure must not block Create: it's a best-effort safety net on top of
// the coordinator's own physical preflight, not the primary guard.
func cliStereoPairGenerationPersistence(
cleanupClient *http.Client,
) (stereopair.GenerationCleanup, stereopair.GenerationPreflight, stereopair.GenerationRename) {
cleanup := func(stereopair.GenerationRef) error {
return nil
}
preflight := func(refs []stereopair.GenerationRef) error {
if err := stereopair.EnsureMargeNoGroupGenerations(cleanupClient, refs); err != nil {
PrintWarning(fmt.Sprintf("stereo-pair external generation preflight inconclusive, proceeding: %v", err))
}
return nil
}
rename := func(stereopair.GenerationRef, string) error {
return nil
}
return cleanup, preflight, rename
}
// groupClientFactory addresses every member directly while retaining the
// effective CLI port and timeout.
func groupClientFactory(config *ClientConfig) stereopair.ClientFactory {
+44
View File
@@ -209,3 +209,47 @@ func testServerHostPort(t *testing.T, serverURL string) (string, int) {
return host, port
}
// TestCLIStereoPairGenerationPersistenceSkipsWritesButAttemptsRead covers
// the CLI's generation-lifecycle wiring: cleanup and rename must never push
// to a Marge backend -- the speaker itself self-reports its own group
// teardown/rename to whatever backend it's configured with (see
// HandleMargeDeleteGroup/HandleMargeModifyGroup, only ever called by
// speakers). Preflight still attempts its read-only dangling-generation
// check, but a failure there (network error, wrong credentials, real Bose
// cloud rejecting us, ...) must not block Create.
func TestCLIStereoPairGenerationPersistenceSkipsWritesButAttemptsRead(t *testing.T) {
getCalls := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/device/LEFT-ID/group") {
getCalls++
http.Error(w, "unauthorized", http.StatusUnauthorized)
return
}
t.Fatalf("unexpected external %s %s: cleanup/rename must not write to a backend the CLI doesn't own", r.Method, r.URL.Path)
}))
defer server.Close()
cleanup, preflight, rename := cliStereoPairGenerationPersistence(server.Client())
ref := stereopair.GenerationRef{
DeviceID: "LEFT-ID", AccountID: "ACCOUNT1", MargeURL: server.URL + "/marge",
GroupID: "7654321",
}
if err := rename(ref, "Renamed living room"); err != nil {
t.Fatalf("rename = %v, want nil (speaker self-reports its own rename)", err)
}
if err := cleanup(ref); err != nil {
t.Fatalf("cleanup = %v, want nil (speaker self-reports its own teardown)", err)
}
if err := preflight([]stereopair.GenerationRef{ref}); err != nil {
t.Fatalf("preflight = %v, want nil: an unauthenticated external check must not block Create", err)
}
if getCalls != 1 {
t.Fatalf("external GET calls = %d, want exactly 1 (preflight must still attempt the read)", getCalls)
}
}