Pre-push review: T-1883/bugfix-unknown-mcp-tools-return-the-wrong-json-rpc-code

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 9f7d464..de0706d 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased]  ### Fixed+- MCP `tools/call` now returns JSON-RPC `-32602 Invalid Params` for unknown tool names and maintenance tools disabled in Settings (T-1883), while retaining useful messages, omitting disabled tools from `tools/list`, and preserving `-32601 Method Not Found` for unsupported top-level methods. Handler and HTTP-route regressions cover the classification boundaries. - 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`.

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex a92a972..e7d6df6 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -211,11 +211,13 @@ final class MCPToolHandler {         }          // Gate maintenance tools behind the settings toggle. Distinct message so-        // callers can tell a disabled tool from an unknown one (AC 5.5).+        // callers can tell a disabled tool from an unknown one (AC 5.5). The+        // outer tools/call method is supported; only its tool-name parameter is+        // invalid under the negotiated MCP protocol contract.         if MCPToolDefinitions.maintenanceToolNames.contains(name), !settings.maintenanceToolsEnabled {             return JSONRPCResponse.error(                 id: id,-                code: JSONRPCErrorCode.methodNotFound,+                code: JSONRPCErrorCode.invalidParams,                 message: "Tool '\(name)' is disabled. Enable maintenance tools in Transit Settings."             )         }@@ -260,7 +262,7 @@ final class MCPToolHandler {         default:             return JSONRPCResponse.error(                 id: id,-                code: JSONRPCErrorCode.methodNotFound,+                code: JSONRPCErrorCode.invalidParams,                 message: "Unknown tool: \(name)"             )         }

