workouts/PLAN-watch-session-end.md

# Plan — Fix the watch workout-session leak (reliable session end)

Fixes the bug behind "when the split ends, the watch app keeps coming back up even if I
close it." Companion to [`WATCH-SYNC-SCENARIOS.md`](WATCH-SYNC-SCENARIOS.md) (scenarios
S4 / S6 / S7 and the closing section).

## Problem

The watch's `HKWorkoutSession` grants the app persistent foreground runtime — a running
session is *exactly* what makes watchOS keep re-foregrounding the app on every wrist
raise. The session must be ended (`finishAndSave()` or `discard()`) when the run ends.

Today the **only** thing that ends it is `ActiveWorkoutGateView`'s
`.onChange(of: activeWorkouts)` firing on a non-empty → empty transition. That requires a
**mounted, foregrounded gate view** to witness the transition. When the run ends from the
**phone** (S4 complete-last-exercise, S6 "Save Workout" / start-new-split, S7 discard) the
watch app is typically backgrounded — kept alive only by the session — or gets torn down
and rebuilt by watchOS with an already-empty list. The `onChange` never fires, the session
stays `running`, and the app leaks. The watch-originated end (S5) works only because that
view happens to be alive on the watch at that moment.

## Root cause

Ending the session is a **lifecycle/data concern implemented as a view-level side
effect.** The trigger is bound to SwiftUI view lifetime instead of the authoritative data.

## Design principle (already validated in this codebase)

