Fix rare duplicate cards caused by stale selections

ComputerSolitaire/Game/Shared/GamePersistence.swift

@@ -908,12 +908,6 @@ enum GameStatisticsStore {
 }
 
 nonisolated private extension GameState {
-    var allCards: [Card] {
-        stock + waste + freeCells.compactMap { $0 } + foundations.flatMap { $0 }
-            + tableau.flatMap { $0 } + pyramid.compactMap { $0 } + discard
-            + triPeaks.compactMap { $0 } + reserve
-    }
-
     var isValidForPersistence: Bool {
         guard foundations.count == variant.foundationPileCount else { return false }
         guard freeCells.count == 4 else { return false }

ComputerSolitaire/Game/Shared/GameSession.swift

@@ -746,6 +746,12 @@ final class SolitaireViewModel {
     }
 
     func refreshAutoFinishAvailability() {
+        // Every mutating flow ends here, making it the shared checkpoint for
+        // the deck integrity invariant.
+        assert(
+            state.hasNoDuplicateCardIDs,
+            "Board corruption: a card instance appears more than once"
+        )
         isAutoFinishAvailable = AutoFinishPlanner.canAutoFinish(in: state)
         isHintAvailable = !isWin && HintAdvisor.anyPlayerMoveExists(in: state)
     }
@@ -935,7 +941,21 @@ extension SolitaireViewModel {
     }
 
     func tryMoveSelection(to destination: Destination) -> Bool {
-        guard let selection, let movingCard = selection.cards.first else { return false }
+        guard let staleSelection = selection else { return false }
+        // A selection is a snapshot of the board, and moves apply only after an
+        // animated flight, so the board may have moved on beneath it (for
+        // example, a tap queued during another move's drop flight). Applying a
+        // stale selection duplicates cards: `removeSelection` removes by
+        // position while the destination receives the snapshot's copies.
+        // Re-derive the cards from the current state and refuse on mismatch.
+        guard let selection = liveSelection(matching: staleSelection),
+              let movingCard = selection.cards.first else {
+            self.selection = nil
+            return false
+        }
+        if selection != staleSelection {
+            self.selection = selection
+        }
 
         switch destination {
         case .foundation(let index):
@@ -1019,6 +1039,39 @@ extension SolitaireViewModel {
         }
     }
 
+    /// Re-derives `selection` from the current state, returning fresh copies
+    /// of the cards its source holds right now. Returns nil when the source no
+    /// longer holds exactly the selected cards — the selection predates a
+    /// mutation, and applying it would corrupt the board (see
+    /// `tryMoveSelection`).
+    func liveSelection(matching selection: Selection) -> Selection? {
+        let liveCards: [Card]?
+        switch selection.source {
+        case .waste:
+            liveCards = state.waste.last.map { [$0] }
+        case .foundation(let pile):
+            guard state.foundations.indices.contains(pile) else { return nil }
+            liveCards = state.foundations[pile].last.map { [$0] }
+        case .freeCell(let slot):
+            guard state.freeCells.indices.contains(slot) else { return nil }
+            liveCards = state.freeCells[slot].map { [$0] }
+        case .tableau(let pile, let index):
+            guard state.tableau.indices.contains(pile),
+                  state.tableau[pile].indices.contains(index) else { return nil }
+            liveCards = Array(state.tableau[pile][index...])
+        case .pyramid(let index):
+            guard state.pyramid.indices.contains(index) else { return nil }
+            liveCards = state.pyramid[index].map { [$0] }
+        case .triPeaks(let index):
+            guard state.triPeaks.indices.contains(index) else { return nil }
+            liveCards = state.triPeaks[index].map { [$0] }
+        case .reserve:
+            liveCards = state.reserve.last.map { [$0] }
+        }
+        guard let liveCards, liveCards.map(\.id) == selection.cards.map(\.id) else { return nil }
+        return Selection(source: selection.source, cards: liveCards)
+    }
+
     func removeSelection(_ selection: Selection) {
         switch selection.source {
         case .waste:

ComputerSolitaire/Game/Shared/GameState.swift

@@ -111,6 +111,22 @@ nonisolated struct GameState: Equatable, Codable {
         }
     }
 
+    /// Every card currently in play, across all of the variant containers.
+    var allCards: [Card] {
+        stock + waste + freeCells.compactMap { $0 } + foundations.flatMap { $0 }
+            + tableau.flatMap { $0 } + pyramid.compactMap { $0 } + discard
+            + triPeaks.compactMap { $0 } + reserve
+    }
+
+    /// Whether no card instance appears twice on the board. Deliberately
+    /// weaker than the full-deck composition check persistence applies, so
+    /// partially dealt fixture states still satisfy it; a violation always
+    /// means a move-application bug duplicated a card.
+    var hasNoDuplicateCardIDs: Bool {
+        let cards = allCards
+        return Set(cards.map(\.id)).count == cards.count
+    }
+
     static func newGame() -> GameState {
         newGame(variant: .klondike)
     }

ComputerSolitaire/Views/Shared/ContentView.swift

@@ -1344,13 +1344,18 @@ struct ContentView: View {
         guard !viewModel.isDragging else { return }
 
         viewModel.clearPendingAutoMove()
+        // The request holds a snapshot from tap time; a move that landed while
+        // it waited (its tap arrived during a drop flight) may have moved the
+        // board on beneath it. The session would refuse the stale move anyway;
+        // dropping the request here also skips its flight animation.
+        guard let liveSelection = viewModel.liveSelection(matching: request.selection) else { return }
         drag.dragTranslation = .zero
         dragReturnOffset = .zero
         drag.setActiveTarget(nil)
-        viewModel.selection = request.selection
+        viewModel.selection = liveSelection
         viewModel.isDragging = true
 
-        if let firstCard = request.selection.cards.first {
+        if let firstCard = liveSelection.cards.first {
             overlayTilt = cardTilts[firstCard.id] ?? 0
             let tiltSettleDuration = isAutoFinishing ? 0.1 : 0.15
             withAnimation(motion.easeOut(tiltSettleDuration)) {

ComputerSolitaireTests/Shared/StaleSelectionMoveTests.swift

@@ -0,0 +1,149 @@
+import XCTest
+@testable import Computer_Solitaire
+
+/// Regression coverage for the duplicate-card corruption: moves apply after an
+/// animated drop flight, so a selection queued during that window snapshots the
+/// pre-move board — including the in-flight card. Applying such a stale
+/// selection used to remove cards by position while appending the snapshot's
+/// copies, leaving the same card on the board twice. `tryMoveSelection` now
+/// re-derives every selection from the live state and refuses on mismatch.
+@MainActor
+final class StaleSelectionMoveTests: XCTestCase {
+    private func spadeRun(through rank: Rank) -> [Card] {
+        Rank.allCases
+            .filter { $0 <= rank }
+            .map { Card(suit: .spades, rank: $0, isFaceUp: true) }
+    }
+
+    private func allBoardCards(in viewModel: SolitaireViewModel) -> [Card] {
+        viewModel.state.allCards
+    }
+
+    /// The confirmed corruption sequence, driven in the exact order
+    /// ContentView does: tap 7♠ (auto-move to foundation queued, flight
+    /// starts), tap the 8♥ beneath it during the flight (queues a stale
+    /// [8♥, 7♠] stack), land flight one, then process the stale request.
+    func testStaleSelectionQueuedDuringDropFlightIsRefused() {
+        let viewModel = SolitaireViewModel(variant: .klondike)
+
+        let eightOfHearts = Card(suit: .hearts, rank: .eight, isFaceUp: true)
+        let sevenOfSpades = Card(suit: .spades, rank: .seven, isFaceUp: true)
+        let nineOfClubs = Card(suit: .clubs, rank: .nine, isFaceUp: true)
+
+        var state = viewModel.state
+        state.foundations = [spadeRun(through: .six), [], [], []]
+        state.tableau = [
+            [eightOfHearts, sevenOfSpades],
+            [nineOfClubs],
+            [], [], [], [], []
+        ]
+        state.stock = []
+        state.waste = []
+        state.wasteDrawCount = 0
+        viewModel.state = state
+
+        // Tap 1: queues the 7♠'s auto-move; no model mutation yet.
+        viewModel.handleTableauTap(pileIndex: 0, cardIndex: 1)
+        guard let firstRequest = viewModel.pendingAutoMove else {
+            return XCTFail("expected first auto-move to queue")
+        }
+        XCTAssertEqual(firstRequest.destination, .foundation(0))
+
+        // ContentView installs the selection and defers handleDrop for the
+        // drop flight.
+        viewModel.clearPendingAutoMove()
+        viewModel.selection = firstRequest.selection
+        viewModel.isDragging = true
+
+        // Tap 2 lands mid-flight: the pile still holds the in-flight 7♠, so
+        // the queued request snapshots the stale [8♥, 7♠] stack.
+        viewModel.handleTableauTap(pileIndex: 0, cardIndex: 0)
+        guard let secondRequest = viewModel.pendingAutoMove else {
+            return XCTFail("expected second auto-move to queue")
+        }
+        XCTAssertEqual(secondRequest.destination, .tableau(1))
+
+        // Flight one lands and applies its move.
+        XCTAssertTrue(viewModel.handleDrop(to: firstRequest.destination))
+        XCTAssertEqual(viewModel.state.foundations[0].last?.id, sevenOfSpades.id)
+
+        // The stale request must be refused at both defense layers: the
+        // re-derivation ContentView performs, and the session funnel itself.
+        viewModel.clearPendingAutoMove()
+        XCTAssertNil(viewModel.liveSelection(matching: secondRequest.selection))
+        viewModel.selection = secondRequest.selection
+        XCTAssertFalse(viewModel.handleDrop(to: secondRequest.destination))
+        XCTAssertNil(viewModel.selection)
+
+        // The board must be untouched by the refused move.
+        XCTAssertEqual(viewModel.state.tableau[0].map(\.id), [eightOfHearts.id])
+        XCTAssertEqual(viewModel.state.tableau[1].map(\.id), [nineOfClubs.id])
+        let allCards = allBoardCards(in: viewModel)
+        XCTAssertEqual(allCards.filter { $0.id == sevenOfSpades.id }.count, 1)
+        XCTAssertEqual(allCards.filter { $0.id == eightOfHearts.id }.count, 1)
+        XCTAssertTrue(viewModel.state.hasNoDuplicateCardIDs)
+    }
+
+    /// The waste variant of the race: a waste-top selection held across a
+    /// stock draw must not pop the newly drawn card while appending the old
+    /// top's copy elsewhere.
+    func testWasteSelectionHeldAcrossStockDrawIsRefused() {
+        let viewModel = SolitaireViewModel(variant: .klondike)
+        viewModel.setStockDrawCount(1)
+
+        let sevenOfSpades = Card(suit: .spades, rank: .seven, isFaceUp: true)
+        let queenOfDiamonds = Card(suit: .diamonds, rank: .queen)
+        let eightOfHearts = Card(suit: .hearts, rank: .eight, isFaceUp: true)
+
+        var state = viewModel.state
+        state.foundations = Array(repeating: [], count: 4)
+        state.tableau = [[eightOfHearts], [], [], [], [], [], []]
+        state.stock = [queenOfDiamonds]
+        state.waste = [sevenOfSpades]
+        state.wasteDrawCount = 1
+        viewModel.state = state
+
+        // Selection captured from the waste top, then a draw lands beneath it
+        // before the deferred move applies.
+        viewModel.selection = Selection(source: .waste, cards: [sevenOfSpades])
+        viewModel.drawFromStock()
+        XCTAssertEqual(viewModel.state.waste.last?.id, queenOfDiamonds.id)
+
+        XCTAssertFalse(viewModel.handleDrop(to: .tableau(0)))
+
+        XCTAssertEqual(
+            viewModel.state.waste.map(\.id),
+            [sevenOfSpades.id, queenOfDiamonds.id]
+        )
+        XCTAssertEqual(viewModel.state.tableau[0].map(\.id), [eightOfHearts.id])
+        XCTAssertTrue(viewModel.state.hasNoDuplicateCardIDs)
+    }
+
+    /// The validation must not get in the way of a normal, up-to-date move.
+    func testFreshSelectionStillMoves() {
+        let viewModel = SolitaireViewModel(variant: .klondike)
+
+        let eightOfHearts = Card(suit: .hearts, rank: .eight, isFaceUp: true)
+        let sevenOfSpades = Card(suit: .spades, rank: .seven, isFaceUp: true)
+
+        var state = viewModel.state
+        state.foundations = Array(repeating: [], count: 4)
+        state.tableau = [[sevenOfSpades], [eightOfHearts], [], [], [], [], []]
+        state.stock = []
+        state.waste = []
+        state.wasteDrawCount = 0
+        viewModel.state = state
+
+        viewModel.selection = Selection(
+            source: .tableau(pile: 0, index: 0),
+            cards: [sevenOfSpades]
+        )
+        XCTAssertTrue(viewModel.handleDrop(to: .tableau(1)))
+        XCTAssertEqual(
+            viewModel.state.tableau[1].map(\.id),
+            [eightOfHearts.id, sevenOfSpades.id]
+        )
+        XCTAssertTrue(viewModel.state.tableau[0].isEmpty)
+        XCTAssertTrue(viewModel.state.hasNoDuplicateCardIDs)
+    }
+}

README.md

@@ -3,7 +3,7 @@
   <br><span style="font-family: monospace;">Computer Solitaire</span>
 </h1>
 
-Computer Solitaire is a fully native Solitaire app for iOS, iPadOS, and macOS. [Find it on the App Store](https://apps.apple.com/us/app/computer-solitaire/id6761316105).
+Computer Solitaire is a fully native Solitaire app for iOS, iPadOS, and macOS. It includes things you enjoy. [Find it on the App Store](https://apps.apple.com/us/app/computer-solitaire/id6761316105).
 
 <p align="center">
   <img src="./docs/screenshots/screen-grab-ios.png" alt="iOS screenshot" height="420" />