Pre-push review: T-1991 metadata shape validation

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex b48b7bc..b974125 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -1,89 +1,90 @@ # 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). - 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/Intents/CreateTaskIntent.swift b/Transit/Transit/Intents/CreateTaskIntent.swiftindex 0c2ff5e..848b0d9 100644--- a/Transit/Transit/Intents/CreateTaskIntent.swift+++ b/Transit/Transit/Intents/CreateTaskIntent.swift@@ -237,83 +237,88 @@ struct CreateTaskIntent: AppIntent {         in project: Project,         using milestoneService: MilestoneService     ) -> (milestone: Milestone?, error: String?) {         do {             let milestone = try milestoneService.findByDisplayID(displayId)             guard milestone.project?.id == project.id else {                 return (nil, IntentHelpers.mapMilestoneError(.projectMismatch).json)             }             return (milestone, nil)         } catch let error as MilestoneService.Error {             return (nil, IntentHelpers.mapMilestoneError(error).json)         } catch {             return (nil, IntentError.milestoneNotFound(                 hint: "No milestone with displayId \(displayId)"             ).json)         }     }      @MainActor     private static func resolveMilestone(         named name: String,         in project: Project,         using milestoneService: MilestoneService     ) -> (milestone: Milestone?, error: String?) {         do {             guard let milestone = try milestoneService.findByName(name, in: project) else {                 return (nil, IntentError.milestoneNotFound(                     hint: "No milestone named '\(name)' in project '\(project.name)'"                 ).json)             }             return (milestone, nil)         } catch MilestoneService.Error.ambiguousName {             return (nil, IntentError.ambiguousMilestone(                 hint: "Multiple milestones named '\(name)' exist in project '\(project.name)'"             ).json)         } catch {             return (nil, IntentError.internalError(                 hint: "Failed to look up milestone: \(error)"             ).json)         }     }      /// Parses the optional `priority` field. Absent -> `.medium`;     /// present-but-non-string -> INVALID_INPUT; present-but-unknown -> INVALID_PRIORITY     /// (Decision 9). Priority is non-clearable, so there is no "absent = clear" case.     private static func parsePriority(_ json: [String: Any]) -> Result<TaskPriority, IntentError> {         guard let raw = json["priority"] else { return .success(.medium) }         guard let str = raw as? String else {             return .failure(.invalidInput(hint: "priority must be a string"))         }         guard let priority = TaskPriority(rawValue: str) else {             return .failure(.invalidPriority(hint: "Unknown priority: \(str)"))         }         return .success(priority)     }      private static func validateInput(_ json: [String: Any]) -> IntentError? {         guard json["name"] != nil else {             return .invalidInput(hint: "Missing required field: name")         }         guard let name = json["name"] as? String else {             return .invalidInput(hint: "name must be a string")         }         guard !name.isEmpty else {             return .invalidInput(hint: "Missing required field: name")         }         guard json["type"] != nil else {             return .invalidInput(hint: "Missing required field: type")         }         guard let typeString = json["type"] as? String else {             return .invalidInput(hint: "type must be a string")         }         guard TaskType(rawValue: typeString) != nil else {             return .invalidType(hint: "Unknown type: \(typeString)")         }         // Reject non-string description: as? String silently drops         // present-but-wrong-type values, making a malformed request look successful. [T-1192]         if let rawDescription = json["description"], rawDescription as? String == nil {             return .invalidInput(hint: "description must be a string")         }+        // A present metadata value must be an object. Keep the distinction from omission so+        // malformed values cannot be silently converted to nil by stringMetadata [T-1991].+        if let rawMetadata = json["metadata"], !IntentHelpers.isMetadataObject(rawMetadata) {+            return .invalidInput(hint: "metadata must be an object")+        }         return nil     } }

