devices: attribute firmware to the strap it came from, not the last to connect

Packages/WhoopProtocol/Sources/WhoopProtocol/FirmwareGateDiagnostic.swift

@@ -0,0 +1,35 @@
+import Foundation
+
+/// Why a WHOOP 5/MG hello carried no firmware version, in enough detail to re-derive the offset.
+///
+/// The version is read as four bytes at `pay[93]`, guarded by `pay[93] == 50` ("5.0 generation"). Both
+/// the offset and the guard are anchored to a SINGLE capture at firmware 50.38.1.0, and the decoder's own
+/// comment says so: the name+token region ahead of the version is fixed-width *on that firmware*, and
+/// re-verifying the offset across firmwares is explicitly left as future work. The guards fail closed, so
+/// a strap that does not match simply reports nothing.
+///
+/// That silence has two very different causes and no way to tell them apart from a log:
+///  - the byte at 93 is not 50 — a different generation marker, offset probably still right;
+///  - the name+token region is a different width, so byte 93 is not the version at all and the offset has
+///    MOVED. This is the one the decoder comment warns about, and the one a user cannot diagnose.
+///
+/// So the line carries what distinguishes them: the payload length, the byte actually at 93, where the
+/// printable-ASCII device name ended (the run whose width shifts everything after it), and a hex window
+/// spanning the region the version should sit in. A capture from an undecoded strap then locates the real
+/// offset without another round trip.
+///
+/// Deliberately does NOT loosen the guard. Widening it blindly would render unrelated bytes as a version,
+/// which is worse than "unknown" because it looks authoritative.
+///
+/// Pure, and the window is clamped to the payload, so a short or malformed frame cannot trap. Kotlin
+/// twin: `com.noop.protocol.firmwareGateDiagnostic`.
+public func firmwareGateDiagnostic(payload: [UInt8], nameEndIndex: Int) -> String {
+    let at93 = payload.count > 93 ? String(payload[93]) : "n/a"
+    let lo = min(88, payload.count)
+    let hi = min(101, payload.count)
+    let window = lo < hi
+        ? payload[lo..<hi].map { String(format: "%02x", $0) }.joined()
+        : ""
+    return "fw gate: no version decoded — len=\(payload.count) at93=\(at93) expected=50"
+        + " nameEnd=\(nameEndIndex) hex[\(lo)..<\(hi)]=\(window)"
+}

Packages/WhoopProtocol/Sources/WhoopProtocol/Interpreter.swift

@@ -838,6 +838,11 @@ private func decodeWhoop5CommandResponse(_ frame: [UInt8], fb: FieldBuilder, sch
         }
         if pay.count >= 97, pay[93] == 50 {
             fb.parsed["fw_version"] = .string("\(pay[93]).\(pay[94]).\(pay[95]).\(pay[96])")
+        } else {
+            // The guards fail closed by design, which left a strap reporting no firmware with no way to
+            // say WHY - a different generation byte and a MOVED offset look identical from a log. Carry
+            // the evidence instead; see `firmwareGateDiagnostic`.
+            fb.parsed["fw_gate"] = .string(firmwareGateDiagnostic(payload: pay, nameEndIndex: i))
         }
     }
 }

Packages/WhoopProtocol/Tests/WhoopProtocolTests/FirmwareGateDiagnosticTests.swift

