chore(db): drop the legacy folder tables and adopt the deferred folder_id FKs (#6051)

* chore(db): drop the legacy folder tables and adopt the deferred folder_id FKs

Contract migration closing out the generic-folders cutover: drops workflow_folder
and workspace_file_folders, and adopts the two folder_id foreign keys 0272
deliberately deferred.

Runs a final INSERT-ONLY reconcile first so no stranded legacy folder is lost by
the drop. Insert-only because 0272 deliberately renamed 47 workflow folders to
dedupe them, and an upsert would revert all 47. In production this is a no-op
(verified 0 stranded); it exists for deployments that never ran the post-drain
reconcile as an operational step, self-hosted upgrades above all.

Hand-written rather than drizzle-generated: the generated form drops the tables
BEFORE adding the FKs with no step to re-root unresolvable folder_ids, so one
dangling id fails ADD CONSTRAINT after the legacy data is already gone; it also
validates the FK inline, holding ACCESS EXCLUSIVE across a full scan of the
1.7M-row workspace_files, and uses DROP TABLE CASCADE. The generated snapshot is
kept so drizzle-kit generate reports no schema changes.

Every re-root path dedupes, because all three destinations are partial unique
indexes keyed on a coalesced nullable column: folder, workflow, and
workspace_files. Parents must also be reachable — an active row re-roots off a
soft-deleted parent, while a soft-deleted row may keep one.

Also hardens the retention sweep, which the new ON DELETE SET NULL exposes: a
surviving active child re-rooted by the FK can collide at the workspace root,
and chunkedBatchDelete turns that 23505 into a permanent per-chunk stall. The
folder cleanup target now renames children first, covering workflows, files and
subfolders, re-asserts eligibility so a restore mid-batch is not stripped, and
treats the deduplicated name as a hint since both allocators can return a
colliding one.

* fix(db): stop the re-root dedupe renaming personal workflows

workflow.workspace_id is nullable, and NULL is treated as EQUAL by PARTITION BY
but UNKNOWN by the = in base_taken. So for personal workflows rn incremented
across the group while no collision was ever detected: the dedupe could only
fire spuriously, renaming a user-visible workflow that needed no rename, since
the unique index treats NULL workspace_id rows as distinct anyway. The file
block already carried the equivalent guard.

Also gives step 1's inserts ON CONFLICT (id) DO NOTHING. It restates the
stranded guard, closing the window between that read's snapshot and the index
check — an operational re-run of 0274, or a live pod, committing into folder
mid-statement would otherwise raise 23505, and migrate.ts retries only 55P03.
0272 and 0274 were already written this way; step 1 was the exception.

Corrects three comments that claimed more than the code delivers: the header's
'no legacy folder is lost' (an id already present under another resource_type
cannot be rescued), the reachability note (a cycle among stranded rows survives
a per-row parent check), and a note made stale by the ON CONFLICT above.

Author: waleedlatif1

apps/sim/background/cleanup-soft-deletes.test.ts

@@ -2,7 +2,13 @@
  * @vitest-environment node
  */
 
-import { dbChainMock, dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing'
+import {
+  dbChainMock,
+  dbChainMockFns,
+  queueTableRows,
+  resetDbChainMock,
+  schemaMock,
+} from '@sim/testing'
 import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
@@ -18,7 +24,13 @@ const {
   mockPrepareChatCleanup,
   mockResolveStorageBillingContext,
   mockSelectRowsByIdChunks,
+  mockDeduplicateWorkflowName,
+  mockAllocateUniqueWorkspaceFileName,
+  mockDeduplicateFolderName,
 } = vi.hoisted(() => ({
+  mockDeduplicateFolderName: vi.fn(async (_tx, _ws, _parent, name: string) => name),
+  mockDeduplicateWorkflowName: vi.fn(async (name: string) => name),
+  mockAllocateUniqueWorkspaceFileName: vi.fn(async (_ws: string, name: string) => name),
   mockBatchDeleteByWorkspaceAndTimestamp: vi.fn(async () => ({ deleted: 0, failed: 0 })),
   mockChunkedBatchDelete: vi.fn(async () => ({ deleted: 0, failed: 0 })),
   mockDecrementStorageUsageForBillingContextInTx: vi.fn(async () => undefined),
@@ -66,6 +78,16 @@ vi.mock('@/lib/uploads', () => ({
 
 vi.mock('@/lib/uploads/server/metadata', () => ({ deleteFileMetadata: mockDeleteFileMetadata }))
 
+vi.mock('@/lib/workflows/utils', () => ({
+  deduplicateWorkflowName: mockDeduplicateWorkflowName,
+}))
+
+vi.mock('@/lib/folders/naming', () => ({ deduplicateFolderName: mockDeduplicateFolderName }))
+
+vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
+  allocateUniqueWorkspaceFileName: mockAllocateUniqueWorkspaceFileName,
+}))
+
 import { runCleanupSoftDeletes } from '@/background/cleanup-soft-deletes'
 
 const basePayload = {
@@ -269,6 +291,7 @@ interface BatchDeleteOptions {
   tableName: string
   requireTimestampNotNull?: boolean
   additionalPredicate?: { type: string; column: unknown; values: unknown[] }
+  onBatch?: (rows: { id: string }[]) => Promise<void>
 }
 
 /**
@@ -331,4 +354,177 @@ describe('folder cleanup target', () => {
       .map(([options]) => options.tableName)
     expect(filtered).toEqual(['free/1/folder'])
   })
+
+  /**
+   * `folder_id` is `ON DELETE SET NULL`, so Postgres re-roots surviving children on its own —
+   * but `workflow` and `workspace_files` each carry a partial unique index keyed on
+   * `coalesce(folder_id, '')`, so an implicit SET NULL can land a child on a name the workspace
+   * root already holds. That aborts the whole DELETE with a 23505, which `chunkedBatchDelete`
+   * turns into `hasMore = false` — folder retention then stalls permanently for that chunk,
+   * re-failing on every later run. `onBatch` renames first so the SET NULL is a no-op.
+   */
+  describe('re-rooting active children before the delete', () => {
+    async function getFolderOnBatch() {
+      const target = await runAndFindFolderTarget()
+      expect(target?.onBatch).toBeTypeOf('function')
+      return target!.onBatch!
+    }
+
+    it('is the only cleanup target that re-roots children', async () => {
+      await runCleanupSoftDeletes(basePayload)
+      const calls = mockBatchDeleteByWorkspaceAndTimestamp.mock.calls as unknown as Array<
+        [BatchDeleteOptions]
+      >
+
+      const withOnBatch = calls
+        .filter(([options]) => options.onBatch !== undefined)
+        .map(([options]) => options.tableName)
+      expect(withOnBatch).toEqual(['free/1/folder'])
+    })
+
+    it('re-roots an active workflow under a deduplicated name', async () => {
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
+      queueTableRows(schemaMock.workflow, [{ id: 'w1', name: 'Report', workspaceId: 'ws-1' }])
+      queueTableRows(schemaMock.workspaceFiles, [])
+      mockDeduplicateWorkflowName.mockResolvedValueOnce('Report (2)')
+
+      await onBatch([{ id: 'folder-1' }])
+
+      // Deduped against the workspace ROOT (folderId null), which is where SET NULL would put it.
+      expect(mockDeduplicateWorkflowName).toHaveBeenCalledWith(
+        'Report',
+        'ws-1',
+        null,
+        expect.anything()
+      )
+      expect(dbChainMockFns.set).toHaveBeenCalledWith({ folderId: null, name: 'Report (2)' })
+    })
+
+    it('re-roots an active workspace file under a deduplicated name', async () => {
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
+      queueTableRows(schemaMock.workflow, [])
+      queueTableRows(schemaMock.workspaceFiles, [
+        { id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
+      ])
+      mockAllocateUniqueWorkspaceFileName.mockResolvedValueOnce('report (2).pdf')
+
+      await onBatch([{ id: 'folder-1' }])
+
+      expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith('ws-1', 'report.pdf', null)
+      expect(dbChainMockFns.set).toHaveBeenCalledWith({
+        folderId: null,
+        originalName: 'report (2).pdf',
+      })
+    })
+
+    it('falls back to an id-suffixed name when the copy-suffix range is exhausted', async () => {
+      // Letting the allocator throw would abort the sweep — the exact stall this guards against.
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
+      queueTableRows(schemaMock.workflow, [])
+      queueTableRows(schemaMock.workspaceFiles, [
+        { id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
+      ])
+      mockAllocateUniqueWorkspaceFileName.mockRejectedValueOnce(new Error('conflict'))
+
+      await expect(onBatch([{ id: 'folder-1' }])).resolves.toBeUndefined()
+
+      expect(dbChainMockFns.set).toHaveBeenCalledWith({
+        folderId: null,
+        originalName: 'report.pdf (f1)',
+      })
+    })
+
+    it('re-roots an active SUBFOLDER, which hits the same unique index', async () => {
+      /**
+       * `folder.parentId` is also ON DELETE SET NULL and
+       * `folder_workspace_resource_parent_name_active_unique` keys on `coalesce(parent_id,'')`,
+       * so purging a parent can collide a surviving child at the root exactly like a workflow
+       * or file. Covering only those two would leave the class half-closed.
+       */
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
+      queueTableRows(schemaMock.workflow, [])
+      queueTableRows(schemaMock.workspaceFiles, [])
+      queueTableRows(schemaMock.folder, [
+        { id: 'sub-1', name: 'Reports', workspaceId: 'ws-1', resourceType: 'knowledge_base' },
+      ])
+      mockDeduplicateFolderName.mockResolvedValueOnce('Reports (1)')
+
+      await onBatch([{ id: 'folder-1' }])
+
+      // Deduped against the ROOT of the child's OWN resourceType, which is where SET NULL lands it.
+      expect(mockDeduplicateFolderName).toHaveBeenCalledWith(
+        expect.anything(),
+        'ws-1',
+        null,
+        'Reports',
+        'knowledge_base'
+      )
+      expect(dbChainMockFns.set).toHaveBeenCalledWith({ parentId: null, name: 'Reports (1)' })
+    })
+
+    it('recovers when the allocator RETURNS a colliding name and the update raises', async () => {
+      /**
+       * `allocateUniqueWorkspaceFileName` fails open — `fileExistsInWorkspace` swallows query
+       * errors and returns false — so it can hand back a name already taken at the root. Only
+       * the UPDATE discovers that, and an uncaught 23505 aborts the batch: the exact stall this
+       * hook prevents. Guarding the name lookup alone is not enough.
+       */
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, [{ id: 'folder-1' }])
+      queueTableRows(schemaMock.workflow, [])
+      queueTableRows(schemaMock.workspaceFiles, [
+        { id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
+      ])
+      mockAllocateUniqueWorkspaceFileName.mockResolvedValueOnce('taken.pdf')
+      dbChainMockFns.update.mockImplementationOnce(() => ({
+        set: () => ({ where: () => Promise.reject(new Error('duplicate key value (23505)')) }),
+      }))
+
+      await expect(onBatch([{ id: 'folder-1' }])).resolves.toBeUndefined()
+
+      expect(dbChainMockFns.set).toHaveBeenCalledWith({
+        folderId: null,
+        originalName: 'report.pdf (f1)',
+      })
+    })
+
+    it('leaves children alone when the folder was restored between select and onBatch', async () => {
+      /**
+       * The DELETE re-asserts eligibility and so correctly skips a restored folder. Without the
+       * same re-assertion here, this hook would still strip and rename that folder's children —
+       * leaving a live folder emptied out. This is the one side effect in the sweep that mutates
+       * rows which survive, so losing the race is user-visible.
+       */
+      const onBatch = await getFolderOnBatch()
+      queueTableRows(schemaMock.folder, []) // restored: no longer soft-deleted past retention
+      // Children ARE queued: without the eligibility re-check these would be re-rooted and
+      // renamed, so the assertions below fail rather than passing for want of candidate rows.
+      queueTableRows(schemaMock.workflow, [{ id: 'w1', name: 'Report', workspaceId: 'ws-1' }])
+      queueTableRows(schemaMock.workspaceFiles, [
+        { id: 'f1', originalName: 'report.pdf', workspaceId: 'ws-1' },
+      ])
+      dbChainMockFns.update.mockClear()
+
+      await onBatch([{ id: 'folder-1' }])
+
+      expect(mockDeduplicateWorkflowName).not.toHaveBeenCalled()
+      expect(mockAllocateUniqueWorkspaceFileName).not.toHaveBeenCalled()
+      expect(dbChainMockFns.update).not.toHaveBeenCalled()
+    })
+
+    it('touches nothing when the batch is empty', async () => {
+      const onBatch = await getFolderOnBatch()
+      dbChainMockFns.select.mockClear()
+      dbChainMockFns.update.mockClear()
+
+      await onBatch([])
+
+      expect(dbChainMockFns.select).not.toHaveBeenCalled()
+      expect(dbChainMockFns.update).not.toHaveBeenCalled()
+    })
+  })
 })