fix(folders): close workflow-folder restore lock race, align restore error codes

Three related restore-path bugs found by Greptile/Cursor this round:

1. performRestoreFolder (workflow folders) read the folder's own `locked`
   flag BEFORE opening the restore transaction. A folder locked in the
   window between that read and the transaction's write could still be
   resurrected and mutated. Restructured to match performRestoreResourceFolder
   (kb/table): the not_found/not_archived/locked checks and the restore
   writes all now happen inside one transaction.

2. Restoring an already-active (non-archived) workflow folder silently
   returned 200/success with zero restoredItems, while the same request for
   knowledge base, table, and file folders correctly returns a 400
   validation error. Now returns errorCode: 'validation' -> 400, consistent
   with the other three resource types.

3. performRestoreFileFolder's 'Folder not found' case returned no
   errorCode, so it fell through the route's default and mapped to 400
   instead of 404 (workflow/kb/table restores already mapped this
   correctly). Added errorCode: 'not_found' to that branch, plus
   errorCode: 'conflict'/'validation'/'internal' to the sibling branches
   for consistency with performCreateFileFolder's existing pattern, and
   stopped leaking the raw internal error message to the client on an
   unexpected failure.

Author: waleedlatif1

apps/sim/lib/folders/orchestration.ts

@@ -311,14 +311,18 @@ async function performRestoreFileFolder(
       return {
         success: false,
         error: 'A folder with this name already exists in this location',
+        errorCode: 'conflict',
       }
     }
     const message = toError(error).message
-    if (message === 'Folder not found' || message === 'Folder is not archived') {
-      return { success: false, error: message }
+    if (message === 'Folder not found') {
+      return { success: false, error: message, errorCode: 'not_found' }
+    }
+    if (message === 'Folder is not archived') {
+      return { success: false, error: message, errorCode: 'validation' }
     }
     logger.error('Failed to restore file folder', { error })
-    return { success: false, error: message }
+    return { success: false, error: 'Internal server error', errorCode: 'internal' }
   }
 }
 

apps/sim/lib/workflows/orchestration/folder-lifecycle.ts

@@ -474,32 +474,30 @@ export async function performRestoreFolder(
 ): Promise<PerformRestoreFolderResult> {
   const { folderId, workspaceId, userId, folderName } = params
 
-  const [existingFolder] = await db
-    .select()
-    .from(folder)
-    .where(and(eq(folder.id, folderId), eq(folder.workspaceId, workspaceId), isWorkflowFolder))
-
-  if (!existingFolder) {
-    return { success: false, error: 'Folder not found', errorCode: 'not_found' }
-  }
-
-  if (!existingFolder.deletedAt) {
-    return { success: true, restoredItems: { folders: 0, workflows: 0 } }
-  }
-
-  // The folder row is soft-deleted, so the generic lock engine (which only reads
-  // active rows) can't see it -- check its own `locked` flag directly.
-  if (existingFolder.locked) {
-    throw new ResourceLockedError('workflow', false, 'Folder is locked')
-  }
-
   const { getWorkspaceWithOwner } = await import('@/lib/workspaces/permissions/utils')
   const ws = await getWorkspaceWithOwner(workspaceId)
   if (!ws || ws.archivedAt) {
     return { success: false, error: 'Cannot restore folder into an archived workspace' }
   }
 
-  const restoredStats = await db.transaction(async (tx) => {
+  const outcome = await db.transaction(async (tx) => {
+    const [existingFolder] = await tx
+      .select()
+      .from(folder)
+      .where(and(eq(folder.id, folderId), eq(folder.workspaceId, workspaceId), isWorkflowFolder))
+
+    if (!existingFolder) return { kind: 'not_found' as const }
+    if (!existingFolder.deletedAt) return { kind: 'not_archived' as const }
+
+    // The folder row is soft-deleted, so the generic lock engine (which only reads
+    // active rows) can't see it -- check its own `locked` flag directly. This read
+    // must happen inside the same transaction as the restore write below (not before
+    // it opens): otherwise a concurrent lock landing between the read and the write
+    // could still resurrect and mutate a folder that is now locked.
+    if (existingFolder.locked) {
+      throw new ResourceLockedError('workflow', false, 'Folder is locked')
+    }
+
     let resolvedParentId = existingFolder.parentId
     if (resolvedParentId) {
       const [parentFolder] = await tx
@@ -520,26 +518,41 @@ export async function performRestoreFolder(
     // TOCTOU window where a concurrent request locks resolvedParentId in between.
     await assertFolderMutable(resolvedParentId, 'workflow', tx)
 
-    return restoreFolderRecursively(folderId, workspaceId, existingFolder.deletedAt!, tx)
+    const stats = await restoreFolderRecursively(
+      folderId,
+      workspaceId,
+      existingFolder.deletedAt,
+      tx
+    )
+    return { kind: 'ok' as const, stats, name: existingFolder.name }
   })
 
-  logger.info('Restored folder and all contents:', { folderId, restoredStats })
+  if (outcome.kind === 'not_found') {
+    return { success: false, error: 'Folder not found', errorCode: 'not_found' }
+  }
+  if (outcome.kind === 'not_archived') {
+    return { success: false, error: 'Folder is not archived', errorCode: 'validation' }
+  }
+
+  const { stats, name } = outcome
+
+  logger.info('Restored folder and all contents:', { folderId, restoredStats: stats })
 
   recordAudit({
     workspaceId,
     actorId: userId,
     action: AuditAction.FOLDER_RESTORED,
     resourceType: AuditResourceType.FOLDER,
     resourceId: folderId,
-    resourceName: folderName ?? existingFolder.name,
-    description: `Restored folder "${folderName ?? existingFolder.name}"`,
+    resourceName: folderName ?? name,
+    description: `Restored folder "${folderName ?? name}"`,
     metadata: {
       affected: {
-        workflows: restoredStats.workflows,
-        subfolders: restoredStats.folders - 1,
+        workflows: stats.workflows,
+        subfolders: stats.folders - 1,
       },
     },
   })
 
-  return { success: true, restoredItems: restoredStats }
+  return { success: true, restoredItems: stats }
 }