@@ -0,0 +1,49 @@
+import XCTest
+@testable import WhoopProtocol
+
+/// Pins the 5/MG firmware-gate diagnostic. Kotlin twin: `FirmwareGateDiagnosticTest`.
+///
+/// The decoder reads the version at pay[93] behind a `pay[93] == 50` guard, both anchored to a single
+/// 50.38.1.0 capture. The guards fail closed, so a strap that does not match reports nothing — and a
+/// different generation byte is indistinguishable from a MOVED offset unless the line says which.
+final class FirmwareGateDiagnosticTests: XCTestCase {
+
+    private func payload(count: Int, at93: UInt8) -> [UInt8] {
+        var p = (0..<count).map { UInt8($0 % 256) }
+        if count > 93 { p[93] = at93 }
+        return p
+    }
+
+    func testReportsTheByteItActuallySawAndTheExpectedOne() {
+        let line = firmwareGateDiagnostic(payload: payload(count: 128, at93: 51), nameEndIndex: 27)
+        XCTAssertTrue(line.contains("at93=51 expected=50"), line)
+        XCTAssertTrue(line.contains("len=128"), line)
+    }
+
+    func testCarriesTheNameEndBecauseThatIsWhatMovesTheOffset() {
+        // The version sits after the name+token region, so where the printable-ASCII name run ended is
+        // the number that lets a reader re-derive a shifted offset.
+        let line = firmwareGateDiagnostic(payload: payload(count: 128, at93: 51), nameEndIndex: 31)
+        XCTAssertTrue(line.contains("nameEnd=31"), line)
+    }
+
+    func testTheHexWindowSpansTheRegionTheVersionShouldOccupy() {
+        let line = firmwareGateDiagnostic(payload: payload(count: 128, at93: 51), nameEndIndex: 27)
+        XCTAssertTrue(line.contains("hex[88..<101]="), line)
+        // 13 bytes, two hex chars each.
+        let hex = line.components(separatedBy: "hex[88..<101]=").last ?? ""
+        XCTAssertEqual(hex.count, 26, hex)
+    }
+
+    func testAShortPayloadCannotTrapAndSaysSo() {
+        // A malformed/truncated hello must not crash the decoder; the window clamps and at93 reports n/a.
+        let line = firmwareGateDiagnostic(payload: [1, 2, 3], nameEndIndex: 2)
+        XCTAssertTrue(line.contains("at93=n/a"), line)
+        XCTAssertTrue(line.contains("len=3"), line)
+    }
+
+    func testAPayloadEndingExactlyAtTheWindowStartYieldsAnEmptyWindow() {
+        let line = firmwareGateDiagnostic(payload: Array(repeating: 0, count: 88), nameEndIndex: 20)
+        XCTAssertTrue(line.hasSuffix("hex[88..<88]="), line)
+    }
+}

Strand/BLE/FirmwareAttribution.swift

@@ -0,0 +1,51 @@
+import Foundation
+
+/// Which firmware string belongs to a device, given what is known about it.
+///
+/// The persisted firmware used to live under one global key, written on any WHOOP connect and read back
+/// for whichever strap was active. On a single-strap install those are the same strap, which is why it
+/// went unnoticed; with two straps paired it reports the OTHER strap's firmware — a WHOOP 5/MG showing a
+/// 4.0's 41.17.6.0 because the 4.0 connected last.
+///
+/// The rule, in order:
+///  - `live` wins: it came from THIS connection's handshake.
+///  - `perDevice` next: this device's own persisted value, from a previous connection.
+///  - `legacyGlobal` ONLY when `pairedCount` is 1. The old global key cannot say which strap it belongs
+///    to, so it is trustworthy exactly when there is only one strap it could have come from. This keeps a
+///    single-strap install reading correctly across the upgrade instead of showing "unknown" until the
+///    next connect; a multi-strap install refuses it, which is the bug.
+///  - otherwise nil — "not known yet" is the honest answer, and better than another strap's number.
+///
+/// Pure so the attribution is unit-tested without defaults, a strap, or a registry. Kotlin twin:
+/// `com.noop.ble.resolveFirmware`.
+enum FirmwareAttribution {
+
+    static func resolve(live: String?,
+                        perDevice: String?,
+                        legacyGlobal: String?,
+                        pairedCount: Int) -> String? {
+        if let live, !live.isBlank { return live }
+        if let perDevice, !perDevice.isBlank { return perDevice }
+        if let legacyGlobal, !legacyGlobal.isBlank, pairedCount == 1 { return legacyGlobal }
+        return nil
+    }
+
+    /// The per-device defaults key for a persisted firmware string.
+    ///
+    /// Keyed on the BLE peripheral identifier rather than the registry id: the BLE layer knows the
+    /// peripheral it connected to, and resolving it to a registry row there would repeat the mis-mapping
+    /// #1527 fixed for `lastSeen` (stamping "the active row" records a sighting of a strap that was never
+    /// connected). The registry row carries the same identifier, so the read side resolves without
+    /// guessing.
+    ///
+    /// Returns nil for a blank identifier, so a caller with none writes nothing rather than writing to a
+    /// key that belongs to no device.
+    static func prefKey(peripheralId: String?) -> String? {
+        guard let p = peripheralId?.trimmingCharacters(in: .whitespaces), !p.isEmpty else { return nil }
+        return "noop.lastFirmware.\(p.lowercased())"
+    }
+}
+
+private extension String {
+    var isBlank: Bool { trimmingCharacters(in: .whitespaces).isEmpty }
+}

Strand/BLE/FrameRouter.swift

@@ -18,12 +18,16 @@ public final class FrameRouter {
         // #900: a fresh connection is a fresh capture session — re-arm the per-command raw-frame dump so
         // each connect can re-capture the disputed COMMAND_RESPONSE prefix once. `family` is set fresh per
         // connection by BLEManager (connectCore), so this is the per-session reset hook.
-        didSet { rawDumpedRespCmds.removeAll() }
+        didSet { rawDumpedRespCmds.removeAll(); loggedFirmwareGate = nil }
     }
 
     /// #900: resp command names (e.g. "GET_BATTERY_LEVEL(26)") whose raw COMMAND_RESPONSE frame has already
     /// been dumped this connection. The provenance dump fires once per command per session so a 4.0's
     /// per-poll battery reads don't flood the strap log. Reset when `family` is set at connect.
