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

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.

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

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