diff --git a/docs/content/docs/guides/ON-DEVICE-INSTALL-WALKTHROUGH.md b/docs/content/docs/guides/ON-DEVICE-INSTALL-WALKTHROUGH.md index e5fdcde..868801e 100644 --- a/docs/content/docs/guides/ON-DEVICE-INSTALL-WALKTHROUGH.md +++ b/docs/content/docs/guides/ON-DEVICE-INSTALL-WALKTHROUGH.md @@ -84,7 +84,7 @@ rm -f /mnt/nv/aftertouch/soundtouch-cli df -h /mnt/nv # confirm space recovered ``` -> **From v0.89.0 onwards the installer prunes stale artefacts automatically** +> **From v0.93.0 onwards the installer prunes stale artefacts automatically** > during every upgrade — manual cleanup should no longer be necessary on > fresh installs. diff --git a/pkg/service/handlers/web/index.html b/pkg/service/handlers/web/index.html index 51e3da0..bbae33a 100644 --- a/pkg/service/handlers/web/index.html +++ b/pkg/service/handlers/web/index.html @@ -963,7 +963,7 @@ @@ -975,7 +975,7 @@ @@ -987,7 +987,7 @@ @@ -999,7 +999,7 @@ diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 0baca47..d6886b9 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -2776,6 +2776,26 @@ function onPlanTargetURLChange() { saved.innerText = "✏️ unsaved change — click \"Save as default\" to persist"; saved.style.color = "#bf6900"; } + + // Re-derive the four service URL fields from the new Target URL, same + // as the initial pre-fill on summary render. fillPlanURLInputs still + // only overwrites fields the user hasn't hand-edited (tracked via + // dataset.autofilled), so this doesn't clobber genuinely manual edits. + // Without this, changing Target Domain to e.g. localhost left the four + // fields pointed at a stale default with no warning until the user + // edited them by hand (#621 follow-up). + const soundcork = document.getElementById("plan-soundcork-mode") && + document.getElementById("plan-soundcork-mode").checked; + fillPlanURLInputs(defaultServiceURLs(v, {soundcorkMode: soundcork})); +} + +// onPlanURLFieldEdited marks a Plan-card URL input as manually edited so +// fillPlanURLInputs stops treating it as an auto-fillable default, then +// re-validates. Wired from each of the four fields' oninput instead of +// calling validatePlanURLs() directly. +function onPlanURLFieldEdited(el) { + el.dataset.autofilled = ""; + validatePlanURLs(); } // saveTargetURLAsDefault posts the current plan-target-url value to @@ -2838,8 +2858,12 @@ function defaultServiceURLs(targetUrl, options = {}) { // fillPlanURLInputs writes the four URLs into the Plan card inputs. // force=true overwrites existing values (used by Reset and the -// Soundcork toggle); force=false only fills empties (used on summary -// render so manual edits survive a refresh). +// Soundcork toggle); force=false only fills empties and fields still +// flagged dataset.autofilled=true (used on summary render and on Target +// URL changes, so manual edits survive but a still-default value tracks +// Target URL). Every field this function writes to is (re-)flagged +// autofilled; onPlanURLFieldEdited clears the flag the moment a user +// types into a field directly. function fillPlanURLInputs(urls, {force = false} = {}) { const fields = [ ["plan-marge-url", urls.marge], @@ -2850,7 +2874,10 @@ function fillPlanURLInputs(urls, {force = false} = {}) { for (const [id, value] of fields) { const el = document.getElementById(id); if (!el) continue; - if (force || !el.value) el.value = value; + if (force || !el.value || el.dataset.autofilled === "true") { + el.value = value; + el.dataset.autofilled = "true"; + } } validatePlanURLs(); } diff --git a/pkg/service/setup/enable_ssh.go b/pkg/service/setup/enable_ssh.go index 1c79d6a..ca64c38 100644 --- a/pkg/service/setup/enable_ssh.go +++ b/pkg/service/setup/enable_ssh.go @@ -183,6 +183,47 @@ func (m *Manager) setBoseURLsViaTelnet(deviceIP, marge, swUpdate string) (string return logs.String(), nil } +// setAllBoseURLsViaTelnet writes all four boseurls (bmx, stats, marge, +// swUpdate) to the runtime layer via `sys configuration ...`, then commits +// them with `envswitch boseurls set`, over the port-17000 shell. Unlike +// setBoseURLsViaTelnet (which only issues the envswitch commit, used by the +// #471 SSH-bootstrap/reset flows that need that specific two-argument +// injection), this mirrors telnetURLs.Commands()'s full sequence so the +// envswitch commit captures fresh values for all four fields, not just two. +func (m *Manager) setAllBoseURLsViaTelnet(deviceIP string, urls telnetURLs) (string, error) { + if m.NewTelnet == nil { + return "", errors.New("telnet not configured: Manager.NewTelnet is nil") + } + + var logs strings.Builder + + t := m.NewTelnet(deviceIP) + if err := t.Dial(); err != nil { + return logs.String(), fmt.Errorf("telnet dial %s:17000: %w", deviceIP, err) + } + + defer func() { _ = t.Close() }() + + if banner, _ := t.Probe(); banner != "" { + fmt.Fprintf(&logs, "Telnet banner: %q\n", strings.TrimSpace(banner)) + } + + for _, cmd := range urls.Commands() { + resp, err := t.SendCommand(cmd) + if err != nil { + return logs.String(), fmt.Errorf("telnet command %q failed: %w", cmd, err) + } + + fmt.Fprintf(&logs, "→ %s\n%s\n", cmd, strings.TrimRight(resp, "\r\n")) + + if isCommandNotFound(resp) { + return logs.String(), fmt.Errorf("device rejected %q (firmware does not expose this command)", cmd) + } + } + + return logs.String(), nil +} + // fwScript is the speaker's persistent iptables script; appending here makes a // rule survive reboot (it is re-applied on boot). const fwScript = "/etc/init.d/Firewalls/update_iptables" diff --git a/pkg/service/setup/enable_ssh_test.go b/pkg/service/setup/enable_ssh_test.go index edb921a..cca2303 100644 --- a/pkg/service/setup/enable_ssh_test.go +++ b/pkg/service/setup/enable_ssh_test.go @@ -157,6 +157,59 @@ func TestSetBoseURLs_RejectsDoubleQuote(t *testing.T) { } } +// TestSetAllBoseURLsViaTelnet_WritesAllFourBeforeEnvswitch is the regression +// test for the stale statsServerUrl/bmxRegistryUrl bug reported in #621: the +// XML migration's telnet resync used to commit `envswitch boseurls set` with +// only marge/swUpdate as arguments, silently freezing whatever stats/bmx +// happened to still be in the runtime layer at that moment. This asserts all +// four `sys configuration` writes land before the single `envswitch` commit, +// matching telnetURLs.Commands()'s known-good sequence. +func TestSetAllBoseURLsViaTelnet_WritesAllFourBeforeEnvswitch(t *testing.T) { + const targetURL = "http://localhost:8000" + + urls := telnetURLs{ + Marge: targetURL, + Stats: targetURL, + SwUpdate: targetURL + "/updates/soundtouch", + BmxRegistry: targetURL + "/bmx/registry/v1/services", + } + + want := urls.Commands() + + resp := make(map[string]string, len(want)) + for _, c := range want { + resp[c] = "OK\n" + } + + f := &fakeTelnet{responses: resp} + m := newFakeTelnetManager(f) + + if _, err := m.setAllBoseURLsViaTelnet("192.0.2.10", urls); err != nil { + t.Fatalf("setAllBoseURLsViaTelnet: %v", err) + } + + if len(f.commands) != len(want) { + t.Fatalf("sent %d commands %q\n want %d %q", len(f.commands), f.commands, len(want), want) + } + + for i, c := range want { + if f.commands[i] != c { + t.Errorf("command %d = %q\n want %q", i, f.commands[i], c) + } + } + + envswitchIdx := len(want) - 1 + for i, c := range f.commands[:envswitchIdx] { + if !strings.HasPrefix(c, "sys configuration ") { + t.Errorf("command %d = %q, want a `sys configuration ...` runtime write before the envswitch commit", i, c) + } + } + + if !strings.HasPrefix(f.commands[envswitchIdx], "envswitch boseurls set ") { + t.Errorf("last command = %q, want the envswitch commit last", f.commands[envswitchIdx]) + } +} + func TestClose17000_RunsFirewallSteps(t *testing.T) { var ran []string diff --git a/pkg/service/setup/setup.go b/pkg/service/setup/setup.go index 9db109d..3c30f88 100644 --- a/pkg/service/setup/setup.go +++ b/pkg/service/setup/setup.go @@ -1089,32 +1089,46 @@ func (m *Manager) migrateViaXML(deviceIP, targetURL, proxyURL string, options ma } } - logs += m.resyncBoseURLsAfterXML(deviceIP, cfg.MargeServerUrl, cfg.SwUpdateUrl) + logs += m.resyncBoseURLsAfterXML(deviceIP, telnetURLs{ + Marge: cfg.MargeServerUrl, + Stats: cfg.StatsServerUrl, + SwUpdate: cfg.SwUpdateUrl, + BmxRegistry: cfg.BmxRegistryUrl, + }) return logs, nil } -// resyncBoseURLsAfterXML re-applies the boseurls over telnet so the runtime -// URL layer matches the XML just written by migrateViaXML. +// resyncBoseURLsAfterXML re-applies all four boseurls over telnet so the +// runtime URL layer matches the XML just written by migrateViaXML. // // The XML migration only updates the persisted SoundTouchSdkPrivateCfg.xml; it // does not touch the runtime/persistence layer that `getpdo // CurrentSystemConfiguration` reports. When SSH was bootstrapped via #471 // (`enable-ssh`), that layer still points at the placeholder boseurls // (https://aftertouch.invalid), so the preflight cross-check keeps warning that -// margeServerUrl/swUpdateUrl differ between transports until a reboot. -// Re-applying the real boseurls over telnet :17000 reconciles it immediately. +// the URLs differ between transports until a reboot. +// +// All four fields are re-applied, not just marge/swUpdate: the closing +// `envswitch boseurls set` commit persists whatever is currently in the +// runtime layer at the moment it runs, not only its own two arguments (see +// docs/content/docs/analysis/TELNET-COMMAND-REFERENCE.md). Committing while +// stats/bmx are still stale in the runtime layer freezes those stale values +// into the persistence layer permanently — a later reboot loads that frozen +// persistence layer, not the XML file, so nothing short of a factory reset +// clears it again. Re-applying the real boseurls over telnet :17000 +// reconciles all four immediately. // // Best-effort: telnet may be unavailable (no port 17000, or it was closed via // --close-17000), in which case a reboot still reconciles the layers, so this // only returns a note and never fails the migration. Returns the log lines to // append. -func (m *Manager) resyncBoseURLsAfterXML(deviceIP, marge, swUpdate string) string { +func (m *Manager) resyncBoseURLsAfterXML(deviceIP string, urls telnetURLs) string { if m.NewTelnet == nil { return "" } - rlogs, rerr := m.setBoseURLsViaTelnet(deviceIP, marge, swUpdate) + rlogs, rerr := m.setAllBoseURLsViaTelnet(deviceIP, urls) if rerr != nil { return fmt.Sprintf("Note: could not re-sync boseurls over telnet (%v); a device reboot will reconcile the runtime layer.\n", rerr) } diff --git a/pkg/service/setup/setup_test.go b/pkg/service/setup/setup_test.go index 68ff496..c3e852e 100644 --- a/pkg/service/setup/setup_test.go +++ b/pkg/service/setup/setup_test.go @@ -2102,25 +2102,41 @@ func TestMigrateViaXML_ReappliesBoseURLsOverTelnet(t *testing.T) { return &mockSSH{runFunc: func(string) (string, error) { return "", nil }} } - ft := &fakeTelnet{banner: "->", responses: map[string]string{}} + wantCmds := telnetURLs{ + Marge: target, + Stats: target, + SwUpdate: target + "/updates/soundtouch", + BmxRegistry: target + "/bmx/registry/v1/services", + }.Commands() + + resp := make(map[string]string, len(wantCmds)) + for _, c := range wantCmds { + resp[c] = "OK\n" + } + + ft := &fakeTelnet{banner: "->", responses: resp} m.NewTelnet = func(string) TelnetClient { return ft } if _, err := m.MigrateSpeaker("192.0.2.10", target, "", nil, MigrationMethodXML); err != nil { t.Fatalf("MigrateSpeaker: %v", err) } - want := `envswitch boseurls set "` + target + `" "` + target + `/updates/soundtouch"` - - var found bool - for _, c := range ft.commands { - if c == want { - found = true - break + // All four `sys configuration` writes must land before the envswitch + // commit — see enable_ssh.go's setAllBoseURLsViaTelnet — otherwise the + // commit freezes whatever stale value was still in the runtime layer for + // any field not passed to it (the #621 statsServerUrl/bmxRegistryUrl bug). + for _, want := range wantCmds { + var found bool + for _, c := range ft.commands { + if c == want { + found = true + break + } } - } - if !found { - t.Errorf("expected boseurls re-apply %q after XML migration; sent: %v", want, ft.commands) + if !found { + t.Errorf("expected boseurls re-apply command %q after XML migration; sent: %v", want, ft.commands) + } } } diff --git a/scripts/on-device-install/install.sh b/scripts/on-device-install/install.sh index cba116c..0146181 100644 --- a/scripts/on-device-install/install.sh +++ b/scripts/on-device-install/install.sh @@ -87,6 +87,24 @@ if [ "$INSTALL_DIR" != "/opt/aftertouch" ]; then ln -sf "$INSTALL_DIR" /opt/aftertouch fi +# Prune any *.backup/*.old/*.new artefacts left behind by an earlier install +# attempt, before doing anything else that needs disk space. /mnt/nv is small +# (tens of MB), and if a previous run died between creating its backup and +# reaching the GC step below (e.g. "no space left on device" during the +# download that follows), that backup would otherwise never get cleaned up -- +# and low free space is exactly what makes the next attempt likely to die the +# same way. Pruning up front makes cleanup idempotent regardless of where a +# prior run was interrupted. +echo "Disk usage before pre-install GC:"; df -h "$INSTALL_DIR" +for f in "$INSTALL_DIR/aftertouch-service".*.backup \ + "$INSTALL_DIR/aftertouch-service".*.old \ + "$INSTALL_DIR/aftertouch-service.new"; do + [ -f "$f" ] || continue + rm -f "$f" + echo "Removed stale artefact: $f" +done +echo "Disk usage after pre-install GC:"; df -h "$INSTALL_DIR" + curl \ -sSL \ -o "$UPDATE_TMP_DIR/binary" \ @@ -112,10 +130,11 @@ mv "$UPDATE_TMP_DIR/binary" "$INSTALL_DIR/aftertouch-service" chmod +x "$INSTALL_DIR/aftertouch-service" # Keep only the backup we just created; prune all older *.backup, *.old, and -# *.new artefacts left by earlier installs. /mnt/nv is small (tens of MB), -# so accumulation quickly causes "no space left on device" during downloads. +# *.new artefacts left by earlier installs. This is a second, defensive pass: +# it only matters if something wrote a stray artefact between the pre-install +# GC above and here (e.g. a concurrent install run). if [ -n "$BACKUP_FILE" ]; then - echo "Disk usage before GC:"; df -h "$INSTALL_DIR" + echo "Disk usage before post-install GC:"; df -h "$INSTALL_DIR" for f in "$INSTALL_DIR/aftertouch-service".*.backup \ "$INSTALL_DIR/aftertouch-service".*.old \ "$INSTALL_DIR/aftertouch-service.new"; do @@ -124,7 +143,7 @@ if [ -n "$BACKUP_FILE" ]; then rm -f "$f" echo "Removed stale artefact: $f" done - echo "Disk usage after GC:"; df -h "$INSTALL_DIR" + echo "Disk usage after post-install GC:"; df -h "$INSTALL_DIR" fi # Settings file sourced by the init script. Written before the service is