Pre-push review: T-1863/bugfix-mcp-accepts-request-ids-that-are-null

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 912191d..5411aeb 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -1,91 +1,92 @@ # Changelog  All notable changes to this project will be documented in this file.  The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ## [Unreleased]  ### Fixed - MCP `create_task` and JSON `CreateTaskIntent` now reject present non-object `metadata` values (string, array, number, boolean, or null) with field-specific errors before task insertion, while omitted metadata and T-723's tolerant handling of values inside valid objects remain unchanged (T-1991). - T-2036: `QueryTasksIntent` now treats empty `completionDate` and `lastStatusChangeDate` objects as valid no-op filters, including when both are present. Raw-object emptiness is preserved before Codable decoding, while nested nulls, malformed non-empty objects, strict dates, reversed ranges, open bounds, and relative precedence remain covered by regression tests.+- MCP request validation now rejects explicit JSON `id: null` as JSON-RPC `-32600 Invalid Request` under protocol `2025-03-26`, while preserving omitted-ID notifications and valid string/integer IDs across single and batch requests (T-1863). - 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-1819: `QueryTasksIntent` now rejects reversed absolute `from`/`to` date ranges for both `completionDate` and `lastStatusChangeDate` with `INVALID_INPUT`, matching visual `FindTasksIntent` behavior. One-sided/open bounds, equal same-day inclusive ranges, relative-filter precedence, strict T-1799 calendar-date parsing, and current calendar/time-zone semantics remain unchanged; cross-surface regressions cover both date fields. - 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`. - `AddTaskIntent` now distinguishes an empty-database/no-selection path (`NO_PROJECTS`) from a stale selected `ProjectEntity` (`PROJECT_NOT_FOUND`) even when the deleted project was the last project in the store (T-1814). The required App Intents project parameter contract is preserved; missing project input against a non-empty store returns `INVALID_INPUT`. Entity resolution happens before task creation, and focused regressions assert exact error cases and no task mutation. - 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. - T-1802: Compact-width portrait dashboard layouts now always use the segmented single-column view, including widths at and above 400 points. Landscape, iPad, and Mac continue to use geometry-based column adaptation through a focused layout-selection helper and regression coverage. - 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. - Cross-device task and milestone promotion now re-probes committed SwiftData state after display-ID allocation and skips assignment when a peer has already supplied a permanent ID (T-2020). Missing or unreadable records fail closed, save-failure recovery preserves peer-merged values, and deterministic two-context tests pin both record types. Because SwiftData has no field-level compare-and-set, this closes the peer-merge-during-await window but cannot prevent two devices whose final local probes both precede either CloudKit merge from racing; the limitation and owner-reservation alternative are documented.  - MCP `initialize` now requires object params with string `protocolVersion`, object `capabilities`, and typed `clientInfo` identity fields (T-1778). Missing or malformed method parameters return JSON-RPC `-32602 Invalid Params`. JSON-RPC request decoding now rejects scalar/null top-level `params` server-wide with `-32600 Invalid Request`. Supported `2025-03-26` requests are echoed and unsupported versions receive the server's latest advertised supported version. Focused handshake tests cover absent params/fields, wrong types, malformed client information, request-shape errors, and both negotiation paths. - MCP server start, stop, and same-port restart now run through one async desired-state lifecycle coordinator (T-1826). Each active listener owns its Hummingbird `ServiceGroup` and run task; teardown explicitly triggers graceful shutdown and awaits the group before rebinding, rather than assuming cancellation completion releases the socket. Graceful shutdown is time-bounded so a stalled request cannot wedge the coordinator, and the group installs no SIGTERM/SIGINT traps (`runService()`'s default) because those change the process's signal disposition permanently. Requests arriving during teardown replace the pending target so a burst converges on the latest state, Settings uses an explicit restart operation, and live loopback regressions cover stop, 20 serialized same-port repetitions, different-port restart, rapid off/on, an occupied port surfacing the bind failure, an invalid port releasing the running listener, bounded shutdown escalation, empty signal configuration, and reusable bind-probe behavior. - Already-cancelled task and milestone creates now stop before an uncontended display-ID allocation, and cancellation during a successful non-cooperative allocation is re-checked before SwiftData insertion (T-1765). `AllocationGate` checks cancellation once it holds the lock and before running the allocation body, while both create services guard their persistence boundary; existing selective `insertOrDelete` save-failure cleanup is unchanged. The gate check is shared by every allocator caller, so a cancelled display-ID promotion or duplicate-cleanup pass now stops at the current record and retries on the next pass. If the counter advance already succeeded, cancellation may leave a harmless gap in the display-ID sequence rather than persisting a ghost record. Four deterministic regressions cover the new timing windows across tasks and milestones, and the two retained queued-cancellation cases now prove that their contender entered and was removed from the gate queue. - The MCP Streamable HTTP endpoint now accepts non-empty JSON-RPC request/notification batches for advertised protocol `2025-03-26` (T-1834). It decodes each member independently, dispatches requests and notifications sequentially, omits notification responses, preserves response-array semantics, returns HTTP 202 with no body for all-notification batches, emits per-member `-32600` errors for invalid entries, handles empty batches as one invalid-request object, and rejects lifecycle-invalid batched `initialize` calls. Route-level regression tests cover each behavior and unchanged single-request responses. - MCP `create_task`, `CreateTaskIntent`, and `AddTaskSheet.persist` now create a task plus optional milestone atomically in one `TaskService` save (T-1768). The relationship is validated and attached before insertion; failed saves delete the pending aggregate instead of relying on a second, fallible compensating deletion. Surface-level failure regressions verify no task remains or is resurrected by a later save. `CreateTaskIntent` maps the merged save's failures per error kind, so a storage failure that was previously reported as `INTERNAL_ERROR` by the separate milestone-assignment step still reports `INTERNAL_ERROR` rather than collapsing into `INVALID_INPUT`; `milestoneProjectMismatch` from the service boundary surfaces as `MILESTONE_PROJECT_MISMATCH`. - Duplicate cleanup now re-probes a task or milestone loser's committed display ID after asynchronous allocation and immediately before mutation (T-2019). Peer changes that land during allocation are preserved and reported as `stale-id`; task cleanup no longer emits a false audit comment for an overwritten peer repair. Deterministic gated regressions cover both record types and the task audit path. - Editor conflict choices now protect against unseen concurrent changes across task, project, and milestone editors (T-1935). **Use Updated Values** rebases the full draft onto current live values while preserving only genuine non-conflicting user edits, and **Keep My Changes** authorizes only the exact conflict field values shown; either action re-alerts when the conflict snapshot changes. - SwiftData test fixtures now retain their backing `ModelContainer` for the full test lifetime (T-2003). `TestModelContainer` is a container-owning fixture instead of a bare-context factory, 207 context-acquisition call sites across 92 test files were migrated, and bespoke context helpers in task-entity, task-creation-result, and report tests now delegate to the shared fixture. An escaped-context lifetime regression plus an executable ownership guard with positive/negative fixtures prevent raw container/context factories from recurring.  - Cross-device milestone name conflicts no longer make name-based operations target an arbitrary record (T-1938). `MilestoneService.findByName` now throws on multiple project-scoped matches; all MCP and App Intent callers report the ambiguity (`AMBIGUOUS_MILESTONE` for intents). Launch/foreground/connectivity maintenance deterministically keeps the oldest milestone name and renames other UUID-distinct records with UUID-derived suffixes, preserving records and task assignments. Cross-context tests simulate CloudKit imports and verify ambiguity reporting, reconciliation, idempotence, and data preservation. - Task mutation surfaces no longer hide duplicate task display IDs behind a not-found error (T-1837). `TaskService.findByDisplayID` already threw `duplicateDisplayID` when several tasks share a `permanentDisplayId` (possible under CloudKit, which cannot enforce uniqueness), but MCP `update_task_status` / `update_task` / `add_comment`, `UpdateStatusIntent`, `IntentHelpers.resolveTask`, and the visual `AddCommentIntent` all collapsed it into `TASK_NOT_FOUND` / "Provide either displayId or taskId", so operators could not tell missing data from store corruption needing display-ID maintenance. Duplicates now report `INTERNAL_ERROR` with the hint `Duplicate task identifier for displayId N` (App Intents — matching the milestone paths and the T-1097 `QueryTasksIntent` fix) or an `isError` result reading `Duplicate task identifier detected for displayId N` (MCP — matching the milestone tools). MCP `query_tasks` uses the same wording instead of leaking `Lookup failed: duplicateDisplayID`. No new error code was introduced. The three MCP call sites now share `resolveTaskArgument`, `UpdateStatusIntent` routes through `IntentHelpers.resolveTask` instead of its own copy of the mapping, and `VisualIntentError` gains a `duplicateIdentifier` case (code `INTERNAL_ERROR`). Regression coverage in `DuplicateTaskDisplayIDIntentTests` and `MCPDuplicateTaskDisplayIDTests`. - The MCP `add_comment` handler now distinguishes a missing required field from a present-but-non-string one for `content` and `authorName` (T-1579). Previously `guard let value = args[key] as? String, !value.isEmpty` collapsed numbers, booleans, arrays, and null into `Missing required argument: content` / `Missing required argument: authorName`, misleading callers into thinking they omitted a field they actually supplied with the wrong type. A present non-string value now returns the field-specific `content must be a string` / `authorName must be a string` before any task resolution or comment creation, matching the hardened pattern used by `update_task_status` after T-1205. Extracted a `requiredString(_:key:)` helper to keep `handleAddComment` within the cyclomatic-complexity limit. Regression coverage added in `MCPAddCommentTypeValidationTests` (numeric/boolean/array/null `content` and `authorName`). - `IntentHelpers.resolveMilestone` now rejects a present-but-non-string `name` identifier (number, boolean, null, array) with `INVALID_INPUT` and the field-specific hint `name must be a string`, instead of treating the failed `as? String` cast as an absent identifier and returning the generic `Provide displayId, milestoneId, or name with project` hint (T-1572). This only affects the `UpdateMilestoneIntent` name+project identifier path; no update fields are applied when the identifier is malformed. The MCP `update_milestone` tool is unaffected (it does not support name-based identification). Regression tests added to `MilestoneRenameTypeValidationTests`. - The MCP `update_milestone` handler and `UpdateMilestoneIntent` now support clearing a milestone description by passing an empty or whitespace-only `description` string, mirroring `update_task` and the milestone edit UI (T-1555). Previously a present `description` was applied verbatim, so `""`/whitespace persisted an empty string and the MCP response kept emitting a `description` key. Non-empty values are now trimmed before storage, and a cleared description sets `milestoneDescription` to nil (the MCP `milestoneToDict` serializer already omits the key when nil). For cross-surface parity, `UpdateMilestoneIntent`'s response now also emits `description` when set and omits it when cleared (previously it never emitted the key). T-343 had fixed only the edit-UI path. Cross-surface regression tests added in `MCPMilestoneClearDescriptionTests` and `UpdateMilestoneClearDescIntentTests`. - MCP `update_task` apply/save failure messages are now uniformly worded `"Update failed: \(error)"` (T-650 phase 4). Previously the handler emitted three separate templates (`"Failed to save: ..."`, `"Failed to set milestone: ..."`, `"Failed to clear milestone: ..."`); none of these are referenced by existing tests, but agents matching the old strings will see new wording. Validation, identifier-resolution, and milestone error messages are unchanged. - `QueryTasksIntentTests` no longer fails to compile after T-1146 added a required `milestoneService:` argument to `QueryTasksIntent.execute`; the missing argument is now supplied at the one stale call site so the test target builds. - Milestone status updates are now idempotent: re-sending the current status no longer rewrites `lastStatusChangeDate` and `completionDate`, so old terminal milestones no longer re-enter the current report window via `ReportLogic`'s `completionDate ?? lastStatusChangeDate` fallback. Applied across `MilestoneService.updateStatus`, `UpdateMilestoneIntent.applyUpdate`, and `MCPToolHandler.applyMilestoneUpdate` (T-923). - `DisplayIDMaintenanceService.reassignDuplicates` now surfaces a fetch failure as a counter-advance warning on both record types instead of silently no-oping the run with empty arrays. - `DisplayIDMaintenanceService` group processing no longer fabricates winner identity (`UUID()` / `""`) for the unreachable empty-group case; the invariant is asserted via `preconditionFailure` and the existing `RecordRef` is propagated. - `IntentHelpers.resolveMilestone` now rejects malformed `milestoneId` and `projectId` with `INVALID_INPUT` instead of silently falling back to name-based lookup (T-753) - Sync toggle in Settings now updates `SyncManager` runtime state immediately instead of only taking effect on next launch (T-699)  ### Changed  - **`update_task` response shape changed (T-650)**. The new `IntentHelpers.taskUpdateResponseDict` builder (used by both the MCP tool and `UpdateTaskIntent`) no longer emits `comments`, `lastStatusChangeDate`, or `completionDate`, and now emits `description` and `metadata` only when non-empty (previously `description` was present as `null` and `metadata` was absent). The new shape is fixed by AC 9.1; clients parsing those removed keys from `update_task` responses must instead call `query_tasks` to fetch them. Identifier-resolution and error envelopes are unchanged. - `DisplayIDMaintenanceService` init no longer takes `taskCounterStore`/`milestoneCounterStore`; the stores are read from each `DisplayIDAllocator.counterStore` (single source of truth). - `IntentHelpers.encodeAsJSONString(_:)` is the shared JSON encoding helper used by `MCPToolHandler` and the maintenance App Intents, replacing three per-site copies of the same `JSONEncoder()` + UTF-8 guard. - `MCPToolHandler` now derives the gated maintenance tool name set from `MCPToolDefinitions.maintenanceToolNames` so the gate stays in sync with the definitions list. - Service-level error messages in `DisplayIDMaintenanceService` use `error.localizedDescription` rather than `"\(error)"` interpolation for more actionable output in the result envelope. - Decision 11 in the duplicate-displayid-cleanup decision log records the choice to use `FetchDescriptor` for the stale-ID guard (instead of `ModelContext.refresh(_:mergeChanges:)`) since the service hands `RecordRef` value types across the scan/reassign boundary rather than `@Model` references.  ### Added  - Effective-priority invariant cross-surface regression test (T-1463, Verification phase). `EffectivePriorityInvariantTests` creates a legacy task with `priorityRawValue = ""` (set directly, bypassing the `priority` setter) and asserts it reads/serializes as `medium` and is matched by a `medium` priority filter on all five read surfaces — board `DashboardLogic`, `MCPQueryFilters.matches`, `IntentHelpers.taskToDict`, `QueryTasksIntent` `applyFilters`, and `IntentHelpers.taskUpdateResponseDict` — plus the accessor itself. This is the guard against any surface reading `priorityRawValue` directly instead of the computed `priority` accessor and silently breaking Req 1.4. - Task priority in the app UI (T-1463, UI phase). The kanban dashboard gains a `PriorityFilterMenu` (multi-select high/medium/low, displayed high→medium→low, ephemeral state that resets on launch) wired into `DashboardView`'s filter row and `DashboardLogic` predicate, matching on the computed `task.priority` so legacy/unset tasks read as medium. Task cards show a new `PriorityIndicator` glyph for high/low only (medium suppressed, read from `TaskPriority.glyphSymbol`). The create sheet (`AddTaskSheet`) and edit screen (`TaskEditView`) get priority pickers (default medium / current value, both iOS and macOS layouts), and `TaskDetailView` shows a priority row. Adds `TaskPriority.displayOrder` (`[.high, .medium, .low]`) for consistent picker/filter ordering. `DashboardView`'s filter inputs were bundled into a `FilterSelection` value type as part of this. - Task priority across the MCP server and App Intents (T-1463, Automation phase). New dedicated `INVALID_PRIORITY` error code (its own `IntentError` case + hint, shared by both surfaces). `TaskUpdateValidator.validatePriority` mirrors `validateType` and threads an optional, non-clearable priority through `ValidatedTaskUpdate` (Decision 8). MCP `create_task` parses/defaults priority (invalid → error, no task created — Req 5.5) and echoes it; `query_tasks` filters by a multi-value `priority` array (mirrors `status`) via `MCPQueryFilters.priorities`; schemas updated in `MCPToolDefinitions`. App Intents: `CreateTaskIntent` validates optional-with-default priority (absent → medium, invalid → `INVALID_PRIORITY`) and echoes it in the response; `QueryTasksIntent` gains a scalar `priority` filter; `UpdateTaskIntent` sets priority via the validator. `IntentHelpers.taskToDict`/`taskUpdateResponseDict` (and the milestone task sub-dicts) serialize `task.priority.rawValue` — always the computed accessor, never `priorityRawValue`, upholding the effective-priority invariant (Req 1.4). The `createTaskIntentJsonContractRemainsCompatible` contract test was updated for the new `priority` key (existing keys unchanged, so callers ignoring new keys stay compatible). - Task priority foundation (T-1463, Foundation phase): new `TaskPriority` enum (low/medium/high) mirroring `TaskType` — `tintColor` for all three, `glyphSymbol` (`nil` only for medium, the single source of truth for "no card glyph"), and `accessibilityLabel`. `TransitTask` gains a CloudKit-safe `priorityRawValue` stored property (default `medium`, same shape as `statusRawValue`/`typeRawValue`) and a computed `priority` accessor that reads any absent/empty/unrecognized raw value as `.medium` (the effective-priority invariant, Req 1.4 — no migration needed). `TaskService.createTask` (both overloads) accepts a defaulted `priority`, and `updateTask` accepts an optional `priority` where omitting it leaves the value unchanged (Decision 8, non-clearable). Automation and UI wiring follow in later phases. - Spec for the task priority feature (T-1463): requirements, design, decision log, and a 20-task implementation plan written to `specs/task-priority/`, with `specs/OVERVIEW.md` updated. Plans a low/medium/high priority field (default medium) shown as a board-card glyph for high/low (medium suppressed), a board filter, in-app create/edit/detail editing, and MCP + App Intent read/set/filter support — using a default-value + computed-accessor backfill (no migration) and a dedicated `INVALID_PRIORITY` error code. Planning only; not yet implemented. - The dashboard now shows SwiftUI's `ContentUnavailableView.search` empty state ("No Results for '<query>'") when text search is the only active filter and no task matches (T-198). Project/type/milestone filters keep the existing "No matching tasks. Clear filters" message, and an empty database still shows "No tasks yet". The search bar stays visible so the query can be edited or cleared. The choice of empty state is a pure `DashboardLogic.emptyStateKind` function (empty-DB > search-only > generic-filter precedence) with unit-test coverage in `DashboardSearchTests` and UI coverage in `SearchEmptyStateUITests`. - Spec for adding a `ContentUnavailableView.search` empty state to the dashboard when text search is the only active filter and yields no matches (T-198). Includes a smolspec, decision log (search-only-vs-generic-filter precedence, empty-database precedence, and extracting empty-state selection into a pure `DashboardLogic.emptyStateKind` function for unit-testability), and a 4-task implementation plan with distributed testing. Deferred follow-up from the text-filter feature. - `docs/agent-notes/mcp-server.md` Tools Exposed table now describes the extended `update_task` (name / description / type / metadata + milestone) with a gotcha noting the omit-vs-clear distinction and the shared validator + response-builder pipeline both surfaces use (T-650 phase 7). - `UpdateTaskAllFieldsParityTests` (T-650 phase 6): 10 cross-surface parity cases (each new field individually, combined multi-field, no-op echo, milestone assign and clear) call both `MCPToolHandler.handleUpdateTask` and `UpdateTaskIntent.execute` against a shared task fixture and assert byte-equal JSON responses. Verifies AC 8.1 in practice. All cases passed on the first run with zero production diffs — both surfaces route through `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, so structural parity is a property of the architecture rather than an emergent coincidence. - `UpdateTaskIntent` (App Intent) now accepts the same `name` / `description` / `type` / `metadata` updates as the MCP tool (T-650 phase 5). `execute()` rewritten as a thin orchestrator over `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, mirroring `MCPToolHandler.handleUpdateTask` structurally so success responses are byte-equivalent across surfaces (AC 8.1). The old `applyMilestoneChange` and `buildResponse` helpers are deleted. The `@Parameter` description enumerates every supported field including the empty-string-clears and `{}`-clears semantics. A preflight identifier guard returns `INVALID_INPUT` ("Provide either displayId or taskId") for payloads with no `taskId`/`displayId` — previously these landed as a misleading `TASK_NOT_FOUND` from the downstream resolver, including the pre-existing failing test `noTaskIdentifierReturnsInvalidInput`, which now passes. - MCP `update_task` now accepts name, description (empty/whitespace clears), type, and metadata (`{}` clears) alongside the existing milestone fields (T-650 phase 4). `MCPToolHandler.handleUpdateTask` rewritten as a thin orchestrator over `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`. The two-stage do/catch (apply with explicit `taskService.rollback()`; save relying on `TaskService.save()`'s built-in `safeRollback`) keeps the task untouched when any field validation, apply, or save step fails — atomic per AC 5.1. Schema (`MCPToolDefinitions.updateTask`) extended with name/description/type/metadata properties; description and metadata prose include `"clear"` so clients know the clear semantics. Identifier-resolution preamble (T-634 / T-808) and milestone error literals are byte-equivalent to the pre-rewrite handler so existing tests pass unchanged. - `IntentHelpers.taskUpdateResponseDict` (T-650 phase 3): shared response builder for `update_task` across MCP and App Intent surfaces. Always emits `taskId`, `name`, `type`, `status`; emits `displayId`, `projectId`, `projectName`, `description`, `milestone` only when non-nil; emits `metadata` only when non-empty. Excludes `comments` and the date fields. 13 tests cover every conditional clause of AC 9.1. - `TaskUpdateValidator.validate` / `apply` / `strictStringMetadata` implementations (T-650 phase 2). `validate` parses raw MCP/Intent argument dicts into a `ValidatedTaskUpdate` covering name (trim + non-empty), description (trim, empty/whitespace clears), type (lowercase enum), metadata (strict-string, `{}` clears), and milestone (precedence `clearMilestone` → `milestoneId` → `milestoneDisplayId` → `milestone`). Milestone error literals are byte-identical to the existing MCP handler so Phase 3 wiring is drop-in. `apply` walks fields in deterministic order with `save: false`, throwing service-layer errors only — callers handle rollback via `TaskService.rollback()`. `TaskUpdateValidatorTests` covers 33 scenarios across all acceptance criteria 1–7. - `TaskUpdateValidator` scaffolding (T-650 phase 1): new `Transit/Transit/Intents/TaskUpdateValidator.swift` declares the `FieldChange<T>`, `MilestoneAction`, `ValidatedTaskUpdate`, and `TaskUpdateValidationError` value types plus stub `validate`/`apply`/`strictStringMetadata` signatures so subsequent phases can write tests against the contract. `TaskService.rollback()` wraps `modelContext.safeRollback()` for handler-level undo after a mid-update throw. - Spec for extending `update_task` MCP tool and `UpdateTaskIntent` to update name, description, type, and metadata atomically alongside the existing milestone fields (T-650). Includes requirements, design (shared `TaskUpdateValidator` with `FieldChange<T>`-based update model, new `IntentHelpers.taskUpdateResponseDict`, `TaskService.rollback` for apply-failure rollback), decision log, and a 12-task implementation plan across 7 phases. - Test for AC 8.1: an allocation failure on one duplicate group does not abort subsequent groups; the loser loop breaks for the failing group only and the next group still reassigns. - `DisplayIDMaintenanceService` is now constructed cross-platform in `TransitApp`, registered via `AppDependencyManager` for App Intents, and exposed via `.environment` on the root `NavigationStack` and `withCoreEnvironments` so `DataMaintenanceView` resolves it on iOS, the macOS Settings window, and the macOS Task Detail window. - `ScanDuplicateDisplayIDsIntent` and `ReassignDuplicateDisplayIDsIntent` App Intents for running duplicate cleanup from Shortcuts. Both reuse the same `JSONEncoder` path as the MCP tools so payloads are byte-equal across surfaces; service errors land inside the JSON envelope rather than thrown. - `DataMaintenanceView` in Settings — scan → confirm (destructive alert) → reassign → result flow, available on iOS and macOS. macOS Settings adds a "Data Maintenance" sidebar category and an "Expose maintenance tools" Toggle in the MCP Server section. - `NavigationDestination.dataMaintenance` case wires the new view into both the iOS settings stack and the macOS settings window. - UI test for the Data Maintenance golden path with seeded duplicates via a new `UITestScenario.duplicateDisplayIds` case; matched accessibility identifiers `dataMaintenance.scanButton/.reassignButton/.confirmButton/.resultList`. - MCP server exposes `scan_duplicate_display_ids` and `reassign_duplicate_display_ids` tools, gated behind a new `MCPSettings.maintenanceToolsEnabled` toggle (UserDefaults `mcpMaintenanceToolsEnabled`, default off). When the toggle is off, both tools are excluded from `tools/list` and `tools/call` returns JSON-RPC `methodNotFound` with a distinct "Tool '<name>' is disabled. Enable maintenance tools in Transit Settings." message. - `MCPToolDefinitions.tools(includingMaintenance:)` helper splits core tools from maintenance tools so the maintenance tools no longer cost MCP context for every agent session. - `DisplayIDAllocator.counterStore` accessor exposes the underlying `CounterStore` so callers needing direct counter access (e.g. `DisplayIDMaintenanceService`'s counter-advance fence) can share the allocator's store. - `DisplayIDMaintenanceService` with `scanDuplicates` and `reassignDuplicates` for repairing tasks/milestones sharing a `permanentDisplayId`. Counter advance happens before loser allocation; reassigned tasks receive a "Transit Maintenance" audit comment recording the old and new display IDs and the date. - `DisplayIDMaintenanceTypes`: `DuplicateReport`, `ReassignmentResult`, `FailureCode`, and Codable encoders matching the JSON shape shared by MCP and App Intents. - `CounterStore.advanceCounter(toAtLeast:retryLimit:)` extension with default implementation that retries on conflict and short-circuits when a racing writer has already moved the counter past the target. - Spec for duplicate display ID cleanup: requirements, design, decision log, and 18-task implementation plan in `specs/duplicate-displayid-cleanup/` - Task detail view shows creation date in secondary style on both iOS and macOS (T-755) - macOS: Task detail opens in a dedicated window instead of a sheet, with share and edit buttons in the toolbar (T-35) - macOS: New Task opens in a dedicated window instead of a sheet (T-35) - macOS: Edit Task replaces detail content in the same window instead of presenting a sheet (T-35) - macOS: `BoardBackground` applied to task detail, edit, and new task windows for consistent styling (T-35) - `TaskDetailWindowView` — macOS-only wrapper managing detail/edit state transitions within a window

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex de3b9be..751e15a 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -1,135 +1,146 @@ #if os(macOS) import Foundation import SwiftData  // swiftlint:disable file_length  @MainActor // swiftlint:disable:next type_body_length final class MCPToolHandler {      private let taskService: TaskService     private let taskFetcher: any TaskFetching     private let projectService: ProjectService     private let commentService: CommentService     private let milestoneService: MilestoneService     private let maintenanceService: DisplayIDMaintenanceService     private let settings: MCPSettings     private let persistence: PersistenceAvailability      /// Tools that only read.     private static let readOnlyToolNames: Set<String> = [         "query_tasks", "query_milestones", "get_projects", "scan_duplicate_display_ids"     ]      /// Tools blocked while fallback storage is active. Derived by subtracting the read-only     /// allow-list from the full tool list, so a newly added tool is treated as mutating     /// (fails safe) without a second edit here [T-1818].     private static let mutatingToolNames: Set<String> = Set(         MCPToolDefinitions.tools(includingMaintenance: true).map(\.name)     ).subtracting(readOnlyToolNames)      init(         taskService: TaskService,         projectService: ProjectService,         commentService: CommentService,         milestoneService: MilestoneService,         maintenanceService: DisplayIDMaintenanceService,         settings: MCPSettings,         persistence: PersistenceAvailability = .shared,         taskFetcher: (any TaskFetching)? = nil     ) {         self.taskService = taskService         self.taskFetcher = taskFetcher ?? taskService         self.projectService = projectService         self.commentService = commentService         self.milestoneService = milestoneService         self.maintenanceService = maintenanceService         self.settings = settings         self.persistence = persistence     }      // MARK: - JSON-RPC Dispatch      /// Returns `nil` for JSON-RPC notifications (no response required).     func handle(_ request: JSONRPCRequest) async -> JSONRPCResponse? {+        // MCP 2025-03-26 requires a present request id to be a string or+        // integer. `JSONRPCRequest` preserves presence separately because an+        // omitted id is a notification while an explicit null is invalid.+        guard request.isNotification || request.id != nil else {+            return JSONRPCResponse.error(+                id: nil,+                code: JSONRPCErrorCode.invalidRequest,+                message: "Invalid Request: id must be a string or integer"+            )+        }+         // JSON-RPC 2.0 §4.2/§5: reject non-"2.0" with -32600, even on notification-shaped envelopes (T-1106).         guard request.jsonrpc == "2.0" else {             return JSONRPCResponse.error(                 id: request.id,                 code: JSONRPCErrorCode.invalidRequest,                 message: "Invalid Request: jsonrpc must be \"2.0\""             )         }          let response: JSONRPCResponse         switch request.method {         case "initialize":             response = handleInitialize(id: request.id, params: request.params)         case "ping":             response = JSONRPCResponse.success(id: request.id, result: EmptyResult())         case "tools/list":             response = handleToolsList(id: request.id)         case "tools/call":             response = await handleToolCall(id: request.id, params: request.params)         default:             response = JSONRPCResponse.error(                 id: request.id,                 code: JSONRPCErrorCode.methodNotFound,                 message: "Unknown method: \(request.method)"             )         }          // Notifications are method calls whose results are intentionally         // discarded, not calls that should be skipped. This matters for         // state-changing tools/call notifications inside a JSON-RPC batch.         return request.isNotification ? nil : response     }      // MARK: - Initialize      private static let latestSupportedProtocolVersion = "2025-03-26"     private static let supportedProtocolVersions: Set<String> = [latestSupportedProtocolVersion]      private struct InvalidInitializeParams: Error {         let message: String     }      private func handleInitialize(id: JSONRPCId?, params: AnyCodable?) -> JSONRPCResponse {         guard let params else {             return invalidInitializeParams(id: id, message: "Missing initialize params")         }         guard let dict = params.value as? [String: Any] else {             return invalidInitializeParams(id: id, message: "Initialize params must be an object")         }          let requestedProtocolVersion: String         do {             requestedProtocolVersion = try initializeProtocolVersion(in: dict)             try validateInitializeCapabilities(in: dict)             try validateInitializeClientInfo(in: dict)         } catch {             return invalidInitializeParams(id: id, message: error.message)         }          let protocolVersion = Self.supportedProtocolVersions.contains(requestedProtocolVersion)             ? requestedProtocolVersion             : Self.latestSupportedProtocolVersion         let version = Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? "1.0"         let result = MCPInitializeResult(             protocolVersion: protocolVersion,             capabilities: MCPServerCapabilities(tools: MCPToolsCapability()),             serverInfo: MCPServerInfo(name: "transit", version: version)         )         return JSONRPCResponse.success(id: id, result: result)     }      private func initializeProtocolVersion(         in params: [String: Any]     ) throws(InvalidInitializeParams) -> String {         guard params["protocolVersion"] != nil else {             throw InvalidInitializeParams(message: "Missing protocolVersion")         }         guard let protocolVersion = params["protocolVersion"] as? String else {             throw InvalidInitializeParams(message: "protocolVersion must be a string")         }

