fix(flipper): refuse Reconnect over BLE instead of tearing down the live link#164
Merged
Conversation
…ive link
Flipper.Reconnect (driven by the /reconnect slash command and POST
/api/reconnect) guarded only bridgeMode, not the BLE transport. On a
BLE-connected Flipper it ran reconnectIfNeededLocked, which:
1. calls transport.Reconnect — the BLE transport tears down and
re-establishes the peripheral, then
2. calls handshake — which writes \x03\r and waits for the text "> " CLI
prompt. BLE exposes only Protobuf RPC, never a text CLI (rpcMode is
permanently latched), so the handshake never sees a prompt, times out, and
the loop retries transport.Reconnect — tearing the working link down again
— until connectTimeout, then returns "reconnect timed out".
Net effect: /reconnect (or /api/reconnect) on a BLE Flipper repeatedly bounced
a healthy connection and reported failure, leaving bleClient session state out
of sync with the freshly re-established transport beneath it. A recovery
command actively broke a working session.
Fix: Flipper.Reconnect now short-circuits IsBLE() (under f.mu, before touching
the transport) with usbOnlyError("reconnect") — a clear, actionable error that
wraps ErrCommandRequiresUSB, consistent with how other CLI-only commands
respond on BLE. The live link is left untouched.
A BLE-aware recovery (re-open the peripheral + RPC client, skipping the CLI
handshake and the device_info identity check) plus BLE drop auto-detection are
larger, BLE-hardware-dependent follow-ups, noted for a steered session.
Found via the internal audit sweep.
Test: TestReconnect_BLE_RefusesWithoutTearingDownLink uses a fake BLE transport
that records whether Reconnect/Close were invoked and asserts the call is
refused (errors.Is ErrCommandRequiresUSB) WITHOUT calling transport.Reconnect.
Proven to FAIL on the pre-fix code (transport.Reconnect called; handshake hit
the ctx deadline).
Verified: task ci green (lint 0 / vet / build / test:full 0 fail / govulncheck
clean); task eval 17/17.
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
1 participant
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.
Summary
A MEDIUM-HIGH correctness bug:
Flipper.Reconnect(driven by the/reconnectslash command andPOST /api/reconnect) guarded onlybridgeMode, not the BLE transport. On a BLE-connected Flipper it ranreconnectIfNeededLocked, which:transport.Reconnect— the BLE transport tears down and re-establishes the peripheral, thenhandshake— which writes\x03\rand waits for the text"> "CLI prompt. BLE exposes only Protobuf RPC, never a text CLI (rpcModeis permanently latched), so the handshake never sees a prompt, times out, and the loop retriestransport.Reconnect— tearing the working link down again — untilconnectTimeout, then returns "reconnect timed out".Net effect:
/reconnect(or/api/reconnect) on a BLE Flipper repeatedly bounced a healthy connection and reported failure, leavingbleClientsession state out of sync — a recovery command actively breaking a working session.Fix
Flipper.Reconnectnow short-circuitsIsBLE()(underf.mu, before touching the transport) withusbOnlyError("reconnect")— a clear, actionable error wrappingErrCommandRequiresUSB, consistent with how other CLI-only commands respond on BLE. The live link is left untouched.A BLE-aware recovery (re-open peripheral + RPC client, skip the CLI handshake + identity check) and BLE drop auto-detection are larger, BLE-hardware-dependent follow-ups, noted for a steered session.
Found via the internal audit sweep.
Verification
TestReconnect_BLE_RefusesWithoutTearingDownLink: a fake BLE transport records whetherReconnect/Closewere invoked; asserts the call is refused (errors.Is ErrCommandRequiresUSB) without callingtransport.Reconnect. Proven to FAIL on the pre-fix code (transport.Reconnectcalled; handshake hit the ctx deadline).task cigreen (lint 0 / vet / build /test:full0 fail / govulncheck clean);task eval17/17.Bug fix — anchors v0.782.0.