fix: clear the CCCD when stopping notifications

[bluetoothbot]

What: stop_notify() now writes 0x0000 to the CCCD on connection-v3 proxies, so the peripheral actually stops notifying.

Why: On proxies advertising REMOTE_CACHING, the esp32 wipes the resolved descriptors to save memory, so start_notify() writes the CCCD itself — that branch is already in the code and documented. Nothing ever undid it. The firmware's notify_characteristic() only calls esp_ble_gattc_register_for_notify / esp_ble_gattc_unregister_for_notify, and neither touches the CCCD. So stop_notify() dropped the proxy-side subscription while the peripheral kept sending notifications for the life of the connection: battery drain on the peripheral, wasted airtime on the proxy, and devices that gate behaviour on subscription state left stuck in streaming mode.

How: Mirror the start_notify branch — same REMOTE_CACHING guard, same get_descriptor(CCCD_UUID) lookup. The CCCD write goes first so the peripheral is quiet before the subscription is released, and the release sits in a finally so a failing write cannot leave the local bookkeeping drifted from the already-popped handle. Non-REMOTE_CACHING firmware resolves descriptors itself and is left alone.

Testing: 176 pass, 100% line + branch coverage retained. Three new cases: CCCD cleared under REMOTE_CACHING, skipped without it, and the proxy subscription still released when the CCCD write raises.

---

Quality Report

Changes: 2 files changed, 123 insertions(+), 3 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4a773ee) to head (860d69a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #410   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          796       835   +39     
  Branches        88        94    +6     
=========================================
+ Hits           796       835   +39     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codspeed]

Merging this PR will not alter performance

✅ 5 untouched benchmarks

---

_{Comparing bluetoothbot:koan/stop-notify-disables-cccd (860d69a) with main (4a773ee)¹}

Footnotes

1. No successful run was found on main (6196df3) during the generation of this report, so 4a773ee was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Lint clean. mypy errors pre-existing in cache.py/connect.py, none in client.py.
• HIGH silent-skip: split the and short-circuit into an explicit if not cccd_descriptor branch that logs a _LOGGER.warning naming the characteristic UUID, so a missing CCCD under REMOTE_CACHING is visible instead of silently leaving the peripheral notifying (src/bleak_esphome/backend/client.py:844).
• MEDIUM exception masking: replaced the bare finally: await notify_stop() with except BaseException: + contextlib.suppress(Exception) around the release, then a normal await notify_stop() on the success path — the CCCD-write error stays the propagated root cause while the subscription is still released, matching how start_notify treats bluetooth_gatt_stop_notify as best-effort (src/bleak_esphome/backend/client.py:867).
• Added test_stop_notify_warns_when_cccd_missing (asserts the warning, no descriptor write, subscription released) and test_stop_notify_cccd_failure_survives_failing_release (release raises during cleanup; BleakError from the CCCD write still propagates) to cover the two new branches.

Stats

2 files changed, 201 insertions(+), 4 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still just calls notify_stop() with no CCCD write or REMOTE_CACHING guard, so the)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run still pending
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Replaced contextlib.suppress(Exception) around the cleanup notify_stop() with try/except BaseException + _LOGGER.warning(..., exc_info=True) naming the handle, per the HIGH finding — a failed proxy-side release is now visible in logs instead of silently discarded.
• Same change covers the MEDIUM cancellation finding: except BaseException catches CancelledError from the cleanup await, so the original CCCD-write error always wins.
• Documented the warn-only missing-CCCD path in stop_notify's docstring: a successful return does not guarantee the peripheral stopped notifying (MEDIUM finding; picked the docstring option over raising to keep behavior backward-compatible for existing callers, closing the two unchecked checklist items).
• Extended test_stop_notify_cccd_failure_survives_failing_release with a caplog assertion on the new warning.
• Added test_stop_notify_cccd_failure_survives_cancelled_release: a CancelledError from the release still surfaces the CCCD BleakError and pops the handle.

Not changed (and why)

• Raising BleakError on the missing-CCCD path (alternative in the MEDIUM finding). It would make stop_notify raise where it previously succeeded, breaking existing callers on firmware that drops the descriptor. Reviewer offered documenting as an equally valid fix; took that.

