Repository navigation
diagnostics: name every paired device in the strap log (and redact the header) - #1633
Merged
Merged
Conversation
The strap log's body is PII-scrubbed as it is appended — Android through WhoopBleClient.log(), Swift as each line enters the ring. The HEADER is not. It is built from the diagnostics block and appended raw at four export sites (LogExport ×2, TestBundleAssembler, LiveState ×2), and LogExport's own comment states the assumption that made this invisible: "Lines are ALREADY PII-scrubbed by WhoopBleClient.log(), so no extra redaction here." True of the body it was written about; not true of the header beside it. That header already carries device ids. A single-strap install is "my-whoop" and harmless, but a re-added or second strap is "whoop-<MAC>" — so the funnel's "no raw biometric samples under '<id>'" line, and the orphan-id line #1620 added, can print a BLE address into a file whose whole purpose is to be attached to a public issue. Run every header line through the same redactor the body already uses. The mask keeps the first and last octet ("F1:••:••:••:••:DE"), so two straps stay distinguishable in a shared log without publishing an address. Found while building the paired-device inventory, which would have widened this from "the active id" to "every id" — so it is a prerequisite rather than a nicety. Both platforms had it; both are fixed here. Tests pin the PROPERTY (no raw MAC survives a header line) rather than the call sites, since a refactor may legitimately move those: the device-id line, the #1620 orphan line, and an address-free id that must pass through untouched. compileFullDebugKotlin clean; full com.noop.testcentre suite (131 tests) green; doc_comment_lint and i18n_audit clean. The Swift edits are app-target and need app-build.
…active one
The "Strap & data" header reads the last-connected PREFS rather than the
registry, so it can only ever describe ONE device. On a two-strap install —
a 4.0 kept alongside a new 5/MG — the log never mentions the second strap
at all.
That leaves the id-bearing lines elsewhere with nothing to be checked
against. `dayOwner day=… readId=my-whoop writeActiveId=my-whoop` tells you
which id a day was read from; the funnel's orphan check names the id
holding a night's samples. Neither is interpretable without knowing which
ids exist, which one is active, and which actually synced.
Add a Devices block enumerating the registry:
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
Sorted ACTIVE first, then most-recently-seen, then by id — a stable order
that never depends on registry iteration, and that puts the row a reader
wants at the top. The active row is deliberately NOT assumed to be the most
recently seen: switching straps leaves the old one with the fresher
lastSeen, which is exactly the case this exists to make readable.
The marker prints ACTIVE by comparing against the registry's activeDeviceId
rather than trusting the row's own status field. When those disagree — the
row says active, the registry points elsewhere — the line shows it instead
of hiding it, which is the corruption a two-strap install can produce.
Both platforms, byte-identical across five cases, diffed rather than
eyeballed. relTime already agreed byte-for-byte across the two, so the
durations match without a second implementation.
Structural asymmetry worth recording: Android's header block is suspend and
reads the registry inline; Swift's `strapStateLines()` is sync and
prefs-only by contract, so the Swift inventory takes its own store handle
inside the async `dynamicLines`. Same output, same position, different seam.
Verified: compileFullDebugKotlin clean; 5 JVM tests plus a twinned
StrandTests file; full com.noop.testcentre suite (131 tests) green;
doc_comment_lint and i18n_audit clean. Swift edits need app-build.
Second time in this session, same cause: the `git add` named the source directories and not StrandTests, so the branch carried the Kotlin inventory tests with no Swift twin — a parity gap in a change whose subject is the two platforms emitting identical lines. gh's "1 uncommitted change" warning is what caught it. The five cases match the Kotlin file one for one, including the ordering case that matters most: the ACTIVE row sorts first even when another strap was seen more recently, which is exactly what switching straps produces.
Parity re-review of my own commit, and it overclaimed. I wrote that the header is "appended raw at four export sites (LogExport x2, TestBundleAssembler, LiveState x2)", implying all of them leaked. The bundle path did not: TestBundleAssembler.assemble applies redactEntries to every entry it produces, on BOTH platforms, so the header inside a bundle was already scrubbed before it shipped. The genuinely unprotected sites are the DIRECT-SHARE ones, which bypass the bundle assembler entirely: Android LogExport (share + scheduled .txt) and Swift LiveState (exportableLogText + scheduledExportText). Four sites, two per platform, and those are where the fix belongs. Reverted the TestBundleAssembler edit. It was harmless — redaction is idempotent — but that file documents itself as "the single scrub point", and adding a second one contradicts the design it states for no gain. The robustness net (BundleRobustness) exists precisely to catch an entry that bypasses redactEntries, which is the right place for that concern. compileFullDebugKotlin clean; full com.noop.testcentre suite (131 tests) green.
ryanbr
added a commit
that referenced
this pull request
Aug 26, 2026
Carries the paired-device inventory and the strap-log header redaction (#1633), so a two-strap log now names every registered device and cannot publish a BLE address while doing it. Version strings stay 10.6.1.
ryanbr
added a commit
that referenced
this pull request
Aug 26, 2026
…o connect (#1634) A WHOOP 5/MG showed 41.17.6.0 in the Devices window — the 4.0's firmware. The persisted value lived under ONE global key, "noop.lastFirmware", 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 paired it reports the other one. The strap log had it worse: its header printed that global key with no live value and no device qualification at all. Resolve per device instead, by one rule shared across both platforms: the live handshake value, else THIS device's own persisted value, else the legacy global key ONLY when a single device is paired — where it cannot have come from anything else. Otherwise nothing, because "unknown" beats another strap's number. The single-device carry stops an existing install regressing to "unknown" on upgrade. Android is wired end to end: the write is keyed by the address THIS connection used (mirroring #1527 — a stamp belongs to the identity the strap adopted, not to whatever is active now), and the Devices card, the strap-log header and the #1633 inventory all read per device. The inventory line gains fw=, so a two-strap log states each strap's firmware. Apple is CORRECT but not complete: FrameRouter has no peripheral identity to key a write on and LiveState carries none, so giving it one is a BLE-path change wanting its own review and hardware verification. Until then Apple writes nothing per-device and the same rule refuses the global whenever more than one device is paired — it degrades to "unknown", never to the wrong strap's number. Attribution alone cannot make a firmware appear, so this also says WHY a 5/MG hello decoded none. The version is four bytes at pay[93] behind a `pay[93] == 50` guard, both anchored to a SINGLE 50.38.1.0 capture; the decoder's own comment notes the name+token region ahead of it is fixed-width "on this firmware" and asks for the offset to be re-verified across firmwares. The guards fail closed, so two very different causes look identical from a log: a different generation byte, or a MOVED offset. The new line carries payload length, the byte actually at 93, where the printable-ASCII name run ended, and a hex window across the region — enough to locate the real offset from one capture. The guard is NOT loosened; widening it blindly would render unrelated bytes as a version, which is worse than "unknown" because it looks authoritative. Verified: app-build green both legs with all three Swift suites confirmed EXECUTED in StrandTests; 638 WhoopProtocol tests green on Linux (the decoder half is pure); 1031 Android tests; the attribution diffed Kotlin-vs-Swift across nine cases and the gate diagnostic across five, both byte-identical; doc_comment_lint and i18n_audit clean. The first app-build run FAILED — `cannot find 'invDevices' in scope`, a registry read consumed inline with no binding left to count. Every local check had passed; that file needs macOS, and app-build is the only thing that compiles it.
rntcruz23
pushed a commit
to rntcruz23/noop
that referenced
this pull request
Aug 26, 2026
Another 320-commit batch, 9 conflicted files. Applied the lesson from the last sync's regression: for every "one side looks deleted" hunk, checked whether that content had moved elsewhere in the file (or a new file) before resolving, rather than assuming a block is safe to keep/drop verbatim. - Strand/Screens/ManualWorkoutSheet.swift: upstream restructured `body` into a `formContent` computed property wrapped by a new #if os(macOS)/#else split (fixing keyboard-avoidance clipping, ryanbr#450/ryanbr#697) - the platform frame/sheet-presentation/keyboardDoneToolbar modifiers that used to sit on the fork's HEAD tail now live on the NEW wrapping `body`, already merged in cleanly earlier in the file. Kept only the fork-only bits genuinely absent elsewhere: the two onChange(of:) reference-prefill triggers. Verified via grep that the platform-modifier chain already appears earlier, unconflicted. - Strand/Screens/SettingsView.swift (2 conflicts): the fork's ryanbr#103 protocolCheckSection/fiveMGVerifiedLine and upstream's new spo2CandidateCard (ryanbr#103/queue-11a - the WHOOP 5/MG @82 SpO2 candidate byte, corr +0.99 vs the WHOOP app over an 8-night validation, split out so Oura-only installs can reach it too) are independent computed properties, each still called exactly once outside the conflict. Reconstructed protocolCheckSection's own closing braces (previously shared with spo2CandidateCard's identical-text tail, which is what made git present it as ambiguous) and kept both in full, plus the fork's auditIcon/auditColor/auditTitle helpers the check section calls. - android WhoopBleClient.kt (2 conflicts): upstream fixed a real bug (ryanbr#1635) - the WHOOP 5/MG bond declaration used to fire on ANY characteristic completion, not just the actual CLIENT_HELLO ack (a DISABLE_ALARM completion landing on the same characteristic was mistaken for a bond). Took upstream's properly-gated version wholesale and re-inserted the one fork-only side effect the old flow had (`auditNote { it.noteBond(...) }`, ryanbr#103) into the correct point inside the new gate. Second conflict: unioned two independent per-connection reset() fields on each side. - android DevicesScreen.kt: upstream fixed a real multi-strap bug (ryanbr#1633 follow-up) - firmware now resolves per-device instead of one global key (a second paired WHOOP could show the first one's firmware). Combined with the fork's verifiedFacts/inputsSummary (ryanbr#103) fields, which upstream's tree doesn't have. - android WorkoutsScreen.kt: upstream's icon-matching refinement (dedicated Kayaking/Surfing/Sailing/ScubaDiving icons, "spin"->cycling, "polo"->swim) is a strict superset of the fork's coarser version - confirmed every branch the fork's HEAD side provided (cricket/baseball/martial-arts/hiit) already exists, unconflicted, immediately after where the conflict closes. Took upstream's side; dropped the now-fully-redundant HEAD fragment. - android SettingsScreen.kt: non-overlapping imports, unioned. - build.gradle.kts + project.yml: upstream well ahead (10.6.1 vs the fork's 9.3.4) - took wholesale. - altstore-source.json: fork manifest kept wholesale, as before. Verified: every touched Kotlin file compiled with the real Kotlin 1.9.24 compiler - zero syntax errors, zero duplicate-declaration/ambiguity errors, zero unresolved references on any symbol this merge touched. Both touched Swift files' brace/paren balance checked and cross-referenced against both parent branches (matching, none introduced by this merge). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012pwRSEnFWdsYMC7CLXWNEW
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits: a PII fix that turned out to be a prerequisite, then the feature.
1. The strap-log header was never redacted
The log body is scrubbed as it is appended — Android via
WhoopBleClient.log(), Swift as each line enters the ring. The header is not. It is built from the diagnostics block and appended raw at the four direct-share export sites — AndroidLogExport(share + scheduled) and SwiftLiveState(exportableLogText+scheduledExportText) — which bypass the bundle assembler entirely.LogExport's own comment records the assumption that hid the real gap:True of the body it describes; not true of the header beside it.
That header already prints device ids. A single-strap install is
my-whoopand harmless — which is why this has gone unnoticed — but a re-added or second strap iswhoop-<MAC>. So the funnel'sno raw biometric samples under '<id>'line, and the orphan-id line #1620 added, can put a BLE address into a file whose entire purpose is being attached to a public issue.Every header line now goes through the same redactor as the body. The mask keeps the first and last octet (
F1:••:••:••:••:DE), so two straps stay distinguishable without publishing an address.I found this while building the inventory below, which would have widened the exposure from the active id to every id. Both platforms had it; both are fixed.
2. Name every paired device
The
Strap & dataheader reads the last-connected prefs, not the registry — so it can only ever describe one device. On a two-strap install the log never mentions the second at all, which leaves every id-bearing line uninterpretable:dayOwner readId=…tells you which id a day was read from, and the funnel's orphan check names the id holding a night's samples, but neither means anything without knowing which ids exist, which is active, and which actually synced.Sorted ACTIVE first, then most-recently-seen, then by id — stable, never dependent on registry iteration order. The active row is deliberately not assumed to be the most recently seen: switching straps leaves the old one with the fresher
lastSeen, which is precisely the case this exists to make readable, and there is a test for it.The
ACTIVEmarker compares against the registry'sactiveDeviceIdrather than trusting the row's ownstatusfield. When those disagree — row says active, registry points elsewhere — the line shows it rather than hiding it. That divergence is a corruption a two-strap install can produce, and it should be visible.Parity
Byte-identical across five cases, diffed rather than eyeballed (Swift formatter extracted, compiled standalone, stdout diffed against the JVM's).
relTimealready agreed byte-for-byte between the platforms, so the durations match without a second implementation.One structural asymmetry, recorded deliberately: Android's header block is
suspendand reads the registry inline; Swift'sstrapStateLines()is sync and prefs-only by contract ("SYNC, offline-safe"), so the Swift inventory takes its own store handle inside the asyncdynamicLines. Same output, same position in the log, different seam — noted so nobody "aligns" them by makingstrapStateLines()async.A limitation on Apple, found in re-review
The ids the inventory prints redact differently on the two platforms, because they are minted
differently:
whoop-<MAC>whoop-F1:••:••:••:••:DE— two straps stay distinguishablewhoop-<CoreBluetooth UUID>whoop-<device>— identical for every strapredactPii's third rule masks a peripheral UUID wholesale, so on Apple every re-added strap readswhoop-<device>. The inventory is still useful there — counts, statuses, models andlastSeenalldiffer per row — but the id cross-reference is not:
dayOwner readId=collapses to the same token,so a row cannot be tied to the day that read it.
This is pre-existing (it is the redactor's behaviour, not this block's) and the inventory inherits it
rather than causing it. Not fixed here. The obvious remedy is a short stable non-identifying
discriminator beside the id — an FNV-1a of the id, per the cross-platform hashing rule, which survives
redaction because it is not the identifier — and that deserves its own change rather than being folded
into this one.
Verification
compileFullDebugKotlinclean; fullcom.noop.testcentresuite (131 tests) green.StrandTestsfile.doc_comment_lintandi18n_audit --ci mainclean.