+    /// #1634: last firmware-gate line logged, so a stable per-connection value is not repeated on every
+    /// hello. Cleared alongside the other per-connection routing state.
+    private var loggedFirmwareGate: String?
+
     private var rawDumpedRespCmds: Set<String> = []
 
     public init(state: LiveState) {
@@ -112,6 +116,14 @@ public final class FrameRouter {
                 // Persist so the debug export can name the firmware offline (state clears on disconnect).
                 UserDefaults.standard.set(fw, forKey: "noop.lastFirmware")
             }
+
+            // #1634: the 5/MG hello decoded no firmware. The guards fail closed by design, so this is the
+            // only place that can say WHY - a different generation byte vs a MOVED offset. Logged once per
+            // connection (the value is stable), so a capture from an undecoded strap carries the evidence.
+            if let gate = parsed.parsed["fw_gate"]?.stringValue, loggedFirmwareGate != gate {
+                loggedFirmwareGate = gate
+                state.append(log: gate, domain: .connection)
+            }
             // Advertising-name replies (WHOOP 4.0 / Harvard). GET (cmd 76) carries the current name in
             // its payload; SET (cmd 77) carries only a result byte. The schema has no field decode for
             // either, so pull them straight from the frame bytes. The COMMAND_RESPONSE inner is

Strand/Screens/DevicesView.swift

@@ -193,10 +193,18 @@ private struct DevicesContent: View {
                     // handshake, so a non-WHOOP active device (Oura) must NOT inherit it. Single last-connected-
                     // strap key, so a not-yet-connected active strap can briefly show the other strap's build on
                     // a multi-WHOOP install until it republishes. Twin of Android.
-                    liveFirmware: device.status == .active
-                        ? (live.strapFirmware
-                            ?? (SourceCoordinator.isWhoop(device) ? UserDefaults.standard.string(forKey: "noop.lastFirmware") : nil))
-                        : nil,
+                    // #1633 follow-up: resolve against THIS device, never the last strap to connect. The old
+                    // fallback read one global key, so with two straps paired a 5/MG reported the 4.0's firmware.
+                    // The legacy key is honoured only when a single device is paired, where it cannot belong to
+                    // anything else.
+                    liveFirmware: FirmwareAttribution.resolve(
+                        live: device.status == .active ? live.strapFirmware : nil,
+                        perDevice: SourceCoordinator.isWhoop(device)
+                            ? FirmwareAttribution.prefKey(peripheralId: device.peripheralId)
+                                .flatMap { UserDefaults.standard.string(forKey: $0) } : nil,
+                        legacyGlobal: SourceCoordinator.isWhoop(device)
+                            ? UserDefaults.standard.string(forKey: "noop.lastFirmware") : nil,
+                        pairedCount: registry.devices.count),
                     // Historical record layout (v24/v25 on WHOOP 4.0) observed from this connection's
                     // backfill. Distinct from the strap firmware build shown as FW.
                     liveHistoryLayout: (device.status == .active && live.connected) ? live.strapRange?.firmwareLayout : nil,

Strand/System/DebugDataDiagnostics.swift

@@ -37,6 +37,12 @@ enum DebugDataDiagnostics {
         let model = WhoopModel(rawValue: d.string(forKey: "selectedWhoopModel") ?? "")?.displayName
             ?? "unknown (never paired)"
         lines.append("Model:       \(model)")
+        // NOTE: still the legacy GLOBAL key, and therefore the last strap to connect rather than this
+        // device's own firmware. strapStateLines is sync and prefs-only by contract (the scheduled export
+        // calls it with no store), and the per-device rule needs a registry read for pairedCount. The
+        // Devices block further down IS resolved per device, so a multi-strap export carries the correct
+        // per-device value there; this line is superseded by it and wants the same follow-up as the Apple
+        // write site, which has no peripheral identity to key on today.
         lines.append("Firmware:    \(d.string(forKey: "noop.lastFirmware") ?? "unknown (connect to record)")")
         let syncSec = d.double(forKey: "lastSyncedAt")
         lines.append("Last sync:   \(syncSec > 0 ? relTime(Date().timeIntervalSince1970 - syncSec) : "never")")
@@ -108,9 +114,25 @@ enum DebugDataDiagnostics {
         // the same position it holds on Android.
         if let invStore = await repo.storeHandle() {
             let invRegistry = DeviceRegistryStore(dbQueue: invStore.registryWriter)
-            let invRows = ((try? invRegistry.all()) ?? []).map {
+            // Bound before the map so the rule below has a count to test. Reading `.all()` inline left
+            // nothing to reference, which app-build caught and no local check could — this file needs
+            // macOS to compile.
+            let invDevices = (try? invRegistry.all()) ?? []
+            let invRows = invDevices.map {
+                // Firmware resolved by the same rule as the Devices card: this device's own persisted
+                // value when there is one, and the LEGACY global key only when a single device is paired
+                // (it cannot have come from anything else). Apple does not yet write the per-device key —
+                // the write site has no peripheral identity to key on — so today this yields the global
+                // value for a single-strap install and "unknown" for a multi-strap one, which is honest
+                // rather than another strap's number.
                 InventoryRow(id: $0.id, brand: $0.brand, model: $0.model,
-                             status: $0.status.rawValue, lastSeenAt: $0.lastSeenAt)
+                             status: $0.status.rawValue, lastSeenAt: $0.lastSeenAt,
+                             firmware: FirmwareAttribution.resolve(
+                                 live: nil,
+                                 perDevice: FirmwareAttribution.prefKey(peripheralId: $0.peripheralId)
+                                     .flatMap { UserDefaults.standard.string(forKey: $0) },
+                                 legacyGlobal: UserDefaults.standard.string(forKey: "noop.lastFirmware"),
+                                 pairedCount: invDevices.count))
             }
             let invActive = (try? invRegistry.activeDeviceId()) ?? nil
             lines.append(contentsOf: deviceInventoryLines(rows: invRows,

Strand/System/DeviceInventory.swift

@@ -7,6 +7,7 @@ struct InventoryRow {
     let model: String
     let status: String
     let lastSeenAt: Int
+    let firmware: String?
 }
 
 /// The strap log's paired-device inventory: every registry row, which one is ACTIVE, and when each was
@@ -46,5 +47,6 @@ func deviceInventoryLines(rows: [InventoryRow],
         let marker = r.id == activeId ? "ACTIVE" : r.status
         let seen = r.lastSeenAt > 0 ? relTime(Double(nowSec - r.lastSeenAt)) : "never"
         return "  device id=\(r.id) status=\(marker) brand=\(r.brand) model=\(r.model) lastSeen=\(seen)"
+            + " fw=\(r.firmware ?? "unknown")"
     }
 }

StrandTests/DeviceInventoryTests.swift

@@ -17,8 +17,8 @@ final class DeviceInventoryTests: XCTestCase {
     }
 
     private func row(_ id: String, _ status: String, _ seen: Int,
-                     model: String = "WHOOP 4.0") -> InventoryRow {
-        InventoryRow(id: id, brand: "WHOOP", model: model, status: status, lastSeenAt: seen)
+                     model: String = "WHOOP 4.0", fw: String? = nil) -> InventoryRow {
+        InventoryRow(id: id, brand: "WHOOP", model: model, status: status, lastSeenAt: seen, firmware: fw)
     }
 
     func testAnEmptyRegistrySaysSoRatherThanPrintingABareHeader() {
@@ -36,16 +36,16 @@ final class DeviceInventoryTests: XCTestCase {
             activeId: "my-whoop", nowSec: now, relTime: rel)
         XCTAssertEqual(lines, [
             "Devices:     2 registered (1 active, 1 paired, 0 archived)",
-            "  device id=my-whoop status=ACTIVE brand=WHOOP model=WHOOP 5.0 / MG lastSeen=3h 10m ago",
-            "  device id=whoop-B status=paired brand=WHOOP model=WHOOP 4.0 lastSeen=10m ago",
+            "  device id=my-whoop status=ACTIVE brand=WHOOP model=WHOOP 5.0 / MG lastSeen=3h 10m ago fw=unknown",
+            "  device id=whoop-B status=paired brand=WHOOP model=WHOOP 4.0 lastSeen=10m ago fw=unknown",
         ])
     }
 
     func testANeverSeenRowReadsNeverRatherThanAHugeDuration() {
         let lines = deviceInventoryLines(rows: [row("whoop-C", "paired", 0)],
                                          activeId: nil, nowSec: 100_000, relTime: rel)
         XCTAssertEqual(lines[1],
-                       "  device id=whoop-C status=paired brand=WHOOP model=WHOOP 4.0 lastSeen=never")
+                       "  device id=whoop-C status=paired brand=WHOOP model=WHOOP 4.0 lastSeen=never fw=unknown")
     }
 
     func testCountsSplitActivePairedAndArchived() {