diff --git a/packages/server/src/broadcast.ts b/packages/server/src/broadcast.ts index d87c8fc..0e42fe5 100644 --- a/packages/server/src/broadcast.ts +++ b/packages/server/src/broadcast.ts @@ -10,7 +10,13 @@ // or the event loop via timers — handlers are synchronous and O(facts). import type { ServerWebSocket } from "bun"; -import { GameSessionRegistry } from "./game-session.js"; +import { + GameSessionRegistry, + applyPendingProfile, + clearPendingProfile, + setPendingProfile, + type GameSession, +} from "./game-session.js"; import { logger } from "./logger.js"; import { incMessages, @@ -265,6 +271,7 @@ export function handleMessage( case "game.end": case "game.presets": case "modifier-profile.updated": + case "modifier-profile.queued": case "error": sendTo( ws, @@ -702,6 +709,13 @@ function handleGameMove( broadcastToRoom(roomCode, deltaMsg); bufferDeltaForDisconnected(roomCode, token, deltaMsg.seq, deltaPayload); + // Turn-boundary pending-profile drain (T2-ADR-1). Runs AFTER the + // move delta is broadcast so clients observe the authoritative + // ordering: "move first, then rule changes". A no-op when no + // profile is queued. Either player's successful move triggers + // this — the pending slot is shared across the room. + applyPendingProfileIfAny(roomCode, session); + // If any preset durations expired during this move's tick, push the // new set so clients stop rendering those rules. We skip the broadcast // when the set is byte-identical to pre-move — the common case — @@ -732,46 +746,35 @@ function handleGameMove( * ## Authority model * * Only the HOST (white — first connected, lowest-indexed player) may - * issue this command. Black can propose profile changes out-of-band, - * but the host has final say. This mirrors how `room.setPresets` - * worked implicitly (tests always sent it from white); we formalise - * the rule here because profiles are more consequential (they mutate - * every piece's capabilities). + * issue this command. Two-player consent (T2-ADR-2) is a separate, + * future flow; this handler remains the host-unilateral path used by + * solo mode and tests. * - * ## Turn-boundary semantics (T20 simplification) + * ## Turn-boundary queue (T2-ADR-1) * - * ADR-3 specifies "apply at the next turn boundary". The full - * implementation would queue the update onto the room and drain it - * after the in-flight `game.move` completes. However, because WS - * message processing is single-threaded per socket AND the server - * completes each `handleMessage` call synchronously before reading - * the next frame, a client cannot interleave a `modifier-profile.update` - * between its own `game.move` request and the server's reply. The - * handler therefore applies immediately; from the client's view this - * IS the turn boundary. + * Unlike T1, receipt does NOT apply the swap. Instead the handler: * - * What this DOESN'T prevent: a client submitting a profile update - * while its OPPONENT's move is in flight. The current simplification - * accepts this — the swap lands between the two players' moves, - * which is still a turn boundary under any reasonable definition. - * If a stricter "only between full game plies" policy is ever - * required, we'll revisit with a proper queue. - * - * ## Pipeline - * - * 1. Authenticate: socket must be bound to the room and be the host. - * 2. Sanity: session must exist, game must not be over. - * 3. Staleness: incoming `version` must equal the session's current - * `profileVersion` — rejects concurrent edits from stale editors. - * 4. Validate: run `validateProfile` against the session's layout. - * Any error → NACK to the sender with the mapped wire code. - * 5. Apply: `GameSession.reconcileProfile` seeds/retracts modifier - * facts via the ADR-3 retract-then-reapply path and bumps the + * 1. Authenticates the sender as the host; resolves the room and + * session; checks liveness (no GAME_OVER) and version freshness. + * 2. Runs `validateProfile` against the layout as an *early + * rejection* gate — an obviously-illegal profile (no king, + * invuln king, attribute overflow) is NACK'd here so the client + * gets immediate feedback instead of discovering the error a + * full turn later. + * 3. On acceptance, stashes the profile on `room.pendingProfile` + * via `setPendingProfile` (last-write-wins — a prior pending is + * overwritten) and ACKs the sender with + * `modifier-profile.queued` carrying the expected post-apply * version. - * 6. Mirror into room state so late joiners see the new profile. - * 7. Broadcast `modifier-profile.updated` to every connection in - * the room (including the sender — they want the authoritative - * echo to confirm their request landed). + * + * The swap itself — `reconcileProfileSwap`, version bump, room + * mirror, `modifier-profile.updated` broadcast — happens ONLY in + * `applyPendingProfileIfAny` after the next successful `applyMove` + * (either player's move drains the queue). + * + * This eliminates the T1 race where an opponent's in-flight move + * validated against a profile they didn't agree to. See T2-ADR-1 + * for full rationale. */ function handleModifierProfileUpdate( ws: ServerWebSocket, @@ -848,7 +851,9 @@ function handleModifierProfileUpdate( } // Reject updates to a finished game — there's no meaningful way - // to "apply" a profile to a position that can no longer change. + // to "apply" a profile to a position that can no longer change, + // and the turn boundary that would drain the queue is never + // going to fire. if (session.getGameOver() !== null) { sendTo( ws, @@ -862,9 +867,15 @@ function handleModifierProfileUpdate( // moved past that, their request is based on outdated state // (another editor swapped while they composed). Reject with the // generic INVALID wire code — same family as "schema-invalid" - // because the OUTCOME is the same (the update won't apply) and - // adding a new wire code for every failure mode bloats the - // taxonomy. + // because the OUTCOME is the same (the update won't apply). + // + // NOTE (T2): `profileVersion` is NOT bumped when a profile is + // merely queued — it advances only when a pending profile + // actually applies at a turn boundary. So a second + // `modifier-profile.update` arriving between receipt and the + // boundary carries the SAME version as the first and passes this + // gate cleanly. That is intentional: it's how last-write-wins is + // expressed on the wire. if (payload.version !== session.getProfileVersion()) { sendTo( ws, @@ -877,10 +888,17 @@ function handleModifierProfileUpdate( return; } - // Validate against the layout. The schema already accepted the + // Early-rejection validation. The schema already accepted the // wire shape; validateProfile enforces game-rule legality // (king presence, invuln-king, attribute ceiling). We report the // FIRST error so the client gets a single actionable code. + // + // Profile validation is stateless w.r.t. session facts — it only + // inspects the profile + layout — so running it here short- + // circuits obviously-bad inputs that would otherwise sit in the + // pending slot until the next move before being rejected. The + // same check runs again in `applyPendingProfile` as defence in + // depth against a future validator that grows stateful. const newProfile = asChessProfile(payload.newProfile); const check = validateProfile(newProfile, room.layout); if (!check.valid) { @@ -896,32 +914,120 @@ function handleModifierProfileUpdate( return; } - // Apply. `reconcileProfile` owns the retract-then-reapply and the - // Hp clamp (ADR-3). It also bumps the internal profileVersion so - // subsequent broadcasts / echoes carry the post-swap number. - session.reconcileProfile(newProfile); + // Stash as pending (T2-ADR-1). Overwrites any previously-queued + // profile — this is the wire-level expression of last-write-wins + // on the single-slot queue. The actual reconcile + version bump + // + broadcast happens in `applyPendingProfileIfAny` AFTER the + // next successful move. + setPendingProfile(room, newProfile, token); - // Mirror onto the Room so `room.joined` late-joiners see the - // current profile, not the original one. The session is the - // authoritative copy for engine state; the room copy is the - // replay source for new connections. - room.profile = newProfile; - - // Broadcast to everyone in the room including the sender. - broadcastToRoom( - roomCode, - envelope("modifier-profile.updated", { - profile: newProfile, - version: session.getProfileVersion(), - appliedAt: "turn-boundary", + // Ack the SUBMITTER only. The opponent doesn't hear about a + // queued pending until it actually lands via + // `modifier-profile.updated` at the next turn boundary. + // pendingVersion = current + 1 (the version the server will + // emit if the pending applies successfully). + sendTo( + ws, + envelope("modifier-profile.queued", { + roomCode, + pendingVersion: session.getProfileVersion() + 1, }), ); logger .child({ clientId: ws.data.clientId, roomCode }) - .info("modifier-profile.update"); + .info("modifier-profile.update (queued)"); } +/** + * Drain the room's pending profile — if any — and emit the + * appropriate wire traffic. Called immediately after a successful + * `applyMove` so observable ordering on the wire is: + * + * 1. game.delta (the move that just happened) + * 2. modifier-profile.updated (if pending applied cleanly) + * — OR — + * error → original proposer (if pending was rejected at + * apply-time; no broadcast fires) + * 3. game.presets / game.end (if applicable) + * + * Callers: `handleGameMove`. No-op when no profile is pending. + */ +function applyPendingProfileIfAny( + roomCode: string, + session: GameSession, +): void { + const room = roomRegistry.getRoom(roomCode); + if (!room) return; + + const result = applyPendingProfile(room, session); + switch (result.kind) { + case "none": + return; + case "applied": { + // Broadcast the authoritative echo to everyone in the room, + // including the sender — same semantics as the T1 immediate + // path, just triggered at the turn boundary instead of on + // receipt. + broadcastToRoom( + roomCode, + envelope("modifier-profile.updated", { + profile: result.profile, + version: result.version, + appliedAt: "turn-boundary", + }), + ); + logger + .child({ roomCode }) + .info("modifier-profile applied at turn boundary"); + return; + } + case "rejected": { + // Apply-time re-validation failed (unlikely with the current + // stateless validator — see the comment in + // `applyPendingProfile` — but handled so future validator + // changes degrade gracefully). Route the NACK to the original + // proposer. If the proposer has disconnected AND the grace + // window expired, the token lookup returns no socket and the + // NACK is silently dropped — at that point the proposer has + // bigger problems than a stale queued profile, and the + // pending slot has already been cleared. + const proposerSocket = findSocketByToken(result.proposerToken); + if (proposerSocket !== undefined) { + sendTo( + proposerSocket, + errorMessage( + mapProfileValidationCode(result.errorCode), + result.errorMessage, + false, + ), + ); + } + logger + .child({ roomCode }) + .warn("pending profile rejected at turn boundary"); + return; + } + } +} + +/** Resolve a player token to its currently-connected socket, or + * undefined if the slot has no live connection. Used when routing + * deferred NACKs (pending-profile apply-time rejection). */ +function findSocketByToken( + token: string, +): ServerWebSocket | undefined { + for (const ws of connections.values()) { + if (ws.data.token === token) return ws; + } + return undefined; +} + +// `clearPendingProfile` is re-exported for tests and future wire +// flows (profile-abort, game-end cleanup) that need to drop the +// slot without going through `applyPendingProfile`. +export { clearPendingProfile }; + function handleSetPresets( ws: ServerWebSocket, payload: RoomSetPresetsPayload, diff --git a/packages/server/src/game-session.ts b/packages/server/src/game-session.ts index 5725edd..d0364e7 100644 --- a/packages/server/src/game-session.ts +++ b/packages/server/src/game-session.ts @@ -15,8 +15,10 @@ import { algebraicToSquare, PresetActivationError, reconcileProfileSwap, + validateProfile, type ActivationRequest, type ModifierProfile, + type ModifierValidationErrorCode, type PresetActivation, type GameResult, type PieceColor, @@ -24,6 +26,8 @@ import { type StartingLayout, } from "@paratype/chess"; +import type { Room } from "./rooms.js"; + // --------------------------------------------------------------------------- // Public types // --------------------------------------------------------------------------- @@ -480,3 +484,119 @@ function mapGameResult( } } } + +// --------------------------------------------------------------------------- +// Turn-boundary profile queue (T2-ADR-1) +// --------------------------------------------------------------------------- +// +// Per ADR, a `modifier-profile.update` accepted at receipt is NOT applied +// immediately — it is stashed on the Room as `pendingProfile` and applied +// AFTER the next successful `applyMove()`. This keeps the game state at +// turn N a pure function of {profile at turn N, moves 1..N} and eliminates +// the race where an opponent's in-flight move validates against a profile +// they didn't agree to. +// +// The queue is deliberately a single slot (not a FIFO): rapid successive +// updates overwrite, so "last write wins" on the same turn. See ADR +// §Rejected Alternatives for why a per-sender queue was not chosen. + +/** + * Stash a freshly-received profile on the Room as pending. Overwrites + * any previously-queued profile (last-write-wins). Caller has already: + * 1. Zod-validated the wire shape. + * 2. Run `validateProfile` against the layout — receipt-time + * legality gate; apply-time re-validation happens later. + * 3. Authenticated the proposer as the host. + * + * `proposerToken` is the player token of the submitter — stored so + * an apply-time NACK can be routed back even across reconnects. + */ +export function setPendingProfile( + room: Room, + profile: ModifierProfile, + proposerToken: string, +): void { + room.pendingProfile = profile; + room.pendingProposerToken = proposerToken; +} + +/** Forget any pending profile. Idempotent — safe to call when the + * slot is already empty. */ +export function clearPendingProfile(room: Room): void { + delete room.pendingProfile; + delete room.pendingProposerToken; +} + +/** + * Outcome of a turn-boundary pending-profile apply attempt. The + * caller (broadcast.ts) uses this to decide what wire traffic to emit: + * - `none` : no pending profile was queued; nothing happens. + * - `applied` : swap succeeded; broadcast `modifier-profile.updated`. + * - `rejected`: apply-time re-validation failed; NACK the proposer + * and clear the slot (no broadcast). + */ +export type PendingProfileApplyResult = + | { kind: "none" } + | { + kind: "applied"; + profile: ModifierProfile; + version: number; + } + | { + kind: "rejected"; + proposerToken: string; + errorCode: ModifierValidationErrorCode; + errorMessage: string; + }; + +/** + * Drain the room's pending profile at a turn boundary. Re-runs + * `validateProfile` against the current layout (defence in depth for + * a future where layout could change mid-queue), then commits the + * swap via `GameSession.reconcileProfile` on success. + * + * On success: pending slot cleared, session profile + version + * updated, room mirror updated, returns `{ kind: "applied", ... }`. + * On failure: pending slot cleared, session and room UNCHANGED, + * returns `{ kind: "rejected", ... }` carrying the original + * proposer's token so the caller can route the NACK. + * + * No pending → `{ kind: "none" }`. Callers typically short-circuit + * on this to avoid allocating an empty broadcast. + */ +export function applyPendingProfile( + room: Room, + session: GameSession, +): PendingProfileApplyResult { + const pending = room.pendingProfile; + const proposerToken = room.pendingProposerToken; + if (pending === undefined || proposerToken === undefined) { + return { kind: "none" }; + } + + // Re-validate. Profile validation is currently stateless w.r.t. + // session facts (it only inspects the profile + layout), so a + // receipt-time pass implies an apply-time pass today. We re-check + // anyway so a future validator that consults session state can't + // silently regress this contract. + const check = validateProfile(pending, room.layout); + if (!check.valid) { + const first = check.errors[0]!; + clearPendingProfile(room); + return { + kind: "rejected", + proposerToken, + errorCode: first.code, + errorMessage: first.message, + }; + } + + // Commit. `reconcileProfile` owns the retract-then-reapply + Hp + // clamp and bumps profileVersion; the room mirror tracks the + // authoritative copy for late joiners. + session.reconcileProfile(pending); + room.profile = pending; + const version = session.getProfileVersion(); + clearPendingProfile(room); + return { kind: "applied", profile: pending, version }; +} diff --git a/packages/server/src/protocol.ts b/packages/server/src/protocol.ts index ba625b9..b0e2631 100644 --- a/packages/server/src/protocol.ts +++ b/packages/server/src/protocol.ts @@ -324,6 +324,30 @@ export type ModifierProfileUpdatedPayload = z.infer< readonly type?: "modifier-profile.updated"; }; +/** + * Server → client: ack that a `modifier-profile.update` request has + * been queued for the next turn boundary (T2-ADR-1). Sent to the + * submitting client ONLY — the opponent doesn't hear about a queued + * profile until it actually lands via `modifier-profile.updated`. + * + * `pendingVersion` is the version the server EXPECTS to emit when + * the pending profile successfully applies — i.e. the current + * `profileVersion` + 1. The client uses it to detect a stale + * round-trip: if `modifier-profile.updated.version > pendingVersion`, + * another swap landed ahead of theirs and their pending was + * overwritten (last-write-wins). + */ +export const ModifierProfileQueuedPayloadSchema = z.object({ + roomCode: RoomCodeSchema, + /** Expected post-apply version. Advisory — the authoritative + * version appears on the ensuing `modifier-profile.updated` + * broadcast once the swap lands. */ + pendingVersion: z.number().int().min(1), +}); +export type ModifierProfileQueuedPayload = z.infer< + typeof ModifierProfileQueuedPayloadSchema +>; + export const RoomJoinPayloadSchema = z.object({ code: RoomCodeSchema, }); @@ -519,6 +543,10 @@ export const ModifierProfileUpdatedMessageSchema = msg( "modifier-profile.updated", ModifierProfileUpdatedPayloadSchema, ); +export const ModifierProfileQueuedMessageSchema = msg( + "modifier-profile.queued", + ModifierProfileQueuedPayloadSchema, +); export const ErrorMessageSchema = msg("error", ErrorPayloadSchema); export const ServerMessageSchema = z.discriminatedUnion("type", [ @@ -529,6 +557,7 @@ export const ServerMessageSchema = z.discriminatedUnion("type", [ GameEndMessageSchema, GamePresetsMessageSchema, ModifierProfileUpdatedMessageSchema, + ModifierProfileQueuedMessageSchema, ErrorMessageSchema, ]); export type ServerMessage = z.infer; @@ -547,6 +576,7 @@ export const AnyMessageSchema = z.discriminatedUnion("type", [ GameEndMessageSchema, GamePresetsMessageSchema, ModifierProfileUpdatedMessageSchema, + ModifierProfileQueuedMessageSchema, ErrorMessageSchema, ]); export type AnyMessage = z.infer; @@ -565,6 +595,7 @@ export const KNOWN_MESSAGE_TYPES = [ "game.end", "game.presets", "modifier-profile.updated", + "modifier-profile.queued", "error", ] as const; export type MessageType = (typeof KNOWN_MESSAGE_TYPES)[number]; diff --git a/packages/server/src/rooms.ts b/packages/server/src/rooms.ts index f42c6e5..19b772e 100644 --- a/packages/server/src/rooms.ts +++ b/packages/server/src/rooms.ts @@ -44,6 +44,26 @@ export interface Room { * and the server replays it onto reconstructed engines. */ profile?: ModifierProfile; + /** + * Queued profile swap awaiting the next turn boundary (T2-ADR-1). + * + * A `modifier-profile.update` request that passes receipt-time + * validation is stored here and applied by the next successful + * `applyMove()`. Last-write-wins: a second update before the + * boundary overwrites the pending slot. Cleared on apply or on + * NACK at apply time. + */ + pendingProfile?: ModifierProfile; + /** + * Stable identity (player token) of the client that submitted the + * currently-pending profile. Used to route an apply-time NACK back + * to the original proposer if the pending profile is re-validated + * and fails at the turn boundary. Token is preferred over socket + * id because tokens survive reconnect, so a brief disconnect + * between queue-and-apply still delivers the NACK when the client + * returns. + */ + pendingProposerToken?: string; } export type JoinResult = diff --git a/packages/server/src/ws.modifier-profile-update.test.ts b/packages/server/src/ws.modifier-profile-update.test.ts index ae93229..0d4a8b0 100644 --- a/packages/server/src/ws.modifier-profile-update.test.ts +++ b/packages/server/src/ws.modifier-profile-update.test.ts @@ -1,17 +1,22 @@ /** - * T20 integration tests: `modifier-profile.update` WebSocket handler. + * T2 integration tests: `modifier-profile.update` WebSocket handler + * with turn-boundary queue semantics (T2-ADR-1). * - * Covers the full server dispatch pipeline for a hot-swap request: - * 1. Happy path — host sends a valid profile update, server validates, - * reconciles the session, bumps the version, and broadcasts - * `modifier-profile.updated` to BOTH players. - * 2. Invalid profile (CANNOT_BE_CAPTURED on king) — NACK to sender, - * no broadcast to the opponent, room.profile unchanged, version - * NOT bumped. - * 3. Non-host sender (black) — `BAD_TOKEN` error back to sender, - * host unaffected, no broadcast. - * 4. Stale version — client's version lags the server → rejected - * with `MODIFIER_PROFILE_INVALID`, no mutation. + * Covers the full server dispatch pipeline for a hot-swap request + * under the new queue model: + * 1. Receipt → `modifier-profile.queued` ACK to submitter only. + * No broadcast yet, session + room profile UNCHANGED. + * 2. Next successful `applyMove` drains the pending slot → + * `modifier-profile.updated` broadcast to both players, version + * bumps, session + room reflect the swap. + * 3. Last-write-wins: two updates between turn boundaries → only + * the second is applied when the move lands. + * 4. Either player's move drains the queue (white's move works; + * black's move works). + * 5. Invalid profile (CANNOT_BE_CAPTURED on king) → early rejection + * on receipt (before queuing). No pending stored. + * 6. Non-host sender, stale version — all still early-reject as in + * T1. * * Harness mirrors `room.create-profile.test.ts`: mock ServerWebSockets * drive `handleMessage` through the real Zod validation + dispatch @@ -88,6 +93,26 @@ function hasMsgOfType(ws: MockWs, type: string): boolean { ); } +/** + * Drain *all* messages of a given type from an inbox and return + * them in order. Useful when a single scenario fires multiple + * broadcasts of the same type (e.g. a `game.delta` per move). + */ +function drainMsgsOfType( + ws: MockWs, + type: string, +): { payload: Record }[] { + const out: { payload: Record }[] = []; + for (let i = ws.sent.length - 1; i >= 0; i--) { + const m = ws.sent[i] as { type?: string; payload?: Record }; + if (m.type === type) { + out.unshift(m as { payload: Record }); + ws.sent.splice(i, 1); + } + } + return out; +} + /** * Envelope `type` and `payload.type` both carry `"modifier-profile.update"`. * The envelope type is what the ClientMessageSchema dispatches on; the @@ -117,7 +142,7 @@ function sendClient( /** Initial profile used to open the room. +1 HP on all white pawns. */ const initialProfile = { - id: "t20-initial", + id: "t2-initial", name: "Initial", description: "All white pawns start with +1 HP.", layoutId: "classic", @@ -134,10 +159,9 @@ const initialProfile = { source: "custom" as const, }; -/** A valid hot-swap target: +1 RangeBonus on rooks. Different kind, - * different pieces, so it's a meaningful diff from initialProfile. */ +/** A valid hot-swap target: +1 RangeBonus on rooks. */ const swapProfile = { - id: "t20-swap", + id: "t2-swap", name: "Swap", description: "Rooks get +1 range.", layoutId: "classic", @@ -154,11 +178,32 @@ const swapProfile = { source: "custom" as const, }; +/** A second valid swap — different id / shape — used to prove + * last-write-wins: if the final applied profile is this one, the + * queue overwrote, as designed. */ +const swapProfileB = { + id: "t2-swap-b", + name: "SwapB", + description: "Bishops get +1 range (overwrites any prior queued).", + layoutId: "classic", + perType: [ + { + kind: "range-bonus" as const, + pieceType: "bishop" as const, + color: "both" as const, + value: 1, + }, + ], + perInstance: [], + version: 1 as const, + source: "custom" as const, +}; + /** Invalid profile: marks both kings as CANNOT_BE_CAPTURED. Validator * flags this as `E_PROFILE_INVULN_KING`; the wire code is * `MODIFIER_PROFILE_INVULN_KING`. */ const invulnKingProfile = { - id: "t20-invuln-kings", + id: "t2-invuln-kings", name: "Invulnerable Kings", description: "Kings cannot be captured.", layoutId: "classic", @@ -202,15 +247,15 @@ function updatePayload( * * Version semantics: `createRoom` → session profileVersion = 1, so * happy-path tests use `version: 1` on their first update and get - * bumped to 2. + * bumped to 2 AFTER the next move applies the pending profile. */ function setupRoom(): { white: MockWs; black: MockWs; code: string; } { - const white = makeMockWs(`T20-W-${crypto.randomUUID()}`); - const black = makeMockWs(`T20-B-${crypto.randomUUID()}`); + const white = makeMockWs(`T2-W-${crypto.randomUUID()}`); + const black = makeMockWs(`T2-B-${crypto.randomUUID()}`); registerConnection(white); registerConnection(black); @@ -231,13 +276,13 @@ function setupRoom(): { // ─── Tests ─────────────────────────────────────────────────────────── -describe("modifier-profile.update WS handler (T20)", () => { - it("host submits a valid profile → broadcast to both players, version bumps, session swapped", () => { +describe("modifier-profile.update WS handler (T2 turn-boundary queue)", () => { + it("valid update on receipt → queued ACK to sender only, no session/room mutation, no broadcast", () => { const { white, black, code } = setupRoom(); - // Sanity: session opened with profileVersion = 1 (profile at create). const preSession = sessionRegistry.get(code); expect(preSession?.getProfileVersion()).toBe(1); + const preProfileId = preSession?.getProfile()?.id; sendClient( white, @@ -245,7 +290,47 @@ describe("modifier-profile.update WS handler (T20)", () => { updatePayload(code, swapProfile, 1), ); - // Both players receive the authoritative echo. + // Sender receives `modifier-profile.queued` — NOT `modifier-profile.updated`. + const queued = nextMsgOfType(white, "modifier-profile.queued"); + expect(queued.payload["roomCode"]).toBe(code); + // Expected post-apply version is current + 1 (advisory ack). + expect(queued.payload["pendingVersion"]).toBe(2); + + // Opponent sees nothing — queued is submitter-only. + expect(hasMsgOfType(black, "modifier-profile.queued")).toBe(false); + expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); + expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); + + // Session + room profile UNCHANGED at receipt time. + const session = sessionRegistry.get(code)!; + expect(session.getProfileVersion()).toBe(1); + expect(session.getProfile()?.id).toBe(preProfileId); + expect(roomRegistry.getRoom(code)?.profile?.id).toBe(preProfileId); + + // Pending slot populated. + expect(roomRegistry.getRoom(code)?.pendingProfile?.id).toBe(swapProfile.id); + }); + + it("queued profile applies AFTER the next successful move → updated broadcast to both, version bumps, facts swap", () => { + const { white, black, code } = setupRoom(); + + sendClient( + white, + "modifier-profile.update", + updatePayload(code, swapProfile, 1), + ); + // Drain the queued ACK so later `updated` assertion is clean. + nextMsgOfType(white, "modifier-profile.queued"); + + // Still no updated broadcast pre-move. + expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); + expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); + + // White plays a legal opening move — this is the turn boundary + // that drains the pending profile queue. + sendClient(white, "game.move", { from: "e2", to: "e4" }); + + // Both players receive the authoritative echo AFTER the move. const updatedForWhite = nextMsgOfType(white, "modifier-profile.updated"); const updatedForBlack = nextMsgOfType(black, "modifier-profile.updated"); @@ -260,27 +345,115 @@ describe("modifier-profile.update WS handler (T20)", () => { expect(echoedProfile.id).toBe(swapProfile.id); expect(echoedProfile.perType).toEqual(swapProfile.perType); - // Session state reflects the swap. - const session = sessionRegistry.get(code); - expect(session?.getProfileVersion()).toBe(2); - expect(session?.getProfile()?.id).toBe(swapProfile.id); + // Session reflects the swap. + const session = sessionRegistry.get(code)!; + expect(session.getProfileVersion()).toBe(2); + expect(session.getProfile()?.id).toBe(swapProfile.id); - // HpBonus facts (from the OLD profile) should be gone; RangeBonus - // facts (from the new profile) should be present. - const facts = session!.getAllFacts(); + // HpBonus facts gone, RangeBonus facts present. + const facts = session.getAllFacts(); expect(facts.some((f) => f.attr === "HpBonus")).toBe(false); expect(facts.some((f) => f.attr === "RangeBonus")).toBe(true); - // Room registry mirror is updated too so late joiners see the new - // profile on `room.joined`. - const room = roomRegistry.getRoom(code); - expect(room?.profile?.id).toBe(swapProfile.id); - - // No error was sent to sender. - expect(hasMsgOfType(white, "error")).toBe(false); + // Room registry mirror updated. + expect(roomRegistry.getRoom(code)?.profile?.id).toBe(swapProfile.id); + // Pending slot drained. + expect(roomRegistry.getRoom(code)?.pendingProfile).toBeUndefined(); }); - it("invalid profile (CANNOT_BE_CAPTURED on king) → NACK to sender, no broadcast, no state mutation", () => { + it("last-write-wins: two rapid updates before a move → only the SECOND applies on next move", () => { + const { white, black, code } = setupRoom(); + + // First update carries version=1. Version is NOT bumped on queue + // (only on apply), so the second update ALSO legitimately carries + // version=1 and should pass the staleness gate. + sendClient( + white, + "modifier-profile.update", + updatePayload(code, swapProfile, 1), + ); + sendClient( + white, + "modifier-profile.update", + updatePayload(code, swapProfileB, 1), + ); + + // Both receive queued ACKs (drain to keep inbox clean). + const q1 = nextMsgOfType(white, "modifier-profile.queued"); + const q2 = nextMsgOfType(white, "modifier-profile.queued"); + expect(q1.payload["pendingVersion"]).toBe(2); + expect(q2.payload["pendingVersion"]).toBe(2); + + // Pending slot holds swapProfileB (the second one) — last-write-wins. + expect(roomRegistry.getRoom(code)?.pendingProfile?.id).toBe( + swapProfileB.id, + ); + + // Turn boundary. + sendClient(white, "game.move", { from: "e2", to: "e4" }); + + // Exactly one `modifier-profile.updated` broadcast (not two). + const whiteUpdated = drainMsgsOfType(white, "modifier-profile.updated"); + const blackUpdated = drainMsgsOfType(black, "modifier-profile.updated"); + expect(whiteUpdated.length).toBe(1); + expect(blackUpdated.length).toBe(1); + + // Only swapProfileB applied — swapProfile was overwritten. + const profile = whiteUpdated[0]!.payload["profile"] as { id: string }; + expect(profile.id).toBe(swapProfileB.id); + expect(sessionRegistry.get(code)?.getProfile()?.id).toBe(swapProfileB.id); + expect(sessionRegistry.get(code)?.getProfileVersion()).toBe(2); + }); + + it("opponent's move drains the queue too (not just the proposer's)", () => { + const { white, black, code } = setupRoom(); + + // Scenario: first cycle is white-queue → white-move (drains). Then + // white queues a SECOND swap and BLACK moves to drain it. This + // proves either player's move triggers the boundary. + + // Round 1: white queues profile A, white moves → drains A. + sendClient( + white, + "modifier-profile.update", + updatePayload(code, swapProfile, 1), + ); + nextMsgOfType(white, "modifier-profile.queued"); + sendClient(white, "game.move", { from: "e2", to: "e4" }); + drainMsgsOfType(white, "modifier-profile.updated"); + drainMsgsOfType(black, "modifier-profile.updated"); + drainMsgsOfType(white, "game.delta"); + drainMsgsOfType(black, "game.delta"); + + // Sanity: session on version 2, swapProfile active, now black to move. + expect(sessionRegistry.get(code)?.getProfileVersion()).toBe(2); + expect(sessionRegistry.get(code)?.getTurn()).toBe("black"); + + // Round 2: white queues profile B (turn still belongs to BLACK). + sendClient( + white, + "modifier-profile.update", + updatePayload(code, swapProfileB, 2), + ); + nextMsgOfType(white, "modifier-profile.queued"); + + // Queue is loaded but not yet applied. + expect(sessionRegistry.get(code)?.getProfileVersion()).toBe(2); + expect(roomRegistry.getRoom(code)?.pendingProfile?.id).toBe( + swapProfileB.id, + ); + + // Black plays — drains the queue. + sendClient(black, "game.move", { from: "e7", to: "e5" }); + + const updatedForWhite = nextMsgOfType(white, "modifier-profile.updated"); + const updatedForBlack = nextMsgOfType(black, "modifier-profile.updated"); + expect(updatedForWhite.payload["version"]).toBe(3); + expect(updatedForBlack.payload["version"]).toBe(3); + expect(sessionRegistry.get(code)?.getProfile()?.id).toBe(swapProfileB.id); + }); + + it("invalid profile (CANNOT_BE_CAPTURED on king) → early NACK on receipt, no pending stored, no broadcast", () => { const { white, black, code } = setupRoom(); const versionBefore = sessionRegistry.get(code)!.getProfileVersion(); const profileBefore = sessionRegistry.get(code)!.getProfile()?.id; @@ -291,28 +464,36 @@ describe("modifier-profile.update WS handler (T20)", () => { updatePayload(code, invulnKingProfile, versionBefore), ); - // Sender gets the specific wire code. + // Sender gets the specific wire code on RECEIPT (not deferred). const err = nextMsgOfType(white, "error"); expect(err.payload["code"]).toBe("MODIFIER_PROFILE_INVULN_KING"); expect(err.payload["fatal"]).toBe(false); expect(String(err.payload["message"])).toMatch(/CANNOT_BE_CAPTURED/); - // Opponent NEVER sees a modifier-profile.updated (broadcast did - // not fire because validation failed before reconcile). + // No queued ack fired either — early rejection path. + expect(hasMsgOfType(white, "modifier-profile.queued")).toBe(false); + expect(hasMsgOfType(black, "modifier-profile.queued")).toBe(false); + + // Opponent NEVER sees a modifier-profile.updated. expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); - // Sender also doesn't see the updated broadcast. expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); + // Pending slot untouched. + expect(roomRegistry.getRoom(code)?.pendingProfile).toBeUndefined(); + // Version and profile unchanged. const session = sessionRegistry.get(code)!; expect(session.getProfileVersion()).toBe(versionBefore); expect(session.getProfile()?.id).toBe(profileBefore); - // Room mirror also unchanged. - expect(roomRegistry.getRoom(code)?.profile?.id).toBe(profileBefore); + // A subsequent move must NOT trigger any deferred profile apply + // (nothing was queued). + sendClient(white, "game.move", { from: "e2", to: "e4" }); + expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); + expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); }); - it("non-host (black) sender → BAD_TOKEN, host unaffected, no broadcast", () => { + it("non-host (black) sender → BAD_TOKEN on receipt, host unaffected, nothing queued", () => { const { white, black, code } = setupRoom(); const versionBefore = sessionRegistry.get(code)!.getProfileVersion(); @@ -322,37 +503,38 @@ describe("modifier-profile.update WS handler (T20)", () => { updatePayload(code, swapProfile, versionBefore), ); - // Black gets a BAD_TOKEN error. const err = nextMsgOfType(black, "error"); expect(err.payload["code"]).toBe("BAD_TOKEN"); expect(String(err.payload["message"])).toMatch(/host/i); - // No updated broadcast to either side. expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); + expect(hasMsgOfType(white, "modifier-profile.queued")).toBe(false); + expect(hasMsgOfType(black, "modifier-profile.queued")).toBe(false); - // Session unchanged. expect(sessionRegistry.get(code)?.getProfileVersion()).toBe(versionBefore); expect(sessionRegistry.get(code)?.getProfile()?.id).toBe(initialProfile.id); + expect(roomRegistry.getRoom(code)?.pendingProfile).toBeUndefined(); }); - it("stale version → MODIFIER_PROFILE_INVALID, no mutation", () => { + it("stale version → MODIFIER_PROFILE_INVALID on receipt, nothing queued", () => { const { white, black, code } = setupRoom(); - // First apply a successful swap so server version moves to 2. + // First apply a full queue → move → applied cycle so the server + // version truly moves to 2. sendClient( white, "modifier-profile.update", updatePayload(code, swapProfile, 1), ); - // Drain the successful broadcasts so inbox asserts are clean below. + nextMsgOfType(white, "modifier-profile.queued"); + sendClient(white, "game.move", { from: "e2", to: "e4" }); nextMsgOfType(white, "modifier-profile.updated"); nextMsgOfType(black, "modifier-profile.updated"); expect(sessionRegistry.get(code)!.getProfileVersion()).toBe(2); - // Now a second update from white carrying version=1 — stale. The - // server must refuse rather than silently apply; otherwise a - // client editing locally while another swap lands would overwrite. + // Second update from white carrying version=1 — stale. Rejected + // on receipt before ever reaching the pending slot. sendClient( white, "modifier-profile.update", @@ -363,12 +545,31 @@ describe("modifier-profile.update WS handler (T20)", () => { expect(err.payload["code"]).toBe("MODIFIER_PROFILE_INVALID"); expect(String(err.payload["message"])).toMatch(/version/i); - // No broadcast from the stale request. + // No queued ACK. + expect(hasMsgOfType(white, "modifier-profile.queued")).toBe(false); + // No broadcast. expect(hasMsgOfType(white, "modifier-profile.updated")).toBe(false); expect(hasMsgOfType(black, "modifier-profile.updated")).toBe(false); - // Session stayed on version=2 with swapProfile. + // Pending slot untouched. + expect(roomRegistry.getRoom(code)?.pendingProfile).toBeUndefined(); + // Session state: still on the swapProfile at version 2. expect(sessionRegistry.get(code)?.getProfileVersion()).toBe(2); expect(sessionRegistry.get(code)?.getProfile()?.id).toBe(swapProfile.id); }); + + // Documented omission: the "valid at receipt, invalid at apply" + // scenario. `validateProfile` is currently stateless w.r.t. session + // state — it only inspects `(profile, layout)` — and the layout is + // immutable for a room's lifetime. So a profile that validates at + // receipt today will always validate at the turn boundary too. + // + // `applyPendingProfile` still re-runs validation as defence in depth + // so that if the validator grows stateful (e.g. rejects a profile + // that would leave a side with zero legal moves from the CURRENT + // position), the pending queue handles the rejection correctly + // without silent data loss. Writing a test for that today would + // require injecting a bespoke layout mutation that doesn't exist in + // the wire surface, so we intentionally defer until a real stateful + // validator lands. });