fix(cleanup): treat the deduplicated re-root name as a hint, not a guarantee

allocateUniqueWorkspaceFileName fails OPEN — fileExistsInWorkspace swallows
query errors and returns false — so it can return a name already taken at the
workspace root. Only the UPDATE discovers that, and the previous guard wrapped
just the name lookup, so the resulting 23505 escaped and aborted the batch: the
same permanent retention stall this hook exists to prevent.

Both re-root paths now retry with an id-suffixed name, which cannot collide, and
a failure of that retry is logged rather than thrown so one unfixable row does
not stop the rest of the batch being made safe.

Author: waleedlatif1

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

@@ -433,6 +433,32 @@ describe('folder cleanup target', () => {
       })
     })
 
+    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

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

@@ -432,6 +432,42 @@ interface CleanupBatchContext {
   label: string
 }
 
+/**
+ * Applies one re-root, treating the deduplicated name as a HINT rather than a guarantee.
+ *
+ * Both name allocators can hand back a name that is already taken: `fileExistsInWorkspace`
+ * swallows query errors and returns `false`, so `allocateUniqueWorkspaceFileName` fails OPEN,
+ * and `deduplicateWorkflowName`'s lookups can throw outright. Either way the UPDATE raises
+ * 23505, and an uncaught 23505 here aborts the batch — precisely the permanent retention stall
+ * this whole hook exists to prevent. So any failure retries with the row id, which is unique by
+ * construction and cannot collide.
+ *
+ * A failure of that retry is swallowed too: one unfixable row must not stop the other children
+ * from being made safe. It is logged at error level because the folder's DELETE can then still
+ * stall on that row via the FK's SET NULL.
+ */
+async function reRootOne(
+  preferred: () => Promise<unknown>,
+  withUniqueName: () => Promise<unknown>,
+  subject: string,
+  label: string
+): Promise<void> {
+  try {
+    await preferred()
+    return
+  } catch (error) {
+    logger.warn(`[${label}] Re-rooting ${subject} under its deduplicated name failed; retrying`, {
+      error,
+    })
+  }
+
+  try {
+    await withUniqueName()
+  } catch (error) {
+    logger.error(`[${label}] Could not re-root ${subject}; its folder delete may stall`, { error })
+  }
+}
+
 /**
  * Re-roots any still-active workflow or workspace file filed under a folder that is about to be
  * hard-deleted, giving it a collision-free name first.
@@ -485,9 +521,24 @@ async function reRootActiveFolderChildren(
     .where(and(inArray(workflow.folderId, expiredIds), isNull(workflow.archivedAt)))
 
   for (const row of workflows) {
-    if (!row.workspaceId) continue
-    const name = await deduplicateWorkflowName(row.name, row.workspaceId, null, cleanupDb)
-    await cleanupDb.update(workflow).set({ folderId: null, name }).where(eq(workflow.id, row.id))
+    const workspaceId = row.workspaceId
+    if (!workspaceId) continue
+    await reRootOne(
+      async () => {
+        const name = await deduplicateWorkflowName(row.name, workspaceId, null, cleanupDb)
+        await cleanupDb
+          .update(workflow)
+          .set({ folderId: null, name })
+          .where(eq(workflow.id, row.id))
+      },
+      () =>
+        cleanupDb
+          .update(workflow)
+          .set({ folderId: null, name: `${row.name} (${row.id})` })
+          .where(eq(workflow.id, row.id)),
+      `workflow ${row.id}`,
+      label
+    )
   }
 
   const files = await cleanupDb
@@ -506,22 +557,28 @@ async function reRootActiveFolderChildren(
     )
 
   for (const row of files) {
-    if (!row.workspaceId) continue
-    /**
-     * `allocateUniqueWorkspaceFileName` throws once the copy-suffix range is exhausted. Falling
-     * back to the row id keeps the sweep running on a name that cannot collide — the opposite
-     * failure (letting it throw) is the stall this function exists to prevent.
-     */
-    let originalName: string
-    try {
-      originalName = await allocateUniqueWorkspaceFileName(row.workspaceId, row.originalName, null)
-    } catch {
-      originalName = `${row.originalName} (${row.id})`
-    }
-    await cleanupDb
-      .update(workspaceFiles)
-      .set({ folderId: null, originalName })
-      .where(eq(workspaceFiles.id, row.id))
+    const workspaceId = row.workspaceId
+    if (!workspaceId) continue
+    await reRootOne(
+      async () => {
+        const originalName = await allocateUniqueWorkspaceFileName(
+          workspaceId,
+          row.originalName,
+          null
+        )
+        await cleanupDb
+          .update(workspaceFiles)
+          .set({ folderId: null, originalName })
+          .where(eq(workspaceFiles.id, row.id))
+      },
+      () =>
+        cleanupDb
+          .update(workspaceFiles)
+          .set({ folderId: null, originalName: `${row.originalName} (${row.id})` })
+          .where(eq(workspaceFiles.id, row.id)),
+      `workspace file ${row.id}`,
+      label
+    )
   }
 
   if (workflows.length > 0 || files.length > 0) {