[codex] Move R2 deletes to delayed prefix

cloudflare_workers/r2/README.md

@@ -0,0 +1,19 @@
+# R2 lifecycle rules
+
+This directory stores bucket-level R2 lifecycle configuration.
+
+`lifecycle.capgo.json` deletes objects under `deleted-after-7-days/` after 7 days. That prefix must stay aligned with `R2_TRASH_PREFIX` in `supabase/functions/_backend/utils/s3.ts`.
+
+Apply the tracked config:
+
+```bash
+bunx wrangler r2 bucket lifecycle set capgo --file cloudflare_workers/r2/lifecycle.capgo.json --force
+```
+
+Verify the rule:
+
+```bash
+bunx wrangler r2 bucket lifecycle list capgo
+```
+
+`lifecycle set` replaces the bucket lifecycle configuration. If the `capgo` bucket already has other lifecycle rules, add them to `lifecycle.capgo.json` before applying it.

cloudflare_workers/r2/lifecycle.capgo.json

@@ -0,0 +1,17 @@
+{
+  "rules": [
+    {
+      "id": "delete-trash-after-7-days",
+      "enabled": true,
+      "conditions": {
+        "prefix": "deleted-after-7-days/"
+      },
+      "deleteObjectsTransition": {
+        "condition": {
+          "type": "Age",
+          "maxAge": 604800
+        }
+      }
+    }
+  ]
+}

supabase/functions/_backend/triggers/on_version_update.ts

