Kibitz update

.github/workflows/lint.yml

@@ -121,7 +121,26 @@ jobs:
                   submodules: recursive
                   lfs: true
 
-            - name: Run spell check
-              uses: streetsidesoftware/cspell-action@v6
+            - uses: actions/setup-node@v4
+              with:
+                  node-version: "20"
+
+            - name: Cache node modules
+              id: cache-node-modules
+              uses: actions/cache@v4
+              with:
+                  path: node_modules
+                  key: ${{ runner.os }}-node-modules-${{ hashFiles('**/yarn.lock') }}
+                  restore-keys: |
+                      ${{ runner.os }}-node-modules-
+
+            - name: yarn install
+              if: steps.cache-node-modules.outputs.cache-hit != 'true'
+              uses: nick-fields/retry@v3
               with:
-                  files: src/**/*.{ts,tsx}
+                  timeout_minutes: 10
+                  max_attempts: 3
+                  command: yarn install
+
+            - name: Run spell check
+              run: yarn spellcheck

.vscode/cspell.json

@@ -154,13 +154,17 @@
         "pofile",
         "ponnuki",
         "postback",
+        "Postcheck",
+        "Postchecks",
         "pprettier",
         "qrcode",
         "quickmatch",
         "rackcdn",
         "rdil",
+        "refetches",
         "rengo",
         "replot",
+        "retryable",
         "rgba",
         "rollups",
         "roundrobin",
@@ -210,6 +214,7 @@
         "unannulable",
         "ungap",
         "unhighlight",
+        "unhydrated",
         "uniquification",
         "unitify",
         "Unranked",

Makefile

@@ -36,6 +36,9 @@ pretty prettier lint-fix format:
 	npm run prettier
 	npm run lint:fix
 
+spellcheck:
+	npm run spellcheck
+
 
 analyze visualizer bundle-visualizer:
 	npm run bundle-visualizer
@@ -47,6 +50,6 @@ GOBAN_SOCKET_WORKER_VERSION=0.2
 update-worker: build
 	cp dist/modules/GobanSocketWorkerScript.js ../ogs-node/src/GobanSocketWorker/GobanSocketWorkerScript-$(GOBAN_SOCKET_WORKER_VERSION).js
 
-.PHONY: dev build test analyze pretty prettier lint-fix .husky visualizer bundle-visualizer update-worker
+.PHONY: dev build test analyze pretty prettier lint-fix spellcheck .husky visualizer bundle-visualizer update-worker
 
 -include Makefile.production

e2e-tests/helpers/game-utils.ts

@@ -9,7 +9,7 @@
  * This program is distributed in the hope that it will be useful,
  */
 
-import { Page } from "@playwright/test";
+import { Locator, Page } from "@playwright/test";
 import { expect } from "@playwright/test";
 
 type BoardSize = "19x19" | "13x13" | "9x9";
