fix: harden desktop BLE — stale peripheral cache, state listener lifetime, acquire deadline

[originalix]

Combined desktop-BLE hardening from the 6.5.0 field investigation (Classic 1S, macOS; frequent disconnects + a 5-minute unkillable hang). Consolidates the former #889 into one branch so the app can pin a single alpha for self-testing.

Fixes

1. Stale cached peripheral after physical disconnect (hd-transport-electron/noble-ble-handler.ts)
On macOS, reconnecting a previously-disconnected cached noble peripheral deterministically returns zero GATT services (18/18 cycles in the field log). disconnectDevice / handleDeviceDisconnect now pass cleanupDiscoveredCache: true, so the next connect resolves a fresh peripheral via tryDirectConnectById or scan — the field-proven path.

2. persistentStateListener died permanently after a renderer soft restart (same file)
The webContents destroyed handler removed the process-lifetime state listener and nothing re-registers it (initializeNoble early-returns once noble is loaded; the poweredOn fast path also skipped first-time registration entirely). Registration now happens right after noble loads, and the destroy handler no longer removes it (deduped by the existing null-guard).

3. Unbounded device.acquire hang (core/src/core/index.ts)
Field case: checkAllFirmwareRelease hung 306 s inside ensureConnected → connectDeviceForBle → device.acquire() after the Electron main process lost an IPC reply; the per-try timer is cleared before the await and cancel only takes effect at poll checkpoints. raceBleAcquire now bounds acquire with a 60 s deadline (BleTimeoutError, handled by the existing retry loop after a cold link-drop) and rejects immediately on the caller's AbortSignal.
Scoped to env === 'desktop-web-ble' only — react-native/lowlevel acquire may legitimately block on a user-driven system bonding prompt, so those envs keep the plain acquire byte-for-byte unchanged.

4. Request-queue task leak on onlyConnectBleDevice early return (same file)
The early return bypassed the normal-path releaseTask; in the field log a completed task haunted every queue snapshot and cancel sweep for 6 minutes. Env-neutral bookkeeping fix, no protocol behavior change.

Blast radius

• noble-ble-handler.ts is loaded only by the Electron main process (desktop). Mobile (hd-transport-react-native), WebUSB, Bridge, and extension transports never load it.
• The hd-core acquire guards are gated on desktop-web-ble; all other envs run the exact previous acquire path.
• The only env-neutral change is the releaseTask bookkeeping fix (4).

Verification

• tsc --noEmit and eslint on both packages: error sets byte-identical to base (pre-existing unresolved-workspace-import findings only).
• Adversarially reviewed; one defect found and fixed in-branch (acquire-promise handlers now attach before the aborted-at-entry early return, and a cancel during the 3 s retry backoff no longer starts a post-cancel acquire).
• Version bumped to 1.2.0-alpha.147 (npm already has up to .146 from side-branch releases) for app-side pinning and self-test.

[originalix]

Note on versioning: this PR bumps all packages to 1.2.0-alpha.147 (npm already has up to alpha.146 from side-branch releases); #889 takes alpha.148. Whichever merges second needs a trivial version rebase.

[originalix]

Automated code review found blocking issues

@originalix

Reviewed commit b2cb2451bd3a. This report always reflects the latest reviewed changes.

Review summary

The PR hardens desktop Electron Noble BLE lifecycle behavior by adding idle unsubscribe-before-disconnect cleanup, stale peripheral cache invalidation, family-specific scan versus direct-connect selection, forced reconnects before service discovery, and bounded desktop BLE acquire waits. It also changes React Native and Electron protocol selection to trust declared V1 values and aligns package versions across the monorepo.

What needs attention: Resolve asynchronous acquire serialization and late completion, make forced reconnect teardown wait for confirmed physical disconnect, preserve method disposal on connect-only success, and require active protocol confirmation before accepting a V1 session.

Issues to address

• P1 · Acquire timeout retries while the original BLE acquire is still in flight
  • Impact: The timeout races only the wrapper promise. The underlying device.acquire() continues running, and the timeout cleanup usually cannot disconnect it because the session identifier is assigned only after acquire resolves. Recursive retry can therefore start a second acquire on the same device, while a late first response may recreate commands or mark the device acquired after the retry has moved on.
  • Suggested action: Cancel or invalidate the underlying acquire and quarantine late completion using the connection-attempt generation; do not begin a retry until the previous operation is safely discarded.
