Pre-push review: T-1820 notification dispatch

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 7525791..1a53f93 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ## [Unreleased] +- T-1820: MCP JSON-RPC notification regressions now prove that valid mutating `tools/call` notifications execute their normal side effects while suppressing only their response. Coverage spans direct handler dispatch, successful and failing single HTTP notifications that return 202/no body, ordered mixed batches where a following valid request observes the mutation while notification failures remain response-suppressed, and a batched `initialize` notification that cannot block the following valid request. The MCP route, batch, and notification suites now share one in-process HTTP transport harness. Explicit-null invalid requests, standalone lifecycle rules, all-notification HTTP semantics, and valid request responses remain covered.+ - T-1816: Regression coverage now directly proves that `query_tasks` validates malformed `status`, `not_status`, `type`, `priority`, `unfinished`, `search`, and `displayId` filters before either a missing milestone name or missing milestone display ID can return a successful no-match `[]`. This pins the validation order introduced by T-1608 / PR #217 without changing its production behavior. - T-1749: Visual Add Task no longer reports `NO_PROJECTS` when it cannot read the project table while resolving an unselected project. The existence check now propagates its SwiftData error through the existing injectable fetch seam, and the visual intent returns `INTERNAL_ERROR` with the storage-failure guidance. Genuine empty stores still return `NO_PROJECTS`; stale selected projects remain on the separate `PROJECT_NOT_FOUND` / storage-failure path. 

diff --git a/Transit/TransitTests/MCPHTTPTestHelpers.swift b/Transit/TransitTests/MCPHTTPTestHelpers.swiftnew file mode 100644index 0000000..320230a--- /dev/null+++ b/Transit/TransitTests/MCPHTTPTestHelpers.swift@@ -0,0 +1,87 @@+#if os(macOS)+import Foundation+import HTTPTypes+import Hummingbird+import Logging+import NIOConcurrencyHelpers+import NIOCore+import NIOEmbedded+import NIOFoundationCompat+@testable import Transit++@MainActor+extension MCPTestHelpers {++    static func respond(+        handler: MCPToolHandler,+        method: HTTPRequest.Method = .post,+        path: String = "/mcp",+        origin: String? = nil,+        authority: String = "127.0.0.1:3141",+        contentType: String? = nil,+        body: String = "",+        loggerLabel: String+    ) async throws -> MCPHTTPTestResponse {+        let responder = MCPServer.makeRouter(handler: handler).buildResponder()+        var headers = HTTPFields()+        if let origin {+            headers[.origin] = origin+        }+        if let contentType {+            headers[.contentType] = contentType+        }+        let request = Request(+            head: HTTPRequest(+                method: method,+                scheme: "http",+                authority: authority,+                path: path,+                headerFields: headers+            ),+            body: RequestBody(buffer: ByteBuffer(string: body))+        )++        let channel = EmbeddedChannel()+        defer { _ = try? channel.finish() }+        let context = BasicRequestContext(+            source: ApplicationRequestContextSource(+                channel: channel,+                logger: Logger(label: loggerLabel)+            )+        )++        let response = try await responder.respond(to: request, context: context)+        let writer = MCPHTTPTestResponseWriter()+        try await response.body.write(writer)+        let data = Data(buffer: writer.collated.withLockedValue { $0 })+        return MCPHTTPTestResponse(+            status: response.status,+            contentType: response.headers[.contentType],+            allow: response.headers[.allow],+            body: data+        )+    }+}++nonisolated struct MCPHTTPTestResponse {+    let status: HTTPResponse.Status+    let contentType: String?+    let allow: String?+    let body: Data++    var json: Any? {+        guard !body.isEmpty else { return nil }+        return try? JSONSerialization.jsonObject(with: body)+    }+}++private nonisolated final class MCPHTTPTestResponseWriter: ResponseBodyWriter {+    let collated = NIOLockedValueBox(ByteBuffer())++    func write(_ buffer: ByteBuffer) async throws {+        collated.withLockedValue { $0.writeImmutableBuffer(buffer) }+    }++    func finish(_: HTTPFields?) async throws {}+}+#endif

