Ignore a connect timeout superseded by a newer attempt on the same peripheral

[krishrathi1]

The bug

BLERadioController.beginCentralConnection's timeout closure (bitchat/Services/BLE/BLERadioController.swift) is scheduled bleConnectTimeoutSeconds (8s) after every connect attempt. When it fires, its only staleness check is:

guard let state = self.linkStateStore.state(forPeripheralID: peripheralID),
      state.isConnecting && !state.isConnected else { return }

It never checks whether this is still the attempt it was scheduled for.

Failure scenario

Ordinary BLE usage: a peer connects, then walks behind an obstacle / into a pocket / through a crowd and disconnects a few seconds later -- well within the 8s timeout window. didDisconnectPeripheral tears the old link state down and tryConnectFromQueue() immediately starts a new attempt, calling beginConnecting again (a fresh lastConnectionAttempt). That new attempt is healthy and mid-flight when the original attempt's timer fires. The guard above only checks isConnecting && !isConnected -- true for the new attempt too -- so the stale timer cancels the new, live connection, tears it down, and calls scheduler.recordConnectionTimeout, applying a discovery-ignore cooldown and a weak-link scoring penalty to a peer that never actually timed out.

Result: a healthy, in-range peer's legitimate reconnection gets cancelled and then artificially deprioritized -- repeatable indefinitely in any marginal-signal environment (pocket, crowd, subway), silently degrading mesh connectivity with nothing a user would recognize as a bug (no crash, no error visible anywhere).

The fix

Captures the Date passed to beginConnecting for this specific attempt (attemptStartedAt) and compares it against state.lastConnectionAttempt inside the timeout closure. beginConnecting overwrites that field on every new attempt for the same peripheralID (so does armPendingBackgroundConnects, which sets it nil for background wake-on-proximity connects, a deliberately different mechanism with no timeout closure of its own). A mismatch means a newer attempt has started since this timer was scheduled, so the stale timer now just logs and returns instead of tearing anything down.

Verification

No Xcode/Swift toolchain available here, so this is verified by tracing the state machine, not compiling:

• Confirmed beginConnecting fully replaces the peripheral's link-state entry (not a partial update) on every call, so any second beginConnecting for the same peripheralID necessarily changes lastConnectionAttempt.
• Confirmed the only other writer of isConnecting: true (armPendingBackgroundConnects) explicitly sets lastConnectionAttempt: nil on purpose (existing comment: "an indefinite pending connect has no attempt clock") and schedules no timeout closure of its own -- so it can never collide with or accidentally satisfy this check.
• Confirmed cancelStalePendingConnects() (the foreground-return sweep) is an independent mechanism keyed on lastConnectionAttempt age directly, unaffected by this change.

No automated regression test. Grepped bitchatTests -- BLERadioController has zero existing test coverage; only its BLEConnectionScheduler dependency is unit-tested in isolation. CBCentralManager/CBPeripheral have no public initializers, so a from-scratch CoreBluetooth mock harness would itself be unverified without a compiler to check it against. Given that, I'd rather ship the 21-line fix (reviewable by inspection, the diff is small) and flag the coverage gap honestly than add unverified test infrastructure alongside an unverified fix. Happy to build test scaffolding for this class as a follow-up if that's wanted, or to have someone with the toolchain confirm behavior directly.

[Chessing234]

traced this through and the mechanism holds — beginConnecting stores the date verbatim (BLELinkStateStore.swift:114) so the == round-trips exactly, and the nil case from armPendingBackgroundConnects correctly reads as "superseded" rather than matching.

two things i'd raise anyway.

using Date as the attempt identity works but couples correctness to timestamps being distinct. a monotonically increasing attempt counter on the link state would say what it means, and can't be confused by a clock adjustment mid-attempt — NTP stepping the clock backwards between beginConnecting and the timeout would make a live attempt look superseded and leak the timer.

second, there's no test. this is precisely the class of bug that comes back, because reproducing it needs a disconnect and reconnect inside the timeout window and nobody does that by hand twice. the timeout closure is hard to test as written, but the decision — given a captured attempt stamp and the current state, should this timeout act — is a pure function and would be worth extracting for one.

[Chessing234]

capturing attemptStartedAt and comparing it to lastConnectionAttempt is the right staleness check — the old isConnecting guard alone couldn't tell a superseded reconnect from the original attempt. looks good.

[krishrathi1]

Updated the PR to address review feedback:

1. Monotonic Attempt Counter: Replaced the Date-based attempt identity with a monotonically increasing �ttemptToken: UInt64 on BLEPeripheralLinkState. This avoids coupling attempt identity to timestamps and eliminates false positive superseded detections caused by clock adjustments/NTP shifts.
2. Pure Decision Helper & Unit Tests: Extracted BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout as a pure function and added unit tests in BLEConnectTimeoutPolicyTests.swift covering matching attempt tokens, superseded tokens, connected state, and missing link state.

[krishrathi1]

Fixed parameter signature in BLEConnectTimeoutPolicyTests.swift (initializeMemory count argument). CI re-triggered.

[krishrathi1]

Refactored BLEConnectTimeoutPolicy.shouldExecuteConnectTimeout to accept pure primitive parameters (capturedAttemptToken, isConnecting, isConnected, currentAttemptToken, isPeripheralConnected). This allows unit testing the policy without instantiating fake CBPeripheral mock pointers, eliminating ARC memory management crashes in iOS runner.