fix(training): defer completion email and share canonical training IDs

The portal's complete-training route awaited the completion-certificate
email before responding. That API call resolves the member, renders a PDF
and calls the email provider, so every final-video completion made the
employee wait seconds for work whose outcome they never see — and Node's
fetch has no default request timeout, so an unreachable API could stall
the response for minutes even though the completion was already durable.

Defer it with `after` so it runs once the response is sent, and bound the
request with an AbortSignal timeout so a hung API can't pin the
invocation open. The email path performs no DB or storage writes, only an
in-memory PDF render and a send, so nothing the UI reads afterwards
depends on it having finished.

The route also restated the valid-video-ID list and the HIPAA eligibility
rule that apps/api owns, a fifth copy of constants already duplicated
across both apps and two API modules. Move them to @trycompai/company —
the existing shared-definitions package for api/app/portal — so the two
completion paths cannot disagree about which videos exist or which orgs
may complete HIPAA training.

The two apps' training data files link to the canonical IDs by type only.
They are imported by client components and by a Trigger.dev task, and
@trycompai/company requires @trycompai/db at runtime, so a value
re-export would have pulled Prisma into those bundles. The type-level tie
still fails the build on drift, in both directions: an unknown ID in a
rendered list, or a canonical ID that no longer matches.

Also fixes the pre-existing training-hipaa spec, whose @db mock was never
given frameworkInstance when the HIPAA gate was added, and adds coverage
for the gate's rejection path.

Author: tofikwest

apps/api/src/frameworks/frameworks-people-score.helper.ts

@@ -1,8 +1,10 @@
 import { BackgroundCheckStatus, db } from '@db';
+import {
+  GENERAL_TRAINING_VIDEO_IDS as GENERAL_TRAINING_IDS,
+  HIPAA_TRAINING_ID,
+} from '@trycompai/company';
 import { filterComplianceMembers } from '../utils/compliance-filters';
 
