Review: fix unit-test flakiness (merged as PR #16)

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex ad13aed..4a4965b 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -93,6 +93,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).  ### Fixed +- Unit-test flakiness — non-retained `ModelContainer` in test helpers (the dominant cause):+  - Seven suites built a SwiftData `ModelContainer` as a local, derived a `ModelContext`/`@Model` from it, and returned only the context (or a struct/context that didn't include the container). A `ModelContext` does not keep its `ModelContainer` alive, so ARC could deallocate it mid-test and the next model access trapped inside SwiftData (`EXC_BREAKPOINT`/`SIGTRAP`). Non-deterministic ARC timing made a different test "crash" each run, with the host restarting and the remaining tests reporting as `0.000s` crashes — i.e. flaky. Confirmed via `~/Library/Logs/DiagnosticReports/Scramble-*.ips` (use-after-dealloc from a model setter called by the test), not the "Xcode 26.5 multi-container toolchain bug" previously assumed.+  - Fixed by retaining the container for the caller's lifetime: `Scramble/ScrambleTests/Notifications/NotificationsServiceTests.swift` (`Setup` gains a `container` field), `RulesEngine/RulesEngineRunnerTests.swift` (`MatchedSeed` gains `container`), `Components/PackingItemRowAccessibilityTests.swift` + `Components/TaskRowAccessibilityTests.swift` (`Setup` gains `container`), and the bare-context helpers `Models/PackingItemContentBridgeTests.swift`, `Models/TripPackingItemBridgeTests.swift`, `Explainability/WhyResolverParticipantHideTests.swift` (helper now returns the `ModelContainer`; callers derive the context). Extends the earlier T-1605 fix (`d663be1`) that patched only two of the affected suites. Same root cause that crashed the on-device test host (see `docs/agent-notes/`); test-only change.++- Unit-test flakiness — async-cancellation race in `TripSyncEventBus`:+  - `Scramble/Scramble/Sharing/TripSyncEventBus.swift` — `start()`'s iteration task now checks `Task.isCancelled` before dispatching each event. `stop()` cancels the task, but an event already buffered in the `AsyncStream` can still be delivered by `for await` on the next resume; whether it reached handlers depended on a cancel-vs-deliver race, so `TripSyncEventBusTests.stopCancelsIteration()` passed some runs and failed others. The guard makes `stop()`'s documented contract ("subsequent yields don't reach handlers") deterministic.++- `LocalWriteHookPBT.mixedZonePartition` test-setup bug (deterministic; was masked by the crash cascade):+  - `Scramble/ScrambleTests/Sharing/LocalWriteHookPBT.swift` — the surviving-zone deletion branch called `hook.commit(context)` + `notifier.calls.removeAll()` mid-staging, which prematurely flushed the already-staged vanishing-zone deletions and erased the notifier record, so the final `commitDeletion` reported no vanishing-zone deletions (Invariant 2 failed for every `deletedPerSurviving >= 1` scenario). Restructured so each surviving zone's to-be-deleted items are created and committed during seeding, then deleted with no intermediate commit — mirroring the real `TripDeletion.delete` reverse-cascade. Production `LocalWriteHook` code was correct and is unchanged.++- `make test` / `make test-quick` reported success even when tests failed:+  - `Makefile` — macOS ships GNU Make 3.81, which predates `.SHELLFLAGS` (added in 3.82), so the file's `pipefail` was silently ignored and `xcodebuild test | xcbeautify` returned xcbeautify's exit code (`0`), masking test/build failures. Added a `PIPEFAIL := set -o pipefail;` prefix to every recipe that pipes to `xcbeautify` (`build`, `test-quick`, `test`, `test-ui`, `install`) so failures propagate regardless of make version (harmless when `xcbeautify` is absent and there is no pipe).++- `docs/agent-notes/testing.md` — rewrote the note: the "Xcode 26.5 — SwiftData multi-container crash" diagnosis was wrong (the crash reports show non-retained-container use-after-dealloc). Now documents the real root cause, how to diagnose it from `.ips` crash reports, the retain-the-container rule, the second `TripSyncEventBus` flake, and the GNU Make 3.81 exit-code-masking caveat.+ - On-device launch failures (SwiftData migration + CloudKit):   - `Scramble/Scramble/Models/Schema.swift`, `Scramble/Scramble/Persistence/ModelStore.swift` — removed the short-lived `SchemaV4` and the V3 → V4 migration stage that crashed on-device with `NSInvalidArgumentException: Duplicate version checksums across stages detected`. `SchemaV4` was byte-identical to `SchemaV3` because its only addition (`Trip.countryCode`) lives on the shared top-level `Trip` class that every schema version references, and SwiftData computes the version checksum from structure, not the version number. `Trip.countryCode` now belongs to `SchemaV3` and is added to existing stores via automatic lightweight inference; `AppMigrationPlan` is back to `[V1, V2, V3]`.   - `Scramble/Scramble/Models/TripPackingItem.swift` (plus `SnapshotMaintenance`, `TripPackingItemRecordTranslator`, `SchemaV3MigrationStage`, `ZoneMigrationCoordinator`, `RecordRepresentable` doc) — converted `TripPackingItem.personSnapshot` from an inverse-less `@Relationship` to a `personSnapshotID: UUID?` value reference (mirroring `TripTask.assigneePersonID`). The `globals` container's SwiftData CloudKit mirror rejected the inverse-less relationship on a real device (`NSCocoaErrorDomain Code=134060 ... requires that all relationships have an inverse: TripPackingItem: personSnapshot`), dropping `globals` to a local-only fallback that silently disabled iCloud sync for `Person` and the master lists. A `personSnapshot` computed bridge keeps every read site unchanged, and the `CKRecord` wire format (`personSnapshotID: String`) is unchanged. Predicates use the stored `personSnapshotID`.

