Pre-push review: T-1835/bugfix-mcp-get-endpoint-returns-404-instead-of-405

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex bd359a9..50c4c81 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -8,6 +8,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 `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/MCP/MCPServer.swift b/Transit/Transit/MCP/MCPServer.swiftindex 8f2784c..6cb4b68 100644--- a/Transit/Transit/MCP/MCPServer.swift+++ b/Transit/Transit/MCP/MCPServer.swift@@ -193,22 +193,19 @@ extension MCPServer {  extension MCPServer { -    /// Builds the Hummingbird router for the single `POST /mcp` endpoint.+    /// Builds the Hummingbird router for the MCP endpoint.+    /// POST carries JSON-RPC; GET is explicitly 405 because no SSE stream is offered.     /// `nonisolated` keeps transport construction independent of MainActor;-    /// route callbacks execute on Hummingbird/NIO and hop to MainActor only-    /// when dispatching through `MCPToolHandler`.+    /// route callbacks execute on Hummingbird/NIO and hop to MainActor when dispatching.     nonisolated static func makeRouter(handler: MCPToolHandler) -> Router<BasicRequestContext> {         let router = Router()+        router.get("mcp") { request, _ -> Response in+            guard Self.isAllowedMCPRequest(request) else { return forbiddenResponse() }+            return Response(status: .methodNotAllowed, headers: [.allow: "POST"])+        }         router.post("mcp") { request, _ -> Response in-            // DNS-rebinding defence required by the MCP Streamable HTTP-            // transport. This must stay the first statement in the route: an-            // untrusted caller's body is never read, decoded, or dispatched.-            if MCPOriginValidator.rejectionReason(-                origin: request.head.headerFields[.origin],-                authority: request.head.authority-            ) != nil {-                return forbiddenResponse()-            }+            // Validate origin before reading the body.+            guard Self.isAllowedMCPRequest(request) else { return forbiddenResponse() }              let body = try await request.body.collect(upTo: 1_048_576)             let data = Data(buffer: body)@@ -359,13 +356,18 @@ extension MCPServer {         )     } -    /// Transport-level rejection. Deliberately not a JSON-RPC error body: the-    /// request never entered a JSON-RPC session, and answering `200` with an-    /// error object would tell an attacker's page that the endpoint is live.-    ///-    /// The body is a fixed string rather than the validator's specific reason,-    /// so a probing script cannot learn whether it was the `Origin` or the-    /// `Host` check that tripped.+    /// Validates Origin and Host/authority before body access or dispatch.+    /// This MCP DNS-rebinding defence must remain first in every MCP route.+    nonisolated private static func isAllowedMCPRequest(_ request: Request) -> Bool {+        let origin = request.head.headerFields[.origin]+        return MCPOriginValidator.rejectionReason(+            origin: origin,+            authority: request.head.authority+        ) == nil+    }++    /// Transport-level rejection, not JSON-RPC: fixed plain text avoids+    /// revealing whether Origin or Host validation failed to a prober.     nonisolated private static func forbiddenResponse() -> Response {         Response(             status: .forbidden,

diff --git a/Transit/TransitTests/MCPServerRouteTests.swift b/Transit/TransitTests/MCPServerRouteTests.swiftnew file mode 100644index 0000000..ad5144d--- /dev/null+++ b/Transit/TransitTests/MCPServerRouteTests.swift@@ -0,0 +1,146 @@+#if os(macOS)+import Foundation+import HTTPTypes+import Hummingbird+import Logging+import NIOCore+import NIOEmbedded+import Testing+@testable import Transit++/// Regression tests for T-1835: the advertised MCP Streamable HTTP endpoint+/// must distinguish an unsupported method on `/mcp` from an unknown route.+///+/// MCP 2025-03-26 requires GET on the MCP endpoint to return 405 when the+/// server does not offer an SSE listening stream. Hummingbird's unmatched+/// route fallback previously returned 404 because only POST was registered.+@MainActor @Suite(.serialized)+struct MCPServerRouteTests {++    @Test func getMcpReturnsMethodNotAllowedWithPostAllowHeader() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .get,+            path: "/mcp"+        )++        #expect(response.status == .methodNotAllowed)+        #expect(response.allow == "POST")+    }++    @Test func postMcpStillDispatchesNormally() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .post,+            path: "/mcp",+            body: #"{"jsonrpc":"2.0","id":1,"method":"ping"}"#+        )++        #expect(response.status == .ok)+    }++    @Test func getMcpStillValidatesUntrustedOrigins() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .get,+            path: "/mcp",+            origin: "https://evil.example.com"+        )++        #expect(response.status == .forbidden)+    }++    @Test func getMcpStillValidatesNonLoopbackHost() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .get,+            path: "/mcp",+            host: "rebind.example.com:3141"+        )++        #expect(response.status == .forbidden)+    }++    @Test func headMcpRemainsNotFoundWithoutAutoGeneratedHeadRoute() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .head,+            path: "/mcp"+        )++        #expect(response.status == .notFound)+        #expect(response.allow == nil)+    }++    @Test func optionsMcpRemainsNotFoundWithoutExplicitOptionsRoute() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .options,+            path: "/mcp"+        )++        #expect(response.status == .notFound)+        #expect(response.allow == nil)+    }++    @Test func getOnUnrelatedPathRemainsNotFound() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = try await respond(+            handler: env.handler,+            method: .get,+            path: "/not-mcp"+        )++        #expect(response.status == .notFound)+        #expect(response.allow == nil)+    }++    private struct RouteResponse {+        let status: HTTPResponse.Status+        let allow: String?+    }++    private func respond(+        handler: MCPToolHandler,+        method: HTTPRequest.Method,+        path: String,+        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))+        )++        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])+    }+}++#endif

diff --git a/docs/agent-notes/mcp-server.md b/docs/agent-notes/mcp-server.mdindex 281ae07..cf7ed7c 100644--- a/docs/agent-notes/mcp-server.md+++ b/docs/agent-notes/mcp-server.md@@ -10,7 +10,7 @@ Embedded MCP server in the Transit macOS app using Hummingbird HTTP server. Expo Claude Code ←→ HTTP POST /mcp (localhost:3141) ←→ MCPServer ←→ MCPToolHandler ←→ TaskService/ProjectService/CommentService ←→ SwiftData ``` -- **Transport**: Streamable HTTP, single `POST /mcp` endpoint, JSON-RPC 2.0+- **Transport**: Streamable HTTP, JSON-RPC 2.0 over `POST /mcp`; `GET /mcp` is explicitly handled with HTTP 405 and `Allow: POST` because Transit does not offer an SSE listening stream - **HTTP server**: Hummingbird 2.x (SwiftNIO-based), binds to `127.0.0.1` only - **Lifecycle**: Opt-in via Settings toggle. Default port 3141. 