• P1 · Protocol V1 is accepted without active device confirmation
  • Impact: Electron BLE and React Native now set V1 solely from expectedProtocol and skip the probe. A stale descriptor, replaced device, or device that actually speaks V2 is then labeled V1 and receives V1 framing, violating protocol detection requirements and causing communication or firmware-reconnect failures.
  • Suggested action: Keep the V1 probe for unconfirmed sessions, or only use the no-probe fast path when the V1 protocol was confirmed on the current connection generation.
• P2 · Forced reconnect still uses the 250 ms teardown timeout
  • Impact: Every cold service-discovery setup now calls forceReconnectPeripheral, but its disconnect operation still resolves after BLE_CLEANUP_TIMEOUT (250 ms). If Noble/CoreBluetooth has not completed teardown, connect() runs against a stale link and can reproduce the GATT/session failure this reconnect is intended to fix.
  • Suggested action: Use the confirmed disconnect timeout and require the peripheral to report disconnected before reconnecting; otherwise obtain a fresh peripheral object.
• P2 · Connect-only success bypasses method disposal
  • Impact: The onlyConnectBleDevice branch now completes request tracing and releases the queue task, but returns before the common cleanup path invokes method.dispose(). Repeated successful preconnects can retain method-owned resources or listeners.
  • Suggested action: Dispose the method before returning, or route connect-only responses through the same cleanup/finally path used by normal calls.

Validation gaps

• No regression test covers timeout or cancellation followed by a late acquire response and retry serialization.
• No transport test verifies that forced reconnect waits for a slow physical disconnect callback.
• No test asserts active protocol probing or stale-descriptor handling for Electron and React Native V1 sessions.
• No test covers method disposal after successful connect-only calls.
• Physical validation is still needed across Classic 1S and Pro2/Neo for idle release, renderer restart, powered-off recovery, and cold reconnect.

[originalix]

Coordination note re #886 (about to merge): merge-tree simulation both orders shows only ONE real code conflict — a single hunk at the connectDeviceForBle call site in ensureConnected (trivial union: keep beginConnectionAttempt and pass abortSignal) — plus 34 package.json version collisions (147 vs 150) that disappear if this PR's bump commit is dropped on rebase. Combined semantics verified compatible (their non-retryable codes skip our retry loop; generation interrupt and acquire deadline are mutually exclusive by construction).

⚠️ npm next line is content-forked: alpha.147 = this branch only (gitHead a34e988), alpha.148/149/150 = #886's branch only (gitHead 0835952, contains none of this PR's fixes; #886's local 'release 147' commit never published). No published version has both fix sets. Plan: after #886 merges, rebase this PR (drop the 147 bump, resolve the one hunk), bump to 1.2.0-alpha.151 and publish — 151 becomes the first version containing both.

[originalix]

Rebased onto onekey post-#886: dropped the alpha.147 bump, resolved the single expected hunk in ensureConnected as the union (kept beginConnectionAttempt, passed abortSignal), verified the merged retry path (BleTimeoutError still retryable, user-interrupt stops retries, #886's own CallQueueActionCancelled reject-list entry supersedes the one this PR previously carried). Re-bumped to 1.2.0-alpha.151 — the first version containing both this PR and #886.

⚠️ Publish is currently blocked: the publish-npm-packages workflow fails reproducibly with lerna E404 on the FIRST PUT (runs 32215957706, 32216269087) while npm status is green — the repo's NPM_TOKEN secret appears expired/revoked as of today (npm masks unauthorized writes as 404; the same workflow published alpha.147/148/149/150 fine yesterday). Needs an admin to rotate the secret, then re-dispatch the workflow on this branch.

[originalix]

1.2.0-alpha.151 published from this branch (gitHead d1fa81c) after the NPM_TOKEN secret was rotated — the earlier E404s were the expired token (same package PUT: 200 at 02:16 UTC, 404 from 04:33; workflow and secret injection ruled out). alpha.151 = first version containing both this PR and #886. app-monorepo#12928 now pins it.

[originalix]

TF regression (build 2026081959 = alpha.151) surfaced one more defect, root-caused from field logs and fixed here (alpha.153):

Keep-alive idle/backstop teardown skipped the CCCD unsubscribe — the only teardown path in the codebase that dropped the link without unsubscribing first. On Classic 1S this leaves the device's notify session half-open: for tens of minutes afterwards (observed >3m47s, <47min), every new link gets full GATT (services/chars/CCCD all confirm) but the device never answers application-protocol traffic — the V1 GetFeatures probe times out at 3s, the transport tears down and retries, producing a visible connect/disconnect loop that ends in DeviceNotFound. Field evidence: 2/2 unsubscribe-first teardowns (app quit) reconnected instantly; the lone unsubscribe-skipping idle teardown caused a 6-attempt loop; 43 min later (zero BLE activity, no app restart) the same direct-connect path succeeded — device-side state had self-cleared.

