Pre-push review: T-1770 persistence-failure classification

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 0948b98..99f9fd3 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ## [Unreleased] +- T-1770: JSON mutation intents now preserve their established typed validation and domain-error payloads while mapping untyped SwiftData fetch/save failures to `INTERNAL_ERROR`. Shared task and milestone identifier resolution uses deterministic injected fetchers; milestone mutations also expose an injected saver. Regression coverage spans create, status update, milestone update, and deletion paths, asserting both the internal-error envelope and no persisted mutation on failure.+ - T-1803: `UpdateStatusIntent` now includes a missing requested `displayId` in its `TASK_NOT_FOUND` hint (`No task with displayId N`), while missing UUIDs retain the existing generic lookup hint and malformed identifiers, duplicate IDs, status validation, and atomic mutation behavior remain unchanged. ### Fixed - T-1613: MCP `query_tasks` now returns the exact tool error `Failed to fetch comments: <error>` when comment serialization cannot read storage, rather than reporting a successful task with `comments: []`. The detailed display-ID and task-list paths share this throwing serialization boundary; genuine empty comment collections retain their successful `comments: []` response. `update_task_status` continues serializing the `Comment` returned by its atomic mutation directly (T-1823), so it performs no post-commit comment fetch that could prompt a retry or duplicate a persisted comment. Deterministic MCP regressions cover both query errors, legitimate empty comments, and zero status-response fetches.

