OUT-3731 | Tests: eligibility SQL unit tests + idempotency integration test

[arpandhakal]

What

Real-Postgres tests for the reminder cron (OUT-3731). Stacked on OUT-3738-observability (PR #1261) — the idempotency test asserts the Sentry capture added there — so review/merge that first.

The ticket asked for "real DB, no mocks", but its cited reference actually mocks the DB and no real-DB test infra existed. So this PR introduces that harness (testcontainers) and the two suites on top of it.

New: integration test harness

• jest.integration.config.ts + test/integration/* + a test:integration script.
• globalSetup boots an ephemeral postgres:16-alpine, runs prisma migrate deploy (the real migration history, so the schema matches prod), and publishes its URL; globalTeardown stops it.
• setup-env forces every worker's DATABASE_URL onto the container (overriding the dev .env), and the test client is hard-guarded to refuse any non-local DB — a misconfig can't truncate a real database.
• Kept separate from yarn test via testPathIgnorePatterns, so the DB-less unit suite still runs in CI. (Note: CI needs a Docker-capable job to run test:integration — not added here.)

Tests

• eligibility.integration.test.ts (8): runs the actual SQL — all six windows hit on their exact day, ±1-day boundary misses, deleted/archived/completed exclusions, company → single company-level row, and the three subtask carve-outs (+ dead-parent variant). Dates anchored to the DB's CURRENT_DATE to avoid UTC-midnight flake.
• reminder-idempotency.integration.test.ts (3): real DB, Copilot/Trigger/Sentry as doubles (batchTrigger fans out to the dispatcher inline so a forced failure exercises the real onFailure). One send → one ledger row; immediate re-run → 0 new (unique constraint); forced Copilot failure → ledger row cleared + tagged Sentry event.

⚠️ Production bug found & fixed

The real DB surfaced that ledger compensation never actually deleted. The global softDelete/softDeleteMany Prisma extension on DBClient rewrites .delete()/.deleteMany() into deletedAt updates for every model, but TaskReminderSents has no deletedAt column — so:

• dispatchReminderEmailOnFailure and the cron's dispatchChunk compensation silently failed,
• the @@unique([taskId, recipientId, reminderType]) constraint would then permanently block any re-send of a terminally-failed reminder.

Both paths now hard-delete via $executeRaw to bypass the extension; the two unit tests that asserted the old .delete/.deleteMany calls were updated.

Testing

• test:integration → 11/11 pass · yarn test (notifications) → 25/25, integration excluded · tsc --noEmit clean · eslint clean.

🤖 Generated with Claude Code

[linear-code]

OUT-3731

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
tasks-app	Ready	Preview, Comment	May 28, 2026 2:25pm

[vercel]

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

[greptile-apps]

Greptile Summary

This PR introduces a real-Postgres integration test harness (testcontainers + jest.integration.config.ts) and two test suites — eligibility.integration.test.ts (8 tests exercising the eligibility SQL against a live DB) and reminder-idempotency.integration.test.ts (3 tests verifying ledger deduplication and failure compensation). It also ships the production cron (send-task-reminders), dispatcher task (dispatch-reminder-email), and the email helper alongside a fix for a production bug where the global softDelete Prisma extension silently converted hard-deletes on TaskReminderSents into no-ops (now routed through $executeRaw).

• New test infrastructure: testcontainers boots an ephemeral postgres:16-alpine, applies the real migration history, and publishes the URL to a temp file read by each Jest worker; a localhost guard prevents any worker from accidentally hitting a real database.
• Production bug fix: dispatchReminderEmailOnFailure and dispatchChunk now use $executeRaw for ledger compensation so the softDelete extension (which rewrites .delete() to set deletedAt) can't silently leave orphaned rows that would permanently block re-sends via the unique constraint.
• Eligibility SQL: single query covering all six reminder windows via CASE/IN; subtask carve-out via LEFT JOIN on alive parents; regex-guarded dueDate::date cast to avoid Postgres type errors on malformed strings.

Confidence Score: 4/5

The production code and test harness are well-constructed; the three issues found are confined to test infrastructure and a defensive SQL pattern, none of which affect the correctness of the reminder send or ledger deduplication.

The core production paths — eligibility SQL, ledger-first insert, chunked batchTrigger, and both hard-delete compensation paths — are correct and thoroughly tested. The findings are limited to the test harness: a missing try/catch in globalSetup that could orphan a Docker container if migrations fail, an overly broad removeAllListeners call in setup-env, and the eligibility SELECT CASE lacking an ELSE clause that would make a future WHERE/CASE drift immediately visible rather than silently propagating a null reminderType.

test/integration/globalSetup.ts (no container cleanup on migration failure) and src/jobs/notifications/eligibility.ts (SELECT CASE without ELSE).

Important Files Changed

Filename	Overview
src/jobs/notifications/eligibility.ts	New file: raw SQL eligibility query covering all 6 reminder windows; subtask carve-out via LEFT JOIN is logically correct; SELECT CASE has no ELSE clause so a WHERE/CASE drift would produce a silent NULL reminderType.
src/jobs/notifications/send-task-reminders.ts	New file: cron orchestrator with per-workspace Bottleneck concurrency, ledger-first insert, chunked batchTrigger, and compensation DELETE via $executeRaw; logic is well-structured and edge cases (empty plan, empty inserted, chunk failure) are handled.
src/jobs/notifications/dispatch-reminder-email.ts	New file: Trigger.dev task with retry policy and onFailure compensation; hard-delete via $executeRaw correctly bypasses the global softDelete extension; Sentry capture deferred to onFailure to avoid transient-retry noise.
test/integration/globalSetup.ts	Boots testcontainer and runs real migrations; no error handling means the container is orphaned if migrate deploy fails before PG is assigned, leaving teardown unable to stop it.
test/integration/db.ts	Test harness: plain PrismaClient (no extensions) with a local-URL guard, seed helpers, and date utilities anchored to the DB clock; well-guarded against non-local connection strings.
test/integration/setup-env.ts	Worker setup: overrides DATABASE_URL before DBClient reads it; process.removeAllListeners('beforeExit') strips all listeners, not just DBClient's — safe for now but brittle if other modules register beforeExit handlers.
src/jobs/notifications/eligibility.integration.test.ts	8 real-DB tests covering all 6 windows, ±1-day boundary misses, exclusion flags, company assignment, and 4 subtask carve-out variants; dates anchored to DB CURRENT_DATE to avoid UTC-midnight flake.
src/jobs/notifications/reminder-idempotency.integration.test.ts	3 real-DB tests: send-once, idempotent re-run, and terminal failure with Sentry tag assertion; Trigger.dev double correctly wires onFailure so the real compensation path is exercised.
src/jobs/notifications/send-task-reminders.test.ts	10 unit tests covering empty eligibility, IU filtering, client/company fan-out, chunking (1200 → 3 batches), chunk-failure compensation, recipient resolution failure isolation, and workspace-level isolation.
prisma/migrations/20260515091539_add_task_reminder_sents_table/migration.sql	Creates TaskReminderSents table with UUID PK, correct unique constraint on (taskId, recipientId, reminderType), and CASCADE FK to Tasks; no deletedAt column by design (hard-delete semantics).
src/jobs/sentry.ts	New file: lazily initializes Sentry for Trigger.dev's standalone Node process; reuses @sentry/nextjs instead of a separate SDK; no-ops gracefully when SENTRY_DSN is absent.
src/app/api/notification/notification.helpers.ts	Adds getReminderEmailDetails covering all 6 TaskReminderType values; subjects correctly omit portal prefix per documented Copilot convention; body text correctly uses quoted task title.

Sequence Diagram

sequenceDiagram
    participant Cron as send-task-reminders (cron)
    participant DB as Postgres (TaskReminderSents)
    participant Trigger as Trigger.dev
    participant Dispatcher as dispatch-reminder-email
    participant Copilot as Copilot API
    participant Sentry as Sentry

    Cron->>DB: getEligibleReminders() — raw SQL, 6 windows
    DB-->>Cron: EligibilityRow[]

    Cron->>Copilot: getWorkspace() per workspace
    Cron->>Copilot: getCompanyClients() for company tasks (fan-out)

    Cron->>DB: "createManyAndReturn (skipDuplicates=true) — ledger insert"
    DB-->>Cron: inserted rows (deduped by unique constraint)

    Cron->>Trigger: batchTrigger(chunk ≤500) — enqueue dispatchers

    alt batchTrigger succeeds
        Trigger->>Dispatcher: run(payload)
        Dispatcher->>Copilot: createNotification()
        Copilot-->>Dispatcher: "{ id }"
        Dispatcher-->>Trigger: "{ ledgerId, notificationId, sent: true }"
    else batchTrigger fails
        Cron->>DB: "$executeRaw DELETE FROM TaskReminderSents WHERE id = ANY(ledgerIds)"
    end

    alt Dispatcher retries exhausted (onFailure)
        Dispatcher->>Sentry: captureException(error, tags)
        Dispatcher->>DB: "$executeRaw DELETE FROM TaskReminderSents WHERE id = ledgerId"
    end

Loading

Comments Outside Diff (1)

1. src/jobs/notifications/eligibility.ts, line 32-43 (link)

   SELECT CASE has no ELSE clause — NULL reminderType would propagate silently

   The WHERE filter and the SELECT CASE cover the same six windows so they should always stay in sync. But if they ever drift (e.g. a new window added to WHERE without updating the CASE), the CASE returns NULL, and NULL::"TaskReminderType" is valid PostgreSQL and flows through Prisma as null. The TypeScript type says reminderType: TaskReminderType (non-nullable), so the violation is invisible at the call site. Downstream, reminderType: null would be passed into createManyAndReturn where the DB's NOT NULL constraint would reject it — failing the entire workspace rather than just the orphaned row. Adding an explicit ELSE NULL (or an ELSE that casts to an impossible value to force a visible error) signals intent and helps future maintainers notice a sync gap.

_{Reviews (1): Last reviewed commit: "Merge branch 'feature/email-reminders' i..." | Re-trigger Greptile}

[greptile-apps]

Container orphaned on migration failure

If execSync('npx prisma migrate deploy', ...) throws (e.g. a failed migration, missing extension), the container has already been started but __PG__ is never assigned. globalTeardown then reads undefined from globalThis.__PG__ and calls container?.stop() — a no-op — so the container keeps running. testcontainers' Ryuk daemon handles this in most CI setups, but wrapping the exec + write in a try/catch that calls container.stop() before rethrowing is a cheaper guarantee than relying on the cleanup agent being present.

[greptile-apps]

process.removeAllListeners('beforeExit') removes every registered listener on that event, not only the one DBClient installs. Any future module that legitimately registers a beforeExit handler (e.g. for graceful shutdown or flushing diagnostics) would be silently stripped. Prefer removing only the specific listener if it is exported, or at minimum name the operation so it is easy to find and revisit.

Suggested change

	process.removeAllListeners('beforeExit')
	// Removes only the DBClient-installed beforeExit handler. If DBClient ever exports
	// the handler function, prefer `process.removeListener('beforeExit', handler)` here.
	process.removeAllListeners('beforeExit')

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!