diff --git a/Transit/Transit/Intents/IntentHelpers.swift b/Transit/Transit/Intents/IntentHelpers.swiftindex 8af8d7c..300b357 100644--- a/Transit/Transit/Intents/IntentHelpers.swift+++ b/Transit/Transit/Intents/IntentHelpers.swift@@ -1,131 +1,138 @@ import CoreFoundation import Foundation  // swiftlint:disable type_body_length file_length /// Shared utilities for App Intent JSON parsing and response encoding. /// Nonisolated because these are pure functions that only use Foundation types. nonisolated enum IntentHelpers {      /// Extracts an integer from a value that may be `Int`, `Double`, or `NSNumber`.     /// JSONSerialization and MCP argument passing may deliver JSON integers as `Double`.     /// Returns `nil` for non-numeric or non-integral values (e.g. 42.5).     ///     /// JSON booleans are rejected explicitly. `JSONSerialization` delivers `true`/`false`     /// as `NSNumber` wrapping a `CFBoolean`, which would otherwise satisfy `as? Int`     /// (returning `1`/`0`) and silently target T-1/M-1 instead of being rejected. [T-1211]     static func parseIntValue(_ value: Any?) -> Int? {         guard let value else { return nil }         if let number = value as? NSNumber, CFGetTypeID(number) == CFBooleanGetTypeID() {             return nil         }         if let intVal = value as? Int { return intVal }         if let doubleVal = value as? Double { return Int(exactly: doubleVal) }         return nil     }      /// Extracts a strict boolean from a JSON value. Returns nil for any non-boolean     /// value, including numeric `1`/`0`, strings, and null. Distinguishes JSON     /// booleans from numeric NSNumbers — `JSONSerialization` delivers both as     /// `NSNumber`, so a plain `as? Bool` would silently accept `1`/`0`. [T-1060]     static func parseBoolValue(_ value: Any?) -> Bool? {         guard let value else { return nil }         // Native Swift Bool — already strictly typed.         if let nativeBool = value as? Bool, !(value is NSNumber) {             return nativeBool         }         // NSNumber: only accept if it actually wraps a CFBoolean (true/false literal).         if let number = value as? NSNumber, CFGetTypeID(number) == CFBooleanGetTypeID() {             return number.boolValue         }         return nil     }      /// Parses a JSON string into a dictionary. Returns nil for malformed input.     static func parseJSON(_ input: String) -> [String: Any]? {         guard let data = input.data(using: .utf8),               let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else {             return nil         }         return json     } +    /// Returns whether a present metadata value has the required JSON object shape.+    /// This intentionally validates only the container; values inside a valid object continue to+    /// follow the tolerant T-723 contract implemented by `stringMetadata(from:)`.+    static func isMetadataObject(_ value: Any?) -> Bool {+        value is [String: Any] || value is [String: String]+    }+     /// Extracts string metadata entries from a JSON object value.     /// Non-string values are ignored because task metadata is stored as `[String: String]`.     static func stringMetadata(from value: Any?) -> [String: String]? {         // Fast path for native callers/tests that already provide the expected metadata type.         if let metadata = value as? [String: String], !metadata.isEmpty {             return metadata         }         guard let dict = value as? [String: Any], !dict.isEmpty else {             return nil         }         let metadata = dict.reduce(into: [String: String]()) { result, pair in             guard let stringValue = pair.value as? String else { return }             result[pair.key] = stringValue         }         return metadata.isEmpty ? nil : metadata     }      /// Encodes a dictionary as a JSON string. Returns an error JSON on failure.     static func encodeJSON(_ dict: [String: Any]) -> String {         guard let data = try? JSONSerialization.data(withJSONObject: dict),               let string = String(data: data, encoding: .utf8) else {             return IntentError.invalidInput(hint: "Failed to encode response").json         }         return string     }      /// Encodes an array of dictionaries as a JSON string.     static func encodeJSONArray(_ array: [[String: Any]]) -> String {         guard let data = try? JSONSerialization.data(withJSONObject: array),               let string = String(data: data, encoding: .utf8) else {             return "[]"         }         return string     }      enum EncodingError: Swift.Error { case utf8 }      /// Encodes an `Encodable` value as a UTF-8 JSON string. Used by both the MCP     /// dispatch handler and the maintenance Intents so they share a byte-equal     /// encoding for the same input.     static func encodeAsJSONString(_ value: some Encodable) throws -> String {         let data = try JSONEncoder().encode(value)         guard let text = String(data: data, encoding: .utf8) else {             throw EncodingError.utf8         }         return text     }      /// The JSON error body every mutating App Intent returns while Transit is running on the     /// in-memory fallback container, or `nil` when writes are durable.     ///     /// INTERNAL_ERROR is the documented code for "the request was well-formed but Transit could     /// not carry it out", which is exactly this case — the caller did nothing wrong, the store     /// did. Reusing it keeps the error vocabulary unchanged for existing CLI callers [T-1836].     @MainActor     static func fallbackStorageErrorJSON(_ persistence: PersistenceAvailability) -> String? {         guard persistence.isFallbackStorageActive else { return nil }         return IntentError.internalError(hint: PersistenceAvailability.unavailableHint).json     }      /// Translates ProjectLookupError to IntentError.     static func mapProjectLookupError(_ error: ProjectLookupError) -> IntentError {         switch error {         case .notFound(let hint):             .projectNotFound(hint: hint)         case .ambiguous(let hint):             .ambiguousProject(hint: hint)         case .storageFailure(let hint):             .internalError(hint: hint)         case .noIdentifier:             .invalidInput(hint: "Either projectId or project name is required")         }     }      /// Translates MilestoneService.Error to IntentError.     static func mapMilestoneError(_ error: MilestoneService.Error) -> IntentError {         switch error {         case .invalidName:             .invalidInput(hint: "Milestone name cannot be empty")         case .milestoneNotFound:

