fix: wait for server confirmation before navigating after pad delete

[JohnMcLear]

Summary

• Client (pad_editor.ts): The delete pad handler now waits for the server's response before navigating:
  • Listens for {disconnect: 'deleted'} on the message event — navigates to / on success
  • Listens for the shout event — shows an alert when a non-creator tries to delete (server sends an error message instead of deleting)
  • 5-second timeout fallback — navigates to / if the server doesn't respond (socket dropped, etc.)
• Server (PadMessageHandler.ts): await the pad.remove() call so the deletion completes before kickSessionsFromPad sends the disconnect response.

Root Cause

The delete pad click handler called sendMessage then immediately set window.location.href = '/'. In Firefox (and mobile Chrome), the browser tears down the WebSocket connection during page navigation before the outgoing message is flushed. The message never reaches the server, so the pad is never deleted — but the user sees the home page and thinks it worked.

Additionally, when a non-creator tries to delete, the server sends a shoutMessage on the shout event (not a disconnect). The old code navigated away before this could be received, so the user never saw the error. Now the client listens for this and displays the message.

Test plan

• Backend: messages.ts tests pass (10/10)
• Backend: pre-existing pad API delete test failures confirmed on develop (not caused by this change)
• Manual (creator, Firefox): create pad, modify, Delete Pad → should delete and redirect
• Manual (creator, Chrome): same test → verify no regression
• Manual (non-creator): open someone else's pad, try Delete Pad → should see error alert, NOT navigate away
• Manual (timeout): disconnect network after clicking Delete Pad → should redirect after 5s

Fixes #7306
Fixes #7311

🤖 Generated with Claude Code

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

Review Summary by Qodo

Fix pad deletion race condition with server confirmation

🐞 Bug fix

Walkthroughs

Description

• Wait for server confirmation before navigating after pad deletion
• Prevents race condition where browser closes WebSocket prematurely
• Await pad.remove() on server to ensure deletion completes
• Fixes silent pad deletion failures in Firefox and mobile Chrome

Diagram

flowchart LR
  A["Client: Delete Pad Click"] --> B["Send PAD_DELETE Message"]
  B --> C["Listen for disconnect: deleted"]
  C --> D["Server: Await pad.remove()"]
  D --> E["Send disconnect Response"]
  E --> F["Client: Navigate to Home"]

Loading

File Changes

1. src/node/handler/PadMessageHandler.ts 🐞 Bug fix +1/-1

Await pad removal completion on server

• Added await keyword to retrievedPad.remove() call
• Ensures pad deletion completes before server sends disconnect response
• Prevents race condition where response sent before deletion finishes

src/node/handler/PadMessageHandler.ts

---

2. src/static/js/pad_editor.ts 🐞 Bug fix +9/-2

Wait for server confirmation before page navigation

• Moved window.location.href = '/' into message listener callback
• Now waits for server's {disconnect: 'deleted'} response before navigating
• Added explanatory comment about the race condition fix
• Prevents browser from closing WebSocket before delete message reaches server

src/static/js/pad_editor.ts

---

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

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Missing PAD_DELETE regression test 📘 Rule violation ⚙ Maintainability

Description

This PR changes pad deletion behavior on both the client and server but does not include an
automated regression test that would fail if the fix were reverted. Without test coverage, the
delete race condition (and related redirect behavior) can be reintroduced unnoticed.

Code

src/static/js/pad_editor.ts[R89-99]

+          // Wait for the server to confirm deletion before navigating away.
+          // Navigating immediately caused a race condition where the browser
+          // (especially Firefox) would close the WebSocket before the delete
+          // message reached the server. See #7306.
+          pad.socket.on('message', (data: any) => {
+            if (data && data.disconnect === 'deleted') {
+              window.location.href = '/';
+            }
+          });
          pad.collabClient.sendMessage({type: 'PAD_DELETE', data:{padId: pad.getPadId()}});
-          // redirect to home page after deletion
-          window.location.href = '/';
        }

Evidence

PR Compliance ID 4 requires a regression test to accompany bug fixes. The PR modifies the Delete Pad
flow (pad_editor.ts) and the server delete handler (PadMessageHandler.ts), but the backend
socket/message test suite (src/tests/backend/specs/messages.ts) contains no PAD_DELETE coverage,
so this fix is not protected by a regression test.

src/static/js/pad_editor.ts[89-99]
src/node/handler/PadMessageHandler.ts[230-265]
src/tests/backend/specs/messages.ts[57-258]
Best Practice: Repository guidelines

