From bdfc905216c32c01d452373e1ff9432a4d40ce82 Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 17:04:09 +0700 Subject: [PATCH 1/5] fix(android): close retention-probe audio-loss windows and make the escalated backoff reachable Review of #21404 left three verified P1s against the merged design (independent astra audit concurred; the fourth, stale-proof-consumption on new GATT, was refuted as stated but is hardened here anyway): - Subscribe before probing: fireDeviceReady now requests the audio CCCD write before the optional retention probe, and the probe read is posted through the same looper as subscribeCharacteristic's deferred enqueue, so the probe can never own the serial GATT queue ahead of capture (firmware drops dequeued frames while connected-but-unsubscribed, and a lost read callback has no queue timeout). - Require a valid RTC before certifying retention: retentionProbeSucceeds now demands the status payload's rtc-valid field; the firmware offline writer drops every request while its clock is unsynced, so mounted-but-timeless storage cannot license longer reconnect gaps. - Preserve a completed proof across disconnected retries and invalidate it on each new GATT connection: RetentionProbeLifecycle.interrupt() becomes cancel() (retires an in-flight probe without erasing completed proof), so delayForFailure can actually reach the advertised 6/12/30/60s schedule; the proof resets in onGattConnected before anything on the new link consumes it, closing the rebooted-into-SD-failure window (the refuted finding 3). Tests: RTC pins in BleReconnectBackoffTest, proof-lifecycle pins in RetentionProbeLifecycleTest, and a new source-reading contract test pinning subscribe-before-probe, the deferred probe enqueue, cancel-vs-reset semantics, and connect-time invalidation. Invariant: INV-CAP-1 (product/invariants/pendant-audio-retention.md). --- .../com/friend/ios/ble/BleReconnectBackoff.kt | 12 ++- .../friend/ios/ble/OmiBleForegroundService.kt | 41 ++++++-- .../friend/ios/ble/RetentionProbeLifecycle.kt | 22 ++++- .../friend/ios/ble/BleReconnectBackoffTest.kt | 23 ++++- .../RetentionCaptureOrderingContractTest.kt | 98 +++++++++++++++++++ .../ios/ble/RetentionProbeLifecycleTest.kt | 24 +++-- ...20261014-ble-reconnect-retention-gate.json | 4 + 7 files changed, 196 insertions(+), 28 deletions(-) create mode 100644 app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt create mode 100644 app/changelog/unreleased/20261014-ble-reconnect-retention-gate.json diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt index c15ddc1f3fe..a3ab8a74a11 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt @@ -93,8 +93,12 @@ internal class BleReconnectBackoff( companion object { /** * Firmware status is exactly four LE uint32s: used, unread, free, RTC valid. - * Only a nonzero used + unread + free total proves initialized storage; - * RTC validity alone cannot license backoff. Null includes failed reads. + * A nonzero used + unread + free total proves initialized storage, and the + * RTC-valid field must also be set: the firmware's offline writer drops every + * request while its clock is unsynced (sd_card.c `process_write_data_req`), + * so a mounted-but-timeless device discards the audio a longer reconnect gap + * would strand. RTC validity alone is still not evidence of usable storage. + * Null includes failed reads. */ fun retentionProbeSucceeds(payload: ByteArray?): Boolean { if (payload == null || payload.size != 16) return false @@ -102,8 +106,8 @@ internal class BleReconnectBackoff( val used = status.int.toLong() and 0xffff_ffffL val unread = status.int.toLong() and 0xffff_ffffL val free = status.int.toLong() and 0xffff_ffffL - status.int // RTC validity is not evidence of usable storage. - return used + unread + free > 0L + val rtcValid = status.int != 0 + return rtcValid && used + unread + free > 0L } val DEFAULT_STEPS_MS = longArrayOf(3_000L, 6_000L, 12_000L, 30_000L, 60_000L) diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt index f647f3dfc7a..2363787248e 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt @@ -225,6 +225,15 @@ class OmiBleForegroundService : Service() { backfillTimeToReconnect(addr, managed) managed.retryCount = 0 reconnectBackoff.reset(addr) + // Any retention proof describes the previous GATT link, not this one: + // the device may have rebooted into a storage failure while it was + // away. Reset before anything on this connection can consume the + // stale proof; fireDeviceReady re-proves storage for this link and + // re-licenses the escalated schedule only on a fresh successful read. + synchronized(managed) { + managed.hasVerifiedRetention = false + managed.retentionProbe.resetVerification() + } managed.hasEverConnected = true managed.currentAttemptEstablished = true managed.pendingReconnect?.let { handler.removeCallbacks(it) } @@ -349,9 +358,18 @@ class OmiBleForegroundService : Service() { private fun fireDeviceReady(address: String, services: List) { val addr = address.uppercase() + // Audio subscription goes first, always. Firmware drops dequeued frames + // while it is connected but not subscribed (transport.c), so anything + // this function puts on the GATT command queue before the CCCD write + // widens the connected-but-deaf window, and a wedged head command would + // block capture for the whole link. The retention probe is an optional + // battery optimization; capture is the product. probeRetentionRuntime + // posts its enqueue behind this subscription's own post, so the CCCD + // write owns the serial queue first and a slow or lost probe read can + // never delay it. + ensureBackgroundAudioSubscription(addr, services) recordVerifiedRetention(addr, services) probeRetentionRuntime(addr) - ensureBackgroundAudioSubscription(addr, services) limitlessDrainEngine.onDeviceReady(addr) if (OmiBleManager.isFlutterAlive) { bleManager.mainHandler.post { @@ -427,10 +445,18 @@ class OmiBleForegroundService : Service() { } } } - try { - bleManager.readCharacteristic(address, STORAGE_DRAIN_SERVICE_UUID, STORAGE_DRAIN_READ_CHAR_UUID, completeProbe) - } catch (e: Exception) { - completeProbe(Result.failure(e)) + // subscribeCharacteristic defers its enqueue through mainHandler, so the + // probe read must defer the same way: enqueueing it directly from + // fireDeviceReady would win the race and take the serial queue ahead of + // the audio CCCD write. A post into the same looper keeps this behind + // the subscription unconditionally. If the link dropped before the post + // runs, readCharacteristic fails the completion and the probe retires. + bleManager.mainHandler.post { + try { + bleManager.readCharacteristic(address, STORAGE_DRAIN_SERVICE_UUID, STORAGE_DRAIN_READ_CHAR_UUID, completeProbe) + } catch (e: Exception) { + completeProbe(Result.failure(e)) + } } } @@ -438,7 +464,10 @@ class OmiBleForegroundService : Service() { synchronized(managed) { managed.retentionProbeTimeout?.let { handler.removeCallbacks(it) } managed.retentionProbeTimeout = null - managed.retentionProbe.interrupt() + // Cancel the in-flight probe only. A completed verification survives + // this link's disconnection: retry selection runs after the link is + // gone and still needs the proof to keep the escalated schedule. + managed.retentionProbe.cancel() } } diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/RetentionProbeLifecycle.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/RetentionProbeLifecycle.kt index 6f14c50dc4c..11e2fdefc57 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/RetentionProbeLifecycle.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/RetentionProbeLifecycle.kt @@ -1,6 +1,17 @@ package com.friend.ios.ble -/** Per-device probe ownership. Callers serialize transitions with their device lock. */ +/** + * Per-device retention probe ownership and its completed proof. Callers + * serialize transitions with their device lock. + * + * Proof lifecycle (INV-CAP-1): a successful read verifies the GATT link it ran + * on, and that proof must survive the disconnected retry attempts that follow — + * a proof erased at every disconnect makes the escalated schedule unreachable + * because retry selection always runs after the link is already gone. Each NEW + * connection resets the proof before any retry logic on the new link can + * consume it, and reads retired by a replacement connection can never verify + * their successor. + */ internal class RetentionProbeLifecycle { class Probe internal constructor(internal val connection: Any) @@ -9,6 +20,8 @@ internal class RetentionProbeLifecycle { @Volatile var runtimeVerified: Boolean = false private set + /** Clears the completed proof. Called when a new GATT connects and when a + * fresh ready event must re-prove storage for its own link. */ fun resetVerification() { runtimeVerified = false } @@ -21,13 +34,14 @@ internal class RetentionProbeLifecycle { fun timeout(probe: Probe): Boolean { if (activeProbe !== probe) return false - interrupt() + cancel() return true } - fun interrupt() { + /** Retires an in-flight probe only. A completed verification survives: + * it is reset when the next GATT connects, not when this one drops. */ + fun cancel() { activeProbe = null - resetVerification() } /** Retired attempts and reads from a replaced connection cannot affect a fresh probe. */ diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt index e6272d744ae..00dd1c1f2e5 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt @@ -134,9 +134,22 @@ class BleReconnectBackoffTest { assertFlatCadenceWithProbe(storageStatus(rtc = 1)) } + @Test + fun `mounted but timeless storage keeps flat cadence even with data on the ring`() { + // INV-CAP-1: the firmware's offline writer drops every request while its + // RTC is unsynced (sd_card.c process_write_data_req), so nonzero storage + // without a valid clock cannot license longer reconnect gaps. + assertFalse(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(free = 4096))) + assertFalse(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = 5, unread = 5, free = 100))) + val backoff = BleReconnectBackoff() + repeat(5) { + assertEquals(3_000L, backoff.delayForFailure("A", false, null, BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = 5, free = 100)))) + } + } + @Test fun `nonzero free bytes license escalation only with advertisement`() { - val runtimeVerified = BleReconnectBackoff.retentionProbeSucceeds(storageStatus(free = 4096)) + val runtimeVerified = BleReconnectBackoff.retentionProbeSucceeds(storageStatus(free = 4096, rtc = 1)) assertTrue(runtimeVerified) val backoff = BleReconnectBackoff() val advertised = true @@ -152,11 +165,11 @@ class BleReconnectBackoffTest { @Test fun `used or unread status proves storage and uint32 totals do not overflow`() { - assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = 1))) - assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(unread = 1))) - assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(free = Int.MIN_VALUE))) + assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = 1, rtc = 1))) + assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(unread = 1, rtc = 1))) + assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(free = Int.MIN_VALUE, rtc = 1))) // Unsigned sum is 2^32, which would wrap to zero in Int arithmetic. - assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = -1, free = 1))) + assertTrue(BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = -1, free = 1, rtc = 1))) } private fun assertFlatCadenceWithProbe(payload: ByteArray?) { diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt new file mode 100644 index 00000000000..a8f43b5d2b3 --- /dev/null +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt @@ -0,0 +1,98 @@ +package com.friend.ios.ble + +import java.io.File +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Pins the capture-first invariants of the BLE foreground service against the + * real Kotlin source. The retention probe is an optional battery optimization; + * the audio CCCD write is the product, so no probe work may precede or delay + * it on the serial GATT queue (AGENTS.md: never lose user audio), and a + * completed retention proof may only be consumed by the connection it was + * proven on. + */ +class RetentionCaptureOrderingContractTest { + private fun serviceSource(): String { + val relativePath = "app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt" + return generateSequence(File(System.getProperty("user.dir") ?: ".").absoluteFile) { it.parentFile } + .map { File(it, relativePath) } + .firstOrNull { it.isFile } + ?.readText() + ?: error("Could not locate $relativePath") + } + + private fun lifecycleSource(): String { + val relativePath = "app/android/app/src/main/kotlin/com/friend/ios/ble/RetentionProbeLifecycle.kt" + return generateSequence(File(System.getProperty("user.dir") ?: ".").absoluteFile) { it.parentFile } + .map { File(it, relativePath) } + .firstOrNull { it.isFile } + ?.readText() + ?: error("Could not locate $relativePath") + } + + @Test + fun `fireDeviceReady subscribes audio before any retention work`() { + val source = serviceSource() + val body = source.substringAfter("private fun fireDeviceReady").substringBefore("private fun ensureBackgroundAudioSubscription") + val subscribe = body.indexOf("ensureBackgroundAudioSubscription(addr, services)") + val record = body.indexOf("recordVerifiedRetention(addr, services)") + val probe = body.indexOf("probeRetentionRuntime(addr)") + assertTrue("audio subscription must run inside fireDeviceReady", subscribe >= 0) + assertTrue("retention advertisement recording must run inside fireDeviceReady", record >= 0) + assertTrue("retention probe must run inside fireDeviceReady", probe >= 0) + assertTrue("audio subscription must precede retention advertisement recording", subscribe < record) + assertTrue("audio subscription must precede the retention probe read", subscribe < probe) + } + + @Test + fun `retention probe read cannot join the serial queue ahead of the audio CCCD write`() { + val source = serviceSource() + // The probe enqueue is posted through the same looper the subscription + // uses, so it always lands behind subscribeCharacteristic's deferred + // enqueue; a direct readCharacteristic call would win that race. + val body = source.substringAfter("private fun probeRetentionRuntime").substringBefore("private fun retireRetentionProbe") + assertTrue( + "probe read must be deferred via mainHandler.post behind the subscription enqueue", + body.contains("mainHandler.post"), + ) + val directCall = Regex("readCharacteristic\\((address|addr)").findAll(body).toList() + assertTrue( + "probe read must not be enqueued directly from the ready path", + directCall.all { match -> + val prefix = body.substringBefore(match.range.first) + prefix.substringAfterLast("mainHandler.post", "").isNotEmpty() + }, + ) + } + + @Test + fun `disconnection cancels the in-flight probe without erasing a completed proof`() { + val lifecycle = lifecycleSource() + val cancel = lifecycle.substringAfter("fun cancel()").substringBefore("/** Retired attempts") + assertTrue("cancel() must not clear the completed verification", !cancel.contains("resetVerification()")) + val service = serviceSource() + val retire = service.substringAfter("private fun retireRetentionProbe").substringBefore("private fun mapGattServices") + assertTrue("disconnection must not reset verification", !retire.contains("resetVerification()")) + assertTrue("disconnection must cancel only the in-flight probe", retire.contains("retentionProbe.cancel()")) + } + + @Test + fun `a new GATT connection invalidates the stale retention proof before retry logic can consume it`() { + val service = serviceSource() + val connect = service.substringAfter("override fun onGattConnected").substringBefore("override fun onGattDisconnected") + val reset = connect.indexOf("retentionProbe.resetVerification()") + val retryReset = connect.indexOf("reconnectBackoff.reset(addr)") + assertTrue("connect must reset the stale retention proof", reset >= 0) + assertTrue("connect must also clear the advertisement flag", connect.contains("hasVerifiedRetention = false")) + // The proof reset sits with the pacing reset, before discovery is even + // requested, so a pre-ready disconnect cannot consume the old proof. + assertTrue("proof reset must not come after pacing reset", reset > retryReset) + val backoff = BleReconnectBackoff() + // And the predicate it gates: RTC-less storage never licenses escalation. + val timeless = java.nio.ByteBuffer.allocate(16).order(java.nio.ByteOrder.LITTLE_ENDIAN) + .putInt(0).putInt(0).putInt(4096).putInt(0).array() + assertEquals(3_000L, backoff.delayForFailure("AA", false, null, BleReconnectBackoff.retentionProbeSucceeds(timeless))) + } +} diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionProbeLifecycleTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionProbeLifecycleTest.kt index a07f5e4bf62..093e4ca40a4 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionProbeLifecycleTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionProbeLifecycleTest.kt @@ -9,7 +9,7 @@ import org.junit.Test class RetentionProbeLifecycleTest { private val storageStatus = java.nio.ByteBuffer.allocate(16).order(java.nio.ByteOrder.LITTLE_ENDIAN) - .putInt(0).putInt(0).putInt(4096).putInt(0).array() + .putInt(0).putInt(0).putInt(4096).putInt(1).array() @Test fun `missing callback times out and next probe can license escalation`() { @@ -27,11 +27,11 @@ class RetentionProbeLifecycleTest { } @Test - fun `disconnect interrupts advertised probe and only replacement GATT result licenses escalation`() { + fun `cancelling an in-flight probe keeps flat cadence and only a fresh result verifies`() { val state = RetentionProbeLifecycle() val oldGatt = Any() val oldProbe = state.start(oldGatt)!! - state.interrupt() + state.cancel() assertFalse(state.inFlight) assertFlat(state) val newGatt = Any() @@ -44,16 +44,22 @@ class RetentionProbeLifecycleTest { } @Test - fun `disconnect resets previously verified retention and requires a fresh probe`() { + fun `completed proof survives disconnected retries until the next connection resets it`() { val state = RetentionProbeLifecycle() - val oldGatt = Any() - assertTrue(state.result(state.start(oldGatt)!!, oldGatt, true, storageStatus)) + val gatt = Any() + assertTrue(state.result(state.start(gatt)!!, gatt, true, storageStatus)) assertTrue(state.runtimeVerified) - state.interrupt() + // The link drops: the service cancels the (already completed) probe and + // retry selection runs while disconnected. The proof must survive both + // or the escalated schedule is unreachable. + state.cancel() assertFalse(state.inFlight) + assertEscalates(state) + // A replacement GATT connects: the stale proof is invalidated before + // anything on the new link consumes it, and a fresh probe re-licenses. + state.resetVerification() assertFlat(state) - val newGatt = Any() - assertTrue(state.result(state.start(newGatt)!!, newGatt, true, storageStatus)) + assertTrue(state.result(state.start(gatt)!!, gatt, true, storageStatus)) assertEscalates(state) } diff --git a/app/changelog/unreleased/20261014-ble-reconnect-retention-gate.json b/app/changelog/unreleased/20261014-ble-reconnect-retention-gate.json new file mode 100644 index 00000000000..20d83f84c3b --- /dev/null +++ b/app/changelog/unreleased/20261014-ble-reconnect-retention-gate.json @@ -0,0 +1,4 @@ +{ + "change": "Android stays connected and catches up faster: your Omi is contacted again within seconds after leaving and returning to range, and its offline recordings are only trusted once the device proves it can keep them", + "platforms": ["android"] +} From 5cb80de2c208915b233c30998ddc06034733746c Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 17:28:08 +0700 Subject: [PATCH 2/5] fix(test): correct substring API misuse and drop a brittle ordering assertion in the retention contract test --- .../ios/ble/RetentionCaptureOrderingContractTest.kt | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt index a8f43b5d2b3..8479864de85 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt @@ -61,7 +61,7 @@ class RetentionCaptureOrderingContractTest { assertTrue( "probe read must not be enqueued directly from the ready path", directCall.all { match -> - val prefix = body.substringBefore(match.range.first) + val prefix = body.substring(0, match.range.first) prefix.substringAfterLast("mainHandler.post", "").isNotEmpty() }, ) @@ -82,13 +82,8 @@ class RetentionCaptureOrderingContractTest { fun `a new GATT connection invalidates the stale retention proof before retry logic can consume it`() { val service = serviceSource() val connect = service.substringAfter("override fun onGattConnected").substringBefore("override fun onGattDisconnected") - val reset = connect.indexOf("retentionProbe.resetVerification()") - val retryReset = connect.indexOf("reconnectBackoff.reset(addr)") - assertTrue("connect must reset the stale retention proof", reset >= 0) + assertTrue("connect must reset the stale retention proof", connect.contains("retentionProbe.resetVerification()")) assertTrue("connect must also clear the advertisement flag", connect.contains("hasVerifiedRetention = false")) - // The proof reset sits with the pacing reset, before discovery is even - // requested, so a pre-ready disconnect cannot consume the old proof. - assertTrue("proof reset must not come after pacing reset", reset > retryReset) val backoff = BleReconnectBackoff() // And the predicate it gates: RTC-less storage never licenses escalation. val timeless = java.nio.ByteBuffer.allocate(16).order(java.nio.ByteOrder.LITTLE_ENDIAN) From 4daad82767569b65704a758b13e6dbfea8d78bbf Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 17:40:34 +0700 Subject: [PATCH 3/5] fix(android): gate the probe behind a queued subscription and re-probe after clock sync Addresses the fresh review round on this PR (P1 x2, P2 x1): - fireDeviceReady now tracks whether the native audio subscription was queued. With no native target and Flutter alive, the retention probe is posted behind the onDeviceReady notification, so Dart's own subscription (queued from that callback) reaches the serial GATT queue before the probe read. With neither native target nor Flutter, no subscription can ever arrive, so the probe cannot delay one. - The probe failure shape 'mounted storage with RTC still unsynced' schedules ONE bounded re-probe 20s after the first (Dart performs performSyncTime after onDeviceReady). Budget is per-link, reset on each new GATT connection. - The third finding (invalidate at attempt start) is rejected: it contradicts the original review's own prescription and re-breaks reachability of the escalated schedule (the finding this PR fixes); the proof boundary stays 'new GATT connection', as prescribed. Tests: contract pins for the Flutter-path deferral and the re-probe budget; backoff pins for storageStatusHasDataWithoutClock. --- .../com/friend/ios/ble/BleReconnectBackoff.kt | 15 +++++ .../friend/ios/ble/OmiBleForegroundService.kt | 62 ++++++++++++++++--- .../friend/ios/ble/BleReconnectBackoffTest.kt | 6 ++ .../RetentionCaptureOrderingContractTest.kt | 21 +++++++ 4 files changed, 94 insertions(+), 10 deletions(-) diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt index a3ab8a74a11..e1f9bcdbb52 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/BleReconnectBackoff.kt @@ -110,6 +110,21 @@ internal class BleReconnectBackoff( return rtcValid && used + unread + free > 0L } + /** + * True when the status payload shows mounted storage (nonzero + * used+unread+free) whose RTC-valid field is still zero — the one + * failure shape a post-clock-sync re-probe can legitimately flip. + */ + fun storageStatusHasDataWithoutClock(payload: ByteArray?): Boolean { + if (payload == null || payload.size != 16) return false + val buffer = java.nio.ByteBuffer.wrap(payload).order(java.nio.ByteOrder.LITTLE_ENDIAN) + val used = buffer.int.toLong() and 0xffff_ffffL + val unread = buffer.int.toLong() and 0xffff_ffffL + val free = buffer.int.toLong() and 0xffff_ffffL + val rtcValid = buffer.int != 0 + return !rtcValid && used + unread + free > 0L + } + val DEFAULT_STEPS_MS = longArrayOf(3_000L, 6_000L, 12_000L, 30_000L, 60_000L) const val FASTEST_DELAY_MS = 3_000L } diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt index 2363787248e..c3f2870ed2e 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt @@ -48,6 +48,11 @@ class OmiBleForegroundService : Service() { private const val MTU_SIZE = 512 private const val STABILITY_TIMER_MS = 60_000L private const val RETENTION_PROBE_TIMEOUT_MS = 10_000L + + /** Dart performs performSyncTime after onDeviceReady; one re-probe after + * this window covers a factory-reset pendant whose first probe failed + * only on RTC validity. Bounded: a single delayed run per failed probe. */ + private const val RETENTION_REPROBE_DELAY_MS = 20_000L // Base delay for CCCD-timeout recovery. Ordinary out-of-range retries use // reconnectBackoff so a fixed 3s loop cannot spin the radio forever. private const val RECONNECT_DELAY_MS = 3_000L @@ -172,7 +177,10 @@ class OmiBleForegroundService : Service() { * callback reports CONNECTED. Used to distinguish fail-to-connect from * an established link dropping. Independent of hasEverConnected which * tracks the full device lifetime. */ - var currentAttemptEstablished: Boolean = false + var currentAttemptEstablished: Boolean = false, + /** Post-clock-sync re-probes already run on the current link; one per + * connection, reset in onGattConnected. */ + var retentionReprobes: Int = 0 ) { internal val reconnectDiagnostics = BleReconnectDiagnostics() internal val retentionProbe = RetentionProbeLifecycle() @@ -233,6 +241,7 @@ class OmiBleForegroundService : Service() { synchronized(managed) { managed.hasVerifiedRetention = false managed.retentionProbe.resetVerification() + managed.retentionReprobes = 0 } managed.hasEverConnected = true managed.currentAttemptEstablished = true @@ -363,37 +372,48 @@ class OmiBleForegroundService : Service() { // this function puts on the GATT command queue before the CCCD write // widens the connected-but-deaf window, and a wedged head command would // block capture for the whole link. The retention probe is an optional - // battery optimization; capture is the product. probeRetentionRuntime - // posts its enqueue behind this subscription's own post, so the CCCD - // write owns the serial queue first and a slow or lost probe read can - // never delay it. - ensureBackgroundAudioSubscription(addr, services) + // battery optimization; capture is the product. + val nativeSubscribed = ensureBackgroundAudioSubscription(addr, services) recordVerifiedRetention(addr, services) - probeRetentionRuntime(addr) limitlessDrainEngine.onDeviceReady(addr) if (OmiBleManager.isFlutterAlive) { bleManager.mainHandler.post { bleManager.flutterApi?.onDeviceReady(addr, services) {} } } + // The probe read must never reach the serial GATT queue ahead of an + // audio subscription. With a native target the subscription is already + // queued above; without one, Dart queues its own subscription from the + // onDeviceReady post above, so posting the probe after that post keeps + // it behind. With neither (no native target, Flutter gone) no + // subscription can ever arrive on this link, so the probe cannot delay + // one. probeRetentionRuntime still defers its enqueue through the same + // looper as subscribeCharacteristic. + when { + nativeSubscribed -> probeRetentionRuntime(addr) + OmiBleManager.isFlutterAlive -> bleManager.mainHandler.post { probeRetentionRuntime(addr) } + else -> probeRetentionRuntime(addr) + } } - private fun ensureBackgroundAudioSubscription(address: String, services: List) { + /** Returns true when this call queued an audio CCCD write for the device. */ + private fun ensureBackgroundAudioSubscription(address: String, services: List): Boolean { // Subscribe for background streaming OR batch capture (whichever is configured) // so native keeps receiving audio after the Flutter engine is gone. Never deferred // to Dart: enabling notifications is idempotent, and waiting on an engine that is // gone left the characteristic subscribed by nobody (issue #10847). val target = backgroundAudioStreamer.configuredAudioTargetFor(address) ?: batchAudioWriter.configuredAudioTargetFor(address) - ?: return + ?: return false val hasTarget = services.any { service -> service.uuid.equals(target.first, ignoreCase = true) && service.characteristicUuids.any { it.equals(target.second, ignoreCase = true) } } - if (!hasTarget) return + if (!hasTarget) return false Log.i(TAG, "Ensuring BLE audio subscription for background transcription on $address") bleManager.subscribeCharacteristic(address, target.first, target.second) + return true } /** INV-CAP-1 requires both advertisement and a successful runtime storage probe. */ @@ -440,6 +460,18 @@ class OmiBleForegroundService : Service() { if (managed.retentionRuntimeVerified) { Log.i(TAG, "Verified runtime offline retention for $address (storage status nonzero)") } else { + val payload = result.getOrNull() + if (payload != null && BleReconnectBackoff.storageStatusHasDataWithoutClock(payload) && + managed.retentionReprobes < 1 + ) { + // Mounted storage with an unsynced clock: Dart performs + // performSyncTime after onDeviceReady, so the same link + // can legitimately gain a valid RTC. One bounded re-probe + // after that sync window keeps the schedule reachable for + // factory-reset devices without weakening the RTC gate. + managed.retentionReprobes += 1 + scheduleRetentionReprobe(address) + } Log.w(TAG, "Runtime offline retention unverified for $address; keeping flat retry cadence: " + (result.exceptionOrNull()?.message ?: "invalid or all-zero storage status")) } @@ -471,6 +503,16 @@ class OmiBleForegroundService : Service() { } } + /** One re-probe on the CURRENT link after Dart's post-ready clock sync. + * Replaced links are ignored: the guard inside probeRetentionRuntime + * rejects a GATT that no longer matches, and a new link probes fresh. */ + private fun scheduleRetentionReprobe(address: String) { + handler.postDelayed({ + val managed = managedDevices[address.uppercase()] ?: return@postDelayed + probeRetentionRuntime(address) + }, RETENTION_REPROBE_DELAY_MS) + } + private fun mapGattServices(services: List): List = services.map { service -> BleService( diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt index 00dd1c1f2e5..1e900a0b4c3 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/BleReconnectBackoffTest.kt @@ -145,6 +145,12 @@ class BleReconnectBackoffTest { repeat(5) { assertEquals(3_000L, backoff.delayForFailure("A", false, null, BleReconnectBackoff.retentionProbeSucceeds(storageStatus(used = 5, free = 100)))) } + // The one failure shape a post-clock-sync re-probe can flip is detected: + // mounted storage whose RTC-valid field is still zero. + assertTrue(BleReconnectBackoff.storageStatusHasDataWithoutClock(storageStatus(used = 5, unread = 5, free = 100))) + assertFalse(BleReconnectBackoff.storageStatusHasDataWithoutClock(storageStatus(used = 5, free = 100, rtc = 1))) + assertFalse(BleReconnectBackoff.storageStatusHasDataWithoutClock(ByteArray(16))) + assertFalse(BleReconnectBackoff.storageStatusHasDataWithoutClock(null)) } @Test diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt index 8479864de85..fcabd1cb8f2 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt @@ -44,6 +44,13 @@ class RetentionCaptureOrderingContractTest { assertTrue("retention probe must run inside fireDeviceReady", probe >= 0) assertTrue("audio subscription must precede retention advertisement recording", subscribe < record) assertTrue("audio subscription must precede the retention probe read", subscribe < probe) + // The probe is gated on an audio subscription being queued: with no + // native target and Flutter alive, the probe waits behind Dart's + // onDeviceReady post so Dart's own subscription can win the queue. + assertTrue( + "probe must be deferred behind the Flutter onDeviceReady post when no native subscription was queued", + body.contains("OmiBleManager.isFlutterAlive -> bleManager.mainHandler.post { probeRetentionRuntime(addr) }"), + ) } @Test @@ -84,10 +91,24 @@ class RetentionCaptureOrderingContractTest { val connect = service.substringAfter("override fun onGattConnected").substringBefore("override fun onGattDisconnected") assertTrue("connect must reset the stale retention proof", connect.contains("retentionProbe.resetVerification()")) assertTrue("connect must also clear the advertisement flag", connect.contains("hasVerifiedRetention = false")) + assertTrue("connect must reset the bounded re-probe budget", connect.contains("retentionReprobes = 0")) val backoff = BleReconnectBackoff() // And the predicate it gates: RTC-less storage never licenses escalation. val timeless = java.nio.ByteBuffer.allocate(16).order(java.nio.ByteOrder.LITTLE_ENDIAN) .putInt(0).putInt(0).putInt(4096).putInt(0).array() assertEquals(3_000L, backoff.delayForFailure("AA", false, null, BleReconnectBackoff.retentionProbeSucceeds(timeless))) } + + @Test + fun `the post-clock-sync re-probe is bounded to one per connection`() { + val service = serviceSource() + assertTrue( + "re-probe scheduling must be guarded by the per-link budget", + service.contains("managed.retentionReprobes < 1"), + ) + assertTrue( + "scheduling a re-probe must consume the budget", + service.contains("managed.retentionReprobes += 1"), + ) + } } From 5288ac400af1fc6a05a48c313d9b9d336768cfce Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 17:54:49 +0700 Subject: [PATCH 4/5] fix(android): bind retention re-probes to their link and make the chain self-healing Round-3 review: a stale re-probe callback could fire on a successor GATT and clear its fresh proof (probeRetentionRuntime captures the current GATT), and a single fixed-delay re-probe could expire before Dart's connect-time GATT work finishes, wasting the per-link budget. Re-probes now carry the originating GATT and attempt number: a replaced link rejects the callback outright, and the chain re-arms only while the probe keeps seeing mounted-storage-without- clock, up to 3 cycles per link (budget reset on each new GATT connection). The RTC gate itself is unchanged. --- .../friend/ios/ble/OmiBleForegroundService.kt | 39 ++++++++++++------- .../RetentionCaptureOrderingContractTest.kt | 12 ++++-- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt index c3f2870ed2e..ebfd74b91ef 100644 --- a/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt +++ b/app/android/app/src/main/kotlin/com/friend/ios/ble/OmiBleForegroundService.kt @@ -49,10 +49,11 @@ class OmiBleForegroundService : Service() { private const val STABILITY_TIMER_MS = 60_000L private const val RETENTION_PROBE_TIMEOUT_MS = 10_000L - /** Dart performs performSyncTime after onDeviceReady; one re-probe after - * this window covers a factory-reset pendant whose first probe failed - * only on RTC validity. Bounded: a single delayed run per failed probe. */ + /** Dart performs performSyncTime after onDeviceReady; re-probes after + * this window cover a factory-reset pendant whose first probe failed + * only on RTC validity. Bounded chain per link, reset on new GATT. */ private const val RETENTION_REPROBE_DELAY_MS = 20_000L + private const val RETENTION_MAX_REPROBES = 3 // Base delay for CCCD-timeout recovery. Ordinary out-of-range retries use // reconnectBackoff so a fixed 3s loop cannot spin the radio forever. private const val RECONNECT_DELAY_MS = 3_000L @@ -461,16 +462,18 @@ class OmiBleForegroundService : Service() { Log.i(TAG, "Verified runtime offline retention for $address (storage status nonzero)") } else { val payload = result.getOrNull() + val attempt = synchronized(managed) { managed.retentionReprobes + 1 } if (payload != null && BleReconnectBackoff.storageStatusHasDataWithoutClock(payload) && - managed.retentionReprobes < 1 + attempt <= RETENTION_MAX_REPROBES ) { // Mounted storage with an unsynced clock: Dart performs // performSyncTime after onDeviceReady, so the same link - // can legitimately gain a valid RTC. One bounded re-probe - // after that sync window keeps the schedule reachable for - // factory-reset devices without weakening the RTC gate. - managed.retentionReprobes += 1 - scheduleRetentionReprobe(address) + // can legitimately gain a valid RTC. Bounded chain of + // re-probes on THIS link covers a factory-reset pendant + // whose connect-time work outruns the first window, + // without weakening the RTC gate. + synchronized(managed) { managed.retentionReprobes = attempt } + scheduleRetentionReprobe(address, gatt, attempt) } Log.w(TAG, "Runtime offline retention unverified for $address; keeping flat retry cadence: " + (result.exceptionOrNull()?.message ?: "invalid or all-zero storage status")) @@ -503,12 +506,20 @@ class OmiBleForegroundService : Service() { } } - /** One re-probe on the CURRENT link after Dart's post-ready clock sync. - * Replaced links are ignored: the guard inside probeRetentionRuntime - * rejects a GATT that no longer matches, and a new link probes fresh. */ - private fun scheduleRetentionReprobe(address: String) { + /** Re-probe on the link that scheduled it. Replaced links are rejected + * (a stale callback must not capture a successor GATT or clear its fresh + * proof), and the chain self-limits: each cycle arms only when the probe + * again sees the mounted-storage-without-clock shape, up to + * RETENTION_MAX_REPROBES per link, reset on each new GATT connection. */ + private fun scheduleRetentionReprobe(address: String, gatt: android.bluetooth.BluetoothGatt, attempt: Int) { + if (attempt > RETENTION_MAX_REPROBES) return handler.postDelayed({ - val managed = managedDevices[address.uppercase()] ?: return@postDelayed + val addr = address.uppercase() + val managed = managedDevices[addr] ?: return@postDelayed + if (bleManager.connectedGatts[addr] !== gatt) return@postDelayed + synchronized(managed) { + if (managed.retentionReprobes < attempt) return@postDelayed + } probeRetentionRuntime(address) }, RETENTION_REPROBE_DELAY_MS) } diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt index fcabd1cb8f2..b3f071f52ae 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt @@ -100,15 +100,21 @@ class RetentionCaptureOrderingContractTest { } @Test - fun `the post-clock-sync re-probe is bounded to one per connection`() { + fun `the post-clock-sync re-probe is bounded and bound to its originating link`() { val service = serviceSource() assertTrue( "re-probe scheduling must be guarded by the per-link budget", - service.contains("managed.retentionReprobes < 1"), + service.contains("attempt <= RETENTION_MAX_REPROBES"), ) assertTrue( "scheduling a re-probe must consume the budget", - service.contains("managed.retentionReprobes += 1"), + service.contains("managed.retentionReprobes = attempt"), + ) + // A stale callback must not fire on a successor GATT: the scheduled + // probe checks the current connection against the originating GATT. + assertTrue( + "re-probe must be bound to its originating GATT", + service.contains("if (bleManager.connectedGatts[addr] !== gatt) return@postDelayed"), ) } } From d0da81a7fb926e1c87f9ea54517c442661e8c4a9 Mon Sep 17 00:00:00 2001 From: David Zhang Date: Wed, 14 Oct 2026 18:34:56 +0700 Subject: [PATCH 5/5] test: pin the finding-3 refutation (probe retired before retry selection) for the re-land record --- .../RetentionCaptureOrderingContractTest.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt index b3f071f52ae..7085228b48e 100644 --- a/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt +++ b/app/android/app/src/test/kotlin/com/friend/ios/ble/RetentionCaptureOrderingContractTest.kt @@ -99,6 +99,32 @@ class RetentionCaptureOrderingContractTest { assertEquals(3_000L, backoff.delayForFailure("AA", false, null, BleReconnectBackoff.retentionProbeSucceeds(timeless))) } + @Test + fun `the disconnect path retires the in-flight probe before retry logic consumes verification`() { + // Refutation pin for the original finding 3 ("stale proof consumed on a + // pre-ready disconnect"): handleDisconnection calls retireRetentionProbe + // BEFORE handleRetryLogic, so at retry-selection time an in-flight probe + // is already cancelled on this path — a never-completed probe cannot be + // consumed, and RetentionProbeLifecycle.result rejects retired probes. + val service = serviceSource() + val disconnect = service.substringAfter("private fun handleDisconnection").substringBefore("private fun handleRetryLogic") + val retire = disconnect.indexOf("retireRetentionProbe(managed)") + val retry = disconnect.indexOf("handleRetryLogic(addr, status)") + assertTrue("disconnect must retire the probe", retire >= 0) + assertTrue("disconnect must select the retry delay", retry >= 0) + assertTrue("probe retirement must precede retry selection", retire < retry) + // And a probe cancelled before completion can never verify anything. + val state = RetentionProbeLifecycle() + val gatt = Any() + val probe = state.start(gatt)!! + state.cancel() + assertTrue(!state.result(probe, gatt, true, storageStatusWithClock)) + assertEquals(false, state.runtimeVerified) + } + + private val storageStatusWithClock = java.nio.ByteBuffer.allocate(16).order(java.nio.ByteOrder.LITTLE_ENDIAN) + .putInt(0).putInt(0).putInt(4096).putInt(1).array() + @Test fun `the post-clock-sync re-probe is bounded and bound to its originating link`() { val service = serviceSource()