@@ -193,7 +193,7 @@ async function updateIt(c: Context, record: Database['public']['Tables']['app_ve
 }
 
 /**
- * Deletes manifest rows and orphaned S3 assets for a removed app version.
+ * Deletes manifest rows and moves orphaned S3 assets to the R2 trash prefix.
  */
 async function deleteManifest(c: Context, record: Database['public']['Tables']['app_versions']['Row']) {
   // Delete manifest entries - first get them to delete from S3
@@ -209,11 +209,11 @@ async function deleteManifest(c: Context, record: Database['public']['Tables']['
     if (manifestEntries && manifestEntries.length > 0) {
       const manifestCount = manifestEntries.length
 
-      // Delete each file from S3
-      const promisesDeleteS3 = []
+      // Move each unreferenced file to the R2 trash prefix.
+      const promisesMoveToTrash = []
       for (const entry of manifestEntries) {
         if (entry.s3_path) {
-          promisesDeleteS3.push(
+          promisesMoveToTrash.push(
             // First delete the manifest row from database
             supabaseAdmin(c)
               .from('manifest')
@@ -245,19 +245,19 @@ async function deleteManifest(c: Context, record: Database['public']['Tables']['
                   // Other versions still use this file, S3 cleanup not needed
                   return
                 }
-                // No other versions use this file, delete from S3
-                cloudlog({ requestId: c.get('requestId'), message: 'deleted manifest file from S3', s3_path: entry.s3_path })
-                return s3.deleteObject(c, entry.s3_path)
-                  .then((deleted) => {
-                    if (!deleted) {
-                      throw simpleError('cannot_delete_manifest_s3', 'Cannot delete S3 object for deleted manifest file', { id: entry.id, s3_path: entry.s3_path })
+                // No other versions use this file, move it to the R2 trash prefix.
+                cloudlog({ requestId: c.get('requestId'), message: 'moving manifest file to R2 trash', s3_path: entry.s3_path })
+                return s3.moveObjectToTrash(c, entry.s3_path)
+                  .then((moved) => {
+                    if (!moved) {
+                      throw simpleError('cannot_move_manifest_s3_to_trash', 'Cannot move S3 object for deleted manifest file to trash', { id: entry.id, s3_path: entry.s3_path })
                     }
                   })
               }),
           )
         }
       }
-      await Promise.all(promisesDeleteS3)
+      await Promise.all(promisesMoveToTrash)
 
       // After deleting manifest entries, update manifest_count and decrement manifest_bundle_count
       const updatePgClient = getPgClient(c, false)
@@ -298,17 +298,17 @@ export async function deleteIt(c: Context, record: Database['public']['Tables'][
   cloudlog({ requestId: c.get('requestId'), message: 'Delete', r2_path: record.r2_path })
 
   if (record.r2_path) {
-    let deleted = false
+    let moved = false
     try {
-      deleted = await s3.deleteObject(c, record.r2_path)
+      moved = await s3.moveObjectToTrash(c, record.r2_path)
     }
     catch (error) {
-      cloudlog({ requestId: c.get('requestId'), message: 'Cannot delete s3 (v2)', error })
-      throw simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path }, error)
+      cloudlog({ requestId: c.get('requestId'), message: 'Cannot move s3 to trash (v2)', error })
+      throw simpleError('cannot_move_s3_to_trash', 'Cannot move S3 object for deleted version to trash', { id: record.id, r2_path: record.r2_path }, error)
     }
 
-    if (!deleted) {
-      throw simpleError('cannot_delete_s3', 'Cannot delete S3 object for deleted version', { id: record.id, r2_path: record.r2_path })
+    if (!moved) {
+      throw simpleError('cannot_move_s3_to_trash', 'Cannot move S3 object for deleted version to trash', { id: record.id, r2_path: record.r2_path })
     }
   }
   else {

supabase/functions/_backend/utils/s3.ts

@@ -73,6 +73,12 @@ function initS3(c: Context) {
   return client
 }
 
+const R2_TRASH_PREFIX = 'deleted-after-7-days/'
+
+function getTrashPath(fileId: string) {
+  return `${R2_TRASH_PREFIX}${fileId}`
+}
+
 export async function getPath(
   c: Context,
   record: Database['public']['Tables']['app_versions']['Row'],
@@ -121,6 +127,37 @@ async function deleteObject(c: Context, fileId: string) {
   return response.status >= 200 && response.status < 300
 }
 
+function isMissingObjectError(error: unknown): boolean {
+  if (!error || typeof error !== 'object')
+    return false
+
+  const candidate = error as { code?: unknown, status?: unknown, statusCode?: unknown }
+  return candidate.status === 404 || candidate.statusCode === 404 || candidate.code === 'NoSuchKey'
+}
+
+async function moveObjectToTrash(c: Context, fileId: string) {
+  if (fileId.startsWith(R2_TRASH_PREFIX))
+    return true
+
+  const client = initS3(c)
+  const trashPath = getTrashPath(fileId)
+  try {
+    await client.copyObject({ sourceKey: fileId }, trashPath)
+    await client.deleteObject(fileId)
+    cloudlog({ requestId: c.get('requestId'), message: 'moved R2 object to trash', fileId, trashPath })
+    return true
+  }
+  catch (error) {
+    if (isMissingObjectError(error)) {
+      cloudlog({ requestId: c.get('requestId'), message: 'R2 object already missing before trash move', fileId, error: serializeStorageError(error) })
+      return true
+    }
+
+    cloudlogErr({ requestId: c.get('requestId'), message: 'move R2 object to trash failed', fileId, trashPath, error: serializeStorageError(error) })
+    return false
+  }
+}
+
 async function deleteObjectsWithPrefix(c: Context, prefix: string): Promise<number> {
   const client = initS3(c)
   let deletedCount = 0
@@ -347,6 +384,7 @@ async function getObject(c: Context, fileId: string): Promise<Response | null> {
 export const s3 = {
   getSize,
   deleteObject,
+  moveObjectToTrash,
   deleteObjectsWithPrefix,
   checkIfExist,
   getSignedUrl,

tests/on-version-update-cleanup.unit.test.ts

@@ -9,21 +9,41 @@ const {
   deleteObject,
   getDrizzleClient,
   getPgClient,
+  manifestDeleteEq,
+  manifestReferenceMaybeSingle,
+  manifestSelectWhere,
+  moveObjectToTrash,
+  pgQuery,
   supabaseAdmin,
 } = vi.hoisted(() => {
   const appVersionsMetaSelectEq = vi.fn()
   const appVersionsMetaSelect = vi.fn(() => ({ eq: appVersionsMetaSelectEq }))
   const appVersionsMetaUpdateEq = vi.fn()
   const appVersionsMetaUpdate = vi.fn(() => ({ eq: appVersionsMetaUpdateEq }))
+  const manifestDeleteEq = vi.fn()
+  const manifestDelete = vi.fn(() => ({ eq: manifestDeleteEq }))
+  const manifestReferenceMaybeSingle = vi.fn()
+  const manifestReferenceLimit = vi.fn(() => ({ maybeSingle: manifestReferenceMaybeSingle }))
+  const manifestReferenceEqFileName = vi.fn(() => ({ limit: manifestReferenceLimit }))
+  const manifestReferenceEqFileHash = vi.fn(() => ({ eq: manifestReferenceEqFileName }))
+  const manifestSelect = vi.fn(() => ({ eq: manifestReferenceEqFileHash }))
   const supabaseFrom = vi.fn((table: string) => {
     if (table === 'app_versions_meta') {
       return {
         select: appVersionsMetaSelect,
         update: appVersionsMetaUpdate,
       }
     }
+    if (table === 'manifest') {
+      return {
+        delete: manifestDelete,
+        select: manifestSelect,
+      }
+    }
     return {}
   })
+  const manifestSelectWhere = vi.fn(async (): Promise<any[]> => [])
+  const pgQuery = vi.fn(async () => ({ rows: [] }))
 
   return {
     appVersionsMetaSelectEq,
@@ -35,11 +55,16 @@ const {
     getDrizzleClient: vi.fn(() => ({
       select: vi.fn(() => ({
         from: vi.fn(() => ({
-          where: vi.fn(async () => []),
+          where: manifestSelectWhere,
         })),
       })),
     })),
-    getPgClient: vi.fn(() => ({})),
+    getPgClient: vi.fn(() => ({ query: pgQuery })),
+    manifestDeleteEq,
+    manifestReferenceMaybeSingle,
+    manifestSelectWhere,
+    moveObjectToTrash: vi.fn(),
+    pgQuery,
     supabaseAdmin: vi.fn(() => ({ from: supabaseFrom })),
     supabaseFrom,
   }
@@ -49,6 +74,7 @@ vi.mock('../supabase/functions/_backend/utils/s3.ts', () => ({
   getPath: vi.fn(),
   s3: {
     deleteObject,
+    moveObjectToTrash,
   },
 }))
 
@@ -96,18 +122,24 @@ describe('on_version_update deleted version cleanup', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     deleteObject.mockResolvedValue(true)
+    moveObjectToTrash.mockResolvedValue(true)
     createStatsMeta.mockResolvedValue({ error: null })
+    manifestSelectWhere.mockResolvedValue([])
+    manifestDeleteEq.mockResolvedValue({ error: null })
+    manifestReferenceMaybeSingle.mockResolvedValue({ data: null, error: null })
+    pgQuery.mockResolvedValue({ rows: [] })
     appVersionsMetaSelectEq.mockReturnValue({
       single: vi.fn(async () => ({ data: { size: 1234 }, error: null })),
     })
     appVersionsMetaUpdateEq.mockResolvedValue({ error: null })
   })
 
-  it('deletes the bundle directly and clears stored size for soft-deleted versions', async () => {
+  it('moves the bundle to trash and clears stored size for soft-deleted versions', async () => {
     const response = await deleteIt(createContext(), createVersion())
 
     expect(response.status).toBe(200)
-    expect(deleteObject).toHaveBeenCalledWith(expect.anything(), 'orgs/org-1/apps/com.cleanup.test/1.0.0.zip')
+    expect(moveObjectToTrash).toHaveBeenCalledWith(expect.anything(), 'orgs/org-1/apps/com.cleanup.test/1.0.0.zip')
+    expect(deleteObject).not.toHaveBeenCalled()
     expect(appVersionsMetaUpdate).toHaveBeenCalledWith({ size: 0 })
     expect(appVersionsMetaUpdateEq).toHaveBeenCalledWith('id', 123)
     expect(createStatsMeta).toHaveBeenCalledWith(expect.anything(), 'com.cleanup.test', 123, -1234)
@@ -117,15 +149,33 @@ describe('on_version_update deleted version cleanup', () => {
     const response = await deleteIt(createContext(), createVersion({ r2_path: null }))
 
     expect(response.status).toBe(200)
-    expect(deleteObject).not.toHaveBeenCalled()
+    expect(moveObjectToTrash).not.toHaveBeenCalled()
     expect(appVersionsMetaUpdate).toHaveBeenCalledWith({ size: 0 })
     expect(createStatsMeta).toHaveBeenCalledWith(expect.anything(), 'com.cleanup.test', 123, -1234)
   })
 
-  it('keeps the queue retryable when R2 deletion fails', async () => {
-    deleteObject.mockResolvedValue(false)
+  it('moves unreferenced manifest files to trash instead of hard deleting them', async () => {
+    manifestSelectWhere.mockResolvedValue([{
+      app_version_id: 123,
+      file_hash: 'manifest-hash',
+      file_name: 'index.js',
+      id: 456,
+      s3_path: 'orgs/org-1/apps/com.cleanup.test/manifest/index.js',
+    }])
+
+    const response = await deleteIt(createContext(), createVersion({ r2_path: null }))
+
+    expect(response.status).toBe(200)
+    expect(moveObjectToTrash).toHaveBeenCalledWith(expect.anything(), 'orgs/org-1/apps/com.cleanup.test/manifest/index.js')
+    expect(deleteObject).not.toHaveBeenCalled()
+    expect(pgQuery).toHaveBeenCalledWith('UPDATE app_versions SET manifest_count = 0 WHERE id = $1', [123])
+    expect(pgQuery).toHaveBeenCalledWith(expect.stringContaining('manifest_bundle_count = GREATEST(manifest_bundle_count - 1, 0)'), ['com.cleanup.test'])
+  })
+
+  it('keeps the queue retryable when moving the bundle to trash fails', async () => {
+    moveObjectToTrash.mockResolvedValue(false)
 
-    await expect(deleteIt(createContext(), createVersion())).rejects.toThrow('Cannot delete S3 object for deleted version')
+    await expect(deleteIt(createContext(), createVersion())).rejects.toThrow('Cannot move S3 object for deleted version to trash')
     expect(appVersionsMetaUpdate).not.toHaveBeenCalled()
     expect(createStatsMeta).not.toHaveBeenCalled()
   })