Agent prompt

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

## Issue description
The Delete Pad bug fix is not accompanied by a regression test, so the race-condition/redirect behavior can regress without being caught by CI.

## Issue Context
This PR changes the client to wait for `{disconnect: 'deleted'}` before navigating and changes the server to `await` pad removal. Add a deterministic automated test that fails if these changes are reverted.

## Fix Focus Areas
- src/tests/backend/specs/messages.ts[57-258]
- src/tests/backend/common.ts[254-264]

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

---

2. Stale delete redirect handler 🐞 Bug ☼ Reliability

Description

The delete click handler registers a new pad.socket.on('message', ...) listener every confirmed
click and never unregisters it, so the handler can persist after a denied/failed delete and redirect
later on any {disconnect:'deleted'} event. This can also accumulate multiple listeners if the user
retries, causing duplicate handlers and potential unexpected navigation behavior.

Code

src/static/js/pad_editor.ts[R89-98]

+          // Wait for the server to confirm deletion before navigating away.
+          // Navigating immediately caused a race condition where the browser
+          // (especially Firefox) would close the WebSocket before the delete
+          // message reached the server. See #7306.
+          pad.socket.on('message', (data: any) => {
+            if (data && data.disconnect === 'deleted') {
+              window.location.href = '/';
+            }
+          });
          pad.collabClient.sendMessage({type: 'PAD_DELETE', data:{padId: pad.getPadId()}});
-          // redirect to home page after deletion
-          window.location.href = '/';

Evidence

The listener is registered inside the click handler and is never removed. If the user is not the pad
creator, the server sends a shout event (not a {disconnect:'deleted'} message), so the redirect
listener never fires and remains registered for the rest of the session; later, if the pad is
deleted and a disconnect message is broadcast, the stale listener will redirect. The existing
pad-level socket message handler already handles obj.disconnect by showing a modal and
disconnecting, so this new redirect behavior should be carefully scoped and cleaned up to avoid
unintended redirects triggered by stale listeners.

src/static/js/pad_editor.ts[86-100]
src/node/handler/PadMessageHandler.ts[230-263]
src/static/js/pad.ts[303-346]
src/static/js/pad_connectionstatus.ts[54-86]

Agent prompt

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

### Issue description
`pad_editor.ts` adds a new `pad.socket.on('message', ...)` listener every time the user confirms pad deletion, but the listener is never removed. If deletion is denied (non-creator) or otherwise doesn’t result in `{disconnect: 'deleted'}`, the listener remains for the session and can later redirect unexpectedly when a `{disconnect:'deleted'}` occurs.

### Issue Context
- The server’s non-creator path emits a `shout` event (no `disconnect:'deleted'`), so the redirect handler will not fire and will remain installed.
- Socket message handling already exists in `pad.ts` and processes `obj.disconnect`.

### Fix Focus Areas
- src/static/js/pad_editor.ts[86-100]

### Suggested fix approach
1. Prevent listener accumulation by either:
  - Removing any previously-registered delete listener before adding a new one, and/or
  - Registering a single listener once (outside the click handler) and gating redirect behind a `deleteInProgress` flag.
2. Ensure cleanup:
  - Call `pad.socket.off('message', handler)` immediately after receiving `{disconnect:'deleted'}`.
  - Also clear the handler (or clear `deleteInProgress`) on known failure paths (e.g., if a denial `shout` is received) and/or after a reasonable timeout to avoid leaving stale behavior behind.

(Implementation note: follow the pattern used in `pad.ts` where a handler is removed via `socket.off('message', h)` once the relevant condition is met.)

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

---

ⓘ The new review experience is currently in Beta. Learn more

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

1. Missing pad_delete regression test 📘 Rule violation ⚙ Maintainability

This PR changes pad deletion behavior on both the client and server but does not include an
automated regression test that would fail if the fix were reverted. Without test coverage, the
delete race condition (and related redirect behavior) can be reintroduced unnoticed.

Agent Prompt

## Issue description
The Delete Pad bug fix is not accompanied by a regression test, so the race-condition/redirect behavior can regress without being caught by CI.

## Issue Context
This PR changes the client to wait for `{disconnect: 'deleted'}` before navigating and changes the server to `await` pad removal. Add a deterministic automated test that fails if these changes are reverted.

## Fix Focus Areas
- src/tests/backend/specs/messages.ts[57-258]
- src/tests/backend/common.ts[254-264]

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