Skip to content

Two-phase pairing identity: hydration contract, non-destructive back-outs, guarded connect paths - #3346

Merged
PhilippeFerreiraDeSousa merged 20 commits into
integration/toolkit-boundaryfrom
pairing-two-phase-identity
Jul 6, 2026
Merged

Two-phase pairing identity: hydration contract, non-destructive back-outs, guarded connect paths#3346
PhilippeFerreiraDeSousa merged 20 commits into
integration/toolkit-boundaryfrom
pairing-two-phase-identity

Conversation

@PhilippeFerreiraDeSousa

@PhilippeFerreiraDeSousa PhilippeFerreiraDeSousa commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Stacked on #3331 (integration/toolkit-boundary); retargets to dev when #3331 lands. The revert this builds on (73246b2) and all touched code live on that branch.

Design-level fix for the pairing-identity desync behind "Set a default glasses device before calling connectDefault" (also reproduced on dev).

The bug

default_wearable (a model string in the phone's persisted settings) could exist without a native default device. The card then renders a paired-looking state whose Connect button calls connectDefault() — which throws, because the SDK's currentDefaultDevice() needs default_wearable and device_name in the native DeviceStore.

How the half-state formed (all three sources confirmed in code):

  1. Select-then-abandon. Tapping a device in the scan eagerly wrote the default: pair() connected with saveAsDefault: true, whose setDefaultDevice() echoes only default_wearable back to the phone's persisted settings (the device_name/device_address echoes happen in handleDeviceReady, i.e. only on success). Abandon the attempt → the phone keeps a model with no device behind it, forever.
  2. Legacy server-sync resurrection. Identity synced to the cloud before Stop syncing per-phone pairing identity to the server #3149; stranded local values persist. Worse: the island copy of the settings store on this branch regressed Stop syncing per-phone pairing identity to the server #3149's saveOnServer: false back to true — identity was being uploaded again. Restored here for the whole identity group.
  3. Asymmetric forget flows. scan.tsx's back handler ran disconnect()+forget() unconditionally, and select-glasses-model forgot any paired glasses on focus — both wiped real pairings when entered (or backed into) while paired.

Why the first attempt failed (the revert this replaces)

40845e5/8c35d9172/ee54f78c5 guarded the connect paths with await BluetoothSdk.getDefaultDevice(). On-device testing showed the native DeviceStore hydrates asynchronously at cold start — it is in-memory on both platforms and only gets the identity keys when JS pushes the persisted settings, and nothing awaited that seed. The first push happened inside the very reconnect call the guard ran ahead of, so genuinely-paired users read a false "absent": launch reconnect silently skipped, Connect misrouted into /pairing/scan, and scan's destructive back handler wiped the real pairing. Reverted in 73246b2.

Per design review: no tri-state/unknown-exporting API. The fix makes the two-state read trustworthy instead.

The fix

1. Hydration contract (4d1fac61b). DeviceStoreHydration.hydrateDeviceStore() = settings load + full native updateBluetoothSettings push, single-flight, timing-logged. toolkit.start() awaits it before resolving, so every post-start getDefaultDevice() read is a trustworthy two-state answer on both platforms (the awaited push is ordered: iOS AsyncFunction(\"update\") resolves after the MainActor applies; Android update is synchronous). Belt-and-suspenders: toolkit.glasses.hasDefaultDevice() awaits hydration internally. Failure surfaces as a rejection — never a false absent — and callers fail open (the ee54f78 pattern); the memo clears so a later read retries. The old "hydration barrier" was a 1s setTimeout in MantleManager.

With the read trustworthy, a boot repair demotes the orphaned population: default_wearable set + native default absent + link idle → demote to pending_wearable (+ clear device_name/device_address, re-push). Runs post-hydration on every start; converges by construction.

2. Non-destructive scan back-out (4b2ef78c1). scan.tsx captures hasDefaultDevice() at entry (defaulting to preserve until the hydrated read lands). Back-out of a fresh pairing keeps today's disconnect+forget; back-out of a re-pair preserves the existing pairing. Cleanup is centralized in toolkit.pairing.abandonAttempt(), which also covers the pairing-failure and unpair-even retries, and handles two subtleties: (a) old glasses still connected → only stop the scan; (b) attempt in flight → cancel it and re-seed the native identity from persisted settings, because the attempt's connect-by-name overwrote the native device_name that connectDefault() targets. The select-glasses-model focus-forget is removed outright.

3. Two-phase identity (4b2ef78c1). Selection-time writes go to pending_wearable: the scan screen marks the chosen model pending on entry, pair() connects with saveAsDefault: false, and the Mentra Live BT-Classic path threads the picked Device through the route instead of pre-writing it as default. Promotion to default_wearable stays where the native layer already does it — handleDeviceReady (pairing success) — which echoes the full identity to the phone; the echo retires the pending marker (DeviceEventRouter rule). The home card and connect button render pending state with Finish pairing (resume the scan for that model) and Pair different glasses affordances. pending_wearable becomes persisted (still never server-synced) so an abandoned selection survives restarts. Zero native code changes — the SDK's success-promotion path was already correct; we stop bypassing it.

Then the guards return (18bd89f5c), unchanged in spirit from the reverted commits, now safe under 1+2: Connect with a chosen model but no native device routes into /pairing/scan for that model; decideReconnect skips quietly (restored unit test); the foreground reconnect passes a fail-open read.

Behavior notes

  • Simulated-glasses users now get their SGC initialized deterministically at toolkit.start() (the seed push applies default_wearable=SimulatedinitSGC), instead of whenever the first incidental push landed.
  • isFirstPairing() is now no default AND no pending — a pending selection is not "first"; OEM hosts should offer finish-pairing.
  • After a failed/abandoned re-pair, the old pairing remains the default until a new pairing succeeds (promotion overwrites it) — previously the old default was clobbered at tap time.

Test evidence

  • cd mobile && bun run compile
  • cd mobile && bun run test — 52 suites, 408 tests ✓ (scan back-out + pending-marker + abandon-on-failure cases added; coordinator skip test restored)
  • cd mobile/modules/island && bun run test — 201 tests ✓ (new DeviceStoreHydration suite: single-flight, rejection + retry-heal, two-state read, demotion conditions)
  • ./scripts/check-mobile-runtime-boundary.sh ✓ (no new host imports)
  • Each commit compiles and tests green independently (bisect-clean).

REQUIRED before merge: on-device cold-start verification (iOS at minimum)

The hydration log line DeviceStoreHydration: native seed complete in <N>ms (default_wearable=…, device_name present=…) must appear before any RECONNECT/guard log.

  1. Paired + kill + relaunch → auto-reconnect works; hydration log precedes the reconnect decision; no /pairing/scan misroute.
  2. Fresh select-then-abandon (pick model → scan → back out; also kill mid-loading) → home shows the finish-pairing card, no connect error, and the card survives another kill+relaunch.
  3. Back-out of a re-pair scan (paired glasses → pair-different flow → back out at scan) → original pairing preserved and still reconnects (checks the native identity re-seed after an in-flight attempt).
  4. Full fresh Mentra Live pairing on iOS, including the Pair Audio (Bluetooth Classic) step — verifies the native Classic watcher priming (setBluetoothClassicTarget) that replaced the selection-time identity writes, and that the loading screen advances to success.
  5. Same on Android when hardware is available.

Note

High Risk
Touches core Bluetooth pairing, identity persistence, cold-start reconnect, and connect routing; incorrect hydration or abandon logic could skip reconnects or wipe pairings despite fail-open guards.

Overview
Fixes "Set a default glasses device before calling connectDefault" when default_wearable existed without a real native default device.

Cold-start hydrationtoolkit.start() now awaits seeding the in-memory native DeviceStore from persisted settings before sync/reconnect run. toolkit.glasses.hasDefaultDevice() is hydration-aware; boot demotes orphaned default_wearable (no native device, link idle) to persisted pending_wearable.

Two-phase identity — Selection writes pending_wearable only; pair() uses saveAsDefault: false so native promotes on success. New toolkit.pairing.identity() / onIdentity() read-model (none | pending | paired). Identity keys are native-authoritative (no server sync, excluded from settings change-push and on-connect replay to stop echo loops).

Safer pairing UXabandonAttempt() replaces unconditional forget() on scan back-out, failures, and retries (preserves re-pairs and in-flight promotions). iOS Mentra Live threads the scan device through routes and primes Classic via setBluetoothClassicTarget. Home/connect UI uses identity for Finish pairing vs connect, and routes to scan when paired in settings but no native device.

Reviewed by Cursor Bugbot for commit a90d918. Bugbot is set up for automated code reviews on this repo. Configure here.

PhilippeFerreiraDeSousa added 3 commits July 3, 2026 21:29
…dentity

The Bluetooth SDK's DeviceStore is in-memory on both platforms: the pairing
identity keys (default_wearable / device_name / device_address) initialize
empty at process start and only become real once JS pushes the persisted
settings over updateBluetoothSettings. Nothing awaited that seed at boot —
the first push happened as a side effect of the first connect call — so
getDefaultDevice() returned null for genuinely-paired users at cold start.
That false "absent" is what made the first hasDefaultDevice guards misfire
and had to be reverted (73246b2).

Make the seed an explicit initialization step with a contract:

- DeviceStoreHydration.hydrateDeviceStore(): settings load + full native
  push, single-flight, logged with cold-start timing. Failure surfaces as a
  rejection (never a false absent); the memo clears so a later read retries.
- toolkit.start() awaits it before resolving, so every post-start
  getDefaultDevice() read is a trustworthy two-state answer. A failed
  hydration logs and continues — guards fail open, the runtime still boots.
- toolkit.glasses.hasDefaultDevice() awaits hydration internally
  (belt-and-suspenders for reads racing start()).
- loadAllSettings() becomes single-flight so the host's module-load call and
  the hydration await share one disk load.

With the read now trustworthy, repair the existing orphan population at
boot: a persisted default_wearable with no native default device (selection
abandoned before pairing succeeded, or identity resurrected from the server
before it stopped syncing) demotes to pending_wearable, which hosts render
as a finish-pairing card instead of a Connect button that would throw "Set
a default glasses device before calling connectDefault".

Also restore saveOnServer:false across the pairing-identity settings group.
PR #3149 flipped these on dev (identity is per-phone, not per-account), but
the island copy of the settings store on the integration branch regressed
them to true, so identity was being uploaded to the server again — the
exact resurrection channel that stranded orphaned identities in the first
place. pending_wearable also becomes persisted (still never synced) so an
abandoned selection still shows its finish-pairing card after a restart.
…tructive

Two-phase pairing identity. The native layer already promotes on success —
handleDeviceReady sets defaultWearable from the connected SGC and echoes the
full identity (default_wearable + device_name + device_address) to the
phone's persisted settings, on both platforms. What poisoned the state was
the EAGER write at attempt time: pair() connected with saveAsDefault:true,
whose setDefaultDevice() echoed default_wearable (and only it) to JS the
moment a device was tapped. Abandon the attempt and the phone keeps a
default_wearable with no device behind it — the "Set a default glasses
device before calling connectDefault" population.

Selection now only marks intent:

- The scan screen writes pending_wearable on entry; the model the user last
  started pairing. toolkit.pairing.pair() connects with saveAsDefault:false,
  so no identity is written until the native promotion at device-ready. The
  promotion echo retires the pending marker (DeviceEventRouter).
- The Mentra Live BT-Classic path threads the picked Device through the
  btclassic route instead of pre-writing it as the default; the post-Classic
  connect uses the threaded device and promotes on success like every other
  path.

Back-outs stop destroying real pairings. scan.tsx's back handler ran
disconnect()+forget() unconditionally — combined with a guard misroute that
is exactly how genuinely-paired users got wiped (73246b2). Cleanup now
goes through toolkit.pairing.abandonAttempt():

- Re-pair with the old glasses still connected (nothing tapped): stop the
  scan, touch nothing else.
- Re-pair with an attempt in flight: cancel it and re-seed the native
  identity from persisted settings — the attempt's connect-by-name
  overwrote the native device_name, so connectDefault() would otherwise
  target the abandoned device.
- Fresh pairing: disconnect + forget as before. The pending marker survives
  so the home card offers to finish pairing.

The scan screen decides fresh-vs-re-pair from a hydrated hasDefaultDevice()
read captured at ENTRY (defaulting to preserve — a race must never wipe a
pairing); the pairing-failure and unpair-even retries use a live read. The
select-glasses-model focus forget is gone: it wiped real pairings whenever
the screen was entered — or backed into — while glasses were paired, and
with no eager writes there is nothing to clean before a fresh flow.
…ing selections

Brings back the guards from the reverted 40845e5/8c35d9172/ee54f78c5, now
safe under the hydration contract: hasDefaultDevice() awaits the device-store
seed, so a cold start can no longer read a false "absent" for paired users,
and the scan screen's back-out no longer wipes a real pairing if a user does
land there.

- Home card + ConnectDeviceButton: Connect on a chosen model with no native
  default device routes into /pairing/scan for that model instead of
  surfacing the "Set a default glasses device" alert. Reads fail open to
  connectDefault(), whose catch is the pre-guard behavior.
- decideReconnect: skip quietly when no default device exists (kills the
  boot-time RECONNECT warns for the orphaned population); the foreground
  reconnect passes a fail-open read so a transient bridge error cannot
  throw out of the app-state handler.

New pending-selection rendering: with two-phase identity a chosen-but-never-
paired model no longer masquerades as a paired default, so the home card and
connect button gain an explicit finish-pairing state (resume the scan for the
pending model, or start over with different glasses) instead of the first-run
pair card losing the user's selection.
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

📋 PR Review Helper

📱 Mobile App Build

Ready to test! (commit a90d918)

📥 Download APK

🕶️ ASG Client Build

Waiting for build...


🔀 Test Locally

gh pr checkout 3346

Comment thread mobile/modules/island/src/services/DeviceStoreHydration.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18bd89f5c9

ℹ️ 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".

Comment thread mobile/src/app/pairing/btclassic.tsx Outdated
Comment on lines 69 to 72
useEffect(() => {
console.log("BTCLASSIC: check deviceName", deviceName)
if (deviceName == "" || deviceName == null) {
console.log("BTCLASSIC: deviceName is empty, cannot continue")
if (!device) {
console.log("BTCLASSIC: no device threaded from the scan screen, cannot continue")
handleBack()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow BT Classic recovery without scan params

When this screen is opened outside the scan flow, no device route param is supplied: PairingSuccessScreen still queues push("/pairing/btclassic") when Classic is missing after pairing, and BtClassicPairing does the same from the home-screen “BT Classic disconnected” alert. In those paired/recovery contexts this new guard immediately navigates back, and handleSuccess also cannot reconnect because it only knows about the route device, so iOS Mentra Live users can no longer complete or recover Bluetooth Classic pairing unless they came directly from scan. Please keep a fallback to the saved default device/name for those existing routes or pass the device there too.

Useful? React with 👍 / 👎.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 issues found across 24 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread mobile/modules/island/src/services/DeviceStoreHydration.ts
Comment thread mobile/modules/island/src/services/DeviceStoreHydration.ts Outdated
Comment thread mobile/src/app/pairing/btclassic.tsx Outdated
toolkit.glasses.connectDefault().catch((error) => {
console.error("Failed to connect default glasses after Bluetooth Classic pairing:", error)
if (!device) return
toolkit.glasses.connect(device, {saveAsDefault: false}).catch((error) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The handleSuccess in btclassic.tsx calls toolkit.glasses.connect(device) without awaiting it before navigating away via pushPrevious(). If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level pair_failure event, the loading screen's onPairFailure listener won't catch it, and the user will wait the full 35-second waitForReady timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/src/app/pairing/btclassic.tsx, line 45:

<comment>The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</comment>

<file context>
@@ -15,20 +16,34 @@ import CrustModule from "@mentra/crust"
-    toolkit.glasses.connectDefault().catch((error) => {
-      console.error("Failed to connect default glasses after Bluetooth Classic pairing:", error)
+    if (!device) return
+    toolkit.glasses.connect(device, {saveAsDefault: false}).catch((error) => {
+      console.error("Failed to connect glasses after Bluetooth Classic pairing:", error)
     })
</file context>

…+ pending precedence

- btclassic.tsx required the scan-threaded device param, but the screen is
  also entered WITHOUT one from paired contexts — the pairing-success step
  stack and the home-screen "Bluetooth Classic disconnected" recovery alert —
  which the new guard immediately bounced back. Fall back to the saved
  default identity there: guide copy uses the persisted device_name and the
  post-Classic connect uses connectDefault(), restoring the pre-change
  behavior for those routes while the scan flow stays two-phase.

- demoteOrphanedDefaultWearable() now awaits the (memoized) hydration itself
  instead of relying on the caller's ordering: judging "native default
  absent" against an unseeded store would demote a valid pairing. A failed
  hydration rejects rather than demoting blind.

- Demotion only backfills pending_wearable when it is empty: a fresher
  pending selection (a pairing started after the orphan formed, persisted
  across the restart) must not be replaced by the stale orphaned model.

- Tests: reset the hydration memo per test (order-independence) and cover
  the new pending-precedence and unhydrated-refusal behaviors.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 3 files (changes from recent commits).

You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread mobile/src/app/pairing/btclassic.tsx Outdated
PhilippeFerreiraDeSousa added 2 commits July 5, 2026 14:25
… setting

connectDefault()'s precondition is the native default device, so judge the
no-scan-device entry by the hydration-aware hasDefaultDevice() read instead
of the device_name setting: a lingering name without a default no longer
strands the screen on a connect that must throw, and a missing name with a
valid default no longer bounces a recoverable session. Fails open (stays) on
a read error — connectDefault()'s catch remains the last line of defense.
On-device testing surfaced the home glasses card flapping between the
first-run and G2 states, with default_wearable oscillating ""↔"Even
Realities G2" in a tight loop. The settings sync was bidirectional with
loop gain 1 for identity keys: GlassesSettingsSync pushed every JS change
to native, native apply echoed every change back via save_setting, and
DeviceEventRouter relayed every echo into setSetting — which triggered the
next push. The boot demotion writing "" while the native layer promoted the
reconnected glasses put two values in flight, and they chased each other
through push→apply→echo→push forever.

Break the loop with an ownership rule: pairing identity (wearable +
controller keys) is native-authoritative — promoted at handleDeviceReady,
cleared at forget, echoed down to JS — and travels JS→native only in the
explicit full seeds (device-store hydration, pre-connect push,
post-demotion re-push). The change-push subscription now excludes the
identity group (diffBluetoothSettingsForPush), so an echoed identity change
can never bounce back up. Also damp the save_setting relay: native
re-echoes the identity block unconditionally at every device-ready, and a
same-value echo must not become a store write (persistence churn +
change-push noise + the log spam that exposed this).
if (event.key === SETTINGS.default_wearable.key && event.value) {
if (settings.getSetting(SETTINGS.pending_wearable.key)) {
await settings.setSetting(SETTINGS.pending_wearable.key, "")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pending cleared on reconnect echo

High Severity

The save_setting handler clears pending_wearable whenever any truthy default_wearable echo arrives, even when that echo is an unchanged identity block from handleDeviceReady on reconnect. During “pair different glasses”, JS can still hold the old default_wearable while pending_wearable names the new model; a background reconnect then drops the pending marker and the finish-pairing / re-pair intent.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5404c52. Configure here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 4 files (changes from recent commits).

You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/modules/island/src/stores/settings.ts">

<violation number="1" location="mobile/modules/island/src/stores/settings.ts:733">
P3: Pairing identity keys are duplicated in two separate arrays, so later key additions/removals can update one list but not the other and reintroduce inconsistent sync behavior. Consider deriving one list from the other (or composing both from a shared base) so identity routing stays correct by construction.</violation>
</file>

<file name="mobile/modules/island/src/services/DeviceEventRouter.ts">

<violation number="1" location="mobile/modules/island/src/services/DeviceEventRouter.ts:131">
P1: The `pending_wearable` clearing fires on every truthy `default_wearable` echo, even when the damping check above correctly identified the value as unchanged (same-value reconnect echo from `handleDeviceReady`). During a "pair different glasses" flow, the old glasses can reconnect in the background and echo the existing `default_wearable` — this passes the `event.key === SETTINGS.default_wearable.key && event.value` gate and incorrectly clears `pending_wearable`, dropping the user's in-progress pairing intent.

The clearing should be conditioned on the default actually having changed (i.e., a real promotion), not just being truthy:</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread mobile/modules/island/src/services/DeviceEventRouter.ts Outdated
Comment thread mobile/modules/island/src/stores/settings.ts Outdated
On-device testing on iOS Mentra Live: pairing the audio device in Settings
never advanced the Pair Audio screen. Native detects Classic pairing by
matching the connected audio route against its device_name (the
_BLE_/_BT_ id suffix pattern), and with device_name empty it hard-returns
bluetoothClassicConnected=false. On dev that name was primed by the
selection-time setDefault/setDeviceName writes; two-phase identity removed
those, and the identity sync exclusion keeps a JS write from reaching
native anyway — so the watcher had nothing to match.

Give the hint its own semantic home instead of restoring the eager
identity write: toolkit.pairing.setBluetoothClassicTarget(device) pushes
device_name to native only (no native→JS echo exists for it, so the
phone's persisted identity stays promotion-only), and the Pair Audio
screen primes it on mount with the scan-threaded device. Paired-context
entries (success stack, recovery alert) need nothing — their native
device_name comes from the hydration seed. The immediate native check on
apply also advances the screen right away when the audio device was
already paired.
Comment thread mobile/src/app/pairing/scan.tsx
PhilippeFerreiraDeSousa added 3 commits July 6, 2026 14:20
setBluetoothClassicTarget calls updateBluetoothSettings, which exists only
on the internal module — the facade's public-entry import made the call
undefined at runtime (render error on the Pair Audio screen) while the
compiler accepted it. Import @mentra/bluetooth-sdk/internal like the
glasses facade already does.
…snapshot

On-device: entering the scan flow from the finish-pairing card and backing
out after the glasses had actually paired left the home screen on the
first-run card with the pairing forgotten. The back handler captured
hasDefaultDevice at ENTRY (fresh = forget on back-out), but promotion can
happen while the flow is open — the glasses finish pairing and promote the
default even as the user backs out of the UI. Passing the stale "fresh"
snapshot into abandonAttempt bypassed the live check that would have said
"a real pairing exists now, preserve it", and forgot the just-promoted
device (pending had already been retired by the promotion, so nothing was
left).

Drop the snapshot entirely: abandonAttempt always decides from the live
hydrated read, which covers every case — re-pair (default exists
throughout: preserve), genuinely unpaired attempt (forget, as before), and
fresh-but-promoted (preserve; the glasses stay connected and the home card
shows the paired device). The parameter goes away with its last caller.
The two-phase pairing-identity rules were enforced by convention across
the scan screen, the event router, the boot demotion, and a parallel
sync-exclusion list. Express them as structure:

- New PairingIdentity service owns the lifecycle as a sum type
  (none | pending | paired, mirroring native currentDefaultDevice) and
  all JS-initiated identity writes. The pairing facade exposes
  identity()/onIdentity() (deduped like glasses.onStatus) and
  markPendingSelection().
- The scan-entry pending write, the boot demotion's settings writes, and
  the promotion-retires-pending rule are module methods invoked from the
  existing call sites.
- Host identity gating (home, DeviceStatus, ConnectDeviceButton) reads
  the projected snapshot instead of raw settings keys.
- PAIRING_IDENTITY_KEYS is derived from a nativeAuthoritative flag on
  the Setting descriptors, so the change-push exclusion cannot drift
  from the declarations.
- glasses.isFirstPairing() restates itself as identity().kind === "none".
Comment thread mobile/src/app/pairing/scan.tsx Outdated
Comment thread mobile/modules/island/src/facades/pairing.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 3 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

<file name="mobile/modules/island/src/facades/pairing.ts">

<violation number="1" location="mobile/modules/island/src/facades/pairing.ts:266">
P2: There's a potential race between native pairing promotion and this abandon path. If the native layer just promoted a successful pairing but the JS `isGlassesConnected` state hasn't refreshed yet, this code takes the `preserve && !connected` branch — which disconnects and re-seeds via `pushAllBluetoothSettings`. If the persisted JS settings still hold the pre-promotion identity at that moment, the push could clobber the freshly promoted native default. The window is narrow (native promoted but connection state stale), but in the scenario this PR explicitly guards against (pairing completes while the user backs out), it's the exact timing that matters. Consider gating the re-seed on whether the persisted `device_name` actually differs from the native default, or skipping the push if a promotion event was recently received.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread mobile/modules/island/src/facades/pairing.ts Outdated
…oke point

With PairingIdentity as the only JS-side identity writer, one log line per
write names the trigger for every lifecycle transition — pending selection,
promotion retiring the marker, demotion (with the fresher-selection-kept
case), and which cleanup branch abandonAttempt took. On-device pairing
issues become diagnosable from a single filtered log window instead of
inferring writers from SETTINGS value churn.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

4 issues found across 13 files (changes from recent commits).

You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

<file name="mobile/modules/island/src/facades/pairing.ts">

<violation number="1" location="mobile/modules/island/src/facades/pairing.ts:206">
P2: The `markPendingSelection` facade exposes a public write-path that persists a raw `model` string directly to the pairing identity state without any validation. If callers pass an empty string, whitespace-only value, or an unexpected model identifier, the app will store an invalid pending identity and may render a broken or confusing finish-pairing affordance. Consider adding a guard at the facade or service boundary to reject empty/whitespace values, trim the input, and optionally validate against the known device-model set before persisting.</violation>
</file>

<file name="mobile/src/app/pairing/scan.tsx">

<violation number="1" location="mobile/src/app/pairing/scan.tsx:41">
P2: Reaching the scan screen writes the pending selection via an async call that can fail, but the returned promise is fire-and-forget. If `markPendingSelection` rejects or returns an error, the pending marker never persists, so the home card won’t show the finish-pairing affordance on restart—which undermines the two-phase identity repair this PR is introducing. Other async pairing calls in this file (e.g. `abandonAttempt`) already log failures with `.catch()`; handle this write the same way.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

onIdentity: (cb: (identity: IdentitySnapshot) => void): (() => void) => subscribePairingIdentity(cb),
/** Mark the chosen model as the pending selection (the scan-entry write); the
* host renders it as a finish-pairing affordance until pairing succeeds. */
markPendingSelection: (model: string) => markPendingSelection(model),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The markPendingSelection facade exposes a public write-path that persists a raw model string directly to the pairing identity state without any validation. If callers pass an empty string, whitespace-only value, or an unexpected model identifier, the app will store an invalid pending identity and may render a broken or confusing finish-pairing affordance. Consider adding a guard at the facade or service boundary to reject empty/whitespace values, trim the input, and optionally validate against the known device-model set before persisting.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/modules/island/src/facades/pairing.ts, line 206:

<comment>The `markPendingSelection` facade exposes a public write-path that persists a raw `model` string directly to the pairing identity state without any validation. If callers pass an empty string, whitespace-only value, or an unexpected model identifier, the app will store an invalid pending identity and may render a broken or confusing finish-pairing affordance. Consider adding a guard at the facade or service boundary to reject empty/whitespace values, trim the input, and optionally validate against the known device-model set before persisting.</comment>

<file context>
@@ -186,6 +194,17 @@ export const pairing = {
+  onIdentity: (cb: (identity: IdentitySnapshot) => void): (() => void) => subscribePairingIdentity(cb),
+  /** Mark the chosen model as the pending selection (the scan-entry write); the
+   * host renders it as a finish-pairing affordance until pairing succeeds. */
+  markPendingSelection: (model: string) => markPendingSelection(model),
+
   /** Start scanning for nearby glasses. Results land on `searchResults()`/`onFound()`. */
</file context>

// Two-phase identity: reaching the scan screen marks the chosen model as the
// PENDING selection. Promotion to `paired` only happens natively when pairing
// succeeds; until then the home card renders a finish-pairing affordance.
toolkit.pairing.markPendingSelection(deviceModel)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Reaching the scan screen writes the pending selection via an async call that can fail, but the returned promise is fire-and-forget. If markPendingSelection rejects or returns an error, the pending marker never persists, so the home card won’t show the finish-pairing affordance on restart—which undermines the two-phase identity repair this PR is introducing. Other async pairing calls in this file (e.g. abandonAttempt) already log failures with .catch(); handle this write the same way.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/src/app/pairing/scan.tsx, line 41:

<comment>Reaching the scan screen writes the pending selection via an async call that can fail, but the returned promise is fire-and-forget. If `markPendingSelection` rejects or returns an error, the pending marker never persists, so the home card won’t show the finish-pairing affordance on restart—which undermines the two-phase identity repair this PR is introducing. Other async pairing calls in this file (e.g. `abandonAttempt`) already log failures with `.catch()`; handle this write the same way.</comment>

<file context>
@@ -30,18 +29,16 @@ export default function SelectGlassesBluetoothScreen() {
-    setPendingWearable(deviceModel)
+    // PENDING selection. Promotion to `paired` only happens natively when pairing
+    // succeeds; until then the home card renders a finish-pairing affordance.
+    toolkit.pairing.markPendingSelection(deviceModel)
   }, [])
 
</file context>
Suggested change
toolkit.pairing.markPendingSelection(deviceModel)
void toolkit.pairing.markPendingSelection(deviceModel).catch((error) => {
console.warn("Failed to mark pending selection:", error)
})

Comment thread mobile/jest.setup.js
Comment thread mobile/modules/island/src/services/PairingIdentity.ts Outdated
PhilippeFerreiraDeSousa added 2 commits July 6, 2026 14:57
On-device Mentra Live pairing wiped itself right after succeeding: the home
screen landed on first-run with nothing paired, and the second run worked.
The identity-transition logs pinned the interleaving — handleDeviceReady
promoted and echoed the identity, then the on-connect FULL settings replay
landed with a stale snapshot and overwrote it:

  MAN: handleDeviceReady(): Mentra Live
  SETTINGS: SET: default_wearable = Mentra Live      (promotion echo)
  PairingIdentity: promotion retires pending
  ...
  MAN: checkCurrentAudioDevice: audioDevicePattern:  (device_name applied "")
  SETTINGS: SET: default_wearable =                  (identity clobbered)

On Mentra Live the store's connected transition arrives together with
device-ready, so the on-connect replay races the promotion echoes; its
getBluetoothSettings snapshot is taken while the save_setting relay (which
awaits storage writes) is still mid-flight, i.e. with pre-promotion empty
identity. The overwrite also emptied the audio-watch pattern, flipping
bluetoothClassicConnected false — which re-queued the Pair Audio step whose
no-default guard then bailed home. The second run only worked because the
first run's echoes had persisted device_name/device_address to JS.

Complete the ownership rule: identity travels JS→native ONLY in the
explicit seeds (hydration, pre-connect, post-demotion, abandon re-seed).
The on-connect replay now pushes the device settings MINUS the identity
group (stripPairingIdentity) — while connected, native owns the identity it
just promoted.
…input guards

- abandonAttempt's preserve branch re-seeds native identity ONLY from a
  COMPLETE paired JS identity (identity().kind === "paired"). Mid-relay — a
  promotion's save_setting echoes still landing — the JS snapshot is
  incomplete by construction, and pushing it would overwrite the fresher
  native identity, the same race class the on-connect replay had. Incomplete
  now skips the push; native already holds the truth.
- The scan screen's pending-selection write and scan start re-run when the
  deviceModel param changes instead of staying pinned to the first mount.
- markPendingSelection guards its public write-path: trims, ignores
  empty/whitespace models, and surfaces persist failures (setSetting resolves
  to a Result — a silent error would drop the finish-pairing affordance).
- demoteDefaultToPending reads the fresher-pending check through
  readIdentityString so a truthy non-string legacy value can't both block the
  backfill and project as no identity at all.
- jest.setup scrubs the pairing-identity keys after every test: the pairing
  mocks delegate identity to the real module over the shared settings store,
  and a leaked pending selection would order-couple unrelated tests.
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

PR Agent Orchestrator State

{
  "cycle": 2,
  "fixRound": 0,
  "totalReviewerRuns": 2,
  "consecutiveNoNewReviews": 2,
  "openFindings": [],
  "resolvedFindings": [],
  "nitFindings": [],
  "phase": "discovery",
  "status": "in_progress",
  "lastPair": [],
  "stagnationFixRounds": 0,
  "lastOpenCount": 0,
  "fingerprintReopenCounts": {},
  "mutedFingerprints": []
}

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Agent Review — cycle 2

✅ No blocking findings · 0 blocking · 0 nits
Reviewers this cycle: none

No model reviews ran this cycle.

Updated automatically by the PR Agent Orchestrator each review cycle. Nits do not block merge.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found across 4 files (changes from recent commits).

You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/modules/island/src/services/PairingIdentity.ts">

<violation number="1" location="mobile/modules/island/src/services/PairingIdentity.ts:101">
P2: When `markPendingSelection` receives an empty or whitespace-only model, it currently returns a successful `Res.ok(undefined)` even though no pending selection is actually written. Because this function returns an `AsyncResult<void, Error>`, a future caller that checks the result will incorrectly assume the finish-pairing marker was persisted. Replacing the no-op with a proper `Res.err(...)` keeps the Result contract honest without affecting existing fire-and-forget call sites.</violation>
</file>

<file name="mobile/src/app/pairing/scan.tsx">

<violation number="1" location="mobile/src/app/pairing/scan.tsx:42">
P2: The `markPendingSelection` write is not awaited before the scan starts. Because the pending-selection effect and the scan effect are independent, `toolkit.pairing.scan` can begin (and even complete a skip-device auto-pair) while the pending wearable marker is still in flight. If the app is killed or the user backs out in that window, the two-phase identity state may be lost or the `abandonAttempt` logic may operate on stale settings. Consider merging the two mount effects into a single async flow that awaits the pending write before starting the scan.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

const trimmed = typeof model === "string" ? model.trim() : ""
if (!trimmed) {
console.warn(`PairingIdentity: ignoring pending selection with empty model (got ${JSON.stringify(model)})`)
return Promise.resolve(Res.ok(undefined)) as AsyncResult<void, Error>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: When markPendingSelection receives an empty or whitespace-only model, it currently returns a successful Res.ok(undefined) even though no pending selection is actually written. Because this function returns an AsyncResult<void, Error>, a future caller that checks the result will incorrectly assume the finish-pairing marker was persisted. Replacing the no-op with a proper Res.err(...) keeps the Result contract honest without affecting existing fire-and-forget call sites.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/modules/island/src/services/PairingIdentity.ts, line 101:

<comment>When `markPendingSelection` receives an empty or whitespace-only model, it currently returns a successful `Res.ok(undefined)` even though no pending selection is actually written. Because this function returns an `AsyncResult<void, Error>`, a future caller that checks the result will incorrectly assume the finish-pairing marker was persisted. Replacing the no-op with a proper `Res.err(...)` keeps the Result contract honest without affecting existing fire-and-forget call sites.</comment>

<file context>
@@ -93,8 +93,21 @@ export function subscribePairingIdentity(cb: (identity: IdentitySnapshot) => voi
+  const trimmed = typeof model === "string" ? model.trim() : ""
+  if (!trimmed) {
+    console.warn(`PairingIdentity: ignoring pending selection with empty model (got ${JSON.stringify(model)})`)
+    return Promise.resolve(Res.ok(undefined)) as AsyncResult<void, Error>
+  }
+  console.log(`PairingIdentity: pending selection -> "${trimmed}"`)
</file context>
Suggested change
return Promise.resolve(Res.ok(undefined)) as AsyncResult<void, Error>
return Promise.resolve(Res.err(new Error(`Invalid model: ${JSON.stringify(model)}`))) as AsyncResult<void, Error>

// PENDING selection. Promotion to `paired` only happens natively when pairing
// succeeds; until then the home card renders a finish-pairing affordance.
toolkit.pairing.markPendingSelection(deviceModel)
}, [deviceModel])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The markPendingSelection write is not awaited before the scan starts. Because the pending-selection effect and the scan effect are independent, toolkit.pairing.scan can begin (and even complete a skip-device auto-pair) while the pending wearable marker is still in flight. If the app is killed or the user backs out in that window, the two-phase identity state may be lost or the abandonAttempt logic may operate on stale settings. Consider merging the two mount effects into a single async flow that awaits the pending write before starting the scan.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mobile/src/app/pairing/scan.tsx, line 42:

<comment>The `markPendingSelection` write is not awaited before the scan starts. Because the pending-selection effect and the scan effect are independent, `toolkit.pairing.scan` can begin (and even complete a skip-device auto-pair) while the pending wearable marker is still in flight. If the app is killed or the user backs out in that window, the two-phase identity state may be lost or the `abandonAttempt` logic may operate on stale settings. Consider merging the two mount effects into a single async flow that awaits the pending write before starting the scan.</comment>

<file context>
@@ -39,7 +39,7 @@ export default function SelectGlassesBluetoothScreen() {
     // succeeds; until then the home card renders a finish-pairing affordance.
     toolkit.pairing.markPendingSelection(deviceModel)
-  }, [])
+  }, [deviceModel])
 
   // useFocusEffect(
</file context>

AsyncResult is an enriched thenable, not a bare Promise<Result> — the cast
in markPendingSelection's early return failed the island module's standalone
tsc (run by the mobile postinstall in CI, which the mobile app tsconfig
doesn't replicate). Construct the no-op result with Res.try_async like every
other AsyncResult producer in the store.
Comment thread mobile/src/app/pairing/scan.tsx
const handlePairFailure = useCallback(
(error: string) => {
toolkit.glasses.forget()
// Clears the failed attempt; when a real pairing predates this attempt

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Loading cancel skips attempt cleanup

Medium Severity

Pairing failure on the loading screen calls toolkit.pairing.abandonAttempt, but header back and loader cancel only call goBack. An in-flight BLE connect started from the flow is not torn down on cancel, unlike on failure, leaving partial native pairing state while the UI moves on.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1d6955e. Configure here.

The header chevron and Cancel button called goBack() directly, which only
runs the abandonAttempt cleanup on iOS (via the beforeRemove listener) —
on Android the registered handler covers hardware back alone, so chevron
and Cancel back-outs skipped disconnect/forget/re-seed entirely. Route all
explicit surfaces through one handler and dedupe with a per-mount ref: an
explicit back-out's goBack() also fires beforeRemove on iOS, which
previously double-ran the cleanup path and popped a second time.
Comment thread mobile/src/app/pairing/scan.tsx

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

<file name="mobile/modules/island/src/facades/pairing.ts">

<violation number="1" location="mobile/modules/island/src/facades/pairing.ts:206">
P2: The `markPendingSelection` facade exposes a public write-path that persists a raw `model` string directly to the pairing identity state without any validation. If callers pass an empty string, whitespace-only value, or an unexpected model identifier, the app will store an invalid pending identity and may render a broken or confusing finish-pairing affordance. Consider adding a guard at the facade or service boundary to reject empty/whitespace values, trim the input, and optionally validate against the known device-model set before persisting.</violation>
</file>

<file name="mobile/src/app/pairing/scan.tsx">

<violation number="1" location="mobile/src/app/pairing/scan.tsx:41">
P2: Reaching the scan screen writes the pending selection via an async call that can fail, but the returned promise is fire-and-forget. If `markPendingSelection` rejects or returns an error, the pending marker never persists, so the home card won’t show the finish-pairing affordance on restart—which undermines the two-phase identity repair this PR is introducing. Other async pairing calls in this file (e.g. `abandonAttempt`) already log failures with `.catch()`; handle this write the same way.</violation>

<violation number="2" location="mobile/src/app/pairing/scan.tsx:42">
P2: The `markPendingSelection` write is not awaited before the scan starts. Because the pending-selection effect and the scan effect are independent, `toolkit.pairing.scan` can begin (and even complete a skip-device auto-pair) while the pending wearable marker is still in flight. If the app is killed or the user backs out in that window, the two-phase identity state may be lost or the `abandonAttempt` logic may operate on stale settings. Consider merging the two mount effects into a single async flow that awaits the pending write before starting the scan.</violation>
</file>

<file name="mobile/modules/island/src/services/PairingIdentity.ts">

<violation number="1" location="mobile/modules/island/src/services/PairingIdentity.ts:101">
P2: When `markPendingSelection` receives an empty or whitespace-only model, it currently returns a successful `Res.ok(undefined)` even though no pending selection is actually written. Because this function returns an `AsyncResult<void, Error>`, a future caller that checks the result will incorrectly assume the finish-pairing marker was persisted. Replacing the no-op with a proper `Res.err(...)` keeps the Result contract honest without affecting existing fire-and-forget call sites.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread mobile/src/app/pairing/scan.tsx
handleBackOut popped unconditionally, so a rapid double-tap on Cancel or
the header chevron popped two routes while the cleanup ran once. Gate the
goBack() on runBackOutCleanup()'s first-run return, matching the
hardware-back handler.
Comment thread mobile/src/components/home/DeviceStatus.tsx
The pending-identity branch ran before any connected-state check, so in
the window where the native layer has promoted and the BLE link is up but
the save_setting echoes are still landing (JS identity momentarily still
pending), home showed finish-pairing actions and a disconnected icon for
an already-connected device. Gate the pending card on not-connected and
let the normal card render with the pending model as its display name
until the echoes complete it to paired — same string, seamless handoff.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="mobile/src/app/pairing/btclassic.tsx">

<violation number="1" location="mobile/src/app/pairing/btclassic.tsx:45">
P2: The `handleSuccess` in `btclassic.tsx` calls `toolkit.glasses.connect(device)` without awaiting it before navigating away via `pushPrevious()`. If the connect promise rejects (e.g., BLE timeout, invalid device) instead of emitting a SDK-level `pair_failure` event, the loading screen's `onPairFailure` listener won't catch it, and the user will wait the full 35-second `waitForReady` timeout before seeing an error. Consider awaiting the connect result (or at minimum, forwarding a timeout/error to navigation) so a failed connect doesn't leave the loading screen hanging.</violation>
</file>

<file name="mobile/modules/island/src/facades/pairing.ts">

<violation number="1" location="mobile/modules/island/src/facades/pairing.ts:206">
P2: The `markPendingSelection` facade exposes a public write-path that persists a raw `model` string directly to the pairing identity state without any validation. If callers pass an empty string, whitespace-only value, or an unexpected model identifier, the app will store an invalid pending identity and may render a broken or confusing finish-pairing affordance. Consider adding a guard at the facade or service boundary to reject empty/whitespace values, trim the input, and optionally validate against the known device-model set before persisting.</violation>
</file>

<file name="mobile/src/app/pairing/scan.tsx">

<violation number="1" location="mobile/src/app/pairing/scan.tsx:41">
P2: Reaching the scan screen writes the pending selection via an async call that can fail, but the returned promise is fire-and-forget. If `markPendingSelection` rejects or returns an error, the pending marker never persists, so the home card won’t show the finish-pairing affordance on restart—which undermines the two-phase identity repair this PR is introducing. Other async pairing calls in this file (e.g. `abandonAttempt`) already log failures with `.catch()`; handle this write the same way.</violation>

<violation number="2" location="mobile/src/app/pairing/scan.tsx:42">
P2: The `markPendingSelection` write is not awaited before the scan starts. Because the pending-selection effect and the scan effect are independent, `toolkit.pairing.scan` can begin (and even complete a skip-device auto-pair) while the pending wearable marker is still in flight. If the app is killed or the user backs out in that window, the two-phase identity state may be lost or the `abandonAttempt` logic may operate on stale settings. Consider merging the two mount effects into a single async flow that awaits the pending write before starting the scan.</violation>
</file>

<file name="mobile/modules/island/src/services/PairingIdentity.ts">

<violation number="1" location="mobile/modules/island/src/services/PairingIdentity.ts:101">
P2: When `markPendingSelection` receives an empty or whitespace-only model, it currently returns a successful `Res.ok(undefined)` even though no pending selection is actually written. Because this function returns an `AsyncResult<void, Error>`, a future caller that checks the result will incorrectly assume the finish-pairing marker was persisted. Replacing the no-op with a proper `Res.err(...)` keeps the Result contract honest without affecting existing fire-and-forget call sites.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread mobile/src/components/home/DeviceStatus.tsx
…selection

In the connected-but-not-promoted window the pending identity reaches the
ordinary card, and its Connect handler's empty-pairedModel fallback sent the
user to model selection — discarding the model the pending selection already
knows. Resume the scan for the pending model instead; only a truly
identity-less state starts over at selection.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a839611. Configure here.

Comment thread mobile/modules/island/src/facades/pairing.ts
The forget branch keyed on the native default-device read alone: if native
lost its default while the persisted settings still described a complete
pairing (a divergence no normal flow produces, but the guards route exactly
such users into the scan), backing out or retrying would forget the real
pairing — the native echoes then clear the JS identity too. Restructure the
decision on both layers: a complete paired JS identity always re-seeds
native (repairing the divergence), a native default with a mid-relay JS
snapshot is left as native truth, and forgetting happens only when BOTH
layers agree there is no pairing.
@PhilippeFerreiraDeSousa
PhilippeFerreiraDeSousa merged commit e61b056 into integration/toolkit-boundary Jul 6, 2026
17 of 19 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.

1 participant