fix(folders): order the cascade so a partial failure stays recoverable

The hooks that walk resources one at a time through their canonical
delete/restore can fail partway. Both cascades were ordered so that a
mid-loop failure became permanent.

Delete stamped children before folders. A failure left some children
soft-deleted at T1 with the folder still active, so the retry minted a
fresh T2 and the T1 children could never be matched by any restore.
Folders are now stamped first, which makes the retry reuse that stamp and
sweep the stragglers into the original snapshot.

Restore had the mirror image: folders came back first, so a later child
failure left the root's deletedAt cleared and every retry short-circuited
on the already-restored early return, stranding whatever had not come
back. Hook-driven child restores now run before the folder transaction, so
a failure leaves the folder archived and the whole restore retryable.

Both orderings trade a transient window where the folder and its contents
disagree for a failure that heals on retry instead of silently losing data.

Also rethrow HttpError from the folder DELETE handler: deleteTable can
still raise a 423 TableLockedError if a lock lands between the subtree
guard and the archive, and the catch was flattening it to a 500.

Author: waleedlatif1

apps/sim/app/api/folders/[id]/route.ts

@@ -7,6 +7,7 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { deleteFolderContract, updateFolderContract } from '@/lib/api/contracts'
 import { parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
+import { HttpError } from '@/lib/core/utils/http-error'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle'
 import { toFolderApi } from '@/lib/folders/queries'
@@ -187,6 +188,11 @@ export const DELETE = withRouteHandler(
         return NextResponse.json({ error: error.message }, { status: error.status })
       }
 
+      // A typed domain error carries its own status — `deleteTable` can still raise a 423
+      // `TableLockedError` if a lock is set between the subtree guard and the archive.
+      // Rethrow so `withRouteHandler` maps it instead of flattening it to a 500.
+      if (error instanceof HttpError) throw error
+
       logger.error('Error deleting folder:', { error })
       return NextResponse.json({ error: 'Internal server error' }, { status: 500 })
     }

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

@@ -165,21 +165,39 @@ describe('collectArchivedSubtreeIds', () => {
 })
 
 describe('archiveFolderCascade', () => {
-  it('archives children then folders under one shared timestamp', async () => {
+  it('stamps folders before children, under one shared timestamp', async () => {
     const { tx, updateCalls } = makeTx({
       updates: [
-        [{ id: 'child-1' }, { id: 'child-2' }],
         [{ id: 'root' }, { id: 'sub' }],
+        [{ id: 'child-1' }, { id: 'child-2' }],
       ],
     })
 
     const counts = await archiveFolderCascade(tx, makeConfig(), 'ws-1', ['root', 'sub'], TIMESTAMP)
 
     expect(counts).toEqual({ folders: 2, children: 2 })
     expect(updateCalls).toHaveLength(2)
-    expect(updateCalls[0].table).toBe(CHILD_TABLE)
-    expect(updateCalls[0].set).toEqual({ archivedAt: TIMESTAMP, updatedAt: TIMESTAMP })
-    expect(updateCalls[1].set).toMatchObject({ deletedAt: TIMESTAMP })
+    // Order is load-bearing: the folder must carry the stamp before any child does, so a
+    // failed cascade can be retried onto the same snapshot instead of minting a new one.
+    expect(updateCalls[0].table).not.toBe(CHILD_TABLE)
+    expect(updateCalls[0].set).toMatchObject({ deletedAt: TIMESTAMP })
+    expect(updateCalls[1].table).toBe(CHILD_TABLE)
+    expect(updateCalls[1].set).toEqual({ archivedAt: TIMESTAMP, updatedAt: TIMESTAMP })
+  })
+
+  it('stamps the folder before invoking an archiveChildren hook', async () => {
+    const seenAtHookTime: number[] = []
+    const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }]] })
+    const archiveChildren = vi.fn().mockImplementation(async () => {
+      seenAtHookTime.push(updateCalls.length)
+      return 3
+    })
+
+    await archiveFolderCascade(tx, makeConfig({ archiveChildren }), 'ws-1', ['root'], TIMESTAMP)
+
+    // The hook walks resources one at a time and can fail partway, so the folder stamp must
+    // already be in place by then for the retry to reuse it.
+    expect(seenAtHookTime).toEqual([1])
   })
 
   it('skips rows that were already soft-deleted independently', async () => {
@@ -192,16 +210,16 @@ describe('archiveFolderCascade', () => {
     }
   })
 
-  it('stamps children with the timestamp it is given, not one of its own', async () => {
-    const { tx, updateCalls } = makeTx({ updates: [[{ id: 'child-1' }], []] })
+  it('stamps every row with the timestamp it is given, not one of its own', async () => {
+    const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }], [{ id: 'child-1' }]] })
 
     await archiveFolderCascade(tx, makeConfig(), 'ws-1', ['root'], TIMESTAMP)
 
     // Regression guard: deleting an already-archived folder reuses that folder's existing
     // deletedAt. A fresh stamp here would strand the children — the folder row keeps its
     // original stamp, so restore would never match them.