diff --git a/Transit/TransitTests/MCPMaintenanceHandlerTests.swift b/Transit/TransitTests/MCPMaintenanceHandlerTests.swiftindex 9a5c85c..b0aa0c8 100644--- a/Transit/TransitTests/MCPMaintenanceHandlerTests.swift+++ b/Transit/TransitTests/MCPMaintenanceHandlerTests.swift@@ -53,7 +53,7 @@ struct MCPMaintenanceHandlerTests {      // MARK: - tools/call gating -    @Test func scanCallWhenDisabledReturnsMethodNotFound() async throws {+    @Test func scanCallWhenDisabledReturnsInvalidParams() async throws {         let env = try MCPTestHelpers.makeEnv()         env.mcpSettings.maintenanceToolsEnabled = false @@ -62,14 +62,14 @@ struct MCPMaintenanceHandlerTests {         )          let error = try requireError(response)-        #expect(error["code"] as? Int == JSONRPCErrorCode.methodNotFound)+        #expect(error["code"] as? Int == JSONRPCErrorCode.invalidParams)         #expect(             error["message"] as? String                 == "Tool 'scan_duplicate_display_ids' is disabled. Enable maintenance tools in Transit Settings."         )     } -    @Test func reassignCallWhenDisabledReturnsMethodNotFound() async throws {+    @Test func reassignCallWhenDisabledReturnsInvalidParams() async throws {         let env = try MCPTestHelpers.makeEnv()         env.mcpSettings.maintenanceToolsEnabled = false @@ -78,7 +78,7 @@ struct MCPMaintenanceHandlerTests {         )          let error = try requireError(response)-        #expect(error["code"] as? Int == JSONRPCErrorCode.methodNotFound)+        #expect(error["code"] as? Int == JSONRPCErrorCode.invalidParams)         #expect(             error["message"] as? String                 == "Tool 'reassign_duplicate_display_ids' is disabled. Enable maintenance tools in Transit Settings."

diff --git a/Transit/TransitTests/MCPServerBatchRequestTests.swift b/Transit/TransitTests/MCPServerBatchRequestTests.swiftindex 57ebfb8..c47c954 100644--- a/Transit/TransitTests/MCPServerBatchRequestTests.swift+++ b/Transit/TransitTests/MCPServerBatchRequestTests.swift@@ -134,6 +134,18 @@ struct MCPServerBatchRequestTests {         #expect(object["result"] != nil)     } +    @Test func unknownToolCallOverRouteReturnsInvalidParams() async throws {+        let response = try await respond(+            body: #"{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"not_a_real_tool","arguments":{}}}"#+        )++        #expect(response.status == .ok)+        let object = try #require(response.json as? [String: Any])+        let error = try #require(object["error"] as? [String: Any])+        #expect(error["code"] as? Int == JSONRPCErrorCode.invalidParams)+        #expect(error["message"] as? String == "Unknown tool: not_a_real_tool")+    }+     // MARK: - Helpers      private func respond(body: String) async throws -> CapturedResponse {

diff --git a/Transit/TransitTests/MCPToolHandlerTests.swift b/Transit/TransitTests/MCPToolHandlerTests.swiftindex d319910..f988cbc 100644--- a/Transit/TransitTests/MCPToolHandlerTests.swift+++ b/Transit/TransitTests/MCPToolHandlerTests.swift@@ -81,6 +81,17 @@ struct MCPToolHandlerTests {         #expect(error["code"] as? Int == -32601)     } +    @Test func unknownToolNameReturnsInvalidParams() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let response = await env.handler.handle(+            MCPTestHelpers.toolCallRequest(tool: "not_a_real_tool", arguments: [:])+        )++        let error = try MCPTestHelpers.jsonRPCError(response)+        #expect(error["code"] as? Int == JSONRPCErrorCode.invalidParams)+        #expect(error["message"] as? String == "Unknown tool: not_a_real_tool")+    }+     // MARK: - create_task      @Test func createTaskSuccess() async throws {

diff --git a/docs/agent-notes/mcp-server.md b/docs/agent-notes/mcp-server.mdindex cf7ed7c..0dcb9f2 100644--- a/docs/agent-notes/mcp-server.md+++ b/docs/agent-notes/mcp-server.md@@ -50,7 +50,7 @@ Key challenge: Hummingbird runs on SwiftNIO event loops (nonisolated), but servi `scan_duplicate_display_ids` and `reassign_duplicate_display_ids` are exposed only when `MCPSettings.maintenanceToolsEnabled` is true. The toggle is persisted under UserDefaults key `mcpMaintenanceToolsEnabled`.  - `MCPToolDefinitions` is split into `coreTools` and `maintenanceTools`; `tools(includingMaintenance:)` returns the right subset. The legacy `all` alias still resolves to `coreTools` only — anything in production should use the helper.-- When the toggle is off, `tools/list` excludes both maintenance tools and `tools/call` for either name returns JSON-RPC `methodNotFound` (-32601) with the literal message `Tool '<name>' is disabled. Enable maintenance tools in Transit Settings.` — distinct from the "Unknown tool" message used for genuinely unknown names.+- When the toggle is off, `tools/list` excludes both maintenance tools and `tools/call` for either name returns JSON-RPC `invalidParams` (-32602) with the literal message `Tool '<name>' is disabled. Enable maintenance tools in Transit Settings.` — distinct from the "Unknown tool" message used for genuinely unknown names. - The toggle takes effect on the next `tools/list` without restart (settings is read live). - Dispatch handlers (`handleScanDuplicateDisplayIds`, `handleReassignDuplicateDisplayIds`) encode `DisplayIDMaintenanceTypes` (Codable structs) via a shared `encodedTextResult(_:)` helper that JSON-encodes any `Encodable` and wraps it in the `MCPToolResult.content[text]` envelope. 

diff --git a/specs/bugfixes/unknown-mcp-tools-return-the-wrong-json-rpc-code/report.md b/specs/bugfixes/unknown-mcp-tools-return-the-wrong-json-rpc-code/report.mdnew file mode 100644index 0000000..858b6d2--- /dev/null+++ b/specs/bugfixes/unknown-mcp-tools-return-the-wrong-json-rpc-code/report.md@@ -0,0 +1,98 @@+# Bugfix Report: Unknown MCP Tools Return the Wrong JSON-RPC Code++**Date:** 2026-08-02+**Status:** Fixed+**Transit ticket:** T-1883++## Description of the Issue++Transit advertises MCP protocol version `2025-03-26`, but a supported `tools/call` request with an unknown tool name returns JSON-RPC `methodNotFound` (`-32601`). The outer method is supported; only the `name` parameter is invalid, so MCP clients receive the wrong classification. The same wrong code is used when a maintenance tool is known but disabled in Settings and therefore omitted from `tools/list`.++**Reproduction steps:**+1. Send `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"invalid_tool_name","arguments":{}}}` to `POST /mcp`.+2. Observe an error code of `-32601`.+3. Disable maintenance tools and call `scan_duplicate_display_ids`; observe the same code despite the useful disabled-tool message.++**Expected:** Both supported `tools/call` cases return `JSONRPCErrorCode.invalidParams` (`-32602`) while preserving their useful messages. `tools/list` continues to omit disabled maintenance tools, and an unknown top-level method continues to return `-32601`.++**Impact:** MCP clients may treat a malformed tool name as an unsupported protocol method, leading to incorrect retry, capability, or fallback behavior.++## Investigation Summary++- **Symptoms examined:** Handler and HTTP route responses for arbitrary unknown tool names, disabled maintenance tools, `tools/list`, and unknown top-level methods.+- **Code inspected:** `MCPToolHandler.handle(_:)`, `handleToolCall`, `MCPToolDefinitions`, existing maintenance gating tests, and `MCPServer.makeRouter` route dispatch.+- **Hypotheses tested:** The protocol method dispatch is correct; the defect is isolated to tool-name classification after `tools/call` has already been recognized.++## Discovered Root Cause++`MCPToolHandler.handleToolCall` used `methodNotFound` in both the disabled-maintenance gate and the default tool-name branch. This conflated an unsupported JSON-RPC method with an invalid tool argument to the supported `tools/call` method.++**Defect type:** Logic error — incorrect error classification.++**Why it occurred:** The handler reused the JSON-RPC method-not-found code for tool dispatch failures, even though MCP tool names are parameters of a supported method and the negotiated protocol requires invalid parameters (`-32602`).++**Contributing factors:** Maintenance tools are intentionally omitted from `tools/list` when disabled, which made the existing message useful but did not change the JSON-RPC classification required for a `tools/call` request.++## Resolution for the Issue++The handler now classifies both tool-name failure paths as invalid parameters while leaving the unsupported top-level method path unchanged.++**Changes made:**+- `Transit/Transit/MCP/MCPToolHandler.swift` — changed the disabled-maintenance gate and unknown-tool fallback from `JSONRPCErrorCode.methodNotFound` to `JSONRPCErrorCode.invalidParams`; retained the existing disabled and unknown-tool messages.+- `Transit/TransitTests/MCPToolHandlerTests.swift` — added arbitrary unknown-tool handler coverage and retained the existing unknown top-level method assertion for `-32601`.+- `Transit/TransitTests/MCPMaintenanceHandlerTests.swift` — updated both disabled maintenance-tool cases to assert `-32602` and exact message preservation; existing `tools/list` omission coverage remains unchanged.+- `Transit/TransitTests/MCPServerBatchRequestTests.swift` — added a route-level `POST /mcp` assertion for HTTP 200 plus JSON-RPC `-32602`.+- `docs/agent-notes/mcp-server.md` — corrected the documented disabled-tool code.++**Approach rationale:** The smallest safe fix is to change only the two error-code selections inside `handleToolCall`. `handle(_:)` continues to use `methodNotFound` for unsupported top-level methods, and the maintenance gate still provides a distinct actionable message while `tools/list` remains settings-driven.++**Alternatives considered:**+- Return `methodNotFound` for disabled maintenance tools because they are omitted from `tools/list` — rejected; the request still invokes the supported `tools/call` method and the negotiated contract classifies the invalid tool parameter as `-32602`.+- Change the messages or remove the disabled-tool distinction — rejected; callers benefit from the existing actionable Settings guidance.+- Add a new error code — rejected; JSON-RPC already defines the required `invalidParams` code.+++## Regression Test++**Test files:**+- `Transit/TransitTests/MCPToolHandlerTests.swift` — arbitrary unknown tool name at the handler boundary.+- `Transit/TransitTests/MCPMaintenanceHandlerTests.swift` — disabled maintenance-tool behavior.+- `Transit/TransitTests/MCPServerBatchRequestTests.swift` — unknown tool name through the HTTP route.++**What it verifies:** A supported `tools/call` with an unknown or disabled tool name returns `-32602` and preserves the existing message; `tools/list` omission and top-level unknown-method behavior remain covered by existing tests.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/MCP/MCPToolHandler.swift` | Classify unknown and disabled tool names as `invalidParams` while preserving top-level method errors. |+| `Transit/TransitTests/MCPToolHandlerTests.swift` | Add arbitrary unknown-tool handler regression and preserve top-level method coverage. |+| `Transit/TransitTests/MCPMaintenanceHandlerTests.swift` | Assert disabled maintenance tools return `-32602` with their existing messages. |+| `Transit/TransitTests/MCPServerBatchRequestTests.swift` | Add route-level unknown-tool JSON-RPC regression. |+| `docs/agent-notes/mcp-server.md` | Align maintenance-tool behavior documentation with `invalidParams`. |+| `CHANGELOG.md` | Record the T-1883 protocol classification fix. |+| `specs/bugfixes/unknown-mcp-tools-return-the-wrong-json-rpc-code/report.md` | Document investigation, resolution, and verification. |++## Verification++**Automated:**+- [x] Regression tests pass+- [x] Full macOS unit test suite passes via `make test-quick`+- [x] SwiftLint and repository validators pass via `make lint` (0 violations in 319 files)++**Manual verification:**+- [x] Route-level test confirms `POST /mcp` returns HTTP 200 with a JSON-RPC `-32602` error for an unknown tool.+- [x] Existing tests confirm disabled tools remain absent from `tools/list` and unsupported top-level methods remain `-32601`.++## Prevention++- Keep JSON-RPC method errors separate from errors caused by parameters to supported MCP methods.+- Add route-level coverage for protocol classification changes, not only direct handler tests.+- Preserve useful error messages and capability-list behavior when changing error codes.++## Related++- MCP Tools specification: https://modelcontextprotocol.io/specification/2025-03-26/server/tools+- Transit ticket T-1883