fix: prevent same-name BLE discovery starvation - #161
Conversation
|
Independent latest-head review at effcf6c found no actionable issues. Reviewer ran all 69 transport tests and checked desktop-ID/authentication preservation, four-probe bound, bounded completed-key retention, cancellation and rotating-address tradeoffs. Full local validation passed (583 tests, Expo Doctor 21/21). Native CI is still running; physical Linux reconnect remains unverified. |
Greptile SummaryThis change improves BLE discovery by limiting concurrent status probes and queueing excess advertisements. However, saved-PC resolution can still discard an advertised target when the waiting queue is full, causing an available PC to time out as undiscoverable. Confidence Score: 4/5Not safe to merge until saved-PC discovery retains or otherwise considers target advertisements received while the waiting queue is full. A focused reproduction confirmed that the resolution path drops a one-time target advertisement after four active probes and 32 queued devices, then times out without probing that target. Files Needing Attention: src/transport/ReactNativeBleTransport.ts
What T-Rex did
|
|
Addressed the automated review findings: queue up to 32 waiting advertisements while four probes are active, drain as slots free, and clear pending work on stop/success/cancel. Evict the oldest completed Windows address when retaining a new address at capacity, without evicting active probes. Added Android/iOS queued-target tests and a 257-address cache regression. Full validation passed again: 586 tests and Expo Doctor 21/21. Latest-head independent review and CI will rerun. |
|
Latest fix also closes the independent-review handoff race: target claim clears the pending queue and blocks further advertisement admission/drain during MTU/service setup. Added held-setup regression proving queued peers do not start. Full validation now passes 587 tests and Expo Doctor 21/21. Re-review and native CI running on 9c67dcf. |
| if (waiting.size < 32) waiting.set(device.id, device); | ||
| return; |
There was a problem hiding this comment.
When four status probes are active and all 32 waiting slots are occupied, this branch discards the next advertisement instead of retaining it. A saved PC advertised only once in that state is never status-probed, so resolveAndConnect times out even though it received the PC's advertisement.
Knowledge Base Used: BLE transport adapter
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/transport/ReactNativeBleTransport.ts
Line: 157-158
Comment:
**Full Queue Drops Targets**
When four status probes are active and all 32 waiting slots are occupied, this branch discards the next advertisement instead of retaining it. A saved PC advertised only once in that state is never status-probed, so `resolveAndConnect` times out even though it received the PC's advertisement.
**Knowledge Base Used:** [BLE transport adapter](https://app.greptile.com/owen-mcgirr/-/custom-context/knowledge-base/switchifyapp/switchify-remote/-/docs/ble-transport.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
Review disposition for Full Queue Drops Targets: acknowledged as an intentional bounded-admission limit, not an actionable P1 for this focused fix. Capacity is four active plus 32 waiting peripheral candidates. Desktop identity is learned only after a status read, so replacing a queued unknown candidate merely changes which potential target is dropped; unbounded retention violates repository resource bounds. Independent re-review concurs: no further blocker. This fixes name-based exclusion within capacity and does not guarantee exhaustive discovery above that limit. Dense-environment rescan scheduling can be separate work. Latest head 9c67dcf passed independent review and 73 transport tests; full validation passed 587 tests, Expo Doctor 21/21. |
|
All checks passed at reviewed head 9c67dcf: Android, iOS, validation and automated review. Independent latest-head review has no actionable findings; bounded-capacity disposition is documented above. User explicitly approved merge and next-beta release. Proceeding with merge; physical phone confirmation remains outstanding. |
Closes #160
Summary
Validation