test(player): adopt httptest.NewTestServer (Go 1.27) in the new discovery test

NewTestServer registers its own t.Cleanup(Close) instead of needing a manual defer, and fails the test on a handler panic instead of just logging it. It defaults to an in-memory transport reachable only via Server.Client(), which wouldn't work here since our production client.NewClient dials a real address rather than using that client -- calling Start() instead of Client() opts back into a real loopback listener, identical to the old NewServer, confirmed by reading the actual go1.27.0 source (server.go's Start implementation). This is a proactive adoption of a new stdlib idiom, not one of the review findings from the previous commit; it doesn't change the goroutine-drain fix from that commit, which is a separate concern Close()'s "wait for outstanding requests" guarantee doesn't fully cover (a goroutine that hasn't started its request yet at Close() time isn't "outstanding"). Verified: 10x -count re-run under -race, full suite + lint clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix(player): address code review findings on #644
2026-08-24 14:47:23 +00:00 · 2026-08-23 22:00:11 +02:00 · 2026-08-23 22:00:11 +02:00 · 2026-08-23 22:00:11 +02:00 · 2026-08-23 20:50:27 +02:00 · 2026-08-23 20:50:27 +02:00
44 changed files with 1826 additions and 446 deletions
@@ -308,7 +308,7 @@ jobs:
        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0
+        uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0

      - name: Set build date
        id: build_date
@@ -514,7 +514,7 @@ jobs:
          echo "commit=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT

      - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0
+        uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0

      - name: Log in to GitHub Container Registry
        uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
@@ -1,5 +1,5 @@
 # golangci-lint configuration for Bose SoundTouch Go Library
-# Compatible with golangci-lint v2.8.0
+# Compatible with golangci-lint v2.13.1
 # See: https://golangci-lint.run/usage/configuration/

 version: "2"
@@ -1,5 +1,5 @@
 # Build stage
-FROM --platform=$BUILDPLATFORM golang:1.26.6-alpine AS builder
+FROM --platform=$BUILDPLATFORM golang:1.27.0-alpine AS builder

 # Declare automatic platform ARGs to make them available in build stage
 # See https://docs.docker.com/reference/dockerfile#automatic-platform-args-in-the-global-scope
@@ -243,7 +243,7 @@ vet:

 lint:
 	@echo "Running golangci-lint..."
-	@which golangci-lint > /dev/null || (echo "golangci-lint not found. Install with: go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest" && exit 1)
+	@which golangci-lint > /dev/null || (echo "golangci-lint not found. Install with: go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest" && exit 1)
 	golangci-lint run

 tidy:
