diff --git a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts index 7f1c7cdf52..f7ddc36ae0 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts @@ -1,15 +1,30 @@ import { db } from '@db'; import { generateObject } from 'ai'; -import { processPolicyUpdate } from './update-policy-helpers'; +import { + processPolicyUpdate, + updatePolicyInDatabase, +} from './update-policy-helpers'; jest.mock('@db', () => ({ db: { organization: { findUnique: jest.fn() }, policy: { findUnique: jest.fn(), update: jest.fn() }, frameworkEditorPolicyTemplate: { findUnique: jest.fn() }, - policyVersion: { create: jest.fn(), deleteMany: jest.fn() }, + policyVersion: { + findFirst: jest.fn(), + create: jest.fn(), + deleteMany: jest.fn(), + }, $transaction: jest.fn(), }, + Prisma: { + PrismaClientKnownRequestError: class PrismaClientKnownRequestError {}, + }, + PolicyStatus: { + draft: 'draft', + published: 'published', + needs_review: 'needs_review', + }, })); jest.mock('@trigger.dev/sdk', () => ({ @@ -79,6 +94,7 @@ describe('processPolicyUpdate (individual policy regeneration)', () => { cb({ policy: { update: jest.fn() }, policyVersion: { + findFirst: jest.fn().mockResolvedValue(null), create: jest.fn(({ data }: { data: { content: unknown[] } }) => { storedContent = data.content; return { id: 'pv_1' }; @@ -127,3 +143,188 @@ describe('processPolicyUpdate (individual policy regeneration)', () => { expect(result.policyName).toBe('Information Security Policy'); }); }); + +// CS-766: Regenerating a PUBLISHED, signed policy must not touch the live +// version. It must append a new DRAFT version (for the approval workflow) while +// leaving policy.content, currentVersionId, signedBy, pdfUrl and the existing +// versions intact. Only publishing that draft (elsewhere) clears signedBy and +// re-triggers signing. +describe('updatePolicyInDatabase (published policy regeneration)', () => { + const REGEN_CONTENT = [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Regenerated draft content' }], + }, + ]; + + let txPolicyUpdate: jest.Mock; + let txVersionCreate: jest.Mock; + let txVersionDeleteMany: jest.Mock; + let txVersionFindFirst: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + + // A published policy: v1 is the current, signed, live version. + (db.policy.findUnique as jest.Mock).mockResolvedValue({ + id: 'pol_1', + status: 'published', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Published v1' }] }, + ], + currentVersionId: 'pv_1', + signedBy: ['mem_a', 'mem_b'], + pdfUrl: 'org_1/policies/pol_1/v1.pdf', + versions: [{ id: 'pv_1', pdfUrl: null, version: 1 }], + }); + + txPolicyUpdate = jest.fn(); + txVersionCreate = jest.fn(() => ({ id: 'pv_2' })); + txVersionDeleteMany = jest.fn(); + txVersionFindFirst = jest.fn().mockResolvedValue({ version: 1 }); + + (db.$transaction as jest.Mock).mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ + policy: { update: txPolicyUpdate }, + policyVersion: { + findFirst: txVersionFindFirst, + create: txVersionCreate, + deleteMany: txVersionDeleteMany, + }, + }), + ); + }); + + it('appends a new draft version and preserves the published version + signatures', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // Existing versions (and their PDFs) must survive — the published version + // must not be destroyed. + expect(txVersionDeleteMany).not.toHaveBeenCalled(); + + // A brand-new version is appended at the next number (not overwriting v1). + expect(txVersionCreate).toHaveBeenCalledTimes(1); + const createData = txVersionCreate.mock.calls[0][0].data; + expect(createData.version).toBe(2); + expect(createData.changelog).toBe('Regenerated policy content'); + expect(JSON.stringify(createData.content)).toContain( + 'Regenerated draft content', + ); + + // The published policy row must NOT be mutated: no signature wipe, no live + // content swap, no currentVersion repoint. + const policyUpdateData = txPolicyUpdate.mock.calls.map( + (call) => (call[0] as { data?: Record })?.data ?? {}, + ); + for (const data of policyUpdateData) { + expect(data).not.toHaveProperty('signedBy'); + expect(data).not.toHaveProperty('content'); + expect(data).not.toHaveProperty('currentVersionId'); + } + }); +}); + +// CS-766 follow-up: Regenerating a DRAFT policy (never published, unsigned) must +// SURFACE the regenerated content. The editor renders the current version's +// content (falling back to policy.content), so regeneration overwrites the +// current draft version IN PLACE and syncs policy.content/draftContent — it must +// NOT append an unattached version that leaves the draft showing stale text. +describe('updatePolicyInDatabase (draft policy regeneration)', () => { + const REGEN_CONTENT = [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Regenerated draft content' }], + }, + ]; + + let txPolicyUpdate: jest.Mock; + let txVersionUpdate: jest.Mock; + let txVersionCreate: jest.Mock; + let txVersionDeleteMany: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + + // A draft policy uploaded as a PDF: displayFormat is 'PDF' and both the + // policy and its current version carry a stale pdfUrl (the old document). + (db.policy.findUnique as jest.Mock).mockResolvedValue({ + id: 'pol_1', + status: 'draft', + currentVersionId: 'pv_1', + displayFormat: 'PDF', + pdfUrl: 'org_1/policies/pol_1/uploaded.pdf', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Stale draft' }] }, + ], + signedBy: [], + versions: [ + { id: 'pv_1', pdfUrl: 'org_1/policies/pol_1/v1.pdf', version: 1 }, + ], + }); + + txPolicyUpdate = jest.fn(); + txVersionUpdate = jest.fn(); + txVersionCreate = jest.fn(() => ({ id: 'pv_2' })); + txVersionDeleteMany = jest.fn(); + + (db.$transaction as jest.Mock).mockImplementation( + async (cb: (tx: unknown) => Promise) => + cb({ + policy: { update: txPolicyUpdate }, + policyVersion: { + update: txVersionUpdate, + create: txVersionCreate, + deleteMany: txVersionDeleteMany, + findFirst: jest.fn().mockResolvedValue({ version: 1 }), + }, + }), + ); + }); + + it('overwrites the current draft version in place and syncs policy content (no unattached version)', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // The regenerated content overwrites the CURRENT draft version in place so + // the editor (which reads currentVersion.content) surfaces it. + expect(txVersionUpdate).toHaveBeenCalledTimes(1); + const versionUpdate = txVersionUpdate.mock.calls[0][0]; + expect(versionUpdate.where.id).toBe('pv_1'); + expect(JSON.stringify(versionUpdate.data.content)).toContain( + 'Regenerated draft content', + ); + + // No unattached extra version is appended (and nothing is deleted) for a + // draft — the working version is edited in place. + expect(txVersionCreate).not.toHaveBeenCalled(); + expect(txVersionDeleteMany).not.toHaveBeenCalled(); + + // policy.content AND draftContent advance to the regenerated content so the + // draft no longer shows stale text; currentVersionId is not repointed. + expect(txPolicyUpdate).toHaveBeenCalledTimes(1); + const policyUpdate = txPolicyUpdate.mock.calls[0][0].data; + expect(JSON.stringify(policyUpdate.content)).toContain( + 'Regenerated draft content', + ); + expect(JSON.stringify(policyUpdate.draftContent)).toContain( + 'Regenerated draft content', + ); + expect(policyUpdate).not.toHaveProperty('currentVersionId'); + }); + + it('clears stale PDF references and switches to EDITOR display when the draft was uploaded as a PDF', async () => { + await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'); + + // Regeneration produces EDITOR content: the policy must switch back to the + // editor and drop its stale policy-level PDF, otherwise the page opens on + // the PDF tab / export keeps serving the old uploaded document. + const policyUpdate = txPolicyUpdate.mock.calls[0][0].data; + expect(policyUpdate.displayFormat).toBe('EDITOR'); + expect(policyUpdate.pdfUrl).toBeNull(); + + // The current version's stale PDF (used first by render/export via + // currentVersion.pdfUrl ?? policy.pdfUrl) must be cleared too. + const versionUpdate = txVersionUpdate.mock.calls[0][0]; + expect(versionUpdate.data.pdfUrl).toBeNull(); + }); +}); diff --git a/apps/api/src/trigger/policies/update-policy-helpers.ts b/apps/api/src/trigger/policies/update-policy-helpers.ts index 7913092f1d..3cff8397c4 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.ts @@ -1,9 +1,8 @@ -import { db } from '@db'; +import { db, Prisma, PolicyStatus } from '@db'; import type { FrameworkEditorFramework, FrameworkEditorPolicyTemplate, Policy, - Prisma, } from '@db'; import { logger } from '@trigger.dev/sdk'; import { processTemplate } from './process-policy-template'; @@ -67,6 +66,11 @@ export async function fetchOrganizationAndPolicy( return { organization, policy, policyTemplate }; } +// Mirror PoliciesService.versionCreateRetries: retry version creation on a +// unique-constraint race so two near-simultaneous regenerations don't collide +// on the [policyId, version] key. +const POLICY_VERSION_CREATE_RETRIES = 3; + export async function updatePolicyInDatabase( policyId: string, content: Record[], @@ -75,79 +79,99 @@ export async function updatePolicyInDatabase( try { const policy = await db.policy.findUnique({ where: { id: policyId }, - include: { versions: { select: { id: true, pdfUrl: true } } }, + select: { id: true, status: true, currentVersionId: true }, }); if (!policy) throw new Error(`Policy not found: ${policyId}`); - // Delete S3 files for existing versions - const pdfUrlsToDelete = policy.versions - .map((v) => v.pdfUrl) - .filter((url): url is string => !!url); - - if (pdfUrlsToDelete.length > 0) { - try { - const { S3Client, DeleteObjectCommand } = - await import('@aws-sdk/client-s3'); - const bucketName = process.env.APP_AWS_BUCKET_NAME; - if (bucketName) { - const s3 = new S3Client({ - region: process.env.AWS_REGION || 'us-east-1', + const versionContent = content as unknown as Prisma.InputJsonValue[]; + + // A draft policy has never been published: there is no live, signed content + // to protect. The editor renders the CURRENT version's content + // (currentVersion.content, falling back to policy.content), so regeneration + // must overwrite the draft's working content IN PLACE — mirroring how + // PoliciesService.updateById persists draft content edits. Appending an + // unattached version instead (the published path below) would leave + // policy.content / currentVersionId pointing at the old text, so the user + // regenerates but keeps seeing the stale draft (CS-766). + // + // A draft can be in PDF display mode (the user uploaded a PDF as its + // content): displayFormat = 'PDF' with pdfUrl set on the policy and/or the + // current version. Regeneration produces EDITOR content, so we must clear + // those stale PDF references and switch displayFormat back to 'EDITOR' — + // otherwise the page opens on the PDF tab and export/render (which use + // currentVersion.pdfUrl ?? policy.pdfUrl) keep serving the old uploaded + // document instead of the regenerated content (CS-766). + if (policy.status === PolicyStatus.draft) { + await db.$transaction(async (tx) => { + if (policy.currentVersionId) { + await tx.policyVersion.update({ + where: { id: policy.currentVersionId }, + data: { + content: versionContent, + changelog: 'Regenerated policy content', + pdfUrl: null, + }, }); - await Promise.allSettled( - pdfUrlsToDelete.map((pdfUrl) => - s3.send( - new DeleteObjectCommand({ Bucket: bucketName, Key: pdfUrl }), - ), - ), - ); } - } catch (s3Error) { - logger.error(`Error deleting S3 files during regeneration: ${s3Error}`); - } - } - - await db.$transaction(async (tx) => { - // Clear version references first to avoid FK constraint issues during deletion. - // Clear approverId alongside pendingVersionId so the two fields never diverge - // — any lingering approverId without a pending version produces the inconsistent - // state behind CS-254/260/261 ("No pending version to approve"). - if (policy.versions.length > 0) { await tx.policy.update({ where: { id: policyId }, data: { - currentVersionId: null, - pendingVersionId: null, - approverId: null, + content: versionContent, + draftContent: versionContent, + pdfUrl: null, + displayFormat: 'EDITOR', }, }); - await tx.policyVersion.deleteMany({ where: { policyId } }); - } - - const newVersion = await tx.policyVersion.create({ - data: { - policyId, - version: 1, - content: content as unknown as Prisma.InputJsonValue[], - publishedById: memberId || null, - changelog: 'Regenerated policy content', - }, }); + return; + } - await tx.policy.update({ - where: { id: policyId }, - data: { - content: content as unknown as Prisma.InputJsonValue[], - draftContent: content as unknown as Prisma.InputJsonValue[], - currentVersionId: newVersion.id, - pendingVersionId: null, - approverId: null, - signedBy: [], - pdfUrl: null, - displayFormat: 'EDITOR', - }, - }); - }); + // Published / needs_review: regeneration must NOT mutate the live policy. + // This previously deleted every version (and its PDF) and overwrote + // policy.content / currentVersionId while clearing signedBy — replacing the + // live, signed policy with unreviewed AI content and wiping every signature + // (CS-766). Instead, create a new DRAFT version holding the regenerated + // content and leave the published content, currentVersionId, signatures, PDF + // and existing versions untouched. The draft is reviewed through the normal + // version workflow; only publishing it (which clears signedBy) re-triggers + // signing. + for ( + let attempt = 1; + attempt <= POLICY_VERSION_CREATE_RETRIES; + attempt += 1 + ) { + try { + await db.$transaction(async (tx) => { + const latestVersion = await tx.policyVersion.findFirst({ + where: { policyId }, + orderBy: { version: 'desc' }, + select: { version: true }, + }); + const nextVersion = (latestVersion?.version ?? 0) + 1; + + await tx.policyVersion.create({ + data: { + policyId, + version: nextVersion, + content: versionContent, + publishedById: memberId || null, + changelog: 'Regenerated policy content', + }, + }); + }); + return; + } catch (error) { + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === 'P2002' && + attempt < POLICY_VERSION_CREATE_RETRIES + ) { + continue; + } + throw error; + } + } } catch (dbError) { logger.error(`Failed to update policy in database: ${dbError}`); throw dbError; diff --git a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyHeaderActions.tsx b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyHeaderActions.tsx index b3a131d8dd..c59ed78125 100644 --- a/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyHeaderActions.tsx +++ b/apps/app/src/app/(app)/[orgId]/policies/[policyId]/components/PolicyHeaderActions.tsx @@ -86,7 +86,7 @@ export function PolicyHeaderActions({ if (toastIdRef.current) { toast.dismiss(toastIdRef.current); } - toast.success('Policy content updated!'); + toast.success('New draft version created for review'); setIsRegenerating(false); setRunInfo(null); toastIdRef.current = null; @@ -259,9 +259,9 @@ export function PolicyHeaderActions({ Regenerate Policy - This will generate new policy content using your org context and frameworks. It will - delete all existing versions and their PDFs for this policy. This cannot be undone. - Continue? + This will generate new policy content using your org context and frameworks and save + it as a new draft version. Your published policy and its signatures stay unchanged + until you review and approve the draft. Continue?