ecg: admit the four ECG probe opcodes behind a closed send gate (the three Labrador toggles become constructible)

[Zebsi235]

Split 4 of the MG ECG research series; split 3 is merged (#1941 → 7ec7e778). This is the review-sensitive one.

What this PR does

Adds TOGGLE_LABRADOR_DATA_GENERATION(124), TOGGLE_LABRADOR_RAW_SAVE(125) and
TOGGLE_LABRADOR_FILTERED(139) to CommandNumber, adds the ECG probe opt-in key
(PuffinExperiment.ecgProbe, noopWhoop5Ecg — the same key string macOS already uses), and wires a send
gate into the single send() chokepoint:

private fun ecgSendAdmitted(cmd: CommandNumber): Boolean {
    if (!EcgResearchAllowList.isProbeOpcode(cmd.rawValue)) return false
    if (connectedFamily != DeviceFamily.WHOOP5) return false
    if (!whoop5Variant().isMG) return false
    if (!_state.value.connected) return false
    return puffinExperiment.ecgProbe
}

The gate admits four opcodes, not three. PROBE_OPCODES is {123, 124, 125, 139} and ecgSendAdmitted
keys off that set, so SELECT_WRIST(123) — already constructible on main, but absent from the 5/MG send guard and
therefore skipped today — becomes sendable to an attested MG whenever the probe opt-in is on. It is the one opcode of
the four the allow-list flags as persistent wrist config rather than a session toggle: a toggle stops when the
session does, a wrist-config write does not. That is a different risk class, and it is why the probe PR offers no
control for it (its right/left mapping is unverified on hardware and one value the measured firmware refuses).

This answers the comment it removes. Enums.kt excluded these three on purpose: "Android has no ECG
app layer and sends none of them, so growing the SENDER enum to buy a label would widen what the command
sender can express for nothing." The label half is still right — #893's read-only CommandNames table
names every opcode without making any constructible, and nothing here is added for a label. What changed
is the premise: the gated MG ECG research probe (#891/#1100) is an Android ECG app layer, and it does send
them, exactly as Apple's WhoopCommand already does for the same probe.

Adding them does not widen the reachable surface, which is the property that matters. Every send
passes the one send() chokepoint; there the opcode must be in the positive allow-list
EcgResearchAllowList.PROBE_OPCODES (a closed set of four — #1941), the probe opt-in must be on, and the
strap must be a connected, positively-attested MG. The firmware-load family three codes above 139
(142/143/144) is not expressible through it — asserted by senderEnumStaysCuratedAfterTheLabelFix, which
is untouched and still green.

The enum is also a lookup table, and that side does not leak either (checked in review): CommandNumber.fromRaw
has one caller in the tree, advanceDeviceConfigProbe, independently gated on DeviceConfigReadProbe.isReadOnlyOpcode,
so 124/125/139 becoming non-null there changes nothing.

With this PR merged and nothing else, no code path sends any of the four. There is no ECG caller on
Android yet and no Settings row sets the opt-in. The enum widening and the gate that bounds it land
together, so the probe that needs them (next PR) lands against a gate already in main, and tightens it
(arming state, stop-override) rather than introducing it. A reviewer can check "nothing sends" by looking
for a caller: there is none.

Type of change

• Bug fix
• New feature
• Refactor / cleanup
• Documentation
• CI / tooling

How it was tested

Android unit tests, full suite on 3ec6f49f (the code head), base main @ 0e6845cd: 5,554 tests, 0 failures,
0 errors, 662 classes — main's 5,553 plus the restored guard. Counted from the JUnit XML with the results
directory cleared first, oracle synced in its own invocation. 986b9153 on top is comment-only (the review's two
notes); CommandCatalogueTest + EcgResearchAllowListTest and doc_comment_lint re-run on it.

The tests that change, deliberately:

• EcgResearchAllowListTest.everyDispatchableOpcodeIsAConstructibleCommandNumber — restored. It asserts
  every allow-listed opcode is a constructible CommandNumber, so an allow-listed opcode the sender cannot
  form is caught. It was held out of ecg: the pure research modules — allow-list, stats and log #1941 because it would have failed there for the correct reason
  (CommandNumber must carry opcode 139); this PR is what makes it true.
• CommandCatalogueTest.theMgEcgTogglesAreNamedButNotSendable → …AndNowConstructibleBehindTheSendGate.
  The assertNull on 124/125/139 flips to asserting their raw values, with the reasoning in its KDoc.
  Its neighbour senderEnumStaysCuratedAfterTheLabelFix (25/36/37/38/45/99/142/143/144 stay
  unconstructible) is untouched — the evidence that moving the enum's top from 123 to 139 dragged nothing
  in.

Gates: Tools/doc_comment_lint.py clean · Tools/i18n_audit.py --ci upstream/main clean (no strings —
there is deliberately no Settings row in this PR).

Not run on hardware — by construction nothing here can send. Swift: n/a, no Swift touched;
swift test unchanged.

What this does NOT claim

• That anything sends. Nothing can, and the body says how to check.
• Anything about what the strap does with 123/124/125/139 — the hardware evidence belongs to the probe PR.
• That the 4.0/5.0 negative case is exercised on hardware here; the gate refuses on family and isMG
  by construction, and the probe PR carries the hardware run.

Checklist

• Swift package tests pass for any package I touched — n/a, no Swift touched
• Android unit tests pass (./gradlew testFullDebugUnitTest, full suite)
• No new build warnings introduced
• UI changes use only StrandDesign tokens — n/a, no UI
• No hardcoded hex frame bytes; protocol facts live in the schema / decoders — the three enum values
  are the schema numbers, already in CommandNames
• Follows the conventions in docs/CONTRIBUTING.md
• I did not commit generated output or any secrets/keystores

Related issues

#891, #1100 · builds on #1941 (the allow-list, merged as 7ec7e778), #1765, #1727 · #893 (the label table this
deliberately does not duplicate). Does not close anything.

[ryanbr]

Reviewed at 3ec6f49f. The safety argument holds, and I checked it rather than taking the body's word for it. Two notes, neither of them code.

What I verified independently

• Full suite reproduces exactly. 5,554 tests, 662 classes, 0 failures, 0 errors, results directory cleared first and the oracle synced in its own invocation. Same numbers the body claims.
• Nothing can send. No caller anywhere forms the three new entries, ecgProbe has no writer in the tree (no Settings row, as stated), and ecgSendAdmitted has exactly one call site: the send() chokepoint. So it returns false in any build of this branch.
• The MG attestation is genuinely positive. isMG is this == MG and UNKNOWN is its own case, so an unattested strap fails the gate rather than passing it by default. That is the part that would have been easy to get wrong.
• Widening the enum cannot leak through the decode side. CommandNumber.fromRaw has one caller in the whole tree, advanceDeviceConfigProbe, and it is independently gated on DeviceConfigReadProbe.isReadOnlyOpcode. So 124/125/139 becoming non-null there changes nothing. Worth stating because "the enum is also a lookup table" is the non-obvious half of the risk, and the body only argues the send half.

The gate admits four opcodes, not three

PROBE_OPCODES is {123, 124, 125, 139}, and ecgSendAdmitted keys off that set, so the new clause admits SELECT_WRIST(123) as well. On main, 123 is not in the 5/MG send guard at all, so it is skipped today. After this, it is sendable to an attested MG whenever the probe opt-in is on.

The inline comment in WhoopBleClient does say "the ECG opcodes 123/124/125/139", so this is deliberate and I am not asking for a code change. It is needed by the probe and it belongs inside the same gate. But three other places understate it:

• the title and body say "the three Labrador opcodes"
• the test comment says SELECT_WRIST(123) was already here and is untouched by this change

That last one reads as "123 does not change", when what is untouched is the enum entry. The send surface for 123 does change, and it is the one opcode of the four the allow-list itself flags as PERSISTENT wrist config rather than a session toggle. Different risk class from the other three: a toggle stops when the session does, a persistent config write does not.

Could you say four in the body, and give 123 a sentence saying it is persistent and newly admitted? The reviewer heuristic the body offers ("check nothing sends by looking for a caller") stays true either way, but someone auditing the wire surface later should not have to derive the fourth opcode from the allow-list.

A test lost its doc comment

The KDoc that used to sit above noForbiddenOpcodeIsReachableThroughTheProbeCases was the "the companion guard is deliberately NOT here" note. Repurposing it for the new everyDispatchableOpcodeIsAConstructibleCommandNumber was the right call since the deferral is over, but it leaves noForbiddenOpcodeIsReachableThroughTheProbeCases with no doc at all. Worth a one-liner.

On the change itself

The reasoning is right and the sequencing is the part I want to call out as good practice: landing the enum widening and the gate that bounds it together, so the probe arrives against a gate already in main and can only tighten it. That is the opposite of the usual order, and the usual order is how a gate ends up shipping a release behind the thing it is supposed to bound.

senderEnumStaysCuratedAfterTheLabelFix staying untouched and green is the evidence that carries the most weight here, since it is the assertion that moving the enum's top from 123 to 139 dragged none of 142/143/144 in with it.

[Zebsi235]

Thanks — and for the decode-side check; the body now states that half too, credited to the review.

Both notes taken in 986b9153 (comment-only; CommandCatalogueTest + EcgResearchAllowListTest and doc_comment_lint re-run):

• Four, not three. Title and body say so, and 123 has its own paragraph: already constructible on main but skipped by the 5/MG send guard, newly admitted through this gate to an attested MG with the opt-in on, and the one of the four the allow-list flags as persistent wrist config. The test comment now says what changes for it — the send surface, not the enum entry.
• noForbiddenOpcodeIsReachableThroughTheProbeCases has its one-liner back.

[ryanbr]

Re-reviewed at 986b9153. The two things I held this on are done: the body now leads with the gate admitting four opcodes rather than three, and the restored test KDoc is back. Thank you @Zebsi235.

Because this one widens what the command sender can express to a strap, I checked the containment argument link by link rather than reading it:

• The allow-list is the pre-existing closed set of four and already annotates 123 as persistent wrist config, so the send gate is not inventing a set.
• The gate is a five-way conjunction: allow-listed opcode, family WHOOP5, isMG, connected, opt-in on.
• The opt-in defaults false, is prefs-backed, and nothing outside tests calls its setter. There is no Settings row.
• Nothing sends. Every occurrence of the three new enum names in the tree is a doc comment or a label string. No call site exists.
• The chokepoint claim is literally true. writeQueue.add has exactly one caller, enqueueWrite, which has exactly two callers, both inside send() and both after the skip guard. Nothing reaches the characteristic by another route.
• 142/143/144 remain unconstructible, asserted by senderEnumStaysCuratedAfterTheLabelFix, which this PR genuinely does not touch. The only diff line naming it is a cross-reference added to a neighbouring KDoc.
• Full Android suite locally: 662 classes, 5,554 tests, 0 failures, 0 errors, which is your figure exactly. CI is green on 3 checks, the right roster for an Android-only change.

So the "with this merged and nothing else, no code path sends any of the four" claim is accurate, and I was able to check it the way the body says a reviewer can: by looking for a caller and finding none.

One placement change I would like

The SELECT_WRIST(123) explanation landed in CommandCatalogueTest's KDoc. That is the right home for the constructibility change, but the sentence is about the send surface, and the send surface is ecgSendAdmitted. Someone auditing what this strap can be told to do reads the gate, not a test file.

Could you move or copy those three lines into ecgSendAdmitted's own KDoc? It already explains why nothing can reach it today; the fact that one of the four it admits is persistent wrist config rather than a session toggle belongs in the same paragraph.

A question for the next PR, not a change here

The gate keys off PROBE_OPCODES, so it admits 123, while the probe PR deliberately offers no control for it because the right/left mapping is unverified and one value the measured firmware refuses. That leaves an opcode admitted by the send gate that nothing is ever intended to drive.

Today that is harmless, since nothing sends at all, and I can see the argument for one source of truth: a second, narrower "sendable" set could drift from what Whoop5Ecg.commandFrame forms. So I am not asking you to split it.

What I would like is that the probe PR treats 123's admission as an open question to argue, rather than as settled by this one. If that PR ships without a control for it, the gate should narrow to the three session toggles at that point. Worth a line in ecgSendAdmitted saying so, so the next reader inherits the question rather than the assumption.

Move the SELECT_WRIST note to the gate and I am happy to merge this.