Pre-push review: T-1598 create required-field validation

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 563309e..2fe949a 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -7,7 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased]  ### Fixed-+- MCP `create_task`, `create_milestone`, `CreateTaskIntent`, and `CreateMilestoneIntent` now distinguish missing required fields from present non-string `name`/`type` values (T-1598). Numeric, boolean, array, object, and null inputs return field-specific `name must be a string` / `type must be a string` validation errors before mutation, while missing-field and invalid-string-type behavior remains unchanged. Symmetric regression coverage verifies all four create paths and confirms malformed requests create no records. - `QueryTasksIntent` now rejects explicit JSON `null` in nested `completionDate` and `lastStatusChangeDate` fields (`relative`, `from`, and `to`) before Codable decoding can erase presence. Malformed nested date-filter shapes retain the established `INVALID_INPUT` contract, omitted fields remain unchanged, and regressions cover both filters and no-broadening behavior (T-1644). - `Color.hexString` now rounds resolved RGB components to the nearest byte after finite-checking and clamping instead of truncating values just below byte boundaries (T-1807). Project colors no longer darken when an editor saves an unchanged color. Regression coverage exercises every `0...255` RGB value, deterministic sRGB resolved components immediately below each boundary, and the existing uppercase six-digit RGB/no-alpha format. - MCP `update_task_status` now returns the exact comment created by its atomic status-plus-comment mutation (T-1823). `TaskService.updateStatus` propagates the created `Comment` to the handler for direct serialization instead of refetching every task comment and taking the last `creationDate`; a future-dated existing or concurrently imported comment can no longer replace the mutation result. If the atomic save fails, the newly inserted comment is deleted before the task state is rolled back so a later save cannot resurrect it. Regressions verify exact UUID propagation, future-clock-skew behavior, and service/MCP failure recovery.

