From b37755b99b7af48890bf835d5830b17efe9f84cc Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 16:05:46 +0545 Subject: [PATCH 01/15] feat(OUT-3730): add send-task-reminders scheduled task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Daily 00:00 UTC cron that walks getEligibleReminders, fans out company- assigned rows to current members via getCompanyClients, and dispatches email-only notifications via sendReminderEmail. Idempotency lives in the ledger insert: a single batched INSERT ... ON CONFLICT (taskId, recipientId, reminderType) DO NOTHING RETURNING ... runs *before* any Copilot call, so retried cron runs and in-flight duplicates can never double-send. Only rows that come back from RETURNING are net-new and proceed to the send phase. On Copilot failure we DELETE the ledger row so the next cron run retries; a failing DELETE is logged distinctly so on-call can clean up the stuck row. Per-workspace CopilotAPI is minted from any task.createdById + workspaceId via encodePayload — same shape as cmd/backfill-missed-emails. Workspace bottleneck = 5 matches WORKSPACE_CONCURRENCY in auto-archive. allSettled keeps a failing workspace from aborting the sweep. IU rows are filtered in the cron rather than in the SQL to keep OUT-3736's contract untouched. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/index.ts | 1 + .../notifications/send-task-reminders.test.ts | 237 +++++++++++++++++ src/jobs/notifications/send-task-reminders.ts | 239 ++++++++++++++++++ 3 files changed, 477 insertions(+) create mode 100644 src/jobs/notifications/send-task-reminders.test.ts create mode 100644 src/jobs/notifications/send-task-reminders.ts diff --git a/src/jobs/notifications/index.ts b/src/jobs/notifications/index.ts index 1bbf77b3d..552e8240b 100644 --- a/src/jobs/notifications/index.ts +++ b/src/jobs/notifications/index.ts @@ -2,3 +2,4 @@ export { deleteTaskNotifications } from './delete-task-notifications' export { sendTaskCreateNotifications } from './send-task-create-notifications' export { sendTaskUpdateNotifications } from './send-task-update-notifications' export { sendCommentCreateNotifications } from './send-comment-create-notifications' +export { sendTaskReminders } from './send-task-reminders' diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts new file mode 100644 index 000000000..efc2597dd --- /dev/null +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -0,0 +1,237 @@ +import { AssigneeType, TaskReminderType } from '@prisma/client' + +// Mocks must be configured before requiring the SUT. Variables referenced inside the +// jest.mock factory must start with `mock` so the babel-jest allow-list lets the closure +// see them once the const declarations have run. +const mockQueryRaw = jest.fn() +const mockTaskFindMany = jest.fn() +const mockTaskReminderSentDelete = jest.fn() + +const mockGetEligibleReminders = jest.fn() +const mockSendReminderEmail = jest.fn() + +const mockGetWorkspace = jest.fn() +const mockGetCompanyClients = jest.fn() +const mockCopilotApiCtor = jest.fn() + +jest.mock('@trigger.dev/sdk/v3', () => ({ + schedules: { + task: ({ run }: { run: (payload: unknown, ctx?: unknown) => unknown }) => ({ run }), + }, + logger: { log: jest.fn(), error: jest.fn(), warn: jest.fn() }, +})) + +jest.mock('@/lib/db', () => ({ + __esModule: true, + default: { + getInstance: () => ({ + $queryRaw: mockQueryRaw, + task: { findMany: mockTaskFindMany }, + taskReminderSent: { delete: mockTaskReminderSentDelete }, + }), + }, +})) + +jest.mock('@/utils/crypto', () => ({ + encodePayload: jest.fn(() => 'stub-token'), +})) + +jest.mock('@/utils/CopilotAPI', () => ({ + CopilotAPI: jest.fn().mockImplementation((...args: unknown[]) => { + mockCopilotApiCtor(...args) + return { + getWorkspace: mockGetWorkspace, + getCompanyClients: mockGetCompanyClients, + } + }), +})) + +jest.mock('./eligibility', () => ({ + getEligibleReminders: (...args: unknown[]) => mockGetEligibleReminders(...args), +})) + +jest.mock('./send-reminder-email', () => ({ + sendReminderEmail: (...args: unknown[]) => mockSendReminderEmail(...args), +})) + +// Bypass Bottleneck's rate-limiting in tests but preserve sequential ordering per instance +// via a promise chain. Matches the pattern used in auto-archive-completed-tasks.test.ts so +// FIFO mockResolvedValueOnce queues drain deterministically. +jest.mock('bottleneck', () => ({ + __esModule: true, + default: jest.fn().mockImplementation(() => { + let chain: Promise = Promise.resolve() + return { + schedule: (fn: () => Promise) => { + const next = chain.then(() => fn()) + chain = next.catch(() => undefined) + return next + }, + } + }), +})) + +import { sendTaskReminders } from './send-task-reminders' + +type RunResult = { sent: number; failed: number; skipped: number; workspaceCount: number } +const runJob = async (): Promise => { + const { run } = sendTaskReminders as unknown as { + run: (payload: { timestamp: Date }) => Promise + } + return run({ timestamp: new Date() }) +} + +const workspace = { id: 'ws_1', brandName: 'Acme' } + +const buildRow = (overrides: Partial[1]> = {}) => ({ + taskId: 'task_1', + workspaceId: 'ws_1', + assigneeId: 'client_1', + assigneeType: AssigneeType.client, + companyId: 'company_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + ...overrides, +}) + +describe('sendTaskReminders', () => { + beforeEach(() => { + jest.clearAllMocks() + mockQueryRaw.mockReset() + mockTaskFindMany.mockReset() + mockTaskReminderSentDelete.mockReset() + mockGetEligibleReminders.mockReset() + mockSendReminderEmail.mockReset() + mockGetWorkspace.mockReset() + mockGetCompanyClients.mockReset() + mockCopilotApiCtor.mockReset() + mockGetWorkspace.mockResolvedValue(workspace) + }) + + it('exits cleanly when no rows are eligible', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([]) + + const result = await runJob() + + expect(result).toEqual({ sent: 0, failed: 0, skipped: 0, workspaceCount: 0 }) + expect(mockQueryRaw).not.toHaveBeenCalled() + expect(mockSendReminderEmail).not.toHaveBeenCalled() + expect(mockCopilotApiCtor).not.toHaveBeenCalled() + }) + + it('filters out internalUser rows before any DB or Copilot work', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ assigneeType: AssigneeType.internalUser, assigneeId: 'iu_1', companyId: null }), + ]) + + const result = await runJob() + + expect(result.workspaceCount).toBe(0) + expect(mockTaskFindMany).not.toHaveBeenCalled() + expect(mockQueryRaw).not.toHaveBeenCalled() + expect(mockSendReminderEmail).not.toHaveBeenCalled() + }) + + it('sends one reminder for a client-assigned task and writes one ledger row', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) + mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) + mockQueryRaw.mockResolvedValueOnce([ + { + id: 'ledger_1', + taskId: 'task_1', + recipientId: 'client_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + }, + ]) + mockSendReminderEmail.mockResolvedValueOnce('notif_1') + + const result = await runJob() + + expect(result).toEqual({ sent: 1, failed: 0, skipped: 0, workspaceCount: 1 }) + expect(mockSendReminderEmail).toHaveBeenCalledTimes(1) + expect(mockSendReminderEmail.mock.calls[0][0]).toMatchObject({ + task: { id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }, + recipientClientId: 'client_1', + recipientCompanyId: 'company_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + isCompanyRecipient: false, + workspace, + }) + }) + + it('treats ON CONFLICT returning zero rows as fully-skipped (re-run idempotency)', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) + mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) + mockQueryRaw.mockResolvedValueOnce([]) + + const result = await runJob() + + expect(result).toEqual({ sent: 0, failed: 0, skipped: 1, workspaceCount: 1 }) + expect(mockSendReminderEmail).not.toHaveBeenCalled() + }) + + it('fans out a company-assigned task to one send per current member', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ + assigneeType: AssigneeType.company, + assigneeId: 'company_1', + companyId: 'company_1', + }), + ]) + mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) + mockGetCompanyClients.mockResolvedValueOnce([{ id: 'm_1' }, { id: 'm_2' }, { id: 'm_3' }]) + mockQueryRaw.mockResolvedValueOnce([ + { id: 'l_1', taskId: 'task_1', recipientId: 'm_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + { id: 'l_2', taskId: 'task_1', recipientId: 'm_2', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + { id: 'l_3', taskId: 'task_1', recipientId: 'm_3', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + ]) + mockSendReminderEmail.mockResolvedValue('notif') + + const result = await runJob() + + expect(result).toEqual({ sent: 3, failed: 0, skipped: 0, workspaceCount: 1 }) + expect(mockSendReminderEmail).toHaveBeenCalledTimes(3) + const recipientIds = mockSendReminderEmail.mock.calls.map((c) => c[0].recipientClientId).sort() + expect(recipientIds).toEqual(['m_1', 'm_2', 'm_3']) + expect(mockSendReminderEmail.mock.calls[0][0].isCompanyRecipient).toBe(true) + }) + + it('compensates the ledger when Copilot send fails', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) + mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) + mockQueryRaw.mockResolvedValueOnce([ + { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + ]) + mockSendReminderEmail.mockRejectedValueOnce(new Error('copilot 5xx')) + mockTaskReminderSentDelete.mockResolvedValueOnce({ id: 'ledger_1' }) + + const result = await runJob() + + expect(result).toEqual({ sent: 0, failed: 1, skipped: 0, workspaceCount: 1 }) + expect(mockTaskReminderSentDelete).toHaveBeenCalledWith({ where: { id: 'ledger_1' } }) + }) + + it('does not abort the sweep when one workspace throws (per-workspace isolation)', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ workspaceId: 'ws_bad', taskId: 'task_bad' }), + buildRow({ workspaceId: 'ws_good', taskId: 'task_good', assigneeId: 'client_good' }), + ]) + // ws_bad findMany throws; ws_good completes a single send. + mockTaskFindMany + .mockRejectedValueOnce(new Error('db blew up')) + .mockResolvedValueOnce([{ id: 'task_good', title: 'Submit timesheet', createdById: 'iu_good' }]) + mockQueryRaw.mockResolvedValueOnce([ + { + id: 'ledger_g', + taskId: 'task_good', + recipientId: 'client_good', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + }, + ]) + mockSendReminderEmail.mockResolvedValueOnce('notif_good') + + const result = await runJob() + + expect(result.workspaceCount).toBe(2) + expect(result.sent).toBe(1) + }) +}) diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts new file mode 100644 index 000000000..bbba086fe --- /dev/null +++ b/src/jobs/notifications/send-task-reminders.ts @@ -0,0 +1,239 @@ +import 'server-only' + +import { copilotAPIKey } from '@/config' +import DBClient from '@/lib/db' +import { ClientResponse } from '@/types/common' +import { CopilotAPI } from '@/utils/CopilotAPI' +import { encodePayload } from '@/utils/crypto' +import { AssigneeType, Prisma, TaskReminderType } from '@prisma/client' +import { logger, schedules } from '@trigger.dev/sdk/v3' +import Bottleneck from 'bottleneck' + +import { EligibilityRow, getEligibleReminders } from './eligibility' +import { sendReminderEmail } from './send-reminder-email' + +const WORKSPACE_CONCURRENCY = 5 + +type WorkspaceTotals = { sent: number; failed: number; skipped: number } + +type TaskInfo = { id: string; title: string; createdById: string } + +type Recipient = { clientId: string; companyId: string | null } + +type LedgerInsertedRow = { + id: string + taskId: string + recipientId: string + reminderType: TaskReminderType +} + +type LedgerPlanEntry = { + row: EligibilityRow + task: TaskInfo + recipient: Recipient +} + +export const sendTaskReminders = schedules.task({ + id: 'send-task-reminders', + cron: '0 0 * * *', + maxDuration: 3000, + run: async (payload) => { + const db = DBClient.getInstance() + + const allRows = await getEligibleReminders(db) + // IUs are deliberately excluded from reminder emails — see EligibilityRow typedoc + // in ./eligibility.ts. The eligibility SQL still emits IU rows for symmetry; the + // filter lives here so OUT-3736's contract stays untouched. + const rows = allRows.filter((r) => r.assigneeType !== AssigneeType.internalUser) + + const byWorkspace = new Map() + for (const row of rows) { + const bucket = byWorkspace.get(row.workspaceId) + if (bucket) bucket.push(row) + else byWorkspace.set(row.workspaceId, [row]) + } + + logger.log('send-task-reminders: sweep starting', { + totalEligible: allRows.length, + afterIuFilter: rows.length, + eligibleWorkspaces: byWorkspace.size, + workspaceConcurrency: WORKSPACE_CONCURRENCY, + runAt: payload.timestamp, + }) + + const totals = { sent: 0, failed: 0, skipped: 0 } + let processed = 0 + const workspaceCount = byWorkspace.size + + const workspaceBottleneck = new Bottleneck({ maxConcurrent: WORKSPACE_CONCURRENCY }) + + await Promise.allSettled( + Array.from(byWorkspace.entries()).map(([workspaceId, workspaceRows]) => + workspaceBottleneck.schedule(async () => { + let wsTotals: WorkspaceTotals = { sent: 0, failed: 0, skipped: 0 } + try { + wsTotals = await processWorkspace(db, workspaceId, workspaceRows) + } catch (err) { + // Per-workspace isolation: one bad workspace shouldn't abort the sweep. + logger.error('send-task-reminders: workspace failed', { + workspaceId, + error: serializeError(err), + }) + } finally { + totals.sent += wsTotals.sent + totals.failed += wsTotals.failed + totals.skipped += wsTotals.skipped + processed += 1 + logger.log( + `[${processed}/${workspaceCount}] workspace ${workspaceId}: sent ${wsTotals.sent}, failed ${wsTotals.failed}, skipped ${wsTotals.skipped}`, + { workspaceId, ...wsTotals, processed, eligibleWorkspaces: workspaceCount }, + ) + } + }), + ), + ) + + logger.log('send-task-reminders: sweep complete', { + ...totals, + workspaceCount, + totalEligible: allRows.length, + }) + + return { ...totals, workspaceCount } + }, +}) + +const processWorkspace = async ( + db: ReturnType, + workspaceId: string, + rows: EligibilityRow[], +): Promise => { + // Fetch the task fields we need that aren't on EligibilityRow (title, createdById). + // Kept here rather than in eligibility.ts to leave OUT-3736's contract intact. + const taskIds = Array.from(new Set(rows.map((r) => r.taskId))) + const tasks = await db.task.findMany({ + where: { id: { in: taskIds } }, + select: { id: true, title: true, createdById: true }, + }) + const taskById = new Map(tasks.map((t) => [t.id, t])) + + // Mint a per-workspace Copilot client. Cron has no request-bound user, so we encode + // an IU token from any task's createdById + workspaceId — same shape as + // src/cmd/backfill-missed-emails/index.ts:97-99. + const senderIu = tasks[0]?.createdById + if (!senderIu) { + logger.warn('send-task-reminders: no IU found to mint workspace token, skipping', { + workspaceId, + rowCount: rows.length, + }) + return { sent: 0, failed: 0, skipped: 0 } + } + const token = encodePayload(copilotAPIKey, { internalUserId: senderIu, workspaceId }) + const copilot = new CopilotAPI(token) + const workspace = await copilot.getWorkspace() + + // Plan: fan out company rows to one entry per current member; client rows stay 1:1. + // Members no longer in the company are filtered naturally — they don't come back from + // getCompanyClients, per OUT-3736 ticket. + const plan: LedgerPlanEntry[] = [] + for (const row of rows) { + const task = taskById.get(row.taskId) + if (!task) continue + const recipients = await resolveRecipients(copilot, row) + for (const recipient of recipients) { + plan.push({ row, task, recipient }) + } + } + + if (plan.length === 0) return { sent: 0, failed: 0, skipped: 0 } + + // Ledger insert is the idempotency boundary. ON CONFLICT DO NOTHING ensures a retried + // cron run can never double-send: only the rows that come back from RETURNING are + // net-new claims to send. + const valuesSql = Prisma.join( + plan.map( + (entry) => Prisma.sql`( + gen_random_uuid(), + ${entry.row.taskId}::uuid, + ${workspaceId}, + ${entry.recipient.clientId}::uuid, + ${entry.row.reminderType}::"TaskReminderType", + NOW() + )`, + ), + ) + const inserted = await db.$queryRaw` + INSERT INTO "TaskReminderSents" ("id", "taskId", "workspaceId", "recipientId", "reminderType", "sentAt") + VALUES ${valuesSql} + ON CONFLICT ("taskId", "recipientId", "reminderType") DO NOTHING + RETURNING "id"::text AS "id", + "taskId"::text AS "taskId", + "recipientId"::text AS "recipientId", + "reminderType" + ` + + const insertedKey = (taskId: string, recipientId: string, type: TaskReminderType) => `${taskId}|${recipientId}|${type}` + const insertedById = new Map(inserted.map((r) => [insertedKey(r.taskId, r.recipientId, r.reminderType), r.id])) + + const skipped = plan.length - inserted.length + + let sent = 0 + let failed = 0 + + for (const entry of plan) { + const ledgerId = insertedById.get(insertedKey(entry.row.taskId, entry.recipient.clientId, entry.row.reminderType)) + if (!ledgerId) continue // already-sent (ON CONFLICT skipped this one) + + try { + await sendReminderEmail({ + task: entry.task, + recipientClientId: entry.recipient.clientId, + recipientCompanyId: entry.recipient.companyId, + reminderType: entry.row.reminderType, + isCompanyRecipient: entry.row.assigneeType === AssigneeType.company, + workspace, + copilot, + }) + sent += 1 + } catch (err) { + // Compensate: drop the ledger row so the next cron run retries this (task, recipient, type). + // If the DELETE itself fails the row stays in the ledger and we won't retry — that's + // a permanent miss, logged distinctly so on-call can clean up. + failed += 1 + logger.error('send-task-reminders: Copilot send failed, compensating ledger', { + workspaceId, + taskId: entry.row.taskId, + recipientClientId: entry.recipient.clientId, + reminderType: entry.row.reminderType, + error: serializeError(err), + }) + try { + await db.taskReminderSent.delete({ where: { id: ledgerId } }) + } catch (deleteErr) { + logger.error('send-task-reminders: ledger compensation DELETE failed, reminder will not retry', { + workspaceId, + ledgerId, + taskId: entry.row.taskId, + recipientClientId: entry.recipient.clientId, + reminderType: entry.row.reminderType, + error: serializeError(deleteErr), + }) + } + } + } + + return { sent, failed, skipped } +} + +const resolveRecipients = async (copilot: CopilotAPI, row: EligibilityRow): Promise => { + if (row.assigneeType === AssigneeType.client) { + return [{ clientId: row.assigneeId, companyId: row.companyId }] + } + if (row.assigneeType === AssigneeType.company) { + const members: ClientResponse[] = await copilot.getCompanyClients(row.assigneeId) + return members.map((m) => ({ clientId: m.id, companyId: row.assigneeId })) + } + return [] +} + +const serializeError = (err: unknown) => (err instanceof Error ? { message: err.message, stack: err.stack } : err) From 67ac802e1edd65d123a63b746cf3968883196612 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 16:56:18 +0545 Subject: [PATCH 02/15] refactor(OUT-3730): init per-workspace CopilotAPI via workspace-scoped apiKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops the IU-token mint and uses the workspace-scoped apiKey pattern that the SDK patch already supports when COPILOT_ENV is set on the Trigger.dev runtime — same env that auto-archive's dispatch-task-archived-webhook relies on. Two wins: - No "pick a random task's createdById to forge a token" fallback, which was structurally awkward (the IU we mint as had no semantic meaning). - One fewer crypto call per workspace per cron run. senderId for the email itself still comes from task.createdById in sendReminderEmail — that's unchanged, since the IU who created the task is the legitimate sender identity for the reminder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notifications/send-task-reminders.test.ts | 26 ++++++++++++++++--- src/jobs/notifications/send-task-reminders.ts | 20 +++++--------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts index efc2597dd..5a3a73c58 100644 --- a/src/jobs/notifications/send-task-reminders.test.ts +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -21,6 +21,10 @@ jest.mock('@trigger.dev/sdk/v3', () => ({ logger: { log: jest.fn(), error: jest.fn(), warn: jest.fn() }, })) +jest.mock('@/config', () => ({ + copilotAPIKey: 'test-api-key', +})) + jest.mock('@/lib/db', () => ({ __esModule: true, default: { @@ -32,10 +36,6 @@ jest.mock('@/lib/db', () => ({ }, })) -jest.mock('@/utils/crypto', () => ({ - encodePayload: jest.fn(() => 'stub-token'), -})) - jest.mock('@/utils/CopilotAPI', () => ({ CopilotAPI: jest.fn().mockImplementation((...args: unknown[]) => { mockCopilotApiCtor(...args) @@ -158,6 +158,24 @@ describe('sendTaskReminders', () => { }) }) + it('initializes CopilotAPI with a workspace-scoped apiKey (no user token mint)', async () => { + mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) + mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) + mockQueryRaw.mockResolvedValueOnce([ + { + id: 'ledger_1', + taskId: 'task_1', + recipientId: 'client_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + }, + ]) + mockSendReminderEmail.mockResolvedValueOnce('notif_1') + + await runJob() + + expect(mockCopilotApiCtor).toHaveBeenCalledWith('', 'ws_1/test-api-key') + }) + it('treats ON CONFLICT returning zero rows as fully-skipped (re-run idempotency)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index bbba086fe..2ac3bbcc5 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -4,7 +4,6 @@ import { copilotAPIKey } from '@/config' import DBClient from '@/lib/db' import { ClientResponse } from '@/types/common' import { CopilotAPI } from '@/utils/CopilotAPI' -import { encodePayload } from '@/utils/crypto' import { AssigneeType, Prisma, TaskReminderType } from '@prisma/client' import { logger, schedules } from '@trigger.dev/sdk/v3' import Bottleneck from 'bottleneck' @@ -115,21 +114,14 @@ const processWorkspace = async ( where: { id: { in: taskIds } }, select: { id: true, title: true, createdById: true }, }) + if (tasks.length === 0) return { sent: 0, failed: 0, skipped: 0 } const taskById = new Map(tasks.map((t) => [t.id, t])) - // Mint a per-workspace Copilot client. Cron has no request-bound user, so we encode - // an IU token from any task's createdById + workspaceId — same shape as - // src/cmd/backfill-missed-emails/index.ts:97-99. - const senderIu = tasks[0]?.createdById - if (!senderIu) { - logger.warn('send-task-reminders: no IU found to mint workspace token, skipping', { - workspaceId, - rowCount: rows.length, - }) - return { sent: 0, failed: 0, skipped: 0 } - } - const token = encodePayload(copilotAPIKey, { internalUserId: senderIu, workspaceId }) - const copilot = new CopilotAPI(token) + // Per-workspace Copilot client using a workspace-scoped apiKey. The SDK patch + // (src/lib/patch-copilot-node-sdk.js) accepts `${workspaceId}/${apiKey}` as the auth key + // directly when COPILOT_ENV is set on the Trigger.dev runtime (`local` for prod, + // `__SECRET_STAGING__` for staging) — no user token needed. Empty token = no user context. + const copilot = new CopilotAPI('', `${workspaceId}/${copilotAPIKey}`) const workspace = await copilot.getWorkspace() // Plan: fan out company rows to one entry per current member; client rows stay 1:1. From abef42e5de9da3652920909ff8f7f874450dd59b Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 17:00:52 +0545 Subject: [PATCH 03/15] refactor(OUT-3730): swap raw INSERT for prisma createManyAndReturn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prisma 5.14+ exposes createManyAndReturn, which compiles to exactly the INSERT ... ON CONFLICT DO NOTHING RETURNING shape the cron needs but does it as a typed Prisma call. Drops: - The Prisma.sql / Prisma.join template assembly. - Manual ::uuid and ::"TaskReminderType" casts (Prisma handles via the model's @db.Uuid / enum typing). - The hand-written gen_random_uuid() in VALUES — the model already sets id via @default(dbgenerated("gen_random_uuid()")), so Postgres fills it in automatically when Prisma omits it from the INSERT. - The LedgerInsertedRow shim type (now inferred from the Prisma model). Net 15 lines shorter, no behavior change. skipDuplicates: true compiles to ON CONFLICT DO NOTHING against the existing (taskId, recipientId, reminderType) unique constraint, and createManyAndReturn only returns the rows that actually got inserted — identical semantics to the previous raw query. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notifications/send-task-reminders.test.ts | 26 +++++----- src/jobs/notifications/send-task-reminders.ts | 47 ++++++------------- 2 files changed, 29 insertions(+), 44 deletions(-) diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts index 5a3a73c58..fa6e4ee72 100644 --- a/src/jobs/notifications/send-task-reminders.test.ts +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -3,8 +3,8 @@ import { AssigneeType, TaskReminderType } from '@prisma/client' // Mocks must be configured before requiring the SUT. Variables referenced inside the // jest.mock factory must start with `mock` so the babel-jest allow-list lets the closure // see them once the const declarations have run. -const mockQueryRaw = jest.fn() const mockTaskFindMany = jest.fn() +const mockTaskReminderSentCreateManyAndReturn = jest.fn() const mockTaskReminderSentDelete = jest.fn() const mockGetEligibleReminders = jest.fn() @@ -29,9 +29,11 @@ jest.mock('@/lib/db', () => ({ __esModule: true, default: { getInstance: () => ({ - $queryRaw: mockQueryRaw, task: { findMany: mockTaskFindMany }, - taskReminderSent: { delete: mockTaskReminderSentDelete }, + taskReminderSent: { + createManyAndReturn: mockTaskReminderSentCreateManyAndReturn, + delete: mockTaskReminderSentDelete, + }, }), }, })) @@ -96,8 +98,8 @@ const buildRow = (overrides: Partial[1]> = {}) describe('sendTaskReminders', () => { beforeEach(() => { jest.clearAllMocks() - mockQueryRaw.mockReset() mockTaskFindMany.mockReset() + mockTaskReminderSentCreateManyAndReturn.mockReset() mockTaskReminderSentDelete.mockReset() mockGetEligibleReminders.mockReset() mockSendReminderEmail.mockReset() @@ -113,7 +115,7 @@ describe('sendTaskReminders', () => { const result = await runJob() expect(result).toEqual({ sent: 0, failed: 0, skipped: 0, workspaceCount: 0 }) - expect(mockQueryRaw).not.toHaveBeenCalled() + expect(mockTaskReminderSentCreateManyAndReturn).not.toHaveBeenCalled() expect(mockSendReminderEmail).not.toHaveBeenCalled() expect(mockCopilotApiCtor).not.toHaveBeenCalled() }) @@ -127,14 +129,14 @@ describe('sendTaskReminders', () => { expect(result.workspaceCount).toBe(0) expect(mockTaskFindMany).not.toHaveBeenCalled() - expect(mockQueryRaw).not.toHaveBeenCalled() + expect(mockTaskReminderSentCreateManyAndReturn).not.toHaveBeenCalled() expect(mockSendReminderEmail).not.toHaveBeenCalled() }) it('sends one reminder for a client-assigned task and writes one ledger row', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) - mockQueryRaw.mockResolvedValueOnce([ + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', taskId: 'task_1', @@ -161,7 +163,7 @@ describe('sendTaskReminders', () => { it('initializes CopilotAPI with a workspace-scoped apiKey (no user token mint)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) - mockQueryRaw.mockResolvedValueOnce([ + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', taskId: 'task_1', @@ -179,7 +181,7 @@ describe('sendTaskReminders', () => { it('treats ON CONFLICT returning zero rows as fully-skipped (re-run idempotency)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) - mockQueryRaw.mockResolvedValueOnce([]) + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([]) const result = await runJob() @@ -197,7 +199,7 @@ describe('sendTaskReminders', () => { ]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockGetCompanyClients.mockResolvedValueOnce([{ id: 'm_1' }, { id: 'm_2' }, { id: 'm_3' }]) - mockQueryRaw.mockResolvedValueOnce([ + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'l_1', taskId: 'task_1', recipientId: 'm_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, { id: 'l_2', taskId: 'task_1', recipientId: 'm_2', reminderType: TaskReminderType.NO_DUE_DATE_3D }, { id: 'l_3', taskId: 'task_1', recipientId: 'm_3', reminderType: TaskReminderType.NO_DUE_DATE_3D }, @@ -216,7 +218,7 @@ describe('sendTaskReminders', () => { it('compensates the ledger when Copilot send fails', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) - mockQueryRaw.mockResolvedValueOnce([ + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, ]) mockSendReminderEmail.mockRejectedValueOnce(new Error('copilot 5xx')) @@ -237,7 +239,7 @@ describe('sendTaskReminders', () => { mockTaskFindMany .mockRejectedValueOnce(new Error('db blew up')) .mockResolvedValueOnce([{ id: 'task_good', title: 'Submit timesheet', createdById: 'iu_good' }]) - mockQueryRaw.mockResolvedValueOnce([ + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_g', taskId: 'task_good', diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index 2ac3bbcc5..f888c3e47 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -4,7 +4,7 @@ import { copilotAPIKey } from '@/config' import DBClient from '@/lib/db' import { ClientResponse } from '@/types/common' import { CopilotAPI } from '@/utils/CopilotAPI' -import { AssigneeType, Prisma, TaskReminderType } from '@prisma/client' +import { AssigneeType, TaskReminderType } from '@prisma/client' import { logger, schedules } from '@trigger.dev/sdk/v3' import Bottleneck from 'bottleneck' @@ -19,13 +19,6 @@ type TaskInfo = { id: string; title: string; createdById: string } type Recipient = { clientId: string; companyId: string | null } -type LedgerInsertedRow = { - id: string - taskId: string - recipientId: string - reminderType: TaskReminderType -} - type LedgerPlanEntry = { row: EligibilityRow task: TaskInfo @@ -139,30 +132,20 @@ const processWorkspace = async ( if (plan.length === 0) return { sent: 0, failed: 0, skipped: 0 } - // Ledger insert is the idempotency boundary. ON CONFLICT DO NOTHING ensures a retried - // cron run can never double-send: only the rows that come back from RETURNING are - // net-new claims to send. - const valuesSql = Prisma.join( - plan.map( - (entry) => Prisma.sql`( - gen_random_uuid(), - ${entry.row.taskId}::uuid, - ${workspaceId}, - ${entry.recipient.clientId}::uuid, - ${entry.row.reminderType}::"TaskReminderType", - NOW() - )`, - ), - ) - const inserted = await db.$queryRaw` - INSERT INTO "TaskReminderSents" ("id", "taskId", "workspaceId", "recipientId", "reminderType", "sentAt") - VALUES ${valuesSql} - ON CONFLICT ("taskId", "recipientId", "reminderType") DO NOTHING - RETURNING "id"::text AS "id", - "taskId"::text AS "taskId", - "recipientId"::text AS "recipientId", - "reminderType" - ` + // Ledger insert is the idempotency boundary. `skipDuplicates: true` compiles to + // `ON CONFLICT DO NOTHING` against the (taskId, recipientId, reminderType) unique + // constraint, so a retried cron run cannot double-send. `createManyAndReturn` only + // returns the rows that actually got inserted — duplicates skipped by ON CONFLICT + // are absent from the result, which is precisely the "net-new to send" list. + const inserted = await db.taskReminderSent.createManyAndReturn({ + data: plan.map((entry) => ({ + taskId: entry.row.taskId, + workspaceId, + recipientId: entry.recipient.clientId, + reminderType: entry.row.reminderType, + })), + skipDuplicates: true, + }) const insertedKey = (taskId: string, recipientId: string, type: TaskReminderType) => `${taskId}|${recipientId}|${type}` const insertedById = new Map(inserted.map((r) => [insertedKey(r.taskId, r.recipientId, r.reminderType), r.id])) From 2e4ced427398e2a82723a714bfb23afc637ef355 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 17:04:19 +0545 Subject: [PATCH 04/15] chore(OUT-3730): trim comments in reminder cron + helper Strip restating-the-code and ticket-reference comments. Keep three short load-bearing notes: the workspace-scoped apiKey shape, the ledger-before-send ordering, and why we DELETE on Copilot failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/send-reminder-email.ts | 14 ++-------- src/jobs/notifications/send-task-reminders.ts | 27 ++++--------------- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/jobs/notifications/send-reminder-email.ts b/src/jobs/notifications/send-reminder-email.ts index f30441e7d..7b247d4d5 100644 --- a/src/jobs/notifications/send-reminder-email.ts +++ b/src/jobs/notifications/send-reminder-email.ts @@ -15,18 +15,8 @@ export type SendReminderEmailArgs = { copilot: CopilotAPI } -/** - * Dispatches a single task reminder email via Copilot's notification API. - * - * Email-only delivery: omits `deliveryTargets.inProduct` so no in-product notification - * is created. We also deliberately skip writing to `ClientNotification` — - * `ClientNotification` tracks read-state for in-product notifications, which reminders - * don't create. Reminder dedupe state lives in `TaskReminderSent`, which the caller - * inserts on success (the unique constraint is the idempotency primitive). - * - * Throws on Copilot failure. Callers compensate by NOT inserting into - * `TaskReminderSent`, so a future cron run will retry the same `(task, recipient, type)`. - */ +// Email-only: omits deliveryTargets.inProduct and does not write to ClientNotification. +// Reminder dedupe lives in TaskReminderSent (caller's responsibility). export const sendReminderEmail = async ({ task, recipientClientId, diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index f888c3e47..59c2d0d8f 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -33,9 +33,6 @@ export const sendTaskReminders = schedules.task({ const db = DBClient.getInstance() const allRows = await getEligibleReminders(db) - // IUs are deliberately excluded from reminder emails — see EligibilityRow typedoc - // in ./eligibility.ts. The eligibility SQL still emits IU rows for symmetry; the - // filter lives here so OUT-3736's contract stays untouched. const rows = allRows.filter((r) => r.assigneeType !== AssigneeType.internalUser) const byWorkspace = new Map() @@ -66,7 +63,6 @@ export const sendTaskReminders = schedules.task({ try { wsTotals = await processWorkspace(db, workspaceId, workspaceRows) } catch (err) { - // Per-workspace isolation: one bad workspace shouldn't abort the sweep. logger.error('send-task-reminders: workspace failed', { workspaceId, error: serializeError(err), @@ -100,8 +96,6 @@ const processWorkspace = async ( workspaceId: string, rows: EligibilityRow[], ): Promise => { - // Fetch the task fields we need that aren't on EligibilityRow (title, createdById). - // Kept here rather than in eligibility.ts to leave OUT-3736's contract intact. const taskIds = Array.from(new Set(rows.map((r) => r.taskId))) const tasks = await db.task.findMany({ where: { id: { in: taskIds } }, @@ -110,16 +104,11 @@ const processWorkspace = async ( if (tasks.length === 0) return { sent: 0, failed: 0, skipped: 0 } const taskById = new Map(tasks.map((t) => [t.id, t])) - // Per-workspace Copilot client using a workspace-scoped apiKey. The SDK patch - // (src/lib/patch-copilot-node-sdk.js) accepts `${workspaceId}/${apiKey}` as the auth key - // directly when COPILOT_ENV is set on the Trigger.dev runtime (`local` for prod, - // `__SECRET_STAGING__` for staging) — no user token needed. Empty token = no user context. + // Workspace-scoped apiKey: empty token + `${workspaceId}/${apiKey}` is accepted by the + // SDK when COPILOT_ENV is set on the Trigger.dev runtime. const copilot = new CopilotAPI('', `${workspaceId}/${copilotAPIKey}`) const workspace = await copilot.getWorkspace() - // Plan: fan out company rows to one entry per current member; client rows stay 1:1. - // Members no longer in the company are filtered naturally — they don't come back from - // getCompanyClients, per OUT-3736 ticket. const plan: LedgerPlanEntry[] = [] for (const row of rows) { const task = taskById.get(row.taskId) @@ -132,11 +121,7 @@ const processWorkspace = async ( if (plan.length === 0) return { sent: 0, failed: 0, skipped: 0 } - // Ledger insert is the idempotency boundary. `skipDuplicates: true` compiles to - // `ON CONFLICT DO NOTHING` against the (taskId, recipientId, reminderType) unique - // constraint, so a retried cron run cannot double-send. `createManyAndReturn` only - // returns the rows that actually got inserted — duplicates skipped by ON CONFLICT - // are absent from the result, which is precisely the "net-new to send" list. + // Ledger insert before send: the unique constraint is the dedupe primitive. const inserted = await db.taskReminderSent.createManyAndReturn({ data: plan.map((entry) => ({ taskId: entry.row.taskId, @@ -157,7 +142,7 @@ const processWorkspace = async ( for (const entry of plan) { const ledgerId = insertedById.get(insertedKey(entry.row.taskId, entry.recipient.clientId, entry.row.reminderType)) - if (!ledgerId) continue // already-sent (ON CONFLICT skipped this one) + if (!ledgerId) continue try { await sendReminderEmail({ @@ -171,9 +156,7 @@ const processWorkspace = async ( }) sent += 1 } catch (err) { - // Compensate: drop the ledger row so the next cron run retries this (task, recipient, type). - // If the DELETE itself fails the row stays in the ledger and we won't retry — that's - // a permanent miss, logged distinctly so on-call can clean up. + // Delete the ledger row so the next cron run retries. failed += 1 logger.error('send-task-reminders: Copilot send failed, compensating ledger', { workspaceId, From b34ab8c29c89221d6b4074ff56c6bd3b4eed3bf7 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 17:16:09 +0545 Subject: [PATCH 05/15] refactor(OUT-3730): fold title + createdById into EligibilityRow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds t.title and t.createdById to the eligibility SELECT and drops the per-workspace task.findMany. processWorkspace now operates on a single consistent snapshot from the eligibility query — no more two-step read that could pick up divergent state between the query and the send. Same behavior, fewer DB calls, tighter consistency window. The remaining race (task reassigned between eligibility query and Copilot send) is the unavoidable one and was never closable without distributed transactions. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/eligibility.ts | 19 +++++++-------- .../notifications/send-task-reminders.test.ts | 23 +++++-------------- src/jobs/notifications/send-task-reminders.ts | 17 ++------------ 3 files changed, 16 insertions(+), 43 deletions(-) diff --git a/src/jobs/notifications/eligibility.ts b/src/jobs/notifications/eligibility.ts index 87e7915b6..bc5d4d97c 100644 --- a/src/jobs/notifications/eligibility.ts +++ b/src/jobs/notifications/eligibility.ts @@ -4,19 +4,14 @@ import { AssigneeType, TaskReminderType } from '@prisma/client' export type EligibilityRow = { taskId: string workspaceId: string + title: string + createdById: string assigneeId: string assigneeType: AssigneeType - /** - * Company context for the recipient. - * - assigneeType='client' → task.companyId (a client may belong to multiple companies on Copilot; - * this disambiguates which "hat" they're wearing for this task) - * - assigneeType='company' → assigneeId (the company IS the assignee; caller fans out to members) - * - assigneeType='internalUser'→ null (IUs have no company concept and never receive email notifications) - * - * Required for ClientNotifications inserts (unique key includes companyId) and for - * Copilot's recipientCompanyId on email-bearing notifications. See - * src/app/api/notification/notification.service.ts:558. - */ + // companyId derivation per assigneeType: + // client → task.companyId (disambiguates which company "hat" the client wears) + // company → assigneeId (the company IS the assignee) + // internalUser → null (IUs don't receive email reminders) companyId: string | null reminderType: TaskReminderType } @@ -38,6 +33,8 @@ export const getEligibleReminders = async (db: ReturnType ({ __esModule: true, default: { getInstance: () => ({ - task: { findMany: mockTaskFindMany }, taskReminderSent: { createManyAndReturn: mockTaskReminderSentCreateManyAndReturn, delete: mockTaskReminderSentDelete, @@ -57,8 +55,7 @@ jest.mock('./send-reminder-email', () => ({ })) // Bypass Bottleneck's rate-limiting in tests but preserve sequential ordering per instance -// via a promise chain. Matches the pattern used in auto-archive-completed-tasks.test.ts so -// FIFO mockResolvedValueOnce queues drain deterministically. +// via a promise chain so FIFO mockResolvedValueOnce queues drain deterministically. jest.mock('bottleneck', () => ({ __esModule: true, default: jest.fn().mockImplementation(() => { @@ -88,6 +85,8 @@ const workspace = { id: 'ws_1', brandName: 'Acme' } const buildRow = (overrides: Partial[1]> = {}) => ({ taskId: 'task_1', workspaceId: 'ws_1', + title: 'Submit timesheet', + createdById: 'iu_1', assigneeId: 'client_1', assigneeType: AssigneeType.client, companyId: 'company_1', @@ -98,7 +97,6 @@ const buildRow = (overrides: Partial[1]> = {}) describe('sendTaskReminders', () => { beforeEach(() => { jest.clearAllMocks() - mockTaskFindMany.mockReset() mockTaskReminderSentCreateManyAndReturn.mockReset() mockTaskReminderSentDelete.mockReset() mockGetEligibleReminders.mockReset() @@ -120,7 +118,7 @@ describe('sendTaskReminders', () => { expect(mockCopilotApiCtor).not.toHaveBeenCalled() }) - it('filters out internalUser rows before any DB or Copilot work', async () => { + it('filters out internalUser rows before any Copilot work', async () => { mockGetEligibleReminders.mockResolvedValueOnce([ buildRow({ assigneeType: AssigneeType.internalUser, assigneeId: 'iu_1', companyId: null }), ]) @@ -128,14 +126,12 @@ describe('sendTaskReminders', () => { const result = await runJob() expect(result.workspaceCount).toBe(0) - expect(mockTaskFindMany).not.toHaveBeenCalled() expect(mockTaskReminderSentCreateManyAndReturn).not.toHaveBeenCalled() expect(mockSendReminderEmail).not.toHaveBeenCalled() }) it('sends one reminder for a client-assigned task and writes one ledger row', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) - mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', @@ -162,7 +158,6 @@ describe('sendTaskReminders', () => { it('initializes CopilotAPI with a workspace-scoped apiKey (no user token mint)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) - mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', @@ -180,7 +175,6 @@ describe('sendTaskReminders', () => { it('treats ON CONFLICT returning zero rows as fully-skipped (re-run idempotency)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) - mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([]) const result = await runJob() @@ -197,7 +191,6 @@ describe('sendTaskReminders', () => { companyId: 'company_1', }), ]) - mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockGetCompanyClients.mockResolvedValueOnce([{ id: 'm_1' }, { id: 'm_2' }, { id: 'm_3' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'l_1', taskId: 'task_1', recipientId: 'm_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, @@ -217,7 +210,6 @@ describe('sendTaskReminders', () => { it('compensates the ledger when Copilot send fails', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) - mockTaskFindMany.mockResolvedValueOnce([{ id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, ]) @@ -235,11 +227,8 @@ describe('sendTaskReminders', () => { buildRow({ workspaceId: 'ws_bad', taskId: 'task_bad' }), buildRow({ workspaceId: 'ws_good', taskId: 'task_good', assigneeId: 'client_good' }), ]) - // ws_bad findMany throws; ws_good completes a single send. - mockTaskFindMany - .mockRejectedValueOnce(new Error('db blew up')) - .mockResolvedValueOnce([{ id: 'task_good', title: 'Submit timesheet', createdById: 'iu_good' }]) - mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ + // ws_bad's ledger insert throws; ws_good completes a single send. + mockTaskReminderSentCreateManyAndReturn.mockRejectedValueOnce(new Error('db blew up')).mockResolvedValueOnce([ { id: 'ledger_g', taskId: 'task_good', diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index 59c2d0d8f..e6857a223 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -15,13 +15,10 @@ const WORKSPACE_CONCURRENCY = 5 type WorkspaceTotals = { sent: number; failed: number; skipped: number } -type TaskInfo = { id: string; title: string; createdById: string } - type Recipient = { clientId: string; companyId: string | null } type LedgerPlanEntry = { row: EligibilityRow - task: TaskInfo recipient: Recipient } @@ -96,14 +93,6 @@ const processWorkspace = async ( workspaceId: string, rows: EligibilityRow[], ): Promise => { - const taskIds = Array.from(new Set(rows.map((r) => r.taskId))) - const tasks = await db.task.findMany({ - where: { id: { in: taskIds } }, - select: { id: true, title: true, createdById: true }, - }) - if (tasks.length === 0) return { sent: 0, failed: 0, skipped: 0 } - const taskById = new Map(tasks.map((t) => [t.id, t])) - // Workspace-scoped apiKey: empty token + `${workspaceId}/${apiKey}` is accepted by the // SDK when COPILOT_ENV is set on the Trigger.dev runtime. const copilot = new CopilotAPI('', `${workspaceId}/${copilotAPIKey}`) @@ -111,11 +100,9 @@ const processWorkspace = async ( const plan: LedgerPlanEntry[] = [] for (const row of rows) { - const task = taskById.get(row.taskId) - if (!task) continue const recipients = await resolveRecipients(copilot, row) for (const recipient of recipients) { - plan.push({ row, task, recipient }) + plan.push({ row, recipient }) } } @@ -146,7 +133,7 @@ const processWorkspace = async ( try { await sendReminderEmail({ - task: entry.task, + task: { id: entry.row.taskId, title: entry.row.title, createdById: entry.row.createdById }, recipientClientId: entry.recipient.clientId, recipientCompanyId: entry.recipient.companyId, reminderType: entry.row.reminderType, From 84f82d2a1881924d6625782b7aeea6c6601f4dbc Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 25 May 2026 17:20:51 +0545 Subject: [PATCH 06/15] chore(OUT-3736): trim comments in eligibility.ts Drop the type-field annotation, the function docstring, and shorten the three inline SQL comments to one line each. Keeps the genuinely load-bearing notes (subtask carve-out, IS DISTINCT FROM rationale, the CASE WHEN evaluation-order guarantee). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/eligibility.ts | 38 +++++++-------------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/src/jobs/notifications/eligibility.ts b/src/jobs/notifications/eligibility.ts index bc5d4d97c..028dde4fe 100644 --- a/src/jobs/notifications/eligibility.ts +++ b/src/jobs/notifications/eligibility.ts @@ -8,26 +8,13 @@ export type EligibilityRow = { createdById: string assigneeId: string assigneeType: AssigneeType - // companyId derivation per assigneeType: - // client → task.companyId (disambiguates which company "hat" the client wears) - // company → assigneeId (the company IS the assignee) - // internalUser → null (IUs don't receive email reminders) companyId: string | null reminderType: TaskReminderType } -/** - * Returns one row per (task, assignee, reminderType) eligible for a reminder today. - * - * Company-assigned tasks emit a single row with assigneeType='company' and assigneeId - * set to the company id. Caller fans those out to individual members via Copilot — - * the SQL deliberately stops at the company boundary so DB has no Copilot dependency. - * - * Already-sent reminders are NOT filtered here. The TaskReminderSents unique constraint - * is the dedupe primitive at insert time, so a retried cron run is idempotent without - * an extra NOT EXISTS check (the windows are exact-day so day-N reminders don't repeat - * in normal operation). - */ +// Company-assigned tasks emit one row at the company level; caller fans out to members. +// Already-sent reminders are not filtered here — TaskReminderSents' unique constraint is +// the dedupe primitive at insert time. export const getEligibleReminders = async (db: ReturnType): Promise => { return db.$queryRaw` SELECT @@ -51,11 +38,8 @@ export const getEligibleReminders = async (db: ReturnType Date: Tue, 26 May 2026 15:30:22 +0545 Subject: [PATCH 07/15] fix(OUT-3735): drop ` portal:` prefix from reminder subjects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's email service prepends ` portal:` to every notification subject server-side. Our reminder copy helper was also prepending it, producing doubled subjects like: "Assembly + Outside portal: Assembly + Outside portal: [Overdue] ..." The existing `getEmailDetails` (for non-reminder emails) emits bare subjects for this reason — reminders should match that convention. Side effect: closes the open PRD-verbatim question on DUE_DATE_OVERDUE_7D. The PRD's inconsistent inclusion of `{Company} portal:` was a description of the rendered subject, not what the code should emit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notification.helpers.test.ts.snap | 24 +++++++++--------- .../notification/notification.helpers.test.ts | 9 ++++--- .../api/notification/notification.helpers.ts | 25 ++++++------------- .../notifications/send-reminder-email.test.ts | 4 +-- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap index 0904c2538..81f1621a7 100644 --- a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap +++ b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap @@ -10,7 +10,7 @@ Please make sure to complete this task by the due date.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Due Soon] Task due in 3 days", + "subject": "[Due Soon] Task due in 3 days", "title": "View task", }, "DUE_DATE_OVERDUE_3D": { @@ -19,7 +19,7 @@ Please make sure to complete this task by the due date.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Overdue] Task was due 3 days ago", + "subject": "[Overdue] Task was due 3 days ago", "title": "View task", }, "DUE_DATE_OVERDUE_7D": { @@ -30,7 +30,7 @@ Please complete this task as soon as possible.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Overdue] Task overdue by one week", + "subject": "[Overdue] Task overdue by one week", "title": "View task", }, "DUE_DATE_TODAY": { @@ -41,7 +41,7 @@ Please complete this task as soon as possible.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Due Soon] Task due today", + "subject": "[Due Soon] Task due today", "title": "View task", }, "NO_DUE_DATE_3D": { @@ -52,7 +52,7 @@ If you've already completed this task, please mark it as done in the portal.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Reminder] You have a task to complete", + "subject": "[Reminder] You have a task to complete", "title": "View task", }, "NO_DUE_DATE_7D": { @@ -63,7 +63,7 @@ If you've already completed this task, please mark it as done in the portal.", "taskId": "task_1", }, "header": "A task was assigned to your company", - "subject": "Acme portal: [Reminder] Task still pending", + "subject": "[Reminder] Task still pending", "title": "View task", }, } @@ -79,7 +79,7 @@ Please make sure to complete this task by the due date.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Due Soon] Task due in 3 days", + "subject": "[Due Soon] Task due in 3 days", "title": "View task", }, "DUE_DATE_OVERDUE_3D": { @@ -88,7 +88,7 @@ Please make sure to complete this task by the due date.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Overdue] Task was due 3 days ago", + "subject": "[Overdue] Task was due 3 days ago", "title": "View task", }, "DUE_DATE_OVERDUE_7D": { @@ -99,7 +99,7 @@ Please complete this task as soon as possible.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Overdue] Task overdue by one week", + "subject": "[Overdue] Task overdue by one week", "title": "View task", }, "DUE_DATE_TODAY": { @@ -110,7 +110,7 @@ Please complete this task as soon as possible.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Due Soon] Task due today", + "subject": "[Due Soon] Task due today", "title": "View task", }, "NO_DUE_DATE_3D": { @@ -121,7 +121,7 @@ If you've already completed this task, please mark it as done in the portal.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Reminder] You have a task to complete", + "subject": "[Reminder] You have a task to complete", "title": "View task", }, "NO_DUE_DATE_7D": { @@ -132,7 +132,7 @@ If you've already completed this task, please mark it as done in the portal.", "taskId": "task_1", }, "header": "A task was assigned to you", - "subject": "Acme portal: [Reminder] Task still pending", + "subject": "[Reminder] Task still pending", "title": "View task", }, } diff --git a/src/app/api/notification/notification.helpers.test.ts b/src/app/api/notification/notification.helpers.test.ts index 414fadc2b..5874c181e 100644 --- a/src/app/api/notification/notification.helpers.test.ts +++ b/src/app/api/notification/notification.helpers.test.ts @@ -39,10 +39,11 @@ describe('getReminderEmailDetails', () => { expect(result[TaskReminderType.NO_DUE_DATE_3D].header).toBe('A task was assigned to your team') }) - it('falls back gracefully when brandName is missing', () => { - const noBrand: WorkspaceResponse = { ...workspace, brandName: undefined } - const result = getReminderEmailDetails(noBrand, task, false) - expect(result[TaskReminderType.NO_DUE_DATE_3D].subject).toBe('portal: [Reminder] You have a task to complete') + it('omits any ` portal:` prefix from subjects (Copilot prepends it server-side)', () => { + const result = getReminderEmailDetails(workspace, task, false) + for (const variant of Object.values(TaskReminderType)) { + expect(result[variant].subject).not.toMatch(/portal:/i) + } }) it('emits ctaParams with the task id for every variant', () => { diff --git a/src/app/api/notification/notification.helpers.ts b/src/app/api/notification/notification.helpers.ts index 708af2eae..7b889f761 100644 --- a/src/app/api/notification/notification.helpers.ts +++ b/src/app/api/notification/notification.helpers.ts @@ -202,16 +202,8 @@ export const getEmailDetails = ( } } -/** - * Helper function that returns reminder email content for each TaskReminderType variant. - * Lifecycle is independent from `getEmailDetails` (which is keyed by NotificationTaskActions). - * @param {WorkspaceResponse} workspace - Workspace whose brandName fronts the subject and - * whose labels resolve the company term. - * @param {Pick} task - Task being reminded about. Used for ctaParams and body interpolation. - * @param {boolean} isCompanyRecipient - True if recipient is a company (header reads "your {groupTerm}"), - * false for an individual recipient (header reads "you"). - * @returns Reminder email content keyed by TaskReminderType. - */ +// Subjects intentionally omit any ` portal:` prefix — Copilot's email +// service prepends that itself, and adding it here results in a duplicated prefix. export const getReminderEmailDetails = ( workspace: WorkspaceResponse, task: Pick, @@ -226,7 +218,6 @@ export const getReminderEmailDetails = ( ctaParams: { taskId: string } } > => { - const portalPrefix = `${workspace.brandName ?? ''} portal:`.trimStart() const labels = getWorkspaceLabels(workspace) const header = isCompanyRecipient ? `A task was assigned to your ${labels.groupTerm}` : 'A task was assigned to you' const ctaParams = { taskId: task.id } @@ -234,42 +225,42 @@ export const getReminderEmailDetails = ( return { [TaskReminderType.NO_DUE_DATE_3D]: { - subject: `${portalPrefix} [Reminder] You have a task to complete`, + subject: '[Reminder] You have a task to complete', header, title, body: `This is a friendly reminder that you have a task ‘${task.title}’ assigned to you that's still pending completion.\n\nIf you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.NO_DUE_DATE_7D]: { - subject: `${portalPrefix} [Reminder] Task still pending`, + subject: '[Reminder] Task still pending', header, title, body: `This is a friendly reminder that you have a task ‘${task.title}’ that was assigned to you a week ago and is still pending.\n\nIf you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.DUE_DATE_BEFORE_3D]: { - subject: `${portalPrefix} [Due Soon] Task due in 3 days`, + subject: '[Due Soon] Task due in 3 days', header, title, body: `This is a friendly reminder that you have a task ‘${task.title}’ due in 3 days.\n\nPlease make sure to complete this task by the due date.`, ctaParams, }, [TaskReminderType.DUE_DATE_TODAY]: { - subject: `${portalPrefix} [Due Soon] Task due today`, + subject: '[Due Soon] Task due today', header, title, body: `This is a friendly reminder that you have a task ‘${task.title}’ due today.\n\nPlease complete this task as soon as possible.`, ctaParams, }, [TaskReminderType.DUE_DATE_OVERDUE_3D]: { - subject: `${portalPrefix} [Overdue] Task was due 3 days ago`, + subject: '[Overdue] Task was due 3 days ago', header, title, body: `This is a friendly reminder that the task ‘${task.title}’ is now overdue. It was due 3 days ago and is still pending completion.`, ctaParams, }, [TaskReminderType.DUE_DATE_OVERDUE_7D]: { - subject: `${portalPrefix} [Overdue] Task overdue by one week`, + subject: '[Overdue] Task overdue by one week', header, title, body: `This is a friendly reminder that the task ‘${task.title}’ is now one week overdue.\n\nPlease complete this task as soon as possible.`, diff --git a/src/jobs/notifications/send-reminder-email.test.ts b/src/jobs/notifications/send-reminder-email.test.ts index 5628c4403..1728420f1 100644 --- a/src/jobs/notifications/send-reminder-email.test.ts +++ b/src/jobs/notifications/send-reminder-email.test.ts @@ -57,7 +57,7 @@ describe('sendReminderEmail', () => { recipientCompanyId: 'company_1', }) expect(payload.deliveryTargets.email).toEqual({ - subject: 'Acme portal: [Reminder] You have a task to complete', + subject: '[Reminder] You have a task to complete', header: 'A task was assigned to you', title: 'View task', body: expect.stringContaining('‘Submit timesheet’'), @@ -80,7 +80,7 @@ describe('sendReminderEmail', () => { const payload = createNotification.mock.calls[0][0] expect(payload.deliveryTargets.email.header).toBe('A task was assigned to your company') - expect(payload.deliveryTargets.email.subject).toBe('Acme portal: [Due Soon] Task due today') + expect(payload.deliveryTargets.email.subject).toBe('[Due Soon] Task due today') }) it('omits recipientCompanyId when null', async () => { From 29d5a0047127bbca8ea20834c2b5706d147df788 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 15:36:10 +0545 Subject: [PATCH 08/15] fix(OUT-3735): use