diff --git a/Transit/Transit/Extensions/ModelContext+Save.swift b/Transit/Transit/Extensions/ModelContext+Save.swiftindex d70f48b..318233a 100644--- a/Transit/Transit/Extensions/ModelContext+Save.swift+++ b/Transit/Transit/Extensions/ModelContext+Save.swift@@ -61,3 +61,23 @@ extension ModelContext {         }     } }++extension ModelContext {++    /// Applies `mutation`, then persists through an injected save closure. This+    /// is the injectable counterpart to `saveOrRollback(save:_:)`: updates and+    /// deletes still recover with `safeRollback()` if either the mutation or the+    /// supplied save fails.+    func saveOrRollback(+        save: (ModelContext) throws -> Void,+        _ mutation: () throws -> Void = {}+    ) throws {+        do {+            try mutation()+            try save(self)+        } catch {+            safeRollback()+            throw error+        }+    }+}

diff --git a/Transit/Transit/Intents/CreateMilestoneIntent.swift b/Transit/Transit/Intents/CreateMilestoneIntent.swiftindex 3c8e41a..57d4a3e 100644--- a/Transit/Transit/Intents/CreateMilestoneIntent.swift+++ b/Transit/Transit/Intents/CreateMilestoneIntent.swift@@ -86,7 +86,7 @@ struct CreateMilestoneIntent: AppIntent {         } catch let error as MilestoneService.Error {             return IntentHelpers.mapMilestoneError(error).json         } catch {-            return IntentError.invalidInput(hint: "Milestone creation failed").json+            return IntentError.internalError(hint: "Milestone creation failed").json         }          var response: [String: Any] = [

diff --git a/Transit/Transit/Intents/CreateTaskIntent.swift b/Transit/Transit/Intents/CreateTaskIntent.swiftindex 848b0d9..b20b259 100644--- a/Transit/Transit/Intents/CreateTaskIntent.swift+++ b/Transit/Transit/Intents/CreateTaskIntent.swift@@ -246,8 +246,8 @@ struct CreateTaskIntent: AppIntent {         } catch let error as MilestoneService.Error {             return (nil, IntentHelpers.mapMilestoneError(error).json)         } catch {-            return (nil, IntentError.milestoneNotFound(-                hint: "No milestone with displayId \(displayId)"+            return (nil, IntentError.internalError(+                hint: "Failed to look up milestone: \(error)"             ).json)         }     }

diff --git a/Transit/Transit/Intents/DeleteMilestoneIntent.swift b/Transit/Transit/Intents/DeleteMilestoneIntent.swiftindex d37e584..6c8f47c 100644--- a/Transit/Transit/Intents/DeleteMilestoneIntent.swift+++ b/Transit/Transit/Intents/DeleteMilestoneIntent.swift@@ -65,7 +65,7 @@ struct DeleteMilestoneIntent: AppIntent {         do {             try milestoneService.deleteMilestone(milestone)         } catch {-            return IntentError.invalidInput(hint: "Deletion failed").json+            return IntentError.internalError(hint: "Deletion failed").json         }          var response: [String: Any] = [@@ -110,10 +110,12 @@ struct DeleteMilestoneIntent: AppIntent {         }         do {             return .success(try milestoneService.findByDisplayID(displayId))+        } catch MilestoneService.Error.milestoneNotFound {+            return .failure(.milestoneNotFound(hint: "No milestone with displayId \(displayId)"))         } catch let error as MilestoneService.Error {             return .failure(IntentHelpers.mapMilestoneError(error))         } catch {-            return .failure(.milestoneNotFound(hint: "No milestone with displayId \(displayId)"))+            return .failure(.internalError(hint: "Failed to look up milestone: \(error)"))         }     } @@ -130,8 +132,12 @@ struct DeleteMilestoneIntent: AppIntent {         case .success(let uuid?):             do {                 return .success(try milestoneService.findByID(uuid))-            } catch {+            } catch MilestoneService.Error.milestoneNotFound {                 return .failure(.milestoneNotFound(hint: "No milestone with ID \(uuid.uuidString)"))+            } catch let error as MilestoneService.Error {+                return .failure(IntentHelpers.mapMilestoneError(error))+            } catch {+                return .failure(.internalError(hint: "Failed to look up milestone: \(error)"))             }         }     }

diff --git a/Transit/Transit/Intents/IntentHelpers.swift b/Transit/Transit/Intents/IntentHelpers.swiftindex 81df9ae..1b6ff58 100644--- a/Transit/Transit/Intents/IntentHelpers.swift+++ b/Transit/Transit/Intents/IntentHelpers.swift@@ -167,9 +167,7 @@ nonisolated enum IntentHelpers {         } catch TaskService.Error.taskNotFound {             return .failure(.taskNotFound(hint: taskNotFoundHint(from: json)))         } catch {-            return .failure(.taskNotFound(-                hint: "Provide either displayId (integer) or taskId (UUID)"-            ))+            return .failure(.internalError(hint: "Failed to look up task: \(error)"))         }     } @@ -255,8 +253,12 @@ nonisolated enum IntentHelpers {             return .success(try milestoneService.findByDisplayID(displayId))         } catch MilestoneService.Error.duplicateDisplayID {             return .failure(.internalError(hint: "Duplicate milestone identifier for displayId \(displayId)"))-        } catch {+        } catch MilestoneService.Error.milestoneNotFound {             return .failure(.milestoneNotFound(hint: "No milestone with displayId \(displayId)"))+        } catch let error as MilestoneService.Error {+            return .failure(mapMilestoneError(error))+        } catch {+            return .failure(.internalError(hint: "Failed to look up milestone: \(error)"))         }     } @@ -272,9 +274,13 @@ nonisolated enum IntentHelpers {         case .success(let uuid?):             do {                 return .success(try milestoneService.findByID(uuid))-            } catch {+            } catch MilestoneService.Error.milestoneNotFound {                 let idStr = json["milestoneId"] as? String ?? "unknown"                 return .failure(.milestoneNotFound(hint: "No milestone with ID \(idStr)"))+            } catch let error as MilestoneService.Error {+                return .failure(mapMilestoneError(error))+            } catch {+                return .failure(.internalError(hint: "Failed to look up milestone: \(error)"))             }         }     }@@ -442,9 +448,7 @@ nonisolated enum IntentHelpers {             } catch let error as MilestoneService.Error {                 return mapMilestoneError(error).json             } catch {-                return IntentError.milestoneNotFound(-                    hint: "No milestone with displayId \(milestoneDisplayId)"-                ).json+                return IntentError.internalError(hint: "Failed to assign milestone").json             }         } else if let milestoneName = json["milestone"] as? String {             return assignMilestone(named: milestoneName, to: task, using: milestoneService)@@ -478,7 +482,7 @@ nonisolated enum IntentHelpers {         } catch let error as MilestoneService.Error {             return mapMilestoneError(error).json         } catch {-            return IntentError.invalidInput(hint: "Failed to assign milestone").json+            return IntentError.internalError(hint: "Failed to assign milestone").json         }     } }