Commit `8cbe078` fixed the *same class* of fragility in the ephemeral live-run mirror by
adding `repairFromDurable` — the run screen stops trusting the transient frame and
**reconciles against the authoritative `WorkoutDocument`**. We apply the same move one
layer down: drive `session.end()` from the authoritative cache apply, in a **long-lived
coordinator** that survives view rebuilds and runs on the WatchConnectivity delegate
callback (which fires with or without any mounted view, including in the
session's background runtime).

No new wire message is required — the authoritative push already carries the signal (the
run leaves the active set on completion, or is pruned entirely on discard). A dedicated
durable "run ended" wire type was considered and rejected for now: it widens the protocol
for no gain over reconciling the state we already push.

## The fix

### 1. New pure decision seam — `SessionEndPlanner`

Mirrors the existing `WatchCacheApplier` pattern: a pure, session-free function that is
trivially unit-testable, with the impure glue kept thin around it.

```swift
enum SessionEndDecision: Equatable {
    case none
    case discard
    case finish(WorkoutDocument)   // save to Health, attach metrics, forward to phone
}

enum SessionEndPlanner {
    static func activeIDs(in workouts: [WorkoutDocument]) -> Set<String> {
        Set(workouts
            .filter { [.notStarted, .inProgress].contains(WorkoutStatus(rawValue: $0.status)) }
            .map(\.id))
    }

    /// `previouslyActiveIDs` is the active set captured at the *last* reconcile. The end
    /// only fires on a genuine non-empty → empty transition, which is what makes the
    /// launch race safe: at startup the session may be `running` before the run document
    /// has arrived, but `previouslyActiveIDs` is still empty, so we never discard a run
    /// we simply haven't heard about yet.
    static func decide(isRunning: Bool,
                       previouslyActiveIDs: Set<String>,
                       workouts: [WorkoutDocument]) -> SessionEndDecision {
        guard isRunning,
              activeIDs(in: workouts).isEmpty,
              !previouslyActiveIDs.isEmpty else { return .none }
        let finished = previouslyActiveIDs
            .compactMap { id in workouts.first { $0.id == id } }
            .first { WorkoutStatus(rawValue: $0.status) == .completed }
        return finished.map(SessionEndDecision.finish) ?? .discard
    }
}
```

### 2. New `WorkoutSessionCoordinator` (watch, `@MainActor`, owned by `WatchAppServices`)

Holds the same `WorkoutSessionManager` instance the delegate starts, plus the cache. Its
`reconcile()` is the single owner of session-end; `endAndForward` is lifted verbatim from
today's `ActiveWorkoutGateView.endSession`.

```swift
@MainActor
final class WorkoutSessionCoordinator {
    private let sessionManager: WorkoutSessionManager
    private let bridge: WatchConnectivityBridge   // to forward captured metrics
    private let context: ModelContext
    private var previouslyActiveIDs: Set<String> = []
    private var isEnding = false                  // guards the async finish from re-entry

    init(sessionManager: WorkoutSessionManager, bridge: WatchConnectivityBridge, context: ModelContext) { … }

    func reconcile() {
        guard !isEnding else { return }
        let workouts = (try? context.fetch(FetchDescriptor<Workout>()))?
            .map(WorkoutDocument.init(from:)) ?? []
        let decision = SessionEndPlanner.decide(
            isRunning: sessionManager.isRunning,
            previouslyActiveIDs: previouslyActiveIDs,
            workouts: workouts)
        previouslyActiveIDs = SessionEndPlanner.activeIDs(in: workouts)
        switch decision {
        case .none:            return
        case .discard:         sessionManager.discard()
        case .finish(let doc): endAndForward(doc)
        }
    }

    private func endAndForward(_ doc: WorkoutDocument) {
        isEnding = true
        Task {
            defer { isEnding = false }
            guard var metrics = await sessionManager.finishAndSave() else { return }
            metrics.totalVolume = WorkoutVolume.total(doc.logs)
            var updated = doc
            updated.metrics = metrics
            updated.updatedAt = Date()
            bridge.update(workout: updated)   // optimistic upsert + forward (queued if offline)
        }
    }
}
```

The watch cache holds only active + recently-pruned runs, so the fetch-all-map in
`reconcile()` is bounded and cheap.

### 3. Wiring

- **`WatchConnectivityBridge`** — add `var onWorkoutsChanged: (() -> Void)?` and fire it
  after every authoritative cache mutation, once the write is committed:
  - at the end of `applyState(...)` **on the success branch** (after `lastSyncDate = Date()`);
  - at the end of `update(workout:)` (after `try? context.save()`).
  These are the two places the watch's workout set changes: phone pushes (S4/S6/S7) and
  the watch's own optimistic edits (S5, including offline).
- **`WatchAppServices`** — hold the coordinator; replace `activate()` with
  `bootstrap(sessionManager:)` that stores the passed-in manager, builds the coordinator,
  sets `bridge.onWorkoutsChanged = { [weak coordinator] in coordinator?.reconcile() }`
  (weak break for the retain cycle), calls `bridge.activate()`, then calls
  `coordinator.reconcile()` once to seed `previouslyActiveIDs` from whatever is already
  applied.
- **`WorkoutsWatchApp`** — `.task { services.bootstrap(sessionManager: appDelegate.sessionManager) }`.
  The same `appDelegate.sessionManager` instance stays injected into the view tree via
  `.environment(...)` (the live HUD still reads it) — ownership is unchanged; the
  coordinator merely references it.
- **`ActiveWorkoutGateView`** — delete `.onChange(of: activeWorkouts.map(\.id))`, delete
  `endSession(previouslyActive:)`, and drop the now-unused
  `@Environment(WorkoutSessionManager.self)`. Everything else (list, `requestSync`,
  `popIfNavigatedRunUnavailable`) is untouched. The session end now has exactly one owner.
- **XcodeGen** — run `xcodegen generate` (new source file in the watch target; `project.yml`
  already globs `Workouts Watch App/`, so no `project.yml` edit).

### Fixed flow (contrast with S4/S6 in the scenarios doc)

```mermaid
sequenceDiagram
    participant PB as PhoneBridge
    participant WB as WatchBridge
    participant Coord as SessionCoordinator
    participant WS as Watch session

    PB->>WB: appContext (run completed — leaves active set) / (run pruned — discard)
    WB->>WB: WatchCacheApplier.apply → context.save
    WB->>Coord: onWorkoutsChanged → reconcile()
    Coord->>Coord: decide(isRunning, previouslyActive, cache)
    alt was active, now empty, a completed run survives
        Coord->>WS: finishAndSave() → session.end() + save HKWorkout
        Coord->>WB: update(workout + metrics) → forward to phone
    else was active, now empty, run absent
        Coord->>WS: discard() → session.end()
    end
    Note over WB,Coord: runs on the delegate callback — no mounted view required
```

## Why this covers every scenario

| Scenario | Trigger for `reconcile()` | Result |
|---|---|---|
| S1 launch race (session running before run doc lands) | first `applyState` | `previouslyActive` empty → **no** premature discard |
| S4 phone completes last exercise (watch backgrounded) | `applyState` on the push | session ends — no view needed |
| S6 phone "Save Workout" / start-new-split | `applyState` on the push | session ends |
| S7 phone discards | `applyState` on the prune push | `discard()` |
| S5 watch completes last exercise | local `update(workout:)` | ends immediately, **works offline** (no phone round-trip) |
| Watch relaunched cold | `bootstrap` seed reconcile | `isRunning == false` → no-op |

## Testing

- **`SessionEndPlannerTests`** (new, `Workouts Watch AppTests`, pure — no HealthKit / no
  SwiftData), pinning the decision table:
  - launch race: `isRunning`, prev `[]`, workouts `[]` → `.none`
  - complete: prev `["A"]`, workouts `[A.completed]` → `.finish(A)`
  - discard: prev `["A"]`, workouts `[]` → `.discard`
  - partial: prev `["A","B"]`, workouts `[A.completed, B.inProgress]` → `.none`
  - not running: `isRunning == false` → `.none`
  - already ended: prev `[]`, workouts `[A.completed]` → `.none`
  - `activeIDs(in:)` classification
- The coordinator itself stays a thin fetch-and-delegate shell (the same reason
  `WatchCacheApplier` is tested but `WatchConnectivityBridge` is not) — `WorkoutSessionManager`
  wraps `HKHealthStore`, which isn't unit-testable.
- **Manual device verification** (the real proof — this is inherently a background /
  view-lifecycle bug):
  1. Start a split on the phone → watch launches, session runs.
  2. With the **watch screen off / app backgrounded**, tap **Save Workout** on the phone →
     the watch app must stop resurfacing on wrist raise; the workout must appear in Health.
  3. Repeat with **Discard** → app stops resurfacing; nothing saved to Health.
  4. Start a split, put the **phone in airplane mode**, finish on the watch → session ends
     locally; metrics forward once connectivity returns.
  5. Start a split and immediately background the watch before the doc syncs → confirm the
     session is **not** discarded (launch race).

## Out of scope (tracked separately)

- **Per-log merge in `ingestFromWatch`** — the timestamp-not-content intake gate
  (documented at `SyncEngine.ingestFromWatch`; memory item "H1 per-log merge"). Independent
  of session end.
- **A durable "run ended" wire signal** — considered and deferred; the authoritative state
  push is a sufficient signal.
- **System-initiated session ends** (`WorkoutSessionManager.workoutSession(_:didChangeTo:)`,
  the `.ended`/`.stopped` branch) only `clear()` local state — no `finishWorkout()` save, no
  metrics forwarded. This coordinator handles *app*-driven ends (the leak); a watchOS-driven
  end (rare — e.g. the OS reclaims the session) still drops the HKWorkout and metrics. Left
  as a known residual: there may be no run document to attach to at that moment, and the
  frequency is low.

## Status

Implemented (session-end coordinator + `SessionEndPlanner` + `SessionEndPlannerTests`);
`ActiveWorkoutGateView`'s view-level trigger removed. The per-log merge and the residual
above remain open.

## Rollout

Single watch-side change set, no schema or wire-format change, no phone-side edits. Ship
behind normal build; verify on device with the checklist above; add a one-line CHANGELOG
entry (end-user visible: "The watch workout view now closes reliably when you end a workout
on your iPhone").