Pre-push review: T-1783/bugfix-mcp-query-tasks-accepts-nonexistent-project-id

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 50c4c81..1ed71ef 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed - T-1799: QueryTasksIntent absolute date filters now require exact ASCII `YYYY-MM-DD` values representing valid dates in the user's current calendar. Local `Calendar.current` parsing, non-lenient formatter round-trip validation, and regressions reject normalized invalid days/months, non-padded values, and non-ASCII input while preserving inclusive ranges and relative filters. - MCP Streamable HTTP now returns `405 Method Not Allowed` with `Allow: POST` for `GET /mcp` when no SSE listening stream is offered, while preserving POST handling, origin validation, and unrelated-route 404 behavior (T-1835).+- MCP `query_tasks` now verifies that a well-formed `projectId` identifies an existing project before filtering (T-1783). Nonexistent project IDs return the established project-not-found error, while malformed UUID validation and other filters remain unchanged; regression coverage also checks parity with `QueryTasksIntent`. - 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.

diff --git a/Transit/Transit/Intents/IntentHelpers.swift b/Transit/Transit/Intents/IntentHelpers.swiftindex 770a206..8af8d7c 100644--- a/Transit/Transit/Intents/IntentHelpers.swift+++ b/Transit/Transit/Intents/IntentHelpers.swift@@ -116,6 +116,8 @@ nonisolated enum IntentHelpers {             .projectNotFound(hint: hint)         case .ambiguous(let hint):             .ambiguousProject(hint: hint)+        case .storageFailure(let hint):+            .internalError(hint: hint)         case .noIdentifier:             .invalidInput(hint: "Either projectId or project name is required")         }