diff --git a/Makefile b/Makefileindex 13a58bc..c0e8d74 100644--- a/Makefile+++ b/Makefile@@ -19,6 +19,14 @@ else PIPE_PRETTY = endif +# The .SHELLFLAGS pipefail above is ignored by GNU make 3.81 (the macOS system+# default; .SHELLFLAGS landed in 3.82). Without pipefail, `xcodebuild | xcbeautify`+# returns xcbeautify's exit code (0) and SILENTLY MASKS test/build failures —+# `make test` would report success on a red suite. Prefix every piped recipe+# with this so pipefail is set in the recipe's own shell regardless of make+# version (harmless when xcbeautify is absent and there is no pipe).+PIPEFAIL := set -o pipefail;+ SWIFTLINT := $(shell command -v swiftlint 2>/dev/null) SWIFT_FORMAT := $(shell command -v swift-format 2>/dev/null) @@ -59,7 +67,7 @@ help:  .PHONY: build build:-	xcodebuild build \+	$(PIPEFAIL) xcodebuild build \ 		-project $(PROJECT) \ 		-scheme $(SCHEME) \ 		-destination 'platform=iOS Simulator,name=$(SIMULATOR)' \@@ -79,7 +87,7 @@ clean: # extra XCTest UI runner and is the dominant cost of a full `test` run). .PHONY: test-quick test-quick:-	xcodebuild test \+	$(PIPEFAIL) xcodebuild test \ 		-project $(PROJECT) \ 		-scheme $(SCHEME) \ 		-destination 'platform=iOS Simulator,name=$(SIMULATOR)' \@@ -94,7 +102,7 @@ test-quick: # simulator clones race on launch / status-bar overrides and produce flakes. .PHONY: test test:-	xcodebuild test \+	$(PIPEFAIL) xcodebuild test \ 		-project $(PROJECT) \ 		-scheme $(SCHEME) \ 		-destination 'platform=iOS Simulator,name=$(SIMULATOR)' \@@ -106,7 +114,7 @@ test:  .PHONY: test-ui test-ui:-	xcodebuild test \+	$(PIPEFAIL) xcodebuild test \ 		-project $(PROJECT) \ 		-scheme $(SCHEME) \ 		-destination 'platform=iOS Simulator,name=$(SIMULATOR)' \@@ -134,7 +142,7 @@ install: 		echo "Error: No '$(DEVICE_MODEL)' device found. Connect one or override DEVICE_MODEL=..."; \ 		exit 1; \ 	fi-	xcodebuild build \+	$(PIPEFAIL) xcodebuild build \ 		-project $(PROJECT) -scheme $(SCHEME) \ 		-destination 'id=$(DEVICE_ID)' -configuration $(CONFIG) \ 		-derivedDataPath $(DERIVED_DATA) $(PIPE_PRETTY)

