The unbonded offload probe's silence budget must outlive the process (#1635)

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

@@ -142,11 +142,56 @@ class PuffinExperiment(private val prefs: SharedPreferences) {
      * Its own switch because it SENDS to the strap and, if the strap answers, writes its clock — the same
      * line every other state-changing probe here sits behind ([isDeepDataEnabled], [broadcastHr],
      * [ecgRawData]). Nothing is written until the strap has proved it answers a read-only GET_CLOCK, and a
-     * refusal is latched per device, so opting in costs one link and not a loop.
+     * refusal is latched per device and silence spends a bounded, persisted budget, so opting in costs at
+     * most [UNBONDED_PROBE_MAX_SILENT_LINKS] links on a strap and not a loop. It said "one link and not a
+     * loop" while costing 18 across 24 connects, which is the correction this doc exists to record.
+     *
+     * Turning it ON also clears every strap's silence budget ([unbondedProbeSilentLinksPrefKey]). That
+     * budget is now persisted, so without this a strap that spent it would never probe again — and
+     * silently, the give-up line having latched on a run the user may never have seen. This setter is the
+     * one place the intent is unambiguous: sampling the switch at connect cannot see it flipped off and
+     * on while the link sits idle, which is exactly what a user does after being told to turn it off.
      */
     var unbondedOffload: Boolean
         get() = prefs.getBoolean(KEY_UNBONDED_OFFLOAD, false)
-        set(v) = prefs.edit().putBoolean(KEY_UNBONDED_OFFLOAD, v).apply()
+        set(v) {
+            val rearms = unbondedProbeBudgetRearms(v, prefs.getBoolean(KEY_UNBONDED_OFFLOAD, false))
+            val e = prefs.edit().putBoolean(KEY_UNBONDED_OFFLOAD, v)
+            // Every strap, not just the connected one: the switch is global, so "try again" is too, and
+            // this setter is the only path that runs with no device in hand.
+            if (rearms) {
+                prefs.all.keys
+                    .filter { it.startsWith(UNBONDED_PROBE_SILENT_LINKS_KEY_PREFIX) }
+                    .forEach { e.remove(it) }
+            }
+            e.apply()
+        }
+
+    /**
+     * The probe's persisted silence budget for one strap — links that subscribed the puffin
+     * characteristics and drew no answer, capped by [UNBONDED_PROBE_MAX_SILENT_LINKS].
+     *
+     * It lives HERE, and not beside the refusal latch in `NoopPrefs`, on purpose. The setter above clears
+     * these by prefix because it has no device in hand, and a sweep can only reach its own prefs file:
+     * written to `NoopPrefs` and swept from `noop_experiments`, re-enabling the switch would clear nothing
+     * and the probe would stay retired forever, silently. Keeping the budget on the object that owns the
+     * switch makes that drift unrepresentable rather than merely documented.
+     *
+     * Unreadable prefs read as 0 — the probe's other gates bound it, and a prefs failure must not be the
+     * thing that keeps a spent budget spent.
+     */
+    fun unbondedProbeSilentLinks(peripheralId: String?): Int = runCatching {
+        unbondedProbeSilentLinksPrefKey(peripheralId)?.let { prefs.getInt(it, 0) } ?: 0
+    }.getOrDefault(0)
+
+    /** Record that budget. A null address (no device in hand) is a no-op, as the read is. */
+    fun setUnbondedProbeSilentLinks(peripheralId: String?, value: Int) {
+        runCatching {
+            unbondedProbeSilentLinksPrefKey(peripheralId)?.let {
+                prefs.edit().putInt(it, value).apply()
+            }
+        }
+    }
 
     /** True if the user opted in to "Ask Android to pair" (#1635, default false): NOOP calls
      *  `BluetoothDevice.createBond()` explicitly instead of relying on a write to the encrypted

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

@@ -50,7 +50,7 @@ internal fun shouldProbeUnbondedOffload(
     helloWrittenThisLink: Boolean,
     alreadyProbedThisLink: Boolean,
     previouslyRefused: Boolean,
-    silentLinksSoFar: Int = 0,
+    silentLinksSoFar: Int,
 ): Boolean {
     if (!isWhoop5) return false
     if (!optedIn) return false
@@ -69,31 +69,49 @@ internal fun shouldProbeUnbondedOffload(
 /**
  * How many links may end in SILENCE before the probe retires itself.
  *
- * A refusal latches per device; silence deliberately does not, because the subscriptions were accepted and
- * one quiet link is not the strap's verdict. But "not latched" cannot mean "forever": the probe re-runs on
- * every reconnect, and a strap that reconnects often would re-ask a question already answered the same way
- * three times, which is the exact shape of the unbounded retry this whole area keeps producing.
+ * A refusal is the strap's verdict and latches outright; silence is weaker evidence — the subscriptions
+ * were accepted, and one quiet link says little — so it spends a budget instead. But weaker evidence
+ * cannot mean unbounded: the probe re-runs on every reconnect, and a strap that reconnects often would
+ * re-ask a question already answered the same way three times, which is the exact shape of the retry this
+ * whole area keeps producing.
+ *
+ * Counted PER DEVICE and persisted ([unbondedProbeSilentLinksPrefKey]), not per process. It was per
+ * process, and that was wrong in the field: the foreground service restarts often, every restart re-armed
+ * three more links, and each of those links is torn down ~4.8s after the subscriptions reach the air. One
+ * capture caught 18 probe starts across 24 connects — the unbounded retry this budget exists to prevent,
+ * surviving it by outliving the only thing that bounded it. Bounded within a process and endless across
+ * them is not bounded.
  *
- * Counted per app process, like [HELLO_OVERRIDE_MAX_ATTEMPTS], and reset by a genuine answer. A process
- * restart resets it too — the honest limit of an in-memory bound, and the loop it exists to stop runs
- * within one process.
+ * Cleared by a genuine answer, and by turning the experiment off and on — see [PuffinExperiment].
  */
 internal const val UNBONDED_PROBE_MAX_SILENT_LINKS = 3
 
-/** Has the probe any budget left after [silentLinksSoFar] links that subscribed but drew no reply? */
+/** Has the probe any budget left after [silentLinksSoFar] links that ended without an answer — whether
+ *  they subscribed and stayed quiet, or were torn down while being asked? */
 internal fun unbondedProbeStillWorthAsking(
     silentLinksSoFar: Int,
     cap: Int = UNBONDED_PROBE_MAX_SILENT_LINKS,
 ): Boolean = silentLinksSoFar < cap
 
 /**
- * The line printed when the probe retires on repeated silence, so the log says why it stopped rather than
- * leaving a reader to notice its absence — the same failure the CLIENT_HELLO's silent suppression caused.
+ * The line printed when the probe retires, so the log says why it stopped rather than leaving a reader to
+ * notice its absence — the same failure the CLIENT_HELLO's silent suppression caused.
+ *
+ * It must NOT claim the strap served the subscriptions. It said so, and that was only ever true of the
+ * links that subscribed and then stayed quiet. Since #1749 a link LOST mid-probe charges the same budget,
+ * and those links confirm no subscribes at all — so on the 31 Aug capture, where every charge was a lost
+ * link, this line would have recorded "serves those characteristics unbonded" about a strap that had
+ * demonstrated the opposite. Overclaiming from the weaker evidence is the mistake this file is built to
+ * avoid, and the retirement line is exactly where a future reader goes looking. The per-link lines
+ * ([unbondedProbeLinkLostLine], [unbondedProbeSilentLine]) already say which happened each time; this one
+ * states only what holds either way.
  */
 internal fun unbondedProbeGaveUpLine(silentLinks: Int): String =
-    "Unbonded offload probe: $silentLinks links have subscribed the puffin chars and drawn no" +
-        " COMMAND_RESPONSE — this strap serves those characteristics unbonded but does not act on commands" +
-        " written to them. Not asking again this session. History offload is unavailable until the strap" +
+    "Unbonded offload probe: $silentLinks links have ended with no COMMAND_RESPONSE — this strap either" +
+        " does not act on puffin commands written over an unencrypted link, or does not hold the link up" +
+        " long enough to answer one; the per-link lines above say which happened each time." +
+        " Not asking this strap again — turn the experiment off and on to retry." +
+        " History offload is unavailable until the strap" +
         " completes a handshake (#1635)."
 
 /**
@@ -150,6 +168,35 @@ internal fun unbondedProbeSupersedesLine(explicitBondOptedIn: Boolean): String =
 internal fun unbondedOffloadRefusedPrefKey(peripheralId: String?): String? =
     peripheralId?.trim()?.takeIf { it.isNotEmpty() }?.let { "noop.unbondedOffloadRefused.${it.lowercase()}" }
 
+/**
+ * Does writing [PuffinExperiment.unbondedOffload] hand every strap's silence budget back?
+ *
+ * The off->ON EDGE, and not merely "on". A setter that cleared on every true would return the budget to
+ * any caller that rewrites the current value, and the budget's whole job is to stop a retry that takes the
+ * link down with it. Only the two switches write it today; making this the setter's property rather than
+ * its callers' is what keeps that true.
+ */
+internal fun unbondedProbeBudgetRearms(optedInNow: Boolean, optedInBefore: Boolean): Boolean =
+    optedInNow && !optedInBefore
+
+/** Prefix of every persisted silence budget, so opting back in can clear them all without knowing which
+ *  straps have one. Sole reason it is a constant rather than an inlined string. */
+internal const val UNBONDED_PROBE_SILENT_LINKS_KEY_PREFIX = "noop.unbondedOffloadSilentLinks."
+
+/**
+ * Persisted key for "how many links on this strap subscribed the puffin characteristics and drew no
+ * answer" — the silence budget of [UNBONDED_PROBE_MAX_SILENT_LINKS], per device and lowercased for the
+ * same reason [firmwarePrefKey] is.
+ *
+ * Deliberately NOT [unbondedOffloadRefusedPrefKey], though a spent budget stops the probe just as a
+ * refusal does. A refusal is the strap's answer and is final; a spent budget is ours — we stopped asking —
+ * and the user can hand it back. Sharing one key would have the log report a refusal that never happened,
+ * which is precisely the conflation [UnbondedProbeEvidence] exists to prevent.
+ */
+internal fun unbondedProbeSilentLinksPrefKey(peripheralId: String?): String? =
+    peripheralId?.trim()?.takeIf { it.isNotEmpty() }
+        ?.let { UNBONDED_PROBE_SILENT_LINKS_KEY_PREFIX + it.lowercase() }
+
 /**
  * What a frame arriving on a puffin notify characteristic proves about an unbonded link.
  *

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

@@ -2425,10 +2425,24 @@ class WhoopBleClient(
     /** #1635: how many times the probe has stood aside for the DIS chain on this link. */
     private var unbondedProbeDeferrals = 0
 
-    /** #1635: links that subscribed the puffin chars and drew no reply. Per PROCESS, deliberately NOT
-     *  cleared by [reset] — its whole job is to bound a retry that spans reconnects. A refusal latches per
-     *  device instead; silence is weaker evidence and gets a budget rather than a verdict. */
-    private var unbondedProbeSilentLinks = 0
+    /** #1635: links that subscribed the puffin chars and drew no reply — the silence budget for the
+     *  connected strap, read through prefs on every use.
+     *
+     *  Was a field, and the field was the bug: the foreground service restarts, the count went back to
+     *  zero, and the probe bought three more link-killing attempts on a strap that had already answered
+     *  the same way three times (18 starts across 24 connects, 31 Aug). Reading it through prefs is a
+     *  handful of cheap lookups per link and leaves exactly one source of truth.
+     *
+     *  Held by [PuffinExperiment] rather than `NoopPrefs`, unlike the refusal latch beside it: the switch's
+     *  setter clears these budgets by prefix and can only sweep its own prefs file. */
+    private val unbondedProbeSilentLinks: Int
+        get() = runCatching {
+            PuffinExperiment.from(context).unbondedProbeSilentLinks(lastDeviceAddress)
+        }.getOrDefault(0)
+
+    private fun setUnbondedProbeSilentLinks(value: Int) {
+        runCatching { PuffinExperiment.from(context).setUnbondedProbeSilentLinks(lastDeviceAddress, value) }
+    }
 
     /** True when the user asked to disconnect; suppresses the auto-rescan (Swift `intentionalDisconnect`).
      *  Written on the main looper (connect/disconnect/keep-alive bounce) and read on the GATT binder
@@ -4850,6 +4864,21 @@ class WhoopBleClient(
         }
     }
 
+    /**
+     * Charge one link that subscribed the puffin characteristics and produced no answer, retiring the
+     * probe for this strap when the budget runs out.
+     *
+     * Both the "asked and heard nothing" path and the "link died mid-probe" path end here, and they must:
+     * a quiet link and a link torn down by the very subscriptions we wrote are the same evidence about
+     * this strap. Counting only the first is how the probe kept re-running across 24 connects.
+     */
+    private fun chargeUnbondedProbeSilence() {
+        val spent = unbondedProbeSilentLinks + 1
+        setUnbondedProbeSilentLinks(spent)
+        if (unbondedProbeStillWorthAsking(spent)) return
+        log(unbondedProbeGaveUpLine(spent))
+    }
+
     /**
      * Stage 2: the subscriptions landed, so ask the strap a read-only question.
      *
@@ -4898,22 +4927,20 @@ class WhoopBleClient(
                 waitedMs = waited,
                 sawNotifications = unbondedProbeEvidence == UnbondedProbeEvidence.SERVES_NOTIFICATIONS,
             ))
-            // Not latched per device: the subscriptions were accepted, so this says the strap did not
+            // Not the refusal latch: the subscriptions were accepted, so this says the strap did not
             // answer THIS time, and the carve-out the refusal path makes for a pairing in flight would
-            // apply here with less certainty, not more. Once-per-LINK does not bound it, though — the
-            // probe re-runs on every reconnect — so silence spends a per-process budget instead of writing
-            // a permanent verdict from an ambiguous result.
-            unbondedProbeSilentLinks++
-            if (!unbondedProbeStillWorthAsking(unbondedProbeSilentLinks)) {
-                log(unbondedProbeGaveUpLine(unbondedProbeSilentLinks))
-            }
+            // apply here with less certainty, not more. Once-per-LINK does not bound it either — the probe
+            // re-runs on every reconnect — so silence spends a budget rather than writing a verdict from
+            // an ambiguous result. The budget is persisted, because the process is not a bound.
+            chargeUnbondedProbeSilence()
             return
         }
         log(unbondedProbeAnsweredLine())
         log(unbondedProbeBacklogCaveatLine())
         // A genuine answer clears the silence budget: whatever the quiet links were, they were not this
-        // strap declining to talk, and a later reconnect must not inherit their count.
-        unbondedProbeSilentLinks = 0
+        // strap declining to talk, and a later reconnect — or a later app launch — must not inherit
+        // their count.
+        setUnbondedProbeSilentLinks(0)
         // The proven 5/MG handshake tail, minus the hello that cannot happen: clock the strap, then offload.
         // Clock-before-history is mandatory — an un-clocked 5/MG discards sensor data rather than banking it
         // — and it is only reached here because the strap has just demonstrated it answers commands.
@@ -9868,10 +9895,7 @@ class WhoopBleClient(
                         System.currentTimeMillis() - unbondedProbeAskedAtMs else -1L,
                 ),
             )
-            unbondedProbeSilentLinks++
-            if (!unbondedProbeStillWorthAsking(unbondedProbeSilentLinks)) {
-                log(unbondedProbeGaveUpLine(unbondedProbeSilentLinks))
-            }
+            chargeUnbondedProbeSilence()
         }
         unbondedProbeStartedThisLink = false
         unbondedProbeDeferrals = 0