diff --git a/Transit/Transit/MCP/MCPToolHandler.swift b/Transit/Transit/MCP/MCPToolHandler.swiftindex e7d6df6..de3b9be 100644--- a/Transit/Transit/MCP/MCPToolHandler.swift+++ b/Transit/Transit/MCP/MCPToolHandler.swift@@ -232,160 +232,165 @@ final class MCPToolHandler {                 id: id, result: errorResult(PersistenceAvailability.unavailableHint)             )         }          let result: MCPToolResult         switch name {         case "create_task":             result = await handleCreateTask(arguments)         case "update_task_status":             result = handleUpdateStatus(arguments)         case "query_tasks":             result = handleQueryTasks(arguments)         case "add_comment":             result = handleAddComment(arguments)         case "get_projects":             result = handleGetProjects()         case "create_milestone":             result = await handleCreateMilestone(arguments)         case "query_milestones":             result = handleQueryMilestones(arguments)         case "update_milestone":             result = handleUpdateMilestone(arguments)         case "delete_milestone":             result = handleDeleteMilestone(arguments)         case "update_task":             result = handleUpdateTask(arguments)         case "scan_duplicate_display_ids":             result = handleScanDuplicateDisplayIds()         case "reassign_duplicate_display_ids":             result = await handleReassignDuplicateDisplayIds()         default:             return JSONRPCResponse.error(                 id: id,                 code: JSONRPCErrorCode.invalidParams,                 message: "Unknown tool: \(name)"             )         }          return JSONRPCResponse.success(id: id, result: result)     }      // MARK: - Maintenance Dispatch      private func handleScanDuplicateDisplayIds() -> MCPToolResult {         do {             let report = try maintenanceService.scanDuplicates()             return textResult(try IntentHelpers.encodeAsJSONString(report))         } catch {             return errorResult("Failed to scan duplicates: \(error.localizedDescription)")         }     }      private func handleReassignDuplicateDisplayIds() async -> MCPToolResult {         let result = await maintenanceService.reassignDuplicates()         do {             return textResult(try IntentHelpers.encodeAsJSONString(result))         } catch {             return errorResult("Failed to encode reassignment result: \(error.localizedDescription)")         }     }      // MARK: - create_task      // swiftlint:disable:next cyclomatic_complexity function_body_length     private func handleCreateTask(_ args: [String: Any]) async -> MCPToolResult {         let name: String         switch requiredString(args, key: "name") {         case .success(let value): name = value         case .failure(.message(let message)): return errorResult(message)         }         guard args["type"] != nil else {             return errorResult("Missing required argument: type")         }         guard let typeRaw = args["type"] as? String else {             return errorResult("type must be a string")         }         guard let taskType = TaskType(rawValue: typeRaw) else {             let valid = TaskType.allCases.map(\.rawValue).joined(separator: ", ")             return errorResult("Invalid type: \(typeRaw). Must be one of: \(valid)")         }+        // A present metadata value must be an object. Keep the distinction from omission so+        // malformed values cannot be silently converted to nil by stringMetadata [T-1991].+        if let rawMetadata = args["metadata"], !IntentHelpers.isMetadataObject(rawMetadata) {+            return errorResult("metadata must be an object")+        }          // Priority is optional and defaults to medium. A present-but-invalid value         // is rejected before any task is created (Req 5.5).         let priority: TaskPriority         if let priorityRaw = args["priority"] {             guard let priorityStr = priorityRaw as? String else {                 return errorResult("priority must be a string")             }             guard let parsed = TaskPriority(rawValue: priorityStr) else {                 let valid = TaskPriority.allCases.map(\.rawValue).joined(separator: ", ")                 return errorResult("Invalid priority: \(priorityStr). Must be one of: \(valid)")             }             priority = parsed         } else {             priority = .medium         }          // Reject malformed or non-string projectId when the key is present         // [T-743, T-788].         let projectId: UUID?         switch parseProjectIdArgument(args) {         case .failure(.message(let message)): return errorResult(message)         case .success(let parsed): projectId = parsed         }         // Reject a present non-string `project` when projectId is absent [T-1453].         // Without this guard `as? String` silently drops the malformed value and the         // request falls through to the generic missing-project error instead of         // surfacing the type mismatch. projectId-takes-precedence is preserved: when a         // valid projectId is present the `project` name is ignored regardless of type.         if projectId == nil, let rawProject = args["project"], !(rawProject is String) {             return errorResult("project must be a string")         }         let projectName = args["project"] as? String         let project: Project         switch projectService.findProject(id: projectId, name: projectName) {         case .success(let found):             project = found         case .failure(let error):             return errorResult(IntentHelpers.mapProjectLookupError(error).hint)         }          // Pre-validate milestone before creating the task to avoid orphans.         // Reject non-integer milestoneDisplayId when key is present [T-613]         if args["milestoneDisplayId"] != nil, IntentHelpers.parseIntValue(args["milestoneDisplayId"]) == nil {             return errorResult("milestoneDisplayId must be an integer")         }         var resolvedMilestone: Milestone?         if let milestoneDisplayId = IntentHelpers.parseIntValue(args["milestoneDisplayId"]) {             do {                 resolvedMilestone = try milestoneService.findByDisplayID(milestoneDisplayId)             } catch MilestoneService.Error.milestoneNotFound {                 return errorResult("No milestone with displayId \(milestoneDisplayId)")             } catch MilestoneService.Error.duplicateDisplayID {                 return errorResult("Duplicate milestone identifier detected for displayId \(milestoneDisplayId)")             } catch {                 return errorResult("Failed to find milestone: \(error)")             }             guard let milestoneProject = resolvedMilestone?.project,                   milestoneProject.id == project.id else {                 return errorResult("Milestone and task must belong to the same project")             }         } else if args["milestone"] != nil {             // Reject non-string milestone values [T-1114]. Without this guard             // a numeric or boolean milestone arg would fall through to the             // "absent" branch and the task would be created without the             // requested assignment.             guard let milestoneName = args["milestone"] as? String else {                 return errorResult("milestone must be a string")             }             do {                 guard let milestone = try milestoneService.findByName(milestoneName, in: project) else {                     return errorResult("No milestone named '\(milestoneName)' in project '\(project.name)'")                 }                 resolvedMilestone = milestone             } catch MilestoneService.Error.ambiguousName {                 return errorResult(                     "Multiple milestones named '\(milestoneName)' exist in project '\(project.name)'"                 )             } catch {                 return errorResult("Failed to look up milestone: \(error)")

