From 5a216e5f5d78305b4b1231372c7ffff6b1ab39bc Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 09:25:18 +1200 Subject: [PATCH 1/3] fix: stamp a device as seen when it connects, not when it was added (#1527) A WHOOP that syncs every day reported "Last seen 45 d ago" on its Devices card. The same session's strap log said "Last sync: just now" and carried 37 day rows. `lastSeenAt` was written in four places and none of them was a connection: the schema migration that creates the row, the add-device wizard, DataSourcesScreen, and the promote-to-active query. The BLE path made no registry write at all, so a strap could connect, bond, offload and score for weeks without the column moving. It was reporting time-since-ADDED. Stamped now from `connectedPeripheralChanged` on both platforms, which is where the connected strap is already resolved to a registry row and already refuses to overwrite a row pinned to a different strap: - connect, identity adopted or already matching -> stamp - the #52 stale-pin re-adopt (Swift) -> stamp, it is the same confirmed strap - a DIFFERENT strap under this row -> no stamp; that strap is not this row - disconnect -> stamp, because the strap was in hand right up to that instant The disconnect edge matters on its own: stamping only on connect would leave an overnight session reading "Last seen 10 h ago" the moment the link dropped. A null republish with no prior link is not a sighting and writes nothing. Once per connect and once per disconnect, not per packet -- a write per second would buy no more truth. Archived rows are excluded in the SQL on both sides: "Removed - data kept" is a deliberate resting state and a stray connect must not resurrect one into looking live. Scoped to the WHOOP path, which is what was reported; the other source kinds have their own connect paths and are unchanged. Verified: 6 new tests in SourceCoordinatorAdoptionTest covering both edges, the different-strap guard, the bare-null case and the archived guard; full Android suite 4195 tests, 0 failures (--no-build-cache --rerun-tasks, 4189 before). Swift half validated by app-build -- see the PR. --- .../WhoopStore/DeviceRegistryStore.swift | 14 +++ Strand/BLE/SourceCoordinator.swift | 23 ++++- Strand/Data/DeviceRegistry.swift | 7 ++ .../java/com/noop/ble/SourceCoordinator.kt | 28 +++++- .../main/java/com/noop/data/DeviceRegistry.kt | 5 + .../java/com/noop/data/DeviceRegistryDao.kt | 9 ++ .../analytics/RegistryDayOwnerSourceTest.kt | 6 ++ .../noop/ble/SourceCoordinatorAdoptionTest.kt | 92 +++++++++++++++++++ .../java/com/noop/data/DeviceRegistryTest.kt | 6 ++ 9 files changed, 185 insertions(+), 5 deletions(-) diff --git a/Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift b/Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift index aa1bd58f9e..7788dc3642 100644 --- a/Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift +++ b/Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift @@ -76,6 +76,20 @@ public struct DeviceRegistryStore: Sendable { } } + /// Stamp a row as seen right now. Called when a strap actually connects or drops, because nothing + /// else in the BLE path writes `lastSeenAt` — before #1527 it was only ever set when the row was + /// created or promoted to active, so the Devices card reported time-since-ADDED and a strap syncing + /// every day could read "Last seen 45 d ago". + /// + /// Archived rows are excluded: "Removed - data kept" is a deliberate resting state, and a stray + /// connect must not quietly resurrect one into looking live. + public func touchLastSeen(_ id: String, at ts: Int) throws { + try dbQueue.write { db in + try db.execute(sql: "UPDATE pairedDevice SET lastSeenAt = ? WHERE id = ? AND status != 'archived'", + arguments: [ts, id]) + } + } + /// Adopt (or clear) the stable BLE identity for a registry row. `peripheralId` is the /// CBPeripheral.identifier.uuidString on iOS/Mac; passing nil un-adopts it. public func setPeripheralId(_ id: String, peripheralId: String?) throws { diff --git a/Strand/BLE/SourceCoordinator.swift b/Strand/BLE/SourceCoordinator.swift index d7500aa2d0..9ad309a3a3 100644 --- a/Strand/BLE/SourceCoordinator.swift +++ b/Strand/BLE/SourceCoordinator.swift @@ -489,6 +489,15 @@ final class SourceCoordinator: ObservableObject { // MARK: - Identity adoption + /// Stamp the ACTIVE WHOOP row as seen now, for the disconnect edge where there is no uuid left to + /// resolve against. Guarded on the row still being a WHOOP so a strap-switch mid-session cannot stamp + /// whatever happens to be active now. (#1527) + private func touchActiveWhoopLastSeen() { + let activeId = registry.activeDeviceId + guard isWhoop(activeId) else { return } + registry.touchLastSeen(activeId) + } + /// The BLE engine connected to a WHOOP peripheral (`uuid`). Persist that stable identity onto the /// CURRENTLY ACTIVE device when it's a WHOOP and hasn't adopted one yet — so the legacy "my-whoop" /// learns its strap's id on first connect, and a freshly-paired WHOOP confirms its identity. @@ -508,8 +517,16 @@ final class SourceCoordinator: ObservableObject { // Track the live strap's uuid for the WHOOP->WHOOP adopt-in-place skip (#74). nil is a // disconnect/never-connected republish: clear it so a later make-active can't wrongly match a stale // link, then fall through to the existing ignore. + let previouslyConnectedTo = connectedWhoopUuid connectedWhoopUuid = uuid - guard let uuid else { return } + guard let uuid else { + // Disconnect edge. The strap was in hand right up to this instant, so stamp it NOW rather than + // leaving the connect-time value: after a ten-hour overnight session that would have the card + // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually + // connected — a nil republish with no prior link is not a sighting. (#1527) + if previouslyConnectedTo != nil { touchActiveWhoopLastSeen() } + return + } let activeId = registry.activeDeviceId guard isWhoop(activeId), @@ -519,8 +536,9 @@ final class SourceCoordinator: ObservableObject { case .none: // First connect for this WHOOP row → adopt the strap's stable identity. registry.setPeripheralId(activeId, peripheralId: uuid) + registry.touchLastSeen(activeId) case .some(uuid): - break // already adopted this exact strap → nothing to do + registry.touchLastSeen(activeId) // already adopted this exact strap → only the sighting is new case .some(let existing): // A DIFFERENT strap connected under this WHOOP row. Re-adopt ONLY when this is the #52 stale-pin // handoff — i.e. the engine is genuinely encrypted-bonded to the strap whose id just arrived. @@ -531,6 +549,7 @@ final class SourceCoordinator: ObservableObject { if live.encryptedBond { live.append(log: "Multi-WHOOP (#52): active device \(activeId) was pinned to strap \(existing) which refused to bond — re-adopting the working strap \(uuid).") registry.setPeripheralId(activeId, peripheralId: uuid) + registry.touchLastSeen(activeId) } else { live.append(log: "Multi-WHOOP: active device \(activeId) is registered to strap \(existing) but \(uuid) connected — not overwriting.") } diff --git a/Strand/Data/DeviceRegistry.swift b/Strand/Data/DeviceRegistry.swift index 84b23a44b0..5060fae5d7 100644 --- a/Strand/Data/DeviceRegistry.swift +++ b/Strand/Data/DeviceRegistry.swift @@ -125,6 +125,13 @@ final class DeviceRegistry: ObservableObject { reload() } + /// Stamp a device as seen right now — a real connect or disconnect, not every inbound packet, which + /// would be a write per second for no more truth. Refreshes the published list. Best-effort. (#1527) + func touchLastSeen(_ id: String, at ts: Int = Int(Date().timeIntervalSince1970)) { + try? store.touchLastSeen(id, at: ts) + reload() + } + /// Find the paired device that has adopted a given BLE peripheral, if any. A plain read of the /// store (no reload) — returns nil on any error or when no row has adopted that peripheral yet. func device(forPeripheralId peripheralId: String) -> PairedDevice? { diff --git a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt index 5b0573684c..233968ebeb 100644 --- a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt +++ b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt @@ -212,6 +212,15 @@ class SourceCoordinator( } } + /** Stamp the ACTIVE WHOOP row as seen now, for the disconnect edge where there is no address left to + * resolve against. Guarded on the row still being a WHOOP so a strap switch mid-session cannot stamp + * whatever happens to be active now. Twin of the Swift helper. (#1527) */ + private suspend fun touchActiveWhoopLastSeen() { + val activeId = registry.activeDeviceId() ?: return + if (!isWhoop(activeId, registry.all())) return + registry.touchLastSeen(activeId) + } + /** * The BLE engine connected to a WHOOP strap at [address] (null on disconnect). Persist that stable * identity onto the CURRENTLY ACTIVE device when it's a WHOOP and hasn't adopted one yet — so the @@ -230,8 +239,18 @@ class SourceCoordinator( // Track the live strap's address for the WHOOP->WHOOP adopt-in-place skip (#74). A null address is a // disconnect/never-connected republish: clear it so a later make-active can't wrongly match a stale // link, then fall through to the existing ignore. + val previouslyConnectedTo = connectedWhoopAddress connectedWhoopAddress = address - if (address == null) return + if (address == null) { + // Disconnect edge. The strap was in hand right up to this instant, so stamp it NOW rather than + // leaving the connect-time value: after a ten-hour overnight session that would have the card + // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually + // connected — a null republish with no prior link is not a sighting. (#1527) + if (previouslyConnectedTo != null) { + scope.launch { runCatching { touchActiveWhoopLastSeen() } } + } + return + } scope.launch { val activeId = registry.activeDeviceId() ?: return@launch val devices = registry.all() @@ -240,11 +259,14 @@ class SourceCoordinator( val existing = row.peripheralId when { - existing == null -> + existing == null -> { // First connect for this WHOOP row → adopt the strap's stable identity (its address). registry.setPeripheralId(activeId, address) + registry.touchLastSeen(activeId) + } existing.equals(address, ignoreCase = true) -> { - // Already adopted this exact strap → nothing to do. + // Already adopted this exact strap → only the sighting is new. (#1527) + registry.touchLastSeen(activeId) } else -> // A DIFFERENT strap connected under this WHOOP row. Never silently overwrite — that would diff --git a/android/app/src/main/java/com/noop/data/DeviceRegistry.kt b/android/app/src/main/java/com/noop/data/DeviceRegistry.kt index 200499f35f..22ec1f9cf2 100644 --- a/android/app/src/main/java/com/noop/data/DeviceRegistry.kt +++ b/android/app/src/main/java/com/noop/data/DeviceRegistry.kt @@ -121,6 +121,11 @@ class DeviceRegistry( } } + /** Stamp a device as seen right now — a real connect or disconnect, not every inbound packet, which + * would be a write per second for no more truth. Twin of Swift `DeviceRegistry.touchLastSeen`. (#1527) */ + suspend fun touchLastSeen(id: String, now: Long = System.currentTimeMillis() / 1000) = + dao.touchLastSeen(id, now) + /** Archive a device — keeps its row and samples (invariant I4). */ suspend fun archive(id: String) = dao.archiveDevice(id) diff --git a/android/app/src/main/java/com/noop/data/DeviceRegistryDao.kt b/android/app/src/main/java/com/noop/data/DeviceRegistryDao.kt index 02bbd6402e..7275f2839d 100644 --- a/android/app/src/main/java/com/noop/data/DeviceRegistryDao.kt +++ b/android/app/src/main/java/com/noop/data/DeviceRegistryDao.kt @@ -34,6 +34,15 @@ interface DeviceRegistryDao { @Query("UPDATE pairedDevice SET status = 'active', lastSeenAt = :now WHERE id = :id") suspend fun promote(id: String, now: Long) + /** Stamp a device as seen right now. Nothing in the BLE path wrote `lastSeenAt` before #1527 — it was + * set only when the row was created or promoted to active — so the Devices card reported + * time-since-ADDED, and a strap syncing daily could read "Last seen 45 d ago". + * + * Archived rows are excluded: "Removed - data kept" is a deliberate resting state and a stray connect + * must not quietly resurrect one into looking live. Twin of the Swift store's `touchLastSeen`. */ + @Query("UPDATE pairedDevice SET lastSeenAt = :now WHERE id = :id AND status != 'archived'") + suspend fun touchLastSeen(id: String, now: Long) + /** Archive a device (keeps the row + its samples — invariant I4). */ @Query("UPDATE pairedDevice SET status = 'archived' WHERE id = :id") suspend fun archiveDevice(id: String) diff --git a/android/app/src/test/java/com/noop/analytics/RegistryDayOwnerSourceTest.kt b/android/app/src/test/java/com/noop/analytics/RegistryDayOwnerSourceTest.kt index 3169fce5c8..dc0008b194 100644 --- a/android/app/src/test/java/com/noop/analytics/RegistryDayOwnerSourceTest.kt +++ b/android/app/src/test/java/com/noop/analytics/RegistryDayOwnerSourceTest.kt @@ -44,6 +44,12 @@ class RegistryDayOwnerSourceTest { devices[id]?.let { devices[id] = it.copy(model = model) } } override suspend fun renameDevice(id: String, nickname: String?) {} + override suspend fun touchLastSeen(id: String, now: Long) { + // #1527, mirrors the query's `AND status != 'archived'`. + devices[id]?.let { + if (it.status != DeviceStatus.archived.name) devices[id] = it.copy(lastSeenAt = now) + } + } override suspend fun setPeripheralId(id: String, peripheralId: String?) { devices[id]?.let { devices[id] = it.copy(peripheralId = peripheralId) } } diff --git a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt index b84ed1eb39..ce46f4799f 100644 --- a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt +++ b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt @@ -60,6 +60,12 @@ class SourceCoordinatorAdoptionTest { override suspend fun setPeripheralId(id: String, peripheralId: String?) { devices[id]?.let { devices[id] = it.copy(peripheralId = peripheralId) } } + override suspend fun touchLastSeen(id: String, now: Long) { + // Reproduces the query's `AND status != 'archived'` guard. + devices[id]?.let { + if (it.status != DeviceStatus.archived.name) devices[id] = it.copy(lastSeenAt = now) + } + } override suspend fun deviceForPeripheralId(peripheralId: String): PairedDeviceRow? = devices.values.firstOrNull { it.peripheralId == peripheralId } override suspend fun setDayOwner(row: DayOwnershipRow) { owners[row.day] = row } @@ -384,4 +390,90 @@ class SourceCoordinatorAdoptionTest { assertEquals("a different WHOOP must drop the current link", 1, stops) assertEquals("a different WHOOP must reconnect", 1, starts) } + + // MARK: - "Last seen" is stamped by the link, not by the wizard (#1527) + + /** + * The regression: a strap that connects must move `lastSeenAt`. Nothing in the BLE path wrote the + * column before this, so the Devices card reported time-since-ADDED and a strap syncing every day + * read "Last seen 45 d ago". + */ + @Test + fun aConnectStampsTheActiveRowAsSeen() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01") + } + coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:01") + assertTrue("a connect must move lastSeenAt off its seeded value", + dao.devices["my-whoop"]!!.lastSeenAt > 100) + } + + /** A first connect adopts the identity AND records the sighting — the two are the same event. */ + @Test + fun aFirstConnectAdoptsTheIdentityAndStamps() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = null) + } + coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:01") + assertEquals("AA:BB:CC:DD:EE:01", dao.devices["my-whoop"]!!.peripheralId) + assertTrue(dao.devices["my-whoop"]!!.lastSeenAt > 100) + } + + /** + * The disconnect edge stamps too. Without it the card would report the time of the CONNECT, so an + * overnight session would read "Last seen 10 h ago" the instant the link dropped. The connect stamp is + * rewound here so the assertion can only pass on the disconnect's own write. + */ + @Test + fun aDisconnectStampsTheRowAsSeenRatherThanLeavingTheConnectTime() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01") + } + val coordinator = coordinatorOver(dao) + coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01") + dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100) // rewind the connect stamp + coordinator.connectedPeripheralChanged(null) + assertTrue("the disconnect edge must stamp the row itself", + dao.devices["my-whoop"]!!.lastSeenAt > 100) + } + + /** A null republish with no live link is not a sighting — nothing was ever connected to record. */ + @Test + fun aNullWithNoPriorConnectIsNotASighting() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01") + } + coordinatorOver(dao).connectedPeripheralChanged(null) + assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt) + } + + /** + * The guard that matters on a multi-WHOOP install: a DIFFERENT strap connecting under this row must + * neither overwrite its identity nor claim to be a sighting OF IT. Same reasoning as the peripheralId + * guard above — that strap is not this row. + */ + @Test + fun aDifferentStrapDoesNotStampThisRow() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01") + } + coordinatorOver(dao).connectedPeripheralChanged("AA:BB:CC:DD:EE:99") + assertEquals("AA:BB:CC:DD:EE:01", dao.devices["my-whoop"]!!.peripheralId) + assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt) + } + + /** + * "Removed - data kept" is a deliberate resting state: a stray connect must not quietly resurrect an + * archived row into looking live. The real guard is the query's `AND status != 'archived'`; the fake + * reproduces it, so this pins the CONTRACT callers rely on rather than Room's SQL. + */ + @Test + fun anArchivedRowIsNotStamped() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["old-whoop"] = whoopRow("old-whoop", peripheralId = "AA:BB:CC:DD:EE:02") + .copy(status = DeviceStatus.archived.name) + } + registryWith(dao).touchLastSeen("old-whoop") + assertEquals(100, dao.devices["old-whoop"]!!.lastSeenAt) + } } diff --git a/android/app/src/test/java/com/noop/data/DeviceRegistryTest.kt b/android/app/src/test/java/com/noop/data/DeviceRegistryTest.kt index 97fbb06a5b..fa9b475580 100644 --- a/android/app/src/test/java/com/noop/data/DeviceRegistryTest.kt +++ b/android/app/src/test/java/com/noop/data/DeviceRegistryTest.kt @@ -62,6 +62,12 @@ class DeviceRegistryTest { override suspend fun renameDevice(id: String, nickname: String?) { devices[id]?.let { devices[id] = it.copy(nickname = nickname) } } + override suspend fun touchLastSeen(id: String, now: Long) { + // #1527, mirrors the query's `AND status != 'archived'`. + devices[id]?.let { + if (it.status != DeviceStatus.archived.name) devices[id] = it.copy(lastSeenAt = now) + } + } override suspend fun setPeripheralId(id: String, peripheralId: String?) { devices[id]?.let { devices[id] = it.copy(peripheralId = peripheralId) } From 79f83c04565edd71313969ab3f58c7af60752e49 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 09:46:14 +1200 Subject: [PATCH 2/3] fix: resolve the disconnect stamp by adopted identity, not by whatever is active Found re-reviewing the branch. The disconnect edge stamped `activeDeviceId`, and the doc claimed the `isWhoop` guard stopped a strap switch from stamping the wrong row. It does not: it only checks the active row is A WHOOP, so on a multi-WHOOP install strap A could drop while B was active and B would be recorded as seen despite never having been connected. That is the same mis-mapping the peripheralId guard three lines below exists to prevent, so the disconnect now resolves the row by the identity the strap adopted -- the address it was connected on -- and stamps that. An address no row has adopted stamps nothing rather than guessing. Simpler as well as more correct: the identity lookup subsumes the isWhoop check, because only a WHOOP row can have adopted a WHOOP peripheral id. Two tests, both verified to FAIL against the previous implementation and pass against this one: - a strap switch mid-session stamps the strap that was connected, not the newly-active one - a disconnect from a never-adopted address stamps nothing Verified: full Android suite 4197 tests, 0 failures (--no-build-cache --rerun-tasks); doc lint and i18n audit clean. Swift half via app-build. --- Strand/BLE/SourceCoordinator.swift | 17 +++---- .../java/com/noop/ble/SourceCoordinator.kt | 17 +++---- .../noop/ble/SourceCoordinatorAdoptionTest.kt | 44 +++++++++++++++++++ 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/Strand/BLE/SourceCoordinator.swift b/Strand/BLE/SourceCoordinator.swift index 9ad309a3a3..5b5e87d4cd 100644 --- a/Strand/BLE/SourceCoordinator.swift +++ b/Strand/BLE/SourceCoordinator.swift @@ -489,13 +489,14 @@ final class SourceCoordinator: ObservableObject { // MARK: - Identity adoption - /// Stamp the ACTIVE WHOOP row as seen now, for the disconnect edge where there is no uuid left to - /// resolve against. Guarded on the row still being a WHOOP so a strap-switch mid-session cannot stamp - /// whatever happens to be active now. (#1527) - private func touchActiveWhoopLastSeen() { - let activeId = registry.activeDeviceId - guard isWhoop(activeId) else { return } - registry.touchLastSeen(activeId) + /// Stamp the row belonging to the strap that just DISCONNECTED, resolved by the identity it adopted + /// rather than by whatever is active now. On a multi-WHOOP install those are not the same row: make + /// another strap active while this one is live, and stamping "the active row" would record a sighting + /// of a strap that was never connected — the same mis-mapping the peripheralId guard above exists to + /// prevent. A uuid no row has adopted stamps nothing. (#1527) + private func touchLastSeen(forStrap uuid: String) { + guard let device = registry.device(forPeripheralId: uuid) else { return } + registry.touchLastSeen(device.id) } /// The BLE engine connected to a WHOOP peripheral (`uuid`). Persist that stable identity onto the @@ -524,7 +525,7 @@ final class SourceCoordinator: ObservableObject { // leaving the connect-time value: after a ten-hour overnight session that would have the card // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually // connected — a nil republish with no prior link is not a sighting. (#1527) - if previouslyConnectedTo != nil { touchActiveWhoopLastSeen() } + if let previouslyConnectedTo { touchLastSeen(forStrap: previouslyConnectedTo) } return } diff --git a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt index 233968ebeb..208d79cdde 100644 --- a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt +++ b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt @@ -212,13 +212,14 @@ class SourceCoordinator( } } - /** Stamp the ACTIVE WHOOP row as seen now, for the disconnect edge where there is no address left to - * resolve against. Guarded on the row still being a WHOOP so a strap switch mid-session cannot stamp - * whatever happens to be active now. Twin of the Swift helper. (#1527) */ - private suspend fun touchActiveWhoopLastSeen() { - val activeId = registry.activeDeviceId() ?: return - if (!isWhoop(activeId, registry.all())) return - registry.touchLastSeen(activeId) + /** Stamp the row belonging to the strap that just DISCONNECTED, resolved by the identity it adopted + * rather than by whatever is active now. On a multi-WHOOP install those are not the same row: make + * another strap active while this one is live, and stamping "the active row" would record a sighting of + * a strap that was never connected — the same mis-mapping the peripheralId guard below exists to + * prevent. An address no row has adopted stamps nothing. Twin of the Swift helper. (#1527) */ + private suspend fun touchLastSeenForStrap(address: String) { + val row = registry.deviceForPeripheralId(address) ?: return + registry.touchLastSeen(row.id) } /** @@ -247,7 +248,7 @@ class SourceCoordinator( // reading "Last seen 10 h ago" the moment the link dropped. Only when we were actually // connected — a null republish with no prior link is not a sighting. (#1527) if (previouslyConnectedTo != null) { - scope.launch { runCatching { touchActiveWhoopLastSeen() } } + scope.launch { runCatching { touchLastSeenForStrap(previouslyConnectedTo) } } } return } diff --git a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt index ce46f4799f..ad70bca045 100644 --- a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt +++ b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt @@ -462,6 +462,50 @@ class SourceCoordinatorAdoptionTest { assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt) } + /** + * The disconnect stamps the strap that was CONNECTED, not whatever is active by the time it drops. + * Make another WHOOP active while this one is live — a multi-WHOOP install can — and resolving "the + * active row" would record a sighting of a strap nobody connected. That is the same mis-mapping the + * peripheralId guard exists to prevent, so the disconnect resolves by adopted identity instead. + */ + @Test + fun theDisconnectStampsTheStrapThatWasConnectedNotWhateverIsActiveNow() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["whoop-a"] = whoopRow("whoop-a", peripheralId = "AA:BB:CC:DD:EE:01") + devices["whoop-b"] = whoopRow("whoop-b", peripheralId = "AA:BB:CC:DD:EE:02") + .copy(status = DeviceStatus.paired.name) + } + val coordinator = coordinatorOver(dao) + coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01") + + // whoop-b becomes active while whoop-a is still the live link; rewind both stamps so the + // assertions can only be satisfied by the disconnect's own write. + dao.devices["whoop-a"] = dao.devices["whoop-a"]!!.copy( + status = DeviceStatus.paired.name, lastSeenAt = 100) + dao.devices["whoop-b"] = dao.devices["whoop-b"]!!.copy( + status = DeviceStatus.active.name, lastSeenAt = 100) + + coordinator.connectedPeripheralChanged(null) + + assertTrue("the strap that was connected must be the one stamped", + dao.devices["whoop-a"]!!.lastSeenAt > 100) + assertEquals("a strap that was never connected must not be stamped", + 100, dao.devices["whoop-b"]!!.lastSeenAt) + } + + /** An address no row has adopted stamps nothing rather than guessing at the active row. */ + @Test + fun aDisconnectFromAnUnadoptedStrapStampsNothing() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "AA:BB:CC:DD:EE:01") + } + val coordinator = coordinatorOver(dao) + coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:77") // different strap, never adopted + dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100) + coordinator.connectedPeripheralChanged(null) + assertEquals(100, dao.devices["my-whoop"]!!.lastSeenAt) + } + /** * "Removed - data kept" is a deliberate resting state: a stray connect must not quietly resurrect an * archived row into looking live. The real guard is the query's `AND status != 'archived'`; the fake From cfa2a916b5dfdb3770ad86963a3ddba7adc73907 Mon Sep 17 00:00:00 2001 From: Fanboynz Date: Sat, 22 Aug 2026 10:07:08 +1200 Subject: [PATCH 3/3] fix: resolve the disconnect strap the way the adopt guard does, case-insensitively (Android) Second re-review pass. The connect path treats a stored address differing only in case as the SAME strap -- `existing.equals(address, ignoreCase = true)`, which reads as deliberate, someone saw MAC-case variance -- but the disconnect resolved it through `deviceForPeripheralId`, whose `WHERE peripheralId = ?` is case-SENSITIVE in SQLite. So a case-different address would adopt and stamp on connect, then silently stamp nothing on disconnect: the same quiet staleness this branch exists to remove, reintroduced through a stricter lookup on one edge than the other. Matched to the adjacent guard instead of going through the shared query, which other callers use and which the Swift store mirrors exactly. Deliberately NOT changed on Swift: its connect path compares exactly (`case .some(uuid)`), so its exact lookup already agrees with its own adopt rule. Each side matches ITS OWN rule; making them match each other would break one of them. Also checked this pass and found clean: the Swift publisher is `.removeDuplicates()` before the sink, so a republish of the same uuid cannot write repeatedly. One test, verified to FAIL against the exact-match lookup and pass against this one. Full Android suite 4198 tests, 0 failures (--no-build-cache --rerun-tasks). Swift and Packages are untouched by this commit, so app-build 32530048388 still covers the Apple half. --- .../java/com/noop/ble/SourceCoordinator.kt | 11 +++++++++-- .../noop/ble/SourceCoordinatorAdoptionTest.kt | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt index 208d79cdde..f56d4de32b 100644 --- a/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt +++ b/android/app/src/main/java/com/noop/ble/SourceCoordinator.kt @@ -216,9 +216,16 @@ class SourceCoordinator( * rather than by whatever is active now. On a multi-WHOOP install those are not the same row: make * another strap active while this one is live, and stamping "the active row" would record a sighting of * a strap that was never connected — the same mis-mapping the peripheralId guard below exists to - * prevent. An address no row has adopted stamps nothing. Twin of the Swift helper. (#1527) */ + * prevent. An address no row has adopted stamps nothing. + * + * Matched the way the connect path below matches — `ignoreCase` — and NOT through + * [DeviceRegistry.deviceForPeripheralId], whose `WHERE peripheralId = ?` is case-SENSITIVE in SQLite. + * A stored address differing only in case is the same strap to the guard below, so resolving it more + * strictly here would adopt on connect and then silently stamp nothing on disconnect — the same + * quiet staleness this whole change exists to remove. The Swift twin keeps its exact lookup because + * its connect path compares exactly too; each side matches ITS OWN adopt rule. (#1527) */ private suspend fun touchLastSeenForStrap(address: String) { - val row = registry.deviceForPeripheralId(address) ?: return + val row = registry.all().firstOrNull { it.peripheralId.equals(address, ignoreCase = true) } ?: return registry.touchLastSeen(row.id) } diff --git a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt index ad70bca045..d86a7dba31 100644 --- a/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt +++ b/android/app/src/test/java/com/noop/ble/SourceCoordinatorAdoptionTest.kt @@ -493,6 +493,24 @@ class SourceCoordinatorAdoptionTest { 100, dao.devices["whoop-b"]!!.lastSeenAt) } + /** + * The disconnect must resolve identity the way the connect path does. That comparison is + * `ignoreCase` — a stored address differing only in case is the SAME strap to the adopt guard — so a + * stricter lookup here would adopt on connect and then quietly stamp nothing on disconnect. + */ + @Test + fun aDisconnectResolvesTheStrapCaseInsensitivelyLikeTheAdoptGuard() = runBlocking { + val dao = FakeRegistryDao().apply { + devices["my-whoop"] = whoopRow("my-whoop", peripheralId = "aa:bb:cc:dd:ee:01") + } + val coordinator = coordinatorOver(dao) + coordinator.connectedPeripheralChanged("AA:BB:CC:DD:EE:01") // same strap, upper-cased + dao.devices["my-whoop"] = dao.devices["my-whoop"]!!.copy(lastSeenAt = 100) + coordinator.connectedPeripheralChanged(null) + assertTrue("a case-different address is the same strap to the adopt guard, so it must stamp", + dao.devices["my-whoop"]!!.lastSeenAt > 100) + } + /** An address no row has adopted stamps nothing rather than guessing at the active row. */ @Test fun aDisconnectFromAnUnadoptedStrapStampsNothing() = runBlocking {