Skip to content

Commit eeda0c1

Browse files
committed
fix(db): dedupe every re-root path in the folder contract migration
The migration re-roots rows in three places and each lands them in a namespace covered by a partial unique index keyed on a coalesced nullable column, so a name that is free in the source scope can already be taken in the destination. - step 1's free-suffix probe only consulted the folder table, so a stranded row keeping the name 'X (1)' was invisible to the probe run for a stranded 'X' and both were assigned the same name - step 2 re-rooted dangling folder_ids to NULL with no dedupe at all, colliding on the workflow and workspace_files active-unique indexes - a stranded folder whose parent lives in another workspace passed the parent resolver but fails the folder_parent_resource_type_match trigger Also re-roots active children before the retention sweep hard-deletes a folder: the new ON DELETE SET NULL would otherwise collide on those same indexes, and the batch delete turns that into a permanent stall.
1 parent 82f3f32 commit eeda0c1

4 files changed

Lines changed: 328 additions & 65 deletions

File tree

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ import { prepareChatCleanup } from '@/lib/cleanup/chat-cleanup'
3232
import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
3333
import type { StorageContext } from '@/lib/uploads'
3434
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
35+
import { allocateUniqueWorkspaceFileName } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
3536
import { deleteFileMetadata } from '@/lib/uploads/server/metadata'
37+
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
3638

3739
const logger = createLogger('CleanupSoftDeletes')
3840

@@ -424,6 +426,75 @@ async function cleanupExpiredKnowledgeBases(
424426
* workspace files → S3 storage) are handled explicitly so the SELECT that drives
425427
* the external cleanup and the SELECT that drives the DB delete see the same rows.
426428
*/
429+
/**
430+
* Re-roots any still-active workflow or workspace file filed under a folder that is about to be
431+
* hard-deleted, giving it a collision-free name first.
432+
*
433+
* The `folder_id` FKs are `ON DELETE SET NULL`, so Postgres already re-roots these rows on its
434+
* own. The problem is the name: both tables carry a partial unique index keyed on
435+
* `coalesce(folder_id, '')`, so an implicit SET NULL can land a row on a name the workspace root
436+
* already holds and abort the whole DELETE with a 23505. `chunkedBatchDelete` counts that as a
437+
* failed batch and stops, and the same poison row re-fails on every later run — folder retention
438+
* would stall permanently for that workspace chunk. Renaming here leaves the SET NULL a no-op.
439+
*
440+
* An active child inside a soft-deleted folder is already an anomaly — the delete cascade
441+
* archives children — so this normally selects nothing, which is why the per-row loop is fine.
442+
*/
443+
async function reRootActiveFolderChildren(folderIds: string[], label: string): Promise<void> {
444+
if (folderIds.length === 0) return
445+
446+
const workflows = await cleanupDb
447+
.select({ id: workflow.id, name: workflow.name, workspaceId: workflow.workspaceId })
448+
.from(workflow)
449+
.where(and(inArray(workflow.folderId, folderIds), isNull(workflow.archivedAt)))
450+
451+
for (const row of workflows) {
452+
if (!row.workspaceId) continue
453+
const name = await deduplicateWorkflowName(row.name, row.workspaceId, null, cleanupDb)
454+
await cleanupDb.update(workflow).set({ folderId: null, name }).where(eq(workflow.id, row.id))
455+
}
456+
457+
const files = await cleanupDb
458+
.select({
459+
id: workspaceFiles.id,
460+
originalName: workspaceFiles.originalName,
461+
workspaceId: workspaceFiles.workspaceId,
462+
})
463+
.from(workspaceFiles)
464+
.where(
465+
and(
466+
inArray(workspaceFiles.folderId, folderIds),
467+
isNull(workspaceFiles.deletedAt),
468+
eq(workspaceFiles.context, 'workspace')
469+
)
470+
)
471+
472+
for (const row of files) {
473+
if (!row.workspaceId) continue
474+
/**
475+
* `allocateUniqueWorkspaceFileName` throws once the copy-suffix range is exhausted. Falling
476+
* back to the row id keeps the sweep running on a name that cannot collide — the opposite
477+
* failure (letting it throw) is the stall this function exists to prevent.
478+
*/
479+
let originalName: string
480+
try {
481+
originalName = await allocateUniqueWorkspaceFileName(row.workspaceId, row.originalName, null)
482+
} catch {
483+
originalName = `${row.originalName} (${row.id})`
484+
}
485+
await cleanupDb
486+
.update(workspaceFiles)
487+
.set({ folderId: null, originalName })
488+
.where(eq(workspaceFiles.id, row.id))
489+
}
490+
491+
if (workflows.length > 0 || files.length > 0) {
492+
logger.warn(
493+
`[${label}] Re-rooted ${workflows.length} workflow(s) and ${files.length} file(s) out of folders being purged`
494+
)
495+
}
496+
}
497+
427498
const CLEANUP_TARGETS = [
428499
{
429500
table: folderTable,
@@ -442,6 +513,11 @@ const CLEANUP_TARGETS = [
442513
'knowledge_base',
443514
'table',
444515
]),
516+
onBatch: (rows: { id: string }[]) =>
517+
reRootActiveFolderChildren(
518+
rows.map(({ id }) => id),
519+
'cleanup'
520+
),
445521
name: 'folder',
446522
},
447523
{
@@ -702,6 +778,7 @@ export async function runCleanupSoftDeletes(payload: CleanupJobPayload): Promise
702778
tableName: `${label}/${target.name}`,
703779
requireTimestampNotNull: true,
704780
additionalPredicate: 'additionalPredicate' in target ? target.additionalPredicate : undefined,
781+
onBatch: 'onBatch' in target ? target.onBatch : undefined,
705782
dbClient: cleanupDb,
706783
})
707784
totalDeleted += result.deleted

apps/sim/lib/cleanup/batch-delete.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,12 @@ export interface BatchDeleteOptions {
214214
* so a cleanup pass only ever removes the kind it owns.
215215
*/
216216
additionalPredicate?: SQL
217+
/**
218+
* Runs on each selected batch before its DELETE, for side effects that must observe exactly
219+
* the rows about to be removed. Forwarded to `chunkedBatchDelete`; see `deleteFilter` there
220+
* for the restore-race window this opens.
221+
*/
222+
onBatch?: (rows: { id: string }[]) => Promise<void>
217223
batchSize?: number
218224
maxBatches?: number
219225
workspaceChunkSize?: number

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ function withCopySuffix(fileName: string, n: number): string {
267267
/**
268268
* Picks a display name that does not collide with an active workspace file (`original_name`).
269269
*/
270-
async function allocateUniqueWorkspaceFileName(
270+
export async function allocateUniqueWorkspaceFileName(
271271
workspaceId: string,
272272
baseName: string,
273273
folderId?: string | null

0 commit comments

Comments
 (0)