From 3b7ba8bd2f643e546197e8cc11e649c33a5bf6c6 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 9 Aug 2026 11:49:49 +0200 Subject: [PATCH] feat(cli): auto-pair unpaired devices before the enable-ssh injection #515 comment 5230833551: on a genuinely unpaired (factory-reset) device, margeServerUrl is reportedly never polled at all, so the boseurls SSH-enable injection has no read cycle to fire on regardless of any command delay. enable-ssh now checks /info first and, if margeAccountUUID is empty, pairs the device via the existing PairAccount helper (HTTP /setMargeAccount, telnet fallback) before running the injection. Adds setup.Manager.EnsureMargeAccountPaired plus --no-auto-pair (skip entirely) and --account (use a specific 7-digit ID instead of a generated one, e.g. to match one already in the datastore) flags on enable-ssh. Pairing failure is a warning, not fatal, since the claim is unconfirmed on this specific hardware and existing working flows must not regress. Co-Authored-By: Claude Sonnet 5 --- cmd/soundtouch-cli/cmd_setup.go | 51 ++++++++++++ pkg/service/setup/marge_pairing.go | 39 +++++++++ pkg/service/setup/marge_pairing_test.go | 106 ++++++++++++++++++++++++ 3 files changed, 196 insertions(+) diff --git a/cmd/soundtouch-cli/cmd_setup.go b/cmd/soundtouch-cli/cmd_setup.go index f89e3d4..65efe09 100644 --- a/cmd/soundtouch-cli/cmd_setup.go +++ b/cmd/soundtouch-cli/cmd_setup.go @@ -600,6 +600,41 @@ func runEnableSSHInjection(m *setup.Manager, host, serviceURL string, fullConfig return nil } +// ensureMargeAccountPaired checks /info and pairs an unpaired device before +// the SSH-enable injection runs — see setup.EnsureMargeAccountPaired for why. +// Pairing failure is logged as a warning, not fatal: the claim that an +// unpaired device never polls margeServerUrl is not yet confirmed on every +// device this command targets, so the injection is still worth attempting +// even if the pairing step itself couldn't be verified. +func ensureMargeAccountPaired(m *setup.Manager, deviceIP, wantAccountID string) { + var t setup.TelnetClient + + if m.NewTelnet != nil { + t = m.NewTelnet(deviceIP) + + if dialErr := t.Dial(); dialErr != nil { + t = nil + } else { + defer func() { _ = t.Close() }() + } + } + + accountID, alreadyPaired, logs, err := m.EnsureMargeAccountPaired(deviceIP, wantAccountID, t) + if logs != "" { + fmt.Print(logs) + } + + switch { + case err != nil: + PrintWarning(fmt.Sprintf("Pairing check failed (%v) — continuing anyway; the SSH-enable injection may not "+ + "fire on an unpaired device (#515).", err)) + case alreadyPaired: + fmt.Printf("Device already paired (margeAccountUUID=%s).\n", accountID) + default: + fmt.Printf("Device was unpaired — paired it with generated account %s so margeServerUrl gets polled (#515).\n", accountID) + } +} + func setupEnableSSHCmd() *cli.Command { return &cli.Command{ Name: "enable-ssh", @@ -630,6 +665,18 @@ func setupEnableSSHCmd() *cli.Command { "the same commands sent back-to-back left sshd down after reboot, but succeeded sent one at a time with ~7s gaps. Raise this if the " + "default doesn't work on your device; 0 sends everything back-to-back (the old behavior)", }, + &cli.BoolFlag{ + Name: "no-auto-pair", + Usage: "Skip the automatic pairing check: by default, enable-ssh reads /info first and pairs an unpaired " + + "(factory-reset) device with an account ID, since an unpaired device reportedly never " + + "polls margeServerUrl at all (#515) — the injection would have nothing to fire on otherwise", + }, + &cli.StringFlag{ + Name: "account", + Usage: "Only used when the device is unpaired and --no-auto-pair is not set: 7-digit account ID to pair " + + "with (empty = generate one). Use this if you already know which account this device should end up " + + "on (e.g. to match one already in the datastore) rather than getting a random one now", + }, &cli.BoolFlag{ Name: "no-reset-urls", Usage: "Skip restoring clean boseurls after SSH is up (leaves the injected marge URL in place)", @@ -663,6 +710,10 @@ func setupEnableSSHCmd() *cli.Command { serviceURL = "https://aftertouch.invalid" } + if !c.Bool("no-auto-pair") { + ensureMargeAccountPaired(m, cfg.Host, c.String("account")) + } + if err := runEnableSSHInjection(m, cfg.Host, serviceURL, c.Bool("full-config"), c.Duration("command-delay")); err != nil { return err } diff --git a/pkg/service/setup/marge_pairing.go b/pkg/service/setup/marge_pairing.go index e4c8e9b..1ff33fd 100644 --- a/pkg/service/setup/marge_pairing.go +++ b/pkg/service/setup/marge_pairing.go @@ -108,6 +108,45 @@ func (m *Manager) PairAccount(deviceIP, accountID string, t TelnetClient) (PairA return result, logs.String(), nil } +// EnsureMargeAccountPaired reads the device's /info and, if margeAccountUUID +// is empty (a genuinely unpaired, factory-reset device), pairs it via +// PairAccount using wantAccountID if given, otherwise a freshly generated ID. +// See #515 comment 5230833551: on an unpaired device, margeServerUrl is +// reportedly never polled at all, so the boseurls SSH-enable injection has no +// read cycle to fire on regardless of command delay — pairing first gives it +// one. accountID is empty when GetLiveDeviceInfo itself fails; otherwise it +// is either the device's existing margeAccountUUID (alreadyPaired=true) or +// the account ID just paired with. +func (m *Manager) EnsureMargeAccountPaired(deviceIP, wantAccountID string, t TelnetClient) (accountID string, alreadyPaired bool, logs string, err error) { + info, infoErr := m.GetLiveDeviceInfo(deviceIP) + if infoErr != nil { + return "", false, "", fmt.Errorf("read /info: %w", infoErr) + } + + if info.MargeAccountUUID != "" { + return info.MargeAccountUUID, true, "", nil + } + + target := wantAccountID + if target == "" { + generated, genErr := GenerateAccountID(nil) + if genErr != nil { + return "", false, "", fmt.Errorf("generate account id: %w", genErr) + } + + target = generated + } else if !IsValidAccountID(target) { + return "", false, "", fmt.Errorf("invalid account id %q: must be exactly 7 digits", target) + } + + _, pairLogs, pairErr := m.PairAccount(deviceIP, target, t) + if pairErr != nil { + return target, false, pairLogs, pairErr + } + + return target, false, pairLogs, nil +} + // probeSetMargeAccount fetches /supportedURLs and reports whether // /setMargeAccount is in the listing. func (m *Manager) probeSetMargeAccount(deviceIP string) (bool, error) { diff --git a/pkg/service/setup/marge_pairing_test.go b/pkg/service/setup/marge_pairing_test.go index b5c14b0..1d6c05a 100644 --- a/pkg/service/setup/marge_pairing_test.go +++ b/pkg/service/setup/marge_pairing_test.go @@ -2,6 +2,7 @@ package setup import ( "errors" + "fmt" "io" "net" "net/http" @@ -21,6 +22,7 @@ type fakeDevice struct { postStatus int // status code returned for POST /setMargeAccount postDelay time.Duration gotPostBody string + margeAccountUUID string // served by /info; empty means "unpaired" } func newFakeDevice(t *testing.T) *fakeDevice { @@ -55,6 +57,11 @@ func newFakeDevice(t *testing.T) *fakeDevice { w.WriteHeader(d.postStatus) }) + mux.HandleFunc("/info", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/xml") + fmt.Fprintf(w, `%s`, d.margeAccountUUID) + }) + d.srv = httptest.NewServer(mux) u := d.srv.URL[len("http://"):] @@ -254,6 +261,105 @@ func TestPairAccount_TelnetTransportErrorReturned(t *testing.T) { } } +func TestEnsureMargeAccountPaired_AlreadyPairedSkipsPairing(t *testing.T) { + d := newFakeDevice(t) + d.margeAccountUUID = "1234567" + + f := &fakeTelnet{} + + m := NewManager("", nil, nil) + + accountID, alreadyPaired, _, err := m.EnsureMargeAccountPaired(d.addr, "", f) + if err != nil { + t.Fatalf("EnsureMargeAccountPaired: %v", err) + } + + if !alreadyPaired { + t.Error("alreadyPaired should be true") + } + + if accountID != "1234567" { + t.Errorf("accountID = %q, want the existing margeAccountUUID", accountID) + } + + if len(f.commands) != 0 || d.gotPostBody != "" { + t.Error("pairing should not have been attempted for an already-paired device") + } +} + +func TestEnsureMargeAccountPaired_UnpairedGeneratesAndPairs(t *testing.T) { + d := newFakeDevice(t) + d.margeAccountUUID = "" + + m := NewManager("", nil, nil) + + accountID, alreadyPaired, _, err := m.EnsureMargeAccountPaired(d.addr, "", nil) + if err != nil { + t.Fatalf("EnsureMargeAccountPaired: %v", err) + } + + if alreadyPaired { + t.Error("alreadyPaired should be false for an unpaired device") + } + + if !IsValidAccountID(accountID) { + t.Errorf("accountID %q is not a valid generated ID", accountID) + } + + if !strings.Contains(d.gotPostBody, ""+accountID+"") { + t.Errorf("device received %q, want it to be paired with the generated %q", d.gotPostBody, accountID) + } +} + +func TestEnsureMargeAccountPaired_UnpairedUsesWantAccountID(t *testing.T) { + d := newFakeDevice(t) + d.margeAccountUUID = "" + + m := NewManager("", nil, nil) + + accountID, alreadyPaired, _, err := m.EnsureMargeAccountPaired(d.addr, "7654321", nil) + if err != nil { + t.Fatalf("EnsureMargeAccountPaired: %v", err) + } + + if alreadyPaired { + t.Error("alreadyPaired should be false for an unpaired device") + } + + if accountID != "7654321" { + t.Errorf("accountID = %q, want the requested 7654321", accountID) + } + + if !strings.Contains(d.gotPostBody, "7654321") { + t.Errorf("device received %q, want the requested account id", d.gotPostBody) + } +} + +func TestEnsureMargeAccountPaired_RejectsInvalidWantAccountID(t *testing.T) { + d := newFakeDevice(t) + d.margeAccountUUID = "" + + m := NewManager("", nil, nil) + + _, _, _, err := m.EnsureMargeAccountPaired(d.addr, "not-7-digits", nil) + if err == nil { + t.Fatal("expected an error for an invalid --account value") + } +} + +func TestEnsureMargeAccountPaired_PropagatesPairingFailure(t *testing.T) { + d := newFakeDevice(t) + d.margeAccountUUID = "" + d.supportsSetMarge = false + + m := NewManager("", nil, nil) + + _, _, _, err := m.EnsureMargeAccountPaired(d.addr, "1234567", nil) + if err == nil { + t.Fatal("expected an error when HTTP pairing is unsupported and no telnet client is given") + } +} + func TestIsValidAccountID(t *testing.T) { cases := []struct { in string