@@ -87,6 +87,11 @@ export const clickOnGobanIntersection = async (
     page: Page,
     coord: string,
     boardSize: BoardSize = "19x19",
+    // Optional locator to scope which Goban to click. Defaults to the first
+    // pointer-bound goban on the page. Pass a scoped locator when more than
+    // one such goban can be present (e.g. Kibitz compare mode where the
+    // main board and the secondary analysis board are both interactive).
+    gobanLocator?: Locator,
 ) => {
     const boardLetters: { [size: string]: string } = {
         // cspell:disable
@@ -125,7 +130,7 @@ export const clickOnGobanIntersection = async (
     // Row: N (top) -> 0, 1 (bottom) -> N-1
     const row = sizeNumber - rowNumber;
 
-    const goban = page.locator(".Goban[data-pointers-bound]");
+    const goban = gobanLocator ?? page.locator(".Goban[data-pointers-bound]");
     await goban.waitFor({ state: "visible" });
     const box = await goban.boundingBox();
     if (!box) {

e2e-tests/kibitz/kibitz-basic-room.ts

@@ -19,24 +19,38 @@
 
 import type { CreateContextOptions } from "@helpers";
 import { BrowserContext, expect } from "@playwright/test";
+import {
+    acceptDirectChallenge,
+    createDirectChallenge,
+    defaultChallengeSettings,
+} from "@helpers/challenge-utils";
+import { playMoves, resignActiveGame } from "@helpers/game-utils";
+import { expectOGSClickableByName } from "@helpers/matchers";
 
 import { createKibitzRoomForLiveGame } from "./kibitz-helpers";
 
 /*
- * Verify the basic room flow: an authenticated user can create a Kibitz
- * room pointing at a live in-progress game, the room renders correctly
- * (board, room list, presence panel), and the user can post a chat
- * message that shows up in the room stream.
+ * Verify the basic room flow end-to-end:
  *
- * Exercises the create-room API path, the watcher navigation into the
- * room, and the comm-server kibitz-<roomId> Redis chat channel.
+ *   1. An authenticated user can create a Kibitz room pointing at a live
+ *      in-progress game, the room renders correctly, and the user can post
+ *      a chat message that shows up in the room stream.
+ *   2. The room owner can change the watched game: the players end their
+ *      first game, start a fresh one, and the owner switches the room's
+ *      board via the settings popover -> game picker overlay -> Change board
+ *      flow (POST /api/v1/kibitz/rooms/:id/change-board).
+ *
+ * Exercises the create-room API path, the comm-server kibitz-<roomId>
+ * Redis chat channel, and the change-board endpoint plus its UIPush
+ * propagation back to the watcher's room view.
  */
 export const kibitzBasicRoomTest = async ({
     createContext,
 }: {
     createContext: (options?: CreateContextOptions) => Promise<BrowserContext>;
 }) => {
-    const { watcherPage, roomId } = await createKibitzRoomForLiveGame(createContext);
+    const { watcherPage, blackPlayerPage, whitePlayerPage, whiteUsername, roomId } =
+        await createKibitzRoomForLiveGame(createContext);
 
     // URL ends up at /kibitz/<roomId>.
     expect(watcherPage.url()).toMatch(new RegExp(`/kibitz/${roomId}$`));
@@ -83,4 +97,120 @@ export const kibitzBasicRoomTest = async ({
     );
     await expect(roomFeed.getByText(message)).toBeVisible({ timeout: 10000 });
     console.log("[kibitz basic-room] chat message rendered in room stream");
+
+    // Phase 2: change the room's watched game. The players end their current
+    // game, start a fresh one, and the watcher (room owner) switches the
+    // board via the settings popover.
+
+    // The prelude only played 4 moves; OGS gates "resign" behind a 6-move
+    // threshold (before that the action is "cancel game", which has a
+    // different confirmation dialog that resignActiveGame doesn't match).
+    // Play two more moves so the next turn (move 7) is resign-eligible for
+    // blackPlayerPage. Coordinates are non-conflicting with the prelude's
+    // E5/G5/E7/G7.
+    console.log("[kibitz basic-room] playing two more moves so resign is available");
+    await playMoves(blackPlayerPage, whitePlayerPage, ["C3", "G3"], "9x9");
+
+    console.log("[kibitz basic-room] black resigning the first game");
+    await resignActiveGame(blackPlayerPage);
+
+    // Give the second game a title whose HEAD differs from the prelude's
+    // first game ("E2E Kibitz live source game"). The board-subtitle-link
+    // assertion below matches a head-anchored regex, so anchoring the
+    // distinguishing prefix at character 0 makes the check robust against
+    // future text-truncation (the title is currently rendered verbatim but
+    // the CSS already truncates visually with ellipsis, and a future
+    // refactor could JS-truncate too).
+    const secondGameTitleHead = "Kibitz target";
+    const secondGameName = `${secondGameTitleHead} (phase 2)`;
+    console.log("[kibitz basic-room] starting a second live game between the same players");
+    await createDirectChallenge(blackPlayerPage, whiteUsername, {
+        ...defaultChallengeSettings,
+        gameName: secondGameName,
+        boardSize: "9x9",
+        speed: "live",
+        timeControl: "byoyomi",
+        mainTime: "600",
+        timePerPeriod: "60",
+        periods: "5",
+        ranked: false,
+    });
+    await acceptDirectChallenge(whitePlayerPage);
+
+    // Capture the second game's id from the white player URL, same approach
+    // as the prelude.
+    await whitePlayerPage.waitForURL(/\/(game|play)\/\d+/, { timeout: 30000 });
+    const secondGameUrl = new URL(whitePlayerPage.url());
+    const secondGameMatch = secondGameUrl.pathname.match(/\/(game|play)\/(\d+)/);
+    if (!secondGameMatch) {
+        throw new Error(
+            `Expected /game/<id> or /play/<id> URL on white player page after second challenge, got ${secondGameUrl.pathname}`,
+        );
+    }
+    const secondGameId = Number(secondGameMatch[2]);
+    console.log(`[kibitz basic-room] second game id: ${secondGameId}`);
+
+    // Open the room settings popover and click "Change live game" -- this is
+    // gated on canChangeBoard, which is owner-or-moderator
+    // (kibitz/permissions.py:42-50). The watcher created the room, so they
+    // have it.
+    const gearButton = watcherPage.locator(".board-settings-button");
+    await expect(gearButton).toBeVisible({ timeout: 15000 });
+    await expect(gearButton).toBeOGSClickable();
+    await gearButton.click();
+
+    const popover = watcherPage.locator(".popover-container .KibitzRoomSettingsPopover");
+    await expect(popover).toBeVisible({ timeout: 15000 });
+
+    const changeBoardMenuButton = popover.getByRole("button", { name: /^Change live game$/ });
+    await expect(changeBoardMenuButton).toBeVisible({ timeout: 15000 });
+    await expect(changeBoardMenuButton).toBeOGSClickable();
+    console.log("[kibitz basic-room] owner clicking Change live game");
+    await changeBoardMenuButton.click();
+    // Clicking Change live game closes the popover and opens the game-picker
+    // overlay (KibitzRoomStage.tsx:1735-1738 -- close_all_popovers ->
+    // onChangeBoard).
+    await expect(popover).toBeHidden({ timeout: 15000 });
+
+    // The game-picker overlay is the same component as the create-room flow,
+    // in "change-board" mode (KibitzGamePickerOverlay.tsx). The game-id input
+    // has the same id (kibitz-game-picker-input); the footer's confirm
+    // button reads "Change board" instead of "Create room" in this mode.
+    const overlay = watcherPage.locator(".KibitzGamePickerOverlay");
+    await expect(overlay).toBeVisible({ timeout: 15000 });
+
+    const gameIdInput = overlay.locator("#kibitz-game-picker-input");
+    await expect(gameIdInput).toBeVisible({ timeout: 15000 });
+    await gameIdInput.fill(String(secondGameId));
+    await expect(gameIdInput).toHaveValue(String(secondGameId));
+
+    const loadButton = await expectOGSClickableByName(watcherPage, /^Load$/);
+    await loadButton.click();
+
+    // Wait for the overlay's footer confirm button to enable, then click. The
+    // disabled state lifts once the picker has resolved the entered game id
+    // and built the preview (canChangeBoard becomes true in the component).
+    const confirmChangeButton = overlay.getByRole("button", { name: /^Change board$/ });
+    await expect(confirmChangeButton).toBeVisible({ timeout: 15000 });
+    await expect(confirmChangeButton).toBeEnabled({ timeout: 15000 });
+    console.log("[kibitz basic-room] confirming Change board");
+    await confirmChangeButton.click();
+
+    // After the POST /change-board round-trip and the board-changed UIPush
+    // (KibitzRoomChangeBoard.post:329) propagate, the header's
+    // .board-subtitle-link href should reference the new game id
+    // (KibitzRoomStage.tsx:5281: href={`/game/${mainGame.game_id}`}). Two
+    // assertions on the same element: the structural href attribute (proves
+    // the room is bound to the new game's id) and the content-level text
+    // (proves the new game's metadata, not just its id, made it through).
+    // The text assertion uses a head-anchored regex so a future visual or
+    // JS truncation of the title would not silently weaken the check.
+    const boardSubtitleLink = watcherPage.locator(".board-subtitle-link");
+    await expect(boardSubtitleLink).toHaveAttribute("href", `/game/${secondGameId}`, {
+        timeout: 15000,
+    });
+    await expect(boardSubtitleLink).toHaveText(new RegExp(`^${secondGameTitleHead}`), {
+        timeout: 15000,
+    });
+    console.log(`[kibitz basic-room] room now watching game ${secondGameId}`);
 };

e2e-tests/kibitz/kibitz-basic.spec.ts

@@ -17,7 +17,11 @@
 
 import { ogsTest } from "@helpers";
 import { kibitzBasicRoomTest } from "./kibitz-basic-room";
+import { kibitzEditRoomDetailsTest } from "./kibitz-edit-room-details";
+import { kibitzShareVariationTest } from "./kibitz-share-variation";
 
 ogsTest.describe("@Kibitz basic flows", () => {
     ogsTest("Create room, navigate in, and post a chat message", kibitzBasicRoomTest);
+    ogsTest("Share an analysis variation", kibitzShareVariationTest);
+    ogsTest("Owner edits room details; non-owner cannot", kibitzEditRoomDetailsTest);
 });