PR review: #241 — Fix T-1937: Keep heartbeat launch-scoped

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 00cb77e..1fc6ea7 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -7,6 +7,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased]  - T-1862: Single-record `displayId` queries now reserve successful `[]` results for explicit task/milestone not-found outcomes. `QueryTasksIntent` and `QueryMilestonesIntent` surface other direct-lookup failures as source-specific `INTERNAL_ERROR` payloads, while MCP `query_milestones` returns the matching tool error; duplicate-ID error contracts remain unchanged. Deterministic injected-fetch regressions assert exact not-found, duplicate, and storage-failure behavior across the affected JSON and MCP paths.+- T-1937: iCloud Sync preference changes now remain restart-scoped for the heartbeat as well as the live SwiftData container. An active launch keeps its MCP freshness heartbeat running while the preference is toggled off and back on; a CloudKit-free fallback launch cannot start one from a preference-on. Explicit MCP lifecycle shutdown remains the only runtime stop path, and MCP-start failure handling from T-1767 is unchanged.+ - T-1824: MCP `query_milestones` and `QueryMilestonesIntent` now resolve every well-formed `projectId` through `ProjectService` before filtering. A missing project returns the established project-not-found error and an unreadable project store retains its exact storage error, for both full-list and `displayId` queries; malformed UUID validation and `projectId`-over-name precedence are unchanged. Cross-surface regressions cover those contracts. - T-1821: MCP Settings now reconciles a committed port change—including focus loss or Settings closure—through the existing serialized listener lifecycle. A debounced coordinator keeps only the latest enabled change, cancels pending work when MCP is disabled, and flushes pending work on view teardown; invalid ports retain the lifecycle's existing stop/error behavior. The setup command now follows the server's active listener port rather than a persisted draft, and focused state plus live loopback regressions cover coalescing, disabled-server, presentation, and focus-loss replacement behavior. - T-2018: Task editing requires the selected project UUID to resolve to a live project model before Save. When a project disappears while the editor is open, Save remains disabled and both iOS and macOS show an inline, VoiceOver-labelled recovery message explaining that the user must choose an available project to re-enable Save. The task-edit applier still fails closed before any mutation when a changed project has no resolved model, while unchanged project fields still accept nil and project-move milestone clearing/validation remain intact.

