From 7b6c39d8f0664ee58aab90b8cee2c5dfe9c9ad39 Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 12 Jul 2026 00:37:05 -0400 Subject: [PATCH] Guard every persisted read of retained workout models with isLive A remotely deleted @Model traps on any persisted-property read, including the observed expression of .onChange, which SwiftUI evaluates on every body pass. Guard those expressions, the onAppear takeover read, and the summary sheet's document mapping. Claude-Session: https://claude.ai/code/session_01PKptrgbx74peTwHGRxBojv --- .../Views/WorkoutLogListView.swift | 10 ++++---- Workouts/Views/WorkoutLogs/ExerciseView.swift | 18 ++++++++++----- .../WorkoutLogs/WorkoutLogListView.swift | 8 +++---- .../WorkoutLogs/WorkoutSummaryView.swift | 23 +++++++++++++------ 4 files changed, 38 insertions(+), 21 deletions(-) diff --git a/Workouts Watch App/Views/WorkoutLogListView.swift b/Workouts Watch App/Views/WorkoutLogListView.swift index 3d2080a..c299ff4 100644 --- a/Workouts Watch App/Views/WorkoutLogListView.swift +++ b/Workouts Watch App/Views/WorkoutLogListView.swift @@ -112,10 +112,12 @@ struct WorkoutLogListView: View { // ingesting our latest edit) must not regress what the user is looking // at. Clock skew across devices degrades this to a skipped absorb — // never to a clobber of local state. - .onChange(of: workout.updatedAt) { _, incoming in - // ...but never absorb from an entity a push has just deleted: mapping a - // deleted @Model traps. - if incoming > doc.updatedAt, let fresh = WorkoutDocument(fromLive: workout) { + .onChange(of: workout.isLive ? workout.updatedAt : nil) { _, incoming in + // ...and never absorb from an entity a push has just deleted: the observed + // expression above is evaluated on every body pass, so even it must be + // liveness-guarded — reading a persisted property on a dead @Model traps. + if let incoming, incoming > doc.updatedAt, + let fresh = WorkoutDocument(fromLive: workout) { doc = fresh } } diff --git a/Workouts/Views/WorkoutLogs/ExerciseView.swift b/Workouts/Views/WorkoutLogs/ExerciseView.swift index c9f5012..8aadf5d 100644 --- a/Workouts/Views/WorkoutLogs/ExerciseView.swift +++ b/Workouts/Views/WorkoutLogs/ExerciseView.swift @@ -71,24 +71,30 @@ struct ExerciseView: View { } .onAppear { refreshDocIfNeeded() - // Take over this run: the watch parks and locks it while we're editing here. - services.watchBridge.setEditingWorkout(workout.id) + // Take over this run: the watch parks and locks it while we're editing + // here. Guarded — even `id` is a persisted property and traps when read + // on an entity a remote delete has already reaped. + if workout.isLive { + services.watchBridge.setEditingWorkout(workout.id) + } } .onDisappear { services.watchBridge.setEditingWorkout(nil) } // Reflect external changes (e.g. a set completed on the watch) live. Each edit // rewrites the whole workout file, so the cache always holds the latest — pulling - // it in can't revert a newer local tap. - .onChange(of: workout.updatedAt) { _, _ in + // it in can't revert a newer local tap. The observed expression is evaluated on + // every body pass, so it must be liveness-guarded like any persisted read. + .onChange(of: workout.isLive ? workout.updatedAt : nil) { _, _ in absorbExternalUpdate() } } /// Pull an externally-changed workout into the working copy so the plan and notes - /// update without leaving and re-entering the screen. + /// update without leaving and re-entering the screen. Skipped if the entity died + /// (remote delete) — mapping a dead @Model traps. private func absorbExternalUpdate() { - doc = WorkoutDocument(from: workout) + if let fresh = WorkoutDocument(fromLive: workout) { doc = fresh } } @ViewBuilder diff --git a/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift b/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift index 9374596..c9ee76a 100644 --- a/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift +++ b/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift @@ -168,10 +168,10 @@ struct WorkoutLogListView: View { } .navigationTitle(doc.routineName ?? Routine.unnamed) // Absorb edits made in pushed children (ExerciseView/Plan/Notes) once the - // cache reflects them, so the list shows live status on return. - .onChange(of: workout.updatedAt) { _, _ in - // Skip if the entity was deleted out from under us (e.g. a remote delete - // while this screen is open) — mapping a deleted @Model traps. + // cache reflects them, so the list shows live status on return. The observed + // expression itself must be liveness-guarded too: it's evaluated on every + // body pass, and reading a persisted property on a dead @Model traps. + .onChange(of: workout.isLive ? workout.updatedAt : nil) { _, _ in if let fresh = WorkoutDocument(fromLive: workout) { doc = fresh } } .toolbar { diff --git a/Workouts/Views/WorkoutLogs/WorkoutSummaryView.swift b/Workouts/Views/WorkoutLogs/WorkoutSummaryView.swift index 8a9403e..3bf0840 100644 --- a/Workouts/Views/WorkoutLogs/WorkoutSummaryView.swift +++ b/Workouts/Views/WorkoutLogs/WorkoutSummaryView.swift @@ -26,19 +26,22 @@ struct WorkoutSummaryView: View { var body: some View { NavigationStack { Group { - if let workout { + // The fromLive guard also covers the query vending a workout whose + // cache row was deleted within the same view-graph update (remote + // delete / reconcile prune) — reading a dead @Model traps. + if let workout, let doc = WorkoutDocument(fromLive: workout) { ScrollView { VStack(spacing: 20) { Image(systemName: "checkmark.seal.fill") .font(.system(size: 48)) .foregroundStyle(.green) .padding(.top, 8) - Text(workout.routineName ?? Routine.unnamed) + Text(doc.routineName ?? Routine.unnamed) .font(.title2.bold()) WorkoutMetricsView(workout: workout) - if workout.metrics == nil { + if doc.metrics == nil { Label("Saving to Apple Health…", systemImage: "heart.text.square") .font(.footnote) .foregroundStyle(.secondary) @@ -71,15 +74,21 @@ struct WorkoutMetricsView: View { let workout: Workout @AppStorage("weightUnit") private var weightUnit: WeightUnit = .lb - private var metrics: WorkoutMetrics? { workout.metrics } + // Every read goes through this liveness-guarded snapshot: the cache row can be + // deleted out from under a retained `Workout` (remote delete, reconcile prune) + // while a parent re-evaluates mid-transition, and reading a persisted property + // on a dead @Model traps — see `WorkoutDocument.init?(fromLive:)`. + private var doc: WorkoutDocument? { WorkoutDocument(fromLive: workout) } + + private var metrics: WorkoutMetrics? { doc?.metrics } private var durationSeconds: Int? { - guard let end = workout.end else { return nil } - return max(0, Int(end.timeIntervalSince(workout.start))) + guard let doc, let end = doc.end else { return nil } + return max(0, Int(end.timeIntervalSince(doc.start))) } private var volume: Double { - metrics?.totalVolume ?? WorkoutVolume.total(WorkoutDocument(from: workout).logs) + metrics?.totalVolume ?? WorkoutVolume.total(doc?.logs ?? []) } var body: some View {