workouts/BULLETPROOFING.md

# 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.

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).

---

## What's already solid (verified)

Re-verified against code, not just prior docs — these need no work and should not be re-litigated:

- **Per-log merge** (`WorkoutMergePlanner`) — unordered T3/T4 delivery and concurrent edits to different exercises commute; phone-authored `deletedLogIDs` tombstones disambiguate absent logs.
- **Tombstone veto** (`SyncEngine.ingestFromWatch`) — a stale watch push can never resurrect a deleted workout; authoritative state is re-pushed instead.
- **Latest-wins context + authoritative prune** (`WatchCacheApplier`) — "absent ⇒ no longer active" reliably ends the watch session on phone-side delete/discard, and a nil-decode can never wipe real rows.
- **Launch-race guard** (`SessionEndPlanner.decide`: `!previouslyActiveIDs.isEmpty`) — a freshly launched session is never killed just because its run document hasn't synced yet (empty-baseline case).
- **Timer hardening** — countdown/haptic logic is wall-clock anchored on both platforms; sleep and Always-On drift self-correct without double-firing.
- **Pop-on-prune** (`ActiveWorkoutGateView.popIfNavigatedRunUnavailable`) — deletion/discard of the navigated run, or a phone edit lock, pops the watch back to the gate.

---

## HIGH — the session lifecycle has a single point of failure

### H1 — The `HKWorkoutSession` can only ever be born via the T1 one-shot  ·  **fixed**

