commit 1d68ac83a5a2d52d9818fe7edbdb0db766858ac7
parent e2d863acabce19c1e224da2dc3fb386547482d1c
Author: Michael Camilleri <[email protected]>
Date: Tue, 9 Jun 2026 10:44:32 +0900
Reassert the active read lease after a sibling collapse
The read horizon and the 'a device is actively present' lease share one
last-writer-wins scalar on a single Player record per (game, account).
Two prior attempts tried to stop a leaving device from collapsing
another device's active lease purely from that scalar -- a sibling-row
recompute (e2d863a) and an unexpired-future floor (0ccbe90). Neither can
work: with one row per author there is no per-device lease to recompute
or protect, so the recompute degenerated to the single LWW row and the
floor held stale leases by treating a real close as a stale echo.
This commit accepts the LWW model in GameStore and move the correction
to where the information actually exists. noteIncomingReadCursor goes
back to plain adopt. AppServices, which knows app foreground state and
the puzzle on screen, watches the incoming-cursor drain: when a sibling
publishes a past-dated readAt (a session close) while this device is
still viewing that same puzzle, it immediately reasserts its own
.activeLease instead of waiting up to readLeaseRefreshFloor for the next
renewal tick. This shortens the collapse window from minutes to one sync
round trip; it is self-damping because future-dated echoes never
retrigger it.
The reassert is decided before an await, so the user can leave the
puzzle in the gap and strand a future lease on a game no one is viewing
-- the same stuck-lease symptom the floor produced. publishReadCursor
gains an opt-in requireActivePuzzle guard that re-reads the strict
activePuzzleID (no leave-grace tail) synchronously in the write's
critical section, so a concurrent leave deterministically wins. Existing
callers are unaffected.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
3 files changed, 53 insertions(+), 98 deletions(-)
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -975,40 +975,18 @@ final class GameStore {
return try context.fetch(request).first
}
- /// 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.
+ /// Applies this account's `Player.readAt` from a sibling device under
+ /// last-writer-wins: SyncEngine has already accepted this record as the
+ /// freshest server version, so adopt the value directly as the account's
+ /// resolved read horizon. A leaving sibling can pull the horizon back below
+ /// an active sibling's future lease, but that collapse is bounded and
+ /// self-healing: the still-present device re-asserts its lease as soon as it
+ /// processes the inbound close (`AppServices`'s incoming-cursor drain), and a
+ /// foreground device marks inbound peer moves read on arrival regardless.
+ /// Representing "A left while C is still here" without that collapse would
+ /// require per-device Player rows, which do not exist (one row per author).
func noteIncomingReadCursor(gameID: UUID, readAt: Date) {
- 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)
+ _ = setReadCursor(gameID: gameID, readAt: readAt)
}
/// Sets the per-account read horizon for other-author moves. When
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -504,6 +504,20 @@ final class AppServices {
seenAt: readAt,
publishAccountSeen: false
)
+ } else if NotificationState.activePuzzleID() == gameID {
+ // A past-dated readAt is a sibling closing its session, which
+ // under last-writer-wins just pulled the shared account
+ // horizon back to that close time. This device is still
+ // actively viewing the same puzzle, so it still holds a
+ // presence lease — re-assert it now instead of waiting up to
+ // `readLeaseRefreshFloor` (5 min) for the next renewal tick.
+ // `requireActivePuzzle` re-checks inside the write so a leave
+ // racing this inbound can't strand a stale future lease.
+ await self?.publishReadCursor(
+ for: gameID,
+ mode: .activeLease,
+ requireActivePuzzle: true
+ )
}
}
}
@@ -3212,7 +3226,8 @@ final class AppServices {
func publishReadCursor(
for gameID: UUID,
- mode: ReadCursorPublishMode = .activeLease
+ mode: ReadCursorPublishMode = .activeLease,
+ requireActivePuzzle: Bool = false
) async {
guard let authorID = identity.currentID, !authorID.isEmpty else { return }
// A read lease asserts "the user is actively present on this puzzle," so
@@ -3225,6 +3240,19 @@ final class AppServices {
syncMonitor.note("readCursor(activeLease) skipped for \(gameID.uuidString): backgrounded")
return
}
+ // A re-assert triggered by an inbound sibling close (`requireActivePuzzle`)
+ // is decided before an `await`, so the user may have left the puzzle in
+ // the gap. Re-check here, synchronously in the write's critical section,
+ // that this is still the puzzle on screen. The strict `activePuzzleID`
+ // (no leave-grace tail) flips synchronously on `.onDisappear`/`.background`,
+ // so a concurrent leave deterministically wins and we never strand a
+ // future lease on a puzzle no device is actually viewing.
+ if case .activeLease = mode,
+ requireActivePuzzle,
+ NotificationState.activePuzzleID() != gameID {
+ syncMonitor.note("readCursor(activeLease) skipped for \(gameID.uuidString): not active puzzle")
+ return
+ }
let now = Date()
let didUpdate: Bool
switch mode {
diff --git a/Tests/Unit/GameStoreUnreadMovesTests.swift b/Tests/Unit/GameStoreUnreadMovesTests.swift
@@ -77,25 +77,6 @@ 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()
@@ -322,7 +303,7 @@ struct GameStoreUnreadMovesTests {
#expect(!summary.hasUnreadOtherMoves)
}
- @Test("A sibling's readAt recomputes the horizon from sibling leases")
+ @Test("A sibling's readAt is adopted last-writer-wins")
func incomingReadCursorSetsBadgeHorizon() throws {
let persistence = makeTestPersistence()
let store = makeTestStore(
@@ -349,63 +330,31 @@ struct GameStoreUnreadMovesTests {
#expect(entity.lastReadOtherMoveAt == nil)
#expect(store.unreadOtherMovesGameCount() == 1)
- // `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.
+ // All of an account's devices share one Player record, so the inbound
+ // `readAt` is the account's resolved horizon: adopt it verbatim.
+ // A sibling read up to `earlier` — older than the latest move, badge fires.
store.noteIncomingReadCursor(gameID: gameID, readAt: earlier)
#expect(entity.lastReadOtherMoveAt == earlier)
#expect(store.unreadOtherMovesGameCount() == 1)
- // A reads up to `later` — catches the latest move, badge clears.
- deviceA.readAt = later
- try ctx.save()
+ // A sibling reads up to `later` — catches the latest move, badge clears.
store.noteIncomingReadCursor(gameID: gameID, readAt: later)
#expect(entity.lastReadOtherMoveAt == later)
#expect(store.unreadOtherMovesGameCount() == 0)
- // A opens an active session and leases the horizon into the future.
- deviceA.readAt = future
- try ctx.save()
+ // A sibling opens an active session and leases the horizon into the
+ // future; the lease is adopted and keeps the badge suppressed.
store.noteIncomingReadCursor(gameID: gameID, readAt: future)
#expect(entity.lastReadOtherMoveAt == future)
-
- // 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)
- // 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()
+ // That sibling leaves and publishes the current time. Under
+ // last-writer-wins the single shared scalar simply moves back to the
+ // close value — there is no per-device lease here to protect it. The
+ // collapse is bounded and self-healing: a still-present device re-asserts
+ // its own lease from `AppServices`'s incoming-cursor drain, not here.
store.noteIncomingReadCursor(gameID: gameID, readAt: later)
- let horizon = try #require(entity.lastReadOtherMoveAt)
- #expect(horizon < future)
- #expect(horizon >= later)
+ #expect(entity.lastReadOtherMoveAt == later)
#expect(store.unreadOtherMovesGameCount() == 0)
}
