commit e2d863acabce19c1e224da2dc3fb386547482d1c
parent 0ccbe9098e70b00874bf830af231e03501d6b7eb
Author: Michael Camilleri <[email protected]>
Date: Tue, 9 Jun 2026 09:31:30 +0900
Recompute the read horizon from sibling leases
The previous commit (0ccbe90) stopped a stale sibling record from
collapsing an active future lease by refusing, in setReadCursor, any
incoming readAt below an unexpired horizon. But that floor cannot tell a
stale echo from a legitimate clean close: both arrive as 'a sibling
published a value below the current horizon'. The direct .currentTime
close only fixes the leaving device's own store, not its siblings.
The discriminator is not the incoming value but whether any other
same-account device still leases above it — and that already lives in
the per-device PlayerEntity.readAt rows (the same leases peer-presence
is computed from). This commit drops the floor and instead recomputes
the account horizon in noteIncomingReadCursor as the max over our own
devices' Player leases (the incoming sibling's row is updated by
applyPlayerRecord first). Being a max, a stale echo can never pull the
horizon below an active sibling's lease; a clean close lowers it only
once no sibling leases above the present. The recompute is floored at
the elapsed portion of the current horizon, so a lagging local row can't
re-surface read moves, and at the incoming readAt, so a forward lease
from a not-yet-synced accountSeen push still advances it.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
3 files changed, 110 insertions(+), 45 deletions(-)
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -975,31 +975,51 @@ final class GameStore {
return try context.fetch(request).first
}
- /// Applies this account's `Player.readAt` value from a sibling device.
- /// 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.
+ /// Applies this account's `Player.readAt` from a sibling device. The account
+ /// read horizon is the furthest-ahead lease across our own devices, so it is
+ /// recomputed as the max over every same-account `PlayerEntity.readAt` row
+ /// (the incoming sibling's row was already updated by `applyPlayerRecord`)
+ /// rather than blindly adopting this one value. Because it is a max, a leaving
+ /// device's stale echo can never pull the horizon below an active sibling's
+ /// lease; a clean close only lowers it once no sibling still leases above it.
+ /// The recompute is floored at the already-elapsed portion of the current
+ /// horizon — so a lagging local row can't re-surface moves we have read —
+ /// and at `readAt` itself, so a forward lease from a not-yet-synced record
+ /// (an `accountSeen` push) still advances it.
func noteIncomingReadCursor(gameID: UUID, readAt: Date) {
- _ = setReadCursor(gameID: gameID, readAt: readAt, protectActiveLease: true)
+ let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ request.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
+ request.fetchLimit = 1
+ guard let entity = try? context.fetch(request).first else { return }
+
+ var horizon = readAt
+ if let current = entity.lastReadOtherMoveAt {
+ // The elapsed portion of an active lease is "already read"; only its
+ // future tail is presence that a recompute may drop.
+ horizon = max(horizon, min(current, Date()))
+ }
+ if let localAuthorID = authorIDProvider(), !localAuthorID.isEmpty {
+ let players = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity")
+ players.predicate = NSPredicate(
+ format: "game == %@ AND authorID == %@", entity, localAuthorID
+ )
+ if let maxSiblingReadAt = (try? context.fetch(players))?
+ .compactMap(\.readAt).max() {
+ horizon = max(horizon, maxSiblingReadAt)
+ }
+ }
+ _ = setReadCursor(gameID: gameID, readAt: horizon)
}
/// 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,
- protectActiveLease: Bool = false
+ minimumExistingReadAt: Date? = nil
) -> Bool {
let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
request.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
@@ -1012,12 +1032,6 @@ 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,11 +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. 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.
+ // genuinely stale fetches. Adopting the cursor here is *not* monotonic:
+ // `noteIncomingReadCursor` recomputes the account horizon as the max over
+ // our devices' Player leases, so a leaving sibling's past horizon cannot
+ // collapse another sibling's live presence lease, yet a clean close still
+ // lowers it once no sibling leases above it.
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
@@ -77,6 +77,25 @@ struct GameStoreUnreadMovesTests {
try ctx.save()
}
+ @discardableResult
+ private func addPlayerRow(
+ for entity: GameEntity,
+ gameID: UUID,
+ authorID: String,
+ recordName: String,
+ readAt: Date?,
+ in ctx: NSManagedObjectContext
+ ) throws -> PlayerEntity {
+ let player = PlayerEntity(context: ctx)
+ player.game = entity
+ player.authorID = authorID
+ player.ckRecordName = recordName
+ player.updatedAt = Date()
+ player.readAt = readAt
+ try ctx.save()
+ return player
+ }
+
@Test("Other-author Moves update marks the shared game unread")
func otherAuthorMoveMarksSharedGameUnread() throws {
let persistence = makeTestPersistence()
@@ -303,11 +322,15 @@ struct GameStoreUnreadMovesTests {
#expect(!summary.hasUnreadOtherMoves)
}
- @Test("A sibling's readAt sets the cursor but cannot collapse an active lease")
+ @Test("A sibling's readAt recomputes the horizon from sibling leases")
func incomingReadCursorSetsBadgeHorizon() throws {
let persistence = makeTestPersistence()
- let store = makeTestStore(persistence: persistence)
- let (entity, gameID) = try makeSharedGame(in: persistence.viewContext)
+ let store = makeTestStore(
+ persistence: persistence,
+ authorIDProvider: { Self.localAuthorID }
+ )
+ let ctx = persistence.viewContext
+ let (entity, gameID) = try makeSharedGame(in: ctx)
let earlier = Date(timeIntervalSinceNow: -30)
let later = Date(timeIntervalSinceNow: -10)
@@ -317,7 +340,7 @@ struct GameStoreUnreadMovesTests {
gameID: gameID,
authorID: Self.otherAuthorID,
updatedAt: later,
- in: persistence.viewContext
+ in: ctx
)
store.noteIncomingMovesUpdate(
gameIDs: [gameID],
@@ -326,36 +349,64 @@ struct GameStoreUnreadMovesTests {
#expect(entity.lastReadOtherMoveAt == nil)
#expect(store.unreadOtherMovesGameCount() == 1)
- // A sibling read up to `earlier` — older than the latest move, so the
- // badge still fires but the floor moves forward.
+ // `applyPlayerRecord` updates the publishing device's Player row before
+ // invoking the cursor callback, so the test mirrors that ordering: it
+ // moves device A's row, then notifies the store.
+ let deviceA = try addPlayerRow(
+ for: entity,
+ gameID: gameID,
+ authorID: Self.localAuthorID,
+ recordName: "player-A",
+ readAt: earlier,
+ in: ctx
+ )
+
+ // A read up to `earlier` — older than the latest move, badge still fires.
store.noteIncomingReadCursor(gameID: gameID, readAt: earlier)
#expect(entity.lastReadOtherMoveAt == earlier)
#expect(store.unreadOtherMovesGameCount() == 1)
- // A sibling read up to `later` — catches the latest, badge clears.
+ // A reads up to `later` — catches the latest move, badge clears.
+ deviceA.readAt = later
+ try ctx.save()
store.noteIncomingReadCursor(gameID: gameID, readAt: later)
#expect(entity.lastReadOtherMoveAt == later)
#expect(store.unreadOtherMovesGameCount() == 0)
- // Active reads can lease the cursor into the future.
+ // A opens an active session and leases the horizon into the future.
+ deviceA.readAt = future
+ try ctx.save()
store.noteIncomingReadCursor(gameID: gameID, readAt: future)
#expect(entity.lastReadOtherMoveAt == future)
- #expect(store.unreadOtherMovesGameCount() == 0)
- // 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)
+ // Device B is also actively present (its own future lease). A then leaves
+ // and publishes the current time. A's now-past horizon must NOT collapse
+ // the account horizon — B still leases above it.
+ let deviceB = try addPlayerRow(
+ for: entity,
+ gameID: gameID,
+ authorID: Self.localAuthorID,
+ recordName: "player-B",
+ readAt: future,
+ in: ctx
+ )
+ deviceA.readAt = later
+ try ctx.save()
+ store.noteIncomingReadCursor(gameID: gameID, readAt: later)
#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)
+ // B closes too. With no sibling leasing above the present, the clean
+ // close is honored: the horizon drops out of the future (down to the
+ // already-elapsed floor), so a *later* peer move would badge again while
+ // moves up to now stay read.
+ deviceB.readAt = later
+ try ctx.save()
+ store.noteIncomingReadCursor(gameID: gameID, readAt: later)
+ let horizon = try #require(entity.lastReadOtherMoveAt)
+ #expect(horizon < future)
+ #expect(horizon >= later)
+ #expect(store.unreadOtherMovesGameCount() == 0)
}
@Test("Active read leases refresh only when the horizon is below the floor")
