fix(tables): make folder cascades retry-safe

Author: j15z

apps/sim/lib/folders/bulk.test.ts

@@ -3,24 +3,28 @@
  */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockListActiveFolderRows } = vi.hoisted(() => ({
-  mockListActiveFolderRows: vi.fn(),
-}))
+const { mockDeleteFolder, mockListActiveFolderRows, mockNotifyFolderResourceChanged } = vi.hoisted(
+  () => ({
+    mockDeleteFolder: vi.fn(),
+    mockListActiveFolderRows: vi.fn(),
+    mockNotifyFolderResourceChanged: vi.fn(),
+  })
+)
 
 vi.mock('@/lib/folders/queries', () => ({
   listActiveFolderRows: mockListActiveFolderRows,
 }))
 
 vi.mock('@/lib/folders/orchestration', () => ({
-  deleteFolder: vi.fn(),
+  deleteFolder: mockDeleteFolder,
   updateFolder: vi.fn(),
 }))
 
 vi.mock('@/lib/realtime/notify', () => ({
-  notifyFolderResourceChanged: vi.fn(),
+  notifyFolderResourceChanged: mockNotifyFolderResourceChanged,
 }))
 
-import { planFolderSelection } from '@/lib/folders/bulk'
+import { bulkDeleteFolders, planFolderSelection } from '@/lib/folders/bulk'
 
 /**
  * `a` holds `a1`, which holds `a1x`. `b` is a sibling with nothing inside it, so a plan can
@@ -94,3 +98,78 @@ describe('planFolderSelection', () => {
     expect(result.selected).toEqual([{ id: 'b', name: 'B' }])
   })
 })
+
+describe('bulkDeleteFolders', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  const deleteFolders = (folders: Array<{ id: string; name: string }>) =>
+    bulkDeleteFolders({
+      workspaceId: 'ws-1',
+      resourceType: 'table',
+      userId: 'user-1',
+      folders,
+      countKey: 'tables',
+      referenceCheckCompleted: true,
+    })
+
+  it('retries a target folder after a later referrer folder succeeds', async () => {
+    mockDeleteFolder
+      .mockResolvedValueOnce({
+        success: false,
+        error: 'Target is referenced by Referrer',
+        errorCode: 'conflict',
+      })
+      .mockResolvedValueOnce({
+        success: true,
+        deletedItems: { folders: 1, tables: 1 },
+      })
+      .mockResolvedValueOnce({
+        success: true,
+        deletedItems: { folders: 1, tables: 1 },
+      })
+
+    const result = await deleteFolders([
+      { id: 'target-folder', name: 'Target' },
+      { id: 'referrer-folder', name: 'Referrer' },
+    ])
+
+    expect(result).toEqual({
+      succeeded: [
+        { id: 'referrer-folder', name: 'Referrer' },
+        { id: 'target-folder', name: 'Target' },
+      ],
+      failed: [],
+      folderCount: 2,
+      resourceCount: 2,
+    })
+    expect(mockDeleteFolder.mock.calls.map(([params]) => params.folderId)).toEqual([
+      'target-folder',
+      'referrer-folder',
+      'target-folder',
+    ])
+    expect(mockNotifyFolderResourceChanged).toHaveBeenCalledOnce()
+  })
+
+  it('stops when a reference cycle makes no progress', async () => {
+    mockDeleteFolder.mockResolvedValue({
+      success: false,
+      error: 'Referenced by another selected folder',
+      errorCode: 'conflict',
+    })
+
+    const result = await deleteFolders([
+      { id: 'folder-a', name: 'A' },
+      { id: 'folder-b', name: 'B' },
+    ])
+
+    expect(result.succeeded).toEqual([])
+    expect(result.failed).toEqual([
+      { id: 'folder-a', name: 'A', reason: 'Referenced by another selected folder' },
+      { id: 'folder-b', name: 'B', reason: 'Referenced by another selected folder' },
+    ])
+    expect(mockDeleteFolder).toHaveBeenCalledTimes(2)
+    expect(mockNotifyFolderResourceChanged).not.toHaveBeenCalled()
+  })
+})

apps/sim/lib/folders/bulk.ts

@@ -244,6 +244,10 @@ export async function bulkMoveFolders(params: {
  * notification replaces a per-folder storm of identical invalidations, and it
  * fires from a `finally` so a batch cut short by an internal fault still
  * announces the folders it did archive.
+ *
+ * A reference conflict is retried only after another folder succeeds. This
+ * resolves cross-folder dependency chains without relying on request order,
+ * while a cycle makes no progress and terminates after one wave.
  */
 export async function bulkDeleteFolders(params: {
   workspaceId: string
@@ -260,30 +264,55 @@ export async function bulkDeleteFolders(params: {
   let resourceCount = 0
 
   try {
-    for (const folder of params.folders) {
-      const result = await deleteFolder(
-        {
-          resourceType: params.resourceType,
-          folderId: folder.id,
-          workspaceId: params.workspaceId,
-          userId: params.userId,
-          folderName: folder.name,
-        },
-        {
-          projectAudit: false,
-          notify: false,
-          referenceCheckCompleted: params.referenceCheckCompleted,
+    let pending = [...params.folders]
+    while (pending.length > 0) {
+      const retryable: BulkFolderAffected[] = []
+      const retryReasons = new Map<string, string>()
+      let completedInWave = 0
+
+      for (const folder of pending) {
+        const result = await deleteFolder(
+          {
+            resourceType: params.resourceType,
+            folderId: folder.id,
+            workspaceId: params.workspaceId,
+            userId: params.userId,
+            folderName: folder.name,
+          },
+          {
+            projectAudit: false,
+            notify: false,
+            referenceCheckCompleted: params.referenceCheckCompleted,
+          }
+        )
+        if (result.success) {
+          succeeded.push(folder)
+          folderCount += result.deletedItems?.folders ?? 0
+          resourceCount += result.deletedItems?.[params.countKey] ?? 0
+          completedInWave += 1
+          continue
         }
-      )
-      if (result.success) {
-        succeeded.push(folder)
-        folderCount += result.deletedItems?.folders ?? 0
-        resourceCount += result.deletedItems?.[params.countKey] ?? 0
-        continue
+        if (result.errorCode === 'internal') {
+          throw new Error(result.error ?? 'Failed to delete folder')
+        }
+        if (result.errorCode === 'conflict') {
+          retryable.push(folder)
+          retryReasons.set(folder.id, result.error ?? 'Failed to delete folder')
+          continue
+        }
+        failed.push({ ...folder, reason: result.error ?? 'Failed to delete folder' })
+      }
+
+      if (completedInWave === 0) {
+        for (const folder of retryable) {
+          failed.push({
+            ...folder,
+            reason: retryReasons.get(folder.id) ?? 'Failed to delete folder',
+          })
+        }
+        break
       }
-      if (result.errorCode === 'internal')
-        throw new Error(result.error ?? 'Failed to delete folder')
-      failed.push({ ...folder, reason: result.error ?? 'Failed to delete folder' })
+      pending = retryable
     }
   } finally {
     if (succeeded.length > 0) {

apps/sim/lib/folders/orchestration.test.ts

@@ -784,6 +784,15 @@ describe('deleteFolder', () => {
       error: 'Table deletion is locked',
       errorCode: 'locked',
     })
+    const archiveTimestamp = mockArchiveFolderCascade.mock.calls[0][4]
+    expect(mockRestoreFolderRows).toHaveBeenCalledWith(
+      dbChainMock.db,
+      resourceConfig.current,
+      'ws-1',
+      ['folder-1'],
+      archiveTimestamp,
+      expect.any(Date)
+    )
   })
 
   it('hands the guard the whole resolved subtree, not just the root', async () => {

apps/sim/lib/folders/orchestration.ts

@@ -863,6 +863,7 @@ async function deleteFolderWithoutTreeLock(
   } catch (error) {
     const classified = asOrchestrationError(error)
     if (classified?.code === 'locked' || classified?.code === 'conflict') {
+      await restoreFolderRows(db, config, workspaceId, folderIds, timestamp, new Date())
       return { success: false, error: classified.message, errorCode: classified.code }
     }
     throw error

apps/sim/lib/table/reference-columns/availability.ts

@@ -9,7 +9,7 @@ export function areTableReferenceColumnsEnabled(): Promise<boolean> {
   return isFeatureEnabled('table-reference-columns')
 }
 
-/** Rejects mutations that introduce or reconfigure a Reference column. */
+/** Rejects operations that expose or mutate Reference-column behavior. */
 export async function assertTableReferenceColumnsEnabled(): Promise<void> {
   if (!(await areTableReferenceColumnsEnabled())) {
     throw new OrchestrationError('forbidden', TABLE_REFERENCE_COLUMNS_DISABLED_MESSAGE)

apps/sim/lib/table/service.test.ts

@@ -77,7 +77,11 @@ import {
 const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8'
 
 describe('listActiveTableNames', () => {
-  beforeEach(() => resetDbChainMock())
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+    mocks.assertTableReferenceColumnsEnabled.mockResolvedValue(undefined)
+  })
 
   it('returns the name projection without loading table schemas', async () => {
     queueTableRows(schemaMock.userTableDefinitions, [{ id: 'table-1', name: 'Accounts' }])
@@ -102,6 +106,17 @@ describe('listActiveTableNames', () => {
 
   it('skips the database when no table IDs are requested', async () => {
     await expect(listActiveTableNames(WORKSPACE_ID, [])).resolves.toEqual([])
+    expect(mocks.assertTableReferenceColumnsEnabled).toHaveBeenCalledOnce()
+    expect(dbChainMockFns.select).not.toHaveBeenCalled()
+  })
+
+  it('rejects name lookups before querying when reference columns are disabled', async () => {
+    mocks.assertTableReferenceColumnsEnabled.mockRejectedValueOnce({ code: 'forbidden' })
+
+    await expect(listActiveTableNames(WORKSPACE_ID, ['table-1'])).rejects.toMatchObject({
+      code: 'forbidden',
+    })
+
     expect(dbChainMockFns.select).not.toHaveBeenCalled()
   })
 })

apps/sim/lib/table/service.ts

@@ -384,6 +384,7 @@ export async function listActiveTableNames(
   workspaceId: string,
   tableIds: readonly string[]
 ): Promise<Array<Pick<TableDefinition, 'id' | 'name'>>> {
+  await assertTableReferenceColumnsEnabled()
   if (tableIds.length === 0) return []
 
   return db