Skip to content

feat(emails): send 'Your data is ready!' email after ingest completes#119

Open
tifa2UP wants to merge 2 commits intomainfrom
feat/data-ready-email
Open

feat(emails): send 'Your data is ready!' email after ingest completes#119
tifa2UP wants to merge 2 commits intomainfrom
feat/data-ready-email

Conversation

@tifa2UP
Copy link
Member

@tifa2UP tifa2UP commented Mar 6, 2026

Summary

  • New email template (data-ready-email.tsx) notifying users when their uploaded data finishes processing, with tips and an "Open Playground" CTA
  • Email is sent only when all ingest jobs in the namespace are done — no email if other jobs are still processing
  • Best-effort delivery — email sending is wrapped in try/catch so it never breaks the ingest job

Changes

  • packages/emails/src/templates/data-ready-email.tsx — New React Email template using existing DefaultLayout, Button, and Footer components. Includes namespace name, getting-started tips, and a link to the playground.
  • packages/emails/src/index.ts — Export DataReadyEmail
  • packages/jobs/src/tasks/ingest.ts:
    • Added slug and name to the namespace Prisma select
    • After ingest job completion + webhook emission, checks if no other active jobs remain in the namespace
    • If all clear, sends the notification email to the org owner via getOrganizationOwner() with variant: "notifications"

Greptile Summary

