fix(admin): don't let one unreadable pad empty the Manage-pads list (#7935)

[JohnMcLear]

Closes #7935.

Symptom

Pads exist (visible on the welcome page, returned by the listAllPads API) but the admin Manage pads UI shows none. Reported on 3.3.0 with Docker + Traefik + PostgreSQL 18.

Root cause

The welcome-page "recent pads" list is browser localStorage, so it reflects pads this browser opened — not the DB. The admin list comes from the /settings socket padLoad handler → listAllPads() → findKeys('pad:*', '*:*:*').

The admin's default lastEdited sort forces a full scan, hydrating every pad via getPad(). findKeys('pad:*','*:*:*') returns every pad:-prefixed key, including legacy/foreign/migration-corrupted records. A record whose stored value is a non-object (a JSON string — exactly what a botched dirty.db→Postgres migration produces) makes Pad.init throw Cannot use 'in' operator to search for 'pool' (Pad.ts:591).

The handler had no try/catch, so a single bad record:

1. emitted nothing → the SPA showed an empty "No results" list forever (the reported symptom), and
2. produced an unhandled rejection that reaches server.ts → process.exit — taking the whole server down.

Verified against a real PostgreSQL 18.4 instance: ueberdb2 6.1.8 findKeys, listAllPads, and getStats all work correctly there. PG18 is not the cause — the trigger is one unreadable pad:* record, which can happen on any DB backend.

Fix

• loadMeta wraps getPad() in try/catch: a failed read logs a warning and returns zeroed metadata, so the bad pad surfaces for deletion instead of hiding every other pad.
• The whole padLoad handler is wrapped so it always emits a terminal results:padLoad (the SPA can't hang) and never bubbles to the process-level rejection handler (the server can't exit from this path).

Tests

• backend — tests/backend/specs/admin/padLoadResilience.ts: injects a corrupt pad:* record next to a readable pad and asserts the listing still returns both. Fails without the fix (no results:padLoad reply within 10s + server Exiting), passes with it.
• e2e — tests/frontend-new/admin-spec/admin_pads_page.spec.ts: create/open a pad → it appears in the welcome-page recent-pads list → it appears in the /admin Manage-pads UI.

Local gates: 7/7 admin backend specs pass (5 existing + 2 new), both Playwright admin specs pass, ts-check clean.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

Code Review by Qodo

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

1. Unreadable pads can't delete ✓ Resolved 🐞 Bug ≡ Correctness

Description

padLoad now returns unreadable pad IDs with zeroed metadata, but the existing deletePad flow only
deletes pads that pass doesPadExists() (requires value.atext) and can be hydrated via getPad(), so
migration-corrupted pad records stored as strings will remain listed but undeletable and will not
emit any delete result.

Code

