From 3d1ed4a05357626e96d07a94da865e4d6ae8081a Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 11:35:33 +0300 Subject: [PATCH 01/30] fix(web): enforce player ownership on board interaction BoardInteraction fell back to the side to move when no player colour was given, and the game route mounted its board without one and enabled input whenever the game was not over. Spectators could select, drag and premove; the board was interactive before the joined role arrived; an off-turn player could pick up the opponent's pieces and queue premoves with them; and a finished-game join briefly enabled input. - Ownership is explicit: a colour, null (nobody) or 'side-to-move' (boards without players only). setPlayerColor drops the selection, a pending promotion and queued premoves on a real change; drop() re-checks the origin's owner. Legality stays oracle-driven for every variant. - BoardView.setPlayerColor closes an open promotion chooser and abandons a drag; focus, keyboard navigation and flip are untouched. mountBoard exposes it with the same churn guard as setInputEnabled. - The game route mounts fail-closed and derives owner and input from one GameSync snapshot from both onColor and onActionState, so callback order cannot open a window; a finished game latches. Tests: core, view and route suites plus a backend Playwright spec with a real two-player game and a spectator (click, keyboard and drag). The finished-board step of illegal-move-feedback.spec.ts relied on the old fallback and now tries White's own premove. --- packages/web/e2e/board-ownership.spec.ts | 160 ++++++++++ .../web/e2e/illegal-move-feedback.spec.ts | 12 +- packages/web/playwright.config.ts | 1 + packages/web/src/app/board.ts | 20 +- packages/web/src/app/game-mount.ts | 19 +- packages/web/src/core/interaction.ts | 48 ++- packages/web/src/ui/board-view.ts | 12 + packages/web/test/board-a11y.test.ts | 149 +++++++++- .../web/test/board-ownership-route.test.ts | 277 ++++++++++++++++++ packages/web/test/board-ownership.test.ts | 183 ++++++++++++ scripts/test/check-test-topology.test.mjs | 6 +- 11 files changed, 866 insertions(+), 21 deletions(-) create mode 100644 packages/web/e2e/board-ownership.spec.ts create mode 100644 packages/web/test/board-ownership-route.test.ts create mode 100644 packages/web/test/board-ownership.test.ts diff --git a/packages/web/e2e/board-ownership.spec.ts b/packages/web/e2e/board-ownership.spec.ts new file mode 100644 index 00000000..0a4a6a98 --- /dev/null +++ b/packages/web/e2e/board-ownership.spec.ts @@ -0,0 +1,160 @@ +import { expect, test, type APIRequestContext, type BrowserContext, type Locator, type Page } from '@playwright/test'; +import { randomUUID } from 'node:crypto'; + +test.skip(!process.env['GAMBIT_E2E_BACKEND'], 'requires running backend'); + +interface Registered { + readonly handle: string; + readonly user: { readonly id: string }; + readonly tokens: { readonly refreshToken: string }; +} + +async function register(request: APIRequestContext, prefix: string): Promise { + const handle = `${prefix}-${randomUUID().replaceAll('-', '').slice(0, 10)}`; + const registration = await request.post('/v1/auth/register', { + data: { handle, password: 'test-password-123', email: `${handle}@example.test` }, + }); + expect(registration.ok()).toBeTruthy(); + return { handle, ...(await registration.json()) }; +} + +async function signIn(context: BrowserContext, auth: Registered): Promise { + await context.addCookies([{ + name: 'gambit_refresh', + value: auth.tokens.refreshToken, + domain: 'localhost', + path: '/v1/auth', + httpOnly: true, + secure: false, + sameSite: 'Strict', + }]); + await context.addInitScript(({ userHandle, userId }) => { + localStorage.setItem('gambit-session', JSON.stringify({ handle: userHandle, userId })); + }, { userHandle: auth.handle, userId: auth.user.id }); +} + +/** Every `move` frame the page sends over any WebSocket: the only way a move reaches the server. */ +function recordMoveFrames(page: Page): string[] { + const moves: string[] = []; + page.on('websocket', (socket) => { + socket.on('framesent', ({ payload }) => { + const frame = JSON.parse(String(payload)) as { t?: string; uci?: string }; + if (frame.t === 'move' && frame.uci) moves.push(frame.uci); + }); + }); + return moves; +} + +/** Keyboard select `from`, then activate `to` (both by focusing the cell directly). */ +async function keyboardMove(page: Page, board: Locator, from: string, to: string): Promise { + await board.locator(`[data-square="${from}"]`).focus(); + await page.keyboard.press('Enter'); + await board.locator(`[data-square="${to}"]`).focus(); + await page.keyboard.press('Enter'); +} + +/** Try every gesture kind on `from`→`to`: click, keyboard (Enter and Space), and drag. */ +async function tryAllGestures(page: Page, board: Locator, from: string, to: string): Promise { + await board.locator(`[data-square="${from}"]`).click(); + await board.locator(`[data-square="${to}"]`).click(); + await keyboardMove(page, board, from, to); + await board.locator(`[data-square="${from}"]`).focus(); + await page.keyboard.press('Space'); + await board.locator(`[data-square="${to}"]`).focus(); + await page.keyboard.press('Space'); + await board.locator(`[data-square="${from}"]`).dragTo(board.locator(`[data-square="${to}"]`)); +} + +async function expectUntouched(board: Locator, squares: Readonly>): Promise { + await expect(board.locator('[aria-selected="true"]')).toHaveCount(0); + await expect(board.locator('[aria-description*="premove"]')).toHaveCount(0); + for (const [sq, label] of Object.entries(squares)) { + await expect(board.locator(`[data-square="${sq}"]`)).toHaveAttribute('aria-label', label); + } +} + +test('players move and premove only their own colour; a spectator moves nothing by any gesture', async ({ browser, request }) => { + const white = await register(request, 'e2e-own-w'); + const black = await register(request, 'e2e-own-b'); + const gameResponse = await request.post('/e2e/games', { data: { whiteId: white.user.id, blackId: black.user.id } }); + expect(gameResponse.ok()).toBeTruthy(); + const game = await gameResponse.json(); + + const whiteContext = await browser.newContext(); + const blackContext = await browser.newContext(); + const spectatorContext = await browser.newContext(); + try { + await signIn(whiteContext, white); + await signIn(blackContext, black); + + const whitePage = await whiteContext.newPage(); + const whiteMoves = recordMoveFrames(whitePage); + await whitePage.goto(`/game/${game.gameId}`); + const whiteBoard = whitePage.locator('.cb-board'); + const whiteStatus = whitePage.locator('#status'); + await expect(whiteStatus).toHaveText(/your move/i, { timeout: 15_000 }); + + const blackPage = await blackContext.newPage(); + const blackMoves = recordMoveFrames(blackPage); + await blackPage.goto(`/game/${game.gameId}`); + const blackBoard = blackPage.locator('.cb-board'); + await expect(blackPage.locator('#meta-role')).toHaveText(/black/i, { timeout: 15_000 }); + + // Black, off-turn: White is the side to move, but White's pieces are not Black's. + await tryAllGestures(blackPage, blackBoard, 'e2', 'e4'); + await expectUntouched(blackBoard, { e2: 'e2, white pawn', e4: 'e4, empty' }); + await expect(blackPage.locator('#move-feedback')).toBeEmpty(); + expect(blackMoves).toEqual([]); + + // A spectator: every gesture on either colour does nothing, and is not called illegal. + const spectator = await spectatorContext.newPage(); + const spectatorMoves = recordMoveFrames(spectator); + await spectator.goto(`/game/${game.gameId}`); + await expect(spectator.locator('#meta-role')).toHaveText(/spectat/i, { timeout: 15_000 }); + const spectatorBoard = spectator.locator('.cb-board'); + for (const [from, to] of [['e2', 'e4'], ['e7', 'e5']] as const) { + await tryAllGestures(spectator, spectatorBoard, from, to); + } + await expectUntouched(spectatorBoard, { e2: 'e2, white pawn', e4: 'e4, empty', e7: 'e7, black pawn', e5: 'e5, empty' }); + await expect(spectator.locator('#move-feedback')).toBeEmpty(); + // Read-only, not dead: keyboard navigation still moves focus around the grid. + await spectatorBoard.locator('[data-square="e2"]').focus(); + await spectator.keyboard.press('ArrowUp'); + await expect(spectatorBoard.locator('[data-square="e3"]')).toBeFocused(); + expect(spectatorMoves).toEqual([]); + + // White, on turn: Black's pieces are not selectable; the legal keyboard move commits. + await whiteBoard.locator('[data-square="e7"]').click(); + await expect(whiteBoard.locator('[aria-selected="true"]')).toHaveCount(0); + await keyboardMove(whitePage, whiteBoard, 'e2', 'e4'); + await expect(whiteStatus).toHaveText(/black to move/i, { timeout: 15_000 }); + expect(whiteMoves).toEqual(['e2e4']); + + // White, off-turn: Black is now the side to move, and still not White's to touch. + await tryAllGestures(whitePage, whiteBoard, 'e7', 'e5'); + await expectUntouched(whiteBoard, { e7: 'e7, black pawn', e5: 'e5, empty' }); + // An own-colour premove still queues, and is not sent. + await keyboardMove(whitePage, whiteBoard, 'd2', 'd4'); + await expect(whiteBoard.locator('[data-square="d4"]')).toHaveAttribute('aria-description', /premove/); + expect(whiteMoves).toEqual(['e2e4']); + + // Black, on turn: White's pieces still are not Black's; Black's legal move commits. + await expect(blackBoard.locator('[data-square="e4"]')).toHaveAttribute('aria-label', 'e4, white pawn', { timeout: 15_000 }); + await blackBoard.locator('[data-square="d2"]').click(); + await expect(blackBoard.locator('[aria-selected="true"]')).toHaveCount(0); + await keyboardMove(blackPage, blackBoard, 'e7', 'e5'); + await expect(blackBoard.locator('[data-square="e5"]')).toHaveAttribute('aria-label', 'e5, black pawn', { timeout: 15_000 }); + expect(blackMoves).toEqual(['e7e5']); + + // The spectator saw both moves arrive and is still read-only after the updates. + await expect(spectatorBoard.locator('[data-square="e5"]')).toHaveAttribute('aria-label', 'e5, black pawn', { timeout: 15_000 }); + await tryAllGestures(spectator, spectatorBoard, 'g1', 'f3'); + await tryAllGestures(spectator, spectatorBoard, 'g8', 'f6'); + await expectUntouched(spectatorBoard, { g1: 'g1, white knight', f3: 'f3, empty', g8: 'g8, black knight', f6: 'f6, empty' }); + expect(spectatorMoves).toEqual([]); + } finally { + await spectatorContext.close(); + await blackContext.close(); + await whiteContext.close(); + } +}); diff --git a/packages/web/e2e/illegal-move-feedback.spec.ts b/packages/web/e2e/illegal-move-feedback.spec.ts index 19b4b560..d4aeccd0 100644 --- a/packages/web/e2e/illegal-move-feedback.spec.ts +++ b/packages/web/e2e/illegal-move-feedback.spec.ts @@ -126,14 +126,14 @@ test('a keyboard player hears a rejected move, nothing is sent, and the next leg await page.click('#confirm-resign-yes'); await expect(status).toHaveText(/resignation/i, { timeout: 15_000 }); const finalStatus = await status.textContent(); - // It ended on Black's turn; off-turn the board offers the side to move, so try Black's pawn. - await board.locator('[data-square="e7"]').focus(); + // It ended on Black's turn, so a live board would take White's own off-turn premove; this one must not. + await board.locator('[data-square="d2"]').focus(); await page.keyboard.press('Enter'); - await expect(board.locator('[data-square="e7"]')).toHaveAttribute('aria-selected', 'false'); - await page.keyboard.press('ArrowDown'); - await page.keyboard.press('ArrowDown'); + await expect(board.locator('[data-square="d2"]')).toHaveAttribute('aria-selected', 'false'); + await page.keyboard.press('ArrowUp'); + await page.keyboard.press('ArrowUp'); await page.keyboard.press('Enter'); - await expect(board.locator('[data-square="e5"]')).not.toHaveAttribute('aria-description', /premove/); + await expect(board.locator('[data-square="d4"]')).not.toHaveAttribute('aria-description', /premove/); await expect(status).toHaveText(finalStatus ?? ''); await expect(feedback).toBeEmpty(); expect(sentMoves).toEqual(['e2e4']); diff --git a/packages/web/playwright.config.ts b/packages/web/playwright.config.ts index 1c4b6e21..34082d70 100644 --- a/packages/web/playwright.config.ts +++ b/packages/web/playwright.config.ts @@ -28,6 +28,7 @@ const backendSpecs = [ 'account-security-sessions.spec.ts', 'achievements.spec.ts', 'analysis.spec.ts', + 'board-ownership.spec.ts', 'forum.spec.ts', 'game-actions.spec.ts', 'game-keyboard.spec.ts', diff --git a/packages/web/src/app/board.ts b/packages/web/src/app/board.ts index 07bf18cb..1188ea7e 100644 --- a/packages/web/src/app/board.ts +++ b/packages/web/src/app/board.ts @@ -20,6 +20,7 @@ import type { LegalMoveOracle } from '../ports/move-oracle.js'; import { applyMove } from '../core/mover.js'; import { STARTING_FEN } from '../core/position.js'; import type { Premove } from '../core/premove.js'; +import type { Color } from '../core/board.js'; import { createI18nManager, type I18nManager } from '../i18n/manager.js'; import { createLtrElement } from '../i18n/bidi.js'; @@ -63,6 +64,11 @@ export interface MountBoardOptions { readonly onMove?: (uci: string) => void; /** Localization manager for board status copy; optional for test resilience. */ readonly i18n?: I18nManager; + /** + * The colour this client may move, or `null` for none until {@link MountedBoard.setPlayerColor} + * says otherwise. Omit only for a board without players, which moves whichever side is to move. + */ + readonly playerColor?: Color | null; } /** Handle to the mounted board. */ @@ -76,6 +82,8 @@ export interface MountedBoard { setTurn: (myTurn: boolean) => void; /** Accept or ignore move input (off once the game is over). */ setInputEnabled: (enabled: boolean) => void; + /** Change whose pieces may be moved (`null`: nobody, e.g. a spectator). */ + setPlayerColor: (color: Color | null) => void; /** Set the board orientation ('white' or 'black' perspective). */ setOrientation: (orientation: 'white' | 'black') => void; /** @@ -133,7 +141,11 @@ export function mountBoard( let fen = STARTING_FEN; const oracle = options?.oracle ?? new NullMoveOracle(); const onMove = options?.onMove; - const interaction = new BoardInteraction({ oracle, myTurn: true }); + const interaction = new BoardInteraction({ + oracle, + myTurn: true, + ...(options?.playerColor !== undefined ? { playerColor: options.playerColor } : {}), + }); const i18n = options?.i18n ?? createI18nManager(); type StatusKey = 'board.status.played' | 'board.status.premoveSet'; @@ -256,6 +268,12 @@ export function mountBoard( clearFeedback(); view.setInputEnabled(enabled); }, + // Same churn guard as input: the game route re-asserts the owner on every action-state update. + setPlayerColor: (color: Color | null) => { + if (color === interaction.playerColor) return; + clearFeedback(); + view.setPlayerColor(color); + }, setOrientation: (orientation: 'white' | 'black') => { if (view.orientationColor !== orientation) view.flip(); }, diff --git a/packages/web/src/app/game-mount.ts b/packages/web/src/app/game-mount.ts index 99fccb34..501f2779 100644 --- a/packages/web/src/app/game-mount.ts +++ b/packages/web/src/app/game-mount.ts @@ -1189,9 +1189,23 @@ export function mountGame(deps: GameMountDependencies): MountedGame { controller.submitMove(uci); }, i18n, + // Nobody owns the pieces until the server says who we are. + playerColor: null, }, ); + // Move input belongs to a joined player, for their own colour, while the game is live. Both inputs + // are read from one authoritative snapshot, so whichever of onColor / onActionState reports first + // cannot open a window between them; and an ended game never goes live again. + let boardFinished = false; + const syncBoardOwnership = (): void => { + const { myColor, status } = gameSync.getState(); + if (status?.over === true) boardFinished = true; + const owner = boardFinished || myColor === null ? null : myColor === 'w' ? 'white' : 'black'; + board.setPlayerColor(owner); + board.setInputEnabled(owner !== null); + }; + const renderMetadata = (state: GameMetadataState): void => { let liveAnnouncement = ''; @@ -1458,6 +1472,7 @@ export function mountGame(deps: GameMountDependencies): MountedGame { }, onColor: (color) => { if (color === 'b') board.setOrientation('black'); + syncBoardOwnership(); }, onMetadata: (state) => { lastMetadataState = state; @@ -1475,8 +1490,8 @@ export function mountGame(deps: GameMountDependencies): MountedGame { }, onActionState: (state) => { lastActionState = state; - // A finished board takes no moves, premoves or rejections; its only gesture was submitting. - board.setInputEnabled(!state.isOver); + // A finished or spectated board takes no moves, premoves or rejections. + syncBoardOwnership(); renderActionState(state); }, }, diff --git a/packages/web/src/core/interaction.ts b/packages/web/src/core/interaction.ts index 8ee1337d..954940af 100644 --- a/packages/web/src/core/interaction.ts +++ b/packages/web/src/core/interaction.ts @@ -43,10 +43,20 @@ export type GestureResult = */ | { readonly kind: 'illegal'; readonly from: Square; readonly to: Square }; +/** + * Whose pieces gestures may pick up: one colour (a player), `null` (nobody: a spectator, or a player + * whose colour the server has not confirmed yet), or `'side-to-move'` (a standalone board with no + * players, such as analysis or a study). + */ +export type BoardOwner = Color | null | 'side-to-move'; + export interface BoardInteractionOptions { readonly oracle: LegalMoveOracle; - /** The side this client may move. Omit to allow whichever side is to move. */ - readonly playerColor?: Color; + /** + * The side this client may move, or `null` for none. Omit only on a board without players: it then + * moves whichever side is to move. + */ + readonly playerColor?: Color | null; /** Whether it is currently this client's turn to move. Default true. */ readonly myTurn?: boolean; /** Max chained premoves (see PremoveQueue). Default 1. */ @@ -61,7 +71,7 @@ interface Pending { export class BoardInteraction { private readonly oracle: LegalMoveOracle; - private readonly playerColor: Color | undefined; + private owner: BoardOwner; private readonly premoves: PremoveQueue; private pieces = new Map(); @@ -75,7 +85,7 @@ export class BoardInteraction { constructor(options: BoardInteractionOptions) { this.oracle = options.oracle; - this.playerColor = options.playerColor; + this.owner = options.playerColor === undefined ? 'side-to-move' : options.playerColor; this.myTurn = options.myTurn ?? true; this.premoves = new PremoveQueue( options.premoveDepth !== undefined ? { maxDepth: options.premoveDepth } : {}, @@ -119,6 +129,22 @@ export class BoardInteraction { this.premoves.clear(); } + /** + * Change whose pieces this client may move. A gesture, promotion or premove begun for the previous + * owner is dropped, so nothing made under one colour completes under another — or under none. + */ + setPlayerColor(color: Color | null): void { + if (color === this.owner) return; + this.owner = color; + this.clearSelection(); + this.pending = null; + this.premoves.clear(); + } + + get playerColor(): BoardOwner { + return this.owner; + } + get acceptsInput(): boolean { return this.inputEnabled; } @@ -133,14 +159,15 @@ export class BoardInteraction { // ---- queries -------------------------------------------------------------- - /** The colour this client is allowed to move right now. */ - private movableColor(): Color { - return this.playerColor ?? this.sideToMove; + /** The colour this client is allowed to move right now, or `null` for none. */ + private movableColor(): Color | null { + return this.owner === 'side-to-move' ? this.sideToMove : this.owner; } private isOwnPiece(sq: Square): boolean { const p = this.pieces.get(sq); - return p !== undefined && (p.color === (this.movableColor() === 'white' ? 'w' : 'b')); + const color = this.movableColor(); + return p !== undefined && color !== null && p.color === (color === 'white' ? 'w' : 'b'); } private isPromotion(from: Square, to: Square): boolean { @@ -179,6 +206,11 @@ export class BoardInteraction { this.clearSelection(); return { kind: 'deselect' }; } + // A drop normally follows a successful dragStart, but the owner can change in between. + if (!this.isOwnPiece(from)) { + this.clearSelection(); + return { kind: 'none' }; + } this.setSelection(from); return this.attempt(to); } diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index d8e6f506..2e3a6064 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -188,6 +188,18 @@ export class BoardView { this.render(); } + /** + * Change whose pieces may be moved (`null`: nobody). A drag or promotion chooser opened for the + * previous owner is abandoned with it; focus and keyboard navigation are untouched. + */ + setPlayerColor(color: Color | null): void { + if (color === this.interaction.playerColor) return; + if (this.overlay) this.cancelPromotion(); + this.cancelDrag(); + this.interaction.setPlayerColor(color); + this.render(); + } + /** Toggle the board between white-at-bottom and black-at-bottom orientations. */ flip(): void { this.orientation = this.orientation === 'white' ? 'black' : 'white'; diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 6c482a3d..049821eb 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -3,7 +3,7 @@ import assert from 'node:assert/strict'; import { BoardView } from '../src/ui/board-view.js'; import { BoardInteraction } from '../src/core/interaction.js'; import { StaticMoveOracle } from '../src/ports/move-oracle.js'; -import type { Square } from '../src/core/board.js'; +import type { Color, Square } from '../src/core/board.js'; import { mountBoard } from '../src/app/board.js'; import { I18n } from '../src/i18n/manager.js'; import { enMessages } from '../src/i18n/catalog/en.js'; @@ -665,7 +665,9 @@ function centreOf(sq: string): { clientX: number; clientY: number } { * The elements go through a variable, so this compiles against a board that does not know the * feedback element yet: the RED run fails on behaviour, not on a type error. */ -function mountWithFeedback(options: { i18n?: I18n; root?: FakeBoardRoot; feedback?: FakeDOMNode } = {}) { +function mountWithFeedback( + options: { i18n?: I18n; root?: FakeBoardRoot; feedback?: FakeDOMNode; playerColor?: Color | null } = {}, +) { const root = options.root ?? new FakeBoardRoot(); const feedback = options.feedback ?? Object.assign(new FakeDOMNode('p'), { ownerDocument: root.ownerDocument }); const moves: string[] = []; @@ -674,6 +676,7 @@ function mountWithFeedback(options: { i18n?: I18n; root?: FakeBoardRoot; feedbac oracle: new StaticMoveOracle({ [START_FEN]: { e2: ['e3', 'e4'], g1: ['f3', 'h3'] } }), onMove: (uci) => moves.push(uci), ...(options.i18n ? { i18n: options.i18n } : {}), + ...(options.playerColor !== undefined ? { playerColor: options.playerColor } : {}), }); board.setPosition(START_FEN); const press = (sq: string, key = 'Enter'): void => { @@ -952,3 +955,145 @@ test('remounting onto the same elements announces each rejection exactly once', assert.deepEqual(first.moves, []); assert.deepEqual(second.moves, []); }); + +// ---- board ownership ---- + +/** The square holding the single roving tab stop. */ +function rovingSquare(root: FakeBoardRoot): string | null { + const roving = root.querySelectorAll('[tabindex="0"]'); + assert.equal(roving.length, 1, 'exactly one roving tab stop'); + return roving[0]?.getAttribute('data-square') ?? null; +} + +test('a spectator board submits nothing from click, keyboard or drag, and says nothing', () => { + withDragGlobals((win) => { + const { root, feedback, moves, press, click } = mountWithFeedback({ playerColor: null }); + click('e2'); + click('e4'); + press('e2'); + press('e4'); + press('e2', ' '); + press('e3', ' '); + drag(root, win, 'e2', 'e4'); + drag(root, win, 'e7', 'e5'); + assert.deepEqual(moves, []); + assert.equal(root.querySelector('[aria-selected="true"]'), null, 'nothing is ever selected'); + assert.equal(root.querySelector('.cb-dragging'), null, 'no piece lifts'); + assert.equal(feedback.children.length, 0, 'a spectator is not told their gesture was illegal'); + }); +}); + +test('a read-only board keeps keyboard navigation, focus and flipping', () => { + const { root, board, press } = mountWithFeedback({ playerColor: null }); + const a8 = root.querySelector('[data-square="a8"]'); + assert.ok(a8); + press('a8', 'ArrowRight'); + assert.equal(rovingSquare(root), 'b8', 'arrow keys move the roving focus'); + assert.equal(root.querySelector('[data-square="b8"]')?.focused, true); + press('b8', 'End'); + assert.equal(rovingSquare(root), 'h8'); + assert.equal(root.querySelector('[data-square="e2"]')?.getAttribute('aria-label'), 'e2, white pawn'); + + board.view.flip(); + assert.equal(board.view.orientationColor, 'black'); + assert.equal(root.querySelector('[data-square="h1"]')?.getAttribute('aria-rowindex'), '1', 'flipped grid semantics'); +}); + +test('a player board accepts only its own colour from click, keyboard and drag', () => { + withDragGlobals((win) => { + const { root, board, moves, press, click } = mountWithFeedback({ playerColor: 'black' }); + board.setTurn(false); // White to move: Black is off-turn + click('e2'); + press('e2'); + drag(root, win, 'e2', 'e4'); + assert.equal(root.querySelector('[aria-selected="true"]'), null, "White's pieces are not Black's to select"); + assert.equal(root.querySelector('[aria-description="premove"]'), null, 'no opponent-coloured premove'); + + drag(root, win, 'd7', 'd5'); + assert.equal(root.querySelector('[data-square="d5"]')?.getAttribute('aria-description'), 'premove', 'own premove queues by drag'); + press('e7'); + press('e5'); + assert.equal(root.querySelector('[data-square="e5"]')?.getAttribute('aria-description'), 'premove', 'own premove queues by keyboard'); + assert.deepEqual(moves, []); + }); +}); + +test('becoming a spectator mid-drag drops the floating piece and the drag cannot complete', () => { + withDragGlobals((win) => { + const { root, board, moves } = mountWithFeedback({ playerColor: 'white' }); + const body = (globalThis.document as unknown as { body: FakeDOMNode }).body; + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1 }); + win.dispatchEvent('pointermove', { ...centreOf('e3'), pointerId: 1 }); + assert.equal(body.children.length, 1); + + board.setPlayerColor(null); + assert.equal(body.children.length, 0); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); + assert.deepEqual(moves, []); + assert.equal(win.listenerCount('pointermove'), 0); + }); +}); + +test('a change of owner closes an open promotion chooser and clears a queued premove', () => { + const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; + const root = new FakeBoardRoot(); + const moves: string[] = []; + const board = mountBoard({ boardEl: root as unknown as HTMLElement }, { + oracle: new StaticMoveOracle({ [fen]: { e7: ['e8'] } }), + onMove: (uci) => moves.push(uci), + playerColor: 'white', + }); + board.setPosition(fen); + const prevDoc = Object.getOwnPropertyDescriptor(globalThis, 'document'); + Object.defineProperty(globalThis, 'document', { configurable: true, value: root.ownerDocument }); + try { + const press = (sq: string): void => { + root.dispatchEvent('keydown', { key: 'Enter', target: root.querySelector(`[data-square="${sq}"]`), preventDefault: () => undefined }); + }; + press('e7'); + press('e8'); + assert.ok(root.querySelector('[role="dialog"]'), 'promotion chooser open'); + board.setPlayerColor(null); + assert.equal(root.querySelector('[role="dialog"]'), null, 'chooser closed with the ownership'); + + board.setPlayerColor('white'); + board.setTurn(false); + press('e7'); + press('e8'); + const choice = root.querySelector('[role="dialog"]')?.querySelector('button'); + assert.ok(choice, 'promotion chooser open for the premove'); + choice.dispatchEvent('click', { preventDefault: () => undefined, stopPropagation: () => undefined }); + assert.equal(root.querySelector('[data-square="e8"]')?.getAttribute('aria-description'), 'premove'); + board.setPlayerColor('black'); + assert.equal(root.querySelector('[data-square="e8"]')?.getAttribute('aria-description'), null, 'premove cleared'); + assert.deepEqual(moves, []); + } finally { + if (prevDoc) Object.defineProperty(globalThis, 'document', prevDoc); + else Reflect.deleteProperty(globalThis, 'document'); + } +}); + +test('re-asserting the same owner leaves an unheard rejection and the cells alone', () => { + const { root, board, press, announced } = mountWithFeedback({ playerColor: 'white' }); + press('e2'); + press('e5'); + const cell = root.querySelector('[data-square="e2"]'); + board.setPlayerColor('white'); + assert.equal(announced(), ILLEGAL_MOVE_TEXT); + assert.equal(root.querySelector('[data-square="e2"]'), cell, 'no re-render'); +}); + +test('remounting keeps no ownership from the previous mount and submits once', () => { + const first = mountWithFeedback({ playerColor: 'white' }); + first.press('e2'); + const spectator = mountWithFeedback({ root: first.root, feedback: first.feedback, playerColor: null }); + spectator.press('e2'); + spectator.press('e4'); + assert.deepEqual(first.moves, [], 'the old mount and its selection are gone'); + assert.deepEqual(spectator.moves, []); + + const player = mountWithFeedback({ root: first.root, feedback: first.feedback, playerColor: 'white' }); + player.press('e2'); + player.press('e4'); + assert.deepEqual([first.moves, spectator.moves, player.moves], [[], [], ['e2e4']], 'one submission, from the live mount'); +}); diff --git a/packages/web/test/board-ownership-route.test.ts b/packages/web/test/board-ownership-route.test.ts new file mode 100644 index 00000000..4d6d92dc --- /dev/null +++ b/packages/web/test/board-ownership-route.test.ts @@ -0,0 +1,277 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createApp } from '../src/app/composition.js'; +import { mountGame } from '../src/app/game-mount.js'; +import type { StateView } from '../src/net/ws-protocol.js'; +import { createGameDocument, makeFinishedState, makeState } from './support/analysis-fixtures.js'; +import { FakeSocketFactory } from './support/fake-socket.js'; +import { FakeTransport, json } from './support/fake-transport.js'; + +/** + * The game route's board ownership: who may move which pieces, driven only by the authoritative + * `joined` role and game status. Gestures go through the real `BoardView` click handler, and a move + * counts as submitted when the route calls `controller.submitMove` — before `GameSync` gets a chance + * to drop it, so a spectator's attempt cannot hide behind the server-side guard. + */ + +const START = 'rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1'; +const AFTER_E4 = 'rnbqkbnr/pppppppp/8/8/4P3/8/PPPP1PPP/RNBQKBNR b KQkq - 0 1'; +const BOARD_PX = 800; + +type Role = 'white' | 'black' | 'spectator'; + +function live(fen: string, turn: 'w' | 'b', legalMoves: StateView['legalMoves']): StateView { + return { ...makeState(fen, turn === 'w' ? 0 : 1, turn), legalMoves }; +} + +function setup(options: { open?: boolean } = {}) { + const sockets = new FakeSocketFactory(); + const app = createApp({ + config: { apiBaseUrl: 'https://api.test', wsUrl: 'wss://api.test/ws' }, + wsFactory: sockets.factory, + httpTransport: new FakeTransport(() => json(404, {})), + }); + const { doc, elements } = createGameDocument(); + const boardEl = elements.get('board')!; + Object.assign(boardEl, { getBoundingClientRect: () => ({ left: 0, top: 0, width: BOARD_PX, height: BOARD_PX }) }); + // The fake document has no text nodes; without one the board writes its status as plain text. + Object.defineProperty(elements.get('status')!, 'ownerDocument', { value: undefined }); + const mounted = mountGame({ + doc, + boardEl: boardEl as unknown as HTMLElement, + gameId: 'g-test-1', + createGameSync: app.createGameSync, + createGameOracle: app.createGameOracle, + getAccessToken: () => 'token', + client: app.api, + token: 'token', + initialSessionId: 'u1', + restorePromise: Promise.resolve(null), + i18n: app.i18n, + }); + const submitted: string[] = []; + const submit = mounted.controller.submitMove.bind(mounted.controller); + mounted.controller.submitMove = (uci: string) => { + submitted.push(uci); + return submit(uci); + }; + const enabled: boolean[] = []; + const setInputEnabled = mounted.board.setInputEnabled; + mounted.board.setInputEnabled = (on: boolean) => { + enabled.push(on); + setInputEnabled(on); + }; + if (options.open !== false) sockets.last.open(); + + const orientation = (): 'white' | 'black' => mounted.board.view.orientationColor; + const click = (sq: string): void => { + const file = sq.charCodeAt(0) - 97; + const rank = Number(sq[1]) - 1; + const cell = BOARD_PX / 8; + const col = orientation() === 'white' ? file : 7 - file; + const row = orientation() === 'white' ? 7 - rank : rank; + for (const fn of boardEl.listeners['click'] ?? []) { + fn({ clientX: col * cell + cell / 2, clientY: row * cell + cell / 2 } as unknown as Event); + } + }; + const cellAttr = (sq: string, attr: string): string | null => { + const match = new RegExp(`data-square="${sq}"[^>]*?${attr}="([^"]*)"`).exec(boardEl.innerHTML); + return match?.[1] ?? null; + }; + const selected = (): string[] => + [...boardEl.innerHTML.matchAll(/data-square="(\w\d)"[^>]*?aria-selected="true"/g)].map((m) => m[1]!); + const premoves = (): string[] => + [...boardEl.innerHTML.matchAll(/class="[^"]*cb-premove[^"]*"[^>]*?data-square="(\w\d)"/g)].map((m) => m[1]!); + const moveFrames = (): string[] => + sockets.last.sent.map((f) => JSON.parse(f) as { t: string; uci?: string }).filter((f) => f.t === 'move').map((f) => f.uci!); + const join = (role: Role, state: StateView): void => { + sockets.last.emit({ t: 'joined', gameId: 'g-test-1', role, state }); + }; + const sync = (state: StateView): void => { + sockets.last.emit({ t: 'state', gameId: 'g-test-1', state }); + }; + return { + open: () => sockets.last.open(), + elements, mounted, submitted, enabled, click, cellAttr, selected, premoves, moveFrames, join, sync, orientation, + feedback: () => elements.get('move-feedback')?.innerHTML ?? '', + dispose: () => { + mounted.dispose?.(); + mounted.analysis.dispose(); + mounted.connectivity.dispose(); + mounted.controller.dispose(); + app.dispose(); + }, + }; +} + +test('route: before the role is known the board takes no gesture and is never enabled', () => { + const r = setup(); + try { + r.click('e2'); + r.click('e4'); + assert.deepEqual(r.selected(), [], 'no selection before joined'); + assert.deepEqual(r.premoves(), [], 'no premove before joined'); + assert.deepEqual(r.submitted, []); + assert.equal(r.feedback(), '', 'no rejection either: nobody owns the pieces yet'); + assert.ok(!r.enabled.includes(true), `input never enabled before join (calls: ${r.enabled.join(',')})`); + } finally { + r.dispose(); + } +}); + +test('route: straight after mount, before the socket even opens, the board takes no gesture', () => { + const r = setup({ open: false }); + try { + r.click('e2'); + r.click('e4'); + assert.deepEqual(r.selected(), []); + assert.deepEqual(r.premoves(), []); + assert.deepEqual(r.submitted, []); + r.open(); + r.click('e2'); + assert.deepEqual(r.selected(), [], 'still nobody once connected but not joined'); + } finally { + r.dispose(); + } +}); + +test('route: a spectator selects nothing and submits nothing, on either side to move', () => { + const r = setup(); + try { + r.join('spectator', live(START, 'w', { e2: ['e3', 'e4'] })); + for (const [from, to] of [['e2', 'e4'], ['e7', 'e5']] as const) { + r.click(from); + assert.deepEqual(r.selected(), [], `${from} not selectable by a spectator`); + r.click(to); + } + assert.deepEqual(r.submitted, [], 'the route never asked to submit a spectator move'); + assert.deepEqual(r.moveFrames(), []); + assert.deepEqual(r.premoves(), []); + assert.equal(r.feedback(), '', 'spectator blocking is not reported as an illegal move'); + assert.ok(!r.enabled.includes(true), `spectator input never enabled (calls: ${r.enabled.join(',')})`); + } finally { + r.dispose(); + } +}); + +test('route: a spectator stays read-only across sync updates and turn changes', () => { + const r = setup(); + try { + r.join('spectator', live(START, 'w', { e2: ['e3', 'e4'] })); + r.sync(live(AFTER_E4, 'b', { e7: ['e6', 'e5'] })); + r.click('e7'); + r.click('e5'); + r.sync(live(AFTER_E4, 'b', { e7: ['e6', 'e5'] })); + r.click('d2'); + r.click('d4'); + assert.deepEqual(r.selected(), []); + assert.deepEqual(r.submitted, []); + assert.deepEqual(r.premoves(), []); + assert.ok(!r.enabled.includes(true)); + } finally { + r.dispose(); + } +}); + +test('route: the white player moves a white piece on their turn', () => { + const r = setup(); + try { + r.join('white', live(START, 'w', { e2: ['e3', 'e4'] })); + assert.equal(r.enabled.at(-1), true, 'a live player gets input'); + r.click('e2'); + assert.deepEqual(r.selected(), ['e2']); + r.click('e4'); + assert.deepEqual(r.submitted, ['e2e4']); + assert.deepEqual(r.moveFrames(), ['e2e4']); + } finally { + r.dispose(); + } +}); + +test('route: off-turn, the white player cannot select or premove Black (the side to move)', () => { + const r = setup(); + try { + r.join('white', live(AFTER_E4, 'b', { e7: ['e6', 'e5'] })); + r.click('e7'); + assert.deepEqual(r.selected(), [], 'the opponent-coloured side to move is not ours'); + r.click('e5'); + assert.deepEqual(r.premoves(), []); + assert.deepEqual(r.submitted, []); + + r.click('d2'); + assert.deepEqual(r.selected(), ['d2'], 'our own piece is still selectable off-turn'); + r.click('d4'); + assert.deepEqual(r.premoves().sort(), ['d2', 'd4'], 'an own-colour premove queues'); + assert.deepEqual(r.submitted, [], 'a premove is not a submission'); + } finally { + r.dispose(); + } +}); + +test('route: off-turn, the black player cannot select or premove White (the side to move)', () => { + const r = setup(); + try { + r.join('black', live(START, 'w', { e2: ['e3', 'e4'] })); + assert.equal(r.orientation(), 'black'); + r.click('e2'); + assert.deepEqual(r.selected(), []); + r.click('e4'); + assert.deepEqual(r.premoves(), []); + assert.deepEqual(r.submitted, []); + + r.click('e7'); + assert.deepEqual(r.selected(), ['e7']); + r.click('e5'); + assert.deepEqual(r.premoves().sort(), ['e5', 'e7']); + assert.deepEqual(r.submitted, []); + } finally { + r.dispose(); + } +}); + +test('route: joining a finished game never enables input, in either callback order', () => { + for (const role of ['white', 'black', 'spectator'] as const) { + const r = setup(); + try { + r.join(role, makeFinishedState(START)); + assert.ok(!r.enabled.includes(true), `${role}: no transient enable (calls: ${r.enabled.join(',')})`); + r.click(role === 'black' ? 'e7' : 'e2'); + assert.deepEqual(r.selected(), []); + assert.deepEqual(r.submitted, []); + } finally { + r.dispose(); + } + } +}); + +test('route: once finished, a later live-looking sync never re-enables the board', () => { + const r = setup(); + try { + r.join('white', live(START, 'w', { e2: ['e3', 'e4'] })); + r.sync(makeFinishedState(START)); + assert.equal(r.enabled.at(-1), false); + r.sync(live(START, 'w', { e2: ['e3', 'e4'] })); + r.click('e2'); + r.click('e4'); + assert.deepEqual(r.selected(), []); + assert.deepEqual(r.submitted, []); + assert.equal(r.enabled.at(-1), false, 'finished wins'); + } finally { + r.dispose(); + } +}); + +test('route: a Chess960 game keeps the same ownership boundary', () => { + const fen = 'rbqknnbr/1ppppp2/p5pp/8/2P5/2Q5/PPBPPPPP/R2KNNBR b KQkq - 0 4'; + const r = setup(); + try { + r.join('white', { ...live(fen, 'b', { b7: ['b6', 'b5'] }), variant: 'chess960', chess960StartId: 700 }); + r.click('b7'); + assert.deepEqual(r.selected(), [], 'Black is to move but is not ours'); + r.click('d1'); + assert.deepEqual(r.selected(), ['d1']); + assert.deepEqual(r.submitted, []); + } finally { + r.dispose(); + } +}); diff --git a/packages/web/test/board-ownership.test.ts b/packages/web/test/board-ownership.test.ts new file mode 100644 index 00000000..2c10d781 --- /dev/null +++ b/packages/web/test/board-ownership.test.ts @@ -0,0 +1,183 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { BoardInteraction } from '../src/core/interaction.js'; +import type { Color } from '../src/core/board.js'; +import { StaticMoveOracle } from '../src/ports/move-oracle.js'; + +/** + * Board ownership in the interaction core: a player moves only their own colour, a spectator or a + * not-yet-resolved role moves nothing, and a change of owner leaves nothing of the old one behind. + * Legality still comes only from the oracle; ownership is just the piece colour on the square. + */ + +const START = 'rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1'; +const START_BLACK = 'rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR b KQkq - 0 1'; +// White pawn on e7 with Black to move: White's off-turn promotion premove. +const PROMO_B = '4k3/4P3/8/8/8/8/8/4K3 b - - 0 1'; +const PROMO_W = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; +// Chess960 start position 700 (cleared queenside), Black to move. +const SP700_BLACK = 'rbqknnbr/1ppppp2/p5pp/8/2P5/2Q5/PPBPPPPP/R2KNNBR b KQkq - 0 4'; + +function oracle(): StaticMoveOracle { + return new StaticMoveOracle({ + [START]: { e2: ['e3', 'e4'], g1: ['f3', 'h3'] }, + [START_BLACK]: { e7: ['e6', 'e5'], g8: ['f6', 'h6'] }, + [PROMO_B]: { e8: ['d8', 'f8'] }, + [PROMO_W]: { e7: ['e8'] }, + [SP700_BLACK]: { b7: ['b6', 'b5'] }, + }); +} + +function make(fen: string, playerColor: Color | null, myTurn: boolean): BoardInteraction { + const bi = new BoardInteraction({ oracle: oracle(), playerColor, myTurn }); + bi.setPosition(fen); + return bi; +} + +/** Every gesture a spectator could try, for one origin/destination pair. */ +function tryEverything(bi: BoardInteraction, from: string, to: string): string[] { + const kinds = [bi.tap(from as never).kind, bi.tap(to as never).kind]; + kinds.push(bi.dragStart(from as never).kind, bi.drop(from as never, to as never).kind); + return kinds; +} + +test('ownership: a spectator selects, drags, drops and premoves nothing on either turn', () => { + for (const [fen, myTurn] of [[START, true], [START, false], [START_BLACK, false]] as const) { + const bi = make(fen, null, myTurn); + for (const [from, to] of [['e2', 'e4'], ['e7', 'e5'], ['g1', 'f3']] as const) { + assert.deepEqual(tryEverything(bi, from, to), ['none', 'none', 'none', 'none'], `${fen} ${from}-${to}`); + } + assert.equal(bi.highlights().selected, null); + assert.equal(bi.hasPremove, false); + assert.equal(bi.awaitingPromotion, false); + assert.equal(bi.applyPremove().kind, 'none'); + } +}); + +test('ownership: an unresolved role fails closed until a colour arrives', () => { + const bi = make(START, null, true); + assert.equal(bi.tap('e2').kind, 'none'); + assert.equal(bi.dragStart('e2').kind, 'none'); + bi.setPlayerColor('white'); + assert.equal(bi.tap('e2').kind, 'select'); + assert.deepEqual(bi.tap('e4'), { kind: 'move', move: { from: 'e2', to: 'e4' } }); +}); + +test('ownership: the White player selects only White, on and off turn', () => { + const on = make(START, 'white', true); + assert.equal(on.tap('e7').kind, 'none'); + assert.equal(on.dragStart('e7').kind, 'none'); + assert.equal(on.tap('e2').kind, 'select'); + + const off = make(START_BLACK, 'white', false); + assert.equal(off.tap('e7').kind, 'none', 'Black is to move, but Black is not ours'); + assert.equal(off.dragStart('g8').kind, 'none'); + assert.equal(off.drop('e7', 'e5').kind, 'none', 'a drop from an opponent piece is not a premove'); + assert.equal(off.hasPremove, false); + assert.equal(off.tap('d2').kind, 'select'); +}); + +test('ownership: the Black player selects only Black, on and off turn', () => { + const on = make(START_BLACK, 'black', true); + assert.equal(on.tap('e2').kind, 'none'); + assert.equal(on.tap('e7').kind, 'select'); + assert.deepEqual(on.tap('e5'), { kind: 'move', move: { from: 'e7', to: 'e5' } }); + + const off = make(START, 'black', false); + assert.equal(off.tap('e2').kind, 'none', 'White is to move, but White is not ours'); + assert.equal(off.dragStart('g1').kind, 'none'); + assert.equal(off.drop('e2', 'e4').kind, 'none'); + assert.equal(off.hasPremove, false); +}); + +test('ownership: an own-colour premove queues off-turn and applies when the turn arrives', () => { + const bi = make(START_BLACK, 'white', false); + assert.equal(bi.tap('e2').kind, 'select'); + assert.deepEqual(bi.tap('e4'), { kind: 'premove', premove: { from: 'e2', to: 'e4' } }); + bi.setPosition(START); + bi.setTurn(true); + assert.deepEqual(bi.applyPremove(), { kind: 'move', move: { from: 'e2', to: 'e4' } }); +}); + +test('ownership: own-piece reselection and an own promotion premove still work', () => { + const bi = make(START_BLACK, 'white', false); + bi.tap('e2'); + assert.deepEqual(bi.tap('g1'), { kind: 'select', square: 'g1' }, 'reselect another own piece'); + + const promo = make(PROMO_B, 'white', false); + promo.tap('e7'); + assert.deepEqual(promo.tap('e8'), { kind: 'promotion', from: 'e7', to: 'e8', premove: true }); + assert.deepEqual(promo.resolvePromotion('n'), { kind: 'premove', premove: { from: 'e7', to: 'e8', promotion: 'n' } }); +}); + +test('ownership: a real player still gets the illegal-move result for an on-turn miss', () => { + const bi = make(START, 'white', true); + bi.tap('e2'); + assert.deepEqual(bi.tap('e5'), { kind: 'illegal', from: 'e2', to: 'e5' }); +}); + +test('ownership: a finished board stays inert whatever the owner', () => { + const bi = make(START, 'white', true); + bi.setInputEnabled(false); + assert.equal(bi.tap('e2').kind, 'none'); + bi.setPlayerColor('black'); + bi.setPlayerColor('white'); + assert.equal(bi.tap('e2').kind, 'none'); +}); + +test('ownership: a change of owner clears a stale selection', () => { + const bi = make(START, 'white', true); + bi.tap('e2'); + bi.setPlayerColor(null); + assert.equal(bi.highlights().selected, null); + assert.deepEqual([...bi.highlights().legal], []); + assert.equal(bi.tap('e4').kind, 'none', 'nothing left to complete'); +}); + +test('ownership: a change of owner clears a pending promotion', () => { + const bi = make(PROMO_W, 'white', true); + bi.tap('e7'); + assert.equal(bi.tap('e8').kind, 'promotion'); + bi.setPlayerColor(null); + assert.equal(bi.awaitingPromotion, false); + assert.equal(bi.resolvePromotion('q').kind, 'none', 'the old promotion cannot be completed'); +}); + +test('ownership: a change of owner clears a stale premove', () => { + for (const next of [null, 'black'] as const) { + const bi = make(START_BLACK, 'white', false); + bi.tap('e2'); + bi.tap('e4'); + assert.equal(bi.hasPremove, true); + bi.setPlayerColor(next); + assert.equal(bi.hasPremove, false, `cleared on change to ${next}`); + assert.deepEqual([...bi.highlights().premove], []); + } +}); + +test('ownership: re-asserting the same owner keeps the selection and premove', () => { + const bi = make(START_BLACK, 'white', false); + bi.tap('e2'); + bi.tap('e4'); + bi.tap('g1'); + bi.setPlayerColor('white'); + assert.equal(bi.hasPremove, true); + assert.equal(bi.highlights().selected, 'g1'); +}); + +test('ownership: a Chess960 position follows the same boundary', () => { + const bi = make(SP700_BLACK, 'white', false); + assert.equal(bi.tap('b7').kind, 'none', 'Black is to move, but Black is not ours'); + assert.equal(bi.tap('d1').kind, 'select', "White's king on d1 is ours"); + const black = make(SP700_BLACK, 'black', true); + assert.equal(black.tap('d1').kind, 'none'); + black.tap('b7'); + assert.deepEqual(black.tap('b5'), { kind: 'move', move: { from: 'b7', to: 'b5' } }); +}); + +test('ownership: a standalone board without players still moves the side to move', () => { + const bi = new BoardInteraction({ oracle: oracle() }); + bi.setPosition(START_BLACK); + assert.equal(bi.tap('e7').kind, 'select'); + assert.deepEqual(bi.tap('e5'), { kind: 'move', move: { from: 'e7', to: 'e5' } }); +}); diff --git a/scripts/test/check-test-topology.test.mjs b/scripts/test/check-test-topology.test.mjs index 21eea2c5..b6316a98 100644 --- a/scripts/test/check-test-topology.test.mjs +++ b/scripts/test/check-test-topology.test.mjs @@ -450,8 +450,9 @@ test('topology: extractPlaywrightPatterns fails closed on non-literal static exp test('topology: getPlaywrightDiscoveredFiles derives reachable files directly from Playwright CLI', () => { const discovered = getPlaywrightDiscoveredFiles('packages/web'); - assert.equal(discovered.size, 31); + assert.equal(discovered.size, 32); assert.ok(discovered.has('packages/web/e2e/illegal-move-feedback.spec.ts')); + assert.ok(discovered.has('packages/web/e2e/board-ownership.spec.ts')); assert.ok(discovered.has('packages/web/e2e/game-actions.spec.ts')); assert.ok(discovered.has('packages/web/e2e/app-loads.spec.ts')); assert.ok(discovered.has('packages/web/e2e/localization-state.spec.ts')); @@ -470,7 +471,8 @@ test('topology: backend-free Playwright discovers only the eleven offline specs' assert.ok(offline.has('packages/web/e2e/seek-creator-rating.spec.ts')); assert.ok(!offline.has('packages/web/e2e/game-actions.spec.ts')); assert.ok(!offline.has('packages/web/e2e/illegal-move-feedback.spec.ts')); - assert.equal(full.size, 31); + assert.ok(!offline.has('packages/web/e2e/board-ownership.spec.ts')); + assert.equal(full.size, 32); }); test('topology: falsification regression proves validation flags ignored test files unreachable', () => { From 6068e0be7e57faea79019f453cea441e4caa3f52 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 14:39:56 +0300 Subject: [PATCH 02/30] docs: record M15 Increment 87 player board ownership --- docs/PROJECT_STATE.md | 48 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 608f4cec..c836117b 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 86: Accessible local illegal-move feedback._ +_Last updated: 2026-10-03 — M15 Increment 87: Player board ownership and read-only spectators._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 86: Accessible local illegal-move feedback._ Prior: _Last updated: 2026-10-03 — M15 Increment 85: Trust terminal-analysis external review corrections._ @@ -4996,3 +4998,47 @@ Addresses four blocking review findings identified by ChatGPT independent review - **Independent review**: none was available. Codex rejected its configured model on this account, the Gemini CLI tier is ineligible, and agy returned a 429 quota error. The review was first-party only. - **Concurrency**: implemented on `2dd6d4d`. PR #88 (trust terminal-analysis retry isolation, M15 Increment 85) merged first as `f99a9e1` and was merged normally into this branch (no rebase, no force). Its entry and header chain are preserved unchanged, and this entry is renumbered to Increment 86. The only conflict was this file; #88 touches persistence, trust worker, helm and scripts, and no web or browser code. The validation above was rerun on the integrated tree, recorded below. - **Deliberate limits**: no reason text (the oracle exposes none); no feedback for a queued premove invalidated later (no production caller applies premoves today, so this is separate scope); no sound, vibration or animation; the board's own English ARIA labels remain outside the catalog as before. The owner performs the merge. + +## M15 Increment 87 — Player board ownership and read-only spectators (2026-10-03) + +- **Problem**, reverified on `origin/main` `d5af5be`: `BoardInteraction.movableColor()` fell back to the side to move when no `playerColor` was given, and the game route mounted its board without one and enabled input whenever `!isOver`. So a spectator could select, drag and premove pieces; the board was interactive before the `joined` role arrived (a pre-join tap queued a premove); an off-turn player could pick up the **opponent's** pieces (the side to move) and queue premoves with them; and joining a finished game briefly enabled input (`true` then `false`). `GameSync.submitMove` already refused spectator and off-turn moves, so no illegal move reached the server, but the board offered gestures it had no right to. +- **Contract** (`packages/web/src/core/interaction.ts`): ownership is explicit, `BoardOwner = Color | null | 'side-to-move'`. A player (`'white'`/`'black'`) picks up only their own colour, on and off turn. `null` (a spectator, or a player whose colour is not confirmed yet) picks up nothing: every tap, drag start, drop, keyboard activation, promotion and premove resolves to `none`, so spectators never see "illegal move". `'side-to-move'` is kept only for boards with no players (fallback, analysis, studies, endgame, learning), which still omit the option. `setPlayerColor(color | null)` drops the selection, a pending promotion and queued premoves on a real change and is a no-op otherwise. `drop()` re-checks the origin's owner, since the owner can change between drag start and drop. Ownership is the piece's colour on the square; legality is still only the oracle's, so no variant rule enters the client. +- **View and mount** (`ui/board-view.ts`, `app/board.ts`): `BoardView.setPlayerColor` also closes an open promotion chooser and abandons a drag in progress (float and window listeners), then renders. Focus, roving keyboard navigation and flipping are untouched, so a read-only board stays fully inspectable. `mountBoard` takes a `playerColor` option and exposes `setPlayerColor` with the same churn guard as `setInputEnabled`, so a no-op re-assertion neither clears an unheard rejection nor rebuilds the grid. +- **Game route** (`app/game-mount.ts`): the board mounts with `playerColor: null`. One `syncBoardOwnership()` reads `myColor` and `status.over` from the same `GameSync` snapshot and is called from both `onColor` and `onActionState`, so neither callback order can open a window. A finished game latches and never goes live again. Input is enabled exactly when there is an owner. No API, WebSocket or server change; the server stays authoritative. +- **Tests**: + - `board-ownership.test.ts` (14, core): spectators on both turns; fail-closed startup until a colour arrives; White-only and Black-only selection on and off turn, including opponent drops; own premove queued and applied; reselection and promotion premove; the illegal-move result for a real player; a finished board inert under owner changes; owner changes clearing selection, pending promotion and premoves; same-owner no-op; Chess960 (start position 700); the standalone side-to-move board. + - `board-a11y.test.ts` (+7, real `mountBoard`/`BoardView` on the fake DOM): spectator click, keyboard Enter/Space and drag submit nothing and announce nothing; keyboard navigation, focus and flip on a read-only board; own-colour-only input off-turn by click, keyboard and drag, including an own premove by drag; ownership loss mid-drag; ownership change closing the promotion chooser and clearing a premove; same-owner churn guard; remount carrying no ownership and submitting once. + - `board-ownership-route.test.ts` (10, real `mountGame` with fake socket, gestures through the board's click handler, `controller.submitMove` and `setInputEnabled` spied): no input before the socket opens or before `joined`; spectators read-only across syncs; a White legal move; off-turn White and Black limited to their own pieces; a finished join never enabled for any role; finished wins over a later live-looking sync; Chess960. + - Backend Playwright `board-ownership.spec.ts`: a real two-player game with a spectator. Black off-turn and the spectator try click, keyboard Enter and Space, and drag on both colours: nothing is selected or premoved, no `move` frame is sent, no message appears, and the spectator can still move focus with the arrow keys. White's keyboard e2–e4 commits; off-turn White cannot touch Black and queues an own d2–d4 premove without sending it; Black cannot touch White and its keyboard e7–e5 commits; the spectator stays read-only after both moves. + - `illegal-move-feedback.spec.ts`: the finished-board step tried Black's pawn because "off-turn the board offers the side to move". That premise is now false, so the step would pass vacuously; it now tries White's own premove d2–d4. +- **RED evidence**: on `d5af5be` the route tests compiled and 8 of 9 failed on behaviour (a pre-join premove, spectator selection, opponent selection off-turn for both colours, a transient `true,false` enable on a finished join, Chess960); the legal-move control passed. The new browser spec, run against main's web sources, failed because off-turn Black queued a premove of White's e2–e4. The core and view tests target the new `setPlayerColor` API and could not compile on main; their behaviour is pinned by the mutations below. +- **Falsification**: 16 compiled mutations, sources backed up to disk and restored by SHA-256; 15 killed by tests, none by compile errors: + - the route effectively using `playerColor ?? sideToMove` + - spectators treated as a player + - opponent-coloured premoves allowed off-turn + - mouse and drag guarded in the view but keyboard not (null owner falling back to the side to move) + - an owner change keeping a stale premove, pending promotion or selection (three mutations) + - finished not latched, and finished ignored + - remount not tearing down the previous mount + - `drop` ignoring ownership + - the view keeping a drag, or a promotion chooser, across an owner change + - no churn guard on `mountBoard.setPlayerColor` + - the route enabling input regardless of owner + + One mutation is equivalent: removing the route's `playerColor: null` at mount. `mountGame` is synchronous and `controller.start()` emits the pre-join state immediately, which sets the owner to `null` before the function returns, so no user event can land in between (a test clicking before the socket opens confirms it). The option is kept so fail-closed startup does not depend on that ordering. +- **Validation** (sequentially, on `3d1ed4a` over `d5af5be`): + - build, lint, all 8 `check:*` guards including `check:test-topology`, and `test:scripts` (312); + - web unit (1,456) and the hermetic suite (3,946 across 19 workspaces), with zero skips; + - static Playwright (187 of 187), 4 workers, 0 retries, with Avast Web/Network Shield off at the owner's direction. An earlier shield-on static run failed 3 non-board tests (lobby create-seek, email verification) waiting on local responses, and those specs then passed 270 of 270 in isolation (diagnostic, `--repeat-each=3`); + - backend Playwright (`GAMBIT_E2E_BACKEND=1`, 4 workers, 0 retries): **232 of 232**, 0 failed, 0 skipped, in the normal host state (shield on). Ports 4173 and 4174 were checked free beforehand, and a read-only commit sampler showed `vite preview` alive from start to teardown (minimum commit headroom 3,374 MB). + - Earlier backend runs, recorded so that none is mistaken for a pass: shield on, 231 of 232 (headless Chromium gone before `app-loads.spec.ts` started); shield off, invalid twice (`vite preview` exited natively with `0xC0000409`, then connection-refused for nearly every test); shield on after the separate preview diagnostic closed, 230 of 232 (an empty achievements count after 15 s, and the same Chromium launch fault), with the preview alive throughout. Every game and board spec passed in each run where the server was alive. The preview diagnostic found no product defect and did not prove a root cause; nothing in Playwright, Vite, timeouts, retries, workers or host settings was changed for these runs. +- **Independent review**: Gemini 3.8 Flash High was quota-blocked (`RESOURCE_EXHAUSTED`, 429), so the configured fallback, Claude Sonnet 4.6 Thinking via agy, reviewed the implementation read-only. A first fallback attempt failed on an invocation error (`--effort` is unsupported for that model), not on quota. The review returned six findings: + - Valid: own-colour premove by drag was untested. The test was added. + - Rejected: input enablement and ownership are two redundant locks. Both are idempotent, render only on a real change and run synchronously; keeping input enablement explicit is deliberate. + - Rejected: restoring the authoritative position after a game review could expose highlights. Game review runs only on finished games, where the owner is `null` and no selection can exist. + - Rejected: the view's churn guard is dead for standalone boards. The first call from `'side-to-move'` is a real change and later calls hit the guard. + - Rejected: `applyPremove` does not check the owner. It has no production caller, and the queue is always empty under a `null` owner. + - Rejected: `suppressClick` survives a cancelled drag. The drag's own trailing click consumes it, as on the existing path that disables input. + + The exact-final-head review is recorded in the PR. +- **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 8245eea12effc8319f778f436fd3f833a9ac3ccb Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 15:01:56 +0300 Subject: [PATCH 03/30] fix(web): drop the trailing click of a gesture cut short by an owner change Qodo on 6068e0b: BoardView.setPlayerColor cancelled an in-progress pointer gesture, which removed its pointer-up handler, so the gesture's trailing click reached tap() and could select the new owner's piece (e.g. a swipe begun before `joined` landed). An owner change during a gesture now suppresses that click, and every pointerdown resets the suppression so an abandoned gesture cannot swallow the next real click. --- packages/web/src/ui/board-view.ts | 5 +++++ packages/web/test/board-a11y.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 2e3a6064..08c75fb4 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -195,6 +195,9 @@ export class BoardView { setPlayerColor(color: Color | null): void { if (color === this.interaction.playerColor) return; if (this.overlay) this.cancelPromotion(); + // A pointer gesture still in progress belongs to the previous owner. Cancelling it removes the + // pointer-up handler, so swallow its trailing click here or it would tap for the new owner. + if (this.releaseDragListeners) this.suppressClick = true; this.cancelDrag(); this.interaction.setPlayerColor(color); this.render(); @@ -303,6 +306,8 @@ export class BoardView { if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; + // A new gesture: a suppression left by one whose click never came must not swallow this one's. + this.suppressClick = false; this.dragFrom = sq; this.dragging = false; this.startX = event.clientX; diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 049821eb..fa7ef855 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1034,6 +1034,28 @@ test('becoming a spectator mid-drag drops the floating piece and the drag cannot }); }); +test('a gesture cut short by an owner change selects nothing with its trailing click; the next click works', () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: null }); + // A swipe begun before the role arrives: nobody owns the pieces, so no drag starts. + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1 }); + win.dispatchEvent('pointermove', { ...centreOf('e4'), pointerId: 1 }); + board.setPlayerColor('white'); // `joined` lands mid-gesture + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); + root.dispatchEvent('click', centreOf('e2')); // the gesture's own trailing click + assert.equal(root.querySelector('[aria-selected="true"]'), null, 'the cancelled gesture selects nothing'); + + // A gesture abandoned off the board leaves no click behind; it must not swallow the next real one. + root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 2 }); + board.setPlayerColor(null); + board.setPlayerColor('white'); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 3 }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 3 }); + root.dispatchEvent('click', centreOf('e2')); + assert.equal(root.querySelector('[data-square="e2"]')?.getAttribute('aria-selected'), 'true', 'an ordinary click still selects'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From a623b11d812c4c9ce184c552770f0b1f4979a3fd Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 19:49:35 +0300 Subject: [PATCH 04/30] docs: record the Increment 87 review correction and its validation --- docs/PROJECT_STATE.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index c836117b..99cbf608 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -5007,7 +5007,7 @@ Addresses four blocking review findings identified by ChatGPT independent review - **Game route** (`app/game-mount.ts`): the board mounts with `playerColor: null`. One `syncBoardOwnership()` reads `myColor` and `status.over` from the same `GameSync` snapshot and is called from both `onColor` and `onActionState`, so neither callback order can open a window. A finished game latches and never goes live again. Input is enabled exactly when there is an owner. No API, WebSocket or server change; the server stays authoritative. - **Tests**: - `board-ownership.test.ts` (14, core): spectators on both turns; fail-closed startup until a colour arrives; White-only and Black-only selection on and off turn, including opponent drops; own premove queued and applied; reselection and promotion premove; the illegal-move result for a real player; a finished board inert under owner changes; owner changes clearing selection, pending promotion and premoves; same-owner no-op; Chess960 (start position 700); the standalone side-to-move board. - - `board-a11y.test.ts` (+7, real `mountBoard`/`BoardView` on the fake DOM): spectator click, keyboard Enter/Space and drag submit nothing and announce nothing; keyboard navigation, focus and flip on a read-only board; own-colour-only input off-turn by click, keyboard and drag, including an own premove by drag; ownership loss mid-drag; ownership change closing the promotion chooser and clearing a premove; same-owner churn guard; remount carrying no ownership and submitting once. + - `board-a11y.test.ts` (+8, real `mountBoard`/`BoardView` on the fake DOM): spectator click, keyboard Enter/Space and drag submit nothing and announce nothing; keyboard navigation, focus and flip on a read-only board; own-colour-only input off-turn by click, keyboard and drag, including an own premove by drag; ownership loss mid-drag; ownership change closing the promotion chooser and clearing a premove; same-owner churn guard; a gesture cut short by an owner change selecting nothing with its trailing click; remount carrying no ownership and submitting once. - `board-ownership-route.test.ts` (10, real `mountGame` with fake socket, gestures through the board's click handler, `controller.submitMove` and `setInputEnabled` spied): no input before the socket opens or before `joined`; spectators read-only across syncs; a White legal move; off-turn White and Black limited to their own pieces; a finished join never enabled for any role; finished wins over a later live-looking sync; Chess960. - Backend Playwright `board-ownership.spec.ts`: a real two-player game with a spectator. Black off-turn and the spectator try click, keyboard Enter and Space, and drag on both colours: nothing is selected or premoved, no `move` frame is sent, no message appears, and the spectator can still move focus with the arrow keys. White's keyboard e2–e4 commits; off-turn White cannot touch Black and queues an own d2–d4 premove without sending it; Black cannot touch White and its keyboard e7–e5 commits; the spectator stays read-only after both moves. - `illegal-move-feedback.spec.ts`: the finished-board step tried Black's pawn because "off-turn the board offers the side to move". That premise is now false, so the step would pass vacuously; it now tries White's own premove d2–d4. @@ -5038,7 +5038,16 @@ Addresses four blocking review findings identified by ChatGPT independent review - Rejected: restoring the authoritative position after a game review could expose highlights. Game review runs only on finished games, where the owner is `null` and no selection can exist. - Rejected: the view's churn guard is dead for standalone boards. The first call from `'side-to-move'` is a real change and later calls hit the guard. - Rejected: `applyPremove` does not check the owner. It has no production caller, and the queue is always empty under a `null` owner. - - Rejected: `suppressClick` survives a cancelled drag. The drag's own trailing click consumes it, as on the existing path that disables input. + - Rejected at first: `suppressClick` survives a cancelled drag. The stated reason, that the drag's own trailing click consumes the flag, was wrong. Cancelling a gesture removes its pointer-up handler, so the flag is never set and the trailing click reaches `tap`. Qodo found the real defect; see the correction below. The exact-final-head review is recorded in the PR. +- **Exact-head review correction** (Qodo, 1 bug on `6068e0b`, valid; Greptile 5/5 with no findings on the same head): an owner change during a pointer gesture cancelled it and removed its pointer-up handler, so the gesture's trailing click reached `tap`. A swipe begun before `joined` landed could therefore select the new owner's piece under the release point. `BoardView.setPlayerColor` now suppresses that click when a gesture is in progress, and every `pointerdown` resets the suppression, so a gesture abandoned off the board cannot swallow the next real click. A RED test (`board-a11y.test.ts`) came first, and two mutations, one undoing each half, are both killed. The exact-head review of `6068e0b` had been a strict self-review, because Gemini and Sonnet were both quota-blocked (429). It repeated the mistaken dismissal of the Sonnet finding above, so it did not catch this defect. +- **Validation of the corrected tree** (`8245eea`, run sequentially; nothing in Playwright, Vite, timeouts, retries, workers or host security settings was changed): + - build, lint, all 8 `check:*` guards and `test:scripts` (312); + - web unit (1,457) and the hermetic suite (3,947 across 19 workspaces), with zero skips; + - static Playwright **187 of 187** (4 workers, 0 retries), with Avast Web/Network Shield off for that one run at the owner's direction; + - backend Playwright **232 of 232** (4 workers, 0 retries), shield on. Ports 4173 and 4174 were free. A read-only sampler showed `vite preview` alive from start to teardown, minimum commit headroom 11,868 MB, minimum physical available 5,595 MB, and no pagefile growth. + - Runs on this tree that did not pass, recorded so that none is mistaken for one: + - Static with the shield on, three times: 185, 186 and 185 of 187. The failures were lobby `#create-seek` left disabled because session restore never completed (at 1440, 1024 and 768 px), an email-verification response never handled, and once headless Chromium gone at launch. In the last of these, commit headroom never fell below 9.5 GB, so memory was ruled out, and early pages took 10, 20, 30 and 40 s, the Avast loopback-stall pattern. The same tests took under a second with the shield off. + - Backend with the shield on: 230 of 232. A Playwright **test worker** exited with `0xC0000409` (so `game-vs-bot` never ran), and a Black player's page joined as "Spectating" in `game-responsive`. Commit headroom fell to 523 MB during that run and Windows expanded the pagefile, consistent with the commit-exhaustion mechanism but not proven to cause it. The only source change since the passing `3d1ed4a` run was the five-line click suppression in `board-view.ts`. Before the passing run above, the owner reduced unrelated host load: another project's `vite preview` holding port 4173, that project's Vitest run, and this task's idle original interactive session were stopped. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 98fb8b98d34e545ae726366835e17e39e3dfd64f Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 20:05:37 +0300 Subject: [PATCH 05/30] fix(web): clear click suppression when the release that set it makes no click Greptile on a623b11: a gesture cut short by an owner change and released off the board left suppressClick set, so the next click with no pointer events (assistive technology, programmatic activation) was swallowed. An ordinary drag dropped off the board leaked the same way. Suppression now covers only the click the release itself produces: it clears on the next tick, since the browser dispatches that click straight after pointer-up. An owner change waits for the cut-short gesture's release (or pointercancel) instead of setting a sticky flag; the wait ends at that release, at the next pointerdown, or on destroy. --- packages/web/src/ui/board-view.ts | 42 ++++++++++++++++++--- packages/web/test/board-a11y.test.ts | 55 ++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 08c75fb4..f8aca559 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -109,6 +109,8 @@ export class BoardView { private focusedSquare: Square | null = null; /** Removes the window listeners of the drag in progress; null when no drag is listening. */ private releaseDragListeners: (() => void) | null = null; + /** Stops waiting for the release of a gesture an owner change cut short; null when none is pending. */ + private releaseCancelledGesture: (() => void) | null = null; // Held as fields so `destroy` can remove the very same references `addEventListener` received. private readonly onClick = (e: MouseEvent): void => this.handleClick(e); private readonly onPointerDown = (e: PointerEvent): void => this.handlePointerDown(e); @@ -149,6 +151,7 @@ export class BoardView { */ destroy(): void { this.cancelDrag(); + this.releaseCancelledGesture?.(); this.closeOverlay(); this.root.removeEventListener('click', this.onClick); this.root.removeEventListener('pointerdown', this.onPointerDown); @@ -196,8 +199,8 @@ export class BoardView { if (color === this.interaction.playerColor) return; if (this.overlay) this.cancelPromotion(); // A pointer gesture still in progress belongs to the previous owner. Cancelling it removes the - // pointer-up handler, so swallow its trailing click here or it would tap for the new owner. - if (this.releaseDragListeners) this.suppressClick = true; + // pointer-up handler, so wait for its release here or its trailing click would tap for the new owner. + if (this.releaseDragListeners) this.awaitCancelledRelease(); this.cancelDrag(); this.interaction.setPlayerColor(color); this.render(); @@ -306,8 +309,8 @@ export class BoardView { if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; - // A new gesture: a suppression left by one whose click never came must not swallow this one's. - this.suppressClick = false; + // A new gesture means the one an owner change cut short is over. + this.releaseCancelledGesture?.(); this.dragFrom = sq; this.dragging = false; this.startX = event.clientX; @@ -328,6 +331,35 @@ export class BoardView { }; } + /** + * Swallow the click this pointer release produces, and nothing later. The browser dispatches that + * click straight after pointer-up, before any timer runs, so the flag clears on the next tick: a + * release that makes no click (off the board) must not leave a flag that eats a later click, such + * as one from assistive technology that sends no pointer events. + */ + private suppressReleaseClick(): void { + this.suppressClick = true; + setTimeout(() => { + this.suppressClick = false; + }, 0); + } + + /** Wait for the release of a gesture an owner change cut short, and swallow only its click. */ + private awaitCancelledRelease(): void { + this.releaseCancelledGesture?.(); + const end = (): void => { + this.releaseCancelledGesture?.(); + this.suppressReleaseClick(); + }; + window.addEventListener('pointerup', end); + window.addEventListener('pointercancel', end); + this.releaseCancelledGesture = (): void => { + window.removeEventListener('pointerup', end); + window.removeEventListener('pointercancel', end); + this.releaseCancelledGesture = null; + }; + } + /** Abandon a drag in progress: stop listening, drop the floating piece, forget the gesture. */ private cancelDrag(): void { this.releaseDragListeners?.(); @@ -365,7 +397,7 @@ export class BoardView { this.endFloat(); this.dragging = false; this.dragFrom = null; - this.suppressClick = true; + this.suppressReleaseClick(); if (target) { this.dispatch(this.interaction.drop(from, target)); } else { diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index fa7ef855..fe200edf 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -690,23 +690,34 @@ function mountWithFeedback( } /** Install the window/document globals a pointer drag touches, for the duration of `run`. */ -function withDragGlobals(run: (win: FakeDOMNode) => void): void { +function installDragGlobals(): { win: FakeDOMNode; restore: () => void } { const win = new FakeDOMNode('window'); const doc = { createElement: (tag: string) => new FakeDOMNode(tag), body: new FakeDOMNode('body') }; const prevWindow = Object.getOwnPropertyDescriptor(globalThis, 'window'); const prevDocument = Object.getOwnPropertyDescriptor(globalThis, 'document'); Object.defineProperty(globalThis, 'window', { configurable: true, value: win }); Object.defineProperty(globalThis, 'document', { configurable: true, value: doc }); - try { - run(win); - } finally { + const restore = (): void => { if (prevWindow) Object.defineProperty(globalThis, 'window', prevWindow); else Reflect.deleteProperty(globalThis, 'window'); if (prevDocument) Object.defineProperty(globalThis, 'document', prevDocument); else Reflect.deleteProperty(globalThis, 'document'); + }; + return { win, restore }; +} + +function withDragGlobals(run: (win: FakeDOMNode) => void): void { + const { win, restore } = installDragGlobals(); + try { + run(win); + } finally { + restore(); } } +/** Let pending timers run, as the browser does between one input event and the next. */ +const nextTask = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)); + function drag(root: FakeBoardRoot, win: FakeDOMNode, from: string, to: string): void { root.dispatchEvent('pointerdown', { ...centreOf(from), pointerId: 1 }); win.dispatchEvent('pointermove', { ...centreOf(to), pointerId: 1 }); @@ -1031,6 +1042,14 @@ test('becoming a spectator mid-drag drops the floating piece and the drag cannot win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); assert.deepEqual(moves, []); assert.equal(win.listenerCount('pointermove'), 0); + assert.equal(win.listenerCount('pointerup'), 0, 'the cut-short gesture stops waiting once released'); + + // Disposed while still waiting for a cut-short gesture's release: nothing stays attached. + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 2 }); + board.setPlayerColor('white'); + board.dispose(); + assert.equal(win.listenerCount('pointerup'), 0, 'disposal stops waiting for the release'); + assert.equal(win.listenerCount('pointercancel'), 0); }); }); @@ -1056,6 +1075,34 @@ test('a gesture cut short by an owner change selects nothing with its trailing c }); }); +test('a click with no pointer gesture, such as from assistive technology, is never swallowed by an earlier release', async () => { + const { win, restore } = installDragGlobals(); + try { + const { root, board } = mountWithFeedback({ playerColor: null }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + const offBoard = { clientX: 900, clientY: 900 }; + + // A gesture cut short by an owner change, released off the board: no click follows it. + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1 }); + board.setPlayerColor('white'); + win.dispatchEvent('pointerup', { ...offBoard, pointerId: 1 }); + await nextTask(); + root.dispatchEvent('click', centreOf('e2')); // an activation that sends no pointer events + assert.equal(selected(), 'e2', 'the next click is not eaten by the cut-short gesture'); + root.dispatchEvent('click', centreOf('e2')); // deselect again + + // An ordinary drag dropped off the board: no click follows that release either. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 2 }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 2 }); + win.dispatchEvent('pointerup', { ...offBoard, pointerId: 2 }); + await nextTask(); + root.dispatchEvent('click', centreOf('e2')); + assert.equal(selected(), 'e2', 'the next click is not eaten by an off-board drop'); + } finally { + restore(); + } +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 631b37371ae889a50bf150310180103119be631c Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 21:55:54 +0300 Subject: [PATCH 06/30] docs: record the Greptile correction and Linux backend validation for Increment 87 --- docs/PROJECT_STATE.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 99cbf608..68996af8 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: Player board ownership and read-only spectators._ +_Last updated: 2026-10-03 — M15 Increment 87: Greptile click-suppression correction and Linux backend validation._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: Player board ownership and read-only spectators._ Prior: _Last updated: 2026-10-03 — M15 Increment 86: Accessible local illegal-move feedback._ @@ -5050,4 +5052,20 @@ Addresses four blocking review findings identified by ChatGPT independent review - Runs on this tree that did not pass, recorded so that none is mistaken for one: - Static with the shield on, three times: 185, 186 and 185 of 187. The failures were lobby `#create-seek` left disabled because session restore never completed (at 1440, 1024 and 768 px), an email-verification response never handled, and once headless Chromium gone at launch. In the last of these, commit headroom never fell below 9.5 GB, so memory was ruled out, and early pages took 10, 20, 30 and 40 s, the Avast loopback-stall pattern. The same tests took under a second with the shield off. - Backend with the shield on: 230 of 232. A Playwright **test worker** exited with `0xC0000409` (so `game-vs-bot` never ran), and a Black player's page joined as "Spectating" in `game-responsive`. Commit headroom fell to 523 MB during that run and Windows expanded the pagefile, consistent with the commit-exhaustion mechanism but not proven to cause it. The only source change since the passing `3d1ed4a` run was the five-line click suppression in `board-view.ts`. Before the passing run above, the owner reduced unrelated host load: another project's `vite preview` holding port 4173, that project's Vitest run, and this task's idle original interactive session were stopped. +- **Second exact-head review correction** (Greptile on `a623b11`, confidence 5/5 with one non-blocking finding, valid; Qodo 0 bugs and 0 rule violations on the same head, with its earlier finding resolved): the first fix set `suppressClick` so that only a later click cleared it. A gesture cut short by an owner change and released off the board produces no click, so the flag stayed set and the next click with no pointer events (assistive technology, programmatic activation) was swallowed. An ordinary drag dropped off the board leaked the same way. + - Fix (`98fb8b9`): suppression now covers only the click the release itself produces, clearing on the next tick, because the browser dispatches that click straight after pointer-up. An owner change during a gesture waits for that gesture's pointer-up or pointercancel instead of setting a sticky flag, and the wait ends at that release, at the next pointerdown, or on `destroy`. The `pointerdown` reset added in `8245eea` was replaced by ending the wait. + - RED test first (`board-a11y.test.ts`): a click with no pointer gesture after a cut-short release, and after an off-board drop, both failed on `a623b11`. The mid-drag owner-change test also asserts that no `pointerup` or `pointercancel` listener survives the release or disposal. Six mutations were compiled and all were killed: release suppression never clearing, the owner change not waiting, pointerdown not ending the wait, destroy leaving the wait, the waiter staying after firing, and the drop path back to a sticky flag. + - The exact-head review of `a623b11` was again a strict self-review, because Gemini and Sonnet were both quota-blocked (429). Its claim that "only an off-board click can consume a stale flag" was wrong, because clicks without pointer events exist, so it did not catch this defect. +- **Validation of `98fb8b9`** (sequentially; nothing in Playwright, Vite, timeouts, retries, workers, Node or host security settings was changed): + - build, lint, all 8 `check:*` guards and `test:scripts` (312); + - web unit (1,458) and the hermetic suite (3,948 across 19 workspaces), with zero skips; + - static Playwright **187 of 187** (4 workers, 0 retries) on Windows, with Avast Web/Network Shield off for that one run at the owner's direction; + - backend Playwright **232 of 232** on Linux. The exact tree came from `git archive 98fb8b9`, with no Windows `node_modules` or uncommitted files, and ran on WSL2 Ubuntu 26.04 with Node 22.23.3 (the version CI uses), `npm ci`, `npm run build`, Playwright's own `install-deps chromium` and `GAMBIT_E2E_BACKEND=1 npx playwright test --retries=0`: 4 workers, 0 failed, 0 skipped, 0 flaky, 0 retries. + - Windows backend runs on this tree, recorded so that none is mistaken for a pass: + - invalid once: `vite preview` exited with `0xC0000409` about 75 s in, with system commit headroom about 13.5 GB and the preview's memory flat; + - 230 of 232 with `cdb` attached to the preview only: a Playwright test worker exited with `0xC0000409`, and the analysis panel's run button stayed disabled. The preview survived with no exception. + - **232 of 232** with `cdb` attached to the preview and every test worker, with zero exceptions captured in any process; + - 231 of 232 uninstrumented: headless Chromium was gone at `browser.newContext` before one test body started. + + These are recorded as host or environmental evidence only. Both processes that died run Node 24.15.0 with Avast's `aswhook.dll` injected, the crashes happened with the shield on and off and at high and low commit headroom, and the instrumented runs captured no failure. No root cause was proven, and the Windows instability is not fixed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 38e1bdd2dc5091c86834b93618ca87f745ce6eff Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:02:15 +0300 Subject: [PATCH 07/30] docs: correct Increment 87 over-claims and record the exact-head review --- docs/PROJECT_STATE.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 68996af8..54a1a5a4 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -5053,7 +5053,7 @@ Addresses four blocking review findings identified by ChatGPT independent review - Static with the shield on, three times: 185, 186 and 185 of 187. The failures were lobby `#create-seek` left disabled because session restore never completed (at 1440, 1024 and 768 px), an email-verification response never handled, and once headless Chromium gone at launch. In the last of these, commit headroom never fell below 9.5 GB, so memory was ruled out, and early pages took 10, 20, 30 and 40 s, the Avast loopback-stall pattern. The same tests took under a second with the shield off. - Backend with the shield on: 230 of 232. A Playwright **test worker** exited with `0xC0000409` (so `game-vs-bot` never ran), and a Black player's page joined as "Spectating" in `game-responsive`. Commit headroom fell to 523 MB during that run and Windows expanded the pagefile, consistent with the commit-exhaustion mechanism but not proven to cause it. The only source change since the passing `3d1ed4a` run was the five-line click suppression in `board-view.ts`. Before the passing run above, the owner reduced unrelated host load: another project's `vite preview` holding port 4173, that project's Vitest run, and this task's idle original interactive session were stopped. - **Second exact-head review correction** (Greptile on `a623b11`, confidence 5/5 with one non-blocking finding, valid; Qodo 0 bugs and 0 rule violations on the same head, with its earlier finding resolved): the first fix set `suppressClick` so that only a later click cleared it. A gesture cut short by an owner change and released off the board produces no click, so the flag stayed set and the next click with no pointer events (assistive technology, programmatic activation) was swallowed. An ordinary drag dropped off the board leaked the same way. - - Fix (`98fb8b9`): suppression now covers only the click the release itself produces, clearing on the next tick, because the browser dispatches that click straight after pointer-up. An owner change during a gesture waits for that gesture's pointer-up or pointercancel instead of setting a sticky flag, and the wait ends at that release, at the next pointerdown, or on `destroy`. The `pointerdown` reset added in `8245eea` was replaced by ending the wait. + - Fix (`98fb8b9`): suppression now covers only the click the release itself produces, clearing on the next tick. For mouse and pen, the browser dispatches that click straight after pointer-up. For touch, where a tap's click is synthesized from a later gesture event, this ordering is not verified (see the exact-head review below). An owner change during a gesture waits for that gesture's pointer-up or pointercancel instead of setting a sticky flag, and the wait ends at that release, at the next pointerdown, or on `destroy`. The `pointerdown` reset added in `8245eea` was replaced by ending the wait. - RED test first (`board-a11y.test.ts`): a click with no pointer gesture after a cut-short release, and after an off-board drop, both failed on `a623b11`. The mid-drag owner-change test also asserts that no `pointerup` or `pointercancel` listener survives the release or disposal. Six mutations were compiled and all were killed: release suppression never clearing, the owner change not waiting, pointerdown not ending the wait, destroy leaving the wait, the waiter staying after firing, and the drop path back to a sticky flag. - The exact-head review of `a623b11` was again a strict self-review, because Gemini and Sonnet were both quota-blocked (429). Its claim that "only an off-board click can consume a stale flag" was wrong, because clicks without pointer events exist, so it did not catch this defect. - **Validation of `98fb8b9`** (sequentially; nothing in Playwright, Vite, timeouts, retries, workers, Node or host security settings was changed): @@ -5067,5 +5067,11 @@ Addresses four blocking review findings identified by ChatGPT independent review - **232 of 232** with `cdb` attached to the preview and every test worker, with zero exceptions captured in any process; - 231 of 232 uninstrumented: headless Chromium was gone at `browser.newContext` before one test body started. - These are recorded as host or environmental evidence only. Both processes that died run Node 24.15.0 with Avast's `aswhook.dll` injected, the crashes happened with the shield on and off and at high and low commit headroom, and the instrumented runs captured no failure. No root cause was proven, and the Windows instability is not fixed. + These are recorded as host or environmental evidence only. Both processes that died run Node 24.15.0 with Avast's `aswhook.dll` injected, the crashes happened with the shield on and off and at high and low commit headroom, and the instrumented runs captured no exception, although the run with `cdb` on the preview only still failed when an uninstrumented worker died. No root cause was proven, and the Windows instability is not fixed. +- **Exact-final-head review** (head `631b373`): Gemini 3.8 Flash High and Claude Sonnet 4.6 were both quota-blocked (429). Under the hierarchy's last step, a strict Claude review was run by a fresh read-only reviewer agent with no shared context. **APPROVE WITH NITS**: no CRITICAL or HIGH defects, no hole in the ownership boundary, and both review fixes traced correct, with the unit tests failing if either fix is reverted. Its LOW items are recorded here, not fixed, because each needs a code change and full gate reruns: + - A cut-short **touch** tap could still select a piece for the new owner if its synthesized click arrives after the next tick. It never submits a move, and it needs an owner change during that tap. + - The release handlers do not filter on `pointerId`, so a second finger lifting ends the wait or a drag early. The drag half predates this PR. + - An ordinary drag does not listen for `pointercancel` (predates this PR). + - The click-timing fixes are covered by fake-DOM unit tests only, with no browser touch or interrupted-gesture case. The mutation harness is outside the repository. + - Two over-claims in this entry, the click ordering and "captured no failure", were corrected in the commit that adds this bullet. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 2ae8589f623d99d2a4c6d3b8444f1de9778b91af Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:15:10 +0300 Subject: [PATCH 08/30] fix(web): tie click suppression to the pointer that produced the click Qodo and Greptile on 38e1bdd: clearing suppression on a next-tick timer assumed a release's click arrives in the same task, which touch does not guarantee, so a late touch click from a completed drag or from a gesture an owner change cut short could still act. Qodo also found that the cut-short wait ended on any pointer's release, so a second finger lifting first let the original finger's click through. Suppression now names the pointer whose next click must be swallowed and matches the click's own pointerId whenever it arrives. A click with no pointer behind it (pointerType '', e.g. assistive technology) is never swallowed. The wait ends only on that pointer's release or cancel, a new press by the same pointer, or destroy; a new press by the same pointer also clears a stale entry from a release that made no click. Engines whose clicks carry no pointer fields keep suppressing the release's own click. --- packages/web/src/ui/board-view.ts | 57 +++++++++++------- packages/web/test/board-a11y.test.ts | 87 +++++++++++++++++++++++++++- 2 files changed, 119 insertions(+), 25 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index f8aca559..055aadab 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -60,6 +60,18 @@ function pieceClass(color: string, role: string): string { } const DRAG_THRESHOLD = 6; +/** + * Whether `event` is the click produced by pointer `pointerId`. Browsers deliver `click` as a + * PointerEvent carrying the id of the pointer behind it, whenever that click arrives. A click with no + * pointer (assistive technology, keyboard, `element.click()`) has `pointerType` '' and never counts. An + * engine whose clicks carry no pointer fields counts every click, keeping the release's own suppressed. + */ +function isClickOf(event: MouseEvent, pointerId: number): boolean { + const click = event as Partial; + if (click.pointerType === '') return false; + return click.pointerId === undefined || click.pointerId === pointerId; +} + /** A resolved user gesture: either a committed move or a queued premove. */ export type ResolvedMove = | { readonly kind: 'move'; readonly move: Premove } @@ -104,13 +116,19 @@ export class BoardView { private startY = 0; private pointerId: number | null = null; private floatEl: HTMLElement | null = null; - private suppressClick = false; + /** + * The pointer whose next click must be swallowed: one that just released a drag, or a gesture an + * owner change cut short. Null when none. Cleared by that click or by the same pointer pressing again. + */ + private suppressClickOf: number | null = null; private overlay: HTMLElement | null = null; private focusedSquare: Square | null = null; /** Removes the window listeners of the drag in progress; null when no drag is listening. */ private releaseDragListeners: (() => void) | null = null; /** Stops waiting for the release of a gesture an owner change cut short; null when none is pending. */ private releaseCancelledGesture: (() => void) | null = null; + /** The pointer whose cut-short gesture is being waited for; null when none. */ + private cancelledGesturePointer: number | null = null; // Held as fields so `destroy` can remove the very same references `addEventListener` received. private readonly onClick = (e: MouseEvent): void => this.handleClick(e); private readonly onPointerDown = (e: PointerEvent): void => this.handlePointerDown(e); @@ -200,7 +218,7 @@ export class BoardView { if (this.overlay) this.cancelPromotion(); // A pointer gesture still in progress belongs to the previous owner. Cancelling it removes the // pointer-up handler, so wait for its release here or its trailing click would tap for the new owner. - if (this.releaseDragListeners) this.awaitCancelledRelease(); + if (this.releaseDragListeners && this.pointerId !== null) this.awaitCancelledRelease(this.pointerId); this.cancelDrag(); this.interaction.setPlayerColor(color); this.render(); @@ -229,8 +247,8 @@ export class BoardView { } private handleClick(event: MouseEvent): void { - if (this.suppressClick) { - this.suppressClick = false; + if (this.suppressClickOf !== null && isClickOf(event, this.suppressClickOf)) { + this.suppressClickOf = null; return; } if (this.overlay) return; @@ -309,8 +327,9 @@ export class BoardView { if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; - // A new gesture means the one an owner change cut short is over. - this.releaseCancelledGesture?.(); + // The same pointer pressing again: its earlier release made no click here, and any wait for it is over. + if (event.pointerId === this.suppressClickOf) this.suppressClickOf = null; + if (event.pointerId === this.cancelledGesturePointer) this.releaseCancelledGesture?.(); this.dragFrom = sq; this.dragging = false; this.startX = event.clientX; @@ -332,30 +351,24 @@ export class BoardView { } /** - * Swallow the click this pointer release produces, and nothing later. The browser dispatches that - * click straight after pointer-up, before any timer runs, so the flag clears on the next tick: a - * release that makes no click (off the board) must not leave a flag that eats a later click, such - * as one from assistive technology that sends no pointer events. + * Wait for the release of the pointer whose gesture an owner change cut short, then swallow that + * pointer's click whenever it arrives. Other pointers' releases are ignored: on an engine whose + * clicks carry no pointer id, ending the wait early would swallow another pointer's click. */ - private suppressReleaseClick(): void { - this.suppressClick = true; - setTimeout(() => { - this.suppressClick = false; - }, 0); - } - - /** Wait for the release of a gesture an owner change cut short, and swallow only its click. */ - private awaitCancelledRelease(): void { + private awaitCancelledRelease(pointerId: number): void { this.releaseCancelledGesture?.(); - const end = (): void => { + const end = (e: PointerEvent): void => { + if (e.pointerId !== pointerId) return; this.releaseCancelledGesture?.(); - this.suppressReleaseClick(); + this.suppressClickOf = pointerId; }; window.addEventListener('pointerup', end); window.addEventListener('pointercancel', end); + this.cancelledGesturePointer = pointerId; this.releaseCancelledGesture = (): void => { window.removeEventListener('pointerup', end); window.removeEventListener('pointercancel', end); + this.cancelledGesturePointer = null; this.releaseCancelledGesture = null; }; } @@ -397,7 +410,7 @@ export class BoardView { this.endFloat(); this.dragging = false; this.dragFrom = null; - this.suppressReleaseClick(); + this.suppressClickOf = event.pointerId; if (target) { this.dispatch(this.interaction.drop(from, target)); } else { diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index fe200edf..390e2ca4 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -715,6 +715,15 @@ function withDragGlobals(run: (win: FakeDOMNode) => void): void { } } +/** + * A click from assistive technology, the keyboard or `element.click()`: browsers deliver it as a + * PointerEvent with no pointer behind it (`pointerType` '' and a pointerId of -1). + */ +const atClick = (sq: string, pointerId?: number): object => ({ ...centreOf(sq), pointerType: '', ...(pointerId !== undefined ? { pointerId } : {}) }); + +/** A click produced by a real pointer, carrying that pointer's id and type, as browsers deliver it. */ +const pointerClick = (sq: string, pointerId: number, pointerType = 'touch'): object => ({ ...centreOf(sq), pointerId, pointerType }); + /** Let pending timers run, as the browser does between one input event and the next. */ const nextTask = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)); @@ -1087,22 +1096,94 @@ test('a click with no pointer gesture, such as from assistive technology, is nev board.setPlayerColor('white'); win.dispatchEvent('pointerup', { ...offBoard, pointerId: 1 }); await nextTask(); - root.dispatchEvent('click', centreOf('e2')); // an activation that sends no pointer events + root.dispatchEvent('click', atClick('e2')); // an activation with no pointer behind it, and no pointer id assert.equal(selected(), 'e2', 'the next click is not eaten by the cut-short gesture'); - root.dispatchEvent('click', centreOf('e2')); // deselect again + root.dispatchEvent('click', atClick('e2', -1)); // deselect again // An ordinary drag dropped off the board: no click follows that release either. root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 2 }); win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 2 }); win.dispatchEvent('pointerup', { ...offBoard, pointerId: 2 }); await nextTask(); - root.dispatchEvent('click', centreOf('e2')); + root.dispatchEvent('click', atClick('e2')); assert.equal(selected(), 'e2', 'the next click is not eaten by an off-board drop'); + root.dispatchEvent('click', atClick('e2')); // deselect again + + // A mouse drag dropped off the board, then an ordinary click by that same mouse. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointerup', { ...offBoard, pointerId: 1, pointerType: 'mouse' }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + root.dispatchEvent('click', pointerClick('e2', 1, 'mouse')); + assert.equal(selected(), 'e2', 'pressing again clears the earlier release that made no click'); + root.dispatchEvent('click', atClick('e2', -1)); // deselect again + + // A gesture cut short by an owner change whose release never arrived, then that pointer presses again. + board.setPlayerColor(null); + root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 1, pointerType: 'mouse' }); + board.setPlayerColor('white'); // the release of this press is lost + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + root.dispatchEvent('click', pointerClick('e2', 1, 'mouse')); + assert.equal(selected(), 'e2', 'a new press by that pointer ends the wait, so its own click works'); } finally { restore(); } }); +test('a touch click that arrives after its release, in a later task, is still swallowed', async () => { + const { win, restore } = installDragGlobals(); + try { + const { root, board, moves } = mountWithFeedback({ playerColor: null }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + + // A touch tap cut short by an owner change: its click is synthesized after the release. + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 11, pointerType: 'touch' }); + board.setPlayerColor('white'); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 11, pointerType: 'touch' }); + await nextTask(); + root.dispatchEvent('click', pointerClick('e2', 11)); + assert.equal(selected(), null, 'the cut-short tap selects nothing for the new owner'); + + // A touch drag that wobbles back onto its own piece; its click comes late, after another finger's tap. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 12, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 12, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('g1'), pointerId: 12, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 12, pointerType: 'touch' }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 13, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 13, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('e2', 13)); + assert.equal(selected(), 'e2', "another finger's own tap is not swallowed"); + root.dispatchEvent('click', pointerClick('e2', 13)); // deselect again + await nextTask(); + root.dispatchEvent('click', pointerClick('g1', 12)); + assert.equal(selected(), null, "the drag's late click does not select the piece it was dropped on"); + assert.deepEqual(moves, [], 'nothing was submitted'); + } finally { + restore(); + } +}); + +test('a second finger releasing first does not end the wait for the gesture an owner change cut short', () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: null }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 21, pointerType: 'touch' }); // finger 1 + board.setPlayerColor('white'); // `joined` lands while finger 1 is down + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 22, pointerType: 'touch' }); // finger 2 + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 22, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('g1', 22)); + assert.equal(selected(), 'g1', "finger 2's own tap, begun under the new owner, selects"); + + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 21, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('e2', 21)); + assert.equal(selected(), 'g1', "finger 1's cut-short tap is still swallowed"); + assert.equal(win.listenerCount('pointerup'), 0, 'the wait ends with its own pointer'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 79686a5c0af4397ea0893343a2318f8a3094dfbc Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:28:17 +0300 Subject: [PATCH 09/30] docs: record the pointer-matched suppression fix and its Linux validation --- docs/PROJECT_STATE.md | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 54a1a5a4..46f40b2f 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: Greptile click-suppression correction and Linux backend validation._ +_Last updated: 2026-10-03 — M15 Increment 87: pointer-matched click suppression and full Linux validation._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: Greptile click-suppression correction and Linux backend validation._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: Player board ownership and read-only spectators._ @@ -5074,4 +5076,26 @@ Addresses four blocking review findings identified by ChatGPT independent review - An ordinary drag does not listen for `pointercancel` (predates this PR). - The click-timing fixes are covered by fake-DOM unit tests only, with no browser touch or interrupted-gesture case. The mutation harness is outside the repository. - Two over-claims in this entry, the click ordering and "captured no failure", were corrected in the commit that adds this bullet. +- **Third exact-head review correction** (on `38e1bdd`): CI was green, including M6 acceptance at 232 passed with 0 flaky. Three findings were raised, all valid: + - Qodo #1 and Greptile (one root cause): clearing suppression on a next-tick timer assumed a release's click arrives in the same task. Touch does not guarantee that, so a late touch click from a completed drag, or from a gesture an owner change cut short, could still act on the board. + - Qodo #2: the cut-short wait ended on any pointer's release, so a second finger lifting first let the original finger's click through. + + These are the first two LOW items recorded by the exact-head review above, now fixed. + - Fix (`2ae8589`): suppression is tied to the pointer, not to time. `BoardView` records which pointer's next click must be swallowed (a drag's release, or a gesture an owner change cut short) and matches the click's own `pointerId` whenever it arrives, because browsers deliver `click` as a PointerEvent carrying that id. A click with `pointerType` '' (assistive technology, keyboard, `element.click()`) is never swallowed. + - The cut-short wait ends only on that pointer's release or cancel, on a new press by the same pointer, or on `destroy`. A new press by the same pointer also clears a stale entry left by a release that made no click. There is no timer. + - An engine whose clicks carry no pointer fields keeps suppressing the release's own click; this is the fallback path. + - RED tests came first (`board-a11y.test.ts`): a touch click arriving in a later task after a cut-short release, and two interleaved fingers, both failed on `38e1bdd`. Further cases cover: + - a drag that wobbles back onto its own piece with a late click; + - another finger's tap between a drag release and its click; + - a mouse re-press after an off-board drop; + - a re-press after a lost release; + - an assistive-technology click with no pointer id. + - Eleven compiled mutations, one per part of the fix, were all killed by tests, and the source was restored byte-identical. The mutated parts were: the `pointerType` rule, pointer-id matching, the wait's own-pointer check, the re-press clearing a stale entry, the re-press ending the wait, the drag release and the cut-short release recording suppression, the owner change waiting, `destroy`, the waiter removing itself, and a swallowed click clearing the entry. + - Of that review's LOW items, `pointercancel` on an ordinary drag (predates this PR) and the lack of a real-browser touch test remain. +- **Validation of `2ae8589`, entirely on Linux** (the exact tree from `git archive 2ae8589`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3, npm 10.9.9 and the repository's normal worker calculation, giving 4 workers): + - `npm ci` and `npm run build` succeeded. + - hermetic suite **3,950 of 3,950** across 19 workspaces, with zero skips (web 1,460); + - static Playwright **187 of 187**, 0 flaky, `--retries=0`; + - backend Playwright **232 of 232**, 0 flaky, `--retries=0`. + - On Windows, build, lint, all 8 guards and `test:scripts` (312) passed on this commit. The Windows hermetic run failed one file, `packages/api` `resources.test.js`, at file level with no failing assertion. That file passed 14 of 14 in three isolated reruns, and this PR does not touch `packages/api`. It is recorded with the other host faults above, without a proven cause. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 1bbb5bfa930fdd57bcd390824eff740748975faa Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:37:26 +0300 Subject: [PATCH 10/30] fix(web): bound the no-pointer-id click fallback and ignore other pointers' drag releases Independent exact-head review of 79686a5: - On an engine whose click carries no pointerId, a stale suppression (e.g. a touch drag released off the board, which makes no click) matched the next tap's click and swallowed it once. In that fallback a click now counts as the release's own only if no press came after the release (a press counter, not a timer). Engines with pointer ids are unchanged. - The drag's pointerup listener accepted any pointer, so another finger's release could drop the carried piece at its own coordinates. It now ignores releases from other pointers. --- packages/web/src/ui/board-view.ts | 45 ++++++++++++++++++---------- packages/web/test/board-a11y.test.ts | 29 ++++++++++++++++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 055aadab..8a965ff8 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -60,18 +60,6 @@ function pieceClass(color: string, role: string): string { } const DRAG_THRESHOLD = 6; -/** - * Whether `event` is the click produced by pointer `pointerId`. Browsers deliver `click` as a - * PointerEvent carrying the id of the pointer behind it, whenever that click arrives. A click with no - * pointer (assistive technology, keyboard, `element.click()`) has `pointerType` '' and never counts. An - * engine whose clicks carry no pointer fields counts every click, keeping the release's own suppressed. - */ -function isClickOf(event: MouseEvent, pointerId: number): boolean { - const click = event as Partial; - if (click.pointerType === '') return false; - return click.pointerId === undefined || click.pointerId === pointerId; -} - /** A resolved user gesture: either a committed move or a queued premove. */ export type ResolvedMove = | { readonly kind: 'move'; readonly move: Premove } @@ -121,6 +109,10 @@ export class BoardView { * owner change cut short. Null when none. Cleared by that click or by the same pointer pressing again. */ private suppressClickOf: number | null = null; + /** Presses seen on the board, so a click with no pointer id can be tied to the release before it. */ + private pressCount = 0; + /** {@link pressCount} when {@link suppressClickOf} was set. */ + private suppressedAtPress = 0; private overlay: HTMLElement | null = null; private focusedSquare: Square | null = null; /** Removes the window listeners of the drag in progress; null when no drag is listening. */ @@ -247,7 +239,7 @@ export class BoardView { } private handleClick(event: MouseEvent): void { - if (this.suppressClickOf !== null && isClickOf(event, this.suppressClickOf)) { + if (this.isSuppressedClick(event)) { this.suppressClickOf = null; return; } @@ -324,6 +316,7 @@ export class BoardView { } private handlePointerDown(event: PointerEvent): void { + this.pressCount += 1; if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; @@ -338,6 +331,7 @@ export class BoardView { this.releaseDragListeners?.(); const move = (e: PointerEvent): void => this.handlePointerMove(e); const up = (e: PointerEvent): void => { + if (e.pointerId !== this.pointerId) return; // another pointer's release is not this drag's this.releaseDragListeners?.(); this.handlePointerUp(e); }; @@ -360,7 +354,7 @@ export class BoardView { const end = (e: PointerEvent): void => { if (e.pointerId !== pointerId) return; this.releaseCancelledGesture?.(); - this.suppressClickOf = pointerId; + this.suppressClickFrom(pointerId); }; window.addEventListener('pointerup', end); window.addEventListener('pointercancel', end); @@ -373,6 +367,27 @@ export class BoardView { }; } + /** Swallow the next click produced by `pointerId`, whenever it arrives. */ + private suppressClickFrom(pointerId: number): void { + this.suppressClickOf = pointerId; + this.suppressedAtPress = this.pressCount; + } + + /** + * Whether `event` is the click the recorded release produced. Modern browsers deliver `click` as a + * PointerEvent carrying the id of the pointer behind it, whenever it arrives, so the id decides. A + * click with no pointer (assistive technology, keyboard, `element.click()`) has `pointerType` '' and + * never counts. On an engine whose clicks carry no pointer fields, a click counts only if no press + * came after the release: a later tap is a new gesture, not that release's click. + */ + private isSuppressedClick(event: MouseEvent): boolean { + if (this.suppressClickOf === null) return false; + const click = event as Partial; + if (click.pointerType === '') return false; + if (click.pointerId === undefined) return this.pressCount === this.suppressedAtPress; + return click.pointerId === this.suppressClickOf; + } + /** Abandon a drag in progress: stop listening, drop the floating piece, forget the gesture. */ private cancelDrag(): void { this.releaseDragListeners?.(); @@ -410,7 +425,7 @@ export class BoardView { this.endFloat(); this.dragging = false; this.dragFrom = null; - this.suppressClickOf = event.pointerId; + this.suppressClickFrom(event.pointerId); if (target) { this.dispatch(this.interaction.drop(from, target)); } else { diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 390e2ca4..69a8aef6 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1184,6 +1184,35 @@ test('a second finger releasing first does not end the wait for the gesture an o }); }); +test('on an engine whose clicks carry no pointer id, a stale suppression never eats a later tap', () => { + withDragGlobals((win) => { + const { root } = mountWithFeedback({ playerColor: 'white' }); + // A touch drag released off the board: real engines make no click for it. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 41, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 41, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { clientX: 900, clientY: 900, pointerId: 41, pointerType: 'touch' }); + // A new tap by a new touch pointer, whose click arrives as a plain MouseEvent (no pointer fields). + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 42, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 42, pointerType: 'touch' }); + root.dispatchEvent('click', centreOf('e2')); + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'the new tap selects'); + }); +}); + +test("another pointer's release does not drop the piece a drag is carrying", () => { + withDragGlobals((win) => { + const { root, moves } = mountWithFeedback({ playerColor: 'white' }); + const body = (globalThis.document as unknown as { body: FakeDOMNode }).body; + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 51, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('e3'), pointerId: 51, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 52, pointerType: 'touch' }); // another finger + assert.deepEqual(moves, [], 'nothing dropped by the other finger'); + assert.equal(body.children.length, 1, 'the drag is still carrying its piece'); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 51, pointerType: 'touch' }); + assert.deepEqual(moves, ['e2e4'], 'the dragging finger drops it'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 066f0469627567eafb5bd4393670963f356617d8 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:45:28 +0300 Subject: [PATCH 11/30] docs: record the bounded click fallback and its Linux validation --- docs/PROJECT_STATE.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 46f40b2f..2cbee480 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: pointer-matched click suppression and full Linux validation._ +_Last updated: 2026-10-03 — M15 Increment 87: bounded no-pointer-id click fallback and drag release ownership._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: pointer-matched click suppression and full Linux validation._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: Greptile click-suppression correction and Linux backend validation._ @@ -5098,4 +5100,14 @@ Addresses four blocking review findings identified by ChatGPT independent review - static Playwright **187 of 187**, 0 flaky, `--retries=0`; - backend Playwright **232 of 232**, 0 flaky, `--retries=0`. - On Windows, build, lint, all 8 guards and `test:scripts` (312) passed on this commit. The Windows hermetic run failed one file, `packages/api` `resources.test.js`, at file level with no failing assertion. That file passed 14 of 14 in three isolated reruns, and this PR does not touch `packages/api`. It is recorded with the other host faults above, without a proven cause. +- **Exact-head review of `79686a5` and its correction**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS**: no defect on Chromium or modern Firefox, and the 11 mutations map one-to-one to assertions. Two of its findings were fixed in `1bbb5bf`, with RED tests first: + - (MEDIUM, conditional) In the fallback for engines whose `click` carries no `pointerId`, a stale entry matched any later click. For example, a touch drag released off the board makes no click, so the next tap's click was swallowed once. In that fallback a click now counts as the release's own only if no press came after the release. This uses a press counter, not a timer; engines with pointer ids are unchanged. + - (LOW) The drag's `pointerup` listener accepted any pointer, so another finger's release could drop the carried piece at its own coordinates. It now ignores other pointers. + + The wording "browsers deliver click as a PointerEvent" above means modern browsers; older engines take the fallback. Not fixed: a second press during a live drag leaves the floating piece behind (this predates the PR). Mutation run on `1bbb5bf`: 14 compiled mutations, 13 killed by tests. The survivor, "a swallowed click keeps its entry", is an equivalent mutant: any later click from that pointer follows a new press by it, which already clears the entry, and the press counter covers the fallback. +- **Validation of `1bbb5bf`, entirely on Linux** (the exact tree from `git archive 1bbb5bf`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,952 of 3,952** across 19 workspaces, with zero skips (web 1,462); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint, all 8 guards and `test:scripts` (312). - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From f98a8b9d645bae4e4b37eae07b4dc921f488f2ff Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:53:04 +0300 Subject: [PATCH 12/30] fix(web): end a drag on its own pointercancel and on a new press Independent exact-head review of 066f046: drags never listened for pointercancel, so a touch drag the browser took over (a pan, a system gesture) stayed live with its floating piece; after the drag's pointerup started ignoring other pointers, no later event recovered it. A new press during a live drag also dropped the old drag's listeners without removing its floating piece. A drag now cancels on its own pointer's pointercancel, like a drop off the board (selection cleared, floating piece removed). A press that never became a drag keeps an existing selection. A new press calls cancelDrag() before starting, so no floating piece is left behind. --- packages/web/src/ui/board-view.ts | 14 +++++++++- packages/web/test/board-a11y.test.ts | 39 ++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 8a965ff8..eea42a0c 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -323,23 +323,35 @@ export class BoardView { // The same pointer pressing again: its earlier release made no click here, and any wait for it is over. if (event.pointerId === this.suppressClickOf) this.suppressClickOf = null; if (event.pointerId === this.cancelledGesturePointer) this.releaseCancelledGesture?.(); + // A new press ends any drag still in progress, its floating piece included. + this.cancelDrag(); this.dragFrom = sq; this.dragging = false; this.startX = event.clientX; this.startY = event.clientY; this.pointerId = event.pointerId; - this.releaseDragListeners?.(); const move = (e: PointerEvent): void => this.handlePointerMove(e); const up = (e: PointerEvent): void => { if (e.pointerId !== this.pointerId) return; // another pointer's release is not this drag's this.releaseDragListeners?.(); this.handlePointerUp(e); }; + // The browser can take a pointer over (a pan, a system gesture); that pointer then never releases. + const cancel = (e: PointerEvent): void => { + if (e.pointerId !== this.pointerId) return; + const wasDragging = this.dragging; + this.cancelDrag(); + if (!wasDragging) return; + this.interaction.cancelPromotion(); + this.render(); + }; window.addEventListener('pointermove', move); window.addEventListener('pointerup', up); + window.addEventListener('pointercancel', cancel); this.releaseDragListeners = (): void => { window.removeEventListener('pointermove', move); window.removeEventListener('pointerup', up); + window.removeEventListener('pointercancel', cancel); this.releaseDragListeners = null; }; } diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 69a8aef6..296de605 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1213,6 +1213,45 @@ test("another pointer's release does not drop the piece a drag is carrying", () }); }); +test('a drag the browser cancels drops its floating piece and stops listening', () => { + withDragGlobals((win) => { + const { root, moves } = mountWithFeedback({ playerColor: 'white' }); + const body = (globalThis.document as unknown as { body: FakeDOMNode }).body; + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 61, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('e3'), pointerId: 61, pointerType: 'touch' }); + assert.equal(body.children.length, 1, 'the drag shows a floating piece'); + win.dispatchEvent('pointercancel', { pointerId: 62, pointerType: 'touch' }); // another pointer's cancel + assert.equal(body.children.length, 1, "another pointer's cancel leaves the drag alone"); + win.dispatchEvent('pointercancel', { pointerId: 61, pointerType: 'touch' }); // e.g. a pan takes over + assert.equal(body.children.length, 0, 'the floating piece is gone'); + assert.equal(root.querySelector('.cb-dragging'), null); + assert.equal(win.listenerCount('pointermove') + win.listenerCount('pointerup') + win.listenerCount('pointercancel'), 0); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 61, pointerType: 'touch' }); + assert.deepEqual(moves, [], 'a cancelled drag moves nothing'); + assert.equal(root.querySelector('[aria-selected="true"]'), null, 'the cancelled drag leaves nothing selected'); + + // A press that never became a drag (a pan starting on the board) keeps an earlier selection. + root.dispatchEvent('click', atClick('d2')); + root.dispatchEvent('pointerdown', { ...centreOf('a2'), pointerId: 63, pointerType: 'touch' }); + win.dispatchEvent('pointercancel', { pointerId: 63, pointerType: 'touch' }); + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'd2'); + }); +}); + +test('a new press during a live drag ends that drag, floating piece included', () => { + withDragGlobals((win) => { + const { root, moves } = mountWithFeedback({ playerColor: 'white' }); + const body = (globalThis.document as unknown as { body: FakeDOMNode }).body; + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 71, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('e3'), pointerId: 71, pointerType: 'touch' }); + assert.equal(body.children.length, 1); + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 72, pointerType: 'touch' }); + assert.equal(body.children.length, 0, "the first drag's floating piece is not left behind"); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 71, pointerType: 'touch' }); + assert.deepEqual(moves, [], 'the abandoned drag drops nothing'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From dd523550420cb0e4ce0fd65bc5c0f59e1dc1115f Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 22:59:46 +0300 Subject: [PATCH 13/30] docs: record the drag cancellation fix and its Linux validation --- docs/PROJECT_STATE.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 2cbee480..7a51d897 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: bounded no-pointer-id click fallback and drag release ownership._ +_Last updated: 2026-10-03 — M15 Increment 87: drag cancellation and full Linux validation of the final code._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: bounded no-pointer-id click fallback and drag release ownership._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: pointer-matched click suppression and full Linux validation._ @@ -5110,4 +5112,16 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,952 of 3,952** across 19 workspaces, with zero skips (web 1,462); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint, all 8 guards and `test:scripts` (312). +- **Exact-head review of `066f046` and its correction**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS**, with no merge-blocking defect. Its one actionable finding is fixed in `f98a8b9`, with RED tests first. + - The finding: drags never listened for `pointercancel`, so a touch drag the browser took over (a pan, a system gesture) stayed live with its floating piece. The repository sets no `touch-action`, so this is plausible. Since `1bbb5bf` made the drag's `pointerup` ignore other pointers, no later event recovered such a drag. + - The fix: a drag now cancels on its own pointer's `pointercancel`, like a drop off the board (selection cleared, floating piece removed). A press that never became a drag keeps an existing selection. A new press now calls `cancelDrag()` before starting, which also fixes the pre-existing "second press during a live drag leaves the floating piece behind", previously recorded as not fixed. + - Seven compiled mutations of the new code were all killed by tests: no listener, any pointer's cancel, a new press only detaching listeners, no re-render, the selection kept, a non-drag cancel clearing the selection, and the listener not removed. + - Its other note, that a press outside any square keeps a stale entry, is harmless: the only click that entry could eat is one the board ignores anyway. +- **Validation of `f98a8b9`, entirely on Linux** (the exact tree from `git archive f98a8b9`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,954 of 3,954** across 19 workspaces, with zero skips (web 1,464); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. + + This supersedes the earlier "remain" notes: of the LOW items, only the lack of a real-browser touch test is still open. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 59362b96e05a4cb786607ddb216dab507558bc69 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 23:06:27 +0300 Subject: [PATCH 14/30] fix(web): a new press abandons another pointer's gesture without acting on it Independent exact-head review of dd52355: a press during another pointer's live drag only detached that drag's listeners. Its selection stayed, its release went unwatched, and its trailing click reached tap() with the piece still selected, so an abandoned drag could submit a move. Owner changes and new presses now share abandonGesture(): wait for the abandoned pointer's release and swallow its click, and undo a drag it started (selection cleared, re-rendered). A re-press by the same pointer (its release was lost) only resets, so its own click still works. --- packages/web/src/ui/board-view.ts | 26 ++++++++++++++++++++------ packages/web/test/board-a11y.test.ts | 19 +++++++++++++++++-- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index eea42a0c..c6258447 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -208,10 +208,8 @@ export class BoardView { setPlayerColor(color: Color | null): void { if (color === this.interaction.playerColor) return; if (this.overlay) this.cancelPromotion(); - // A pointer gesture still in progress belongs to the previous owner. Cancelling it removes the - // pointer-up handler, so wait for its release here or its trailing click would tap for the new owner. - if (this.releaseDragListeners && this.pointerId !== null) this.awaitCancelledRelease(this.pointerId); - this.cancelDrag(); + // A pointer gesture still in progress belongs to the previous owner. + this.abandonGesture(); this.interaction.setPlayerColor(color); this.render(); } @@ -323,8 +321,10 @@ export class BoardView { // The same pointer pressing again: its earlier release made no click here, and any wait for it is over. if (event.pointerId === this.suppressClickOf) this.suppressClickOf = null; if (event.pointerId === this.cancelledGesturePointer) this.releaseCancelledGesture?.(); - // A new press ends any drag still in progress, its floating piece included. - this.cancelDrag(); + // A new press ends any gesture still in progress. Another pointer's gesture is abandoned (its + // release must not tap); the same pointer pressing again means its release was lost. + if (this.releaseDragListeners && this.pointerId !== event.pointerId) this.abandonGesture(); + else this.cancelDrag(); this.dragFrom = sq; this.dragging = false; this.startX = event.clientX; @@ -400,6 +400,20 @@ export class BoardView { return click.pointerId === this.suppressClickOf; } + /** + * Abandon the pointer gesture in progress without acting on it. Cancelling removes its pointer-up + * handler, so wait for its release and swallow that click, or it would tap; a drag it started is + * undone (selection cleared, floating piece removed). + */ + private abandonGesture(): void { + if (this.releaseDragListeners && this.pointerId !== null) this.awaitCancelledRelease(this.pointerId); + const wasDragging = this.dragging; + this.cancelDrag(); + if (!wasDragging) return; + this.interaction.cancelPromotion(); + this.render(); + } + /** Abandon a drag in progress: stop listening, drop the floating piece, forget the gesture. */ private cancelDrag(): void { this.releaseDragListeners?.(); diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 296de605..83813779 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1247,8 +1247,23 @@ test('a new press during a live drag ends that drag, floating piece included', ( assert.equal(body.children.length, 1); root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 72, pointerType: 'touch' }); assert.equal(body.children.length, 0, "the first drag's floating piece is not left behind"); - win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 71, pointerType: 'touch' }); - assert.deepEqual(moves, [], 'the abandoned drag drops nothing'); + assert.equal(root.querySelector('[aria-selected="true"]'), null, "the abandoned drag's selection is cleared"); + assert.equal(root.querySelector('.cb-dragging'), null, 'its source piece is no longer shown as dragged'); + win.dispatchEvent('pointerup', { ...centreOf('d2'), pointerId: 71, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('d2', 71)); // the abandoned drag's own trailing click, on another own piece + assert.equal(root.querySelector('[aria-selected="true"]'), null, "the abandoned drag's click selects nothing"); + assert.deepEqual(moves, [], 'the abandoned drag submits nothing, by drop or by click'); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 72, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('g1', 72)); + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'g1', 'the new press works normally'); + root.dispatchEvent('click', atClick('g1', -1)); // deselect again + + // The same mouse pressing again while its previous press is still live (its release was lost). + root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 1, pointerType: 'mouse' }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); + root.dispatchEvent('click', pointerClick('e2', 1, 'mouse')); + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'a re-press does not wait on itself'); }); }); From b1441a47850181736c48e08fa624a2264d9b22cd Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 23:12:48 +0300 Subject: [PATCH 15/30] docs: record the abandoned-gesture fix and its Linux validation --- docs/PROJECT_STATE.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 7a51d897..062b0a17 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: drag cancellation and full Linux validation of the final code._ +_Last updated: 2026-10-03 — M15 Increment 87: abandoned gestures never act, and full Linux validation._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: drag cancellation and full Linux validation of the final code._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: bounded no-pointer-id click fallback and drag release ownership._ @@ -5124,4 +5126,14 @@ Addresses four blocking review findings identified by ChatGPT independent review - on Windows: lint passed. This supersedes the earlier "remain" notes: of the LOW items, only the lack of a real-browser touch test is still open. +- **Exact-head review of `dd52355` and its correction**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS.** Its finding is fixed in `59362b9`, with a RED test first. + - The finding: a press during another pointer's live drag only detached that drag's listeners. Its selection stayed and its release went unwatched, so its trailing click reached `tap` with the piece still selected and **an abandoned drag could submit a move**. `f98a8b9` had removed only the floating piece; the selection gap was older. The "fixes the pre-existing floating piece" wording in the previous bullet was therefore true only for the clone, until this fix. + - The fix: owner changes and new presses now share `abandonGesture()`, which waits for the abandoned pointer's release, swallows that click, and undoes a drag it started (selection cleared, re-rendered). A re-press by the same pointer, whose release was lost, only resets, so its own click still works. + - Six compiled mutations were all killed: a new press only cancelling the drag, no wait, selection kept, no re-render, a same-pointer re-press also abandoning, and the owner change not abandoning. + - Of the earlier "not fixed" notes, the floating piece and the selection left by a second press are now both fixed. +- **Validation of `59362b9`, entirely on Linux** (the exact tree from `git archive 59362b9`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,954 of 3,954** across 19 workspaces, with zero skips (web 1,464); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From d569298a1538359f9919a6cfb6e0df4d4b5b211d Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 23:33:13 +0300 Subject: [PATCH 16/30] fix(web): track abandoned-gesture waits and click suppressions per pointer Independent exact-head review of b1441a4: a single wait slot and a single suppression slot meant a third simultaneous pointer could defeat them (abandoning A replaced the pending wait on C, so C's click still tapped), and setInputEnabled(false) still only cancelled a drag. Waits are now a set of awaited pointer ids served by one window listener pair (attached while any is awaited), and suppressions a map of pointer id to the press count at its release. Each pointer's release, click, re-press and the legacy no-pointer-id fallback work as before, per pointer. setInputEnabled(false) now abandons the gesture like an owner change. The PR #87 test that asserted no pointerup listener after a game ends now asserts the new contract: drag listeners go at once, and only the wait for the abandoned release remains until that release. --- packages/web/src/ui/board-view.ts | 89 ++++++++++++++-------------- packages/web/test/board-a11y.test.ts | 55 ++++++++++++++++- 2 files changed, 97 insertions(+), 47 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index c6258447..155c9fee 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -105,23 +105,20 @@ export class BoardView { private pointerId: number | null = null; private floatEl: HTMLElement | null = null; /** - * The pointer whose next click must be swallowed: one that just released a drag, or a gesture an - * owner change cut short. Null when none. Cleared by that click or by the same pointer pressing again. + * Pointers whose next click must be swallowed (a drag's release, or an abandoned gesture's), each + * with {@link pressCount} at that release. An entry goes with that click or the same pointer pressing again. */ - private suppressClickOf: number | null = null; + private readonly suppressedClicks = new Map(); /** Presses seen on the board, so a click with no pointer id can be tied to the release before it. */ private pressCount = 0; - /** {@link pressCount} when {@link suppressClickOf} was set. */ - private suppressedAtPress = 0; + /** Pointers whose gesture was abandoned and whose release is still awaited. */ + private readonly awaitingRelease = new Set(); private overlay: HTMLElement | null = null; private focusedSquare: Square | null = null; /** Removes the window listeners of the drag in progress; null when no drag is listening. */ private releaseDragListeners: (() => void) | null = null; - /** Stops waiting for the release of a gesture an owner change cut short; null when none is pending. */ - private releaseCancelledGesture: (() => void) | null = null; - /** The pointer whose cut-short gesture is being waited for; null when none. */ - private cancelledGesturePointer: number | null = null; // Held as fields so `destroy` can remove the very same references `addEventListener` received. + private readonly onAwaitedRelease = (e: PointerEvent): void => this.handleAwaitedRelease(e); private readonly onClick = (e: MouseEvent): void => this.handleClick(e); private readonly onPointerDown = (e: PointerEvent): void => this.handlePointerDown(e); private readonly onKeyDown = (e: KeyboardEvent): void => this.handleKeyDown(e); @@ -161,7 +158,7 @@ export class BoardView { */ destroy(): void { this.cancelDrag(); - this.releaseCancelledGesture?.(); + for (const pointerId of [...this.awaitingRelease]) this.stopAwaiting(pointerId); this.closeOverlay(); this.root.removeEventListener('click', this.onClick); this.root.removeEventListener('pointerdown', this.onPointerDown); @@ -196,7 +193,7 @@ export class BoardView { /** Accept or ignore move input; disabling also dismisses an open promotion chooser. */ setInputEnabled(enabled: boolean): void { if (!enabled && this.overlay) this.cancelPromotion(); - if (!enabled) this.cancelDrag(); + if (!enabled) this.abandonGesture(); this.interaction.setInputEnabled(enabled); this.render(); } @@ -237,10 +234,7 @@ export class BoardView { } private handleClick(event: MouseEvent): void { - if (this.isSuppressedClick(event)) { - this.suppressClickOf = null; - return; - } + if (this.consumeSuppressedClick(event)) return; if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; @@ -319,8 +313,8 @@ export class BoardView { const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; // The same pointer pressing again: its earlier release made no click here, and any wait for it is over. - if (event.pointerId === this.suppressClickOf) this.suppressClickOf = null; - if (event.pointerId === this.cancelledGesturePointer) this.releaseCancelledGesture?.(); + this.suppressedClicks.delete(event.pointerId); + this.stopAwaiting(event.pointerId); // A new press ends any gesture still in progress. Another pointer's gesture is abandoned (its // release must not tap); the same pointer pressing again means its release was lost. if (this.releaseDragListeners && this.pointerId !== event.pointerId) this.abandonGesture(); @@ -357,47 +351,50 @@ export class BoardView { } /** - * Wait for the release of the pointer whose gesture an owner change cut short, then swallow that - * pointer's click whenever it arrives. Other pointers' releases are ignored: on an engine whose - * clicks carry no pointer id, ending the wait early would swallow another pointer's click. + * Wait for the release of a pointer whose gesture was abandoned, then swallow that pointer's click + * whenever it arrives. Any number of pointers can be awaited at once; each waits only for itself. */ private awaitCancelledRelease(pointerId: number): void { - this.releaseCancelledGesture?.(); - const end = (e: PointerEvent): void => { - if (e.pointerId !== pointerId) return; - this.releaseCancelledGesture?.(); - this.suppressClickFrom(pointerId); - }; - window.addEventListener('pointerup', end); - window.addEventListener('pointercancel', end); - this.cancelledGesturePointer = pointerId; - this.releaseCancelledGesture = (): void => { - window.removeEventListener('pointerup', end); - window.removeEventListener('pointercancel', end); - this.cancelledGesturePointer = null; - this.releaseCancelledGesture = null; - }; + if (this.awaitingRelease.size === 0) { + window.addEventListener('pointerup', this.onAwaitedRelease); + window.addEventListener('pointercancel', this.onAwaitedRelease); + } + this.awaitingRelease.add(pointerId); + } + + private handleAwaitedRelease(event: PointerEvent): void { + if (!this.awaitingRelease.has(event.pointerId)) return; + this.stopAwaiting(event.pointerId); + this.suppressClickFrom(event.pointerId); + } + + private stopAwaiting(pointerId: number): void { + if (!this.awaitingRelease.delete(pointerId) || this.awaitingRelease.size > 0) return; + window.removeEventListener('pointerup', this.onAwaitedRelease); + window.removeEventListener('pointercancel', this.onAwaitedRelease); } /** Swallow the next click produced by `pointerId`, whenever it arrives. */ private suppressClickFrom(pointerId: number): void { - this.suppressClickOf = pointerId; - this.suppressedAtPress = this.pressCount; + this.suppressedClicks.set(pointerId, this.pressCount); } /** - * Whether `event` is the click the recorded release produced. Modern browsers deliver `click` as a - * PointerEvent carrying the id of the pointer behind it, whenever it arrives, so the id decides. A - * click with no pointer (assistive technology, keyboard, `element.click()`) has `pointerType` '' and - * never counts. On an engine whose clicks carry no pointer fields, a click counts only if no press - * came after the release: a later tap is a new gesture, not that release's click. + * If `event` is a click a recorded release produced, forget that entry and return true. Modern + * browsers deliver `click` as a PointerEvent carrying the id of the pointer behind it, whenever it + * arrives, so the id decides. A click with no pointer (assistive technology, keyboard, + * `element.click()`) has `pointerType` '' and never counts. On an engine whose clicks carry no pointer + * fields, a click counts only if no press came after a recorded release: a later tap is a new gesture. */ - private isSuppressedClick(event: MouseEvent): boolean { - if (this.suppressClickOf === null) return false; + private consumeSuppressedClick(event: MouseEvent): boolean { + if (this.suppressedClicks.size === 0) return false; const click = event as Partial; if (click.pointerType === '') return false; - if (click.pointerId === undefined) return this.pressCount === this.suppressedAtPress; - return click.pointerId === this.suppressClickOf; + if (click.pointerId !== undefined) return this.suppressedClicks.delete(click.pointerId); + for (const [pointerId, atPress] of this.suppressedClicks) { + if (atPress === this.pressCount) return this.suppressedClicks.delete(pointerId); + } + return false; } /** diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 83813779..bb933465 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -826,12 +826,14 @@ test('a game ending mid-drag drops the floating piece and the drag cannot resume assert.equal(root.querySelector('.cb-dragging'), null); assert.equal(win.listenerCount('pointermove'), 0, 'the drag stops listening even if no pointer-up ever comes'); - assert.equal(win.listenerCount('pointerup'), 0); + // The abandoned drag's release is awaited (so its late click cannot act), and nothing else. + assert.equal(win.listenerCount('pointerup'), 1, 'only the wait for the abandoned release remains'); win.dispatchEvent('pointermove', { ...centreOf('e4'), pointerId: 1 }); win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); assert.equal(body.children.length, 0, 'later pointer events do not resume the drag'); assert.deepEqual(moves, []); + assert.equal(win.listenerCount('pointerup') + win.listenerCount('pointercancel'), 0, 'the wait ends with that release'); }); }); @@ -1196,6 +1198,17 @@ test('on an engine whose clicks carry no pointer id, a stale suppression never e win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 42, pointerType: 'touch' }); root.dispatchEvent('click', centreOf('e2')); assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'the new tap selects'); + root.dispatchEvent('click', centreOf('e2')); // deselect again (no press: a pointer-less activation) + + // A drag that wobbles back onto its piece: its own click (no pointer fields) is swallowed, once. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 43, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 43, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('g1'), pointerId: 43, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 43, pointerType: 'touch' }); + root.dispatchEvent('click', centreOf('g1')); + assert.equal(root.querySelector('[aria-selected="true"]'), null, "the drag's own click is swallowed"); + root.dispatchEvent('click', centreOf('e2')); // the next activation, with no press since: not that click + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'only one click is swallowed'); }); }); @@ -1267,6 +1280,46 @@ test('a new press during a live drag ends that drag, floating piece included', ( }); }); +test('every abandoned pointer is waited for, however many fingers are down at once', () => { + withDragGlobals((win) => { + const { root, moves } = mountWithFeedback({ playerColor: 'white' }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + root.dispatchEvent('click', atClick('b1', -1)); // an earlier selection, made before any of this + // C presses (a plain press, not a drag), then A presses (C abandoned), then B presses (A abandoned). + root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 81, pointerType: 'touch' }); // C + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 82, pointerType: 'touch' }); // A + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 83, pointerType: 'touch' }); // B + assert.equal(selected(), 'b1', 'abandoning presses that never dragged leaves an earlier selection alone'); + root.dispatchEvent('click', atClick('b1', -1)); // deselect again + // C releases first: its click must not tap, even though A was abandoned after it. + win.dispatchEvent('pointerup', { ...centreOf('d2'), pointerId: 81, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('d2', 81)); + assert.equal(selected(), null, "the first abandoned press's click does not tap"); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 82, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('e2', 82)); + assert.equal(selected(), null, "the second abandoned press's click does not tap"); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 83, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('g1', 83)); + assert.equal(selected(), 'g1', 'the live press taps normally'); + assert.deepEqual(moves, []); + assert.equal(win.listenerCount('pointerup') + win.listenerCount('pointercancel'), 0, 'nothing is still waited for'); + }); +}); + +test('disabling input mid-drag abandons the drag: its late click does not act if input returns', () => { + withDragGlobals((win) => { + const { root, board, moves } = mountWithFeedback({ playerColor: 'white' }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 91, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('e3'), pointerId: 91, pointerType: 'touch' }); + board.setInputEnabled(false); + board.setInputEnabled(true); + win.dispatchEvent('pointerup', { ...centreOf('d2'), pointerId: 91, pointerType: 'touch' }); + root.dispatchEvent('click', pointerClick('d2', 91)); + assert.equal(root.querySelector('[aria-selected="true"]'), null, "the abandoned drag's click selects nothing"); + assert.deepEqual(moves, []); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 3bd6965700a5fcca21fc2e23f94c8dfe517f754a Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 23:40:25 +0300 Subject: [PATCH 17/30] docs: record per-pointer gesture tracking and its Linux validation --- docs/PROJECT_STATE.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 062b0a17..70c7e726 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: abandoned gestures never act, and full Linux validation._ +_Last updated: 2026-10-03 — M15 Increment 87: per-pointer gesture tracking and full Linux validation._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: abandoned gestures never act, and full Linux validation._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: drag cancellation and full Linux validation of the final code._ @@ -5136,4 +5138,16 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,954 of 3,954** across 19 workspaces, with zero skips (web 1,464); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint passed. +- **Exact-head review of `b1441a4` and its correction**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS**, with no single-gesture path that acts. + - Its LOW finding: one wait slot and one suppression slot meant a third simultaneous pointer could defeat them. Abandoning A replaced the pending wait on an earlier abandoned C, so C's click still tapped. + - Its nits: no test covered a press interrupting another pointer's non-drag gesture, and `setInputEnabled(false)` still only cancelled a drag. + - All three are addressed in `d569298`. Waits are a set of awaited pointer ids, served by one window listener pair that is attached while any pointer is awaited. Suppressions are a map from pointer id to the press count at its release. Each pointer's release, click, re-press and the no-pointer-id fallback behave per pointer. `setInputEnabled(false)` abandons the gesture like an owner change. + - The PR #87 test that asserted no `pointerup` listener after a game ends now asserts the new contract: the drag's listeners go at once, and only the wait for the abandoned release remains until that release. + - RED tests came first (three simultaneous fingers; input disabled and re-enabled mid-drag). + - A full mutation sweep of the click and drag logic: 25 compiled mutations, all killed by tests. Two had survived the first sweep: abandoning a non-drag press clearing an earlier selection, and a swallowed fallback click keeping its entry. They were killed after the missing assertions were added. +- **Validation of `d569298`, entirely on Linux** (the exact tree from `git archive d569298`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,956 of 3,956** across 19 workspaces, with zero skips (web 1,466); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 5f7eb728cb5b5b5487e85bb72f0bf8f656870b7c Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sat, 3 Oct 2026 23:58:26 +0300 Subject: [PATCH 18/30] fix(web): tie a pointer-less click to the last released pointer Greptile on 3bd6965: on an engine whose clicks carry no pointerId, the fallback matched clicks with a press count shared by all fingers. If finger A was abandoned after finger B pressed and A released without a click, A's suppression was recorded at B's press count and B's genuine tap was swallowed. A click without a pointer id is now taken to be the last released pointer's (a browser dispatches a click straight after its own pointer's release) and is swallowed only if that pointer is suppressed; the shared press counter is gone. An awaited pointer's pointercancel now ends its wait without recording a suppression, since a cancelled pointer never clicks (the optional LOW from the independent review of 3bd6965). Engines with pointer ids are unchanged. --- packages/web/src/ui/board-view.ts | 38 +++++++++++++++------------- packages/web/test/board-a11y.test.ts | 24 ++++++++++++++++++ 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 155c9fee..bdc12598 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -105,12 +105,15 @@ export class BoardView { private pointerId: number | null = null; private floatEl: HTMLElement | null = null; /** - * Pointers whose next click must be swallowed (a drag's release, or an abandoned gesture's), each - * with {@link pressCount} at that release. An entry goes with that click or the same pointer pressing again. + * Pointers whose next click must be swallowed (a drag's release, or an abandoned gesture's). An + * entry goes with that click or with the same pointer pressing again. */ - private readonly suppressedClicks = new Map(); - /** Presses seen on the board, so a click with no pointer id can be tied to the release before it. */ - private pressCount = 0; + private readonly suppressedClicks = new Set(); + /** + * The pointer released most recently, so a click with no pointer id can be tied to the release that + * produced it: a browser dispatches a click straight after its own pointer's release. + */ + private lastReleasedPointer: number | null = null; /** Pointers whose gesture was abandoned and whose release is still awaited. */ private readonly awaitingRelease = new Set(); private overlay: HTMLElement | null = null; @@ -118,7 +121,8 @@ export class BoardView { /** Removes the window listeners of the drag in progress; null when no drag is listening. */ private releaseDragListeners: (() => void) | null = null; // Held as fields so `destroy` can remove the very same references `addEventListener` received. - private readonly onAwaitedRelease = (e: PointerEvent): void => this.handleAwaitedRelease(e); + private readonly onAwaitedRelease = (e: PointerEvent): void => this.handleAwaitedRelease(e, true); + private readonly onAwaitedCancel = (e: PointerEvent): void => this.handleAwaitedRelease(e, false); private readonly onClick = (e: MouseEvent): void => this.handleClick(e); private readonly onPointerDown = (e: PointerEvent): void => this.handlePointerDown(e); private readonly onKeyDown = (e: KeyboardEvent): void => this.handleKeyDown(e); @@ -308,7 +312,6 @@ export class BoardView { } private handlePointerDown(event: PointerEvent): void { - this.pressCount += 1; if (this.overlay) return; const sq = this.squareAt(event.clientX, event.clientY); if (!sq) return; @@ -327,6 +330,7 @@ export class BoardView { const move = (e: PointerEvent): void => this.handlePointerMove(e); const up = (e: PointerEvent): void => { if (e.pointerId !== this.pointerId) return; // another pointer's release is not this drag's + this.lastReleasedPointer = e.pointerId; this.releaseDragListeners?.(); this.handlePointerUp(e); }; @@ -357,26 +361,29 @@ export class BoardView { private awaitCancelledRelease(pointerId: number): void { if (this.awaitingRelease.size === 0) { window.addEventListener('pointerup', this.onAwaitedRelease); - window.addEventListener('pointercancel', this.onAwaitedRelease); + window.addEventListener('pointercancel', this.onAwaitedCancel); } this.awaitingRelease.add(pointerId); } - private handleAwaitedRelease(event: PointerEvent): void { + private handleAwaitedRelease(event: PointerEvent, released: boolean): void { if (!this.awaitingRelease.has(event.pointerId)) return; this.stopAwaiting(event.pointerId); + // A cancelled pointer never clicks, so only a real release leaves a click to swallow. + if (!released) return; + this.lastReleasedPointer = event.pointerId; this.suppressClickFrom(event.pointerId); } private stopAwaiting(pointerId: number): void { if (!this.awaitingRelease.delete(pointerId) || this.awaitingRelease.size > 0) return; window.removeEventListener('pointerup', this.onAwaitedRelease); - window.removeEventListener('pointercancel', this.onAwaitedRelease); + window.removeEventListener('pointercancel', this.onAwaitedCancel); } /** Swallow the next click produced by `pointerId`, whenever it arrives. */ private suppressClickFrom(pointerId: number): void { - this.suppressedClicks.set(pointerId, this.pressCount); + this.suppressedClicks.add(pointerId); } /** @@ -384,17 +391,14 @@ export class BoardView { * browsers deliver `click` as a PointerEvent carrying the id of the pointer behind it, whenever it * arrives, so the id decides. A click with no pointer (assistive technology, keyboard, * `element.click()`) has `pointerType` '' and never counts. On an engine whose clicks carry no pointer - * fields, a click counts only if no press came after a recorded release: a later tap is a new gesture. + * fields, the click is taken to be the last released pointer's, which is swallowed only if suppressed. */ private consumeSuppressedClick(event: MouseEvent): boolean { if (this.suppressedClicks.size === 0) return false; const click = event as Partial; if (click.pointerType === '') return false; - if (click.pointerId !== undefined) return this.suppressedClicks.delete(click.pointerId); - for (const [pointerId, atPress] of this.suppressedClicks) { - if (atPress === this.pressCount) return this.suppressedClicks.delete(pointerId); - } - return false; + const pointerId = click.pointerId ?? this.lastReleasedPointer; + return pointerId !== null && this.suppressedClicks.delete(pointerId); } /** diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index bb933465..a59f9d0c 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1320,6 +1320,30 @@ test('disabling input mid-drag abandons the drag: its late click does not act if }); }); +test("on an engine whose clicks carry no pointer id, an abandoned finger's silent release never eats another finger's tap", () => { + withDragGlobals((win) => { + const { root } = mountWithFeedback({ playerColor: 'white' }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 101, pointerType: 'touch' }); // A + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 102, pointerType: 'touch' }); // B abandons A + win.dispatchEvent('pointerup', { clientX: 900, clientY: 900, pointerId: 101, pointerType: 'touch' }); // A: no click + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 102, pointerType: 'touch' }); + root.dispatchEvent('click', centreOf('g1')); // B's click, with no pointer fields + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'g1', "B's tap selects"); + }); +}); + +test('an abandoned pointer that the browser cancels leaves nothing to swallow', () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: null }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 111, pointerType: 'touch' }); + board.setPlayerColor('white'); // abandons the press + win.dispatchEvent('pointercancel', { pointerId: 111, pointerType: 'touch' }); // no click will follow + assert.equal(win.listenerCount('pointerup') + win.listenerCount('pointercancel'), 0, 'the wait is over'); + root.dispatchEvent('click', centreOf('e2')); // a later activation with no pointer fields and no press + assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'it is not swallowed'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 8314d11338e0b5e7bcc3ff8a81d1132305e7f43b Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 00:46:18 +0300 Subject: [PATCH 19/30] docs: record the last-release click fallback and its Linux validation --- docs/PROJECT_STATE.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 70c7e726..918feb41 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-03 — M15 Increment 87: per-pointer gesture tracking and full Linux validation._ +_Last updated: 2026-10-04 — M15 Increment 87: pointer-less clicks tied to the last release, and full Linux validation._ + +Prior: _Last updated: 2026-10-03 — M15 Increment 87: per-pointer gesture tracking and full Linux validation._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: abandoned gestures never act, and full Linux validation._ @@ -5150,4 +5152,15 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,956 of 3,956** across 19 workspaces, with zero skips (web 1,466); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint passed. +- **Exact-head review of `3bd6965` and the gates on it**: + - Independent review: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran it. Verdict **APPROVE**, with one optional LOW: an awaited pointer's `pointercancel` recorded a suppression even though a cancelled pointer never clicks. + - Pushed head `3bd6965`: CI was all green, including M6 acceptance. Qodo reported 0 bugs, 0 rule violations and 0 requirement gaps, with all earlier findings resolved. + - Greptile, confidence 5/5, raised one non-blocking finding. On an engine whose clicks carry no `pointerId`, the fallback's press count was shared by all fingers. If finger A was abandoned after finger B pressed and A released without a click, A's suppression was recorded at B's count, and B's genuine tap was swallowed. + - Both are fixed in `5f7eb72`, with RED tests first. A click without a pointer id is now taken to be the last released pointer's (a browser dispatches a click straight after its own pointer's release) and is swallowed only if that pointer is suppressed. The shared press counter is gone. An awaited pointer's `pointercancel`, on its own handler, ends the wait without recording a suppression. Engines with pointer ids are unchanged. + - Full mutation sweep of the click and drag logic, updated to the new code: 27 compiled mutations, all killed by tests. +- **Validation of `5f7eb72`, entirely on Linux** (the exact tree from `git archive 5f7eb72`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,958 of 3,958** across 19 workspaces, with zero skips (web 1,468); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From b6246cd9d5f068447159f3abea4220b7d93daf4a Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 01:28:02 +0300 Subject: [PATCH 20/30] fix(web): bound click records and put abandoned clicks first on engines without pointer ids Qodo on 8314d11: off-board drops (and, earlier, cancelled pointers) recorded click suppressions that no click could ever consume, and touch pointer ids are never reused, so records grew for the life of the board. Releases off the board now record nothing, and pending records are capped (oldest evicted), which also bounds on-board touch drags that make no click. Greptile on 8314d11: on an engine whose clicks carry no pointerId, finger A's abandoned gesture could produce a delayed click after finger B's release, and the last-released rule attributed it to B, so it acted. Without a pointer id a click cannot be attributed, so a choice is unavoidable: safety first. While an abandoned gesture's click is pending, a pointer-less click is taken to be that one (an abandoned gesture never acts; at worst one genuine tap is swallowed and repeated). Drag-release suppressions keep the last-released rule. Engines with pointer ids are unchanged. Tests now create stale entries with on-board no-click releases so every protection stays reachable; a redundant re-add line and a dead write are removed. --- packages/web/src/ui/board-view.ts | 44 +++++++++++++++++++------- packages/web/test/board-a11y.test.ts | 47 +++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index bdc12598..2e8febab 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -60,6 +60,13 @@ function pieceClass(color: string, role: string): string { } const DRAG_THRESHOLD = 6; +/** + * Most click suppressions kept at once. A release's own click arrives within a few events, but a touch + * drag usually makes no click at all and touch pointer ids are never reused, so without a cap its entry + * would stay for the life of the board. The oldest entries are evicted first. + */ +const MAX_PENDING_CLICKS = 16; + /** A resolved user gesture: either a committed move or a queued premove. */ export type ResolvedMove = | { readonly kind: 'move'; readonly move: Premove } @@ -105,10 +112,11 @@ export class BoardView { private pointerId: number | null = null; private floatEl: HTMLElement | null = null; /** - * Pointers whose next click must be swallowed (a drag's release, or an abandoned gesture's). An - * entry goes with that click or with the same pointer pressing again. + * Pointers whose next click must be swallowed, each marked `true` if it is an abandoned gesture's + * (begun under a previous owner, or cut short by another pointer) and `false` for a drag's own + * release. An entry goes with that click or with the same pointer pressing again. */ - private readonly suppressedClicks = new Set(); + private readonly suppressedClicks = new Map(); /** * The pointer released most recently, so a click with no pointer id can be tied to the release that * produced it: a browser dispatches a click straight after its own pointer's release. @@ -371,8 +379,8 @@ export class BoardView { this.stopAwaiting(event.pointerId); // A cancelled pointer never clicks, so only a real release leaves a click to swallow. if (!released) return; - this.lastReleasedPointer = event.pointerId; - this.suppressClickFrom(event.pointerId); + // A release off the board makes no click here, so there is nothing to swallow. + if (this.squareAt(event.clientX, event.clientY)) this.suppressClickFrom(event.pointerId, true); } private stopAwaiting(pointerId: number): void { @@ -382,23 +390,35 @@ export class BoardView { } /** Swallow the next click produced by `pointerId`, whenever it arrives. */ - private suppressClickFrom(pointerId: number): void { - this.suppressedClicks.add(pointerId); + private suppressClickFrom(pointerId: number, abandoned: boolean): void { + this.suppressedClicks.set(pointerId, abandoned); + for (const oldest of this.suppressedClicks.keys()) { + if (this.suppressedClicks.size <= MAX_PENDING_CLICKS) break; + this.suppressedClicks.delete(oldest); + } } /** * If `event` is a click a recorded release produced, forget that entry and return true. Modern * browsers deliver `click` as a PointerEvent carrying the id of the pointer behind it, whenever it * arrives, so the id decides. A click with no pointer (assistive technology, keyboard, - * `element.click()`) has `pointerType` '' and never counts. On an engine whose clicks carry no pointer - * fields, the click is taken to be the last released pointer's, which is swallowed only if suppressed. + * `element.click()`) has `pointerType` '' and never counts. + * + * On an engine whose clicks carry no pointer fields, a click cannot be attributed to a pointer, so a + * choice is unavoidable. Safety first: while an abandoned gesture's click is pending, the click is + * taken to be that one, because an abandoned gesture must never act; at worst one genuine tap is + * swallowed and is simply repeated. Otherwise it is taken to be the last released pointer's. */ private consumeSuppressedClick(event: MouseEvent): boolean { if (this.suppressedClicks.size === 0) return false; const click = event as Partial; if (click.pointerType === '') return false; - const pointerId = click.pointerId ?? this.lastReleasedPointer; - return pointerId !== null && this.suppressedClicks.delete(pointerId); + if (click.pointerId !== undefined) return this.suppressedClicks.delete(click.pointerId); + for (const [pointerId, abandoned] of this.suppressedClicks) { + if (abandoned) return this.suppressedClicks.delete(pointerId); + } + const last = this.lastReleasedPointer; + return last !== null && this.suppressedClicks.delete(last); } /** @@ -452,8 +472,8 @@ export class BoardView { this.endFloat(); this.dragging = false; this.dragFrom = null; - this.suppressClickFrom(event.pointerId); if (target) { + this.suppressClickFrom(event.pointerId, false); // a drop off the board makes no click here this.dispatch(this.interaction.drop(from, target)); } else { this.interaction.cancelPromotion(); diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index a59f9d0c..f941170d 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1093,10 +1093,10 @@ test('a click with no pointer gesture, such as from assistive technology, is nev const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; const offBoard = { clientX: 900, clientY: 900 }; - // A gesture cut short by an owner change, released off the board: no click follows it. + // A gesture cut short by an owner change, released on the board, whose click never arrives. root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1 }); board.setPlayerColor('white'); - win.dispatchEvent('pointerup', { ...offBoard, pointerId: 1 }); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); await nextTask(); root.dispatchEvent('click', atClick('e2')); // an activation with no pointer behind it, and no pointer id assert.equal(selected(), 'e2', 'the next click is not eaten by the cut-short gesture'); @@ -1111,10 +1111,10 @@ test('a click with no pointer gesture, such as from assistive technology, is nev assert.equal(selected(), 'e2', 'the next click is not eaten by an off-board drop'); root.dispatchEvent('click', atClick('e2')); // deselect again - // A mouse drag dropped off the board, then an ordinary click by that same mouse. + // A mouse drag dropped on the board whose click is lost, then an ordinary click by that same mouse. root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 1, pointerType: 'mouse' }); win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 1, pointerType: 'mouse' }); - win.dispatchEvent('pointerup', { ...offBoard, pointerId: 1, pointerType: 'mouse' }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 1, pointerType: 'mouse' }); root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 1, pointerType: 'mouse' }); root.dispatchEvent('click', pointerClick('e2', 1, 'mouse')); @@ -1189,10 +1189,10 @@ test('a second finger releasing first does not end the wait for the gesture an o test('on an engine whose clicks carry no pointer id, a stale suppression never eats a later tap', () => { withDragGlobals((win) => { const { root } = mountWithFeedback({ playerColor: 'white' }); - // A touch drag released off the board: real engines make no click for it. + // A touch drag released on the board: a touch drag makes no click, so its entry stays. root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 41, pointerType: 'touch' }); win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 41, pointerType: 'touch' }); - win.dispatchEvent('pointerup', { clientX: 900, clientY: 900, pointerId: 41, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 41, pointerType: 'touch' }); // A new tap by a new touch pointer, whose click arrives as a plain MouseEvent (no pointer fields). root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 42, pointerType: 'touch' }); win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 42, pointerType: 'touch' }); @@ -1337,13 +1337,46 @@ test('an abandoned pointer that the browser cancels leaves nothing to swallow', const { root, board } = mountWithFeedback({ playerColor: null }); root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 111, pointerType: 'touch' }); board.setPlayerColor('white'); // abandons the press - win.dispatchEvent('pointercancel', { pointerId: 111, pointerType: 'touch' }); // no click will follow + win.dispatchEvent('pointercancel', { ...centreOf('e2'), pointerId: 111, pointerType: 'touch' }); // no click will follow assert.equal(win.listenerCount('pointerup') + win.listenerCount('pointercancel'), 0, 'the wait is over'); root.dispatchEvent('click', centreOf('e2')); // a later activation with no pointer fields and no press assert.equal(root.querySelector('[aria-selected="true"]')?.getAttribute('data-square'), 'e2', 'it is not swallowed'); }); }); +test('drags whose release makes no click leave a bounded number of click records, none for off-board drops', () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: 'white' }); + const pending = (): number => (board.view as unknown as { suppressedClicks: Set }).suppressedClicks.size; + const drag = (id: number, end: { clientX: number; clientY: number }): void => { + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: id, pointerType: 'touch' }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: id, pointerType: 'touch' }); + win.dispatchEvent('pointerup', { ...end, pointerId: id, pointerType: 'touch' }); + }; + for (let id = 200; id < 230; id++) drag(id, { clientX: 900, clientY: 900 }); // off the board: no click can come + assert.equal(pending(), 0, 'an off-board drop records nothing'); + for (let id = 300; id < 400; id++) drag(id, centreOf('g1')); // on the board, but a touch drag makes no click + assert.ok(pending() <= 16, `records stay bounded (${pending()})`); + root.dispatchEvent('click', pointerClick('g1', 399)); // the most recent release's click is still swallowed + assert.equal(root.querySelector('[aria-selected="true"]'), null); + }); +}); + +test("on an engine whose clicks carry no pointer id, an abandoned finger's delayed click never acts", () => { + withDragGlobals((win) => { + const { root } = mountWithFeedback({ playerColor: 'white' }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 121, pointerType: 'touch' }); // A + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 122, pointerType: 'touch' }); // B abandons A + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 121, pointerType: 'touch' }); // A releases + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 122, pointerType: 'touch' }); // B releases + root.dispatchEvent('click', centreOf('e2')); // A's delayed click, after B's release, with no pointer fields + assert.equal(selected(), null, "the abandoned finger's click selects nothing"); + root.dispatchEvent('click', centreOf('g1')); // B's own click + assert.equal(selected(), 'g1', "B's tap then works"); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 39c2f9d781501f6129cbe7b3156ea3fd8c4cc663 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 01:35:28 +0300 Subject: [PATCH 21/30] docs: record bounded click records and the safety-first pointer-less click rule --- docs/PROJECT_STATE.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 918feb41..6d8fbd2c 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-04 — M15 Increment 87: pointer-less clicks tied to the last release, and full Linux validation._ +_Last updated: 2026-10-04 — M15 Increment 87: bounded click records, safety-first pointer-less clicks, full Linux validation._ + +Prior: _Last updated: 2026-10-04 — M15 Increment 87: pointer-less clicks tied to the last release, and full Linux validation._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: per-pointer gesture tracking and full Linux validation._ @@ -5163,4 +5165,16 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,958 of 3,958** across 19 workspaces, with zero skips (web 1,468); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint passed. +- **Gates on `8314d11` and their correction**: the independent review gave **APPROVE**. CI was all green, including M6 acceptance. Two non-blocking findings came in; both are fixed in `b6246cd`, with RED tests first. + - **Qodo, 1 bug (performance):** off-board drops recorded click suppressions that no click could consume. Its cancelled-pointer half was already fixed in `5f7eb72`. Touch pointer ids are never reused, so records grew for the life of the board, and an on-board touch drag, which usually makes no click, grows them the same way. Releases off the board now record nothing, and pending records are capped at 16, oldest evicted. + - **Greptile, confidence 5/5:** on an engine whose clicks carry no `pointerId`, finger A's abandoned gesture could produce a delayed click after finger B's release. The last-released rule attributed it to B, so it acted. + - Without a pointer id a click cannot be attributed to a finger, so every rule fails in one direction. The press counter swallowed a genuine tap (Greptile on `3bd6965`); "last released" let an abandoned click act (Greptile on `8314d11`). The choice made is **safety first**: while an abandoned gesture's click is pending, a pointer-less click is taken to be that one. An abandoned gesture never acts; at worst one genuine tap is swallowed and repeated. Drag-release suppressions keep the last-released rule, and engines with pointer ids are unchanged. + - Because off-board releases now record nothing, older tests that made stale entries with off-board releases no longer reached their protections. A mutation sweep showed this as 7 survivors. Those tests now use on-board releases with no click, and the cancel test's `pointercancel` carries coordinates on the board. + - Two lines became provably redundant (a re-add and a last-released write on awaited releases) and were removed instead of being kept untested. + - The final sweep of the click and drag logic: 32 compiled mutations, all killed by tests. +- **Validation of `b6246cd`, entirely on Linux** (the exact tree from `git archive b6246cd`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,960 of 3,960** across 19 workspaces, with zero skips (web 1,470); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 13a05c905cc321578f139c75fab155c47521c45c Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 01:46:40 +0300 Subject: [PATCH 22/30] fix(web): only recent releases can claim a pointer-less click Independent exact-head review of 39c2f9d: on an engine whose clicks carry no pointerId (reportedly including Safari/iOS, unverified), an abandoned touch that never produced a click left an abandoned record with no expiry, so each later genuine tap was swallowed against one stale record, up to the cap: dead taps spread over any amount of time. Records now carry their release's time, and on the pointer-less path records older than CLICK_WINDOW_MS (1 s: a release's click comes within milliseconds, or ~300 ms when touch holds it for double-tap detection) are dropped before matching. Times come from the events' own timeStamp. Engines with pointer ids are unchanged. --- packages/web/src/ui/board-view.ts | 43 +++++++++++++++++++--------- packages/web/test/board-a11y.test.ts | 20 ++++++++++++- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 2e8febab..f6e8d62d 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -67,6 +67,18 @@ const DRAG_THRESHOLD = 6; */ const MAX_PENDING_CLICKS = 16; +/** + * How long after a release its click can still arrive. A browser sends a release's click within a few + * milliseconds for mouse and pen; for touch it can hold the click back for double-tap detection (around + * 300 ms). A click later than this is a new activation, never that release's. + */ +const CLICK_WINDOW_MS = 1000; + +/** An event's own time, or now for an event without one. */ +function eventTime(event: Event): number { + return typeof event.timeStamp === 'number' ? event.timeStamp : performance.now(); +} + /** A resolved user gesture: either a committed move or a queued premove. */ export type ResolvedMove = | { readonly kind: 'move'; readonly move: Premove } @@ -112,11 +124,11 @@ export class BoardView { private pointerId: number | null = null; private floatEl: HTMLElement | null = null; /** - * Pointers whose next click must be swallowed, each marked `true` if it is an abandoned gesture's - * (begun under a previous owner, or cut short by another pointer) and `false` for a drag's own - * release. An entry goes with that click or with the same pointer pressing again. + * Pointers whose next click must be swallowed: `abandoned` is true for an abandoned gesture's release + * (begun under a previous owner, or cut short by another pointer) and false for a drag's own release; + * `at` is the release's time. An entry goes with that click or with the same pointer pressing again. */ - private readonly suppressedClicks = new Map(); + private readonly suppressedClicks = new Map(); /** * The pointer released most recently, so a click with no pointer id can be tied to the release that * produced it: a browser dispatches a click straight after its own pointer's release. @@ -380,7 +392,7 @@ export class BoardView { // A cancelled pointer never clicks, so only a real release leaves a click to swallow. if (!released) return; // A release off the board makes no click here, so there is nothing to swallow. - if (this.squareAt(event.clientX, event.clientY)) this.suppressClickFrom(event.pointerId, true); + if (this.squareAt(event.clientX, event.clientY)) this.suppressClickFrom(event, true); } private stopAwaiting(pointerId: number): void { @@ -390,8 +402,8 @@ export class BoardView { } /** Swallow the next click produced by `pointerId`, whenever it arrives. */ - private suppressClickFrom(pointerId: number, abandoned: boolean): void { - this.suppressedClicks.set(pointerId, abandoned); + private suppressClickFrom(release: PointerEvent, abandoned: boolean): void { + this.suppressedClicks.set(release.pointerId, { abandoned, at: eventTime(release) }); for (const oldest of this.suppressedClicks.keys()) { if (this.suppressedClicks.size <= MAX_PENDING_CLICKS) break; this.suppressedClicks.delete(oldest); @@ -405,17 +417,22 @@ export class BoardView { * `element.click()`) has `pointerType` '' and never counts. * * On an engine whose clicks carry no pointer fields, a click cannot be attributed to a pointer, so a - * choice is unavoidable. Safety first: while an abandoned gesture's click is pending, the click is - * taken to be that one, because an abandoned gesture must never act; at worst one genuine tap is - * swallowed and is simply repeated. Otherwise it is taken to be the last released pointer's. + * choice is unavoidable. Only releases within {@link CLICK_WINDOW_MS} can be its own; older records + * are dropped. Safety first: while an abandoned gesture's click is still due, the click is taken to + * be that one, because an abandoned gesture must never act; at worst a genuine tap within that window + * is swallowed and simply repeated. Otherwise it is taken to be the last released pointer's. */ private consumeSuppressedClick(event: MouseEvent): boolean { if (this.suppressedClicks.size === 0) return false; const click = event as Partial; if (click.pointerType === '') return false; if (click.pointerId !== undefined) return this.suppressedClicks.delete(click.pointerId); - for (const [pointerId, abandoned] of this.suppressedClicks) { - if (abandoned) return this.suppressedClicks.delete(pointerId); + const now = eventTime(event); + for (const [pointerId, record] of this.suppressedClicks) { + if (now - record.at > CLICK_WINDOW_MS) this.suppressedClicks.delete(pointerId); + } + for (const [pointerId, record] of this.suppressedClicks) { + if (record.abandoned) return this.suppressedClicks.delete(pointerId); } const last = this.lastReleasedPointer; return last !== null && this.suppressedClicks.delete(last); @@ -473,7 +490,7 @@ export class BoardView { this.dragging = false; this.dragFrom = null; if (target) { - this.suppressClickFrom(event.pointerId, false); // a drop off the board makes no click here + this.suppressClickFrom(event, false); // a drop off the board makes no click here this.dispatch(this.interaction.drop(from, target)); } else { this.interaction.cancelPromotion(); diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index f941170d..c97ec180 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1347,7 +1347,7 @@ test('an abandoned pointer that the browser cancels leaves nothing to swallow', test('drags whose release makes no click leave a bounded number of click records, none for off-board drops', () => { withDragGlobals((win) => { const { root, board } = mountWithFeedback({ playerColor: 'white' }); - const pending = (): number => (board.view as unknown as { suppressedClicks: Set }).suppressedClicks.size; + const pending = (): number => (board.view as unknown as { suppressedClicks: Map }).suppressedClicks.size; const drag = (id: number, end: { clientX: number; clientY: number }): void => { root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: id, pointerType: 'touch' }); win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: id, pointerType: 'touch' }); @@ -1377,6 +1377,24 @@ test("on an engine whose clicks carry no pointer id, an abandoned finger's delay }); }); +test('on an engine whose clicks carry no pointer id, old abandoned releases never turn later taps into dead taps', () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: null }); + const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; + // Two touches cut short by owner changes, released on the board, whose clicks never come (pans). + for (const [id, owner] of [[131, 'white'], [132, null], [133, 'white']] as const) { + if (id !== 133) root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: id, pointerType: 'touch', timeStamp: 1000 }); + board.setPlayerColor(owner); + if (id !== 133) win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: id, pointerType: 'touch', timeStamp: 1000 }); + } + // Two seconds later, genuine taps whose clicks arrive as plain MouseEvents. + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 3000 }); + assert.equal(selected(), 'e2', 'the first later tap acts'); + root.dispatchEvent('click', { ...centreOf('d2'), timeStamp: 3100 }); + assert.equal(selected(), 'd2', 'and so does the next one'); + }); +}); + test('a change of owner closes an open promotion chooser and clears a queued premove', () => { const fen = '4k3/4P3/8/8/8/8/8/4K3 w - - 0 1'; const root = new FakeBoardRoot(); From 242ad1cee8cf1a9d5c889c933c871974c68b42db Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 01:53:13 +0300 Subject: [PATCH 23/30] docs: record time-bounded pointer-less click matching and its Linux validation --- docs/PROJECT_STATE.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 6d8fbd2c..4b9cfc33 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-04 — M15 Increment 87: bounded click records, safety-first pointer-less clicks, full Linux validation._ +_Last updated: 2026-10-04 — M15 Increment 87: time-bounded pointer-less click matching and full Linux validation._ + +Prior: _Last updated: 2026-10-04 — M15 Increment 87: bounded click records, safety-first pointer-less clicks, full Linux validation._ Prior: _Last updated: 2026-10-04 — M15 Increment 87: pointer-less clicks tied to the last release, and full Linux validation._ @@ -5177,4 +5179,16 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,960 of 3,960** across 19 workspaces, with zero skips (web 1,470); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint passed. +- **Exact-head review of `39c2f9d` and its correction**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS**, with no ownership regression and no case where an abandoned gesture's click acts. + - Its finding: the previous bullet's "at worst one genuine tap is swallowed" was an over-claim. Abandoned records had no expiry. On an engine whose clicks carry no `pointerId`, an abandoned touch that never made a click left a record that neither a re-press nor a matching click could remove, because touch ids are never reused. Each later genuine tap was then swallowed against one stale record, up to the cap of 16: dead taps spread over any amount of time. + - The reviewer believes Safari and iOS still send `click` without a `pointerId`. That is not verified, but if it holds, this fallback is the main iOS touch path. + - The fix (`13a05c9`): records carry their release's time, and on the pointer-less path, records older than `CLICK_WINDOW_MS` (1 s) are dropped before matching. A release's click arrives within milliseconds, or about 300 ms when touch holds it back for double-tap detection. Times come from the events' own `timeStamp`. + - The accurate cost of the safety-first rule is therefore: at most one genuine tap per abandoned release, and only within about a second of that release. Engines with pointer ids are unchanged. + - A RED test came first: two stale abandoned releases, then genuine taps two seconds later, which must act. + - Full mutation sweep: 35 compiled mutations, all killed by tests, including no purge, an unbounded window and a zero window. +- **Validation of `13a05c9`, entirely on Linux** (the exact tree from `git archive 13a05c9`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; + - on Windows: lint passed. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 2b52090131948929f67ec5b133428ba79089290f Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 02:01:21 +0300 Subject: [PATCH 24/30] test(web): pin the pointer-less click window from both sides Independent exact-head review of 242ad1c: only one test used explicit event times, so shrinking CLICK_WINDOW_MS (e.g. to 50 ms) or flipping its comparison went unnoticed. An abandoned release's click 300 ms later must still be swallowed, and a click just past the window must act. --- packages/web/test/board-a11y.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index c97ec180..98ab7ecc 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1392,6 +1392,23 @@ test('on an engine whose clicks carry no pointer id, old abandoned releases neve assert.equal(selected(), 'e2', 'the first later tap acts'); root.dispatchEvent('click', { ...centreOf('d2'), timeStamp: 3100 }); assert.equal(selected(), 'd2', 'and so does the next one'); + root.dispatchEvent('click', atClick('d2', -1)); // deselect again + + // Inside the window, an abandoned release's late click is still swallowed (a touch click can be held back). + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 134, pointerType: 'touch', timeStamp: 5000 }); + board.setPlayerColor(null); + board.setPlayerColor('white'); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 134, pointerType: 'touch', timeStamp: 5000 }); + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 5300 }); + assert.equal(selected(), null, 'a click 300 ms after its abandoned release is swallowed'); + + // Just past the window, a click is a new activation. + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 135, pointerType: 'touch', timeStamp: 7000 }); + board.setPlayerColor(null); + board.setPlayerColor('white'); + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 135, pointerType: 'touch', timeStamp: 7000 }); + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 8001 }); + assert.equal(selected(), 'e2', 'a click more than the window later acts'); }); }); From 6f496209e58356d73e852b57cbaf3a5bb3d1f962 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 02:07:49 +0300 Subject: [PATCH 25/30] docs: record the pinned click-window tests and their Linux validation --- docs/PROJECT_STATE.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 4b9cfc33..d63dc496 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,7 +6,9 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-04 — M15 Increment 87: time-bounded pointer-less click matching and full Linux validation._ +_Last updated: 2026-10-04 — M15 Increment 87: click-window tests pinned and full Linux validation._ + +Prior: _Last updated: 2026-10-04 — M15 Increment 87: time-bounded pointer-less click matching and full Linux validation._ Prior: _Last updated: 2026-10-04 — M15 Increment 87: bounded click records, safety-first pointer-less clicks, full Linux validation._ @@ -5191,4 +5193,12 @@ Addresses four blocking review findings identified by ChatGPT independent review - hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`; - on Windows: lint passed. +- **Exact-head review of `242ad1c` and its follow-up**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE WITH NITS**, with no code defect. + - Test gap: only one test used explicit event times, so a regression that shrank `CLICK_WINDOW_MS` (to 50 ms, say) or flipped its comparison would have gone unnoticed and let a late abandoned click act. In `2b52090` that test now requires an abandoned release's click 300 ms later to be swallowed, and a click just past the window to act. This is a test change only; the source is identical to `13a05c9`. + - Mutation sweep: 37 compiled mutations, all killed by tests, including the shrunken window and the flipped comparison. + - Wording: the previous bullet's timing ("within milliseconds, or about 300 ms when touch holds it back for double-tap detection") is the design assumption behind the 1 s window, not verified browser behaviour. Modern mobile browsers with a viewport meta tag may not delay clicks at all, and the window is generous either way. Whether Safari/iOS click events carry a `pointerId` also remains unverified. +- **Validation of `2b52090`, entirely on Linux** (the exact tree from `git archive 2b52090`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): + - `npm ci` and build succeeded; + - hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); + - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 431df304697bfba3c153ef2b27fbdb376a30c9f9 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 02:23:36 +0300 Subject: [PATCH 26/30] docs: record the merge with #89 and its Linux validation --- docs/PROJECT_STATE.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index e1038638..6a3c84e6 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -5219,4 +5219,9 @@ Addresses four blocking review findings identified by ChatGPT independent review - `npm ci` and build succeeded; - hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); - static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 flaky with `--retries=0`. +- **Merged with `origin/main` `abab2bc` (#89, Increment 87) in `2e878c5`** (normal merge, parents `6f49620` and `abab2bc`; owner-approved). #89 had claimed Increment 87, so this entry and its header lines were renumbered to Increment 88. The only conflict was this file: #89's Increment 87 entry and header lines are kept unchanged, main's previous "Last updated" line became a "Prior:" line, and nothing else from main was altered. No source, test or config file conflicted. #89's topology and harness changes (`scripts/check-test-topology.mjs`, `scripts/run-gateway-tests.mjs`, the new `scripts/test/gateway-signal-route.test.mjs`, CI, `Dockerfile.gateway-test`, gateway test) merged cleanly beside this PR's Playwright spec count of 32. +- **Validation of the merge `2e878c5`**: + - on Windows: `npm ci`, build and lint passed; all eight `check:*` guards passed, including `check:test-topology` (479 test files across 21 suites, all placed, classified and reachable); `test:scripts` **315 of 315**, with zero skips (up from 312 with #89's new script test), including the topology tests "backend-free Playwright discovers only the eleven offline specs" and the 32-spec discovery count; + - entirely on Linux (the exact tree from `git archive 2e878c5`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): `npm ci` and build succeeded; hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 failed, 0 skipped and 0 flaky with `--retries=0`; no config, timeout, worker or test changes. +- **Exact-head review of `2e878c5`**: Gemini (429, resets in 87 h) and Sonnet (429, resets in 153 h) were still quota-blocked, so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE**, with no critical, high or medium finding. It covered Qodo's off-board release / accumulated click-record finding (resolved), Greptile's delayed abandoned-click finding (resolved, with the documented no-pointer-id trade-off), per-pointer `pointerId` handling, assistive-technology and pointer-less clicks, multi-touch and interleaved pointers, #89's merged topology and harness changes (no semantic conflict), and this file's merge resolution. Low notes, left as they are: the no-pointer-id fail-safe can swallow one genuine tap within 1 s of an abandoned release; an ignored press (off the board or with an overlay open) does not clear that pointer's stale record, which is bounded and time-limited; `lastReleasedPointer` also updates on ordinary taps, which is harmless. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From fcb91b99af48a43e0d83e343a2a9d9fe16280e49 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 02:45:19 +0300 Subject: [PATCH 27/30] fix(web): an abandoned release swallows every pointer-less click in its window On engines whose clicks carry no pointer id, a newer finger's click could use up an abandoned finger's record before that finger's delayed click arrived, letting the abandoned click act. The record now stays until its window closes, so both ambiguous clicks are swallowed in either order. --- packages/web/src/ui/board-view.ts | 11 +++--- packages/web/test/board-a11y.test.ts | 57 +++++++++++++++++++--------- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index f6e8d62d..408a97f2 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -418,9 +418,10 @@ export class BoardView { * * On an engine whose clicks carry no pointer fields, a click cannot be attributed to a pointer, so a * choice is unavoidable. Only releases within {@link CLICK_WINDOW_MS} can be its own; older records - * are dropped. Safety first: while an abandoned gesture's click is still due, the click is taken to - * be that one, because an abandoned gesture must never act; at worst a genuine tap within that window - * is swallowed and simply repeated. Otherwise it is taken to be the last released pointer's. + * are dropped. Safety first: an abandoned gesture must never act, and its click may come before or + * after another finger's, so every click within that window of an abandoned release is swallowed, + * and the record stays until the window closes. At worst a genuine tap within that window is + * swallowed and simply repeated. Otherwise it is taken to be the last released pointer's. */ private consumeSuppressedClick(event: MouseEvent): boolean { if (this.suppressedClicks.size === 0) return false; @@ -431,8 +432,8 @@ export class BoardView { for (const [pointerId, record] of this.suppressedClicks) { if (now - record.at > CLICK_WINDOW_MS) this.suppressedClicks.delete(pointerId); } - for (const [pointerId, record] of this.suppressedClicks) { - if (record.abandoned) return this.suppressedClicks.delete(pointerId); + for (const record of this.suppressedClicks.values()) { + if (record.abandoned) return true; } const last = this.lastReleasedPointer; return last !== null && this.suppressedClicks.delete(last); diff --git a/packages/web/test/board-a11y.test.ts b/packages/web/test/board-a11y.test.ts index 98ab7ecc..4dc6ad72 100644 --- a/packages/web/test/board-a11y.test.ts +++ b/packages/web/test/board-a11y.test.ts @@ -1068,20 +1068,20 @@ test('a gesture cut short by an owner change selects nothing with its trailing c withDragGlobals((win) => { const { root, board } = mountWithFeedback({ playerColor: null }); // A swipe begun before the role arrives: nobody owns the pieces, so no drag starts. - root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1 }); - win.dispatchEvent('pointermove', { ...centreOf('e4'), pointerId: 1 }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 1, timeStamp: 1000 }); + win.dispatchEvent('pointermove', { ...centreOf('e4'), pointerId: 1, timeStamp: 1000 }); board.setPlayerColor('white'); // `joined` lands mid-gesture - win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1 }); - root.dispatchEvent('click', centreOf('e2')); // the gesture's own trailing click + win.dispatchEvent('pointerup', { ...centreOf('e4'), pointerId: 1, timeStamp: 1000 }); + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 1010 }); // the gesture's own trailing click assert.equal(root.querySelector('[aria-selected="true"]'), null, 'the cancelled gesture selects nothing'); - // A gesture abandoned off the board leaves no click behind; it must not swallow the next real one. - root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 2 }); + // Later, a gesture abandoned off the board leaves no click behind; it must not swallow the next real one. + root.dispatchEvent('pointerdown', { ...centreOf('d2'), pointerId: 2, timeStamp: 3000 }); board.setPlayerColor(null); board.setPlayerColor('white'); - root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 3 }); - win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 3 }); - root.dispatchEvent('click', centreOf('e2')); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 3, timeStamp: 3000 }); + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 3, timeStamp: 3000 }); + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 3010 }); assert.equal(root.querySelector('[data-square="e2"]')?.getAttribute('aria-selected'), 'true', 'an ordinary click still selects'); }); }); @@ -1366,14 +1366,37 @@ test("on an engine whose clicks carry no pointer id, an abandoned finger's delay withDragGlobals((win) => { const { root } = mountWithFeedback({ playerColor: 'white' }); const selected = (): string | null => root.querySelector('[aria-selected="true"]')?.getAttribute('data-square') ?? null; - root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 121, pointerType: 'touch' }); // A - root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 122, pointerType: 'touch' }); // B abandons A - win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 121, pointerType: 'touch' }); // A releases - win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 122, pointerType: 'touch' }); // B releases - root.dispatchEvent('click', centreOf('e2')); // A's delayed click, after B's release, with no pointer fields - assert.equal(selected(), null, "the abandoned finger's click selects nothing"); - root.dispatchEvent('click', centreOf('g1')); // B's own click - assert.equal(selected(), 'g1', "B's tap then works"); + // The clicks of A and B cannot be told apart, in either order, so both are swallowed within the window. + for (const [first, second, base] of [['e2', 'g1', 1000], ['g1', 'e2', 5000]] as const) { + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: base + 1, pointerType: 'touch', timeStamp: base }); // A + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: base + 2, pointerType: 'touch', timeStamp: base }); // B abandons A + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: base + 1, pointerType: 'touch', timeStamp: base + 10 }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: base + 2, pointerType: 'touch', timeStamp: base + 20 }); + root.dispatchEvent('click', { ...centreOf(first), timeStamp: base + 50 }); // no pointer fields + assert.equal(selected(), null, `the first click (${first}) selects nothing`); + root.dispatchEvent('click', { ...centreOf(second), timeStamp: base + 300 }); + assert.equal(selected(), null, `the second click (${second}) selects nothing: the abandoned one never acts`); + root.dispatchEvent('click', { ...centreOf('g1'), timeStamp: base + 1011 }); // past the window: a new tap + assert.equal(selected(), 'g1', 'a tap after the window acts'); + root.dispatchEvent('click', atClick('g1', -1)); // deselect again + } + }); +}); + +test("on an engine whose clicks carry no pointer id, an abandoned release never uses up a drag's own suppression", () => { + withDragGlobals((win) => { + const { root, board } = mountWithFeedback({ playerColor: null }); + root.dispatchEvent('pointerdown', { ...centreOf('e2'), pointerId: 141, pointerType: 'touch', timeStamp: 1000 }); + board.setPlayerColor('white'); // abandons A + win.dispatchEvent('pointerup', { ...centreOf('e2'), pointerId: 141, pointerType: 'touch', timeStamp: 1000 }); + // B drags a knight and wobbles back onto it near the end of A's window. + root.dispatchEvent('pointerdown', { ...centreOf('g1'), pointerId: 142, pointerType: 'touch', timeStamp: 1800 }); + win.dispatchEvent('pointermove', { ...centreOf('f3'), pointerId: 142, pointerType: 'touch', timeStamp: 1850 }); + win.dispatchEvent('pointermove', { ...centreOf('g1'), pointerId: 142, pointerType: 'touch', timeStamp: 1880 }); + win.dispatchEvent('pointerup', { ...centreOf('g1'), pointerId: 142, pointerType: 'touch', timeStamp: 1900 }); + root.dispatchEvent('click', { ...centreOf('e2'), timeStamp: 1950 }); // A's late click: swallowed + root.dispatchEvent('click', { ...centreOf('g1'), timeStamp: 2100 }); // B's own click, after A's window + assert.equal(root.querySelector('[aria-selected="true"]'), null, "the drag's own click is still swallowed"); }); }); From 5793636f7b2ce953c3cc6d70eaf95552ce2593ca Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 02:47:48 +0300 Subject: [PATCH 28/30] docs(web): say when a suppressed click keeps its record --- packages/web/src/ui/board-view.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/web/src/ui/board-view.ts b/packages/web/src/ui/board-view.ts index 408a97f2..f75fe8db 100644 --- a/packages/web/src/ui/board-view.ts +++ b/packages/web/src/ui/board-view.ts @@ -411,7 +411,8 @@ export class BoardView { } /** - * If `event` is a click a recorded release produced, forget that entry and return true. Modern + * If `event` is a click a recorded release produced, return true (and forget that entry, except as + * described below for an abandoned release on an engine without pointer ids). Modern * browsers deliver `click` as a PointerEvent carrying the id of the pointer behind it, whenever it * arrives, so the id decides. A click with no pointer (assistive technology, keyboard, * `element.click()`) has `pointerType` '' and never counts. From cbb06a2831953613f7d0e0e52afeb5493f469670 Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 11:32:01 +0300 Subject: [PATCH 29/30] docs: record the abandoned-click fix and its Linux validation --- docs/PROJECT_STATE.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 6a3c84e6..9e36edbf 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -5224,4 +5224,20 @@ Addresses four blocking review findings identified by ChatGPT independent review - on Windows: `npm ci`, build and lint passed; all eight `check:*` guards passed, including `check:test-topology` (479 test files across 21 suites, all placed, classified and reachable); `test:scripts` **315 of 315**, with zero skips (up from 312 with #89's new script test), including the topology tests "backend-free Playwright discovers only the eleven offline specs" and the 32-spec discovery count; - entirely on Linux (the exact tree from `git archive 2e878c5`, with no Windows `node_modules` or uncommitted files, on WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): `npm ci` and build succeeded; hermetic suite **3,961 of 3,961** across 19 workspaces, with zero skips (web 1,471); static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 failed, 0 skipped and 0 flaky with `--retries=0`; no config, timeout, worker or test changes. - **Exact-head review of `2e878c5`**: Gemini (429, resets in 87 h) and Sonnet (429, resets in 153 h) were still quota-blocked, so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE**, with no critical, high or medium finding. It covered Qodo's off-board release / accumulated click-record finding (resolved), Greptile's delayed abandoned-click finding (resolved, with the documented no-pointer-id trade-off), per-pointer `pointerId` handling, assistive-technology and pointer-less clicks, multi-touch and interleaved pointers, #89's merged topology and harness changes (no semantic conflict), and this file's merge resolution. Low notes, left as they are: the no-pointer-id fail-safe can swallow one genuine tap within 1 s of an abandoned release; an ignored press (off the board or with an overlay open) does not clear that pointer's stale record, which is bounded and time-limited; `lastReleasedPointer` also updates on ordinary taps, which is harmless. +- **Pushed `431df30`; gates on it**: CI green; Greptile 5/5 on that exact head with no findings. Qodo raised a new **Medium** finding on that head, "An abandoned tap can select a piece", in `consumeSuppressedClick`. On an engine whose clicks carry no pointer id, a newer finger's click deleted an abandoned finger's record. If that click arrived before the abandoned finger's delayed click, the delayed click then reached `interaction.tap()` under the current owner. +- **Fix in `fcb91b9`**: + - On the no-pointer-id path, an abandoned record now swallows every click within `CLICK_WINDOW_MS` (1 s) of its release and is not deleted. It goes when the window closes or when the same pointer presses again. Both ambiguous clicks are swallowed, in either order. + - Clicks that carry a pointer id still delete only their own record. Assistive-technology clicks (`pointerType ''`) are still never swallowed. Drag records and the last-released fallback are unchanged. + - The trade-off widens slightly: every genuine pointer-less tap within 1 s of an abandoned release is swallowed and simply repeated. + - RED first: the delayed-click test now covers both click orders, plus a tap after the window that acts. It failed before the fix. + - New test: an abandoned release never uses up a drag's own suppression. + - One older test ("a gesture cut short by an owner change …") got explicit event times 2 s apart, so its second half (an off-board abandoned gesture must not swallow a later tap) falls outside the first half's window. Its assertions are unchanged. + - Mutation sweep: 39 compiled mutations, all killed by tests. It includes the old consume-on-first-click behaviour, removing the abandoned rule, and an abandoned rule that also deletes the last drag record. That last mutant first survived; the new drag test kills it. +- **Exact-head review of `fcb91b9`**: Gemini and Sonnet were still quota-blocked (429), so a fresh read-only Claude reviewer agent ran the strict review. **APPROVE**, with no critical, high or medium finding. It found Qodo's finding resolved in both orders, no regression for pointer-id, assistive-technology, drag, multi-touch, cap or purge behaviour, and the older test's timing change legitimate. + - Low: the doc comment's first line still said a matching click always forgets its entry. Corrected in **`5793636`** (comment only). + - Low, left as is: a drag record whose click the abandoned rule swallowed can linger and swallow one genuine tap after the abandoned window closes, within its own 1 s. +- **Validation of `5793636`**: + - A first Linux run was stopped during its build step because Claude Code reclaimed background work when the Windows host ran low on memory. The owner approved the resume. The host was then re-checked (3.9 GB of 15.7 GB physical free; commit 13.9 of 31.4 GB) and no stale WSL, npm or Playwright processes remained. + - On Windows: lint passed; all eight `check:*` guards passed; `test:scripts` **315 of 315**, with zero skips; board tests 74 of 74. + - Entirely on Linux, rerun from scratch (a fresh `git archive 5793636`, a new Linux tree, no Windows `node_modules` or uncommitted files, WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): `npm ci` and build succeeded; hermetic suite **3,962 of 3,962** across 19 workspaces, with zero skips (web 1,472, up one for the new test); static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 failed, 0 skipped and 0 flaky with `--retries=0`; no config, timeout, worker or test changes. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge. From 33736e6fc50dd82c48dd9fee09e389ec24ffc68c Mon Sep 17 00:00:00 2001 From: Hussein Mohamed Date: Sun, 4 Oct 2026 11:44:21 +0300 Subject: [PATCH 30/30] docs: keep one current Increment 88 line in the project-state header --- docs/PROJECT_STATE.md | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/docs/PROJECT_STATE.md b/docs/PROJECT_STATE.md index 9e36edbf..2a7c7c2f 100644 --- a/docs/PROJECT_STATE.md +++ b/docs/PROJECT_STATE.md @@ -6,29 +6,7 @@ > to read **only this file** and continue immediately. Updated after every > milestone and every significant architectural step. -_Last updated: 2026-10-04 — M15 Increment 88: Player board ownership, merged with Increment 87 (#89) and revalidated._ - -Prior: _Last updated: 2026-10-04 — M15 Increment 88: click-window tests pinned and full Linux validation._ - -Prior: _Last updated: 2026-10-04 — M15 Increment 88: time-bounded pointer-less click matching and full Linux validation._ - -Prior: _Last updated: 2026-10-04 — M15 Increment 88: bounded click records, safety-first pointer-less clicks, full Linux validation._ - -Prior: _Last updated: 2026-10-04 — M15 Increment 88: pointer-less clicks tied to the last release, and full Linux validation._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: per-pointer gesture tracking and full Linux validation._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: abandoned gestures never act, and full Linux validation._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: drag cancellation and full Linux validation of the final code._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: bounded no-pointer-id click fallback and drag release ownership._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: pointer-matched click suppression and full Linux validation._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: Greptile click-suppression correction and Linux backend validation._ - -Prior: _Last updated: 2026-10-03 — M15 Increment 88: Player board ownership and read-only spectators._ +_Last updated: 2026-10-04 — M15 Increment 88: Player board ownership and read-only spectators, merged with Increment 87 (#89) and revalidated._ Prior: _Last updated: 2026-10-03 — M15 Increment 87: Windows SIGTERM harness external review corrections._ @@ -5240,4 +5218,5 @@ Addresses four blocking review findings identified by ChatGPT independent review - A first Linux run was stopped during its build step because Claude Code reclaimed background work when the Windows host ran low on memory. The owner approved the resume. The host was then re-checked (3.9 GB of 15.7 GB physical free; commit 13.9 of 31.4 GB) and no stale WSL, npm or Playwright processes remained. - On Windows: lint passed; all eight `check:*` guards passed; `test:scripts` **315 of 315**, with zero skips; board tests 74 of 74. - Entirely on Linux, rerun from scratch (a fresh `git archive 5793636`, a new Linux tree, no Windows `node_modules` or uncommitted files, WSL2 Ubuntu 26.04 with Node 22.23.3 and npm 10.9.9, 4 workers): `npm ci` and build succeeded; hermetic suite **3,962 of 3,962** across 19 workspaces, with zero skips (web 1,472, up one for the new test); static Playwright **187 of 187** and backend Playwright **232 of 232**, both 0 failed, 0 skipped and 0 flaky with `--retries=0`; no config, timeout, worker or test changes. +- **Pushed `cbb06a2`; gates on it**: an independent exact-final-head review approved it. Greptile 5/5 on that exact head with no findings. Qodo marked the abandoned-tap finding Resolved, but raised one new maintainability finding: the header held twelve "Prior:" lines that were all intermediate states of this increment. Main keeps at most a few per increment, and that history is already in this entry's body. The header now has one current Increment 88 line, followed by main's previous line as the first "Prior:" line. Only lines this PR had added were removed; main's header lines are all unchanged, so against main the file is still purely additive. Docs only; no source, test or config change. - **Deliberate limits**: no production caller applies queued premoves (`applyPremove` is exercised only by tests; unchanged here). Studies and lesson boards mount without players and show positions with `setTurn(false)`, which on any board means "premove", not "read-only"; that is a separate surface and is unchanged. The owner performs the merge.