PR #233 review: T-1858 milestone create dismissal

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 53245a4..7525791 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -7,12 +7,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased]  - T-1816: Regression coverage now directly proves that `query_tasks` validates malformed `status`, `not_status`, `type`, `priority`, `unfinished`, `search`, and `displayId` filters before either a missing milestone name or missing milestone display ID can return a successful no-match `[]`. This pins the validation order introduced by T-1608 / PR #217 without changing its production behavior. - T-1749: Visual Add Task no longer reports `NO_PROJECTS` when it cannot read the project table while resolving an unselected project. The existence check now propagates its SwiftData error through the existing injectable fetch seam, and the visual intent returns `INTERNAL_ERROR` with the storage-failure guidance. Genuine empty stores still return `NO_PROJECTS`; stale selected projects remain on the separate `PROJECT_NOT_FOUND` / storage-failure path.  - T-1711: MCP `get_projects` now returns the exact tool error `Failed to fetch milestones: <error>` when a project-scoped milestone fetch fails, rather than a successful partial list that omits milestone metadata. The scoped service fetch preserves its injected storage error; deterministic regressions cover the failure, an empty project's omitted `milestones` field, and correctly scoped milestones for another project.+- T-1858: New Milestone now retains its asynchronous create operation for the editor lifetime. iOS disables its back and interactive dismissal controls while a create is in flight; iOS/macOS disappearance cancels only a still-pending create, allowing T-1765's persistence-boundary cancellation check to prevent a ghost milestone. A successful create transitions to a distinct pre-dismissal state before calling `dismiss()`, so the successful dismissal never cancels itself. Focused lifecycle tests cover in-flight disappearance, successful dismissal, save-error retry, and cancellation completion; edit-mode merge/conflict behavior is unchanged. - T-1699: Sync heartbeat fetch failures no longer create duplicate `SyncHeartbeat` singleton rows. `SyncManager` now logs and skips a beat when its singleton fetch fails, then retries on the existing next timer interval; a successful empty fetch still creates the record and a successful existing fetch still updates it. The save remains best-effort and timer lifecycle is unchanged. Deterministic `SyncManagerTests` verify zero insertion, later recovery, and the existing/missing cases. - T-1838: Open report views now refresh relative ranges at the next relevant local calendar boundary and whenever the scene returns active. Report filtering and labels share one captured time/calendar snapshot, so they cannot straddle a boundary; scheduling is DST- and calendar-aware, uses a cancellable one-shot task rather than a polling timer, and leaves App Intent and absolute-range behavior unchanged. - T-1675: Project-scoped milestone-name lookups now preserve the exact SwiftData fetch failure through shared assignment as `INTERNAL_ERROR` (`Failed to look up milestone: <error>`), matching JSON/MCP create, update, and scoped-query adapters. The milestone service's injected fetcher is now available in MCP test setup; deterministic regressions prove no-match and ambiguity remain distinct, cross-project unscoped filtering is untouched, and failed lookups create or mutate nothing. - T-1825: Dashboard milestone filters now live-observe local, MCP, and CloudKit milestone changes, retaining selected Done or Abandoned rows within the active project scope so they remain visible and individually deselectable. Open rows remain first and follow their displayed title order; terminal selections are appended deterministically and deduplicated by UUID. Selected rows expose the native selected accessibility trait once, while inaccessible or deleted selections still leave Clear available. Add Task remains open-only.  - T-1657: Project lookup storage failures now remain distinguishable from missing projects across JSON intents, MCP create/query/milestone tools, and visual Add Task. `QueryMilestonesIntent` no longer returns a successful empty array when its project-name lookup is unreadable, while valid missing/ambiguous names and project-ID precedence retain their prior behavior. Visual Add Task now reports `INTERNAL_ERROR` for a failed project read rather than a stale-selection `PROJECT_NOT_FOUND`; deterministic failing-fetch regressions verify cross-surface parity and no task or milestone insertion.

