[Aschenbath]

Two authorized CLIENT_READY requests on one socket can finish out of order, leaving the socket in both rooms and allowing the older request to overwrite the new revision state.

Track request order before asynchronous work and commit the join generation after access is granted. Superseded joins discard their remaining handshake/reconnect output; revision updates also stop when their join generation changes. A new authorized join leaves the previous room and preserves the existing last-socket-per-author leave notification behavior. A later denied request leaves the earlier authorized join valid.

Fixes #8460.

Added 14 deterministic regressions using real Socket.IO connections and controlled hook/DB pauses. On the original implementation, 13 fail; all 14 pass with the fix. Updated the hook documentation in both formats.

Validation on Windows, Node 24.22.0:

• 72 focused/socket/security tests passed; full backend: 1,909 passed, 22 pending. Five shell-script tests fail identically on the original implementation (1,895 passed there).
• Vitest: 856 passed, one skipped. TypeScript check, admin/login build and VitePress documentation build passed.
• Complete Chromium/Firefox rerun: 570 passed, one flaky test passed on retry, three skipped, exit 0. The scroll test also flakes on the original implementation. The first full run additionally failed the rename-input test; it passed on the full rerun and on the original full suite (571 passed, three skipped).
• pnpm run lint cannot start on either version: the locked ESLint 10 needs a flat config, while the repository provides .eslintrc.cjs. The locked TypeScript ESLint parser also rejects TS 7. The legacy AsciiDoc build requires an unavailable asciidoctor; the VitePress build succeeds.

Prepared with Codex.

UI changes

• This PR doesn't add or change anything users see or interact with.
• This PR changes the UI, and I clicked through it myself in a browser and I'm happy with the UI/UX. If an AI tool wrote any of it, only tick this after you've tried it yourself.

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Discard superseded CLIENT_READY joins

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Prevent older authorized joins from overwriting a newer join’s room, snapshot, or revision state.
• Leave the previous room on an authorized join while preserving last-socket-per-author leave
 notifications.
• Add deterministic socket regressions and document how superseded joins affect hooks.

Diagram