@@ -15,6 +15,7 @@ import (

 	"github.com/gesellix/bose-soundtouch/pkg/models"
 	"github.com/gesellix/bose-soundtouch/pkg/service/constants"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 	"github.com/gesellix/bose-soundtouch/pkg/service/setup"
 	"github.com/urfave/cli/v2"
 	"golang.org/x/term"
@@ -674,9 +675,9 @@ func setupEnableSSHCmd() *cli.Command {
 			},
 			&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",
+				Usage: "Only used when the device is unpaired and --no-auto-pair is not set: account ID to pair with " +
+					"(empty = generate a fresh 7-digit 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",
@@ -1974,7 +1975,7 @@ func setupPairCmd() *cli.Command {
 		Usage:  "Pair the speaker with an account via WebSocket SETUP state machine",
 		Before: RequireHost,
 		Flags: []cli.Flag{
-			&cli.StringFlag{Name: "account", Usage: "7-digit account ID (empty = generate)"},
+			&cli.StringFlag{Name: "account", Usage: "Account ID to pair with (empty = generate a fresh 7-digit one)"},
 			&cli.StringFlag{Name: "mode", Value: "full", Usage: "full (state machine) or bare (setMargeAccount only — experimental)"},
 			&cli.StringFlag{Name: "service-url", Value: "http://aftertouch.local:8000", Usage: "AfterTouch base URL (also populates <boseServer>/<updateServer> in setMargeAccount)"},
 			&cli.StringFlag{Name: "name", Usage: "Speaker name to set during pairing (empty = keep current)"},
@@ -1998,8 +1999,8 @@ func setupPairCmd() *cli.Command {
 				fmt.Printf("Generated account id: %s\n", accountID)
 			}

-			if !setup.IsValidAccountID(accountID) {
-				return fmt.Errorf("invalid account id %q: must be 7 digits", accountID)
+			if !datastore.IsSafeIdentifier(accountID) {
+				return fmt.Errorf("invalid account id %q: must be a non-empty, path-safe identifier", accountID)
 			}

 			switch mode {
@@ -88,7 +88,7 @@ go build -o soundtouch-player
 ./soundtouch-player -port 8888

 # Connect to specific device
-./soundtouch-player -host 192.0.2.100
+./soundtouch-player --devices 192.0.2.100
 ```

 ### Command Line Options
@@ -137,7 +137,7 @@ device datastore).
 The application automatically discovers SoundTouch devices using:
 - **mDNS discovery** for local network devices
 - **UPnP/SSDP discovery** as fallback
- **Manual device addition** via IP address
+- **Configured devices** via `--devices`, retried whenever discovery runs

 ### Real-time Updates
 The interface maintains WebSocket connections to each device for instant updates of:
@@ -161,7 +161,7 @@ func main() {
 				log.Printf("Trusting AfterTouch service CA from %s", sanitizeLog(caPath))
 			}

-			discoveryService := soundtouchweb.NewDiscoveryService(ifaceName)
+			discoveryService := soundtouchweb.NewDiscoveryService(ifaceName, manualHosts...)

 			// Discover devices on startup
 			go func() {
@@ -170,6 +170,11 @@ func main() {

 				webApp.BroadcastDiscoveryStatus("starting", webApp.DeviceCount())

+				// Register configured devices immediately rather than waiting for
+				// the full mDNS/UPnP sweep below (bounded by cfg.DiscoveryTimeout,
+				// currently 10s) to complete. manualHosts are also folded into
+				// discoveryService's PreferredDevices so a host that's offline
+				// right now still gets retried on every subsequent discovery pass.
 				for _, host := range manualHosts {
 					webApp.AddDeviceByHost(host, 8090, "manual")
 				}
@@ -39,11 +39,8 @@ func TestPrintRoutes(t *testing.T) {
 		// Now we might have "soundtouch-service.setupRouter.func1"
 		// or "command-line-arguments.setupRouter.func1"
 		// or "main.setupRouter.func1"
-		// Let's remove the first part if it's a known varying package name
-		if idx := strings.Index(handlerName, "setupRouter"); idx != -1 {
-			handlerName = handlerName[idx:]
-		}
-		// In case it's not setupRouter but still has a package prefix
+		// Remove the leading package/binary-name segment(s), whatever form
+		// they take.
 		for {
 			dotIdx := strings.Index(handlerName, ".")
 			if dotIdx == -1 {
@@ -169,7 +169,7 @@ GET      /streaming/sourceproviders                                   handlers.(
 GET      /updates/soundtouch                                          handlers.(*Server).HandleMargeSoftwareUpdate-fm
 GET      /v1/auth                                                     handlers.(*Server).HandleSpeakerAuth-fm
 GET      /v1/blacklist/{deviceId}                                     setupRouter
-GET      /web/*                                                       setupRouter.(*Server).HandleWeb
+GET      /web/*                                                       handlers.(*Server).HandleWeb
 HEAD     /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm
 HEAD     /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter/* handlers.(*Server).HandleSiriusXMLiveAdapterSubpath-fm
 OPTIONS  /core02/svc-bmx-adapter-siriusxm-everest-eco1/prod/live-adapter handlers.(*Server).HandleSiriusXMLiveAdapter-fm
@@ -35,7 +35,7 @@ services:
      start_period: 3s

  spotify-mock:
-    image: golang:1.26.6-alpine
+    image: golang:1.27.0-alpine
    container_name: spotify-mock
    working_dir: /app
    volumes:
@@ -53,7 +53,7 @@ services:
      start_period: 3s

  amazon-mock:
-    image: golang:1.26.6-alpine
+    image: golang:1.27.0-alpine
    container_name: amazon-mock
    working_dir: /app
    volumes:
@@ -71,7 +71,7 @@ services:
      start_period: 3s

  tunein-mock:
-    image: golang:1.26.6-alpine
+    image: golang:1.27.0-alpine
    container_name: tunein-mock
    working_dir: /app
    volumes:
@@ -566,7 +566,7 @@ func (m *MockClient) GetNowPlaying() (*models.NowPlaying, error) {

 ```dockerfile
 # test/docker/Dockerfile
-FROM golang:1.25-alpine
+FROM golang:1.27.0-alpine

 WORKDIR /app
 COPY . .
@@ -115,25 +115,25 @@ type ProductionSoundTouchService struct {
 type Config struct {
    // Server settings
    ListenAddr      string        `env:"LISTEN_ADDR" default:":8080"`
-    
+
    // SoundTouch settings
    DeviceHosts     []string      `env:"DEVICE_HOSTS" separator:","`
    DiscoveryTimeout time.Duration `env:"DISCOVERY_TIMEOUT" default:"30s"`
    RequestTimeout   time.Duration `env:"REQUEST_TIMEOUT" default:"15s"`
    MaxRetries       int           `env:"MAX_RETRIES" default:"3"`
-    
+
    // Connection pool
    MaxConnections   int           `env:"MAX_CONNECTIONS" default:"10"`
    IdleTimeout      time.Duration `env:"IDLE_TIMEOUT" default:"5m"`
-    
+
    // Monitoring
    MetricsEnabled   bool          `env:"METRICS_ENABLED" default:"true"`
    HealthCheckInterval time.Duration `env:"HEALTH_CHECK_INTERVAL" default:"30s"`
-    
+
    // Logging
    LogLevel        string         `env:"LOG_LEVEL" default:"info"`
    LogFormat       string         `env:"LOG_FORMAT" default:"json"`
-    
+
    // Security
    EnableTLS       bool          `env:"ENABLE_TLS" default:"false"`
    TLSCertFile     string        `env:"TLS_CERT_FILE"`
@@ -145,7 +145,7 @@ func LoadConfig() (*Config, error) {
    if err := env.Parse(cfg); err != nil {
        return nil, fmt.Errorf("failed to parse config: %w", err)
    }
-    
+
    return cfg, cfg.Validate()
 }

@@ -153,15 +153,15 @@ func (c *Config) Validate() error {
    if len(c.DeviceHosts) == 0 {
        return fmt.Errorf("at least one device host must be specified")
    }
-    
+
    if c.RequestTimeout < time.Second {
        return fmt.Errorf("request timeout must be at least 1 second")
    }
-    
+
    if c.EnableTLS && (c.TLSCertFile == "" || c.TLSKeyFile == "") {
        return fmt.Errorf("TLS cert and key files required when TLS is enabled")
    }
-    
+
    return nil
 }
 ```
@@ -191,7 +191,7 @@ pool:
 monitoring:
  metrics_enabled: true
  health_check_interval: "30s"
-  
+
 logging:
  level: "info"
  format: "json"
@@ -203,12 +203,12 @@ func LoadConfigFromFile(path string) (*Config, error) {
    if err != nil {
        return nil, err
    }
-    
+
    var cfg Config
    if err := yaml.Unmarshal(data, &cfg); err != nil {
        return nil, err
    }
-    
+
    return &cfg, cfg.Validate()
 }
 ```
@@ -224,14 +224,14 @@ func LoadConfigFromFile(path string) (*Config, error) {
 type SecureNetworkConfig struct {
    // Allowed source IP ranges
    AllowedCIDRs    []string
-    
+
    // Rate limiting
    RateLimit       int
    RateLimitWindow time.Duration
-    
+
    // TLS configuration
    TLSConfig       *tls.Config
-    
+
    // Timeouts for security
    ReadTimeout     time.Duration
    WriteTimeout    time.Duration
@@ -240,7 +240,7 @@ type SecureNetworkConfig struct {

 func NewSecureServer(config SecureNetworkConfig) *http.Server {
    mux := http.NewServeMux()
-    
+
    // Add middleware
    handler := applyMiddleware(mux,
        corsMiddleware(),
@@ -249,7 +249,7 @@ func NewSecureServer(config SecureNetworkConfig) *http.Server {
        loggingMiddleware(),
        metricsMiddleware(),
    )
-    
+
    return &http.Server{
        Handler:      handler,
        TLSConfig:    config.TLSConfig,
@@ -275,12 +275,12 @@ func (r *DeviceControlRequest) Validate() error {
    if err := validate.Struct(r); err != nil {
        return fmt.Errorf("validation failed: %w", err)
    }
-    
+
    // Additional business logic validation
    if r.Action == "volume" && r.Volume == nil {
        return fmt.Errorf("volume value required for volume action")
    }
-    
+
    return nil
 }
 ```
@@ -302,12 +302,12 @@ func loadSecretsFromK8s() (*SecretsConfig, error) {
    if err != nil {
        return nil, err
    }
-    
+
    tlsKey, err := os.ReadFile("/etc/secrets/tls.key")
    if err != nil {
        return nil, err
    }
-    
+
    return &SecretsConfig{
        TLSCert: string(tlsCert),
        TLSKey:  string(tlsKey),
@@ -335,21 +335,21 @@ type Logger struct {

 func NewLogger(level, format, component string) (*Logger, error) {
    logger := logrus.New()
-    
+
    // Set level
    logLevel, err := logrus.ParseLevel(level)
    if err != nil {
        return nil, err
    }
    logger.SetLevel(logLevel)
-    
+
    // Set format
    if format == "json" {
        logger.SetFormatter(&logrus.JSONFormatter{
            TimestampFormat: time.RFC3339,
        })
    }
-    
+
    return &Logger{
        Logger:    logger,
        component: component,
@@ -376,15 +376,15 @@ type Metrics struct {
    RequestsTotal     prometheus.CounterVec
    RequestDuration   prometheus.HistogramVec
    RequestsInFlight  prometheus.GaugeVec
-    
+
    // Device metrics
    DevicesConnected  prometheus.Gauge
    DeviceHealth      prometheus.GaugeVec
    WebSocketConnections prometheus.Gauge
-    
+
    // Error metrics
    ErrorsTotal       prometheus.CounterVec
-    
+
    // Business metrics
    VolumeChanges     prometheus.CounterVec
    SourceChanges     prometheus.CounterVec
@@ -400,7 +400,7 @@ func NewMetrics() *Metrics {
            },
            []string{"method", "endpoint", "status"},
        ),
-        
+
        RequestDuration: *prometheus.NewHistogramVec(
            prometheus.HistogramOpts{
                Name: "soundtouch_request_duration_seconds",
@@ -409,14 +409,14 @@ func NewMetrics() *Metrics {
            },
            []string{"method", "endpoint"},
        ),
-        
+
        DevicesConnected: prometheus.NewGauge(
            prometheus.GaugeOpts{
                Name: "soundtouch_devices_connected",
                Help: "Number of connected devices",
            },
        ),
-        
+
        DeviceHealth: *prometheus.NewGaugeVec(
            prometheus.GaugeOpts{
                Name: "soundtouch_device_health",
@@ -425,7 +425,7 @@ func NewMetrics() *Metrics {
            []string{"device_id", "device_name"},
        ),
    }
-    
+
    // Register metrics
    prometheus.MustRegister(
        m.RequestsTotal,
@@ -433,7 +433,7 @@ func NewMetrics() *Metrics {
        m.DevicesConnected,
        m.DeviceHealth,
    )
-    
+
    return m
 }

@@ -457,7 +457,7 @@ type HealthChecker struct {
 func (hc *HealthChecker) Start(ctx context.Context) {
    ticker := time.NewTicker(hc.interval)
    defer ticker.Stop()
-    
+
    for {
        select {
        case <-ctx.Done():
@@ -470,7 +470,7 @@ func (hc *HealthChecker) Start(ctx context.Context) {

 func (hc *HealthChecker) checkAllDevices() {
    var wg sync.WaitGroup
-    
+
    for deviceID, device := range hc.manager.devices {
        wg.Add(1)
        go func(id string, dev *DeviceInfo) {
@@ -478,18 +478,18 @@ func (hc *HealthChecker) checkAllDevices() {
            hc.checkDevice(id, dev)
        }(deviceID, device)
    }
-    
+
    wg.Wait()
 }

 func (hc *HealthChecker) checkDevice(deviceID string, device *DeviceInfo) {
    ctx, cancel := context.WithTimeout(context.Background(), hc.timeout)
    defer cancel()
-    
+
    start := time.Now()
    err := device.Client.Ping()
    duration := time.Since(start)
-    
+
    if err != nil {
        device.Status = DeviceStatusUnhealthy
        hc.metrics.DeviceHealth.WithLabelValues(deviceID, device.Name).Set(0)
@@ -507,14 +507,14 @@ func (hc *HealthChecker) HealthHandler() http.HandlerFunc {
    return func(w http.ResponseWriter, r *http.Request) {
        healthy := 0
        total := 0
-        
+
        for _, device := range hc.manager.devices {
            total++
            if device.Status == DeviceStatusHealthy {
                healthy++
            }
        }
-        
+
        status := map[string]interface{}{
            "status": "ok",
            "devices": map[string]interface{}{
@@ -524,14 +524,14 @@ func (hc *HealthChecker) HealthHandler() http.HandlerFunc {
            },
            "timestamp": time.Now().UTC(),
        }
-        
+
        w.Header().Set("Content-Type", "application/json")
-        
+
        if healthy < total {
            w.WriteHeader(http.StatusServiceUnavailable)
            status["status"] = "degraded"
        }
-        
+
        json.NewEncoder(w).Encode(status)
    }
 }
@@ -560,16 +560,16 @@ func NewConnectionPool(maxIdle, maxActive int, idleTimeout time.Duration) *Conne
        maxActive:   maxActive,
        idleTimeout: idleTimeout,
    }
-    
+
    // Start cleanup goroutine
    go cp.cleanup()
-    
+
    return cp
 }

 func (cp *ConnectionPool) Get(host string, port int) (*client.Client, error) {
    key := fmt.Sprintf("%s:%d", host, port)
-    
+
    // Check if connection exists and is valid
    if val, ok := cp.clients.Load(key); ok {
        conn := val.(*pooledConnection)
@@ -580,35 +580,35 @@ func (cp *ConnectionPool) Get(host string, port int) (*client.Client, error) {
        // Connection expired, remove it
        cp.clients.Delete(key)
    }
-    
+
    // Check active connection limit
    if atomic.LoadInt64(&cp.activeCount) >= int64(cp.maxActive) {
        return nil, fmt.Errorf("connection pool exhausted")
    }
-    
+
    // Create new connection
    config := client.ClientConfig{
        Host:    host,
        Port:    port,
        Timeout: 15 * time.Second,
    }
-    
+
    newClient := client.NewClient(config)
-    
+
    // Test connection
    if err := newClient.Ping(); err != nil {
        return nil, fmt.Errorf("failed to connect to %s:%d: %w", host, port, err)
    }
-    
+
    conn := &pooledConnection{
        client:   newClient,
        lastUsed: time.Now(),
        created:  time.Now(),
    }
-    
+
    cp.clients.Store(key, conn)
    atomic.AddInt64(&cp.activeCount, 1)
-    
+
    return newClient, nil
 }

@@ -621,7 +621,7 @@ type pooledConnection struct {
 func (cp *ConnectionPool) cleanup() {
    ticker := time.NewTicker(cp.idleTimeout / 2)
    defer ticker.Stop()
-    
+
    for range ticker.C {
        now := time.Now()
        cp.clients.Range(func(key, val interface{}) bool {
@@ -649,10 +649,10 @@ func NewCacheManager() *CacheManager {
    return &CacheManager{
        // Device info rarely changes, cache for 1 hour
        deviceInfoCache: cache.New(1*time.Hour, 2*time.Hour),
-        
+
        // Capabilities never change, cache for 24 hours
        capabilitiesCache: cache.New(24*time.Hour, 48*time.Hour),
-        
+
        // Volume changes frequently, cache for 5 seconds
        volumeCache: cache.New(5*time.Second, 10*time.Second),
    }
@@ -662,12 +662,12 @@ func (cm *CacheManager) GetDeviceInfo(deviceID string, fetcher func() (*models.D
    if cached, found := cm.deviceInfoCache.Get(deviceID); found {
        return cached.(*models.DeviceInfo), nil
    }
-    
+
    info, err := fetcher()
    if err != nil {
        return nil, err
    }
-    
+
    cm.deviceInfoCache.Set(deviceID, info, cache.DefaultExpiration)
    return info, nil
 }
@@ -702,7 +702,7 @@ func NewResilientSoundTouchService(client *client.Client) *ResilientSoundTouchSe
            log.Printf("Circuit breaker '%s' changed from '%s' to '%s'", name, from, to)
        },
    }
-    
+
    return &ResilientSoundTouchService{
        client: client,
        cb:     gobreaker.NewCircuitBreaker(settings),
@@ -713,12 +713,12 @@ func (r *ResilientSoundTouchService) SetVolume(deviceID string, volume int) erro
    result, err := r.cb.Execute(func() (interface{}, error) {
        return nil, r.client.SetVolume(volume)
    })
-    
+
    if err != nil {
        r.metrics.ErrorsTotal.WithLabelValues("circuit_breaker", "volume").Inc()
        return err
    }
-    
+
    return result.(error)
 }
 ```
@@ -730,16 +730,16 @@ func (app *Application) Run(ctx context.Context) error {
    // Setup signal handling
    sigChan := make(chan os.Signal, 1)
    signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM)
-    
+
    // Start services
    g, ctx := errgroup.WithContext(ctx)
-    
+
    // HTTP server
    server := &http.Server{
        Addr:    app.config.ListenAddr,
        Handler: app.handler,
    }
-    
+
    g.Go(func() error {
        app.logger.Info("Starting HTTP server", "addr", app.config.ListenAddr)
        if err := server.ListenAndServe(); err != http.ErrServerClosed {
@@ -747,38 +747,38 @@ func (app *Application) Run(ctx context.Context) error {
        }
        return nil
    })
-    
+
    // Health checker
    g.Go(func() error {
        return app.healthChecker.Start(ctx)
    })
-    
+
    // WebSocket manager
    g.Go(func() error {
        return app.wsManager.Start(ctx)
    })
-    
+
    // Wait for shutdown signal
    go func() {
        <-sigChan
        app.logger.Info("Shutdown signal received")
-        
+
        // Graceful shutdown with timeout
        shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
        defer cancel()
-        
+
        // Shutdown HTTP server
        if err := server.Shutdown(shutdownCtx); err != nil {
            app.logger.Error("HTTP server shutdown error", "error", err)
        }
-        
+
        // Close WebSocket connections
        app.wsManager.Shutdown(shutdownCtx)
-        
+
        // Close connection pool
        app.connectionPool.Close()
    }()
-    
+
    return g.Wait()
 }
 ```
@@ -791,7 +791,7 @@ func (app *Application) Run(ctx context.Context) error {

 ```dockerfile
 # Dockerfile
-FROM golang:1.25-alpine AS builder
+FROM golang:1.27.0-alpine AS builder

 WORKDIR /app
 COPY go.mod go.sum ./
@@ -830,7 +830,7 @@ services:
    networks:
      - soundtouch-net
    restart: unless-stopped
-    
+
  prometheus:
    image: prom/prometheus:latest
    ports:
@@ -839,7 +839,7 @@ services:
      - ./prometheus.yml:/etc/prometheus/prometheus.yml
    networks:
      - soundtouch-net
-      
+
  grafana:
    image: grafana/grafana:latest
    ports:
@@ -1011,7 +1011,7 @@ groups:
    annotations:
      summary: "SoundTouch device {{ $labels.device_name }} is unhealthy"
      description: "Device {{ $labels.device_id }} has been unhealthy for more than 2 minutes"
-      
+
  - alert: HighErrorRate
    expr: rate(soundtouch_errors_total[5m]) > 0.1
    for: 5m
@@ -1020,7 +1020,7 @@ groups:
    annotations:
      summary: "High error rate detected"
      description: "Error rate is {{ $value }} errors/second over the last 5 minutes"
-      
+
  - alert: ServiceDown
    expr: up{job="soundtouch"} == 0
    for: 1m
@@ -1040,33 +1040,33 @@ func (m *Manager) BackupConfigurations() error {
        Timestamp: time.Now(),
        Devices:   make(map[string]DeviceConfig),
    }
-    
+
    for deviceID, device := range m.devices {
        config := DeviceConfig{}
-        
+
        // Backup presets
        if presets, err := device.Client.GetPresets(); err == nil {
            config.Presets = presets
        }
-        
+
        // Backup settings
        if volume, err := device.Client.GetVolume(); err == nil {
            config.Volume = volume.TargetVolume
        }
-        
+
        if bass, err := device.Client.GetBass(); err == nil {
            config.Bass = bass.TargetBass
        }
-        
+
        backup.Devices[deviceID] = config
    }
-    
+
    // Save to file
    data, err := json.MarshalIndent(backup, "", "  ")
    if err != nil {
        return err
    }
-    
+
    filename := fmt.Sprintf("backup_%s.json", time.Now().Format("2006-01-02_15-04-05"))
    return os.WriteFile(filepath.Join(m.config.BackupDir, filename), data, 0644)
 }
@@ -1083,7 +1083,7 @@ func init() {
            runtime.GOMAXPROCS(int(limit))
        }
    }
-    
+
    // Set GC target percentage
    if os.Getenv("GOGC") == "" {
        debug.SetGCPerc
@@ -1,8 +1,8 @@
 module navigation-station-demo

-go 1.26.6
+go 1.27.0

-require github.com/gesellix/bose-soundtouch v0.123.0
+require github.com/gesellix/bose-soundtouch v0.128.0

 require github.com/gorilla/websocket v1.5.3 // indirect

@@ -1,8 +1,8 @@
 module preset-management-example

-go 1.26.6
+go 1.27.0

-require github.com/gesellix/bose-soundtouch v0.123.0
+require github.com/gesellix/bose-soundtouch v0.128.0

 require github.com/gorilla/websocket v1.5.3 // indirect

@@ -1,15 +1,15 @@
 module github.com/gesellix/bose-soundtouch

-go 1.26.6
+go 1.27.0

 require (
 	filippo.io/age v1.3.1
 	github.com/chromedp/chromedp v0.16.0
-	github.com/go-chi/chi/v5 v5.3.1
+	github.com/go-chi/chi/v5 v5.3.2
 	github.com/google/gopacket v1.1.19
 	github.com/gorilla/websocket v1.5.3
 	github.com/hashicorp/mdns v1.0.7
-	github.com/miekg/dns v1.1.72
+	github.com/miekg/dns v1.1.73
 	github.com/russross/blackfriday/v2 v2.1.0
 	github.com/sergi/go-diff v1.4.0
 	github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c
@@ -33,8 +33,6 @@ require (
 	github.com/gobwas/ws v1.4.0 // indirect
 	github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342 // indirect
 	golang.org/x/image v0.45.0 // indirect
-	golang.org/x/sync v0.22.0 // indirect
 	golang.org/x/sys v0.47.0 // indirect
 	golang.org/x/text v0.41.0 // indirect
-	golang.org/x/tools v0.49.0 // indirect
 )
@@ -17,8 +17,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6N
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/go-chi/chi/v5 v5.3.1 h1:3j4HZLGZQ3JpMCrPJF/Jl3mYJfWLKBfNJ6quurUGCf8=
-github.com/go-chi/chi/v5 v5.3.1/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto=
+github.com/go-chi/chi/v5 v5.3.2 h1:5YQkICvTCSZ25hoRsyJazN0scjzKGiu4VAUc7H1o1nY=
+github.com/go-chi/chi/v5 v5.3.2/go.mod h1:R+tYY2hNuVUUjxoPtqUdgBqevM9s9njzkTLutVsOCto=
 github.com/go-json-experiment/json v0.0.0-20260623181947-01eb4420fa68 h1:KZaTBSyshWX3MP5jukJcNSuXDQTO+rNpt0J564dX/eg=
 github.com/go-json-experiment/json v0.0.0-20260623181947-01eb4420fa68/go.mod h1:tphK2c80bpPhMOI4v6bIc2xWywPfbqi1Z06+RcrMkDg=
 github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU=
@@ -27,8 +27,6 @@ github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
 github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
 github.com/gobwas/ws v1.4.0 h1:CTaoG1tojrh4ucGPcoJFiAQUAsEWekEWvLy7GsVNqGs=
 github.com/gobwas/ws v1.4.0/go.mod h1:G3gNqMNtPppf5XUz7O4shetPpcZ1VJ7zt18dlUeakrc=
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8=
 github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo=
 github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
@@ -40,8 +38,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80 h1:6Yzfa6GP0rIo/kULo2bwGEkFvCePZ3qHDDTC3/J9Swo=
 github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80/go.mod h1:imJHygn/1yfhB7XSJJKlFZKl/J+dCPAknuiaGOshXAs=
-github.com/miekg/dns v1.1.72 h1:vhmr+TF2A3tuoGNkLDFK9zi36F2LS+hKTRW0Uf8kbzI=
-github.com/miekg/dns v1.1.72/go.mod h1:+EuEPhdHOsfk6Wk5TT2CzssZdqkmFhf8r+aVyDEToIs=
+github.com/miekg/dns v1.1.73 h1:uhT8nJxmTrPJYClxVxTCX+CVn6qnzSiybRk72Z6DgrE=
+github.com/miekg/dns v1.1.73/go.mod h1:RW2Obtfd5NZHvOFe3zYG0W8koWOQtAzyHaLo8vASBuQ=
 github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde h1:x0TT0RDC7UhAVbbWWBzr41ElhJx5tXPWkIHA2HWPRuw=
 github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
@@ -89,8 +87,6 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8=
 golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M=
 golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
-golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI=
-golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
@@ -33,11 +33,30 @@ func exists(path string) bool {
 	return err == nil
 }

-// isSafeIdentifier returns true if the given identifier is safe to use
-// as a single path component (for account IDs, device IDs, etc.).
-// It rejects empty strings, path separators, and parent directory references.
-func isSafeIdentifier(id string) bool {
-	if id == "" {
+// maxSafeIdentifierLength bounds account/device IDs accepted from a
+// speaker or third-party pairing tool. Well under typical filesystem
+// path-component limits (255 bytes); generous for any realistic
+// margeAccountUUID or MAC-derived device ID.
+const maxSafeIdentifierLength = 128
+
+// IsSafeIdentifier returns true if the given identifier is safe to use
+// as a single path component (for account IDs, device IDs, etc.), and
+// safe to embed in the other places these values end up: XML sent to a
+// speaker, log lines, and datastore-key comparisons. It rejects empty
+// or overlong strings, path separators, and parent directory
+// references.
+//
+// The allowed character set intentionally excludes XML/HTML-special
+// characters (`< > & " '`), whitespace, and shell/URL metacharacters
+// (see #634's `postSetMargeAccount`, which interpolates an account ID
+// into an XML body, and `PairAccount`, which interpolates one into a
+// literal `envswitch accountid set <id>` telnet command line) even
+// though it accepts more than Bose's own 7-digit account format —
+// devices paired via third-party or manual tooling (e.g. the
+// USB-stick SSH-enable method) can report arbitrary margeAccountUUID
+// values such as "stick@local".
+func IsSafeIdentifier(id string) bool {
+	if id == "" || len(id) > maxSafeIdentifierLength {
 		return false
 	}

@@ -46,14 +65,17 @@ func isSafeIdentifier(id string) bool {
 		return false
 	}

-	// Allow a conservative set of characters commonly found in IDs:
-	// letters, digits, underscore, dash, dot, and colon (for MAC-like IDs).
+	// Letters, digits, and a conservative set of punctuation seen in
+	// real-world IDs: underscore, dash, dot, colon (MAC-like IDs), and
+	// '@' (e.g. "stick@local"). Everything else — including all XML,
+	// HTML, shell, and URL metacharacters, whitespace, and control
+	// characters — is rejected.
 	for i := 0; i < len(id); i++ {
 		c := id[i]
 		if (c >= 'a' && c <= 'z') ||
 			(c >= 'A' && c <= 'Z') ||
 			(c >= '0' && c <= '9') ||
-			c == '_' || c == '-' || c == '.' || c == ':' {
+			c == '_' || c == '-' || c == '.' || c == ':' || c == '@' {
 			continue
 		}

@@ -928,7 +950,10 @@ func (ds *DataStore) parseDeviceInfoFile(path string) (*models.ServiceDeviceInfo

 // GetPresets retrieves all presets for the specified account and device.
 func (ds *DataStore) GetPresets(account, device string) ([]models.ServicePreset, error) {
-	presets, needsRewrite, err := ds.readPresetsLocked(account, device)
+	ds.fileMutex.RLock()
+	presets, needsRewrite, err := ds.readPresetsNoLock(account, device)
+	ds.fileMutex.RUnlock()
+
 	if err != nil {
 		return nil, err
 	}
@@ -944,18 +969,48 @@ func (ds *DataStore) GetPresets(account, device string) ([]models.ServicePreset,
 	return presets, nil
 }

-// readPresetsLocked is the locked read half of GetPresets. It returns the
-// parsed presets and a flag indicating whether the on-disk file used the
-// legacy <ContentItem> (capital C) format that needs rewriting.
-func (ds *DataStore) readPresetsLocked(account, device string) ([]models.ServicePreset, bool, error) {
-	ds.fileMutex.RLock()
-	defer ds.fileMutex.RUnlock()
+// MutatePresets atomically reads the current preset list, transforms it via
+// mutate, and persists the result — holding a single write lock for the
+// entire read-mutate-write cycle. Calling GetPresets followed by a separate
+// SavePresets leaves a lost-update window open: two concurrent callers can
+// each read the same starting list, mutate different entries, and the
+// second writer's SavePresets silently clobbers the first writer's update.
+// That's the exact interleave that dropped a preset during #614's rapid-fire
+// repro (overlapping PUT .../preset/N requests). Callers that read-then-write
+// a single device's presets should use this instead of GetPresets+SavePresets.
+func (ds *DataStore) MutatePresets(account, device string, mutate func(current []models.ServicePreset) ([]models.ServicePreset, error)) ([]models.ServicePreset, error) {
+	ds.fileMutex.Lock()
+	defer ds.fileMutex.Unlock()

+	current, _, err := ds.readPresetsNoLock(account, device)
+	if err != nil {
+		return nil, err
+	}
+
+	next, err := mutate(current)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := ds.savePresetsNoLock(account, device, next); err != nil {
+		return nil, err
+	}
+
+	return next, nil
+}
+
+// readPresetsNoLock is the lock-free read half of GetPresets/MutatePresets.
+// Callers must already hold ds.fileMutex (for reading or writing). It
+// returns the parsed presets and a flag indicating whether the on-disk file
+// used the legacy <ContentItem> (capital C) format that needs rewriting.
+func (ds *DataStore) readPresetsNoLock(account, device string) ([]models.ServicePreset, bool, error) {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.PresetsFile)

 	data, err := ds.rootReadFile(path)
 	if err != nil {
 		if os.IsNotExist(err) {
+			log.Printf("[Datastore] readPresetsNoLock: no Presets.xml at %s — reporting no presets", sanitizeLog(path))
+
 			return []models.ServicePreset{}, false, nil
 		}

@@ -967,7 +1022,7 @@ func (ds *DataStore) readPresetsLocked(account, device string) ([]models.Service
 	// error, so the device-level /presets endpoint returns an empty list
 	// instead of HTTP 500. See #458.
 	if len(bytes.TrimSpace(data)) == 0 {
-		log.Printf("[Datastore] readPresetsLocked: empty/0-byte Presets.xml at %s — treating as no presets (#458)", sanitizeLog(path))
+		log.Printf("[Datastore] readPresetsNoLock: empty/0-byte Presets.xml at %s — treating as no presets (#458)", sanitizeLog(path))

 		return []models.ServicePreset{}, false, nil
 	}
@@ -999,7 +1054,7 @@ func (ds *DataStore) readPresetsLocked(account, device string) ([]models.Service
 	needsRewrite := !bytes.Equal(normalized, data)

 	if err := xml.Unmarshal(normalized, &presetsWrap); err != nil {
-		log.Printf("[Datastore] readPresetsLocked: malformed Presets.xml at %s (%s) — treating as no presets (#458)", sanitizeLog(path), sanitizeErr(err))
+		log.Printf("[Datastore] readPresetsNoLock: malformed Presets.xml at %s (%s) — treating as no presets (#458)", sanitizeLog(path), sanitizeErr(err))

 		return []models.ServicePreset{}, false, nil
 	}
@@ -1053,7 +1108,7 @@ func repairLeakedSource(account, device, label, persistedSource, sourceID string
 		return persistedSource
 	}

-	sources, err := ds.getConfiguredSourcesLocked(account, device)
+	sources, err := ds.getConfiguredSourcesNoLock(account, device)
 	if err != nil {
 		return persistedSource
 	}
@@ -1079,11 +1134,11 @@ func isLeakedSourceValue(s string) bool {
 	return s == "" || s == "Audio"
 }

-// getConfiguredSourcesLocked is GetConfiguredSources without the
+// getConfiguredSourcesNoLock is GetConfiguredSources without the
 // fileMutex.RLock() — callers must already hold it. Used by
 // repairLeakedSource from within GetPresets/GetRecents which already
 // hold the lock.
-func (ds *DataStore) getConfiguredSourcesLocked(account, device string) ([]models.ConfiguredSource, error) {
+func (ds *DataStore) getConfiguredSourcesNoLock(account, device string) ([]models.ConfiguredSource, error) {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.SourcesFile)

 	data, err := ds.rootReadFile(path)
@@ -1131,6 +1186,12 @@ func (ds *DataStore) SavePresets(account, device string, presets []models.Servic
 	ds.fileMutex.Lock()
 	defer ds.fileMutex.Unlock()

+	return ds.savePresetsNoLock(account, device, presets)
+}
+
+// savePresetsNoLock is the lock-free write half of
+// SavePresets/MutatePresets. Callers must already hold ds.fileMutex.Lock().
+func (ds *DataStore) savePresetsNoLock(account, device string, presets []models.ServicePreset) error {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.PresetsFile)
 	if err := ds.rootMkdirAll(filepath.Dir(path), 0755); err != nil {
 		return err
@@ -1294,11 +1355,45 @@ func (ds *DataStore) GetRecents(account, device string) ([]models.ServiceRecent,
 	ds.fileMutex.RLock()
 	defer ds.fileMutex.RUnlock()

+	return ds.readRecentsNoLock(account, device)
+}
+
+// MutateRecents atomically reads the current recents list, transforms it
+// via mutate, and persists the result — holding a single write lock for the
+// entire read-mutate-write cycle. See MutatePresets for why this matters: a
+// separate GetRecents followed by SaveRecents leaves a lost-update window
+// open between concurrent callers.
+func (ds *DataStore) MutateRecents(account, device string, mutate func(current []models.ServiceRecent) ([]models.ServiceRecent, error)) ([]models.ServiceRecent, error) {
+	ds.fileMutex.Lock()
+	defer ds.fileMutex.Unlock()
+
+	current, err := ds.readRecentsNoLock(account, device)
+	if err != nil {
+		return nil, err
+	}
+
+	next, err := mutate(current)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := ds.saveRecentsNoLock(account, device, next); err != nil {
+		return nil, err
+	}
+
+	return next, nil
+}
+
+// readRecentsNoLock is the lock-free read half of GetRecents/MutateRecents.
+// Callers must already hold ds.fileMutex (for reading or writing).
+func (ds *DataStore) readRecentsNoLock(account, device string) ([]models.ServiceRecent, error) {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.RecentsFile)

 	data, err := ds.rootReadFile(path)
 	if err != nil {
 		if os.IsNotExist(err) {
+			log.Printf("[Datastore] GetRecents: no Recents.xml at %s — reporting no recents", sanitizeLog(path))
+
 			return []models.ServiceRecent{}, nil
 		}

@@ -1404,6 +1499,12 @@ func (ds *DataStore) SaveRecents(account, device string, recents []models.Servic
 	ds.fileMutex.Lock()
 	defer ds.fileMutex.Unlock()

+	return ds.saveRecentsNoLock(account, device, recents)
+}
+
+// saveRecentsNoLock is the lock-free write half of
+// SaveRecents/MutateRecents. Callers must already hold ds.fileMutex.Lock().
+func (ds *DataStore) saveRecentsNoLock(account, device string, recents []models.ServiceRecent) error {
 	dir := ds.AccountDeviceDir(account, device)
 	if err := ds.rootMkdirAll(dir, 0755); err != nil {
 		return err
@@ -1509,7 +1610,7 @@ func (ds *DataStore) SaveDeviceInfo(account, device string, info *models.Service
 		return fmt.Errorf("device ID/name cannot be empty")
 	}

-	if !isSafeIdentifier(device) {
+	if !IsSafeIdentifier(device) {
 		return fmt.Errorf("invalid device ID")
 	}

@@ -1517,7 +1618,7 @@ func (ds *DataStore) SaveDeviceInfo(account, device string, info *models.Service
 		return fmt.Errorf("account ID cannot be empty")
 	}

-	if !isSafeIdentifier(account) {
+	if !IsSafeIdentifier(account) {
 		return fmt.Errorf("invalid account ID")
 	}

@@ -1705,6 +1806,10 @@ func (ds *DataStore) SaveAccountInfo(accountID string, info *models.ServiceAccou
 		return nil
 	}

+	if !IsSafeIdentifier(accountID) {
+		return fmt.Errorf("invalid account ID")
+	}
+
 	dir := ds.AccountDir(accountID)
 	if err := ds.rootMkdirAll(dir, 0755); err != nil {
 		return err
@@ -1905,6 +2010,40 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf
 	ds.fileMutex.RLock()
 	defer ds.fileMutex.RUnlock()

+	return ds.readConfiguredSourcesNoLock(account, device)
+}
+
+// MutateConfiguredSources atomically reads the current configured-source
+// list, transforms it via mutate, and persists the result — holding a
+// single write lock for the entire read-mutate-write cycle. See
+// MutatePresets for why this matters: a separate GetConfiguredSources
+// followed by SaveConfiguredSources leaves a lost-update window open
+// between concurrent callers.
+func (ds *DataStore) MutateConfiguredSources(account, device string, mutate func(current []models.ConfiguredSource) ([]models.ConfiguredSource, error)) ([]models.ConfiguredSource, error) {
+	ds.fileMutex.Lock()
+	defer ds.fileMutex.Unlock()
+
+	current, err := ds.readConfiguredSourcesNoLock(account, device)
+	if err != nil {
+		return nil, err
+	}
+
+	next, err := mutate(current)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := ds.saveConfiguredSourcesNoLock(account, device, next); err != nil {
+		return nil, err
+	}
+
+	return next, nil
+}
+
+// readConfiguredSourcesNoLock is the lock-free read half of
+// GetConfiguredSources/MutateConfiguredSources. Callers must already hold
+// ds.fileMutex (for reading or writing).
+func (ds *DataStore) readConfiguredSourcesNoLock(account, device string) ([]models.ConfiguredSource, error) {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.SourcesFile)

 	// defaultSources is the fallback used whenever there is no usable
@@ -2076,6 +2215,13 @@ func (ds *DataStore) SaveConfiguredSources(account, device string, sources []mod
 	ds.fileMutex.Lock()
 	defer ds.fileMutex.Unlock()

+	return ds.saveConfiguredSourcesNoLock(account, device, sources)
+}
+
+// saveConfiguredSourcesNoLock is the lock-free write half of
+// SaveConfiguredSources/MutateConfiguredSources. Callers must already hold
+// ds.fileMutex.Lock().
+func (ds *DataStore) saveConfiguredSourcesNoLock(account, device string, sources []models.ConfiguredSource) error {
 	path := filepath.Join(ds.AccountDeviceDir(account, device), constants.SourcesFile)
 	if err := ds.rootMkdirAll(filepath.Dir(path), 0755); err != nil {
 		return err
@@ -2,6 +2,7 @@ package datastore

 import (
 	"os"
+	"strings"
 	"testing"

 	"github.com/gesellix/bose-soundtouch/pkg/models"
@@ -19,6 +20,10 @@ func TestIsSafeIdentifier(t *testing.T) {
 		{"abc-123", true},
 		{"abc.123", true},
 		{"00:11:22:33:44:55", true},
+		// #634: third-party/manual pairing tools (e.g. the USB-stick
+		// SSH-enable method) can report a non-numeric margeAccountUUID.
+		{"stick@local", true},
+		{strings.Repeat("a", maxSafeIdentifierLength), true},
 		{"", false},
 		{"/", false},
 		{"\\", false},
@@ -30,7 +35,6 @@ func TestIsSafeIdentifier(t *testing.T) {
 		{"a..b", false},
 		{"a b", false},
 		{"a!b", false},
-		{"a@b", false},
 		{"a#b", false},
 		{"a$b", false},
 		{"a%b", false},
@@ -39,12 +43,17 @@ func TestIsSafeIdentifier(t *testing.T) {
 		{"a*b", false},
 		{"a(b", false},
 		{"a)b", false},
+		{"a<b", false},
+		{"a>b", false},
+		{`a"b`, false},
+		{"a'b", false},
+		{strings.Repeat("a", maxSafeIdentifierLength+1), false},
 	}

 	for _, test := range tests {
-		result := isSafeIdentifier(test.id)
+		result := IsSafeIdentifier(test.id)
 		if result != test.expected {
-			t.Errorf("isSafeIdentifier(%q) = %v; expected %v", test.id, result, test.expected)
+			t.Errorf("IsSafeIdentifier(%q) = %v; expected %v", test.id, result, test.expected)
 		}
 	}
 }
@@ -72,6 +81,8 @@ func TestSaveDeviceInfo_Validation(t *testing.T) {
 		{"acc1", "dev/1", true, "invalid device ID"},
 		{"acc..1", "dev1", true, "invalid account ID"},
 		{"acc1", "dev..1", true, "invalid device ID"},
+		// #634: a non-numeric margeAccountUUID is now accepted.
+		{"stick@local", "dev1", false, ""},
 	}

 	for _, test := range tests {
@@ -85,3 +96,40 @@ func TestSaveDeviceInfo_Validation(t *testing.T) {
 		}
 	}
 }
+
+func TestSaveAccountInfo_Validation(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "datastore-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	ds := NewDataStore(tmpDir)
+
+	tests := []struct {
+		account string
+		wantErr bool
+		errMsg  string
+	}{
+		{"acc1", false, ""},
+		// #634: a non-numeric margeAccountUUID reported via
+		// POST /streaming/account (see HandleMargeCreateAccount) must
+		// be validated the same way SaveDeviceInfo already validates
+		// device-reported account IDs.
+		{"stick@local", false, ""},
+		{"acc/1", true, "invalid account ID"},
+		{"acc..1", true, "invalid account ID"},
+		{"a<b", true, "invalid account ID"},
+	}
+
+	for _, test := range tests {
+		err := ds.SaveAccountInfo(test.account, &models.ServiceAccountInfo{AccountID: test.account})
+		if (err != nil) != test.wantErr {
+			t.Errorf("SaveAccountInfo(%q) error = %v, wantErr %v", test.account, err, test.wantErr)
+			continue
+		}
+		if test.wantErr && err.Error() != test.errMsg {
+			t.Errorf("SaveAccountInfo(%q) error message = %q, want %q", test.account, err.Error(), test.errMsg)
+		}
+	}
+}
@@ -13,6 +13,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"sort"
@@ -255,7 +256,12 @@ func (s *Server) addServiceHTTP(tw *tar.Writer, client *http.Client, devices []m
 		if !seenAccounts[dev.AccountID] {
 			seenAccounts[dev.AccountID] = true
 			pfx := "http/service/account-" + dev.AccountID
-			acct := base + "/streaming/account/" + dev.AccountID
+			// url.PathEscape, not raw concatenation: account/device IDs can
+			// contain characters like '@' (#634) that are safe as datastore
+			// keys but would otherwise need escaping to survive as URL path
+			// segments intact (e.g. a literal '?' or '#' would truncate the
+			// path here, though IsSafeIdentifier already excludes those).
+			acct := base + "/streaming/account/" + url.PathEscape(dev.AccountID)
 			tryAdd(pfx+"/full.xml", acct+"/full")
 			tryAdd(pfx+"/sources.xml", acct+"/sources")
 			tryAdd(pfx+"/presets.xml", acct+"/presets")
@@ -266,7 +272,7 @@ func (s *Server) addServiceHTTP(tw *tar.Writer, client *http.Client, devices []m
 		}

 		dpfx := "http/service/account-" + dev.AccountID + "/device-" + dev.DeviceID
-		dpath := base + "/streaming/account/" + dev.AccountID + "/device/" + dev.DeviceID
+		dpath := base + "/streaming/account/" + url.PathEscape(dev.AccountID) + "/device/" + url.PathEscape(dev.DeviceID)
 		tryAdd(dpfx+"/presets.xml", dpath+"/presets")
 		tryAdd(dpfx+"/recents.xml", dpath+"/recents")
 	}
@@ -14,6 +14,7 @@ import (

 	"github.com/gesellix/bose-soundtouch/pkg/models"
 	"github.com/gesellix/bose-soundtouch/pkg/service/constants"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 	"github.com/gesellix/bose-soundtouch/pkg/service/marge"
 	"github.com/go-chi/chi/v5"
 )
@@ -64,6 +65,11 @@ func (s *Server) HandleMargeCreateAccount(w http.ResponseWriter, r *http.Request
 		}
 	}

+	if !datastore.IsSafeIdentifier(id) {
+		http.Error(w, "Invalid account ID", http.StatusBadRequest)
+		return
+	}
+
 	info := &models.ServiceAccountInfo{
 		AccountID:         id,
 		PreferredLanguage: req.PreferredLanguage,
@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"strings"

+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 	"github.com/gesellix/bose-soundtouch/pkg/service/health"
 	"github.com/gesellix/bose-soundtouch/pkg/service/setup"
 	"github.com/go-chi/chi/v5"
@@ -61,12 +62,12 @@ type pairAccountResponse struct {
 	Error  string                  `json:"error,omitempty"`
 }

-// HandlePairAccount associates the device with the supplied 7-digit account ID,
+// HandlePairAccount associates the device with the supplied account ID,
 // trying HTTP /setMargeAccount first and falling back to telnet
 // `envswitch accountid set`.
 //
 // Query params:
-//   - account_id (required) — must pass setup.IsValidAccountID
+//   - account_id (required) — must pass datastore.IsSafeIdentifier
 func (s *Server) HandlePairAccount(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "deviceId")
 	if deviceID == "" {
@@ -75,8 +76,8 @@ func (s *Server) HandlePairAccount(w http.ResponseWriter, r *http.Request) {
 	}

 	accountID := r.URL.Query().Get("account_id")
-	if !setup.IsValidAccountID(accountID) {
-		writeJSONError(w, http.StatusBadRequest, "account_id must be exactly 7 digits")
+	if !datastore.IsSafeIdentifier(accountID) {
+		writeJSONError(w, http.StatusBadRequest, "account_id must be a non-empty, path-safe identifier")
 		return
 	}

@@ -145,7 +146,7 @@ func (s *Server) completeSpeakerPairingFix(target health.Target) (string, error)
 	}

 	accountID := target.Account
-	if !setup.IsValidAccountID(accountID) {
+	if !datastore.IsSafeIdentifier(accountID) {
 		known, _ := s.ds.ListAccounts()

 		generated, genErr := setup.GenerateAccountID(known)
@@ -1274,7 +1274,16 @@ func (s *Server) HandleTestDNSRedirection(w http.ResponseWriter, r *http.Request
 	}
 }

-// HandleInitialSync fetches presets, recents and sources from the device and saves them to the datastore.
+// HandleInitialSync fetches presets, recents and sources from the device
+// and saves them to the datastore.
+//
+// If applying the fetched presets/recents would shrink what's already
+// stored, the sync is not applied — the response comes back 409 with the
+// diff describing what would be removed — unless the caller passes
+// ?confirmed=true, in which case it's applied unconditionally. Every call
+// re-fetches live from the speaker at that moment (see
+// setup.SyncDeviceData), so a confirmed retry re-checks current reality
+// rather than replaying a possibly-stale earlier response.
 func (s *Server) HandleInitialSync(w http.ResponseWriter, r *http.Request) {
 	deviceID := chi.URLParam(r, "deviceId")
 	if deviceID == "" {
@@ -1288,13 +1297,25 @@ func (s *Server) HandleInitialSync(w http.ResponseWriter, r *http.Request) {
 		return
 	}

-	if err := s.sm.SyncDeviceData(deviceIP); err != nil {
+	confirmed := r.URL.Query().Get("confirmed") == "true"
+
+	result, err := s.sm.SyncDeviceData(deviceIP, confirmed)
+	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}

-	w.WriteHeader(http.StatusOK)
-	_, _ = w.Write([]byte(`{"ok": true}`))
+	w.Header().Set("Content-Type", "application/json")
+
+	if !result.Applied {
+		w.WriteHeader(http.StatusConflict)
+	} else {
+		w.WriteHeader(http.StatusOK)
+	}
+
+	if encodeErr := json.NewEncoder(w).Encode(result); encodeErr != nil {
+		log.Printf("HandleInitialSync: failed to encode result for device %s: %s", sanitizeLog(deviceID), sanitizeErr(encodeErr))
+	}
 }

 // HandleRebootDevice reboots a device.
@@ -0,0 +1,153 @@
+package handlers
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+	"github.com/gesellix/bose-soundtouch/pkg/service/setup"
+	"github.com/go-chi/chi/v5"
+)
+
+// TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed is
+// an HTTP-level regression test for #614's Sync-button data-loss bug (see
+// setup.TestSyncDeviceData_DestructiveSyncRequiresConfirmation for the
+// lower-level coverage of the same fix): a device already has more presets
+// stored than the mock speaker's live /presets now reports. The first,
+// unconfirmed sync request must come back 409 with the diff and must not
+// write anything; a retry with ?confirmed=true must apply it.
+func TestHandleInitialSync_DestructiveSyncReturns409ThenAppliesWhenConfirmed(t *testing.T) {
+	const (
+		accountID = "1234567"
+		deviceID  = "AABBCCDDEEFF"
+	)
+
+	// A real local server, not a black-hole IP: notifySpeakerSourcesUpdated
+	// (part of the confirmed-apply path) uses its own HTTP client rather
+	// than the injectable sm.HTTPGet, so it needs somewhere real to fail
+	// fast against (404) instead of timing out.
+	mockDevice := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/info":
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprintf(w, `<?xml version="1.0" encoding="UTF-8"?><info deviceID="%s"><name>Test Device</name><type>SoundTouch 20</type><margeAccountUUID>%s</margeAccountUUID></info>`, deviceID, accountID)
+		case "/presets":
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?><presets><preset id="1"><ContentItem source="LOCAL_INTERNET_RADIO" type="stationurl" location="/x" isPresetable="true"><itemName>Station 1</itemName></ContentItem></preset></presets>`)
+		case "/recents":
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?><recents></recents>`)
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer mockDevice.Close()
+
+	deviceIP := mockDevice.Listener.Addr().String()
+
+	tempDir, err := os.MkdirTemp("", "handlers-sync-destructive-guard-*")
+	if err != nil {
+		t.Fatalf("tempdir: %v", err)
+	}
+	defer func() { _ = os.RemoveAll(tempDir) }()
+
+	ds := datastore.NewDataStore(tempDir)
+
+	seeded := []models.ServicePreset{
+		{ID: "1", ButtonNumber: "1", ServiceContentItem: models.ServiceContentItem{Name: "Station 1"}},
+		{ID: "2", ButtonNumber: "2", ServiceContentItem: models.ServiceContentItem{Name: "Station 2"}},
+	}
+	if err := ds.SavePresets(accountID, deviceID, seeded); err != nil {
+		t.Fatalf("seed SavePresets: %v", err)
+	}
+
+	if err := ds.SaveDeviceInfo(accountID, deviceID, &models.ServiceDeviceInfo{
+		DeviceID:  deviceID,
+		AccountID: accountID,
+		IPAddress: deviceIP,
+	}); err != nil {
+		t.Fatalf("SaveDeviceInfo: %v", err)
+	}
+
+	sm := setup.NewManager("http://localhost:8000", ds, nil)
+
+	server := NewServer(ds, sm, "http://localhost:8000", false, false, false)
+
+	r := chi.NewRouter()
+	r.Post("/api/setup/sync/{deviceId}", server.HandleInitialSync)
+
+	ts := httptest.NewServer(r)
+	defer ts.Close()
+
+	// First, unconfirmed request: must be refused with 409.
+	resp, err := http.Post(ts.URL+"/api/setup/sync/"+deviceID, "application/json", nil)
+	if err != nil {
+		t.Fatalf("POST sync (unconfirmed): %v", err)
+	}
+
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != http.StatusConflict {
+		body, _ := io.ReadAll(resp.Body)
+		t.Fatalf("expected 409 for a destructive unconfirmed sync, got %d: %s", resp.StatusCode, body)
+	}
+
+	var result setup.SyncResult
+	if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
+		t.Fatalf("decode 409 body: %v", err)
+	}
+
+	if result.Applied {
+		t.Fatal("expected Applied=false in the 409 response")
+	}
+
+	if !result.Destructive {
+		t.Fatal("expected Destructive=true in the 409 response")
+	}
+
+	presetsAfterRefusal, err := ds.GetPresets(accountID, deviceID)
+	if err != nil {
+		t.Fatalf("GetPresets after refused sync: %v", err)
+	}
+
+	if len(presetsAfterRefusal) != 2 {
+		t.Fatalf("expected the original 2 presets to survive the refused sync, got %d", len(presetsAfterRefusal))
+	}
+
+	// Retry, confirmed: must apply.
+	resp2, err := http.Post(ts.URL+"/api/setup/sync/"+deviceID+"?confirmed=true", "application/json", nil)
+	if err != nil {
+		t.Fatalf("POST sync (confirmed): %v", err)
+	}
+
+	defer func() { _ = resp2.Body.Close() }()
+
+	if resp2.StatusCode != http.StatusOK {
+		body, _ := io.ReadAll(resp2.Body)
+		t.Fatalf("expected 200 for a confirmed sync, got %d: %s", resp2.StatusCode, body)
+	}
+
+	var confirmedResult setup.SyncResult
+	if err := json.NewDecoder(resp2.Body).Decode(&confirmedResult); err != nil {
+		t.Fatalf("decode 200 body: %v", err)
+	}
+
+	if !confirmedResult.Applied {
+		t.Fatal("expected Applied=true after confirming")
+	}
+
+	presetsAfterConfirm, err := ds.GetPresets(accountID, deviceID)
+	if err != nil {
+		t.Fatalf("GetPresets after confirmed sync: %v", err)
+	}
+
+	if len(presetsAfterConfirm) != 1 {
+		t.Fatalf("expected confirmed sync to shrink to 1 preset, got %d", len(presetsAfterConfirm))
+	}
+}
@@ -0,0 +1,125 @@
+package handlers
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+	"github.com/gesellix/bose-soundtouch/pkg/service/setup"
+)
+
+// TestIssue634_NonNumericMargeAccountUUIDDoesNotLoseDevice reproduces
+// https://github.com/gesellix/Bose-SoundTouch/issues/634
+//
+// A SoundTouch 10 had SSH enabled via the USB-stick method (rather than
+// AfterTouch's own telnet-based enable-ssh flow) and, when discovered,
+// reported a `margeAccountUUID` of `stick@local` instead of the usual
+// 7-digit numeric Bose account ID. `handleDiscoveredDevice`
+// (pkg/service/handlers/server.go) passes MargeAccountUUID straight
+// through to DataStore.SaveDeviceInfo, which used to reject anything
+// containing "@" as an "invalid account ID" via isSafeIdentifier's
+// strict alnum-only allowlist. The device was never persisted at all.
+//
+// The fix widened datastore.IsSafeIdentifier to accept any device-reported
+// identifier that's safe to use as a path component / XML value /
+// telnet-command token, rather than requiring Bose's own 7-digit numeric
+// format. setup's separate, stricter 7-digit-only IsValidAccountID was
+// deleted outright in favor of calling datastore.IsSafeIdentifier directly
+// everywhere an account ID needs validating — one validator, not two. So
+// handleDiscoveredDevice needed no changes: it already passed
+// MargeAccountUUID through unmodified, and now the datastore accepts it.
+//
+// What this test locks in:
+//
+//   - A speaker reporting a non-numeric margeAccountUUID is saved
+//     under that account verbatim (not coerced to "default" — "default"
+//     remains reserved for a genuinely empty/unpaired margeAccountUUID).
+//
+// What this test would catch if it flipped:
+//
+//   - If IsSafeIdentifier's allowlist regresses to reject "@" again,
+//     GetDeviceInfo below would error with "invalid account ID" instead
+//     of returning the device — the #634 symptom.
+func TestIssue634_NonNumericMargeAccountUUIDDoesNotLoseDevice(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "issue634-*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	const deviceInfoXML = `<info deviceID="001122334455">
+<name>Kitchen SoundTouch</name>
+<type>SoundTouch 10</type>
+<margeAccountUUID>stick@local</margeAccountUUID>
+<components>
+<component>
+<componentCategory>SCM</componentCategory>
+<softwareVersion>27.0.6.46330.5043500 epdbuild.trunk.hepdswbld04.2022-08-04T11:20:29</softwareVersion>
+<serialNumber>I6332527703739342000020</serialNumber>
+</component>
+<component>
+<componentCategory>PackagedProduct</componentCategory>
+<softwareVersion>27.0.6.46330.5043500 epdbuild.trunk.hepdswbld04.2022-08-04T11:20:29</softwareVersion>
+<serialNumber>069231P63364828AE</serialNumber>
+</component>
+</components>
+<margeURL>https://streaming.bose.com</margeURL>
+<networkInfo type="SCM">
+<macAddress>001122334455</macAddress>
+<ipAddress>203.0.113.10</ipAddress>
+</networkInfo>
+<moduleType>sm2</moduleType>
+<variant>rhino</variant>
+<variantMode>normal</variantMode>
+<countryCode>US</countryCode>
+<regionCode>US</regionCode>
+</info>`
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/info" {
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprint(w, deviceInfoXML)
+		} else {
+			http.NotFound(w, r)
+		}
+	}))
+	defer server.Close()
+
+	deviceIP := server.URL[len("http://"):]
+	ds := datastore.NewDataStore(tempDir)
+	sm := setup.NewManager(server.URL, ds, nil)
+
+	srv := NewServer(ds, sm, server.URL, false, false, false)
+
+	discoveredDevice := models.DiscoveredDevice{
+		Host:            deviceIP,
+		Name:            "Legacy Discovery Name",
+		ModelID:         "SoundTouch 10",
+		SerialNo:        "",
+		DiscoveryMethod: "UPnP",
+	}
+
+	t.Logf("Test scenario: /info reports non-numeric margeAccountUUID %q", "stick@local")
+
+	srv.handleDiscoveredDevice(discoveredDevice)
+
+	const (
+		expectedAccountID = "stick@local"
+		expectedDeviceID  = "001122334455"
+	)
+
+	deviceInfo, err := ds.GetDeviceInfo(expectedAccountID, expectedDeviceID)
+	if err != nil {
+		t.Fatalf("device was not saved under account %q: %v (this is the #634 symptom — "+
+			"SaveDeviceInfo rejects the raw margeAccountUUID as an invalid account ID)",
+			expectedAccountID, err)
+	}
+
+	if deviceInfo.Name != "Kitchen SoundTouch" {
+		t.Errorf("Name = %q, want %q", deviceInfo.Name, "Kitchen SoundTouch")
+	}
+}
@@ -598,7 +598,19 @@ async function fetchDevices() {
        if (devices.length === 0) {
            container.innerHTML = "No devices known yet.";
        } else {
-            let html = "<table><tr><th>Name & Model</th><th>IP Address</th><th>Device & Account ID</th><th>Firmware & Serial</th><th>Method</th><th>Action</th></tr>";
+            // Built via DOM APIs rather than innerHTML/template strings: device
+            // fields (name, IDs, serials, ...) come from speakers and third-party
+            // pairing tools (see #634) and are not restricted to HTML/JS-safe
+            // characters, so they must never be parsed as markup or concatenated
+            // into inline event-handler attributes.
+            const table = document.createElement("table");
+            const headerRow = document.createElement("tr");
+            for (const label of ["Name & Model", "IP Address", "Device & Account ID", "Firmware & Serial", "Method", "Action"]) {
+                const th = document.createElement("th");
+                th.textContent = label;
+                headerRow.appendChild(th);
+            }
+            table.appendChild(headerRow);

            // Clear and repopulate selectors
            const currentSyncVal = syncSelector.value;
@@ -612,25 +624,83 @@ async function fetchDevices() {

            devices.forEach((d) => {
                const methodLabel = d.discovery_method === "manual" ? "👤 Manual" : "🔍 Auto";
-                html += `
-                    <tr id="device-row-${d.device_id}">
-                        <td class="col-name-model"><div class="col-name">${d.name}</div><div class="col-model" style="font-size: 0.8em; color: #666;">${d.product_code}</div></td>
-                        <td class="col-ip">${d.ip_address}</td>
-                        <td class="col-ids"><div class="col-deviceid">${d.device_id}</div><div class="col-accountid" style="font-size: 0.8em; color: #666;">${d.account_id || "default"}</div></td>
-                        <td class="col-fw-serial"><div class="col-firmware">${d.firmware_version || "0.0.0"}</div><div class="col-serial" style="font-size: 0.8em; color: #666;">${d.device_serial_number}</div></td>
-                        <td class="col-method">${methodLabel}</td>
-                        <td>
-                            <button onclick="toggleDeviceSummary('${d.device_id}')">Inspect</button>
-                            <button onclick="prepareSync('${d.device_id}')">Sync Data</button>
-                            <button onclick="prepareMigration('${d.device_id}')">Migrate</button>
-                            <button id="prime-spotify-${d.device_id}" class="btn-spotify" style="display: none;" onclick="primeSpotify('${d.device_id}')">Prime Spotify</button>
-                            <button class="btn-danger" onclick="removeDevice('${d.device_id}', '${d.name}')">Remove</button>
-                        </td>
-                    </tr>
-                    <tr id="device-summary-${d.device_id}" style="display: none;">
-                        <td colspan="6" id="device-summary-cell-${d.device_id}" style="background: #fafafa; padding: 12px;"></td>
-                    </tr>
-                `;
+
+                const nameModelCell = document.createElement("td");
+                nameModelCell.className = "col-name-model";
+                const nameDiv = document.createElement("div");
+                nameDiv.className = "col-name";
+                nameDiv.textContent = d.name;
+                const modelDiv = document.createElement("div");
+                modelDiv.className = "col-model";
+                modelDiv.style.cssText = "font-size: 0.8em; color: #666;";
+                modelDiv.textContent = d.product_code;
+                nameModelCell.append(nameDiv, modelDiv);
+
+                const ipCell = document.createElement("td");
+                ipCell.className = "col-ip";
+                ipCell.textContent = d.ip_address;
+
+                const idsCell = document.createElement("td");
+                idsCell.className = "col-ids";
+                const deviceIdDiv = document.createElement("div");
+                deviceIdDiv.className = "col-deviceid";
+                deviceIdDiv.textContent = d.device_id;
+                const accountIdDiv = document.createElement("div");
+                accountIdDiv.className = "col-accountid";
+                accountIdDiv.style.cssText = "font-size: 0.8em; color: #666;";
+                accountIdDiv.textContent = d.account_id || "default";
+                idsCell.append(deviceIdDiv, accountIdDiv);
+
+                const fwCell = document.createElement("td");
+                fwCell.className = "col-fw-serial";
+                const fwDiv = document.createElement("div");
+                fwDiv.className = "col-firmware";
+                fwDiv.textContent = d.firmware_version || "0.0.0";
+                const serialDiv = document.createElement("div");
+                serialDiv.className = "col-serial";
+                serialDiv.style.cssText = "font-size: 0.8em; color: #666;";
+                serialDiv.textContent = d.device_serial_number;
+                fwCell.append(fwDiv, serialDiv);
+
+                const methodCell = document.createElement("td");
+                methodCell.className = "col-method";
+                methodCell.textContent = methodLabel;
+
+                const makeActionButton = (label, onClick, extra) => {
+                    const btn = document.createElement("button");
+                    btn.textContent = label;
+                    btn.addEventListener("click", onClick);
+                    if (extra) Object.assign(btn, extra);
+                    return btn;
+                };
+
+                const actionCell = document.createElement("td");
+                actionCell.append(
+                    makeActionButton("Inspect", () => toggleDeviceSummary(d.device_id)),
+                    makeActionButton("Sync Data", () => prepareSync(d.device_id)),
+                    makeActionButton("Migrate", () => prepareMigration(d.device_id)),
+                    makeActionButton("Prime Spotify", () => primeSpotify(d.device_id), {
+                        id: `prime-spotify-${d.device_id}`,
+                        className: "btn-spotify",
+                    }),
+                    makeActionButton("Remove", () => removeDevice(d.device_id, d.name), {className: "btn-danger"}),
+                );
+                actionCell.querySelector(".btn-spotify").style.display = "none";
+
+                const row = document.createElement("tr");
+                row.id = `device-row-${d.device_id}`;
+                row.append(nameModelCell, ipCell, idsCell, fwCell, methodCell, actionCell);
+
+                const summaryRow = document.createElement("tr");
+                summaryRow.id = `device-summary-${d.device_id}`;
+                summaryRow.style.display = "none";
+                const summaryCell = document.createElement("td");
+                summaryCell.colSpan = 6;
+                summaryCell.id = `device-summary-cell-${d.device_id}`;
+                summaryCell.style.cssText = "background: #fafafa; padding: 12px;";
+                summaryRow.appendChild(summaryCell);
+
+                table.append(row, summaryRow);

                const optSync = document.createElement("option");
                optSync.value = d.device_id;
@@ -649,8 +719,7 @@ async function fetchDevices() {
                    eventSelector.appendChild(optEvent);
                }
            });
-            html += "</table>";
-            container.innerHTML = html;
+            container.replaceChildren(table);

            if (currentSyncVal) syncSelector.value = currentSyncVal;
            if (currentMigrationVal) migrationSelector.value = currentMigrationVal;
@@ -807,6 +876,57 @@ function getDeviceDisplayName(deviceId) {
    return deviceId;
 }

+// buildSyncConfirmMessage renders a human-readable summary of a destructive
+// SyncResult (see setup.SyncResult/SyncResourceDiff) for window.confirm() —
+// e.g. "Sync would remove 1 preset: Ici Roussillon. Continue?".
+function buildSyncConfirmMessage(result) {
+    const lines = ["This Data Sync would remove data that's currently stored:"];
+    for (const diff of result.diffs || []) {
+        if (!diff.destructive) {
+            continue;
+        }
+        const removedNote = diff.removed && diff.removed.length ? ": " + diff.removed.join(", ") : "";
+        lines.push("- " + diff.resource + ": " + diff.currentCount + " → " + diff.incomingCount + removedNote);
+    }
+    lines.push("This usually means the speaker's own live data was incomplete at this moment. Continue anyway?");
+    return lines.join("\n");
+}
+
+// renderSyncResultList builds a <ul> summarising a successful SyncResult —
+// one <li> per resource, e.g. "presets: 6 → 6", plus a sources count. Built
+// via DOM APIs (not innerHTML string concatenation) since preset/recent
+// names ultimately come from user-editable station names on the speaker.
+function renderSyncResultList(result) {
+    const ul = document.createElement("ul");
+
+    for (const diff of result.diffs || []) {
+        const li = document.createElement("li");
+        li.textContent = diff.resource + ": " + diff.currentCount + " → " + diff.incomingCount;
+        ul.appendChild(li);
+    }
+
+    const sourcesLi = document.createElement("li");
+    sourcesLi.textContent = "sources: " + (result.sourcesCount >= 0 ? result.sourcesCount : "sync failed");
+    ul.appendChild(sourcesLi);
+
+    return ul;
+}
+
+async function requestSync(deviceId, confirmed) {
+    let url = "/api/setup/sync/" + encodeURIComponent(deviceId);
+    if (confirmed) {
+        url += "?confirmed=true";
+    }
+    const response = await fetch(url, {method: "POST"});
+    let result = null;
+    try {
+        result = await response.clone().json();
+    } catch (e) {
+        // Non-JSON error body (e.g. a plain-text 500) — handled below via response.text().
+    }
+    return {response, result};
+}
+
 async function startSync() {
    const deviceId = document.getElementById("sync-device-list").value;
    if (!deviceId) {
@@ -826,14 +946,30 @@ async function startSync() {
    log.innerHTML = "";

    try {
-        const response = await fetch("/api/setup/sync/" + encodeURIComponent(deviceId), {method: "POST"},);
-        if (response.ok) {
+        let {response, result} = await requestSync(deviceId, false);
+
+        if (response.status === 409 && result) {
+            if (!confirm(buildSyncConfirmMessage(result))) {
+                status.style.backgroundColor = "#eef";
+                status.textContent = "Sync cancelled for " + display + " — nothing was changed.";
+                return;
+            }
+
+            ({response, result} = await requestSync(deviceId, true));
+        }
+
+        if (response.ok && result) {
            status.style.backgroundColor = "#dfd";
            status.textContent = "✅ Sync completed successfully for " + display + "!";
            results.style.display = "block";
-            log.textContent = "Data fetched and saved to local datastore for " + display + ".\nPresets: OK\nRecents: OK\nSources: OK";
+
+            log.innerHTML = "";
+            const intro = document.createElement("p");
+            intro.textContent = "Data fetched and saved to local datastore for " + display + ".";
+            log.appendChild(intro);
+            log.appendChild(renderSyncResultList(result));
        } else {
-            const err = await response.text();
+            const err = result ? JSON.stringify(result) : await response.text();
            throw new Error(err);
        }
    } catch (error) {
@@ -924,7 +1060,14 @@ async function fetchAccountList() {
        const data = await response.json();
        const selector = document.getElementById("account-selector");
        if (selector) {
-            selector.innerHTML = data.accounts.map(acc => `<option value="${acc}">${acc}</option>`).join("");
+            // Account IDs can contain non-alphanumeric characters (e.g.
+            // "stick@local", #634) — built via DOM APIs, not innerHTML.
+            selector.replaceChildren(...data.accounts.map(acc => {
+                const opt = document.createElement("option");
+                opt.value = acc;
+                opt.textContent = acc;
+                return opt;
+            }));
            if (data.accounts.length > 0) {
                fetchAccountDetails(selector.value);
            }
@@ -949,7 +1092,7 @@ async function fetchAccountDetails(accountId) {
    try {
        const response = await fetch(`/api/mgmt/accounts/${encodeURIComponent(accountId)}`);
        if (!response.ok) {
-            if (metadataEl) metadataEl.innerHTML = `<span style="color:red">Failed to load account details: ${response.statusText}</span>`;
+            if (metadataEl) metadataEl.innerHTML = `<span style="color:red">Failed to load account details: ${escapeHtml(response.statusText)}</span>`;
            return;
        }
        const data = await response.json();
@@ -964,7 +1107,7 @@ async function fetchAccountDetails(accountId) {
            metadataEl.innerHTML = `
                ${warningNotice}
                <table style="width: 100%; font-size: 0.9em;">
-                    <tr><td style="padding: 4px"><strong>Account ID:</strong></td><td style="padding: 4px">${data.account.account_id}</td></tr>
+                    <tr><td style="padding: 4px"><strong>Account ID:</strong></td><td style="padding: 4px">${escapeHtml(data.account.account_id)}</td></tr>
                    <tr><td style="padding: 4px"><strong>Language:</strong></td><td style="padding: 4px">
                        <select id="account-language-select" style="font-size: 0.9em; padding: 2px;">
                            <option value="en" ${data.account.preferred_language === "en" || !data.account.preferred_language ? "selected" : ""}>en</option>
@@ -983,7 +1126,7 @@ async function fetchAccountDetails(accountId) {
                                }, {});
                                return Object.entries(grouped).map(([pName, settings]) => `
                                    <div style="margin-bottom: 8px;">
-                                        <strong>${pName}</strong>
+                                        <strong>${escapeHtml(pName)}</strong>
                                        <ul style="margin: 2px 0 0 0; padding-left: 20px; list-style-type: disc;">
                                            ${settings.map(s => {
                                                if ((s.provider_name === "SPOTIFY" || s.provider_id === "15") && s.key_name === "STREAMING_QUALITY") {
@@ -991,9 +1134,9 @@ async function fetchAccountDetails(accountId) {
                                                        <li style="margin-bottom: 4px;">
                                                            Music Streaming Quality:
                                                            <select class="provider-setting-select"
-                                                                    data-account-id="${data.account.account_id}"
-                                                                    data-provider-id="${s.provider_id}"
-                                                                    data-key="${s.key_name}"
+                                                                    data-account-id="${escapeHtml(data.account.account_id)}"
+                                                                    data-provider-id="${escapeHtml(s.provider_id)}"
+                                                                    data-key="${escapeHtml(s.key_name)}"
                                                                    style="font-size: 0.9em; padding: 2px; margin-left: 4px;">
                                                                <option value="1" ${s.value === "1" ? "selected" : ""}>Fastest Streaming - up to 128 kbit/s</option>
                                                                <option value="2" ${s.value === "2" ? "selected" : ""}>Balanced Quality and Speed - up to 192 kbit/s</option>
@@ -1003,7 +1146,7 @@ async function fetchAccountDetails(accountId) {
                                                        </li>
                                                    `;
                                                }
-                                                return `<li>${s.key_name}: ${s.value}</li>`;
+                                                return `<li>${escapeHtml(s.key_name)}: ${escapeHtml(s.value)}</li>`;
                                            }).join("")}
                                        </ul>
                                    </div>
@@ -1024,7 +1167,7 @@ async function fetchAccountDetails(accountId) {
                        statusEl.style.color = "#666";
                    }
                    try {
-                        const response = await fetch(`/api/mgmt/accounts/${data.account.account_id}/language`, {
+                        const response = await fetch(`/api/mgmt/accounts/${encodeURIComponent(data.account.account_id)}/language`, {
                            method: "POST",
                            headers: {
                                "Content-Type": "application/json",
@@ -1068,7 +1211,7 @@ async function fetchAccountDetails(accountId) {
                    }

                    try {
-                        const response = await fetch(`/api/mgmt/accounts/${accID}/provider-settings`, {
+                        const response = await fetch(`/api/mgmt/accounts/${encodeURIComponent(accID)}/provider-settings`, {
                            method: "POST",
                            headers: {
                                "Content-Type": "application/json",
@@ -1110,27 +1253,27 @@ async function fetchAccountDetails(accountId) {

            devicesEl.innerHTML = data.devices.map(device => `
                <div class="summary-box" style="margin-bottom: 15px; border-left: 5px solid #007bff; padding: 15px;">
-                    <div style="display: flex; justify-content: space-between; cursor: pointer; align-items: center;" onclick="toggleInfo('device-details-${device.device_id}')">
-                        <h4 style="margin: 0">${device.name || "Unnamed Device"} (${device.product_code})</h4>
+                    <div class="device-summary-header" data-toggle-target="device-details-${escapeHtml(device.device_id)}" style="display: flex; justify-content: space-between; cursor: pointer; align-items: center;">
+                        <h4 style="margin: 0">${escapeHtml(device.name || "Unnamed Device")} (${escapeHtml(device.product_code)})</h4>
                        <div style="font-size: 0.8em; color: #666">
-                            ${device.ip_address} | ${device.device_id} <span style="font-size: 1.2em; vertical-align: middle;">&#9662;</span>
+                            ${escapeHtml(device.ip_address)} | ${escapeHtml(device.device_id)} <span style="font-size: 1.2em; vertical-align: middle;">&#9662;</span>
                        </div>
                    </div>

-                    <div id="device-details-${device.device_id}" style="display: none; margin-top: 15px; padding-top: 10px; border-top: 1px solid #eee">
+                    <div id="device-details-${escapeHtml(device.device_id)}" style="display: none; margin-top: 15px; padding-top: 10px; border-top: 1px solid #eee">
                        <div style="display: grid; grid-template-columns: 1fr 1fr; gap: 20px">
                            <div>
                                <h5 style="margin: 10px 0 5px 0">Device Metadata</h5>
                                <div style="font-size: 0.85em; background: #f8f9fa; padding: 8px; border-radius: 4px; border: 1px solid #e9ecef">
-                                    <strong>Serial:</strong> ${device.device_serial_number || device.serial_number || "N/A"}<br>
-                                    <strong>MAC:</strong> ${device.mac_address || "N/A"}<br>
-                                    <strong>Version:</strong> ${device.firmware_version || "N/A"}<br>
-                                    <strong>Discovery:</strong> ${device.discovery_method || "N/A"}
+                                    <strong>Serial:</strong> ${escapeHtml(device.device_serial_number || device.serial_number || "N/A")}<br>
+                                    <strong>MAC:</strong> ${escapeHtml(device.mac_address || "N/A")}<br>
+                                    <strong>Version:</strong> ${escapeHtml(device.firmware_version || "N/A")}<br>
+                                    <strong>Discovery:</strong> ${escapeHtml(device.discovery_method || "N/A")}
                                </div>

                                <h5 style="margin: 15px 0 5px 0">Hardware Components</h5>
                                <ul style="font-size: 0.8em; padding-left: 20px; margin: 0">
-                                    ${device.components ? device.components.map(c => `<li><strong>${c.category || c.type || 'Component'}</strong>: ${c.firmware_version || 'N/A'} <br><small style="color:#777">S/N: ${c.serial_number || 'N/A'}</small></li>`).join("") : "<li>No components found</li>"}
+                                    ${device.components ? device.components.map(c => `<li><strong>${escapeHtml(c.category || c.type || 'Component')}</strong>: ${escapeHtml(c.firmware_version || 'N/A')} <br><small style="color:#777">S/N: ${escapeHtml(c.serial_number || 'N/A')}</small></li>`).join("") : "<li>No components found</li>"}
                                </ul>
                            </div>

@@ -1150,14 +1293,14 @@ async function fetchAccountDetails(accountId) {
                                                const account = (s.account && s.account !== s.username && s.account !== name) ? ` [${s.account}]` : "";
                                                const finalName = name || s.type || "Unknown Source";
                                                if (finalName) {
-                                                    sourceLabel = `<br><small style="color: #666; font-size: 0.85em;">via ${finalName}${account}</small>`;
+                                                    sourceLabel = `<br><small style="color: #666; font-size: 0.85em;">via ${escapeHtml(finalName)}${escapeHtml(account)}</small>`;
                                                }
                                            }
                                        }

                                        return `
                                            <div style="border: 1px solid #ddd; padding: 5px; font-size: 0.8em; background: ${p ? "#e6ffed" : "#f8f9fa"}; border-radius: 3px;">
-                                                <strong>#${i + 1}</strong>: ${itemName}${sourceLabel}
+                                                <strong>#${i + 1}</strong>: ${escapeHtml(itemName)}${sourceLabel}
                                            </div>
                                        `;
                                    }).join("")}
@@ -1175,13 +1318,13 @@ async function fetchAccountDetails(accountId) {
                                                const account = (s.account && s.account !== s.username && s.account !== sName) ? ` [${s.account}]` : "";
                                                const finalSName = sName || s.type || "Unknown Source";
                                                if (finalSName) {
-                                                    sourceLabel = `<br><small style="color: #666; font-size: 0.9em;">via ${finalSName}${account}</small>`;
+                                                    sourceLabel = `<br><small style="color: #666; font-size: 0.9em;">via ${escapeHtml(finalSName)}${escapeHtml(account)}</small>`;
                                                }
                                            }
                                            const dateRaw = r.last_played_at || r.created_on;
                                            const dateObj = dateRaw ? (isNaN(Number(dateRaw)) ? new Date(dateRaw) : new Date(Number(dateRaw) * 1000)) : null;
                                            const dateStr = dateObj ? dateObj.toLocaleString('sv-SE') : 'N/A'; // sv-SE produces YYYY-MM-DD HH:MM:SS with 24h time
-                                            return `<li>${name}${sourceLabel} <br><small style="color:#888">${dateStr}</small></li>`;
+                                            return `<li>${escapeHtml(name)}${sourceLabel} <br><small style="color:#888">${escapeHtml(dateStr)}</small></li>`;
                                        }).join("") : "<li>No recents</li>"}
                                    </ul>
                                </div>
@@ -1196,8 +1339,8 @@ async function fetchAccountDetails(accountId) {
                                    const usernameSuffix = (s.username && s.username !== "Local") ? ` (${s.username})` : "";
                                    const accountSuffix = (s.account && s.account !== s.username && s.account !== sourceName) ? ` [${s.account}]` : "";
                                    return `
-                                        <span style="background: #eefbff; color: #0056b3; border: 1px solid #b8daff; padding: 2px 8px; border-radius: 12px; font-size: 0.75em" title="Source Type: ${s.type}">
-                                            ${sourceName}${usernameSuffix}${accountSuffix}
+                                        <span style="background: #eefbff; color: #0056b3; border: 1px solid #b8daff; padding: 2px 8px; border-radius: 12px; font-size: 0.75em" title="Source Type: ${escapeHtml(s.type)}">
+                                            ${escapeHtml(sourceName)}${escapeHtml(usernameSuffix)}${escapeHtml(accountSuffix)}
                                        </span>
                                    `;
                                }).join("") : "<small style='color:#999'>None</small>"}
@@ -1206,10 +1349,17 @@ async function fetchAccountDetails(accountId) {
                    </div>
                </div>
            `).join("");
+
+            // data-toggle-target (not an inline onclick) avoids re-embedding
+            // speaker-controlled device_id inside a JS-string-in-HTML-attribute
+            // context, which HTML-escaping alone cannot make safe.
+            devicesEl.querySelectorAll(".device-summary-header").forEach(el => {
+                el.addEventListener("click", () => toggleInfo(el.dataset.toggleTarget));
+            });
        }

    } catch (error) {
-        if (metadataEl) metadataEl.innerHTML = `<span style="color:red">Error: ${error.message}</span>`;
+        if (metadataEl) metadataEl.innerHTML = `<span style="color:red">Error: ${escapeHtml(error.message)}</span>`;
        console.error("Failed to fetch account details", error);
    }
 }
@@ -1767,7 +1917,7 @@ async function fetchDeviceEvents(deviceId) {
    list.innerHTML = '<tr><td colspan="3" style="padding: 20px; text-align: center; color: #666;">Loading events...</td></tr>';

    try {
-        const response = await fetch(`/api/setup/devices/${deviceId}/events`);
+        const response = await fetch(`/api/setup/devices/${encodeURIComponent(deviceId)}/events`);
        const data = await response.json();
        const events = data.events;

@@ -1907,7 +2057,7 @@ async function removeDevice(deviceId, name) {
    }

    try {
-        const response = await fetch(`/api/setup/devices/${deviceId}`, {
+        const response = await fetch(`/api/setup/devices/${encodeURIComponent(deviceId)}`, {
            method: "DELETE",
        });

@@ -4804,14 +4954,14 @@ async function toggleDeviceSummary(deviceId) {
        const resp = await fetch(`/api/setup/device-summary/${encodeURIComponent(deviceId)}`);
        if (!resp.ok) {
            const txt = await resp.text();
-            cell.innerHTML = `<span style="color:#c62828;">Summary failed: ${resp.status} ${escapeHTML(txt)}</span>`;
+            cell.innerHTML = `<span style="color:#c62828;">Summary failed: ${resp.status} ${escapeHtml(txt)}</span>`;
            return;
        }
        const data = await resp.json();
        cell.innerHTML = "";
        cell.appendChild(renderDeviceSummary(data));
    } catch (e) {
-        cell.innerHTML = `<span style="color:#c62828;">Summary failed: ${escapeHTML(e.message || String(e))}</span>`;
+        cell.innerHTML = `<span style="color:#c62828;">Summary failed: ${escapeHtml(e.message || String(e))}</span>`;
    }
 }

@@ -5016,12 +5166,3 @@ function unreachableBlock(probe) {

    return wrap;
 }
-
-function escapeHTML(s) {
-    return String(s)
-        .replace(/&/g, "&amp;")
-        .replace(/</g, "&lt;")
-        .replace(/>/g, "&gt;")
-        .replace(/"/g, "&quot;")
-        .replace(/'/g, "&#39;");
-}
@@ -215,7 +215,7 @@ func detectOrphanDefaultEntries(ds *datastore.DataStore, paired []models.Service

 			if speakerAccount != info.account {
 				log.Printf("[Health] consistency: speaker %s reports margeAccountUUID=%s but ListAllDevices picked %s — preferring the speaker's answer for orphan-deletion suggestions",
-					deviceID, speakerAccount, info.account)
+					sanitizeLog(deviceID), sanitizeLog(speakerAccount), sanitizeLog(info.account))
 			}
 		}

@@ -289,21 +289,21 @@ func deleteOrphanAccountEntry(ds *datastore.DataStore, target Target) (string, e
 		if speakerAccount := fetchSpeakerMargeAccount(ctx, speakerIP); speakerAccount != "" {
 			if speakerAccount == target.Account {
 				return "", fmt.Errorf("speaker %s reports margeAccountUUID=%s — refusing to delete <data-dir>/accounts/%s/devices/%s because it's the speaker's currently-active binding (re-paired since the consistency check ran?)",
-					target.Device, speakerAccount, target.Account, target.Device)
+					sanitizeLog(target.Device), sanitizeLog(speakerAccount), sanitizeLog(target.Account), sanitizeLog(target.Device))
 			}

 			log.Printf("[Health] deleteOrphanAccountEntry: speaker %s confirmed margeAccountUUID=%s; target account %s is stale, proceeding with delete",
-				target.Device, speakerAccount, target.Account)
+				sanitizeLog(target.Device), sanitizeLog(speakerAccount), sanitizeLog(target.Account))
 		} else {
 			log.Printf("[Health] deleteOrphanAccountEntry: speaker %s at %s not reachable for re-confirmation; relying on operator's Confirm click",
-				target.Device, speakerIP)
+				sanitizeLog(target.Device), sanitizeLog(speakerIP))
 		}
 	} else {
-		log.Printf("[Health] deleteOrphanAccountEntry: no IP recorded for device %s — skipping speaker re-probe", target.Device)
+		log.Printf("[Health] deleteOrphanAccountEntry: no IP recorded for device %s — skipping speaker re-probe", sanitizeLog(target.Device))
 	}

 	if target.Account == accountIDDefaultPlaceholder {
-		log.Printf("[Health] deleteOrphanAccountEntry: deleting the \"default\" placeholder entry for device %s; this is normal after pairing completed", target.Device)
+		log.Printf("[Health] deleteOrphanAccountEntry: deleting the \"default\" placeholder entry for device %s; this is normal after pairing completed", sanitizeLog(target.Device))
 	}

 	path := ds.AccountDeviceDir(target.Account, target.Device)
@@ -316,7 +316,7 @@ func deleteOrphanAccountEntry(ds *datastore.DataStore, target Target) (string, e
 	}

 	log.Printf("[Health] Removed orphan account entry %s (account=%s device=%s) at operator request",
-		path, target.Account, target.Device)
+		path, sanitizeLog(target.Account), sanitizeLog(target.Device))

 	return fmt.Sprintf("Removed stale account entry %s for device %s.", target.Account, target.Device), nil
 }
@@ -479,7 +479,7 @@ func reclassifyCanonicalSourceIDs(ds *datastore.DataStore, target Target) (strin
 	for i := range sources {
 		if newID, ok := rename[sources[i].ID]; ok {
 			log.Printf("[Health] Re-classify %s: id %s → %s (account=%s device=%s)",
-				sources[i].SourceKeyType, sources[i].ID, newID, target.Account, target.Device)
+				sanitizeLog(sources[i].SourceKeyType), sanitizeLog(sources[i].ID), sanitizeLog(newID), sanitizeLog(target.Account), sanitizeLog(target.Device))

 			sources[i].ID = newID

@@ -30,9 +30,12 @@ func suggestAccountForPairing(ds *datastore.DataStore, deviceID string) string {
 	return ""
 }

-// isSevenDigitAccountID mirrors setup.IsValidAccountID without
-// importing the setup package (which would pull in SSH/telnet/certmgr
-// transitively — see the boundary comment near speakerInfoXML).
+// isSevenDigitAccountID is intentionally narrower than
+// datastore.IsSafeIdentifier: it filters suggestAccountForPairing's
+// candidates down to directories that look like a real Bose-issued
+// account, not merely safe-to-use ones (a device-reported value like
+// "stick@local", #634, is a safe identifier but not something to
+// suggest as a pre-existing "real" account to reuse).
 func isSevenDigitAccountID(s string) bool {
 	if len(s) != 7 {
 		return false
@@ -0,0 +1,13 @@
+package health
+
+import "strings"
+
+// sanitizeLog strips newline characters from s to prevent log-injection
+// (CodeQL go/log-injection). Values from speakers (e.g. margeAccountUUID
+// read live via :8090/info) may contain attacker-controlled newlines.
+func sanitizeLog(s string) string {
+	s = strings.ReplaceAll(s, "\n", `\n`)
+	s = strings.ReplaceAll(s, "\r", `\r`)
+
+	return s
+}
@@ -0,0 +1,88 @@
+package marge
+
+import (
+	"fmt"
+	"os"
+	"sync"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+// TestConcurrentUpdatePresetNoLostUpdates is a regression test for #614's
+// 2026-08-23 reproduction: a reporter's script stored six presets via rapid,
+// overlapping PUT .../preset/N requests (visible in the speaker's own log as
+// interleaved connection IDs, never waiting for one PUT to complete before
+// firing the next). One preset silently vanished from Presets.xml.
+//
+// UpdatePreset used to do GetPresets, mutate one slot, SavePresets as three
+// separate steps with no lock spanning them — a classic lost-update race:
+// two concurrent calls can each read the same starting list, mutate
+// different slots, and the second writer's SavePresets clobbers the first
+// writer's update. Fixed by routing the write through
+// datastore.MutatePresets, which holds a single write lock for the whole
+// read-mutate-write cycle.
+func TestConcurrentUpdatePresetNoLostUpdates(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "marge-concurrent-update-preset-*")
+	if err != nil {
+		t.Fatalf("tempdir: %v", err)
+	}
+	defer func() { _ = os.RemoveAll(tempDir) }()
+
+	ds := datastore.NewDataStore(tempDir)
+
+	account := "1234567"
+	device := "B0D5CC25479C"
+
+	const presetCount = 6
+
+	var wg sync.WaitGroup
+
+	errs := make([]error, presetCount)
+
+	for i := 1; i <= presetCount; i++ {
+		wg.Add(1)
+
+		go func(presetNumber int) {
+			defer wg.Done()
+
+			// sourceid 10003 is the canonical LOCAL_INTERNET_RADIO built-in
+			// (see CanonicalSourceByID) — same shape as Henri's own repro
+			// script, which stored six LOCAL_INTERNET_RADIO presets.
+			putXML := []byte(fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?>
+<preset>
+    <name>Station %d</name>
+    <sourceid>10003</sourceid>
+    <location>/custom/v1/playback/station%d</location>
+    <contentItemType>stationurl</contentItemType>
+</preset>`, presetNumber, presetNumber))
+
+			_, err := UpdatePreset(ds, account, device, presetNumber, putXML)
+			errs[presetNumber-1] = err
+		}(i)
+	}
+
+	wg.Wait()
+
+	for i, err := range errs {
+		if err != nil {
+			t.Fatalf("UpdatePreset(preset=%d) returned error: %v", i+1, err)
+		}
+	}
+
+	presets, err := ds.GetPresets(account, device)
+	if err != nil {
+		t.Fatalf("GetPresets: %v", err)
+	}
+
+	if len(presets) != presetCount {
+		t.Fatalf("expected %d presets after %d concurrent UpdatePreset calls, got %d: %+v", presetCount, presetCount, len(presets), presets)
+	}
+
+	for i, p := range presets {
+		want := fmt.Sprintf("Station %d", i+1)
+		if p.Name != want {
+			t.Errorf("preset slot %d: expected name %q, got %q — a concurrent update was lost", i+1, want, p.Name)
+		}
+	}
+}
@@ -752,6 +752,13 @@ func CreateAccountDevice(ds *datastore.DataStore, account, deviceID string) (mod
 	device.Presets = mapPresetsToFullResponse(presets, sources)
 	device.Recents = mapRecentsToFullResponse(recents, sources)

+	if len(device.Presets) != len(presets) {
+		log.Printf("[Marge] /full: device %s — read %d preset(s) from disk, embedding %d after source mapping",
+			sanitizeLog(deviceID), len(presets), len(device.Presets))
+	} else {
+		log.Printf("[Marge] /full: device %s — embedding %d preset(s)", sanitizeLog(deviceID), len(device.Presets))
+	}
+
 	return device, nil
 }

@@ -1501,19 +1508,18 @@ func AccountFullToXML(ds *datastore.DataStore, account string) ([]byte, error) {

 // RemovePreset clears a preset for the specified account and device.
 func RemovePreset(ds *datastore.DataStore, account, device string, presetNumber int) error {
-	presets, err := ds.GetPresets(account, device)
-	if err != nil {
-		return err
-	}
+	_, err := ds.MutatePresets(account, device, func(presets []models.ServicePreset) ([]models.ServicePreset, error) {
+		if presetNumber < 1 || presetNumber > len(presets) {
+			// Preset doesn't exist or index out of range, nothing to do
+			return presets, nil
+		}

-	if presetNumber < 1 || presetNumber > len(presets) {
-		// Preset doesn't exist or index out of range, nothing to do
-		return nil
-	}
+		presets[presetNumber-1] = models.ServicePreset{}

-	presets[presetNumber-1] = models.ServicePreset{}
+		return presets, nil
+	})

-	return ds.SavePresets(account, device, presets)
+	return err
 }

 // resolvePresetSource resolves the source a preset PUT is referencing,
@@ -1522,42 +1528,74 @@ func RemovePreset(ds *datastore.DataStore, account, device string, presetNumber
 // returns the matched source plus the possibly-extended sources slice
 // (since auto-add appends). Returns (nil, sources) when no match could be
 // resolved — UpdatePreset turns that into a 500 with a diagnostic log line.
-func resolvePresetSource(ds *datastore.DataStore, account, device string, sources []models.ConfiguredSource, sourceID string, presetNumber int) (*models.ConfiguredSource, []models.ConfiguredSource) {
+// findConfiguredSource looks up sourceID in sources, first by exact ID and
+// then — since the speaker sometimes sends the symbolic provider name (e.g.
+// <sourceid>TUNEIN</sourceid>) instead of a numeric ID — by SourceKeyType
+// for the handful of providers known to do that.
+func findConfiguredSource(sources []models.ConfiguredSource, sourceID string) *models.ConfiguredSource {
 	for i := range sources {
 		if sources[i].ID == sourceID {
-			return &sources[i], sources
+			return &sources[i]
 		}
 	}

-	// Fallback: SourceID is the symbolic provider name (the speaker
-	// sometimes sends e.g. <sourceid>TUNEIN</sourceid> instead of a
-	// numeric ID); match by SourceKeyType.
 	if sourceID == constants.ProviderInternetRadio || sourceID == constants.ProviderTunein || sourceID == constants.ProviderSpotify || sourceID == constants.ProviderAmazon {
 		for i := range sources {
 			if sources[i].SourceKeyType == sourceID {
-				return &sources[i], sources
+				return &sources[i]
 			}
 		}
 	}

+	return nil
+}
+
+func resolvePresetSource(ds *datastore.DataStore, account, device string, sources []models.ConfiguredSource, sourceID string, presetNumber int) (*models.ConfiguredSource, []models.ConfiguredSource) {
+	if src := findConfiguredSource(sources, sourceID); src != nil {
+		return src, sources
+	}
+
 	// Auto-add a canonical built-in source the speaker referenced but
 	// AfterTouch hasn't been told about (post-factory-reset state). For
 	// account-bound sources (Spotify, Amazon) we can't synthesise
 	// credentials, so the caller will reject the PUT instead.
-	if canonical, ok := ds.CanonicalSourceByID(sourceID); ok {
-		log.Printf("[Marge] UpdatePreset(preset=%d): auto-adding canonical source id=%s type=%s providerid=%s — speaker referenced a built-in source not yet in AfterTouch's configured-sources list; saving so the preset can land",
-			presetNumber, sanitizeLog(canonical.ID), sanitizeLog(canonical.SourceKeyType), sanitizeLog(canonical.SourceProviderID))
+	canonical, ok := ds.CanonicalSourceByID(sourceID)
+	if !ok {
+		return nil, sources
+	}
+
+	log.Printf("[Marge] UpdatePreset(preset=%d): auto-adding canonical source id=%s type=%s providerid=%s — speaker referenced a built-in source not yet in AfterTouch's configured-sources list; saving so the preset can land",
+		presetNumber, sanitizeLog(canonical.ID), sanitizeLog(canonical.SourceKeyType), sanitizeLog(canonical.SourceProviderID))
+
+	// Read-mutate-write atomically against the persisted list, not the
+	// possibly-stale `sources` snapshot the caller already read — a
+	// concurrent PUT for a different preset could be auto-adding (or have
+	// just added) a source at the same time, and a plain Get+Save here
+	// would silently lose whichever write landed second.
+	updated, saveErr := ds.MutateConfiguredSources(account, device, func(current []models.ConfiguredSource) ([]models.ConfiguredSource, error) {
+		if src := findConfiguredSource(current, sourceID); src != nil {
+			// Another concurrent caller already added it; nothing to do.
+			return current, nil
+		}
+
+		return append(current, canonical), nil
+	})
+	if saveErr != nil {
+		log.Printf("[Marge] UpdatePreset(preset=%d): SaveConfiguredSources after auto-add failed: %s — the preset will land but the source may not survive a service restart",
+			presetNumber, sanitizeErr(saveErr))

 		sources = append(sources, canonical)
-		if saveErr := ds.SaveConfiguredSources(account, device, sources); saveErr != nil {
-			log.Printf("[Marge] UpdatePreset(preset=%d): SaveConfiguredSources after auto-add failed: %s — the preset will land but the source may not survive a service restart",
-				presetNumber, sanitizeErr(saveErr))
-		}

 		return &sources[len(sources)-1], sources
 	}

-	return nil, sources
+	if src := findConfiguredSource(updated, sourceID); src != nil {
+		return src, updated
+	}
+
+	updated = append(updated, canonical)
+
+	return &updated[len(updated)-1], updated
 }

 // UpdatePreset updates or creates a preset for the specified account and device.
@@ -1567,11 +1605,6 @@ func UpdatePreset(ds *datastore.DataStore, account, device string, presetNumber
 		return nil, err
 	}

-	presets, err := ds.GetPresets(account, device)
-	if err != nil {
-		presets = []models.ServicePreset{}
-	}
-
 	var newPresetElem struct {
 		Name            string `xml:"name"`
 		Username        string `xml:"username"`
@@ -1637,14 +1670,19 @@ func UpdatePreset(ds *datastore.DataStore, account, device string, presetNumber
 		Username:     newPresetElem.Name,
 	}

-	// Ensure presets list is large enough
-	for len(presets) < presetNumber {
-		presets = append(presets, models.ServicePreset{})
-	}
+	// Read-mutate-write atomically: a concurrent PUT for a different preset
+	// number racing this one must not be able to clobber it. See
+	// MutatePresets — this is the exact interleave that dropped a preset
+	// during #614's rapid-fire repro.
+	if _, err = ds.MutatePresets(account, device, func(presets []models.ServicePreset) ([]models.ServicePreset, error) {
+		for len(presets) < presetNumber {
+			presets = append(presets, models.ServicePreset{})
+		}

-	presets[presetNumber-1] = presetObj
+		presets[presetNumber-1] = presetObj

-	if err = ds.SavePresets(account, device, presets); err != nil {
+		return presets, nil
+	}); err != nil {
 		return nil, err
 	}

@@ -1773,11 +1811,6 @@ func AddRecent(ds *datastore.DataStore, account, device string, sourceXML []byte
 		return nil, err
 	}

-	recents, err := ds.GetRecents(account, device)
-	if err != nil && !os.IsNotExist(err) {
-		return nil, err
-	}
-
 	var input recentInput
 	if err := xml.Unmarshal(sourceXML, &input); err != nil {
 		return nil, err
@@ -1822,9 +1855,20 @@ func AddRecent(ds *datastore.DataStore, account, device string, sourceXML []byte
 	syncMatchingSource(matchingSrc, input)

 	utcTime := parseLastPlayedAt(input.LastPlayedAt)
-	recentObj, recents := updateOrCreateRecent(recents, input.Name, matchingSrc, input.ContentItemType, input.Location, device, utcTime)

-	if err := ds.SaveRecents(account, device, recents); err != nil {
+	// Read-mutate-write atomically: a concurrent AddRecent/preset call for
+	// the same device racing this one must not be able to clobber it. See
+	// MutatePresets/MutateRecents for why a plain GetRecents+SaveRecents
+	// isn't safe here.
+	var recentObj *models.ServiceRecent
+
+	if _, err := ds.MutateRecents(account, device, func(recents []models.ServiceRecent) ([]models.ServiceRecent, error) {
+		var updated []models.ServiceRecent
+
+		recentObj, updated = updateOrCreateRecent(recents, input.Name, matchingSrc, input.ContentItemType, input.Location, device, utcTime)
+
+		return updated, nil
+	}); err != nil {
 		return nil, err
 	}

@@ -1852,7 +1896,7 @@ func learnSource(ds *datastore.DataStore, account, device string, sources []mode
 			matchingSrc.SecretType = constants.CredentialTypeToken
 		}

-		persistLearnedSource(ds, account, device, sources, matchingSrc)
+		persistLearnedSource(ds, account, device, matchingSrc)
 	}

 	return matchingSrc, sourceLearned
@@ -2002,26 +2046,24 @@ func updateSourceFields(src *models.ConfiguredSource, credentialValue, sourceNam
 	return learned
 }

-func persistLearnedSource(ds *datastore.DataStore, account, device string, sources []models.ConfiguredSource, matchingSrc *models.ConfiguredSource) {
-	updatedSources := make([]models.ConfiguredSource, len(sources))
-	copy(updatedSources, sources)
+func persistLearnedSource(ds *datastore.DataStore, account, device string, matchingSrc *models.ConfiguredSource) {
+	// Read-mutate-write atomically against the persisted list, not a
+	// snapshot the caller read earlier — AddRecent and UpdatePreset can
+	// both be learning/auto-adding sources for the same device
+	// concurrently, and a plain Get+Save here would silently lose
+	// whichever write landed second.
+	_, err := ds.MutateConfiguredSources(account, device, func(sources []models.ConfiguredSource) ([]models.ConfiguredSource, error) {
+		for i := range sources {
+			if sources[i].ID == matchingSrc.ID {
+				sources[i] = *matchingSrc

-	found := false
-
-	for i := range updatedSources {
-		if updatedSources[i].ID == matchingSrc.ID {
-			updatedSources[i] = *matchingSrc
-			found = true
-
-			break
+				return sources, nil
+			}
 		}
-	}

-	if !found {
-		updatedSources = append(updatedSources, *matchingSrc)
-	}
-
-	if err := ds.SaveConfiguredSources(account, device, updatedSources); err != nil {
+		return append(sources, *matchingSrc), nil
+	})
+	if err != nil {
 		log.Printf("[MARGE_ERR] Failed to persist learned source for %s: %s", sanitizeLog(device), sanitizeErr(err))
 	}
 }
@@ -2407,7 +2449,6 @@ func AddSource(ds *datastore.DataStore, account, username, providerID, secret, s
 		}

 		devID := entry.Name()
-		sources, _ := ds.GetConfiguredSources(account, devID)

 		newSrc := models.ConfiguredSource{
 			ID:               sourceID,
@@ -2437,37 +2478,39 @@ func AddSource(ds *datastore.DataStore, account, username, providerID, secret, s

 		PrepareConfiguredSource(&newSrc)

-		// Update or append. Most providers are singletons (one account each), so
-		// the same provider replaces the existing entry. STORED_MUSIC is the
-		// exception: each DLNA media server is a separate account (username =
-		// "<UDN>/0"), so it must only replace when the account also matches.
-		// Otherwise registering a second media server overwrites the first, which
-		// then vanishes from /full + /sources and the speaker drops it (only one
-		// media server could ever stay registered).
-		replaced := false
+		// Read-mutate-write atomically against the persisted list, not a
+		// snapshot read before the loop body — see MutateConfiguredSources.
+		_, err := ds.MutateConfiguredSources(account, devID, func(sources []models.ConfiguredSource) ([]models.ConfiguredSource, error) {
+			// Update or append. Most providers are singletons (one account
+			// each), so the same provider replaces the existing entry.
+			// STORED_MUSIC is the exception: each DLNA media server is a
+			// separate account (username = "<UDN>/0"), so it must only
+			// replace when the account also matches. Otherwise registering
+			// a second media server overwrites the first, which then
+			// vanishes from /full + /sources and the speaker drops it
+			// (only one media server could ever stay registered).
+			for i := range sources {
+				sameProvider := sources[i].SourceProviderID == providerID
+				if providerID == strconv.Itoa(constants.StoredMusicProviderID) {
+					// Match on the persisted account identity
+					// (SourceKey.Account), not Username, which does not
+					// round-trip through the datastore.
+					sameProvider = sameProvider && sources[i].SourceKey.Account == username
+				}

-		for i := range sources {
-			sameProvider := sources[i].SourceProviderID == providerID
-			if providerID == strconv.Itoa(constants.StoredMusicProviderID) {
-				// Match on the persisted account identity (SourceKey.Account),
-				// not Username, which does not round-trip through the datastore.
-				sameProvider = sameProvider && sources[i].SourceKey.Account == username
+				if sameProvider ||
+					(providerID == strconv.Itoa(constants.SpotifyProviderID) && sources[i].SourceKey.Type == constants.ProviderSpotify) {
+					sources[i] = newSrc
+
+					return sources, nil
+				}
 			}

-			if sameProvider ||
-				(providerID == strconv.Itoa(constants.SpotifyProviderID) && sources[i].SourceKey.Type == constants.ProviderSpotify) {
-				sources[i] = newSrc
-				replaced = true
-
-				break
-			}
+			return append(sources, newSrc), nil
+		})
+		if err != nil {
+			log.Printf("[Marge] AddSource: failed to save source %s for device %s: %s", sanitizeLog(newSrc.SourceKey.Type), sanitizeLog(devID), sanitizeErr(err))
 		}
-
-		if !replaced {
-			sources = append(sources, newSrc)
-		}
-
-		_ = ds.SaveConfiguredSources(account, devID, sources)
 	}

 	return sourceID, nil
@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"time"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 )

 // InitPlan describes everything required to take a factory-reset (or
@@ -250,8 +252,8 @@ func (m *Manager) runURLRewrite(plan InitPlan, emit func(StepKind, string, StepS
 // ID, or validating a user-supplied value.
 func (m *Manager) resolveAccountID(plan InitPlan, info *DeviceInfoXML, emit func(StepKind, string, StepStatus, error)) (InitPlan, error) {
 	if plan.AccountID != "" {
-		if !IsValidAccountID(plan.AccountID) {
-			invalidErr := fmt.Errorf("invalid AccountID %q: must be exactly 7 digits", plan.AccountID)
+		if !datastore.IsSafeIdentifier(plan.AccountID) {
+			invalidErr := fmt.Errorf("invalid AccountID %q: must be a non-empty, path-safe identifier", plan.AccountID)
 			emit(StepGenerateAccountID, "validate account ID", StatusFailed, invalidErr)

 			return plan, invalidErr
@@ -260,7 +262,7 @@ func (m *Manager) resolveAccountID(plan InitPlan, info *DeviceInfoXML, emit func
 		return plan, nil
 	}

-	if info.MargeAccountUUID != "" && IsValidAccountID(info.MargeAccountUUID) {
+	if info.MargeAccountUUID != "" && datastore.IsSafeIdentifier(info.MargeAccountUUID) {
 		plan.AccountID = info.MargeAccountUUID
 		emit(StepGenerateAccountID, "reuse existing margeAccountUUID="+plan.AccountID, StatusOK, nil)

@@ -9,6 +9,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 )

 // fakeSession is a StateMachine that records the order of
@@ -195,11 +197,13 @@ func TestExecuteInitPlan_ReusesExistingAccountUUID(t *testing.T) {
 }

 func TestExecuteInitPlan_GeneratesAccountWhenDeviceUUIDInvalid(t *testing.T) {
-	// Devices that report a non-7-digit UUID (e.g. a stale local value) must
-	// not be reused — we treat them as factory-reset for ID purposes.
+	// Devices that report an unsafe/malformed UUID (e.g. containing a path
+	// separator) must not be reused — we treat them as factory-reset for ID
+	// purposes. A merely non-numeric UUID (e.g. "stick@local", #634) IS
+	// reused now; see resolveAccountID/datastore.IsSafeIdentifier.
 	info := &fakeInfoResponder{
 		deviceID:       "AABBCCDDEEFF",
-		paired:         "not-7-digits",
+		paired:         "not/valid",
 		postInitPaired: "", // we'll learn the generated ID from the result
 	}
 	sess := &fakeSession{}
@@ -220,11 +224,11 @@ func TestExecuteInitPlan_GeneratesAccountWhenDeviceUUIDInvalid(t *testing.T) {
 		t.Fatalf("ExecuteInitPlan: %v", err)
 	}

-	if !IsValidAccountID(got.AccountID) {
-		t.Errorf("got.AccountID = %q, want a valid 7-digit ID", got.AccountID)
+	if !datastore.IsSafeIdentifier(got.AccountID) {
+		t.Errorf("got.AccountID = %q, want a valid generated ID", got.AccountID)
 	}

-	if got.AccountID == "not-7-digits" {
+	if got.AccountID == "not/valid" {
 		t.Error("orchestrator should not reuse an invalid UUID")
 	}
 }
@@ -236,7 +240,7 @@ func TestExecuteInitPlan_RejectsInvalidSuppliedAccountID(t *testing.T) {

 	plan := InitPlan{
 		DeviceIP:       "192.0.2.10",
-		AccountID:      "abc",
+		AccountID:      "abc/def",
 		SkipURLRewrite: true,
 	}

@@ -123,7 +123,7 @@ func TestIssue234_FactoryResetSpeakerSyncsReducedSources(t *testing.T) {
 	// SyncDeviceData derives accountID/deviceID from /info; with
 	// an empty margeAccountUUID the account falls through to
 	// "default".
-	if err := m.SyncDeviceData(deviceIP); err != nil {
+	if _, err := m.SyncDeviceData(deviceIP, false); err != nil {
 		t.Fatalf("SyncDeviceData: %v", err)
 	}

@@ -1,6 +1,7 @@
 package setup

 import (
+	"bytes"
 	"crypto/rand"
 	"encoding/xml"
 	"errors"
@@ -11,6 +12,8 @@ import (
 	"net/http"
 	"strings"
 	"time"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 )

 // PairAccountTimeouts bounds every step of the pairing call so a wedged
@@ -44,8 +47,8 @@ func (m *Manager) PairAccount(deviceIP, accountID string, t TelnetClient) (PairA
 		logs   strings.Builder
 	)

-	if !IsValidAccountID(accountID) {
-		return result, "", fmt.Errorf("invalid account ID %q: must be exactly 7 digits", accountID)
+	if !datastore.IsSafeIdentifier(accountID) {
+		return result, "", fmt.Errorf("invalid account ID %q: must be a non-empty, path-safe identifier", accountID)
 	}

 	supported, supportedErr := m.probeSetMargeAccount(deviceIP)
@@ -84,6 +87,9 @@ func (m *Manager) PairAccount(deviceIP, accountID string, t TelnetClient) (PairA

 	result.TelnetAttempted = true

+	// Safe to concatenate: datastore.IsSafeIdentifier (checked above) rejects
+	// any whitespace or control characters, so accountID can't smuggle extra
+	// tokens into this single-line telnet command.
 	cmd := "envswitch accountid set " + accountID

 	resp, err := t.SendCommand(cmd)
@@ -135,8 +141,8 @@ func (m *Manager) EnsureMargeAccountPaired(deviceIP, wantAccountID string, t Tel
 		}

 		target = generated
-	} else if !IsValidAccountID(target) {
-		return "", false, "", fmt.Errorf("invalid account id %q: must be exactly 7 digits", target)
+	} else if !datastore.IsSafeIdentifier(target) {
+		return "", false, "", fmt.Errorf("invalid account id %q: must be a non-empty, path-safe identifier", target)
 	}

 	_, pairLogs, pairErr := m.PairAccount(deviceIP, target, t)
@@ -196,9 +202,18 @@ func (m *Manager) probeSetMargeAccount(deviceIP string) (bool, error) {
 func (m *Manager) postSetMargeAccount(deviceIP, accountID string) error {
 	url := buildDeviceURL(deviceIP, "/setMargeAccount")

+	// accountID is XML-escaped rather than interpolated raw:
+	// datastore.IsSafeIdentifier already excludes '<', '>', '&', '\'', '"'
+	// (see #634), but escaping here too means this stays well-formed even
+	// if that gate is ever bypassed.
+	var escapedAccountID bytes.Buffer
+	if err := xml.EscapeText(&escapedAccountID, []byte(accountID)); err != nil {
+		return fmt.Errorf("escape account ID: %w", err)
+	}
+
 	body := fmt.Sprintf(
 		`<PairDeviceWithAccount><accountId>%s</accountId><userAuthToken>aftertouch</userAuthToken></PairDeviceWithAccount>`,
-		accountID,
+		escapedAccountID.String(),
 	)

 	client := &http.Client{
@@ -312,23 +327,6 @@ func buildDeviceURL(deviceIP, path string) string {
 	return "http://" + deviceIP + ":8090" + path
 }

-// IsValidAccountID reports whether s is a syntactically valid SoundTouch
-// account ID — exactly 7 numeric digits, the format used by every
-// Bose-cloud-issued ID we have observed in captures.
-func IsValidAccountID(s string) bool {
-	if len(s) != 7 {
-		return false
-	}
-
-	for _, ch := range s {
-		if ch < '0' || ch > '9' {
-			return false
-		}
-	}
-
-	return true
-}
-
 // GenerateAccountID returns a fresh 7-digit account ID that does not collide
 // with any value in known. It uses crypto/rand and re-rolls on collision.
 func GenerateAccountID(known []string) (string, error) {
@@ -10,6 +10,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
 )

 // fakeDevice spins up an httptest.Server that pretends to be the SoundTouch
@@ -313,7 +315,7 @@ func TestEnsureMargeAccountPaired_UnpairedGeneratesAndPairs(t *testing.T) {
 		t.Error("alreadyPaired should be false for an unpaired device")
 	}

-	if !IsValidAccountID(accountID) {
+	if !datastore.IsSafeIdentifier(accountID) {
 		t.Errorf("accountID %q is not a valid generated ID", accountID)
 	}

@@ -352,7 +354,7 @@ func TestEnsureMargeAccountPaired_RejectsInvalidWantAccountID(t *testing.T) {

 	m := NewManager("", nil, nil)

-	_, _, _, err := m.EnsureMargeAccountPaired(d.addr, "not-7-digits", nil)
+	_, _, _, err := m.EnsureMargeAccountPaired(d.addr, "not/valid", nil)
 	if err == nil {
 		t.Fatal("expected an error for an invalid --account value")
 	}
@@ -475,28 +477,9 @@ func TestPreflightInitPlan_UnrecognisedStatusFailsClosed(t *testing.T) {
 	}
 }

-func TestIsValidAccountID(t *testing.T) {
-	cases := []struct {
-		in   string
-		want bool
-	}{
-		{"1234567", true},
-		{"0000000", true},
-		{"9999999", true},
-		{"", false},
-		{"123456", false},
-		{"12345678", false},
-		{"123456a", false},
-		{"-123456", false},
-		{" 123456", false},
-	}
-
-	for _, tc := range cases {
-		if got := IsValidAccountID(tc.in); got != tc.want {
-			t.Errorf("IsValidAccountID(%q) = %v, want %v", tc.in, got, tc.want)
-		}
-	}
-}
+// Account-ID format validation is now solely datastore.IsSafeIdentifier's
+// responsibility (see datastore.TestIsSafeIdentifier); setup no longer has
+// its own account-ID validator to test.

 func TestGenerateAccountID_AvoidsCollisions(t *testing.T) {
 	id, err := GenerateAccountID(nil)
@@ -504,7 +487,7 @@ func TestGenerateAccountID_AvoidsCollisions(t *testing.T) {
 		t.Fatalf("GenerateAccountID(nil): %v", err)
 	}

-	if !IsValidAccountID(id) {
+	if !datastore.IsSafeIdentifier(id) {
 		t.Errorf("generated ID %q is not valid", id)
 	}

@@ -2635,12 +2635,51 @@ func (m *Manager) resolveIP(host string, client SSHClient) (string, error) {
 		ErrResolvedFromServiceOnly, host, resolved)
 }

-// SyncDeviceData fetches presets, recents and sources from the device and saves them to the datastore.
-func (m *Manager) SyncDeviceData(deviceIP string) error {
+// SyncResourceDiff describes what a Data Sync would change for one
+// datastore resource (presets or recents): what's currently stored versus
+// what the speaker's own live :8090 API returned just now.
+type SyncResourceDiff struct {
+	Resource      string   `json:"resource"`
+	CurrentCount  int      `json:"currentCount"`
+	IncomingCount int      `json:"incomingCount"`
+	Removed       []string `json:"removed,omitempty"`
+	Destructive   bool     `json:"destructive"`
+}
+
+// SyncResult is the outcome of a SyncDeviceData call: whether it actually
+// wrote anything, and the per-resource diff that led to that decision.
+type SyncResult struct {
+	Applied     bool               `json:"applied"`
+	Destructive bool               `json:"destructive"`
+	Diffs       []SyncResourceDiff `json:"diffs"`
+	// SourcesCount is the number of configured sources saved for this
+	// device, or -1 if the sources fetch failed. Sources are synced
+	// unconditionally (see syncSources) — there's no diff/confirm gate for
+	// them — so this is a plain count rather than a SyncResourceDiff.
+	SourcesCount int `json:"sourcesCount"`
+}
+
+// SyncDeviceData fetches presets, recents and sources from the device and
+// saves them to the datastore.
+//
+// Presets and recents are fetched live from the speaker's own :8090 API and
+// would previously overwrite the datastore unconditionally — including with
+// an empty or shrunk list if the speaker's own local cache happened to be
+// stale or incomplete at that exact moment (e.g. right after a burst of
+// preset writes, or shortly after a reboot before the speaker has resynced
+// with Marge). That's a real, confirmed mechanism for #614's "Sync wipes my
+// presets" reports. Now: if applying would shrink either list relative to
+// what's already stored, SyncDeviceData does NOT write — it reports the
+// diff instead — unless confirmed is true. There is no cached "preview"
+// state: every call (confirmed or not) re-fetches live from the speaker at
+// that moment, so confirming re-checks reality rather than replaying a
+// possibly-stale earlier snapshot. Sources are left unconditional, as
+// before — a source-list change is comparatively low-risk and self-healing.
+func (m *Manager) SyncDeviceData(deviceIP string, confirmed bool) (SyncResult, error) {
 	// 1. Fetch info to get Serial Number (account identifier)
 	info, err := m.GetLiveDeviceInfo(deviceIP)
 	if err != nil {
-		return fmt.Errorf("failed to get device info: %w", err)
+		return SyncResult{}, fmt.Errorf("failed to get device info: %w", err)
 	}

 	log.Printf("Starting sync for device at %s: Name='%s', DeviceID='%s', SerialNumber='%s'",
@@ -2652,7 +2691,7 @@ func (m *Manager) SyncDeviceData(deviceIP string) error {
 	deviceID := info.DeviceID
 	if deviceID == "" {
 		log.Printf("No deviceID found in /info response for device '%s' at %s", sanitizeLog(info.Name), sanitizeLog(deviceIP))
-		return fmt.Errorf("no deviceID found in /info response for device at %s - cannot sync without canonical device identifier", deviceIP)
+		return SyncResult{}, fmt.Errorf("no deviceID found in /info response for device at %s - cannot sync without canonical device identifier", deviceIP)
 	}

 	log.Printf("Using deviceID '%s' for sync operations (MAC address from /info)", sanitizeLog(deviceID))
@@ -2676,14 +2715,42 @@ func (m *Manager) SyncDeviceData(deviceIP string) error {
 		accountID = "default"
 	}

-	// 2. Fetch Presets from :8090
-	m.syncPresets(deviceIP, accountID, deviceID)
+	// 2. Diff presets and recents against a fresh live fetch, before writing
+	// anything.
+	presetDiff, incomingPresets, presetErr := m.presetSyncDiff(deviceIP, accountID, deviceID)
+	if presetErr != nil {
+		log.Printf("[SYNC_ERR] Failed to fetch presets for %s: %v", sanitizeLog(deviceIP), presetErr)
+	}

-	// 3. Fetch Recents from :8090
-	m.syncRecents(deviceIP, accountID, deviceID)
+	recentDiff, incomingRecents, recentErr := m.recentSyncDiff(deviceIP, accountID, deviceID)
+	if recentErr != nil {
+		log.Printf("[SYNC_ERR] Failed to fetch recents for %s: %v", sanitizeLog(deviceIP), recentErr)
+	}
+
+	result := SyncResult{
+		Diffs:       []SyncResourceDiff{presetDiff, recentDiff},
+		Destructive: presetDiff.Destructive || recentDiff.Destructive,
+	}
+
+	if result.Destructive && !confirmed {
+		log.Printf("[SYNC] Sync for %s would shrink stored data (presets %d->%d, recents %d->%d) — awaiting confirmation, not writing anything",
+			sanitizeLog(deviceIP), presetDiff.CurrentCount, presetDiff.IncomingCount, recentDiff.CurrentCount, recentDiff.IncomingCount)
+
+		return result, nil
+	}
+
+	// 3. Apply presets/recents (skip whichever one failed to fetch, leaving
+	// the existing stored data untouched rather than wiping it).
+	if presetErr == nil {
+		_ = m.DataStore.SavePresets(accountID, deviceID, incomingPresets)
+	}
+
+	if recentErr == nil {
+		_ = m.DataStore.SaveRecents(accountID, deviceID, incomingRecents)
+	}

 	// 4. Fetch Sources
-	m.syncSources(deviceIP, accountID, deviceID)
+	result.SourcesCount = m.syncSources(deviceIP, accountID, deviceID)

 	// 5. Nudge the device to re-render its source list. After a factory
 	// reset (issue #234) the speaker's /sources only lists the always-on
@@ -2697,28 +2764,113 @@ func (m *Manager) SyncDeviceData(deviceIP string) error {
 	// 6. Create off-device backup of system configuration
 	_ = m.BackupConfigOffDevice(deviceIP)

-	return nil
+	result.Applied = true
+
+	return result, nil
 }

-func (m *Manager) syncPresets(deviceIP, accountID, deviceID string) {
+// presetSyncDiff fetches the live preset list from the speaker and compares
+// it against what's currently stored, without writing anything.
+func (m *Manager) presetSyncDiff(deviceIP, accountID, deviceID string) (SyncResourceDiff, []models.ServicePreset, error) {
+	current, _ := m.DataStore.GetPresets(accountID, deviceID)
+
+	incoming, err := m.fetchLivePresets(deviceIP)
+	if err != nil {
+		return SyncResourceDiff{Resource: "presets", CurrentCount: len(current), IncomingCount: len(current)}, nil, err
+	}
+
+	return diffPresets(current, incoming), incoming, nil
+}
+
+// recentSyncDiff fetches the live recents list from the speaker and
+// compares it against what's currently stored, without writing anything.
+func (m *Manager) recentSyncDiff(deviceIP, accountID, deviceID string) (SyncResourceDiff, []models.ServiceRecent, error) {
+	current, _ := m.DataStore.GetRecents(accountID, deviceID)
+
+	incoming, err := m.fetchLiveRecents(deviceIP)
+	if err != nil {
+		return SyncResourceDiff{Resource: "recents", CurrentCount: len(current), IncomingCount: len(current)}, nil, err
+	}
+
+	return diffRecents(current, incoming), incoming, nil
+}
+
+// diffPresets compares a stored preset list against a freshly-fetched one.
+// Removed lists the names of presets present in current but absent (by
+// button/slot ID) from incoming — this is what tells an operator "Sync
+// would remove preset 6: Ici Roussillon" instead of just a bare count.
+func diffPresets(current, incoming []models.ServicePreset) SyncResourceDiff {
+	incomingIDs := make(map[string]bool, len(incoming))
+	for i := range incoming {
+		if incoming[i].ID != "" {
+			incomingIDs[incoming[i].ID] = true
+		}
+	}
+
+	var removed []string
+
+	for i := range current {
+		if current[i].ID != "" && current[i].Name != "" && !incomingIDs[current[i].ID] {
+			removed = append(removed, current[i].Name)
+		}
+	}
+
+	return SyncResourceDiff{
+		Resource:      "presets",
+		CurrentCount:  len(current),
+		IncomingCount: len(incoming),
+		Removed:       removed,
+		Destructive:   len(incoming) < len(current),
+	}
+}
+
+// diffRecents compares a stored recents list against a freshly-fetched one.
+// Recents have no stable per-entry ID the way presets do (they're an
+// ordered, time-sorted, size-capped list), so entries are matched by
+// content Location instead.
+func diffRecents(current, incoming []models.ServiceRecent) SyncResourceDiff {
+	incomingLocations := make(map[string]bool, len(incoming))
+	for i := range incoming {
+		if incoming[i].Location != "" {
+			incomingLocations[incoming[i].Location] = true
+		}
+	}
+
+	var removed []string
+
+	for i := range current {
+		if current[i].Location != "" && current[i].Name != "" && !incomingLocations[current[i].Location] {
+			removed = append(removed, current[i].Name)
+		}
+	}
+
+	return SyncResourceDiff{
+		Resource:      "recents",
+		CurrentCount:  len(current),
+		IncomingCount: len(incoming),
+		Removed:       removed,
+		Destructive:   len(incoming) < len(current),
+	}
+}
+
+// fetchLivePresets fetches the current preset list straight from the
+// speaker's own local :8090 API. It does not touch the datastore.
+func (m *Manager) fetchLivePresets(deviceIP string) ([]models.ServicePreset, error) {
 	presetsURL := fmt.Sprintf("http://%s:8090/presets", deviceIP)
 	if _, _, splitErr := net.SplitHostPort(deviceIP); splitErr == nil {
 		presetsURL = fmt.Sprintf("http://%s/presets", deviceIP)
 	}

-	log.Printf("[SYNC] Syncing presets for %s", sanitizeLog(deviceIP))
-
 	resp, err := m.HTTPGet(presetsURL)
 	if err != nil {
-		log.Printf("[SYNC_ERR] Failed to fetch presets for %s: %v", sanitizeLog(deviceIP), err)
-		return
+		return nil, err
 	}

 	defer func() { _ = resp.Body.Close() }()

 	var ps models.Presets
 	if decodeErr := xml.NewDecoder(resp.Body).Decode(&ps); decodeErr != nil {
-		return
+		return nil, decodeErr
 	}

 	var servicePresets []models.ServicePreset
@@ -2762,10 +2914,28 @@ func (m *Manager) syncPresets(deviceIP, accountID, deviceID string) {
 		})
 	}

-	_ = m.DataStore.SavePresets(accountID, deviceID, servicePresets)
+	return servicePresets, nil
 }

-func (m *Manager) syncRecents(deviceIP, accountID, deviceID string) {
+// syncPresets fetches the live preset list and unconditionally persists it.
+// Used directly by tests exercising the raw fetch+save behaviour; the
+// button-driven path goes through SyncDeviceData's diff/confirm guard
+// instead.
+func (m *Manager) syncPresets(deviceIP, accountID, deviceID string) {
+	log.Printf("[SYNC] Syncing presets for %s", sanitizeLog(deviceIP))
+
+	presets, err := m.fetchLivePresets(deviceIP)
+	if err != nil {
+		log.Printf("[SYNC_ERR] Failed to fetch presets for %s: %v", sanitizeLog(deviceIP), err)
+		return
+	}
+
+	_ = m.DataStore.SavePresets(accountID, deviceID, presets)
+}
+
+// fetchLiveRecents fetches the current recents list straight from the
+// speaker's own local :8090 API. It does not touch the datastore.
+func (m *Manager) fetchLiveRecents(deviceIP string) ([]models.ServiceRecent, error) {
 	recentsURL := fmt.Sprintf("http://%s:8090/recents", deviceIP)
 	if _, _, splitErr := net.SplitHostPort(deviceIP); splitErr == nil {
 		recentsURL = fmt.Sprintf("http://%s/recents", deviceIP)
@@ -2773,14 +2943,14 @@ func (m *Manager) syncRecents(deviceIP, accountID, deviceID string) {

 	resp, err := m.HTTPGet(recentsURL)
 	if err != nil {
-		return
+		return nil, err
 	}

 	defer func() { _ = resp.Body.Close() }()

 	var rr models.RecentsResponse
 	if decodeErr := xml.NewDecoder(resp.Body).Decode(&rr); decodeErr != nil {
-		return
+		return nil, decodeErr
 	}

 	var serviceRecents []models.ServiceRecent
@@ -2807,10 +2977,28 @@ func (m *Manager) syncRecents(deviceIP, accountID, deviceID string) {
 		})
 	}

-	_ = m.DataStore.SaveRecents(accountID, deviceID, serviceRecents)
+	return serviceRecents, nil
 }

-func (m *Manager) syncSources(deviceIP, accountID, deviceID string) {
+// syncRecents fetches the live recents list and unconditionally persists
+// it. Used directly by tests exercising the raw fetch+save behaviour; the
+// button-driven path goes through SyncDeviceData's diff/confirm guard
+// instead.
+func (m *Manager) syncRecents(deviceIP, accountID, deviceID string) {
+	recents, err := m.fetchLiveRecents(deviceIP)
+	if err != nil {
+		return
+	}
+
+	_ = m.DataStore.SaveRecents(accountID, deviceID, recents)
+}
+
+// syncSources fetches the device's configured sources (via SSH first, then
+// falling back to :8090/sources) and persists them. It returns the number
+// of sources actually saved, or -1 if neither path produced anything to
+// save (so the caller/UI can distinguish "synced zero sources" from "sync
+// didn't run").
+func (m *Manager) syncSources(deviceIP, accountID, deviceID string) int {
 	client := m.NewSSH(deviceIP)

 	sourcesXML, err := client.Run("cat /mnt/nv/BoseApp-Persistence/1/Sources.xml")
@@ -2835,7 +3023,7 @@ func (m *Manager) syncSources(deviceIP, accountID, deviceID string) {

 			_ = m.DataStore.SaveConfiguredSources(accountID, deviceID, srs.Sources)

-			return
+			return len(srs.Sources)
 		}
 	}

@@ -2847,46 +3035,50 @@ func (m *Manager) syncSources(deviceIP, accountID, deviceID string) {

 	resp, err := m.HTTPGet(sourcesURL)
 	if err != nil {
-		return
+		return -1
 	}

 	defer func() { _ = resp.Body.Close() }()

 	var srs models.Sources
-	if decodeErr := xml.NewDecoder(resp.Body).Decode(&srs); decodeErr == nil {
-		var configuredSources []models.ConfiguredSource
+	if decodeErr := xml.NewDecoder(resp.Body).Decode(&srs); decodeErr != nil {
+		return -1
+	}

-		for _, s := range srs.SourceItem {
-			cs := models.ConfiguredSource{
-				DisplayName: s.DisplayName,
-				Secret:      "",
-				SecretType:  "",
-			}
-			if s.Status == "READY" {
-				cs.SecretType = "token"
-			}
+	var configuredSources []models.ConfiguredSource

-			if s.Source == constants.ProviderSpotify {
-				cs.SecretType = "token_version_3"
-			}
-
-			cs.SourceKey.Type = s.Source
-			cs.SourceKey.Account = s.SourceAccount
-			// Also set legacy fields for now
-			cs.SourceKeyType = s.Source
-			cs.SourceKeyAccount = s.SourceAccount
-
-			configuredSources = append(configuredSources, cs)
+	for _, s := range srs.SourceItem {
+		cs := models.ConfiguredSource{
+			DisplayName: s.DisplayName,
+			Secret:      "",
+			SecretType:  "",
+		}
+		if s.Status == "READY" {
+			cs.SecretType = "token"
 		}

-		// Drop device-local/transient sources without a resolvable
-		// sourceproviderid (e.g. STORED_MUSIC_MEDIA_RENDERER, UPNP).
-		// Persisting them causes /full to emit an empty <sourceproviderid>
-		// which the speaker rejects as INVALID_SOURCE (#334).
-		configuredSources = filterServableSources(configuredSources, deviceID)
+		if s.Source == constants.ProviderSpotify {
+			cs.SecretType = "token_version_3"
+		}

-		_ = m.DataStore.SaveConfiguredSources(accountID, deviceID, configuredSources)
+		cs.SourceKey.Type = s.Source
+		cs.SourceKey.Account = s.SourceAccount
+		// Also set legacy fields for now
+		cs.SourceKeyType = s.Source
+		cs.SourceKeyAccount = s.SourceAccount
+
+		configuredSources = append(configuredSources, cs)
 	}
+
+	// Drop device-local/transient sources without a resolvable
+	// sourceproviderid (e.g. STORED_MUSIC_MEDIA_RENDERER, UPNP).
+	// Persisting them causes /full to emit an empty <sourceproviderid>
+	// which the speaker rejects as INVALID_SOURCE (#334).
+	configuredSources = filterServableSources(configuredSources, deviceID)
+
+	_ = m.DataStore.SaveConfiguredSources(accountID, deviceID, configuredSources)
+
+	return len(configuredSources)
 }

 // filterServableSources returns a copy of srcs containing only sources that
@@ -0,0 +1,142 @@
+package setup
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"testing"
+
+	"github.com/gesellix/bose-soundtouch/pkg/models"
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+// TestSyncDeviceData_DestructiveSyncRequiresConfirmation is a regression
+// test for #614's 2026-08-23 finding: SyncDeviceData used to overwrite the
+// datastore unconditionally with whatever the speaker's live :8090 API
+// returned, even if that snapshot had fewer presets than what was already
+// stored — e.g. because the speaker's own local cache was stale or
+// incomplete at that exact moment. This is a real, confirmed mechanism for
+// "Sync wipes my presets" reports.
+//
+// A device already has 3 stored presets. The mock speaker's live /presets
+// only reports 1. The first (unconfirmed) sync must NOT write anything and
+// must report the shrink; a confirmed retry must apply it.
+func TestSyncDeviceData_DestructiveSyncRequiresConfirmation(t *testing.T) {
+	const (
+		accountID = "1234567"
+		deviceID  = "AABBCCDDEEFF"
+	)
+
+	mockDevice := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/info":
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprintf(w, `<?xml version="1.0" encoding="UTF-8"?>
+<info deviceID="%s">
+    <name>Test Device</name>
+    <type>SoundTouch 20</type>
+    <margeAccountUUID>%s</margeAccountUUID>
+</info>`, deviceID, accountID)
+		case "/presets":
+			// Only one preset survived on the speaker's own live cache —
+			// the datastore already has three (seeded below).
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?>
+<presets>
+    <preset id="1">
+        <ContentItem source="LOCAL_INTERNET_RADIO" type="stationurl" location="/custom/v1/playback/station1" isPresetable="true">
+            <itemName>Station 1</itemName>
+        </ContentItem>
+    </preset>
+</presets>`)
+		case "/recents":
+			w.Header().Set("Content-Type", "application/xml")
+			fmt.Fprint(w, `<?xml version="1.0" encoding="UTF-8"?><recents></recents>`)
+		default:
+			w.WriteHeader(http.StatusNotFound)
+		}
+	}))
+	defer mockDevice.Close()
+
+	tempDir, err := os.MkdirTemp("", "sync-destructive-guard-*")
+	if err != nil {
+		t.Fatalf("tempdir: %v", err)
+	}
+	defer func() { _ = os.RemoveAll(tempDir) }()
+
+	ds := datastore.NewDataStore(tempDir)
+
+	seeded := []models.ServicePreset{
+		{ID: "1", ButtonNumber: "1", ServiceContentItem: models.ServiceContentItem{Name: "Station 1"}},
+		{ID: "2", ButtonNumber: "2", ServiceContentItem: models.ServiceContentItem{Name: "Station 2"}},
+		{ID: "3", ButtonNumber: "3", ServiceContentItem: models.ServiceContentItem{Name: "Station 3"}},
+	}
+	if err := ds.SavePresets(accountID, deviceID, seeded); err != nil {
+		t.Fatalf("seed SavePresets: %v", err)
+	}
+
+	m := NewManager("http://localhost:8000", ds, nil)
+	deviceIP := mockDevice.Listener.Addr().String()
+
+	// Unconfirmed: must refuse to write and report the shrink.
+	result, err := m.SyncDeviceData(deviceIP, false)
+	if err != nil {
+		t.Fatalf("SyncDeviceData(confirmed=false): %v", err)
+	}
+
+	if result.Applied {
+		t.Fatal("expected unconfirmed destructive sync to NOT apply")
+	}
+
+	if !result.Destructive {
+		t.Fatal("expected result.Destructive=true for a 3->1 preset shrink")
+	}
+
+	var presetDiff *SyncResourceDiff
+	for i := range result.Diffs {
+		if result.Diffs[i].Resource == "presets" {
+			presetDiff = &result.Diffs[i]
+		}
+	}
+
+	if presetDiff == nil {
+		t.Fatal("expected a presets diff in the result")
+	}
+
+	if presetDiff.CurrentCount != 3 || presetDiff.IncomingCount != 1 {
+		t.Errorf("expected presets diff 3->1, got %d->%d", presetDiff.CurrentCount, presetDiff.IncomingCount)
+	}
+
+	if len(presetDiff.Removed) != 2 {
+		t.Errorf("expected 2 removed preset names (slots 2 and 3), got %v", presetDiff.Removed)
+	}
+
+	presetsAfterRefusal, err := ds.GetPresets(accountID, deviceID)
+	if err != nil {
+		t.Fatalf("GetPresets after refused sync: %v", err)
+	}
+
+	if len(presetsAfterRefusal) != 3 {
+		t.Fatalf("expected the original 3 presets to survive an unconfirmed destructive sync, got %d", len(presetsAfterRefusal))
+	}
+
+	// Confirmed: must re-check fresh state and apply.
+	result, err = m.SyncDeviceData(deviceIP, true)
+	if err != nil {
+		t.Fatalf("SyncDeviceData(confirmed=true): %v", err)
+	}
+
+	if !result.Applied {
+		t.Fatal("expected confirmed destructive sync to apply")
+	}
+
+	presetsAfterConfirm, err := ds.GetPresets(accountID, deviceID)
+	if err != nil {
+		t.Fatalf("GetPresets after confirmed sync: %v", err)
+	}
+
+	if len(presetsAfterConfirm) != 1 {
+		t.Fatalf("expected confirmed sync to shrink to 1 preset, got %d", len(presetsAfterConfirm))
+	}
+}
@@ -64,7 +64,7 @@ func TestSyncDeviceData_UsesDeviceID(t *testing.T) {
 	manager := NewManager("http://localhost:8000", ds, cm)

 	// Test SyncDeviceData
-	err := manager.SyncDeviceData(serverHost)
+	_, err := manager.SyncDeviceData(serverHost, false)
 	if err != nil {
 		t.Fatalf("SyncDeviceData failed: %v", err)
 	}
@@ -130,7 +130,7 @@ func TestSyncDeviceData_NoDeviceID_ShouldFail(t *testing.T) {
 	manager := NewManager("http://localhost:8000", ds, cm)

 	// Test SyncDeviceData - should fail
-	err := manager.SyncDeviceData(serverHost)
+	_, err := manager.SyncDeviceData(serverHost, false)
 	if err == nil {
 		t.Fatal("SyncDeviceData should have failed when deviceID is empty")
 	}
@@ -221,7 +221,7 @@ func TestSyncDeviceData_FallbackToExistingDeviceMapping(t *testing.T) {
 	manager := NewManager("http://localhost:8000", ds, cm)

 	// Sync should work and use MAC address
-	err := manager.SyncDeviceData(serverHost)
+	_, err := manager.SyncDeviceData(serverHost, false)
 	if err != nil {
 		t.Fatalf("SyncDeviceData failed: %v", err)
 	}
@@ -279,7 +279,7 @@ func TestSyncSources_Format(t *testing.T) {
 	deviceIP := mockDevice.Listener.Addr().String()
 	accountID := "1234567"
 	deviceID := "001122334455"
-	err = m.SyncDeviceData(deviceIP)
+	_, err = m.SyncDeviceData(deviceIP, false)
 	if err != nil {
 		t.Fatalf("SyncDeviceData failed: %v", err)
 	}
@@ -3,6 +3,7 @@ package soundtouchweb
 import (
 	"context"
 	"log"
+	"strings"
 	"sync"
 	"time"

@@ -10,12 +11,18 @@ import (
 	"github.com/gesellix/bose-soundtouch/pkg/config"
 	"github.com/gesellix/bose-soundtouch/pkg/discovery"
 	"github.com/gesellix/bose-soundtouch/pkg/service/soundtouchweb/webtypes"
+	"github.com/gesellix/bose-soundtouch/pkg/speaker"
 )

 // NewDiscoveryService loads config and returns a unified discovery service
 // preconfigured for the web UI's use (10 s discovery timeout, cache on).
 // When discoveryInterface is non-empty, mDNS/UPnP are pinned to that NIC.
-func NewDiscoveryService(discoveryInterface string) *discovery.UnifiedDiscoveryService {
+// configuredHosts (e.g. from --devices) are folded into cfg.PreferredDevices
+// alongside any already loaded from PREFERRED_DEVICES (deduplicated by
+// host), so they're retried on every subsequent DiscoverDevices pass, not
+// just once at startup -- a host that's offline now still gets picked up
+// once it comes online.
+func NewDiscoveryService(discoveryInterface string, configuredHosts ...string) *discovery.UnifiedDiscoveryService {
 	cfg, err := config.LoadFromEnv()
 	if err != nil {
 		log.Printf("Failed to load config: %v, using defaults", err)
@@ -30,6 +37,23 @@ func NewDiscoveryService(discoveryInterface string) *discovery.UnifiedDiscoveryS
 		cfg.DiscoveryInterface = discoveryInterface
 	}

+	existingHosts := make(map[string]bool, len(cfg.PreferredDevices))
+	for _, d := range cfg.PreferredDevices {
+		existingHosts[d.Host] = true
+	}
+
+	for _, host := range configuredHosts {
+		if host == "" || existingHosts[host] {
+			continue
+		}
+
+		cfg.PreferredDevices = append(cfg.PreferredDevices, config.DeviceConfig{
+			Host: host,
+			Port: speaker.HTTPPort,
+		})
+		existingHosts[host] = true
+	}
+
 	return discovery.NewUnifiedDiscoveryService(cfg)
 }

@@ -164,6 +188,21 @@ func (app *WebApp) DiscoverDevices(ctx context.Context, discoveryService *discov
 	log.Printf("Found %d devices", len(devices))

 	for _, device := range devices {
-		app.AddDeviceByHost(device.Host, device.Port, "discovered")
+		app.AddDeviceByHost(device.Host, device.Port, classifySource(device.DiscoveryMethod))
 	}
 }
+
+// classifySource labels a discovered device "manual" if it came from (at
+// least in part) a configured host, "discovered" otherwise. discoveryMethod
+// can be a "+"-joined composite (e.g. "Configuration+mDNS/Bonjour") when
+// mergeDeviceData combines a configured host with the same device found via
+// mDNS/UPnP in the same sweep -- match by substring, not exact equality, so
+// a manually configured host that's also independently discoverable still
+// gets labeled "manual".
+func classifySource(discoveryMethod string) string {
+	if strings.Contains(discoveryMethod, "Configuration") {
+		return "manual"
+	}
+
+	return "discovered"
+}
@@ -0,0 +1,99 @@
+package soundtouchweb
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+func TestDiscoverDevicesRetriesConfiguredHosts(t *testing.T) {
+	var available atomic.Bool
+	var infoRequests atomic.Int32
+
+	// NewTestServer (Go 1.27) registers its own t.Cleanup(Close) instead of
+	// needing a manual defer, and fails the test on a handler panic. It
+	// defaults to an in-memory transport reachable only via Server.Client(),
+	// but our production client.NewClient dials a real address, so Start()
+	// (rather than Client()) is used here to get a real loopback listener,
+	// same as the old NewServer -- see https://pkg.go.dev/net/http/httptest#NewTestServer.
+	server := httptest.NewTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/info" {
+			http.NotFound(w, r)
+			return
+		}
+
+		infoRequests.Add(1)
+		if !available.Load() {
+			http.Error(w, "offline", http.StatusServiceUnavailable)
+			return
+		}
+
+		w.Header().Set("Content-Type", "application/xml")
+		_, _ = w.Write([]byte(`<info deviceID="TESTDEVICE"><name>Configured speaker</name><type>SoundTouch 10</type></info>`))
+	}))
+	server.Start()
+
+	t.Setenv("UPNP_ENABLED", "false")
+	t.Setenv("MDNS_ENABLED", "false")
+	t.Setenv("PREFERRED_DEVICES", "")
+
+	configuredHost := strings.TrimPrefix(server.URL, "http://")
+	discoveryService := NewDiscoveryService("", configuredHost)
+	app := NewWebApp()
+
+	app.DiscoverDevices(context.Background(), discoveryService)
+	if got := app.DeviceCount(); got != 0 {
+		t.Fatalf("device count after offline probe = %d, want 0", got)
+	}
+
+	available.Store(true)
+	app.DiscoverDevices(context.Background(), discoveryService)
+	if got := app.DeviceCount(); got != 1 {
+		t.Fatalf("device count after retry = %d, want 1", got)
+	}
+	if got := infoRequests.Load(); got != 2 {
+		t.Fatalf("/info request count = %d, want 2", got)
+	}
+
+	if !app.RemoveDevice(configuredHost) {
+		t.Fatal("configured device was not registered under its host")
+	}
+
+	// AddDeviceByHost spawns a one-shot status-update goroutine and a 30s-
+	// ticker poll loop on successful registration. RemoveDevice signals the
+	// ticker loop to exit via conn.Done() but doesn't wait for it to actually
+	// observe the close, and the one-shot goroutine has no cancellation at
+	// all. Give them a moment to finish before the deferred server.Close()
+	// runs, so a still-in-flight request against the closing httptest server
+	// doesn't produce log noise or -race flakiness.
+	time.Sleep(50 * time.Millisecond)
+}
+
+// TestClassifySource covers the case the test above can't reach without a
+// real mDNS/UPnP sweep: mergeDeviceData joins discovery methods with "+"
+// when a configured host is also found via mDNS/UPnP in the same pass (see
+// pkg/discovery/unified.go), so DiscoveryMethod is not always exactly
+// "Configuration" for a manually configured device.
+func TestClassifySource(t *testing.T) {
+	tests := []struct {
+		discoveryMethod string
+		want            string
+	}{
+		{"Configuration", "manual"},
+		{"Configuration+mDNS/Bonjour", "manual"},
+		{"mDNS/Bonjour+Configuration", "manual"},
+		{"mDNS/Bonjour", "discovered"},
+		{"SSDP/UPnP", "discovered"},
+		{"", "discovered"},
+	}
+
+	for _, tt := range tests {
+		if got := classifySource(tt.discoveryMethod); got != tt.want {
+			t.Errorf("classifySource(%q) = %q, want %q", tt.discoveryMethod, got, tt.want)
+		}
+	}
+}
@@ -21,7 +21,11 @@ If your device doesn't expose the port, you can still use the on-device installe

 The storage space on the SoundTouch devices is very limited — stock rootfs typically has only a few MB free (e.g. ~4 MB on the ST20, see issue #268), well below the AfterTouch binary's ~12 MB. To work around this, the installer puts everything on `/mnt/nv/aftertouch` by default (the persistent partition, typically ~30 MB free) and points `/opt/aftertouch` at it via a symlink so the init script and runtime paths stay unchanged. Override the install target with `INSTALL_DIR=/some/path` if you've got room elsewhere.

-The space limitation also means we are currently unsure on how to update the system, because two binaries are already too large. We are currently working on this - both by checking how we can make the binaries smaller, but also on how we can extend the storage space (e.g. by running AfterTouch from a USB drive).
+Updating is genuinely tight on this partition, since the old binary, the new one, and a rollback backup can't all comfortably fit at once, and binaries only keep growing (the Go toolchain's own defaults alone add hundreds of KB per major version, independent of anything in this project). The installer handles this in a few ways:
+- The rollback backup is gzip-compressed (`.backup.gz`) rather than a plain copy, cutting its footprint by roughly a third.
+- Before downloading anything, it checks whether there's actually enough free space for the update, using the new binary's real size (a HEAD request), not a guess.
+- If there's enough room for the update itself but not enough extra for a backup, it asks for confirmation before proceeding without one — reading from `/dev/tty` since the installer is normally run as `curl | sh`. The default (empty input, or no `/dev/tty` available at all) is always to abort rather than silently skip the backup; set `AFTERTOUCH_FORCE_NO_BACKUP=yes` to skip that prompt for unattended/scripted installs.
+- If there isn't even enough room for the update itself, it aborts before downloading anything, rather than leaving a partially-overwritten, non-executable binary in place.

 ### Logs

@@ -177,14 +181,39 @@ redirect to discover the newest tag. If that lookup fails (offline, or a `curl`
 build without `-w` support), it falls back to a pinned version baked into the
 script.

-> **Tip — rollback:** if the new binary misbehaves, the installer left a `.backup` file alongside it:
+> **Tip — rollback:** if the new binary misbehaves, the installer left a backup file
+> alongside it, gzip-compressed as `.backup.gz` (plain `.backup`, uncompressed, if
+> `gzip` wasn't available on your device):
 > ```bash
-> ls /mnt/nv/aftertouch/aftertouch-service*.backup
+> ls /mnt/nv/aftertouch/aftertouch-service*.backup*
+> # .backup.gz (compressed):
+> gunzip -c /mnt/nv/aftertouch/aftertouch-service.<old-version>.backup.gz \
+>    > /mnt/nv/aftertouch/aftertouch-service
+> chmod +x /mnt/nv/aftertouch/aftertouch-service
+> # or, for an uncompressed .backup:
 > cp /mnt/nv/aftertouch/aftertouch-service.<old-version>.backup \
 >    /mnt/nv/aftertouch/aftertouch-service
 > /etc/init.d/aftertouch restart
 > ```

+## Environment Variables
+
+All of these go on `sh`, not `curl` — in a pipe, each command is a separate
+process, so `VAR=X curl ... | sh` silently does NOT set it for `sh` (the one
+that actually reads it). Use `curl -sSL .../install.sh | VAR=X sh` instead.
+
+| Variable                     | Default                                             | Purpose                                                                                                                                                                                                                                                                                 |
+|------------------------------|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `VERSION`                    | resolved automatically to the latest GitHub release | Install a specific version instead of latest. See [Updating AfterTouch](#updating-aftertouch) for the three equivalent ways to set this.                                                                                                                                                |
+| `INSTALL_DIR`                | `/mnt/nv/aftertouch`                                | Where AfterTouch is installed. `/opt/aftertouch` is symlinked here so the init script's hardcoded path keeps working.                                                                                                                                                                   |
+| `AFTERTOUCH_FORCE_NO_BACKUP` | unset                                               | Skip the interactive "not enough space for a backup, continue anyway?" prompt and proceed without a rollback backup. For unattended/scripted installs only — interactively, the installer always asks (or aborts if no terminal is available) rather than silently skipping the backup. |
+| `AFTERTOUCH_LAN_PORT`        | `auto`                                              | Written into `aftertouch.conf` (not consumed by the installer itself beyond that). Controls whether/which LAN entry-port gets redirected to AfterTouch. See [Model Support Matrix](../../docs/content/docs/reference/MODEL-SUPPORT-MATRIX.md).                                          |
+| `UPDATE_TMP_DIR`             | `/media/aftertouch`                                 | Scratch directory for the downloaded binary before it replaces the installed one. Should stay on a different filesystem than `INSTALL_DIR` (tmpfs by default) so the download itself doesn't compete with `INSTALL_DIR` for space.                                                      |
+| `GH_REPO`                    | `gesellix/Bose-SoundTouch`                          | Install from a fork instead, e.g. for testing an unmerged branch's release.                                                                                                                                                                                                             |
+| `BINARY_URL`                 | derived from `GH_REPO`/`VERSION`                    | Override the service binary's download URL entirely, bypassing `GH_REPO`/`VERSION` for this one file.                                                                                                                                                                                   |
+| `INIT_SCRIPT_URL`            | derived from `GH_REPO`/`VERSION`                    | Override the init script's download URL entirely, bypassing `GH_REPO`/`VERSION` for this one file.                                                                                                                                                                                      |
+| `FALLBACK_VERSION`           | `0.123.0`                                           | Used only if the latest-release lookup fails (offline, rate-limited, or a `curl` build without `-w` support).                                                                                                                                                                           |
+
 ## Uninstallation

 Before uninstall, you might want to revert the migration, especially the changes to the server URLs (even though having configured an unresponsive local server probably is about as bad as having configured unresponsive Bose servers). To uninstall AfterTouch, run the following command on the speaker.
@@ -97,6 +97,7 @@ fi
 # 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".*.backup.gz \
          "$INSTALL_DIR/aftertouch-service".*.old \
          "$INSTALL_DIR/aftertouch-service.new"; do
  [ -f "$f" ] || continue
@@ -105,6 +106,86 @@ for f in "$INSTALL_DIR/aftertouch-service".*.backup \
 done
 echo "Disk usage after pre-install GC:"; df -h "$INSTALL_DIR"

+# --- Preflight disk-space check ------------------------------------------
+# /mnt/nv is small (tens of MB) and binaries keep growing (Go 1.27 alone
+# added ~640KB to this binary via its own new stdlib defaults, unrelated to
+# this project's code). A prior attempt on real hardware ran out of space
+# mid-replace and left a truncated, non-executable binary in place: UBIFS is
+# a log-structured flash filesystem, so space freed by overwriting the old
+# binary isn't necessarily reusable by the time the new one needs to land.
+# Check upfront, with a safety margin, instead of discovering this mid-write.
+#
+# The new binary's size comes from a HEAD request rather than a hardcoded
+# threshold, so this doesn't go stale as binaries grow across releases.
+NEW_BINARY_BYTES=$(curl -sSLI --fail "$BINARY_URL" 2>/dev/null \
+  | tr -d '\r' \
+  | awk 'tolower($1) == "content-length:" {v=$2} END {print v}') || true
+
+AVAILABLE_KB=$(df -Pk "$INSTALL_DIR" | awk 'NR==2 {print $4}')
+
+CURRENT_BINARY_KB=0
+if [ -f "$INSTALL_DIR/aftertouch-service" ]; then
+  CURRENT_BINARY_KB=$(du -k "$INSTALL_DIR/aftertouch-service" | awk '{print $1}')
+fi
+
+# Flat margin, not a percentage: covers UBIFS's own reserved/GC headroom on
+# this log-structured flash filesystem plus general slack.
+SAFETY_MARGIN_KB=5120 # 5 MB
+
+SKIP_BACKUP=no
+
+if [ -n "$NEW_BINARY_BYTES" ]; then
+  NEW_BINARY_KB=$((NEW_BINARY_BYTES / 1024))
+  # Backups compress to roughly 70% of the original size in practice
+  # (observed: a ~14.8MB binary gzipped to ~10.1MB); used as a conservative
+  # estimate since the real ratio isn't known until compression actually runs.
+  BACKUP_ESTIMATE_KB=$((CURRENT_BINARY_KB * 7 / 10))
+
+  NEEDED_WITH_BACKUP_KB=$((NEW_BINARY_KB + BACKUP_ESTIMATE_KB + SAFETY_MARGIN_KB))
+  NEEDED_NO_BACKUP_KB=$((NEW_BINARY_KB + SAFETY_MARGIN_KB))
+
+  if [ "$AVAILABLE_KB" -ge "$NEEDED_WITH_BACKUP_KB" ]; then
+    : # plenty of room; proceed normally, with a backup
+  elif [ "$AVAILABLE_KB" -ge "$NEEDED_NO_BACKUP_KB" ]; then
+    echo "WARNING: not enough free space on $INSTALL_DIR to keep a rollback" >&2
+    echo "backup this time (${AVAILABLE_KB}KB available; ~${NEEDED_WITH_BACKUP_KB}KB" >&2
+    echo "wanted with a backup, ~${NEEDED_NO_BACKUP_KB}KB without one)." >&2
+    echo "Continuing will replace the current binary with NO way to" >&2
+    echo "automatically undo it if something goes wrong." >&2
+    if [ -n "${AFTERTOUCH_FORCE_NO_BACKUP:-}" ]; then
+      echo "Proceeding without a backup (AFTERTOUCH_FORCE_NO_BACKUP is set)." >&2
+      SKIP_BACKUP=yes
+    elif [ -r /dev/tty ] && [ -w /dev/tty ]; then
+      printf 'Continue without a backup? [y/N] ' > /dev/tty
+      REPLY=""
+      read -r REPLY < /dev/tty || true
+      case "$REPLY" in
+        [Yy]*) SKIP_BACKUP=yes ;;
+        *)
+          echo "Aborting: refusing to proceed without a backup. Free up space" >&2
+          echo "on $INSTALL_DIR and try again, or set AFTERTOUCH_FORCE_NO_BACKUP=yes" >&2
+          echo "to proceed without one non-interactively." >&2
+          exit 1
+          ;;
+      esac
+    else
+      echo "No interactive terminal available to confirm; aborting." >&2
+      echo "Set AFTERTOUCH_FORCE_NO_BACKUP=yes to proceed without a backup" >&2
+      echo "non-interactively." >&2
+      exit 1
+    fi
+  else
+    echo "ERROR: not enough free space on $INSTALL_DIR to install AfterTouch" >&2
+    echo "$VERSION safely (${AVAILABLE_KB}KB available, ~${NEEDED_NO_BACKUP_KB}KB" >&2
+    echo "needed). Free up space and try again." >&2
+    exit 1
+  fi
+else
+  echo "WARNING: could not determine the new binary's size ahead of time" >&2
+  echo "(HEAD request to $BINARY_URL failed); skipping the preflight" >&2
+  echo "disk-space check." >&2
+fi
+
 curl \
  -sSL \
  -o "$UPDATE_TMP_DIR/binary" \
@@ -114,15 +195,34 @@ curl \
 # Back up the current binary before overwriting so a one-step rollback
 # is always available.  The version string comes from the binary itself;
 # if it is absent (very old build or corrupted) we fall back to a timestamp.
+# Skipped entirely when the preflight check above decided (with the
+# operator's explicit confirmation, or AFTERTOUCH_FORCE_NO_BACKUP) that
+# there isn't room for one.
 BACKUP_FILE=""
-if [ -f "$INSTALL_DIR/aftertouch-service" ]; then
+if [ -f "$INSTALL_DIR/aftertouch-service" ] && [ "$SKIP_BACKUP" != "yes" ]; then
  current_version=$("$INSTALL_DIR/aftertouch-service" --version 2>/dev/null \
    | awk '{print $NF}') || true
  if [ -z "$current_version" ] || [ "$current_version" = "dev" ]; then
    current_version=$(date +%Y%m%d-%H%M%S)
  fi
+  # Binaries are tens of MB and only growing (see #614 investigation into
+  # Go 1.27's default binary-size increase), while /mnt/nv is small (tens of
+  # MB total). Stream straight into the compressed file rather than cp-then-
+  # gzip: at this point in the script the old binary is still live AND the
+  # newly-downloaded one is already sitting in $UPDATE_TMP_DIR, so an
+  # intermediate uncompressed backup copy would briefly need all three full
+  # copies on disk at once -- exactly the kind of moment that has already
+  # caused "no space left on device" failures here. Best effort: if gzip is
+  # missing, or the stream fails partway (e.g. disk fills mid-compress),
+  # fall back to a plain uncompressed copy exactly as before.
  BACKUP_FILE="$INSTALL_DIR/aftertouch-service.${current_version}.backup"
-  cp -p "$INSTALL_DIR/aftertouch-service" "$BACKUP_FILE"
+  if command -v gzip >/dev/null 2>&1 \
+      && gzip -c < "$INSTALL_DIR/aftertouch-service" > "$BACKUP_FILE.gz"; then
+    BACKUP_FILE="$BACKUP_FILE.gz"
+  else
+    rm -f "$BACKUP_FILE.gz"
+    cp -p "$INSTALL_DIR/aftertouch-service" "$BACKUP_FILE"
+  fi
  echo "Backed up current binary ($current_version) → $BACKUP_FILE"
 fi

@@ -136,6 +236,7 @@ chmod +x "$INSTALL_DIR/aftertouch-service"
 if [ -n "$BACKUP_FILE" ]; then
  echo "Disk usage before post-install GC:"; df -h "$INSTALL_DIR"
  for f in "$INSTALL_DIR/aftertouch-service".*.backup \
+            "$INSTALL_DIR/aftertouch-service".*.backup.gz \
            "$INSTALL_DIR/aftertouch-service".*.old \
            "$INSTALL_DIR/aftertouch-service.new"; do
    [ -f "$f" ] || continue