diff --git a/Transit/Transit/Services/SyncManager.swift b/Transit/Transit/Services/SyncManager.swiftindex 4430c32..26a4edd 100644--- a/Transit/Transit/Services/SyncManager.swift+++ b/Transit/Transit/Services/SyncManager.swift@@ -66,18 +66,14 @@ final class SyncManager {      // MARK: - Public API -    /// Toggles CloudKit sync on or off. Persists the preference to UserDefaults.-    /// Stops the heartbeat immediately when sync is disabled; re-enabling sync-    /// does not restart the heartbeat (a new `startHeartbeat` call is needed).+    /// Toggles CloudKit sync on or off and persists the preference to UserDefaults.     ///-    /// Deliberately does **not** touch `isCloudSyncActive`: the live container keeps-    /// whatever CloudKit mode it launched with until the app is relaunched.+    /// Deliberately does **not** touch the heartbeat or `isCloudSyncActive`: the live+    /// container and its heartbeat keep the CloudKit mode they launched with until the+    /// app is relaunched. Explicit MCP lifecycle operations own heartbeat start/stop.     func setSyncEnabled(_ enabled: Bool) {         isSyncEnabled = enabled         UserDefaults.standard.set(enabled, forKey: Self.syncEnabledKey)-        if !enabled {-            stopHeartbeat()-        }     }      /// Records the CloudKit mode the live `ModelContainer` was actually created with.@@ -115,10 +111,11 @@ final class SyncManager {     var isHeartbeatRunning: Bool { heartbeatTask != nil }      /// Starts a 60-second repeating heartbeat that writes to SwiftData,-    /// triggering CloudKit to pull pending remote changes.+    /// triggering CloudKit to pull pending remote changes. Scheduling is gated on the+    /// launch-fixed `isCloudSyncActive` mode, not the mutable sync preference.     func startHeartbeat(context: ModelContext) {-        heartbeatTask?.cancel()-        guard isSyncEnabled else { return }+        stopHeartbeat()+        guard isCloudSyncActive else { return }          heartbeatTask = Task {             while !Task.isCancelled {

diff --git a/Transit/TransitTests/SyncManagerTests.swift b/Transit/TransitTests/SyncManagerTests.swiftindex 4fa35bf..1c3b3cb 100644--- a/Transit/TransitTests/SyncManagerTests.swift+++ b/Transit/TransitTests/SyncManagerTests.swift@@ -63,31 +63,92 @@ struct SyncManagerTests {         }     } +    // MARK: - T-1937 Regression: heartbeat follows launch mode+     @Test-    func setSyncEnabled_false_stopsHeartbeat() {-        withSavedDefaults {+    func startHeartbeatSchedulesForCloudActiveLaunch() throws {+        try withSavedDefaults {+            UserDefaults.standard.set(true, forKey: "syncEnabled")+            let fixture = try TestModelContainer()+            let manager = SyncManager()+            manager.recordActiveCloudSync(true)+            manager.startHeartbeat(context: fixture.context)+            defer { manager.stopHeartbeat() }++            #expect(manager.isHeartbeatRunning,+                    "A CloudKit-active launch must schedule its heartbeat without a preference change")+        }+    }++    @Test+    func settingSyncOffKeepsHeartbeatRunningForCloudActiveLaunch() throws {+        try withSavedDefaults {+            UserDefaults.standard.set(true, forKey: "syncEnabled")+            let fixture = try TestModelContainer()             let manager = SyncManager()-            // Simulate a running heartbeat by starting one (requires a context,-            // but we can verify via isHeartbeatRunning after disable).-            // Even without an active heartbeat, disabling sync must nil the task.+            manager.recordActiveCloudSync(true)+            manager.startHeartbeat(context: fixture.context)+            defer { manager.stopHeartbeat() }+             manager.setSyncEnabled(false)+             #expect(manager.isSyncEnabled == false)-            #expect(manager.isHeartbeatRunning == false)+            #expect(manager.syncChangeRequiresRestart)+            #expect(manager.isHeartbeatRunning,+                    "A restart-scoped preference change must not stop the active launch heartbeat")         }     }      @Test-    func setSyncEnabled_reEnable_doesNotRestartHeartbeat() {-        withSavedDefaults {+    func settingSyncOnDoesNotStartHeartbeatForCloudInactiveLaunch() throws {+        try withSavedDefaults {+            UserDefaults.standard.set(false, forKey: "syncEnabled")+            let fixture = try TestModelContainer()             let manager = SyncManager()-            // Disable then re-enable: heartbeat should NOT auto-restart-            manager.setSyncEnabled(false)-            #expect(manager.isHeartbeatRunning == false)+            manager.recordActiveCloudSync(false)+            manager.setSyncEnabled(true)+            manager.startHeartbeat(context: fixture.context)++            #expect(manager.isSyncEnabled)+            #expect(manager.syncChangeRequiresRestart)+            #expect(!manager.isHeartbeatRunning,+                    "A preference change cannot enable a CloudKit-free container before relaunch")+        }+    }++    @Test+    func revertingSyncPreferenceLeavesActiveLaunchHeartbeatRunning() throws {+        try withSavedDefaults {+            UserDefaults.standard.set(true, forKey: "syncEnabled")+            let fixture = try TestModelContainer()+            let manager = SyncManager()+            manager.recordActiveCloudSync(true)+            manager.startHeartbeat(context: fixture.context)+            defer { manager.stopHeartbeat() } +            manager.setSyncEnabled(false)             manager.setSyncEnabled(true)-            #expect(manager.isSyncEnabled == true)-            #expect(manager.isHeartbeatRunning == false,-                    "Re-enabling sync must not restart the heartbeat; a new startHeartbeat call is required")++            #expect(!manager.syncChangeRequiresRestart)+            #expect(manager.isHeartbeatRunning,+                    "Reverting the preference must leave the launch-scoped heartbeat unchanged")+        }+    }++    @Test+    func explicitHeartbeatStopStillControlsActiveLaunchLifecycle() throws {+        try withSavedDefaults {+            UserDefaults.standard.set(true, forKey: "syncEnabled")+            let fixture = try TestModelContainer()+            let manager = SyncManager()+            manager.recordActiveCloudSync(true)+            manager.startHeartbeat(context: fixture.context)+            manager.setSyncEnabled(false)++            manager.stopHeartbeat()++            #expect(!manager.isHeartbeatRunning,+                    "Explicit MCP lifecycle shutdown must still stop the heartbeat")         }     } 