-    expect(updateCalls[0].set).toEqual({ archivedAt: TIMESTAMP, updatedAt: TIMESTAMP })
-    expect(updateCalls[1].set).toMatchObject({ deletedAt: TIMESTAMP })
+    expect(updateCalls[0].set).toMatchObject({ deletedAt: TIMESTAMP })
+    expect(updateCalls[1].set).toEqual({ archivedAt: TIMESTAMP, updatedAt: TIMESTAMP })
   })
 
   it('delegates to archiveChildren when a resource archives through its own lifecycle', async () => {

apps/sim/lib/folders/cascade.ts

@@ -80,12 +80,21 @@ function childFilter(config: FolderResourceConfig, workspaceId: string, folderId
  * Soft-deletes every folder in `folderIds` and every resource contained by them, stamping
  * one shared `timestamp` across the whole cascade.
  *
- * The shared timestamp is load-bearing: {@link restoreFolderCascade} resurrects only rows
- * whose soft-delete timestamp matches the folder's exactly, which is what stops a restore
- * from also reviving siblings that were deleted independently.
+ * The shared timestamp is load-bearing: the restore path resurrects only rows whose
+ * soft-delete timestamp matches the folder's exactly, which is what stops a restore from
+ * also reviving siblings that were deleted independently.
  *
- * Children are archived before folders so a concurrent reader never sees an active
- * resource inside a folder that has already vanished from the tree.
+ * Folders are stamped BEFORE their children, which is what makes a failed cascade
+ * recoverable. `archiveChildren` hooks walk resources one at a time through their canonical
+ * delete, so a mid-loop failure can leave some children archived and some not. With the
+ * folder already stamped, `deleteFolder` reuses that same `deletedAt` on the retry and the
+ * stragglers join the original snapshot. Stamping children first would leave the folder
+ * active, so a retry would mint a fresh timestamp and the partially-archived children could
+ * never be matched by any restore again.
+ *
+ * The cost is a window where the folder reads as deleted while a resource inside it is still
+ * active. That is the strictly better failure: it is transient and self-healing on retry,
+ * where the alternative silently and permanently strands data.
  */
 export async function archiveFolderCascade(
   tx: DbOrTx,
@@ -94,16 +103,6 @@ export async function archiveFolderCascade(
   folderIds: string[],
   timestamp: Date
 ): Promise<FolderCascadeCounts> {
-  const children = config.archiveChildren
-    ? await config.archiveChildren({ workspaceId, folderIds, timestamp })
-    : (
-        await tx
-          .update(config.table)
-          .set(config.buildSoftDeleteSet(timestamp, timestamp))
-          .where(and(childFilter(config, workspaceId, folderIds), isNull(config.deletedColumn)))
-          .returning({ id: config.idColumn })
-      ).length
-
   const archivedFolders = await tx
     .update(folderTable)
     .set({ deletedAt: timestamp, updatedAt: timestamp })
@@ -117,6 +116,16 @@ export async function archiveFolderCascade(
     )
     .returning({ id: folderTable.id })
 
+  const children = config.archiveChildren
+    ? await config.archiveChildren({ workspaceId, folderIds, timestamp })
+    : (
+        await tx
+          .update(config.table)
+          .set(config.buildSoftDeleteSet(timestamp, timestamp))
+          .where(and(childFilter(config, workspaceId, folderIds), isNull(config.deletedColumn)))
+          .returning({ id: config.idColumn })
+      ).length
+
   return { folders: archivedFolders.length, children }
 }
 

apps/sim/lib/folders/lifecycle.ts

@@ -411,9 +411,29 @@ export async function restoreFolder(params: RestoreFolderParams): Promise<Restor
     }
   }
 
-  let restored: { folderIds: string[]; counts: { folders: number; children: number } }
+  const folderIds = await collectArchivedSubtreeIds(
+    db,
+    workspaceId,
+    resourceType,
+    folderId,
+    archivedAt
+  )
+
+  // Resources with a `restoreChildren` hook come back BEFORE the folder rows. Those hooks
+  // call canonical restores that open their own transactions, so they cannot run inside the
+  // one below — and doing them first is what keeps a partial failure recoverable. Restoring
+  // folders first would clear the root's `deletedAt`, so a later child failure would
+  // short-circuit every retry on the `!archivedAt` early return above, stranding whatever
+  // had not come back yet. In this order a failure leaves the folder archived and the whole
+  // restore simply retryable; children already restored no longer match the timestamp and
+  // are skipped.
+  const hookChildren = config.restoreChildren
+    ? await config.restoreChildren({ workspaceId, folderIds, timestamp: archivedAt })
+    : null
+
+  let counts: { folders: number; children: number }
   try {
-    restored = await db.transaction(async (tx) => {
+    counts = await db.transaction(async (tx) => {
       const now = new Date()
 
       let resolvedParentId = folder.parentId
@@ -455,25 +475,13 @@ export async function restoreFolder(params: RestoreFolderParams): Promise<Restor
         await tx.update(folderTable).set({ name: restoredName }).where(eq(folderTable.id, folderId))
       }
 
-      const folderIds = await collectArchivedSubtreeIds(
-        tx,
-        workspaceId,
-        resourceType,
-        folderId,
-        archivedAt
-      )
-
       const folders = await restoreFolderRows(tx, config, workspaceId, folderIds, archivedAt, now)
 
-      // A resource with a `restoreChildren` hook restores through its own canonical path,
-      // which opens its own transactions — running that nested inside this one would hold a
-      // folder-row transaction open across unrelated connections. Those run after the commit
-      // below instead; the default row-update path stays inside the transaction.
-      const children = config.restoreChildren
-        ? 0
-        : await restoreFolderChildren(tx, config, workspaceId, folderIds, archivedAt, now)
+      const children =
+        hookChildren ??
+        (await restoreFolderChildren(tx, config, workspaceId, folderIds, archivedAt, now))
 
-      return { folderIds, counts: { folders, children } }
+      return { folders, children }
     })
   } catch (error) {
     // Clearing `deletedAt` brings the row back under the partial unique index on active
@@ -485,17 +493,6 @@ export async function restoreFolder(params: RestoreFolderParams): Promise<Restor
     throw error
   }
 
-  const counts = config.restoreChildren
-    ? {
-        folders: restored.counts.folders,
-        children: await config.restoreChildren({
-          workspaceId,
-          folderIds: restored.folderIds,
-          timestamp: archivedAt,
-        }),
-      }
-    : restored.counts
-
   logger.info('Restored folder and all contents', { folderId, resourceType, counts })
 
   recordAudit({