feat: Add recruiter notification engine#240
Conversation
- Implemented a durable outbox pattern for recruiter email notifications (new applicant, candidate reply, AI scoring). - Added `notificationOutbox` and `notificationPreference` schema tables. - Added scheduled workers for instant dispatch and daily digests. - Added instance-wide `NOTIFICATIONS_ENABLED` server kill-switch. - Added `notifications` feature flag for UI visibility. - Included email suppression tracking to maintain sender reputation.
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds recruiter notification preferences, durable outbox delivery, digest scheduling, webhook status processing, and notification settings UI. It also adds application deletion and excludes quarantined candidates from application, analytics, interview, job, and chatbot queries. ChangesRecruiter notifications
Application deletion and quarantine visibility
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant EventProducer
participant enqueueNotification
participant notification_outbox
participant runNotificationDispatch
participant sendNotificationEmail
participant ResendWebhook
EventProducer->>enqueueNotification: enqueue typed notification
enqueueNotification->>notification_outbox: insert deduplicated pending row
runNotificationDispatch->>notification_outbox: claim due rows or digest groups
runNotificationDispatch->>sendNotificationEmail: render and send email
sendNotificationEmail-->>notification_outbox: persist provider message id and sent state
ResendWebhook->>processNotificationStatusEvent: process delivery status
processNotificationStatusEvent->>notification_outbox: update status and suppression data
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
JoachimLK
changed the title
|
🚅 Deployed to the reqcore-pr-240 environment in applirank
|
Implement logic to force notification preferences to "off" for the demo account, preventing unwanted emails during demos. Updates include enforcing this in API handlers, notification resolution, and the seeding script.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
server/database/migrations/0046_far_hairball.sql (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSquash these index alterations into the initial migration.
Since both
0045and0046are introduced in the same PR, consider squashing them to maintain a clean migration history and avoid creating and immediately dropping indexes on newly created tables. Consolidating them will also resolve the false-positive warnings from the static analyzer regarding concurrent index operations.You can achieve this by deleting both migration SQL files (along with their specific metadata records in the
meta/folder) and re-running your migration generation command to produce a single, unified file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/database/migrations/0046_far_hairball.sql` around lines 1 - 6, Squash the index changes from migration 0046 into the initial migration by deleting both migration SQL files and their corresponding metadata records under meta/, then regenerate migrations to produce one unified migration containing the final index definitions without create-and-drop operations.tests/unit/notification-delivery-status.test.ts (1)
24-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDigest "leader row" identity isn't actually asserted.
The mock's
where: () => Promise.resolve()discards its argument, so this test can't detect a regression in the digest-bucket identity clause (delivery-status.ts lines 57-64) it's named for — it only checks theset()payload.♻️ Capture and assert the where() clause
update: () => ({ set: (values: Record<string, unknown>) => { setCalls.push(values) - return { where: () => Promise.resolve() } + return { where: (clause: unknown) => { whereCalls.push(clause); return Promise.resolve() } } }, }),Also applies to: 86-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/notification-delivery-status.test.ts` around lines 24 - 38, Update the test mock’s update().set() chain to capture the argument passed to where(), then assert that the digest leader-row identity clause is included alongside the existing set() payload assertions. Apply the same capture and assertion to the additional affected test case, using the symbols and expectations already present in notification-delivery-status.test.ts.server/utils/notifications/templates.ts (1)
133-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the Zod 4 top-level helpers here
z.string().url()is deprecated in favor ofz.url(), andz.number().finite()is now redundant becausez.number()already rejects non-finite values. Swapping them keeps the schema aligned with Zod 4 and avoids deprecated chaining.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/utils/notifications/templates.ts` around lines 133 - 142, Update basePayloadSchema to use the Zod 4 top-level z.url() helper for applicationUrl, and simplify analysis_completed’s compositeScore schema by removing the redundant finite() chain while preserving its number, range, and nullable constraints.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/admin/notification-dispatch.post.ts`:
- Around line 16-39: Update the request-body handling in the notification
dispatch handler around bodySchema and readValidatedBody so an absent body is
treated as an empty object before validation. Preserve the schema defaults,
allowing bodyless cron requests to proceed with digest set to false.
In `@server/api/notification-preferences/index.patch.ts`:
- Around line 30-37: Wrap the per-preference upserts in the
notification-preferences update flow in a database transaction so all entries
succeed or the entire operation rolls back. Update the loop around
db.insert(notificationPreference) to use the repository’s established
transaction API, or replace it with a single batched upsert if supported, while
preserving the existing conflict target and channelMode updates.
---
Nitpick comments:
In `@server/database/migrations/0046_far_hairball.sql`:
- Around line 1-6: Squash the index changes from migration 0046 into the initial
migration by deleting both migration SQL files and their corresponding metadata
records under meta/, then regenerate migrations to produce one unified migration
containing the final index definitions without create-and-drop operations.
In `@server/utils/notifications/templates.ts`:
- Around line 133-142: Update basePayloadSchema to use the Zod 4 top-level
z.url() helper for applicationUrl, and simplify analysis_completed’s
compositeScore schema by removing the redundant finite() chain while preserving
its number, range, and nullable constraints.
In `@tests/unit/notification-delivery-status.test.ts`:
- Around line 24-38: Update the test mock’s update().set() chain to capture the
argument passed to where(), then assert that the digest leader-row identity
clause is included alongside the existing set() payload assertions. Apply the
same capture and assertion to the additional affected test case, using the
symbols and expectations already present in
notification-delivery-status.test.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4217305-2ee2-42ad-9077-d95306c75a79
📒 Files selected for processing (36)
.env.exampleapp/components/SettingsMobileNav.vueapp/components/SettingsSidebar.vueapp/composables/useNotificationPreferences.tsapp/pages/dashboard/settings/notifications.vueee/server/api/webhooks/resend.post.tsnuxt.config.tsserver/api/admin/notification-dispatch.post.tsserver/api/applications/[id]/analyze.post.tsserver/api/notification-preferences/index.get.tsserver/api/notification-preferences/index.patch.tsserver/api/public/jobs/[slug]/apply.post.tsserver/database/migrations/0045_clammy_sleeper.sqlserver/database/migrations/0046_far_hairball.sqlserver/database/migrations/meta/0045_snapshot.jsonserver/database/migrations/meta/0046_snapshot.jsonserver/database/migrations/meta/_journal.jsonserver/database/schema/app.tsserver/tasks/notification-digest.tsserver/tasks/notification-dispatch.tsserver/utils/ai/autoScore.tsserver/utils/email.tsserver/utils/env.tsserver/utils/notifications/delivery-status.tsserver/utils/notifications/dispatch.tsserver/utils/notifications/enqueue.tsserver/utils/notifications/recipients.tsserver/utils/notifications/templates.tsserver/utils/notifications/urls.tsshared/feature-flags.tsshared/notifications.tstests/unit/notification-delivery-status.test.tstests/unit/notification-dispatch.test.tstests/unit/notification-enqueue.test.tstests/unit/notification-recipients.test.tstests/unit/notification-templates.test.ts
| const bodySchema = z.object({ | ||
| digest: z.boolean().optional().default(false), | ||
| batchSize: z.number().int().min(1).max(500).optional(), | ||
| }) | ||
|
|
||
| export default defineEventHandler(async (event) => { | ||
| const cronSecret = getHeader(event, 'x-cron-secret') | ||
| let source: 'cron_endpoint' | 'interactive' = 'interactive' | ||
|
|
||
| if (cronSecret && env.CRON_SECRET) { | ||
| const supplied = Buffer.from(cronSecret) | ||
| const expected = Buffer.from(env.CRON_SECRET) | ||
| const valid = supplied.length === expected.length && timingSafeEqual(supplied, expected) | ||
| if (!valid) { | ||
| throw createError({ statusCode: 403, statusMessage: 'Invalid cron secret' }) | ||
| } | ||
| source = 'cron_endpoint' | ||
| } | ||
| else { | ||
| // Reuse an existing high-trust permission — draining the queue is an admin op. | ||
| await requirePermission(event, { organization: ['update'] }) | ||
| } | ||
|
|
||
| const body = await readValidatedBody(event, bodySchema.parse) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
h3 readValidatedBody empty POST body undefined zod object parse
💡 Result:
When using readValidatedBody in H3 with an empty POST body, the underlying readBody function typically returns undefined if no body is present [1]. Because readValidatedBody passes this undefined value directly to your validation schema or function, passing a Zod object schema directly (e.g., readValidatedBody(event, z.object({...}))) will result in a validation failure [2][1]. Zod's .parse() method will throw a ZodError when called with undefined because an empty input does not satisfy the requirements of a Zod object schema [2][3]. To handle empty POST bodies gracefully: 1. Use .safeParse(): Instead of passing the schema directly to readValidatedBody, you can use a custom validation function that handles the case where the body is undefined [3]. 2. Defaulting: If you expect an empty body to be valid, you may need to ensure your Zod schema allows for it or provide a default object during the validation step [4]. Example using a safe validation pattern: ts import { readValidatedBody, createError } from "h3"; import { z } from "zod"; const mySchema = z.object({ name: z.string(), }); app.post("/", async (event) => { const body = await readValidatedBody(event, (data) => { // If body is undefined/empty, decide how to handle it if (data === undefined) { return {}; // Or return a default object that satisfies schema } return mySchema.safeParse(data); }); // Handle the result of the validation if (!body.success) { throw createError({ statusCode: 400, message: "Invalid body" }); } return body.data; }); Keep in mind that readValidatedBody is designed to throw a 400 Validation Error automatically if the provided validation function returns a failing result or throws [3][1]. If you want to customize the error behavior for empty bodies, using the safe parsing pattern above is the recommended approach [3].
Citations:
- 1: https://github.com/unjs/h3/blob/main/src/utils/body.ts
- 2: https://www.h3.dev/utils/request
- 3: https://h3.dev/examples/validate-data
- 4: feat:
readValidatedBodyandgetValidatedQueryutils h3js/h3#459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target handler and nearby usages of readValidatedBody in the repo.
git ls-files 'server/api/admin/notification-dispatch.post.ts' 'server/api/**/*.ts' | sed -n '1,120p'
printf '\n--- target file ---\n'
cat -n server/api/admin/notification-dispatch.post.ts | sed -n '1,120p'
printf '\n--- readValidatedBody usages ---\n'
rg -n "readValidatedBody\\(" server -g '*.ts'Repository: reqcore-inc/reqcore
Length of output: 13118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find in-repo callers of the notification dispatch endpoint and inspect related admin cron patterns.
printf '%s\n' '--- callers of notification-dispatch ---'
rg -n "notification-dispatch|api/admin/notification-dispatch" .
printf '\n%s\n' '--- retention-cleanup for comparison ---'
cat -n server/api/admin/retention-cleanup.post.ts | sed -n '1,120p'Repository: reqcore-inc/reqcore
Length of output: 2188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the notification-dispatch task and how it is scheduled.
printf '%s\n' '--- server/tasks/notification-dispatch.ts ---'
cat -n server/tasks/notification-dispatch.ts | sed -n '1,160p'
printf '\n%s\n' '--- nuxt.config.ts task config around notification-dispatch ---'
sed -n '280,330p' nuxt.config.ts
printf '\n%s\n' '--- docs/env mentions ---'
sed -n '45,75p' .env.exampleRepository: reqcore-inc/reqcore
Length of output: 4591
Handle missing request bodies before validating. readValidatedBody(event, bodySchema.parse) will reject an empty cron POST because z.object(...).parse(undefined) throws, so bodyless calls to /api/admin/notification-dispatch fail before dispatch starts. Default undefined to {} or wrap the schema so the optional digest path still works.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/admin/notification-dispatch.post.ts` around lines 16 - 39, Update
the request-body handling in the notification dispatch handler around bodySchema
and readValidatedBody so an absent body is treated as an empty object before
validation. Preserve the schema defaults, allowing bodyless cron requests to
proceed with digest set to false.
| for (const pref of preferences) { | ||
| await db.insert(notificationPreference) | ||
| .values({ userId, organizationId: orgId, type: pref.type, channelMode: pref.channelMode }) | ||
| .onConflictDoUpdate({ | ||
| target: [notificationPreference.userId, notificationPreference.organizationId, notificationPreference.type], | ||
| set: { channelMode: pref.channelMode, updatedAt: new Date() }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Wrap the per-preference upserts in a transaction (or batch them).
Each preference is upserted in its own statement with no transaction wrapping. A failure mid-loop leaves the user with a partially-applied set of preferences and no rollback.
♻️ Proposed fix: single batched upsert
- for (const pref of preferences) {
- await db.insert(notificationPreference)
- .values({ userId, organizationId: orgId, type: pref.type, channelMode: pref.channelMode })
- .onConflictDoUpdate({
- target: [notificationPreference.userId, notificationPreference.organizationId, notificationPreference.type],
- set: { channelMode: pref.channelMode, updatedAt: new Date() },
- })
- }
+ await db.insert(notificationPreference)
+ .values(preferences.map(pref => ({
+ userId, organizationId: orgId, type: pref.type, channelMode: pref.channelMode,
+ })))
+ .onConflictDoUpdate({
+ target: [notificationPreference.userId, notificationPreference.organizationId, notificationPreference.type],
+ set: { channelMode: sql`excluded.channel_mode`, updatedAt: new Date() },
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const pref of preferences) { | |
| await db.insert(notificationPreference) | |
| .values({ userId, organizationId: orgId, type: pref.type, channelMode: pref.channelMode }) | |
| .onConflictDoUpdate({ | |
| target: [notificationPreference.userId, notificationPreference.organizationId, notificationPreference.type], | |
| set: { channelMode: pref.channelMode, updatedAt: new Date() }, | |
| }) | |
| } | |
| await db.insert(notificationPreference) | |
| .values(preferences.map(pref => ({ | |
| userId, organizationId: orgId, type: pref.type, channelMode: pref.channelMode, | |
| }))) | |
| .onConflictDoUpdate({ | |
| target: [notificationPreference.userId, notificationPreference.organizationId, notificationPreference.type], | |
| set: { channelMode: sql`excluded.channel_mode`, updatedAt: new Date() }, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/notification-preferences/index.patch.ts` around lines 30 - 37,
Wrap the per-preference upserts in the notification-preferences update flow in a
database transaction so all entries succeed or the entire operation rolls back.
Update the loop around db.insert(notificationPreference) to use the repository’s
established transaction API, or replace it with a single batched upsert if
supported, while preserving the existing conflict target and channelMode
updates.
Prevents the migration from running if the schema changes already exist in the Drizzle ledger from migrations 0045 and 0046.
- Add `interview_response` notification type - Remove `analysis_completed` notification type and associated outbox entries - Implement notification for interview status changes (accepted/declined/tentative) - Add database migration to update `notification_type` enum and cleanup legacy data
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| /** Durable notification outbox dispatch with leases, retries, and digest grouping. */ | ||
| import { and, asc, eq, gt, inArray, lte, sql } from 'drizzle-orm' | ||
| import { emailSuppression, notificationOutbox } from '../../database/schema/app' | ||
| import { isServerFeatureEnabled } from '../featureFlags' |
Implement a DELETE endpoint for applications, allowing users to remove specific applications and their associated data (comments, properties, and attachments). Added UI triggers, backend logic, and validation to ensure scheduled interviews are cancelled before deletion.
Move the S3 object deletion logic after the database transaction to prevent orphaned database records if the transaction fails.
Decouple notification logic from database transactions to ensure that notification delivery failures cannot roll back primary business operations. Updated all notification producers to enqueue events after the DB transaction commits.
1 participant
notificationOutboxandnotificationPreferenceschema tables.NOTIFICATIONS_ENABLEDserver kill-switch.notificationsfeature flag for UI visibility.Summary
Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit