scramble branch feature/copy-master-packing-items commits 8 (ahead of origin/main) files 15 touched (4 prod, 4 test, 7 docs/spec) lines +1874 / -14

Pre-push review: copy-master-packing-items

Copy a master packing item to other people from the Master Lists tab — independent deep-equal copies the rules engine materialises onto trips.

At a glance

  • Feature complete: per-row “Copy to people…” affordance → target picker → independent MasterPackingItem copies → rules-engine materialisation onto matching trips.
  • Conditions copied by value (decoded ItemConditions, not the raw blob) — deep-equal and independent for free via value semantics.
  • Master save atomic; trip materialisation best-effort — different containers (globals vs tripsLocal) decouple the two failure regimes.
  • Single comparator, single data source: picker eligibility and the confirm-time skip both read Person.masterPackingItems via normalizedName; a test proves the unsaved-insert race (AC 3.5) is covered.
  • Review fixes applied: PersonAvatar added to the picker row, eligibility computed once per body, a testable copyOutcome seam with branch tests, a Person.displayName helper, and the rules-engine triggers note updated.

Verdict

Ready to push

All 21 acceptance criteria are implemented and traced to symbols; four parallel review agents (reuse, quality, efficiency, spec/docs) found no blockers and no spec divergences. The findings raised were one reuse-major (picker row missing PersonAvatar), an efficiency/quality recompute, one spec-major test gap (the performCopy save-failure / all-skipped / deferred branches), and minor cleanups — all fixed in this review. One caveat before merge: verification ran against a flaky simulator test-host; the new suites were confirmed green on a single booted simulator, but a clean-environment make test is advisable.

Review findings

8 raised · 5 fixed · 3 skipped

Jump to findings →

Commits

Three-level explanation

What this does

Each person in the Master Lists tab has their own packing items, and every item carries the rules that decide when it appears on a trip. Re-typing “underwear” or “shirts” for each family member is tedious. This adds a “Copy to people…” action: pick the others, and each gets an independent copy — same name, same rules — which the app then adds to every matching trip automatically.

Why it matters

One action replaces re-entering the same item per person, and the copy behaves exactly like one authored by hand.

Key concepts

  • Master item — a reusable per-person template (name + when-it-applies rules).
  • Rules engine — reads templates and drops real items onto trips; we only create templates.
  • Independent copy — copies aren’t linked to the original; editing one later doesn’t change the others.

Components

  • MasterPersistence.copyPacking(source:toPersonIDs:in:) — one MasterPackingItem per eligible target (de-duped ids, skips same-name owners), inserted into globals without saving. Returns value-only CopyResult. Pure helpers normalizedName and copyToastMessage.
  • CopyPackingItemSheet — the picker; a @MainActor static eligibleTargets(source:people:) seam computes eligibility from Person.masterPackingItems via the same normalizedName the skip uses. Multi-select, ineligible rows disabled, empty-state, confirm gated on selection ∩ eligible.
  • MasterPackingList — per-row swipe + context affordance (gated on source eligibility), one SheetTarget enum behind a single .sheet(item:), the toast host, and performCopy.

Approach

performCopy mirrors MasterPackingEditor.runEngineAndDismiss: copyPacking → save (rollback + toast on throw) → runForAllNonPastTrips() (best-effort) → confirmation toast. Branch selection is extracted into a pure copyOutcome(createdCount:save:runEngine:) so save-failure / all-skipped / deferred paths are unit-testable.

Trade-offs

  • Conditions copied by value (deep-equal + independent) rather than byte-copying the blob.
  • Skip, not overwrite, on same-name collision — never clobbers customised conditions.
  • Master save atomic; recompute best-effort (different containers).

Technical deep dive

  • Single comparator over one data source. Picker display eligibility and the confirm-time skip both read Person.masterPackingItems through normalizedName; a test proves SwiftData surfaces an unsaved in-context same-name insert through that relationship, so the AC 3.5 re-check holds against a picker→confirm race.
  • Container boundary is load-bearing. LocalWriteHook.mapping(for:) returns nil for Person/Master*, so master creation bypasses the hook and saves to globals; only the engine’s trip-zone writes go through the hook. Copied masters sync via the globals CloudKit mirror; copied trip items reach participants via the hook-backed apply path — both reused.
  • Deferred-toast scope. runForAllNonPastTrips swallows per-trip apply failures (rollback + continue) and throws only at the top level, so the deferred toast covers a top-level recompute failure; per-trip failures self-heal on the next trigger (Decision 8 — shared runner unchanged).

Architecture impact

No schema change. Additive: one view, one persistence helper family, a per-row affordance + toast host. SheetTarget collapses the list’s sheets into one modifier with a "copy-\(id)" discriminator preventing .copy/.edit identity collision.

Things to watch

  • Eligibility scan is O(people × items), computed once per body — fine at family scale, memoise if rosters grow.
  • Save-failure/deferred branches are covered at the copyOutcome seam, not a forced-SwiftData-failure integration test.

Important changes — detailed

MasterPersistence: copyPacking + value-only CopyResult

MasterPersistence.swift

Why it matters. Core of the feature — creates the independent copies, owns the same-name skip and de-dupe, and is the sole filter authority at confirm time.

What to look at. MasterPersistence.swift — copyPacking(source:toPersonIDs:in:), normalizedName, copyToastMessage, CopyResult

Takeaway. Copy a SwiftData @Model’s Codable-blob field by passing the decoded value type into the initializer — value semantics give an independent, deep-equal copy and a fresh blob for free, no byte-copy.
Rationale. Helpers mutate without saving (caller owns mutate→save→run-engine, matching createPacking/applyPacking); CopyResult carries only value types so no @Model crosses an isolation boundary.

MasterPackingList: performCopy with a testable copyOutcome seam

MasterPackingList.swift

Why it matters. Orchestrates save→engine→toast across two containers; the extracted seam makes the save-failure, all-skipped, and deferred branches unit-testable without forcing a real SwiftData save failure.

What to look at. MasterPackingList.swift — performCopy, copyOutcome(createdCount:save:runEngine:), CopyOutcome enum

Takeaway. When orchestration mixes pure branch logic with hard-to-fail side effects (a SwiftData save), inject the effects as throwing closures and return an enum outcome — the decision logic becomes testable in isolation.
Rationale. A pre-push review flagged the save-failure user-facing contract (AC 3.6) as untested; forcing an in-memory SwiftData save failure is impractical, so the seam is the contract under test.

CopyPackingItemSheet: eligibility from one source, PersonAvatar row

CopyPackingItemSheet.swift

Why it matters. Picker correctness depends on display eligibility and the persistence skip never diverging; the avatar restores the app-wide person-identity affordance.

What to look at. CopyPackingItemSheet.swift — eligibleTargets(source:people:), single-pass body, PersonAvatar row

Takeaway. To guarantee a UI gate and a backend guard can’t diverge, have both read the same data path (Person.masterPackingItems) through the same comparator (normalizedName) — not just the same function over different inputs.
Rationale. Review found the original sheet used a separate @Query whose data path differed from the persistence skip; unifying on the relationship closes that gap. Eligibility is now computed once per body rather than 2+N times.

Single SheetTarget enum behind one .sheet(item:)

MasterPackingList.swift

Why it matters. Avoids the SwiftUI multiple-.sheet conflict when adding the copy presentation alongside create/edit.

What to look at. MasterPackingList.swift — SheetTarget { create, edit(id), copy(id) }

