Kibitz update

[GreenAsJade]

Fixes everything ;)

[github-actions]

Code Review — Bugs & Potential Issues

1. recenterGoban() called with stale metrics in the debounced resize path

In KibitzBoard.tsx, the new onResize callback calls recenterGoban() unconditionally at the end, regardless of whether setSquareSizeBasedOnDisplayWidth has actually run:

if (no_debounce) {
    gobanController.setSquareSizeBasedOnDisplayWidth(targetDisplayWidth);
} else {
    resizeDebounceRef.current = setTimeout(() => onResizeRef.current(true), 10);
}

recenterGoban(); // runs before square size is updated in the no_debounce=false path

OgsResizeDetector calls onResize() with no argument (so no_debounce=false). In that path, setSquareSizeBasedOnDisplayWidth is only scheduled via a 10ms timeout. recenterGoban() immediately calls computeMetrics() on the old square size and writes incorrect left/transform values to the DOM. Ten milliseconds later the true resize fires and corrects the centering.

This is most visible in fitMode="contain" layouts, where both the scale and left offset are wrong for the duration of the debounce window, causing a brief layout shift on every container resize.

---

2. isCurrentSecondaryLoadController staleness check weakened for same-game variation switches

In KibitzRoomStage.tsx, the closure-local isCurrentSecondaryLoadController (inside the variation-apply effect) previously read controller.goban.config?.game_id live as the staleness sentinel. The PR changes it to compare against the closed-over expectedSecondaryBoardGameId:

-const controllerGameId = secondaryBoardController.goban.config?.game_id ?? null;
 return Boolean(
     secondaryBoardController &&
     context &&
     context.controller === secondaryBoardController &&
     context.roomId === currentRoomIdRef.current &&
-    context.gameId === controllerGameId &&
+    context.gameId === expectedSecondaryBoardGameId &&   // closed over at effect setup
     \!isDetachedBoardController(secondaryBoardController),
 );

setSecondaryBoardController also stores expectedSecondaryBoardGameId into context.gameId. So context.gameId === expectedSecondaryBoardGameId compares a value the context was stored with against the same value captured at effect-setup time — the check is a tautology for the lifetime of that effect closure.

When a user switches between two variations of the same game (same game_id, different variation.id), the board does not remount and the epoch is not bumped. The effect re-runs because selectedVariationApplyKey changes, and the old effect's disposed = true cleanup provides a secondary guard on most code paths. However, a few call sites invoke isCurrentSecondaryLoadController() as the first gate without checking disposed (e.g. applyVisibleVariationsToLoadedBase, reloadBaseThenApplyVisibleVariations), leaving a window where a stale variation-load callback can pass the staleness check and apply the previous variation's data to the secondary board.

The old live read from controller.goban.config.game_id was immune to this because the controller's config does not change between variation switches within the same game.

[github-actions]

Code Review: One confirmed issue found.

requestSecondaryBoardDetachedRemount: duplicate debug guard emits stale epoch/nonce

File: src/views/Kibitz/KibitzRoomStage.tsx

The function now has two consecutive if (isKibitzVariationDebugEnabled()) blocks. The second block emits 'secondary-board:detached-remount-requested' BEFORE the mutations that give the remount its identity (secondaryBoardControllerEpochRef.current += 1 and bumpSecondaryBoardRemountNonce() both execute after both blocks). The nonce and epoch in the 'requested' event are therefore the pre-bump values, but the subsequent 'secondary-board:remount-controller-ready' event uses the post-bump epoch. When correlating these two events in debug logs the nonces will not match, making it appear as if they belong to different operations.

The two if blocks should be merged into one block, and the second event ('detached-remount-requested') should either be emitted after the mutations or removed entirely, since the 'detached-remount-reset' event already captures the pre-reset state.

[github-actions]

Code Review

I found one confirmed bug and one plausible regression. No nit-picks.

---

Bug: requestSelectedGameBaseSnapshot — stale cache permanently blocks refetch

