Pre-push review: T-650 update_task extended to all fields

diff --git a/Transit/Transit/Intents/TaskUpdateValidator.swift b/Transit/Transit/Intents/TaskUpdateValidator.swiftnew file mode 100644index 0000000..f6a9e4a--- /dev/null+++ b/Transit/Transit/Intents/TaskUpdateValidator.swift@@ -0,0 +1,396 @@+import Foundation++/// Shared validator and applier for `update_task` across the MCP tool and+/// `UpdateTaskIntent`. Both surfaces parse JSON into `[String: Any]`, resolve+/// the target task, then call `validate` to produce a `ValidatedTaskUpdate`+/// and (if any changes are present) `apply` to mutate the task in memory.+/// Saving is the caller's responsibility so a single transaction covers+/// every mutation. [T-650]+@MainActor+enum TaskUpdateValidator {++    /// Walks the JSON args in a deterministic order (name → description → type+    /// → metadata → milestone) and produces a fully-validated update. Pure:+    /// never mutates `task` or the model context. Milestone resolution uses+    /// `milestoneService` for read-only lookups. The walk order is not part+    /// of the public contract (AC 5.2) — callers MUST NOT depend on which+    /// invalid field surfaces when multiple are invalid.+    static func validate(+        _ args: [String: Any],+        task: TransitTask,+        milestoneService: MilestoneService+    ) -> Result<ValidatedTaskUpdate, TaskUpdateValidationError> {+        // name+        let name: String?+        switch validateName(args) {+        case .failure(let error): return .failure(error)+        case .success(let value): name = value+        }++        // description+        let description: FieldChange<String>+        switch validateDescription(args) {+        case .failure(let error): return .failure(error)+        case .success(let value): description = value+        }++        // type+        let type: TaskType?+        switch validateType(args) {+        case .failure(let error): return .failure(error)+        case .success(let value): type = value+        }++        // metadata+        let metadata: FieldChange<[String: String]>+        switch strictStringMetadata(from: args["metadata"]) {+        case .failure(let error): return .failure(error)+        case .success(let value): metadata = value+        }++        // milestone+        let milestoneAction: MilestoneAction?+        switch validateMilestone(args, task: task, milestoneService: milestoneService) {+        case .failure(let error): return .failure(error)+        case .success(let value): milestoneAction = value+        }++        return .success(+            ValidatedTaskUpdate(+                name: name,+                description: description,+                type: type,+                metadata: metadata,+                milestoneAction: milestoneAction+            )+        )+    }++    /// Applies a validated update to the task in memory via the service layer.+    /// Calls `taskService.updateTask(..., save: false)` once with every+    /// non-milestone field and `milestoneService.setMilestone(..., save: false)`+    /// at most once. Throws service-layer errors only+    /// (`TaskService.Error` / `MilestoneService.Error`); validation errors are+    /// returned by `validate` and never thrown from `apply`. The caller is+    /// responsible for invoking `taskService.rollback()` to undo any partial+    /// in-memory mutation if a throw lands between the two service calls.+    static func apply(+        _ update: ValidatedTaskUpdate,+        to task: TransitTask,+        taskService: TaskService,+        milestoneService: MilestoneService+    ) throws {+        // Walk order: name → description → type → metadata → milestone.+        // Only call updateTask if at least one of its fields changes — otherwise+        // we'd issue a no-op service call that still adds nothing observable but+        // is wasted work.+        let (descArg, clearDesc): (String?, Bool) = switch update.description {+        case .noChange: (nil, false)+        case .set(let value): (value, false)+        case .clear: (nil, true)+        }++        let metadataArg: [String: String]? = switch update.metadata {+        case .noChange: nil+        case .set(let dict): dict+        case .clear: [:]+        }++        let hasFieldChange = update.name != nil+            || update.description.isChange+            || update.type != nil+            || update.metadata.isChange++        if hasFieldChange {+            try taskService.updateTask(+                task,+                name: update.name,+                description: descArg,+                clearDescription: clearDesc,+                type: update.type,+                metadata: metadataArg,+                save: false+            )+        }++        switch update.milestoneAction {+        case .none:+            break+        case .assign(let milestone):+            try milestoneService.setMilestone(milestone, on: task, save: false)+        case .clear:+            try milestoneService.setMilestone(nil, on: task, save: false)+        }+    }++    /// Strict metadata coercion. `JSONSerialization` delivers JSON numbers and+    /// booleans as `NSNumber`, which would silently pass a permissive+    /// `[String: String]` cast in some paths; this helper explicitly rejects+    /// any non-string value. Co-located with the validator because its only+    /// caller is `validate` and its error type is feature-specific.+    static func strictStringMetadata(+        from value: Any?+    ) -> Result<FieldChange<[String: String]>, TaskUpdateValidationError> {+        guard let value else {+            return .success(.noChange)+        }++        // Native [String: String] from Swift callers and tests.+        if let strict = value as? [String: String] {+            return .success(strict.isEmpty ? .clear : .set(strict))+        }++        // JSONSerialization-style [String: Any]: strict-cast each value.+        if let dict = value as? [String: Any] {+            if dict.isEmpty {+                return .success(.clear)+            }+            var coerced: [String: String] = [:]+            coerced.reserveCapacity(dict.count)+            for (key, raw) in dict {+                guard let stringValue = raw as? String else {+                    return .failure(.invalidInput("metadata values must be strings"))+                }+                coerced[key] = stringValue+            }+            return .success(.set(coerced))+        }++        return .failure(.invalidInput("metadata must be an object with string values"))+    }++    // MARK: - Field Validators++    private static func validateName(+        _ args: [String: Any]+    ) -> Result<String?, TaskUpdateValidationError> {+        guard let raw = args["name"] else { return .success(nil) }+        guard let str = raw as? String else {+            return .failure(.invalidInput("name must be a string"))+        }+        let trimmed = str.trimmingCharacters(in: .whitespacesAndNewlines)+        guard !trimmed.isEmpty else {+            return .failure(.invalidInput("Task name cannot be empty"))+        }+        return .success(trimmed)+    }++    private static func validateDescription(+        _ args: [String: Any]+    ) -> Result<FieldChange<String>, TaskUpdateValidationError> {+        guard let raw = args["description"] else { return .success(.noChange) }+        guard let str = raw as? String else {+            return .failure(.invalidInput("description must be a string"))+        }+        let trimmed = str.trimmingCharacters(in: .whitespacesAndNewlines)+        return .success(trimmed.isEmpty ? .clear : .set(trimmed))+    }++    private static func validateType(+        _ args: [String: Any]+    ) -> Result<TaskType?, TaskUpdateValidationError> {+        guard let raw = args["type"] else { return .success(nil) }+        guard let str = raw as? String else {+            return .failure(.invalidInput("type must be a string"))+        }+        guard let type = TaskType(rawValue: str) else {+            let valid = TaskType.allCases.map(\.rawValue).joined(separator: ", ")+            return .failure(.invalidInput("Invalid type: \(str). Must be one of: \(valid)"))+        }+        return .success(type)+    }++    /// Mirrors the precedence in the existing MCP handler and+    /// `UpdateTaskIntent.applyMilestoneChange`: clearMilestone → milestoneDisplayId → milestone (name).+    /// `clearMilestone: true` is emitted as `.clear` unconditionally — even when+    /// the task is already unassigned — to preserve existing handler behavior+    /// (the save is a no-op for the milestone field but the request is still a+    /// "change" for hasChanges purposes; see Phase 3 test+    /// `clearMilestone_onAlreadyUnassigned_savesAnyway`).+    private static func validateMilestone(+        _ args: [String: Any],+        task: TransitTask,+        milestoneService: MilestoneService+    ) -> Result<MilestoneAction?, TaskUpdateValidationError> {+        // clearMilestone (boolean) takes precedence+        if args.keys.contains("clearMilestone") {+            guard let clear = IntentHelpers.parseBoolValue(args["clearMilestone"]) else {+                return .failure(.invalidInput("clearMilestone must be a boolean"))+            }+            if clear {+                return .success(.clear)+            }+            // clearMilestone: false → fall through, but only resolve other milestone+            // fields if they are present (matches existing behavior).+        }++        if args["milestoneDisplayId"] != nil {+            return resolveMilestoneByDisplayId(args, task: task, milestoneService: milestoneService)+        }++        if args["milestone"] != nil {+            return resolveMilestoneByName(args, task: task, milestoneService: milestoneService)+        }++        return .success(nil)+    }++    private static func resolveMilestoneByDisplayId(+        _ args: [String: Any],+        task: TransitTask,+        milestoneService: MilestoneService+    ) -> Result<MilestoneAction?, TaskUpdateValidationError> {+        guard let displayId = IntentHelpers.parseIntValue(args["milestoneDisplayId"]) else {+            return .failure(.invalidInput("milestoneDisplayId must be an integer"))+        }+        do {+            let milestone = try milestoneService.findByDisplayID(displayId)+            return projectMatched(milestone: milestone, task: task)+        } catch MilestoneService.Error.duplicateDisplayID {+            return .failure(.duplicateMilestoneDisplayID(+                message: "Duplicate milestone identifier detected for displayId \(displayId)"+            ))+        } catch {+            return .failure(.milestoneNotFound(+                message: "No milestone with displayId \(displayId)"+            ))+        }+    }++    private static func resolveMilestoneByName(+        _ args: [String: Any],+        task: TransitTask,+        milestoneService: MilestoneService+    ) -> Result<MilestoneAction?, TaskUpdateValidationError> {+        guard let name = args["milestone"] as? String else {+            return .failure(.invalidInput("milestone must be a string"))+        }+        guard let project = task.project else {+            return .failure(.projectRequiredForMilestone)+        }+        guard let milestone = milestoneService.findByName(name, in: project) else {+            return .failure(.milestoneNotFound(+                message: "No milestone named '\(name)' in project '\(project.name)'"+            ))+        }+        return projectMatched(milestone: milestone, task: task)+    }++    /// Validates that a resolved milestone's project matches the task's project.+    /// Throws-via-Result so the caller can surface either `.milestoneProjectMismatch`+    /// or `.projectRequiredForMilestone` depending on which precondition fails.+    private static func projectMatched(+        milestone: Milestone,+        task: TransitTask+    ) -> Result<MilestoneAction?, TaskUpdateValidationError> {+        guard let taskProject = task.project else {+            return .failure(.projectRequiredForMilestone)+        }+        guard milestone.project?.id == taskProject.id else {+            return .failure(.milestoneProjectMismatch)+        }+        return .success(.assign(milestone))+    }+}++// MARK: - Value Types++/// A pure data carrier for a fully-validated update. Field representation+/// makes invalid states unrepresentable:+/// - Non-clearable fields (`name`, `type`) use `Optional<T>` — `nil` = no change.+/// - Clearable fields (`description`, `metadata`) use `FieldChange<T>` to+///   distinguish "set" from "clear" cleanly.+/// - Milestone uses its own enum because "assign" carries a `Milestone`+///   instance that resolution has already located.+struct ValidatedTaskUpdate {+    let name: String?+    let description: FieldChange<String>+    let type: TaskType?+    let metadata: FieldChange<[String: String]>+    let milestoneAction: MilestoneAction?++    var hasChanges: Bool {+        name != nil+            || description.isChange+            || type != nil+            || metadata.isChange+            || milestoneAction != nil+    }+}++/// Represents a single field's update intent. `noChange` means the field was+/// omitted from the request; `set(T)` carries a validated value; `clear`+/// signals an explicit clear (e.g. `description: ""` or `metadata: {}`).+enum FieldChange<T> {+    case noChange+    case set(T)+    case clear++    var isChange: Bool {+        if case .noChange = self { false } else { true }+    }+}++/// Milestone update intent. `assign(Milestone)` carries the already-resolved+/// milestone so `apply` does not need to re-look it up.+///+/// `clear` is emitted unconditionally whenever `clearMilestone: true` is in the+/// request args, even when the task already has no milestone assigned. This+/// preserves the existing MCP handler's behavior (the save is a no-op for the+/// milestone field but still counts as a "change" for `hasChanges` purposes).+enum MilestoneAction {+    case assign(Milestone)+    case clear+}++// MARK: - Errors++/// Structured validation/resolution errors emitted by `TaskUpdateValidator`.+/// Each surface (MCP / App Intent) renders these into its own error envelope+/// via `mcpMessage` or `intentError`. Error-message text is allowed to differ+/// across surfaces (AC 5.2 carve-out).+enum TaskUpdateValidationError: Error {+    case invalidInput(String)+    case milestoneNotFound(message: String)+    case duplicateMilestoneDisplayID(message: String)+    case milestoneProjectMismatch+    case projectRequiredForMilestone++    /// Message string used by the MCP surface's `errorResult(...)`. Mirrors+    /// the literals previously hard-coded in `MCPToolHandler.handleUpdateTask`+    /// for milestone errors so existing tests and callers see no change.+    var mcpMessage: String {+        switch self {+        case .invalidInput(let message),+             .milestoneNotFound(let message),+             .duplicateMilestoneDisplayID(let message):+            return message+        case .milestoneProjectMismatch:+            return "Milestone and task must belong to the same project"+        case .projectRequiredForMilestone:+            return "Task must belong to a project before assigning a milestone"+        }+    }++    /// Projection to `IntentError` for the App Intent surface. Mirrors the+    /// mapping previously in `UpdateTaskIntent.applyMilestoneChange` and+    /// `IntentHelpers.mapMilestoneError`.+    var intentError: IntentError {+        switch self {+        case .invalidInput(let hint):+            return .invalidInput(hint: hint)+        case .milestoneNotFound(let message):+            return .milestoneNotFound(hint: message)+        case .duplicateMilestoneDisplayID(let message):+            return .internalError(hint: message)+        case .milestoneProjectMismatch:+            return .milestoneProjectMismatch(+                hint: "Milestone and task must belong to the same project"+            )+        case .projectRequiredForMilestone:+            return .invalidInput(+                hint: "Task must belong to a project before assigning a milestone"+            )+        }+    }+}

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex f3aec33..6bb8886 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -783,8 +783,17 @@ extension MCPToolHandler {  extension MCPToolHandler { -    // swiftlint:disable:next cyclomatic_complexity function_body_length+    /// Updates one or more mutable fields on a task in a single atomic call.+    ///+    /// Field validation, milestone resolution, and applier logic are delegated+    /// to `TaskUpdateValidator` so that the MCP tool and `UpdateTaskIntent`+    /// share identical semantics. The identifier-resolution preamble preserves+    /// the existing T-634/T-808 behavior — present-but-malformed identifiers+    /// surface as field-specific INVALID_INPUT messages, not as a generic+    /// not-found. The response shape is built by+    /// `IntentHelpers.taskUpdateResponseDict` (AC 9.1).     private func handleUpdateTask(_ args: [String: Any]) -> MCPToolResult {+        // Identifier resolution (preserve existing T-634 / T-808 behavior)         // Reject non-integer displayId when key is present [T-634]         if args["displayId"] != nil, IntentHelpers.parseIntValue(args["displayId"]) == nil {             return errorResult("displayId must be an integer")@@ -801,69 +810,43 @@ extension MCPToolHandler {             return errorResult("Provide either displayId (integer) or taskId (UUID string)")         } -        // Handle milestone assignment (save: false — deferred to single atomic save below)-        // Reject non-integer milestoneDisplayId when key is present [T-613]-        if args["milestoneDisplayId"] != nil, IntentHelpers.parseIntValue(args["milestoneDisplayId"]) == nil {-            return errorResult("milestoneDisplayId must be an integer")+        // Validate every field before applying any change. The validator is+        // pure — no mutations occur on success or failure, so an early return+        // here leaves the task untouched.+        let update: ValidatedTaskUpdate+        switch TaskUpdateValidator.validate(args, task: task, milestoneService: milestoneService) {+        case .success(let validated):+            update = validated+        case .failure(let error):+            return errorResult(error.mcpMessage)         }-        // Reject non-boolean clearMilestone when key is present [T-1060]-        let clearMilestoneValue: Bool?-        if args.keys.contains("clearMilestone") {-            guard let parsed = IntentHelpers.parseBoolValue(args["clearMilestone"]) else {-                return errorResult("clearMilestone must be a boolean")-            }-            clearMilestoneValue = parsed-        } else {-            clearMilestoneValue = nil++        // No-op echo: when the request includes only an identifier (and no+        // mutating field), skip the save and return the current task JSON.+        guard update.hasChanges else {+            return textResult(IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task)))         }-        if clearMilestoneValue == true {-            do {-                try milestoneService.setMilestone(nil, on: task, save: false)-            } catch {-                return errorResult("Failed to clear milestone: \(error)")-            }-        } else if let milestoneDisplayId = IntentHelpers.parseIntValue(args["milestoneDisplayId"]) {-            do {-                let milestone = try milestoneService.findByDisplayID(milestoneDisplayId)-                try milestoneService.setMilestone(milestone, on: task, save: false)-            } catch MilestoneService.Error.milestoneNotFound {-                return errorResult("No milestone with displayId \(milestoneDisplayId)")-            } catch MilestoneService.Error.duplicateDisplayID {-                return errorResult("Duplicate milestone identifier detected for displayId \(milestoneDisplayId)")-            } catch MilestoneService.Error.projectMismatch {-                return errorResult("Milestone and task must belong to the same project")-            } catch MilestoneService.Error.projectRequired {-                return errorResult("Task must belong to a project before assigning a milestone")-            } catch {-                return errorResult("Failed to set milestone: \(error)")-            }-        } else if args["milestone"] != nil {-            // Reject non-string milestone values [T-1114]. See handleCreateTask-            // for the same guard.-            guard let milestoneName = args["milestone"] as? String else {-                return errorResult("milestone must be a string")-            }-            guard let project = task.project else {-                return errorResult("Task must belong to a project before assigning a milestone")-            }-            guard let milestone = milestoneService.findByName(milestoneName, in: project) else {-                return errorResult("No milestone named '\(milestoneName)' in project '\(project.name)'")-            }-            do {-                try milestoneService.setMilestone(milestone, on: task, save: false)-            } catch {-                return errorResult("Failed to set milestone: \(error)")-            }++        // Apply in memory. If a service call throws between the two underlying+        // service calls (`updateTask` then `setMilestone`), explicitly roll+        // back so any partial mutation does not leak into the saved state.+        do {+            try TaskUpdateValidator.apply(+                update, to: task, taskService: taskService, milestoneService: milestoneService+            )+        } catch {+            taskService.rollback()+            return errorResult("Update failed: \(error)")         } +        // Save. `TaskService.save()` already calls `safeRollback()` on failure.         do {             try taskService.save()         } catch {-            return errorResult("Failed to save: \(error)")+            return errorResult("Update failed: \(error)")         } -        let formatter = ISO8601DateFormatter()-        return textResult(IntentHelpers.encodeJSON(taskToDict(task, formatter: formatter)))+        return textResult(IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task)))     } } 

diff --git a/Transit/Transit/MCP/MCPToolDefinitions.swift b/Transit/Transit/MCP/MCPToolDefinitions.swiftindex 58f602e..5c26953 100644--- a/Transit/Transit/MCP/MCPToolDefinitions.swift+++ b/Transit/Transit/MCP/MCPToolDefinitions.swift@@ -226,7 +226,7 @@ nonisolated enum MCPToolDefinitions {     )      // swiftlint:disable:next line_length-    private static let updateTaskDescription = "Update a task's properties. Currently supports milestone assignment. Identify task by displayId or taskId."+    private static let updateTaskDescription = "Update a task's mutable fields (name, description, type, metadata, milestone) in a single atomic call. Identify task by displayId or taskId."      static let updateTask = MCPToolDefinition(         name: "update_task",@@ -235,6 +235,17 @@ nonisolated enum MCPToolDefinitions {             properties: [                 "displayId": .integer("Task display ID (e.g. 42 for T-42)"),                 "taskId": .string("Task UUID"),+                "name": .string("New task name (trimmed; must be non-empty after trim)"),+                "description": .string(+                    "New task description. Pass \"\" or whitespace-only to clear."+                ),+                "type": .stringEnum(+                    "New task type (exact lowercase match required)",+                    values: TaskType.allCases.map(\.rawValue)+                ),+                "metadata": .object(+                    "Replaces the entire metadata dictionary. Pass {} to clear all metadata. Values must be strings."+                ),                 "milestone": .string("Milestone name (within task's project). Use clearMilestone to unassign."),                 "milestoneDisplayId": .integer("Milestone display ID (e.g. 3 for M-3, takes precedence over name)"),                 "clearMilestone": .boolean("Set to true to remove milestone assignment")

diff --git a/Transit/Transit/Intents/UpdateTaskIntent.swift b/Transit/Transit/Intents/UpdateTaskIntent.swiftindex 8e3f324..9b147b3 100644--- a/Transit/Transit/Intents/UpdateTaskIntent.swift+++ b/Transit/Transit/Intents/UpdateTaskIntent.swift@@ -1,26 +1,55 @@ import AppIntents import Foundation -/// Updates a task's properties (currently milestone assignment) via JSON input.+/// Updates a task's mutable fields (name, description, type, metadata, milestone) via JSON input. /// Exposed as "Transit: Update Task" in Shortcuts. [req 13.5]+///+/// Field validation, milestone resolution, and applier logic are delegated to+/// `TaskUpdateValidator` so this intent shares identical semantics with the MCP+/// `update_task` tool. The response shape is built by+/// `IntentHelpers.taskUpdateResponseDict` (AC 9.1). struct UpdateTaskIntent: AppIntent {     nonisolated(unsafe) static var title: LocalizedStringResource = "Transit: Update Task"      nonisolated(unsafe) static var description = IntentDescription(-        "Update a task's properties. Currently supports milestone assignment.",+        "Update a task's mutable fields (name, description, type, metadata, milestone) in a single atomic call.",         categoryName: "Tasks",         resultValueName: "Task JSON"     )      nonisolated(unsafe) static var openAppWhenRun: Bool = true +    /// Prose for the `input` @Parameter description. Exposed as a static so unit+    /// tests can assert on its content without reflecting over `@Parameter`+    /// metadata. The @Parameter macro requires a literal `LocalizedStringResource`+    /// at compile time, so the same text is duplicated in the @Parameter+    /// declaration below. Keep the two in sync. [Test seam: T-650 AC 8.2]+    nonisolated static let inputParameterDescription: String = """+    JSON object with a task identifier and optional update fields. \+    Identify task with "displayId" (integer) or "taskId" (UUID). \+    Update fields (omit any to preserve): \+    "name" (string, trimmed, non-empty); \+    "description" (string, trimmed; pass "" or whitespace-only to clear); \+    "type" (string, one of: bug, feature, chore, research, documentation); \+    "metadata" (object with string values; replaces entire metadata dict; pass {} to clear all). \+    Milestone fields: "milestoneDisplayId" (integer), "milestone" (name within task's project), \+    or "clearMilestone" (boolean, true to remove). \+    Example: {"displayId": 42, "name": "Rename", "description": "new", "metadata": {"k": "v"}}+    """+     @Parameter(         title: "Input JSON",         description: """-        JSON object with a task identifier and update fields. Identify task with "displayId" (integer) \-        or "taskId" (UUID). Milestone assignment: "milestoneDisplayId" (integer), "milestone" (name within \-        task's project), or "clearMilestone" (boolean, true to remove). \-        Example: {"displayId": 42, "milestoneDisplayId": 1}+        JSON object with a task identifier and optional update fields. \+        Identify task with "displayId" (integer) or "taskId" (UUID). \+        Update fields (omit any to preserve): \+        "name" (string, trimmed, non-empty); \+        "description" (string, trimmed; pass "" or whitespace-only to clear); \+        "type" (string, one of: bug, feature, chore, research, documentation); \+        "metadata" (object with string values; replaces entire metadata dict; pass {} to clear all). \+        Milestone fields: "milestoneDisplayId" (integer), "milestone" (name within task's project), \+        or "clearMilestone" (boolean, true to remove). \+        Example: {"displayId": 42, "name": "Rename", "description": "new", "metadata": {"k": "v"}}         """     )     var input: String@@ -58,65 +87,63 @@ struct UpdateTaskIntent: AppIntent {             return IntentError.invalidInput(hint: "Expected valid JSON object").json         } +        // Preflight identifier guard. "No task identifier provided" is structurally+        // different from "identifier provided but no match" — the former is+        // INVALID_INPUT, the latter is TASK_NOT_FOUND. Without this guard,+        // `taskService.resolveTask` would throw `.taskNotFound` for both. [T-650]+        let hasIdentifier = json["taskId"] != nil || json["displayId"] != nil+        guard hasIdentifier else {+            return IntentError.invalidInput(+                hint: "Provide either displayId (integer) or taskId (UUID string)"+            ).json+        }+         let task: TransitTask         switch IntentHelpers.resolveTask(from: json, taskService: taskService) {         case .success(let found): task = found         case .failure(let error): return error.json         } -        if let error = applyMilestoneChange(json, task: task, milestoneService: milestoneService) {-            return error+        // Validate every field before applying any change. The validator is+        // pure — no mutations occur on success or failure, so an early return+        // here leaves the task untouched.+        let update: ValidatedTaskUpdate+        switch TaskUpdateValidator.validate(json, task: task, milestoneService: milestoneService) {+        case .success(let validated):+            update = validated+        case .failure(let error):+            return error.intentError.json         } -        return buildResponse(task)-    }--    // MARK: - Private Helpers--    @MainActor-    private static func applyMilestoneChange(-        _ json: [String: Any],-        task: TransitTask,-        milestoneService: MilestoneService-    ) -> String? {-        // T-1060: When the key is present, require a real boolean. Strings, numbers,-        // and null must be rejected instead of silently treated as absent.-        if json.keys.contains("clearMilestone") {-            guard let clearMilestone = IntentHelpers.parseBoolValue(json["clearMilestone"]) else {-                return IntentError.invalidInput(hint: "clearMilestone must be a boolean").json-            }-            if clearMilestone {-                do {-                    try milestoneService.setMilestone(nil, on: task)-                } catch let error as MilestoneService.Error {-                    return IntentHelpers.mapMilestoneError(error).json-                } catch {-                    return IntentError.invalidInput(hint: "Failed to clear milestone").json-                }-                return nil-            }+        // No-op echo: when the request includes only an identifier (and no+        // mutating field), skip the save and return the current task JSON.+        guard update.hasChanges else {+            return IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task))         }-        return IntentHelpers.assignMilestone(from: json, to: task, milestoneService: milestoneService)-    } -    @MainActor-    private static func buildResponse(_ task: TransitTask) -> String {-        var response: [String: Any] = [-            "taskId": task.id.uuidString,-            "name": task.name,-            "status": task.statusRawValue,-            "type": task.typeRawValue-        ]-        if let displayId = task.permanentDisplayId {-            response["displayId"] = displayId+        // Apply in memory. If a service call throws between the two underlying+        // service calls (`updateTask` then `setMilestone`), explicitly roll+        // back so any partial mutation does not leak into the saved state.+        do {+            try TaskUpdateValidator.apply(+                update, to: task, taskService: taskService, milestoneService: milestoneService+            )+        } catch {+            taskService.rollback()+            return IntentError.internalError(+                hint: "Update failed: \(error.localizedDescription)"+            ).json         }-        if let project = task.project {-            response["projectId"] = project.id.uuidString-            response["projectName"] = project.name-        }-        if let milestone = task.milestone {-            response["milestone"] = IntentHelpers.milestoneInfoDict(milestone)++        // Save. `TaskService.save()` already calls `safeRollback()` on failure.+        do {+            try taskService.save()+        } catch {+            return IntentError.internalError(+                hint: "Update failed: \(error.localizedDescription)"+            ).json         }-        return IntentHelpers.encodeJSON(response)++        return IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task))     } }

diff --git a/Transit/Transit/Intents/IntentHelpers.swift b/Transit/Transit/Intents/IntentHelpers.swiftindex 44aa815..40fef1f 100644--- a/Transit/Transit/Intents/IntentHelpers.swift+++ b/Transit/Transit/Intents/IntentHelpers.swift@@ -291,6 +291,47 @@ nonisolated enum IntentHelpers {         return dict     } +    /// Builds the response dictionary for `update_task` post-update.+    ///+    /// Implements AC 9.1 of the T-650 spec: the shape is computed purely from+    /// the task's current model state, never from the request payload. Always+    /// includes `taskId`, `name`, `type`, and `status`. Includes `displayId`,+    /// `projectId`, `projectName`, `description`, `metadata`, and `milestone`+    /// only when their post-update value is present and non-empty. Excludes+    /// `comments`, `creationDate`, `lastStatusChangeDate`, and `completionDate`.+    ///+    /// `task.taskDescription` is read through an `if let` + non-empty guard so+    /// that a nil or empty stored value is omitted from the dictionary — using+    /// `task.taskDescription as Any` would encode nil as `NSNull`, which would+    /// violate the AC's omission rule. [T-650]+    @MainActor+    static func taskUpdateResponseDict(_ task: TransitTask) -> [String: Any] {+        var dict: [String: Any] = [+            "taskId": task.id.uuidString,+            "name": task.name,+            "type": task.typeRawValue,+            "status": task.statusRawValue+        ]+        if let displayId = task.permanentDisplayId {+            dict["displayId"] = displayId+        }+        if let project = task.project {+            dict["projectId"] = project.id.uuidString+            dict["projectName"] = project.name+        }+        if let description = task.taskDescription, !description.isEmpty {+            dict["description"] = description+        }+        let metadata = task.metadata+        if !metadata.isEmpty {+            dict["metadata"] = metadata+        }+        if let milestone = task.milestone {+            dict["milestone"] = milestoneInfoDict(milestone)+        }+        return dict+    }+     /// Builds a milestone info dictionary for inclusion in task responses.     @MainActor     static func milestoneInfoDict(_ milestone: Milestone) -> [String: Any] {

diff --git a/Transit/Transit/Services/TaskService.swift b/Transit/Transit/Services/TaskService.swiftindex 7f6cbbf..ed3e7ed 100644--- a/Transit/Transit/Services/TaskService.swift+++ b/Transit/Transit/Services/TaskService.swift@@ -354,4 +354,12 @@ final class TaskService {             throw error         }     }++    /// Reverts in-memory mutations on the model context. Used by handlers to+    /// undo partial changes when a multi-step update (e.g. updateTask followed+    /// by setMilestone) throws between steps and the caller wants to abandon+    /// the transaction before save. [T-650]+    func rollback() {+        modelContext.safeRollback()+    } }

diff --git a/Transit/TransitTests/TaskUpdateValidatorTests.swift b/Transit/TransitTests/TaskUpdateValidatorTests.swiftnew file mode 100644index 0000000..e0447c0--- /dev/null+++ b/Transit/TransitTests/TaskUpdateValidatorTests.swift@@ -0,0 +1,708 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++// swiftlint:disable file_length+// swiftlint:disable type_body_length++/// Tests for `TaskUpdateValidator.validate`, `.apply`, and `.strictStringMetadata`.+/// Mirrors the structure of `UpdateTaskIntentTests` (Swift Testing,+/// `@MainActor @Suite(.serialized)`). See specs/update-task-all-fields/design.md+/// for the validator contract and AC references. [T-650]+@MainActor @Suite(.serialized)+struct TaskUpdateValidatorTests {++    // MARK: - Fixture++    private struct Services {+        let task: TaskService+        let milestone: MilestoneService+        let project: ProjectService+        let context: ModelContext+    }++    private func makeServices() throws -> Services {+        let context = try TestModelContainer.newContext()+        let taskStore = InMemoryCounterStore()+        let taskAllocator = DisplayIDAllocator(store: taskStore)+        let milestoneStore = InMemoryCounterStore()+        let milestoneAllocator = DisplayIDAllocator(store: milestoneStore)+        return Services(+            task: TaskService(modelContext: context, displayIDAllocator: taskAllocator),+            milestone: MilestoneService(modelContext: context, displayIDAllocator: milestoneAllocator),+            project: ProjectService(modelContext: context),+            context: context+        )+    }++    @discardableResult+    private func makeProject(in context: ModelContext, name: String = "Test Project") -> Project {+        let project = Project(name: name, description: "A test project", gitRepo: nil, colorHex: "#FF0000")+        context.insert(project)+        return project+    }++    private func makeTask(+        in context: ModelContext,+        name: String = "Task 1",+        description: String? = nil,+        type: TaskType = .feature,+        project: Project,+        milestone: Milestone? = nil,+        metadata: [String: String]? = nil,+        displayId: Int = 10+    ) -> TransitTask {+        let task = TransitTask(+            name: name,+            description: description,+            type: type,+            project: project,+            displayID: .permanent(displayId),+            metadata: metadata+        )+        StatusEngine.initializeNewTask(task)+        task.milestone = milestone+        context.insert(task)+        return task+    }++    @discardableResult+    private func makeMilestone(+        in context: ModelContext,+        name: String,+        project: Project,+        displayId: Int+    ) -> Milestone {+        let milestone = Milestone(name: name, description: nil, project: project, displayID: .permanent(displayId))+        context.insert(milestone)+        return milestone+    }++    /// Round-trips a Swift dictionary through JSONSerialization so non-string+    /// values (numbers, booleans) materialize as NSNumber, matching what+    /// `IntentHelpers.parseJSON` delivers to handlers. Used to exercise the+    /// strict metadata path which must reject NSNumber values.+    private func jsonRoundTrip(_ json: String) throws -> [String: Any] {+        let data = try #require(json.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    private func unwrapSuccess(+        _ result: Result<ValidatedTaskUpdate, TaskUpdateValidationError>,+        sourceLocation: SourceLocation = #_sourceLocation+    ) throws -> ValidatedTaskUpdate {+        switch result {+        case .success(let value): return value+        case .failure(let error):+            Issue.record("Expected success, got failure: \(error)", sourceLocation: sourceLocation)+            throw TestFailure.unexpectedFailure+        }+    }++    private func unwrapFailure(+        _ result: Result<ValidatedTaskUpdate, TaskUpdateValidationError>,+        sourceLocation: SourceLocation = #_sourceLocation+    ) throws -> TaskUpdateValidationError {+        switch result {+        case .success(let value):+            Issue.record("Expected failure, got success: \(value)", sourceLocation: sourceLocation)+            throw TestFailure.unexpectedSuccess+        case .failure(let error): return error+        }+    }++    private enum TestFailure: Error { case unexpectedSuccess; case unexpectedFailure }++    private func isInvalidInput(_ error: TaskUpdateValidationError, contains substring: String? = nil) -> Bool {+        if case .invalidInput(let message) = error {+            guard let substring else { return true }+            return message.contains(substring)+        }+        return false+    }++    // MARK: - Name (AC 1.1–1.4)++    @Test func validate_nameSet_trimsAndApplies() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["name": "  hello  "], task: task, milestoneService: svc.milestone+        )+        let update = try unwrapSuccess(result)+        #expect(update.name == "hello")+        #expect(update.hasChanges == true)+    }++    @Test func validate_nameEmpty_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["name": ""], task: task, milestoneService: svc.milestone+        )+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error), "Expected .invalidInput, got \(error)")+    }++    @Test func validate_nameWhitespaceOnly_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["name": "   "], task: task, milestoneService: svc.milestone+        )+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error), "Expected .invalidInput, got \(error)")+    }++    @Test func validate_nameNonString_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        // JSON `42` round-trips as NSNumber, which would fail `as? String`.+        let args = try jsonRoundTrip("{\"name\": 42}")+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error, contains: "name"), "Expected name-specific invalidInput, got \(error)")+    }++    // MARK: - Description (AC 2.1–2.4)++    @Test func validate_descriptionSet_trimsAndApplies() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["description": "  text  "], task: task, milestoneService: svc.milestone+        )+        let update = try unwrapSuccess(result)+        guard case .set(let value) = update.description else {+            Issue.record("Expected .set, got \(update.description)")+            return+        }+        #expect(value == "text")+        #expect(update.hasChanges == true)+    }++    @Test func validate_descriptionEmpty_clears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["description": ""], task: task, milestoneService: svc.milestone+        )+        let update = try unwrapSuccess(result)+        guard case .clear = update.description else {+            Issue.record("Expected .clear, got \(update.description)")+            return+        }+        #expect(update.hasChanges == true)+    }++    @Test func validate_descriptionWhitespace_clears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["description": "   "], task: task, milestoneService: svc.milestone+        )+        let update = try unwrapSuccess(result)+        guard case .clear = update.description else {+            Issue.record("Expected .clear, got \(update.description)")+            return+        }+        #expect(update.hasChanges == true)+    }++    @Test func validate_descriptionNonString_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args = try jsonRoundTrip("{\"description\": 42}")+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        #expect(+            isInvalidInput(error, contains: "description"),+            "Expected description-specific invalidInput, got \(error)"+        )+    }++    // MARK: - Type (AC 3.1–3.4)++    @Test func validate_typeValid_setsLowercase() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, type: .bug, project: project)++        let result = TaskUpdateValidator.validate(+            ["type": "feature"], task: task, milestoneService: svc.milestone+        )+        let update = try unwrapSuccess(result)+        #expect(update.type == .feature)+        #expect(update.hasChanges == true)+    }++    @Test func validate_typeInvalid_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["type": "epic"], task: task, milestoneService: svc.milestone+        )+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error), "Expected .invalidInput, got \(error)")+    }++    @Test func validate_typeNonString_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args = try jsonRoundTrip("{\"type\": 1}")+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error, contains: "type"), "Expected type-specific invalidInput, got \(error)")+    }++    // MARK: - Metadata (AC 4.1–4.4)++    @Test func validate_metadataDict_replaces() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args: [String: Any] = ["metadata": ["a": "1", "b": "2"]]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        guard case .set(let dict) = update.metadata else {+            Issue.record("Expected .set, got \(update.metadata)")+            return+        }+        #expect(dict == ["a": "1", "b": "2"])+        #expect(update.hasChanges == true)+    }++    @Test func validate_metadataEmptyDict_clears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args: [String: Any] = ["metadata": [String: String]()]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        guard case .clear = update.metadata else {+            Issue.record("Expected .clear, got \(update.metadata)")+            return+        }+        #expect(update.hasChanges == true)+    }++    @Test func validate_metadataNonObject_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let result = TaskUpdateValidator.validate(+            ["metadata": "string"], task: task, milestoneService: svc.milestone+        )+        let error = try unwrapFailure(result)+        #expect(isInvalidInput(error, contains: "metadata"), "Expected metadata-specific invalidInput, got \(error)")+    }++    @Test func validate_metadataNonStringValues_rejects() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        // {"metadata": {"a": 123}} — 123 round-trips as NSNumber and must fail strict cast.+        let args = try jsonRoundTrip("{\"metadata\": {\"a\": 123}}")+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        if case .invalidInput(let message) = error {+            #expect(message == "metadata values must be strings", "Expected literal AC 4.4 message, got: \(message)")+        } else {+            Issue.record("Expected .invalidInput, got \(error)")+        }+    }++    // MARK: - Milestone (parity with existing handler)++    @Test func validate_setMilestoneByDisplayId_returnsAssignAction() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "v1.0", project: project, displayId: 1)+        let task = makeTask(in: svc.context, project: project)++        let args: [String: Any] = ["milestoneDisplayId": 1]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        guard case .assign(let assigned) = update.milestoneAction else {+            Issue.record("Expected .assign, got \(String(describing: update.milestoneAction))")+            return+        }+        #expect(assigned.id == milestone.id)+        #expect(update.hasChanges == true)+    }++    @Test func validate_setMilestoneByDisplayId_notFound_returnsMilestoneNotFound() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args: [String: Any] = ["milestoneDisplayId": 999]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        if case .milestoneNotFound(let message) = error {+            // Match MCP handler's existing literal exactly.+            #expect(message == "No milestone with displayId 999")+        } else {+            Issue.record("Expected .milestoneNotFound, got \(error)")+        }+    }++    @Test func validate_setMilestoneByDisplayId_wrongProject_returnsMilestoneProjectMismatch() throws {+        let svc = try makeServices()+        let alpha = makeProject(in: svc.context, name: "Alpha")+        let beta = makeProject(in: svc.context, name: "Beta")+        makeMilestone(in: svc.context, name: "v1.0", project: beta, displayId: 1)+        let task = makeTask(in: svc.context, project: alpha)++        let args: [String: Any] = ["milestoneDisplayId": 1]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let error = try unwrapFailure(result)+        if case .milestoneProjectMismatch = error {+            // Expected+        } else {+            Issue.record("Expected .milestoneProjectMismatch, got \(error)")+        }+    }++    @Test func validate_clearMilestone_returnsClearAction() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "v1.0", project: project, displayId: 1)+        let task = makeTask(in: svc.context, project: project, milestone: milestone)++        let args: [String: Any] = ["clearMilestone": true]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        guard case .clear = update.milestoneAction else {+            Issue.record("Expected .clear, got \(String(describing: update.milestoneAction))")+            return+        }+        #expect(update.hasChanges == true)+    }++    @Test func validate_clearMilestoneOnUnassigned_stillReturnsClear() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        // Task without milestone.+        let task = makeTask(in: svc.context, project: project)+        #expect(task.milestone == nil)++        let args: [String: Any] = ["clearMilestone": true]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        guard case .clear = update.milestoneAction else {+            Issue.record("Expected .clear even on unassigned task, got \(String(describing: update.milestoneAction))")+            return+        }+        #expect(update.hasChanges == true)+    }++    // MARK: - hasChanges semantics (AC 5.1, 7.1, 7.2)++    @Test func validate_identifierOnlyArgs_hasChangesFalse() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project, displayId: 10)++        let args: [String: Any] = ["taskId": task.id.uuidString]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        #expect(update.hasChanges == false)+    }++    @Test func validate_unknownFieldsIgnored_stillHasChangesFalse() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, project: project)++        let args: [String: Any] = [+            "taskId": task.id.uuidString,+            "frob": "bar",+            "nmae": "typo"+        ]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        #expect(update.hasChanges == false)+    }++    // MARK: - strictStringMetadata (AC 4.4)++    @Test func strictStringMetadata_dictOfStrings_succeeds() throws {+        let result = TaskUpdateValidator.strictStringMetadata(from: ["a": "1", "b": "2"])+        switch result {+        case .success(let change):+            guard case .set(let dict) = change else {+                Issue.record("Expected .set, got \(change)")+                return+            }+            #expect(dict == ["a": "1", "b": "2"])+        case .failure(let error):+            Issue.record("Expected success, got failure: \(error)")+        }+    }++    @Test func strictStringMetadata_NSNumberValue_rejects() throws {+        let args = try jsonRoundTrip("{\"a\": 1}")+        // `args` is a `[String: Any]` whose value is NSNumber.+        let result = TaskUpdateValidator.strictStringMetadata(from: args)+        switch result {+        case .success(let change):+            Issue.record("Expected failure for NSNumber value, got success: \(change)")+        case .failure(let error):+            if case .invalidInput(let message) = error {+                #expect(+                    message == "metadata values must be strings",+                    "Expected literal AC 4.4 message, got: \(message)"+                )+            } else {+                Issue.record("Expected .invalidInput, got \(error)")+            }+        }+    }++    @Test func strictStringMetadata_nilOrMissing_returnsNoChange() throws {+        let result = TaskUpdateValidator.strictStringMetadata(from: nil)+        switch result {+        case .success(let change):+            guard case .noChange = change else {+                Issue.record("Expected .noChange, got \(change)")+                return+            }+        case .failure(let error):+            Issue.record("Expected .noChange success, got failure: \(error)")+        }+    }++    @Test func strictStringMetadata_nonDictInput_rejects() throws {+        let result = TaskUpdateValidator.strictStringMetadata(from: "not a dict")+        switch result {+        case .success(let change):+            Issue.record("Expected failure for non-dict, got success: \(change)")+        case .failure(let error):+            #expect(isInvalidInput(error, contains: "metadata"), "Expected metadata invalidInput, got \(error)")+        }+    }++    @Test func strictStringMetadata_emptyDict_returnsClear() throws {+        let result = TaskUpdateValidator.strictStringMetadata(from: [String: String]())+        switch result {+        case .success(let change):+            guard case .clear = change else {+                Issue.record("Expected .clear, got \(change)")+                return+            }+        case .failure(let error):+            Issue.record("Expected .clear success, got failure: \(error)")+        }+    }++    // MARK: - apply (AC 5.1, 7.1, 7.2)++    @Test func apply_setsNameDescriptionTypeMetadata_inMemory_noSaveSideEffect() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Old", type: .bug, project: project)+        // Flush any previous unsaved state (from insertion). The fixture inserts but+        // does not save; baseline for the test is "context already dirty from insert".+        try svc.context.save()+        #expect(svc.context.hasChanges == false)++        let args: [String: Any] = [+            "name": "  New  ",+            "description": "  Body  ",+            "type": "feature",+            "metadata": ["k": "v"]+        ]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)++        try TaskUpdateValidator.apply(+            update, to: task,+            taskService: svc.task, milestoneService: svc.milestone+        )++        #expect(task.name == "New")+        #expect(task.taskDescription == "Body")+        #expect(task.type == .feature)+        #expect(task.metadata == ["k": "v"])+        // apply must not save — the context should still be dirty.+        #expect(svc.context.hasChanges == true)+    }++    @Test func apply_clearDescriptionAndMetadata_sets_nilAndEmpty() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(+            in: svc.context,+            description: "old description",+            project: project,+            metadata: ["a": "1"]+        )+        try svc.context.save()++        let args: [String: Any] = [+            "description": "",+            "metadata": [String: String]()+        ]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)++        try TaskUpdateValidator.apply(+            update, to: task,+            taskService: svc.task, milestoneService: svc.milestone+        )++        #expect(task.taskDescription == nil)+        #expect(task.metadata.isEmpty)+    }++    @Test func apply_noChangeFields_areUntouched() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(+            in: svc.context, name: "Keep", description: "Keep desc",+            type: .chore, project: project, metadata: ["k": "v"]+        )+        try svc.context.save()++        // Empty args — should produce no-op update with hasChanges == false.+        let result = TaskUpdateValidator.validate([:], task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)+        #expect(update.hasChanges == false)++        // Even if a caller chose to call apply, nothing should change.+        try TaskUpdateValidator.apply(+            update, to: task,+            taskService: svc.task, milestoneService: svc.milestone+        )++        #expect(task.name == "Keep")+        #expect(task.taskDescription == "Keep desc")+        #expect(task.type == .chore)+        #expect(task.metadata == ["k": "v"])+    }++    @Test func apply_milestoneAssign_callsSetMilestone_inMemory() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "v1.0", project: project, displayId: 1)+        let task = makeTask(in: svc.context, project: project)+        try svc.context.save()++        let args: [String: Any] = ["milestoneDisplayId": 1]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)++        try TaskUpdateValidator.apply(+            update, to: task,+            taskService: svc.task, milestoneService: svc.milestone+        )++        #expect(task.milestone?.id == milestone.id)+        #expect(svc.context.hasChanges == true)+    }++    @Test func apply_milestoneClear_callsSetMilestone_withNil() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "v1.0", project: project, displayId: 1)+        let task = makeTask(in: svc.context, project: project, milestone: milestone)+        try svc.context.save()++        let args: [String: Any] = ["clearMilestone": true]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)++        try TaskUpdateValidator.apply(+            update, to: task,+            taskService: svc.task, milestoneService: svc.milestone+        )++        #expect(task.milestone == nil)+        #expect(svc.context.hasChanges == true)+    }++    /// Induce a service-layer throw mid-apply and confirm:+    /// 1. `apply` propagates the throw (does not catch).+    /// 2. The caller can recover by calling `taskService.rollback()` after which+    ///    the pre-apply state is restored.+    ///+    /// The scenario: validator returns an `assign` action for a milestone whose+    /// project we mutate after validation but before apply, forcing+    /// `MilestoneService.setMilestone` to throw `projectMismatch`. The earlier+    /// in-memory field mutations from `taskService.updateTask` should then be+    /// rolled back.+    @Test func apply_milestoneThrows_propagates_callerCanRollback() throws {+        let svc = try makeServices()+        let alpha = makeProject(in: svc.context, name: "Alpha")+        let beta = makeProject(in: svc.context, name: "Beta")+        let milestone = makeMilestone(in: svc.context, name: "v1.0", project: alpha, displayId: 1)+        let task = makeTask(+            in: svc.context, name: "Original", description: "Original desc",+            type: .bug, project: alpha+        )+        // Persist the baseline so rollback has a clean state to revert to.+        try svc.context.save()++        // Validate against the matching project so the validator succeeds.+        let args: [String: Any] = [+            "name": "Renamed",+            "description": "New desc",+            "milestoneDisplayId": 1+        ]+        let result = TaskUpdateValidator.validate(args, task: task, milestoneService: svc.milestone)+        let update = try unwrapSuccess(result)++        // Now break the precondition: move the milestone to a different project so+        // setMilestone throws projectMismatch mid-apply.+        milestone.project = beta+        try svc.context.save()++        // apply should throw because milestone's project no longer matches task's.+        var threw = false+        do {+            try TaskUpdateValidator.apply(+                update, to: task,+                taskService: svc.task, milestoneService: svc.milestone+            )+        } catch {+            threw = true+        }+        #expect(threw, "Expected apply to throw when milestone setter rejects")++        // At this point the task may have been mutated in memory by the earlier+        // updateTask call. Caller rolls back to discard partial state.+        svc.task.rollback()++        #expect(task.name == "Original", "Name should revert after rollback")+        #expect(task.taskDescription == "Original desc", "Description should revert after rollback")+    }+}++// swiftlint:enable type_body_length+// swiftlint:enable file_length

diff --git a/Transit/TransitTests/MCPUpdateTaskTests.swift b/Transit/TransitTests/MCPUpdateTaskTests.swiftindex 2dbc9f9..bb7f8d6 100644--- a/Transit/TransitTests/MCPUpdateTaskTests.swift+++ b/Transit/TransitTests/MCPUpdateTaskTests.swift@@ -4,6 +4,9 @@ import SwiftData import Testing @testable import Transit +// swiftlint:disable file_length+// swiftlint:disable type_body_length+ /// Tests for the MCP `update_task` tool handler, including milestone assignment /// and the T-531 fix for save-failure rollback. @MainActor @Suite(.serialized)@@ -192,6 +195,668 @@ struct MCPUpdateTaskTests {         #expect(!env.context.hasChanges, "Context should not have dirty state after failed update_task")         #expect(task.milestone == nil, "Task milestone should be nil after failed cross-project assignment")     }++    // MARK: - T-650 Phase 4: Field Updates++    /// Round-trips a JSON string through JSONSerialization so non-string values+    /// (numbers, booleans) materialize as NSNumber, matching what `IntentHelpers.parseJSON`+    /// delivers to handlers.+    private static func jsonRoundTrip(_ json: String) throws -> [String: Any] {+        let data = try #require(json.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    // MARK: - Name (AC 1.x)++    @Test func updateName_setsTrimmedName() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "  hello  "]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["name"] as? String == "hello")+        #expect(task.name == "hello")+    }++    @Test func updateName_rejectsEmpty() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": ""]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.name == "Original")+    }++    @Test func updateName_rejectsWhitespaceOnly() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "   "]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.name == "Original")+    }++    @Test func updateName_rejectsNonString() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let args = try Self.jsonRoundTrip("{\"displayId\": \(taskDisplayId), \"name\": 42}")+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task", arguments: args+        ))++        #expect(try MCPTestHelpers.isError(response))+        let message = try MCPTestHelpers.errorText(response)+        #expect(message.contains("name"), "Expected name-specific error, got: \(message)")+        #expect(task.name == "Original")+    }++    // MARK: - Description (AC 2.x)++    @Test func updateDescription_setsTrimmed() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "description": "  text  "]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["description"] as? String == "text")+        #expect(task.taskDescription == "text")+    }++    @Test func updateDescription_emptyClears() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "current", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "description": ""]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["description"] == nil, "Response should omit description when cleared")+        #expect(task.taskDescription == nil)+    }++    @Test func updateDescription_whitespaceClears() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "current", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "description": "   "]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["description"] == nil, "Response should omit description when cleared")+        #expect(task.taskDescription == nil)+    }++    @Test func updateDescription_rejectsNonString() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "current", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let args = try Self.jsonRoundTrip("{\"displayId\": \(taskDisplayId), \"description\": 42}")+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task", arguments: args+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.taskDescription == "current")+    }++    // MARK: - Type (AC 3.x)++    @Test func updateType_setsValidType() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .bug, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "type": "feature"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["type"] as? String == "feature")+        #expect(task.type == .feature)+    }++    @Test func updateType_rejectsInvalidValue() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .bug, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "type": "epic"]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.type == .bug)+    }++    @Test func updateType_rejectsNonString() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .bug, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let args = try Self.jsonRoundTrip("{\"displayId\": \(taskDisplayId), \"type\": 1}")+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task", arguments: args+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.type == .bug)+    }++    // MARK: - Metadata (AC 4.x)++    @Test func updateMetadata_replacesEntireDict() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1", "b": "2"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "metadata": ["c": "3"]]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        let metadata = try #require(result["metadata"] as? [String: String])+        #expect(metadata == ["c": "3"])+        #expect(task.metadata == ["c": "3"])+    }++    @Test func updateMetadata_emptyDictClears() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "metadata": [String: String]()]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["metadata"] == nil, "Response should omit metadata when cleared")+        #expect(task.metadata.isEmpty)+    }++    @Test func updateMetadata_rejectsNonObject() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "metadata": "string"]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(task.metadata == ["a": "1"])+    }++    @Test func updateMetadata_rejectsNonStringValues() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        // Round-trip via JSON so the `1` becomes NSNumber, not Swift Int.+        let args = try Self.jsonRoundTrip(+            "{\"displayId\": \(taskDisplayId), \"metadata\": {\"a\": 1}}"+        )+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task", arguments: args+        ))++        #expect(try MCPTestHelpers.isError(response))+        let message = try MCPTestHelpers.errorText(response)+        #expect(message == "metadata values must be strings", "Got: \(message)")+        #expect(task.metadata == ["a": "1"])+    }++    // MARK: - Omission Preservation (AC 1.4, 2.4, 3.4, 4.5)++    @Test func omittingNamePreservesIt() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "X", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "description": "ignored"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["name"] as? String == "X")+        #expect(task.name == "X")+    }++    @Test func omittingDescriptionPreservesIt() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "current", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        // Update something other than description+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "Renamed"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["description"] as? String == "current")+        #expect(task.taskDescription == "current")+    }++    @Test func omittingTypePreservesIt() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .bug, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "Renamed"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["type"] as? String == "bug")+        #expect(task.type == .bug)+    }++    @Test func omittingMetadataPreservesIt() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "Renamed"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        let metadata = try #require(result["metadata"] as? [String: String])+        #expect(metadata == ["a": "1"])+        #expect(task.metadata == ["a": "1"])+    }++    // MARK: - Atomicity (AC 5.x)++    @Test func updateMultipleFields_allAppliedAtomically() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: "old", type: .bug, project: project,+            metadata: ["a": "1"]+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let milestoneDisplayId = try #require(milestone.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: [+                "displayId": taskDisplayId,+                "name": "Renamed",+                "description": "new desc",+                "type": "chore",+                "metadata": ["new": "val"],+                "milestoneDisplayId": milestoneDisplayId+            ]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["name"] as? String == "Renamed")+        #expect(result["description"] as? String == "new desc")+        #expect(result["type"] as? String == "chore")++        #expect(task.name == "Renamed")+        #expect(task.taskDescription == "new desc")+        #expect(task.type == .chore)+        #expect(task.metadata == ["new": "val"])+        #expect(task.milestone?.id == milestone.id)+    }++    @Test func updateMixed_invalidFieldRollsBackAll() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: "old", type: .bug, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        // Valid name + invalid type → whole call rejected+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: [+                "displayId": taskDisplayId,+                "name": "NewName",+                "type": "epic"  // invalid+            ]+        ))++        #expect(try MCPTestHelpers.isError(response))+        // Nothing should have changed+        #expect(task.name == "Original")+        #expect(task.taskDescription == "old")+        #expect(task.type == .bug)+        #expect(task.metadata == ["a": "1"])+    }++    @Test func applyThrows_taskUntouched() async throws {+        // Drive an apply-time error via cross-project milestone mismatch+        // while also requesting field updates. The handler must roll back+        // any in-memory field mutations so the task fields are not modified.+        let env = try MCPTestHelpers.makeEnv()+        let projectA = MCPTestHelpers.makeProject(in: env.context, name: "Alpha")+        let projectB = MCPTestHelpers.makeProject(in: env.context, name: "Beta")+        let task = try await env.taskService.createTask(+            name: "Original", description: "old", type: .bug, project: projectA,+            metadata: ["a": "1"]+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: projectB+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let milestoneDisplayId = try #require(milestone.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: [+                "displayId": taskDisplayId,+                "name": "NewName",+                "description": "new desc",+                "type": "chore",+                "metadata": ["new": "val"],+                "milestoneDisplayId": milestoneDisplayId  // mismatched project+            ]+        ))++        #expect(try MCPTestHelpers.isError(response))+        // All field mutations must be rolled back+        #expect(task.name == "Original")+        #expect(task.taskDescription == "old")+        #expect(task.type == .bug)+        #expect(task.metadata == ["a": "1"])+        #expect(task.milestone == nil)+        #expect(!env.context.hasChanges, "Context should be clean after rollback")+    }++    // MARK: - No-op + Unknown Fields (AC 6.x)++    @Test func identifierOnly_doesNotSave_returnsCurrent() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "desc", type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let originalLastStatusChange = task.lastStatusChangeDate++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["name"] as? String == "Task")+        #expect(result["description"] as? String == "desc")+        // No mutation → no timestamp tick from any side-effect+        #expect(task.lastStatusChangeDate == originalLastStatusChange)+        // Context should not have any pending changes either+        #expect(!env.context.hasChanges)+    }++    @Test func metadataEmpty_isMutation_triggersSave() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "metadata": [String: String]()]+        ))++        #expect(try !MCPTestHelpers.isError(response))+        #expect(task.metadata.isEmpty)+    }++    @Test func unknownFieldsIgnored_doNotBlockNoOp() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "desc", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "frob": "bar"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["name"] as? String == "Task")+        #expect(result["description"] as? String == "desc")+        #expect(task.name == "Task")+    }++    // MARK: - Milestone Parity (AC 7.x)++    @Test func updateMilestoneAndName_singleSave() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let milestoneDisplayId = try #require(milestone.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: [+                "displayId": taskDisplayId,+                "name": "Renamed",+                "milestoneDisplayId": milestoneDisplayId+            ]+        ))++        #expect(try !MCPTestHelpers.isError(response))+        #expect(task.name == "Renamed")+        #expect(task.milestone?.id == milestone.id)+    }++    /// Documents that clearMilestone on an already-unassigned task is treated+    /// as a (redundant) save — acceptable per the design.+    @Test func clearMilestone_onAlreadyUnassigned_savesAnyway() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project+        )+        #expect(task.milestone == nil)+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "clearMilestone": true]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["milestone"] == nil, "Response should omit milestone when nil")+        #expect(task.milestone == nil)+    }++    // MARK: - Response Shape (AC 9.1)++    @Test func responseOmitsClearedDescriptionAndMetadata() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "old", type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: [+                "displayId": taskDisplayId,+                "description": "",+                "metadata": [String: String]()+            ]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["description"] == nil)+        #expect(result["metadata"] == nil)+    }++    @Test func responseExcludesCommentsAndDateFields() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "desc", type: .feature, project: project+        )+        _ = try env.commentService.addComment(+            to: task, content: "First", authorName: "Tester", isAgent: false+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task",+            arguments: ["displayId": taskDisplayId, "name": "Renamed"]+        ))++        let result = try MCPTestHelpers.decodeResult(response)+        #expect(result["comments"] == nil)+        #expect(result["creationDate"] == nil)+        #expect(result["lastStatusChangeDate"] == nil)+        #expect(result["completionDate"] == nil)+    }++    // MARK: - Schema (AC 8.2)++    @Test func toolsListIncludesNewUpdateTaskFields() {+        let schema = MCPToolDefinitions.updateTask.inputSchema+        let properties = try? #require(schema.properties)+        guard let properties else { return }++        #expect(properties["name"] != nil)+        #expect(properties["description"] != nil)+        #expect(properties["type"] != nil)+        #expect(properties["metadata"] != nil)++        if let descProse = properties["description"]?.description {+            #expect(descProse.contains("clear"), "description prose should mention 'clear'; got: \(descProse)")+        } else {+            Issue.record("description property has no prose")+        }+        if let metaProse = properties["metadata"]?.description {+            #expect(+                metaProse.contains("clear") || metaProse.contains("{}"),+                "metadata prose should mention 'clear' or '{}'; got: \(metaProse)"+            )+        } else {+            Issue.record("metadata property has no prose")+        }+    } }+// swiftlint:enable type_body_length  #endif

diff --git a/Transit/TransitTests/UpdateTaskIntentTests.swift b/Transit/TransitTests/UpdateTaskIntentTests.swiftindex 5514f5a..fae4010 100644--- a/Transit/TransitTests/UpdateTaskIntentTests.swift+++ b/Transit/TransitTests/UpdateTaskIntentTests.swift@@ -3,6 +3,9 @@ import SwiftData import Testing @testable import Transit +// swiftlint:disable file_length+// swiftlint:disable type_body_length+ @MainActor @Suite(.serialized) struct UpdateTaskIntentTests { @@ -227,6 +230,25 @@ struct UpdateTaskIntentTests {         #expect(parsed["error"] as? String == "INVALID_INPUT")     } +    /// T-650 regression: `name` is an *update field*, not a task identifier+    /// (AC 6.1). A payload that only contains `name` (no `taskId` / `displayId`)+    /// must surface as INVALID_INPUT, not TASK_NOT_FOUND.+    @Test func nameOnlyWithoutIdentifierReturnsInvalidInput() throws {+        let svc = try makeServices()++        let input = """+        {"name":"renamed"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+    }+     @Test func malformedJSONReturnsInvalidInput() throws {         let svc = try makeServices() @@ -328,4 +350,684 @@ struct UpdateTaskIntentTests {         #expect(parsed["error"] == nil, "clearMilestone:false should not error")         #expect(task.milestone?.id == milestone.id, "Milestone should remain assigned when clearMilestone is false")     }++    // MARK: - T-650 Phase 5: Field Updates++    /// Round-trips a JSON string through JSONSerialization so non-string values+    /// (numbers, booleans) materialize as NSNumber, matching what+    /// `IntentHelpers.parseJSON` delivers to the intent's `execute`.+    private static func jsonRoundTrip(_ json: String) throws -> [String: Any] {+        let data = try #require(json.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    // MARK: - Name (AC 1.x)++    @Test func updateName_setsTrimmedName() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)++        let input = """+        {"displayId":10,"name":"  hello  "}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["name"] as? String == "hello")+        #expect(task.name == "hello")+    }++    @Test func updateName_rejectsEmpty() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)++        let input = """+        {"displayId":10,"name":""}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.name == "Original")+    }++    @Test func updateName_rejectsWhitespaceOnly() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)++        let input = """+        {"displayId":10,"name":"   "}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.name == "Original")+    }++    @Test func updateName_rejectsNonString() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)++        let input = """+        {"displayId":10,"name":42}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        let hint = parsed["hint"] as? String ?? ""+        #expect(hint.contains("name"), "Expected name-specific hint, got: \(hint)")+        #expect(task.name == "Original")+    }++    // MARK: - Description (AC 2.x)++    @Test func updateDescription_setsTrimmed() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)++        let input = """+        {"displayId":10,"description":"  text  "}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["description"] as? String == "text")+        #expect(task.taskDescription == "text")+    }++    @Test func updateDescription_emptyClears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "current"++        let input = """+        {"displayId":10,"description":""}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["description"] == nil, "Response should omit description when cleared")+        #expect(task.taskDescription == nil)+    }++    @Test func updateDescription_whitespaceClears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "current"++        let input = """+        {"displayId":10,"description":"   "}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["description"] == nil, "Response should omit description when cleared")+        #expect(task.taskDescription == nil)+    }++    @Test func updateDescription_rejectsNonString() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "current"++        let input = """+        {"displayId":10,"description":42}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.taskDescription == "current")+    }++    // MARK: - Type (AC 3.x)++    @Test func updateType_setsValidType() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.type = .bug++        let input = """+        {"displayId":10,"type":"feature"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["type"] as? String == "feature")+        #expect(task.type == .feature)+    }++    @Test func updateType_rejectsInvalidValue() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.type = .bug++        let input = """+        {"displayId":10,"type":"epic"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.type == .bug)+    }++    @Test func updateType_rejectsNonString() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.type = .bug++        let input = """+        {"displayId":10,"type":1}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.type == .bug)+    }++    // MARK: - Metadata (AC 4.x)++    @Test func updateMetadata_replacesEntireDict() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1", "b": "2"]++        let input = """+        {"displayId":10,"metadata":{"c":"3"}}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        let metadata = try #require(parsed["metadata"] as? [String: String])+        #expect(metadata == ["c": "3"])+        #expect(task.metadata == ["c": "3"])+    }++    @Test func updateMetadata_emptyDictClears() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"metadata":{}}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["metadata"] == nil, "Response should omit metadata when cleared")+        #expect(task.metadata.isEmpty)+    }++    @Test func updateMetadata_rejectsNonObject() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"metadata":"string"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        #expect(task.metadata == ["a": "1"])+    }++    @Test func updateMetadata_rejectsNonStringValues() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"metadata":{"a":1}}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        let hint = parsed["hint"] as? String ?? ""+        #expect(hint == "metadata values must be strings", "Got: \(hint)")+        #expect(task.metadata == ["a": "1"])+    }++    // MARK: - Omission Preservation (AC 1.4, 2.4, 3.4, 4.5)++    @Test func omittingNamePreservesIt() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "X", project: project, displayId: 10)++        let input = """+        {"displayId":10,"description":"ignored"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["name"] as? String == "X")+        #expect(task.name == "X")+    }++    @Test func omittingDescriptionPreservesIt() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "current"++        let input = """+        {"displayId":10,"name":"Renamed"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["description"] as? String == "current")+        #expect(task.taskDescription == "current")+    }++    @Test func omittingTypePreservesIt() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.type = .bug++        let input = """+        {"displayId":10,"name":"Renamed"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["type"] as? String == "bug")+        #expect(task.type == .bug)+    }++    @Test func omittingMetadataPreservesIt() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"name":"Renamed"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        let metadata = try #require(parsed["metadata"] as? [String: String])+        #expect(metadata == ["a": "1"])+        #expect(task.metadata == ["a": "1"])+    }++    // MARK: - Atomicity (AC 5.x)++    @Test func updateMultipleFields_allAppliedAtomically() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "Sprint 1", project: project, displayId: 1)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)+        task.taskDescription = "old"+        task.type = .bug+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"name":"Renamed","description":"new desc",+        "type":"chore","metadata":{"new":"val"},"milestoneDisplayId":1}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["name"] as? String == "Renamed")+        #expect(parsed["description"] as? String == "new desc")+        #expect(parsed["type"] as? String == "chore")++        #expect(task.name == "Renamed")+        #expect(task.taskDescription == "new desc")+        #expect(task.type == .chore)+        #expect(task.metadata == ["new": "val"])+        #expect(task.milestone?.id == milestone.id)+    }++    @Test func updateMixed_invalidFieldRollsBackAll() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)+        task.taskDescription = "old"+        task.type = .bug+        task.metadata = ["a": "1"]++        // Valid name + invalid type → whole call rejected+        let input = """+        {"displayId":10,"name":"NewName","type":"epic"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] as? String == "INVALID_INPUT")+        // Nothing should have changed+        #expect(task.name == "Original")+        #expect(task.taskDescription == "old")+        #expect(task.type == .bug)+        #expect(task.metadata == ["a": "1"])+    }++    @Test func applyThrows_taskUntouched() throws {+        // Drive an apply-time error via cross-project milestone mismatch+        // while also requesting field updates. The handler must roll back+        // any in-memory field mutations so the task fields are not modified.+        let svc = try makeServices()+        let alpha = makeProject(in: svc.context, name: "Alpha")+        let beta = makeProject(in: svc.context, name: "Beta")+        let milestone = makeMilestone(in: svc.context, name: "Sprint 1", project: beta, displayId: 1)+        let task = makeTask(in: svc.context, name: "Original", project: alpha, displayId: 10)+        task.taskDescription = "old"+        task.type = .bug+        task.metadata = ["a": "1"]+        _ = milestone+        // Persist setup so post-update `hasChanges` reflects only the update call,+        // not the test scaffolding's insert/mutation traffic.+        try svc.context.save()++        let input = """+        {"displayId":10,"name":"NewName","description":"new desc",+        "type":"chore","metadata":{"new":"val"},"milestoneDisplayId":1}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] != nil, "Expected error, got: \(parsed)")+        // All field mutations must be rolled back+        #expect(task.name == "Original")+        #expect(task.taskDescription == "old")+        #expect(task.type == .bug)+        #expect(task.metadata == ["a": "1"])+        #expect(task.milestone == nil)+        #expect(!svc.context.hasChanges, "Context should be clean after rollback")+    }++    // MARK: - No-op + Unknown Fields (AC 6.x)++    @Test func identifierOnly_doesNotSave_returnsCurrent() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "desc"+        task.metadata = ["a": "1"]+        try svc.context.save()+        let originalLastStatusChange = task.lastStatusChangeDate++        let input = """+        {"displayId":10}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["name"] as? String == "Task")+        #expect(parsed["description"] as? String == "desc")+        // No mutation → no timestamp tick from any side-effect+        #expect(task.lastStatusChangeDate == originalLastStatusChange)+        // Context should not have any pending changes either+        #expect(!svc.context.hasChanges)+    }++    @Test func metadataEmpty_isMutation_triggersSave() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"metadata":{}}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] == nil)+        #expect(task.metadata.isEmpty)+    }++    @Test func unknownFieldsIgnored_doNotBlockNoOp() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "desc"++        let input = """+        {"displayId":10,"frob":"bar"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["name"] as? String == "Task")+        #expect(parsed["description"] as? String == "desc")+        #expect(task.name == "Task")+    }++    // MARK: - Milestone Parity (AC 7.x)++    @Test func updateMilestoneAndName_singleSave() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let milestone = makeMilestone(in: svc.context, name: "Sprint 1", project: project, displayId: 1)+        let task = makeTask(in: svc.context, name: "Original", project: project, displayId: 10)++        let input = """+        {"displayId":10,"name":"Renamed","milestoneDisplayId":1}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["error"] == nil)+        #expect(task.name == "Renamed")+        #expect(task.milestone?.id == milestone.id)+    }++    /// Documents that clearMilestone on an already-unassigned task is treated+    /// as a (redundant) save — acceptable per the design.+    @Test func clearMilestone_onAlreadyUnassigned_savesAnyway() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        #expect(task.milestone == nil)++        let input = """+        {"displayId":10,"clearMilestone":true}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["milestone"] == nil, "Response should omit milestone when nil")+        #expect(task.milestone == nil)+    }++    // MARK: - Response Shape (AC 9.1)++    @Test func responseOmitsClearedDescriptionAndMetadata() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "old"+        task.metadata = ["a": "1"]++        let input = """+        {"displayId":10,"description":"","metadata":{}}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["description"] == nil)+        #expect(parsed["metadata"] == nil)+    }++    @Test func responseExcludesCommentsAndDateFields() throws {+        let svc = try makeServices()+        let project = makeProject(in: svc.context)+        let task = makeTask(in: svc.context, name: "Task", project: project, displayId: 10)+        task.taskDescription = "desc"++        let input = """+        {"displayId":10,"name":"Renamed"}+        """++        let result = UpdateTaskIntent.execute(+            input: input, taskService: svc.task,+            milestoneService: svc.milestone, projectService: svc.project+        )++        let parsed = try parseJSON(result)+        #expect(parsed["comments"] == nil)+        #expect(parsed["creationDate"] == nil)+        #expect(parsed["lastStatusChangeDate"] == nil)+        #expect(parsed["completionDate"] == nil)+    }++    // MARK: - Parameter Description (AC 8.2)++    @Test func inputParameterDescriptionContainsClearWording() {+        // The Input @Parameter description must document the empty-string-clears+        // semantic for description and the {} clears semantic for metadata so+        // that Shortcuts users see the same field guidance the MCP tool schema+        // provides. Asserted via the static test seam mirrored into the+        // @Parameter declaration.+        let description = UpdateTaskIntent.inputParameterDescription+        #expect(description.contains("description"), "Parameter description should mention 'description'")+        #expect(description.contains("metadata"), "Parameter description should mention 'metadata'")+        // Substring "clear" must appear at least twice (description and metadata)+        let occurrences = description.components(separatedBy: "clear").count - 1+        #expect(+            occurrences >= 2,+            "Parameter description should mention 'clear' at least twice; got \(occurrences) occurrence(s)"+        )+    } }++// swiftlint:enable type_body_length+// swiftlint:enable file_length

diff --git a/Transit/TransitTests/UpdateTaskAllFieldsParityTests.swift b/Transit/TransitTests/UpdateTaskAllFieldsParityTests.swiftnew file mode 100644index 0000000..973bacc--- /dev/null+++ b/Transit/TransitTests/UpdateTaskAllFieldsParityTests.swift@@ -0,0 +1,313 @@+#if os(macOS)+import Foundation+import SwiftData+import Testing+@testable import Transit++/// Cross-surface parity tests for the `update_task` MCP tool and+/// `UpdateTaskIntent` App Intent (T-650 AC 8.1).+///+/// Both surfaces share `TaskUpdateValidator` and+/// `IntentHelpers.taskUpdateResponseDict`, so structural equality of the+/// success response is guaranteed by construction. These tests make that+/// contract explicit, so a future divergence (an extra key in one surface, a+/// JSON serialization quirk, etc.) is caught at test time rather than+/// silently changing the wire format for one caller.+///+/// Strategy: each test prepares a single task and a single set of update args,+/// then invokes both surfaces against that same task in the same context. The+/// first call mutates the task; the second call sees the post-mutation state+/// and either:+///   - re-applies the same change (idempotent, response unchanged), or+///   - is a true no-op for identifier-only cases.+/// Either way, both responses are computed from the task's final model state+/// via the shared response builder, so the dictionaries must match exactly.+///+/// Error-response parity is explicitly NOT asserted here — AC 5.2 permits the+/// two surfaces to surface different error messages for the same invalid+/// input. Success-only coverage is what this test exists for.+@MainActor @Suite(.serialized)+struct UpdateTaskAllFieldsParityTests {++    // MARK: - Helpers++    /// Parses both JSON responses and asserts they are structurally equal.+    /// Bridging through `NSDictionary` gives us deep, order-independent+    /// equality on nested dicts and arrays for free.+    private func assertParity(+        mcp mcpJSON: String,+        intent intentJSON: String,+        sourceLocation: SourceLocation = #_sourceLocation+    ) throws {+        let mcpData = try #require(mcpJSON.data(using: .utf8))+        let intentData = try #require(intentJSON.data(using: .utf8))+        let mcpDict = try #require(+            try JSONSerialization.jsonObject(with: mcpData) as? [String: Any]+        )+        let intentDict = try #require(+            try JSONSerialization.jsonObject(with: intentData) as? [String: Any]+        )++        // Sanity guard: success-only contract. If either surface returned an+        // error envelope, the test setup is wrong — surface the underlying+        // payload rather than letting the equality check report a confusing+        // diff between two error shapes.+        #expect(+            mcpDict["error"] == nil,+            "MCP returned error envelope: \(mcpDict)",+            sourceLocation: sourceLocation+        )+        #expect(+            intentDict["error"] == nil,+            "Intent returned error envelope: \(intentDict)",+            sourceLocation: sourceLocation+        )++        let mcpNS = mcpDict as NSDictionary+        let intentNS = intentDict as NSDictionary+        #expect(+            mcpNS == intentNS,+            "MCP and Intent responses diverge.\nMCP: \(mcpDict)\nIntent: \(intentDict)",+            sourceLocation: sourceLocation+        )+    }++    /// Extracts the JSON text payload from an MCP `tools/call` response.+    private func mcpResponseText(_ response: JSONRPCResponse?) throws -> String {+        let unwrapped = try #require(response, "Expected a JSON-RPC response but got nil")+        let data = try JSONEncoder().encode(unwrapped)+        let json = try JSONSerialization.jsonObject(with: data) as? [String: Any]+        let result = try #require(json?["result"] as? [String: Any])+        let content = try #require(result["content"] as? [[String: Any]])+        return try #require(content.first?["text"] as? String)+    }++    /// Invokes the MCP `update_task` tool with the given arguments and+    /// returns the JSON string payload from the response.+    private func callMCP(+        env: MCPTestEnv,+        arguments: [String: Any]+    ) async throws -> String {+        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "update_task", arguments: arguments+        ))+        return try mcpResponseText(response)+    }++    /// Invokes `UpdateTaskIntent.execute` directly with the given JSON input+    /// and returns the JSON string payload.+    private func callIntent(env: MCPTestEnv, inputJSON: String) -> String {+        UpdateTaskIntent.execute(+            input: inputJSON,+            taskService: env.taskService,+            milestoneService: env.milestoneService,+            projectService: env.projectService+        )+    }++    /// Encodes a `[String: Any]` argument dict to a JSON string suitable for+    /// `UpdateTaskIntent.execute`. Sorts keys so the wire string is+    /// deterministic — the input shape is what matters for parity, not+    /// JSON-string byte-identity.+    private func encodeArgsAsJSON(_ args: [String: Any]) throws -> String {+        let data = try JSONSerialization.data(+            withJSONObject: args, options: [.sortedKeys]+        )+        return try #require(String(data: data, encoding: .utf8))+    }++    /// Runs the same update args through both surfaces against the same task+    /// in the same context, then asserts the JSON responses match.+    private func runParityCase(+        env: MCPTestEnv,+        arguments: [String: Any],+        sourceLocation: SourceLocation = #_sourceLocation+    ) async throws {+        let intentInput = try encodeArgsAsJSON(arguments)+        let mcpJSON = try await callMCP(env: env, arguments: arguments)+        let intentJSON = callIntent(env: env, inputJSON: intentInput)+        try assertParity(mcp: mcpJSON, intent: intentJSON, sourceLocation: sourceLocation)+    }++    // MARK: - Per-Field Parity++    @Test func updateName_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "name": "  renamed  "]+        )+    }++    @Test func updateDescription_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "old", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "description": "new desc"]+        )+    }++    @Test func clearDescription_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "current", type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "description": ""]+        )+    }++    @Test func updateType_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .bug, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "type": "feature"]+        )+    }++    @Test func updateMetadata_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "metadata": ["k": "v"]]+        )+    }++    @Test func clearMetadata_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "metadata": [String: String]()]+        )+    }++    // MARK: - Combined / Atomic++    /// AC 8.1's load-bearing case: a single call exercising every supported+    /// update field (including a milestone assignment) must produce the same+    /// response shape from both surfaces. If this passes, the per-field+    /// cases above are belt-and-braces — but each per-field case still+    /// catches narrower regressions (e.g., a key omission rule that only+    /// affects metadata).+    @Test func combinedAllFields_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Original", description: "old", type: .bug, project: project,+            metadata: ["a": "1"]+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let milestoneDisplayId = try #require(milestone.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: [+                "displayId": taskDisplayId,+                "name": "Renamed",+                "description": "new desc",+                "type": "chore",+                "metadata": ["new": "val"],+                "milestoneDisplayId": milestoneDisplayId+            ]+        )+    }++    // MARK: - No-Op Echo++    @Test func noOpEcho_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: "desc", type: .feature, project: project,+            metadata: ["a": "1"]+        )+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId]+        )+    }++    // MARK: - Milestone++    @Test func milestoneAssign_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: project+        )+        let taskDisplayId = try #require(task.permanentDisplayId)+        let milestoneDisplayId = try #require(milestone.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: [+                "displayId": taskDisplayId,+                "milestoneDisplayId": milestoneDisplayId+            ]+        )+    }++    @Test func milestoneClear_parity() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let task = try await env.taskService.createTask(+            name: "Task", description: nil, type: .feature, project: project+        )+        let milestone = try await env.milestoneService.createMilestone(+            name: "Sprint 1", description: nil, project: project+        )+        try env.milestoneService.setMilestone(milestone, on: task)+        #expect(task.milestone != nil)+        let taskDisplayId = try #require(task.permanentDisplayId)++        try await runParityCase(+            env: env,+            arguments: ["displayId": taskDisplayId, "clearMilestone": true]+        )+    }+}++#endif

diff --git a/Transit/TransitTests/IntentHelpersTaskUpdateResponseDictTests.swift b/Transit/TransitTests/IntentHelpersTaskUpdateResponseDictTests.swiftnew file mode 100644index 0000000..335650d--- /dev/null+++ b/Transit/TransitTests/IntentHelpersTaskUpdateResponseDictTests.swift@@ -0,0 +1,256 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// Tests for `IntentHelpers.taskUpdateResponseDict(_:)`. Verifies the AC 9.1+/// response shape: always-present keys, omitted-when-nil/empty keys, and the+/// explicit exclusion of `comments`, `creationDate`, `lastStatusChangeDate`,+/// and `completionDate`. [T-650]+@MainActor @Suite(.serialized)+struct IntentHelpersTaskUpdateResponseDictTests {++    // MARK: - Fixture++    private struct Fixture {+        let context: ModelContext+        let project: Project+    }++    private func makeFixture(includeProject: Bool = true) throws -> Fixture {+        let context = try TestModelContainer.newContext()+        let project = Project(+            name: "Test Project", description: "A test project", gitRepo: nil, colorHex: "#FF0000"+        )+        if includeProject {+            context.insert(project)+        }+        return Fixture(context: context, project: project)+    }++    private func makeTask(+        in context: ModelContext,+        name: String = "Sample Task",+        description: String? = nil,+        type: TaskType = .feature,+        project: Project?,+        milestone: Milestone? = nil,+        metadata: [String: String]? = nil,+        displayId: Int? = 42+    ) -> TransitTask {+        // TransitTask requires a project at init; we then overwrite to test the+        // unassigned-project case (the model field is optional even though the+        // initializer demands one).+        let placeholderProject = project ?? Project(+            name: "Placeholder", description: "", gitRepo: nil, colorHex: "#000000"+        )+        let displayID: DisplayID = displayId.map { .permanent($0) } ?? .provisional+        let task = TransitTask(+            name: name,+            description: description,+            type: type,+            project: placeholderProject,+            displayID: displayID,+            metadata: metadata+        )+        StatusEngine.initializeNewTask(task)+        if project == nil {+            task.project = nil+        }+        task.milestone = milestone+        context.insert(task)+        return task+    }++    private func makeMilestone(+        in context: ModelContext,+        name: String = "v1.0",+        project: Project,+        displayId: Int = 1+    ) -> Milestone {+        let milestone = Milestone(+            name: name, description: nil, project: project,+            displayID: .permanent(displayId)+        )+        context.insert(milestone)+        return milestone+    }++    // MARK: - Always-present keys++    @Test func allFieldsPopulated_includesEveryKey() throws {+        let fix = try makeFixture()+        let milestone = makeMilestone(in: fix.context, project: fix.project)+        let task = makeTask(+            in: fix.context,+            name: "Full Task",+            description: "A description",+            type: .bug,+            project: fix.project,+            milestone: milestone,+            metadata: ["key": "value"],+            displayId: 42+        )++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict["taskId"] as? String == task.id.uuidString)+        #expect(dict["name"] as? String == "Full Task")+        #expect(dict["type"] as? String == "bug")+        #expect(dict["status"] as? String == task.statusRawValue)+        #expect(dict["displayId"] as? Int == 42)+        #expect(dict["projectId"] as? String == fix.project.id.uuidString)+        #expect(dict["projectName"] as? String == "Test Project")+        #expect(dict["description"] as? String == "A description")+        #expect(dict["metadata"] as? [String: String] == ["key": "value"])+        let milestoneDict = dict["milestone"] as? [String: Any]+        #expect(milestoneDict != nil)+        #expect(milestoneDict?["name"] as? String == "v1.0")+    }++    @Test func nameAlwaysPresent() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, name: "Renamed", project: fix.project)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict["name"] as? String == "Renamed")+    }++    @Test func statusAlwaysPresent() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, project: fix.project)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        // Initialized via StatusEngine.initializeNewTask → idea.+        #expect(dict["status"] as? String == task.statusRawValue)+        #expect(dict["status"] as? String == "idea")+    }++    @Test func typeAlwaysPresent() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, type: .chore, project: fix.project)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict["type"] as? String == task.typeRawValue)+        #expect(dict["type"] as? String == "chore")+    }++    // MARK: - Conditionally-present keys (omitted when nil/empty)++    @Test func noDisplayId_omitsDisplayId() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, project: fix.project, displayId: nil)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict.keys.contains("displayId") == false)+    }++    @Test func noProject_omitsProjectKeys() throws {+        let fix = try makeFixture(includeProject: false)+        let task = makeTask(in: fix.context, project: nil)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict.keys.contains("projectId") == false)+        #expect(dict.keys.contains("projectName") == false)+        #expect(dict.keys.contains("milestone") == false)+    }++    @Test func noDescription_omitsDescription() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, description: nil, project: fix.project)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict.keys.contains("description") == false)+    }++    @Test func emptyDescription_omitsDescription() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, description: "", project: fix.project)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        // Per AC 9.1 the response must omit "" / null / {} for cleared fields.+        // The model stores taskDescription as String? — an explicit empty string+        // through this path must still be omitted by the response builder.+        #expect(dict.keys.contains("description") == false)+    }++    @Test func emptyMetadata_omitsMetadata() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, project: fix.project, metadata: nil)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(task.metadata.isEmpty)+        #expect(dict.keys.contains("metadata") == false)+    }++    @Test func nonEmptyMetadata_includesMetadata() throws {+        let fix = try makeFixture()+        let task = makeTask(+            in: fix.context, project: fix.project, metadata: ["a": "1"]+        )++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict["metadata"] as? [String: String] == ["a": "1"])+    }++    @Test func noMilestone_omitsMilestone() throws {+        let fix = try makeFixture()+        let task = makeTask(in: fix.context, project: fix.project, milestone: nil)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict.keys.contains("milestone") == false)+    }++    @Test func withMilestone_usesMilestoneInfoDict() throws {+        let fix = try makeFixture()+        let milestone = makeMilestone(in: fix.context, project: fix.project, displayId: 7)+        let task = makeTask(in: fix.context, project: fix.project, milestone: milestone)++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        // The milestone summary must match the existing milestoneInfoDict helper+        // verbatim — it is the single source of truth for the milestone shape.+        let expected = IntentHelpers.milestoneInfoDict(milestone)+        let actual = try #require(dict["milestone"] as? [String: Any])+        #expect(actual["milestoneId"] as? String == expected["milestoneId"] as? String)+        #expect(actual["name"] as? String == expected["name"] as? String)+        #expect(actual["displayId"] as? Int == expected["displayId"] as? Int)+        #expect(actual.keys.sorted() == expected.keys.sorted())+    }++    // MARK: - Excluded keys++    @Test func alwaysExcludesCommentsAndDateFields() throws {+        let fix = try makeFixture()+        let task = makeTask(+            in: fix.context,+            description: "Has comments and dates",+            project: fix.project,+            metadata: ["k": "v"]+        )+        // Populate excluded fields so the assertion is meaningful.+        let comment = Comment(+            content: "First comment", authorName: "Tester", isAgent: false, task: task+        )+        fix.context.insert(comment)+        task.completionDate = Date()+        task.lastStatusChangeDate = Date()++        let dict = IntentHelpers.taskUpdateResponseDict(task)++        #expect(dict.keys.contains("comments") == false)+        #expect(dict.keys.contains("creationDate") == false)+        #expect(dict.keys.contains("lastStatusChangeDate") == false)+        #expect(dict.keys.contains("completionDate") == false)+    }+}

diff --git a/Transit/TransitTests/QueryTasksIntentTests.swift b/Transit/TransitTests/QueryTasksIntentTests.swiftindex c280c99..ed1a14d 100644--- a/Transit/TransitTests/QueryTasksIntentTests.swift+++ b/Transit/TransitTests/QueryTasksIntentTests.swift@@ -257,7 +257,10 @@ struct QueryTasksIntentTests {         svc.context.insert(second)          let result = QueryTasksIntent.execute(-            input: "{\"displayId\":42}", projectService: svc.project, taskService: svc.task+            input: "{\"displayId\":42}",+            projectService: svc.project,+            taskService: svc.task,+            milestoneService: svc.milestone         )          let parsed = try parseJSON(result)

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 0d1b392..a37e28d 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ### Fixed +- MCP `update_task` apply/save failure messages are now uniformly worded `"Update failed: \(error)"` (T-650 phase 4). Previously the handler emitted three separate templates (`"Failed to save: ..."`, `"Failed to set milestone: ..."`, `"Failed to clear milestone: ..."`); none of these are referenced by existing tests, but agents matching the old strings will see new wording. Validation, identifier-resolution, and milestone error messages are unchanged.+- `QueryTasksIntentTests` no longer fails to compile after T-1146 added a required `milestoneService:` argument to `QueryTasksIntent.execute`; the missing argument is now supplied at the one stale call site so the test target builds. - Milestone status updates are now idempotent: re-sending the current status no longer rewrites `lastStatusChangeDate` and `completionDate`, so old terminal milestones no longer re-enter the current report window via `ReportLogic`'s `completionDate ?? lastStatusChangeDate` fallback. Applied across `MilestoneService.updateStatus`, `UpdateMilestoneIntent.applyUpdate`, and `MCPToolHandler.applyMilestoneUpdate` (T-923). - `DisplayIDMaintenanceService.reassignDuplicates` now surfaces a fetch failure as a counter-advance warning on both record types instead of silently no-oping the run with empty arrays. - `DisplayIDMaintenanceService` group processing no longer fabricates winner identity (`UUID()` / `""`) for the unreachable empty-group case; the invariant is asserted via `preconditionFailure` and the existing `RecordRef` is propagated.@@ -24,6 +26,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ### Added +- `docs/agent-notes/mcp-server.md` Tools Exposed table now describes the extended `update_task` (name / description / type / metadata + milestone) with a gotcha noting the omit-vs-clear distinction and the shared validator + response-builder pipeline both surfaces use (T-650 phase 7).+- `UpdateTaskAllFieldsParityTests` (T-650 phase 6): 10 cross-surface parity cases (each new field individually, combined multi-field, no-op echo, milestone assign and clear) call both `MCPToolHandler.handleUpdateTask` and `UpdateTaskIntent.execute` against a shared task fixture and assert byte-equal JSON responses. Verifies AC 8.1 in practice. All cases passed on the first run with zero production diffs — both surfaces route through `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, so structural parity is a property of the architecture rather than an emergent coincidence.+- `UpdateTaskIntent` (App Intent) now accepts the same `name` / `description` / `type` / `metadata` updates as the MCP tool (T-650 phase 5). `execute()` rewritten as a thin orchestrator over `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, mirroring `MCPToolHandler.handleUpdateTask` structurally so success responses are byte-equivalent across surfaces (AC 8.1). The old `applyMilestoneChange` and `buildResponse` helpers are deleted. The `@Parameter` description enumerates every supported field including the empty-string-clears and `{}`-clears semantics. A preflight identifier guard returns `INVALID_INPUT` ("Provide either displayId or taskId") for payloads with no `taskId`/`displayId` — previously these landed as a misleading `TASK_NOT_FOUND` from the downstream resolver, including the pre-existing failing test `noTaskIdentifierReturnsInvalidInput`, which now passes.+- MCP `update_task` now accepts name, description (empty/whitespace clears), type, and metadata (`{}` clears) alongside the existing milestone fields (T-650 phase 4). `MCPToolHandler.handleUpdateTask` rewritten as a thin orchestrator over `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`. The two-stage do/catch (apply with explicit `taskService.rollback()`; save relying on `TaskService.save()`'s built-in `safeRollback`) keeps the task untouched when any field validation, apply, or save step fails — atomic per AC 5.1. Schema (`MCPToolDefinitions.updateTask`) extended with name/description/type/metadata properties; description and metadata prose include `"clear"` so clients know the clear semantics. Identifier-resolution preamble (T-634 / T-808) and milestone error literals are byte-equivalent to the pre-rewrite handler so existing tests pass unchanged.+- `IntentHelpers.taskUpdateResponseDict` (T-650 phase 3): shared response builder for `update_task` across MCP and App Intent surfaces. Always emits `taskId`, `name`, `type`, `status`; emits `displayId`, `projectId`, `projectName`, `description`, `milestone` only when non-nil; emits `metadata` only when non-empty. Excludes `comments` and the date fields. 13 tests cover every conditional clause of AC 9.1.+- `TaskUpdateValidator.validate` / `apply` / `strictStringMetadata` implementations (T-650 phase 2). `validate` parses raw MCP/Intent argument dicts into a `ValidatedTaskUpdate` covering name (trim + non-empty), description (trim, empty/whitespace clears), type (lowercase enum), metadata (strict-string, `{}` clears), and milestone (precedence `clearMilestone` → `milestoneId` → `milestoneDisplayId` → `milestone`). Milestone error literals are byte-identical to the existing MCP handler so Phase 3 wiring is drop-in. `apply` walks fields in deterministic order with `save: false`, throwing service-layer errors only — callers handle rollback via `TaskService.rollback()`. `TaskUpdateValidatorTests` covers 33 scenarios across all acceptance criteria 1–7.+- `TaskUpdateValidator` scaffolding (T-650 phase 1): new `Transit/Transit/Intents/TaskUpdateValidator.swift` declares the `FieldChange<T>`, `MilestoneAction`, `ValidatedTaskUpdate`, and `TaskUpdateValidationError` value types plus stub `validate`/`apply`/`strictStringMetadata` signatures so subsequent phases can write tests against the contract. `TaskService.rollback()` wraps `modelContext.safeRollback()` for handler-level undo after a mid-update throw.+- Spec for extending `update_task` MCP tool and `UpdateTaskIntent` to update name, description, type, and metadata atomically alongside the existing milestone fields (T-650). Includes requirements, design (shared `TaskUpdateValidator` with `FieldChange<T>`-based update model, new `IntentHelpers.taskUpdateResponseDict`, `TaskService.rollback` for apply-failure rollback), decision log, and a 12-task implementation plan across 7 phases. - Test for AC 8.1: an allocation failure on one duplicate group does not abort subsequent groups; the loser loop breaks for the failing group only and the next group still reassigns. - `DisplayIDMaintenanceService` is now constructed cross-platform in `TransitApp`, registered via `AppDependencyManager` for App Intents, and exposed via `.environment` on the root `NavigationStack` and `withCoreEnvironments` so `DataMaintenanceView` resolves it on iOS, the macOS Settings window, and the macOS Task Detail window. - `ScanDuplicateDisplayIDsIntent` and `ReassignDuplicateDisplayIDsIntent` App Intents for running duplicate cleanup from Shortcuts. Both reuse the same `JSONEncoder` path as the MCP tools so payloads are byte-equal across surfaces; service errors land inside the JSON envelope rather than thrown.

diff --git a/docs/agent-notes/mcp-server.md b/docs/agent-notes/mcp-server.mdindex 0808aad..e65e87c 100644--- a/docs/agent-notes/mcp-server.md+++ b/docs/agent-notes/mcp-server.md@@ -38,6 +38,7 @@ Key challenge: Hummingbird runs on SwiftNIO event loops (nonisolated), but servi |------|-------------| | `create_task` | Create a new task (name and type required; at least one of project / projectId required to identify the project; description and metadata optional) | | `update_task_status` | Change task status (by displayId or taskId) |+| `update_task` | Update a task's mutable fields — any combination of `name`, `description`, `type`, `metadata`, and milestone assignment (`milestone` / `milestoneDisplayId` / `clearMilestone`) — in a single atomic call. Identify task by displayId or taskId. | | `query_tasks` | List tasks with optional status/type/project filters; includes comments | | `add_comment` | Add a comment to a task (by displayId or taskId); always sets `isAgent: true` | @@ -104,3 +105,4 @@ MCPToolHandler, App Intents, and IntentHelpers all delegate to these methods rat - **Always validate UUID filter parameters separately from presence checks.** Never use `flatMap(UUID.init)` or combined conditionals like `if let str = ..., let uuid = UUID(str)` for user-provided filter values — these silently drop invalid inputs. Instead, first check if the key is present, then validate the UUID format with a guard, returning an explicit error on failure. See `handleQueryTasks` for the correct pattern; `handleQueryMilestones` uses `resolveProjectFilter` helper (T-665). This applies to both query and create flows — T-743 fixed the same pattern in `handleCreateTask`, `handleCreateMilestone`, `CreateTaskIntent`, and `CreateMilestoneIntent`. The `resolveMilestone` helper also follows this pattern for both `displayId` (T-634) and `milestoneId` (T-769). - **Always validate enum filter values in query handlers.** When a handler accepts `status`, `not_status`, or `type` as filter parameters, validate each value against the enum's `allCases` before using them. Invalid values must return `isError: true` with a message listing valid options — never silently filter to empty results. Helper: `validateEnumFilter(_:key:type:)` in `MCPToolHandler` — generic over any `RawRepresentable & CaseIterable` enum with `String` raw values (T-732). - **Reuse IntentHelpers instead of implementing private handler-local helpers.** `IntentHelpers` provides shared utilities for metadata extraction, JSON parsing, task/milestone resolution, and error mapping. Don't duplicate this logic in MCPToolHandler — use the shared version to ensure consistent behavior across MCP and App Intent paths (T-723).+- **`update_task` distinguishes "omit" from "clear".** Omitting a field leaves the stored value untouched; passing `description: ""` or whitespace-only clears the description, and passing `metadata: {}` clears all metadata. An identifier-only request (no mutating field) is a no-op echo — the task is returned without a save. Both `update_task` and `UpdateTaskIntent` route through `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, so success payloads are equivalent across surfaces (enforced by `UpdateTaskAllFieldsParityTests`).

diff --git a/specs/OVERVIEW.md b/specs/OVERVIEW.mdindex 9adf933..8ebc1c3 100644--- a/specs/OVERVIEW.md+++ b/specs/OVERVIEW.md@@ -30,6 +30,7 @@ | [Home Screen Quick Actions](#home-screen-quick-actions) | 2026-03-24 | Done | iOS Home Screen quick action to create a new task | | [Sync Heartbeat](#sync-heartbeat) | 2026-03-28 | No Tasks | Periodic SwiftData write to force CloudKit sync on macOS | | [Duplicate Display ID Cleanup](#duplicate-display-id-cleanup) | 2026-04-25 | Done | Scan and reassign tasks/milestones sharing a permanentDisplayId |+| [Update Task All Fields](#update-task-all-fields) | 2026-05-22 | Done | Extend update_task MCP tool and App Intent to update name, description, type, and metadata |  --- @@ -282,3 +283,12 @@ Scan and reassign tasks/milestones sharing a permanentDisplayId. - [design.md](duplicate-displayid-cleanup/design.md) - [requirements.md](duplicate-displayid-cleanup/requirements.md) - [tasks.md](duplicate-displayid-cleanup/tasks.md)++## Update Task All Fields++Extend the `update_task` MCP tool and `UpdateTaskIntent` App Intent to update name, description, type, and metadata atomically alongside the existing milestone fields (T-650).++- [decision_log.md](update-task-all-fields/decision_log.md)+- [design.md](update-task-all-fields/design.md)+- [requirements.md](update-task-all-fields/requirements.md)+- [tasks.md](update-task-all-fields/tasks.md)

diff --git a/specs/update-task-all-fields/requirements.md b/specs/update-task-all-fields/requirements.mdnew file mode 100644index 0000000..e3df826--- /dev/null+++ b/specs/update-task-all-fields/requirements.md@@ -0,0 +1,110 @@+# Requirements: Extend update_task to Support All Task Fields (T-650)++## Introduction++The `update_task` MCP tool and the `UpdateTaskIntent` App Intent currently only support updating a task's milestone assignment. Agents and CLI callers cannot rename a task, edit its description, change its type, or update its metadata without opening the app UI. This spec extends both surfaces to update every mutable task field in a single atomic call, while preserving the existing milestone behavior.++## Non-Goals++- Updating task status (already covered by `update_task_status`)+- Changing a task's project (deferred; cross-project moves have downstream display/milestone implications)+- Per-key metadata patching (full replacement only — see Decision 2)+- Updating multiple tasks in one call (batch updates)+- Visual `EditTaskIntent` (Shortcuts-friendly edit flow) — deferred to a separate ticket; this spec only updates the JSON-based `UpdateTaskIntent` and its MCP counterpart+- Tracking edit history or a "last modified" timestamp on tasks (the model has no such field)+- Renaming display IDs or task UUIDs+- Special handling for terminal-state tasks (Done / Abandoned) — they remain editable like any other task+- Enforcing length limits on `name`, `description`, or `metadata` keys/values — the underlying CloudKit constraints (~1 MB row) are surfaced as save errors++## Requirements++### 1. Name Update++**User Story:** As an agent, I want to rename a task via `update_task`, so that I can keep task names accurate as work evolves without opening the app.++**Acceptance Criteria:**++1. <a name="1.1"></a>WHEN the request includes a `name` field of type string with a non-empty value after trimming leading/trailing whitespace, the system SHALL set the task's name to the trimmed value and persist the change.+2. <a name="1.2"></a>WHEN the request includes a `name` field of type string that is empty or contains only whitespace, the system SHALL return an error `"Task name cannot be empty"` and SHALL NOT modify any task field.+3. <a name="1.3"></a>WHEN the request includes a `name` field that is not a string (number, boolean, null, array, object), the system SHALL return an error `"name must be a string"` and SHALL NOT modify any task field.+4. <a name="1.4"></a>WHEN the request omits the `name` field, the system SHALL leave the task's name unchanged.++### 2. Description Update++**User Story:** As an agent, I want to edit or clear a task's description, so that I can keep notes and context current.++**Acceptance Criteria:**++1. <a name="2.1"></a>WHEN the request includes a `description` field of type string with a non-empty value after trimming leading/trailing whitespace, the system SHALL set the task's description to the trimmed value and persist the change.+2. <a name="2.2"></a>WHEN the request includes a `description` field of type string that is empty or contains only whitespace, the system SHALL clear the task's stored description (set to nil) so that a subsequent fetch of the task returns no description value.+3. <a name="2.3"></a>WHEN the request includes a `description` field that is not a string (number, boolean, null, array, object), the system SHALL return an error `"description must be a string"` and SHALL NOT modify any task field.+4. <a name="2.4"></a>WHEN the request omits the `description` field, the system SHALL leave the task's description unchanged.++### 3. Type Update++**User Story:** As an agent, I want to reclassify a task's type, so that I can correct misclassifications (e.g., reclassify a bug as a chore).++**Acceptance Criteria:**++1. <a name="3.1"></a>WHEN the request includes a `type` field of type string whose value matches one of `bug`, `feature`, `chore`, `research`, `documentation` (exact lowercase match, no canonicalization), the system SHALL set the task's type to that value and persist the change.+2. <a name="3.2"></a>WHEN the request includes a `type` field of type string whose value does not match any valid `TaskType`, the system SHALL return an error of the form `"Invalid type: {value}. Must be one of: bug, feature, chore, research, documentation"` and SHALL NOT modify any task field.+3. <a name="3.3"></a>WHEN the request includes a `type` field that is not a string, the system SHALL return an error `"type must be a string"` and SHALL NOT modify any task field.+4. <a name="3.4"></a>WHEN the request omits the `type` field, the system SHALL leave the task's type unchanged.++### 4. Metadata Update (Full Replacement)++**User Story:** As an agent, I want to overwrite a task's metadata dictionary in one call, so that I can sync agent-side state to the task without read-modify-write per key.++**Acceptance Criteria:**++1. <a name="4.1"></a>WHEN the request includes a `metadata` field of type object whose entries all have string values, the system SHALL replace the task's entire metadata dictionary with the provided entries and persist the change.+2. <a name="4.2"></a>WHEN the request includes a `metadata` field of type object that is empty (`{}`), the system SHALL clear the task's stored metadata so that a subsequent fetch returns no metadata.+3. <a name="4.3"></a>WHEN the request includes a `metadata` field that is not a JSON object (string, number, boolean, null, array), the system SHALL return an error `"metadata must be an object with string values"` and SHALL NOT modify any task field.+4. <a name="4.4"></a>WHEN the request includes a `metadata` object whose values contain any non-string entry (number, boolean, null, nested object, array), the system SHALL return an error `"metadata values must be strings"` and SHALL NOT modify any task field. This requires strict validation; the existing `IntentHelpers.stringMetadata` helper silently drops non-string values and SHALL NOT be reused for this enforcement.+5. <a name="4.5"></a>WHEN the request omits the `metadata` field, the system SHALL leave the task's metadata unchanged.++### 5. Atomic Multi-Field Updates++**User Story:** As an agent, I want multiple field updates in a single call to be all-or-nothing, so that a malformed value cannot leave the task in a partially updated state.++**Acceptance Criteria:**++1. <a name="5.1"></a>WHEN the request includes multiple valid update fields (any combination of `name`, `description`, `type`, `metadata`, plus existing milestone fields), the system SHALL apply all changes and SHALL persist them in a single save operation.+2. <a name="5.2"></a>IF any one provided update field fails validation, THEN the system SHALL return an error describing one validation failure and SHALL leave every task field unchanged. The choice of which failure to surface when multiple fields are invalid is unspecified — callers SHALL NOT depend on a particular ordering, and the MCP tool and App Intent MAY surface different error messages for the same multi-field-invalid input (parity per [8.1](#8.1) covers successful response shape, not error-message text).+3. <a name="5.3"></a>IF the underlying save operation fails after validation succeeds (including CloudKit-size or other persistence-layer errors), THEN the system SHALL invoke `ModelContext.safeRollback()` to revert any in-memory mutations and SHALL return an error describing the save failure. All save-time failures route through this single path; there is no separate error code for size-limit failures.++### 6. Identifier-Only No-Op++**User Story:** As an agent, I want to call `update_task` with only an identifier and no update fields, so that I can echo current task state for validation without spurious writes.++**Acceptance Criteria:**++1. <a name="6.1"></a>WHEN the request includes only a task identifier (`displayId` or `taskId`) and no field that semantically mutates the task — meaning the request omits `name`, `description`, `type`, `metadata`, `milestone`, `milestoneDisplayId`, and `clearMilestone` — the system SHALL resolve the task, SHALL NOT invoke a save operation, and SHALL return the current task JSON.+2. <a name="6.2"></a>An explicit clear request (`description: ""`, `description: "   "`, `metadata: {}`, `clearMilestone: true`) IS a mutation and SHALL trigger save, even when no other update fields are present.+3. <a name="6.3"></a>Unknown JSON fields not defined by this tool's schema SHALL be silently ignored. The presence of unknown fields SHALL NOT tip a request from the no-op path in [6.1](#6.1) to the save path; conversely, the presence of a known mutating field SHALL always trigger save, regardless of any unknown fields alongside it.++### 7. Backward Compatibility with Milestone Fields++**User Story:** As an agent already using `update_task` for milestone assignment, I want existing milestone behavior to keep working, so that my current scripts and prompts do not break.++**Acceptance Criteria:**++1. <a name="7.1"></a>The system SHALL continue to accept `milestone`, `milestoneDisplayId`, and `clearMilestone` with the same semantics as before this change.+2. <a name="7.2"></a>WHEN the request combines milestone fields with name, description, type, or metadata updates, the system SHALL validate every field before applying any change and SHALL persist the combined result in a single save.++### 8. Parity Between MCP Tool and App Intent++**User Story:** As a user invoking the feature via either Claude Code (MCP) or Apple Shortcuts (App Intent), I want both surfaces to accept the same input shape and apply the same rules, so that I can switch between them without surprise.++**Acceptance Criteria:**++1. <a name="8.1"></a>The MCP `update_task` tool and the `UpdateTaskIntent` App Intent SHALL accept the same JSON input shape, apply the same validation rules, and return responses carrying the same field values and the same field-omission semantics (representation format may differ — JSON string vs `IntentResult` — but the data SHALL be equivalent).+2. <a name="8.2"></a>The MCP tool definition (`tools/list`) input schema SHALL describe every supported input field (`name`, `description`, `type`, `metadata`, plus existing milestone and identifier fields), and the prose description for each field SHALL document non-obvious behavior — including that `description: ""` clears the description and `metadata: {}` clears all metadata. The `UpdateTaskIntent` `Input` parameter's `description` SHALL contain the same field documentation.++### 9. Response Shape++**User Story:** As a caller, I want the post-update response to reflect the updated task fully, so that I do not need a follow-up `query_tasks` call to confirm the change.++**Acceptance Criteria:**++1. <a name="9.1"></a>The response JSON SHALL use the detailed task shape, computed purely from the task's post-update model state (not from which fields the request touched). It SHALL include `taskId`, `name`, `type`, `status`, `displayId` (when allocated), `projectId`, `projectName`, `description` (when non-nil and non-empty), `metadata` (when non-empty), and `milestone` summary (when assigned). Fields whose post-update value is empty/nil SHALL be omitted from the response — they SHALL NOT appear as `""`, `null`, or `{}`. The response SHALL NOT include `comments`, `creationDate`, `lastStatusChangeDate`, or `completionDate`.

diff --git a/specs/update-task-all-fields/design.md b/specs/update-task-all-fields/design.mdnew file mode 100644index 0000000..48a2408--- /dev/null+++ b/specs/update-task-all-fields/design.md@@ -0,0 +1,348 @@+# Design: Extend update_task to Support All Task Fields (T-650)++## Overview++Extend the MCP `update_task` tool and `UpdateTaskIntent` to update `name`, `description`, `type`, and `metadata` in addition to the existing milestone fields. Both surfaces share a single validator and applier so that validation rules and applied changes are guaranteed identical. The service layer already supports every field via `TaskService.updateTask`; this work is entirely in the handler/intent and the tool schema.++## Architecture++### Call flow (both surfaces)++```+   ┌── MCP handleUpdateTask ──┐         ┌── UpdateTaskIntent.execute ──┐+   │ parse JSON args           │         │ parse JSON input              │+   │ resolve task              │         │ resolve task                  │+   └──────────┬────────────────┘         └──────────────┬────────────────┘+              │                                         │+              ▼                                         ▼+   ┌─────────────────────────────────────────────────────────────────┐+   │ TaskUpdateValidator.validate(args, task, milestoneService)      │+   │  → Result<ValidatedTaskUpdate, TaskUpdateValidationError>       │+   │  ── walks: name → description → type → metadata → milestone     │+   │  ── purely functional; no model mutations                       │+   └──────────────────────────┬──────────────────────────────────────┘+                              │+              ┌───────────────┴──────────────┐+              │                              │+        update.hasChanges == false?   update.hasChanges == true?+              │                              │+              ▼                              ▼+        skip save                  TaskUpdateValidator.apply(update,+        return current task echo   task, taskService, milestoneService)+                                            │+                                            ▼+                                   try taskService.save()+                                   (which already wraps safeRollback)+                                            │+                                            ▼+                              IntentHelpers.taskUpdateResponseDict(task)+                              → JSON+```++Each surface maps `TaskUpdateValidationError` to its own error envelope:+- MCP: `errorResult(error.mcpMessage)` → `MCPToolResult(isError: true, ...)`+- App Intent: `error.intentError.json` → IntentError-encoded string++### Module placement++A new file `Transit/Transit/Intents/TaskUpdateValidator.swift` holds the shared validator, applier, value types, error enum, **and** the strict metadata helper. Co-locating `strictStringMetadata` with the validator avoids creating a dependency edge from generic `IntentHelpers.swift` to the feature-specific `TaskUpdateValidationError` enum — the helper has exactly one caller.++`IntentHelpers.swift` gains one small addition:+- `taskUpdateResponseDict(_:) -> [String: Any]` — builds the AC 9.1 response shape (no `comments`, no date fields, omits cleared fields). This is the *only* response builder used by `update_task` post-T-650; `UpdateTaskIntent.buildResponse` (Transit/Transit/Intents/UpdateTaskIntent.swift:103) is deleted as part of this change. Cannot reuse `taskToDict(detailed: true)` because it emits `description` as `task.taskDescription as Any` which encodes `nil` as `NSNull` (violates [9.1](#9.1)) and it also emits `lastStatusChangeDate`/`completionDate`.++### Integration points++| Site | Change |+|---|---|+| `MCPToolHandler.handleUpdateTask` (Transit/Transit/MCP/MCPToolHandler.swift:787) | Replace milestone-only logic with: validate → apply → save → response |+| `UpdateTaskIntent.execute` (Transit/Transit/Intents/UpdateTaskIntent.swift:50) | Same shape; surface-specific error translation |+| `UpdateTaskIntent.buildResponse` / `applyMilestoneChange` (Transit/Transit/Intents/UpdateTaskIntent.swift:77,103) | **Delete.** Replaced by `IntentHelpers.taskUpdateResponseDict` and the shared validator/applier. |+| `MCPToolDefinitions.updateTask` (Transit/Transit/MCP/MCPToolDefinitions.swift:231) | Add `name`, `description`, `type`, `metadata` to schema; update `updateTaskDescription` |+| `UpdateTaskIntent.Input` parameter description | Document new fields incl. empty-string-clears semantic |+| `TaskService` (Transit/Transit/Services/TaskService.swift) | Add `func rollback() { modelContext.safeRollback() }` for handler-level rollback when `apply()` throws (apply-failure rollback gap, see Error Handling section) |++The existing milestone path (`clearMilestone`, `milestone`, `milestoneDisplayId`) is preserved by folding it into `TaskUpdateValidator` rather than running it before/after; this is what makes [5.1](#5.1)/[7.2](#7.2) atomic.++## Components and Interfaces++### TaskUpdateValidator (new file)++```swift+// Transit/Transit/Intents/TaskUpdateValidator.swift++@MainActor+enum TaskUpdateValidator {+    static func validate(+        _ args: [String: Any],+        task: TransitTask,+        milestoneService: MilestoneService+    ) -> Result<ValidatedTaskUpdate, TaskUpdateValidationError>++    static func apply(+        _ update: ValidatedTaskUpdate,+        to task: TransitTask,+        taskService: TaskService,+        milestoneService: MilestoneService+    ) throws++    // Strict metadata coercion. Sole caller is `validate`; co-located to avoid+    // dropping a feature-specific error type into IntentHelpers.+    static func strictStringMetadata(+        from value: Any?+    ) -> Result<FieldChange<[String: String]>, TaskUpdateValidationError>+}++/// A pure data carrier for a fully-validated update. Field representation+/// makes invalid states unrepresentable:+/// - Non-clearable fields (`name`, `type`) use `Optional<T>` — nil = no change.+/// - Clearable fields (`description`, `metadata`) use `FieldChange<T>` to+///   distinguish "set" from "clear" cleanly.+/// - Milestone uses its own enum because "assign" carries a `Milestone`+///   instance that resolution has already located.+struct ValidatedTaskUpdate {+    let name: String?                       // trimmed, non-empty; nil = no change+    let description: FieldChange<String>    // .set(trimmed-non-empty), .clear, or .noChange+    let type: TaskType?                     // nil = no change+    let metadata: FieldChange<[String: String]>  // .set(non-empty), .clear (was {}), or .noChange+    let milestoneAction: MilestoneAction?   // nil = no change++    var hasChanges: Bool {+        name != nil || description.isChange || type != nil+            || metadata.isChange || milestoneAction != nil+    }+}++enum FieldChange<T> {+    case noChange+    case set(T)+    case clear++    var isChange: Bool { if case .noChange = self { false } else { true } }+}++enum MilestoneAction {+    case assign(Milestone)+    case clear+}++enum TaskUpdateValidationError {+    case invalidInput(String)              // type/value validation: "name must be a string", "Invalid type: x. ..."+    case milestoneNotFound(message: String)+    case duplicateMilestoneDisplayID(message: String)  // mirrors existing MilestoneService.Error.duplicateDisplayID handling+    case milestoneProjectMismatch+    case projectRequiredForMilestone++    var mcpMessage: String { /* extract a single string for errorResult */ }+    var intentError: IntentError {+        switch self {+        case .invalidInput(let hint): .invalidInput(hint: hint)+        case .milestoneNotFound(let m): .milestoneNotFound(hint: m)+        case .duplicateMilestoneDisplayID(let m): .internalError(hint: m)+        case .milestoneProjectMismatch: .milestoneProjectMismatch(hint: "Milestone and task must belong to the same project")+        case .projectRequiredForMilestone: .invalidInput(hint: "Task must belong to a project before assigning a milestone")+        }+    }+}+```++The `FieldChange<T>` enum makes `description` and `metadata` self-describing — the applier pattern-matches on the case rather than juggling an optional + boolean flag. Translation to `TaskService.updateTask`'s `(description: String?, clearDescription: Bool)` parameter pair happens inside `apply`:++```swift+// apply translates FieldChange to service parameters+let (descArg, clearDesc): (String?, Bool) = switch update.description {+case .noChange: (nil, false)+case .set(let s): (s, false)+case .clear:     (nil, true)+}+```++**Behavioral contract for `validate`:**+- Pure: never mutates `task` or any model.+- Stops at the first failure encountered while walking fields in this order: `name`, `description`, `type`, `metadata`, milestone fields. The order is deterministic in the implementation, but not part of the public contract per [5.2](#5.2) — tests for "any one of multiple invalid fields produces an error" SHALL NOT depend on which one surfaces.+- Milestone resolution requires the milestone service to call `findByDisplayID` / `findByName`, which read the model context. Reads are allowed; writes are not. Atomicity between read and the subsequent `apply` is provided by `@MainActor` isolation — no other actor can mutate the context between the validator returning and the applier running.++**Behavioral contract for `apply`:**+- Calls `taskService.updateTask(task, ..., save: false)` once with all non-milestone fields if any are present.+- Calls `milestoneService.setMilestone(_, on:, save: false)` once if milestone changed. Note: `clearMilestone: true` against an already-unassigned task counts as a "change" for the purposes of `hasChanges` and will trigger a (no-op) save. This matches the existing pre-T-650 behavior and is left as-is — the redundant save is harmless and the alternative (per-field equality checks) is more code with no observable benefit.+- Performs no `save()`; that is the caller's responsibility so a single transaction covers every mutation ([5.1](#5.1)).++### Strict metadata helper (co-located in TaskUpdateValidator.swift)++`TaskUpdateValidator.strictStringMetadata(from:) -> Result<FieldChange<[String: String]>, TaskUpdateValidationError>`++Behavioral contract:+- `value == nil` → `.success(.noChange)` (field omitted)+- `value` is `[String: String]` empty, or `[String: Any]` empty → `.success(.clear)` (caller passed `{}`)+- `value` is `[String: String]` non-empty → `.success(.set(value))`+- `value` is `[String: Any]` with all-string values → `.success(.set(coerced))`. "String values" means Swift `String`; JSON numbers and booleans deserialised by `JSONSerialization` as `NSNumber` fail the `as? String` cast and are rejected by the next rule.+- `value` is `[String: Any]` with any non-string entry → `.failure(.invalidInput("metadata values must be strings"))`+- `value` is any other JSON type → `.failure(.invalidInput("metadata must be an object with string values"))`++### Response builder (IntentHelpers extension)++```swift+extension IntentHelpers {+    @MainActor+    static func taskUpdateResponseDict(_ task: TransitTask) -> [String: Any]+}+```++Returns the AC 9.1 shape exactly:+- Always present: `taskId`, `name`, `type`, `status`+- Present when non-nil: `displayId`, `projectId`, `projectName`+- Present when non-nil **and non-empty**: `description`+- Present when non-empty: `metadata`+- Present when assigned: `milestone` (via existing `milestoneInfoDict`)+- Never present: `comments`, `creationDate`, `lastStatusChangeDate`, `completionDate`++The output is purely a function of the task's current model state, not of which fields the request touched ([9.1](#9.1)).++### MCPToolHandler.handleUpdateTask (rewritten)++```swift+private func handleUpdateTask(_ args: [String: Any]) -> MCPToolResult {+    // Identifier resolution (preserve existing T-634 / T-808 behavior)+    if args["displayId"] != nil, IntentHelpers.parseIntValue(args["displayId"]) == nil {+        return errorResult("displayId must be an integer")+    }+    let task: TransitTask+    do { task = try taskService.resolveTask(from: args) }+    catch TaskService.Error.invalidIdentifier(let field) {+        return errorResult(IntentHelpers.invalidIdentifierHint(for: field))+    } catch {+        return errorResult("Provide either displayId (integer) or taskId (UUID string)")+    }++    // Validate+    let update: ValidatedTaskUpdate+    switch TaskUpdateValidator.validate(args, task: task, milestoneService: milestoneService) {+    case .success(let v): update = v+    case .failure(let e): return errorResult(e.mcpMessage)+    }++    // No-op echo+    guard update.hasChanges else {+        return textResult(IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task)))+    }++    // Apply + save. Two distinct catch paths to satisfy AC 5.2 (apply mid-throw+    // must roll back partial mutations) and AC 5.3 (save failure path).+    do {+        try TaskUpdateValidator.apply(update, to: task, taskService: taskService, milestoneService: milestoneService)+    } catch {+        taskService.rollback()  // undo any partial in-memory mutations from updateTask before setMilestone threw+        return errorResult("Update failed: \(error)")+    }+    do {+        try taskService.save()  // wraps safeRollback internally on save failure+    } catch {+        return errorResult("Update failed: \(error)")+    }++    return textResult(IntentHelpers.encodeJSON(IntentHelpers.taskUpdateResponseDict(task)))+}+```++### UpdateTaskIntent.execute (rewritten)++Mirror structure; differs only in:+- Returns a JSON string directly (not `MCPToolResult`).+- Maps `TaskUpdateValidationError → IntentError → .json` instead of `errorResult(...)`.+- Save and apply failures map to `IntentError.internalError(hint:)` so the App Intent JSON envelope carries the correct error code (matches `IntentError`'s existing INTERNAL_ERROR mapping, not raw error interpolation).+- Existing helpers (`IntentHelpers.parseJSON`, `IntentHelpers.resolveTask`) handle parse/resolve.++### MCPToolDefinitions.updateTask (extended schema)++```swift+static let updateTask = MCPToolDefinition(+    name: "update_task",+    description: "Update a task's mutable fields (name, description, type, metadata, milestone) "+              + "in a single atomic call. Identify task by displayId or taskId.",+    inputSchema: .object(properties: [+        "displayId": .integer("Task display ID (e.g. 42 for T-42)"),+        "taskId": .string("Task UUID"),+        "name": .string("New task name (trimmed; must be non-empty after trim)"),+        "description": .string("New description. Pass \"\" or whitespace-only to clear."),+        "type": .string("New type. Exact lowercase: bug, feature, chore, research, documentation."),+        "metadata": .object("Replaces entire metadata dictionary. Pass {} to clear all. Values must be strings."),+        "milestone": .string("Milestone name (within task's project). Use clearMilestone to unassign."),+        "milestoneDisplayId": .integer("Milestone display ID (takes precedence over name)"),+        "clearMilestone": .boolean("Set true to remove milestone assignment")+    ], required: [])+)+```++The prose for `description` and `metadata` carries the discoverability requirement from [8.2](#8.2).++## Data Models++None. No SwiftData schema changes. `ValidatedTaskUpdate`, `MilestoneAction`, and `TaskUpdateValidationError` are transient value types in the validator file.++## Error Handling++| Failure source | MCP surface | App Intent surface |+|---|---|---|+| Identifier missing/invalid (existing) | `errorResult("Provide either displayId ...")` or field-specific hint | `IntentError.invalidInput(...)` |+| Non-string `name`/`description`/`type` | `errorResult("name must be a string")` etc. | `IntentError.invalidInput(hint:)` |+| Empty-after-trim `name` | `errorResult("Task name cannot be empty")` | `IntentError.invalidInput(hint:)` |+| Invalid `type` enum value | `errorResult("Invalid type: x. Must be one of: ...")` | same hint, wrapped in `INVALID_INPUT` |+| Non-object `metadata` | `errorResult("metadata must be an object with string values")` | same hint, INVALID_INPUT |+| Non-string `metadata` value | `errorResult("metadata values must be strings")` | same hint, INVALID_INPUT |+| Milestone not found | `errorResult("No milestone with displayId N")` | `IntentError.milestoneNotFound(hint:)` |+| Milestone duplicate displayID | `errorResult("Duplicate milestone identifier detected for displayId N")` | `IntentError.internalError(hint:)` (mirrors existing `assignMilestone` behavior) |+| Milestone/project mismatch | `errorResult("Milestone and task must belong to the same project")` | `IntentError.milestoneProjectMismatch(hint:)` |+| Apply mid-throw | `errorResult("Update failed: ...")` + explicit `taskService.rollback()` | `IntentError.internalError(hint:)` + explicit `taskService.rollback()` |+| Save failure | `errorResult("Update failed: ...")` (rollback inside `taskService.save()`) | `IntentError.internalError(hint:)` |++Error-message text is allowed to differ across surfaces ([5.2](#5.2) carve-out); the validator emits a structured error, each surface chooses how to render it.++## Testing Strategy++### Unit tests++Two test files extended; both use Swift Testing + `MCPTestHelpers` / direct `UpdateTaskIntent.execute` calls with in-memory `TestModelContainer`.++**Coverage matrix per AC** (added test names suggested in parentheses):++| AC | MCP test | Intent test |+|---|---|---|+| [1.1](#1.1) name set | `updateName_setsTrimmedName` | parallel |+| [1.2](#1.2) name empty/whitespace rejected | `updateName_rejectsEmptyAndWhitespace` | parallel |+| [1.3](#1.3) name non-string rejected | `updateName_rejectsNonString` | parallel |+| [1.4](#1.4) name omission preserves | `omittingNamePreservesIt` (combined with other omission ACs below) | parallel |+| [2.1](#2.1) description set + trim | `updateDescription_setsTrimmed` | parallel |+| [2.2](#2.2) description empty / whitespace clears | `updateDescription_emptyAndWhitespaceClears` | parallel |+| [2.3](#2.3) description non-string rejected | `updateDescription_rejectsNonString` | parallel |+| [2.4](#2.4) description omission preserves | `omittingDescriptionPreservesIt` | parallel |+| [3.4](#3.4) type omission preserves | `omittingTypePreservesIt` | parallel |+| [4.5](#4.5) metadata omission preserves | `omittingMetadataPreservesIt` | parallel |+| [3.1](#3.1) type set | `updateType_setsValidType` | parallel |+| [3.2](#3.2) invalid type rejected | `updateType_rejectsInvalidValue` | parallel |+| [3.3](#3.3) type non-string rejected | `updateType_rejectsNonString` | parallel |+| [4.1](#4.1) metadata replace | `updateMetadata_replacesEntireDict` | parallel |+| [4.2](#4.2) metadata `{}` clears | `updateMetadata_emptyDictClears` | parallel |+| [4.3](#4.3) metadata non-object rejected | `updateMetadata_rejectsNonObject` | parallel |+| [4.4](#4.4) metadata non-string value rejected | `updateMetadata_rejectsNonStringValues` | parallel |+| [5.1](#5.1) atomic multi-field update | `updateMultipleFields_allAppliedAtomically` | parallel |+| [5.2](#5.2) validation failure leaves all fields unchanged | `updateMixed_invalidFieldRollsBackAll` | parallel |+| [5.3](#5.3) save failure path | **No handler-level test.** `TaskService` is `final` with no protocol seam; introducing one is out of scope for T-650 (would affect every handler). The catch branch is one line. AC 5.3 coverage is provided by the existing `TaskService.save()` service-layer test that asserts `safeRollback()` runs on failure. This carve-out is documented in the test file. |+| [5.2](#5.2) apply-throw rollback | `applyThrows_taskUntouched` — stubs `milestoneService.setMilestone` to throw after `taskService.updateTask` mutates, asserts task fields are reverted. Achievable via `MilestoneService` test seam if available; otherwise covered by inspection of the handler's catch block plus the existing `setMilestone` error-path service tests. |+| [6.1](#6.1)/[6.2](#6.2) no-op echo | `identifierOnly_doesNotSave_returnsCurrent` + `metadataEmpty_isMutation_triggersSave` | parallel |+| [6.3](#6.3) unknown fields ignored | `unknownFieldsIgnored_doNotBlockNoOp` | parallel |+| [7.1](#7.1) milestone backward compat | existing milestone tests must continue to pass unchanged | parallel |+| [7.2](#7.2) milestone + field atomic | `updateMilestoneAndName_singleSave` | parallel |+| [7.x](#7.1) `clearMilestone` on already-unassigned task | `clearMilestone_onAlreadyUnassigned_savesAnyway` — documents the redundant-save behavior so a future refactor doesn't silently optimise it away | parallel |+| [8.2](#8.2) tool schema includes new fields and clearing prose | `toolsListIncludesNewUpdateTaskFields` — asserts each of `name`/`description`/`type`/`metadata` is present and that `description`'s prose contains the substring `"clear"` and `metadata`'s prose contains `"clear"`/`"{}"` (exact-substring match, not just non-empty) | parallel test reads `UpdateTaskIntent.$input.description` substring on the same wording |+| [9.1](#9.1) response shape: cleared fields omitted | `responseOmitsClearedDescriptionAndMetadata` + `responseExcludesCommentsAndDateFields` | parallel |++### Cross-surface parity test++One additional test asserts that for the same valid `args` (success case only), MCP and Intent return JSON with equivalent data payloads — parses both, asserts dict equality on the full key set. This enforces [8.1](#8.1) at the contract level without requiring byte-equal serialisation. Error-response parity is explicitly NOT asserted; per [5.2](#5.2) the two surfaces may surface different error messages for the same invalid input. Covering both in one test would mask omission-rule divergence that this contract test exists to catch.++### Property-based testing++Not applicable. The contract has no algebraic invariants worth generating inputs against — every behavior is a small set of input shape × field combinations, and example-based coverage is exhaustive.++### Schema test++Assert that `MCPToolDefinitions.updateTask.inputSchema` lists all new properties (`name`, `description`, `type`, `metadata`) with non-empty `description` strings ([8.2](#8.2)).

diff --git a/specs/update-task-all-fields/decision_log.md b/specs/update-task-all-fields/decision_log.mdnew file mode 100644index 0000000..0ddda59--- /dev/null+++ b/specs/update-task-all-fields/decision_log.md@@ -0,0 +1,126 @@+# Decision Log: Extend update_task to Support All Task Fields++## Decision 1: Description Clearing via Empty / Whitespace-Only String++**Date**: 2026-05-22+**Status**: accepted++### Context++The `update_task` tool needs a way for a caller to clear a task's description. Three signaling options were considered: a JSON empty string (`""`), JSON `null`, and an explicit boolean flag (`clearDescription: true`). `TaskService.updateTask` already supports a `clearDescription` flag at the service layer.++A secondary question: how should whitespace-only descriptions (`"   "`, `"\n\n"`) be treated? The name field trims and rejects empty-after-trim; description could either mirror name (trim + reject), trim and treat empty-after-trim as a clear signal, or store whitespace verbatim.++### Decision++Treat any `description` field whose value is the empty string OR is only whitespace as the clear signal: trim leading/trailing whitespace; if the trimmed value is empty, clear the description. Otherwise, store the trimmed value. JSON `null` and any explicit `clearDescription` flag are rejected as invalid for this tool's surface area.++### Rationale++Empty string is the simplest wire-format contract: one field, one type (string), one validation rule (must be a string). Callers don't need to know about a side-channel flag. Tasks render no differently when the description is empty vs absent, so collapsing those states loses no observable information. Trimming and treating whitespace-only as a clear signal is consistent with how `name` handles whitespace and prevents storing visually-empty garbage.++This semantic is non-obvious from JSON Schema alone — JSON Schema can only state that the field is a string. AC 8.2 requires the prose description in `tools/list` and the App Intent parameter description to document this behavior explicitly.++### Alternatives Considered++- **Explicit `clearDescription` flag**: Mirrors `clearMilestone` for consistency within this tool — Rejected because it doubles the surface area for an operation with no useful "set to literal empty string" alternative, and because the empty-string signal is already unambiguous.+- **JSON `null`**: Standard JSON way to express "no value" — Rejected because it complicates Swift `[String: Any]` decoding and the existing pattern across MCP handlers is to reject `null` for typed fields.+- **Trim + reject empty-after-trim**: Strictest; treats description like name — Rejected because it leaves no inline clearing path, forcing callers to add a flag or omit the field entirely.+- **No trim; store whitespace verbatim**: Most literal — Rejected because storing `"   "` is never what the caller wants and diverges from name's trim rule.++### Consequences++**Positive:**+- Smallest schema surface+- One validation rule per field+- Consistent whitespace handling between `name` and `description`+- Handler translates `description: ""` or whitespace-only to `clearDescription: true` when calling the service++**Negative:**+- A caller who legitimately wants to set the description to a literal whitespace-only string cannot — acceptable because the model treats those states identically in display/query+- The empty-string-clears semantic must be discoverable through prose documentation rather than schema type information+- The `update_task` tool now has two clearing idioms: `clearMilestone: true` (boolean flag) and `description: ""` (sentinel value). A future change could converge them, but breaking the existing `clearMilestone` contract is out of scope here++---++## Decision 2: Metadata Full Replacement with Last-Writer-Wins Semantics++**Date**: 2026-05-22+**Status**: accepted++### Context++The user brief said "support clearing individual keys" for metadata. Two semantics were considered: (a) full replacement — the provided dict overwrites the stored dict entirely; (b) partial merge with null-clears-key — provided keys overwrite, omitted keys are preserved, null values clear specific keys. `TaskService.updateTask` currently does full replacement.++CloudKit sync means a task's metadata can be mutated by other devices between read and write, even when only one agent is calling `update_task`. The concurrency story matters for any chosen semantic.++### Decision++Pass `metadata` as a full dictionary that replaces the task's existing metadata in its entirety. To clear individual keys, the caller must read current metadata, remove the unwanted keys, and submit the modified dict. Submitting `{}` clears all metadata. Updates are last-writer-wins: a concurrent write from another device or the UI between the caller's read and `update_task`'s save can be silently overwritten.++### Rationale++Full replacement matches existing service behavior and requires no service-layer change. The wire format stays clean ([String: String] with no nullable values). The user brief's intent — "let agents clear keys" — is satisfied: the operation is supported, just via read-modify-write instead of patch-style null markers.++Last-writer-wins is acceptable for the Transit single-user model. Multi-device CloudKit writers are infrequent in practice (most metadata is written by one agent at a time), and the cost of a more sophisticated merge strategy (CRDT-style or version vectors) is far higher than the cost of the rare lost update.++### Alternatives Considered++- **Merge with null-clears-key**: `metadata: {"a": "v", "b": null}` sets a, removes b, leaves others — Rejected because it requires a new service method (`patchMetadata`), introduces `[String: String?]` Swift handling and null detection in `[String: Any]` decoding, and complicates validation (string-or-null vs string-only).+- **Separate `metadata` (replace) + `metadataPatch` (merge)**: Most expressive — Rejected as doubled surface area, doubled tests, doubled docs for a need the user can satisfy with read-modify-write today.+- **Optimistic concurrency with version tokens**: Add a `metadataVersion` or similar to detect concurrent writes — Rejected as significant model and protocol complexity for a low-probability problem in a single-user app.++### Consequences++**Positive:**+- No service-layer change+- Validation reduces to "object whose values are all strings"+- Wire format mirrors `create_task`'s `metadata` argument++**Negative:**+- Partial updates require a prior `query_tasks` call+- Concurrent writers (other devices, the UI, future second agents) can silently lose updates — documented as last-writer-wins++---++## Decision 3: Identifier-Only Call Is a No-Op Echo++**Date**: 2026-05-22+**Status**: accepted++### Context++A caller might invoke `update_task` with only `displayId` or `taskId` and no other fields — either as a probe ("does this task exist?") or accidentally. Two options: (a) treat as a no-op that returns the current task; (b) return an error requiring at least one update field.++A related ambiguity: which fields count as "mutating"? An explicit clear (`description: ""`, `metadata: {}`, `clearMilestone: true`) is structurally similar to a "no field" but is semantically a write.++### Decision++When the request includes only an identifier and no semantically mutating field, the handler resolves the task, skips the save operation, and returns the current task JSON. Explicit clears count as mutations and trigger save. Unknown JSON fields are silently ignored and do not block the no-op path.++### Rationale++The call is harmless and the response is useful (it doubles as a single-task lookup with the same response shape). Skipping the save avoids spurious CloudKit churn. An error here would reject a useful client pattern for no real benefit.++Treating explicit clears as mutations is the only consistent reading: `metadata: {}` is the documented signal for "clear all metadata" (AC 4.2) and must invoke save.++Silently ignoring unknown fields matches every other MCP handler in the codebase. It is forward-compatible: a future client sending a newer-version field will not break against an older build.++### Alternatives Considered++- **Return an error**: More explicit — Rejected because it rejects a benign call and provides no protective value.+- **Return current task and a `noOp: true` marker**: Lets callers distinguish probe from update — Rejected as protocol noise for limited benefit; callers can compare before/after themselves if needed.+- **Reject unknown fields**: Catches client typos early — Rejected because it breaks forward compatibility and diverges from the rest of the MCP surface.++### Consequences++**Positive:**+- Useful echo / existence-check pattern for clients+- No write amplification on probe calls+- Forward-compatible against future schema additions++**Negative:**+- Slightly less explicit about intent — a caller who forgot to add update fields gets no signal+- Client typos (`nmae`, `descrption`) are silently ignored — acceptable trade for forward compatibility++---

diff --git a/specs/update-task-all-fields/tasks.md b/specs/update-task-all-fields/tasks.mdnew file mode 100644index 0000000..1c6d13b--- /dev/null+++ b/specs/update-task-all-fields/tasks.md@@ -0,0 +1,132 @@+---+references:+    - specs/update-task-all-fields/requirements.md+    - specs/update-task-all-fields/design.md+    - specs/update-task-all-fields/decision_log.md+---+# Tasks: Extend update_task to Support All Task Fields (T-650)++## Foundation++- [x] 1. Add rollback() method to TaskService <!-- id:2r6h8qp -->+  - Single public method `func rollback() { modelContext.safeRollback() }` so handlers can revert in-memory mutations when apply() throws mid-way. Wiring-only task; no test required (the underlying safeRollback already has service-layer tests).+  - Requirements: [5.2](requirements.md#5.2), [5.3](requirements.md#5.3)+  - References: Transit/Transit/Services/TaskService.swift, Transit/Transit/Extensions/ModelContext+SafeRollback.swift++- [x] 2. Create TaskUpdateValidator.swift with value types and function stubs <!-- id:2r6h8qk -->+  - New file: Transit/Transit/Intents/TaskUpdateValidator.swift+  - Declare value types: FieldChange<T> (case noChange/set(T)/clear with isChange computed property), MilestoneAction (case assign(Milestone)/clear), ValidatedTaskUpdate struct (name: String?, description: FieldChange<String>, type: TaskType?, metadata: FieldChange<[String:String]>, milestoneAction: MilestoneAction?, hasChanges: Bool), TaskUpdateValidationError enum with cases .invalidInput(String), .milestoneNotFound(message:), .duplicateMilestoneDisplayID(message:), .milestoneProjectMismatch, .projectRequiredForMilestone+  - Add computed properties mcpMessage: String and intentError: IntentError on TaskUpdateValidationError (duplicateMilestoneDisplayID maps to .internalError, milestoneProjectMismatch to .milestoneProjectMismatch, etc.)+  - Stub function signatures only (return fatalError or .failure(.invalidInput("not implemented"))) for validate, apply, strictStringMetadata so test files can import the symbols+  - Types-only / wiring task per design — exempt from TDD pairing rule+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [5.1](requirements.md#5.1), [5.2](requirements.md#5.2), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2), [8.1](requirements.md#8.1)+  - References: specs/update-task-all-fields/design.md++## Shared validator and applier++- [x] 3. Write tests for TaskUpdateValidator (validate, apply, strictStringMetadata) <!-- id:2r6h8ql -->+  - New test file: Transit/TransitTests/TaskUpdateValidatorTests.swift (Swift Testing, @MainActor @Suite(.serialized))+  - validate: cover all field combinations — name (trim/empty/non-string), description (trim+set/empty-clears/whitespace-clears/non-string), type (valid lowercase/invalid/non-string), metadata (replace/{}/non-object/non-string values via NSNumber from JSONSerialization)+  - validate: milestone path delegates to existing patterns (milestone displayId/name/clearMilestone) and surfaces .milestoneNotFound, .duplicateMilestoneDisplayID, .milestoneProjectMismatch correctly+  - validate: returns Result with hasChanges flag — identifier-only args produce hasChanges == false even with unknown fields present+  - strictStringMetadata: explicitly test that JSONSerialization-decoded numbers (NSNumber) fail the cast and return .failure(.invalidInput("metadata values must be strings"))+  - apply: covers .set/.clear/.noChange translation for description and metadata, MilestoneAction.assign/.clear, atomicity (mutations applied in-memory, no save() called by apply itself)+  - apply: throws if milestoneService.setMilestone throws — test caller can call taskService.rollback() to undo+  - Blocked-by: 2r6h8qk (Create TaskUpdateValidator.swift with value types and function stubs)+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [5.1](requirements.md#5.1), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2)+  - References: specs/update-task-all-fields/design.md, Transit/TransitTests/MCPTestHelpers.swift++- [x] 4. Implement TaskUpdateValidator.validate, apply, and strictStringMetadata <!-- id:2r6h8qm -->+  - Walk fields in order: name → description → type → metadata → milestone (deterministic order, not part of public contract per AC 5.2)+  - validate is pure: never mutates the task; milestone resolution uses milestoneService read-only lookups+  - apply translates FieldChange<String> for description into TaskService.updateTask (description: String?, clearDescription: Bool) parameter pair; for metadata translates FieldChange<[String:String]> into the service metadata: parameter (.clear becomes [:], .set(d) becomes d, .noChange omits)+  - Both service calls use save: false; apply throws (does not catch) so caller handles rollback+  - Make all tests from task 3 pass+  - Blocked-by: 2r6h8ql (Write tests for TaskUpdateValidator (validate, apply, strictStringMetadata))+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [5.1](requirements.md#5.1), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2)+  - References: Transit/Transit/Intents/TaskUpdateValidator.swift, Transit/Transit/Services/TaskService.swift++## Response builder++- [x] 5. Write tests for IntentHelpers.taskUpdateResponseDict <!-- id:2r6h8qn -->+  - Append to or create a new test in Transit/TransitTests for the helper+  - Verify always-present keys: taskId, name, type, status+  - Verify present-when-non-nil: displayId, projectId, projectName, description, milestone+  - Verify present-when-non-empty: metadata+  - Verify a cleared description (taskDescription == nil) and empty metadata are OMITTED — not present as "", null, or {}+  - Verify excluded keys: comments, creationDate, lastStatusChangeDate, completionDate+  - Requirements: [9.1](requirements.md#9.1)+  - References: specs/update-task-all-fields/design.md, Transit/Transit/Intents/IntentHelpers.swift++- [x] 6. Implement IntentHelpers.taskUpdateResponseDict <!-- id:2r6h8qo -->+  - Add `@MainActor static func taskUpdateResponseDict(_ task: TransitTask) -> [String: Any]`+  - Build the dict per AC 9.1 — omit nil description (do NOT use `task.taskDescription as Any`), omit empty metadata+  - Use existing milestoneInfoDict for milestone summary+  - Make tests from task 5 pass+  - Blocked-by: 2r6h8qn (Write tests for IntentHelpers.taskUpdateResponseDict)+  - Requirements: [9.1](requirements.md#9.1)+  - References: Transit/Transit/Intents/IntentHelpers.swift++## MCP surface++- [x] 7. Extend MCPUpdateTaskTests.swift with new field-update tests <!-- id:2r6h8qq -->+  - Add tests per design matrix: updateName_setsTrimmedName, updateName_rejectsEmptyAndWhitespace, updateName_rejectsNonString, updateDescription_setsTrimmed, updateDescription_emptyAndWhitespaceClears, updateDescription_rejectsNonString, updateType_setsValidType, updateType_rejectsInvalidValue, updateType_rejectsNonString, updateMetadata_replacesEntireDict, updateMetadata_emptyDictClears, updateMetadata_rejectsNonObject, updateMetadata_rejectsNonStringValues+  - Omission preservation: omittingNamePreservesIt, omittingDescriptionPreservesIt, omittingTypePreservesIt, omittingMetadataPreservesIt+  - Atomicity: updateMultipleFields_allAppliedAtomically, updateMixed_invalidFieldRollsBackAll, applyThrows_taskUntouched+  - No-op + unknown fields: identifierOnly_doesNotSave_returnsCurrent, metadataEmpty_isMutation_triggersSave, unknownFieldsIgnored_doNotBlockNoOp+  - Milestone parity: updateMilestoneAndName_singleSave, clearMilestone_onAlreadyUnassigned_savesAnyway (documents redundant-save behavior)+  - Response shape: responseOmitsClearedDescriptionAndMetadata, responseExcludesCommentsAndDateFields+  - Schema test (AC 8.2): toolsListIncludesNewUpdateTaskFields — assert presence of name/description/type/metadata in MCPToolDefinitions.updateTask.inputSchema AND that description's prose contains 'clear' and metadata's prose contains 'clear' or '{}' (exact-substring match)+  - Existing milestone tests (setMilestoneByDisplayId, clearMilestone, etc.) MUST continue to pass — do not delete or modify them+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [1.4](requirements.md#1.4), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [2.4](requirements.md#2.4), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [3.4](requirements.md#3.4), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [4.5](requirements.md#4.5), [5.1](requirements.md#5.1), [5.2](requirements.md#5.2), [6.1](requirements.md#6.1), [6.2](requirements.md#6.2), [6.3](requirements.md#6.3), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2), [8.2](requirements.md#8.2), [9.1](requirements.md#9.1)+  - References: Transit/TransitTests/MCPUpdateTaskTests.swift, specs/update-task-all-fields/design.md++- [x] 8. Rewrite MCPToolHandler.handleUpdateTask and extend MCPToolDefinitions.updateTask schema <!-- id:2r6h8qr -->+  - Rewrite handleUpdateTask (currently at line 787) following design's code sketch — preserve identifier-resolution preamble (T-634/T-808), then call TaskUpdateValidator.validate, handle no-op echo, two-stage do/catch (apply with explicit taskService.rollback(), then save with implicit safeRollback)+  - Use IntentHelpers.taskUpdateResponseDict for the response+  - Extend MCPToolDefinitions.updateTask.inputSchema with name/description/type/metadata properties — prose for description must contain 'clear' (e.g., 'Pass "" or whitespace-only to clear.'), prose for metadata must contain 'clear' (e.g., 'Pass {} to clear all. Values must be strings.')+  - Update updateTaskDescription (the tool top-level description string) to mention all supported fields+  - Make all tests from task 7 pass+  - Blocked-by: 2r6h8qp (Add rollback() method to TaskService), 2r6h8qm (Implement TaskUpdateValidator.validate, apply, and strictStringMetadata), 2r6h8qo (Implement IntentHelpers.taskUpdateResponseDict), 2r6h8qq (Extend MCPUpdateTaskTests.swift with new field-update tests)+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [1.4](requirements.md#1.4), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [2.4](requirements.md#2.4), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [3.4](requirements.md#3.4), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [4.5](requirements.md#4.5), [5.1](requirements.md#5.1), [5.2](requirements.md#5.2), [6.1](requirements.md#6.1), [6.2](requirements.md#6.2), [6.3](requirements.md#6.3), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2), [8.2](requirements.md#8.2), [9.1](requirements.md#9.1)+  - References: Transit/Transit/MCP/MCPToolHandler.swift, Transit/Transit/MCP/MCPToolDefinitions.swift, specs/update-task-all-fields/design.md++## App Intent surface++- [x] 9. Extend UpdateTaskIntentTests.swift with parallel field-update tests <!-- id:2r6h8qs -->+  - Parallel coverage to task 7 — call UpdateTaskIntent.execute directly, parse the returned JSON string+  - Test that UpdateTaskIntent.Input parameter description contains 'clear' wording for description and metadata (AC 8.2 parity)+  - Existing milestone tests in this file MUST continue to pass — do not delete or modify them+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [1.4](requirements.md#1.4), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [2.4](requirements.md#2.4), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [3.4](requirements.md#3.4), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [4.5](requirements.md#4.5), [5.1](requirements.md#5.1), [5.2](requirements.md#5.2), [6.1](requirements.md#6.1), [6.2](requirements.md#6.2), [6.3](requirements.md#6.3), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2), [8.1](requirements.md#8.1), [8.2](requirements.md#8.2), [9.1](requirements.md#9.1)+  - References: Transit/TransitTests/UpdateTaskIntentTests.swift, specs/update-task-all-fields/design.md++- [x] 10. Rewrite UpdateTaskIntent.execute, delete buildResponse and applyMilestoneChange, extend Input parameter description <!-- id:2r6h8qt -->+  - Rewrite execute() to mirror the MCP handler: parse JSON, resolve task, validate, no-op echo, apply with explicit rollback on throw, save (TaskService.save wraps rollback), encode response via IntentHelpers.taskUpdateResponseDict+  - Map TaskUpdateValidationError to its intentError projection then to .json+  - Map apply/save failures to IntentError.internalError(hint:), not raw error interpolation+  - DELETE: private static func applyMilestoneChange (line 77) and private static func buildResponse (line 103) — they are subsumed by the validator/applier and taskUpdateResponseDict+  - Update the Input @Parameter description to list every supported field including name, description (with empty-string-clears note), type, metadata (with {} clears note)+  - Make all tests from task 9 pass+  - Blocked-by: 2r6h8qp (Add rollback() method to TaskService), 2r6h8qm (Implement TaskUpdateValidator.validate, apply, and strictStringMetadata), 2r6h8qo (Implement IntentHelpers.taskUpdateResponseDict), 2r6h8qs (Extend UpdateTaskIntentTests.swift with parallel field-update tests)+  - Requirements: [1.1](requirements.md#1.1), [1.2](requirements.md#1.2), [1.3](requirements.md#1.3), [1.4](requirements.md#1.4), [2.1](requirements.md#2.1), [2.2](requirements.md#2.2), [2.3](requirements.md#2.3), [2.4](requirements.md#2.4), [3.1](requirements.md#3.1), [3.2](requirements.md#3.2), [3.3](requirements.md#3.3), [3.4](requirements.md#3.4), [4.1](requirements.md#4.1), [4.2](requirements.md#4.2), [4.3](requirements.md#4.3), [4.4](requirements.md#4.4), [4.5](requirements.md#4.5), [5.1](requirements.md#5.1), [5.2](requirements.md#5.2), [6.1](requirements.md#6.1), [6.2](requirements.md#6.2), [6.3](requirements.md#6.3), [7.1](requirements.md#7.1), [7.2](requirements.md#7.2), [8.1](requirements.md#8.1), [8.2](requirements.md#8.2), [9.1](requirements.md#9.1)+  - References: Transit/Transit/Intents/UpdateTaskIntent.swift++## Cross-surface parity++- [x] 11. Write UpdateTaskAllFieldsParityTests.swift <!-- id:2r6h8qu -->+  - New file: Transit/TransitTests/UpdateTaskAllFieldsParityTests.swift+  - For a fixed set of valid update args (covering each new field individually and a combined multi-field case), call BOTH the MCP handler and UpdateTaskIntent.execute, parse the JSON responses, assert dict equality on the full key set+  - Success cases only — error message divergence is allowed per AC 5.2+  - Verifies AC 8.1 contract holds in practice; if it fails, fix whichever surface diverges+  - Blocked-by: 2r6h8qr (Rewrite MCPToolHandler.handleUpdateTask and extend MCPToolDefinitions.updateTask schema), 2r6h8qt (Rewrite UpdateTaskIntent.execute, delete buildResponse and applyMilestoneChange, extend Input parameter description)+  - Requirements: [8.1](requirements.md#8.1)+  - References: Transit/TransitTests/MCPUpdateTaskTests.swift, Transit/TransitTests/UpdateTaskIntentTests.swift++## Documentation++- [x] 12. Update docs/agent-notes/mcp-server.md for extended update_task <!-- id:2r6h8qv -->+  - Update the Tools Exposed table entry for update_task to reflect that it now supports name/description/type/metadata in addition to milestone fields+  - Optionally add a one-line gotcha about the empty-string-clears semantic+  - Blocked-by: 2r6h8qr (Rewrite MCPToolHandler.handleUpdateTask and extend MCPToolDefinitions.updateTask schema), 2r6h8qt (Rewrite UpdateTaskIntent.execute, delete buildResponse and applyMilestoneChange, extend Input parameter description)+  - Requirements: [8.1](requirements.md#8.1), [8.2](requirements.md#8.2)+  - References: docs/agent-notes/mcp-server.md