diff --git a/Transit/TransitTests/MCPNotificationDispatchTests.swift b/Transit/TransitTests/MCPNotificationDispatchTests.swiftnew file mode 100644index 0000000..d7cecd8--- /dev/null+++ b/Transit/TransitTests/MCPNotificationDispatchTests.swift@@ -0,0 +1,181 @@+#if os(macOS)+import Foundation+import HTTPTypes+import SwiftData+import Testing+@testable import Transit++/// T-1820: a JSON-RPC notification executes its method normally and only+/// suppresses its JSON-RPC response. These cases cover the direct handler and+/// Streamable HTTP transport paths for mutating tools.+@MainActor @Suite(.serialized)+struct MCPNotificationDispatchTests {++    @Test func directToolCallNotificationExecutesMutationButSuppressesResponse() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let request = try JSONDecoder().decode(+            JSONRPCRequest.self,+            from: Data(+                """+                {+                  "jsonrpc":"2.0",+                  "method":"tools/call",+                  "params":{+                    "name":"create_task",+                    "arguments":{+                      "name":"Direct notification task",+                      "type":"bug",+                      "projectId":"\(project.id.uuidString)"+                    }+                  }+                }+                """.utf8+            )+        )++        #expect(request.isNotification)+        #expect(await env.handler.handle(request) == nil)++        let tasks = try env.context.fetch(FetchDescriptor<TransitTask>())+        #expect(tasks.map(\.name) == ["Direct notification task"])+    }++    @Test func singleToolCallNotificationOverHTTPExecutesMutationWithAcceptedNoBody() 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 displayId = try #require(task.permanentDisplayId)++        let response = try await respond(handler: env.handler, body: """+        {+          "jsonrpc":"2.0",+          "method":"tools/call",+          "params":{+            "name":"add_comment",+            "arguments":{+              "displayId":\(displayId),+              "content":"Created over HTTP",+              "authorName":"TestBot"+            }+          }+        }+        """)++        #expect(response.status == .accepted)+        #expect(response.body.isEmpty)+        let comments = try env.commentService.fetchComments(for: task.id)+        #expect(comments.map(\.content) == ["Created over HTTP"])+    }++    @Test func singleToolCallFailureNotificationOverHTTPIsAcceptedWithNoBody() async throws {+        let env = try MCPTestHelpers.makeEnv()++        let response = try await respond(handler: env.handler, body: """+        {+          "jsonrpc":"2.0",+          "method":"tools/call",+          "params":{"name":"not_a_real_tool","arguments":{}}+        }+        """)++        #expect(response.status == .accepted)+        #expect(response.body.isEmpty, "Notification errors must not produce a JSON-RPC response")+    }++    @Test func mixedBatchExecutesNotificationBeforeRequestAndSuppressesNotificationFailures() 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 displayId = try #require(task.permanentDisplayId)++        let response = try await respond(handler: env.handler, body: """+        [+          {+            "jsonrpc":"2.0",+            "method":"tools/call",+            "params":{+              "name":"update_task_status",+              "arguments":{"displayId":\(displayId),"status":"planning"}+            }+          },+          {+            "jsonrpc":"2.0",+            "id":"after-mutation",+            "method":"tools/call",+            "params":{+              "name":"query_tasks",+              "arguments":{"displayId":\(displayId)}+            }+          },+          {+            "jsonrpc":"2.0",+            "method":"tools/call",+            "params":{"name":"not_a_real_tool","arguments":{}}+          }+        ]+        """)++        #expect(response.status == .ok)+        let objects = try #require(response.json as? [[String: Any]])+        #expect(+            objects.count == 1,+            "Notification success and failure responses must both be suppressed"+        )+        let object = try #require(objects.first)+        #expect(object["id"] as? String == "after-mutation")++        let result = try #require(object["result"] as? [String: Any])+        let content = try #require(result["content"] as? [[String: Any]])+        let text = try #require(content.first?["text"] as? String)+        let returnedTasks = try #require(+            try JSONSerialization.jsonObject(with: Data(text.utf8)) as? [[String: Any]]+        )+        #expect(returnedTasks.map { $0["status"] as? String } == ["planning"])+        #expect(task.status == .planning)+    }++    @Test func batchedInitializeNotificationIsSuppressedAndDoesNotBlockFollowingRequest() async throws {+        let env = try MCPTestHelpers.makeEnv()++        let response = try await respond(handler: env.handler, body: """+        [+          {+            "jsonrpc":"2.0",+            "method":"initialize",+            "params":{+              "protocolVersion":"2025-03-26",+              "capabilities":{},+              "clientInfo":{"name":"Test Client","version":"1.0"}+            }+          },+          {"jsonrpc":"2.0","id":"after-initialize","method":"ping"}+        ]+        """)++        #expect(response.status == .ok)+        let objects = try #require(response.json as? [[String: Any]])+        #expect(objects.count == 1, "The initialize notification must not produce a response")+        let object = try #require(objects.first)+        #expect(object["id"] as? String == "after-initialize")+        #expect(object["result"] != nil, "A rejected lifecycle notification must not block requests after it")+    }++    private func respond(+        handler: MCPToolHandler,+        body: String+    ) async throws -> MCPHTTPTestResponse {+        try await MCPTestHelpers.respond(+            handler: handler,+            contentType: "application/json",+            body: body,+            loggerLabel: "mcp-notification-dispatch-tests"+        )+    }+}++#endif

