ble: let the bond give-up see an unanswered handshake

android/app/src/main/java/com/noop/ble/BondRefusalGiveUp.kt

@@ -78,6 +78,25 @@ class BondRefusalGiveUp(
                 "strap in pairing mode (tap until the LEDs flash blue), and if it is listed in your Bluetooth " +
                 "settings choose Forget This Device. Then tap Connect to try again."
 
+        /**
+         * The paused hint for a bond that failed WITHOUT the strap ever answering (#1635).
+         *
+         * [pausedHint] names a cause — the strap still held by the official WHOOP app, or a stale OS
+         * pairing — which is well founded when the refusal arrived as INSUFFICIENT_AUTHENTICATION or
+         * INSUFFICIENT_ENCRYPTION: the strap actively said no. It is NOT founded when the CLIENT_HELLO
+         * simply goes unanswered and the link drops on a timer, which is a different observation with
+         * several possible causes. Telling that user to close the WHOOP app would be a guess dressed as
+         * instruction, and if it is wrong they have no way to know.
+         *
+         * So this describes what was observed and offers the one action that is definitely theirs to
+         * take, without asserting why. Pure; no em-dash.
+         */
+        fun pausedHintHandshakeUnanswered(): String =
+            "NOOP stopped retrying because the secure handshake with your strap never completes: the " +
+                "strap does not answer, and the link drops a few seconds later. Auto-reconnect is paused " +
+                "so it stops draining both batteries. Tap Connect to try again, and if it keeps happening " +
+                "please share your strap log."
+
         /**
          * #750: a short OPAQUE token for the epitaph, derived from the strap's device id.
          *

android/app/src/main/java/com/noop/ble/BondStateTraceHelpers.kt

@@ -0,0 +1,30 @@
+package com.noop.ble
+
+import com.noop.protocol.DeviceFamily
+
+/**
+ * Does this disconnect count as the strap refusing to bond?
+ *
+ * Two observations mean the same thing for the purpose of giving up: the strap answered the bond write
+ * with INSUFFICIENT_AUTHENTICATION / INSUFFICIENT_ENCRYPTION, or it never answered the CLIENT_HELLO at
+ * all. The second was invisible to the give-up until now, because the gate tested only the status — and
+ * an unanswered handshake presents as a plain local terminate (status 22). The consequence, measured on
+ * a real strap: eight consecutive connect attempts all logging "attempt 1", retrying every ~11 seconds
+ * indefinitely, because the streak never incremented and the give-up never latched.
+ *
+ * Deliberately does NOT widen what counts as an AUTH refusal — [helloUnacked] is a separate signal
+ * carried separately, so the user-facing guidance can stay honest about which was observed. An auth
+ * refusal supports naming a cause; an unanswered handshake does not.
+ *
+ * Pure so the rule is unit-tested without a radio.
+ */
+internal fun countsAsBondRefusal(
+    isAuthRefusalStatus: Boolean,
+    helloUnacked: Boolean,
+    alreadyBonded: Boolean,
+    family: DeviceFamily,
+): Boolean {
+    if (alreadyBonded) return false                 // bonded already — not a pairing problem
+    if (family != DeviceFamily.WHOOP5) return false // the 4.0 bonds cleanly; this is the 5/MG handshake
+    return isAuthRefusalStatus || helloUnacked
+}

android/app/src/main/java/com/noop/ble/WhoopBleClient.kt

@@ -4909,10 +4909,17 @@ class WhoopBleClient(
      *  iOS BLEManager, which only sets pairingHint on the puffin link). Independent of the multi-WHOOP
      *  pin recovery in [noteBondRefusalIfPinned], which is left untouched. The guidance is mirrored into
      *  [statusNote] (already rendered on the Live screen) so it surfaces with no UI-layer change. */
-    private fun noteBondRefusalForPairingHint(status: Int, failedAddress: String?) {
-        if (!isInsufficientAuthStatus(status)) return
-        if (didBond) return                                       // already bonded — not a pairing problem
-        if (connectedFamily != DeviceFamily.WHOOP5) return        // WHOOP 4 bonds cleanly; hint is 5/MG-only
+    private fun noteBondRefusalForPairingHint(
+        status: Int,
+        failedAddress: String?,
+        helloUnacked: Boolean = false,
+    ) {
+        // #1635: an unanswered CLIENT_HELLO is as much a refusal as an auth rejection, for the purpose of
+        // giving up. It was invisible here because the gate tested only the status, and an unanswered
+        // handshake presents as a plain local terminate - so the streak never grew and the loop never
+        // stopped. The two signals stay SEPARATE below: only one of them supports naming a cause.
+        val authRefusal = isInsufficientAuthStatus(status)
+        if (!countsAsBondRefusal(authRefusal, helloUnacked, didBond, connectedFamily)) return
         bondRefusalStreak++
         if (bondGiveUp.gaveUp) {
             // #78 hole-4: a refusal during a paused-state salvage probe must not stomp the paused hint
@@ -4926,7 +4933,12 @@ class WhoopBleClient(
             if (_state.value.pairingHint == null) {
                 log("WHOOP 5/MG: encrypted bond refused $bondRefusalStreak times — surfacing pairing guidance (#78)")
             }
-            _state.update { it.copy(pairingHint = PAIRING_HINT_TEXT, statusNote = PAIRING_HINT_TEXT) }
+            // PAIRING_HINT_TEXT names a specific cause (still bonded to the official app). Only an auth
+            // refusal is evidence for that; an unanswered handshake is not, so it waits for the give-up's
+            // honest paused hint rather than being told to close an app that may be irrelevant.
+            if (authRefusal) {
+                _state.update { it.copy(pairingHint = PAIRING_HINT_TEXT, statusNote = PAIRING_HINT_TEXT) }
+            }
         }
         // #747 / #750: feed the same refusal into the give-up tracker. Once it crosses the higher threshold
         // (the pairing hint has had several cycles to be acted on), pause auto-reconnect so we stop hammering
@@ -4939,7 +4951,12 @@ class WhoopBleClient(
                 standingConnectWhilePausedIfDue(justTripped = true)
             val opaque = BondRefusalGiveUp.opaqueId(failedAddress ?: "device")
             log(BondRefusalGiveUp.epitaphLine(bondGiveUp.refusals, opaque))
-            _state.update { it.copy(pairingHint = BondRefusalGiveUp.pausedHint()) }
+            // An auth refusal supports naming a cause; an unanswered handshake does not, so it gets the
+            // hint that describes what was seen instead of asserting why (#1635).
+            _state.update {
+                it.copy(pairingHint = if (authRefusal) BondRefusalGiveUp.pausedHint()
+                        else BondRefusalGiveUp.pausedHintHandshakeUnanswered())
+            }
             if (testCentre.active(com.noop.testcentre.TestDomain.CONNECTION)) {
                 log("bond gaveUp refusals=${bondGiveUp.refusals} id=$opaque (auto-reconnect paused)",
                     com.noop.testcentre.TestDomain.CONNECTION)
@@ -5311,10 +5328,28 @@ class WhoopBleClient(
             // Port of didWriteValueFor: a CONFIRMED-write completion (no error) == bonding succeeded.
             if (status != BluetoothGatt.GATT_SUCCESS) {
                 log("Confirmed write failed: status=$status")
-                // #1635: a failed write owes no ack. Leaving the window open would let the NEXT completion
-                // on fd4b0002 — DISABLE_ALARM and every other puffin command share it — satisfy both halves
-                // of the bond gate below and declare a bond the strap never granted, which is the whole bug.
-                if (characteristic.uuid == WHOOP5_CMD_WRITE_CHAR) clientHelloWriteAtMs = 0L
+                // #1635: a FAILED completion is still a completion, and it carries two obligations.
+                //
+                // Report it: before this, a failed callback produced no line at all and then a false
+                // "NO write callback" at disconnect — the outcome line tells the truth instead.
+                //
+                // And release the window, but ONLY for the hello's own write. Clearing it stops the
+                // disconnect path counting the same cycle twice (which would trip the give-up at half the
+                // intended streak on exactly the auth refusals that already worked), and leaving it open on
+                // a hello that definitively failed would let the NEXT completion on fd4b0002 — DISABLE_ALARM
+                // and every other puffin command share it — satisfy both halves of the bond gate below and
+                // declare a bond the strap never granted. A FOREIGN write failing says nothing about the
+                // hello, so it must not consume the window a genuine ack is still owed.
+                val failedHelloChar = characteristic.uuid == WHOOP5_CMD_WRITE_CHAR
+                if (clientHelloWriteAtMs > 0L) {
+                    log(clientHelloOutcomeLine(
+                        isHelloChar = failedHelloChar,
+                        charUuid = characteristic.uuid.toString(),
+                        elapsedMs = System.currentTimeMillis() - clientHelloWriteAtMs,
+                        status = gattWriteStatusLabel(status),
+                    ))
+                }
+                if (failedHelloChar) clientHelloWriteAtMs = 0L
                 // Multi-WHOOP stale-pin recovery (#52). A status of INSUFFICIENT_AUTHENTICATION (5) /
                 // INSUFFICIENT_ENCRYPTION (15) on the bond write == the strap refused the encrypted bond
                 // (the Android twin of the iOS "Encryption/Authentication is insufficient" error). When a
@@ -8064,13 +8099,21 @@ class WhoopBleClient(
 
     @SuppressLint("MissingPermission")
     private fun handleDisconnect(status: Int) {
+        val helloWasUnacked = clientHelloWriteAtMs > 0L
         // #1635: a CLIENT_HELLO that was accepted by the stack and never completed leaves no trace at
         // all - the dominant shape in the field capture (14 of 16). Say so before the state is reset.
         if (clientHelloWriteAtMs > 0L) {
             log(clientHelloOutcomeLine(false, null, System.currentTimeMillis() - clientHelloWriteAtMs, null))
             clientHelloWriteAtMs = 0L
         }
 
+        // #1635: and count it as a refusal so the give-up can latch. Only when the status is NOT an auth
+        // rejection - those already reach noteBondRefusalForPairingHint from onCharacteristicWrite, and
+        // counting the same cycle twice would trip the give-up at half the intended streak.
+        if (helloWasUnacked && !isInsufficientAuthStatus(status)) {
+            noteBondRefusalForPairingHint(status, lastDeviceAddress, helloUnacked = true)
+        }
+
         // #1151: flush any pending frame-timing window so the frames right before this drop are recorded
         // (not stranded), and the next connection starts a fresh window rather than spanning the gap. No-op
         // when capture is off. Do it BEFORE the connect-down line so the summary reads before the drop.

android/app/src/test/java/com/noop/ble/BondRefusalCoverageTest.kt

@@ -0,0 +1,64 @@
+package com.noop.ble
+
+import com.noop.protocol.DeviceFamily
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * Pins what counts as a bond refusal (#1635).
+ *
+ * The measured bug: on a WHOOP 5/MG whose CLIENT_HELLO is never answered, the link drops as a plain
+ * local terminate. The give-up gate tested only the status, so an unanswered handshake never
+ * incremented the streak — eight consecutive attempts all logged "attempt 1" and it retried every ~11
+ * seconds indefinitely.
+ */
+class BondRefusalCoverageTest {
+
+    private val w5 = DeviceFamily.WHOOP5
+    private val w4 = DeviceFamily.WHOOP4
+
+    @Test
+    fun `an unanswered handshake now counts, which is the fix`() {
+        assertTrue(countsAsBondRefusal(isAuthRefusalStatus = false, helloUnacked = true,
+                                       alreadyBonded = false, family = w5))
+    }
+
+    @Test
+    fun `an auth rejection still counts, unchanged`() {
+        assertTrue(countsAsBondRefusal(isAuthRefusalStatus = true, helloUnacked = false,
+                                       alreadyBonded = false, family = w5))
+    }
+
+    @Test
+    fun `an ordinary disconnect with neither signal does not count`() {
+        // Guards the obvious over-reach: a normal drop after a healthy session must not accrue toward
+        // giving up, or a working strap would eventually pause itself.
+        assertFalse(countsAsBondRefusal(isAuthRefusalStatus = false, helloUnacked = false,
+                                        alreadyBonded = false, family = w5))
+    }
+
+    @Test
+    fun `an already-bonded link never counts, whatever the signal`() {
+        assertFalse(countsAsBondRefusal(true, true, alreadyBonded = true, family = w5))
+    }
+
+    @Test
+    fun `a WHOOP 4 never counts — this is the 5 or MG handshake`() {
+        assertFalse(countsAsBondRefusal(true, true, alreadyBonded = false, family = w4))
+    }
+
+    @Test
+    fun `the unanswered-handshake hint does not name a cause it cannot know`() {
+        val unanswered = BondRefusalGiveUp.pausedHintHandshakeUnanswered()
+        val refused = BondRefusalGiveUp.pausedHint()
+        // The auth-refusal hint names the official WHOOP app, which an auth rejection is evidence for.
+        assertTrue(refused.contains("official WHOOP app"))
+        // The unanswered-handshake hint must NOT, because nothing observed supports it.
+        assertFalse(unanswered.contains("official WHOOP app"))
+        assertFalse(unanswered.contains("Forget This Device"))
+        // It should still describe what was seen and offer the action that is the user's to take.
+        assertTrue(unanswered.contains("never completes"))
+        assertTrue(unanswered.contains("Tap Connect"))
+    }
+}