diff --git a/specs/bugfixes/restart-scoped-sync-preference-heartbeat/report.md b/specs/bugfixes/restart-scoped-sync-preference-heartbeat/report.mdnew file mode 100644index 0000000..b50bc1c--- /dev/null+++ b/specs/bugfixes/restart-scoped-sync-preference-heartbeat/report.md@@ -0,0 +1,84 @@+# Bugfix Report: Restart-Scoped Sync Preference Changes Heartbeat++**Date:** 2026-08-05+**Status:** Fixed++## Description of the Issue++The iCloud Sync preference is explicitly restart-scoped: a live SwiftData container continues in the CloudKit mode selected at launch. Despite that contract, `SyncManager.setSyncEnabled(false)` stopped the live heartbeat immediately, and `startHeartbeat` used the mutable preference rather than the launch-fixed mode.++**Reproduction steps:**+1. Launch Transit with iCloud Sync active and MCP enabled, so the heartbeat is running.+2. Turn iCloud Sync off in Settings, then turn it back on without relaunching.+3. Observe that Settings reports no pending restart change, but the heartbeat remains stopped until MCP is toggled or Transit restarts.++**Impact:** The MCP workflow could stop proactively pulling remote CloudKit changes for the rest of the launch, while the UI said its effective sync mode was unchanged.++## Investigation Summary++- **Symptoms examined:** Preference-off immediately cancelled the heartbeat; restoring the preference did not recreate it.+- **Code inspected:** `SyncManager.setSyncEnabled`, `SyncManager.startHeartbeat`, `SettingsView` sync/MCP controls, and `TransitApp.startMCPServerIfEnabled`.+- **Hypotheses tested:** The live CloudKit container mode is fixed at launch and already represented by `isCloudSyncActive`; only heartbeat lifecycle incorrectly read `isSyncEnabled`.++## Discovered Root Cause++`SyncManager` conflated its mutable persisted preference with the fixed CloudKit mode of the current launch at the heartbeat lifecycle boundary.++**Defect type:** Logic error / stale state authority.++**Why it occurred:** T-699 implemented immediate runtime preference propagation and stopped the timer on disable. T-1797/T-1857 later established that direct CloudKit work must follow `isCloudSyncActive` because the container cannot be live-reconfigured, but this timer lifecycle retained the former preference-based behavior.++**Contributing factors:** Existing tests asserted immediate-stop semantics without starting a real heartbeat, so they did not model the launch-mode contract or the off/on reversion sequence.++## Resolution for the Issue++**Changes made:**+- `Transit/Transit/Services/SyncManager.swift` — `setSyncEnabled` now persists only the restart-scoped preference; it does not alter the live heartbeat.+- `Transit/Transit/Services/SyncManager.swift` — `startHeartbeat` replaces any existing task and gates new scheduling on the launch-fixed `isCloudSyncActive` mode.+- `Transit/TransitTests/SyncManagerTests.swift` — replaced obsolete immediate-stop expectations with active-launch, inactive-fallback, preference-reversion, and explicit-stop regressions.++**Approach rationale:** The live container cannot change CloudKit configuration until relaunch, so heartbeat eligibility must share its single fixed authority. `stopHeartbeat()` remains the explicit lifecycle control used by the MCP toggle; `TransitApp.startMCPServerIfEnabled` remains unchanged, preserving T-1767’s start-after-MCP-attempt behavior.++**Alternatives considered:**+- Restart the heartbeat when the preference is restored — rejected because it leaves the active launch incorrectly stopped after the off toggle.+- Rebuild the live SwiftData container on preference changes — rejected by Decision 21 and outside T-1937’s scope.++## Regression Test++**Test file:** `Transit/TransitTests/SyncManagerTests.swift`+**Test names:** `settingSyncOffKeepsHeartbeatRunningForCloudActiveLaunch`, `settingSyncOnDoesNotStartHeartbeatForCloudInactiveLaunch`, `revertingSyncPreferenceLeavesActiveLaunchHeartbeatRunning`, `explicitHeartbeatStopStillControlsActiveLaunchLifecycle`++**What it verifies:** An active launch keeps its timer through preference-off and reversion; an inactive fallback launch cannot begin a timer from a preference-on; explicit lifecycle shutdown still stops the timer.++**Run command:** `make test-quick`++## Affected Files++| File | Change |+|------|--------|+| `Transit/Transit/Services/SyncManager.swift` | Base heartbeat eligibility on launch-fixed CloudKit mode; remove preference-driven stop |+| `Transit/TransitTests/SyncManagerTests.swift` | Add active/fallback/reversion/MCP lifecycle regressions |+| `specs/bugfixes/restart-scoped-sync-preference-heartbeat/report.md` | Investigation and resolution record |+| `CHANGELOG.md` | User-visible fix entry |++## Verification++**Automated:**+- [x] Regression tests fail before the fix (pre-fix `make test-quick` exited 2)+- [x] Regression tests pass after the fix (`make test-quick`; focused iOS `TransitTests/SyncManagerTests`)+- [ ] Full test suite passes — `make test` exceeded the 120-second command limit after reaching test execution and surfaced unrelated existing UI failures in `testClearAll`, `testEditViewPreservesTaskMilestone`, and `testDataMaintenanceGoldenPath`+- [x] Linters/validators pass (`make lint`)++**Manual verification:**+- Inspect the macOS Settings wiring: only the MCP enable toggle invokes `startHeartbeat`/`stopHeartbeat`; iCloud Sync only calls `setSyncEnabled`, which now preserves live launch behavior.++## Prevention++- Treat `isCloudSyncActive` as the sole authority for operations whose availability is fixed by the live container.+- Keep explicit MCP lifecycle operations as the only runtime start/stop control for the heartbeat.++## Related++- T-1937+- T-1797 / T-1857 — active CloudKit mode and restart-scoped preference contract+- T-1767 — heartbeat lifecycle after MCP startup failure (preserved)