diff --git a/Transit/Transit/MCP/MCPTypes.swift b/Transit/Transit/MCP/MCPTypes.swiftindex ee5297e..5e89769 100644--- a/Transit/Transit/MCP/MCPTypes.swift+++ b/Transit/Transit/MCP/MCPTypes.swift@@ -1,124 +1,126 @@ #if os(macOS) import Foundation  // All types in this file are `nonisolated` because they are pure data types // used across actor boundaries (NIO event loop ↔ MainActor).  // MARK: - JSON-RPC 2.0  nonisolated struct JSONRPCRequest: Decodable, Sendable {     let jsonrpc: String     let id: JSONRPCId?     let method: String     let params: AnyCodable?     /// True when the source JSON omitted the `id` member entirely (a JSON-RPC-    /// notification). An explicit `"id": null` is NOT a notification and must-    /// receive a response per JSON-RPC 2.0 §4.1.+    /// notification). An explicit `"id": null` is also not a notification,+    /// but MCP validation rejects it as an invalid request.     let isNotification: Bool      init(         jsonrpc: String,         id: JSONRPCId?,         method: String,         params: AnyCodable?,         isNotification: Bool = false     ) {         self.jsonrpc = jsonrpc         self.id = id         self.method = method         self.params = params         self.isNotification = isNotification     }      private enum CodingKeys: String, CodingKey {         case jsonrpc, id, method, params     }      init(from decoder: Decoder) throws {         let container = try decoder.container(keyedBy: CodingKeys.self)         // Lenient decode: missing/non-string `jsonrpc` becomes "" so the handler yields -32600, not -32700 (T-1106).         jsonrpc = (try? container.decodeIfPresent(String.self, forKey: .jsonrpc)) ?? ""         method = try container.decode(String.self, forKey: .method)         params = try container.decodeIfPresent(AnyCodable.self, forKey: .params)-        // Distinguish "id member absent" (notification) from "id present with-        // value null" (still a request, requires response with id: null).+        // Distinguish an omitted `id` member (notification) from an explicit+        // `null` value. MCP validates the latter as an invalid request in the+        // handler, so presence must remain available even though both values+        // map to a nil Swift ID.         if container.contains(.id) {             isNotification = false             id = try container.decodeIfPresent(JSONRPCId.self, forKey: .id)         } else {             isNotification = true             id = nil         }     } }  nonisolated struct JSONRPCResponse: Encodable, Sendable {     let jsonrpc: String = "2.0"     let id: JSONRPCId?     let result: AnyCodable?     let error: JSONRPCError?      static func success(id: JSONRPCId?, result: some Encodable) -> JSONRPCResponse {         JSONRPCResponse(id: id, result: AnyCodable(result), error: nil)     }      static func error(         id: JSONRPCId?,         code: Int,         message: String,         data: (some Encodable)? = nil as String?     ) -> JSONRPCResponse {         JSONRPCResponse(             id: id,             result: nil,             error: JSONRPCError(code: code, message: message, data: data.map { AnyCodable($0) })         )     }      private enum CodingKeys: String, CodingKey {         case jsonrpc, id, result, error     }      func encode(to encoder: Encoder) throws {         // Per JSON-RPC 2.0 §5, the `id` member MUST always be present in a         // response. Use a nil-to-null encoding rather than synthesized         // `encodeIfPresent`, which would omit the key entirely.         var container = encoder.container(keyedBy: CodingKeys.self)         try container.encode(jsonrpc, forKey: .jsonrpc)         if let id {             try container.encode(id, forKey: .id)         } else {             try container.encodeNil(forKey: .id)         }         try container.encodeIfPresent(result, forKey: .result)         try container.encodeIfPresent(error, forKey: .error)     } }  nonisolated struct JSONRPCError: Encodable, Sendable {     let code: Int     let message: String     let data: AnyCodable? }  // MARK: - JSON-RPC Error Codes  nonisolated enum JSONRPCErrorCode {     static let parseError = -32700     static let invalidRequest = -32600     static let methodNotFound = -32601     static let invalidParams = -32602     static let internalError = -32603 }  // MARK: - JSON-RPC ID (can be string or integer)  nonisolated enum JSONRPCId: Codable, Equatable, Sendable {     case string(String)     case integer(Int)      init(from decoder: Decoder) throws {         let container = try decoder.singleValueContainer()         if let intVal = try? container.decode(Int.self) {             self = .integer(intVal)             return