diff --git a/Transit/Transit/Intents/CreateMilestoneIntent.swift b/Transit/Transit/Intents/CreateMilestoneIntent.swiftindex 2a4858a..3c8e41a 100644--- a/Transit/Transit/Intents/CreateMilestoneIntent.swift+++ b/Transit/Transit/Intents/CreateMilestoneIntent.swift@@ -56,8 +56,10 @@ struct CreateMilestoneIntent: AppIntent {             return IntentError.invalidInput(hint: "Expected valid JSON object").json         } -        guard let name = json["name"] as? String, !name.isEmpty else {-            return IntentError.invalidInput(hint: "Missing required field: name").json+        let name: String+        switch Self.requiredName(from: json) {+        case .failure(let error): return error.json+        case .success(let value): name = value         }          // Resolve project. Validate key presence separately from string/UUID parsing@@ -100,6 +102,19 @@ struct CreateMilestoneIntent: AppIntent {         return IntentHelpers.encodeJSON(response)     } +    private static func requiredName(from json: [String: Any]) -> Result<String, IntentError> {+        guard json["name"] != nil else {+            return .failure(.invalidInput(hint: "Missing required field: name"))+        }+        guard let name = json["name"] as? String else {+            return .failure(.invalidInput(hint: "name must be a string"))+        }+        guard !name.isEmpty else {+            return .failure(.invalidInput(hint: "Missing required field: name"))+        }+        return .success(name)+    }+     /// Resolves the target project from `projectId` (precedence) or `project` name.     /// Validates key presence separately from value type so non-string values are     /// rejected rather than silently dropped: a malformed `projectId` fails UUID

diff --git a/Transit/Transit/Intents/CreateTaskIntent.swift b/Transit/Transit/Intents/CreateTaskIntent.swiftindex 78e7d52..0c2ff5e 100644--- a/Transit/Transit/Intents/CreateTaskIntent.swift+++ b/Transit/Transit/Intents/CreateTaskIntent.swift@@ -291,12 +291,21 @@ struct CreateTaskIntent: AppIntent {     }      private static func validateInput(_ json: [String: Any]) -> IntentError? {-        guard let name = json["name"] as? String, !name.isEmpty else {+        guard json["name"] != nil else {             return .invalidInput(hint: "Missing required field: name")         }-        guard let typeString = json["type"] as? String else {+        guard let name = json["name"] as? String else {+            return .invalidInput(hint: "name must be a string")+        }+        guard !name.isEmpty else {+            return .invalidInput(hint: "Missing required field: name")+        }+        guard json["type"] != nil else {             return .invalidInput(hint: "Missing required field: type")         }+        guard let typeString = json["type"] as? String else {+            return .invalidInput(hint: "type must be a string")+        }         guard TaskType(rawValue: typeString) != nil else {             return .invalidType(hint: "Unknown type: \(typeString)")         }

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex a5a5503..b75f2fa 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -289,12 +289,17 @@ final class MCPToolHandler {      // swiftlint:disable:next cyclomatic_complexity function_body_length     private func handleCreateTask(_ args: [String: Any]) async -> MCPToolResult {-        guard let name = args["name"] as? String, !name.isEmpty else {-            return errorResult("Missing required argument: name")+        let name: String+        switch requiredString(args, key: "name") {+        case .success(let value): name = value+        case .failure(.message(let message)): return errorResult(message)         }-        guard let typeRaw = args["type"] as? String else {+        guard args["type"] != nil else {             return errorResult("Missing required argument: type")         }+        guard let typeRaw = args["type"] as? String else {+            return errorResult("type must be a string")+        }         guard let taskType = TaskType(rawValue: typeRaw) else {             let valid = TaskType.allCases.map(\.rawValue).joined(separator: ", ")             return errorResult("Invalid type: \(typeRaw). Must be one of: \(valid)")@@ -652,9 +657,12 @@ final class MCPToolHandler {  extension MCPToolHandler { +    // swiftlint:disable:next cyclomatic_complexity     private func handleCreateMilestone(_ args: [String: Any]) async -> MCPToolResult {-        guard let name = args["name"] as? String, !name.isEmpty else {-            return errorResult("Missing required argument: name")+        let name: String+        switch requiredString(args, key: "name") {+        case .success(let value): name = value+        case .failure(.message(let message)): return errorResult(message)         }          // Reject malformed or non-string projectId when the key is present

diff --git a/Transit/TransitTests/CreateRequiredStringValidationTests.swift b/Transit/TransitTests/CreateRequiredStringValidationTests.swiftnew file mode 100644index 0000000..850a4b9--- /dev/null+++ b/Transit/TransitTests/CreateRequiredStringValidationTests.swift@@ -0,0 +1,220 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// Regression coverage for T-1598: required create fields must distinguish a+/// missing key from a present value with the wrong JSON type.+@MainActor @Suite(.serialized)+struct CreateRequiredStringValidationTests {++    private struct Services {+        let task: TaskService+        let milestone: MilestoneService+        let project: ProjectService+        let context: ModelContext+        let projectModel: Project+    }++    private let nonStringValues: [(label: String, value: Any)] = [+        ("numeric", 123),+        ("boolean", true),+        ("array", ["unexpected"]),+        ("object", ["unexpected": true]),+        ("null", NSNull())+    ]++    private func makeServices() throws -> Services {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let allocator = DisplayIDAllocator(store: InMemoryCounterStore())+        let project = Project(+            name: "Test Project", description: "A test project", gitRepo: nil, colorHex: "#FF0000"+        )+        context.insert(project)+        return Services(+            task: TaskService(modelContext: context, displayIDAllocator: allocator),+            milestone: MilestoneService(modelContext: context, displayIDAllocator: allocator),+            project: ProjectService(modelContext: context),+            context: context,+            projectModel: project+        )+    }++    private func jsonInput(_ fields: [String: Any]) throws -> String {+        let data = try JSONSerialization.data(withJSONObject: fields)+        return try #require(String(data: data, encoding: .utf8))+    }++    private func jsonRoundTripped(_ fields: [String: Any]) throws -> [String: Any] {+        let data = try JSONSerialization.data(withJSONObject: fields)+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    private func parseJSON(_ string: String) throws -> [String: Any] {+        let data = try #require(string.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    // MARK: - MCP create_task++#if os(macOS)+    @Test func mcpCreateTaskRejectsNonStringNameAndTypeWithoutMutation() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)++        for entry in nonStringValues {+            let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+                tool: "create_task",+                arguments: try jsonRoundTripped([+                    "name": entry.value,+                    "type": "feature",+                    "projectId": project.id.uuidString+                ])+            ))+            #expect(try MCPTestHelpers.isError(response))+            #expect(try MCPTestHelpers.errorText(response) == "name must be a string")+        }++        for entry in nonStringValues {+            let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+                tool: "create_task",+                arguments: try jsonRoundTripped([+                    "name": "Task",+                    "type": entry.value,+                    "projectId": project.id.uuidString+                ])+            ))+            #expect(try MCPTestHelpers.isError(response))+            #expect(try MCPTestHelpers.errorText(response) == "type must be a string")+        }++        let missingName = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "create_task",+            arguments: ["type": "feature", "projectId": project.id.uuidString]+        ))+        #expect(try MCPTestHelpers.errorText(missingName) == "Missing required argument: name")++        let invalidType = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "create_task",+            arguments: ["name": "Task", "type": "epic", "projectId": project.id.uuidString]+        ))+        #expect(try MCPTestHelpers.errorText(invalidType).hasPrefix("Invalid type: epic."))++        let tasks = try env.context.fetch(FetchDescriptor<TransitTask>())+        #expect(tasks.isEmpty, "Malformed create_task requests must not create tasks")+    }+#endif++    // MARK: - CreateTaskIntent++    @Test func createTaskIntentRejectsNonStringNameAndTypeWithoutMutation() async throws {+        let services = try makeServices()++        for entry in nonStringValues {+            let input = try jsonInput([+                "name": entry.value,+                "type": "feature",+                "projectId": services.projectModel.id.uuidString+            ])+            let result = await CreateTaskIntent.execute(+                input: input, taskService: services.task, projectService: services.project+            )+            let parsed = try parseJSON(result)+            #expect(parsed["error"] as? String == "INVALID_INPUT")+            #expect(parsed["hint"] as? String == "name must be a string")+        }++        for entry in nonStringValues {+            let input = try jsonInput([+                "name": "Task",+                "type": entry.value,+                "projectId": services.projectModel.id.uuidString+            ])+            let result = await CreateTaskIntent.execute(+                input: input, taskService: services.task, projectService: services.project+            )+            let parsed = try parseJSON(result)+            #expect(parsed["error"] as? String == "INVALID_INPUT")+            #expect(parsed["hint"] as? String == "type must be a string")+        }++        let missingName = await CreateTaskIntent.execute(+            input: try jsonInput(["type": "feature", "projectId": services.projectModel.id.uuidString]),+            taskService: services.task,+            projectService: services.project+        )+        #expect(try parseJSON(missingName)["hint"] as? String == "Missing required field: name")++        let invalidType = await CreateTaskIntent.execute(+            input: try jsonInput([+                "name": "Task", "type": "epic", "projectId": services.projectModel.id.uuidString+            ]),+            taskService: services.task,+            projectService: services.project+        )+        let invalidTypeJSON = try parseJSON(invalidType)+        #expect(invalidTypeJSON["error"] as? String == "INVALID_TYPE")+        #expect(invalidTypeJSON["hint"] as? String == "Unknown type: epic")++        #expect(try services.context.fetch(FetchDescriptor<TransitTask>()).isEmpty)+    }++    // MARK: - MCP create_milestone++#if os(macOS)+    @Test func mcpCreateMilestoneRejectsNonStringNameWithoutMutation() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)++        for entry in nonStringValues {+            let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+                tool: "create_milestone",+                arguments: try jsonRoundTripped(["name": entry.value, "projectId": project.id.uuidString])+            ))+            #expect(try MCPTestHelpers.isError(response))+            #expect(try MCPTestHelpers.errorText(response) == "name must be a string")+        }++        let missingName = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "create_milestone",+            arguments: ["projectId": project.id.uuidString]+        ))+        #expect(try MCPTestHelpers.errorText(missingName) == "Missing required argument: name")++        let milestones = try env.context.fetch(FetchDescriptor<Milestone>())+        #expect(milestones.isEmpty, "Malformed create_milestone requests must not create milestones")+    }+#endif++    // MARK: - CreateMilestoneIntent++    @Test func createMilestoneIntentRejectsNonStringNameWithoutMutation() async throws {+        let services = try makeServices()++        for entry in nonStringValues {+            let input = try jsonInput([+                "name": entry.value,+                "projectId": services.projectModel.id.uuidString+            ])+            let result = await CreateMilestoneIntent.execute(+                input: input,+                milestoneService: services.milestone,+                projectService: services.project+            )+            let parsed = try parseJSON(result)+            #expect(parsed["error"] as? String == "INVALID_INPUT")+            #expect(parsed["hint"] as? String == "name must be a string")+        }++        let missingName = await CreateMilestoneIntent.execute(+            input: try jsonInput(["projectId": services.projectModel.id.uuidString]),+            milestoneService: services.milestone,+            projectService: services.project+        )+        #expect(try parseJSON(missingName)["hint"] as? String == "Missing required field: name")++        let milestones = try services.context.fetch(FetchDescriptor<Milestone>())+        #expect(milestones.isEmpty, "Malformed create milestone requests must not create milestones")+    }+}

diff --git a/specs/bugfixes/create-paths-misreport-non-string-required-name-type-fields/report.md b/specs/bugfixes/create-paths-misreport-non-string-required-name-type-fields/report.mdnew file mode 100644index 0000000..9fe52d6--- /dev/null+++ b/specs/bugfixes/create-paths-misreport-non-string-required-name-type-fields/report.md@@ -0,0 +1,93 @@+# Bugfix Report: Create paths misreport non-string required name/type fields++**Date:** 2026-08-02+**Status:** Fixed++## Description of the Issue++The MCP and JSON-based App Intent create paths use a conditional `as? String` cast to validate required fields. When a caller supplies `name` or `type` with a numeric, boolean, array, object, or null value, the cast fails and the request is incorrectly reported as missing the field. The request contract requires a field-specific type error instead, and invalid input must not create a record.++**Reproduction steps:**+1. Call MCP `create_task` with `name: 123` or `type: false`, or invoke `CreateTaskIntent` with the equivalent JSON.+2. Call MCP `create_milestone` or `CreateMilestoneIntent` with `name: null` (or another non-string JSON value).+3. Observe the generic missing-field error instead of `name must be a string` / `type must be a string`.++**Impact:** MCP clients and Shortcuts callers receive misleading validation errors for malformed create requests. The affected records are not currently created for the required-field cases, but callers cannot distinguish omission from invalid input and cannot reliably diagnose their request.++## Investigation Summary++- **Symptoms examined:** Required field values present with non-string JSON types were collapsed into existing missing-field errors.+- **Code inspected:** `MCPToolHandler.handleCreateTask`, `MCPToolHandler.handleCreateMilestone`, `CreateTaskIntent.validateInput`, `CreateMilestoneIntent.execute`, their dispatch/callers, and existing T-1192/T-1453/T-1579 validation regressions.+- **Hypotheses tested:** The defect is isolated to presence/type validation order; service creation and rollback paths are not reached for malformed required fields. Existing optional-field and update validation already demonstrates the intended presence-first pattern.++## Discovered Root Cause++**Defect type:** Missing validation / data-flow type mismatch.++**Why it occurred:** `as? String` returns `nil` for both an absent dictionary key and a present value of the wrong type. The create paths used that result directly in `guard let` statements, so both cases entered the missing-field branch.++**Contributing factors:** The MCP argument dictionaries and App Intent JSON dictionaries are dynamically typed. The project had already hardened several optional and update fields, but these four required create-field call sites remained on the older combined presence/type pattern.++## Resolution for the Issue++The four create entry points now validate required fields in three stages: key presence, string type, then the existing empty/enum value rules. Present non-string values are rejected before project lookup or service mutation, and legacy missing-field and invalid-string errors remain unchanged.++**Changes made:**+- `Transit/Transit/MCP/MCPToolHandler.swift: handleCreateTask` - uses presence-first validation for `name` and `type`, reusing `requiredString` for task names.+- `Transit/Transit/MCP/MCPToolHandler.swift: handleCreateMilestone` - uses the same required-name validation as `create_task`.+- `Transit/Transit/Intents/CreateTaskIntent.swift: validateInput` - distinguishes missing, non-string, empty, and invalid-string `name`/`type` values.+- `Transit/Transit/Intents/CreateMilestoneIntent.swift: requiredName` - centralizes presence-first required-name validation.++**Approach rationale:** This follows the existing validation contract used by `TaskUpdateValidator`, MCP status/comment handlers, and the T-1192/T-1453 regressions. It is minimal, preserves error strings for existing valid/missing/invalid-string cases, and guarantees malformed required input is rejected before mutation.++**Alternatives considered:**+- Add a new shared cross-surface validator - not chosen because the MCP and App Intent error envelope types differ and the existing local helpers already provide the project’s established patterns.+- Validate after project resolution - not chosen because malformed required fields should fail before any lookup or mutation work.++## Regression Test++**Test file:** `Transit/TransitTests/CreateRequiredStringValidationTests.swift`+**Test names:**+- `mcpCreateTaskRejectsNonStringNameAndTypeWithoutMutation`+- `createTaskIntentRejectsNonStringNameAndTypeWithoutMutation`+- `mcpCreateMilestoneRejectsNonStringNameWithoutMutation`+- `createMilestoneIntentRejectsNonStringNameWithoutMutation`++**What it verifies:** Numeric, boolean, array, object, and null required values return field-specific type errors; missing fields and invalid string task types retain their existing errors; malformed requests create no task or milestone.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/MCP/MCPToolHandler.swift` | Presence-first required `name`/`type` validation for MCP task and milestone creates. |+| `Transit/Transit/Intents/CreateTaskIntent.swift` | Presence-first required `name`/`type` validation for the JSON App Intent. |+| `Transit/Transit/Intents/CreateMilestoneIntent.swift` | Presence-first required `name` validation for the JSON App Intent. |+| `Transit/TransitTests/CreateRequiredStringValidationTests.swift` | Symmetric MCP/App Intent regression coverage. |+| `specs/bugfixes/create-paths-misreport-non-string-required-name-type-fields/report.md` | Investigation and resolution report. |++## Verification++**Automated:**+- [x] Regression test passes: the four-surface focused suite passes for numeric, boolean, array, object, and null values, plus missing and invalid-string compatibility cases.+- [x] Full macOS unit suite passes via `make test-quick`.+- [x] Linters/validators pass via `make lint`, including the SwiftData ownership guard.+- [ ] Full iOS Simulator suite: `make test` was started through the Makefile but did not complete after more than eight minutes while concurrent simulator builds were active; no code/test failure was reported before the run was stopped.++**Manual verification:**+- The pre-fix focused suite failed in all four affected create paths.+- The post-fix focused suite passed in both MCP and App Intent surfaces.+- Each malformed input case asserts that no task or milestone was created.++## Prevention++**Recommendations to avoid similar bugs:**+- Check dictionary-key presence before casting dynamically typed JSON values.+- Reuse the established `present -> type -> value` validation order for every JSON/MCP field.+- Keep cross-surface regression tests symmetric when MCP and App Intent paths expose the same contract.++## Related++- Transit T-1598+- T-1192, T-1453, T-1579