diff --git a/Scramble/Scramble/Sharing/TripSyncEventBus.swift b/Scramble/Scramble/Sharing/TripSyncEventBus.swiftindex 47e16e3..e56e4ce 100644--- a/Scramble/Scramble/Sharing/TripSyncEventBus.swift+++ b/Scramble/Scramble/Sharing/TripSyncEventBus.swift@@ -104,6 +104,13 @@ final class TripSyncEventBus {     let coordinator = coordinatorHandler     iterationTask = Task { @MainActor [events] in       for await event in events {+        // Honour cancellation before dispatching. `stop()` cancels this+        // task, but an event already buffered in the stream can still be+        // delivered by `for await` on the next resume; without this guard+        // whether that buffered event reaches handlers depends on the+        // cancel-vs-deliver race. Checking here makes `stop()`'s contract+        // ("subsequent yields don't reach handlers") deterministic.+        if Task.isCancelled { break }         Self.dispatch(event, to: orchestrator, label: "orchestrator")         Self.dispatch(event, to: coordinator, label: "coordinator")       }

diff --git a/Scramble/ScrambleTests/Components/PackingItemRowAccessibilityTests.swift b/Scramble/ScrambleTests/Components/PackingItemRowAccessibilityTests.swiftindex 895b701..505d170 100644--- a/Scramble/ScrambleTests/Components/PackingItemRowAccessibilityTests.swift+++ b/Scramble/ScrambleTests/Components/PackingItemRowAccessibilityTests.swift@@ -91,6 +91,11 @@ struct PackingItemRowAccessibilityTests {   // MARK: - Helpers    struct Setup {+    /// Retains the container for the setup's lifetime. A `ModelContext` does+    /// not keep its `ModelContainer` alive, so without this the container+    /// deallocates when the helper returns and any later model access traps+    /// inside SwiftData (SIGTRAP).+    let container: ModelContainer     let context: ModelContext   } @@ -100,7 +105,7 @@ struct PackingItemRowAccessibilityTests {       schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none     )     let container = try ModelContainer(for: schema, configurations: [config])-    return Setup(context: container.mainContext)+    return Setup(container: container, context: container.mainContext)   }    static func makeItem(

diff --git a/Scramble/ScrambleTests/Components/TaskRowAccessibilityTests.swift b/Scramble/ScrambleTests/Components/TaskRowAccessibilityTests.swiftindex 4854a54..660d38c 100644--- a/Scramble/ScrambleTests/Components/TaskRowAccessibilityTests.swift+++ b/Scramble/ScrambleTests/Components/TaskRowAccessibilityTests.swift@@ -134,6 +134,11 @@ struct TaskRowAccessibilityTests {   // MARK: - Helpers    struct Setup {+    /// Retains the container for the setup's lifetime. A `ModelContext` does+    /// not keep its `ModelContainer` alive, so without this the container+    /// deallocates when the helper returns and any later model access traps+    /// inside SwiftData (SIGTRAP).+    let container: ModelContainer     let context: ModelContext   } @@ -143,6 +148,6 @@ struct TaskRowAccessibilityTests {       schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none     )     let container = try ModelContainer(for: schema, configurations: [config])-    return Setup(context: container.mainContext)+    return Setup(container: container, context: container.mainContext)   } }

diff --git a/Scramble/ScrambleTests/Explainability/WhyResolverParticipantHideTests.swift b/Scramble/ScrambleTests/Explainability/WhyResolverParticipantHideTests.swiftindex a07b56c..0a40eb0 100644--- a/Scramble/ScrambleTests/Explainability/WhyResolverParticipantHideTests.swift+++ b/Scramble/ScrambleTests/Explainability/WhyResolverParticipantHideTests.swift@@ -16,22 +16,26 @@ import Testing @MainActor struct WhyResolverParticipantHideTests { -  private func makeContext() throws -> ModelContext {+  // Returns the container (not the context) so the caller retains it. A+  // `ModelContext` does not keep its `ModelContainer` alive; returning a bare+  // context lets the container deallocate and the next model access traps+  // inside SwiftData (SIGTRAP).+  private func makeContainer() throws -> ModelContainer {     let schema = Schema(versionedSchema: SchemaV3.self)     let config = ModelConfiguration(       schema: schema,       isStoredInMemoryOnly: true,       cloudKitDatabase: .none     )-    let container = try ModelContainer(for: schema, configurations: [config])-    return ModelContext(container)+    return try ModelContainer(for: schema, configurations: [config])   }    // MARK: - TripTask    @Test("Participant: rule-driven task with missing master returns nil")   func participantTaskWithUnresolvedMasterIsHidden() throws {-    let context = try makeContext()+    let container = try makeContainer()+    let context = ModelContext(container)     let trip = Trip(name: "Trip", startDate: .now, endDate: .now)     context.insert(trip)     let task = TripTask(@@ -53,7 +57,8 @@ struct WhyResolverParticipantHideTests {    @Test("Owner: rule-driven task with missing master returns .ruleMasterDeleted")   func ownerTaskWithUnresolvedMasterFallsBack() throws {-    let context = try makeContext()+    let container = try makeContainer()+    let context = ModelContext(container)     let trip = Trip(name: "Trip", startDate: .now, endDate: .now)     context.insert(trip)     let task = TripTask(@@ -71,7 +76,8 @@ struct WhyResolverParticipantHideTests {    @Test("Participant: manual tasks still resolve to .manual (no hide)")   func participantManualTaskStillRenders() throws {-    let context = try makeContext()+    let container = try makeContainer()+    let context = ModelContext(container)     let trip = Trip(name: "Trip", startDate: .now, endDate: .now)     context.insert(trip)     let task = TripTask(@@ -95,7 +101,8 @@ struct WhyResolverParticipantHideTests {    @Test("Participant: rule-driven packing item with missing master returns nil")   func participantPackingItemWithUnresolvedMasterIsHidden() throws {-    let context = try makeContext()+    let container = try makeContainer()+    let context = ModelContext(container)     let trip = Trip(name: "Trip", startDate: .now, endDate: .now)     context.insert(trip)     let item = TripPackingItem(@@ -118,7 +125,8 @@ struct WhyResolverParticipantHideTests {    @Test("Owner: rule-driven packing item with missing master falls back to .ruleMasterDeleted")   func ownerPackingItemWithUnresolvedMasterFallsBack() throws {-    let context = try makeContext()+    let container = try makeContainer()+    let context = ModelContext(container)     let trip = Trip(name: "Trip", startDate: .now, endDate: .now)     context.insert(trip)     let item = TripPackingItem(

diff --git a/Scramble/ScrambleTests/Models/PackingItemContentBridgeTests.swift b/Scramble/ScrambleTests/Models/PackingItemContentBridgeTests.swiftindex b505c2a..60a5ef6 100644--- a/Scramble/ScrambleTests/Models/PackingItemContentBridgeTests.swift+++ b/Scramble/ScrambleTests/Models/PackingItemContentBridgeTests.swift@@ -16,7 +16,8 @@ struct TripPackingItemContentBridgeTests {    @Test("subItems get/set round-trips through subItemsData")   func subItemsRoundTrip() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Toys")     context.insert(item) @@ -29,7 +30,8 @@ struct TripPackingItemContentBridgeTests {    @Test("setting subItems = [] clears subItemsData to nil")   func emptyListClearsData() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Books")     context.insert(item) @@ -43,7 +45,8 @@ struct TripPackingItemContentBridgeTests {    @Test("non-nil empty Data() reads back as []")   func nonNilEmptyDataReadsAsEmpty() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Snacks")     context.insert(item) @@ -55,7 +58,8 @@ struct TripPackingItemContentBridgeTests {    @Test("garbage Data decodes to []")   func garbageDataDecodesToEmpty() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Gear")     context.insert(item) @@ -65,7 +69,8 @@ struct TripPackingItemContentBridgeTests {    @Test("note set then cleared")   func noteSetThenCleared() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Camera")     context.insert(item) @@ -80,7 +85,8 @@ struct TripPackingItemContentBridgeTests {    @Test("note via init")   func noteViaInit() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Charger", note: "USB-C only")     context.insert(item)     try context.save()@@ -89,7 +95,8 @@ struct TripPackingItemContentBridgeTests {    @Test("skip -> restore leaves note and subItems unchanged")   func skipRestorePreservesContent() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Toys", note: "soft ones only")     context.insert(item)     item.subItems = ["bear", "blocks"]@@ -106,11 +113,15 @@ struct TripPackingItemContentBridgeTests {     #expect(item.subItems == ["bear", "blocks"])   } -  private static func makeContext() throws -> ModelContext {+  // Returns the container (not the context) so the caller retains it. A+  // `ModelContext` does not keep its `ModelContainer` alive; returning a bare+  // context lets the container deallocate and the next model access traps+  // inside SwiftData (SIGTRAP).+  private static func makeContainer() throws -> ModelContainer {     let schema = Schema(versionedSchema: SchemaV3.self)     let config = ModelConfiguration(       schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none     )-    return try ModelContainer(for: schema, configurations: [config]).mainContext+    return try ModelContainer(for: schema, configurations: [config])   } }

diff --git a/Scramble/ScrambleTests/Models/TripPackingItemBridgeTests.swift b/Scramble/ScrambleTests/Models/TripPackingItemBridgeTests.swiftindex 8866a0a..cc0ffad 100644--- a/Scramble/ScrambleTests/Models/TripPackingItemBridgeTests.swift+++ b/Scramble/ScrambleTests/Models/TripPackingItemBridgeTests.swift@@ -17,7 +17,8 @@ struct TripPackingItemBridgeTests {    @Test("Fast path: resolves via trip.participantSnapshots")   func resolvesViaParticipantSnapshots() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let trip = Trip(name: "T", startDate: .now, endDate: .now)     context.insert(trip)     let snapshot = TripPersonSnapshot(personID: UUID(), name: "Alice", trip: trip)@@ -33,7 +34,8 @@ struct TripPackingItemBridgeTests {    @Test("Fallback path: resolves via modelContext fetch when the trip side isn't wired")   func resolvesViaContextFetchWhenTripNil() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let snapshot = TripPersonSnapshot(personID: UUID(), name: "Bob")     context.insert(snapshot)     // No trip relationship and no participantSnapshots, so the in-memory@@ -49,7 +51,8 @@ struct TripPackingItemBridgeTests {    @Test("Nil personSnapshotID resolves to nil")   func nilIDResolvesNil() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Towel")     context.insert(item)     try context.save()@@ -60,7 +63,8 @@ struct TripPackingItemBridgeTests {    @Test("Dangling personSnapshotID (no matching snapshot) resolves to nil")   func danglingIDResolvesNil() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let item = TripPackingItem(name: "Boots")     item.personSnapshotID = UUID()  // points at nothing     context.insert(item)@@ -71,7 +75,8 @@ struct TripPackingItemBridgeTests {    @Test("Setter stores the snapshot's id and clears on nil")   func setterStoresID() throws {-    let context = try Self.makeContext()+    let container = try Self.makeContainer()+    let context = container.mainContext     let snapshot = TripPersonSnapshot(personID: UUID(), name: "Cara")     context.insert(snapshot)     let item = TripPackingItem(name: "Map")@@ -83,11 +88,15 @@ struct TripPackingItemBridgeTests {     #expect(item.personSnapshotID == nil)   } -  private static func makeContext() throws -> ModelContext {+  // Returns the container (not the context) so the caller retains it. A+  // `ModelContext` does not keep its `ModelContainer` alive; returning a bare+  // context lets the container deallocate and the next model access traps+  // inside SwiftData (SIGTRAP).+  private static func makeContainer() throws -> ModelContainer {     let schema = Schema(versionedSchema: SchemaV3.self)     let config = ModelConfiguration(       schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none     )-    return try ModelContainer(for: schema, configurations: [config]).mainContext+    return try ModelContainer(for: schema, configurations: [config])   } }

diff --git a/Scramble/ScrambleTests/Notifications/NotificationsServiceTests.swift b/Scramble/ScrambleTests/Notifications/NotificationsServiceTests.swiftindex 4c295eb..86b2caf 100644--- a/Scramble/ScrambleTests/Notifications/NotificationsServiceTests.swift+++ b/Scramble/ScrambleTests/Notifications/NotificationsServiceTests.swift@@ -177,6 +177,11 @@ struct NotificationsServiceTests {   // MARK: - Helpers    struct Setup {+    /// Retains the container for the setup's lifetime. A `ModelContext` does+    /// not keep its `ModelContainer` alive, so without this the container+    /// deallocates when the helper returns and any later model access traps+    /// inside SwiftData (SIGTRAP).+    let container: ModelContainer     let stub: StubNotificationCenter     let service: NotificationsService     let context: ModelContext@@ -222,7 +227,7 @@ struct NotificationsServiceTests {       coalesceWindow: .milliseconds(100)     )     return Setup(-      stub: stub, service: service, context: context, calendar: cal+      container: container, stub: stub, service: service, context: context, calendar: cal     )   } }

diff --git a/Scramble/ScrambleTests/RulesEngine/RulesEngineRunnerTests.swift b/Scramble/ScrambleTests/RulesEngine/RulesEngineRunnerTests.swiftindex d8e3a54..a2ff485 100644--- a/Scramble/ScrambleTests/RulesEngine/RulesEngineRunnerTests.swift+++ b/Scramble/ScrambleTests/RulesEngine/RulesEngineRunnerTests.swift@@ -318,6 +318,11 @@ struct RulesEngineRunnerTests {   /// Bundle returned by `seedMatchedPackingItem` for the independence   /// assertions (a struct rather than a 4-tuple per SwiftLint).   private struct MatchedSeed {+    /// Retains the container for the seed's lifetime. A `ModelContext` does+    /// not keep its `ModelContainer` alive, so without this the container+    /// deallocates when the helper returns and any later model access traps+    /// inside SwiftData (SIGTRAP).+    let container: ModelContainer     let context: ModelContext     let trip: Trip     let person: Person@@ -355,7 +360,8 @@ struct RulesEngineRunnerTests {     let runner = RulesEngineRunner(context: context)     _ = try runner.runForTrip(trip)     let item = try #require(try context.fetch(FetchDescriptor<TripPackingItem>()).first)-    return MatchedSeed(context: context, trip: trip, person: person, item: item)+    return MatchedSeed(+      container: container, context: context, trip: trip, person: person, item: item)   }    @Test("Req 7.1/7.2: recompute leaves an existing item's note + subItems unchanged")

diff --git a/Scramble/ScrambleTests/Sharing/LocalWriteHookPBT.swift b/Scramble/ScrambleTests/Sharing/LocalWriteHookPBT.swiftindex 9e280f1..ef64fd7 100644--- a/Scramble/ScrambleTests/Sharing/LocalWriteHookPBT.swift+++ b/Scramble/ScrambleTests/Sharing/LocalWriteHookPBT.swift@@ -77,9 +77,18 @@ struct LocalWriteHookPBT {     let notifier = RecordingNotifier()     let hook = LocalWriteHook(notifier: notifier) -    // Seed surviving zones: trip + TripZoneState per zone.+    // Seed surviving zones: trip + TripZoneState per zone, plus the items+    // each surviving zone will later delete. Those to-be-deleted items are+    // created and committed HERE, during seeding, so they exist as committed+    // records before any deletion is staged. Creating them mid-staging (with+    // the intermediate `hook.commit` they would then require) prematurely+    // flushes the vanishing-zone deletions staged below, so the final+    // `commitDeletion` would no longer see them — which is not how the real+    // `TripDeletion.delete` cascade behaves (it stages everything, then+    // commits once).     var survivingZoneIDs: [CKRecordZone.ID] = []     var survivingTrips: [Trip] = []+    var survivingDeletedItems: [[TripPackingItem]] = []     for index in 0..<scenario.survivingZoneCount {       let trip = Trip(name: "Surviving\(index)", startDate: .now, endDate: .now)       context.insert(trip)@@ -93,8 +102,15 @@ struct LocalWriteHookPBT {         zoneScope: "private"       )       context.insert(state)+      var delItems: [TripPackingItem] = []+      for delIndex in 0..<scenario.deletedPerSurviving {+        let item = TripPackingItem(trip: trip, name: "s-del-\(delIndex)")+        context.insert(item)+        delItems.append(item)+      }       survivingZoneIDs.append(zoneID)       survivingTrips.append(trip)+      survivingDeletedItems.append(delItems)     }     try hook.commit(context)     notifier.calls.removeAll()@@ -161,14 +177,13 @@ struct LocalWriteHookPBT {       context.delete(state)     } -    // Surviving-zone deletions: insert + commit some items then delete them.+    // Surviving-zone deletions: delete the items committed during seeding,+    // and stage fresh dirty items. No intermediate commit here — everything+    // is committed once via `commitDeletion` below, mirroring the real+    // reverse-cascade.     for (index, trip) in survivingTrips.enumerated() {       let zoneID = survivingZoneIDs[index]-      for delIndex in 0..<scenario.deletedPerSurviving {-        let item = TripPackingItem(trip: trip, name: "s-del-\(delIndex)")-        context.insert(item)-        try hook.commit(context)-        notifier.calls.removeAll()+      for item in survivingDeletedItems[index] {         context.delete(item)         expectedSurvivingDeleted[zoneID, default: []].insert(item.id.uuidString)       }

diff --git a/docs/agent-notes/testing.md b/docs/agent-notes/testing.mdindex 5a8f619..9489db4 100644--- a/docs/agent-notes/testing.md+++ b/docs/agent-notes/testing.md@@ -1,66 +1,110 @@ # Testing notes -## Xcode 26.5 — SwiftData multi-container crash in the test suite+## The unit suite's "flaky crash" was non-retained `ModelContainer`s (NOT a toolchain bug) -**Symptom.** Running `make test-quick` (or `make test`, or any `xcodebuild test`-covering more than a handful of SwiftData suites) crashes mid-run with a cascade-of `0.000s` failures attributed to "Test crashed with signal trap" /-`Crash: Scramble at <SomeSwiftDataTest>`. ~80–100 tests pass first, then the-host process dies and every remaining test in that process reports as crashed;-xcodebuild retries on a fresh simulator clone and crashes again. The crash is an-`EXC_BREAKPOINT` (`SIGTRAP`) raised **inside SwiftData**, not an assertion-failure.+**Symptom.** Running `make test-quick` (or `make test`) intermittently crashed+mid-run: ~80–100 tests passed, then the test host died with an+`EXC_BREAKPOINT` (`SIGTRAP`) raised inside SwiftData, every remaining test in+that process reported as a `0.000s` "crash," and xcodebuild relaunched on a+fresh simulator clone and often crashed again. Which test "crashed" differed+run to run, so it read as flaky. -**Root cause.** Every SwiftData test suite builds its own in-memory container-with a freshly-derived schema:+**Root cause (confirmed June 2026 via crash reports).** Several test helpers+built a SwiftData `ModelContainer` as a local, derived a `ModelContext` (and+sometimes `@Model` objects) from it, and returned only the context — not the+container:  ```swift-let schema = Schema(versionedSchema: SchemaV3.self)   // re-derived per container-let config = ModelConfiguration(schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none)-return try ModelContainer(for: schema, configurations: [config])+// BUG: container is a temporary; nothing the caller holds retains it.+private static func makeContext() throws -> ModelContext {+  let schema = Schema(versionedSchema: SchemaV3.self)+  let config = ModelConfiguration(schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none)+  return try ModelContainer(for: schema, configurations: [config]).mainContext+} ``` -Under the Xcode 26.5 toolchain, creating a **2nd+ `ModelContainer` for the same-`SchemaV3`** within a single test process traps. It is **not** caused by any-feature code — proven by:-- a pre-existing, untouched suite (`TripPackingItemBridgeTests`) crashing 0/N-  when run alone, and-- the same crash reproducing at the pre-feature baseline commit.--It surfaced with the Xcode 26.5 upgrade (June 2026); earlier toolchains-tolerated the repeated-container pattern.--**What still works.**-- `make build` — fine.-- `xcodebuild build-for-testing` — fine (both unit + UI targets compile).-- `make lint` / `make format` — fine.-- A **single** test in its own process (`-only-testing:Suite/func`) sometimes-  runs and reports; often the runner instead reports `0 tests` with scrambled-  output (`"Testing started"` printed after `** TEST SUCCEEDED **`). Either way-  it does not crash — but you cannot rely on it for a clean red/green signal.-- Reusing one shared `Schema` instance across containers avoided the crash in a-  minimal 8-container diagnostic, but **did not** fix a real multi-test suite —-  so schema-sharing is a clue, not the whole fix.--**Recovery that does NOT help:** `simctl shutdown all`, deleting-`~/Library/Developer/XCTestDevices` clones, restarting-`com.apple.CoreSimulator.CoreSimulatorService`, `simctl erase all`, device-reboot. A separate, worse "wedge" (stuck `launchd_sim` surviving `kill -9`) is-cleared by a **machine reboot**, but the reboot does **not** fix the-multi-container `SIGTRAP` above — that is a toolchain regression, not a wedged-service.--**Until it's fixed (proper fix is out of scope of any one feature — it's a-project-wide test-harness change):** verify SwiftData-touching work with-`make build` + `build-for-testing` + `make lint`, and treat a full-suite/whole--suite crash as the toolchain, not a code failure. Do **not** burn time fighting-the simulator. A real fix likely means a shared test-support container/schema-factory used by every suite (and confirming it survives multi-suite runs).--**Operational caution:** do not run two `xcodebuild test` invocations against the-same simulator concurrently (parallel worktrees) — they race and can wedge-CoreSimulator. Also scope any `pkill` to Scramble (`pkill -f "xcodebuild.*Scramble"`),-not a blanket `pkill -f xcodebuild`, which will kill unrelated projects' test runs-on a shared machine.+A `ModelContext` does **not** keep its `ModelContainer` alive. The container+deallocates when the helper returns; the next model access/mutation through the+orphaned context traps inside SwiftData (`_assertionFailure` → `SIGTRAP`). It+manifests as "flaky" because ARC dealloc timing is non-deterministic — under+simulator load it sometimes collects the container mid-test, sometimes not, and+a different orphaned suite loses the race each run.++The earlier theory in this note — "Xcode 26.5 multi-container toolchain bug" —+was **wrong**. The crash reports+(`~/Library/Logs/DiagnosticReports/Scramble-*.ips`) show a use-after-dealloc+from a model setter called directly from the test, e.g.:++```+libswiftCore  _assertionFailure+SwiftData     …+Scramble      TripPackingItem.note.setter        ← mutate through orphaned context+ScrambleTests RulesEngineRunnerTests.deletingItemRemovesNoteAndSubItems()+```++The note's own "evidence" for a toolchain bug (e.g. `TripPackingItemBridgeTests`+crashing 0/N when run alone) was just that suite's own non-retained helper.+Repeated multi-container creation is **not** the problem — creating many+in-memory containers is fine; only dropping one while its context is still in+use is fatal.++**How to diagnose.** Run the suite, then read the newest+`~/Library/Logs/DiagnosticReports/Scramble-*.ips`. Parse the faulting thread:+`SIGTRAP` + `SwiftData` + `_assertionFailure` + a frame in the test that mutates+a model = a non-retained container. Different `.ips` files naming different+tests across one run confirms the non-determinism.++**The fix / the rule.** Every helper that vends a `ModelContext` or `@Model`+must keep the container alive for as long as the caller uses it:++- Helper returns a struct → add a `container: ModelContainer` field (it never+  needs to be read; it exists to retain).+- Helper returns a bare context → return the `ModelContainer` instead and let+  the caller derive `container.mainContext` / `ModelContext(container)`.+- Helper used inside one test body → bind `let container = …; let context =+  container.mainContext` (already-scoped is fine).++This is the same rule as `rules/language-rules/swift.md` → "Retain the+ModelContainer in tests — never use a temporary." Fixed across all seven+affected suites (`NotificationsServiceTests`, `RulesEngineRunnerTests`,+`PackingItemContentBridgeTests`, `TripPackingItemBridgeTests`,+`PackingItemRowAccessibilityTests`, `TaskRowAccessibilityTests`,+`WhyResolverParticipantHideTests`); the earlier T-1605 commit (`d663be1`) had+fixed the same pattern in two category suites but missed the rest. Verified by a+full clean run (single host PID, zero `0.000s` crashes) plus repeated runs of+the SwiftData-heavy suites.++## A second, independent flake: async-cancellation race in `TripSyncEventBus`++Once the crash was fixed and the suite ran to completion,+`TripSyncEventBusTests.stopCancelsIteration()` showed up as genuinely flaky+(passed one run, failed the next). `start()` spawns a `@MainActor` iteration+task; the test calls `start()` then `stop()` (which cancels it) with no+suspension between, so the task only runs when the test next awaits — by which+point an event has already been yielded into the `AsyncStream` buffer. Whether a+cancelled `for await` delivers that buffered element before observing+cancellation is timing-dependent. Fix was in production+(`TripSyncEventBus.start`): check `Task.isCancelled` at the top of the loop+before dispatching, so a stopped bus deterministically never dispatches —+honouring `stop()`'s documented contract.++Lesson: tests that assert the **absence** of an effect after a fixed+`Task.sleep` are inherently racy (you can't poll for "nothing happened"). The+other `TripSyncEventBus` tests use a `waitFor`-style condition poll, which is the+robust pattern for asserting presence.++## Operational cautions (still valid)++- Simulator tests run **serially** (`-parallel-testing-worker-count 1`);+  parallel simulator clones race on launch and produce different flakes. Don't+  change this.+- Do not run two `xcodebuild test` invocations against the same simulator+  concurrently (e.g. parallel worktrees) — they race and can wedge+  CoreSimulator.+- Scope any `pkill` to Scramble (`pkill -f "xcodebuild.*Scramble"`), never a+  blanket `pkill -f xcodebuild`, which kills unrelated projects' runs.+- There are two simulators named "iPhone 17 Pro"; `name=`-based destinations are+  ambiguous but resolve deterministically in practice — prefer a UDID if a run+  ever picks the wrong one.  See `persistence.md` for the SchemaV3 / no-SchemaV4 model rules.