fix(authors): action Qodo review — lastSeen, flag-gating, defensive payloads

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>

admin/src/App.tsx

@@ -12,6 +12,7 @@ import {UpdateBanner} from "./components/UpdateBanner";
 const WS_URL = import.meta.env.DEV ? 'http://localhost:9001' : ''
 export const App = () => {
   const setSettings = useStore(state => state.setSettings);
+  const erasureEnabled = useStore(state => state.gdprAuthorErasureEnabled)
   const {t} = useTranslation()
   const navigate = useNavigate()
   const [sidebarOpen, setSidebarOpen] = useState<boolean>(true)
@@ -106,7 +107,9 @@ export const App = () => {
           <li><NavLink to={"/help"}> <Construction/> <Trans i18nKey="admin_plugins_info"/></NavLink></li>
           <li><NavLink to={"/pads"}><NotepadText/><Trans
             i18nKey="ep_admin_pads:ep_adminpads2_manage-pads"/></NavLink></li>
-          <li><NavLink to={"/authors"}><Users/><Trans i18nKey="ep_admin_authors:title" ns="ep_admin_authors"/></NavLink></li>
+          {erasureEnabled && (
+            <li><NavLink to={"/authors"}><Users/><Trans i18nKey="ep_admin_authors:title" ns="ep_admin_authors"/></NavLink></li>
+          )}
           <li><NavLink to={"/shout"}><PhoneCall/>Communication</NavLink></li>
           <li><NavLink to={"/update"}><Bell/><Trans i18nKey="update.page.title"/></NavLink></li>
         </ul>

src/node/db/AuthorManager.ts

@@ -130,8 +130,11 @@ const mapAuthorWithDBKey = async (mapperkey: string, mapper:string) => {
   }
 
   // there is an author with this mapper
-  // update the timestamp of this author
-  await db.setSub(`globalAuthor:${author}`, ['timestamp'], Date.now());
+  // update the timestamp + lastSeen of this author so the admin
+  // /authors "Last seen" column reflects the most recent connect
+  const now = Date.now();
+  await db.setSub(`globalAuthor:${author}`, ['timestamp'], now);
+  await db.setSub(`globalAuthor:${author}`, ['lastSeen'], now);
 
   // return the author
   return {authorID: author};
@@ -374,11 +377,15 @@ exports.anonymizeAuthor = async (
   // Zero the display identity now — without the `erased` sentinel — so a
   // partial run still hides the name. The sentinel itself is only set at
   // the end (below) so a failure in chat scrub lets the next call resume.
+  // Preserve `lastSeen` so the admin /authors UI's column stays accurate
+  // for erased records (the operator can still see when the author was
+  // last active before erasure).
   if (!dryRun) {
     await db.set(`globalAuthor:${authorID}`, {
       colorId: 0,
       name: null,
       timestamp: Date.now(),
+      lastSeen: existing.lastSeen ?? existing.timestamp ?? null,
       padIDs: existing.padIDs || {},
     });
   }
@@ -414,6 +421,7 @@ exports.anonymizeAuthor = async (
       colorId: 0,
       name: null,
       timestamp: Date.now(),
+      lastSeen: existing.lastSeen ?? existing.timestamp ?? null,
       padIDs: existing.padIDs || {},
       erased: true,
       erasedAt: new Date().toISOString(),
@@ -513,7 +521,11 @@ exports.searchAuthors = async (query: {
       name: rec.name ?? null,
       colorId: rec.colorId ?? null,
       mapper: mappers,
-      lastSeen: typeof rec.lastSeen === 'number' ? rec.lastSeen : null,
+      // Prefer lastSeen; fall back to timestamp for legacy records that
+      // pre-date the new field so the admin /authors column isn't blank.
+      lastSeen: typeof rec.lastSeen === 'number'
+          ? rec.lastSeen
+          : (typeof rec.timestamp === 'number' ? rec.timestamp : null),
       erased,
     });
   }

src/node/hooks/express/adminsettings.ts

@@ -309,8 +309,25 @@ exports.socketio = (hookName: string, {io}: any) => {
 
     const authorManager = require('../../db/AuthorManager');
 
-    socket.on('authorLoad', async (query: any) => {
+    // The admin author-erasure UI (PR #7667) is gated as a single
+    // feature: when gdprAuthorErasure.enabled is false, all three
+    // socket handlers refuse so the page is fully off by default per
+    // project rule "new features behind a feature flag, disabled by
+    // default" (Qodo Compliance ID 6). The destructive
+    // anonymizeAuthor stays gated as before; the read paths
+    // (authorLoad / preview) are also gated so listing data isn't
+    // exposed without an explicit opt-in.
+    const erasureEnabled = () =>
+        !!(settings.gdprAuthorErasure && settings.gdprAuthorErasure.enabled);
+
+    socket.on('authorLoad', async (payload: any) => {
       try {
+        if (!erasureEnabled()) {
+          socket.emit('results:authorLoad',
+              {total: 0, results: [], error: 'disabled'});
+          return;
+        }
+        const query = payload || {};
         const data = await authorManager.searchAuthors({
           pattern: query.pattern || '',
           offset: query.offset || 0,
@@ -327,8 +344,14 @@ exports.socketio = (hookName: string, {io}: any) => {
       }
     });
 
-    socket.on('anonymizeAuthorPreview', async ({authorID}: {authorID: string}) => {
+    socket.on('anonymizeAuthorPreview', async (payload: any) => {
+      const authorID = payload?.authorID;
       try {
+        if (!erasureEnabled()) {
+          socket.emit('results:anonymizeAuthorPreview',
+              {authorID, error: 'disabled'});
+          return;
+        }
         if (!authorID) {
           socket.emit('results:anonymizeAuthorPreview',
               {authorID, error: 'authorID is required'});
@@ -346,9 +369,10 @@ exports.socketio = (hookName: string, {io}: any) => {
       }
     });
 
-    socket.on('anonymizeAuthor', async ({authorID}: {authorID: string}) => {
+    socket.on('anonymizeAuthor', async (payload: any) => {
+      const authorID = payload?.authorID;
       try {
-        if (!settings.gdprAuthorErasure || !settings.gdprAuthorErasure.enabled) {
+        if (!erasureEnabled()) {
           socket.emit('results:anonymizeAuthor', {authorID, error: 'disabled'});
           return;
         }

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

@@ -144,8 +144,12 @@ describe(__filename, function () {
         }
       });
 
-  it('anonymizeAuthorPreview still works when flag is off (read-only)',
+  it('anonymizeAuthorPreview returns {error: "disabled"} when flag is off',
       async function () {
+        // Per Qodo Compliance ID 6 ('new features behind a feature flag,
+        // disabled by default') the preview event is also gated, not just
+        // the live anonymizeAuthor. The page renders its disabled banner
+        // off the socket reply when this fires.
         settings.gdprAuthorErasure.enabled = false;
         try {
           const tag = `prev-off-${Date.now()}`;
@@ -153,9 +157,41 @@ describe(__filename, function () {
               `m-${tag}`, `PrevOff ${tag}`);
           const preview = await ask(socket, 'anonymizeAuthorPreview',
               {authorID}, 'results:anonymizeAuthorPreview');
-          assert.ok(preview.removedExternalMappings >= 1);
+          assert.equal(preview.error, 'disabled');
+          assert.equal(preview.removedExternalMappings, undefined,
+              'no counters should leak when the flag is off');
         } finally {
           settings.gdprAuthorErasure.enabled = true;
         }
       });
+
+  it('authorLoad returns {error: "disabled"} when flag is off',
+      async function () {
+        settings.gdprAuthorErasure.enabled = false;
+        try {
+          const res = await ask(socket, 'authorLoad',
+              {pattern: '', offset: 0, limit: 12, sortBy: 'name',
+               ascending: true, includeErased: false},
+              'results:authorLoad');
+          assert.equal(res.error, 'disabled');
+          assert.deepEqual(res.results, []);
+        } finally {
+          settings.gdprAuthorErasure.enabled = true;
+        }
+      });
+
+  it('handlers do not crash on payload-less emits',
+      async function () {
+        // Pre-Qodo-fix the destructure `({authorID}: ...)` threw before
+        // try/catch when client emitted with no payload. Both gated
+        // handlers now accept `payload: any` and read defensively.
+        const previewRes = await ask(socket, 'anonymizeAuthorPreview',
+            undefined, 'results:anonymizeAuthorPreview');
+        assert.ok(previewRes.error,
+            `expected error, got ${JSON.stringify(previewRes)}`);
+        const eraseRes = await ask(socket, 'anonymizeAuthor',
+            undefined, 'results:anonymizeAuthor');
+        assert.ok(eraseRes.error,
+            `expected error, got ${JSON.stringify(eraseRes)}`);
+      });
 });

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

@@ -14,8 +14,10 @@ test('Shows troubleshooting page manager', async ({page}) => {
   await page.goto('http://localhost:9001/admin/help')
   await page.waitForSelector('.menu')
   const menu =  page.locator('.menu');
-  // Sidebar nav: plugins, settings, help, pads, authors, shout, update.
-  await expect(menu.locator('li')).toHaveCount(7);
+  // Sidebar nav: plugins, settings, help, pads, shout, update.
+  // The Authors link only renders when gdprAuthorErasure.enabled = true,
+  // which the test environment leaves false by default.
+  await expect(menu.locator('li')).toHaveCount(6);
 })
 
 test('Shows a version number', async function ({page}) {