diff --git a/BULLETPROOFING.md b/BULLETPROOFING.md index 215fc7c..d3f843d 100644 --- a/BULLETPROOFING.md +++ b/BULLETPROOFING.md @@ -1,6 +1,6 @@ # Bulletproofing the Watch/Phone UX — Findings -An adversarial gap analysis of the iPhone ↔ Apple Watch communication layer, aimed at making the cross-device workout UX bulletproof. Architecture reference: `DEVICE-COMMUNICATION.md` (transports T1–T4, exchanges P1–P9 / W1–W4). Analysis date: 2026-07-09. +An adversarial gap analysis of the iPhone ↔ Apple Watch communication layer, aimed at making the cross-device workout UX bulletproof. Architecture reference: `DEVICE-COMMUNICATION.md` (transports T1–T4, exchanges P1–P9 / W1–W4). Analysis date: 2026-07-09. **Second pass 2026-07-18** (live-run mirror & flow hand-off — see the dated section at the end), prompted by a field incident. Findings are ranked by user impact. Each carries code evidence, the concrete failure scenario, and a fix direction. Statuses: **open** / **fixed** (update in place as items land). @@ -144,6 +144,67 @@ Observability: Console.app → iPhone → subsystem `dev.rzen.indie.Workouts` sh --- +# 2026-07-18 — Live-run mirror & flow hand-off + +Second-pass analysis prompted by a field incident (2026-07-17, Cardio flow routine: Warm Up → Main Circuit → Cool Off, all single-set duration exercises): run started on the phone, phone locked in pocket, watch ran the flow correctly — but ~5 min into the 30-min Main Circuit the phone was already counting down Main Circuit's terminal "Coming up: Cool Off" rest, then durably completed the phase early; both devices fought (dismiss ↔ resurrect) until manual swipes forced agreement. + +Design context: both devices run the *same* local phase engine; only **human** transitions are broadcast as `LiveProgress` frames — auto-advances are reached independently off shared wall-clock anchors. Flow routines cross exercises via `RunFlowView.advance`, and the incoming exercise broadcasts one hand-off frame. + +### H4 — Remote completion dismisses the flow instead of handing off; cross-log frames are ignored · **fixed** + +> **Fix (2026-07-18):** `RunFlowView` (both platforms) now follows a frame for a *different* log of the same flow workout by advancing to it — forward-only by plan order, and only into a still-runnable log, so a late-delivered frame for an earlier exercise can't yank the flow backwards. And `ExerciseProgressView`'s remote-resolve observer now hands off (`onAdvance(nextUnfinished)`) instead of dismissing when the current exercise completes remotely mid-flow; dismissal remains for skipped/last-exercise/non-flow cases. + +Two halves of one gap. (a) `RunFlowView` filtered incoming frames to `currentLogID` only (phone `RunFlowView.swift:67`, watch `:54`) — the driver's hand-off frame for the *next* exercise was silently dropped, so a follower could only cross an exercise boundary via its own timers (which on the phone need background runtime). (b) When the peer's durable "exercise completed" write landed first, `.onChange(of: log?.status)` called `dismiss()` (phone `ExerciseProgressView.swift:520`, watch `:352`) — popping the whole run, tearing down the Live Activity, sending `liveEnded`, and re-entering later through the mirror cover (a host with different terminal behavior — M3's residual). Which path fired was a pure race between the local timer tick and the ingest of the peer's write inside the same background wake. + +### H5 — The between-exercise rest completes a phase it never ran · **fixed** + +> **Fix (2026-07-18):** wall-clock plausibility gate on *automatic* completion of duration exercises (both platforms): an auto-fire earlier than `startedAt + sets × duration − slack` is provably wrong — the flow-terminal rest then self-repairs by jumping back to `computeResume()` instead of completing, and the Finish page's auto-Done holds at zero awaiting a human tap. Human Done taps are never gated. + +The terminal "Coming up" rest fired `completeExercise()` purely because its countdown window elapsed — regardless of how the page was reached. For a **1-set** exercise any single forward page slip lands on the terminal page (work + 1), so one spurious transition (see L5) durably completed a 30-minute phase ~45 s later; the write then propagated to the watch, whose driver saw `completed` and popped, while its own re-records resurrected the log through the merge — the observed tug-of-war. Likely epicenter of the incident. + +### M5 — Reconciliation dead zone across a single-set duration phase · open (mitigated by H5) + +Between "started" and "completed" of a 1-set duration exercise there is **no durable write** (`recordProgress` caps at `setCount − 1 = 0`) and **no frame** (all transitions are auto) — for a 30-min phase, a half-hour window in which nothing corrects a desynced peer. `repairFromDurable` can't help: it only jumps set boundaries, and a 1-set exercise has none. A lost hand-off frame (`sendMessage` is reachable-only; the backoff retry gives up after 5 attempts ≈ 15 s) leaves a late-waking follower anchored at local `now` — minutes of drift with no repair until the next human transition. + +**Fix direction:** stage auto-advance frames too (the depth-1 latest-wins slot makes this one message per phase), and/or extend the durable repair to recompute the expected phase from `startedAt + duration` for duration exercises. H5's gate already bounds the worst outcome (early durable completion). + +### M6 — `liveVersion` resets on app restart; the peer then drops every frame · **fixed** + +> **Fix (2026-07-18):** both bridges persist the version as a high-water mark in `UserDefaults` (stamped on send, raised on receive), so a relaunched app keeps outranking what the peer has already seen. + +The frame sequence lived in the bridge instance (`PhoneConnectivityBridge.swift:39`, `WatchConnectivityBridge.swift:44`). If the sender's app was killed mid-run (watchdog, memory), its next frames restarted at v1 while the receiver's `lastAppliedVersion` for the run was higher — `isNewer` rejected every one, and since a pocketed phone makes no human transitions to bump the sender's counter back up, the mirror stayed dead for the rest of the run, silently. + +### M7 — Phone-side absorbs lack the watch's strictly-newer guard · **fixed** + +> **Fix (2026-07-18):** the phone list absorb and the mirror cover absorb now require `incoming > doc.updatedAt`, matching the watch (`Workouts Watch App/Views/WorkoutLogListView.swift:115` and its rationale comment). + +`WorkoutLogListView.swift:174` (phone) and `LiveRunCoverView.swift:92` re-seeded the working doc on *any* `updatedAt` change, including a regression (stale cache echo, late-delivered older file version). The next local save then re-stamped the stale content as newest — exactly the clobber the watch-side guard exists to prevent, but on the device that owns the durable write path. + +### M8 — Context push can roll the watch cache back per-log · open + +`WatchCacheApplier` → `CacheMapper.upsertWorkout` applies the phone's copy with **no per-log staleness check** — the watch-side counterpart of H1's per-log merge exists only on the phone ingest path (`SyncEngine.ingestFromWatch`). The open doc is shielded by the strictly-newer absorb, but only at *workout* granularity: a push newer overall yet stale for one log regresses that log in the cache mid-run. Mostly self-healing (the phone's ingest of the watch's next write re-merges and re-pushes), but it can flap the watch UI and feeds L4's rollback mechanics. + +**Fix direction:** per-log `modTime` guard in the watch apply (or full merge symmetry with the phone). + +### L5 — Spurious pager write-backs are indistinguishable from human swipes · open + +Any `TabView` selection write-back not caused by a real swipe takes the `pageChangeCause == nil` branch in the page observer (both platforms) — recorded *and broadcast* as if the user swiped. One instance (the first-layout snap-to-0) is already guarded by `didRestorePage`, but a later relayout snap (foregrounding, page-count change) would broadcast a wrong page to the peer — and on a 1-set exercise any forward slip is the terminal page (see H5). H5's gate plus the new transition logging bound the damage; a structural fix (distinguishing UIKit write-backs from human interaction) is deferred. + +### Diagnostics (added 2026-07-18) + +Both platforms' `ExerciseProgressView` now log every page transition (old → new, cause, anchors), every applied remote frame (version/phase/set), durable repairs, remote-resolve hand-offs, and H5 gate trips under subsystem `dev.rzen.indie.Workouts`, category **`run-flow`** — the next incident should be attributable from Console.app instead of reconstructed. + +### Recommended order (this pass) + +1. **H4 + H5** — the incident killers (follow hand-offs; never auto-complete a phase that provably hasn't run). ✅ +2. **M7, M6** — cheap correctness parity + restart survival. ✅ +3. **Diagnostics** — `run-flow` logging on both platforms. ✅ +4. **M5** — auto-advance keep-alive frames + duration-aware repair (design change; do deliberately). +5. **M8** — per-log guard in the watch context apply. +6. **L5** — structural cause detection for pager write-backs. + +--- + ## Recommended order The common theme of every HIGH: **the watch doesn't own its session lifecycle — it's a passive recipient of a fire-and-forget launch.** Fix in this order: diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b4d527..46b12e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ **July 2026** +Fixed a rare timing glitch that could skip a long timed phase — like a cardio main circuit — when a workout ran on iPhone and Apple Watch together. + +When one device finishes an exercise and moves on, the other now follows along to the next exercise instead of dropping out of the run. + Workouts is now on the Mac: a native app for managing routines and schedules and browsing the exercise library, synced through the same iCloud Drive. Rest time now adjusts in one-second steps — in Settings and in a routine's custom rest — instead of five-second jumps. diff --git a/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift b/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift index 9041ae9..2fdd6e5 100644 --- a/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift +++ b/Workouts Watch App/Connectivity/WatchConnectivityBridge.swift @@ -41,7 +41,12 @@ final class WatchConnectivityBridge: NSObject { /// Monotonic sequence stamped on each live-run frame we send. Bumped to stay ahead of any /// frame we *receive*, so the two devices share one increasing per-run sequence and either /// side can drop a stale / out-of-order delivery (see `LiveProgress.version`). - private var liveVersion = 0 + /// Persisted as a high-water mark: an app relaunch resetting it to 0 would stamp frames + /// the peer has already outranked — and with no human transition on the peer to catch + /// the counter back up, every subsequent frame would be dropped as stale. + private var liveVersion = UserDefaults.standard.integer(forKey: liveVersionDefaultsKey) { + didSet { UserDefaults.standard.set(liveVersion, forKey: liveVersionDefaultsKey) } + } /// The latest live-run message we haven't confirmed reached the phone (depth 1, latest-wins). /// Staged regardless of reachability and re-sent by `flushLive()` when reachability/activation @@ -451,3 +456,6 @@ private enum PendingLive { case progress(LiveProgress) case ended(workoutID: String, logID: String) } + +/// Defaults key for the persisted live-frame version high-water mark (see `liveVersion`). +private let liveVersionDefaultsKey = "liveMirrorVersionHighWater" diff --git a/Workouts Watch App/Views/ExerciseProgressView.swift b/Workouts Watch App/Views/ExerciseProgressView.swift index 0250d69..4d8c87b 100644 --- a/Workouts Watch App/Views/ExerciseProgressView.swift +++ b/Workouts Watch App/Views/ExerciseProgressView.swift @@ -5,6 +5,7 @@ // Copyright 2025 Rouslan Zenetl. All Rights Reserved. // +import os import SwiftUI import WatchKit @@ -23,6 +24,10 @@ import WatchKit /// A dot row tracks progress with one marker per work set — the active set drawn as a /// wider dash, with the gap widening at the rest you're currently in. struct ExerciseProgressView: View { + /// Run-flow breadcrumbs (page transitions, applied frames, repairs) — the mirror's + /// failure modes are cross-device timing races, unattributable without a trail. + private static let log = Logger(subsystem: "dev.rzen.indie.Workouts", category: "run-flow") + @Environment(\.dismiss) private var dismiss /// Live HR straight off the session's builder — drives the target-HR readout when @@ -321,6 +326,7 @@ struct ExerciseProgressView: View { guard didRestorePage else { return } let cause = pageChangeCause pageChangeCause = nil + Self.log.info("page \(oldPage) → \(newPage) cause=\(cause.map { "\($0)" } ?? "human", privacy: .public) log=\(logID, privacy: .public)") switch cause { case .remote: // The phone already recorded and owns this transition — just follow it. @@ -351,13 +357,23 @@ struct ExerciseProgressView: View { } .onChange(of: log?.status) { _, raw in // The log resolved out from under us (the run was completed or ended early on - // the phone). Leave the live flow — staying would let the next recorded set - // write a resolved log back to in-progress, resurrecting it through the merge. - // `locallyResolved` keeps our own Done / flow hand-off from re-dismissing. + // the phone). Staying would let the next recorded set write a resolved log + // back to in-progress, resurrecting it through the merge. In flow mode a + // remote *completion* means the driver has already handed off — follow it to + // the next unfinished exercise instead of falling out of the run. Everything + // else (skipped, last exercise, non-flow) leaves the flow as before. + // `locallyResolved` keeps our own Done / flow hand-off from re-triggering this. guard didRestorePage, !locallyResolved, let status = raw.flatMap(WorkoutStatus.init(rawValue:)), status == .completed || status == .skipped else { return } - dismiss() + locallyResolved = true + if autoAdvance, status == .completed, let next = nextUnfinishedLogID, let onAdvance { + Self.log.info("remote completion of \(logID, privacy: .public) → flow hand-off to \(next, privacy: .public)") + onAdvance(next) + } else { + Self.log.info("remote resolve of \(logID, privacy: .public) (\(status.rawValue, privacy: .public)) → dismiss") + dismiss() + } } .onAppear { guard !didRestorePage else { return } @@ -444,6 +460,7 @@ struct ExerciseProgressView: View { lastAppliedStart = frame.phaseStart if frame.setCount != setCount { setCount = frame.setCount } let target = page(forPhase: frame.phase, setIndex: frame.setIndex) + Self.log.info("apply frame v\(frame.version) \(frame.phase.rawValue, privacy: .public)/\(frame.setIndex) → page \(target) log=\(logID, privacy: .public)") // A followed frame is progress this screen now *knows*, so a durable echo of the // same transition can't re-trigger the repair jump (a remote reset re-baselines). knownStateIndex = frame.phase == .ready ? 0 : max(knownStateIndex, completedSets(for: target)) @@ -471,6 +488,7 @@ struct ExerciseProgressView: View { if log.sets > setCount { setCount = log.sets } let target = base + min(max(0, completed), setCount - 1) * 2 guard target > currentPage else { return } + Self.log.info("durable repair: completed=\(completed) → page \(target) log=\(logID, privacy: .public)") clearAnchor() pageChangeCause = .remote withAnimation { currentPage = target } @@ -482,6 +500,33 @@ struct ExerciseProgressView: View { pageAnchor = nil } + /// Wall-clock plausibility gate for *automatic* completion of a duration exercise: its + /// work windows alone take `sets × duration`, so an auto-fire earlier than that (minus + /// slack) proves the terminal page was reached erroneously — a spurious pager + /// write-back or a stale anchor, not a run that actually happened. Human Done taps are + /// never gated; rep-based exercises have no wall-clock expectation and always pass, as + /// does a log missing its `startedAt` stamp (nothing to judge against). + private var autoCompletionPlausible: Bool { + guard isDuration, let log, let started = log.startedAt else { return true } + let minimum = Double(workDurationSeconds * setCount) - 2 + return Date().timeIntervalSince(started) >= minimum + } + + /// The flow-terminal rest fired before the exercise could have run (see + /// `autoCompletionPlausible`) — jump back to where the durable timestamps say the run + /// actually is instead of durably completing a phase that never happened. + private func repairEarlyTerminal() { + let resume = computeResume() + Self.log.warning("early terminal auto-fire blocked for \(logID, privacy: .public); repairing to page \(resume.page)") + guard currentPage != resume.page else { + pageAnchor = resume.anchor + return + } + pageAnchor = resume.anchor + pageChangeCause = .remote + withAnimation { currentPage = resume.page } + } + /// Inverse of `liveSnapshot`'s page→frame mapping: a frame's phase/set → page index. private func page(forPhase phase: LiveRunPhase, setIndex: Int) -> Int { let set = min(max(0, setIndex), max(0, setCount - 1)) @@ -611,6 +656,9 @@ struct ExerciseProgressView: View { anchorStart: anchorStart, anchorEnd: anchorEnd ) { _ in + // An auto-fire before the exercise could possibly have run proves + // this page was reached erroneously — repair, don't complete. + guard autoCompletionPlausible else { repairEarlyTerminal(); return } completeExercise() // Re-resolve at fire time — the next exercise may have been // finished elsewhere while we rested. None left ⇒ end the run. @@ -629,7 +677,8 @@ struct ExerciseProgressView: View { isActive: isActive, onDone: { completeExercise(); dismiss() }, onOneMore: addSet, - anchorEnd: anchorEnd + anchorEnd: anchorEnd, + autoFireAllowed: { autoCompletionPlausible } ) } } else if cycleIndex.isMultiple(of: 2) { @@ -1191,6 +1240,11 @@ private struct FinishPhaseView: View { /// sender's wall clock instead of local `now + countdown`, so both devices fire together. var anchorEnd: Date? = nil + /// Gates the *automatic* Done only (the tap always works): when false, the countdown + /// holds at zero awaiting a human — the host's wall-clock plausibility check says the + /// exercise can't genuinely be finished yet (see `autoCompletionPlausible`). + var autoFireAllowed: () -> Bool = { true } + @AppStorage("doneCountdownSeconds") private var doneCountdownSeconds: Int = 5 /// Wall-clock deadline for the auto-Done (same Always-On rationale as the rest timer). @@ -1236,7 +1290,7 @@ private struct FinishPhaseView: View { guard isActive, !didFire else { return } let r = Int(ceil(endDate.timeIntervalSinceNow)) remaining = max(0, r) - if r <= 0 { fire() } + if r <= 0, autoFireAllowed() { fire() } } private func fire() { diff --git a/Workouts Watch App/Views/RunFlowView.swift b/Workouts Watch App/Views/RunFlowView.swift index 32ce9b0..2987b53 100644 --- a/Workouts Watch App/Views/RunFlowView.swift +++ b/Workouts Watch App/Views/RunFlowView.swift @@ -59,6 +59,26 @@ struct RunFlowView: View { } .onAppear { bridge.navigatedRunID = currentLogID } .onDisappear { if bridge.navigatedRunID == currentLogID { bridge.navigatedRunID = nil } } + .onChange(of: bridge.liveIncoming) { _, frame in + followRemoteHandoff(frame) + } + } + + /// Follow the driver across an exercise boundary. Auto-advances are never broadcast, so + /// a frame for a *different* log of this flow workout is always the peer's discrete + /// hand-off into the next exercise — which the per-exercise child can't follow (its + /// incoming-frame filter is scoped to `currentLogID`). Forward-only by plan order and + /// only into a still-runnable log, so a late-delivered frame for an earlier exercise + /// can never yank the flow backwards. + private func followRemoteHandoff(_ frame: LiveProgress?) { + guard doc.autoAdvance == true, + let frame, frame.workoutID == doc.id, frame.logID != currentLogID else { return } + let sorted = doc.logs.sorted { $0.order != $1.order ? $0.order < $1.order : $0.id < $1.id } + guard let from = sorted.firstIndex(where: { $0.id == currentLogID }), + let to = sorted.firstIndex(where: { $0.id == frame.logID }), to > from else { return } + let status = WorkoutStatus(rawValue: sorted[to].status) ?? .notStarted + guard status == .notStarted || status == .inProgress else { return } + advance(to: frame.logID) } /// Hand off to the next exercise. Suppresses the leaving exercise's live-ended teardown diff --git a/Workouts/Connectivity/PhoneConnectivityBridge.swift b/Workouts/Connectivity/PhoneConnectivityBridge.swift index fa3a960..c701b31 100644 --- a/Workouts/Connectivity/PhoneConnectivityBridge.swift +++ b/Workouts/Connectivity/PhoneConnectivityBridge.swift @@ -36,7 +36,12 @@ final class PhoneConnectivityBridge: NSObject { /// Monotonic sequence stamped on each live-run frame we send. Bumped to stay ahead of /// any frame we *receive*, so the two devices share one increasing sequence per run and /// either side can drop a stale / out-of-order delivery (see `LiveProgress.version`). - private var liveVersion = 0 + /// Persisted as a high-water mark: an app relaunch resetting it to 0 would stamp frames + /// the peer has already outranked — and with no human transition on the peer to catch + /// the counter back up, every subsequent frame would be dropped as stale. + private var liveVersion = UserDefaults.standard.integer(forKey: liveVersionDefaultsKey) { + didSet { UserDefaults.standard.set(liveVersion, forKey: liveVersionDefaultsKey) } + } /// The latest live-run message we haven't confirmed reached the watch (depth 1, latest-wins). /// Staged regardless of reachability and re-sent by `flushLive()` when reachability/activation @@ -332,3 +337,6 @@ private enum PendingLive { case progress(LiveProgress) case ended(workoutID: String, logID: String) } + +/// Defaults key for the persisted live-frame version high-water mark (see `liveVersion`). +private let liveVersionDefaultsKey = "liveMirrorVersionHighWater" diff --git a/Workouts/Views/WorkoutLogs/ExerciseProgressView.swift b/Workouts/Views/WorkoutLogs/ExerciseProgressView.swift index d79dc12..459f909 100644 --- a/Workouts/Views/WorkoutLogs/ExerciseProgressView.swift +++ b/Workouts/Views/WorkoutLogs/ExerciseProgressView.swift @@ -5,6 +5,7 @@ // Copyright 2025 Rouslan Zenetl. All Rights Reserved. // +import os import SwiftUI import UIKit @@ -29,6 +30,10 @@ import UIKit /// workout stopwatch in big glanceable digits. A row of phase dots tracks progress: /// purple for work, teal for rest, with the current phase drawn as a wider dash. struct ExerciseProgressView: View { + /// Run-flow breadcrumbs (page transitions, applied frames, repairs) — the mirror's + /// failure modes are cross-device timing races, unattributable without a trail. + private static let log = Logger(subsystem: "dev.rzen.indie.Workouts", category: "run-flow") + @Environment(\.dismiss) private var dismiss @Environment(\.verticalSizeClass) private var verticalSizeClass @Environment(\.modelContext) private var modelContext @@ -486,6 +491,7 @@ struct ExerciseProgressView: View { guard didRestorePage else { return } let cause = pageChangeCause pageChangeCause = nil + Self.log.info("page \(oldPage) → \(newPage) cause=\(cause.map { "\($0)" } ?? "human", privacy: .public) log=\(logID, privacy: .public)") switch cause { case .remote: // The watch already recorded and owns this transition — just follow it. @@ -519,13 +525,24 @@ struct ExerciseProgressView: View { } .onChange(of: log?.status) { _, raw in // The log resolved out from under us (the run was completed or ended early on - // another device). Leave the live flow — staying would let the next recorded - // set write a resolved log back to in-progress, resurrecting it through the - // merge. `locallyResolved` keeps our own Done / flow hand-off from re-dismissing. + // another device). Staying would let the next recorded set write a resolved log + // back to in-progress, resurrecting it through the merge. In flow mode a remote + // *completion* means the driver has already handed off — follow it to the next + // unfinished exercise instead of falling out of the run (which would tear down + // the Live Activity and strand the flow in the mirror cover). Everything else + // (skipped, last exercise, non-flow) leaves the flow as before. + // `locallyResolved` keeps our own Done / flow hand-off from re-triggering this. guard didRestorePage, !locallyResolved, let status = raw.flatMap(WorkoutStatus.init(rawValue:)), status == .completed || status == .skipped else { return } - dismiss() + locallyResolved = true + if autoAdvance, status == .completed, let next = nextUnfinishedLogID, let onAdvance { + Self.log.info("remote completion of \(logID, privacy: .public) → flow hand-off to \(next, privacy: .public)") + onAdvance(next) + } else { + Self.log.info("remote resolve of \(logID, privacy: .public) (\(status.rawValue, privacy: .public)) → dismiss") + dismiss() + } } .onAppear { guard !didRestorePage else { return } @@ -666,6 +683,7 @@ struct ExerciseProgressView: View { lastAppliedStart = frame.phaseStart if frame.setCount != setCount { setCount = frame.setCount } let target = page(forPhase: frame.phase, setIndex: frame.setIndex) + Self.log.info("apply frame v\(frame.version) \(frame.phase.rawValue, privacy: .public)/\(frame.setIndex) → page \(target) log=\(logID, privacy: .public)") // A followed frame is progress this screen now *knows*, so a durable echo of the // same transition can't re-trigger the repair jump (a remote reset re-baselines). knownStateIndex = frame.phase == .ready ? 0 : max(knownStateIndex, completedSets(for: target)) @@ -693,6 +711,7 @@ struct ExerciseProgressView: View { if log.sets > setCount { setCount = log.sets } let target = base + min(max(0, completed), setCount - 1) * 2 guard target > currentPage else { return } + Self.log.info("durable repair: completed=\(completed) → page \(target) log=\(logID, privacy: .public)") clearAnchor() pageChangeCause = .remote withAnimation { currentPage = target } @@ -704,6 +723,33 @@ struct ExerciseProgressView: View { pageAnchor = nil } + /// Wall-clock plausibility gate for *automatic* completion of a duration exercise: its + /// work windows alone take `sets × duration`, so an auto-fire earlier than that (minus + /// slack) proves the terminal page was reached erroneously — a spurious pager + /// write-back or a stale anchor, not a run that actually happened. Human Done taps are + /// never gated; rep-based exercises have no wall-clock expectation and always pass, as + /// does a log missing its `startedAt` stamp (nothing to judge against). + private var autoCompletionPlausible: Bool { + guard isDuration, let log, let started = log.startedAt else { return true } + let minimum = Double(workDurationSeconds * setCount) - 2 + return Date().timeIntervalSince(started) >= minimum + } + + /// The flow-terminal rest fired before the exercise could have run (see + /// `autoCompletionPlausible`) — jump back to where the durable timestamps say the run + /// actually is instead of durably completing a phase that never happened. + private func repairEarlyTerminal() { + let resume = computeResume() + Self.log.warning("early terminal auto-fire blocked for \(logID, privacy: .public); repairing to page \(resume.page)") + guard currentPage != resume.page else { + pageAnchor = resume.anchor + return + } + pageAnchor = resume.anchor + pageChangeCause = .remote + withAnimation { currentPage = resume.page } + } + /// Inverse of `liveSnapshot`'s page→frame mapping: a frame's phase/set → page index. private func page(forPhase phase: LiveRunPhase, setIndex: Int) -> Int { let set = min(max(0, setIndex), max(0, setCount - 1)) @@ -786,6 +832,9 @@ struct ExerciseProgressView: View { onActivate: announceComingUp, onCountdownBeat: announceCountdownBeat ) { _ in + // An auto-fire before the exercise could possibly have run proves + // this page was reached erroneously — repair, don't complete. + guard autoCompletionPlausible else { repairEarlyTerminal(); return } completeExercise() // Re-resolve at fire time — the next exercise may have been // finished elsewhere while we rested. None left ⇒ end the run. @@ -805,7 +854,8 @@ struct ExerciseProgressView: View { isActive: isActive, onDone: { completeExercise(); dismiss() }, onOneMore: addSet, - anchorEnd: anchorEnd + anchorEnd: anchorEnd, + autoFireAllowed: { autoCompletionPlausible } ) .overlay(alignment: .bottom) { adjustPill } } @@ -1777,6 +1827,11 @@ private struct FinishPhaseView: View { /// sender's wall clock instead of local `now + countdown`, so both devices fire together. var anchorEnd: Date? = nil + /// Gates the *automatic* Done only (the tap always works): when false, the countdown + /// holds at zero awaiting a human — the host's wall-clock plausibility check says the + /// exercise can't genuinely be finished yet (see `autoCompletionPlausible`). + var autoFireAllowed: () -> Bool = { true } + @AppStorage("doneCountdownSeconds") private var doneCountdownSeconds: Int = 5 /// Wall-clock deadline for the auto-Done. `remaining` is what the Done button shows. @@ -1824,7 +1879,7 @@ private struct FinishPhaseView: View { if max(0, r) != remaining { withAnimation(.snappy(duration: 0.25)) { remaining = max(0, r) } } - if r <= 0 { fire() } + if r <= 0, autoFireAllowed() { fire() } } private func fire() { diff --git a/Workouts/Views/WorkoutLogs/LiveRunCoverView.swift b/Workouts/Views/WorkoutLogs/LiveRunCoverView.swift index 30bd32c..4a5ea3d 100644 --- a/Workouts/Views/WorkoutLogs/LiveRunCoverView.swift +++ b/Workouts/Views/WorkoutLogs/LiveRunCoverView.swift @@ -89,10 +89,13 @@ struct LiveRunCoverView: View { .onAppear { if doc == nil, let workout { doc = WorkoutDocument(from: workout) } } - .onChange(of: workout?.updatedAt) { _, _ in + .onChange(of: workout?.updatedAt) { _, incoming in // Absorb the durable progress the watch drove (currentStateIndex / status). The // live *page* is driven by frames, not this — so re-seeding the doc is safe. - if let workout { doc = WorkoutDocument(from: workout) } + // Strictly newer only (matching the list absorbs): a stale cache echo must not + // regress the working copy, which the next persist would re-stamp as newest. + guard let incoming, incoming > (doc?.updatedAt ?? .distantPast), let workout else { return } + doc = WorkoutDocument(from: workout) } } diff --git a/Workouts/Views/WorkoutLogs/RunFlowView.swift b/Workouts/Views/WorkoutLogs/RunFlowView.swift index 57db4c9..31b0bbc 100644 --- a/Workouts/Views/WorkoutLogs/RunFlowView.swift +++ b/Workouts/Views/WorkoutLogs/RunFlowView.swift @@ -79,6 +79,26 @@ struct RunFlowView: View { // than in the child's onDisappear — no longer cuts off the next exercise's cue. speechAnnouncer?.stop() } + .onChange(of: liveRun.current) { _, frame in + followRemoteHandoff(frame) + } + } + + /// Follow the driver across an exercise boundary. Auto-advances are never broadcast, so + /// a frame for a *different* log of this flow workout is always the peer's discrete + /// hand-off into the next exercise — which the per-exercise child can't follow (its + /// incoming-frame filter is scoped to `currentLogID`). Forward-only by plan order and + /// only into a still-runnable log, so a late-delivered frame for an earlier exercise + /// can never yank the flow backwards. + private func followRemoteHandoff(_ frame: LiveProgress?) { + guard doc.autoAdvance == true, + let frame, frame.workoutID == doc.id, frame.logID != currentLogID else { return } + let sorted = doc.logs.sorted { $0.order != $1.order ? $0.order < $1.order : $0.id < $1.id } + guard let from = sorted.firstIndex(where: { $0.id == currentLogID }), + let to = sorted.firstIndex(where: { $0.id == frame.logID }), to > from else { return } + let status = WorkoutStatus(rawValue: sorted[to].status) ?? .notStarted + guard status == .notStarted || status == .inProgress else { return } + advance(to: frame.logID) } /// Hand off to the next exercise. Suppresses the leaving exercise's live-ended/activity diff --git a/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift b/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift index 7875b8e..d554eac 100644 --- a/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift +++ b/Workouts/Views/WorkoutLogs/WorkoutLogListView.swift @@ -171,8 +171,13 @@ struct WorkoutLogListView: View { // 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 } + // Strictly-newer guard, matching the watch list: an equal stamp is our own + // write echoing back, and a stale cache echo (late-delivered older file + // version) must not regress the working copy mid-run — the next local save + // would re-stamp that stale content as newest and clobber real progress. + .onChange(of: workout.isLive ? workout.updatedAt : nil) { _, incoming in + if let incoming, incoming > doc.updatedAt, + let fresh = WorkoutDocument(fromLive: workout) { doc = fresh } } .toolbar { ToolbarItem(placement: .primaryAction) {