diff --git a/Transit/Transit/Views/Settings/MilestoneCreateSaveLifecycle.swift b/Transit/Transit/Views/Settings/MilestoneCreateSaveLifecycle.swiftnew file mode 100644index 0000000..b615237--- /dev/null+++ b/Transit/Transit/Views/Settings/MilestoneCreateSaveLifecycle.swift@@ -0,0 +1,65 @@+/// Models the create-mode save lifecycle for `MilestoneEditView`.+///+/// A successful save transitions before `dismiss()` so `onDisappear` cannot+/// cancel an operation that has already persisted. A pending create instead+/// transitions to cancellation, allowing the service's persistence-boundary+/// cancellation checks to prevent a ghost milestone.+nonisolated struct MilestoneCreateSaveLifecycle: Equatable {+    private enum State: Equatable {+        case idle+        case saving+        case cancellationPending+        case savedAwaitingDismissal+    }++    private var state: State = .idle++    /// Keeps navigation and interactive dismissal unavailable until either a+    /// save error/cancellation returns the editor to idle or the save dismisses it.+    var blocksDismissal: Bool {+        switch state {+        case .saving, .savedAwaitingDismissal:+            true+        case .idle, .cancellationPending:+            false+        }+    }++    var isCancellationPending: Bool {+        state == .cancellationPending+    }++    /// Starts one create operation. Repeated save actions are ignored while the+    /// current operation owns the lifecycle.+    mutating func beginSave() -> Bool {+        guard state == .idle else { return false }+        state = .saving+        return true+    }++    /// Marks a still-pending create for cancellation when its view disappears.+    /// A completed save intentionally returns false so its own `dismiss()` does+    /// not cancel the operation after persistence has succeeded.+    mutating func cancelForDisappearance() -> Bool {+        guard state == .saving else { return false }+        state = .cancellationPending+        return true+    }++    /// Records persistence success before the view is dismissed.+    mutating func completeSave() -> Bool {+        guard state == .saving else { return false }+        state = .savedAwaitingDismissal+        return true+    }++    mutating func completeFailure() {+        guard state == .saving else { return }+        state = .idle+    }++    mutating func completeCancellation() {+        guard state == .saving || state == .cancellationPending else { return }+        state = .idle+    }+}

