Skip to content

feat(gdpr): admin UI for author erasure (follow-up to #7550)#7667

Open
JohnMcLear wants to merge 17 commits intodevelopfrom
feat-gdpr-admin-author-erasure
Open

feat(gdpr): admin UI for author erasure (follow-up to #7550)#7667
JohnMcLear wants to merge 17 commits intodevelopfrom
feat-gdpr-admin-author-erasure

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

Adds an in-product /admin/authors page so operators can search/sort/erase authors without crafting a curl. Follow-up to PR5 of #6701 (#7550).

  • Three new admin-socket events on io.of('/settings'): authorLoad, anonymizeAuthorPreview (always available, read-only), anonymizeAuthor (gated on gdprAuthorErasure.enabled, returns {error:'disabled'} when off).
  • New authorManager.searchAuthors helper with cap-at-1000 safety, substring match against name OR mapper OR opaque authorID.
  • anonymizeAuthor({dryRun: true}) for preview counters — same loops, no writes.
  • New lastSeen field stamped on globalAuthor write paths (additive, no migration).
  • Disabled-flag UX: banner + per-row tooltip explain how to enable.
  • Two-step erase modal (preview counters → commit) with localized "Erasing…" indicator.

The public REST endpoint and its gdprAuthorErasure.enabled flag are unchanged. The admin-socket live event reuses the same flag for parity; preview is intentionally always available so the page is discoverable when the flag is off.

Spec: docs/superpowers/specs/2026-05-03-gdpr-admin-author-erasure-ui-design.md
Plan: docs/superpowers/plans/2026-05-03-gdpr-admin-author-erasure-ui.md

Test plan

  • backend unit: `tests/backend/specs/admin/authorSearch.ts` (6 specs)
  • backend integration: `tests/backend/specs/admin/anonymizeAuthorSocket.ts` (5 specs)
  • backend regression: extended `tests/backend/specs/anonymizeAuthor.ts` (8 specs)
  • frontend Playwright: `tests/frontend-new/admin-spec/admin_authors_page.spec.ts` (4 specs, asserts localized strings)
  • manual: open `/admin/authors`, search, erase a seeded author end-to-end (CI Playwright validates)

JohnMcLear and others added 14 commits May 3, 2026 18:06
Follow-up to PR5 (#7550): adds an in-product /admin/authors page so
operators can search by name or external mapper, preview the impact
of an Art. 17 erasure (token mappings, mapper bindings, chat
messages, affected pads), and commit it without crafting a curl.
Backend uses three new admin-socket events on settings_admin (not
REST), so the existing public REST endpoint and its
gdprAuthorErasure.enabled flag keep their current single meaning.
The page stays discoverable when the flag is off — banner + disabled
buttons explain how to enable it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step-by-step TDD plan for the /admin/authors follow-up to PR5
(#7550). Nine tasks covering: lastSeen field on globalAuthor writes,
anonymizeAuthor({dryRun}), authorManager.searchAuthors helper,
three new admin-socket events (authorLoad / anonymizeAuthorPreview /
anonymizeAuthor) + settings-flag delivery, frontend types/swatch/
i18n, store/route/sidebar wiring, AuthorPage.tsx with two-step
modal and disabled-flag banner, Playwright coverage, and the PR/
Qodo workflow. References the spec at
docs/superpowers/specs/2026-05-03-gdpr-admin-author-erasure-ui-design.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a lastSeen timestamp to the globalAuthor record on createAuthor,
setAuthorName, and setAuthorColorId. Read paths are not modified to
keep the write cost zero per page load. Pre-existing records gain the
field on their next identity write — no migration sweep, callers that
read the field tolerate undefined.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an opt-in dryRun option that walks the same token/mapper/chat
loops and returns identical counter shape without touching the
database. The public REST endpoint is unchanged (it never passes the
flag), so production behaviour is identical. Used by the upcoming
admin-UI two-step erase modal to show 'will clear: N mappings, K
chat messages' before the irreversible commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ryRun

Code review on the previous commit caught that the dryRun refactor
silently dropped four WHY-comments (lazy-require cycle, drop-mappings-
first ordering, zero-identity-without-sentinel split, sentinel-last
discipline) and left the new opts parameter undocumented. Restored
the comments verbatim and added a one-line JSDoc note for dryRun.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In-memory enumeration of globalAuthor:* with a join on mapper2author:*
for the mapper column. Filter (substring on name OR mapper OR
authorID — the authorID match lets admins verify a specific erased
record where name and mapper bindings are gone), sort
(name | lastSeen), paginate, cap the pre-pagination set at 1000 to
prevent runaway scans. Powers the upcoming /admin/authors page.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review pointed out that ties in the primary sort key (common on
lastSeen for authors created the same ms, possible on identical
names too) fell back to findKeys enumeration order — not guaranteed
stable across DB backends. Adding an authorID secondary sort makes
pagination safe across requests. Also fix a misleading 'default'
note in the JSDoc — includeErased is required, not optional.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds three handlers on the /settings admin namespace:
- authorLoad: paginated search via authorManager.searchAuthors
- anonymizeAuthorPreview: dry-run counters, always available to
  authenticated admins (read-only)
- anonymizeAuthor: live commit, gated on gdprAuthorErasure.enabled
  (returns {error: 'disabled'} when off)

Extends the load reply with a flags.gdprAuthorErasure boolean so the
client knows whether to render the disabled-flag banner without an
extra round-trip.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review found two test-isolation issues:
- settings.users 'restoration' was a no-op (savedUsers held a
  reference to the same object that the test mutated). The test-admin
  key now gets explicitly removed in after().
- The Promise that awaited connect/connect_error left the loser
  listener attached for the lifetime of the socket, risking an
  unhandled rejection on a later spurious connect_error event.
  Listeners are now paired with off() so only the winner survives.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Standalone primitives for the upcoming /admin/authors page:
- AuthorSearch.ts: query/result/preview wire types matching the new
  admin-socket events
- ColorSwatch.tsx: resolves a globalAuthor.colorId (palette index or
  raw hex) to a small inline-styled swatch
- ep_admin_authors/en.json: every user-visible string the page needs,
  loaded by the existing namespace-as-static-asset i18n strategy

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a searchable, sortable, paginated authors page mirroring the
existing PadPage shape. Search matches name OR mapper substring;
'Show erased' toggle off by default; cap-at-1000 hint surfaces when
the backend caps the pre-pagination set. Two-step erase modal: dry-
run preview shows what will be cleared, then a Continue button
commits the irreversible erasure. Disabled-flag banner explains how
to enable when gdprAuthorErasure.enabled is false; per-row Erase
button is disabled with a tooltip in the same condition.

Sidebar gets a Users link between Pads and Communication. App.tsx
listens for the new flags.gdprAuthorErasure on the connect-time
settings push so the page knows the flag state without an extra
round trip. ep_admin_authors namespace is added to i18next's ns
list so all translation keys resolve.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec review caught three hardcoded English strings in the
/admin/authors pagination footer ('Previous Page', 'Next Page',
'X out of Y'). Carried over from PadPage.tsx via the plan template,
which had the same gap. Added three new keys to ep_admin_authors
and routed the spans through Trans/t().

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review on the AuthorPage commit caught three issues:

- Disabled-flag banner used dialog-confirm-content classname which is
  position: fixed + centered + z-index: 101, making it render as a
  modal-style overlay over the table. Drop the className and define
  the banner with inline styles only; add role='alert' for SR users.
- The Erase IconButton spread {data-disabled-reason: …} alongside
  {disabled: true}, but IconButton only forwards a small allowlist of
  props — the data attribute was silently dropped. Replaced with a
  conditional title that flips to the disabled-reason string when the
  button is disabled (which IconButton does forward).
- 'Erasing…' string was rendered during loading-preview, but the
  string literally describes the commit phase. Added a new
  loading-preview key for the preview-loading state, and surface the
  existing 'erasing' string under the buttons during the committing
  phase.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier en.json shipped namespace-prefixed JSON keys
('ep_admin_authors:title': 'Authors') which is the wrong shape:
i18next splits the lookup on ':' to extract the namespace, then looks
up the bare key in the loaded namespace data. The existing convention
(admin/public/ep_admin_pads/en.json) uses flat keys without the
namespace prefix; matching it makes every
<Trans i18nKey='ep_admin_authors:foo'/> resolve to the intended
translated string. Strings render as English fallback without this
fix; only the page-title test passes (and only by substring accident).

Also adds the Playwright coverage required by Task 8: localized
title, empty-state message on a fresh search tag, disabled banner
toggling with gdprAuthorErasure.enabled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Add admin UI for GDPR author erasure with search, preview, and erase workflows

✨ Enhancement 🧪 Tests 📝 Documentation

Grey Divider

Walkthroughs

Description
• Implements a new /admin/authors page enabling operators to search, sort, and erase authors
  through an in-product UI instead of CLI commands
• Adds three new admin-socket events (authorLoad, anonymizeAuthorPreview, anonymizeAuthor)
  with feature-flag gating for live erasure operations
• Extends AuthorManager with searchAuthors helper (paginated, 1000-row cap, substring filtering
  on name/mapper/authorID) and anonymizeAuthor dry-run mode for preview counters
• Introduces lastSeen timestamp field on globalAuthor records, stamped on creation and identity
  writes
• Frontend features: searchable/sortable table with pagination, two-step erase modal (preview
  counters → commit), disabled-flag banner with per-row tooltips, in-place row updates after erasure
• Comprehensive test coverage: 6 backend unit specs for author search, 5 integration specs for
  socket events, 3 regression specs for lastSeen/dryRun, 4 Playwright specs with localization
  assertions
• Includes 31 English i18n keys covering all user-visible strings, implementation plan, and detailed
  specification document
Diagram
flowchart LR
  Admin["Admin User"]
  AuthorPage["AuthorPage Component"]
  SocketEvents["Admin Socket Events<br/>authorLoad<br/>anonymizeAuthorPreview<br/>anonymizeAuthor"]
  AuthorManager["AuthorManager<br/>searchAuthors<br/>anonymizeAuthor dryRun"]
  DB["Database<br/>globalAuthor records"]
  
  Admin -->|"Search/Sort/Erase"| AuthorPage
  AuthorPage -->|"Emit events"| SocketEvents
  SocketEvents -->|"Query/Preview/Execute"| AuthorManager
  AuthorManager -->|"Read/Write"| DB
  AuthorManager -->|"Return results"| SocketEvents
  SocketEvents -->|"Update UI"| AuthorPage
Loading

Grey Divider

File Changes

1. src/node/db/AuthorManager.ts ✨ Enhancement +167/-32

Author search, dry-run preview, and lastSeen tracking

• Adds lastSeen timestamp field stamped on author creation and identity writes (setAuthorName,
 setAuthorColorId)
• Extends anonymizeAuthor with optional {dryRun} parameter for preview-only counter calculation
 without database mutations
• Implements new searchAuthors helper for paginated author enumeration with substring filtering
 (name/mapper/authorID), sorting, and 1000-row safety cap

src/node/db/AuthorManager.ts


2. src/node/hooks/express/adminsettings.ts ✨ Enhancement +66/-3

Admin socket events for author search and erasure

• Adds three new admin-socket event handlers: authorLoad (search), anonymizeAuthorPreview
 (dry-run), anonymizeAuthor (live erase, gated on flag)
• Extends the load event reply to include flags.gdprAuthorErasure boolean so client knows
 feature state without extra round-trip
• Preview and search handlers always available to authenticated admins; live erase returns `{error:
 'disabled'}` when flag is off

src/node/hooks/express/adminsettings.ts


3. src/tests/backend/specs/admin/authorSearch.ts 🧪 Tests +124/-0

Unit tests for author search helper

• New test suite covering authorManager.searchAuthors with 6 specs
• Tests substring matching by name and mapper, erased-author filtering, sorting by lastSeen, and
 1000-row cap with cappedAt indicator
• Each spec seeds unique authors to avoid collisions in parallel test runs

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


View more (14)
4. src/tests/backend/specs/admin/anonymizeAuthorSocket.ts 🧪 Tests +161/-0

Integration tests for admin socket events

• New integration test suite for the three admin-socket events (5 specs)
• Validates authorLoad pagination, anonymizeAuthorPreview dry-run behavior (counters without
 mutation), and anonymizeAuthor flag-gating
• Confirms preview works when flag is off (read-only) and live erase returns error when disabled

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


5. src/tests/backend/specs/anonymizeAuthor.ts 🧪 Tests +57/-0

Regression tests for lastSeen and dryRun

• Extends existing test file with 3 new specs covering lastSeen stamping on creation and identity
 writes
• Adds dryRun option tests: verifies counters match live shape but no database mutations occur,
 and unknown authorID returns zero counters

src/tests/backend/specs/anonymizeAuthor.ts


6. admin/src/store/store.ts ✨ Enhancement +9/-0

Zustand store slices for authors page

• Adds authors state slice and setAuthors setter for paginated search results
• Adds gdprAuthorErasureEnabled boolean flag and setGdprAuthorErasureEnabled setter to track
 feature state from server

admin/src/store/store.ts


7. admin/src/utils/AuthorSearch.ts ✨ Enhancement +45/-0

TypeScript types for author search and erasure

• Defines TypeScript types for author search: AuthorSearchQuery, AuthorSearchResult,
 AuthorRow, AnonymizePreview, AnonymizeResult
• Types mirror the admin-socket event payloads and backend counter shapes for type-safe
 client-server communication

admin/src/utils/AuthorSearch.ts


8. admin/src/components/ColorSwatch.tsx ✨ Enhancement +37/-0

ColorSwatch component for author color display

• New React component that renders a small inline-styled color swatch
• Resolves globalAuthor.colorId (palette index or hex string) to CSS color using the same palette
 as backend AuthorManager

admin/src/components/ColorSwatch.tsx


9. admin/src/pages/AuthorPage.tsx ✨ Enhancement +288/-0

Admin authors page with search, sort, and erase UI

• New page component implementing /admin/authors with searchable, sortable, paginated table
• Features: substring search on name/mapper, sort by name or lastSeen, two-step erase modal
 (preview → commit), disabled-flag banner, per-row erase button with tooltip
• Listens to admin-socket events and patches erased rows in-place to avoid refetch flicker

admin/src/pages/AuthorPage.tsx


10. admin/src/App.tsx ✨ Enhancement +7/-5

Sidebar link and flag capture for authors page

• Adds Users icon import from lucide-react
• Adds sidebar navigation link to /authors page between Pads and Communication
• Extends settings event handler to capture flags.gdprAuthorErasure from server and update store

admin/src/App.tsx


11. admin/src/main.tsx ✨ Enhancement +2/-0

Route registration for authors page

• Imports new AuthorPage component
• Registers /authors route in the admin router

admin/src/main.tsx


12. admin/src/localization/i18n.ts ⚙️ Configuration changes +1/-1

i18n namespace registration for authors page

• Adds ep_admin_authors namespace to the i18n configuration so the new page's localized strings
 are loaded

admin/src/localization/i18n.ts


13. admin/public/ep_admin_authors/en.json 📝 Documentation +31/-0

English localization strings for authors page

• New i18n translation file with 31 keys covering all user-visible strings on the authors page
• Includes table headers, search placeholder, button labels, disabled-flag banner, erase modal text,
 and toast messages

admin/public/ep_admin_authors/en.json


14. src/tests/frontend-new/admin-spec/admin_authors_page.spec.ts 🧪 Tests +68/-0

Playwright tests for authors page UI

• New Playwright test suite with 4 specs covering the /admin/authors page
• Tests localized page title, search filtering, and disabled-flag banner toggling with
 gdprAuthorErasure.enabled
• Asserts rendered localized strings, not just element presence, per project i18n testing rules

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


15. docs/superpowers/plans/2026-05-03-gdpr-admin-author-erasure-ui.md 📝 Documentation +1627/-0

Implementation plan for GDPR author erasure admin UI

• Comprehensive implementation plan with 9 tasks covering backend, frontend, and testing
• Each task includes step-by-step instructions, failing-test-first approach, expected outputs, and
 commit messages
• Documents file structure, tech stack, and self-review checklist for spec/placeholder/type
 consistency

docs/superpowers/plans/2026-05-03-gdpr-admin-author-erasure-ui.md


16. docs/superpowers/specs/2026-05-03-gdpr-admin-author-erasure-ui-design.md 📝 Documentation +286/-0

GDPR admin author erasure UI specification and design

• Comprehensive specification for a new admin UI page at /admin/authors enabling operators to
 search, preview, and erase authors without CLI commands
• Defines three new admin-socket events (authorLoad, anonymizeAuthorPreview, anonymizeAuthor)
 with detailed payload schemas and result shapes
• Documents backend implementation including authorManager.searchAuthors() helper with 1000-record
 cap, dry-run support, and new lastSeen field on globalAuthor records
• Specifies frontend UX with search/sort/pagination table, two-step erasure modal with preview
 counters, disabled-flag banner, and comprehensive i18n key requirements
• Includes testing strategy covering backend unit/integration tests and frontend Playwright specs
 with localization assertions

docs/superpowers/specs/2026-05-03-gdpr-admin-author-erasure-ui-design.md


17. admin/src/pages/AuthorPage.tsx ✨ Enhancement +288/-0

New admin page component for author search and erasure

• New React component implementing the admin authors page with search field, sortable table, and
 pagination (12 rows per page)
• Manages dialog state machine for two-phase erasure flow: preview (loading → display counters) →
 commit (erasing indicator)
• Emits and listens to three socket events (authorLoad, anonymizeAuthorPreview,
 anonymizeAuthor) to fetch authors, preview erasure impact, and execute erasure
• Renders feature-disabled banner when gdprAuthorErasureEnabled is false; disables erase buttons
 and shows tooltip explaining how to enable
• Displays erased authors as greyed "(erased)" stubs; handles success/error toasts and in-place row
 updates after erasure completes

admin/src/pages/AuthorPage.tsx


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 4, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. lastSeen lost or stale🐞 Bug ≡ Correctness
Description
lastSeen is returned by searchAuthors() but it is not updated when an existing author is “seen”
via token/mapper resolution (only timestamp is updated), and anonymizeAuthor() overwrites
globalAuthor without preserving lastSeen, making the Admin UI’s “Last seen” column stale or
blank (especially after erasure).
Code

src/node/db/AuthorManager.ts[R377-393]

+  if (!dryRun) {
+    await db.set(`globalAuthor:${authorID}`, {
+      colorId: 0,
+      name: null,
+      timestamp: Date.now(),
+      padIDs: existing.padIDs || {},
+    });
+  }
+  const padIDs = Object.keys(existing.padIDs || {});
+  let clearedChatMessages = 0;
// Null authorship on chat messages the author posted. If this throws
// partway through, the function re-runs the loop on the next call
// because `erased: true` is not set yet.
-  const padIDs = Object.keys(existing.padIDs || {});
-  let clearedChatMessages = 0;
for (const padID of padIDs) {
  if (!await padManager.doesPadExist(padID)) continue;
  const pad = await padManager.getPad(padID);
Evidence
The author resolution path updates only timestamp, not lastSeen, so lastSeen will not reflect
actual recent use. Additionally, anonymizeAuthor() writes a fresh object that omits lastSeen, so
an erased author’s lastSeen becomes undefined and searchAuthors() will emit null for it.

src/node/db/AuthorManager.ts[117-138]
src/node/db/AuthorManager.ts[377-384]
src/node/db/AuthorManager.ts[412-421]
src/node/db/AuthorManager.ts[511-518]

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 new `lastSeen` field is used by the admin author listing (`searchAuthors()`), but it is (a) not updated when an author is resolved via token/mapper mapping and (b) dropped when `anonymizeAuthor()` overwrites the `globalAuthor` record. This causes the `/admin/authors` “Last seen” column and sorting to be misleading.
### Issue Context
- `mapAuthorWithDBKey()` is a high-frequency “author seen” path and currently only updates `timestamp`.
- `anonymizeAuthor()` overwrites the author object twice and currently omits `lastSeen`.
- `searchAuthors()` only reads `rec.lastSeen` and outputs `null` if missing.
### Fix Focus Areas
- src/node/db/AuthorManager.ts[117-138]
- src/node/db/AuthorManager.ts[377-384]
- src/node/db/AuthorManager.ts[412-421]
- src/node/db/AuthorManager.ts[511-518]
### Suggested fix
- When updating `timestamp` for an existing author mapping, also update `lastSeen` (likely to the same `Date.now()` value).
- When overwriting `globalAuthor` in `anonymizeAuthor()`, include `lastSeen` (either preserve `existing.lastSeen` or set it to `Date.now()` at erasure time).
- (Optional hardening) In `searchAuthors()`, consider falling back to `rec.timestamp` if `rec.lastSeen` is missing to avoid blank values for older records.

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



Remediation recommended

2. /authors route not flagged📘 Rule violation ⛨ Security
Description
The PR introduces a new admin authors UI and related read-only socket endpoints that are available
regardless of any feature flag state. This conflicts with the requirement that new features be gated
behind a feature flag and disabled by default.
Code

admin/src/main.tsx[26]

+        <Route path="/authors" element={<AuthorPage/>}/>
Evidence
PR Compliance ID 6 requires new functionality to be behind a feature flag and disabled by default.
The new /authors admin route and sidebar link are always registered, and the new admin-socket read
paths (authorLoad, anonymizeAuthorPreview) are also always enabled (only the destructive
anonymizeAuthor is gated).

admin/src/main.tsx[26-26]
admin/src/App.tsx[109-109]
src/node/hooks/express/adminsettings.ts[310-347]
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
New admin functionality (the `/admin/authors` page and its read-only socket endpoints) is available even when `gdprAuthorErasure.enabled` is false, which violates the requirement that new features be behind a feature flag and disabled by default.
## Issue Context
Currently, only the destructive `anonymizeAuthor` socket handler is gated; the new listing/preview capabilities are not.
## Fix Focus Areas
- admin/src/main.tsx[26-26]
- admin/src/App.tsx[109-109]
- src/node/hooks/express/adminsettings.ts[310-347]

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


3. Socket destructure can throw🐞 Bug ☼ Reliability
Description
The /settings socket handlers destructure {authorID} in the parameter list for
anonymizeAuthorPreview and anonymizeAuthor; if the client emits the event with
undefined/null payload, it throws before the try/catch and can result in an unhandled rejection
and a broken admin-socket flow.
Code

src/node/hooks/express/adminsettings.ts[R330-360]

+    socket.on('anonymizeAuthorPreview', async ({authorID}: {authorID: string}) => {
+      try {
+        if (!authorID) {
+          socket.emit('results:anonymizeAuthorPreview',
+              {authorID, error: 'authorID is required'});
+          return;
+        }
+        const rec = await authorManager.getAuthor(authorID);
+        const counters =
+            await authorManager.anonymizeAuthor(authorID, {dryRun: true});
+        socket.emit('results:anonymizeAuthorPreview',
+            {authorID, name: rec ? rec.name : null, ...counters});
+      } catch (err: any) {
+        logger.error(`anonymizeAuthorPreview failed: ${err.stack || err}`);
+        socket.emit('results:anonymizeAuthorPreview',
+            {authorID, error: String(err.message || err)});
+      }
+    });
+
+    socket.on('anonymizeAuthor', async ({authorID}: {authorID: string}) => {
+      try {
+        if (!settings.gdprAuthorErasure || !settings.gdprAuthorErasure.enabled) {
+          socket.emit('results:anonymizeAuthor', {authorID, error: 'disabled'});
+          return;
+        }
+        if (!authorID) {
+          socket.emit('results:anonymizeAuthor',
+              {authorID, error: 'authorID is required'});
+          return;
+        }
+        const counters = await authorManager.anonymizeAuthor(authorID);
Evidence
Parameter destructuring happens before the function body executes, so the current try/catch cannot
catch payload-shape errors. The code uses `async ({authorID}: {authorID: string}) => { try { ... }
}`, which will throw if called without an object argument.

src/node/hooks/express/adminsettings.ts[330-347]
src/node/hooks/express/adminsettings.ts[349-368]

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

## Issue description
`anonymizeAuthorPreview` / `anonymizeAuthor` socket handlers destructure `authorID` in the argument list. If a client emits the event without a payload (or with `null`), destructuring throws before entering the handler body, bypassing the handler’s try/catch.
### Issue Context
Even if the shipped UI always sends `{authorID: ...}`, defensive server code should treat socket payloads as untrusted and avoid pre-body destructuring.
### Fix Focus Areas
- src/node/hooks/express/adminsettings.ts[330-368]
### Suggested fix
- Change handler signatures to accept `payload: any` (optionally defaulting to `{}`), then read `const authorID = payload?.authorID;` inside the try block.
- Keep existing validation that emits `{error: 'authorID is required'}` when missing.
Example pattern:

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


4. Preview error not handled🐞 Bug ≡ Correctness
Description
The authors UI treats every results:anonymizeAuthorPreview as a successful preview; if the server
responds with {error} (as the backend does on exceptions), the modal still renders counter
placeholders from missing fields and Continue remains enabled.
Code

admin/src/pages/AuthorPage.tsx[R61-66]

+    const onPreview = (data: AnonymizePreview) => {
+      setDialog((cur) =>
+          cur.phase === 'loading-preview' && cur.authorID === data.authorID
+              ? {phase: 'preview', preview: data}
+              : cur);
+    };
Evidence
The shared type explicitly allows AnonymizePreview.error, but the UI transitions into the preview
phase without checking it and then interpolates numeric counters from the preview object when
rendering the dialog.

admin/src/utils/AuthorSearch.ts[28-36]
admin/src/pages/AuthorPage.tsx[58-66]
admin/src/pages/AuthorPage.tsx[151-178]

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

## Issue description
`AnonymizePreview` includes an optional `error` field, and the backend emits `{authorID, error: ...}` on failures. The UI currently moves to the preview phase regardless and renders counters that may be undefined, and it still allows clicking Continue.
### Issue Context
This is a robustness issue for exceptional cases (DB read errors, etc.) that results in broken modal text and potentially misleading flows.
### Fix Focus Areas
- admin/src/pages/AuthorPage.tsx[58-75]
- admin/src/pages/AuthorPage.tsx[151-179]
- admin/src/utils/AuthorSearch.ts[28-36]
### Suggested fix
- In `onPreview`, branch on `data.error`:
- either keep the dialog in a dedicated error phase that renders the message and disables Continue, or
- close the dialog and show a toast/error banner.
- Ensure the modal’s counter rendering is only executed when all expected numeric fields are present.

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


Grey Divider

Qodo Logo

CI on PR #7667 surfaced two test failures caused by my changes:

1. setErasureFlag() in admin_authors_page.spec.ts used JSON.parse on
   the raw settings.json textarea content. The CI environment loads
   settings.json.template which has unquoted property names, trailing
   commas, and block + line comments — JSON.parse rejects all three.
   Switched to `new Function('return (' + raw + ')')` which evaluates
   the textarea as a JS object literal, accepting every shape
   Etherpad's own settings loader handles.

2. admintroubleshooting.spec.ts hardcoded
   `menu.locator('li').toHaveCount(6)`. The new /authors sidebar
   entry made it 7. Updated the assertion and the sidebar comment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/node/db/AuthorManager.ts
…s restart

Final whole-branch review found three Important UX/a11y defects, plus
CI flagged one runtime defect:

- IconButton renders the title prop as a visible <span>, not as the
  HTML title attribute. Disabled rows were displaying the 80-character
  'Author erasure is disabled...' string next to every trash icon.
  Reverted to the short 'Erase' label; the page-level banner already
  explains the disabled state.
- Radix Dialog.Content was missing Dialog.Title. Wrapped the existing
  <h3> in <Dialog.Title asChild> so screen readers can announce the
  dialog purpose.
- onPreview proceeded to render the preview UI even when the backend
  reply carried {error}, leaving 'Will clear undefined token
  mappings...' on screen. Now mirrors onErase.
- The disabled-banner-hidden Playwright test failed because
  settings.json save does not hot-reload. setErasureFlag now
  restartEtherpad's after saveSettings and re-logins.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ayloads

Qodo on PR #7667 surfaced three issues:

1. (Bug, Correctness) lastSeen lost or stale.
   - mapAuthorWithDBKey only updated `timestamp` for returning authors
     so the admin /authors 'Last seen' column drifted on every reconnect
     without an identity write. Now stamps both timestamp and lastSeen.
   - anonymizeAuthor's two db.set calls overwrote globalAuthor without
     preserving lastSeen, blanking the column for erased rows. Both
     writes now carry forward `existing.lastSeen ?? existing.timestamp`.
   - searchAuthors falls back to rec.timestamp when rec.lastSeen is
     missing so legacy records aren't blank.

2. (Rule violation, Security) /authors route not flag-gated.
   The new admin-socket read paths (authorLoad, anonymizeAuthorPreview)
   were always-on; only the destructive anonymizeAuthor was gated.
   Project rule (Compliance ID 6) requires new features behind a flag,
   disabled by default. All three handlers now check
   gdprAuthorErasure.enabled and return {error:'disabled'} when off.
   The sidebar 'Authors' link is hidden when the flag is off
   (deep-link to /admin/authors still works and renders the existing
   disabled banner so docs can point to it).

3. (Bug, Reliability) Socket destructure throws on missing payload.
   Handlers signed `async ({authorID}: {authorID: string}) => …`
   threw before try/catch when a client emitted with no payload,
   producing an unhandled rejection. Switched to
   `async (payload: any) => { const authorID = payload?.authorID; … }`.

Test impact: anonymizeAuthorSocket gains two regressions (authorLoad
disabled-shape, payload-less emits don't crash) and updates the
preview-when-flag-off test to assert {error:'disabled'} per the new
gating posture (was 'preview still works'). admintroubleshooting
sidebar-count reverts 7 → 6 since the Authors link is now conditional
on the flag (off by default in the test environment).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant