fix(policies): delete detached PDF objects when regenerating a draft

apps/api/src/trigger/policies/update-policy-helpers.spec.ts

@@ -31,6 +31,18 @@ jest.mock('@trigger.dev/sdk', () => ({
   logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() },
 }));
 
+// Detached-PDF cleanup imports the S3 SDK dynamically; the mock intercepts it
+// so tests can assert deletions without touching AWS.
+const s3Send = jest.fn();
+jest.mock('@aws-sdk/client-s3', () => ({
+  S3Client: class {
+    send = s3Send;
+  },
+  DeleteObjectCommand: class {
+    constructor(public readonly input: { Bucket: string; Key: string }) {}
+  },
+}));
+
 jest.mock('ai', () => ({
   generateObject: jest.fn(),
   NoObjectGeneratedError: { isInstance: jest.fn(() => false) },
@@ -239,6 +251,7 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => {
   ];
 
   let txPolicyUpdate: jest.Mock;
+  let txPolicyFind: jest.Mock;
   let txVersionUpdate: jest.Mock;
   let txVersionCreate: jest.Mock;
   let txVersionDeleteMany: jest.Mock;
@@ -254,6 +267,7 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => {
       currentVersionId: 'pv_1',
       displayFormat: 'PDF',
       pdfUrl: 'org_1/policies/pol_1/uploaded.pdf',
+      currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' },
       content: [
         { type: 'paragraph', content: [{ type: 'text', text: 'Stale draft' }] },
       ],
@@ -264,14 +278,20 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => {
     });
 
     txPolicyUpdate = jest.fn();
+    // The in-transaction (row-locked) re-read that captures the detached keys.
+    txPolicyFind = jest.fn().mockResolvedValue({
+      pdfUrl: 'org_1/policies/pol_1/uploaded.pdf',
+      currentVersion: { pdfUrl: 'org_1/policies/pol_1/v1.pdf' },
+    });
     txVersionUpdate = jest.fn();
     txVersionCreate = jest.fn(() => ({ id: 'pv_2' }));
     txVersionDeleteMany = jest.fn();
 
     (db.$transaction as jest.Mock).mockImplementation(
       async (cb: (tx: unknown) => Promise<unknown>) =>
         cb({
-          policy: { update: txPolicyUpdate },
+          $executeRaw: jest.fn(),
+          policy: { update: txPolicyUpdate, findUniqueOrThrow: txPolicyFind },
           policyVersion: {
             update: txVersionUpdate,
             create: txVersionCreate,
@@ -327,4 +347,91 @@ describe('updatePolicyInDatabase (draft policy regeneration)', () => {
     const versionUpdate = txVersionUpdate.mock.calls[0][0];
     expect(versionUpdate.data.pdfUrl).toBeNull();
   });
+
+  describe('detached-PDF S3 cleanup', () => {
+    const ENV_KEYS = [
+      'APP_AWS_BUCKET_NAME',
+      'APP_AWS_ACCESS_KEY_ID',
+      'APP_AWS_SECRET_ACCESS_KEY',
+    ] as const;
+    const ORIGINAL_ENV = Object.fromEntries(
+      ENV_KEYS.map((key) => [key, process.env[key]]),
+    );
+
+    const configureS3Env = () => {
+      process.env.APP_AWS_BUCKET_NAME = 'test-bucket';
+      process.env.APP_AWS_ACCESS_KEY_ID = 'test-key';
+      process.env.APP_AWS_SECRET_ACCESS_KEY = 'test-secret';
+    };
+
+    afterEach(() => {
+      for (const key of ENV_KEYS) {
+        const original = ORIGINAL_ENV[key];
+        if (original === undefined) delete process.env[key];
+        else process.env[key] = original;
+      }
+    });
+
+    it('deletes the PDF keys captured inside the locked transaction', async () => {
+      configureS3Env();
+
+      await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen');
+
+      // The keys come from the in-transaction re-read (not the outer snapshot),
+      // so a PDF uploaded concurrently before the row lock is still covered.
+      expect(txPolicyFind).toHaveBeenCalledTimes(1);
+
+      const deletedKeys = s3Send.mock.calls.map(
+        (call) => (call[0] as { input: { Bucket: string; Key: string } }).input,
+      );
+      expect(deletedKeys).toEqual([
+        { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/uploaded.pdf' },
+        { Bucket: 'test-bucket', Key: 'org_1/policies/pol_1/v1.pdf' },
+      ]);
+    });
+
+    it('deduplicates when the policy and its version share one PDF key', async () => {
+      configureS3Env();
+      txPolicyFind.mockResolvedValue({
+        pdfUrl: 'org_1/policies/pol_1/shared.pdf',
+        currentVersion: { pdfUrl: 'org_1/policies/pol_1/shared.pdf' },
+      });
+
+      await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen');
+
+      expect(s3Send).toHaveBeenCalledTimes(1);
+    });
+
+    it('never fails the regeneration when an S3 delete fails (logged for cleanup)', async () => {
+      configureS3Env();
+      s3Send.mockRejectedValueOnce(new Error('AccessDenied'));
+
+      await expect(
+        updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen'),
+      ).resolves.toBeUndefined();
+      // Both keys are still attempted; the failure is logged, not thrown.
+      expect(s3Send).toHaveBeenCalledTimes(2);
+    });
+
+    it('skips deletion (with a warning) when the S3 configuration is missing', async () => {
+      configureS3Env();
+      delete process.env.APP_AWS_BUCKET_NAME;
+
+      await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen');
+
+      expect(s3Send).not.toHaveBeenCalled();
+    });
+
+    it('does not touch S3 for an editor-mode draft with no PDFs', async () => {
+      configureS3Env();
+      txPolicyFind.mockResolvedValue({
+        pdfUrl: null,
+        currentVersion: { pdfUrl: null },
+      });
+
+      await updatePolicyInDatabase('pol_1', REGEN_CONTENT, 'mem_regen');
+
+      expect(s3Send).not.toHaveBeenCalled();
+    });
+  });
 });

apps/api/src/trigger/policies/update-policy-helpers.ts

@@ -1,3 +1,4 @@
+import { DeleteObjectCommand, S3Client } from '@aws-sdk/client-s3';
 import { db, Prisma, PolicyStatus } from '@db';
 import type {
   FrameworkEditorFramework,
@@ -71,6 +72,43 @@ export async function fetchOrganizationAndPolicy(
 // on the [policyId, version] key.
 const POLICY_VERSION_CREATE_RETRIES = 3;
 
+/**
+ * Best-effort removal of the PDF objects a draft regeneration detached
+ * (pdfUrl stores the raw S3 key — same contract as the policies controller).
+ * Runs AFTER the database transaction commits so a failed delete can never
+ * roll back the content update; failures are logged with their keys so the
+ * objects can be cleaned up later instead of orphaning silently.
+ */
+async function deleteDetachedPdfObjects(keys: string[]): Promise<void> {
+  if (keys.length === 0) return;
+  // Same APP_AWS_* configuration as the other trigger tasks (evidence export)
+  // — but non-throwing: cleanup is best-effort and must never fail the
+  // regeneration, so missing configuration is logged and skipped.
+  const bucketName = process.env.APP_AWS_BUCKET_NAME;
+  const accessKeyId = process.env.APP_AWS_ACCESS_KEY_ID;
+  const secretAccessKey = process.env.APP_AWS_SECRET_ACCESS_KEY;
+  if (!bucketName || !accessKeyId || !secretAccessKey) {
+    logger.warn(
+      `APP_AWS_* S3 configuration missing; skipped deleting detached policy PDFs: ${keys.join(', ')}`,
+    );
+    return;
+  }
+  const s3 = new S3Client({
+    region: process.env.APP_AWS_REGION || 'us-east-1',
+    credentials: { accessKeyId, secretAccessKey },
+    ...(process.env.APP_AWS_ENDPOINT
+      ? { endpoint: process.env.APP_AWS_ENDPOINT, forcePathStyle: true }
+      : {}),
+  });
+  for (const key of keys) {
+    try {
+      await s3.send(new DeleteObjectCommand({ Bucket: bucketName, Key: key }));
+    } catch (error) {
+      logger.warn(`Failed to delete detached policy PDF ${key}: ${error}`);
+    }
+  }
+}
+
 export async function updatePolicyInDatabase(
   policyId: string,
   content: Record<string, unknown>[],
@@ -103,7 +141,18 @@ export async function updatePolicyInDatabase(
     // 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) => {
+      // The uploaded-PDF keys this regeneration detaches, captured INSIDE the
+      // transaction under a row lock: a concurrent PDF upload (a plain UPDATE
+      // on the policy row) blocks on the lock, so the captured keys are
+      // exactly the values the updates below clear — nothing committed in
+      // between can slip an untracked orphan past the cleanup. Deleted from
+      // S3 only after the transaction commits.
+      const detachedPdfKeys = await db.$transaction(async (tx) => {
+        await tx.$executeRaw`SELECT id FROM "Policy" WHERE id = ${policyId} FOR UPDATE`;
+        const current = await tx.policy.findUniqueOrThrow({
+          where: { id: policyId },
+          select: { pdfUrl: true, currentVersion: { select: { pdfUrl: true } } },
+        });
         if (policy.currentVersionId) {
           await tx.policyVersion.update({
             where: { id: policy.currentVersionId },
@@ -123,7 +172,15 @@ export async function updatePolicyInDatabase(
             displayFormat: 'EDITOR',
           },
         });
+        return [
+          ...new Set(
+            [current.pdfUrl, current.currentVersion?.pdfUrl].filter(
+              (key): key is string => !!key,
+            ),
+          ),
+        ];
       });
+      await deleteDetachedPdfObjects(detachedPdfKeys);
       return;
     }