for paragraph breaks in reminder bodies Copilot's email template collapses \n\n, so the two sentences in every reminder body were rendering as a single paragraph. The PRD specifies a paragraph break between the reminder statement and the call-to-action. HTML

survives Copilot's whitespace normalization and renders as the expected visible gap. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notification.helpers.test.ts.snap | 40 +++++-------------- .../api/notification/notification.helpers.ts | 10 ++--- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap index 81f1621a7..7991f1599 100644 --- a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap +++ b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap @@ -3,9 +3,7 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` { "DUE_DATE_BEFORE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days. - -Please make sure to complete this task by the due date.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days.

Please make sure to complete this task by the due date.", "ctaParams": { "taskId": "task_1", }, @@ -23,9 +21,7 @@ Please make sure to complete this task by the due date.", "title": "View task", }, "DUE_DATE_OVERDUE_7D": { - "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue. - -Please complete this task as soon as possible.", + "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue.

Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -34,9 +30,7 @@ Please complete this task as soon as possible.", "title": "View task", }, "DUE_DATE_TODAY": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today. - -Please complete this task as soon as possible.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today.

Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -45,9 +39,7 @@ Please complete this task as soon as possible.", "title": "View task", }, "NO_DUE_DATE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion. - -If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -56,9 +48,7 @@ If you've already completed this task, please mark it as done in the portal.", "title": "View task", }, "NO_DUE_DATE_7D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending. - -If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -72,9 +62,7 @@ If you've already completed this task, please mark it as done in the portal.", exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = ` { "DUE_DATE_BEFORE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days. - -Please make sure to complete this task by the due date.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days.

Please make sure to complete this task by the due date.", "ctaParams": { "taskId": "task_1", }, @@ -92,9 +80,7 @@ Please make sure to complete this task by the due date.", "title": "View task", }, "DUE_DATE_OVERDUE_7D": { - "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue. - -Please complete this task as soon as possible.", + "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue.

Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -103,9 +89,7 @@ Please complete this task as soon as possible.", "title": "View task", }, "DUE_DATE_TODAY": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today. - -Please complete this task as soon as possible.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today.

Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -114,9 +98,7 @@ Please complete this task as soon as possible.", "title": "View task", }, "NO_DUE_DATE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion. - -If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -125,9 +107,7 @@ If you've already completed this task, please mark it as done in the portal.", "title": "View task", }, "NO_DUE_DATE_7D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending. - -If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, diff --git a/src/app/api/notification/notification.helpers.ts b/src/app/api/notification/notification.helpers.ts index 7b889f761..0ec1d34e4 100644 --- a/src/app/api/notification/notification.helpers.ts +++ b/src/app/api/notification/notification.helpers.ts @@ -228,28 +228,28 @@ export const getReminderEmailDetails = ( subject: '[Reminder] You have a task to complete', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ assigned to you that's still pending completion.\n\nIf you've already completed this task, please mark it as done in the portal.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.NO_DUE_DATE_7D]: { subject: '[Reminder] Task still pending', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ that was assigned to you a week ago and is still pending.\n\nIf you've already completed this task, please mark it as done in the portal.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.DUE_DATE_BEFORE_3D]: { subject: '[Due Soon] Task due in 3 days', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ due in 3 days.\n\nPlease make sure to complete this task by the due date.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ due in 3 days.

Please make sure to complete this task by the due date.`, ctaParams, }, [TaskReminderType.DUE_DATE_TODAY]: { subject: '[Due Soon] Task due today', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ due today.\n\nPlease complete this task as soon as possible.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ due today.

Please complete this task as soon as possible.`, ctaParams, }, [TaskReminderType.DUE_DATE_OVERDUE_3D]: { @@ -263,7 +263,7 @@ export const getReminderEmailDetails = ( subject: '[Overdue] Task overdue by one week', header, title, - body: `This is a friendly reminder that the task ‘${task.title}’ is now one week overdue.\n\nPlease complete this task as soon as possible.`, + body: `This is a friendly reminder that the task ‘${task.title}’ is now one week overdue.

Please complete this task as soon as possible.`, ctaParams, }, } From a93fdaff257a649292fd40b40b07533c0c4c4319 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 15:41:38 +0545 Subject: [PATCH 09/15] revert(OUT-3735): restore \n\n separator in reminder bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both

