Phase 2: Work-driven connection lifecycle (Approach B) - #71
Open
itsniper wants to merge 31 commits into
Open
Conversation
Add idleDisconnectInterval to ReliaBLEConfig (global, FR-1.5) and the bluetoothPoweredOff / bluetoothUnsupported error cases documenting the wait-vs-fail radio policy (D-radio). No behavior change.
Add waitUntilPoweredOn() mirroring the authorization-continuation pattern: park continuations on transient radio states (.resetting/.unknown), fail fast with typed errors for terminal states, and resolve all waiters from handleCentralManagerStateUpdate. Wire idleDisconnectInterval through BluetoothActor.init with finite/>=0 validation. Add test hooks. No app path calls waitUntilPoweredOn yet.
Introduce the derived demand signal (work leases + manual-connect holds) that drives the connection lifecycle, and extract the single ensure-linked path: - Add actor-isolated activeLeases (Set<UUID>) + manualConnectHold - extract issueConnect as the ONLY centralManager.connect call site - syncReconnectIntent keeps reconnectEnabled in step with wantsReconnect - reevaluateLink(id:reason:) as the single ensure-linked entry point - applyManualConnectHold / applyManualDisconnect (settling rule) - acquireWorkLease / releaseWorkLease (+ test hooks) - Peripheral.connect: hold-first, then wait, then reevaluateLink(.explicitConnect); disconnect routes to applyManualDisconnect; add lease wrappers - handleDidConnect adds syncReconnectIntent Step 4 of work-driven connection lifecycle (#51).
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
CoreBluetoothMock always resolves a Tier-0 reconnect attempt — relinking on success, or reporting didFailToConnect on failure — so the window in which the cached state is .reconnecting(.system) cannot be held open. The end-to-end test raced that resolution and failed intermittently on slower CI machines in both directions: first the relink won (no limbo observed), then, once the relink was forced to fail, didFailToConnect won (state settled .failed, so no cancel). The production guard is unaffected and still pinned deterministically: idleTeardownDuringCachedSystemReconnectCancels covers the cached arm and idleTeardownDuringLiveConnectingCancels the live .connecting arm — both mutation-verified. Record the real-radio behaviour as on-device check F3.4b. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Do not re-emit bluetoothUnavailable for peripherals already disconnected or failed when the radio dies, so stream UIs keep a clean intentional disconnect. Demo now shows connect/disconnect throws instead of try?, and manual connect logs a warn when the radio gate or link issue fails.
Shorten the already-terminal invalidate observation window and clean up mock connection state so CoreBluetoothMock is less stressed between tests. Restore sticky simulateInitialState pollution with a real power cycle, and assert the missing-peripheral ladder gate from the same actor turn so a later poweredOn idle sweep cannot rewrite .failed.
Publish AwaitingRadio reconnecting when a reconnect-wanting manual hold is registered while Bluetooth is off, and settle immediately to clean disconnected when disconnect clears demand with no live peripheral. Demo shows "Waiting for Bluetooth…" for nil-attempt radio await so stream-only UIs stay honest during F1.3/F4.3-style outages.
Load persisted manual-connect holds on cold start even when CoreBluetooth restores no peripherals, key holds by CBPeripheral UUID for name drift, prefer advertisement local name over GAP device name, and merge Demo SwiftData devices by cbUUID so one radio is not listed twice.
An advertisement that arrives without a local name resolves a peripheral to its cbIdentifier string; the next packet carrying one upgrades the app-facing id. The discovery row was reused, but every other per-id record stayed behind — including the live CBPeripheral, leaving two cbPeripherals keys pointing at one object. The reverse lookup that resolves delegate callbacks then picked arbitrarily, so didConnect could land under an id with no demand: the caller's stream stayed on .connecting while the idle timer tore the link back down seconds later. Identity resolution now migrates every per-id record onto the new id, gated on id drift rather than on a durable hold existing. Alongside that: - Re-drive scheduled work under the new id. Both task registries key their closures by the id captured at arm time, so a moved idle timer or ladder step has to be re-armed rather than only cancelled. - Release work leases by lease UUID. A token minted before a migration names the obsolete id, and a lookup that trusted it leaked demand. - Split the AwaitingRadio projection from a real ladder step. Only the latter reschedules; the former re-drives the link, and discovery now re-issues it once the live reference is bound. - Alias obsolete ids to their replacement so a Peripheral handle vended before an upgrade keeps working, and mirror connection state onto those handles so they cannot disagree with the id that replaced them. Aliases are dropped when a device claims the retired id again. - Harden id(for:) to resolve a duplicate binding in favour of the published id instead of letting dictionary order decide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RqvRXATrnLfBqRbkP7BffS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the work-driven connection lifecycle (Approach B) per
docs/plans/work-driven-connection-lifecycle-2026-08-02.md.Work now drives the link: a
Peripheralconnects because pending work needs a connection, tears the link down after a configurable idle interval when no work and no manual-connect hold remain, gates both reconnect tiers on that same demand signal, and awaits a usable radio instead of silently no-op'ing when Bluetooth is notpoweredOn.Closes #51
Closes #57
Closes #58
Closes #59
Closes #60
Sub-issue checklist
What changed
Demand model. Demand is derived, never stored:
demand(id)= live work leases or a manual-connect hold;wantsReconnect(id)= leases or a hold that asked for auto-reconnect.reconnectEnabledis reduced to a value synced from that signal rather than an independent gate.Single ensure-linked path.
reevaluateLink(id:reason:)is the one entry point, andcentralManager.connect(_:options:)is called from exactly one side-effect-free function,issueConnect— enforcing FR-11.4's "no parallel connection stacks".Radio gating.
startScanningis nowasync throws;.poweredOff/.unsupported/ unauthorized fail fast with typed errors, while.resetting/.unknownare awaited. NewPeripheralError.bluetoothPoweredOffand.bluetoothUnsupported.Idle teardown.
ReliaBLEConfig.idleDisconnectInterval(global, default 5s). Idle teardown routes through the intentional-cancel path, which is what ends Tier-0.Durable manual-connect holds. Persistence changed shape from a set of reconnect-enabled ids to a hold map (
id → reconnectDesired), written only bysyncReconnectIntentand read back at restore.invalidatePeripherals()andshutdown()never touch disk. Work leases are never persisted.Radio-drop projection.
.poweredOffnow joins the invalidate triggers, and demand-carrying peripherals project.reconnecting(source: .library, attempt: nil, nextRetryAt: nil)so the app can distinguish "coming back" from "gone".Breaking changes
ReliaBLEManager.startScanning(services:)is nowasync throws.restoreIdentifier-keyedUserDefaultsvalue changes shape; a value from an older build does not decode and is treated as "no persisted holds". Pre-release, no migration shim — one relaunch after upgrading loses standing holds.Verification
swift build✅swift test— 117 tests ✅ (green on repeated runs)swift package generate-documentation --target ReliaBLE --warnings-as-errors✅Structural invariants asserted:
centralManager.connect(_:options:)appears exactly once (insideissueConnect); exactly one productionUserDefaultswrite (persistHoldMap, reachable only fromsyncReconnectIntent).Review notes
A design review pass raised four P1 findings, all confirmed against the code and fixed in this branch: non-connected teardown paths not cancelling (so Tier-0 kept running),
performReconnectbypassing the radio/live-state gates, scan-waiter cancellation being global rather than waiter-specific, and — the subtlest —cachedSystemReconnect(id:)being evaluated aftersetConnectionState(.disconnected)had overwritten the state it reads, making that half of the teardown cancel predicate dead code.The fixes were mutation-tested rather than trusted: each production guard was deleted in turn and the protecting test had to fail. That process found three regression tests that were vacuous (passing with their guard removed) and they were rewritten to be falsifiable. Each teardown-cancel arm is now pinned independently.
Known limitations, deliberately carried:
applyManualDisconnect's live.connectingarm is behaviorally covered but not cancel-count pinned; the higher-risk cached arm is.🤖 Generated with Claude Code