diff --git a/Transit/TransitTests/MCPServerBatchRequestTests.swift b/Transit/TransitTests/MCPServerBatchRequestTests.swiftindex 7e7439c..1c5bac9 100644--- a/Transit/TransitTests/MCPServerBatchRequestTests.swift+++ b/Transit/TransitTests/MCPServerBatchRequestTests.swift@@ -1,12 +1,6 @@ #if os(macOS) import Foundation import HTTPTypes-import Hummingbird-import Logging-import NIOConcurrencyHelpers-import NIOCore-import NIOEmbedded-import NIOFoundationCompat import SwiftData import Testing @testable import Transit@@ -200,7 +194,7 @@ struct MCPServerBatchRequestTests {      // MARK: - Helpers -    private func respond(body: String) async throws -> CapturedResponse {+    private func respond(body: String) async throws -> MCPHTTPTestResponse {         let env = try MCPTestHelpers.makeEnv()         return try await respond(handler: env.handler, body: body)     }@@ -208,58 +202,14 @@ struct MCPServerBatchRequestTests {     private func respond(         handler: MCPToolHandler,         body: String-    ) async throws -> CapturedResponse {-        let responder = MCPServer.makeRouter(handler: handler).buildResponder()-        let request = Request(-            head: HTTPRequest(-                method: .post,-                scheme: "http",-                authority: "127.0.0.1:3141",-                path: "/mcp",-                headerFields: [.contentType: "application/json"]-            ),-            body: RequestBody(buffer: ByteBuffer(string: body))-        )--        let channel = EmbeddedChannel()-        defer { _ = try? channel.finish() }-        let context = BasicRequestContext(-            source: ApplicationRequestContextSource(-                channel: channel, logger: Logger(label: "mcp-batch-tests")-            )-        )--        let response = try await responder.respond(to: request, context: context)-        let writer = CollatedResponseWriter()-        try await response.body.write(writer)-        let data = Data(buffer: writer.collated.withLockedValue { $0 })-        return CapturedResponse(-            status: response.status,-            contentType: response.headers[.contentType],-            body: data+    ) async throws -> MCPHTTPTestResponse {+        try await MCPTestHelpers.respond(+            handler: handler,+            contentType: "application/json",+            body: body,+            loggerLabel: "mcp-batch-tests"         )     } } -private nonisolated struct CapturedResponse {-    let status: HTTPResponse.Status-    let contentType: String?-    let body: Data--    var json: Any? {-        guard !body.isEmpty else { return nil }-        return try? JSONSerialization.jsonObject(with: body)-    }-}--private nonisolated final class CollatedResponseWriter: ResponseBodyWriter {-    let collated = NIOLockedValueBox(ByteBuffer())--    func write(_ buffer: ByteBuffer) async throws {-        collated.withLockedValue { $0.writeImmutableBuffer(buffer) }-    }--    func finish(_: HTTPFields?) async throws {}-}- #endif