and

...

showed up as literal text — Copilot's email template escapes all HTML in the body. Reverting to \n\n so the source matches the PRD copy verbatim; the paragraph-rendering gap will be fixed platform-side by the Copilot team. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notification.helpers.test.ts.snap | 40 ++++++++++++++----- .../api/notification/notification.helpers.ts | 10 ++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap index 7991f1599..81f1621a7 100644 --- a/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap +++ b/src/app/api/notification/__snapshots__/notification.helpers.test.ts.snap @@ -3,7 +3,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` { "DUE_DATE_BEFORE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days.

Please make sure to complete this task by the due date.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days. + +Please make sure to complete this task by the due date.", "ctaParams": { "taskId": "task_1", }, @@ -21,7 +23,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` "title": "View task", }, "DUE_DATE_OVERDUE_7D": { - "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue.

Please complete this task as soon as possible.", + "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue. + +Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -30,7 +34,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` "title": "View task", }, "DUE_DATE_TODAY": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today.

Please complete this task as soon as possible.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today. + +Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -39,7 +45,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` "title": "View task", }, "NO_DUE_DATE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion. + +If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -48,7 +56,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` "title": "View task", }, "NO_DUE_DATE_7D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending. + +If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -62,7 +72,9 @@ exports[`getReminderEmailDetails matches snapshot for company recipient 1`] = ` exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = ` { "DUE_DATE_BEFORE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days.

Please make sure to complete this task by the due date.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due in 3 days. + +Please make sure to complete this task by the due date.", "ctaParams": { "taskId": "task_1", }, @@ -80,7 +92,9 @@ exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = "title": "View task", }, "DUE_DATE_OVERDUE_7D": { - "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue.

Please complete this task as soon as possible.", + "body": "This is a friendly reminder that the task ‘Submit timesheet’ is now one week overdue. + +Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -89,7 +103,9 @@ exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = "title": "View task", }, "DUE_DATE_TODAY": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today.

