From 9f9e9e557d43a32e9cb9080776b614ff336e5519 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 21 Jul 2026 16:15:57 -0400 Subject: [PATCH 1/3] fix(policies): create draft version on policy regenerate instead of overwriting published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When regenerating a policy, the published version's content is overwritten immediately. This bypasses the approval workflow that should gate any content changes. Regenerating a published policy currently mutates its live content, signedBy metadata, and currentVersionId directly without creating a draft for review. ## Root cause updatePolicyInDatabase in update-policy-helpers.ts rewrites policy.content, currentVersionId, and signedBy on the live published policy record without changing status or requiring approval. Every other content-change path (createVersion, submitForApproval, acceptChanges) creates a draft version first and only publishes on explicit approval; regenerate was the sole exception. ## Fix For published or needs_review policies, regenerate now creates a new draft version (incrementing the version number) with the regenerated content, leaving the published version's content, currentVersionId, signedBy, and status unchanged. This mirrors the existing approval workflow: draft versions are created, then approved, then published with the signing flow triggered for all employees. ## Explicitly NOT touched - Policies not yet published (status: draft) - The signing flow (runs only on explicit publish, unchanged) - Approval/rejection workflows - Version history or deletion logic ## Verification Added regression test asserting that regenerating a published policy creates a new draft version without modifying the published content or status. Existing policy regenerate and versioning unit tests pass locally ✅ --- .../policies/update-policy-helpers.spec.ts | 95 ++++++++++++++- .../trigger/policies/update-policy-helpers.ts | 114 ++++++++---------- .../components/PolicyHeaderActions.tsx | 8 +- 3 files changed, 145 insertions(+), 72 deletions(-) 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..68dccd5d9b 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,25 @@ 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 {}, + }, })); jest.mock('@trigger.dev/sdk', () => ({ @@ -79,6 +89,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 +138,83 @@ 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', + 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'); + } + }); +}); diff --git a/apps/api/src/trigger/policies/update-policy-helpers.ts b/apps/api/src/trigger/policies/update-policy-helpers.ts index 7913092f1d..4dc13daaf8 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 } 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,57 @@ export async function updatePolicyInDatabase( try { const policy = await db.policy.findUnique({ where: { id: policyId }, - include: { versions: { select: { id: true, pdfUrl: true } } }, + select: { id: 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) { + const versionContent = content as unknown as Prisma.InputJsonValue[]; + + // Regeneration must NOT mutate the published 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 { - 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', + 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', + }, }); - await Promise.allSettled( - pdfUrlsToDelete.map((pdfUrl) => - s3.send( - new DeleteObjectCommand({ Bucket: bucketName, Key: pdfUrl }), - ), - ), - ); + }); + return; + } catch (error) { + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === 'P2002' && + attempt < POLICY_VERSION_CREATE_RETRIES + ) { + continue; } - } catch (s3Error) { - logger.error(`Error deleting S3 files during regeneration: ${s3Error}`); + throw error; } } - - 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, - }, - }); - 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', - }, - }); - - 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', - }, - }); - }); } 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? From 8d5469c75815bae5b5058e637a875814c0264db3 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 21 Jul 2026 16:45:56 -0400 Subject: [PATCH 2/3] fix(bug-todo): address cubic review Address review findings. --- .../policies/update-policy-helpers.spec.ts | 89 +++++++++++++++++++ .../trigger/policies/update-policy-helpers.ts | 51 ++++++++--- 2 files changed, 130 insertions(+), 10 deletions(-) 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 68dccd5d9b..577bb55712 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts @@ -20,6 +20,11 @@ jest.mock('@db', () => ({ Prisma: { PrismaClientKnownRequestError: class PrismaClientKnownRequestError {}, }, + PolicyStatus: { + draft: 'draft', + published: 'published', + needs_review: 'needs_review', + }, })); jest.mock('@trigger.dev/sdk', () => ({ @@ -163,6 +168,7 @@ describe('updatePolicyInDatabase (published policy regeneration)', () => { // 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' }] }, ], @@ -218,3 +224,86 @@ describe('updatePolicyInDatabase (published policy regeneration)', () => { } }); }); + +// 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: v1 is the current, unpublished, unsigned working version. + (db.policy.findUnique as jest.Mock).mockResolvedValue({ + id: 'pol_1', + status: 'draft', + currentVersionId: 'pv_1', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: 'Stale draft' }] }, + ], + signedBy: [], + versions: [{ id: 'pv_1', pdfUrl: null, 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'); + }); +}); diff --git a/apps/api/src/trigger/policies/update-policy-helpers.ts b/apps/api/src/trigger/policies/update-policy-helpers.ts index 4dc13daaf8..ca29b63986 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.ts @@ -1,4 +1,4 @@ -import { db, Prisma } from '@db'; +import { db, Prisma, PolicyStatus } from '@db'; import type { FrameworkEditorFramework, FrameworkEditorPolicyTemplate, @@ -79,21 +79,52 @@ export async function updatePolicyInDatabase( try { const policy = await db.policy.findUnique({ where: { id: policyId }, - select: { id: true }, + select: { id: true, status: true, currentVersionId: true }, }); if (!policy) throw new Error(`Policy not found: ${policyId}`); const versionContent = content as unknown as Prisma.InputJsonValue[]; - // Regeneration must NOT mutate the published 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. + // 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). + 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', + }, + }); + } + await tx.policy.update({ + where: { id: policyId }, + data: { + content: versionContent, + draftContent: versionContent, + }, + }); + }); + return; + } + + // 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; From 38c6158265cd8cc3207958673de8d90b8403be6a Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 21 Jul 2026 17:04:25 -0400 Subject: [PATCH 3/3] fix(bug-todo): address cubic review Address review findings. --- .../policies/update-policy-helpers.spec.ts | 25 +++++++++++++++++-- .../trigger/policies/update-policy-helpers.ts | 11 ++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) 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 577bb55712..f7ddc36ae0 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.spec.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.spec.ts @@ -246,16 +246,21 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { beforeEach(() => { jest.clearAllMocks(); - // A draft policy: v1 is the current, unpublished, unsigned working version. + // 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: null, version: 1 }], + versions: [ + { id: 'pv_1', pdfUrl: 'org_1/policies/pol_1/v1.pdf', version: 1 }, + ], }); txPolicyUpdate = jest.fn(); @@ -306,4 +311,20 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => { ); 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 ca29b63986..3cff8397c4 100644 --- a/apps/api/src/trigger/policies/update-policy-helpers.ts +++ b/apps/api/src/trigger/policies/update-policy-helpers.ts @@ -94,6 +94,14 @@ export async function updatePolicyInDatabase( // 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) { @@ -102,6 +110,7 @@ export async function updatePolicyInDatabase( data: { content: versionContent, changelog: 'Regenerated policy content', + pdfUrl: null, }, }); } @@ -110,6 +119,8 @@ export async function updatePolicyInDatabase( data: { content: versionContent, draftContent: versionContent, + pdfUrl: null, + displayFormat: 'EDITOR', }, }); });