fix(client): resolve the local seat from the mode census (#7524)

client/src/hooks/__tests__/useConcedeHandler.test.tsx

@@ -27,7 +27,13 @@ vi.mock("../../game/sessionCleanup", () => ({
   clearPromptOverlayState: () => clearPromptOverlayStateMock(),
 }));
 
-vi.mock("../../stores/gameStore", () => ({
+// Only the store handle and `clearGame` are stubbed. The module's pure
+// helpers — `seatSource` and the `GAME_MODE_TRAITS` census behind it, which
+// `getPlayerId()` consults for the conceding seat — come through for real, so
+// this test concedes as the seat the census actually resolves rather than as a
+// seat the mock asserts.
+vi.mock("../../stores/gameStore", async () => ({
+  ...(await vi.importActual<typeof import("../../stores/gameStore")>("../../stores/gameStore")),
   useGameStore: {
     getState: () => ({
       dispatch: dispatchMock,

client/src/hooks/__tests__/usePlayerId.draftMatch.test.tsx

@@ -0,0 +1,99 @@
+/**
+ * Regression test for the pod-draft guest's collapsed seat.
+ *
+ * `usePlayerId`/`getPlayerId` are the single authority for "which seat is this
+ * client's own" — 54 production call sites read one of them, from the mulligan
+ * modal's `pending.find` in `GamePage` to `gameLoopController`'s priority gate
+ * and `useConcedeHandler`'s `player_id`. Both resolved the answer from a
+ * hand-typed list of game modes that predates `"draft-match"`, so a pod-draft
+ * guest — correctly told `activePlayerId: 1` by `setupDraftMatchAvatars`
+ * (`GameProvider.tsx`) — was answered seat 0, the host's seat.
+ *
+ * The class guard below is the one that would have caught it: for EVERY mode
+ * that seats other humans, a client told "you are seat 1" must not answer 0.
+ * It reads the mode census (`GAME_MODE_TRAITS`) rather than a second hand-typed
+ * list, so a mode added later is covered on the day it is added.
+ *
+ * What these rows do NOT prove: they do not render `GamePage`, so they do not
+ * show the mulligan modal appearing for the guest. They pin the seat authority
+ * that modal's `pending.find(e => e.player === playerId)` reads.
+ */
+import { renderHook } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+
+import { PLAYER_ID, SPECTATOR_PLAYER_ID } from "../../constants/game";
+import type { GameMode } from "../../stores/gameStore";
+import { GAME_MODE_TRAITS, hasRemoteHumans, useGameStore } from "../../stores/gameStore";
+import { useMultiplayerStore } from "../../stores/multiplayerStore";
+import { getPlayerId, usePlayerId } from "../usePlayerId";
+
+/** Derived from the census itself, never hand-typed: a mode added to
+ *  `GAME_MODE_TRAITS` joins this list without anyone remembering to. */
+const ALL_MODES = Object.keys(GAME_MODE_TRAITS) as GameMode[];
+
+function seatMe(gameMode: GameMode, activePlayerId: number | null) {
+  useGameStore.setState({ gameMode });
+  useMultiplayerStore.setState({ activePlayerId, isSpectator: false });
+}
+
+describe("local seat resolution", () => {
+  beforeEach(() => {
+    useGameStore.getState().reset();
+    useMultiplayerStore.setState({ activePlayerId: null, isSpectator: false });
+  });
+
+  afterEach(() => {
+    useGameStore.getState().reset();
+    useMultiplayerStore.setState({ activePlayerId: null, isSpectator: false });
+  });
+
+  it.each(ALL_MODES.filter((mode) => hasRemoteHumans(mode)))(
+    "%s: a client told it holds seat 1 is never answered seat 0",
+    (mode) => {
+      seatMe(mode, 1);
+      // Spectators answer `SPECTATOR_PLAYER_ID` here rather than 1; that is
+      // still not seat 0, so this assertion needs no by-name exception and
+      // stays honest for every mode the census grows.
+      expect(getPlayerId()).not.toBe(PLAYER_ID);
+    },
+  );
+
+  it("seats a pod-draft guest at the seat the pod gave them", () => {
+    seatMe("draft-match", 1);
+    const { result } = renderHook(() => usePlayerId());
+    expect(result.current).toBe(1);
+    expect(getPlayerId()).toBe(1);
+  });
+
+  it("still seats the pod-draft host at 0", () => {
+    seatMe("draft-match", 0);
+    const { result } = renderHook(() => usePlayerId());
+    expect(result.current).toBe(PLAYER_ID);
+    expect(getPlayerId()).toBe(PLAYER_ID);
+  });
+
+  // Counter-direction (the expensive collateral is over-correction): a solo
+  // game must keep answering 0 even when a previous online game left a stale
+  // `activePlayerId` behind. These rows are green with and without the fix —
+  // they nail the behaviour down, they do not evidence it.
+  it.each(ALL_MODES.filter((mode) => !hasRemoteHumans(mode)))(
+    "%s: a solo game ignores a stale wire seat",
+    (mode) => {
+      seatMe(mode, 1);
+      const { result } = renderHook(() => usePlayerId());
+      expect(result.current).toBe(PLAYER_ID);
+      expect(getPlayerId()).toBe(PLAYER_ID);
+    },
+  );
+
+  // Also green either way: the spectator split is a documented contract, not a
+  // side effect. `HudBadges.tsx` states outright that `usePlayerId()` returns
+  // `PLAYER_ID` in spectate mode and gates its second-person copy on
+  // `useSpectatorMode()` because of it.
+  it("keeps the spectator split: 255 from the getter, 0 from the hook", () => {
+    seatMe("spectate", SPECTATOR_PLAYER_ID);
+    const { result } = renderHook(() => usePlayerId());
+    expect(getPlayerId()).toBe(SPECTATOR_PLAYER_ID);
+    expect(result.current).toBe(PLAYER_ID);
+  });
+});

client/src/hooks/usePlayerId.ts

@@ -1,30 +1,57 @@
 import type { PlayerId, WaitingFor } from "../adapter/types";
 import { PLAYER_ID, SPECTATOR_PLAYER_ID } from "../constants/game";
-import { useGameStore } from "../stores/gameStore";
+import type { GameMode } from "../stores/gameStore";
+import { seatSource, useGameStore } from "../stores/gameStore";
 import { useMultiplayerStore } from "../stores/multiplayerStore";
 
-function currentLocalPlayerId(): PlayerId {
-  const gameMode = useGameStore.getState().gameMode;
-  if (gameMode === "spectate") {
-    return SPECTATOR_PLAYER_ID;
-  }
-  if (gameMode && (gameMode === "online" || gameMode === "p2p-host" || gameMode === "p2p-join")) {
-    return useMultiplayerStore.getState().activePlayerId ?? PLAYER_ID;
+/**
+ * The one seat resolver both entry points below share, so they cannot drift the
+ * way the two hand-typed mode lists they replace did. Which modes take their
+ * seat off a wire is the census's question, not this file's: see `seatSource`
+ * in `stores/gameStore.ts`.
+ *
+ * `spectatorSeat` is the one place the two entry points legitimately differ,
+ * and it is a parameter so that the difference is stated rather than repeated.
+ * `getPlayerId()` answers `SPECTATOR_PLAYER_ID`, which `dispatch.ts` reads to
+ * refuse submitting actions. `usePlayerId()` answers `PLAYER_ID`, because
+ * display surfaces render a spectated game from seat 0's side — `HudBadges.tsx`
+ * states that contract outright and gates its second-person copy on
+ * `useSpectatorMode()` because of it. Widening the hook to 255 would empty
+ * every `players[playerId]` lookup on the spectator board.
+ */
+function resolveLocalSeat(
+  gameMode: GameMode | null,
+  activePlayerId: PlayerId | null,
+  spectatorSeat: PlayerId,
+): PlayerId {
+  switch (seatSource(gameMode)) {
+    case "wire-assigned":
+      // Seat not delivered yet (setup still in flight): seat 0 is what every
+      // caller read before the assignment arrived, so this changes nothing.
+      return activePlayerId ?? PLAYER_ID;
+    case "no-seat":
+      return spectatorSeat;
+    case "seat-zero":
+      return PLAYER_ID;
   }
+}
 
-  return PLAYER_ID;
+function currentLocalPlayerId(): PlayerId {
+  return resolveLocalSeat(
+    useGameStore.getState().gameMode,
+    useMultiplayerStore.getState().activePlayerId,
+    SPECTATOR_PLAYER_ID,
+  );
 }
 
-/** React hook: returns the current player's game-assigned ID (0 or 1). Falls back to PLAYER_ID (0) for AI/local mode. */
+/** React hook: the seat this client occupies. Solo modes are seat 0 by
+ *  construction; every mode that takes its seat off a wire reads the assignment
+ *  the server, P2P host, or draft pod delivered. */
 export function usePlayerId(): PlayerId {
   const gameMode = useGameStore((s) => s.gameMode);
   const activePlayerId = useMultiplayerStore((s) => s.activePlayerId);
 
-  if (gameMode && (gameMode === "online" || gameMode === "p2p-host" || gameMode === "p2p-join")) {
-    return activePlayerId ?? PLAYER_ID;
-  }
-
-  return PLAYER_ID;
+  return resolveLocalSeat(gameMode, activePlayerId, PLAYER_ID);
 }
 
 /** Non-React getter for use in plain functions (autoPass, gameLoopController). */

client/src/stores/gameStore.ts

@@ -77,9 +77,26 @@ export type EngineAuthority = "client" | "wire";
  *  nothing they do can desync a peer or leak hidden info across a wire. */
 export type TableCompany = "solo" | "remote-humans";
 
+/** Where this client's OWN seat number comes from.
+ *
+ * `"seat-zero"` — a solo game. There is one local human and the engine seats
+ * them at 0 by construction; nothing on a wire can say otherwise, so a stale
+ * `activePlayerId` left behind by an earlier online game must not be read.
+ *
+ * `"wire-assigned"` — somebody else hands this client its seat: a server
+ * (`playerIdentity` from `WebSocketAdapter`), a P2P host (`game_setup`'s
+ * `assignedPlayerId`), or the pod that paired this match
+ * (`setupDraftMatchAvatars`). `multiplayerStore.activePlayerId` carries it.
+ *
+ * `"no-seat"` — a spectator holds no seat at all. The two seat resolvers in
+ * `usePlayerId.ts` deliberately answer this case differently; see the comment
+ * there for the contract `HudBadges.tsx` depends on. */
+export type SeatSource = "seat-zero" | "wire-assigned" | "no-seat";
+
 interface GameModeTraits {
   readonly authority: EngineAuthority;
   readonly company: TableCompany;
+  readonly seat: SeatSource;
 }
 
 /**
@@ -93,15 +110,15 @@ interface GameModeTraits {
  *
  * `spectate` is `remote-humans` by the *game* it observes, not by the observer.
  */
-const GAME_MODE_TRAITS: Record<GameMode, GameModeTraits> = {
-  "ai": { authority: "client", company: "solo" },
-  "local": { authority: "client", company: "solo" },
-  "native-ai": { authority: "wire", company: "solo" },
-  "online": { authority: "wire", company: "remote-humans" },
-  "p2p-host": { authority: "wire", company: "remote-humans" },
-  "p2p-join": { authority: "wire", company: "remote-humans" },
-  "draft-match": { authority: "wire", company: "remote-humans" },
-  "spectate": { authority: "wire", company: "remote-humans" },
+export const GAME_MODE_TRAITS: Record<GameMode, GameModeTraits> = {
+  "ai": { authority: "client", company: "solo", seat: "seat-zero" },
+  "local": { authority: "client", company: "solo", seat: "seat-zero" },
+  "native-ai": { authority: "wire", company: "solo", seat: "seat-zero" },
+  "online": { authority: "wire", company: "remote-humans", seat: "wire-assigned" },
+  "p2p-host": { authority: "wire", company: "remote-humans", seat: "wire-assigned" },
+  "p2p-join": { authority: "wire", company: "remote-humans", seat: "wire-assigned" },
+  "draft-match": { authority: "wire", company: "remote-humans", seat: "wire-assigned" },
+  "spectate": { authority: "wire", company: "remote-humans", seat: "no-seat" },
 };
 
 /** True when the authoritative engine state lives off this client — i.e. the
@@ -144,6 +161,22 @@ export function hasRemoteHumans(mode: GameMode | null): boolean {
   return mode !== null && GAME_MODE_TRAITS[mode].company === "remote-humans";
 }
 
+/**
+ * The seat axis of the census: where this client's own seat number comes from.
+ *
+ * Read this instead of testing `gameMode` against a list at the call site. The
+ * list form is what seated a pod-draft guest at 0: `"draft-match"` joined the
+ * union long after `usePlayerId`'s list was written, and nothing made the two
+ * meet. A mode added to `GAME_MODE_TRAITS` cannot compile without declaring
+ * its seat source, so it can never again default into somebody else's chair.
+ *
+ * `null` — no game yet — is `"seat-zero"`: nothing has assigned this client
+ * anything.
+ */
+export function seatSource(mode: GameMode | null): SeatSource {
+  return mode === null ? "seat-zero" : GAME_MODE_TRAITS[mode].seat;
+}
+
 interface GameStoreState {
   gameId: string | null;
   gameMode: GameMode | null;