diff --git a/Transit/TransitTests/MCPNullIdTests.swift b/Transit/TransitTests/MCPNullIdTests.swiftindex 77cf6fa..11360df 100644--- a/Transit/TransitTests/MCPNullIdTests.swift+++ b/Transit/TransitTests/MCPNullIdTests.swift@@ -1,72 +1,126 @@ #if os(macOS) import Foundation import Testing @testable import Transit -/// Regression tests for T-847: JSON-RPC requests with explicit `null` id must-/// receive a response with `id: null`, while requests with the id member omitted-/// (notifications) must receive no response.+/// Regression tests for T-1863: MCP 2025-03-26 rejects an explicit JSON `null`+/// request id as Invalid Request. Requests with the id member omitted remain+/// notifications, while string and integer ids remain valid. @MainActor @Suite(.serialized) struct MCPNullIdTests { -    // MARK: - Decoding+    // MARK: - Decoding and presence -    @Test func decodingDistinguishesOmittedIdFromExplicitNull() throws {-        // Per JSON-RPC 2.0, a request without an `id` member is a notification.-        // A request with `"id": null` is NOT a notification — null is a valid id value.+    @Test func decodingPreservesOmittedIdAndExplicitNullPresence() throws {         let omittedJSON = Data(#"{"jsonrpc":"2.0","method":"ping"}"#.utf8)         let nullJSON = Data(#"{"jsonrpc":"2.0","id":null,"method":"ping"}"#.utf8)+        let stringJSON = Data(#"{"jsonrpc":"2.0","id":"request-1","method":"ping"}"#.utf8)+        let integerJSON = Data(#"{"jsonrpc":"2.0","id":1,"method":"ping"}"#.utf8)          let omitted = try JSONDecoder().decode(JSONRPCRequest.self, from: omittedJSON)         let explicitNull = try JSONDecoder().decode(JSONRPCRequest.self, from: nullJSON)--        #expect(omitted.isNotification, "Request with no id member must be treated as a notification")-        #expect(!explicitNull.isNotification, "Request with explicit null id is not a notification")+        let stringID = try JSONDecoder().decode(JSONRPCRequest.self, from: stringJSON)+        let integerID = try JSONDecoder().decode(JSONRPCRequest.self, from: integerJSON)++        #expect(omitted.isNotification, "An omitted id member must be a notification")+        #expect(!explicitNull.isNotification, "An explicit null id is not an omitted id")+        #expect(explicitNull.id == nil, "Explicit null must remain distinguishable by presence")+        #expect(stringID.id == .string("request-1"))+        #expect(integerID.id == .integer(1))     }      // MARK: - Handler behaviour -    @Test func handlerReturnsResponseForExplicitNullId() async throws {+    @Test func handlerRejectsExplicitNullIdAsInvalidRequest() async throws {         let env = try MCPTestHelpers.makeEnv()--        // Request with explicit null id should get a response.         let json = Data(#"{"jsonrpc":"2.0","id":null,"method":"ping"}"#.utf8)         let request = try JSONDecoder().decode(JSONRPCRequest.self, from: json) -        let response = await env.handler.handle(request)-        try #require(response, "Explicit null id must produce a JSON-RPC response")+        let response = try #require(await env.handler.handle(request))+        let error = try #require(response.error)+        #expect(error.code == JSONRPCErrorCode.invalidRequest)+        #expect(response.result == nil)+        #expect(response.id == nil, "Invalid requests use a JSON null response id")     }      @Test func handlerOmitsResponseForMissingId() async throws {         let env = try MCPTestHelpers.makeEnv()--        // Request with no id member is a notification — no response.         let json = Data(#"{"jsonrpc":"2.0","method":"ping"}"#.utf8)         let request = try JSONDecoder().decode(JSONRPCRequest.self, from: json)          let response = await env.handler.handle(request)         #expect(response == nil, "Notifications (omitted id) must not produce a response")     } -    // MARK: - Encoding--    @Test func responseToExplicitNullIdEncodesIdAsNull() async throws {+    @Test(arguments: [+        "initialize", "ping", "tools/list", "tools/call",+        "notifications/initialized", "unknown/method"+    ])+    func handlerRejectsExplicitNullBeforeDispatchingAnyMethod(method: String) async throws {         let env = try MCPTestHelpers.makeEnv()+        let json = Data(#"{"jsonrpc":"2.0","id":null,"method":"\#(method)"}"#.utf8)+        let request = try JSONDecoder().decode(JSONRPCRequest.self, from: json) -        let json = Data(#"{"jsonrpc":"2.0","id":null,"method":"ping"}"#.utf8)+        let response = try #require(await env.handler.handle(request))+        let error = try #require(response.error)+        #expect(error.code == JSONRPCErrorCode.invalidRequest)+        #expect(response.result == nil)+        #expect(response.id == nil)+    }++    @Test(arguments: [+        "initialize", "ping", "tools/list", "tools/call",+        "notifications/initialized", "unknown/method"+    ])+    func handlerOmitsResponseForMissingIdAcrossMethodPaths(method: String) async throws {+        let env = try MCPTestHelpers.makeEnv()+        let json = Data(#"{"jsonrpc":"2.0","method":"\#(method)"}"#.utf8)         let request = try JSONDecoder().decode(JSONRPCRequest.self, from: json) +        let response = await env.handler.handle(request)+        #expect(response == nil, "Notifications must not produce a response")+    }++    @Test func handlerReturnsResponsesForStringAndIntegerIds() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let stringRequest = try JSONDecoder().decode(+            JSONRPCRequest.self,+            from: Data(#"{"jsonrpc":"2.0","id":"request-1","method":"ping"}"#.utf8)+        )+        let integerRequest = try JSONDecoder().decode(+            JSONRPCRequest.self,+            from: Data(#"{"jsonrpc":"2.0","id":1,"method":"ping"}"#.utf8)+        )++        let stringResponse = try #require(await env.handler.handle(stringRequest))+        let integerResponse = try #require(await env.handler.handle(integerRequest))+        #expect(stringResponse.id == .string("request-1"))+        #expect(integerResponse.id == .integer(1))+        #expect(stringResponse.result != nil)+        #expect(integerResponse.result != nil)+    }++    // MARK: - Error envelope encoding++    @Test func invalidNullIdResponseEncodesErrorWithNullId() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let request = try JSONDecoder().decode(+            JSONRPCRequest.self,+            from: Data(#"{"jsonrpc":"2.0","id":null,"method":"ping"}"#.utf8)+        )+         let response = try #require(await env.handler.handle(request))         let data = try JSONEncoder().encode(response)-        let parsed = try JSONSerialization.jsonObject(-            with: data, options: [.fragmentsAllowed]-        ) as? [String: Any]-        let object = try #require(parsed)--        // The id key must be present and explicitly NSNull (not absent).-        #expect(object.keys.contains("id"), "Response must include the id field for null-id requests")-        #expect(object["id"] is NSNull, "id must be encoded as JSON null")+        let object = try #require(+            try JSONSerialization.jsonObject(with: data) as? [String: Any]+        )+        let error = try #require(object["error"] as? [String: Any])++        #expect(object.keys.contains("id"))+        #expect(object["id"] is NSNull)+        #expect(error["code"] as? Int == JSONRPCErrorCode.invalidRequest)+        #expect(object["result"] == nil)     } }  #endif