-const GENERAL_TRAINING_IDS = ['sat-1', 'sat-2', 'sat-3', 'sat-4', 'sat-5'];
-const HIPAA_TRAINING_ID = 'hipaa-sat-1';
 const COMPLETED_BACKGROUND_CHECK_STATUSES = [
   BackgroundCheckStatus.completed,
   BackgroundCheckStatus.completed_with_flags,

apps/api/src/training/training-hipaa.spec.ts

@@ -4,6 +4,9 @@ import { TrainingService } from './training.service';
 jest.mock('@db', () => ({
   db: {
     member: { findFirst: jest.fn(), findUnique: jest.fn() },
+    // markVideoComplete gates hipaa-sat-1 on the org having a HIPAA framework
+    // instance, so the HIPAA paths must be able to resolve one.
+    frameworkInstance: { findFirst: jest.fn() },
     employeeTrainingVideoCompletion: {
       findMany: jest.fn(),
       findFirst: jest.fn(),
@@ -36,6 +39,7 @@ describe('TrainingService — HIPAA training', () => {
 
   describe('markVideoComplete', () => {
     it('should accept hipaa-sat-1 as a valid training ID', async () => {
+      db.frameworkInstance.findFirst.mockResolvedValue({ id: 'frm_1' });
       db.member.findFirst.mockResolvedValue({
         id: 'mem_1',
         organizationId: 'org_1',
@@ -65,6 +69,19 @@ describe('TrainingService — HIPAA training', () => {
       });
     });
 
+    // Mirrors the portal's complete-training route: the two completion paths
+    // must agree on which orgs may complete HIPAA training, or one can create
+    // records — and certificate artifacts — the other would have rejected.
+    it('should reject hipaa-sat-1 when the org has no HIPAA framework', async () => {
+      db.frameworkInstance.findFirst.mockResolvedValue(null);
+
+      await expect(
+        service.markVideoComplete('mem_1', 'org_1', 'hipaa-sat-1'),
+      ).rejects.toThrow(BadRequestException);
+
+      expect(db.employeeTrainingVideoCompletion.create).not.toHaveBeenCalled();
+    });
+
     it('should reject unknown training IDs', async () => {
       await expect(
         service.markVideoComplete('mem_1', 'org_1', 'unknown-id'),

apps/api/src/training/training.service.ts

@@ -5,13 +5,16 @@ import {
   BadRequestException,
 } from '@nestjs/common';
 import { db } from '@db';
+import {
+  GENERAL_TRAINING_VIDEO_IDS as GENERAL_TRAINING_IDS,
+  HIPAA_TRAINING_ID,
+  HIPAA_TRAINING_UNAVAILABLE_MESSAGE,
+  hipaaFrameworkInstanceWhere,
+  isTrainingVideoId,
+} from '@trycompai/company';
 import { TrainingEmailService } from './training-email.service';
 import { TrainingCertificatePdfService } from './training-certificate-pdf.service';
 
-const GENERAL_TRAINING_IDS = ['sat-1', 'sat-2', 'sat-3', 'sat-4', 'sat-5'];
-const HIPAA_TRAINING_ID = 'hipaa-sat-1';
-const ALL_TRAINING_IDS = [...GENERAL_TRAINING_IDS, HIPAA_TRAINING_ID];
-
 @Injectable()
 export class TrainingService {
   private readonly logger = new Logger(TrainingService.name);
@@ -40,19 +43,17 @@ export class TrainingService {
     organizationId: string,
     videoId: string,
   ) {
-    if (!ALL_TRAINING_IDS.includes(videoId)) {
+    if (!isTrainingVideoId(videoId)) {
       throw new BadRequestException(`Invalid video ID: ${videoId}`);
     }
 
     if (videoId === HIPAA_TRAINING_ID) {
       const hipaaInstance = await db.frameworkInstance.findFirst({
-        where: { organizationId, framework: { name: 'HIPAA' } },
+        where: hipaaFrameworkInstanceWhere(organizationId),
         select: { id: true },
       });
       if (!hipaaInstance) {
-        throw new BadRequestException(
-          'HIPAA training is not available for this organization',
-        );
+        throw new BadRequestException(HIPAA_TRAINING_UNAVAILABLE_MESSAGE);
       }
     }
 
@@ -116,7 +117,7 @@ export class TrainingService {
     const completions = await db.employeeTrainingVideoCompletion.findMany({
       where: {
         memberId,
-        videoId: { in: GENERAL_TRAINING_IDS },
+        videoId: { in: [...GENERAL_TRAINING_IDS] },
         completedAt: { not: null },
       },
     });
@@ -140,7 +141,7 @@ export class TrainingService {
     const completions = await db.employeeTrainingVideoCompletion.findMany({
       where: {
         memberId,
-        videoId: { in: GENERAL_TRAINING_IDS },
+        videoId: { in: [...GENERAL_TRAINING_IDS] },
         completedAt: { not: null },
       },
       orderBy: { completedAt: 'desc' },

apps/app/src/lib/data/hipaa-training-content.ts

@@ -1 +1,8 @@
-export const HIPAA_TRAINING_ID = 'hipaa-sat-1';
+import type { TrainingVideoId } from '@trycompai/company';
+
+// Tied to the canonical ID that both training-completion paths validate
+// against. Deliberately a local literal with a type-only link rather than a
+// re-export: client components import this file, and @trycompai/company pulls
+// in @trycompai/db at runtime. The annotation still fails typecheck if the
+// canonical ID ever changes, so the two cannot drift.
+export const HIPAA_TRAINING_ID: Extract<TrainingVideoId, 'hipaa-sat-1'> = 'hipaa-sat-1';

apps/app/src/lib/data/training-videos.ts

@@ -1,3 +1,5 @@
+import type { GeneralTrainingVideoId } from '@trycompai/company';
+
 /**
  * Represents a training video resource that can be used
  * for user education and compliance training.
@@ -15,6 +17,15 @@ export interface TrainingVideo {
   url: string;
 }
 
+/**
+ * A rendered video whose ID is one the training-completion paths actually
+ * accept. `@trycompai/company` owns the canonical ID list, shared with the
+ * NestJS training service and the portal's complete-training route.
+ */
+type CanonicalTrainingVideo = Omit<TrainingVideo, 'id'> & {
+  id: GeneralTrainingVideoId;
+};
+
 export const trainingVideos: readonly TrainingVideo[] = [
   {
     id: 'sat-1',
@@ -51,4 +62,4 @@ export const trainingVideos: readonly TrainingVideo[] = [
     youtubeId: 'Clvfkm6azDs',
     url: 'https://www.youtube.com/watch?v=Clvfkm6azDs',
   },
-] as const;
+] as const satisfies readonly CanonicalTrainingVideo[];

apps/portal/src/app/api/portal/complete-training/route.test.ts

@@ -16,6 +16,7 @@ const mocks = vi.hoisted(() => ({
   completionFindMany: vi.fn(),
   logger: vi.fn(),
   fetch: vi.fn(),
+  afterTasks: [] as Array<() => unknown>,
   env: {
     SERVICE_TOKEN_PORTAL: undefined as string | undefined,
     NEXT_PUBLIC_API_URL: 'http://api.test',
@@ -45,6 +46,25 @@ vi.mock('@/env.mjs', () => ({ env: mocks.env }));
 vi.mock('@/utils/logger', () => ({ logger: mocks.logger }));
 vi.stubGlobal('fetch', mocks.fetch);
 
+// The route defers the completion email with `after` so it runs once the
+// response has been sent. Capture the deferred task instead of running it, so
+// tests can prove the response does NOT wait on the email and then drive the
+// email path explicitly.
+vi.mock('next/server', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('next/server')>();
+  return {
+    ...actual,
+    after: (task: () => unknown) => {
+      mocks.afterTasks.push(task);
+    },
+  };
+});
+
+/** Runs whatever the route deferred, mimicking Next flushing `after` tasks. */
+async function flushAfterTasks(): Promise<void> {
+  await Promise.all(mocks.afterTasks.splice(0).map((task) => task()));
+}
+
 import { POST } from './route';
 
 function makeRequest(body: unknown): NextRequest {
@@ -67,6 +87,7 @@ function makeRawRequest(raw: string): NextRequest {
 describe('POST /api/portal/complete-training', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    mocks.afterTasks.length = 0;
     mocks.env.SERVICE_TOKEN_PORTAL = undefined;
   });
 
@@ -203,9 +224,7 @@ describe('POST /api/portal/complete-training', () => {
   it('rejects an unknown video ID', async () => {
     mocks.getSession.mockResolvedValue({ user: { id: 'user_1' } });
 
-    const res = await POST(
-      makeRequest({ videoId: 'not-a-real-video', organizationId: 'org_1' }),
-    );
+    const res = await POST(makeRequest({ videoId: 'not-a-real-video', organizationId: 'org_1' }));
 
     expect(res.status).toBe(400);
     expect(mocks.memberFindFirst).not.toHaveBeenCalled();
@@ -227,9 +246,7 @@ describe('POST /api/portal/complete-training', () => {
     // No HIPAA framework instance for this org.
     mocks.frameworkInstanceFindFirst.mockResolvedValue(null);
 
-    const res = await POST(
-      makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }),
-    );
+    const res = await POST(makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }));
 
     expect(res.status).toBe(400);
     expect(mocks.frameworkInstanceFindFirst).toHaveBeenCalledWith({
@@ -257,9 +274,7 @@ describe('POST /api/portal/complete-training', () => {
     };
     mocks.completionUpsert.mockResolvedValue(record);
 
-    const res = await POST(
-      makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }),
-    );
+    const res = await POST(makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }));
 
     expect(res.status).toBe(200);
     expect(mocks.completionUpsert).toHaveBeenCalledWith({
@@ -300,15 +315,24 @@ describe('POST /api/portal/complete-training', () => {
     // dev-only `logger` util would have swallowed it outside development).
     const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
 
-    const res = await POST(
-      makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }),
-    );
+    const res = await POST(makeRequest({ videoId: 'hipaa-sat-1', organizationId: 'org_1' }));
 
     // Completion is persisted regardless — the email is best-effort.
     expect(res.status).toBe(200);
+    // ...and the response did not wait on it: the API call that renders the
+    // certificate and sends the mail only happens once `after` is flushed.
+    expect(mocks.fetch).not.toHaveBeenCalled();
+
+    await flushAfterTasks();
+
     expect(mocks.fetch).toHaveBeenCalledWith(
       'http://api.test/v1/training/send-hipaa-completion-email',
-      expect.objectContaining({ method: 'POST' }),
+      // The request is bounded: Node's fetch otherwise waits ~5 minutes, which
+      // would keep the invocation alive long after the response was sent.
+      expect.objectContaining({
+        method: 'POST',
+        signal: expect.any(AbortSignal),
+      }),
     );
     // The HTTP failure must surface (in prod too), not be swallowed as success.
     expect(errorSpy).toHaveBeenCalledWith(

apps/portal/src/app/api/portal/complete-training/route.ts

@@ -1,20 +1,25 @@
 import { auth } from '@/app/lib/auth';
 import { env } from '@/env.mjs';
-import { HIPAA_TRAINING_ID } from '@/lib/data/hipaa-training-content';
-import { trainingVideos } from '@/lib/data/training-videos';
 import { db } from '@db/server';
-import { type NextRequest, NextResponse } from 'next/server';
+import {
+  GENERAL_TRAINING_VIDEO_IDS,
+  HIPAA_TRAINING_ID,
+  HIPAA_TRAINING_UNAVAILABLE_MESSAGE,
+  hipaaFrameworkInstanceWhere,
+  isTrainingVideoId,
+} from '@trycompai/company';
+import { after, type NextRequest, NextResponse } from 'next/server';
 import { z } from 'zod';
 
 export const runtime = 'nodejs';
 export const dynamic = 'force-dynamic';
 
-// Canonical training video IDs, derived from the same source the UI renders.
-const GENERAL_TRAINING_IDS = trainingVideos.map((v) => v.id);
-const VALID_VIDEO_IDS = new Set<string>([
-  ...GENERAL_TRAINING_IDS,
-  HIPAA_TRAINING_ID,
-]);
+// The completion email hits the NestJS API, which resolves the member, renders
+// a PDF certificate and calls the email provider — seconds of work. It runs
+// after the response (see `after` below), but still needs a bound: Node's fetch
+// waits ~5 minutes by default, which would pin the serverless invocation open
+// long after the employee has moved on.
+const COMPLETION_EMAIL_TIMEOUT_MS = 30_000;
 
 const schema = z.object({
   videoId: z.string().min(1),
@@ -58,7 +63,7 @@ export async function POST(req: NextRequest) {
 
   const { videoId, organizationId } = parsed.data;
 
-  if (!VALID_VIDEO_IDS.has(videoId)) {
+  if (!isTrainingVideoId(videoId)) {
     return NextResponse.json({ error: 'Invalid video ID' }, { status: 400 });
   }
 
@@ -76,20 +81,17 @@ export async function POST(req: NextRequest) {
   }
 
   // HIPAA training is only available to orgs that have the HIPAA framework
-  // enabled. Mirror the NestJS training service (markVideoComplete) so this
-  // route can't create HIPAA completion records — and trigger HIPAA
-  // certificate artifacts — for orgs the service would reject, which would
-  // desync the two completion paths.
+  // enabled. The eligibility rule is shared with the NestJS training service
+  // (markVideoComplete) so this route can't create HIPAA completion records —
+  // and trigger HIPAA certificate artifacts — for orgs the service would
+  // reject, which would desync the two completion paths.
   if (videoId === HIPAA_TRAINING_ID) {
     const hipaaInstance = await db.frameworkInstance.findFirst({
-      where: { organizationId, framework: { name: 'HIPAA' } },
+      where: hipaaFrameworkInstanceWhere(organizationId),
       select: { id: true },
     });
     if (!hipaaInstance) {
-      return NextResponse.json(
-        { error: 'HIPAA training is not available for this organization' },
-        { status: 400 },
-      );
+      return NextResponse.json({ error: HIPAA_TRAINING_UNAVAILABLE_MESSAGE }, { status: 400 });
     }
   }
 
@@ -116,11 +118,19 @@ export async function POST(req: NextRequest) {
   // Best-effort: trigger the completion certificate email once the relevant
   // training is fully done. Reuses the NestJS email pipeline via the portal
   // service token, so we don't duplicate the certificate/email logic here.
-  await sendCompletionEmailIfComplete({
-    videoId,
-    memberId: member.id,
-    organizationId,
-  });
+  //
+  // Deferred with `after` so it runs once the response has been sent. The
+  // completion is already durable at this point, and the API call behind it
+  // renders a PDF certificate and talks to the email provider — awaiting that
+  // made the employee's click wait seconds on the happy path, and time out on
+  // the unhappy one, for work whose outcome they never see.
+  after(() =>
+    sendCompletionEmailIfComplete({
+      videoId,
+      memberId: member.id,
+      organizationId,
+    }),
+  );
 
   return NextResponse.json({ success: true, data: record });
 }
@@ -154,13 +164,13 @@ async function sendCompletionEmailIfComplete({
     const completed = await db.employeeTrainingVideoCompletion.findMany({
       where: {
         memberId,
-        videoId: { in: GENERAL_TRAINING_IDS },
+        videoId: { in: [...GENERAL_TRAINING_VIDEO_IDS] },
         completedAt: { not: null },
       },
       select: { id: true },
     });
 
-    if (completed.length === GENERAL_TRAINING_IDS.length) {
+    if (completed.length === GENERAL_TRAINING_VIDEO_IDS.length) {
       await triggerCompletionEmail({
         url: `${apiUrl}/v1/training/send-completion-email`,
         serviceToken,
@@ -199,6 +209,7 @@ async function triggerCompletionEmail({
       'x-organization-id': organizationId,
     },
     body: JSON.stringify({ memberId }),
+    signal: AbortSignal.timeout(COMPLETION_EMAIL_TIMEOUT_MS),
   });
 
   // fetch only rejects on network errors, never on a 4xx/5xx response. Without

apps/portal/src/lib/data/hipaa-training-content.ts

@@ -1,4 +1,11 @@
-export const HIPAA_TRAINING_ID = 'hipaa-sat-1';
+import type { TrainingVideoId } from '@trycompai/company';
+
+// Tied to the canonical ID that both training-completion paths validate
+// against. Deliberately a local literal with a type-only link rather than a
+// re-export: client components import this file, and @trycompai/company pulls
+// in @trycompai/db at runtime. The annotation still fails typecheck if the
+// canonical ID ever changes, so the two cannot drift.
+export const HIPAA_TRAINING_ID: Extract<TrainingVideoId, 'hipaa-sat-1'> = 'hipaa-sat-1';
 
 export interface HipaaTrainingSection {
   title: string;
@@ -59,7 +66,7 @@ Prompt reporting matters even when you are unsure whether PHI was actually expos
 ] as const;
 
 export const hipaaAcknowledgements: readonly string[] = [
-  'I completed the organization\'s general security awareness training and this HIPAA Security Awareness Training Add-On.',
+  "I completed the organization's general security awareness training and this HIPAA Security Awareness Training Add-On.",
   'I understand that PHI includes information in electronic, paper, image, audio, and verbal form.',
   'I will access, use, disclose, transmit, and store PHI only as authorized for my job responsibilities and in accordance with organization policy.',
   'I will use approved safeguards, including strong authentication, secure handling practices, and prompt reporting of suspicious activity or incidents.',