src/node/hooks/express/adminsettings.ts[R175-196]

  const loadMeta = async (padName: string): Promise<PadQueryResult> => {
-        const pad = await padManager.getPad(padName);
-        return {
-          padName,
-          lastEdited: await pad.getLastEdit(),
-          userCount: api.padUsersCount(padName).padUsersCount,
-          revisionNumber: pad.getHeadRevisionNumber(),
-        };
+        // A single unreadable record must not take out the whole listing.
+        // `findKeys('pad:*', '*:*:*')` returns every key under the `pad:`
+        // prefix, including legacy/foreign or migration-corrupted records
+        // (e.g. a value stored as a JSON *string* rather than a pad object,
+        // which makes Pad.init throw `'pool' in value`). Before this guard
+        // one such key rejected the whole `padLoad` handler — the admin
+        // "Manage pads" page then showed *no* pads at all (issue #7935) and
+        // the unhandled rejection could exit the server. Surfacing the bad
+        // pad with zeroed metadata lets an admin see and delete it instead.
+        try {
+          const pad = await padManager.getPad(padName);
+          return {
+            padName,
+            lastEdited: await pad.getLastEdit(),
+            userCount: api.padUsersCount(padName).padUsersCount,
+            revisionNumber: pad.getHeadRevisionNumber(),
+          };
+        } catch (err) {
+          logger.warn(`padLoad: skipping unreadable pad "${padName}": ${err}`);
+          return {padName, lastEdited: 0 as any, userCount: 0, revisionNumber: 0};
+        }

Evidence

The new loadMeta catch explicitly returns a result row for unreadable pads so they appear in the
Manage-pads list, but deletion relies on doesPadExists() (which checks value.atext) and then
hydrates via getPad(); a corrupt record stored as a string will fail the value.atext check and
thus cannot be deleted (and even attempting getPad() is known to throw when the stored value is
not an object).

src/node/hooks/express/adminsettings.ts[175-196]
src/node/hooks/express/adminsettings.ts[286-294]
src/node/db/PadManager.ts[155-164]
src/node/db/Pad.ts[584-592]

Agent prompt

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

## Issue description
`padLoad` now surfaces unreadable pads (zeroed metadata) so admins can delete them, but `deletePad` currently refuses to delete if `doesPadExists()` returns false (which happens for non-object corrupt records) and otherwise attempts `getPad()` (which throws for unreadable records). This makes the surfaced corrupt pad effectively undeletable via the admin UI.
### Issue Context
- Corrupt records can be present under `pad:*` keys and trigger `Pad.init` failures.
- The admin UI’s delete action goes through the `/settings` socket `deletePad` handler.
### Fix Focus Areas
- Update delete handler to support deleting unreadable/corrupt pad records by key/prefix without requiring hydration (`getPad`).
- Ensure the handler always emits a terminal `results:deletePad` reply (success or error) so the UI does not silently do nothing.
- Consider using `padManager.removePad(padId)` (updates padList) and, if needed, a targeted prefix cleanup for `pad:${padId}:revs:*`, `pad2readonly:*`, `readonly2pad:*`, etc., when hydration is impossible.
- src/node/hooks/express/adminsettings.ts[175-196]
- src/node/hooks/express/adminsettings.ts[286-294]
- src/node/db/PadManager.ts[155-205]
- src/node/db/Pad.ts[584-592]

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

---

2. Logs may expose corrupt value ✓ Resolved 🐞 Bug ⛨ Security

Description

The new error handling logs ${err} for unreadable pads and for padLoad failures; for the reported
corruption mode, the thrown error message includes the raw stored value (the corrupt JSON string),
which can leak sensitive pad content into logs and cause log spam if the value is large.

Code

src/node/hooks/express/adminsettings.ts[R193-195]

+        } catch (err) {
+          logger.warn(`padLoad: skipping unreadable pad "${padName}": ${err}`);
+          return {padName, lastEdited: 0 as any, userCount: 0, revisionNumber: 0};

Evidence

Pad.init uses 'pool' in value, which throws when the DB value is a non-object (the documented
corrupt-record scenario). The new code logs the caught error via template-string interpolation, and
the regression test description explicitly notes the thrown message includes ... in , implying the
stored string value can be included in the logged message.

src/node/hooks/express/adminsettings.ts[193-196]
src/node/hooks/express/adminsettings.ts[274-281]
src/node/db/Pad.ts[584-592]
src/tests/backend/specs/admin/padLoadResilience.ts[7-19]

Agent prompt

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

## Issue description
`padLoad` now catches errors but logs them via string interpolation (``${err}``). For the corruption case described (non-object value under `pad:${id}`), the thrown message can include the raw stored value, which may be a large JSON string (potentially containing pad content). Logging it verbatim can leak sensitive data and bloat logs.
### Issue Context
- The failure originates from `Pad.init` when the stored value is not an object.
- Both the per-pad catch and the outer handler catch log the raw error.
### Fix Focus Areas
- Log only safe fields (`err.name`, a sanitized/truncated `err.message`) and strip newlines.
- Avoid including the raw corrupt value in logs; if needed, include a short hash/length instead.
- Apply the same sanitization to both the per-pad warning and the outer handler error log.
- src/node/hooks/express/adminsettings.ts[193-196]
- src/node/hooks/express/adminsettings.ts[274-281]
- src/node/db/Pad.ts[584-592]
- src/tests/backend/specs/admin/padLoadResilience.ts[7-19]

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

---

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

PR Summary by Qodo

Fix admin pad listing when a single pad record is unreadable
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Walkthroughs

Description

• Make /settings padLoad resilient to unreadable pad:* records and always reply
• Surface corrupt pads with zeroed metadata so admins can delete them
• Add backend + Playwright regression coverage for Manage pads listing

Diagram

graph TD
  A["Admin Manage pads UI"] --> B["/settings: padLoad handler"] --> C["padManager.listAllPads()"] --> D{"Needs full scan?"}
  D -->|"No"| G["emit results:padLoad (always)"]
  D -->|"Yes"| E["loadMeta() per pad"] --> F["getPad() (try/catch)"] --> G
  B -.-> H["log warn/error"]
  F -.-> H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Validate/clean pad:* keys at storage layer (DB/ueberdb2)

• ➕ Centralizes corruption handling so other call sites also become resilient
• ➕ Potentially prevents bad keys from being enumerated at all
• ➖ Broader blast radius across backends and more difficult to test safely
• ➖ Requires careful definition of what constitutes a valid pad record

2. Pre-filter candidate pads using the pad index only (avoid findKeys fallout)

• ➕ Reduces exposure to legacy/foreign keys that share the pad:* prefix
• ➕ Can reduce work for lastEdited/full scans depending on implementation
• ➖ May not fully eliminate unreadable records if the index itself is polluted
• ➖ Still needs defensive handling for reads (getPad can fail for other reasons)

3. Background integrity check + admin cleanup UX

• ➕ Proactively surfaces and optionally repairs/removes corrupt records
• ➕ Keeps runtime handler simpler by moving remediation out-of-band
• ➖ More product/ops work (scheduling, reporting, permissions)
• ➖ Does not immediately prevent current runtime failures without a guard

Recommendation: Keep the PR’s approach: defensive isolation in loadMeta() plus a top-level try/catch that always emits results:padLoad. It directly fixes the user-facing hang and prevents process-level unhandled rejections with minimal scope. Consider a follow-up integrity/cleanup mechanism if corrupted pad:* records are common, but the runtime guard should remain regardless.

File Changes

Bug fix (1)

adminsettings.ts Harden /settings padLoad against unreadable pad records +31/-7

Harden /settings padLoad against unreadable pad records

• Wraps per-pad hydration (padManager.getPad) in try/catch to prevent one corrupt pad:* record from breaking the full listing. Adds a top-level try/catch around the padLoad handler to ensure results:padLoad is always emitted (including an error field) and to avoid bubbling to process-level unhandled rejection handling.

src/node/hooks/express/adminsettings.ts

---

Tests (2)

padLoadResilience.ts Backend regression tests for corrupt pad:* resilience +157/-0

Backend regression tests for corrupt pad:* resilience

• Adds a socket-level admin test that injects a corrupt pad:* value (non-object) alongside a normal pad and asserts both appear in padLoad results. Also verifies the fast path (padName sort) still replies promptly even with a corrupt pad present.

src/tests/backend/specs/admin/padLoadResilience.ts

---

admin_pads_page.spec.ts Playwright coverage for welcome recent pads vs admin listing +54/-0

Playwright coverage for welcome recent pads vs admin listing

• Introduces an end-to-end admin suite that creates/opens a pad, verifies it appears in the welcome page recent-pads list, then verifies it is searchable and visible in the /admin Manage pads UI without showing the empty-state regression.

src/tests/frontend-new/admin-spec/admin_pads_page.spec.ts

---

[JohnMcLear]

Thanks @qodo — both findings actioned:

1. Undeletable corrupt pads (4378466)
Valid — surfacing a corrupt pad is pointless if it can't be removed, and deletePad's normal path (doesPadExists → getPad → Pad.remove) can't touch a non-object record. deletePad now falls back to a raw key purge when doesPadExists() is false but a pad:${id} value is present: sweep sub-keys (revs/chat/deletionToken/…) via findKeys, drop the pad2readonly mapping, then padManager.removePad() (main key + pad-list + cache). It now always emits a terminal results:deletePad reply (including for an already-absent id) so the UI clears the row instead of silently doing nothing. Covered by a new backend test ("a surfaced corrupt pad can be deleted from the admin UI").

2. Logging the raw corrupt value (295ad8b)
Valid — the TypeError message embeds the stored value (…'pool' in <value>), which for a real pad can be document text. Added a safeErr() helper (error name + single-line, 120-char-capped message, control chars stripped) used in the per-pad warning, the outer handler error log, the deletePad error log, and the error field returned to the SPA — so no raw pad content, log bloat, or newline injection.

All admin backend specs pass (8: 5 existing + 3 new) and ts-check is clean.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →