From dd474a2d6bf3255111034de9a2a96ac6f4d42aee Mon Sep 17 00:00:00 2001 From: Georgy Butaev <41178744+g-but@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:24:00 +0200 Subject: [PATCH] feat(mentions): mentioning a person finally tells them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `mention` notification type has existed since the notification config was written. It has copy, an icon case in NotificationItem.tsx, and a place in the type union. Nothing has EVER created one. You could write @alice and she would never know — which makes the mention syntax decorative rather than social, and it has been that way the whole time. Fixing it needs exactly what the Cat already uses: notice a mention on a post, resolve it properly, act, retry on failure. So rather than build a second pipeline beside the first, the existing one widens by one word. cat_mention_queue becomes mention_queue and claim_cat_mentions becomes claim_mentions, because the Cat is one mentioned account among many and the name should say so. The prefilter widens with it — from '@cat' to '@' — since a post naming only @alice has to reach the worker too. It is still a PREFILTER and still deliberately dumb. domain/mentions/parse.ts and services/mentions/resolve.ts remain the single authority on what counts as a mention; the worker discards whatever the trigger over-selects, and marks it done rather than failed, because retrying a post that named nobody three times and then logging an error is noise about nothing. One resolve now produces both outcomes: reply if the Cat was named, and tell the people who were. Two features, one query, one definition of what a mention is. MENTIONS IN PRIVATE MESSAGES ARE DELIBERATELY NOT NOTIFIED, and that is a privacy decision rather than an omission. A participant already gets a new_message notification, so a second one for being named is noise. A NON-participant must never be told at all: the notification would disclose that a conversation exists, who is in it, and through the preview part of what was said. Typing a friend's handle in a private chat is not publishing to them. The notification quotes the post rather than sending a bare "you were mentioned", skips the author naming themselves, skips the Cat (it has no inbox), and never throws — the post is already written by then, and losing a notification must not cost the Cat's reply. Co-Authored-By: Claude Opus 5 --- .../unit/services/cat-mention-queue.test.ts | 18 +-- .../unit/services/cat-post-mentions.test.ts | 58 ++++++--- .../services/cat-worker-bootstrap.test.ts | 16 +-- .../unit/services/notify-mentions.test.ts | 113 ++++++++++++++++++ src/services/mentions/note-mention.ts | 4 +- src/services/mentions/notify-mentions.ts | 99 +++++++++++++++ src/services/mentions/queue.ts | 39 +++--- src/services/mentions/worker.ts | 70 +++++++---- ...60826180000_mention_queue_all_mentions.sql | 107 +++++++++++++++++ 9 files changed, 449 insertions(+), 75 deletions(-) create mode 100644 __tests__/unit/services/notify-mentions.test.ts create mode 100644 src/services/mentions/notify-mentions.ts create mode 100644 supabase/migrations/20260826180000_mention_queue_all_mentions.sql diff --git a/__tests__/unit/services/cat-mention-queue.test.ts b/__tests__/unit/services/cat-mention-queue.test.ts index 253364a92..20599cd86 100644 --- a/__tests__/unit/services/cat-mention-queue.test.ts +++ b/__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 { diff --git a/__tests__/unit/services/cat-post-mentions.test.ts b/__tests__/unit/services/cat-post-mentions.test.ts index 29a619a97..4b788da8c 100644 --- a/__tests__/unit/services/cat-post-mentions.test.ts +++ b/__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); + }); }); diff --git a/__tests__/unit/services/cat-worker-bootstrap.test.ts b/__tests__/unit/services/cat-worker-bootstrap.test.ts index 6ae7a6e30..0260c7ad3 100644 --- a/__tests__/unit/services/cat-worker-bootstrap.test.ts +++ b/__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,7 +68,7 @@ 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 }); @@ -76,7 +76,7 @@ describe('the mention worker bootstraps the Cat', () => { 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 }); diff --git a/__tests__/unit/services/notify-mentions.test.ts b/__tests__/unit/services/notify-mentions.test.ts new file mode 100644 index 000000000..e67e5410a --- /dev/null +++ b/__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?' }) + ); + }); +}); diff --git a/src/services/mentions/note-mention.ts b/src/services/mentions/note-mention.ts index bbbc4d9c7..bf5da4a85 100644 --- a/src/services/mentions/note-mention.ts +++ b/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, diff --git a/src/services/mentions/notify-mentions.ts b/src/services/mentions/notify-mentions.ts new file mode 100644 index 000000000..8ad7e22ab --- /dev/null +++ b/src/services/mentions/notify-mentions.ts @@ -0,0 +1,99 @@ +/** + * Telling someone they were mentioned. + * + * The `mention` notification type has existed since the notification config was + * written: it has copy, an icon case in NotificationItem.tsx, and a place in the + * type union. Nothing has ever created one. Mentioning a person on OrangeCat + * notified nobody, which makes the mention syntax decorative — you can write + * `@alice` and she will never know. + * + * WHY ONLY PUBLIC POSTS + * Mentions inside a private conversation are deliberately NOT notified here, + * and that is a privacy decision rather than an omission: + * + * - a participant already gets a `new_message` notification, so a second one + * for being named in it is noise; + * - a NON-participant must never be told. The notification would disclose + * that a conversation exists, who is in it, and — through the preview — part + * of what was said. Someone typing a friend's handle in a private chat is + * not publishing to them. + * + * A post is public (or scoped by its own visibility), so notifying the people + * named in it is exactly what the author intended by naming them. + */ + +import { NotificationDispatcher } from '@/services/notifications/dispatcher'; +import { DATABASE_TABLES } from '@/config/database-tables'; +import { logger } from '@/utils/logger'; +import type { ResolvedMention } from '@/services/mentions/resolve'; +import type { SupabaseClient } from '@supabase/supabase-js'; + +export interface NotifyMentionsInput { + mentions: ResolvedMention[]; + /** Who wrote the post. Never notified about their own mention. */ + authorId: string; + /** The post the mention appears in, for the link. */ + eventId: string; + /** The post's text, trimmed for the notification body. */ + excerpt: string; +} + +/** How much of the post the notification quotes back. */ +const EXCERPT_LIMIT = 140; + +/** + * @returns how many people were notified. Never throws — a notification that + * fails must not cost the post or the Cat's reply. + */ +export async function notifyMentionedPeople( + admin: SupabaseClient, + input: NotifyMentionsInput +): Promise { + const recipients = input.mentions.filter( + // Not the Cat: it has no inbox and does not need telling. + // Not the author: naming yourself is not news. + mention => !mention.isCat && mention.id !== input.authorId + ); + + if (recipients.length === 0) { + return 0; + } + + const authorName = await displayName(admin, input.authorId); + const excerpt = input.excerpt.trim().slice(0, EXCERPT_LIMIT); + let sent = 0; + + for (const recipient of recipients) { + try { + await NotificationDispatcher.dispatch({ + userId: recipient.id, + type: 'mention', + title: `${authorName} mentioned you`, + message: excerpt || `${authorName} mentioned you in a post.`, + sourceEntityType: 'timeline_event', + sourceEntityId: input.eventId, + actionUrl: `/posts/${input.eventId}`, + data: { mentionerName: authorName, context: 'a post' }, + }); + sent += 1; + } catch (error) { + logger.error( + 'Failed to notify a mentioned person', + { recipient: recipient.id, error: error instanceof Error ? error.message : String(error) }, + 'Mentions' + ); + } + } + return sent; +} + +async function displayName(admin: SupabaseClient, userId: string): Promise { + const { data } = await admin + .from(DATABASE_TABLES.PROFILES) + .select('name, username') + .eq('id', userId) + .maybeSingle(); + + const row = data as { name: string | null; username: string | null } | null; + return row?.name?.trim() || row?.username || 'Someone'; +} diff --git a/src/services/mentions/queue.ts b/src/services/mentions/queue.ts index 580428b83..dfa5d1f95 100644 --- a/src/services/mentions/queue.ts +++ b/src/services/mentions/queue.ts @@ -1,14 +1,15 @@ /** - * The record that the Cat owes somebody an answer. + * The record that a post or message has mentions still to process. * - * Producers write here and return; the worker pays the debt. That split is what + * Producers write here and return; the worker does the work. That split is what * keeps an LLM round trip out of the sender's POST, and what stops a dying * process from swallowing a question — the worst outcome for an assistant is a - * request that vanishes with no reply and no error. + * request that vanishes with no reply and no error. The same durability is what + * makes a missed `@alice` notification a retry rather than a loss. * * Every function here is idempotent or atomic at the database, not in * JavaScript: the unique key on (source_type, source_id) makes enqueueing - * at-least-once safe, and `claim_cat_mentions` uses FOR UPDATE SKIP LOCKED so an + * at-least-once safe, and `claim_mentions` uses FOR UPDATE SKIP LOCKED so an * inline run and a timer tick can work the same queue without answering the same * mention twice. */ @@ -16,7 +17,7 @@ import { logger } from '@/utils/logger'; import type { SupabaseClient } from '@supabase/supabase-js'; -export const CAT_MENTION_QUEUE_TABLE = 'cat_mention_queue'; +export const MENTION_QUEUE_TABLE = 'mention_queue'; /** How many times a mention is retried before it is abandoned as failed. */ export const MAX_ATTEMPTS = 3; @@ -44,18 +45,18 @@ export interface ClaimedMention { } /** - * Record that a mention owes a reply. + * Record that a source has mentions to process. * * @returns true when the debt is recorded — including when it was already * recorded, because that is success, not failure. A duplicate insert is the * expected outcome of an at-least-once producer, and the unique constraint is * what makes it harmless. */ -export async function enqueueCatMention( +export async function enqueueMention( admin: SupabaseClient, input: EnqueueInput ): Promise { - const { error } = await admin.from(CAT_MENTION_QUEUE_TABLE).insert({ + const { error } = await admin.from(MENTION_QUEUE_TABLE).insert({ source_type: input.sourceType, source_id: input.sourceId, requester_id: input.requesterId, @@ -71,33 +72,33 @@ export async function enqueueCatMention( return true; } logger.error( - 'Could not queue a Cat mention', + 'Could not queue a mention', { sourceType: input.sourceType, sourceId: input.sourceId, error: error.message }, - 'CatMentionQueue' + 'MentionQueue' ); return false; } /** Atomically take up to `limit` pending mentions, marking them running. */ -export async function claimCatMentions( +export async function claimMentions( admin: SupabaseClient, limit: number ): Promise { - const { data, error } = await admin.rpc('claim_cat_mentions', { p_limit: limit }); + const { data, error } = await admin.rpc('claim_mentions', { p_limit: limit }); if (error) { - logger.error('Could not claim Cat mentions', { error: error.message }, 'CatMentionQueue'); + logger.error('Could not claim mentions', { error: error.message }, 'MentionQueue'); return []; } return (data ?? []) as ClaimedMention[]; } /** Mark a claimed mention answered. */ -export async function completeCatMention( +export async function completeMention( admin: SupabaseClient, id: string ): Promise { await admin - .from(CAT_MENTION_QUEUE_TABLE) + .from(MENTION_QUEUE_TABLE) .update({ status: 'done', finished_at: new Date().toISOString(), last_error: null }) .eq('id', id); } @@ -109,14 +110,14 @@ export async function completeCatMention( * it; abandons it as `failed` once they are exhausted. The error is kept either * way — a queue that discards why it gave up is a queue nobody can debug. */ -export async function failCatMention( +export async function failMention( admin: SupabaseClient, mention: ClaimedMention, reason: string ): Promise { const exhausted = mention.attempts >= MAX_ATTEMPTS; await admin - .from(CAT_MENTION_QUEUE_TABLE) + .from(MENTION_QUEUE_TABLE) .update({ status: exhausted ? 'failed' : 'pending', last_error: reason.slice(0, 500), @@ -126,9 +127,9 @@ export async function failCatMention( if (exhausted) { logger.error( - 'Gave up answering a Cat mention', + 'Gave up processing a mention', { id: mention.id, sourceId: mention.source_id, attempts: mention.attempts, reason }, - 'CatMentionQueue' + 'MentionQueue' ); } } diff --git a/src/services/mentions/worker.ts b/src/services/mentions/worker.ts index ffaf74aee..ba31f33b7 100644 --- a/src/services/mentions/worker.ts +++ b/src/services/mentions/worker.ts @@ -11,11 +11,12 @@ import { ensureCatAccount } from '@/services/mentions/cat-account'; import { replyToConversationMention } from '@/services/mentions/cat-reply'; import { replyToPostMention } from '@/services/mentions/cat-post-reply'; import { resolveMentions } from '@/services/mentions/resolve'; +import { notifyMentionedPeople } from '@/services/mentions/notify-mentions'; import { DATABASE_TABLES } from '@/config/database-tables'; import { - claimCatMentions, - completeCatMention, - failCatMention, + claimMentions, + completeMention, + failMention, type ClaimedMention, } from '@/services/mentions/queue'; import { logger } from '@/utils/logger'; @@ -45,7 +46,7 @@ export async function runCatMentions( // no-op, which is always after the first run. const cat = await ensureCatAccount(admin); - const claimed = await claimCatMentions(admin, limit); + const claimed = await claimMentions(admin, limit); result.claimed = claimed.length; if (claimed.length === 0) { return result; @@ -53,7 +54,7 @@ export async function runCatMentions( if (!cat) { for (const mention of claimed) { - await failCatMention(admin, mention, 'no Cat account'); + await failMention(admin, mention, 'no Cat account'); } result.failed = claimed.length; return result; @@ -63,14 +64,14 @@ export async function runCatMentions( try { const answered = await answer(admin, mention, cat.id); if (answered) { - await completeCatMention(admin, mention.id); + await completeMention(admin, mention.id); result.answered += 1; } else { - await failCatMention(admin, mention, 'nothing to answer'); + await failMention(admin, mention, 'nothing to answer'); result.failed += 1; } } catch (error) { - await failCatMention( + await failMention( admin, mention, error instanceof Error ? error.message : String(error) @@ -99,35 +100,58 @@ async function answer( } if (mention.parent_event_id) { - // The database trigger is a PREFILTER: it queues anything containing the - // substring "@cat", so `@catalogue` and `bob@catering.com` arrive here too. - // The resolver is the authority, and this is where its verdict is applied — - // detection has one implementation, not one per surface. - if (!(await postActuallyTagsTheCat(admin, mention.parent_event_id))) { - return true; // Nothing owed. Resolved, not failed. - } - return replyToPostMention(admin, { eventId: mention.parent_event_id, catId }); + return processPostMentions(admin, mention, catId); } return false; } -/** Ask the real resolver whether the post's text mentions the Cat. */ -async function postActuallyTagsTheCat( +/** + * One resolve, two outcomes. + * + * The trigger is a PREFILTER — it queues any post containing '@', so + * `bob@example.com` and `@catalogue` arrive here too. The resolver is the + * authority and this is the only place its verdict is applied, which is what + * keeps detection from being written once per surface. + * + * Both jobs come from that single answer: reply if the Cat was named, and tell + * the people who were. Nothing named at all is a resolved row, not a failure — + * marking it failed would retry a post that asked for nothing three times and + * then log an error about it. + */ +async function processPostMentions( admin: SupabaseClient, - eventId: string + mention: ClaimedMention, + catId: string ): Promise { + const eventId = mention.parent_event_id as string; + const { data } = await admin .from(DATABASE_TABLES.TIMELINE_EVENTS) - .select('title, description') + .select('title, description, actor_id') .eq('id', eventId) .maybeSingle(); if (!data) { return false; } - const row = data as { title: string | null; description: string | null }; + const row = data as { title: string | null; description: string | null; actor_id: string }; const text = `${row.description ?? ''}\n${row.title ?? ''}`; - const { mentionsCat } = await resolveMentions(admin, text); - return mentionsCat; + + const { mentions, mentionsCat } = await resolveMentions(admin, text); + if (mentions.length === 0) { + return true; + } + + await notifyMentionedPeople(admin, { + mentions, + authorId: row.actor_id, + eventId, + excerpt: row.description ?? row.title ?? '', + }); + + if (!mentionsCat) { + return true; + } + return replyToPostMention(admin, { eventId, catId }); } diff --git a/supabase/migrations/20260826180000_mention_queue_all_mentions.sql b/supabase/migrations/20260826180000_mention_queue_all_mentions.sql new file mode 100644 index 000000000..da1160ec0 --- /dev/null +++ b/supabase/migrations/20260826180000_mention_queue_all_mentions.sql @@ -0,0 +1,107 @@ +-- The queue is about mentions, not only about the Cat. +-- +-- `mention` notifications are defined in config/notification-config.ts, have +-- copy, have a UI case in NotificationItem.tsx and a type in the union — and +-- have NEVER ONCE BEEN CREATED. Mentioning a person on OrangeCat notifies +-- nobody. That is not a Cat problem; it is the social loop missing its most +-- basic feedback, and it has been missing since the type was written. +-- +-- Fixing it needs the same machinery the Cat already uses: notice a mention on +-- a post, resolve it properly, act on it, retry if that fails. So rather than +-- build a second pipeline beside the first, the existing one widens by one +-- word — and the name follows the meaning rather than the history. +-- +-- The prefilter widens with it. It looked for '@cat'; it now looks for '@' at +-- all, because a post mentioning only @alice has to reach the worker too. That +-- is still a PREFILTER and still deliberately dumb: domain/mentions/parse.ts +-- and services/mentions/resolve.ts remain the single authority on what counts +-- as a mention, and the worker discards whatever this over-selects. + +ALTER TABLE IF EXISTS public.cat_mention_queue RENAME TO mention_queue; + +ALTER INDEX IF EXISTS idx_cat_mention_queue_pending RENAME TO idx_mention_queue_pending; + +COMMENT ON TABLE public.mention_queue IS + 'One row per post or message that may contain mentions. Unique on (source_type, source_id) so an at-least-once producer still yields exactly one round of processing. Widened from cat_mention_queue 2026-08-26: the Cat is one mentioned account among many.'; + +-- The claim function follows the table. Same body, same SKIP LOCKED guarantee: +-- an inline run and a timer tick must never process the same row twice. +DROP FUNCTION IF EXISTS public.claim_cat_mentions(integer); + +CREATE OR REPLACE FUNCTION public.claim_mentions(p_limit integer DEFAULT 5) +RETURNS SETOF public.mention_queue + LANGUAGE plpgsql + SET search_path TO 'public' + AS $$ +BEGIN + RETURN QUERY + UPDATE mention_queue q + SET status = 'running', + claimed_at = now(), + attempts = q.attempts + 1 + WHERE q.id IN ( + SELECT c.id + FROM mention_queue c + WHERE c.status = 'pending' + ORDER BY c.created_at + FOR UPDATE SKIP LOCKED + LIMIT GREATEST(p_limit, 1) + ) + RETURNING q.*; +END; +$$; + +COMMENT ON FUNCTION public.claim_mentions IS + 'Atomically claim pending mentions. SKIP LOCKED so an inline run and a timer tick never process the same row twice.'; + +GRANT EXECUTE ON FUNCTION public.claim_mentions(integer) TO service_role; + +-- --------------------------------------------------------------------------- +-- The trigger, widened +-- --------------------------------------------------------------------------- + +CREATE OR REPLACE FUNCTION public.note_mentions_on_timeline_event() +RETURNS trigger + LANGUAGE plpgsql SECURITY DEFINER + SET search_path TO 'public' + AS $$ +DECLARE + v_cat_id uuid; +BEGIN + -- Cheap exit for the overwhelming majority of posts. Anything past here is + -- resolved properly by the worker, so being generous costs one small row. + IF POSITION('@' IN COALESCE(NEW.description, '')) = 0 + AND POSITION('@' IN COALESCE(NEW.title, '')) = 0 THEN + RETURN NEW; + END IF; + + SELECT id INTO v_cat_id FROM profiles WHERE username = 'cat'; + + -- The Cat never processes its own posts: it must not answer itself, and it + -- must not notify people it named while answering someone else. + IF v_cat_id IS NOT NULL AND NEW.actor_id = v_cat_id THEN + RETURN NEW; + END IF; + + INSERT INTO mention_queue (source_type, source_id, requester_id, parent_event_id) + VALUES ('timeline_event', NEW.id, NEW.actor_id, NEW.id) + ON CONFLICT (source_type, source_id) DO NOTHING; + + RETURN NEW; +END; +$$; + +COMMENT ON FUNCTION public.note_mentions_on_timeline_event IS + 'Prefilter only: queues a post that MIGHT contain mentions. services/mentions/resolve.ts is the authority on whether it does.'; + +DROP TRIGGER IF EXISTS trg_note_cat_mention ON public.timeline_events; +DROP TRIGGER IF EXISTS trg_note_mentions ON public.timeline_events; + +CREATE TRIGGER trg_note_mentions + AFTER INSERT ON public.timeline_events + FOR EACH ROW + EXECUTE FUNCTION public.note_mentions_on_timeline_event(); + +DROP FUNCTION IF EXISTS public.note_cat_mention_on_timeline_event(); + +NOTIFY pgrst, 'reload schema';