File: src/views/Kibitz/KibitzRoomStage.tsx

When a cross-room variation snapshot has been successfully fetched but the live game has since advanced past that snapshot, requestSelectedGameBaseSnapshot enters the stale-cache branch and always returns without ever fetching a fresher copy:

if (cachedSnapshot) {
    selectedGameBaseSnapshotBackoffRef.current.set(selectedGameId, requiredSnapshotMoveNumber);
    // ...
    setSelectedGameBaseSnapshot(cachedSnapshot);
    return;   // ← exits without fetching; cache entry is never cleared
}

The backoff check below this block (backoffRequiredSnapshotMoveNumber >= requiredSnapshotMoveNumber) is only reached when cachedSnapshot is null, so it is dead code once a cache entry exists. The cache entry itself is never deleted on staleness — only set on a successful fetch and never cleared. Every subsequent call for the same game re-enters the stale branch, serves the old snapshot, re-records the backoff, and exits.

Consequence: If a cross-room variation is opened after the source game has advanced beyond the move when the snapshot was first fetched, the secondary board will permanently display the variation from the wrong base position with no recovery path.

Fix: Remove (or don't set) the cache entry when it fails the freshness check, so the code falls through to the fetch path. Something like:

if (cachedSnapshot) {
    if (\!cachedSnapshotIsFreshEnough) {
        // Cache is stale; clear it so we fall through to a fresh fetch.
        selectedGameBaseSnapshotCacheRef.current.delete(selectedGameId);
    } else {
        // ...return cached value...
        return;
    }
}

---

Plausible regression: mobile draft board renders without dimension guard when secondaryBoardGame is null

File: src/views/Kibitz/KibitzRoomStage.tsx

The old mobile rendering condition was:

const renderPreviewBoard = Boolean(mobileCompareTargetActive && secondaryBoardGame);

This required secondaryBoardGame to be non-null before rendering the board (ensuring width/height from boardDimensionsOf(secondaryBoardGame) were valid).

The new mobileSecondaryOwner memo computes "draft" from isDraftingVariation alone:

const isDraftingVariation = secondaryPane.variation_source_game_id \!= null;

…which can be true while secondaryBoardGame (= previewGame ?? secondaryPane.variation_source_game) is still null (e.g., if variation_source_game hasn't been populated yet). In that case the board renders with boardDimensionsOf(null) — no explicit width/height props — which may default to 19×19 even for a 9×9 or 13×13 source game.

Worth adding a secondaryBoardGame \!= null guard to mobileSecondaryOwner === "draft" rendering, or ensuring isDraftingVariation is only true once secondaryBoardGame is available.

[github-actions]

Code Review — Bugs & Performance Notes

1. canReconnectController creates a circular dependency on controller remount

File: src/views/Kibitz/useKibitzCurrentGameConnectionKeeper.ts

scheduleReconnectBurst fires 6 times within ~1 second after a controller change (delays: 0 ms, 50 ms, 250 ms, 1000 ms + 2 rAF callbacks). Every one of those calls goes through connect(), which now checks canReconnectController:

const controllerTailMoveNumber =
    controller.goban?.engine?.last_official_move?.move_number ?? 0;
return controllerTailMoveNumber >= liveTailMoveNumber;

When a board controller freshly remounts for a live game that is already at, say, move 70, the controller starts at move 0. canReconnectController returns false and all 6 burst attempts are silently dropped. The board can only hydrate once it receives gamedata, which the server sends in response to game/connect — the very message being blocked. If the goban submodule's internal game/connect (via createGoban) is also disrupted (e.g. a network blip exactly during board initialization), the 20-second keepalive will also fail the guard, because the controller is still at move 0. The board stays permanently unconnected until an external trigger fires (page focus, visibility change, socket reconnect).

The newly removed explicit socket.send("game/connect", ...) in KibitzBoard.tsx was the prior safety valve for exactly this scenario (it fired from handleRestoreToOfficialTail when the normal hydration path failed). Its removal increases reliance on the keeper to rescue the connection — but the keeper is now also guarded.

Suggestion: Consider tracking whether the keeper itself sent the initial game/connect and bypassing canReconnectController for the very first successful connect attempt. Alternatively, schedule a retry loop that re-checks canReconnectController periodically once it fails, rather than silently giving up.

---

2. isCurrentSecondaryLoadController compares context.gameId against expectedSecondaryBoardGameId but setSecondaryBoardController stores it from expectedSecondaryBoardGameId at closure time

File: src/views/Kibitz/KibitzRoomStage.tsx

setSecondaryBoardController is a useCallback that closes over expectedSecondaryBoardGameId. isCurrentSecondaryLoadController, inside the large variation-apply useEffect, compares context.gameId === expectedSecondaryBoardGameId directly — but the expectedSecondaryBoardGameId in the outer closure of setSecondaryBoardController is captured at the time the callback was created, while the one inside isCurrentSecondaryLoadController is the one captured at the time that effect ran.

If selectedVariationGameId (and thus expectedSecondaryBoardGameId) changes between the render that created setSecondaryBoardController and the render that runs the variation-apply effect, the two closures will have different values. The context will hold the old game ID while the effect checks the new one, making isCurrentSecondaryLoadController return false permanently for that controller mount, and every snapshot load attempt aborts as stale.

This is most likely to surface when a user rapidly switches between variations for different games (game A → game B → back to game A before A's board is ready).

Note: onReady is not in the large effect's dependency array, so the effect does not re-run simply because setSecondaryBoardController was re-created. The mismatch can persist until something else (gameId change, remount nonce bump) forces a full re-initialization.

[github-actions]

Code Review — Kibitz Update

Two issues worth flagging:

---

Bug: Stale mainGame?.live in reportMainBoardHydration useCallback

File: src/views/Kibitz/KibitzRoomStage.tsx

Inside reportMainBoardHydration, mainGame?.live is read directly from the closure:

const liveGame = Boolean(mainGame?.live);
const hydrated =
    expectedMoveNumber > 0
        ? officialTailMoveNumber >= expectedMoveNumber
        : \!liveGame && hasMoveTree && officialTailMoveNumber === 0;

But mainGame (i.e. room.current_game) is not listed in the useCallback dependency array — only room.id is. When a live game ends, room.id stays the same while room.current_game.live flips to falsy. The callback is never rebuilt, so liveGame stays stale as true.

The consequence: after a game ends, the zero-move hydration branch (\!liveGame && hasMoveTree && officialTailMoveNumber === 0) is permanently unreachable inside this callback. Hydration can only be reported as true via the officialTailMoveNumber >= expectedMoveNumber path, blocking the snapshot broker until something else forces a callback rebuild.

Fix: add mainGame?.live (or mainGame) to the useCallback dependency array, or access it through a ref that's kept current by the existing useEffect that updates mainBoardExpectedMoveNumberRef.

---

Performance: Move trunk traversal in the render path (KibitzInner)

File: src/views/Kibitz/KibitzInner.tsx

const mainBoardOfficialTailMoveNumber = mainBoardController
    ? (getMoveTreeTrunkTail(mainBoardController.goban.engine.move_tree)?.move_number ?? 0)
    : 0;
const mainBoardCurrentMoveNumber = ...;
const mainBoardLastOfficialMoveNumber = ...;

getMoveTreeTrunkTail does an O(n) linear walk of the trunk on every render. KibitzInner re-renders on any state change (connection state, room changes, etc.) — not just on moves. For a 300-move game this is 300 pointer chases per unrelated render.

There's also a correctness wrinkle: the goban's move tree mutates in-place and doesn't trigger React re-renders on its own. So this value can be stale relative to the actual board state between renders of KibitzInner. KibitzRoomStage already solves this correctly with event-driven setMainBoardOfficialTailMoveNumber updates. If that state can be lifted or plumbed upward, it would avoid both the repeated traversal and the staleness.

[GreenAsJade]

Review item 1: Corner case when game ends on move zero. Not applicable.

Review item 2: Claude reckons:

• Performance: 300 pointer-chases is a few microseconds. Real but not
  a hot path issue.
• Correctness: the value is consumed synchronously by
  isMainBoardSafeForReconnect (line 1279), which is itself called at
  render time. Within a single render the value is consistent with
  what's read from cur_move/last_official_move on the adjacent lines
  (1273-1278), which have the same staleness profile. The bigger
  correctness risk is if anything downstream uses this in a useEffect
  deps array — which it isn't here.

The comment identifies a real architectural inconsistency (two
derivations of the same fact, one event-driven and one not) but
overstates the urgency. A cheaper interim fix is a useMemo keyed on
something that does change on tree updates (e.g., mainBoardController
epoch + last-known move count) — or just accept the cost since it's
microseconds. Full state lift is worth doing only as part of a broader
cleanup of the KibitzInner ↔ KibitzRoomStage controller ownership
boundary.

No action required at this time.

[Johnniedarkoo]

For bug 1: a fix is in. The risk is tiny but so is the fix so why not :)
For bug 2: the assessment is correct, and splitting that ownership was already in the works. So we'll leave that as is.

[github-actions]

Code Review: Kibitz update

Finding 1: GobanEngine instantiated without cleanup in buildSelectedGameBaseSnapshotFromDetails

buildSelectedGameBaseSnapshotFromDetails in KibitzRoomStage.tsx directly instantiates new GobanEngine(engineConfig) to extract snapshot data, but never calls any cleanup on it. This is the first place in the codebase that directly creates a GobanEngine instance rather than obtaining one through GobanController. If GobanEngine registers event listeners, timers, or other global state in its constructor (as a subclass of EventEmitter in the goban library), instances accumulated across repeated variation-game snapshot fetches would leak.

The existing fetchCurrentGameBaseSnapshot in KibitzInner.tsx demonstrates the established pattern: it wraps the controller in a try/finally and calls snapshotController.destroy(). The same discipline should apply here.

---

Finding 2: getMoveTreeTrunkTail O(n) walk on every render of KibitzInner

getMoveTreeTrunkTail (a while-loop trunk traversal, O(n) in move count) is called unconditionally in the render body of KibitzInner.tsx:

const mainBoardOfficialTailMoveNumber = mainBoardController
    ? (getMoveTreeTrunkTail(mainBoardController.goban.engine.move_tree)?.move_number ?? 0)
    : 0;

KibitzInner re-renders on every game event subscription (move-made, gamedata, last_official_move, etc.). For a game in progress with many moves, this traversal runs on every render and grows with move count. The result could be tracked as state updated by a move-made event listener on the controller, or at minimum wrapped in a useMemo invalidated by a move-counter derived from those events.

[github-actions]

Code Review - Kibitz Update

This is a substantial refactor with solid architecture overall. A few issues worth flagging:

---

Bug: boardHostReadinessKey in Goban creation effect triggers unnecessary controller teardowns (KibitzBoard.tsx)

The Goban creation effect includes both boardHostReadyKey and boardHostReadinessKey in its dependency array. boardHostReadinessKey is a memoised string that encodes fitMode, respectContainerBounds, and other props. Since boardHostReadinessKey is in the deps, the effect fires whenever any of those values change. The cleanup path runs (destroying the controller, calling onReady(null), setting goban to null), but then the body hits the early return because boardHostReadyKey has not been updated yet by the readiness useLayoutEffect.

Two concrete consequences:

1. fitMode / respectContainerBounds changes destroy and recreate the Goban. Neither prop appears in the GobanRendererConfig built inside the effect - they only influence the resize/layout callbacks. The controller recreation is wasted work.

2. Every gameId change produces an extra null-board frame. After cleanup the body returns early; the useLayoutEffect fires on the next render, schedules a RAF, and only then calls setBoardHostReadyKey. The board stays null for at least one animation frame even when the host element never unmounted.

Suggested fix: remove boardHostReadinessKey from the creation effect dependency array. The individual values that actually influence GobanRendererConfig (gameId, width, height, etc.) are already listed individually. fitMode and respectContainerBounds do not belong in the creation effect deps since they do not affect controller creation.

---

Bug: mobileDraftSourceSnapshotRefreshNonce can loop indefinitely (KibitzRoomStage.tsx)

The draft-source snapshot fetch effect calls requestSelectedGameBaseSnapshotForGame and on success bumps mobileDraftSourceSnapshotRefreshNonce. The nonce is in the effect dependency array, so it re-runs on the next render. The guard that should break the loop is mobileSecondaryBoardDimensions being truthy - but resolveDraftSourceBoardDimensions can return null even when the snapshot is in the cache (e.g. when getLogicalBoardDimensionsFromSnapshot finds no explicit width/height in the snapshot config).

When that happens: the snapshot is in the cache, requestSelectedGameBaseSnapshotForGame returns it immediately (cache hit), bumpMobileDraftSourceSnapshotRefreshNonce fires again, and the effect loops indefinitely. There is no max-retry limit or circuit-breaker.

A straightforward fix: cap the nonce at a small maximum retry count, or track whether the fetch already ran for a given (gameId, requiredMoveNumber) pair within the effect lifetime and skip the bump if it has.

---

Bug: dead AbortError guard masks incorrect failure classification (KibitzRoomStage.tsx)

In requestSelectedGameBaseSnapshotForGame, the outer catch block checks for AbortError and returns null. However, fetchSelectedGameBaseSnapshot catches all errors from get() internally and returns them as a typed failure result - it never re-throws. The outer AbortError check is therefore unreachable for that code path.

Practical impact: if a get() call is aborted (the requests module creates an AbortController per in-flight request), fetchSelectedGameBaseSnapshot catches it and buildSelectedGameSnapshotFailureFromError classifies it as network-error, recording a 5-second retry backoff in selectedGameBaseSnapshotFailuresRef. Aborted requests should not trigger a backoff. The fix is to detect AbortError inside fetchSelectedGameBaseSnapshot before calling buildSelectedGameSnapshotFailureFromError, or re-throw it so the outer guard can actually catch it.

[github-actions]

Code Review: PR #3579 -- Kibitz update

Large Kibitz feature update: change-board flow, variation sharing, room rename propagation, mobile resize/drag overhaul, anonymous create-room gate, new E2E tests and unit tests. The overall structure is solid and test coverage is thorough.

---

Bug: isCurrentGameBaseSnapshotUsable — new roomId check (KibitzInner.tsx)

The snapshot now requires snapshot.roomId === roomId to be considered usable. However, KibitzCurrentGameBaseSnapshot.roomId is typed as optional (roomId?: string | null in kibitzCurrentGameBaseSnapshotTypes.ts), and snapshots produced by captureCurrentGameBaseSnapshotFromController may not include a roomId if the call site does not pass one. If any snapshot is created without that field, the new guard will reject it as unusable, silently breaking variation loading even when the game id and move counts match.

Worth confirming every snapshot-creation call path sets roomId, or treating a missing snapshot.roomId as a pass-through rather than a rejection.

---

Performance: Eager DOM measurements outside the debug guard in KibitzBoard.tsx

recordCommittedScaledPresentationCheck constructs its details object by spreading getKibitzBoardMetricsSnapshot() and calling summarizeCommittedMobileScaledPresentationCheck() as arguments to recordKibitzBoardSizeEvent. Because JavaScript evaluates arguments before the call, getBoundingClientRect() and getComputedStyle() fire on three DOM elements even though recordKibitzBoardSizeEvent returns immediately when debug mode is off.

scheduleCommittedScaledPresentationPostchecks then unconditionally schedules two chained requestAnimationFrame callbacks that each repeat these reads. The synchronous call inside commitTransientScaledPresentation (which also eagerly spreads getKibitzBoardMetricsSnapshot) adds two more sets. In production every drag release triggers roughly 20 forced-layout reads across three frames for no benefit.

The fix is a debug guard before constructing the argument object:

if (isKibitzBoardSizeDebugEnabled()) {
    recordKibitzBoardSizeEvent('...', { ...getKibitzBoardMetricsSnapshot('...'), ... });
}

The same guard should wrap scheduleCommittedScaledPresentationPostchecks so the RAFs are only scheduled when debug is on. Several other call sites in this file already apply this pattern correctly (e.g. inside applyTransientDragTarget).

[github-actions]

Code Review — Kibitz Update

This is a substantial PR. Most of it is well-structured. A few concrete issues worth addressing:

---

Bug — boardDiv leaked on GobanController constructor failure

src/views/Kibitz/KibitzInner.tsx ~line 182

document.body.appendChild(boardDiv) and new GobanController(config) are both executed before the try block whose finally runs boardDiv.remove(). If the constructor throws, the finally never executes and the hidden boardDiv is permanently orphaned in document.body.

document.body.appendChild(boardDiv);
const snapshotController = new GobanController(config as GobanRendererConfig); // ← outside try

try {
    // ...
} finally {
    snapshotController.destroy();
    boardDiv.remove(); // ← never reached if constructor throws
}

Fix: move boardDiv.appendChild and the constructor call inside the try block.

---

Bug — Synthetic variation/chat ID collision

src/views/Kibitz/KibitzController.ts lines 341 and 386

When msg.message.i (the server-assigned UUID) is absent, the fallback ID is built from the user ID (msg.id) and the second-precision timestamp:

const id = msg.message.i ?? `var-${msg.message.t}-${msg.id}`;
// line 386: same pattern for chat items
const id = msg.message.i ?? `chat-${msg.message.t}-${msg.id}`;

msg.id is the sender's user ID (number), not a per-message identifier. If the same user posts two variations within the same second — not impossible under normal use — both receive identical synthetic IDs and the second one is silently dropped as a duplicate by syncMessagesFromChat.

---

Bug — Single malformed variation blocks all variation rendering

src/views/Kibitz/KibitzRoomStage.tsx lines 496–499

getRequiredVariationSnapshotMoveNumber returns null if any entry in the full visible set has a null analysis_from, even when the selected variation and all other visible ones are perfectly valid:

if (validRequiredSnapshotMoves.length \!== requiredSnapshotMoves.length) {
    return null; // one bad variation poisons the whole set
}

This cascades to isSecondaryVariationSnapshotReady → false, making the entire variation panel non-renderable. Variations whose analysis_from is missing cannot be applied anyway and should be skipped rather than blocking the rest.

---

Performance — Missing negative cache in lookupGameForKibitzCached

src/views/Kibitz/KibitzController.ts ~line 720

When the backend returns a falsy result (game not found), the promise resolves but nothing is stored in _game_lookup_cache. Every subsequent call for the same gameId fires a new network request:

.then((game) => {
    if (game) {                               // falsy result: never cached
        this._game_lookup_cache.set(gameId, game);
    }
    return game;
})

For a game that is perpetually absent or temporarily missing, this creates unbounded repeated lookups. Caching the negative result (or at least a short-TTL sentinel) would avoid the flood.

[github-actions]

Code Review Notes

Bug: Fetch not cancelled at network level on room/game change (KibitzInner.tsx)

When currentGameSnapshotTarget changes (room switch, game change), the cleanup only sets a cancelled boolean flag — the underlying fetch() inside fetchCurrentGameBaseSnapshot runs to full completion:

let cancelled = false;
void fetchCurrentGameBaseSnapshot(game, target.roomId)
    .then((snapshot) => {
        if (cancelled || currentRoomIdRef.current \!== roomIdAtStart) {
            return;  // response body already downloaded
        }
        ...
    });
return () => { cancelled = true; };

If a user switches rooms rapidly, multiple game-state payloads download concurrently and are silently discarded. Depending on payload size, this can waste significant bandwidth. Consider threading an AbortSignal through fetchCurrentGameBaseSnapshot and calling controller.abort() in the cleanup.

---

Performance: boardController in connect deps cascades into keepalive timer and focus listeners (useKibitzCurrentGameConnectionKeeper.ts)

connect now captures boardController in its closure:

const connect = React.useCallback(
    (reason: string) => {
        const controllerGameId = boardController?.goban?.config?.game_id ?? null;
        ...
    },
    [activeGameId, activeGameKey, allowReconnect, boardController, ...],
);

Both the keepalive interval effect and the focus/visibilitychange listener effect depend on connect:

}, [activeGameId, allowReconnect, connect]);  // keepalive setInterval
}, [activeGameId, allowReconnect, connect]);  // focus/visibility listeners