Stats

2 files changed, 254 insertions(+), 4 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only awaits notify_stop() with no CCCD write, and no commit on main address)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run still pending
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Done.
• Let CancelledError from notify_stop() propagate instead of demoting it to a log line and re-raising the CCCD error (narrowed the inner handler from BaseException to Exception), per the swallowed-CancelledError finding; test_stop_notify_cccd_failure_survives_cancelled_release now asserts the cancellation propagates.
• Stopped popping the handle up front: stop_notify now gets the entry and deletes it only after the proxy-side release succeeds, so a failed release leaves the subscription retryable instead of leaking with a silent no-op on retry (both the error path and the previously unguarded success path).
• Extended the stop_notify docstring with the behavior change for existing callers: the CCCD write is a peripheral round trip bounded by the proxy GATT timeout, the method can now raise BleakError where it previously always returned, and a retry resumes the failed part.
• Added test_stop_notify_keeps_entry_when_release_fails covering the success-path release failure, and updated test_stop_notify_cccd_failure_survives_failing_release to assert the entry is retained.

Not changed (and why)

• "Log the leaked handle on the success path too": not needed as stated. On the success path the release failure now propagates to the caller as a real exception and the entry is retained for retry, so nothing is silent — a warning would duplicate the raised error. The warning is kept only in the error path, where the CCCD error masks the release failure.

Stats

2 files changed, 285 insertions(+), 4 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the handle and awaits notify_stop with no CCCD write, and no comm)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Done.
• Replaced both del self._notify_cancels[characteristic.handle] with pop(handle, None) per the HIGH finding — a disconnect (_async_disconnected_cleanup() → _notify_cancels.clear()) or a concurrent stop_notify during the awaited notify_stop() would otherwise raise a bare KeyError, masking the actionable CCCD error in the except BaseException branch. Matches the defensive pop(ble_handle, None) in start_notify's error path.
• Corrected the stop_notify docstring retry claim per the MEDIUM finding: only a failed proxy-side release is retryable; a failed CCCD write is terminal for that subscription (entry dropped, retry returns silently). Chose the docstring fix over a "CCCD still enabled" marker — new state for a case no caller tracks today.

Not changed (and why)

• Missing-CCCD warning-only degradation (MEDIUM, client.py:859-871) — disagree. Raising BleakError here would fail stop_notify for a condition the caller cannot act on, on the path callers use precisely for cleanup and teardown. The descriptor can only be missing at stop time if it disappeared after a successful subscribe, and the proxy-side release still succeeds. The asymmetry with start_notify is intentional: a missing CCCD at subscribe time means notifications were never enabled (a real failure), at unsubscribe time it means degraded cleanup. The behaviour is documented in the docstring. Surfacing it via a return value or callback would change the BleakClient backend contract — out

Stats

