From 57e3765a37e836e0546ccb68cb12b507130ca414 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Wed, 15 Jul 2026 15:47:11 -0700 Subject: [PATCH 1/2] Log today's location on app open when the day is blank Opening the Where app early in the day could show a blank calendar day because passive tracking (Visits + significant-change) hadn't fired yet. On launch and foreground, take a best-effort one-shot GPS fix and persist it through the normal ingest path when today has no GPS sample. - LocationAuthorizationStatus.allowsForegroundFix (When-In-Use or Always) - LocationIngestor.captureTodayIfNeeded(now:): non-blocking single-flight capture that skips when a GPS sample already exists today (a manual entry doesn't count), persists via the shared ingest path (widget + reminder reconcile + change signal), cancelled on deinit/quiesce - WhereSession.captureTodayIfNeeded(): gated on wantsTracking + a usable authorization; called from start() and appBecameActive() - WhereLaunch: foreground-only capture-today step after reconcile-tracking so a headless background relaunch doesn't fire a foreground fix Co-authored-by: Cursor --- Where/AGENTS.md | 6 +- Where/WhereCore/AGENTS.md | 4 +- .../LocationAuthorizationStatus.swift | 7 ++ .../Sources/Location/LocationIngestor.swift | 60 ++++++++++++++ .../Sources/Location/LocationSample.swift | 10 +++ .../Sources/Location/LocationSource.swift | 12 +-- .../Tests/LocationAuthorizationTests.swift | 8 ++ .../Tests/LocationIngestorTests.swift | 72 ++++++++++++++++ .../WhereUI/Sources/Launch/WhereLaunch.swift | 11 +++ .../WhereUI/Sources/Model/WhereSession.swift | 15 ++++ Where/WhereUI/Tests/WhereLaunchTests.swift | 82 +++++++++++++++++++ .../Tests/WhereSessionTrackingTests.swift | 80 +++++++++++++++++- 12 files changed, 356 insertions(+), 11 deletions(-) diff --git a/Where/AGENTS.md b/Where/AGENTS.md index 5d8a8a0b..3464cbb1 100644 --- a/Where/AGENTS.md +++ b/Where/AGENTS.md @@ -63,8 +63,10 @@ Rules the code enforces and agents must preserve: - **Location comes through the `LocationSource` protocol** — production is `CoreLocationSource`; tests and previews use `ScriptedLocationSource`. Besides the passive `sampleStream`, it offers a best-effort one-shot - `requestCurrentLocation()`, used to stamp manual entries; it returns `nil` - rather than throwing when no fix is available. + `requestCurrentLocation()` — used both to stamp manual entries and to log + today when the app is opened on a day with no GPS sample yet + (`LocationIngestor.captureTodayIfNeeded`, driven by `WhereSession` on launch/ + foreground); it returns `nil` rather than throwing when no fix is available. - **Manual entries carry a `ManualEntryAudit`** (when made, an optional note, and a best-effort capture-time `CapturedLocation`). The view-model intents assemble it; `DayJournal`'s write methods take an explicit `audit:` (no diff --git a/Where/WhereCore/AGENTS.md b/Where/WhereCore/AGENTS.md index 76b50ceb..1941b606 100644 --- a/Where/WhereCore/AGENTS.md +++ b/Where/WhereCore/AGENTS.md @@ -81,7 +81,9 @@ internal shape. - **`LocationSource` abstracts GPS.** Production is `CoreLocationSource` (Visits + significant-change); tests/previews use `ScriptedLocationSource`. The one-shot `requestCurrentLocation()` returns `nil` (never throws) when no fix - is available — it only stamps a manual entry's audit trail. + is available — it stamps a manual entry's audit trail and backs + `LocationIngestor.captureTodayIfNeeded(now:)`, which persists a fix for today + (via the normal ingest path) when the app opens on a day with no GPS sample. - **Impossible states trap; recoverable ones surface.** `WhereStore` methods are `async throws` so the CloudKit-backed store can report I/O failure; a `catch` must log via `WhereLog.channel(_:)` (typed `Category`, PII-free) and leave diff --git a/Where/WhereCore/Sources/Location/LocationAuthorizationStatus.swift b/Where/WhereCore/Sources/Location/LocationAuthorizationStatus.swift index e3c3d736..78133984 100644 --- a/Where/WhereCore/Sources/Location/LocationAuthorizationStatus.swift +++ b/Where/WhereCore/Sources/Location/LocationAuthorizationStatus.swift @@ -23,6 +23,13 @@ public enum LocationAuthorizationStatus: Sendable, Hashable { self == .always } + /// Whether a one-shot foreground fix (`requestCurrentLocation()`) can + /// succeed: any granted status, including When-In-Use. Broader than + /// `allowsBackgroundTracking` because a foreground fix doesn't need Always. + public var allowsForegroundFix: Bool { + self == .whenInUse || self == .always + } + /// Whether the user has actively refused (vs. simply not been asked), so /// the UI can route to the Settings app instead of re-prompting. public var isDenied: Bool { diff --git a/Where/WhereCore/Sources/Location/LocationIngestor.swift b/Where/WhereCore/Sources/Location/LocationIngestor.swift index 5f05f827..4a2adfdc 100644 --- a/Where/WhereCore/Sources/Location/LocationIngestor.swift +++ b/Where/WhereCore/Sources/Location/LocationIngestor.swift @@ -38,6 +38,12 @@ public actor LocationIngestor { private var ingestTask: Task? + /// The in-flight one-shot capture spawned by `captureTodayIfNeeded(now:)`, + /// if any. Tracked so overlapping foreground / launch triggers coalesce onto + /// a single fix (single-flight) and so teardown can cancel it. Cleared when + /// the work completes. + private var captureTask: Task? + /// The persist the stream loop is currently awaiting, if any. Tracked so /// `quiesce()` can wait for an in-flight write to commit before a teardown /// wipes the store — gating alone can't, since a persist that already @@ -91,6 +97,7 @@ public actor LocationIngestor { deinit { ingestTask?.cancel() + captureTask?.cancel() } /// Begin (or resume) GPS ingestion. Idempotent: a second call while @@ -165,6 +172,10 @@ public actor LocationIngestor { public func quiesce() async { acceptsSamples = false isMonitoring = false + // Cancel any in-flight one-shot capture; even if its fix still lands + // after this, the `acceptsSamples` gate in `ingest(_:)` drops it. + captureTask?.cancel() + captureTask = nil await locationSource.stop() // Let an already-started persist (and any retry re-enqueue it performs) // settle first, then clear the backlog — so nothing re-adds after. @@ -201,6 +212,55 @@ public actor LocationIngestor { await locationSource.requestCurrentLocation() } + /// Fill in *today* with a best-effort one-shot GPS fix when the day has no + /// GPS sample yet, so opening the app on a fresh day doesn't leave the + /// calendar blank until passive monitoring (Visits / significant-change) + /// next fires. Non-blocking: the fix (which can take up to ~10s) runs on an + /// internal task so launch / foreground callers aren't held up — the + /// persisted sample refreshes readers via the store's change signal. + /// + /// Single-flight: a call while a capture is already in flight is a no-op. + /// Only a *GPS* sample suppresses the fix; a manual entry for today doesn't, + /// since it isn't a passive-tracking data point. Whether to attempt this at + /// all (the user's tracking intent + authorization) is the caller's gate; + /// this stays safe regardless because `requestCurrentLocation()` returns + /// `nil` when no fix is available. + public func captureTodayIfNeeded(now: Date) { + guard captureTask == nil else { return } + captureTask = Task { [weak self] in + await self?.performTodayCapture(now: now) + await self?.clearCaptureTask() + } + } + + private func clearCaptureTask() { + captureTask = nil + } + + /// The body of `captureTodayIfNeeded(now:)`, run on `captureTask`. + private func performTodayCapture(now: Date) async { + let startOfDay = calendar.startOfDay(for: now) + guard let endOfDay = calendar.date(byAdding: .day, value: 1, to: startOfDay) else { + Self.logger.warning("Could not compute today's interval for foreground capture") + return + } + let interval = DateInterval(start: startOfDay, end: endOfDay) + do { + let existing = try await store.samples(in: interval) + if existing.contains(where: \.source.isGPS) { return } + } catch { + // Fail closed: if today's samples can't be read we skip rather than + // risk logging a duplicate fix. Surfaced, not silently swallowed. + Self.logger.warning( + "Skipping foreground capture; could not read today's samples: \(error.localizedDescription)", + ) + return + } + guard let sample = await locationSource.requestCurrentLocation() else { return } + Self.logger.info("Captured one-shot foreground location for today") + await ingest(sample) + } + /// The current location authorization status. public func authorizationStatus() async -> LocationAuthorizationStatus { await locationSource.currentAuthorization() diff --git a/Where/WhereCore/Sources/Location/LocationSample.swift b/Where/WhereCore/Sources/Location/LocationSample.swift index 9240c0a2..b3776b7a 100644 --- a/Where/WhereCore/Sources/Location/LocationSample.swift +++ b/Where/WhereCore/Sources/Location/LocationSample.swift @@ -31,6 +31,16 @@ public enum SampleSource: Sendable, Hashable, Codable { } } + /// Whether this sample came from GPS (passive Visits / significant-change, + /// or a one-shot foreground fix) rather than user-asserted manual or + /// evidence-derived data. Exhaustive so a new case forces a decision here. + public var isGPS: Bool { + switch self { + case .gpsVisit, .gpsSignificantChange: true + case .manual, .evidenceImplied: false + } + } + /// `.evidenceImplied` only — the originating evidence's UUID. Returns /// `nil` for the other cases, so SwiftData can skip writing the column. public var evidenceId: UUID? { diff --git a/Where/WhereCore/Sources/Location/LocationSource.swift b/Where/WhereCore/Sources/Location/LocationSource.swift index 0ad422b0..09114060 100644 --- a/Where/WhereCore/Sources/Location/LocationSource.swift +++ b/Where/WhereCore/Sources/Location/LocationSource.swift @@ -43,11 +43,13 @@ public protocol LocationSource: AnyObject, Sendable { /// Best-effort one-shot GPS fix for "where is the device *right now*". /// /// Unlike the passive `sampleStream` (Visits + significant-change, which can - /// be minutes stale), this actively asks for a fresh fix — used to stamp a - /// manual entry's audit trail with where it was made. Returns `nil` rather - /// than throwing when a fix can't be obtained (permission not granted, - /// timeout, or a location error): the capture is audit metadata, so an - /// absent fix is recorded honestly instead of blocking the entry. + /// be minutes stale), this actively asks for a fresh fix. Two callers use + /// it: stamping a manual entry's audit trail with where it was made, and + /// `LocationIngestor.captureTodayIfNeeded(now:)`, which persists a fix for + /// today when the app opens on a day that has no GPS sample yet. Returns + /// `nil` rather than throwing when a fix can't be obtained (permission not + /// granted, timeout, or a location error), so an absent fix is recorded + /// honestly instead of blocking the caller. func requestCurrentLocation() async -> LocationSample? /// The current authorization status, read on demand. diff --git a/Where/WhereCore/Tests/LocationAuthorizationTests.swift b/Where/WhereCore/Tests/LocationAuthorizationTests.swift index 9c9e7a4c..94cda5e0 100644 --- a/Where/WhereCore/Tests/LocationAuthorizationTests.swift +++ b/Where/WhereCore/Tests/LocationAuthorizationTests.swift @@ -36,6 +36,14 @@ struct LocationAuthorizationTests { #expect(!LocationAuthorizationStatus.denied.allowsBackgroundTracking) } + @Test func allowsForegroundFixForAnyGrantedStatus() { + #expect(LocationAuthorizationStatus.always.allowsForegroundFix) + #expect(LocationAuthorizationStatus.whenInUse.allowsForegroundFix) + #expect(!LocationAuthorizationStatus.notDetermined.allowsForegroundFix) + #expect(!LocationAuthorizationStatus.denied.allowsForegroundFix) + #expect(!LocationAuthorizationStatus.restricted.allowsForegroundFix) + } + @Test func isDeniedCoversDeniedAndRestricted() { #expect(LocationAuthorizationStatus.denied.isDenied) #expect(LocationAuthorizationStatus.restricted.isDenied) diff --git a/Where/WhereCore/Tests/LocationIngestorTests.swift b/Where/WhereCore/Tests/LocationIngestorTests.swift index 44adeb15..7c33a4b0 100644 --- a/Where/WhereCore/Tests/LocationIngestorTests.swift +++ b/Where/WhereCore/Tests/LocationIngestorTests.swift @@ -99,6 +99,78 @@ struct LocationIngestorTests { #expect(await ingestor.currentLocation() == nil) } + @Test func captureTodayPersistsAndReportsFixWhenNoGPSSampleYet() async throws { + let store = try SwiftDataStore.inMemory() + let source = ScriptedLocationSource(authorizationStatus: .whenInUse) + let recorder = OutcomeRecorder() + let ingestor = Self.makeIngestor(store: store, source: source, recorder: recorder) + source.setNextRequestedLocation(sample(at: "2026-03-15T08:05:00-07:00")) + + // No monitoring started (the When-In-Use case): the foreground fix is + // the only way this user's data lands, and it still persists + reports. + await ingestor + .captureTodayIfNeeded(now: WhereCoreTestSupport.iso("2026-03-15T08:00:00-07:00")) + + try await waitUntil { await (try? store.allSamples().count) == 1 } + #expect(await recorder.last?.liveSample != nil) + } + + @Test func captureTodaySkipsWhenGPSSampleAlreadyExistsToday() async throws { + let store = try SwiftDataStore.inMemory() + let source = ScriptedLocationSource(authorizationStatus: .whenInUse) + let recorder = OutcomeRecorder() + let ingestor = Self.makeIngestor(store: store, source: source, recorder: recorder) + // A passive GPS sample already covers today. + try await store + .perform { try await store.add(sample: sample(at: "2026-03-15T02:00:00-07:00")) } + source.setNextRequestedLocation(sample(at: "2026-03-15T08:05:00-07:00")) + + await ingestor + .captureTodayIfNeeded(now: WhereCoreTestSupport.iso("2026-03-15T08:00:00-07:00")) + + // The day is already covered, so no fix is taken. Give the capture task + // time to run and confirm it added nothing. + try await Task.sleep(for: .milliseconds(100)) + #expect(try await store.allSamples().count == 1) + } + + @Test func captureTodayStillCapturesWhenOnlyManualSampleExistsToday() async throws { + let store = try SwiftDataStore.inMemory() + let source = ScriptedLocationSource(authorizationStatus: .whenInUse) + let recorder = OutcomeRecorder() + let ingestor = Self.makeIngestor(store: store, source: source, recorder: recorder) + // A manual entry isn't a passive-tracking data point, so it must not + // suppress the GPS fix. + try await store.perform { + try await store.add(sample: LocationSample( + timestamp: WhereCoreTestSupport.iso("2026-03-15T02:00:00-07:00"), + coordinate: Coordinate(latitude: 37.7749, longitude: -122.4194), + horizontalAccuracy: 0, + source: .manual, + )) + } + source.setNextRequestedLocation(sample(at: "2026-03-15T08:05:00-07:00")) + + await ingestor + .captureTodayIfNeeded(now: WhereCoreTestSupport.iso("2026-03-15T08:00:00-07:00")) + + try await waitUntil { await (try? store.allSamples().count) == 2 } + } + + @Test func captureTodaySkipsWhenSourceHasNoFix() async throws { + let store = try SwiftDataStore.inMemory() + let source = ScriptedLocationSource(authorizationStatus: .whenInUse) + let recorder = OutcomeRecorder() + let ingestor = Self.makeIngestor(store: store, source: source, recorder: recorder) + // No fix scripted → `requestCurrentLocation()` returns nil. + + await ingestor + .captureTodayIfNeeded(now: WhereCoreTestSupport.iso("2026-03-15T08:00:00-07:00")) + + try await Task.sleep(for: .milliseconds(100)) + #expect(try await store.allSamples().isEmpty) + } + @Test func liveSampleIsPersistedAndReported() async throws { let store = try SwiftDataStore.inMemory() let source = ScriptedLocationSource(authorizationStatus: .always) diff --git a/Where/WhereUI/Sources/Launch/WhereLaunch.swift b/Where/WhereUI/Sources/Launch/WhereLaunch.swift index 81e1e921..4dcc46c5 100644 --- a/Where/WhereUI/Sources/Launch/WhereLaunch.swift +++ b/Where/WhereUI/Sources/Launch/WhereLaunch.swift @@ -24,6 +24,10 @@ public enum LaunchStepID: String { case syncAuth = "sync-auth" /// Start or stop GPS ingestion to match the user's intent + authorization. case reconcileTracking = "reconcile-tracking" + /// Take a one-shot GPS fix for today if none is logged yet, so opening the + /// app on a fresh day fills the calendar in. Foreground-only — a headless + /// background relaunch is itself the passive event, so it needs no fix. + case captureToday = "capture-today" /// Push the logging-reminder schedule + badge (backlog + issue count) to the /// reconciler. case reminders @@ -148,6 +152,13 @@ public enum WhereLaunch { LifecycleStep.work(LaunchStepID.reconcileTracking) { _ in await model.session?.reconcileTracking() } + // Foreground-only: a headless background relaunch is itself the + // passive location event, so it neither needs nor should trigger a + // fresh foreground fix. Returns fast (the ingestor spawns the ~10s + // fix internally), so it never delays reaching `.ready`. + LifecycleStep.work(LaunchStepID.captureToday, modes: .foreground) { _ in + await model.session?.captureTodayIfNeeded() + } LifecycleStep.work(LaunchStepID.reminders) { _ in await model.session?.applyReminderConfiguration() } diff --git a/Where/WhereUI/Sources/Model/WhereSession.swift b/Where/WhereUI/Sources/Model/WhereSession.swift index e3645f44..11619564 100644 --- a/Where/WhereUI/Sources/Model/WhereSession.swift +++ b/Where/WhereUI/Sources/Model/WhereSession.swift @@ -150,6 +150,7 @@ public final class WhereSession { await syncAuthorization() observeAuthorizationChanges() await reconcileTracking() + await captureTodayIfNeeded() await applyReminderConfiguration() await applySummaryConfiguration() await applyIssueAlertConfiguration() @@ -166,6 +167,7 @@ public final class WhereSession { public func appBecameActive() async { await syncAuthorization() await reconcileTracking() + await captureTodayIfNeeded() await applyReminderConfiguration() await applySummaryConfiguration() await applyIssueAlertConfiguration() @@ -247,6 +249,19 @@ public final class WhereSession { } } + /// Fill in today with a one-shot GPS fix if the day has no GPS sample yet, + /// so opening the app on a fresh morning doesn't leave the calendar blank + /// until passive tracking next fires. Gated on the user's tracking intent + /// and a usable authorization (When-In-Use is enough for a foreground fix — + /// notably the only way When-In-Use users get any data). The ingestor is + /// non-blocking and reconciles widgets / reminders + pings the read signal + /// on persist. A launch step (see `WhereLaunch.sequence`); also runs on + /// every foreground. + func captureTodayIfNeeded() async { + guard wantsTracking, authorizationStatus.allowsForegroundFix else { return } + await services.ingestor.captureTodayIfNeeded(now: now()) + } + /// Explicitly (re)request location access, e.g. from the "Grant location /// access" button. Drives the system prompt when possible, then syncs the /// status and reconciles tracking so the UI reflects the outcome. diff --git a/Where/WhereUI/Tests/WhereLaunchTests.swift b/Where/WhereUI/Tests/WhereLaunchTests.swift index ef76c9b4..ca30ceda 100644 --- a/Where/WhereUI/Tests/WhereLaunchTests.swift +++ b/Where/WhereUI/Tests/WhereLaunchTests.swift @@ -1,5 +1,6 @@ import Foundation import LifecycleKit +import RegionKit import TestHostSupport import Testing @_spi(Testing) import WhereCore @@ -21,6 +22,20 @@ private func waitUntil( } } +/// Async-predicate variant of `waitUntil`, for polling actor-isolated state +/// (e.g. the store's sample count) that must be `await`ed. +@MainActor +private func waitUntilAsync( + timeout: Duration = .seconds(5), + _ predicate: () async -> Bool, +) async throws { + let deadline = ContinuousClock.now.advanced(by: timeout) + while await !predicate() { + if ContinuousClock.now >= deadline { throw WaitTimeout() } + try await Task.sleep(for: .milliseconds(1)) + } +} + /// Covers the `WhereLaunch` sequence the app drives at startup: the step order /// (parity with `WhereSession.start()`), the onboarding gate, and the headless /// background path. @@ -48,6 +63,35 @@ struct WhereLaunchTests { return WhereModel(services: services, preferences: preferences) } + /// Like `makeModel`, but returns the backing store and location source so a + /// test can script a one-shot fix and assert what the launch persisted. + private func makeModelAndStore( + status: LocationAuthorizationStatus = .always, + preferences: WherePreferences, + ) throws -> (WhereModel, SwiftDataStore, ScriptedLocationSource) { + let store = try SwiftDataStore.inMemory() + let source = ScriptedLocationSource(authorizationStatus: status) + let services = WhereServices( + store: store, + locationSource: source, + reminderScheduler: NoopLoggingReminderScheduler(), + summaryScheduler: NoopDailySummaryScheduler(), + issueAlertScheduler: NoopDataIssueAlertScheduler(), + widgetRefresher: NoopWidgetTimelineRefresher(), + ) + return (WhereModel(services: services, preferences: preferences), store, source) + } + + /// A one-shot fix stamped "now" so it lands on today's calendar day. + private func todayFix() -> LocationSample { + LocationSample( + timestamp: Date(), + coordinate: Coordinate(latitude: 37.7749, longitude: -122.4194), + horizontalAccuracy: 5, + source: .gpsSignificantChange, + ) + } + @Test func sequenceStepsRunInStartParityOrder() throws { // The work steps mirror WhereSession.start()'s order; the only insertion // is the onboarding gate. @@ -58,6 +102,7 @@ struct WhereLaunchTests { .onboarding, .syncAuth, .reconcileTracking, + .captureToday, .reminders, .summary, .issueAlerts, @@ -76,6 +121,43 @@ struct WhereLaunchTests { #expect(model.session?.isTracking == true) } + @Test func coldForegroundLaunchCapturesTodayWhenEmpty() async throws { + let (model, store, source) = try makeModelAndStore( + status: .always, + preferences: makePreferences(), + ) + model.completeOnboarding() + source.setNextRequestedLocation(todayFix()) + + let launcher = WhereLaunch.makeLauncher(model: model, reason: .userForeground) + await launcher.run() + #expect(launcher.phase.isReady) + + // The capture-today step logged today's fix (non-blocking, so wait for + // the persist to land). + try await waitUntilAsync { await (try? store.allSamples().count) == 1 } + } + + @Test func backgroundLaunchSkipsCaptureToday() async throws { + // The capture-today step is foreground-only: a headless background + // relaunch is itself the passive location event, so it must not fire a + // fresh foreground fix even though authorization/intent would allow one. + let (model, store, source) = try makeModelAndStore( + status: .always, + preferences: makePreferences(), + ) + model.completeOnboarding() + source.setNextRequestedLocation(todayFix()) + + let launcher = WhereLaunch.makeLauncher(model: model, reason: .background(.location)) + await launcher.run() + #expect(launcher.phase.isReady) + + // Skipped → nothing captured. (reconcile-tracking started monitoring but + // the scripted source emits no passive samples on its own.) + #expect(try await store.allSamples().isEmpty) + } + @Test func firstRunForegroundLaunchPresentsOnboarding() async throws { let model = try makeModel(status: .notDetermined, preferences: makePreferences()) #expect(!model.hasOnboarded) diff --git a/Where/WhereUI/Tests/WhereSessionTrackingTests.swift b/Where/WhereUI/Tests/WhereSessionTrackingTests.swift index 45b240c1..4f07fbce 100644 --- a/Where/WhereUI/Tests/WhereSessionTrackingTests.swift +++ b/Where/WhereUI/Tests/WhereSessionTrackingTests.swift @@ -1,4 +1,5 @@ import Foundation +import RegionKit import TestHostSupport import Testing @_spi(Testing) import WhereCore @@ -17,9 +18,18 @@ struct WhereSessionTrackingTests { status: LocationAuthorizationStatus, preferences: WherePreferences, ) throws -> (WhereSession, ScriptedLocationSource) { + let (session, source, _) = try makeSessionAndStore(status: status, preferences: preferences) + return (session, source) + } + + private func makeSessionAndStore( + status: LocationAuthorizationStatus, + preferences: WherePreferences, + ) throws -> (WhereSession, ScriptedLocationSource, SwiftDataStore) { + let store = try SwiftDataStore.inMemory() let source = ScriptedLocationSource(authorizationStatus: status) - let services = try WhereServices( - store: SwiftDataStore.inMemory(), + let services = WhereServices( + store: store, locationSource: source, reminderScheduler: NoopLoggingReminderScheduler(), summaryScheduler: NoopDailySummaryScheduler(), @@ -27,7 +37,18 @@ struct WhereSessionTrackingTests { widgetRefresher: NoopWidgetTimelineRefresher(), ) let session = WhereSession(services: services, preferences: preferences) - return (session, source) + return (session, source, store) + } + + /// A one-shot fix stamped "now", so it lands on today's calendar day + /// regardless of when the test runs. + private func todayFix() -> LocationSample { + LocationSample( + timestamp: Date(), + coordinate: Coordinate(latitude: 37.7749, longitude: -122.4194), + horizontalAccuracy: 5, + source: .gpsSignificantChange, + ) } @Test func launchWithAlwaysResumesTracking() async throws { @@ -86,6 +107,47 @@ struct WhereSessionTrackingTests { #expect(session.isTracking) } + @Test func foregroundLogsTodayWhenWantedAndAuthorized() async throws { + // When-In-Use is enough for a foreground fix — and the only way such a + // user gets any data, since passive background tracking needs Always. + let (session, source, store) = try makeSessionAndStore( + status: .whenInUse, + preferences: makePreferences(), + ) + source.setNextRequestedLocation(todayFix()) + + await session.appBecameActive() + + await waitUntilAsync { await (try? store.allSamples().count) == 1 } + } + + @Test func foregroundDoesNotLogTodayWhenTrackingDisabled() async throws { + let (session, source, store) = try makeSessionAndStore( + status: .always, + preferences: makePreferences(), + ) + // The user turned tracking off; opening the app must not silently log. + await session.stopTracking() + source.setNextRequestedLocation(todayFix()) + + await session.appBecameActive() + + // Gating short-circuits before any capture task is spawned. + #expect(try await store.allSamples().isEmpty) + } + + @Test func foregroundDoesNotLogTodayWhenUnauthorized() async throws { + let (session, source, store) = try makeSessionAndStore( + status: .denied, + preferences: makePreferences(), + ) + source.setNextRequestedLocation(todayFix()) + + await session.appBecameActive() + + #expect(try await store.allSamples().isEmpty) + } + /// Guards against a retain cycle through the long-lived authorization /// observer: it captures `[weak self]` and `deinit` cancels it, so dropping /// the last strong reference must deallocate the session even while the @@ -115,4 +177,16 @@ struct WhereSessionTrackingTests { } #expect(predicate(), "condition was not met before timeout") } + + private func waitUntilAsync( + timeout: Duration = .seconds(2), + _ predicate: () async -> Bool, + ) async { + let deadline = ContinuousClock.now.advanced(by: timeout) + while ContinuousClock.now < deadline { + if await predicate() { return } + try? await Task.sleep(for: .milliseconds(5)) + } + #expect(await predicate(), "condition was not met before timeout") + } } From 4973a73ade06e83bac3590b9fb2d7564935b88e8 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Wed, 15 Jul 2026 16:10:34 -0700 Subject: [PATCH 2/2] Track capture persist on its own handle; test single-flight Address a concurrency concern from review: routing the foreground capture through ingest(_:) made it a second writer of the single inFlightIngest slot the stream loop uses, which quiesce() relies on to await an in-flight persist before an erase wipes the store. - Capture now persists via processIngestedSample on its own capturePersistTask handle (single writer, since capture is single-flight), re-checking acceptsSamples synchronously right before registering the handle so quiesce() either skips it or awaits it, never neither. - quiesce() cancels the capture's fix acquisition (best-effort, not awaited so an erase never stalls on a slow ~10s GPS fix) and awaits the capture persist and the stream persist on their independent handles. - Add a single-flight test using a gated location source that holds a fix in flight, proving an overlapping second call is dropped. Co-authored-by: Cursor --- .../Sources/Location/LocationIngestor.swift | 43 ++++++++-- .../Tests/LocationIngestorTests.swift | 83 +++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/Where/WhereCore/Sources/Location/LocationIngestor.swift b/Where/WhereCore/Sources/Location/LocationIngestor.swift index 4a2adfdc..0eac7b73 100644 --- a/Where/WhereCore/Sources/Location/LocationIngestor.swift +++ b/Where/WhereCore/Sources/Location/LocationIngestor.swift @@ -41,9 +41,17 @@ public actor LocationIngestor { /// The in-flight one-shot capture spawned by `captureTodayIfNeeded(now:)`, /// if any. Tracked so overlapping foreground / launch triggers coalesce onto /// a single fix (single-flight) and so teardown can cancel it. Cleared when - /// the work completes. + /// the work completes. This spans the (slow, up to ~10s) fix acquisition, so + /// `quiesce()` cancels it but does *not* await it — see `capturePersistTask`. private var captureTask: Task? + /// The capture's *persist* step, once a fix is in hand — separate from + /// `captureTask` (which also covers the slow fix) so `quiesce()` can await a + /// commit already in progress without stalling on a slow GPS fix. A single + /// writer (capture is single-flight via `captureTask`), so it never clobbers + /// the stream loop's `inFlightIngest` the way a shared slot would. + private var capturePersistTask: Task? + /// The persist the stream loop is currently awaiting, if any. Tracked so /// `quiesce()` can wait for an in-flight write to commit before a teardown /// wipes the store — gating alone can't, since a persist that already @@ -98,6 +106,7 @@ public actor LocationIngestor { deinit { ingestTask?.cancel() captureTask?.cancel() + capturePersistTask?.cancel() } /// Begin (or resume) GPS ingestion. Idempotent: a second call while @@ -172,13 +181,18 @@ public actor LocationIngestor { public func quiesce() async { acceptsSamples = false isMonitoring = false - // Cancel any in-flight one-shot capture; even if its fix still lands - // after this, the `acceptsSamples` gate in `ingest(_:)` drops it. + await locationSource.stop() + // Cancel the one-shot capture's fix acquisition (best-effort — it isn't + // the store write, so we don't await it and never stall the erase on a + // slow GPS fix). If it was still acquiring, the `acceptsSamples` gate + // stops it persisting; if it had already begun a persist, that is tracked + // on `capturePersistTask` and awaited below. captureTask?.cancel() captureTask = nil - await locationSource.stop() - // Let an already-started persist (and any retry re-enqueue it performs) - // settle first, then clear the backlog — so nothing re-adds after. + // Let an already-started persist — the stream loop's and the capture's, + // each on its own single-writer handle — settle before clearing the + // backlog, so nothing commits into the store the caller is about to wipe. + await capturePersistTask?.value await inFlightIngest?.value retryQueue.removeAll() // Clear the durable mirror too; the store is about to be erased, so the @@ -257,8 +271,23 @@ public actor LocationIngestor { return } guard let sample = await locationSource.requestCurrentLocation() else { return } + // The ~10s fix may have straddled a `quiesce()`; re-check the gate before + // persisting, mirroring `ingest(_:)`. The guard and the `capturePersistTask` + // assignment are synchronous (no `await` between), so a concurrent + // `quiesce()` either sees `acceptsSamples == false` here (we skip) or sees + // the handle already set (it awaits us) — never neither. + guard acceptsSamples else { return } Self.logger.info("Captured one-shot foreground location for today") - await ingest(sample) + // Persist via `processIngestedSample` on the capture's own handle rather + // than `ingest(_:)`, so it never shares the stream loop's single + // `inFlightIngest` slot. `quiesce()` awaits this handle independently. + let work = Task { [weak self] in + guard let self else { return } + await processIngestedSample(sample) + } + capturePersistTask = work + await work.value + capturePersistTask = nil } /// The current location authorization status. diff --git a/Where/WhereCore/Tests/LocationIngestorTests.swift b/Where/WhereCore/Tests/LocationIngestorTests.swift index 7c33a4b0..da77eb18 100644 --- a/Where/WhereCore/Tests/LocationIngestorTests.swift +++ b/Where/WhereCore/Tests/LocationIngestorTests.swift @@ -171,6 +171,34 @@ struct LocationIngestorTests { #expect(try await store.allSamples().isEmpty) } + @Test func captureTodayIsSingleFlightWhileFixInFlight() async throws { + let store = try SwiftDataStore.inMemory() + let source = GatedLocationSource(fix: sample(at: "2026-03-15T08:05:00-07:00")) + let recorder = OutcomeRecorder() + let ingestor = LocationIngestor( + store: store, + locationSource: source, + calendar: WhereCoreTestSupport.calendar(), + onPersisted: { outcome in await recorder.record(outcome) }, + ) + let now = WhereCoreTestSupport.iso("2026-03-15T08:00:00-07:00") + + // The first capture parks awaiting the gated fix, holding the + // single-flight slot. + await ingestor.captureTodayIfNeeded(now: now) + try await waitUntil { source.requestCount == 1 } + + // A second call while the first is still in flight is dropped by the + // single-flight guard — it never requests a fix of its own. + await ingestor.captureTodayIfNeeded(now: now) + + // Release the fix: the first capture persists exactly one sample, and the + // second never asked the source for one. + source.openGate() + try await waitUntil { await (try? store.allSamples().count) == 1 } + #expect(source.requestCount == 1) + } + @Test func liveSampleIsPersistedAndReported() async throws { let store = try SwiftDataStore.inMemory() let source = ScriptedLocationSource(authorizationStatus: .always) @@ -409,6 +437,61 @@ struct LocationIngestorTests { private struct WaitTimeout: Error {} +/// `LocationSource` whose one-shot `requestCurrentLocation()` blocks until +/// `openGate()` is called, so a test can hold a capture "in flight" and prove +/// the single-flight guard drops an overlapping second request. Counts fix +/// requests so the test can assert the dropped call never reached the source. +private final class GatedLocationSource: LocationSource, @unchecked Sendable { + let sampleStream: AsyncStream + var authorizationUpdates: AsyncStream { + AsyncStream { _ in } + } + + private let fix: LocationSample + private let lock = NSLock() + private var waiters: [CheckedContinuation] = [] + private var _requestCount = 0 + + init(fix: LocationSample) { + self.fix = fix + sampleStream = AsyncStream { _ in } + } + + var requestCount: Int { + lock.withLock { _requestCount } + } + + func start() async {} + func stop() async {} + func currentAuthorization() async -> LocationAuthorizationStatus { + .whenInUse + } + + func requestPermission() async throws {} + + func requestCurrentLocation() async -> LocationSample? { + await withCheckedContinuation { continuation in + lock.withLock { + _requestCount += 1 + waiters.append(continuation) + } + } + return fix + } + + /// Resume every parked fix request with the scripted fix. + func openGate() { + let resumed = lock.withLock { + let current = waiters + waiters.removeAll() + return current + } + for continuation in resumed { + continuation.resume() + } + } +} + private struct ToggleFailingStoreError: Error {} /// `WhereStore` that lets a test toggle whether `add(sample:)` succeeds; every