This PR adds a "Your data is ready!" transactional email that fires after an ingest job completes, using an existing DefaultLayout/Button/Footer component system and a new DataReadyEmail React Email template. The overall approach is sound and follows established patterns in the codebase, but there is a notable race condition in the delivery logic.

  • Race condition in ingest.ts: The activeJobCount check is not atomic with the job-status transaction. If two jobs in the same namespace complete concurrently, both can observe activeJobCount === 0 after updating their own status and each send the notification email, resulting in duplicate messages to the owner.
  • Unused organization.name field in DataReadyEmail: the prop type declares name but it is never rendered in the template; only slug is actually used.
  • Import ordering in ingest.ts: the two new @agentset/* imports are appended after local ./ imports, breaking the convention used throughout the rest of the file.

Confidence Score: 3/5

  • Safe to merge for functionality, but the race condition should be addressed to prevent duplicate emails being sent when namespace jobs complete concurrently.
  • The core feature is well-implemented and follows existing patterns; email sending is correctly wrapped in a best-effort try/catch. However, the non-atomic activeJobCount check introduces a real risk of duplicate emails when namespace jobs complete concurrently, which is a user-facing correctness issue that warrants a fix. Additionally, minor code organization issues (import ordering and unused type field) should be cleaned up.
  • packages/jobs/src/tasks/ingest.ts — the email-trigger logic needs an atomic or idempotent mechanism to prevent duplicate sends, and import ordering should be corrected.

Last reviewed commit: 8d10761

Greptile also left 3 inline comments on this PR.

Context used:

  • Rule from dashboard - Comprehensive rules to help you write advanced Trigger.dev tasks (source)
  • Rule from dashboard - Guidelines for writing clean, maintainable, and human-readable code. Apply these rules when writing ... (source)

- Add DataReadyEmail template (packages/emails/src/templates/data-ready-email.tsx)
  with namespace name, tips, and 'Open Playground' CTA button
- Export DataReadyEmail from packages/emails/src/index.ts
- Extend namespace Prisma select in ingest.ts to include slug and name
- Send notification email after ingest job completes when no other
  active jobs remain in the namespace (best-effort, wrapped in try/catch)
- Uses getOrganizationOwner() to target org owner via notifications variant
@vercel
Copy link

vercel bot commented Mar 6, 2026

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

Project Deployment Actions Updated (UTC)
app-agentset-ai Ready Ready Preview, Comment Mar 6, 2026 11:14am

Request Review

Comment on lines +609 to +648
const activeJobCount = await db.ingestJob.count({
where: {
namespaceId: ingestionJob.namespace.id,
id: { not: ingestionJob.id },
status: {
in: [
IngestJobStatus.BACKLOG,
IngestJobStatus.QUEUED,
IngestJobStatus.QUEUED_FOR_RESYNC,
IngestJobStatus.PRE_PROCESSING,
IngestJobStatus.PROCESSING,
],
},
},
});

if (activeJobCount === 0) {
const owner = await getOrganizationOwner({
db,
organizationId: ingestionJob.namespace.organization.id,
});

if (owner) {
await sendEmail({
email: owner.email,
subject: "Your data is ready!",
react: DataReadyEmail({
email: owner.email,
namespace: {
name: ingestionJob.namespace.name,
slug: ingestionJob.namespace.slug,
},
organization: owner.organization,
}),
variant: "notifications",
});
}
}
} catch {
// Email notification is best-effort — don't fail the ingest job
Copy link
Contributor

Choose a reason for hiding this comment

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

Race condition: duplicate emails when concurrent jobs finish

The activeJobCount check is not atomic with the status update in the preceding transaction. If two ingest jobs belonging to the same namespace complete at nearly the same time, both will have already set their own status to COMPLETED before either reaches this query. As a result, both will observe activeJobCount === 0 and each will independently send the "Your data is ready!" email — the owner receives two (or more) identical notifications.

A common pattern to prevent this is to use a database-level atomic operation (e.g. a conditional update that only succeeds for one winner) or a distributed lock keyed on namespaceId. Alternatively, a simpler heuristic is to check that the current job was the most-recently completed one (e.g. sort by completedAt DESC and verify this job's ID is first), though that is still not perfectly race-free without a transaction.

// Example: only the "last completer" sends the email by checking completedAt ordering
const latestCompleted = await db.ingestJob.findFirst({
  where: {
    namespaceId: ingestionJob.namespace.id,
    status: IngestJobStatus.COMPLETED,
  },
  orderBy: { completedAt: "desc" },
  select: { id: true },
});

const noActiveJobs = activeJobCount === 0;
const isLatestCompleted = latestCompleted?.id === ingestionJob.id;

if (noActiveJobs && isLatestCompleted) {
  // send email
}

This eliminates the most common case of duplicates, though a proper distributed lock or a unique idempotency key per namespace/send is the most robust solution.

Context Used: Rule from dashboard - Comprehensive rules to help you write advanced Trigger.dev tasks (source)

Comment on lines +30 to +31
import { DataReadyEmail, sendEmail } from "@agentset/emails";
import { getOrganizationOwner } from "@agentset/webhooks/server";
Copy link
Contributor

Choose a reason for hiding this comment

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

Import ordering breaks established convention

The two new @agentset/* package imports are placed after the local ./ imports, which is inconsistent with the rest of the file where all @agentset/* workspace package imports are grouped together before the local ones (lines 3–17).

Suggested change
import { DataReadyEmail, sendEmail } from "@agentset/emails";
import { getOrganizationOwner } from "@agentset/webhooks/server";
import { DataReadyEmail, sendEmail } from "@agentset/emails";
import { getOrganizationOwner } from "@agentset/webhooks/server";
import { getDb } from "../db";

Context Used: Rule from dashboard - Guidelines for writing clean, maintainable, and human-readable code. Apply these rules when writing ... (source)

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!

Comment on lines +22 to +25
};
organization: {
name: string;
slug: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

organization.name is accepted but never used

The organization prop type declares both name and slug, but name is never rendered anywhere in the template. Only organization.slug is used to construct the playground URL. Carrying an unused field through the type signature can mislead future maintainers and requires callers to supply data unnecessarily.

If there are no plans to display the organization name in this email, consider narrowing the type:

Suggested change
};
organization: {
name: string;
slug: string;
organization: {
slug: string;
};

Context Used: Rule from dashboard - Guidelines for writing clean, maintainable, and human-readable code. Apply these rules when writing ... (source)

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.

1 participant