2 files changed, 291 insertions(+), 4 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify at src/bleak_esphome/backend/client.py:949 still only pops the handle and await)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Lint clean.
• Passed GATT_NOTIFY_TIMEOUT explicitly to the CCCD-clear bluetooth_gatt_write_descriptor call so both halves of the notify pair pin the same repo-owned constant (suggestion feat: initial import #1), and updated test_stop_notify_disables_cccd to assert the fourth argument.
• Documented in the stop_notify docstring that a retained entry after a failed release also blocks start_notify on that handle, and that a disconnect clears it (suggestion feat: move mac_to_int helper to bluetooth_data_tools #2).
• Added two regression tests pinning the pop(handle, None) fix against a concurrent _notify_cancels.clear() — one on the success path, one on the CCCD-failure path — which del would fail (suggestion feat: add available property to know when the ESP device is connected #3).
• Extended the REMOTE_CACHING row in docs/usage.md:329 to state that stop_notify likewise skips the CCCD clear without the flag (suggestion feat: add connect scanner helper #4).

Not changed (and why)

• Suggestion feat: move mac_to_int helper to bluetooth_data_tools #2's alternative "reconsider retaining the entry" — kept the retain-on-failed-release semantics and documented the consequence instead, which is the option the reviewer offered first; reverting to an unconditional pop would drop the retry the earlier review round asked for.
• Silent-failure MEDIUM feat: initial import #1 (raise BleakError on missing CCCD) — the reviewer explicitly agreed in the same review to keep the warning-only path ("I agree and am not re-raising it"). A missing CCCD at teardown means degraded cleanup on a path callers use to tear down; raising would break teardown for callers who cannot act on it.
• Silent-failure MEDIUM feat: move mac_to_int helper to bluetooth_data_tools #2 (track un-cleared C

Stats

3 files changed, 363 insertions(+), 5 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the handle and awaits notify_stop() with no CCCD write, so the pe)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Lint clean; mypy errors pre-existing in cache.py/connect.py, none in client.py.
• Missing CCCD under REMOTE_CACHING now raises BleakError with the same message start_notify uses instead of logging a warning and returning success, so stop_notify can no longer report success while the peripheral keeps notifying (silent-failure item 1). Proxy release still happens first; docstring updated; test_stop_notify_warns_when_cccd_missing replaced by test_stop_notify_raises_when_cccd_missing.
• Release failure is now attached to the propagated error via add_note(), spelling out that the handle stays blocked for start_notify until a later stop_notify succeeds or the device disconnects, so the caller can tell it apart from the CCCD failure (item 2). Warning log retained; the existing failing-release test now asserts the note.
• Restored the atomic single-winner semantics: the entry is pop()ed up front again and put back only on the retryable release-failure path, on both the error and success legs, so a concurrent second stop_notify is a no-op instead of a duplicate CCCD write plus duplicate release (item 3). Added test_stop_notify_is_single_winner_when_reentered, which re-enters stop_notify from inside the CCCD write.
• Adjusted two tests that relied on the old silent-skip behavior to clear REMOTE_CACHING instead of stubbing an absent descriptor.

Stats

3 files changed, 408 insertions(+), 13 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=Current stop_notify on main still only pops the cancel and awaits notify_stop() with no CCCD wri)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Both findings verified against current code and fixed.
• HIGH (state resurrected after concurrent cleanup): added _restore_notify_cancel() and routed both restore sites through it. It restores only while _is_connected; after a disconnect it runs the entry's notify_abort() instead — the abort _async_disconnected_cleanup skipped because the entry was already popped — and drops the entry, so a reconnect can start_notify that handle again. Restore uses setdefault so a subscription created after the pop is not clobbered.
• MEDIUM (note lands on an exception the caller never sees): added _with_notes() in api_error_as_bleak_error and applied it at all four re-raise sites, so __notes__ on the underlying api error carry onto the BleakError/TimeoutError the caller catches. stop_notify's release-failure note is now visible in the caught error's traceback.
• Made the release-failure note conditional: the "start_notify stays blocked" sentence is only added when the entry was actually retained (_restore_notify_cancel now returns whether it kept the entry).
• Updated the stop_notify docstring: the note is on the error the caller receives, and a release failure after a disconnect drops the entry rather than retaining it.
• Tests: test_stop_notify_cccd_failure_survives_failing_release now asserts the note on excinfo.value rather than __cause__; added test_stop_notify_drops_entry_when_release_fails_after_disconnect (disconnect mid-CCCD-write → entr

Stats

3 files changed, 540 insertions(+), 21 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=Current stop_notify on main only pops the handle and awaits the proxy-side release with no CCCD wr)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Changes applied. Lint, format, mypy clean.
• Suggestion feat: initial import #1: replaced isinstance(release_err, asyncio.CancelledError) with
• not isinstance(release_err, Exception) in stop_notify, so KeyboardInterrupt and
• SystemExit also propagate instead of being demoted to a note on the CCCD error.
• Silent-failure feat: initial import #1 (dropped CCCD after terminal write failure): added a
• _cccd_dirty handle set. _async_clear_cccd() (extracted from stop_notify)
• records the handle on failure and forgets it on success; a stop_notify that finds
• no subscription entry now re-attempts the descriptor write when the handle is dirty
• instead of returning a silent success while the peripheral keeps notifying.
• _async_disconnected_cleanup() clears the set — the link is gone with it.
• Silent-failure feat: move mac_to_int helper to bluetooth_data_tools #2 (unchecked setdefault): _restore_notify_cancel now returns
• setdefault(...) is notify_cancel, and when a start_notify after the pop already
• owns the handle it runs the stale entry's abort half and logs, instead of reporting
• True and promising a retry that would never happen.
• Tests: new test_stop_notify_retries_cccd_after_a_failed_clear (failed clear →
• retry writes CCCD → third call is a no-op) and
• test_disconnect_forgets_the_outstanding_cccd_clear; extended
• test_stop_notify_release_failure_keeps_newer_subscription to assert the losing
• entry is aborted.
• Docstring: stop_notify's "a failed CCCD write is terminal ... a retry returns
• silently" paragraph

Stats

3 files changed, 654 insertions(+), 21 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the entry and awaits notify_stop with no CCCD write, and no commi)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run still pending
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Lint, format, mypy clean.
• Guarded _cccd_dirty.add() in _async_clear_cccd with if self._is_connected, mirroring _restore_notify_cancel, so a disconnect that lands inside the awaited descriptor write (or a TimeoutAPIError/CancelledError that never triggers a second cleanup) no longer leaves a handle marked dirty across the disconnect — src/bleak_esphome/backend/client.py:1121.
• Moved the missing-descriptor BleakError out of the try in _async_clear_cccd, so a characteristic with no CCCD is not latched as retryable and later stop_notify calls stay the BlueZ-compatible no-op — src/bleak_esphome/backend/client.py:1099.
• Extended test_stop_notify_raises_when_cccd_missing to assert the handle is not dirty and the next stop_notify is a no-op; asserted _cccd_dirty empty in test_stop_notify_drops_entry_when_release_fails_after_disconnect; added test_stop_notify_forgets_cccd_when_disconnected_mid_write for the TimeoutAPIError-after-disconnect path.

Not changed (and why)

• "Expose the retained/blocked state via a dedicated attribute or exception subclass" — disagree, YAGNI. Backend raises whatever error type the CCCD failure produced; a subclass would change the type callers already match on, and an ad-hoc attribute on a foreign exception is not a contract. No consumer needs to branch on this today, and the state is already recoverable without introspection: a later stop_notify retries the release, and a disconnect clears it. add_note plus a WARNI

Stats

3 files changed, 715 insertions(+), 21 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the cancel entry and calls notify_stop() with no CCCD write, )
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bdraco]

