commit 0ccbe9098e70b00874bf830af231e03501d6b7eb
parent 3835f0f66a91a515ccf1a10142b7633df4c0fb56
Author: Michael Camilleri <[email protected]>
Date: Tue, 9 Jun 2026 08:18:39 +0900
Refuse to collapse an active read lease from a sibling horizon
The catch-up-baseline fix (3835f0f) hoisted the read-cursor adoption
above the selection's updatedAt LWW guard in applyPlayerRecord and
justified it by claiming the game-level cursor 'stays monotonic
downstream in setReadCursor'. That claim is wrong:
noteIncomingReadCursor calls setReadCursor without a floor, and
setReadCursor deliberately allows the horizon to move backward. So a
leaving sibling's stale Player record — adopted for its readAt even when
its selection loses the LWW — could drag GameEntity.lastReadOtherMoveAt
back below another sibling's unexpired future lease, reopening the
unread-badge / pause-push window until the active device republished.
The same collapse was already reachable through the accountSeen push and
the realtime cursor drain, which also funnel into
noteIncomingReadCursor.
This commot adds a protectActiveLease floor to setReadCursor: a write
that would move the horizon backward below an unexpired future lease is
refused. The sole sibling-horizon path, noteIncomingReadCursor, now
passes it, closing the collapse across all three routes at once. The
active device still closes its own lease through the direct .currentTime
setReadCursor path, which does not set the flag; once a lease lapses
into the past, sibling horizons move the cursor freely again.
Finally, the now-inaccurate rationale in applyPlayerRecord is corrected:
cursor adoption is not monotonic by design, and its safety rests on the
lease floor rather than a downstream monotonicity that does not exist.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
3 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -976,22 +976,30 @@ final class GameStore {
}
/// Applies this account's `Player.readAt` value from a sibling device.
- /// The sync layer has already accepted the Player record under its normal
- /// last-writer-wins freshness checks, so the value is allowed to move
- /// backward when an active read lease is closed with a current-time write.
+ /// The horizon may move backward — a closed session ships a past readAt —
+ /// but never below an *unexpired* future lease: that lease asserts another
+ /// sibling is actively present, and a leaving device's stale horizon write
+ /// must not collapse it (which would reopen the unread badge / pause-push
+ /// window until the active device republishes). `protectActiveLease` enforces
+ /// that floor; the active device closes its own lease through the direct
+ /// `.currentTime` `setReadCursor` path instead.
func noteIncomingReadCursor(gameID: UUID, readAt: Date) {
- _ = setReadCursor(gameID: gameID, readAt: readAt)
+ _ = setReadCursor(gameID: gameID, readAt: readAt, protectActiveLease: true)
}
/// Sets the per-account read horizon for other-author moves. When
/// `minimumExistingReadAt` is provided, the write is skipped if the
/// current horizon already reaches that floor; active sessions use this
/// to refresh a future read lease only when it is getting close to expiry.
+ /// When `protectActiveLease` is set, the write is skipped if it would move
+ /// the horizon backward below an unexpired future lease — a sibling's stale
+ /// horizon must not collapse another sibling's live presence lease.
@discardableResult
func setReadCursor(
gameID: UUID,
readAt: Date,
- minimumExistingReadAt: Date? = nil
+ minimumExistingReadAt: Date? = nil,
+ protectActiveLease: Bool = false
) -> Bool {
let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
request.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
@@ -1004,6 +1012,12 @@ final class GameStore {
current >= minimumExistingReadAt {
return false
}
+ if protectActiveLease,
+ let current = entity.lastReadOtherMoveAt,
+ current > Date(),
+ readAt < current {
+ return false
+ }
guard entity.lastReadOtherMoveAt != readAt else { return false }
entity.lastReadOtherMoveAt = readAt
saveContext("updateReadAt")
diff --git a/Crossmate/Sync/RecordApplier.swift b/Crossmate/Sync/RecordApplier.swift
@@ -230,8 +230,11 @@ extension SyncEngine {
// ships a stale `updatedAt`. Gating these on it would let a device with a
// fresher local cursor drop the baseline and re-report the same moves as
// a duplicate catch-up banner. The etag guard above already rejects
- // genuinely stale fetches, and the game-level cursor stays monotonic
- // downstream (`setReadCursor`), so adopting here is safe.
+ // genuinely stale fetches. Adopting the cursor here is *not* monotonic —
+ // `noteIncomingReadCursor` deliberately lets a sibling horizon move the
+ // game cursor in either direction — but `setReadCursor` refuses to shrink
+ // an unexpired future lease, so a leaving sibling's past horizon cannot
+ // collapse another sibling's live presence lease.
let incomingReadAt = RecordSerializer.parsePlayerReadAt(from: record)
entity.readAt = incomingReadAt
entity.sessionSnapshot = RecordSerializer.parsePlayerSessionSnapshot(from: record)
diff --git a/Tests/Unit/GameStoreUnreadMovesTests.swift b/Tests/Unit/GameStoreUnreadMovesTests.swift
@@ -303,7 +303,7 @@ struct GameStoreUnreadMovesTests {
#expect(!summary.hasUnreadOtherMoves)
}
- @Test("A sibling's readAt sets the cursor and can close a future lease")
+ @Test("A sibling's readAt sets the cursor but cannot collapse an active lease")
func incomingReadCursorSetsBadgeHorizon() throws {
let persistence = makeTestPersistence()
let store = makeTestStore(persistence: persistence)
@@ -342,9 +342,17 @@ struct GameStoreUnreadMovesTests {
#expect(entity.lastReadOtherMoveAt == future)
#expect(store.unreadOtherMovesGameCount() == 0)
- // Closing that lease is allowed to move the cursor backward. The sync
- // layer only calls this after accepting the Player record under LWW
- // freshness checks, so the store applies the value directly.
+ // A sibling's stale horizon must not collapse an unexpired future
+ // lease — the lease asserts another sibling is actively present, so the
+ // backward write is refused and the badge stays clear.
+ store.noteIncomingReadCursor(gameID: gameID, readAt: earlier)
+ #expect(entity.lastReadOtherMoveAt == future)
+ #expect(store.unreadOtherMovesGameCount() == 0)
+
+ // Once the lease has lapsed into the past (the active device closes it
+ // through the direct `setReadCursor` path), a sibling horizon is free to
+ // move the cursor backward again — a closed-session bump.
+ #expect(store.setReadCursor(gameID: gameID, readAt: later))
store.noteIncomingReadCursor(gameID: gameID, readAt: earlier)
#expect(entity.lastReadOtherMoveAt == earlier)
#expect(store.unreadOtherMovesGameCount() == 1)