This means every time boardController changes (e.g., on reconnect), the 20-second keepalive setInterval is cleared and restarted from zero, and the focus/visibility listeners are removed and re-added. A reconnect event thus resets the keepalive timer, creating a window where no keepalive is sent.

Consider reading boardController from a ref inside connect (a boardControllerRef alongside the already-present previousBoardControllerRef) so connect can remain stable and not drag the interval/listener effects through controller lifecycle events.

[github-actions]

Reviewed the diff for bugs and performance issues.

Missing null guard in handlePointerDownCapture (KibitzBoard.tsx)

In the newly added debug capture handler, gobanDiv.current is read into a local variable and immediately dereferenced without a null check:

const gobanElement = gobanDiv.current;
const gobanRect = gobanElement.getBoundingClientRect(); // potential throw

gobanDiv is initialised with document.createElement("div"), so TypeScript types current as HTMLDivElement (non-nullable) and will not flag this. However, React does reset a MutableRefObject.current to null during unmount; if a pointer event is dispatched in that brief window the call would throw. A one-line guard suffices:

if (\!gobanElement) return;

---

Everything else I looked at checked out:

• The room-updated handler move from _active_room_handlers to _directory_handlers is intentional and covered by the new e2e test; users inside a room remain subscribed to the directory channel so they continue to receive updates.
• The currentGameSnapshotTarget useMemo omits resolvedRoom?.current_game?.live from its deps, but live is a static game-creation attribute that does not change mid-game, so there is no observable staleness in practice.
• predictNativeGobanContentSize only feeds the diagnostics log, not the layout path, so its square-board-only approximation has no visual impact on non-square boards.
• selectedGameBaseSnapshotPendingRef entries are always deleted in a finally block, so there is no leak on abort or error.
• The createRoomLoginRequired early return in both KibitzGamePickerOverlay and KibitzMobileGamePicker is placed after all hook calls, so there is no Rules-of-Hooks violation.

[github-actions]

Code Review

KibitzBoard.tsx — ref read in JSX render (transientDragFinalizingRef)

The outer board container div reads transientDragFinalizingRef.current directly in its style prop to control pointerEvents during drag finalization. Mutating a ref does not schedule a re-render, so when transientDragFinalizingRef.current is set to true (in the pointer-up / finalization path), the outer div's pointerEvents style is not updated until the next independent re-render.

In that window the outer div still receives onPointerDownCapture events. Because the inner goban element correctly has pointerEvents: none set imperatively, a tap in the board area passes through the inner element and is captured by the outer div — which can start a new drag while the finalization animation is still running.

The imperative path (setting gobanElement.style.pointerEvents directly in layout effects and event handlers) works correctly; the issue is only with the JSX-driven outer div style.

Fix: introduce a companion boolean state variable (e.g. transientDragFinalizing) set alongside the ref, and use that state value in the JSX style. The ref can continue to be used in the imperative/layout-effect paths where a synchronous read without triggering a re-render is intentional.