diff --git a/Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swift b/Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swiftnew file mode 100644index 0000000..20148f9--- /dev/null+++ b/Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swift@@ -0,0 +1,134 @@+import Foundation+import SwiftData+import Testing+@testable import Transit++/// Regression coverage for T-1991: a present `metadata` field must be a JSON+/// object. Non-object values must be rejected before task insertion instead of+/// being silently treated as omitted metadata.+@MainActor @Suite(.serialized)+struct CreateTaskMetadataShapeValidationTests {++    private struct IntentServices {+        let task: TaskService+        let project: ProjectService+        let context: ModelContext+        let projectModel: Project+    }++    private let nonObjectValues: [(label: String, value: Any)] = [+        ("string", "owner=sam"),+        ("array", ["owner=sam"]),+        ("number", 42),+        ("boolean", true),+        ("null", NSNull())+    ]++    private func makeIntentServices() throws -> IntentServices {+        let testContainer = try TestModelContainer()+        let context = testContainer.context+        let project = Project(+            name: "Test Project", description: "A test project", gitRepo: nil, colorHex: "#FF0000"+        )+        context.insert(project)+        let allocator = DisplayIDAllocator(store: InMemoryCounterStore())+        return IntentServices(+            task: TaskService(modelContext: context, displayIDAllocator: allocator),+            project: ProjectService(modelContext: context),+            context: context,+            projectModel: project+        )+    }++    private func jsonInput(_ fields: [String: Any]) throws -> String {+        let data = try JSONSerialization.data(withJSONObject: fields)+        return try #require(String(data: data, encoding: .utf8))+    }++    private func parseJSON(_ string: String) throws -> [String: Any] {+        let data = try #require(string.data(using: .utf8))+        return try #require(try JSONSerialization.jsonObject(with: data) as? [String: Any])+    }++    // MARK: - JSON CreateTaskIntent++    @Test func createTaskIntentRejectsEveryNonObjectMetadataShapeWithoutMutation() async throws {+        let services = try makeIntentServices()+        let validInput = try jsonInput([+            "name": "Task without metadata",+            "type": "feature",+            "projectId": services.projectModel.id.uuidString+        ])+        let validResult = await CreateTaskIntent.execute(+            input: validInput,+            taskService: services.task,+            projectService: services.project+        )+        let validTaskID = try #require(try parseJSON(validResult)["taskId"] as? String)+        let validTaskUUID = try #require(UUID(uuidString: validTaskID))++        for entry in nonObjectValues {+            let input = try jsonInput([+                "name": "Task with invalid metadata \(entry.label)",+                "type": "feature",+                "projectId": services.projectModel.id.uuidString,+                "metadata": entry.value+            ])++            let result = await CreateTaskIntent.execute(+                input: input,+                taskService: services.task,+                projectService: services.project+            )++            let parsed = try parseJSON(result)+            #expect(parsed["error"] as? String == "INVALID_INPUT")+            #expect(parsed["hint"] as? String == "metadata must be an object")+        }++        #expect(try services.context.fetch(FetchDescriptor<TransitTask>()).map(\.id) == [validTaskUUID])+    }++#if os(macOS)+    // MARK: - MCP create_task++    @Test func mcpCreateTaskRejectsEveryNonObjectMetadataShapeWithoutMutation() async throws {+        let env = try MCPTestHelpers.makeEnv()+        let project = MCPTestHelpers.makeProject(in: env.context)+        let validResponse = await env.handler.handle(MCPTestHelpers.toolCallRequest(+            tool: "create_task",+            arguments: [+                "name": "Task without metadata",+                "type": "feature",+                "projectId": project.id.uuidString+            ]+        ))+        let validTaskID = try #require(try MCPTestHelpers.decodeResult(validResponse)["taskId"] as? String)+        let validTaskUUID = try #require(UUID(uuidString: validTaskID))++        for entry in nonObjectValues {+            let response = await env.handler.handle(MCPTestHelpers.toolCallRequest(+                tool: "create_task",+                arguments: [+                    "name": "Task with invalid metadata \(entry.label)",+                    "type": "feature",+                    "projectId": project.id.uuidString,+                    "metadata": entry.value+                ]+            ))++            #expect(try MCPTestHelpers.isError(response))+            #expect(try MCPTestHelpers.errorText(response) == "metadata must be an object")+        }++        #expect(try env.context.fetch(FetchDescriptor<TransitTask>()).map(\.id) == [validTaskUUID])+    }++    @Test func mcpCreateTaskSchemaRequiresMetadataObject() throws {+        let metadataSchema = try #require(+            MCPToolDefinitions.createTask.inputSchema.properties?["metadata"]+        )+        #expect(metadataSchema.type == "object")+    }+#endif+}