diff --git a/Transit/Transit/Intents/TaskUpdateValidator.swift b/Transit/Transit/Intents/TaskUpdateValidator.swiftindex 9f1bce1..7d34c5e 100644--- a/Transit/Transit/Intents/TaskUpdateValidator.swift+++ b/Transit/Transit/Intents/TaskUpdateValidator.swift@@ -286,10 +286,12 @@ enum TaskUpdateValidator {             return .failure(.duplicateMilestoneDisplayID(                 message: "Duplicate milestone identifier detected for displayId \(displayId)"             ))-        } catch {+        } catch MilestoneService.Error.milestoneNotFound {             return .failure(.milestoneNotFound(                 message: "No milestone with displayId \(displayId)"             ))+        } catch {+            return .failure(.internalError("Failed to look up milestone: \(error)"))         }     } @@ -316,7 +318,7 @@ enum TaskUpdateValidator {                 message: "Multiple milestones named '\(name)' exist in project '\(project.name)'"             ))         } catch {-            return .failure(.invalidInput("Failed to look up milestone: \(error)"))+            return .failure(.internalError("Failed to look up milestone: \(error)"))         }     } @@ -403,6 +405,7 @@ enum MilestoneAction { enum TaskUpdateValidationError: Error {     case invalidInput(String)     case invalidPriority(String)+    case internalError(String)     case milestoneNotFound(message: String)     case ambiguousMilestoneName(message: String)     case duplicateMilestoneDisplayID(message: String)@@ -416,6 +419,7 @@ enum TaskUpdateValidationError: Error {         switch self {         case .invalidInput(let message),              .invalidPriority(let message),+             .internalError(let message),              .milestoneNotFound(let message),              .ambiguousMilestoneName(let message),              .duplicateMilestoneDisplayID(let message):@@ -436,6 +440,8 @@ enum TaskUpdateValidationError: Error {             return .invalidInput(hint: hint)         case .invalidPriority(let hint):             return .invalidPriority(hint: hint)+        case .internalError(let hint):+            return .internalError(hint: hint)         case .milestoneNotFound(let message):             return .milestoneNotFound(hint: message)         case .ambiguousMilestoneName(let message):

diff --git a/Transit/Transit/Intents/UpdateMilestoneIntent.swift b/Transit/Transit/Intents/UpdateMilestoneIntent.swiftindex 15ececf..12e25a3 100644--- a/Transit/Transit/Intents/UpdateMilestoneIntent.swift+++ b/Transit/Transit/Intents/UpdateMilestoneIntent.swift@@ -84,7 +84,7 @@ struct UpdateMilestoneIntent: AppIntent {             do {                 try milestoneService.save()             } catch {-                return IntentError.invalidInput(hint: "Update failed").json+                return IntentError.internalError(hint: "Update failed").json             }         } 

diff --git a/Transit/Transit/Intents/UpdateStatusIntent.swift b/Transit/Transit/Intents/UpdateStatusIntent.swiftindex cde4421..5e92b4d 100644--- a/Transit/Transit/Intents/UpdateStatusIntent.swift+++ b/Transit/Transit/Intents/UpdateStatusIntent.swift@@ -88,7 +88,7 @@ struct UpdateStatusIntent: AppIntent {         do {             try taskService.updateStatus(task: task, to: newStatus)         } catch {-            return IntentError.invalidInput(hint: "Status update failed").json+            return IntentError.internalError(hint: "Status update failed").json         }          var response: [String: Any] = [

diff --git a/Transit/Transit/Services/MilestoneService.swift b/Transit/Transit/Services/MilestoneService.swiftindex d7b9cc6..ffa53bb 100644--- a/Transit/Transit/Services/MilestoneService.swift+++ b/Transit/Transit/Services/MilestoneService.swift@@ -10,12 +10,10 @@ final class MilestoneService {     private let modelContext: ModelContext     private let displayIDAllocator: DisplayIDAllocator -    /// Store reads for invariants derived from fetch results. Name uniqueness uses-    /// the live fetcher directly. Display-ID guards additionally union a transient-    /// committed-store read so peer-synced IDs cannot be hidden by cached models-    /// (T-1614, T-1621, T-1939).+    /// Injected store reads for direct lookup and collision checks.     private let fetcher: any ModelFetching     private let usedDisplayIDs: UsedDisplayIDs+    private let mutationSave: (ModelContext) throws -> Void      /// Single-flight guard for `promoteProvisionalMilestones`. Prevents     /// concurrent promotion runs from overlapping (T-597).@@ -24,12 +22,14 @@ final class MilestoneService {     init(         modelContext: ModelContext,         displayIDAllocator: DisplayIDAllocator,-        fetcher: (any ModelFetching)? = nil+        fetcher: (any ModelFetching)? = nil,+        mutationSave: @escaping (ModelContext) throws -> Void = { try $0.save() }     ) {         self.modelContext = modelContext         self.displayIDAllocator = displayIDAllocator         self.fetcher = fetcher ?? modelContext         self.usedDisplayIDs = UsedDisplayIDs(modelContext: modelContext, liveFetcher: self.fetcher)+        self.mutationSave = mutationSave     }      // MARK: - CRUD@@ -39,7 +39,7 @@ final class MilestoneService {         name: String,         description: String?,         project: Project,-        save: (ModelContext) throws -> Void = { try $0.save() }+        save: ((ModelContext) throws -> Void)? = nil     ) async throws -> Milestone {         let trimmedName = name.trimmingCharacters(in: .whitespacesAndNewlines)         guard !trimmedName.isEmpty else {@@ -98,7 +98,7 @@ final class MilestoneService {             displayID: displayID         ) -        try modelContext.insertOrDelete(milestone, save: save)+        try modelContext.insertOrDelete(milestone, save: save ?? mutationSave)         return milestone     } @@ -129,7 +129,7 @@ final class MilestoneService {             milestone.milestoneDescription = nil         } -        try modelContext.saveOrRollback()+        try modelContext.saveOrRollback(save: mutationSave)     }      func updateStatus(_ milestone: Milestone, to newStatus: MilestoneStatus) throws {@@ -147,12 +147,12 @@ final class MilestoneService {             milestone.completionDate = nil         } -        try modelContext.saveOrRollback()+        try modelContext.saveOrRollback(save: mutationSave)     }      func deleteMilestone(_ milestone: Milestone) throws {         modelContext.delete(milestone)-        try modelContext.saveOrRollback()+        try modelContext.saveOrRollback(save: mutationSave)     }      // MARK: - Assignment@@ -173,7 +173,7 @@ final class MilestoneService {          task.milestone = milestone -        try modelContext.saveOrRollback(save: save)+        if save { try modelContext.saveOrRollback(save: mutationSave) }     }      // MARK: - Promotion@@ -251,7 +251,7 @@ final class MilestoneService {         let descriptor = FetchDescriptor<Milestone>(             predicate: #Predicate { $0.id == id }         )-        guard let milestone = try modelContext.fetch(descriptor).first else {+        guard let milestone = try fetcher.fetch(descriptor).first else {             throw Error.milestoneNotFound         }         return milestone@@ -261,7 +261,7 @@ final class MilestoneService {         let descriptor = FetchDescriptor<Milestone>(             predicate: #Predicate { $0.permanentDisplayId == displayId }         )-        let milestones = try modelContext.fetch(descriptor)+        let milestones = try fetcher.fetch(descriptor)          guard let first = milestones.first else {             throw Error.milestoneNotFound@@ -283,7 +283,7 @@ final class MilestoneService {         let descriptor = FetchDescriptor<Milestone>(             predicate: #Predicate { $0.project?.id == projectID }         )-        let matches = try modelContext.fetch(descriptor).filter {+        let matches = try fetcher.fetch(descriptor).filter {             MilestoneNamePolicy.normalized($0.name) == normalized         }         guard let match = matches.first else { return nil }@@ -336,7 +336,7 @@ final class MilestoneService {      /// Saves the model context. Rolls back on failure.     func save() throws {-        try modelContext.saveOrRollback()+        try modelContext.saveOrRollback(save: mutationSave)     }      /// Whether the project already holds a milestone with this name

diff --git a/Transit/Transit/Services/TaskService.swift b/Transit/Transit/Services/TaskService.swiftindex c374269..defc65d 100644--- a/Transit/Transit/Services/TaskService.swift+++ b/Transit/Transit/Services/TaskService.swift@@ -42,10 +42,10 @@ final class TaskService {     private let createSave: (ModelContext) throws -> Void     private let statusSave: (ModelContext) throws -> Void -    /// Display IDs feeding the allocator's collision guard. Production unions a-    /// transient committed-store read with the live main-context values; the-    /// injectable live fetcher lets tests simulate stale or unreadable registered-    /// state without replacing the committed-store read (T-1621, T-1939).+    /// Store reads for direct task lookups and display-ID collision guards. Tests+    /// inject a failing fetcher while retaining a writable model context, so+    /// automation surfaces can prove storage failures are not caller errors.+    private let fetcher: any ModelFetching     private let usedDisplayIDs: UsedDisplayIDs      init(@@ -57,9 +57,10 @@ final class TaskService {     ) {         self.modelContext = modelContext         self.displayIDAllocator = displayIDAllocator+        self.fetcher = fetcher ?? modelContext         self.usedDisplayIDs = UsedDisplayIDs(             modelContext: modelContext,-            liveFetcher: fetcher ?? modelContext+            liveFetcher: self.fetcher         )         self.createSave = createSave         self.statusSave = statusSave@@ -332,7 +333,7 @@ final class TaskService {         let descriptor = FetchDescriptor<TransitTask>(             predicate: #Predicate { $0.id == id }         )-        guard let task = try modelContext.fetch(descriptor).first else {+        guard let task = try fetcher.fetch(descriptor).first else {             throw Error.taskNotFound         }         return task@@ -343,7 +344,7 @@ final class TaskService {         let descriptor = FetchDescriptor<TransitTask>(             predicate: #Predicate { $0.permanentDisplayId == displayId }         )-        let tasks = try modelContext.fetch(descriptor)+        let tasks = try fetcher.fetch(descriptor)          guard let first = tasks.first else {             throw Error.taskNotFound

diff --git a/Transit/TransitTests/MilestoneServiceMutationSaveTests.swift b/Transit/TransitTests/MilestoneServiceMutationSaveTests.swiftnew file mode 100644index 0000000..11972a6--- /dev/null+++ b/Transit/TransitTests/MilestoneServiceMutationSaveTests.swift@@ -0,0 +1,33 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++@MainActor @Suite(.serialized)+struct MilestoneServiceMutationSaveTests {++    @Test func setMilestoneRollsBackWhenInjectedSaveFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = Project(name: "Transit", description: "", gitRepo: nil, colorHex: "#000000")+        let milestone = Milestone(name: "Beta", description: nil, project: project, displayID: .permanent(1))+        let task = TransitTask(name: "Task", type: .feature, project: project, displayID: .permanent(1))+        StatusEngine.initializeNewTask(task)+        context.insert(project)+        context.insert(milestone)+        context.insert(task)+        try context.save()++        let service = MilestoneService(+            modelContext: context,+            displayIDAllocator: DisplayIDAllocator(store: InMemoryCounterStore()),+            mutationSave: { _ in throw SaveFailure.simulated }+        )++        #expect(throws: SaveFailure.self) {+            try service.setMilestone(milestone, on: task)+        }+        #expect(task.milestone == nil)+        #expect(!context.hasChanges)+    }+}

diff --git a/Transit/TransitTests/MutationIntentStorageFailureParityTests.swift b/Transit/TransitTests/MutationIntentStorageFailureParityTests.swiftnew file mode 100644index 0000000..6f6b8af--- /dev/null+++ b/Transit/TransitTests/MutationIntentStorageFailureParityTests.swift@@ -0,0 +1,83 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// T-1770 follow-up: shared milestone resolution used by task updates must preserve+/// typed not-found responses while mapping unreadable storage to INTERNAL_ERROR.+@MainActor @Suite(.serialized)+struct MutationIntentStorageFailureParityTests {++    private struct FetchFailure: Swift.Error, CustomStringConvertible {+        var description: String { "simulated milestone fetch failure" }+    }++    private struct FailingFetcher: ModelFetching {+        func fetch<T: PersistentModel>(_ descriptor: FetchDescriptor<T>) throws -> [T] {+            throw FetchFailure()+        }+    }++    private func allocator() -> DisplayIDAllocator {+        DisplayIDAllocator(store: InMemoryCounterStore())+    }++    private func makeTask(in context: ModelContext, project: Project) -> TransitTask {+        let task = TransitTask(name: "Task", type: .feature, project: project, displayID: .permanent(1))+        StatusEngine.initializeNewTask(task)+        context.insert(task)+        return task+    }++    private func makeProject(in context: ModelContext) -> Project {+        let project = Project(name: "Transit", description: "", gitRepo: nil, colorHex: "#000000")+        context.insert(project)+        return project+    }++    private func expectInternalError(_ result: String, hint: String) throws {+        let data = try #require(result.data(using: .utf8))+        let payload = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+        #expect(Set(payload.keys) == Set(["error", "hint"]))+        #expect(payload["error"] as? String == "INTERNAL_ERROR")+        #expect(payload["hint"] as? String == hint)+    }++    @Test func updateTaskReportsInternalErrorWhenMilestoneLookupFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let task = makeTask(in: context, project: project)+        try context.save()++        let result = UpdateTaskIntent.execute(+            input: "{\"displayId\":1,\"milestoneDisplayId\":1}",+            taskService: TaskService(modelContext: context, displayIDAllocator: allocator()),+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            )+        )++        try expectInternalError(result, hint: "Failed to look up milestone: simulated milestone fetch failure")+        #expect(task.milestone == nil)+    }++    @Test func sharedMilestoneAssignmentReportsInternalErrorWhenLookupFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let task = makeTask(in: context, project: project)+        try context.save()++        let result = IntentHelpers.assignMilestone(+            from: ["milestoneDisplayId": 1],+            to: task,+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            )+        )++        try expectInternalError(try #require(result), hint: "Failed to assign milestone")+        #expect(task.milestone == nil)+    }+}

diff --git a/Transit/TransitTests/MutationIntentStorageFailureTests.swift b/Transit/TransitTests/MutationIntentStorageFailureTests.swiftnew file mode 100644index 0000000..e862d1b--- /dev/null+++ b/Transit/TransitTests/MutationIntentStorageFailureTests.swift@@ -0,0 +1,255 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// T-1770: A failed storage read is not a caller error. These tests inject a+/// fetcher that fails while the backing context remains writable, then assert+/// every affected JSON mutation intent returns INTERNAL_ERROR without mutation.+@MainActor @Suite(.serialized)+struct MutationIntentStorageFailureTests {++    private struct FetchFailure: Swift.Error {}++    private struct FailingFetcher: ModelFetching {+        func fetch<T: PersistentModel>(_ descriptor: FetchDescriptor<T>) throws -> [T] {+            throw FetchFailure()+        }+    }++    private func allocator() -> DisplayIDAllocator {+        DisplayIDAllocator(store: InMemoryCounterStore())+    }++    @discardableResult+    private func makeProject(in context: ModelContext) -> Project {+        let project = Project(name: "Transit", description: "", gitRepo: nil, colorHex: "#000000")+        context.insert(project)+        return project+    }++    @discardableResult+    private func makeMilestone(+        in context: ModelContext, project: Project, displayID: Int = 1+    ) -> Milestone {+        let milestone = Milestone(name: "Beta", description: nil, project: project, displayID: .permanent(displayID))+        context.insert(milestone)+        return milestone+    }++    @discardableResult+    private func makeTask(+        in context: ModelContext, project: Project, displayID: Int = 1+    ) -> TransitTask {+        let task = TransitTask(name: "Task", type: .feature, project: project, displayID: .permanent(displayID))+        StatusEngine.initializeNewTask(task)+        context.insert(task)+        return task+    }++    private func expectInternalError(_ result: String) throws {+        let data = try #require(result.data(using: .utf8))+        let payload = try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+        #expect(Set(payload.keys) == Set(["error", "hint"]))+        #expect(payload["error"] as? String == "INTERNAL_ERROR")+    }++    @Test func createTaskReportsInternalErrorWhenMilestoneLookupFails() async throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        makeMilestone(in: context, project: project)+        try context.save()++        let result = await CreateTaskIntent.execute(+            input: """+            {"projectId":"\(project.id.uuidString)","name":"New task","type":"feature","milestoneDisplayId":1}+            """,+            taskService: TaskService(modelContext: context, displayIDAllocator: allocator()),+            projectService: ProjectService(modelContext: context),+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            )+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<TransitTask>()).isEmpty)+    }++    @Test func createMilestoneReportsInternalErrorWhenUniquenessFetchFails() async throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        try context.save()++        let result = await CreateMilestoneIntent.execute(+            input: "{\"projectId\":\"\(project.id.uuidString)\",\"name\":\"Beta\"}",+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            ),+            projectService: ProjectService(modelContext: context)+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<Milestone>()).isEmpty)+    }++    @Test func updateStatusReportsInternalErrorWhenTaskLookupFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let task = makeTask(in: context, project: project)+        try context.save()++        let result = UpdateStatusIntent.execute(+            input: "{\"displayId\":1,\"status\":\"planning\"}",+            taskService: TaskService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            )+        )++        try expectInternalError(result)+        #expect(task.status == .idea)+    }++    @Test func updateMilestoneReportsInternalErrorWhenMilestoneLookupFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let milestone = makeMilestone(in: context, project: project)+        try context.save()++        let result = UpdateMilestoneIntent.execute(+            input: "{\"displayId\":1,\"status\":\"done\"}",+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            ),+            projectService: ProjectService(modelContext: context)+        )++        try expectInternalError(result)+        #expect(milestone.status == .open)+    }++    @Test func deleteMilestoneReportsInternalErrorWhenMilestoneLookupFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let milestone = makeMilestone(in: context, project: project)+        try context.save()++        let result = DeleteMilestoneIntent.execute(+            input: "{\"displayId\":1}",+            milestoneService: MilestoneService(+                modelContext: context, displayIDAllocator: allocator(), fetcher: FailingFetcher()+            )+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<Milestone>()).map(\.id) == [milestone.id])+    }+}++extension MutationIntentStorageFailureTests {++    @Test func createTaskReportsInternalErrorAndDeletesPendingTaskWhenSaveFails() async throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        try context.save()++        let result = await CreateTaskIntent.execute(+            input: "{\"projectId\":\"\(project.id.uuidString)\",\"name\":\"New task\",\"type\":\"feature\"}",+            taskService: TaskService(+                modelContext: context,+                displayIDAllocator: allocator(),+                createSave: { _ in throw SaveFailure.simulated }+            ),+            projectService: ProjectService(modelContext: context)+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<TransitTask>()).isEmpty)+    }++    @Test func createMilestoneReportsInternalErrorAndDeletesPendingMilestoneWhenSaveFails() async throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        try context.save()++        let result = await CreateMilestoneIntent.execute(+            input: "{\"projectId\":\"\(project.id.uuidString)\",\"name\":\"Beta\"}",+            milestoneService: MilestoneService(+                modelContext: context,+                displayIDAllocator: allocator(),+                mutationSave: { _ in throw SaveFailure.simulated }+            ),+            projectService: ProjectService(modelContext: context)+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<Milestone>()).isEmpty)+    }++    @Test func updateStatusReportsInternalErrorAndRollsBackWhenSaveFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let task = makeTask(in: context, project: project)+        try context.save()++        let result = UpdateStatusIntent.execute(+            input: "{\"displayId\":1,\"status\":\"planning\"}",+            taskService: TaskService(+                modelContext: context,+                displayIDAllocator: allocator(),+                statusSave: { _ in throw SaveFailure.simulated }+            )+        )++        try expectInternalError(result)+        #expect(task.status == .idea)+    }++    @Test func updateMilestoneReportsInternalErrorAndRollsBackWhenSaveFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let milestone = makeMilestone(in: context, project: project)+        try context.save()++        let result = UpdateMilestoneIntent.execute(+            input: "{\"displayId\":1,\"status\":\"done\"}",+            milestoneService: MilestoneService(+                modelContext: context,+                displayIDAllocator: allocator(),+                mutationSave: { _ in throw SaveFailure.simulated }+            ),+            projectService: ProjectService(modelContext: context)+        )++        try expectInternalError(result)+        #expect(milestone.status == .open)+    }++    @Test func deleteMilestoneReportsInternalErrorAndRollsBackWhenSaveFails() throws {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = makeProject(in: context)+        let milestone = makeMilestone(in: context, project: project)+        try context.save()++        let result = DeleteMilestoneIntent.execute(+            input: "{\"displayId\":1}",+            milestoneService: MilestoneService(+                modelContext: context,+                displayIDAllocator: allocator(),+                mutationSave: { _ in throw SaveFailure.simulated }+            )+        )++        try expectInternalError(result)+        #expect(try context.fetch(FetchDescriptor<Milestone>()).map(\.id) == [milestone.id])+    }+}

diff --git a/specs/bugfixes/mutation-intent-persistence-errors/report.md b/specs/bugfixes/mutation-intent-persistence-errors/report.mdnew file mode 100644index 0000000..44383c1--- /dev/null+++ b/specs/bugfixes/mutation-intent-persistence-errors/report.md@@ -0,0 +1,113 @@+# Bugfix Report: Mutation Intents Misclassify Persistence Failures++**Date:** 2026-08-04+**Status:** Fixed+**Ticket:** T-1770++## Description of the Issue++Five JSON mutation App Intents treat a valid request that cannot be completed because SwiftData storage failed as caller-invalid input (or, for some lookup paths, as a missing record). Automation clients therefore cannot distinguish retryable storage failures from permanent request errors.++**Reproduction steps:**+1. Submit a valid create, update, or delete request to an affected intent.+2. Make the intent's save or required lookup fetch throw while the backing context remains otherwise usable.+3. Observe an `INVALID_INPUT` / not-found response rather than `INTERNAL_ERROR`.++**Impact:** Retry-capable callers can discard valid work after a transient storage or CloudKit failure, and the same domain failure has inconsistent semantics between mutation intents.++## Investigation Summary++### Phase 1: Initial overview++Expected behavior: JSON validation and typed domain errors remain their established public payloads; untyped persistence or storage errors return `INTERNAL_ERROR`. Actual behavior: `CreateMilestoneIntent`, `UpdateStatusIntent`, `UpdateMilestoneIntent`, and `DeleteMilestoneIntent` use generic catches that return input or not-found payloads. `CreateTaskIntent`'s final creation catch is already correctly mapped, but its display-ID milestone lookup can still turn a generic fetch failure into `MILESTONE_NOT_FOUND`.++### Phase 2: Systematic inspection++- `CreateTaskIntent`: creation errors are already mapped by typed `TaskService.Error` cases with an `INTERNAL_ERROR` default; its display-ID milestone lookup catches every non-`MilestoneService.Error` as not-found.+- `CreateMilestoneIntent`: typed `MilestoneService.Error` values use `IntentHelpers.mapMilestoneError`, but the generic creation catch returns `INVALID_INPUT`.+- `UpdateStatusIntent`: valid status updates catch every service failure as `INVALID_INPUT`.+- `UpdateMilestoneIntent`: its atomic `MilestoneService.save()` failure is caught as `INVALID_INPUT` after mutations have been applied; service recovery must roll them back.+- `DeleteMilestoneIntent`: deletion and untyped identifier-lookup failures are reported as input/not-found errors.+- `TaskService`: creation uses `insertOrDelete`, status updates call `safeRollback()` on any save failure.+- `MilestoneService`: creation uses `insertOrDelete`; save/update/delete use `saveOrRollback`, but it lacks a service-level injected saver and its direct lookup methods bypass the existing injected `ModelFetching` seam.+- `IntentHelpers.resolveTask` and `resolveMilestone` preserve typed not-found/duplicate/validation cases but currently collapse unexpected fetch errors into not-found.++### Phase 3: Root cause analysis++1. Why are callers told their request is invalid? Generic `catch` clauses select `invalidInput` or not-found payloads.+2. Why do generic catches receive storage errors? The services expose raw SwiftData fetch/save failures alongside typed domain errors.+3. Why are generic lookup failures not deterministic in tests? `TaskService.findBy…` and `MilestoneService.findBy…` read `modelContext` directly rather than their injectable fetchers.+4. Why cannot all milestone mutation saves be simulated through an intent? `MilestoneService` accepts a creation-only save closure and `save()` / `deleteMilestone()` use fixed context saves.+5. **Root cause:** Intent error translation conflates typed caller/domain failures with untyped infrastructure failures, and incomplete injection seams leave the distinction untested on every affected path.++**Assumptions validated:** The save helpers provide the required recovery: creates delete only the newly inserted model; updates/deletes `safeRollback()` and re-fault models. The fix must retain these helpers and change response translation/seam routing only.++## Resolution for the Issue++**Changes made:**+- Routed every untyped create/update/delete and direct identifier-lookup failure through `INTERNAL_ERROR`, with typed validation, not-found, duplicate, and domain catches retained ahead of the generic catch.+- Made `TaskService` and `MilestoneService` direct UUID/display-ID/name lookups use their injected `ModelFetching` instance.+- Added `MilestoneService.mutationSave` and a closure-based `ModelContext.saveOrRollback(save:_:)` overload, retaining `safeRollback()` for update/delete failures and `insertOrDelete` cleanup for creates.+- Added ten deterministic parity tests—one failing-fetch and one failing-save path for each affected intent—asserting a two-key `INTERNAL_ERROR` envelope and no persisted mutation.++**Approach rationale:** Reusing the established `UpdateTaskIntent` convention preserves the public error vocabulary and differentiates caller action from infrastructure state. The smallest safe change is at the existing service seam and each final error-translation boundary; rollback behavior remains in the service layer.++**Alternatives considered:**+- Convert all raw service errors into a new typed domain case — rejected because it widens the domain API while callers already have `INTERNAL_ERROR` for this exact condition.+- Leave lookup methods on `modelContext` and test only saves — rejected because generic fetch catches are part of the defect and would remain untestable.++## Regression Test++**Test file:** `Transit/TransitTests/MutationIntentStorageFailureTests.swift`++**What it verifies:** Every affected intent returns a two-key `INTERNAL_ERROR` envelope for a deterministic storage failure and does not create, update, or delete model state.++**Red run command:**+```bash+xcodebuild test -project Transit/Transit.xcodeproj -scheme Transit \+  -destination 'platform=macOS' \+  -only-testing:TransitTests/MutationIntentStorageFailureTests+```++## Affected Files++| File | Change |+|---|---|+| `Transit/Transit/Intents/CreateTaskIntent.swift` | Correct untyped milestone lookup mapping. |+| `Transit/Transit/Intents/CreateMilestoneIntent.swift` | Map generic creation failures to `INTERNAL_ERROR`. |+| `Transit/Transit/Intents/UpdateStatusIntent.swift` | Map generic status-save failures to `INTERNAL_ERROR`. |+| `Transit/Transit/Intents/UpdateMilestoneIntent.swift` | Map generic save failures to `INTERNAL_ERROR`. |+| `Transit/Transit/Intents/DeleteMilestoneIntent.swift` | Map generic delete/lookup failures to `INTERNAL_ERROR`. |+| `Transit/Transit/Intents/IntentHelpers.swift` | Preserve typed mappings and surface unexpected shared lookup errors as internal. |+| `Transit/Transit/Extensions/ModelContext+Save.swift` | Add injected-save rollback overload that preserves `safeRollback()` recovery. |+| `Transit/Transit/Services/TaskService.swift` | Use deterministic injected fetcher for direct lookup. |+| `Transit/Transit/Services/MilestoneService.swift` | Use deterministic injected fetcher and saver for mutation paths. |+| `Transit/TransitTests/MutationIntentStorageFailureTests.swift` | Add failure-path/no-mutation parity coverage. |++## Verification++**Automated:**+- [x] Regression suite fails before the fix: all five deterministic failing-fetch cases failed.+- [x] Regression suite passes after the fix: all ten fetch/save cases pass on macOS.+- [x] Affected intent/service/save-helper suites pass on macOS, including existing typed caller-error payload coverage.+- [x] macOS unit suite passes: `make test-quick`.+- [x] Linter/ownership guard passes: `make lint`.+- [x] Production builds pass: `make build` (iOS Simulator and macOS).+- [ ] Full `make test` did not complete before the 10-minute harness cap; it built successfully, started the new suite, and independently exposed the same three unrelated UI failures below.+- [ ] Focused iOS regression run did not complete before the 10-minute harness cap during simulator build/execution.+- [ ] `make test-ui` reproduces unrelated failures in `TransitUITests.testClearAll`, `TransitUITests.testEditViewPreservesTaskMilestone`, and `DataMaintenanceUITests.testDataMaintenanceGoldenPath`, then exceeds the cap during cleanup. No T-1770 code touches these UI flows.++**Manual verification:** Not required. The injection seams exercise the exact intent paths with a writable in-memory context and deterministic failures.++## Prevention++- Catch typed validation/domain errors before the generic catch; generic infrastructure errors must map to `INTERNAL_ERROR`.+- Direct service lookup methods must use their injectable fetch seam.+- Keep create recovery (`insertOrDelete`) distinct from update/delete recovery (`saveOrRollback` / `safeRollback`) and assert no mutation at the intent boundary.++## Related++- T-1770+- T-1768 — `CreateTaskIntent` creation failure semantics+- T-1614 / T-1621 — deterministic `ModelFetching` seams and fail-closed storage handling+- T-650 — `UpdateTaskIntent` infrastructure-error convention