fix(risks): cs-727 deploy-review fixes (acceptance race, approval drift baseline, error surfacing)

apps/api/src/isms/documents/data-source.ts

@@ -1,5 +1,6 @@
 import { createHash } from 'node:crypto';
 import { db } from '@db';
+import type { Prisma } from '@db';
 import { parseStoredAnswers } from '../wizard/wizard-schema';
 import type { IsmsPlatformData } from './types';
 
@@ -16,10 +17,19 @@ const HIGH_IMPACT = ['major', 'severe'];
 export async function collectPlatformData({
   organizationId,
   frameworkId,
+  client,
 }: {
   organizationId: string;
   frameworkId: string;
+  /**
+   * Optional transaction client. The approval flow passes its transaction so
+   * the drift baseline is read at the SAME point in time as the rows frozen
+   * into the published version — otherwise a concurrent edit between the two
+   * reads makes a just-approved document immediately show as stale.
+   */
+  client?: Prisma.TransactionClient;
 }): Promise<IsmsPlatformData> {
+  const dbc = client ?? db;
   const [
     organization,
     frameworkInstances,
@@ -34,15 +44,15 @@ export async function collectPlatformData({
     partiesRows,
     acceptanceRows,
   ] = await Promise.all([
-    db.organization.findUnique({
+    dbc.organization.findUnique({
       where: { id: organizationId },
       select: { name: true },
     }),
-    db.frameworkInstance.findMany({
+    dbc.frameworkInstance.findMany({
       where: { organizationId },
       select: { framework: { select: { name: true } } },
     }),
-    db.vendor.findMany({
+    dbc.vendor.findMany({
       where: { organizationId },
       select: {
         id: true,
@@ -61,14 +71,14 @@ export async function collectPlatformData({
         assignee: { select: { user: { select: { name: true, email: true } } } },
       },
     }),
-    db.member.count({ where: { organizationId, deactivated: false } }),
-    db.member.groupBy({
+    dbc.member.count({ where: { organizationId, deactivated: false } }),
+    dbc.member.groupBy({
       by: ['department'],
       where: { organizationId, deactivated: false },
       _count: { _all: true },
     }),
-    db.device.count({ where: { organizationId } }),
-    db.risk.findMany({
+    dbc.device.count({ where: { organizationId } }),
+    dbc.risk.findMany({
       where: { organizationId },
       select: {
         id: true,
@@ -86,18 +96,18 @@ export async function collectPlatformData({
         assignee: { select: { user: { select: { name: true, email: true } } } },
       },
     }),
-    db.employeeTrainingVideoCompletion.count({
+    dbc.employeeTrainingVideoCompletion.count({
       where: { member: { organizationId } },
     }),
-    db.frameworkEditorFramework.findUnique({
+    dbc.frameworkEditorFramework.findUnique({
       where: { id: frameworkId },
       select: { name: true },
     }),
-    db.ismsProfile.findUnique({
+    dbc.ismsProfile.findUnique({
       where: { organizationId_frameworkId: { organizationId, frameworkId } },
       select: { answers: true },
     }),
-    db.ismsInterestedParty.findMany({
+    dbc.ismsInterestedParty.findMany({
       where: {
         document: {
           organizationId,
@@ -107,7 +117,7 @@ export async function collectPlatformData({
       },
       select: { id: true, name: true, category: true },
     }),
-    db.riskAcceptance.findMany({
+    dbc.riskAcceptance.findMany({
       where: { organizationId },
       select: { id: true, riskId: true, vendorId: true },
     }),

apps/api/src/isms/documents/risk-methodology-defaults.ts

@@ -3,7 +3,7 @@
 // behavior: the five-band risk level scale (very-low .. very-high, computed as
 // likelihood x impact in apps/api/src/risks/risk-level.ts) and the platform's
 // four treatment strategies (mitigate/avoid/transfer/accept) with their ISO
-// 27002 option names (Modify/Avoid/Share/Retain). Keep text ASCII-safe: the
+// 27001 option names (Modify/Avoid/Share/Retain). Keep text ASCII-safe: the
 // PDF renderer's standard fonts cannot render glyphs like ">=" ligatures.
 
 /** Platform likelihood levels, in ascending order (matches the Likelihood enum). */

apps/api/src/isms/documents/risk-methodology.ts

@@ -168,7 +168,7 @@ export function buildRiskMethodologySections(
     {
       heading: 'Risk level matrix',
       intro:
-        'The risk level is derived from the product of likelihood and impact (1-25), normalized to a 1-10 score and mapped to five bands: Very low (1-5), Low (6-10), Medium (11-15), High (16-20), and Very high (21-25). Each risk in the register carries its calculated level for both its inherent and residual states.',
+        'The risk level is derived from the product of likelihood and impact (1-25), normalized to a 1-10 score (the product divided by 2.5, rounded up) and banded by score: Very low (score 1-2), Low (3-4), Medium (5-6), High (7-8), Very high (9-10). Expressed as the raw product, the bands are: Very low 1-5, Low 6-10, Medium 11-15, High 16-20, Very high 21-25. Each risk in the register carries its calculated level for both its inherent and residual states.',
       table: riskMatrixTable(),
     },
     {

apps/api/src/isms/documents/risk-treatment-readiness.ts

@@ -0,0 +1,34 @@
+import { db } from '@db';
+import type { Prisma } from '@db';
+import { riskTreatmentValidationMessages } from './risk-treatment-plan';
+
+/**
+ * Clause-6.1.3 readiness: the RTP renders from the platform Risk Register +
+ * Vendors (org-scoped), not from register rows of its own — so readiness reads
+ * those tables. Archived risks are out of the plan (see
+ * loadRiskTreatmentExtras). Shared by the submit gate and the page payload.
+ */
+export async function loadRiskTreatmentReadinessMessages({
+  organizationId,
+  client,
+}: {
+  organizationId: string;
+  client?: Prisma.TransactionClient;
+}): Promise<string[]> {
+  const dbc = client ?? db;
+  const [risks, vendors] = await Promise.all([
+    dbc.risk.findMany({
+      where: { organizationId, status: { not: 'archived' } },
+      select: { assigneeId: true },
+    }),
+    dbc.vendor.findMany({
+      where: { organizationId },
+      select: { assigneeId: true },
+    }),
+  ]);
+  return riskTreatmentValidationMessages({
+    riskCount: risks.length,
+    risksWithoutOwner: risks.filter((risk) => !risk.assigneeId).length,
+    vendorsWithoutOwner: vendors.filter((vendor) => !vendor.assigneeId).length,
+  });
+}

apps/api/src/isms/isms.service.approve.spec.ts

@@ -16,6 +16,16 @@ jest.mock('@db', () => ({
     member: { findFirst: jest.fn() },
     $transaction: jest.fn(),
   },
+  Prisma: {
+    TransactionIsolationLevel: { RepeatableRead: 'RepeatableRead' },
+    PrismaClientKnownRequestError: class PrismaClientKnownRequestError extends Error {
+      code: string;
+      constructor(message: string, { code }: { code: string }) {
+        super(message);
+        this.code = code;
+      }
+    },
+  },
 }));
 jest.mock('./documents/data-source', () => ({
   collectPlatformData: jest.fn(),
@@ -140,9 +150,12 @@ describe('IsmsService.approve (CS-701 versioning)', () => {
 
     await service.approve(args);
 
+    // The baseline is collected INSIDE the approval transaction (client
+    // threaded) so it reads the same point in time as the frozen rows.
     expect(mockCollect).toHaveBeenCalledWith({
       organizationId: 'org_1',
       frameworkId: 'fw_1',
+      client: expect.anything(),
     });
     // Re-derives in-tx from the same snapshot: persisted rows and the frozen
     // version come from one pass.

apps/api/src/isms/isms.service.ts

@@ -26,7 +26,7 @@ import {
 import { defaultProcedureText } from './documents/management-review-defaults';
 import { defaultRiskMethodologyNarrative } from './documents/risk-methodology';
 import { loadRiskTreatmentExtras } from './documents/risk-treatment-export-data';
-import { riskTreatmentValidationMessages } from './documents/risk-treatment-plan';
+import { loadRiskTreatmentReadinessMessages } from './documents/risk-treatment-readiness';
 import { updateDraftSnapshot } from './utils/draft-snapshot';
 import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload';
 import { lockDocument } from './utils/document-lock';
@@ -450,20 +450,28 @@ export class IsmsService {
     const document = await this.requireDocument({ documentId, organizationId });
     this.assertPendingApprovalBy({ document, member });
 
-    const snapshot = await collectPlatformData({
-      organizationId,
-      frameworkId: document.frameworkId,
-    });
     const now = new Date();
 
     // Freeze the draft into a new immutable published version and promote it to
     // currentVersion. Editing afterwards reverts status to draft but leaves this
     // published version live and exportable (CS-701).
-    const published = await db.$transaction(async (tx) => {
+    const publish = async (tx: Prisma.TransactionClient) => {
       // Serialize concurrent approvals (and register-row creates, which take the
       // same lock) on this document so they can't interleave and double-publish.
       await lockDocument(tx, documentId);
 
+      // Collect the drift baseline INSIDE the transaction so it reads the same
+      // point in time as the rows frozen into the published version below.
+      // The transaction runs REPEATABLE READ (one MVCC snapshot for every
+      // statement), so this baseline and the rows loaded further down can
+      // never observe different data — a concurrent platform edit commits
+      // entirely before or entirely after this approval.
+      const snapshot = await collectPlatformData({
+        organizationId,
+        frameworkId: document.frameworkId,
+        client: tx,
+      });
+
       // Atomically claim the approval: the check-then-act guard above runs before
       // the transaction, so under READ COMMITTED a racing approve/decline could
       // read the same stale `needs_review`. This conditional update only matches
@@ -512,6 +520,24 @@ export class IsmsService {
         data: { currentVersionId: result.versionId },
       });
       return result;
+    };
+
+    // REPEATABLE READ can abort with a write conflict (P2034) when a racing
+    // lifecycle write commits between our snapshot and our claim update —
+    // retry once; the rerun takes a fresh snapshot and either wins cleanly or
+    // fails the conditional claim with the friendly BadRequestException.
+    const runPublish = () =>
+      db.$transaction(publish, {
+        isolationLevel: Prisma.TransactionIsolationLevel.RepeatableRead,
+      });
+    const published = await runPublish().catch((error: unknown) => {
+      if (
+        error instanceof Prisma.PrismaClientKnownRequestError &&
+        error.code === 'P2034'
+      ) {
+        return runPublish();
+      }
+      throw error;
     });
 
     // Render + upload the frozen exports outside the transaction (Policies
@@ -717,46 +743,14 @@ export class IsmsService {
     }
   }
 
-  /**
-   * Clause-6.1.3 readiness: the RTP renders from the platform Risk Register +
-   * Vendors (org-scoped), not from register rows of its own — so readiness
-   * reads those tables. Archived risks are out of the plan (see
-   * loadRiskTreatmentExtras). Shared by the submit gate and the page payload.
-   */
-  private async riskTreatmentReadinessMessages({
-    organizationId,
-    client,
-  }: {
-    organizationId: string;
-    client?: Prisma.TransactionClient;
-  }): Promise<string[]> {
-    const dbc = client ?? db;
-    const [risks, vendors] = await Promise.all([
-      dbc.risk.findMany({
-        where: { organizationId, status: { not: 'archived' } },
-        select: { assigneeId: true },
-      }),
-      dbc.vendor.findMany({
-        where: { organizationId },
-        select: { assigneeId: true },
-      }),
-    ]);
-    return riskTreatmentValidationMessages({
-      riskCount: risks.length,
-      risksWithoutOwner: risks.filter((risk) => !risk.assigneeId).length,
-      vendorsWithoutOwner: vendors.filter((vendor) => !vendor.assigneeId)
-        .length,
-    });
-  }
-
   private async assertRiskTreatmentPlanComplete({
     tx,
     organizationId,
   }: {
     tx: Prisma.TransactionClient;
     organizationId: string;
   }) {
-    const messages = await this.riskTreatmentReadinessMessages({
+    const messages = await loadRiskTreatmentReadinessMessages({
       organizationId,
       client: tx,
     });
@@ -788,7 +782,7 @@ export class IsmsService {
     }
     const [extras, validationMessages] = await Promise.all([
       loadRiskTreatmentExtras({ organizationId }),
-      this.riskTreatmentReadinessMessages({ organizationId }),
+      loadRiskTreatmentReadinessMessages({ organizationId }),
     ]);
     return { ...extras, validationMessages };
   }

apps/api/src/isms/utils/pdf-renderer.ts

@@ -115,7 +115,7 @@ export function renderIsmsPdf({
     const threeCol = table.headers.length === 3;
     const cellFills = table.cellFills;
     // Never let a header word break mid-word ("Acceptanc/e"): floor each
-    // column at its header's longest word. Wide tables (the 6.1.3 plan has 9
+    // column at its header's longest word. Wide tables (the 6.1.3 plan has 10
     // columns) otherwise get squeezed below the header width by autotable's
     // content-proportional sizing.
     pdf.setFont('helvetica', 'bold');

apps/api/src/risks/risk-acceptances.controller.ts

@@ -56,6 +56,7 @@ export class RiskAcceptancesController {
   @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[401])
   @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[403])
   @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[404])
+  @ApiResponse(LIST_RISK_ACCEPTANCES_RESPONSES[500])
   async listRiskAcceptances(
     @Param('id') riskId: string,
     @OrganizationId() organizationId: string,
@@ -85,6 +86,7 @@ export class RiskAcceptancesController {
   @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[401])
   @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[403])
   @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[404])
+  @ApiResponse(RECORD_RISK_ACCEPTANCE_RESPONSES[500])
   async recordRiskAcceptance(
     @Param('id') riskId: string,
     @Body() dto: CreateRiskAcceptanceDto,

apps/api/src/risks/risk-acceptances.service.spec.ts

@@ -5,6 +5,12 @@ const mockDb = {
   vendor: { findFirst: jest.fn() },
   member: { findFirst: jest.fn() },
   riskAcceptance: { findMany: jest.fn(), create: jest.fn() },
+  // The create paths run inside a transaction with a subject row-lock; the
+  // callback receives this same mock as the transaction client.
+  $transaction: jest.fn(
+    (fn: (tx: typeof mockDb) => unknown): unknown => fn(mockDb),
+  ),
+  $queryRaw: jest.fn().mockResolvedValue([]),
 };
 
 jest.mock('@db', () => ({ db: mockDb }));
@@ -67,6 +73,22 @@ describe('RiskAcceptancesService', () => {
       expect(view.levelLabel).toBe('Very low');
     });
 
+    it('row-locks the risk and writes through the same transaction', async () => {
+      mockDb.risk.findFirst.mockResolvedValue(baseRisk);
+      mockDb.member.findFirst.mockResolvedValue(activeMember);
+      mockDb.riskAcceptance.create.mockResolvedValue(storedRow);
+
+      await service.createForRisk('rsk_1', ORG, {});
+
+      // The subject lock serializes concurrent residual edits with the
+      // read-freeze-insert sequence, so a fresh acceptance can never be
+      // recorded against an already-superseded rating.
+      expect(mockDb.$transaction).toHaveBeenCalledTimes(1);
+      expect(mockDb.$queryRaw).toHaveBeenCalledTimes(1);
+      const rawQuery = mockDb.$queryRaw.mock.calls[0][0].join('?');
+      expect(rawQuery).toContain('FOR UPDATE');
+    });
+
     it('defaults the acceptor to the risk owner (assignee)', async () => {
       mockDb.risk.findFirst.mockResolvedValue(baseRisk);
       mockDb.member.findFirst.mockResolvedValue(activeMember);