Integrate durable zone event delivery and beacon lifecycle handling - 4

[Pixelobserver]

AI Policy

• I have read the Open Home Foundation AI Policy.

Select exactly one:

• I have not used AI for this contribution.
• AI assistance was used for this contribution.
• AI fully generated the code for this contribution, but I've reviewed and understood it before submitting and will respond without AI during review.

Summary

This is part 4 of the split requested for #5629.

Integrate ZoneManager with the atomic event outbox, persistent background webhook transport and beacon scanning lifecycle.

• Persist events and delivery-start markers before starting uploads.
• Drain queued events in order and retain stable identifiers across retries.
• Reconcile restored uploads and their success, failure or absent-task results.
• Avoid uploading events that expire while their delivery marker is written.
• Resume pending delivery on application and location wakes.
• Acquire the next beacon scanning owner before releasing the previous owner.
• Add integration tests for persistence, recovery, storage failures, expiration and scan handoffs.

This does not guarantee exactly-once delivery across server processing, network failures and app termination.

Dependencies and scope

Depends on:

• Add persistent background webhook uploads - 1 #5703 — Persistent background webhooks
• Add an atomic zone event outbox - 2  #5730 — Atomic zone event outbox
• Improve bounded iBeacon ranging and region reconciliation - 3 #5735 — Bounded iBeacon ranging

This branch contains only ZoneManager integration and its tests: two files, based directly on main. It does not include the foundation implementations or their commits.

It is not independently buildable without those foundations. Please merge the foundation PRs first, then update and verify this branch before merging part 4.

Screenshots

N/A — no visual changes or new settings.

Link to pull request in Documentation repository

Pending. Documentation should describe delivery/retry behavior and its limitations.

Any other notes

Completed local checks:

• Full SwiftFormat 0.53.1 check.
• Two-file scope and whitespace checks.
• Test target and scheme membership inspection.

Pending verification:

• Xcode compilation and XCTest execution.
• Full lint, including SwiftLint.
• Measured patch coverage of at least 90%.
• Real-device background delivery and beacon transition testing.

No successful Xcode test run or measured coverage is claimed for this revision.

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate delivery and lifecycle issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Integrates ZoneManager with durable zone-event delivery and beacon scanning lifecycle handling.

Changes:

• Adds ordered outbox persistence, retries, reconciliation, and stable event IDs.
• Handles application/location wakes and beacon scan handoffs.
• Expands integration coverage for persistence, recovery, failures, and expiration.

File summaries

File	Review summary
Sources/App/ZoneManager/ZoneManager.swift	Durable delivery and scan lifecycle integration. Findings: one critical issue (1 vote) and three moderate issues (2, 1, and 1 votes) involving API resolution, delivery flushing, retry cancellation, and main-thread location access.
Tests/App/ZoneManager/ZoneManager.test.swift	Integration and recovery tests. One nit (1 vote): rename a test to reflect ordering rather than coalescing behavior.

Review details

Suppressed comments (3)

Sources/App/ZoneManager/ZoneManager.swift:481

• DispatchWorkItem.cancel() is cooperative, so a retry block canceled by attach or by an empty flush can still execute. This block then unconditionally clears whichever retry item has since been installed and flushes immediately, allowing a fast subsequent failure to lose its new backoff timer and retry repeatedly. Gate the closure with an identity/generation check before clearing and flushing.

        let workItem = DispatchWorkItem { [weak self] in
            guard let self else { return }
            zoneEventRetryWorkItem = nil
            flushPendingZoneEvents()

Sources/App/ZoneManager/ZoneManager.swift:101

• This reads locationManager.monitoredRegions on the main thread, even though syncNow documents that this property performs synchronous locationd/XPC work and must be read off-main. A slow location daemon can therefore reintroduce the field hang this change is intended to avoid. Move this read into the existing off-main sync path (and update the lifecycle test to wait for the asynchronous handoff).

        collector.startForegroundBeaconScanning(
            in: locationManager.monitoredRegions,
            manager: locationManager
        )
        collector.stopBackgroundBeaconMonitoring(manager: locationManager)

Tests/App/ZoneManager/ZoneManager.test.swift:1182

• This test does not exercise coalescing: the outbox coalesces adjacent unstarted beacon events with the same event type, while this test queues ios.zone_entered followed by ios.zone_exited and expects both events to be delivered. Rename it so the test name describes the ordering behavior it actually covers.

    func testCoalescedBeaconExitWaitsForInFlightEntry() async throws {

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Disabled-location recovery, orphaned started events, and main-thread Core Location reads can stall delivery or app lifecycle handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

Sources/App/ZoneManager/ZoneManager.swift:95

• Flush the durable outbox before this location-source guard. If tracking is disabled after a delivery fails and the app is then suspended, the retry timer may not run; subsequent foreground wakes return here without retrying, and location wakes have also been disabled. This leaves queued work stuck until process recreation, contrary to the wake-recovery behavior.
  Sources/App/ZoneManager/ZoneManager.swift:99
• This reads CLLocationManager.monitoredRegions on the main-thread app lifecycle callback. The existing syncNow documentation at lines 535-537 says this getter performs synchronous XPC and was moved off-main because it caused the app's top field hang. Cache the synchronized region set or fetch it on regionSyncQueue, then perform only the collector handoff on main.

This issue also appears on line 109 of the same file.

Sources/App/ZoneManager/ZoneManager.swift:111

• This second lifecycle path also evaluates monitoredRegions on the main thread, reintroducing the synchronous locationd XPC hang called out at lines 535-537. Use the same off-main/cached region snapshot as the foreground handoff and keep only collector mutation on main.

        collector.startBackgroundBeaconMonitoring(
            in: locationManager.monitoredRegions,
            manager: locationManager

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced