Skip to content

fix(pad): remove deleted pads from the welcome screen recent list - #8237

Open
JohnMcLear wants to merge 2 commits into
developfrom
fix/8201-recent-pads-deleted
Open

JohnMcLear wants to merge 2 commits into
developfrom
fix/8201-recent-pads-deleted

Conversation

@JohnMcLear

@JohnMcLear JohnMcLear commented Sep 17, 2026

Copy link
Copy Markdown
Member

Fixes #8201

Root cause

The welcome page's "Recent pads" list is rendered purely from the recentPads entry in localStorage (src/static/skins/colibris/index.js). The pad page adds and updates entries (skins/colibris/pad.js, pad_userlist.ts), but nothing ever removed one. Deleting a pad (creator button or recovery token) redirected to the homepage, which still listed it, and clicking it recreated an empty pad.

Fix

In src/static/js/pad.ts, when the server sends {disconnect: 'deleted'} (broadcast to every socket on the pad by PadMessageHandler.deletePad), the client removes the current pad from recentPads before showing the disconnect modal. This handles:

  • the creator's Delete pad button,
  • deletion with a recovery token from another device,
  • any other open tab on the pad at deletion time.

The name is derived from the URL path, exactly as the entry was stored, and stored names are also compared after the same guarded decodeURIComponent the welcome screen applies (legacy encoded entries); storage access is wrapped in try/catch.

Out of scope: a pad deleted via the HTTP API or admin UI while this browser had no tab open will still be listed until visited, since the list is client-side only.

Tests

New Playwright spec src/tests/frontend-new/specs/recent_pads_delete.spec.ts:

  1. creator deletes via #delete-pad -> homepage recentPads no longer contains the pad and no .recent-pad link for it is rendered;
  2. second browser context deletes with the token -> same assertions on that device, and the creator's still-open tab also drops the pad;
  3. a legacy URL-encoded entry (FRONTEND_TESTS_notes%26ideas_..., as older versions stored) is removed as well (follow-up from Qodo review; fails without the name normalization).

Before the fix (develop), both failed:

Expected value: not "FRONTEND_TESTS54ac2859-..."
Received array:     ["FRONTEND_TESTS54ac2859-..."]

After the fix: all three pass on chromium and firefox. Related specs (recent_pads, pad_deletion_token, pad_settings, delete) pass on chromium (16/16).

Backend mocha: 1673 passing, 22 pending, 0 failing. Vitest: 840/840 passing. tsc --noEmit clean for the touched files.

🤖 Generated with Claude Code

https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi

When the server disconnects a pad with reason "deleted", remove that pad
from the recentPads localStorage list so the homepage stops showing it
(and stops offering a link that silently recreates it). Covers the
creator delete button, the recovery-token path, and other tabs that were
open on the pad.

Fixes #8201

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
@qodo-code-review

Copy link
Copy Markdown

ⓘ 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

Copy link
Copy Markdown

PR Summary by Qodo

Remove deleted pads from the welcome screen recent list

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove deleted pads from browser-local recent history after server deletion notifications.
• Prevent stale welcome links from silently recreating deleted pads.
• Cover creator, recovery-token, and open-tab deletion flows with Playwright tests.
Diagram

sequenceDiagram
  actor User
  participant Deleter as Deleting Tab
  participant Server as Pad Server
  participant Clients as Open Pad Tabs
  participant Storage as localStorage
  participant Home as Welcome Screen
  User->>Deleter: Delete pad
  Deleter->>Server: Delete request
  Server-->>Clients: disconnect deleted
  Clients->>Storage: Remove pad entry
  Clients-->>Home: Redirect
  Home->>Storage: Read recent pads
Loading
High-Level Assessment

The event-driven cleanup is the best fit for the existing client-local recent-pad design. Hooking the canonical server deletion broadcast covers creator, recovery-token, and other connected-tab cases; cleanup only in deletion controls would miss remote tabs, while server-backed history or homepage existence checks would add disproportionate complexity.

Files changed (2) +83 / -0

Bug fix (1) +20 / -0
pad.tsRemove deleted pads from local recent history +20/-0

Remove deleted pads from local recent history

• Adds defensive cleanup of the current pad from the recentPads localStorage entry when the server reports a deleted disconnect. It matches both the decoded URL name and runtime pad ID, while tolerating unavailable or malformed storage.

src/static/js/pad.ts

Tests (1) +63 / -0
recent_pads_delete.spec.tsTest recent-pad cleanup across deletion flows +63/-0

Test recent-pad cleanup across deletion flows

• Adds Playwright coverage for creator deletion and recovery-token deletion from a second browser context. The tests verify localStorage, welcome-screen rendering, and cleanup in another still-open tab receiving the deletion broadcast.

src/tests/frontend-new/specs/recent_pads_delete.spec.ts

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Encoded recent pads survive deletion ✓ Resolved 🐞 Bug ≡ Correctness
Description
forgetRecentPad() compares each raw stored p.name only with the decoded URL name and canonical
pad ID. When an older entry contains URL encoding such as notes%26ideas, deleting notes&ideas
removes any newer decoded entry but leaves the encoded entry available on the welcome screen.
Code

src/static/js/pad.ts[311]

+        recentPads.filter((p) => p == null || (p.name !== padName && p.name !== pad.getPadId()))));
Evidence
The deletion helper decodes the current URL name but compares it against the unmodified stored name.
The welcome-page reader explicitly documents that older stored names can already be URL-encoded and
decodes them before constructing links, while the current writer stores decoded names; therefore
both representations can exist and the legacy representation does not match the new filter.

src/static/js/pad.ts[302-311]
src/static/skins/colibris/index.js[73-82]
src/static/skins/colibris/pad.js[10-25]

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

## Issue description
Legacy `recentPads` entries may contain URL-encoded names, but deletion compares their raw names against decoded identifiers. This leaves a supported legacy entry visible after its pad is deleted.
## Fix Focus Areas
- src/static/js/pad.ts[302-314]
- src/static/skins/colibris/index.js[73-81]
## Recommended Fix
Normalize each non-null stored pad name before comparison, using the same guarded `decodeURIComponent` behavior as the welcome screen. Remove an entry when either its raw or normalized name matches `padName` or `pad.getPadId()`, while preserving safe handling for malformed percent encoding.

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


Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/static/js/pad.ts Outdated
Older versions stored URL-encoded pad names in recentPads; normalize each
stored name with the same guarded decodeURIComponent the welcome screen
uses before comparing, so those entries are removed too (Qodo review).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012kA75NPq8nGRidAwhPXeCi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Welcome screen not refreshed

1 participant