Repository navigation
fix(app): keep a live native BLE link from showing as Disconnected - #21430
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aef2c67600
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // DeviceService holds the link but the provider never heard. Republish | ||
| // synchronously with the checks above: nothing can intervene, and no | ||
| // transport event will publish it. | ||
| _provider.onDeviceConnectionStateChanged(deviceId, DeviceConnectionState.connected); |
There was a problem hiding this comment.
When resume reconciliation finds an already-connected service link, this call is not a synchronous publication: DeviceProvider.onDeviceConnectionStateChanged waits 100 ms in _connectDebouncer, and the eventual handler never rechecks the saved device ID. If another flow changes the pairing in that window—for example, rayban_meta_input_picker_sheet.dart::_connectWithDeviceService stores the selected input before awaiting its connection—the queued old-device admission can call setConnectedDevice, write the old device back to preferences, and start its connection side effects. Revalidate deviceId at admission (or make this queued reconciliation cancellable) so the pre-probe pairing check still holds when adoption executes.
Useful? React with 👍 / 👎.
aef2c67 to
74eb20a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74eb20a977
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| // Otherwise native replays device-ready into a fresh transport, whose own | ||
| // connected event publishes through the same path as any native connect. | ||
| await deviceService.ensureConnection(deviceId, force: true); |
There was a problem hiding this comment.
When a resume reconcile reaches this no-existing-connection branch while the user changes pairing during ensureConnection, the eventual old-device connected event enters _handleDeviceConnected with requirePersistedPair: false. The held-service branch is now guarded, but the Ray-Ban picker persists the newly selected device before awaiting its connection (app/lib/widgets/rayban_meta_input_picker_sheet.dart:101-103), so this branch can still publish the old device, write it back to preferences, and start its capture side effects. Carry the probed pairing constraint through the fresh connection event or cancel that adoption when the persisted pairing changes.
Useful? React with 👍 / 👎.
| _services = []; | ||
|
|
||
| try { | ||
| _nativeReleased = true; |
There was a problem hiding this comment.
When unmanageDevice is pending and ultimately throws, an already-in-flight onDeviceReady can be delivered during this await. Setting _nativeReleased first makes _handleDeviceReady discard it; clearing the flag in the catch cannot replay that event, and a DeviceService.disconnectDevice caller then disposes the transport, leaving Dart disconnected while native still owns the live link until another resume. Buffer/replay a pending ready when the release fails rather than dropping it permanently.
Useful? React with 👍 / 👎.
74eb20a to
d895f6f
Compare
A launch connect that times out before the pendant arrives used to dispose its transport, which unregistered the peripheral from BleBridge while native kept connecting. The late device-ready then reached nobody and the app showed Disconnected over a live link until it was killed. BasedHardware#21302 keeps the transport on timeout; this closes the footguns that made the path possible and still leave siblings open: - BleBridge.registerPeripheral returns an owner-scoped disposer, so a retired transport cannot unregister its replacement. - NativeBleTransport.disconnect releases native based on its own intent, not the mirrored state, and ignores a ready in flight after release. - DeviceService.disconnectDevice disposes the transport it drops. - NativeLinkReconciler re-adopts a link native already holds when the app resumes; native replays device-ready without touching the radio. - Transports ignore state updates after dispose. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reconciler's republish of a DeviceService-held link went through onDeviceConnectionStateChanged, so admission queued behind the 100 ms connect debouncer. The debounced handler never rechecked the saved device: a pairing persisted in that window (an input picker seeding another microphone) was overwritten when the stale admission called setConnectedDevice, writing the old device back to preferences and starting its connect side effects. Route the reconciler's republish through a dedicated readoptServiceLink admission that requires the persisted pairing to still match when the handler runs. The shared connected-event path is unchanged: onboarding legitimately connects before persisting a first pairing. Addresses chatgpt-codex-connector review thread PRRT_kwDOLkKqys6sA9ST.
… failed release Two follow-ups from review: - The reconciler's fresh-connection branch still published through the unguarded generic connected path. A pairing persisted while the link formed (an input picker seeding another microphone) was overwritten when that admission ran. The reconciler now queues its guarded readoptServiceLink admission in place of the pending generic one: the connect debouncer is single-slot and the reconcile continuation runs as the next microtask inside the 100 ms window, so the guarded admission always replaces the unguarded one. - A ready delivered while unmanageDevice was pending was dropped by the _nativeReleased guard even when the release then failed, stranding a live native link until the next resume. The transport now buffers the pending ready and replays it after a failed release, exactly like a ready arriving after the failure. Addresses chatgpt-codex-connector threads PRRT_kwDOLkKqys6sEx56 and PRRT_kwDOLkKqys6sEx59.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d895f6fd90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // single-slot, and this runs as the next microtask after the transport's | ||
| // event, well inside its 100 ms window — so a pairing that moved on while | ||
| // the link was forming is never published. | ||
| final connection = await deviceService.ensureConnection(deviceId, force: true); |
There was a problem hiding this comment.
When fresh adoption takes more than 100 ms, the ordinary connected debounce has already fired and its unguarded _handleDeviceConnected is running (usually waiting on DeviceService's mutex), so this later readoptServiceLink cannot replace it. Fresh evidence in the current tree is that DeviceConnection.connect emits connected before it awaits ping() and device.getDeviceInfo(this) (device_connection.dart:239-249,274-283), with Omi/Limitless initialization adding still more awaited work, which invalidates the “next microtask” assumption here. If the Ray-Ban picker persists another device during that work, the generic callback can publish the old device and rewrite the pairing before the guarded callback arrives; carry the reconciliation constraint into the connected event itself rather than relying on the debounce window.
Useful? React with 👍 / 👎.
| if (requirePersistedPair && SharedPreferencesUtil().btDevice.id != deviceId) { | ||
| Logger.debug('Discarding readoption of $deviceId: pairing moved on before admission'); | ||
| return; |
There was a problem hiding this comment.
When the user changes pairing after this check but while _onDeviceConnected is awaiting device setup, the stale readoption is still allowed to finish: setConnectedDevice can later assign SharedPreferencesUtil().btDevice in getDeviceInfo() (device_provider.dart:389-401) and the continuation starts battery, capture, and WAL side effects for the old link. Fresh evidence in the current tree is that the newly guarded handler invokes _onDeviceConnected without carrying any expected-pair token through its async continuation. Preserve or revalidate that token through publication so a picker selection made during those BLE awaits cannot be overwritten.
Useful? React with 👍 / 👎.
Threads 4/5 on the late-ready review: a resume reconcile authorizes the persisted pairing at probe time, but the fresh path's generic connected admission can start running while the link is still forming (DeviceConnection.connect emits connected before awaiting ping and getDeviceInfo), and device setup publication awaits link checks after admission. Arm the constraint on the provider before the link forms, gate admission, both btDevice persists, and the setup continuation (before battery/capture/WAL side effects and before the final publication), and clear it on session reset. A pairing persisted in any of those windows (the Ray-Ban input picker seeding another microphone before awaiting its connection) is no longer published over; verified by two provider-level regressions that fail on the previous head.
d895f6f to
cf6a8aa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf6a8aa287
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| final link = (connection, connection.connectionEpoch); | ||
| if (_admittedLink == link) return; | ||
| _admittedLink = link; |
There was a problem hiding this comment.
When resume admission for A reaches this assignment and the Ray-Ban picker then seeds B while setConnectedDevice(A) is awaiting, the new pairing guard abandons A but leaves (connection, epoch) latched. If B fails, rayban_meta_input_picker_sheet.dart:101-112 restores A; subsequent resume readoptions see the same live A/epoch and are dropped by the equality check immediately above, recreating the stuck Disconnected state until a manual or physical reconnect. Fresh evidence after the prior pairing comments is that none of the newly added guard returns clears _admittedLink; only commit the latch after setup succeeds or clear it on every abandoned setup.
Useful? React with 👍 / 👎.
1 participant
Summary
Closes the ways the app can show a pendant as Disconnected while native BLE holds a live, ready link. In that state, a pendant that is visibly connected in iOS Settings (blue light) never streams audio until the app is killed.
Root cause (observed on a real iPhone, prod 1.0.563+1377). The app opened at 22:42:30 and sent one
connectrequest at 22:42:34. Dart's launch connect gave up at its 60s deadline. The pendant arrived at 22:43:54, 80s after the request, and native finished service discovery at 22:43:56. After that the app process sent zero GATT reads, writes, or notify subscriptions until it was restarted. The only traffic was the native 10s RSSI poll, which runs only whileperipheral.state == .connected. This is the pre-#21302 path: the timeout tore the connection down and disposed its transport. That dispose unregistered the peripheral fromBleBridge, butNativeBleTransport.disconnect()returned early because Dart's mirror already said disconnected, so it never sentunmanageDevice. Native kept connecting and delivered device-ready to nobody. #21302 (merged 2026-10-13 14:45 UTC) keeps the transport on timeout. A new provider-level test reproduces the report: it fails once3826d2fd^and passes on main.This PR removes the footguns that made that path possible and still leave sibling paths open on main:
BleBridge.registerPeripheralnow returns a disposer that unregisters only while the caller still owns the peripheral. Before, unregistration was by id alone, so a retired transport disposed after its replacement registered would silence the replacement's device-ready.NativeBleTransport.disconnect()now callsunmanageDeviceunless this transport already released native. Before, it skipped the call whenever Dart's mirrored state said disconnected, even though native keeps connecting after a Dart-side timeout. A device-ready already in flight after release is ignored, so it cannot re-adopt the link behind an explicit disconnect.DeviceService.disconnectDevicedisposes the transport it drops, matching_teardownConnection. Before, the dropped transport stayed registered and could swallow the next device-ready: tap Disconnect while native is reconnecting, native reconnects, and the ready lands on an orphan whose connection no longer listens.NativeLinkReconciler, bound inmain.dart, runs on app resume. If the provider shows the paired device disconnected but native reports its peripheral connected, it re-checks that the same id is still paired and still disconnected after the async probe. It then callsDeviceService.ensureConnection(id, force: true)for exactly that id and publishes the result throughDeviceProvider.onDeviceConnectionStateChanged, the entry point native events use. That covers both a missing transport and aDeviceServiceconnection the provider never heard about. Native answersmanageDevicefor a connected peripheral by replaying device-ready (iOSreplayReady/hydrateReady, AndroidadoptConnectedDevice), so this does not touch the radio and never starts a physical connect. It covers any future lost-ready path, not one cause.Also:
unmanageDevicefails, the transport stays unreleased, so the release can be retried and native readies are still adopted._updateStateignores events afterdispose()instead of throwing on a closed controller.DeviceProvider._handleDeviceConnectedadmits each physical connect once, keyed on(connection, connectionEpoch)and cleared whenever the mirror disconnects. Overlapping connected events (a native connect still initializing plus a resume reconcile) no longer run capture start, battery listeners or WAL attach twice.Line-Count-Exception: app/lib/providers/device_provider.dart | 1488 -> 1576 | the per-connect admission guard and the readoption pairing constraint (armed admission, gated persists and setup continuation) live in the provider's own connect entry point; the reconcile logic itself is split into native_link_reconciler.dart
UI-Evidence: none -- no visible UI change; main.dart only binds the device provider app-lifecycle listener
Verification
flutter test test/providers/device_provider_late_ready_test.dart: 14 tests. Each invariant was mutation-checked: reverting any one of the ten fixes (owner-scoped bridge release, intent-based disconnect, the ready guard after release, dispose indisconnectDevice, the reconcile native probe, the held-link republish, the reconcile id re-check, retryable failed release, the per-connect admission guard, and the admission reset on any mirror disconnect) fails exactly its test. The late-ready provider test also fails once3826d2fd^.gpt-6.1-solvia agentctl (read-only brief). Five rounds viaagentctl continueon one session, each re-reviewing the delta. Rounds 1–4 found eight P2 defects in earlier revisions, all fixed and pinned by tests above: the reconcile didn't republish a linkDeviceServicealready held; the probed id could differ from the connected id; a failed release was recorded as released; synthetic publishes across awaits (double admission, overriding a real disconnect, publishing a re-paired device); overlapping admissions for one physical connect; and an admission latch surviving provider-only disconnects. Round 5: "Nothing blocks merge in this delta."bash app/test.sh(full hermetic suite + spine) with Flutter 3.44.5 on macOS: spine 138/138; suite 5,500 passed, 21 skipped, 4 failed. The four failures (apps_category_section_large_text_test×2,ai_app_generator_large_text_test,settings_search_header_stability_test) fail identically on unmodifiedorigin/mainand are fixed by the open fix(app): restore green Flutter tests on main (#21302 #21340 #21341 #21343 follow-ups) #21425. That PR also fixesdevice_service_connection_failure_test, which this PR leaves alone to avoid a conflict.flutter analyze: no new issues (two pre-existing infos).Not verified on device: the resume reconcile has not been exercised on a physical phone. The incident evidence (bluetoothd/runningboard logs) comes from the reporting iPhone.
Product invariants affected
main.dartchanges.Failure class
Failure-Class: FC-superseded-connection-mutates-live-session
🤖 Generated with Claude Code