Skip to content

fix: prevent same-name BLE discovery starvation - #161

Merged
enaboapps merged 3 commits into
mainfrom
codex/same-name-discovery-160
Sep 12, 2026
Merged

enaboapps merged 3 commits into
mainfrom
codex/same-name-discovery-160

Conversation

@enaboapps

Copy link
Copy Markdown
Contributor

Closes #160

Summary

  • Key probe deduplication by peripheral ID, never Bluetooth name, in discovery and selected-PC resolution.
  • Preserve four-probe concurrency, duplicate-address suppression and cancellation; bound completed Windows-address retention.
  • A new/rotated address must be probed to establish identity, even if its name matches a known PC. Existing rotation tests now assert that contract.
  • Add overlapping same-name discovery/reconnect regressions on Android and iOS, plus probe-limit/cleanup coverage.

Validation

  • npm run validate passed: 66 suites / 583 tests, lint, TypeScript, version checks and Expo Doctor 21/21.
  • Before fix: fake overlapping same-name advertisements yielded 15 nonmatching reads, zero target reads and timeout; distinct names connected.
  • No real input or Bluetooth hardware used in automated tests. Physical Linux symptom correlation and installed-phone retest remain outstanding.
  • Independent latest-head review and CI pending. No version bump, merge or release included.

@enaboapps

Copy link
Copy Markdown
Contributor Author

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.

@enaboapps
enaboapps marked this pull request as ready for review September 12, 2026 08:53
@greptile-apps

greptile-apps Bot commented Sep 12, 2026

Copy link
Copy Markdown

Greptile Summary

This 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/5

Not 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

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced proof for a posted P1 finding and referenced the associated review comment for details.
  • T-Rex produced proof for a second posted P1 finding and referenced its review comment for details.
  • T-Rex ran the requested verification, but its local artifact references were not uploaded.

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Saved-PC target advertisement is discarded when the resolution waiting queue is full

    • Bug
      • With four active status probes and 32 queued devices, a newly advertised saved-PC target is not added to waiting. If it is not advertised again, no probe is started for it and resolveAndConnect times out.
    • Cause
      • In the resolution scan callback, capacity handling only calls waiting.set(device.id, device) when waiting.size < 32; otherwise it returns without retaining or probing the advertised device.
    • Fix
      • Change the bounded waiting-queue policy so an incoming target candidate is retained or an existing queued candidate is evicted when capacity is reached, then add this focused regression test to the normal transport test suite.

    T-Rex Ran code and verified through T-Rex

Prompt To Fix All With AI
### Issue 1
src/transport/ReactNativeBleTransport.ts:157-158
**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.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (2): Last reviewed commit: "fix: stop waiting discovery once the tar..." | Re-trigger Greptile

@enaboapps

Copy link
Copy Markdown
Contributor Author

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.

@enaboapps

Copy link
Copy Markdown
Contributor Author

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.

Comment on lines +157 to +158
if (waiting.size < 32) waiting.set(device.id, device);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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

T-Rex 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.

@enaboapps

Copy link
Copy Markdown
Contributor Author

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.

@enaboapps

Copy link
Copy Markdown
Contributor Author

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.

@enaboapps
enaboapps merged commit 88df628 into main Sep 12, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix same-name BLE devices starving discovery and reconnect

1 participant