Please complete this task as soon as possible.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ due today. + +Please complete this task as soon as possible.", "ctaParams": { "taskId": "task_1", }, @@ -98,7 +114,9 @@ exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = "title": "View task", }, "NO_DUE_DATE_3D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ assigned to you that's still pending completion. + +If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, @@ -107,7 +125,9 @@ exports[`getReminderEmailDetails matches snapshot for individual recipient 1`] = "title": "View task", }, "NO_DUE_DATE_7D": { - "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.", + "body": "This is a friendly reminder that you have a task ‘Submit timesheet’ that was assigned to you a week ago and is still pending. + +If you've already completed this task, please mark it as done in the portal.", "ctaParams": { "taskId": "task_1", }, diff --git a/src/app/api/notification/notification.helpers.ts b/src/app/api/notification/notification.helpers.ts index 0ec1d34e4..7b889f761 100644 --- a/src/app/api/notification/notification.helpers.ts +++ b/src/app/api/notification/notification.helpers.ts @@ -228,28 +228,28 @@ export const getReminderEmailDetails = ( subject: '[Reminder] You have a task to complete', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ assigned to you that's still pending completion.

If you've already completed this task, please mark it as done in the portal.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ assigned to you that's still pending completion.\n\nIf you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.NO_DUE_DATE_7D]: { subject: '[Reminder] Task still pending', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ that was assigned to you a week ago and is still pending.

If you've already completed this task, please mark it as done in the portal.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ that was assigned to you a week ago and is still pending.\n\nIf you've already completed this task, please mark it as done in the portal.`, ctaParams, }, [TaskReminderType.DUE_DATE_BEFORE_3D]: { subject: '[Due Soon] Task due in 3 days', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ due in 3 days.

Please make sure to complete this task by the due date.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ due in 3 days.\n\nPlease make sure to complete this task by the due date.`, ctaParams, }, [TaskReminderType.DUE_DATE_TODAY]: { subject: '[Due Soon] Task due today', header, title, - body: `This is a friendly reminder that you have a task ‘${task.title}’ due today.

Please complete this task as soon as possible.`, + body: `This is a friendly reminder that you have a task ‘${task.title}’ due today.\n\nPlease complete this task as soon as possible.`, ctaParams, }, [TaskReminderType.DUE_DATE_OVERDUE_3D]: { @@ -263,7 +263,7 @@ export const getReminderEmailDetails = ( subject: '[Overdue] Task overdue by one week', header, title, - body: `This is a friendly reminder that the task ‘${task.title}’ is now one week overdue.

Please complete this task as soon as possible.`, + body: `This is a friendly reminder that the task ‘${task.title}’ is now one week overdue.\n\nPlease complete this task as soon as possible.`, ctaParams, }, } From cbab0ebc019e483726ebfaeb9c36aa073b54be95 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 16:16:30 +0545 Subject: [PATCH 10/15] perf(OUT-3730): offload reminder sends to dispatchReminderEmail task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors auto-archive's dispatchTaskArchivedWebhook pattern. The cron used to call copilot.createNotification sequentially within each workspace; a company task with 50 members forced 50 serial round-trips inside the scheduled task's wall-clock budget. Now the cron: 1. Resolves recipients (still includes copilot.getCompanyClients fan-out). 2. Inserts the ledger with ON CONFLICT DO NOTHING. 3. batchTriggers one dispatch-reminder-email per net-new ledger row. Each dispatch-reminder-email is its own Trigger.dev task with: * queue.concurrencyLimit = 5 (global parallelism across all workspaces). * retry.maxAttempts = 3 with exponential backoff (transient 5xx no longer costs a day of reminders). * onFailure hook that DELETEs the ledger row after retries exhaust, so the next cron run retries. Compensating in onFailure (not inline catch) avoids dropping the ledger on transient failures a retry would recover. Cron's per-workspace totals shift from {sent, failed, skipped} to {enqueued, skipped} — per-send success/failure is now tracked in the dispatcher's Trigger.dev logs. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../dispatch-reminder-email.test.ts | 110 +++++++++++++++ .../notifications/dispatch-reminder-email.ts | 72 ++++++++++ src/jobs/notifications/index.ts | 1 + .../notifications/send-task-reminders.test.ts | 132 ++++++------------ src/jobs/notifications/send-task-reminders.ts | 74 ++++------ 5 files changed, 252 insertions(+), 137 deletions(-) create mode 100644 src/jobs/notifications/dispatch-reminder-email.test.ts create mode 100644 src/jobs/notifications/dispatch-reminder-email.ts diff --git a/src/jobs/notifications/dispatch-reminder-email.test.ts b/src/jobs/notifications/dispatch-reminder-email.test.ts new file mode 100644 index 000000000..a9188e6dc --- /dev/null +++ b/src/jobs/notifications/dispatch-reminder-email.test.ts @@ -0,0 +1,110 @@ +import { TaskReminderType } from '@prisma/client' + +const mockSendReminderEmail = jest.fn() +const mockTaskReminderSentDelete = jest.fn() +const mockCopilotApiCtor = jest.fn() + +jest.mock('@trigger.dev/sdk/v3', () => ({ + task: ({ run }: { run: (payload: unknown) => unknown }) => ({ run }), + tasks: { onFailure: () => undefined }, + logger: { log: jest.fn(), error: jest.fn(), warn: jest.fn() }, +})) + +jest.mock('@/config', () => ({ copilotAPIKey: 'test-api-key' })) + +jest.mock('@/lib/db', () => ({ + __esModule: true, + default: { + getInstance: () => ({ + taskReminderSent: { delete: mockTaskReminderSentDelete }, + }), + }, +})) + +jest.mock('@/utils/CopilotAPI', () => ({ + CopilotAPI: jest.fn().mockImplementation((...args: unknown[]) => { + mockCopilotApiCtor(...args) + return {} + }), +})) + +jest.mock('./send-reminder-email', () => ({ + sendReminderEmail: (...args: unknown[]) => mockSendReminderEmail(...args), +})) + +import { + DispatchReminderEmailPayload, + dispatchReminderEmailOnFailure, + dispatchReminderEmailRun, +} from './dispatch-reminder-email' + +const buildPayload = (overrides: Partial = {}): DispatchReminderEmailPayload => ({ + ledgerId: 'ledger_1', + workspaceId: 'ws_1', + task: { id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }, + recipientClientId: 'client_1', + recipientCompanyId: 'company_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + isCompanyRecipient: false, + workspace: { id: 'ws_1', brandName: 'Acme' }, + ...overrides, +}) + +describe('dispatchReminderEmail', () => { + beforeEach(() => { + jest.clearAllMocks() + mockSendReminderEmail.mockReset() + mockTaskReminderSentDelete.mockReset() + mockCopilotApiCtor.mockReset() + }) + + describe('run', () => { + it('mints a workspace-scoped CopilotAPI and forwards the payload to sendReminderEmail', async () => { + mockSendReminderEmail.mockResolvedValueOnce('notif_1') + + const result = await dispatchReminderEmailRun(buildPayload()) + + expect(mockCopilotApiCtor).toHaveBeenCalledWith('', 'ws_1/test-api-key') + expect(mockSendReminderEmail).toHaveBeenCalledTimes(1) + expect(mockSendReminderEmail.mock.calls[0][0]).toMatchObject({ + task: { id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }, + recipientClientId: 'client_1', + recipientCompanyId: 'company_1', + reminderType: TaskReminderType.NO_DUE_DATE_3D, + isCompanyRecipient: false, + }) + expect(result).toEqual({ ledgerId: 'ledger_1', notificationId: 'notif_1', sent: true }) + }) + + it('rethrows so Trigger.dev can apply its retry policy', async () => { + mockSendReminderEmail.mockRejectedValueOnce(new Error('copilot 5xx')) + + await expect(dispatchReminderEmailRun(buildPayload())).rejects.toThrow('copilot 5xx') + expect(mockTaskReminderSentDelete).not.toHaveBeenCalled() // compensation is onFailure's job, not run's + }) + }) + + describe('onFailure', () => { + it('deletes the ledger row so the next cron run can retry', async () => { + mockTaskReminderSentDelete.mockResolvedValueOnce({ id: 'ledger_1' }) + + await dispatchReminderEmailOnFailure({ + payload: buildPayload(), + error: new Error('all retries exhausted'), + }) + + expect(mockTaskReminderSentDelete).toHaveBeenCalledWith({ where: { id: 'ledger_1' } }) + }) + + it('does not throw if the ledger DELETE itself fails (logs and moves on)', async () => { + mockTaskReminderSentDelete.mockRejectedValueOnce(new Error('db blew up')) + + await expect( + dispatchReminderEmailOnFailure({ + payload: buildPayload(), + error: new Error('all retries exhausted'), + }), + ).resolves.toBeUndefined() + }) + }) +}) diff --git a/src/jobs/notifications/dispatch-reminder-email.ts b/src/jobs/notifications/dispatch-reminder-email.ts new file mode 100644 index 000000000..481d0960a --- /dev/null +++ b/src/jobs/notifications/dispatch-reminder-email.ts @@ -0,0 +1,72 @@ +import 'server-only' + +import { copilotAPIKey } from '@/config' +import DBClient from '@/lib/db' +import { WorkspaceResponse } from '@/types/common' +import { CopilotAPI } from '@/utils/CopilotAPI' +import { Task, TaskReminderType } from '@prisma/client' +import { logger, task, tasks } from '@trigger.dev/sdk/v3' + +import { sendReminderEmail } from './send-reminder-email' + +export type DispatchReminderEmailPayload = { + ledgerId: string + workspaceId: string + task: Pick + recipientClientId: string + recipientCompanyId: string | null + reminderType: TaskReminderType + isCompanyRecipient: boolean + workspace: WorkspaceResponse +} + +const TASK_ID = 'dispatch-reminder-email' + +const serializeError = (err: unknown) => (err instanceof Error ? { message: err.message, stack: err.stack } : err) + +export const dispatchReminderEmailRun = async (payload: DispatchReminderEmailPayload) => { + const copilot = new CopilotAPI('', `${payload.workspaceId}/${copilotAPIKey}`) + const notificationId = await sendReminderEmail({ + task: payload.task, + recipientClientId: payload.recipientClientId, + recipientCompanyId: payload.recipientCompanyId, + reminderType: payload.reminderType, + isCompanyRecipient: payload.isCompanyRecipient, + workspace: payload.workspace, + copilot, + }) + return { ledgerId: payload.ledgerId, notificationId, sent: true as const } +} + +// Fires after Trigger.dev exhausts all retries. Compensating here (instead of inside run's +// catch) avoids dropping the ledger row on transient failures a retry would have recovered. +export const dispatchReminderEmailOnFailure = async ({ payload, error }: { payload: unknown; error: unknown }) => { + const p = payload as DispatchReminderEmailPayload + logger.error('dispatch-reminder-email: retries exhausted, compensating ledger', { + ledgerId: p.ledgerId, + workspaceId: p.workspaceId, + taskId: p.task.id, + recipientClientId: p.recipientClientId, + reminderType: p.reminderType, + error: serializeError(error), + }) + const db = DBClient.getInstance() + try { + await db.taskReminderSent.delete({ where: { id: p.ledgerId } }) + } catch (deleteErr) { + logger.error('dispatch-reminder-email: ledger compensation DELETE failed, reminder will not retry', { + ledgerId: p.ledgerId, + error: serializeError(deleteErr), + }) + } +} + +export const dispatchReminderEmail = task({ + id: TASK_ID, + queue: { concurrencyLimit: 5 }, + retry: { maxAttempts: 3, factor: 2, minTimeoutInMs: 1_000, maxTimeoutInMs: 15_000, randomize: true }, + maxDuration: 30, + run: dispatchReminderEmailRun, +}) + +tasks.onFailure(TASK_ID, dispatchReminderEmailOnFailure) diff --git a/src/jobs/notifications/index.ts b/src/jobs/notifications/index.ts index 552e8240b..8046314bc 100644 --- a/src/jobs/notifications/index.ts +++ b/src/jobs/notifications/index.ts @@ -3,3 +3,4 @@ export { sendTaskCreateNotifications } from './send-task-create-notifications' export { sendTaskUpdateNotifications } from './send-task-update-notifications' export { sendCommentCreateNotifications } from './send-comment-create-notifications' export { sendTaskReminders } from './send-task-reminders' +export { dispatchReminderEmail } from './dispatch-reminder-email' diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts index 999a98656..201079532 100644 --- a/src/jobs/notifications/send-task-reminders.test.ts +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -1,14 +1,8 @@ import { AssigneeType, TaskReminderType } from '@prisma/client' -// Mocks must be configured before requiring the SUT. Variables referenced inside the -// jest.mock factory must start with `mock` so the babel-jest allow-list lets the closure -// see them once the const declarations have run. const mockTaskReminderSentCreateManyAndReturn = jest.fn() -const mockTaskReminderSentDelete = jest.fn() - const mockGetEligibleReminders = jest.fn() -const mockSendReminderEmail = jest.fn() - +const mockBatchTrigger = jest.fn() const mockGetWorkspace = jest.fn() const mockGetCompanyClients = jest.fn() const mockCopilotApiCtor = jest.fn() @@ -20,18 +14,13 @@ jest.mock('@trigger.dev/sdk/v3', () => ({ logger: { log: jest.fn(), error: jest.fn(), warn: jest.fn() }, })) -jest.mock('@/config', () => ({ - copilotAPIKey: 'test-api-key', -})) +jest.mock('@/config', () => ({ copilotAPIKey: 'test-api-key' })) jest.mock('@/lib/db', () => ({ __esModule: true, default: { getInstance: () => ({ - taskReminderSent: { - createManyAndReturn: mockTaskReminderSentCreateManyAndReturn, - delete: mockTaskReminderSentDelete, - }, + taskReminderSent: { createManyAndReturn: mockTaskReminderSentCreateManyAndReturn }, }), }, })) @@ -39,10 +28,7 @@ jest.mock('@/lib/db', () => ({ jest.mock('@/utils/CopilotAPI', () => ({ CopilotAPI: jest.fn().mockImplementation((...args: unknown[]) => { mockCopilotApiCtor(...args) - return { - getWorkspace: mockGetWorkspace, - getCompanyClients: mockGetCompanyClients, - } + return { getWorkspace: mockGetWorkspace, getCompanyClients: mockGetCompanyClients } }), })) @@ -50,12 +36,10 @@ jest.mock('./eligibility', () => ({ getEligibleReminders: (...args: unknown[]) => mockGetEligibleReminders(...args), })) -jest.mock('./send-reminder-email', () => ({ - sendReminderEmail: (...args: unknown[]) => mockSendReminderEmail(...args), +jest.mock('./dispatch-reminder-email', () => ({ + dispatchReminderEmail: { batchTrigger: (...args: unknown[]) => mockBatchTrigger(...args) }, })) -// Bypass Bottleneck's rate-limiting in tests but preserve sequential ordering per instance -// via a promise chain so FIFO mockResolvedValueOnce queues drain deterministically. jest.mock('bottleneck', () => ({ __esModule: true, default: jest.fn().mockImplementation(() => { @@ -72,11 +56,9 @@ jest.mock('bottleneck', () => ({ import { sendTaskReminders } from './send-task-reminders' -type RunResult = { sent: number; failed: number; skipped: number; workspaceCount: number } +type RunResult = { enqueued: number; skipped: number; workspaceCount: number } const runJob = async (): Promise => { - const { run } = sendTaskReminders as unknown as { - run: (payload: { timestamp: Date }) => Promise - } + const { run } = sendTaskReminders as unknown as { run: (payload: { timestamp: Date }) => Promise } return run({ timestamp: new Date() }) } @@ -98,13 +80,13 @@ describe('sendTaskReminders', () => { beforeEach(() => { jest.clearAllMocks() mockTaskReminderSentCreateManyAndReturn.mockReset() - mockTaskReminderSentDelete.mockReset() mockGetEligibleReminders.mockReset() - mockSendReminderEmail.mockReset() + mockBatchTrigger.mockReset() mockGetWorkspace.mockReset() mockGetCompanyClients.mockReset() mockCopilotApiCtor.mockReset() mockGetWorkspace.mockResolvedValue(workspace) + mockBatchTrigger.mockResolvedValue({ batchId: 'b1' }) }) it('exits cleanly when no rows are eligible', async () => { @@ -112,9 +94,9 @@ describe('sendTaskReminders', () => { const result = await runJob() - expect(result).toEqual({ sent: 0, failed: 0, skipped: 0, workspaceCount: 0 }) + expect(result).toEqual({ enqueued: 0, skipped: 0, workspaceCount: 0 }) expect(mockTaskReminderSentCreateManyAndReturn).not.toHaveBeenCalled() - expect(mockSendReminderEmail).not.toHaveBeenCalled() + expect(mockBatchTrigger).not.toHaveBeenCalled() expect(mockCopilotApiCtor).not.toHaveBeenCalled() }) @@ -126,27 +108,24 @@ describe('sendTaskReminders', () => { const result = await runJob() expect(result.workspaceCount).toBe(0) - expect(mockTaskReminderSentCreateManyAndReturn).not.toHaveBeenCalled() - expect(mockSendReminderEmail).not.toHaveBeenCalled() + expect(mockBatchTrigger).not.toHaveBeenCalled() }) - it('sends one reminder for a client-assigned task and writes one ledger row', async () => { + it('enqueues one dispatch per net-new ledger row (client-assigned)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ - { - id: 'ledger_1', - taskId: 'task_1', - recipientId: 'client_1', - reminderType: TaskReminderType.NO_DUE_DATE_3D, - }, + { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, ]) - mockSendReminderEmail.mockResolvedValueOnce('notif_1') const result = await runJob() - expect(result).toEqual({ sent: 1, failed: 0, skipped: 0, workspaceCount: 1 }) - expect(mockSendReminderEmail).toHaveBeenCalledTimes(1) - expect(mockSendReminderEmail.mock.calls[0][0]).toMatchObject({ + expect(result).toEqual({ enqueued: 1, skipped: 0, workspaceCount: 1 }) + expect(mockBatchTrigger).toHaveBeenCalledTimes(1) + const batch = mockBatchTrigger.mock.calls[0][0] + expect(batch).toHaveLength(1) + expect(batch[0].payload).toMatchObject({ + ledgerId: 'ledger_1', + workspaceId: 'ws_1', task: { id: 'task_1', title: 'Submit timesheet', createdById: 'iu_1' }, recipientClientId: 'client_1', recipientCompanyId: 'company_1', @@ -156,17 +135,11 @@ describe('sendTaskReminders', () => { }) }) - it('initializes CopilotAPI with a workspace-scoped apiKey (no user token mint)', async () => { + it('initializes CopilotAPI with a workspace-scoped apiKey', async () => { mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ - { - id: 'ledger_1', - taskId: 'task_1', - recipientId: 'client_1', - reminderType: TaskReminderType.NO_DUE_DATE_3D, - }, + { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, ]) - mockSendReminderEmail.mockResolvedValueOnce('notif_1') await runJob() @@ -179,17 +152,13 @@ describe('sendTaskReminders', () => { const result = await runJob() - expect(result).toEqual({ sent: 0, failed: 0, skipped: 1, workspaceCount: 1 }) - expect(mockSendReminderEmail).not.toHaveBeenCalled() + expect(result).toEqual({ enqueued: 0, skipped: 1, workspaceCount: 1 }) + expect(mockBatchTrigger).not.toHaveBeenCalled() }) - it('fans out a company-assigned task to one send per current member', async () => { + it('fans out a company-assigned task to one dispatch per current member', async () => { mockGetEligibleReminders.mockResolvedValueOnce([ - buildRow({ - assigneeType: AssigneeType.company, - assigneeId: 'company_1', - companyId: 'company_1', - }), + buildRow({ assigneeType: AssigneeType.company, assigneeId: 'company_1', companyId: 'company_1' }), ]) mockGetCompanyClients.mockResolvedValueOnce([{ id: 'm_1' }, { id: 'm_2' }, { id: 'm_3' }]) mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ @@ -197,29 +166,19 @@ describe('sendTaskReminders', () => { { id: 'l_2', taskId: 'task_1', recipientId: 'm_2', reminderType: TaskReminderType.NO_DUE_DATE_3D }, { id: 'l_3', taskId: 'task_1', recipientId: 'm_3', reminderType: TaskReminderType.NO_DUE_DATE_3D }, ]) - mockSendReminderEmail.mockResolvedValue('notif') const result = await runJob() - expect(result).toEqual({ sent: 3, failed: 0, skipped: 0, workspaceCount: 1 }) - expect(mockSendReminderEmail).toHaveBeenCalledTimes(3) - const recipientIds = mockSendReminderEmail.mock.calls.map((c) => c[0].recipientClientId).sort() - expect(recipientIds).toEqual(['m_1', 'm_2', 'm_3']) - expect(mockSendReminderEmail.mock.calls[0][0].isCompanyRecipient).toBe(true) - }) - - it('compensates the ledger when Copilot send fails', async () => { - mockGetEligibleReminders.mockResolvedValueOnce([buildRow()]) - mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ - { id: 'ledger_1', taskId: 'task_1', recipientId: 'client_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + expect(result).toEqual({ enqueued: 3, skipped: 0, workspaceCount: 1 }) + expect(mockBatchTrigger).toHaveBeenCalledTimes(1) + const batch = mockBatchTrigger.mock.calls[0][0] + expect(batch).toHaveLength(3) + expect(batch.map((b: { payload: { recipientClientId: string } }) => b.payload.recipientClientId).sort()).toEqual([ + 'm_1', + 'm_2', + 'm_3', ]) - mockSendReminderEmail.mockRejectedValueOnce(new Error('copilot 5xx')) - mockTaskReminderSentDelete.mockResolvedValueOnce({ id: 'ledger_1' }) - - const result = await runJob() - - expect(result).toEqual({ sent: 0, failed: 1, skipped: 0, workspaceCount: 1 }) - expect(mockTaskReminderSentDelete).toHaveBeenCalledWith({ where: { id: 'ledger_1' } }) + expect(batch[0].payload.isCompanyRecipient).toBe(true) }) it('does not abort the sweep when one workspace throws (per-workspace isolation)', async () => { @@ -227,20 +186,15 @@ describe('sendTaskReminders', () => { buildRow({ workspaceId: 'ws_bad', taskId: 'task_bad' }), buildRow({ workspaceId: 'ws_good', taskId: 'task_good', assigneeId: 'client_good' }), ]) - // ws_bad's ledger insert throws; ws_good completes a single send. - mockTaskReminderSentCreateManyAndReturn.mockRejectedValueOnce(new Error('db blew up')).mockResolvedValueOnce([ - { - id: 'ledger_g', - taskId: 'task_good', - recipientId: 'client_good', - reminderType: TaskReminderType.NO_DUE_DATE_3D, - }, - ]) - mockSendReminderEmail.mockResolvedValueOnce('notif_good') + mockTaskReminderSentCreateManyAndReturn + .mockRejectedValueOnce(new Error('db blew up')) + .mockResolvedValueOnce([ + { id: 'ledger_g', taskId: 'task_good', recipientId: 'client_good', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + ]) const result = await runJob() expect(result.workspaceCount).toBe(2) - expect(result.sent).toBe(1) + expect(result.enqueued).toBe(1) }) }) diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index e6857a223..064f23ddb 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -8,12 +8,12 @@ import { AssigneeType, TaskReminderType } from '@prisma/client' import { logger, schedules } from '@trigger.dev/sdk/v3' import Bottleneck from 'bottleneck' +import { dispatchReminderEmail, DispatchReminderEmailPayload } from './dispatch-reminder-email' import { EligibilityRow, getEligibleReminders } from './eligibility' -import { sendReminderEmail } from './send-reminder-email' const WORKSPACE_CONCURRENCY = 5 -type WorkspaceTotals = { sent: number; failed: number; skipped: number } +type WorkspaceTotals = { enqueued: number; skipped: number } type Recipient = { clientId: string; companyId: string | null } @@ -47,7 +47,7 @@ export const sendTaskReminders = schedules.task({ runAt: payload.timestamp, }) - const totals = { sent: 0, failed: 0, skipped: 0 } + const totals = { enqueued: 0, skipped: 0 } let processed = 0 const workspaceCount = byWorkspace.size @@ -56,7 +56,7 @@ export const sendTaskReminders = schedules.task({ await Promise.allSettled( Array.from(byWorkspace.entries()).map(([workspaceId, workspaceRows]) => workspaceBottleneck.schedule(async () => { - let wsTotals: WorkspaceTotals = { sent: 0, failed: 0, skipped: 0 } + let wsTotals: WorkspaceTotals = { enqueued: 0, skipped: 0 } try { wsTotals = await processWorkspace(db, workspaceId, workspaceRows) } catch (err) { @@ -65,12 +65,11 @@ export const sendTaskReminders = schedules.task({ error: serializeError(err), }) } finally { - totals.sent += wsTotals.sent - totals.failed += wsTotals.failed + totals.enqueued += wsTotals.enqueued totals.skipped += wsTotals.skipped processed += 1 logger.log( - `[${processed}/${workspaceCount}] workspace ${workspaceId}: sent ${wsTotals.sent}, failed ${wsTotals.failed}, skipped ${wsTotals.skipped}`, + `[${processed}/${workspaceCount}] workspace ${workspaceId}: enqueued ${wsTotals.enqueued}, skipped ${wsTotals.skipped}`, { workspaceId, ...wsTotals, processed, eligibleWorkspaces: workspaceCount }, ) } @@ -106,7 +105,7 @@ const processWorkspace = async ( } } - if (plan.length === 0) return { sent: 0, failed: 0, skipped: 0 } + if (plan.length === 0) return { enqueued: 0, skipped: 0 } // Ledger insert before send: the unique constraint is the dedupe primitive. const inserted = await db.taskReminderSent.createManyAndReturn({ @@ -120,54 +119,33 @@ const processWorkspace = async ( }) const insertedKey = (taskId: string, recipientId: string, type: TaskReminderType) => `${taskId}|${recipientId}|${type}` - const insertedById = new Map(inserted.map((r) => [insertedKey(r.taskId, r.recipientId, r.reminderType), r.id])) - - const skipped = plan.length - inserted.length - - let sent = 0 - let failed = 0 - - for (const entry of plan) { - const ledgerId = insertedById.get(insertedKey(entry.row.taskId, entry.recipient.clientId, entry.row.reminderType)) - if (!ledgerId) continue - - try { - await sendReminderEmail({ + const planByKey = new Map( + plan.map((e) => [insertedKey(e.row.taskId, e.recipient.clientId, e.row.reminderType), e]), + ) + + const triggers: { payload: DispatchReminderEmailPayload }[] = [] + for (const row of inserted) { + const entry = planByKey.get(insertedKey(row.taskId, row.recipientId, row.reminderType)) + if (!entry) continue + triggers.push({ + payload: { + ledgerId: row.id, + workspaceId, task: { id: entry.row.taskId, title: entry.row.title, createdById: entry.row.createdById }, recipientClientId: entry.recipient.clientId, recipientCompanyId: entry.recipient.companyId, reminderType: entry.row.reminderType, isCompanyRecipient: entry.row.assigneeType === AssigneeType.company, workspace, - copilot, - }) - sent += 1 - } catch (err) { - // Delete the ledger row so the next cron run retries. - failed += 1 - logger.error('send-task-reminders: Copilot send failed, compensating ledger', { - workspaceId, - taskId: entry.row.taskId, - recipientClientId: entry.recipient.clientId, - reminderType: entry.row.reminderType, - error: serializeError(err), - }) - try { - await db.taskReminderSent.delete({ where: { id: ledgerId } }) - } catch (deleteErr) { - logger.error('send-task-reminders: ledger compensation DELETE failed, reminder will not retry', { - workspaceId, - ledgerId, - taskId: entry.row.taskId, - recipientClientId: entry.recipient.clientId, - reminderType: entry.row.reminderType, - error: serializeError(deleteErr), - }) - } - } + }, + }) + } + + if (triggers.length > 0) { + await dispatchReminderEmail.batchTrigger(triggers) } - return { sent, failed, skipped } + return { enqueued: triggers.length, skipped: plan.length - inserted.length } } const resolveRecipients = async (copilot: CopilotAPI, row: EligibilityRow): Promise => { From 7c99164a6fb500fb6517da0a670225af61a5f999 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 16:39:45 +0545 Subject: [PATCH 11/15] fix(OUT-3730): chunk batchTrigger at 500 and compensate ledger on failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trigger.dev caps batchTrigger at 500 items per call. A workspace with a single company task fanning out to 1700+ members blew past that and threw BatchTriggerError, leaving the ledger rows orphaned — the unique constraint then blocked any future cron from re-sending those reminders. Two fixes: 1. Chunk triggers into 500-item batches so any workspace fits. 2. On per-chunk batchTrigger failure, deleteMany the chunk's ledger rows so the next cron run can retry. Same compensation contract as the per-row dispatcher's onFailure hook, just scoped to the chunk. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notifications/send-task-reminders.test.ts | 58 ++++++++++++++++++- src/jobs/notifications/send-task-reminders.ts | 34 ++++++++++- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts index 201079532..d662727fe 100644 --- a/src/jobs/notifications/send-task-reminders.test.ts +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -1,6 +1,7 @@ import { AssigneeType, TaskReminderType } from '@prisma/client' const mockTaskReminderSentCreateManyAndReturn = jest.fn() +const mockTaskReminderSentDeleteMany = jest.fn() const mockGetEligibleReminders = jest.fn() const mockBatchTrigger = jest.fn() const mockGetWorkspace = jest.fn() @@ -20,7 +21,10 @@ jest.mock('@/lib/db', () => ({ __esModule: true, default: { getInstance: () => ({ - taskReminderSent: { createManyAndReturn: mockTaskReminderSentCreateManyAndReturn }, + taskReminderSent: { + createManyAndReturn: mockTaskReminderSentCreateManyAndReturn, + deleteMany: mockTaskReminderSentDeleteMany, + }, }), }, })) @@ -80,6 +84,7 @@ describe('sendTaskReminders', () => { beforeEach(() => { jest.clearAllMocks() mockTaskReminderSentCreateManyAndReturn.mockReset() + mockTaskReminderSentDeleteMany.mockReset() mockGetEligibleReminders.mockReset() mockBatchTrigger.mockReset() mockGetWorkspace.mockReset() @@ -181,6 +186,57 @@ describe('sendTaskReminders', () => { expect(batch[0].payload.isCompanyRecipient).toBe(true) }) + it('chunks batchTrigger calls so a workspace with >500 fanned-out sends still enqueues', async () => { + // One company task fanning out to 1200 members → 1200 dispatch payloads → 3 chunks of 500. + const members = Array.from({ length: 1200 }, (_, i) => ({ id: `m_${i}` })) + const ledgerRows = members.map((m, i) => ({ + id: `l_${i}`, + taskId: 'task_1', + recipientId: m.id, + reminderType: TaskReminderType.NO_DUE_DATE_3D, + })) + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ assigneeType: AssigneeType.company, assigneeId: 'company_1', companyId: 'company_1' }), + ]) + mockGetCompanyClients.mockResolvedValueOnce(members) + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce(ledgerRows) + + const result = await runJob() + + expect(result).toEqual({ enqueued: 1200, skipped: 0, workspaceCount: 1 }) + expect(mockBatchTrigger).toHaveBeenCalledTimes(3) + expect(mockBatchTrigger.mock.calls[0][0]).toHaveLength(500) + expect(mockBatchTrigger.mock.calls[1][0]).toHaveLength(500) + expect(mockBatchTrigger.mock.calls[2][0]).toHaveLength(200) + expect(mockTaskReminderSentDeleteMany).not.toHaveBeenCalled() + }) + + it('compensates the ledger when a batchTrigger chunk fails', async () => { + const members = Array.from({ length: 800 }, (_, i) => ({ id: `m_${i}` })) + const ledgerRows = members.map((m, i) => ({ + id: `l_${i}`, + taskId: 'task_1', + recipientId: m.id, + reminderType: TaskReminderType.NO_DUE_DATE_3D, + })) + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ assigneeType: AssigneeType.company, assigneeId: 'company_1', companyId: 'company_1' }), + ]) + mockGetCompanyClients.mockResolvedValueOnce(members) + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce(ledgerRows) + // First chunk (500) succeeds, second (300) fails. + mockBatchTrigger.mockResolvedValueOnce({ batchId: 'b1' }).mockRejectedValueOnce(new Error('trigger.dev 5xx')) + + const result = await runJob() + + expect(result.enqueued).toBe(500) + expect(mockTaskReminderSentDeleteMany).toHaveBeenCalledTimes(1) + const deleteArgs = mockTaskReminderSentDeleteMany.mock.calls[0][0] + expect(deleteArgs.where.id.in).toHaveLength(300) // failed chunk's ledger rows + expect(deleteArgs.where.id.in[0]).toBe('l_500') + expect(deleteArgs.where.id.in[299]).toBe('l_799') + }) + it('does not abort the sweep when one workspace throws (per-workspace isolation)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([ buildRow({ workspaceId: 'ws_bad', taskId: 'task_bad' }), diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index 064f23ddb..1127e737a 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -12,6 +12,9 @@ import { dispatchReminderEmail, DispatchReminderEmailPayload } from './dispatch- import { EligibilityRow, getEligibleReminders } from './eligibility' const WORKSPACE_CONCURRENCY = 5 +// Trigger.dev caps batchTrigger at 500 items per call; chunk so a single workspace with +// thousands of fanned-out sends still gets enqueued. +const BATCH_TRIGGER_CHUNK_SIZE = 500 type WorkspaceTotals = { enqueued: number; skipped: number } @@ -141,11 +144,36 @@ const processWorkspace = async ( }) } - if (triggers.length > 0) { - await dispatchReminderEmail.batchTrigger(triggers) + let enqueued = 0 + for (let i = 0; i < triggers.length; i += BATCH_TRIGGER_CHUNK_SIZE) { + const chunk = triggers.slice(i, i + BATCH_TRIGGER_CHUNK_SIZE) + try { + await dispatchReminderEmail.batchTrigger(chunk) + enqueued += chunk.length + } catch (err) { + // Compensate: drop the chunk's ledger rows so the next cron run retries them. + // Without this, the rows are orphans: the unique constraint blocks future inserts + // but no dispatcher will ever consume them. + const ledgerIds = chunk.map((t) => t.payload.ledgerId) + logger.error('send-task-reminders: batchTrigger failed, compensating ledger', { + workspaceId, + chunkSize: chunk.length, + chunkOffset: i, + error: serializeError(err), + }) + try { + await db.taskReminderSent.deleteMany({ where: { id: { in: ledgerIds } } }) + } catch (deleteErr) { + logger.error('send-task-reminders: ledger compensation deleteMany failed, ledger rows orphaned', { + workspaceId, + ledgerIds, + error: serializeError(deleteErr), + }) + } + } } - return { enqueued: triggers.length, skipped: plan.length - inserted.length } + return { enqueued, skipped: plan.length - inserted.length } } const resolveRecipients = async (copilot: CopilotAPI, row: EligibilityRow): Promise => { From d34045832a7dc70db5131693aad71431ebb234f9 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 17:12:39 +0545 Subject: [PATCH 12/15] =?UTF-8?q?refactor(OUT-3730):=20rename=20row=20?= =?UTF-8?q?=E2=86=92=20task=20per=20PR=20review=20(priosshrsth)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer's three rename suggestions, plus the cascaded references: * allRows → eligibleTasks * rows (filtered) → tasks * byWorkspace → tasksByWorkspace * workspaceRows param → workspaceTasks * processWorkspace's `rows` param → `tasks` * LedgerPlanEntry.row field → .task (so entry.row.X reads as entry.task.X) * Loop variable in resolveRecipients renamed for symmetry Variable referring to inserted ledger rows (`for (const row of inserted)`) intentionally kept as `row` — that's a SQL row, not a task. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/send-task-reminders.ts | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index 1127e737a..e863990b4 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -21,7 +21,7 @@ type WorkspaceTotals = { enqueued: number; skipped: number } type Recipient = { clientId: string; companyId: string | null } type LedgerPlanEntry = { - row: EligibilityRow + task: EligibilityRow recipient: Recipient } @@ -32,36 +32,36 @@ export const sendTaskReminders = schedules.task({ run: async (payload) => { const db = DBClient.getInstance() - const allRows = await getEligibleReminders(db) - const rows = allRows.filter((r) => r.assigneeType !== AssigneeType.internalUser) + const eligibleTasks = await getEligibleReminders(db) + const tasks = eligibleTasks.filter((t) => t.assigneeType !== AssigneeType.internalUser) - const byWorkspace = new Map() - for (const row of rows) { - const bucket = byWorkspace.get(row.workspaceId) - if (bucket) bucket.push(row) - else byWorkspace.set(row.workspaceId, [row]) + const tasksByWorkspace = new Map() + for (const task of tasks) { + const bucket = tasksByWorkspace.get(task.workspaceId) + if (bucket) bucket.push(task) + else tasksByWorkspace.set(task.workspaceId, [task]) } logger.log('send-task-reminders: sweep starting', { - totalEligible: allRows.length, - afterIuFilter: rows.length, - eligibleWorkspaces: byWorkspace.size, + totalEligible: eligibleTasks.length, + afterIuFilter: tasks.length, + eligibleWorkspaces: tasksByWorkspace.size, workspaceConcurrency: WORKSPACE_CONCURRENCY, runAt: payload.timestamp, }) const totals = { enqueued: 0, skipped: 0 } let processed = 0 - const workspaceCount = byWorkspace.size + const workspaceCount = tasksByWorkspace.size const workspaceBottleneck = new Bottleneck({ maxConcurrent: WORKSPACE_CONCURRENCY }) await Promise.allSettled( - Array.from(byWorkspace.entries()).map(([workspaceId, workspaceRows]) => + Array.from(tasksByWorkspace.entries()).map(([workspaceId, workspaceTasks]) => workspaceBottleneck.schedule(async () => { let wsTotals: WorkspaceTotals = { enqueued: 0, skipped: 0 } try { - wsTotals = await processWorkspace(db, workspaceId, workspaceRows) + wsTotals = await processWorkspace(db, workspaceId, workspaceTasks) } catch (err) { logger.error('send-task-reminders: workspace failed', { workspaceId, @@ -83,7 +83,7 @@ export const sendTaskReminders = schedules.task({ logger.log('send-task-reminders: sweep complete', { ...totals, workspaceCount, - totalEligible: allRows.length, + totalEligible: eligibleTasks.length, }) return { ...totals, workspaceCount } @@ -93,7 +93,7 @@ export const sendTaskReminders = schedules.task({ const processWorkspace = async ( db: ReturnType, workspaceId: string, - rows: EligibilityRow[], + tasks: EligibilityRow[], ): Promise => { // Workspace-scoped apiKey: empty token + `${workspaceId}/${apiKey}` is accepted by the // SDK when COPILOT_ENV is set on the Trigger.dev runtime. @@ -101,10 +101,10 @@ const processWorkspace = async ( const workspace = await copilot.getWorkspace() const plan: LedgerPlanEntry[] = [] - for (const row of rows) { - const recipients = await resolveRecipients(copilot, row) + for (const task of tasks) { + const recipients = await resolveRecipients(copilot, task) for (const recipient of recipients) { - plan.push({ row, recipient }) + plan.push({ task, recipient }) } } @@ -113,17 +113,17 @@ const processWorkspace = async ( // Ledger insert before send: the unique constraint is the dedupe primitive. const inserted = await db.taskReminderSent.createManyAndReturn({ data: plan.map((entry) => ({ - taskId: entry.row.taskId, + taskId: entry.task.taskId, workspaceId, recipientId: entry.recipient.clientId, - reminderType: entry.row.reminderType, + reminderType: entry.task.reminderType, })), skipDuplicates: true, }) const insertedKey = (taskId: string, recipientId: string, type: TaskReminderType) => `${taskId}|${recipientId}|${type}` const planByKey = new Map( - plan.map((e) => [insertedKey(e.row.taskId, e.recipient.clientId, e.row.reminderType), e]), + plan.map((e) => [insertedKey(e.task.taskId, e.recipient.clientId, e.task.reminderType), e]), ) const triggers: { payload: DispatchReminderEmailPayload }[] = [] @@ -134,11 +134,11 @@ const processWorkspace = async ( payload: { ledgerId: row.id, workspaceId, - task: { id: entry.row.taskId, title: entry.row.title, createdById: entry.row.createdById }, + task: { id: entry.task.taskId, title: entry.task.title, createdById: entry.task.createdById }, recipientClientId: entry.recipient.clientId, recipientCompanyId: entry.recipient.companyId, - reminderType: entry.row.reminderType, - isCompanyRecipient: entry.row.assigneeType === AssigneeType.company, + reminderType: entry.task.reminderType, + isCompanyRecipient: entry.task.assigneeType === AssigneeType.company, workspace, }, }) @@ -176,13 +176,13 @@ const processWorkspace = async ( return { enqueued, skipped: plan.length - inserted.length } } -const resolveRecipients = async (copilot: CopilotAPI, row: EligibilityRow): Promise => { - if (row.assigneeType === AssigneeType.client) { - return [{ clientId: row.assigneeId, companyId: row.companyId }] +const resolveRecipients = async (copilot: CopilotAPI, task: EligibilityRow): Promise => { + if (task.assigneeType === AssigneeType.client) { + return [{ clientId: task.assigneeId, companyId: task.companyId }] } - if (row.assigneeType === AssigneeType.company) { - const members: ClientResponse[] = await copilot.getCompanyClients(row.assigneeId) - return members.map((m) => ({ clientId: m.id, companyId: row.assigneeId })) + if (task.assigneeType === AssigneeType.company) { + const members: ClientResponse[] = await copilot.getCompanyClients(task.assigneeId) + return members.map((m) => ({ clientId: m.id, companyId: task.assigneeId })) } return [] } From b0b133759c41e21c178435a8f1c9b83b645b0db4 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 17:28:29 +0545 Subject: [PATCH 13/15] fix(OUT-3730): contain getCompanyClients failure to the failing task MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: a single getCompanyClients throw (after Copilot's own withRetry exhausts) would propagate out of the plan loop, leak through processWorkspace, and the outer try/catch would mark the entire workspace as failed — dropping every other eligible task in that workspace for the day, including client-assigned tasks that don't even need fan-out. After: per-task try/catch around resolveRecipients. The failing task is logged and skipped; siblings continue. No added retry — Copilot's internal retry is the only retry layer; this is just blast-radius containment. Resolves greptile P1 on PR #1258. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notifications/send-task-reminders.test.ts | 36 +++++++++++++++++++ src/jobs/notifications/send-task-reminders.ts | 17 ++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/jobs/notifications/send-task-reminders.test.ts b/src/jobs/notifications/send-task-reminders.test.ts index d662727fe..257dc5d5c 100644 --- a/src/jobs/notifications/send-task-reminders.test.ts +++ b/src/jobs/notifications/send-task-reminders.test.ts @@ -237,6 +237,42 @@ describe('sendTaskReminders', () => { expect(deleteArgs.where.id.in[299]).toBe('l_799') }) + it('skips a single task whose getCompanyClients fails without dropping siblings', async () => { + // Two company tasks in the same workspace. The first one's fan-out throws (Copilot + // exhausted its own retries); the second one should still get its reminder enqueued. + mockGetEligibleReminders.mockResolvedValueOnce([ + buildRow({ + taskId: 'task_bad', + assigneeType: AssigneeType.company, + assigneeId: 'company_bad', + companyId: 'company_bad', + }), + buildRow({ + taskId: 'task_good', + assigneeType: AssigneeType.company, + assigneeId: 'company_good', + companyId: 'company_good', + }), + ]) + mockGetCompanyClients + .mockRejectedValueOnce(new Error('copilot 5xx')) + .mockResolvedValueOnce([{ id: 'm_1' }, { id: 'm_2' }]) + mockTaskReminderSentCreateManyAndReturn.mockResolvedValueOnce([ + { id: 'l_1', taskId: 'task_good', recipientId: 'm_1', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + { id: 'l_2', taskId: 'task_good', recipientId: 'm_2', reminderType: TaskReminderType.NO_DUE_DATE_3D }, + ]) + + const result = await runJob() + + expect(result).toEqual({ enqueued: 2, skipped: 0, workspaceCount: 1 }) + expect(mockBatchTrigger).toHaveBeenCalledTimes(1) + const batch = mockBatchTrigger.mock.calls[0][0] + expect(batch.map((b: { payload: { recipientClientId: string } }) => b.payload.recipientClientId).sort()).toEqual([ + 'm_1', + 'm_2', + ]) + }) + it('does not abort the sweep when one workspace throws (per-workspace isolation)', async () => { mockGetEligibleReminders.mockResolvedValueOnce([ buildRow({ workspaceId: 'ws_bad', taskId: 'task_bad' }), diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index e863990b4..5f0521792 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -102,7 +102,22 @@ const processWorkspace = async ( const plan: LedgerPlanEntry[] = [] for (const task of tasks) { - const recipients = await resolveRecipients(copilot, task) + let recipients: Recipient[] + try { + recipients = await resolveRecipients(copilot, task) + } catch (err) { + // Contain blast radius to this task. Copilot is already wrapped in withRetry, so + // a thrown error means retries are exhausted — propagating would drop unrelated + // sibling tasks in the same workspace for the day. + logger.error('send-task-reminders: failed to resolve recipients, skipping task', { + workspaceId, + taskId: task.taskId, + assigneeType: task.assigneeType, + assigneeId: task.assigneeId, + error: serializeError(err), + }) + continue + } for (const recipient of recipients) { plan.push({ task, recipient }) } From 4961236fa78acabe5e1667d50ec00c7c1660eddb Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 20:37:37 +0545 Subject: [PATCH 14/15] refactor(OUT-3730): address PR review feedback (priosshrsth) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Extract serializeError to src/utils/serializeError.ts; drop the duplicated local copy in send-task-reminders.ts and dispatch-reminder-email.ts. * Simplify resolveRecipients — drop the dead `return []` branch since IUs are filtered upstream; the function now reads as "client by default, fan out only for company". * In dispatchReminderEmailOnFailure, replace the `p` alias with a typed destructure of the payload. The SDK's AnyOnFailureHookFunction types the payload as `unknown`, so we still cast once at destructure time, but downstream code reads the meaningful field names directly. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../notifications/dispatch-reminder-email.ts | 20 +++++++++---------- src/jobs/notifications/send-task-reminders.ts | 14 +++++-------- src/utils/serializeError.ts | 2 ++ 3 files changed, 17 insertions(+), 19 deletions(-) create mode 100644 src/utils/serializeError.ts diff --git a/src/jobs/notifications/dispatch-reminder-email.ts b/src/jobs/notifications/dispatch-reminder-email.ts index 481d0960a..8a80ff0db 100644 --- a/src/jobs/notifications/dispatch-reminder-email.ts +++ b/src/jobs/notifications/dispatch-reminder-email.ts @@ -4,6 +4,7 @@ import { copilotAPIKey } from '@/config' import DBClient from '@/lib/db' import { WorkspaceResponse } from '@/types/common' import { CopilotAPI } from '@/utils/CopilotAPI' +import { serializeError } from '@/utils/serializeError' import { Task, TaskReminderType } from '@prisma/client' import { logger, task, tasks } from '@trigger.dev/sdk/v3' @@ -22,8 +23,6 @@ export type DispatchReminderEmailPayload = { const TASK_ID = 'dispatch-reminder-email' -const serializeError = (err: unknown) => (err instanceof Error ? { message: err.message, stack: err.stack } : err) - export const dispatchReminderEmailRun = async (payload: DispatchReminderEmailPayload) => { const copilot = new CopilotAPI('', `${payload.workspaceId}/${copilotAPIKey}`) const notificationId = await sendReminderEmail({ @@ -40,22 +39,23 @@ export const dispatchReminderEmailRun = async (payload: DispatchReminderEmailPay // Fires after Trigger.dev exhausts all retries. Compensating here (instead of inside run's // catch) avoids dropping the ledger row on transient failures a retry would have recovered. +// The SDK types the hook's payload as `unknown`; we cast once via destructure. export const dispatchReminderEmailOnFailure = async ({ payload, error }: { payload: unknown; error: unknown }) => { - const p = payload as DispatchReminderEmailPayload + const { ledgerId, workspaceId, task, recipientClientId, reminderType } = payload as DispatchReminderEmailPayload logger.error('dispatch-reminder-email: retries exhausted, compensating ledger', { - ledgerId: p.ledgerId, - workspaceId: p.workspaceId, - taskId: p.task.id, - recipientClientId: p.recipientClientId, - reminderType: p.reminderType, + ledgerId, + workspaceId, + taskId: task.id, + recipientClientId, + reminderType, error: serializeError(error), }) const db = DBClient.getInstance() try { - await db.taskReminderSent.delete({ where: { id: p.ledgerId } }) + await db.taskReminderSent.delete({ where: { id: ledgerId } }) } catch (deleteErr) { logger.error('dispatch-reminder-email: ledger compensation DELETE failed, reminder will not retry', { - ledgerId: p.ledgerId, + ledgerId, error: serializeError(deleteErr), }) } diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index 5f0521792..db27a88b9 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -2,8 +2,8 @@ import 'server-only' import { copilotAPIKey } from '@/config' import DBClient from '@/lib/db' -import { ClientResponse } from '@/types/common' import { CopilotAPI } from '@/utils/CopilotAPI' +import { serializeError } from '@/utils/serializeError' import { AssigneeType, TaskReminderType } from '@prisma/client' import { logger, schedules } from '@trigger.dev/sdk/v3' import Bottleneck from 'bottleneck' @@ -191,15 +191,11 @@ const processWorkspace = async ( return { enqueued, skipped: plan.length - inserted.length } } +// IU rows are filtered upstream so only client/company assignees reach here. const resolveRecipients = async (copilot: CopilotAPI, task: EligibilityRow): Promise => { - if (task.assigneeType === AssigneeType.client) { + if (task.assigneeType !== AssigneeType.company) { return [{ clientId: task.assigneeId, companyId: task.companyId }] } - if (task.assigneeType === AssigneeType.company) { - const members: ClientResponse[] = await copilot.getCompanyClients(task.assigneeId) - return members.map((m) => ({ clientId: m.id, companyId: task.assigneeId })) - } - return [] + const members = await copilot.getCompanyClients(task.assigneeId) + return members.map((m) => ({ clientId: m.id, companyId: task.assigneeId })) } - -const serializeError = (err: unknown) => (err instanceof Error ? { message: err.message, stack: err.stack } : err) diff --git a/src/utils/serializeError.ts b/src/utils/serializeError.ts new file mode 100644 index 000000000..1bcfff086 --- /dev/null +++ b/src/utils/serializeError.ts @@ -0,0 +1,2 @@ +// JS can throw anything; this turns the unknown into something safe to log. +export const serializeError = (err: unknown) => (err instanceof Error ? { message: err.message, stack: err.stack } : err) From 0f1adb005216e2100e531f29ea3314f063bdae0d Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 26 May 2026 20:40:21 +0545 Subject: [PATCH 15/15] refactor(OUT-3730): extract dispatchChunk helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chunked batchTrigger loop had a nested try/catch and manual index arithmetic inline. Extract the dispatch-or-compensate logic into a small closure so the outer loop reads as just "chunk and accumulate": for (let i = 0; i < triggers.length; i += BATCH_TRIGGER_CHUNK_SIZE) { enqueued += await dispatchChunk(triggers.slice(i, i + BATCH_TRIGGER_CHUNK_SIZE)) } Per-chunk compensation semantics are unchanged. `chunkOffset` dropped from the failure log — workspaceId + chunkSize + log ordering are enough for post-mortem, and the index didn't add diagnostic value worth the noise. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/jobs/notifications/send-task-reminders.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/jobs/notifications/send-task-reminders.ts b/src/jobs/notifications/send-task-reminders.ts index db27a88b9..cca3c79e7 100644 --- a/src/jobs/notifications/send-task-reminders.ts +++ b/src/jobs/notifications/send-task-reminders.ts @@ -159,21 +159,18 @@ const processWorkspace = async ( }) } - let enqueued = 0 - for (let i = 0; i < triggers.length; i += BATCH_TRIGGER_CHUNK_SIZE) { - const chunk = triggers.slice(i, i + BATCH_TRIGGER_CHUNK_SIZE) + // Returns the number actually enqueued. On failure, drops the chunk's ledger rows + // so the next cron run can retry — without this, the unique constraint blocks any + // future insert but no dispatcher exists to consume them. + const dispatchChunk = async (chunk: { payload: DispatchReminderEmailPayload }[]): Promise => { try { await dispatchReminderEmail.batchTrigger(chunk) - enqueued += chunk.length + return chunk.length } catch (err) { - // Compensate: drop the chunk's ledger rows so the next cron run retries them. - // Without this, the rows are orphans: the unique constraint blocks future inserts - // but no dispatcher will ever consume them. const ledgerIds = chunk.map((t) => t.payload.ledgerId) logger.error('send-task-reminders: batchTrigger failed, compensating ledger', { workspaceId, chunkSize: chunk.length, - chunkOffset: i, error: serializeError(err), }) try { @@ -185,9 +182,15 @@ const processWorkspace = async ( error: serializeError(deleteErr), }) } + return 0 } } + let enqueued = 0 + for (let i = 0; i < triggers.length; i += BATCH_TRIGGER_CHUNK_SIZE) { + enqueued += await dispatchChunk(triggers.slice(i, i + BATCH_TRIGGER_CHUNK_SIZE)) + } + return { enqueued, skipped: plan.length - inserted.length } }