fix(isms): management review deploy-review fixes

apps/api/src/isms/documents/management-review-defaults.ts

@@ -74,8 +74,9 @@ export const SEED_REVIEW_INPUT_DEFINITIONS: SeedReviewInputDefinition[] = [
     inputKey: 'c_interested_party_changes',
     inputRef: '(c) Interested-party changes',
     whatItCovers: 'Changes in needs and expectations of interested parties.',
+    // Canonical document titles so the agenda reference is followable.
     whereToFind:
-      'Comp AI > ISMS > Documents > Interested Parties Register & Requirements',
+      'Comp AI > ISMS > Documents > Interested Parties Register & Interested Parties Requirements',
   },
   {
     inputKey: 'd1_nonconformity_trends',
@@ -100,7 +101,9 @@ export const SEED_REVIEW_INPUT_DEFINITIONS: SeedReviewInputDefinition[] = [
     inputRef: '(d.4) Objectives fulfilment',
     whatItCovers:
       'Extent to which information security objectives have been met.',
-    whereToFind: 'Comp AI > ISMS > Documents > Objectives Plan',
+    // Canonical document title so the agenda reference is followable.
+    whereToFind:
+      'Comp AI > ISMS > Documents > Information Security Objectives and Plan',
   },
   {
     inputKey: 'e_interested_party_feedback',

apps/api/src/isms/documents/management-review.spec.ts

@@ -61,6 +61,13 @@ describe('parseReviewAttendees / isReviewSigned', () => {
     expect(parseReviewAttendees(null)).toEqual([]);
     expect(parseReviewAttendees('not-an-array')).toEqual([]);
     expect(parseReviewAttendees([{ memberId: 'mem_1' }])).toEqual([]);
+    // Legacy rows may still carry duplicates — normalized on read.
+    expect(
+      parseReviewAttendees([
+        { memberId: 'mem_1', name: 'Jane' },
+        { memberId: 'mem_1', name: 'Jane (dup)' },
+      ]),
+    ).toEqual([{ memberId: 'mem_1', name: 'Jane' }]);
   });
 
   it('dedupes attendees by member, first occurrence winning', () => {

apps/api/src/isms/documents/management-review.ts

@@ -44,10 +44,16 @@ export type ReviewAttendee = z.infer<typeof reviewAttendeeSchema>;
 
 export const reviewAttendeesSchema = z.array(reviewAttendeeSchema);
 
-/** Parse a stored attendees JSON value defensively (invalid → empty list). */
+/**
+ * Parse a stored attendees JSON value defensively (invalid → empty list,
+ * all-or-nothing like the write schema) and normalize away duplicate members
+ * a legacy row may still carry. The client mirror (parseAttendees in
+ * management-review-constants.ts) applies the same rules so the Submit UI
+ * and the server gate always agree.
+ */
 export function parseReviewAttendees(value: unknown): ReviewAttendee[] {
   const parsed = reviewAttendeesSchema.safeParse(value);
-  return parsed.success ? parsed.data : [];
+  return parsed.success ? dedupeReviewAttendees(parsed.data) : [];
 }
 
 /** A review is signed once the chair's name and date are both captured. */

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

@@ -130,7 +130,16 @@ describe('IsmsService ensureSetup', () => {
       (mockDb.ismsDocument.findMany as jest.Mock)
         .mockResolvedValueOnce([]) // existing-types probe
         .mockResolvedValueOnce([{ id: 'doc_ia', type: 'internal_audit' }]) // created lookup
-        .mockResolvedValueOnce([]); // final list
+        .mockResolvedValueOnce([
+          {
+            id: 'doc_ia',
+            type: 'internal_audit',
+            status: 'draft',
+            requirementId: null,
+            currentVersionId: null,
+            draftNarrative: null,
+          },
+        ]); // final list — the seed now works off this, not the created lookup
 
       await service.ensureSetup(dto);
 
@@ -156,6 +165,70 @@ describe('IsmsService ensureSetup', () => {
       });
     });
 
+    it('heals a PRE-EXISTING Management Review doc whose procedure is still empty', async () => {
+      (
+        mockDb.frameworkEditorFramework.findUnique as jest.Mock
+      ).mockResolvedValue({ id: 'fw_1', requirements: [] });
+      mockTemplates.mockResolvedValue([]);
+      (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({
+        name: 'Acme Corp',
+      });
+      (mockDb.ismsDocument.findMany as jest.Mock)
+        // Every type already exists, so provisioning creates nothing and the
+        // old created-scoped seed would never have run.
+        .mockResolvedValueOnce([
+          { type: 'context_of_organization' },
+          { type: 'interested_parties_register' },
+          { type: 'interested_parties_requirements' },
+          { type: 'isms_scope' },
+          { type: 'leadership_commitment' },
+          { type: 'roles_and_responsibilities' },
+          { type: 'objectives_plan' },
+          { type: 'monitoring' },
+          { type: 'internal_audit' },
+          { type: 'management_review' },
+        ])
+        .mockResolvedValueOnce([
+          {
+            id: 'doc_mr',
+            type: 'management_review',
+            status: 'draft',
+            requirementId: null,
+            currentVersionId: null,
+            draftNarrative: {}, // blank — e.g. a crash between provision and seed
+          },
+          {
+            id: 'doc_ia',
+            type: 'internal_audit',
+            status: 'draft',
+            requirementId: null,
+            currentVersionId: null,
+            draftNarrative: { programme: 'Customer-edited programme.' },
+          },
+        ]); // final list
+
+      await service.ensureSetup(dto);
+
+      // Only the blank document is healed; the populated one is untouched.
+      expect(mockDb.ismsDocument.updateMany).toHaveBeenCalledTimes(1);
+      expect(mockDb.ismsDocument.updateMany).toHaveBeenCalledWith({
+        where: {
+          id: 'doc_mr',
+          OR: [
+            { draftNarrative: { equals: Prisma.AnyNull } },
+            { draftNarrative: { equals: {} } },
+          ],
+        },
+        data: {
+          draftNarrative: {
+            procedure: expect.stringContaining(
+              'Acme Corp holds a management review',
+            ),
+          },
+        },
+      });
+    });
+
     it('reports overdueMetricCount on the monitoring document row (CS-723)', async () => {
       const { addPeriods, periodStartFor } = jest.requireActual<
         typeof import('./utils/metric-periods')

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

@@ -5,6 +5,7 @@ import {
   NotFoundException,
 } from '@nestjs/common';
 import { db, Prisma } from '@db';
+import type { IsmsDocumentType } from '@db';
 import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto';
 import { deriveControlLinks, resolveDocumentPlans } from './utils/ensure-setup-plan';
 import { collectPlatformData } from './documents/data-source';
@@ -79,6 +80,15 @@ export class IsmsService {
       where: { organizationId, frameworkId },
     });
 
+    // Heal empty Programme (9.2) / Procedure (9.3) paragraphs on EVERY setup,
+    // not just for documents created this call: a document provisioned before
+    // its narrative seed shipped (or a crash between provision and seed) would
+    // otherwise stay blank forever. Guarded per doc on the narrative still
+    // being empty, so a populated draft is never overwritten.
+    if (canWrite) {
+      await this.seedNarrativeDefaultsIfEmpty({ documents, organizationId });
+    }
+
     const monitoringDoc = documents.find((doc) => doc.type === 'monitoring');
     const overdueMetricCount = monitoringDoc
       ? await this.countOverdueMetrics(monitoringDoc.id)
@@ -223,53 +233,65 @@ export class IsmsService {
       );
     }
 
-    // Same first-load guarantee for Internal Audit (9.2) and Management Review
-    // (9.3): the Programme / Procedure paragraph opens with its default text.
-    // Each write is conditional on the narrative still being NULL (its
-    // creation state), so it is atomic: under concurrent setup calls — where
-    // the "created" lookup can also match a row the other call just created —
-    // an early customer edit can never be clobbered (the seed simply matches
-    // zero rows).
-    const internalAuditDoc = created.find(
-      (doc) => doc.type === 'internal_audit',
-    );
-    const managementReviewDoc = created.find(
-      (doc) => doc.type === 'management_review',
+    // The Programme (9.2) / Procedure (9.3) narrative seed happens in
+    // ensureSetup (seedNarrativeDefaultsIfEmpty), AFTER the document list is
+    // fetched — so it covers both the documents this call just created and
+    // any pre-existing document whose narrative is still empty.
+  }
+
+  /**
+   * First-load guarantee for Internal Audit (9.2) and Management Review (9.3):
+   * the Programme / Procedure paragraph opens with its default text. Each
+   * write is conditional on the narrative still being empty — NULL (the
+   * creation state) or {}, generateNarrative's definition — so it is atomic:
+   * under concurrent setup calls, an early customer edit can never be
+   * clobbered (the seed simply matches zero rows).
+   */
+  private async seedNarrativeDefaultsIfEmpty({
+    documents,
+    organizationId,
+  }: {
+    documents: Array<{
+      id: string;
+      type: IsmsDocumentType;
+      draftNarrative: Prisma.JsonValue;
+    }>;
+    organizationId: string;
+  }) {
+    const isEmptyNarrative = (narrative: Prisma.JsonValue): boolean =>
+      narrative == null ||
+      (typeof narrative === 'object' &&
+        !Array.isArray(narrative) &&
+        Object.keys(narrative).length === 0);
+
+    const targets = documents.filter(
+      (doc) =>
+        (doc.type === 'internal_audit' || doc.type === 'management_review') &&
+        isEmptyNarrative(doc.draftNarrative),
     );
-    if (internalAuditDoc || managementReviewDoc) {
-      const organization = await db.organization.findUnique({
-        where: { id: organizationId },
-        select: { name: true },
+    if (targets.length === 0) return;
+
+    const organization = await db.organization.findUnique({
+      where: { id: organizationId },
+      select: { name: true },
+    });
+    const organizationName = organization?.name ?? 'The organization';
+    const whileNarrativeEmpty = {
+      OR: [
+        { draftNarrative: { equals: Prisma.AnyNull } },
+        { draftNarrative: { equals: {} } },
+      ],
+    };
+    for (const doc of targets) {
+      await db.ismsDocument.updateMany({
+        where: { id: doc.id, ...whileNarrativeEmpty },
+        data: {
+          draftNarrative:
+            doc.type === 'internal_audit'
+              ? { programme: defaultProgrammeText(organizationName) }
+              : { procedure: defaultProcedureText(organizationName) },
+        },
       });
-      const organizationName = organization?.name ?? 'The organization';
-      // "Empty" matches generateNarrative's definition: NULL (the creation
-      // state) or an empty object — never a populated draft.
-      const whileNarrativeEmpty = {
-        OR: [
-          { draftNarrative: { equals: Prisma.AnyNull } },
-          { draftNarrative: { equals: {} } },
-        ],
-      };
-      if (internalAuditDoc) {
-        await db.ismsDocument.updateMany({
-          where: { id: internalAuditDoc.id, ...whileNarrativeEmpty },
-          data: {
-            draftNarrative: {
-              programme: defaultProgrammeText(organizationName),
-            },
-          },
-        });
-      }
-      if (managementReviewDoc) {
-        await db.ismsDocument.updateMany({
-          where: { id: managementReviewDoc.id, ...whileNarrativeEmpty },
-          data: {
-            draftNarrative: {
-              procedure: defaultProcedureText(organizationName),
-            },
-          },
-        });
-      }
     }
   }
 
@@ -315,7 +337,9 @@ export class IsmsService {
           },
         },
         reviews: {
-          orderBy: { position: 'asc' },
+          // Same deterministic ordering as EXPORT_DOCUMENT_INCLUDE: the client
+          // computes the carried-forward lists from this order.
+          orderBy: [{ position: 'asc' }, { createdAt: 'asc' }, { id: 'asc' }],
           include: {
             inputs: { orderBy: { position: 'asc' } },
             actions: { orderBy: { position: 'asc' } },

apps/api/src/isms/utils/export-payload.ts

@@ -81,7 +81,10 @@ export const EXPORT_DOCUMENT_INCLUDE = {
     },
   },
   reviews: {
-    orderBy: { position: 'asc' },
+    // Deterministic tie-breakers: review order drives which actions carry
+    // forward into which review's input (a), so two reviews sharing a
+    // position must order the same on every read.
+    orderBy: [{ position: 'asc' }, { createdAt: 'asc' }, { id: 'asc' }],
     include: {
       inputs: { orderBy: { position: 'asc' } },
       actions: { orderBy: { position: 'asc' } },

apps/api/src/isms/utils/review-participants.ts

@@ -40,7 +40,9 @@ export async function resolveReviewParticipantDefaults({
       select: {
         roleKey: true,
         assignments: {
-          orderBy: { position: 'asc' },
+          // Deterministic tie-breaker: two assignments sharing a position must
+          // resolve the same default chair/SPO on every read.
+          orderBy: [{ position: 'asc' }, { id: 'asc' }],
           select: { memberId: true },
         },
       },

apps/app/src/app/(app)/[orgId]/documents/isms/components/CarriedForwardActions.tsx

@@ -73,8 +73,12 @@ export function CarriedForwardActions({
                 </TableCell>
                 <TableCell>{action.description}</TableCell>
                 <TableCell>
+                  {/* An empty roster means the people request was unavailable
+                      to this user, not that the owner left — stay neutral. */}
                   {action.ownerMemberId
-                    ? (memberNameById[action.ownerMemberId] ?? 'Former member')
+                    ? memberOptions.length === 0
+                      ? 'Unknown member'
+                      : (memberNameById[action.ownerMemberId] ?? 'Former member')
                     : '—'}
                 </TableCell>
                 <TableCell>{action.dueDate?.slice(0, 10) ?? '—'}</TableCell>

apps/app/src/app/(app)/[orgId]/documents/isms/components/ReviewActionRow.tsx

@@ -244,8 +244,12 @@ export function ReviewActionRow({
       </TableCell>
       <TableCell>{action.description}</TableCell>
       <TableCell>
+        {/* An empty roster means the people request was unavailable to this
+            user, not that the owner left — stay neutral. */}
         {action.ownerMemberId
-          ? (memberNameById[action.ownerMemberId] ?? 'Former member')
+          ? memberOptions.length === 0
+            ? 'Unknown member'
+            : (memberNameById[action.ownerMemberId] ?? 'Former member')
           : '—'}
       </TableCell>
       <TableCell>{action.dueDate?.slice(0, 10) ?? '—'}</TableCell>

apps/app/src/app/(app)/[orgId]/documents/isms/components/ReviewFields.tsx

@@ -23,6 +23,7 @@ import type { ReviewDetailsFormValues } from './management-review-schema';
 import { IsmsFieldLabel } from './shared';
 
 const NO_VERDICT = 'no-verdict';
+const NO_CHAIR = 'no-chair';
 
 interface ReviewFieldsProps {
   control: Control<ReviewDetailsFormValues>;
@@ -62,11 +63,18 @@ function ChairSelect({
     );
   }
   return (
-    <Select value={value || undefined} onValueChange={(next) => onChange(next ?? '')}>
+    <Select
+      value={value || NO_CHAIR}
+      onValueChange={(next) =>
+        onChange(!next || next === NO_CHAIR ? '' : next)
+      }
+    >
       <SelectTrigger aria-label="Chair">
         <SelectValue placeholder="Select the chair" />
       </SelectTrigger>
       <SelectContent>
+        {/* Explicit clear: a stale chair must be removable, not only replaceable. */}
+        <SelectItem value={NO_CHAIR}>No chair yet</SelectItem>
         {options.map((option) => (
           <SelectItem key={option} value={option}>
             {option}

apps/app/src/app/(app)/[orgId]/documents/isms/components/ReviewOutputsSection.tsx

@@ -59,6 +59,13 @@ export function ReviewOutputsSection({
     if (!isEditing) reset(toFormValues(review));
   }, [review, isEditing, reset]);
 
+  // If the review becomes signed (or the caller loses edit rights) while the
+  // outputs editor is open, close it — otherwise the textareas would linger
+  // with stale local edits and no Save action.
+  useEffect(() => {
+    if (!canEdit && isEditing) setIsEditing(false);
+  }, [canEdit, isEditing]);
+
   const handleSave = handleSubmit(async (values) => {
     try {
       await onSave(values);
@@ -117,7 +124,7 @@ export function ReviewOutputsSection({
         changed. Actions arising are recorded in the table below.
       </Text>
 
-      {isEditing ? (
+      {isEditing && canEdit ? (
         <Stack gap="3">
           <IsmsFieldLabel label="Decisions on continual improvement">
             <Controller