diff --git a/Transit/Transit/Views/Settings/MilestoneEditView.swift b/Transit/Transit/Views/Settings/MilestoneEditView.swiftindex b1561b7..b5a4b85 100644--- a/Transit/Transit/Views/Settings/MilestoneEditView.swift+++ b/Transit/Transit/Views/Settings/MilestoneEditView.swift@@ -7,21 +7,26 @@ struct MilestoneEditView: View {     @Environment(\.dismiss) private var dismiss     @Environment(\.resolvedTheme) private var resolvedTheme      /// The draft, its baseline, and the load-once guard, held as one value so a     /// second `onAppear` cannot discard in-flight edits.     @State private var form = MilestoneEditForm()-    @State private var isSaving = false+    @State private var createSaveLifecycle = MilestoneCreateSaveLifecycle()+    @State private var createSaveTask: Task<Void, Never>?     @State private var errorMessage: String?      /// Set when a save finds fields that both the user and an external writer     /// changed. Presence drives the conflict alert.     @State private var pendingConflict: MilestoneEditMerge?      private var isEditing: Bool { milestone != nil } +    private var isSaving: Bool {+        createSaveLifecycle.blocksDismissal+    }+     var body: some View {         formContent             .alert("Save Failed", isPresented: $errorMessage.isPresent) {                 Button("OK") { errorMessage = nil }             } message: {                 Text(errorMessage ?? "")@@ -29,12 +34,13 @@ struct MilestoneEditView: View {             .editConflictAlert(                 subject: "Milestone",                 conflict: $pendingConflict,                 keepMine: { saveExisting(consentingTo: $0) },                 useTheirs: { adoptLiveValues(for: $0) }             )+            .onDisappear { cancelCreateSaveForDisappearance() }     }      private var formContent: some View {         #if os(macOS)         macOSForm         #else@@ -54,12 +60,13 @@ struct MilestoneEditView: View {             }         }         .navigationTitle(isEditing ? "Edit Milestone" : "New Milestone")         .navigationBarTitleDisplayMode(.inline)         .navigationBarBackButtonHidden(true)         .toolbar { editToolbar }+        .interactiveDismissDisabled(isSaving)         .onAppear { loadMilestone() }     }     #endif      // MARK: - macOS Layout @@ -109,12 +116,13 @@ struct MilestoneEditView: View {     @ToolbarContentBuilder     private var editToolbar: some ToolbarContent {         ToolbarItem(placement: .cancellationAction) {             Button { dismiss() } label: {                 Image(systemName: "chevron.left")             }+            .disabled(isSaving)         }         ToolbarItem(placement: .confirmationAction) {             Button("Save", systemImage: "checkmark") { save() }                 .disabled(!form.canSave || isSaving)         }     }@@ -124,13 +132,13 @@ struct MilestoneEditView: View {     private func loadMilestone() {         guard let milestone else { return }         form.load(from: milestone)     }      private func save() {-        guard form.canSave else { return }+        guard form.canSave, !isSaving else { return }         if milestone == nil {             createMilestone()         } else {             saveExisting()         }     }@@ -163,28 +171,52 @@ struct MilestoneEditView: View {             return         }         dismiss()     }      private func createMilestone() {+        guard createSaveLifecycle.beginSave() else { return }         let edited = form.edited-        isSaving = true-        Task {-            defer { isSaving = false }++        let saveTask = Task { @MainActor in             do {                 try await milestoneService.createMilestone(                     name: edited.name,                     description: edited.description.isEmpty ? nil : edited.description,                     project: project                 )+                try Task.checkCancellation()++                // Record success before dismissal so the resulting `onDisappear`+                // cannot cancel an operation that has already persisted.+                guard createSaveLifecycle.completeSave() else { return }+                createSaveTask = nil+                dismiss()+            } catch is CancellationError {+                finishCancelledCreate()             } catch {-                errorMessage = error.localizedDescription-                return+                if Task.isCancelled {+                    finishCancelledCreate()+                } else {+                    createSaveLifecycle.completeFailure()+                    createSaveTask = nil+                    errorMessage = error.localizedDescription+                }             }-            dismiss()         }+        createSaveTask = saveTask+    }++    private func cancelCreateSaveForDisappearance() {+        guard createSaveLifecycle.cancelForDisappearance() else { return }+        createSaveTask?.cancel()+    }++    private func finishCancelledCreate() {+        createSaveLifecycle.completeCancellation()+        createSaveTask = nil     }      /// Recomputes before using external values. Changed conflict fields or     /// values cause a new alert; otherwise the form performs a complete safe     /// rebase and remains open for review.     private func adoptLiveValues(for shownConflict: MilestoneEditMerge) {

diff --git a/Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swift b/Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swiftnew file mode 100644index 0000000..bf1e694--- /dev/null+++ b/Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swift@@ -0,0 +1,64 @@+import Foundation+import Testing+@testable import Transit++/// Regression tests for T-1858. The editor must retain and cancel a create+/// operation only while it is truly in flight; a completed save must be allowed+/// to dismiss without its own disappearance being treated as cancellation.+@MainActor+struct MilestoneCreateSaveLifecycleTests {++    @Test func disappearanceDuringInFlightCreateRequestsCancellation() {+        var lifecycle = MilestoneCreateSaveLifecycle()++        let didBegin = lifecycle.beginSave()+        let blocksWhileSaving = lifecycle.blocksDismissal+        let didRequestCancellation = lifecycle.cancelForDisappearance()++        #expect(didBegin)+        #expect(blocksWhileSaving)+        #expect(didRequestCancellation)+        #expect(lifecycle.isCancellationPending)+    }++    @Test func successfulSaveDoesNotCancelWhenDismissalMakesViewDisappear() {+        var lifecycle = MilestoneCreateSaveLifecycle()++        let didBegin = lifecycle.beginSave()+        let didCompleteSave = lifecycle.completeSave()+        let blocksWhileDismissing = lifecycle.blocksDismissal+        let didRequestCancellation = lifecycle.cancelForDisappearance()++        #expect(didBegin)+        #expect(didCompleteSave)+        #expect(blocksWhileDismissing)+        #expect(didRequestCancellation == false)+        #expect(lifecycle.isCancellationPending == false)+    }++    @Test func failedSaveReenablesDismissalAndRetry() {+        var lifecycle = MilestoneCreateSaveLifecycle()++        let didBegin = lifecycle.beginSave()+        lifecycle.completeFailure()+        let blocksAfterFailure = lifecycle.blocksDismissal+        let didRetry = lifecycle.beginSave()++        #expect(didBegin)+        #expect(blocksAfterFailure == false)+        #expect(didRetry)+    }++    @Test func completingCancellationReenablesTheEditor() {+        var lifecycle = MilestoneCreateSaveLifecycle()++        let didBegin = lifecycle.beginSave()+        let didRequestCancellation = lifecycle.cancelForDisappearance()+        lifecycle.completeCancellation()++        #expect(didBegin)+        #expect(didRequestCancellation)+        #expect(lifecycle.blocksDismissal == false)+        #expect(lifecycle.isCancellationPending == false)+    }+}

diff --git a/specs/bugfixes/milestone-create-dismissal/report.md b/specs/bugfixes/milestone-create-dismissal/report.mdnew file mode 100644index 0000000..8c8904d--- /dev/null+++ b/specs/bugfixes/milestone-create-dismissal/report.md@@ -0,0 +1,102 @@+# Bugfix Report: Milestone Create Dismissal++**Date:** 2026-08-05+**Status:** Fixed++## Description of the Issue++`MilestoneEditView` starts create-mode persistence in an unretained `Task`. While display-ID allocation is awaiting, the iOS back control remains available and macOS navigation can replace the editor. The view disappears but the operation continues, eventually persisting a milestone that the user had cancelled.++**Reproduction steps:**+1. Open **New Milestone** and enter a valid name.+2. Tap Save while display-ID allocation is slow or offline.+3. Navigate back on iOS, or change the Settings category on macOS.+4. Wait for allocation to finish.+5. Observe that the milestone is persisted despite the editor having been dismissed.++**Impact:** A cancellation can create an unexpected milestone. The risk is highest when CloudKit display-ID allocation suspends for a noticeable time.++## Investigation Summary++### Phase 1 — Initial overview++Expected behavior: the create operation either completes while the editor remains active, or a true view disappearance cancels the pending operation before persistence.++Actual behavior: the create task outlives the editor. The UI does not represent a cancellation boundary for the async create.++### Phase 2 — Systematic inspection++- `Transit/Transit/Views/Settings/MilestoneEditView.swift` creates an unstructured `Task` and does not retain it.+- The iOS custom back button is not disabled while `isSaving` is true; sheet-style interactive dismissal is also not blocked.+- No `onDisappear` lifecycle hook cancels an in-flight create, so macOS Settings-category navigation can also orphan the operation.+- `Transit/Transit/Services/MilestoneService.swift` already preserves the T-1765 invariant: after the allocation await it calls `Task.checkCancellation()` before constructing or saving a `Milestone`.+- Edit mode is synchronous and uses merge/conflict behavior; it must remain unchanged.++### Phase 3 — Root cause analysis++1. Why does dismissal persist a milestone? The create task continues after the view disappears.+2. Why does it continue? The view does not retain the task or cancel it on disappearance.+3. Why can the view disappear during save? The iOS back control remains enabled, and macOS navigation can replace the detail stack.+4. Why would cancellation be safe? T-1765 checks cancellation after allocation and immediately before the persistence boundary.+5. Root cause: `MilestoneEditView` has no explicit state machine separating an in-flight save from a successfully completed save that is about to dismiss.++**Defect type:** Async lifecycle race / missing cancellation boundary.++**Assumption validated:** The T-1765 service guard is after the only suspension in `createMilestone` and before insertion, so cancellation while allocation is pending cannot persist a milestone.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/Views/Settings/MilestoneEditView.swift` — retains the create-mode `Task`, disables the iOS back and interactive dismissal paths while saving, and cancels a still-pending operation when the editor disappears on either platform. Successful persistence marks completion before `dismiss()`, so that disappearance is intentionally not cancelled. Cancellation errors stay silent; ordinary save errors still remain in the editor and surface through the existing alert. Edit-mode merge/conflict save behavior is unchanged.+- `Transit/Transit/Views/Settings/MilestoneCreateSaveLifecycle.swift` — adds the small lifecycle state helper separating `saving`, `cancellationPending`, and `savedAwaitingDismissal`.+- `Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swift` — adds deterministic lifecycle regressions.++**Approach rationale:** The retained operation gives `onDisappear` a concrete cancellation target. The lifecycle helper ensures cancellation is requested only before the service returns; T-1765’s post-allocation `Task.checkCancellation()` then guarantees that a true cancellation cannot cross the milestone insert/save boundary. Marking success before dismissal prevents the successful route from being mistaken for cancellation.++**Alternatives considered:**+- Disable only the iOS back button — rejected because it does not cover swipe navigation or macOS category/detail replacement.+- Cancel every operation from `onDisappear` — rejected because a completed save’s `dismiss()` would spuriously cancel its own task.+- Change `MilestoneService` persistence behavior — rejected because T-1765 already provides the necessary cancellation check and the defect is view lifecycle ownership.++## Regression Test++**Test file:** `Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swift`++**Test names:** `disappearanceDuringInFlightCreateRequestsCancellation`, `successfulSaveDoesNotCancelWhenDismissalMakesViewDisappear`, `failedSaveReenablesDismissalAndRetry`, and `completingCancellationReenablesTheEditor`.++**What they verify:** The view-state model requests cancellation only for a genuinely pending create, keeps successful dismissal distinct from cancellation, and recovers after errors or cancellation. Existing `CancelledCreateTests` verifies T-1765 prevents a milestone from persisting after cancellation during allocation.++**Run commands:** `make test-quick`; targeted iOS `xcodebuild test` for `MilestoneCreateSaveLifecycleTests` and `CancelledCreateTests`.++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/Views/Settings/MilestoneEditView.swift` | Retain/cancel create task and gate iOS dismissal. |+| `Transit/Transit/Views/Settings/MilestoneCreateSaveLifecycle.swift` | Testable create-save lifecycle state helper. |+| `Transit/TransitTests/MilestoneCreateSaveLifecycleTests.swift` | Lifecycle regressions for T-1858. |+| `CHANGELOG.md` | Records the user-visible lifecycle fix. |++## Verification++**Automated:**+- [x] `make test-quick` passes.+- [x] Targeted iOS `MilestoneCreateSaveLifecycleTests` and `CancelledCreateTests` pass.+- [x] `make lint` passes.+- [ ] `make test` did not reach completion before the runner’s five-minute timeout. It compiled the iOS app and ran a large portion of `TransitTests` without a reported failure before timing out.+- [ ] `make test-ui` has unrelated existing failures in `TransitUITests.testClearAll`, `TransitUITests.testEditViewPreservesTaskMilestone`, and `DataMaintenanceUITests.testDataMaintenanceGoldenPath`, accompanied by simulator debugger-version-store errors. No UI test exercises milestone creation.++**Manual verification:**+- Not run: a deterministic UI reproduction needs a delayed allocator injection seam that the production UI-test harness does not expose. The lifecycle helper plus T-1765’s deterministic service tests cover the relevant cancellation boundary.++## Prevention++- Async view-owned persistence must retain its task and give disappearance a deliberate cancellation policy.+- Represent “save completed and now dismissing” separately from “save in progress” so lifecycle hooks cannot cancel successful work.+- Keep cancellation checks at service persistence boundaries for non-cooperative external awaits (T-1765).++## Related++- T-1858 — Milestone creation continues after the editor is dismissed.+- T-1765 — Pre-cancelled creates can still persist records.+- `Transit/Transit/Views/AddTask/AddTaskSheet.swift` — existing iOS save-dismissal guard pattern.