Changes: (1) armIdleDisconnect now runs unsubscribeNotifications before disconnectDevice; (2) the physical disconnect wait is raised from the 250ms resolve-cap to a 3s confirm window with a warning when the OS never confirms — previously an incomplete teardown left zero log trace; (3) warn added to the transport Logger interface.

Also flagging for firmware: the 1S taking tens of minutes to clear a session after an unsubscribed link drop is worth a device-side look.

[originalix]

One more field-driven change (alpha.154): BLE_IDLE_DISCONNECT_MS 180s → 20s. TF regression established that the Classic 1S goes protocol-deaf (SoftDevice keeps advertising/accepting connections and serving GATT, but the application layer never answers protocol traffic; only a device reboot reliably recovers) when the link is dropped after sitting idle for ~3 minutes — while disconnects right after traffic have never produced a deaf window across 6.5.0's entire per-call-teardown history and 3/3 hot-disconnect samples today. Note: the unsubscribe-first fix alone did NOT prevent it (18:11 unsubscribed idle teardown → deaf 2.5min later), so a hot disconnect ~20s after the last op is the mitigation that keeps us inside the proven-safe pattern; it also shrinks the phone-invisibility window from 3min to 20s. Within-workflow reuse is unaffected (every write re-arms the timer). Trade-offs: >20s user think-time between calls costs one ~2-4s reconnect; the portfolio-sync connected-only lease will rarely find a live link anymore (@wabicai FYI — graceful degradation, but worth a look). A firmware issue for the underlying deaf-sleep behavior is being filed separately.

[originalix]

P2: [Clearing the discovered peripheral cache on every disconnect breaks the non-advertising fallback]

This new cleanupDiscoveredCache: true runs on every manual or idle disconnect, so the next reconnect no longer has the previous Peripheral object as a fallback. After that, connectDevice() can only recover through tryDirectConnectById() or performTargetedScan().

That is not equivalent. tryDirectConnectById() returns undefined whenever connectAsync is unavailable or during its 15-second cooldown, and performTargetedScan() only succeeds if the device advertises again. A bonded device that stays silent between calls can now regress from reconnecting successfully to DeviceNotFound.

Please keep the last discovered peripheral until a replacement peripheral has been resolved successfully, or limit cache eviction to the backend cases where direct-connect is guaranteed to cover the reconnect path.

[originalix]

Fixed in 3dd1b964d8 (1.2.0-alpha.166), close to your first suggestion. cleanupDevice no longer drops the evicted Peripheral outright: it moves it into a process-lifetime stalePeripherals map, and connectDevice uses it only as the last resort, after both a targeted scan and tryDirectConnectById come up empty (and clears it as soon as a fresh peripheral is resolved). A bonded device that stays silent — or a connect-by-id sitting in its 15s cooldown — therefore keeps the old reconnect path instead of regressing to DeviceNotFound, while a stale object is never preferred again, which is what the eviction was for: reconnecting a retrieved/cached peripheral is what produces the link where GATT resolves from cache but the device answers no protocol traffic.

[originalix]

Correction: this fix is deferred, not shipped. It landed in d6be62d0f (published as 1.2.0-alpha.166) and has now been reverted in 72e137fca.

Reason: this file's connection path is currently under field validation against a 6.5.0 control build — the cold-connect route, the forced reset and the cache eviction were each derived from device logs, and changing them mid-validation would invalidate the comparison. The finding itself stands and is queued to land once the root cause is confirmed.

Nothing under test was affected: the app PR pins 1.2.0-alpha.165, which never contained these changes.

[originalix]

P1: [Timed-out acquire can mutate a newer connection attempt]

When the deadline or abort wins, raceBleAcquire rejects but the underlying Device.acquire keeps running. The timeout cleanup only disconnects the current link and marks transport state; it does not invalidate the acquire generation. If the old acquire resolves after the retry starts, Device.acquire can still commit mainId, deviceAcquired, commands, and protocol state for the stale link, corrupting the newer session.

Invalidate the abandoned acquire with the same interruption or generation mechanism used for cancellation, and guard the state-commit section so a late completion cannot overwrite the active connection.

[originalix]

Fixed in 3dd1b964d8 (published as 1.2.0-alpha.166).

connectDeviceForBle now calls device.beginConnectionAttempt() in the deadline/abort catch, before the disconnect attempt. That supersedes the abandoned acquire, so when it settles later the generation guard already in Device.acquire (interruptedAttempt === attempt || connectionAttempt !== attempt) drops the link it built and throws DeviceInterruptedFromUser instead of committing mainId, deviceAcquired, commands or protocol state over the newer attempt. The bump also covers the case you called out where mainId is still unset, so the existing disconnect guard is skipped.