diff --git a/Transit/TransitTests/MCPServerRouteTests.swift b/Transit/TransitTests/MCPServerRouteTests.swiftindex ad5144d..81ac738 100644--- a/Transit/TransitTests/MCPServerRouteTests.swift+++ b/Transit/TransitTests/MCPServerRouteTests.swift@@ -1,10 +1,6 @@ #if os(macOS) import Foundation import HTTPTypes-import Hummingbird-import Logging-import NIOCore-import NIOEmbedded import Testing @testable import Transit @@ -101,11 +97,6 @@ struct MCPServerRouteTests {         #expect(response.allow == nil)     } -    private struct RouteResponse {-        let status: HTTPResponse.Status-        let allow: String?-    }-     private func respond(         handler: MCPToolHandler,         method: HTTPRequest.Method,@@ -113,33 +104,16 @@ struct MCPServerRouteTests {         origin: String? = nil,         host: String = "127.0.0.1:3141",         body: String = ""-    ) async throws -> RouteResponse {-        let responder = MCPServer.makeRouter(handler: handler).buildResponder()-        var headers = HTTPFields()-        if let origin {-            headers[.origin] = origin-        }-        let request = Request(-            head: HTTPRequest(-                method: method,-                scheme: "http",-                authority: host,-                path: path,-                headerFields: headers-            ),-            body: RequestBody(buffer: ByteBuffer(string: body))+    ) async throws -> MCPHTTPTestResponse {+        try await MCPTestHelpers.respond(+            handler: handler,+            method: method,+            path: path,+            origin: origin,+            authority: host,+            body: body,+            loggerLabel: "mcp-route-tests"         )--        let channel = EmbeddedChannel()-        defer { _ = try? channel.finish() }-        let context = BasicRequestContext(-            source: ApplicationRequestContextSource(-                channel: channel, logger: Logger(label: "mcp-route-tests")-            )-        )--        let response = try await responder.respond(to: request, context: context)-        return RouteResponse(status: response.status, allow: response.headers[.allow])     } } 

