From 5c201289fb6c44bb6b942eb470d430d8447bea4d Mon Sep 17 00:00:00 2001 From: rzen Date: Fri, 10 Jul 2026 11:32:52 -0400 Subject: [PATCH] Survive deleted-and-saved @Models on the started-run navigation path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestFlight 2.3 (125) "crashed when watch ended an exercise": the isDeleted guard from 85e1582 only covers the delete→save window. Once the deletion is saved the model unregisters — isDeleted reads false again, modelContext goes nil, and any persisted-property read still traps (_InitialBackingData.getValue). StartedWorkoutNavigator retained the run's @Model in @State for the whole workout, so an observer remove/re-add churn (e.g. iCloud reachability flapping) invalidated it underneath the pushed screen, and the watch's completion push triggered the rebuild that read it. Two layers: the fromLive/SplitDetailView guards now also require a non-nil modelContext, and StartedWorkoutNavigator pushes a plain id route, re-fetching the entity fresh on every destination build — a re-imported run resolves to its live instance; a gone run shows a placeholder instead of trapping. Claude-Session: https://claude.ai/code/session_01PVNBVKp5bcq52X722uMjwT --- CHANGELOG.md | 2 ++ Shared/Model/Mappers.swift | 8 +++++- Workouts/Views/Splits/SplitDetailView.swift | 10 ++++--- .../Views/WorkoutLogs/WorkoutLogsView.swift | 27 +++++++++++++++---- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c78e9f7..0556ef5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ **July 2026** +Finishing an exercise from the watch no longer risks crashing the workout screen on the iPhone. + Deleting a workout no longer crashes the app. While you rest between exercises, the workout screen now previews the next exercise's figure and name so you can see what's coming up. diff --git a/Shared/Model/Mappers.swift b/Shared/Model/Mappers.swift index 319ed44..ade5a21 100644 --- a/Shared/Model/Mappers.swift +++ b/Shared/Model/Mappers.swift @@ -62,8 +62,14 @@ extension WorkoutDocument { /// map — eagerly for *every* row in the parent list, including during the view-graph /// update that fires the instant a workout is deleted. Any map of an entity not /// already known to be live must go through this guard. + /// + /// `isDeleted` alone is not enough: it is true only between `context.delete()` and + /// `context.save()`. Once the deletion is saved the model becomes *unregistered* — + /// `isDeleted` reads false again, `modelContext` goes nil, and any persisted-property + /// read still traps. A `@Model` retained across time (not freshly fetched) can reach + /// this map in that state, so check both. init?(fromLive workout: Workout) { - guard !workout.isDeleted else { return nil } + guard !workout.isDeleted, workout.modelContext != nil else { return nil } self.init(from: workout) } diff --git a/Workouts/Views/Splits/SplitDetailView.swift b/Workouts/Views/Splits/SplitDetailView.swift index b3f5076..d095525 100644 --- a/Workouts/Views/Splits/SplitDetailView.swift +++ b/Workouts/Views/Splits/SplitDetailView.swift @@ -32,9 +32,13 @@ struct SplitDetailView: View { // A closure-based `NavigationLink` builds this destination eagerly for every // row in the parent list, including during the update that fires when a split // is deleted — and reading any persisted property (even `id`) on a deleted - // `@Model` traps. An empty id maps to no live split, so `body` shows the - // "Split Unavailable" state and dismisses; the row is on its way out anyway. - _splitID = State(initialValue: split.isDeleted ? "" : split.id) + // `@Model` traps. `isDeleted` alone misses a deletion that has already been + // saved (the model unregisters: `isDeleted` false again, `modelContext` nil, + // reads still trap), so check both. An empty id maps to no live split, so + // `body` shows the "Split Unavailable" state and dismisses; the row is on its + // way out anyway. + let live = !split.isDeleted && split.modelContext != nil + _splitID = State(initialValue: live ? split.id : "") } private var split: Split? { diff --git a/Workouts/Views/WorkoutLogs/WorkoutLogsView.swift b/Workouts/Views/WorkoutLogs/WorkoutLogsView.swift index ae20568..855f9bd 100644 --- a/Workouts/Views/WorkoutLogs/WorkoutLogsView.swift +++ b/Workouts/Views/WorkoutLogs/WorkoutLogsView.swift @@ -322,14 +322,31 @@ struct SplitPickerSheet: View { /// file→observer→cache loop round-trips, so we poll the cache for the entity and push it /// the moment it lands. private struct StartedWorkoutNavigator: ViewModifier { + /// The pushed state is a plain value wrapping the run's id — never a retained + /// `@Model`. This screen stays pushed for the whole workout, and the run's entity + /// can be deleted and re-imported underneath it (observer remove/re-add churn, a + /// remote delete); a `@Model` held in `@State` would be *invalidated* by then — + /// deleted-and-saved models read `isDeleted == false` yet trap on any property + /// access (the TestFlight 2.3 (125) "crashed when watch ended an exercise" report). + private struct RunRoute: Identifiable, Hashable { let id: String } + @Environment(\.modelContext) private var modelContext @Binding var pendingWorkoutID: String? - @State private var resolvedWorkout: Workout? + @State private var resolvedRun: RunRoute? func body(content: Content) -> some View { content - .navigationDestination(item: $resolvedWorkout) { workout in - WorkoutLogListView(workout: workout) + .navigationDestination(item: $resolvedRun) { route in + // Re-fetch on every build so the destination always maps a live + // entity; a re-imported run resolves to its fresh instance. + if let workout = CacheMapper.fetchWorkout(id: route.id, in: modelContext) { + WorkoutLogListView(workout: workout) + } else { + ContentUnavailableView( + "Workout Unavailable", + systemImage: "xmark.circle", + description: Text("This workout is no longer on this device.")) + } } .onChange(of: pendingWorkoutID) { _, id in guard let id else { return } @@ -344,8 +361,8 @@ private struct StartedWorkoutNavigator: ViewModifier { Task { for _ in 0..<20 { try? await Task.sleep(for: .milliseconds(150)) - if let workout = CacheMapper.fetchWorkout(id: id, in: modelContext) { - resolvedWorkout = workout + if CacheMapper.fetchWorkout(id: id, in: modelContext) != nil { + resolvedRun = RunRoute(id: id) pendingWorkoutID = nil return }