Your stronger suggestion — moving the deadline into the Electron transport so it aborts the underlying IPC — is the better end state and I have not done it here; this keeps the fix inside Core where the retry lives. Happy to follow up with the transport-level abort in a separate change.

Duplicate threads for this same finding are being resolved together; this one carries the resolution.

[originalix]

Correction: this fix is deferred, not shipped. It landed in d6be62d0f (published as 1.2.0-alpha.166) and has now been reverted in 72e137fca.

Reason: this file's connection path is currently under field validation against a 6.5.0 control build — the cold-connect route, the forced reset and the cache eviction were each derived from device logs, and changing them mid-validation would invalidate the comparison. The finding itself stands and is queued to land once the root cause is confirmed.

Nothing under test was affected: the app PR pins 1.2.0-alpha.165, which never contained these changes.

[originalix]

P1: [Timed-out acquire can mutate a newer connection attempt]

When the deadline or abort wins, raceBleAcquire rejects but the underlying Device.acquire keeps running. The timeout cleanup only disconnects the current link when device.mainId is already set; on a first acquire there may be no session to disconnect, and the acquire generation is not invalidated. If the old acquire resolves after the retry starts, Device.acquire can still commit mainId, deviceAcquired, commands, and protocol state for the stale link, corrupting the newer session.

Invalidate the abandoned acquire with the same interruption or generation mechanism used for cancellation, and guard the state-commit section so a late completion cannot overwrite the active connection.

[originalix]

P1: [Acquire timeout retries while the original BLE acquire is still in flight]

When the 60-second race expires, only the wrapper promise is rejected; the underlying device.acquire() continues running. Device.mainId is assigned after that promise resolves, so this guard normally has no session to disconnect and the recursive retry starts a second acquire on the same device. A late response from the first acquire can then recreate commands or mark the device acquired after the retry has moved on.

Please invalidate/cancel the underlying acquire (or advance the connection-attempt generation so late completion is discarded) and wait for that operation to be safely quarantined before starting a retry.

[originalix]

P2: [Connect-only success path still skips method disposal]

This early return now completes tracing and removes the request task, but it still bypasses the common cleanup block that calls method.dispose(). Repeated successful connect-only/preconnect calls can therefore retain method-owned resources or listeners.

Dispose the method before returning from this branch, or route the response through the same cleanup/finally path used by normal calls.

[originalix]

P1: [Acquire timeout retries while the original BLE acquire is still in flight]

When the 60-second race expires, only the wrapper promise is rejected; the underlying device.acquire() continues running. Device.mainId is assigned after that promise resolves, so this guard normally has no session to disconnect and the recursive retry starts a second acquire on the same device. A late response from the first acquire can then recreate commands or mark the device acquired after the retry has moved on.

Please invalidate or cancel the underlying acquire, or advance the connection-attempt generation so late completion is discarded, and quarantine that operation before starting a retry.

[originalix]

P2: [Connect-only success path still skips method disposal]

This early return now completes tracing and removes the request task, but it still bypasses the common cleanup block that calls method.dispose(). Repeated successful connect-only/preconnect calls can therefore retain method-owned resources or listeners.

Dispose the method before returning from this branch, or route the response through the same cleanup/finally path used by normal calls.

[originalix]

P1: [Acquire timeout retries while the original BLE acquire is still in flight]

When the deadline or cancellation fires, only the wrapper promise is rejected; the underlying device.acquire() continues running. Device.mainId is usually assigned after that promise resolves, so this cleanup often has no session to disconnect and the retry starts a second acquire on the same device. A late response from the first acquire can then recreate commands or mark the device acquired after the retry has moved on.

Please cancel or invalidate the underlying acquire and quarantine late completion with the connection-attempt generation before starting a retry.

[originalix]

P1: [Protocol V1 is accepted without active device confirmation]

This branch sets V1 solely from expectedProtocol and skips any device response. After a reconnect, firmware transition, or device replacement, that metadata can be stale; the transport will label a V2 endpoint as V1 and send the wrong framing, causing communication or firmware-reconnect failures.

Please retain the V1 probe unless V1 was confirmed on the current connection generation, using metadata only to choose probe order.

[originalix]

P2: [Connect-only success bypasses method disposal]

This early return now completes tracing and removes the request task, but it still bypasses the common cleanup path that calls method.dispose(). Repeated successful connect-only/preconnect calls can retain method-owned resources or listeners.

Please dispose the method before returning from this branch, or route the response through the same cleanup/finally path used by normal calls.