this looks messy. could be extracted into a function. also needs a dry audit

[bluetoothbot]

Agreed on both counts.

DRY audit finds two duplications. First: start_notify's v3 branch and _async_clear_cccd both do REMOTE_CACHING guard, get_descriptor(CCCD_UUID), a character-for-character identical missing-descriptor BleakError, debug log, then bluetooth_gatt_write_descriptor. Only payload bytes and timeout differ. Extract _async_write_cccd(char, value, timeout); both call sites collapse to one line, and two copies of that error string stop drifting.

Second: await notify_stop() appears twice, each wrapped in its own except BaseException calling _restore_notify_cancel. Extract _async_release_notify(handle, notify_cancel); nested try/except/try/except flattens.

Combined with dropping __notes__ machinery, roughly 45 lines go. Every branch already has a test, so a refactor stays verifiable — 248 pass, 100% branch coverage on client.py.

[bdraco]

Trim the AI essays. The comments are now massive here

[bluetoothbot]

@bdraco: Trim the AI essays. The comments are now massive here

Agreed, and measurable. 89 of 211 added non-blank lines in client.py read as prose against 122 lines of code. stop_notify docstring grew 22 lines describing internal state transitions.

Biggest single cut: delete __notes__ machinery. _with_notes, retained flag, and a 13-line conditional add_note all duplicate a _LOGGER.warning(..., exc_info=True) already firing at that same site. __notes__ stays invisible to except BleakError as e: str(e), so no caller can branch on it. Removal drops ~40 lines plus one helper, one TypeVar, and four decorator call sites whose blast radius covers every wrapped bluetooth operation.