diff --git a/specs/bugfixes/mcp-notifications-acknowledged-without-executing/report.md b/specs/bugfixes/mcp-notifications-acknowledged-without-executing/report.mdnew file mode 100644index 0000000..1ee7f2a--- /dev/null+++ b/specs/bugfixes/mcp-notifications-acknowledged-without-executing/report.md@@ -0,0 +1,88 @@+# Bugfix Report: MCP Notifications Acknowledged Without Executing++**Date:** 2026-08-05+**Status:** Fixed++## Description of the Issue++A JSON-RPC notification omits the `id` member. It must still execute its method, but it must not produce a JSON-RPC response. The historical handler returned `nil` before dispatching notification-shaped requests, so mutating `tools/call` notifications could receive HTTP 202 without creating their intended task or comment.++**Reproduction steps:**+1. Send a `tools/call` notification with no `id` for a mutating tool.+2. Observe the HTTP 202/no-body transport acknowledgement.+3. Verify that the requested mutation is persisted.++**Impact:** Agent clients can silently lose task-management mutations while receiving the expected notification acknowledgement.++## Investigation Summary++- **Symptoms examined:** Missing side effects despite correct notification response suppression.+- **Code inspected:** `MCPToolHandler.handle(_:)`, `MCPServer` single and batch routes, JSON-RPC ID decoding, and MCP lifecycle/batch tests.+- **Hypotheses tested:** Whether the route skipped dispatch, batch response filtering skipped dispatch, or the handler returned before its method switch.++The current branch already contains the production correction from `8b98e1f` (T-1834): `MCPToolHandler.handle(_:)` computes the method response after normal dispatch and only then returns `nil` for a notification. Targeted baseline tests passed before this ticket's test additions.++## Discovered Root Cause++The original defect was a **logic error**: notification response suppression was applied before method dispatch rather than after it. That conflated “do not send a response” with “do not execute the method.”++The current implementation corrects the causation chain by dispatching all valid requests through the normal method/tool path, then discarding only notification responses. Explicit `id: null` remains a non-notification invalid request; batched `initialize` retains its lifecycle rejection; HTTP transports return 202 only when no response objects remain.++## Resolution for the Issue++**Changes made:**+- `Transit/TransitTests/MCPNotificationDispatchTests.swift` — added T-1820 regression coverage for direct handler dispatch, a single HTTP mutation notification, and an ordered mixed batch with a notification failure.++**Approach rationale:** The production dispatcher already applies response suppression at the correct boundary. The new tests pin its required behavior across the direct handler and HTTP transport paths without making an unnecessary source change.++**Alternatives considered:**+- Rework the existing handler — rejected because the current branch already dispatches before suppressing notification responses.+- Cover only all-notification batches — rejected because it would not prove direct handler behavior, single-request HTTP mutation behavior, or ordering against a following valid request.++## Regression Test++**Test file:** `Transit/TransitTests/MCPNotificationDispatchTests.swift`++**Test names:**+- `directToolCallNotificationExecutesMutationButSuppressesResponse`+- `singleToolCallNotificationOverHTTPExecutesMutationWithAcceptedNoBody`+- `singleToolCallFailureNotificationOverHTTPIsAcceptedWithNoBody`+- `mixedBatchExecutesNotificationBeforeRequestAndSuppressesNotificationFailures`+- `batchedInitializeNotificationIsSuppressedAndDoesNotBlockFollowingRequest`++**What they verify:** A no-ID mutation executes through the handler and returns no response; successful and failing single HTTP notifications both return HTTP 202/no body, with the successful `add_comment` notification persisting its mutation; a mixed batch applies its notification status mutation before a valid request observes it and suppresses both successful and failed notification responses; and a batched `initialize` notification remains response-suppressed without preventing the following request from completing.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/TransitTests/MCPHTTPTestHelpers.swift` | Shared in-process HTTP request/response harness for MCP route, batch, and notification tests. |+| `Transit/TransitTests/MCPNotificationDispatchTests.swift` | Direct, successful and failing single HTTP, ordered mixed-batch mutation, and batched lifecycle-notification regressions. |+| `Transit/TransitTests/MCPServerBatchRequestTests.swift` | Migrated batch transport tests to the shared harness. |+| `Transit/TransitTests/MCPServerRouteTests.swift` | Migrated route transport tests to the shared harness. |+| `specs/bugfixes/mcp-notifications-acknowledged-without-executing/report.md` | Investigation and resolution record. |+| `CHANGELOG.md` | Unreleased T-1820 regression-coverage entry. |++## Verification++**Automated:**+- [x] Focused `MCPNotificationDispatchTests` pass.+- [x] Full macOS unit suite passes (`make test-quick`).+- [x] Linters and the SwiftData ownership guard pass (`make lint`).+- [ ] `make test` did not complete cleanly: its unit portion ran, then the UI portion reported existing-looking failures in `TransitUITests.testClearAll`, `TransitUITests.testEditViewPreservesTaskMilestone`, and `DataMaintenanceUITests.testDataMaintenanceGoldenPath` before the runner timed out.++**Manual verification:** Inspected `MCPToolHandler.handle(_:)` and `MCPServer.batchResponse`; valid dispatch occurs before notification response suppression, batch dispatch remains sequential, and all-notification batches return HTTP 202 with no body.++## Prevention++- Keep notification side-effect assertions alongside response-suppression assertions.+- Test handler and HTTP transport independently, including ordered mixed batches and notification errors.+- Preserve the `isNotification` presence distinction so explicit JSON `null` IDs remain invalid requests rather than notifications.++## Related++- T-1820+- T-1834 / commit `8b98e1f` — JSON-RPC batch support that contains the production dispatch-before-suppression fix.+- T-1863 — explicit-null request-ID validation.