diff --git a/specs/bugfixes/create-task-paths-silently-ignore-non-object-metadata/report.md b/specs/bugfixes/create-task-paths-silently-ignore-non-object-metadata/report.mdnew file mode 100644index 0000000..0494e48--- /dev/null+++ b/specs/bugfixes/create-task-paths-silently-ignore-non-object-metadata/report.md@@ -0,0 +1,144 @@+# Bugfix Report: Create Task Paths Silently Ignore Non-Object Metadata++**Date:** 2026-08-02+**Status:** Fixed+**Transit ticket:** T-1991++## Description of the Issue++When the `metadata` key is present in JSON `CreateTaskIntent` input or an MCP+`create_task` request, callers can supply a string, array, number, boolean, or+null. Both create paths silently treat those values as omitted metadata and+successfully insert a task. This violates the documented contract that+`metadata` is an object and loses the caller's input without an error.++**Reproduction steps:**+1. Create a valid task request with `metadata` set to a non-object JSON value.+2. Send it through `CreateTaskIntent.execute` or MCP `tools/call` for+   `create_task`.+3. Observe a success response and a newly inserted task with empty metadata.++**Impact:** Automation callers receive a misleading success response and may+believe metadata was persisted when it was discarded. The bug affects both+JSON/App Intent and macOS MCP integrations.++## Investigation Summary++The investigation followed the systematic debugging workflow:++- **Phase 1 — Initial overview:** The expected behavior is field-specific+  rejection before mutation for every non-object shape, while an omitted key+  remains valid. The actual behavior is successful insertion with no metadata.+- **Phase 2 — Systematic inspection:** `CreateTaskIntent.execute` validates+  required fields and then passes `json["metadata"]` to+  `IntentHelpers.stringMetadata`; `MCPToolHandler.handleCreateTask` follows+  the same pattern with `args["metadata"]`. The helper returns `nil` for+  non-dictionaries, and `TaskService.createTask` treats `nil` as omitted+  metadata. No caller checks whether the key was present first.+- **Phase 3 — Root cause analysis:** The shared helper intentionally filters+  values inside a valid object for T-723, but its `nil` result conflates+  absent metadata, empty/all-non-string objects, and invalid top-level shapes.+  The create callers never distinguish those cases before task creation.+- **Phase 4 — Proposed solution:** Add a present-key container-shape check in+  each create path before project/milestone resolution and insertion. Return+  `INVALID_INPUT` with `metadata must be an object` from the intent and an MCP+  tool error with the same field-specific message. Continue routing valid+  object values through `stringMetadata` so non-string entries inside the+  object remain dropped under T-723.++## Discovered Root Cause++`IntentHelpers.stringMetadata(from:)` accepts only dictionaries and returns+`nil` for all other values. Both create paths pass that result directly to+`TaskService.createTask`, where `nil` means no metadata was supplied. Because+presence is not validated separately, a malformed present value is+indistinguishable from omission.++**Defect type:** Missing input validation / data-flow ambiguity.++**Why it occurred:** The helper was designed to preserve the T-723 behavior+of dropping non-string entries inside valid metadata objects, but callers used+its optional output as both parser and validator. That made invalid container+shapes silently follow the omission path.++**Contributing factors:** Existing tests covered valid objects and mixed values+inside objects, but did not cover malformed top-level metadata shapes or+assert no mutation on rejection.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/Intents/IntentHelpers.swift` - added `isMetadataObject` to validate the top-level container without changing the tolerant string-entry parser.+- `Transit/Transit/Intents/CreateTaskIntent.swift` - rejects present non-object metadata with `INVALID_INPUT` and `metadata must be an object` before project resolution or task creation.+- `Transit/Transit/MCP/MCPToolHandler.swift` - rejects present non-object metadata with an MCP tool error using the same field-specific message before mutation.+- `Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swift` - adds symmetric coverage for string, array, number, boolean, and null metadata values and asserts no task is inserted on either surface.+- `CHANGELOG.md` - documents the T-1991 behavior change under Unreleased/Fixed.++**Approach rationale:** Presence and container shape are validated separately+from metadata extraction. This preserves omission behavior and keeps valid+objects on the established `stringMetadata(from:)` path, where non-string+entries are dropped under T-723 rather than coerced or rejected.++**Alternatives considered:**+- Make `stringMetadata(from:)` throw or return a new error type - rejected+  because it would blur the existing parser's tolerant T-723 contract and+  require changing unrelated callers.+- Reject non-string values inside metadata objects - rejected because T-723+  explicitly establishes that those values are dropped.+- Coerce non-object values into strings - rejected because it would continue+  losing the caller's intended structure and violate the object schema.++## Regression Test++**Test file:** `Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swift`++**Test names:**+- `createTaskIntentRejectsEveryNonObjectMetadataShapeWithoutMutation`+- `mcpCreateTaskRejectsEveryNonObjectMetadataShapeWithoutMutation`++**What it verifies:** A present metadata string, array, number, boolean, or+null is rejected with a field-specific error and neither surface inserts a+task. Existing T-723 tests continue to verify that non-string values inside a+valid object are dropped.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/TransitTests/CreateTaskMetadataShapeValidationTests.swift` | Add symmetric regression and no-mutation coverage for both create paths. |+| `Transit/Transit/Intents/IntentHelpers.swift` | Add shared top-level metadata object-shape validation while preserving tolerant value filtering. |+| `Transit/Transit/Intents/CreateTaskIntent.swift` | Add present-key metadata object validation. |+| `Transit/Transit/MCP/MCPToolHandler.swift` | Add present-key metadata object validation. |+| `CHANGELOG.md` | Add an unreleased fixed entry for T-1991. |+| `specs/bugfixes/create-task-paths-silently-ignore-non-object-metadata/report.md` | Record investigation, resolution, and verification. |++## Verification++**Automated:**+- [x] Regression tests fail before the fix and pass after it. The red+  `make test-quick` run reported all six invalid-shape invocations as failing;+  the focused post-fix suite passed.+- [x] Full macOS unit suite passes via `make test-quick`, including both new+  T-1991 tests and the existing omission/T-723 metadata tests.+- [x] Linters/validators pass via `make lint` with 0 SwiftLint violations and+  all SwiftData ownership guard checks passing.++**Manual verification:**+- [x] Existing omission tests still pass for both create surfaces.+- [x] Existing T-723 tests still pass: valid objects preserve string values and+  drop non-string values.++## Prevention++- Validate presence and top-level shape before passing optional parsed values to+  mutating service methods.+- Keep symmetric App Intent/MCP regression tests for equivalent JSON contracts.+- Continue treating non-string entries inside a valid metadata object according+  to the established T-723 contract.++## Related++- Transit ticket T-1991+- T-723: MCP create_task drops non-string metadata values inside valid objects