> **Fix (2026-07-09):** the coordinator now self-starts a session on any reconcile that finds an active run with none running (`SessionEndPlanner.shouldStart` / `runToStart`; activity type resolved from the run's split), and `WorkoutSessionManager.recover()` re-adopts a crash/reboot-orphaned session at launch (`recoverActiveWorkoutSession`). T1 is now a fast-launch optimization, not the sole session source.

`WorkoutSessionManager.start(with:)` has exactly one caller: `WatchAppDelegate.handle(_:)` (`WatchAppDelegate.swift:31`), i.e. the phone's `startWatchApp(toHandle:)` handoff. That handoff is best-effort `try?` with **no retry** (`WorkoutLauncher.swift:37-40`). There is no `HKHealthStore.recoverActiveWorkoutSession` anywhere, no `WKExtendedRuntimeSession`, and nothing gates run-flow entry on `sessionManager.isRunning`.

Failure scenarios (all real):

1. **T1 fails silently** (watch briefly unreachable, Bluetooth blip, watch on charger at start) → the entire workout runs sessionless: no HR/energy, no Health save, no wrist-raise resurfacing, and the watch app **fully suspends** wrist-down — rest haptics and done-countdowns don't fire until the user manually reopens the app. The user gets no hint anything is wrong.
2. **Watch app crash or reboot mid-run** → watchOS expects the app to reclaim the live session via `recoverActiveWorkoutSession`; unimplemented, so the session is orphaned — metrics lost, zombie session may linger.
3. **User manually opens an active run on the watch** (run arrived via T2; T1 never fired for this device state) → same sessionless degradation.

**Fix direction:** make the watch own its session lifecycle. Self-start a session when entering (or reconciling into) an active run with `isRunning == false`; implement session recovery at launch. T1 then becomes a fast-launch optimization rather than the sole source of sessions. This also absorbs most of H2 and much of the M-tier degradation.

### H2 — "End Current & Start New" swallows the new run's session  ·  **fixed**

> **Fix (2026-07-09):** absorbed by H1's self-start (a swallowed T1 heals at the next reconcile), plus a session-age rule in the planner: a `.finish` decided for a session younger than `minimumFinishAge` (30 s) demotes to `.discard`, so the transient "old run completed, new run not yet pushed" context can no longer save a junk seconds-long HKWorkout attributed to the old run — the session is discarded and self-start brings it back when the new run's push lands.

`endActiveThenStart` (`WorkoutLogsView.swift:271-281`) runs the end-saves in a `Task` but calls `start(with:)` synchronously — so **T1 goes out while the old run's session is still running on the watch**. `handle(_:)` → `start(with:)` hits the `session == nil` idempotency guard (`WorkoutSessionManager.swift:61`) and is **silently dropped**. Moments later the "A completed" push arrives and the coordinator finishes A's session — leaving run B sessionless (all of H1's fallout). The one-shot was consumed while blocked; nothing replays it.

Timing variant: if B's session *does* start (old one already ended) but the transient "A completed, B not yet present" push applies next, `SessionEndPlanner.decide` sees `isRunning`, empty active set, non-empty baseline `{A}` → `.finish(A)` — **ending B's seconds-old session and attributing its metrics to A**. The empty-baseline launch guard doesn't cover a *non-empty stale* baseline.

**Fix direction:** on the watch, when `handle(_:)` arrives while a session is running, stash the configuration and start it immediately after the current session finishes; and/or a short grace window after `session.startActivity(...)` during which `decide` refuses `.finish`/`.discard`. H1's self-start largely absorbs this too.

### H3 — A system-ended session silently drops the Health save  ·  **fixed**

> **Fix (2026-07-09):** `salvageSystemEndedSession` — a system-driven `.ended`/`.stopped` now ends collection and finishes the builder's workout (saving the `HKWorkout` to Health) instead of clearing everything. Residual: the salvaged workout isn't linked to a run document (`healthKitWorkoutUUID` stays nil) and no metrics are forwarded — the runs are still active, so there's no completed run to attach them to.

`workoutSession(_:didChangeTo:)` for `.ended`/`.stopped` only calls `clear()` (`WorkoutSessionManager.swift:199-202`) — no `endCollection`/`finishWorkout`, no metrics capture, no forward to the phone. `PLAN-watch-session-end.md` lists this as a known residual. Rare (OS reclaim under resource pressure), but it is a "my workout never made it to Health" support report waiting to happen.

**Fix direction:** on a system-driven `.ended`, attempt `finishAndSave()` (or at minimum salvage the builder's collected data) before clearing, and forward whatever metrics exist.

---

## MEDIUM — stranding and stale-UI traps

### M1 — Edit locks have no expiry and outlive their editor  ·  **fixed** (expiry) / open (split-lock scope)

> **Fix (2026-07-09):** `PhoneConnectivityBridge.setLocksSuspended` — the scene-phase hook publishes the locks as *cleared* while the app is backgrounded (without forgetting them locally) and re-asserts them on return to foreground, so a pocketed or force-quit phone can no longer park the watch's run indefinitely. The split-lock *scope* (viewing `SplitDetailView` parks runs) was deliberately left as-is: that screen hosts inline edits (reorder, swipe-delete, add), so narrowing the lock to its sheets would reopen the clobber risk it exists to prevent.

`editingWorkoutID` / `editingSplitID` ride in the latest-wins context and are cleared only by `onDisappear` (`ExerciseView.swift:78`, `SplitDetailView.swift:55`). Force-quit the phone with an editor open — or just leave it open in a pocket — and the **last-pushed context says "editing" indefinitely**; the watch parks the run ("Editing on iPhone…") until the phone app next runs `pushAll`. Compounding it, `SplitDetailView` is a mostly-*read* screen: merely **viewing** a split in Settings parks any active watch run sourced from it (`SplitDetailView.swift:54`).

**Fix direction:** clear the locks on phone `scenePhase` → background/inactive; scope the split lock to the actual edit sheets rather than the detail screen.

### M2 — Remote completion doesn't pop the watch's open exercise screen  ·  **fixed**

> **Fix (2026-07-09):** both platforms' `ExerciseProgressView` now observe `log?.status` and dismiss when the log resolves to `.completed`/`.skipped` remotely (a `locallyResolved` flag keeps their own Done / flow hand-off from re-dismissing) — a stale open flow can no longer write a resolved log back to in-progress. The watch also gained the phone's `startsSkipped` terminal page (`SkippedPhaseView`), so opening a skipped exercise shows a static badge instead of a live, resurrectable flow. Gate-level popping was deliberately not added: with the screen-level dismissal the resurrection path is closed, and reviewing a just-completed run's list is legitimate.

`popIfNavigatedRunUnavailable` pops on prune and on lock — but a run flipping to `.completed`/`.skipped` remotely **stays in the cache** (recently-completed runs are still pushed for ~24 h), so no pop fires. `repairFromDurable`'s safety net is gated on `status == .inProgress`, going silent exactly when needed. And the watch's `ExerciseProgressView` lacks the phone's `.skipped` handling, so a user sitting on a stale open exercise can keep ticking and write it back to a **non-terminal status** — resurrecting a skip through the per-log merge, since their edit is "newer".

**Fix direction:** pop (or overlay a terminal state) when the navigated run leaves the active set, not only when it leaves the cache; port the phone's `.skipped` terminal handling to the watch's progress view.

### M3 — Follower mirror cover freezes in auto-advance splits  ·  **fixed** (freeze) / open (mid-set staleness)

> **Fix (2026-07-09):** the terminal between-exercise rest now falls back to `dismiss()` when there's no `onAdvance` host (the live-mirror cover on both platforms) — the exercise still completes durably, the cover closes instead of freezing at 0:00, and the driver's next-exercise frame re-presents it. Residual: a cover sitting on a *count-up work page* when the driver dies still has no staleness timeout — acceptable because that surface is a take-over driver by design and dismissible by hand.

The live-mirror cover (`LiveRunCoverView`, both platforms) is not a passive display — it runs the same local phase engine as a real driver, seeded from the last frame's anchors, and keeps advancing if the driver goes silent. Single-exercise runs self-heal (Finish auto-Done dismisses). But the cover is wired with `onAdvance: nil`, so in a flow-mode (auto-advance) split, when its local chain reaches the terminal between-exercise rest it completes the exercise (durable data stays correct) then **freezes at a 0:00 countdown** — the exercise hand-off exists only in `RunFlowView`, which the cover doesn't use. Related: if the driver dies without `sendLiveEnded`, the follower has **no staleness timeout** at all.

**Fix direction:** give the cover a terminal-rest hand-off (advance or dismiss), and add a staleness timeout that dismisses the cover when no frame arrives past `phaseEnd` + grace.

### M4 — `updateApplicationContext` size ceiling; failure is log-only  ·  **fixed**

> **Fix (2026-07-09):** `pushAll` now degrades on failure instead of freezing the watch: a failed push retries with the recently-completed tail dropped (display-only on the watch — active runs, splits, settings, and locks all still go through), logged as a warning; only a failure of the slim push too remains an error. Deeper tiers (trimming embedded logs) weren't needed — active runs are a handful by construction.

`pushAll` sends *all* splits plus all active and ≤25 recent-completed runs with their full logs. A heavy user (many splits × many exercises; long runs with `setEntries`) can plausibly exceed WatchConnectivity's ~65 KB context limit. On throw, the error is logged (`PhoneConnectivityBridge.swift:121-124`) and **the watch silently never hears about anything again** — functionally the same total-freeze as the schema-mismatch trap, but reachable in production.

**Fix direction:** on payload-too-large, degrade in tiers (drop completed runs, then trim embedded logs to what the watch renders) and re-push; surface the condition in Diagnostics.

---

## LOW

### L1 — Nondeterministic metrics attribution with parallel runs  ·  **fixed**

`SessionEndPlanner.decide` picked the completed survivor by iterating `previouslyActiveIDs` — a `Set`, so iteration order was undefined. With two parallel runs finishing together, the `HKWorkout`'s metrics attached to an arbitrary one.

> **Fix (2026-07-09):** `decide` now picks the most recently started completed survivor, ties broken by id (landed with the H-tier session-lifecycle commit; pinned by `parallelCompletions_pickMostRecentlyStarted`).

### L2 — `requestSync` failures vanish  ·  **fixed** (logged)

`sendMessage(WCPayload.requestSyncMessage(), replyHandler: nil, errorHandler: nil)` — a failed pull was dropped invisibly.

> **Fix (2026-07-09):** the send now logs its error. Deliberately no retry — the activation/reachability edges re-pull and the durable T2 slot delivers regardless.

### L3 — Decode-failure freeze is silent on the watch  ·  **fixed**

The schema-mismatch trap (see `DEVICE-COMMUNICATION.md` §6) is production-reachable after all: a user's phone app can update days before the watch app auto-updates, and every push in between fails to decode — silently.

> **Fix (2026-07-09):** `WatchConnectivityBridge.schemaMismatch` is set when a push fails to decode (cleared on the next good apply), and `ActiveWorkoutGateView` shows an "Update both apps to resume sync" banner while it's set — the freeze is now labeled instead of silent.

### L4 — Possible log-progress rollback from a late follower write  ·  open · Variant A refuted on device

> **Device result (2026-07-10, Variant A):** refuted — after the gap, the phone kept its higher completed-set count and the watch corrected *upward*. Read carefully, though: neither `WorkoutMergePlanner.merge` nor `ingestFromWatch` contains any progress guard — the merge is pure newest-`modTime`-wins per log — so what won was **timing**, not structure (most likely the phone's log ended up with the newer stamp by the time the queued watch write delivered). The mechanics remain permissive; the pinning test and the regressive-write detector log below are still worth adding, and **Variant B** (watch rest-expiry auto-advance instead of a manual swipe) is untested.

Both the driver and a follower cover persist auto-advance progress via whole-log newest-`updatedAt`-wins merge (not a monotonic max on progress). A follower resuming late from suspend could in theory regress a log if its delayed write's timestamp outranks the driver's more-advanced state before the durable echo catches it up. Unconfirmed on device.

Two caveats that shaped the deferral: the merge *mechanics* are provable without a device (a `WorkoutMergePlannerTests` case can pin that a stale-content/newer-stamp log wins), but whether real UI timing ever produces such a write is the device question. And the obvious fix — making `currentStateIndex` monotonic — is wrong: swiping back to Ready is a *deliberate* reset to 0 that must propagate, so a real fix has to distinguish "reset" from "stale echo" (per-log reset marker, or comparing set-entry contents).

#### Device test protocol

The merge runs **phone-side** (`SyncEngine.ingestFromWatch`), so the repro is: *the watch sends a semantically-behind write stamped newer than the phone's more-advanced one.* Three ingredients: a connectivity gap, divergence on both sides during the gap (phone further ahead; watch one step past its own last-known state), and the watch's write stamped **later**.

**Test rig** (created 2026-07-09): the **L4 Merge Test** split — orange, `testtube.2` icon, one exercise (Bench Press 5×8, rep-based so nothing but rests advances on its own), rest 45 s, auto-advance off — written directly into the iCloud container as `Splits/01KX513THZZ6RSYQ0ZA558BT5W.json`. It is ordinary user data; swipe-delete it in-app when done.

1. Start the workout from the phone's split picker; open Bench Press on **both** devices (both on Set 1's work page).
2. **Sever the link.** Phone-side Airplane Mode does **not** work (confirmed in practice, 2026-07-10) — modern iOS deliberately keeps Bluetooth (and often Wi-Fi) alive for the watch, and the Control Center BT tile explicitly preserves the watch link. Use one of these, in order of preference:
   - **A — Watch: Airplane Mode ON** (watch Control Center). Kills the watch's own BT + Wi-Fi radios; the phone stays fully usable.
   - **B — Phone: Settings → Bluetooth → Off *and* Settings → Wi-Fi → Off.** Must be the real switches in the Settings app, not the CC tiles (the tiles only disconnect, and WC falls back to Wi-Fi if either radio survives).
   - **C — Range separation:** with Wi-Fi off on the phone (Settings switch), walk the phone well out of Bluetooth range (another floor / far end of the home). Slowest to take effect but needs no settings on the watch.

   Whichever method you use, step 3's sanity check is the actual gate — don't trust the toggle, trust the check.
3. **Sanity-check the gap is real:** swipe a page on the phone — the watch must *not* follow. If it follows, the link is still up; do not proceed.
4. **Phone first — get ahead:** swipe through two full work→rest cycles (~2–3 sets recorded, stamped `t0`). Note the exact completed-set count.
5. Wait ~10 s, then **watch — one step:** swipe forward one page (less progress, newer stamp `t1 > t0`; the update queues via `transferUserInfo`). *Variant B (the original hypothesis): don't touch the watch — let its 45 s rest countdown expire on its own; the auto-advance records with a fresh stamp the same way.*
6. **Restore the link** (undo whatever you used in step 2 — Airplane Mode off / radios back on / walk back into range). Wait up to a minute — the queued update delivers, the phone merges, the echo pushes back.
7. **Read the phone's exercise screen / log list:**
   - **Confirmed:** the phone's completed-set count **drops** to the watch's lower number (and the echo drags the watch to the same rolled-back state). Phone-recorded set entries for the "lost" sets vanishing from the rest screen is the same signal.
   - **Refuted (for this path):** the phone keeps its higher count and the watch corrects *upward* — an existing guard won the race.

Observability: Console.app → iPhone → subsystem `dev.rzen.indie.Workouts` shows `phone-bridge` receiving the update, but there is no merge-outcome logging yet — the verdict is the UI diff. Offered but not yet added: a regressive-write detector log in `ingestFromWatch` (warn when an incoming log wins on `modTime` with a lower `currentStateIndex`) — worth keeping permanently as a grep-able smoking gun — plus the merge-planner pinning test above.

---

## 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:

1. **H1** — watch self-starts + recovers its session (one coherent change in `WorkoutSessionManager` / coordinator / run-flow entry; collapses most of H2, part of M-tier).
2. **H2** — pending-config replay and/or post-start grace window in `SessionEndPlanner`.
3. **H3** — salvage the Health save on system-ended sessions.
4. **M1** — lock expiry on scene background; narrow the split lock.
5. **M4** — tiered payload degradation + Diagnostics surfacing.
6. Then M2, M3, and the L-tier as opportunity allows.