Takeaway. Fold sibling sheet presentations into one Identifiable enum behind a single .sheet(item:); use a string-prefixed id ("copy-\(id)") so two cases over the same model id don’t collide on sheet identity.
Rationale. The "copy-" prefix is load-bearing: without it .copy(x) and .edit(x) hash to the same identity and the second sheet won’t present. Matches the existing MasterTasksList/TaskForm discriminator convention.

Key decisions

Copy at the master level, not trip level.

Conditions live only on per-person MasterPackingItem; TripPackingItem has none. A master-level copy is the only place “requirements stay the same” is literally true and reusable across trips. (Decision 1)

Skip same-name owners, never overwrite.

A target already owning a same-name item is skipped (marked ineligible in the picker, re-checked at confirm), never overwritten — protects deliberately customised conditions. Case-insensitive, trimmed. (Decision 2)

Confirmation hosted on the list; all-skipped is a no-op success.

The toast hosts on MasterPackingList (which survives the picker dismissing), and an all-skipped confirm creates nothing, skips save + engine, and reports success. (Decision 5)

Post-copy recompute is best-effort; shared runner unchanged.

runForAllNonPastTrips swallows per-trip failures and throws only top-level; the deferred toast covers a top-level failure, per-trip failures self-heal on the next trigger. The shared runner was deliberately not modified for this feature. (Decision 8)

Toast duration 5s vs the app-wide 3s default.

The copy confirmation uses duration: 5 so it isn’t lost while the picker sheet dismisses; commented at the call site. A deliberate, commented deviation.

Review findings

SeverityAreaFindingResolution
majorCopyPackingItemSheet row (reuse)Picker row omitted PersonAvatar that every other person-row surface uses — a reuse miss and a person-colour design-pillar inconsistency.Added PersonAvatar(name:colorKey:size:.standard) leading the row, mirroring TripEditorPeoplePicker.personRow.
majorperformCopy orchestration (spec/test)performCopy save-failure / all-skipped / deferred-recompute branches (AC 3.6/3.7/4.2) had no direct test — only the underlying helpers and one happy-path UI test.Extracted pure copyOutcome(createdCount:save:runEngine:) seam and added CopyOutcomeTests covering all four branches incl. effect-not-called assertions.
minorCopyPackingItemSheet (efficiency/quality)eligibleTargets recomputed 2+N times per body eval (eligibleIDs / hasEligibleTarget / canConfirm / per row).Compute eligibility once in body and thread eligibleIDs into the rows/empty-state/confirm checks; static helper kept for tests.
minorPerson model (reuse)person.name.isEmpty ? "Unnamed" : name literal duplicated; no shared helper.Added Person.displayName extension and routed the new copy-feature call site through it (pre-existing sites left out of scope).
minordocs/agent-notes/rules-engine.mdThe recompute-triggers table omitted the new MasterPackingList.performCopy caller of runForAllNonPastTrips().Added a one-line trigger-table row for the copy flow.
minorMasterPersistence.formatNames (reuse)formatNames hand-rolls Oxford-comma list joining that Foundation's ListFormatter / formatted(.list(type:.and)) provides.Skipped: switching couples the hardened exact-string toast tests to locale; the app's localization is project-deferred. Revisit at localization time.
minorperformCopy vs MasterPackingEditor (quality)performCopy duplicates the ~3-line runner construction shared with MasterPackingEditor.runEngineAndDismiss.Skipped: a true extraction means modifying the untouched MasterPackingEditor (regression risk) for marginal DRY; the mirroring is intentional house pattern.
minorSheetTarget.id (quality)SheetTarget.id uses a stringly-typed "copy-\(id)" discriminator.Skipped: justified and commented; matches the existing MasterTasksList/TaskForm convention.

Per-file diffs

