From e9d560ad14c1b992b50a791307a5461550523173 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 18:08:26 -0700 Subject: [PATCH 1/5] feat(folders): add the generic resourceType-driven folder engine Folder create/update/delete/restore/reorder existed once per resource type in intent but only once in code, hardcoded to workflows. Extract a config-driven engine so knowledge bases and tables get folders from the same implementation instead of a fourth copy. - lib/folders/config.ts declares FOLDER_RESOURCES: the child table, its id/folderId/workspace/soft-delete columns, the TS property key backing the soft-delete column, membership scope, audit label, and cascade count key. Workflow-only behavior (archiving through the workflow lifecycle, the last-workflow delete guard, restoring schedules and webhooks) is declared as data on the workflow entry, not branched on. - lib/folders/cascade.ts performs archive/restore under one shared timestamp, in a fixed number of statements regardless of subtree depth. - lib/folders/lifecycle.ts is the single create/update/delete/restore implementation; the workflow perform* functions become thin wrappers so existing callers are untouched. - Widen the folder contract to workflow | knowledge_base | table and thread resourceType through all four routes and the React Query hooks, including the query keys so the three folder trees stop sharing a cache entry. - Move the folder cycle check out of lib/workflows/utils.ts: it was hardcoded to resourceType 'workflow' and could walk out of one resource's tree into another's. --- .../sim/app/api/folders/[id]/restore/route.ts | 20 +- apps/sim/app/api/folders/[id]/route.test.ts | 44 +- apps/sim/app/api/folders/[id]/route.ts | 45 +- .../sim/app/api/folders/reorder/route.test.ts | 27 + apps/sim/app/api/folders/reorder/route.ts | 37 +- apps/sim/app/api/folders/route.test.ts | 3 +- apps/sim/app/api/folders/route.ts | 21 +- apps/sim/background/cleanup-soft-deletes.ts | 11 +- apps/sim/hooks/queries/folders.ts | 143 +++-- apps/sim/hooks/queries/utils/folder-cache.ts | 19 +- apps/sim/hooks/queries/utils/folder-keys.ts | 19 +- apps/sim/lib/api/contracts/folders.ts | 40 +- apps/sim/lib/folders/cascade.test.ts | 358 +++++++++++ apps/sim/lib/folders/cascade.ts | 183 ++++++ apps/sim/lib/folders/config.ts | 304 +++++++++ apps/sim/lib/folders/lifecycle.ts | 469 ++++++++++++++ apps/sim/lib/folders/naming.ts | 3 +- apps/sim/lib/folders/queries.ts | 29 + apps/sim/lib/posthog/events.ts | 3 + .../orchestration/folder-lifecycle.ts | 599 +----------------- apps/sim/lib/workflows/utils.ts | 30 - .../src/mocks/folders-lifecycle.mock.ts | 35 + packages/testing/src/mocks/index.ts | 2 + .../testing/src/mocks/workflows-utils.mock.ts | 2 - 24 files changed, 1721 insertions(+), 725 deletions(-) create mode 100644 apps/sim/lib/folders/cascade.test.ts create mode 100644 apps/sim/lib/folders/cascade.ts create mode 100644 apps/sim/lib/folders/config.ts create mode 100644 apps/sim/lib/folders/lifecycle.ts create mode 100644 packages/testing/src/mocks/folders-lifecycle.mock.ts diff --git a/apps/sim/app/api/folders/[id]/restore/route.ts b/apps/sim/app/api/folders/[id]/restore/route.ts index 5ad28b90b28..9f0331e86cb 100644 --- a/apps/sim/app/api/folders/[id]/restore/route.ts +++ b/apps/sim/app/api/folders/[id]/restore/route.ts @@ -5,8 +5,9 @@ import { restoreFolderContract } from '@/lib/api/contracts' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { restoreFolder } from '@/lib/folders/lifecycle' import { captureServerEvent } from '@/lib/posthog/server' -import { performRestoreFolder } from '@/lib/workflows/orchestration/folder-lifecycle' +import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('RestoreFolderAPI') @@ -23,29 +24,36 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route const parsed = await parseRequest(restoreFolderContract, request, context) if (!parsed.success) return parsed.response const { id: folderId } = parsed.data.params - const { workspaceId } = parsed.data.body + const { workspaceId, resourceType } = parsed.data.body const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) if (permission !== 'admin' && permission !== 'write') { return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 }) } - const result = await performRestoreFolder({ + const result = await restoreFolder({ + resourceType, folderId, workspaceId, userId: session.user.id, }) if (!result.success) { - return NextResponse.json({ error: result.error }, { status: 400 }) + return NextResponse.json( + { error: result.error }, + { status: statusForOrchestrationError(result.errorCode) } + ) } - logger.info(`Restored folder ${folderId}`, { restoredItems: result.restoredItems }) + logger.info(`Restored folder ${folderId}`, { + resourceType, + restoredItems: result.restoredItems, + }) captureServerEvent( session.user.id, 'folder_restored', - { folder_id: folderId, workspace_id: workspaceId }, + { folder_id: folderId, workspace_id: workspaceId, resource_type: resourceType }, { groups: { workspace: workspaceId } } ) diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index d636429248b..7d53a2e62f7 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -8,16 +8,14 @@ import { authMockFns, createMockRequest, dbChainMockFns, + foldersLifecycleMock, + foldersLifecycleMockFns, type MockUser, permissionsMock, permissionsMockFns, queueTableRows, resetDbChainMock, schemaMock, - workflowsOrchestrationMock, - workflowsOrchestrationMockFns, - workflowsUtilsMock, - workflowsUtilsMockFns, } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' @@ -36,8 +34,11 @@ const { mockLogger } = vi.hoisted(() => { } }) -const mockPerformDeleteFolder = workflowsOrchestrationMockFns.mockPerformDeleteFolder -const mockPerformUpdateFolder = workflowsOrchestrationMockFns.mockPerformUpdateFolder +const mockDeleteFolder = foldersLifecycleMockFns.mockDeleteFolder +const mockUpdateFolder = foldersLifecycleMockFns.mockUpdateFolder + +/** Parent ids the mocked engine treats as closing a cycle for the folder under test. */ +const cyclicParentIds = new Set() const mockGetUserEntityPermissions = permissionsMockFns.mockGetUserEntityPermissions @@ -48,8 +49,7 @@ vi.mock('@sim/logger', () => ({ getRequestContext: () => undefined, })) vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock) -vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock) -vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock) +vi.mock('@/lib/folders/lifecycle', () => foldersLifecycleMock) import { DELETE, PUT } from '@/app/api/folders/[id]/route' @@ -101,11 +101,11 @@ describe('Individual Folder API Route', () => { resetDbChainMock() mockGetUserEntityPermissions.mockResolvedValue('admin') - mockPerformDeleteFolder.mockResolvedValue({ + mockDeleteFolder.mockResolvedValue({ success: true, deletedItems: { folders: 1, workflows: 0 }, }) - mockPerformUpdateFolder.mockImplementation(async (params) => { + mockUpdateFolder.mockImplementation(async (params) => { if (params.parentId && params.parentId === params.folderId) { return { success: false, @@ -113,13 +113,7 @@ describe('Individual Folder API Route', () => { errorCode: 'validation', } } - if ( - params.parentId && - (await workflowsUtilsMockFns.mockCheckForCircularReference( - params.folderId, - params.parentId - )) - ) { + if (params.parentId && cyclicParentIds.has(params.parentId)) { return { success: false, error: 'Cannot create circular folder reference', @@ -140,7 +134,7 @@ describe('Individual Folder API Route', () => { }, } }) - workflowsUtilsMockFns.mockCheckForCircularReference.mockResolvedValue(false) + cyclicParentIds.clear() }) describe('PUT /api/folders/[id]', () => { @@ -368,7 +362,7 @@ describe('Individual Folder API Route', () => { workspaceId: 'workspace-123', }) - workflowsUtilsMockFns.mockCheckForCircularReference.mockResolvedValue(true) + cyclicParentIds.add('folder-1') const req = createMockRequest('PUT', { name: 'Updated Folder 3', @@ -382,9 +376,8 @@ describe('Individual Folder API Route', () => { const data = await response.json() expect(data).toHaveProperty('error', 'Cannot create circular folder reference') - expect(workflowsUtilsMockFns.mockCheckForCircularReference).toHaveBeenCalledWith( - 'folder-3', - 'folder-1' + expect(mockUpdateFolder).toHaveBeenCalledWith( + expect.objectContaining({ folderId: 'folder-3', parentId: 'folder-1' }) ) }) }) @@ -405,7 +398,8 @@ describe('Individual Folder API Route', () => { const data = await response.json() expect(data).toHaveProperty('success', true) expect(data).toHaveProperty('deletedItems') - expect(mockPerformDeleteFolder).toHaveBeenCalledWith({ + expect(mockDeleteFolder).toHaveBeenCalledWith({ + resourceType: 'workflow', folderId: 'folder-1', workspaceId: 'workspace-123', userId: TEST_USER.id, @@ -458,7 +452,7 @@ describe('Individual Folder API Route', () => { const data = await response.json() expect(data).toHaveProperty('success', true) - expect(mockPerformDeleteFolder).toHaveBeenCalled() + expect(mockDeleteFolder).toHaveBeenCalled() }) it('should allow folder deletion for admin permissions', async () => { @@ -476,7 +470,7 @@ describe('Individual Folder API Route', () => { const data = await response.json() expect(data).toHaveProperty('success', true) - expect(mockPerformDeleteFolder).toHaveBeenCalled() + expect(mockDeleteFolder).toHaveBeenCalled() }) it('should handle database errors during deletion', async () => { diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index 60753cb0685..5ca48a240b9 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -4,13 +4,13 @@ import { createLogger } from '@sim/logger' import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow' import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' -import { updateFolderContract } from '@/lib/api/contracts' +import { deleteFolderContract, updateFolderContract } from '@/lib/api/contracts' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle' import { toFolderApi } from '@/lib/folders/queries' import { captureServerEvent } from '@/lib/posthog/server' -import { performDeleteFolder, performUpdateFolder } from '@/lib/workflows/orchestration' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' /** Maps an orchestration errorCode to its HTTP status; mirrors the POST /api/folders route. */ @@ -47,13 +47,13 @@ export const PUT = withRouteHandler( if (!parsed.success) return parsed.response const { id } = parsed.data.params + const { resourceType } = parsed.data.query const { name, locked, parentId, sortOrder } = parsed.data.body - // Verify the folder exists const existingFolder = await db .select() .from(folderTable) - .where(and(eq(folderTable.id, id), eq(folderTable.resourceType, 'workflow'))) + .where(and(eq(folderTable.id, id), eq(folderTable.resourceType, resourceType))) .then((rows) => rows[0]) if (!existingFolder) { @@ -81,15 +81,19 @@ export const PUT = withRouteHandler( ) } - const hasNonLockUpdate = Object.keys(parsed.data.body).some((key) => key !== 'locked') - if (hasNonLockUpdate) { - await assertFolderMutable(id) - } - if (parentId !== undefined) { - await assertFolderMutable(parentId) + // Folder locking is a workflow-only feature; other resource types leave `locked` false. + if (resourceType === 'workflow') { + const hasNonLockUpdate = Object.keys(parsed.data.body).some((key) => key !== 'locked') + if (hasNonLockUpdate) { + await assertFolderMutable(id) + } + if (parentId !== undefined) { + await assertFolderMutable(parentId) + } } - const result = await performUpdateFolder({ + const result = await updateFolder({ + resourceType, folderId: id, workspaceId: existingFolder.workspaceId, userId: session.user.id, @@ -120,20 +124,22 @@ export const PUT = withRouteHandler( // DELETE - Delete a folder and all its contents export const DELETE = withRouteHandler( - async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => { + async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { try { const session = await getSession() if (!session?.user?.id) { return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) } - const { id } = await params + const parsed = await parseRequest(deleteFolderContract, request, context) + if (!parsed.success) return parsed.response + const { id } = parsed.data.params + const { resourceType } = parsed.data.query - // Verify the folder exists const existingFolder = await db .select() .from(folderTable) - .where(and(eq(folderTable.id, id), eq(folderTable.resourceType, 'workflow'))) + .where(and(eq(folderTable.id, id), eq(folderTable.resourceType, resourceType))) .then((rows) => rows[0]) if (!existingFolder) { @@ -153,9 +159,12 @@ export const DELETE = withRouteHandler( ) } - await assertFolderMutable(id) + if (resourceType === 'workflow') { + await assertFolderMutable(id) + } - const result = await performDeleteFolder({ + const result = await deleteFolder({ + resourceType, folderId: id, workspaceId: existingFolder.workspaceId, userId: session.user.id, @@ -171,7 +180,7 @@ export const DELETE = withRouteHandler( captureServerEvent( session.user.id, 'folder_deleted', - { workspace_id: existingFolder.workspaceId }, + { workspace_id: existingFolder.workspaceId, resource_type: resourceType }, { groups: { workspace: existingFolder.workspaceId } } ) diff --git a/apps/sim/app/api/folders/reorder/route.test.ts b/apps/sim/app/api/folders/reorder/route.test.ts index c396bd5e46d..869aaa89f73 100644 --- a/apps/sim/app/api/folders/reorder/route.test.ts +++ b/apps/sim/app/api/folders/reorder/route.test.ts @@ -66,6 +66,33 @@ describe('PUT /api/folders/reorder', () => { expect(data).toMatchObject({ success: true, updated: 1 }) }) + it('maps a sibling-name collision from a reparent to a 409', async () => { + mockWhere + .mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }]) + .mockReturnValueOnce([{ id: 'parent-1', workspaceId: 'workspace-123', archivedAt: null }]) + .mockReturnValueOnce([ + { id: 'folder-1', parentId: null }, + { id: 'parent-1', parentId: null }, + ]) + + const uniqueViolation = Object.assign(new Error('duplicate key value'), { code: '23505' }) + mockDb.transaction.mockImplementationOnce(async () => { + throw uniqueViolation + }) + + const req = createMockRequest('PUT', { + workspaceId: 'workspace-123', + resourceType: 'knowledge_base', + updates: [{ id: 'folder-1', sortOrder: 0, parentId: 'parent-1' }], + }) + + const response = await PUT(req) + + expect(response.status).toBe(409) + const data = await response.json() + expect(data.error).toBe('A folder with this name already exists in this location') + }) + it('rejects a parentId that belongs to another workspace', async () => { mockWhere .mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }]) diff --git a/apps/sim/app/api/folders/reorder/route.ts b/apps/sim/app/api/folders/reorder/route.ts index d734cea4047..84306aa5588 100644 --- a/apps/sim/app/api/folders/reorder/route.ts +++ b/apps/sim/app/api/folders/reorder/route.ts @@ -2,6 +2,7 @@ import { db } from '@sim/db' import { folder as folderTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow' +import { getPostgresErrorCode } from '@sim/utils/errors' import { and, eq, inArray } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { reorderFoldersContract } from '@/lib/api/contracts' @@ -25,7 +26,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { try { const parsed = await parseRequest(reorderFoldersContract, req, {}) if (!parsed.success) return parsed.response - const { workspaceId, updates } = parsed.data.body + const { workspaceId, resourceType, updates } = parsed.data.body const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId) if (!permission || permission === 'read') { @@ -39,7 +40,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { const existingFolders = await db .select({ id: folderTable.id, workspaceId: folderTable.workspaceId }) .from(folderTable) - .where(and(inArray(folderTable.id, folderIds), eq(folderTable.resourceType, 'workflow'))) + .where(and(inArray(folderTable.id, folderIds), eq(folderTable.resourceType, resourceType))) const validIds = new Set( existingFolders.filter((f) => f.workspaceId === workspaceId).map((f) => f.id) @@ -64,7 +65,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { }) .from(folderTable) .where( - and(inArray(folderTable.id, targetParentIds), eq(folderTable.resourceType, 'workflow')) + and(inArray(folderTable.id, targetParentIds), eq(folderTable.resourceType, resourceType)) ) const validParentIds = new Set( @@ -86,7 +87,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { .select({ id: folderTable.id, parentId: folderTable.parentId }) .from(folderTable) .where( - and(eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, 'workflow')) + and(eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, resourceType)) ) const parentById = new Map() @@ -114,16 +115,19 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { } } - for (const update of validUpdates) { - await assertFolderMutable(update.id) - if (update.parentId !== undefined) { - await assertFolderMutable(update.parentId) + // Folder locking is a workflow-only feature; other resource types leave `locked` false. + if (resourceType === 'workflow') { + for (const update of validUpdates) { + await assertFolderMutable(update.id) + if (update.parentId !== undefined) { + await assertFolderMutable(update.parentId) + } } } await db.transaction(async (tx) => { for (const update of validUpdates) { - const updateData: Record = { + const updateData: Partial = { sortOrder: update.sortOrder, updatedAt: new Date(), } @@ -133,12 +137,12 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { await tx .update(folderTable) .set(updateData) - .where(and(eq(folderTable.id, update.id), eq(folderTable.resourceType, 'workflow'))) + .where(and(eq(folderTable.id, update.id), eq(folderTable.resourceType, resourceType))) } }) logger.info( - `[${requestId}] Reordered ${validUpdates.length} folders in workspace ${workspaceId}` + `[${requestId}] Reordered ${validUpdates.length} ${resourceType} folders in workspace ${workspaceId}` ) return NextResponse.json({ success: true, updated: validUpdates.length }) @@ -147,6 +151,17 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { return NextResponse.json({ error: error.message }, { status: error.status }) } + // Reorder can reparent, which moves a folder into a new sibling set and brings it under + // the partial unique index on active (workspaceId, resourceType, parent, name). The user + // picked both the name and the destination here, so this is a conflict to surface, not a + // name to silently deduplicate. + if (getPostgresErrorCode(error) === '23505') { + return NextResponse.json( + { error: 'A folder with this name already exists in this location' }, + { status: 409 } + ) + } + logger.error(`[${requestId}] Error reordering folders`, error) return NextResponse.json({ error: 'Failed to reorder folders' }, { status: 500 }) } diff --git a/apps/sim/app/api/folders/route.test.ts b/apps/sim/app/api/folders/route.test.ts index 35dd03c15c0..bc6e72de1f0 100644 --- a/apps/sim/app/api/folders/route.test.ts +++ b/apps/sim/app/api/folders/route.test.ts @@ -545,8 +545,9 @@ describe('Folders API Route', () => { const data = await response.json() expect(data).toHaveProperty('error', 'Internal server error') - expect(mockLogger.error).toHaveBeenCalledWith('Failed to create workflow folder', { + expect(mockLogger.error).toHaveBeenCalledWith('Failed to create folder', { error: expect.any(Error), + resourceType: 'workflow', }) }) diff --git a/apps/sim/app/api/folders/route.ts b/apps/sim/app/api/folders/route.ts index c6a493dcfdc..3d8852eb67a 100644 --- a/apps/sim/app/api/folders/route.ts +++ b/apps/sim/app/api/folders/route.ts @@ -5,9 +5,9 @@ import { createFolderContract, listFoldersContract } from '@/lib/api/contracts' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { createFolder } from '@/lib/folders/lifecycle' import { listFoldersForWorkspace, toFolderApi } from '@/lib/folders/queries' import { captureServerEvent } from '@/lib/posthog/server' -import { performCreateFolder } from '@/lib/workflows/orchestration' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('FoldersAPI') @@ -64,6 +64,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const { id: clientId, name, + resourceType, workspaceId, parentId, sortOrder: providedSortOrder, @@ -82,10 +83,14 @@ export const POST = withRouteHandler(async (request: NextRequest) => { ) } - await assertFolderMutable(parentId ?? null) + // Folder locking is a workflow-only feature; other resource types leave `locked` false. + if (resourceType === 'workflow') { + await assertFolderMutable(parentId ?? null) + } - const result = await performCreateFolder({ + const result = await createFolder({ id: clientId, + resourceType, userId: session.user.id, workspaceId, name, @@ -102,12 +107,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const newFolder = result.folder - logger.info('Created new folder:', { id: newFolder.id, name, workspaceId, parentId }) + logger.info('Created new folder', { + id: newFolder.id, + name, + resourceType, + workspaceId, + parentId, + }) captureServerEvent( session.user.id, 'folder_created', - { workspace_id: workspaceId }, + { workspace_id: workspaceId, resource_type: resourceType }, { groups: { workspace: workspaceId } } ) diff --git a/apps/sim/background/cleanup-soft-deletes.ts b/apps/sim/background/cleanup-soft-deletes.ts index 4ec2b4e02dc..d486195cb5e 100644 --- a/apps/sim/background/cleanup-soft-deletes.ts +++ b/apps/sim/background/cleanup-soft-deletes.ts @@ -416,10 +416,13 @@ const CLEANUP_TARGETS = [ table: folderTable, softDeleteCol: folderTable.deletedAt, wsCol: folderTable.workspaceId, - // `folder` is shared by all four resource types. Only workflow folders are cut over to - // it; file/knowledge_base/table rows are still owned elsewhere, so this pass must not - // hard-delete them. - additionalPredicate: eq(folderTable.resourceType, 'workflow'), + /** + * `folder` is shared by all four resource types, but file folders are still written to + * `workspace_file_folders` and this pass must not hard-delete rows it does not own. One + * widened predicate rather than a target per type: same table, same soft-delete column, + * same workspace scoping — splitting it would only multiply the batched scans. + */ + additionalPredicate: inArray(folderTable.resourceType, ['workflow', 'knowledge_base', 'table']), name: 'folder', }, { diff --git a/apps/sim/hooks/queries/folders.ts b/apps/sim/hooks/queries/folders.ts index 12ab17b2877..c82f65416cc 100644 --- a/apps/sim/hooks/queries/folders.ts +++ b/apps/sim/hooks/queries/folders.ts @@ -10,6 +10,7 @@ import { listFoldersContract, reorderFoldersContract, restoreFolderContract, + type ServedFolderResourceType, updateFolderContract, } from '@/lib/api/contracts' import { getFolderMap } from '@/hooks/queries/utils/folder-cache' @@ -52,10 +53,11 @@ export function mapFolder(folder: FolderApi): WorkflowFolder { async function fetchFolders( workspaceId: string, scope: FolderQueryScope = 'active', + resourceType: ServedFolderResourceType = 'workflow', signal?: AbortSignal ): Promise { const { folders } = await requestJson(listFoldersContract, { - query: { workspaceId, scope }, + query: { workspaceId, scope, resourceType }, signal, }) return folders.map(mapFolder) @@ -63,12 +65,17 @@ async function fetchFolders( export function useFolders( workspaceId?: string, - options?: { scope?: FolderQueryScope; enabled?: boolean } + options?: { + scope?: FolderQueryScope + enabled?: boolean + resourceType?: ServedFolderResourceType + } ) { const scope = options?.scope ?? 'active' + const resourceType = options?.resourceType ?? 'workflow' return useQuery({ - queryKey: folderKeys.list(workspaceId, scope), - queryFn: ({ signal }) => fetchFolders(workspaceId as string, scope, signal), + queryKey: folderKeys.list(workspaceId, scope, resourceType), + queryFn: ({ signal }) => fetchFolders(workspaceId as string, scope, resourceType, signal), enabled: Boolean(workspaceId) && (options?.enabled ?? true), placeholderData: keepPreviousData, staleTime: FOLDER_LIST_STALE_TIME, @@ -78,10 +85,13 @@ export function useFolders( const selectFolderMap = (folders: WorkflowFolder[]): Record => Object.fromEntries(folders.map((folder) => [folder.id, folder])) -export function useFolderMap(workspaceId?: string) { +export function useFolderMap( + workspaceId?: string, + resourceType: ServedFolderResourceType = 'workflow' +) { return useQuery({ - queryKey: folderKeys.list(workspaceId), - queryFn: ({ signal }) => fetchFolders(workspaceId as string, 'active', signal), + queryKey: folderKeys.list(workspaceId, 'active', resourceType), + queryFn: ({ signal }) => fetchFolders(workspaceId as string, 'active', resourceType, signal), enabled: Boolean(workspaceId), placeholderData: keepPreviousData, staleTime: FOLDER_LIST_STALE_TIME, @@ -91,6 +101,7 @@ export function useFolderMap(workspaceId?: string) { interface CreateFolderVariables { workspaceId: string + resourceType?: ServedFolderResourceType name: string parentId?: string sortOrder?: number @@ -99,12 +110,14 @@ interface CreateFolderVariables { interface UpdateFolderVariables { workspaceId: string + resourceType?: ServedFolderResourceType id: string updates: Partial> } interface DeleteFolderVariables { workspaceId: string + resourceType?: ServedFolderResourceType id: string } @@ -119,7 +132,9 @@ interface DuplicateFolderVariables { /** * Creates optimistic mutation handlers for folder operations */ -function createFolderMutationHandlers( +function createFolderMutationHandlers< + TVariables extends { workspaceId: string; resourceType?: ServedFolderResourceType }, +>( queryClient: ReturnType, name: string, createOptimisticFolder: ( @@ -131,26 +146,36 @@ function createFolderMutationHandlers(queryClient, { name, - getQueryKey: (variables) => folderKeys.list(variables.workspaceId), - getSnapshot: (variables) => ({ ...getFolderMap(variables.workspaceId) }), + getQueryKey: (variables) => + folderKeys.list(variables.workspaceId, 'active', variables.resourceType ?? 'workflow'), + getSnapshot: (variables) => ({ + ...getFolderMap(variables.workspaceId, variables.resourceType ?? 'workflow'), + }), generateTempId: customGenerateTempId ?? (() => generateTempId('temp-folder')), createOptimisticItem: (variables, tempId) => { - const previousFolders = getFolderMap(variables.workspaceId) + const previousFolders = getFolderMap( + variables.workspaceId, + variables.resourceType ?? 'workflow' + ) return createOptimisticFolder(variables, tempId, previousFolders) }, applyOptimisticUpdate: (tempId, item) => { - queryClient.setQueryData(folderKeys.list(item.workspaceId), (old) => [ - ...(old ?? []), - item, - ]) + queryClient.setQueryData( + folderKeys.list(item.workspaceId, 'active', item.resourceType), + (old) => [...(old ?? []), item] + ) }, replaceOptimisticEntry: (tempId, data) => { - queryClient.setQueryData(folderKeys.list(data.workspaceId), (old) => - (old ?? []).map((folder) => (folder.id === tempId ? data : folder)) + queryClient.setQueryData( + folderKeys.list(data.workspaceId, 'active', data.resourceType), + (old) => (old ?? []).map((folder) => (folder.id === tempId ? data : folder)) ) }, rollback: (snapshot, variables) => { - queryClient.setQueryData(folderKeys.list(variables.workspaceId), Object.values(snapshot)) + queryClient.setQueryData( + folderKeys.list(variables.workspaceId, 'active', variables.resourceType ?? 'workflow'), + Object.values(snapshot) + ) }, }) } @@ -172,7 +197,7 @@ export function useCreateFolder() { userId: '', workspaceId: variables.workspaceId, parentId: variables.parentId || null, - resourceType: 'workflow' as const, + resourceType: variables.resourceType ?? 'workflow', locked: false, sortOrder: variables.sortOrder ?? @@ -191,9 +216,14 @@ export function useCreateFolder() { ) return useMutation({ - mutationFn: async ({ workspaceId, sortOrder, ...payload }: CreateFolderVariables) => { + mutationFn: async ({ + workspaceId, + sortOrder, + resourceType = 'workflow', + ...payload + }: CreateFolderVariables) => { const { folder } = await requestJson(createFolderContract, { - body: { ...payload, workspaceId, sortOrder }, + body: { ...payload, workspaceId, sortOrder, resourceType }, }) return mapFolder(folder) }, @@ -205,15 +235,23 @@ export function useUpdateFolder() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ workspaceId, id, updates }: UpdateFolderVariables) => { + mutationFn: async ({ + workspaceId: _workspaceId, + resourceType = 'workflow', + id, + updates, + }: UpdateFolderVariables) => { const { folder } = await requestJson(updateFolderContract, { params: { id }, + query: { resourceType }, body: updates, }) return mapFolder(folder) }, onSettled: (_data, _error, variables) => { - queryClient.invalidateQueries({ queryKey: folderKeys.list(variables.workspaceId) }) + queryClient.invalidateQueries({ + queryKey: folderKeys.resource(variables.resourceType ?? 'workflow'), + }) }, }) } @@ -222,11 +260,17 @@ export function useDeleteFolderMutation() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ workspaceId: _workspaceId, id }: DeleteFolderVariables) => { - return requestJson(deleteFolderContract, { params: { id } }) + mutationFn: async ({ + workspaceId: _workspaceId, + resourceType = 'workflow', + id, + }: DeleteFolderVariables) => { + return requestJson(deleteFolderContract, { params: { id }, query: { resourceType } }) }, onSettled: (_data, _error, variables) => { - queryClient.invalidateQueries({ queryKey: folderKeys.lists() }) + const resourceType = variables.resourceType ?? 'workflow' + queryClient.invalidateQueries({ queryKey: folderKeys.resource(resourceType) }) + if (resourceType !== 'workflow') return return invalidateWorkflowLists(queryClient, variables.workspaceId, ['active', 'archived']) }, }) @@ -234,6 +278,7 @@ export function useDeleteFolderMutation() { interface RestoreFolderVariables { workspaceId: string + resourceType?: ServedFolderResourceType folderId: string } @@ -241,14 +286,20 @@ export function useRestoreFolder() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async ({ workspaceId, folderId }: RestoreFolderVariables) => { + mutationFn: async ({ + workspaceId, + resourceType = 'workflow', + folderId, + }: RestoreFolderVariables) => { return requestJson(restoreFolderContract, { params: { id: folderId }, - body: { workspaceId }, + body: { workspaceId, resourceType }, }) }, onSettled: (_data, _error, variables) => { - queryClient.invalidateQueries({ queryKey: folderKeys.lists() }) + const resourceType = variables.resourceType ?? 'workflow' + queryClient.invalidateQueries({ queryKey: folderKeys.resource(resourceType) }) + if (resourceType !== 'workflow') return return invalidateWorkflowLists(queryClient, variables.workspaceId, ['active', 'archived']) }, }) @@ -318,6 +369,7 @@ export function useDuplicateFolderMutation() { interface ReorderFoldersVariables { workspaceId: string + resourceType?: ServedFolderResourceType updates: Array<{ id: string sortOrder: number @@ -329,18 +381,24 @@ export function useReorderFolders() { const queryClient = useQueryClient() return useMutation({ - mutationFn: async (variables: ReorderFoldersVariables): Promise => { - await requestJson(reorderFoldersContract, { body: variables }) + mutationFn: async ({ + resourceType = 'workflow', + ...variables + }: ReorderFoldersVariables): Promise => { + await requestJson(reorderFoldersContract, { body: { ...variables, resourceType } }) }, onMutate: async (variables) => { - await queryClient.cancelQueries({ queryKey: folderKeys.list(variables.workspaceId) }) - - const snapshot = queryClient.getQueryData( - folderKeys.list(variables.workspaceId) + const listKey = folderKeys.list( + variables.workspaceId, + 'active', + variables.resourceType ?? 'workflow' ) + await queryClient.cancelQueries({ queryKey: listKey }) + + const snapshot = queryClient.getQueryData(listKey) const updatesById = new Map(variables.updates.map((update) => [update.id, update])) - queryClient.setQueryData(folderKeys.list(variables.workspaceId), (old) => { + queryClient.setQueryData(listKey, (old) => { if (!old?.length) return old return old.map((folder) => { const update = updatesById.get(folder.id) @@ -357,11 +415,20 @@ export function useReorderFolders() { }, onError: (_error, variables, context) => { if (context?.snapshot) { - queryClient.setQueryData(folderKeys.list(variables.workspaceId), context.snapshot) + queryClient.setQueryData( + folderKeys.list(variables.workspaceId, 'active', variables.resourceType ?? 'workflow'), + context.snapshot + ) } }, onSettled: (_data, _error, variables) => { - queryClient.invalidateQueries({ queryKey: folderKeys.list(variables.workspaceId) }) + queryClient.invalidateQueries({ + queryKey: folderKeys.list( + variables.workspaceId, + 'active', + variables.resourceType ?? 'workflow' + ), + }) }, }) } diff --git a/apps/sim/hooks/queries/utils/folder-cache.ts b/apps/sim/hooks/queries/utils/folder-cache.ts index 158f558bb60..b3a43472ba8 100644 --- a/apps/sim/hooks/queries/utils/folder-cache.ts +++ b/apps/sim/hooks/queries/utils/folder-cache.ts @@ -1,15 +1,26 @@ +import type { FolderResourceType } from '@/lib/api/contracts/folders' import { getQueryClient } from '@/app/_shell/providers/get-query-client' import { folderKeys } from '@/hooks/queries/utils/folder-keys' import type { WorkflowFolder } from '@/stores/folders/types' const EMPTY_FOLDERS: WorkflowFolder[] = [] -export function getFolders(workspaceId: string): WorkflowFolder[] { +export function getFolders( + workspaceId: string, + resourceType: FolderResourceType = 'workflow' +): WorkflowFolder[] { return ( - getQueryClient().getQueryData(folderKeys.list(workspaceId)) ?? EMPTY_FOLDERS + getQueryClient().getQueryData( + folderKeys.list(workspaceId, 'active', resourceType) + ) ?? EMPTY_FOLDERS ) } -export function getFolderMap(workspaceId: string): Record { - return Object.fromEntries(getFolders(workspaceId).map((folder) => [folder.id, folder])) +export function getFolderMap( + workspaceId: string, + resourceType: FolderResourceType = 'workflow' +): Record { + return Object.fromEntries( + getFolders(workspaceId, resourceType).map((folder) => [folder.id, folder]) + ) } diff --git a/apps/sim/hooks/queries/utils/folder-keys.ts b/apps/sim/hooks/queries/utils/folder-keys.ts index 2fecf659f2d..5d2d724b64c 100644 --- a/apps/sim/hooks/queries/utils/folder-keys.ts +++ b/apps/sim/hooks/queries/utils/folder-keys.ts @@ -1,8 +1,23 @@ +import type { FolderResourceType } from '@/lib/api/contracts/folders' + export type FolderQueryScope = 'active' | 'archived' +/** + * `resourceType` is part of the key, not an implicit default, because one workspace holds + * an independent folder tree per resource. Without it the Knowledge, Tables, and Workflows + * folder lists would share a single cache entry and overwrite each other. + * + * Typed against the full `FolderResourceType` rather than the narrower set the API serves, + * so a cached row's own `resourceType` can be used to address its list without a cast. + */ export const folderKeys = { all: ['folders'] as const, lists: () => [...folderKeys.all, 'list'] as const, - list: (workspaceId: string | undefined, scope: FolderQueryScope = 'active') => - [...folderKeys.lists(), workspaceId ?? '', scope] as const, + resource: (resourceType: FolderResourceType = 'workflow') => + [...folderKeys.lists(), resourceType] as const, + list: ( + workspaceId: string | undefined, + scope: FolderQueryScope = 'active', + resourceType: FolderResourceType = 'workflow' + ) => [...folderKeys.resource(resourceType), workspaceId ?? '', scope] as const, } diff --git a/apps/sim/lib/api/contracts/folders.ts b/apps/sim/lib/api/contracts/folders.ts index a75b4bd3a7e..ec2e6317a32 100644 --- a/apps/sim/lib/api/contracts/folders.ts +++ b/apps/sim/lib/api/contracts/folders.ts @@ -5,6 +5,18 @@ import { defineRouteContract } from '@/lib/api/contracts/types' export const folderResourceTypeSchema = z.enum(['workflow', 'file', 'knowledge_base', 'table']) export type FolderResourceType = z.output +/** + * The resource types the generic folder engine actually serves. `file` is excluded on + * purpose: file folders still write the legacy `workspace_file_folders` table, so accepting + * the value here would answer "you have no folders" against a surface that plainly does. + * It gains an entry when the file cutover lands. + */ +export const servedFolderResourceTypeSchema = z + .enum(['workflow', 'knowledge_base', 'table']) + .default('workflow') + +export type ServedFolderResourceType = z.output + export const folderScopeSchema = z.enum(['active', 'archived']) export const folderSchema = z.object({ @@ -29,23 +41,22 @@ export type FolderApi = z.output export const listFoldersQuerySchema = z.object({ workspaceId: z.string({ error: 'Workspace ID is required' }).min(1, 'Workspace ID is required'), - /** - * Only workflow folders are served today — file folders are still written to - * `workspace_file_folders` and kb/table have no writer yet. Narrowed rather than - * accepting the full enum and silently answering "you have none", matching the same - * choice made on the create and reorder bodies. Widens as each type's writers land. - */ - resourceType: z.literal('workflow').default('workflow'), + resourceType: servedFolderResourceTypeSchema, scope: folderScopeSchema.default('active'), }) +/** + * Addresses which resource's folder tree an id-keyed route operates on. Required even + * though folder ids are UUIDs: without it, a caller holding a knowledge-base folder id + * could drive it through the workflow surface. + */ +export const folderResourceTypeQuerySchema = z.object({ + resourceType: servedFolderResourceTypeSchema, +}) + export const createFolderBodySchema = z.object({ id: z.string().uuid().optional(), - /** - * No `resourceType` here on purpose. The create path still writes workflow folders - * only, and accepting a value the route cannot honor would silently create the wrong - * kind of folder. It is added back with the generic `folder` table cutover. - */ + resourceType: servedFolderResourceTypeSchema, name: z.string().trim().min(1, 'Name is required').max(255, 'Name is too long'), workspaceId: z.string().min(1, 'Workspace ID is required'), /** Mirrors `updateFolderBodySchema.parentId` so explicit `null` (root folder) is accepted on create. */ @@ -71,6 +82,7 @@ export const updateFolderBodySchema = z.object({ export const restoreFolderBodySchema = z.object({ workspaceId: z.string({ error: 'Workspace ID is required' }).min(1, 'Workspace ID is required'), + resourceType: servedFolderResourceTypeSchema, }) export const duplicateFolderBodySchema = z.object({ @@ -82,7 +94,7 @@ export const duplicateFolderBodySchema = z.object({ export const reorderFoldersBodySchema = z.object({ workspaceId: z.string(), - /** See `createFolderBodySchema` — reorder still operates on workflow folders only. */ + resourceType: servedFolderResourceTypeSchema, updates: z .array( z.object({ @@ -134,6 +146,7 @@ export const updateFolderContract = defineRouteContract({ method: 'PUT', path: '/api/folders/[id]', params: folderIdParamsSchema, + query: folderResourceTypeQuerySchema, body: updateFolderBodySchema, response: { mode: 'json', @@ -147,6 +160,7 @@ export const deleteFolderContract = defineRouteContract({ method: 'DELETE', path: '/api/folders/[id]', params: folderIdParamsSchema, + query: folderResourceTypeQuerySchema, response: { mode: 'json', schema: z.object({ diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts new file mode 100644 index 00000000000..69b4940c70a --- /dev/null +++ b/apps/sim/lib/folders/cascade.test.ts @@ -0,0 +1,358 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + archiveFolderCascade, + collectActiveSubtreeIds, + collectArchivedSubtreeIds, + type DbOrTx, + restoreFolderCascade, + toCascadeCounts, +} from '@/lib/folders/cascade' +import { FOLDER_RESOURCES, type FolderResourceConfig } from '@/lib/folders/config' + +interface SelectCall { + where: unknown +} + +interface UpdateCall { + table: unknown + set: Record + where: unknown +} + +/** + * Chainable stand-in for a drizzle handle. Select chains are awaited after `.where()`; + * update chains after `.returning()`. Results are dequeued in call order, so a test states + * exactly what each successive statement sees. + */ +function makeTx(options: { selects?: unknown[][]; updates?: unknown[][] } = {}) { + const selectQueue = [...(options.selects ?? [])] + const updateQueue = [...(options.updates ?? [])] + const selectCalls: SelectCall[] = [] + const updateCalls: UpdateCall[] = [] + + const tx = { + select: () => ({ + from: () => ({ + where: (where: unknown) => { + selectCalls.push({ where }) + return Promise.resolve(selectQueue.shift() ?? []) + }, + }), + }), + update: (table: unknown) => ({ + set: (set: Record) => ({ + where: (where: unknown) => { + const rows = updateQueue.shift() ?? [] + const call: UpdateCall = { table, set, where } + updateCalls.push(call) + return { + returning: () => Promise.resolve(rows), + then: (resolve: (value: unknown) => unknown) => Promise.resolve(rows).then(resolve), + } + }, + }), + }), + } + + return { tx: tx as unknown as DbOrTx, selectCalls, updateCalls } +} + +const CHILD_TABLE = { name: 'child_table' } +const DEPENDENT_TABLE = { name: 'dependent_table' } + +function makeConfig(overrides: Partial = {}): FolderResourceConfig { + return { + resourceType: 'table', + label: 'table', + countKey: 'tables', + table: CHILD_TABLE as never, + idColumn: 'child.id' as never, + folderIdColumn: 'child.folderId' as never, + workspaceColumn: 'child.workspaceId' as never, + deletedColumn: 'child.archivedAt' as never, + deletedKey: 'archivedAt', + buildSoftDeleteSet: (timestamp, now) => ({ archivedAt: timestamp, updatedAt: now }), + ...overrides, + } +} + +/** Flattens the nested `and(...)` objects the drizzle operator mocks produce. */ +function flattenConditions(condition: unknown): Array> { + if (!condition || typeof condition !== 'object') return [] + const node = condition as Record + if (node.type === 'and' && Array.isArray(node.conditions)) { + return node.conditions.flatMap(flattenConditions) + } + return [node] +} + +function hasCondition( + condition: unknown, + predicate: (node: Record) => boolean +): boolean { + return flattenConditions(condition).some(predicate) +} + +const TIMESTAMP = new Date('2026-01-01T00:00:00.000Z') +const NOW = new Date('2026-02-02T00:00:00.000Z') + +describe('collectActiveSubtreeIds', () => { + it('returns the folder plus every active descendant', async () => { + const { tx, selectCalls } = makeTx({ + selects: [ + [ + { id: 'root', parentId: null }, + { id: 'child', parentId: 'root' }, + { id: 'grandchild', parentId: 'child' }, + { id: 'unrelated', parentId: null }, + ], + ], + }) + + const ids = await collectActiveSubtreeIds(tx, 'ws-1', 'table', 'root') + + expect(ids).toEqual(['root', 'child', 'grandchild']) + expect(selectCalls).toHaveLength(1) + expect(hasCondition(selectCalls[0].where, (node) => node.type === 'isNull')).toBe(true) + }) + + it('scopes the walk to the workspace and resourceType', async () => { + const { tx, selectCalls } = makeTx({ selects: [[]] }) + + await collectActiveSubtreeIds(tx, 'ws-1', 'knowledge_base', 'root') + + expect(hasCondition(selectCalls[0].where, (node) => node.right === 'knowledge_base')).toBe(true) + expect(hasCondition(selectCalls[0].where, (node) => node.right === 'ws-1')).toBe(true) + }) +}) + +describe('collectArchivedSubtreeIds', () => { + it('matches on the exact cascade timestamp so unrelated archived folders stay archived', async () => { + const { tx, selectCalls } = makeTx({ + selects: [ + [ + { id: 'root', parentId: null }, + { id: 'child', parentId: 'root' }, + ], + ], + }) + + const ids = await collectArchivedSubtreeIds(tx, 'ws-1', 'table', 'root', TIMESTAMP) + + expect(ids).toEqual(['root', 'child']) + expect(hasCondition(selectCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true) + }) + + it('terminates on a parent cycle instead of recursing forever', async () => { + const { tx } = makeTx({ + selects: [ + [ + { id: 'a', parentId: 'b' }, + { id: 'b', parentId: 'a' }, + ], + ], + }) + + const ids = await collectArchivedSubtreeIds(tx, 'ws-1', 'table', 'a', TIMESTAMP) + + expect(ids).toEqual(['a', 'b']) + }) +}) + +describe('archiveFolderCascade', () => { + it('archives children then folders under one shared timestamp', async () => { + const { tx, updateCalls } = makeTx({ + updates: [ + [{ id: 'child-1' }, { id: 'child-2' }], + [{ id: 'root' }, { id: 'sub' }], + ], + }) + + 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 }) + }) + + it('skips rows that were already soft-deleted independently', async () => { + const { tx, updateCalls } = makeTx({ updates: [[], []] }) + + await archiveFolderCascade(tx, makeConfig(), 'ws-1', ['root'], TIMESTAMP) + + for (const call of updateCalls) { + expect(hasCondition(call.where, (node) => node.type === 'isNull')).toBe(true) + } + }) + + it('delegates to archiveChildren when a resource archives through its own lifecycle', async () => { + const archiveChildren = vi.fn().mockResolvedValue(7) + const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }]] }) + + const counts = await archiveFolderCascade( + tx, + makeConfig({ archiveChildren }), + 'ws-1', + ['root', 'sub'], + TIMESTAMP + ) + + expect(archiveChildren).toHaveBeenCalledWith({ + workspaceId: 'ws-1', + folderIds: ['root', 'sub'], + timestamp: TIMESTAMP, + }) + expect(counts).toEqual({ folders: 1, children: 7 }) + // Only the folder table is touched directly — the hook owns the child writes. + expect(updateCalls).toHaveLength(1) + expect(updateCalls[0].table).not.toBe(CHILD_TABLE) + }) +}) + +describe('restoreFolderCascade', () => { + const dependents = [ + { + table: DEPENDENT_TABLE as never, + childIdColumn: 'dependent.childId' as never, + deletedColumn: 'dependent.archivedAt' as never, + buildRestoreSet: (now: Date) => ({ archivedAt: null, updatedAt: now }), + }, + ] + + it('restores folders, children, and dependents matching the cascade timestamp', async () => { + const { tx, updateCalls } = makeTx({ + updates: [[{ id: 'root' }, { id: 'sub' }], [{ id: 'child-1' }, { id: 'child-2' }], []], + }) + + const counts = await restoreFolderCascade( + tx, + makeConfig({ restoreDependents: dependents }), + 'ws-1', + ['root', 'sub'], + TIMESTAMP, + NOW + ) + + expect(counts).toEqual({ folders: 2, children: 2 }) + expect(updateCalls).toHaveLength(3) + expect(updateCalls[1].set).toEqual({ archivedAt: null, updatedAt: NOW }) + expect(updateCalls[2].table).toBe(DEPENDENT_TABLE) + expect( + hasCondition(updateCalls[2].where, (node) => { + return node.type === 'inArray' && Array.isArray(node.values) && node.values.length === 2 + }) + ).toBe(true) + }) + + it('issues a fixed number of statements regardless of subtree depth', async () => { + const deepSubtree = ['a', 'b', 'c', 'd', 'e', 'f'] + const { tx, updateCalls } = makeTx({ + updates: [deepSubtree.map((id) => ({ id })), [{ id: 'child-1' }], []], + }) + + await restoreFolderCascade( + tx, + makeConfig({ restoreDependents: dependents }), + 'ws-1', + deepSubtree, + TIMESTAMP, + NOW + ) + + // One UPDATE for folders, one for children, one per dependent table — never per folder. + expect(updateCalls).toHaveLength(2 + dependents.length) + }) + + it('skips dependent writes when nothing was restored', async () => { + const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }], []] }) + + const counts = await restoreFolderCascade( + tx, + makeConfig({ restoreDependents: dependents }), + 'ws-1', + ['root'], + TIMESTAMP, + NOW + ) + + expect(counts).toEqual({ folders: 1, children: 0 }) + expect(updateCalls).toHaveLength(2) + }) + + it('restores only rows carrying the folder’s own soft-delete timestamp', async () => { + const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }], [{ id: 'child-1' }], []] }) + + await restoreFolderCascade( + tx, + makeConfig({ restoreDependents: dependents }), + 'ws-1', + ['root'], + TIMESTAMP, + NOW + ) + + for (const call of updateCalls) { + expect(hasCondition(call.where, (node) => node.right === TIMESTAMP)).toBe(true) + } + }) +}) + +describe('toCascadeCounts', () => { + it('reports the child count under the resource’s own key', () => { + expect(toCascadeCounts(makeConfig(), { folders: 2, children: 3 })).toEqual({ + folders: 2, + tables: 3, + }) + expect( + toCascadeCounts(makeConfig({ countKey: 'knowledgeBases' }), { folders: 1, children: 0 }) + ).toEqual({ folders: 1, knowledgeBases: 0 }) + }) +}) + +describe('FOLDER_RESOURCES', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('declares one entry per folder resource type', () => { + expect(Object.keys(FOLDER_RESOURCES).sort()).toEqual([ + 'file', + 'knowledge_base', + 'table', + 'workflow', + ]) + }) + + it('keys every entry consistently with its own resourceType', () => { + for (const [key, config] of Object.entries(FOLDER_RESOURCES)) { + expect(config.resourceType).toBe(key) + } + }) + + it('gives every resource a distinct cascade count key', () => { + const countKeys = Object.values(FOLDER_RESOURCES).map((config) => config.countKey) + expect(new Set(countKeys).size).toBe(countKeys.length) + }) + + it('builds soft-delete payloads that write the declared soft-delete property', () => { + for (const config of Object.values(FOLDER_RESOURCES)) { + const archiveSet = config.buildSoftDeleteSet(TIMESTAMP, NOW) + const restoreSet = config.buildSoftDeleteSet(null, NOW) + expect(archiveSet[config.deletedKey]).toBe(TIMESTAMP) + expect(restoreSet[config.deletedKey]).toBeNull() + } + }) + + it('restores dependents to an active state', () => { + for (const config of Object.values(FOLDER_RESOURCES)) { + for (const dependent of config.restoreDependents ?? []) { + expect(Object.values(dependent.buildRestoreSet(NOW))).toContain(null) + } + } + }) +}) diff --git a/apps/sim/lib/folders/cascade.ts b/apps/sim/lib/folders/cascade.ts new file mode 100644 index 00000000000..82ae303f73b --- /dev/null +++ b/apps/sim/lib/folders/cascade.ts @@ -0,0 +1,183 @@ +import type { db } from '@sim/db' +import { folder as folderTable } from '@sim/db/schema' +import { and, eq, inArray, isNull, type SQL } from 'drizzle-orm' +import type { FolderCascadeCountsApi, FolderResourceType } from '@/lib/api/contracts/folders' +import type { FolderResourceConfig } from '@/lib/folders/config' +import { collectDescendantFolderIds } from '@/lib/folders/subtree' + +/** Narrow enough for both `db` and an open transaction handle. */ +export type DbOrTx = Pick + +export interface FolderCascadeCounts { + /** Folders in the cascade, including the root. */ + folders: number + /** Resources of the folder's own type that moved with it. */ + children: number +} + +/** + * Resolves the folder plus every *active* descendant, in one query. Used by delete: an + * archived descendant was archived independently and keeps its own timestamp, so it must + * not be swept into this cascade. + */ +export async function collectActiveSubtreeIds( + tx: DbOrTx, + workspaceId: string, + resourceType: FolderResourceType, + folderId: string +): Promise { + const activeFolders = await tx + .select({ id: folderTable.id, parentId: folderTable.parentId }) + .from(folderTable) + .where( + and( + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, resourceType), + isNull(folderTable.deletedAt) + ) + ) + + return [folderId, ...collectDescendantFolderIds(activeFolders, folderId)] +} + +/** + * Resolves the folder plus every descendant archived in the same cascade, in one query. + * + * Matching on the exact `timestamp` is what stops a restore from also reviving folders + * that were archived independently before or after — and is why this cannot reuse + * {@link collectActiveSubtreeIds}, which by definition cannot see an archived subtree. + */ +export async function collectArchivedSubtreeIds( + tx: DbOrTx, + workspaceId: string, + resourceType: FolderResourceType, + folderId: string, + timestamp: Date +): Promise { + const archivedFolders = await tx + .select({ id: folderTable.id, parentId: folderTable.parentId }) + .from(folderTable) + .where( + and( + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, resourceType), + eq(folderTable.deletedAt, timestamp) + ) + ) + + return [folderId, ...collectDescendantFolderIds(archivedFolders, folderId)] +} + +function childFilter(config: FolderResourceConfig, workspaceId: string, folderIds: string[]): SQL { + return and( + inArray(config.folderIdColumn, folderIds), + eq(config.workspaceColumn, workspaceId), + config.scope + ) as SQL +} + +/** + * 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. + * + * Children are archived before folders so a concurrent reader never sees an active + * resource inside a folder that has already vanished from the tree. + */ +export async function archiveFolderCascade( + tx: DbOrTx, + config: FolderResourceConfig, + workspaceId: string, + folderIds: string[], + timestamp: Date +): Promise { + 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 }) + .where( + and( + inArray(folderTable.id, folderIds), + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, config.resourceType), + isNull(folderTable.deletedAt) + ) + ) + .returning({ id: folderTable.id }) + + return { folders: archivedFolders.length, children } +} + +/** + * Restores `folderIds` — the subtree resolved by {@link collectArchivedSubtreeIds} — plus + * every resource archived at the same `timestamp`, then reactivates the dependent rows + * (schedules, webhooks, chats) hanging off those resources. + * + * Fixed query count regardless of subtree depth: one UPDATE for the folders, one for the + * resources, and one per declared dependent table. + */ +export async function restoreFolderCascade( + tx: DbOrTx, + config: FolderResourceConfig, + workspaceId: string, + folderIds: string[], + timestamp: Date, + now: Date +): Promise { + const restoredFolders = await tx + .update(folderTable) + .set({ deletedAt: null, updatedAt: now }) + .where( + and( + inArray(folderTable.id, folderIds), + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, config.resourceType), + eq(folderTable.deletedAt, timestamp) + ) + ) + .returning({ id: folderTable.id }) + + const restoredChildren = await tx + .update(config.table) + .set(config.buildSoftDeleteSet(null, now)) + .where(and(childFilter(config, workspaceId, folderIds), eq(config.deletedColumn, timestamp))) + .returning({ id: config.idColumn }) + + const childIds = restoredChildren.map((row) => row.id as string) + + if (childIds.length > 0) { + for (const dependent of config.restoreDependents ?? []) { + await tx + .update(dependent.table) + .set(dependent.buildRestoreSet(now)) + .where( + and(inArray(dependent.childIdColumn, childIds), eq(dependent.deletedColumn, timestamp)) + ) + } + } + + return { folders: restoredFolders.length, children: childIds.length } +} + +/** + * Maps internal cascade counts onto the per-resourceType shape the API returns, so a + * `knowledge_base` folder reports `knowledgeBases` and a `table` folder reports `tables`. + */ +export function toCascadeCounts( + config: FolderResourceConfig, + counts: FolderCascadeCounts +): FolderCascadeCountsApi { + return { folders: counts.folders, [config.countKey]: counts.children } +} diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts new file mode 100644 index 00000000000..072a412f3d8 --- /dev/null +++ b/apps/sim/lib/folders/config.ts @@ -0,0 +1,304 @@ +import { + chat, + knowledgeBase, + userTableDefinitions, + webhook, + workflow, + workflowMcpTool, + workflowSchedule, + workspaceFiles, +} from '@sim/db/schema' +import { eq, type SQL } from 'drizzle-orm' +import type { PgColumn, PgTable } from 'drizzle-orm/pg-core' +import type { FolderResourceType } from '@/lib/api/contracts/folders' + +/** + * Counts of cascaded resources returned by a folder delete/restore, keyed per resource + * type so a caller can render "3 workflows" vs "3 tables" without inspecting the folder. + */ +export type FolderChildCountKey = 'workflows' | 'files' | 'knowledgeBases' | 'tables' + +/** + * A table whose rows hang off a foldered resource and share its soft-delete lifecycle — + * e.g. a workflow's schedules and webhooks. Restored alongside the resource so a folder + * restore brings back a fully functional resource rather than a headless row. + * + * Archive is deliberately not expressed here: the archive direction has side effects + * beyond a row update (deactivating deployments, notifying the socket, external webhook + * teardown) and is owned by {@link FolderResourceConfig.archiveChildren}. + */ +export interface FolderDependentTable { + table: PgTable + /** Column on `table` holding the parent resource's id. */ + childIdColumn: PgColumn + /** Soft-delete timestamp column, matched exactly against the cascade timestamp. */ + deletedColumn: PgColumn + /** Typed at the definition site so a dropped or renamed column fails to compile. */ + buildRestoreSet: (now: Date) => Record +} + +/** Everything the cascade needs to archive the resources inside a folder. */ +export interface ArchiveChildrenContext { + workspaceId: string + /** The folder plus every active descendant folder, already resolved. */ + folderIds: string[] + /** Shared across the whole cascade; restore matches on it exactly. */ + timestamp: Date +} + +/** Reason a delete must be refused, in the orchestration error vocabulary. */ +export interface FolderDeleteRejection { + error: string + errorCode: 'validation' | 'conflict' +} + +/** + * Everything that differs between the four folder-bearing resource types, expressed as + * data. The folder engine in `lib/folders/lifecycle.ts` and the cascade in + * `lib/folders/cascade.ts` read this instead of branching on `resourceType`, so + * create/update/delete/restore/reorder each exist exactly once and adding a fifth + * foldered resource means adding one entry here. + */ +export interface FolderResourceConfig { + resourceType: FolderResourceType + /** Human-readable noun used in audit-log descriptions and log lines. */ + label: string + countKey: FolderChildCountKey + /** Table holding the resources that live *inside* folders of this type. */ + table: PgTable + idColumn: PgColumn + folderIdColumn: PgColumn + workspaceColumn: PgColumn + /** Soft-delete timestamp column; the resource is active while this is null. */ + deletedColumn: PgColumn + /** + * Property key backing {@link deletedColumn}. Drizzle's `.set()` takes TypeScript + * property names while `.where()` takes column objects, so the cascade needs both. + * These genuinely differ per table (`archivedAt` vs `deletedAt`), which is exactly the + * delta this config exists to capture. The `.set()` payloads themselves are built by + * {@link buildSoftDeleteSet} so they stay typed against the concrete table. + */ + deletedKey: 'deletedAt' | 'archivedAt' + /** + * Builds the `.set()` payload that soft-deletes (`timestamp`) or restores (`null`) a + * child row. Declared per resource with `satisfies Partial` + * so a dropped or renamed column is a compile error rather than a silent no-op — never + * hand the cascade a `Record` literal. + */ + buildSoftDeleteSet: (timestamp: Date | null, now: Date) => Record + /** Narrows which rows of `table` participate in folder membership at all. */ + scope?: SQL + /** + * Column used to place a newly created folder above existing siblings. Only workflows + * order their resources alongside folders; knowledge bases and tables have no per-row + * sort order, so the new folder's position is derived from sibling folders alone. + */ + sortOrderColumn?: PgColumn + /** Rows restored alongside each resource; see {@link FolderDependentTable}. */ + restoreDependents?: FolderDependentTable[] + /** + * Replaces the cascade's default "one UPDATE over the child table" when archiving a + * resource has side effects the row update cannot express. Returns the number of + * resources archived. + */ + archiveChildren?: (context: ArchiveChildrenContext) => Promise + /** + * Runs before any write on delete. Returns a rejection to refuse the delete, or `null` + * to proceed. + */ + guardDelete?: (context: { + workspaceId: string + folderIds: string[] + }) => Promise +} + +/** + * Archives the workflows in a folder subtree through the workflow lifecycle rather than a + * bare UPDATE: archiving a workflow also deactivates its deployments, tears down external + * webhooks, notifies the realtime socket, and republishes MCP tool lists. + * + * Imported lazily so knowledge-base and table folder routes do not pull the workflow + * executor, socket client, and MCP pub/sub into their module graph. + */ +async function archiveWorkflowChildren({ + workspaceId, + folderIds, + timestamp, +}: ArchiveChildrenContext): Promise { + const [{ db }, { and, eq: eqOp, inArray, isNull }, { archiveWorkflowsByIdsInWorkspace }] = + await Promise.all([ + import('@sim/db'), + import('drizzle-orm'), + import('@/lib/workflows/lifecycle'), + ]) + + const workflowsInFolders = await db + .select({ id: workflow.id }) + .from(workflow) + .where( + and( + inArray(workflow.folderId, folderIds), + eqOp(workflow.workspaceId, workspaceId), + isNull(workflow.archivedAt) + ) + ) + + if (workflowsInFolders.length === 0) return 0 + + await archiveWorkflowsByIdsInWorkspace( + workspaceId, + workflowsInFolders.map((entry) => entry.id), + { requestId: `folder-cascade-${folderIds[0]}`, archivedAt: timestamp } + ) + + return workflowsInFolders.length +} + +/** + * Refuses to archive the last active workflow(s) in a workspace. A workspace with zero + * active workflows renders an unopenable editor, so the workflow surface has always + * blocked this; knowledge bases and tables have no such requirement. + */ +async function guardLastWorkflows({ + workspaceId, + folderIds, +}: { + workspaceId: string + folderIds: string[] +}): Promise { + const [{ db }, { and, eq: eqOp, inArray, isNull }] = await Promise.all([ + import('@sim/db'), + import('drizzle-orm'), + ]) + + const [inFolders, inWorkspace] = await Promise.all([ + db + .select({ id: workflow.id }) + .from(workflow) + .where( + and( + inArray(workflow.folderId, folderIds), + eqOp(workflow.workspaceId, workspaceId), + isNull(workflow.archivedAt) + ) + ), + db + .select({ id: workflow.id }) + .from(workflow) + .where(and(eqOp(workflow.workspaceId, workspaceId), isNull(workflow.archivedAt))), + ]) + + if (inFolders.length > 0 && inFolders.length >= inWorkspace.length) { + return { + error: 'Cannot delete folder containing the only workflow(s) in the workspace', + errorCode: 'validation', + } + } + + return null +} + +export const FOLDER_RESOURCES: Record = { + workflow: { + resourceType: 'workflow', + label: 'workflow', + countKey: 'workflows', + table: workflow, + idColumn: workflow.id, + folderIdColumn: workflow.folderId, + workspaceColumn: workflow.workspaceId, + deletedColumn: workflow.archivedAt, + deletedKey: 'archivedAt', + buildSoftDeleteSet: (timestamp, now) => + ({ archivedAt: timestamp, updatedAt: now }) satisfies Partial, + sortOrderColumn: workflow.sortOrder, + restoreDependents: [ + { + table: workflowSchedule, + childIdColumn: workflowSchedule.workflowId, + deletedColumn: workflowSchedule.archivedAt, + buildRestoreSet: (now) => + ({ archivedAt: null, updatedAt: now }) satisfies Partial< + typeof workflowSchedule.$inferInsert + >, + }, + { + table: webhook, + childIdColumn: webhook.workflowId, + deletedColumn: webhook.archivedAt, + buildRestoreSet: (now) => + ({ archivedAt: null, updatedAt: now }) satisfies Partial, + }, + { + table: chat, + childIdColumn: chat.workflowId, + deletedColumn: chat.archivedAt, + buildRestoreSet: (now) => + ({ archivedAt: null, updatedAt: now }) satisfies Partial, + }, + { + table: workflowMcpTool, + childIdColumn: workflowMcpTool.workflowId, + deletedColumn: workflowMcpTool.archivedAt, + buildRestoreSet: (now) => + ({ archivedAt: null, updatedAt: now }) satisfies Partial< + typeof workflowMcpTool.$inferInsert + >, + }, + ], + archiveChildren: archiveWorkflowChildren, + guardDelete: guardLastWorkflows, + }, + file: { + resourceType: 'file', + label: 'file', + countKey: 'files', + table: workspaceFiles, + idColumn: workspaceFiles.id, + folderIdColumn: workspaceFiles.folderId, + workspaceColumn: workspaceFiles.workspaceId, + deletedColumn: workspaceFiles.deletedAt, + deletedKey: 'deletedAt', + buildSoftDeleteSet: (timestamp) => + ({ deletedAt: timestamp }) satisfies Partial, + /** + * `workspace_files` also stores copilot/chat/execution artifacts and profile pictures; + * only files surfaced on the Files page live in folders. + */ + scope: eq(workspaceFiles.context, 'workspace'), + }, + knowledge_base: { + resourceType: 'knowledge_base', + label: 'knowledge base', + countKey: 'knowledgeBases', + table: knowledgeBase, + idColumn: knowledgeBase.id, + folderIdColumn: knowledgeBase.folderId, + workspaceColumn: knowledgeBase.workspaceId, + deletedColumn: knowledgeBase.deletedAt, + deletedKey: 'deletedAt', + buildSoftDeleteSet: (timestamp, now) => + ({ deletedAt: timestamp, updatedAt: now }) satisfies Partial< + typeof knowledgeBase.$inferInsert + >, + }, + table: { + resourceType: 'table', + label: 'table', + countKey: 'tables', + table: userTableDefinitions, + idColumn: userTableDefinitions.id, + folderIdColumn: userTableDefinitions.folderId, + workspaceColumn: userTableDefinitions.workspaceId, + deletedColumn: userTableDefinitions.archivedAt, + deletedKey: 'archivedAt', + buildSoftDeleteSet: (timestamp, now) => + ({ archivedAt: timestamp, updatedAt: now }) satisfies Partial< + typeof userTableDefinitions.$inferInsert + >, + }, +} + +export function folderResourceConfig(resourceType: FolderResourceType): FolderResourceConfig { + return FOLDER_RESOURCES[resourceType] +} diff --git a/apps/sim/lib/folders/lifecycle.ts b/apps/sim/lib/folders/lifecycle.ts new file mode 100644 index 00000000000..ed3b5a4b251 --- /dev/null +++ b/apps/sim/lib/folders/lifecycle.ts @@ -0,0 +1,469 @@ +import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { db } from '@sim/db' +import { folder as folderTable } from '@sim/db/schema' +import { createLogger } from '@sim/logger' +import { getPostgresErrorCode } from '@sim/utils/errors' +import { generateId } from '@sim/utils/id' +import { and, eq, isNull, min } from 'drizzle-orm' +import type { FolderCascadeCountsApi, FolderResourceType } from '@/lib/api/contracts/folders' +import { + archiveFolderCascade, + collectActiveSubtreeIds, + collectArchivedSubtreeIds, + restoreFolderCascade, + toCascadeCounts, +} from '@/lib/folders/cascade' +import { folderResourceConfig } from '@/lib/folders/config' +import { deduplicateFolderName } from '@/lib/folders/naming' +import { wouldCreateFolderCycle } from '@/lib/folders/queries' +import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' + +const logger = createLogger('FolderLifecycle') + +const DUPLICATE_NAME_ERROR = 'A folder with this name already exists in this location' + +export interface CreateFolderParams { + resourceType: FolderResourceType + userId: string + workspaceId: string + name: string + id?: string + parentId?: string | null + sortOrder?: number +} + +export interface FolderMutationResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + folder?: typeof folderTable.$inferSelect +} + +export interface UpdateFolderParams { + resourceType: FolderResourceType + folderId: string + workspaceId: string + userId: string + name?: string + locked?: boolean + parentId?: string | null + sortOrder?: number +} + +export interface DeleteFolderParams { + resourceType: FolderResourceType + folderId: string + workspaceId: string + userId: string + folderName?: string +} + +export interface DeleteFolderResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + deletedItems?: FolderCascadeCountsApi +} + +export interface RestoreFolderParams { + resourceType: FolderResourceType + folderId: string + workspaceId: string + userId: string + folderName?: string +} + +export interface RestoreFolderResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + restoredItems?: FolderCascadeCountsApi +} + +/** + * Verifies that a prospective parent folder exists, belongs to the target workspace, is of + * the same `resourceType`, and is not archived. + * + * The resourceType check is not defensive padding: the DB trigger + * `folder_parent_resource_type_match` enforces the same invariant, so an app layer that + * skipped it would surface a raw trigger error instead of a 400. + */ +async function assertParentFolderInWorkspace( + resourceType: FolderResourceType, + parentId: string, + workspaceId: string +): Promise<{ error: string; errorCode: OrchestrationErrorCode } | null> { + const [parent] = await db + .select({ + workspaceId: folderTable.workspaceId, + archivedAt: folderTable.deletedAt, + }) + .from(folderTable) + .where(and(eq(folderTable.id, parentId), eq(folderTable.resourceType, resourceType))) + .limit(1) + + if (!parent || parent.workspaceId !== workspaceId || parent.archivedAt) { + return { error: 'Parent folder not found', errorCode: 'validation' } + } + + return null +} + +/** + * Places a new folder above its existing siblings. Workflows share one ordering space with + * their folders, so their `sortOrder` participates; knowledge bases and tables have no + * per-row ordering and are ignored via the absent `sortOrderColumn`. + */ +async function nextFolderSortOrder( + resourceType: FolderResourceType, + workspaceId: string, + parentId: string | null | undefined +): Promise { + const config = folderResourceConfig(resourceType) + const folderParentCondition = parentId + ? eq(folderTable.parentId, parentId) + : isNull(folderTable.parentId) + + const folderMinPromise = db + .select({ minSortOrder: min(folderTable.sortOrder) }) + .from(folderTable) + .where( + and( + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, resourceType), + folderParentCondition + ) + ) + + const childMinPromise: Promise> = config.sortOrderColumn + ? db + .select({ minSortOrder: min(config.sortOrderColumn) }) + .from(config.table) + .where( + and( + eq(config.workspaceColumn, workspaceId), + parentId ? eq(config.folderIdColumn, parentId) : isNull(config.folderIdColumn), + config.scope + ) + ) + : Promise.resolve([]) + + const [[folderResult], [childResult]] = await Promise.all([folderMinPromise, childMinPromise]) + + const candidates = [folderResult?.minSortOrder, childResult?.minSortOrder] + .filter((value) => value != null) + .map(Number) + .filter((value) => Number.isFinite(value)) + + return candidates.length > 0 ? Math.min(...candidates) - 1 : 0 +} + +export async function createFolder(params: CreateFolderParams): Promise { + const config = folderResourceConfig(params.resourceType) + + try { + const folderId = params.id || generateId() + const parentId = params.parentId || null + + if (parentId) { + if (parentId === folderId) { + return { success: false, error: 'Folder cannot be its own parent', errorCode: 'validation' } + } + const parentError = await assertParentFolderInWorkspace( + params.resourceType, + parentId, + params.workspaceId + ) + if (parentError) return { success: false, ...parentError } + } + + const sortOrder = + params.sortOrder !== undefined + ? params.sortOrder + : await nextFolderSortOrder(params.resourceType, params.workspaceId, parentId) + + const [folder] = await db + .insert(folderTable) + .values({ + id: folderId, + resourceType: params.resourceType, + name: params.name.trim(), + userId: params.userId, + workspaceId: params.workspaceId, + parentId, + sortOrder, + }) + .returning() + + logger.info('Created folder', { + folderId, + resourceType: params.resourceType, + workspaceId: params.workspaceId, + parentId, + }) + + recordAudit({ + workspaceId: params.workspaceId, + actorId: params.userId, + action: AuditAction.FOLDER_CREATED, + resourceType: AuditResourceType.FOLDER, + resourceId: folderId, + resourceName: folder.name, + description: `Created ${config.label} folder "${folder.name}"`, + metadata: { + name: folder.name, + workspaceId: params.workspaceId, + folderResourceType: params.resourceType, + parentId: parentId || undefined, + sortOrder: folder.sortOrder, + }, + }) + + return { success: true, folder } + } catch (error) { + // The partial unique index on active (workspaceId, resourceType, parent, name) makes a + // duplicate sibling name rejectable here. Create is a path where the user chooses the + // name, so surface a 409 rather than silently deduplicating. + if (getPostgresErrorCode(error) === '23505') { + return { success: false, error: DUPLICATE_NAME_ERROR, errorCode: 'conflict' } + } + logger.error('Failed to create folder', { error, resourceType: params.resourceType }) + return { success: false, error: 'Internal server error', errorCode: 'internal' } + } +} + +export async function updateFolder(params: UpdateFolderParams): Promise { + try { + if (params.parentId && params.parentId === params.folderId) { + return { success: false, error: 'Folder cannot be its own parent', errorCode: 'validation' } + } + + if (params.parentId) { + const parentError = await assertParentFolderInWorkspace( + params.resourceType, + params.parentId, + params.workspaceId + ) + if (parentError) return { success: false, ...parentError } + + const wouldCreateCycle = await wouldCreateFolderCycle( + params.folderId, + params.parentId, + params.resourceType + ) + if (wouldCreateCycle) { + return { + success: false, + error: 'Cannot create circular folder reference', + errorCode: 'validation', + } + } + } + + // Typed against the table rather than `Record`: the loose type is what + // let `color`/`isExpanded` survive an earlier cutover after the create path dropped them. + const updates: Partial = { updatedAt: new Date() } + if (params.name !== undefined) updates.name = params.name.trim() + if (params.locked !== undefined) updates.locked = params.locked + if (params.parentId !== undefined) updates.parentId = params.parentId || null + if (params.sortOrder !== undefined) updates.sortOrder = params.sortOrder + + const [folder] = await db + .update(folderTable) + .set(updates) + .where( + and( + eq(folderTable.id, params.folderId), + eq(folderTable.workspaceId, params.workspaceId), + eq(folderTable.resourceType, params.resourceType) + ) + ) + .returning() + + if (!folder) { + return { success: false, error: 'Folder not found', errorCode: 'not_found' } + } + + logger.info('Updated folder', { + folderId: params.folderId, + resourceType: params.resourceType, + updates, + }) + + return { success: true, folder } + } catch (error) { + if (getPostgresErrorCode(error) === '23505') { + return { success: false, error: DUPLICATE_NAME_ERROR, errorCode: 'conflict' } + } + logger.error('Failed to update folder', { error, resourceType: params.resourceType }) + return { success: false, error: 'Internal server error', errorCode: 'internal' } + } +} + +/** + * Soft-deletes a folder and everything under it. The subtree is resolved once, handed to + * the resource's optional delete guard, then archived under one shared timestamp so + * {@link restoreFolder} can bring back exactly this set and nothing else. + */ +export async function deleteFolder(params: DeleteFolderParams): Promise { + const { resourceType, folderId, workspaceId, userId, folderName } = params + const config = folderResourceConfig(resourceType) + + const folderIds = await collectActiveSubtreeIds(db, workspaceId, resourceType, folderId) + + const rejection = await config.guardDelete?.({ workspaceId, folderIds }) + if (rejection) { + return { success: false, error: rejection.error, errorCode: rejection.errorCode } + } + + const counts = await archiveFolderCascade(db, config, workspaceId, folderIds, new Date()) + + logger.info('Deleted folder and all contents', { folderId, resourceType, counts }) + + recordAudit({ + workspaceId, + actorId: userId, + action: AuditAction.FOLDER_DELETED, + resourceType: AuditResourceType.FOLDER, + resourceId: folderId, + resourceName: folderName, + description: `Deleted ${config.label} folder "${folderName || folderId}"`, + metadata: { + folderResourceType: resourceType, + affected: { + [config.countKey]: counts.children, + subfolders: Math.max(counts.folders - 1, 0), + }, + }, + }) + + return { success: true, deletedItems: toCascadeCounts(config, counts) } +} + +/** + * Restores an archived folder together with everything archived alongside it. + * + * Two name hazards are handled before the cascade runs, both inside the transaction: + * a folder whose parent is still archived is re-rooted, and the restored name is + * deduplicated against the *resolved* parent's active siblings — the caller cannot rename + * an archived folder, so a taken name would otherwise make it permanently unrestorable. + */ +export async function restoreFolder(params: RestoreFolderParams): Promise { + const { resourceType, folderId, workspaceId, userId, folderName } = params + const config = folderResourceConfig(resourceType) + + const [folder] = await db + .select() + .from(folderTable) + .where( + and( + eq(folderTable.id, folderId), + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, resourceType) + ) + ) + + if (!folder) { + return { success: false, error: 'Folder not found', errorCode: 'not_found' } + } + + const archivedAt = folder.deletedAt + if (!archivedAt) { + return { success: true, restoredItems: toCascadeCounts(config, { folders: 0, children: 0 }) } + } + + 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', + errorCode: 'validation', + } + } + + let counts: { folders: number; children: number } + try { + counts = await db.transaction(async (tx) => { + const now = new Date() + + let resolvedParentId = folder.parentId + if (folder.parentId) { + const [parentFolder] = await tx + .select({ archivedAt: folderTable.deletedAt }) + .from(folderTable) + .where( + and(eq(folderTable.id, folder.parentId), eq(folderTable.resourceType, resourceType)) + ) + + if (!parentFolder || parentFolder.archivedAt) { + resolvedParentId = null + await tx + .update(folderTable) + .set({ parentId: null }) + .where(and(eq(folderTable.id, folderId), eq(folderTable.resourceType, resourceType))) + } + } + + // Safe to rename while the row is still archived: the unique index only covers active + // rows, so this cannot collide before the cascade below clears `deletedAt`. Only the + // restore root can conflict — descendants come back alongside the siblings they were + // archived with. + const restoredName = await deduplicateFolderName( + tx, + workspaceId, + resolvedParentId, + folder.name, + resourceType + ) + if (restoredName !== folder.name) { + logger.info('Renamed folder on restore to avoid a sibling name conflict', { + folderId, + resourceType, + from: folder.name, + to: restoredName, + }) + await tx.update(folderTable).set({ name: restoredName }).where(eq(folderTable.id, folderId)) + } + + const folderIds = await collectArchivedSubtreeIds( + tx, + workspaceId, + resourceType, + folderId, + archivedAt + ) + + return restoreFolderCascade(tx, config, workspaceId, folderIds, archivedAt, now) + }) + } catch (error) { + // Clearing `deletedAt` brings the row back under the partial unique index on active + // (workspaceId, resourceType, parent, name). Dedup above covers the restore root, but a + // concurrent create can still take the name between the check and the write. + if (getPostgresErrorCode(error) === '23505') { + return { success: false, error: DUPLICATE_NAME_ERROR, errorCode: 'conflict' } + } + throw error + } + + logger.info('Restored folder and all contents', { folderId, resourceType, counts }) + + recordAudit({ + workspaceId, + actorId: userId, + action: AuditAction.FOLDER_RESTORED, + resourceType: AuditResourceType.FOLDER, + resourceId: folderId, + resourceName: folderName ?? folder.name, + description: `Restored ${config.label} folder "${folderName ?? folder.name}"`, + metadata: { + folderResourceType: resourceType, + affected: { + [config.countKey]: counts.children, + subfolders: Math.max(counts.folders - 1, 0), + }, + }, + }) + + return { success: true, restoredItems: toCascadeCounts(config, counts) } +} diff --git a/apps/sim/lib/folders/naming.ts b/apps/sim/lib/folders/naming.ts index f6b5f2a4de0..500380d6bd1 100644 --- a/apps/sim/lib/folders/naming.ts +++ b/apps/sim/lib/folders/naming.ts @@ -1,4 +1,5 @@ -import { type db, folder as folderTable } from '@sim/db' +import type { db } from '@sim/db' +import { folder as folderTable } from '@sim/db/schema' import { and, eq, isNull } from 'drizzle-orm' import type { FolderResourceType } from '@/lib/api/contracts/folders' diff --git a/apps/sim/lib/folders/queries.ts b/apps/sim/lib/folders/queries.ts index dfc6062150b..a9ea707b0be 100644 --- a/apps/sim/lib/folders/queries.ts +++ b/apps/sim/lib/folders/queries.ts @@ -20,6 +20,35 @@ export function toFolderApi(row: typeof folder.$inferSelect): FolderApi { } } +/** + * Walks up from `parentId` to check whether reparenting `folderId` under it would close a + * cycle. Scoped to `resourceType` so the walk cannot escape into another resource's tree + * via an id the caller supplied. + */ +export async function wouldCreateFolderCycle( + folderId: string, + parentId: string, + resourceType: FolderResourceType +): Promise { + let currentParentId: string | null = parentId + const visited = new Set() + + while (currentParentId) { + if (visited.has(currentParentId) || currentParentId === folderId) return true + visited.add(currentParentId) + + const [parent] = await db + .select({ parentId: folder.parentId }) + .from(folder) + .where(and(eq(folder.id, currentParentId), eq(folder.resourceType, resourceType))) + .limit(1) + + currentParentId = parent?.parentId || null + } + + return false +} + /** Shared by `GET /api/folders` and the sidebar prefetch so the query never drifts between them. */ export async function listFoldersForWorkspace( workspaceId: string, diff --git a/apps/sim/lib/posthog/events.ts b/apps/sim/lib/posthog/events.ts index ce7c2657dbd..aa520209e87 100644 --- a/apps/sim/lib/posthog/events.ts +++ b/apps/sim/lib/posthog/events.ts @@ -632,10 +632,12 @@ export interface PostHogEventMap { folder_created: { workspace_id: string + resource_type?: string } folder_deleted: { workspace_id: string + resource_type?: string } folder_renamed: { @@ -657,6 +659,7 @@ export interface PostHogEventMap { folder_restored: { folder_id: string workspace_id: string + resource_type?: string } logs_filter_applied: { diff --git a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts index 4159442b056..61058b4a660 100644 --- a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts @@ -1,23 +1,17 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' -import { db } from '@sim/db' -import { - chat, - folder as folderTable, - webhook, - workflow, - workflowMcpTool, - workflowSchedule, -} from '@sim/db/schema' -import { createLogger } from '@sim/logger' -import { getPostgresErrorCode } from '@sim/utils/errors' -import { generateId } from '@sim/utils/id' -import { and, eq, inArray, isNull, min } from 'drizzle-orm' -import { deduplicateFolderName } from '@/lib/folders/naming' -import { archiveWorkflowsByIdsInWorkspace } from '@/lib/workflows/lifecycle' +import type { folder as folderTable } from '@sim/db/schema' +import { createFolder, deleteFolder, restoreFolder, updateFolder } from '@/lib/folders/lifecycle' import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' -import { checkForCircularReference } from '@/lib/workflows/utils' -const logger = createLogger('FolderLifecycle') +/** + * Workflow-bound entry points into the generic folder engine in `lib/folders/lifecycle.ts`. + * + * The engine is resourceType-driven and owns the actual writes; these wrappers exist so the + * workflow callers that predate it (the folders API, the copilot workflow tools, the + * resource-restore orchestrator) keep a single, workflow-shaped signature. Everything that + * differs for workflows — the last-workflow delete guard, archiving through the workflow + * lifecycle so deployments and webhooks tear down, and restoring schedules/webhooks/chats — + * is declared as data on the `workflow` entry of `FOLDER_RESOURCES`, not branched on here. + */ export interface PerformCreateFolderParams { userId: string @@ -25,7 +19,6 @@ export interface PerformCreateFolderParams { name: string id?: string parentId?: string | null - color?: string sortOrder?: number } @@ -46,317 +39,8 @@ export interface PerformUpdateFolderParams { sortOrder?: number } -export interface PerformUpdateFolderResult { - success: boolean - error?: string - errorCode?: OrchestrationErrorCode - folder?: typeof folderTable.$inferSelect -} - -/** - * Verifies that a prospective parent folder exists, belongs to the target - * workspace, and is not archived. Mirrors the validation in the duplicate - * route's `assertTargetParentFolderMutable` so a caller cannot reparent a - * folder to a non-existent id or to a folder in another workspace. Returns - * an error result when invalid, or `null` when the parent is acceptable. - */ -async function assertParentFolderInWorkspace( - parentId: string, - workspaceId: string -): Promise<{ error: string; errorCode: OrchestrationErrorCode } | null> { - const [parent] = await db - .select({ - workspaceId: folderTable.workspaceId, - archivedAt: folderTable.deletedAt, - }) - .from(folderTable) - .where(and(eq(folderTable.id, parentId), eq(folderTable.resourceType, 'workflow'))) - .limit(1) - - if (!parent || parent.workspaceId !== workspaceId || parent.archivedAt) { - return { error: 'Parent folder not found', errorCode: 'validation' } - } - - return null -} - -async function nextFolderSortOrder( - workspaceId: string, - parentId: string | null | undefined -): Promise { - const folderParentCondition = parentId - ? eq(folderTable.parentId, parentId) - : isNull(folderTable.parentId) - const workflowParentCondition = parentId - ? eq(workflow.folderId, parentId) - : isNull(workflow.folderId) - - const [[folderResult], [workflowResult]] = await Promise.all([ - db - .select({ minSortOrder: min(folderTable.sortOrder) }) - .from(folderTable) - .where( - and( - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - folderParentCondition - ) - ), - db - .select({ minSortOrder: min(workflow.sortOrder) }) - .from(workflow) - .where(and(eq(workflow.workspaceId, workspaceId), workflowParentCondition)), - ]) - - const minSortOrder = [folderResult?.minSortOrder, workflowResult?.minSortOrder].reduce< - number | null - >((currentMin, candidate) => { - if (candidate == null) return currentMin - if (currentMin == null) return candidate - return Math.min(currentMin, candidate) - }, null) - - return minSortOrder != null ? minSortOrder - 1 : 0 -} - -export async function performCreateFolder( - params: PerformCreateFolderParams -): Promise { - try { - const folderId = params.id || generateId() - const parentId = params.parentId || null - - if (parentId) { - if (parentId === folderId) { - return { - success: false, - error: 'Folder cannot be its own parent', - errorCode: 'validation', - } - } - const parentError = await assertParentFolderInWorkspace(parentId, params.workspaceId) - if (parentError) return { success: false, ...parentError } - } - - const sortOrder = - params.sortOrder !== undefined - ? params.sortOrder - : await nextFolderSortOrder(params.workspaceId, parentId) - - const [folder] = await db - .insert(folderTable) - .values({ - id: folderId, - resourceType: 'workflow', - name: params.name.trim(), - userId: params.userId, - workspaceId: params.workspaceId, - parentId, - sortOrder, - }) - .returning() - - logger.info('Created workflow folder', { folderId, workspaceId: params.workspaceId, parentId }) - - recordAudit({ - workspaceId: params.workspaceId, - actorId: params.userId, - action: AuditAction.FOLDER_CREATED, - resourceType: AuditResourceType.FOLDER, - resourceId: folderId, - resourceName: folder.name, - description: `Created folder "${folder.name}"`, - metadata: { - name: folder.name, - workspaceId: params.workspaceId, - parentId: parentId || undefined, - sortOrder: folder.sortOrder, - }, - }) - - return { success: true, folder } - } catch (error) { - // `folder` carries a unique index on (workspaceId, resourceType, parent, name) for active - // rows that `workflow_folder` never had, so a duplicate sibling name is newly rejectable - // here. Map it to a 409 rather than letting it surface as a 500 — the client-side dedup - // in useFolderCreateWithDedup is best-effort and races, and the copilot/import paths - // create folders by name. - if (getPostgresErrorCode(error) === '23505') { - return { - success: false, - error: 'A folder with this name already exists in this location', - errorCode: 'conflict', - } - } - logger.error('Failed to create workflow folder', { error }) - return { success: false, error: 'Internal server error', errorCode: 'internal' } - } -} - -export async function performUpdateFolder( - params: PerformUpdateFolderParams -): Promise { - try { - if (params.parentId && params.parentId === params.folderId) { - return { success: false, error: 'Folder cannot be its own parent', errorCode: 'validation' } - } - - if (params.parentId) { - const parentError = await assertParentFolderInWorkspace(params.parentId, params.workspaceId) - if (parentError) return { success: false, ...parentError } - - const wouldCreateCycle = await checkForCircularReference(params.folderId, params.parentId) - if (wouldCreateCycle) { - return { - success: false, - error: 'Cannot create circular folder reference', - errorCode: 'validation', - } - } - } - - // Typed against the table rather than `Record`: the loose type is what - // let `color`/`isExpanded` survive the cutover here after the create path dropped them. - const updates: Partial = { updatedAt: new Date() } - if (params.name !== undefined) updates.name = params.name.trim() - if (params.locked !== undefined) updates.locked = params.locked - if (params.parentId !== undefined) updates.parentId = params.parentId || null - if (params.sortOrder !== undefined) updates.sortOrder = params.sortOrder - - const [folder] = await db - .update(folderTable) - .set(updates) - .where( - and( - eq(folderTable.id, params.folderId), - eq(folderTable.workspaceId, params.workspaceId), - eq(folderTable.resourceType, 'workflow') - ) - ) - .returning() - - if (!folder) { - return { success: false, error: 'Folder not found', errorCode: 'not_found' } - } - - logger.info('Updated workflow folder', { folderId: params.folderId, updates }) - - return { success: true, folder } - } catch (error) { - if (getPostgresErrorCode(error) === '23505') { - return { - success: false, - error: 'A folder with this name already exists in this location', - errorCode: 'conflict', - } - } - logger.error('Failed to update workflow folder', { error }) - return { success: false, error: 'Internal server error', errorCode: 'internal' } - } -} - -/** - * Recursively deletes a folder: removes child folders first, archives non-archived - * workflows in each folder via {@link archiveWorkflowsByIdsInWorkspace}, then deletes - * the folder row. - */ -async function deleteFolderRecursively( - folderId: string, - workspaceId: string, - archivedAt?: Date -): Promise<{ folders: number; workflows: number }> { - const timestamp = archivedAt ?? new Date() - const stats = { folders: 0, workflows: 0 } - - const childFolders = await db - .select({ id: folderTable.id }) - .from(folderTable) - .where( - and( - eq(folderTable.parentId, folderId), - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - isNull(folderTable.deletedAt) - ) - ) - - for (const childFolder of childFolders) { - const childStats = await deleteFolderRecursively(childFolder.id, workspaceId, timestamp) - stats.folders += childStats.folders - stats.workflows += childStats.workflows - } - - const workflowsInFolder = await db - .select({ id: workflow.id }) - .from(workflow) - .where( - and( - eq(workflow.folderId, folderId), - eq(workflow.workspaceId, workspaceId), - isNull(workflow.archivedAt) - ) - ) - - if (workflowsInFolder.length > 0) { - await archiveWorkflowsByIdsInWorkspace( - workspaceId, - workflowsInFolder.map((entry) => entry.id), - { requestId: `folder-${folderId}`, archivedAt: timestamp } - ) - stats.workflows += workflowsInFolder.length - } - - await db - .update(folderTable) - .set({ deletedAt: timestamp }) - .where(and(eq(folderTable.id, folderId), eq(folderTable.resourceType, 'workflow'))) - stats.folders += 1 - - return stats -} - -/** - * Counts non-archived workflows in the folder and all descendant folders. - */ -async function countWorkflowsInFolderRecursively( - folderId: string, - workspaceId: string -): Promise { - let count = 0 - - const workflowsInFolder = await db - .select({ id: workflow.id }) - .from(workflow) - .where( - and( - eq(workflow.folderId, folderId), - eq(workflow.workspaceId, workspaceId), - isNull(workflow.archivedAt) - ) - ) +export interface PerformUpdateFolderResult extends PerformCreateFolderResult {} - count += workflowsInFolder.length - - const childFolders = await db - .select({ id: folderTable.id }) - .from(folderTable) - .where( - and( - eq(folderTable.parentId, folderId), - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - isNull(folderTable.deletedAt) - ) - ) - - for (const childFolder of childFolders) { - count += await countWorkflowsInFolderRecursively(childFolder.id, workspaceId) - } - - return count -} - -/** Parameters for {@link performDeleteFolder}. */ export interface PerformDeleteFolderParams { folderId: string workspaceId: string @@ -364,257 +48,42 @@ export interface PerformDeleteFolderParams { folderName?: string } -/** Outcome of {@link performDeleteFolder}. */ export interface PerformDeleteFolderResult { success: boolean error?: string errorCode?: OrchestrationErrorCode - deletedItems?: { folders: number; workflows: number } + deletedItems?: { folders: number; workflows?: number } } -/** - * Performs a full folder deletion: enforces the last-workflow guard, - * recursively archives child workflows and sub-folders, and records - * an audit entry. Both the folders API DELETE handler and the copilot - * delete_folder tool must use this function. - */ -export async function performDeleteFolder( - params: PerformDeleteFolderParams -): Promise { - const { folderId, workspaceId, userId, folderName } = params - - const workflowsInFolder = await countWorkflowsInFolderRecursively(folderId, workspaceId) - const totalWorkflowsInWorkspace = await db - .select({ id: workflow.id }) - .from(workflow) - .where(and(eq(workflow.workspaceId, workspaceId), isNull(workflow.archivedAt))) - - if (workflowsInFolder > 0 && workflowsInFolder >= totalWorkflowsInWorkspace.length) { - return { - success: false, - error: 'Cannot delete folder containing the only workflow(s) in the workspace', - errorCode: 'validation', - } - } - - const deletionStats = await deleteFolderRecursively(folderId, workspaceId) - - logger.info('Deleted folder and all contents:', { folderId, deletionStats }) - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.FOLDER_DELETED, - resourceType: AuditResourceType.FOLDER, - resourceId: folderId, - resourceName: folderName, - description: `Deleted folder "${folderName || folderId}"`, - metadata: { - affected: { - workflows: deletionStats.workflows, - subfolders: deletionStats.folders - 1, - }, - }, - }) +export interface PerformRestoreFolderParams extends PerformDeleteFolderParams {} - return { success: true, deletedItems: deletionStats } +export interface PerformRestoreFolderResult { + success: boolean + error?: string + errorCode?: OrchestrationErrorCode + restoredItems?: { folders: number; workflows?: number } } -/** - * Recursively restores a folder and its children/workflows within a transaction. - * Only restores workflows whose `archivedAt` matches the folder's — workflows - * individually deleted before the folder are left archived. - */ -async function restoreFolderRecursively( - folderId: string, - workspaceId: string, - folderArchivedAt: Date, - tx: Parameters[0]>[0] -): Promise<{ folders: number; workflows: number }> { - const stats = { folders: 0, workflows: 0 } - - await tx - .update(folderTable) - .set({ deletedAt: null }) - .where(and(eq(folderTable.id, folderId), eq(folderTable.resourceType, 'workflow'))) - stats.folders += 1 - - const archivedWorkflows = await tx - .select({ id: workflow.id }) - .from(workflow) - .where( - and( - eq(workflow.folderId, folderId), - eq(workflow.workspaceId, workspaceId), - eq(workflow.archivedAt, folderArchivedAt) - ) - ) - - if (archivedWorkflows.length > 0) { - const workflowIds = archivedWorkflows.map((wf) => wf.id) - const now = new Date() - const restoreSet = { archivedAt: null, updatedAt: now } - - await tx.update(workflow).set(restoreSet).where(inArray(workflow.id, workflowIds)) - await tx - .update(workflowSchedule) - .set(restoreSet) - .where(inArray(workflowSchedule.workflowId, workflowIds)) - await tx.update(webhook).set(restoreSet).where(inArray(webhook.workflowId, workflowIds)) - await tx.update(chat).set(restoreSet).where(inArray(chat.workflowId, workflowIds)) - await tx - .update(workflowMcpTool) - .set(restoreSet) - .where(inArray(workflowMcpTool.workflowId, workflowIds)) - - stats.workflows += archivedWorkflows.length - } - - const archivedChildren = await tx - .select({ id: folderTable.id }) - .from(folderTable) - .where( - and( - eq(folderTable.parentId, folderId), - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow'), - eq(folderTable.deletedAt, folderArchivedAt) - ) - ) - - for (const child of archivedChildren) { - const childStats = await restoreFolderRecursively(child.id, workspaceId, folderArchivedAt, tx) - stats.folders += childStats.folders - stats.workflows += childStats.workflows - } - - return stats +export function performCreateFolder( + params: PerformCreateFolderParams +): Promise { + return createFolder({ ...params, resourceType: 'workflow' }) } -/** Parameters for {@link performRestoreFolder}. */ -export interface PerformRestoreFolderParams { - folderId: string - workspaceId: string - userId: string - folderName?: string +export function performUpdateFolder( + params: PerformUpdateFolderParams +): Promise { + return updateFolder({ ...params, resourceType: 'workflow' }) } -/** Outcome of {@link performRestoreFolder}. */ -export interface PerformRestoreFolderResult { - success: boolean - error?: string - restoredItems?: { folders: number; workflows: number } +export function performDeleteFolder( + params: PerformDeleteFolderParams +): Promise { + return deleteFolder({ ...params, resourceType: 'workflow' }) } -/** - * Restores an archived folder and all its archived children and workflows. - * If the folder's parent is still archived, moves it to the root level. - */ -export async function performRestoreFolder( +export function performRestoreFolder( params: PerformRestoreFolderParams ): Promise { - const { folderId, workspaceId, userId, folderName } = params - - const [folder] = await db - .select() - .from(folderTable) - .where( - and( - eq(folderTable.id, folderId), - eq(folderTable.workspaceId, workspaceId), - eq(folderTable.resourceType, 'workflow') - ) - ) - - if (!folder) { - return { success: false, error: 'Folder not found' } - } - - if (!folder.deletedAt) { - return { success: true, restoredItems: { folders: 0, workflows: 0 } } - } - - 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' } - } - - let restoredStats: { folders: number; workflows: number } - try { - restoredStats = await db.transaction(async (tx) => { - // A folder whose parent is still archived is re-rooted, so the name it has to be - // unique against is its *resolved* parent's sibling set, not its original one. - let resolvedParentId = folder.parentId - if (folder.parentId) { - const [parentFolder] = await tx - .select({ archivedAt: folderTable.deletedAt }) - .from(folderTable) - .where(and(eq(folderTable.id, folder.parentId), eq(folderTable.resourceType, 'workflow'))) - - if (!parentFolder || parentFolder.archivedAt) { - resolvedParentId = null - await tx - .update(folderTable) - .set({ parentId: null }) - .where(and(eq(folderTable.id, folderId), eq(folderTable.resourceType, 'workflow'))) - } - } - - // Restore is a recovery action — the caller cannot rename an archived folder, so a - // name already taken by an active sibling would leave them permanently unable to - // restore. Dedup instead of failing. Safe to rename while the row is still archived: - // the unique index only covers active rows, so this cannot collide before the - // recursive restore below clears `deletedAt`. Only the restore root can conflict — - // descendants come back alongside the siblings they were archived with. - const restoredName = await deduplicateFolderName( - tx, - workspaceId, - resolvedParentId, - folder.name - ) - if (restoredName !== folder.name) { - logger.info('Renamed folder on restore to avoid a sibling name conflict', { - folderId, - from: folder.name, - to: restoredName, - }) - await tx.update(folderTable).set({ name: restoredName }).where(eq(folderTable.id, folderId)) - } - - return restoreFolderRecursively(folderId, workspaceId, folder.deletedAt!, tx) - }) - } catch (error) { - // Restoring clears `deletedAt`, which brings the row back under the generic table's - // partial unique index on active (workspaceId, resourceType, parent, name) — a - // constraint `workflow_folder` never had. If a sibling has since taken the name, report - // it as a conflict rather than a 500, so the caller can rename and retry. - if (getPostgresErrorCode(error) === '23505') { - return { - success: false, - error: 'A folder with this name already exists in this location', - } - } - throw error - } - - logger.info('Restored folder and all contents:', { folderId, restoredStats }) - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.FOLDER_RESTORED, - resourceType: AuditResourceType.FOLDER, - resourceId: folderId, - resourceName: folderName ?? folder.name, - description: `Restored folder "${folderName ?? folder.name}"`, - metadata: { - affected: { - workflows: restoredStats.workflows, - subfolders: restoredStats.folders - 1, - }, - }, - }) - - return { success: true, restoredItems: restoredStats } + return restoreFolder({ ...params, resourceType: 'workflow' }) } diff --git a/apps/sim/lib/workflows/utils.ts b/apps/sim/lib/workflows/utils.ts index 682a736d951..8a4d91dbf78 100644 --- a/apps/sim/lib/workflows/utils.ts +++ b/apps/sim/lib/workflows/utils.ts @@ -510,36 +510,6 @@ export async function verifyFolderWorkspace( return Boolean(row) } -/** - * Walks the parent chain upward from `parentId` to check whether re-parenting `folderId` - * under it would form a cycle. Returns true when a cycle would be created. - */ -export async function checkForCircularReference( - folderId: string, - parentId: string -): Promise { - let currentParentId: string | null = parentId - const visited = new Set() - - while (currentParentId) { - if (visited.has(currentParentId) || currentParentId === folderId) { - return true - } - - visited.add(currentParentId) - - const [parent] = await db - .select({ parentId: folderTable.parentId }) - .from(folderTable) - .where(and(eq(folderTable.id, currentParentId), eq(folderTable.resourceType, 'workflow'))) - .limit(1) - - currentParentId = parent?.parentId || null - } - - return false -} - export async function listFolders(workspaceId: string) { return db .select({ diff --git a/packages/testing/src/mocks/folders-lifecycle.mock.ts b/packages/testing/src/mocks/folders-lifecycle.mock.ts new file mode 100644 index 00000000000..52a3a281144 --- /dev/null +++ b/packages/testing/src/mocks/folders-lifecycle.mock.ts @@ -0,0 +1,35 @@ +import { vi } from 'vitest' + +/** + * Controllable mock functions for `@/lib/folders/lifecycle` — the generic, + * resourceType-driven folder engine behind every `/api/folders` route. + * All defaults are bare `vi.fn()` — configure per-test as needed. + * + * @example + * ```ts + * import { foldersLifecycleMockFns } from '@sim/testing' + * + * foldersLifecycleMockFns.mockCreateFolder.mockResolvedValue({ success: true, folder }) + * ``` + */ +export const foldersLifecycleMockFns = { + mockCreateFolder: vi.fn(), + mockUpdateFolder: vi.fn(), + mockDeleteFolder: vi.fn(), + mockRestoreFolder: vi.fn(), +} + +/** + * Static mock module for `@/lib/folders/lifecycle`. + * + * @example + * ```ts + * vi.mock('@/lib/folders/lifecycle', () => foldersLifecycleMock) + * ``` + */ +export const foldersLifecycleMock = { + createFolder: foldersLifecycleMockFns.mockCreateFolder, + updateFolder: foldersLifecycleMockFns.mockUpdateFolder, + deleteFolder: foldersLifecycleMockFns.mockDeleteFolder, + restoreFolder: foldersLifecycleMockFns.mockRestoreFolder, +} diff --git a/packages/testing/src/mocks/index.ts b/packages/testing/src/mocks/index.ts index 2c60725e703..92b192fc9fc 100644 --- a/packages/testing/src/mocks/index.ts +++ b/packages/testing/src/mocks/index.ts @@ -92,6 +92,8 @@ export { mockNextFetchResponse, setupGlobalFetchMock, } from './fetch.mock' +// Generic folder engine mocks (for @/lib/folders/lifecycle) +export { foldersLifecycleMock, foldersLifecycleMockFns } from './folders-lifecycle.mock' // Hybrid auth mocks export { hybridAuthMock, hybridAuthMockFns } from './hybrid-auth.mock' // Input validation mocks diff --git a/packages/testing/src/mocks/workflows-utils.mock.ts b/packages/testing/src/mocks/workflows-utils.mock.ts index 4065d44fabf..328502836f1 100644 --- a/packages/testing/src/mocks/workflows-utils.mock.ts +++ b/packages/testing/src/mocks/workflows-utils.mock.ts @@ -24,7 +24,6 @@ export const workflowsUtilsMockFns = { mockUpdateWorkflowRecord: vi.fn(), mockDeleteWorkflowRecord: vi.fn(), mockSetWorkflowVariables: vi.fn(), - mockCheckForCircularReference: vi.fn(), mockListFolders: vi.fn(), } @@ -58,6 +57,5 @@ export const workflowsUtilsMock = { updateWorkflowRecord: workflowsUtilsMockFns.mockUpdateWorkflowRecord, deleteWorkflowRecord: workflowsUtilsMockFns.mockDeleteWorkflowRecord, setWorkflowVariables: workflowsUtilsMockFns.mockSetWorkflowVariables, - checkForCircularReference: workflowsUtilsMockFns.mockCheckForCircularReference, listFolders: workflowsUtilsMockFns.mockListFolders, } From a28eb45300558cc253246c42b2d3433a7708d556 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 18:29:49 -0700 Subject: [PATCH 2/5] fix(folders): route kb and table cascades through their canonical delete Review found three ways the generic cascade diverged from the behavior each resource defines for itself. - Deleting an already-archived folder stamped its still-active children with a fresh timestamp while the folder row kept the original. Restore matches the folder's stamp, so those children were stranded permanently. Delete now reuses the folder's existing deletedAt. - A knowledge base is more than its own row: deleteKnowledgeBase also archives its documents and pauses its connectors. The generic row update left that graph live, so a connector kept syncing into a KB the UI showed as deleted. The kb config now delegates to the canonical delete and restore. - deleteTable refuses to archive a delete-locked table, because archiving destroys access to every row. Deleting the folder around it bypassed that control entirely. Tables now declare a guardDelete that refuses the whole folder up front, so the cascade never archives half a subtree and then stops. deleteKnowledgeBase and deleteTable gain an optional archivedAt so a bulk caller can stamp one shared timestamp, matching archiveWorkflow's existing option; single-resource callers are unchanged. Adds the restoreChildren hook that pairs with archiveChildren, sequenced after the folder transaction commits since the canonical restores open their own. Also: make resourceType a required argument on deduplicateFolderName and listFoldersForWorkspace so a future caller cannot silently operate on the workflow tree, and give folder routes one shared status mapper rather than three copies that already disagreed. --- .../app/api/folders/[id]/duplicate/route.ts | 3 +- .../sim/app/api/folders/[id]/restore/route.ts | 4 +- apps/sim/app/api/folders/[id]/route.test.ts | 19 ++ apps/sim/app/api/folders/[id]/route.ts | 16 +- apps/sim/app/api/folders/route.ts | 8 +- .../workspace/[workspaceId]/home/prefetch.ts | 4 +- .../[workspaceId]/lib/prefetch.test.ts | 2 +- .../app/workspace/[workspaceId]/prefetch.ts | 4 +- apps/sim/lib/folders/cascade.test.ts | 64 ++++++ apps/sim/lib/folders/cascade.ts | 57 +++++- apps/sim/lib/folders/config.ts | 189 ++++++++++++++++-- apps/sim/lib/folders/lifecycle.ts | 62 +++++- apps/sim/lib/folders/naming.ts | 2 +- apps/sim/lib/folders/queries.ts | 2 +- apps/sim/lib/folders/status.ts | 25 +++ apps/sim/lib/knowledge/service.ts | 10 +- apps/sim/lib/table/service.ts | 9 +- .../orchestration/folder-lifecycle.ts | 3 +- 18 files changed, 422 insertions(+), 61 deletions(-) create mode 100644 apps/sim/lib/folders/status.ts diff --git a/apps/sim/app/api/folders/[id]/duplicate/route.ts b/apps/sim/app/api/folders/[id]/duplicate/route.ts index d846ebdf3f8..2e481ec551c 100644 --- a/apps/sim/app/api/folders/[id]/duplicate/route.ts +++ b/apps/sim/app/api/folders/[id]/duplicate/route.ts @@ -112,7 +112,8 @@ export const POST = withRouteHandler( tx, targetWorkspaceId, targetParentId, - name + name, + 'workflow' ) await tx.insert(folderTable).values({ diff --git a/apps/sim/app/api/folders/[id]/restore/route.ts b/apps/sim/app/api/folders/[id]/restore/route.ts index 9f0331e86cb..0022d3e8c8c 100644 --- a/apps/sim/app/api/folders/[id]/restore/route.ts +++ b/apps/sim/app/api/folders/[id]/restore/route.ts @@ -6,8 +6,8 @@ import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { restoreFolder } from '@/lib/folders/lifecycle' +import { folderMutationStatus } from '@/lib/folders/status' import { captureServerEvent } from '@/lib/posthog/server' -import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('RestoreFolderAPI') @@ -41,7 +41,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route if (!result.success) { return NextResponse.json( { error: result.error }, - { status: statusForOrchestrationError(result.errorCode) } + { status: folderMutationStatus(result.errorCode) } ) } diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index 7d53a2e62f7..cf7efcb6ad2 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -407,6 +407,25 @@ describe('Individual Folder API Route', () => { }) }) + it('surfaces a delete-locked resource as 423, not a generic 500', async () => { + mockAuthenticatedUser() + queueFolderLookup() + mockDeleteFolder.mockResolvedValueOnce({ + success: false, + error: 'Cannot delete folder: table Ledger is delete-locked', + errorCode: 'locked', + }) + + const req = createMockRequest('DELETE') + const params = Promise.resolve({ id: 'folder-1' }) + + const response = await DELETE(req, { params }) + + expect(response.status).toBe(423) + const data = await response.json() + expect(data.error).toBe('Cannot delete folder: table Ledger is delete-locked') + }) + it('should return 401 for unauthenticated delete requests', async () => { mockUnauthenticated() diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index 5ca48a240b9..dad0f3236fd 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -10,17 +10,10 @@ import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle' import { toFolderApi } from '@/lib/folders/queries' +import { folderMutationStatus } from '@/lib/folders/status' import { captureServerEvent } from '@/lib/posthog/server' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' -/** Maps an orchestration errorCode to its HTTP status; mirrors the POST /api/folders route. */ -function folderMutationStatus(errorCode: string | undefined): number { - if (errorCode === 'validation') return 400 - if (errorCode === 'conflict') return 409 - if (errorCode === 'not_found') return 404 - return 500 -} - const logger = createLogger('FoldersIDAPI') // PUT - Update a folder @@ -172,9 +165,10 @@ export const DELETE = withRouteHandler( }) if (!result.success) { - const status = - result.errorCode === 'not_found' ? 404 : result.errorCode === 'validation' ? 400 : 500 - return NextResponse.json({ error: result.error }, { status }) + return NextResponse.json( + { error: result.error }, + { status: folderMutationStatus(result.errorCode) } + ) } captureServerEvent( diff --git a/apps/sim/app/api/folders/route.ts b/apps/sim/app/api/folders/route.ts index 3d8852eb67a..0fac766bce6 100644 --- a/apps/sim/app/api/folders/route.ts +++ b/apps/sim/app/api/folders/route.ts @@ -7,18 +7,12 @@ import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { createFolder } from '@/lib/folders/lifecycle' import { listFoldersForWorkspace, toFolderApi } from '@/lib/folders/queries' +import { folderMutationStatus } from '@/lib/folders/status' import { captureServerEvent } from '@/lib/posthog/server' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('FoldersAPI') -function folderMutationStatus(errorCode: string | undefined): number { - if (errorCode === 'validation') return 400 - if (errorCode === 'conflict') return 409 - if (errorCode === 'not_found') return 404 - return 500 -} - // GET - Fetch folders for a workspace export const GET = withRouteHandler(async (request: NextRequest) => { try { diff --git a/apps/sim/app/workspace/[workspaceId]/home/prefetch.ts b/apps/sim/app/workspace/[workspaceId]/home/prefetch.ts index adf7a42630b..508eca5002e 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/prefetch.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/prefetch.ts @@ -28,10 +28,10 @@ export async function prefetchHomeLists( ): Promise { await Promise.all([ queryClient.prefetchQuery({ - queryKey: folderKeys.list(workspaceId, 'active'), + queryKey: folderKeys.list(workspaceId, 'active', 'workflow'), queryFn: async () => { const { folders } = await prefetchInternalJson<{ folders?: FolderApi[] }>( - `/api/folders?workspaceId=${workspaceId}&scope=active` + `/api/folders?workspaceId=${workspaceId}&scope=active&resourceType=workflow` ) return (folders ?? []).map(mapFolder) }, diff --git a/apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts b/apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts index f5317f1d5f9..7c5257c47a5 100644 --- a/apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts @@ -126,7 +126,7 @@ describe('workspace list prefetches', () => { await prefetchHomeLists(client, WORKSPACE_ID) expect(mockPrefetchInternalJson).toHaveBeenCalledWith( - `/api/folders?workspaceId=${WORKSPACE_ID}&scope=active` + `/api/folders?workspaceId=${WORKSPACE_ID}&scope=active&resourceType=workflow` ) const cachedFolders = client.getQueryData(folderKeys.list(WORKSPACE_ID, 'active')) as Array<{ id: string diff --git a/apps/sim/app/workspace/[workspaceId]/prefetch.ts b/apps/sim/app/workspace/[workspaceId]/prefetch.ts index f8e2d3c3afe..425e53831ee 100644 --- a/apps/sim/app/workspace/[workspaceId]/prefetch.ts +++ b/apps/sim/app/workspace/[workspaceId]/prefetch.ts @@ -83,9 +83,9 @@ export async function prefetchWorkspaceSidebar( staleTime: MOTHERSHIP_CHAT_LIST_STALE_TIME, }), queryClient.prefetchQuery({ - queryKey: folderKeys.list(workspaceId, 'active'), + queryKey: folderKeys.list(workspaceId, 'active', 'workflow'), queryFn: async () => { - const rows = await listFoldersForWorkspace(workspaceId, 'active') + const rows = await listFoldersForWorkspace(workspaceId, 'active', 'workflow') return rows.map(mapFolder) }, staleTime: FOLDER_LIST_STALE_TIME, diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 69b4940c70a..9c7c22b8e94 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -8,9 +8,11 @@ import { collectArchivedSubtreeIds, type DbOrTx, restoreFolderCascade, + restoreFolderRows, toCascadeCounts, } from '@/lib/folders/cascade' import { FOLDER_RESOURCES, type FolderResourceConfig } from '@/lib/folders/config' +import { folderMutationStatus } from '@/lib/folders/status' interface SelectCall { where: unknown @@ -190,6 +192,18 @@ 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' }], []] }) + + 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 }) + }) + it('delegates to archiveChildren when a resource archives through its own lifecycle', async () => { const archiveChildren = vi.fn().mockResolvedValue(7) const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }]] }) @@ -302,6 +316,39 @@ describe('restoreFolderCascade', () => { }) }) +describe('restoreFolderRows', () => { + it('restores only folders carrying the cascade timestamp', async () => { + const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }, { id: 'sub' }]] }) + + const folders = await restoreFolderRows( + tx, + makeConfig(), + 'ws-1', + ['root', 'sub'], + TIMESTAMP, + NOW + ) + + expect(folders).toBe(2) + expect(updateCalls).toHaveLength(1) + expect(hasCondition(updateCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true) + }) +}) + +describe('folderMutationStatus', () => { + it('maps a locked resource to 423, matching the single-resource delete', () => { + expect(folderMutationStatus('locked')).toBe(423) + }) + + it('maps the shared orchestration codes', () => { + expect(folderMutationStatus('validation')).toBe(400) + expect(folderMutationStatus('not_found')).toBe(404) + expect(folderMutationStatus('conflict')).toBe(409) + expect(folderMutationStatus('internal')).toBe(500) + expect(folderMutationStatus(undefined)).toBe(500) + }) +}) + describe('toCascadeCounts', () => { it('reports the child count under the resource’s own key', () => { expect(toCascadeCounts(makeConfig(), { folders: 2, children: 3 })).toEqual({ @@ -319,6 +366,23 @@ describe('FOLDER_RESOURCES', () => { vi.clearAllMocks() }) + it('pairs every archiveChildren hook with a restoreChildren hook', () => { + // An archive hook exists because archiving touches more than the child row. Without the + // matching restore hook, a folder restore would revive the resource and strand whatever + // the archive hook took down with it. + for (const config of Object.values(FOLDER_RESOURCES)) { + if (!config.archiveChildren) continue + const hasRestorePath = Boolean(config.restoreChildren) || Boolean(config.restoreDependents) + expect(hasRestorePath).toBe(true) + } + }) + + it('guards the delete of resources that gate their own deletion', () => { + // Tables refuse deletion while delete-locked; deleting the folder around one must not + // become a way around that control. + expect(FOLDER_RESOURCES.table.guardDelete).toBeDefined() + }) + it('declares one entry per folder resource type', () => { expect(Object.keys(FOLDER_RESOURCES).sort()).toEqual([ 'file', diff --git a/apps/sim/lib/folders/cascade.ts b/apps/sim/lib/folders/cascade.ts index 82ae303f73b..c29abe40627 100644 --- a/apps/sim/lib/folders/cascade.ts +++ b/apps/sim/lib/folders/cascade.ts @@ -121,21 +121,19 @@ export async function archiveFolderCascade( } /** - * Restores `folderIds` — the subtree resolved by {@link collectArchivedSubtreeIds} — plus - * every resource archived at the same `timestamp`, then reactivates the dependent rows - * (schedules, webhooks, chats) hanging off those resources. + * Un-archives the folder rows of a restore cascade — the subtree resolved by + * {@link collectArchivedSubtreeIds}, matched on the exact `timestamp`. * - * Fixed query count regardless of subtree depth: one UPDATE for the folders, one for the - * resources, and one per declared dependent table. + * One statement regardless of subtree depth. Returns how many folders came back. */ -export async function restoreFolderCascade( +export async function restoreFolderRows( tx: DbOrTx, config: FolderResourceConfig, workspaceId: string, folderIds: string[], timestamp: Date, now: Date -): Promise { +): Promise { const restoredFolders = await tx .update(folderTable) .set({ deletedAt: null, updatedAt: now }) @@ -149,13 +147,33 @@ export async function restoreFolderCascade( ) .returning({ id: folderTable.id }) + return restoredFolders.length +} + +/** + * Un-archives the resources a restore cascade covers, plus the dependent rows (schedules, + * webhooks, chats) hanging off them — the default path, for resources whose restore is a + * plain row update. + * + * Fixed statement count regardless of subtree depth: one UPDATE for the resources and one + * per declared dependent table. Resources whose restore needs more than this declare + * {@link FolderResourceConfig.restoreChildren} instead and never reach here. + */ +export async function restoreFolderChildren( + tx: DbOrTx, + config: FolderResourceConfig, + workspaceId: string, + folderIds: string[], + timestamp: Date, + now: Date +): Promise { const restoredChildren = await tx .update(config.table) .set(config.buildSoftDeleteSet(null, now)) .where(and(childFilter(config, workspaceId, folderIds), eq(config.deletedColumn, timestamp))) .returning({ id: config.idColumn }) - const childIds = restoredChildren.map((row) => row.id as string) + const childIds = restoredChildren.map((row) => String(row.id)) if (childIds.length > 0) { for (const dependent of config.restoreDependents ?? []) { @@ -168,7 +186,28 @@ export async function restoreFolderCascade( } } - return { folders: restoredFolders.length, children: childIds.length } + return childIds.length +} + +/** + * Restores a folder subtree and the resources inside it, via the default row-update path. + * + * Only valid for resources without a {@link FolderResourceConfig.restoreChildren} hook — + * those hooks call canonical single-resource restores that open their own transactions and + * therefore must not run nested inside this one. `restoreFolder` sequences that case itself. + */ +export async function restoreFolderCascade( + tx: DbOrTx, + config: FolderResourceConfig, + workspaceId: string, + folderIds: string[], + timestamp: Date, + now: Date +): Promise { + const folders = await restoreFolderRows(tx, config, workspaceId, folderIds, timestamp, now) + const children = await restoreFolderChildren(tx, config, workspaceId, folderIds, timestamp, now) + + return { folders, children } } /** diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 072a412f3d8..70b34e6e971 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -37,19 +37,22 @@ export interface FolderDependentTable { buildRestoreSet: (now: Date) => Record } -/** Everything the cascade needs to archive the resources inside a folder. */ -export interface ArchiveChildrenContext { +/** Everything the cascade needs to archive or restore the resources inside a folder. */ +export interface CascadeChildrenContext { workspaceId: string - /** The folder plus every active descendant folder, already resolved. */ + /** The folder plus every descendant folder in the cascade, already resolved. */ folderIds: string[] /** Shared across the whole cascade; restore matches on it exactly. */ timestamp: Date } -/** Reason a delete must be refused, in the orchestration error vocabulary. */ +/** + * Reason a delete must be refused. `locked` maps to 423, matching what the table domain + * returns when a mutation lock blocks the equivalent single-resource delete. + */ export interface FolderDeleteRejection { error: string - errorCode: 'validation' | 'conflict' + errorCode: 'validation' | 'conflict' | 'locked' } /** @@ -98,13 +101,28 @@ export interface FolderResourceConfig { restoreDependents?: FolderDependentTable[] /** * Replaces the cascade's default "one UPDATE over the child table" when archiving a - * resource has side effects the row update cannot express. Returns the number of - * resources archived. + * resource has side effects the row update cannot express — dependent graphs, lock + * checks, deployment teardown. Returns the number of resources archived. + * + * Where a canonical single-resource delete already exists, delegate to it rather than + * reimplementing its writes here: that is what keeps the folder cascade from drifting + * away from the resource's own delete path. */ - archiveChildren?: (context: ArchiveChildrenContext) => Promise + archiveChildren?: (context: CascadeChildrenContext) => Promise + /** + * Mirror of {@link archiveChildren} for restore. Required whenever `archiveChildren` is + * set and the archive touched more than the child row, otherwise a restore would revive + * the resource while leaving its dependents archived. + * + * Restoring a resource can also collide with its OWN active-name unique index (knowledge + * bases and tables both have one), which the canonical restore resolves by renaming — + * another reason to delegate rather than clear the tombstone directly. + */ + restoreChildren?: (context: CascadeChildrenContext) => Promise /** * Runs before any write on delete. Returns a rejection to refuse the delete, or `null` - * to proceed. + * to proceed. Use for invariants that must hold across the whole subtree, so the cascade + * either runs completely or not at all rather than failing partway through. */ guardDelete?: (context: { workspaceId: string @@ -124,7 +142,7 @@ async function archiveWorkflowChildren({ workspaceId, folderIds, timestamp, -}: ArchiveChildrenContext): Promise { +}: CascadeChildrenContext): Promise { const [{ db }, { and, eq: eqOp, inArray, isNull }, { archiveWorkflowsByIdsInWorkspace }] = await Promise.all([ import('@sim/db'), @@ -145,13 +163,13 @@ async function archiveWorkflowChildren({ if (workflowsInFolders.length === 0) return 0 - await archiveWorkflowsByIdsInWorkspace( + // Report what the lifecycle actually archived, not what this select saw — a workflow + // archived concurrently between the two would otherwise be double-counted. + return archiveWorkflowsByIdsInWorkspace( workspaceId, workflowsInFolders.map((entry) => entry.id), { requestId: `folder-cascade-${folderIds[0]}`, archivedAt: timestamp } ) - - return workflowsInFolders.length } /** @@ -198,6 +216,146 @@ async function guardLastWorkflows({ return null } +/** + * Selects the ids of resources inside a folder subtree whose soft-delete column is in the + * requested state — active (`null`) when archiving, or stamped with the cascade timestamp + * when restoring. + */ +async function selectChildIds( + config: FolderResourceConfig, + { workspaceId, folderIds, timestamp }: CascadeChildrenContext, + state: 'active' | 'archived' +): Promise { + const [{ db }, { and, eq: eqOp, inArray, isNull }] = await Promise.all([ + import('@sim/db'), + import('drizzle-orm'), + ]) + + const rows = await db + .select({ id: config.idColumn }) + .from(config.table) + .where( + and( + inArray(config.folderIdColumn, folderIds), + eqOp(config.workspaceColumn, workspaceId), + state === 'active' ? isNull(config.deletedColumn) : eqOp(config.deletedColumn, timestamp), + config.scope + ) + ) + + return rows.map((row) => String(row.id)) +} + +/** + * Archives the knowledge bases in a folder subtree through the canonical KB delete, which + * also archives their documents and pauses their connectors. A bare `knowledge_base` row + * update would leave that graph live — connectors would keep syncing into a KB the UI shows + * as deleted. + */ +async function archiveKnowledgeBaseChildren(context: CascadeChildrenContext): Promise { + const { deleteKnowledgeBase } = await import('@/lib/knowledge/service') + const ids = await selectChildIds(FOLDER_RESOURCES.knowledge_base, context, 'active') + + for (const id of ids) { + await deleteKnowledgeBase(id, `folder-cascade-${context.folderIds[0]}`, { + archivedAt: context.timestamp, + }) + } + + return ids.length +} + +/** + * Restores the knowledge bases this cascade archived, through the canonical KB restore so + * documents and connectors come back with them and the KB is renamed if its name was taken + * while it was gone. + */ +async function restoreKnowledgeBaseChildren(context: CascadeChildrenContext): Promise { + const { restoreKnowledgeBase } = await import('@/lib/knowledge/service') + const ids = await selectChildIds(FOLDER_RESOURCES.knowledge_base, context, 'archived') + + for (const id of ids) { + await restoreKnowledgeBase(id, `folder-cascade-${context.folderIds[0]}`) + } + + return ids.length +} + +/** + * Archives the tables in a folder subtree through the canonical table delete, so the + * `deleteLocked` guard in its WHERE clause still applies. {@link guardLockedTables} has + * already refused the whole folder if any table is locked, so this should not encounter one. + */ +async function archiveTableChildren(context: CascadeChildrenContext): Promise { + const { deleteTable } = await import('@/lib/table/service') + const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'active') + + for (const id of ids) { + await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, undefined, { + archivedAt: context.timestamp, + }) + } + + return ids.length +} + +/** + * Restores the tables this cascade archived, through the canonical table restore so a table + * whose name was taken while it was gone is renamed instead of tripping the active-name + * unique index. + */ +async function restoreTableChildren(context: CascadeChildrenContext): Promise { + const { restoreTable } = await import('@/lib/table/service') + const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'archived') + + for (const id of ids) { + await restoreTable(id, `folder-cascade-${context.folderIds[0]}`) + } + + return ids.length +} + +/** + * Refuses to delete a folder containing a delete-locked table. + * + * `deleteTable` gates archiving on `deleteLocked` because archiving destroys access to every + * row. Deleting the folder around it must not become a way to bypass that control. Checked + * across the whole subtree up front so the cascade never archives half the tables and then + * stops at a locked one. + */ +async function guardLockedTables({ + workspaceId, + folderIds, +}: { + workspaceId: string + folderIds: string[] +}): Promise { + const [{ db }, { and, eq: eqOp, inArray, isNull }] = await Promise.all([ + import('@sim/db'), + import('drizzle-orm'), + ]) + + const locked = await db + .select({ name: userTableDefinitions.name }) + .from(userTableDefinitions) + .where( + and( + inArray(userTableDefinitions.folderId, folderIds), + eqOp(userTableDefinitions.workspaceId, workspaceId), + isNull(userTableDefinitions.archivedAt), + eqOp(userTableDefinitions.deleteLocked, true) + ) + ) + + if (locked.length === 0) return null + + const names = locked.map((row) => row.name).join(', ') + return { + error: `Cannot delete folder: ${locked.length === 1 ? 'table' : 'tables'} ${names} ${locked.length === 1 ? 'is' : 'are'} delete-locked`, + errorCode: 'locked', + } +} + export const FOLDER_RESOURCES: Record = { workflow: { resourceType: 'workflow', @@ -281,6 +439,8 @@ export const FOLDER_RESOURCES: Record ({ deletedAt: timestamp, updatedAt: now }) satisfies Partial< typeof knowledgeBase.$inferInsert >, + archiveChildren: archiveKnowledgeBaseChildren, + restoreChildren: restoreKnowledgeBaseChildren, }, table: { resourceType: 'table', @@ -296,6 +456,9 @@ export const FOLDER_RESOURCES: Record ({ archivedAt: timestamp, updatedAt: now }) satisfies Partial< typeof userTableDefinitions.$inferInsert >, + archiveChildren: archiveTableChildren, + restoreChildren: restoreTableChildren, + guardDelete: guardLockedTables, }, } diff --git a/apps/sim/lib/folders/lifecycle.ts b/apps/sim/lib/folders/lifecycle.ts index ed3b5a4b251..90f85a3d875 100644 --- a/apps/sim/lib/folders/lifecycle.ts +++ b/apps/sim/lib/folders/lifecycle.ts @@ -10,12 +10,14 @@ import { archiveFolderCascade, collectActiveSubtreeIds, collectArchivedSubtreeIds, - restoreFolderCascade, + restoreFolderChildren, + restoreFolderRows, toCascadeCounts, } from '@/lib/folders/cascade' import { folderResourceConfig } from '@/lib/folders/config' import { deduplicateFolderName } from '@/lib/folders/naming' import { wouldCreateFolderCycle } from '@/lib/folders/queries' +import type { FolderMutationErrorCode } from '@/lib/folders/status' import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' const logger = createLogger('FolderLifecycle') @@ -61,7 +63,7 @@ export interface DeleteFolderParams { export interface DeleteFolderResult { success: boolean error?: string - errorCode?: OrchestrationErrorCode + errorCode?: FolderMutationErrorCode deletedItems?: FolderCascadeCountsApi } @@ -304,11 +306,32 @@ export async function updateFolder(params: UpdateFolderParams): Promise { const { resourceType, folderId, workspaceId, userId, folderName } = params const config = folderResourceConfig(resourceType) + const [existing] = await db + .select({ deletedAt: folderTable.deletedAt }) + .from(folderTable) + .where( + and( + eq(folderTable.id, folderId), + eq(folderTable.workspaceId, workspaceId), + eq(folderTable.resourceType, resourceType) + ) + ) + .limit(1) + + if (!existing) { + return { success: false, error: 'Folder not found', errorCode: 'not_found' } + } + const folderIds = await collectActiveSubtreeIds(db, workspaceId, resourceType, folderId) const rejection = await config.guardDelete?.({ workspaceId, folderIds }) @@ -316,7 +339,13 @@ export async function deleteFolder(params: DeleteFolderParams): Promise { + restored = await db.transaction(async (tx) => { const now = new Date() let resolvedParentId = folder.parentId @@ -434,7 +463,17 @@ export async function restoreFolder(params: RestoreFolderParams): Promise { const siblingRows = await tx .select({ name: folderTable.name }) diff --git a/apps/sim/lib/folders/queries.ts b/apps/sim/lib/folders/queries.ts index a9ea707b0be..897aea5b956 100644 --- a/apps/sim/lib/folders/queries.ts +++ b/apps/sim/lib/folders/queries.ts @@ -53,7 +53,7 @@ export async function wouldCreateFolderCycle( export async function listFoldersForWorkspace( workspaceId: string, scope: FolderQueryScope, - resourceType: FolderResourceType = 'workflow' + resourceType: FolderResourceType ): Promise { const scopeFilter = scope === 'archived' ? isNotNull(folder.deletedAt) : isNull(folder.deletedAt) diff --git a/apps/sim/lib/folders/status.ts b/apps/sim/lib/folders/status.ts new file mode 100644 index 00000000000..ddea423bd0b --- /dev/null +++ b/apps/sim/lib/folders/status.ts @@ -0,0 +1,25 @@ +import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' + +/** + * Folder mutations can fail for one reason the shared orchestration vocabulary has no word + * for: a resource inside the folder carries a mutation lock. Kept as a folder-local widening + * rather than pushed into `OrchestrationErrorCode`, which is shared with deployment and + * workflow surfaces that have no such state. + */ +export type FolderMutationErrorCode = OrchestrationErrorCode | 'locked' + +/** + * Maps a folder mutation error code to its HTTP status. Shared by every folder route so the + * surfaces cannot drift — `locked` in particular must reach the client as 423, matching what + * the table domain returns when the same lock blocks a single-table delete. + * + * Deliberately kept out of `lib/folders/lifecycle.ts`: this is a pure mapping with no + * database access, and routes need it to stay real in tests that mock the lifecycle module. + */ +export function folderMutationStatus(errorCode: FolderMutationErrorCode | undefined): number { + if (errorCode === 'validation') return 400 + if (errorCode === 'not_found') return 404 + if (errorCode === 'conflict') return 409 + if (errorCode === 'locked') return 423 + return 500 +} diff --git a/apps/sim/lib/knowledge/service.ts b/apps/sim/lib/knowledge/service.ts index d9717c553b1..469def4fbb1 100644 --- a/apps/sim/lib/knowledge/service.ts +++ b/apps/sim/lib/knowledge/service.ts @@ -665,12 +665,18 @@ export async function getKnowledgeBaseById( /** * Delete a knowledge base (soft delete) + * + * `options.archivedAt` lets a bulk caller stamp every row it archives with one shared + * timestamp, which is how the folder cascade later identifies exactly what it archived and + * restores that set and nothing else. Mirrors `archiveWorkflow`'s option of the same name. + * Defaults to now, so single-KB callers are unaffected. */ export async function deleteKnowledgeBase( knowledgeBaseId: string, - requestId: string + requestId: string, + options?: { archivedAt?: Date } ): Promise { - const now = new Date() + const now = options?.archivedAt ?? new Date() await db.transaction(async (tx) => { await tx.execute(sql`SELECT 1 FROM knowledge_base WHERE id = ${knowledgeBaseId} FOR UPDATE`) diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index ef55b56ca55..22bdca95170 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -747,13 +747,18 @@ export async function updateTableMetadata( * * @param tableId - Table ID to delete * @param requestId - Request ID for logging + * @param actingUserId - User performing the delete, for audit + * @param options.archivedAt - Shared timestamp for a bulk archive (folder cascade), so the + * matching restore can identify exactly the set it archived. Defaults to now, leaving + * single-table callers unaffected. Mirrors `archiveWorkflow`'s option of the same name. */ export async function deleteTable( tableId: string, requestId: string, - actingUserId?: string + actingUserId?: string, + options?: { archivedAt?: Date } ): Promise { - const now = new Date() + const now = options?.archivedAt ?? new Date() // Archiving destroys access to every row, so it is gated on the delete lock. // The guard is inline in the WHERE (atomic — no separate read, no TOCTOU); // a zero-row result is then disambiguated below (locked vs already-archived). diff --git a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts index 61058b4a660..78320b60ca6 100644 --- a/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts +++ b/apps/sim/lib/workflows/orchestration/folder-lifecycle.ts @@ -1,5 +1,6 @@ import type { folder as folderTable } from '@sim/db/schema' import { createFolder, deleteFolder, restoreFolder, updateFolder } from '@/lib/folders/lifecycle' +import type { FolderMutationErrorCode } from '@/lib/folders/status' import type { OrchestrationErrorCode } from '@/lib/workflows/orchestration/types' /** @@ -51,7 +52,7 @@ export interface PerformDeleteFolderParams { export interface PerformDeleteFolderResult { success: boolean error?: string - errorCode?: OrchestrationErrorCode + errorCode?: FolderMutationErrorCode deletedItems?: { folders: number; workflows?: number } } From 84fcd6c88a0f6e828fb7b7ca7c1e06c2f463ae61 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 28 Jul 2026 18:41:14 -0700 Subject: [PATCH 3/5] 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. --- apps/sim/app/api/folders/[id]/route.ts | 6 +++ apps/sim/lib/folders/cascade.test.ts | 36 ++++++++++++----- apps/sim/lib/folders/cascade.ts | 39 +++++++++++------- apps/sim/lib/folders/lifecycle.ts | 55 ++++++++++++-------------- 4 files changed, 83 insertions(+), 53 deletions(-) diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index dad0f3236fd..8119ffb9c57 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/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 }) } diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 9c7c22b8e94..6df5a2bc70b 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -165,11 +165,11 @@ 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' }], ], }) @@ -177,9 +177,27 @@ describe('archiveFolderCascade', () => { 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 () => { diff --git a/apps/sim/lib/folders/cascade.ts b/apps/sim/lib/folders/cascade.ts index c29abe40627..7f9b1d5acda 100644 --- a/apps/sim/lib/folders/cascade.ts +++ b/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 { - 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 } } diff --git a/apps/sim/lib/folders/lifecycle.ts b/apps/sim/lib/folders/lifecycle.ts index 90f85a3d875..afbbf7524e2 100644 --- a/apps/sim/lib/folders/lifecycle.ts +++ b/apps/sim/lib/folders/lifecycle.ts @@ -411,9 +411,29 @@ export async function restoreFolder(params: RestoreFolderParams): Promise { + 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 Date: Tue, 28 Jul 2026 18:53:45 -0700 Subject: [PATCH 4/5] fix(folders): reach nested stragglers on retry and confine locking to workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ons from the cascade reordering. The delete subtree walk still collected active folders only. Since the cascade now stamps folders before children, a failure during the child pass leaves intermediate folders archived, and an active-only walk drops them — so resources still live underneath were never reached on the retry and stayed outside every later timestamp-matched restore. The walk now admits folders that are active OR already carry this cascade's own timestamp, which reaches the stragglers while still excluding folders archived independently under a different stamp. Locking was only half-confined to workflows. `assertFolderMutable` was gated but the admin check and the `locked` write were not, so an admin could persist `locked` on a knowledge-base or table folder — a column nothing reads for those types — and a non-admin update that merely mentioned `locked` was rejected on a type where locking does not exist. Locking is now a declared capability on the resource config; the routes reject the field outright for types that lack it, and the engine refuses to write it as a backstop for the copilot tools that call it directly. --- apps/sim/app/api/folders/[id]/route.test.ts | 20 +++++++++ apps/sim/app/api/folders/[id]/route.ts | 23 ++++++++--- apps/sim/app/api/folders/reorder/route.ts | 3 +- apps/sim/app/api/folders/route.ts | 3 +- apps/sim/lib/folders/cascade.test.ts | 45 ++++++++++++++++++--- apps/sim/lib/folders/cascade.ts | 26 +++++++----- apps/sim/lib/folders/config.ts | 11 +++++ apps/sim/lib/folders/lifecycle.ts | 28 ++++++++----- 8 files changed, 127 insertions(+), 32 deletions(-) diff --git a/apps/sim/app/api/folders/[id]/route.test.ts b/apps/sim/app/api/folders/[id]/route.test.ts index cf7efcb6ad2..25db5804c3c 100644 --- a/apps/sim/app/api/folders/[id]/route.test.ts +++ b/apps/sim/app/api/folders/[id]/route.test.ts @@ -244,6 +244,26 @@ describe('Individual Folder API Route', () => { expect(data).toHaveProperty('folder') }) + it('rejects a locked write on a resource type that has no lock semantics', async () => { + mockAuthenticatedUser() + queueFolderLookup() + + const req = createMockRequest( + 'PUT', + { locked: true }, + {}, + 'http://localhost:3000/api/folders/folder-1?resourceType=knowledge_base' + ) + const params = Promise.resolve({ id: 'folder-1' }) + + const response = await PUT(req, { params }) + + expect(response.status).toBe(400) + const data = await response.json() + expect(data.error).toBe('Folder locking is only supported for workflow folders') + expect(mockUpdateFolder).not.toHaveBeenCalled() + }) + it('should return 400 when trying to set folder as its own parent', async () => { mockAuthenticatedUser() diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index 8119ffb9c57..fa9079bfc50 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -9,6 +9,7 @@ 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 { folderResourceConfig } from '@/lib/folders/config' import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle' import { toFolderApi } from '@/lib/folders/queries' import { folderMutationStatus } from '@/lib/folders/status' @@ -68,15 +69,27 @@ export const PUT = withRouteHandler( ) } - if (locked !== undefined && workspacePermission !== 'admin') { + // Locking is workflow-only. Reject the field outright for the other types rather than + // dropping it silently, and keep the admin gate and the lock checks behind the same + // capability so a non-workflow folder can neither be 403'd by a field that has no + // meaning for it nor persist a `locked` value nothing will ever read. + const supportsLocking = Boolean(folderResourceConfig(resourceType).supportsLocking) + + if (locked !== undefined && !supportsLocking) { return NextResponse.json( - { error: 'Admin access required to lock folders' }, - { status: 403 } + { error: 'Folder locking is only supported for workflow folders' }, + { status: 400 } ) } - // Folder locking is a workflow-only feature; other resource types leave `locked` false. - if (resourceType === 'workflow') { + if (supportsLocking) { + if (locked !== undefined && workspacePermission !== 'admin') { + return NextResponse.json( + { error: 'Admin access required to lock folders' }, + { status: 403 } + ) + } + const hasNonLockUpdate = Object.keys(parsed.data.body).some((key) => key !== 'locked') if (hasNonLockUpdate) { await assertFolderMutable(id) diff --git a/apps/sim/app/api/folders/reorder/route.ts b/apps/sim/app/api/folders/reorder/route.ts index 84306aa5588..a1e531be85b 100644 --- a/apps/sim/app/api/folders/reorder/route.ts +++ b/apps/sim/app/api/folders/reorder/route.ts @@ -10,6 +10,7 @@ import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { generateRequestId } from '@/lib/core/utils/request' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { folderResourceConfig } from '@/lib/folders/config' import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils' const logger = createLogger('FolderReorderAPI') @@ -116,7 +117,7 @@ export const PUT = withRouteHandler(async (req: NextRequest) => { } // Folder locking is a workflow-only feature; other resource types leave `locked` false. - if (resourceType === 'workflow') { + if (folderResourceConfig(resourceType).supportsLocking) { for (const update of validUpdates) { await assertFolderMutable(update.id) if (update.parentId !== undefined) { diff --git a/apps/sim/app/api/folders/route.ts b/apps/sim/app/api/folders/route.ts index 0fac766bce6..84eba3c8cab 100644 --- a/apps/sim/app/api/folders/route.ts +++ b/apps/sim/app/api/folders/route.ts @@ -5,6 +5,7 @@ import { createFolderContract, listFoldersContract } from '@/lib/api/contracts' import { parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { folderResourceConfig } from '@/lib/folders/config' import { createFolder } from '@/lib/folders/lifecycle' import { listFoldersForWorkspace, toFolderApi } from '@/lib/folders/queries' import { folderMutationStatus } from '@/lib/folders/status' @@ -78,7 +79,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } // Folder locking is a workflow-only feature; other resource types leave `locked` false. - if (resourceType === 'workflow') { + if (folderResourceConfig(resourceType).supportsLocking) { await assertFolderMutable(parentId ?? null) } diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 6df5a2bc70b..91c33b598dd 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -4,8 +4,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { archiveFolderCascade, - collectActiveSubtreeIds, collectArchivedSubtreeIds, + collectCascadeSubtreeIds, type DbOrTx, restoreFolderCascade, restoreFolderRows, @@ -101,8 +101,8 @@ function hasCondition( const TIMESTAMP = new Date('2026-01-01T00:00:00.000Z') const NOW = new Date('2026-02-02T00:00:00.000Z') -describe('collectActiveSubtreeIds', () => { - it('returns the folder plus every active descendant', async () => { +describe('collectCascadeSubtreeIds', () => { + it('returns the folder plus every descendant in the cascade', async () => { const { tx, selectCalls } = makeTx({ selects: [ [ @@ -114,17 +114,41 @@ describe('collectActiveSubtreeIds', () => { ], }) - const ids = await collectActiveSubtreeIds(tx, 'ws-1', 'table', 'root') + const ids = await collectCascadeSubtreeIds(tx, 'ws-1', 'table', 'root', TIMESTAMP) expect(ids).toEqual(['root', 'child', 'grandchild']) expect(selectCalls).toHaveLength(1) - expect(hasCondition(selectCalls[0].where, (node) => node.type === 'isNull')).toBe(true) + }) + + it('admits folders already stamped by this cascade so a retry reaches nested stragglers', async () => { + // The cascade stamps folders before children, so a failure during the child pass leaves + // intermediate folders archived. An active-only walk would drop `child` here and never + // reach the resources still live under it. + const { tx, selectCalls } = makeTx({ + selects: [ + [ + { id: 'root', parentId: null }, + { id: 'child', parentId: 'root' }, + { id: 'grandchild', parentId: 'child' }, + ], + ], + }) + + const ids = await collectCascadeSubtreeIds(tx, 'ws-1', 'table', 'root', TIMESTAMP) + + expect(ids).toEqual(['root', 'child', 'grandchild']) + // Either still active, or carrying this cascade's own stamp — never another snapshot's. + const clause = flattenConditions(selectCalls[0].where).find((node) => node.type === 'or') + expect(clause).toBeDefined() + const branches = (clause?.conditions ?? []) as Array> + expect(branches.some((node) => node.type === 'isNull')).toBe(true) + expect(branches.some((node) => node.right === TIMESTAMP)).toBe(true) }) it('scopes the walk to the workspace and resourceType', async () => { const { tx, selectCalls } = makeTx({ selects: [[]] }) - await collectActiveSubtreeIds(tx, 'ws-1', 'knowledge_base', 'root') + await collectCascadeSubtreeIds(tx, 'ws-1', 'knowledge_base', 'root', TIMESTAMP) expect(hasCondition(selectCalls[0].where, (node) => node.right === 'knowledge_base')).toBe(true) expect(hasCondition(selectCalls[0].where, (node) => node.right === 'ws-1')).toBe(true) @@ -395,6 +419,15 @@ describe('FOLDER_RESOURCES', () => { } }) + it('grants lock semantics to workflows only', () => { + // `folder.locked` predates the generic table and is deliberately not extended. Anything + // that flips a second resource to lockable has to add the UI and authz to match. + const lockable = Object.values(FOLDER_RESOURCES) + .filter((config) => config.supportsLocking) + .map((config) => config.resourceType) + expect(lockable).toEqual(['workflow']) + }) + it('guards the delete of resources that gate their own deletion', () => { // Tables refuse deletion while delete-locked; deleting the folder around one must not // become a way around that control. diff --git a/apps/sim/lib/folders/cascade.ts b/apps/sim/lib/folders/cascade.ts index 7f9b1d5acda..fa730b17dec 100644 --- a/apps/sim/lib/folders/cascade.ts +++ b/apps/sim/lib/folders/cascade.ts @@ -1,6 +1,6 @@ import type { db } from '@sim/db' import { folder as folderTable } from '@sim/db/schema' -import { and, eq, inArray, isNull, type SQL } from 'drizzle-orm' +import { and, eq, inArray, isNull, or, type SQL } from 'drizzle-orm' import type { FolderCascadeCountsApi, FolderResourceType } from '@/lib/api/contracts/folders' import type { FolderResourceConfig } from '@/lib/folders/config' import { collectDescendantFolderIds } from '@/lib/folders/subtree' @@ -16,28 +16,36 @@ export interface FolderCascadeCounts { } /** - * Resolves the folder plus every *active* descendant, in one query. Used by delete: an - * archived descendant was archived independently and keeps its own timestamp, so it must - * not be swept into this cascade. + * Resolves the folder plus every descendant that belongs to this delete cascade, in one + * query: folders still active, OR already stamped with this cascade's own `timestamp`. + * + * Both halves are load-bearing. Excluding other archived folders is what keeps a descendant + * archived independently — with its own timestamp — from being swept into this snapshot. + * Including folders stamped with *this* timestamp is what makes a retry work: the cascade + * stamps folders before children, so a failure partway through the child pass leaves nested + * subfolders already archived. An active-only walk would drop those intermediate folders and + * never reach the still-active resources beneath them, leaving them outside every future + * timestamp-matched restore. */ -export async function collectActiveSubtreeIds( +export async function collectCascadeSubtreeIds( tx: DbOrTx, workspaceId: string, resourceType: FolderResourceType, - folderId: string + folderId: string, + timestamp: Date ): Promise { - const activeFolders = await tx + const cascadeFolders = await tx .select({ id: folderTable.id, parentId: folderTable.parentId }) .from(folderTable) .where( and( eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, resourceType), - isNull(folderTable.deletedAt) + or(isNull(folderTable.deletedAt), eq(folderTable.deletedAt, timestamp)) ) ) - return [folderId, ...collectDescendantFolderIds(activeFolders, folderId)] + return [folderId, ...collectDescendantFolderIds(cascadeFolders, folderId)] } /** diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 70b34e6e971..6fd76dae2f0 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -89,6 +89,16 @@ export interface FolderResourceConfig { * hand the cascade a `Record` literal. */ buildSoftDeleteSet: (timestamp: Date | null, now: Date) => Record + /** + * Whether folders of this type participate in the folder-locking feature. + * + * Only workflow folders do. `folder.locked` exists because workflow-folder locking shipped + * before the generic table; it is deliberately not extended to the other resource types. + * Declared here rather than checked as `resourceType === 'workflow'` at each call site, so + * every surface that touches locking asks the same question and a future lockable resource + * is one flag rather than a hunt through routes. + */ + supportsLocking?: boolean /** Narrows which rows of `table` participate in folder membership at all. */ scope?: SQL /** @@ -404,6 +414,7 @@ export const FOLDER_RESOURCES: Record >, }, ], + supportsLocking: true, archiveChildren: archiveWorkflowChildren, guardDelete: guardLastWorkflows, }, diff --git a/apps/sim/lib/folders/lifecycle.ts b/apps/sim/lib/folders/lifecycle.ts index afbbf7524e2..b26d9f2a00e 100644 --- a/apps/sim/lib/folders/lifecycle.ts +++ b/apps/sim/lib/folders/lifecycle.ts @@ -8,8 +8,8 @@ import { and, eq, isNull, min } from 'drizzle-orm' import type { FolderCascadeCountsApi, FolderResourceType } from '@/lib/api/contracts/folders' import { archiveFolderCascade, - collectActiveSubtreeIds, collectArchivedSubtreeIds, + collectCascadeSubtreeIds, restoreFolderChildren, restoreFolderRows, toCascadeCounts, @@ -235,6 +235,8 @@ export async function createFolder(params: CreateFolderParams): Promise { + const config = folderResourceConfig(params.resourceType) + try { if (params.parentId && params.parentId === params.folderId) { return { success: false, error: 'Folder cannot be its own parent', errorCode: 'validation' } @@ -266,7 +268,9 @@ export async function updateFolder(params: UpdateFolderParams): Promise = { updatedAt: new Date() } if (params.name !== undefined) updates.name = params.name.trim() - if (params.locked !== undefined) updates.locked = params.locked + // Backstop for the route's rejection: the engine is also reachable from the copilot + // tools, and a `locked` value on a type that has no lock semantics must never persist. + if (params.locked !== undefined && config.supportsLocking) updates.locked = params.locked if (params.parentId !== undefined) updates.parentId = params.parentId || null if (params.sortOrder !== undefined) updates.sortOrder = params.sortOrder @@ -332,20 +336,24 @@ export async function deleteFolder(params: DeleteFolderParams): Promise Date: Tue, 28 Jul 2026 19:04:45 -0700 Subject: [PATCH 5/5] fix(folders): route the delete lock check through the locking capability The DELETE handler was the one branch still testing resourceType directly after locking became a declared capability, so a future lockable resource would silently skip the mutable check on delete. --- apps/sim/app/api/folders/[id]/route.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/app/api/folders/[id]/route.ts b/apps/sim/app/api/folders/[id]/route.ts index fa9079bfc50..5b237776b63 100644 --- a/apps/sim/app/api/folders/[id]/route.ts +++ b/apps/sim/app/api/folders/[id]/route.ts @@ -166,7 +166,7 @@ export const DELETE = withRouteHandler( ) } - if (resourceType === 'workflow') { + if (folderResourceConfig(resourceType).supportsLocking) { await assertFolderMutable(id) }