graph TD
  A["CLIENT_READY"] --> B["Access check"] --> C{"Access granted?"} -->|Yes| E["Commit generation and swap rooms"] --> F{"Current generation?"} -->|Yes| H["Publish snapshot and revisions"]
  C -->|No| D["Retain prior join"]
  F -->|No| G["Discard stale work"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Serialize CLIENT_READY requests per socket

• ➕ Removes overlapping join handshakes and simplifies ordering.
• ➖ A slow hook or database call blocks newer requests, including requests that should supersede it.

2. Cancel earlier asynchronous work

• ➕ Could avoid some unnecessary work after a newer join succeeds.
• ➖ Existing plugin hooks and database calls do not share a reliable cancellation contract; output and revision guards would still be needed.

Recommendation: Keep the generation-based approach. It lets a newer authorized join proceed without waiting for older hooks, preserves an earlier join when a later request is denied, and guards work that cannot be cancelled.

Files changed (4) +416 / -21

Bug fix (1) +58 / -21

PadMessageHandler.tsGuard asynchronous joins and revision updates by join generation +58/-21

Guard asynchronous joins and revision updates by join generation

• Reserves request order before asynchronous work, commits a generation only after access is granted, and drops superseded handshake or reconnect output. An authorized move leaves the previous room using shared leave-notification logic; snapshot baselines and in-flight revision updates are guarded against later joins or disconnects.

src/node/handler/PadMessageHandler.ts

Tests (1) +336 / -0

clientReadyJoinRace.tsAdd deterministic Socket.IO join-race regressions +336/-0

Add deterministic Socket.IO join-race regressions

• Adds 14 tests using real connections and controlled hook or database pauses. Covers out-of-order joins, denied requests, same-pad rejoins, room presence, reconnect output, disconnects, and in-flight revision updates.

src/tests/backend/specs/clientReadyJoinRace.ts

Documentation (2) +22 / -0

hooks_server-side.adocDocument superseded joins and leave-hook behavior in AsciiDoc +11/-0

Document superseded joins and leave-hook behavior in AsciiDoc

• Explains when a later authorized CLIENT_READY discards earlier hook results and handshake output. Clarifies that moving pads triggers userLeave only when the author’s last socket leaves.

doc/api/hooks_server-side.adoc

hooks_server-side.mdDocument superseded joins and leave-hook behavior in Markdown +11/-0

Document superseded joins and leave-hook behavior in Markdown

• Mirrors the AsciiDoc guidance on authorized versus denied competing joins, clientReady output, and last-socket-per-author leave notifications.

doc/api/hooks_server-side.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. A failing leave hook strands the socket with no pad 🐞 Bug ☼ Reliability

Description

In the pad-move path, handleMessage leaves the old room and commits the target session before
awaiting notifyUserLeave, which performs an author-color lookup and runs the userLeave plugin
hook. If either rejects, the new join stops before handleClientReady, leaving the socket outside
both rooms and without CLIENT_VARS; a slow hook also delays every pad switch.

Code

src/node/handler/PadMessageHandler.ts[R611-614]

+    if (previousSession && previousSession.padId !== pending.padId) {
+      await notifyUserLeave(socket, previousSession);
+      if (!isCurrentJoin()) return;
+    }

Evidence

The new awaited notifyUserLeave call sits after removal from the old room and before the security
and message hooks and handleClientReady. Its author-color lookup and userLeave hook can reject,
and no handler catches those errors, so they reject handleMessage for CLIENT_READY before the
new-room handshake runs. Previously, this notification ran only from handleDisconnect.

src/node/handler/PadMessageHandler.ts[600-615]
src/node/handler/PadMessageHandler.ts[276-318]
src/node/handler/PadMessageHandler.ts[600-614]
src/node/handler/PadMessageHandler.ts[294-318]
src/static/js/pluginfw/hooks.ts[346-353]
src/node/handler/PadMessageHandler.ts[678-682]
src/node/handler/PadMessageHandler.ts[1592-1598]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A previous-pad leave notification runs inline during a CLIENT_READY pad move, after the old room has been left and the target session committed. A rejection aborts the new join, while a slow `userLeave` hook delays it.

## Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[600-615]
- src/node/handler/PadMessageHandler.ts[294-318]

## Recommended Fix
Handle `notifyUserLeave` independently of join completion: start it without awaiting and attach a `.catch` that logs the error with its traceback through `messageLogger.error`. Continue the new handshake, retaining the `isCurrentJoin()` check and the existing last-socket presence check.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. A plugin-dropped join leaves the socket in no room 🐞 Bug ≡ Correctness

Description

handleMessage leaves the previous pad room and commits the new session before the
handleMessageSecurity and handleMessage hooks decide whether to accept CLIENT_READY. If either
hook throws or handleMessage returns null, the handler does not reach the new room join or send
CLIENT_VARS, leaving the connected socket in neither pad room.

Code

src/node/handler/PadMessageHandler.ts[R601-605]

+    const previousSession = thisSession.padId && socket.rooms.has(thisSession.padId)
+      ? {...thisSession} : null;
+    // Stop old-room updates while preparing the new snapshot, including same-pad rejoins.
+    if (previousSession) socket.leave(previousSession.padId);
+    thisSession.joinGeneration = joinGeneration;

Evidence

The transition leaves the previous room and commits joinGeneration at lines 601–605, before the
message hooks run. The handleMessage hook can return early at lines 660–663, before
handleClientReady joins the target room; a hook failure can likewise prevent that join after the
previous room has been left.

src/node/handler/PadMessageHandler.ts[600-610]
src/node/handler/PadMessageHandler.ts[639-669]
src/node/handler/PadMessageHandler.ts[600-614]
src/node/handler/PadMessageHandler.ts[639-668]
src/node/handler/PadMessageHandler.ts[678-682]
src/node/handler/PadMessageHandler.ts[1592-1598]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
On a CLIENT_READY move, the handler leaves the previous room and commits the new session before the message hooks accept the request. If a hook drops or throws, the socket does not complete the new join.

## Fix Focus Areas
- src/node/handler/PadMessageHandler.ts[600-615]
- src/node/handler/PadMessageHandler.ts[639-669]

## Recommended Fix
Run the message hooks before committing the pending join or leaving the previous room. Once the hooks accept the request, recheck the join generation and session identity, then perform the room transition before starting the handshake.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 Overlapping CLIENT_READY on one socket can join two pad rooms

✅ REVIEW.md

Review mode: Auto: 🧠 Deep: Dense concurrency guards across many handler paths in socket join logic.

Tip of the day

💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