Click to expand.

Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swift Added +169 / -0
diff --git a/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swift b/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swiftnew file mode 100644index 0000000..ae0d1a7--- /dev/null+++ b/Scramble/Scramble/Features/MasterLists/CopyPackingItemSheet.swift@@ -0,0 +1,169 @@+import SwiftData+import SwiftUI++#if canImport(UIKit)+  import UIKit+#endif++/// Target-person picker for copying a single `MasterPackingItem` to other+/// people (Req 1.2). Presentation only: it owns the multi-select state and+/// passes the RAW selected ids to `onCopy` — `MasterPersistence.copyPacking`+/// is the sole authority that drops ineligible / raced-out targets, so the+/// sheet never re-filters on confirm (design "Components and Interfaces").+///+/// Eligibility for display is read off `Person.masterPackingItems` via the+/// SAME `MasterPersistence.normalizedName` comparator the persistence skip+/// uses, so the picker display and the confirm-time skip agree.+@MainActor struct CopyPackingItemSheet: View {+  let source: MasterPackingItem+  /// Raw selected person ids; the parent (`MasterPackingList.performCopy`)+  /// does the work and `copyPacking` re-checks eligibility.+  let onCopy: ([UUID]) -> Void+  let onCancel: () -> Void++  @Query(sort: \Person.name) private var allPeople: [Person]+  @Environment(\.theme) private var theme+  @Environment(\.colorScheme) private var colorScheme++  @State private var selected: Set<UUID> = []++  /// Eligible target people for a copy of `source`: everyone EXCEPT the source+  /// owner (2.1), MINUS anyone who already owns a same-name item (2.3). Uses+  /// `MasterPersistence.normalizedName` — the same comparator `copyPacking`+  /// applies — over each person's `masterPackingItems` relationship, the same+  /// data path the persistence skip reads.+  static func eligibleTargets(source: MasterPackingItem, people: [Person]) -> [Person] {+    let ownerID = source.person?.id+    let sourceKey = MasterPersistence.normalizedName(source.name)+    return people.filter { person in+      guard person.id != ownerID else { return false }+      let alreadyOwns = (person.masterPackingItems ?? []).contains {+        MasterPersistence.normalizedName($0.name) == sourceKey+      }+      return !alreadyOwns+    }+  }++  /// People rendered in the picker: everyone except the owner, sorted by name+  /// (2.1). Ineligible (same-name owner) people stay in the list but disabled.+  private var targetPeople: [Person] {+    allPeople.filter { $0.id != source.person?.id }+  }++  var body: some View {+    // Compute eligibility ONCE per body pass (the O(people×items) scan), then+    // thread `eligibleIDs` into the rows / empty-state / confirm-enabled checks+    // instead of each computed property re-running the scan.+    let eligible = Self.eligibleTargets(source: source, people: allPeople)+    let eligibleIDs = Set(eligible.map(\.id))+    let hasEligibleTarget = !eligibleIDs.isEmpty+    // Confirm enabled only when at least one *eligible* person is selected+    // (2.4). Intersecting against `eligibleIDs` means a stale selection of a+    // since-ineligible person cannot enable the button.+    let canConfirm = !selected.isDisjoint(with: eligibleIDs)++    return NavigationStack {+      Group {+        if hasEligibleTarget {+          peopleList(eligibleIDs: eligibleIDs)+        } else {+          emptyState+        }+      }+      .navigationTitle("Copy “\(source.name)”")+      .navigationBarTitleDisplayMode(.inline)+      .toolbar {+        ToolbarItem(placement: .cancellationAction) {+          Button("Cancel") { onCancel() }+        }+        ToolbarItem(placement: .confirmationAction) {+          Button("Copy") { confirm() }+            .disabled(!canConfirm)+            .accessibilityLabel("Copy item to selected people")+            #if DEBUG+              .accessibilityIdentifier("copyPacking.confirm")+            #endif+        }+      }+    }+  }++  // MARK: - Subviews++  private func peopleList(eligibleIDs: Set<UUID>) -> some View {+    let variant = theme.variant(for: colorScheme)+    return List {+      Section {+        ForEach(targetPeople) { person in+          row(for: person, eligibleIDs: eligibleIDs, variant: variant)+        }+      } footer: {+        Text("Already-owned items are skipped.")+      }+    }+    .listStyle(.insetGrouped)+  }++  @ViewBuilder+  private func row(+    for person: Person,+    eligibleIDs: Set<UUID>,+    variant: ThemeVariant+  ) -> some View {+    let isEligible = eligibleIDs.contains(person.id)+    let isSelected = selected.contains(person.id)++    Button {+      toggle(person.id)+    } label: {+      HStack(spacing: 12) {+        PersonAvatar(name: person.name, colorKey: person.colorKey, size: .standard)+        Text(person.displayName)+          .foregroundStyle(isEligible ? variant.textPrimary : variant.textSecondary)+        Spacer()+        if !isEligible {+          Text("already has it")+            .font(.footnote)+            .foregroundStyle(variant.textSecondary)+        } else if isSelected {+          Image(systemName: "checkmark")+            .foregroundStyle(variant.accent)+        }+      }+      .contentShape(Rectangle())+    }+    .buttonStyle(.plain)+    .disabled(!isEligible)+    .accessibilityValue(isEligible ? "" : "already has this item")+    #if DEBUG+      .accessibilityIdentifier("copyPacking.person.\(person.name)")+    #endif+  }++  private var emptyState: some View {+    ContentUnavailableView(+      "No one else needs this",+      systemImage: "checkmark.circle",+      description: Text("Everyone else already has an item named “\(source.name)”.")+    )+    #if DEBUG+      .accessibilityIdentifier("copyPacking.emptyState")+    #endif+  }++  // MARK: - Interaction++  private func toggle(_ id: UUID) {+    if selected.contains(id) {+      selected.remove(id)+    } else {+      selected.insert(id)+    }+  }++  private func confirm() {+    // Pass the RAW selected ids; copyPacking is the sole authority that drops+    // ineligible / raced-out targets (design "Components and Interfaces").+    onCopy(Array(selected))+  }+}
Scramble/Scramble/Features/MasterLists/MasterPackingList.swift Modified +183 / -14
diff --git a/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift b/Scramble/Scramble/Features/MasterLists/MasterPackingList.swiftindex 12b0fd4..66a4c56 100644--- a/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift+++ b/Scramble/Scramble/Features/MasterLists/MasterPackingList.swift@@ -1,26 +1,45 @@ import SwiftData import SwiftUI +#if canImport(UIKit)+  import UIKit+#endif+ /// AC 2.1 — list every `MasterPackingItem` grouped by `Person`, sorted by /// `Person.name` ascending, people with zero items omitted, per-person count /// in the header. AC 2.7 — when no `Person` exists, show empty state and hide /// the "+ Add item" affordance.+///+/// Copy feature: each eligible row also exposes a "Copy to people…" action via+/// a trailing swipe + long-press context menu (Decision 7 — net-new UI; the+/// whole-row tap still opens the editor). Confirming the picker runs the+/// save → engine → toast sequence in `performCopy`. @MainActor struct MasterPackingList: View {   @Query(sort: \MasterPackingItem.name) private var allItems: [MasterPackingItem]   @Query(sort: \Person.name) private var allPeople: [Person]   @Environment(\.theme) private var theme   @Environment(\.colorScheme) private var colorScheme+  @Environment(\.modelContext) private var modelContext+  @Environment(\.tripsLocalContainer) private var tripsLocalContainer+  @Environment(\.localWriteHook) private var hook -  @State private var editTarget: EditTarget?+  @State private var sheetTarget: SheetTarget?+  @State private var toastMessage: String? -  enum EditTarget: Identifiable, Hashable {+  enum SheetTarget: Identifiable, Hashable {     case create     case edit(PersistentIdentifier)+    case copy(PersistentIdentifier)      var id: AnyHashable {       switch self {       case .create: AnyHashable("create")       case .edit(let id): AnyHashable(id)+      // The "copy-" prefix is load-bearing: it keeps `.copy(x)` and `.edit(x)`+      // from resolving to the same `.sheet(item:)` identity for one item id.+      // Without it, presenting an edit sheet then a copy sheet for the same+      // item would reuse the prior identity and the copy sheet wouldn't show.+      case .copy(let id): AnyHashable("copy-\(id)")       }     }   }@@ -44,15 +63,7 @@ import SwiftUI           if !items.isEmpty {             Section {               ForEach(items) { item in-                Button {-                  editTarget = .edit(item.persistentModelID)-                } label: {-                  Text(item.name.isEmpty ? "Unnamed item" : item.name)-                    .foregroundStyle(variant.textPrimary)-                    .frame(maxWidth: .infinity, alignment: .leading)-                    .contentShape(Rectangle())-                }-                .buttonStyle(.plain)+                itemRow(item, variant: variant)               }             } header: {               HStack {@@ -67,19 +78,71 @@ import SwiftUI          Section {           DashedAddButton(title: "Add item", accent: variant.accent) {-            editTarget = .create+            sheetTarget = .create           }         }       }       .listStyle(.insetGrouped)-      .sheet(item: $editTarget) { target in+      .sheet(item: $sheetTarget) { target in         sheet(for: target)       }+      // 5s (vs the app-wide 3s default) is deliberate so the post-copy+      // confirmation isn't lost while the picker sheet finishes dismissing.+      .transientToast(message: $toastMessage, duration: 5)+    }+  }++  // MARK: - Rows++  @ViewBuilder+  private func itemRow(_ item: MasterPackingItem, variant: ThemeVariant) -> some View {+    let canCopy = isCopyEligible(item)+    Button {+      sheetTarget = .edit(item.persistentModelID)+    } label: {+      Text(item.name.isEmpty ? "Unnamed item" : item.name)+        .foregroundStyle(variant.textPrimary)+        .frame(maxWidth: .infinity, alignment: .leading)+        .contentShape(Rectangle())+    }+    .buttonStyle(.plain)+    .swipeActions(edge: .trailing, allowsFullSwipe: false) {+      if canCopy {+        Button {+          sheetTarget = .copy(item.persistentModelID)+        } label: {+          Label("Copy to people…", systemImage: "person.2")+        }+        .tint(variant.accent)+      }     }+    .contextMenu {+      if canCopy {+        Button {+          sheetTarget = .copy(item.persistentModelID)+        } label: {+          Label("Copy to people…", systemImage: "person.2")+        }+      }+    }+    #if DEBUG+      .accessibilityIdentifier("masterPacking.itemRow.\(item.name)")+    #endif+  }++  /// Source eligibility for the copy action (Req 1.3 / Decision 6): at least+  /// two people exist, the source has an owner, and its trimmed name is+  /// non-empty.+  private func isCopyEligible(_ item: MasterPackingItem) -> Bool {+    guard allPeople.count >= 2 else { return false }+    guard item.person != nil else { return false }+    return !item.name.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty   } +  // MARK: - Sheet+   @ViewBuilder-  private func sheet(for target: EditTarget) -> some View {+  private func sheet(for target: SheetTarget) -> some View {     switch target {     case .create:       MasterPackingEditor(mode: .create)@@ -87,6 +150,112 @@ import SwiftUI       if let item = allItems.first(where: { $0.persistentModelID == id }) {         MasterPackingEditor(mode: .edit(item))       }+    case .copy(let id):+      // An unresolved `.copy` id (source deleted / owner changed between row+      // tap and presentation) renders nothing; dismissing writes nothing.+      if let item = allItems.first(where: { $0.persistentModelID == id }) {+        CopyPackingItemSheet(+          source: item,+          onCopy: { ids in+            sheetTarget = nil+            performCopy(source: item, toPersonIDs: ids)+          },+          onCancel: { sheetTarget = nil }+        )+      }     }   }++  // MARK: - Copy++  /// Branch selection of the post-`copyPacking` orchestration, extracted as a+  /// pure function so it is unit-testable without SwiftData. It owns ONLY the+  /// control flow; `performCopy` supplies the real save / run-engine closures+  /// and maps the outcome onto the rollback + toast side effects.+  enum CopyOutcome: Equatable {+    case nothingToCopy+    case saveFailed+    case copied(deferred: Bool)+  }++  /// Decides the outcome of a copy:+  /// - `createdCount == 0` → `.nothingToCopy` — calls NEITHER `save` nor+  ///   `runEngine`.+  /// - else `try save()`; on throw → `.saveFailed` — does NOT call `runEngine`.+  /// - else `try runEngine()`; on throw → `.copied(deferred: true)`.+  /// - else → `.copied(deferred: false)`.+  static func copyOutcome(+    createdCount: Int,+    save: () throws -> Void,+    runEngine: () throws -> Void+  ) -> CopyOutcome {+    guard createdCount > 0 else { return .nothingToCopy }+    do {+      try save()+    } catch {+      return .saveFailed+    }+    do {+      try runEngine()+    } catch {+      return .copied(deferred: true)+    }+    return .copied(deferred: false)+  }++  /// save → engine → toast sequence (design Flow). `copyPacking` only inserts;+  /// this method owns `save()`, the recompute, and the user-facing toast — the+  /// list stays mounted while the picker dismisses, so the toast renders here.+  /// The branch selection is delegated to the pure `copyOutcome(...)`; this+  /// method maps the outcome onto rollback + the existing toast wording.+  private func performCopy(source: MasterPackingItem, toPersonIDs ids: [UUID]) {+    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: ids,+      in: modelContext+    )++    // Materialise onto matching non-past trips (4.1). Best-effort: a top-level+    // recompute failure keeps the saved masters and surfaces the deferred-+    // update wording (4.2), mirroring MasterPackingEditor.runEngineAndDismiss.+    let runner = RulesEngineRunner(+      context: tripsLocalContainer.mainContext,+      mastersContext: modelContext,+      hook: hook+    )++    let outcome = Self.copyOutcome(+      createdCount: result.createdCount,+      save: { try modelContext.save() },+      runEngine: { _ = try runner.runForAllNonPastTrips() }+    )++    switch outcome {+    case .saveFailed:+      modelContext.rollback()+      announce("Copy failed — try again.")+    case .copied(deferred: true):+      announce("Copied. Some trips couldn't be updated — they'll sync on next launch.")+    case .nothingToCopy, .copied(deferred: false):+      // All targets skipped at confirm (3.7) reports everyone already had it;+      // a clean copy reports who received it — both via copyToastMessage.+      announce(toastMessage(for: result))+    }+  }++  private func toastMessage(for result: CopyResult) -> String {+    MasterPersistence.copyToastMessage(+      copiedNames: result.copiedNames,+      skippedNames: result.skippedNames+    )+  }++  /// Shows the toast and posts a VoiceOver announcement, matching+  /// `PackingItemRow`'s move-announcement pattern.+  private func announce(_ message: String) {+    toastMessage = message+    #if canImport(UIKit)+      UIAccessibility.post(notification: .announcement, argument: message)+    #endif+  } }
Scramble/Scramble/Features/MasterLists/MasterPersistence.swift Modified +105 / -0
diff --git a/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift b/Scramble/Scramble/Features/MasterLists/MasterPersistence.swiftindex db20707..223a51f 100644--- a/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift+++ b/Scramble/Scramble/Features/MasterLists/MasterPersistence.swift@@ -2,6 +2,19 @@ import Foundation import SwiftData import os +/// Outcome of `MasterPersistence.copyPacking`. Value-only — it carries no+/// `@Model` instances out of the helper, so it crosses isolation boundaries+/// freely and feeds `copyToastMessage` from any context.+nonisolated struct CopyResult: Sendable {+  /// Copies inserted into the context, NOT yet saved.+  var createdCount: Int+  /// Names of the target people that received a copy.+  var copiedNames: [String]+  /// Names of the target people skipped because they already owned a+  /// same-name item.+  var skippedNames: [String]+}+ /// Helpers for applying master drafts to the model context. Mirrors /// `TripPersistence`: the helpers mutate but do NOT call `context.save()` — /// the master editor closure owns the mutate → save → run-engine sequence@@ -69,6 +82,98 @@ import os     context.delete(item)   } +  // MARK: - Copy master packing to people++  /// Trimmed + case-insensitive key for same-name detection (Req 2.3 / 3.5).+  /// `nonisolated` because `MasterPersistence` is a `@MainActor` enum but this+  /// is a pure `String` derivation safe to call from any context (and from+  /// unit tests without `await`).+  nonisolated static func normalizedName(_ raw: String) -> String {+    raw.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()+  }++  /// Builds the 5.1 / 5.2 / 3.7 confirmation strings from the plain name+  /// arrays (not the `@Model`-bearing `CopyResult`) so it stays `nonisolated`+  /// and unit-testable. Three outcomes:+  /// - copied-only: lists the people who received the item;+  /// - copied-with-skips: lists copied people and notes who was skipped;+  /// - all-skipped (empty `copiedNames`): states everyone already had it.+  nonisolated static func copyToastMessage(+    copiedNames: [String],+    skippedNames: [String]+  ) -> String {+    let copied = formatNames(copiedNames)+    let skipped = formatNames(skippedNames)++    if copiedNames.isEmpty {+      return "Everyone already had this item — skipped \(skipped)."+    }+    if skippedNames.isEmpty {+      return "Copied to \(copied)."+    }+    return "Copied to \(copied). Skipped \(skipped) — already had it."+  }++  /// Inserts one copy per target person that does not already own a same-name+  /// item. De-duplicates `toPersonIDs` (it is the sole authority on what gets+  /// created). Does NOT call `save()` — the caller owns the+  /// mutate → save → run-engine sequence, per the `MasterPersistence`+  /// convention. The same-name check covers Req 3.5 (a target that became a+  /// same-name owner since the picker opened).+  @discardableResult+  static func copyPacking(+    source: MasterPackingItem,+    toPersonIDs: [UUID],+    in context: ModelContext+  ) -> CopyResult {+    let trimmedName = source.name.trimmingCharacters(in: .whitespacesAndNewlines)+    let sourceConditions = source.conditions+    let sourceKey = normalizedName(trimmedName)++    var result = CopyResult(createdCount: 0, copiedNames: [], skippedNames: [])+    var seen = Set<UUID>()++    for id in toPersonIDs {+      guard seen.insert(id).inserted else { continue }+      guard let target = resolvePerson(id: id, in: context) else { continue }++      let alreadyOwns = (target.masterPackingItems ?? []).contains {+        normalizedName($0.name) == sourceKey+      }+      if alreadyOwns {+        result.skippedNames.append(target.name)+        continue+      }++      let copy = MasterPackingItem(+        name: trimmedName,+        person: target,+        conditions: sourceConditions+      )+      context.insert(copy)+      result.createdCount += 1+      result.copiedNames.append(target.name)+    }++    return result+  }++  /// Joins names with commas and a trailing "and" for the final element so the+  /// toast reads naturally for one, two, or many people.+  private nonisolated static func formatNames(_ names: [String]) -> String {+    switch names.count {+    case 0:+      return ""+    case 1:+      return names[0]+    case 2:+      return "\(names[0]) and \(names[1])"+    default:+      let head = names.dropLast().joined(separator: ", ")+      return "\(head), and \(names[names.count - 1])"+    }+  }+   // MARK: - Internal    private static func resolvePerson(id: UUID?, in context: ModelContext) -> Person? {
Scramble/Scramble/Models/Person.swift Modified +6 / -0
diff --git a/Scramble/Scramble/Models/Person.swift b/Scramble/Scramble/Models/Person.swiftindex a2b637b..b2b7c88 100644--- a/Scramble/Scramble/Models/Person.swift+++ b/Scramble/Scramble/Models/Person.swift@@ -29,6 +29,12 @@ final class Person { extension Person {   var initial: String { name.firstGraphemeUppercased } +  /// Display-safe name: the person's `name`, or `"Unnamed"` when it is empty.+  /// `nonisolated` to match the other pure derivations in this extension (the+  /// file inherits `MainActor` isolation but this is a pure `String`+  /// derivation).+  nonisolated var displayName: String { name.isEmpty ? "Unnamed" : name }+   /// Short-form name derivation per Phase 4 design §"Short-form name   /// derivation". Trims whitespace, returns the first space-separated token,   /// or the full trimmed name when there is no space, or `"?"` when the name
Scramble/Scramble/Persistence/UITestSeed.swift Modified +15 / -0
diff --git a/Scramble/Scramble/Persistence/UITestSeed.swift b/Scramble/Scramble/Persistence/UITestSeed.swiftindex ecc5667..98530af 100644--- a/Scramble/Scramble/Persistence/UITestSeed.swift+++ b/Scramble/Scramble/Persistence/UITestSeed.swift@@ -17,6 +17,10 @@       case twoQualifyingTrips = "two-qualifying-trips"       case singleEditableTrip = "single-editable-trip"       case masterListsEmptyWithPerson = "master-lists-empty-with-person"+      /// Copy feature: two people ("Alex" owns a "Socks" master packing item,+      /// "Sam" owns nothing) so the per-row "Copy to people…" action is eligible+      /// (>= 2 people) and Sam is an eligible copy target.+      case masterPackingCopyTwoPeople = "master-packing-copy-two-people"       case masterTaskAdvancedConditions = "master-task-advanced-conditions"       case phase2RulesFixture = "phase2-rules-fixture"       case phase2RulesFixtureSunTrip = "phase2-rules-fixture-sun-trip"@@ -152,6 +156,7 @@         let end = calendar.date(byAdding: .day, value: 35, to: day) ?? day         context.insert(Trip(name: "Sample Trip", startDate: start, endDate: end))       case .masterListsEmptyWithPerson,+        .masterPackingCopyTwoPeople,         .masterTaskAdvancedConditions,         .phase2RulesFixture,         .phase2RulesFixtureSunTrip,@@ -194,6 +199,16 @@         context.insert(           MasterPackingItem(name: "Seeded item", person: alex, conditions: .always)         )+      case .masterPackingCopyTwoPeople:+        // Two people so the copy action is eligible (Req 1.3 needs >= 2).+        // Alex owns "Socks"; Sam owns nothing → Sam is an eligible target.+        let alex = Person(name: "Alex", colorKey: "blue")+        let sam = Person(name: "Sam", colorKey: "red")+        context.insert(alex)+        context.insert(sam)+        context.insert(+          MasterPackingItem(name: "Socks", person: alex, conditions: .always)+        )       case .masterTaskAdvancedConditions:         // Out-of-domain weather value forces AdvancedConditionView per AC 3.7a.         context.insert(Person(name: "Alex", colorKey: "blue"))
Scramble/ScrambleTests/MasterLists/CopyPackingTests.swift Added +609 / -0
diff --git a/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swift b/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swiftnew file mode 100644index 0000000..b338e22--- /dev/null+++ b/Scramble/ScrambleTests/MasterLists/CopyPackingTests.swift@@ -0,0 +1,609 @@+import Foundation+import SwiftData+import Testing++@testable import Scramble++// MARK: - Pure helpers (Task 1)++@Suite("MasterPersistence.normalizedName")+struct NormalizedNameTests {++  @Test("Trims surrounding whitespace and lowercases")+  func trimsAndLowercases() {+    #expect(MasterPersistence.normalizedName("  Socks ") == "socks")+    #expect(MasterPersistence.normalizedName("SOCKS") == "socks")+    #expect(MasterPersistence.normalizedName("Socks") == "socks")+  }++  @Test("'  Socks ' and 'socks' normalise equal")+  func trimmedVariantsMatch() {+    #expect(+      MasterPersistence.normalizedName("  Socks ")+        == MasterPersistence.normalizedName("socks")+    )+  }++  @Test("Empty and whitespace-only normalise to empty string")+  func emptyAndWhitespaceMapToEmpty() {+    #expect(MasterPersistence.normalizedName("") == "")+    #expect(MasterPersistence.normalizedName("   ") == "")+    #expect(MasterPersistence.normalizedName("\n\t ") == "")+  }+}++@Suite("MasterPersistence.copyToastMessage")+struct CopyToastMessageTests {++  // The three branches share name fragments ("Alice", "Bob"), so a bare+  // `contains(name)` check would still pass if two branches were transposed.+  // Each assertion below pins a DISTINGUISHING phrase from the source string so+  // swapping the copied-only / copied-with-skips / all-skipped arms fails a test.+  // Source phrasing (MasterPersistence.copyToastMessage):+  //   copied-only:        "Copied to \(copied)."+  //   copied-with-skips:  "Copied to \(copied). Skipped \(skipped) — already had it."+  //   all-skipped:        "Everyone already had this item — skipped \(skipped)."+  private static let skippedClause = "already had it"+  private static let copiedClause = "Copied to"+  private static let allSkippedLead = "Everyone already had this item"++  @Test("Copied-only: copied-clause present, skipped-clause absent")+  func copiedOnly() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: ["Alice", "Bob"],+      skippedNames: []+    )+    #expect(message.contains("Alice"))+    #expect(message.contains("Bob"))+    // Distinguishes from copied-with-skips: it copies, it does NOT skip anyone.+    #expect(message.contains(Self.copiedClause))+    #expect(message.contains(Self.skippedClause) == false)+    #expect(message.contains("Skipped") == false)+    #expect(message.contains(Self.allSkippedLead) == false)+  }++  @Test("Copied-with-skips: BOTH the copied-clause and the skipped-clause present")+  func copiedWithSkips() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: ["Alice"],+      skippedNames: ["Bob"]+    )+    #expect(message.contains("Alice"))+    #expect(message.contains("Bob"))+    // Distinguishes from BOTH other branches: copies AND notes a skip.+    #expect(message.contains(Self.copiedClause))+    #expect(message.contains(Self.skippedClause))+    // Not the all-skipped branch, which would lead with "Everyone…".+    #expect(message.contains(Self.allSkippedLead) == false)+  }++  @Test("All-skipped: 'already had it' wording, no 'Copied to' clause")+  func allSkipped() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: [],+      skippedNames: ["Alice", "Bob"]+    )+    #expect(message.contains("Alice"))+    #expect(message.contains("Bob"))+    // Distinguishes from the two copied branches: nobody received a copy, so the+    // "Copied to" clause must NOT leak through. The all-skipped string leads with+    // "Everyone already had this item — skipped …" and uses the verb "skipped";+    // it does NOT carry the copied-with-skips-only trailing "— already had it"+    // clause, so asserting that clause is ABSENT here guards against transposing+    // these two skip-bearing branches.+    #expect(message.contains(Self.allSkippedLead))+    #expect(message.contains("skipped"))+    #expect(message.contains(Self.skippedClause) == false)+    #expect(message.contains(Self.copiedClause) == false)+  }++  // Exercises the formatNames joiner indirectly through copyToastMessage (the+  // helper is private). Single name → bare name; two → "X and Y"; three+ →+  // oxford "A, B, and C". Asserted against the copied-only branch so the joined+  // string is the whole list of names.+  @Test("formatNames join: 1 name renders the bare name")+  func formatNamesSingle() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: ["Alice"],+      skippedNames: []+    )+    #expect(message.contains("Copied to Alice."))+    // No join words for a single name.+    #expect(message.contains(" and ") == false)+    #expect(message.contains(",") == false)+  }++  @Test("formatNames join: 2 names use the 'X and Y' join, no comma")+  func formatNamesPair() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: ["Alice", "Bob"],+      skippedNames: []+    )+    #expect(message.contains("Copied to Alice and Bob."))+    // Two names join with " and " and NO comma (distinguishes from 3+).+    #expect(message.contains(", and ") == false)+  }++  @Test("formatNames join: 3+ names use the oxford 'A, B, and C' join")+  func formatNamesOxford() {+    let message = MasterPersistence.copyToastMessage(+      copiedNames: ["Alice", "Bob", "Carol"],+      skippedNames: []+    )+    #expect(message.contains("Copied to Alice, Bob, and Carol."))+  }+}++// MARK: - copyPacking (Task 3)++@Suite("MasterPersistence.copyPacking", .serialized)+@MainActor+struct CopyPackingHelperTests {++  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])+  }++  @Test("One copy per eligible target, each owned by the right person; trimmed name (3.1/3.2)")+  func createsOneCopyPerEligibleTarget() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    let source = MasterPackingItem(name: "  Socks  ", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id, bob.id],+      in: context+    )++    #expect(result.createdCount == 2)+    #expect(Set(result.copiedNames) == ["Alice", "Bob"])+    #expect(result.skippedNames.isEmpty)++    let copies = try context.fetch(FetchDescriptor<MasterPackingItem>())+      .filter { $0.id != source.id }+    #expect(copies.count == 2)+    // Names are trimmed copies of the source (3.2).+    #expect(copies.allSatisfy { $0.name == "Socks" })+    // Each copy is owned by exactly one of the targets (3.1).+    let aliceCopy = try #require(copies.first { $0.person?.id == alice.id })+    let bobCopy = try #require(copies.first { $0.person?.id == bob.id })+    #expect(aliceCopy.name == "Socks")+    #expect(bobCopy.name == "Socks")+  }++  @Test("Skips a target already owning a same-name item; trimmed + case-insensitive (2.3/3.5)")+  func skipsSameNameOwnerCaseInsensitive() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    // Alice already owns "socks" — source is "  Socks " → normalised equal.+    let aliceExisting = MasterPackingItem(name: "socks", person: alice, conditions: .always)+    context.insert(aliceExisting)+    let source = MasterPackingItem(name: "  Socks ", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id],+      in: context+    )++    #expect(result.createdCount == 0)+    #expect(result.copiedNames.isEmpty)+    #expect(result.skippedNames == ["Alice"])+    // No new item created for Alice.+    let aliceItems = (alice.masterPackingItems ?? [])+    #expect(aliceItems.count == 1)+  }++  @Test("All targets already own it → createdCount == 0 and nothing inserted (3.7)")+  func allTargetsSkippedInsertsNothing() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    context.insert(MasterPackingItem(name: "Socks", person: alice, conditions: .always))+    context.insert(MasterPackingItem(name: "Socks", person: bob, conditions: .always))+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let before = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id, bob.id],+      in: context+    )++    #expect(result.createdCount == 0)+    #expect(result.copiedNames.isEmpty)+    #expect(Set(result.skippedNames) == ["Alice", "Bob"])+    let after = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+    #expect(after == before)+  }++  @Test("Duplicate ids in toPersonIDs produce exactly one copy (de-dupe)")+  func deDupesRepeatedIDs() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id, alice.id, alice.id],+      in: context+    )++    #expect(result.createdCount == 1)+    #expect(result.copiedNames == ["Alice"])+    #expect((alice.masterPackingItems ?? []).count == 1)+  }++  @Test("Skips a same-name owner whose item was inserted but NOT saved (3.5 race)")+  func skipsUnsavedSameNameInsert() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    // The load-bearing race (Req 3.5): a same-name item lands for the target+    // AFTER the picker opened but BEFORE the copy runs — and crucially before+    // any save(). The skip re-check reads `Person.masterPackingItems`, so it+    // must surface this pending in-context insert. We deliberately do NOT call+    // context.save() here.+    let aliceUnsaved = MasterPackingItem(name: "Socks", person: alice, conditions: .always)+    context.insert(aliceUnsaved)+    #expect(context.hasChanges == true)++    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id],+      in: context+    )++    // If this fails — i.e. the unsaved insert is invisible through the+    // relationship and the helper creates a duplicate — that is a real defect,+    // NOT a test to relax.+    #expect(result.createdCount == 0)+    #expect(result.copiedNames.isEmpty)+    #expect(result.skippedNames == ["Alice"])+    // Only the one pending insert exists for Alice; no copy was added.+    #expect((alice.masterPackingItems ?? []).count == 1)+  }++  @Test("Unresolvable person id is neither copied nor skipped (off-roster contract)")+  func unresolvableIDIsNeitherCopiedNorSkipped() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    context.insert(owner)+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    // A random id resolves to no Person in the store.+    let ghost = UUID()+    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [ghost],+      in: context+    )++    // An unresolved id is dropped silently: not a copy, and NOT a skip (a skip+    // means "a real person already had it"). Pin that asymmetry.+    #expect(result.createdCount == 0)+    #expect(result.copiedNames.isEmpty)+    #expect(result.skippedNames.isEmpty)+  }++  @Test("Pure mechanism: does NOT guard an empty/whitespace source name (Decision 6)")+  func doesNotGuardEmptySourceName() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    // Source name trims to "" — the UI source-eligibility gate (Decision 6 /+    // Req 1.3) blocks this upstream; the helper itself does NOT, so it copies an+    // empty-named item. This test documents the helper's actual behaviour, not a+    // guard we want it to grow.+    let source = MasterPackingItem(name: "   ", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let result = MasterPersistence.copyPacking(+      source: source,+      toPersonIDs: [alice.id],+      in: context+    )++    // The helper is a pure mechanism: it creates a copy with the trimmed+    // (empty) name and reports Alice as copied.+    #expect(result.createdCount == 1)+    #expect(result.copiedNames == ["Alice"])+    #expect(result.skippedNames.isEmpty)+    let copy = try #require(+      try context.fetch(FetchDescriptor<MasterPackingItem>())+        .first { $0.person?.id == alice.id }+    )+    #expect(copy.name == "")+  }++  @Test("Conditions fidelity: advanced nested form copied by value; source unchanged (3.3/3.4)")+  func conditionsFidelityAndIndependence() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    let nested: ItemConditions = .all([+      .any([+        .match(attribute: .weather, anyOf: ["rain", "snow"]),+        .match(attribute: .weather, anyOf: ["wind"]),+      ]),+      .match(attribute: .purpose, anyOf: ["hiking"]),+    ])+    let source = MasterPackingItem(name: "Layers", person: owner, conditions: nested)+    context.insert(source)+    try context.save()++    MasterPersistence.copyPacking(source: source, toPersonIDs: [alice.id], in: context)++    let copy = try #require(+      try context.fetch(FetchDescriptor<MasterPackingItem>())+        .first { $0.person?.id == alice.id }+    )+    // Decoded value equality, NOT conditionsData byte-equality.+    #expect(copy.conditions == source.conditions)+    #expect(copy.conditions == nested)++    // Mutating the copy's conditions leaves the source untouched (3.4).+    copy.conditions = .always+    #expect(source.conditions == nested)+    #expect(copy.conditions == .always)+  }+}++// MARK: - copyOutcome branch selection (pure orchestration)++/// Drives `MasterPackingList.copyOutcome`, the pure branch-selection extracted+/// from `performCopy`. These tests assert the control flow ONLY — which closures+/// run and the returned `CopyOutcome` — using recording flags, so they need no+/// SwiftData. The toast wording and rollback side effects stay in `performCopy`.+@Suite("MasterPackingList.copyOutcome")+@MainActor+struct CopyOutcomeTests {++  struct CopyError: Error {}++  @Test("createdCount 0 → .nothingToCopy; neither save nor runEngine runs (3.7)")+  func nothingToCopySkipsSaveAndEngine() {+    var didSave = false+    var didRun = false+    let outcome = MasterPackingList.copyOutcome(+      createdCount: 0,+      save: { didSave = true },+      runEngine: { didRun = true }+    )+    #expect(outcome == .nothingToCopy)+    #expect(didSave == false)+    #expect(didRun == false)+  }++  @Test("save throws → .saveFailed; runEngine is NOT invoked (3.6)")+  func saveFailureSkipsEngine() {+    var didRun = false+    let outcome = MasterPackingList.copyOutcome(+      createdCount: 2,+      save: { throw CopyError() },+      runEngine: { didRun = true }+    )+    #expect(outcome == .saveFailed)+    #expect(didRun == false)+  }++  @Test("save ok + runEngine throws → .copied(deferred: true) (4.2)")+  func engineFailureIsDeferred() {+    var didSave = false+    let outcome = MasterPackingList.copyOutcome(+      createdCount: 2,+      save: { didSave = true },+      runEngine: { throw CopyError() }+    )+    #expect(outcome == .copied(deferred: true))+    #expect(didSave == true)+  }++  @Test("save ok + runEngine ok → .copied(deferred: false)")+  func cleanCopyIsNotDeferred() {+    var didSave = false+    var didRun = false+    let outcome = MasterPackingList.copyOutcome(+      createdCount: 2,+      save: { didSave = true },+      runEngine: { didRun = true }+    )+    #expect(outcome == .copied(deferred: false))+    #expect(didSave == true)+    #expect(didRun == true)+  }+}++// MARK: - Engine integration + save atomicity (Task 5)++@Suite("Copy sequence: materialisation + atomicity", .serialized)+@MainActor+struct CopySequenceTests {++  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])+  }++  private static func rainyAttributes() -> TripAttributes {+    var a = TripAttributes()+    a.toggle(.weather, value: "rain")+    return a+  }++  private static func sunnyAttributes() -> TripAttributes {+    var a = TripAttributes()+    a.toggle(.weather, value: "sun")+    return a+  }++  /// Seeds a trip with the given attributes plus a `TripPersonSnapshot` for the+  /// target person, which Phase 5.1's engine requires to materialise a+  /// rule-driven `TripPackingItem` (an orphan otherwise).+  private static func seedTrip(+    name: String,+    attributes: TripAttributes,+    person: Person,+    in context: ModelContext+  ) -> Trip {+    let trip = Trip(name: name, startDate: .now, endDate: .now, attributes: attributes)+    context.insert(trip)+    let snapshot = TripPersonSnapshot(+      personID: person.id,+      name: person.name,+      colourID: person.colorKey,+      initialSource: "name",+      isRosterMember: true,+      trip: trip+    )+    context.insert(snapshot)+    return trip+  }++  @Test("Materialisation: copy → save → engine puts item only on the matching trip (4.1)")+  func materialisesOntoMatchingTripOnly() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    context.insert(owner)+    context.insert(alice)+    // Source matches rainy trips only.+    let source = MasterPackingItem(+      name: "Rain jacket",+      person: owner,+      conditions: .match(attribute: .weather, anyOf: ["rain"])+    )+    context.insert(source)++    let rainy = Self.seedTrip(+      name: "Rainy", attributes: Self.rainyAttributes(), person: alice, in: context)+    let sunny = Self.seedTrip(+      name: "Sunny", attributes: Self.sunnyAttributes(), person: alice, in: context)+    try context.save()++    let result = MasterPersistence.copyPacking(+      source: source, toPersonIDs: [alice.id], in: context)+    #expect(result.createdCount == 1)+    try context.save()++    let hook = LocalWriteHook(notifier: RecordingNotifier())+    let runner = RulesEngineRunner(context: context, mastersContext: context, hook: hook)+    _ = try runner.runForAllNonPastTrips()++    let packs = try context.fetch(FetchDescriptor<TripPackingItem>())+    let aliceOnRainy = packs.filter {+      $0.trip?.id == rainy.id && $0.personSnapshot?.personID == alice.id+    }+    let aliceOnSunny = packs.filter {+      $0.trip?.id == sunny.id && $0.personSnapshot?.personID == alice.id+    }+    #expect(aliceOnRainy.count == 1)+    #expect(aliceOnRainy.first?.name == "Rain jacket")+    #expect(aliceOnSunny.isEmpty)+  }++  @Test("Atomicity: copyPacking inserts but does not save; rollback persists zero copies (3.6)")+  func rollbackLeavesNoCopiesAndNoEngineRun() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let baseline = try context.fetch(FetchDescriptor<MasterPackingItem>()).count++    let result = MasterPersistence.copyPacking(+      source: source, toPersonIDs: [alice.id, bob.id], in: context)+    #expect(result.createdCount == 2)+    // Inserted but not saved — the helper never calls save().+    #expect(context.hasChanges == true)++    // Simulate the caller's save-failure path: drop all uncommitted inserts.+    context.rollback()+    #expect(context.hasChanges == false)++    let after = try context.fetch(FetchDescriptor<MasterPackingItem>()).count+    #expect(after == baseline)++    // The engine is never invoked on the failure path — assert no trip-level+    // items exist (no recompute ran). (A notifier assertion was removed here: an+    // unwired LocalWriteHook can never record a call, so it was vacuous —+    // copyPacking touches globals, not a trip zone, and never goes through the+    // hook; only the engine run would, and the engine never runs on rollback.)+    let packs = try context.fetch(FetchDescriptor<TripPackingItem>())+    #expect(packs.isEmpty)+  }+}
Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swift Added +104 / -0
diff --git a/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swift b/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swiftnew file mode 100644index 0000000..db00ae2--- /dev/null+++ b/Scramble/ScrambleTests/MasterLists/CopyPackingItemSheetEligibilityTests.swift@@ -0,0 +1,104 @@+import Foundation+import SwiftData+import Testing++@testable import Scramble++/// Task 6 — drives `CopyPackingItemSheet.eligibleTargets(source:people:)`, the+/// static helper that resolves the eligible target people for the picker. It+/// excludes the source owner (2.1) and any person who already owns a same-name+/// item (2.3), using `MasterPersistence.normalizedName` as the SAME comparator+/// `copyPacking` applies. When every other person is ineligible the set is+/// empty, which drives the picker's 2.5 empty-state.+@Suite("CopyPackingItemSheet.eligibleTargets", .serialized)+@MainActor+struct CopyPackingItemSheetEligibilityTests {++  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])+  }++  @Test("Source owner is excluded from the eligible set (2.1)")+  func ownerExcluded() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let eligible = CopyPackingItemSheet.eligibleTargets(+      source: source,+      people: [owner, alice, bob]+    )++    let ids = Set(eligible.map(\.id))+    #expect(ids == [alice.id, bob.id])+    #expect(ids.contains(owner.id) == false)+  }++  @Test("A person already owning a same-name item is ineligible; trimmed + case-insensitive (2.3)")+  func sameNameOwnerIneligible() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    // Alice already owns "socks" — source is "  Socks " → normalises equal.+    context.insert(MasterPackingItem(name: "socks", person: alice, conditions: .always))+    let source = MasterPackingItem(name: "  Socks ", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let eligible = CopyPackingItemSheet.eligibleTargets(+      source: source,+      people: [owner, alice, bob]+    )++    let ids = Set(eligible.map(\.id))+    // Alice is dropped (same-name owner); Bob remains; owner is always excluded.+    #expect(ids == [bob.id])+    #expect(ids.contains(alice.id) == false)+  }++  @Test("When every other person is ineligible the eligible set is empty (drives 2.5)")+  func everyoneIneligibleYieldsEmpty() throws {+    let container = try Self.makeContainer()+    let context = container.mainContext++    let owner = Person(name: "Owner", colorKey: "blue")+    let alice = Person(name: "Alice", colorKey: "cyan")+    let bob = Person(name: "Bob", colorKey: "orange")+    context.insert(owner)+    context.insert(alice)+    context.insert(bob)+    context.insert(MasterPackingItem(name: "Socks", person: alice, conditions: .always))+    context.insert(MasterPackingItem(name: "SOCKS", person: bob, conditions: .always))+    let source = MasterPackingItem(name: "Socks", person: owner, conditions: .always)+    context.insert(source)+    try context.save()++    let eligible = CopyPackingItemSheet.eligibleTargets(+      source: source,+      people: [owner, alice, bob]+    )++    #expect(eligible.isEmpty)+  }+}
Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swift Added +82 / -0
diff --git a/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swift b/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swiftnew file mode 100644index 0000000..800a78d--- /dev/null+++ b/Scramble/ScrambleUITests/CopyMasterPackingItemUITests.swift@@ -0,0 +1,82 @@+import XCTest++/// Task 9 — happy-path coverage for the "Copy to people…" flow on the Master+/// Lists packing list. Seeds two people (Alex owns "Socks"; Sam owns nothing),+/// reveals the per-row copy action, selects Sam, confirms, and asserts the+/// sheet dismisses and the confirmation toast names Sam (Req 1.1 / 1.2 / 5.1).+/// Driven entirely through the `accessibilityIdentifier`s added in Tasks 7/8.+final class CopyMasterPackingItemUITests: XCTestCase {++  override func setUpWithError() throws {+    continueAfterFailure = false+  }++  @MainActor+  private func launchedApp(fixture: String) -> XCUIApplication {+    let app = XCUIApplication()+    app.launchArguments = ["-uitest", "1", "-seed-fixture", fixture]+    app.launch()++    let marker = app.descendants(matching: .any)+      .matching(identifier: "modelStore.in-memory")+      .firstMatch+    XCTAssertTrue(marker.waitForExistence(timeout: 5))+    return app+  }++  @MainActor+  private func openMasterLists(_ app: XCUIApplication) {+    let tab = app.tabBars.buttons["Master Lists"]+    XCTAssertTrue(tab.waitForExistence(timeout: 3))+    tab.tap()+    XCTAssertTrue(app.navigationBars["Master Lists"].waitForExistence(timeout: 3))+  }++  @MainActor+  func testCopyToPersonShowsConfirmationToast() {+    let app = launchedApp(fixture: "master-packing-copy-two-people")+    openMasterLists(app)+    // Packing Items is the default segment.++    let row = app.descendants(matching: .any)+      .matching(identifier: "masterPacking.itemRow.Socks")+      .firstMatch+    XCTAssertTrue(row.waitForExistence(timeout: 3), "Seeded 'Socks' row should be visible")++    // Reveal the trailing swipe action and tap "Copy to people…".+    row.swipeLeft()+    let copyAction = app.buttons["Copy to people…"]+    XCTAssertTrue(+      copyAction.waitForExistence(timeout: 3),+      "Trailing swipe should reveal the 'Copy to people…' action"+    )+    copyAction.tap()++    // The picker presents Sam as an eligible target (Alex is the owner).+    let samRow = app.descendants(matching: .any)+      .matching(identifier: "copyPacking.person.Sam")+      .firstMatch+    XCTAssertTrue(samRow.waitForExistence(timeout: 3), "Sam should be an eligible copy target")+    samRow.tap()++    let confirm = app.buttons["copyPacking.confirm"]+    XCTAssertTrue(confirm.waitForExistence(timeout: 3))+    XCTAssertTrue(confirm.isEnabled, "Confirm should enable once Sam is selected")+    confirm.tap()++    // Assert the toast FIRST: it is set the instant `performCopy` runs (right+    // after dismissing the picker) and self-dismisses after a few seconds, so+    // checking it before the slower dismissal poll avoids racing its timeout.+    let toast = app.staticTexts["Copied to Sam."]+    XCTAssertTrue(+      toast.waitForExistence(timeout: 5),+      "A confirmation toast naming Sam should appear on the list"+    )++    // And the picker is gone (its confirm button no longer exists).+    XCTAssertFalse(+      confirm.exists,+      "The copy sheet should dismiss after confirming"+    )+  }+}
docs/agent-notes/rules-engine.md Modified +1 / -0
diff --git a/docs/agent-notes/rules-engine.md b/docs/agent-notes/rules-engine.mdindex f0620ec..5919ba5 100644--- a/docs/agent-notes/rules-engine.md+++ b/docs/agent-notes/rules-engine.md@@ -42,6 +42,7 @@ The flag is scoped to a single `TripTask` record — deleting on Trip A does not | 5.1 | Trip created | `TripListView.swift` `onSave` closure (create) calls `runner.runForTrip(newTrip)` after `context.save()` | | 5.2 | Trip attributes edited | `TripDetailView.swift` `onSave` closure for the edit sheet, same pattern | | 5.3 | Master item saved/deleted | `MasterTaskEditor.swift` / `MasterPackingEditor.swift` `runEngineAndDismiss()` calls `runner.runForAllNonPastTrips()` |+| 5.3 | Master packing item copied to other people | `MasterPackingList.swift` `performCopy(...)` also calls `runner.runForAllNonPastTrips()` after the copy save (copy-master-packing-items flow) | | 5.4 | Cold launch | `ScrambleApp.init()` — runs after `UITestSeed.applyIfRequested(...)`, before `WindowGroup` mounts. Synchronous on purpose to satisfy the Phase 1 auto-open ordering invariant (Phase 1 AC 5.6). | | 5.7 | Foreground transition | `RootView.swift` `onChange(of: scenePhase)` with a `hasBeenBackgrounded: Bool` latch (see Decision 9) | 

Things to double-check

Run make test in a clean environment before merge.

This worktree’s simulator test-host has a launch-crash flake that makes xcodebuild’s xcresult aggregate report misleading failures while the action exits 0. The new Copy/eligibility/outcome suites and the UI test were confirmed green on a single booted simulator, but a full clean-environment make test is advisable to rule out interaction with untouched suites.

Toast duration deviation.

The copy confirmation toast uses duration: 5 vs the app-wide 3s default — deliberate (so it isn’t lost during picker dismissal) and commented at the call site.