fix(vehicle): prevent unsafe BLE write timeout failover#70
Merged
Conversation
added 5 commits
July 12, 2026 17:02
A GATT write that reaches backend I/O and then fails/times out was wrapped as BluetoothTransportError, the same exception raised for a provably pre-submission failure. VehicleRouter fails over on BluetoothTransportError, so a submitted-then-ambiguous write could be replayed on the cloud secondary and double-execute a non-idempotent command - field data showed such writes reaching the vehicle about as often as not. Split the write path in VehicleBluetooth._send: only BleakCharacteristicNotFoundError (raised synchronously before any backend I/O) stays BluetoothTransportError. Every other write failure races an already-armed broadcast watcher, then raises plain BluetoothTimeout, which lands in the same confirmation-ladder handling as a lost ack (BluetoothUnconfirmedCommand + raise_unconfirmed) instead of being treated as safe to retry. _send_optimistic gets the same treatment since it bypasses the ladder. Also close a related gap found during review: get_private_key() wrote its PEM with the process default mode instead of 0600, unlike the sibling get_rsa_private_key().
This was referenced Jul 12, 2026
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.
Intent
Close the last BLE write-timeout double-execution gap in the VehicleBluetooth/VehicleRouter confirmation ladder, per an upstream Home Assistant review finding confirmed by field data (tfa-ble-timeout-rates-j3 study: of 3 verifiable BLE transport-errors, 2 had actually executed on the vehicle). Previously, VehicleBluetooth._send() wrapped ANY write_gatt_char failure (BleakError or TimeoutError, whether pre-submission or mid-backend-I/O) into BluetoothTransportError, and VehicleRouter fails over to the cloud secondary on that exception - so a write that was actually submitted and reached the vehicle before the local wait failed could be replayed on the cloud transport, double-executing a non-idempotent command (locks, toggles, schedule add/remove, etc).
Fix: split write-failure classification by delivery certainty in _send()'s write try/except. Only BleakCharacteristicNotFoundError - raised synchronously by bleak while resolving the WRITE_UUID characteristic, strictly before any backend I/O - stays BluetoothTransportError (provably pre-submission, safe for Router to retry). Every other BleakError/TimeoutError from write_gatt_char happens inside backend I/O (D-Bus/CoreBluetooth/an ESPHome proxy) where delivery can't be proven either way, so instead of raising BluetoothTransportError it: (1) races any already-armed broadcast watcher for the remaining window (a matching VCSEC status broadcast can still confirm success despite the failed write call), then (2) raises plain BluetoothTimeout. Because BluetoothUnconfirmedCommand subclasses BluetoothTimeout, this lands in the exact same 'except BluetoothTimeout' ladder in _sendVehicleSecurity/_sendInfotainment that already handles a lost post-write ack (verify-by-state rung, then raise_unconfirmed gating) - no new exception type needed. _send_optimistic() (used by confirmation='optimistic') previously ONLY ever raised BluetoothTransportError unconditionally on any write failure ('consulting nothing else'); it now also follows this same split - a provable pre-submission failure still always raises, but a delivery-ambiguous failure now follows raise_unconfirmed instead of always raising, since even optimistic/no-wait mode must not let a fallback router blind-retry a command that may have already reached the car. This required threading a command-name string into _send_optimistic for its debug logging (previously only took domain+bytes).
Deliberately scoped narrow: reads (_getVehicleSecurity/_getInfotainment) are NOT touched by this change - they don't override the ladder, so a write-ambiguous read still propagates plain BluetoothTimeout and Router still fails over on it normally, which stays safe since reads carry no double-execution risk. The pre-write connect_if_needed() path (already outside this try/except, wrapped separately by connect()) is unaffected.
Also fixed, found during self-review while investigating this: get_private_key() in tesla.py wrote its PEM (the Fleet API/BLE command-signing EC key) with the process default file mode instead of 0600 - confirmed the upstream HA review's claim was accurate. Mirrored the exact existing chmod(path, 0o600) pattern already used by the sibling get_rsa_private_key() a few lines below in the same file (that one already had it correct), wrapped in the same try/except OSError: pass. This was judged small and low-risk enough (4-line mechanical fix copying an established in-file pattern, not a new design decision) to include in this PR rather than deferring, plus one new test asserting the created file's mode is 0o600.
Tests added: tests/test_ble_send_transport.py (split BleakCharacteristicNotFoundError-vs-other-write-failure classification, driving _send() directly with a mocked GATT client), tests/test_ble_broadcast_confirmation.py new WriteFailureBroadcastRaceTests class (a matching broadcast still confirms success despite write_gatt_char raising, and a write timeout with no broadcast still raises BluetoothUnconfirmedCommand - drives the real unmocked _send state machine), tests/test_ble_optimistic_and_best_effort.py (optimistic mode's new raise_unconfirmed-following behavior for ambiguous writes), tests/test_ble_write_timeout_router.py (new file - the actual double-exec regression test: a real VehicleBluetooth wrapped in a real VehicleRouter with a fake cloud fallback, proving the fallback's door_lock is never invoked when the primary's write is submitted-then-ambiguous, contrasted with a provable pre-submission failure which still correctly fails over), tests/test_tesla_private_key.py (new file - PEM file mode assertion). Updated existing tests in test_ble_send_transport.py that previously asserted BluetoothTransportError for a generic BleakError/TimeoutError from write_gatt_char - those now correctly assert BluetoothTimeout instead, since only the characteristic-not-found case is pre-submission-provable.
Docs: docs/bluetooth_vehicles.md's confirmation-ladder section gained a new markdown table with an explicit GATT-write rung row describing the pre-submission-vs-ambiguous split; the BluetoothTransportError paragraph and the optimistic-mode/raise_unconfirmed bullets were also corrected since they previously said 'only a write/transport failure raises' unconditionally, which is no longer true. AGENTS.md (the actual file behind the CLAUDE.md symlink) got a new bullet documenting the write-delivery-certainty split plus corrections to three existing stale cross-references that previously implied ALL write_gatt_char failures raise BluetoothTransportError.
All local checks pass: ruff check, pyright --strict on tesla_fleet_api (tests/ is intentionally not pyright-strict per this repo's existing CI config, confirmed by reading .github/workflows), and pytest (291 tests, up from 290 baseline). No CHANGELOG.md or other auto-generated files were touched.
What Changed
BluetoothTransportError, while ambiguous backend write failures now flow through the timeout/unconfirmed confirmation ladder to avoid unsafe router replays.Routercoverage for ambiguous BLE writes, with regression tests for fallback suppression and safe pre-submission failover.0600permissions and refreshed BLE timeout documentation to match the new behavior.Risk Assessment
✅ Low: The change is narrowly scoped to BLE write-failure classification, preserves the router no-replay invariant for ambiguous mutating writes, and includes focused regression coverage and matching docs.
Testing
Inspected the target diff, ran the focused BLE transport/router/optimistic/private-key tests, generated a JSON evidence transcript showing router fallback behavior and
0o600key mode, then ran the full test suite; all automated tests passed after retrying the evidence script withuv run pythondue to no systempythonbinary.Evidence: BLE write-timeout behavior evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
tesla_fleet_api/tesla/vehicle/bluetooth.py:744- On an ambiguouswrite_gatt_charfailure, this path waits only for the broadcast watcher and never drains the addressed response queue. If the write actually reached the car and an addressed ack or rejection arrives before/after the local write timeout,_send()still raisesBluetoothTimeout; the mutating wrapper can then return best-effort success withraise_unconfirmed=False, hiding a real car-side rejection. Treat this like the normal post-write wait: race_await_responseas well as any broadcast watcher before falling through to unconfirmed.🔧 Fix: Handle addressed replies after write failures
2 issues (1 warning, 1 info) still open:
tesla_fleet_api/tesla/vehicle/bluetooth.py:732- GenericBleakErroris not always delivery-ambiguous: Bleak backends can raiseBleakError("Not connected")before issuing the backend write call (for example, BlueZ checks connection state beforeWriteValue: https://raw.githubusercontent.com/hbldh/bleak/v2.0.0/bleak/backends/bluezdbus/client.py). If the link drops afterconnect_if_needed()but beforewrite_gatt_char(), this now becomesBluetoothTimeout, so mutating commands can return best-effort success or block router failover even though nothing was submitted. Classify known pre-submissionBleakErrorcases such as not-connected asBluetoothTransportErrorbefore the ambiguous branch.README.md:175- The top-level README still says BLE GATT write failures raiseBluetoothTransportError, but this change intentionally makes most write failures raiseBluetoothTimeout/BluetoothUnconfirmedCommand. Update this summary so users who catchBluetoothTransportErrorseparately do not miss the new ambiguous-write behavior.🔧 Fix: Classify disconnected BLE writes pre-submission
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
git diff --stat 623bfb19d1e9243c47cb485954722d9585fa2d38..HEADandgit diff --name-only 623bfb19d1e9243c47cb485954722d9585fa2d38..HEADuv run pytest tests/test_ble_send_transport.py::SendTransportErrorTests tests/test_ble_broadcast_confirmation.py::WriteFailureBroadcastRaceTests tests/test_ble_optimistic_and_best_effort.py tests/test_ble_write_timeout_router.py tests/test_tesla_private_key.pypython - <<'PY' > /tmp/no-mistakes-evidence/01KXAJ9P4GFAZD0615S31N64MG/ble_write_timeout_behavior.json(failed becausepythonis not installed in PATH; retried successfully withuv run python)uv run python - <<'PY' > /tmp/no-mistakes-evidence/01KXAJ9P4GFAZD0615S31N64MG/ble_write_timeout_behavior.jsonuv run pytest tests✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.