From 3a357833313e1487e325992e83c673a78f01f57e Mon Sep 17 00:00:00 2001 From: kutesir Date: Fri, 10 Jul 2026 01:52:56 +0300 Subject: [PATCH] workout: fix iCloud sync stomping the Watch-logged streak, remove Undo (KC-67) Root cause: CloudSyncManager.kvStoreChanged blindly overwrote any externally-changed iCloud key, including workoutDates - a lifetime tally that every local write path (HealthKit sync, manual finish) carefully dedups/unions. A stale iCloud snapshot racing a fresh Watch-sync write could silently replace the correct array with an older, smaller one. Now unions instead of overwriting for that key specifically. Also: removed the Undo action/row from DotProgressCard entirely (Watch/ Health is the source of truth once a day is logged), and restored the confirmation dialog on Finish Workout so every state-changing action confirms first. Co-Authored-By: Claude Opus 4.8 --- KisaniCal/ISSUES.md | 56 +++++++++++++++++++++++ KisaniCal/Managers/CloudSyncManager.swift | 12 ++++- KisaniCal/Models/ExerciseModels.swift | 14 ------ KisaniCal/Views/WorkoutView.swift | 28 +++--------- 4 files changed, 73 insertions(+), 37 deletions(-) diff --git a/KisaniCal/ISSUES.md b/KisaniCal/ISSUES.md index 17d4c4d..d6e01eb 100644 --- a/KisaniCal/ISSUES.md +++ b/KisaniCal/ISSUES.md @@ -2348,3 +2348,59 @@ Not build-verified (no iOS runtime here). Self-reviewed: balanced braces/ parens confirmed per-file. Same caveat as KC-65: tasks created before that fix still have no real `createdAt` and fall through to the anchor/inferred heuristics in both widgets. + +## KC-67 — Watch/Health workout streak got stomped by a stale iCloud sync + +Status: Implemented (not build-verified — no iOS runtime here) +Reported by: User — streak reset to 3 after a workout was already logged +by the Watch, plus a request for Finish/Clear-All confirmations and to +remove Undo +Area: Workout (`DotProgressCard`, `WorkoutViewModel`, `CloudSyncManager`) + +### Root cause +`workoutDates` is a lifetime tally (KC-40 — `Set(workoutDates).count`, +append-only by design) and every local write path already respects that: +`logWorkoutCompleted` dedups by key before appending, `syncFromHealthKit` +only unions HealthKit's dates in. But `CloudSyncManager.kvStoreChanged` — +the handler for "iCloud KV store changed externally" — did a **blind +overwrite**: `UserDefaults.kisani.set(val, forKey: key)` for every changed +key, no merge. Since `save()` pushes `workoutDates` to iCloud on every +write (including the Watch/HealthKit sync path), a stale iCloud snapshot +arriving after a fresh local write (races are inherent to `NSUbiquitous +KeyValueStore` — it's eventually consistent) would silently replace the +correct, larger local array with an older, smaller one — exactly matching +"the workout was already logged by the Watch, then tapping Finish reset +the streak." + +### Change +- `CloudSyncManager.kvStoreChanged`: for keys matching + `kisani.workout.completedDates.*`, union the incoming iCloud value with + whatever's already local instead of overwriting — an external change can + now only ever add days, never erase ones this device already knows + about. Every other synced key keeps the existing overwrite behavior + (unaffected — this tally is the only append-only-by-design value among + them). +- `DotProgressCard`: removed the `onUndo` action, its row, and the + `isDoneToday`/`isMissedToday` branch that showed it — the done/missed + state now shows the status header + progress dots with no action row + underneath at all (per "remove the Undo action from the logged workout + state"). Watch/Health is the source of truth once a day is logged; + there's no in-card way to contest that. +- Removed `onUndo:` at the `DotProgressCard(...)` call site in + `WorkoutView.swift`, and deleted `WorkoutViewModel.unmarkWorkoutDone`/ + `unmarkWorkoutMissed` (now dead — Undo was their only caller). +- Re-added the confirmation dialog on "Finish Workout" (previously + reverted in KC-63's follow-up at the user's request; re-requested here + alongside the streak fix) — every state-changing action on the card + (Finish, Check All, Clear All, Didn't Train) now confirms first. + +Files: `KisaniCal/Managers/CloudSyncManager.swift`, +`KisaniCal/Views/WorkoutView.swift`, `KisaniCal/Models/ExerciseModels.swift`. + +### Note +Not build-verified (no iOS runtime here). Self-reviewed: balanced braces/ +parens confirmed per-file. Structurally, the action row (with the Finish +button) is only ever rendered when `isDoneToday`/`isMissedToday` are both +false, so a watch-logged day already hides Finish entirely rather than +needing a runtime guard against double-logging — the real gap was the +CloudSync race above, not the button itself. diff --git a/KisaniCal/Managers/CloudSyncManager.swift b/KisaniCal/Managers/CloudSyncManager.swift index 60b1e78..12f1201 100644 --- a/KisaniCal/Managers/CloudSyncManager.swift +++ b/KisaniCal/Managers/CloudSyncManager.swift @@ -84,7 +84,17 @@ final class CloudSyncManager { else { return } for key in changedKeys { - if let val = kv.object(forKey: key) { + guard let val = kv.object(forKey: key) else { continue } + if key.hasPrefix("kisani.workout.completedDates."), let incoming = val as? [String] { + // Workout dates are an append-only lifetime tally, and the Watch/ + // HealthKit sync writes here independently of this device — a + // blind overwrite can race a fresh local write with a stale + // iCloud snapshot and silently drop already-logged days. Union + // instead, so an external change can only ever add days, never + // erase ones this device already knows about. + let existing = Set(UserDefaults.kisani.stringArray(forKey: key) ?? []) + UserDefaults.kisani.set(Array(existing.union(incoming)), forKey: key) + } else { UserDefaults.kisani.set(val, forKey: key) } } diff --git a/KisaniCal/Models/ExerciseModels.swift b/KisaniCal/Models/ExerciseModels.swift index 1275768..aca758c 100644 --- a/KisaniCal/Models/ExerciseModels.swift +++ b/KisaniCal/Models/ExerciseModels.swift @@ -663,20 +663,6 @@ class WorkoutViewModel: ObservableObject { save() } - /// Undo a "done" mark for a specific day — back to pending. Does NOT mark it - /// missed (that's a separate, explicit choice). Logged set data is kept. - func unmarkWorkoutDone(on date: Date) { - let key = iso.string(from: date) - workoutDates.removeAll { $0 == key } - save() - } - - /// Undo a "missed" mark — back to pending, without claiming it was done. - func unmarkWorkoutMissed(on date: Date) { - let key = iso.string(from: date) - missedDates.removeAll { $0 == key } - save() - } // MARK: - Compensation (recover a missed workout on a rest day) diff --git a/KisaniCal/Views/WorkoutView.swift b/KisaniCal/Views/WorkoutView.swift index ce2d300..e94accb 100644 --- a/KisaniCal/Views/WorkoutView.swift +++ b/KisaniCal/Views/WorkoutView.swift @@ -146,13 +146,7 @@ struct WorkoutView: View { onFinish: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutDone(on: Date()) } }, onMarkAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(true) } }, onClearAllSets: { withAnimation(KisaniSpring.snappy) { vm.setAllSetsDone(false) } }, - onNotCompleted: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutMissed(on: Date()) } }, - onUndo: { - withAnimation(KisaniSpring.snappy) { - if vm.isWorkoutMissed(on: Date()) { vm.unmarkWorkoutMissed(on: Date()) } - else { vm.unmarkWorkoutDone(on: Date()) } - } - } + onNotCompleted: { withAnimation(KisaniSpring.snappy) { vm.markWorkoutMissed(on: Date()) } } ) .listRowBackground(Color.clear) .listRowSeparator(.hidden) @@ -604,7 +598,6 @@ struct DotProgressCard: View { let onMarkAllSets: () -> Void let onClearAllSets: () -> Void let onNotCompleted: () -> Void - let onUndo: () -> Void /// Every quick action that changes today's state is confirmed first — a /// stray tap here shouldn't silently rewrite the day. @@ -648,9 +641,10 @@ struct DotProgressCard: View { DotGridProgress(progress: total > 0 ? progress : 0, cs: cs) } - if isDoneToday || isMissedToday { - undoRow - } else { + // Watch/Health is the source of truth once today is logged — no + // in-card way to undo that from here; the set checklist above is + // only for confirming sets, not for contesting the logged day. + if !(isDoneToday || isMissedToday) { actionRow } } @@ -740,7 +734,7 @@ struct DotProgressCard: View { private var actionRow: some View { VStack(spacing: 8) { Button { - if done == total && total > 0 { onFinish() } else { pendingAction = .finish } + pendingAction = .finish } label: { Label(done == total && total > 0 ? "Finish Workout" : "Finish Anyway", systemImage: "checkmark.circle.fill") @@ -789,16 +783,6 @@ struct DotProgressCard: View { } } - private var undoRow: some View { - HStack { - Spacer() - Button(action: onUndo) { - Label("Undo", systemImage: "arrow.uturn.backward") - .font(AppFonts.sans(11, weight: .semibold)) - } - .foregroundColor(AppColors.accent) - } - } } // MARK: - Workout Stats