Skip to content

OUT-3730 | Trigger.dev job: send-task-reminders scheduled task - #1258

Merged
arpandhakal merged 17 commits into
feature/email-remindersfrom
OUT-3730
May 27, 2026
Merged

OUT-3730 | Trigger.dev job: send-task-reminders scheduled task#1258
arpandhakal merged 17 commits into
feature/email-remindersfrom
OUT-3730

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

Changes

  • Add src/jobs/notifications/send-task-reminders.ts — Trigger.dev schedules.task running at 0 0 * * * (00:00 UTC) that walks eligibility, fans out company tasks, writes the dedupe ledger, and dispatches reminder emails.
  • Add src/jobs/notifications/send-task-reminders.test.ts — 7 tests covering empty input, IU filtering, happy path, re-run idempotency, company fan-out, Copilot-failure compensation, per-workspace isolation.
  • Register sendTaskReminders in src/jobs/notifications/index.ts so Trigger.dev's dirs auto-discovery picks it up.

Implements OUT-3730.

Per-workspace flow

  1. Group eligibility rows by workspaceId; filter out assigneeType='internalUser' (kept in the cron rather than the SQL so OUT-3736's contract stays untouched).
  2. prisma.task.findMany to fetch { id, title, createdById } for the workspace's task ids.
  3. Mint a per-workspace CopilotAPI from any task's createdById + workspaceId via encodePayload — same shape as cmd/backfill-missed-emails. Resolve the Workspace once per workspace.
  4. For each eligibility row, resolve recipients: client rows stay 1:1; company rows fan out via copilot.getCompanyClients(assigneeId). Members no longer in the company drop out naturally.
  5. Single batched ledger INSERT (per ticket spec, verbatim):
    INSERT INTO "TaskReminderSents" (...)
    VALUES ...
    ON CONFLICT ("taskId", "recipientId", "reminderType") DO NOTHING
    RETURNING ...
    Only rows returned by RETURNING are net-new claims to send.
  6. For each net-new row: call sendReminderEmail. On Copilot failure → DELETE FROM "TaskReminderSents" WHERE id = ? so the next cron run retries. A failing DELETE is logged distinctly (permanent miss).

Idempotency boundary

Ledger insert happens before the send. The unique constraint guarantees that a retry, an in-flight duplicate, or a manual re-trigger can never double-send.

Concurrency

  • Bottleneck({ maxConcurrent: 5 }) for workspaces, matching WORKSPACE_CONCURRENCY in auto-archive-completed-tasks.ts.
  • Promise.allSettled for the workspace loop so a failing workspace doesn't abort the sweep.
  • Sequential within a workspace — per-workspace volume is low (tens, maybe hundreds), and per-workspace Copilot rate limits favor sequential.

Testing Criteria

  • yarn test src/jobs/notifications/send-task-reminders.test.ts — 7/7 passing.
  • yarn test src/jobs/notifications/send-reminder-email.test.ts — 5/5 still passing (unchanged).
  • npx tsc --noEmit — 0 errors project-wide.
  • npx prettier --check on the new + modified files — clean.
  • Deferred to OUT-3732 — staging dry-run, validate against a real database, monitor first three 00:00 UTC runs.
  • Deferred to OUT-3731 — fixture-based eligibility SQL tests (mocks here cover the cron's control flow, not the underlying SQL).

Notes

  • Base branch is OUT-3737, not feature/email-reminders — stacked PR so reviewers see only the cron diff here.
  • Depends on OUT-3737 (sendReminderEmail), OUT-3736 (getEligibleReminders), OUT-3735 (getReminderEmailDetails), OUT-3734 (TaskReminderSents + enum).
  • senderId = task.createdById (the IU who created the task) is the chosen identity; flag in review if reminders should use a different sender.
  • IU eligibility rows are filtered in the cron rather than the SQL. Reason: keeps OUT-3736 untouched. We pay one extra row per IU-assigned task per day in the eligibility result, which is negligible.
  • The cron does not register itself with index.ts for the IU-only delete-task-notifications pattern — it's a schedules.task, picked up automatically by dirs: ['./src/jobs'] in trigger.config.ts. The index.ts export is for symmetry with the other notification triggers.

Impact & Surface Area of Change

  • Net-new cron, off by default until the Trigger.dev schedule registers in production. Zero impact on existing notification flows.
  • New DB writes go only to TaskReminderSents (OUT-3734). No reads/writes to ClientNotification, InternalUserNotification, ActivityLog, or Task.
  • New Copilot calls: getWorkspace + getCompanyClients (once per workspace), createNotification (one per send). All wrapped in the existing withRetry.
  • Watch the first three production runs (OUT-3732 covers monitoring): per-workspace [X/Y] workspace ws_…: sent N, failed M, skipped K log + final sweep summary.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
@linear-code

linear-code Bot commented May 25, 2026

Copy link
Copy Markdown

OUT-3730

@vercel

vercel Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tasks-app Ready Ready Preview, Comment May 26, 2026 3:00pm

Request Review

@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown

Greptile Summary

Adds a new Trigger.dev scheduled cron job (send-task-reminders) that runs daily at 00:00 UTC, walks eligible (task, assignee, reminderType) rows, fans company-assigned tasks out to individual members via Copilot, writes a deduplication ledger using an INSERT … ON CONFLICT DO NOTHING … RETURNING pattern, and dispatches reminder emails — compensating the ledger on send failure so the next run retries.

  • Core idempotency is sound: the unique constraint on (taskId, recipientId, reminderType) is the dedupe primitive; RETURNING correctly identifies net-new rows; ledger compensation on Copilot error is implemented and tested.
  • Per-workspace concurrency uses Bottleneck({ maxConcurrent: 5 }) + Promise.allSettled for workspace-level isolation, matching the existing pattern.
  • Fan-out error isolation has a gap: a getCompanyClients failure inside the plan-building loop propagates out of processWorkspace, causing the workspace-level catch to fire and silently abandoning any client-assigned tasks that appeared after the failing row in that workspace's batch.

Confidence Score: 3/5

Safe to merge only after addressing the getCompanyClients error-propagation issue; a transient Copilot failure for one company row will silently drop reminders for unrelated client-assigned tasks in the same workspace.

The idempotency design, ledger compensation, and workspace-level isolation are all solid. The gap is in the plan-building loop inside processWorkspace: a thrown error from getCompanyClients bubbles out of the entire function, causing the workspace-level catch to fire and abandoning any client-assigned rows that come after the failing company row. This is a real present defect on the changed path that the test suite does not exercise.

src/jobs/notifications/send-task-reminders.ts — specifically the plan-building for loop around resolveRecipients.

Important Files Changed

Filename Overview
src/jobs/notifications/send-task-reminders.ts New scheduled Trigger.dev cron job. Core idempotency logic (ledger INSERT … ON CONFLICT) and workspace fan-out are correct, but the plan-building loop leaves company-row getCompanyClients errors unhandled per-row, which can abort an entire workspace and silently skip otherwise-healthy client-assigned tasks.
src/jobs/notifications/send-task-reminders.test.ts Good coverage of the main paths (empty input, IU filter, happy path, idempotency, company fan-out, ledger compensation, workspace isolation). The getCompanyClients-throws scenario is not tested, so the per-row propagation bug is not caught here.
src/jobs/notifications/index.ts Trivial export addition; no issues.

Sequence Diagram

sequenceDiagram
    participant Trigger as Trigger.dev (00:00 UTC)
    participant Job as send-task-reminders
    participant DB as Prisma / Postgres
    participant Copilot as CopilotAPI

    Trigger->>Job: run(payload)
    Job->>DB: getEligibleReminders()
    DB-->>Job: allRows (incl. IU rows)
    Job->>Job: filter out internalUser rows
    Job->>Job: group by workspaceId

    loop Each workspace (up to 5 concurrent via Bottleneck)
        Job->>DB: task.findMany
        DB-->>Job: tasks [id, title, createdById]
        Job->>Copilot: new CopilotAPI(encodePayload(...))
        Job->>Copilot: getWorkspace()
        Copilot-->>Job: workspace

        loop Each eligibility row (sequential)
            alt "assigneeType = company"
                Job->>Copilot: getCompanyClients(assigneeId)
                Copilot-->>Job: members[]
                Note over Job: fan out 1 plan entry per member
            else "assigneeType = client"
                Note over Job: 1 plan entry 1:1
            end
        end

        Job->>DB: INSERT INTO TaskReminderSents ON CONFLICT DO NOTHING RETURNING
        DB-->>Job: inserted rows (net-new only)

        loop Each net-new ledger row
            Job->>Copilot: sendReminderEmail
            alt success
                Note over Job: sent += 1
            else Copilot error
                Job->>DB: taskReminderSent.delete
                Note over Job: failed += 1
            end
        end
    end

    Job-->>Trigger: sent, failed, skipped, workspaceCount
Loading

Reviews (1): Last reviewed commit: "feat(OUT-3730): add send-task-reminders ..." | Re-trigger Greptile

Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
…d apiKey

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) <noreply@anthropic.com>
@vercel

vercel Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Deployment failed with the following error:

Deploying Serverless Functions to multiple regions is restricted to the Pro and Enterprise plans.

Learn More: https://vercel.link/multiple-function-regions

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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>

@priosshrsth priosshrsth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arpandhakal I would suggest to have better variable naming in send-task-reminders.ts if possible.

Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
@arpandhakal
arpandhakal changed the base branch from OUT-3737 to feature/email-reminders May 25, 2026 12:38
Copilot's email service prepends `<workspace.brandName> 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) <noreply@anthropic.com>
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 <br><br> survives Copilot's whitespace normalization and renders as
the expected visible gap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both <br><br> and <p>...</p> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…lure

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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Comment thread src/jobs/notifications/dispatch-reminder-email.ts Outdated
Comment thread src/jobs/notifications/send-task-reminders.ts Outdated
Comment on lines +162 to +190
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),
})
}
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arpandhakal Can we simplify this?

* 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@arpandhakal
arpandhakal merged commit 0b03697 into feature/email-reminders May 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants