PR review: #203 — Fix T-1807: Project Color Hex Round-Trip

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 275e494..563309e 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  - `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. - 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 - `UITestScenario` extracted to its own file for cleaner `TransitApp` organisation - `withCoreEnvironments` helper in `TransitApp` to consolidate shared environment modifiers across window scenes  ### Changed  - macOS: Cmd+N command opens a new task window via `openWindow` instead of using focused bindings (T-35) - macOS: Save buttons use `.confirmationAction` toolbar placement with "Save" text; iOS retains checkmark icon (T-35) - macOS: Edit Task back chevron uses `.navigation` placement (leading side) instead of `.cancellationAction` (T-35) - macOS: Removed inline `.borderedProminent` save buttons from ProjectEditView, MilestoneEditView, and TaskEditView macOS forms — save is now in the toolbar (T-35) - Fixed milestone picker in TaskEditView and AddTaskSheet to use UUID-based tags, avoiding `Hashable` requirement on `@Model` classes

diff --git a/Transit/Transit/Extensions/Color+Codable.swift b/Transit/Transit/Extensions/Color+Codable.swiftindex 3c056a5..14ccfc6 100644--- a/Transit/Transit/Extensions/Color+Codable.swift+++ b/Transit/Transit/Extensions/Color+Codable.swift@@ -1,28 +1,37 @@ import SwiftUI  extension Color {     /// Create a Color from a hex string (e.g., "#FF5733" or "FF5733").     init(hex: String) {         let cleaned = hex.trimmingCharacters(in: .whitespacesAndNewlines)             .replacingOccurrences(of: "#", with: "")          var rgb: UInt64 = 0         Scanner(string: cleaned).scanHexInt64(&rgb)          let red = Double((rgb >> 16) & 0xFF) / 255.0         let green = Double((rgb >> 8) & 0xFF) / 255.0         let blue = Double(rgb & 0xFF) / 255.0          self.init(red: red, green: green, blue: blue)     }      /// Convert to a hex string for storage (e.g., "FF5733").     /// Uses Color.Resolved to avoid UIColor/NSColor actor isolation issues.     var hexString: String {         let resolved = self.resolve(in: EnvironmentValues())-        let red = Int(max(0, min(1, resolved.red)) * 255)-        let green = Int(max(0, min(1, resolved.green)) * 255)-        let blue = Int(max(0, min(1, resolved.blue)) * 255)+        let red = roundedByte(from: resolved.red)+        let green = roundedByte(from: resolved.green)+        let blue = roundedByte(from: resolved.blue)         return String(format: "%02X%02X%02X", red, green, blue)     }++    /// Quantize a resolved sRGB component to its nearest 8-bit channel value.+    /// Resolved components can be a fraction below an exact byte boundary due+    /// to platform color-space conversion, so truncation would darken colors.+    private func roundedByte(from component: Float) -> Int {+        guard component.isFinite else { return 0 }+        let clamped = max(0, min(1, component))+        return Int((clamped * 255).rounded())+    } }

diff --git a/Transit/TransitTests/ColorHexRoundTripTests.swift b/Transit/TransitTests/ColorHexRoundTripTests.swiftnew file mode 100644index 0000000..fba4d66--- /dev/null+++ b/Transit/TransitTests/ColorHexRoundTripTests.swift@@ -0,0 +1,105 @@+import Foundation+import SwiftUI+import Testing+@testable import Transit++@MainActor @Suite(.serialized)+struct ColorHexRoundTripTests {++    @Test func everyRedByteRoundTripsWithoutDarkening() {+        for byte in 0...255 {+            let expected = String(format: "%02X0000", byte)+            #expect(Color(hex: expected).hexString == expected, "Red byte \(byte) did not round-trip")+        }+    }++    @Test func everyGreenByteRoundTripsWithoutDarkening() {+        for byte in 0...255 {+            let expected = String(format: "00%02X00", byte)+            #expect(Color(hex: expected).hexString == expected, "Green byte \(byte) did not round-trip")+        }+    }++    @Test func everyBlueByteRoundTripsWithoutDarkening() {+        for byte in 0...255 {+            let expected = String(format: "0000%02X", byte)+            #expect(Color(hex: expected).hexString == expected, "Blue byte \(byte) did not round-trip")+        }+    }++    @Test func componentsJustBelowEveryByteBoundaryRoundToNearestByte() {+        for byte in 0...255 {+            let component = byte == 0+                ? 0.0+                : (Float(byte) / 255.0).nextDown++            #expect(+                Color(+                    Color.Resolved(+                        colorSpace: .sRGB,+                        red: Float(component), green: 0, blue: 0+                    )+                ).hexString == String(format: "%02X0000", byte),+                "Red boundary component \(byte) did not round-trip"+            )+            #expect(+                Color(+                    Color.Resolved(+                        colorSpace: .sRGB,+                        red: 0, green: Float(component), blue: 0+                    )+                ).hexString == String(format: "00%02X00", byte),+                "Green boundary component \(byte) did not round-trip"+            )+            #expect(+                Color(+                    Color.Resolved(+                        colorSpace: .sRGB,+                        red: 0, green: 0, blue: Float(component)+                    )+                ).hexString == String(format: "0000%02X", byte),+                "Blue boundary component \(byte) did not round-trip"+            )+        }+    }++    @Test func resolvedComponentsAreClampedBeforeRounding() {+        let color = Color(+            Color.Resolved(+                colorSpace: .sRGB,+                red: -0.25,+                green: 1.25,+                blue: 0.5+            )+        )++        #expect(color.hexString == "00FF80")+    }++    @Test func nonFiniteResolvedComponentsUseSafeZeroFallback() {+        let color = Color(+            Color.Resolved(+                colorSpace: .sRGB,+                red: .nan,+                green: .infinity,+                blue: -.infinity+            )+        )++        #expect(color.hexString == "000000")+    }++    @Test func hexStringUsesUppercaseSixDigitRGBWithoutHashAndIgnoresAlpha() {+        let color = Color(+            .sRGB,+            red: 0x12 / 255.0,+            green: 0xAB / 255.0,+            blue: 0xCD / 255.0,+            opacity: 0.25+        )++        #expect(color.hexString == "12ABCD")+        #expect(color.hexString.hasPrefix("#") == false)+        #expect(color.hexString.count == 6)+    }+}

diff --git a/specs/bugfixes/project-color-hex-round-trip-darkens-some-channels/report.md b/specs/bugfixes/project-color-hex-round-trip-darkens-some-channels/report.mdnew file mode 100644index 0000000..771a80f--- /dev/null+++ b/specs/bugfixes/project-color-hex-round-trip-darkens-some-channels/report.md@@ -0,0 +1,88 @@+# Bugfix Report: Project Color Hex Round-Trip Darkens Some Channels++**Date:** 2026-08-02+**Status:** Fixed++## Description of the Issue++Project colors can darken when an existing project is opened and saved without changing the color. `Color.hexString` converts resolved RGB components to byte values with `Int(component * 255)`. When a resolved component is just below an exact byte boundary, truncation lowers that channel by one byte.++**Reproduction steps:**+1. Store a project color such as `#10xxxx`.+2. Open the project editor and save without changing the color.+3. Observe that the serialized color can contain `0F` for the original `10` channel.++**Impact:** Project colors can drift darker after ordinary edit/save cycles. Repeated edits can compound the drift across affected channels.++## Investigation Summary++The investigation followed the systematic debugging workflow: establish the expected and actual serialization behavior, inspect the conversion implementation and callers, classify boundary and data-flow defects, and verify the defect with deterministic resolved-color tests.++- **Symptoms examined:** Values such as `0x10` becoming `0x0F` and `0x3F` becoming `0x3E`; alpha and stored-format behavior were also checked.+- **Code inspected:** `Transit/Extensions/Color+Codable.swift`, `Views/Settings/ProjectEditView.swift`, `Services/ProjectEditMerge.swift`, project model/service color fields, and all `hexString`/`Color(hex:)` call sites.+- **Hypotheses tested:** The project editor's merge/save path was not independently rewriting the color; the loss occurs in resolved-component-to-byte conversion. The serializer intentionally stores RGB only, so opacity is not part of this bug.++## Discovered Root Cause++`Color.hexString` clamps each resolved component and immediately converts `component * 255` to `Int`. Integer conversion truncates toward zero. SwiftUI's resolved components can be representationally just below an exact byte boundary, so a mathematically exact byte is serialized as the preceding byte.++**Defect type:** Numeric conversion / boundary condition.++**Why it occurred:** The implementation treated floating-point resolved components as exact byte fractions and used truncation rather than nearest-byte quantization.++**Contributing factors:** SwiftUI color resolution is platform-aware and may use floating-point values that differ slightly from the original picker input. Project storage uses a six-digit uppercase RGB string without a leading `#`; opacity is intentionally omitted.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/Extensions/Color+Codable.swift:21-37` - Replaced truncating `Int(component * 255)` conversions with a shared `roundedByte` helper that rejects non-finite values, clamps components to `0...1`, rounds to the nearest byte, and then converts to `Int`.+- `Transit/TransitTests/ColorHexRoundTripTests.swift:8-105` - Added all-byte RGB round-trip tests, deterministic just-below-boundary tests using explicit sRGB `Color.Resolved` values at Float precision, explicit lower/upper clamping and non-finite fallback coverage, and format/opacity assertions.+- `CHANGELOG.md` - Documented the fixed behavior and regression coverage.++**Approach rationale:** Rounding after clamping directly addresses the numeric root cause while leaving `Color.resolve(in: EnvironmentValues())` in place. That preserves the existing platform-aware SwiftUI resolution path and avoids the UIColor/NSColor actor-isolation issues documented for this project. The six-digit uppercase RGB format and intentional omission of opacity remain unchanged.++**Alternatives considered:**+- Converting through `UIColor`/`NSColor` - Not chosen because it introduces platform-specific code and actor-isolation concerns without fixing the quantization rule.+- Changing the stored format to include alpha - Not chosen because project color storage is intentionally RGB-only and the bug is channel quantization, not opacity handling.++## Regression Test++**Test file:** `Transit/TransitTests/ColorHexRoundTripTests.swift`+**Test name:** `everyRedByteRoundTripsWithoutDarkening`, `everyGreenByteRoundTripsWithoutDarkening`, `everyBlueByteRoundTripsWithoutDarkening`, `componentsJustBelowEveryByteBoundaryRoundToNearestByte`, `resolvedComponentsAreClampedBeforeRounding`, `nonFiniteResolvedComponentsUseSafeZeroFallback`++**What it verifies:** Every RGB byte value `0...255` survives `Color(hex:)`/`hexString` round-tripping, deterministic sRGB `Color.Resolved` components immediately below every byte boundary round to the intended byte, out-of-range components clamp to the valid byte range, and NaN/infinite components use the safe zero fallback. The format test verifies uppercase six-digit RGB without `#` and confirms opacity remains excluded.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/Extensions/Color+Codable.swift` | Nearest-byte quantization for resolved RGB components. |+| `Transit/TransitTests/ColorHexRoundTripTests.swift` | Regression coverage for RGB byte round trips, boundary rounding, alpha, and format. |+| `specs/bugfixes/project-color-hex-round-trip-darkens-some-channels/report.md` | Investigation and resolution report. |+| `CHANGELOG.md` | User-visible bugfix entry. |++## Verification++**Automated:**+- [x] Focused regression tests pass on macOS (`ColorHexRoundTripTests`, 7 tests)+- [x] Focused regression tests pass on iOS Simulator (`ColorHexRoundTripTests`, 7 tests)+- [x] macOS unit suite passes via `make test-quick`+- [x] Linters/validators pass via `make lint` (0 violations)+- [ ] Full iOS `make test` is not clean: unrelated UI tests `TransitUITests.testClearAll`, `TransitUITests.testEditViewPreservesTaskMilestone`, `TransitUITests.testSettingsHasBackChevron`, `TransitUITests.testSettingsWithNoProjectsShowsCreatePrompt`, `TransitUITests.testTappingGearPushesSettingsView`, and `DataMaintenanceUITests.testDataMaintenanceGoldenPath` failed after the app and unit tests built successfully.++**Manual verification:**+- The pre-fix targeted run failed the color regressions against the truncating implementation.+- The post-fix targeted runs passed all seven tests on both macOS and iOS, including every RGB byte value, all three channels immediately below each Float byte boundary, lower/upper clamping, and non-finite fallback handling.+- The tests construct explicit sRGB `Color.Resolved` values at the framework's `Float` component precision, so the boundary assertions do not depend on one platform's incidental floating-point representation.++## Prevention++**Recommendations to avoid similar bugs:**+- Quantize resolved floating-point color components by rounding to the nearest byte after clamping.+- Keep explicit tests for every byte boundary and the persisted six-digit RGB format.++## Related++- Transit T-1807