Skip to content

Commit 62975bf

Browse files
committed
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.
1 parent 40741fd commit 62975bf

18 files changed

Lines changed: 422 additions & 61 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ export const POST = withRouteHandler(
112112
tx,
113113
targetWorkspaceId,
114114
targetParentId,
115-
name
115+
name,
116+
'workflow'
116117
)
117118

118119
await tx.insert(folderTable).values({

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import { parseRequest } from '@/lib/api/server'
66
import { getSession } from '@/lib/auth'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { restoreFolder } from '@/lib/folders/lifecycle'
9+
import { folderMutationStatus } from '@/lib/folders/status'
910
import { captureServerEvent } from '@/lib/posthog/server'
10-
import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types'
1111
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1212

1313
const logger = createLogger('RestoreFolderAPI')
@@ -41,7 +41,7 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route
4141
if (!result.success) {
4242
return NextResponse.json(
4343
{ error: result.error },
44-
{ status: statusForOrchestrationError(result.errorCode) }
44+
{ status: folderMutationStatus(result.errorCode) }
4545
)
4646
}
4747

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,25 @@ describe('Individual Folder API Route', () => {
407407
})
408408
})
409409

410+
it('surfaces a delete-locked resource as 423, not a generic 500', async () => {
411+
mockAuthenticatedUser()
412+
queueFolderLookup()
413+
mockDeleteFolder.mockResolvedValueOnce({
414+
success: false,
415+
error: 'Cannot delete folder: table Ledger is delete-locked',
416+
errorCode: 'locked',
417+
})
418+
419+
const req = createMockRequest('DELETE')
420+
const params = Promise.resolve({ id: 'folder-1' })
421+
422+
const response = await DELETE(req, { params })
423+
424+
expect(response.status).toBe(423)
425+
const data = await response.json()
426+
expect(data.error).toBe('Cannot delete folder: table Ledger is delete-locked')
427+
})
428+
410429
it('should return 401 for unauthenticated delete requests', async () => {
411430
mockUnauthenticated()
412431

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,10 @@ import { getSession } from '@/lib/auth'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111
import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle'
1212
import { toFolderApi } from '@/lib/folders/queries'
13+
import { folderMutationStatus } from '@/lib/folders/status'
1314
import { captureServerEvent } from '@/lib/posthog/server'
1415
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1516

16-
/** Maps an orchestration errorCode to its HTTP status; mirrors the POST /api/folders route. */
17-
function folderMutationStatus(errorCode: string | undefined): number {
18-
if (errorCode === 'validation') return 400
19-
if (errorCode === 'conflict') return 409
20-
if (errorCode === 'not_found') return 404
21-
return 500
22-
}
23-
2417
const logger = createLogger('FoldersIDAPI')
2518

2619
// PUT - Update a folder
@@ -172,9 +165,10 @@ export const DELETE = withRouteHandler(
172165
})
173166

174167
if (!result.success) {
175-
const status =
176-
result.errorCode === 'not_found' ? 404 : result.errorCode === 'validation' ? 400 : 500
177-
return NextResponse.json({ error: result.error }, { status })
168+
return NextResponse.json(
169+
{ error: result.error },
170+
{ status: folderMutationStatus(result.errorCode) }
171+
)
178172
}
179173

180174
captureServerEvent(

apps/sim/app/api/folders/route.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,12 @@ import { getSession } from '@/lib/auth'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
88
import { createFolder } from '@/lib/folders/lifecycle'
99
import { listFoldersForWorkspace, toFolderApi } from '@/lib/folders/queries'
10+
import { folderMutationStatus } from '@/lib/folders/status'
1011
import { captureServerEvent } from '@/lib/posthog/server'
1112
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1213

1314
const logger = createLogger('FoldersAPI')
1415

15-
function folderMutationStatus(errorCode: string | undefined): number {
16-
if (errorCode === 'validation') return 400
17-
if (errorCode === 'conflict') return 409
18-
if (errorCode === 'not_found') return 404
19-
return 500
20-
}
21-
2216
// GET - Fetch folders for a workspace
2317
export const GET = withRouteHandler(async (request: NextRequest) => {
2418
try {

apps/sim/app/workspace/[workspaceId]/home/prefetch.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ export async function prefetchHomeLists(
2828
): Promise<void> {
2929
await Promise.all([
3030
queryClient.prefetchQuery({
31-
queryKey: folderKeys.list(workspaceId, 'active'),
31+
queryKey: folderKeys.list(workspaceId, 'active', 'workflow'),
3232
queryFn: async () => {
3333
const { folders } = await prefetchInternalJson<{ folders?: FolderApi[] }>(
34-
`/api/folders?workspaceId=${workspaceId}&scope=active`
34+
`/api/folders?workspaceId=${workspaceId}&scope=active&resourceType=workflow`
3535
)
3636
return (folders ?? []).map(mapFolder)
3737
},

apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ describe('workspace list prefetches', () => {
126126
await prefetchHomeLists(client, WORKSPACE_ID)
127127

128128
expect(mockPrefetchInternalJson).toHaveBeenCalledWith(
129-
`/api/folders?workspaceId=${WORKSPACE_ID}&scope=active`
129+
`/api/folders?workspaceId=${WORKSPACE_ID}&scope=active&resourceType=workflow`
130130
)
131131
const cachedFolders = client.getQueryData(folderKeys.list(WORKSPACE_ID, 'active')) as Array<{
132132
id: string

apps/sim/app/workspace/[workspaceId]/prefetch.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ export async function prefetchWorkspaceSidebar(
8383
staleTime: MOTHERSHIP_CHAT_LIST_STALE_TIME,
8484
}),
8585
queryClient.prefetchQuery({
86-
queryKey: folderKeys.list(workspaceId, 'active'),
86+
queryKey: folderKeys.list(workspaceId, 'active', 'workflow'),
8787
queryFn: async () => {
88-
const rows = await listFoldersForWorkspace(workspaceId, 'active')
88+
const rows = await listFoldersForWorkspace(workspaceId, 'active', 'workflow')
8989
return rows.map(mapFolder)
9090
},
9191
staleTime: FOLDER_LIST_STALE_TIME,

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ import {
88
collectArchivedSubtreeIds,
99
type DbOrTx,
1010
restoreFolderCascade,
11+
restoreFolderRows,
1112
toCascadeCounts,
1213
} from '@/lib/folders/cascade'
1314
import { FOLDER_RESOURCES, type FolderResourceConfig } from '@/lib/folders/config'
15+
import { folderMutationStatus } from '@/lib/folders/status'
1416

1517
interface SelectCall {
1618
where: unknown
@@ -190,6 +192,18 @@ describe('archiveFolderCascade', () => {
190192
}
191193
})
192194

195+
it('stamps children with the timestamp it is given, not one of its own', async () => {
196+
const { tx, updateCalls } = makeTx({ updates: [[{ id: 'child-1' }], []] })
197+
198+
await archiveFolderCascade(tx, makeConfig(), 'ws-1', ['root'], TIMESTAMP)
199+
200+
// Regression guard: deleting an already-archived folder reuses that folder's existing
201+
// deletedAt. A fresh stamp here would strand the children — the folder row keeps its
202+
// original stamp, so restore would never match them.
203+
expect(updateCalls[0].set).toEqual({ archivedAt: TIMESTAMP, updatedAt: TIMESTAMP })
204+
expect(updateCalls[1].set).toMatchObject({ deletedAt: TIMESTAMP })
205+
})
206+
193207
it('delegates to archiveChildren when a resource archives through its own lifecycle', async () => {
194208
const archiveChildren = vi.fn().mockResolvedValue(7)
195209
const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }]] })
@@ -302,6 +316,39 @@ describe('restoreFolderCascade', () => {
302316
})
303317
})
304318

319+
describe('restoreFolderRows', () => {
320+
it('restores only folders carrying the cascade timestamp', async () => {
321+
const { tx, updateCalls } = makeTx({ updates: [[{ id: 'root' }, { id: 'sub' }]] })
322+
323+
const folders = await restoreFolderRows(
324+
tx,
325+
makeConfig(),
326+
'ws-1',
327+
['root', 'sub'],
328+
TIMESTAMP,
329+
NOW
330+
)
331+
332+
expect(folders).toBe(2)
333+
expect(updateCalls).toHaveLength(1)
334+
expect(hasCondition(updateCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true)
335+
})
336+
})
337+
338+
describe('folderMutationStatus', () => {
339+
it('maps a locked resource to 423, matching the single-resource delete', () => {
340+
expect(folderMutationStatus('locked')).toBe(423)
341+
})
342+
343+
it('maps the shared orchestration codes', () => {
344+
expect(folderMutationStatus('validation')).toBe(400)
345+
expect(folderMutationStatus('not_found')).toBe(404)
346+
expect(folderMutationStatus('conflict')).toBe(409)
347+
expect(folderMutationStatus('internal')).toBe(500)
348+
expect(folderMutationStatus(undefined)).toBe(500)
349+
})
350+
})
351+
305352
describe('toCascadeCounts', () => {
306353
it('reports the child count under the resource’s own key', () => {
307354
expect(toCascadeCounts(makeConfig(), { folders: 2, children: 3 })).toEqual({
@@ -319,6 +366,23 @@ describe('FOLDER_RESOURCES', () => {
319366
vi.clearAllMocks()
320367
})
321368

369+
it('pairs every archiveChildren hook with a restoreChildren hook', () => {
370+
// An archive hook exists because archiving touches more than the child row. Without the
371+
// matching restore hook, a folder restore would revive the resource and strand whatever
372+
// the archive hook took down with it.
373+
for (const config of Object.values(FOLDER_RESOURCES)) {
374+
if (!config.archiveChildren) continue
375+
const hasRestorePath = Boolean(config.restoreChildren) || Boolean(config.restoreDependents)
376+
expect(hasRestorePath).toBe(true)
377+
}
378+
})
379+
380+
it('guards the delete of resources that gate their own deletion', () => {
381+
// Tables refuse deletion while delete-locked; deleting the folder around one must not
382+
// become a way around that control.
383+
expect(FOLDER_RESOURCES.table.guardDelete).toBeDefined()
384+
})
385+
322386
it('declares one entry per folder resource type', () => {
323387
expect(Object.keys(FOLDER_RESOURCES).sort()).toEqual([
324388
'file',

apps/sim/lib/folders/cascade.ts

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,21 +121,19 @@ export async function archiveFolderCascade(
121121
}
122122

123123
/**
124-
* Restores `folderIds` — the subtree resolved by {@link collectArchivedSubtreeIds} — plus
125-
* every resource archived at the same `timestamp`, then reactivates the dependent rows
126-
* (schedules, webhooks, chats) hanging off those resources.
124+
* Un-archives the folder rows of a restore cascade — the subtree resolved by
125+
* {@link collectArchivedSubtreeIds}, matched on the exact `timestamp`.
127126
*
128-
* Fixed query count regardless of subtree depth: one UPDATE for the folders, one for the
129-
* resources, and one per declared dependent table.
127+
* One statement regardless of subtree depth. Returns how many folders came back.
130128
*/
131-
export async function restoreFolderCascade(
129+
export async function restoreFolderRows(
132130
tx: DbOrTx,
133131
config: FolderResourceConfig,
134132
workspaceId: string,
135133
folderIds: string[],
136134
timestamp: Date,
137135
now: Date
138-
): Promise<FolderCascadeCounts> {
136+
): Promise<number> {
139137
const restoredFolders = await tx
140138
.update(folderTable)
141139
.set({ deletedAt: null, updatedAt: now })
@@ -149,13 +147,33 @@ export async function restoreFolderCascade(
149147
)
150148
.returning({ id: folderTable.id })
151149

150+
return restoredFolders.length
151+
}
152+
153+
/**
154+
* Un-archives the resources a restore cascade covers, plus the dependent rows (schedules,
155+
* webhooks, chats) hanging off them — the default path, for resources whose restore is a
156+
* plain row update.
157+
*
158+
* Fixed statement count regardless of subtree depth: one UPDATE for the resources and one
159+
* per declared dependent table. Resources whose restore needs more than this declare
160+
* {@link FolderResourceConfig.restoreChildren} instead and never reach here.
161+
*/
162+
export async function restoreFolderChildren(
163+
tx: DbOrTx,
164+
config: FolderResourceConfig,
165+
workspaceId: string,
166+
folderIds: string[],
167+
timestamp: Date,
168+
now: Date
169+
): Promise<number> {
152170
const restoredChildren = await tx
153171
.update(config.table)
154172
.set(config.buildSoftDeleteSet(null, now))
155173
.where(and(childFilter(config, workspaceId, folderIds), eq(config.deletedColumn, timestamp)))
156174
.returning({ id: config.idColumn })
157175

158-
const childIds = restoredChildren.map((row) => row.id as string)
176+
const childIds = restoredChildren.map((row) => String(row.id))
159177

160178
if (childIds.length > 0) {
161179
for (const dependent of config.restoreDependents ?? []) {
@@ -168,7 +186,28 @@ export async function restoreFolderCascade(
168186
}
169187
}
170188

171-
return { folders: restoredFolders.length, children: childIds.length }
189+
return childIds.length
190+
}
191+
192+
/**
193+
* Restores a folder subtree and the resources inside it, via the default row-update path.
194+
*
195+
* Only valid for resources without a {@link FolderResourceConfig.restoreChildren} hook —
196+
* those hooks call canonical single-resource restores that open their own transactions and
197+
* therefore must not run nested inside this one. `restoreFolder` sequences that case itself.
198+
*/
199+
export async function restoreFolderCascade(
200+
tx: DbOrTx,
201+
config: FolderResourceConfig,
202+
workspaceId: string,
203+
folderIds: string[],
204+
timestamp: Date,
205+
now: Date
206+
): Promise<FolderCascadeCounts> {
207+
const folders = await restoreFolderRows(tx, config, workspaceId, folderIds, timestamp, now)
208+
const children = await restoreFolderChildren(tx, config, workspaceId, folderIds, timestamp, now)
209+
210+
return { folders, children }
172211
}
173212

174213
/**

0 commit comments

Comments
 (0)