fix(handlers): remove stale account entry when MoveDevice target dir exists

When handleDiscoveredDevice calls MoveDevice and the target device
directory already exists (pre-existing duplicate state), os.Rename
fails with ENOTEMPTY/EEXIST leaving the stale source account entry
on disk. Because SaveDeviceInfo has just written fresh data under
accountID, it is safe to unconditionally remove the stale source
entry afterward — RemoveDevice returns nil when the path is already
gone (successful rename), so this is a no-op in the happy path and
a cleanup in the failure path.

Adds TestHandleDiscoveredDevice_CrossAccountMigration_TargetExists
which seeds a device under two real accounts (old sorts alphabetically
first so findExistingDeviceInfoByDeviceID picks it as storedAccount),
triggers discovery with the new account as MargeAccountUUID, and
asserts that after the cycle only the new account entry exists.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

pkg/service/handlers/mac_discovery_integration_test.go

@@ -463,6 +463,107 @@ func TestHandleDiscoveredDevice_CrossAccountMigration(t *testing.T) {
 	}
 }
 
+// TestHandleDiscoveredDevice_CrossAccountMigration_TargetExists covers the case
+// where both the old and the new account directories already have data for the
+// device before discovery fires. os.Rename fails because the target dir exists,
+// so MoveDevice cannot atomically relocate the directory. The stale source entry
+// must still be removed via the unconditional RemoveDevice fallback, and
+// ListAllDevices must return exactly one entry after the cycle.
+func TestHandleDiscoveredDevice_CrossAccountMigration_TargetExists(t *testing.T) {
+	tempDir := t.TempDir()
+	ds := datastore.NewDataStore(tempDir)
+
+	const (
+		deviceID = "F4E11E930BEB"
+		// oldAccount sorts alphabetically BEFORE newAccount so that
+		// ListAllDevices (which sorts accounts alphabetically, pushing "default"
+		// to the back) picks it first. That means findExistingDeviceInfoByDeviceID
+		// returns oldAccount as storedAccount, the MargeAccountUUID mismatch
+		// condition fires, MoveDevice fails because newAccount already has a
+		// device dir, and the RemoveDevice fallback must clean up the stale entry.
+		oldAccount = "account-aaa-stale"
+		newAccount = "account-zzz-live"
+	)
+
+	// Seed the device under both accounts (simulates a pre-existing duplicate).
+	for _, acc := range []string{oldAccount, newAccount} {
+		info := &models.ServiceDeviceInfo{
+			DeviceID:  deviceID,
+			AccountID: acc,
+			Name:      "Stale Name",
+			IPAddress: "192.0.2.42",
+		}
+		if err := ds.SaveDeviceInfo(acc, deviceID, info); err != nil {
+			t.Fatalf("SaveDeviceInfo under %s: %v", acc, err)
+		}
+	}
+
+	// Confirm both exist before discovery.
+	all, err := ds.ListAllDevices()
+	if err != nil {
+		t.Fatalf("ListAllDevices (before): %v", err)
+	}
+	// ListAllDevices deduplicates; both real accounts → alphabetically-first wins.
+	if len(all) != 1 {
+		t.Logf("ListAllDevices before discovery: %d entries (expected 1 due to dedup): %+v", len(all), all)
+	}
+
+	// Mock /info — reports newAccount as the canonical MargeAccountUUID.
+	deviceInfoXML := fmt.Sprintf(`<info deviceID="%s">
+<name>Updated Name</name>
+<type>SoundTouch 10</type>
+<margeAccountUUID>%s</margeAccountUUID>
+<networkInfo type="SCM">
+<macAddress>%s</macAddress>
+<ipAddress>192.0.2.42</ipAddress>
+</networkInfo>
+<moduleType>sm2</moduleType>
+</info>`, deviceID, newAccount, deviceID)
+
+	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://"):]
+	sm := setup.NewManager(server.URL, ds, nil)
+	srv := NewServer(ds, sm, "http://localhost", false, false, false)
+
+	srv.handleDiscoveredDevice(models.DiscoveredDevice{
+		Host:            deviceIP,
+		Name:            "Discovery Name",
+		DiscoveryMethod: "mDNS",
+	})
+
+	// New account must have fresh data.
+	info, err := ds.GetDeviceInfo(newAccount, deviceID)
+	if err != nil {
+		t.Fatalf("GetDeviceInfo under %s: %v", newAccount, err)
+	}
+	if info.Name != "Updated Name" {
+		t.Errorf("Name: want %q, got %q", "Updated Name", info.Name)
+	}
+
+	// Old account stale entry must be gone.
+	if _, err := ds.GetDeviceInfo(oldAccount, deviceID); err == nil {
+		t.Errorf("stale entry still present under old account %s", oldAccount)
+	}
+
+	// No duplicates in ListAllDevices.
+	all, err = ds.ListAllDevices()
+	if err != nil {
+		t.Fatalf("ListAllDevices (after): %v", err)
+	}
+	if len(all) != 1 {
+		t.Errorf("ListAllDevices: want 1 device, got %d: %+v", len(all), all)
+	}
+}
+
 func TestMACBasedDeviceDiscovery_FallbackScenario(t *testing.T) {
 	// Test scenario where /info endpoint is not available
 	tempDir, err := os.MkdirTemp("", "mac-fallback-test-*")

pkg/service/handlers/server.go

@@ -1099,6 +1099,19 @@ func (s *Server) handleDiscoveredDevice(d models.DiscoveredDevice) {
 		return
 	}
 
+	// If the device was (or needed to be) relocated to a different account, ensure the
+	// stale source entry is gone. MoveDevice's rename is a no-op if the target already
+	// existed (e.g. partial duplicate state), leaving the source dir behind; removing it
+	// here is safe because SaveDeviceInfo above has already written fresh data to
+	// accountID. RemoveDevice returns nil when the path does not exist, so this is also
+	// a harmless no-op when MoveDevice already renamed the directory successfully.
+	if storedAccount != "" && storedAccount != accountID {
+		if err := s.ds.RemoveDevice(storedAccount, deviceID); err != nil {
+			log.Printf("Failed to remove stale device entry for %s in %s: %v",
+				deviceID, storedAccount, err)
+		}
+	}
+
 	// 8. Create default Sources.xml only when no sources file exists yet
 	if !s.ds.HasConfiguredSources(accountID, deviceID) {
 		if sources, err := s.ds.GetConfiguredSources(accountID, deviceID); err == nil {