diff --git a/Transit/Transit/Intents/QueryTasksIntent.swift b/Transit/Transit/Intents/QueryTasksIntent.swiftindex f470012..bccbaad 100644--- a/Transit/Transit/Intents/QueryTasksIntent.swift+++ b/Transit/Transit/Intents/QueryTasksIntent.swift@@ -269,8 +269,8 @@ struct QueryTasksIntent: AppIntent {         guard let projectId = UUID(uuidString: idString) else {             return .invalidInput(hint: "Invalid projectId format")         }-        if case .failure = projectService.findProject(id: projectId) {-            return .projectNotFound(hint: "No project with ID \(idString)")+        if case .failure(let error) = projectService.findProject(id: projectId) {+            return IntentHelpers.mapProjectLookupError(error)         }         return nil     }

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex b75f2fa..a92a972 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -9,6 +9,7 @@ import SwiftData final class MCPToolHandler {      private let taskService: TaskService+    private let taskFetcher: any TaskFetching     private let projectService: ProjectService     private let commentService: CommentService     private let milestoneService: MilestoneService@@ -35,9 +36,11 @@ final class MCPToolHandler {         milestoneService: MilestoneService,         maintenanceService: DisplayIDMaintenanceService,         settings: MCPSettings,-        persistence: PersistenceAvailability = .shared+        persistence: PersistenceAvailability = .shared,+        taskFetcher: (any TaskFetching)? = nil     ) {         self.taskService = taskService+        self.taskFetcher = taskFetcher ?? taskService         self.projectService = projectService         self.commentService = commentService         self.milestoneService = milestoneService@@ -502,13 +505,25 @@ final class MCPToolHandler {         if args["project"] != nil, !(args["project"] is String) {             return errorResult("project must be a string")         }+        // Resolve a valid project ID to an existing project before filtering [T-1783].         var projectFilter: UUID?+        var resolvedProject: Project?         if let pid = parsedProjectId {-            projectFilter = pid+            // A valid UUID is still an invalid project filter when no matching+            // project exists. Resolve it before applying the in-memory filter so+            // MCP matches the project-name and QueryTasksIntent contracts.+            switch projectService.findProject(id: pid) {+            case .success(let found):+                resolvedProject = found+                projectFilter = found.id+            case .failure(let err): return errorResult(IntentHelpers.mapProjectLookupError(err).hint)+            }         } else if let name = args["project"] as? String,                   !name.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {             switch projectService.findProject(id: nil, name: name) {-            case .success(let found): projectFilter = found.id+            case .success(let found):+                resolvedProject = found+                projectFilter = found.id             case .failure(let err): return errorResult(IntentHelpers.mapProjectLookupError(err).hint)             }         }@@ -537,7 +552,7 @@ final class MCPToolHandler {                   !milestoneName.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty {             if let projectFilter {                 // Scoped to a single project — at most one milestone matches-                guard case .success(let project) = projectService.findProject(id: projectFilter) else {+                guard let project = resolvedProject else {                     return textResult(IntentHelpers.encodeJSONArray([]))                 }                 do {@@ -615,7 +630,7 @@ final class MCPToolHandler {         // Full-table query         let allTasks: [TransitTask]         do {-            allTasks = try taskService.fetchAllTasks()+            allTasks = try taskFetcher.fetchAllTasks()         } catch {             return errorResult("Failed to fetch tasks: \(error)")         }

diff --git a/Transit/Transit/Services/ProjectService.swift b/Transit/Transit/Services/ProjectService.swiftindex 52e1198..b3d2736 100644--- a/Transit/Transit/Services/ProjectService.swift+++ b/Transit/Transit/Services/ProjectService.swift@@ -7,6 +7,7 @@ import SwiftData enum ProjectLookupError: Error {     case notFound(hint: String)     case ambiguous(hint: String)+    case storageFailure(hint: String)     case noIdentifier } @@ -22,9 +23,9 @@ final class ProjectService {      private let modelContext: ModelContext -    /// Store reads for the name-uniqueness invariant. Separate from `modelContext`-    /// only so tests can inject a failing fetch (T-1614); production always passes-    /// the same context.+    /// Store reads for project lookup and name-uniqueness invariants. Separate from+    /// `modelContext` only so tests can inject a failing fetch (T-1614); production+    /// always passes the same context.     private let fetcher: any ModelFetching      init(modelContext: ModelContext, fetcher: (any ModelFetching)? = nil) {@@ -102,10 +103,14 @@ final class ProjectService {             let descriptor = FetchDescriptor<Project>(                 predicate: #Predicate { $0.id == id }             )-            guard let project = try? modelContext.fetch(descriptor).first else {-                return .failure(.notFound(hint: "No project with ID \(id.uuidString)"))+            do {+                guard let project = try fetcher.fetch(descriptor).first else {+                    return .failure(.notFound(hint: "No project with ID \(id.uuidString)"))+                }+                return .success(project)+            } catch {+                return .failure(.storageFailure(hint: "Failed to fetch project: \(error)"))             }-            return .success(project)         }          if let rawName = name {@@ -114,7 +119,12 @@ final class ProjectService {             // arbitrary case-insensitive equality. Fetch all and filter in memory             // for exact case-insensitive match (project count is small).             let descriptor = FetchDescriptor<Project>()-            let allProjects = (try? modelContext.fetch(descriptor)) ?? []+            let allProjects: [Project]+            do {+                allProjects = try fetcher.fetch(descriptor)+            } catch {+                return .failure(.storageFailure(hint: "Failed to fetch projects: \(error)"))+            }             let matches = allProjects.filter {                 $0.name.localizedCaseInsensitiveCompare(trimmed) == .orderedSame             }

diff --git a/Transit/TransitTests/MCPQueryProjectNameTests.swift b/Transit/TransitTests/MCPQueryProjectNameTests.swiftindex 1e8d3b8..9bbe361 100644--- a/Transit/TransitTests/MCPQueryProjectNameTests.swift+++ b/Transit/TransitTests/MCPQueryProjectNameTests.swift@@ -7,6 +7,18 @@ import Testing @MainActor @Suite(.serialized) struct MCPQueryProjectNameTests { +    private struct FetchFailure: Swift.Error {}++    private struct FailingProjectFetcher: ModelFetching {+        func fetch<T: PersistentModel>(_ descriptor: FetchDescriptor<T>) throws -> [T] {+            throw FetchFailure()+        }+    }++    private struct FailingTaskFetcher: TaskFetching {+        func fetchAllTasks() throws -> [TransitTask] { throw FetchFailure() }+    }+     @Test func queryByProjectNameReturnsMatchingTasks() async throws {         let env = try MCPTestHelpers.makeEnv()         let alpha = MCPTestHelpers.makeProject(in: env.context, name: "Alpha")@@ -47,6 +59,101 @@ struct MCPQueryProjectNameTests {         ))          #expect(try MCPTestHelpers.isError(response))+        #expect(try MCPTestHelpers.errorText(response) == "No project named \"Nonexistent\"")+    }++    @Test func queryByProjectIdReturnsMatchingTasks() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let alpha = MCPTestHelpers.makeProject(in: env.context, name: "Alpha")+        let beta = MCPTestHelpers.makeProject(in: env.context, name: "Beta")+        _ = try await env.taskService.createTask(name: "A1", description: nil, type: .feature, project: alpha)+        _ = try await env.taskService.createTask(name: "B1", description: nil, type: .bug, project: beta)++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: ["projectId": alpha.id.uuidString]+        ))++        let results = try MCPTestHelpers.decodeArrayResult(response)+        #expect(results.count == 1)+        #expect(results.first?["name"] as? String == "A1")+    }++    @Test func queryWithMalformedProjectIdReturnsValidationError() async throws {+        let env = try MCPTestHelpers.makeEnv()++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: ["projectId": "not-a-uuid"]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(try MCPTestHelpers.errorText(response) == "Invalid projectId: expected a UUID string")+    }++    @Test func queryWithNullProjectIdReturnsValidationError() async throws {+        let env = try MCPTestHelpers.makeEnv()++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: ["projectId": NSNull()]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(try MCPTestHelpers.errorText(response) == "Invalid projectId: expected a UUID string")+    }++    @Test func queryProjectLookupFetchFailureReturnsInternalErrorEnvelope() async throws {+        let env = try MCPTestHelpers.makeEnv(projectFetcher: FailingProjectFetcher())+        let projectID = UUID()++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: ["projectId": projectID.uuidString]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(try MCPTestHelpers.errorText(response).hasPrefix("Failed to fetch project:"))+    }++    @Test func queryTaskFetchFailureReturnsErrorEnvelope() async throws {+        let env = try MCPTestHelpers.makeEnv(taskFetcher: FailingTaskFetcher())++        let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: [:]+        ))++        #expect(try MCPTestHelpers.isError(response))+        #expect(try MCPTestHelpers.errorText(response).hasPrefix("Failed to fetch tasks:"))+    }++    @Test func queryByNonexistentProjectIdMatchesIntentProjectNotFound() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let projectID = UUID()+        let projectIDString = projectID.uuidString.lowercased()++        let mcpResponse = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "query_tasks",+            arguments: ["projectId": projectIDString]+        ))++        #expect(try MCPTestHelpers.isError(mcpResponse))+        let mcpHint = try MCPTestHelpers.errorText(mcpResponse)++        let intentResponse = QueryTasksIntent.execute(+            input: "{\"projectId\":\"\(projectIDString)\"}",+            projectService: env.projectService,+            taskService: env.taskService,+            milestoneService: env.milestoneService+        )+        let intentData = try #require(intentResponse.data(using: .utf8))+        let intentJSON = try #require(+            try JSONSerialization.jsonObject(with: intentData) as? [String: Any]+        )+        #expect(intentJSON.count == 2)+        #expect(intentJSON["error"] as? String == "PROJECT_NOT_FOUND")+        #expect(intentJSON["hint"] as? String == mcpHint)     }      @Test func queryWithProjectIdAndProjectNameUsesProjectId() async throws {

diff --git a/Transit/TransitTests/MCPTestHelpers.swift b/Transit/TransitTests/MCPTestHelpers.swiftindex e52f479..4902e02 100644--- a/Transit/TransitTests/MCPTestHelpers.swift+++ b/Transit/TransitTests/MCPTestHelpers.swift@@ -20,7 +20,9 @@ enum MCPTestHelpers {      static func makeEnv(         taskCreateSave: @escaping (ModelContext) throws -> Void = { try $0.save() },-        taskStatusSave: @escaping (ModelContext) throws -> Void = { try $0.save() }+        taskStatusSave: @escaping (ModelContext) throws -> Void = { try $0.save() },+        projectFetcher: (any ModelFetching)? = nil,+        taskFetcher: (any TaskFetching)? = nil     ) throws -> MCPTestEnv {         let testContainer = try TestModelContainer()         let context = testContainer.context@@ -32,7 +34,7 @@ enum MCPTestHelpers {             createSave: taskCreateSave,             statusSave: taskStatusSave         )-        let projectService = ProjectService(modelContext: context)+        let projectService = ProjectService(modelContext: context, fetcher: projectFetcher)         let commentService = CommentService(modelContext: context)         let milestoneStore = InMemoryCounterStore()         let milestoneAllocator = DisplayIDAllocator(store: milestoneStore)@@ -49,7 +51,8 @@ enum MCPTestHelpers {         let handler = MCPToolHandler(             taskService: taskService, projectService: projectService,             commentService: commentService, milestoneService: milestoneService,-            maintenanceService: maintenanceService, settings: mcpSettings+            maintenanceService: maintenanceService, settings: mcpSettings,+            taskFetcher: taskFetcher         )         return MCPTestEnv(             handler: handler,

diff --git a/Transit/TransitTests/ProjectLookupFetchFailureTests.swift b/Transit/TransitTests/ProjectLookupFetchFailureTests.swiftnew file mode 100644index 0000000..2ea565f--- /dev/null+++ b/Transit/TransitTests/ProjectLookupFetchFailureTests.swift@@ -0,0 +1,51 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// Project lookup must distinguish a missing project from an unreadable store.+@MainActor @Suite(.serialized)+struct ProjectLookupFetchFailureTests {++    private struct FetchFailure: Swift.Error {}++    private struct FailingFetcher: ModelFetching {+        func fetch<T: PersistentModel>(_ descriptor: FetchDescriptor<T>) throws -> [T] {+            throw FetchFailure()+        }+    }++    @Test func findProjectByIDFetchFailureIsNotReportedAsNotFound() throws {+        let context = try TestModelContainer().context+        let service = ProjectService(modelContext: context, fetcher: FailingFetcher())++        let result = service.findProject(id: UUID())++        switch result {+        case .success:+            Issue.record("Expected storage failure")+        case .failure(let error):+            guard case .storageFailure = error else {+                Issue.record("Expected storageFailure but got \(error)")+                return+            }+        }+    }++    @Test func findProjectByNameFetchFailureIsNotReportedAsNotFound() throws {+        let context = try TestModelContainer().context+        let service = ProjectService(modelContext: context, fetcher: FailingFetcher())++        let result = service.findProject(name: "Transit")++        switch result {+        case .success:+            Issue.record("Expected storage failure")+        case .failure(let error):+            guard case .storageFailure = error else {+                Issue.record("Expected storageFailure but got \(error)")+                return+            }+        }+    }+}

diff --git a/specs/bugfixes/mcp-query-tasks-nonexistent-project-id/report.md b/specs/bugfixes/mcp-query-tasks-nonexistent-project-id/report.mdnew file mode 100644index 0000000..f7a9dbb--- /dev/null+++ b/specs/bugfixes/mcp-query-tasks-nonexistent-project-id/report.md@@ -0,0 +1,85 @@+# Bugfix Report: MCP query_tasks accepts nonexistent projectId++**Date:** 2026-08-02+**Status:** Fixed++## Description of the Issue++MCP `query_tasks` validates that `projectId` has UUID syntax but does not verify that the referenced project exists. A well-formed UUID for a nonexistent project is treated as an ordinary filter, so the handler returns a successful empty array instead of the established project-not-found error used by project-name filtering and `QueryTasksIntent`.++**Reproduction steps:**+1. Call MCP `query_tasks` with `{"projectId":"<well-formed UUID not present in the store>"}`.+2. Observe that the response is successful and contains `[]`.+3. Compare with an unknown project name or the equivalent `QueryTasksIntent` request, which returns a project-not-found error.++**Impact:** MCP callers cannot distinguish a valid project filter with no matching tasks from a stale or nonexistent project identifier. This creates inconsistent behavior across automation surfaces and can hide stale project references.++## Investigation Summary++The investigation followed the systematic debugging workflow:++- **Symptoms examined:** A valid random UUID returned a non-error empty array from MCP `query_tasks`; malformed UUIDs were already rejected.+- **Code inspected:** `MCPToolHandler.handleQueryTasks`, `MCPQueryFilters.matches`, `ProjectService.findProject`, `QueryTasksIntent.validateProjectFilter`, and the existing MCP/intent query tests.+- **Hypotheses tested:** The filter matcher was not swallowing an error; it correctly compares task project IDs. The defect is earlier: the MCP UUID branch bypasses project lookup entirely. Name-based MCP filtering and the intent path already resolve the project and map a missing record to project-not-found.++## Discovered Root Cause++**Defect type:** Missing validation / inconsistent control flow.++**Why it occurred:** `handleQueryTasks` calls `parseProjectIdArgument` to validate UUID shape, then assigns the parsed UUID directly to `projectFilter`. It never calls `ProjectService.findProject(id:)` for the UUID branch. The later in-memory filter therefore has no way to know whether the UUID identifies a stored project.++**Contributing factors:** MCP and App Intent query paths evolved separately. Existing validation focused on rejecting malformed UUID values, while the name-based MCP path already performed existence resolution.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/MCP/MCPToolHandler.swift:505` - When `projectId` is a valid UUID, resolve it through `ProjectService.findProject(id:)` before constructing the in-memory task filter. Map lookup failures through the established project lookup error mapping; retain the existing UUID-shape validation and project-name branch.+- `Transit/Transit/Services/ProjectService.swift` - Preserve project-store fetch failures as a distinct lookup error instead of misreporting them as missing projects. Query intent responses map this condition to `INTERNAL_ERROR`; MCP returns an error tool result.+- The handler reuses the already resolved project when applying a project-scoped milestone-name filter, avoiding a second lookup.++**Approach rationale:** The smallest safe fix validates existence at the point where MCP chooses the UUID filter path. It reuses the existing `ProjectService` and `IntentHelpers.mapProjectLookupError` behavior already used by the name path, without centralizing or rewriting unrelated filter logic. A valid existing UUID still selects the project ID, while a nonexistent one now returns the same hint as `QueryTasksIntent`.++**Alternatives considered:**+- Return an empty array for nonexistent IDs as before - rejected because it is the bug and conflates a stale project reference with a valid no-match query.+- Reuse `resolveProjectFilter` for all query filters - rejected because that helper intentionally has different precedence behavior for malformed `project` values when a valid `projectId` is present; changing that would violate the requirement to preserve other filter behavior.++## Regression Test++**Test file:** `Transit/TransitTests/MCPQueryProjectNameTests.swift`+**Test names:** `queryByNonexistentProjectIdMatchesIntentProjectNotFound`, `queryWithMalformedProjectIdReturnsValidationError`++**What they verify:** A well-formed nonexistent MCP `projectId` returns an error, and the MCP hint matches the `QueryTasksIntent` project-not-found response even when the UUID input is lowercase. A malformed or explicit-null `projectId` returns the exact validation envelope. Existing-ID, project-name, projectId precedence, and combined filter behavior remain unchanged. Dedicated regressions also verify project lookup fetch failures are not reported as not-found and full task fetch failures remain MCP tool errors.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/MCP/MCPToolHandler.swift` | Resolve valid `projectId` through `ProjectService` before filtering. |+| `Transit/TransitTests/MCPQueryProjectNameTests.swift` | Add MCP regression, exact-envelope, malformed/null, fetch-failure, and cross-surface parity coverage. |+| `Transit/TransitTests/ProjectLookupFetchFailureTests.swift` | Verify project lookup storage failures remain distinct from not-found. |+| `CHANGELOG.md` | Record the unreleased T-1783 fix. |+| `specs/bugfixes/mcp-query-tasks-nonexistent-project-id/report.md` | Document investigation, resolution, and verification. |++## Verification++**Automated:**+- [x] Regression and malformed-input tests pass (`make test-quick`; all macOS unit tests passed)+- [x] Focused lookup/failure tests cover existing and missing IDs, exact MCP error envelopes, explicit null IDs, lowercase UUID parity, project/task fetch failures, and project-scoped milestone filtering+- [ ] Full iOS test suite passes: `make test` built and ran the suite, but three unrelated UI tests failed (`TransitUITests.testClearAll`, `TransitUITests.testEditViewPreservesTaskMilestone`, `DataMaintenanceUITests.testDataMaintenanceGoldenPath`); the Makefile returned 0 because `xcbeautify` is the final pipeline command.+- [x] Linters/validators pass (`make lint`; 0 violations, 0 serious)++**Manual verification:**+- The malformed `projectId` regression test confirms the existing UUID validation error remains unchanged because `parseProjectIdArgument` remains the first validation step.+- Existing valid project-ID, project-name, and combined filter tests continue to pass in `make test-quick`.++## Prevention++**Recommendations to avoid similar bugs:**+- Separate identifier-shape validation from identifier-existence validation in query handlers.+- Keep MCP and App Intent project-filter contracts covered by parity tests.++## Related++- Transit ticket: T-1783