diff --git a/Transit/TransitTests/MCPServerBatchRequestTests.swift b/Transit/TransitTests/MCPServerBatchRequestTests.swiftindex c47c954..7e7439c 100644--- a/Transit/TransitTests/MCPServerBatchRequestTests.swift+++ b/Transit/TransitTests/MCPServerBatchRequestTests.swift@@ -29,160 +29,212 @@ struct MCPServerBatchRequestTests {         ]         """)          #expect(response.status == .ok)         #expect(response.contentType == "application/json")         let objects = try #require(response.json as? [[String: Any]])         #expect(objects.count == 2)         #expect(objects.contains { $0["id"] as? Int == 1 && $0["result"] != nil })         #expect(objects.contains { object in             object["id"] as? String == "missing"                 && (object["error"] as? [String: Any])?["code"] as? Int                     == JSONRPCErrorCode.methodNotFound         })     }      @Test func allNotificationBatchReturnsAcceptedWithNoBody() async throws {         let response = try await respond(body: """         [           {"jsonrpc":"2.0","method":"ping"},           {"jsonrpc":"2.0","method":"notifications/initialized"}         ]         """)          #expect(response.status == .accepted)         #expect(response.body.isEmpty)     }      @Test func toolCallNotificationIsDispatchedButGetsNoResponse() async throws {         let env = try MCPTestHelpers.makeEnv()         let project = MCPTestHelpers.makeProject(in: env.context)         let response = try await respond(handler: env.handler, body: """         [{           "jsonrpc":"2.0",           "method":"tools/call",           "params":{             "name":"create_task",             "arguments":{               "name":"Notification task",               "type":"bug",               "projectId":"\(project.id.uuidString)"             }           }         }]         """)          #expect(response.status == .accepted)         #expect(response.body.isEmpty)         let tasks = try env.context.fetch(FetchDescriptor<TransitTask>())         #expect(tasks.map(\.name) == ["Notification task"])     }      @Test func emptyBatchReturnsSingleInvalidRequestObject() async throws {         let response = try await respond(body: "[]")          #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.invalidRequest)         #expect(object["id"] is NSNull)     }      @Test func invalidBatchMembersEachProduceAnErrorResponse() async throws {         let response = try await respond(body: """         [           17,           {"jsonrpc":"2.0","id":2,"method":"ping"},           {"jsonrpc":"2.0","id":false,"method":"ping"}         ]         """)          #expect(response.status == .ok)         let objects = try #require(response.json as? [[String: Any]])         #expect(objects.count == 3)         #expect(objects.filter { object in             (object["error"] as? [String: Any])?["code"] as? Int                 == JSONRPCErrorCode.invalidRequest         }.count == 2)         #expect(objects.contains { $0["id"] as? Int == 2 && $0["result"] != nil })     } +    @Test func singleExplicitNullIdReturnsInvalidRequestWithNullId() async throws {+        let response = try await respond(+            body: #"{"jsonrpc":"2.0","id":null,"method":"ping"}"#+        )++        #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.invalidRequest)+        #expect(object["id"] is NSNull)+        #expect(object["result"] == nil)+    }++    @Test(arguments: [+        "initialize", "ping", "tools/list", "tools/call",+        "notifications/initialized", "unknown/method"+    ])+    func singleExplicitNullIdIsReturnedForEveryMethodPath(method: String) async throws {+        let response = try await respond(+            body: #"{"jsonrpc":"2.0","id":null,"method":"\#(method)"}"#+        )++        #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.invalidRequest)+        #expect(object["id"] is NSNull)+        #expect(object["result"] == nil)+    }++    @Test func batchExplicitNullIdIsInvalidWhileStringAndIntegerIdsRemainValid() async throws {+        let response = try await respond(body: """+        [+          {"jsonrpc":"2.0","id":null,"method":"ping"},+          {"jsonrpc":"2.0","id":7,"method":"ping"},+          {"jsonrpc":"2.0","id":"request-1","method":"ping"},+          {"jsonrpc":"2.0","method":"ping"}+        ]+        """)++        #expect(response.status == .ok)+        let objects = try #require(response.json as? [[String: Any]])+        #expect(objects.count == 3, "The omitted-id notification must have no response")++        let nullResponse = try #require(objects.first { $0["id"] is NSNull })+        let nullError = try #require(nullResponse["error"] as? [String: Any])+        #expect(nullError["code"] as? Int == JSONRPCErrorCode.invalidRequest)+        #expect(nullResponse["result"] == nil)+        #expect(objects.contains { $0["id"] as? Int == 7 && $0["result"] != nil })+        #expect(objects.contains { $0["id"] as? String == "request-1" && $0["result"] != nil })+    }+     @Test func initializeInsideBatchIsRejectedWithoutRejectingOtherRequests() async throws {         let response = try await respond(body: """         [           {"jsonrpc":"2.0","id":1,"method":"initialize"},           {"jsonrpc":"2.0","id":2,"method":"ping"}         ]         """)          #expect(response.status == .ok)         let objects = try #require(response.json as? [[String: Any]])         #expect(objects.count == 2)         let initializeResponse = try #require(objects.first { $0["id"] as? Int == 1 })         let error = try #require(initializeResponse["error"] as? [String: Any])         #expect(error["code"] as? Int == JSONRPCErrorCode.invalidRequest)         #expect(objects.contains { $0["id"] as? Int == 2 && $0["result"] != nil })     }      @Test func singleRequestStillReturnsAResponseObject() async throws {         let response = try await respond(             body: #"{"jsonrpc":"2.0","id":1,"method":"ping"}"#         )          #expect(response.status == .ok)         let object = try #require(response.json as? [String: Any])         #expect(object["id"] as? Int == 1)         #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 {         let env = try MCPTestHelpers.makeEnv()         return try await respond(handler: env.handler, body: body)     }      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         )

diff --git a/docs/agent-notes/mcp-server.md b/docs/agent-notes/mcp-server.mdindex 0dcb9f2..24e320f 100644--- a/docs/agent-notes/mcp-server.md+++ b/docs/agent-notes/mcp-server.md@@ -74,92 +74,92 @@ Gotchas:  - **Never reject a missing `Origin`.** Claude Code and CLI callers send no   `Origin` header at all; rejecting absence breaks the entire agent integration. - **Rejections are plain HTTP 403, not JSON-RPC errors.** The request never   entered a JSON-RPC session, and a `200` with an error object would confirm to   an attacker's page that the endpoint is live. The body is a fixed   `"Forbidden"` rather than the specific reason, so a prober cannot tell whether   the `Origin` or the `Host` check tripped. - **The `Host` header arrives as `request.head.authority`, not   `headerFields[.host]`.** Hummingbird's `HTTP1ToHTTPServerCodec` calls   `HTTPRequest(head, secure:splitCookie:)`, which lifts `Host` into the   authority pseudo-header. The `rawHTTP1*` tests in   `MCPServerOriginValidationTests` drive that exact conversion, so the mapping   is proven rather than assumed. - **Don't parse the origin with `URL`/`URLComponents`.** They are lenient by   design — `URL(string: "http://127.0.0.1@evil.example.com")?.host` is   `evil.example.com`. `MCPOriginValidator` parses by hand and fails closed. - **Host matching is exact.** `*.localhost` resolves to loopback on some   resolvers, so subdomains are not accepted. - Any new route added to `MCPServer` must repeat the check. If a second route   ever appears, promote it to a Hummingbird middleware instead. - `MCPSettings.validPortRange` is `nonisolated` so the validator (which runs on   NIO threads) can reuse it.  ## JSON-RPC Methods Handled  `initialize`, `notifications/initialized`, `ping`, `tools/list`, `tools/call`  ### Initialize handshake validation (T-1778)  `initialize` requires an object `params` containing string `protocolVersion`, object `capabilities`, and object `clientInfo`. `clientInfo.name` and `clientInfo.version` are required strings; optional `clientInfo.title` must also be a string. Missing fields and object/array method-parameter mismatches return JSON-RPC `invalidParams` (`-32602`); scalar or null top-level `params` violate the JSON-RPC request shape and return `invalidRequest` (`-32600`). Transit currently supports only `2025-03-26`: that version is echoed when requested, while any unsupported requested version receives the latest advertised supported version (`2025-03-26`) per the MCP lifecycle negotiation rule.  ## JSON-RPC Batch Envelopes (T-1834)  `POST /mcp` accepts either one `JSONRPCRequest` or a non-empty JSON-RPC batch. `MCPServer.decodeIncomingRequest` preserves invalid batch members so each produces its own `-32600` response, while an empty batch produces one standalone `-32600` object as required by JSON-RPC 2.0.  Batch dispatch is sequential on the MainActor to keep mutations against the shared SwiftData context deterministic. Responses to request members remain inside an array even when notifications are omitted. A batch containing only notifications returns HTTP 202 with no body; notifications still execute their method/tool path before the result is discarded. Batched `initialize` requests are rejected with `-32600` because MCP 2025-03-26 requires initialization to be a standalone request. Single requests continue to return one response object.  ## Dependencies  - **Hummingbird 2.x** — Transit's only *declared* external SPM dependency - Added via Xcode SPM integration in project.pbxproj - Also pulls in SwiftNIO and swift-service-lifecycle transitively - `MCPServer.swift` imports `ServiceLifecycle` directly (for `ServiceGroup`), so   that transitive module is effectively a source-level dependency even though it   is not declared as a package product on the target  ## Entitlements  Added `com.apple.security.network.server` and `com.apple.security.network.client` to `Transit.entitlements`.  ## Testing with curl  ```bash # List tools curl -X POST http://localhost:3141/mcp -H 'Content-Type: application/json' \   -d '{"jsonrpc":"2.0","id":1,"method":"tools/list"}'  # Create task curl -X POST http://localhost:3141/mcp -H 'Content-Type: application/json' \   -d '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"create_task","arguments":{"name":"Test","type":"feature"}}}' ```  ## Claude Code Integration  ```bash claude mcp add transit --transport http http://localhost:3141/mcp ```  ## Task Resolution  Task resolution (looking up a task by display ID or UUID) is centralised in `TaskService.resolveTask(from:)`: - `resolveTask(from identifier: String)` — accepts display ID as integer string or UUID string - `resolveTask(from dict: [String: Any])` — accepts dictionary with `displayId` or `taskId` keys, uses `IntentHelpers.parseIntValue` for numeric handling  MCPToolHandler, App Intents, and IntentHelpers all delegate to these methods rather than implementing their own resolution logic.  ## Gotchas  - `nonisolated` on struct/enum declarations is essential in this project due to `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor`. Without it, all types inherit MainActor isolation, breaking Codable conformance on NIO threads.-- `JSONRPCRequest.id` currently cannot distinguish an omitted id from an explicit JSON `null` id because both decode to `nil`. `MCPToolHandler.handle(_:)` uses `id == nil` to detect notifications, so explicit-null requests are dropped without a response. T-847 tracks preserving id presence separately from id value.+- `JSONRPCRequest` preserves the distinction between an omitted `id` member and an explicit JSON `null` using `isNotification`. MCP 2025-03-26 requires present IDs to be strings or integers, so `MCPToolHandler.handle(_:)` rejects the `id == nil && !isNotification` state with `-32600 Invalid Request`; omitted-id notifications still execute and return no response. T-847 established the presence flag, but its expectation that explicit null should be accepted is superseded by T-1863. - `ByteBuffer(data:)` requires explicit `import NIOFoundationCompat` — not available from just `import Hummingbird`. - Don't reference `self` in `Task.detached` closures on `MCPServer` — causes "sending 'self' risks data races" error. Capture dependencies explicitly. - **Name-based filters must handle cross-project duplicates.** Milestone names (and potentially other name-resolved entities) can be duplicated across projects. When filtering by name without a project scope, use `Set<UUID>` to collect all matching IDs rather than taking just the first match (T-292). - **Multi-field updates must be atomic.** When a handler updates multiple fields (e.g., status + name), validate all inputs before mutating any model state, then save once. Don't call separate service methods that each `save()` independently — a failure in the second leaves the first already persisted (T-391). - **Always pass `save: false` when calling service methods from handlers.** Service methods like `setMilestone` default to `save: true` for standalone use, but handlers must defer saving to a single atomic `save()` at the end. Always add `safeRollback()` in the catch block of that save (T-531). - **Do not add compensating unescape/transform logic for JSON-RPC input.** JSON-RPC transport handles string encoding correctly: `\n` in JSON decodes to a real newline, `\\n` decodes to literal backslash-n. A previous `unescapeNewlines` helper that globally replaced `\n` with real newlines corrupted legitimate content (T-576 reverted T-561). Pass MCP string arguments through to the service layer unmodified. - **Always validate `displayId` and `milestoneDisplayId` before use.** When a handler accepts an integer ID from args, check `args["key"] != nil` first, then `IntentHelpers.parseIntValue(args["key"])`. If the key is present but the value is not a valid integer (string, float), return an error immediately — never silently fall through to broader queries or alternate resolution paths. Pattern established in T-613 (milestoneDisplayId) and T-634 (displayId across all handlers). - **Always validate UUID filter parameters separately from presence checks.** Never use `flatMap(UUID.init)` or combined conditionals like `if let str = ..., let uuid = UUID(str)` for user-provided filter values — these silently drop invalid inputs. Instead, first check if the key is present, then validate the UUID format with a guard, returning an explicit error on failure. See `handleQueryTasks` for the correct pattern; `handleQueryMilestones` uses `resolveProjectFilter` helper (T-665). This applies to both query and create flows — T-743 fixed the same pattern in `handleCreateTask`, `handleCreateMilestone`, `CreateTaskIntent`, and `CreateMilestoneIntent`. The `resolveMilestone` helper also follows this pattern for both `displayId` (T-634) and `milestoneId` (T-769). - **Always validate enum filter values in query handlers.** When a handler accepts `status`, `not_status`, or `type` as filter parameters, validate each value against the enum's `allCases` before using them. Invalid values must return `isError: true` with a message listing valid options — never silently filter to empty results. Helper: `validateEnumFilter(_:key:type:)` in `MCPToolHandler` — generic over any `RawRepresentable & CaseIterable` enum with `String` raw values (T-732). - **Reuse IntentHelpers instead of implementing private handler-local helpers.** `IntentHelpers` provides shared utilities for metadata extraction, JSON parsing, task/milestone resolution, and error mapping. Don't duplicate this logic in MCPToolHandler — use the shared version to ensure consistent behavior across MCP and App Intent paths (T-723). - **`update_task` distinguishes "omit" from "clear".** Omitting a field leaves the stored value untouched; passing `description: ""` or whitespace-only clears the description, and passing `metadata: {}` clears all metadata. An identifier-only request (no mutating field) is a no-op echo — the task is returned without a save. Both `update_task` and `UpdateTaskIntent` route through `TaskUpdateValidator` + `IntentHelpers.taskUpdateResponseDict`, so success payloads are equivalent across surfaces (enforced by `UpdateTaskAllFieldsParityTests`).

diff --git a/specs/bugfixes/mcp-rejects-explicit-null-request-ids/report.md b/specs/bugfixes/mcp-rejects-explicit-null-request-ids/report.mdnew file mode 100644index 0000000..6def343--- /dev/null+++ b/specs/bugfixes/mcp-rejects-explicit-null-request-ids/report.md@@ -0,0 +1,109 @@+# Bugfix Report: MCP Rejects Explicit Null Request IDs++**Date:** 2026-08-02+**Status:** Fixed+**Transit ticket:** T-1863++## Description of the Issue++Transit advertises MCP protocol `2025-03-26`, which requires a JSON-RPC request ID to be a string or integer when present; an explicit `"id": null` is invalid. Transit accepted an explicit null ID as a normal request because its decoder correctly distinguished ID presence, but the handler did not reject the present-but-null state.++**Reproduction steps:**+1. Enable Transit's MCP server.+2. POST `{ "jsonrpc": "2.0", "id": null, "method": "ping" }` to `/mcp`.+3. Before the fix, observe a successful ping response with `id: null` instead of JSON-RPC `-32600 Invalid Request`.++**Impact:** MCP clients sending malformed request envelopes received a positive acknowledgement. The same protocol defect affected single requests and batch members, while notification requests with an omitted ID had to continue executing without a response.++## Investigation Summary++The request model, handler, HTTP route, batch dispatcher, existing T-847 tests, T-847 history, MCP notes, and decode-classification regressions were inspected.++- **Symptoms examined:** Explicit null IDs reached method dispatch and returned successful results; omitted IDs were intentionally suppressed; string and integer IDs worked; batches preserved invalid members and response arrays.+- **Code inspected:** `MCPTypes.swift` (`JSONRPCRequest` presence tracking and response encoding), `MCPToolHandler.swift` (`handle` notification suppression), `MCPServer.swift` (single and batch routing), `MCPServerDecodeTypes.swift`, `MCPNullIdTests.swift`, and `MCPServerBatchRequestTests.swift`.+- **Hypotheses tested:** The decoder losing ID presence was ruled out: `container.contains(.id)` already sets `isNotification` correctly. Batch handling was also ruled out as the primary cause: it delegates request members to the same handler and preserves null-ID error envelopes. T-847's generic JSON-RPC expectation that explicit null should receive a response was obsolete for this MCP endpoint because MCP 2025-03-26 forbids null request IDs.++## Discovered Root Cause++`JSONRPCRequest` represents an explicit null ID as `id == nil` plus `isNotification == false`, which is sufficient to preserve presence. `MCPToolHandler.handle` only checked `request.id` indirectly through the final notification suppression and therefore dispatched the present-but-null request as a normal method call.++**Defect type:** Missing protocol validation at the handler boundary.++**Why it occurred:** T-847 intentionally fixed the ambiguity between omitted IDs and explicit null IDs according to generic JSON-RPC semantics. The later MCP protocol revision is stricter, but no MCP-specific validation was added to the now-preserved explicit-null state, and the T-847 regressions encoded the old expectation as successful behavior.++**Contributing factors:** The response encoder correctly emits a null `id` for error envelopes, and batch handling already has the required per-member response structure, so the defect was easy to miss when inspecting only transport serialization.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/MCP/MCPToolHandler.swift` - Reject the presence-aware `id == nil && !isNotification` state with JSON-RPC `-32600 Invalid Request` and a null response ID before method dispatch. Omitted-ID notifications still execute and return no response; string and integer IDs are unchanged.+- `Transit/Transit/MCP/MCPTypes.swift` - Clarify that decoding preserves ID presence for MCP-layer validation rather than treating explicit null as a valid request ID.+- `Transit/TransitTests/MCPNullIdTests.swift` - Replace obsolete T-847 success expectations with focused presence, handler, valid-ID, notification, error-code, and response-envelope regressions.+- `Transit/TransitTests/MCPServerBatchRequestTests.swift` - Verify single-request and batch HTTP behavior, including per-member null-ID errors and unchanged string/integer responses.+- `docs/agent-notes/mcp-server.md` - Replace the stale T-847 gotcha with the current presence-aware MCP validation rule.+- `CHANGELOG.md` - Record the protocol correction under Unreleased fixes.++**Approach rationale:** The decoder already preserves the only information needed to distinguish an omitted ID from explicit null, and the handler is the existing JSON-RPC protocol-validation boundary for other request-shape rules. Validating there avoids changing the batch decoder's per-member preservation behavior and keeps the existing `JSONRPCResponse` encoder responsible for the required `id: null` error envelope.++**Alternatives considered:**+- Treat explicit null as a notification - rejected because it would silently discard an invalid request and violate MCP's request-ID rule.+- Remove the `isNotification` flag or make the decoder throw for explicit null - rejected because omitted-ID notifications need the presence bit, and the handler already centralizes JSON-RPC/MCP request validation while batch decoding must preserve members for per-member errors.+- Return a successful response with `id: null` as T-847 required - rejected because that generic JSON-RPC behavior conflicts with the MCP 2025-03-26 contract advertised by Transit.++## Regression Test++**Test files:**+- `Transit/TransitTests/MCPNullIdTests.swift`+- `Transit/TransitTests/MCPServerBatchRequestTests.swift`++**Tests:**+- `decodingPreservesOmittedIdAndExplicitNullPresence`+- `handlerRejectsExplicitNullIdAsInvalidRequest`+- `invalidNullIdResponseEncodesErrorWithNullId`+- `handlerOmitsResponseForMissingId`+- `handlerReturnsResponsesForStringAndIntegerIds`+- `singleExplicitNullIdReturnsInvalidRequestWithNullId`+- `batchExplicitNullIdIsInvalidWhileStringAndIntegerIdsRemainValid`++**What they verify:** Presence-aware decoding distinguishes omitted IDs from explicit null IDs; explicit null is rejected with `-32600` and an explicitly encoded JSON null response ID; notifications remain response-free; string and integer IDs remain successful; and the same semantics hold for HTTP single requests and JSON-RPC batches.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/MCP/MCPToolHandler.swift` | Reject explicit null request IDs as invalid MCP requests. |+| `Transit/Transit/MCP/MCPTypes.swift` | Document presence-aware ID decoding. |+| `Transit/TransitTests/MCPNullIdTests.swift` | Replace obsolete T-847 expectations and add focused protocol regressions. |+| `Transit/TransitTests/MCPServerBatchRequestTests.swift` | Add single-route and batch-level null-ID response regressions. |+| `docs/agent-notes/mcp-server.md` | Document the current MCP request-ID contract and T-847 supersession. |+| `specs/bugfixes/mcp-rejects-explicit-null-request-ids/report.md` | Investigation and resolution record. |+| `CHANGELOG.md` | Record the MCP protocol correction. |++## Verification++**Automated:**+- [x] Focused regression suites pass: `MCPNullIdTests` and `MCPServerBatchRequestTests` via macOS `xcodebuild test`.+- [x] Full macOS unit suite passes: `make test-quick`.+- [x] Linters/validators pass: `make lint` (including the model-container ownership guard).+- [x] macOS build passes: `make build-macos`.+- [~] `make test` completed after a warmed-cache retry with 1,198 passed and 3 pre-existing UI failures (`testClearAll`, `testEditViewPreservesTaskMilestone`, and `testDataMaintenanceGoldenPath`); none exercise MCP request handling. The first attempt timed out during initial iOS dependency compilation.+- [~] `make test-ui` completed with 18 passed and the same 3 unrelated UI failures. Xcode also emitted repeated `DebuggerLLDB.DebuggerVersionStore.StoreError` warnings during the run.++**Manual verification:**+- [x] Compared single, notification, valid-ID, and batch response envelopes against the MCP 2025-03-26 request-ID rule.+- [x] Confirmed the invalid response envelope retains an explicit `id: null`, while notification-only batches remain HTTP 202 with no body.++## Prevention++- Keep protocol-version-specific validation separate from generic JSON-RPC decoding assumptions.+- Preserve explicit field presence in decoders whenever omitted and null have different protocol meanings.+- Retain route-level single and batch regressions for every request-envelope rule.+- When a protocol revision supersedes an older ticket's behavior, update both the regression expectations and the related implementation notes.++## Related++- Transit ticket T-1863+- Supersedes the generic JSON-RPC expectation implemented by T-847 (PR #121).+- MCP 2025-03-26 basic protocol: https://modelcontextprotocol.io/specification/2025-03-26/basic/