diff --git a/specs/bugfixes/mcp-get-endpoint-returns-404-instead-of-405/report.md b/specs/bugfixes/mcp-get-endpoint-returns-404-instead-of-405/report.mdnew file mode 100644index 0000000..84c7e7f--- /dev/null+++ b/specs/bugfixes/mcp-get-endpoint-returns-404-instead-of-405/report.md@@ -0,0 +1,156 @@+# Bugfix Report: MCP GET Endpoint Returns 404 Instead of 405++**Date:** 2026-08-02+**Status:** Fixed+**Transit ticket:** T-1835++## Description of the Issue++Transit advertises the MCP Streamable HTTP transport version `2025-03-26`, but+its Hummingbird router registers only `POST /mcp`. A client probing the MCP+endpoint with `GET /mcp` therefore reaches Hummingbird's unmatched-route+fallback and receives `404 Not Found`.++For Streamable HTTP, GET is the endpoint's optional server-to-client SSE+operation. When Transit does not offer an SSE listening stream, the MCP+endpoint must respond with `405 Method Not Allowed`, ideally including+`Allow: POST`, so a client can distinguish an unsupported endpoint method from+an unknown path.++**Reproduction steps:**+1. Enable the MCP server in Transit Settings.+2. Send `GET http://127.0.0.1:3141/mcp`.+3. Observe `404 Not Found` instead of `405 Method Not Allowed`.++**Impact:** MCP clients probing the advertised endpoint cannot distinguish an+unsupported SSE listening stream from a missing MCP endpoint. POST-based MCP+operation is unaffected.++## Investigation Summary++The investigation followed the systematic debugging workflow:++- **Symptoms examined:** The ticket reports `GET /mcp` returning 404. A+  route-level regression test reproduces that response on the real Hummingbird+  router.+- **Code inspected:** `Transit/Transit/MCP/MCPServer.swift` registers the+  single `POST /mcp` route; the POST closure performs origin validation,+  request decoding, and JSON-RPC dispatch. Existing lifecycle, origin, and+  batch tests establish the route test harness and unchanged behavior.+- **Hypotheses tested:** This is not a lifecycle or binding problem: the+  router can be exercised without starting a listener. It is not a JSON-RPC+  decoding problem: GET has no route match and never reaches the POST body+  handling. The supplied ticket identifies Hummingbird's+  `NotFoundResponder` as the source of the 404.++## Discovered Root Cause++`MCPServer.makeRouter(handler:)` registers `POST /mcp` but does not register a+GET handler for the same path. Hummingbird consequently treats the valid MCP+endpoint path as an unknown route for GET and returns its default 404 response.++**Defect type:** Missing route / protocol contract mismatch.++**Why it occurred:** The initial Streamable HTTP implementation modeled+Transit as POST-only and omitted the explicit method response required when no+SSE listening stream is offered.++**Contributing factors:** Hummingbird's unmatched-route fallback does not+encode the MCP endpoint's method semantics, so the distinction must be made by+the application router.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/MCP/MCPServer.swift:196-217` - registers an explicit+  `GET /mcp` route. It repeats the existing origin/authority validation and+  returns HTTP 405 with `Allow: POST` when the request is from an allowed+  local client. The existing POST route and its body/JSON-RPC dispatch path+  are unchanged.+- `Transit/TransitTests/MCPServerRouteTests.swift` - adds route-level tests+  for the 405 status, `Allow: POST` header, unchanged POST success, GET origin+  rejection, and unrelated-path 404 behavior.+- `docs/agent-notes/mcp-server.md` and+  `specs/mcp-server/implementation.md` - clarify that GET is explicitly+  handled as an unsupported SSE operation while POST remains the data path.++**Approach rationale:** The fix is deliberately at the Hummingbird router+boundary. Registering GET on the exact `/mcp` path lets Hummingbird distinguish+an unsupported method from an unknown path, while the `Allow` header tells MCP+clients that POST is the supported operation. Repeating origin validation+before returning 405 preserves the transport security policy for every MCP+request. No lifecycle or POST handler code needs to change.++**Alternatives considered:**+- **Leave GET unmatched and rely on Hummingbird's 404** - rejected because it+  violates the advertised Streamable HTTP `2025-03-26` endpoint contract and+  prevents clients from distinguishing method support from route absence.+- **Add a global method-not-allowed middleware** - rejected because the+  behavior is specific to the MCP endpoint and a route-local handler is the+  smallest change with no unrelated-path effects.+- **Return 405 without origin validation** - rejected because MCP origin+  validation applies to incoming transport requests; the new route must not+  create a browser-origin bypass.+++## Regression Test++**Test file:** `Transit/TransitTests/MCPServerRouteTests.swift`++**Test names:**+- `getMcpReturnsMethodNotAllowedWithPostAllowHeader`+- `postMcpStillDispatchesNormally`+- `getMcpStillValidatesUntrustedOrigins`+- `getMcpStillValidatesNonLoopbackHost`+- `headMcpRemainsNotFoundWithoutAutoGeneratedHeadRoute`+- `optionsMcpRemainsNotFoundWithoutExplicitOptionsRoute`+- `getOnUnrelatedPathRemainsNotFound`++**What it verifies:** The real Hummingbird router returns 405 and `Allow: POST`+for GET on `/mcp`, keeps POST dispatch working, retains origin and host rejection+on the new route, confirms unsupported HEAD/OPTIONS use the framework's 404+fallback, and continues returning 404 for an unrelated path.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/MCP/MCPServer.swift` | Explicit GET `/mcp` method response with origin validation. |+| `Transit/TransitTests/MCPServerRouteTests.swift` | New route-level regression coverage. |+| `docs/agent-notes/mcp-server.md` | MCP transport behavior and endpoint documentation. |+| `specs/mcp-server/implementation.md` | MCP implementation architecture documentation. |+| `specs/bugfixes/mcp-get-endpoint-returns-404-instead-of-405/report.md` | Investigation and resolution record. |+| `CHANGELOG.md` | Unreleased fixed-entry for T-1835. |++## Verification++**Automated:**+- [x] Regression tests fail before the fix, reproducing the incorrect 404 and missing endpoint method response.+- [x] `make test-quick` passes after the fix, including all existing macOS unit tests and the seven T-1835 route tests.+- [x] `make lint` passes in strict mode.+- [x] `make build-macos` passes for the affected macOS target.++**Manual verification:**+- [x] Route-level request dispatch confirms `GET /mcp` returns 405 with+  `Allow: POST`.+- [x] Route-level request dispatch confirms a normal `POST /mcp` ping still+  returns HTTP 200.+- [x] Route-level request dispatch confirms `GET /not-mcp` remains 404 without+  an `Allow` header.+++## Prevention++- Register explicit handlers for every HTTP method required by an advertised+  transport contract; do not rely on an unmatched-route fallback for method+  semantics.+- Keep route-level tests for both supported and intentionally unsupported+  methods, including the `Allow` header and an unrelated-path 404 control.++## Related++- Transit ticket T-1835+- MCP Streamable HTTP transport contract for `2025-03-26`:+  https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#listening-for-messages-from-the-server

