Fix Scenario B migration: keep legacy accessory to preserve HomeKit customisations (#246)

* Initial plan

* Fix Scenario B: keep legacy accessory to preserve HomeKit customisations

Agent-Logs-Url: https://github.com/homebridge-plugins/homebridge-updater/sessions/979794f8-58d7-4fbb-9b49-3283a8a12e03

Co-authored-by: donavanbecker <9875439+donavanbecker@users.noreply.github.com>

* Tighten Scenario B assertions: use toHaveBeenLastCalledWith and call count

Agent-Logs-Url: https://github.com/homebridge-plugins/homebridge-updater/sessions/01011440-32cb-45fb-a605-53e4eb523985

Co-authored-by: donavanbecker <9875439+donavanbecker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: donavanbecker <9875439+donavanbecker@users.noreply.github.com>
Co-authored-by: Donavan Becker <beckersmarthome@icloud.com>

Author: Copilot

package-lock.json

@@ -89,13 +89,15 @@ describe('PluginUpdatePlatform legacy UUID migration', () => {
     const legacyUpdateSensorUuid = mockApi.hap.uuid.generate(LEGACY_UPDATE_SENSOR_UUID_KEY)
 
     let cachedLegacyUpdateAccessory: any = undefined
+    let cachedCurrentUpdateAccessory: any = undefined
     let updateSensorCached = false
 
     // Simulate configureAccessory for each cached accessory
     for (const accessory of cachedAccessories) {
       if (accessory.UUID === updateSensorUuid) {
         mockUpdateSensor.configureAccessory(accessory)
         updateSensorCached = true
+        cachedCurrentUpdateAccessory = accessory
         continue
       }
       if (accessory.UUID === legacyUpdateSensorUuid) {
@@ -112,9 +114,15 @@ describe('PluginUpdatePlatform legacy UUID migration', () => {
       if (!cachedLegacyUpdateAccessory) return
 
       if (updateSensorCached) {
-        mockApi.unregisterPlatformAccessories(PLUGIN_NAME, PLATFORM_NAME, [cachedLegacyUpdateAccessory])
+        // Both UUIDs found — remove the newer empty accessory, promote the legacy one
+        // so the user's HomeKit customisations are preserved.
+        if (cachedCurrentUpdateAccessory) {
+          mockApi.unregisterPlatformAccessories(PLUGIN_NAME, PLATFORM_NAME, [cachedCurrentUpdateAccessory])
+          cachedCurrentUpdateAccessory = undefined
+        }
+        mockUpdateSensor.configureAccessory(cachedLegacyUpdateAccessory)
         cachedLegacyUpdateAccessory = undefined
-        mockLog.info('Removed stale legacy update sensor accessory (migrated to current UUID)')
+        mockLog.info('Migrated update sensor to legacy cached accessory (preserved HomeKit customisations)')
         return
       }
 
@@ -160,21 +168,24 @@ describe('PluginUpdatePlatform legacy UUID migration', () => {
     expect(registered).toBe(true)
   })
 
-  it('Scenario B: both legacy and current UUID in cache — must unregister the stale legacy accessory', () => {
+  it('Scenario B: both legacy and current UUID in cache — must preserve legacy accessory and remove the newer empty duplicate', () => {
     const { mockLog, mockApi, mockUpdateSensor, uuidMap } = buildMocks()
 
     const v3Accessory = { UUID: uuidMap[UPDATE_SENSOR_UUID_KEY], displayName: 'PluginUpdate' }
     const legacyAccessory = { UUID: uuidMap[LEGACY_UPDATE_SENSOR_UUID_KEY], displayName: 'Plugin Update Check' }
 
     runMigrationSimulation(mockApi, mockLog, mockUpdateSensor, uuidMap, [v3Accessory, legacyAccessory])
 
-    // v3 accessory configured as update sensor
-    expect(mockUpdateSensor.configureAccessory).toHaveBeenCalledWith(v3Accessory)
-    // Legacy unregistered
-    expect(mockApi.unregisterPlatformAccessories).toHaveBeenCalledWith(PLUGIN_NAME, PLATFORM_NAME, [legacyAccessory])
-    // No new accessory created (v3 already registered)
+    // configureAccessory called exactly twice: once for the v3 (during restore) then
+    // again for the legacy during migration — legacy must be the final (last) call.
+    expect(mockUpdateSensor.configureAccessory).toHaveBeenCalledTimes(2)
+    expect(mockUpdateSensor.configureAccessory).toHaveBeenNthCalledWith(1, v3Accessory)
+    expect(mockUpdateSensor.configureAccessory).toHaveBeenLastCalledWith(legacyAccessory)
+    // Current-UUID empty duplicate is removed
+    expect(mockApi.unregisterPlatformAccessories).toHaveBeenCalledWith(PLUGIN_NAME, PLATFORM_NAME, [v3Accessory])
+    // No new accessory created
     expect(mockApi.registerPlatformAccessories).not.toHaveBeenCalled()
-    expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('Removed stale legacy update sensor accessory'))
+    expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('Migrated update sensor to legacy cached accessory'))
   })
 
   it('Scenario B (legacy processed first): same result regardless of configureAccessory order', () => {
@@ -186,8 +197,12 @@ describe('PluginUpdatePlatform legacy UUID migration', () => {
     // Legacy accessory arrives first in configureAccessory order
     runMigrationSimulation(mockApi, mockLog, mockUpdateSensor, uuidMap, [legacyAccessory, v3Accessory])
 
-    expect(mockUpdateSensor.configureAccessory).toHaveBeenCalledWith(v3Accessory)
-    expect(mockApi.unregisterPlatformAccessories).toHaveBeenCalledWith(PLUGIN_NAME, PLATFORM_NAME, [legacyAccessory])
+    // Only the v3 accessory is processed during restore (legacy is held), then the
+    // migration promotes the legacy one — so configureAccessory is still called twice
+    // and the legacy accessory is always the last call.
+    expect(mockUpdateSensor.configureAccessory).toHaveBeenCalledTimes(2)
+    expect(mockUpdateSensor.configureAccessory).toHaveBeenLastCalledWith(legacyAccessory)
+    expect(mockApi.unregisterPlatformAccessories).toHaveBeenCalledWith(PLUGIN_NAME, PLATFORM_NAME, [v3Accessory])
     expect(mockApi.registerPlatformAccessories).not.toHaveBeenCalled()
   })
 

src/Platform.HAP.test.ts

@@ -18,6 +18,8 @@ export class PluginUpdatePlatform implements DynamicPlatformPlugin {
   private cachedFailureAccessory?: PlatformAccessory
   /** Cached accessory with the legacy v2.x UUID — resolved in handleLegacyMigration */
   private cachedLegacyUpdateAccessory?: PlatformAccessory
+  /** Cached accessory with the current UUID — tracked so it can be replaced by the legacy one in Scenario B */
+  private cachedCurrentUpdateAccessory?: PlatformAccessory
   /** True when a v3 update-sensor accessory was found in the cache (current UUID) */
   private updateSensorCached = false
 
@@ -46,19 +48,28 @@ export class PluginUpdatePlatform implements DynamicPlatformPlugin {
    * 1. Only the legacy UUID is in the cache (fresh v2→v3 migration): configure the
    *    update sensor on the legacy accessory so that {@link UpdateSensor.addUpdateSensor}
    *    sees `registered = true` and does NOT create a duplicate.
-   * 2. Both legacy and current UUIDs are in the cache (user already ran v3 once):
-   *    unregister the stale legacy accessory and let the current-UUID accessory continue.
+   * 2. Both legacy and current UUIDs are in the cache (user already ran a newer version
+   *    once before this migration existed, creating a duplicate): remove the newer
+   *    empty accessory and promote the legacy one so that the user's HomeKit
+   *    customisations (room, name, automations) that are tied to the legacy UUID are
+   *    preserved.
    */
   private handleLegacyMigration(): void {
     if (!this.cachedLegacyUpdateAccessory) {
       return
     }
 
     if (this.updateSensorCached) {
-      // Current-UUID accessory already recognised — remove the stale legacy one
-      this.api.unregisterPlatformAccessories(PLUGIN_NAME, PLATFORM_NAME, [this.cachedLegacyUpdateAccessory])
+      // Both UUIDs found — the current-UUID accessory was created empty when the legacy
+      // one was first ignored.  Prefer the legacy accessory so that any HomeKit
+      // customisations the user applied to it are retained.
+      if (this.cachedCurrentUpdateAccessory) {
+        this.api.unregisterPlatformAccessories(PLUGIN_NAME, PLATFORM_NAME, [this.cachedCurrentUpdateAccessory])
+        this.cachedCurrentUpdateAccessory = undefined
+      }
+      this.updateSensor.configureAccessory(this.cachedLegacyUpdateAccessory)
       this.cachedLegacyUpdateAccessory = undefined
-      this.log.info('Removed stale legacy update sensor accessory (migrated to current UUID)')
+      this.log.info('Migrated update sensor to legacy cached accessory (preserved HomeKit customisations)')
       return
     }
 
@@ -103,6 +114,7 @@ export class PluginUpdatePlatform implements DynamicPlatformPlugin {
     if (accessory.UUID === this.updateSensorUuid) {
       this.updateSensor.configureAccessory(accessory)
       this.updateSensorCached = true
+      this.cachedCurrentUpdateAccessory = accessory
       return
     }
     // Restore for failure sensor