Keep only genuinely non-obvious comments: why BaseException, why pop up front, why _is_connected guards restore. One or two lines each.

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Refactor applied. Lint/format/mypy clean on client.py (5 pre-existing mypy errors remain in untouched cache.py/connect.py).
• Extracted _get_cccd() (REMOTE_CACHING guard + get_descriptor(CCCD_UUID) + the single missing-descriptor BleakError) and _async_write_cccd(descriptor, value, timeout) (debug log + descriptor write); start_notify's v3 branch and _async_clear_cccd now share both, so the duplicated error string, guard, and write call exist once — per @bdraco's DRY audit and @bluetoothbot's suggestion ＃1.
• Extracted _async_release_notify(handle, notify_cancel, best_effort=…); the two await notify_stop() sites with their own except BaseException + _restore_notify_cancel collapse to two one-line calls, flattening the nested try/except/try/except in stop_notify.
• Dropped the __notes__ machinery: removed _with_notes(), the _E TypeVar, the four wrapped raises in api_error_as_bleak_error, and the err.add_note(...) block. The secondary release failure is now reported only via the existing WARNING log; _restore_notify_cancel no longer returns a bool that existed solely to word the note.
• Log the interrupted CCCD failure at WARNING before a non-Exception release error (cancellation/SystemExit) wins — addresses the MEDIUM "actionable error dropped on the cancellation path".
• Trimmed the now-stale stop_notify/_async_clear_cccd docstring prose that described the note mechanism.
• Updated `test_stop_notify_cccd_failure_survives

Stats

3 files changed, 714 insertions(+), 36 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=Current stop_notify on main only pops the entry and awaits notify_stop() with no CCCD write, so )
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run still pending
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Done.
• Dropped _restore_notify_cancel and the best_effort/_async_release_notify machinery per suggestion ＃1. Verified in the installed aioesphomeapi (client.py:1368-1390): stop_notify() awaits a body with no await, and bluetooth_gatt_stop_notify is sync + idempotent, so there is no cancellation point and a failure means the API connection is already gone — restoring the entry recorded an unretryable failure as retryable. stop_notify now releases inline with a single except Exception that logs the WARNING on the unwind path; ~55 lines and three docstrings gone.
• Fixed the dirty-retry race (suggestion ＃2): the _cccd_dirty.discard now runs only when the handle is not (re)subscribed, so a start_notify racing a clear leaves the handle dirty for the next stop_notify instead of silently declaring the CCCD clean.
• Fixed the MEDIUM silent no-op: a failed CCCD enable write in start_notify now marks the handle dirty too. Moved the dirty bookkeeping into _async_write_cccd (now takes the characteristic handle) so both call sites share one guarded add/discard; _async_clear_cccd is a thin wrapper. The missing-descriptor path still never latches dirty.
• Trimmed the stop_notify docstring and the inline comments the removed helpers referenced.
• Tests: removed the two cases that only covered the restore machinery (..._drops_entry_when_release_fails_after_disconnect, ..._release_failure_keeps_newer_subscription), updated three release-failur

Stats

5 files changed, 593 insertions(+), 59 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the notify cancel and awaits notify_stop() with no CCCD write)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Done. 44 lines removed, 29 added.
• Documented the retry contract in the public start_notify docstring: on a REMOTE_CACHING proxy a failed CCCD write may leave the peripheral notifying, and callers keeping the connection alive should call stop_notify on the same characteristic (suggestion feat: initial import #1).
• Renamed _async_write_cccd's handle parameter to char_handle so it cannot be silently confused with cccd_descriptor.handle.
• Trimmed prose per @bdraco's "trim the AI essays" ask: condensed the _get_cccd / _async_write_cccd / _async_clear_cccd docstrings and the _cccd_dirty, up-front-pop, dirty-retry, release-failure, and post-write comments; dropped the aioesphomeapi-idempotency essay. Net −15 lines, no behavior change.

Not changed (and why)

• docs/usage.md REMOTE_CACHING row — reviewer marked it optional; that row documents flag-absent behavior, and the failed-start_notify caveat belongs on the method contract, not in the flag table. Adding it would also work against the same review's prose-trim ask.
• Stale PR description (2 files / 123 insertions / 176 tests / finally) — PR body text, not code; regenerated by the tooling on push.

Stats

5 files changed, 578 insertions(+), 59 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops _notify_cancels and calls notify_stop() with no CCCD write,)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

Previous review — superseded by a newer review below.

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• Both suggestions applied.
• Corrected the stop_notify docstring retry claim: the missing-CCCD case raises on every call and has nothing to retry (suggestion feat: initial import #1).
• Added char_handle to the consolidated CCCD debug log so the descriptor handle is linked to its characteristic (suggestion feat: move mac_to_int helper to bluetooth_data_tools #2).

Not changed (and why)

• @bdraco "messy, needs a DRY audit" — already addressed in earlier commits on this branch (_get_cccd / _async_write_cccd / _async_clear_cccd extracted, __notes__ machinery dropped); the review's own checklist marks it closed.
• MEDIUM "swallowed exception with no retry path" (release failure log-only) — disagree. The retry machinery for the proxy-side release existed and was deliberately removed in 481ec0c after the maintainer called the nested-retry shape messy. Re-adding a _release_dirty set restores that complexity for a path that cannot self-heal usefully: the failing notify_stop() is a proxy round trip on a link that just failed a CCCD write, and any disconnect clears the subscription anyway. The failure is logged at WARNING with exc_info, and the CCCD error the caller receives is the actionable root cause. Flagging for human decision rather than relitigating.

Stats

5 files changed, 581 insertions(+), 59 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the notify cancel and calls notify_stop() with no CCCD clear,)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bluetoothbot]

PR Review — fix: clear the CCCD when stopping notifications

Both prior suggestions are applied and verified; the fix is correct and thoroughly tested. One doc clause overstates what the firmware does. Merge-ready.

Verified at af25b5b by re-running everything rather than trusting the diff:

• Suggestion feat: initial import #1 closed. The stop_notify docstring now says "retries a write that failed. A characteristic with no client config descriptor raises on every call and has nothing to retry" — it no longer contradicts test_stop_notify_raises_when_cccd_missing.

• Suggestion feat: move mac_to_int helper to bluetooth_data_tools #2 closed. The consolidated CCCD debug line now carries char_handle alongside cccd_descriptor.handle, so log-only triage can tie a write back to its characteristic.

• 248 pass, 100% line + branch coverage (828 stmts, 186 branches, 0 missed). ruff clean, black clean, mypy on src reports only the 5 pre-existing errors in cache.py/connect.py — client.py itself is clean. The Quality Report's "Tests: failed" does not reproduce.

• New mutation this round: I moved notify_stop() ahead of the CCCD clear to check whether the "peripheral quiet first" ordering is actually pinned. 7 tests fail. The ordering is behaviourally locked, not just commented.

• The _NotifyCancel TypeAlias under TYPE_CHECKING is safe — client.py has from __future__ import annotations, and build_ext.py's TO_CYTHONIZE list contains only scanner.py, so no Cython annotation evaluation is in play.

• I retract last round's MEDIUM on the swallowed release failure. Reading aioesphomeapi.client: bluetooth_gatt_stop_notify pops _notify_callbacks and calls remove_callback() before send_message. So when it raises, the host-side callback is already gone and only the wire enable=False is lost — and send_message fails only when the connection is not established, at which point the proxy drops the subscription on disconnect anyway. @bluetoothbot's rationale for declining the _release_dirty set holds up; the leak is not durable, and WARNING-and-continue is the right call.

• One doc clause in docs/usage.md:330 states the firmware clears the CCCD without REMOTE_CACHING; the legacy ESP_GATTC_UNREG_FOR_NOTIFY_EVT handler only logs, so the residual leak persists on that path and the table now says otherwise (details in the inline comment).

• The PR description is still stale: it claims 2 files / 123 insertions / 176 tests and a finally block, against an actual 5 files / 581 insertions / 248 tests and a try/except with an explicit trailing release — which is not equivalent to finally, since the release-failure swallow applies only on the error path. Worth refreshing so the human reading it sees the shipped design.

---

✅ Resolved since last review (2)

Previously-flagged issues verified fixed

• src/bleak_esphome/backend/client.py:954 The docstring's retry promise is broader than the code delivers
• src/bleak_esphome/backend/client.py:1037 The consolidated debug log drops the characteristic handle

---

---

Checklist

• CCCD clear is correct and mirrors the start_notify branch
• Dirty-flag lifecycle cannot leak state across connections
• Unretryable failures are not recorded as retryable
• Docstrings match implemented behavior (prior suggestion feat: initial import #1 closed)
• Debug logging is self-sufficient for triage (prior suggestion feat: move mac_to_int helper to bluetooth_data_tools #2 closed)
• Write-before-release ordering is pinned by tests (mutation-verified)
• No errors silently swallowed without a durable leak
• No races on shared bookkeeping (up-front pop, post-write resubscribe guard)
• No unbounded collection growth (_cccd_dirty bounded by characteristics, cleared on disconnect)
• Type annotations safe at runtime (future annotations; client.py not cythonized)
• Documentation accurate against firmware behavior
• Test coverage of new branches (248 pass, 100% line + branch)
• Lint, format, and type checks clean (no new mypy errors in client.py)
• Tests verify behavior, not source text
• Public API change is within bleak's documented backend contract
• No scope creep (client.py hunks confined to the notify path)
• No security-sensitive surface touched

ℹ️ Triage summary

1 pre-existing finding(s) on unchanged code suppressed (freeze).

---

Silent Failure Analysis

🟡 **MEDIUM** — swallowed exception on unrecoverable state
src/bleak_esphome/backend/client.py:988-997

Risk: The handle was already popped from _notify_cancels up front, so a failed release is demoted to a log with no way back: a retried stop_notify sees no entry (and only retries the CCCD via _cccd_dirty), and _async_disconnected_cleanup can no longer invoke the discarded notify_abort, leaving the proxy subscription and its callback live for the rest of the connection while the caller only ever sees the CCCD error.

try:
    await notify_stop()
except Exception:
    _LOGGER.warning(
        "%s: Failed to release the proxy notify subscription for "
        "handle %s; the proxy may keep forwarding notifications", ...)

Fix: Give the release the same retry affordance as the CCCD — re-insert the notify_cancel tuple (or record the handle in a pending-release set) before logging, or attach the release failure to the propagated error via raise ... from / an ExceptionGroup so the caller can act on it.

---

Automated review by Kōan (Claude) HEAD=af25b5b 7 min 35s

[bluetoothbot]

Rebase with requested adjustments

Branch koan/stop-notify-disables-cccd was rebased onto main and review feedback was applied.

Changes applied

• 249 pass, 100% line + branch on client.py, mypy clean.
• Extracted _async_release_notify(handle, notify_cancel) per the DRY audit; both await notify_stop() call sites in stop_notify collapse to one call each (src/bleak_esphome/backend/client.py:1003).
• Fixed the MEDIUM silent-failure finding: a failing release now puts the (stop, abort) pair back under its handle via setdefault, so _async_disconnected_cleanup can still abort it and a later stop_notify retries it. Guarded on _is_connected, mirroring _cccd_dirty, so a restore cannot survive into the next connection.
• Refreshed the stop_notify docstring: retry now covers both the CCCD write and the proxy-side release.
• Updated three tests to the restore contract, added test_stop_notify_release_failure_after_disconnect_is_not_restored for the disconnected branch.

Not changed (and why)

• bdraco's "messy / extract a function / DRY audit" on the CCCD write path — already done in earlier passes (_get_cccd, _async_write_cccd, _async_clear_cccd); only the release half remained, applied above.
• Review's alternative fixes (raise ... from, ExceptionGroup) — restore keeps the CCCD error as the sole actionable root cause while making the release retryable; chaining would surface the secondary failure without recovering the lost abort callable.
• PR-body drift ("the release sits in a finally") — body text, not code; shipped design is an explicit trailing release with a swallow only on the error path. B

Stats

5 files changed, 632 insertions(+), 59 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=main's stop_notify still only pops the handle and awaits notify_stop() with no CCCD write, and n)
• Rebased koan/stop-notify-disables-cccd onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed koan/stop-notify-disables-cccd to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan