crossmate

DesignFlaws.md (103886B)

---

 # Crossmate Audit
 
 A phased audit of the Crossmate codebase. Each phase covers one conceptual
 piece of the app and is reviewed in its own pass, since the whole codebase is
 too large to review in one sitting.
 
 **Focus for every phase:** correctness, security, performance, and code
 quality/style. Findings that don't fit the phase they're discovered in should
 still be recorded under that phase, with a note pointing at the phase they
 actually belong to.
 
 **Out of scope:** `Crossmake/` (separate offline puzzle-authoring SPM
 package, not part of the app build).
 
 **Status legend:** Not started / In progress / Done
 
 ---
 
 ## Phase 1 — CloudKit Sync Engine
 
 **Status:** Done
 
 The core of the app: raw `CKSyncEngine`, per-game zones, record
 serialization/merging, and conflict handling. Highest complexity, highest
 blast radius if wrong.
 
 - `Crossmate/Sync/SyncEngine.swift`
 - `Crossmate/Sync/CloudZones.swift`
 - `Crossmate/Sync/CloudQuery.swift`
 - `Crossmate/Sync/RecordApplier.swift`
 - `Crossmate/Sync/RecordBuilder.swift`
 - `Crossmate/Sync/RecordSerializer.swift`
 - `Crossmate/Sync/GridStateMerger.swift`
 - `Crossmate/Sync/Moves.swift`
 - `Crossmate/Sync/MovesUpdater.swift`
 - `Crossmate/Sync/PeerChangeLedger.swift`
 - `Crossmate/Sync/PlayerSelectionPublisher.swift`
 - `Crossmate/Sync/RecentChanges.swift`
 - `Crossmate/Sync/SessionMonitor.swift`
 - `Crossmate/Sync/Presence.swift`
 - `Crossmate/Sync/AuthorIdentity.swift`
 - `Crossmate/Sync/Archive.swift`
 - `Crossmate/Sync/GameArchiver.swift`
 - `Crossmate/Sync/CloudDiagnostics.swift`
 - `Crossmate/Sync/SyncState+Helpers.swift`
 - `Crossmate/Services/CloudService.swift` (sync composition root)
 - `Tests/Unit/Sync/*`, `Tests/Unit/RecordSerializer*Tests.swift`,
   `Tests/Unit/GridStateMergerTests.swift`, `Tests/Unit/MovesUpdaterTests.swift`,
   `Tests/Unit/PeerChangeLedgerTests.swift`, `Tests/Unit/RecentChangesTests.swift`,
   `Tests/Unit/SyncMonitorTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 Overall this layer is in good shape: the LWW/etag guards, the
 `serverRecordChanged` re-enqueue handling, the burst/drain routing, and the
 zone-orphaning teardown are carefully reasoned and heavily documented. The
 merge/ledger/publisher helpers are clean and well-tested. The items below are
 the exceptions, ordered by practical priority.
 
 **1. (Security — high) Account push address/secret `Decision`s are adopted
 without verifying they came from the private `account` zone.**
 `RecordSerializer.parseAccountPushAddressDecision` /
 `parseAccountPushSecretDecision` (`RecordSerializer.swift:219`, `:245`)
 validate only the record type/name/kind/payload. They do not check the record's
 zone or database scope. `SyncEngine.handleFetchedRecordZoneChanges` calls them
 for every fetched `Decision` record (`SyncEngine.swift:1514`, `:1517`) on both
 the private and shared engines, then dispatches the surfaced values to
 `onAccountPushAddress` / `onAccountPushSecret` (`SyncEngine.swift:1656`,
 `:1666`).
 
 The push secret is the HMAC key used by `deriveGameAddress`, and the comments
 say it should live only in the private account zone. A writable non-account
 zone can therefore carry a record named `decision-account-pushSecret`, type
 `Decision`, `kind=account`, with attacker-chosen payload/version, and this
 device will adopt it. At minimum this can poison push registration and cause a
 push-delivery DoS. The sibling `nickname` decision already gates on
 `record.recordID.zoneID.zoneName == accountZoneID.zoneName && databaseScope == 0`
 (`RecordSerializer.swift:1153`); account push address/secret adoption should
 use the same private-account-zone guard, either in the parsers or at their fetch
 call site. The `serverRecordChanged` conflict path (`SyncEngine.swift:1889`,
 `:1892`) is less exposed because the server record comes from the account-zone
 write being resolved.
 
 **2. (Correctness/data loss — high) `GameArchiver.promoteRevoked` deletes the
 original game even when archive materialization fails.**
 `GameArchiver.promoteRevoked` discards the result of
 `Archive.materialize(payload, in:)` (`GameArchiver.swift:221`) and then
 unconditionally deletes the original `GameEntity` (`:222-227`).
 `Archive.materialize` returns `nil` if `payload.puzzleSource` is empty
 (`Archive.swift:392`). A cloud archive payload can have an empty puzzle source
 if the `CKAsset` read fails, because `Archive.payload(from:)` converts any
 missing/unreadable asset to `""` (`Archive.swift:352-354`).
 
 Failure scenario: a completed shared game is revoked; the archive record fetches
 but the puzzle source asset is unavailable or unreadable; no replacement row is
 created, yet the original row is deleted. Only delete the original after
 materialization succeeds, or fall back to the local snapshot when the cloud
 payload is incomplete.
 
 **3. (Security/performance — medium) `puzzleSource` CKAssets are read without a
 size cap.**
 `RecordSerializer.applyGameRecord` reads the entire `puzzleSource` asset with
 `String(contentsOf: fileURL, encoding: .utf8)` (`RecordSerializer.swift:870-901`).
 Anyone with write access to a shared game zone can cause every syncing device
 to load that asset fully into memory. Crossword sources are inherently small,
 so enforce a conservative byte limit before decoding and reject or ignore
 oversized assets.
 
 **4. (Performance — medium) Sync actors use blocking Core Data work in async
 paths.**
 The sync layer frequently calls `context.performAndWait` from async actor
 methods, including `SyncEngine.handleFetchedRecordZoneChanges`
 (`SyncEngine.swift:1466`), `MovesUpdater.performFlush`
 (`MovesUpdater.swift:158`), and `PlayerSelectionPublisher`
 (`PlayerSelectionPublisher.swift:203`). This may be a deliberate serialization
 tradeoff, but blocking a Swift concurrency worker during Core Data fetch/save
 work can become a scalability problem during bursty sync across many games.
 Consider a systematic pass to move hot paths to `await context.perform { ... }`
 where ordering constraints allow it. Treat this as a performance audit item
 rather than an immediate correctness bug.
 
 **5. (Performance — low/medium) `RecordBuilder.buildRecord` creates a fresh
 background context per outbound record.**
 `RecordBuilder.buildRecord` creates a new background context at
 `RecordBuilder.swift:48` and performs one lookup/build inside it. Because this
 runs once per pending outbound record, draining a large CKSyncEngine batch
 allocates one Core Data context per record. Hoisting context creation to the
 batch caller would reduce overhead and share the row cache across a drain.
 
 **6. (Resource — low) Archive temp files are left for OS cleanup.**
 `Archive.asset(for:ext:)` writes a new file in `FileManager.default.temporaryDirectory`
 for every archive asset (`Archive.swift:300-306`). `GameArchiver.write` can
 re-save archives across retry/reconcile passes, and no path explicitly removes
 these files after CloudKit has consumed them. This is usually bounded by OS temp
 eviction, but a cleanup hook after successful `CKModifyRecordsOperation` would
 avoid accumulating orphaned archive asset files.
 
 **7. (Correctness/resource — low) Private-DB zone deletion does not share the
 full zone-orphan cleanup path.**
 When a private database zone deletion is fetched, `handleFetchedDatabaseChanges`
 deletes the local `GameEntity` (`SyncEngine.swift:1346-1415`) but does not
 remove queued `pendingRecordZoneChanges` or `pendingPings` for that zone.
 `applyZoneOrphaning` does remove both (`SyncEngine.swift:2033-2054`). This
 should self-heal when the queued send later fails with `.zoneNotFound`, but the
 fetch-side deletion path could use the same helper to avoid a futile retry and
 keep transient state consistent.
 
 **8. (Performance — low) `GameArchiver.write` ensures the archive zone before
 every save.**
 `GameArchiver.write` calls `ensureZone(zoneID)` unconditionally
 (`GameArchiver.swift:139-143`). This is idempotent and correct, but can add an
 avoidable CloudKit round trip on repeated archive reconcile passes. Cache known
 archive zones or tolerate `zoneAlreadyExists` after a create-once attempt.
 
 **9. (Correctness/consistency — low) Equal-timestamp move-cell merges use an
 implicit incoming-wins tie-break.**
 `mergeIncomingMovesCells` replaces the local cell whenever
 `existing.updatedAt <= incoming.updatedAt` (`RecordSerializer.swift:1014-1027`).
 `GridStateMerger.shouldReplace` uses a deterministic tie-break on author and
 device after timestamp equality (`GridStateMerger.swift:78-86`). This merge is
 scoped to a single `(author, device)` moves row, so impact is likely low, but
 aligning the tie-breaks would reduce surprise if the helper is reused.
 
 **10. (Code quality — low) `applyPlayerRecord` drops the whole record when
 `name` is absent.**
 `RecordApplier.applyPlayerRecord` returns early if `record["name"]` is missing
 (`RecordApplier.swift:231`), which also drops selection, presence, read cursor,
 and time-log fields. Today `playerRecord` always writes `name`, so this is a
 future partial-fetch sharp edge rather than a live bug.
 
 **11. (Code quality — low) `MovesUpdater.enqueue(actingAuthorID:)` is a dead
 parameter.**
 `GameMutator` passes `actingAuthorID` into `MovesUpdater.enqueue`
 (`GameMutator.swift:371`), but `MovesUpdater.enqueue` ignores the parameter
 (`MovesUpdater.swift:77`). Journaling attribution is handled separately in
 `GameMutator`, so either remove the parameter or wire it to behavior that needs
 it.
 
 **Non-findings and deferred checks:**
 - `serverRecordChanged` re-enqueue handling appears correct: the
   `recoverServerChangedSave` and `recoveredSaves`/`decisionWins` paths
   explicitly re-add `.saveRecord` after adopting server system fields.
 - `CloudQuery` desired-key lists are centralized in `RecordSerializer` and
   cross-checked against written fields; the old duplicated-list footgun is not
   present here.
 - `GridStateMerger`'s primary merge logic is deterministic and covered by
   tests.
 - Record-name parsers reject malformed names rather than force-unwrapping
   untrusted CloudKit record names.
 - `CloudDiagnostics` does not appear to log tokens, emails, or full record
   dumps.
 - Re-check account push address/secret adoption and republish behavior in
   Phase 4 alongside `AccountPushCoordinator`.
 - Re-check non-monotonic `PlayerEntity.readThrough` adoption in Phase 2 when
   durable read cursor semantics are reviewed.
 - Re-check `RecordSerializer.applyDecisionRecord` friend/nickname/block/
   encryption-key cases, `Presence` trust boundaries, and `ShareController`
   join confirmation in Phase 3.
 
 ---
 
 ## Phase 2 — Persistence & Data Model
 
 **Status:** Done
 
 Core Data model, the local durable store, journal/replay, and the domain
 models layered on top.
 
 - `Crossmate/Persistence/PersistenceController.swift`
 - `Crossmate/Persistence/GameStore.swift`
 - `Crossmate/Persistence/GameMutator.swift`
 - `Crossmate/Persistence/Journal.swift`
 - `Crossmate/Persistence/JournalReplay.swift`
 - `Crossmate/Models/CrossmateModel.xcdatamodeld/`
 - `Crossmate/Models/Game.swift`, `GameEntity+ContentKey.swift`
 - `Crossmate/Models/Square.swift`, `CellMark.swift`
 - `Crossmate/Models/PlayerRoster.swift`, `PlayerSession.swift`,
   `PlayerSelection.swift`, `PlayerColor.swift`, `PlayerPreferences.swift`
 - `Crossmate/Models/ParticipantSummaries.swift`, `GameCursorStore.swift`,
   `GameViewedStore.swift`, `TimeLog.swift`, `TipStore.swift`
 - `Tests/Unit/GameStore*Tests.swift`, `GameMutatorTests.swift`,
   `JournalReplayTests.swift`, `JournalUploadTests.swift`,
   `PersistenceRecoveryTests.swift`, `GameCursorStoreTests.swift`,
   `GameViewedStoreTests.swift`, `TimeLogTests.swift`, `TipStoreTests.swift`,
   `PlayerColorTests.swift`, `PlayerRosterTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 No security findings this phase — the model layer doesn't cross a trust
 boundary itself (author-ID/zone validation lives in `RecordSerializer`/
 `RecordApplier`, already covered in Phase 1). The main themes are a silent
 failure path in the move journal (bounded in practice by the journal
 context's merge policy), one clock-reconciliation edge case, and a few spots
 where synchronous Core Data work runs on the main actor / render path despite
 an `async` signature promising otherwise. Both passes agreed on the seven
 items below; the severity of #1 is tempered on the second pass, and #4/#6
 carry a note that the fix is a local index change, not a CloudKit schema
 deploy.
 
 **1. (Correctness — medium) `MovesJournal.persist` silently swallows Core Data
 save failures, which can permanently truncate a finished game's uploaded
 replay journal.** `Journal.swift:404-423` calls `ctx.save()` and, on failure,
 only does `print("MovesJournal: failed to persist entry: \(error)")` plus
 `ctx.rollback()` — no `eventLog`, unlike every other Core Data writer in this
 phase (`GameStore.saveContext`, `PersistenceController.backfillCachedSummaryFields`).
 `MovesJournal` isn't even constructed with an `eventLog` reference
 (`GameStore.swift:566`: `MovesJournal(persistence: persistence)`). This
 matters because the completion/replay-upload path deliberately reads the
 *persisted* Core Data rows through a separate background context rather than
 the in-memory `entries` list — see the doc comment on `flush()`
 (`Journal.swift:394-402`): the Phase 2 upload "reconstructs the asset from
 Core Data on a *separate* background context." A dropped `persist` (e.g. a
 non-optional-relationship validation failure if `entity.game` resolves to nil
 because the game row was concurrently deleted, or any other one-off Core Data
 error) removes that touch from the uploaded Journal asset permanently, with
 nothing beyond a console `print` to explain why a replay is missing a move.
 Severity was reconsidered on the second pass: the `mergeByPropertyStoreTrump`
 policy set in `MovesJournal.init` (`Journal.swift:163`) already neutralises
 the one *known* failure (the 133020 merge conflict), and the residual
 save-failure surface is narrow — the `game` relationship is optional, so a
 concurrently-deleted game just nulls it rather than failing validation, and
 `gameID` is always set. So the permanent-truncation scenario is real but
 uncommon; the silent, un-logged swallow (inconsistent with every other writer
 in the layer) is the core problem regardless of how often it fires. Fix:
 thread `eventLog` into `MovesJournal` and log persist failures at `error`
 level, mirroring the rest of the Persistence layer.
 
 **2. (Correctness — medium) `TimeLog.open`'s crash-reconciliation can
 fabricate solve time on a fast relaunch.** `TimeLog.swift:72-84`. When
 `reconcileStale` fires (first open of a game since launch, healing a session
 left dangling by a previous run's crash), the banked interval end is
 `boundedEnd = min(beatAt + openGrace(4min), openStart + maxSessionCap)` —
 never clamped to `now`. If the user relaunches well under 4 minutes after the
 crash, `boundedEnd` lands *after* the actual relaunch instant, so the sealed
 interval extends past `now` and the immediately-following live session
 (`openStart = now`) starts inside that still-"future" interval;
 `TimeLog.unionSeconds` then merges the overlap, crediting real elapsed time
 plus up to ~4 minutes of dead/crashed time that was never played. The
 existing test (`staleSessionReconciledOnReopen`,
 `Tests/Unit/TimeLogTests.swift:172`) only exercises `now=200_000`, far past
 `beatAt+openGrace=340`, so it never hits this boundary. Fix: also clamp
 `boundedEnd` to `now`.
 
 **3. (Performance — medium) `PlayerRoster.refresh()` blocks the main actor on
 a synchronous Core Data fetch instead of yielding it.** `PlayerRoster.swift:256`.
 `refresh()` is an `async @MainActor` method that creates a background context
 and then calls `ctx.performAndWait { ... }` (not `await ctx.perform`).
 `performAndWait` blocks the calling thread — the main thread here — until the
 background context finishes fetching the `GameEntity`, all its
 `PlayerEntity`/`MovesEntity` rows, and every nicknamed `FriendEntity`.
 `refresh()` is triggered by `.playerRosterShouldRefresh`, posted from
 `RecordApplier`/`SyncEngine` on roster-relevant inbound CloudKit changes,
 which can land mid co-solve session — each such event stalls the UI for the
 fetch's duration rather than freeing it, despite the `async` signature. This
 is a sharper, UI-facing instance of Phase 1 finding #4 (blocking Core Data
 work in async paths). Fix: `await ctx.perform { ... }`. Related render-path
 instance (second pass): `PlayerRoster.solveTime` (`PlayerRoster.swift:107-130`)
 fetches the `GameEntity` plus *every* `PlayerEntity` and JSON-decodes each
 `timeLog` synchronously on each call, and it drives the live header clock. It
 runs on `viewContext` (so a main-thread fetch is at least expected there), but
 it's the same "synchronous Core Data on the render path" theme and belongs in
 this cluster.
 
 **4. (Performance — medium) `GameEntity` has zero `fetchIndex` entries**
 despite `id`, `ckRecordName`, `databaseScope`, and `puzzleResourceID`/`title`
 being hit with exact-match `NSPredicate`s constantly —
 `"id == %@"` appears roughly 90 times in `GameStore.swift` alone (`loadGame`,
 `deleteGame`, `markCompleted`, `setEngagement`/`setNotification`,
 `ensurePushCredentials`, etc.), and `"ckRecordName == %@"` lookups against
 `GameEntity` appear across `RecordApplier`, `SyncEngine`, `RecordBuilder`,
 `RecordSerializer`, `MovesUpdater`, and `PlayerSelectionPublisher`. As the
 local library grows, every one of these becomes a full table scan. Add
 `fetchIndex`es on `id` and `ckRecordName` at minimum
 (`CrossmateModel.xcdatamodel/contents:3-44`). Note (second pass): these are
 Core Data `fetchIndex`es (local SQLite), not CloudKit schema — the app uses
 raw `CKSyncEngine`, not `NSPersistentCloudKitContainer` — so adding them (here
 and in #6) needs only a lightweight migration on next launch and *no*
 Production CloudKit schema deploy.
 
 **5. (Performance — low/medium) `MovesJournal` loads a game's entire journal
 synchronously on the main actor the first time it's touched.**
 `Journal.swift:361-364` (`ensureLoaded`) calls `load(_:)`, which runs an
 unbounded `JournalEntity` fetch via `ctx.performAndWait` (`:376-385`) — and
 `ensureLoaded` is called from `record`, `recordedEntries`, `canUndo`,
 `canRedo`, `planUndo`, and `planRedo`. `canUndo`/`canRedo` are read by the
 toolbar essentially as soon as a puzzle screen renders, so opening a
 heavily-edited or long-undo-history game can briefly stall the UI on this
 one-time-per-session fetch. Consider prefetching asynchronously before the
 puzzle screen becomes interactive, or fetching lazily off-main and updating
 state once ready.
 
 **6. (Performance — low) `MovesEntity`/`PlayerEntity` also lack a
 `ckRecordName` index.** Only composite indexes exist
 (`byGameAndAuthorAndDevice`, `byGameAndAuthor`), but plain
 `"ckRecordName == %@"` lookups against these entities happen in
 `GameStore.ensureMovesEntity` (`GameStore.swift:2769`), `RecordApplier`,
 `RecordSerializer`, `MovesUpdater`, and `PlayerSelectionPublisher`/
 `PlayerNamePublisher` — none benefit from the existing composite indexes.
 
 **7. (Code quality — low) `CellEntity` carries four dead attributes.**
 `ckRecordName`, `ckSystemFields`, `databaseScope`, and `lastSyncedAt`
 (`contents:81-90`) are declared on `CellEntity` but never read or written
 anywhere in the codebase (verified against every `CellEntity(context:`
 construction site). Looks like leftover schema from an earlier
 per-cell-record sync design, since replaced by the Moves-record model
 (`CellEntity` is now purely a derived local cache). Safe to drop.
 
 **Non-findings and deferred checks:**
 - `GameEntity`'s `cells`/`journal`/`moves`/`peerChanges`/`players`
   relationships are all `deletionRule="Cascade"`, matching
   `GameStore.resetAllData`'s doc-comment assumption — confirmed directly
   against the schema, not just the comment.
 - `Game.swift`'s completion-cache bookkeeping (filled/wrong counts, gap-cell
   exclusion, revealed-cell locking) matches `GameMutatorTests` exactly,
   including author-preservation on a same-letter rewrite.
 - `CellMark`'s code/decode round-trip is lossless and exhaustive; unknown
   codes safely default to `.none`.
 - `PlayerColor`'s companion/color assignment hashing is deterministic and
   array-bounds-safe.
 - The manual `Puzzle.Direction` `rawValue`/`init?(rawValue:)` extension in
   `PlayerSelection.swift:13-30` (across=0, down=1) is consistent with the
   independent manual encoding `Journal.swift:447` uses for the same field —
   checked directly since a mismatch here would silently flip undo/redo
   cursor direction between the Core Data and JSON-upload paths.
 - No force-unwraps on untrusted/CloudKit-sourced data anywhere in this
   file set.
 - `GameCursorStore.swift`, `GameViewedStore.swift`, `ParticipantSummaries.swift`:
   straightforward, no bugs found.
 - `TipStore.markDismissed` re-inserts into an already-dismissed set on a
   repeat dismiss, firing a redundant (harmless) `UserDefaults` write —
   not worth a fix on its own. (The working-tree change to this file is
   catalog-only — four new tips — and doesn't touch this path.)
 - The Phase 1 deferred check on non-monotonic `PlayerEntity.readThrough`
   adoption resolves as a non-finding: `RecordApplier.swift:285-301` adopts it
   under last-writer-wins *outside* the `updatedAt` guard deliberately, with a
   bounded, self-healing dip documented in place (a still-present device
   re-asserts its lease on draining the inbound close). The separate *local*
   read watermark `GameEntity.readThroughAt` (`GameStore.swift:1766-1792`) is
   properly monotonic.
 
 ---
 
 ## Phase 3 — Friends, Sharing & Invites
 
 **Status:** Done
 
 Friend graph, `CKShare`-based collaboration, invite links, and the two
 Workers that back link resolution and the realtime room.
 
 - `Crossmate/Sync/FriendController.swift`, `FriendZone.swift`
 - `Crossmate/Sync/ShareController.swift`
 - `Crossmate/Services/InviteCoordinator.swift`
 - `Crossmate/Services/ShareLinkRoute.swift`, `ShareLinkShortener.swift`
 - `Crossmate/Models/FriendEntity+DisplayName.swift`,
   `InviteEntity+DisplayName.swift`
 - `Shared/NicknameDirectory.swift`, `FriendEncryptionKeyDirectory.swift`
 - `Crossmate/Services/KeychainHelper.swift`
 - `Crossmate/Views/Friends/*`
 - `Workers/link-worker.js`, `Workers/room-worker.js`
 - `Tests/Unit/Sync/FriendControllerNicknameReplayTests.swift`,
   `FriendModelTests.swift`, `FriendZoneTests.swift`, `ShareRoutingTests.swift`,
   `Tests/Unit/NicknameDirectoryTests.swift`, `ShareLinkRouteTests.swift`,
   `ShareLinkShortenerTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 The dominant theme is the same trust-boundary class Phase 1 flagged: several
 inbound-`Decision` cases in `RecordSerializer.applyDecisionRecord` are adopted
 without checking the record's zone or database scope. Phase 3 makes that gap
 *concretely reachable*: a friendship gives the other party `.readWrite` on the
 `friend-<pairKey>` inbox zone **you own** (private DB, `scope == 0`), so any
 non-blocked friend can write arbitrary `Decision`/`Ping` records into a zone
 your private `CKSyncEngine` fetches. The two properly-gated cases (`name`,
 `nickname`) are the exception, not the rule — `block`, `left`, and
 `encryptionKey` all under-check. The `ShareController` owner gating, the two
 Workers' auth, and the link/token validation are all in good shape (see
 non-findings). The carry-over items are addressed as findings #1, #3, #4 (2)
 and non-findings (3).
 
 **1. (Security — high) `block` and `left` `Decision`s are adopted from any
 fetched zone, so a friend can tamper with your relationships and delete your
 local shared games.** `RecordSerializer.applyDecisionRecord`'s `block` case
 (`RecordSerializer.swift:1055-1074`) and `left` case (`:1075-1090`) apply with
 no zone/scope gate at all — unlike the sibling `nickname` case, which requires
 `record.recordID.zoneID.zoneName == accountZoneID.zoneName && databaseScope == 0`
 (`:1153-1155`). Both are legitimately written only into the private `account`
 zone (`FriendController.setBlocked` and `ShareController.leaveShare` both go
 through `SyncEngine.enqueueDecision`, which targets `accountZoneID`,
 `SyncEngine.swift:918-919`), but `applyDecisionRecord` runs for *every* fetched
 `Decision` on both engines (`SyncEngine.swift:1520-1525`). Concrete exploit: Bob
 is an accepted friend, so he holds `.readWrite` on Alice's inbox zone
 `friend-<pairKey(Alice,Bob)>` (`FriendController.saveZoneWideShare`,
 `FriendController.swift:609-623`). Bob saves a record named
 `decision-block-<Carol>` (`kind=block`, `payload="1"`, a large `version`) into
 that zone. Alice's private engine fetches her own inbox, the `block` case
 adopts it, and Alice's `FriendEntity` for Carol is marked `isBlocked` at Bob's
 version — silently blocking a third party (Alice can no longer invite Carol;
 `FriendController.sendInvite` throws `.friendBlocked`, `:366`) and forcing a
 version fight to undo it. The same write with `decision-left-<gameID>` hits the
 `left` case, which hard-deletes Alice's local participant `GameEntity` for any
 shared game she's in (`:1083-1090`) — a self-healing but disruptive
 kick/DoS. (Bob cannot un-block *himself* this way: blocking downgrades him to
 `.readOnly`, so he loses the inbox write access — the server-side enforcement
 holds. The exposure is tampering with the victim's state toward *third
 parties*, and `left`-driven local deletion.) Fix: gate both cases on the
 private `account` zone + `scope == 0`, exactly as `nickname` does.
 
 **2. (Security — medium) `applyFriendPing` accepts and starts syncing a share
 whose zone name is never checked against the bootstrap payload's `pairKey`.**
 `FriendController.applyFriendPing` validates the *acceptor* against the payload
 (`FriendZone.canAcceptBootstrap` requires
 `pairKey(localAuthorID, payload.ownerAuthorID) == payload.pairKey`,
 `FriendZone.swift:92-96`), but after fetching the share metadata it trusts the
 accepted zone wholesale: `outboxZoneID = metadata.share.recordID.zoneID`
 (`FriendController.swift:196`) with no check that
 `outboxZoneID.zoneName == FriendZone.zoneName(pairKey: payload.pairKey)`. A
 co-player (bootstrap Pings ride a shared *game* zone, so the attacker must
 already share a game) can mint a `CKShare` on a zone he owns but *names*
 `friend-<pairKey(Alice,Carol)>`, then send a `.friend` Ping whose payload sets
 `ownerAuthorID = Bob` and `pairKey = pairKey(Alice,Bob)` (so `canAcceptBootstrap`
 passes). Alice accepts and now syncs, at `scope == 1`, a zone Bob controls whose
 *name* impersonates the Alice–Carol pair. This is the enabler for finding #3,
 and independently means Bob-controlled `Decision`/`Ping` records flow into
 Alice's fetch under a forged pair identity. Fix: after fetching metadata, reject
 the accept unless `outboxZoneID.zoneName == FriendZone.zoneName(pairKey: payload.pairKey)`.
 
 **3. (Security — medium) `encryptionKey` `Decision` adoption omits the
 scope-0 gate its sibling `name` case has, allowing a third party's
 friend-channel key to be poisoned.** The `encryptionKey` case
 (`RecordSerializer.swift:1172-1187`) verifies the zone *name* matches
 `friend-<pairKey(localAuthorID, key)>` but — unlike the `name` case, which also
 requires `databaseScope == 0` (`:1116`) precisely to reject a record seen in a
 zone we merely joined — applies at any scope. Combined with finding #2, Bob (a
 mutual co-player who therefore knows Carol's authorID) shares a zone named
 `friend-<pairKey(Alice,Carol)>`, gets Alice to accept it (scope 1), and writes
 `decision-encryptionKey-<Carol>` carrying an attacker-chosen
 `FriendEncryptionKeyPayload`. Alice's fetch adopts it via
 `FriendEncryptionKeyDirectory.upsert(payload, for: Carol)`. That payload holds
 the `address` and `contentKey` used when Alice later sends Carol an invite push
 (`AccountPushCoordinator.swift:473`) and when the NSE decrypts one
 (`NotificationService.swift:62`), so a poisoned entry misroutes Alice's invite
 push for Carol to an attacker-chosen address and/or breaks decryption — an
 invite-push disclosure/DoS scoped to that pair. Fix: add `databaseScope == 0`
 to the `encryptionKey` gate (fixing #2 also closes the practical path).
 
 **4. (Security — high, fix owned by Phase 4) The friend inbox is the concrete
 writable zone that makes Phase 1 finding #1 (account push address/secret
 adoption) exploitable.** Phase 1 #1 noted that
 `parseAccountPushAddressDecision` / `parseAccountPushSecretDecision`
 (`RecordSerializer.swift:219`, `:245`) validate only record
 type/name/kind/payload and are called for every fetched `Decision`
 (`SyncEngine.swift:1514-1518`) but speculated about "a writable non-account
 zone." Phase 3 confirms one exists and is trivially reachable: any accepted
 friend holds `.readWrite` on your `friend-<pairKey>` inbox in the **private**
 database (`scope == 0`). A friend can therefore save a record named
 `decision-account-pushSecret` (`kind=account`, attacker payload, high `version`)
 into that inbox; your private engine fetches it and surfaces it to
 `onAccountPushSecret`. Because the push secret is the HMAC key from which every
 per-game push address is derived (`RecordSerializer.deriveGameAddress`,
 `:263-273`), adopting a foreign secret lets an attacker DoS push delivery
 across all of the victim's games and, if the attacker knows the secret they
 planted, predict/target the victim's per-game push addresses. This raises the
 practical severity of Phase 1 #1. `applyDecisionRecord`'s own `account`-kind
 handling isn't the issue (there is none); the gap is these two dedicated
 parsers plus their unconditional call site. Phase 4 should gate both parsers
 (or their fetch call site) on the private `account` zone + `scope == 0` when it
 reviews `AccountPushCoordinator`. Recorded here because Phase 3 surfaces the
 attack vector; the adoption/republish code is Phase 4.
 
 **5. (Security/resource — low) An `.invite` Ping's `puzzleSource` is trusted
 and materialised into a playable game before any shared-zone validation, with
 no size cap.** `InviteCoordinator.applyInvitePings` stores
 `payload.puzzleSource` from the friend-zone Ping onto the durable `InviteEntity`
 (`InviteCoordinator.swift:217`), and `acceptInvite` feeds it to
 `cloudService.acceptShare(prefetchedPuzzleSource:)` to build a local game before
 the shared-zone fetch (`:307-313`). The inviter controls this string, and the
 friend-zone `Ping.payload` is an unindexed record field bounded only by
 CloudKit's ~1 MB record limit — the comment (`FriendZone.swift:128-134`)
 assumes "a few KB." A malicious inviter can ship a ~1 MB blob to bloat the
 invitee's `InviteEntity` and force an oversized XD parse on accept. This is the
 friend-invite analogue of Phase 1 finding #3 (`puzzleSource` CKAsset without a
 size cap); the XD-parse hardening itself is Phase 5. Fix: enforce a
 conservative byte cap on `payload.puzzleSource` before storing/using it, and
 treat oversize as "fetch from the shared zone instead."
 
 **6. (Security/resource — low) `room-worker.js` allows unauthenticated room
 creation and has no rate limiting.** `register` (`room-worker.js:81-109`)
 creates a Durable Object and persists a secret for *any* well-formed body
 (`isAcceptableSecret` only requires ≥32 decoded bytes, `:249-258`); the room ID
 is an arbitrary path segment (`roomRouteFromPath`, `:242-245`) fed straight to
 `idFromName`. Anyone can therefore mint unbounded rooms (storage/cost) with no
 auth and no per-IP throttle on `register`/`socket`. Rooms self-expire on idle
 TTL (`alarm` → `deleteAll`, `:191-196`), so the blast radius is bounded in time,
 but a burst is uncapped. The TOFU secret model itself is sound — first-write
 wins, `timingSafeEqual` on re-register, and the secret never travels on a
 connect (`:93-104`, `:125-128`). Fix: consider a lightweight rate limit / proof
 requirement on `register`, or accept the risk explicitly given the TTL bound.
 (In-room `authorID`/`deviceID` are self-asserted query params signed by the
 *shared* room secret, so any room member can connect as any authorID and the
 relay does not bind frame authorship — an engagement/presence spoofing surface
 that belongs to Phase 4's engagement review.)
 
 **7. (Code quality — low) `FriendController.seedOwnNameDecision`'s `scope`
 parameter is effectively fixed.** It is only ever called with `scope: 1`
 (`FriendController.swift:206`), and `establish` doesn't seed a name at all
 (deferred to the acceptor's `applyFriendPing`). Not a bug, but the parameter
 implies a flexibility that doesn't exist; inline it or document the single
 caller.
 
 **8. (Security — medium) A `.decline` Ping is trusted to evict a share
 participant without checking that the decliner was ever invited to the named
 game.** `InviteCoordinator.applyDeclinePing` (`InviteCoordinator.swift:700-
 725`) acts on any inbound `.decline` Ping addressed to the local user and
 calls `ShareController.removeFriendParticipant(fromGameID: ping.gameID,
 userRecordName: ping.authorID)` (`ShareController.swift:211-254`). Both
 `ping.gameID` and `ping.authorID` are plain CKRecord fields
 (`Ping.parseRecord`, `Presence.swift:77-108`) on a record written into the
 *recipient's own inbox* zone — i.e. self-reported by whichever friend wrote
 it, with no check that this particular friend has any connection to the
 named game at all. `removeFriendParticipant` only checks that the local user
 owns the named game (`databaseScope == 0`) and that the named
 `userRecordName` currently holds a seat on *that* game's share; it never
 confirms a matching outstanding invite exists for `(gameID, authorID)`, or
 that the Ping's sender is the one who received that invite. Concretely: any
 established friend — not necessarily a collaborator on the target game — who
 can name a live `(gameID, authorID)` pair for one of the owner's *other*
 shared games (e.g. a former collaborator on that other game, or anyone who
 otherwise learned the pair) can forge a `.decline` Ping into the owner's
 inbox and evict that unrelated participant from a game the forger was never
 part of. The practical bar is knowing a live `(gameID, authorID)` pair, but
 nothing ties the claimed decliner identity to the actual CloudKit writer, and
 every accepted friend has permanent inbox write access (the same durable
 property finding #1 relies on) to attempt it at will. Fix: before freeing the
 seat, cross-check against a durable record of invites this owner actually
 sent for that `(gameID, authorID)` pair rather than trusting the Ping's
 self-reported fields.
 
 **9. (Code quality — low) `ShareController` hardcodes the zone-wide share
 record name instead of reusing the existing constant.**
 `ShareController.zoneWideShareRecordName = "cloudkit.zoneshare"`
 (`ShareController.swift:10`) duplicates the value of the system constant
 `CKRecordNameZoneWideShare`, which `FriendController.swift:521,632` and
 `PlayerRoster.swift:525` already use directly for the identical purpose.
 Harmless today (the literal is correct), but needless duplication that could
 silently drift. Replace with `CKRecordNameZoneWideShare`.
 
 **Non-findings and deferred checks:**
 - **Carry-over #2 resolved:** `nickname`'s account-zone+scope-0 gate
   (`RecordSerializer.swift:1153-1155`) is *not* the lone exception — `name`
   carries an equivalent pair-zone-name + `scope == 0` gate (`:1106-1116`), and
   both are correct (the `name` gate even documents rejecting a scope-1 sighting
   of the friend's own inbox). The exceptions that *under*-check are `block`,
   `left` (finding #1) and `encryptionKey` (finding #3).
 - **Carry-over #3 (`ShareController` join confirmation) resolved as a
   non-finding:** `confirmSeatAfterJoin` / `hasLostSeat`
   (`ShareController.swift:684-732`) is explicitly *cooperative* — CloudKit can't
   enforce a participant cap, so an over-cap joiner leaves voluntarily and a
   transient failure lets the join stand. It reads its own identity from
   `container.userRecordID()` and the server share, adds no trust boundary, and a
   non-cooperating client is the documented, accepted residual. Ticket-seat
   consumption uses correct optimistic-lock retries (`:852-897`).
 - `ShareController`'s owner-only gating is consistent and correct:
   `prepareShareRecord`, `createShareLink`, `addFriendParticipant`,
   `removeFriendParticipant`, and `existingShareLink` all guard
   `entity.databaseScope == 0`; `leaveShare`/`confirmSeatAfterJoin` guard
   `== 1`. `removeParticipant` refuses to drop the owner (`role != .owner`).
 - The `sessionInvitedAuthorIDs` re-assert on every invite save correctly
   guards the eventually-consistent-share window without ever *removing* a
   participant a fetched share carries (`ShareController.swift:44-51`,
   `:171-195`).
 - `room-worker.js` connect auth is solid: HMAC-SHA256 over
   `roomID|authorID|deviceID|timestamp|nonce`, ±120 s skew window, single-use
   nonces with TTL pruning, and `timingSafeEqual` on the signature
   (`:111-155`). No secret is logged or returned.
 - `link-worker.js` is a safe stateless redirector: the token is constrained to
   RFC 3986 unreserved chars (`TOKEN_PATTERN`, `:45`) so the target can only ever
   be `https://www.icloud.com/share/<token>` (no open redirect), title decode is
   charset-checked → strict-UTF-8 → control-stripped → capped → HTML-escaped at
   the interpolation site (`:279-301`), and every other interpolated value
   (`token`, `origin`, `shape`) is charset-validated upstream. `escapeHTML` omits
   `'`, but every interpolation site is a double-quoted attribute or element text
   where `'` is inert. Silhouette rendering is bounded to ≤35×35
   (`parseShape`, `:232`). No ReDoS in the crawler/token patterns.
 - `ShareLinkRoute` / `ShareLinkShortener` token validation mirrors the worker's
   and rejects percent-encoded slash smuggling (`ShareLinkShortenerTests` line
   156-165); `iCloudShareURL`'s force-unwrap is safe behind the charset check.
 - Invite tokens are not a Crossmate secret to guess: joining is gated by the
   underlying `CKShare` acceptance (Apple-enforced), not by link
   unguessability; the short link carries only the iCloud share token, which is
   as strong as CloudKit makes it.
 - `KeychainHelper` uses `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`
   (no iCloud keychain sync, sensible for device-local secrets), update-then-add
   upsert, and surfaces non-`errSecItemNotFound` failures as thrown errors.
 - `NicknameDirectory` / `FriendEncryptionKeyDirectory` are App-Group JSON
   mirrors rebuilt from Core Data ground truth; empty-save removes the file, and
   the `@TaskLocal` test override keeps parallel suites isolated. (The *source*
   of a `FriendEncryptionKeyDirectory` entry is the trust gap in finding #3, not
   the directory storage itself.)
 - `FriendZone.pairKey` is a symmetric SHA-256 of the sorted author pair
   (64-hex, deterministic, self-excluding), giving both devices the same zone
   name without coordination; covered by `FriendZoneTests`.
 - Re-check the `encryptionKey`/push-address/push-secret *adoption* side and the
   in-room `authorID` self-assertion in Phase 4 alongside
   `AccountPushCoordinator` and the engagement layer (findings #4, #6).
 
 ---
 
 ## Phase 4 — Push Notifications & Engagement
 
 **Status:** Done
 
 Push credential registration, badge/session pushes, the realtime engagement
 (co-solving) layer, the notification service extension, and the push Worker.
 
 - `Crossmate/Services/AccountPushCoordinator.swift`, `PushClient.swift`,
   `PushRequestAuthenticator.swift`, `BadgeCoordinator.swift`
 - `Crossmate/Sync/GamePushCredentials.swift`
 - `Crossmate/Services/SessionPushPlanner.swift`, `SessionCoordinator.swift`
 - `Crossmate/Services/EngagementHost.swift`,
   `EngagementHostEnvironment.swift`, `EngagementLifecycle.swift`
 - `Crossmate/Sync/EngagementCoordinator.swift`
 - `Crossmate/Services/AnnouncementCenter.swift`,
   `PlayerNamePublisher.swift`, `DriveMonitor.swift`, `InputMonitor.swift`
 - `Shared/PushPayload.swift`, `PushPayloadCipher.swift`,
   `NotificationState.swift`, `PuzzleNotificationText.swift`
 - `Crossmate/Models/PuzzleNotificationText+GameEntity.swift`
 - `NotificationService/NotificationService.swift`
 - `Workers/push-worker.js`
 - `Tests/Unit/AccountPushCoordinatorTests.swift`,
   `AnnouncementCenterTests.swift`, `Tests/Unit/Sync/GamePushCredentialsTests.swift`,
   `Tests/Unit/Sync/EngagementCoordinatorTests.swift`,
   `SessionPushPlannerTests.swift`, `PlayerNamePublisherTests.swift`,
   `NotificationStateTests.swift`, `PuzzleNotificationTextTests.swift`,
   `PushPayloadTests.swift`, `PushPayloadCipherTests.swift`,
   `NotificationNavigationBrokerTests.swift`, `OpenPuzzleBannerTests.swift`,
   `PlayerSessionNavigationTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 Both carry-over items from Phase 1/3 resolve *in* this phase, and both are
 concrete, high-severity gaps rather than theoretical ones: the account
 push-secret adoption path (carried since Phase 1) has no defense-in-depth
 anywhere downstream of the ungated parsers, and the engagement relay's
 self-asserted identity (carried since Phase 3) turns out to durably corrupt
 shared grid content, not just spoof presence. Elsewhere the push/crypto core —
 App Attest enrolment, HMAC request signing, AES-GCM payload sealing — is
 sound and both passes independently verified it closely. The recurring
 "blocking Core Data on an async main-actor path" theme from Phases 1/2
 reappears at three new call sites, found independently by both passes.
 
 **1. (Security — high) Account push address/secret `Decision`s are adopted
 with no account-zone gate anywhere in the pipeline, and a friend can hijack
 the HMAC push secret with an equal-version write.** This is the confirmed
 resolution of Phase 1 finding #1 / Phase 3 finding #4; the fix is owned here.
 `RecordSerializer.parseAccountPushAddressDecision` / `parseAccountPushSecretDecision`
 (`RecordSerializer.swift:219-227`, `:245-255`) validate only record
 type/name/kind/payload — never zone or database scope.
 `SyncEngine.handleFetchedRecordZoneChanges`'s `Decision` case
 (`SyncEngine.swift:1513-1519`) calls both parsers for every fetched Decision
 on both engines; `scope` is already in scope two lines later (passed into
 `applyDecisionRecord(..., databaseScope: scope)`) but is never checked before
 appending to `effects.accountPushAddresses`/`accountPushSecrets`. The
 `serverRecordChanged` conflict twin (`SyncEngine.swift:1888-1894`) has the
 same gap, lower-exposure since it only fires on this device's own
 account-zone save attempts.
 
 Tracing the adoption side confirms there is no second line of defense.
 `AccountPushCoordinator.adoptInboundPushAddress` (`AccountPushCoordinator.swift:126-130`)
 unconditionally caches whatever address it's handed and reconciles.
 `adoptInboundPushSecret` (`:140-149`) gates only on
 `version > local || (version == local && secret != current)` — and because a
 `Decision` with no `version` field maps to `decisionBaseVersion` (1), the
 *same* value a fresh local mint uses (`RecordSerializer.swift:235`,
 `AccountPushCoordinator.swift:350`), an attacker doesn't even need a high
 version: a `decision-account-pushSecret` record with no version field and a
 differing payload wins outright under the equal-version "server wins" branch.
 Adoption caches the foreign secret and calls `reconcilePushRegistration`,
 which re-derives every per-game push address via `deriveGameAddress`
 (`RecordSerializer.swift:263`), rewrites local Player rows, and — per
 `updatePushRegistration(republishPlayerRows: true)`
 (`AccountPushCoordinator.swift:168-190`) — re-publishes the poisoned
 derivation into every shared game's synced `Player.pushAddress` field via
 `syncEngine.enqueuePlayer(gameID:authorID:reason:"pushAddress")`
 (`:178-184`), visible to every co-player in each affected game, not just the
 attacker. Combined with Phase 3's confirmed reachable vector (any accepted
 friend holds `.readWrite` on your private-DB `friend-<pairKey>` inbox, which
 your private engine fetches), this DoSes push delivery across the victim's
 entire push surface and, since the attacker chose the secret, lets them
 predict/target the victim's derived push address for any game.
 
 Fix: gate both call sites (`SyncEngine.swift:1513-1519` and `:1888-1894`) on
 `record.recordID.zoneID.zoneName == accountZoneID.zoneName && databaseScope == 0`,
 mirroring the `nickname`/`name` gates already present in
 `applyDecisionRecord` (`RecordSerializer.swift:1106-1116`, `:1153-1155`). The
 call site is the right place — the parsers are also used on the local-save
 reconciliation path where scope/zoneID aren't otherwise threaded through and
 the gate would be redundant. Separately, tighten the equal-version branch so
 a foreign inbox copy with a missing/matching version can never win.
 
 **2. (Security — high) The realtime engagement channel trusts each frame's
 self-asserted `authorID`/`deviceID`/`cellAuthorID`/`updatedAt`, letting a
 room member durably corrupt and misattribute shared grid content — the
 concrete client-side consequence of Phase 3 finding #6's spoofable room
 identity.** `room-worker.js`'s `webSocketMessage` (`room-worker.js:157-170`)
 is a blind byte relay: `peer.send(data)` forwards the raw frame to every
 other connected socket with zero inspection or identity binding — so a
 frame's `authorID`/`cellAuthorID`/`deviceID` fields are entirely
 self-chosen, not even tied to the already-spoofable connect-time query-string
 `authorID` Phase 3 flagged. `EngagementLifecycle.handleEngagementMessage`'s
 `.cellEdit`/`.cellEditBatch` cases (`EngagementLifecycle.swift:346-376`) hand
 the decoded edit straight to `store.applyRealtimeCellEdit(s)` with no
 roster/identity check. `GameStore.applyRealtimeCellEdits`
 (`GameStore.swift:884-953`) only rejects an empty `authorID`/`deviceID` or a
 `deviceID` equal to the *local* device (`:897-902`) — unlike
 `EngagementStore.set` (`EngagementStore.swift:48-65`), which at least
 requires a cursor's `authorID` to resolve to a known roster player color
 before accepting it. The edit's `letter`/`mark`/`updatedAt`/`cellAuthorID`
 are written straight into the same local `MovesEntity` cache that legitimate
 CloudKit-fetched Moves records for that identity populate, using the
 attacker-supplied `updatedAt` unclamped against the local clock.
 
 The escalation: when the real CloudKit Moves record for the impersonated
 `(authorID, deviceID)` eventually syncs, `RecordSerializer.applyMovesRecord`
 merges cell-by-cell against the existing cache via `mergeIncomingMovesCells`
 (`RecordSerializer.swift:1014-1027`, the same tie-break flagged in Phase 1
 finding #9), keeping whichever `updatedAt` is newer. A frame stamped with a
 far-future `updatedAt` therefore wins that per-cell LWW race permanently —
 the genuine author's real, later edits to that exact cell can never overwrite
 it again through incremental sync, on any device that was live-connected to
 the room when the frame arrived. Concrete exploit: Bob, an accepted
 collaborator (so he legitimately holds the room secret), sends a
 `cellEditBatch` claiming `authorID`/`cellAuthorID` = Carol (a real co-player)
 with `updatedAt` far in the future; every live peer applies it, and Carol's
 genuine future keystrokes at that cell never win again on those devices.
 Mitigating factors: presence (`hasPeer`) is derived from `Player.readAt`
 leases in Core Data, not the room (`EngagementLifecycle.swift:116-121`), so
 the spoof can't fake presence; no push is emitted from an inbound frame; and
 the corruption never re-uploads to CloudKit (outbound Moves are always built
 from the local user's own identity), so blast radius is bounded to devices
 live-connected at attack time — but for those devices it is durable, not
 transient.
 
 Fix: have `room-worker.js` stamp the connection's own HMAC-verified
 `authorID`/`deviceID` onto (or strip from) relayed frames rather than passing
 client-chosen identity through untouched; on the client, validate
 `edit.authorID` against the known roster before writing into the durable
 cache (mirroring `EngagementStore.set`'s gate); and/or clamp
 `edit.updatedAt` to `≤ now + small skew` so a spoof can't win LWW durably —
 ideally combined with keeping live-channel content in a short-lived overlay
 that any subsequent real CloudKit fetch unconditionally supersedes, rather
 than merging it into the persisted cache CloudKit's own merge also reads
 from.
 
 **3. (Security/Availability — medium) `push-worker.js` has no rate limiting
 at either the authenticated `/publish` endpoint or the unauthenticated App
 Attest bootstrap endpoints.** Every client-side send throttle
 (`SessionCoordinator.nudgeCooldown` = 60s,
 `AccountPushCoordinator.accountSeenCoalesceWindow` = 30s, etc.) is enforced
 only in `PushClient`/`SessionCoordinator` — nothing in `handlePublish`
 (`push-worker.js:414-497`) limits how often a given `credID`/address may be
 published to. A malicious collaborator already has everything needed to
 bypass the app entirely: `GamePushCredentials` (secret + `credID`) is synced
 into the Game record's `notification` field for every participant
 (`RecordSerializer.swift:413`, `:858-868`), and every other participant's
 derived `pushAddress` is a plain field on the synced `Player` record
 (`RecordSerializer.swift:560-563`, `:624-631`) — both readable directly off
 CloudKit by any collaborator. A co-player can script raw, correctly-signed
 `/publish` calls at unlimited frequency, sending real alert pushes (sound +
 banner) to every co-player with none of the app's UI-layer cooldowns
 applying — the caller is fully App-Attest-authenticated, so this can't be
 dismissed as requiring attacker infrastructure. Separately, `/attest/challenge`
 and `/attest/register` (`push-worker.js:143-162`, `:164-222`) necessarily run
 *before* auth (they bootstrap the key) and take an attacker-chosen
 `deviceID`; `pruneExpired` only sweeps the same `deviceID` prefix
 (`:153`, `:313-323`), so an attacker rotating `deviceID` accumulates
 `appattest-challenge:*` storage rows faster than they TTL out. Both are the
 same class of gap Phase 3 flagged for `room-worker.js`'s open `register`
 (finding #6); rooms/challenges self-expire so the blast radius is
 time-bounded, but a burst is uncapped. Fix: add a per-`credID`/per-address
 sliding-window rate limit on `/publish`, and a lightweight per-IP/per-device
 throttle on the attest bootstrap endpoints.
 
 **4. (Performance — medium) Three new call sites repeat the recurring Phase
 1/2 "blocking Core Data on an async `@MainActor` path" pattern — found
 independently by both review passes.** Each creates a background context and
 calls `ctx.performAndWait { ... }` synchronously from an `@MainActor` chain
 with no intervening `await`, blocking the main thread for the fetch's
 duration despite the enclosing methods being `async`: `SessionCoordinator.pushPlan`
 (`SessionCoordinator.swift:521-553`), invoked from `nudge`,
 `publishJoinPush`, `publishSessionEndPush`, `publishCompletionPush`, and
 `publishReplayPush` — all reachable from the active puzzle's foreground path,
 and the fetch pulls the game plus all its `PlayerEntity` rows;
 `AccountPushCoordinator.friendEncryptionKeyTargets`
 (`AccountPushCoordinator.swift:232-251`), invoked from every
 `reconcilePushRegistration` and `publishInvitePush`; and
 `PlayerNamePublisher.friendZoneTargets` / `upsertPlayerRecord`
 (`PlayerNamePublisher.swift:167`, `:192`), invoked from `fanOut`/
 `publishName(for:)` — the latter on every shared-puzzle open. Fix: convert to
 `await ctx.perform { ... }` throughout.
 
 **5. (Resource — low) Game push credentials are never expired or cleaned up
 after a game is deleted or archived.** `GamePushCredentials` is deliberately
 durable/no-expiry by design (`GamePushCredentials.swift:25`), unlike
 `EngagementRoomCredentials`'s 10-minute TTL, but no client call anywhere
 unregisters a game credential or its bound addresses from `push-worker.js` —
 `PushClient.unregister(bindings:)` exists but is only driven by
 `setAddresses`'s diff against the current live binding set, never by game
 deletion. This slowly accumulates orphaned `gamecred:<uuid>` and
 `addr:<credID>:*` Durable Object storage entries for every game a user ever
 plays and later deletes/archives.
 
 **6. (Correctness/privacy — low) A failed push `unregister` is silently
 forgotten, leaving a stale address registered with the worker.**
 `PushClient.reconcile` (`PushClient.swift:170-203`) awaits
 `unregister(bindings:)`, which catches and swallows its own errors
 (`:448-467`) rather than throwing, then unconditionally advances
 `lastRegistered = signature` (`:196`). If the DELETE fails (transport blip,
 worker 5xx), the removed bindings drop out of `lastRegistered.bindings`, so
 the next reconcile computes an empty `toRemove` and never retries — the
 stale address→token mapping persists on the worker, so after leaving a game
 or switching accounts the device can keep receiving that game's pushes until
 an APNs 410 or token rotation clears it. Fix: only advance `lastRegistered`
 for bindings that actually unregistered, or re-drive removals until
 confirmed — the register side already models this correctly by leaving
 `lastRegistered` unchanged on failure.
 
 **Non-findings and deferred checks:**
 - `PushPayloadCipher` uses AES-GCM correctly (combined nonce‖ciphertext‖tag
   box, random nonce, ≥32-byte key enforcement) and fails closed to `nil` on
   any decode/verify failure — no plaintext leak on error
   (`PushPayloadCipher.swift:26-52`). `NotificationService.swift:52-98`
   correctly falls back to the generic cleartext body when `enc` is absent or
   undecryptable, and never force-unwraps worker-sourced fields.
 - `PushRequestAuthenticator`/`push-worker.js`'s App Attest scheme (challenge/
   attest/assertion, full CBOR/DER/cert-chain parsing, per-device
   nonce+timestamp binding, `timingSafeEqual` throughout) is sound and matches
   Apple's documented scheme; `GamePushSigner`'s HMAC game-participation
   signature (`credID|bodyHash|timestamp|nonce`, with `bodyHash` covering
   `credID`) is correctly bound so `credID` can't be swapped without breaking
   App-Attest auth. Considerably more robust than `room-worker.js`'s
   HMAC-only connect auth reviewed in Phase 3.
 - `push-worker.js`'s game-credential registration and `/publish` *are* gated
   behind full App Attest auth (checked before routing,
   `push-worker.js:19-49`) — a meaningful contrast with `room-worker.js`'s
   fully open `register` (Phase 3 finding #6); only the missing rate limit
   (finding #3) carries equivalent risk forward.
 - Account-scoped pushes (the `acct-<UUID>` address) are not spoofable
   without the account zone: the address is random, not secret-derived, and
   published only into the private account zone
   (`push-worker.js:405-413`, `:529-563`).
 - `EngagementHost`'s socket connect auth mirrors the Phase-3-approved
   room-worker connect auth (HMAC over
   `roomID|authorID|deviceID|timestamp|nonce`, secret never in the URL);
   `EngagementCoordinator`'s connect/migrate/teardown state machine and
   stale-connection sweep are clean.
 - `applyGameRecord`'s adoption of the `engagement`/`notification` fields
   (`RecordSerializer.swift:848-868`) is correctly scoped — these live on the
   Game record itself, already resolved to the correct per-game zone/entity,
   so "whoever can write this game's own zone" is the intended trust boundary
   (the game's own CKShare participants), unlike the account-zone Decision
   gap in finding #1.
 - `AnnouncementCenter`, `BadgeCoordinator`/`NotificationState`'s
   `seenAt`/`suppressedUntil` ledger, `PuzzleNotificationText`(+`GameEntity`),
   `DriveMonitor`, `InputMonitor` are straightforward and correct; no bugs
   found. No force-unwraps on untrusted CloudKit- or worker-sourced data
   anywhere in the Phase 4 file set.
 - `GamePushCredentialsTests` confirms the HMAC key-derivation contract
   (base64url-decoded secret bytes, not UTF-8) matches the worker's
   `hmacSHA256` exactly — no drift risk.
 - `fromAuthorID` on a push is worker-forwarded and attacker-settable but is
   display-only: the NSE only rewrites the body when the sealed payload
   actually decrypts, so a spoofed `fromAuthorID` on an undecryptable push
   yields the generic body, not a nickname leak.
 - Carry over to **Phase 5**: `DriveMonitor.readSource`/`importFile` reads a
   user-selected local/iCloud-Drive `.xd`/`.puz` file with no size cap before
   `PUZToXDConverter.convert` — not a remote trust boundary like Phase 1 #3 /
   Phase 3 #5, but worth folding into Phase 5's general XD/PUZ parse hardening
   review.
 - Carry over to **Phase 6**: finding #2's exploit rides the same LWW
   tie-break flagged in Phase 1 finding #9; a `now`-clamp or monotonic guard
   there would also harden the CloudKit ingest path, worth revisiting
   alongside gameplay/grid state in Phase 6.
 - Carry over to **Phase 7**: `NotificationNavigationBroker`/
   `PlayerSessionNavigation` live in the app-shell composition root, outside
   this phase's file list; a shallow look shows they only route an
   already-parsed `gameID` to in-app navigation with no obvious trust-boundary
   issue, but they deserve a full pass in Phase 7.
 
 ---
 
 ## Phase 5 — Puzzle Import & Format Conversion
 
 **Status:** Done
 
 Bringing puzzles into the app: NYT auth/fetch, `.puz`/NYT-JSON → XD
 conversion, and the XD format itself.
 
 - `Crossmate/Services/NYTAuthService.swift`, `NYTPuzzleFetcher.swift`,
   `NYTPuzzleUpgrader.swift`, `NYTToXDConverter.swift`
 - `Crossmate/Services/PUZToXDConverter.swift`, `ImportService.swift`
 - `Crossmate/Models/XD.swift`, `XDFileType.swift`, `XDMarkup.swift`
 - `Crossmate/Models/PuzzleCatalog.swift`, `PuzzleSource.swift`,
   `Puzzle.swift`
 - `Puzzles/cm-starter/*`, `Puzzles/debug/*`
 - `Tests/Unit/NYTAuthServiceTests.swift`, `NYTPuzzleUpgraderTests.swift`,
   `NYTToXDConverterTests.swift`, `PUZToXDConverterTests.swift`,
   `PuzzleCatalogTests.swift`, `XDAcceptTests.swift`, `XDMarkupTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 This is the phase where the Phase 1 finding #3 / Phase 3 finding #5 carry-over
 ("`puzzleSource` reaches the parser with no size cap") cashes out concretely —
 and the result is sharper than either earlier phase assumed: the parser itself
 has an unmemoized exponential algorithm that a **sub-300-byte** input can drive
 into a multi-hour hang, so a byte-size cap alone would not have closed the
 gap. Both passes found this independently; Sonnet 5 additionally built a
 standalone harness from the real source and empirically measured it. The
 second major finding — a `.puz` rebus-placeholder allocator that both
 misassigns reserved characters and traps on `UInt8` overflow — was also found
 independently by both passes and empirically confirmed. All file:line
 references below were spot-checked directly against the current source.
 
 **1. (Security/Performance — high, empirically confirmed) `XD.segment` is an
 unmemoized exponential backtracking search, reachable from attacker-controlled
 `puzzleSource` on the CloudKit sync path and, via invite acceptance, on the
 main actor.** `XD.segment` (`XD.swift:705-748`) assigns free-length substrings
 from a clue's `~` answer onto a word's still-unsolved cells via plain
 recursion (`recurse(cellIndex:answerIndex:current:)`, `:706-742`). The
 `maxResults` cap (`results.count < maxResults` at `:709`) only prunes *after*
 a complete valid segmentation is found; when the crafted answer text has
 **zero** valid segmentations against a word's already-crossing-fixed cells,
 the full composition tree is exhausted before returning empty. For a run of
 *E* adjacent unsolved cells constrained only by one trailing fixed-letter
 cell, this is `O(2^E)`-ish combinatorial work. Sonnet 5 reproduced this on
 real device hardware from the extracted source: a 2-column synthetic grid (one
 down-word of unconstrained cells + one crossing-fixed cell) measured 2.13s at
 14 free cells / slack 10 (~1.14M compositions) and **32.2s at 16 free cells /
 slack 12** (~17.4M compositions, from a ~250-byte source file), extrapolating
 to minutes-to-hours at 18-20 free cells — all comfortably under 1 KB.
 
 Reachability has no existing mitigation: `RecordSerializer.applyGameRecord`
 calls `XD.parse(source)` on the synced Game record's `puzzleSource` CKAsset
 with no complexity guard (`RecordSerializer.swift:875`) — the same asset
 Phase 1 #3 flagged for size only. Concretely for invites, the exact Phase 3 #5
 vector completes the chain: `InviteCoordinator.acceptInvite`'s
 inviter-controlled `payload.puzzleSource` flows through
 `CloudService.acceptShare(prefetchedPuzzleSource:)`
 (`CloudService.swift:55`, `:82`) into
 `GameStore.constructJoinedGame(gameID:zoneID:source:)`
 (`GameStore.swift:1202`), which calls `try XD.parse(source)` directly at
 `:1209` — and `GameStore` is `@MainActor` (`GameStore.swift:504`), confirmed
 by direct inspection, so this is a **synchronous main-thread hang**: a friend
 sends one crafted invite and the victim's UI freezes solid the moment they tap
 Accept (likely followed by an iOS watchdog kill). The same call also fires at
 ordinary game-open/local `.xd` import. Recursion depth is bounded by word
 length, so this is pure combinatorial-time DoS, not a stack overflow. Fix:
 memoize `segment` on `(cellIndex, answerIndex)` — this turns it into standard
 `O(word.count × answer.count)` word-break DP, since the failing subproblems
 repeat — and/or cap total recursive-call count and free-cell run length ahead
 of a fail-closed error. This finding supersedes the "just add a size cap"
 framing from Phase 1 #3 / Phase 3 #5: a byte cap remains worth adding for
 memory/parse-cost reasons generally (see #5), but it does not bound this
 specific attack.
 
 **2. (Correctness/Security — high, empirically confirmed) `PUZToXDConverter`'s
 rebus-placeholder allocator is keyed by cell index instead of by distinct
 rebus value, so it both misassigns reserved grammar characters at low counts
 and hard-crashes via `UInt8` overflow past ~207 rebus-covered cells.** The
 loop at `PUZToXDConverter.swift:78-87` walks every rebus-covered cell once
 (`for index in 0..<cellCount where isOpen(...)`) and assigns a fresh key from
 `nextRebusKey`, plain (non-`&+=`) `UInt8` arithmetic starting at
 `Character("1").asciiValue!`: `guard rebusKeys[index] == nil else { continue }`
 never fires because `rebusKeys` is keyed by cell **index** and each index is
 visited exactly once — the guard cannot dedupe by *value*, unlike the correct
 value-keyed `NYTToXDConverter.rebusLookup` (`NYTToXDConverter.swift:150-163`,
 whose header comment at `:22-23` explicitly documents having already hit and
 fixed this exact class of bug: "walking ASCII naïvely from `'1'` overflows
 into `=` by the 13th key, which produced an unparseable `==VALUE` entry"). So
 even a `.puz` with only a handful of *distinct* rebus values spread across
 enough cells burns through placeholders one-per-cell: the 13th assigned key is
 `=`, breaking the `Rebus: key=value` grammar into an unparseable entry; the
 16th is `@`, which silently re-parses as a *circled*-cell marker instead of a
 rebus key. At the 207th rebus-covered cell, `nextRebusKey` reaches 256 and
 `nextRebusKey += 1` **traps**, crashing the app outright. Sonnet 5 confirmed
 the crash empirically against a synthetic 256-cell 16×16 `.puz` with only 3
 distinct rebus values mapped across all cells (exit 133, fatal arithmetic-overflow
 trap), proving the bug is cell-count-keyed, not distinct-value-keyed as a
 size-based mitigation might assume. `PUZToXDConverterTests.swift` has no
 rebus/`GRBS`/`RTBL` coverage at all, which is exactly why this shipped
 unnoticed. Reachable via any locally-imported `.puz` (Files app, AirDrop,
 email) — a sufficiently rebus-heavy variety puzzle could trigger the
 `=`/`@` corruption by accident, no malice required. Fix: dedupe by rebus
 *value* into a `[String: Character]` map exactly mirroring
 `NYTToXDConverter.rebusLookup`, reusing its curated, reserved-character-avoiding
 `rebusPlaceholders` set (`NYTToXDConverter.swift:24-30`) instead of raw ASCII
 walking, and use `&+=` or a bounds check regardless.
 
 **2b. (Correctness — medium; NOT found in this audit, discovered 2026-07-01
 while fixing #2 — TODO H2b) `PUZToXDConverter.parseRebus` reads the `GRBS` grid
 byte directly as the `RTBL` key, but `.puz` stores `GRBS = RTBL_key + 1`.** The
 loop at `PUZToXDConverter.swift:338-343` did `let key = Int(grid[gridIndex]);
 table[key]`, treating the 1-indexed grid byte as a 0-indexed table key. The
 `.puz` format reserves `GRBS` byte 0 for "no rebus" and stores `1 + n` for the
 square whose solution is `RTBL` key `n` — confirmed independently by the puz
 FileFormat wiki ("A value of 1+n indicates a rebus square … key n in the RTBL
 section"), puzpy (`self.table[i] = k + 1`), and other implementations. Effect on
 a *real* Across Lite rebus file: a single-value rebus (`RTBL` key 0, `GRBS` byte
 1) looked up the absent `table[1]` and silently dropped every rebus cell — the
 cell fell back to its raw single-letter solution byte; a multi-value rebus
 mis-mapped each cell to the *next* table entry and dropped the last. Silent
 corruption, never a crash, which is why it went unnoticed (and why the #2
 allocator crash was in practice only reachable from a hand-crafted file, since
 no real rebus `.puz` ever populated the rebus map). Fix applied: `table[key - 1]`
 with the grid byte guarded `> 0`; regression test `singleRebusValueDecodes`.
 
 **3. (Security/Correctness — medium) `NYTToXDConverter.convert` doesn't
 validate that `dimensions.width`/`height` are positive before using them,
 crashing on a malformed NYT response; a sibling cell-index access is
 similarly unguarded.** `convert(jsonData:)` extracts `width`/`height` as plain
 `Int` from JSON (`NYTToXDConverter.swift:71-75`) and only checks
 `cells.count == width * height` (`:81-83`) before looping
 `for row in 0..<height { for col in 0..<width { ... } }` (`:173-175`). A
 response with `width: -1, height: -1` and a single-element `cells` array
 satisfies `(-1)*(-1) == 1 == cells.count` and passes every guard, then traps
 with `Fatal error: Range requires lowerBound <= upperBound` — confirmed by
 running the converter against a crafted minimal body. Separately,
 `let answerStr = cellIndices.compactMap { answers[$0] }` (`:241`) subscripts
 `answers` with `clue["cells"]` indices with no bounds check — an
 out-of-range/negative index is a fatal array access — while the sibling
 `acceptedAnswerVariants` correctly guards the same kind of access with
 `answers.indices.contains` (`:330`). NYT's API is comparatively trusted, but
 it's still externally-supplied structure parsed as untrusted input per this
 phase's brief; a corrupted proxy/cache, API regression, or on-device MITM
 crashes the fetch/upgrade flow. Fix: `guard width > 0, height > 0` alongside
 the existing cell-count check, and guard every `answers[$0]` access the same
 way `acceptedAnswerVariants` already does.
 
 **4. (Security/privacy — medium) `NYTPuzzleFetcher` unconditionally logs a
 partial NYT session cookie and full request/response bodies via raw `print`,
 not gated by `#if DEBUG`.** `fetchPuzzle` prints
 `"Cookie header: NYT-S=\(cookie.prefix(40))..."`
 (`NYTPuzzleFetcher.swift:99`, confirmed) — 40 characters of the live session
 cookie that authenticates every NYT request — plus the request URL, full
 response bodies up to 1000-3000 chars, and the entire converted `.xd` source
 (`:97-133`, confirmed), on every single fetch, in release builds, since `print`
 is not stripped. `NYTAuthService.completeSignIn` logs only cookie *names* and
 *domains*, never values (confirmed) — the more careful pattern already exists
 in this file set and simply wasn't applied here. This is a real departure from
 the discipline Phases 1/3/4 verified for `CloudDiagnostics` and the push/crypto
 layers (never log tokens/secrets even truncated); a 40-char cookie prefix
 reaching Console.app/sysdiagnose/MDM log collection/screen-share is a genuine,
 if not catastrophic (NYT-S is an ordinary web session cookie, not a
 cryptographic key), credential-exposure surface. `fetchPuzzleList`, which
 carries the same logging, also has no callers anywhere in `Crossmate/` or
 `Tests/` — dead debug scaffolding left in the shipped file. Fix: delete all of
 these prints (or route a redacted line through the same event-logging pattern
 used elsewhere), and remove the dead `fetchPuzzleList` method or wire it up.
 
 **5. (Performance — low) No length/complexity cap on clue text compounds two
 independently-quadratic-or-worse render/parse paths.** `XDMarkup.segments`
 (`XDMarkup.swift:61-86`) rescans from `i+2` to end-of-string on every
 unterminated markup open (`{` + valid marker with no matching close) and falls
 through to a single-character advance rather than skipping the failed scan —
 clue text with many repeated unterminated sequences (e.g. `{/{/{/{/...`)
 triggers a full rescan per occurrence. `Puzzle.init`'s `parseCrossReferences`
 (`Puzzle.swift:305-336`) runs a lazy-quantified regex over every clue's text at
 construction time, which — per finding #1's reachability — executes on the
 sync worker for attacker-influenced clue text; not exponential, but unbounded
 polynomial cost purely because clue length is unbounded. Both are lower
 severity than finding #1 (clue prose is normally short, and `XDMarkup` runs on
 render rather than the sync/accept hot path) but reinforce the same theme: add
 a conservative byte/length cap on `puzzleSource` and per-clue text before
 parsing, at every boundary identified in Phase 1 #3 / Phase 3 #5 / the Phase 4
 `DriveMonitor` carry-over (`ImportService.importGame`'s uncapped
 `String(contentsOf:)`/`Data(contentsOf:)` at `ImportService.swift:27,29` is the
 same gap). This bounds memory/parse cost generally even though, per finding
 #1, it does not alone bound the exponential case.
 
 **6. (Consistency — low) Bundled/debug `.xd` sources are stamped `CmVer: 3`
 against a current parser version of 7.** Every file under `Puzzles/cm-starter/*`
 and `Puzzles/debug/*` carries `CmVer: 3` while `XD.currentCmVersion == 7`
 (`XD.swift:7`). All 13 bundled/debug puzzles were parsed end-to-end with the
 real parser (not just cross-checked against manifests) and load correctly
 regardless, and grid/`blockMask` consistency against each manifest was also
 verified directly — this is purely a stale version stamp. Worth confirming
 whether bundled-puzzle staleness/refresh logic (Phase 2) treats a `CmVer`
 behind the parser version as meaningful, or whether it should — otherwise
 just re-stamp the bundled sources to track the authoring tool's actual output.
 
 **7. (Code quality — low) `PUZToXDConverter` indexes `Data` with absolute
 offsets that assume a zero-based buffer.** `data[0x2C]`, `asciiString(in:range:)`,
 and `Array(data[solutionStart..<fillStart])` (`PUZToXDConverter.swift:37-52`)
 use literal/absolute offsets into `Data`. Safe with the current caller
 (`Data(contentsOf:)` is zero-based) but would crash on any `Data` slice with a
 non-zero `startIndex`. Normalize with `data.startIndex + offset`.
 
 **Non-findings and deferred checks:**
 - No force-unwraps in `XD.swift` on untrusted input: all regex captures are
   `Int`-guarded, grid/word walks stay bounds-checked through `isOpen`/`isBlock`,
   and `Numbering`/segment array accesses stay within grid bounds. The only
   algorithmic hazard is `segment` itself (finding #1), not a crash.
 - `XDMarkup` degrades safely (no crash) on an unterminated span or an invalid
   `{` marker — only the perf cost in finding #5.
 - `XD.Numbering.build` and `Puzzle.init`'s independent clue-numbering
   computation agree (checked directly; a mismatch would silently misnumber
   clues).
 - `NYTToXDConverter`'s *own* rebus placeholder set correctly avoids every
   reserved grid/header character and is tested
   (`manyRebusFillsAvoidReservedKeys`) — the exact hardening `PUZToXDConverter`
   lacks (finding #2); its header comment even documents having hit and fixed
   the identical bug class previously.
 - `NYTAuthService` credential handling is sound: cookie/email go through
   `KeychainHelper` (`kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`, no
   iCloud sync, proper upsert — consistent with Phase 3's review of the same
   helper); `cookieLoadResult`/keychain-load logging records only status and a
   `hasData` boolean, never cookie content (contrast with finding #4); a locked
   device correctly maps `errSecInteractionNotAllowed` to
   `.temporarilyUnavailable` rather than a false sign-out.
 - `NYTAuthService.extractAccountGraphQLConfiguration`'s HTML-scraping regex
   (`firstJSONStringValue`) is single-pass with mutually-exclusive alternation
   on first character — no catastrophic backtracking, and it reconstructs a
   well-formed JSON literal to decode escapes safely rather than hand-parsing
   them.
 - `NYTPuzzleUpgrader.structuralDivergence` correctly gates source replacement
   on identical dimensions + block layout + per-cell solution; both
   `.mismatched`/`.failed` paths preserve the player's existing moves, and
   `.plan` only fires for owner-side NYT puzzles.
 - `PUZToXDConverter.decodeString`'s CP1252→Latin1→UTF8 fallback chain
   terminates in a non-optional decode; the non-rebus parsing (dimensions,
   string table, `GEXT`/circled cells, clue-count reconciliation) validates
   every offset/length against `data.count` before slicing, and `width`/`height`
   are single bytes (≤255) so `cellCount` can't overflow.
 - `Puzzle.init`'s loops can't see a mismatched width/height, because `Puzzle`
   is only ever constructed from an already-successfully-parsed `XD`, whose
   `width`/`height` are derived from the same grid data its rows were built
   from.
 - `PuzzleCatalog` manifest loading is defensive (`try?` throughout, skips any
   bundle directory with an unreadable/undecodable manifest) and reads only
   app-bundle resources — not a trust boundary, no user input reaches it.
 - `ImportService.importGame` is a thin, straightforward extension dispatcher;
   its missing size cap is the same Phase 4 `DriveMonitor` carry-over folded
   into finding #5 above (and, per finding #1, a size cap wouldn't have stopped
   that specific attack anyway).
 - Nothing new carries forward to Phase 6/7 beyond what's already noted above —
   the XD/PUZ/NYT parsing surface is self-contained within this file set, and
   finding #1's exploit path terminates at `GameStore`/`XD.parse` reviewed here,
   even though its *reachability* runs through Phase 2/3/6 call sites.
 
 ---
 
 ## Phase 6 — Gameplay & Puzzle UI
 
 **Status:** Done
 
 The actual solving experience: the puzzle screen, custom keyboard, replay,
 and session state.
 
 - `Crossmate/Views/Puzzle/*`
 - `Crossmate/Services/PuzzleSession.swift`, `ReplayLoader.swift`
 - `Crossmate/Models/ReplayControls.swift`, `CheckResult.swift`
 - `Crossmate/Services/GridSilhouette.swift`
 - `Tests/Unit/PuzzleSessionTests.swift`, `ReplayCacheTests.swift`,
   `ReplayControlsTests.swift`, `GridSilhouetteTests.swift`,
   `GameSummaryThumbnailTests.swift`, `PendingEditFlagTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 This is one of the cleaner phases so far. `GridView`'s single-`Canvas` redesign
 is carefully reasoned about render cost, `PuzzleSession`'s grace-timer/
 background-assertion state machine is cleanly modeled and well-tested, and
 `GridSilhouette`'s codec is defensive with no force-unwraps anywhere in the
 file set. The puzzle UI itself turns out to be a pure, passive consumer of
 already-merged grid state — it does not re-implement or second-guess any
 last-writer-wins tie-break, which resolves the Phase 4 carry-over as a
 non-finding for new gameplay-side logic (see below), though tracing the
 consumers confirms concretely how a Phase 4 finding #2 spoof would surface in
 this layer. The one substantive bug both passes independently zeroed in on
 different corners of is a replay-completeness gap in the app-shell
 composition root that can permanently cache an incomplete scrubber for a
 session; the rest is low-severity robustness/quality, including a recurring
 "redundant full-grid pass" pattern this audit has flagged in earlier phases.
 
 **1. (Correctness — medium) The app-shell's "local-only" replay fast path
 infers a game has no other contributors from an unverified local Core Data
 snapshot, and can permanently cache an empty/incomplete replay for the rest
 of the session.** `CrossmateApp.swift:787-797`:
 ```swift
 let entries = store.localJournalEntries(for: gameID)
 let otherDevices = store.contributingDevices(for: gameID)
     .filter { $0.deviceID != localDeviceID }
 if otherDevices.isEmpty {
     return .ready(ReplayTimeline(merging: [entries]))
 }
 ```
 `contributingDevices(for:)` (`GameStore.swift:2313-2324`) only reads this
 device's already-synced local `MovesEntity` rows — unlike
 `ReplayLoader.loadReplay` (`ReplayLoader.swift:45-120`), which derives
 `expectedDevices` from a live CKQuery and gates on every contributor's journal
 having landed. If this device's Moves catch-up sync for the game hasn't
 finished yet — the common case the first time a device opens an
 already-completed shared game (freshly accepted an invite to a finished
 puzzle, a reinstall, an iCloud restore, or a collaborator returning to a game
 finished while they were away) — `contributingDevices` sees zero remote rows
 even though real collaborators exist, so the shortcut wrongly takes the solo
 branch and builds `ReplayTimeline(merging: [entries])` from this device's own
 (often empty) local journal, bypassing `ReplayAssembler`'s strict-completeness
 gate entirely. That wrong result is cached as `.ready` by
 `ReplayAssembler.memoised` (`JournalReplay.swift:173-189`) into
 `ReplayLoader.replayTimelineCache` (`ReplayLoader.swift:23`); because
 `ReplayControls.load` is a no-op once `.ready` (`ReplayControls.swift:169-174`)
 and the memo short-circuits on every later call for that `gameID`
 (`JournalReplay.swift:180-183`), every subsequent reopen of the puzzle within
 the same app session keeps serving the wrong scrubber even after the real
 Moves records finish syncing in the background — only a full app relaunch
 clears the in-memory cache. Fix: gate the fast path on a hard, durable signal
 that the game was never shared (e.g. `!session.mutator.isShared` /
 `GameEntity.ckShareRecordName == nil`) rather than on the currently-synced
 local Moves cache; for anything shared, always defer to
 `services.replays.loadReplay`, which already handles the true solo-but-shared
 case correctly via its own `expectedDevices` check.
 
 **2. (Correctness — low) A whitespace-only rebus commit writes a phantom
 "filled" cell that silently blocks completion.**
 `PlayerSession.committedRebusValue` (`PlayerSession.swift:488-493`) returns
 the buffer unchanged whenever it contains no non-whitespace character —
 `guard buffer.rangeOfCharacter(from: .whitespacesAndNewlines.inverted) != nil
 else { return buffer }` — so a whitespace-only buffer commits as a
 non-empty, visually-blank cell instead of clearing it. The reachable path is
 the rebus modal's system `TextField` (`PuzzleView.swift:623-637`), which,
 unlike the custom keyboard, permits a space; the blank-looking cell then reads
 as filled to `PuzzleScoreboard.filledCellCount` and `Game.completionState`, so
 a fully-filled grid resolves to `filledWithErrors` with no visibly wrong cell
 for the player to find. This lives in `PlayerSession.swift`, a Phase 2 file,
 but is recorded here since the rebus-modal gameplay path is what surfaces it.
 Fix: treat a whitespace-only committed value as empty, mirroring the
 empty-buffer case.
 
 **3. (Code quality/robustness — low) Three render-path dictionaries built with
 the trapping `Dictionary(uniqueKeysWithValues:)` sit alongside a fourth that
 deliberately defends against duplicate keys.** `GridView.body`'s
 `authorTintByID` (`GridView.swift:42-46`), `PuzzleScoreboard.scores`
 (`PuzzleScoreboard.swift:124`), and `SuccessPanel.contributions`
 (`SuccessPanel.swift:178`) all build a `[String: T]` from
 `roster.entries`/`entries` with `Dictionary(uniqueKeysWithValues:)`, which
 traps on a duplicate `authorID`. `RecentChangeBorders.resolve`, in the same
 `GridView.swift` file, already hedges the equivalent construction with
 `Dictionary(..., uniquingKeysWith: { first, _ in first })`
 (`GridView.swift:687-690`), showing the author has considered duplicates
 possible. Today this is safe by construction —
 `ParticipantSummaries.remoteParticipants` de-duplicates via a `Set` before the
 local entry is prepended (`ParticipantSummaries.swift:59-66`,
 `PlayerRoster.swift:393`) — but the three trapping sites should adopt the same
 `uniquingKeysWith` form for defense-in-depth, since a future change to roster
 assembly would otherwise turn a benign data glitch into a hard crash while a
 puzzle is on screen.
 
 **4. (Code quality — low) `GridSilhouette.decode` accepts over-length /
 trailing-garbage payloads instead of requiring an exact bit count.** `decode`
 checks only `bits.count == storedCount` after `unpackBits` truncates to
 `storedCount` (`GridSilhouette.swift:106-108`); it never rejects a payload
 carrying *more* base64url bytes than the dimensions require, so two distinct
 segments can decode to the same grid and a segment with appended junk still
 decodes successfully. Bounded (≤35×35) and low-impact (only paints a
 link-tap placeholder), but a strict `bytes.count == (storedCount + 7) / 8`
 check would make the codec total and match the encoder's output exactly.
 
 **5. (Performance/code quality — low) The live scoreboard does repeated
 full-grid passes on every keystroke, and duplicates the same
 author-normalization/scoring logic across two files.**
 `PuzzleScoreboard.swift`'s `fillableCellCount` (`:39-43`), `filledCellCount`
 (`:45-56`), `revealedSquareCount` (`:58-69`), and `scores` (`:111-175`) each
 independently loop over every cell — four full-grid passes recomputed on each
 render, one of which (`fillableCellCount`) doesn't depend on mutable state at
 all — and `SuccessPanel.swift`'s `revealedSquareCount` (`:27-40`) and
 `contributions` (`:161-232`) repeat the pattern; `SuccessPanel`'s
 `normalizedAuthorID` (`:252-257`) is a near-verbatim duplicate of
 `PuzzleScoreboard.normalizedAuthorID` (`:177-182`), with the roster-fallback/
 extra-contributor bucketing logic duplicated almost line-for-line between the
 two files. Puzzles are bounded to 35×35, so this is genuinely negligible in
 practice (no Core Data involved, unlike the recurring blocking-fetch theme
 from Phases 1/2/4) — recorded for the redundant-work/duplication pattern, not
 as a performance emergency. Worth consolidating into one shared per-grid-stats
 helper reused by both views.
 
 **Non-findings and deferred checks:**
 - **Phase 4 finding #2 / Phase 1 finding #9 carry-over resolved:** grepped
   every `updatedAt`/`cellAuthorID`/`letterAuthorID` use across
   `Views/Puzzle/*` and confirmed the gameplay UI adds no independent
   clamping, trust check, or tie-break logic of its own — it is a passive
   renderer of `session.game.squares[r][c]` and replayed `ReplayFrame.cells`.
   The only `updatedAt` comparison in this file set
   (`GridView.swift:540`, `RemoteCursorTints.remoteTrackTints`) is cosmetic
   cursor-track tint precedence, not cell content. `Square.enqueuedAt`,
   applied in `GameStore.restore` (`GameStore.swift:2467-2520`, Phase 2,
   covered by this phase's `PendingEditFlagTests.swift`), protects a
   locally-buffered keystroke from being clobbered by an inbound merge, but
   that is a separate, correctly-functioning mechanism from the Phase 4 #2
   spoof path. Tracing the consumers does confirm the concrete UI-visible
   blast radius of that already-recorded issue, though: a poisoned
   `cellAuthorID` from a spoofed realtime frame would misattribute the
   author tint in `GridView` (`GridView.swift:78`, `:91`) and miscount
   per-player credit in `PuzzleScoreboard.scores` /
   `SuccessPanel.contributions` (`PuzzleScoreboard.swift:118`,
   `SuccessPanel.swift:143`, `:173`) — useful color for Phase 4's
   already-owned fix, not a new gameplay-side bug.
 - `PuzzleSession.swift`'s grace-timer/background-assertion state machine
   (`scheduleEndPush`/`cancelPendingEndPush`/`fireEndPush`) is correctly
   serialized with no interleaving `await` between guard and mutation, and its
   `UIBackgroundTaskIdentifier`/idle bookkeeping matches `PuzzleSessionTests`'s
   interleaving cases (including expedited-expiration and supersede paths)
   exactly.
 - Replay data is a trust boundary but is display-only and crash-safe: a
   malicious co-player's uploaded journal can misrender/misattribute a
   *finished* game's replay, but never writes back to the live `Game` or to
   CloudKit, and every consumer of a replayed cursor position is bounds-checked
   (`Puzzle.swift:366-431`, `GridView.swift:74`, `SuccessPanel.swift:240`,
   `ReplayControls`'s scrub/playback state machine).
 - `ReplayLoader`'s own CloudKit paths are non-blocking
   (`store.cachedRemoteJournals`/`cacheRemoteJournals` correctly use
   `await ctx.perform`, `GameStore.swift:1459-1525`). Its one synchronous
   main-actor read, `store.localReplaySource` → `movesJournal.recordedEntries`
   (`ReplayLoader.swift:57`), is **Phase 2 finding #5** reaffirmed from a new
   call site (fires once on the finish-banner path), not a new finding.
 - `PlayerRoster.refresh`'s `ctx.performAndWait` (`PlayerRoster.swift:256`) is
   also reached from this phase's UI (colour change in
   `PuzzleToolbarModifier.swift:218`, `PuzzleLifecycleModifier`) — **Phase 2
   finding #3** reaffirmed from a new call site, not a new finding.
 - No force-unwraps (`try!`, `as!`, `.first!`/`.last!`, bare postfix `!`)
   anywhere in `Views/Puzzle/*`, `PuzzleSession.swift`, `ReplayLoader.swift`,
   `ReplayControls.swift`, `CheckResult.swift`, or `GridSilhouette.swift`.
   `HardwareKeyboardInputView`'s HID-usage mapping safely rejects any
   unrecognized `UIKeyCommand.input` via `guard`/`default: return nil`
   (`HardwareKeyboardInputView.swift:120-171`), and custom `Layout`
   implementations (`KeyboardRow`, `PuzzleGridLayerLayout`/
   `PuzzleGridGeometry`) guard degenerate (zero-size/zero-count) input.
 - `PuzzleCommands`/`PuzzleActionTarget` equality is deliberately keyed on
   `session === && isEnabled` to stop a UIKit menu-builder reentrancy deadlock
   (`PuzzleCommands.swift:26-28`) — correct as documented, not a staleness bug.
 - `CheckResult` is a trivial two-case enum with no correctness surface;
   cross-reference hatching (`CellPatterns`/`CrossRefLines`) is pure geometry
   with `max(2, …)` step floors, no divide-by-zero.
 - `GameSummaryThumbnailTests.swift`, `ReplayCacheTests.swift`, and
   `PendingEditFlagTests.swift` exercise Phase 2 source (`GameStore.swift`'s
   `GameSummary`, `cacheRemoteJournals`/`cachedRemoteJournals`, and
   `restore`'s pending-edit flag) rather than any Phase 6 file directly; each
   was checked against the corresponding Phase 2 source and found accurate.
 - Carry over to **Phase 7**: `PuzzleView`'s `.task` beat sets
   `session.recentChanges = loadRecentChanges()` after a 750ms hold
   (`PuzzleView.swift:229-233`); `loadRecentChanges` is injected from
   `AppServices` (Phase 7) — worth confirming it isn't a synchronous
   main-thread Core Data fetch when Phase 7 reviews the composition root,
   alongside the existing Phase 4 carry-over note on
   `NotificationNavigationBroker`/`PlayerSessionNavigation`.
 
 ---
 
 ## Phase 7 — App Shell & General UI
 
 **Status:** Done
 
 Everything else: app-wide composition/DI, general browsing/list/settings
 UI, and cross-cutting diagnostics.
 
 - `Crossmate/Services/AppServices.swift`, `DebuggingMonitors.swift`
 - `Crossmate/Views/Browse/*`, `Views/GameList/*`, `Views/Settings/*`,
   `Views/Components/*`
 - `Crossmate/CrossmateApp.swift` (app-shell composition root; not listed
   under any earlier phase, reviewed here for the first time)
 - `Tests/Unit/LogScrubberTests.swift`
 
 ### Findings (consolidated from Sonnet 5 and Opus 4.8, reviewed 2026-07-01)
 
 Both passes independently confirmed the two outstanding carry-overs
 (`NotificationNavigationBroker`/"`PlayerSessionNavigation`" from Phase 4,
 `loadRecentChanges` from Phase 6) as non-findings, and both independently
 found the recurring "blocking Core Data on an async `@MainActor` path"
 pattern at new composition-root call sites in `AppServices.swift`. The two
 passes disagreed sharply on one item — Sonnet 5 flagged `RecordEditorView`
 as a high-severity finding, Opus 4.8 assessed the same view as a non-finding
 ("a power-user CloudKit-Console substitute, not a cross-trust-boundary
 surface"). This was resolved by direct verification of the source below:
 Sonnet 5's read is correct and Opus 4.8's is not — the view's `scope: .shared`
 option targets `container.sharedCloudDatabase`, i.e. zones *other users*
 have shared with the signed-in user, so it is a cross-trust-boundary surface
 by design, not merely an owner-facing tool.
 
 **1. (Security — high, verified) A raw CloudKit record browser/editor/deleter
 is reachable by any user via a client-side toggle with no other gate, turning
 every trust-boundary gap this audit has flagged in Phases 1/3/4 into a
 one-tap, no-code exploit.** `RecordEditorView.swift` lets the user pick
 `scope: .private` or `.shared`, list every zone in that scope
 (`listZonesForEdit`, `CloudDiagnostics.swift:151-162`), list every record of a
 chosen type in a zone (`queryRecordsForEdit`, `:169-190`), fetch/edit/save any
 record by name (`fetchRecordForEdit`/`saveRecordForEdit`, `:122-145`), and
 delete any record via swipe (`deleteRecordForEdit`, `:195-204`) — all backed
 by nothing but the CloudKit API itself, confirmed directly against the
 source. The only gate is `SettingsView.swift:9`'s
 `@AppStorage("debugMode") private var debugMode = false`, flipped by tapping
 the About footer text four times (`SettingsView.swift:127-133`,
 `easterEggTaps >= 4 { debugMode.toggle() }`) — a runtime UserDefaults flag any
 user can flip, not a `#if DEBUG` compile gate, developer entitlement, or
 server-side check. Once toggled, "Record Editor" appears as an ordinary
 `NavigationLink` in Settings → Debugging (`SettingsView.swift:94-96`).
 
 Critically, `scope: .shared` routes to `container.sharedCloudDatabase`
 (`CloudDiagnostics.swift:126-128`, `:139-141`, `:152-154`, `:176-177`,
 `:199-201`) — CloudKit's shared database, which surfaces every zone *another
 user* has shared with the signed-in account. This is exactly the class of
 writable-but-not-yours zone this audit has repeatedly identified as the
 concrete attack surface: any accepted friend's `friend-<pairKey>` inbox that
 the app itself doesn't expose as browsable (Phase 1 finding #1, Phase 3
 findings #1/#3/#4), and any shared game's zone. With this view, a user needs
 no reverse-engineered client and no custom CloudKit code to carry out those
 findings by hand: pick "Shared", "List zones", pick a `friend-<pairKey>`
 zone, type "Decision" as the record type, "List records in zone", tap a
 `decision-account-pushSecret` or `decision-block-<X>` record (or none —
 create one blind by record name), add/edit a String field, "Save changes".
 The swipe-to-delete action equally lets any participant with zone
 `.readWrite` (which CKShare zone-sharing grants at the zone level, not
 per-record) delete a co-player's `Game`/`Player`/`MovesEntity` records
 outright, bypassing every app-level ownership check. Fix: compile this view
 out of release builds (`#if DEBUG`) rather than gating it behind a
 user-flippable preference; if a Production-facing inspection tool is still
 wanted for support purposes, make it read-only and restrict it to the
 signed-in user's own private-database zones.
 
 **2. (Performance — medium) `AppServices` blocks the main actor with
 `ctx.performAndWait` at three call sites — the same recurring pattern flagged
 in Phases 1, 2, 4, and 6, now at the composition-root layer that wires those
 calls together.** `AppServices.start(appDelegate:)` creates a background
 context and calls `nicknameCtx.performAndWait { FriendEntity
 .rebuildNicknameDirectory(...); GameEntity.rebuildContentKeyDirectory(...) }`
 (`AppServices.swift:678-685`) synchronously on the `@MainActor`-isolated
 `start()`, during cold launch; `rebuildContentKeyDirectory`
 (`GameEntity+ContentKey.swift:13-25`) fetches every `GameEntity` with a
 non-nil `notification` field — effectively every game that has ever minted
 push credentials, not just the small friends table the neighboring comment
 describes. Separately, `reconcilePendingJournalUploads` — called from every
 `runFreshenGameList` (foreground, appear, manual pull-to-refresh, and
 remote-push-driven freshens) — does two more `ctx.performAndWait` blocks on a
 fresh background context (`AppServices.swift:1268-1271`, a `completedAt !=
 nil AND journalUploaded == NO` fetch against `GameEntity`, which per Phase 2
 finding #4 has no `fetchIndex`, so it's a full-table scan; and `:1288-1295`,
 the conditional mark-done save). All three block the calling thread — the
 main actor, since `AppServices` is `@MainActor` with no intervening `await` —
 despite the enclosing methods being `async`. The inconsistency is visible in
 the same file: `logPlayerLeaseSnapshot`, `hasPresentPeer`, `soonestPeerLease`,
 and `presentPeers` (`AppServices.swift:2225-2373`) all correctly use
 `context.perform` wrapped in `withCheckedContinuation` instead of
 `performAndWait` — the non-blocking pattern is already established elsewhere
 in this exact file. Fix: convert all three sites to the same
 `context.perform`/`withCheckedContinuation` (or `await ctx.perform`) pattern.
 
 **3. (Correctness — low) `GameListView`'s "Leave Puzzle" failure is captured
 into a dead `leaveError` state that is never displayed, unlike every sibling
 destructive action.** `leaveShare(game:)` catches a thrown error from
 `shareController.leaveShare(gameID:)` into `@State private var leaveError:
 Error?` (`GameListView.swift:61`, `:777-785`), and grepping the file confirms
 those are the *only* two references to `leaveError` — no `.alert` or
 `Announcement` ever reads it. Every other destructive action in the same file
 (resign, delete, decline) posts an `Announcement` on failure via
 `Self.destructiveActionErrorID`, and the equivalent in-puzzle leave flow
 (`PuzzleView.swift:62,609` → `PuzzleModifiers.swift:364-374`) wires its own
 `leaveError` to a `"Couldn't Leave"` alert — so the pattern is known and used
 correctly one screen over. A network blip or CloudKit error during a Game
 List "Leave" swipe fails completely silently: the row stays in the list with
 no feedback. Fix: post an `Announcement` (or add the missing `.alert`)
 mirroring the in-puzzle leave flow.
 
 **4. (Security — low) `NYTLoginView`'s sign-in-redirect detector uses a
 substring hostname check instead of an exact/suffix match.**
 `NYTWebView.Coordinator.webView(_:decidePolicyFor:)` gates cookie extraction
 on `url.host?.contains("nytimes.com") == true`
 (`NYTLoginView.swift:71`, confirmed) rather than an exact match or
 `hasSuffix(".nytimes.com")` — the standard hostname-validation footgun (it
 would also match `nytimes.com.attacker.tld`). Exploitability is narrow: the
 `WKWebView`'s only navigation entry point is the hardcoded
 `NYTAuthService.loginURL` (an `nytimes.com` URL) with no address bar, so
 reaching a look-alike host would require an actual open redirect on NYT's own
 login flow, and the extracted cookie store only ever contains cookies this
 WebView session itself set. Still a one-line fix worth making since the
 substring check offers no real benefit over an exact/suffix match.
 
 **5. (Performance/code quality — low) The "recent changes" read does a
 redundant, unbounded diagnostic Core Data fetch on every puzzle-open and
 foreground-resume beat.** The injected `loadRecentChanges` closure
 (`CrossmateApp.swift:805-817`, driving `PuzzleView`'s `.task` beat,
 `PuzzleView.swift:232`) and `recaptureRecentChanges`
 (`CrossmateApp.swift:1052-1062`, driving background→foreground resume) each
 call both `store.recentlyChangedCells(...)` (bounded, `changedAt > since`,
 `GameStore.swift:2209-2224`) and `store.recentChangesDiagnosticSummary(...)`
 (`GameStore.swift:2227-2233`) — the latter runs a **second, unbounded** fetch
 of every `PeerChangeEntity` row for the game (predicate is `gameID == %@`
 only, no time filter) purely to build one diagnostic log line. Both run on
 `GameStore.context`, which is `persistence.viewContext`
 (`GameStore.swift:508`) — a main-thread context, so this is the ordinary
 "synchronous Core Data on the render path" class already accepted elsewhere
 (Phase 2 finding #3, Phase 6's `solveTime` non-finding), not a cross-queue
 blocking stall; **this resolves the Phase 6 carry-over** on `loadRecentChanges`
 as a non-finding on the threading question specifically. The genuinely new
 issue is the redundant unbounded diagnostic fetch doubling the ledger read on
 every open/resume. Fix: gate the diagnostic-summary fetch behind an actual
 diagnostics flag, or derive both the summary and the cell map from a single
 fetch.
 
 **6. (Performance — low) `DiagnosticsView` rebuilds and rewrites the entire
 event-log dump on every log event while the screen is open.**
 `diagnosticDump` (`DiagnosticsView.swift:251-273`) string-joins the whole
 `eventLog.entries` buffer (up to 30,000 entries, `DebuggingMonitors.swift:279`)
 as a computed property; `.onChange(of: diagnosticDump)`
 (`DiagnosticsView.swift:142`) forces SwiftUI to recompute that full O(n) join
 on every body evaluation just to diff it, and each change triggers
 `refreshDiagnosticShareFile` → `writeDiagnosticShareFile` (`:155-169`), which
 rewrites the entire multi-MB dump to `temporaryDirectory` atomically. During
 an active co-solve the log churns at ~5 events/sec (documented at
 `DebuggingMonitors.swift:271-273`), so simply having the Diagnostics screen
 open triggers several full rebuilds + full-file disk rewrites per second.
 Bounded to a rarely-open debug screen. Fix: rebuild the share file lazily,
 only when the `ShareLink` is actually invoked.
 
 **7. (Code quality — low) `GameListView` wraps its entire body in a
 `GeometryReader` solely to read container height.** `GameListView.body` opens
 with `GeometryReader { geometry in ... }` (`GameListView.swift:74`) only to
 pass `geometry.size` into `usesRoomierType(for:)`
 (`size.height >= 760 && dynamicTypeSize <= .medium`, `:787-789`) — this
 contradicts the project's documented preference for `containerRelativeFrame`/
 size-class environment values over `GeometryReader`. Functionally harmless
 here (single read, no layout feedback loop), but worth swapping for a
 vertical-size-class or `containerRelativeFrame` check.
 
 **Non-findings and deferred checks:**
 - **Phase 4 carry-over resolved:** `NotificationNavigationBroker`
   (`CrossmateApp.swift:309-376`) and its siblings `CloudShareAcceptanceBroker`
   (`:379-406`) and `ShareLinkBroker` (`:416-441`) are `@MainActor` singletons
   with an identical buffer-until-handler-set pattern to survive the
   cold-launch race before `RootView.task` wires up handlers.
   `NotificationNavigationBroker` only routes an already-parsed `gameID` (from
   `AppDelegate.gameID(from:)`, which validates a UUID string or a
   `game-<uuid>` zone-name pattern and returns `nil` on anything malformed)
   into in-app navigation — no trust-boundary issue. "`PlayerSessionNavigation`"
   turned out not to be a composition-root type at all: it is only the name of
   `Tests/Unit/PlayerSessionNavigationTests.swift`, which exercises
   `PlayerSession`'s clue-navigation methods (Phase 2/6 territory) — the
   original Phase 4 note conflated a test filename with a type. That test
   file's `rebusCommitPreservesAllWhitespace` case incidentally locks in, as
   expected behavior, the exact whitespace-only-rebus bug Phase 6 finding #2
   already flagged (confirmatory, not new).
 - **Phase 6 carry-over resolved:** see finding #5 — `loadRecentChanges` is a
   synchronous main-thread fetch on `viewContext`, the same accepted class as
   Phase 2 finding #3, not the background-context-`performAndWait` anti-pattern
   this audit has been flagging; the redundant unbounded diagnostic fetch it
   also does is recorded as its own item above.
 - `DebuggingMonitors.swift` (`PerformanceMonitor`, `EventLog`, `SyncMonitor`,
   `LogScrubber`) is careful and well-tested: `LogScrubber.scrub` (truncates
   UUIDs, `_`+32-hex/bare-32-hex tokens, and iCloud share-link tokens) runs on
   the full formatted message at `EventLog.append` (`:320-321`), so
   `SyncMonitor.recordError`'s `NSError.userInfo` dump (`:415-418`) is scrubbed
   too, not just the separately-stored description — verified directly against
   `LogScrubberTests`' cases. One honest caveat: base64/base64url secrets (the
   account push secret, content/encryption keys) are not hex and would pass
   through unscrubbed, but nothing in this phase's file set (or elsewhere,
   outside the already-recorded Phase 5 `NYTPuzzleFetcher` cookie finding) logs
   those — headroom, not a live leak.
 - No force-unwraps (`try!`, `as!`, `.first!`/`.last!`, bare postfix `!`) on
   untrusted/CloudKit-sourced or user-controlled data anywhere in scope
   (confirmed by grep across the full file list); the only `!` sites are on
   hardcoded constants (`TimeZone(identifier: "America/New_York")!`, two
   hardcoded `URL(string:)!` links in `AboutView.swift`) or modulo-bounded
   weekday-symbol lookups in `NYTBrowseView.swift`.
 - The inviter-controlled `invite.gridSilhouette`, decoded via
   `GridSilhouette.decode` on the Game List and accept path
   (`GameListView.swift:602`, `:696`), is a real CloudKit trust boundary but
   was already verified crash-safe and bounded (≤35×35) in Phase 6 non-finding
   #4; a `nil` decode here just renders no thumbnail.
 - The invite-accept path (`GameListView.accept`, `:693-717`) only forwards the
   inviter-controlled `shareURL`/`pingRecordName` to
   `InviteCoordinator.acceptInvite` (Phase 3), which owns validation. The
   account-control push path (`AppServices.handleAccountControlPush`,
   `:1714-1774`) adopts `readAt`/`gameID` only from `accountJoined`/
   `accountSeen` pushes targeting the random, account-private `acct-<UUID>`
   address (Phase 4 non-finding: not spoofable without the account zone) and
   self-guards on `senderDeviceID == localDeviceID` — sibling-device-scoped,
   not a co-player boundary.
 - `GameShareSheet.describe` (`GameShareItem.swift:355-361`) copies a full
   unscrubbed `nsError.userInfo` dump to the pasteboard on "Copy Error",
   bypassing `LogScrubber` unlike the shareable diagnostics dump — owner-facing
   and transient, not raised as a numbered finding, but worth aligning with the
   scrubber if this ad-hoc error surface is ever meant to be shared further.
 - `PerformanceMonitor`, `EventLog`/`EventLogStore` (bounded 30k entries + 24h
   retention, debounced off-main persist), `AnnouncementCenter`-driven banners,
   `NewGameSheet`/`ImportedBrowseView` (thin dispatchers into Phase 5's
   import/XD path), and the remaining `Views/Components/*` are straightforward
   with no bugs found. `FriendAvatarView`'s ring-reveal `Task` is properly
   cancelled on view changes. `ImportedBrowseView`'s uncapped
   `DriveMonitor.readSource`/`importFile` is the already-recorded Phase 4→5
   carry-over, not a new finding.
 - Nothing new carries forward — Phase 7 is the last phase in the plan, and
   every app-shell issue found here terminates in a layer already reviewed
   (Phase 2 indexing, Phase 4 push scope, Phase 5 import path, Phase 6
   replay/recent-changes).
 
 ---
 
 ## Process
 
 Work through phases in order above (roughly risk/complexity order). For each
 phase:
 
 1. Mark it "In progress".
 2. Review the listed files (and anything they pull in that isn't listed
    elsewhere).
 3. Record findings inline in this file under the phase, or in a linked doc if
    long.
 4. Mark it "Done" and move to the next phase.