feat(mentions): mentioning a person finally tells them

__tests__/unit/services/cat-mention-queue.test.ts

@@ -9,7 +9,7 @@
  * and is treated as already queued. That bug was written and caught here.
  */
 
-import { enqueueCatMention, failCatMention, MAX_ATTEMPTS } from '@/services/mentions/queue';
+import { enqueueMention, failMention, MAX_ATTEMPTS } from '@/services/mentions/queue';
 import { noteCatMention } from '@/services/mentions/note-mention';
 
 function adminSpy(opts: { insertError?: { code?: string; message: string } } = {}) {
@@ -20,10 +20,10 @@ function adminSpy(opts: { insertError?: { code?: string; message: string } } = {
   return { admin: admin as never, insert, update };
 }
 
-describe('enqueueCatMention', () => {
+describe('enqueueMention', () => {
   it('records the debt', async () => {
     const { admin, insert } = adminSpy();
-    await expect(enqueueCatMention(admin, {
+    await expect(enqueueMention(admin, {
       sourceType: 'message', sourceId: 'm1', requesterId: 'u1', conversationId: 'c1',
     })).resolves.toBe(true);
     expect(insert).toHaveBeenCalledWith(expect.objectContaining({ source_id: 'm1' }));
@@ -33,29 +33,29 @@ describe('enqueueCatMention', () => {
     // 23505 = unique violation. An at-least-once producer firing twice must not
     // look like a failure, or the caller retries forever.
     const { admin } = adminSpy({ insertError: { code: '23505', message: 'duplicate key' } });
-    await expect(enqueueCatMention(admin, {
+    await expect(enqueueMention(admin, {
       sourceType: 'message', sourceId: 'm1', requesterId: 'u1', conversationId: 'c1',
     })).resolves.toBe(true);
   });
 
   it('reports a real failure as a failure', async () => {
     const { admin } = adminSpy({ insertError: { code: '42P01', message: 'no such table' } });
-    await expect(enqueueCatMention(admin, {
+    await expect(enqueueMention(admin, {
       sourceType: 'message', sourceId: 'm1', requesterId: 'u1', conversationId: 'c1',
     })).resolves.toBe(false);
   });
 });
 
-describe('failCatMention', () => {
+describe('failMention', () => {
   it('returns the mention to pending while attempts remain', async () => {
     const { admin, update } = adminSpy();
-    await failCatMention(admin, { attempts: 1 } as never, 'timeout');
+    await failMention(admin, { attempts: 1 } as never, 'timeout');
     expect(update).toHaveBeenCalledWith(expect.objectContaining({ status: 'pending' }));
   });
 
   it('abandons it once attempts are exhausted, keeping the reason', async () => {
     const { admin, update } = adminSpy();
-    await failCatMention(admin, { attempts: MAX_ATTEMPTS } as never, 'model unreachable');
+    await failMention(admin, { attempts: MAX_ATTEMPTS } as never, 'model unreachable');
     expect(update).toHaveBeenCalledWith(
       expect.objectContaining({ status: 'failed', last_error: 'model unreachable' })
     );
@@ -70,7 +70,7 @@ describe('noteCatMention', () => {
       insert,
       client: {
         from: (table: string) => {
-          if (table === 'cat_mention_queue') {
+          if (table === 'mention_queue') {
             return { insert };
           }
           return {

__tests__/unit/services/cat-post-mentions.test.ts

@@ -12,9 +12,10 @@
 
 const replyToPostMention = jest.fn().mockResolvedValue(true);
 const resolveMentions = jest.fn();
-const claimCatMentions = jest.fn();
-const completeCatMention = jest.fn();
-const failCatMention = jest.fn();
+const notifyMentionedPeople = jest.fn().mockResolvedValue(0);
+const claimMentions = jest.fn();
+const completeMention = jest.fn();
+const failMention = jest.fn();
 
 jest.mock('@/services/mentions/cat-account', () => ({
   ensureCatAccount: jest.fn().mockResolvedValue({ id: 'cat-1', username: 'cat' }),
@@ -28,10 +29,13 @@ jest.mock('@/services/mentions/cat-reply', () => ({
 jest.mock('@/services/mentions/resolve', () => ({
   resolveMentions: (...a: unknown[]) => resolveMentions(...a),
 }));
+jest.mock('@/services/mentions/notify-mentions', () => ({
+  notifyMentionedPeople: (...a: unknown[]) => notifyMentionedPeople(...a),
+}));
 jest.mock('@/services/mentions/queue', () => ({
-  claimCatMentions: (...a: unknown[]) => claimCatMentions(...a),
-  completeCatMention: (...a: unknown[]) => completeCatMention(...a),
-  failCatMention: (...a: unknown[]) => failCatMention(...a),
+  claimMentions: (...a: unknown[]) => claimMentions(...a),
+  completeMention: (...a: unknown[]) => completeMention(...a),
+  failMention: (...a: unknown[]) => failMention(...a),
   MAX_ATTEMPTS: 3,
 }));
 
@@ -51,21 +55,30 @@ const postMention = {
 const admin = (description: string) =>
   ({
     from: () => ({
-      select: () => ({ eq: () => ({ maybeSingle: () => Promise.resolve({ data: { title: null, description }, error: null }) }) }),
+      select: () => ({
+        eq: () => ({
+          maybeSingle: () =>
+            Promise.resolve({ data: { title: null, description, actor_id: 'u1' }, error: null }),
+        }),
+      }),
     }),
   }) as never;
 
 beforeEach(() => {
+  notifyMentionedPeople.mockClear().mockResolvedValue(0);
   replyToPostMention.mockClear().mockResolvedValue(true);
   resolveMentions.mockReset();
-  claimCatMentions.mockReset().mockResolvedValue([postMention]);
-  completeCatMention.mockReset();
-  failCatMention.mockReset();
+  claimMentions.mockReset().mockResolvedValue([postMention]);
+  completeMention.mockReset();
+  failMention.mockReset();
 });
 
 describe('wall-post mentions', () => {
   it('answers a post that really tags the Cat', async () => {
-    resolveMentions.mockResolvedValue({ mentions: [], mentionsCat: true });
+    resolveMentions.mockResolvedValue({
+      mentions: [{ id: 'cat-1', username: 'cat', isCat: true }],
+      mentionsCat: true,
+    });
     const result = await runCatMentions(admin('@cat is this goal realistic?'));
     expect(replyToPostMention).toHaveBeenCalledWith(expect.anything(), {
       eventId: 'e1',
@@ -89,16 +102,33 @@ describe('wall-post mentions', () => {
     await runCatMentions(admin('the @catalogue'));
     // Marking it failed would retry it three times and then log an error about
     // a post that never asked the Cat anything.
-    expect(completeCatMention).toHaveBeenCalled();
-    expect(failCatMention).not.toHaveBeenCalled();
+    expect(completeMention).toHaveBeenCalled();
+    expect(failMention).not.toHaveBeenCalled();
   });
 
   it('does not consult the resolver for a private-message mention', async () => {
-    claimCatMentions.mockResolvedValue([
+    claimMentions.mockResolvedValue([
       { ...postMention, conversation_id: 'c1', parent_event_id: null },
     ]);
     await runCatMentions(admin('irrelevant'));
     // Those arrive through an API route that already resolved them.
     expect(resolveMentions).not.toHaveBeenCalled();
   });
+
+  it('notifies the people a post names, even when the Cat is not among them', async () => {
+    // This is the case that never worked: `@alice` in a post told alice nothing,
+    // because the type existed and nothing ever created one.
+    resolveMentions.mockResolvedValue({
+      mentions: [{ id: 'u-alice', username: 'alice', isCat: false }],
+      mentionsCat: false,
+    });
+    const result = await runCatMentions(admin('thoughts on this @alice?'));
+
+    expect(notifyMentionedPeople).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({ eventId: 'e1', authorId: 'u1' })
+    );
+    expect(replyToPostMention).not.toHaveBeenCalled();
+    expect(result.answered).toBe(1);
+  });
 });

__tests__/unit/services/cat-worker-bootstrap.test.ts

@@ -14,15 +14,15 @@
  */
 
 const ensureCatAccount = jest.fn();
-const claimCatMentions = jest.fn();
+const claimMentions = jest.fn();
 
 jest.mock('@/services/mentions/cat-account', () => ({
   ensureCatAccount: (...a: unknown[]) => ensureCatAccount(...a),
 }));
 jest.mock('@/services/mentions/queue', () => ({
-  claimCatMentions: (...a: unknown[]) => claimCatMentions(...a),
-  completeCatMention: jest.fn(),
-  failCatMention: jest.fn(),
+  claimMentions: (...a: unknown[]) => claimMentions(...a),
+  completeMention: jest.fn(),
+  failMention: jest.fn(),
   MAX_ATTEMPTS: 3,
 }));
 jest.mock('@/services/mentions/cat-reply', () => ({
@@ -33,7 +33,7 @@ import { runCatMentions } from '@/services/mentions/worker';
 
 beforeEach(() => {
   ensureCatAccount.mockReset().mockResolvedValue({ id: 'cat-1', username: 'cat' });
-  claimCatMentions.mockReset().mockResolvedValue([]);
+  claimMentions.mockReset().mockResolvedValue([]);
 });
 
 describe('the mention worker bootstraps the Cat', () => {
@@ -50,7 +50,7 @@ describe('the mention worker bootstraps the Cat', () => {
       order.push('ensure');
       return { id: 'cat-1', username: 'cat' };
     });
-    claimCatMentions.mockImplementation(async () => {
+    claimMentions.mockImplementation(async () => {
       order.push('claim');
       return [];
     });
@@ -68,15 +68,15 @@ describe('the mention worker bootstraps the Cat', () => {
   });
 
   it('answers a claimed mention once the account exists', async () => {
-    claimCatMentions.mockResolvedValue([
+    claimMentions.mockResolvedValue([
       { id: 'q1', source_type: 'message', source_id: 'm1', requester_id: 'u1', conversation_id: 'c1', parent_event_id: null, attempts: 1 },
     ]);
     await expect(runCatMentions({} as never)).resolves.toMatchObject({ claimed: 1, answered: 1 });
   });
 
   it('fails claimed mentions rather than speaking as nobody', async () => {
     ensureCatAccount.mockResolvedValue(null);
-    claimCatMentions.mockResolvedValue([
+    claimMentions.mockResolvedValue([
       { id: 'q1', source_type: 'message', source_id: 'm1', requester_id: 'u1', conversation_id: 'c1', parent_event_id: null, attempts: 1 },
     ]);
     await expect(runCatMentions({} as never)).resolves.toMatchObject({ failed: 1, answered: 0 });

__tests__/unit/services/notify-mentions.test.ts

@@ -0,0 +1,113 @@
+/**
+ * Mentioning a person notified nobody.
+ *
+ * The `mention` notification type has existed since the notification config was
+ * written — copy, icon case, type union — and nothing has ever created one. That
+ * made the mention syntax decorative: you could write `@alice` and she would
+ * never know.
+ *
+ * The rules pinned here are the ones that decide whether the notification is
+ * welcome rather than noise, and one of them is a privacy rule rather than a
+ * courtesy: a mention inside a PRIVATE conversation is never notified, because
+ * telling a non-participant would disclose that the conversation exists, who is
+ * in it, and part of what was said.
+ */
+
+const dispatch = jest.fn().mockResolvedValue(undefined);
+jest.mock('@/services/notifications/dispatcher', () => ({
+  NotificationDispatcher: { dispatch: (...a: unknown[]) => dispatch(...a) },
+}));
+
+import { notifyMentionedPeople } from '@/services/mentions/notify-mentions';
+
+const admin = {
+  from: () => ({
+    select: () => ({
+      eq: () => ({ maybeSingle: () => Promise.resolve({ data: { name: 'Georgy', username: 'g' }, error: null }) }),
+    }),
+  }),
+} as never;
+
+const alice = { id: 'u-alice', username: 'alice', isCat: false };
+const cat = { id: 'u-cat', username: 'cat', isCat: true };
+
+beforeEach(() => dispatch.mockClear());
+
+describe('notifyMentionedPeople', () => {
+  it('tells a mentioned person, with a link to the post', async () => {
+    const sent = await notifyMentionedPeople(admin, {
+      mentions: [alice],
+      authorId: 'u-author',
+      eventId: 'e1',
+      excerpt: 'thoughts on this @alice?',
+    });
+
+    expect(sent).toBe(1);
+    expect(dispatch).toHaveBeenCalledWith(
+      expect.objectContaining({
+        userId: 'u-alice',
+        type: 'mention',
+        title: 'Georgy mentioned you',
+        actionUrl: '/posts/e1',
+      })
+    );
+  });
+
+  it('does not notify the Cat — it has no inbox', async () => {
+    await notifyMentionedPeople(admin, {
+      mentions: [cat],
+      authorId: 'u-author',
+      eventId: 'e1',
+      excerpt: '@cat what do you think?',
+    });
+    expect(dispatch).not.toHaveBeenCalled();
+  });
+
+  it('does not notify you about naming yourself', async () => {
+    await notifyMentionedPeople(admin, {
+      mentions: [alice],
+      authorId: 'u-alice',
+      eventId: 'e1',
+      excerpt: 'as @alice I would say',
+    });
+    expect(dispatch).not.toHaveBeenCalled();
+  });
+
+  it('notifies each mentioned person once', async () => {
+    const bob = { id: 'u-bob', username: 'bob', isCat: false };
+    const sent = await notifyMentionedPeople(admin, {
+      mentions: [alice, bob, cat],
+      authorId: 'u-author',
+      eventId: 'e1',
+      excerpt: '@alice @bob @cat',
+    });
+    expect(sent).toBe(2);
+    expect(dispatch).toHaveBeenCalledTimes(2);
+  });
+
+  it('never throws when a notification fails', async () => {
+    // A post is already written by this point. Losing a notification is bad;
+    // losing the Cat's reply because a notification failed would be worse.
+    dispatch.mockRejectedValueOnce(new Error('smtp down'));
+    await expect(
+      notifyMentionedPeople(admin, {
+        mentions: [alice],
+        authorId: 'u-author',
+        eventId: 'e1',
+        excerpt: 'hi @alice',
+      })
+    ).resolves.toBe(0);
+  });
+
+  it('quotes the post rather than sending a bare "you were mentioned"', async () => {
+    await notifyMentionedPeople(admin, {
+      mentions: [alice],
+      authorId: 'u-author',
+      eventId: 'e1',
+      excerpt: '  is this funding goal realistic @alice?  ',
+    });
+    expect(dispatch).toHaveBeenCalledWith(
+      expect.objectContaining({ message: 'is this funding goal realistic @alice?' })
+    );
+  });
+});

src/services/mentions/note-mention.ts

@@ -12,7 +12,7 @@ import { CAT_USERNAME } from '@/config/cat-identity';
 import { DATABASE_TABLES } from '@/config/database-tables';
 import { normalizeUsername } from '@/config/usernames';
 import { resolveMentions } from '@/services/mentions/resolve';
-import { enqueueCatMention } from '@/services/mentions/queue';
+import { enqueueMention } from '@/services/mentions/queue';
 import { logger } from '@/utils/logger';
 import type { SupabaseClient } from '@supabase/supabase-js';
 
@@ -58,7 +58,7 @@ export async function noteCatMention(
       return false;
     }
 
-    return await enqueueCatMention(admin, {
+    return await enqueueMention(admin, {
       sourceType: 'message',
       sourceId: input.messageId,
       requesterId: input.senderId,