diff --git a/specs/mcp-server/implementation.md b/specs/mcp-server/implementation.mdindex 2d0b79d..109243d 100644--- a/specs/mcp-server/implementation.md+++ b/specs/mcp-server/implementation.md@@ -44,7 +44,7 @@ All MCP code is behind `#if os(macOS)` guards. iOS builds are unaffected.  The architecture has three layers: -1. **Transport layer** (`MCPServer`): Hummingbird HTTP server with a single `POST /mcp` route. Runs in a `Task.detached` to keep `Application.run()` off the MainActor. Parses raw HTTP into `JSONRPCRequest`, dispatches to the handler, serialises the response back to HTTP.+1. **Transport layer** (`MCPServer`): Hummingbird HTTP server with JSON-RPC over `POST /mcp`; `GET /mcp` is explicitly handled with HTTP 405 and `Allow: POST` because Transit does not offer an SSE listening stream. Runs in a `Task.detached` to keep `Application.run()` off the MainActor. Parses raw HTTP into `JSONRPCRequest`, dispatches to the handler, serialises the response back to HTTP.  2. **Protocol layer** (`MCPToolHandler`): `@MainActor` class that owns references to `TaskService` and `ProjectService`. Dispatches JSON-RPC methods (`initialize`, `tools/list`, `tools/call`, `ping`) and translates tool arguments into service calls. Reuses `IntentHelpers` for JSON encoding and project lookup error mapping — same code path as the App Intents layer.